netdev - RE: [PATCH net-next] net: phy: microchip_t1: add cable test support for lan87xx 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]

Date:   Wed, 27 Oct 2021 14:30:38 +0000
From:   <Yuiko.Oshino@...rochip.com>
To:     <andrew@...n.ch>
CC:     <davem@...emloft.net>, <netdev@...r.kernel.org>,
        <Nisar.Sayed@...rochip.com>, <UNGLinuxDriver@...rochip.com>
Subject: RE: [PATCH net-next] net: phy: microchip_t1: add cable test support
 for lan87xx phy

>-----Original Message-----
>From: Andrew Lunn <andrew@...n.ch>
>Sent: Friday, October 22, 2021 3:57 PM
>To: Yuiko Oshino - C18177 <Yuiko.Oshino@...rochip.com>
>Cc: davem@...emloft.net; netdev@...r.kernel.org; Nisar Sayed - I17970
><Nisar.Sayed@...rochip.com>; UNGLinuxDriver
><UNGLinuxDriver@...rochip.com>
>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
>
>> +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,
>> +                      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.
>
>> +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