Received-SPF: pass (google.com: domain of linux-kernel+bounces-7080-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
From: Suman Ghosh <sumang@marvell.com>
To: Sneh Shah <quic_snehshah@quicinc.com>, Vinod Koul <vkoul@kernel.org>,
        Bhupesh Sharma <bhupesh.sharma@linaro.org>,
        Alexandre Torgue
	<alexandre.torgue@foss.st.com>,
        Jose Abreu <joabreu@synopsys.com>,
        "David S.
 Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>, Jakub
 Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        Maxime Coquelin
	<mcoquelin.stm32@gmail.com>,
        "netdev@vger.kernel.org"
	<netdev@vger.kernel.org>,
        "linux-arm-msm@vger.kernel.org"
	<linux-arm-msm@vger.kernel.org>,
        "linux-stm32@st-md-mailman.stormreply.com"
	<linux-stm32@st-md-mailman.stormreply.com>,
        "linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
        "linux-kernel@vger.kernel.org"
	<linux-kernel@vger.kernel.org>,
        Andrew Halaney <ahalaney@redhat.com>
CC: "kernel@quicinc.com" <kernel@quicinc.com>
Subject: RE: [EXT] [PATCH net-next] net: stmmac: Fix ethool link settings ops
 for integrated PCS
Thread-Topic: [EXT] [PATCH net-next] net: stmmac: Fix ethool link settings ops
 for integrated PCS
Thread-Index: AQHaMblEmtyfYpPBF0+XqX0QHVJjpLCyP73g
Date: Wed, 20 Dec 2023 14:46:09 +0000
Message-ID: 
 <SJ0PR18MB5216E4E90502F0B37602B06CDB96A@SJ0PR18MB5216.namprd18.prod.outlook.com>
References: <20231218135032.27209-1-quic_snehshah@quicinc.com>
In-Reply-To: <20231218135032.27209-1-quic_snehshah@quicinc.com>
Accept-Language: en-IN, en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: 
 =?us-ascii?Q?MI5+I6bSUx2TmyLNcP5Ka47M80ZcA9XeFGAult9c9Si6edEfM6U0Niqs0GIP?=
 =?us-ascii?Q?HNcxxZewR0jg3B6pKS7+z1aIy0Jbhess+RbkKejdr8VAaoiWAmoSHGXEnLmj?=
 =?us-ascii?Q?MYfltIugVDO0DY315vcMgT9f+FbfE1LQfbCPCK1bQ3z47U7kDD4Qr/B7F9TF?=
 =?us-ascii?Q?oqWdxCqjI3pUJo2iIMNHWpindn0mchotxLTZSvpJElPJz/WjCnOKxu/+eeHr?=
 =?us-ascii?Q?TzzekYbmvprGA870UwTsehG2uiY0AZrNF8BozA3+Be9JqlgVD6MaK017Obn+?=
 =?us-ascii?Q?E+IcQ9S3mjSaG7Y/5hEWTuAxEzRDkq0/rSuI7aVhnaS/19xWvmvM+NBAWbiX?=
 =?us-ascii?Q?rNkrGHky74qD36LcGDtNdEq7QNB0wrcGHWKFhE/jPhhbSYtmcBUfxvpxNUNF?=
 =?us-ascii?Q?b+oFZcNaU4bGSgIxUl/ISoypneMmdSi+Rtav8Oc01tTujVcviRU8TC44ZzsP?=
 =?us-ascii?Q?+pVC3OdAoXSbvnumdCpyZ/GHD2tBClt/bZEDQRfiaIulbdQ0jp0iOJXaD2SW?=
 =?us-ascii?Q?QcSx8cSn+M4a1mZcifwrLUH5UWmaUyrXJTBVkgzjmWDAipZzRXHG+rK1BH+E?=
 =?us-ascii?Q?zr8i6HZ3jR3/ueoHVYjF5ggYo2xSUW5inF1bV9oCLSbfkpFPEnXyigj3um8t?=
 =?us-ascii?Q?1sfUwuI3h97zSjhc40128xUfz1ImAbuevxeZuvh4Tf9OI3EEB6U5rZ21KRqG?=
 =?us-ascii?Q?i9n6RdNDEEAo4ChD4wGZ3SbtqpGbUCR5MDBd/nSpxL+SfhckBNtgvMC4AoDA?=
 =?us-ascii?Q?kIrNb6Kr7whTkWinylgW2Ilk4SacnQlBWCht8eGbcx0PhyCMcN/dOQNCS+0b?=
 =?us-ascii?Q?gLGIIvIMh3kXvNIAnCreSAOTPXeA3Vz4jJEMSFufsEA0bGbNkVBpwmx+QJ8l?=
 =?us-ascii?Q?7zI9hertbqDDt/z258FtbCLJSsV1cnCcqphqcxgeCE8LIwaz8jOvYdRMnheQ?=
 =?us-ascii?Q?tY9NrOnsw69YMqAXMBrRuu5wgxBf0CcytNHMu4+k4g1KyK0iTbHWRf//f+Zh?=
 =?us-ascii?Q?4Gjesbo7+Z/ZlTOxmZWTHJwkXmrV4F4jqJUOfksQgK2FhhR85hUg/RLwy7i6?=
 =?us-ascii?Q?Lv8CkHchr2uGo5DgMO+IvP7Q1CJwXuQbOlYCdh3KQ3dFDv+QSQNXssFmV2dS?=
 =?us-ascii?Q?85bBdTk3g/yGm0lRl9W/aFRJYTBsenvTPTHtcT1AR7a0f8hSUmS3XmQNXFuD?=
 =?us-ascii?Q?KreiUkMZgTRsfqpGVPjb4FOmbiouE9G0+k1d3aI/hLor0YbI/H7NIGJhvJVH?=
 =?us-ascii?Q?nIOEPHq3RUizCwA5IhWIHeLd/n+DXM1rWReqMxBFIaqJppj0gFp1LMkWFFYv?=
 =?us-ascii?Q?1hBsEhjoh9yD7D/Ku82+Mw2qcjAK7PpVW2YKfsrit5LvAGLTvqjlfguLMlG4?=
 =?us-ascii?Q?nLXMfYbhb7yE/sGZUkAqsI5nhQ4VdVYyjyI/oJOo55umv/HbSRWu/Cju2Lev?=
 =?us-ascii?Q?+gJDWJJ6wwB7G3kf+9+c5XN7TMfHc+hvVkPjubJQOaNdrbiRRKHQI877kA7z?=
 =?us-ascii?Q?nC8IaFtolaiXt0nErcF7A2Pirm0NMaWkyG/TVbhdKba6KHgPSnYdEjUqlvz4?=
 =?us-ascii?Q?lrrBnWBEKv6V9mso5Q4=3D?=
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: marvell.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: SJ0PR18MB5216.namprd18.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 7be1c800-caf0-4ca5-ce96-08dc016a6750
X-MS-Exchange-CrossTenant-originalarrivaltime: 20 Dec 2023 14:46:09.0715
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 70e1fb47-1155-421d-87fc-2e58f638b6e0
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: AxizVEWiR0XX9RUKDcTdVabYd/luOKITargiAVlPV1cdOJD1AHPTskUdwjX65sgfimy/uREdIDoxSkZ52aptNQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR18MB5728
X-Proofpoint-GUID: PRwomSnG37_VMMg5p2zKL1LEfAnl6EUD
X-Proofpoint-ORIG-GUID: PRwomSnG37_VMMg5p2zKL1LEfAnl6EUD
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.272,Aquarius:18.0.997,Hydra:6.0.619,FMLib:17.11.176.26
 definitions=2023-12-09_02,2023-12-07_01,2023-05-22_02

> 	struct stmmac_priv *priv =3D netdev_priv(dev);
>
>-	if (priv->hw->pcs & STMMAC_PCS_RGMII ||
>-	    priv->hw->pcs & STMMAC_PCS_SGMII) {
>+	if (!(priv->plat->flags & STMMAC_FLAG_HAS_INTEGRATED_PCS) &&
[Suman] A personal preference here, I think the code will be more readable =
if we handle the !if condition @ the beginning. Something like,
if (priv->plat->flags & STMMAC_FLAG_HAS_INTEGRATED_PCS ||
	!(priv->hw->pcs & STMMAC_PCS_RGMII) || !(priv->hw->pcs & STMMAC_PCS_RGMII)=
)
	return phylink_ethtool_ksettings_get(priv->phylink, cmd);

and keep the rest of the code without any check. But it is up-to you.

>+	    (priv->hw->pcs & STMMAC_PCS_RGMII ||
>+	     priv->hw->pcs & STMMAC_PCS_SGMII)) {
> 		struct rgmii_adv adv;
> 		u32 supported, advertising, lp_advertising;
>
>@@ -397,8 +398,9 @@ stmmac_ethtool_set_link_ksettings(struct net_device
>*dev,  {
> 	struct stmmac_priv *priv =3D netdev_priv(dev);
>
>-	if (priv->hw->pcs & STMMAC_PCS_RGMII ||
>-	    priv->hw->pcs & STMMAC_PCS_SGMII) {
>+	if (!(priv->plat->flags & STMMAC_FLAG_HAS_INTEGRATED_PCS) &&
>+	    (priv->hw->pcs & STMMAC_PCS_RGMII ||
>+	     priv->hw->pcs & STMMAC_PCS_SGMII)) {
> 		/* Only support ANE */
> 		if (cmd->base.autoneg !=3D AUTONEG_ENABLE)
> 			return -EINVAL;
>--
>2.17.1
>