[PATCH v1 0/2] firewire: Simplify storing pointers in device id struct

[PATCH v1 0/2] firewire: Simplify storing pointers in device id struct

[Uwe Kleine-König (The Capable Hub)]

Hello,

<linux/mod_devicetable.h> contains several device_id structs for various
device types.

Most of them have one of:

 - kernel_ulong_t driver_data (sometimes called "driver_info", sometimes 
   the type is plain unsigned long)
 - const void *data (sometimes called "driver_data" or "context", sometimes not const)

A considerable amount of drivers for the first category uses the
unsigned long variable to store a pointer. This involves casting both
for assignment and usage.

An additional complication exists for the CHERI hardware extension
where sizeof(void *) > sizeof(unsigned long). So with that an unsigned
long variable cannot be used to store a pointer.

To address both issues this series replaces the unsigned long variable
by an anonymous union containing both an unsigned long and a pointer.

For all non-CHERI architectures this isn't an ABI change because all
have sizeof(void *) == sizeof(unsigned long).

The first patch changes the definition of struct ieee1394_device_id. The
second drops some casts in sound drivers. (There are no other firewire
drivers that could benefit.) I adapted all sound drivers in a single
patch, tell me if I should split per driver.

For merging I suggest to take the whole series via the ALSA tree in the
next merge window, as there are no modified files that are specific to
firewire only and the second patch depends on the first.

Best regards
Uwe

Uwe Kleine-König (The Capable Hub) (2):
  firewire: Simplify storing pointers in device id struct
  ALSA: firewire: Make use of ieee1394's .driver_data_ptr

 include/linux/mod_devicetable.h |  5 ++++-
 sound/firewire/dice/dice.c      | 34 ++++++++++++++++-----------------
 sound/firewire/fireface/ff.c    | 12 ++++++------
 sound/firewire/motu/motu.c      |  6 +++---
 sound/firewire/oxfw/oxfw.c      |  4 ++--
 5 files changed, 32 insertions(+), 29 deletions(-)


base-commit: 028ef9c96e96197026887c0f092424679298aae8
-- 
2.47.3

[PATCH v1 1/2] firewire: Simplify storing pointers in device id struct

[Uwe Kleine-König (The Capable Hub)]

On all current Linux architectures sizeof(long) == sizeof(void *) and
this is used a lot through the kernel. For example it enables the usual
practice to store pointers in ieee1394_device_id's .driver_data member.

This works fine, but involves casting and thus isn't type-safe.
Additionally with the CHERI architecture extension there are machines
with sizeof(void *) > sizeof(long) for with the traditional approach of
storing a pointer in .driver_data doesn't work.

By replacing the plain unsigned long .driver_data by an anonymous union,
most of the casting can be dropped and it yields a working solution for
CHERI.

All users of struct ieee1394_device_id are initialized in a way that is
compatible with the new definition, so no adaptions are needed there.

Signed-off-by: Uwe Kleine-König (The Capable Hub) <u.kleine-koenig@baylibre.com>
---
 include/linux/mod_devicetable.h | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
index 5b1725fe9707..2ee6b66ca9a2 100644
--- a/include/linux/mod_devicetable.h
+++ b/include/linux/mod_devicetable.h
@@ -61,7 +61,10 @@ struct ieee1394_device_id {
 	__u32 model_id;
 	__u32 specifier_id;
 	__u32 version;
-	kernel_ulong_t driver_data;
+	union {
+		kernel_ulong_t driver_data;
+		const void *driver_data_ptr;
+	};
 };
 
 
-- 
2.47.3

[PATCH v1 2/2] ALSA: firewire: Make use of ieee1394's .driver_data_ptr

[Uwe Kleine-König (The Capable Hub)]

Recently struct ieee1394_device_id gained a new member to store a pointer
to driver data. Make use of that to get rid of a bunch of casts.

Signed-off-by: Uwe Kleine-König (The Capable Hub) <u.kleine-koenig@baylibre.com>
---
 sound/firewire/dice/dice.c   | 34 +++++++++++++++++-----------------
 sound/firewire/fireface/ff.c | 12 ++++++------
 sound/firewire/motu/motu.c   |  6 +++---
 sound/firewire/oxfw/oxfw.c   |  4 ++--
 4 files changed, 28 insertions(+), 28 deletions(-)

