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

[Dutta, Anurag]

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 ?