diff mbox series

[1/5] dt-bindings: i2c: Add Qualcomm Geni based QUP i2c bindings

Message ID 20220402051206.6115-2-singh.kuldeep87k@gmail.com (mailing list archive)
State Not Applicable
Headers show
Series [1/5] dt-bindings: i2c: Add Qualcomm Geni based QUP i2c bindings | expand

Commit Message

Kuldeep Singh April 2, 2022, 5:12 a.m. UTC 
GENI(generic interface) based Qualcomm Universal Peripheral controller
can support multiple serial interfaces like spi,uart and i2c.

Unlike other i2c controllers, QUP i2c bindings are present in parent
schema. Move it out from parent to an individual binding and let parent
refer to child schema later on.

Please note, current schema isn't complete as it misses out few
properties and thus, add these missing properties along the process.

Signed-off-by: Kuldeep Singh <singh.kuldeep87k@gmail.com>
---
 .../bindings/i2c/qcom,i2c-geni-qcom.yaml      | 110 ++++++++++++++++++
 1 file changed, 110 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i2c/qcom,i2c-geni-qcom.yaml

Comments

Krzysztof Kozlowski April 2, 2022, 12:29 p.m. UTC | #1 
On 02/04/2022 07:12, Kuldeep Singh wrote:
> GENI(generic interface) based Qualcomm Universal Peripheral controller
> can support multiple serial interfaces like spi,uart and i2c.
> 
> Unlike other i2c controllers, QUP i2c bindings are present in parent
> schema. Move it out from parent to an individual binding and let parent
> refer to child schema later on.
> 
> Please note, current schema isn't complete as it misses out few
> properties and thus, add these missing properties along the process.
> 
> Signed-off-by: Kuldeep Singh <singh.kuldeep87k@gmail.com>
> ---
>  .../bindings/i2c/qcom,i2c-geni-qcom.yaml      | 110 ++++++++++++++++++
>  1 file changed, 110 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/i2c/qcom,i2c-geni-qcom.yaml
> 
> diff --git a/Documentation/devicetree/bindings/i2c/qcom,i2c-geni-qcom.yaml b/Documentation/devicetree/bindings/i2c/qcom,i2c-geni-qcom.yaml
> new file mode 100644
> index 000000000000..01a02e680ea3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i2c/qcom,i2c-geni-qcom.yaml
> @@ -0,0 +1,110 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/i2c/qcom,i2c-geni-qcom.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: Qualcomm Geni based QUP I2C Controller
> +
> +maintainers:
> +  - Andy Gross <agross@kernel.org>
> +  - Bjorn Andersson <bjorn.andersson@linaro.org>
> +
> +allOf:
> +  - $ref: /schemas/i2c/i2c-controller.yaml#
> +
> +properties:
> +  compatible:
> +    enum:
> +      - qcom,geni-i2c

Just const, no enum. There are no other flavors of this (unless you
think there are?).

> +
> +  clocks:
> +    maxItems: 1
> +
> +  clock-names:
> +    const: se
> +
> +  clock-frequency:
> +    description: Desired I2C bus clock frequency in Hz
> +    default: 100000
> +
> +  interconnects:
> +    maxItems: 3
> +
> +  interconnect-names:
> +    items:
> +      - const: qup-core
> +      - const: qup-config
> +      - const: qup-memory
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  power-domains:
> +    maxItems: 1
> +
> +  reg:
> +    maxItems: 1
> +
> +  required-opps:
> +    maxItems: 1

I have doubts this is correct property. Usually it is part of the
opp-table. I see sc7180 needs this, but I think it is a mistake. Do you
know how it is supposed to work?


Best regards,
Krzysztof
Krzysztof Kozlowski April 2, 2022, 6:24 p.m. UTC | #2 
On 02/04/2022 07:12, Kuldeep Singh wrote:
> GENI(generic interface) based Qualcomm Universal Peripheral controller
> can support multiple serial interfaces like spi,uart and i2c.
> 

Few more comments.

(...)

> +
> +  clock-frequency:
> +    description: Desired I2C bus clock frequency in Hz

Skip description, it's common for I2C controllers.

> +    default: 100000
> +
> +  interconnects:
> +    maxItems: 3
> +
> +  interconnect-names:
> +    items:
> +      - const: qup-core
> +      - const: qup-config
> +      - const: qup-memory
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  power-domains:
> +    maxItems: 1
> +
> +  reg:
> +    maxItems: 1
> +
> +  required-opps:
> +    maxItems: 1
> +
> +  dmas:
> +    maxItems: 2
> +
> +  dma-names:
> +    items:
> +      - const: tx
> +      - const: rx
> +
> +  pinctrl-0: true
> +  pinctrl-1: true
> +
> +  pinctrl-names:
> +    minItems: 1
> +    items:
> +      - const: default
> +      - const: sleep
> +
> +  "#address-cells":
> +    const: 1
> +
> +  "#size-cells":
> +    const: 0

