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

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.

> > 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.

> > NAKed-by: Andy Shevchenko <[email protected]>

The tag still stays. But nobody prevents media maintainers to do what they want
:-)

> > 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.

-- 
With Best Regards,
Andy Shevchenko