Re: [PATCH net-next v20 2/3] net: phy: Add support for Template Control register for PMA

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] uapi: testmode mask uses kernel-only GENMASK macro
- [Low] net: phy: missing linux/bitfield.h include for FIELD_PREP
--

commit b5c025498496747ab794fc2b34dd0903358ffea9
Author: Kyle Switch <kyle.switch@motor-comm.com>

net: phy: Add support for Template Control register for PMA

This commit adds support for the 10GBASE-T PMA Template Test Mode register
field. It introduces configuration functions and macro definitions to allow
selecting test modes used for PHY validation.

> diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c
> index 870920311f9a0..c5f5753f7194b 100644
> --- a/drivers/net/phy/phy-c45.c
> +++ b/drivers/net/phy/phy-c45.c
[ ... ]
> +int genphy_c45_template_testmode(struct phy_device *phydev, u16 test_mode)
> +{
> +	u16 ctrl;
> +
> +	if (test_mode > MDIO_PMA_10GBT_TESTMODE_7)
> +		return -EOPNOTSUPP;
> +
> +	ctrl = FIELD_PREP(MDIO_PMA_10GBT_TESTMODE_MASK, test_mode);

[Severity: Low]
Does genphy_c45_template_testmode() rely on an implicit inclusion of
linux/bitfield.h? Using FIELD_PREP without explicitly including its required
header might lead to build failures if header dependencies change in the
future or on certain architectures.

> +	return phy_modify_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_PMA_10GBT_TESTMODE,
> +			      MDIO_PMA_10GBT_TESTMODE_MASK, ctrl);
> +}
> +EXPORT_SYMBOL_GPL(genphy_c45_template_testmode);
[ ... ]
> diff --git a/include/uapi/linux/mdio.h b/include/uapi/linux/mdio.h
> index 06f4bc3c20c71..353ca13c64fca 100644
> --- a/include/uapi/linux/mdio.h
> +++ b/include/uapi/linux/mdio.h
[ ... ]
> @@ -320,6 +321,17 @@
>  /* PMA 10GBASE-R Fast Retrain status and control register. */
>  #define MDIO_PMA_10GBR_FSRT_ENABLE	0x0001	/* Fast retrain enable */
>  
> +/* PMA 10GBASE-T Template Test Mode Register*/
> +#define MDIO_PMA_10GBT_TESTMODE_MASK GENMASK(15, 13) /* Template test mode */

[Severity: High]
Can this macro break userspace compilation? GENMASK is a kernel-only construct,
so defining a UAPI macro with it could cause a build regression for
userspace applications that include linux/mdio.h and evaluate
MDIO_PMA_10GBT_TESTMODE_MASK.

> +#define MDIO_PMA_10GBT_TESTMODE_NORMAL 0x0 /* Template Normal */
> +#define MDIO_PMA_10GBT_TESTMODE_1 0x1 /* Template TestMode1 */

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260917131441.3428554-1-kyle.switch@motor-comm.com?part=2