linux-kernel - Re: [PATCH net-next v2 6/6] net: phy: microchip_t1s: add support for Microchip LAN865x Rev.B0 PHYs

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 23 May 2023 06:13:50 +0000
From:   <Parthiban.Veerasooran@...rochip.com>
To:     <andrew@...n.ch>
CC:     <hkallweit1@...il.com>, <linux@...linux.org.uk>,
        <davem@...emloft.net>, <edumazet@...gle.com>, <kuba@...nel.org>,
        <pabeni@...hat.com>, <netdev@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>,
        <ramon.nordin.rodriguez@...roamp.se>,
        <Horatiu.Vultur@...rochip.com>, <Woojung.Huh@...rochip.com>,
        <Nicolas.Ferre@...rochip.com>, <Thorsten.Kummermehr@...rochip.com>
Subject: Re: [PATCH net-next v2 6/6] net: phy: microchip_t1s: add support for
 Microchip LAN865x Rev.B0 PHYs

Hi Andrew,

On 22/05/23 6:26 pm, Andrew Lunn wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
>> +static int lan865x_setup_cfgparam(struct phy_device *phydev)
>> +{
>> +     u16 cfg_results[5];
>> +     u16 cfg_params[ARRAY_SIZE(lan865x_revb0_fixup_cfg_regs)];
>> +     s8 offsets[2];
>> +     int ret;
> 
> Reverse Christmas tree please.
Ah yes, surely will correct it in the next version.
> 
>> +
>> +     ret = lan865x_generate_cfg_offsets(phydev, offsets);
>> +     if (ret)
>> +             return ret;
>> +
>> +     ret = lan865x_read_cfg_params(phydev, cfg_params);
> 
> Is this doing a read from fuses? Is anything documented about this?
> What the values mean? Would a board designer ever need to use
> different values? Or is this just a case of 'trust us', you don't need
> to understand this magic.
Yes, it is a read from fuses and those values are specific/unique for 
each PHY chip. Those values are calculated based on some characteristics 
of the PHY chip behavior for optimal performance and they are fused in 
the PHY chip for the driver to configure it during the initialization. 
This is done in the production/testing stage of the PHY chip. As it is 
specific to PHY chip, a board designer doesn't have any influence on 
this and need not to worry about it. Unfortunately they can't be 
documented anywhere as they are design specific. So simply 'trust us'.

Best Regards,
Parthiban V
> 
>> +     if (ret)
>> +             return ret;
>> +
>> +     cfg_results[0] = (cfg_params[0] & 0x000F) |
>> +                       FIELD_PREP(GENMASK(15, 10), 9 + offsets[0]) |
>> +                       FIELD_PREP(GENMASK(15, 4), 14 + offsets[0]);
>> +     cfg_results[1] = (cfg_params[1] & 0x03FF) |
>> +                       FIELD_PREP(GENMASK(15, 10), 40 + offsets[1]);
>> +     cfg_results[2] = (cfg_params[2] & 0xC0C0) |
>> +                       FIELD_PREP(GENMASK(15, 8), 5 + offsets[0]) |
>> +                       (9 + offsets[0]);
>> +     cfg_results[3] = (cfg_params[3] & 0xC0C0) |
>> +                       FIELD_PREP(GENMASK(15, 8), 9 + offsets[0]) |
>> +                       (14 + offsets[0]);
>> +     cfg_results[4] = (cfg_params[4] & 0xC0C0) |
>> +                       FIELD_PREP(GENMASK(15, 8), 17 + offsets[0]) |
>> +                       (22 + offsets[0]);
>> +
>> +     return lan865x_write_cfg_params(phydev, cfg_results);
>> +}
> 
> 
>          Andrew
>