Thread (69 messages) 69 messages, 8 authors, 2026-02-27

Re: [PATCH v8 3/4] gpio: rpmsg: add generic rpmsg GPIO driver

From: Shenwei Wang <shenwei.wang@nxp.com>
Date: 2026-02-20 19:09:33
Also in: imx, linux-devicetree, linux-doc, linux-gpio, linux-remoteproc, lkml

-----Original Message-----
From: Andrew Lunn <andrew@lunn.ch>
Sent: Friday, February 20, 2026 11:42 AM
To: Shenwei Wang <shenwei.wang@nxp.com>
Cc: Arnaud POULIQUEN <arnaud.pouliquen@foss.st.com>; Linus Walleij
[off-list ref]; Bartosz Golaszewski [off-list ref]; Jonathan Corbet
[off-list ref]; Rob Herring [off-list ref]; Krzysztof Kozlowski
[off-list ref]; Conor Dooley [off-list ref]; Bjorn Andersson
[off-list ref]; Mathieu Poirier [off-list ref]; Frank Li
[off-list ref]; Sascha Hauer [off-list ref]; Shuah Khan
[off-list ref]; linux-gpio@vger.kernel.org; linux-
doc@vger.kernel.org; linux-kernel@vger.kernel.org; Pengutronix Kernel Team
[off-list ref]; Fabio Estevam [off-list ref]; Peng Fan
[off-list ref]; devicetree@vger.kernel.org; linux-
remoteproc@vger.kernel.org; imx@lists.linux.dev; linux-arm-
kernel@lists.infradead.org; dl-linux-imx [off-list ref]; Bartosz
Golaszewski [off-list ref]
Subject: [EXT] Re: [PATCH v8 3/4] gpio: rpmsg: add generic rpmsg GPIO driver
quoted
quoted
quoted
struct msg_hdr {
    u8  id;
    u32 size;
    u8  flags;
};
That is just a bad design. The point of not allowing __packed is
that it forces you to design your structures correctly. Maybe AI has
no idea of taste, but Maintainer do and would not allow a u32 to be unaligned
like this.
quoted
quoted
Let’s keep the discussion technical instead of taste. 😊

My point with the earlier example was simply to illustrate how layout
differences can happen across architectures or compilers. I’m more
interested in understanding how you would prefer this specific
structure to be defined so that it avoids unaligned fields while still maintaining a
stable on‑wire format.

struct msg_hdr {
    u32 size;
    u8  id;
    u8  flags;
};

The compiler will lay this out as you expect, it won't add any padding between
the fields.
Yes, most compilers will lay this out without inserting padding between the fields. 
However, for a communication packet, we cannot freely reorder or tweak members 
just to satisfy alignment rules—the field order itself becomes part of the protocol definition.

Even within netdev, where you’re very familiar, the classic ethhdr still carries the packed annotation 
despite being naturally aligned:

struct ethhdr {
	unsigned char	h_dest[ETH_ALEN];	/* destination eth addr	*/
	unsigned char	h_source[ETH_ALEN];	/* source ether addr	*/
	__be16		h_proto;		/* packet type ID field	*/
} __attribute__((packed));

Thanks,
Shenwei
You need to be careful with sizeof(struct msg_hdr). On 8 and 16 bit machines, it is
probably 6. On 32, or 64 bit machine it is probably 8.

I would say having a 6 byte message is probably a bad design, and you should try
to make the u32 a u16 if you can. Again, not using the __packed is making you
think about the design, and probably makes the design better as a result.

    Andrew
  
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help