All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Amber Kao" <amber.kao@ite.com.tw>
Cc: robh@kernel.org, devicetree@vger.kernel.org, conor+dt@kernel.org
Subject: Re: [PATCH 1/2] dt-bindings: usb: Add ITE IT885x support
Date: Mon, 15 Jun 2026 06:03:29 +0000	[thread overview]
Message-ID: <20260615060329.F18241F00A3A@smtp.kernel.org> (raw)
In-Reply-To: <20260615-ucsi-itepd-feature-v1-1-a826cfd0df6a@ite.com.tw>

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] The device tree binding defines a generic `gpios` property and redundantly maps it to the exact same hardware pin as the `interrupts` property, which is a structural anti-pattern.
- [Low] The device tree binding example uses a vendor-specific node name instead of a generic functional name.
--

commit db50e109009ce6583e099c466366e81f9dc8a5ee
Author: Amber Kao <amber.kao@ite.com.tw>

dt-bindings: usb: Add ITE IT885x support

Add device tree binding documentation for the ITE IT885x. The ITE IT885x is
an I2C-based USB Type-C Power Delivery (PD) controller.

> diff --git a/Documentation/devicetree/bindings/usb/ite,itepd-it885x.yaml b/Documentation/devicetree/bindings/usb/ite,itepd-it885x.yaml
> new file mode 100644
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/ite,itepd-it885x.yaml
[ ... ]
> +properties:
> +  compatible:
> +    const: ite,itepd-it885x
> +
> +  reg:
> +    maxItems: 1
> +
> +  gpios:
> +    maxItems: 1

[Severity: Low]
Does this property need a functional prefix? Device tree schemas generally
require GPIO consumers to have a prefix, such as irq-gpios, rather than
using the bare gpios property.

[ ... ]
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        itepd@40 {

[Severity: Low]
Should this node name use a generic functional name instead of a vendor
specific one? A generic name like typec@40 or usb-typec@40 would align
better with the devicetree specification recommendations.

> +            compatible = "ite,itepd-it885x";
> +            reg = <0x40>;
> +            gpios = <&tlmm 129 GPIO_ACTIVE_LOW>;
> +            interrupts-extended = <&tlmm 129 IRQ_TYPE_EDGE_FALLING>;

[Severity: Low]
Is it necessary to map the exact same physical pin to both gpios and
interrupts-extended here? If this is used merely to poll the interrupt line
state during probe, the hardware description shouldn't encode software
workarounds.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260615-ucsi-itepd-feature-v1-0-a826cfd0df6a@ite.com.tw?part=1

  reply	other threads:[~2026-06-15  6:03 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-15 13:47 [PATCH 0/2] Add support for ITE IT885x USB PD controller Amber Kao
2026-06-15 13:47 ` [PATCH 1/2] dt-bindings: usb: Add ITE IT885x support Amber Kao
2026-06-15  6:03   ` sashiko-bot [this message]
2026-06-15 16:41   ` Conor Dooley
2026-06-15 13:47 ` [PATCH 2/2] usb: typec: ucsi: Add ITE IT885x Type-C PD controller driver Amber Kao
2026-06-15  6:09   ` sashiko-bot
2026-06-18 10:25   ` Heikki Krogerus

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=20260615060329.F18241F00A3A@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=amber.kao@ite.com.tw \
    --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.