mbox series

[0/2] Add pinctrl support for QDU1000/QRU1000 SoCs

Message ID 20221001030546.28220-1-quic_molvera@quicinc.com
Headers show
Series Add pinctrl support for QDU1000/QRU1000 SoCs | expand

Message

Melody Olvera Oct. 1, 2022, 3:05 a.m. UTC 
This patchset adds pinctrl support for the Qualcomm QDU1000 and QRU1000
SoCs.

The Qualcomm Technologies, Inc. Distributed Unit 1000 and Radio Unit
1000 are new SoCs meant for enabling Open RAN solutions. See more at
https://www.qualcomm.com/content/dam/qcomm-martech/dm-assets/documents/qualcomm_5g_ran_platforms_product_brief.pdf

Melody Olvera (2):
  dt-bindings: pinctrl: qcom: Add QDU1000 and QRU1000 pinctrl bindings
  pinctrl: qcom: Add QDU1000/QRU1000 pinctrl driver

 .../pinctrl/qcom,qdru1000-pinctrl.yaml        |  124 ++
 drivers/pinctrl/qcom/Kconfig                  |   10 +
 drivers/pinctrl/qcom/Makefile                 |    1 +
 drivers/pinctrl/qcom/pinctrl-qdru1000.c       |   59 +
 drivers/pinctrl/qcom/pinctrl-qdru1000.h       | 1896 +++++++++++++++++
 5 files changed, 2090 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pinctrl/qcom,qdru1000-pinctrl.yaml
 create mode 100644 drivers/pinctrl/qcom/pinctrl-qdru1000.c
 create mode 100644 drivers/pinctrl/qcom/pinctrl-qdru1000.h


base-commit: 987a926c1d8a40e4256953b04771fbdb63bc7938

Comments

Krzysztof Kozlowski Oct. 1, 2022, 9:20 a.m. UTC | #1 
On 01/10/2022 05:05, 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,qdru1000-pinctrl.yaml        | 133 ++++++++++++++++++
>  1 file changed, 133 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pinctrl/qcom,qdru1000-pinctrl.yaml
> 
> diff --git a/Documentation/devicetree/bindings/pinctrl/qcom,qdru1000-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/qcom,qdru1000-pinctrl.yaml
> new file mode 100644
> index 000000000000..e8d938303231
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pinctrl/qcom,qdru1000-pinctrl.yaml
> @@ -0,0 +1,133 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/bindings/pinctrl/qcom,qdru1000-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.
> +
> +properties:
> +  compatible:
> +    items:
> +      - const: qcom,qdu1000-tlmm
> +      - const: qcom,qru1000-tlmm
> +
> +  reg:
> +    items:
> +      - description: Base address of TLMM register space
> +      - description: Size of TLMM register space
> +
> +  interrupts:
> +    minItems: 0

Cannot be 0 of interrupts.

> +    maxItems: 1
> +    items:
> +      - const: TLMM summary IRQ
> +
> +  interrupt-controller: true
> +
> +  '#interrupt-cells':
> +    const: 2
> +
> +  gpio-controller: true
> +
> +  '#gpio-cells':
> +    const: 2
> +
> +  wakeup-parent:
> +    maxItems: 1
> +    description:
> +      Please refer to ../gpio/gpio.txt and ../interrupt-controller/interrupts.txt for
> +      a general description of GPIO and interrupt bindings.
> +
> +      Please refer to pinctrl-bindings.txt in this directory for details of the
> +      common pinctrl bindings used by client devices, including the meaning of the
> +      phrase "pin configuration node".
> +
> +      The pin configuration nodes act as a container for an arbitrary number of
> +      subnodes. Each of these subnodes represents some desired configuration for a
> +      pin, a group, or a list of pins or groups. This configuration can include the
> +      mux function to select on those pin(s)/group(s), and various pin configuration
> +      parameters, such as pull-up, drive strength, etc.
> +
> +
> +# PIN CONFIGURATION NODES
> +patternPropetries:
> +  '^.*$':
> +    if:
> +      type: object
> +    then:

Nope, that's not correct binding. It does not work. It never worked.

Please do it exactly like:
https://lore.kernel.org/linux-devicetree/20220930200529.331223-1-krzysztof.kozlowski@linaro.org/T/#m08b62ef5d873a52a5cbf3c53b25eff03726e7a16
https://lore.kernel.org/linux-devicetree/20220927173702.5200-1-krzysztof.kozlowski@linaro.org/T/#t


