[PATCH v6 0/4] hwmon: pmbus: add MPS MPQ8646 support

All of lore.kernel.org
 help / color / mirror / Atom feed

* [PATCH v6 0/4] hwmon: pmbus: add MPS MPQ8646 support
@ 2026-07-29 21:51 ` Vincent Jardin
  0 siblings, 0 replies; 21+ messages in thread
From: Vincent Jardin via B4 Relay @ 2026-07-29 21:51 UTC (permalink / raw)
  To: Guenter Roeck, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Jonathan Corbet, Shuah Khan, Charles Hsu
  Cc: Markus Elfring, linux-hwmon, linux-kernel, devicetree, linux-doc,
	Vincent Jardin

Add support for the Monolithic Power Systems MPQ8646 step-down
converter as a PMBus device.

  1/4 pmbus core: export pmbus_check_and_notify_faults() so a chip
      driver can run the SMBALERT# path check and notify sequence
      from a polling work item when the chip's SMBALERT# pin is
      unavailable to the CPU.
  2/4 dt-bindings: add the MPQ8646 compatible to the existing
      mps,mpq8785 schema (same mps,vout-fb-divider-ratio-permille
      property, same constraints).
  3/4 the MPQ8646 driver: PMBus telemetry
  4/4 the provisioning/NVM write entries, compiled out by default
      thanks to SENSORS_MPQ8646_DEBUG_UNSAFE.

To: Guenter Roeck <linux@roeck-us.net>
To: Rob Herring <robh@kernel.org>
To: Krzysztof Kozlowski <krzk+dt@kernel.org>
To: Conor Dooley <conor+dt@kernel.org>
To: Jonathan Corbet <corbet@lwn.net>
To: Shuah Khan <skhan@linuxfoundation.org>
To: Charles Hsu <ythsu0511@gmail.com>
Cc: Markus Elfring <Markus.Elfring@web.de>
Cc: linux-hwmon@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: linux-doc@vger.kernel.org
Signed-off-by: Vincent Jardin <vjardin@free.fr>

Changes in v6:
- core: replace the pmbus_get_hwmon_device() accessor with an exported
  pmbus_check_and_notify_faults() (suggested by Guenter Roeck on v5)
- driver: drop the bring-up probe/instrumentation debugfs entries
  (challenged by Guenter Roeck on v4)
- new patch 4/4: move the write/provisioning debugfs entries
  behind a new default-n SENSORS_MPQ8646_DEBUG_UNSAFE
- move the debugfs files into the client's pmbus debugfs directory
  (pmbus_get_debugfs_dir()), mask using VOUT_MODE checks, MPQ8646_DEBUG()
  using dev_dbg(), drop the 0xFF VOUT_MODE probe check and the
  source-attribution comments (many inline reviews reported
  by Guenter Roeck on v5)
- driver: drop the WRITE_PROTECT read shim: the HW implements the
  register (reads back 0x00, unprotected), so the core handles write
  protection as designed (reported by Guenter Roeck on v5)
- dt-bindings: fold the MPQ8646 into the existing mps,mpq8785.yaml
  (suggested by Krzysztof Kozlowski on v5)
- scope locking, guard() and scoped_guard() (suggested by Markus
  Elfring on v5)
- XXX TBD: should pmbus_check_and_notify_faults() also update
  the pmbus registered thermal zones, or is that follow-up material ?
  Currently, neither the SMBALERT# path nor this helper does it.
- Link to v5: https://lore.kernel.org/r/20260724-mpq8646_v0-v5-0-5915b5ae9aa2@free.fr

Changes in v5:
- driver: fix the hwmon channel indices used by the in*_alarm poll
  notifications (thanks Sashiko AI, v4)
- driver: remove the shared debugfs root on module unload, by creating
  it in module_init() and removing it in module_exit(). It also drops
  the v4 per-probe root mutex and debugfs_lookup()
  (Sashiko AI review of v4)
- doc: add Documentation/hwmon/mpq8646.rst to the toctree (kernel test robot)
- doc: fix a malformed table and drop the stale probe_page_write row
  (removed since v3) and fix typo
- Link to v4: https://lore.kernel.org/r/20260723-mpq8646_v0-v4-0-b6323d5fc524@free.fr

Changes in v4 (all reported by the Sashiko AI review on v3):
- driver: register debugfs only after a successful pmbus_do_probe() --
  the debugfs handlers take pmbus_lock() and rely on the pmbus clientdata
  that pmbus_do_probe() installs (NULL-deref window in v3)
- driver: in remove(), unregister debugfs before cancel_delayed_work_sync()
  so a debugfs write cannot re-arm the poll worker after it is cancelled
- driver: serialise the shared debugfs root create against concurrent
  probes with a static mutex
- driver: move the remaining last_probe_* updates (clear_protection_last,
  store_all, restore_all) inside mps_lock
- dt-bindings: describe the hardware, not the driver, in the description
- Link to v3: https://lore.kernel.org/r/20260723-mpq8646_v0-v3-0-1af1f3d38848@free.fr

Changes in v3 (all reported by the Sashiko AI review on v2):
- driver: alarm_poll_interval_ms is now a debugfs fops
- driver: pmbus_lock() around the async raw i2c accesses: the poll
  worker and the CLEAR_LAST_FAULT force sequence, the nvmem snapshot read
  keeps mps_lock (read-only, no chip-state change)
- driver: update last_probe_rc/last_probe_data under mps_lock to avoid
  torn diagnostics
- driver: remove the dead PMBUS_PAGE swallow in write_byte
- driver: remove the probe_page_write debugfs hook
- Link to v2: https://lore.kernel.org/r/20260723-mpq8646_v0-v2-0-3c4cb71f23c0@free.fr

Changes in v2:
- driver: register debugfs only after the DT-property validation, to
  avoid an early probe error to avoid dangling debugfs entries
- driver: do not schedule the alarm-poll worker when SMBALERT# (irq)
  is wired, and do not re-arm it from the worker in that case
- driver: dput() the dentry returned by debugfs_lookup()
- driver: hold mps_lock around the nvmem snapshot reads
- driver: fix VID coefficients comment
- dt-bindings: fix typo of the commit message
- Link to v1: https://lore.kernel.org/r/20260723-mpq8646_v0-v1-0-14363c75916f@free.fr

---
Vincent Jardin (4):
      hwmon: (pmbus/core) export pmbus_check_and_notify_faults()
      dt-bindings: hwmon: pmbus: add MPS MPQ8646
      hwmon: pmbus: add MPQ8646 driver
      hwmon: pmbus: mpq8646: gate the writes

 .../bindings/hwmon/pmbus/mps,mpq8785.yaml          |   5 +-
 Documentation/hwmon/index.rst                      |   1 +
 Documentation/hwmon/mpq8646.rst                    | 298 +++++++
 MAINTAINERS                                        |   7 +
 drivers/hwmon/pmbus/Kconfig                        |  28 +
 drivers/hwmon/pmbus/Makefile                       |   1 +
 drivers/hwmon/pmbus/mpq8646.c                      | 954 +++++++++++++++++++++
 drivers/hwmon/pmbus/pmbus.h                        |   1 +
 drivers/hwmon/pmbus/pmbus_core.c                   |  14 +-
 9 files changed, 1305 insertions(+), 4 deletions(-)
---
base-commit: 248951ddc14de84de3910f9b13f51491a8cd91df
change-id: 20260723-mpq8646_v0-3383cb574d7a

Best regards,
-- 
Vincent Jardin <vjardin@free.fr>



^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v6 0/4] hwmon: pmbus: add MPS MPQ8646 support
@ 2026-07-29 21:51 ` Vincent Jardin
  0 siblings, 0 replies; 21+ messages in thread
From: Vincent Jardin @ 2026-07-29 21:51 UTC (permalink / raw)
  To: Guenter Roeck, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Jonathan Corbet, Shuah Khan, Charles Hsu
  Cc: Markus Elfring, linux-hwmon, linux-kernel, devicetree, linux-doc,
	Vincent Jardin

Add support for the Monolithic Power Systems MPQ8646 step-down
converter as a PMBus device.

  1/4 pmbus core: export pmbus_check_and_notify_faults() so a chip
      driver can run the SMBALERT# path check and notify sequence
      from a polling work item when the chip's SMBALERT# pin is
      unavailable to the CPU.
  2/4 dt-bindings: add the MPQ8646 compatible to the existing
      mps,mpq8785 schema (same mps,vout-fb-divider-ratio-permille
      property, same constraints).
  3/4 the MPQ8646 driver: PMBus telemetry
  4/4 the provisioning/NVM write entries, compiled out by default
      thanks to SENSORS_MPQ8646_DEBUG_UNSAFE.

To: Guenter Roeck <linux@roeck-us.net>
To: Rob Herring <robh@kernel.org>
To: Krzysztof Kozlowski <krzk+dt@kernel.org>
To: Conor Dooley <conor+dt@kernel.org>
To: Jonathan Corbet <corbet@lwn.net>
To: Shuah Khan <skhan@linuxfoundation.org>
To: Charles Hsu <ythsu0511@gmail.com>
Cc: Markus Elfring <Markus.Elfring@web.de>
Cc: linux-hwmon@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: linux-doc@vger.kernel.org
Signed-off-by: Vincent Jardin <vjardin@free.fr>

Changes in v6:
- core: replace the pmbus_get_hwmon_device() accessor with an exported
  pmbus_check_and_notify_faults() (suggested by Guenter Roeck on v5)
- driver: drop the bring-up probe/instrumentation debugfs entries
  (challenged by Guenter Roeck on v4)
- new patch 4/4: move the write/provisioning debugfs entries
  behind a new default-n SENSORS_MPQ8646_DEBUG_UNSAFE
- move the debugfs files into the client's pmbus debugfs directory
  (pmbus_get_debugfs_dir()), mask using VOUT_MODE checks, MPQ8646_DEBUG()
  using dev_dbg(), drop the 0xFF VOUT_MODE probe check and the
  source-attribution comments (many inline reviews reported
  by Guenter Roeck on v5)
- driver: drop the WRITE_PROTECT read shim: the HW implements the
  register (reads back 0x00, unprotected), so the core handles write
  protection as designed (reported by Guenter Roeck on v5)
- dt-bindings: fold the MPQ8646 into the existing mps,mpq8785.yaml
  (suggested by Krzysztof Kozlowski on v5)
- scope locking, guard() and scoped_guard() (suggested by Markus
  Elfring on v5)
- XXX TBD: should pmbus_check_and_notify_faults() also update
  the pmbus registered thermal zones, or is that follow-up material ?
  Currently, neither the SMBALERT# path nor this helper does it.
- Link to v5: https://lore.kernel.org/r/20260724-mpq8646_v0-v5-0-5915b5ae9aa2@free.fr

Changes in v5:
- driver: fix the hwmon channel indices used by the in*_alarm poll
  notifications (thanks Sashiko AI, v4)
- driver: remove the shared debugfs root on module unload, by creating
  it in module_init() and removing it in module_exit(). It also drops
  the v4 per-probe root mutex and debugfs_lookup()
  (Sashiko AI review of v4)
- doc: add Documentation/hwmon/mpq8646.rst to the toctree (kernel test robot)
- doc: fix a malformed table and drop the stale probe_page_write row
  (removed since v3) and fix typo
- Link to v4: https://lore.kernel.org/r/20260723-mpq8646_v0-v4-0-b6323d5fc524@free.fr

Changes in v4 (all reported by the Sashiko AI review on v3):
- driver: register debugfs only after a successful pmbus_do_probe() --
  the debugfs handlers take pmbus_lock() and rely on the pmbus clientdata
  that pmbus_do_probe() installs (NULL-deref window in v3)
- driver: in remove(), unregister debugfs before cancel_delayed_work_sync()
  so a debugfs write cannot re-arm the poll worker after it is cancelled
- driver: serialise the shared debugfs root create against concurrent
  probes with a static mutex
- driver: move the remaining last_probe_* updates (clear_protection_last,
  store_all, restore_all) inside mps_lock
- dt-bindings: describe the hardware, not the driver, in the description
- Link to v3: https://lore.kernel.org/r/20260723-mpq8646_v0-v3-0-1af1f3d38848@free.fr

Changes in v3 (all reported by the Sashiko AI review on v2):
- driver: alarm_poll_interval_ms is now a debugfs fops
- driver: pmbus_lock() around the async raw i2c accesses: the poll
  worker and the CLEAR_LAST_FAULT force sequence, the nvmem snapshot read
  keeps mps_lock (read-only, no chip-state change)
- driver: update last_probe_rc/last_probe_data under mps_lock to avoid
  torn diagnostics
- driver: remove the dead PMBUS_PAGE swallow in write_byte
- driver: remove the probe_page_write debugfs hook
- Link to v2: https://lore.kernel.org/r/20260723-mpq8646_v0-v2-0-3c4cb71f23c0@free.fr

Changes in v2:
- driver: register debugfs only after the DT-property validation, to
  avoid an early probe error to avoid dangling debugfs entries
- driver: do not schedule the alarm-poll worker when SMBALERT# (irq)
  is wired, and do not re-arm it from the worker in that case
- driver: dput() the dentry returned by debugfs_lookup()
- driver: hold mps_lock around the nvmem snapshot reads
- driver: fix VID coefficients comment
- dt-bindings: fix typo of the commit message
- Link to v1: https://lore.kernel.org/r/20260723-mpq8646_v0-v1-0-14363c75916f@free.fr

---
Vincent Jardin (4):
      hwmon: (pmbus/core) export pmbus_check_and_notify_faults()
      dt-bindings: hwmon: pmbus: add MPS MPQ8646
      hwmon: pmbus: add MPQ8646 driver
      hwmon: pmbus: mpq8646: gate the writes

 .../bindings/hwmon/pmbus/mps,mpq8785.yaml          |   5 +-
 Documentation/hwmon/index.rst                      |   1 +
 Documentation/hwmon/mpq8646.rst                    | 298 +++++++
 MAINTAINERS                                        |   7 +
 drivers/hwmon/pmbus/Kconfig                        |  28 +
 drivers/hwmon/pmbus/Makefile                       |   1 +
 drivers/hwmon/pmbus/mpq8646.c                      | 954 +++++++++++++++++++++
 drivers/hwmon/pmbus/pmbus.h                        |   1 +
 drivers/hwmon/pmbus/pmbus_core.c                   |  14 +-
 9 files changed, 1305 insertions(+), 4 deletions(-)
---
base-commit: 248951ddc14de84de3910f9b13f51491a8cd91df
change-id: 20260723-mpq8646_v0-3383cb574d7a

Best regards,
-- 
Vincent Jardin <vjardin@free.fr>


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v6 1/4] hwmon: (pmbus/core) export pmbus_check_and_notify_faults()
  2026-07-29 21:51 ` Vincent Jardin
@ 2026-07-29 21:51   ` Vincent Jardin
  -1 siblings, 0 replies; 21+ messages in thread
From: Vincent Jardin via B4 Relay @ 2026-07-29 21:51 UTC (permalink / raw)
  To: Guenter Roeck, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Jonathan Corbet, Shuah Khan, Charles Hsu
  Cc: Markus Elfring, linux-hwmon, linux-kernel, devicetree, linux-doc,
	Vincent Jardin

From: Vincent Jardin <vjardin@free.fr>

Factor of pmbus_fault_handler() into an exported helper so
drivers can notify sequence from a polling work item on boards
for which the chip's SMBALERT# pin is not wired to the
CPU. The interrupt handler becomes a thin wrapper.

Like the SMBALERT# path, the helper notifies and then clears the
latched faults unconditionally, so a polling caller inherits exactly
the interrupt semantics.

Signed-off-by: Vincent Jardin <vjardin@free.fr>
---
 drivers/hwmon/pmbus/pmbus.h      |  1 +
 drivers/hwmon/pmbus/pmbus_core.c | 14 +++++++++++---
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/hwmon/pmbus/pmbus.h b/drivers/hwmon/pmbus/pmbus.h
index 23e3eda58870..1504d8b9e34c 100644
--- a/drivers/hwmon/pmbus/pmbus.h
+++ b/drivers/hwmon/pmbus/pmbus.h
@@ -560,6 +560,7 @@ int pmbus_write_byte_data(struct i2c_client *client, int page, u8 reg,
 int pmbus_update_byte_data(struct i2c_client *client, int page, u8 reg,
 			   u8 mask, u8 value);
 void pmbus_clear_faults(struct i2c_client *client);
+void pmbus_check_and_notify_faults(struct i2c_client *client);
 bool pmbus_check_byte_register(struct i2c_client *client, int page, int reg);
 bool pmbus_check_word_register(struct i2c_client *client, int page, int reg);
 int pmbus_do_probe(struct i2c_client *client, struct pmbus_driver_info *info);
diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
index 3143b9e0316c..c52052b30bc8 100644
--- a/drivers/hwmon/pmbus/pmbus_core.c
+++ b/drivers/hwmon/pmbus/pmbus_core.c
@@ -3437,10 +3437,9 @@ static int pmbus_write_smbalert_mask(struct i2c_client *client, u8 page, u8 reg,
 	return ret;
 }
 
-static irqreturn_t pmbus_fault_handler(int irq, void *pdata)
+void pmbus_check_and_notify_faults(struct i2c_client *client)
 {
-	struct pmbus_data *data = pdata;
-	struct i2c_client *client = to_i2c_client(data->dev);
+	struct pmbus_data *data = i2c_get_clientdata(client);
 	int i, status, event;
 
 	guard(pmbus_lock)(client);
@@ -3453,6 +3452,15 @@ static irqreturn_t pmbus_fault_handler(int irq, void *pdata)
 	}
 
 	pmbus_clear_faults(client);
+}
+EXPORT_SYMBOL_NS_GPL(pmbus_check_and_notify_faults, "PMBUS");
+
+static irqreturn_t pmbus_fault_handler(int irq, void *pdata)
+{
+	struct pmbus_data *data = pdata;
+	struct i2c_client *client = to_i2c_client(data->dev);
+
+	pmbus_check_and_notify_faults(client);
 
 	return IRQ_HANDLED;
 }

-- 
2.43.0



