Linux-i3c Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Lakshay Piplani" <lakshay.piplani@nxp.com>
Cc: robh@kernel.org, conor+dt@kernel.org, devicetree@vger.kernel.org,
	linux-i3c@lists.infradead.org, Frank.Li@kernel.org
Subject: Re: [PATCH v12 2/7] dt-bindings: i3c: Add NXP P3H2x4x i3c-hub support
Date: Wed, 17 Jun 2026 11:13:31 +0000	[thread overview]
Message-ID: <20260617111331.7AB671F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260617110355.1591844-3-lakshay.piplani@nxp.com>

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] The binding documents GPIO capability for target ports but omits the standard GPIO provider properties, violating subsystem guidelines.
- [Medium] The binding fails to include compatible strings for the 4-port variants and unconditionally allows up to 8 ports.
- [Low] The unit address in the I3C example does not match its `reg` property.
--

commit af3b77595fece3e15864b37b9b683be835a0e1eb
Author: Aman Kumar Pandey <aman.kumarpandey@nxp.com>

dt-bindings: i3c: Add NXP P3H2x4x i3c-hub support

This commit adds device tree bindings for the NXP P3H2x4x multi-port I3C hub
family. It supports connecting a host via I3C/I2C/SMBus to multiple downstream
peripherals across up to 8 ports.

> diff --git a/Documentation/devicetree/bindings/i3c/nxp,p3h2840.yaml b/Documentation/devicetree/bindings/i3c/nxp,p3h2840.yaml
> new file mode 100644
> index 0000000000000..c080eeb0eeaa8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i3c/nxp,p3h2840.yaml
> @@ -0,0 +1,291 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# Copyright 2025 NXP
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/i3c/nxp,p3h2840.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: NXP P3H2X4X I3C HUB
> +
> +maintainers:
> +  - Aman Kumar Pandey <aman.kumarpandey@nxp.com>
> +  - Vikash Bansal <vikash.bansal@nxp.com>
> +  - Lakshay Piplani <lakshay.piplani@nxp.com>
> +
> +description: |
> +  P3H2x4x (P3H2440/P3H2441/P3H2840/P3H2841) is a family of multiport I3C
> +  hub devices that connect to:-
> +  1. A host CPU via I3C/I2C/SMBus bus on upstream side and connect to multiple
> +     peripheral devices on the downstream  side.
> +  2. Have two Controller Ports which can support either
> +     I2C/SMBus or I3C buses and connect to a CPU, BMC or SOC.
> +  3. P3H2840/ P3H2841 are 8 port I3C hub with eight I3C/I2C Target Port.
> +  4. P3H2440/ P3H2441 are 4 port I3C hub with four I3C/I2C Target Port.
> +     Target ports can be configured as I2C/SMBus, I3C or GPIO and connect to
> +     peripherals.

[Severity: Medium]
Since the description advertises that target ports can be configured as GPIO,
should this binding include the standard GPIO provider properties like
gpio-controller and #gpio-cells?

Because unevaluatedProperties is false and there are no child node patterns
defined for GPIO ports in patternProperties, won't adding a GPIO child node
fail schema validation?

> +
> +properties:
> +  compatible:
> +    const: nxp,p3h2840

[Severity: Medium]
The commit message and description state the binding supports the P3H2440
and P3H2441 4-port hubs alongside the 8-port P3H2840/P3H2841.

Should compatible strings be included for the 4-port variants here as well?

[ ... ]

> +patternProperties:
> +  "^i3c@[0-7]$":
> +    type: object
> +    $ref: /schemas/i3c/i3c.yaml#
> +    unevaluatedProperties: false
> +
> +    properties:
> +      reg:
> +        description:
> +          The I3C HUB Target Port number.
> +        maximum: 7

[Severity: Medium]
Does unconditionally allowing a maximum port index of 7 cause issues for the
4-port variants mentioned in the description?

It seems this allows invalid configurations that define non-existent ports
(4-7) for the 4-port hardware.

[ ... ]

> +examples:
> +  - |
> +    i3c {
> +        #address-cells = <3>;
> +        #size-cells = <0>;
> +
> +        hub@70,236153000c2 {
> +            reg = <0x70 0x236 0x3000c2>;

[Severity: Low]
Does the unit address in this example match its reg property?

The reg property combines to a provisioned ID of 0x236003000c2, which appears
to differ from 236153000c2 in the unit address. Could this mismatch trigger a
simple_bus_reg warning during make dt_binding_check?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260617110355.1591844-1-lakshay.piplani@nxp.com?part=2

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

  reply	other threads:[~2026-06-17 11:13 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-17 11:03 [PATCH v12 0/7] Add support for NXP P3H2x4x I3C hub driver Lakshay Piplani
2026-06-17 11:03 ` [PATCH v12 1/7] i3c: master: Add APIs for I3C hub support Lakshay Piplani
2026-06-17 11:20   ` sashiko-bot
2026-06-17 11:03 ` [PATCH v12 2/7] dt-bindings: i3c: Add NXP P3H2x4x i3c-hub support Lakshay Piplani
2026-06-17 11:13   ` sashiko-bot [this message]
2026-06-17 11:03 ` [PATCH v12 3/7] mfd: p3h2x4x: Add driver for NXP P3H2x4x i3c hub and on-die regulator Lakshay Piplani
2026-06-17 11:24   ` sashiko-bot
2026-06-17 11:03 ` [PATCH v12 4/7] regulator: p3h2x4x: Add driver for on-die regulators in NXP P3H2x4x i3c hub Lakshay Piplani
2026-06-17 11:17   ` sashiko-bot
2026-06-17 11:03 ` [PATCH v12 5/7] i3c: hub: Add support for the I3C interface in the I3C hub Lakshay Piplani
2026-06-17 11:18   ` sashiko-bot
2026-06-17 11:03 ` [PATCH v12 6/7] i3c: hub: p3h2x4x: Add support for NXP P3H2x4x I3C hub functionality Lakshay Piplani
2026-06-17 11:18   ` sashiko-bot
2026-06-17 11:03 ` [PATCH v12 7/7] i3c: hub: p3h2x4x: Add SMBus slave mode support Lakshay Piplani
2026-06-17 11:24   ` 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=20260617111331.7AB671F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=Frank.Li@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=lakshay.piplani@nxp.com \
    --cc=linux-i3c@lists.infradead.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