linux-kernel - Re: [EXT] Re: [PATCH v2 1/2] dt-bindings: i3c: Add NXP P3H2x4x i3c-hub support

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: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <aC9xFMKG88OOH2xV@lizhi-Precision-Tower-5810>
Date: Thu, 22 May 2025 14:46:44 -0400
From: Frank Li <Frank.li@....com>
To: Krzysztof Kozlowski <krzk@...nel.org>
Cc: Aman Kumar Pandey <aman.kumarpandey@....com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-i3c@...ts.infradead.org" <linux-i3c@...ts.infradead.org>,
	"alexandre.belloni@...tlin.com" <alexandre.belloni@...tlin.com>,
	"krzk+dt@...nel.org" <krzk+dt@...nel.org>,
	"robh@...nel.org" <robh@...nel.org>,
	"conor+dt@...nel.org" <conor+dt@...nel.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	Vikash Bansal <vikash.bansal@....com>,
	Priyanka Jain <priyanka.jain@....com>,
	Shashank Rebbapragada <shashank.rebbapragada@....com>
Subject: Re: [EXT] Re: [PATCH v2 1/2] dt-bindings: i3c: Add NXP P3H2x4x
 i3c-hub support

On Wed, May 14, 2025 at 12:10:11PM +0200, Krzysztof Kozlowski wrote:
> On 12/05/2025 13:45, Aman Kumar Pandey wrote:
> >>>
> >>> Signed-off-by: Aman Kumar Pandey <aman.kumarpandey@....com>
> >>> Signed-off-by: Vikash Bansal <vikash.bansal@....com>
> >>> ---
> >>> V1 -> V2: Cleaned up coding style and addressed review comments
> >>> ---
> >>>  .../bindings/i3c/p3h2840-i3c-hub.yaml         | 332 ++++++++++++++++++
> >>
> >> Not much improved. I have doubts that you really looked at other bindings.
> >>
> >> Filename matching compatible.
> >>
> >>
> >
> > Thanks for reviewing the patch.
> > There is no existing entry like this one. It is the first device tree entry for any i3c target device being added to Linux upstream.
> > For I3C target device, compatible is not required as per i3c.yml. I am adding compatible for backward compatibility with i2c binding ( i3c target device have backward compatibility).
> > Should I use compatible matching filename in this case ?
>
> Do you have compatible here? Yes, you have, so I do not get what is
> unclear in my comment. Rename the binding to match the compatible, which
> you have here.
>
>
> >
> >>>  MAINTAINERS                                   |   8 +
> >>>  2 files changed, 340 insertions(+)
> >
> >
> >>> +
> >>> +properties:
> >>> +  $nodename:
> >>> +    pattern: "^hub@[@.*]+,[0-9a-f]+$"
> >>
> >> Drop
> >>
> >
> > Sorry, did not understand your point. Could you please give some clarification ?
>
> Drop this nodename. Remove it.
>
> >
> >>> +
> >
> >>> +
> >>> +  cp0-ldo-enable:
> >>> +    type: boolean
> >>> +    description:
> >>> +      Enables the on-die LDO for controller Port 0.
> >>> +
> >>> +  cp1-ldo-enable:
> >>> +    type: boolean
> >>> +    description:
> >>> +      Enables the on-die LDO for controller Port 1.
> >>> +
> >>> +  tp0145-ldo-enable:
> >>> +    type: boolean
> >>> +    description:
> >>> +      Enables the on-die LDO for target ports 0/1/4/5.
> >>> +
> >>> +  tp2367-ldo-enable:
> >>> +    type: boolean
> >>> +    description:
> >>> +      Enables the on-die LDO for target ports 2/3/6/7.
> >>> +
> >>
> >> NAK for all above properties.
> >>
> >
> > These are not vender specific properties. all these properties are as per i3c hub specification.
> >  What should I change for above properties ?
>
>
> What do they represent? Regulators? Why would you add custom properties
> for controlling regulators? Why would you enable LDOs for ports which
> have nothing connected? Why would you not enable LDOs for ports which
> have something there? Why these are not properties of the ports?
>

power supply cp[0,1] has two options: one from internal ldo, the other from the
other regulator.

So I think it should be *-supply

cp0-supply:
  description: phandle to cp0 power supply.

because internal ldo can provide difference voltage, I suggest export as
standard regulator

regulators:
  properties:
    cp0:
      type: object
      $ref: /schemas/regulator/regulator.yaml#
      unevaluatedProperties: false

Frank


> So many questions.
>
>
> Best regards,
> Krzysztof