linux-kernel - 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 for Android: free password hash cracker in your pocket

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

Message-ID: <b36e07f0-a8d6-4623-8531-4c3eef5dece6@theobroma-systems.com>
Date:   Mon, 4 Dec 2023 10:43:56 +0100
From:   Quentin Schulz <quentin.schulz@...obroma-systems.com>
To:     Andrew Lunn <andrew@...n.ch>, Heiko Stuebner <heiko@...ech.de>
Cc:     hkallweit1@...il.com, 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 Andrew,

On 12/1/23 23:15, Andrew Lunn wrote:
> [You don't often get email from andrew@...n.ch. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> On Fri, Dec 01, 2023 at 03:24:53PM +0100, 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.
>>
>> With boards often having multiple phy options and the mdio-bus being able
>> to actually probe devices, this feels like a step back.
>>
>> So check for the existence of a phy-clock per the -dtbinding in the
>> of_mdiobus_register_phy() and enable the clock around the
>> fwnode_mdiobus_register_phy() call which tries to determine the phy-id.
> 
> Why handle this separately to the reset GPIO and the reset controller?
> 

I was just wondering about this as well. Right now, we put the reset on 
the MAC controller device tree node for our board (c.f. 
https://lore.kernel.org/linux-arm-kernel/20231201191103.343097-1-heiko@sntech.de/) 
because otherwise it doesn't work.

I assume this is because the phy net subsystem is not handling the reset 
at the proper time (it does so before probing the PHY driver, which is 
too late because the auto-detection mechanism has already run before and 
the MAC couldn't find the PHY ID since the PHY wasn't reset properly at 
that time).

I think essentially we would need to have both the reset assert/deassert 
and clock enabling/disabling in the same location as this patch is 
suggesting to make all of this work.

I'll not investigate this, because Florian NACK'ed the whole thing. I do 
not personally have an interest in fixing only the reset, because we 
need the clock to be enabled for the auto-detection mechanism to work.

Cheers,
Quentin