add MAX17616/MAX17616A driver #2949

kseerp · 2025-09-23T09:35:56Z

PR Description

Add MAX17616/MAX17616A Current-Limiter with Overvoltage/Surge, Undervoltage, Reverse Polarity, Loss of Ground Protection with PMBus Interface

Lore: https://patchew.org/linux/20250930-upstream-max17616-v1-0-1525a85f126c@analog.com/

PR Type

Bug fix (a change that fixes an issue)
New feature (a change that adds new functionality)
Breaking change (a change that affects other repos or cause CIs to fail)

PR Checklist

I have conducted a self-review of my own code changes
I have tested the changes on the relevant hardware
I have updated the documentation outside this repo accordingly (if there is the case)

nunojsa

Also missing kconfig.adi patch. And you know the drill already :). Please send this upstream.

nunojsa · 2025-09-23T13:46:35Z

Documentation/devicetree/bindings/hwmon/pmbus/adi,max17616.yaml

+  compatible:
+    enum:
+      - adi,max17616
+      - adi,max17616a


Why 2 compatibles? What's the difference between the variants? It seems that in terms of SW there's none (apart from the name)...

nunojsa · 2025-09-23T13:50:49Z

drivers/hwmon/pmbus/max17616.c

+		return dev_err_probe(dev, ret, "Failed to read MFR_MODEL\n");
+
+	if ((strncmp(buf + 1, "MAX17616", 8) && strncmp(buf + 1, "MAX17616A", 9)))
+		return dev_err_probe(dev, -ENODEV, "Unsupported device\n");


