Re: [PATCH] ACPI: rename acpi_arm_init to acpi_arch_init

[Sudeep Holla <sudeep.holla@arm.com>]

On Fri, Jul 26, 2024 at 11:03:01PM +0800, Miao Wang via B4 Relay wrote:
> From: Miao Wang <shankerwangmiao@gmail.com>
> 
> 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.

> Signed-off-by: Miao Wang <shankerwangmiao@gmail.com>
> ---
>  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

--
Regards,
Sudeep