diff --git a/sound/firewire/dice/dice.c b/sound/firewire/dice/dice.c
index f7a50bae4b55..58f14aadc73d 100644
--- a/sound/firewire/dice/dice.c
+++ b/sound/firewire/dice/dice.c
@@ -148,7 +148,7 @@ static int dice_probe(struct fw_unit *unit, const struct ieee1394_device_id *ent
 	snd_dice_detect_formats_t detect_formats;
 	int err;
 
-	if (!entry->driver_data && entry->vendor_id != OUI_SSL) {
+	if (!entry->driver_data_ptr && entry->vendor_id != OUI_SSL) {
 		err = check_dice_category(unit);
 		if (err < 0)
 			return -ENODEV;
@@ -164,10 +164,10 @@ static int dice_probe(struct fw_unit *unit, const struct ieee1394_device_id *ent
 	dev_set_drvdata(&unit->device, dice);
 	dice->card = card;
 
-	if (!entry->driver_data)
+	if (!entry->driver_data_ptr)
 		detect_formats = snd_dice_stream_detect_current_formats;
 	else
-		detect_formats = (snd_dice_detect_formats_t)entry->driver_data;
+		detect_formats = entry->driver_data_ptr;
 
 	// Below models are compliant to IEC 61883-1/6 and have no quirk at high sampling transfer
 	// frequency.
@@ -255,7 +255,7 @@ static void dice_bus_reset(struct fw_unit *unit)
 		.model_id	= (model), \
 		.specifier_id	= (vendor), \
 		.version	= DICE_INTERFACE, \
-		.driver_data = (kernel_ulong_t)(data), \
+		.driver_data_ptr = (data), \
 	}
 
 static const struct ieee1394_device_id dice_id_table[] = {
@@ -267,7 +267,7 @@ static const struct ieee1394_device_id dice_id_table[] = {
 				  IEEE1394_MATCH_MODEL_ID,
 		.vendor_id	= OUI_MAUDIO,
 		.model_id	= 0x000010,
-		.driver_data = (kernel_ulong_t)snd_dice_detect_extension_formats,
+		.driver_data_ptr = snd_dice_detect_extension_formats,
 	},
 	/* M-Audio Profire 610 has a different value in version field. */
 	{
@@ -275,7 +275,7 @@ static const struct ieee1394_device_id dice_id_table[] = {
 				  IEEE1394_MATCH_MODEL_ID,
 		.vendor_id	= OUI_MAUDIO,
 		.model_id	= 0x000011,
-		.driver_data = (kernel_ulong_t)snd_dice_detect_extension_formats,
+		.driver_data_ptr = snd_dice_detect_extension_formats,
 	},
 	/* TC Electronic Konnekt 24D. */
 	{
@@ -283,7 +283,7 @@ static const struct ieee1394_device_id dice_id_table[] = {
 				  IEEE1394_MATCH_MODEL_ID,
 		.vendor_id	= OUI_TCELECTRONIC,
 		.model_id	= 0x000020,
-		.driver_data = (kernel_ulong_t)snd_dice_detect_tcelectronic_formats,
+		.driver_data_ptr = snd_dice_detect_tcelectronic_formats,
 	},
 	/* TC Electronic Konnekt 8. */
 	{
@@ -291,7 +291,7 @@ static const struct ieee1394_device_id dice_id_table[] = {
 				  IEEE1394_MATCH_MODEL_ID,
 		.vendor_id	= OUI_TCELECTRONIC,
 		.model_id	= 0x000021,
-		.driver_data = (kernel_ulong_t)snd_dice_detect_tcelectronic_formats,
+		.driver_data_ptr = snd_dice_detect_tcelectronic_formats,
 	},
 	/* TC Electronic Studio Konnekt 48. */
 	{
@@ -299,7 +299,7 @@ static const struct ieee1394_device_id dice_id_table[] = {
 				  IEEE1394_MATCH_MODEL_ID,
 		.vendor_id	= OUI_TCELECTRONIC,
 		.model_id	= 0x000022,
-		.driver_data = (kernel_ulong_t)snd_dice_detect_tcelectronic_formats,
+		.driver_data_ptr = snd_dice_detect_tcelectronic_formats,
 	},
 	/* TC Electronic Konnekt Live. */
 	{
@@ -307,7 +307,7 @@ static const struct ieee1394_device_id dice_id_table[] = {
 				  IEEE1394_MATCH_MODEL_ID,
 		.vendor_id	= OUI_TCELECTRONIC,
 		.model_id	= 0x000023,
-		.driver_data = (kernel_ulong_t)snd_dice_detect_tcelectronic_formats,
+		.driver_data_ptr = snd_dice_detect_tcelectronic_formats,
 	},
 	/* TC Electronic Desktop Konnekt 6. */
 	{
@@ -315,7 +315,7 @@ static const struct ieee1394_device_id dice_id_table[] = {
 				  IEEE1394_MATCH_MODEL_ID,
 		.vendor_id	= OUI_TCELECTRONIC,
 		.model_id	= 0x000024,
-		.driver_data = (kernel_ulong_t)snd_dice_detect_tcelectronic_formats,
+		.driver_data_ptr = snd_dice_detect_tcelectronic_formats,
 	},
 	/* TC Electronic Impact Twin. */
 	{
@@ -323,7 +323,7 @@ static const struct ieee1394_device_id dice_id_table[] = {
 				  IEEE1394_MATCH_MODEL_ID,
 		.vendor_id	= OUI_TCELECTRONIC,
 		.model_id	= 0x000027,
-		.driver_data = (kernel_ulong_t)snd_dice_detect_tcelectronic_formats,
+		.driver_data_ptr = snd_dice_detect_tcelectronic_formats,
 	},
 	/* TC Electronic Digital Konnekt x32. */
 	{
@@ -331,7 +331,7 @@ static const struct ieee1394_device_id dice_id_table[] = {
 				  IEEE1394_MATCH_MODEL_ID,
 		.vendor_id	= OUI_TCELECTRONIC,
 		.model_id	= 0x000030,
-		.driver_data = (kernel_ulong_t)snd_dice_detect_tcelectronic_formats,
+		.driver_data_ptr = snd_dice_detect_tcelectronic_formats,
 	},
 	/* Alesis iO14/iO26. */
 	{
@@ -339,7 +339,7 @@ static const struct ieee1394_device_id dice_id_table[] = {
 				  IEEE1394_MATCH_MODEL_ID,
 		.vendor_id	= OUI_ALESIS,
 		.model_id	= MODEL_ALESIS_IO_BOTH,
-		.driver_data = (kernel_ulong_t)snd_dice_detect_alesis_formats,
+		.driver_data_ptr = snd_dice_detect_alesis_formats,
 	},
 	// Alesis MasterControl.
 	{
@@ -347,7 +347,7 @@ static const struct ieee1394_device_id dice_id_table[] = {
 				  IEEE1394_MATCH_MODEL_ID,
 		.vendor_id	= OUI_ALESIS,
 		.model_id	= 0x000002,
-		.driver_data = (kernel_ulong_t)snd_dice_detect_alesis_mastercontrol_formats,
+		.driver_data_ptr = snd_dice_detect_alesis_mastercontrol_formats,
 	},
 	/* Mytek Stereo 192 DSD-DAC. */
 	{
@@ -355,7 +355,7 @@ static const struct ieee1394_device_id dice_id_table[] = {
 				  IEEE1394_MATCH_MODEL_ID,
 		.vendor_id	= OUI_MYTEK,
 		.model_id	= 0x000002,
-		.driver_data = (kernel_ulong_t)snd_dice_detect_mytek_formats,
+		.driver_data_ptr = snd_dice_detect_mytek_formats,
 	},
 	// Solid State Logic, Duende Classic and Mini.
 	// NOTE: each field of GUID in config ROM is not compliant to standard
@@ -469,7 +469,7 @@ static const struct ieee1394_device_id dice_id_table[] = {
 		.model_id	= OUI_TEAC,
 		.specifier_id	= OUI_TEAC,
 		.version	= 0x800006,
-		.driver_data = (kernel_ulong_t)snd_dice_detect_teac_formats,
+		.driver_data_ptr = snd_dice_detect_teac_formats,
 	},
 	{ }
 };
diff --git a/sound/firewire/fireface/ff.c b/sound/firewire/fireface/ff.c
index 5d2c4fbf4434..13472822d2be 100644
--- a/sound/firewire/fireface/ff.c
+++ b/sound/firewire/fireface/ff.c
@@ -70,7 +70,7 @@ static int snd_ff_probe(struct fw_unit *unit, const struct ieee1394_device_id *e
 	init_waitqueue_head(&ff->hwdep_wait);
 
 	ff->unit_version = entry->version;
-	ff->spec = (const struct snd_ff_spec *)entry->driver_data;
+	ff->spec = entry->driver_data_ptr;
 
 	err = snd_ff_transaction_register(ff);
 	if (err < 0)
@@ -186,7 +186,7 @@ static const struct ieee1394_device_id snd_ff_id_table[] = {
 		.specifier_id	= OUI_RME,
 		.version	= SND_FF_UNIT_VERSION_FF800,
 		.model_id	= 0x101800,
-		.driver_data	= (kernel_ulong_t)&spec_ff800,
+		.driver_data_ptr = &spec_ff800,
 	},
 	/* Fireface 400 */
 	{
@@ -198,7 +198,7 @@ static const struct ieee1394_device_id snd_ff_id_table[] = {
 		.specifier_id	= OUI_RME,
 		.version	= SND_FF_UNIT_VERSION_FF400,
 		.model_id	= 0x101800,
-		.driver_data	= (kernel_ulong_t)&spec_ff400,
+		.driver_data_ptr = &spec_ff400,
 	},
 	// Fireface UFX.
 	{
@@ -210,7 +210,7 @@ static const struct ieee1394_device_id snd_ff_id_table[] = {
 		.specifier_id	= OUI_RME,
 		.version	= SND_FF_UNIT_VERSION_UFX,
 		.model_id	= 0x101800,
-		.driver_data	= (kernel_ulong_t)&spec_ufx_802,
+		.driver_data_ptr = &spec_ufx_802,
 	},
 	// Fireface UCX.
 	{
@@ -222,7 +222,7 @@ static const struct ieee1394_device_id snd_ff_id_table[] = {
 		.specifier_id	= OUI_RME,
 		.version	= SND_FF_UNIT_VERSION_UCX,
 		.model_id	= 0x101800,
-		.driver_data	= (kernel_ulong_t)&spec_ucx,
+		.driver_data_ptr = &spec_ucx,
 	},
 	// Fireface 802.
 	{
@@ -234,7 +234,7 @@ static const struct ieee1394_device_id snd_ff_id_table[] = {
 		.specifier_id	= OUI_RME,
 		.version	= SND_FF_UNIT_VERSION_802,
 		.model_id	= 0x101800,
-		.driver_data	= (kernel_ulong_t)&spec_ufx_802,
+		.driver_data_ptr = &spec_ufx_802,
 	},
 	{}
 };
diff --git a/sound/firewire/motu/motu.c b/sound/firewire/motu/motu.c
index fd2a9dddbfa6..1fec6c8cdf6c 100644
--- a/sound/firewire/motu/motu.c
+++ b/sound/firewire/motu/motu.c
@@ -78,7 +78,7 @@ static int motu_probe(struct fw_unit *unit, const struct ieee1394_device_id *ent
 	dev_set_drvdata(&unit->device, motu);
 	motu->card = card;
 
-	motu->spec = (const struct snd_motu_spec *)entry->driver_data;
+	motu->spec = entry->driver_data_ptr;
 	mutex_init(&motu->mutex);
 	spin_lock_init(&motu->lock);
 	init_waitqueue_head(&motu->hwdep_wait);
@@ -148,7 +148,7 @@ static void motu_bus_update(struct fw_unit *unit)
 	snd_motu_transaction_reregister(motu);
 }
 
-#define SND_MOTU_DEV_ENTRY(model, data)			\
+#define SND_MOTU_DEV_ENTRY(model, data_ptr)		\
 {							\
 	.match_flags	= IEEE1394_MATCH_VENDOR_ID |	\
 			  IEEE1394_MATCH_SPECIFIER_ID |	\
@@ -156,7 +156,7 @@ static void motu_bus_update(struct fw_unit *unit)
 	.vendor_id	= OUI_MOTU,			\
 	.specifier_id	= OUI_MOTU,			\
 	.version	= model,			\
-	.driver_data	= (kernel_ulong_t)data,		\
+	.driver_data_ptr = data_ptr,			\
 }
 
 static const struct ieee1394_device_id motu_id_table[] = {
diff --git a/sound/firewire/oxfw/oxfw.c b/sound/firewire/oxfw/oxfw.c
index 5039bd79b18e..38a3c3b150df 100644
--- a/sound/firewire/oxfw/oxfw.c
+++ b/sound/firewire/oxfw/oxfw.c
@@ -95,7 +95,7 @@ static int name_card(struct snd_oxfw *oxfw, const struct ieee1394_device_id *ent
 
 	/* to apply card definitions */
 	if (entry->vendor_id == VENDOR_GRIFFIN || entry->vendor_id == VENDOR_LACIE) {
-		info = (const struct compat_info *)entry->driver_data;
+		info = entry->driver_data_ptr;
 		d = info->driver_name;
 		v = info->vendor_name;
 		m = info->model_name;
@@ -321,7 +321,7 @@ static const struct compat_info lacie_speakers = {
 	.model_id     = model, \
 	.specifier_id = SPECIFIER_1394TA, \
 	.version      = VERSION_AVC, \
-	.driver_data  = (kernel_ulong_t)data, \
+	.driver_data_ptr = data, \
 }
 
 static const struct ieee1394_device_id oxfw_id_table[] = {
-- 
2.47.3

Re: [PATCH v1 2/2] ALSA: firewire: Make use of ieee1394's .driver_data_ptr

[Andy Shevchenko]

On Sun, Apr 19, 2026 at 08:42:14AM +0200, Uwe Kleine-König (The Capable Hub) wrote:
> Recently struct ieee1394_device_id gained a new member to store a pointer
> to driver data. Make use of that to get rid of a bunch of casts.

...

> -	if (!entry->driver_data)
> +	if (!entry->driver_data_ptr)
>  		detect_formats = snd_dice_stream_detect_current_formats;
>  	else
> -		detect_formats = (snd_dice_detect_formats_t)entry->driver_data;
> +		detect_formats = entry->driver_data_ptr;

While at it, I would negate the conditional:

	if (entry->driver_data_ptr)
		detect_formats = entry->driver_data_ptr;
	else
		detect_formats = snd_dice_stream_detect_current_formats;

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 0/2] firewire: Simplify storing pointers in device id struct

[Takashi Sakamoto]

Hi,

Thanks for the patches. As far as I can see, they can be applied neither
any compilation failures and running regressions.

We are in the middle of merge window for v7.2. I had not planned to send any
changes to upstream for firewire subsystem, but there is still some time
before it closes. If the sound subsystem maintainer does not mind, I
would like to proceed.

Just out of curiosity, what does the CHERI extension adopted to RISC-V
architecture require in terms of kernel programming? Is taking extra
care when storing pointer values in long-type variables sufficient in
driver code?


Thanks

Takashi Sakamoto

On Sun, Apr 19, 2026 at 08:42:12AM +0200, Uwe Kleine-König (The Capable Hub) wrote:
> Hello,
> 
> <linux/mod_devicetable.h> contains several device_id structs for various
> device types.
> 
> Most of them have one of:
> 
>  - kernel_ulong_t driver_data (sometimes called "driver_info", sometimes 
>    the type is plain unsigned long)
>  - const void *data (sometimes called "driver_data" or "context", sometimes not const)
> 
> A considerable amount of drivers for the first category uses the
> unsigned long variable to store a pointer. This involves casting both
> for assignment and usage.
> 
> An additional complication exists for the CHERI hardware extension
> where sizeof(void *) > sizeof(unsigned long). So with that an unsigned
> long variable cannot be used to store a pointer.
> 
> To address both issues this series replaces the unsigned long variable
> by an anonymous union containing both an unsigned long and a pointer.
> 
> For all non-CHERI architectures this isn't an ABI change because all
> have sizeof(void *) == sizeof(unsigned long).
> 
> The first patch changes the definition of struct ieee1394_device_id. The
> second drops some casts in sound drivers. (There are no other firewire
> drivers that could benefit.) I adapted all sound drivers in a single
> patch, tell me if I should split per driver.
> 
> For merging I suggest to take the whole series via the ALSA tree in the
> next merge window, as there are no modified files that are specific to
> firewire only and the second patch depends on the first.
> 
> Best regards
> Uwe
> 
> Uwe Kleine-König (The Capable Hub) (2):
>   firewire: Simplify storing pointers in device id struct
>   ALSA: firewire: Make use of ieee1394's .driver_data_ptr
> 
>  include/linux/mod_devicetable.h |  5 ++++-
>  sound/firewire/dice/dice.c      | 34 ++++++++++++++++-----------------
>  sound/firewire/fireface/ff.c    | 12 ++++++------
>  sound/firewire/motu/motu.c      |  6 +++---
>  sound/firewire/oxfw/oxfw.c      |  4 ++--
>  5 files changed, 32 insertions(+), 29 deletions(-)
> 
> 
> base-commit: 028ef9c96e96197026887c0f092424679298aae8
> -- 
> 2.47.3
> 

Re: [PATCH v1 0/2] firewire: Simplify storing pointers in device id struct

[Christian A. Ehrhardt]

Hi,

On Mon, Apr 20, 2026 at 06:08:16PM +0900, Takashi Sakamoto wrote:
> Just out of curiosity, what does the CHERI extension adopted to RISC-V
> architecture require in terms of kernel programming? Is taking extra
> care when storing pointer values in long-type variables sufficient in
> driver code?

That is a significant part but there is more to it (entry code,
register size changes, the UABI should better be CHERI aware, ...).

But the issue that a pointer does not fit into an unsigned long
is an issue that pops up all over the kernel while most other
changes are more localized.

There is a working linux kernel in the CHERI alliance github here:
   https://github.com/CHERI-Alliance/linux/tree/codasip-cheri-riscv-6.18
That definitely needs more cleanup but it does work.
Previous work from ARM for the morello project (another CHERI
enabled platform) is available here:
   https://git.morello-project.org/morello/kernel/linux
These should give a rough idea of what is required.

Fixing unsigned long vs. uintptr_t issues helps a lot with this
because it reduces the diff. But it is also a general cleanup.

Best regards,
Christian

Re: [PATCH v1 0/2] firewire: Simplify storing pointers in device id struct

[Takashi Sakamoto]

Hi,

On Mon, Apr 20, 2026 at 07:39:32PM +0200, Christian A. Ehrhardt wrote:
> 
> Hi,
> 
> On Mon, Apr 20, 2026 at 06:08:16PM +0900, Takashi Sakamoto wrote:
> > Just out of curiosity, what does the CHERI extension adopted to RISC-V
> > architecture require in terms of kernel programming? Is taking extra
> > care when storing pointer values in long-type variables sufficient in
> > driver code?
> 
> That is a significant part but there is more to it (entry code,
> register size changes, the UABI should better be CHERI aware, ...).
> 
> But the issue that a pointer does not fit into an unsigned long
> is an issue that pops up all over the kernel while most other
> changes are more localized.
> 
> There is a working linux kernel in the CHERI alliance github here:
>    https://github.com/CHERI-Alliance/linux/tree/codasip-cheri-riscv-6.18
> That definitely needs more cleanup but it does work.
> Previous work from ARM for the morello project (another CHERI
> enabled platform) is available here:
>    https://git.morello-project.org/morello/kernel/linux
> These should give a rough idea of what is required.
> 
> Fixing unsigned long vs. uintptr_t issues helps a lot with this
> because it reduces the diff. But it is also a general cleanup.

Thsnks for the references. It looks like there is not much to consider
outside of mm subsystem. But I have some concerns if supporting
ARM/RISC-V adoptation of CHERI extension in Linux FireWire subsystem.

Any structures in UAPI header of this subsystem are defined with
an assumption that the size of pointer in the existing System V
architectures is up to 64 bits at most. We can see many usage of
'__u64' type member for pointers (e.g. 'rom' in fw_cdev_get_info
structure). I imagine to need defining specific structures for this kind
of 'fat' pointer. (The same assumption lays on compat ioctl.)

As another concern is that the padding in structure. As long as I know,
any 64 bit architecture for System V ABI has 8 bit alignment rule, and
any structure in UAPI header of this subsystem are carefully defined not
to have different sizes between x86/32bit/64bit architectures, except for
'fw_cdev_event_response' structure (see 'drivers/firewire/uapi-test.c').
As a quick glance, the size of pointer in ARM CHERI extension seems to be
129 bit. In this case, what size of alignment rule is applied? Is there
7 bit padding after pointer member in any aggregates?


Thanks

Takashi Sakamoto

Re: [PATCH v1 0/2] firewire: Simplify storing pointers in device id struct

[Uwe Kleine-König (The Capable Hub)]

[-- Attachment #1: Type: text/plain, Size: 4300 bytes --]

Hello Takashi,

On Tue, Apr 21, 2026 at 09:53:57PM +0900, Takashi Sakamoto wrote:
> On Mon, Apr 20, 2026 at 07:39:32PM +0200, Christian A. Ehrhardt wrote:
> > On Mon, Apr 20, 2026 at 06:08:16PM +0900, Takashi Sakamoto wrote:
> > > Just out of curiosity, what does the CHERI extension adopted to RISC-V
> > > architecture require in terms of kernel programming? Is taking extra
> > > care when storing pointer values in long-type variables sufficient in
> > > driver code?
> > 
> > That is a significant part but there is more to it (entry code,
> > register size changes, the UABI should better be CHERI aware, ...).
> > 
> > But the issue that a pointer does not fit into an unsigned long
> > is an issue that pops up all over the kernel while most other
> > changes are more localized.
> > 
> > There is a working linux kernel in the CHERI alliance github here:
> >    https://github.com/CHERI-Alliance/linux/tree/codasip-cheri-riscv-6.18
> > That definitely needs more cleanup but it does work.
> > Previous work from ARM for the morello project (another CHERI
> > enabled platform) is available here:
> >    https://git.morello-project.org/morello/kernel/linux
> > These should give a rough idea of what is required.
> > 
> > Fixing unsigned long vs. uintptr_t issues helps a lot with this
> > because it reduces the diff. But it is also a general cleanup.
> 
> Thsnks for the references. It looks like there is not much to consider
> outside of mm subsystem. But I have some concerns if supporting
> ARM/RISC-V adoptation of CHERI extension in Linux FireWire subsystem.
> 
> Any structures in UAPI header of this subsystem are defined with
> an assumption that the size of pointer in the existing System V
> architectures is up to 64 bits at most. We can see many usage of
> '__u64' type member for pointers (e.g. 'rom' in fw_cdev_get_info
> structure). I imagine to need defining specific structures for this kind
> of 'fat' pointer. (The same assumption lays on compat ioctl.)

The Standard C answer to that is: The assumption that you can fit a
pointer in an unsigned long or u64 is not generally justified. This is
"only" given for all current Linux archtectures. And if you want an
integer type to store a pointer, use uintptr_t. (And my position is that
you better try to keep pointers in pointer variables, though there are
some situations where you don't come around the conversion to an integer
type. That's why I introduced the union instead of converting to
uintptr_t.)

> As another concern is that the padding in structure. As long as I know,
> any 64 bit architecture for System V ABI has 8 bit alignment rule, and
> any structure in UAPI header of this subsystem are carefully defined not
> to have different sizes between x86/32bit/64bit architectures, except for
> 'fw_cdev_event_response' structure (see 'drivers/firewire/uapi-test.c').
> As a quick glance, the size of pointer in ARM CHERI extension seems to be
> 129 bit. In this case, what size of alignment rule is applied? Is there
> 7 bit padding after pointer member in any aggregates?

On CHERI pointers must be 128 bit aligned. Unaligned pointers yield a
hardware exception on usage. But that the ABI for CHERI is different
due to different sizes of some types is only a concern for CHERI-enabled
platforms. On non-CHERI everything stays as it is as
sizeof(union { kernel_ulong_t; void *; }) == sizeof(kernel_ulong_t) ==
sizeof(void *) == sizeof(uintptr_t). And there is no state that needs to
be maintained on CHERI-enabled systems, as they are new.

A pointer is 128 bit plus a validity flag that is stored elsewhere
(which is necessary to not give processes a means to craft valid
pointers). So for the purpose of a user space program and also for most
parts of the kernel, a pointer can be considered to be 128 bit as the
additional bit is (mostly) managed by hardware.

I see the challenge to introduce CHERI in mainline Linux somewhat
similar to the time when the first 64bit architectures came up and the
kernel still contained various assumptions about pointers and longs
being 32bit. Assuming CHERI keeps traction things will be addressed
over time and for the purpose of this series shouldn't be a concern now.

Best regards
Uwe

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v1 0/2] firewire: Simplify storing pointers in device id struct

[Andy Shevchenko]

On Tue, Apr 21, 2026 at 04:07:42PM +0200, Uwe Kleine-König (The Capable Hub) wrote:
> On Tue, Apr 21, 2026 at 09:53:57PM +0900, Takashi Sakamoto wrote:
> > On Mon, Apr 20, 2026 at 07:39:32PM +0200, Christian A. Ehrhardt wrote:
> > > On Mon, Apr 20, 2026 at 06:08:16PM +0900, Takashi Sakamoto wrote:

...

> > Thanks for the references. It looks like there is not much to consider
> > outside of mm subsystem. But I have some concerns if supporting
> > ARM/RISC-V adoptation of CHERI extension in Linux FireWire subsystem.
> > 
> > Any structures in UAPI header of this subsystem are defined with
> > an assumption that the size of pointer in the existing System V
> > architectures is up to 64 bits at most. We can see many usage of
> > '__u64' type member for pointers (e.g. 'rom' in fw_cdev_get_info
> > structure). I imagine to need defining specific structures for this kind
> > of 'fat' pointer. (The same assumption lays on compat ioctl.)
> 
> The Standard C answer to that is: The assumption that you can fit a
> pointer in an unsigned long or u64 is not generally justified. This is
> "only" given for all current Linux archtectures. And if you want an
> integer type to store a pointer, use uintptr_t.

No, please don't. Linus was clear about this. Use `unsigned long` in that case.

> (And my position is that
> you better try to keep pointers in pointer variables, though there are
> some situations where you don't come around the conversion to an integer
> type. That's why I introduced the union instead of converting to
> uintptr_t.)

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 0/2] firewire: Simplify storing pointers in device id struct

[Uwe Kleine-König (The Capable Hub)]

[-- Attachment #1: Type: text/plain, Size: 2020 bytes --]

Hello Andy,

On Wed, Apr 22, 2026 at 10:19:21AM +0300, Andy Shevchenko wrote:
> On Tue, Apr 21, 2026 at 04:07:42PM +0200, Uwe Kleine-König (The Capable Hub) wrote:
> > On Tue, Apr 21, 2026 at 09:53:57PM +0900, Takashi Sakamoto wrote:
> > > On Mon, Apr 20, 2026 at 07:39:32PM +0200, Christian A. Ehrhardt wrote:
> > > > On Mon, Apr 20, 2026 at 06:08:16PM +0900, Takashi Sakamoto wrote:
> 
> ...
> 
> > > Thanks for the references. It looks like there is not much to consider
> > > outside of mm subsystem. But I have some concerns if supporting
> > > ARM/RISC-V adoptation of CHERI extension in Linux FireWire subsystem.
> > > 
> > > Any structures in UAPI header of this subsystem are defined with
> > > an assumption that the size of pointer in the existing System V
> > > architectures is up to 64 bits at most. We can see many usage of
> > > '__u64' type member for pointers (e.g. 'rom' in fw_cdev_get_info
> > > structure). I imagine to need defining specific structures for this kind
> > > of 'fat' pointer. (The same assumption lays on compat ioctl.)
> > 
> > The Standard C answer to that is: The assumption that you can fit a
> > pointer in an unsigned long or u64 is not generally justified. This is
> > "only" given for all current Linux archtectures. And if you want an
> > integer type to store a pointer, use uintptr_t.
> 
> No, please don't. Linus was clear about this. Use `unsigned long` in that case.

On CHERI we have sizeof(unsigned long) = 4 and sizeof(void *) = 8, so
what Linus wants doesn't work.

But let me note that we don't have to resolve that question for this
patch set. We're still far away from working on mainlining the core
changes for CHERI that most probaly will require some casting between
pointers and integer types. My plan for now is to work on changes (like
the one here) where we have both, a benefit for mainline and as a side
effect a simplification for CHERI enablement. So I wouldn't mind to
delay the discussion.

Best regards
Uwe

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v1 0/2] firewire: Simplify storing pointers in device id struct

[Uwe Kleine-König (The Capable Hub)]

[-- Attachment #1: Type: text/plain, Size: 1777 bytes --]

Hello again,

On Wed, Apr 22, 2026 at 10:30:24AM +0200, Uwe Kleine-König (The Capable Hub) wrote:
> On Wed, Apr 22, 2026 at 10:19:21AM +0300, Andy Shevchenko wrote:
> > On Tue, Apr 21, 2026 at 04:07:42PM +0200, Uwe Kleine-König (The Capable Hub) wrote:
> > > On Tue, Apr 21, 2026 at 09:53:57PM +0900, Takashi Sakamoto wrote:
> > > > On Mon, Apr 20, 2026 at 07:39:32PM +0200, Christian A. Ehrhardt wrote:
> > > > > On Mon, Apr 20, 2026 at 06:08:16PM +0900, Takashi Sakamoto wrote:
> > 
> > ...
> > 
> > > > Thanks for the references. It looks like there is not much to consider
> > > > outside of mm subsystem. But I have some concerns if supporting
> > > > ARM/RISC-V adoptation of CHERI extension in Linux FireWire subsystem.
> > > > 
> > > > Any structures in UAPI header of this subsystem are defined with
> > > > an assumption that the size of pointer in the existing System V
> > > > architectures is up to 64 bits at most. We can see many usage of
> > > > '__u64' type member for pointers (e.g. 'rom' in fw_cdev_get_info
> > > > structure). I imagine to need defining specific structures for this kind
> > > > of 'fat' pointer. (The same assumption lays on compat ioctl.)
> > > 
> > > The Standard C answer to that is: The assumption that you can fit a
> > > pointer in an unsigned long or u64 is not generally justified. This is
> > > "only" given for all current Linux archtectures. And if you want an
> > > integer type to store a pointer, use uintptr_t.
> > 
> > No, please don't. Linus was clear about this. Use `unsigned long` in that case.
> 
> On CHERI we have sizeof(unsigned long) = 4 and sizeof(void *) = 8, so
> what Linus wants doesn't work.

Correction, we have sizeof(unsigned long) = 8 and sizeof(void *) = 16 or
course.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 484 bytes --]

Re: [PATCH v1 0/2] firewire: Simplify storing pointers in device id struct

[Andy Shevchenko]

On Wed, Apr 22, 2026 at 10:40:09AM +0200, Uwe Kleine-König (The Capable Hub) wrote:
> On Wed, Apr 22, 2026 at 10:30:24AM +0200, Uwe Kleine-König (The Capable Hub) wrote:
> > On Wed, Apr 22, 2026 at 10:19:21AM +0300, Andy Shevchenko wrote:
> > > On Tue, Apr 21, 2026 at 04:07:42PM +0200, Uwe Kleine-König (The Capable Hub) wrote:
> > > > On Tue, Apr 21, 2026 at 09:53:57PM +0900, Takashi Sakamoto wrote:
> > > > > On Mon, Apr 20, 2026 at 07:39:32PM +0200, Christian A. Ehrhardt wrote:
> > > > > > On Mon, Apr 20, 2026 at 06:08:16PM +0900, Takashi Sakamoto wrote:

...

> > > > > Thanks for the references. It looks like there is not much to consider
> > > > > outside of mm subsystem. But I have some concerns if supporting
> > > > > ARM/RISC-V adoptation of CHERI extension in Linux FireWire subsystem.
> > > > > 
> > > > > Any structures in UAPI header of this subsystem are defined with
> > > > > an assumption that the size of pointer in the existing System V
> > > > > architectures is up to 64 bits at most. We can see many usage of
> > > > > '__u64' type member for pointers (e.g. 'rom' in fw_cdev_get_info
> > > > > structure). I imagine to need defining specific structures for this kind
> > > > > of 'fat' pointer. (The same assumption lays on compat ioctl.)
> > > > 
> > > > The Standard C answer to that is: The assumption that you can fit a
> > > > pointer in an unsigned long or u64 is not generally justified. This is
> > > > "only" given for all current Linux archtectures. And if you want an
> > > > integer type to store a pointer, use uintptr_t.
> > > 
> > > No, please don't. Linus was clear about this. Use `unsigned long` in that case.
> > 
> > On CHERI we have sizeof(unsigned long) = 4 and sizeof(void *) = 8, so
> > what Linus wants doesn't work.
> 
> Correction, we have sizeof(unsigned long) = 8 and sizeof(void *) = 16 or
> course.

CHERI is not specified for 32-bit platforms?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 0/2] firewire: Simplify storing pointers in device id struct

[Uwe Kleine-König (The Capable Hub)]

[-- Attachment #1: Type: text/plain, Size: 2748 bytes --]

On Wed, Apr 22, 2026 at 12:40:43PM +0300, Andy Shevchenko wrote:
> On Wed, Apr 22, 2026 at 10:40:09AM +0200, Uwe Kleine-König (The Capable Hub) wrote:
> > On Wed, Apr 22, 2026 at 10:30:24AM +0200, Uwe Kleine-König (The Capable Hub) wrote:
> > > On Wed, Apr 22, 2026 at 10:19:21AM +0300, Andy Shevchenko wrote:
> > > > On Tue, Apr 21, 2026 at 04:07:42PM +0200, Uwe Kleine-König (The Capable Hub) wrote:
> > > > > On Tue, Apr 21, 2026 at 09:53:57PM +0900, Takashi Sakamoto wrote:
> > > > > > On Mon, Apr 20, 2026 at 07:39:32PM +0200, Christian A. Ehrhardt wrote:
> > > > > > > On Mon, Apr 20, 2026 at 06:08:16PM +0900, Takashi Sakamoto wrote:
> 
> ...
> 
> > > > > > Thanks for the references. It looks like there is not much to consider
> > > > > > outside of mm subsystem. But I have some concerns if supporting
> > > > > > ARM/RISC-V adoptation of CHERI extension in Linux FireWire subsystem.
> > > > > > 
> > > > > > Any structures in UAPI header of this subsystem are defined with
> > > > > > an assumption that the size of pointer in the existing System V
> > > > > > architectures is up to 64 bits at most. We can see many usage of
> > > > > > '__u64' type member for pointers (e.g. 'rom' in fw_cdev_get_info
> > > > > > structure). I imagine to need defining specific structures for this kind
> > > > > > of 'fat' pointer. (The same assumption lays on compat ioctl.)
> > > > > 
> > > > > The Standard C answer to that is: The assumption that you can fit a
> > > > > pointer in an unsigned long or u64 is not generally justified. This is
> > > > > "only" given for all current Linux archtectures. And if you want an
> > > > > integer type to store a pointer, use uintptr_t.
> > > > 
> > > > No, please don't. Linus was clear about this. Use `unsigned long` in that case.
> > > 
> > > On CHERI we have sizeof(unsigned long) = 4 and sizeof(void *) = 8, so
> > > what Linus wants doesn't work.
> > 
> > Correction, we have sizeof(unsigned long) = 8 and sizeof(void *) = 16 or
> > course.
> 
> CHERI is not specified for 32-bit platforms?

(Attention, half-knowledge alert; take the stuff I'm saying with a grain
of salt.)

There are 32bit riscv CHERI machines (even real hardware[1], while for
64 bit riscv there is currently only qemu and an expensive FPGA by
Codasip), but a difficulty there is that there is >1 incompatible
variant about the semantics of the additional pointer bits which makes
working on that a bit more difficult and ugly. And I'm not aware of an
effort to make Linux work on those. Also I believe that all users who
want CHERI and Linux will stick to 64bit.

Best regards
Uwe

[1] https://lowrisc.org/news/unveiling-sonata-affordable-cheri-hardware-for-embedded-systems/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v1 0/2] firewire: Simplify storing pointers in device id struct

[Christian A. Ehrhardt]

Hi,

this is getting off topic but....

On Tue, Apr 21, 2026 at 09:53:57PM +0900, Takashi Sakamoto wrote:
> > Fixing unsigned long vs. uintptr_t issues helps a lot with this
> > because it reduces the diff. But it is also a general cleanup.
> 
> Thsnks for the references. It looks like there is not much to consider
> outside of mm subsystem. But I have some concerns if supporting
> ARM/RISC-V adoptation of CHERI extension in Linux FireWire subsystem.
> 
> Any structures in UAPI header of this subsystem are defined with
> an assumption that the size of pointer in the existing System V
> architectures is up to 64 bits at most. We can see many usage of
> '__u64' type member for pointers (e.g. 'rom' in fw_cdev_get_info
> structure). I imagine to need defining specific structures for this kind
> of 'fat' pointer. (The same assumption lays on compat ioctl.)

As far as in-memory syscall arguments are concerned a full support
for CHERI will essentially require a new UABI and also a new variant
of compat support to run standard non-CHERI aware 64-bit binaries.
This is the main reason for the size of the patchset.

> As another concern is that the padding in structure. As long as I know,
> any 64 bit architecture for System V ABI has 8 bit alignment rule, and
> any structure in UAPI header of this subsystem are carefully defined not
> to have different sizes between x86/32bit/64bit architectures, except for
> 'fw_cdev_event_response' structure (see 'drivers/firewire/uapi-test.c').
> As a quick glance, the size of pointer in ARM CHERI extension seems to be
> 129 bit. In this case, what size of alignment rule is applied? Is there
> 7 bit padding after pointer member in any aggregates?

The in-memory representation of the fat pointer is 128-bit on a 64-bit
system. The 129-th bit (called the "tag bit") is not directly accessible
but managed out-of-band by the memory controller or some similar entity.
It is set on valid pointers only and is implicitly cleared when memory
is accessed with non-pointer instructions.

Best regards,
Christian

Re: [PATCH v1 0/2] firewire: Simplify storing pointers in device id struct

[Takashi Sakamoto]

Hi,

It is unclear who is responsible for maintaining the 'ieee1394_device_id'
structure in include/linux/mod_devicetable.h, but if it falls under my
responsibility (which seems likely), I would prefer to postpone applying
these patches, or at least exclude them from this merge window.

After reading the discussions around the UAPI, I am not fully convinced
that your patches appear to provide clear benefits to existing
IEEE 1394 bus users or their software. From my perspective, the motivation
appears to be primarily related to the CHERI extension work.

As you know, this subsystem is quite marginal in the Linux kernel
codebase. Given that, it might be worth considering whether this
subsystem should be excluded from the build target when capability
pointers are enabled (e.g. via Kconfig, if available), since it does not
appear to work outside the ILP32 or LP64 data models. It may be worth
carefully considering where effort is best spent. I can understand the
merits of CHERI extensions, but changes related to this subsystem would
likely be acceptable only after the kernel core functions have been
updated.

That said, this is just my current view. I would welcome any feedback or
objections. Besides, it is still one of my tasks to figure out how to
adapt the UAPI structures and the firewire core implementations to
non-ILP32/LP64 data models.


Thanks

Takashi Sakamoto

On Sun, Apr 19, 2026 at 08:42:12AM +0200, Uwe Kleine-König (The Capable Hub) wrote:
> Hello,
> 
> <linux/mod_devicetable.h> contains several device_id structs for various
> device types.
> 
> Most of them have one of:
> 
>  - kernel_ulong_t driver_data (sometimes called "driver_info", sometimes 
>    the type is plain unsigned long)
>  - const void *data (sometimes called "driver_data" or "context", sometimes not const)
> 
> A considerable amount of drivers for the first category uses the
> unsigned long variable to store a pointer. This involves casting both
> for assignment and usage.
> 
> An additional complication exists for the CHERI hardware extension
> where sizeof(void *) > sizeof(unsigned long). So with that an unsigned
> long variable cannot be used to store a pointer.
> 
> To address both issues this series replaces the unsigned long variable
> by an anonymous union containing both an unsigned long and a pointer.
> 
> For all non-CHERI architectures this isn't an ABI change because all
> have sizeof(void *) == sizeof(unsigned long).
> 
> The first patch changes the definition of struct ieee1394_device_id. The
> second drops some casts in sound drivers. (There are no other firewire
> drivers that could benefit.) I adapted all sound drivers in a single
> patch, tell me if I should split per driver.
> 
> For merging I suggest to take the whole series via the ALSA tree in the
> next merge window, as there are no modified files that are specific to
> firewire only and the second patch depends on the first.
> 
> Best regards
> Uwe
> 
> Uwe Kleine-König (The Capable Hub) (2):
>   firewire: Simplify storing pointers in device id struct
>   ALSA: firewire: Make use of ieee1394's .driver_data_ptr
> 
>  include/linux/mod_devicetable.h |  5 ++++-
>  sound/firewire/dice/dice.c      | 34 ++++++++++++++++-----------------
>  sound/firewire/fireface/ff.c    | 12 ++++++------
>  sound/firewire/motu/motu.c      |  6 +++---
>  sound/firewire/oxfw/oxfw.c      |  4 ++--
>  5 files changed, 32 insertions(+), 29 deletions(-)
> 
> 
> base-commit: 028ef9c96e96197026887c0f092424679298aae8
> -- 
> 2.47.3
> 

Re: [PATCH v1 0/2] firewire: Simplify storing pointers in device id struct

[Uwe Kleine-König (The Capable Hub)]

[-- Attachment #1: Type: text/plain, Size: 4100 bytes --]

Hello Takashi,

On Thu, Apr 23, 2026 at 11:19:59PM +0900, Takashi Sakamoto wrote:
> It is unclear who is responsible for maintaining the 'ieee1394_device_id'
> structure in include/linux/mod_devicetable.h, but if it falls under my
> responsibility (which seems likely),

It matches my understanding that it's indeed you who is responsible for
ieee1394_device_id.

> I would prefer to postpone applying these patches, or at least exclude
> them from this merge window.

I don't expect them to go in for v7.1-rc1. My idea was to send this kind
of patch during the merge window to allow to get it into next soon after
the merge window closed to allow some cooking in next.
 
> After reading the discussions around the UAPI, I am not fully convinced
> that your patches appear to provide clear benefits to existing
> IEEE 1394 bus users or their software. From my perspective, the motivation
> appears to be primarily related to the CHERI extension work.

Yes and no. My motivation to work on these is triggered by CHERI indeed.
But I already worked on this kind of update even before I knew about
CHERI to get rid of the casts. (Back then it was about i2c, see
https://lore.kernel.org/all/20240426213832.915485-2-u.kleine-koenig@pengutronix.de/.)

For me this series is a sweet spot, because it allows to reduce the
CHERI patch stack but at the same time reduces the amount of (IMHO ugly)
casts involved in handling pointers in .driver_data. With my mainline
maintainer hat on, I consider the latter alone a good enough reason to
apply patches like this. One issue it addresses en passant is that a
part of the casts back to a pointer drop the const attribute, see the
output of

	git grep '\*).*driver_data' | grep -v '\<const\>'

. I didn't check for false positives in this list, but I guess there are
not many. This doesn't affect firewire though, there the casts are all
fine.

As my patch series doesn't introduce changes to the compiled drivers,
the effect on bus users or their software is admittedly quite limited of
course :-), but I'd hope that you see a benefit in the 2nd patch even if
CHERI isn't (and probably shouldn't be) a motivation for you.

The reason I mentioned CHERI was mainly to be transparent about my
motivation, but this triggered more discussion than I liked, distracting
from the advantages of the changes for non-CHERI archs.

> As you know, this subsystem is quite marginal in the Linux kernel
> codebase. Given that, it might be worth considering whether this
> subsystem should be excluded from the build target when capability
> pointers are enabled (e.g. via Kconfig, if available), since it does not
> appear to work outside the ILP32 or LP64 data models. It may be worth
> carefully considering where effort is best spent. I can understand the
> merits of CHERI extensions, but changes related to this subsystem would
> likely be acceptable only after the kernel core functions have been
> updated.

I wasn't aware about this limitation. struct ieee1394_device_id just
happend to be near the top of include/linux/mod_devicetable.h. (There is
only pci_device_id before, but I wanted to start with a smaller part of
this quest.)
 
> That said, this is just my current view. I would welcome any feedback or
> objections. Besides, it is still one of my tasks to figure out how to
> adapt the UAPI structures and the firewire core implementations to
> non-ILP32/LP64 data models.

With you talking much about UAPI I wonder if you're aware that for all
current Linux architectures my patch set doesn't introduce any changes
in the binary interface because all have sizeof(unsigned long) ==
sizeof(union { unsigned long; const void *; }). And the two adaptions
(my introduction of the union for driver_data + your UAPI structure
adaptions) should not restrict each other, right?

So I'd be glad if you forgot about CHERI and just judge the patchset for
the things it achieves for current mainline. Of course I hope you agree
it's a nice cleanup eventually and apply it.

Best regards
Uwe

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v1 0/2] firewire: Simplify storing pointers in device id struct

[Takashi Sakamoto]

Hi,

On Thu, Apr 23, 2026 at 06:53:12PM +0200, Uwe Kleine-König (The Capable Hub) wrote:
> Hello Takashi,
> 
> On Thu, Apr 23, 2026 at 11:19:59PM +0900, Takashi Sakamoto wrote:
> > It is unclear who is responsible for maintaining the 'ieee1394_device_id'
> > structure in include/linux/mod_devicetable.h, but if it falls under my
> > responsibility (which seems likely),
> 
> It matches my understanding that it's indeed you who is responsible for
> ieee1394_device_id.
> 
> > I would prefer to postpone applying these patches, or at least exclude
> > them from this merge window.
> 
> I don't expect them to go in for v7.1-rc1. My idea was to send this kind
> of patch during the merge window to allow to get it into next soon after
> the merge window closed to allow some cooking in next.
>  
> > After reading the discussions around the UAPI, I am not fully convinced
> > that your patches appear to provide clear benefits to existing
> > IEEE 1394 bus users or their software. From my perspective, the motivation
> > appears to be primarily related to the CHERI extension work.
> 
> Yes and no. My motivation to work on these is triggered by CHERI indeed.
> But I already worked on this kind of update even before I knew about
> CHERI to get rid of the casts. (Back then it was about i2c, see
> https://lore.kernel.org/all/20240426213832.915485-2-u.kleine-koenig@pengutronix.de/.)
> 
> For me this series is a sweet spot, because it allows to reduce the
> CHERI patch stack but at the same time reduces the amount of (IMHO ugly)
> casts involved in handling pointers in .driver_data. With my mainline
> maintainer hat on, I consider the latter alone a good enough reason to
> apply patches like this. One issue it addresses en passant is that a
> part of the casts back to a pointer drop the const attribute, see the
> output of
> 
> 	git grep '\*).*driver_data' | grep -v '\<const\>'
> 
> . I didn't check for false positives in this list, but I guess there are
> not many. This doesn't affect firewire though, there the casts are all
> fine.
> 
> As my patch series doesn't introduce changes to the compiled drivers,
> the effect on bus users or their software is admittedly quite limited of
> course :-), but I'd hope that you see a benefit in the 2nd patch even if
> CHERI isn't (and probably shouldn't be) a motivation for you.
> 
> The reason I mentioned CHERI was mainly to be transparent about my
> motivation, but this triggered more discussion than I liked, distracting
> from the advantages of the changes for non-CHERI archs.
> 
> > As you know, this subsystem is quite marginal in the Linux kernel
> > codebase. Given that, it might be worth considering whether this
> > subsystem should be excluded from the build target when capability
> > pointers are enabled (e.g. via Kconfig, if available), since it does not
> > appear to work outside the ILP32 or LP64 data models. It may be worth
> > carefully considering where effort is best spent. I can understand the
> > merits of CHERI extensions, but changes related to this subsystem would
> > likely be acceptable only after the kernel core functions have been
> > updated.
> 
> I wasn't aware about this limitation. struct ieee1394_device_id just
> happend to be near the top of include/linux/mod_devicetable.h. (There is
> only pci_device_id before, but I wanted to start with a smaller part of
> this quest.)
>  
> > That said, this is just my current view. I would welcome any feedback or
> > objections. Besides, it is still one of my tasks to figure out how to
> > adapt the UAPI structures and the firewire core implementations to
> > non-ILP32/LP64 data models.
> 
> With you talking much about UAPI I wonder if you're aware that for all
> current Linux architectures my patch set doesn't introduce any changes
> in the binary interface because all have sizeof(unsigned long) ==
> sizeof(union { unsigned long; const void *; }). And the two adaptions
> (my introduction of the union for driver_data + your UAPI structure
> adaptions) should not restrict each other, right?
> 
> So I'd be glad if you forgot about CHERI and just judge the patchset for
> the things it achieves for current mainline. Of course I hope you agree
> it's a nice cleanup eventually and apply it.

Setting aside CHERI-related considerations, what concrete benefits do
these patches provide to the current kernel? Are those benefits clearly
explained in the cover letter and the patch comments?

From the cover letter, I understand that:

> A considerable amount of drivers for the first category uses the
> unsigned long variable to store a pointer. This involves casting both
> for assignment and usage.

The conversion between 'unsigned long' and 'void *' are not inherently
invalid within ILP32/LP64 data models. As I see it, the main significant
effect of these patches is to add a 'const' qualifier to pointer values
(and improve type safety, presumably in your view).

If that is the case, why do the patch comments spend so much lines
describing the size of unsigned long and pointer types, and CHERI
extension? These points do not seem directly relevant to the practical
benefits described above.

It would be better to repost the patches with clearer and more focused
comments so that subsystem maintainers are more likely to apply them
without the background discussion about the CHERI extension.


Thanks

Takashi Sakamoto

Re: [PATCH v1 0/2] firewire: Simplify storing pointers in device id struct

[Uwe Kleine-König (The Capable Hub)]

[-- Attachment #1: Type: text/plain, Size: 2306 bytes --]

Hello Takashi,

On Mon, Apr 27, 2026 at 03:37:19PM +0900, Takashi Sakamoto wrote:
> On Thu, Apr 23, 2026 at 06:53:12PM +0200, Uwe Kleine-König (The Capable Hub) wrote:
> > A considerable amount of drivers for the first category uses the
> > unsigned long variable to store a pointer. This involves casting both
> > for assignment and usage.
> 
> The conversion between 'unsigned long' and 'void *' are not inherently
> invalid within ILP32/LP64 data models.

Agreed, it's not invalid. But casts are always a part in C code where
strange or even bad things can happen as the (weak) type-safety of C is
overridden. Arguably there is still casting involved as driver_data_ptr
is a void pointer, but this is near enough to the actually type of the
driver data to not require an explicit cast. In my eyes reducing the
number of casts is a maintenance improvement. That's a bit similar to
reducing the number of unsafe parts in Rust code. It reduces the amount
of code you have to keep an eye on when thinking about correctness of
the code and it makes drivers suspicious that use both .driver_data and
.driver_data_ptr (which is easy to grep for).

> As I see it, the main significant
> effect of these patches is to add a 'const' qualifier to pointer values
> (and improve type safety, presumably in your view).

For me the main benefit is that the explicit casts can go away. This is
what is implemented in the 2nd patch with changes like

-               detect_formats = (snd_dice_detect_formats_t)entry->driver_data;
+               detect_formats = entry->driver_data_ptr;

but yes, that the const is kept this way is also a nice upside. I'm
currently working on a similar patch set for pci_device_id and this
indeed shows a few missing `const`s.

> If that is the case, why do the patch comments spend so much lines
> describing the size of unsigned long and pointer types, and CHERI
> extension? These points do not seem directly relevant to the practical
> benefits described above.
> 
> It would be better to repost the patches with clearer and more focused
> comments so that subsystem maintainers are more likely to apply them
> without the background discussion about the CHERI extension.

I'll try that. Thanks for your feedback.

Best regards
Uwe

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]