diff mbox series

[v4,1/2] dt-bindings: pinctrl: qcom: Add QDU1000 and QRU1000 pinctrl

Message ID 20221118182039.29236-2-quic_molvera@quicinc.com
State Superseded
Headers show
Series Add pinctrl support for QDU1000/QRU1000 SoCs | expand

Commit Message

Melody Olvera Nov. 18, 2022, 6:20 p.m. UTC
Add device tree bindings for QDU1000 and QRU1000 TLMM devices.

Signed-off-by: Melody Olvera <quic_molvera@quicinc.com>
---
 .../bindings/pinctrl/qcom,qdu1000-tlmm.yaml   | 134 ++++++++++++++++++
 1 file changed, 134 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pinctrl/qcom,qdu1000-tlmm.yaml

Comments

Melody Olvera Nov. 21, 2022, 8:38 p.m. UTC | #1
On 11/20/2022 4:58 AM, Krzysztof Kozlowski wrote:
> On 18/11/2022 19:20, Melody Olvera wrote:
>> Add device tree bindings for QDU1000 and QRU1000 TLMM devices.
>>
>> Signed-off-by: Melody Olvera <quic_molvera@quicinc.com>
>> ---
>>  .../bindings/pinctrl/qcom,qdu1000-tlmm.yaml   | 134 ++++++++++++++++++
>>  1 file changed, 134 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/pinctrl/qcom,qdu1000-tlmm.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/pinctrl/qcom,qdu1000-tlmm.yaml b/Documentation/devicetree/bindings/pinctrl/qcom,qdu1000-tlmm.yaml
>> new file mode 100644
>> index 000000000000..cb0c496d8666
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/pinctrl/qcom,qdu1000-tlmm.yaml
>> @@ -0,0 +1,134 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/pinctrl/qcom,qdu1000-tlmm.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Qualcomm Technologies, Inc. QDU1000/QRU1000 TLMM block
>> +
>> +maintainers:
>> +  - Melody Olvera <quic_molvera@quicinc.com>
>> +
>> +description: |
>> +  This Top Level Mode Multiplexer block (TLMM) is found in the QDU1000 and
>> +  QRU1000 platforms.
> It's better to keep consistent style which allows to do easy
> search/replace, than to have new files using their own sentences. So
> keep it the same as was unified in few recent commits.

Ok... Just making sure that's what you want. Last PS you gave comments to change
the wording of this description to remove "This binding describes..." as we've done
in all the other qcom pinctrl/tlmm bindings. I can change the wording back to the
original, just want to be clear here.

>
>> +allOf:
>> +  - $ref: /schemas/pinctrl/qcom,tlmm-common.yaml#
>> +
>> +properties:
>> +  compatible:
>> +    const: qcom,qdu1000-tlmm
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  interrupts: true
>> +  interrupt-controller: true
>> +  "#interrupt-cells": true
>> +  gpio-controller: true
>> +
>> +  gpio-reserved-ranges:
>> +    minItems: 1
>> +    maxItems: 75
> Technically you can have up to 76 separate ranges (151 GPIOs).

Fair enough. I can fix this.

Thanks,
Melody
>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>
>
> Best regards,
> Krzysztof
>
Krzysztof Kozlowski Nov. 22, 2022, 7:48 a.m. UTC | #2
On 21/11/2022 21:38, Melody Olvera wrote:
> 
> 
> On 11/20/2022 4:58 AM, Krzysztof Kozlowski wrote:
>> On 18/11/2022 19:20, Melody Olvera wrote:
>>> Add device tree bindings for QDU1000 and QRU1000 TLMM devices.
>>>
>>> Signed-off-by: Melody Olvera <quic_molvera@quicinc.com>
>>> ---
>>>  .../bindings/pinctrl/qcom,qdu1000-tlmm.yaml   | 134 ++++++++++++++++++
>>>  1 file changed, 134 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/pinctrl/qcom,qdu1000-tlmm.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/pinctrl/qcom,qdu1000-tlmm.yaml b/Documentation/devicetree/bindings/pinctrl/qcom,qdu1000-tlmm.yaml
>>> new file mode 100644
>>> index 000000000000..cb0c496d8666
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/pinctrl/qcom,qdu1000-tlmm.yaml
>>> @@ -0,0 +1,134 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/pinctrl/qcom,qdu1000-tlmm.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Qualcomm Technologies, Inc. QDU1000/QRU1000 TLMM block
>>> +
>>> +maintainers:
>>> +  - Melody Olvera <quic_molvera@quicinc.com>
>>> +
>>> +description: |
>>> +  This Top Level Mode Multiplexer block (TLMM) is found in the QDU1000 and
>>> +  QRU1000 platforms.
>> It's better to keep consistent style which allows to do easy
>> search/replace, than to have new files using their own sentences. So
>> keep it the same as was unified in few recent commits.
> 
> Ok... Just making sure that's what you want. Last PS you gave comments to change
> the wording of this description to remove "This binding describes..." as we've done
> in all the other qcom pinctrl/tlmm bindings. I can change the wording back to the
> original, just want to be clear here.

