In a lot of places we are using of_device_is_compatible() to check for quirks
etc that are simply not possible on some targets, i.e. a piece of hardware
that needs special handling is only on one specific ARM machine and your
target isn't even ARM.
Add of_device_is_possible_and_compatible() that also takes a Kconfig
symbol and checks if that is enabled before calling of_device_is_compatible().
The Kconfig symbol is build time constant and the compiler should
remove the call to of_device_is_compatible() if it is unneeded and also
remove the data for the compatible string.
Another merit of this is that in places were we are checking for quirks
outside of drivers themselves, i.e. in the gpio and spi subsystems where
some legacy devicetree handling is being handled for specific devices
is in the core code, when the drivers that need the quirks are removed
their Kconfig symbol should also be removed and it'll be easier to spot
that the quirk handling can also go.
Signed-off-by: Daniel Palmer <daniel@thingy.jp>
---
include/linux/of.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/include/linux/of.h b/include/linux/of.h
index 9bbdcf25a2b4..70be20b0be22 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -358,6 +358,8 @@ extern int of_property_read_string_helper(const struct device_node *np,
const char **out_strs, size_t sz, int index);
extern int of_device_is_compatible(const struct device_node *device,
const char *);
+#define of_device_is_possible_and_compatible(symbol, device, string) \
+ (IS_ENABLED(symbol) && of_device_is_compatible(device, string))
extern int of_device_compatible_match(const struct device_node *device,
const char *const *compat);
extern bool of_device_is_available(const struct device_node *device);
--
2.51.0
On Wed, Jan 07, 2026 at 12:07:30PM +0900, Daniel Palmer wrote: > In a lot of places we are using of_device_is_compatible() to check for quirks I'm assuming 'a lot' is not just 3 places? Got a rough estimate? This seems fine to me assuming there are more. > etc that are simply not possible on some targets, i.e. a piece of hardware > that needs special handling is only on one specific ARM machine and your > target isn't even ARM. > > Add of_device_is_possible_and_compatible() that also takes a Kconfig > symbol and checks if that is enabled before calling of_device_is_compatible(). > > The Kconfig symbol is build time constant and the compiler should > remove the call to of_device_is_compatible() if it is unneeded and also > remove the data for the compatible string. > > Another merit of this is that in places were we are checking for quirks > outside of drivers themselves, i.e. in the gpio and spi subsystems where > some legacy devicetree handling is being handled for specific devices > is in the core code, when the drivers that need the quirks are removed > their Kconfig symbol should also be removed and it'll be easier to spot > that the quirk handling can also go. > > Signed-off-by: Daniel Palmer <daniel@thingy.jp> > --- > include/linux/of.h | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/include/linux/of.h b/include/linux/of.h > index 9bbdcf25a2b4..70be20b0be22 100644 > --- a/include/linux/of.h > +++ b/include/linux/of.h > @@ -358,6 +358,8 @@ extern int of_property_read_string_helper(const struct device_node *np, > const char **out_strs, size_t sz, int index); > extern int of_device_is_compatible(const struct device_node *device, > const char *); > +#define of_device_is_possible_and_compatible(symbol, device, string) \ > + (IS_ENABLED(symbol) && of_device_is_compatible(device, string)) > extern int of_device_compatible_match(const struct device_node *device, > const char *const *compat); > extern bool of_device_is_available(const struct device_node *device); > -- > 2.51.0 >
Hi Rob, On Fri, 9 Jan 2026 at 08:38, Rob Herring <robh@kernel.org> wrote: > > On Wed, Jan 07, 2026 at 12:07:30PM +0900, Daniel Palmer wrote: > > In a lot of places we are using of_device_is_compatible() to check for quirks > > I'm assuming 'a lot' is not just 3 places? Got a rough estimate? > > This seems fine to me assuming there are more. In core code (like the gpio core, and not in a specific driver) there are only a few places. I think around 10. There are more when we get into drivers that handle lots of variants of the same hardware and check the compatible string during probe. (There are ~700 calls to of_device_is_compatible() in drivers/, most of which seems to be quirks checking during probe). Since this didn't get shot down I'll prepare a version that has more examples. Thanks, Daniel
Hi Daniel,
thanks for your patch!
I like the idea in this patch set. Footprint is something we need
to think more about, if for nothing else so for the increasing tendency
of RAM prices.
On Fri, Jan 9, 2026 at 3:52 AM Daniel Palmer <daniel@thingy.jp> wrote:
> On Fri, 9 Jan 2026 at 08:38, Rob Herring <robh@kernel.org> wrote:
> >
> > On Wed, Jan 07, 2026 at 12:07:30PM +0900, Daniel Palmer wrote:
> > > In a lot of places we are using of_device_is_compatible() to check for quirks
> >
> > I'm assuming 'a lot' is not just 3 places? Got a rough estimate?
> >
> > This seems fine to me assuming there are more.
>
> In core code (like the gpio core, and not in a specific driver) there
> are only a few places. I think around 10.
Actually, if you look in gpiolib-of.c in e.g. of_gpio_try_fixup_polarity()
you find:
#if IS_ENABLED(CONFIG_LCD_HX8357)
/*
* Himax LCD controllers used incorrectly named
* "gpios-reset" property and also specified wrong
* polarity.
*/
{ "himax,hx8357", "gpios-reset", false },
{ "himax,hx8369", "gpios-reset", false },
#endif
etc etc etc.
It is actually a similar idea just clunkier, but maybe necessary
for these quirks since they are in a table.
But as you can see, there is some of the same thinking: if it's
not configured in, then we need to compile it out.
Yours,
Linus Walleij
On Fri, Jan 09, 2026 at 11:51:52AM +0900, Daniel Palmer wrote: > Hi Rob, > > On Fri, 9 Jan 2026 at 08:38, Rob Herring <robh@kernel.org> wrote: > > > > On Wed, Jan 07, 2026 at 12:07:30PM +0900, Daniel Palmer wrote: > > > In a lot of places we are using of_device_is_compatible() to check for quirks > > > > I'm assuming 'a lot' is not just 3 places? Got a rough estimate? > > > > This seems fine to me assuming there are more. > > In core code (like the gpio core, and not in a specific driver) there > are only a few places. I think around 10. > There are more when we get into drivers that handle lots of variants > of the same hardware and check the compatible string during probe. > (There are ~700 calls to of_device_is_compatible() in drivers/, most > of which seems to be quirks checking during probe). Generally in drivers, it is preferred to use match data rather than of_device_is_compatible(). And if we're going in and touching of_device_is_compatible() in drivers, that's what we want to do. Using match data of course doesn't help your cause of reducing size. I suppose you could define a macro that includes a compatible in the match table or not. If the match data is function ptrs, then if those functions aren't referenced, they would be dropped by the compiler. Rob
On Fri, Jan 9, 2026 at 6:29 AM Rob Herring <robh@kernel.org> wrote: > > On Fri, Jan 09, 2026 at 11:51:52AM +0900, Daniel Palmer wrote: > > Hi Rob, > > > > On Fri, 9 Jan 2026 at 08:38, Rob Herring <robh@kernel.org> wrote: > > > > > > On Wed, Jan 07, 2026 at 12:07:30PM +0900, Daniel Palmer wrote: > > > > In a lot of places we are using of_device_is_compatible() to check for quirks > > > > > > I'm assuming 'a lot' is not just 3 places? Got a rough estimate? > > > > > > This seems fine to me assuming there are more. > > > > In core code (like the gpio core, and not in a specific driver) there > > are only a few places. I think around 10. > > There are more when we get into drivers that handle lots of variants > > of the same hardware and check the compatible string during probe. > > (There are ~700 calls to of_device_is_compatible() in drivers/, most > > of which seems to be quirks checking during probe). > > Generally in drivers, it is preferred to use match data rather than > of_device_is_compatible(). And if we're going in and touching > of_device_is_compatible() in drivers, that's what we want to do. Using > match data of course doesn't help your cause of reducing size. I suppose > you could define a macro that includes a compatible in the match table > or not. If the match data is function ptrs, then if those functions > aren't referenced, they would be dropped by the compiler. For the 10 or so instances in the core, I'm not sure the macro is even worth it. It's just hiding the IS_ENABLED() and obscuring the intent for not much of a reduction in code size. Not going to Nack it if Rob agrees, but I don't see the point of the macro. I see the point behind the idea though. Also, if we do land it, maybe call it "enabled" instead of "possible"? That lines up better with IS_ENABLED. -Saravana > > Rob
Hi Saravana,
On Mon, 12 Jan 2026 at 11:32, Saravana Kannan <saravanak@kernel.org> wrote:
> For the 10 or so instances in the core, I'm not sure the macro is even
> worth it. It's just hiding the IS_ENABLED() and obscuring the intent
> for not much of a reduction in code size. Not going to Nack it if Rob
> agrees, but I don't see the point of the macro. I see the point behind
> the idea though.
Understood. This is very niche. In my case by doing stuff like this I
could make the kernel small enough for my userland to boot again.
I have no mmu, 8MB of RAM, CPU frequency in the tens of MHz. Not very common.
But I think it'd also be nice if all of the tons of arm64 machines out
there don't check for hacks for stuff that is only in ppc and
therefore can never happen. :)
While looking through the users of of_device_is_compatible() I also
found some, I think, bad patterns that are probably worse than the
unneeded code/data.
For example, in drivers/usb/host/uhci-platform.c we have:
static int uhci_hcd_platform_probe(struct platform_device *pdev)
{
...
if (of_device_is_compatible(np, "aspeed,ast2400-uhci") ||
of_device_is_compatible(np, "aspeed,ast2500-uhci") ||
of_device_is_compatible(np, "aspeed,ast2600-uhci")) {
uhci->is_aspeed = 1;
dev_info(&pdev->dev,
"Enabled Aspeed implementation workarounds\n");
}
...
The compiler will always include those calls if this driver is built.
But the is_aspeed flag they are setting is accessed via a macro that
always returns false if the aspeed platform that needs workarounds
isn't enabled to remove the unneeded code.
I don't think this is a bug that can cause problems but this means if
you use a DT with those strings and the kernel doesn't have that
platform enabled (and it somehow boots far enough etc etc) you'd see
this message saying the workarounds are enabled but actually the
compiler removed all of that code and the message is a lie.
I think Rob wrote basically the same thing but it seems like
of_device_is_compatible() in probe functions is just a bad idea and
workaround flags etc should be in the match data not decided at
runtime with of_device_is_compatible().
Cheers,
Daniel
© 2016 - 2026 Red Hat, Inc.