Re: [PATCH v11 4/9] dt-bindings: i3c: Add NXP P3H2x4x i3c-hub support - sashiko-bot

From: sashiko-bot@kernel.org
To: "Lakshay Piplani" <lakshay.piplani@nxp.com>
Cc: Frank.Li@kernel.org, conor+dt@kernel.org,
	devicetree@vger.kernel.org, robh@kernel.org,
	linux-i3c@lists.infradead.org
Subject: Re: [PATCH v11 4/9] dt-bindings: i3c: Add NXP P3H2x4x i3c-hub support
Date: Fri, 12 Jun 2026 11:33:53 +0000	[thread overview]
Message-ID: <20260612113354.14AA31F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260612111816.3688240-5-lakshay.piplani@nxp.com>

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Missing GPIO provider properties and constraints in the DT binding schema, violating subsystem guidelines.
- [Medium] Missing compatible strings and port constraints for the documented 4-port hardware variants (P3H2440/P3H2441).
--

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

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

Adds bindings for the NXP P3H2x4x multiport I3C hub family. These devices
connect to a host via I3C/I2C/SMBus and allow communication with multiple
downstream peripherals.

> 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

[ ... ]

> +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]
If the target ports can be configured as GPIO, should the schema include the
corresponding provider properties (such as gpio-controller and #gpio-cells)?
Without these properties defined, device trees attempting to use the
documented GPIO capabilities will be strictly rejected due to
unevaluatedProperties: false at the top level.

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

[Severity: Medium]
The description mentions support for the P3H2440 and P3H2441 4-port hardware
variants, but the compatible property is restricted strictly to nxp,p3h2840.
Should the compatible strings for the 4-port variants also be included here?

[ ... ]

> +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]
The maximum port number is currently hardcoded to 7 for all variants. If the
4-port variants (P3H2440/P3H2441) are supported, wouldn't we need conditional
if-then constraints to restrict the reg maximum to 3 for the 4-port hubs?
Otherwise, invalid device trees defining non-existent ports on 4-port hubs
might pass validation.

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

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