Re: [PATCH net-next 7/9] net: phy: icplus: select page before writing control register
From: Michael Walle
Date: Wed Feb 10 2021 - 06:24:02 EST
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