Re: [PATCH 2/2] net: phy: realtek: add dt property to disable broadcast PHY address

From: Andrew Lunn
Date: Mon Dec 02 2024 - 21:54:31 EST


> > I think you can do this without needing a new property. The DT binding
> > has:
> >
> > reg = <4>;
> >
> > This is the address the PHY should respond on. If reg is not 0, then
> > broadcast is not wanted.
> >
> First, broadcast has no relationship with PHY address, it allows MAC
> broadcast command to all supported PHY on the MDIO bus.
>
> I can't assume that there's no user use this feature to configure multiple
> PHY devices (e.g. there's like 3 or more PHYs on board, their address
> represented as 1, 2, 3. When this feature is enabled (default behavior),
> users can send commands to address 0 to configure common parameters shared
> by these PHYs) at the same time.

phylib does not do that. Each PHY is considered a single entity. User
space could in theory do it via phy_do_ioctl(), but that is a very
risky thing to do, there is no locking, and you are likely to confuse
phylib and/or the PHY driver.

So we don't actually need the broadcast feature.

> Again, the broadcast address is shared by all PHYs on MDIO which
> support this feature, it's handy for MAC to change multiple PHYs
> setting at the same time.

Please point me at a MAC driver doing this.

> I would recommend to add this feature, because it doesn't change the
> behavior of this driver, and allows this PHY works together with
> other PHY or switch chip which don't support this feature, like mt7530 or
> Marvell ones.

I agree we should be disabling this when it is safe to disable, but i
don't agree we need a new property, reg is sufficient.

Andrew