[PATCH v1 0/8] zorro: Improve handling of pointers in zorro_device_id::driver_data

[PATCH v1 0/8] zorro: Improve handling of pointers in zorro_device_id::driver_data

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

Hello,

this series is about improving the handling of pointers in struct
zorro_device_id's driver_data.

While it's ok on all current Linux platforms to store a pointer in an
unsigned long variable, it involves casting that loses type information.
This can be nicely seen in patch #7 where after profiting from patch #6
the compiler notices a missing const.

Preparing for that change, all zorro_device_ids are converted to use
named initializers, which is also a nice cleanup that could stand for
itself, as it improves readability for humans. (That is necessary
because an anonymous union can be initialized by name, but not using a
list initializer.)

My motivation for this series is the CHERI hardware extension. With that
pointers are bigger than longs and thus you cannot store pointers in
zorro_device_id::driver_data. So this series is also about getting
support for CHERI into the mainline, but I hope the clean up effects
mentioned above are justification enough to accept this series.

The dependencies in this series are as follows:

 - Patch #5 depends on #1, #2
 - Patches #7 and #8 depend on patch #6.

So if the ata maintainers agreed to merge their patch #1 via scsi, and
Geert agrees to patch #5 and that it's also merged via scsi, patches #1,
#2, #6 and #7 can go in without further coordination.

Patches #3, #4 and #5 are only about using the same initialization style
for all zorro_device_id and can go in without coordination.

Best regards
Uwe

Uwe Kleine-König (The Capable Hub) (8):
  ata: pata_budda: Use named initializer for zorro_device_id
  scsi: Use named initializer for zorro_device_id
  net: Use named initializer for zorro_device_id arrays
  i2c: icy: Use named initializer for zorro_device_id arrays
  video: fm2fb: Use named initializer for zorro_device_id array
  zorro: Simplify storing pointers in device id struct
  scsi: zorro7xx: Make use of struct zorro_device_id::driver_data_ptr
  video: cirrusfb: Make use of struct zorro_device_id::driver_data_ptr

 drivers/ata/pata_buddha.c             |  8 ++++----
 drivers/i2c/busses/i2c-icy.c          |  4 ++--
 drivers/net/ethernet/8390/hydra.c     |  4 ++--
 drivers/net/ethernet/8390/xsurf100.c  |  4 ++--
 drivers/net/ethernet/8390/zorro8390.c |  6 +++---
 drivers/net/ethernet/amd/a2065.c      |  8 ++++----
 drivers/net/ethernet/amd/ariadne.c    |  4 ++--
 drivers/scsi/a2091.c                  |  6 +++---
 drivers/scsi/gvp11.c                  | 17 ++++++++--------
 drivers/scsi/zorro7xx.c               | 16 +++++++--------
 drivers/scsi/zorro_esp.c              |  2 +-
 drivers/video/fbdev/cirrusfb.c        | 28 +++++++++++++--------------
 drivers/video/fbdev/fm2fb.c           |  6 +++---
 include/linux/mod_devicetable.h       |  6 +++++-
 14 files changed, 62 insertions(+), 57 deletions(-)


base-commit: d387b06f7c15b4639244ad66b4b0900c6a02b430
-- 
2.47.3

[PATCH v1 5/8] video: fm2fb: Use named initializer for zorro_device_id array

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

Using named initializers is more explicit and thus easier to parse for a
human.

While touching this array, drop the explicit zero from the list terminator.

This change doesn't introduce changes to the compiled zorro_device_id
array.

Signed-off-by: Uwe Kleine-König (The Capable Hub) <u.kleine-koenig@baylibre.com>
---
 drivers/video/fbdev/fm2fb.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/video/fbdev/fm2fb.c b/drivers/video/fbdev/fm2fb.c
