RE: [PATCH] Enable ACPI_ADR_SPACE_PCI_CONFIG in acpi_gbl_default_address_spaces only when ACPI_PCI_CONFIGURED is defined

[Moore, Robert]

-----Original Message-----
From: Weidong Cui <weidongcui@xxxxxxxxx> 
Sent: Thursday, March 04, 2021 9:06 AM
To: Moore, Robert <robert.moore@xxxxxxxxx>
Cc: Kaneda, Erik <erik.kaneda@xxxxxxxxx>; Wysocki, Rafael J <rafael.j.wysocki@xxxxxxxxx>; Xinyang Ge <aegiryy@xxxxxxxxx>; linux-acpi@xxxxxxxxxxxxxxx; devel@xxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Len Brown <lenb@xxxxxxxxxx>
Subject: Re: [PATCH] Enable ACPI_ADR_SPACE_PCI_CONFIG in acpi_gbl_default_address_spaces only when ACPI_PCI_CONFIGURED is defined

> Well, I don't like the fact that PCI_CONFIGURED would have to be defined by each current host:
>
> > +#ifdef ACPI_PCI_CONFIGURED
>
> I would rather the logic be reversed:
>
> > +#ifdef ACPI_PCI_NOT_CONFIGURED

Thank you for the comments, Erik and Bob!

ACPI_PCI_CONFIGURED is defined in aclinux.h (see below) and used in several places in evhandler.c and exregion.c.
I'm not sure why we want to introduce ACPI_PCI_NOT_CONFIGURED.  Bob, I don't understand your concerns about "have to be defined by each current host".  Can you please shed some light on it?

It is required in aclinux.h, and thus it is required in every host-dependent configuration file (acfreebsd.h, acmacosx.h, acnetbsd.h, achaiku.h, etc.) I would rather not force these host-specific header files to change.
Bob


#ifdef CONFIG_PCI
#define ACPI_PCI_CONFIGURED
#endif

> -----Original Message-----
> From: Kaneda, Erik <erik.kaneda@xxxxxxxxx>
> Sent: Wednesday, March 03, 2021 10:29 AM
> To: Weidong Cui <weidongcui@xxxxxxxxx>; Moore, Robert 
> <robert.moore@xxxxxxxxx>; Wysocki, Rafael J 
> <rafael.j.wysocki@xxxxxxxxx>
> Cc: Xinyang Ge <aegiryy@xxxxxxxxx>; linux-acpi@xxxxxxxxxxxxxxx; 
> devel@xxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Len Brown 
> <lenb@xxxxxxxxxx>
> Subject: RE: [PATCH] Enable ACPI_ADR_SPACE_PCI_CONFIG in 
> acpi_gbl_default_address_spaces only when ACPI_PCI_CONFIGURED is 
> defined
>
> This looks good to me. Bob, do you have any comments?
>
> Erik
>
> > -----Original Message-----
> > From: Weidong Cui <weidongcui@xxxxxxxxx>
> > Sent: Monday, February 8, 2021 7:18 PM
> > To: Moore, Robert <robert.moore@xxxxxxxxx>; Kaneda, Erik 
> > <erik.kaneda@xxxxxxxxx>; Wysocki, Rafael J 
> > <rafael.j.wysocki@xxxxxxxxx>; Len Brown <lenb@xxxxxxxxxx>
> > Cc: Weidong Cui <weidongcui@xxxxxxxxx>; Xinyang Ge 
> > <aegiryy@xxxxxxxxx>; linux-acpi@xxxxxxxxxxxxxxx; devel@xxxxxxxxxx;
> > linux- kernel@xxxxxxxxxxxxxxx
> > Subject: [PATCH] Enable ACPI_ADR_SPACE_PCI_CONFIG in 
> > acpi_gbl_default_address_spaces only when ACPI_PCI_CONFIGURED is 
> > defined
> >
> > Signed-off-by: Weidong Cui <weidongcui@xxxxxxxxx>
> > Signed-off-by: Xinyang Ge <aegiryy@xxxxxxxxx>
> > ---
> >  drivers/acpi/acpica/evhandler.c | 2 ++
> >  include/acpi/acconfig.h         | 4 ++++
> >  2 files changed, 6 insertions(+)
> >
> > diff --git a/drivers/acpi/acpica/evhandler.c 
> > b/drivers/acpi/acpica/evhandler.c index 5884eba04..4c25ad433 100644
> > --- a/drivers/acpi/acpica/evhandler.c
> > +++ b/drivers/acpi/acpica/evhandler.c
> > @@ -26,7 +26,9 @@ acpi_ev_install_handler(acpi_handle obj_handle,
> >  u8 acpi_gbl_default_address_spaces[ACPI_NUM_DEFAULT_SPACES] = {
> >       ACPI_ADR_SPACE_SYSTEM_MEMORY,
> >       ACPI_ADR_SPACE_SYSTEM_IO,
> > +#ifdef ACPI_PCI_CONFIGURED
> >       ACPI_ADR_SPACE_PCI_CONFIG,
> > +#endif
> >       ACPI_ADR_SPACE_DATA_TABLE
> >  };
> >
> > diff --git a/include/acpi/acconfig.h b/include/acpi/acconfig.h index
> > a225eff49..790999028 100644
> > --- a/include/acpi/acconfig.h
> > +++ b/include/acpi/acconfig.h
> > @@ -162,7 +162,11 @@
> >  /* Maximum space_ids for Operation Regions */
> >
> >  #define ACPI_MAX_ADDRESS_SPACE          255
> > +#ifdef ACPI_PCI_CONFIGURED
> >  #define ACPI_NUM_DEFAULT_SPACES         4
> > +#else
> > +#define ACPI_NUM_DEFAULT_SPACES         3
> > +#endif
> >
> >  /* Array sizes.  Used for range checking also */
> >
> > --
> > 2.24.3 (Apple Git-128)
>