All of lore.kernel.org
 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: 3+ 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]
2026-07-22 23:21   ` Rob Herring

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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.