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;
From:   <Divya.Koppera@microchip.com>
To:     <michael@walle.cc>, <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
Thread-Topic: [PATCH net-next] net: phy: micrel: Adding SQI support for
 lan8814 phy
Thread-Index: AQHYuFmMgzj2knw/J0+AGHw88JSECa3A3vGAgAACdoCAAAmpAIAEdMlg
Date:   Mon, 29 Aug 2022 05:50:44 +0000
Message-ID: <CO1PR11MB47715CFC7E22969BD00F9E6AE2769@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>
 <421712ea840fbe5edffcae4a6cb08150@walle.cc>
In-Reply-To: <421712ea840fbe5edffcae4a6cb08150@walle.cc>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?a53V4wDgeMP2JpTYGynILsN/eP4+d99eOoTbu8Pk2oOcyVs9V3Qmg7/XegUD?=
 =?us-ascii?Q?zCnwt86Qg06LiHxPPahbklCpOqhYe0CzQitk4f0H08fIQ+cqQucMLjFfVBX6?=
 =?us-ascii?Q?H+dscfnd8w53LnpINU16oRSFOnqYzGpkJHOS6oR9K2yVb+VYFkE/K5zA7fCw?=
 =?us-ascii?Q?gx+uBXExF5mA41O27fBYcn7WvenoyzKBPaGeWrcHRAD7xhBf+VgLrovxRZWz?=
 =?us-ascii?Q?OQB38cZtGY5fjMzfXDLjlXnCFmxQRe4b7T+7ypCa2oBHvg9swNlT61/Fzxvb?=
 =?us-ascii?Q?IdHjwU9OfjC7w4gdyyEt9+4QaErffT36hJ/cCJSJFq/+bRTfe6Aaus5ZlZKz?=
 =?us-ascii?Q?B4axpVjwaTVqkRY8Is/2Ued9JJuCmDXmgHZiAnnTiov4ReKXUOwWJ+nN8JaK?=
 =?us-ascii?Q?9+dc9y4JGBemvRS4thMDE6/QYB0zhiBd8IfqGS0rW3JBeMNeT9jIQ6q5S6eV?=
 =?us-ascii?Q?J8HnQYj4T8Tr54LruWt+uOnGn9beK1qbB5hZinwm4lq6Vs3uGrvqLRankHOK?=
 =?us-ascii?Q?n7sNu2WozFECLoQICjOoRr4d2idC5sSxeEokJaaqu6FTwvwCtlKJrTiDeDkA?=
 =?us-ascii?Q?k/5DNIumkdUY7+BTOoiayDaN7rPv76d+cMxFEX750oFXCYDupMhbmZMr19dZ?=
 =?us-ascii?Q?NHZe6xKy/2xdKERqqHez3HPolMeopPxK+hUNX4f2MxChINkChmQb/OAayldP?=
 =?us-ascii?Q?0OhYzN3QCdaX8hXBUle27AVi+hhyori3kcv6XkOTzQaYQs3UGP+fz4LfDIlF?=
 =?us-ascii?Q?eZvu7Bll0ISI2SBZ/Msb3+H9mWqXHOhCEbEv1wn5BHQXUVPcI2mdRtdDOXUw?=
 =?us-ascii?Q?zftWb+VWQfwzxqCrWImWdQC5WI/4ngZxgp13B9+wTOPH7FXeYvHQ6LTH/MsU?=
 =?us-ascii?Q?FS8cIOz1S1e4hLrdNxLWMU8jqsJHNrm8RTpiVZMRORY4wjJFaxBfvG5XUkA5?=
 =?us-ascii?Q?vYYtPcVcJcOPOOcWkiRIjVBqJ9f2Q9p0T4LMCy/OO9xZOQLXRapJNK75p39n?=
 =?us-ascii?Q?VAhMPE2WqUZ5FZd/UElylKzdoM5FG59OodyCcK6N71VvA0kwNfZWdhkqWiYV?=
 =?us-ascii?Q?N2zxNMI55p745naRk7FGD7f3RP+TwFMlLlAd3oFzCavJ5HX49fq1oZ0zOlpH?=
 =?us-ascii?Q?WhMl11n6rBTCl31xXttX0r4grHLUWGGjJXAyzJV1o+T75K3SRnRtttnO9JMb?=
 =?us-ascii?Q?VSLXW6qAEe+roCem6OTrHT/kmBE6zzFpAftU7FAQ7l3+4E38VnOiubsKO2gW?=
 =?us-ascii?Q?sWaYZcqTF4KTyglqd8x+H17z4vaTYxOs7diJCmaWx2QlN9hTnMmyJxxxazVX?=
 =?us-ascii?Q?5razMV4MUAbxyriOGNM8m8XsJbEHFTtDnuzJqJaa9eUWKVrVAEGQ+vhWJD7X?=
 =?us-ascii?Q?RoGShcHa7oYnAREyBx2oE818m6d1a19xLgP0gHlpfBRytg8AZTQiqALnqy7P?=
 =?us-ascii?Q?dkjx29s7Fsn4TmXEabGkOYHDu9lQUZw8p/cWjHZuFdOHwX+kMAnNDNPu4EPv?=
 =?us-ascii?Q?dmy/WiA9H8zwATipZt8MnDvu6WWicEcXy1vwSeiWA2WX1jcZ+t09LLBsZMDD?=
 =?us-ascii?Q?sN6fcpIgDlHSYzmtdi9ENivI1XDK2gjO5XQ/Ddf1okao0O5YIpQ6JMqR6ohE?=
 =?us-ascii?Q?yQ=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: CO1PR11MB4771.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 63d0908d-649e-478e-2de9-08da898269e1
