Re: [PATCH net-next 1/1] net: mdio: reset PHY before attempting to access ID register

Buday Csaba
2025-11-27T11:17:34Z

On Tue, Nov 25, 2025 at 06:43:35PM -0800, Jakub Kicinski wrote:
> On Tue, 25 Nov 2025 12:15:51 +0100 Buday Csaba wrote:
> > When the ID of an Ethernet PHY is not provided by the 'compatible'
> > string in the device tree, its actual ID is read via the MDIO bus.
> > For some PHYs this could be unsafe, since a hard reset may be
> > necessary to safely access the MDIO registers.
>
> You may be missing exports because it doesn't build with allmodconfig:
>
> ERROR: modpost: "mdio_device_register_reset" [drivers/net/mdio/fwnode_mdio.ko] undefined!
> ERROR: modpost: "mdio_device_unregister_reset" [drivers/net/mdio/fwnode_mdio.ko] undefined!
> --
> pw-bot: cr
>

I require some advice on how to do it properly.
The high level functionality belongs to either fwnode_mdio.c or maybe
phy_device.c

The latter may be better, since get_phy_device() already performs some
kind of recovery for PHYs with a certain unexpected behaviour.

But the low level functionality: registering the reset properties are
now in their proper place in mdio_device.c

These three source files build into three different modules, so I only
see the following options:

a) make mdio_device_register_reset() and its counterpart public
But we have already agreed that they should not be, and keep them
internal

b) create a new helper function in mdio_device.c, and make that public
This could work, but then what is the point of hiding
mdio_device_register_reset()? My idea was something like
mdio_device_reset_strobe(), which calls register/unregister reset
while also performing the assertion/deassertion of the reset.
But such a function is unsafe on an already established mdio_device,
so making that exported may be questionable as well.
It is possible to work around that, but then it is getting out of hand
fast, and does not follow the keep it simple and stupid principle.

c) what about using EXPORT_SYMBOL_FOR_MODULES() on the problematic
functions? Are there any objections against it?
This type of export is rarely used in the kernel, so I am uncertain
about that. Is using it on functions declared in private headers
also discouraged?

Thank you in advance,
Csaba