lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20260114-wonderful-vermilion-starfish-ad6ec9@quoll>
Date: Wed, 14 Jan 2026 09:23:22 +0100
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Aman Kumar Pandey <aman.kumarpandey@....com>
Cc: linux-kernel@...r.kernel.org, linux-i3c@...ts.infradead.org, 
	alexandre.belloni@...tlin.com, krzk+dt@...nel.org, robh@...nel.org, conor+dt@...nel.org, 
	devicetree@...r.kernel.org, broonie@...nel.org, lee@...nel.org, Frank.Li@....com, 
	lgirdwood@...il.com, vikash.bansal@....com, priyanka.jain@....com, 
	shashank.rebbapragada@....com
Subject: Re: [PATCH v4 2/5] dt-bindings: i3c: Add NXP P3H2x4x i3c-hub support

On Tue, Jan 13, 2026 at 01:45:26PM +0200, Aman Kumar Pandey wrote:
> Add bindings for the NXP P3H2x4x (P3H2440/P3H2441/P3H2840/P3H2841)
> multiport I3C hub family. These devices connect to a host via
> I3C/I2C/SMBus and allow communication with multiple downstream
> peripherals.
> 
> Signed-off-by: Aman Kumar Pandey <aman.kumarpandey@....com>
> Signed-off-by: Vikash Bansal <vikash.bansal@....com>
> 
> ---
> Changes in v4:
>  - Fixed DT binding check warning
>  - Removed SW properties: ibi-enable, local-dev, and always-enable
> 
> Changes in v3:
>  - Added MFD (Multi-Function Device) support for I3C hub and on-die regulator
>  - Added Regulator supply node
> 
> Changes in v2:
>  - Fixed DT binding check warning
>  - Revised logic for parsing DTS nodes
> ---
> ---
>  .../bindings/mfd/nxp,p3h2840-i3c-hub.yaml     | 240 ++++++++++++++++++
>  MAINTAINERS                                   |   8 +
>  2 files changed, 248 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/nxp,p3h2840-i3c-hub.yaml
> 
> diff --git a/Documentation/devicetree/bindings/mfd/nxp,p3h2840-i3c-hub.yaml b/Documentation/devicetree/bindings/mfd/nxp,p3h2840-i3c-hub.yaml
> new file mode 100644
> index 000000000000..b6853a1d1893
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/nxp,p3h2840-i3c-hub.yaml

Same comment as before, filename matching compatible.

> @@ -0,0 +1,240 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# Copyright 2025 NXP
> +

Drop blank line.

> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mfd/nxp,p3h2840-i3c-hub.yaml#

This is not a MFD device. Place it in appropriate place, e.g. i2c or i3c
controllers directory.

> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: NXP P3H2X4X I3C HUB
> +
> +maintainers:
> +  - Aman Kumar Pandey <aman.kumarpandey@....com>
> +  - Vikash Bansal <vikash.bansal@....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.
> +
> +allOf:
> +  - $ref: /schemas/i3c/i3c.yaml#
> +
> +properties:
> +  compatible:
> +    items:

Drop items, not needed, and use directly const.

> +      - const: nxp,p3h2840
> +
> +  cp0-ldo-microvolt:
> +    description:
> +      Selects the voltage for controller Port 0, in microvolts.

This is confusing. Why do you have voltage choice of cp0 here and also
in cp0 regulator? Only regulator node is needed.

