Messages in this thread | | | Subject | Re: [RFC PATCH] net: phy/mdio: enable mmd indirect access through phy_mii_ioctl() | From | Grygorii Strashko <> | Date | Tue, 2 Nov 2021 19:19:46 +0200 |
| |
On 02/11/2021 14:39, Russell King (Oracle) wrote: > On Tue, Nov 02, 2021 at 01:49:42AM +0100, Andrew Lunn wrote: >>> The use of the indirect registers is specific to PHYs, and we already >>> know that various PHYs don't support indirect access, and some emulate >>> access to the EEE registers - both of which are handled at the PHY >>> driver level. >> >> That is actually an interesting point. Should the ioctl call actually >> use the PHY driver read_mmd and write_mmd? Or should it go direct to >> the bus? realtek uses MII_MMD_DATA for something to do with suspend, >> and hence it uses genphy_write_mmd_unsupported(), or it has its own >> function emulating MMD operations. >> >> So maybe the ioctl handler actually needs to use __phy_read_mmd() if >> there is a phy at the address, rather than go direct to the bus? >> >> Or maybe we should just say no, you should do this all from userspace, >> by implementing C45 over C22 in userspace, the ioctl allows that, the >> kernel does not need to be involved. > > Yes and no. There's a problem accessing anything that involves some kind > of indirect or paged access with the current API - you can only do one > access under the bus lock at a time, which makes the whole thing > unreliable. We've accepted that unreliability on the grounds that this > interface is for debugging only, so if it does go wrong, you get to keep > all the pieces!
Right, MMD indirect access is 4 MDIO bus transactions.
> > The paged access case is really no different from the indirect C45 case. > They're both exactly the same type of indirect access, just using > different registers. > > That said, the MII ioctls are designed to be a bus level thing - you can > address anything on the MII bus with them. Pushing the ioctl up to the > PHY layer means we need to find the right phy device to operate on.
The phy_read_mmd/__phy_read_mmd() was the first thing i considered, but rejected exactly because of the possibility to access any MDIO device through this ioctls.
in general, it can be called with check (mii->phy_id = pl->phydev->mdio.addr)
> What > if we attempt a C45 access at an address that there isn't a phy device? > > For example, what would be the effect of trying a C45 indirect access to > a DSA switch?
in case, C22/C22 MMD It will fail to read, seems no issues, and phytool will just return 0xfffb.
First, there seems was previous attempt to do the same [1].
Also, there is some historical ... mess in this area :( There are:
- generic_mii_ioctl() - 33 users (2005, it's older), uses struct mii_if_info
- mdio_mii_ioctl() - 7 users (2009), uses struct mdio_if_info
- phy_mii_ioctl() - 29 users, including phylink (2005), need PHY to get MDIO bus
- phy_do_ioctl()->phy_mii_ioctl() - 10 users (2020)
- phy_do_ioctl_running()->phy_mii_ioctl() - 22 users (2020)
- phylink_mii_ioctl() (also calls phy_mii_ioctl(), but only for SIOCSHWTSTAMP) - 9 users, including DSA (2017) need PHY to get MDIO bus, also uses PHY for c45 detection, but any phy_id can be passed.
- SIOCSMIIREG custom implementation - 32 users
> > Personally, my feeling would be that if we want to solve this, we need > to solve this properly - we need to revise the interface so it's > possible to request the kernel to perform a group of MII operations, so > that userspace can safely access any paged/indirect register. With that > solved, there will be no issue with requiring userspace to know what > it's doing with indirect C45 accesses. >
It would require MDIO bus lock, which is not a solution, or some sort of batch processing, like for mmd: w reg1 val1 w reg2 val2 w reg1 val3 r reg2
What Kernel interface do you have in mind?
Sry, but I have to note that demand for this become terribly high, min two pings in months
[1] https://www.spinics.net/lists/netdev/msg653629.html
-- Best regards, grygorii
| |