Skip to content

Conversation

@tinnedkarma
Copy link
Contributor

@tinnedkarma tinnedkarma commented Sep 10, 2025

Summary

Currently the mdio communication is part of the monolithic 'full netdevs'.
This commit serves as an way to add modularity for the netdevs drivers.
A new upperhalf/lowerhalf mdio device comes with this commit that manages the data transfer ofer mdio interface.

It resembles the upperhalf/lowerhalf approach of the netdevs or i2c bitbang.

It is still in the draft because there are still code style issues, but I ask you to review this PR nevertheless

Impact

A new upperhalf mdio bus is now added as a set of include/nuttx/net/mdio.h and drivers/net/mdio.c files
which "sets" the available mdio commands (read, write, reset). Acts as the bus "controller". It block the bus during the transaction and calls the apropriate lowerhalf function.

A new lowerhalf mdio bus for the stm32h7 mcu family was added which implements the actual mdio transfers.

A new Kconfig option was added MDIO_BUS in the drivers/net directory

Testing

This PR have low impact over the nuttx codebase. The new files are included to the build system based on the
CONFIG_MDIO_BUS
Currently, only stm32h7 has the new mdio bus.

@github-actions github-actions bot added Area: Networking Effects networking subsystem Arch: arm Issues related to ARM (32-bit) architecture Size: L The size of the change in this PR is large labels Sep 10, 2025
@tinnedkarma tinnedkarma force-pushed the add-mdio-bus branch 7 times, most recently from d08e96a to 61ca78d Compare September 10, 2025 13:32
@acassis
Copy link
Contributor

acassis commented Sep 10, 2025

@tinnedkarma this is great addition, this way we will have a standard way to communicate with PHY.

Could you please create a basic Documentation about this driver? Unfortunately your drivers documentation are really "shy", the best references you can use will be:

https://nuttx.apache.org/docs/latest/components/drivers/special/mtd.html
https://nuttx.apache.org/docs/latest/components/drivers/special/lcd.html

Thank again for your this great contribution

@tinnedkarma
Copy link
Contributor Author

tinnedkarma commented Sep 10, 2025

@tinnedkarma this is great addition, this way we will have a standard way to communicate with PHY.

Could you please create a basic Documentation about this driver? Unfortunately your drivers documentation are really "shy", the best references you can use will be:

https://nuttx.apache.org/docs/latest/components/drivers/special/mtd.html https://nuttx.apache.org/docs/latest/components/drivers/special/lcd.html

Thank again for your this great contribution

Hi @acassis, you are right, I'm also overdue with the documentation for some other contributions.
There are things all over the place (my intention are not to be rude, sorry), still figuring out what should get where. I've opened this pull request as draft to have feedback early on for things such as file locations, design approaches and other such big/obvious things.
I am working on the documentation in the mean time, but I would need some review over the code to know if at least I'm going the right way. If the code needs changes, the documentation will need them as well.

There are some things that differs from standard nuttx "approach". Please have a look over them.

  • The "main" struct is called mdio_bus_s, although from what I can see, nuttx have only devices but no buses, as a difference to other unix-like oses.

  • Usually, the struct that holds the ops is statically allocated (g_arch_something_ops), along with the struct that handles the hardware info/data. I see a lot of uses for the later struct, as a introspection into hardware, but the ops struct feels like littering the global variables space (much more things to search while in gdb), so my stm32 mdio driver does not offer that global ops struct.

I'll get this PR out of draft after the documentation is added, but in the mean time, can you have a look over the point above?

@github-actions github-actions bot added Area: Documentation Improvements or additions to documentation Size: XL The size of the change in this PR is very large. Consider breaking down the PR into smaller pieces. labels Sep 10, 2025
@tinnedkarma tinnedkarma marked this pull request as ready for review September 11, 2025 10:00
acassis
acassis previously approved these changes Sep 11, 2025
@acassis
Copy link
Contributor

acassis commented Sep 11, 2025

@tinnedkarma very good Documentation! Kudos!

@acassis
Copy link
Contributor

acassis commented Sep 12, 2025

ping @xiaoxiang781216

@xiaoxiang781216
Copy link
Contributor

xiaoxiang781216 commented Sep 12, 2025

i will review at the weekend

@ppisa
Copy link
Contributor