> +    enum: [1000000, 1100000, 1200000, 1800000]
> +    default: 1000000
> +
> +  cp1-ldo-microvolt:
> +    description:
> +      Selects the voltage for controller Port 1, in microvolts.
> +    enum: [1000000, 1100000, 1200000, 1800000]
> +    default: 1000000
> +
> +  tp0145-ldo-microvolt:
> +    description:
> +      Selects the voltage for target Port 0/1/4/5, in microvolts.
> +    enum: [1000000, 1100000, 1200000, 1800000]
> +    default: 1000000
> +
> +  tp2367-ldo-microvolt:
> +    description:
> +      Selects the voltage for target Port 2/3/6/7, in microvolts.
> +    enum: [1000000, 1100000, 1200000, 1800000]
> +    default: 1000000
> +
> +  tp0145-pullup-ohms:
> +    description:
> +      Selects the pull up resistance for target Port 0/1/4/5, in ohms.
> +    enum: [250, 500, 1000, 2000]
> +    default: 500
> +
> +  tp2367-pullup-ohms:
> +    description:
> +      Selects the pull up resistance for target Port 2/3/6/7, in ohms.
> +    enum: [250, 500, 1000, 2000]
> +    default: 500
> +
> +  cp0-io-strength-ohms:
> +    description:
> +      Selects the IO drive strength for controller Port 0, in ohms.
> +    enum: [20, 30, 40, 50]
> +    default: 20
> +
> +  cp1-io-strength-ohms:
> +    description:
> +      Selects the IO drive strength for controller Port 1, in ohms.
> +    enum: [20, 30, 40, 50]
> +    default: 20
> +
> +  tp0145-io-strength-ohms:
> +    description:
> +      Selects the IO drive strength for target port 0/1/4/5, in ohms.
> +    enum: [20, 30, 40, 50]
> +    default: 20
> +
> +  tp2367-io-strength-ohms:
> +    description:
> +      Selects the IO drive strength for target port 2/3/6/7, in ohms.
> +    enum: [20, 30, 40, 50]
> +    default: 20
> +
> +  cp0-supply:
> +    description: Phandle to voltage regulator providing power to controller port 0.

s/handle to voltage regulator providing power// because it is completely
redundant. Just say port 0 power supply.

> +
> +  cp1-supply:
> +    description: Phandle to voltage regulator providing power to controller port 1.
> +
> +  tp0145-supply:
> +    description: Phandle to voltage regulator providing power to target port 0/1/4/5.
> +
> +  tp2367-supply:
> +    description: Phandle to voltage regulator providing power to target port 2/3/6/7.
> +
> +  regulators:
> +    type: object
> +    additionalProperties: false
> +
> +    properties:
> +      cp0:
> +        type: object
> +        $ref: /schemas/regulator/regulator.yaml#
> +        unevaluatedProperties: false
> +
> +      cp1:
> +        type: object
> +        $ref: /schemas/regulator/regulator.yaml#
> +        unevaluatedProperties: false
> +
> +      tp0145:
> +        type: object
> +        $ref: /schemas/regulator/regulator.yaml#
> +        unevaluatedProperties: false
> +
> +      tp2367:
> +        type: object
> +        $ref: /schemas/regulator/regulator.yaml#
> +        unevaluatedProperties: false
> +
> +patternProperties:
> +  "^i3c@[0-7]$":
> +    type: object
> +    $ref: /schemas/i3c/i3c.yaml#
> +    unevaluatedProperties: false
> +
> +    properties:
> +      reg:
> +        description:
> +          The I3C HUB Target Port number.
> +        maxItems: 1
> +
> +      pullup-enable:
> +        type: boolean
> +        description:
> +          Enables the on-die pull-up for Target Port.
> +
> +  "^(i2c|smbus)@[0-7]$":
> +    type: object
> +    $ref: /schemas/i2c/i2c-controller.yaml#
> +    unevaluatedProperties: false
> +
> +    properties:
> +      reg:
> +        description:
> +          The I3C HUB Target Port number.
> +        maxItems: 1
> +
> +      pullup-enable:
> +        type: boolean
> +        description:
> +          Enables the on-die pull-up for Target Port.
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    i3c {
> +        #address-cells = <3>;
> +        #size-cells = <0>;
> +
> +        hub@70,236153000c2 {
> +            reg = <0x70 0x236 0x3000c2>;
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +            assigned-address = <0x50>;
> +
> +            cp0-ldo-microvolt = <1800000>;
> +            cp1-ldo-microvolt = <1800000>;
> +            tp0145-ldo-microvolt = <1800000>;
> +            tp2367-ldo-microvolt = <1800000>;
> +            tp0145-pullup-ohm = <1000>;
> +            tp2367-pullup-ohm = <1000>;
> +            cp0-io-strength-ohm = <50>;
> +            cp1-io-strength-ohm = <50>;
> +            tp0145-io-strength-ohm = <50>;
> +            tp2367-io-strength-ohm = <50>;
> +            cp0-supply = <&cp0>;

This makes no sense ether. How supply of cp0 regulator can be cp0?


> +            tp0145-supply = <&tp0145>;
> +            tp2367-supply = <&tp2367>;
> +
> +            regulators {
> +                cp0 {
> +                    regulator-name = "cp0";
> +                    regulator-min-microvolt = <1000000>;
> +                    regulator-max-microvolt = <1800000>;
> +                };

Best regards,
Krzysztof


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