mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Charles Keepax <ckeepax@opensource.cirrus.com>
To: <lee@kernel.org>
Cc: <linux-kernel@vger.kernel.org>, <patches@opensource.cirrus.com>
Subject: [PATCH] mfd: cs42l43: Update patching revision check
Date: Tue, 23 Apr 2024 11:23:39 +0100	[thread overview]
Message-ID: <20240423102339.2363400-1-ckeepax@opensource.cirrus.com> (raw)

The firmware can only be patched once. The current code checks if the
firmware supports the features required by the driver and then patches
if it does not. This could lead to the device being patched twice if
the device was patched before the driver took control, but with a
firmware that doesn't support the features the driver requires. This
would fail but potentially in unpredictable ways.

The check should actually check the device is at the ROM version, and
patch the device if it is. Then a separate later check should error out
if the devices firmware is still too old to be supported. This will at
least fail in a clean way with a nice error message.

Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
---
 drivers/mfd/cs42l43.c | 36 ++++++++++++++++++++++++++++++------
 1 file changed, 30 insertions(+), 6 deletions(-)

diff --git a/drivers/mfd/cs42l43.c b/drivers/mfd/cs42l43.c
index a0fb2dc6c3b2..ae8fd37afb75 100644
--- a/drivers/mfd/cs42l43.c
+++ b/drivers/mfd/cs42l43.c
@@ -43,6 +43,9 @@
 #define CS42L43_MCU_UPDATE_TIMEOUT_US		500000
 #define CS42L43_MCU_UPDATE_RETRIES		5
 
+#define CS42L43_MCU_ROM_REV			0x2001
+#define CS42L43_MCU_ROM_BIOS_REV		0x0000
+
 #define CS42L43_MCU_SUPPORTED_REV		0x2105
 #define CS42L43_MCU_SHADOW_REGS_REQUIRED_REV	0x2200
 #define CS42L43_MCU_SUPPORTED_BIOS_REV		0x0001
@@ -709,6 +712,23 @@ static void cs42l43_mcu_load_firmware(const struct firmware *firmware, void *con
 	complete(&cs42l43->firmware_download);
 }
 
+static int cs42l43_mcu_is_hw_compatible(struct cs42l43 *cs42l43,
+					unsigned int mcu_rev,
+					unsigned int bios_rev)
+{
+	/*
+	 * The firmware has two revision numbers bringing either of them up to a
+	 * supported version will provide the disable the driver requires.
+	 */
+	if (mcu_rev < CS42L43_MCU_SUPPORTED_REV &&
+	    bios_rev < CS42L43_MCU_SUPPORTED_BIOS_REV) {
+		dev_err(cs42l43->dev, "Firmware too old to support disable\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 /*
  * The process of updating the firmware is split into a series of steps, at the
  * end of each step a soft reset of the device might be required which will
@@ -745,11 +765,10 @@ static int cs42l43_mcu_update_step(struct cs42l43 *cs42l43)
 		  ((mcu_rev & CS42L43_FW_SUBMINOR_REV_MASK) >> 8);
 
 	/*
-	 * The firmware has two revision numbers bringing either of them up to a
-	 * supported version will provide the features the driver requires.
+	 * The firmware has two revision numbers both of them being at the ROM
+	 * revision indicates no patch has been applied.
 	 */
-	patched = mcu_rev >= CS42L43_MCU_SUPPORTED_REV ||
-		  bios_rev >= CS42L43_MCU_SUPPORTED_BIOS_REV;
+	patched = mcu_rev != CS42L43_MCU_ROM_REV || bios_rev != CS42L43_MCU_ROM_BIOS_REV;
 	/*
 	 * Later versions of the firmwware require the driver to access some
 	 * features through a set of shadow registers.
@@ -794,10 +813,15 @@ static int cs42l43_mcu_update_step(struct cs42l43 *cs42l43)
 			return cs42l43_mcu_stage_2_3(cs42l43, shadow);
 		}
 	case CS42L43_MCU_BOOT_STAGE3:
-		if (patched)
+		if (patched) {
+			ret = cs42l43_mcu_is_hw_compatible(cs42l43, mcu_rev, bios_rev);
+			if (ret)
+				return ret;
+
 			return cs42l43_mcu_disable(cs42l43);
-		else
+		} else {
 			return cs42l43_mcu_stage_3_2(cs42l43);
+		}
 	case CS42L43_MCU_BOOT_STAGE4:
 		return 0;
 	default:
-- 
2.39.2


             reply	other threads:[~2024-04-23 10:23 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-23 10:23 Charles Keepax [this message]
2024-05-02 16:56 ` (subset) " Lee Jones

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240423102339.2363400-1-ckeepax@opensource.cirrus.com \
    --to=ckeepax@opensource.cirrus.com \
    --cc=lee@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=patches@opensource.cirrus.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®