[PATCH v7 4/4] hwmon: pmbus: mpq8646: gate the writes

Vincent Jardin via B4 Relay <[email protected]> Thu, 30 Jul 2026 17:44:02 +0200
Newsgroups org.kernel.vger.linux-hwmon,org.kernel.feeds.b4-sent,org.kernel.vger.linux-devicetree,org.kernel.vger.linux-doc,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
From: Vincent Jardin <[email protected]>

The write cases of debugfs entries are provisioning and bench
helpers. By designs the MPQ8646 powers the CPU core rail,
so a wrong write can brown out the board or persist a bad
setpoint into finite-cycle NVM.
Said differently: some wrong register writes can and likely
will physically damage or destroy the chip and/or the board.

To be safe, it is disabled by default behind a
CONFIG_SENSORS_MPQ8646_DEBUG_UNSAFE and we print an explicit warning
banner at probe time when they are built in.

Signed-off-by: Vincent Jardin <[email protected]>
---
 Documentation/hwmon/mpq8646.rst |  39 ++++++
 drivers/hwmon/pmbus/Kconfig     |  20 +++
 drivers/hwmon/pmbus/mpq8646.c   | 288 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 347 insertions(+)

diff --git a/Documentation/hwmon/mpq8646.rst b/Documentation/hwmon/mpq8646.rst
index d3ede656ea15..d606d957581b 100644
--- a/Documentation/hwmon/mpq8646.rst
+++ b/Documentation/hwmon/mpq8646.rst
@@ -246,6 +246,45 @@ The only writable entry is ``alarm_poll_interval_ms`` (the alarm-poll
 worker cadence, see above), it is driver-local and never touches the
 chip.
 
+Unsafe provisioning
+-------------------
+
+Available only when ``CONFIG_SENSORS_MPQ8646_DEBUG_UNSAFE=y``
+(``default n``, it is for boards bring up only). They WRITE to the regulator.
+On many designs the MPQ8646 powers the main CPU core rail, so a wrong write can
+brown out the board or persist a bad setpoint into finite-cycle NVM.
+WARNING: Some wrong register writes can and likely will physically damage or
+destroy the chip and/or the board. An explicit warning banner is printed at probe
+time when this option is enabled.
+
+===============================  ===============  =========================================================
+File                             PMBus / MFR cmd  Description
+===============================  ===============  =========================================================
+``store_all``                    0x15             STORE_USER_ALL Send-Byte (commit RAM config to NVM)
+``restore_all``                  0x16             RESTORE_USER_ALL Send-Byte (revert RAM to last-NVM image)
+``clear_protection_last``        0x08             CLEAR_LAST_FAULT Send-Byte
+``clear_protection_last_force``  --               gated CLEAR_LAST_FAULT, see below
+``on_off_config``                0x02 (byte)      PMBus vs CTRL-pin on/off source + active polarity
+``vout_margin_high``             0x25 (word)      margin-high VOUT setpoint
+``vout_margin_low``              0x26 (word)      margin-low VOUT setpoint
+``mfr_pmbus_lock``               0xEE (word)      programmable PMBus write-lock
+``mfr_product_rev_user``         0xC2 (word)      user-programmable product revision
+===============================  ===============  =========================================================
+
+``clear_protection_last`` writes ``CLEAR_LAST_FAULT`` (0x08) Send-Byte.
+The chip silently no-ops unless ``MFR_CFG_EXT`` (0xF5) bit[6] is set.
+
+``clear_protection_last_force`` performs the unlock with the following
+six-step dancing:
+
+1. read ``WRITE_PROTECT`` (0x10) and ``MFR_CFG_EXT`` (0xF5) for restore
+2. clear ``WRITE_PROTECT`` if set
+3. set ``MFR_CFG_EXT`` bit[6] = 1, preserving other bits
+4. send ``CLEAR_LAST_FAULT`` (0x08)
+5. restore ``MFR_CFG_EXT`` (with retry to handle the chip's
+   undocumented post-NVM-write busy window)
+6. restore ``WRITE_PROTECT``
+
 Devicetree
 ----------
 
diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
index 9f44e76b0b64..e50a54fc884d 100644
--- a/drivers/hwmon/pmbus/Kconfig
+++ b/drivers/hwmon/pmbus/Kconfig
@@ -627,6 +627,26 @@ config SENSORS_MPQ8646
 	  This driver can also be built as a module. If so, the module
 	  will be called mpq8646.
 
+config SENSORS_MPQ8646_DEBUG_UNSAFE
+	bool "MPQ8646 unsafe write/provisioning debugfs (DANGEROUS)"
+	depends on SENSORS_MPQ8646 && DEBUG_FS
+	default n
+	help
+	  Expose additional WRITE-able debugfs files in the client's
+	  pmbus debugfs directory,
+	  /sys/kernel/debug/i2c/i2c-<bus>/<bus>-<addr>/
+
+	  These are provisioning and bring-up aids. On many designs the
+	  MPQ8646 powers the SoC core rail, so a wrong write can brown out
+	  or permanently mis-provision the board.
+
+	  WARNING: Wrong register writes can and likely will physically
+	  damage or destroy the chip and/or the board.
+
+	  It shall never be set in a production, shipping, or default configuration.
+
+	  If unsure, say N.
+
 config SENSORS_PIM4328
 	tristate "Flex PIM4328 and compatibles"
 	help
diff --git a/drivers/hwmon/pmbus/mpq8646.c b/drivers/hwmon/pmbus/mpq8646.c
index 5133a2471873..6ac82beb0981 100644
--- a/drivers/hwmon/pmbus/mpq8646.c
+++ b/drivers/hwmon/pmbus/mpq8646.c
@@ -7,6 +7,7 @@
 
 #include <linux/bitops.h>
 #include <linux/debugfs.h>
+#include <linux/delay.h>
 #include <linux/i2c.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
@@ -23,8 +24,18 @@
 #define MPQ8646_ALARM_POLL_MS_DEFAULT	1000
 
 /* MPS vendor-extended command codes (NOT in PMBus 1.3 Part II) */
+#define MPS_CLEAR_LAST_FAULT		0x08
+#define MPS_MFR_CFG_EXT			0xF5
+#define MPS_MFR_CFG_EXT_CLR_LAST_EN	BIT(6)
 #define MPS_PROTECTION_LAST		0xFB
 
+/*
+ * PMBus 1.3 NVM commit / revert commands. MPS equivalent of
+ * STORE_ALL (15h) and RESTORE_ALL (16h).
+ */
+#define PMBUS_STORE_USER_ALL		0x15
+#define PMBUS_RESTORE_USER_ALL		0x16
+
 /* PMBus 1.3 timing / UVLO command codes */
 #define PMBUS_VIN_ON			0x35
 #define PMBUS_VIN_OFF			0x36
@@ -36,10 +47,32 @@
 /* MPS vendor-extended observability / identity registers */
 #define MPS_MFR_CONFIG_ID		0xC0
 #define MPS_MFR_CONFIG_CODE_REV		0xC1
+#define MPS_MFR_PRODUCT_REV_USER	0xC2
 #define MPS_MFR_SILICON_REV		0xC3
 #define MPS_MFR_RETRY_TIMES		0xF4
 #define MPS_MFR_VBOOT_CFG		0xFC
 
+/*
+ * MPS_MFR_PMBUS_LOCK (EEh): 16-bit WORD whose low two bits gate
+ * subsequent PMBus writes
+ *   bits[1:0] = 00  -- unlocked (POR default)
+ *               01  -- lock all writes EXCEPT VOUT_COMMAND (0x21)
+ *                      so the operator can still DVFS the rail
+ *               11  -- lock all writes
+ * A negative-going PG edge resets these bits to 00, the lock
+ * is operationally reversible without a full chip POR.
+ */
+#define MPS_MFR_PMBUS_LOCK		0xEE
+
+/*
+ * Retry parameters for the MFR_CFG_EXT gate-close write after
+ * CLEAR_LAST_FAULT. Bench-observed NVM-busy NACK window on this
+ * silicon is about 1 ms; the datasheet does not have information.
+ */
+#define MPQ8646_NVM_RETRY_MAX		5
+#define MPQ8646_NVM_RETRY_DELAY_US_MIN	2000
+#define MPQ8646_NVM_RETRY_DELAY_US_MAX	4000
+
 #define MPQ8646_DEBUG(client, fmt, ...) \
 	dev_dbg(&(client)->dev, fmt, ##__VA_ARGS__)
 
@@ -501,6 +534,259 @@ static int mpq8646_dbg_reg_show(struct seq_file *s, void *unused)
 }
 DEFINE_SHOW_ATTRIBUTE(mpq8646_dbg_reg);
 
