On 2/14/19 11:26 AM, tristram...@microchip.com wrote: >>>>> + /* read only dropped counters when link is not up */ >>>>> + if (p->link_just_down) >>>>> + p->link_just_down = 0; >>>>> + else if (!p->phydev.link) >>>>> + mib->cnt_ptr = dev->reg_mib_cnt; >>>> >>>> This link_just_down stuff is not clear at all. Why can the drop >>>> counters not be read when the link is up? >>> >>> All of the MIB counters, except some that may be marked by driver, do >>> not get updated when the link is down, so it is a waste of time to read >>> them. >> >> Can you use netif_running() to determine that condition? Maintaining your >> own set of variables when the PHY state machine should already determine >> the link state sounds redundant if not error prone. >> > > The driver can store the PHY device pointer passed to it when the port is > enabled. But I am a little worried that pointer can be changed or completely > gone as it is out of control of the driver.
The per-port network device is accessible from dp->slave so you can do netif_running(dp->slave) from your driver, what are you talking about here? > >>> My intention is the driver eventually reads the MIB counters at least >>> every second or faster so that the ethtool API called to show MIB >>> counters gets them from memory rather than starting a read operation. >>> For now that API is called from user space with the ethtool utility, so >>> it may not be called too often and too fast. But theoretically that >>> API can be called from a program continually. >>> >>> For simple switches that do not need to do anything special the MIB >>> read operation does not cause any issue except CPU load, for more >>> complicate switches that need to do some background operations too many >>> read operation can affect some critical functions. >> >> Some switches have a MIB autocast feature taking a snapshot which AFAIR is >> internally implemented as a fast read register with no contention on other >> registers internally, do you have something similar? > > There is no such function in the switch. Every MIB counter read has to go > through a single SPI transfer using indirect access. There are no table-like > stored values that a single SPI transfer can retrieve all. > > For i2C the access is even slower, but then I do not expect this access > mechanism is used when the switch can do more complex things. > Is that something you are considering to change for future designs? -- Florian
