Thanks Florian, I'll keep the Asix PHY driver separate from ax88796 for now. Mainly to simplify testing. Let's see whether it can be used by any other MAC - can still fold it into ax88796 later.
Cheers, Michael On Wed, Apr 18, 2018 at 6:08 AM, Florian Fainelli <f.faine...@gmail.com> wrote: > On 04/17/2018 06:01 AM, Andrew Lunn wrote: >> On Tue, Apr 17, 2018 at 07:18:10AM +0200, Michael Karcher wrote: >>> [Andrew, sorry for the dup. I did hit reply-to-auhor instead of >>> reply-to-all first.] >>> >>> Andrew Lunn schrieb: >>>>>> This should really be fixed in the PHY driver, not the MAC. >>>>> >>>>> OK - do you want this separate, or as part of this series? Might have >>>>> a few side effects on more commonly used hardware, perhaps? >>>> >>>> Hi Michael >>>> >>>> What PHY driver is used? >>> The ax88796b comes with its own integrated (buggy) PHY needing this >>> workaround. This PHY has its own ID which is not known by Linux, so it is >>> using the genphy driver as fallback. >>> >>>> In the driver you can implement a .soft_reset >>>> function which first does the dummy write, and then uses >>>> genphy_soft_reset() to do the actual reset. >>> We could do that - but I dont't see the point in creating a PHY driver >>> that is only ever used by this MAC driver, just to add a single line to >>> the genphy driver. If the same PHY might be used with a different MAC, >>> you definitely would have a point there, though. >> >> >> Hi Michael >> >> We try to keep the core code clean, and put all workarounds for buggy >> hardware in drivers specific to them. It just helps keep the core code >> maintainable. >> >> I would prefer a driver specific to this PHY with the workaround. But >> lets see what Florian says. > > If you are already using the generic PHY driver, coming up with a custom > one that only overrides the soft_reset and/or config_init callback is > really not that much work, and as Andrew says, it helps make things > clearer and properly isolated. As far as where to place that driver, you > can either create a new file under drivers/net/phy/* or you can even > register a phy_driver instance from within ax88796 if that makes it any > clearer. > > FWIW, there are plenty of examples where there is a PHY driver used by a > single MAC, and that is perfectly fine, because the abstraction is still > preserved. > -- > Florian
