Thread (14 messages) 14 messages, 3 authors, 2026-02-07

Re: [PATCH net-next v13 4/4] net: dsa: add basic initial driver for MxL862xx switches

From: Vladimir Oltean <olteanv@gmail.com>
Date: 2026-02-07 22:14:37
Also in: linux-devicetree, lkml

On Fri, Feb 06, 2026 at 04:43:19PM +0000, Daniel Golle wrote:
I've spent an hour studying the pack_fields() API and it's (well
written) documentation. The only example of it's use in the current
kernel I could find is the Intel E800 (ICE) driver. And there it does
make sense as it is handling conversion between CPU and hardware formats
in the hotpath for DMA descriptors, a total of 3 different structs, each
with their individual accessor functions.

Using this approach for this switch driver would require writing a lot
of boilerplate code, accessor functions for each and every struct,
and a struct definition once unpacked for the host platform and then
again using the PACKED_FIELD(...) notation for the hardware format.
Surely, most of that could be auto-generated using the existing
vendor drivers API definition. Yet (at least to me) it feels like
over-engineering and also it would require rewriting most of the driver
which has been discussed for almost 2 months now.

Also note that the driver doesn't need the naturally aligned version of
all these structs in native CPU endian -- they are not used for further
processing anything, you can see that because they aren't ever used as a
function parameters, but only ever as exchange formats when
communicating with the firmware.
OK. If the fields were packed more densely maybe the tradeoff would have
looked differently then. But you're talking to an MCU and not to hardware.
And you don't need to keep a local direct representation of the data
passed through those packed buffers. Your arguments are valid.
Maybe I'm missing something obvious here and there is a more simple way
to use this API, some generic macros using compiler introspection to
magically handle everything without needing to write packed and unpacked
struct definitions and individual pack/unpack boiler-plate functions for
each struct. If so, please provide me with an example or explain how you
imagine the pack_fields() API to be used in the context of this driver
and it's total of at more than 30 different structs which will be used
for all the different firmware function I will need to use in order to
implement phylink_pcs as well as the various offloading and VLAN-related
functionality the driver should have in the end (ie. the structs you
currently see in the mxl862xx-api.h file are just a fraction of what I
hope to add there by follow-up series)
No, the API usage example is how you imagine it. There's a structure
where you only need to pull in the fields you care about (and in
whatever order), rather than every other unrelated tidbit you don't
currently need and possibly never will (like ingress_marking_mode, etc etc).
And another array of PACKED_FIELD() where you say where each field goes.
You probably don't need a pack_fields() and an unpack_fields() call for
the same data structure in most cases, just one or the other.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help