Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Peng Fan <peng.fan@nxp.com>
To:     Mathieu Poirier <mathieu.poirier@linaro.org>,
        "bjorn.andersson@linaro.org" <bjorn.andersson@linaro.org>,
        "ohad@wizery.com" <ohad@wizery.com>
CC:     "linux-remoteproc@vger.kernel.org" <linux-remoteproc@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH 04/13] remoteproc: Properly represent the attached state
Thread-Topic: [PATCH 04/13] remoteproc: Properly represent the attached state
Thread-Index: AQHWe8jHBhVcjpKGeU6R9CCJtEvMZqmYLoig
Date:   Thu, 15 Oct 2020 01:33:21 +0000
Message-ID: <DB6PR0402MB276089D3879EBA0C1AA3F09988020@DB6PR0402MB2760.eurprd04.prod.outlook.com>
References: <20200826164529.224476-1-mathieu.poirier@linaro.org>
 <20200826164529.224476-5-mathieu.poirier@linaro.org>
In-Reply-To: <20200826164529.224476-5-mathieu.poirier@linaro.org>
Accept-Language: en-US
Content-Language: en-US
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 34ee6980-a612-4b40-c793-08d870aa4d1b
X-MS-Exchange-CrossTenant-originalarrivaltime: 15 Oct 2020 01:33:21.1553
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: 8PQjaaUtrkuZKBGY1Y/77uNLrxf5EdlgoUJ5b0rFenh7xpkuhj0gwSTrDSqzLumH5AdljFl0/b7NPyGVGBcYDg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB7PR04MB4090
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

> Subject: [PATCH 04/13] remoteproc: Properly represent the attached state
>=20
> 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 manipulatin=
g
> two variables, i.e rproc::autonomous and rproc::state, get rid of the for=
mer
> and simply use the newly introduced RPROC_ATTACHED state.
>=20
> 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(-)
>=20
> 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;
>  	}
>=20
> -	rproc->state =3D RPROC_RUNNING;
> +	rproc->state =3D RPROC_ATTACHED;
>=20
>  	dev_info(dev, "remote processor %s is now attached\n", rproc->name);
>=20
> @@ -1636,14 +1636,6 @@ static int rproc_stop(struct rproc *rproc, bool
> crashed)
>=20
>  	rproc->state =3D RPROC_OFFLINE;
>=20
> -	/*
> -	 * The remote processor has been stopped and is now offline, which
> means
> -	 * that the next time it is brought back online the remoteproc core wil=
l
> -	 * be responsible to load its firmware.  As such it is no longer
> -	 * autonomous.
> -	 */
> -	rproc->autonomous =3D false;
> -
>  	dev_info(dev, "stopped remote processor %s\n", rproc->name);
>=20
>  	return 0;
> @@ -1994,16 +1986,6 @@ int rproc_add(struct rproc *rproc)
>  	if (ret < 0)
>  		return ret;
>=20
> -	/*
> -	 * 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 =3D=3D RPROC_DETACHED)
> -		rproc->autonomous =3D true;
> -
>  	/* if rproc is marked always-on, request it to boot */
>  	if (rproc->auto_boot) {
>  		ret =3D 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, struc=
t
> 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 =3D=3D RPROC_ATTACHED)
>  		firmware =3D "unknown";
>=20
>  	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-start=
ed
> - * @autonomous: true if an external entity has booted the remote process=
or
>   * @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>