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

Vincent Jardin via B4 Relay <[email protected]>
Newsgroups org.kernel.vger.linux-doc,org.kernel.feeds.b4-sent,org.kernel.vger.linux-devicetree,org.kernel.vger.linux-hwmon,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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.