> +      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-9][0-9]|20[0-9])"
> +              - enum: [ sdc2_clk, sdc2_cmd, sdc2_data, ufs_reset ]
> +            minItems: 1
> +            maxItems: 36
> +        function:
> +          description:
> +            Specify the alternative function to be configured for the
> +            specified pins. Functions are only valid for gpio pins.
> +          enum: [gpio, aon_cam, atest_char, atest_char0, atest_char1, atest_char2, atest_char3,
> +            atest_usb0, atest_usb00, atest_usb01, atest_usb02, atest_usb03, audio_ref, cam_mclk,
> +            cci_async, cci_i2c, cci_timer0, cci_timer1, cci_timer2, cci_timer3, cci_timer4,
> +            cmu_rng0, cmu_rng1, cmu_rng2, cmu_rng3, coex_uart1, coex_uart2, cri_trng, cri_trng0,
> +            cri_trng1, dbg_out, ddr_bist, ddr_pxi0, ddr_pxi1, ddr_pxi2, ddr_pxi3, dp_hot, gcc_gp1,
> +            gcc_gp2, gcc_gp3, ibi_i3c, jitter_bist, mdp_vsync, mdp_vsync0, mdp_vsync1, mdp_vsync2,
> +            mdp_vsync3, mi2s0_data0, mi2s0_data1, mi2s0_sck, mi2s0_ws, mi2s2_data0, mi2s2_data1,
> +            mi2s2_sck, mi2s2_ws, mss_grfc0, mss_grfc1, mss_grfc10, mss_grfc11, mss_grfc12,
> +            mss_grfc2, mss_grfc3, mss_grfc4, mss_grfc5, mss_grfc6, mss_grfc7, mss_grfc8, mss_grfc9,
> +            nav_0, nav_1, nav_2, pcie0_clkreqn, pcie1_clkreqn, phase_flag0, phase_flag1,
> +            phase_flag10, phase_flag11, phase_flag12, phase_flag13, phase_flag14, phase_flag15,
> +            phase_flag16, phase_flag17, phase_flag18, phase_flag19, phase_flag2, phase_flag20,
> +            phase_flag21, phase_flag22, phase_flag23, phase_flag24, phase_flag25, phase_flag26,
> +            phase_flag27, phase_flag28, phase_flag29, phase_flag3, phase_flag30, phase_flag31,
> +            phase_flag4, phase_flag5, phase_flag6, phase_flag7, phase_flag8, phase_flag9, pll_bist,
> +            pll_clk, pri_mi2s, prng_rosc0, prng_rosc1, prng_rosc2, prng_rosc3, qdss_cti, qdss_gpio,
> +            qdss_gpio0, qdss_gpio1, qdss_gpio10, qdss_gpio11, qdss_gpio12, qdss_gpio13, qdss_gpio14,
> +            qdss_gpio15, qdss_gpio2, qdss_gpio3, qdss_gpio4, qdss_gpio5, qdss_gpio6, qdss_gpio7,
> +            qdss_gpio8, qdss_gpio9, qlink0_enable, qlink0_request, qlink0_wmss, qlink1_enable,
> +            qlink1_request, qlink1_wmss, qlink2_enable, qlink2_request, qlink2_wmss, qspi0, qspi1,
> +            qspi2, qspi3, qspi_clk, qspi_cs, qup0, qup1, qup10, qup11, qup12, qup13, qup14, qup15,
> +            qup16, qup17, qup18, qup19, qup2, qup20, qup21, qup3, qup4, qup5, qup6, qup7, qup8,
> +            qup9, qup_l4, qup_l5, qup_l6, sd_write, sdc40, sdc41, sdc42, sdc43, sdc4_clk, sdc4_cmd,
> +            sec_mi2s, tb_trig, tgu_ch0, tgu_ch1, tgu_ch2, tgu_ch3, tmess_prng0, tmess_prng1,
> +            tmess_prng2, tmess_prng3, tsense_pwm1, tsense_pwm2, uim0_clk, uim0_data, uim0_present,
> +            uim0_reset, uim1_clk, uim1_data, uim1_present, uim1_reset, usb2phy_ac, usb_phy, vfr_0,
> +            vfr_1, vsense_trigger]
> +        drive-strength:
> +          enum: [2, 4, 6, 8, 10, 12, 14, 16]
> +          default: 2
> +          description:
> +            Selects the drive strength for the specified pins, in mA.
> +        bias-pull-down: true
> +        bias-pull-up: true
> +        bias-disable: true
> +        output-high: true
> +        output-low: true

