linux-kernel - Re: [PATCH 1/3] spi: tegra210-quad: use device_reset_optional() instead of device

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: <f3e47d12-f6be-4bb5-b87b-84aa0037e1ef@sirena.org.uk>
Date: Mon, 17 Mar 2025 17:24:24 +0000
From: Mark Brown <broonie@...ian.org>
To: Breno Leitao <leitao@...ian.org>
Cc: Thierry Reding <thierry.reding@...il.com>,
	Jonathan Hunter <jonathanh@...dia.com>,
	Sowjanya Komatineni <skomatineni@...dia.com>,
	Laxman Dewangan <ldewangan@...dia.com>, linux-tegra@...r.kernel.org,
	linux-spi@...r.kernel.org, linux-kernel@...r.kernel.org,
	rmikey@...a.com, kernel-team@...a.com
Subject: Re: [PATCH 1/3] spi: tegra210-quad: use device_reset_optional()
 instead of device_reset()

On Mon, Mar 17, 2025 at 09:56:43AM -0700, Breno Leitao wrote:
> Hello Mark,
> 
> On Mon, Mar 17, 2025 at 04:45:31PM +0000, Mark Brown wrote:
> > On Mon, Mar 17, 2025 at 08:44:01AM -0700, Breno Leitao wrote:
> > > My UEFI machines with tegra210-quad consistently report "device reset
> > > failed". Investigation showed this isn't an actual failure
> > > - __device_reset() returns -ENOENT because ACPI has no "*_RST" method.

> > That's not the case, it's returning an error because there is no reset
> > controller discoverable via any mechanism. 

> Sorry, I was not very familiar with this subsystem, but I chase down
> __device_reset(), and I found the return was coming from:

> 	int __device_reset(struct device *dev, bool optional)
> 	{
> 		acpi_handle handle = ACPI_HANDLE(dev);
> 		if (handle) {
> 			if (!acpi_has_method(handle, "_RST"))
> 				return optional ? 0 : -ENOENT;

> > There's no specific handling for ACPI here.  

> Do you mean no _RST method as stated above?

That's only happening in the case where the device has an ACPI handle,
the SPI driver has no idea why the reset API failed to look up a reset
controller.  Your change is to the SPI driver, not the reset framework.

> > It's also not clear that this is a false positive, the
> > driver did indeed fail to reset the device and especially for the error
> > handling case that seems like relevant information.

> If the driver failed to reset the device, then device_reset_optional()
> it will return an error code, but it will not return an error code if
> the RST method is not found, right?

> Sorry, if I am mis-understading the code here.

Clearly if no reset controller is available then the driver will have
been unable to reset the hardware.  That seems like something it
actually wanted to do, especially in the error handling case - it's a
lot less likely that we'll recover things without the reset happening.
During probe it's possibly not so urgent but at other times it seems
more relevant.

> > At the very least the changelog should be clarified.

> What would you add to the changelog to make this clear?

For starters the mention of ACPI is irrelevant to what the SPI driver is
doing.  This sounds like a change specific to ACPI but it affects all
users.

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)