netdev - Re: [PATCH] net: fec: Fix PHY init after phy_reset_after_clk

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: <02ce2afb-7b9f-ba35-63a5-7496c7a39e6e@denx.de>
Date:   Fri, 4 Sep 2020 00:03:41 +0200
From:   Marek Vasut <marex@...x.de>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     netdev@...r.kernel.org,
        Christoph Niedermaier <cniedermaier@...electronics.com>,
        "David S . Miller" <davem@...emloft.net>,
        NXP Linux Team <linux-imx@....com>,
        Richard Leitner <richard.leitner@...data.com>,
        Shawn Guo <shawnguo@...nel.org>
Subject: Re: [PATCH] net: fec: Fix PHY init after phy_reset_after_clk_enable()

On 9/3/20 11:53 PM, Andrew Lunn wrote:
> On Thu, Sep 03, 2020 at 11:36:39PM +0200, Marek Vasut wrote:
>> On 9/3/20 11:00 PM, Andrew Lunn wrote:
>>> On Thu, Sep 03, 2020 at 10:27:12PM +0200, Marek Vasut wrote:
>>>> The phy_reset_after_clk_enable() does a PHY reset, which means the PHY
>>>> loses its register settings. The fec_enet_mii_probe() starts the PHY
>>>> and does the necessary calls to configure the PHY via PHY framework,
>>>> and loads the correct register settings into the PHY. Therefore,
>>>> fec_enet_mii_probe() should be called only after the PHY has been
>>>> reset, not before as it is now.
>>>
>>> I think this patch is related to what Florian is currently doing with
>>> PHY clocks.
>>
>> Which is what ? Details please.
> 
> Have you used b4 before?
> 
> b4 am 20200903043947.3272453-1-f.fainelli@...il.com

That might be a fix for the long run, but I doubt there's any chance to
backport it all to stable, is there ?

>>> I think a better fix for the original problem is for the SMSC PHY
>>> driver to control the clock itself. If it clk_prepare_enables() the
>>> clock, it knows it will not be shut off again by the FEC run time
>>> power management.
>>
>> The FEC MAC is responsible for generating the clock, the PHY clock are
>> not part of the clock framework as far as I can tell.
> 
> I'm not sure this is true. At least:
> 
> https://elixir.bootlin.com/linux/latest/source/arch/arm/boot/dts/imx6ul-kontron-n6x1x-s.dtsi#L123
> 
> and there are a few more examples:
> 
> imx6ul-14x14-evk.dtsi:			clocks = <&clks IMX6UL_CLK_ENET_REF>;
> imx6ul-kontron-n6x1x-s.dtsi:			clocks = <&clks IMX6UL_CLK_ENET_REF>;
> imx6ul-kontron-n6x1x-som-common.dtsi:			clocks = <&clks IMX6UL_CLK_ENET_REF>;
> imx6ull-myir-mys-6ulx.dtsi:			clocks = <&clks IMX6UL_CLK_ENET_REF>;
> imx6ul-phytec-phycore-som.dtsi:			clocks = <&clks IMX6UL_CLK_ENET_REF>;
> 
> Maybe it is just IMX6?

This is reference clock for the FEC inside the SoC, you probably want to
control the clock going out of the SoC and into the PHY, which is
different clock than the one described in the DT, right ?