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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAHwZ4N3rPCtXMUW1R_1zs14G-2wyOQnTOH+hqryE+7rq7013fg@mail.gmail.com>
Date: Tue, 3 Dec 2024 11:21:47 +0800
From: 万致远 <kmlinuxm@...il.com>
To: Andrew Lunn <andrew@...n.ch>
Cc: kuba@...nel.org, netdev@...r.kernel.org, linux-kernel@...r.kernel.org, 
	willy.liu@...ltek.com, Yuki Lee <febrieac@...look.com>
Subject: Re: [PATCH 2/2] net: phy: realtek: add dt property to disable
 broadcast PHY address

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