Received-SPF: pass (google.com: domain of linux-kernel+bounces-43087-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
From: Richard Fitzgerald <rf@opensource.cirrus.com>
To: <broonie@kernel.org>, <tiwai@suse.com>
CC: <alsa-devel@alsa-project.org>, <linux-sound@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>, <patches@opensource.cirrus.com>,
        "Richard
 Fitzgerald" <rf@opensource.cirrus.com>
Subject: [PATCH 06/18] ASoC: cs35l56: Remove buggy checks from cs35l56_is_fw_reload_needed()
Date: Mon, 29 Jan 2024 16:27:25 +0000
Message-ID: <20240129162737.497-7-rf@opensource.cirrus.com>
In-Reply-To: <20240129162737.497-1-rf@opensource.cirrus.com>
References: <20240129162737.497-1-rf@opensource.cirrus.com>
Precedence: bulk
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Content-Type: text/plain

Remove the check of fw_patched from cs35l56_is_fw_reload_needed().
Also remove the redundant check for control of the reset GPIO.

The fw_patched flag is set when cs35l56_dsp_work() has completed its
steps to download firmware and power-up wm_adsp. There was a check in
cs35l56_is_fw_reload_needed() to make a quick exit of 'false' if
!fw_patched. The original idea was that the system might be suspended
before the driver has ever made any attempt to download firmware, and
in that case the driver doesn't need to return to a patched state
because it was never in a patched state.

This check of fw_patched is buggy because it prevented ever recovering
from a failed patch. If a previous attempt to patch and reboot the
silicon had failed it would leave fw_patched==false. This would mean
the driver never attempted another download even though the fault may
have been cleared (by a hard reset, for example).

It is also a redundant check because the calling code already makes
a quick exit if cs35l56_component_probe() has not been called, which
deals with the original intent of this check but in a safer way.

The check for reset GPIO is redundant: if the silicon was hard-reset
the FIRMWARE_MISSING flag will be 1. But this check created an
expectation that the suspend/resume code toggles reset. This can't
easily be protected against accidental code breakage. The only reason
for the check was to skip runtime-resuming the driver to read the
PROTECTION_STATUS register when it already knows it reset the silicon.
But in that case the driver will have to be runtime-resumed to do
the firmware download. So it created an assumption for no benefit.

Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Fixes: 8a731fd37f8b ("ASoC: cs35l56: Move utility functions to shared file")
---
 sound/soc/codecs/cs35l56-shared.c | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/sound/soc/codecs/cs35l56-shared.c b/sound/soc/codecs/cs35l56-shared.c
index 953ba066bab1..0cd572de73a9 100644
--- a/sound/soc/codecs/cs35l56-shared.c
+++ b/sound/soc/codecs/cs35l56-shared.c
@@ -400,17 +400,6 @@ int cs35l56_is_fw_reload_needed(struct cs35l56_base *cs35l56_base)
 	unsigned int val;
 	int ret;
 
-	/* Nothing to re-patch if we haven't done any patching yet. */
-	if (!cs35l56_base->fw_patched)
-		return false;
-
-	/*
-	 * If we have control of RESET we will have asserted it so the firmware
-	 * will need re-patching.
-	 */
-	if (cs35l56_base->reset_gpio)
-		return true;
-
 	/*
 	 * In secure mode FIRMWARE_MISSING is cleared by the BIOS loader so
 	 * can't be used here to test for memory retention.
-- 
2.39.2