Re: [PATCH v4] dt-bindings: pinctrl: microchip,pic32mzda-pinctrl: Convert to DT schema

[Rob Herring <robh@kernel.org>]

On Wed, Jul 22, 2026 at 12:44:14PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> - [High] The top-level `additionalProperties` logic using `oneOf` silently accepts undefined non-object properties and breaks validation for standard container nodes due to injected `phandle` properties.
> - [Medium] The `$defs/pin-node` definition bypasses all type and constraint checking for standard pin configuration properties because it fails to reference `pincfg-node.yaml`.
> - [Low] The removal of `#address-cells` and `#size-cells` from the schema creates a mismatch with the existing `pic32mzda.dtsi`, which will cause `dtbs_check` warnings.
> --
> 
> --- Patch [1]: [PATCH v4] dt-bindings: pinctrl: microchip,pic32mzda-pinctrl: Convert to DT schema ---
> commit 238b9bfa23edd44c05fbb35e962f0ab9f0bb9f25
> Author: Udaya Kiran Challa <challauday369@gmail.com>
> Date:   Wed Jul 22 12:32:43 2026 +0000
> 
> dt-bindings: pinctrl: microchip,pic32mzda-pinctrl: Convert to DT schema
>     
> This commit converts the Microchip PIC32 Pin Controller devicetree binding
> from legacy text format to DT schema. It consolidates pin-node properties and
> updates the schema layout to support nested or direct pin configurations.
> 
> Link to v3: https://lore.kernel.org/all/20260719183431.124061-1-challauday369@gmail.com/
> Link to v2: https://lore.kernel.org/all/20260707144223.120417-1-challauday369@gmail.com/
> Link to v1: https://lore.kernel.org/all/20260705132521.159522-1-challauday369@gmail.com/
> 
> > diff --git a/Documentation/devicetree/bindings/pinctrl/microchip,pic32mzda-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/microchip,pic32mzda-pinctrl.yaml
> > new file mode 100644
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pinctrl/microchip,pic32mzda-pinctrl.yaml
> > @@ -0,0 +1,107 @@
> [ ... ]
> > +properties:
> > +  compatible:
> > +    const: microchip,pic32mzda-pinctrl
> > +
> > +  reg:
> > +    maxItems: 1
> 
> [Severity: Low]
> Even though the changelog mentions removing #address-cells and #size-cells
> from the schema, aren't these properties still present in the
> pinctrl@1f801400 node in arch/mips/boot/dts/pic32/pic32mzda.dtsi? 
> 
> Will their removal from the schema cause dtbs_check warnings for the
> existing in-tree device tree?

Yes, but warning is correct.

> 
> > +
> > +  clocks:
> > +    maxItems: 1
> > +
> > +$defs:
> > +  pin-node:
> > +    type: object
> > +    $ref: /schemas/pinctrl/pinmux-node.yaml#
> > +
> > +    properties:
> > +      pins:
> > +        description: List of pins to which the configuration applies.
> > +        items:
> > +          type: string
> > +        minItems: 1
> > +
> > +      function: true
> > +
> > +      bias-pull-up: true
> > +      bias-pull-down: true
> > +      input-enable: true
> > +      output-low: true
> > +      output-high: true
> 
> [Severity: Medium]
> Does this bypass type and constraint checking for the generic pin
> configuration properties?
> 
> By assigning these generic properties to true without also including a
> $ref: /schemas/pinctrl/pincfg-node.yaml#, it looks like the schema
> validator might silently accept any value or type for them, missing
> mutually exclusive constraints like having both output-low and
> output-high present.

This should be addressed.

> 
> > +
> > +      microchip,digital:
> > +        description: Enable digital I/O.
> > +        type: boolean
> > +
> [ ... ]
> > +    required:
> > +      - pins
> > +
> > +    unevaluatedProperties: false
> > +
> > +additionalProperties:
> > +  oneOf:
> > +    - additionalProperties:
> > +        $ref: '#/$defs/pin-node'
> > +    - $ref: '#/$defs/pin-node'
> 
> [Severity: High]
> Will this oneOf construct silently accept undefined scalar properties?
> 
> Because the first branch lacks a type: object constraint, a typoed
> scalar property might trivially pass the first branch and satisfy the
> oneOf.

The $ref has a 'type: object' so we should be fine.

> 
> Also, for valid child container nodes referenced via a phandle, the
> devicetree compiler automatically injects a phandle property. Since
> that injected phandle is not an object, won't it fail the $ref type
> check here and cause spurious dtbs_check errors for perfectly valid
> hardware configurations?

I think the $defs should get fixed up with 'phandle', etc. added. You 
can check the generated processed-schemas.yaml.

Rob