You miss blank lines in several places. Please do it exactly like I
shown for sdm845.

> +      required:
> +        - pins
> +        - function
> +      additionalProperties: false
> +
> +examples:
> +  - |
> +    tlmm: pinctrl@03000000 {
> +      compatible = "qcom,qdu10000-tlmm";
> +      reg = <0x03000000 0xdc2000>;
> +      interrupts = <0 208 0>;

Use defines.

> +      gpio-controller;
> +      #gpio-cells = <2>;
> +      interrupt-controller;
> +      #interrupt-cells = <2>;
> +      wakeup-parent = <&pdc>;

Missing children.

> +    };

Best regards,
Krzysztof
Melody Olvera Oct. 4, 2022, 4:41 p.m. UTC | #2 
On 10/1/2022 4:20 AM, Krzysztof Kozlowski wrote:
> On 01/10/2022 05:05, 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,qdru1000-pinctrl.yaml        | 133 ++++++++++++++++++
>>  1 file changed, 133 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/pinctrl/qcom,qdru1000-pinctrl.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/pinctrl/qcom,qdru1000-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/qcom,qdru1000-pinctrl.yaml
>> new file mode 100644
>> index 000000000000..e8d938303231
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/pinctrl/qcom,qdru1000-pinctrl.yaml
>> @@ -0,0 +1,133 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/bindings/pinctrl/qcom,qdru1000-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.
>> +
>> +properties:
>> +  compatible:
>> +    items:
>> +      - const: qcom,qdu1000-tlmm
>> +      - const: qcom,qru1000-tlmm
>> +
>> +  reg:
>> +    items:
>> +      - description: Base address of TLMM register space
>> +      - description: Size of TLMM register space
>> +
>> +  interrupts:
>> +    minItems: 0
> Cannot be 0 of interrupts.
>
>> +    maxItems: 1
>> +    items:
>> +      - const: TLMM summary IRQ
>> +
>> +  interrupt-controller: true
>> +
>> +  '#interrupt-cells':
>> +    const: 2
>> +
>> +  gpio-controller: true
>> +
>> +  '#gpio-cells':
>> +    const: 2
>> +
>> +  wakeup-parent:
>> +    maxItems: 1
>> +    description:
>> +      Please refer to ../gpio/gpio.txt and ../interrupt-controller/interrupts.txt for
>> +      a general description of GPIO and interrupt bindings.
>> +
>> +      Please refer to pinctrl-bindings.txt in this directory for details of the
>> +      common pinctrl bindings used by client devices, including the meaning of the
>> +      phrase "pin configuration node".
>> +
>> +      The pin configuration nodes act as a container for an arbitrary number of
>> +      subnodes. Each of these subnodes represents some desired configuration for a
>> +      pin, a group, or a list of pins or groups. This configuration can include the
>> +      mux function to select on those pin(s)/group(s), and various pin configuration
>> +      parameters, such as pull-up, drive strength, etc.
>> +
>> +
>> +# PIN CONFIGURATION NODES
>> +patternPropetries:
>> +  '^.*$':
>> +    if:
>> +      type: object
>> +    then:
> Nope, that's not correct binding. It does not work. It never worked.
>
> Please do it exactly like:
> https://lore.kernel.org/linux-devicetree/20220930200529.331223-1-krzysztof.kozlowski@linaro.org/T/#m08b62ef5d873a52a5cbf3c53b25eff03726e7a16
> https://lore.kernel.org/linux-devicetree/20220927173702.5200-1-krzysztof.kozlowski@linaro.org/T/#t
Understood; will correct the binding.
>
>> +      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-9][0-9]|20[0-9])"
>> +              - enum: [ sdc2_clk, sdc2_cmd, sdc2_data, ufs_reset ]
>> +            minItems: 1
>> +            maxItems: 36
>> +        function:
>> +          description:
>> +            Specify the alternative function to be configured for the
>> +            specified pins. Functions are only valid for gpio pins.
>> +          enum: [gpio, aon_cam, atest_char, atest_char0, atest_char1, atest_char2, atest_char3,
>> +            atest_usb0, atest_usb00, atest_usb01, atest_usb02, atest_usb03, audio_ref, cam_mclk,
>> +            cci_async, cci_i2c, cci_timer0, cci_timer1, cci_timer2, cci_timer3, cci_timer4,
>> +            cmu_rng0, cmu_rng1, cmu_rng2, cmu_rng3, coex_uart1, coex_uart2, cri_trng, cri_trng0,
>> +            cri_trng1, dbg_out, ddr_bist, ddr_pxi0, ddr_pxi1, ddr_pxi2, ddr_pxi3, dp_hot, gcc_gp1,
>> +            gcc_gp2, gcc_gp3, ibi_i3c, jitter_bist, mdp_vsync, mdp_vsync0, mdp_vsync1, mdp_vsync2,
>> +            mdp_vsync3, mi2s0_data0, mi2s0_data1, mi2s0_sck, mi2s0_ws, mi2s2_data0, mi2s2_data1,
>> +            mi2s2_sck, mi2s2_ws, mss_grfc0, mss_grfc1, mss_grfc10, mss_grfc11, mss_grfc12,
>> +            mss_grfc2, mss_grfc3, mss_grfc4, mss_grfc5, mss_grfc6, mss_grfc7, mss_grfc8, mss_grfc9,
>> +            nav_0, nav_1, nav_2, pcie0_clkreqn, pcie1_clkreqn, phase_flag0, phase_flag1,
>> +            phase_flag10, phase_flag11, phase_flag12, phase_flag13, phase_flag14, phase_flag15,
>> +            phase_flag16, phase_flag17, phase_flag18, phase_flag19, phase_flag2, phase_flag20,
>> +            phase_flag21, phase_flag22, phase_flag23, phase_flag24, phase_flag25, phase_flag26,
>> +            phase_flag27, phase_flag28, phase_flag29, phase_flag3, phase_flag30, phase_flag31,
>> +            phase_flag4, phase_flag5, phase_flag6, phase_flag7, phase_flag8, phase_flag9, pll_bist,
>> +            pll_clk, pri_mi2s, prng_rosc0, prng_rosc1, prng_rosc2, prng_rosc3, qdss_cti, qdss_gpio,
>> +            qdss_gpio0, qdss_gpio1, qdss_gpio10, qdss_gpio11, qdss_gpio12, qdss_gpio13, qdss_gpio14,
>> +            qdss_gpio15, qdss_gpio2, qdss_gpio3, qdss_gpio4, qdss_gpio5, qdss_gpio6, qdss_gpio7,
>> +            qdss_gpio8, qdss_gpio9, qlink0_enable, qlink0_request, qlink0_wmss, qlink1_enable,
>> +            qlink1_request, qlink1_wmss, qlink2_enable, qlink2_request, qlink2_wmss, qspi0, qspi1,
>> +            qspi2, qspi3, qspi_clk, qspi_cs, qup0, qup1, qup10, qup11, qup12, qup13, qup14, qup15,
>> +            qup16, qup17, qup18, qup19, qup2, qup20, qup21, qup3, qup4, qup5, qup6, qup7, qup8,
>> +            qup9, qup_l4, qup_l5, qup_l6, sd_write, sdc40, sdc41, sdc42, sdc43, sdc4_clk, sdc4_cmd,
>> +            sec_mi2s, tb_trig, tgu_ch0, tgu_ch1, tgu_ch2, tgu_ch3, tmess_prng0, tmess_prng1,
>> +            tmess_prng2, tmess_prng3, tsense_pwm1, tsense_pwm2, uim0_clk, uim0_data, uim0_present,
>> +            uim0_reset, uim1_clk, uim1_data, uim1_present, uim1_reset, usb2phy_ac, usb_phy, vfr_0,
>> +            vfr_1, vsense_trigger]
>> +        drive-strength:
>> +          enum: [2, 4, 6, 8, 10, 12, 14, 16]
>> +          default: 2
>> +          description:
>> +            Selects the drive strength for the specified pins, in mA.
>> +        bias-pull-down: true
>> +        bias-pull-up: true
>> +        bias-disable: true
>> +        output-high: true
>> +        output-low: true
> You miss blank lines in several places. Please do it exactly like I
> shown for sdm845.
Ditto for above.
>
>> +      required:
>> +        - pins
>> +        - function
>> +      additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    tlmm: pinctrl@03000000 {
>> +      compatible = "qcom,qdu10000-tlmm";
>> +      reg = <0x03000000 0xdc2000>;
>> +      interrupts = <0 208 0>;
> Use defines.
Understood.
>
>> +      gpio-controller;
>> +      #gpio-cells = <2>;
>> +      interrupt-controller;
>> +      #interrupt-cells = <2>;
>> +      wakeup-parent = <&pdc>;
> Missing children.
Will add.
>
>> +    };
> Best regards,
> Krzysztof
Thanks,
Melody