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: <1b2d2410dd9c300da1ffe015ed4835208416798b.camel@pengutronix.de>
Date: Fri, 05 Sep 2025 16:53:54 +0200
From: Philipp Zabel <p.zabel@...gutronix.de>
To: Tommaso Merciai <tommaso.merciai.xr@...renesas.com>, 
	tomm.merciai@...il.com
Cc: linux-renesas-soc@...r.kernel.org, biju.das.jz@...renesas.com, Vinod
 Koul <vkoul@...nel.org>, Geert Uytterhoeven <geert+renesas@...der.be>,
 Fabrizio Castro <fabrizio.castro.jz@...esas.com>, Lad Prabhakar
 <prabhakar.mahadev-lad.rj@...renesas.com>,  Wolfram Sang
 <wsa+renesas@...g-engineering.com>, Uwe Kleine-König
 <u.kleine-koenig@...libre.com>, dmaengine@...r.kernel.org, 
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/4] dmaengine: sh: rz-dmac: Use
 devm_add_action_or_reset()

On Fr, 2025-09-05 at 16:44 +0200, Tommaso Merciai wrote:
> Slightly simplify rz_dmac_probe() by using devm_add_action_or_reset()
> for reset cleanup.
> 
> Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@...renesas.com>
> ---
>  drivers/dma/sh/rz-dmac.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c
> index 0b526cc4d24be..0bc11a6038383 100644
> --- a/drivers/dma/sh/rz-dmac.c
> +++ b/drivers/dma/sh/rz-dmac.c
> @@ -905,6 +905,11 @@ static int rz_dmac_parse_of(struct device *dev, struct rz_dmac *dmac)
>  	return rz_dmac_parse_of_icu(dev, dmac);
>  }
>  
> +static void rz_dmac_reset_control_assert(void *data)
> +{
> +	reset_control_assert(data);
> +}
> +
>  static int rz_dmac_probe(struct platform_device *pdev)
>  {
>  	const char *irqname = "error";
> @@ -977,6 +982,12 @@ static int rz_dmac_probe(struct platform_device *pdev)
>  	if (ret)
>  		goto err_pm_runtime_put;
>  
> +	ret = devm_add_action_or_reset(&pdev->dev,
> +				       rz_dmac_reset_control_assert,
> +				       dmac->rstc);
> +	if (ret)
> +		goto err_pm_runtime_put;
> +
>  	for (i = 0; i < dmac->n_channels; i++) {
>  		ret = rz_dmac_chan_probe(dmac, &dmac->channels[i], i);
>  		if (ret < 0)
> @@ -1031,7 +1042,6 @@ static int rz_dmac_probe(struct platform_device *pdev)
>  				  channel->lmdesc.base_dma);
>  	}
>  
> -	reset_control_assert(dmac->rstc);
>  err_pm_runtime_put:
>  	pm_runtime_put(&pdev->dev);
>  
> @@ -1053,7 +1063,6 @@ static void rz_dmac_remove(struct platform_device *pdev)
>  				  channel->lmdesc.base,
>  				  channel->lmdesc.base_dma);
>  	}
> -	reset_control_assert(dmac->rstc);

This patch changes cleanup order by effectively moving the
reset_control_assert() after pm_runtime_put(). The commit message does
not explain that this is safe to do.

If this is ok, I'd move the reset_control_assert() up before
pm_runtime_enable/resume_and_get().

regards
Philipp

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