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

[Suman Ghosh]

> 	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
>