[PATCH v3 0/5] ACPI / boot: Few amendments

[PATCH v3 0/5] ACPI / boot: Few amendments

[Andy Shevchenko]

This series does few amendments to architectural ACPI code related to
boot, in particularly to arch/x86/kernel/acpi/boot.c.

First two patches are amendments to satisfy compiler and static analyzer
(the order is changed from first version which had been applied; in case
  of partial update first patch is already in tree).

Third patch might be useful on platforms when debugging *PIC related
code path to see how many legacy IRQs are registered.

Fourth and fifth patches are preparation for some interesting
implementation of ACPI HW reduced platforms (note, this does not mean
it's against specification, patch 5 actually about following
specification).

Fifth patch might be subject to additional discussions.

Series has been tested on ASuS T100ta transformer
(ACPI HW reduced bit is set). No regressions found.

In v3:
- fix compilation error on ia64
- extend patch 5 and drop WIP status
- append RB tag for patch 2 (hope it stays for slightly fixed ia64 code)

In v2:
- fix function declarations in ia64 and arm64 as well (Hanjun)
- add three more patches

Andy Shevchenko (5):
  ACPI / boot: Don't define unused variables
  ACPI / boot: Correct address space of __acpi_map_table()
  ACPI / boot: Add number of legacy IRQs to debug output
  ACPI / boot: Not all platform require acpi_reduced_hw_init()
  ACPI / boot: Don't handle SCI on HW reduced platforms

 arch/arm64/kernel/acpi.c    |  4 ++--
 arch/ia64/kernel/acpi.c     |  6 +++---
 arch/x86/kernel/acpi/boot.c | 17 ++++++++++-------
 drivers/acpi/bus.c          |  2 +-
 drivers/acpi/osl.c          |  2 +-
 include/linux/acpi.h        |  7 +++++--
 6 files changed, 22 insertions(+), 16 deletions(-)

-- 
2.11.0

[PATCH v3 1/5] ACPI / boot: Don't define unused variables

[Andy Shevchenko]

Some code in acpi_parse_x2apic() conditionally compiled, though parts of
it are being used in any case. This annoys gcc.

arch/x86/kernel/acpi/boot.c: In function ‘acpi_parse_x2apic’:
arch/x86/kernel/acpi/boot.c:203:5: warning: variable ‘enabled’ set but not used [-Wunused-but-set-variable]
  u8 enabled;
     ^~~~~~~
arch/x86/kernel/acpi/boot.c:202:6: warning: variable ‘apic_id’ set but not used [-Wunused-but-set-variable]
  int apic_id;
      ^~~~~~~

