Hello Jarkko,
On Fri, May 22, 2026 at 12:43:23AM +0300, Jarkko Sakkinen wrote:
> On Mon, May 18, 2026 at 03:40:35PM +0200, Uwe Kleine-König (The Capable Hub)
> wrote:
> > The union added to struct i2c_device_id enables further cleanups like:
> >
> > diff --git a/drivers/regulator/ad5398.c b/drivers/regulator/ad5398.c
> > index 0123ca8157a8..dfb0b07500a7 100644
> > --- a/drivers/regulator/ad5398.c
> > +++ b/drivers/regulator/ad5398.c
> > @@ -207,8 +207,8 @@ struct ad5398_current_data_format {
> > static const struct ad5398_current_data_format df_10_4_120 = {10, 4,
> > 0, 120000};
> >
> > static const struct i2c_device_id ad5398_id[] = {
> > - { .name = "ad5398", .driver_data = (kernel_ulong_t)&df_10_4_120
> > },
> > - { .name = "ad5821", .driver_data = (kernel_ulong_t)&df_10_4_120
> > },
> > + { .name = "ad5398", .driver_data_ptr = &df_10_4_120 },
> > + { .name = "ad5821", .driver_data_ptr = &df_10_4_120 },
> > { }
> > };
> > MODULE_DEVICE_TABLE(i2c, ad5398_id);
> > @@ -219,8 +219,7 @@ static int ad5398_probe(struct i2c_client *client)
> > struct regulator_init_data *init_data =
> > dev_get_platdata(&client->dev);
> > struct regulator_config config = { };
> > struct ad5398_chip_info *chip;
> > - const struct ad5398_current_data_format *df =
> > - (struct ad5398_current_data_format
> > *)id->driver_data;
> > + const struct ad5398_current_data_format *df =
> > id->driver_data_ptr;
> >
> > chip = devm_kzalloc(&client->dev, sizeof(*chip), GFP_KERNEL);
> > if (!chip)[redacted the example as it didn't compile as presented, added "_ptr" on last hunk's + line.] > > that are an improvement for readability (again!) and it keeps some > > properties of the pointers (here: being const) without having to pay > > attention for that. (I didn't find a tpm driver that benefits, so this > > is "only" a regulator driver example.) > > > > My additional motivation for this effort is CHERI[1]. This is a hardware > > extension that uses 128 bit pointers but unsigned long is still 64 bit. > > So with CHERI you cannot store pointers in unsigned long variables. > > I don't understand why any of this should be merged to be honest and > why I should care about CHERI when reviewing mainline patches. While I think and hope that CHERI will become relevant for the industry soon, it's ok to not care about CHERI today and I mostly mentioned it to be transparent about *my* motivation. Our eventual goal is to bring CHERI support to mainline linux so my team mates and I have to find a way to get patches like that in. In my eyes this compares well to PREEMPT RT: With that you have to follow more rules in some situations and implementing these and running an RT kernel makes bugs pop up that also affect mainline. So I claim that working on CHERI is obviously beneficial to folks who have CHERI hardware, but also helps those who don't as CHERI is a way to easily spot a relevant set of bugs. > Clean up can be side-effect but not a purpose. Oh, I disagree. Having code in a state where you can easily see what happens helps to concentrate on the parts that are more complicated. So it's a win for maintenance and lowering the entry bar for people who are not used to Linux kernel code. There are parts in the kernel that are complicated, and we won't get rid of them, because operating systems are complicated. But my POV here is that making it easier where it's possible is a good thing and a reason for itself. You might call that a paper cut only, but these add up. Also with the union in i2c_device_id the compiler warns you if some code is lacking a "const". So it becomes harder to make mistakes, this is also a reason that in my book is good enough for itself. Best regards Uwe
signature.asc
Description: PGP signature
