| Message ID | 20250326141641.3471906-2-quic_msavaliy@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add Qualcomm i3c master controller driver support | expand |
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@quicinc.com> >> --- >> .../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@quicinc.com> >> + >> +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 >
On 28/03/2025 15:28, Mukesh Kumar Savaliya wrote: > 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@quicinc.com> >>> --- >>> .../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". I still see the "master" in the compatible and filename. > > >>> @@ -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@quicinc.com> >>> + >>> +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. I don't see any talks about const, what are you referring to? > " > > 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 ? I love such arguments - let's find whatever old bindings and whatever antipattern. What was the outcome of the discussion? I think was about geni, no? What did Rob write? So how did it became i3c-master? Best regards, Krzysztof
On 3/28/2025 8:03 PM, Krzysztof Kozlowski wrote: > On 28/03/2025 15:28, Mukesh Kumar Savaliya wrote: >> 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@quicinc.com> >>>> --- >>>> .../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". > > I still see the "master" in the compatible and filename. > File Name : qcom,i3c-geni-qcom.yaml Compatible name : "qcom,i3c-geni" + Remove master from subject too "dt-bindings: i3c: Add Qualcomm I3C controller" will this be fine ? >> >> >>>> @@ -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@quicinc.com> >>>> + >>>> +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. > > I don't see any talks about const, what are you referring to? > +properties: + compatible: + : qcom,i3c-geni will this be fine ? >> " >> >> 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 ? > > I love such arguments - let's find whatever old bindings and whatever > antipattern. Sure, once i know what should be the ideal pattern which is unique/should be unique, will share you other files. > > What was the outcome of the discussion? I think was about geni, no? What > did Rob write? So how did it became i3c-master? > yes, so "qcom,i3c-geni" is fine ? > Best regards, > Krzysztof
On 29/03/2025 10:08, Mukesh Kumar Savaliya wrote: >>>>> + 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. >> >> I don't see any talks about const, what are you referring to? >> > +properties: > + compatible: > + : qcom,i3c-geni > will this be fine ? Yes, I think that was also suggested by Rob. Or rather follow existing style so qcom,geni-i3c for the compatible and filename. Best regards, Krzysztof
On 3/29/2025 4:36 PM, Krzysztof Kozlowski wrote: > On 29/03/2025 10:08, Mukesh Kumar Savaliya wrote: >>>>>> + 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. >>> >>> I don't see any talks about const, what are you referring to? >>> >> +properties: >> + compatible: >> + : qcom,i3c-geni >> will this be fine ? > > Yes, I think that was also suggested by Rob. Or rather follow existing > style so qcom,geni-i3c for the compatible and filename. > Sure, makes sense. With below i3c becomes similar in naming conventions to spi, i2c, serial uart. Driver file name : i3c-qcom-geni.c dt-binding file name: qcom,i3c-geni-qcom.yaml compatible = "qcom,geni-i3c"; > > Best regards, > Krzysztof
On 29/03/2025 15:31, Mukesh Kumar Savaliya wrote: > dt-binding file name: qcom,i3c-geni-qcom.yaml No. I already asked to use compatible as filename. Don't create your own rules. > compatible = "qcom,geni-i3c"; >> >> Best regards, >> Krzysztof > Best regards, Krzysztof
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 @@ -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@quicinc.com> + +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 + + reg: + maxItems: 1 + + clocks: + maxItems: 1 + + interrupts: + maxItems: 1 + +required: + - compatible + - reg + - clocks + - clock-names + - interrupts + +unevaluatedProperties: false + +examples: + - | + #include <dt-bindings/interrupt-controller/arm-gic.h> + #include <dt-bindings/clock/qcom,sm8550-gcc.h> + + i3c@884000 { + compatible = "qcom,i3c-master"; + reg = <0x00884000 0x4000>; + clocks = <&gcc GCC_QUPV3_WRAP2_S1_CLK>; + clock-names = "se-clk"; + interrupts = <&intc GIC_SPI 583 IRQ_TYPE_LEVEL_HIGH>; + #address-cells = <3>; + #size-cells = <0>; + }; +...
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@quicinc.com> --- .../bindings/i3c/qcom,i3c-master.yaml | 60 +++++++++++++++++++ 1 file changed, 60 insertions(+) create mode 100644 Documentation/devicetree/bindings/i3c/qcom,i3c-master.yaml