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

[Takashi S. <o-t...@sa...>]

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
>