netdev - RE: [EXTERNAL] Re: [PATCH net 0/4] Restore and fix PHY reset for SMSC LAN8720

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: <CY4PR1701MB187877D03FC80867FED80DA1DF100@CY4PR1701MB1878.namprd17.prod.outlook.com>
Date:   Mon, 2 Nov 2020 15:24:00 +0000
From:   "Badel, Laurent" <LaurentBadel@...on.com>
To:     Jakub Kicinski <kuba@...nel.org>
CC:     "davem@...emloft.net" <davem@...emloft.net>,
        "m.felsch@...gutronix.de" <m.felsch@...gutronix.de>,
        "fugang.duan@....com" <fugang.duan@....com>,
        "andrew@...n.ch" <andrew@...n.ch>,
        Heiner Kallweit <hkallweit1@...il.com>,
        "linux@...linux.org.uk" <linux@...linux.org.uk>,
        "p.zabel@...gutronix.de" <p.zabel@...gutronix.de>,
        "lgirdwood@...il.com" <lgirdwood@...il.com>,
        "broonie@...nel.org" <broonie@...nel.org>,
        "robh+dt@...nel.org" <robh+dt@...nel.org>,
        "richard.leitner@...data.com" <richard.leitner@...data.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "f.fainelli@...il.com" <f.fainelli@...il.com>,
        "Quette, Arnaud" <ArnaudQuette@...on.com>
Subject: RE: [EXTERNAL]  Re: [PATCH net 0/4] Restore and fix PHY reset for
 SMSC LAN8720



> 

-----------------------------
Eaton Industries Manufacturing GmbH ~ Registered place of business: Route de la Longeraie 7, 1110, Morges, Switzerland 

-----------------------------

-----Original Message-----
> From: Jakub Kicinski <kuba@...nel.org>
> Sent: Thursday, October 29, 2020 12:10 AM
> To: Badel, Laurent <LaurentBadel@...on.com>
<snip>
> Subject: [EXTERNAL] Re: [PATCH net 0/4] Restore and fix PHY reset for SMSC
> LAN8720
> 
> On Tue, 27 Oct 2020 23:25:01 +0000 Badel, Laurent wrote:
> > Subject: [PATCH net 0/4] Restore and fix PHY reset for SMSC LAN8720
> >
> > Description:
> > A recent patchset [1] added support in the SMSC PHY driver for
> > managing the ref clock and therefore removed the
> PHY_RST_AFTER_CLK_EN
> > flag for the
> > LAN8720 chip. The ref clock is passed to the SMSC driver through a new
> > property "clocks" in the device tree.
> >
> > There appears to be two potential caveats:
> > (i) Building kernel 5.9 without updating the DT with the "clocks"
> > property for SMSC PHY, would break systems previously relying on the
> > PHY reset workaround (SMSC driver cannot grab the ref clock, so it is
> > still managed by FEC, but the PHY is not reset because
> > PHY_RST_AFTER_CLK_EN is not set). This may lead to occasional loss of
> > ethernet connectivity in these systems, that is difficult to debug.
> >
> > (ii) This defeats the purpose of a previous commit [2] that disabled
> > the ref clock for power saving reasons. If a ref clock for the PHY is
> > specified in DT, the SMSC driver will keep it always on (confirmed
> > with scope). While this removes the need for additional PHY resets
> > (only a single reset is needed after power up), this prevents the FEC
> > from saving power by disabling the refclk. Since there may be use
> > cases where one is interested in saving power, keep this option
> > available when no ref clock is specified for the PHY, by fixing issues with
> the PHY reset.
> >
> > Main changes proposed to address this:
> > (a) Restore PHY_RST_AFTER_CLK_EN for LAN8720, but explicitly clear it
> > if the SMSC driver succeeds in retrieving the ref clock.
> > (b) Fix phy_reset_after_clk_enable() to work in interrupt mode, by
> > re-configuring the PHY registers after reset.
> >
> > Tests: against net tree 5.9, including allyes/no/modconfig. 10 pieces
> > of an iMX28-EVK-based board were tested, 3 of which were found to
> > exhibit issues when the "clocks" property was left unset. Issues were
> > fixed by the present patchset.
> >
> > References:
> > [1] commit d65af21842f8 ("net: phy: smsc: LAN8710/20: remove
> >     PHY_RST_AFTER_CLK_EN flag")
> >     commit bedd8d78aba3 ("net: phy: smsc: LAN8710/20: add phy refclk in
> >     support")
> > [2] commit e8fcfcd5684a ("net: fec: optimize the clock management to
> save
> >     power")
> 
> Please resend with git send-email, if you can.
> 

My apologies. I will see if I manage to set up git to send emails with my account, 
but if not I will make sure to check the formatting more thoroughly.
Thanks also for taking the time to detail the defects. 
Best regards.

> All the patches have a "Subject: [PATCH" line in the message body, and Fixes
> tags are line-wrapped (they should be one line even if they are long).
> 
> > Laurent Badel (5):
> >   net:phy:smsc: enable PHY_RST_AFTER_CLK_EN if ref clock is not set
> >   net:phy:smsc: expand documentation of clocks property
> >   net:phy: add phy_device_reset_status() support
> >   net:phy: fix phy_reset_after_clk_enable()
> >   net:phy: add SMSC PHY reset on PM restore
> 
> There are only 4 patches in the series.