Re: [PATCH 1/3] remoteproc: Add Arm remoteproc driver

[Rob Herring]

On Fri, Mar 01, 2024 at 04:42:25PM +0000, abdellatif.elkhlifi@xxxxxxx wrote:
> From: Abdellatif El Khlifi <abdellatif.elkhlifi@xxxxxxx>
> 
> introduce remoteproc support for Arm remote processors
> 
> The supported remote processors are those that come with a reset
> control register and a reset status register. The driver allows to
> switch on or off the remote processor.
> 
> The current use case is Corstone-1000 External System (Cortex-M3).
> 
> The driver can be extended to support other remote processors
> controlled with a reset control and a reset status registers.
> 
> The driver also supports control of multiple remote processors at the
> same time.
> 
> Signed-off-by: Abdellatif El Khlifi <abdellatif.elkhlifi@xxxxxxx>
> ---
>  MAINTAINERS                    |   6 +
>  drivers/remoteproc/Kconfig     |  18 ++
>  drivers/remoteproc/Makefile    |   1 +
>  drivers/remoteproc/arm_rproc.c | 395 +++++++++++++++++++++++++++++++++
>  4 files changed, 420 insertions(+)
>  create mode 100644 drivers/remoteproc/arm_rproc.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 8d1052fa6a69..54d6a40feea5 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1764,6 +1764,12 @@ S:	Maintained
>  F:	Documentation/devicetree/bindings/interrupt-controller/arm,vic.yaml
>  F:	drivers/irqchip/irq-vic.c
>  
> +ARM REMOTEPROC DRIVER
> +M:	Abdellatif El Khlifi <abdellatif.elkhlifi@xxxxxxx>
> +L:	linux-remoteproc@xxxxxxxxxxxxxxx
> +S:	Maintained
> +F:	drivers/remoteproc/arm_rproc.c
> +
>  ARM SMC WATCHDOG DRIVER
>  M:	Julius Werner <jwerner@xxxxxxxxxxxx>
>  R:	Evan Benn <evanbenn@xxxxxxxxxxxx>
> diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
> index 48845dc8fa85..57fbac454a5d 100644
> --- a/drivers/remoteproc/Kconfig
> +++ b/drivers/remoteproc/Kconfig
> @@ -365,6 +365,24 @@ config XLNX_R5_REMOTEPROC
>  
>  	  It's safe to say N if not interested in using RPU r5f cores.
>  
> +config ARM_REMOTEPROC
> +	tristate "Arm remoteproc support"

Too generic of a name. It should say Corstone or Corstone-1000. Here and 
everywhere you use just 'Arm'.

> +	depends on HAS_IOMEM && ARM64

depends on ARM64 || (HAS_IOMEM && COMPILE_TEST)

That gets us wider build coverage. You should check at least x86 
allmodconfig passes.

> +	default n

The default is already n, so drop.

> +	help
> +	  Say y here to support Arm remote processors via the remote
> +	  processor framework.
> +
> +	  The supported processors are those that come with a reset control register
> +	  and a reset status register. The design can be extended to support different
> +	  processors meeting these requirements.
> +	  The driver also supports control of multiple remote cores at the same time.
> +
> +	  Supported remote cores:
> +	      Corstone-1000 External System (Cortex-M3)
> +
> +	  It's safe to say N here.
> +
>  endif # REMOTEPROC
>  
>  endmenu
> diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
> index 91314a9b43ce..73126310835b 100644
> --- a/drivers/remoteproc/Makefile
> +++ b/drivers/remoteproc/Makefile
> @@ -39,3 +39,4 @@ obj-$(CONFIG_STM32_RPROC)		+= stm32_rproc.o
>  obj-$(CONFIG_TI_K3_DSP_REMOTEPROC)	+= ti_k3_dsp_remoteproc.o
>  obj-$(CONFIG_TI_K3_R5_REMOTEPROC)	+= ti_k3_r5_remoteproc.o
>  obj-$(CONFIG_XLNX_R5_REMOTEPROC)	+= xlnx_r5_remoteproc.o
> +obj-$(CONFIG_ARM_REMOTEPROC)		+= arm_rproc.o
> diff --git a/drivers/remoteproc/arm_rproc.c b/drivers/remoteproc/arm_rproc.c
> new file mode 100644
> index 000000000000..6afa78ae7ad3
> --- /dev/null
> +++ b/drivers/remoteproc/arm_rproc.c
> @@ -0,0 +1,395 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright 2024 Arm Limited and/or its affiliates <open-source-office@xxxxxxx>

We don't normally put OSO email in here.

> + *
> + * Authors:
> + *   Abdellatif El Khlifi <abdellatif.elkhlifi@xxxxxxx>

That's recorded in the commit message and by git, so no need to put it 
in the file.

> + */