Re: [PATCH phy-next 5/5] phy: lynx-28g: add support for 25GBASER

[Josua Mayer]

Am 13.05.26 um 13:22 schrieb Vladimir Oltean:

> On Wed, May 13, 2026 at 11:00:32AM +0000, Josua Mayer wrote:
>> Wouldn't it be more clear instead of indirect lane offset shift with
>> lynx_28g_e25g_pcvt, to instead fix the E25G_CFG definition?:
>>
>> -#define E25G_CFG(id) (28 - (id) * 4) /* Offset into PCCD */
>> +#define E25G_CFG(id) ((id) * 4)      /* Offset into PCCD */
>>
>> This is equivalent when inserting (7 - lane) into E25G_CFG id:
>>
>> (28 - (id) * 4) = (28 - (7 - lane) * 4) = (28 - 7*4 + lane*4)
> The 'indirect' lane offset shift is actually a two-step lookup:
> lane -> protocol converter index
> protocol converted index -> offset into PCCD
>
> LX2160ARM documents PCCD fields as:
> E25GA_CFG, aka E25G_CFG(0) in code: 30:28
> E25GB_CFG, aka E25G_CFG(1) in code: 26:24
> ...
> E25GH_CFG, aka E25G_CFG(7) in code: 2:0
>
> The odd bit is that lane 0 uses E25G protocol converter 7, unlike, say,
> 1G and 10G where we have a lane:pcvt identity mapping.
> lynx_28g_e25g_pcvt() performs that translation.
>
> Additionally, for locating E40GA_CFG, E40GB_CFG in PCCE, I've adopted
> the same scheme downstream, where E40G_CFG() returns bits 30:28 for
> argument 0 (pcvt A) and bits 26:24 for argument 1 (pcvt B), and it is
> called with this lane->pcvt translation function:
>
> static int lynx_28g_e40g_pcvt(int lane)
> {
> 	return lane < 4 ? 1 : 0;
> }
>
> Are you saying that merging the two lookups would be more clear because
> as a reader you'd get to ask yourself less questions (the code would
> flow more naturally) despite the non-trivial lane<->pcvt mapping,
Exactly for this reason. Fewer look-up steps.
RM dos not document this mapping, but now you do in source-code.

Perhaps a short comment on the function will help, e.g.
/* get protocol converter id for lane */

> or why
> exactly? For me it is the exact opposite. I can follow the RM
> definitions and then I have a separate function that tells me how lanes
> are mapped to the protocol converters.
I think you give a good reason here!