diff mbox series

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

Message ID 20221014221025.7372-2-quic_molvera@quicinc.com (mailing list archive)
State Superseded
Headers show
Series Add pinctrl support for QDU1000/QRU1000 SoCs | expand

Commit Message

Melody Olvera Oct. 14, 2022, 10:10 p.m. UTC
Add documentation details for device tree bindings for QDU1000 and QRU1000
TLMM devices.

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

Comments

Krzysztof Kozlowski Oct. 15, 2022, 1:38 p.m. UTC | #1
On 14/10/2022 18:10, Melody Olvera wrote:
> Add documentation details for device tree bindings for QDU1000 and QRU1000
> TLMM devices.
> 
> Signed-off-by: Melody Olvera <quic_molvera@quicinc.com>
> ---
>  .../pinctrl/qcom,qdu1000-pinctrl.yaml         | 159 ++++++++++++++++++
>  1 file changed, 159 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pinctrl/qcom,qdu1000-pinctrl.yaml
> 
> diff --git a/Documentation/devicetree/bindings/pinctrl/qcom,qdu1000-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/qcom,qdu1000-pinctrl.yaml
> new file mode 100644
> index 000000000000..2ec661e5e1e0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pinctrl/qcom,qdu1000-pinctrl.yaml

Filename matching compatible, so qcom,qdu1000-tlmm.yaml

> @@ -0,0 +1,159 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pinctrl/qcom,qdu1000-pinctrl.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 binding describes the Top Level Mode Multiplexer block (TLMM) found
> +  in the QDU1000 and QRU1000 platforms.
> +
> +allOf:
> +  - $ref: /schemas/pinctrl/qcom,tlmm-common.yaml#
> +
> +properties:
> +  compatible:
> +    items:
> +      - const: qcom,qdu1000-tlmm
> +      - const: qcom,qru1000-tlmm
> +
> +  reg:
> +    maxItems: 1

Blank line