ppisa commented Sep 14, 2025

Please, check compatibility with PHY access according to Clause 45 / MMD Ethernet standard implemented by @matiamic and mainlined in

#16926 include/net/if.h: Add mechanism for MMD access with SIOCxMIIREG ioctls

The final IOCTLs has been defined Linux kernel compatible way. The MMD has been used only for SPI OA MAC-PHY drivers till now #16936 but many already included MAC drivers can be extended to support MMD if underlaying hardware supports it. There is high probability that Elektroline adds that support for integrated SAMv7 MAC is some time (@michallenc and @Cynerd are closer to such planning).

If the given STM32 family provides MMD functionality in PHY/MDIO interface then it would worth to think about testing it implementing it with MMD support from the start.

The MMD PHY API functions should be considered from start in each case.

MMD is important today to allow seamlessly connect and configure PLCA T1S PHYs directly to existing MCUs where probably many of them include MMD support in MDIO interface.

@tinnedkarma
Copy link
Contributor Author

Please, check compatibility with PHY access according to Clause 45 / MMD Ethernet standard implemented by @matiamic and mainlined in

#16926 include/net/if.h: Add mechanism for MMD access with SIOCxMIIREG ioctls

The final IOCTLs has been defined Linux kernel compatible way. The MMD has been used only for SPI OA MAC-PHY drivers till now #16936 but many already included MAC drivers can be extended to support MMD if underlaying hardware supports it. There is high probability that Elektroline adds that support for integrated SAMv7 MAC is some time (@michallenc and @Cynerd are closer to such planning).

If the given STM32 family provides MMD functionality in PHY/MDIO interface then it would worth to think about testing it implementing it with MMD support from the start.

The MMD PHY API functions should be considered from start in each case.

MMD is important today to allow seamlessly connect and configure PLCA T1S PHYs directly to existing MCUs where probably many of them include MMD support in MDIO interface.

Let me start by presenting the concept/design of my work. Then we'll have a common starting point to make is suit everyone's work.

Lately, my work was mainly on the stm32 mcus. There, the netdevs (usually living in stm32_ethernet.c or stm32_eth.c) handles everything. From register access to memory mamagement to packets that gets in and out of the net subsystem. It is plain hard to understand the logic there, and it also led to code duplication across st mcu families.
I try to improve what is going on there as follows:

  • MDIO handling, a separate upperhalf/lowerhalf device that handles the mdio data transation. Nothing more, nothing less. Upperhalf as generic as possible, handling as much as posible. Lowerhalf concerned with the registers.
  • PHY handling, a separate upperhalf/lowerhalf device that holds a handle to mdio bus/device. It should handle negotiations, both forced and auto, device detection (phy id) and interrupt signalling.

You mention IOCTLs. I did not plan anything of this sort. The current upperhalf/lowerhalf netdev approach want to handle the IOCTLs, so no IOCTLs for MDIO or PHY. My take on the netdevs architecture is as follows: netdev->phy->mdio

  • lowerhalf netdev take a phy handle to any phy. Things like link management, power management through if_up, if_down
  • lowerhalf phy holds a handle to mdio bus. It controls higher level interaction over mdio bus, negotiations and status.

So to try giving an answer. I think ioctls should be handled two layers up, by the netdev, not the mdio bus, this is why there is nothing ioctl related in my bus.

Anything else is up for debate. I would really need some feedback for the phy api, or we can work in parallel after agreeing on a common approach.

The current changes for the stm32_ethernet.c file is part of my testing, work validation, I'll continue updating this file when I start work on the phy.

@matiamic
Copy link
Contributor

Great, then I see here two approaches:

  • I'll update the Kconfig file with an clause choice, so the phy driver (or user) can y select c22, c45, or both. I'll update mdio bus so it provides independent mdio_read/write_c22 and mdio_read/write_c45, each guarded by the choice above.

  • I'll add a new an new parameter (enum or int) to mdio_bus_s to specify what clauses it supports. I'll add the prtad parameter to the function signature, and pass it as NULL if c22 is used.

For the second approach it might be worth considering the encoding introduced in #16926: Add mechanism for MMD access with SIOCxMIIREG ioctls. Usage in driver here.

But special functions for c22 and c45 seem clearer. As seen in mii_bus struct in Linux.

