Re: [PATCH 01/21] arm64: Add ADI ADSP-SC598 SoC

[Arnd Bergmann]

On Thu, Sep 12, 2024, at 18:24, Arturs Artamonovs via B4 Relay wrote:
> From: Arturs Artamonovs <arturs.artamonovs@xxxxxxxxxx>
>
> Add ADSP-SC598 platform.
>

> --- a/arch/arm64/Kconfig.platforms
> +++ b/arch/arm64/Kconfig.platforms
> @@ -292,6 +292,19 @@ config ARCH_ROCKCHIP
>  	  This enables support for the ARMv8 based Rockchip chipsets,
>  	  like the RK3368.
> 
> +config ARCH_SC59X_64
> +	bool "ADI 64-bit SC59X Platforms"
> +	select TIMER_OF
> +	select GPIOLIB
> +	select PINCTRL
> +	select COMMON_CLK_ADI_SC598
> +	select PINCTRL_ADSP
> +	select ADI_ADSP_IRQ
> +	select COUNTER

You can remove the 'select' statements above and just
make your drivers 'default ARCH_SC59X_64'.

It may also help to pick a more generic name for the platform
in case someone wants to add support for SC57x/SC58x later,
assuming these use some of the same drivers,.

The Kconfig change can normally go into the same patch
as the MAINTAINERS file update, but should be separate
from any of the drivers.

> --- /dev/null
> +++ b/drivers/soc/adi/Makefile
> @@ -0,0 +1,5 @@
> +# SPDX-License-Identifier: GPL-2.0
> +
> +# todo modularize; already depends on CONFIG_ARCH_SC59X_64 though
> +
> +obj-y += system.o
> diff --git a/drivers/soc/adi/system.c b/drivers/soc/adi/system.c

I'm confused about the purpose of this driver. Please
split this out into a separate patch and add a detailed
description of how it is actually being used, since it
does not interact with any of the normal subsystems.


> diff --git a/include/linux/soc/adi/adsp-gpio-port.h 
> b/include/linux/soc/adi/adsp-gpio-port.h

> --- /dev/null
> +++ b/include/linux/soc/adi/cpu.h

> --- /dev/null
> +++ b/include/linux/soc/adi/rcu.h
> @@ -0,0 +1,55 @@

> diff --git a/include/linux/soc/adi/sc59x.h 
> b/include/linux/soc/adi/sc59x.h

> --- /dev/null
> +++ b/include/linux/soc/adi/sc59x.h

I don't see these files being included in the driver you add
here, maybe they got added by accident here?

       Arnd