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

[Vladimir Oltean]

On 2/6/19 12:10 AM, Joe Hershberger wrote:
> On Tue, Feb 5, 2019 at 9:20 AM Carlo Caione <ccaione at baylibre.com> wrote:
>>
>> On 05/02/2019 00:15, Joe Hershberger wrote:
>>> On Mon, Feb 4, 2019 at 5:39 PM Vladimir Oltean <vladimir.oltean at nxp.com> wrote:
>>
>> /cut
>>>> Which brings me to my next point.
>>>> If we can't properly make the distinction between an indirect C22 MMD
>>>> access and a proper C45 MMD access, and hence not keeping proper API
>>>> compatibility with Linux kernel, aren't we better off going back to
>>>> square 1 and using phy_read_mmd_indirect and phy_write_mmd_indirect?
>>>
>>> I think we can and should make the new wrapper functions remain named
>>> phy_*_mmd_indirect and the names of the override functions in the phy
>>> driver ops should be *_mmd_indirect. The override is still for an
>>> indirect access of c45 registers, just an apparently non-standard one.
>>> It is this way in Linux as well.

I guess it is just me who is still unclear on this?
Since Russell King's patch "3b85d8d net: phy: remove the indirect MMD 
read/write methods", the Linux API is no longer like that (the 
phy_driver pointers phy_read_mmd and phy_read_mmd_indirect were merged 
into one).
Just want to make sure that everybody is on the same page and we agreed 
on API compatibility with pre-3b85d8d Linux.

>>
>> Alright then. I'll prepare a V5.
>>
>> A couple on notes:
>>
>> 1. I'd prefer the parameters of the "mdio" command to be name "rimmd"
>> and "wimmd" for "r/w indirect MMD" to keep the (twisted) logic of the
>> mdio command code of differentiating the parameters according to
>> argv[1][1] and r/w according to argv[1][0]
> 
> Is there a reason you want to keep the mmd in there? It seems implied
> by doing any access using the mdio command.
> 
> Maybe wi and ri or windirect and rindirect or wind and rind?
> 

What about exposing the indirect read as
"mii read   <addr> [<indirect_devad>.]<reg>"?
It should be clear to most people (and if it isn't, it should be 
clarified) that the legacy "mii" command is clause 22 only, therefore 
the "<mmd>.<reg>" syntactic sugar must logically mean that indirect 
access is what's going on when applied to "mii". The implementation can 
freely call phy_read_mmd_indirect if it parses such syntax.

Just my 2c. Either way, exposing an explicit command for indirect access 
means that U-boot commits long-term to not trying to implicitly know 
about, and populate, phydev->is_c45.

-Vladimir