^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v6 1/4] hwmon: (pmbus/core) export pmbus_check_and_notify_faults()
@ 2026-07-29 21:51   ` Vincent Jardin
  0 siblings, 0 replies; 21+ messages in thread
From: Vincent Jardin @ 2026-07-29 21:51 UTC (permalink / raw)
  To: Guenter Roeck, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Jonathan Corbet, Shuah Khan, Charles Hsu
  Cc: Markus Elfring, linux-hwmon, linux-kernel, devicetree, linux-doc,
	Vincent Jardin

Factor of pmbus_fault_handler() into an exported helper so
drivers can notify sequence from a polling work item on boards
for which the chip's SMBALERT# pin is not wired to the
CPU. The interrupt handler becomes a thin wrapper.

Like the SMBALERT# path, the helper notifies and then clears the
latched faults unconditionally, so a polling caller inherits exactly
the interrupt semantics.

Signed-off-by: Vincent Jardin <vjardin@free.fr>
---
 drivers/hwmon/pmbus/pmbus.h      |  1 +
 drivers/hwmon/pmbus/pmbus_core.c | 14 +++++++++++---
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/hwmon/pmbus/pmbus.h b/drivers/hwmon/pmbus/pmbus.h
index 23e3eda58870..1504d8b9e34c 100644
--- a/drivers/hwmon/pmbus/pmbus.h
+++ b/drivers/hwmon/pmbus/pmbus.h
@@ -560,6 +560,7 @@ int pmbus_write_byte_data(struct i2c_client *client, int page, u8 reg,
 int pmbus_update_byte_data(struct i2c_client *client, int page, u8 reg,
 			   u8 mask, u8 value);
 void pmbus_clear_faults(struct i2c_client *client);
+void pmbus_check_and_notify_faults(struct i2c_client *client);
 bool pmbus_check_byte_register(struct i2c_client *client, int page, int reg);
 bool pmbus_check_word_register(struct i2c_client *client, int page, int reg);
 int pmbus_do_probe(struct i2c_client *client, struct pmbus_driver_info *info);
diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
index 3143b9e0316c..c52052b30bc8 100644
--- a/drivers/hwmon/pmbus/pmbus_core.c
+++ b/drivers/hwmon/pmbus/pmbus_core.c
@@ -3437,10 +3437,9 @@ static int pmbus_write_smbalert_mask(struct i2c_client *client, u8 page, u8 reg,
 	return ret;
 }
 
-static irqreturn_t pmbus_fault_handler(int irq, void *pdata)
+void pmbus_check_and_notify_faults(struct i2c_client *client)
 {
-	struct pmbus_data *data = pdata;
-	struct i2c_client *client = to_i2c_client(data->dev);
+	struct pmbus_data *data = i2c_get_clientdata(client);
 	int i, status, event;
 
 	guard(pmbus_lock)(client);
@@ -3453,6 +3452,15 @@ static irqreturn_t pmbus_fault_handler(int irq, void *pdata)
 	}
 
 	pmbus_clear_faults(client);
+}
+EXPORT_SYMBOL_NS_GPL(pmbus_check_and_notify_faults, "PMBUS");
+
+static irqreturn_t pmbus_fault_handler(int irq, void *pdata)
+{
+	struct pmbus_data *data = pdata;
+	struct i2c_client *client = to_i2c_client(data->dev);
+
+	pmbus_check_and_notify_faults(client);
 
 	return IRQ_HANDLED;
 }

-- 
2.43.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH v6 1/4] hwmon: (pmbus/core) export pmbus_check_and_notify_faults()
  2026-07-29 21:51   ` Vincent Jardin
  (?)
@ 2026-07-29 22:01   ` sashiko-bot
  -1 siblings, 0 replies; 21+ messages in thread
From: sashiko-bot @ 2026-07-29 22:01 UTC (permalink / raw)
  To: Vincent Jardin; +Cc: linux-hwmon, robh, conor+dt, devicetree

> From: Vincent Jardin <vjardin@free.fr>
> 
> Factor of pmbus_fault_handler() into an exported helper so
> drivers can notify sequence from a polling work item on boards
> for which the chip's SMBALERT# pin is not wired to the
> CPU. The interrupt handler becomes a thin wrapper.
> 
> Like the SMBALERT# path, the helper notifies and then clears the
> latched faults unconditionally, so a polling caller inherits exactly
> the interrupt semantics.
> 
> Signed-off-by: Vincent Jardin <vjardin@free.fr>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729-mpq8646_v0-v6-0-98848151cdb4@free.fr?part=1


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v6 2/4] dt-bindings: hwmon: pmbus: add MPS MPQ8646
  2026-07-29 21:51 ` Vincent Jardin
@ 2026-07-29 21:51   ` Vincent Jardin
  -1 siblings, 0 replies; 21+ messages in thread
From: Vincent Jardin via B4 Relay @ 2026-07-29 21:51 UTC (permalink / raw)
  To: Guenter Roeck, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Jonathan Corbet, Shuah Khan, Charles Hsu
  Cc: Markus Elfring, linux-hwmon, linux-kernel, devicetree, linux-doc,
	Vincent Jardin

From: Vincent Jardin <vjardin@free.fr>

The MPS MPQ8646 PMBus step-down converter uses the same
mps,vout-fb-divider-ratio-permille property with the same constraints
(maximum 2047, default 706) as the mps,mpq8785, so add its compatible
to the existing schema.

Signed-off-by: Vincent Jardin <vjardin@free.fr>
---
 Documentation/devicetree/bindings/hwmon/pmbus/mps,mpq8785.yaml | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/mps,mpq8785.yaml b/Documentation/devicetree/bindings/hwmon/pmbus/mps,mpq8785.yaml
index 90970a0433e9..d7a32fb3b7af 100644
--- a/Documentation/devicetree/bindings/hwmon/pmbus/mps,mpq8785.yaml
+++ b/Documentation/devicetree/bindings/hwmon/pmbus/mps,mpq8785.yaml
@@ -18,6 +18,7 @@ properties:
       - mps,mpm3695
       - mps,mpm3695-25
       - mps,mpm82504
+      - mps,mpq8646
       - mps,mpq8785
 
   reg:
@@ -52,7 +53,9 @@ allOf:
   - if:
       properties:
         compatible:
-          const: mps,mpq8785
+          enum:
+            - mps,mpq8646
+            - mps,mpq8785
     then:
       properties:
         mps,vout-fb-divider-ratio-permille:

-- 
2.43.0



^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v6 2/4] dt-bindings: hwmon: pmbus: add MPS MPQ8646
@ 2026-07-29 21:51   ` Vincent Jardin
  0 siblings, 0 replies; 21+ messages in thread
From: Vincent Jardin @ 2026-07-29 21:51 UTC (permalink / raw)
  To: Guenter Roeck, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Jonathan Corbet, Shuah Khan, Charles Hsu
  Cc: Markus Elfring, linux-hwmon, linux-kernel, devicetree, linux-doc,
	Vincent Jardin

The MPS MPQ8646 PMBus step-down converter uses the same
mps,vout-fb-divider-ratio-permille property with the same constraints
(maximum 2047, default 706) as the mps,mpq8785, so add its compatible
to the existing schema.

Signed-off-by: Vincent Jardin <vjardin@free.fr>
---
 Documentation/devicetree/bindings/hwmon/pmbus/mps,mpq8785.yaml | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/mps,mpq8785.yaml b/Documentation/devicetree/bindings/hwmon/pmbus/mps,mpq8785.yaml
index 90970a0433e9..d7a32fb3b7af 100644
--- a/Documentation/devicetree/bindings/hwmon/pmbus/mps,mpq8785.yaml
+++ b/Documentation/devicetree/bindings/hwmon/pmbus/mps,mpq8785.yaml
@@ -18,6 +18,7 @@ properties:
       - mps,mpm3695
       - mps,mpm3695-25
       - mps,mpm82504
+      - mps,mpq8646
       - mps,mpq8785
 
   reg:
@@ -52,7 +53,9 @@ allOf:
   - if:
       properties:
         compatible:
-          const: mps,mpq8785
+          enum:
+            - mps,mpq8646
+            - mps,mpq8785
     then:
       properties:
         mps,vout-fb-divider-ratio-permille:

-- 
2.43.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH v6 2/4] dt-bindings: hwmon: pmbus: add MPS MPQ8646
  2026-07-29 21:51   ` Vincent Jardin
  (?)
@ 2026-07-29 22:01   ` sashiko-bot
  2026-07-30  4:12     ` Guenter Roeck
  -1 siblings, 1 reply; 21+ messages in thread
From: sashiko-bot @ 2026-07-29 22:01 UTC (permalink / raw)
  To: Vincent Jardin; +Cc: robh, conor+dt, linux-hwmon, devicetree

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The newly added compatible string 'mps,mpq8646' lacks driver support in the mpq8785 PMBus driver.
--

--- Patch [2]: [PATCH v6 2/4] dt-bindings: hwmon: pmbus: add MPS MPQ8646 ---
commit 341628a3dba9f658492690f20d45d96084eb12d5
Author: Vincent Jardin <vjardin@free.fr>

dt-bindings: hwmon: pmbus: add MPS MPQ8646

This commit adds the mps,mpq8646 compatible string to the existing MPQ8785
device tree schema. The MPQ8646 step-down converter shares the same
constraints for the mps,vout-fb-divider-ratio-permille property as the
mpq8785.

> diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/mps,mpq8785.yaml b/Documentation/devicetree/bindings/hwmon/pmbus/mps,mpq8785.yaml
> index 90970a0433e95..d7a32fb3b7af6 100644
> --- a/Documentation/devicetree/bindings/hwmon/pmbus/mps,mpq8785.yaml
> +++ b/Documentation/devicetree/bindings/hwmon/pmbus/mps,mpq8785.yaml
> @@ -18,6 +18,7 @@ properties:
>        - mps,mpm3695
>        - mps,mpm3695-25
>        - mps,mpm82504
> +      - mps,mpq8646

[Severity: Medium]
Should the corresponding driver (drivers/hwmon/pmbus/mpq8785.c) also be
updated to match this new compatible string?

Because the DT schema specifies the compatible string as an enum rather than
allowing a fallback (e.g. to "mps,mpq8785"), it appears the MPQ8646 device 
will fail to probe. The OF match table in mpq8785_of_match currently lacks 
an entry for this new hardware:

static const struct of_device_id __maybe_unused mpq8785_of_match[] = {
	{ .compatible = "mps,mpm3695", .data = (void *)mpm3695 },
	{ .compatible = "mps,mpm3695-25", .data = (void *)mpm3695_25 },
	{ .compatible = "mps,mpm82504", .data = (void *)mpm82504 },
	{ .compatible = "mps,mpq8785", .data = (void *)mpq8785 },
	{ }
};

The I2C ID table in mpq8785_id is similarly missing an "mpq8646" entry.
Without these additions, will a device instantiated via this new standalone
compatible string be completely ignored by the driver?

>        - mps,mpq8785

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729-mpq8646_v0-v6-0-98848151cdb4@free.fr?part=2

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v6 2/4] dt-bindings: hwmon: pmbus: add MPS MPQ8646
  2026-07-29 22:01   ` sashiko-bot
@ 2026-07-30  4:12     ` Guenter Roeck
  0 siblings, 0 replies; 21+ messages in thread
From: Guenter Roeck @ 2026-07-30  4:12 UTC (permalink / raw)
  To: sashiko-reviews, Vincent Jardin; +Cc: robh, conor+dt, linux-hwmon, devicetree

On 7/29/26 15:01, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] The newly added compatible string 'mps,mpq8646' lacks driver support in the mpq8785 PMBus driver.
> --
> 
> --- Patch [2]: [PATCH v6 2/4] dt-bindings: hwmon: pmbus: add MPS MPQ8646 ---
> commit 341628a3dba9f658492690f20d45d96084eb12d5
> Author: Vincent Jardin <vjardin@free.fr>
> 
> dt-bindings: hwmon: pmbus: add MPS MPQ8646
> 
> This commit adds the mps,mpq8646 compatible string to the existing MPQ8785
> device tree schema. The MPQ8646 step-down converter shares the same
> constraints for the mps,vout-fb-divider-ratio-permille property as the
> mpq8785.
> 
>> diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/mps,mpq8785.yaml b/Documentation/devicetree/bindings/hwmon/pmbus/mps,mpq8785.yaml
>> index 90970a0433e95..d7a32fb3b7af6 100644
>> --- a/Documentation/devicetree/bindings/hwmon/pmbus/mps,mpq8785.yaml
>> +++ b/Documentation/devicetree/bindings/hwmon/pmbus/mps,mpq8785.yaml
>> @@ -18,6 +18,7 @@ properties:
>>         - mps,mpm3695
>>         - mps,mpm3695-25
>>         - mps,mpm82504
>> +      - mps,mpq8646
> 
> [Severity: Medium]
> Should the corresponding driver (drivers/hwmon/pmbus/mpq8785.c) also be
> updated to match this new compatible string?
> 
> Because the DT schema specifies the compatible string as an enum rather than
> allowing a fallback (e.g. to "mps,mpq8785"), it appears the MPQ8646 device
> will fail to probe. The OF match table in mpq8785_of_match currently lacks
> an entry for this new hardware:
> 
> static const struct of_device_id __maybe_unused mpq8785_of_match[] = {
> 	{ .compatible = "mps,mpm3695", .data = (void *)mpm3695 },
> 	{ .compatible = "mps,mpm3695-25", .data = (void *)mpm3695_25 },
> 	{ .compatible = "mps,mpm82504", .data = (void *)mpm82504 },
> 	{ .compatible = "mps,mpq8785", .data = (void *)mpq8785 },
> 	{ }
> };
> 
> The I2C ID table in mpq8785_id is similarly missing an "mpq8646" entry.
> Without these additions, will a device instantiated via this new standalone
> compatible string be completely ignored by the driver?
> 
>>         - mps,mpq8785
> 

Sashiko for sure doesn't understand that the same bindings file is used for multiple
drivers. For DT maintainers - is there some common guidance for situations like this ?

Guenter


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v6 2/4] dt-bindings: hwmon: pmbus: add MPS MPQ8646
  2026-07-29 21:51   ` Vincent Jardin
  (?)
  (?)
@ 2026-07-30  4:10   ` Guenter Roeck
  2026-07-30  6:55     ` Vincent Jardin
  -1 siblings, 1 reply; 21+ messages in thread
From: Guenter Roeck @ 2026-07-30  4:10 UTC (permalink / raw)
  To: vjardin, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Jonathan Corbet, Shuah Khan, Charles Hsu
  Cc: Markus Elfring, linux-hwmon, linux-kernel, devicetree, linux-doc

On 7/29/26 14:51, Vincent Jardin via B4 Relay wrote:
> From: Vincent Jardin <vjardin@free.fr>
> 
> The MPS MPQ8646 PMBus step-down converter uses the same
> mps,vout-fb-divider-ratio-permille property with the same constraints
> (maximum 2047, default 706) as the mps,mpq8785, so add its compatible
> to the existing schema.
> 

The driver is a separate driver, making this a bit unusual.
I'll leave it up to the devicetree maintainers to decide if
this should be a new bindings file or not.

Guenter

> Signed-off-by: Vincent Jardin <vjardin@free.fr>
> ---
>   Documentation/devicetree/bindings/hwmon/pmbus/mps,mpq8785.yaml | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/mps,mpq8785.yaml b/Documentation/devicetree/bindings/hwmon/pmbus/mps,mpq8785.yaml
> index 90970a0433e9..d7a32fb3b7af 100644
> --- a/Documentation/devicetree/bindings/hwmon/pmbus/mps,mpq8785.yaml
> +++ b/Documentation/devicetree/bindings/hwmon/pmbus/mps,mpq8785.yaml
> @@ -18,6 +18,7 @@ properties:
>         - mps,mpm3695
>         - mps,mpm3695-25
>         - mps,mpm82504
> +      - mps,mpq8646
>         - mps,mpq8785
>   
>     reg:
> @@ -52,7 +53,9 @@ allOf:
>     - if:
>         properties:
>           compatible:
> -          const: mps,mpq8785
> +          enum:
> +            - mps,mpq8646
> +            - mps,mpq8785
>       then:
>         properties:
>           mps,vout-fb-divider-ratio-permille:
> 


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v6 2/4] dt-bindings: hwmon: pmbus: add MPS MPQ8646
  2026-07-30  4:10   ` Guenter Roeck
@ 2026-07-30  6:55     ` Vincent Jardin
  0 siblings, 0 replies; 21+ messages in thread
From: Vincent Jardin @ 2026-07-30  6:55 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Jonathan Corbet,
	Shuah Khan, Charles Hsu, Markus Elfring, linux-hwmon,
	linux-kernel, devicetree, linux-doc

Hi Guenter,

Le 29/07/26 21:10, Guenter Roeck a écrit :
> On 7/29/26 14:51, Vincent Jardin via B4 Relay wrote:
> > From: Vincent Jardin <vjardin@free.fr>
> > 
> > The MPS MPQ8646 PMBus step-down converter uses the same
> > mps,vout-fb-divider-ratio-permille property with the same constraints
> > (maximum 2047, default 706) as the mps,mpq8785, so add its compatible
> > to the existing schema.
> > 
> 
> The driver is a separate driver, making this a bit unusual.
> I'll leave it up to the devicetree maintainers to decide if
> this should be a new bindings file or not.

Because of Krzysztof's review of v5: he asked exactly for it
since mps,mpq8785.yaml already carries the same property, the same
constraints and the same defaults:

  see  https://lore.kernel.org/all/20260727-sceptical-finicky-waxbill-d7ecbf@quoll/
  "...Everything is the same as mps,mpq8785.yam, so why it cannot be added
  there? Just grow the list of if:then: to cover your specific
  constraints."

v5 had it as a separate mps,mpq8646.yaml file. I'll follow whatever
the DT maintainers prefer, but so should we consider that it complies
with Krzysztof's argument ?

Best regards,
  Vincent

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v6 3/4] hwmon: pmbus: add MPQ8646 driver
  2026-07-29 21:51 ` Vincent Jardin
@ 2026-07-29 21:51   ` Vincent Jardin
  -1 siblings, 0 replies; 21+ messages in thread
From: Vincent Jardin via B4 Relay @ 2026-07-29 21:51 UTC (permalink / raw)
  To: Guenter Roeck, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Jonathan Corbet, Shuah Khan, Charles Hsu
  Cc: Markus Elfring, linux-hwmon, linux-kernel, devicetree, linux-doc,
	Vincent Jardin

From: Vincent Jardin <vjardin@free.fr>

Add a new driver for the MPS MPQ8646 that is a PMBus device.

Beyond basic PMBus telemetry, the driver adds:
  - alarm acknowledge via inX_reset_history.
  - STATUS_WORD MPS-extended bit decode and the NVM-backed
    PROTECTION_LAST post-mortem, exposed as a read-only debugfs
    decoder.
  - In-driver alarm-poll fallback work item (thanks lm90) for
    boards without SMBALERT

Signed-off-by: Vincent Jardin <vjardin@free.fr>
---
 Documentation/hwmon/index.rst   |   1 +
 Documentation/hwmon/mpq8646.rst | 261 +++++++++++++++
 MAINTAINERS                     |   7 +
 drivers/hwmon/pmbus/Kconfig     |  11 +
 drivers/hwmon/pmbus/Makefile    |   1 +
 drivers/hwmon/pmbus/mpq8646.c   | 685 ++++++++++++++++++++++++++++++++++++++++
 6 files changed, 966 insertions(+)

diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst
index 29130df44d12..be52f9f4dc70 100644
--- a/Documentation/hwmon/index.rst
+++ b/Documentation/hwmon/index.rst
@@ -201,6 +201,7 @@ Hardware Monitoring Kernel Drivers
    mp5990
    mp9941
    mp9945
+   mpq8646
    mpq8785
    nct6683
    nct6775
diff --git a/Documentation/hwmon/mpq8646.rst b/Documentation/hwmon/mpq8646.rst
new file mode 100644
index 000000000000..d3ede656ea15
--- /dev/null
+++ b/Documentation/hwmon/mpq8646.rst
@@ -0,0 +1,261 @@
+.. SPDX-License-Identifier: GPL-2.0-only
+.. Copyright (c) 2026 Free Mobile - Vincent Jardin <vjardin@free.fr>
+
+Kernel driver mpq8646
+=====================
+
+Supported chips:
+
+  * MPS MPQ8646
+
+    Prefix: 'mpq8646'
+
+Author:
+  - Vincent Jardin <vjardin@free.fr>
+
+Chip-identity
+-------------
+
+Support the boards that are designed with the MPS MPQ8646 probed thanks
+to``MFR_MODEL`` register.
+
+This driver targets the MPQ8646 silicon specifically.
+
+Description
+-----------
+
+The MPQ8646 is a fully integrated, PMBus-compatible, high-frequency,
+synchronous buck converter. It offers a compact solution that
+achieves up high Amps output current per phase, with excellent load
+and line regulation over a wide input supply range. The chip
+operates at high efficiency over a wide output current load range.
+
+The PMBus interface provides converter configurations and key
+parameters monitoring.
+
+The device adopts MPS's proprietary multi-phase digital
+constant-on-time (MCOT) control, which provides fast transient
+response and eases loop stabilization. The MCOT scheme also allows
+multiple devices or channels to be connected in parallel with
+excellent current sharing and phase interleaving for high-current
+applications.
+
+Fully integrated protection features include over-current
+protection (OCP), over-voltage protection (OVP), under-voltage
+protection (UVP), and over-temperature protection (OTP).
+
+This device is compliant with:
+
+- PMBus rev 1.3 interface.
+
+The driver exports the following attributes via the 'sysfs' files
+for input voltage:
+
+- in1_input
+- in1_label
+- in1_max
+- in1_max_alarm
+- in1_min
+- in1_min_alarm
+- in1_crit
+- in1_crit_alarm
+
+The driver provides the following attributes for output voltage:
+
+- in2_input
+- in2_label
+- in2_alarm
+- in2_max
+- in2_max_alarm
+- in2_min
+- in2_min_alarm
+- in2_crit
+- in2_crit_alarm
+- in2_lcrit
+- in2_lcrit_alarm
+
+The driver provides the following attributes for output current:
+
+- curr1_input
+- curr1_label
+- curr1_max
+- curr1_max_alarm
+- curr1_crit
+- curr1_crit_alarm
+
+The driver provides the following attributes for temperature:
+
+- temp1_input
+- temp1_max
+- temp1_max_alarm
+- temp1_crit
+- temp1_crit_alarm
+
+Alarm acknowledgment
+---------------------
+
+The hwmon-class ``inX_alarm``/``currX_alarm``/``tempX_alarm`` files are
+read-only in pmbus_core. The driver exposes the standard
+``PMBUS_VIRT_RESET_*_HISTORY`` virtual-register channel for fault
+acknowledgment: writing ``1`` to ``inX_reset_history`` /
+``currX_reset_history`` / ``tempX_reset_history`` sends the chip a
+``CLEAR_FAULTS`` (0x03) Send-Byte, which clears the latched
+``STATUS_WORD`` / ``STATUS_VOUT`` / ``STATUS_IOUT`` /
+``STATUS_INPUT`` / ``STATUS_TEMPERATURE`` bits the chip is currently
+exposing.
+
+The MPS-specific NVM post-mortem register
+``PROTECTION_LAST`` (0xFB) is not cleared by this path; it can be read
+(decoded) via the ``protection_last`` debugfs entry described below.
+
+Regulator framework integration
+-------------------------------
+
+When ``CONFIG_REGULATOR=y`` is set, the chip is
+exposed under ``/sys/class/regulator/`` and accepts the standard
+regulator framework operations:
+
+- ``regulator_enable()`` / ``regulator_disable()`` -> ``OPERATION`` (0x01)
+- ``regulator_set_voltage()`` -> ``VOUT_COMMAND`` (0x21)
+- ``regulator_get_voltage()`` -> ``READ_VOUT`` (0x8B)
+- ``regulator_is_enabled()`` -> ``OPERATION`` bit-decode
+
+The single regulator descriptor is named ``"vout"`` (page 0). For a
+multi-page configuration the descriptor table can be extended in
+the driver.
+
+In-driver alarm-poll fallback
+-----------------------------
+
+On boards where the chip's ``SMBALERT#`` pin is unavailable to the
+SoC, ``pmbus_core::pmbus_irq_setup`` cannot deliver SMBALERT-driven
+``poll(POLLPRI)`` wakes or ``udev change@...`` events on the
+``inX_alarm`` files. The driver provides a ``delayed_work``-based
+polling fallback that
+periodically invokes the pmbus core fault check,
+``pmbus_check_and_notify_faults()``, which sends the notifications and
+then clears the latched faults, exactly like the ``SMBALERT#``
+interrupt path. The frequency of polling is tunable:
+
+::
+
+  /sys/kernel/debug/i2c/i2c-<bus>/<bus>-<addr>/alarm_poll_interval_ms
+
+The default is 1000 ms.
+
+Set it to 0 to disable. The worker self-suppresses
+when ``client->irq != 0`` (i.e. when DT supplies an
+``interrupts = <...>``  property on the regulator node), so adding
+``SMBALERT#`` wiring is a zero-driver-change uplift on a future
+board rev.
+
+MPS post-mortem (PROTECTION_LAST)
+---------------------------------
+
+The MPQ8646 silicon keeps a single 16-bit NVM-backed record of the
+last protection event in ``PROTECTION_LAST`` (0xFB). It survives
+chip power/reset. The following debugfs entries expose it:
+
+::
+
+  /sys/kernel/debug/i2c/i2c-<bus>/<bus>-<addr>/protection_last     (RO)
+  /sys/kernel/debug/i2c/i2c-<bus>/<bus>-<addr>/status_decoded      (RO)
+
+``protection_last`` decodes the 16-bit value based on the datasheet
+fault names (``INIT_FAULT``, ``NVM_CRC_ERROR``, ``NVM_FAULT``,
+``OC_PHASE_FAULT``, ``OTP_SELF_FAULT``, ``SWITCH_PRD_FAULT``,
+``VIN_OV_FAULT``, ``VOUT_OV_FAULT``, ``VOUT_UV_FAULT``,
+``OC_TOT_FAULT``, ``VIN_UVLO_FAULT``, ``DRMOS_OTP``).
+
+``status_decoded`` reads ``STATUS_WORD`` (0x79) and renders the
+16 bits with MPS-extension labels (bit12 = ``NVM_SUMMARY``,
+bit8 = ``WATCH_DOG``, bit0 = ``DRMOS_FAULT``) instead of the
+PMBus 1.3 spec generic names.
+
+NVMEM snapshot
+--------------
+
+When ``CONFIG_NVMEM=y`` is set, the chip's NVM-backed
+observability registers are exposed as a single 16-byte read-only
+``nvmem_device`` at ``/sys/bus/nvmem/devices/<i2c-name>/nvmem``.
+Layout (little-endian for 16-bit fields, zero-fill on per-entry read
+failure and for the reserved tail):
+
+::
+
+  offset 0..1   PROTECTION_LAST  (0xFB)  word
+  offset 2..3   MFR_RETRY_TIMES  (0xF4)  word
+  offset 4..5   MFR_CONFIG_ID    (0xC0)  word
+  offset 6..7   MFR_VBOOT_CFG    (0xFC)  word
+  offset 8      MFR_SILICON_REV  (0xC3)  byte
+  offset 9..15  reserved (zero)
+
+Suitable for single-``cat`` post-mortem capture by a fleet daemon.
+
+Diagnostics and introspection
+-----------------------------
+
+When ``CONFIG_DEBUG_FS=y`` is set, the driver adds a read-only
+decode surface to the client's pmbus debugfs directory,
+``/sys/kernel/debug/i2c/i2c-<bus>/<bus>-<addr>/``.
+The driver intentionally exposes no raw register poke/peek debugfs;
+use i2c-dev (``i2cget``/``i2cset``/``i2ctransfer``) for that.
+
+Identity / observability (read-only):
+
+==========================  ==================  =================================
+File                        PMBus / MFR cmd     Description
+==========================  ==================  =================================
+``mfr_config_id``           0xC0 (word)         board / SKU NVM identifier
+``mfr_config_code_rev``     0xC1 (word)         NVM image revision (PART_RECOG +
+                                                config code rev fields)
+``mfr_silicon_rev``         0xC3 (byte)         die revision
+``mfr_retry_times``         0xF4 (word)         per-fault-class recovery-mode
+                                                configuration (NOT a retry
+                                                count). Four 4-bit fields
+                                                [15:12]=OTP, [11:8]=VOUT_OV,
+                                                [7:4]=VOUT_UV, [3:0]=OCP.
+                                                Each field: 0x0=latch-off,
+                                                0x1..0xE=retry N times then
+                                                latch off, 0xF=hiccup
+                                                (retry indefinitely). The
+                                                chip exposes no in-NVM
+                                                retry-event counter; track
+                                                transitions externally if
+                                                needed (poll
+                                                ``protection_last`` or watch
+                                                ``inX_alarm`` / ``temp1_alarm``
+                                                ``poll(POLLPRI)`` wakes).
+``mfr_vboot_cfg``           0xFC (word)         ADDR/VBOOT latched at POR
+==========================  ==================  =================================
+
+Timing / UVLO knobs (read-only):
+
+==========================  ==================  =================================
+File                        PMBus cmd           Description
+==========================  ==================  =================================
+``vin_on``                  0x35 (word)         UVLO turn-on threshold
+``vin_off``                 0x36 (word)         UVLO turn-off threshold
+``ton_delay``               0x60 (word)         soft-start delay
+``ton_rise``                0x61 (word)         soft-start ramp time
+``toff_delay``              0x64 (word)         soft-stop delay
+``toff_fall``               0x65 (word)         soft-stop ramp time
+==========================  ==================  =================================
+
+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.
+
+Devicetree
+----------
+
+The driver uses ``compatible = "mps,mpq8646"``. There are some few optional
+properties:
+
+- ``mps,vout-fb-divider-ratio-permille`` : it writes ``VOUT_SCALE_LOOP``
+  (0x29) at probe to compensate for an external resistor divider in
+  the VOUT feedback path. The valid range is 11-bit.
+- ``interrupts = <...>`` : if the board routes the chip's
+  ``SMBALERT#`` pin to a SoC GPIO, declaring it here lights up
+  ``pmbus_core::pmbus_irq_setup`` and disables the in-driver
+  alarm-poll fallback
diff --git a/MAINTAINERS b/MAINTAINERS
index 1ab8736850ea..88dc83d98896 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -18328,6 +18328,13 @@ S:	Maintained
 F:	Documentation/hwmon/mp9945.rst
 F:	drivers/hwmon/pmbus/mp9945.c
 
+MPS MPQ8646 PMBUS DRIVER
+M:	Vincent Jardin <vjardin@free.fr>
+L:	linux-hwmon@vger.kernel.org
+S:	Maintained
+F:	Documentation/hwmon/mpq8646.rst
+F:	drivers/hwmon/pmbus/mpq8646.c
+
 MR800 AVERMEDIA USB FM RADIO DRIVER
 M:	Alexey Klimov <alexey.klimov@linaro.org>
 L:	linux-media@vger.kernel.org
diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
index c8cda160b5f8..9f44e76b0b64 100644
--- a/drivers/hwmon/pmbus/Kconfig
+++ b/drivers/hwmon/pmbus/Kconfig
@@ -616,6 +616,17 @@ config SENSORS_MPQ8785
 	  This driver can also be built as a module. If so, the module will
 	  be called mpq8785.
 
+config SENSORS_MPQ8646
+	tristate "MPS MPQ8646"
+	depends on REGULATOR || !REGULATOR
+	depends on NVMEM || !NVMEM
+	help
+	  If you say yes here you get hardware monitoring support for the
+	  Monolithic Power Systems MPQ8646.
+
+	  This driver can also be built as a module. If so, the module
+	  will be called mpq8646.
+
 config SENSORS_PIM4328
 	tristate "Flex PIM4328 and compatibles"
 	help
diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile
index ffc05f493213..6f8fda966600 100644
--- a/drivers/hwmon/pmbus/Makefile
+++ b/drivers/hwmon/pmbus/Makefile
@@ -60,6 +60,7 @@ obj-$(CONFIG_SENSORS_MP9941)	+= mp9941.o
 obj-$(CONFIG_SENSORS_MP9945)	+= mp9945.o
 obj-$(CONFIG_SENSORS_MPQ7932)	+= mpq7932.o
 obj-$(CONFIG_SENSORS_MPQ8785)	+= mpq8785.o
+obj-$(CONFIG_SENSORS_MPQ8646)	+= mpq8646.o
 obj-$(CONFIG_SENSORS_PLI1209BC)	+= pli1209bc.o
 obj-$(CONFIG_SENSORS_PM6764TR)	+= pm6764tr.o
 obj-$(CONFIG_SENSORS_PXE1610)	+= pxe1610.o
