[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