netdev - RE: [EXT] [PATCH net-next] net: stmmac: Fix ethool link settings ops for integrated PCS

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: 
 <SJ0PR18MB5216E4E90502F0B37602B06CDB96A@SJ0PR18MB5216.namprd18.prod.outlook.com>
Date: Wed, 20 Dec 2023 14:46:09 +0000
From: Suman Ghosh <sumang@...vell.com>
To: Sneh Shah <quic_snehshah@...cinc.com>, Vinod Koul <vkoul@...nel.org>,
        Bhupesh Sharma <bhupesh.sharma@...aro.org>,
        Alexandre Torgue
	<alexandre.torgue@...s.st.com>,
        Jose Abreu <joabreu@...opsys.com>,
        "David S.
 Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>, Jakub
 Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Maxime Coquelin
	<mcoquelin.stm32@...il.com>,
        "netdev@...r.kernel.org"
	<netdev@...r.kernel.org>,
        "linux-arm-msm@...r.kernel.org"
	<linux-arm-msm@...r.kernel.org>,
        "linux-stm32@...md-mailman.stormreply.com"
	<linux-stm32@...md-mailman.stormreply.com>,
        "linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>,
        Andrew Halaney <ahalaney@...hat.com>
CC: "kernel@...cinc.com" <kernel@...cinc.com>
Subject: RE: [EXT] [PATCH net-next] net: stmmac: Fix ethool link settings ops
 for integrated PCS

> 	struct stmmac_priv *priv = 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 = 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 != AUTONEG_ENABLE)
> 			return -EINVAL;
>--
>2.17.1
>