Skip to content

hwmon: pmbus: Add Analog Devices MAX20830 driver#3183

Open
actorreno wants to merge 3 commits intomainfrom
dev_max20830
Open

hwmon: pmbus: Add Analog Devices MAX20830 driver#3183
actorreno wants to merge 3 commits intomainfrom
dev_max20830

Conversation

@actorreno
Copy link
Contributor

PR Description

This PR adds support for the Analog Devices MAX20830 step-down DC-DC
switching regulator with PMBus interface.

Datasheet: MAX20830

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 compiled my changes, including the documentation
  • I have tested the changes on the relevant hardware
  • I have updated the documentation outside this repo accordingly
  • I have provided links for the relevant upstream lore

@nunojsa
Copy link
Collaborator

nunojsa commented Mar 13, 2026

@actorreno, please go first through the bot for reviews :)

https://github.com/analogdevicesinc/linux/actions/workflows/llm.yml

Naturally you can disagree with some feedback but odds are that it will catch some stuff before I (and others) jump in reviewing.

@actorreno
Copy link
Contributor Author

Hi

I tried running the workflow by putting the sha of the latest commit of this branch to the field when running the LLM Agent
https://github.com/analogdevicesinc/linux/actions/runs/23044576411

It checked my commits (3 commits) and well I think plus 4 more earlier commits.
Do you usually put something else in the prompt field like "only max20830 branch"?

But for now, all three commits seems to be "Accepted as is" in the review :)

@nunojsa
Copy link
Collaborator

nunojsa commented Mar 13, 2026

It checked my commits (3 commits) and well I think plus 4 more earlier commits.

I would sat the base sha is the one you need to set. Not sure if head defaults to HEAD

But for now, all three commits seems to be "Accepted as is" in the review :)

Cool

Copy link
Collaborator

@gastmaier gastmaier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, just notes

About the CI/CD: no, it does not infer the base sha. I understand it would be a nice to have feature.
On the other hand, there are scenarios that you do want to not consider some commits, like API updates.

reg:
maxItems: 1

vcc-supply: true
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
vcc-supply: true
vddh-supply:
avdd-supply:

A description would also be nicer than true
datasheet p2

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will add,

regarding vddh, I did wonder what should be followed in the supply if vddh in the datasheet or the "usual" vcc.
MAX17616 (a device I saw while doing this) and MAX20830 denotes vcc as the internal LDO, but max17616.yaml uses vcc for its supply.

thanks!

required:
- compatible
- reg
- vcc-supply
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

vddh is required, LDOIN optional

@@ -0,0 +1,60 @@
.. SPDX-License-Identifier: GPL-2.0
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may be asked to delete this file if it does not provide significant value (e.g., explain really hard to understand parts that cannot be understood from the code or dt-binding)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do think it doesn't add much. should I just drop the file?

-----------

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is a strong no-no.
It isn't even referencing with Sphinx references.
it is also obvious

Add device tree documentation for MAX20830 step-down DC-DC switching
regulator with PMBus interface.

Signed-off-by: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>
Add support for MAX20830 step-down DC-DC switching regulator with
PMBus interface. It allows monitoring of input/output voltage,
output current and temperature through the PMBus serial interface.

Signed-off-by: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>
Add MAX20830 to the ADI Kconfig.

Signed-off-by: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>
@actorreno
Copy link
Contributor Author

v2:

  • max20830.c
    -> replaced "i2c_smbus_read_block_data" to "i2c_smbus_read_i2c_block_data", more useable by boards like rpi
  • max20830.yaml
    -> renamed vcc to vddh as per datasheet. Added feedback above
  • max20830.rst
    -> removed usage and edited raw directory "Documentation/hwmon/pmbus.rst" to ":doc:/hwmon/pmbus"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants