Re: [PATCH net-next v13 3/5] net: dsa: motorcomm: use fixed-width GENMASK_U*() helpers

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The commit message for "net: dsa: motorcomm: use fixed-width
  GENMASK_U*() helpers" does not match the diff.

--

net: dsa: motorcomm: use fixed-width GENMASK_U*() helpers

This commit changes several GENMASK() uses in the motorcomm DSA driver to
GENMASK_U16() or GENMASK_U32(). The changed uses are the internal_mask
entries in yt921x_infos[], the ctrl value in yt921x_chip_setup_dsa() and
the phy_mask in yt921x_mbus_int_init(). The commit message calls it a type
cleanup with no functional change.

> Some masks assigned to u16 and u32 fields are defined with
> GENMASK(),which is an unsigned long constant and wider than
> the field type on 64-bit builds. Convert the affected
> internal_mask entries in chip.c and the YT921X_MBUS_CTRL_*
> masks used by mdio_bus.c to fixed-width helpers.

[Severity: Low]
Does this description match the diff?

The patch doesn't touch drivers/net/dsa/motorcomm/mdio_bus.h. The
YT921X_MBUS_CTRL_* masks there still use plain GENMASK():

#define  YT921X_MBUS_CTRL_PORT_M		GENMASK(25, 21)
#define  YT921X_MBUS_CTRL_REG_M			GENMASK(20, 16)
#define  YT921X_MBUS_CTRL_TYPE_M		GENMASK(11, 8)  /* wild guess */
#define  YT921X_MBUS_CTRL_OP_M			GENMASK(3, 2)  /* wild guess */

The masks are still like this at the end of the series too.

The only change in mdio_bus.c is to the PHY probe mask in
yt921x_mbus_int_init(), which is not an MBUS_CTRL register mask:

-	mbus->phy_mask = (u32)~GENMASK(max_ports - 1, 0);
+	mbus->phy_mask = ~GENMASK_U32(max_ports - 1, 0);

For chip.c, the message only mentions the internal_mask entries. The patch
also changes the local ctrl value in yt921x_chip_setup_dsa(), which is
written to YT921X_FILTER_UNK_UCAST:

-	ctrl = GENMASK(10, 0);
+	ctrl = GENMASK_U32(10, 0);

Could the commit message be updated to describe the phy_mask and ctrl
changes, and to drop the YT921X_MBUS_CTRL_* claim?

Also, there's a missing space after the comma in "GENMASK(),which".

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261011025043.1859940-1-kyle.switch%40motor-comm.com