[U-Boot] [PATCH v4 3/3] cmd: mdio: Add new parameter to access MMD PHY registers

[Vladimir Oltean]

On 25.01.2019 12:12, Carlo Caione wrote:
> On 24/01/2019 20:48, Vladimir Oltean wrote:
>> On 1/24/19 10:19 PM, Carlo Caione wrote:
>>> On 24/01/2019 20:12, Vladimir Oltean wrote:
>>>>
>>
>> I can't completely answer that, TBH I don't even know who is supposed to
>> make that distinction.
> 
> In the kernel that distinction is made by the driver itself, hence my
> question. See [0].
> 
>> For Freescale parts that is a call for the MDIO bus driver to make, for
>> good or bad (see drivers/net/fm/memac_phy.c where dev_addr is compared
>> to MDIO_DEVAD_NONE).
> 
>> And in your patch, phy_write_mmd is only a wrapper over bus->write in
>> the end, with some more logic to handle C22 indirection.
>> So my question of unifying "mdio rmmd" with "mdio read" translates into:
> 
>> Does it make sense to also handle the check with MDIO_DEVAD_NONE in
>> phy_write_mmd, instead of jumping straight ahead to perform indirection?
> 
> Honestly I'm not quite sure of all the possible implications here IMO
> the safest bet here is just to follow what's done by the kernel. Maybe
> Joe can step in about this.
> 
> In general we have 3 possible cases:
> 
> 1) your driver is doing something non-standard when accessing the MMDs
> and we deal with that using the PHY driver hooks
> 2) your PHY is C22 and you have to use the indirect method
> 3) your PHY is C45 and you can use the direct register reading (mangling
> a bit the address apparently)
> 
> The kernel is dealing with all the cases, U-Boot is only dealing with
> C22 PHYs (cases 1 and 2) because AFAICT there isn't yet a generic way to
> detect if the PHY is C22 or C45.
> 
> I'm not sure if the indirect method works also for C45 PHYs.
> 
>> The goal would then be to just call phy_write_mmd from cmd/mdio.c
>> regardless of the target PHY's clause.
> 
> Again I wrote that patch only assuming that we were going to deal with
> C22 PHYs. At this point I wonder if the C22 indirect method works also
> for C45 PHYs. If that's the case than the phy_write_mmd should already
> work regardless of the target PHY clause.
> 
> Cheers.
> 
> [0]
> https://elixir.bootlin.com/linux/latest/source/drivers/net/phy/phy-core.c#L296
> 

I'm not suggesting to use C22 indirection if the PHY already supports 
native C45 addressing. Even if that worked, it would be a pointless 
exercise in all but a few cases (like the MDIO controller does not 
support C22, but the PHY does support both C22 and C45).
I was just wondering out loud whether the introduction of the "mdio 
rmmd" command is justified or not. I now understand that using e.g. 
"mdio read 1.3" will confuse the command for clause 45 PHY's because it 
won't know whether it should access the PHY via native C45 or via 
indirect C22 (obviously it shouldn't do the latter). So in lack of a 
clear distinction mechanism, I now think that a new command truly is 
necessary for performing indirect C45 access on C22.
What I am still not convinced of, however, is whether those commands 
should be called "rmmd" and "wmmd". It is not immediately obvious from 
the command description that this is what they are for, and a user may 
attempt to use them for C45 PHY's as well, which will probably not yield 
the intended result.

-Vladimir