diff --git a/drivers/hwmon/pmbus/mpq8646.c b/drivers/hwmon/pmbus/mpq8646.c
new file mode 100644
index 000000000000..1fd41d89e5be
--- /dev/null
+++ b/drivers/hwmon/pmbus/mpq8646.c
@@ -0,0 +1,685 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Driver for MPS MPQ8646 step-down converter.
+ *
+ * Copyright (c) 2026 Free Mobile - Vincent Jardin <vjardin@free.fr>
+ */
+
+#include <linux/bitops.h>
+#include <linux/debugfs.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/nvmem-provider.h>
+#include <linux/of_device.h>
+#include <linux/pmbus.h>
+#include <linux/property.h>
+#include <linux/regulator/driver.h>
+#include <linux/seq_file.h>
+#include <linux/workqueue.h>
+#include "pmbus.h"
+
+/* Default cadence for the in-driver alarm-poll fallback (ms) */
+#define MPQ8646_ALARM_POLL_MS_DEFAULT	1000
+
+/* MPS vendor-extended command codes (NOT in PMBus 1.3 Part II) */
+#define MPS_PROTECTION_LAST		0xFB
+
+/* PMBus 1.3 timing / UVLO command codes */
+#define PMBUS_VIN_ON			0x35
+#define PMBUS_VIN_OFF			0x36
+#define PMBUS_TON_DELAY			0x60
+#define PMBUS_TON_RISE			0x61
+#define PMBUS_TOFF_DELAY		0x64
+#define PMBUS_TOFF_FALL			0x65
+
+/* MPS vendor-extended observability / identity registers */
+#define MPS_MFR_CONFIG_ID		0xC0
+#define MPS_MFR_CONFIG_CODE_REV		0xC1
+#define MPS_MFR_SILICON_REV		0xC3
+#define MPS_MFR_RETRY_TIMES		0xF4
+#define MPS_MFR_VBOOT_CFG		0xFC
+
+#define MPQ8646_DEBUG(client, fmt, ...) \
+	dev_dbg(&(client)->dev, fmt, ##__VA_ARGS__)
+
+/*
+ * Maximum legal value for VOUT_SCALE_LOOP (0x29) on the MPQ8646 silicon
+ * The chip uses an 11-bit VOUT feedback-divider scale register.
+ */
+#define MPQ8646_VOUT_SCALE_LOOP_MAX	GENMASK(10, 0)
+
+/* Forward declaration */
+struct mpq8646_dbg_reg_ctx;
+
+/* Per-instance state */
+struct mpq8646_priv {
+	struct pmbus_driver_info info;	/* must be first, container_of target */
+	struct i2c_client	*client;
+
+	/* Serialises CLEAR_LAST_FAULT sequences and PROTECTION_LAST reads */
+	struct mutex		mps_lock;
+
+	/* Set alarm_poll_interval_ms = 0 to disable. */
+	struct delayed_work	alarm_poll_work;
+	u32			alarm_poll_interval_ms;
+
+#ifdef CONFIG_DEBUG_FS
+	/* the only debugfs file that can re-arm the poll worker */
+	struct dentry		*dbg_poll;
+	struct mpq8646_dbg_reg_ctx	*dbg_reg_ctx;
+#endif
+};
+
+static inline struct mpq8646_priv *mpq8646_priv_from_client(struct i2c_client *client)
+{
+	const struct pmbus_driver_info *info = pmbus_get_driver_info(client);
+
+	return container_of(info, struct mpq8646_priv, info);
+}
+
+/*
+ * VID-mode m/b/R coefficients, same values as the mpq8785 driver for
+ * the MPS VID encoding. Per the PMBus Direct formula used by
+ * pmbus_core, X = (Y * 10^-R - b) / m, so m=64 / b=0 / R=1 yields
+ * 1.5625 mV per LSB.
+ */
+#define MPQ8646_VID_M			64
+#define MPQ8646_VID_B			0
+#define MPQ8646_VID_R			1
+
+static int mpq8646_identify(struct i2c_client *client,
+			    struct pmbus_driver_info *info)
+{
+	int vout_mode;
+
+	vout_mode = pmbus_read_byte_data(client, 0, PMBUS_VOUT_MODE);
+	if (vout_mode < 0)
+		return vout_mode;
+
+	switch (vout_mode & PB_VOUT_MODE_MODE_MASK) {
+	case PB_VOUT_MODE_LINEAR:
+		info->format[PSC_VOLTAGE_OUT] = linear;
+		break;
+	case PB_VOUT_MODE_VID:
+	case PB_VOUT_MODE_DIRECT:
+		info->format[PSC_VOLTAGE_OUT] = direct;
+		info->m[PSC_VOLTAGE_OUT] = MPQ8646_VID_M;
+		info->b[PSC_VOLTAGE_OUT] = MPQ8646_VID_B;
+		info->R[PSC_VOLTAGE_OUT] = MPQ8646_VID_R;
+		break;
+	default:
+		return -ENODEV;
+	}
+
+	return 0;
+};
+
+static int mpq8646_read_byte_data(struct i2c_client *client, int page, int reg)
+{
+	int ret;
+
+	MPQ8646_DEBUG(client, "read_byte_data page=%d reg=0x%02x\n", page, reg);
+
+	switch (reg) {
+	case PMBUS_VOUT_MODE:
+		ret = pmbus_read_byte_data(client, page, reg);
+		MPQ8646_DEBUG(client, "  VOUT_MODE raw=0x%02x ret=%d\n", ret, ret);
+		if (ret < 0)
+			return ret;
+
+		if ((ret & PB_VOUT_MODE_MODE_MASK) == PB_VOUT_MODE_VID)
+			return PB_VOUT_MODE_DIRECT;
+
+		return ret;
+	case PMBUS_STATUS_BYTE:
+	case PMBUS_STATUS_CML:
+	case PMBUS_STATUS_OTHER:
+	case PMBUS_STATUS_MFR_SPECIFIC:
+	case PMBUS_STATUS_FAN_12:
+	case PMBUS_STATUS_FAN_34:
+		return -ENXIO;
+	case PMBUS_MFR_LOCATION:
+	case PMBUS_MFR_DATE:
+	case PMBUS_MFR_SERIAL:
+	case PMBUS_IC_DEVICE_ID:
+	case PMBUS_IC_DEVICE_REV:
+		MPQ8646_DEBUG(client, "  unsupported mfr-info reg 0x%02x -> ENXIO\n", reg);
+		return -ENXIO;
+	default:
+		return -ENODATA;
+	}
+}
+
+/*
+ * Reference: ltc2978.c::ltc2978_write_word_data which uses the
+ * same virtual-register channel for its real chip-side peak reset.
+ */
+static int mpq8646_write_word_data(struct i2c_client *client, int page,
+				   int reg, u16 word)
+{
+	struct mpq8646_priv *priv = mpq8646_priv_from_client(client);
+	int rc;
+
+	switch (reg) {
+	case PMBUS_VIRT_RESET_VIN_HISTORY:
+	case PMBUS_VIRT_RESET_VOUT_HISTORY:
+	case PMBUS_VIRT_RESET_IOUT_HISTORY:
+	case PMBUS_VIRT_RESET_TEMP_HISTORY:
+		MPQ8646_DEBUG(client, "reset_history virt reg=0x%04x -> CLEAR_FAULTS\n",
+			      reg);
+		rc = i2c_smbus_write_byte(priv->client, PMBUS_CLEAR_FAULTS);
+		if (rc < 0)
+			MPQ8646_DEBUG(client, "  CLEAR_FAULTS rc=%d\n", rc);
+		return rc < 0 ? rc : 0;
+	default:
+		return -ENODATA;	/* let pmbus_core do the direct write */
+	}
+}
+
+static int mpq8646_read_word_data(struct i2c_client *client, int page,
+				  int phase, int reg)
+{
+	int rc;
+
+	MPQ8646_DEBUG(client, "read_word_data page=%d phase=%d reg=0x%02x\n",
+		      page, phase, reg);
+
+	switch (reg) {
+	case PMBUS_READ_VIN:
+	case PMBUS_READ_VOUT:
+	case PMBUS_READ_IOUT:
+	case PMBUS_READ_TEMPERATURE_1:
+	case PMBUS_STATUS_WORD:
+	case PMBUS_VOUT_OV_FAULT_LIMIT:
+	case PMBUS_VOUT_OV_WARN_LIMIT:
+	case PMBUS_VOUT_UV_WARN_LIMIT:
+	case PMBUS_VOUT_UV_FAULT_LIMIT:
+	case PMBUS_IOUT_OC_FAULT_LIMIT:
+	case PMBUS_IOUT_OC_WARN_LIMIT:
+	case PMBUS_OT_FAULT_LIMIT:
+	case PMBUS_OT_WARN_LIMIT:
+	case PMBUS_VIN_OV_FAULT_LIMIT:
+	case PMBUS_VIN_OV_WARN_LIMIT:
+	case PMBUS_VIN_UV_WARN_LIMIT:
+	case PMBUS_VIN_UV_FAULT_LIMIT:
+	case PMBUS_MFR_VIN_MAX:
+	case PMBUS_MFR_VOUT_MAX:
+	case PMBUS_MFR_IOUT_MAX:
+	case PMBUS_MFR_MAX_TEMP_1:
+		break;
+	case PMBUS_VIRT_RESET_VIN_HISTORY:
+	case PMBUS_VIRT_RESET_VOUT_HISTORY:
+	case PMBUS_VIRT_RESET_IOUT_HISTORY:
+	case PMBUS_VIRT_RESET_TEMP_HISTORY:
+		return 0;
+	default:
+		return -ENODATA;
+	}
+
+	rc = i2c_smbus_read_word_data(client, reg);
+
+	MPQ8646_DEBUG(client, "  reg=0x%02x rc=%d\n", reg, rc);
+	return rc;
+}
+
+static struct pmbus_driver_info mpq8646_info = {
+	.pages = 1,
+	.format[PSC_VOLTAGE_IN] = direct,
+	.format[PSC_CURRENT_OUT] = direct,
+	.format[PSC_TEMPERATURE] = direct,
+	.m[PSC_VOLTAGE_IN] = 4,
+	.b[PSC_VOLTAGE_IN] = 0,
+	.R[PSC_VOLTAGE_IN] = 1,
+	.m[PSC_CURRENT_OUT] = 16,
+	.b[PSC_CURRENT_OUT] = 0,
+	.R[PSC_CURRENT_OUT] = 0,
+	.m[PSC_TEMPERATURE] = 1,
+	.b[PSC_TEMPERATURE] = 0,
+	.R[PSC_TEMPERATURE] = 0,
+	.func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_VOUT |
+		   PMBUS_HAVE_IOUT | PMBUS_HAVE_TEMP |
+		   PMBUS_HAVE_STATUS_INPUT | PMBUS_HAVE_STATUS_VOUT |
+		   PMBUS_HAVE_STATUS_IOUT | PMBUS_HAVE_STATUS_TEMP,
+};
+
+#if IS_ENABLED(CONFIG_REGULATOR)
+static const struct regulator_desc mpq8646_reg_desc[] = {
+	PMBUS_REGULATOR("vout", 0),
+};
+#endif /* CONFIG_REGULATOR */
+
+#if IS_ENABLED(CONFIG_NVMEM)
+/*
+ * Expose using
+ *   /sys/bus/nvmem/devices/<i2c-name>/nvmem
+ *
+ * Layout (16 bytes):
+ *   offset 0..1   PROTECTION_LAST (0xFB)    word, LE     -- NVM
+ *   offset 2..3   MFR_RETRY_TIMES (0xF4)    word, LE     -- NVM
+ *   offset 4..5   MFR_CONFIG_ID   (0xC0)    word, LE     -- NVM
+ *   offset 6..7   MFR_VBOOT_CFG   (0xFC)    word, LE     -- NVM
+ *   offset 8      MFR_SILICON_REV (0xC3)    byte         -- NVM
+ *   offset 9..15  reserved (zero-fill, leaves room for additions)
+ */
+#define MPQ8646_NVMEM_SIZE	16
+
+struct mpq8646_nvmem_entry {
+	unsigned int	off;
+	u8		reg;
+	bool		is_word;
+};
+
+static const struct mpq8646_nvmem_entry mpq8646_nvmem_map[] = {
+	{ 0, MPS_PROTECTION_LAST, true  },
+	{ 2, MPS_MFR_RETRY_TIMES, true  },
+	{ 4, MPS_MFR_CONFIG_ID,   true  },
+	{ 6, MPS_MFR_VBOOT_CFG,   true  },
+	{ 8, MPS_MFR_SILICON_REV, false },
+};
+
+static int mpq8646_nvmem_read(void *data, unsigned int offset, void *val,
+			      size_t bytes)
+{
+	struct mpq8646_priv *priv = data;
+	u8 *out = val;
+	size_t i;
+
+	if (offset >= MPQ8646_NVMEM_SIZE)
+		return -EINVAL;
+	if (offset + bytes > MPQ8646_NVMEM_SIZE)
+		bytes = MPQ8646_NVMEM_SIZE - offset;
+
+	memset(out, 0, bytes);
+
+	guard(mutex)(&priv->mps_lock);
+	for (i = 0; i < ARRAY_SIZE(mpq8646_nvmem_map); i++) {
+		const struct mpq8646_nvmem_entry *e = &mpq8646_nvmem_map[i];
+		unsigned int e_start = e->off;
+		unsigned int e_end = e_start + (e->is_word ? 2 : 1);
+		u8 raw[2];
+		int rc;
+		unsigned int j;
+
+		if (e_end <= offset || e_start >= offset + bytes)
+			continue;	/* outside requested slice */
+
+		if (e->is_word)
+			rc = i2c_smbus_read_word_data(priv->client, e->reg);
+		else
+			rc = i2c_smbus_read_byte_data(priv->client, e->reg);
+		if (rc < 0)
+			continue;	/* leave the zero-fill in place */
+
+		raw[0] = rc & 0xff;
+		raw[1] = (rc >> 8) & 0xff;
+
+		for (j = 0; j < e_end - e_start; j++) {
+			unsigned int abs = e_start + j;
+
+			if (abs >= offset && abs < offset + bytes)
+				out[abs - offset] = raw[j];
+		}
+	}
+	return 0;
+}
+#endif /* CONFIG_NVMEM */
+
+static const struct i2c_device_id mpq8646_id[] = {
+	{ "mpq8646", 0 },
+	{ },
+};
+MODULE_DEVICE_TABLE(i2c, mpq8646_id);
+
+static const struct of_device_id __maybe_unused mpq8646_of_match[] = {
+	{ .compatible = "mps,mpq8646" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, mpq8646_of_match);
+
+static struct pmbus_platform_data mpq8646_no_pec_pdata = {
+	.flags = PMBUS_NO_CAPABILITY,
+};
+
+#ifdef CONFIG_DEBUG_FS
+/*
+ * Read-only decode cases in the client's pmbus debugfs directory:
+ * the MPS-specific STATUS_WORD and PROTECTION_LAST bit decode plus the
+ * identity/timing registers.
+ */
+
+struct mpq_status_bit {
+	u16		mask;
+	const char	*name;
+};
+
+static const struct mpq_status_bit mpq8646_status_word_bits[] = {
+	{ PB_STATUS_VOUT,         "VOUT" },
+	{ PB_STATUS_IOUT_POUT,    "IOUT_POUT" },
+	{ PB_STATUS_INPUT,        "INPUT" },
+	{ PB_STATUS_WORD_MFR,     "NVM_SUMMARY" },
+	{ PB_STATUS_POWER_GOOD_N, "POWER_GOOD#" },
+	{ PB_STATUS_FANS,         "FANS" },
+	{ PB_STATUS_OTHER,        "OTHER" },
+	{ PB_STATUS_UNKNOWN,      "WATCH_DOG" },
+	{ PB_STATUS_BUSY,         "BUSY" },
+	{ PB_STATUS_OFF,          "OFF" },
+	{ PB_STATUS_VOUT_OV,      "VOUT_OV_FAULT" },
+	{ PB_STATUS_IOUT_OC,      "IOUT_OC_FAULT" },
+	{ PB_STATUS_VIN_UV,       "VIN_UV_FAULT" },
+	{ PB_STATUS_TEMPERATURE,  "TEMP" },
+	{ PB_STATUS_CML,          "CML" },
+	{ PB_STATUS_NONE_ABOVE,   "DRMOS_FAULT" },
+	{ /* sentinel */ }
+};
+
+/* PROTECTION_LAST (0xFB) bit names, it survives chip POR */
+static const struct mpq_status_bit mpq8646_protection_last_bits[] = {
+	{ BIT(15), "INIT_FAULT" },
+	{ BIT(14), "NVM_CRC_ERROR" },
+	{ BIT(13), "NVM_FAULT" },
+	{ BIT(12), "OC_PHASE_FAULT" },
+	{ BIT(11), "OTP_SELF_FAULT" },
+	{ BIT(9),  "SWITCH_PRD_FAULT" },
+	{ BIT(8),  "VIN_OV_FAULT" },
+	{ BIT(7),  "VOUT_OV_FAULT" },
+	{ BIT(6),  "VOUT_UV_FAULT" },
+	{ BIT(5),  "OC_TOT_FAULT" },
+	{ BIT(4),  "VIN_UVLO_FAULT" },
+	{ BIT(3),  "DRMOS_OTP" },
+	{ /* sentinel */ }
+};
+
+static void mpq8646_print_bits(struct seq_file *s, u16 v,
+			       const struct mpq_status_bit *tab)
+{
+	const struct mpq_status_bit *t;
+	bool first = true;
+
+	seq_printf(s, "0x%04x", v);
+	if (!v) {
+		seq_puts(s, " [clean]\n");
+		return;
+	}
+	seq_puts(s, " [");
+	for (t = tab; t->mask; t++) {
+		if (v & t->mask) {
+			if (!first)
+				seq_putc(s, ' ');
+			seq_puts(s, t->name);
+			first = false;
+		}
+	}
+	seq_puts(s, "]\n");
+}
+
+static int mpq8646_dbg_status_decoded_show(struct seq_file *s, void *unused)
+{
+	struct mpq8646_priv *priv = s->private;
+	int rc;
+
+	rc = i2c_smbus_read_word_data(priv->client, PMBUS_STATUS_WORD);
+	if (rc < 0) {
+		seq_printf(s, "ERROR: STATUS_WORD read failed (%d)\n", rc);
+		return 0;
+	}
+	seq_puts(s, "STATUS_WORD: ");
+	mpq8646_print_bits(s, (u16)rc, mpq8646_status_word_bits);
+	seq_puts(s, "(MPS extensions: bit12=NVM_SUMMARY, bit8=WATCH_DOG, bit0=DRMOS_FAULT)\n");
+	return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(mpq8646_dbg_status_decoded);
+
+static int mpq8646_dbg_protection_last_show(struct seq_file *s, void *unused)
+{
+	struct mpq8646_priv *priv = s->private;
+	int rc;
+
+	scoped_guard(mutex, &priv->mps_lock)
+		rc = i2c_smbus_read_word_data(priv->client, MPS_PROTECTION_LAST);
+
+	if (rc < 0) {
+		seq_printf(s, "ERROR: PROTECTION_LAST read failed (%d)\n", rc);
+		return 0;
+	}
+	seq_puts(s, "PROTECTION_LAST: ");
+	mpq8646_print_bits(s, (u16)rc, mpq8646_protection_last_bits);
+	seq_puts(s, "(NVM-backed, survives chip POR)\n");
+	return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(mpq8646_dbg_protection_last);
+
+struct mpq8646_dbg_reg {
+	u8		reg;
+	bool		is_word;
+	const char	*name;
+};
+
+static const struct mpq8646_dbg_reg mpq8646_dbg_regs[] = {
+	/* MPS vendor extensions (read-only identity / observability) */
+	{ MPS_MFR_CONFIG_ID,        true,  "mfr_config_id" },
+	{ MPS_MFR_CONFIG_CODE_REV,  true,  "mfr_config_code_rev" },
+	{ MPS_MFR_SILICON_REV,      false, "mfr_silicon_rev" },
+	{ MPS_MFR_RETRY_TIMES,      true,  "mfr_retry_times" },
+	{ MPS_MFR_VBOOT_CFG,        true,  "mfr_vboot_cfg" },
+	/* PMBus 1.3 standard timing / UVLO (read-only introspection) */
+	{ PMBUS_VIN_ON,             true,  "vin_on" },
+	{ PMBUS_VIN_OFF,            true,  "vin_off" },
+	{ PMBUS_TON_DELAY,          true,  "ton_delay" },
+	{ PMBUS_TON_RISE,           true,  "ton_rise" },
+	{ PMBUS_TOFF_DELAY,         true,  "toff_delay" },
+	{ PMBUS_TOFF_FALL,          true,  "toff_fall" },
+};
+
+struct mpq8646_dbg_reg_ctx {
+	struct mpq8646_priv		*priv;
+	const struct mpq8646_dbg_reg	*desc;
+};
+
+static int mpq8646_dbg_reg_show(struct seq_file *s, void *unused)
+{
+	struct mpq8646_dbg_reg_ctx *ctx = s->private;
+	int rc;
+
+	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) {
+		seq_printf(s, "ERROR: reg 0x%02x (%s) read failed (%d)\n",
+			   ctx->desc->reg, ctx->desc->name, rc);
+		return 0;
+	}
+	seq_printf(s, "0x%0*x\n", ctx->desc->is_word ? 4 : 2, rc);
+	return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(mpq8646_dbg_reg);
+
+static int mpq8646_dbg_poll_interval_get(void *data, u64 *val)
+{
+	struct mpq8646_priv *priv = data;
+
+	*val = priv->alarm_poll_interval_ms;
+	return 0;
+}
+
+static int mpq8646_dbg_poll_interval_set(void *data, u64 val)
+{
+	struct mpq8646_priv *priv = data;
+	bool was_off = !priv->alarm_poll_interval_ms;
+
+	priv->alarm_poll_interval_ms = (u32)val;
+
+	if (val && was_off && !priv->client->irq)
+		schedule_delayed_work(&priv->alarm_poll_work,
+				      msecs_to_jiffies((u32)val));
+	return 0;
+}
+DEFINE_DEBUGFS_ATTRIBUTE(mpq8646_dbg_poll_interval_fops,
+			 mpq8646_dbg_poll_interval_get,
+			 mpq8646_dbg_poll_interval_set, "%llu\n");
+
+static void mpq8646_debugfs_register(struct mpq8646_priv *priv)
+{
+	struct dentry *root;
+	size_t i;
+
+	root = pmbus_get_debugfs_dir(priv->client);
+	if (!root)
+		return;
+
+	/* MPS extensions: status decode + NVM-backed PROTECTION_LAST */
+	debugfs_create_file("status_decoded", 0400, root, priv,
+			    &mpq8646_dbg_status_decoded_fops);
+	debugfs_create_file("protection_last", 0400, root, priv,
+			    &mpq8646_dbg_protection_last_fops);
+	priv->dbg_poll =
+		debugfs_create_file_unsafe("alarm_poll_interval_ms", 0600,
+					   root, priv,
+					   &mpq8646_dbg_poll_interval_fops);
+
+	priv->dbg_reg_ctx = devm_kcalloc(&priv->client->dev,
+					 ARRAY_SIZE(mpq8646_dbg_regs),
+					 sizeof(*priv->dbg_reg_ctx),
+					 GFP_KERNEL);
+	if (!priv->dbg_reg_ctx)
+		return;
+	for (i = 0; i < ARRAY_SIZE(mpq8646_dbg_regs); i++) {
+		priv->dbg_reg_ctx[i].priv = priv;
+		priv->dbg_reg_ctx[i].desc = &mpq8646_dbg_regs[i];
+		debugfs_create_file(mpq8646_dbg_regs[i].name, 0400,
+				    root, &priv->dbg_reg_ctx[i],
+				    &mpq8646_dbg_reg_fops);
+	}
+}
+
+static void mpq8646_debugfs_unregister(struct mpq8646_priv *priv)
+{
+	debugfs_remove(priv->dbg_poll);
+}
+#else
+static inline void mpq8646_debugfs_register(struct mpq8646_priv *priv) {}
+static inline void mpq8646_debugfs_unregister(struct mpq8646_priv *priv) {}
+#endif /* CONFIG_DEBUG_FS */
+
+static void mpq8646_alarm_poll_work(struct work_struct *work)
+{
+	struct mpq8646_priv *priv = container_of(to_delayed_work(work),
+						 struct mpq8646_priv,
+						 alarm_poll_work);
+
+	if (priv->client->irq)
+		return;	/* SMBALERT# wired; polling not needed */
+
+	if (!priv->alarm_poll_interval_ms)
+		return;	/* polling disabled; don't re-arm */
+
+	pmbus_check_and_notify_faults(priv->client);
+
+	schedule_delayed_work(&priv->alarm_poll_work,
+			      msecs_to_jiffies(priv->alarm_poll_interval_ms));
+}
+
+static int mpq8646_probe(struct i2c_client *client)
+{
+	struct device *dev = &client->dev;
+	struct pmbus_driver_info *info;
+	struct mpq8646_priv *priv;
+	u32 voltage_scale;
+	int ret;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+	priv->client = client;
+	mutex_init(&priv->mps_lock);
+	memcpy(&priv->info, &mpq8646_info, sizeof(priv->info));
+	info = &priv->info;
+
+	info->identify = mpq8646_identify;
+	info->read_byte_data = mpq8646_read_byte_data;
+	info->read_word_data = mpq8646_read_word_data;
+	info->write_word_data = mpq8646_write_word_data;
+	dev->platform_data = &mpq8646_no_pec_pdata;
+
+#if IS_ENABLED(CONFIG_REGULATOR)
+	info->reg_desc = mpq8646_reg_desc;
+	info->num_regulators = ARRAY_SIZE(mpq8646_reg_desc);
+#endif
+
+	INIT_DELAYED_WORK(&priv->alarm_poll_work, mpq8646_alarm_poll_work);
+	priv->alarm_poll_interval_ms = MPQ8646_ALARM_POLL_MS_DEFAULT;
+
+	if (!device_property_read_u32(dev, "mps,vout-fb-divider-ratio-permille",
+				      &voltage_scale)) {
+		if (voltage_scale > MPQ8646_VOUT_SCALE_LOOP_MAX)
+			return -EINVAL;
+
+		ret = i2c_smbus_write_word_data(client, PMBUS_VOUT_SCALE_LOOP,
+						voltage_scale);
+		if (ret)
+			return ret;
+	}
+
+	ret = pmbus_do_probe(client, info);
+	if (ret)
+		return ret;
+
+	mpq8646_debugfs_register(priv);
+
+	if (!client->irq)
+		schedule_delayed_work(&priv->alarm_poll_work,
+				      msecs_to_jiffies(priv->alarm_poll_interval_ms));
+
+#if IS_ENABLED(CONFIG_NVMEM)
+	{
+		struct nvmem_config cfg = {
+			.dev		= &client->dev,
+			.name		= dev_name(&client->dev),
+			.owner		= THIS_MODULE,
+			.read_only	= true,
+			.root_only	= true,
+			.word_size	= 1,
+			.stride		= 1,
+			.size		= MPQ8646_NVMEM_SIZE,
+			.reg_read	= mpq8646_nvmem_read,
+			.priv		= priv,
+		};
+		struct nvmem_device *nv = devm_nvmem_register(&client->dev, &cfg);
+
+		if (IS_ERR(nv))
+			dev_warn(&client->dev,
+				 "nvmem snapshot register failed (%pe)\n",
+				 nv);
+	}
+#endif
+	return 0;
+};
+
+static void mpq8646_remove(struct i2c_client *client)
+{
+	struct mpq8646_priv *priv = mpq8646_priv_from_client(client);
+
+	mpq8646_debugfs_unregister(priv);
+	cancel_delayed_work_sync(&priv->alarm_poll_work);
+}
+
+static struct i2c_driver mpq8646_driver = {
+	.driver = {
+		   .name = "mpq8646",
+		   .of_match_table = of_match_ptr(mpq8646_of_match),
+	},
+	.probe = mpq8646_probe,
+	.remove = mpq8646_remove,
+	.id_table = mpq8646_id,
+};
+
+module_i2c_driver(mpq8646_driver);
+
+MODULE_AUTHOR("Vincent Jardin <vjardin@free.fr>");
+MODULE_DESCRIPTION("PMBus driver for MPS MPQ8646 (extended observability)");
+MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS("PMBUS");

-- 
2.43.0



^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v6 3/4] hwmon: pmbus: add MPQ8646 driver
@ 2026-07-29 21:51   ` Vincent Jardin
  0 siblings, 0 replies; 21+ messages in thread
From: Vincent Jardin @ 2026-07-29 21:51 UTC (permalink / raw)
  To: Guenter Roeck, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Jonathan Corbet, Shuah Khan, Charles Hsu
  Cc: Markus Elfring, linux-hwmon, linux-kernel, devicetree, linux-doc,
	Vincent Jardin

Add a new driver for the MPS MPQ8646 that is a PMBus device.

Beyond basic PMBus telemetry, the driver adds:
  - alarm acknowledge via inX_reset_history.
  - STATUS_WORD MPS-extended bit decode and the NVM-backed
    PROTECTION_LAST post-mortem, exposed as a read-only debugfs
    decoder.
  - In-driver alarm-poll fallback work item (thanks lm90) for
    boards without SMBALERT

Signed-off-by: Vincent Jardin <vjardin@free.fr>
---
 Documentation/hwmon/index.rst   |   1 +
 Documentation/hwmon/mpq8646.rst | 261 +++++++++++++++
 MAINTAINERS                     |   7 +
 drivers/hwmon/pmbus/Kconfig     |  11 +
 drivers/hwmon/pmbus/Makefile    |   1 +
 drivers/hwmon/pmbus/mpq8646.c   | 685 ++++++++++++++++++++++++++++++++++++++++
 6 files changed, 966 insertions(+)

diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst
index 29130df44d12..be52f9f4dc70 100644
--- a/Documentation/hwmon/index.rst
+++ b/Documentation/hwmon/index.rst
@@ -201,6 +201,7 @@ Hardware Monitoring Kernel Drivers
    mp5990
    mp9941
    mp9945
+   mpq8646
    mpq8785
    nct6683
    nct6775
diff --git a/Documentation/hwmon/mpq8646.rst b/Documentation/hwmon/mpq8646.rst
new file mode 100644
index 000000000000..d3ede656ea15
--- /dev/null
+++ b/Documentation/hwmon/mpq8646.rst
@@ -0,0 +1,261 @@
+.. SPDX-License-Identifier: GPL-2.0-only
+.. Copyright (c) 2026 Free Mobile - Vincent Jardin <vjardin@free.fr>
+
+Kernel driver mpq8646
+=====================
+
+Supported chips:
+
+  * MPS MPQ8646
+
+    Prefix: 'mpq8646'
+
+Author:
+  - Vincent Jardin <vjardin@free.fr>
+
+Chip-identity
+-------------
+
+Support the boards that are designed with the MPS MPQ8646 probed thanks
+to``MFR_MODEL`` register.
+
+This driver targets the MPQ8646 silicon specifically.
+
+Description
+-----------
+
+The MPQ8646 is a fully integrated, PMBus-compatible, high-frequency,
+synchronous buck converter. It offers a compact solution that
+achieves up high Amps output current per phase, with excellent load
+and line regulation over a wide input supply range. The chip
+operates at high efficiency over a wide output current load range.
+
+The PMBus interface provides converter configurations and key
+parameters monitoring.
+
+The device adopts MPS's proprietary multi-phase digital
+constant-on-time (MCOT) control, which provides fast transient
+response and eases loop stabilization. The MCOT scheme also allows
+multiple devices or channels to be connected in parallel with
+excellent current sharing and phase interleaving for high-current
+applications.
+
+Fully integrated protection features include over-current
+protection (OCP), over-voltage protection (OVP), under-voltage
+protection (UVP), and over-temperature protection (OTP).
+
+This device is compliant with:
+
+- PMBus rev 1.3 interface.
+
+The driver exports the following attributes via the 'sysfs' files
+for input voltage:
+
+- in1_input
+- in1_label
+- in1_max
+- in1_max_alarm
+- in1_min
+- in1_min_alarm
+- in1_crit
+- in1_crit_alarm
+
+The driver provides the following attributes for output voltage:
+
+- in2_input
+- in2_label
+- in2_alarm
+- in2_max
+- in2_max_alarm
+- in2_min
+- in2_min_alarm
+- in2_crit
+- in2_crit_alarm
+- in2_lcrit
+- in2_lcrit_alarm
+
+The driver provides the following attributes for output current:
+
+- curr1_input
+- curr1_label
+- curr1_max
+- curr1_max_alarm
+- curr1_crit
+- curr1_crit_alarm
+
+The driver provides the following attributes for temperature:
+
+- temp1_input
+- temp1_max
+- temp1_max_alarm
+- temp1_crit
+- temp1_crit_alarm
+
+Alarm acknowledgment
+---------------------
+
+The hwmon-class ``inX_alarm``/``currX_alarm``/``tempX_alarm`` files are
+read-only in pmbus_core. The driver exposes the standard
+``PMBUS_VIRT_RESET_*_HISTORY`` virtual-register channel for fault
+acknowledgment: writing ``1`` to ``inX_reset_history`` /
+``currX_reset_history`` / ``tempX_reset_history`` sends the chip a
+``CLEAR_FAULTS`` (0x03) Send-Byte, which clears the latched
+``STATUS_WORD`` / ``STATUS_VOUT`` / ``STATUS_IOUT`` /
+``STATUS_INPUT`` / ``STATUS_TEMPERATURE`` bits the chip is currently
+exposing.
+
+The MPS-specific NVM post-mortem register
+``PROTECTION_LAST`` (0xFB) is not cleared by this path; it can be read
+(decoded) via the ``protection_last`` debugfs entry described below.
+
+Regulator framework integration
+-------------------------------
+
+When ``CONFIG_REGULATOR=y`` is set, the chip is
+exposed under ``/sys/class/regulator/`` and accepts the standard
+regulator framework operations:
+
+- ``regulator_enable()`` / ``regulator_disable()`` -> ``OPERATION`` (0x01)
+- ``regulator_set_voltage()`` -> ``VOUT_COMMAND`` (0x21)
+- ``regulator_get_voltage()`` -> ``READ_VOUT`` (0x8B)
+- ``regulator_is_enabled()`` -> ``OPERATION`` bit-decode
+
+The single regulator descriptor is named ``"vout"`` (page 0). For a
+multi-page configuration the descriptor table can be extended in
+the driver.
+
+In-driver alarm-poll fallback
+-----------------------------
+
+On boards where the chip's ``SMBALERT#`` pin is unavailable to the
+SoC, ``pmbus_core::pmbus_irq_setup`` cannot deliver SMBALERT-driven
+``poll(POLLPRI)`` wakes or ``udev change@...`` events on the
+``inX_alarm`` files. The driver provides a ``delayed_work``-based
+polling fallback that
+periodically invokes the pmbus core fault check,
+``pmbus_check_and_notify_faults()``, which sends the notifications and
+then clears the latched faults, exactly like the ``SMBALERT#``
+interrupt path. The frequency of polling is tunable:
+
+::
+
+  /sys/kernel/debug/i2c/i2c-<bus>/<bus>-<addr>/alarm_poll_interval_ms
+
+The default is 1000 ms.
+
+Set it to 0 to disable. The worker self-suppresses
+when ``client->irq != 0`` (i.e. when DT supplies an
+``interrupts = <...>``  property on the regulator node), so adding
+``SMBALERT#`` wiring is a zero-driver-change uplift on a future
+board rev.
+
+MPS post-mortem (PROTECTION_LAST)
+---------------------------------
+
+The MPQ8646 silicon keeps a single 16-bit NVM-backed record of the
+last protection event in ``PROTECTION_LAST`` (0xFB). It survives
+chip power/reset. The following debugfs entries expose it:
+
+::
+
+  /sys/kernel/debug/i2c/i2c-<bus>/<bus>-<addr>/protection_last     (RO)
+  /sys/kernel/debug/i2c/i2c-<bus>/<bus>-<addr>/status_decoded      (RO)
+
+``protection_last`` decodes the 16-bit value based on the datasheet
+fault names (``INIT_FAULT``, ``NVM_CRC_ERROR``, ``NVM_FAULT``,
+``OC_PHASE_FAULT``, ``OTP_SELF_FAULT``, ``SWITCH_PRD_FAULT``,
+``VIN_OV_FAULT``, ``VOUT_OV_FAULT``, ``VOUT_UV_FAULT``,
+``OC_TOT_FAULT``, ``VIN_UVLO_FAULT``, ``DRMOS_OTP``).
+
+``status_decoded`` reads ``STATUS_WORD`` (0x79) and renders the
+16 bits with MPS-extension labels (bit12 = ``NVM_SUMMARY``,
+bit8 = ``WATCH_DOG``, bit0 = ``DRMOS_FAULT``) instead of the
+PMBus 1.3 spec generic names.
+
+NVMEM snapshot
+--------------
+
+When ``CONFIG_NVMEM=y`` is set, the chip's NVM-backed
+observability registers are exposed as a single 16-byte read-only
+``nvmem_device`` at ``/sys/bus/nvmem/devices/<i2c-name>/nvmem``.
+Layout (little-endian for 16-bit fields, zero-fill on per-entry read
+failure and for the reserved tail):
+
+::
+
+  offset 0..1   PROTECTION_LAST  (0xFB)  word
+  offset 2..3   MFR_RETRY_TIMES  (0xF4)  word
+  offset 4..5   MFR_CONFIG_ID    (0xC0)  word
+  offset 6..7   MFR_VBOOT_CFG    (0xFC)  word
+  offset 8      MFR_SILICON_REV  (0xC3)  byte
+  offset 9..15  reserved (zero)
+
+Suitable for single-``cat`` post-mortem capture by a fleet daemon.
+
+Diagnostics and introspection
+-----------------------------
+
+When ``CONFIG_DEBUG_FS=y`` is set, the driver adds a read-only
+decode surface to the client's pmbus debugfs directory,
+``/sys/kernel/debug/i2c/i2c-<bus>/<bus>-<addr>/``.
+The driver intentionally exposes no raw register poke/peek debugfs;
+use i2c-dev (``i2cget``/``i2cset``/``i2ctransfer``) for that.
+
+Identity / observability (read-only):
+
+==========================  ==================  =================================
+File                        PMBus / MFR cmd     Description
+==========================  ==================  =================================
+``mfr_config_id``           0xC0 (word)         board / SKU NVM identifier
+``mfr_config_code_rev``     0xC1 (word)         NVM image revision (PART_RECOG +
+                                                config code rev fields)
+``mfr_silicon_rev``         0xC3 (byte)         die revision
+``mfr_retry_times``         0xF4 (word)         per-fault-class recovery-mode
+                                                configuration (NOT a retry
+                                                count). Four 4-bit fields
+                                                [15:12]=OTP, [11:8]=VOUT_OV,
+                                                [7:4]=VOUT_UV, [3:0]=OCP.
+                                                Each field: 0x0=latch-off,
+                                                0x1..0xE=retry N times then
+                                                latch off, 0xF=hiccup
+                                                (retry indefinitely). The
+                                                chip exposes no in-NVM
+                                                retry-event counter; track
+                                                transitions externally if
+                                                needed (poll
+                                                ``protection_last`` or watch
+                                                ``inX_alarm`` / ``temp1_alarm``
+                                                ``poll(POLLPRI)`` wakes).
+``mfr_vboot_cfg``           0xFC (word)         ADDR/VBOOT latched at POR
+==========================  ==================  =================================
+
+Timing / UVLO knobs (read-only):
+
+==========================  ==================  =================================
+File                        PMBus cmd           Description
+==========================  ==================  =================================
+``vin_on``                  0x35 (word)         UVLO turn-on threshold
+``vin_off``                 0x36 (word)         UVLO turn-off threshold
+``ton_delay``               0x60 (word)         soft-start delay
+``ton_rise``                0x61 (word)         soft-start ramp time
+``toff_delay``              0x64 (word)         soft-stop delay
+``toff_fall``               0x65 (word)         soft-stop ramp time
+==========================  ==================  =================================
+
+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.
+
+Devicetree
+----------
+
+The driver uses ``compatible = "mps,mpq8646"``. There are some few optional
+properties:
+
+- ``mps,vout-fb-divider-ratio-permille`` : it writes ``VOUT_SCALE_LOOP``
+  (0x29) at probe to compensate for an external resistor divider in
+  the VOUT feedback path. The valid range is 11-bit.
+- ``interrupts = <...>`` : if the board routes the chip's
+  ``SMBALERT#`` pin to a SoC GPIO, declaring it here lights up
+  ``pmbus_core::pmbus_irq_setup`` and disables the in-driver
+  alarm-poll fallback
diff --git a/MAINTAINERS b/MAINTAINERS
index 1ab8736850ea..88dc83d98896 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -18328,6 +18328,13 @@ S:	Maintained
 F:	Documentation/hwmon/mp9945.rst
 F:	drivers/hwmon/pmbus/mp9945.c
 
+MPS MPQ8646 PMBUS DRIVER
+M:	Vincent Jardin <vjardin@free.fr>
+L:	linux-hwmon@vger.kernel.org
+S:	Maintained
+F:	Documentation/hwmon/mpq8646.rst
+F:	drivers/hwmon/pmbus/mpq8646.c
+
 MR800 AVERMEDIA USB FM RADIO DRIVER
 M:	Alexey Klimov <alexey.klimov@linaro.org>
 L:	linux-media@vger.kernel.org
diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
index c8cda160b5f8..9f44e76b0b64 100644
--- a/drivers/hwmon/pmbus/Kconfig
+++ b/drivers/hwmon/pmbus/Kconfig
@@ -616,6 +616,17 @@ config SENSORS_MPQ8785
 	  This driver can also be built as a module. If so, the module will
 	  be called mpq8785.
 
+config SENSORS_MPQ8646
+	tristate "MPS MPQ8646"
+	depends on REGULATOR || !REGULATOR
+	depends on NVMEM || !NVMEM
+	help
+	  If you say yes here you get hardware monitoring support for the
+	  Monolithic Power Systems MPQ8646.
+
+	  This driver can also be built as a module. If so, the module
+	  will be called mpq8646.
+
 config SENSORS_PIM4328
 	tristate "Flex PIM4328 and compatibles"
 	help
diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile
index ffc05f493213..6f8fda966600 100644
--- a/drivers/hwmon/pmbus/Makefile
+++ b/drivers/hwmon/pmbus/Makefile
@@ -60,6 +60,7 @@ obj-$(CONFIG_SENSORS_MP9941)	+= mp9941.o
 obj-$(CONFIG_SENSORS_MP9945)	+= mp9945.o
 obj-$(CONFIG_SENSORS_MPQ7932)	+= mpq7932.o
 obj-$(CONFIG_SENSORS_MPQ8785)	+= mpq8785.o
+obj-$(CONFIG_SENSORS_MPQ8646)	+= mpq8646.o
 obj-$(CONFIG_SENSORS_PLI1209BC)	+= pli1209bc.o
 obj-$(CONFIG_SENSORS_PM6764TR)	+= pm6764tr.o
 obj-$(CONFIG_SENSORS_PXE1610)	+= pxe1610.o
diff --git a/drivers/hwmon/pmbus/mpq8646.c b/drivers/hwmon/pmbus/mpq8646.c
new file mode 100644
index 000000000000..1fd41d89e5be
--- /dev/null
+++ b/drivers/hwmon/pmbus/mpq8646.c
@@ -0,0 +1,685 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Driver for MPS MPQ8646 step-down converter.
+ *
+ * Copyright (c) 2026 Free Mobile - Vincent Jardin <vjardin@free.fr>
+ */
+
+#include <linux/bitops.h>
+#include <linux/debugfs.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/nvmem-provider.h>
+#include <linux/of_device.h>
+#include <linux/pmbus.h>
+#include <linux/property.h>
+#include <linux/regulator/driver.h>
+#include <linux/seq_file.h>
+#include <linux/workqueue.h>
+#include "pmbus.h"
+
+/* Default cadence for the in-driver alarm-poll fallback (ms) */
+#define MPQ8646_ALARM_POLL_MS_DEFAULT	1000
+
+/* MPS vendor-extended command codes (NOT in PMBus 1.3 Part II) */
+#define MPS_PROTECTION_LAST		0xFB
+
+/* PMBus 1.3 timing / UVLO command codes */
+#define PMBUS_VIN_ON			0x35
+#define PMBUS_VIN_OFF			0x36
+#define PMBUS_TON_DELAY			0x60
+#define PMBUS_TON_RISE			0x61
+#define PMBUS_TOFF_DELAY		0x64
+#define PMBUS_TOFF_FALL			0x65
+
+/* MPS vendor-extended observability / identity registers */
+#define MPS_MFR_CONFIG_ID		0xC0
+#define MPS_MFR_CONFIG_CODE_REV		0xC1
+#define MPS_MFR_SILICON_REV		0xC3
+#define MPS_MFR_RETRY_TIMES		0xF4
+#define MPS_MFR_VBOOT_CFG		0xFC
+
+#define MPQ8646_DEBUG(client, fmt, ...) \
+	dev_dbg(&(client)->dev, fmt, ##__VA_ARGS__)
+
+/*
+ * Maximum legal value for VOUT_SCALE_LOOP (0x29) on the MPQ8646 silicon
+ * The chip uses an 11-bit VOUT feedback-divider scale register.
+ */
+#define MPQ8646_VOUT_SCALE_LOOP_MAX	GENMASK(10, 0)
+
+/* Forward declaration */
+struct mpq8646_dbg_reg_ctx;
+
+/* Per-instance state */
+struct mpq8646_priv {
+	struct pmbus_driver_info info;	/* must be first, container_of target */
+	struct i2c_client	*client;
+
+	/* Serialises CLEAR_LAST_FAULT sequences and PROTECTION_LAST reads */
+	struct mutex		mps_lock;
+
+	/* Set alarm_poll_interval_ms = 0 to disable. */
+	struct delayed_work	alarm_poll_work;
+	u32			alarm_poll_interval_ms;
+
+#ifdef CONFIG_DEBUG_FS
+	/* the only debugfs file that can re-arm the poll worker */
+	struct dentry		*dbg_poll;
+	struct mpq8646_dbg_reg_ctx	*dbg_reg_ctx;
+#endif
+};
+
+static inline struct mpq8646_priv *mpq8646_priv_from_client(struct i2c_client *client)
+{
+	const struct pmbus_driver_info *info = pmbus_get_driver_info(client);
+
+	return container_of(info, struct mpq8646_priv, info);
+}
+
+/*
+ * VID-mode m/b/R coefficients, same values as the mpq8785 driver for
+ * the MPS VID encoding. Per the PMBus Direct formula used by
+ * pmbus_core, X = (Y * 10^-R - b) / m, so m=64 / b=0 / R=1 yields
+ * 1.5625 mV per LSB.
+ */
+#define MPQ8646_VID_M			64
+#define MPQ8646_VID_B			0
+#define MPQ8646_VID_R			1
+
+static int mpq8646_identify(struct i2c_client *client,
+			    struct pmbus_driver_info *info)
+{
+	int vout_mode;
+
+	vout_mode = pmbus_read_byte_data(client, 0, PMBUS_VOUT_MODE);
+	if (vout_mode < 0)
+		return vout_mode;
+
+	switch (vout_mode & PB_VOUT_MODE_MODE_MASK) {
+	case PB_VOUT_MODE_LINEAR:
+		info->format[PSC_VOLTAGE_OUT] = linear;
+		break;
+	case PB_VOUT_MODE_VID:
+	case PB_VOUT_MODE_DIRECT:
+		info->format[PSC_VOLTAGE_OUT] = direct;
+		info->m[PSC_VOLTAGE_OUT] = MPQ8646_VID_M;
+		info->b[PSC_VOLTAGE_OUT] = MPQ8646_VID_B;
+		info->R[PSC_VOLTAGE_OUT] = MPQ8646_VID_R;
+		break;
+	default:
+		return -ENODEV;
+	}
+
+	return 0;
+};
+
+static int mpq8646_read_byte_data(struct i2c_client *client, int page, int reg)
+{
+	int ret;
+
+	MPQ8646_DEBUG(client, "read_byte_data page=%d reg=0x%02x\n", page, reg);
+
+	switch (reg) {
+	case PMBUS_VOUT_MODE:
+		ret = pmbus_read_byte_data(client, page, reg);
+		MPQ8646_DEBUG(client, "  VOUT_MODE raw=0x%02x ret=%d\n", ret, ret);
+		if (ret < 0)
+			return ret;
+
+		if ((ret & PB_VOUT_MODE_MODE_MASK) == PB_VOUT_MODE_VID)
+			return PB_VOUT_MODE_DIRECT;
+
+		return ret;
+	case PMBUS_STATUS_BYTE:
+	case PMBUS_STATUS_CML:
+	case PMBUS_STATUS_OTHER:
+	case PMBUS_STATUS_MFR_SPECIFIC:
+	case PMBUS_STATUS_FAN_12:
+	case PMBUS_STATUS_FAN_34:
+		return -ENXIO;
+	case PMBUS_MFR_LOCATION:
+	case PMBUS_MFR_DATE:
+	case PMBUS_MFR_SERIAL:
+	case PMBUS_IC_DEVICE_ID:
+	case PMBUS_IC_DEVICE_REV:
+		MPQ8646_DEBUG(client, "  unsupported mfr-info reg 0x%02x -> ENXIO\n", reg);
+		return -ENXIO;
+	default:
+		return -ENODATA;
+	}
+}
+
+/*
+ * Reference: ltc2978.c::ltc2978_write_word_data which uses the
+ * same virtual-register channel for its real chip-side peak reset.
+ */
+static int mpq8646_write_word_data(struct i2c_client *client, int page,
+				   int reg, u16 word)
+{
+	struct mpq8646_priv *priv = mpq8646_priv_from_client(client);
+	int rc;
+
+	switch (reg) {
+	case PMBUS_VIRT_RESET_VIN_HISTORY:
+	case PMBUS_VIRT_RESET_VOUT_HISTORY:
+	case PMBUS_VIRT_RESET_IOUT_HISTORY:
+	case PMBUS_VIRT_RESET_TEMP_HISTORY:
+		MPQ8646_DEBUG(client, "reset_history virt reg=0x%04x -> CLEAR_FAULTS\n",
+			      reg);
+		rc = i2c_smbus_write_byte(priv->client, PMBUS_CLEAR_FAULTS);
+		if (rc < 0)
+			MPQ8646_DEBUG(client, "  CLEAR_FAULTS rc=%d\n", rc);
+		return rc < 0 ? rc : 0;
+	default:
+		return -ENODATA;	/* let pmbus_core do the direct write */
+	}
+}
+
+static int mpq8646_read_word_data(struct i2c_client *client, int page,
+				  int phase, int reg)
+{
+	int rc;
+
+	MPQ8646_DEBUG(client, "read_word_data page=%d phase=%d reg=0x%02x\n",
+		      page, phase, reg);
+
+	switch (reg) {
+	case PMBUS_READ_VIN:
+	case PMBUS_READ_VOUT:
+	case PMBUS_READ_IOUT:
+	case PMBUS_READ_TEMPERATURE_1:
+	case PMBUS_STATUS_WORD:
+	case PMBUS_VOUT_OV_FAULT_LIMIT:
+	case PMBUS_VOUT_OV_WARN_LIMIT:
+	case PMBUS_VOUT_UV_WARN_LIMIT:
+	case PMBUS_VOUT_UV_FAULT_LIMIT:
+	case PMBUS_IOUT_OC_FAULT_LIMIT:
+	case PMBUS_IOUT_OC_WARN_LIMIT:
+	case PMBUS_OT_FAULT_LIMIT:
+	case PMBUS_OT_WARN_LIMIT:
+	case PMBUS_VIN_OV_FAULT_LIMIT:
+	case PMBUS_VIN_OV_WARN_LIMIT:
+	case PMBUS_VIN_UV_WARN_LIMIT:
+	case PMBUS_VIN_UV_FAULT_LIMIT:
+	case PMBUS_MFR_VIN_MAX:
+	case PMBUS_MFR_VOUT_MAX:
+	case PMBUS_MFR_IOUT_MAX:
+	case PMBUS_MFR_MAX_TEMP_1:
+		break;
+	case PMBUS_VIRT_RESET_VIN_HISTORY:
+	case PMBUS_VIRT_RESET_VOUT_HISTORY:
+	case PMBUS_VIRT_RESET_IOUT_HISTORY:
+	case PMBUS_VIRT_RESET_TEMP_HISTORY:
+		return 0;
+	default:
+		return -ENODATA;
+	}
+
+	rc = i2c_smbus_read_word_data(client, reg);
+
+	MPQ8646_DEBUG(client, "  reg=0x%02x rc=%d\n", reg, rc);
+	return rc;
+}
+
+static struct pmbus_driver_info mpq8646_info = {
+	.pages = 1,
+	.format[PSC_VOLTAGE_IN] = direct,
+	.format[PSC_CURRENT_OUT] = direct,
+	.format[PSC_TEMPERATURE] = direct,
+	.m[PSC_VOLTAGE_IN] = 4,
+	.b[PSC_VOLTAGE_IN] = 0,
+	.R[PSC_VOLTAGE_IN] = 1,
+	.m[PSC_CURRENT_OUT] = 16,
+	.b[PSC_CURRENT_OUT] = 0,
+	.R[PSC_CURRENT_OUT] = 0,
+	.m[PSC_TEMPERATURE] = 1,
+	.b[PSC_TEMPERATURE] = 0,
+	.R[PSC_TEMPERATURE] = 0,
+	.func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_VOUT |
+		   PMBUS_HAVE_IOUT | PMBUS_HAVE_TEMP |
+		   PMBUS_HAVE_STATUS_INPUT | PMBUS_HAVE_STATUS_VOUT |
+		   PMBUS_HAVE_STATUS_IOUT | PMBUS_HAVE_STATUS_TEMP,
+};
+
+#if IS_ENABLED(CONFIG_REGULATOR)
+static const struct regulator_desc mpq8646_reg_desc[] = {
+	PMBUS_REGULATOR("vout", 0),
+};
+#endif /* CONFIG_REGULATOR */
+
+#if IS_ENABLED(CONFIG_NVMEM)
+/*
+ * Expose using
+ *   /sys/bus/nvmem/devices/<i2c-name>/nvmem
+ *
+ * Layout (16 bytes):
+ *   offset 0..1   PROTECTION_LAST (0xFB)    word, LE     -- NVM
+ *   offset 2..3   MFR_RETRY_TIMES (0xF4)    word, LE     -- NVM
+ *   offset 4..5   MFR_CONFIG_ID   (0xC0)    word, LE     -- NVM
+ *   offset 6..7   MFR_VBOOT_CFG   (0xFC)    word, LE     -- NVM
+ *   offset 8      MFR_SILICON_REV (0xC3)    byte         -- NVM
+ *   offset 9..15  reserved (zero-fill, leaves room for additions)
+ */
+#define MPQ8646_NVMEM_SIZE	16
+
+struct mpq8646_nvmem_entry {
+	unsigned int	off;
+	u8		reg;
+	bool		is_word;
+};
+
+static const struct mpq8646_nvmem_entry mpq8646_nvmem_map[] = {
+	{ 0, MPS_PROTECTION_LAST, true  },
+	{ 2, MPS_MFR_RETRY_TIMES, true  },
+	{ 4, MPS_MFR_CONFIG_ID,   true  },
+	{ 6, MPS_MFR_VBOOT_CFG,   true  },
+	{ 8, MPS_MFR_SILICON_REV, false },
+};
+
+static int mpq8646_nvmem_read(void *data, unsigned int offset, void *val,
+			      size_t bytes)
+{
+	struct mpq8646_priv *priv = data;
+	u8 *out = val;
+	size_t i;
+
+	if (offset >= MPQ8646_NVMEM_SIZE)
+		return -EINVAL;
+	if (offset + bytes > MPQ8646_NVMEM_SIZE)
+		bytes = MPQ8646_NVMEM_SIZE - offset;
+
+	memset(out, 0, bytes);
+
+	guard(mutex)(&priv->mps_lock);
+	for (i = 0; i < ARRAY_SIZE(mpq8646_nvmem_map); i++) {
+		const struct mpq8646_nvmem_entry *e = &mpq8646_nvmem_map[i];
+		unsigned int e_start = e->off;
+		unsigned int e_end = e_start + (e->is_word ? 2 : 1);
+		u8 raw[2];
+		int rc;
+		unsigned int j;
+
+		if (e_end <= offset || e_start >= offset + bytes)
+			continue;	/* outside requested slice */
+
+		if (e->is_word)
+			rc = i2c_smbus_read_word_data(priv->client, e->reg);
+		else
+			rc = i2c_smbus_read_byte_data(priv->client, e->reg);
+		if (rc < 0)
+			continue;	/* leave the zero-fill in place */
+
+		raw[0] = rc & 0xff;
+		raw[1] = (rc >> 8) & 0xff;
+
+		for (j = 0; j < e_end - e_start; j++) {
+			unsigned int abs = e_start + j;
+
+			if (abs >= offset && abs < offset + bytes)
+				out[abs - offset] = raw[j];
+		}
+	}
+	return 0;
+}
+#endif /* CONFIG_NVMEM */
+
+static const struct i2c_device_id mpq8646_id[] = {
+	{ "mpq8646", 0 },
+	{ },
+};
+MODULE_DEVICE_TABLE(i2c, mpq8646_id);
+
+static const struct of_device_id __maybe_unused mpq8646_of_match[] = {
+	{ .compatible = "mps,mpq8646" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, mpq8646_of_match);
+
+static struct pmbus_platform_data mpq8646_no_pec_pdata = {
+	.flags = PMBUS_NO_CAPABILITY,
+};
+
+#ifdef CONFIG_DEBUG_FS
+/*
+ * Read-only decode cases in the client's pmbus debugfs directory:
+ * the MPS-specific STATUS_WORD and PROTECTION_LAST bit decode plus the
+ * identity/timing registers.
+ */
+
+struct mpq_status_bit {
+	u16		mask;
+	const char	*name;
+};
+
+static const struct mpq_status_bit mpq8646_status_word_bits[] = {
+	{ PB_STATUS_VOUT,         "VOUT" },
+	{ PB_STATUS_IOUT_POUT,    "IOUT_POUT" },
+	{ PB_STATUS_INPUT,        "INPUT" },
+	{ PB_STATUS_WORD_MFR,     "NVM_SUMMARY" },
+	{ PB_STATUS_POWER_GOOD_N, "POWER_GOOD#" },
+	{ PB_STATUS_FANS,         "FANS" },
+	{ PB_STATUS_OTHER,        "OTHER" },
+	{ PB_STATUS_UNKNOWN,      "WATCH_DOG" },
+	{ PB_STATUS_BUSY,         "BUSY" },
+	{ PB_STATUS_OFF,          "OFF" },
+	{ PB_STATUS_VOUT_OV,      "VOUT_OV_FAULT" },
+	{ PB_STATUS_IOUT_OC,      "IOUT_OC_FAULT" },
+	{ PB_STATUS_VIN_UV,       "VIN_UV_FAULT" },
+	{ PB_STATUS_TEMPERATURE,  "TEMP" },
+	{ PB_STATUS_CML,          "CML" },
+	{ PB_STATUS_NONE_ABOVE,   "DRMOS_FAULT" },
+	{ /* sentinel */ }
+};
+
+/* PROTECTION_LAST (0xFB) bit names, it survives chip POR */
+static const struct mpq_status_bit mpq8646_protection_last_bits[] = {
+	{ BIT(15), "INIT_FAULT" },
+	{ BIT(14), "NVM_CRC_ERROR" },
+	{ BIT(13), "NVM_FAULT" },
+	{ BIT(12), "OC_PHASE_FAULT" },
+	{ BIT(11), "OTP_SELF_FAULT" },
+	{ BIT(9),  "SWITCH_PRD_FAULT" },
+	{ BIT(8),  "VIN_OV_FAULT" },
+	{ BIT(7),  "VOUT_OV_FAULT" },
+	{ BIT(6),  "VOUT_UV_FAULT" },
+	{ BIT(5),  "OC_TOT_FAULT" },
+	{ BIT(4),  "VIN_UVLO_FAULT" },
+	{ BIT(3),  "DRMOS_OTP" },
+	{ /* sentinel */ }
+};
+
+static void mpq8646_print_bits(struct seq_file *s, u16 v,
+			       const struct mpq_status_bit *tab)
+{
+	const struct mpq_status_bit *t;
+	bool first = true;
+
+	seq_printf(s, "0x%04x", v);
+	if (!v) {
+		seq_puts(s, " [clean]\n");
+		return;
+	}
+	seq_puts(s, " [");
+	for (t = tab; t->mask; t++) {
+		if (v & t->mask) {
+			if (!first)
+				seq_putc(s, ' ');
+			seq_puts(s, t->name);
+			first = false;
+		}
+	}
+	seq_puts(s, "]\n");
+}
+
+static int mpq8646_dbg_status_decoded_show(struct seq_file *s, void *unused)
+{
+	struct mpq8646_priv *priv = s->private;
+	int rc;
+
+	rc = i2c_smbus_read_word_data(priv->client, PMBUS_STATUS_WORD);
+	if (rc < 0) {
+		seq_printf(s, "ERROR: STATUS_WORD read failed (%d)\n", rc);
+		return 0;
+	}
+	seq_puts(s, "STATUS_WORD: ");
+	mpq8646_print_bits(s, (u16)rc, mpq8646_status_word_bits);
+	seq_puts(s, "(MPS extensions: bit12=NVM_SUMMARY, bit8=WATCH_DOG, bit0=DRMOS_FAULT)\n");
+	return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(mpq8646_dbg_status_decoded);
+
+static int mpq8646_dbg_protection_last_show(struct seq_file *s, void *unused)
+{
+	struct mpq8646_priv *priv = s->private;
+	int rc;
+
+	scoped_guard(mutex, &priv->mps_lock)
+		rc = i2c_smbus_read_word_data(priv->client, MPS_PROTECTION_LAST);
+
+	if (rc < 0) {
+		seq_printf(s, "ERROR: PROTECTION_LAST read failed (%d)\n", rc);
+		return 0;
+	}
+	seq_puts(s, "PROTECTION_LAST: ");
+	mpq8646_print_bits(s, (u16)rc, mpq8646_protection_last_bits);
+	seq_puts(s, "(NVM-backed, survives chip POR)\n");
+	return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(mpq8646_dbg_protection_last);
+
+struct mpq8646_dbg_reg {
+	u8		reg;
+	bool		is_word;
+	const char	*name;
+};
+
+static const struct mpq8646_dbg_reg mpq8646_dbg_regs[] = {
+	/* MPS vendor extensions (read-only identity / observability) */
+	{ MPS_MFR_CONFIG_ID,        true,  "mfr_config_id" },
+	{ MPS_MFR_CONFIG_CODE_REV,  true,  "mfr_config_code_rev" },
+	{ MPS_MFR_SILICON_REV,      false, "mfr_silicon_rev" },
+	{ MPS_MFR_RETRY_TIMES,      true,  "mfr_retry_times" },
+	{ MPS_MFR_VBOOT_CFG,        true,  "mfr_vboot_cfg" },
+	/* PMBus 1.3 standard timing / UVLO (read-only introspection) */
+	{ PMBUS_VIN_ON,             true,  "vin_on" },
+	{ PMBUS_VIN_OFF,            true,  "vin_off" },
+	{ PMBUS_TON_DELAY,          true,  "ton_delay" },
+	{ PMBUS_TON_RISE,           true,  "ton_rise" },
+	{ PMBUS_TOFF_DELAY,         true,  "toff_delay" },
+	{ PMBUS_TOFF_FALL,          true,  "toff_fall" },
+};
+
+struct mpq8646_dbg_reg_ctx {
+	struct mpq8646_priv		*priv;
+	const struct mpq8646_dbg_reg	*desc;
+};
+
+static int mpq8646_dbg_reg_show(struct seq_file *s, void *unused)
+{
+	struct mpq8646_dbg_reg_ctx *ctx = s->private;
+	int rc;
+
+	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) {
+		seq_printf(s, "ERROR: reg 0x%02x (%s) read failed (%d)\n",
+			   ctx->desc->reg, ctx->desc->name, rc);
+		return 0;
+	}
+	seq_printf(s, "0x%0*x\n", ctx->desc->is_word ? 4 : 2, rc);
+	return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(mpq8646_dbg_reg);
+
+static int mpq8646_dbg_poll_interval_get(void *data, u64 *val)
+{
+	struct mpq8646_priv *priv = data;
+
+	*val = priv->alarm_poll_interval_ms;
+	return 0;
+}
+
+static int mpq8646_dbg_poll_interval_set(void *data, u64 val)
+{
+	struct mpq8646_priv *priv = data;
+	bool was_off = !priv->alarm_poll_interval_ms;
+
+	priv->alarm_poll_interval_ms = (u32)val;
+
+	if (val && was_off && !priv->client->irq)
+		schedule_delayed_work(&priv->alarm_poll_work,
+				      msecs_to_jiffies((u32)val));
+	return 0;
+}
+DEFINE_DEBUGFS_ATTRIBUTE(mpq8646_dbg_poll_interval_fops,
+			 mpq8646_dbg_poll_interval_get,
+			 mpq8646_dbg_poll_interval_set, "%llu\n");
+
+static void mpq8646_debugfs_register(struct mpq8646_priv *priv)
+{
+	struct dentry *root;
+	size_t i;
+
+	root = pmbus_get_debugfs_dir(priv->client);
+	if (!root)
+		return;
+
+	/* MPS extensions: status decode + NVM-backed PROTECTION_LAST */
+	debugfs_create_file("status_decoded", 0400, root, priv,
+			    &mpq8646_dbg_status_decoded_fops);
+	debugfs_create_file("protection_last", 0400, root, priv,
+			    &mpq8646_dbg_protection_last_fops);
+	priv->dbg_poll =
+		debugfs_create_file_unsafe("alarm_poll_interval_ms", 0600,
+					   root, priv,
+					   &mpq8646_dbg_poll_interval_fops);
+
+	priv->dbg_reg_ctx = devm_kcalloc(&priv->client->dev,
+					 ARRAY_SIZE(mpq8646_dbg_regs),
+					 sizeof(*priv->dbg_reg_ctx),
+					 GFP_KERNEL);
+	if (!priv->dbg_reg_ctx)
+		return;
+	for (i = 0; i < ARRAY_SIZE(mpq8646_dbg_regs); i++) {
+		priv->dbg_reg_ctx[i].priv = priv;
+		priv->dbg_reg_ctx[i].desc = &mpq8646_dbg_regs[i];
+		debugfs_create_file(mpq8646_dbg_regs[i].name, 0400,
+				    root, &priv->dbg_reg_ctx[i],
+				    &mpq8646_dbg_reg_fops);
+	}
+}
+
+static void mpq8646_debugfs_unregister(struct mpq8646_priv *priv)
+{
+	debugfs_remove(priv->dbg_poll);
+}
+#else
+static inline void mpq8646_debugfs_register(struct mpq8646_priv *priv) {}
+static inline void mpq8646_debugfs_unregister(struct mpq8646_priv *priv) {}
+#endif /* CONFIG_DEBUG_FS */
+
+static void mpq8646_alarm_poll_work(struct work_struct *work)
+{
+	struct mpq8646_priv *priv = container_of(to_delayed_work(work),
+						 struct mpq8646_priv,
+						 alarm_poll_work);
+
+	if (priv->client->irq)
+		return;	/* SMBALERT# wired; polling not needed */
+
+	if (!priv->alarm_poll_interval_ms)
+		return;	/* polling disabled; don't re-arm */
+
+	pmbus_check_and_notify_faults(priv->client);
+
+	schedule_delayed_work(&priv->alarm_poll_work,
+			      msecs_to_jiffies(priv->alarm_poll_interval_ms));
+}
+
+static int mpq8646_probe(struct i2c_client *client)
+{
+	struct device *dev = &client->dev;
+	struct pmbus_driver_info *info;
+	struct mpq8646_priv *priv;
+	u32 voltage_scale;
+	int ret;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+	priv->client = client;
+	mutex_init(&priv->mps_lock);
+	memcpy(&priv->info, &mpq8646_info, sizeof(priv->info));
+	info = &priv->info;
+
+	info->identify = mpq8646_identify;
+	info->read_byte_data = mpq8646_read_byte_data;
+	info->read_word_data = mpq8646_read_word_data;
+	info->write_word_data = mpq8646_write_word_data;
+	dev->platform_data = &mpq8646_no_pec_pdata;
+
+#if IS_ENABLED(CONFIG_REGULATOR)
+	info->reg_desc = mpq8646_reg_desc;
+	info->num_regulators = ARRAY_SIZE(mpq8646_reg_desc);
+#endif
+
+	INIT_DELAYED_WORK(&priv->alarm_poll_work, mpq8646_alarm_poll_work);
+	priv->alarm_poll_interval_ms = MPQ8646_ALARM_POLL_MS_DEFAULT;
+
+	if (!device_property_read_u32(dev, "mps,vout-fb-divider-ratio-permille",
+				      &voltage_scale)) {
+		if (voltage_scale > MPQ8646_VOUT_SCALE_LOOP_MAX)
+			return -EINVAL;
+
+		ret = i2c_smbus_write_word_data(client, PMBUS_VOUT_SCALE_LOOP,
+						voltage_scale);
+		if (ret)
+			return ret;
+	}
+
+	ret = pmbus_do_probe(client, info);
+	if (ret)
+		return ret;
+
+	mpq8646_debugfs_register(priv);
+
+	if (!client->irq)
+		schedule_delayed_work(&priv->alarm_poll_work,
+				      msecs_to_jiffies(priv->alarm_poll_interval_ms));
+
+#if IS_ENABLED(CONFIG_NVMEM)
+	{
+		struct nvmem_config cfg = {
+			.dev		= &client->dev,
+			.name		= dev_name(&client->dev),
+			.owner		= THIS_MODULE,
+			.read_only	= true,
+			.root_only	= true,
+			.word_size	= 1,
+			.stride		= 1,
+			.size		= MPQ8646_NVMEM_SIZE,
+			.reg_read	= mpq8646_nvmem_read,
+			.priv		= priv,
+		};
+		struct nvmem_device *nv = devm_nvmem_register(&client->dev, &cfg);
+
+		if (IS_ERR(nv))
+			dev_warn(&client->dev,
+				 "nvmem snapshot register failed (%pe)\n",
+				 nv);
+	}
+#endif
+	return 0;
+};
+
+static void mpq8646_remove(struct i2c_client *client)
+{
+	struct mpq8646_priv *priv = mpq8646_priv_from_client(client);
+
+	mpq8646_debugfs_unregister(priv);
+	cancel_delayed_work_sync(&priv->alarm_poll_work);
+}
+
+static struct i2c_driver mpq8646_driver = {
+	.driver = {
+		   .name = "mpq8646",
+		   .of_match_table = of_match_ptr(mpq8646_of_match),
+	},
+	.probe = mpq8646_probe,
+	.remove = mpq8646_remove,
+	.id_table = mpq8646_id,
+};
+
+module_i2c_driver(mpq8646_driver);
+
+MODULE_AUTHOR("Vincent Jardin <vjardin@free.fr>");
+MODULE_DESCRIPTION("PMBus driver for MPS MPQ8646 (extended observability)");
+MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS("PMBUS");

-- 
2.43.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH v6 3/4] hwmon: pmbus: add MPQ8646 driver
  2026-07-29 21:51   ` Vincent Jardin
  (?)
@ 2026-07-29 22:05   ` sashiko-bot
  2026-07-30  4:12     ` Guenter Roeck
  -1 siblings, 1 reply; 21+ messages in thread
From: sashiko-bot @ 2026-07-29 22:05 UTC (permalink / raw)
  To: Vincent Jardin; +Cc: conor+dt, robh, linux-hwmon, devicetree

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Missing named initializers in `struct i2c_device_id` array.
--

--- Patch [3]: [PATCH v6 3/4] hwmon: pmbus: add MPQ8646 driver ---
commit 63fabec4213f8ada1bf01d09967620e66fc9f459
Author: Vincent Jardin <vjardin@free.fr>

hwmon: pmbus: add MPQ8646 driver

This commit adds a new driver for the MPS MPQ8646 PMBus device. It provides
basic PMBus telemetry, alarm acknowledgement, STATUS_WORD extended bit
decoding, NVM-backed post-mortem data, and an in-driver alarm-poll fallback.

> diff --git a/drivers/hwmon/pmbus/mpq8646.c b/drivers/hwmon/pmbus/mpq8646.c
> new file mode 100644
> index 0000000000000..1fd41d89e5bef
> --- /dev/null
> +++ b/drivers/hwmon/pmbus/mpq8646.c
[ ... ]
> +static const struct i2c_device_id mpq8646_id[] = {
> +	{ "mpq8646", 0 },
> +	{ },
> +};

[Severity: Low]
This isn't a bug, but should this array use named initializers? The I2C
subsystem guidelines specify that initialized arrays of type struct
i2c_device_id must use named initializers, such as:
	{ .name = "mpq8646", .driver_data = 0 },

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729-mpq8646_v0-v6-0-98848151cdb4@free.fr?part=3

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v6 3/4] hwmon: pmbus: add MPQ8646 driver
  2026-07-29 22:05   ` sashiko-bot
@ 2026-07-30  4:12     ` Guenter Roeck
  0 siblings, 0 replies; 21+ messages in thread
From: Guenter Roeck @ 2026-07-30  4:12 UTC (permalink / raw)
  To: sashiko-reviews, Vincent Jardin; +Cc: conor+dt, robh, linux-hwmon, devicetree

On 7/29/26 15:05, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] Missing named initializers in `struct i2c_device_id` array.
> --
> 
> --- Patch [3]: [PATCH v6 3/4] hwmon: pmbus: add MPQ8646 driver ---
> commit 63fabec4213f8ada1bf01d09967620e66fc9f459
> Author: Vincent Jardin <vjardin@free.fr>
> 
> hwmon: pmbus: add MPQ8646 driver
> 
> This commit adds a new driver for the MPS MPQ8646 PMBus device. It provides
> basic PMBus telemetry, alarm acknowledgement, STATUS_WORD extended bit
> decoding, NVM-backed post-mortem data, and an in-driver alarm-poll fallback.
> 
>> diff --git a/drivers/hwmon/pmbus/mpq8646.c b/drivers/hwmon/pmbus/mpq8646.c
>> new file mode 100644
>> index 0000000000000..1fd41d89e5bef
>> --- /dev/null
>> +++ b/drivers/hwmon/pmbus/mpq8646.c
> [ ... ]
>> +static const struct i2c_device_id mpq8646_id[] = {
>> +	{ "mpq8646", 0 },
>> +	{ },
>> +};
> 
> [Severity: Low]
> This isn't a bug, but should this array use named initializers? The I2C
> subsystem guidelines specify that initialized arrays of type struct
> i2c_device_id must use named initializers, such as:
> 	{ .name = "mpq8646", .driver_data = 0 },
> 

Yes, this needs to be a named initializer.

Thanks,
Guenter


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v6 4/4] hwmon: pmbus: mpq8646: gate the writes
  2026-07-29 21:51 ` Vincent Jardin
@ 2026-07-29 21:51   ` Vincent Jardin
  -1 siblings, 0 replies; 21+ messages in thread
