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

[<Parthiban.Veerasooran@microchip.com>]

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
>