[PATCH 0/2] reset: Add devm_reset_control_array_get_exclusive_released()

[PATCH 0/2] reset: Add devm_reset_control_array_get_exclusive_released()

[Patrice Chotard]

Add the released variant of devm_reset_control_array_get_exclusive().
Needed by spi-smt32-ospi driver as same reset line is also used also
by stm32-omm driver.

Signed-off-by: Patrice Chotard <patrice.chotard@foss.st.com>
---
Patrice Chotard (2):
      reset: Add devm_reset_control_array_get_exclusive_released()
      spi: stm32-ospi: Make usage of reset_control_acquire/release() API

 drivers/spi/spi-stm32-ospi.c | 4 +++-
 include/linux/reset.h        | 6 ++++++
 2 files changed, 9 insertions(+), 1 deletion(-)
---
base-commit: 4a65326311aba694faafcef9e3c0ef7ae1b722e6
change-id: 20250410-b4-upstream_ospi_reset_update-ffe26d13593e

Best regards,
-- 
Patrice Chotard <patrice.chotard@foss.st.com>

[PATCH 1/2] reset: Add devm_reset_control_array_get_exclusive_released()

[Patrice Chotard]

Add the released variant of devm_reset_control_array_get_exclusive().
Needed by spi-smt32-ospi driver as same reset line is ulso used by
stm32-omm driver.

Signed-off-by: Patrice Chotard <patrice.chotard@foss.st.com>
---
 include/linux/reset.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/include/linux/reset.h b/include/linux/reset.h
index 2986ced69a026947c8aafa89cdddf1e4088caef7..840d75d172f6239540bd12ab701103b7f02a624b 100644
--- a/include/linux/reset.h
+++ b/include/linux/reset.h
@@ -1004,6 +1004,12 @@ devm_reset_control_array_get_exclusive(struct device *dev)
 	return devm_reset_control_array_get(dev, RESET_CONTROL_EXCLUSIVE);
 }
 
+static inline struct reset_control *
+devm_reset_control_array_get_exclusive_released(struct device *dev)
+{
+	return devm_reset_control_array_get(dev, RESET_CONTROL_EXCLUSIVE_RELEASED);
+}
+
 static inline struct reset_control *
 devm_reset_control_array_get_shared(struct device *dev)
 {

-- 
2.25.1

[PATCH 2/2] spi: stm32-ospi: Make usage of reset_control_acquire/release() API

[Patrice Chotard]

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@foss.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);
 	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) {
+		reset_control_acquire(ospi->rstc);
 		reset_control_assert(ospi->rstc);
 		udelay(2);
 		reset_control_deassert(ospi->rstc);
+		reset_control_release(ospi->rstc);
 	}
 
 	ret = spi_register_controller(ctrl);

-- 
2.25.1

Re: [PATCH 2/2] spi: stm32-ospi: Make usage of reset_control_acquire/release() API

[Philipp Zabel]

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@foss.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

Re: [PATCH 1/2] reset: Add devm_reset_control_array_get_exclusive_released()

[Philipp Zabel]

On Do, 2025-04-10 at 14:23 +0200, Patrice Chotard wrote:
> Add the released variant of devm_reset_control_array_get_exclusive().
> Needed by spi-smt32-ospi driver as same reset line is ulso used by
> stm32-omm driver.
> 
> Signed-off-by: Patrice Chotard <patrice.chotard@foss.st.com>
> ---
>  include/linux/reset.h | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/include/linux/reset.h b/include/linux/reset.h
> index 2986ced69a026947c8aafa89cdddf1e4088caef7..840d75d172f6239540bd12ab701103b7f02a624b 100644
> --- a/include/linux/reset.h
> +++ b/include/linux/reset.h
> @@ -1004,6 +1004,12 @@ devm_reset_control_array_get_exclusive(struct device *dev)
>  	return devm_reset_control_array_get(dev, RESET_CONTROL_EXCLUSIVE);
>  }
>  
> +static inline struct reset_control *
> +devm_reset_control_array_get_exclusive_released(struct device *dev)
> +{
> +	return devm_reset_control_array_get(dev, RESET_CONTROL_EXCLUSIVE_RELEASED);
> +}

Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>