From: Vincent Jardin via B4 Relay @ 2026-07-29 21:51 UTC (permalink / raw)
  To: Guenter Roeck, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Jonathan Corbet, Shuah Khan, Charles Hsu
  Cc: Markus Elfring, linux-hwmon, linux-kernel, devicetree, linux-doc,
	Vincent Jardin

From: Vincent Jardin <vjardin@free.fr>

The write cases of debugfs entries are provisioning and bench
helpers. By designs the MPQ8646 powers the CPU cores rail,
so a wrong write can brown out the board or persist a bad
setpoint into finite-cycle NVM.

Signed-off-by: Vincent Jardin <vjardin@free.fr>
---
 Documentation/hwmon/mpq8646.rst |  37 ++++++
 drivers/hwmon/pmbus/Kconfig     |  17 +++
 drivers/hwmon/pmbus/mpq8646.c   | 269 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 323 insertions(+)

diff --git a/Documentation/hwmon/mpq8646.rst b/Documentation/hwmon/mpq8646.rst
index d3ede656ea15..6207cfd89f7f 100644
--- a/Documentation/hwmon/mpq8646.rst
+++ b/Documentation/hwmon/mpq8646.rst
@@ -246,6 +246,43 @@ 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. The first
+gated write logs a one-shot warning.
+
+===============================  ===============  =========================================================
+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..9c6d3de61a75 100644
--- a/drivers/hwmon/pmbus/Kconfig
+++ b/drivers/hwmon/pmbus/Kconfig
@@ -627,6 +627,23 @@ 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.
+
+	  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 1fd41d89e5be..0034cf591934 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__)
 
