netdev - RE: [PATCH] arm64: dts: ls1046ardb: Set aqr106 phy mode to usxgmii

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

Message-ID: <DB8PR04MB6985EB9D28A17723C8C061CCECD30@DB8PR04MB6985.eurprd04.prod.outlook.com>
Date:   Thu, 23 Apr 2020 12:59:16 +0000
From:   Madalin Bucur <madalin.bucur@....com>
To:     Sascha Hauer <s.hauer@...gutronix.de>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>
CC:     Shawn Guo <shawnguo@...nel.org>, Leo Li <leoyang.li@....com>,
        Russell King - ARM Linux admin <linux@...linux.org.uk>,
        Andrew Lunn <andrew@...n.ch>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "joe.hershberger@...com" <joe.hershberger@...com>,
        "u-boot@...ts.denx.de" <u-boot@...ts.denx.de>
Subject: RE: [PATCH] arm64: dts: ls1046ardb: Set aqr106 phy mode to usxgmii

> -----Original Message-----
> From: Sascha Hauer <s.hauer@...gutronix.de>
> Sent: Thursday, April 23, 2020 1:22 PM
> To: linux-arm-kernel@...ts.infradead.org
> Cc: Madalin Bucur <madalin.bucur@....com>; Shawn Guo
> <shawnguo@...nel.org>; Leo Li <leoyang.li@....com>; Sascha Hauer
> <s.hauer@...gutronix.de>
> Subject: [PATCH] arm64: dts: ls1046ardb: Set aqr106 phy mode to usxgmii
> 
> The AQR107 family of phy devices do not support xgmii, but usxgmii
> instead. Since ce64c1f77a9d ("net: phy: aquantia: add USXGMII support
> and warn if XGMII mode is set") the kernel warns about xgmii being
> used. Change device tree to usxgmii.
> 
> Signed-off-by: Sascha Hauer <s.hauer@...gutronix.de>
> ---
>  arch/arm64/boot/dts/freescale/fsl-ls1046a-rdb.dts | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1046a-rdb.dts
> b/arch/arm64/boot/dts/freescale/fsl-ls1046a-rdb.dts
> index d53ccc56bb63..02fbef92b96a 100644
> --- a/arch/arm64/boot/dts/freescale/fsl-ls1046a-rdb.dts
> +++ b/arch/arm64/boot/dts/freescale/fsl-ls1046a-rdb.dts
> @@ -151,7 +151,7 @@ ethernet@...00 {
> 
>  	ethernet@...00 { /* 10GEC1 */
>  		phy-handle = <&aqr106_phy>;
> -		phy-connection-type = "xgmii";
> +		phy-connection-type = "usxgmii";
>  	};
> 
>  	ethernet@...00 { /* 10GEC2 */
> --
> 2.26.1

Hi Sascha,

thank you for trying to correct this problem. Unfortunately
"usxgmii" here is incorrect too, as that mode is not supported
by the LS1046A SoC. The connection mode used, as documented
by the SoC and PHY datasheets, is XFI. Unfortunately there was
resistance against including this connection type in the list
supported by the kernel (please note the distinction between
connection type and connection mode). At a certain moment the
two were aliased and the kernel uses connection mode, not
connection type. While we should describe here the hardware,
the board connection type (XFI), in the kernel the connection
mode was lately preferred (10G-BaseR). So, today we cannot use
"xfi" here, as the hardware description property should read.
The closest thing we can use is "10gbase-r". Unfortunately, in
u-boot support for "xfi" is already in place [1] and the device
tree should be different for the two for this reason - this goes
against the spirit of the device tree that should not depend on
the software using it...

I had on my agenda to fix this problem, had to stop when "xfi"
was rejected, at the time not even "10gbase-r" was an option.
Also worth noting here is that, while we change "xgmii" to a
correct/better value, we should also tolerate the old variant,
as there are users in the wild unable/unwilling to update the
device tree and backwards compatibility should be ensured,
further complicating the matter.

Regards,
Madalin

[1] I mention u-boot here because it's the default boot loader used by
LS1046ARDB and there are some interdependences with it, making things
even more complicated than they seem: u-boot currently performs a
fix-up for this device tree field, based on RCW (reset configuration
word), resulting in an override of the value provided to the booting
kernel. Some relevant u-boot commits:

https://github.com/u-boot/u-boot/commit/17285fc2833e0db04a2bd3d411cdf1a3e387de83
https://github.com/u-boot/u-boot/commit/8a141d6e9cc1841082e4c996703eafb037ec63ad