+#ifdef CONFIG_SENSORS_MPQ8646_DEBUG_UNSAFE
+/*
+ * Write/provisioning data, disabled by default: NVM commit and
+ * revert, the CLEAR_LAST_FAULT sequences and a small set of named
+ * writable registers.
+ */
+
+static void mpq8646_unsafe_banner(struct mpq8646_priv *priv)
+{
+	dev_warn(&priv->client->dev,
+		 "**********************************************************\n"
+		 "**   WARNING WARNING WARNING WARNING WARNING WARNING    **\n"
+		 "**                                                      **\n"
+		 "** The MPQ8646 provisioning debugfs writes are enabled. **\n"
+		 "** Wrong register writes can and likely will physically **\n"
+		 "** damage or destroy the chip and/or the board.         **\n"
+		 "**                                                      **\n"
+		 "** If you see this message and you are not debugging    **\n"
+		 "** the kernel, report this immediately to your system   **\n"
+		 "** administrator!                                       **\n"
+		 "**                                                      **\n"
+		 "**   WARNING WARNING WARNING WARNING WARNING WARNING    **\n"
+		 "**********************************************************\n");
+}
+
+static int mpq8646_dbg_clear_protection_last(void *data, u64 val)
+{
+	struct mpq8646_priv *priv = data;
+	int rc;
+
+	if (!val)
+		return 0;
+
+	guard(pmbus_lock)(priv->client);
+	scoped_guard(mutex, &priv->mps_lock)
+		rc = i2c_smbus_write_byte(priv->client, MPS_CLEAR_LAST_FAULT);
+	if (rc < 0)
+		dev_warn(&priv->client->dev,
+			 "clear_protection_last: CLEAR_LAST_FAULT write failed (%d)\n",
+			 rc);
+	return rc;
+}
+DEFINE_DEBUGFS_ATTRIBUTE(mpq8646_dbg_clear_protection_last_fops,
+			 NULL, mpq8646_dbg_clear_protection_last, "%llu\n");
+
+static int mpq8646_dbg_clear_protection_last_force(void *data, u64 val)
+{
+	struct mpq8646_priv *priv = data;
+	int rc, ret;
+	int wp_orig, cfg_orig;
+
+	if (!val)
+		return 0;
+
+	guard(pmbus_lock)(priv->client);
+	guard(mutex)(&priv->mps_lock);
+
+	wp_orig = i2c_smbus_read_byte_data(priv->client, PMBUS_WRITE_PROTECT);
+	if (wp_orig < 0) {
+		dev_warn(&priv->client->dev,
+			 "clear_protection_last_force: WRITE_PROTECT read failed (%d), aborting\n",
+			 wp_orig);
+		return wp_orig;
+	}
+	cfg_orig = i2c_smbus_read_word_data(priv->client, MPS_MFR_CFG_EXT);
+	if (cfg_orig < 0) {
+		dev_warn(&priv->client->dev,
+			 "clear_protection_last_force: MFR_CFG_EXT read failed (%d), aborting\n",
+			 cfg_orig);
+		return cfg_orig;
+	}
+
+	if (wp_orig != 0) {
+		rc = i2c_smbus_write_byte_data(priv->client,
+					       PMBUS_WRITE_PROTECT, 0);
+		if (rc < 0) {
+			dev_warn(&priv->client->dev,
+				 "clear_protection_last_force: WP clear failed (%d), aborting\n",
+				 rc);
+			return rc;
+		}
+	}
+
+	ret = i2c_smbus_write_word_data(priv->client, MPS_MFR_CFG_EXT,
+					(u16)cfg_orig | MPS_MFR_CFG_EXT_CLR_LAST_EN);
+	if (ret < 0) {
+		dev_warn(&priv->client->dev,
+			 "clear_protection_last_force: gate open failed (%d)\n",
+			 ret);
+	} else {
+		ret = i2c_smbus_write_byte(priv->client, MPS_CLEAR_LAST_FAULT);
+		if (ret < 0)
+			dev_warn(&priv->client->dev,
+				 "clear_protection_last_force: CLEAR_LAST_FAULT failed (%d) even with gate open\n",
+				 ret);
+
+		for (int attempt = 0; attempt < MPQ8646_NVM_RETRY_MAX; attempt++) {
+			rc = i2c_smbus_write_word_data(priv->client,
+						       MPS_MFR_CFG_EXT,
+						       (u16)cfg_orig);
+			if (rc >= 0)
+				break;
+			usleep_range(MPQ8646_NVM_RETRY_DELAY_US_MIN,
+				     MPQ8646_NVM_RETRY_DELAY_US_MAX);
+		}
+		if (rc < 0) {
+			dev_warn(&priv->client->dev,
+				 "clear_protection_last_force: MFR_CFG_EXT restore failed after retries (%d): gate may stay open until POR\n",
+				 rc);
+			if (ret == 0)
+				ret = rc;
+		}
+	}
+
+	if (wp_orig != 0) {
+		rc = i2c_smbus_write_byte_data(priv->client,
+					       PMBUS_WRITE_PROTECT,
+					       (u8)wp_orig);
+		if (rc < 0) {
+			dev_warn(&priv->client->dev,
+				 "clear_protection_last_force: WP restore failed (%d)\n",
+				 rc);
+			if (ret == 0)
+				ret = rc;
+		}
+	}
+	return ret;
+}
+DEFINE_DEBUGFS_ATTRIBUTE(mpq8646_dbg_clear_protection_last_force_fops,
+			 NULL, mpq8646_dbg_clear_protection_last_force, "%llu\n");
+
+static int mpq8646_dbg_store_all(void *data, u64 val)
+{
+	struct mpq8646_priv *priv = data;
+	int rc;
+
+	if (!val)
+		return 0;
+
+	guard(pmbus_lock)(priv->client);
+	scoped_guard(mutex, &priv->mps_lock)
+		rc = i2c_smbus_write_byte(priv->client, PMBUS_STORE_USER_ALL);
+	if (rc < 0)
+		dev_warn(&priv->client->dev,
+			 "store_all: STORE_USER_ALL (0x15) write failed (%d)\n",
+			 rc);
+	return rc;
+}
+DEFINE_DEBUGFS_ATTRIBUTE(mpq8646_dbg_store_all_fops,
+			 NULL, mpq8646_dbg_store_all, "%llu\n");
+
+static int mpq8646_dbg_restore_all(void *data, u64 val)
+{
+	struct mpq8646_priv *priv = data;
+	int rc;
+
+	if (!val)
+		return 0;
+
+	guard(pmbus_lock)(priv->client);
+	scoped_guard(mutex, &priv->mps_lock)
+		rc = i2c_smbus_write_byte(priv->client, PMBUS_RESTORE_USER_ALL);
+	if (rc < 0)
+		dev_warn(&priv->client->dev,
+			 "restore_all: RESTORE_USER_ALL (0x16) write failed (%d)\n",
+			 rc);
+	return rc;
+}
+DEFINE_DEBUGFS_ATTRIBUTE(mpq8646_dbg_restore_all_fops,
+			 NULL, mpq8646_dbg_restore_all, "%llu\n");
+
+static const struct mpq8646_dbg_reg mpq8646_dbg_regs_unsafe[] = {
+	/* PMBus 1.3 control / margin */
+	{ PMBUS_ON_OFF_CONFIG,      false, "on_off_config" },
+	{ PMBUS_VOUT_MARGIN_HIGH,   true,  "vout_margin_high" },
+	{ PMBUS_VOUT_MARGIN_LOW,    true,  "vout_margin_low" },
+	/* MPS PMBus-level write-protect */
+	{ MPS_MFR_PMBUS_LOCK,       true,  "mfr_pmbus_lock" },
+	/* MPS user-writable product revision */
+	{ MPS_MFR_PRODUCT_REV_USER, true,  "mfr_product_rev_user" },
+};
+
+static int mpq8646_dbg_reg_get(void *data, u64 *val)
+{
+	struct mpq8646_dbg_reg_ctx *ctx = data;
+	int rc;
+
+	guard(pmbus_lock)(ctx->priv->client);
+	if (ctx->desc->is_word)
+		rc = i2c_smbus_read_word_data(ctx->priv->client,
+					      ctx->desc->reg);
+	else
+		rc = i2c_smbus_read_byte_data(ctx->priv->client,
+					      ctx->desc->reg);
+	if (rc < 0)
+		return rc;
+	*val = rc;
+	return 0;
+}
+
+static int mpq8646_dbg_reg_set(void *data, u64 val)
+{
+	struct mpq8646_dbg_reg_ctx *ctx = data;
+	int rc;
+
+	guard(pmbus_lock)(ctx->priv->client);
+	if (ctx->desc->is_word)
+		rc = i2c_smbus_write_word_data(ctx->priv->client,
+					       ctx->desc->reg, (u16)val);
+	else
+		rc = i2c_smbus_write_byte_data(ctx->priv->client,
+					       ctx->desc->reg, (u8)val);
+	return rc < 0 ? rc : 0;
+}
+DEFINE_DEBUGFS_ATTRIBUTE(mpq8646_dbg_reg_rw_fops,
+			 mpq8646_dbg_reg_get, mpq8646_dbg_reg_set, "0x%llx\n");
+
+static void mpq8646_debugfs_register_unsafe(struct mpq8646_priv *priv,
+					    struct dentry *root)
+{
+	struct mpq8646_dbg_reg_ctx *ctx;
+	size_t i;
+
+	mpq8646_unsafe_banner(priv);
+
+	debugfs_create_file_unsafe("clear_protection_last", 0200, root, priv,
+				   &mpq8646_dbg_clear_protection_last_fops);
+	debugfs_create_file_unsafe("clear_protection_last_force", 0200, root,
+				   priv,
+				   &mpq8646_dbg_clear_protection_last_force_fops);
+	debugfs_create_file_unsafe("store_all", 0200, root, priv,
+				   &mpq8646_dbg_store_all_fops);
+	debugfs_create_file_unsafe("restore_all", 0200, root, priv,
+				   &mpq8646_dbg_restore_all_fops);
+
+	ctx = devm_kcalloc(&priv->client->dev,
+			   ARRAY_SIZE(mpq8646_dbg_regs_unsafe),
+			   sizeof(*ctx), GFP_KERNEL);
+	if (!ctx)
+		return;
+	for (i = 0; i < ARRAY_SIZE(mpq8646_dbg_regs_unsafe); i++) {
+		ctx[i].priv = priv;
+		ctx[i].desc = &mpq8646_dbg_regs_unsafe[i];
+		debugfs_create_file_unsafe(mpq8646_dbg_regs_unsafe[i].name,
+					   0600, root, &ctx[i],
+					   &mpq8646_dbg_reg_rw_fops);
+	}
+}
+#else
+static inline void mpq8646_debugfs_register_unsafe(struct mpq8646_priv *priv,
+						   struct dentry *root) {}
+#endif /* CONFIG_SENSORS_MPQ8646_DEBUG_UNSAFE */
+
 static int mpq8646_dbg_poll_interval_get(void *data, u64 *val)
 {
 	struct mpq8646_priv *priv = data;
@@ -557,6 +843,8 @@ static void mpq8646_debugfs_register(struct mpq8646_priv *priv)
 				    root, &priv->dbg_reg_ctx[i],
 				    &mpq8646_dbg_reg_fops);
 	}
+
+	mpq8646_debugfs_register_unsafe(priv, root);
 }
 
 static void mpq8646_debugfs_unregister(struct mpq8646_priv *priv)

-- 
2.43.0