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: <20250110-married-program-83bc1a997ce8@thorsis.com>
Date: Fri, 10 Jan 2025 11:40:26 +0100
From: Alexander Dahl <ada@...rsis.com>
To: Csókás, Bence <csokas.bence@...lan.hu>,
	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>,
	Tudor Ambarus <tudor.ambarus@...aro.org>,
	Jinjie Ruan <ruanjinjie@...wei.com>
Subject: Re: [PATCH v3 2/2] spi: atmel-quadspi: Add support for sama7g5 QSPI

Hello,

Am Thu, Jan 09, 2025 at 05:27:58PM +0100 schrieb Alexander Dahl:
> Hello Bence,
> 
> I had another round of intense looking at the code, this time I
> focused on pm_runtime handling.  Although I just learned about the
> basic concepts, I think the ported patch has some mistakes.  I'll
> comment here, because I don't have a SAMA7G5 to test, and I'm not
> confident enough to fix the code, but I think it's worth reporting.
> See below.
> 
> Am Thu, Nov 28, 2024 at 06:43:15PM +0100 schrieb Csókás, Bence:
> > From: Tudor Ambarus <tudor.ambarus@...rochip.com>
> > 

[…]

> > +	/* Release the chip-select. */
> > +	ret = atmel_qspi_reg_sync(aq);
> > +	if (ret) {
> > +		pm_runtime_mark_last_busy(&aq->pdev->dev);
> > +		pm_runtime_put_autosuspend(&aq->pdev->dev);
> > +		return ret;
> > +	}
> > +	atmel_qspi_write(QSPI_CR_LASTXFER, aq, QSPI_CR);
> > +
> > +	return atmel_qspi_wait_for_completion(aq, QSPI_SR_CSRA);
> > +}
> 
> This function atmel_qspi_sama7g5_transfer() seems to be called from
> atmel_qspi_exec_op() through ops->transfer() only.  I think the two
> lines in the error handling of atmel_qspi_reg_sync() lead to
> unbalanced calls of pm_runtime_xxx.  Compare with
> atmel_qspi_transfer() which has no calls to pm_runtime, everything is
> covered by atmel_qspi_exec_op() in this case where the pm_runtime
> calls surround ->set_cfg() and ->transfer().  Right?

This problem has been addressed in downstream kernel (linux4sam) by
Claudiu Beznea back in 2023 already:

https://github.com/linux4sam/linux-at91/commit/e59f646f516088fdab6d8213d8acda0c1063ec21

[…]

> The whole call tree from atmel_qspi_sama7g5_setup() downwards is not
> covered by pm_runtime get and put calls, although heavily doing i/o.
> Further down in atmel_qspi_setup() there's a write to QSPI_SCR which
> seems to be handled correctly.

Same for this:

https://github.com/linux4sam/linux-at91/commit/5ff0e74c1d548599fe85113e2f1817cb8a052b15

Some hunks of that seem to have made it to upstream, not sure?

Maybe Microchip should upstream those fixes, now that SAMA7G5 support
was ported to mainline?

Greets
Alex