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-next>] [day] [month] [year] [list]

Message-ID: <CAHwZ4N0gbTvXFYCawbOUFWk7yitTeAWwUmfmb7RU68n-md8x4Q@mail.gmail.com>
Date: Tue, 3 Dec 2024 10:37:01 +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

On 2024/12/3 8:04, Andrew Lunn wrote:
> On Tue, Dec 03, 2024 at 03:50:29AM +0800, Zhiyuan Wan wrote:
>> This patch add support to disable 'broadcast PHY address' feature of
>> RTL8211F.
>>
>> This feature is enabled defaultly after a reset of this transceiver.
>> When this feature is enabled, the phy not only responds to the
>> configuration PHY address by pin states on board, but also responds
>> to address 0, the optional broadcast address of the MDIO bus.
>>
>> But not every transceiver supports this feature, when RTL8211
>> shares one MDIO bus with other transceivers which doesn't support
>> this feature, like mt7530 switch chip (integrated in mt7621 SoC),
>> it usually causes address conflict, leads to the
>> port of RTL8211FS stops working.
>
> 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. And they can configure each PHY by it's
own address without influencing other PHY too.

> If reg is 0, it means one of two things:
>
> The DT author did not know about this broadcast feature, the PHY
> appeared at address 0, so they wrote that. It might actually be
> strapped to another address, but it does not matter.
>
Well, that's possible. A misconfiguration on DT with only ONE PHY may
just works, if we disable this feature by default, may cause some device
stops working.

> The DT author wants it to use the broadcast address, it might even be
> strapped to address 0.
>
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. But each PHY must have their own address,
and the address usually can't be broadcast address (0).

> Am i missing anything?
>
>       Andrew

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.

Sincerely,

Zhiyuan Wan