netdev - Re: [PATCH net-next 5/7] net: phy: introduce ethtool_phy

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: <ZuJyJT-HgXJFe5ul@pengutronix.de>
Date: Thu, 12 Sep 2024 06:46:29 +0200
From: Oleksij Rempel <o.rempel@...gutronix.de>
To: Maxime Chevallier <maxime.chevallier@...tlin.com>
Cc: davem@...emloft.net, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org, thomas.petazzoni@...tlin.com,
	Andrew Lunn <andrew@...n.ch>, Jakub Kicinski <kuba@...nel.org>,
	Eric Dumazet <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>,
	Russell King <linux@...linux.org.uk>,
	linux-arm-kernel@...ts.infradead.org,
	Christophe Leroy <christophe.leroy@...roup.eu>,
	Herve Codina <herve.codina@...tlin.com>,
	Florian Fainelli <f.fainelli@...il.com>,
	Heiner Kallweit <hkallweit1@...il.com>,
	Vladimir Oltean <vladimir.oltean@....com>,
	Marek Behún <kabel@...nel.org>,
	Köry Maincent <kory.maincent@...tlin.com>
Subject: Re: [PATCH net-next 5/7] net: phy: introduce ethtool_phy_ops to get
 and set phy configuration

Hi Maxime,

On Wed, Sep 11, 2024 at 11:27:09PM +0200, Maxime Chevallier wrote:
...
>  
> +/**
> + * struct phy_device_config - General PHY device configuration parameters for
> + * status reporting and bulk configuration
> + *
> + * A structure containing generic PHY device information, allowing to expose
> + * internal status to userspace, and perform PHY configuration in a controlled
> + * manner.
> + *
> + * @isolate: The MII-side isolation status of the PHY
> + * @loopback: The loopback state of the PHY
> + */
> +struct phy_device_config {
> +	bool isolate;
> +	bool loopback;
> +};
 
I would recommend to have loopback enum. There are different levels of
loopback:
https://www.ti.com/document-viewer/DP83TD510E/datasheet#GUID-50834313-DEF1-42FB-BA00-9B0902B2D7E4/TITLE-SNLS656SNLS5055224

I imagine something like this:

/*
 * enum phy_loopback_mode - PHY loopback modes
 * These modes represent different loopback configurations to
 * facilitate in-circuit testing of the PHY's digital and analog paths.
 */
enum phy_loopback_mode {
	PHY_LOOPBACK_NONE = 0,		/* No loopback mode enabled */
	PHY_LOOPBACK_MII,		/* MII Loopback: MAC to PHY internal loopback */
	PHY_LOOPBACK_PCS,		/* PCS Loopback: PCS layer loopback, no signal processing */
	PHY_LOOPBACK_DIGITAL,		/* Digital Loopback: Loops back entire digital TX/RX path */
	PHY_LOOPBACK_ANALOG,		/* Analog Loopback: Loops back after analog front-end */
	PHY_LOOPBACK_FAR_END		/* Far-End (Reverse) Loopback: Receiver to MAC interface loopback */
};

At same time, one interface will have multiple loopback providers, except of
multiple PHYs, MAC will provide it too.

I assume, we need a bit field per component to reflect supported loopback modes.

If you have time, please take a look at net/core/selftests.c this will be
one of consumers which should walk over different loopback levels to find the
location of potential problem.

Regards,
Oleksij
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |