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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The `function` property enum restricts the allowed mux functions to only four UART-related values (`U2TX`, `U2RX`, `U4TX`, `U4RX`), breaking validation for all other hardware peripherals.
- [High] The `function` property is missing from the top-level pin configuration node's properties, preventing its use on direct, single-level configuration nodes.
- [High] The regular expressions used in `patternProperties` enforce non-standard, brittle naming conventions and will incorrectly reject valid device tree node names.
--

commit bc456fcb7e299ea733020d3953dfb0867baf36ff
Author: Udaya Kiran Challa <challauday369@gmail.com>

dt-bindings: pinctrl: microchip,pic32mzda-pinctrl: Convert to DT schema
    
Convert Microchip PIC32 Pin Controller devicetree binding
from legacy text format to DT schema.

> diff --git a/Documentation/devicetree/bindings/pinctrl/microchip,pic32mzda-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/microchip,pic32mzda-pinctrl.yaml
> new file mode 100644
> index 0000000000000..87ac5aace5e36
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pinctrl/microchip,pic32mzda-pinctrl.yaml
> @@ -0,0 +1,141 @@
[ ... ]
> +patternProperties:
> +  '^[a-z0-9]+[_-].*$':

[Severity: High]
Does this regular expression inappropriately reject valid and standard
device tree node names? 

By requiring an underscore or hyphen, this outer regex prevents generic
single-word node names like "spi0" for a pin group container.

> +    type: object
> +    description: |
> +      Pin configuration node with pin muxing and configuration properties.
> +      Can be either a direct pin configuration node or a container node
> +      with child pin configuration nodes.
> +
> +    properties:
> +      pins:
> +        description: |
> +          List of pins to which the configuration applies.
> +        items:
> +          type: string
> +        minItems: 1
> +
> +      bias-pull-up: true
> +      bias-pull-down: true
> +      input-enable: true
> +      output-low: true
> +      output-high: true

[Severity: High]
Is the "function" property missing from this properties block? 

Because the patternProperties node defines additionalProperties: false
later on, omitting "function" here forces all users to use a nested
container structure. This seems to contradict the description which states
the node can be a direct pin configuration node.

> +
> +      microchip,digital:
> +        description: Enable digital I/O.
> +        type: boolean
> +
> +      microchip,analog:
> +        description: Enable analog I/O.
> +        type: boolean
> +
> +    patternProperties:
> +      '^[a-z]+[0-9]+-[a-z]+$':

[Severity: High]
Could this inner regular expression impose a brittle naming structure? 

Because it requires a strict "<alpha><num>-<alpha>" pattern, it will
reject valid node names like "i2c-sda" (no number) or "tx" (no hyphen).

> +        type: object
> +        description: |
> +          Child pin configuration node.
> +
> +        properties:
> +          pins:
> +            items:
> +              type: string
> +            minItems: 1
> +
> +          function:
> +            description: |
> +              A string containing the name of the function to mux pin.
> +            enum: [ U2TX, U2RX, U4TX, U4RX ]

[Severity: High]
Is it intentional to restrict the allowed mux functions to only these
four UART-related values? 

This enum arbitrarily limits the allowed strings and will break
validation for other hardware peripherals like IC3, SDI1, and OC3 which
are supported by the driver.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260705132521.159522-1-challauday369@gmail.com?part=1