Staging/max77533 by sean-park-pub · Pull Request #3491 · analogdevicesinc/linux

sean-park-pub · 2026-08-14T10:20:37Z

PR Description

Please replace this comment with a summary of your changes, and add any context
necessary to understand them. List any dependencies required for this change.
To check the checkboxes below, insert a 'x' between square brackets (without
any space), or simply check them after publishing the PR.
If you changes include a breaking change, please specify dependent PRs in the
description and try to push all related PRs simultaneously.

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

github-actions · 2026-08-14T10:41:37Z

LLM review

This series adds a regulator driver and dt-bindings for the ADI/Maxim MAX77533 buck converter.

run: 31792726869

`5dcfc1512838a` - Add MAX77533 regulator driver

Build failure: max77533_parse_dt() calls FIELD_PREP(MAX77533_BITS_SFT_STRT, sel) at line 218 but the file never includes linux/bitfield.h. This breaks build_gcc_x86_64 and build_llvm_x86_64 with:

drivers/regulator/max77533-regulator.c:218:36: error: implicit declaration of function 'FIELD_PREP' [-Wimplicit-function-declaration]

Reproduced locally with set_arch gcc_x86 + auto_set_kconfig + make drivers/regulator/max77533-regulator.o. Adding #include <linux/bitfield.h> fixes it; a fixup patch is provided. After the fix, the object builds cleanly and make C=2 (sparse) reports nothing further.

Register mapping checked against the datasheet: cross-referenced CONFIG_A/CONFIG_B bitfields, the current-limit table, the soft-start table, the I2C slave address options, and POK polarity against max77533.pdf. All match the driver and the dt-binding (adi,max77533-regulator.yaml); no bug found there.

`56c3cf63f84b3` - dt-bindings for MAX77533

Validated with dt-doc-validate and dt-extract-example - schema and example are well formed, no issues found.

CI warnings

checkpatch reports "Missing commit description" on all three commits (Kconfig hunk, dt-binding, defconfig) since the commit bodies only contain the title - worth a brief description before merging, but not a functional issue. The "does MAINTAINERS need updating?" hint is consistent with how the sibling MAX77503/MAX77541 regulator drivers were merged (no dedicated entry), so no action needed there.

Verification data

max77533.pdf was located via the ADI PDF sitemap and fetched pre-converted (docling) as markdown, then used to verify the CONFIG_A (0x00) / CONFIG_B (0x01) register bitfields, the peak-current-limit and soft-start decode tables, the I2C slave address options, and POK pin polarity against the driver's BIT()/GENMASK() definitions and the dt-binding.

Suggested patches

Apply the suggested patches with:

cd path/to/repository
export GITHUB_TOKEN=ghp_***
apply-patches --repo=analogdevicesinc/linux 31792726869

Install instructions

The following one-liner installs the script if not present already:

curl -fSsL "https://raw.githubusercontent.com/analogdevicesinc/doctools/refs/heads/main/ci/scripts/apply-patches.sh"      -o ~/.local/bin/apply-patches.sh &&   grep -q "/apply-patches.sh" ~/.bashrc || echo "source ~/.local/bin/apply-patches.sh" >> $_ ; . $_

More information at AI Usage.

github-actions · 2026-08-14T11:39:38Z

LLM review

This series adds a regulator driver and dt-bindings for the ADI/Maxim MAX77533 buck converter.

run: 31796333622

`63d5be934d09` - Add MAX77533 regulator driver

