netdev - RE: [PATCH] ARM: dts: ls1021: Fix SGMII PCS link remaining down after PHY disconnect

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: <AM6PR04MB586318A19F0EE5D9B92B01BC8F5E0@AM6PR04MB5863.eurprd04.prod.outlook.com>
Date:   Mon, 25 Mar 2019 18:46:39 +0000
From:   Leo Li <leoyang.li@....com>
To:     Vladimir Oltean <vladimir.oltean@....com>,
        "shawnguo@...nel.org" <shawnguo@...nel.org>,
        Claudiu Manoil <claudiu.manoil@....com>
CC:     "robh+dt@...nel.org" <robh+dt@...nel.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "davem@...emloft.net" <davem@...emloft.net>
Subject: RE: [PATCH] ARM: dts: ls1021: Fix SGMII PCS link remaining down after
 PHY disconnect



> -----Original Message-----
> From: Vladimir Oltean
> Sent: Monday, March 25, 2019 4:31 AM
> To: shawnguo@...nel.org; Leo Li <leoyang.li@....com>; Claudiu Manoil
> <claudiu.manoil@....com>
> Cc: robh+dt@...nel.org; linux-arm-kernel@...ts.infradead.org;
> devicetree@...r.kernel.org; linux-kernel@...r.kernel.org;
> netdev@...r.kernel.org; davem@...emloft.net; Vladimir Oltean
> <vladimir.oltean@....com>
> Subject: [PATCH] ARM: dts: ls1021: Fix SGMII PCS link remaining down after
> PHY disconnect
> 
> Each eTSEC MAC has its own TBI (SGMII) PCS and private MDIO bus.
> But due to a DTS oversight, both SGMII-compatible MACs of the LS1021 SoC
> are pointing towards the same internal PCS. Therefore nobody is controlling
> the internal PCS of eTSEC0.
> 
> Upon initial ndo_open, the SGMII link is ok by virtue of U-boot initialization.
> But upon an ifdown/ifup sequence, the code path from ndo_open ->
> init_phy -> gfar_configure_serdes does not get executed for the PCS of
> eTSEC0 (and is executed twice for MAC eTSEC1). So the SGMII link remains
> down for eTSEC0. On the LS1021A-TWR board, to signal this failure condition,
> the PHY driver keeps printing
> '803x_aneg_done: SGMII link is not ok'.
> 
> Fixes: 055223d4d22d ("ARM: dts: ls1021a: Enable the eTSEC ports on QDS
> and TWR")
> Signed-off-by: Vladimir Oltean <vladimir.oltean@....com>
> ---
>  arch/arm/boot/dts/ls1021a-twr.dts | 9 ++++++++-
>  arch/arm/boot/dts/ls1021a.dtsi    | 9 +++++++++
>  2 files changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/boot/dts/ls1021a-twr.dts
> b/arch/arm/boot/dts/ls1021a-twr.dts
> index 97e1fb7..9b1fe99 100644
> --- a/arch/arm/boot/dts/ls1021a-twr.dts
> +++ b/arch/arm/boot/dts/ls1021a-twr.dts
> @@ -145,7 +145,7 @@
>  };
> 
>  &enet0 {
> -	tbi-handle = <&tbi1>;
> +	tbi-handle = <&tbi0>;
>  	phy-handle = <&sgmii_phy2>;
>  	phy-connection-type = "sgmii";
>  	status = "okay";
> @@ -225,6 +225,13 @@
>  	sgmii_phy2: ethernet-phy@2 {
>  		reg = <0x2>;
>  	};
> +	tbi0: tbi-phy@1f {
> +		reg = <0x1f>;
> +		device_type = "tbi-phy";
> +	};
> +};
> +
> +&mdio1 {
>  	tbi1: tbi-phy@1f {
>  		reg = <0x1f>;
>  		device_type = "tbi-phy";
> diff --git a/arch/arm/boot/dts/ls1021a.dtsi
> b/arch/arm/boot/dts/ls1021a.dtsi index b4f2723..3a3d264 100644
> --- a/arch/arm/boot/dts/ls1021a.dtsi
> +++ b/arch/arm/boot/dts/ls1021a.dtsi
> @@ -709,6 +709,15 @@
>  			      <0x0 0x2d10030 0x0 0x4>;
>  		};
> 
> +		mdio1: mdio@...4000 {
> +			compatible = "gianfar";

I know it is not your fault to use this compatible string as it already used this for mdio node in the device tree, but it is somewhat ambiguous to use the same compatible string as the ethernet controller.  And this dts is the only one in all kernel device trees using this compatible string for mdio node.  I would think it will be more clear to use "fsl,etsec2-mdio" or "fsl,etsec2-tbi".

> +			device_type = "mdio";
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			reg = <0x0 0x2d64000 0x0 0x4000>,
> +			      <0x0 0x2d50030 0x0 0x4>;
> +		};
> +
>  		ptp_clock@...0e00 {
>  			compatible = "fsl,etsec-ptp";
>  			reg = <0x0 0x2d10e00 0x0 0xb0>;
> --
> 2.7.4