> +  interrupts: true
> +  interrupt-controller: true
> +  "#interrupt-cells": true
> +  gpio-controller: true
> +
> +  gpio-reserved-ranges:
> +    minItems: 1
> +    maxItems: 75
> +
> +  gpio-line-names:
> +    maxItems: 150
> +
> +  "#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])$"
> +            - enum: [ ufs_reset, sdc2_clk, sdc2_cmd, sdc2_data ]
> +        minItems: 1
> +        maxItems: 36
> +
> +      function:
> +        description:
> +          Specify the alternative function to be configured for the specified
> +          pins.
> +        enum: [ adsp_ext, agera_pll, atest_char, atest_tsens, atest_tsens2,
> +                atest_usb1, atest_usb10, atest_usb11, atest_usb12, atest_usb13,
> +                atest_usb2, atest_usb20, atest_usb21, atest_usb22, atest_usb23,
> +                audio_ref, btfm_slimbus, cam_mclk, cci_async, cci_i2c,
> +                cci_timer0, cci_timer1, cci_timer2, cci_timer3, cci_timer4,
> +                cri_trng, cri_trng0, cri_trng1, dbg_out, ddr_bist, ddr_pxi0,
> +                ddr_pxi1, ddr_pxi2, ddr_pxi3, edp_hot, edp_lcd, gcc_gp1,
> +                gcc_gp2, gcc_gp3, gpio, jitter_bist, ldo_en, ldo_update,
> +                lpass_slimbus, mdp_vsync, mdp_vsync0, mdp_vsync1, mdp_vsync2,
> +                mdp_vsync3, mss_lte, m_voc, nav_pps, pa_indicator, pci_e0,
> +                pci_e1, phase_flag, pll_bist, pll_bypassnl, pll_reset,
> +                pri_mi2s, pri_mi2s_ws, prng_rosc, qdss, qdss_cti, qlink_enable,
> +                qlink_request, qspi_clk, qspi_cs, qspi_data, qua_mi2s, qup0,
> +                qup1, qup10, qup11, qup12, qup13, qup14, qup15, qup2, qup3,
> +                qup4, qup5, qup6, qup7, qup8, qup9, qup_l4, qup_l5, qup_l6,
> +                sdc4_clk, sdc4_cmd, sdc4_data, sd_write, sec_mi2s, sp_cmu,
> +                spkr_i2s, ter_mi2s, tgu_ch0, tgu_ch1, tgu_ch2, tgu_ch3,
> +                tsense_pwm1, tsense_pwm2, tsif1_clk, tsif1_data, tsif1_en,
> +                tsif1_error, tsif1_sync, tsif2_clk, tsif2_data, tsif2_en,
> +                tsif2_error, tsif2_sync, uim1_clk, uim1_data, uim1_present,
> +                uim1_reset, uim2_clk, uim2_data, uim2_present, uim2_reset,
> +                uim_batt, usb_phy, vfr_1, vsense_trigger, wlan1_adc0,
> +                wlan1_adc1, wlan2_adc0, wlan2_adc1]
> +
> +      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@03000000 {
> +        compatible = "qcom,qdu1000-pinctrl";
> +        reg = <0x03000000 0xdc2000>;
> +        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_intc>;
> +
> +        cci0-default-state {

You have now bindings for your new SoC and you still did not run them
against your own DTS... The purpose of DT schema is not only to have
them, but actually to use them.

There is no point for us to do manual review if automated tools can do
the work, right?

So please test your DTS.

Best regards,
Krzysztof
Rob Herring (Arm) Oct. 16, 2022, 10:58 p.m. UTC | #2
On Fri, 14 Oct 2022 15:10:24 -0700, Melody Olvera wrote:
> Add documentation details for device tree bindings for QDU1000 and QRU1000
> TLMM devices.
> 
> Signed-off-by: Melody Olvera <quic_molvera@quicinc.com>
> ---
>  .../pinctrl/qcom,qdu1000-pinctrl.yaml         | 159 ++++++++++++++++++
>  1 file changed, 159 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pinctrl/qcom,qdu1000-pinctrl.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/pinctrl/qcom,qdu1000-pinctrl.example.dts:23.26-59.11: Warning (unit_address_format): /example-0/pinctrl@03000000: unit name should not have leading 0s
Documentation/devicetree/bindings/pinctrl/qcom,qdu1000-pinctrl.example.dtb:0:0: /example-0/pinctrl@03000000: failed to match any schema with compatible: ['qcom,qdu1000-pinctrl']

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.
Melody Olvera Oct. 19, 2022, 6:22 p.m. UTC | #3
On 10/15/2022 6:38 AM, Krzysztof Kozlowski wrote:
> On 14/10/2022 18:10, Melody Olvera wrote:
>> Add documentation details for device tree bindings for QDU1000 and QRU1000
>> TLMM devices.
>>
>> Signed-off-by: Melody Olvera <quic_molvera@quicinc.com>
>> ---
>>  .../pinctrl/qcom,qdu1000-pinctrl.yaml         | 159 ++++++++++++++++++
>>  1 file changed, 159 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/pinctrl/qcom,qdu1000-pinctrl.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/pinctrl/qcom,qdu1000-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/qcom,qdu1000-pinctrl.yaml
>> new file mode 100644
>> index 000000000000..2ec661e5e1e0
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/pinctrl/qcom,qdu1000-pinctrl.yaml
> Filename matching compatible, so qcom,qdu1000-tlmm.yaml
Got it.
>> @@ -0,0 +1,159 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/pinctrl/qcom,qdu1000-pinctrl.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 binding describes the Top Level Mode Multiplexer block (TLMM) found
>> +  in the QDU1000 and QRU1000 platforms.
>> +
>> +allOf:
>> +  - $ref: /schemas/pinctrl/qcom,tlmm-common.yaml#
>> +
>> +properties:
>> +  compatible:
>> +    items:
>> +      - const: qcom,qdu1000-tlmm
>> +      - const: qcom,qru1000-tlmm
>> +
>> +  reg:
>> +    maxItems: 1
> Blank line
Sure.
>
>> +  interrupts: true
>> +  interrupt-controller: true
>> +  "#interrupt-cells": true
>> +  gpio-controller: true
>> +
>> +  gpio-reserved-ranges:
>> +    minItems: 1
>> +    maxItems: 75
>> +
>> +  gpio-line-names:
>> +    maxItems: 150
>> +
>> +  "#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])$"
>> +            - enum: [ ufs_reset, sdc2_clk, sdc2_cmd, sdc2_data ]
>> +        minItems: 1
>> +        maxItems: 36
>> +
>> +      function:
>> +        description:
>> +          Specify the alternative function to be configured for the specified
>> +          pins.
>> +        enum: [ adsp_ext, agera_pll, atest_char, atest_tsens, atest_tsens2,
>> +                atest_usb1, atest_usb10, atest_usb11, atest_usb12, atest_usb13,
>> +                atest_usb2, atest_usb20, atest_usb21, atest_usb22, atest_usb23,
>> +                audio_ref, btfm_slimbus, cam_mclk, cci_async, cci_i2c,
>> +                cci_timer0, cci_timer1, cci_timer2, cci_timer3, cci_timer4,
>> +                cri_trng, cri_trng0, cri_trng1, dbg_out, ddr_bist, ddr_pxi0,
>> +                ddr_pxi1, ddr_pxi2, ddr_pxi3, edp_hot, edp_lcd, gcc_gp1,
>> +                gcc_gp2, gcc_gp3, gpio, jitter_bist, ldo_en, ldo_update,
>> +                lpass_slimbus, mdp_vsync, mdp_vsync0, mdp_vsync1, mdp_vsync2,
>> +                mdp_vsync3, mss_lte, m_voc, nav_pps, pa_indicator, pci_e0,
>> +                pci_e1, phase_flag, pll_bist, pll_bypassnl, pll_reset,
>> +                pri_mi2s, pri_mi2s_ws, prng_rosc, qdss, qdss_cti, qlink_enable,
>> +                qlink_request, qspi_clk, qspi_cs, qspi_data, qua_mi2s, qup0,
>> +                qup1, qup10, qup11, qup12, qup13, qup14, qup15, qup2, qup3,
>> +                qup4, qup5, qup6, qup7, qup8, qup9, qup_l4, qup_l5, qup_l6,
>> +                sdc4_clk, sdc4_cmd, sdc4_data, sd_write, sec_mi2s, sp_cmu,
>> +                spkr_i2s, ter_mi2s, tgu_ch0, tgu_ch1, tgu_ch2, tgu_ch3,
>> +                tsense_pwm1, tsense_pwm2, tsif1_clk, tsif1_data, tsif1_en,
>> +                tsif1_error, tsif1_sync, tsif2_clk, tsif2_data, tsif2_en,
>> +                tsif2_error, tsif2_sync, uim1_clk, uim1_data, uim1_present,
>> +                uim1_reset, uim2_clk, uim2_data, uim2_present, uim2_reset,
>> +                uim_batt, usb_phy, vfr_1, vsense_trigger, wlan1_adc0,
>> +                wlan1_adc1, wlan2_adc0, wlan2_adc1]
>> +
>> +      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@03000000 {
>> +        compatible = "qcom,qdu1000-pinctrl";
>> +        reg = <0x03000000 0xdc2000>;
>> +        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_intc>;
>> +
>> +        cci0-default-state {
> You have now bindings for your new SoC and you still did not run them
> against your own DTS... The purpose of DT schema is not only to have
> them, but actually to use them.
>
> There is no point for us to do manual review if automated tools can do
> the work, right?
>
> So please test your DTS.
Yes this is very fair. Apologies; I'm new to this process so I'm still learning about the
tools at my disposal. I believe I've figured out running dt_bindings_check and dtbs_check;
future patchsets will have these run prior to submitting.

Thanks,
Melody
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/pinctrl/qcom,qdu1000-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/qcom,qdu1000-pinctrl.yaml
new file mode 100644
index 000000000000..2ec661e5e1e0
--- /dev/null
+++ b/Documentation/devicetree/bindings/pinctrl/qcom,qdu1000-pinctrl.yaml
@@ -0,0 +1,159 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pinctrl/qcom,qdu1000-pinctrl.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 binding describes the Top Level Mode Multiplexer block (TLMM) found
+  in the QDU1000 and QRU1000 platforms.
+
+allOf:
+  - $ref: /schemas/pinctrl/qcom,tlmm-common.yaml#
+
+properties:
+  compatible:
+    items:
+      - const: qcom,qdu1000-tlmm
+      - const: qcom,qru1000-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: 150
+
+  "#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])$"
+            - enum: [ ufs_reset, sdc2_clk, sdc2_cmd, sdc2_data ]
+        minItems: 1
+        maxItems: 36
+
+      function:
+        description:
+          Specify the alternative function to be configured for the specified
+          pins.
+        enum: [ adsp_ext, agera_pll, atest_char, atest_tsens, atest_tsens2,
+                atest_usb1, atest_usb10, atest_usb11, atest_usb12, atest_usb13,
+                atest_usb2, atest_usb20, atest_usb21, atest_usb22, atest_usb23,
+                audio_ref, btfm_slimbus, cam_mclk, cci_async, cci_i2c,
+                cci_timer0, cci_timer1, cci_timer2, cci_timer3, cci_timer4,
+                cri_trng, cri_trng0, cri_trng1, dbg_out, ddr_bist, ddr_pxi0,
+                ddr_pxi1, ddr_pxi2, ddr_pxi3, edp_hot, edp_lcd, gcc_gp1,
+                gcc_gp2, gcc_gp3, gpio, jitter_bist, ldo_en, ldo_update,
+                lpass_slimbus, mdp_vsync, mdp_vsync0, mdp_vsync1, mdp_vsync2,
+                mdp_vsync3, mss_lte, m_voc, nav_pps, pa_indicator, pci_e0,
+                pci_e1, phase_flag, pll_bist, pll_bypassnl, pll_reset,
+                pri_mi2s, pri_mi2s_ws, prng_rosc, qdss, qdss_cti, qlink_enable,
+                qlink_request, qspi_clk, qspi_cs, qspi_data, qua_mi2s, qup0,
+                qup1, qup10, qup11, qup12, qup13, qup14, qup15, qup2, qup3,
+                qup4, qup5, qup6, qup7, qup8, qup9, qup_l4, qup_l5, qup_l6,
+                sdc4_clk, sdc4_cmd, sdc4_data, sd_write, sec_mi2s, sp_cmu,
+                spkr_i2s, ter_mi2s, tgu_ch0, tgu_ch1, tgu_ch2, tgu_ch3,
+                tsense_pwm1, tsense_pwm2, tsif1_clk, tsif1_data, tsif1_en,
+                tsif1_error, tsif1_sync, tsif2_clk, tsif2_data, tsif2_en,
+                tsif2_error, tsif2_sync, uim1_clk, uim1_data, uim1_present,
+                uim1_reset, uim2_clk, uim2_data, uim2_present, uim2_reset,
+                uim_batt, usb_phy, vfr_1, vsense_trigger, wlan1_adc0,
+                wlan1_adc1, wlan2_adc0, wlan2_adc1]
+
+      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@03000000 {
+        compatible = "qcom,qdu1000-pinctrl";
+        reg = <0x03000000 0xdc2000>;
+        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_intc>;
+
+        cci0-default-state {
+            pins = "gpio17", "gpio18";
+            function = "cci_i2c";
+
+            bias-pull-up;
+            drive-strength = <2>;
+        };
+
+        cam0-default-state {
+            rst-pins {
+                pins = "gpio9";
+                function = "gpio";
+
+                drive-strength = <16>;
+                bias-disable;
+            };
+
+            mclk0-pins {
+                pins = "gpio13";
+                function = "cam_mclk";
+
+                drive-strength = <16>;
+                bias-disable;
+            };
+        };
+    };