Devicetree
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Udaya Kiran Challa" <challauday369@gmail.com>
Cc: devicetree@vger.kernel.org, robh@kernel.org, conor+dt@kernel.org
Subject: Re: [PATCH v4] dt-bindings: pinctrl: microchip,pic32mzda-pinctrl: Convert to DT schema
Date: Wed, 22 Jul 2026 12:44:14 +0000	[thread overview]
Message-ID: <20260722124415.63DD11F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260722123243.341346-1-challauday369@gmail.com>

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?

> +
> +  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.

> +
> +      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.

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?

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

      reply	other threads:[~2026-07-22 12:44 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-22 12:32 [PATCH v4] dt-bindings: pinctrl: microchip,pic32mzda-pinctrl: Convert to DT schema Udaya Kiran Challa
2026-07-22 12:44 ` sashiko-bot [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260722124415.63DD11F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=challauday369@gmail.com \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=robh@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox