Re: [PATCH net-next v5 12/12] gve: add link status/speed ctrl ops

[netdev-bot+sashiko@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 0 · Low: 1

- [Low] Unrelated whitespace-only churn bundled into the functional hunk
  of gve_get_link_ksettings().

--

gve: add link status/speed ctrl ops

This converts the gve link status check into a control plane op and adds
an AQ-mode implementation that reads the device status register into
priv->link_up.  It also adds a report_link_speed op that wraps
gve_adminq_report_link_speed().

> diff --git a/drivers/net/ethernet/google/gve/gve_ethtool.c b/drivers/net/ethernet/google/gve/gve_ethtool.c
> index dd1c44fedc771..a3220c6514382 100644
> --- a/drivers/net/ethernet/google/gve/gve_ethtool.c
> +++ b/drivers/net/ethernet/google/gve/gve_ethtool.c
> @@ -749,12 +749,11 @@ static int gve_get_link_ksettings(struct net_device *netdev,
>  	int err = 0;
>  
>  	if (priv->link_speed == 0)
> -		err = gve_adminq_report_link_speed(priv);
> +		err = priv->ctrl_ops->report_link_speed(priv);
>  
>  	cmd->base.speed = priv->link_speed;
>  
>  	cmd->base.duplex = DUPLEX_FULL;
> -
>  	return err;

[Severity: Low]
This isn't a bug, but is the removal of the blank line between
cmd->base.duplex = DUPLEX_FULL; and return err; in
gve_get_link_ksettings() intentional?

The only functional change described for gve_ethtool.c is swapping
gve_adminq_report_link_speed(priv) for priv->ctrl_ops->report_link_speed(priv),
and neither the commit message nor the v3 changelog mentions a whitespace
change here.  Could that blank line be left alone to keep the hunk limited to
the ctrl op conversion?

>  }
>  

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260903215606.31633-1-hramamurthy%40google.com