On 01/23/2015 08:43 AM, Rafael J. Wysocki wrote:
Hi Al,
On Thursday, January 22, 2015 05:44:37 PM al.stone@linaro.org wrote:
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. I plan to propose such a change to the ACPI specification in the near future.
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 simply moving the functions implementing _OSI to arch-dependent locations. For x86, there should be no change in functionality. For ia64, while it does duplicate some code from x86, there is no longer any connection to the ACPI blacklist code that is only used by x86. For arm64, any use of the _OSI method generates a warning that it has been deprecated, and then always returns false; i.e., that the capability being queried for, whether OS name or functionality, is not supported. This is the first six of the patches.
While I seem to understand the motivation, I don't really like moving code around, especially in the ia64 area where we have limited means to test the changes. And I hate code duplication pretty much in any form.
I'm not too wild about it either, but it seemed to avoid #ifdefs when I was trying to think it through.
Also I don't see anything wrong with sharing code between x86 and ia64 like we do today.
Fair enough. That's entirely up to you; the only reason I saw for separating them was removing the one call to check the blacklist on ia64. That would have a tiny time savings at boot time, but it is definitely down in the noise level.
So, why don't you move the _OSI handling out of osl.c into a separate file that will only build if something like CONFIG_ARCH_SPECIFIC_ACPI_OSI is not set (and same for blacklist.c). Then, you'll only need to make ARM64 set CONFIG_ARCH_SPECIFIC_ACPI_OSI and provide the necessary functions and you can leave the other architectures alone.
Does that make sense to you?
It does. I'll rewrite these based on that approach. Definitely a lot less code motion involved...
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.
Since we are breaking apart code previously shared, I have tried to make it so that applying the x86 patches alone will continue to compile, at the expense of breaking the build on non-x86 platforms. However, once all of the patches are applied, we should be able to compile on all three architectures. It is best to treat these as one whole.
I have only done simple testing with these patches on arm64 and x86 (AMD Seattle and a Lenovo t440s ThinkPad, respectively). Things seem to work as they should once booted, but this is a very, very small sample of possible machines. The ia64 patches cross-compile, but I personally have no way to test them.
The arm64 patches also rely on having applied Hanjun's patches for ACPI 5.1 on arm64 [0]. The x86 and ia64 parts are not dependent on that patch set, though, and should be usable independently (i.e., patches 1, 3, 4 and 6).
NB: some of the patches do not pass checkpatch.pl; the failures I saw were all part of the original code but are only showing up because that code is changing location, so I have left them as is. If necessary, they could be fixed but I was more concerned about the number of changes needed on ia64 and not having any way to test them.
[0] https://lkml.org/lkml/2015/1/14/586
Al Stone (6): ia64: ACPI: move kernel acpi files to a directory arm64: ACPI: move kernel acpi files to a directory x86: ACPI: create arch-dependent version of acpi_osi_handler() ia64: ACPI: create arch-dependent version of acpi_osi_handler() arm64: ACPI: create arch-dependent version of acpi_osi_handler() x86: ia64: arm64: ACPI: move _OSI support functions to arch-dependent locations
Hanjun Guo (1): ACPI: use Linux as ACPI_OS_NAME for _OS on ARM64
arch/arm64/Kconfig | 1 + arch/arm64/kernel/Makefile | 2 +- arch/arm64/kernel/acpi.c | 359 -------------- arch/arm64/kernel/acpi/Makefile | 1 + arch/arm64/kernel/acpi/acpi.c | 359 ++++++++++++++ arch/arm64/kernel/acpi/osi.c | 26 + arch/ia64/kernel/Makefile | 2 +- arch/ia64/kernel/acpi-ext.c | 104 ---- arch/ia64/kernel/acpi.c | 1000 -------------------------------------- arch/ia64/kernel/acpi/Makefile | 1 + arch/ia64/kernel/acpi/acpi-ext.c | 104 ++++ arch/ia64/kernel/acpi/acpi.c | 1000 ++++++++++++++++++++++++++++++++++++++ arch/ia64/kernel/acpi/osi.c | 119 +++++ arch/x86/kernel/acpi/Makefile | 2 +- arch/x86/kernel/acpi/blacklist.c | 327 +++++++++++++ arch/x86/kernel/acpi/boot.c | 5 +- arch/x86/kernel/acpi/osi.c | 255 ++++++++++ drivers/acpi/Kconfig | 8 + drivers/acpi/Makefile | 1 - drivers/acpi/blacklist.c | 323 ------------ drivers/acpi/osl.c | 217 --------- include/acpi/acconfig.h | 2 + include/acpi/platform/aclinux.h | 4 + include/linux/acpi.h | 4 +- 24 files changed, 2215 insertions(+), 2011 deletions(-) delete mode 100644 arch/arm64/kernel/acpi.c create mode 100644 arch/arm64/kernel/acpi/Makefile create mode 100644 arch/arm64/kernel/acpi/acpi.c create mode 100644 arch/arm64/kernel/acpi/osi.c delete mode 100644 arch/ia64/kernel/acpi-ext.c delete mode 100644 arch/ia64/kernel/acpi.c create mode 100644 arch/ia64/kernel/acpi/Makefile create mode 100644 arch/ia64/kernel/acpi/acpi-ext.c create mode 100644 arch/ia64/kernel/acpi/acpi.c create mode 100644 arch/ia64/kernel/acpi/osi.c create mode 100644 arch/x86/kernel/acpi/blacklist.c create mode 100644 arch/x86/kernel/acpi/osi.c delete mode 100644 drivers/acpi/blacklist.c