These are not needed, they come from schema.

Best regards,
Krzysztof
Kuldeep Singh April 2, 2022, 7:34 p.m. UTC | #3 
On Sat, Apr 02, 2022 at 08:24:25PM +0200, Krzysztof Kozlowski wrote:
> On 02/04/2022 07:12, Kuldeep Singh wrote:
> > GENI(generic interface) based Qualcomm Universal Peripheral controller
> > can support multiple serial interfaces like spi,uart and i2c.
> > 
> 
> Few more comments.
> 
> (...)
> 
> > +
> > +  clock-frequency:
> > +    description: Desired I2C bus clock frequency in Hz
> 
> Skip description, it's common for I2C controllers.

ok.

> 
> > +    default: 100000
> > +
> > +  interconnects:
> > +    maxItems: 3
> > +
> > +  interconnect-names:
> > +    items:
> > +      - const: qup-core
> > +      - const: qup-config
> > +      - const: qup-memory
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  power-domains:
> > +    maxItems: 1
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  required-opps:
> > +    maxItems: 1
> > +
> > +  dmas:
> > +    maxItems: 2
> > +
> > +  dma-names:
> > +    items:
> > +      - const: tx
> > +      - const: rx
> > +
> > +  pinctrl-0: true
> > +  pinctrl-1: true
> > +
> > +  pinctrl-names:
> > +    minItems: 1
> > +    items:
> > +      - const: default
> > +      - const: sleep
> > +
> > +  "#address-cells":
> > +    const: 1
> > +
> > +  "#size-cells":
> > +    const: 0
> 
> These are not needed, they come from schema.

Yes. I will update in v2. Thanks!

-Kuldeep
Kuldeep Singh April 2, 2022, 7:44 p.m. UTC | #4 
On Sat, Apr 02, 2022 at 02:29:59PM +0200, Krzysztof Kozlowski wrote:
> On 02/04/2022 07:12, Kuldeep Singh wrote:
> > GENI(generic interface) based Qualcomm Universal Peripheral controller
> > can support multiple serial interfaces like spi,uart and i2c.
> > 
> > Unlike other i2c controllers, QUP i2c bindings are present in parent
> > schema. Move it out from parent to an individual binding and let parent
> > refer to child schema later on.
> > 
> > Please note, current schema isn't complete as it misses out few
> > properties and thus, add these missing properties along the process.
> > 
> > Signed-off-by: Kuldeep Singh <singh.kuldeep87k@gmail.com>
> > ---
> >  .../bindings/i2c/qcom,i2c-geni-qcom.yaml      | 110 ++++++++++++++++++
> >  1 file changed, 110 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/i2c/qcom,i2c-geni-qcom.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/i2c/qcom,i2c-geni-qcom.yaml b/Documentation/devicetree/bindings/i2c/qcom,i2c-geni-qcom.yaml
> > new file mode 100644
> > index 000000000000..01a02e680ea3
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/i2c/qcom,i2c-geni-qcom.yaml
> > @@ -0,0 +1,110 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: "http://devicetree.org/schemas/i2c/qcom,i2c-geni-qcom.yaml#"
> > +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> > +
> > +title: Qualcomm Geni based QUP I2C Controller
> > +
> > +maintainers:
> > +  - Andy Gross <agross@kernel.org>
> > +  - Bjorn Andersson <bjorn.andersson@linaro.org>
> > +
> > +allOf:
> > +  - $ref: /schemas/i2c/i2c-controller.yaml#
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - qcom,geni-i2c
> 
> Just const, no enum. There are no other flavors of this (unless you
> think there are?).

There are no other users. Will change it to const.

> 
> > +
> > +  clocks:
> > +    maxItems: 1
> > +
> > +  clock-names:
> > +    const: se
> > +
> > +  clock-frequency:
> > +    description: Desired I2C bus clock frequency in Hz
> > +    default: 100000
> > +
> > +  interconnects:
> > +    maxItems: 3
> > +
> > +  interconnect-names:
> > +    items:
> > +      - const: qup-core
> > +      - const: qup-config
> > +      - const: qup-memory
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  power-domains:
> > +    maxItems: 1
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  required-opps:
> > +    maxItems: 1
> 
> I have doubts this is correct property. Usually it is part of the
> opp-table. I see sc7180 needs this, but I think it is a mistake. Do you
> know how it is supposed to work?

