net/netdev/netdev_ioctl.c: Add new ioctls for MMD register access

[matiamic]

Summary

This patch adds SIOCGMMDREG and SIOCSMMDREG ioctl commands with accompanying logic.
SIOCGMMDREG: Get MMD register.
SIOCGMMDREG: Set MMD register.
A new structure mmd_ioctl_data_s is introduced into include/net/if.h in order to accommodate command data.
MMD registers are an extension to the MII management interface, both are defined in IEEE 802.3.

Impact

Network drivers may implement these ioctl commands. When implemented, PHY features configurable by
MMD access can now be managed from the user application.

Testing

The testing was performed by a testing application during development of a driver for 10BASE-T1x SPI MAC-PHYs.
This mechanism was used to control PHY behavior, especially to configure 10BASE-T1S PLCA access method.

[acassis]

@matiamic please update https://nuttx.apache.org/docs/latest/components/net/netdev.html to contain this new struct and ioctl.

[xiaoxiang781216]

does the new struct and ioctl number follow Linux definition? @matiamic

[matiamic]

I think Linux uses SIOCETHTOOL exclusively to access MMD registers. The SIOCxMMDREG op is not present in Linux.

[ppisa]

The current main purpose for these operations is to allow PLCA configuration in PHY devices (802.3cg 10BASE-T1S).

Linux kernel uses NetLink for this configuration and command added into ethtool, see ethtool.git/tree/netlink/plca.c. We have looked into NuttX ethtool and it would be quite lot of rework to implement that.

The PLCA configuration operations are provided as methods of PHY

https://elixir.bootlin.com/linux/v6.16.3/source/include/linux/ethtool.h#L1289

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 include/uapi/linux/if.h includes pointer field ifru_data which points to the structure mii_ioctl_data.

NuttX provides this structure in include/net/if.h.

In Linux, it seems that MDIO_PHY_ID_C45 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 which provides information if the Clause 45 is supported.

Actual solution has advantage that devices which do not support MMD report by unsupported 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

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);

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.

[xiaoxiang781216]

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 include/uapi/linux/if.h includes pointer field ifru_data which points to the structure mii_ioctl_data.

NuttX provides this structure in include/net/if.h.

In Linux, it seems that MDIO_PHY_ID_C45 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 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

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.

[matiamic]

Thank you @ppisa for the detailed explanation and for correcting me on this.

@xiaoxiang781216 Even though the responsibility is on the driver side, I skimmed through a dozen of net drivers implementing SIOCxMIIREG ioctls and I couldn't find one that actually checks for valid phyid. The most I've seen a driver do is to mask top 11 bits.

[ppisa]

@xiaoxiang781216 and @matiamic . I think that some flags or callback into struct net_driver_s should be added which inform if Clause 22 or 45 or both are supported. Linux kernel checks that on base of the list registered PHY per network device, see (mdio->mode_support & MDIO_SUPPORTS_C45) in mdio_mii_ioctl. But we can start with such information per each MAC if it supports C45 or not.

Only thing to decide is if it should go into some struct net_driver_s field or query function. For flags, d_flags / IFF_xxx is more runtime control, state, not features. d_lltype is not flag based. So some new field or capability query function d_qeury. or it can be internal ioctl to query features which would call d_ioctl and if it is not implemented by driver it would report some default for now... I.e. C22 support.

[matiamic]

I have opened a new pull request #16926. I approached the MMD access the way it is approached in Linux using SIOCxMIIREG commands. No new ioctl commands are introduced.

[xiaoxiang781216]

@xiaoxiang781216 and @matiamic . I think that some flags or callback into struct net_driver_s should be added which inform if Clause 22 or 45 or both are supported. Linux kernel checks that on base of the list registered PHY per network device, see (mdio->mode_support & MDIO_SUPPORTS_C45) in mdio_mii_ioctl. But we can start with such information per each MAC if it supports C45 or not.

