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

From: 万致远
Date: Mon Dec 02 2024 - 22:22:06 EST


At 2024/12/3 10:54, Andrew Lunn wrote:
>>> 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 know some cursed user-mode program do that but seems no kernel 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.
>
Okay, so I will left PHYAD == 0 stay untouched, and PHYAD != 0 disables
broadcast feature.
> Andrew
Sincerely,

Zhiyuan Wan