netdev - Re: [PATCH net v2] phy: mscc: Fix PTP for vsc8574 and VSC8572

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250926121245.eqynm74h22d7fhrn@DEN-DL-M31836.microchip.com>
Date: Fri, 26 Sep 2025 14:12:45 +0200
From: Horatiu Vultur <horatiu.vultur@...rochip.com>
To: "Russell King (Oracle)" <linux@...linux.org.uk>
CC: Vladimir Oltean <vladimir.oltean@....com>, Jakub Kicinski
	<kuba@...nel.org>, <andrew@...n.ch>, <hkallweit1@...il.com>,
	<davem@...emloft.net>, <edumazet@...gle.com>, <pabeni@...hat.com>,
	<richardcochran@...il.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 v2] phy: mscc: Fix PTP for vsc8574 and VSC8572

The 09/26/2025 11:19, Russell King (Oracle) wrote:

Hi,

> 
> On Fri, Sep 26, 2025 at 12:52:03PM +0300, Vladimir Oltean wrote:
> > On Fri, Sep 26, 2025 at 10:46:47AM +0100, Russell King (Oracle) wrote:
> > > On Mon, Sep 22, 2025 at 02:33:01PM +0200, Horatiu Vultur wrote:
> > > > Thanks for the advice.
> > > > What about to make the PHY_ID_VSC8572 and PHY_ID_VSC8574 to use
> > > > vsc8584_probe() and then in this function just have this check:
> > > >
> > > > ---
> > > > if ((phydev->phy_id & 0xfffffff0) != PHY_ID_VSC8572 &&
> > > >     (phydev->phy_id & 0xfffffff0) != PHY_ID_VSC8574) {
> > > >   if ((phydev->phy_id & MSCC_DEV_REV_MASK) != VSC8584_REVB) {
> > > >           dev_err(&phydev->mdio.dev, "Only VSC8584 revB is supported.\n");
> > > >           return -ENOTSUPP;
> > > >   }
> > > > }
> > >
> > > Please, no, not like this. Have a look how the driver already compares
> > > PHY IDs in the rest of the code.
> > >
> > > When a PHY driver is matched, the PHY ID is compared using the
> > > .phy_id and .phy_id_mask members of the phy_driver structure.
> > >
> > > The .phy_id is normally stuff like PHY_ID_VSC8572 and PHY_ID_VSC8574.
> > >
> > > When the driver is probed, phydev->drv is set to point at the
> > > appropriate phy_driver structure. Thus, the tests can be simplified
> > > to merely looking at phydev->drv->phy_id:
> > >
> > >     if (phydev->drv->phy_id != PHY_ID_VSC8572 &&
> > >         phydev->drv->phy_id != PHY_ID_VSC8574 &&
> > >         (phydev->phy_id & MSCC_DEV_REV_MASK) != VSC8584_REVB) {
> > > ...
> > >
> > > Alternatively, please look at the phy_id*() and phydev_id_compare()
> > > families of functions.
> > >
> > > --
> > > RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> > > FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
> >
> > The phydev->phy_id comparisons are problematic with clause 45 PHYs where
> > this field is zero, but with clause 22 they should technically work.
> > It's good to know coming from a phylib maintainer that it's preferable
> > to use phydev->drv->phy_id everywhere (I also wanted to comment on this
> > aspect, but because technically nothing is broken, I didn't).
> 
> Yes indeed, which is another reason to use phydev->drv->* as these
> get matched against the C22 and C45 IDs during PHY probe.
> 
> If we wish to get more clever (in terms of wanthing to know the
> revision without knowing how the driver was matched) we could store
> the matched ID in phydev, as read from hardware. This would also get
> around the problem that where the ID is provided in the DT compatible,
> we would have the real hardware-read ID to check things like the
> revision against, rather than a fixed value in DT.

Thanks for the explanation and for suggestion.
I will use your suggestion in the next version.

> 
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

-- 
/Horatiu