@@ -498,6 +531,240 @@ 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_warn_once(struct mpq8646_priv *priv)
+{
+	dev_warn_once(&priv->client->dev,
+		      "UNSAFE debugfs write to core-rail regulator; board integrity not guaranteed (CONFIG_SENSORS_MPQ8646_DEBUG_UNSAFE)\n");
+}
+
+static int mpq8646_dbg_clear_protection_last(void *data, u64 val)
+{
+	struct mpq8646_priv *priv = data;
+	int rc;
+
+	if (!val)
+		return 0;
+
+	mpq8646_unsafe_warn_once(priv);
+	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 0;
+}
+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, last_rc;
+	int wp_orig, cfg_orig;
+
+	if (!val)
+		return 0;
+
+	mpq8646_unsafe_warn_once(priv);
+	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 0;
+	}
+	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 0;
+	}
+
+	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 0;
+		}
+	}
+
+	rc = i2c_smbus_write_word_data(priv->client, MPS_MFR_CFG_EXT,
+				       (u16)cfg_orig | MPS_MFR_CFG_EXT_CLR_LAST_EN);
+	if (rc < 0) {
+		dev_warn(&priv->client->dev,
+			 "clear_protection_last_force: gate open failed (%d)\n",
+			 rc);
+		goto restore_wp;
+	}
+
+	last_rc = i2c_smbus_write_byte(priv->client, MPS_CLEAR_LAST_FAULT);
+	if (last_rc < 0)
+		dev_warn(&priv->client->dev,
+			 "clear_protection_last_force: CLEAR_LAST_FAULT failed (%d) even with gate open\n",
+			 last_rc);
+
+	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);
+
+restore_wp:
+	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);
+	}
+	return 0;
+}
+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;
+
+	mpq8646_unsafe_warn_once(priv);
+	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 0;
+}
+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;
+
+	mpq8646_unsafe_warn_once(priv);
+	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 0;
+}
+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;
+
+	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;
+
+	mpq8646_unsafe_warn_once(ctx->priv);
+	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;
+
+	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;
@@ -554,6 +821,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