Do we really want to have the above? And I guess that if ((strncmp(buf + 1, "MAX17616", 8)) should be enough, no? AFAICT, the two variants are pretty much compatible?

nunojsa · 2025-09-23T13:51:19Z

drivers/hwmon/pmbus/max17616.c

+static struct i2c_driver max17616_driver = {
+	.driver = {
+		.name = "max17616",
+		.of_match_table = of_match_ptr(max17616_of_match),


please drop of_match_ptr()

kseerp · 2025-10-13T05:57:11Z

Series applied upstream, add ADI compliant version.

Add device tree documentation for MAX17616/MAX17616A current-limiter with overvoltage/surge, undervoltage, reverse polarity, loss of ground protection with PMBus interface. Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com> Acked-by: Conor Dooley <conor.dooley@microchip.com> Link: https://lore.kernel.org/r/20250930-upstream-max17616-v1-1-1525a85f126c@analog.com Signed-off-by: Guenter Roeck <linux@roeck-us.net>

Add interrupt property to document the SMBALERT pin functionality for fault condition signal. Suggested-by: Marcelo Schmitt <marcelo.schmitt@analog.com> Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com> Acked-by: Conor Dooley <conor.dooley@microchip.com> Link: https://lore.kernel.org/r/20251013-upstream-max17616-v1-1-0e15002479c3@analog.com Signed-off-by: Guenter Roeck <linux@roeck-us.net>

Add support for MAX17616/MAX17616A current-limiter with overvoltage/surge, undervoltage, reverse polarity, loss of ground protection with PMBus interface. The PMBus interface allows monitoring of input/output voltages, output current and temperature. Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com> Link: https://lore.kernel.org/r/20250930-upstream-max17616-v1-2-1525a85f126c@analog.com [groeck: Fixed htmldocs 'WARNING: Title underline too short' as reported by Kriish Sharma <kriish.sharma2006@gmail.com>] Signed-off-by: Guenter Roeck <linux@roeck-us.net>

Add MAX17616 to the ADI Kconfig. Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>

kseerp · 2025-10-16T05:50:33Z

Follow up patch and updates added

gastmaier · 2025-10-16T07:55:26Z

drivers/hwmon/pmbus/max17616.c

+
+static int max17616_probe(struct i2c_client *client)
+{
+	return pmbus_do_probe(client, &max17616_info);


From CI:
ERROR: modpost: module max17616 uses symbol pmbus_do_probe from namespace PMBUS, but does not import it.

you are using a symbol from PMBUS, but not setting on the Kconfig that your driver depends on it.

Don't think that's the issue. I think the problem is that MODULE_IMPORT_NS("PMBUS"); needs to be MODULE_IMPORT_NS(PMBUS);

There was a conversion upstream to quote the namespace that happened after 6.12.

I do notice in our tree the unquoted syntax MODULE_IMPORT_NS(PMBUS); is used. I will update the driver to make it compliant to our tree.

gastmaier · 2025-10-16T07:56:21Z

Documentation/hwmon/max17616.rst

+in1_label         "vin"
+in1_input         Measured input voltage
+in1_alarm	  Input voltage alarm
+in2_label	  "vout1"
+in2_input	  Measured output voltage
+curr1_label	  "iout1"
+curr1_input	  Measured output current.
+curr1_alarm	  Output current alarm
+temp1_input       Measured temperature
+temp1_alarm       Chip temperature alarm


Are you mixing tabs and spaces here?
Consider just using spaces

gastmaier · 2025-10-16T07:58:00Z

Documentation/hwmon/max17616.rst

+  * Analog Devices MAX17616/MAX17616A
+
+    Prefix: 'max17616'
+
+    Addresses scanned: -
+
+    Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/max17616-max17616a.pdf


Suggested change

* Analog Devices MAX17616/MAX17616A

Prefix: 'max17616'

Addresses scanned: -

Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/max17616-max17616a.pdf

- | Analog Devices MAX17616/MAX17616A

| Prefix: 'max17616'

| Addresses scanned: -

| Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/max17616-max17616a.pdf

You can force linebreaks using this syntax

gastmaier · 2025-10-16T08:02:40Z

Documentation/hwmon/max17616.rst

+
+Author:
+
+  - Kim Seer Paller <kimseer.paller@analog.com>


leading whitespace has a meaning in rst, it will nest the list in a block.
It is not a big deal, but does have an effect.

gastmaier · 2025-10-16T08:03:10Z

Documentation/hwmon/max17616.rst

+-----------
+
+This driver does not auto-detect devices. You will have to instantiate
+the devices explicitly. Please see Documentation/i2c/instantiating-devices.rst


Suggested change

the devices explicitly. Please see Documentation/i2c/instantiating-devices.rst

the devices explicitly. Please see :doc:`/i2c/instantiating-devices`

https://www.sphinx-doc.org/en/master/usage/referencing.html#role-doc

Note these are patches already applied upstream. So any change needs to go first there

I used existing driver's rst as template. Should I update to consistent spacing/syntax or keep the current format for consistency with other docs?

kseerp requested review from CiprianRegus, amiclaus, btogorean, buha, chegbeli, danmois, dbogdan, machschmitt, mhennerich, nunojsa and rbolboac as code owners September 23, 2025 09:35

kseerp force-pushed the dev-max17616 branch from c3062cc to 4b58025 Compare September 23, 2025 09:53

nunojsa reviewed Sep 23, 2025

View reviewed changes

kseerp force-pushed the dev-max17616 branch from 4b58025 to aad1fc3 Compare October 13, 2025 05:55

kseerp added 4 commits October 16, 2025 13:44

hwmon: Kconfig.adi: imply MAX17616

9b0d059

Add MAX17616 to the ADI Kconfig. Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>

kseerp force-pushed the dev-max17616 branch from aad1fc3 to 9b0d059 Compare October 16, 2025 05:44

gastmaier reviewed Oct 16, 2025

View reviewed changes

	the devices explicitly. Please see Documentation/i2c/instantiating-devices.rst
	the devices explicitly. Please see :doc:`/i2c/instantiating-devices`

add MAX17616/MAX17616A driver #2949

Are you sure you want to change the base?

add MAX17616/MAX17616A driver #2949

Uh oh!

Conversation

kseerp commented Sep 23, 2025 • edited by gastmaier Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Description

PR Type

PR Checklist

Uh oh!

nunojsa left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kseerp commented Oct 13, 2025

Uh oh!

kseerp commented Oct 16, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gastmaier Oct 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gastmaier Oct 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

kseerp commented Sep 23, 2025 •

edited by gastmaier

Loading

gastmaier Oct 16, 2025 •

edited

Loading

gastmaier Oct 16, 2025 •

edited

Loading