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]

Date:   Fri, 4 Sep 2020 00:45:44 +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/4/20 12:08 AM, Andrew Lunn wrote:
>>> 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 ?
> 
> No. For stable we need something simpler.

Like this patch ?

>>>>> 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 ?
> 
> I _think_ this is the external clock which is feed to the PHY. Why
> else put it in the phy node in DT? And it has the name "rmii-ref"
> which again suggests it is the RMII clock, not something internal to
> the FEC.
> 
> To be sure, we would need to check the datasheet.

On iMX6Q where I have this issue (which btw is a very different SoC than
iMX6UL), this is not part of the PHY node. See
arch/arm/boot/dts/imx6qdl.dtsi . The SoC generates the clock and feeds
it into both the FEC and the PHY there.

Either way, this seems way out of scope for a bugfix which just corrects
the order of PHY reset/init, doesn't it?