Re: [PATCH 1/2] dt-bindings: power: supply: maxim,max8903: add DC and USB input current-limit controls

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] The schema does not enforce mutual dependencies for functionally coupled properties (`dc-current-limit-gpios` with `dc-current-limit-mapping`, and `usb-current-limit-gpios` with `usb-current-limit-values`).
- [Low] The property `usb-current-limit-values` violates the standard devicetree unit suffix naming convention.
- [Low] The device tree node name in the added example violates standard Devicetree naming conventions.
--

commit 2e0e545e4d1ddb26950bce515555057419610f75
Author: Herman van Hazendonk <github.com@herrie.org>

dt-bindings: power: supply: maxim,max8903: add DC and USB input current-limit controls

Add four optional properties to the MAX8903 charger binding to describe
board-level GPIO control of the DC and USB input current limits.

> diff --git a/Documentation/devicetree/bindings/power/supply/maxim,max8903.yaml b/Documentation/devicetree/bindings/power/supply/maxim,max8903.yaml
> index 86af383789992..5e970ebc08df6 100644
> --- a/Documentation/devicetree/bindings/power/supply/maxim,max8903.yaml
> +++ b/Documentation/devicetree/bindings/power/supply/maxim,max8903.yaml
> @@ -44,6 +44,41 @@ properties:
>      maxItems: 1
>      description: USB suspend pin (active high, output)
>  
> +  dc-current-limit-gpios:
> +    minItems: 1
> +    maxItems: 4

[Severity: Medium]
Should the schema enforce mutual dependencies between these new properties at
the root level?

Defining a GPIO mux control without the mapping of bit patterns to current
limits is invalid, and conversely, providing a mapping without the GPIOs to
control it has no effect. Similarly, specifying usb-current-limit-values
without usb-current-limit-gpios doesn't seem meaningful.

Without a 'dependencies' or 'dependentRequired' block at the root of the
schema, would incomplete devicetree configurations silently pass
dt_binding_check validation?

> +    description:
> +      GPIOs controlling DC input current limit via resistor mux.
> +      Used with dc-current-limit-mapping to select charging current.
> +
> +  dc-current-limit-mapping:
> +    $ref: /schemas/types.yaml#/definitions/uint32-matrix

[ ... ]

> +  usb-current-limit-gpios:
> +    maxItems: 1
> +    description:
> +      GPIO controlling USB input current limit.
> +      Low = usb-current-limit-values[0], High = usb-current-limit-values[1].
> +
> +  usb-current-limit-values:
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    minItems: 2
> +    maxItems: 2
> +    default: [100000, 500000]

[Severity: Low]
Does this property name violate the standard devicetree unit suffix naming
conventions?

Properties that express a microamp value should generally end with the
-microamp suffix (e.g., usb-current-limit-microamp) to allow automatic unit
type validation by the dtschema tooling.

> +    description:
> +      USB current limits in microamps for GPIO low and high states.
> +      Default is 100mA (low) and 500mA (high) per USB specification.
> +
>  required:
>    - compatible

[ ... ]

> @@ -65,3 +100,26 @@ examples:
>        chg-gpios = <&gpio3 15 GPIO_ACTIVE_LOW>;
>        cen-gpios = <&gpio2 5 GPIO_ACTIVE_LOW>;
>      };
> +  - |
> +    /* Example with DC and USB current limit control */
> +    #include <dt-bindings/gpio/gpio.h>
> +    charger-with-current-limit {
> +      compatible = "maxim,max8903";

[Severity: Low]
Could this trigger dtc compiler warnings or dt-schema validation errors when
generic node name patterns are strictly enforced?

The Devicetree Specification recommends node names be generic and reflect the
function of the device. Should this be renamed to a generic name like
'charger'?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260603071519.807604-1-github.com@herrie.org?part=1