-----Original Message-----
From: Mark Brown <broonie@kernel.org>
Sent: 29 November 2021 17:57
To: Krishna Yarlagadda <redacted>; Philipp Zabel
[off-list ref]
Cc: Laxman Dewangan <ldewangan@nvidia.com>; Thierry Reding
[off-list ref]; Jonathan Hunter [off-list ref];
Sowjanya Komatineni [off-list ref]; linux-
tegra@vger.kernel.org; linux-spi@vger.kernel.org; linux-
kernel@vger.kernel.org
Subject: Re: [PATCH 2/2] spi: tegra210-quad: add acpi support

On Mon, Nov 29, 2021 at 09:09:30AM +0000, Krishna Yarlagadda wrote:

quoted

quoted

quoted

+#ifdef CONFIG_ACPI
+	if (ACPI_FAILURE(acpi_evaluate_object(ACPI_HANDLE(tqspi->dev),
+					      "_RST", NULL, NULL)))
+		dev_err(tqspi->dev, "failed to reset device\n"); #endif

quoted

quoted

What happens when this runs on a DT system?

quoted

For a DT system reset handle would be present and this code will not
run

This code is really unclearly structured, the early return doesn't match the
normal kernel style and the ifdefs aren't helping with clarity.  Please
restructure it so it's clear that *both* cases have checks for the correct
firmware type being present.

I will move each reset method into their own function for readability.

That said frankly I'd expect this handling of ACPI reset to be pushed into the
reset code, it's obviously not good to be open coding this in drivers when this
looks like it's completely generic to any ACPI object so shouldn't be being
open coded in individual driers especially with the ifdefery.  Shouldn't the
reset API be able to figure out that an object with _RST has a reset control
and provide access to it through the reset API?

Common reset apis are not handling _RST. Each driver is implementing
_RST method in ACPI and calling from drivers.