linux-kernel - Re: [PATCH v3 2/2] spi: atmel-quadspi: Add support for sama7g5 QSPI

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] [day] [month] [year] [list]

Message-ID: <2768b17c-bba8-452c-8148-cfbf1ed5393f@prolan.hu>
Date: Tue, 14 Jan 2025 18:21:52 +0100
From: Csókás Bence <csokas.bence@...lan.hu>
To: <linux-spi@...r.kernel.org>, <linux-arm-kernel@...ts.infradead.org>,
	<linux-kernel@...r.kernel.org>, Tudor Ambarus <tudor.ambarus@...rochip.com>,
	Varshini Rajendran <varshini.rajendran@...rochip.com>, Mark Brown
	<broonie@...nel.org>, Nicolas Ferre <nicolas.ferre@...rochip.com>, "Alexandre
 Belloni" <alexandre.belloni@...tlin.com>, Claudiu Beznea
	<claudiu.beznea@...on.dev>
Subject: Re: [PATCH v3 2/2] spi: atmel-quadspi: Add support for sama7g5 QSPI

Hi,

On 2025. 01. 10. 8:56, Alexander Dahl wrote:
> Hello,
> 
> found another suspicious area regarding pm_runtime in
> atmel_qspi_probe(), see below (irrelevant stuff dropped this time) …
> 
> Am Thu, Nov 28, 2024 at 06:43:15PM +0100 schrieb Csókás, Bence:
>> From: Tudor Ambarus <tudor.ambarus@...rochip.com>
>>
> 
> […]
> 
>>   	pm_runtime_set_autosuspend_delay(&pdev->dev, 500);
>>   	pm_runtime_use_autosuspend(&pdev->dev);
>> @@ -745,7 +1444,9 @@ static int atmel_qspi_probe(struct platform_device *pdev)
>>   	pm_runtime_enable(&pdev->dev);
>>   	pm_runtime_get_noresume(&pdev->dev);
>>   
>> -	atmel_qspi_init(aq);
>> +	err = atmel_qspi_init(aq);
>> +	if (err)
>> +		goto dma_release;
> 
> This error handling leads to unbalanced pm_runtime right?  The block
> below rolls back pm_runtime before jumping to 'dma_release', this here
> does not.
> 
>>   	err = spi_register_controller(ctrl);
>>   	if (err) {
>> @@ -753,13 +1454,16 @@ static int atmel_qspi_probe(struct platform_device *pdev)
>>   		pm_runtime_disable(&pdev->dev);
>>   		pm_runtime_set_suspended(&pdev->dev);
>>   		pm_runtime_dont_use_autosuspend(&pdev->dev);
>> -		goto disable_qspick;
>> +		goto dma_release;
>>   	}
> 
> This hunk got it right?

Hm, I think this would be best handled by switching to 
`devm_pm_runtime_enable()`... I'll add that to my upcoming "make 
everything use devm_*()"-series... (but I'm waiting on new functions to 
be added to dmaengine for that).

Bence