index 25d2e716edf2..a62c8d86bf69 100644
--- a/drivers/video/fbdev/fm2fb.c
+++ b/drivers/video/fbdev/fm2fb.c
@@ -212,9 +212,9 @@ static int fm2fb_setcolreg(u_int regno, u_int red, u_int green, u_int blue,
 static int fm2fb_probe(struct zorro_dev *z, const struct zorro_device_id *id);
 
 static const struct zorro_device_id fm2fb_devices[] = {
-	{ ZORRO_PROD_BSC_FRAMEMASTER_II },
-	{ ZORRO_PROD_HELFRICH_RAINBOW_II },
-	{ 0 }
+	{ .id = ZORRO_PROD_BSC_FRAMEMASTER_II },
+	{ .id = ZORRO_PROD_HELFRICH_RAINBOW_II },
+	{ }
 };
 MODULE_DEVICE_TABLE(zorro, fm2fb_devices);
 
-- 
2.47.3

[PATCH v1 6/8] zorro: 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
zorro_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.

All users of struct zorro_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 | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
index 3b0c9a251a2e..2673a1bd82c4 100644
--- a/include/linux/mod_devicetable.h
+++ b/include/linux/mod_devicetable.h
@@ -640,7 +640,11 @@ struct mdio_device_id {
 
 struct zorro_device_id {
 	__u32 id;			/* Device ID or ZORRO_WILDCARD */
-	kernel_ulong_t driver_data;	/* Data private to the driver */
+	union {
+		/* Data private to the driver */
+		kernel_ulong_t driver_data;
+		const void *driver_data_ptr;
+	};
 };
 
 #define ZORRO_WILDCARD			(0xffffffff)	/* not official */
-- 
2.47.3

[PATCH v1 8/8] video: cirrusfb: Make use of struct zorro_device_id::driver_data_ptr

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

Usage of .driver_data_ptr allows to drop several casts and so make the
driver a bit more type safe.

While touching the zorro_device_id array, drop an unneeded explicit zero
in the list terminator.

Signed-off-by: Uwe Kleine-König (The Capable Hub) <u.kleine-koenig@baylibre.com>
---
 drivers/video/fbdev/cirrusfb.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/video/fbdev/cirrusfb.c b/drivers/video/fbdev/cirrusfb.c
index 2693b5cc053f..7e07e775b393 100644
--- a/drivers/video/fbdev/cirrusfb.c
+++ b/drivers/video/fbdev/cirrusfb.c
@@ -321,25 +321,25 @@ static const struct zorrocl zcl_picasso4_z2 = {
 
 static const struct zorro_device_id cirrusfb_zorro_table[] = {
 	{
-		.id		= ZORRO_PROD_HELFRICH_SD64_REG,
-		.driver_data	= (unsigned long)&zcl_sd64,
+		.id = ZORRO_PROD_HELFRICH_SD64_REG,
+		.driver_data_ptr = &zcl_sd64,
 	}, {
-		.id		= ZORRO_PROD_HELFRICH_PICCOLO_REG,
-		.driver_data	= (unsigned long)&zcl_piccolo,
+		.id = ZORRO_PROD_HELFRICH_PICCOLO_REG,
+		.driver_data_ptr = &zcl_piccolo,
 	}, {
-		.id	= ZORRO_PROD_VILLAGE_TRONIC_PICASSO_II_II_PLUS_REG,
-		.driver_data	= (unsigned long)&zcl_picasso,
+		.id = ZORRO_PROD_VILLAGE_TRONIC_PICASSO_II_II_PLUS_REG,
+		.driver_data_ptr = &zcl_picasso,
 	}, {
-		.id		= ZORRO_PROD_GVP_EGS_28_24_SPECTRUM_REG,
-		.driver_data	= (unsigned long)&zcl_spectrum,
+		.id = ZORRO_PROD_GVP_EGS_28_24_SPECTRUM_REG,
+		.driver_data_ptr = &zcl_spectrum,
 	}, {
-		.id		= ZORRO_PROD_VILLAGE_TRONIC_PICASSO_IV_Z3,
-		.driver_data	= (unsigned long)&zcl_picasso4_z3,
+		.id = ZORRO_PROD_VILLAGE_TRONIC_PICASSO_IV_Z3,
+		.driver_data_ptr = &zcl_picasso4_z3,
 	}, {
-		.id		= ZORRO_PROD_VILLAGE_TRONIC_PICASSO_IV_Z2_REG,
-		.driver_data	= (unsigned long)&zcl_picasso4_z2,
+		.id = ZORRO_PROD_VILLAGE_TRONIC_PICASSO_IV_Z2_REG,
+		.driver_data_ptr = &zcl_picasso4_z2,
 	},
-	{ 0 }
+	{ }
 };
 MODULE_DEVICE_TABLE(zorro, cirrusfb_zorro_table);
 #endif /* CONFIG_ZORRO */
@@ -2208,7 +2208,7 @@ static int cirrusfb_zorro_register(struct zorro_dev *z,
 	if (!info)
 		return -ENOMEM;
 
-	zcl = (const struct zorrocl *)ent->driver_data;
+	zcl = ent->driver_data_ptr;
 	btype = zcl->type;
 	regbase = zorro_resource_start(z) + zcl->regoffset;
 	ramsize = zcl->ramsize;
-- 
2.47.3

Re: [PATCH v1 0/8] zorro: Improve handling of pointers in zorro_device_id::driver_data

[Geert Uytterhoeven]

Hi Uwe,

On Tue, 26 May 2026 at 16:17, Uwe Kleine-König (The Capable Hub)
<u.kleine-koenig@baylibre.com> wrote:
> this series is about improving the handling of pointers in struct
> zorro_device_id's driver_data.
>
> While it's ok on all current Linux platforms to store a pointer in an
> unsigned long variable, it involves casting that loses type information.
> This can be nicely seen in patch #7 where after profiting from patch #6
> the compiler notices a missing const.
>
> Preparing for that change, all zorro_device_ids are converted to use
> named initializers, which is also a nice cleanup that could stand for
> itself, as it improves readability for humans. (That is necessary
> because an anonymous union can be initialized by name, but not using a
> list initializer.)
>
> My motivation for this series is the CHERI hardware extension. With that
> pointers are bigger than longs and thus you cannot store pointers in
> zorro_device_id::driver_data. So this series is also about getting
> support for CHERI into the mainline, but I hope the clean up effects
> mentioned above are justification enough to accept this series.

Thanks for your series!

> The dependencies in this series are as follows:
>
>  - Patch #5 depends on #1, #2

s/5/6/?

>  - Patches #7 and #8 depend on patch #6.
>
> So if the ata maintainers agreed to merge their patch #1 via scsi, and
> Geert agrees to patch #5 and that it's also merged via scsi, patches #1,

s/5/6/?

> #2, #6 and #7 can go in without further coordination.
>
> Patches #3, #4 and #5 are only about using the same initialization style
> for all zorro_device_id and can go in without coordination.
>
> Best regards
> Uwe
>
> Uwe Kleine-König (The Capable Hub) (8):
>   ata: pata_budda: Use named initializer for zorro_device_id
>   scsi: Use named initializer for zorro_device_id
>   net: Use named initializer for zorro_device_id arrays
>   i2c: icy: Use named initializer for zorro_device_id arrays
>   video: fm2fb: Use named initializer for zorro_device_id array
>   zorro: Simplify storing pointers in device id struct
>   scsi: zorro7xx: Make use of struct zorro_device_id::driver_data_ptr
>   video: cirrusfb: Make use of struct zorro_device_id::driver_data_ptr

Gr{oetje,eeting}s,

                        Geert


--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v1 0/8] zorro: Improve handling of pointers in zorro_device_id::driver_data

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

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

On Tue, May 26, 2026 at 05:01:48PM +0200, Geert Uytterhoeven wrote:
> Hi Uwe,
> 
> On Tue, 26 May 2026 at 16:17, Uwe Kleine-König (The Capable Hub)
> <u.kleine-koenig@baylibre.com> wrote:
> > this series is about improving the handling of pointers in struct
> > zorro_device_id's driver_data.
> >
> > While it's ok on all current Linux platforms to store a pointer in an
> > unsigned long variable, it involves casting that loses type information.
> > This can be nicely seen in patch #7 where after profiting from patch #6
> > the compiler notices a missing const.
> >
> > Preparing for that change, all zorro_device_ids are converted to use
> > named initializers, which is also a nice cleanup that could stand for
> > itself, as it improves readability for humans. (That is necessary
> > because an anonymous union can be initialized by name, but not using a
> > list initializer.)
> >
> > My motivation for this series is the CHERI hardware extension. With that
> > pointers are bigger than longs and thus you cannot store pointers in
> > zorro_device_id::driver_data. So this series is also about getting
> > support for CHERI into the mainline, but I hope the clean up effects
> > mentioned above are justification enough to accept this series.
> 
> Thanks for your series!
> 
> > The dependencies in this series are as follows:
> >
> >  - Patch #5 depends on #1, #2
> 
> s/5/6/?
> 
> >  - Patches #7 and #8 depend on patch #6.
> >
> > So if the ata maintainers agreed to merge their patch #1 via scsi, and
> > Geert agrees to patch #5 and that it's also merged via scsi, patches #1,
> 
> s/5/6/?

Yes, indeed. And I tried so hard to get all the numbers right :-\

Best regards
Uwe

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

Claude review: zorro: Improve handling of pointers in zorro_device_id::driver_data

[Claude Code Review Bot]

Overall Series Review

Subject: zorro: Improve handling of pointers in zorro_device_id::driver_data
Author: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig=20=28The=20Capable=20Hub=29?=
 <u.kleine-koenig@baylibre.com>
Patches: 6
Reviewed: 2026-05-27T14:47:32.957704

---

This is a well-structured cleanup series by Uwe Kleine-König targeting the `zorro_device_id` struct to improve type safety when storing pointers in `driver_data`. The mbox contains only 3 of 8 patches (5, 6, 8) — those relevant to the video/fbdev and the core header change. The remaining patches (1-4, 7) touch ata, scsi, net, and i2c subsystems and were filtered out.

The core idea (patch 6) is sound: replacing `kernel_ulong_t driver_data` with an anonymous union containing both `driver_data` and `const void *driver_data_ptr` eliminates unsafe `(unsigned long)` pointer casts. This is a standard pattern already used in other device ID structs in the kernel (e.g., `platform_device_id`). The motivation — CHERI compatibility — is legitimate, and the cleanup stands on its own merits regardless.

The patches are clean, mechanical, and correct. The anonymous union approach preserves backward compatibility with all existing users. No functional changes are introduced.

**Recommendation:** The series looks good. No correctness issues found.

---
Generated by Claude Code Patch Reviewer

Claude review: video: fm2fb: Use named initializer for zorro_device_id array

[Claude Code Review Bot]

Patch Review

**Status: Looks good.**

This converts two `zorro_device_id` entries from positional to named initializers:

```c
-	{ ZORRO_PROD_BSC_FRAMEMASTER_II },
-	{ ZORRO_PROD_HELFRICH_RAINBOW_II },
-	{ 0 }
+	{ .id = ZORRO_PROD_BSC_FRAMEMASTER_II },
+	{ .id = ZORRO_PROD_HELFRICH_RAINBOW_II },
+	{ }
```

The change is straightforward and correct. The `{ 0 }` to `{ }` change for the terminator is a minor style cleanup — both produce identical binary output (all-zeros), but `{ }` is the idiomatic way to express "zero-initialized struct" without implying a specific field. This is required preparation for patch 6, since the anonymous union cannot be initialized via positional syntax.

No concerns.

---
Generated by Claude Code Patch Reviewer

Claude review: zorro: Simplify storing pointers in device id struct

[Claude Code Review Bot]

Patch Review

**Status: Looks good.**

The core change in `include/linux/mod_devicetable.h`:

```c
 struct zorro_device_id {
 	__u32 id;			/* Device ID or ZORRO_WILDCARD */
-	kernel_ulong_t driver_data;	/* Data private to the driver */
+	union {
+		/* Data private to the driver */
+		kernel_ulong_t driver_data;
+		const void *driver_data_ptr;
+	};
 };
```

This is clean and backward compatible. Key observations:

1. **Union sizing:** `kernel_ulong_t` is `unsigned long long` on 32-bit arches (for cross-compilation with file2alias), so the union is at least 8 bytes. On all current architectures, `sizeof(void *)` <= `sizeof(kernel_ulong_t)`, so the union doesn't change struct size or layout. On CHERI, pointers would be larger, but that's exactly the scenario this enables.

2. **Anonymous union:** Correct choice — it allows `ent->driver_data` and `ent->driver_data_ptr` to be used interchangeably without changing any syntax at call sites.

3. **Backward compatibility:** All existing users that set `.driver_data = (unsigned long)&foo` continue to compile and work. The patches 7 and 8 then convert them to `.driver_data_ptr` at their own pace. This is the right incremental approach.

4. **const correctness:** Using `const void *` is appropriate — it preserves const-ness of the pointed-to data, which the `(unsigned long)` cast was losing.

One minor note: the commit message says "no adaptions are needed" for existing users — this is correct because all current initializers use `.driver_data = ...` named syntax or positional syntax that maps to `.id` (the first field), and `driver_data` remains accessible at the same name.

No concerns.

---
Generated by Claude Code Patch Reviewer

Claude review: video: cirrusfb: Make use of struct zorro_device_id::driver_data_ptr

[Claude Code Review Bot]

Patch Review

**Status: Looks good.**

This patch converts cirrusfb to use the new `driver_data_ptr` field. The table initialization changes from:

```c
-		.driver_data	= (unsigned long)&zcl_sd64,
+		.driver_data_ptr = &zcl_sd64,
```

And the consumption site changes from:

```c
-	zcl = (const struct zorrocl *)ent->driver_data;
+	zcl = ent->driver_data_ptr;
```

Both directions of the conversion are correct:

1. **Initialization side:** Drops the `(unsigned long)` cast. The `const struct zorrocl *` is implicitly converted to `const void *` — this is safe and preserves const.

2. **Usage side:** `ent->driver_data_ptr` is `const void *`, which is implicitly converted to `const struct zorrocl *` in the assignment to `zcl`. This is valid C and preserves the const qualifier that was previously being explicitly cast (actually correctly in the old code, but the cast was fragile — e.g., if someone changed the type of `zcl`, the old `(const struct zorrocl *)` cast would silently hide any mismatch).

3. **Terminator cleanup:** `{ 0 }` → `{ }` is consistent with patch 5.

4. **Alignment reformatting:** The patch also drops the column-aligned `=` signs (e.g., `.id\t\t=` → `.id =`). This is a minor style preference; the kernel doesn't mandate column alignment in struct initializers, so this is fine as a drive-by cleanup.

No concerns.

---
Generated by Claude Code Patch Reviewer