Re: [PATCH 1/2] dt-bindings: mfd: as3722: Convert to json-schema

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [Medium] dt-bindings: mfd: missing const constraint for gpio-cells
- [Medium] dt-bindings: mfd: additionalProperties false breaks validation
- [Medium] dt-bindings: mfd: missing core regulator schema reference in sub-nodes
- [Medium] dt-bindings: mfd: missing additionalProperties in container nodes
- [Low] dt-bindings: mfd: missing required GPIO provider properties
--

commit 3390b1809db67797a4f31079a198e4285905107a
Author: Thierry Reding <treding@nvidia.com>
Date:   Mon Sep 28 16:24:40 2026 +0000

    dt-bindings: mfd: as3722: Convert to json-schema
    
    Convert the AMS AS3722 PMIC bindings from the free-form text format to
    json-schema.

> diff --git a/Documentation/devicetree/bindings/mfd/ams,as3722.yaml b/Documentation/devicetree/bindings/mfd/ams,as3722.yaml
> new file mode 100644
> index 0000000000000..6a64445003966
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/ams,as3722.yaml
> @@ -0,0 +1,276 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---

[ ... ]

> +  # from gpio.yaml
> +  gpio-controller: true
> +  "#gpio-cells": true

[Severity: Medium]
Does this schema allow invalid configurations for "#gpio-cells" (e.g., <1>
or <3>) to silently pass validation instead of enforcing the hardware's
actual requirements? The DT bindings subsystem guidelines mandate that
each cell-count property must have a const constraint matching the hardware.

> +
> +  # optional properties
> +  ams,enable-internal-int-pullup:

[ ... ]

> +  pinmux:
> +    type: object
> +    description: |
> +      Device has 8 GPIO pins which can be configured as GPIO as well as the
> +      special IO functions.

[Severity: Medium]
Does this pinmux object definition omit additionalProperties: false? If
omitted, invalid or misspelled child nodes under pinmux might be silently
accepted instead of being flagged as errors since additionalProperties
defaults to true.

> +
> +      Please refer to pinctrl-bindings.txt in this directory for details of
> +      the common pinctrl bindings used by client devices, including the

[ ... ]

> +        required:
> +          - pins
> +
> +  regulators:
> +    type: object
> +    description: Device has multiple DCDC and LDOs. The node "regulators" is
> +      required if regulator functionality is needed.

[Severity: Medium]
Like the pinmux node above, does this regulators object also need
additionalProperties: false to ensure invalid regulator nodes (e.g., sd7
or ldo8) do not silently pass validation?

> +
> +    properties:
> +      vsup-sd2-supply:

[ ... ]

> +      vin-ldo11-supply:
> +        description: input supply for LDO11
> +
> +    patternProperties:
> +      "^(sd[0-6]|ldo[0-7]|ldo9|ldo10|ldo11)$":
> +        type: object
> +        description: These sub-nodes must be named after one of the regulators
> +          found on the AS3277. Each sub-node should contain the constraints and
> +          initialization information for that regulator.

[Severity: Medium]
Do these regulator sub-nodes need to link to the core regulator schema
using a reference to /schemas/regulator/regulator.yaml#? Without this
reference, standard regulator properties like regulator-min-microvolt
are unvalidated and typos will be silently accepted by the schema checker.

> +
> +        properties:
> +          ams,ext-control:

[ ... ]

> +          ams,enable-tracking:
> +            $ref: /schemas/types.yaml#/definitions/flag
> +            description: Enable tracking with SD1, only supported by LDO3.
> +
> +additionalProperties: false

[Severity: Medium]
Will this strict filter fail to account for properties brought in by refs or
standard core schemas like gpio.yaml? Using additionalProperties: false
only evaluates properties explicitly listed in the local properties block,
which will falsely reject valid standard properties. Should this be
unevaluatedProperties: false instead?

> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupt-controller
> +  - "#interrupt-cells"

[Severity: Low]
Does this required list omit the mandatory provider properties for a
hardware device acting as a GPIO controller? The subsystem guidelines
explicitly state that hardware provider capabilities must be included in
the required list (e.g., gpio-controller, #gpio-cells), otherwise
incomplete device tree nodes can pass schema validation.

> +
> +allOf:
> +  - $ref: /schemas/gpio/gpio.yaml
> +
> +examples:
> +  - |
> +    #include <dt-bindings/mfd/as3722.h>
> +
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        pmic@48 {
> +            compatible = "ams,as3722";
> +            reg = <0x48>;
> +
> +            ams,system-power-controller;
> +
> +            interrupt-parent = <&intc>;
> +            interrupt-controller;
> +            #interrupt-cells = <2>;
> +
> +            gpio-controller;
> +            #gpio-cells = <2>;
> +
> +            pinctrl-names = "default";
> +            pinctrl-0 = <&as3722_default>;

[Severity: Medium]
Will the inclusion of pinctrl-names and pinctrl-0 in this example trigger
validation failures, since they are missing from the schema's local
properties list and additionalProperties is set to false?

> +
> +            as3722_default: pinmux {
> +                gpio0 {
> +                    pins = "gpio0";

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260928-as3722-bindings-v1-0-35f423a9f2b2@nvidia.com?part=1