Re: [PATCH 6/9] net: avoid copies before linkmode_and()

[Jijie Shao <shaojijie@huawei.com>]

on 2026/9/8 5:54, Yury Norov wrote:
> Avoid separate linkmode_copy() calls by passing the original source
> bitmap directly to linkmode_and().
>
> In phy_ethtool_ksettings_set(), also use the return value of
> linkmode_and() when validating that the requested advertisement
> contains a supported link mode.

Please include "hns3" in the subject prefix (e.g., "net: hns3: ...")
so it's clear which driver this patch belongs to. I didn't immediately
find the hns3 driver modifications from the current title.

>
> Signed-off-by: Yury Norov <ynorov@nvidia.com>
> ---
>   drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mdio.c | 5 ++---
>   drivers/net/phy/phy.c                                   | 7 +++----
>   2 files changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mdio.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mdio.c
> index cf881108fa57..5801da6100b8 100644
> --- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mdio.c
> +++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mdio.c
> @@ -205,7 +205,6 @@ int hclge_mac_connect_phy(struct hnae3_handle *handle)
>   	struct hclge_dev *hdev = vport->back;
>   	struct net_device *netdev = hdev->vport[0].nic.netdev;
>   	struct phy_device *phydev = hdev->hw.mac.phydev;
> -	__ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
>   	int ret;
>   
>   	if (!phydev)
> @@ -223,8 +222,8 @@ int hclge_mac_connect_phy(struct hnae3_handle *handle)
>   		return ret;
>   	}
>   
> -	linkmode_copy(mask, hdev->hw.mac.supported);
> -	linkmode_and(phydev->supported, phydev->supported, mask);
> +	linkmode_and(phydev->supported, phydev->supported,
> +		     hdev->hw.mac.supported);
>   	linkmode_copy(phydev->advertising, phydev->supported);
>   
>   	/* supported flag is Pause and Asym Pause, but default advertising
> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> index fce9bc7be330..cd71186cd842 100644
> --- a/drivers/net/phy/phy.c
> +++ b/drivers/net/phy/phy.c
> @@ -1160,6 +1160,7 @@ int phy_ethtool_ksettings_set(struct phy_device *phydev,
>   			      const struct ethtool_link_ksettings *cmd)
>   {
>   	__ETHTOOL_DECLARE_LINK_MODE_MASK(advertising);
> +	bool has_advertising;
>   	u8 autoneg = cmd->base.autoneg;
>   	u8 duplex = cmd->base.duplex;
>   	u32 speed = cmd->base.speed;
> @@ -1167,17 +1168,15 @@ int phy_ethtool_ksettings_set(struct phy_device *phydev,
>   	if (cmd->base.phy_address != phydev->mdio.addr)
>   		return -EINVAL;
>   
> -	linkmode_copy(advertising, cmd->link_modes.advertising);
> -
>   	/* We make sure that we don't pass unsupported values in to the PHY */
> -	linkmode_and(advertising, advertising, phydev->supported);
> +	has_advertising = linkmode_and(advertising, cmd->link_modes.advertising, phydev->supported);


To keep the file clean, could you please update this part to match thesurrounding code style?
Specifically, we order local variables fromlongest to shortest (reverse Christmas tree) and
enforce an 80-characterline limit.

Thanks,
Jijie Shao


>   
>   	/* Verify the settings we care about. */
>   	if (autoneg != AUTONEG_ENABLE && autoneg != AUTONEG_DISABLE)
>   		return -EINVAL;
>   
>   	if (autoneg == AUTONEG_ENABLE &&
> -	    (linkmode_empty(advertising) ||
> +	    (!has_advertising ||
>   	     !linkmode_test_bit(ETHTOOL_LINK_MODE_Autoneg_BIT,
>   				phydev->supported)))
>   		return -EINVAL;