Re: [PATCH 1/2] dt-bindings: hwmon: pmbus: add max20830

[sashiko-bot@kernel.org]

Sashiko AI review found 3 potential issue(s):
- [Medium] Incomplete Regulator Schema
- [Medium] Missing Hardware Capability (Interrupts)
- [Low] Devicetree Best Practices Violation (Node Name)
--

commit 20a4d8dcd2f3344ac22d7d2e974bae1ef57874ed
Author: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>

dt-bindings: hwmon: pmbus: add max20830

This commit adds device tree documentation for the MAX20830 step-down DC-DC
switching regulator with PMBus interface. It allows monitoring of voltage,
current, and temperature.

> diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/adi,max20830.yaml b/Documentation/devicetree/bindings/hwmon/pmbus/adi,max20830.yaml
> new file mode 100644
> index 0000000000000..b20f3be176615
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/pmbus/adi,max20830.yaml
[ ... ]
> +properties:
> +  compatible:
> +    const: adi,max20830
> +
> +  reg:
> +    maxItems: 1
> +

Since this is a PMBus device that typically signals hardware faults to the
host via the standard SMBALERT# pin, should the schema define the optional
interrupts property?

> +  vddh-supply:
> +    description:
> +      Phandle to the regulator that provides the VDDH power supply.
[ ... ]
> +required:
> +  - compatible
> +  - reg
> +  - vddh-supply
> +
> +additionalProperties: false

The commit message explicitly states the MAX20830 is a step-down DC-DC
switching regulator with an adjustable output, but the schema omits any
reference to the core regulator schema or a regulators child node.

Because additionalProperties is set to false, does this prevent board
designers from describing the hardware's standard output constraints,
such as regulator-min-microvolt or regulator-max-microvolt, without
causing a schema validation error?

> +
> +examples:
> +  - |
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        hwmon@30 {

This isn't a bug, but the device node name in the example uses a Linux
subsystem name. Should it use a generic functional name representing the
hardware, such as regulator@30 or power-sensor@30?

> +            compatible = "adi,max20830";
> +            reg = <0x30>;
> +            vddh-supply = <&vddh>;
> +        };
> +    };
> +...

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260414-dev_max20830-v1-0-210d3f82c571@analog.com?part=1