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: <037cfb16-0d40-47bc-bafe-7a1b3964d6ec@lunn.ch>
Date:   Fri, 16 Jun 2023 16:16:49 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     "Russell King (Oracle)" <linux@...linux.org.uk>
Cc:     Jianhui Zhao <zhaojh329@...il.com>, hkallweit1@...il.com,
        davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
        pabeni@...hat.com, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH V3] net: phy: Add sysfs attribute for PHY c45 identifiers.

On Fri, Jun 16, 2023 at 08:49:41AM +0100, Russell King (Oracle) wrote:
> On Wed, Jun 14, 2023 at 09:45:22PM +0800, Jianhui Zhao wrote:
> > +static const struct attribute_group phy_dev_c45_ids_group = {
> > +	.name = "c45_ids",
> > +	.attrs = phy_c45_id_attrs
> > +};
> 
> One last thing - is there any point to creating these attributes if
> the PHY isn't c45?
> 
> We could add here:
> 
> 	.is_visible = phy_dev_c45_visible,
> 
> with:
> 
> static umode_t phy_dev_c45_visible(struct kobject *kobj,
> 				   struct attribute *attr, int foo)
> {
> 	struct phy_device *phydev = to_phy_device(kobj_to_dev(kobj));
> 
> 	return phydev->is_c45 ? attr->mode : 0;
> }
> 
> which will only show the c45 attributes if the PHY is a c45 PHY.
> 
> Andrew, any opinions?

There are PHYs which get detected via their C22 ID, but the driver
then uses C45. So phydev->is_c45 could be false yet the device does
have C45 IDs. But i don't see a good solution to this. If the point of
these values is to aid debugging matching devices to drivers, this
does not really matter. Its C22 ID is what will be used, and that
sysfs file will be populated.

So this .is_visible seems reasonable.

I suppose there is the flip condition. Do we want the C22 sysfs file
visible if there is no C22 ID? But that is probably ABI, we cannot
change it now.

   Andrew

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