Re: [PATCH v2 1/3] dt-bindings: i3c: Add Qualcomm I3C master controller

[Mukesh Kumar Savaliya]

Thanks Krzysztof for immediate review !

On 3/26/2025 7:53 PM, Krzysztof Kozlowski wrote:
> On 26/03/2025 15:16, Mukesh Kumar Savaliya wrote:
> > Add device tree bindings for the Qualcomm I3C controller. This includes
> > the necessary documentation and properties required to describe the
> > hardware in the device tree.
> >
> > Signed-off-by: Mukesh Kumar Savaliya <quic_msavaliy@xxxxxxxxxxx>
> > ---
> >   .../bindings/i3c/qcom,i3c-master.yaml         | 60 +++++++++++++++++++
> >   1 file changed, 60 insertions(+)
> >   create mode 100644 Documentation/devicetree/bindings/i3c/qcom,i3c-master.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/i3c/qcom,i3c-master.yaml b/Documentation/devicetree/bindings/i3c/qcom,i3c-master.yaml
> > new file mode 100644
> > index 000000000000..af6b393f2327
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/i3c/qcom,i3c-master.yaml
>
> Naming: nothing improved.
Sorry, i didn't get exactly. To your comment "Filename matching 
compatible." i have Changed the compatible string to "qcom,i3c-master" 
and now it's matching to file name.

shall i make filename as qcom,i3c-master-qcom.yaml ? If this is wrong, 
please suggest.

Removed "bindings" from the subject line too. I have removed "master" 
from the description, shall i remove from subject also and keep only 
controller ? I kept master controller thinking it may be good as "master 
controller" instead "controller".


> > @@ -0,0 +1,60 @@
> > +# SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/i3c/qcom,i3c-master.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Qualcomm Geni based QUP I3C Controller
> > +
> > +maintainers:
> > +  - Mukesh Kumar Savaliya <quic_msavaliy@xxxxxxxxxxx>
> > +
> > +description:
> > +  I3C in master mode supports up to 12.5MHz, SDR mode data transfer in mixed
> > +  bus mode (I2C and I3C target devices on same i3c bus). It also supports
> > +  hotjoin, IBI mechanism.
> > +
> > +  I3C Controller nodes must be child of GENI based Qualcomm Universal
> > +  Peripharal. Please refer GENI based QUP wrapper controller node bindings
> > +  described in Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml.
> > +
> > +allOf:
> > +  - $ref: i3c.yaml#
> > +
> > +properties:
> > +  compatible:
> > +    const: qcom,i3c-master
>
> And this got worse. It makes also no sense either: how can you claim
> that this covers all possible future I3C masters from Qualcomm?
>
> What was the resolution of previous discussion?
Below was my understanding and reply.
"
I think i should remove const. kept it for now as no other compatible to 
be added as of now.
let me remove const.
SoC name is not required, as this compatible is generic to all the SOCs.
"

But i realized i missed to remove const, sorry for that.


I have also checked for qcom,spi-geni-qcom.yaml, it has below :
properties:
  compatible:
    const: qcom,geni-spi

Let me know if i can correct for SPI in separate new patch ?

> Best regards,
> Krzysztof