linux-kernel - Re: [PATCH 2/2] net: fec: Create device link between phy dev and mac dev

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: <355a8611-b60e-1166-0f7b-87a194debd07@gmail.com>
Date:   Wed, 16 Nov 2022 15:27:39 -0800
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Andrew Lunn <andrew@...n.ch>,
        Xiaolei Wang <xiaolei.wang@...driver.com>
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
Subject: Re: [PATCH 2/2] net: fec: Create device link between phy dev and mac
 dev

On 11/16/22 07:07, Andrew Lunn wrote:
> On Wed, Nov 16, 2022 at 10:43:05PM +0800, Xiaolei Wang wrote:
>> On imx6sx, there are two fec interfaces, but the external
>> phys can only be configured by fec0 mii_bus. That means
>> the fec1 can't work independently, it only work when the
>> fec0 is active. It is alright in the normal boot since the
>> fec0 will be probed first. But then the fec0 maybe moved
>> behind of fec1 in the dpm_list due to various device link.

Humm, but if FEC1 depends upon its PHY to be available by the FEC0 MDIO 
bus provider, then surely we will need to make sure that FEC0's MDIO bus 
is always functional, and that includes surviving re-ordering as well as 
any sort of run-time power management that can occur.

>> So in system suspend and resume, we would get the following
>> warning when configuring the external phy of fec1 via the
>> fec0 mii_bus due to the inactive of fec0. In order to fix
>> this issue, we create a device link between phy dev and fec0.
>> This will make sure that fec0 is always active when fec1
>> is in active mode.

Still not clear to me how the proposed fix works, let alone how it does 
not leak device links since there is no device_link_del(), also you are 
going to be creating guaranteed regressions by putting that change in 
the PHY library.

It seems to me that you need to address a more fundamental issue within 
the FEC driver and how it registers its internal MDIO busses.

The device link between the PHY and MAC does have the MDIO bus in 
between as a device.

Have you verified your patch is still needed even with 
557d5dc83f6831b4e54d141e9b121850406f9a60 ("net: fec: use mac-managed PHY 
PM")?
-- 
Florian