Re: [PATCH] spi: cadence-quadspi: Fix clock enable underflows due to runtime PM

["Dutta, Anurag" <a-dutta@ti.com>]

On 05-12-2025 18:55, Mark Brown wrote:
> On Fri, Dec 05, 2025 at 06:28:06PM +0530, Dutta, Anurag wrote:
>
>> Hi Mark and Nishanth The below seems to work for me on j721e. Let me know
>> your thoughts.
>> Also, the error actually comes from :
>> if (cqspi->use_direct_mode) {
>>      ret = cqspi_request_mmap_dma(cqspi);
>>      if (ret == -EPROBE_DEFER)
>>      goto probe_setup_failed;
>> }
>> And not from flash_setup().
> Great, thanks for confirming.  We should probably ensure that has some
> logging...
>
>> @@ -2024,7 +2024,7 @@ static int cqspi_probe(struct platform_device *pdev)
>>   probe_reset_failed:
>>          if (cqspi->is_jh7110)
>>                  cqspi_jh7110_disable_clk(pdev, cqspi);
>> -       clk_disable_unprepare(cqspi->clk);
>> +       pm_runtime_force_suspend(dev);
>>   probe_clk_failed:
> The trouble with that is that in the !CONFIG_PM case
> pm_runtime_force_suspend() is defined as:
>
>    static inline int pm_runtime_force_suspend(struct device *dev) { return 0; }
>
> so we'll leak the clock enable.  If we could just require runtime PM
> this would be an awful lot easier to deal with.
So, can we maintain an internal state of the clock(enabled/disabled) in 
the  struct cqspi_st ?
Before every, clk_disable_unprepare()/clk_prepare_enable(), we check if 
the clock is actually
enabled/disabled by checking the state of "atomic_t clock_enabled" 
within struct cqspi_st.
And, when we do clk_disable_unprepare()/clk_prepare_enable(), we set the 
value of clock_enabled
accordingly.

Is this a good approach, given we take care of race conditions as well ?