RE: [PATCH net-next] net: phy: microchip_t1: add cable test support for lan87xx phy

-----Original Message-----
From: Andrew Lunn <andrew@lunn.ch>
Sent: Friday, October 22, 2021 3:57 PM
To: Yuiko Oshino - C18177 <redacted>
Cc: davem@davemloft.net; netdev@vger.kernel.org; Nisar Sayed - I17970
[off-list ref]; UNGLinuxDriver
[off-list ref]
Subject: Re: [PATCH net-next] net: phy: microchip_t1: add cable test support for
lan87xx phy

EXTERNAL EMAIL: Do not click links or open attachments unless you know the
content is safe

quoted

+static int lan87xx_cable_test_start(struct phy_device *phydev) {
+     static const struct access_ereg_val cable_test[] = {
+             /* min wait */
+             {PHYACC_ATTR_MODE_WRITE, PHYACC_ATTR_BANK_DSP, 93,
+              0, 0},
+             /* max wait */
+             {PHYACC_ATTR_MODE_WRITE, PHYACC_ATTR_BANK_DSP, 94,
+              10, 0},
+             /* pulse cycle */
+             {PHYACC_ATTR_MODE_WRITE, PHYACC_ATTR_BANK_DSP, 95,
+              90, 0},
+             /* cable diag thresh */
+             {PHYACC_ATTR_MODE_WRITE, PHYACC_ATTR_BANK_DSP, 92,
+              60, 0},
+             /* max gain */
+             {PHYACC_ATTR_MODE_WRITE, PHYACC_ATTR_BANK_DSP, 79,
+              31, 0},
+             /* clock align for each iteration */
+             {PHYACC_ATTR_MODE_MODIFY, PHYACC_ATTR_BANK_DSP, 55,
+              0, 0x0038},
+             /* max cycle wait config */
+             {PHYACC_ATTR_MODE_WRITE, PHYACC_ATTR_BANK_DSP, 94,
+              70, 0},
+             /* start cable diag*/
+             {PHYACC_ATTR_MODE_WRITE, PHYACC_ATTR_BANK_DSP, 90,
+              1, 0},
+     };
+     int rc, i;
+
+     rc = microchip_cable_test_start_common(phydev);
+     if (rc < 0)
+             return rc;
+
+     /* start cable diag */
+     /* check if part is alive - if not, return diagnostic error */
+     rc = access_ereg(phydev, PHYACC_ATTR_MODE_READ,

PHYACC_ATTR_BANK_SMI,

quoted

+                      0x00, 0);
+     if (rc < 0)
+             return rc;
+
+     if (rc != 0x2100)
+             return -ENODEV;

What does this actually mean? Would -EOPNOTSUPP be better?

This register should return the value of 0x2100. So if the return value is different, then I assume there is no device.

quoted

+static int lan87xx_cable_test_report_trans(u32 result) {
+     switch (result) {
+     case 0:
+             return ETHTOOL_A_CABLE_RESULT_CODE_OK;
+     case 1:
+             return ETHTOOL_A_CABLE_RESULT_CODE_OPEN;
+     case 2:
+             return ETHTOOL_A_CABLE_RESULT_CODE_SAME_SHORT;

Please add some #defines for 0, 1, 2.

Sure, will do.

      Andrew

Thank you.
Yuiko