Re: [PATCH 10/10] ACPI: IORT: Allow COMPILE_TEST of IORT

[Jason Gunthorpe <jgg@nvidia.com>]

On Thu, Nov 30, 2023 at 12:12:26PM +0100, Lorenzo Pieralisi wrote:
> On Wed, Nov 29, 2023 at 03:12:40PM -0400, Jason Gunthorpe wrote:
> > On Wed, Nov 29, 2023 at 01:55:04PM +0100, Lorenzo Pieralisi wrote:
> > 
> > > I don't think it should be done this way. Is the goal compile testing
> > > IORT code ? 
> > 
> > Yes
> > 
> > > If so, why are we forcing it through the SMMU (only because
> > > it can be compile tested while eg SMMUv3 driver can't ?) menu entry ?
> > 
> > Because something needs to select it, and SMMU is one of the places
> > that are implicitly using it.
> > 
> > It isn't (and shouldn't be) a user selectable kconfig. Currently the
> > only thing that selects it is the ARM64 master kconfig.
> 
> I never said it should be a user selectable kconfig. I said that
> I don't like using the SMMU entry (only) to select it just because
> that entry allows COMPILE_TEST.

So you would like each of the drivers that use it to select it?

> > SMMUv3 doesn't COMPILE_TEST so it picks up the dependency transitivity
> > through ARM64. I'm not sure why IORT was put as a global ARM64 kconfig
> > dependency and not put in the places that directly need it.
> 
> Because IORT is used by few ARM64 system IPs (that are enabled by
> default, eg GIC), it makes sense to have a generic ARM64 select (if ACPI).

IMHO that is not a good way to use kconfig, it is obfuscating and
doesn't support things like COMPILE_TEST.

> > > Maybe we can move IORT code into drivers/acpi and add a silent config
> > > option there with a dependency on ARM64 || COMPILE_TEST.
> > 
> > That seems pretty weird to me, this is the right way to approach it,
> > IMHO. Making an entire directory condition is pretty incompatible with
> > COMPILE_TEST as a philosophy.
> 
> That's not what I was suggesting. I was suggesting to move iort.c (or
> some portions of it) into drivers/acpi if we care enough to compile test
> it on arches !ARM64.
> 
> It is also weird to have a file in drivers/acpi/arm64 that you want
> to compile test on other arches IMO (and I don't think it is very useful
> to compile test it either).

Why? Just because the directory is named "arm64" doesn't mean it
should be excluded from COMPILE_TEST. arch/arm64 yes, but not random
directories in the driver tree.

Stuff under drivers/ should strive to get 100% COMPILE_TEST coverage
as much as practical. This makes everyone else's life easier.

Jason