Re: [PATCH v7 32/34] dt-bindings: pinctrl: airoha: add support of en7523 pin controller

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [Medium] The binding schema is missing the 'airoha,chip-scu' phandle property, which is explicitly parsed by the pinctrl driver.
- [Medium] The dt-binding example misleadingly presents the pinctrl node as a standalone top-level device, which will cause probe failure due to a missing parent regmap.
- [Medium] The 'groups' property constraints in the 'allOf' conditionals incorrectly use 'enum' instead of 'items: enum' for single-item arrays, which will cause valid device trees to fail dt-schema validation.
- [Medium] The top-level schema incorrectly uses 'additionalProperties: false' instead of 'unevaluatedProperties: false' when including a reference to 'pinctrl.yaml#'.
- [Low] The example node incorrectly assigns an integer value to the boolean property 'drive-open-drain'.
--

--- Patch [32]: [PATCH v7 32/34] dt-bindings: pinctrl: airoha: add support of en7523 pin controller ---
commit 995063565828879967b4a8fd673a347a158aa81e
Author: Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu>

dt-bindings: pinctrl: airoha: add support of en7523 pin controller

This patch adds device tree binding schema for Airoha EN7523 SoC pin
controller.

> diff --git a/Documentation/devicetree/bindings/pinctrl/airoha,en7523-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/airoha,en7523-pinctrl.yaml
> new file mode 100644
> index 0000000000000..db8e39af1a152
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pinctrl/airoha,en7523-pinctrl.yaml
> @@ -0,0 +1,375 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pinctrl/airoha,en7523-pinctrl.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Airoha EN7523 Pin Controller
> +
> +maintainers:
> +  - Lorenzo Bianconi <lorenzo@kernel.org>
> +
> +description:
> +  The Airoha's EN7523 Pin controller is used to control SoC pins.
> +
> +properties:
> +  compatible:
> +    const: airoha,en7523-pinctrl

[Severity: Medium]
Should the airoha,chip-scu property be documented here as well?

If a device tree includes the airoha,chip-scu phandle, which the driver
specifically looks for via syscon_regmap_lookup_by_phandle(), it will fail
validation because the property is completely undocumented in this schema.

[ ... ]

> +          - if:
> +              properties:
> +                function:
> +                  const: pwm
> +            then:
> +              properties:
> +                groups:
> +                  enum: [gpio0, gpio1, gpio2, gpio3, gpio4, gpio5, gpio6,
> +                         gpio7, gpio8, gpio9, gpio10, gpio11, gpio12, gpio13,

[Severity: Medium]
Could this cause validation failures for valid device trees?

It looks like groups is constrained using enum instead of items: enum for
single-item arrays. Because device tree string-arrays are evaluated as arrays,
using enum for a scalar string expectation might cause dt_binding_check to
reject legitimate configurations like pwm or pon functions. Is this same
pattern present in the other conditionals?

[ ... ]

> +    additionalProperties: false
> +
> +additionalProperties: false

[Severity: Medium]
Will this top-level directive reject standard pinctrl properties?

Since the schema includes a reference to pinctrl.yaml# in its allOf block,
using additionalProperties: false instead of unevaluatedProperties: false
might cause the schema to reject standard properties like pinctrl-0 or
pinctrl-names on the provider node.

> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +    pinctrl {
> +      compatible = "airoha,en7523-pinctrl";

[Severity: Medium]
Would placing the pinctrl node at the top level without a parent syscon cause
probe failures?

The driver relies on device_node_to_regmap(dev->parent->of_node) during probe.
If users copy this example and place the pinctrl node directly under the root
bus, it appears the driver will fail to find the parent regmap.

[ ... ]

> +      pcie1-rst-pins {
> +        conf {
> +          pins = "pcie_reset1";
> +          drive-open-drain = <1>;

[Severity: Low]
Does assigning an integer value to drive-open-drain violate the core schema?

The core pincfg-node.yaml schema expects boolean properties to be present
without an assigned value, but the <1> assignment creates an integer array.

> +        };
> +      };

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260727074234.3761170-1-mikhail.kshevetskiy@iopsys.eu?part=32