^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v6 4/4] hwmon: pmbus: mpq8646: gate the writes
@ 2026-07-29 21:51   ` Vincent Jardin
  0 siblings, 0 replies; 21+ messages in thread
From: Vincent Jardin @ 2026-07-29 21:51 UTC (permalink / raw)
  To: Guenter Roeck, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Jonathan Corbet, Shuah Khan, Charles Hsu
  Cc: Markus Elfring, linux-hwmon, linux-kernel, devicetree, linux-doc,
	Vincent Jardin

The write cases of debugfs entries are provisioning and bench
helpers. By designs the MPQ8646 powers the CPU cores rail,
so a wrong write can brown out the board or persist a bad
setpoint into finite-cycle NVM.

Signed-off-by: Vincent Jardin <vjardin@free.fr>
---
 Documentation/hwmon/mpq8646.rst |  37 ++++++
 drivers/hwmon/pmbus/Kconfig     |  17 +++
 drivers/hwmon/pmbus/mpq8646.c   | 269 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 323 insertions(+)

diff --git a/Documentation/hwmon/mpq8646.rst b/Documentation/hwmon/mpq8646.rst
index d3ede656ea15..6207cfd89f7f 100644
--- a/Documentation/hwmon/mpq8646.rst
+++ b/Documentation/hwmon/mpq8646.rst
@@ -246,6 +246,43 @@ 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. The first
+gated write logs a one-shot warning.
+
+===============================  ===============  =========================================================
+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..9c6d3de61a75 100644
--- a/drivers/hwmon/pmbus/Kconfig
+++ b/drivers/hwmon/pmbus/Kconfig
@@ -627,6 +627,23 @@ 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.
+
+	  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 1fd41d89e5be..0034cf591934 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__)
 
@@ -498,6 +531,240 @@ 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_warn_once(struct mpq8646_priv *priv)
+{
+	dev_warn_once(&priv->client->dev,
+		      "UNSAFE debugfs write to core-rail regulator; board integrity not guaranteed (CONFIG_SENSORS_MPQ8646_DEBUG_UNSAFE)\n");
+}
+
+static int mpq8646_dbg_clear_protection_last(void *data, u64 val)
+{
+	struct mpq8646_priv *priv = data;
+	int rc;
+
+	if (!val)
+		return 0;
+
+	mpq8646_unsafe_warn_once(priv);
+	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 0;
+}
+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, last_rc;
+	int wp_orig, cfg_orig;
+
+	if (!val)
+		return 0;
+
+	mpq8646_unsafe_warn_once(priv);
+	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 0;
+	}
+	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 0;
+	}
+
+	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 0;
+		}
+	}
+
+	rc = i2c_smbus_write_word_data(priv->client, MPS_MFR_CFG_EXT,
+				       (u16)cfg_orig | MPS_MFR_CFG_EXT_CLR_LAST_EN);
+	if (rc < 0) {
+		dev_warn(&priv->client->dev,
+			 "clear_protection_last_force: gate open failed (%d)\n",
+			 rc);
+		goto restore_wp;
+	}
+
+	last_rc = i2c_smbus_write_byte(priv->client, MPS_CLEAR_LAST_FAULT);
+	if (last_rc < 0)
+		dev_warn(&priv->client->dev,
+			 "clear_protection_last_force: CLEAR_LAST_FAULT failed (%d) even with gate open\n",
+			 last_rc);
+
+	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);
+
+restore_wp:
+	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);
+	}
+	return 0;
+}
+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;
+
+	mpq8646_unsafe_warn_once(priv);
+	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 0;
+}
+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;
+
+	mpq8646_unsafe_warn_once(priv);
+	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 0;
+}
+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;
+
+	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;
+
+	mpq8646_unsafe_warn_once(ctx->priv);
+	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;
+
+	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;
@@ -554,6 +821,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


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH v6 4/4] hwmon: pmbus: mpq8646: gate the writes
  2026-07-29 21:51   ` Vincent Jardin
  (?)
