Received-SPF: pass (google.com: domain of linux-kernel+bounces-74414-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
From: "Madalin Bucur (OSS)" <madalin.bucur@oss.nxp.com>
To: Vladimir Oltean <vladimir.oltean@nxp.com>, "netdev@vger.kernel.org"
	<netdev@vger.kernel.org>
CC: "David S. Miller" <davem@davemloft.net>, Eric Dumazet
	<edumazet@google.com>, Jakub Kicinski <kuba@kernel.org>, Paolo Abeni
	<pabeni@redhat.com>, Sean Anderson <sean.anderson@seco.com>, Russell King
	<linux@armlinux.org.uk>, Zachary Goldstein
	<zachary.goldstein@concurrent-rt.com>, "linux-kernel@vger.kernel.org"
	<linux-kernel@vger.kernel.org>
Subject: RE: [PATCH net] net: dpaa: fman_memac: accept phy-interface-type =
 "10gbase-r" in the device tree
Thread-Topic: [PATCH net] net: dpaa: fman_memac: accept phy-interface-type =
 "10gbase-r" in the device tree
Thread-Index: AQHaZE0LMk4KrQ7VzUK8EAUOlg4LgbEUiC/Q
Date: Wed, 21 Feb 2024 09:31:56 +0000
Message-ID:
 <DB9PR04MB8185D8B7368797935FCDB5B2EC572@DB9PR04MB8185.eurprd04.prod.outlook.com>
References: <20240220223442.1275946-1-vladimir.oltean@nxp.com>
In-Reply-To: <20240220223442.1275946-1-vladimir.oltean@nxp.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-messagesentrepresentingtype: 1
x-ms-publictraffictype: Email
x-ms-traffictypediagnostic: DB9PR04MB8185:EE_|DU2PR04MB8789:EE_
x-ms-office365-filtering-correlation-id: 3616977b-e85e-4c42-1dba-08dc32bff29c
x-ms-exchange-sharedmailbox-routingagent-processed: True
x-ld-processed: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635,ExtAddr
x-ms-exchange-senderadcheck: 1
x-ms-exchange-antispam-relay: 0
x-microsoft-antispam: BCL:0;
x-microsoft-antispam-message-info:
 X+eEAb1fwwOHqXFU7hYk0oddHqs59NiwSZKNxMGDx8yL/d+nSXMvNAAIwLAq3UUzQVSf1T2o3bmawYMDEFnJaSLXGf0eS8+vzAW7WNJpkaBueQtn2Wbjmw9xKbnFwhfTF6bSfHaJOD2B1DVAL5UBa2SX8WTjVWG4vZyk/JzIRZlW1Y7ubZhlkTTePcC7ehtFcQ2q7TpYFVbKxHY/fDfRYqFd4K0wm0PA1dTGNKOKSTSrkPjaM3uN3tjbZGtTmW6u1YkHrWFkFcPbUiZW80Y1+yIqHuJ11y16DfhVI6ExaPgd0i4Sh9NMm+VzS2W3FYqpz2tq551KiUsJob3lFJhREFnLtkMW4ZLpGKyplSR/iTUSLaPw6yLKKC9oMUYm7vIFxyZuamnhgWSbzvHbRDH7+09uFDUsixueSivTxijTsA/DaSqTtjWsD+VpdGpaL9FkEsKUaB5MD+MUX7z3F1O92Q7YYyHh+q+JfK5uaPRUKU7S57CHjaiAn8keZBg0xPKmoMLB5ugwmWU5A5gp1aXB7xbv6VnHowPMk67i4OqfIdJT1DNly+d/5IbvaRkTi5mABQr42/AegsOBnYA4rgsWDIiRN+RyHg7Q1oP7HddLd/8=
x-forefront-antispam-report:
 CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:DB9PR04MB8185.eurprd04.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230031)(230273577357003)(38070700009);DIR:OUT;SFP:1101;
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0:
 =?us-ascii?Q?3l68cBdpZ+cdJW+ClXfaJLVVVcV6H+U0EJ12HwDlT4RLsJkGvPXGkz0xa+Jn?=
 =?us-ascii?Q?H8cc65I6HysAhba4pWfrREIbMfOvcZqTMwX8vAZS9YNI48b2ZXU7/1NCK6zw?=
 =?us-ascii?Q?Z3QmiLsbNVwjs93z+8HLgCfUyiLe4jriL2HLQxeaxYtT1YwbNnIUZYq0pIoY?=
 =?us-ascii?Q?pwO/ItleTztlK5/B8OIom9O6TpK2JNQuCwa0nf48GFPM0KLrkJ3vfrshCEVA?=
 =?us-ascii?Q?Ho7r3F9JfFxiZBgTejAlB06z/zPpARRPaooqx6fFVzgnEZje1gmwmS/AZHD8?=
 =?us-ascii?Q?Djelh22d6O4gdFXVXkIwLB74mA2kUMtDOOOILCMGSftkF8Q1Zk5wsuiP/pPR?=
 =?us-ascii?Q?qUCpyKK09YNaUfznYeidUCAX9ZXCinBBXqeT7sHQPdycQcuF7JGjbHjGGt7l?=
 =?us-ascii?Q?9w1IULqbrnUjaczfkb/NQzofYn5DOHjTYY0Lq/d1a+cz8mz/Fyw4cuE7bvYi?=
 =?us-ascii?Q?qeeCnJzjheh4bB1GvP9doaZ+m9dgZ4W+uUAZLiEQMsslFrmYqS7o8sAmX34D?=
 =?us-ascii?Q?Hw/UJgmeGfK99w/BXVNppEPMK8np8betuUvNMMh/QhVZFYzdLJYdmN81C2Ga?=
 =?us-ascii?Q?942N/mgYnv10PiXyA9Zt5j6cHHUwzZrkSNxWfewGb6SFv+YfHlMt7Q9mTNYr?=
 =?us-ascii?Q?oNghKXsXtOfBpkJYulzItEcIuRvNfirOJLlpOfO+pgL8L+ATDrU1prOwzDaU?=
 =?us-ascii?Q?vOh05LgyLttgVbezkowKgioGLUVgUS4jMT422yyRSucst8LMnFY3B6OCTuym?=
 =?us-ascii?Q?fD8ZtjFdwrXZdqAFKPLbWJZnxL0gLXF9AJssgrPIHL77C7PR3BDJW6WtVSf+?=
 =?us-ascii?Q?PmIDx9RSNgbuTsEUh4jREdzyLZgDqiROjIO56OHuM3wnJ6svvG/KCmhG2dzV?=
 =?us-ascii?Q?RWQIWD3RZ2rKLt0hckLJigDRSufsMk66MkOjmk4l8LYxRsrA9YF2KZQZAviy?=
 =?us-ascii?Q?kKbnZDzxLEXLAOoHm2kaqfTDkTbwn2A23rNKlx2pBdBpOhVp7naErFGHGZ9y?=
 =?us-ascii?Q?J52UNkjTM5MoemHdFrRxVTGinl5Ko14WYfd63hnPk70oSftiQUpXUbjV3G7g?=
 =?us-ascii?Q?WYjMJnOVyQVFravAtgJJyO9V0IqhmqOnQgCekRilwP0+Og7qFHnyCFA2pgB4?=
 =?us-ascii?Q?nN8fOyKU26C+CllFS4PGs9tEytw0clrX2xTBsdv3vPrEpqFA0REqbQzaDdhA?=
 =?us-ascii?Q?90gJJH/hYZVdzjN2GL/I+oHdXMV33vlHK6bAfTyPcoeAbePnG70MhK/0tFag?=
 =?us-ascii?Q?y/24EMhPh9y6hedZiIhXPGM7T1HEKfvlVXpP8nt2HGsPgItfAglYxdBIbtfI?=
 =?us-ascii?Q?IjM3dyeQlAUUhhs6KF62VTQQp+y6sJ/eSxweO8Yv5fI4jzraZyU91v4SYE6a?=
 =?us-ascii?Q?OlNSKa7XGlFsXsJfNzy7xTOLdAmkr8Wcort0L9NSKcjqdYXAulDAK6lz3mjN?=
 =?us-ascii?Q?1eJ/ixbaUAY7XL4/roQFgxdIOqC0SBQnC+2Skgz1T5ToL89SskbtVF+EU8pn?=
 =?us-ascii?Q?CcwnMztHPNfinvPvEwXlNcam3JuroKrtwQoDE+BRqxlcTOqpoBhLBi7LEO5O?=
 =?us-ascii?Q?eNdn5datfbjvMDRRpOci07l4lJJ/Ej+Ua6vpIn+K?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
