Re: [PATCH 2/6] remoteproc/pru: Add a PRU remoteproc driver

2020-11-18 Thread Grzegorz Jaszczyk
Hi Suman,

On Tue, 17 Nov 2020 at 20:41, Suman Anna  wrote:
>
> Hi Greg,
>
> I have a few minor comments below..
>
> On 11/14/20 2:46 AM, Grzegorz Jaszczyk wrote:
> > From: Suman Anna 
> >
> > The Programmable Real-Time Unit Subsystem (PRUSS) consists of
> > dual 32-bit RISC cores (Programmable Real-Time Units, or PRUs)
> > for program execution. This patch adds a remoteproc platform
> > driver for managing the individual PRU RISC cores life cycle.
> >
> > The PRUs do not have a unified address space (have an Instruction
> > RAM and a primary Data RAM at both 0x0). The PRU remoteproc driver
> > therefore uses a custom remoteproc core ELF loader ops. The added
> > .da_to_va ops is only used to provide translations for the PRU
> > Data RAMs. This remoteproc driver does not have support for error
> > recovery and system suspend/resume features. Different compatibles
> > are used to allow providing scalability for instance-specific device
> > data if needed. The driver uses a default firmware-name retrieved
> > from device-tree for each PRU core, and the firmwares are expected
> > to be present in the standard Linux firmware search paths. They can
> > also be adjusted by userspace if required through the sysfs interface
> > provided by the remoteproc core.
> >
> > The PRU remoteproc driver uses a client-driven boot methodology: it
> > does _not_ support auto-boot so that the PRU load and boot is dictated
> > by the corresponding client drivers for achieving various usecases.
> > This allows flexibility for the client drivers or applications to set
> > a firmware name (if needed) based on their desired functionality and
> > boot the PRU. The sysfs bind and unbind attributes have also been
> > suppressed so that the PRU devices cannot be unbound and thereby
> > shutdown a PRU from underneath a PRU client driver.
> >
> > The driver currently supports the AM335x, AM437x, AM57xx and 66AK2G
> > SoCs, and support for other TI SoCs will be added in subsequent
> > patches.
> >
> > Co-developed-by: Andrew F. Davis 
> > Signed-off-by: Andrew F. Davis 
> > Signed-off-by: Suman Anna 
> > Co-developed-by: Grzegorz Jaszczyk 
> > Signed-off-by: Grzegorz Jaszczyk 
> > ---
> >  drivers/remoteproc/Kconfig |  12 +
> >  drivers/remoteproc/Makefile|   1 +
> >  drivers/remoteproc/pru_rproc.c | 428 +
> >  3 files changed, 441 insertions(+)
> >  create mode 100644 drivers/remoteproc/pru_rproc.c
> >
> > diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
> > index d99548fb5dde..3e3865a7cd78 100644
> > --- a/drivers/remoteproc/Kconfig
> > +++ b/drivers/remoteproc/Kconfig
> > @@ -125,6 +125,18 @@ config KEYSTONE_REMOTEPROC
> > It's safe to say N here if you're not interested in the Keystone
> > DSPs or just want to use a bare minimum kernel.
> >
> > +config PRU_REMOTEPROC
> > + tristate "TI PRU remoteproc support"
> > + depends on TI_PRUSS
> > + default TI_PRUSS
> > + help
> > +   Support for TI PRU remote processors present within a PRU-ICSS
> > +   subsystem via the remote processor framework.
> > +
> > +   Say Y or M here to support the Programmable Realtime Unit (PRU)
> > +   processors on various TI SoCs. It's safe to say N here if you're
> > +   not interested in the PRU or if you are unsure.
> > +
> >  config QCOM_PIL_INFO
> >   tristate
> >
> > diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
> > index da2ace4ec86c..bb26c9e4ef9c 100644
> > --- a/drivers/remoteproc/Makefile
> > +++ b/drivers/remoteproc/Makefile
> > @@ -18,6 +18,7 @@ obj-$(CONFIG_OMAP_REMOTEPROC)   += 
> > omap_remoteproc.o
> >  obj-$(CONFIG_WKUP_M3_RPROC)  += wkup_m3_rproc.o
> >  obj-$(CONFIG_DA8XX_REMOTEPROC)   += da8xx_remoteproc.o
> >  obj-$(CONFIG_KEYSTONE_REMOTEPROC)+= keystone_remoteproc.o
> > +obj-$(CONFIG_PRU_REMOTEPROC) += pru_rproc.o
> >  obj-$(CONFIG_QCOM_PIL_INFO)  += qcom_pil_info.o
> >  obj-$(CONFIG_QCOM_RPROC_COMMON)  += qcom_common.o
> >  obj-$(CONFIG_QCOM_Q6V5_COMMON)   += qcom_q6v5.o
> > diff --git a/drivers/remoteproc/pru_rproc.c b/drivers/remoteproc/pru_rproc.c
> > new file mode 100644
> > index ..c94c8e965c21
> > --- /dev/null
> > +++ b/drivers/remoteproc/pru_rproc.c
> > @@ -0,0 +1,428 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * PRU-ICSS remoteproc driver for various TI SoCs
> > + *
> > + * Copyright (C) 2014-2020 Texas Instruments Incorporated - 
> > https://www.ti.com/
> > + *
> > + * Author(s):
> > + *   Suman Anna 
> > + *   Andrew F. Davis 
> > + *   Grzegorz Jaszczyk  for Texas Instruments
> > + */
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#include "remoteproc_internal.h"
> > +#include "remoteproc_elf_helpers.h"
> > +
> > +/* PRU_ICSS_PRU_CTRL registers */
> > +#define PRU_CTRL_CTRL0x
> > +#define PRU_CTRL_STS 0x0004
> > 

Re: [PATCH 2/6] remoteproc/pru: Add a PRU remoteproc driver

2020-11-17 Thread Suman Anna
Hi Greg,

I have a few minor comments below..

On 11/14/20 2:46 AM, Grzegorz Jaszczyk wrote:
> From: Suman Anna 
> 
> The Programmable Real-Time Unit Subsystem (PRUSS) consists of
> dual 32-bit RISC cores (Programmable Real-Time Units, or PRUs)
> for program execution. This patch adds a remoteproc platform
> driver for managing the individual PRU RISC cores life cycle.
> 
> The PRUs do not have a unified address space (have an Instruction
> RAM and a primary Data RAM at both 0x0). The PRU remoteproc driver
> therefore uses a custom remoteproc core ELF loader ops. The added
> .da_to_va ops is only used to provide translations for the PRU
> Data RAMs. This remoteproc driver does not have support for error
> recovery and system suspend/resume features. Different compatibles
> are used to allow providing scalability for instance-specific device
> data if needed. The driver uses a default firmware-name retrieved
> from device-tree for each PRU core, and the firmwares are expected
> to be present in the standard Linux firmware search paths. They can
> also be adjusted by userspace if required through the sysfs interface
> provided by the remoteproc core.
> 
> The PRU remoteproc driver uses a client-driven boot methodology: it
> does _not_ support auto-boot so that the PRU load and boot is dictated
> by the corresponding client drivers for achieving various usecases.
> This allows flexibility for the client drivers or applications to set
> a firmware name (if needed) based on their desired functionality and
> boot the PRU. The sysfs bind and unbind attributes have also been
> suppressed so that the PRU devices cannot be unbound and thereby
> shutdown a PRU from underneath a PRU client driver.
> 
> The driver currently supports the AM335x, AM437x, AM57xx and 66AK2G
> SoCs, and support for other TI SoCs will be added in subsequent
> patches.
> 
> Co-developed-by: Andrew F. Davis 
> Signed-off-by: Andrew F. Davis 
> Signed-off-by: Suman Anna 
> Co-developed-by: Grzegorz Jaszczyk 
> Signed-off-by: Grzegorz Jaszczyk 
> ---
>  drivers/remoteproc/Kconfig |  12 +
>  drivers/remoteproc/Makefile|   1 +
>  drivers/remoteproc/pru_rproc.c | 428 +
>  3 files changed, 441 insertions(+)
>  create mode 100644 drivers/remoteproc/pru_rproc.c
> 
> diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
> index d99548fb5dde..3e3865a7cd78 100644
> --- a/drivers/remoteproc/Kconfig
> +++ b/drivers/remoteproc/Kconfig
> @@ -125,6 +125,18 @@ config KEYSTONE_REMOTEPROC
> It's safe to say N here if you're not interested in the Keystone
> DSPs or just want to use a bare minimum kernel.
>  
> +config PRU_REMOTEPROC
> + tristate "TI PRU remoteproc support"
> + depends on TI_PRUSS
> + default TI_PRUSS
> + help
> +   Support for TI PRU remote processors present within a PRU-ICSS
> +   subsystem via the remote processor framework.
> +
> +   Say Y or M here to support the Programmable Realtime Unit (PRU)
> +   processors on various TI SoCs. It's safe to say N here if you're
> +   not interested in the PRU or if you are unsure.
> +
>  config QCOM_PIL_INFO
>   tristate
>  
> diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
> index da2ace4ec86c..bb26c9e4ef9c 100644
> --- a/drivers/remoteproc/Makefile
> +++ b/drivers/remoteproc/Makefile
> @@ -18,6 +18,7 @@ obj-$(CONFIG_OMAP_REMOTEPROC)   += 
> omap_remoteproc.o
>  obj-$(CONFIG_WKUP_M3_RPROC)  += wkup_m3_rproc.o
>  obj-$(CONFIG_DA8XX_REMOTEPROC)   += da8xx_remoteproc.o
>  obj-$(CONFIG_KEYSTONE_REMOTEPROC)+= keystone_remoteproc.o
> +obj-$(CONFIG_PRU_REMOTEPROC) += pru_rproc.o
>  obj-$(CONFIG_QCOM_PIL_INFO)  += qcom_pil_info.o
>  obj-$(CONFIG_QCOM_RPROC_COMMON)  += qcom_common.o
>  obj-$(CONFIG_QCOM_Q6V5_COMMON)   += qcom_q6v5.o
> diff --git a/drivers/remoteproc/pru_rproc.c b/drivers/remoteproc/pru_rproc.c
> new file mode 100644
> index ..c94c8e965c21
> --- /dev/null
> +++ b/drivers/remoteproc/pru_rproc.c
> @@ -0,0 +1,428 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * PRU-ICSS remoteproc driver for various TI SoCs
> + *
> + * Copyright (C) 2014-2020 Texas Instruments Incorporated - 
> https://www.ti.com/
> + *
> + * Author(s):
> + *   Suman Anna 
> + *   Andrew F. Davis 
> + *   Grzegorz Jaszczyk  for Texas Instruments
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "remoteproc_internal.h"
> +#include "remoteproc_elf_helpers.h"
> +
> +/* PRU_ICSS_PRU_CTRL registers */
> +#define PRU_CTRL_CTRL0x
> +#define PRU_CTRL_STS 0x0004
> +
> +/* CTRL register bit-fields */
> +#define CTRL_CTRL_SOFT_RST_N BIT(0)
> +#define CTRL_CTRL_EN BIT(1)
> +#define CTRL_CTRL_SLEEPING   BIT(2)
> +#define CTRL_CTRL_CTR_EN BIT(3)
> +#define CTRL_CTRL_SINGLE_STEPBIT(8)
> +#define CTRL_CTRL_RUNSTATE   BIT(15)
> +
>