linux-kernel - Re: [PATCH v4 net-next 4/5] drivers/net/phy: add helpers to get/set PLCA configuration

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: <Y49Ky7aCUZxE5Fwg@lunn.ch>
Date:   Tue, 6 Dec 2022 14:59:39 +0100
From:   Andrew Lunn <andrew@...n.ch>
To:     Piergiorgio Beruto <piergiorgio.beruto@...il.com>
Cc:     Heiner Kallweit <hkallweit1@...il.com>,
        Russell King <linux@...linux.org.uk>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>, linux-kernel@...r.kernel.org,
        netdev@...r.kernel.org, Oleksij Rempel <o.rempel@...gutronix.de>
Subject: Re: [PATCH v4 net-next 4/5] drivers/net/phy: add helpers to get/set
 PLCA configuration

> +/* MDIO Manageable Devices (MMDs). */
> +#define MDIO_MMD_OATC14		MDIO_MMD_VEND2

As i said in a comment somewhere, i would prefer you use
MDIO_MMD_VEND2, not MDIO_MMD_OATC14. We want the gentle reminder that
these registers can contain anything the vendor wants, including, but
not limited to, PLCA.

> +/* Open Alliance TC14 PLCA CTRL0 register */
> +#define MDIO_OATC14_PLCA_EN	0x8000  /* PLCA enable */
> +#define MDIO_OATC14_PLCA_RST	0x4000  /* PLCA reset */

These are bits, so use the BIT macro. When this was part of mii.h,
that file used this hex format so it made sense to follow that
format. Now you are in a few file, you should use the macro.

> +/* Open Alliance TC14 PLCA CTRL1 register */
> +#define MDIO_OATC14_PLCA_NCNT	0xff00	/* PLCA node count */
> +#define MDIO_OATC14_PLCA_ID	0x00ff	/* PLCA local node ID */
> +
> +/* Open Alliance TC14 PLCA STATUS register */
> +#define MDIO_OATC14_PLCA_PST	0x8000	/* PLCA status indication */
> +
> +/* Open Alliance TC14 PLCA TOTMR register */
> +#define MDIO_OATC14_PLCA_TOT	0x00ff
> +
> +/* Open Alliance TC14 PLCA BURST register */
> +#define MDIO_OATC14_PLCA_MAXBC	0xff00
> +#define MDIO_OATC14_PLCA_BTMR	0x00ff
> +
> +#endif /* __MDIO_OPEN_ALLIANCE__ */
> diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c
> index a87a4b3ffce4..dace5d3b29ad 100644
> --- a/drivers/net/phy/phy-c45.c
> +++ b/drivers/net/phy/phy-c45.c
> @@ -8,6 +8,8 @@
>  #include <linux/mii.h>
>  #include <linux/phy.h>
>  
> +#include "mdio-open-alliance.h"
> +
>  /**
>   * genphy_c45_baset1_able - checks if the PMA has BASE-T1 extended abilities
>   * @phydev: target phy_device struct
> @@ -931,6 +933,184 @@ int genphy_c45_fast_retrain(struct phy_device *phydev, bool enable)
>  }
>  EXPORT_SYMBOL_GPL(genphy_c45_fast_retrain);
>  
> +/**
> + * genphy_c45_plca_get_cfg - get PLCA configuration from standard registers
> + * @phydev: target phy_device struct
> + * @plca_cfg: output structure to store the PLCA configuration
> + *
> + * Description: if the PHY complies to the Open Alliance TC14 10BASE-T1S PLCA
> + *   Management Registers specifications, this function can be used to retrieve
> + *   the current PLCA configuration from the standard registers in MMD 31.
> + */
> +int genphy_c45_plca_get_cfg(struct phy_device *phydev,
> +			    struct phy_plca_cfg *plca_cfg)
> +{
> +	int ret;
> +
> +	ret = phy_read_mmd(phydev, MDIO_MMD_OATC14, MDIO_OATC14_PLCA_IDVER);
> +	if (ret < 0)
> +		return ret;
> +
> +	plca_cfg->version = ret;

It would be good to verify this value, and return -ENODEV if it is not
valid.

	Andrew