I think the inclusion of the c45 functionality from the start would be nice. As @ppisa noted it is needed for configuration of the PLCA in 10BASE-T1S PHYs.

At the first glance.
I see the ioctl function mapping, and it currently returning OA_TC6_IOCTL_CMD_NOT_IMPLEMENTED.

You are probably looking into chip-specific drivers. The MDIO access in question is implemented in the generic driver here.

Also, talking about mac-phy, mac is onboard the external chip, so your lowerhalf mdio bus implementation will be managing spi translations, some part of the Control transactions, if i understand correctly. I'm not even sure that having a explicit mdio bus would help you, maybe is easier just to treat everything as spi data regardless if it is Ethernet MAC Frame data transactions or Control transactions. I'll need more time to read the datasheets.

I think you are right, having explicit mdio bus would not help here. If I understand correctly, your concern is to provide standard way of exchanging configuration between MAC drivers and external PHYs. The concern of the OA-TC6 driver in this regard is to expose MMD registers to user apps (such as plcatool) - at this moment the direct implementation of the SIOCxMIIREG ioctls seems like the best option.

@tinnedkarma tinnedkarma force-pushed the add-mdio-bus branch 6 times, most recently from 1d5e8f3 to 3ab8287 Compare September 15, 2025 13:42
@tinnedkarma
Copy link
Contributor Author

So the last thing to clarify then is how we handle both c22 and c45 clauses.
@matiamic I understand that you can agree on the kconfig approach?
@xiaoxiang781216 @acassis Maybe you can have an last word?

@acassis
Copy link
Contributor

acassis commented Sep 16, 2025

So the last thing to clarify then is how we handle both c22 and c45 clauses. @matiamic I understand that you can agree on the kconfig approach? @xiaoxiang781216 @acassis Maybe you can have an last word?

I think adding an option in Kconfig is fine (although I prefer when we remove items from Kconfig than when we add items there, since NuttX has too many entries)

acassis
acassis previously approved these changes Sep 16, 2025
@tinnedkarma
Copy link
Contributor Author

So the last thing to clarify then is how we handle both c22 and c45 clauses. @matiamic I understand that you can agree on the kconfig approach? @xiaoxiang781216 @acassis Maybe you can have an last word?

I think adding an option in Kconfig is fine (although I prefer when we remove items from Kconfig than when we add items there, since NuttX has too many entries)

I agree, there a some Kconfig files that a thousands of lines long. But I see it as the better option here. I'll update the docs, and the source files with the function signatures. The votes will reset though.

@xiaoxiang781216
Copy link
Contributor

xiaoxiang781216 commented Sep 17, 2025

should we support c45 clauses in this patch or put into new pr? @tinnedkarma

@tinnedkarma
Copy link
Contributor Author

@xiaoxiang781216 I'll add the kconfig options and guard both read/write functions for c22 today. I'll open a new pr with the c45 function signatures, as this thread is getting long and hard to follow.

@tinnedkarma tinnedkarma dismissed stale reviews from acassis and xiaoxiang781216 via e0b65b8 September 17, 2025 09:18
@tinnedkarma tinnedkarma force-pushed the add-mdio-bus branch 2 times, most recently from e0b65b8 to 67d513b Compare September 17, 2025 13:47
Currently the mdio communication is part of the monolithic 'full netdevs'.
This commit serves as an way to add modularity for the netdevs drivers.
A new upperhalf/lowerhalf mdio device comes with this commit that manages the data transfer ofer mdio interface.

Signed-off-by: Luchian Mihai <luchiann.mihai@gmail.com>
@tinnedkarma
Copy link
Contributor Author

@xiaoxiang781216 Is there anything else to do in this PR?
I would like to add next changes after this PR is merged

@xiaoxiang781216 xiaoxiang781216 merged commit 95efa6f into apache:master Sep 25, 2025
40 checks passed
@tinnedkarma tinnedkarma deleted the add-mdio-bus branch October 10, 2025 11:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Arch: arm Issues related to ARM (32-bit) architecture Area: Documentation Improvements or additions to documentation Area: Networking Effects networking subsystem Size: L The size of the change in this PR is large Size: XL The size of the change in this PR is very large. Consider breaking down the PR into smaller pieces.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants