[U-Boot] [PATCH V2 1/4] drivers: Introduce a simplified remoteproc framework

Simon Glass sjg at chromium.org
Wed Sep 2 05:46:37 CEST 2015


Hi Nishanth,

On 27 August 2015 at 22:07, Nishanth Menon <nm at ti.com> wrote:
> Many System on Chip(SoC) solutions are complex with multiple processors
> on the same die dedicated to either general purpose of specialized
> functions. Many examples do exist in today's SoCs from various vendors.
> Typical examples are micro controllers such as an ARM M3/M0 doing a
> offload of specific function such as event integration or power
> management or controlling camera etc.
>
> Traditionally, the responsibility of loading up such a processor with a
> firmware and communication has been with a High Level Operating
> System(HLOS) such as Linux. However, there exists classes of products
> where Linux would need to expect services from such a processor or the
> delay of Linux and operating system being able to load up such a
> firmware is unacceptable.
>
> To address these needs, we need some minimal capability to load such a
> system and ensure it is started prior to an Operating System(Linux or
> any other) is started up.
>
> NOTE: This is NOT meant to be a solve-all solution, instead, it tries to
> address certain class of SoCs and products that need such a solution.
>
> A very simple model is introduced here as part of the initial support
> that supports microcontrollers with internal memory (no MMU, no
> execution from external memory, or specific image format needs). This
> basic framework can then (hopefully) be extensible to other complex SoC
> processor support as need be.
>
> Signed-off-by: Nishanth Menon <nm at ti.com>
> ---
> Changes in V2:
>         - review comments incorporated from v1

Ah yes, but which ones?!

>
> V1: https://patchwork.ozlabs.org/patch/510198/
>
>  common/Kconfig                                     |   5 +
>  common/Makefile                                    |   1 +
>  common/cmd_remoteproc.c                            | 281 ++++++++++++
>  doc/device-tree-bindings/remoteproc/remoteproc.txt |  14 +
>  doc/driver-model/remoteproc-framework.txt          | 168 ++++++++
>  drivers/Kconfig                                    |   2 +
>  drivers/Makefile                                   |   1 +
>  drivers/remoteproc/Kconfig                         |  15 +
>  drivers/remoteproc/Makefile                        |   7 +
>  drivers/remoteproc/rproc-uclass.c                  | 472 +++++++++++++++++++++
>  include/dm/uclass-id.h                             |   1 +
>  include/remoteproc.h                               | 102 +++++
>  12 files changed, 1069 insertions(+)
>  create mode 100644 common/cmd_remoteproc.c
>  create mode 100644 doc/device-tree-bindings/remoteproc/remoteproc.txt
>  create mode 100644 doc/driver-model/remoteproc-framework.txt
>  create mode 100644 drivers/remoteproc/Kconfig
>  create mode 100644 drivers/remoteproc/Makefile
>  create mode 100644 drivers/remoteproc/rproc-uclass.c
>  create mode 100644 include/remoteproc.h
>
[snip]

Reviewed-by: Simon Glass <sjg at chromium.org>

A few nits below.

> diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
> index c744044bb8aa..3c8a4ed78c40 100644
> --- a/include/dm/uclass-id.h
> +++ b/include/dm/uclass-id.h
> @@ -47,6 +47,7 @@ enum uclass_id {
>         UCLASS_PMIC,            /* PMIC I/O device */
>         UCLASS_REGULATOR,       /* Regulator device */
>         UCLASS_RESET,           /* Reset device */
> +       UCLASS_REMOTEPROC,      /* Remote Processor device */
>         UCLASS_RTC,             /* Real time clock device */
>         UCLASS_SERIAL,          /* Serial UART */
>         UCLASS_SPI,             /* SPI bus */
> diff --git a/include/remoteproc.h b/include/remoteproc.h
> new file mode 100644
> index 000000000000..463e52ee28af
> --- /dev/null
> +++ b/include/remoteproc.h
> @@ -0,0 +1,102 @@
> +/*
> + * (C) Copyright 2015
> + * Texas Instruments Incorporated - http://www.ti.com/
> + * SPDX-License-Identifier:    GPL-2.0+
> + */
> +
> +#ifndef _RPROC_H_
> +#define _RPROC_H_
> +
> +/*
> + * XXX XXX XXX
> + * *IMPORTANT* NOTE: THE PLATFORM DATA SUPPORT IS NOT MEANT FOR USE WITH NEWER
> + * PLATFORMS. THIS IS MEANT ONLY FOR LEGACY DEVICES. THIS MODE OF
> + * INITIALIZATION *WILL* BE EVENTUALLY REMOVED ONCE ALL NECESSARY
> + * PLATFORMS HAVE MOVED TO DM/FDT.
> + * XXX XXX XXX
> + */

This can be lower case and you should remove the XXXX stuff.

> +#include <dm/platdata.h>       /* For platform data support - non dt world */
> +
> +/**
> + * enum rproc_mem_type - What type of memory model does the rproc use
> + * @RPROC_INTERNAL_MEMORY_MAPPED: Remote processor uses own memory and is memory
> + *     mapped to the host processor over an address range.
> + *
> + * Please note that this is an enumeration of memory model of different types
> + * of remote processors. Few of the remote processors do have own internal
> + * memories, while others use external memory for instruction and data.
> + */
> +enum rproc_mem_type {
> +       RPROC_INTERNAL_MEMORY_MAPPED    = 0,
> +};
> +
> +/**
> + * struct dm_rproc_uclass_pdata - platform data for a CPU
> + * @name: Platform-specific way of naming the Remote proc
> + * @mem_type: one of 'enum rproc_mem_type'
> + * @driver_plat_data: driver specific platform data that may be needed.
> + *
> + * This can be accessed with dev_get_uclass_platdata() for any UCLASS_REMOTEPROC
> + * device.
> + *
> + */
> +struct dm_rproc_uclass_pdata {
> +       const char *name;
> +       enum rproc_mem_type mem_type;
> +       void *driver_plat_data;
> +};
> +
> +/**
> + * struct dm_rproc_ops - Operations that are provided by remote proc driver
> + * @init:      Initialize the remoteproc device invoked after probe (optional)
> + *             Return 0 on success, -ve error on fail
> + * @load:      Load the remoteproc device using data provided(mandatory)
> + *             This takes the following additional arguments.
> + *                     addr- Address of the binary image to be loaded
> + *                     size- Size of the binary image to be loaded
> + *             Return 0 on success, -ve error on fail
> + * @start:     Start the remoteproc device (mandatory)
> + *             Return 0 on success, -ve error on fail
> + * @stop:      Stop the remoteproc device (optional)
> + *             Return 0 on success, -ve error on fail
> + * @reset:     Reset the remote proc device (optional)
> + *             Return 0 on success, -ve error on fail
> + * @is_running:        Check if the remote processor is running(optional)
> + *             Return 0 on success, 1 if not running, -ve on others errors
> + * @ping:      Ping the remote device for basic communication check(optional)
> + *             Return 0 on success, 1 if not responding, -ve on other errors
> + */
> +struct dm_rproc_ops {
> +       int (*init)(struct udevice *dev);
> +       int (*load)(struct udevice *dev, ulong addr, ulong size);
> +       int (*start)(struct udevice *dev);
> +       int (*stop)(struct udevice *dev);
> +       int (*reset)(struct udevice *dev);
> +       int (*is_running)(struct udevice *dev);
> +       int (*ping)(struct udevice *dev);
> +};
> +
> +/* Accessor */
> +#define rproc_get_ops(dev) ((struct dm_rproc_ops *)(dev)->driver->ops)
> +
> +#ifdef CONFIG_REMOTEPROC
> +int rproc_init(void);
> +bool rproc_is_initialized(void);
> +int rproc_load(int id, ulong addr, ulong size);
> +int rproc_start(int id);
> +int rproc_stop(int id);
> +int rproc_reset(int id);
> +int rproc_ping(int id);
> +int rproc_is_running(int id);

Can you move your function comments to here? This where you define
your API, and it is the file that people will read.

> +#else
> +static inline int rproc_init(void) { return -ENOSYS; }
> +static inline bool rproc_is_initialized(void) { return false; }
> +static inline int rproc_load(int id, ulong addr, ulong size) { return -ENOSYS; }
> +static inline int rproc_start(int id) { return -ENOSYS; }
> +static inline int rproc_stop(int id) { return -ENOSYS; }
> +static inline int rproc_reset(int id) { return -ENOSYS; }
> +static inline int rproc_ping(int id) { return -ENOSYS; }
> +static inline int rproc_is_running(int id) { return -ENOSYS; }
> +#endif
> +
> +#endif /* _RPROC_H_ */
> --
> 2.1.4
>

Regards,
Simon


More information about the U-Boot mailing list