Re: [PATCH v1 3/3] net: mtk_eth: an8855: sync SerDes analog tuning with Linux driver

[Mikhail Zhilkin <csharper2005@gmail.com>]

Hi David,

On 11/10/26 00:25, David Lechner wrote:
>> +	/* TX FIR - improve TX EYE */
> 
> Probably should mention in this comment that these are 2500BASE-X values.

Agreed, I'll update the comment to clarify this.

>> +	an8855_reg_read(priv, AN8855_INTF_CTRL_10, &val);
>> +	val &= ~(0x1f << 24);
>> +	val |= (0x04 << 24);
>> +	val &= ~(0x1f << 16);
>> +	val |= (0x00 << 16);
> 
> nit: don't really need this line - unless we are going to follow up later
> with macros to give all of these names. And using GENMAK/FIELD_PREP/etc
> in a followup would be nice too. Don't combine that with fixes patches
> though.

Good point. I'll drop the redundant bitwise OR line in v2. I agree that
migrating to GENMASK/FIELD_PREP would be great for readability.

>> +	/* RX CDR - improve RX Jitter Tolerance */
>> +	an8855_reg_read(priv, AN8855_RG_QP_CDR_LPF_BOT_LIM, &val);
>> +	val &= ~(0x7 << 24);
>> +	val |= (0x5 << 24);
>> +	val &= ~(0x7 << 20);
>> +	val |= (0x5 << 20);
>> +	an8855_reg_write(priv, AN8855_RG_QP_CDR_LPF_BOT_LIM, val);
>> +
> 
> I don't know if order matters here, but it looks like the proposed Linux
> driver does this at the beginning of the init sequence rather than at the
> end.

You're right, the Linux driver applies these tuning parameters earlier.
I will move this block to the beginning of the init sequence in v2 to
mirror the behavior.

Thanks!
Mikhail