Re: [PATCH net-next v2 4/7] net: phy: adin1100: Add separate driver entries for ADIN1110/ADIN2111

[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: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The commit message for "net: phy: adin1100: Add separate
  driver entries for ADIN1110/ADIN2111" says the patch is "not a…

--

net: phy: adin1100: Add separate driver entries for ADIN1110/ADIN2111

This patch splits the single adin_driver[] entry into three entries, one
each for ADIN1100, ADIN1110 and ADIN2111. The old entry used a 0xffffffcf
mask to match all three parts. Each new entry uses PHY_ID_MATCH_MODEL(),
as the mdio_device_id table already does, and keeps the same callbacks.

> diff --git a/drivers/net/phy/adin1100.c b/drivers/net/phy/adin1100.c
> index 8f9753d4318c8..b946cb86b6c41 100644
> --- a/drivers/net/phy/adin1100.c
> +++ b/drivers/net/phy/adin1100.c
> @@ -322,8 +322,7 @@ static int adin_probe(struct phy_device *phydev)
>  
>  static struct phy_driver adin_driver[] = {
>  	{
> -		.phy_id			= PHY_ID_ADIN1100,
> -		.phy_id_mask		= 0xffffffcf,
> +		PHY_ID_MATCH_MODEL(PHY_ID_ADIN1100),

[Severity: Medium]
The commit message says:

  The callbacks are unchanged, so this is not a functional change apart
  from the reported driver name.

Doesn't this change also alter which PHY IDs bind to the driver?

The old entry used PHY_ID_ADIN1100 (0x0283bc81) with a 0xffffffcf mask.
That mask ignores bits [5:4] but keeps the revision bits [3:0]. So
phy_bus_match()->genphy_match_phy_device() only matched these four IDs,
all with revision 1:

  0x0283bc81, 0x0283bc91, 0x0283bca1, 0x0283bcb1

PHY_ID_MATCH_MODEL() uses PHY_ID_MATCH_MODEL_MASK from
include/linux/phy.h:

  #define PHY_ID_MATCH_MODEL_MASK GENMASK(31, 4)

That mask ignores the revision nibble completely. After this patch:

- Every silicon revision of models 0x8, 0x9 and 0xa binds to the ADIN
  driver. Before, any revision other than 1 fell back to the generic PHY
  driver.

- 0x0283bcb1 (model 0xb) was accepted by the old mask, but no new entry
  matches it.

The 0xffffffcf mask came from commit 875b718ac380 ("net: phy: adin1100:
add PHY IDs of adin1110/adin2111"). That commit replaced a
revision-agnostic PHY_ID_MATCH_MODEL(PHY_ID_ADIN1100) entry, and adin_tbl
stayed revision-agnostic. This patch partly reverts that narrowing.

Could the commit message describe the change in revision matching? It
would also help to say whether the change is a deliberate fix, perhaps
with a Fixes: tag for 875b718ac380, or a side effect.

The later patches in the series keep PHY_ID_MATCH_MODEL(), so this
difference is still there at the end of the series.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261109085952.105331-1-iustin.stolniceanu%40analog.com