[PATCH v3 0/9] Start deprecating _OSI on new architectures

[PATCH v3 0/9] Start deprecating _OSI on new architectures

[al.stone]

From: Al Stone <al.stone@linaro.org>

The use of the ACPI _OSI method in Linux has a long and sordid history.
Instead of perpetuating past complications on new architectures, the 
consensus amongst those writing the ACPI specification and those using
it seems to be to ultimately deprecate the use of _OSI.  A change request
has been submitted (but not yet decided upon) to modify the ACPI spec
accordingly.

In the meantime, these patches rearrange the implementation of _OSI so
that it can be deprecated, or ultimately removed completely, on at least
arm64 platforms.  This is done by separating out the _OSI implementation
and moving it into a new file.  For x86 and ia64, there is no change in
functionality.  But, this allows us to provide a separate implementation
of _OSI for arm64 that generates a warning that it has been deprecated, 
and always returns false; i.e., that the capability being queried for, 
whether OS name or functionality, is not supported.  Patches 0005
through 0008 provide these changes; the first four patches are solely
cleanup to the file drivers/acpi/osl.c made so that checkpatch will not
complain.

The final patch changes the default value for the _OS_ method for arm64
only.  Since there is no need to pretend to be older versions of Windows,
or any other OS at all, the _OS_ method will return "Linux" on arm64.
One can still use the acpi_os_name kernel parameter if there is a need
to use some other value.

The first seven patches do not depend on arm64 support for ACPI and could
be used independently.  The last two patches make much more sense when used
in conjunction with Hanjun's patches for ACPI 5.1 on arm64 [0].  In fact,
patch 0008 cannot be applied without [0].

These have been through some simple testing on two different x86 laptops,
and all seems well (Lenovo t440s and t430s ThinkPads).  The arm64 code has
been tested on an AMD Seattle system.  Unfortunately, for ia64, all I could
do was cross-compile the code; I have no access to hardware to test on.

NB: the first four patches are solely cleanup to drivers/acpi/osl.c based
on the results from checkpatch and can be treated independently.  However,
the remainder of the patch set assumes this cleanup has been done.  There
are some checkpatch warnings still remaining in osl.c -- specifically about
use of volatiles and one line of 81 characters -- that these patches 
intentionally do not correct as they do not appear to need correcting.