X-MS-Exchange-CrossTenant-originalarrivaltime: 29 Aug 2022 05:50:44.1309
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 3f4057f3-b418-4d4e-ba84-d55b4e897d88
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: ZWQlllKUWDCkKfY4uCgKdD1KdLyqoMcUmManVr6ZDKWR2TrXPoh8lCW5WJOEvleyXKHCaX9EeTO/XiNul8jn2tJaWIwm7ILgG6rKLIKUJh4=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN7PR11MB6994
X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,SPF_HELO_PASS,
        SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no
        version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi Michael,

> -----Original Message-----
> From: Michael Walle <michael@walle.cc>
> Sent: Friday, August 26, 2022 2:56 PM
> To: Divya Koppera - I30481 <Divya.Koppera@microchip.com>; Oleksij Rempel
> <o.rempel@pengutronix.de>
> Cc: UNGLinuxDriver <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
>=20
> EXTERNAL EMAIL: Do not click links or open attachments unless you know th=
e
> content is safe
>=20
> [+ Oleksij Rempel]
>=20
> Hi,
>=20
> 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>
>=20
> ..
>=20
> >> > +#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 =3D lanphy_read_page_reg(phydev, 1, LAN8814_DCQ_CTRL);
> >> > +     if (val < 0)
> >> > +             return val;
> >> > +
> >> > +     val &=3D ~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.
>=20
> I have the AN4286/DS00004286A ("LAN8804/LAN8814 GPHY Register
> Definitions"). Maybe there is a newer version of it.
>=20

I just looked into it, it doesn't have SQI registers. I requested internal =
team to add SQI register set in published document.

> >
> >> 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/
>=20
> That one is for the 100base-t1 which has only one pair.
>=20
> > In lan8814 phy only channel 0 is used for 100base-tx. So returning SQI
> > value for channel 0.
>=20
> What if the other pairs are bad? Maybe Oleksij has an opinion here.
>=20
> 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?
>=20

Yes Channel 0 is correct.

> 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.
>=20

Yeah, It needs uAPI extension.

> -michael