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

[Aman Kumar Pandey]

> -----Original Message-----
> From: Krzysztof Kozlowski <krzk at kernel.org>
> Sent: Thursday, May 8, 2025 11:38 AM
> To: Aman Kumar Pandey <aman.kumarpandey at nxp.com>; linux-
> kernel at vger.kernel.org; linux-i3c at lists.infradead.org;
> alexandre.belloni at bootlin.com; krzk+dt at kernel.org; robh at kernel.org;
> conor+dt at kernel.org; devicetree at vger.kernel.org
> Cc: Vikash Bansal <vikash.bansal at nxp.com>; Priyanka Jain
> <priyanka.jain at nxp.com>; Shashank Rebbapragada
> <shashank.rebbapragada at nxp.com>; Frank Li <frank.li at nxp.com>
> Subject: [EXT] Re: [PATCH v2 1/2] dt-bindings: i3c: Add NXP P3H2x4x i3c-hub
> support
> 
> Caution: This is an external email. Please take care when clicking links or
> opening attachments. When in doubt, report the message using the 'Report
> this email' button
> 
> 
> On 08/05/2025 06:57, 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 at nxp.com>
> > Signed-off-by: Vikash Bansal <vikash.bansal at nxp.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 ?

> >  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 ?

> > +

> > +
> > +  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 ?

> > +  cp0-ldo-microvolt:
> 
> Do you see anywhere device properties named like that? Without prefix?
> 
> I am not going to review the rest. You did not try hard enough to fulfill
> previous review request, you did not really test it.
> 
> Start from scratch from latest accepted schema or from example-schema.
> 


> Best regards,
> Krzysztof