netdev - RE: [PATCH net-next 1/5] enetc: Fix if

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:   Mon, 9 Sep 2019 16:24:01 +0000
From:   Claudiu Manoil <claudiu.manoil@....com>
To:     Andrew Lunn <andrew@...n.ch>
CC:     "David S . Miller" <davem@...emloft.net>,
        Alexandru Marginean <alexandru.marginean@....com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: RE: [PATCH net-next 1/5] enetc: Fix if_mode extraction

>-----Original Message-----
>From: Andrew Lunn <andrew@...n.ch>
>Sent: Friday, September 6, 2019 10:58 PM
>To: Claudiu Manoil <claudiu.manoil@....com>
>Cc: David S . Miller <davem@...emloft.net>; Alexandru Marginean
><alexandru.marginean@....com>; netdev@...r.kernel.org
>Subject: Re: [PATCH net-next 1/5] enetc: Fix if_mode extraction
>
>On Fri, Sep 06, 2019 at 05:15:40PM +0300, Claudiu Manoil wrote:
>> Fix handling of error return code. Before this fix,
>> the error code was handled as unsigned type.
>> Also, on this path if if_mode not found then just handle
>> it as fixed link (i.e mac2mac connection).
>>
>> Signed-off-by: Claudiu Manoil <claudiu.manoil@....com>
>> ---
>>  drivers/net/ethernet/freescale/enetc/enetc_pf.c | 17 ++++++-----------
>>  1 file changed, 6 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/freescale/enetc/enetc_pf.c
>b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
>> index 7d6513ff8507..3a556646a2fb 100644
>> --- a/drivers/net/ethernet/freescale/enetc/enetc_pf.c
>> +++ b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
>> @@ -751,6 +751,7 @@ static int enetc_of_get_phy(struct enetc_ndev_priv
>*priv)
>>  	struct enetc_pf *pf = enetc_si_priv(priv->si);
>>  	struct device_node *np = priv->dev->of_node;
>>  	struct device_node *mdio_np;
>> +	int phy_mode;
>>  	int err;
>>
>>  	if (!np) {
>> @@ -784,17 +785,11 @@ static int enetc_of_get_phy(struct enetc_ndev_priv
>*priv)
>>  		}
>>  	}
>>
>> -	priv->if_mode = of_get_phy_mode(np);
>> -	if (priv->if_mode < 0) {
>> -		dev_err(priv->dev, "missing phy type\n");
>> -		of_node_put(priv->phy_node);
>> -		if (of_phy_is_fixed_link(np))
>> -			of_phy_deregister_fixed_link(np);
>> -		else
>> -			enetc_mdio_remove(pf);
>> -
>> -		return -EINVAL;
>> -	}
>
>Hi Claudiu
>
>It is not clear to me why it is no longer necessary to deregister the
>fixed link, or remove the mdio bus?
>
>> +	phy_mode = of_get_phy_mode(np);
>> +	if (phy_mode < 0)
>> +		priv->if_mode = PHY_INTERFACE_MODE_NA; /* fixed link */
>> +	else
>> +		priv->if_mode = phy_mode;
>

Hi Andrew,

The MAC2MAC connections are defined as fixed-link too, but without
phy-mode/phy-connection-type properties.  We don't want to de-register
these links.  Initial code was bogus in this regard.  So this MODE_NA interface
mode is reserved for the current representation of enetc ethernet ports
that are MAC2MAC connected with switch ports.  Also true that the current
upstream driver does not have the MAC2MAC connections enabled.

Your question however leads to the following discussion 😊.  The LS1028 SoC
has internal MAC2MAC connections between enetc eth ports and switch ports.
However the switch driver for LS1028a is not available upstream yet, it needs
to be re-worked as a DSA driver as you know (if you remember the discussions
we had on switchdev vs DSA for the Felix driver).  Now that we're at it, how
would you like to represent these MAC2MAC connections?

Current proposal is:
			ethernet@0,2 { /* SoC internal, connected to switch port 4 */
				compatible = "fsl,enetc";
				reg = <0x000200 0 0 0 0>;
				fixed-link {
					speed = <1000>;
					full-duplex;
				};
			};
			switch@0,5 {
				compatible = "mscc,felix-switch";
				[...]
				ports {
					#address-cells = <1>;
					#size-cells = <0>;

					/* external ports */
					[...]
					/* internal SoC ports */
					port@4 { /* connected to ENETC port2 */
						reg = <4>;
						fixed-link {
							speed = <1000>;
							full-duplex;
						};
					};
					port@5 { /* CPU port, connected to ENETC port3 */
						reg = <5>;
						ethernet = <&enetc_port3>;
						fixed-link {
							speed = <1000>;
							full-duplex;
						};
					};
				};
			};
			enetc_port3: ethernet@0,6 { /* SoC internal connected to switch port 5 */
				compatible = "fsl,enetc";
				reg = <0x000600 0 0 0 0>;
				fixed-link {
					speed = <1000>;
					full-duplex;
				};
			};
		};

Thanks.

Claudiu