xiaoxiang781216 commented on PR #16911: URL: https://github.com/apache/nuttx/pull/16911#issuecomment-3226687419
Thanks for the detailed explanation. > For direct MDIO and MDD registers access, Linux kernel provides SIOCGMIIPHY, SIOCGMIIREG, SIOCSMIIREG similar as these provided by NuttX. > > In Linux kernel, the SIOCGMIIREG, SIOCSMIIREG accepts structure [struct ifreq](https://elixir.bootlin.com/linux/v6.16.3/C/ident/ifreq) [include/uapi/linux/if.h](https://elixir.bootlin.com/linux/v6.16.3/source/include/uapi/linux/if.h#L234) includes pointer field ifru_data which points to the structure [mii_ioctl_data](https://elixir.bootlin.com/linux/v6.16.3/source/include/uapi/linux/mii.h#L178). > > NuttX provides this structure in [include/net/if.h](https://github.com/apache/nuttx/blob/master/include/net/if.h#L167). > > In Linux, it seems that [MDIO_PHY_ID_C45](https://elixir.bootlin.com/linux/v6.16.3/source/include/uapi/linux/mdio.h#L456) bit set in `phy_id` field specifies that the `mii_ioctl_data` are in fact MMD data. Then the bits 5 to 9 specify PRTAD and bits 0 to 4 DEVAD. > > It is possible to define the same mapping in NuttX and reuse SIOCGMIIREG/SIOCSMIIREG. Problem is that then all device drivers has to be visited to check if they correctly report than IEEE Std 802.3 Clause 45 MDIO Managed Devices (MMD) access is not supported. > > Another option is to define some field of other mechanism in NuttX [struct net_driver_s](https://github.com/apache/nuttx/blob/f4142626b9b4a161fc67791a9c09965da6fc6ab2/include/nuttx/net/netdev.h#L308) which provides information if the Clause 45 is supported. > > Actual solution has advantage that devices which do not support MMD report by unsupported IOCTL. > but the driver which implement SIOCGMIIREG/SIOCSMIIREG suppose to check phy_id and reject the unknown id anyway regardless whether the unknown id is related to MDD, so why do we need do the check before invoke netdev->ioctl. > In a long term, it would be better to switch to phy register access routines which do not pass the pointer to userspace into network drivers and define MDIO access inside kernel similar to the Linux solution [see](https://elixir.bootlin.com/linux/v6.16.3/source/include/linux/mdio.h#L139) > > ``` > int (*mdio_read)(struct net_device *dev, int prtad, int devad, u16 addr); > int (*mdio_write)(struct net_device *dev, int prtad, int devad, u16 addr, u16 val); > ``` > yes, after we finish the public interface definition, we can add the new callback to netdev driver. > So actual proposal is the least intrusive on the other hand defines new IOCTLs, They can be ued only as internal one and external ones can be translated to them according to Linux kernel MMD encoding rules. > > It is possible to switch to Linux kernel encoding alone, but then we need to add some flags to network devices to announce Clause 22 and Clause 45 support. > > If even that is not wanted then all network drivers with SIOCGMIIREG, SIOCSMIIREG implemented needs to be revised and ensure fail on unsupported Clause 45 access. But, it' s always the driver's responsibility to reject the unknown phy id. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
