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:
>> From: Miao Wang <shankerwangmiao@xxxxxxxxx>
>>
>> So that we avoid arch-specific code in general ACPI initialization flow.
>> Other architectures can also have chance to define their own
>> arch-specific acpi initialization process if necessary.
>>
>
> Nice, but I assume you are adding something similar to another arch(riscv
> or loongarch ?). It would be nice to have those changes as well together to
> make it easy to understand the intention much quicker.
Yes, you are right about it. I'm trying to add some codes for loongarch,
after DSDT is loaded and namespace is created, before the devices are
enumerated, so I'll have chance to add a _DEP method to one of the device
using acpi_install_method to provide compatibility for some early loongarch
devices which are produced before the loongarch related ACPI standard is
finalized.
>
>> Signed-off-by: Miao Wang <shankerwangmiao@xxxxxxxxx>
>> ---
>> arch/arm64/include/asm/acpi.h | 2 ++
>> drivers/acpi/arm64/init.c | 2 +-
>> drivers/acpi/bus.c | 2 +-
>> include/linux/acpi.h | 6 +++---
>> 4 files changed, 7 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
>> index a407f9cd549e..0d24e920e143 100644
>> --- a/arch/arm64/include/asm/acpi.h
>> +++ b/arch/arm64/include/asm/acpi.h
>> @@ -188,4 +188,6 @@ static inline void acpi_map_cpus_to_nodes(void) { }
>>
>> #define ACPI_TABLE_UPGRADE_MAX_PHYS MEMBLOCK_ALLOC_ACCESSIBLE
>>
>> +#define ACPI_HAVE_ARCH_INIT
>> +
>
> There is nothing core arm66 arch specific in acpi_arm_init() and hence it
> is in drivers/acpi/arm64. I would like to avoid adding anything in arch/arm64
> if possible. Also I don't think we need to define this ACPI_HAVE_ARCH_INIT
>
>> #endif /*_ASM_ACPI_H*/
>> diff --git a/drivers/acpi/arm64/init.c b/drivers/acpi/arm64/init.c
>> index d0c8aed90fd1..7a47d8095a7d 100644
>> --- a/drivers/acpi/arm64/init.c
>> +++ b/drivers/acpi/arm64/init.c
>> @@ -2,7 +2,7 @@
>> #include <linux/acpi.h>
>> #include "init.h"
>>
>> -void __init acpi_arm_init(void)
>> +void __init acpi_arch_init(void)
>
> Keep the name acpi_arm_init as is.
>
>> {
>> if (IS_ENABLED(CONFIG_ACPI_AGDI))
>> acpi_agdi_init();
>> diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
>> index 284bc2e03580..662f69e379ef 100644
>> --- a/drivers/acpi/bus.c
>> +++ b/drivers/acpi/bus.c
>> @@ -1458,7 +1458,7 @@ static int __init acpi_init(void)
>> acpi_viot_early_init();
>> acpi_hest_init();
>> acpi_ghes_init();
>> - acpi_arm_init();
>> + acpi_arch_init();
>
> Here we need acpi_arch_init() like you have changed.
>
>> acpi_scan_init();
>> acpi_ec_init();
>> acpi_debugfs_init();
>> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
>> index f0b95c76c707..3c3a83499c2d 100644
>> --- a/include/linux/acpi.h
>> +++ b/include/linux/acpi.h
>> @@ -1517,10 +1517,10 @@ static inline int find_acpi_cpu_topology_hetero_id(unsigned int cpu)
>> }
>> #endif
>>
>> -#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.
> --
> Regards,
> Sudeep