X-Mailing-List: linux-kernel@vger.kernel.org
List-Id: <linux-kernel.vger.kernel.org>
List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org>
List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org>
MIME-Version: 1.0
X-OriginatorOrg: oss.nxp.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: DB9PR04MB8185.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 3616977b-e85e-4c42-1dba-08dc32bff29c
X-MS-Exchange-CrossTenant-originalarrivaltime: 21 Feb 2024 09:31:56.9982
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: pdB8kROY91iQ5z402rSwkKgzzjiWBUeb3kSXRZrB0vgVMqEIfvp5yQnRJ80gCju1xyF+FZ9NgmVUrHG1Ndd3lA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DU2PR04MB8789

> -----Original Message-----
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> Sent: Wednesday, February 21, 2024 12:35 AM
> To: netdev@vger.kernel.org
> Cc: David S. Miller <davem@davemloft.net>; Eric Dumazet
> <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo Abeni
> <pabeni@redhat.com>; Madalin Bucur <madalin.bucur@nxp.com>; Sean
> Anderson <sean.anderson@seco.com>; Russell King <linux@armlinux.org.uk>;
> Zachary Goldstein <zachary.goldstein@concurrent-rt.com>; linux-
> kernel@vger.kernel.org
> Subject: [PATCH net] net: dpaa: fman_memac: accept phy-interface-type =3D
> "10gbase-r" in the device tree
>=20
> Since commit 5d93cfcf7360 ("net: dpaa: Convert to phylink"), we support
> the "10gbase-r" phy-mode through a driver-based conversion of "xgmii",
> but we still don't actually support it when the device tree specifies
> "10gbase-r" proper.
>=20
> This is because boards such as LS1046A-RDB do not define pcs-handle-names
> (for whatever reason) in the ethernet@f0000 device tree node, and the
> code enters through this code path:
>=20
> 	err =3D of_property_match_string(mac_node, "pcs-handle-names", "xfi");
> 	// code takes neither branch and falls through
> 	if (err >=3D 0) {
> 		(...)
> 	} else if (err !=3D -EINVAL && err !=3D -ENODATA) {
> 		goto _return_fm_mac_free;
> 	}
>=20
> 	(...)
>=20
> 	/* For compatibility, if pcs-handle-names is missing, we assume this
> 	 * phy is the first one in pcsphy-handle
> 	 */
> 	err =3D of_property_match_string(mac_node, "pcs-handle-names",
> "sgmii");
> 	if (err =3D=3D -EINVAL || err =3D=3D -ENODATA)
> 		pcs =3D memac_pcs_create(mac_node, 0); // code takes this
> branch
> 	else if (err < 0)
> 		goto _return_fm_mac_free;
> 	else
> 		pcs =3D memac_pcs_create(mac_node, err);
>=20
> 	// A default PCS is created and saved in "pcs"
>=20
> 	// This determination fails and mistakenly saves the default PCS
> 	// memac->sgmii_pcs instead of memac->xfi_pcs, because at this
> 	// stage, mac_dev->phy_if =3D=3D PHY_INTERFACE_MODE_10GBASER.
> 	if (err && mac_dev->phy_if =3D=3D PHY_INTERFACE_MODE_XGMII)
> 		memac->xfi_pcs =3D pcs;
> 	else
> 		memac->sgmii_pcs =3D pcs;
>=20
> In other words, in the absence of pcs-handle-names, the default
> xfi_pcs assignment logic only works when in the device tree we have
> PHY_INTERFACE_MODE_XGMII.
>=20
> By reversing the order between the fallback xfi_pcs assignment and the
> "xgmii" overwrite with "10gbase-r", we are able to support both values
> in the device tree, with identical behavior.
>=20
> Currently, it is impossible to make the s/xgmii/10gbase-r/ device tree
> conversion, because it would break forward compatibility (new device
> tree with old kernel). The only way to modify existing device trees to
> phy-interface-mode =3D "10gbase-r" is to fix stable kernels to accept thi=
s
> value and handle it properly.
>=20
> One reason why the conversion is desirable is because with pre-phylink
> kernels, the Aquantia PHY driver used to warn about the improper use
> of PHY_INTERFACE_MODE_XGMII [1]. It is best to have a single (latest)
> device tree that works with all supported stable kernel versions.
>=20
> Note that the blamed commit does not constitute a regression per se.
> Older stable kernels like 6.1 still do not work with "10gbase-r", but
> for a different reason. That is a battle for another time.
>=20
> [1] https://lore.kernel.org/netdev/20240214-ls1046-dts-use-10gbase-r-v1-1=
-
> 8c2d68547393@concurrent-rt.com/
>=20
> Fixes: 5d93cfcf7360 ("net: dpaa: Convert to phylink")
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
>  .../net/ethernet/freescale/fman/fman_memac.c   | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
>=20
> diff --git a/drivers/net/ethernet/freescale/fman/fman_memac.c
> b/drivers/net/ethernet/freescale/fman/fman_memac.c
> index e30bf75b1d48..0996759907a8 100644
> --- a/drivers/net/ethernet/freescale/fman/fman_memac.c
> +++ b/drivers/net/ethernet/freescale/fman/fman_memac.c
> @@ -1076,6 +1076,14 @@ int memac_initialization(struct mac_device
> *mac_dev,
>  	unsigned long		 capabilities;
>  	unsigned long		*supported;
>=20
> +	/* The internal connection to the serdes is XGMII, but this isn't
> +	 * really correct for the phy mode (which is the external connection).
> +	 * However, this is how all older device trees say that they want
> +	 * 10GBASE-R (aka XFI), so just convert it for them.
> +	 */
> +	if (mac_dev->phy_if =3D=3D PHY_INTERFACE_MODE_XGMII)
> +		mac_dev->phy_if =3D PHY_INTERFACE_MODE_10GBASER;
> +
>  	mac_dev->phylink_ops		=3D &memac_mac_ops;
>  	mac_dev->set_promisc		=3D memac_set_promiscuous;
>  	mac_dev->change_addr		=3D memac_modify_mac_address;
> @@ -1142,7 +1150,7 @@ int memac_initialization(struct mac_device *mac_dev=
,
>  	 * (and therefore that xfi_pcs cannot be set). If we are defaulting to
>  	 * XGMII, assume this is for XFI. Otherwise, assume it is for SGMII.
>  	 */
> -	if (err && mac_dev->phy_if =3D=3D PHY_INTERFACE_MODE_XGMII)
> +	if (err && mac_dev->phy_if =3D=3D PHY_INTERFACE_MODE_10GBASER)
>  		memac->xfi_pcs =3D pcs;
>  	else
>  		memac->sgmii_pcs =3D pcs;
> @@ -1156,14 +1164,6 @@ int memac_initialization(struct mac_device
> *mac_dev,
>  		goto _return_fm_mac_free;
>  	}
>=20
> -	/* The internal connection to the serdes is XGMII, but this isn't
> -	 * really correct for the phy mode (which is the external connection).
> -	 * However, this is how all older device trees say that they want
> -	 * 10GBASE-R (aka XFI), so just convert it for them.
> -	 */
> -	if (mac_dev->phy_if =3D=3D PHY_INTERFACE_MODE_XGMII)
> -		mac_dev->phy_if =3D PHY_INTERFACE_MODE_10GBASER;
> -
>  	/* TODO: The following interface modes are supported by (some)
> hardware
>  	 * but not by this driver:
>  	 * - 1000BASE-KX
> --
> 2.34.1

Thank you for the detailed description of the situation.

Acked-by: Madalin Bucur <madalin.bucur@oss.nxp.com>