Not sure how exactly it works. I took reference from
Documentation/devicetree/bindings/clock/qcom,videocc.yaml on how to add
required-opps.

-Kuldeep
Krzysztof Kozlowski April 2, 2022, 7:57 p.m. UTC | #5 
On 02/04/2022 21:44, Kuldeep Singh wrote:
> On Sat, Apr 02, 2022 at 02:29:59PM +0200, Krzysztof Kozlowski wrote:
>> On 02/04/2022 07:12, Kuldeep Singh wrote:
>>> GENI(generic interface) based Qualcomm Universal Peripheral controller
>>> can support multiple serial interfaces like spi,uart and i2c.
>>>
	
>>> +
>>> +  power-domains:
>>> +    maxItems: 1
>>> +
>>> +  reg:
>>> +    maxItems: 1
>>> +
>>> +  required-opps:
>>> +    maxItems: 1
>>
>> I have doubts this is correct property. Usually it is part of the
>> opp-table. I see sc7180 needs this, but I think it is a mistake. Do you
>> know how it is supposed to work?
> 
> Not sure how exactly it works. I took reference from
> Documentation/devicetree/bindings/clock/qcom,videocc.yaml on how to add
> required-opps.
>

I see now that power domains consumer bindings also mention it:
Documentation/devicetree/bindings/power/power_domain.txt
and it might be actually used via __genpd_dev_pm_attach().

Let's keep it then.

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/i2c/qcom,i2c-geni-qcom.yaml b/Documentation/devicetree/bindings/i2c/qcom,i2c-geni-qcom.yaml
new file mode 100644
index 000000000000..01a02e680ea3
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/qcom,i2c-geni-qcom.yaml
@@ -0,0 +1,110 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/i2c/qcom,i2c-geni-qcom.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Qualcomm Geni based QUP I2C Controller
+
+maintainers:
+  - Andy Gross <agross@kernel.org>
+  - Bjorn Andersson <bjorn.andersson@linaro.org>
+
+allOf:
+  - $ref: /schemas/i2c/i2c-controller.yaml#
+
+properties:
+  compatible:
+    enum:
+      - qcom,geni-i2c
+
+  clocks:
+    maxItems: 1
+
+  clock-names:
+    const: se
+
+  clock-frequency:
+    description: Desired I2C bus clock frequency in Hz
+    default: 100000
+
+  interconnects:
+    maxItems: 3
+
+  interconnect-names:
+    items:
+      - const: qup-core
+      - const: qup-config
+      - const: qup-memory
+
+  interrupts:
+    maxItems: 1
+
+  power-domains:
+    maxItems: 1
+
+  reg:
+    maxItems: 1
+
+  required-opps:
+    maxItems: 1
+
+  dmas:
+    maxItems: 2
+
+  dma-names:
+    items:
+      - const: tx
+      - const: rx
+
+  pinctrl-0: true
+  pinctrl-1: true
+
+  pinctrl-names:
+    minItems: 1
+    items:
+      - const: default
+      - const: sleep
+
+  "#address-cells":
+    const: 1
+
+  "#size-cells":
+    const: 0
+
+required:
+  - compatible
+  - interrupts
+  - clocks
+  - clock-names
+  - reg
+  - "#address-cells"
+  - "#size-cells"
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/clock/qcom,gcc-sc7180.h>
+    #include <dt-bindings/interconnect/qcom,sc7180.h>
+    #include <dt-bindings/power/qcom-rpmpd.h>
+
+    i2c@88000 {
+        compatible = "qcom,geni-i2c";
+        reg = <0x00880000 0x4000>;
+        clock-names = "se";
+        clocks = <&gcc GCC_QUPV3_WRAP0_S0_CLK>;
+        pinctrl-names = "default";
+        pinctrl-0 = <&qup_i2c0_default>;
+        interrupts = <GIC_SPI 601 IRQ_TYPE_LEVEL_HIGH>;
+        #address-cells = <1>;
+        #size-cells = <0>;
+        interconnects = <&qup_virt MASTER_QUP_CORE_0 0 &qup_virt SLAVE_QUP_CORE_0 0>,
+                        <&gem_noc MASTER_APPSS_PROC 0 &config_noc SLAVE_QUP_0 0>,
+                        <&aggre1_noc MASTER_QUP_0 0 &mc_virt SLAVE_EBI1 0>;
+        interconnect-names = "qup-core", "qup-config", "qup-memory";
+        power-domains = <&rpmhpd SC7180_CX>;
+        required-opps = <&rpmhpd_opp_low_svs>;
+    };
+...