Re: [PATCH RFC net-next 06/23] net: dsa: lantiq_gswip: load model-specific microcode

From: Daniel Golle
Date: Tue Aug 19 2025 - 10:11:06 EST


On Tue, Aug 19, 2025 at 03:17:31PM +0200, Andrew Lunn wrote:
> > I didn't consider that the size of the array elements needs to be known
> > when defining struct gswip_hw_info in lantiq_gswip.h.
> > So the only reasonable solution is to make also the definition of
> > struct gswip_pce_microcode into lantiq_gswip.h, so lantiq_pce.h won't
> > have to be included before or by lantiq_gswip.h itself.
>
> What i've done in the past is define a structure which describes the
> firmware. Two members, a pointer to the list of values, and a length
> of the list of values. You can construct this structure using
> ARRAY_SIZE(), and export it.

It is true that with one more layer of indirection I could have a separate
(allocated) struct gswip_pce_firmware with two elements
.pce_microcode = &gswip_pce_microcode,
.pce_microcode_size = ARRAY_SIZE(gswip_pce_microcode),

However, see below why I don't think this would actually solve the whole
problem.

> You should then be able to put the odd
> val4, val3, val2, val1 structure, and the macro together in one header
> file, and use it in two places to define the firmware blobs for the
> different devices.

I think this is the root of the misunderstanding here.
The odd val4, val3, val2, val1 struct is not only used to define the
microcode instructions, but it is also used to load the microcode into
the hardware, see
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/dsa/lantiq_gswip.c?h=v6.17-rc2#n743

Now, as long as there is only one header lantiq_pce.h defining such
microcode along with the odd struct, all this is not a problem.
However, the problem arises once there is more than one such header,
which will be required to support the newer MaxLinear hardware.
Which of the two (lantiq_pce.h or gsw1xx_pce.h?) is suppose to define
the struct? Which of the two should be included in the share module
('lantiq_gswip_common') which will take care of loading the microcode
into the hardware? (imho: none of them, both headers are hardware
specific and should be included by the respective specific modules
for either switch family)