Re: [PATCH] ACPI: rename acpi_arm_init to acpi_arch_init

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2024/7/27 2:23, Miao Wang wrote:

2024年7月27日 01:55,Sunil V L <sunilvl@xxxxxxxxxxxxxxxx> 写道:

On Sat, Jul 27, 2024 at 12:39:03AM +0800, Miao Wang wrote:
Hi,

Thanks for your quick reply.

2024年7月27日 00:05,Sudeep Holla <sudeep.holla@xxxxxxx> 写道:

On Fri, Jul 26, 2024 at 11:03:01PM +0800, Miao Wang via B4 Relay wrote:
[...]

-#ifdef CONFIG_ARM64
-void acpi_arm_init(void);
+#ifdef ACPI_HAVE_ARCH_INIT
+void acpi_arch_init(void);

This is bit inconsistent. The Makefile is still conditional on
CONFIG_ARM64 while here you move to ACPI_HAVE_ARCH_INIT.
So while not just undefine and redefine acpi_arch_init to acpi_arm_init.
Something like this must work ?

#define acpi_arch_init() do { }while(0)

#ifdef CONFIG_ARM64
#undef acpi_arch_init
#define acpi_arch_init() acpi_arm_init()
#endif

It will work. However I can see the pattern in other parts, where
the definition of a macro named HAVE_xxx is checked, and define an
inline static function with empty body if such macro is not defined
or define a function prototype with the same name otherwise, like
acpi_arch_set_root_pointer. I'm just trying to follow this pattern.

I was thinking to make it weak function similar to cpc_read_ffh().
Wouldn't it be better than ifdefery?

I believe there would be performance loss for those arches with a stub
function definition if a weak function is used (correct me if wrong).
So the approach with a static inline stub is more common in the kernel
code.

ACPI init is not in the hot code path, no worries for the performance
loss.

Weak function is my preference too :)

Thanks
Hanjun




[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]
  Powered by Linux