netdev - Re: [PATCH net v4 1/2] phy: mscc: Use PHY_ID_MATCH_MODEL for VSC8584, VSC8582, VSC8575, VSC856X

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20251022075655.m42pxagwvqg3x3y6@DEN-DL-M31836.microchip.com>
Date: Wed, 22 Oct 2025 09:56:55 +0200
From: Horatiu Vultur <horatiu.vultur@...rochip.com>
To: "Russell King (Oracle)" <linux@...linux.org.uk>
CC: <andrew@...n.ch>, <hkallweit1@...il.com>, <davem@...emloft.net>,
	<edumazet@...gle.com>, <kuba@...nel.org>, <pabeni@...hat.com>,
	<richardcochran@...il.com>, <vladimir.oltean@....com>,
	<vadim.fedorenko@...ux.dev>, <christophe.jaillet@...adoo.fr>,
	<rosenp@...il.com>, <steen.hegelund@...rochip.com>, <netdev@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH net v4 1/2] phy: mscc: Use PHY_ID_MATCH_MODEL for
 VSC8584, VSC8582, VSC8575, VSC856X

The 10/21/2025 10:09, Russell King (Oracle) wrote:

Hi,

> 
> On Fri, Oct 17, 2025 at 08:48:18AM +0200, Horatiu Vultur wrote:
> > As the PHYs VSC8584, VSC8582, VSC8575 and VSC856X exists only as rev B,
> > we can use PHY_ID_MATCH_MODEL to match exactly on revision B of the PHY.
> 
> I don't follow this. PHY_ID_MATCH_MODEL() uses a mask of bits 31:4,
> omitting the revision field. So that is equivalent to a .phy_id_mask
> of 0xfffffff0, which is what the code already uses.

I totally understand why you don't understand this as this is I made big
mistake!
I was supposed to use PHY_ID_MATCH_EXACT instead of PHY_ID_MATCH_MODEL.

> 
> > Because of this change then there is not need the check if it is a
> > different revision than rev B in the function vsc8584_probe() as we
> > already know that this will never happen.
> 
> Since bits 3:0 are masked out, this statement seems to be false.
> 
> > @@ -2587,9 +2576,8 @@ static struct phy_driver vsc85xx_driver[] = {
> >       .config_inband  = vsc85xx_config_inband,
> >  },
> >  {
> > -     .phy_id         = PHY_ID_VSC856X,
> > +     PHY_ID_MATCH_MODEL(PHY_ID_VSC856X),
> >       .name           = "Microsemi GE VSC856X SyncE",
> > -     .phy_id_mask    = 0xfffffff0,
> >       /* PHY_GBIT_FEATURES */
> >       .soft_reset     = &genphy_soft_reset,
> >       .config_init    = &vsc8584_config_init,
> > @@ -2667,9 +2655,8 @@ static struct phy_driver vsc85xx_driver[] = {
> >       .config_inband  = vsc85xx_config_inband,
> >  },
> >  {
> > -     .phy_id         = PHY_ID_VSC8575,
> > +     PHY_ID_MATCH_MODEL(PHY_ID_VSC8575),
> >       .name           = "Microsemi GE VSC8575 SyncE",
> > -     .phy_id_mask    = 0xfffffff0,
> >       /* PHY_GBIT_FEATURES */
> >       .soft_reset     = &genphy_soft_reset,
> >       .config_init    = &vsc8584_config_init,
> > @@ -2693,9 +2680,8 @@ static struct phy_driver vsc85xx_driver[] = {
> >       .config_inband  = vsc85xx_config_inband,
> >  },
> >  {
> > -     .phy_id         = PHY_ID_VSC8582,
> > +     PHY_ID_MATCH_MODEL(PHY_ID_VSC8582),
> >       .name           = "Microsemi GE VSC8582 SyncE",
> > -     .phy_id_mask    = 0xfffffff0,
> >       /* PHY_GBIT_FEATURES */
> >       .soft_reset     = &genphy_soft_reset,
> >       .config_init    = &vsc8584_config_init,
> > @@ -2719,9 +2705,8 @@ static struct phy_driver vsc85xx_driver[] = {
> >       .config_inband  = vsc85xx_config_inband,
> >  },
> >  {
> > -     .phy_id         = PHY_ID_VSC8584,
> > +     PHY_ID_MATCH_MODEL(PHY_ID_VSC8584),
> >       .name           = "Microsemi GE VSC8584 SyncE",
> > -     .phy_id_mask    = 0xfffffff0,
> >       /* PHY_GBIT_FEATURES */
> >       .soft_reset     = &genphy_soft_reset,
> >       .config_init    = &vsc8584_config_init,
> 
> Due to what I've said above, the above part of the patch is a cleanup,
> and functionally is a no-op.
> 
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

-- 
/Horatiu