On Mon, Aug 17, 2015 at 2:18 PM, David Miller <da...@davemloft.net> wrote: > > From: <shh....@gmail.com> > Date: Fri, 14 Aug 2015 12:23:40 +0800 > > > From: Shaohui Xie <shaohui....@freescale.com> > > > > Currently, if phy state is PHY_RUNNING, we always register a CHANGE > > when phy works in polling or interrupt ignored, this will make the > > adjust_link being called even the phy link did Not changed. > > > > checking the phy link to make sure the link did changed before we > > register a CHANGE, if link did not changed, we do nothing. > > > > Signed-off-by: Shaohui Xie <shaohui....@freescale.com> > > Applied, thank you.
My 3rd attempt to send this, so I apologize. Hopefully this final attempt is finally in plain text. Clearly, it's been a while... Florian had some good objections to this patch. Doing a PHY status read is a very time-consuming operation to do every iteration (it's probably thousands of cycles), and the idea is supposed to be that the read only happens in the PHY_CHANGELINK state. Every cycle of the state machine will read the status, and a change will require that the status be read twice. Worst of all, if you were to change between two links quickly, the state machine will NEVER notice the change, because it is only looking for the link up/down state to change. One solution might be to modify genphy_read_status() so that it doesn't do the dummy read up front to clear the sticky low bit. This *might* make it so that PHY_RUNNING always catches a link change. What I'm not 100% sure of is whether you can guarantee that any link change will require the link to first go down. We'd have to read the spec, though I suspect this would work. The other concern with this solution is that something might be depending on the old behavior. Certainly phy_change() would need to make sure to do that "dummy" read sometime after interrupts are enabled. Another solution is to revert this patch, and modify adjust_link() functions to only make changes if the state has changed (They all should be doing this, anyway, but I can imagine that some drivers/devices might have difficulty determining what's changed). Anyone have other ideas? I'm happy to implement the first solution, but I should note I don't have a lot of test hardware these days. Andy -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
