netdev - Re: [PATCH net] netsec: ignore 'phy-mode' device property on ACPI systems

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] [day] [month] [year] [list]

Message-ID: <20201020161253.72b99808@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Date:   Tue, 20 Oct 2020 16:12:53 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     Ard Biesheuvel <ardb@...nel.org>, netdev@...r.kernel.org,
        Jassi Brar <jaswinder.singh@...aro.org>,
        Ilias Apalodimas <ilias.apalodimas@...aro.org>,
        Masami Hiramatsu <mhiramat@...nel.org>,
        Willy Liu <willy.liu@...ltek.com>,
        "David S. Miller" <davem@...emloft.net>,
        Masahisa Kojima <masahisa.kojima@...aro.org>,
        Serge Semin <fancer.lancer@...il.com>,
        Rob Herring <robh+dt@...nel.org>, devicetree@...r.kernel.org
Subject: Re: [PATCH net] netsec: ignore 'phy-mode' device property on ACPI
 systems

On Tue, 20 Oct 2020 14:50:01 +0200 Andrew Lunn wrote:
> On Sun, Oct 18, 2020 at 06:36:25PM +0200, Ard Biesheuvel wrote:
> > Since commit bbc4d71d63549bc ("net: phy: realtek: fix rtl8211e rx/tx
> > delay config"), the Realtek PHY driver will override any TX/RX delay
> > set by hardware straps if the phy-mode device property does not match.
> > 
> > This is causing problems on SynQuacer based platforms (the only SoC
> > that incorporates the netsec hardware), since many were built with
> > this Realtek PHY, and shipped with firmware that defines the phy-mode
> > as 'rgmii', even though the PHY is configured for TX and RX delay using
> > pull-ups.
> >   
> > >From the driver's perspective, we should not make any assumptions in  
> > the general case that the PHY hardware does not require any initial
> > configuration. However, the situation is slightly different for ACPI
> > boot, since it implies rich firmware with AML abstractions to handle
> > hardware details that are not exposed to the OS. So in the ACPI case,
> > it is reasonable to assume that the PHY comes up in the right mode,
> > regardless of whether the mode is set by straps, by boot time firmware
> > or by AML executed by the ACPI interpreter.
> > 
> > So let's ignore the 'phy-mode' device property when probing the netsec
> > driver in ACPI mode, and hardcode the mode to PHY_INTERFACE_MODE_NA,
> > which should work with any PHY provided that it is configured by the
> > time the driver attaches to it. While at it, document that omitting
> > the mode is permitted for DT probing as well, by setting the phy-mode
> > DT property to the empty string.
> > 
> > Fixes: 533dd11a12f6 ("net: socionext: Add Synquacer NetSec driver")
> > Signed-off-by: Ard Biesheuvel <ardb@...nel.org>  
> 
> Reviewed-by: Andrew Lunn <andrew@...n.ch>

Thanks, applied.

Just to be on the safe side please make sure to CC Rob & DT list 
if your patch touches anything device tree.

> --- a/Documentation/devicetree/bindings/net/socionext-netsec.txt
> +++ b/Documentation/devicetree/bindings/net/socionext-netsec.txt
> @@ -30,7 +30,9 @@ Optional properties: (See ethernet.txt file in the same directory)
>  - max-frame-size: See ethernet.txt in the same directory.
>  
>  The MAC address will be determined using the optional properties
> -defined in ethernet.txt.
> +defined in ethernet.txt. The 'phy-mode' property is required, but may
> +be set to the empty string if the PHY configuration is programmed by
> +the firmware or set by hardware straps, and needs to be preserved.
>  
>  Example:
>  	eth0: ethernet@...d0000 {