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

[Carlo Caione]

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