netdev - Re: [PATCH] net: mdio: enable optional clock when registering a phy from devicetree

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

Message-ID: <10f8e599-940b-4b7c-8c82-8d505007f19b@theobroma-systems.com>
Date: Mon, 4 Dec 2023 11:14:12 +0100
From: Quentin Schulz <quentin.schulz@...obroma-systems.com>
To: Florian Fainelli <f.fainelli@...il.com>, Heiko Stuebner
 <heiko@...ech.de>, andrew@...n.ch, hkallweit1@...il.com
Cc: linux@...linux.org.uk, davem@...emloft.net, edumazet@...gle.com,
 kuba@...nel.org, pabeni@...hat.com, netdev@...r.kernel.org,
 linux-kernel@...r.kernel.org, Heiko Stuebner <heiko.stuebner@...rry.de>
Subject: Re: [PATCH] net: mdio: enable optional clock when registering a phy
 from devicetree

Hi Florian, Heiko,

On 12/1/23 23:41, Florian Fainelli wrote:
> On 12/1/23 06:24, Heiko Stuebner wrote:
>> From: Heiko Stuebner <heiko.stuebner@...rry.de>
>>
>> The ethernet-phy binding (now) specifys that phys can declare a clock
>> supply. Phy driver itself will handle this when probing the phy-driver.
>>
>> But there is a gap when trying to detect phys, because the mdio-bus needs
>> to talk to the phy to get its phy-id. Using actual phy-ids in the dt like
>>         compatible = "ethernet-phy-id0022.1640",
>>                      "ethernet-phy-ieee802.3-c22";
>> of course circumvents this, but in turn hard-codes the phy.
> 
> But it is the established practice for situations like those where you 
> need specific resources to be available in order to identify the device 
> you are trying to probe/register.
> 
> You can get away here with the clock API because it can operate on 
> device_node, and you might be able with a bunch of other "resources" 
> subsystems, but for instance with regulators, that won't work, we need a 
> "struct device" which won't be created because that is exactly what we 
> are trying to do.
> 
> Also this only works for OF, not for ACPI or other yet to come firmware 
> interface.
> 
> Sorry but NACK.
> 
> I am sympathetic to the idea that if you have multiple boards and you 
> may have multiple PHY vendors this may not really scale, but in 2023 you 
> have boot loaders aware of the Device Tree which can do all sorts of 
> live DTB patching to provide the kernel with a "perfect" view of the world.

There's a strong push towards unifying the device tree across all pieces 
of SW involved, sometimes going as far as only having one binary passed 
between SW stages (e.g. U-Boot passes its own DT to TF-A, and then to 
the Linux kernel without actually loading anything aside from the Linux 
kernel Image binary) if I remember correctly (haven't really followed 
tbh). So, this is kinda a step backward for this effort. I don't like 
relying on bootloader to make the kernel work, this is usually not a 
great thing. I understand the reasons but am still a bit sad to not see 
this done in the kernel.

Heiko, I would personally put the ID of the PHY to be the most likely 
encountered in the Linux kernel Device Tree so that if we somehow have a 
broken bootloader, there's a chance some devices still work properly. HW 
department said ksz9131 so we can go forward with this. In U-Boot DT, we 
would need a -u-boot.dtsi we change to the auto-detection compatible and 
we do the magic the Linux kernel doesn't want to do and hope it's fine 
for U-Boot maintainers. Once properly detected, we fixup the DT before 
booting the kernel.

Cheers,
Quentin