Only thing to decide is if it should go into some struct net_driver_s field or query function. For flags, d_flags / IFF_xxx is more runtime control, state, not features. d_lltype is not flag based. So some new field or capability query function d_qeury. or it can be internal ioctl to query features which would call d_ioctl and if it is not implemented by driver it would report some default for now... I.e. C22 support.

to support per netdev checksum offload, we add d_features to net_driver_s recently, we can add MDIO_SUPPORTS_C45 to this field.

[ppisa]

The Linux kernel style for MMD PHY registers access is proposed in alternative pull request #16926 which superseded this request if approved. Combining of Clause 22 and Clause 45 accesses in the single IOCTL is a little more readable but does not resources for another and NuttX specific IOCTL.

There should be followup discussion how to report which MACs are acapable of C45 accesses to PHYs. All controllers with MDIO IOCTLs support should be revised to not attempt interpret incorrect way C45 as C22 and vice versa. Current state is that there are even no checks that classical MDIO addresses and registers are in range. But it should be discussed the first and is mostly another topic than MMD and PLCA for T1S.

[matiamic]

All controllers with MDIO IOCTLs support should be revised to not attempt interpret incorrect way C45 as C22 and vice versa.

The following is a list of drivers that might need to be revisited.
Acquired with grep -lr SIOC.MIIREG . | grep -e '\./arch' -e '\./drivers' (and manually checked)

./arch/risc-v/src/mpfs/mpfs_ethernet.c
./arch/risc-v/src/litex/litex_emac.c
./arch/arm/src/gd32f4/gd32f4xx_enet.c
./arch/arm/src/lpc43xx/lpc43_ethernet.c
./arch/arm/src/s32k1xx/s32k1xx_enet.c
./arch/arm/src/imxrt/imxrt_enet.c
./arch/arm/src/sam34/sam_emac.c
./arch/arm/src/samd5e5/sam_gmac.c
./arch/arm/src/imx6/imx_enet.c
./arch/arm/src/stm32/stm32_eth.c
./arch/arm/src/s32k3xx/s32k3xx_emac.c
./arch/arm/src/lpc17xx_40xx/lpc17_40_ethernet.c
./arch/arm/src/stm32f7/stm32_ethernet.c
./arch/arm/src/at32/at32_eth.c
./arch/arm/src/stm32h7/stm32_ethernet.c
./arch/arm/src/kinetis/kinetis_enet.c
./arch/arm/src/tiva/tm4c/tm4c_ethernet.c
./arch/arm/src/sama5/sam_emacb.c
./arch/arm/src/sama5/sam_emaca.c
./arch/arm/src/sama5/sam_gmac.c
./arch/arm/src/samv7/sam_emac.c
./arch/arm/src/lpc54xx/lpc54_ethernet.c
./arch/arm/src/stm32h5/stm32_ethernet.c
./arch/renesas/src/rx65n/rx65n_eth.c
./arch/arm64/src/imx9/imx9_enet.c
./arch/arm64/src/zynq-mpsoc/zynq_enet.c
./arch/xtensa/src/esp32/esp32_emac.c
./drivers/net/lan91c111.c
./drivers/net/lan9250.c

[acassis]

@matiamic I think these @ppisa comments could be used to create the missing Documentation

[ppisa]

There is great potential to enhance NuttX networking documentation from @matiamic bachelor thesis. We have already discussed it locally and mentioned on the list. But it would not fit into GSoC time unfortunately. On the other hand, I consider final T1S state as big success myself. But it is still to be reviewed by others) as really very good, including more different chips (thanks to OnSemi provided kits and tools and other lent from VZLU who have tested more manufacturers MAC-PHY underradiation as future potential technology for their space applications.

[xiaoxiang781216]

since #16926 is merged, we can close this pr now.

[acassis]

@matiamic yes, I read Michal's bachelor thesis some weeks ago and it is really good, he explains many things about netdev that the NuttX documentation doesn't explain.

[matiamic]

Thank you @acassis, nice of you to read and compliment. As @ppisa wrote, we discussed this. I want to take parts from the thesis and transform it into NuttX docs in near future, but I will not be able to fit it into the GSoC timeline.