I propose to have the same wording as other Qualcomm TLMM bindings,
however you changed it to something not the same. Therefore I wonder -
why having here different wording than all other bindings?

By going back to original - what do you mean? If it matches all others,
then yes, but I doubt it.

Just to be sure - are you working on proper (recent) trees or something old?

Best regards,
Krzysztof
Melody Olvera Nov. 22, 2022, 3:23 p.m. UTC | #3
On 11/22/2022 1:48 AM, Krzysztof Kozlowski wrote:
> On 21/11/2022 21:38, Melody Olvera wrote:
>>
>> On 11/20/2022 4:58 AM, Krzysztof Kozlowski wrote:
>>> On 18/11/2022 19:20, Melody Olvera wrote:
>>>> Add device tree bindings for QDU1000 and QRU1000 TLMM devices.
>>>>
>>>> Signed-off-by: Melody Olvera <quic_molvera@quicinc.com>
>>>> ---
>>>>  .../bindings/pinctrl/qcom,qdu1000-tlmm.yaml   | 134 ++++++++++++++++++
>>>>  1 file changed, 134 insertions(+)
>>>>  create mode 100644 Documentation/devicetree/bindings/pinctrl/qcom,qdu1000-tlmm.yaml
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/pinctrl/qcom,qdu1000-tlmm.yaml b/Documentation/devicetree/bindings/pinctrl/qcom,qdu1000-tlmm.yaml
>>>> new file mode 100644
>>>> index 000000000000..cb0c496d8666
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/pinctrl/qcom,qdu1000-tlmm.yaml
>>>> @@ -0,0 +1,134 @@
>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>>> +%YAML 1.2
>>>> +---
>>>> +$id: http://devicetree.org/schemas/pinctrl/qcom,qdu1000-tlmm.yaml#
>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>> +
>>>> +title: Qualcomm Technologies, Inc. QDU1000/QRU1000 TLMM block
>>>> +
>>>> +maintainers:
>>>> +  - Melody Olvera <quic_molvera@quicinc.com>
>>>> +
>>>> +description: |
>>>> +  This Top Level Mode Multiplexer block (TLMM) is found in the QDU1000 and
>>>> +  QRU1000 platforms.
>>> It's better to keep consistent style which allows to do easy
>>> search/replace, than to have new files using their own sentences. So
>>> keep it the same as was unified in few recent commits.
>> Ok... Just making sure that's what you want. Last PS you gave comments to change
>> the wording of this description to remove "This binding describes..." as we've done
>> in all the other qcom pinctrl/tlmm bindings. I can change the wording back to the
>> original, just want to be clear here.
> I propose to have the same wording as other Qualcomm TLMM bindings,
> however you changed it to something not the same. Therefore I wonder -
> why having here different wording than all other bindings?
>
> By going back to original - what do you mean? If it matches all others,
> then yes, but I doubt it.
>
> Just to be sure - are you working on proper (recent) trees or something old?

Original matched how it was done on other Qualcomm TLMM bindings. Feedback
was to drop "This binding describes..." from [1], but all the Qualcomm TLMM
bindings start with "This binding describes...". I'm looking at qcom tree for-next
branch; should be recent, no?

[1] https://lore.kernel.org/all/37c53d8c-2810-509a-7404-7ca24d79fed8@linaro.org/

Thanks,
Melody

