Re: [PATCH 3/3] media: i2c: Use ACPI_PTR() for ACPI ID tables

On Sun, Aug 30, 2026 at 10:06:54AM +0300, Andy Shevchenko wrote:
> On Fri, Aug 28, 2026 at 06:44:04PM +0300, Sakari Ailus wrote:
> > On Fri, Aug 28, 2026 at 06:22:43PM +0300, Andy Shevchenko wrote:
> > > On Fri, Aug 28, 2026 at 06:10:55PM +0300, Sakari Ailus wrote:
> > > > On Fri, Aug 28, 2026 at 03:30:10PM +0200, Andy Shevchenko wrote:
> > > > > On Fri, Aug 28, 2026 at 11:43:28AM +0300, Sakari Ailus wrote:
> > > > > > Add ACPI_PTR() to the ACPI ID tables where missing. The patch has been
> > > > > > produced with the following command:
> > > > > > 
> > > > > > git grep -lP 'acpi_match_table.*=\s*(?!ACPI_PTR)' \
> > > > > > 	drivers/media/|while read i; do
> > > > > > 	perl -i -pe 's/\.acpi_match_table\s*=\s*\K(\w+),/ACPI_PTR($1),/' $i;
> > > > > > done
> > > > > 
> > > > > NAK.
> > > > > 
> > > > > It doesn't explain why we need that. This also inconsistent with OF type of
> > > > > IDs.
> > > > 
> > > > ACPI IDs are used on ACPI only while compatible strings are used on both
> > > > ACPI and OF. Thus the ACPI ID table is redundant if CONFIG_ACPI is
> > > > disabled.
> > > 
> > > They may be informative for the cases when on the working say OF system I can
> > > get the IDs for ACPI based platforms at run-time.
> > 
> > That's a bit far-fetched I think. Have you ever done that? :-)
> 
> I have not working with OF platforms, but I'm pretty much see value of this
> possibility.
> 
> > Media drivers are probably one of the largest groups of drivers that are as
> > relevant for ACPI as they are for OF.
> > 
> > The of_match_ptr() that used to be present in nearly all media drivers has
> > been largely eliminated in the same context and the only reason for that is
> > that the table is also relevant for ACPI systems.
> 
> So, if we ever will have a code to use ACPI IDs in OF enumeration, this will
> have the same argument.
> 
> > The case is indeed the
> > opposite to the ACPI equivalent so I really don't see why we shouldn't just
> > drop it where it's, well, redundant.
> 
> The use of the unneeded macro makes code harder to follow. Still ID tables
> are useful on their own.

About 20 drivers currently in Media tree use this "unneeded" macro in this
context. (These either have #ifdefs, __maybe_unused or depend on ACPI
directly, and the two first then can be removed after the two first patches
in the series.)

The rest of the reasoning you provided above is rather improbable IMHO.

> 
> > > Also you just save a couple of dozens of bytes. This is peanuts and unneeded
> > > churn. Again, here is my formal
> > 
> > I recall you often comment on struct field ordering to save, well,
> > typically less than dozens of bytes, too.
> 
> There is a difference, the ID tables are from RO sections, the structures
> are runtime and grow linearly as many objects of a type are created. Among
> two the priority of reducing runtime memory footprint has a priority over
> the RO sections.

These are often device specific and typically, albeit not always, there's
one device (especially camera sensors etc.) of a kind in a system.

> 
> > > NAKed-by: Andy Shevchenko <[email protected]>
> 
> The tag still stays. But nobody prevents media maintainers to do what they want
> :-)

Kernel documentation appears not to recognise such a tag. This can always
be found in lore though.

> 
> > > In case you want to override that, please keep it in the commit message (among
> > > other tags). And yes, we have such a tag in the Git history of Linux kernel.
> 

-- 
Regards,

Sakari Ailus