Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
Date:   Fri, 26 Aug 2022 11:26:12 +0200
From:   Michael Walle <michael@walle.cc>
To:     Divya.Koppera@microchip.com,
        Oleksij Rempel <o.rempel@pengutronix.de>
Cc:     UNGLinuxDriver@microchip.com, andrew@lunn.ch, davem@davemloft.net,
        edumazet@google.com, hkallweit1@gmail.com, kuba@kernel.org,
        linux-kernel@vger.kernel.org, linux@armlinux.org.uk,
        netdev@vger.kernel.org, pabeni@redhat.com
Subject: Re: [PATCH net-next] net: phy: micrel: Adding SQI support for lan8814
 phy
In-Reply-To: <CO1PR11MB477162C762EF35B0E115B952E2759@CO1PR11MB4771.namprd11.prod.outlook.com>
References: <20220825080549.9444-1-Divya.Koppera@microchip.com>
 <20220826084249.1031557-1-michael@walle.cc>
 <CO1PR11MB477162C762EF35B0E115B952E2759@CO1PR11MB4771.namprd11.prod.outlook.com>
User-Agent: Roundcube Webmail/1.4.13
Message-ID: <421712ea840fbe5edffcae4a6cb08150@walle.cc>
Content-Type: text/plain; charset=US-ASCII;
 format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

[+ Oleksij Rempel]

Hi,

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

..

>> > +#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@pengutronix.de/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