>
> Best regards,
> Krzysztof
>
Krzysztof Kozlowski Nov. 23, 2022, 8:06 a.m. UTC | #4
On 22/11/2022 16:23, Melody Olvera wrote:
> 
> 
> On 11/22/2022 1:48 AM, Krzysztof Kozlowski wrote:
>> On 21/11/2022 21:38, Melody Olvera wrote:
>>>
>>> On 11/20/2022 4:58 AM, Krzysztof Kozlowski wrote:
>>>> On 18/11/2022 19:20, Melody Olvera wrote:
>>>>> Add device tree bindings for QDU1000 and QRU1000 TLMM devices.
>>>>>
>>>>> Signed-off-by: Melody Olvera <quic_molvera@quicinc.com>
>>>>> ---
>>>>>  .../bindings/pinctrl/qcom,qdu1000-tlmm.yaml   | 134 ++++++++++++++++++
>>>>>  1 file changed, 134 insertions(+)
>>>>>  create mode 100644 Documentation/devicetree/bindings/pinctrl/qcom,qdu1000-tlmm.yaml
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/pinctrl/qcom,qdu1000-tlmm.yaml b/Documentation/devicetree/bindings/pinctrl/qcom,qdu1000-tlmm.yaml
>>>>> new file mode 100644
>>>>> index 000000000000..cb0c496d8666
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/pinctrl/qcom,qdu1000-tlmm.yaml
>>>>> @@ -0,0 +1,134 @@
>>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>>>> +%YAML 1.2
>>>>> +---
>>>>> +$id: http://devicetree.org/schemas/pinctrl/qcom,qdu1000-tlmm.yaml#
>>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>>> +
>>>>> +title: Qualcomm Technologies, Inc. QDU1000/QRU1000 TLMM block
>>>>> +
>>>>> +maintainers:
>>>>> +  - Melody Olvera <quic_molvera@quicinc.com>
>>>>> +
>>>>> +description: |
>>>>> +  This Top Level Mode Multiplexer block (TLMM) is found in the QDU1000 and
>>>>> +  QRU1000 platforms.
>>>> It's better to keep consistent style which allows to do easy
>>>> search/replace, than to have new files using their own sentences. So
>>>> keep it the same as was unified in few recent commits.
>>> Ok... Just making sure that's what you want. Last PS you gave comments to change
>>> the wording of this description to remove "This binding describes..." as we've done
>>> in all the other qcom pinctrl/tlmm bindings. I can change the wording back to the
>>> original, just want to be clear here.
>> I propose to have the same wording as other Qualcomm TLMM bindings,
>> however you changed it to something not the same. Therefore I wonder -
>> why having here different wording than all other bindings?
>>
>> By going back to original - what do you mean? If it matches all others,
>> then yes, but I doubt it.
>>
>> Just to be sure - are you working on proper (recent) trees or something old?
> 
> Original matched how it was done on other Qualcomm TLMM bindings. Feedback
> was to drop "This binding describes..." from [1], but all the Qualcomm TLMM
> bindings start with "This binding describes...". I'm looking at qcom tree for-next
> branch; should be recent, no?

No. It's not recent for anything else than managed by Bjorn. You need to
base the patches on maintainer's trees, which is usually the easiest to
achieve via linux-next (especially that these changes were in my tree
for some time before I sent them to Linus).

Your all other patches might have similar issues - wrong base or not
good example/starting point.

Best regards,
Krzysztof
Melody Olvera Nov. 23, 2022, 5:30 p.m. UTC | #5
On 11/23/2022 2:06 AM, Krzysztof Kozlowski wrote:
> On 22/11/2022 16:23, Melody Olvera wrote:
>>
>> On 11/22/2022 1:48 AM, Krzysztof Kozlowski wrote:
>>> On 21/11/2022 21:38, Melody Olvera wrote:
>>>> On 11/20/2022 4:58 AM, Krzysztof Kozlowski wrote:
>>>>> On 18/11/2022 19:20, Melody Olvera wrote:
>>>>>> Add device tree bindings for QDU1000 and QRU1000 TLMM devices.
>>>>>>
>>>>>> Signed-off-by: Melody Olvera <quic_molvera@quicinc.com>
>>>>>> ---
>>>>>>  .../bindings/pinctrl/qcom,qdu1000-tlmm.yaml   | 134 ++++++++++++++++++
>>>>>>  1 file changed, 134 insertions(+)
>>>>>>  create mode 100644 Documentation/devicetree/bindings/pinctrl/qcom,qdu1000-tlmm.yaml
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/pinctrl/qcom,qdu1000-tlmm.yaml b/Documentation/devicetree/bindings/pinctrl/qcom,qdu1000-tlmm.yaml
>>>>>> new file mode 100644
>>>>>> index 000000000000..cb0c496d8666
>>>>>> --- /dev/null
>>>>>> +++ b/Documentation/devicetree/bindings/pinctrl/qcom,qdu1000-tlmm.yaml
>>>>>> @@ -0,0 +1,134 @@
>>>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>>>>> +%YAML 1.2
>>>>>> +---
>>>>>> +$id: http://devicetree.org/schemas/pinctrl/qcom,qdu1000-tlmm.yaml#
>>>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>>>> +
>>>>>> +title: Qualcomm Technologies, Inc. QDU1000/QRU1000 TLMM block
>>>>>> +
>>>>>> +maintainers:
>>>>>> +  - Melody Olvera <quic_molvera@quicinc.com>
>>>>>> +
>>>>>> +description: |
>>>>>> +  This Top Level Mode Multiplexer block (TLMM) is found in the QDU1000 and
>>>>>> +  QRU1000 platforms.
>>>>> It's better to keep consistent style which allows to do easy
>>>>> search/replace, than to have new files using their own sentences. So
>>>>> keep it the same as was unified in few recent commits.
>>>> Ok... Just making sure that's what you want. Last PS you gave comments to change
>>>> the wording of this description to remove "This binding describes..." as we've done
>>>> in all the other qcom pinctrl/tlmm bindings. I can change the wording back to the
>>>> original, just want to be clear here.
>>> I propose to have the same wording as other Qualcomm TLMM bindings,
>>> however you changed it to something not the same. Therefore I wonder -
>>> why having here different wording than all other bindings?
>>>
>>> By going back to original - what do you mean? If it matches all others,
>>> then yes, but I doubt it.
>>>
>>> Just to be sure - are you working on proper (recent) trees or something old?
>> Original matched how it was done on other Qualcomm TLMM bindings. Feedback
>> was to drop "This binding describes..." from [1], but all the Qualcomm TLMM
>> bindings start with "This binding describes...". I'm looking at qcom tree for-next
>> branch; should be recent, no?
> No. It's not recent for anything else than managed by Bjorn. You need to
> base the patches on maintainer's trees, which is usually the easiest to
> achieve via linux-next (especially that these changes were in my tree
> for some time before I sent them to Linus).

Ah ok; looking at linux-next looks like it should be "Top Level Mode Multiplexer pin
controller in Qualcomm QDU1000 and QRU1000 SoCs."

Thanks,
Melody

>
> Your all other patches might have similar issues - wrong base or not
> good example/starting point.
>
> Best regards,
> Krzysztof
>
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/pinctrl/qcom,qdu1000-tlmm.yaml b/Documentation/devicetree/bindings/pinctrl/qcom,qdu1000-tlmm.yaml
new file mode 100644
index 000000000000..cb0c496d8666
--- /dev/null
+++ b/Documentation/devicetree/bindings/pinctrl/qcom,qdu1000-tlmm.yaml
@@ -0,0 +1,134 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pinctrl/qcom,qdu1000-tlmm.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm Technologies, Inc. QDU1000/QRU1000 TLMM block
+
+maintainers:
+  - Melody Olvera <quic_molvera@quicinc.com>
+
+description: |
+  This Top Level Mode Multiplexer block (TLMM) is found in the QDU1000 and
+  QRU1000 platforms.
+
+allOf:
+  - $ref: /schemas/pinctrl/qcom,tlmm-common.yaml#
+
+properties:
+  compatible:
+    const: qcom,qdu1000-tlmm
+
+  reg:
+    maxItems: 1
+
+  interrupts: true
+  interrupt-controller: true
+  "#interrupt-cells": true
+  gpio-controller: true
+
+  gpio-reserved-ranges:
+    minItems: 1
+    maxItems: 75
+
+  gpio-line-names:
+    maxItems: 151
+
+  "#gpio-cells": true
+  gpio-ranges: true
+  wakeup-parent: true
+
+patternProperties:
+  "-state$":
+    oneOf:
+      - $ref: "#/$defs/qcom-qdu1000-tlmm-state"
+      - patternProperties:
+          "-pins$":
+            $ref: "#/$defs/qcom-qdu1000-tlmm-state"
+        additionalProperties: false
+
+$defs:
+  qcom-qdu1000-tlmm-state:
+    type: object
+    description:
+      Pinctrl node's client devices use subnodes for desired pin configuration.
+      Client device subnodes use below standard properties.
+    $ref: qcom,tlmm-common.yaml#/$defs/qcom-tlmm-state
+
+    properties:
+      pins:
+        description:
+          List of gpio pins affected by the properties specified in this
+          subnode.
+        items:
+          oneOf:
+            - pattern: "^gpio([0-9]|[1-9][0-9]|1[0-4][0-9]|150)$"
+            - enum: [ sdc1_rclk, sdc1_clk, sdc1_cmd, sdc1_data ]
+        minItems: 1
+        maxItems: 36
+
+      function:
+        description:
+          Specify the alternative function to be configured for the specified
+          pins.
+        enum: [ atest_char, atest_usb, char_exec, CMO_PRI, cmu_rng,
+                dbg_out_clk, ddr_bist, ddr_pxi1, ddr_pxi2, ddr_pxi3, ddr_pxi4,
+                ddr_pxi5, ddr_pxi6, ddr_pxi7, eth012_int_n, eth345_int_n,
+                gcc_gp1, gcc_gp2, gcc_gp3, gpio, gps_pps_in, hardsync_pps_in,
+                intr_c, jitter_bist_ref, pcie_clkreqn, phase_flag, pll_bist,
+                pll_clk, prng_rosc, qdss_cti, qdss_gpio, qlink0_enable,
+                qlink0_request, qlink0_wmss, qlink1_enable, qlink1_request,
+                qlink1_wmss, qlink2_enable, qlink2_request, qlink2_wmss,
+                qlink3_enable, qlink3_request, qlink3_wmss, qlink4_enable,
+                qlink4_request, qlink4_wmss, qlink5_enable, qlink5_request,
+                qlink5_wmss, qlink6_enable, qlink6_request, qlink6_wmss,
+                qlink7_enable, qlink7_request, qlink7_wmss, qspi_clk, qspi_cs,
+                qspi0, qspi1, qspi2, qspi3, qup00, qup01, qup02, qup03, qup04,
+                qup05, qup06, qup07, qup08, qup10, qup11, qup12, qup13, qup14,
+                qup15, qup16, qup17, qup20, qup21, qup22, SI5518_INT, smb_alert,
+                smb_clk, smb_dat, tb_trig, tgu_ch0, tgu_ch1, tgu_ch2, tgu_ch3,
+                tgu_ch4, tgu_ch5, tgu_ch6, tgu_ch7, tmess_prng0, tmess_prng1,
+                tmess_prng2, tmess_prng3, tod_pps_in, tsense_pwm1, tsense_pwm2,
+                usb2phy_ac, usb_con_det, usb_dfp_en, usb_phy, vfr_0, vfr_1,
+                vsense_trigger ]
+
+      bias-disable: true
+      bias-pull-down: true
+      bias-pull-up: true
+      drive-strength: true
+      input-enable: true
+      output-high: true
+      output-low: true
+
+    required:
+      - pins
+
+    additionalProperties: false
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    pinctrl@f000000 {
+        compatible = "qcom,qdu1000-tlmm";
+        reg = <0xf000000 0x1000000>;
+        interrupts = <GIC_SPI 208 IRQ_TYPE_LEVEL_HIGH>;
+        gpio-controller;
+        #gpio-cells = <2>;
+        interrupt-controller;
+        #interrupt-cells = <2>;
+        gpio-ranges = <&tlmm 0 0 151>;
+        wakeup-parent = <&pdc>;
+
+        uart0-default-state {
+            pins = "gpio6", "gpio7", "gpio8", "gpio9";
+            function = "qup00";
+        };
+    };