Re: [PATCH v12 1/5] dt-bindings: regulator: Document MediaTek MT6373 PMIC Regulators

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Medium] The commit message erroneously describes the MT6363 PMIC instead of the MT6373 PMIC.
- [Medium] The `maxItems` constraint for `regulator-allowed-modes` in buck regulators restricts the array to 3 items, despite 4 valid modes being defined.
- [Low] Incomplete documentation for the `vs1-ldo1-supply` property.
- [Low] Repetitive wording in the descriptions for `regulator-allowed-modes`.
--

commit c2db0db607c8ddf5d3875d65350f6abb072b3b6a
Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

dt-bindings: regulator: Document MediaTek MT6373 PMIC Regulators

This commit adds device tree bindings for the regulators found in the MediaTek
MT6373 PMIC. It defines the constraints and allowed operating modes for 10
BUCK and 23 LDO regulators, along with their input supplies.

> dt-bindings: regulator: Document MediaTek MT6373 PMIC Regulators
> 
> Add bindings for the regulators found in the MediaTek MT6363 PMIC,

This isn't a bug, but should the commit message say MT6373 instead of MT6363?
The patch adds the mediatek,mt6373-regulator.yaml schema, but the body mentions
MT6363.

> diff --git a/Documentation/devicetree/bindings/regulator/mediatek,mt6373-regulator.yaml b/Documentation/devicetree/bindings/regulator/mediatek,mt6373-regulator.yaml
> new file mode 100644
> index 0000000000000..4562f291fc0ae
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/mediatek,mt6373-regulator.yaml

[ ... ]

> +  vs1-ldo1-supply:
> +    description: Input supply for vant18, vaud18, vcn18io

Since the schema also defines vrf18-aif and vrfio18-aif LDOs under the
"^vrf(09|12|13|18|io18)-aif$" pattern, and they are powered by vs1-ldo1,
could this description be updated to include them?

[ ... ]

> +  "^vbuck[0-9]$":
> +    type: object
> +    $ref: regulator.yaml#
> +    properties:
> +      regulator-allowed-modes:
> +        description: |
> +          Allowed Buck regulator operating modes allowed. Valid values below.

This isn't a bug, but the word "allowed" is repeated in this description.

[ ... ]

> +            3 - Forced Ultra Low Power mode for ultra low load, this greatly
> +                reduces the maximum output power, makes the regulator to be
> +                efficient only for ultra light load, and greatly reduces the
> +                quiescent current (Iq) of the buck.
> +        maxItems: 3
> +        items:
> +          enum: [ 0, 1, 2, 3 ]

If a board design supports and configures all four operating modes
simultaneously (e.g., <0 1 2 3>), will it fail schema validation since
maxItems is restricted to 3?

Should maxItems be updated to 4 to match the number of defined enum values?

[ ... ]

> +$defs:
> +  ldo-common:
> +    type: object
> +    $ref: regulator.yaml#
> +    unevaluatedProperties: false
> +    properties:
> +      regulator-allowed-modes:
> +        description: |
> +          Allowed LDO regulator operating modes allowed. Valid values below.

This isn't a bug, but the word "allowed" is also repeated in this description.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260511101355.122478-1-angelogioacchino.delregno@collabora.com?part=1