linux-kernel - Re: [PATCH 2/2] spi: stm32-ospi: Make usage of reset_control

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: <2bb410e34babc4c66895e8e74cf014f89127914d.camel@pengutronix.de>
Date: Thu, 10 Apr 2025 14:48:07 +0200
From: Philipp Zabel <p.zabel@...gutronix.de>
To: Patrice Chotard <patrice.chotard@...s.st.com>, Mark Brown
 <broonie@...nel.org>, Maxime Coquelin <mcoquelin.stm32@...il.com>,
 Alexandre Torgue <alexandre.torgue@...s.st.com>
Cc: linux-kernel@...r.kernel.org, linux-spi@...r.kernel.org, 
 linux-stm32@...md-mailman.stormreply.com,
 linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH 2/2] spi: stm32-ospi: Make usage of
 reset_control_acquire/release() API

On Do, 2025-04-10 at 14:23 +0200, Patrice Chotard wrote:
> As ospi reset is consumed by both OMM and OSPI drivers, use the reset
> acquire/release mechanism which ensure exclusive reset usage.
> 
> This avoid to call reset_control_get/put() in OMM driver each time
> we need to reset OSPI children and guarantee the reset line stays
> deasserted.
> 
> Signed-off-by: Patrice Chotard <patrice.chotard@...s.st.com>
> ---
>  drivers/spi/spi-stm32-ospi.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/spi/spi-stm32-ospi.c b/drivers/spi/spi-stm32-ospi.c
> index 668022098b1eac3628f0677e6d786e5a267346be..96fa362432f13c19e4dde63d964a0db64c8ade95 100644
> --- a/drivers/spi/spi-stm32-ospi.c
> +++ b/drivers/spi/spi-stm32-ospi.c
> @@ -804,7 +804,7 @@ static int stm32_ospi_get_resources(struct platform_device *pdev)
>  		return ret;
>  	}
>  
> -	ospi->rstc = devm_reset_control_array_get_optional_exclusive(dev);
> +	ospi->rstc = devm_reset_control_array_get_exclusive_released(dev);

Why does this drop _optional?

Also, since _acquire() is right below in the same function, I see no
benefit in requesting the reset control in released state.

>  	if (IS_ERR(ospi->rstc))
>  		return dev_err_probe(dev, PTR_ERR(ospi->rstc),
>  				     "Can't get reset\n");
> @@ -937,9 +937,11 @@ static int stm32_ospi_probe(struct platform_device *pdev)
>  		goto err_pm_enable;
>  
>  	if (ospi->rstc) {

This check only makes sense if the reset control (array) is optional,
otherwise ospi->rstc can never be NULL.

> +		reset_control_acquire(ospi->rstc);

This is missing error handling. Alternatively, you could just use the
normal request function to get an already-acquired reset control.

>  		reset_control_assert(ospi->rstc);
>  		udelay(2);
>  		reset_control_deassert(ospi->rstc);
> +		reset_control_release(ospi->rstc);
>  	}
>  
>  	ret = spi_register_controller(ctrl);

regards
Philipp