On 2/6/19 12:10 AM, Joe Hershberger wrote:
On Tue, Feb 5, 2019 at 9:20 AM Carlo Caione ccaione@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@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:
- 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