RE: [PATCH 04/13] remoteproc: Properly represent the attached state

Peng Fan
2020-10-15T01:33:32Z

> Subject: [PATCH 04/13] remoteproc: Properly represent the attached state
>
> There is a need to know when a remote processor has been attached to
> rather than booted by the remoteproc core. In order to avoid manipulating
> two variables, i.e rproc::autonomous and rproc::state, get rid of the former
> and simply use the newly introduced RPROC_ATTACHED state.
>
> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> ---
> drivers/remoteproc/remoteproc_core.c | 20 +-------------------
> drivers/remoteproc/remoteproc_sysfs.c | 5 +----
> include/linux/remoteproc.h | 2 --
> 3 files changed, 2 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/remoteproc/remoteproc_core.c
> b/drivers/remoteproc/remoteproc_core.c
> index 7d78c9a9d88f..bffaa9ea7c8f 100644
> --- a/drivers/remoteproc/remoteproc_core.c
> +++ b/drivers/remoteproc/remoteproc_core.c
> @@ -1421,7 +1421,7 @@ static int rproc_attach(struct rproc *rproc)
> goto stop_rproc;
> }
>
> - rproc->state = RPROC_RUNNING;
> + rproc->state = RPROC_ATTACHED;
>
> dev_info(dev, "remote processor %s is now attached\n", rproc->name);
>
> @@ -1636,14 +1636,6 @@ static int rproc_stop(struct rproc *rproc, bool
> crashed)
>
> rproc->state = RPROC_OFFLINE;
>
> - /*
> - * The remote processor has been stopped and is now offline, which
> means
> - * that the next time it is brought back online the remoteproc core will
> - * be responsible to load its firmware. As such it is no longer
> - * autonomous.
> - */
> - rproc->autonomous = false;
> -
> dev_info(dev, "stopped remote processor %s\n", rproc->name);
>
> return 0;
> @@ -1994,16 +1986,6 @@ int rproc_add(struct rproc *rproc)
> if (ret < 0)
> return ret;
>
> - /*
> - * Remind ourselves the remote processor has been attached to rather
> - * than booted by the remoteproc core. This is important because the
> - * RPROC_DETACHED state will be lost as soon as the remote processor
> - * has been attached to. Used in firmware_show() and reset in
> - * rproc_stop().
> - */
> - if (rproc->state == RPROC_DETACHED)
> - rproc->autonomous = true;
> -
> /* if rproc is marked always-on, request it to boot */
> if (rproc->auto_boot) {
> ret = rproc_trigger_auto_boot(rproc); diff --git
> a/drivers/remoteproc/remoteproc_sysfs.c
> b/drivers/remoteproc/remoteproc_sysfs.c
> index 2d575e6c9eb8..c152d11a4d3c 100644
> --- a/drivers/remoteproc/remoteproc_sysfs.c
> +++ b/drivers/remoteproc/remoteproc_sysfs.c
> @@ -21,11 +21,8 @@ static ssize_t firmware_show(struct device *dev, struct
> device_attribute *attr,
> * If the remote processor has been started by an external
> * entity we have no idea of what image it is running. As such
> * simply display a generic string rather then rproc->firmware.
> - *
> - * Here we rely on the autonomous flag because a remote processor
> - * may have been attached to and currently in a running state.
> */
> - if (rproc->autonomous)
> + if (rproc->state == RPROC_ATTACHED)
> firmware = "unknown";
>
> return sprintf(buf, "%s\n", firmware); diff --git
> a/include/linux/remoteproc.h b/include/linux/remoteproc.h index
> 4e107615121a..fe383392a821 100644
> --- a/include/linux/remoteproc.h
> +++ b/include/linux/remoteproc.h
> @@ -510,7 +510,6 @@ struct rproc_dump_segment {
> * @table_sz: size of @cached_table
> * @has_iommu: flag to indicate if remote processor is behind an MMU
> * @auto_boot: flag to indicate if remote processor should be auto-started
> - * @autonomous: true if an external entity has booted the remote processor
> * @dump_segments: list of segments in the firmware
> * @nb_vdev: number of vdev currently handled by rproc
> * @char_dev: character device of the rproc @@ -547,7 +546,6 @@ struct
> rproc {
> size_t table_sz;
> bool has_iommu;
> bool auto_boot;
> - bool autonomous;
> struct list_head dump_segments;
> int nb_vdev;
> u8 elf_class;
> --

Looks good.

Reviewed-by: Peng Fan <peng.fan@nxp.com>