Ordering bug: max77533_probe() calls max77533_parse_dt() (which writes the
adi,soft-start-us / adi,peak-current-limit-microamp selections into
CONFIG_A's SFT_STRT[1:0]/I_PEAK fields) before devm_regulator_register().
The same regulator_desc also wires soft_start_reg/mask and csel_reg/mask to
those exact bits for the generic regulator-soft-start / regulator-min|max-microamp
DT properties. set_machine_constraints() in drivers/regulator/core.c applies
those generic constraints during registration, i.e. after max77533_parse_dt()
already ran, and regulator_set_soft_start_regmap() unconditionally forces the
full mask (8ms). So a board that combines both property styles will have its
adi,soft-start-us/adi,peak-current-limit-microamp value silently overwritten.
Fix: apply the vendor properties after devm_regulator_register() (patch attached,
build-verified for x86).

Register mapping was cross-checked against max77533.pdf: CONFIG_A/CONFIG_B
bitfields, current-limit table, voltage table, I2C address options, and compatible
strings all match the driver and dt-binding exactly - no discrepancy found there.
get_status()'s POK-low -> REGULATOR_STATUS_ERROR and the FPWM/SKIP <->
REGULATOR_MODE_FAST/NORMAL mapping also match the existing, upstream
max77857-regulator.c convention, so those are not flagged.

`56c3cf63f84b` - dt-bindings for MAX77533

Validated with dt-doc-validate - schema and example are well formed, no issues found.

CI warnings

checkpatch "Missing commit description" on the dt-binding, defconfig, and Kconfig
commits is reproduced locally and is accurate (commit bodies contain only the title) -
worth a short description before merging, not a functional issue. The smatch warning
on kernel/module/main.c:1444 from build_gcc_arm is unrelated to this series - that
file isn't touched by any commit in this range.

Verification data

max77533.pdf was located via the ADI PDF sitemap and fetched pre-converted (docling)
as max77533.md, then used to verify the CONFIG_A/CONFIG_B register bitfields,
current-limit and voltage decode tables, I2C slave address table, and the POK/soft-start
behavior description referenced in the ordering-bug analysis above. The driver was also
rebuilt for x86 and arm (set_arch gcc_x86/gcc_arm + auto_set_kconfig + make drivers/regulator/max77533-regulator.o), plus make C=2, make W=1, and
smatch -p=kernel - all clean.

Suggested patches

Apply the suggested patches with:

cd path/to/repository
export GITHUB_TOKEN=ghp_***
apply-patches --repo=analogdevicesinc/linux 31796333622

Install instructions

The following one-liner installs the script if not present already:

curl -fSsL "https://raw.githubusercontent.com/analogdevicesinc/doctools/refs/heads/main/ci/scripts/apply-patches.sh"      -o ~/.local/bin/apply-patches.sh &&   grep -q "/apply-patches.sh" ~/.bashrc || echo "source ~/.local/bin/apply-patches.sh" >> $_ ; . $_

More information at AI Usage.

nunojsa

Here it goes my first pass review! Still need to look at bindings in follow up reviews! Also, is the end goal to upstream the driver?

I also wonder why we need a dedicated defconfig for a driver like this (is it just for your testing?).

Also bear in mind you need better commit messages

nunojsa · 2026-08-18T15:37:14Z

@@ -0,0 +1,323 @@
+// SPDX-License-Identifier: GPL-2.0-or-later


typically GPL-2.0-only (not sure if exactly like this the string)

It would be changed to below in the next commit.

// SPDX-License-Identifier: GPL-2.0-only

nunojsa · 2026-08-18T15:37:22Z

@@ -0,0 +1,323 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2025 Analog Devices, Inc.


It would be changed to below in the next commit.

/*

Copyright (c) 2026 Analog Devices, Inc.

ADI regulator driver for MAX77533.
*/

nunojsa · 2026-08-18T15:37:50Z

+#include <linux/of.h>
+#include <linux/regmap.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/of_regulator.h>


not stating something is missing but make sure you IWYU

nunojsa · 2026-08-18T15:38:19Z

+static const struct i2c_device_id max77533_id[] = {
+	{ "max77533" },
+	{ }
+};


These tale should match the above of_device_id

i2c_device_id would be expanded in the next commit.

nunojsa · 2026-08-18T15:38:57Z

+		return -ENOMEM;
+
+	max77533->dev = dev;
+	i2c_set_clientdata(client, max77533);


I don't see i2c_get_clientdata() so the above seems not needed to me

i2c_set_clientdata() would be removed in the next commit.

nunojsa · 2026-08-18T15:41:55Z

+	max77533->rdev = devm_regulator_register(dev, &max77533_desc, &config);
+	if (IS_ERR(max77533->rdev))
+		return dev_err_probe(dev, PTR_ERR(max77533->rdev),
+				     "Failed to register regulator\n");


The above is typically the last thing we want to do! For example, do we somehow depende on the max77533_pok_irq_handler() on the regulator callbacks?

Yes, max77533_pok_irq_handler() depends on max77533->rdev being valid
since it calls regulator_notifier_call_chain(). Therefore, the regulator
must be registered BEFORE requesting the IRQ, which is the current order.

The IRQ error handling has been changed to hard errors as requested.

nunojsa · 2026-08-18T15:42:25Z

+
+	if (of_property_read_bool(np, "adi,en-logic-and"))
+		regmap_update_bits(max77533->regmap, MAX77533_REG_CFG,
+				   MAX77533_BIT_EN_LOGIC, MAX77533_BIT_EN_LOGIC);


don't ignore return error from regmap_update_bits()

Error handling would be added in the next commit.

nunojsa · 2026-08-18T15:43:55Z

+	int ret;
+
+	if (!np)
+		return 0;


no need for the above check

It would be removed in the next commit.

nunojsa · 2026-08-18T15:46:22Z

+	if (!(val & MAX77533_BIT_EN))
+		return REGULATOR_STATUS_OFF;
+
+	if (max77533->pok_gpio) {


Can you tell the usage of this gpio (being lazy to no check the datasheet for now :))?

POK (Power OK) is an open-drain output that indicates the regulator
output status:

High: Output voltage is within regulation (typically within ±8% of target)

Low: Output voltage is below the POK threshold (under-voltage condition)
This is used to detect power-good status and under-voltage events.
When POK goes low, we notify REGULATOR_EVENT_UNDER_VOLTAGE; when it goes high, we notify REGULATOR_EVENT_VOLTAGE_CHANGE to indicate recovery.

nunojsa · 2026-08-18T15:47:30Z

+		return REGULATOR_STATUS_OFF;
+
+	if (max77533->pok_gpio) {
+		int pok = gpiod_get_value_cansleep(max77533->pok_gpio);


In theory we do not need to check if an option gpio exists because gpiolib will return 0 if desc is NULL! That's why I'm wondering about this gpio usage and why we treat 0 as REGULATOR_STATUS_ERROR

When POK is low (0), it indicates that the output voltage has dropped below the POK threshold, which is an under-voltage fault condition. This is why we return REGULATOR_STATUS_ERROR rather than REGULATOR_STATUS_OFF.

The regulator is still enabled (EN bit is set), but the output is not in regulation - this is an error state, not an intentional off state.

Regarding the NULL check: you're correct that gpiod_get_value_cansleep() returns 0 for NULL desc. However, I kept the explicit check for clarity since the logic path differs - without POK GPIO we assume the regulator is ON when enabled, while with POK GPIO we verify actual output status.

sean-park-pub · 2026-08-19T02:29:35Z

A defconfig is just for a testing, so I'll add REGULATOR_MAX77533 to Kconfig.adi.
The next commit would have a better commit message.

The ultimate goal is to upstream this driver to mainline Linux.
However, in the short term, our customer is waiting to receive the
driver first, so we are prioritizing delivering a working driver to them.

Once the customer delivery is complete, I will address all review feedback and prepare a proper patch series for submission to
linux-kernel and linux-pm mailing lists.

Add REGULATOR_MAX77533 to KERNEL_ALL_ADI_DRIVERS config to enable building the MAX77533 regulator driver with the ADI driver collection. Signed-off-by: Sean Park <sean.park@analog.com>

Add device tree bindings documentation for the Maxim MAX77533 step-down regulator and its variants. Signed-off-by: Sean Park <sean.park@analog.com>

Add support for the Maxim MAX77533 high-efficiency step-down converter. The MAX77533 provides a configurable output voltage from 0.8V to 5.0V with 50mV steps and supports up to 3A output current. Features supported: - Output voltage regulation (0.8V - 5.0V, 50mV steps) - Enable/disable control via I2C - Peak current limit selection (500mA / 2A) - Soft-start time configuration - FPWM / Auto-PFM mode selection - Active discharge control - Power-OK GPIO monitoring with interrupt support The driver supports all device variants: - MAX77533, MAX77533A, MAX77533Q - MAX77533B-12, MAX77533B-18, MAX77533B-33 Signed-off-by: Sean Park <sean.park@analog.com>

sean-park-pub · 2026-08-19T04:31:51Z

Hi @nunojsa

I've updated driver code regarding to your advice. Could you please confirm that again?

sean-park-pub requested review from CiprianRegus, amiclaus, btogorean, buha, chegbeli, danmois, dbogdan, machschmitt, mhennerich, nunojsa, rbolboac and stefpopa as code owners August 14, 2026 10:20

mhennerich added the llm review Request a review from a LLM Reviewer label Aug 14, 2026

sean-park-pub force-pushed the staging/max77533 branch from 5dcfc15 to 63d5be9 Compare August 14, 2026 10:40

mhennerich added llm review Request a review from a LLM Reviewer and removed llm review Request a review from a LLM Reviewer labels Aug 14, 2026

nunojsa reviewed Aug 18, 2026

View reviewed changes

sean-park-pub force-pushed the staging/max77533 branch from 63d5be9 to ea878f8 Compare August 19, 2026 02:42

sean-park-pub added 3 commits August 19, 2026 13:01

Kconfig.adi: Add MAX77533 regulator to ADI drivers

a7ab101

Add REGULATOR_MAX77533 to KERNEL_ALL_ADI_DRIVERS config to enable building the MAX77533 regulator driver with the ADI driver collection. Signed-off-by: Sean Park <sean.park@analog.com>

dt-bindings: regulator: Add Maxim MAX77533 bindings

857ef2a

Add device tree bindings documentation for the Maxim MAX77533 step-down regulator and its variants. Signed-off-by: Sean Park <sean.park@analog.com>

sean-park-pub force-pushed the staging/max77533 branch from ea878f8 to 64de809 Compare August 19, 2026 04:09

		@@ -0,0 +1,323 @@
		// SPDX-License-Identifier: GPL-2.0-or-later

Uh oh!

Conversation

sean-park-pub commented Aug 14, 2026

PR Description

PR Type

PR Checklist

Uh oh!

github-actions Bot commented Aug 14, 2026

LLM review

5dcfc1512838a - Add MAX77533 regulator driver

56c3cf63f84b3 - dt-bindings for MAX77533

CI warnings

Verification data

Suggested patches

Uh oh!

github-actions Bot commented Aug 14, 2026

LLM review

63d5be934d09 - Add MAX77533 regulator driver

56c3cf63f84b - dt-bindings for MAX77533

CI warnings

Verification data

Suggested patches

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!

sean-park-pub Aug 19, 2026 • 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!

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!

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!

Choose a reason for hiding this comment

Uh oh!

sean-park-pub commented Aug 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sean-park-pub commented Aug 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

`5dcfc1512838a` - Add MAX77533 regulator driver

`56c3cf63f84b3` - dt-bindings for MAX77533

`63d5be934d09` - Add MAX77533 regulator driver

`56c3cf63f84b` - dt-bindings for MAX77533

sean-park-pub Aug 19, 2026 •

edited

Loading

sean-park-pub commented Aug 19, 2026 •

edited

Loading