Changes in v3:
  -- add in cleanup to osl.c based on checkpatch output
  -- put arch-specific _OSI implementation in the correct place (arch/*)
  -- modify CONFIG item names and make them so they are not user selectable
  -- get rid of the BLACKLIST config item; it wasn't really needed.

Changes in v2:
  -- significant simplification based on Rafael's comments
  -- ACPI spec change request has now been submitted


[0] https://lkml.org/lkml/2015/2/2/261


Al Stone (9):
  ACPI: fix all errors reported by cleanpatch.pl in osl.c
  ACPI: clear up warnings on use of printk reported by checkpatch.pl
  ACPI: clean up checkpatch warnings for various bits of syntax
  ACPI: clean up checkpatch warnings for items with possible semantic
    value
  ACPI: move acpi_os_handler() so it can be made arch-dependent later
  ACPI: move _OSI support functions to allow arch-dependent
    implementation
  ACPI: enable arch-specific compilation for _OSI and the blacklist
  ACPI: arm64: use an arch-specific ACPI _OSI method and ACPI blacklist
  ACPI: arm64: use "Linux" as ACPI_OS_NAME for _OS on arm64

 arch/arm64/Kconfig                 |   6 +
 arch/arm64/kernel/Makefile         |   2 +-
 arch/arm64/kernel/acpi-blacklist.c |  20 ++
 arch/arm64/kernel/acpi-osi.c       |  25 +++
 drivers/acpi/Kconfig               |   3 +
 drivers/acpi/Makefile              |   5 +
 drivers/acpi/osi.c                 | 245 +++++++++++++++++++++++
 drivers/acpi/osl.c                 | 387 ++++++++-----------------------------
 include/acpi/acconfig.h            |   2 +
 include/acpi/platform/aclinux.h    |   4 +
 include/linux/acpi.h               |  14 +-
 11 files changed, 406 insertions(+), 307 deletions(-)
 create mode 100644 arch/arm64/kernel/acpi-blacklist.c
 create mode 100644 arch/arm64/kernel/acpi-osi.c
 create mode 100644 drivers/acpi/osi.c

-- 
2.1.0

[PATCH v3 1/9] ACPI: fix all errors reported by cleanpatch.pl in osl.c

[al.stone]

From: Al Stone <al.stone@linaro.org>

In preparation for later splitting out some of the arch-dependent code from
osl.c, clean up the errors reported by checkpatch.pl.  They fell into these
classes:

   -- remove the FSF address from the GPL notice
   -- "foo * bar" should be "foo *bar" (and the ** variation of same)
   -- a return is not a function, so parentheses are not required.

Signed-off-by: Al Stone <al.stone@linaro.org>
---
 drivers/acpi/osl.c | 38 +++++++++++++++++---------------------
 1 file changed, 17 insertions(+), 21 deletions(-)

diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index 39748bb..1dc3a3b 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -19,10 +19,6 @@
  *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  *  GNU General Public License for more details.
  *
- *  You should have received a copy of the GNU General Public License
- *  along with this program; if not, write to the Free Software
- *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
- *
  * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  *
  */
@@ -526,7 +522,7 @@ EXPORT_SYMBOL(acpi_os_unmap_generic_address);
 
 #ifdef ACPI_FUTURE_USAGE
 acpi_status
-acpi_os_get_physical_address(void *virt, acpi_physical_address * phys)
+acpi_os_get_physical_address(void *virt, acpi_physical_address *phys)
 {
 	if (!phys || !virt)
 		return AE_BAD_PARAMETER;
@@ -543,7 +539,7 @@ static char acpi_os_name[ACPI_MAX_OVERRIDE_LEN];
 
 acpi_status
 acpi_os_predefined_override(const struct acpi_predefined_names *init_val,
-			    acpi_string * new_val)
+			    acpi_string *new_val)
 {
 	if (!init_val || !new_val)
 		return AE_BAD_PARAMETER;
@@ -714,8 +710,8 @@ static void acpi_table_taint(struct acpi_table_header *table)
 
 
 acpi_status
-acpi_os_table_override(struct acpi_table_header * existing_table,
-		       struct acpi_table_header ** new_table)
+acpi_os_table_override(struct acpi_table_header *existing_table,
+		       struct acpi_table_header **new_table)
 {
 	if (!existing_table || !new_table)
 		return AE_BAD_PARAMETER;
@@ -889,7 +885,7 @@ u64 acpi_os_get_timer(void)
 	return time_ns;
 }
 
-acpi_status acpi_os_read_port(acpi_io_address port, u32 * value, u32 width)
+acpi_status acpi_os_read_port(acpi_io_address port, u32 *value, u32 width)
 {
 	u32 dummy;
 
@@ -1046,7 +1042,7 @@ acpi_os_write_memory(acpi_physical_address phys_addr, u64 value, u32 width)
 }
 
 acpi_status
-acpi_os_read_pci_configuration(struct acpi_pci_id * pci_id, u32 reg,
+acpi_os_read_pci_configuration(struct acpi_pci_id *pci_id, u32 reg,
 			       u64 *value, u32 width)
 {
 	int result, size;
@@ -1074,11 +1070,11 @@ acpi_os_read_pci_configuration(struct acpi_pci_id * pci_id, u32 reg,
 				reg, size, &value32);
 	*value = value32;
 
-	return (result ? AE_ERROR : AE_OK);
+	return result ? AE_ERROR : AE_OK;
 }
 
 acpi_status
-acpi_os_write_pci_configuration(struct acpi_pci_id * pci_id, u32 reg,
+acpi_os_write_pci_configuration(struct acpi_pci_id *pci_id, u32 reg,
 				u64 value, u32 width)
 {
 	int result, size;
@@ -1101,7 +1097,7 @@ acpi_os_write_pci_configuration(struct acpi_pci_id * pci_id, u32 reg,
 				PCI_DEVFN(pci_id->device, pci_id->function),
 				reg, size, value);
 
-	return (result ? AE_ERROR : AE_OK);
+	return result ? AE_ERROR : AE_OK;
 }
 
 static void acpi_os_execute_deferred(struct work_struct *work)
@@ -1247,7 +1243,7 @@ bool acpi_queue_hotplug_work(struct work_struct *work)
 }
 
 acpi_status
-acpi_os_create_semaphore(u32 max_units, u32 initial_units, acpi_handle * handle)
+acpi_os_create_semaphore(u32 max_units, u32 initial_units, acpi_handle *handle)
 {
 	struct semaphore *sem = NULL;
 
@@ -1735,7 +1731,7 @@ void acpi_os_release_lock(acpi_spinlock lockp, acpi_cpu_flags flags)
  ******************************************************************************/
 
 acpi_status
-acpi_os_create_cache(char *name, u16 size, u16 depth, acpi_cache_t ** cache)
+acpi_os_create_cache(char *name, u16 size, u16 depth, acpi_cache_t **cache)
 {
 	*cache = kmem_cache_create(name, size, 0, 0, NULL);
 	if (*cache == NULL)
@@ -1756,10 +1752,10 @@ acpi_os_create_cache(char *name, u16 size, u16 depth, acpi_cache_t ** cache)
  *
  ******************************************************************************/
 
-acpi_status acpi_os_purge_cache(acpi_cache_t * cache)
+acpi_status acpi_os_purge_cache(acpi_cache_t *cache)
 {
 	kmem_cache_shrink(cache);
-	return (AE_OK);
+	return AE_OK;
 }
 
 /*******************************************************************************
@@ -1775,10 +1771,10 @@ acpi_status acpi_os_purge_cache(acpi_cache_t * cache)
  *
  ******************************************************************************/
 
-acpi_status acpi_os_delete_cache(acpi_cache_t * cache)
+acpi_status acpi_os_delete_cache(acpi_cache_t *cache)
 {
 	kmem_cache_destroy(cache);
-	return (AE_OK);
+	return AE_OK;
 }
 
 /*******************************************************************************
@@ -1795,10 +1791,10 @@ acpi_status acpi_os_delete_cache(acpi_cache_t * cache)
  *
  ******************************************************************************/
 
-acpi_status acpi_os_release_object(acpi_cache_t * cache, void *object)
+acpi_status acpi_os_release_object(acpi_cache_t *cache, void *object)
 {
 	kmem_cache_free(cache, object);
-	return (AE_OK);
+	return AE_OK;
 }
 #endif
 
-- 
2.1.0

[PATCH v3 2/9] ACPI: clear up warnings on use of printk reported by checkpatch.pl

[al.stone]

From: Al Stone <al.stone@linaro.org>

In preparation for later splitting out some of the arch-dependent code from
osl.c, clear up all the warnings reported by checkpatch.pl where pr_* should
be used instead of printk(KERN_* ...).

Signed-off-by: Al Stone <al.stone@linaro.org>
---
 drivers/acpi/osl.c | 46 +++++++++++++++++++---------------------------
 1 file changed, 19 insertions(+), 27 deletions(-)

diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index 1dc3a3b..865317c 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -141,7 +141,7 @@ static u32 acpi_osi_handler(acpi_string interface, u32 supported)
 {
 	if (!strcmp("Linux", interface)) {
 
-		printk_once(KERN_NOTICE FW_BUG PREFIX
+		pr_notice_once(FW_BUG PREFIX
 			"BIOS _OSI(Linux) query %s%s\n",
 			osi_linux.enable ? "honored" : "ignored",
 			osi_linux.cmdline ? " via cmdline" :
@@ -230,10 +230,10 @@ void acpi_os_vprintf(const char *fmt, va_list args)
 	if (acpi_in_debugger) {
 		kdb_printf("%s", buffer);
 	} else {
-		printk(KERN_CONT "%s", buffer);
+		pr_cont("%s", buffer);
 	}
 #else
-	printk(KERN_CONT "%s", buffer);
+	pr_cont("%s", buffer);
 #endif
 }
 
@@ -261,8 +261,7 @@ acpi_physical_address __init acpi_os_get_root_pointer(void)
 		else if (efi.acpi != EFI_INVALID_TABLE_ADDR)
 			return efi.acpi;
 		else {
-			printk(KERN_ERR PREFIX
-			       "System description tables not found\n");
+			pr_err(PREFIX "System description tables not found\n");
 			return 0;
 		}
 	} else if (IS_ENABLED(CONFIG_ACPI_LEGACY_TABLES_LOOKUP)) {
@@ -372,7 +371,7 @@ acpi_os_map_iomem(acpi_physical_address phys, acpi_size size)
 	acpi_size pg_sz;
 
 	if (phys > ULONG_MAX) {
-		printk(KERN_ERR PREFIX "Cannot map memory that high\n");
+		pr_err(PREFIX "Cannot map memory that high\n");
 		return NULL;
 	}
 
@@ -546,8 +545,8 @@ acpi_os_predefined_override(const struct acpi_predefined_names *init_val,
 
 	*new_val = NULL;
 	if (!memcmp(init_val->name, "_OS_", 4) && strlen(acpi_os_name)) {
-		printk(KERN_INFO PREFIX "Overriding _OS definition to '%s'\n",
-		       acpi_os_name);
+		pr_info(PREFIX "Overriding _OS definition to '%s'\n",
+			acpi_os_name);
 		*new_val = acpi_os_name;
 	}
 
@@ -824,15 +823,14 @@ acpi_os_install_interrupt_handler(u32 gsi, acpi_osd_handler handler,
 		return AE_ALREADY_ACQUIRED;
 
 	if (acpi_gsi_to_irq(gsi, &irq) < 0) {
-		printk(KERN_ERR PREFIX "SCI (ACPI GSI %d) not registered\n",
-		       gsi);
+		pr_err(PREFIX "SCI (ACPI GSI %d) not registered\n", gsi);
 		return AE_OK;
 	}
 
 	acpi_irq_handler = handler;
 	acpi_irq_context = context;
 	if (request_irq(irq, acpi_irq, IRQF_SHARED, "acpi", acpi_irq)) {
-		printk(KERN_ERR PREFIX "SCI (IRQ%d) allocation failed\n", irq);
+		pr_err(PREFIX "SCI (IRQ%d) allocation failed\n", irq);
 		acpi_irq_handler = NULL;
 		return AE_NOT_ACQUIRED;
 	}
@@ -1173,8 +1171,7 @@ acpi_status acpi_os_execute(acpi_execute_type type,
 	ret = queue_work_on(0, queue, &dpc->work);
 
 	if (!ret) {
-		printk(KERN_ERR PREFIX
-			  "Call to queue_work() failed.\n");
+		pr_err(PREFIX "Call to queue_work() failed.\n");
 		status = AE_ERROR;
 		kfree(dpc);
 	}
@@ -1371,7 +1368,7 @@ acpi_status acpi_os_signal(u32 function, void *info)
 {
 	switch (function) {
 	case ACPI_SIGNAL_FATAL:
-		printk(KERN_ERR PREFIX "Fatal opcode executed\n");
+		pr_err(PREFIX "Fatal opcode executed\n");
 		break;
 	case ACPI_SIGNAL_BREAKPOINT:
 		/*
@@ -1440,7 +1437,7 @@ void __init acpi_osi_setup(char *str)
 		return;
 
 	if (str == NULL || *str == '\0') {
-		printk(KERN_INFO PREFIX "_OSI method disabled\n");
+		pr_info(PREFIX "_OSI method disabled\n");
 		acpi_gbl_create_osi_method = FALSE;
 		return;
 	}
@@ -1498,7 +1495,7 @@ static void __init acpi_cmdline_osi_linux(unsigned int enable)
 
 void __init acpi_dmi_osi_linux(int enable, const struct dmi_system_id *d)
 {
-	printk(KERN_NOTICE PREFIX "DMI detected: %s\n", d->ident);
+	pr_notice(PREFIX "DMI detected: %s\n", d->ident);
 
 	if (enable == -1)
 		return;
@@ -1527,7 +1524,7 @@ static void __init acpi_osi_setup_late(void)
 		status = acpi_update_interfaces(ACPI_DISABLE_ALL_VENDOR_STRINGS);
 
 		if (ACPI_SUCCESS(status))
-			printk(KERN_INFO PREFIX "Disabled all _OSI OS vendors\n");
+			pr_info(PREFIX "Disabled all _OSI OS vendors\n");
 	}
 
 	for (i = 0; i < OSI_STRING_ENTRIES_MAX; i++) {
@@ -1540,12 +1537,12 @@ static void __init acpi_osi_setup_late(void)
 			status = acpi_install_interface(str);
 
 			if (ACPI_SUCCESS(status))
-				printk(KERN_INFO PREFIX "Added _OSI(%s)\n", str);
+				pr_info(PREFIX "Added _OSI(%s)\n", str);
 		} else {
 			status = acpi_remove_interface(str);
 
 			if (ACPI_SUCCESS(status))
-				printk(KERN_INFO PREFIX "Deleted _OSI(%s)\n", str);
+				pr_info(PREFIX "Deleted _OSI(%s)\n", str);
 		}
 	}
 }
@@ -1646,12 +1643,8 @@ int acpi_check_resource_conflict(const struct resource *res)
 	if (clash) {
 		if (acpi_enforce_resources != ENFORCE_RESOURCES_NO) {
 			if (acpi_enforce_resources == ENFORCE_RESOURCES_LAX)
-				printk(KERN_NOTICE "ACPI: This conflict may"
-				       " cause random problems and system"
-				       " instability\n");
-			printk(KERN_INFO "ACPI: If an ACPI driver is available"
-			       " for this device, you should use it instead of"
-			       " the native driver\n");
+				pr_notice("ACPI: This conflict may cause random problems and system instability\n");
+			pr_info("ACPI: If an ACPI driver is available for this device, you should use it instead of the native driver\n");
 		}
 		if (acpi_enforce_resources == ENFORCE_RESOURCES_STRICT)
 			return -EBUSY;
@@ -1810,8 +1803,7 @@ early_param("acpi_no_static_ssdt", acpi_no_static_ssdt_setup);
 
 static int __init acpi_disable_return_repair(char *s)
 {
-	printk(KERN_NOTICE PREFIX
-	       "ACPI: Predefined validation mechanism disabled\n");
+	pr_notice(PREFIX "ACPI: Predefined validation mechanism disabled\n");
 	acpi_gbl_disable_auto_repair = TRUE;
 
 	return 1;
-- 
2.1.0

[PATCH v3 3/9] ACPI: clean up checkpatch warnings for various bits of syntax

[al.stone]

From: Al Stone <al.stone@linaro.org>

In preparation for later splitting out some of the arch-dependent code from
osl.c, clean up a bunch of warnings for odd bits of syntax:

   -- remove CVS keyword markers
   -- remove a space from between a function name and an opening parenthesis
   -- clean up all but one line > 80 characters (one just looks silly if you
      make it less than 80)
   -- add blank lines after declarations in functions
   -- remove extraneous braces ({})

Signed-off-by: Al Stone <al.stone@linaro.org>
---
 drivers/acpi/osl.c | 75 ++++++++++++++++++++++++++++++++----------------------
 1 file changed, 44 insertions(+), 31 deletions(-)

diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index 865317c..46317ff 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -1,5 +1,5 @@
 /*
- *  acpi_osl.c - OS-dependent functions ($Revision: 83 $)
+ *  acpi_osl.c - OS-dependent functions
  *
  *  Copyright (C) 2000       Andrew Henroid
  *  Copyright (C) 2001, 2002 Andy Grover <andrew.grover@intel.com>
@@ -161,7 +161,7 @@ static u32 acpi_osi_handler(acpi_string interface, u32 supported)
 	return supported;
 }
 
-static void __init acpi_request_region (struct acpi_generic_address *gas,
+static void __init acpi_request_region(struct acpi_generic_address *gas,
 	unsigned int length, char *desc)
 {
 	u64 addr;
@@ -180,33 +180,41 @@ static void __init acpi_request_region (struct acpi_generic_address *gas,
 
 static int __init acpi_reserve_resources(void)
 {
-	acpi_request_region(&acpi_gbl_FADT.xpm1a_event_block, acpi_gbl_FADT.pm1_event_length,
-		"ACPI PM1a_EVT_BLK");
+	acpi_request_region(&acpi_gbl_FADT.xpm1a_event_block,
+			    acpi_gbl_FADT.pm1_event_length,
+			    "ACPI PM1a_EVT_BLK");
 
-	acpi_request_region(&acpi_gbl_FADT.xpm1b_event_block, acpi_gbl_FADT.pm1_event_length,
-		"ACPI PM1b_EVT_BLK");
+	acpi_request_region(&acpi_gbl_FADT.xpm1b_event_block,
+			    acpi_gbl_FADT.pm1_event_length,
+			    "ACPI PM1b_EVT_BLK");
 
-	acpi_request_region(&acpi_gbl_FADT.xpm1a_control_block, acpi_gbl_FADT.pm1_control_length,
-		"ACPI PM1a_CNT_BLK");
+	acpi_request_region(&acpi_gbl_FADT.xpm1a_control_block,
+			    acpi_gbl_FADT.pm1_control_length,
+			    "ACPI PM1a_CNT_BLK");
 
-	acpi_request_region(&acpi_gbl_FADT.xpm1b_control_block, acpi_gbl_FADT.pm1_control_length,
-		"ACPI PM1b_CNT_BLK");
+	acpi_request_region(&acpi_gbl_FADT.xpm1b_control_block,
+			    acpi_gbl_FADT.pm1_control_length,
+			    "ACPI PM1b_CNT_BLK");
 
 	if (acpi_gbl_FADT.pm_timer_length == 4)
-		acpi_request_region(&acpi_gbl_FADT.xpm_timer_block, 4, "ACPI PM_TMR");
+		acpi_request_region(&acpi_gbl_FADT.xpm_timer_block, 4,
+				    "ACPI PM_TMR");
 
-	acpi_request_region(&acpi_gbl_FADT.xpm2_control_block, acpi_gbl_FADT.pm2_control_length,
-		"ACPI PM2_CNT_BLK");
+	acpi_request_region(&acpi_gbl_FADT.xpm2_control_block,
+			    acpi_gbl_FADT.pm2_control_length,
+			    "ACPI PM2_CNT_BLK");
 
 	/* Length of GPE blocks must be a non-negative multiple of 2 */
 
 	if (!(acpi_gbl_FADT.gpe0_block_length & 0x1))
 		acpi_request_region(&acpi_gbl_FADT.xgpe0_block,
-			       acpi_gbl_FADT.gpe0_block_length, "ACPI GPE0_BLK");
+				    acpi_gbl_FADT.gpe0_block_length,
+				    "ACPI GPE0_BLK");
 
 	if (!(acpi_gbl_FADT.gpe1_block_length & 0x1))
 		acpi_request_region(&acpi_gbl_FADT.xgpe1_block,
-			       acpi_gbl_FADT.gpe1_block_length, "ACPI GPE1_BLK");
+				    acpi_gbl_FADT.gpe1_block_length,
+				    "ACPI GPE1_BLK");
 
 	return 0;
 }
@@ -215,6 +223,7 @@ device_initcall(acpi_reserve_resources);
 void acpi_os_printf(const char *fmt, ...)
 {
 	va_list args;
+
 	va_start(args, fmt);
 	acpi_os_vprintf(fmt, args);
 	va_end(args);
@@ -227,11 +236,10 @@ void acpi_os_vprintf(const char *fmt, va_list args)
 	vsprintf(buffer, fmt, args);
 
 #ifdef ENABLE_DEBUGGER
-	if (acpi_in_debugger) {
+	if (acpi_in_debugger)
 		kdb_printf("%s", buffer);
-	} else {
+	else
 		pr_cont("%s", buffer);
-	}
 #else
 	pr_cont("%s", buffer);
 #endif
@@ -879,6 +887,7 @@ void acpi_os_stall(u32 us)
 u64 acpi_os_get_timer(void)
 {
 	u64 time_ns = ktime_to_ns(ktime_get());
+
 	do_div(time_ns, 100);
 	return time_ns;
 }
@@ -891,15 +900,14 @@ acpi_status acpi_os_read_port(acpi_io_address port, u32 *value, u32 width)
 		value = &dummy;
 
 	*value = 0;
-	if (width <= 8) {
+	if (width <= 8)
 		*(u8 *) value = inb(port);
-	} else if (width <= 16) {
+	else if (width <= 16)
 		*(u16 *) value = inw(port);
-	} else if (width <= 32) {
+	else if (width <= 32)
 		*(u32 *) value = inl(port);
-	} else {
+	else
 		BUG();
-	}
 
 	return AE_OK;
 }
@@ -908,15 +916,14 @@ EXPORT_SYMBOL(acpi_os_read_port);
 
 acpi_status acpi_os_write_port(acpi_io_address port, u32 value, u32 width)
 {
-	if (width <= 8) {
+	if (width <= 8)
 		outb(value, port);
-	} else if (width <= 16) {
+	else if (width <= 16)
 		outw(value, port);
-	} else if (width <= 32) {
+	else if (width <= 32)
 		outl(value, port);
-	} else {
+	else
 		BUG();
-	}
 
 	return AE_OK;
 }
@@ -932,6 +939,7 @@ static inline u64 read64(const volatile void __iomem *addr)
 static inline u64 read64(const volatile void __iomem *addr)
 {
 	u64 l, h;
+
 	l = readl(addr);
 	h = readl(addr+4);
 	return l | (h << 32);
@@ -1116,8 +1124,8 @@ static void acpi_os_execute_deferred(struct work_struct *work)
  *
  * RETURN:      Status
  *
- * DESCRIPTION: Depending on type, either queues function for deferred execution or
- *              immediately executes function on a separate thread.
+ * DESCRIPTION: Depending on type, either queues function for deferred
+ *		execution or immediately executes function on a separate thread.
  *
  ******************************************************************************/
 
@@ -1128,6 +1136,7 @@ acpi_status acpi_os_execute(acpi_execute_type type,
 	struct acpi_os_dpc *dpc;
 	struct workqueue_struct *queue;
 	int ret;
+
 	ACPI_DEBUG_PRINT((ACPI_DB_EXEC,
 			  "Scheduling function [%p(%p)] for deferred execution.\n",
 			  function, context));
@@ -1199,7 +1208,8 @@ struct acpi_hp_work {
 
 static void acpi_hotplug_work_fn(struct work_struct *work)
 {
-	struct acpi_hp_work *hpw = container_of(work, struct acpi_hp_work, work);
+	struct acpi_hp_work *hpw = container_of(work, struct acpi_hp_work,
+						work);
 
 	acpi_os_wait_events_complete();
 	acpi_device_hotplug(hpw->adev, hpw->src);
@@ -1693,6 +1703,7 @@ void acpi_os_delete_lock(acpi_spinlock handle)
 acpi_cpu_flags acpi_os_acquire_lock(acpi_spinlock lockp)
 {
 	acpi_cpu_flags flags;
+
 	spin_lock_irqsave(lockp, flags);
 	return flags;
 }
@@ -1869,6 +1880,7 @@ acpi_status acpi_os_prepare_sleep(u8 sleep_state, u32 pm1a_control,
 				  u32 pm1b_control)
 {
 	int rc = 0;
+
 	if (__acpi_os_prepare_sleep)
 		rc = __acpi_os_prepare_sleep(sleep_state,
 					     pm1a_control, pm1b_control);
@@ -1890,6 +1902,7 @@ acpi_status acpi_os_prepare_extended_sleep(u8 sleep_state, u32 val_a,
 				  u32 val_b)
 {
 	int rc = 0;
+
 	if (__acpi_os_prepare_extended_sleep)
 		rc = __acpi_os_prepare_extended_sleep(sleep_state,
 					     val_a, val_b);
-- 
2.1.0

[PATCH v3 4/9] ACPI: clean up checkpatch warnings for items with possible semantic value

[al.stone]

From: Al Stone <al.stone@linaro.org>

In preparation for later splitting out some of the arch-dependent code from
osl.c, clean up some warnings from checkpatch that fall into more semantic
issues; none of these should change functionality, but they do touch lines
of code with semantic significance:

   -- replaced #include <asm/foo.h> with #include <linux/foo.h>
   -- replaced extern that was only being used for sizeof() with a #define
   -- removed use of else after breaks/returns when not useful
   -- moved __initdata to the proper place in a definition
   -- moved EXPORT_SYMBOL to a line immediately after the function
   -- removed unnecessary return statements from void functions

Signed-off-by: Al Stone <al.stone@linaro.org>
---
 drivers/acpi/osl.c | 31 ++++++++++---------------------
 1 file changed, 10 insertions(+), 21 deletions(-)

diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index 46317ff..af6dda7 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -40,9 +40,8 @@
 #include <linux/list.h>
 #include <linux/jiffies.h>
 #include <linux/semaphore.h>
-
-#include <asm/io.h>
-#include <asm/uaccess.h>
+#include <linux/io.h>
+#include <linux/uaccess.h>
 
 #include "internal.h"
 
@@ -66,7 +65,7 @@ struct acpi_os_dpc {
 int acpi_in_debugger;
 EXPORT_SYMBOL(acpi_in_debugger);
 
-extern char line_buf[80];
+#define DEBUGGER_LINE_BUFLEN	80
 #endif				/*ENABLE_DEBUGGER */
 
 static int (*__acpi_os_prepare_sleep)(u8 sleep_state, u32 pm1a_ctrl,
@@ -268,10 +267,8 @@ acpi_physical_address __init acpi_os_get_root_pointer(void)
 			return efi.acpi20;
 		else if (efi.acpi != EFI_INVALID_TABLE_ADDR)
 			return efi.acpi;
-		else {
-			pr_err(PREFIX "System description tables not found\n");
-			return 0;
-		}
+		pr_err(PREFIX "System description tables not found\n");
+		return 0;
 	} else if (IS_ENABLED(CONFIG_ACPI_LEGACY_TABLES_LOOKUP)) {
 		acpi_physical_address pa = 0;
 
@@ -594,7 +591,7 @@ static const char * const table_sigs[] = {
 #define ACPI_HEADER_SIZE sizeof(struct acpi_table_header)
 
 #define ACPI_OVERRIDE_TABLES 64
-static struct cpio_data __initdata acpi_initrd_files[ACPI_OVERRIDE_TABLES];
+static struct cpio_data acpi_initrd_files[ACPI_OVERRIDE_TABLES] __initdata;
 
 #define MAP_CHUNK_SIZE   (NR_FIX_BTMAPS << PAGE_SHIFT)
 
@@ -806,10 +803,10 @@ static irqreturn_t acpi_irq(int irq, void *dev_id)
 	if (handled) {
 		acpi_irq_handled++;
 		return IRQ_HANDLED;
-	} else {
-		acpi_irq_not_handled++;
-		return IRQ_NONE;
 	}
+
+	acpi_irq_not_handled++;
+	return IRQ_NONE;
 }
 
 acpi_status
@@ -911,7 +908,6 @@ acpi_status acpi_os_read_port(acpi_io_address port, u32 *value, u32 width)
 
 	return AE_OK;
 }
-
 EXPORT_SYMBOL(acpi_os_read_port);
 
 acpi_status acpi_os_write_port(acpi_io_address port, u32 value, u32 width)
@@ -927,7 +923,6 @@ acpi_status acpi_os_write_port(acpi_io_address port, u32 value, u32 width)
 
 	return AE_OK;
 }
-
 EXPORT_SYMBOL(acpi_os_write_port);
 
 #ifdef readq
@@ -1362,7 +1357,7 @@ u32 acpi_os_get_line(char *buffer)
 	if (acpi_in_debugger) {
 		u32 chars;
 
-		kdb_read(buffer, sizeof(line_buf));
+		kdb_read(buffer, sizeof(DEBUGGER_LINE_BUFLEN));
 
 		/* remove the CR kdb includes */
 		chars = strlen(buffer) - 1;
@@ -1490,8 +1485,6 @@ static void __init set_osi_linux(unsigned int enable)
 		acpi_osi_setup("Linux");
 	else
 		acpi_osi_setup("!Linux");
-
-	return;
 }
 
 static void __init acpi_cmdline_osi_linux(unsigned int enable)
@@ -1499,8 +1492,6 @@ static void __init acpi_cmdline_osi_linux(unsigned int enable)
 	osi_linux.cmdline = 1;	/* cmdline set the default and override DMI */
 	osi_linux.dmi = 0;
 	set_osi_linux(enable);
-
-	return;
 }
 
 void __init acpi_dmi_osi_linux(int enable, const struct dmi_system_id *d)
@@ -1512,8 +1503,6 @@ void __init acpi_dmi_osi_linux(int enable, const struct dmi_system_id *d)
 
 	osi_linux.dmi = 1;	/* DMI knows that this box asks OSI(Linux) */
 	set_osi_linux(enable);
-
-	return;
 }
 
 /*
-- 
2.1.0

[PATCH v3 5/9] ACPI: move acpi_os_handler() so it can be made arch-dependent later

[al.stone]

From: Al Stone <al.stone@linaro.org>

In order to deprecate the use of _OSI for arm64 or other new architectures,
we need to make the default handler something we can change for various
platforms.  This patch moves the definition of acpi_osi_handler() -- the
function used by ACPICA as a callback for evaluating _OSI -- into a separate
file.  Subsequent patches will change which files get built so that we can
then build the version of _OSI we need for a particular architecture.

There is no functional change.

Signed-off-by: Al Stone <al.stone@linaro.org>
---
 drivers/acpi/Makefile |  2 +-
 drivers/acpi/osi.c    | 95 +++++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/acpi/osl.c    | 24 -------------
 include/linux/acpi.h  |  1 +
 4 files changed, 97 insertions(+), 25 deletions(-)
 create mode 100644 drivers/acpi/osi.c

diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index 13536d8..97191eb 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -18,7 +18,7 @@ obj-y				+= acpi.o \
 					acpica/
 
 # All the builtin files are in the "acpi." module_param namespace.
-acpi-y				+= osl.o utils.o reboot.o
+acpi-y				+= osl.o utils.o reboot.o osi.o
 acpi-y				+= nvs.o
 
 # Power management related files
diff --git a/drivers/acpi/osi.c b/drivers/acpi/osi.c
new file mode 100644
index 0000000..f23aa70
--- /dev/null
+++ b/drivers/acpi/osi.c
@@ -0,0 +1,95 @@
+/*
+ *  osi.c - _OSI implementation (moved from drivers/acpi/osl.c)
+ *
+ *  Copyright (C) 2000       Andrew Henroid
+ *  Copyright (C) 2001, 2002 Andy Grover <andrew.grover@intel.com>
+ *  Copyright (C) 2001, 2002 Paul Diefenbaugh <paul.s.diefenbaugh@intel.com>
+ *  Copyright (c) 2008 Intel Corporation
+ *   Author: Matthew Wilcox <willy@linux.intel.com>
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ */
+
+#include <linux/acpi.h>
+
+#define _COMPONENT		ACPI_OS_SERVICES
+ACPI_MODULE_NAME("osi");
+
+#define PREFIX			"ACPI: "
+
+/*
+ * The story of _OSI(Linux)
+ *
+ * From pre-history through Linux-2.6.22,
+ * Linux responded TRUE upon a BIOS OSI(Linux) query.
+ *
+ * Unfortunately, reference BIOS writers got wind of this
+ * and put OSI(Linux) in their example code, quickly exposing
+ * this string as ill-conceived and opening the door to
+ * an un-bounded number of BIOS incompatibilities.
+ *
+ * For example, OSI(Linux) was used on resume to re-POST a
+ * video card on one system, because Linux at that time
+ * could not do a speedy restore in its native driver.
+ * But then upon gaining quick native restore capability,
+ * Linux has no way to tell the BIOS to skip the time-consuming
+ * POST -- putting Linux at a permanent performance disadvantage.
+ * On another system, the BIOS writer used OSI(Linux)
+ * to infer native OS support for IPMI!  On other systems,
+ * OSI(Linux) simply got in the way of Linux claiming to
+ * be compatible with other operating systems, exposing
+ * BIOS issues such as skipped device initialization.
+ *
+ * So "Linux" turned out to be a really poor chose of
+ * OSI string, and from Linux-2.6.23 onward we respond FALSE.
+ *
+ * BIOS writers should NOT query _OSI(Linux) on future systems.
+ * Linux will complain on the console when it sees it, and return FALSE.
+ * To get Linux to return TRUE for your system  will require
+ * a kernel source update to add a DMI entry,
+ * or boot with "acpi_osi=Linux"
+ */
+
+static struct osi_linux {
+	unsigned int	enable:1;
+	unsigned int	dmi:1;
+	unsigned int	cmdline:1;
+	unsigned int	default_disabling:1;
+} osi_linux = {0, 0, 0, 0};
+
+u32 acpi_osi_handler(acpi_string interface, u32 supported)
+{
+	if (!strcmp("Linux", interface)) {
+
+		pr_notice_once(FW_BUG PREFIX
+			"BIOS _OSI(Linux) query %s%s\n",
+			osi_linux.enable ? "honored" : "ignored",
+			osi_linux.cmdline ? " via cmdline" :
+			osi_linux.dmi ? " via DMI" : "");
+	}
+
+	if (!strcmp("Darwin", interface)) {
+		/*
+		 * Apple firmware will behave poorly if it receives positive
+		 * answers to "Darwin" and any other OS. Respond positively
+		 * to Darwin and then disable all other vendor strings.
+		 */
+		acpi_update_interfaces(ACPI_DISABLE_ALL_VENDOR_STRINGS);
+		supported = ACPI_UINT32_MAX;
+	}
+
+	return supported;
+}
diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index af6dda7..823a08f 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -136,30 +136,6 @@ static struct osi_linux {
 	unsigned int	default_disabling:1;
 } osi_linux = {0, 0, 0, 0};
 
-static u32 acpi_osi_handler(acpi_string interface, u32 supported)
-{
-	if (!strcmp("Linux", interface)) {
-
-		pr_notice_once(FW_BUG PREFIX
-			"BIOS _OSI(Linux) query %s%s\n",
-			osi_linux.enable ? "honored" : "ignored",
-			osi_linux.cmdline ? " via cmdline" :
-			osi_linux.dmi ? " via DMI" : "");
-	}
-
-	if (!strcmp("Darwin", interface)) {
-		/*
-		 * Apple firmware will behave poorly if it receives positive
-		 * answers to "Darwin" and any other OS. Respond positively
-		 * to Darwin and then disable all other vendor strings.
-		 */
-		acpi_update_interfaces(ACPI_DISABLE_ALL_VENDOR_STRINGS);
-		supported = ACPI_UINT32_MAX;
-	}
-
-	return supported;
-}
-
 static void __init acpi_request_region(struct acpi_generic_address *gas,
 	unsigned int length, char *desc)
 {
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 9a01d5d..a7c402a 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -281,6 +281,7 @@ static inline int acpi_video_display_switch_support(void)
 extern int acpi_blacklisted(void);
 extern void acpi_dmi_osi_linux(int enable, const struct dmi_system_id *d);
 extern void acpi_osi_setup(char *str);
+extern u32 acpi_osi_handler(acpi_string interface, u32 supported);
 
 #ifdef CONFIG_ACPI_NUMA
 int acpi_get_node(acpi_handle handle);
-- 
2.1.0

[PATCH v3 6/9] ACPI: move _OSI support functions to allow arch-dependent implementation

[al.stone]

From: Al Stone <al.stone@linaro.org>

Having moved the _OSI callback function needed by ACPICA from
drivers/acpi/osl.c to drivers/acpi/osi.c, we now move all the
remaining _OSI support functions to osi.c.

This patch is much larger than I had wanted it to be; several of the
functions that implemented acpi_osi* command line options, or did the
set up of the interfaces to be provided by _OSI, shared a static struct.
Hence, I ended up moving a bunch of code at once rather than perhaps a
function at a time.

With this patch, all the _OSI-associated code has now been moved
to osi.c, and we next change the build so that we can make the
_OSI implementation arch-dependent.

There is no functional change.

Signed-off-by: Al Stone <al.stone@linaro.org>
---
 drivers/acpi/osi.c   | 152 ++++++++++++++++++++++++++++++++++++++++-
 drivers/acpi/osl.c   | 187 ---------------------------------------------------
 include/linux/acpi.h |   5 +-
 3 files changed, 154 insertions(+), 190 deletions(-)

diff --git a/drivers/acpi/osi.c b/drivers/acpi/osi.c
index f23aa70..9943b7a 100644
--- a/drivers/acpi/osi.c
+++ b/drivers/acpi/osi.c
@@ -30,6 +30,8 @@ ACPI_MODULE_NAME("osi");
 
 #define PREFIX			"ACPI: "
 
+static void __init acpi_osi_setup_late(void);
+
 /*
  * The story of _OSI(Linux)
  *
@@ -68,10 +70,14 @@ static struct osi_linux {
 	unsigned int	dmi:1;
 	unsigned int	cmdline:1;
 	unsigned int	default_disabling:1;
-} osi_linux = {0, 0, 0, 0};
+	unsigned int	interfaces_added:1;
+} osi_linux = {0, 0, 0, 0, 0};
 
 u32 acpi_osi_handler(acpi_string interface, u32 supported)
 {
+	if (!osi_linux.interfaces_added)
+		acpi_osi_setup_late();
+
 	if (!strcmp("Linux", interface)) {
 
 		pr_notice_once(FW_BUG PREFIX
@@ -93,3 +99,147 @@ u32 acpi_osi_handler(acpi_string interface, u32 supported)
 
 	return supported;
 }
+
+#define	OSI_STRING_LENGTH_MAX 64	/* arbitrary */
+#define	OSI_STRING_ENTRIES_MAX 16	/* arbitrary */
+
+struct osi_setup_entry {
+	char string[OSI_STRING_LENGTH_MAX];
+	bool enable;
+};
+
+static struct osi_setup_entry
+		osi_setup_entries[OSI_STRING_ENTRIES_MAX] = {
+	{"Module Device", true},
+	{"Processor Device", true},
+	{"3.0 _SCP Extensions", true},
+	{"Processor Aggregator Device", true},
+};
+
+void __init acpi_osi_setup(char *str)
+{
+	struct osi_setup_entry *osi;
+	bool enable = true;
+	int i;
+
+	if (!acpi_gbl_create_osi_method)
+		return;
+
+	if (str == NULL || *str == '\0') {
+		pr_info(PREFIX "_OSI method disabled\n");
+		acpi_gbl_create_osi_method = FALSE;
+		return;
+	}
+
+	if (*str == '!') {
+		str++;
+		if (*str == '\0') {
+			osi_linux.default_disabling = 1;
+			return;
+		} else if (*str == '*') {
+			acpi_update_interfaces(ACPI_DISABLE_ALL_STRINGS);
+			for (i = 0; i < OSI_STRING_ENTRIES_MAX; i++) {
+				osi = &osi_setup_entries[i];
+				osi->enable = false;
+			}
+			return;
+		}
+		enable = false;
+	}
+
+	for (i = 0; i < OSI_STRING_ENTRIES_MAX; i++) {
+		osi = &osi_setup_entries[i];
+		if (!strcmp(osi->string, str)) {
+			osi->enable = enable;
+			break;
+		} else if (osi->string[0] == '\0') {
+			osi->enable = enable;
+			strncpy(osi->string, str, OSI_STRING_LENGTH_MAX);
+			break;
+		}
+	}
+}
+
+static void __init set_osi_linux(unsigned int enable)
+{
+	if (osi_linux.enable != enable)
+		osi_linux.enable = enable;
+
+	if (osi_linux.enable)
+		acpi_osi_setup("Linux");
+	else
+		acpi_osi_setup("!Linux");
+}
+
+static void __init acpi_cmdline_osi_linux(unsigned int enable)
+{
+	osi_linux.cmdline = 1;	/* cmdline set the default and override DMI */
+	osi_linux.dmi = 0;
+	set_osi_linux(enable);
+}
+
+void __init acpi_dmi_osi_linux(int enable, const struct dmi_system_id *d)
+{
+	pr_notice(PREFIX "DMI detected: %s\n", d->ident);
+
+	if (enable == -1)
+		return;
+
+	osi_linux.dmi = 1;	/* DMI knows that this box asks OSI(Linux) */
+	set_osi_linux(enable);
+}
+
+/*
+ * Modify the list of "OS Interfaces" reported to BIOS via _OSI
+ *
+ * empty string disables _OSI
+ * string starting with '!' disables that string
+ * otherwise string is added to list, augmenting built-in strings
+ */
+static void __init acpi_osi_setup_late(void)
+{
+	struct osi_setup_entry *osi;
+	char *str;
+	int i;
+	acpi_status status;
+
+	if (osi_linux.default_disabling) {
+		status = acpi_update_interfaces(ACPI_DISABLE_ALL_VENDOR_STRINGS);
+
+		if (ACPI_SUCCESS(status))
+			pr_info(PREFIX "Disabled all _OSI OS vendors\n");
+	}
+
+	for (i = 0; i < OSI_STRING_ENTRIES_MAX; i++) {
+		osi = &osi_setup_entries[i];
+		str = osi->string;
+
+		if (*str == '\0')
+			break;
+		if (osi->enable) {
+			status = acpi_install_interface(str);
+
+			if (ACPI_SUCCESS(status))
+				pr_info(PREFIX "Added _OSI(%s)\n", str);
+		} else {
+			status = acpi_remove_interface(str);
+
+			if (ACPI_SUCCESS(status))
+				pr_info(PREFIX "Deleted _OSI(%s)\n", str);
+		}
+	}
+}
+
+static int __init osi_setup(char *str)
+{
+	if (str && !strcmp("Linux", str))
+		acpi_cmdline_osi_linux(1);
+	else if (str && !strcmp("!Linux", str))
+		acpi_cmdline_osi_linux(0);
+	else
+		acpi_osi_setup(str);
+
+	return 1;
+}
+
+__setup("acpi_osi=", osi_setup);
diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index 823a08f..1eb5813e 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -94,48 +94,6 @@ struct acpi_ioremap {
 static LIST_HEAD(acpi_ioremaps);
 static DEFINE_MUTEX(acpi_ioremap_lock);
 
-static void __init acpi_osi_setup_late(void);
-
-/*
- * The story of _OSI(Linux)
- *
- * From pre-history through Linux-2.6.22,
- * Linux responded TRUE upon a BIOS OSI(Linux) query.
- *
- * Unfortunately, reference BIOS writers got wind of this
- * and put OSI(Linux) in their example code, quickly exposing
- * this string as ill-conceived and opening the door to
- * an un-bounded number of BIOS incompatibilities.
- *
- * For example, OSI(Linux) was used on resume to re-POST a
- * video card on one system, because Linux at that time
- * could not do a speedy restore in its native driver.
- * But then upon gaining quick native restore capability,
- * Linux has no way to tell the BIOS to skip the time-consuming
- * POST -- putting Linux at a permanent performance disadvantage.
- * On another system, the BIOS writer used OSI(Linux)
- * to infer native OS support for IPMI!  On other systems,
- * OSI(Linux) simply got in the way of Linux claiming to
- * be compatible with other operating systems, exposing
- * BIOS issues such as skipped device initialization.
- *
- * So "Linux" turned out to be a really poor chose of
- * OSI string, and from Linux-2.6.23 onward we respond FALSE.
- *
- * BIOS writers should NOT query _OSI(Linux) on future systems.
- * Linux will complain on the console when it sees it, and return FALSE.
- * To get Linux to return TRUE for your system  will require
- * a kernel source update to add a DMI entry,
- * or boot with "acpi_osi=Linux"
- */
-
-static struct osi_linux {
-	unsigned int	enable:1;
-	unsigned int	dmi:1;
-	unsigned int	cmdline:1;
-	unsigned int	default_disabling:1;
-} osi_linux = {0, 0, 0, 0};
-
 static void __init acpi_request_region(struct acpi_generic_address *gas,
 	unsigned int length, char *desc)
 {
@@ -1392,150 +1350,6 @@ static int __init acpi_os_name_setup(char *str)
 
 __setup("acpi_os_name=", acpi_os_name_setup);
 
-#define	OSI_STRING_LENGTH_MAX 64	/* arbitrary */
-#define	OSI_STRING_ENTRIES_MAX 16	/* arbitrary */
-
-struct osi_setup_entry {
-	char string[OSI_STRING_LENGTH_MAX];
-	bool enable;
-};
-
-static struct osi_setup_entry
-		osi_setup_entries[OSI_STRING_ENTRIES_MAX] __initdata = {
-	{"Module Device", true},
-	{"Processor Device", true},
-	{"3.0 _SCP Extensions", true},
-	{"Processor Aggregator Device", true},
-};
-
-void __init acpi_osi_setup(char *str)
-{
-	struct osi_setup_entry *osi;
-	bool enable = true;
-	int i;
-
-	if (!acpi_gbl_create_osi_method)
-		return;
-
-	if (str == NULL || *str == '\0') {
-		pr_info(PREFIX "_OSI method disabled\n");
-		acpi_gbl_create_osi_method = FALSE;
-		return;
-	}
-
-	if (*str == '!') {
-		str++;
-		if (*str == '\0') {
-			osi_linux.default_disabling = 1;
-			return;
-		} else if (*str == '*') {
-			acpi_update_interfaces(ACPI_DISABLE_ALL_STRINGS);
-			for (i = 0; i < OSI_STRING_ENTRIES_MAX; i++) {
-				osi = &osi_setup_entries[i];
-				osi->enable = false;
-			}
-			return;
-		}
-		enable = false;
-	}
-
-	for (i = 0; i < OSI_STRING_ENTRIES_MAX; i++) {
-		osi = &osi_setup_entries[i];
-		if (!strcmp(osi->string, str)) {
-			osi->enable = enable;
-			break;
-		} else if (osi->string[0] == '\0') {
-			osi->enable = enable;
-			strncpy(osi->string, str, OSI_STRING_LENGTH_MAX);
-			break;
-		}
-	}
-}
-
-static void __init set_osi_linux(unsigned int enable)
-{
-	if (osi_linux.enable != enable)
-		osi_linux.enable = enable;
-
-	if (osi_linux.enable)
-		acpi_osi_setup("Linux");
-	else
-		acpi_osi_setup("!Linux");
-}
-
-static void __init acpi_cmdline_osi_linux(unsigned int enable)
-{
-	osi_linux.cmdline = 1;	/* cmdline set the default and override DMI */
-	osi_linux.dmi = 0;
-	set_osi_linux(enable);
-}
-
-void __init acpi_dmi_osi_linux(int enable, const struct dmi_system_id *d)
-{
-	pr_notice(PREFIX "DMI detected: %s\n", d->ident);
-
-	if (enable == -1)
-		return;
-
-	osi_linux.dmi = 1;	/* DMI knows that this box asks OSI(Linux) */
-	set_osi_linux(enable);
-}
-
-/*
- * Modify the list of "OS Interfaces" reported to BIOS via _OSI
- *
- * empty string disables _OSI
- * string starting with '!' disables that string
- * otherwise string is added to list, augmenting built-in strings
- */
-static void __init acpi_osi_setup_late(void)
-{
-	struct osi_setup_entry *osi;
-	char *str;
-	int i;
-	acpi_status status;
-
-	if (osi_linux.default_disabling) {
-		status = acpi_update_interfaces(ACPI_DISABLE_ALL_VENDOR_STRINGS);
-
-		if (ACPI_SUCCESS(status))
-			pr_info(PREFIX "Disabled all _OSI OS vendors\n");
-	}
-
-	for (i = 0; i < OSI_STRING_ENTRIES_MAX; i++) {
-		osi = &osi_setup_entries[i];
-		str = osi->string;
-
-		if (*str == '\0')
-			break;
-		if (osi->enable) {
-			status = acpi_install_interface(str);
-
-			if (ACPI_SUCCESS(status))
-				pr_info(PREFIX "Added _OSI(%s)\n", str);
-		} else {
-			status = acpi_remove_interface(str);
-
-			if (ACPI_SUCCESS(status))
-				pr_info(PREFIX "Deleted _OSI(%s)\n", str);
-		}
-	}
-}
-
-static int __init osi_setup(char *str)
-{
-	if (str && !strcmp("Linux", str))
-		acpi_cmdline_osi_linux(1);
-	else if (str && !strcmp("!Linux", str))
-		acpi_cmdline_osi_linux(0);
-	else
-		acpi_osi_setup(str);
-
-	return 1;
-}
-
-__setup("acpi_osi=", osi_setup);
-
 /*
  * Disable the auto-serialization of named objects creation methods.
  *
@@ -1816,7 +1630,6 @@ acpi_status __init acpi_os_initialize1(void)
 	BUG_ON(!kacpi_notify_wq);
 	BUG_ON(!kacpi_hotplug_wq);
 	acpi_install_interface_handler(acpi_osi_handler);
-	acpi_osi_setup_late();
 	return AE_OK;
 }
 
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index a7c402a..71cdeb6 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -279,8 +279,9 @@ static inline int acpi_video_display_switch_support(void)
 #endif /* defined(CONFIG_ACPI_VIDEO) || defined(CONFIG_ACPI_VIDEO_MODULE) */
 
 extern int acpi_blacklisted(void);
-extern void acpi_dmi_osi_linux(int enable, const struct dmi_system_id *d);
-extern void acpi_osi_setup(char *str);
+extern void __init acpi_dmi_osi_linux(int enable,
+				      const struct dmi_system_id *d);
+extern void __init acpi_osi_setup(char *str);
 extern u32 acpi_osi_handler(acpi_string interface, u32 supported);
 
 #ifdef CONFIG_ACPI_NUMA
-- 
2.1.0

[PATCH v3 7/9] ACPI: enable arch-specific compilation for _OSI and the blacklist

[al.stone]

From: Al Stone <al.stone@linaro.org>

Whether arch-specific functions are used or not now depends on the config
option CONFIG_ARCH_SPECIFIC_ACPI_OSI.  By default, this is set false which
causes the x86/ia64 versions to be used, just as is done today.  Setting
this option true causes architecture-specific implementations to be built
instead; this patch also provides arm64 implementations.

For x86/ia64, there is no functional change.  Other architectures will have
to add files to provide their specific functionality.

Signed-off-by: Al Stone <al.stone@linaro.org>
---
 drivers/acpi/Kconfig  | 3 +++
 drivers/acpi/Makefile | 7 ++++++-
 include/linux/acpi.h  | 8 ++++++++
 3 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index b340509..4d91f22 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -53,6 +53,9 @@ config ACPI_SLEEP
 	depends on SUSPEND || HIBERNATION
 	default y
 
+config ARCH_SPECIFIC_ACPI_OSI
+	def_bool n
+
 config ACPI_PROCFS_POWER
 	bool "Deprecated power /proc/acpi directories"
 	depends on PROC_FS
diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index 97191eb..1428464 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -18,9 +18,14 @@ obj-y				+= acpi.o \
 					acpica/
 
 # All the builtin files are in the "acpi." module_param namespace.
-acpi-y				+= osl.o utils.o reboot.o osi.o
+acpi-y				+= osl.o utils.o reboot.o
 acpi-y				+= nvs.o
 
+# _OSI related files
+ifneq ($(CONFIG_ARCH_SPECIFIC_ACPI_OSI), y)
+acpi-y				+= osi.o
+endif
+
 # Power management related files
 acpi-y				+= wakeup.o
 acpi-y				+= sleep.o
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 71cdeb6..97185a1 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -279,9 +279,17 @@ static inline int acpi_video_display_switch_support(void)
 #endif /* defined(CONFIG_ACPI_VIDEO) || defined(CONFIG_ACPI_VIDEO_MODULE) */
 
 extern int acpi_blacklisted(void);
+
+#ifdef CONFIG_ARCH_SPECIFIC_ACPI_OSI
+static inline void __init acpi_dmi_osi_linux(int enable,
+					     const struct dmi_system_id *d) { }
+static inline void __init acpi_osi_setup(char *str) { }
+#else
 extern void __init acpi_dmi_osi_linux(int enable,
 				      const struct dmi_system_id *d);
 extern void __init acpi_osi_setup(char *str);
+#endif
+
 extern u32 acpi_osi_handler(acpi_string interface, u32 supported);
 
 #ifdef CONFIG_ACPI_NUMA
-- 
2.1.0

[PATCH v3 8/9] ACPI: arm64: use an arch-specific ACPI _OSI method and ACPI blacklist

[al.stone]

From: Al Stone <al.stone@linaro.org>

Now that all of the _OSI functionality has been separated out, we can
provide arch-specific functionality for it.  This also allows us to do
the same for the acpi_blacklisted() function.  We also make sure the
defaults for the arm64 kernel are set so that the arch-specific _OSI
method and blacklist are always used for ACPI.

For arm64, any use of _OSI will issue a warning that it is deprecated.
All use of _OSI will return false -- i.e., it will return no useful
information to any firmware using it.  The ability to temporarily turn
on _OSI, or turn off _OSI, or affect it in other ways from the command
line is no longer available for arm64, either.  The blacklist for ACPI
on arm64 is empty.  This will, of course, require ACPI to be enabled
for arm64.

Signed-off-by: Al Stone <al.stone@linaro.org>
---
 arch/arm64/Kconfig                 |  1 +
 arch/arm64/kernel/Makefile         |  2 +-
 arch/arm64/kernel/acpi-blacklist.c | 20 ++++++++++++++++++++
 arch/arm64/kernel/acpi-osi.c       | 25 +++++++++++++++++++++++++
 4 files changed, 47 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm64/kernel/acpi-blacklist.c
 create mode 100644 arch/arm64/kernel/acpi-osi.c

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 3f08727..e441d28 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -7,6 +7,7 @@ config ARM64
 	select ARCH_HAS_SG_CHAIN
 	select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
 	select ARCH_USE_CMPXCHG_LOCKREF
+	select ARCH_SPECIFIC_ACPI_OSI if ACPI
 	select ARCH_SUPPORTS_ATOMIC_RMW
 	select ARCH_WANT_OPTIONAL_GPIOLIB
 	select ARCH_WANT_COMPAT_IPC_PARSE_VERSION
diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
index 79bdd3b..b5e1268 100644
--- a/arch/arm64/kernel/Makefile
+++ b/arch/arm64/kernel/Makefile
@@ -35,7 +35,7 @@ arm64-obj-$(CONFIG_KGDB)		+= kgdb.o
 arm64-obj-$(CONFIG_EFI)			+= efi.o efi-stub.o efi-entry.o
 arm64-obj-$(CONFIG_PCI)			+= pci.o
 arm64-obj-$(CONFIG_ARMV8_DEPRECATED)	+= armv8_deprecated.o
-arm64-obj-$(CONFIG_ACPI)		+= acpi.o
+arm64-obj-$(CONFIG_ACPI)		+= acpi.o acpi-osi.o acpi-blacklist.o
 
 obj-y					+= $(arm64-obj-y) vdso/
 obj-m					+= $(arm64-obj-m)
diff --git a/arch/arm64/kernel/acpi-blacklist.c b/arch/arm64/kernel/acpi-blacklist.c
new file mode 100644
index 0000000..1be6a56
--- /dev/null
+++ b/arch/arm64/kernel/acpi-blacklist.c
@@ -0,0 +1,20 @@
+/*
+ *  ARM64 Specific ACPI Blacklist Support
+ *
+ *  Copyright (C) 2015, Linaro Ltd.
+ *	Author: Al Stone <al.stone@linaro.org>
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ */
+
+#define pr_fmt(fmt) "ACPI: " fmt
+
+#include <linux/acpi.h>
+
+/* The arm64 ACPI blacklist is currently empty.  */
+int __init acpi_blacklisted(void)
+{
+	return 0;
+}
diff --git a/arch/arm64/kernel/acpi-osi.c b/arch/arm64/kernel/acpi-osi.c
new file mode 100644
index 0000000..bb351f4
--- /dev/null
+++ b/arch/arm64/kernel/acpi-osi.c
@@ -0,0 +1,25 @@
+/*
+ *  ARM64 Specific ACPI _OSI Support
+ *
+ *  Copyright (C) 2015, Linaro Ltd.
+ *	Author: Al Stone <al.stone@linaro.org>
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ */
+
+#define pr_fmt(fmt) "ACPI: " fmt
+
+#include <linux/acpi.h>
+
+/*
+ * Consensus is to deprecate _OSI for all new ACPI-supported architectures.
+ * So, for arm64, reduce _OSI to a warning message, and tell the firmware
+ * nothing of value.
+ */
+u32 acpi_osi_handler(acpi_string interface, u32 supported)
+{
+	pr_warn("_OSI was called, but is deprecated for this architecture.\n");
+	return false;
+}
-- 
2.1.0

[PATCH v3 9/9] ACPI: arm64: use "Linux" as ACPI_OS_NAME for _OS on arm64

[al.stone]

From: Al Stone <al.stone@linaro.org>

ACPI_OS_NAME is globally defined as "Microsoft Windows NT" for now.
That doesn't make much sense in the ARM context, so set it to "Linux"
when CONFIG_ARM64.

If it is necessary to change the return value from \_OS_ (that is, return
some value other than the default in ACPI_OS_NAME), use the kernel parameter
"acpi_os_name=<string>".

Many thanks to Rafael Wysocki for this greatly simplified form of the patch.

Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
Signed-off-by: Al Stone <al.stone@linaro.org>
---
 arch/arm64/Kconfig              | 5 +++++
 include/acpi/acconfig.h         | 2 ++
 include/acpi/platform/aclinux.h | 4 ++++
 3 files changed, 11 insertions(+)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index e441d28..d812113 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -87,6 +87,11 @@ config ARM64
 config 64BIT
 	def_bool y
 
+config ACPI_OS_NAME
+	string
+	default "Linux"
+	depends on ACPI
+
 config ARCH_PHYS_ADDR_T_64BIT
 	def_bool y
 
diff --git a/include/acpi/acconfig.h b/include/acpi/acconfig.h
index 5a0a3e5..1980bf4 100644
--- a/include/acpi/acconfig.h
+++ b/include/acpi/acconfig.h
@@ -69,7 +69,9 @@
  * code that will not execute the _OSI method unless _OS matches the string
  * below.  Therefore, change this string at your own risk.
  */
+#ifndef ACPI_OS_NAME
 #define ACPI_OS_NAME                    "Microsoft Windows NT"
+#endif
 
 /* Maximum objects in the various object caches */
 
diff --git a/include/acpi/platform/aclinux.h b/include/acpi/platform/aclinux.h
index 1ba7c19..a871cdd 100644
--- a/include/acpi/platform/aclinux.h
+++ b/include/acpi/platform/aclinux.h
@@ -69,6 +69,10 @@
 #define ACPI_REDUCED_HARDWARE 1
 #endif
 
+#ifdef CONFIG_ACPI_OS_NAME
+#define ACPI_OS_NAME CONFIG_ACPI_OS_NAME
+#endif
+
 #include <linux/string.h>
 #include <linux/kernel.h>
 #include <linux/ctype.h>
-- 
2.1.0

Re: [PATCH v3 1/9] ACPI: fix all errors reported by cleanpatch.pl in osl.c

[Hanjun Guo]

On 2015年02月25日 08:36, al.stone@linaro.org wrote:
> From: Al Stone <al.stone@linaro.org>
>
> In preparation for later splitting out some of the arch-dependent code from
> osl.c, clean up the errors reported by checkpatch.pl.  They fell into these
> classes:
>
>     -- remove the FSF address from the GPL notice
>     -- "foo * bar" should be "foo *bar" (and the ** variation of same)
>     -- a return is not a function, so parentheses are not required.
>
> Signed-off-by: Al Stone <al.stone@linaro.org>

Reviewd-by: Hanjun Guo <hanjun.guo@linaro.org>

Thanks
Hanjun

Re: [PATCH v3 2/9] ACPI: clear up warnings on use of printk reported by checkpatch.pl

[Hanjun Guo]

On 2015年02月25日 08:36, al.stone@linaro.org wrote:
> From: Al Stone <al.stone@linaro.org>
>
> In preparation for later splitting out some of the arch-dependent code from
> osl.c, clear up all the warnings reported by checkpatch.pl where pr_* should
> be used instead of printk(KERN_* ...).
>
> Signed-off-by: Al Stone <al.stone@linaro.org>
> ---
>   drivers/acpi/osl.c | 46 +++++++++++++++++++---------------------------
>   1 file changed, 19 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
> index 1dc3a3b..865317c 100644
> --- a/drivers/acpi/osl.c
> +++ b/drivers/acpi/osl.c
> @@ -141,7 +141,7 @@ static u32 acpi_osi_handler(acpi_string interface, u32 supported)
>   {
>   	if (!strcmp("Linux", interface)) {
>
> -		printk_once(KERN_NOTICE FW_BUG PREFIX
> +		pr_notice_once(FW_BUG PREFIX

I think you can use pr_fmt() and then remove all the PREFIX in
this patch, just

#define pr_fmt(fmt) "ACPI: " fmt

in top of this file before all #includes.

Thanks
Hanjun

Re: [Linaro-acpi] [PATCH v3 3/9] ACPI: clean up checkpatch warnings for various bits of syntax

[Hanjun Guo]

On 2015年02月25日 08:36, al.stone@linaro.org wrote:
> From: Al Stone<al.stone@linaro.org>
>
> In preparation for later splitting out some of the arch-dependent code from
> osl.c, clean up a bunch of warnings for odd bits of syntax:
>
>     -- remove CVS keyword markers
>     -- remove a space from between a function name and an opening parenthesis
>     -- clean up all but one line > 80 characters (one just looks silly if you
>        make it less than 80)
>     -- add blank lines after declarations in functions
>     -- remove extraneous braces ({})
>
> Signed-off-by: Al Stone<al.stone@linaro.org>

Reviewd-by: Hanjun Guo <hanjun.guo@linaro.org>

Thanks
Hanjun

Re: [Linaro-acpi] [PATCH v3 4/9] ACPI: clean up checkpatch warnings for items with possible semantic value

[Hanjun Guo]

On 2015年02月25日 08:36, al.stone@linaro.org wrote:
> From: Al Stone <al.stone@linaro.org>
>
> In preparation for later splitting out some of the arch-dependent code from
> osl.c, clean up some warnings from checkpatch that fall into more semantic
> issues; none of these should change functionality, but they do touch lines
> of code with semantic significance:
>
>     -- replaced #include <asm/foo.h> with #include <linux/foo.h>
>     -- replaced extern that was only being used for sizeof() with a #define
>     -- removed use of else after breaks/returns when not useful
>     -- moved __initdata to the proper place in a definition
>     -- moved EXPORT_SYMBOL to a line immediately after the function
>     -- removed unnecessary return statements from void functions
>
> Signed-off-by: Al Stone <al.stone@linaro.org>
> ---
>   drivers/acpi/osl.c | 31 ++++++++++---------------------
>   1 file changed, 10 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
> index 46317ff..af6dda7 100644
> --- a/drivers/acpi/osl.c
> +++ b/drivers/acpi/osl.c
> @@ -40,9 +40,8 @@
>   #include <linux/list.h>
>   #include <linux/jiffies.h>
>   #include <linux/semaphore.h>
> -
> -#include <asm/io.h>
> -#include <asm/uaccess.h>
> +#include <linux/io.h>
> +#include <linux/uaccess.h>
>
>   #include "internal.h"
>
> @@ -66,7 +65,7 @@ struct acpi_os_dpc {
>   int acpi_in_debugger;
>   EXPORT_SYMBOL(acpi_in_debugger);
>
> -extern char line_buf[80];
> +#define DEBUGGER_LINE_BUFLEN	80
>   #endif				/*ENABLE_DEBUGGER */
>
>   static int (*__acpi_os_prepare_sleep)(u8 sleep_state, u32 pm1a_ctrl,
> @@ -268,10 +267,8 @@ acpi_physical_address __init acpi_os_get_root_pointer(void)
>   			return efi.acpi20;
>   		else if (efi.acpi != EFI_INVALID_TABLE_ADDR)
>   			return efi.acpi;
> -		else {
> -			pr_err(PREFIX "System description tables not found\n");
> -			return 0;
> -		}
> +		pr_err(PREFIX "System description tables not found\n");
> +		return 0;
>   	} else if (IS_ENABLED(CONFIG_ACPI_LEGACY_TABLES_LOOKUP)) {
>   		acpi_physical_address pa = 0;
>
> @@ -594,7 +591,7 @@ static const char * const table_sigs[] = {
>   #define ACPI_HEADER_SIZE sizeof(struct acpi_table_header)
>
>   #define ACPI_OVERRIDE_TABLES 64
> -static struct cpio_data __initdata acpi_initrd_files[ACPI_OVERRIDE_TABLES];
> +static struct cpio_data acpi_initrd_files[ACPI_OVERRIDE_TABLES] __initdata;
>
>   #define MAP_CHUNK_SIZE   (NR_FIX_BTMAPS << PAGE_SHIFT)
>
> @@ -806,10 +803,10 @@ static irqreturn_t acpi_irq(int irq, void *dev_id)
>   	if (handled) {
>   		acpi_irq_handled++;
>   		return IRQ_HANDLED;
> -	} else {
> -		acpi_irq_not_handled++;
> -		return IRQ_NONE;
>   	}
> +
> +	acpi_irq_not_handled++;
> +	return IRQ_NONE;
>   }
>
>   acpi_status
> @@ -911,7 +908,6 @@ acpi_status acpi_os_read_port(acpi_io_address port, u32 *value, u32 width)
>
>   	return AE_OK;
>   }
> -
>   EXPORT_SYMBOL(acpi_os_read_port);
>
>   acpi_status acpi_os_write_port(acpi_io_address port, u32 value, u32 width)
> @@ -927,7 +923,6 @@ acpi_status acpi_os_write_port(acpi_io_address port, u32 value, u32 width)
>
>   	return AE_OK;
>   }
> -
>   EXPORT_SYMBOL(acpi_os_write_port);
>
>   #ifdef readq
> @@ -1362,7 +1357,7 @@ u32 acpi_os_get_line(char *buffer)
>   	if (acpi_in_debugger) {
>   		u32 chars;
>
> -		kdb_read(buffer, sizeof(line_buf));
> +		kdb_read(buffer, sizeof(DEBUGGER_LINE_BUFLEN));

I think kdb_read(buffer, DEBUGGER_LINE_BUFLEN); will be fine.

other than that,

Reviewd-by: Hanjun Guo <hanjun.guo@linaro.org>

Thanks
Hanjun

>
>   		/* remove the CR kdb includes */
>   		chars = strlen(buffer) - 1;
> @@ -1490,8 +1485,6 @@ static void __init set_osi_linux(unsigned int enable)
>   		acpi_osi_setup("Linux");
>   	else
>   		acpi_osi_setup("!Linux");
> -
> -	return;
>   }
>
>   static void __init acpi_cmdline_osi_linux(unsigned int enable)
> @@ -1499,8 +1492,6 @@ static void __init acpi_cmdline_osi_linux(unsigned int enable)
>   	osi_linux.cmdline = 1;	/* cmdline set the default and override DMI */
>   	osi_linux.dmi = 0;
>   	set_osi_linux(enable);
> -
> -	return;
>   }
>
>   void __init acpi_dmi_osi_linux(int enable, const struct dmi_system_id *d)
> @@ -1512,8 +1503,6 @@ void __init acpi_dmi_osi_linux(int enable, const struct dmi_system_id *d)
>
>   	osi_linux.dmi = 1;	/* DMI knows that this box asks OSI(Linux) */
>   	set_osi_linux(enable);
> -
> -	return;
>   }
>
>   /*
>

Re: [Linaro-acpi] [PATCH v3 5/9] ACPI: move acpi_os_handler() so it can be made arch-dependent later

[Hanjun Guo]

On 2015年02月25日 08:36, al.stone@linaro.org wrote:
> From: Al Stone <al.stone@linaro.org>
>
> In order to deprecate the use of _OSI for arm64 or other new architectures,
> we need to make the default handler something we can change for various
> platforms.  This patch moves the definition of acpi_osi_handler() -- the
> function used by ACPICA as a callback for evaluating _OSI -- into a separate
> file.  Subsequent patches will change which files get built so that we can
> then build the version of _OSI we need for a particular architecture.
>
> There is no functional change.
>
> Signed-off-by: Al Stone <al.stone@linaro.org>
> ---
>   drivers/acpi/Makefile |  2 +-
>   drivers/acpi/osi.c    | 95 +++++++++++++++++++++++++++++++++++++++++++++++++++
>   drivers/acpi/osl.c    | 24 -------------
>   include/linux/acpi.h  |  1 +
>   4 files changed, 97 insertions(+), 25 deletions(-)
>   create mode 100644 drivers/acpi/osi.c
>
> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
> index 13536d8..97191eb 100644
> --- a/drivers/acpi/Makefile
> +++ b/drivers/acpi/Makefile
> @@ -18,7 +18,7 @@ obj-y				+= acpi.o \
>   					acpica/
>
>   # All the builtin files are in the "acpi." module_param namespace.
> -acpi-y				+= osl.o utils.o reboot.o
> +acpi-y				+= osl.o utils.o reboot.o osi.o
>   acpi-y				+= nvs.o
>
>   # Power management related files
> diff --git a/drivers/acpi/osi.c b/drivers/acpi/osi.c
> new file mode 100644
> index 0000000..f23aa70
> --- /dev/null
> +++ b/drivers/acpi/osi.c
> @@ -0,0 +1,95 @@
> +/*
> + *  osi.c - _OSI implementation (moved from drivers/acpi/osl.c)
> + *
> + *  Copyright (C) 2000       Andrew Henroid
> + *  Copyright (C) 2001, 2002 Andy Grover <andrew.grover@intel.com>
> + *  Copyright (C) 2001, 2002 Paul Diefenbaugh <paul.s.diefenbaugh@intel.com>
> + *  Copyright (c) 2008 Intel Corporation
> + *   Author: Matthew Wilcox <willy@linux.intel.com>
> + *
> + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation; either version 2 of the License, or
> + *  (at your option) any later version.
> + *
> + *  This program is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> + *
> + */
> +
> +#include <linux/acpi.h>
> +
> +#define _COMPONENT		ACPI_OS_SERVICES
> +ACPI_MODULE_NAME("osi");
> +
> +#define PREFIX			"ACPI: "

Hi Al, remove PREFIX here and use pr_fmt() instead as
I mentioned in previous patch :)

Thanks
Hanjun

Re: [PATCH v3 2/9] ACPI: clear up warnings on use of printk reported by checkpatch.pl

[Al Stone]

On 02/25/2015 05:55 AM, Hanjun Guo wrote:
> On 2015年02月25日 08:36, al.stone@linaro.org wrote:
>> From: Al Stone <al.stone@linaro.org>
>>
>> In preparation for later splitting out some of the arch-dependent code from
>> osl.c, clear up all the warnings reported by checkpatch.pl where pr_* should
>> be used instead of printk(KERN_* ...).
>>
>> Signed-off-by: Al Stone <al.stone@linaro.org>
>> ---
>>   drivers/acpi/osl.c | 46 +++++++++++++++++++---------------------------
>>   1 file changed, 19 insertions(+), 27 deletions(-)
>>
>> diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
>> index 1dc3a3b..865317c 100644
>> --- a/drivers/acpi/osl.c
>> +++ b/drivers/acpi/osl.c
>> @@ -141,7 +141,7 @@ static u32 acpi_osi_handler(acpi_string interface, u32
>> supported)
>>   {
>>       if (!strcmp("Linux", interface)) {
>>
>> -        printk_once(KERN_NOTICE FW_BUG PREFIX
>> +        pr_notice_once(FW_BUG PREFIX
> 
> I think you can use pr_fmt() and then remove all the PREFIX in
> this patch, just
> 
> #define pr_fmt(fmt) "ACPI: " fmt
> 
> in top of this file before all #includes.
> 
> Thanks
> Hanjun

Argh.  Yup, you're right.  That was an oversight on my part.
Fixed in next version.

-- 
ciao,
al
-----------------------------------
Al Stone
Software Engineer
Linaro Enterprise Group
al.stone@linaro.org
-----------------------------------

Re: [Linaro-acpi] [PATCH v3 4/9] ACPI: clean up checkpatch warnings for items with possible semantic value

[Al Stone]

On 02/25/2015 06:08 AM, Hanjun Guo wrote:
> On 2015年02月25日 08:36, al.stone@linaro.org wrote:
>> From: Al Stone <al.stone@linaro.org>
>>
>> In preparation for later splitting out some of the arch-dependent code from
>> osl.c, clean up some warnings from checkpatch that fall into more semantic
>> issues; none of these should change functionality, but they do touch lines
>> of code with semantic significance:
>>
>>     -- replaced #include <asm/foo.h> with #include <linux/foo.h>
>>     -- replaced extern that was only being used for sizeof() with a #define
>>     -- removed use of else after breaks/returns when not useful
>>     -- moved __initdata to the proper place in a definition
>>     -- moved EXPORT_SYMBOL to a line immediately after the function
>>     -- removed unnecessary return statements from void functions
>>
>> Signed-off-by: Al Stone <al.stone@linaro.org>
>> ---
>>   drivers/acpi/osl.c | 31 ++++++++++---------------------
>>   1 file changed, 10 insertions(+), 21 deletions(-)
>>
>> diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
>> index 46317ff..af6dda7 100644
>> --- a/drivers/acpi/osl.c
>> +++ b/drivers/acpi/osl.c
>> @@ -40,9 +40,8 @@
>>   #include <linux/list.h>
>>   #include <linux/jiffies.h>
>>   #include <linux/semaphore.h>
>> -
>> -#include <asm/io.h>
>> -#include <asm/uaccess.h>
>> +#include <linux/io.h>
>> +#include <linux/uaccess.h>
>>
>>   #include "internal.h"
>>
>> @@ -66,7 +65,7 @@ struct acpi_os_dpc {
>>   int acpi_in_debugger;
>>   EXPORT_SYMBOL(acpi_in_debugger);
>>
>> -extern char line_buf[80];
>> +#define DEBUGGER_LINE_BUFLEN    80
>>   #endif                /*ENABLE_DEBUGGER */
>>
>>   static int (*__acpi_os_prepare_sleep)(u8 sleep_state, u32 pm1a_ctrl,
>> @@ -268,10 +267,8 @@ acpi_physical_address __init acpi_os_get_root_pointer(void)
>>               return efi.acpi20;
>>           else if (efi.acpi != EFI_INVALID_TABLE_ADDR)
>>               return efi.acpi;
>> -        else {
>> -            pr_err(PREFIX "System description tables not found\n");
>> -            return 0;
>> -        }
>> +        pr_err(PREFIX "System description tables not found\n");
>> +        return 0;
>>       } else if (IS_ENABLED(CONFIG_ACPI_LEGACY_TABLES_LOOKUP)) {
>>           acpi_physical_address pa = 0;
>>
>> @@ -594,7 +591,7 @@ static const char * const table_sigs[] = {
>>   #define ACPI_HEADER_SIZE sizeof(struct acpi_table_header)
>>
>>   #define ACPI_OVERRIDE_TABLES 64
>> -static struct cpio_data __initdata acpi_initrd_files[ACPI_OVERRIDE_TABLES];
>> +static struct cpio_data acpi_initrd_files[ACPI_OVERRIDE_TABLES] __initdata;
>>
>>   #define MAP_CHUNK_SIZE   (NR_FIX_BTMAPS << PAGE_SHIFT)
>>
>> @@ -806,10 +803,10 @@ static irqreturn_t acpi_irq(int irq, void *dev_id)
>>       if (handled) {
>>           acpi_irq_handled++;
>>           return IRQ_HANDLED;
>> -    } else {
>> -        acpi_irq_not_handled++;
>> -        return IRQ_NONE;
>>       }
>> +
>> +    acpi_irq_not_handled++;
>> +    return IRQ_NONE;
>>   }
>>
>>   acpi_status
>> @@ -911,7 +908,6 @@ acpi_status acpi_os_read_port(acpi_io_address port, u32
>> *value, u32 width)
>>
>>       return AE_OK;
>>   }
>> -
>>   EXPORT_SYMBOL(acpi_os_read_port);
>>
>>   acpi_status acpi_os_write_port(acpi_io_address port, u32 value, u32 width)
>> @@ -927,7 +923,6 @@ acpi_status acpi_os_write_port(acpi_io_address port, u32
>> value, u32 width)
>>
>>       return AE_OK;
>>   }
>> -
>>   EXPORT_SYMBOL(acpi_os_write_port);
>>
>>   #ifdef readq
>> @@ -1362,7 +1357,7 @@ u32 acpi_os_get_line(char *buffer)
>>       if (acpi_in_debugger) {
>>           u32 chars;
>>
>> -        kdb_read(buffer, sizeof(line_buf));
>> +        kdb_read(buffer, sizeof(DEBUGGER_LINE_BUFLEN));
> 
> I think kdb_read(buffer, DEBUGGER_LINE_BUFLEN); will be fine.
> 
> other than that,
> 
> Reviewd-by: Hanjun Guo <hanjun.guo@linaro.org>
> 
> Thanks
> Hanjun

Whups.  Boy, was that just plain wrong.  Thanks.  Fixed.

>>
>>           /* remove the CR kdb includes */
>>           chars = strlen(buffer) - 1;
>> @@ -1490,8 +1485,6 @@ static void __init set_osi_linux(unsigned int enable)
>>           acpi_osi_setup("Linux");
>>       else
>>           acpi_osi_setup("!Linux");
>> -
>> -    return;
>>   }
>>
>>   static void __init acpi_cmdline_osi_linux(unsigned int enable)
>> @@ -1499,8 +1492,6 @@ static void __init acpi_cmdline_osi_linux(unsigned int
>> enable)
>>       osi_linux.cmdline = 1;    /* cmdline set the default and override DMI */
>>       osi_linux.dmi = 0;
>>       set_osi_linux(enable);
>> -
>> -    return;
>>   }
>>
>>   void __init acpi_dmi_osi_linux(int enable, const struct dmi_system_id *d)
>> @@ -1512,8 +1503,6 @@ void __init acpi_dmi_osi_linux(int enable, const struct
>> dmi_system_id *d)
>>
>>       osi_linux.dmi = 1;    /* DMI knows that this box asks OSI(Linux) */
>>       set_osi_linux(enable);
>> -
>> -    return;
>>   }
>>
>>   /*
>>


-- 
ciao,
al
-----------------------------------
Al Stone
Software Engineer
Linaro Enterprise Group
al.stone@linaro.org
-----------------------------------

Re: [PATCH v3 8/9] ACPI: arm64: use an arch-specific ACPI _OSI method and ACPI blacklist

[Will Deacon]

Hi Al,

On Wed, Feb 25, 2015 at 12:36:24AM +0000, al.stone@linaro.org wrote:
> diff --git a/arch/arm64/kernel/acpi-blacklist.c b/arch/arm64/kernel/acpi-blacklist.c
> new file mode 100644
> index 0000000..1be6a56
> --- /dev/null
> +++ b/arch/arm64/kernel/acpi-blacklist.c
> @@ -0,0 +1,20 @@
> +/*
> + *  ARM64 Specific ACPI Blacklist Support
> + *
> + *  Copyright (C) 2015, Linaro Ltd.
> + *	Author: Al Stone <al.stone@linaro.org>
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License version 2 as
> + *  published by the Free Software Foundation.
> + */
> +
> +#define pr_fmt(fmt) "ACPI: " fmt
> +
> +#include <linux/acpi.h>
> +
> +/* The arm64 ACPI blacklist is currently empty.  */
> +int __init acpi_blacklisted(void)
> +{
> +	return 0;
> +}
> diff --git a/arch/arm64/kernel/acpi-osi.c b/arch/arm64/kernel/acpi-osi.c
> new file mode 100644
> index 0000000..bb351f4
> --- /dev/null
> +++ b/arch/arm64/kernel/acpi-osi.c
> @@ -0,0 +1,25 @@
> +/*
> + *  ARM64 Specific ACPI _OSI Support
> + *
> + *  Copyright (C) 2015, Linaro Ltd.
> + *	Author: Al Stone <al.stone@linaro.org>
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License version 2 as
> + *  published by the Free Software Foundation.
> + */
> +
> +#define pr_fmt(fmt) "ACPI: " fmt
> +
> +#include <linux/acpi.h>
> +
> +/*
> + * Consensus is to deprecate _OSI for all new ACPI-supported architectures.
> + * So, for arm64, reduce _OSI to a warning message, and tell the firmware
> + * nothing of value.
> + */
> +u32 acpi_osi_handler(acpi_string interface, u32 supported)
> +{
> +	pr_warn("_OSI was called, but is deprecated for this architecture.\n");
> +	return false;
> +}

This kinda feels backwards to me. If _OSI is going away, then the default
should be "the architecture doesn't need to do anything", rather than have
new architectures defining a bunch of empty, useless stub code.

Anyway we could make this the default in core code and have architectures
that *do* want _OSI override that behaviour, instead of the other way around?

Cheers,

Will

Re: [PATCH v3 8/9] ACPI: arm64: use an arch-specific ACPI _OSI method and ACPI blacklist

[Al Stone]

On 03/02/2015 10:29 AM, Will Deacon wrote:
> Hi Al,
> 
> On Wed, Feb 25, 2015 at 12:36:24AM +0000, al.stone@linaro.org wrote:
>> diff --git a/arch/arm64/kernel/acpi-blacklist.c b/arch/arm64/kernel/acpi-blacklist.c
>> new file mode 100644
>> index 0000000..1be6a56
>> --- /dev/null
>> +++ b/arch/arm64/kernel/acpi-blacklist.c
>> @@ -0,0 +1,20 @@
>> +/*
>> + *  ARM64 Specific ACPI Blacklist Support
>> + *
>> + *  Copyright (C) 2015, Linaro Ltd.
>> + *	Author: Al Stone <al.stone@linaro.org>
>> + *
>> + *  This program is free software; you can redistribute it and/or modify
>> + *  it under the terms of the GNU General Public License version 2 as
>> + *  published by the Free Software Foundation.
>> + */
>> +
>> +#define pr_fmt(fmt) "ACPI: " fmt
>> +
>> +#include <linux/acpi.h>
>> +
>> +/* The arm64 ACPI blacklist is currently empty.  */
>> +int __init acpi_blacklisted(void)
>> +{
>> +	return 0;
>> +}
>> diff --git a/arch/arm64/kernel/acpi-osi.c b/arch/arm64/kernel/acpi-osi.c
>> new file mode 100644
>> index 0000000..bb351f4
>> --- /dev/null
>> +++ b/arch/arm64/kernel/acpi-osi.c
>> @@ -0,0 +1,25 @@
>> +/*
>> + *  ARM64 Specific ACPI _OSI Support
>> + *
>> + *  Copyright (C) 2015, Linaro Ltd.
>> + *	Author: Al Stone <al.stone@linaro.org>
>> + *
>> + *  This program is free software; you can redistribute it and/or modify
>> + *  it under the terms of the GNU General Public License version 2 as
>> + *  published by the Free Software Foundation.
>> + */
>> +
>> +#define pr_fmt(fmt) "ACPI: " fmt
>> +
>> +#include <linux/acpi.h>
>> +
>> +/*
>> + * Consensus is to deprecate _OSI for all new ACPI-supported architectures.
>> + * So, for arm64, reduce _OSI to a warning message, and tell the firmware
>> + * nothing of value.
>> + */
>> +u32 acpi_osi_handler(acpi_string interface, u32 supported)
>> +{
>> +	pr_warn("_OSI was called, but is deprecated for this architecture.\n");
>> +	return false;
>> +}
> 
> This kinda feels backwards to me. If _OSI is going away, then the default
> should be "the architecture doesn't need to do anything", rather than have
> new architectures defining a bunch of empty, useless stub code.
> 
> Anyway we could make this the default in core code and have architectures
> that *do* want _OSI override that behaviour, instead of the other way around?
> 
> Cheers,
> 
> Will
> 

We could do that; I personally don't have a strong preference either way,
so I'm inclined to make it whatever structure Rafael thinks is proper since
it affects ACPI code most.  That being said, the current patch structure
made sense to me since it wasn't distorting existing code much -- and given
the pure number of x86/ia64 machines vs ARM machines using ACPI, that seemed
the more cautious approach.

@Rafael: do you have an opinion/preference?

-- 
ciao,
al
-----------------------------------
Al Stone
Software Engineer
Linaro Enterprise Group
al.stone@linaro.org
-----------------------------------

Re: [PATCH v3 1/9] ACPI: fix all errors reported by cleanpatch.pl in osl.c

[Rafael J. Wysocki]

On Tuesday, February 24, 2015 05:36:17 PM al.stone@linaro.org wrote:
> From: Al Stone <al.stone@linaro.org>
> 
> In preparation for later splitting out some of the arch-dependent code from
> osl.c, clean up the errors reported by checkpatch.pl.  They fell into these
> classes:
> 
>    -- remove the FSF address from the GPL notice
>    -- "foo * bar" should be "foo *bar" (and the ** variation of same)
>    -- a return is not a function, so parentheses are not required.
> 
> Signed-off-by: Al Stone <al.stone@linaro.org>

checkpatch.pl is irrelevant here.  You're trying to make the coding style be
more consistent with the coding style of the rest of the kernel.

The warnings from checkpatch.pl are meaningless for the existing code, so
it should not be used to justify changes in that code.

Of course, the same applies to patches [2-4/9].


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH v3 6/9] ACPI: move _OSI support functions to allow arch-dependent implementation

[Rafael J. Wysocki]

On Tuesday, February 24, 2015 05:36:22 PM al.stone@linaro.org wrote:
> From: Al Stone <al.stone@linaro.org>
> 
> Having moved the _OSI callback function needed by ACPICA from
> drivers/acpi/osl.c to drivers/acpi/osi.c, we now move all the
> remaining _OSI support functions to osi.c.
> 
> This patch is much larger than I had wanted it to be; several of the
> functions that implemented acpi_osi* command line options, or did the
> set up of the interfaces to be provided by _OSI, shared a static struct.
> Hence, I ended up moving a bunch of code at once rather than perhaps a
> function at a time.
> 
> With this patch, all the _OSI-associated code has now been moved
> to osi.c, and we next change the build so that we can make the
> _OSI implementation arch-dependent.
> 
> There is no functional change.
> 
> Signed-off-by: Al Stone <al.stone@linaro.org>
> ---
>  drivers/acpi/osi.c   | 152 ++++++++++++++++++++++++++++++++++++++++-
>  drivers/acpi/osl.c   | 187 ---------------------------------------------------
>  include/linux/acpi.h |   5 +-
>  3 files changed, 154 insertions(+), 190 deletions(-)
> 
> diff --git a/drivers/acpi/osi.c b/drivers/acpi/osi.c
> index f23aa70..9943b7a 100644
> --- a/drivers/acpi/osi.c
> +++ b/drivers/acpi/osi.c
> @@ -30,6 +30,8 @@ ACPI_MODULE_NAME("osi");
>  
>  #define PREFIX			"ACPI: "
>  
> +static void __init acpi_osi_setup_late(void);
> +
>  /*
>   * The story of _OSI(Linux)
>   *
> @@ -68,10 +70,14 @@ static struct osi_linux {
>  	unsigned int	dmi:1;
>  	unsigned int	cmdline:1;
>  	unsigned int	default_disabling:1;
> -} osi_linux = {0, 0, 0, 0};
> +	unsigned int	interfaces_added:1;
> +} osi_linux = {0, 0, 0, 0, 0};
>  
>  u32 acpi_osi_handler(acpi_string interface, u32 supported)
>  {
> +	if (!osi_linux.interfaces_added)
> +		acpi_osi_setup_late();
> +

This wasn't there in the old code.

Please don't mix moving code around with adding new things to it.


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH v3 7/9] ACPI: enable arch-specific compilation for _OSI and the blacklist

[Rafael J. Wysocki]

On Tuesday, February 24, 2015 05:36:23 PM al.stone@linaro.org wrote:
> From: Al Stone <al.stone@linaro.org>
> 
> Whether arch-specific functions are used or not now depends on the config
> option CONFIG_ARCH_SPECIFIC_ACPI_OSI.  By default, this is set false which
> causes the x86/ia64 versions to be used, just as is done today.  Setting
> this option true causes architecture-specific implementations to be built
> instead; this patch also provides arm64 implementations.
> 
> For x86/ia64, there is no functional change.  Other architectures will have
> to add files to provide their specific functionality.
> 
> Signed-off-by: Al Stone <al.stone@linaro.org>

This one is fine by me.

> ---
>  drivers/acpi/Kconfig  | 3 +++
>  drivers/acpi/Makefile | 7 ++++++-
>  include/linux/acpi.h  | 8 ++++++++
>  3 files changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index b340509..4d91f22 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -53,6 +53,9 @@ config ACPI_SLEEP
>  	depends on SUSPEND || HIBERNATION
>  	default y
>  
> +config ARCH_SPECIFIC_ACPI_OSI
> +	def_bool n
> +
>  config ACPI_PROCFS_POWER
>  	bool "Deprecated power /proc/acpi directories"
>  	depends on PROC_FS
> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
> index 97191eb..1428464 100644
> --- a/drivers/acpi/Makefile
> +++ b/drivers/acpi/Makefile
> @@ -18,9 +18,14 @@ obj-y				+= acpi.o \
>  					acpica/
>  
>  # All the builtin files are in the "acpi." module_param namespace.
> -acpi-y				+= osl.o utils.o reboot.o osi.o
> +acpi-y				+= osl.o utils.o reboot.o
>  acpi-y				+= nvs.o
>  
> +# _OSI related files
> +ifneq ($(CONFIG_ARCH_SPECIFIC_ACPI_OSI), y)
> +acpi-y				+= osi.o
> +endif
> +
>  # Power management related files
>  acpi-y				+= wakeup.o
>  acpi-y				+= sleep.o
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index 71cdeb6..97185a1 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -279,9 +279,17 @@ static inline int acpi_video_display_switch_support(void)
>  #endif /* defined(CONFIG_ACPI_VIDEO) || defined(CONFIG_ACPI_VIDEO_MODULE) */
>  
>  extern int acpi_blacklisted(void);
> +
> +#ifdef CONFIG_ARCH_SPECIFIC_ACPI_OSI
> +static inline void __init acpi_dmi_osi_linux(int enable,
> +					     const struct dmi_system_id *d) { }
> +static inline void __init acpi_osi_setup(char *str) { }
> +#else
>  extern void __init acpi_dmi_osi_linux(int enable,
>  				      const struct dmi_system_id *d);
>  extern void __init acpi_osi_setup(char *str);
> +#endif
> +
>  extern u32 acpi_osi_handler(acpi_string interface, u32 supported);
>  
>  #ifdef CONFIG_ACPI_NUMA
> 

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH v3 8/9] ACPI: arm64: use an arch-specific ACPI _OSI method and ACPI blacklist

[Rafael J. Wysocki]

On Monday, March 02, 2015 12:00:21 PM Al Stone wrote:
> On 03/02/2015 10:29 AM, Will Deacon wrote:
> > Hi Al,
> > 
> > On Wed, Feb 25, 2015 at 12:36:24AM +0000, al.stone@linaro.org wrote:
> >> diff --git a/arch/arm64/kernel/acpi-blacklist.c b/arch/arm64/kernel/acpi-blacklist.c
> >> new file mode 100644
> >> index 0000000..1be6a56
> >> --- /dev/null
> >> +++ b/arch/arm64/kernel/acpi-blacklist.c
> >> @@ -0,0 +1,20 @@
> >> +/*
> >> + *  ARM64 Specific ACPI Blacklist Support
> >> + *
> >> + *  Copyright (C) 2015, Linaro Ltd.
> >> + *	Author: Al Stone <al.stone@linaro.org>
> >> + *
> >> + *  This program is free software; you can redistribute it and/or modify
> >> + *  it under the terms of the GNU General Public License version 2 as
> >> + *  published by the Free Software Foundation.
> >> + */
> >> +
> >> +#define pr_fmt(fmt) "ACPI: " fmt
> >> +
> >> +#include <linux/acpi.h>
> >> +
> >> +/* The arm64 ACPI blacklist is currently empty.  */
> >> +int __init acpi_blacklisted(void)
> >> +{
> >> +	return 0;
> >> +}
> >> diff --git a/arch/arm64/kernel/acpi-osi.c b/arch/arm64/kernel/acpi-osi.c
> >> new file mode 100644
> >> index 0000000..bb351f4
> >> --- /dev/null
> >> +++ b/arch/arm64/kernel/acpi-osi.c
> >> @@ -0,0 +1,25 @@
> >> +/*
> >> + *  ARM64 Specific ACPI _OSI Support
> >> + *
> >> + *  Copyright (C) 2015, Linaro Ltd.
> >> + *	Author: Al Stone <al.stone@linaro.org>
> >> + *
> >> + *  This program is free software; you can redistribute it and/or modify
> >> + *  it under the terms of the GNU General Public License version 2 as
> >> + *  published by the Free Software Foundation.
> >> + */
> >> +
> >> +#define pr_fmt(fmt) "ACPI: " fmt
> >> +
> >> +#include <linux/acpi.h>
> >> +
> >> +/*
> >> + * Consensus is to deprecate _OSI for all new ACPI-supported architectures.
> >> + * So, for arm64, reduce _OSI to a warning message, and tell the firmware
> >> + * nothing of value.
> >> + */
> >> +u32 acpi_osi_handler(acpi_string interface, u32 supported)
> >> +{
> >> +	pr_warn("_OSI was called, but is deprecated for this architecture.\n");
> >> +	return false;
> >> +}
> > 
> > This kinda feels backwards to me. If _OSI is going away, then the default
> > should be "the architecture doesn't need to do anything", rather than have
> > new architectures defining a bunch of empty, useless stub code.
> > 
> > Anyway we could make this the default in core code and have architectures
> > that *do* want _OSI override that behaviour, instead of the other way around?
> > 
> > Cheers,
> > 
> > Will
> > 
> 
> We could do that; I personally don't have a strong preference either way,
> so I'm inclined to make it whatever structure Rafael thinks is proper since
> it affects ACPI code most.  That being said, the current patch structure
> made sense to me since it wasn't distorting existing code much -- and given
> the pure number of x86/ia64 machines vs ARM machines using ACPI, that seemed
> the more cautious approach.
> 
> @Rafael: do you have an opinion/preference?

My preference is to avoid changes in the existing code at least for the time
being.  Especially if the changes in question are going to affect ia64, unless
you have an Itanium machine where you can readily test those, that is. :-)


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH v3 8/9] ACPI: arm64: use an arch-specific ACPI _OSI method and ACPI blacklist

[Rafael J. Wysocki]

On Tuesday, February 24, 2015 05:36:24 PM al.stone@linaro.org wrote:
> From: Al Stone <al.stone@linaro.org>
> 
> Now that all of the _OSI functionality has been separated out, we can
> provide arch-specific functionality for it.  This also allows us to do
> the same for the acpi_blacklisted() function.  We also make sure the
> defaults for the arm64 kernel are set so that the arch-specific _OSI
> method and blacklist are always used for ACPI.
> 
> For arm64, any use of _OSI will issue a warning that it is deprecated.
> All use of _OSI will return false -- i.e., it will return no useful
> information to any firmware using it.  The ability to temporarily turn
> on _OSI, or turn off _OSI, or affect it in other ways from the command
> line is no longer available for arm64, either.  The blacklist for ACPI
> on arm64 is empty.  This will, of course, require ACPI to be enabled
> for arm64.
> 
> Signed-off-by: Al Stone <al.stone@linaro.org>
> ---
>  arch/arm64/Kconfig                 |  1 +
>  arch/arm64/kernel/Makefile         |  2 +-
>  arch/arm64/kernel/acpi-blacklist.c | 20 ++++++++++++++++++++
>  arch/arm64/kernel/acpi-osi.c       | 25 +++++++++++++++++++++++++
>  4 files changed, 47 insertions(+), 1 deletion(-)
>  create mode 100644 arch/arm64/kernel/acpi-blacklist.c
>  create mode 100644 arch/arm64/kernel/acpi-osi.c
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 3f08727..e441d28 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -7,6 +7,7 @@ config ARM64
>  	select ARCH_HAS_SG_CHAIN
>  	select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
>  	select ARCH_USE_CMPXCHG_LOCKREF
> +	select ARCH_SPECIFIC_ACPI_OSI if ACPI
>  	select ARCH_SUPPORTS_ATOMIC_RMW
>  	select ARCH_WANT_OPTIONAL_GPIOLIB
>  	select ARCH_WANT_COMPAT_IPC_PARSE_VERSION
> diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
> index 79bdd3b..b5e1268 100644
> --- a/arch/arm64/kernel/Makefile
> +++ b/arch/arm64/kernel/Makefile
> @@ -35,7 +35,7 @@ arm64-obj-$(CONFIG_KGDB)		+= kgdb.o
>  arm64-obj-$(CONFIG_EFI)			+= efi.o efi-stub.o efi-entry.o
>  arm64-obj-$(CONFIG_PCI)			+= pci.o
>  arm64-obj-$(CONFIG_ARMV8_DEPRECATED)	+= armv8_deprecated.o
> -arm64-obj-$(CONFIG_ACPI)		+= acpi.o
> +arm64-obj-$(CONFIG_ACPI)		+= acpi.o acpi-osi.o acpi-blacklist.o
>  
>  obj-y					+= $(arm64-obj-y) vdso/
>  obj-m					+= $(arm64-obj-m)
> diff --git a/arch/arm64/kernel/acpi-blacklist.c b/arch/arm64/kernel/acpi-blacklist.c
> new file mode 100644
> index 0000000..1be6a56
> --- /dev/null
> +++ b/arch/arm64/kernel/acpi-blacklist.c
> @@ -0,0 +1,20 @@
> +/*
> + *  ARM64 Specific ACPI Blacklist Support
> + *
> + *  Copyright (C) 2015, Linaro Ltd.
> + *	Author: Al Stone <al.stone@linaro.org>
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License version 2 as
> + *  published by the Free Software Foundation.
> + */
> +
> +#define pr_fmt(fmt) "ACPI: " fmt
> +
> +#include <linux/acpi.h>
> +
> +/* The arm64 ACPI blacklist is currently empty.  */
> +int __init acpi_blacklisted(void)
> +{
> +	return 0;
> +}
> diff --git a/arch/arm64/kernel/acpi-osi.c b/arch/arm64/kernel/acpi-osi.c
> new file mode 100644
> index 0000000..bb351f4
> --- /dev/null
> +++ b/arch/arm64/kernel/acpi-osi.c
> @@ -0,0 +1,25 @@
> +/*
> + *  ARM64 Specific ACPI _OSI Support
> + *
> + *  Copyright (C) 2015, Linaro Ltd.
> + *	Author: Al Stone <al.stone@linaro.org>
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License version 2 as
> + *  published by the Free Software Foundation.
> + */
> +
> +#define pr_fmt(fmt) "ACPI: " fmt
> +
> +#include <linux/acpi.h>
> +
> +/*
> + * Consensus is to deprecate _OSI for all new ACPI-supported architectures.
> + * So, for arm64, reduce _OSI to a warning message, and tell the firmware
> + * nothing of value.
> + */
> +u32 acpi_osi_handler(acpi_string interface, u32 supported)
> +{
> +	pr_warn("_OSI was called, but is deprecated for this architecture.\n");

I'd prefer "ACPI _OSI is not implemented for this architecture".

> +	return false;
> +}
> 

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH v3 9/9] ACPI: arm64: use "Linux" as ACPI_OS_NAME for _OS on arm64

[Rafael J. Wysocki]

On Tuesday, February 24, 2015 05:36:25 PM al.stone@linaro.org wrote:
> From: Al Stone <al.stone@linaro.org>
> 
> ACPI_OS_NAME is globally defined as "Microsoft Windows NT" for now.
> That doesn't make much sense in the ARM context, so set it to "Linux"
> when CONFIG_ARM64.
> 
> If it is necessary to change the return value from \_OS_ (that is, return
> some value other than the default in ACPI_OS_NAME), use the kernel parameter
> "acpi_os_name=<string>".
> 
> Many thanks to Rafael Wysocki for this greatly simplified form of the patch.
> 
> Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
> Signed-off-by: Al Stone <al.stone@linaro.org>

Yup, fine by me.

> ---
>  arch/arm64/Kconfig              | 5 +++++
>  include/acpi/acconfig.h         | 2 ++
>  include/acpi/platform/aclinux.h | 4 ++++
>  3 files changed, 11 insertions(+)
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index e441d28..d812113 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -87,6 +87,11 @@ config ARM64
>  config 64BIT
>  	def_bool y
>  
> +config ACPI_OS_NAME
> +	string
> +	default "Linux"
> +	depends on ACPI
> +
>  config ARCH_PHYS_ADDR_T_64BIT
>  	def_bool y
>  
> diff --git a/include/acpi/acconfig.h b/include/acpi/acconfig.h
> index 5a0a3e5..1980bf4 100644
> --- a/include/acpi/acconfig.h
> +++ b/include/acpi/acconfig.h
> @@ -69,7 +69,9 @@
>   * code that will not execute the _OSI method unless _OS matches the string
>   * below.  Therefore, change this string at your own risk.
>   */
> +#ifndef ACPI_OS_NAME
>  #define ACPI_OS_NAME                    "Microsoft Windows NT"
> +#endif
>  
>  /* Maximum objects in the various object caches */
>  
> diff --git a/include/acpi/platform/aclinux.h b/include/acpi/platform/aclinux.h
> index 1ba7c19..a871cdd 100644
> --- a/include/acpi/platform/aclinux.h
> +++ b/include/acpi/platform/aclinux.h
> @@ -69,6 +69,10 @@
>  #define ACPI_REDUCED_HARDWARE 1
>  #endif
>  
> +#ifdef CONFIG_ACPI_OS_NAME
> +#define ACPI_OS_NAME CONFIG_ACPI_OS_NAME
> +#endif
> +
>  #include <linux/string.h>
>  #include <linux/kernel.h>
>  #include <linux/ctype.h>
> 

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH v3 1/9] ACPI: fix all errors reported by cleanpatch.pl in osl.c

[Al Stone]

On 03/04/2015 04:04 PM, Rafael J. Wysocki wrote:
> On Tuesday, February 24, 2015 05:36:17 PM al.stone@linaro.org wrote:
>> From: Al Stone <al.stone@linaro.org>
>>
>> In preparation for later splitting out some of the arch-dependent code from
>> osl.c, clean up the errors reported by checkpatch.pl.  They fell into these
>> classes:
>>
>>    -- remove the FSF address from the GPL notice
>>    -- "foo * bar" should be "foo *bar" (and the ** variation of same)
>>    -- a return is not a function, so parentheses are not required.
>>
>> Signed-off-by: Al Stone <al.stone@linaro.org>
> 
> checkpatch.pl is irrelevant here.  You're trying to make the coding style be
> more consistent with the coding style of the rest of the kernel.
> 
> The warnings from checkpatch.pl are meaningless for the existing code, so
> it should not be used to justify changes in that code.
> 
> Of course, the same applies to patches [2-4/9].
> 
> 

Okay, I'm puzzled.  In the last version of these patches, I asked if I
should clean up osl.c as long as I was creating the new osi.c file.  I
understood the reply to mean it would also be good to correct osl.c [0]
from checkpatch's point of view.  I took that to mean errors (patch [1/9])
and warnings (patches [2-4/9]) -- so that's what I did.  What did I
misunderstand from that reply?

If these changes are objectionable, then I'll drop these from the next
version of the patch set; I'm not hung up on insisting on either of the
kernel's or ACPI's coding style -- I try to adapt as needed.  I only did
the patches because I thought it was helping out with some long-term
maintenance type work.


[0] https://lkml.org/lkml/2015/2/4/749

-- 
ciao,
al
-----------------------------------
Al Stone
Software Engineer
Red Hat, Inc.
ahs3@redhat.com
-----------------------------------

Re: [PATCH v3 1/9] ACPI: fix all errors reported by cleanpatch.pl in osl.c

[Al Stone]

On 03/04/2015 05:25 PM, Rafael J. Wysocki wrote:
> On Wednesday, March 04, 2015 04:56:12 PM Al Stone wrote:
>> On 03/04/2015 04:04 PM, Rafael J. Wysocki wrote:
>>> On Tuesday, February 24, 2015 05:36:17 PM al.stone@linaro.org wrote:
>>>> From: Al Stone <al.stone@linaro.org>
>>>>
>>>> In preparation for later splitting out some of the arch-dependent code from
>>>> osl.c, clean up the errors reported by checkpatch.pl.  They fell into these
>>>> classes:
>>>>
>>>>    -- remove the FSF address from the GPL notice
>>>>    -- "foo * bar" should be "foo *bar" (and the ** variation of same)
>>>>    -- a return is not a function, so parentheses are not required.
>>>>
>>>> Signed-off-by: Al Stone <al.stone@linaro.org>
>>>
>>> checkpatch.pl is irrelevant here.  You're trying to make the coding style be
>>> more consistent with the coding style of the rest of the kernel.
>>>
>>> The warnings from checkpatch.pl are meaningless for the existing code, so
>>> it should not be used to justify changes in that code.
>>>
>>> Of course, the same applies to patches [2-4/9].
>>>
>>>
>>
>> Okay, I'm puzzled.  In the last version of these patches, I asked if I
>> should clean up osl.c as long as I was creating the new osi.c file.  I
>> understood the reply to mean it would also be good to correct osl.c [0]
>> from checkpatch's point of view.  I took that to mean errors (patch [1/9])
>> and warnings (patches [2-4/9]) -- so that's what I did.  What did I
>> misunderstand from that reply?
>>
>> If these changes are objectionable, then I'll drop these from the next
>> version of the patch set; I'm not hung up on insisting on either of the
>> kernel's or ACPI's coding style -- I try to adapt as needed.  I only did
>> the patches because I thought it was helping out with some long-term
>> maintenance type work.
> 
> The changes are basically OK, but the justification is bogus to me.
> "I'm making the chagne, because checkpatch.pl told me so" is a pretty bad
> explanation in my view.  It is much better to say "This file does not
> adhere to the general kernel coding style and since I'm going to split it
> into pieces and I want those pieces to follow the coding style more closely,
> make changes as follows."
> 
> So this is more about the changelogs (and subjects) than the code changes
> themselves.

Aha.  That makes much more sense to me.  Sorry if I was being a bit dense;
I'll rev these for the next version so it's far clearer.  Thanks for being
patient :).

-- 
ciao,
al
-----------------------------------
Al Stone
Software Engineer
Red Hat, Inc.
ahs3@redhat.com
-----------------------------------

Re: [PATCH v3 1/9] ACPI: fix all errors reported by cleanpatch.pl in osl.c

[Rafael J. Wysocki]

On Wednesday, March 04, 2015 04:56:12 PM Al Stone wrote:
> On 03/04/2015 04:04 PM, Rafael J. Wysocki wrote:
> > On Tuesday, February 24, 2015 05:36:17 PM al.stone@linaro.org wrote:
> >> From: Al Stone <al.stone@linaro.org>
> >>
> >> In preparation for later splitting out some of the arch-dependent code from
> >> osl.c, clean up the errors reported by checkpatch.pl.  They fell into these
> >> classes:
> >>
> >>    -- remove the FSF address from the GPL notice
> >>    -- "foo * bar" should be "foo *bar" (and the ** variation of same)
> >>    -- a return is not a function, so parentheses are not required.
> >>
> >> Signed-off-by: Al Stone <al.stone@linaro.org>
> > 
> > checkpatch.pl is irrelevant here.  You're trying to make the coding style be
> > more consistent with the coding style of the rest of the kernel.
> > 
> > The warnings from checkpatch.pl are meaningless for the existing code, so
> > it should not be used to justify changes in that code.
> > 
> > Of course, the same applies to patches [2-4/9].
> > 
> > 
> 
> Okay, I'm puzzled.  In the last version of these patches, I asked if I
> should clean up osl.c as long as I was creating the new osi.c file.  I
> understood the reply to mean it would also be good to correct osl.c [0]
> from checkpatch's point of view.  I took that to mean errors (patch [1/9])
> and warnings (patches [2-4/9]) -- so that's what I did.  What did I
> misunderstand from that reply?
> 
> If these changes are objectionable, then I'll drop these from the next
> version of the patch set; I'm not hung up on insisting on either of the
> kernel's or ACPI's coding style -- I try to adapt as needed.  I only did
> the patches because I thought it was helping out with some long-term
> maintenance type work.

The changes are basically OK, but the justification is bogus to me.
"I'm making the chagne, because checkpatch.pl told me so" is a pretty bad
explanation in my view.  It is much better to say "This file does not
adhere to the general kernel coding style and since I'm going to split it
into pieces and I want those pieces to follow the coding style more closely,
make changes as follows."

So this is more about the changelogs (and subjects) than the code changes
themselves.


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH v3 8/9] ACPI: arm64: use an arch-specific ACPI _OSI method and ACPI blacklist

[Will Deacon]

On Wed, Mar 04, 2015 at 11:14:50PM +0000, Rafael J. Wysocki wrote:
> On Monday, March 02, 2015 12:00:21 PM Al Stone wrote:
> > On 03/02/2015 10:29 AM, Will Deacon wrote:
> > > On Wed, Feb 25, 2015 at 12:36:24AM +0000, al.stone@linaro.org wrote:
> > >> diff --git a/arch/arm64/kernel/acpi-blacklist.c b/arch/arm64/kernel/acpi-blacklist.c
> > >> new file mode 100644
> > >> index 0000000..1be6a56
> > >> --- /dev/null
> > >> +++ b/arch/arm64/kernel/acpi-blacklist.c
> > >> @@ -0,0 +1,20 @@
> > >> +/*
> > >> + *  ARM64 Specific ACPI Blacklist Support
> > >> + *
> > >> + *  Copyright (C) 2015, Linaro Ltd.
> > >> + *	Author: Al Stone <al.stone@linaro.org>
> > >> + *
> > >> + *  This program is free software; you can redistribute it and/or modify
> > >> + *  it under the terms of the GNU General Public License version 2 as
> > >> + *  published by the Free Software Foundation.
> > >> + */
> > >> +
> > >> +#define pr_fmt(fmt) "ACPI: " fmt
> > >> +
> > >> +#include <linux/acpi.h>
> > >> +
> > >> +/* The arm64 ACPI blacklist is currently empty.  */
> > >> +int __init acpi_blacklisted(void)
> > >> +{
> > >> +	return 0;
> > >> +}
> > >> diff --git a/arch/arm64/kernel/acpi-osi.c b/arch/arm64/kernel/acpi-osi.c
> > >> new file mode 100644
> > >> index 0000000..bb351f4
> > >> --- /dev/null
> > >> +++ b/arch/arm64/kernel/acpi-osi.c
> > >> @@ -0,0 +1,25 @@
> > >> +/*
> > >> + *  ARM64 Specific ACPI _OSI Support
> > >> + *
> > >> + *  Copyright (C) 2015, Linaro Ltd.
> > >> + *	Author: Al Stone <al.stone@linaro.org>
> > >> + *
> > >> + *  This program is free software; you can redistribute it and/or modify
> > >> + *  it under the terms of the GNU General Public License version 2 as
> > >> + *  published by the Free Software Foundation.
> > >> + */
> > >> +
> > >> +#define pr_fmt(fmt) "ACPI: " fmt
> > >> +
> > >> +#include <linux/acpi.h>
> > >> +
> > >> +/*
> > >> + * Consensus is to deprecate _OSI for all new ACPI-supported architectures.
> > >> + * So, for arm64, reduce _OSI to a warning message, and tell the firmware
> > >> + * nothing of value.
> > >> + */
> > >> +u32 acpi_osi_handler(acpi_string interface, u32 supported)
> > >> +{
> > >> +	pr_warn("_OSI was called, but is deprecated for this architecture.\n");
> > >> +	return false;
> > >> +}
> > > 
> > > This kinda feels backwards to me. If _OSI is going away, then the default
> > > should be "the architecture doesn't need to do anything", rather than have
> > > new architectures defining a bunch of empty, useless stub code.
> > > 
> > > Anyway we could make this the default in core code and have architectures
> > > that *do* want _OSI override that behaviour, instead of the other way around?
> > > 
> > We could do that; I personally don't have a strong preference either way,
> > so I'm inclined to make it whatever structure Rafael thinks is proper since
> > it affects ACPI code most.  That being said, the current patch structure
> > made sense to me since it wasn't distorting existing code much -- and given
> > the pure number of x86/ia64 machines vs ARM machines using ACPI, that seemed
> > the more cautious approach.
> > 
> > @Rafael: do you have an opinion/preference?
> 
> My preference is to avoid changes in the existing code at least for the time
> being.  Especially if the changes in question are going to affect ia64, unless
> you have an Itanium machine where you can readily test those, that is. :-)

Well, this code doesn't even need to compiled for ia64 if we have those
architectures that want to use _OSI select a Kconfig symbol for it, so I
don't think the testing argument is really that valid. I appreciate that you
want to avoid changing the existing code, but I also don't want to add this
sort of stuff to the architecture code, when it really has nothing to do
with the architecture.

Will

Re: [PATCH v3 8/9] ACPI: arm64: use an arch-specific ACPI _OSI method and ACPI blacklist

[Rafael J. Wysocki]

On Thu, Mar 5, 2015 at 11:17 AM, Will Deacon <will.deacon@arm.com> wrote:
> On Wed, Mar 04, 2015 at 11:14:50PM +0000, Rafael J. Wysocki wrote:
>> On Monday, March 02, 2015 12:00:21 PM Al Stone wrote:
>> > On 03/02/2015 10:29 AM, Will Deacon wrote:
>> > > On Wed, Feb 25, 2015 at 12:36:24AM +0000, al.stone@linaro.org wrote:
>> > >> diff --git a/arch/arm64/kernel/acpi-blacklist.c b/arch/arm64/kernel/acpi-blacklist.c
>> > >> new file mode 100644
>> > >> index 0000000..1be6a56
>> > >> --- /dev/null
>> > >> +++ b/arch/arm64/kernel/acpi-blacklist.c
>> > >> @@ -0,0 +1,20 @@
>> > >> +/*
>> > >> + *  ARM64 Specific ACPI Blacklist Support
>> > >> + *
>> > >> + *  Copyright (C) 2015, Linaro Ltd.
>> > >> + *      Author: Al Stone <al.stone@linaro.org>
>> > >> + *
>> > >> + *  This program is free software; you can redistribute it and/or modify
>> > >> + *  it under the terms of the GNU General Public License version 2 as
>> > >> + *  published by the Free Software Foundation.
>> > >> + */
>> > >> +
>> > >> +#define pr_fmt(fmt) "ACPI: " fmt
>> > >> +
>> > >> +#include <linux/acpi.h>
>> > >> +
>> > >> +/* The arm64 ACPI blacklist is currently empty.  */
>> > >> +int __init acpi_blacklisted(void)
>> > >> +{
>> > >> +        return 0;
>> > >> +}
>> > >> diff --git a/arch/arm64/kernel/acpi-osi.c b/arch/arm64/kernel/acpi-osi.c
>> > >> new file mode 100644
>> > >> index 0000000..bb351f4
>> > >> --- /dev/null
>> > >> +++ b/arch/arm64/kernel/acpi-osi.c
>> > >> @@ -0,0 +1,25 @@
>> > >> +/*
>> > >> + *  ARM64 Specific ACPI _OSI Support
>> > >> + *
>> > >> + *  Copyright (C) 2015, Linaro Ltd.
>> > >> + *      Author: Al Stone <al.stone@linaro.org>
>> > >> + *
>> > >> + *  This program is free software; you can redistribute it and/or modify
>> > >> + *  it under the terms of the GNU General Public License version 2 as
>> > >> + *  published by the Free Software Foundation.
>> > >> + */
>> > >> +
>> > >> +#define pr_fmt(fmt) "ACPI: " fmt
>> > >> +
>> > >> +#include <linux/acpi.h>
>> > >> +
>> > >> +/*
>> > >> + * Consensus is to deprecate _OSI for all new ACPI-supported architectures.
>> > >> + * So, for arm64, reduce _OSI to a warning message, and tell the firmware
>> > >> + * nothing of value.
>> > >> + */
>> > >> +u32 acpi_osi_handler(acpi_string interface, u32 supported)
>> > >> +{
>> > >> +        pr_warn("_OSI was called, but is deprecated for this architecture.\n");
>> > >> +        return false;
>> > >> +}
>> > >
>> > > This kinda feels backwards to me. If _OSI is going away, then the default
>> > > should be "the architecture doesn't need to do anything", rather than have
>> > > new architectures defining a bunch of empty, useless stub code.
>> > >
>> > > Anyway we could make this the default in core code and have architectures
>> > > that *do* want _OSI override that behaviour, instead of the other way around?
>> > >
>> > We could do that; I personally don't have a strong preference either way,
>> > so I'm inclined to make it whatever structure Rafael thinks is proper since
>> > it affects ACPI code most.  That being said, the current patch structure
>> > made sense to me since it wasn't distorting existing code much -- and given
>> > the pure number of x86/ia64 machines vs ARM machines using ACPI, that seemed
>> > the more cautious approach.
>> >
>> > @Rafael: do you have an opinion/preference?
>>
>> My preference is to avoid changes in the existing code at least for the time
>> being.  Especially if the changes in question are going to affect ia64, unless
>> you have an Itanium machine where you can readily test those, that is. :-)
>
> Well, this code doesn't even need to compiled for ia64 if we have those
> architectures that want to use _OSI select a Kconfig symbol for it, so I
> don't think the testing argument is really that valid. I appreciate that you
> want to avoid changing the existing code, but I also don't want to add this
> sort of stuff to the architecture code, when it really has nothing to do
> with the architecture.

OK, so consider this.

_OSI may be deprecated in the spec for *new* implementations.

However, there still are many systems out there that use _OSI and
we'll need to support
them going forward.  So while the spec people may think that they have
deprecated
_OSI, the reality is that in the kernel it is not going to be
deprecated as long as there
are systems using it that we need to support.

So the whole "_OSI is going away" argument is simply bogus and useless.

That aside, yes, we can use a Kconfig symbol to select from x86 and ia64
and compile the generic code conditional on that.  That would be fine by me.

Rafael