But this might not even be necessary, see next patch.

regards
Philipp

Re: [PATCH 2/2] spi: stm32-ospi: Make usage of reset_control_acquire/release() API

[Patrice CHOTARD]

On 4/10/25 14:48, Philipp Zabel wrote:
> 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@foss.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?

Hi Philip

I wrongly based this patchset on the reset/next branch instead of the spi/for-next which include this ospi fix [1].
which make resets a required property. I will rebased it on last spi/for-next.

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

As explained in commit message, OSPI reset are also used by OMM driver which is parent of OSPI.

If i use devm_reset_control_array_get_exclusive() instead of devm_reset_control_array_get_exclusive_released()
here, i got the following kernel warning:

[    8.654378] ------------[ cut here ]------------
[    8.656524] WARNING: CPU: 1 PID: 385 at drivers/reset/core.c:799 __reset_control_get_internal+0x70/0x1d0
[    8.665999] Modules linked in: spi_stm32_ospi(+) hantro_vpu v4l2_vp9 dwmac_stm32(+) stmmac_platform v4l2_h264 v4l2_jpeg v4l2_mem2mem stmmac videobu6
emon.
[    8.691282] CPU: 1 UID: 0 PID: 385 Comm: (udev-worker) Not tainted 6.15.0-rc1-next-20250408-00018-g0f9105d08519 #22 PREEMPT 
[    8.691301] Hardware name: STMicroelectronics STM32MP257F-EV1 Evaluation Board (DT)
[    8.691307] pstate: 00000005 (nzcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[    8.691317] pc : __reset_control_get_internal+0x70/0x1d0
[    8.691336] lr : __of_reset_control_get+0x1a4/0x270
[    8.691348] sp : ffff80008359b5f0
[    8.691352] x29: ffff80008359b5f0 x28: 0000000000000000 x27: ffff80007b06c100
[    8.691371] x26: ffff80007b06c118 x25: 0000000000000001 x24: 0000000000000000
[    8.691385] x23: 0000000000000004 x22: ffff000082ecc780 x21: 0000000000000005
[    8.691399] x20: ffff000082ecc7a0 x19: ffff000083898d00 x18: 00000000ffffffff
[    8.691414] x17: ffff000082ff9a00 x16: ffff0000802d6800 x15: ffff80008359b4c0
[    8.691429] x14: 0000000000000001 x13: 007473696c5f7974 x12: 0000000000000001
[    8.691444] x11: 0000000000000003 x10: ffff80008257ec4f x9 : 0000000000000028
[    8.691459] x8 : 0101010101010101 x7 : 00000000736c6c65 x6 : 000000000080f2e5
[    8.691473] x5 : ffff80008359b698 x4 : 0000000000000000 x3 : 0000000000000005
[    8.691487] x2 : 0000000000000004 x1 : 0000000000000005 x0 : 0000000000000005
[    8.691501] Call trace:
[    8.691506]  __reset_control_get_internal+0x70/0x1d0 (P)
[    8.691522]  __of_reset_control_get+0x1a4/0x270
[    8.691535]  of_reset_control_array_get+0x9c/0x174
[    8.691549]  devm_reset_control_array_get+0x50/0xb0
[    8.691563]  stm32_ospi_get_resources+0xd4/0x344 [spi_stm32_ospi]
[    8.691584]  stm32_ospi_probe+0xf8/0x3d0 [spi_stm32_ospi]

Which means that bool acquired is set.

This is due to usage of devm_reset_control_array_get_exclusive() which sets flags to RESET_CONTROL_EXCLUSIVE
on an already controlled reset line.


> 
>>  	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.

Right, i will remove this check.

> 
>> +		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.

Ok, i will add a check.


[1] https://patches.linaro.org/project/linux-spi/patch/20250324-upstream_ospi_required_resets-v2-2-85a48afcedec@foss.st.com/

Thanks
Patrice

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