Re: [PATCH net-next 10/16] net: pcs: xpcs: Add generic DW XPCS MDIO-device support

Vladimir Oltean
2023-12-05T12:23:24Z

On Tue, Dec 05, 2023 at 02:35:46PM +0300, Serge Semin wrote:
> Omg, thank you very much for testing the series straight away and
> sorry for the immediate trouble it caused. I'll need some more time
> for investigation. I'll get back to this topic a bit later on this
> week.

Don't worry, I got suspicious when I was CCed to review only a one-line
change in patch 11/16. It's never about that one line, is it?)

Anyway, the NULL dev->p is a symptom of device_add() not having been
called, most likely from mdio_device_register().

I'll be honest and say that I still don't quite understand what you're
trying to achieve. You're trying to bind the hardcoded mdio_devices
created by xpcs_create() to a driver? That was attempted a while ago by
Sean Anderson with the Lynx PCS. Are you aware of the fact that even in
the good case in which binding the driver actually works, the user can
then come along and unbind it from the PCS device, and phylink isn't
prepared to handle that, so it will crash the kernel upon the next
phylink_pcs call?

The pcs-rzn1-miic.c driver puts a device_link to the MAC to at least
tear down the whole thing when the PCS is unbound, which is saner than
crashing the kernel. I don't see the equivalent protection mechanism here?

Can't the xpcs continue to live without a bound driver? Having a
compatible string in the OF description is perfectly fine though,
and should absolutely not preclude that.