Re: [PATCH net-next] net: phy: micrel: Adding SQI support for lan8814 phy

From: Michael Walle
Date: Fri Aug 26 2022 - 05:27:13 EST


[+ Oleksij Rempel]

Hi,

Am 2022-08-26 11:11, schrieb Divya.Koppera@xxxxxxxxxxxxx:
> 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@xxxxxxxxxxxxx>

..

> +#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.

This register values are present in GPHY hard macro as below

4.2.225 DCQ Control Register
Index (In Decimal): EP 1.230 Size: 16 bits

Can you give me the name of the datasheet which you are following, so
that I'll check and let you know the reason.

I have the AN4286/DS00004286A ("LAN8804/LAN8814 GPHY Register
Definitions"). Maybe there is a newer version of it.


So I can only guess that this "channel mask" is for the 4 rx/tx pairs on GbE?

Yes channel mask is for wire pair.

And you only seem to evaluate one of them. Is that the correct thing to do
here?


I found in below link is that, get_SQI returns sqi value for 100 base-t1 phy's
https://lore.kernel.org/netdev/20200519075200.24631-2-o.rempel@xxxxxxxxxxxxxx/T/

That one is for the 100base-t1 which has only one pair.

In lan8814 phy only channel 0 is used for 100base-tx. So returning SQI
value for channel 0.

What if the other pairs are bad? Maybe Oleksij has an opinion here.

Also 100baseTX (and 10baseT) has two pairs, one for transmitting and one
for receiving. I guess you meassure the SQI on the receiving side. So is
channel 0 correct here?

Again this is the first time I hear about SQI but it puzzles me that
it only evaluate one pair in this case. So as a user who reads this
SQI might be misleaded.

-michael