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

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

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

Hello,

v1 of this series can be found at
https://lore.kernel.org/all/cover.1776579304.git.u.kleine-koenig@baylibre.com
.

The changes introduced here are the same as before, but the commit log
of the first patch is (hopefully) improved to better point out the
advantage of the approach for mainline. The second patch demonstrates
the explicit casts can be dropped after the first patch.

The patch series intends to not change the runtime behaviour, however
the 2nd patch introduces a few changes to the generated code. Looking at
these for an arm64 build they only affected register allocation (so
where x0 was used before it's x1 after the patch). I'm not proficient in
x86 assembly enough to understand the changes there, but I guess they
also don't affect the runtime behaviour.

My motivation for this patch set is to reduce the patch stack for Linux
CHERI support. This affects firewire because with CHERI you cannot store
a pointer in an unsigned long variable. But I hope these changes qualify
as cleanup worth to be applied even without considering CHERI.

For merging I suggest to take the whole series via the ALSA tree during
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: 254f49634ee16a731174d2ae34bc50bd5f45e731
-- 
2.47.3

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

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

Technically it is fine (on all current Linux architectures) to store a
pointer in an unsigned long variable. However this needs explicit
casting which is an easy source for type mismatches.

By replacing the plain unsigned long .driver_data in struct
ieee1394_device_id by an anonymous union, most of the casting can be
dropped. There is still some implicit casting involved (between a void *
and a driver specific pointer type), but that's better than the approach
to store a pointer in an unsigned long variable as this doesn't lose the
information that the data being pointed to is const.

This also helps the porting effort to let Linux support CHERI where an
unsigned long is unsuitable to hold a pointer.

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 23ff24080dfd..3b0c9a251a2e 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 v2 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 v2 2/2] ALSA: firewire: Make use of ieee1394's .driver_data_ptr

[Andy Shevchenko]

On Mon, May 11, 2026 at 12:45:03PM +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, it can be swapped for better readability

	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