Re: [PATCH net-next v11 05/15] ax88179_178a: Add netdev2data() convenience function

[netdev-bot+sashiko@kernel.org]

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

Critical: 0 · High: 0 · Medium: 0 · Low: 2

- [Low] Naming-convention / global-namespace violation: the new helper is
  defined non-static in drivers/net/usb/ax88179_lib.c:131 and declared…
- [Low] Reported for completeness, but verified NOT a defect: the concern
  was that `netdev2data()` is added with zero callers and that the…

--

ax88179_178a: Add netdev2data() convenience function

This commit adds a small helper to the ax88179 library that returns the
ax88179_data structure belonging to a net_device by going through
netdev_priv() and usbnet->driver_priv. It is declared in
ax88179_lib.h alongside the other library entry points.

> diff --git a/drivers/net/usb/ax88179_lib.c b/drivers/net/usb/ax88179_lib.c
> index 761701ebc0692..ea36aef3d3e45 100644
> --- a/drivers/net/usb/ax88179_lib.c
> +++ b/drivers/net/usb/ax88179_lib.c
> @@ -128,6 +128,13 @@ int ax88179_write_cmd(struct usbnet *dev, u8 cmd, u16 value, u16 index,
>  	return ret;
>  }
>  
> +struct ax88179_data *netdev2data(struct net_device *net)
> +{
> +	struct usbnet *dev = netdev_priv(net);
> +
> +	return dev->driver_priv;
> +}
> +

[Severity: Low]
This isn't a bug, but would a prefixed name be preferable here?

Every other cross-file symbol in this library carries the ax88179_
prefix: ax88179_read_cmd(), ax88179_write_cmd(), ax88179_mdio_read(),
ax88179_status(), ax88179_get_wol(), ax88179_set_multicast() and so on.
netdev2data() is defined non-static in ax88179_lib.c, so with
CONFIG_USB_NET_AX88179_178A=y the unprefixed name ends up in the
vmlinux/kallsyms global namespace and backtraces no longer say which
driver it belongs to.

Since drivers/net/usb/Makefile links ax88179_lib.o into the multi-object
ax88179 module, the function cannot simply be made static in the .c file.
Would either of these work instead?

  ax88179_netdev2data()

or, given the body is two lines, a static inline in ax88179_lib.h.

I checked at the end of the series (5c3c73e9) and the symbol is still
global and unprefixed, so no later patch renames or localizes it.

[Severity: Low]
Also worth noting for completeness, and this turned out not to be a
problem: at this commit netdev2data() has no callers, and the existing
open-coded netdev_priv() + ->driver_priv sequences in the same file
(ax88179_get_wol(), ax88179_set_wol(), ax88179_set_multicast()) are left
untouched.

Checking the rest of the series shows the consumers arrive in the later
patches, with drivers/net/usb/ax88179a_devices.c calling netdev2data()
in six places at 5c3c73e9, so the intermediate commit builds cleanly and
changes no behaviour. Could the commit message mention which follow-up
patch first uses the helper?

>  void ax88179_status(struct usbnet *dev, struct urb *urb)
>  {
>  	struct ax88179_int_data *event;
> diff --git a/drivers/net/usb/ax88179_lib.h b/drivers/net/usb/ax88179_lib.h
> index b2ee1356727cf..9ff2a94a6fb2d 100644
> --- a/drivers/net/usb/ax88179_lib.h
> +++ b/drivers/net/usb/ax88179_lib.h
> @@ -181,6 +181,7 @@ void ax88179_write_cmd_async(struct usbnet *dev, u8 cmd, u16 value, u16 index,
>  			     u16 size, void *data);
>  int ax88179_mdio_read(struct net_device *netdev, int phy_id, int loc);
>  void ax88179_mdio_write(struct net_device *netdev, int phy_id, int loc, int val);
> +struct ax88179_data *netdev2data(struct net_device *net);
>  void ax88179_status(struct usbnet *dev, struct urb *urb);
>  void ax88179_get_wol(struct net_device *net, struct ethtool_wolinfo *wolinfo);
>  int ax88179_set_wol(struct net_device *net, struct ethtool_wolinfo *wolinfo);

The new declaration sits in the middle of the ax88179_-prefixed block,
which is where the naming inconsistency above is most visible.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260914-ax88179a-v11-0-5ea7a925ba6d%40birger-koblitz.de