| Message ID | 20240407063341.3710801-1-sathyanarayanan.kuppuswamy@linux.intel.com (mailing list archive) |
|---|---|
| State | In Next |
| Delegated to: | Rafael Wysocki |
| Headers | show |
| Series | [v1] ACPI: Declare acpi_blacklisted() only if CONFIG_X86 is enabled | expand |
On Sat, Apr 06, 2024 at 11:33:41PM -0700, Kuppuswamy Sathyanarayanan wrote: > The function acpi_blacklisted() is defined only when CONFIG_X86 is > enabled. So to keep it consistent, protect its declaration with > CONFIG_X86. ... > extern char acpi_video_backlight_string[]; > extern long acpi_is_video_device(acpi_handle handle); > +#ifdef CONFIG_X86 > extern int acpi_blacklisted(void); > +#endif > extern void acpi_osi_setup(char *str); > extern bool acpi_osi_is_win8(void); IIRC there is already similar ifdeffery, can we just move the declaration there?
On Mon, Apr 8, 2024 at 7:53 AM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > On Sat, Apr 06, 2024 at 11:33:41PM -0700, Kuppuswamy Sathyanarayanan wrote: > > The function acpi_blacklisted() is defined only when CONFIG_X86 is > > enabled. So to keep it consistent, protect its declaration with > > CONFIG_X86. > > ... > > > extern char acpi_video_backlight_string[]; > > extern long acpi_is_video_device(acpi_handle handle); > > +#ifdef CONFIG_X86 > > extern int acpi_blacklisted(void); > > +#endif > > extern void acpi_osi_setup(char *str); > > extern bool acpi_osi_is_win8(void); > > IIRC there is already similar ifdeffery, can we just move the declaration > there? There is none for CONFIG_X86. We only have some combinations or derived config checks like below: #if defined(CONFIG_X86) || defined(CONFIG_LOONGARCH) #ifdef CONFIG_X86_IO_APIC > > -- > With Best Regards, > Andy Shevchenko > >
On Mon, Apr 08, 2024 at 08:42:48AM -0700, Kuppuswamy Sathyanarayanan wrote: > On Mon, Apr 8, 2024 at 7:53 AM Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: > > On Sat, Apr 06, 2024 at 11:33:41PM -0700, Kuppuswamy Sathyanarayanan wrote: ... > > > extern char acpi_video_backlight_string[]; > > > extern long acpi_is_video_device(acpi_handle handle); > > > +#ifdef CONFIG_X86 > > > extern int acpi_blacklisted(void); > > > +#endif > > > extern void acpi_osi_setup(char *str); > > > extern bool acpi_osi_is_win8(void); > > > > IIRC there is already similar ifdeffery, can we just move the declaration > > there? > > There is none for CONFIG_X86. We only have some combinations or > derived config checks like below: > > #if defined(CONFIG_X86) || defined(CONFIG_LOONGARCH) > #ifdef CONFIG_X86_IO_APIC Okay, it seems I mixed this with acpi_bus.h.
On Sun, Apr 7, 2024 at 8:33 AM Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com> wrote: > > The function acpi_blacklisted() is defined only when CONFIG_X86 is > enabled. So to keep it consistent, protect its declaration with > CONFIG_X86. > > Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com> > --- > include/linux/acpi.h | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/include/linux/acpi.h b/include/linux/acpi.h > index 34829f2c517a..3ad6bed9eb4f 100644 > --- a/include/linux/acpi.h > +++ b/include/linux/acpi.h > @@ -421,7 +421,9 @@ extern char *wmi_get_acpi_device_uid(const char *guid); > > extern char acpi_video_backlight_string[]; > extern long acpi_is_video_device(acpi_handle handle); > +#ifdef CONFIG_X86 > extern int acpi_blacklisted(void); > +#endif > extern void acpi_osi_setup(char *str); > extern bool acpi_osi_is_win8(void); > > -- Applied (as 6.10 material), but it looks to me like this declaration could be moved away from this header file at all, as the function is only used in one place in arch-x86 code. Thanks!
On Mon, Apr 22, 2024 at 06:37:59PM +0200, Rafael J. Wysocki wrote: > On Sun, Apr 7, 2024 at 8:33 AM Kuppuswamy Sathyanarayanan > <sathyanarayanan.kuppuswamy@linux.intel.com> wrote: ... > Applied (as 6.10 material), but it looks to me like this declaration > could be moved away from this header file at all, as the function is > only used in one place in arch-x86 code. Yes, we probably may move it to asm/acpi.h for x86. I don't remember if I ever expressed that idea, but I definitely was thinking about this.
On 4/23/24 6:02 AM, Andy Shevchenko wrote: > On Mon, Apr 22, 2024 at 06:37:59PM +0200, Rafael J. Wysocki wrote: >> On Sun, Apr 7, 2024 at 8:33 AM Kuppuswamy Sathyanarayanan >> <sathyanarayanan.kuppuswamy@linux.intel.com> wrote: > ... > >> Applied (as 6.10 material), but it looks to me like this declaration >> could be moved away from this header file at all, as the function is >> only used in one place in arch-x86 code. > Yes, we probably may move it to asm/acpi.h for x86. > > I don't remember if I ever expressed that idea, but I definitely was thinking > about this. > Makes sense. I have moved it to asm/acpi.h https://lore.kernel.org/lkml/20240429040441.748479-1-sathyanarayanan.kuppuswamy@linux.intel.com/T/#u
diff --git a/include/linux/acpi.h b/include/linux/acpi.h index 34829f2c517a..3ad6bed9eb4f 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -421,7 +421,9 @@ extern char *wmi_get_acpi_device_uid(const char *guid); extern char acpi_video_backlight_string[]; extern long acpi_is_video_device(acpi_handle handle); +#ifdef CONFIG_X86 extern int acpi_blacklisted(void); +#endif extern void acpi_osi_setup(char *str); extern bool acpi_osi_is_win8(void);
The function acpi_blacklisted() is defined only when CONFIG_X86 is enabled. So to keep it consistent, protect its declaration with CONFIG_X86. Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com> --- include/linux/acpi.h | 2 ++ 1 file changed, 2 insertions(+)