Messages in this thread | | | Date | Wed, 10 Feb 2021 12:14:35 +0100 | From | Michael Walle <> | Subject | Re: [PATCH net-next 7/9] net: phy: icplus: select page before writing control register |
| |
Am 2021-02-10 11:49, schrieb Russell King - ARM Linux admin: > On Wed, Feb 10, 2021 at 11:38:18AM +0100, Michael Walle wrote: >> Am 2021-02-10 11:30, schrieb Russell King - ARM Linux admin: >> > On Wed, Feb 10, 2021 at 08:03:07AM +0100, Heiner Kallweit wrote: >> > > On 09.02.2021 17:40, Michael Walle wrote: >> > > > +out: >> > > > + return phy_restore_page(phydev, oldpage, err); >> > > >> > > If a random page was set before entering config_init, do we actually >> > > want >> > > to restore it? Or wouldn't it be better to set the default page as >> > > part >> > > of initialization? >> > >> > I think you've missed asking one key question: does the paging on this >> > PHY affect the standardised registers at 0..15 inclusive, or does it >> > only affect registers 16..31? >> >> For this PHY it affects only registers >=16. But that doesn't >> invaldiate >> the point that for other PHYs this might affect all regsisters. Eg. >> ones >> where you could select between fiber and copper pages, right? > > You are modifying the code using ip101a_g_* functions, which is only > used for the IP101A and IP101G PHYs. Do these devices support fiber > in a way that change the first 16 registers?
The PHY doesn't support fiber and register 0..15 are always available regardless of the selected page for the IP101G.
genphy_() stuff will work, but the IP101G PHY driver specific functions, like interrupt and mdix will break if someone is messing with the page register from userspace.
So Heiner's point was, that there are other PHY drivers which also break when a user changes registers from userspace and no one seemed to cared about that for now.
I guess it boils down to: how hard should we try to get the driver behave correctly if the user is changing registers. Or can we just make the assumption that if the PHY driver sets the page selection to its default, all the other callbacks will work on this page.
-michael
| |