Re: [PATCH net-next v3 1/6] net: dsa: motorcomm: Rename MIB stuffs
From: netdev-bot+sashiko@kernel.org
Date: 2026-09-09 15:16:13
Also in:
lkml
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: Critical: 0 · High: 0 · Medium: 0 · Low: 1 - [Low] Stale comment after the rename: `drivers/net/dsa/motorcomm/chip.c:41` still reads "/* Must agree with… -- net: dsa: motorcomm: Rename MIB stuffs This renames yt921x_read_mib() to yt921x_mib_read(), yt921x_poll_mib() to yt921x_mib_poll(), and struct yt921x_mib to struct yt921x_mib_stats, in preparation for a larger MIB refactor. No functional change is intended.
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/dsa/motorcomm/chip.c b/drivers/net/dsa/motorcomm/chip.c index d663af010f431..ad5fe1ccb1d71 100644 --- a/drivers/net/dsa/motorcomm/chip.c +++ b/drivers/net/dsa/motorcomm/chip.c@@ -545,11 +545,11 @@ yt921x_mbus_ext_init(struct yt921x_priv *priv, struct device_node *mnp) } /* Read and handle overflow of 32bit MIBs. MIB buffer must be zeroed before. */ -static int yt921x_read_mib(struct yt921x_priv *priv, int port) +static int yt921x_mib_read(struct yt921x_priv *priv, int port) { struct yt921x_port *pp = &priv->ports[port]; struct device *dev = to_device(priv); - struct yt921x_mib *mib = &pp->mib; + struct yt921x_mib_stats *mib = &pp->mib; int res = 0;
[ ... ]
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/dsa/motorcomm/chip.h b/drivers/net/dsa/motorcomm/chip.h index 83cd454955ddb..805febb486838 100644 --- a/drivers/net/dsa/motorcomm/chip.h +++ b/drivers/net/dsa/motorcomm/chip.h@@ -861,7 +861,7 @@ enum yt921x_fdb_entry_status { #define yt921x_port_is_internal(port) ((port) < 8) #define yt921x_port_is_external(port) ((port) == 8 || (port) == 9) -struct yt921x_mib { +struct yt921x_mib_stats { u64 rx_broadcast; u64 rx_pause; u64 rx_multicast;
[Severity: Low]
Should the comment above yt921x_mib_descs[] in chip.c be renamed too? It
still points at the old type name:
drivers/net/dsa/motorcomm/chip.c
/* Must agree with yt921x_mib
*
* Unstructured fields (name != NULL) will appear in get_ethtool_stats(),
* ...
*/
static const struct yt921x_mib_desc yt921x_mib_descs[] = {
After this commit there is no struct yt921x_mib left in the tree, so the
comment names a type that no longer exists.
This comment is also the only thing documenting the invariant that
yt921x_mib_read() relies on:
for (size_t i = 0; i < ARRAY_SIZE(yt921x_mib_descs); i++) {
const struct yt921x_mib_desc *desc = &yt921x_mib_descs[i];
u32 reg = YT921X_MIBn_DATA0(port) + desc->offset;
u64 *valp = &((u64 *)mib)[i];
The descriptor table order and count must match the u64 field order and
count of the (now renamed) struct, and there is no static_assert() or
BUILD_BUG_ON() tying ARRAY_SIZE(yt921x_mib_descs) to
sizeof(struct yt921x_mib_stats) / sizeof(u64). The invariant itself still
holds here (39 descriptors, 39 u64 fields, same order), so this is only a
comment-versus-definition mismatch.
Worth noting for the rest of the series: by the end of the series the same
comment survives in drivers/net/dsa/motorcomm/mib.c, but struct yt921x_mib
has been reintroduced there as a different container type (work, syncp,
stats, data[]). At that point the comment points at a struct that the
table must not agree with, since the table has to match
struct yt921x_mib_stats. Would it make sense to update the comment in this
patch, and possibly add a compile-time assertion on the entry count?
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260906131414.2683362-1-mmyangfl%40gmail.com