linux-kernel - Re: [PATCH 2/4] remoteproc: imx_rproc: Add runtime ops copy to support dynamic behavior

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aQDoD6xh454nzILm@lizhi-Precision-Tower-5810>
Date: Tue, 28 Oct 2025 11:58:07 -0400
From: Frank Li <Frank.li@....com>
To: Peng Fan <peng.fan@....com>
Cc: Bjorn Andersson <andersson@...nel.org>,
	Mathieu Poirier <mathieu.poirier@...aro.org>,
	Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>, Shawn Guo <shawnguo@...nel.org>,
	Sascha Hauer <s.hauer@...gutronix.de>,
	Pengutronix Kernel Team <kernel@...gutronix.de>,
	Fabio Estevam <festevam@...il.com>,
	Daniel Baluta <daniel.baluta@....com>,
	linux-remoteproc@...r.kernel.org, devicetree@...r.kernel.org,
	imx@...ts.linux.dev, linux-arm-kernel@...ts.infradead.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/4] remoteproc: imx_rproc: Add runtime ops copy to
 support dynamic behavior

On Tue, Oct 28, 2025 at 04:18:02PM +0800, Peng Fan wrote:
> Structure imx_rproc_dcfg contains a const pointer to imx_rproc_plat_ops,
> which defines the start/stop/detect_mode operations for a remote processor.
> To preserve the const correctness of the static configuration while
> allowing runtime modification of ops behavior, this patch introduces a new

Needn't "this patch", just "introduce a new ..."

> imx_rproc_plat_ops member in struct imx_rproc named `ops`.
>
> During initialization, the contents of dcfg->ops are copied into priv->ops.
> This enables the driver to safely override or customize specific ops at

Nit: Enable the driver ..

> runtime without affecting the original const configuration.
>
> This change improves flexibility for platforms that require dynamic

Nit: Improve ...

Generally, needn't words like "This change[patch]" in commit message.
simple use impesative mode, like

Do foo for ...
Make foo to ...

Reviewed-by: Frank Li <Frank.Li@....com>

> operation switching (e.g. i.MX95 Logical Machine ops and CPU ops).
>
> Signed-off-by: Peng Fan <peng.fan@....com>
> ---
>  drivers/remoteproc/imx_rproc.c | 27 +++++++++++++--------------
>  1 file changed, 13 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
> index 02e155e967942d745de4ccd96f9008e4211f9b36..4ffd2415295be5e60c8eb8ea5126b3562bf703fe 100644
> --- a/drivers/remoteproc/imx_rproc.c
> +++ b/drivers/remoteproc/imx_rproc.c
> @@ -116,6 +116,7 @@ struct imx_rproc {
>  	u32				entry;		/* cpu start address */
>  	u32				core_index;
>  	struct dev_pm_domain_list	*pd_list;
> +	struct imx_rproc_plat_ops	ops;
>  };
>
>  static const struct imx_rproc_att imx_rproc_att_imx93[] = {
> @@ -315,7 +316,6 @@ static int imx_rproc_scu_api_start(struct rproc *rproc)
>  static int imx_rproc_start(struct rproc *rproc)
>  {
>  	struct imx_rproc *priv = rproc->priv;
> -	const struct imx_rproc_dcfg *dcfg = priv->dcfg;
>  	struct device *dev = priv->dev;
>  	int ret;
>
> @@ -323,10 +323,10 @@ static int imx_rproc_start(struct rproc *rproc)
>  	if (ret)
>  		return ret;
>
> -	if (!dcfg->ops || !dcfg->ops->start)
> +	if (!priv->ops.start)
>  		return -EOPNOTSUPP;
>
> -	ret = dcfg->ops->start(rproc);
> +	ret = priv->ops.start(rproc);
>  	if (ret)
>  		dev_err(dev, "Failed to enable remote core!\n");
>
> @@ -372,14 +372,13 @@ static int imx_rproc_scu_api_stop(struct rproc *rproc)
>  static int imx_rproc_stop(struct rproc *rproc)
>  {
>  	struct imx_rproc *priv = rproc->priv;
> -	const struct imx_rproc_dcfg *dcfg = priv->dcfg;
>  	struct device *dev = priv->dev;
>  	int ret;
>
> -	if (!dcfg->ops || !dcfg->ops->stop)
> +	if (!priv->ops.stop)
>  		return -EOPNOTSUPP;
>
> -	ret = dcfg->ops->stop(rproc);
> +	ret = priv->ops.stop(rproc);
>  	if (ret)
>  		dev_err(dev, "Failed to stop remote core\n");
>  	else
> @@ -590,12 +589,11 @@ static int imx_rproc_scu_api_detach(struct rproc *rproc)
>  static int imx_rproc_detach(struct rproc *rproc)
>  {
>  	struct imx_rproc *priv = rproc->priv;
> -	const struct imx_rproc_dcfg *dcfg = priv->dcfg;
>
> -	if (!dcfg->ops || !dcfg->ops->detach)
> +	if (!priv->ops.detach)
>  		return -EOPNOTSUPP;
>
> -	return dcfg->ops->detach(rproc);
> +	return priv->ops.detach(rproc);
>  }
>
>  static struct resource_table *imx_rproc_get_loaded_rsc_table(struct rproc *rproc, size_t *table_sz)
> @@ -995,18 +993,16 @@ static int imx_rproc_scu_api_detect_mode(struct rproc *rproc)
>
>  static int imx_rproc_detect_mode(struct imx_rproc *priv)
>  {
> -	const struct imx_rproc_dcfg *dcfg = priv->dcfg;
> -
>  	/*
>  	 * To i.MX{7,8} ULP, Linux is under control of RTOS, no need
> -	 * dcfg->ops or dcfg->ops->detect_mode, it is state RPROC_DETACHED.
> +	 * priv->ops.detect_mode, it is state RPROC_DETACHED.
>  	 */
> -	if (!dcfg->ops || !dcfg->ops->detect_mode) {
> +	if (!priv->ops.detect_mode) {
>  		priv->rproc->state = RPROC_DETACHED;
>  		return 0;
>  	}
>
> -	return dcfg->ops->detect_mode(priv->rproc);
> +	return priv->ops.detect_mode(priv->rproc);
>  }
>
>  static int imx_rproc_sys_off_handler(struct sys_off_data *data)
> @@ -1056,6 +1052,9 @@ static int imx_rproc_probe(struct platform_device *pdev)
>  	priv->dcfg = dcfg;
>  	priv->dev = dev;
>
> +	if (dcfg->ops)
> +		memcpy(&priv->ops, dcfg->ops, sizeof(struct imx_rproc_plat_ops));
> +
>  	dev_set_drvdata(dev, rproc);
>  	priv->workqueue = create_workqueue(dev_name(dev));
>  	if (!priv->workqueue) {
>
> --
> 2.37.1
>