@ 2026-07-29 22:07   ` sashiko-bot
  2026-07-30  4:14     ` Guenter Roeck
  -1 siblings, 1 reply; 21+ messages in thread
From: sashiko-bot @ 2026-07-29 22:07 UTC (permalink / raw)
  To: Vincent Jardin; +Cc: conor+dt, robh, devicetree, linux-hwmon

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Debugfs attributes perform I2C accesses without acquiring necessary locks, risking race conditions and bypassing hardware write protection.
- [Medium] Mixing goto-based error handling with guard() declarations in the same function.
- [Medium] Debugfs write functions mask negative error codes by returning 0 (success) on hardware failure.
--

--- Patch [4]: [PATCH v6 4/4] hwmon: pmbus: mpq8646: gate the writes ---
commit ec4599677b214a57f0796000d18d9f3b6e5d4fb5
Author: Vincent Jardin <vjardin@free.fr>

hwmon: pmbus: mpq8646: gate the writes

This commit adds unsafe debugfs provisioning and benchmarking helpers
to write specific registers. These operations are restricted due to the
risk of browning out the board or persisting bad setpoints.

> diff --git a/drivers/hwmon/pmbus/mpq8646.c b/drivers/hwmon/pmbus/mpq8646.c
> index 1fd41d89e5bef..0034cf5919345 100644
> --- a/drivers/hwmon/pmbus/mpq8646.c
> +++ b/drivers/hwmon/pmbus/mpq8646.c
[ ... ]
> +static int mpq8646_dbg_clear_protection_last(void *data, u64 val)
> +{
[ ... ]
> +	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 0;
> +}

[Severity: Medium]
Should this propagate the negative error code from rc instead of returning 0?
Returning 0 masks hardware faults and might lead userspace provisioning
scripts to assume the write succeeded.

[ ... ]
> +static int mpq8646_dbg_clear_protection_last_force(void *data, u64 val)
> +{
[ ... ]
> +	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 0;
> +	}

[Severity: Medium]
Like above, should this return the negative error code in wp_orig so the I2C
read failure is reported to the caller?

[ ... ]
> +	rc = i2c_smbus_write_word_data(priv->client, MPS_MFR_CFG_EXT,
> +				       (u16)cfg_orig | MPS_MFR_CFG_EXT_CLR_LAST_EN);
> +	if (rc < 0) {
> +		dev_warn(&priv->client->dev,
> +			 "clear_protection_last_force: gate open failed (%d)\n",
> +			 rc);
> +		goto restore_wp;
> +	}

[Severity: Medium]
Does this mix goto-based error handling with guard() declarations in the
same function?

The cleanup guidelines specify that functions should either use scope-based
cleanup exclusively or not at all, to avoid confusing ownership semantics.

[ ... ]
> +static int mpq8646_dbg_store_all(void *data, u64 val)
> +{
[ ... ]
> +	mpq8646_unsafe_warn_once(priv);
> +	scoped_guard(mutex, &priv->mps_lock)
> +		rc = i2c_smbus_write_byte(priv->client, PMBUS_STORE_USER_ALL);

[Severity: High]
Should this also acquire the global pmbus_lock? 

Issuing STORE_USER_ALL without the pmbus_lock might expose an unprotected
NVM-busy window to PMBus core background polling.

> +	if (rc < 0)
> +		dev_warn(&priv->client->dev,
> +			 "store_all: STORE_USER_ALL (0x15) write failed (%d)\n",
> +			 rc);
> +	return 0;
> +}

[Severity: Medium]
Does this need to return the negative error code in rc instead of 0 on
failure?

[ ... ]
> +static int mpq8646_dbg_restore_all(void *data, u64 val)
> +{
[ ... ]
> +	mpq8646_unsafe_warn_once(priv);
> +	scoped_guard(mutex, &priv->mps_lock)
> +		rc = i2c_smbus_write_byte(priv->client, PMBUS_RESTORE_USER_ALL);

[Severity: High]
As with mpq8646_dbg_store_all(), should this also hold pmbus_lock to
prevent NVM-busy window races?

> +	if (rc < 0)
> +		dev_warn(&priv->client->dev,
> +			 "restore_all: RESTORE_USER_ALL (0x16) write failed (%d)\n",
> +			 rc);
> +	return 0;
> +}

[Severity: Medium]
Should this return the negative error code in rc?

[ ... ]
> +static int mpq8646_dbg_reg_set(void *data, u64 val)
> +{
> +	struct mpq8646_dbg_reg_ctx *ctx = data;
> +	int rc;
> +
> +	mpq8646_unsafe_warn_once(ctx->priv);
> +	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);

[Severity: High]
Does this need to acquire pmbus_lock or mps_lock before performing the I2C
write?

If another thread concurrently calls mpq8646_dbg_clear_protection_last_force()
which temporarily lowers the PMBUS_WRITE_PROTECT register, could a direct
write here slip through and bypass the hardware write protection?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729-mpq8646_v0-v6-0-98848151cdb4@free.fr?part=4

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v6 4/4] hwmon: pmbus: mpq8646: gate the writes
  2026-07-29 22:07   ` sashiko-bot
@ 2026-07-30  4:14     ` Guenter Roeck
  0 siblings, 0 replies; 21+ messages in thread
From: Guenter Roeck @ 2026-07-30  4:14 UTC (permalink / raw)
  To: sashiko-reviews, Vincent Jardin; +Cc: conor+dt, robh, devicetree, linux-hwmon

On 7/29/26 15:07, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> - [High] Debugfs attributes perform I2C accesses without acquiring necessary locks, risking race conditions and bypassing hardware write protection.
> - [Medium] Mixing goto-based error handling with guard() declarations in the same function.
> - [Medium] Debugfs write functions mask negative error codes by returning 0 (success) on hardware failure.
> --
> 

Feedback seems valid. Please address.

Thanks,
Guenter

> --- Patch [4]: [PATCH v6 4/4] hwmon: pmbus: mpq8646: gate the writes ---
> commit ec4599677b214a57f0796000d18d9f3b6e5d4fb5
> Author: Vincent Jardin <vjardin@free.fr>
> 
> hwmon: pmbus: mpq8646: gate the writes
> 
> This commit adds unsafe debugfs provisioning and benchmarking helpers
> to write specific registers. These operations are restricted due to the
> risk of browning out the board or persisting bad setpoints.
> 
>> diff --git a/drivers/hwmon/pmbus/mpq8646.c b/drivers/hwmon/pmbus/mpq8646.c
>> index 1fd41d89e5bef..0034cf5919345 100644
>> --- a/drivers/hwmon/pmbus/mpq8646.c
>> +++ b/drivers/hwmon/pmbus/mpq8646.c
> [ ... ]
>> +static int mpq8646_dbg_clear_protection_last(void *data, u64 val)
>> +{
> [ ... ]
>> +	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 0;
>> +}
> 
> [Severity: Medium]
> Should this propagate the negative error code from rc instead of returning 0?
> Returning 0 masks hardware faults and might lead userspace provisioning
> scripts to assume the write succeeded.
> 
> [ ... ]
>> +static int mpq8646_dbg_clear_protection_last_force(void *data, u64 val)
>> +{
> [ ... ]
>> +	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 0;
>> +	}
> 
> [Severity: Medium]
> Like above, should this return the negative error code in wp_orig so the I2C
> read failure is reported to the caller?
> 
> [ ... ]
>> +	rc = i2c_smbus_write_word_data(priv->client, MPS_MFR_CFG_EXT,
>> +				       (u16)cfg_orig | MPS_MFR_CFG_EXT_CLR_LAST_EN);
>> +	if (rc < 0) {
>> +		dev_warn(&priv->client->dev,
>> +			 "clear_protection_last_force: gate open failed (%d)\n",
>> +			 rc);
>> +		goto restore_wp;
>> +	}
> 
> [Severity: Medium]
> Does this mix goto-based error handling with guard() declarations in the
> same function?
> 
> The cleanup guidelines specify that functions should either use scope-based
> cleanup exclusively or not at all, to avoid confusing ownership semantics.
> 
> [ ... ]
>> +static int mpq8646_dbg_store_all(void *data, u64 val)
>> +{
> [ ... ]
>> +	mpq8646_unsafe_warn_once(priv);
>> +	scoped_guard(mutex, &priv->mps_lock)
>> +		rc = i2c_smbus_write_byte(priv->client, PMBUS_STORE_USER_ALL);
> 
> [Severity: High]
> Should this also acquire the global pmbus_lock?
> 
> Issuing STORE_USER_ALL without the pmbus_lock might expose an unprotected
> NVM-busy window to PMBus core background polling.
> 
>> +	if (rc < 0)
>> +		dev_warn(&priv->client->dev,
>> +			 "store_all: STORE_USER_ALL (0x15) write failed (%d)\n",
>> +			 rc);
>> +	return 0;
>> +}
> 
> [Severity: Medium]
> Does this need to return the negative error code in rc instead of 0 on
> failure?
> 
> [ ... ]
>> +static int mpq8646_dbg_restore_all(void *data, u64 val)
>> +{
> [ ... ]
>> +	mpq8646_unsafe_warn_once(priv);
>> +	scoped_guard(mutex, &priv->mps_lock)
>> +		rc = i2c_smbus_write_byte(priv->client, PMBUS_RESTORE_USER_ALL);
> 
> [Severity: High]
> As with mpq8646_dbg_store_all(), should this also hold pmbus_lock to
> prevent NVM-busy window races?
> 
>> +	if (rc < 0)
>> +		dev_warn(&priv->client->dev,
>> +			 "restore_all: RESTORE_USER_ALL (0x16) write failed (%d)\n",
>> +			 rc);
>> +	return 0;
>> +}
> 
> [Severity: Medium]
> Should this return the negative error code in rc?
> 
> [ ... ]
>> +static int mpq8646_dbg_reg_set(void *data, u64 val)
>> +{
>> +	struct mpq8646_dbg_reg_ctx *ctx = data;
>> +	int rc;
>> +
>> +	mpq8646_unsafe_warn_once(ctx->priv);
>> +	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);
> 
> [Severity: High]
> Does this need to acquire pmbus_lock or mps_lock before performing the I2C
> write?
> 
> If another thread concurrently calls mpq8646_dbg_clear_protection_last_force()
> which temporarily lowers the PMBUS_WRITE_PROTECT register, could a direct
> write here slip through and bypass the hardware write protection?
> 


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v6 4/4] hwmon: pmbus: mpq8646: gate the writes
  2026-07-29 21:51   ` Vincent Jardin
  (?)
  (?)
@ 2026-07-30  4:26   ` Guenter Roeck
  -1 siblings, 0 replies; 21+ messages in thread
From: Guenter Roeck @ 2026-07-30  4:26 UTC (permalink / raw)
  To: vjardin, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Jonathan Corbet, Shuah Khan, Charles Hsu
  Cc: Markus Elfring, linux-hwmon, linux-kernel, devicetree, linux-doc

On 7/29/26 14:51, Vincent Jardin via B4 Relay wrote:
> From: Vincent Jardin <vjardin@free.fr>
> 
> The write cases of debugfs entries are provisioning and bench
> helpers. By designs the MPQ8646 powers the CPU cores rail,
> so a wrong write can brown out the board or persist a bad
> setpoint into finite-cycle NVM.
> 
> Signed-off-by: Vincent Jardin <vjardin@free.fr>
> ---
>   Documentation/hwmon/mpq8646.rst |  37 ++++++
>   drivers/hwmon/pmbus/Kconfig     |  17 +++
>   drivers/hwmon/pmbus/mpq8646.c   | 269 ++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 323 insertions(+)
> 
> diff --git a/Documentation/hwmon/mpq8646.rst b/Documentation/hwmon/mpq8646.rst
> index d3ede656ea15..6207cfd89f7f 100644
> --- a/Documentation/hwmon/mpq8646.rst
> +++ b/Documentation/hwmon/mpq8646.rst
> @@ -246,6 +246,43 @@ 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. The first
> +gated write logs a one-shot warning.
> +

This and the explanation in Kconfig need to be much stronger,
along the line of

"WARNING: Wrong register writes can and likely will physically damage
or destroy the chip and/or the board".

> +===============================  ===============  =========================================================
> +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..9c6d3de61a75 100644
> --- a/drivers/hwmon/pmbus/Kconfig
> +++ b/drivers/hwmon/pmbus/Kconfig
> @@ -627,6 +627,23 @@ 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.
> +
> +	  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 1fd41d89e5be..0034cf591934 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__)
>   
> @@ -498,6 +531,240 @@ 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_warn_once(struct mpq8646_priv *priv)
> +{
> +	dev_warn_once(&priv->client->dev,
> +		      "UNSAFE debugfs write to core-rail regulator; board integrity not guaranteed (CONFIG_SENSORS_MPQ8646_DEBUG_UNSAFE)\n");

This is also not strong enough. You insist adding this functionality to the driver.
Please inform the user that they are about to shoot themselves into the foot.
This is much worse than "board integrity not guaranteed".

CONFIG_SENSORS_MPQ8646_DEBUG_UNSAFE in the message is pretty pointless.

Also, the message should be shown when the driver is loaded, as multi-line
message, similar to the unhashed kernel memory address warning.

**********************************************************
**   NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE   **
**                                                      **
** This system shows unhashed kernel memory addresses   **
** via the console, logs, and other interfaces. This    **
** might reduce the security of your system.            **
**                                                      **
** If you see this message and you are not debugging    **
** the kernel, report this immediately to your system   **
** administrator!                                       **
**                                                      **
**   NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE   **
**********************************************************

only as WARNING not as NOTICE.

I'll leave it up to you to find an appropriate and strong enough message.

Thanks,
Guenter


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v6 4/4] hwmon: pmbus: mpq8646: gate the writes
  2026-07-29 21:51   ` Vincent Jardin
                     ` (2 preceding siblings ...)
  (?)
@ 2026-07-30  5:00   ` Markus Elfring
  -1 siblings, 0 replies; 21+ messages in thread
From: Markus Elfring @ 2026-07-30  5:00 UTC (permalink / raw)
  To: Vincent Jardin, linux-hwmon, devicetree, Charles Hsu,
	Conor Dooley, Günter Röck, Jonathan Corbet,
	Krzysztof Kozlowski, Rob Herring, Shuah Khan
  Cc: linux-kernel, linux-doc

> The write cases of debugfs entries are provisioning and bench
> helpers. By designs the MPQ8646 powers the CPU cores rail,
> so a wrong write can brown out the board or persist a bad
> setpoint into finite-cycle NVM.
See also once more:
* https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v7.2-rc5#n94

* https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v7.2-rc5#n669


Regards,
Markus

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2026-07-30  6:56 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-29 21:51 [PATCH v6 0/4] hwmon: pmbus: add MPS MPQ8646 support Vincent Jardin via B4 Relay
2026-07-29 21:51 ` Vincent Jardin
2026-07-29 21:51 ` [PATCH v6 1/4] hwmon: (pmbus/core) export pmbus_check_and_notify_faults() Vincent Jardin via B4 Relay
2026-07-29 21:51   ` Vincent Jardin
2026-07-29 22:01   ` sashiko-bot
2026-07-29 21:51 ` [PATCH v6 2/4] dt-bindings: hwmon: pmbus: add MPS MPQ8646 Vincent Jardin via B4 Relay
2026-07-29 21:51   ` Vincent Jardin
2026-07-29 22:01   ` sashiko-bot
2026-07-30  4:12     ` Guenter Roeck
2026-07-30  4:10   ` Guenter Roeck
2026-07-30  6:55     ` Vincent Jardin
2026-07-29 21:51 ` [PATCH v6 3/4] hwmon: pmbus: add MPQ8646 driver Vincent Jardin via B4 Relay
2026-07-29 21:51   ` Vincent Jardin
2026-07-29 22:05   ` sashiko-bot
2026-07-30  4:12     ` Guenter Roeck
2026-07-29 21:51 ` [PATCH v6 4/4] hwmon: pmbus: mpq8646: gate the writes Vincent Jardin via B4 Relay
2026-07-29 21:51   ` Vincent Jardin
2026-07-29 22:07   ` sashiko-bot
2026-07-30  4:14     ` Guenter Roeck
2026-07-30  4:26   ` Guenter Roeck
2026-07-30  5:00   ` Markus Elfring

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.