Re: [PATCH v3 1/3] module: add SCMI device table alias support

Uwe Kleine-König <[email protected]> Mon, 24 Aug 2026 14:22:29 +0200

Newsgroups	org.infradead.lists.linux-arm-kernel,dev.linux.lists.imx,org.kernel.vger.arm-scmi,org.kernel.vger.linux-arm-msm,org.kernel.vger.linux-kernel
Message-ID	<aow2GFAfy84k-N2B@monoceros>

Hello,

I just replied to v2 and only then found out there is a v3 already.
There is something wrong with my mail filtering, sorry.

I'm forwarding my review comments to here (the part that still applies).

On Thu, Aug 20, 2026 at 09:48:48PM +0200, Hans de Goede wrote:
> diff --git a/include/linux/device-id/scmi.h b/include/linux/device-id/scmi.h
> new file mode 100644
> index 000000000000..4150481524d4
> --- /dev/null
> +++ b/include/linux/device-id/scmi.h
> @@ -0,0 +1,19 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +#ifndef LINUX_DEVICE_ID_SCMI_H
> +#define LINUX_DEVICE_ID_SCMI_H
> +
> +#ifdef __KERNEL__
> +#include <linux/types.h>
> +typedef unsigned long kernel_ulong_t;
> +#endif
> +
> +#define SCMI_NAME_SIZE		32
> +#define SCMI_MODULE_PREFIX	"scmi:"
> +
> +struct scmi_device_id {
> +	__u8 protocol_id;
> +	char name[SCMI_NAME_SIZE];
> +	kernel_ulong_t driver_data;

Please use

	union {
		kernel_ulong_t driver_data;
		const void *driver_data_ptr;
	};

here instead of the plain driver_data member (or don't add driver_data
at all if not needed). This yields better type-safety, see e.g.
https://lore.kernel.org/all/[email protected]
for an extended explanation.

> +};
> +
> +#endif /* ifndef LINUX_DEVICE_ID_SCMI_H */
> diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
> index a397213bedac..d241421c36aa 100644
> --- a/include/linux/mod_devicetable.h
> +++ b/include/linux/mod_devicetable.h
> @@ -49,6 +49,7 @@
>  #include "device-id/pnp.h"
>  #include "device-id/rio.h"
>  #include "device-id/rpmsg.h"
> +#include "device-id/scmi.h"

Please don't add new includes to mod_devicetable.h. That header is ugly
and each include makes it worse. Just add <linux/device-id/scmi.h> to
the modpost source.

>  #include "device-id/sdio.h"
>  #include "device-id/sdw.h"
>  #include "device-id/serio.h"

Thanks
Uwe

signature.asc (application/pgp-signature, 488 B)