All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Esteban Urrutia" <esteuwu@proton.me>
Cc: conor+dt@kernel.org, robh@kernel.org, devicetree@vger.kernel.org
Subject: Re: [PATCH 1/2] dt-bindings: usb: Add Parade PS5169 redriver bindings
Date: Tue, 28 Jul 2026 01:34:03 +0000	[thread overview]
Message-ID: <20260728013403.BA71D1F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260727-ps5169-bup-v1-1-c76a8ef3b239@proton.me>

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The schema incorrectly uses `additionalProperties: false` at the top level, rejecting inherited properties from referenced schemas and breaking altmode support for the PS5170 variant.
- [Medium] The `ports` object lacks an `unevaluatedProperties: false` constraint, rendering the `if-then` restriction ineffective and silently allowing `port@2` on the PS5169 variant.
--

--- Patch [1]: [PATCH 1/2] dt-bindings: usb: Add Parade PS5169 redriver bindings ---
commit dc85f2d66c6d67f68f982e513db1687e5793eecf
Author: Esteban Urrutia <esteuwu@proton.me>

dt-bindings: usb: Add Parade PS5169 redriver bindings

Add bindings for the Parade PS5169 and PS5170 USB Type-C linear redrivers.
The difference between PS5169 and PS5170 is that PS5170 features an
embedded AUX switch, while PS5169 does not.
Future-proof the binding by adding PS5170 as well.

diff --git a/Documentation/devicetree/bindings/usb/parade,ps5169.yaml b/Documentation/devicetree/bindings/usb/parade,ps5169.yaml
new file mode 100644
index 0000000000000..23ca8e6c0491f
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/parade,ps5169.yaml
[ ... ]
> +  parade,usb-tx-set-gain:
> +    type: boolean
> +    description:
> +      If this property is specified a gain of -0.9 dB will be applied on the USB transmit channels.
> +
> +  ports:
> +    $ref: /schemas/graph.yaml#/properties/ports
> +    properties:

[Severity: Medium]
Should this ports definition include unevaluatedProperties: false?

Without it, any unlisted extra properties are permitted by default.

This means the if-then block later in the schema will successfully define
port@2 for PS5170, but it won't prevent port@2 from being silently accepted
for the base PS5169 device that lacks an AUX switch.

> +      port@0:
> +        $ref: /schemas/graph.yaml#/properties/port
> +        description: Super Speed (SS) Output endpoint to the Type-C connector
[ ... ]
> +allOf:
> +  - $ref: usb-switch.yaml#
> +  - $ref: usb-switch-ports.yaml#
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: parade,ps5170
> +    then:
> +      properties:
> +        ports:
> +          properties:
> +            port@2:
> +              $ref: /schemas/graph.yaml#/properties/port
> +              description:
> +                Sideband Use (SBU) AUX lines endpoint to the Type-C connector for the purpose of
> +                handling altmode muxing and orientation switching.
> +
> +additionalProperties: false

[Severity: Medium]
Does this need to be unevaluatedProperties: false instead of
additionalProperties: false?

In JSON schema, additionalProperties only considers the explicitly listed
local properties and ignores properties imported via $ref in allOf.

Because mode-switch (inherited from usb-switch.yaml) is omitted from the
local properties list, additionalProperties: false will reject it. This
might break validation for the PS5170 variant acting as an altmode switch.

> +
> +examples:
> +  - |
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260727-ps5169-bup-v1-0-c76a8ef3b239@proton.me?part=1

  reply	other threads:[~2026-07-28  1:34 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-28  1:19 [PATCH 0/2] Add support for Parade PS5169 USB Type-C linear redriver Esteban Urrutia via B4 Relay
2026-07-28  1:19 ` Esteban Urrutia
2026-07-28  1:19 ` [PATCH 1/2] dt-bindings: usb: Add Parade PS5169 redriver bindings Esteban Urrutia via B4 Relay
2026-07-28  1:19   ` Esteban Urrutia
2026-07-28  1:34   ` sashiko-bot [this message]
2026-07-28  1:19 ` [PATCH 2/2] usb: typec: Add support for Parade PS5169 USB Type-C redriver Esteban Urrutia via B4 Relay
2026-07-28  1:19   ` Esteban Urrutia
2026-07-28  1:36   ` sashiko-bot

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=20260728013403.BA71D1F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=esteuwu@proton.me \
    --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.