Re: [PATCH v3 09/16] platform/x86: x86-android-tablets: Add include defining struct dmi_system_id

["Uwe Kleine-König (The Capable Hub)" <u.kleine-koenig@baylibre.com>]

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

Hello Ilpo,

On Tue, Jun 30, 2026 at 05:58:54PM +0300, Ilpo Järvinen wrote:
> On Mon, 29 Jun 2026, Uwe Kleine-König (The Capable Hub) wrote:
> > On Mon, Jun 29, 2026 at 02:38:35PM +0300, Ilpo Järvinen wrote:
> > > On Sun, 28 Jun 2026, Uwe Kleine-König (The Capable Hub) wrote:
> > > 
> > > > Currently <linux/i2c.h> includes <linux/mod_devicetable.h> transitively
> > > > which ensures that struct dmi_system_id is defined in
> > > > drivers/platform/x86/x86-android-tablets/x86-android-tablets.h. However
> > > > this include in <linux/i2c.h> will be replaced by one for i2c_device_id
> > > > only. To ensure that dmi_system_id is available add the include for that
> > > > explicitly.
> > > > 
> > > > Acked-by: Danilo Krummrich <dakr@kernel.org>
> > > > Signed-off-by: Uwe Kleine-König (The Capable Hub) <u.kleine-koenig@baylibre.com>
> > > > ---
> > > >  drivers/platform/x86/x86-android-tablets/x86-android-tablets.h | 1 +
> > > >  1 file changed, 1 insertion(+)
> > > > 
> > > > diff --git a/drivers/platform/x86/x86-android-tablets/x86-android-tablets.h b/drivers/platform/x86/x86-android-tablets/x86-android-tablets.h
> > > > index 2498390958ad..c756961ae5fd 100644
> > > > --- a/drivers/platform/x86/x86-android-tablets/x86-android-tablets.h
> > > > +++ b/drivers/platform/x86/x86-android-tablets/x86-android-tablets.h
> > > > @@ -13,6 +13,7 @@
> > > >  #include <linux/gpio/consumer.h>
> > > >  #include <linux/i2c.h>
> > > >  #include <linux/irqdomain_defs.h>
> > > > +#include <linux/device-id/dmi.h>
> > > >  #include <linux/spi/spi.h>
> > > >  
> > > >  struct gpio_desc;
> > > 
> > > Indirect include patchs are never a good idea as it always makes header 
> > > reorganizations minefield. So for this change,
> > > 
> > > Acked-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> > > 
> > > 
> > > When it comes to the series, I certainly like the direction it goes very 
> > > much (never have been fan of mod_devicetable.h) but I'd have preferred 
> > > stepwise approach over trying to introduce it to some mid-rc.
> > 
> > The hurdle here is that at least the header part isn't easily
> > automateable.
> 
> Patch 1?

No, patch 15.

> > (Well it is, but the script that I have now to check all
> > .c files already takes longer than an hour to run. I guess it's not as
> > optimal as it could, but still.) And so getting this ready for inclusion
> > in next and keeping it up to date until the merge window opens is a
> > tough job.
> > 
> > The further downside is that each modification to one of the highly used
> > headers is expensive (in rebuild time) when these are merged one after
> > another (or when bisecting). So getting these changes in in one batch is
> > beneficial.
> > 
> > So while I agree with your preference, I don't think it's feasible.
> 
> While I understand keeping it up-to-date must be a pain, I'm not entirely 
> convinced by the rebuild time argument as it has been the status quo so 
> far too for anything touching mod_devicetable.h.

Yes. I don't know about others, but big rebuilds annoy me regularily.
And with another quest that involves touching most struct *_device_id
this really gets a pain. And just because "we" endured something in the
past, doesn't mean we shouldn't improve.

> If one commit would move all linux/device-id/* AND ADD them all into 
> mod_devicetable.h as includes, that's just one rebuild more (and one 
> rebuild will occur anyway whichever way you architect this).

Yes, one rebuild isn't possible to prevent. You're talking about patch
#1, right?

> Only if includes would be then removed one-by-one from mod_devicetable.h 
> (e.g. per subsystem basis), I can see it causing n rebuilds (+conflicts), 
> but they could be removed in bulk as well.

Removing those from mod_devicetable.h isn't part of the plan. But
replacing `#include <linux/mod_devicetable.h>` by `#include
<linux/device-id/of.h>` in <linux/of.h> triggers again many rebuilds
when done separately.
And then for <linux/pci.h> and again for <linux/platform_driver.h> and
.... That's why those are included here, too, to at least use the "one
rebuild needed" window for the high-impact follow up changes, and
trigger a rebuild only once (unless you happen to hit an inbetween
commit while bisecting).

> What you can achieve is preventing those "normal" mod_devicetable.h 
> changes enforcing rebuilds right after this has been applied, but that is 
> just the rebuilds as status quo would have without this series and no 
> more.

This is exactly the goal. I don't get the "just" part of your argument.
The point is that a modification to struct i2c_device_id should only
trigger a rebuild of i2c drivers, but not of pci, platform, spi and all
the others, too.

Best regards
Uwe

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