Linux Media Controller development
 help / color / mirror / Atom feed
From: Andy Shevchenko <andriy.shevchenko@intel.com>
To: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: linux-media@vger.kernel.org, linux-acpi@vger.kernel.org,
	rafael@kernel.org
Subject: Re: [PATCH 3/3] media: i2c: Use ACPI_PTR() for ACPI ID tables
Date: Sun, 30 Aug 2026 10:06:54 +0300	[thread overview]
Message-ID: <apPWjluHkxwHCagh@ashevche-desk.local> (raw)
In-Reply-To: <apGsxDddKkRsP-Un@kekkonen.localdomain>

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 <andriy.shevchenko@intel.com>

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



  reply	other threads:[~2026-08-30  7:06 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-28  8:43 [PATCH 0/3] Make ACPI_PTR() use PTR_IF() Sakari Ailus
2026-08-28  8:43 ` [PATCH 1/3] media: ipu-bridge: Drop redundant ACPI_PTR() usage Sakari Ailus
2026-08-28  9:58   ` Rafael J. Wysocki (Intel)
2026-08-28  8:43 ` [PATCH 2/3] ACPI: Use PTR_IF() for ACPI_PTR() Sakari Ailus
2026-08-28  9:56   ` Rafael J. Wysocki (Intel)
2026-08-28  8:43 ` [PATCH 3/3] media: i2c: Use ACPI_PTR() for ACPI ID tables Sakari Ailus
2026-08-28 13:30   ` Andy Shevchenko
2026-08-28 15:10     ` Sakari Ailus
2026-08-28 15:22       ` Andy Shevchenko
2026-08-28 15:44         ` Sakari Ailus
2026-08-30  7:06           ` Andy Shevchenko [this message]
2026-08-31 10:55             ` Sakari Ailus

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=apPWjluHkxwHCagh@ashevche-desk.local \
    --to=andriy.shevchenko@intel.com \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=rafael@kernel.org \
    --cc=sakari.ailus@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox