netdev - RE: [PATCH net] net: phy: microchip_t1: add 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] [day] [month] [year] [list]

Message-ID: <BL0PR11MB28976D2B5BB9A7CCCE5C99608ECB0@BL0PR11MB2897.namprd11.prod.outlook.com>
Date:   Mon, 30 Mar 2020 13:55:09 +0000
From:   <Yuiko.Oshino@...rochip.com>
To:     <davem@...emloft.net>
CC:     <davem@...emloft.net>, <UNGLinuxDriver@...rochip.com>,
        <netdev@...r.kernel.org>
Subject: RE: [PATCH net] net: phy: microchip_t1: add lan87xx_phy_init to
 initialize the lan87xx phy.

>From: David Miller <davem@...emloft.net>
>Sent: Saturday, March 21, 2020 11:12 PM

>
>From: Yuiko Oshino <yuiko.oshino@...rochip.com>
>Date: Tue, 17 Mar 2020 15:08:38 -0400
>
>> lan87xx_phy_init() initializes the lan87xx phy.
>>
>> fixes: 3e50d2da5850 ("Add driver for Microchip LAN87XX T1 PHYs")
>> Signed-off-by: Yuiko Oshino <yuiko.oshino@...rochip.com>
>
>"Fixes: " should be capitalized.
>
>You commit message is way too terse.
>
>> +     static const struct access_ereg_val init[] = {
>> +             {PHYACC_ATTR_MODE_WRITE, PHYACC_ATTR_BANK_AFE, 0x0B,
>0x000A},
>> +             {PHYACC_ATTR_MODE_READ, PHYACC_ATTR_BANK_MISC, 0x8, 0},
>> +             {PHYACC_ATTR_MODE_WRITE, PHYACC_ATTR_BANK_MISC, 0x8, 0},
>> +             {PHYACC_ATTR_MODE_WRITE, PHYACC_ATTR_BANK_MISC, 0x10,
>0x0009},
>> +             {PHYACC_ATTR_MODE_WRITE, PHYACC_ATTR_BANK_SMI, 0x17, 0},
>> +             /* TC10 Config */
>> +             {PHYACC_ATTR_MODE_WRITE, PHYACC_ATTR_BANK_MISC, 0x20,
>0x0023},
>> +             {PHYACC_ATTR_MODE_WRITE, PHYACC_ATTR_BANK_PCS, 0x20,
>0x3C3C},
>> +             {PHYACC_ATTR_MODE_WRITE, PHYACC_ATTR_BANK_MISC, 0x21,
>0x274F},
>> +             {PHYACC_ATTR_MODE_WRITE, PHYACC_ATTR_BANK_MISC, 0x20,
>0x80A7},
>> +             {PHYACC_ATTR_MODE_WRITE, PHYACC_ATTR_BANK_MISC, 0x24,
>0x9110},
>> +             {PHYACC_ATTR_MODE_WRITE, PHYACC_ATTR_BANK_MISC, 0x20,
>0x0087},
>> +             /* HW Init */
>> +             {PHYACC_ATTR_MODE_WRITE, PHYACC_ATTR_BANK_SMI, 0x1A,
>0x0300},
>> +     };
>
>What do these registers do, and what do the values programmed into them do?
>
>If you don't know, how can you know if this code is correct?
>
>You must document this as much as possible.
>
>Thank you.

Hi David,
Thank you for your comments.
I will improve and submit v1.
The initialization scripts also will be changed as our hardware engineers updated them.

Thank you.
Yuiko