Re-arrange the code to avoid compiling unused variables.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 arch/x86/kernel/acpi/boot.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index 6bb680671088..09ddb3cd627a 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -199,8 +199,10 @@ static int __init
 acpi_parse_x2apic(struct acpi_subtable_header *header, const unsigned long end)
 {
 	struct acpi_madt_local_x2apic *processor = NULL;
+#ifdef CONFIG_X86_X2APIC
 	int apic_id;
 	u8 enabled;
+#endif
 
 	processor = (struct acpi_madt_local_x2apic *)header;
 
@@ -209,9 +211,10 @@ acpi_parse_x2apic(struct acpi_subtable_header *header, const unsigned long end)
 
 	acpi_table_print_madt_entry(header);
 
+#ifdef CONFIG_X86_X2APIC
 	apic_id = processor->local_apic_id;
 	enabled = processor->lapic_flags & ACPI_MADT_ENABLED;
-#ifdef CONFIG_X86_X2APIC
+
 	/*
 	 * We need to register disabled CPU as well to permit
 	 * counting disabled CPUs. This allows us to size
-- 
2.11.0

[PATCH v3 2/5] ACPI / boot: Correct address space of __acpi_map_table()

[Andy Shevchenko]

Sparse complains about wrong address space used in __acpi_map_table()
and in __acpi_unmap_table().

arch/x86/kernel/acpi/boot.c:127:29: warning: incorrect type in return expression (different address spaces)
arch/x86/kernel/acpi/boot.c:127:29:    expected char *
arch/x86/kernel/acpi/boot.c:127:29:    got void [noderef] <asn:2>*
arch/x86/kernel/acpi/boot.c:135:23: warning: incorrect type in argument 1 (different address spaces)
arch/x86/kernel/acpi/boot.c:135:23:    expected void [noderef] <asn:2>*addr
arch/x86/kernel/acpi/boot.c:135:23:    got char *map

Correct address space to be in align of type of returned and passed
parameter.

Reviewed-by: Hanjun Guo <guohanjun@huawei.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 arch/arm64/kernel/acpi.c    | 4 ++--
 arch/ia64/kernel/acpi.c     | 6 +++---
 arch/x86/kernel/acpi/boot.c | 4 ++--
 include/linux/acpi.h        | 4 ++--
 4 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
index e25c11e727fe..b3162715ed78 100644
--- a/arch/arm64/kernel/acpi.c
+++ b/arch/arm64/kernel/acpi.c
@@ -95,7 +95,7 @@ static int __init dt_scan_depth1_nodes(unsigned long node,
  * __acpi_map_table() will be called before page_init(), so early_ioremap()
  * or early_memremap() should be called here to for ACPI table mapping.
  */
-char *__init __acpi_map_table(unsigned long phys, unsigned long size)
+void __init __iomem *__acpi_map_table(unsigned long phys, unsigned long size)
 {
 	if (!size)
 		return NULL;
@@ -103,7 +103,7 @@ char *__init __acpi_map_table(unsigned long phys, unsigned long size)
 	return early_memremap(phys, size);
 }
 
-void __init __acpi_unmap_table(char *map, unsigned long size)
+void __init __acpi_unmap_table(void __iomem *map, unsigned long size)
 {
 	if (!map || !size)
 		return;
diff --git a/arch/ia64/kernel/acpi.c b/arch/ia64/kernel/acpi.c
index 7508c306aa9e..1d29b2f8726b 100644
--- a/arch/ia64/kernel/acpi.c
+++ b/arch/ia64/kernel/acpi.c
@@ -159,12 +159,12 @@ int acpi_request_vector(u32 int_type)
 	return vector;
 }
 
-char *__init __acpi_map_table(unsigned long phys_addr, unsigned long size)
+void __init __iomem *__acpi_map_table(unsigned long phys, unsigned long size)
 {
-	return __va(phys_addr);
+	return __va(phys);
 }
 
-void __init __acpi_unmap_table(char *map, unsigned long size)
+void __init __acpi_unmap_table(void __iomem *map, unsigned long size)
 {
 }
 
diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index 09ddb3cd627a..6d5b1346268a 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -118,7 +118,7 @@ static u32 isa_irq_to_gsi[NR_IRQS_LEGACY] __read_mostly = {
  * This is just a simple wrapper around early_ioremap(),
  * with sanity checks for phys == 0 and size == 0.
  */
-char *__init __acpi_map_table(unsigned long phys, unsigned long size)
+void __init __iomem *__acpi_map_table(unsigned long phys, unsigned long size)
 {
 
 	if (!phys || !size)
@@ -127,7 +127,7 @@ char *__init __acpi_map_table(unsigned long phys, unsigned long size)
 	return early_ioremap(phys, size);
 }
 
-void __init __acpi_unmap_table(char *map, unsigned long size)
+void __init __acpi_unmap_table(void __iomem *map, unsigned long size)
 {
 	if (!map || !size)
 		return;
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index c749eef1daa1..7443af1d16e7 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -228,8 +228,8 @@ struct acpi_subtable_proc {
 	int count;
 };
 
-char * __acpi_map_table (unsigned long phys_addr, unsigned long size);
-void __acpi_unmap_table(char *map, unsigned long size);
+void __iomem *__acpi_map_table(unsigned long phys, unsigned long size);
+void __acpi_unmap_table(void __iomem *map, unsigned long size);
 int early_acpi_boot_init(void);
 int acpi_boot_init (void);
 void acpi_boot_table_init (void);
-- 
2.11.0

re: [patch v3 2/5] acpi / boot: correct address space of __acpi_map_table()

[yury norov]

Hi Andy,

> diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
> index 09ddb3cd627a..6d5b1346268a 100644
> --- a/arch/x86/kernel/acpi/boot.c
> +++ b/arch/x86/kernel/acpi/boot.c
> @@ -118,7 +118,7 @@ static u32 isa_irq_to_gsi[nr_irqs_legacy] __read_mostly = {
>   * this is just a simple wrapper around early_ioremap(),
>   * with sanity checks for phys == 0 and size == 0.
>   */
> -char *__init __acpi_map_table(unsigned long phys, unsigned long size)
> +void __init __iomem *__acpi_map_table(unsigned long phys, unsigned long size)
>  {
>  
>  	if (!phys || !size)
> @@ -127,7 +127,7 @@ char *__init __acpi_map_table(unsigned long phys, unsigned long size)
>  	return early_ioremap(phys, size);
>  }

I just found this patch in next tree, and it seems it breaks build for
arm64, and probably for ia64:

arch/arm64/kernel/acpi.c:98:14: error: conflicting types for ‘__acpi_map_table’
 char *__init __acpi_map_table(unsigned long phys, unsigned long size)
              ^~~~~~~~~~~~~~~~
in file included from arch/arm64/kernel/acpi.c:18:0:
./include/linux/acpi.h:231:15: note: previous declaration of ‘__acpi_map_table’ was here
 void __iomem *__acpi_map_table(unsigned long phys_addr, unsigned long size);
               ^~~~~~~~~~~~~~~~
arch/arm64/kernel/acpi.c:106:13: error: conflicting types for ‘__acpi_unmap_table’
 void __init __acpi_unmap_table(char *map, unsigned long size)
             ^~~~~~~~~~~~~~~~~~
in file included from arch/arm64/kernel/acpi.c:18:0:
./include/linux/acpi.h:232:6: note: previous declaration of ‘__acpi_unmap_table’ was here
 void __acpi_unmap_table(void __iomem *map, unsigned long size);
      ^~~~~~~~~~~~~~~~~~

the patch below fixes it. (Tested on arm64 only.)

Yury

>From 711e07171de68e71508695078bf6b897773a79ec Mon Sep 17 00:00:00 2001
From: Yury Norov <ynorov@caviumnetworks.com>
Date: Wed, 19 Jul 2017 16:00:02 +0300
Subject: [PATCH] ACPI / boot: fix prototypes for __acpi_{un}map_table for
 arm64 and ia64

The prototypes were changed in patch "ACPI / boot: Correct address space
of  __acpi_map_table()"

Signed-off-by: Yury Norov <ynorov@caviumnetworks.com>
---
 arch/arm64/kernel/acpi.c | 4 ++--
 arch/ia64/kernel/acpi.c  | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
index e25c11e727fe..177a871c059d 100644
--- a/arch/arm64/kernel/acpi.c
+++ b/arch/arm64/kernel/acpi.c
@@ -95,7 +95,7 @@ static int __init dt_scan_depth1_nodes(unsigned long node,
  * __acpi_map_table() will be called before page_init(), so early_ioremap()
  * or early_memremap() should be called here to for ACPI table mapping.
  */
-char *__init __acpi_map_table(unsigned long phys, unsigned long size)
+void __iomem *__acpi_map_table(unsigned long phys, unsigned long size)
 {
 	if (!size)
 		return NULL;
@@ -103,7 +103,7 @@ char *__init __acpi_map_table(unsigned long phys, unsigned long size)
 	return early_memremap(phys, size);
 }
 
-void __init __acpi_unmap_table(char *map, unsigned long size)
+void __acpi_unmap_table(void __iomem *map, unsigned long size)
 {
 	if (!map || !size)
 		return;
diff --git a/arch/ia64/kernel/acpi.c b/arch/ia64/kernel/acpi.c
index 7508c306aa9e..462a275c0479 100644
--- a/arch/ia64/kernel/acpi.c
+++ b/arch/ia64/kernel/acpi.c
@@ -159,12 +159,12 @@ int acpi_request_vector(u32 int_type)
 	return vector;
 }
 
-char *__init __acpi_map_table(unsigned long phys_addr, unsigned long size)
+void __iomem *__init __acpi_map_table(unsigned long phys_addr, unsigned long size)
 {
 	return __va(phys_addr);
 }
 
-void __init __acpi_unmap_table(char *map, unsigned long size)
+void __init __acpi_unmap_table(void __iomem *map, unsigned long size)
 {
 }
 
-- 
2.11.0

Re: [patch v3 2/5] acpi / boot: correct address space of __acpi_map_table()

[Yury Norov]

On Wed, Jul 19, 2017 at 04:06:36PM +0300, yury norov wrote:
> Hi Andy,
> 
> > diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
> > index 09ddb3cd627a..6d5b1346268a 100644
> > --- a/arch/x86/kernel/acpi/boot.c
> > +++ b/arch/x86/kernel/acpi/boot.c
> > @@ -118,7 +118,7 @@ static u32 isa_irq_to_gsi[nr_irqs_legacy] __read_mostly = {
> >   * this is just a simple wrapper around early_ioremap(),
> >   * with sanity checks for phys == 0 and size == 0.
> >   */
> > -char *__init __acpi_map_table(unsigned long phys, unsigned long size)
> > +void __init __iomem *__acpi_map_table(unsigned long phys, unsigned long size)
> >  {
> >  
> >  	if (!phys || !size)
> > @@ -127,7 +127,7 @@ char *__init __acpi_map_table(unsigned long phys, unsigned long size)
> >  	return early_ioremap(phys, size);
> >  }
> 
> I just found this patch in next tree, and it seems it breaks build for
> arm64, and probably for ia64:
> 
> arch/arm64/kernel/acpi.c:98:14: error: conflicting types for ‘__acpi_map_table’
>  char *__init __acpi_map_table(unsigned long phys, unsigned long size)
>               ^~~~~~~~~~~~~~~~
> in file included from arch/arm64/kernel/acpi.c:18:0:
> ./include/linux/acpi.h:231:15: note: previous declaration of ‘__acpi_map_table’ was here
>  void __iomem *__acpi_map_table(unsigned long phys_addr, unsigned long size);
>                ^~~~~~~~~~~~~~~~
> arch/arm64/kernel/acpi.c:106:13: error: conflicting types for ‘__acpi_unmap_table’
>  void __init __acpi_unmap_table(char *map, unsigned long size)
>              ^~~~~~~~~~~~~~~~~~
> in file included from arch/arm64/kernel/acpi.c:18:0:
> ./include/linux/acpi.h:232:6: note: previous declaration of ‘__acpi_unmap_table’ was here
>  void __acpi_unmap_table(void __iomem *map, unsigned long size);
>       ^~~~~~~~~~~~~~~~~~
> 
> the patch below fixes it. (Tested on arm64 only.)

Ah, sorry. This is correct version.

>From 484d3dd1d20f7c9494e85c4ac669909155366958 Mon Sep 17 00:00:00 2001
From: Yury Norov <ynorov@caviumnetworks.com>
Date: Wed, 19 Jul 2017 16:00:02 +0300
Subject: [PATCH] ACPI / boot: fix prototypes for __acpi_{un}map_table for
 arm64 and ia64

The prototypes were changed in patch "ACPI / boot: Correct address space
of  __acpi_map_table()"

Signed-off-by: Yury Norov <ynorov@caviumnetworks.com>
---
 arch/arm64/kernel/acpi.c | 4 ++--
 arch/ia64/kernel/acpi.c  | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
index e25c11e727fe..c7acba3e17d9 100644
--- a/arch/arm64/kernel/acpi.c
+++ b/arch/arm64/kernel/acpi.c
@@ -95,7 +95,7 @@ static int __init dt_scan_depth1_nodes(unsigned long node,
  * __acpi_map_table() will be called before page_init(), so early_ioremap()
  * or early_memremap() should be called here to for ACPI table mapping.
  */
-char *__init __acpi_map_table(unsigned long phys, unsigned long size)
+void __iomem __init *__acpi_map_table(unsigned long phys, unsigned long size)
 {
 	if (!size)
 		return NULL;
@@ -103,7 +103,7 @@ char *__init __acpi_map_table(unsigned long phys, unsigned long size)
 	return early_memremap(phys, size);
 }
 
-void __init __acpi_unmap_table(char *map, unsigned long size)
+void __init __acpi_unmap_table(void __iomem *map, unsigned long size)
 {
 	if (!map || !size)
 		return;
diff --git a/arch/ia64/kernel/acpi.c b/arch/ia64/kernel/acpi.c
index 7508c306aa9e..4df611746dab 100644
--- a/arch/ia64/kernel/acpi.c
+++ b/arch/ia64/kernel/acpi.c
@@ -159,12 +159,12 @@ int acpi_request_vector(u32 int_type)
 	return vector;
 }
 
-char *__init __acpi_map_table(unsigned long phys_addr, unsigned long size)
+void __iomem __init *__acpi_map_table(unsigned long phys_addr, unsigned long size)
 {
 	return __va(phys_addr);
 }
 
-void __init __acpi_unmap_table(char *map, unsigned long size)
+void __init __acpi_unmap_table(void __iomem *map, unsigned long size)
 {
 }
 
-- 
2.11.0

Re: [patch v3 2/5] acpi / boot: correct address space of __acpi_map_table()

[Andy Shevchenko]

On Wed, 2017-07-19 at 16:40 +0300, Yury Norov wrote:
> On Wed, Jul 19, 2017 at 04:06:36PM +0300, yury norov wrote:

> > I just found this patch in next tree, and it seems it breaks build
> > for
> > arm64, and probably for ia64:

I think you found previous version (v1). _This_ patch (v3 of it)
actually solves that.

> > the patch below fixes it. (Tested on arm64 only.)
> 
> Ah, sorry. This is correct version.

Thank you, I think Rafael might rebase with clean version (v3) his tree
which will fix this.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

[PATCH v3 3/5] ACPI / boot: Add number of legacy IRQs to debug output

[Andy Shevchenko]

Sometimes it's useful to have when mp_config_acpi_legacy_irqs() is called.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 arch/x86/kernel/acpi/boot.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index 6d5b1346268a..0186d3bae610 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -1078,7 +1078,7 @@ static void __init mp_config_acpi_legacy_irqs(void)
 	mp_bus_id_to_type[MP_ISA_BUS] = MP_BUS_ISA;
 #endif
 	set_bit(MP_ISA_BUS, mp_bus_not_pci);
-	pr_debug("Bus #%d is ISA\n", MP_ISA_BUS);
+	pr_debug("Bus #%d is ISA (nIRQs: %d)\n", MP_ISA_BUS, nr_legacy_irqs());
 
 	/*
 	 * Use the default configuration for the IRQs 0-15.  Unless
-- 
2.11.0

[PATCH v3 4/5] ACPI / boot: Not all platform require acpi_reduced_hw_init()

[Andy Shevchenko]

Some platform might take care of legacy devices on theirs own.
Let's allow them to do that by exporting a weak function.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 arch/x86/kernel/acpi/boot.c | 2 +-
 include/linux/acpi.h        | 3 +++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index 0186d3bae610..71c0feae60a4 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -1348,7 +1348,7 @@ static int __init dmi_ignore_irq0_timer_override(const struct dmi_system_id *d)
  *
  * We initialize the Hardware-reduced ACPI model here:
  */
-static void __init acpi_reduced_hw_init(void)
+void __init __weak acpi_reduced_hw_init(void)
 {
 	if (acpi_gbl_reduced_hardware) {
 		/*
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 7443af1d16e7..5a19b2090db1 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -230,6 +230,8 @@ struct acpi_subtable_proc {
 
 void __iomem *__acpi_map_table(unsigned long phys, unsigned long size);
 void __acpi_unmap_table(void __iomem *map, unsigned long size);
+
+void acpi_reduced_hw_init(void);
 int early_acpi_boot_init(void);
 int acpi_boot_init (void);
 void acpi_boot_table_init (void);
@@ -682,6 +684,7 @@ static inline struct device *acpi_get_first_physical_node(struct acpi_device *ad
 static inline void acpi_early_init(void) { }
 static inline void acpi_subsystem_init(void) { }
 
+static inline void acpi_reduced_hw_init(void) { }
 static inline int early_acpi_boot_init(void)
 {
 	return 0;
-- 
2.11.0

Re: [PATCH v3 4/5] ACPI / boot: Not all platform require acpi_reduced_hw_init()

[Rafael J. Wysocki]

On Tuesday, July 18, 2017 06:04:19 PM Andy Shevchenko wrote:
> Some platform might take care of legacy devices on theirs own.
> Let's allow them to do that by exporting a weak function.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

I'd rather do it at the time when acpi_reduced_hw_init() actually needs to be
overridden by at least one platform.

Thanks,
Rafael

Re: [PATCH v3 4/5] ACPI / boot: Not all platform require acpi_reduced_hw_init()

[Andy Shevchenko]

On Sat, Jul 22, 2017 at 1:25 AM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> On Tuesday, July 18, 2017 06:04:19 PM Andy Shevchenko wrote:
>> Some platform might take care of legacy devices on theirs own.
>> Let's allow them to do that by exporting a weak function.
>>
>> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>
> I'd rather do it at the time when acpi_reduced_hw_init() actually needs to be
> overridden by at least one platform.

Do you mean as folded into some other patch or just as a preparatory
patch in some future series?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 4/5] ACPI / boot: Not all platform require acpi_reduced_hw_init()

[Rafael J. Wysocki]

On Saturday, July 22, 2017 04:53:52 AM Andy Shevchenko wrote:
> On Sat, Jul 22, 2017 at 1:25 AM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> > On Tuesday, July 18, 2017 06:04:19 PM Andy Shevchenko wrote:
> >> Some platform might take care of legacy devices on theirs own.
> >> Let's allow them to do that by exporting a weak function.
> >>
> >> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> >
> > I'd rather do it at the time when acpi_reduced_hw_init() actually needs to be
> > overridden by at least one platform.
> 
> Do you mean as folded into some other patch or just as a preparatory
> patch in some future series?
> 
> 

Any of the above would work for me.

Thanks,
Rafael

Re: [PATCH v3 4/5] ACPI / boot: Not all platform require acpi_reduced_hw_init()

[Andy Shevchenko]

On Sun, Jul 23, 2017 at 1:02 AM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> On Saturday, July 22, 2017 04:53:52 AM Andy Shevchenko wrote:
>> On Sat, Jul 22, 2017 at 1:25 AM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
>> > On Tuesday, July 18, 2017 06:04:19 PM Andy Shevchenko wrote:

>> > I'd rather do it at the time when acpi_reduced_hw_init() actually needs to be
>> > overridden by at least one platform.
>>
>> Do you mean as folded into some other patch or just as a preparatory
>> patch in some future series?

> Any of the above would work for me.

Logically it should be a separate change as I can see (I have already
locally prepared patches for that one platform).

Thanks for review.

P.S. Are you going to apply first 3 then from this series?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 4/5] ACPI / boot: Not all platform require acpi_reduced_hw_init()

[Rafael J. Wysocki]

On Sun, Jul 23, 2017 at 12:13 AM, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Sun, Jul 23, 2017 at 1:02 AM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
>> On Saturday, July 22, 2017 04:53:52 AM Andy Shevchenko wrote:
>>> On Sat, Jul 22, 2017 at 1:25 AM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
>>> > On Tuesday, July 18, 2017 06:04:19 PM Andy Shevchenko wrote:
>
>>> > I'd rather do it at the time when acpi_reduced_hw_init() actually needs to be
>>> > overridden by at least one platform.
>>>
>>> Do you mean as folded into some other patch or just as a preparatory
>>> patch in some future series?
>
>> Any of the above would work for me.
>
> Logically it should be a separate change as I can see (I have already
> locally prepared patches for that one platform).
>
> Thanks for review.
>
> P.S. Are you going to apply first 3 then from this series?

Yes, I am.

Re: [PATCH v3 4/5] ACPI / boot: Not all platform require acpi_reduced_hw_init()

[Rafael J. Wysocki]

On Sunday, July 23, 2017 12:14:27 AM Rafael J. Wysocki wrote:
> On Sun, Jul 23, 2017 at 12:13 AM, Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> > On Sun, Jul 23, 2017 at 1:02 AM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> >> On Saturday, July 22, 2017 04:53:52 AM Andy Shevchenko wrote:
> >>> On Sat, Jul 22, 2017 at 1:25 AM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> >>> > On Tuesday, July 18, 2017 06:04:19 PM Andy Shevchenko wrote:
> >
> >>> > I'd rather do it at the time when acpi_reduced_hw_init() actually needs to be
> >>> > overridden by at least one platform.
> >>>
> >>> Do you mean as folded into some other patch or just as a preparatory
> >>> patch in some future series?
> >
> >> Any of the above would work for me.
> >
> > Logically it should be a separate change as I can see (I have already
> > locally prepared patches for that one platform).
> >
> > Thanks for review.
> >
> > P.S. Are you going to apply first 3 then from this series?
> 
> Yes, I am.

So applied now, thanks!

[PATCH v3 5/5] ACPI / boot: Don't handle SCI on HW reduced platforms

[Andy Shevchenko]

As per note in 5.2.9 Fixed ACPI Description Table (FADT) chapter of ACPI
specification OSPM will ignore fields related to the ACPI HW register
interface, one of which is SCI_INT.

Follow the spec and ignore any configuration done for interrupt line
defined by SCI_INT if FADT specifies HW reduced platform.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 arch/x86/kernel/acpi/boot.c | 4 ++--
 drivers/acpi/bus.c          | 2 +-
 drivers/acpi/osl.c          | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index 71c0feae60a4..c767efa53ee0 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -489,7 +489,7 @@ acpi_parse_int_src_ovr(struct acpi_subtable_header * header,
 
 	acpi_table_print_madt_entry(header);
 
-	if (intsrc->source_irq == acpi_gbl_FADT.sci_interrupt) {
+	if (intsrc->source_irq == acpi_gbl_FADT.sci_interrupt && !acpi_gbl_reduced_hardware) {
 		acpi_sci_ioapic_setup(intsrc->source_irq,
 				      intsrc->inti_flags & ACPI_MADT_POLARITY_MASK,
 				      (intsrc->inti_flags & ACPI_MADT_TRIGGER_MASK) >> 2,
@@ -1184,7 +1184,7 @@ static int __init acpi_parse_madt_ioapic_entries(void)
 	 * If BIOS did not supply an INT_SRC_OVR for the SCI
 	 * pretend we got one so we can set the SCI flags.
 	 */
-	if (!acpi_sci_override_gsi)
+	if (!acpi_sci_override_gsi && !acpi_gbl_reduced_hardware)
 		acpi_sci_ioapic_setup(acpi_gbl_FADT.sci_interrupt, 0, 0,
 				      acpi_gbl_FADT.sci_interrupt);
 
diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index af74b420ec83..1f3dde3bc8f8 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -1035,7 +1035,7 @@ void __init acpi_early_init(void)
 	}
 
 #ifdef CONFIG_X86
-	if (!acpi_ioapic) {
+	if (!acpi_ioapic && !acpi_gbl_reduced_hardware) {
 		/* compatible (0) means level (3) */
 		if (!(acpi_sci_flags & ACPI_MADT_TRIGGER_MASK)) {
 			acpi_sci_flags &= ~ACPI_MADT_TRIGGER_MASK;
diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index db78d353bab1..4d84118a000b 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -553,7 +553,7 @@ acpi_os_install_interrupt_handler(u32 gsi, acpi_osd_handler handler,
 	 * ACPI interrupts different from the SCI in our copy of the FADT are
 	 * not supported.
 	 */
-	if (gsi != acpi_gbl_FADT.sci_interrupt)
+	if (gsi != acpi_gbl_FADT.sci_interrupt || acpi_gbl_reduced_hardware)
 		return AE_BAD_PARAMETER;
 
 	if (acpi_irq_handler)
-- 
2.11.0

Re: [PATCH v3 5/5] ACPI / boot: Don't handle SCI on HW reduced platforms

[Rafael J. Wysocki]

On Tuesday, July 18, 2017 06:04:20 PM Andy Shevchenko wrote:
> As per note in 5.2.9 Fixed ACPI Description Table (FADT) chapter of ACPI
> specification OSPM will ignore fields related to the ACPI HW register
> interface, one of which is SCI_INT.
> 
> Follow the spec and ignore any configuration done for interrupt line
> defined by SCI_INT if FADT specifies HW reduced platform.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Can we invalidate acpi_gbl_FADT.sci_interrupt somehow for
acpi_gbl_reduced_hardware?

The checks added below look somewhat arbitrary and it would be good to
provide some argumentation on why everything is covered by them as needed.

> ---
>  arch/x86/kernel/acpi/boot.c | 4 ++--
>  drivers/acpi/bus.c          | 2 +-
>  drivers/acpi/osl.c          | 2 +-
>  3 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
> index 71c0feae60a4..c767efa53ee0 100644
> --- a/arch/x86/kernel/acpi/boot.c
> +++ b/arch/x86/kernel/acpi/boot.c
> @@ -489,7 +489,7 @@ acpi_parse_int_src_ovr(struct acpi_subtable_header * header,
>  
>  	acpi_table_print_madt_entry(header);
>  
> -	if (intsrc->source_irq == acpi_gbl_FADT.sci_interrupt) {
> +	if (intsrc->source_irq == acpi_gbl_FADT.sci_interrupt && !acpi_gbl_reduced_hardware) {
>  		acpi_sci_ioapic_setup(intsrc->source_irq,
>  				      intsrc->inti_flags & ACPI_MADT_POLARITY_MASK,
>  				      (intsrc->inti_flags & ACPI_MADT_TRIGGER_MASK) >> 2,
> @@ -1184,7 +1184,7 @@ static int __init acpi_parse_madt_ioapic_entries(void)
>  	 * If BIOS did not supply an INT_SRC_OVR for the SCI
>  	 * pretend we got one so we can set the SCI flags.
>  	 */
> -	if (!acpi_sci_override_gsi)
> +	if (!acpi_sci_override_gsi && !acpi_gbl_reduced_hardware)
>  		acpi_sci_ioapic_setup(acpi_gbl_FADT.sci_interrupt, 0, 0,
>  				      acpi_gbl_FADT.sci_interrupt);
>  
> diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> index af74b420ec83..1f3dde3bc8f8 100644
> --- a/drivers/acpi/bus.c
> +++ b/drivers/acpi/bus.c
> @@ -1035,7 +1035,7 @@ void __init acpi_early_init(void)
>  	}
>  
>  #ifdef CONFIG_X86
> -	if (!acpi_ioapic) {
> +	if (!acpi_ioapic && !acpi_gbl_reduced_hardware) {
>  		/* compatible (0) means level (3) */
>  		if (!(acpi_sci_flags & ACPI_MADT_TRIGGER_MASK)) {
>  			acpi_sci_flags &= ~ACPI_MADT_TRIGGER_MASK;
> diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
> index db78d353bab1..4d84118a000b 100644
> --- a/drivers/acpi/osl.c
> +++ b/drivers/acpi/osl.c
> @@ -553,7 +553,7 @@ acpi_os_install_interrupt_handler(u32 gsi, acpi_osd_handler handler,
>  	 * ACPI interrupts different from the SCI in our copy of the FADT are
>  	 * not supported.
>  	 */
> -	if (gsi != acpi_gbl_FADT.sci_interrupt)
> +	if (gsi != acpi_gbl_FADT.sci_interrupt || acpi_gbl_reduced_hardware)
>  		return AE_BAD_PARAMETER;
>  
>  	if (acpi_irq_handler)
> 

Re: [PATCH v3 5/5] ACPI / boot: Don't handle SCI on HW reduced platforms

[Andy Shevchenko]

On Sat, Jul 22, 2017 at 1:28 AM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> On Tuesday, July 18, 2017 06:04:20 PM Andy Shevchenko wrote:
>> As per note in 5.2.9 Fixed ACPI Description Table (FADT) chapter of ACPI
>> specification OSPM will ignore fields related to the ACPI HW register
>> interface, one of which is SCI_INT.
>>
>> Follow the spec and ignore any configuration done for interrupt line
>> defined by SCI_INT if FADT specifies HW reduced platform.

> Can we invalidate acpi_gbl_FADT.sci_interrupt somehow for
> acpi_gbl_reduced_hardware?

At some point there INVALID_ACPI_IRQ is used. There is also helper to
check it acpi_sci_irq_valid().

So, we can use that helper instead.

> The checks added below look somewhat arbitrary and it would be good to
> provide some argumentation on why everything is covered by them as needed.


-- 
With Best Regards,
Andy Shevchenko