linux-kernel - Re: [PATCH net-next] net: phy: micrel: Adding SQI support for lan8814 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: <20220826084249.1031557-1-michael@walle.cc>
Date:   Fri, 26 Aug 2022 10:42:49 +0200
From:   Michael Walle <michael@...le.cc>
To:     divya.koppera@...rochip.com
Cc:     UNGLinuxDriver@...rochip.com, andrew@...n.ch, davem@...emloft.net,
        edumazet@...gle.com, hkallweit1@...il.com, kuba@...nel.org,
        linux-kernel@...r.kernel.org, linux@...linux.org.uk,
        netdev@...r.kernel.org, pabeni@...hat.com,
        Michael Walle <michael@...le.cc>
Subject: Re: [PATCH net-next] net: phy: micrel: Adding SQI support for lan8814 phy

Hi,

> Supports SQI(Signal Quality Index) for lan8814 phy, where
> it has SQI index of 0-7 values and this indicator can be used
> for cable integrity diagnostic and investigating other noise
> sources.
> 
> Signed-off-by: Divya Koppera <Divya.Koppera@...rochip.com>
> ---
>  drivers/net/phy/micrel.c | 35 +++++++++++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
> 
> diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
> index e78d0bf69bc3..3775da7afc64 100644
> --- a/drivers/net/phy/micrel.c
> +++ b/drivers/net/phy/micrel.c
> @@ -1975,6 +1975,13 @@ static int ksz886x_cable_test_get_status(struct phy_device *phydev,
>  #define LAN8814_CLOCK_MANAGEMENT			0xd
>  #define LAN8814_LINK_QUALITY				0x8e
>  
> +#define LAN8814_DCQ_CTRL				0xe6
> +#define LAN8814_DCQ_CTRL_READ_CAPTURE_			BIT(15)

Why does it end with an underscore?

> +#define LAN8814_DCQ_CTRL_CHANNEL_MASK			GENMASK(1, 0)
> +#define LAN8814_DCQ_SQI					0xe4
> +#define LAN8814_DCQ_SQI_MAX				7
> +#define LAN8814_DCQ_SQI_VAL_MASK			GENMASK(3, 1)
> +
>  static int lanphy_read_page_reg(struct phy_device *phydev, int page, u32 addr)
>  {
>  	int data;
> @@ -2927,6 +2934,32 @@ static int lan8814_probe(struct phy_device *phydev)
>  	return 0;
>  }
>  
> +static int lan8814_get_sqi(struct phy_device *phydev)
> +{
> +	int rc, val;
> +
> +	val = lanphy_read_page_reg(phydev, 1, LAN8814_DCQ_CTRL);
> +	if (val < 0)
> +		return val;
> +
> +	val &= ~LAN8814_DCQ_CTRL_CHANNEL_MASK;

I do have a datasheet for this PHY, but it doesn't mention 0xe6 on EP1.
So I can only guess that this "channel mask" is for the 4 rx/tx pairs
on GbE? And you only seem to evaluate one of them. Is that the correct
thing to do here?

-michael


> +	val |= LAN8814_DCQ_CTRL_READ_CAPTURE_;
> +	rc = lanphy_write_page_reg(phydev, 1, LAN8814_DCQ_CTRL, val);
> +	if (rc < 0)
> +		return rc;
> +
> +	rc = lanphy_read_page_reg(phydev, 1, LAN8814_DCQ_SQI);
> +	if (rc < 0)
> +		return rc;
> +
> +	return FIELD_GET(LAN8814_DCQ_SQI_VAL_MASK, rc);
> +}