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

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: <5525272e-7220-4352-bb08-ac66631108e0@ti.com>
Date: Tue, 9 Dec 2025 11:13:43 +0530
From: "Dutta, Anurag" <a-dutta@...com>
To: Mark Brown <broonie@...nel.org>
CC: Nishanth Menon <nm@...com>, Francesco Dolcini <francesco@...cini.it>,
	Siddharth Vadapalli <s-vadapalli@...com>, <linux-spi@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>, <stable@...r.kernel.org>, "Gujulan Elango,
 Hari Prasath" <gehariprasath@...com>, "Kumar, Udit" <u-kumar1@...com>
Subject: Re: [PATCH] spi: cadence-quadspi: Fix clock enable underflows due to
 runtime PM


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 ?