Re: [PATCH V6 3/7] dt-bindings: pinctrl: qcom: Add support for IPQ9574

[Devi Priya <quic_devipriy@xxxxxxxxxxx>]

Thanks for taking time to review the patch

On 2/3/2023 11:20 AM, Kathiravan T wrote:
> On 2/2/2023 8:36 PM, Devi Priya wrote:
> > Add new binding document for pinctrl on IPQ9574
> >
> > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx>
> > Co-developed-by: Anusha Rao <quic_anusha@xxxxxxxxxxx>
> > Signed-off-by: Anusha Rao <quic_anusha@xxxxxxxxxxx>
> > Signed-off-by: Devi Priya <quic_devipriy@xxxxxxxxxxx>
> > ---
> >   .../bindings/pinctrl/qcom,ipq9574-tlmm.yaml   | 137 ++++++++++++++++++
> >   1 file changed, 137 insertions(+)
> >   create mode 100644 
> > Documentation/devicetree/bindings/pinctrl/qcom,ipq9574-tlmm.yaml
> >
> > diff --git 
> > a/Documentation/devicetree/bindings/pinctrl/qcom,ipq9574-tlmm.yaml 
> > b/Documentation/devicetree/bindings/pinctrl/qcom,ipq9574-tlmm.yaml
> > new file mode 100644
> > index 000000000000..053b15ccb7a5
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pinctrl/qcom,ipq9574-tlmm.yaml
> > @@ -0,0 +1,137 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/pinctrl/qcom,ipq9574-tlmm.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Qualcomm Technologies, Inc. IPQ9574 TLMM block
> > +
> > +maintainers:
> > +  - Bjorn Andersson <andersson@xxxxxxxxxx>
> > +  - Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx>
> > +
> > +description:
> > +  Top Level Mode Multiplexer pin controller in Qualcomm IPQ9574 SoC.
> > +
> > +properties:
> > +  compatible:
> > +    const: qcom,ipq9574-tlmm
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  interrupt-controller: true
> > +  "#interrupt-cells": true
> > +  gpio-controller: true
> > +  "#gpio-cells": true
> > +  gpio-ranges: true
> > +  wakeup-parent: true
> > +
> > +  gpio-reserved-ranges:
> > +    minItems: 1
> > +    maxItems: 33
> > +
> > +  gpio-line-names:
> > +    maxItems: 65
> > +
> > +patternProperties:
> > +  "-state$":
> > +    oneOf:
> > +      - $ref: "#/$defs/qcom-ipq9574-tlmm-state"
> > +      - patternProperties:
> > +          "-pins$":
> > +            $ref: "#/$defs/qcom-ipq9574-tlmm-state"
> > +        additionalProperties: false
> > +
> > +$defs:
> > +  qcom-ipq9574-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-5][0-9]|6[0-4])$"
> > +            - enum: [ qdsd_cmd, qdsd_data0, qdsd_data1, qdsd_data2, 
> > qdsd_cmd,
> > +                      qdsd_data0, qdsd_data1, qdsd_data2, qdsd_data3, 
> > sdc1_clk,
> > +                      sdc1_cmd, sdc1_data, sdc2_clk, sdc2_cmd, 
> > sdc2_data ]
> > +        minItems: 1
> > +        maxItems: 8
> > +
> > +      function:
> > +        description:
> > +          Specify the alternative function to be configured for the 
> > specified
> > +          pins.
> > +
> > +        enum: [ atest_char, atest_char0, atest_char1, atest_char2, 
> > atest_char3,
> > +                audio_pdm0, audio_pdm1, audio_pri, audio_sec, 
> > blsp0_spi, blsp0_uart,
> > +                blsp1_i2c, blsp1_spi, blsp1_uart, blsp2_i2c, blsp2_spi,
> > +                blsp2_uart, blsp3_i2c, blsp3_spi, blsp3_uart, blsp4_i2c,
> > +                blsp4_spi, blsp4_uart, blsp5_i2c, blsp5_uart, cri_trng0,
> > +                cri_trng1, cri_trng3, cxc0, cxc1, dbg_out, dwc_ddrphy,
> > +                gcc_plltest, gcc_tlmm, mac00, mac01, mac10, mac11, mdc,
> > +                mdio, pcie0_clk, pcie0_wake, pcie1_clk, pcie1_wake, 
> > pcie2_clk,
> > +                pcie2_wake, pcie3_clk, pcie3_wake, prng_rosc0, 
> > prng_rosc1,
> > +                prng_rosc2, prng_rosc3, pta1_0, pta1_1, pta1_2, 
> > pta20, pta21,
> > +                pwm00, pwm01, pwm02, pwm03, pwm04, pwm10, pwm11, 
> > pwm12, pwm13,
> > +                pwm14, pwm20, pwm21, pwm22, pwm23, pwm24, pwm30, 
> > pwm31, pwm32,
>
>
> Given that, In driver everything is grouped under a single name 'pwm', 
> are these pwm function names as still valid?
Yes correct.
Have updated the bindings accordingly in V7
> > +                pwm33, qdss_cti_trig_in_a0, qdss_cti_trig_in_a1, 
> > qdss_cti_trig_in_b0,
> > +                qdss_cti_trig_in_b1, qdss_cti_trig_out_a0, 
> > qdss_cti_trig_out_a1,
> > +                qdss_cti_trig_out_b0, qdss_cti_trig_out_b1, 
> > qdss_traceclk_a,
> > +                qdss_traceclk_b, qdss_tracectl_a, qdss_tracectl_b, 
> > qdss_tracedata_a,
> > +                qdss_tracedata_b, qdss_tracedata_b, qspi_clk, 
> > qspi_cs, qspi_data,
> > +                rx0, rx1, sdc_clk, sdc_cmd, sdc_data, sdc_rclk, 
> > tsens_max,
> > +                wci20, wci21, wsa_swrm ]
> > +
> > +      bias-pull-down: true
> > +      bias-pull-up: true
> > +      bias-disable: true
> > +      drive-strength: true
> > +      input-enable: true
> > +      output-high: true
> > +      output-low: true
> > +
> > +    required:
> > +      - pins
> > +
> > +    additionalProperties: false
> > +
> > +allOf:
> > +  - $ref: /schemas/pinctrl/qcom,tlmm-common.yaml#
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +    tlmm: pinctrl@1000000 {
> > +        compatible = "qcom,ipq9574-tlmm";
> > +        reg = <0x01000000 0x300000>;
> > +        interrupts = <GIC_SPI 208 IRQ_TYPE_LEVEL_HIGH>;
> > +        gpio-controller;
> > +        #gpio-cells = <2>;
> > +        interrupt-controller;
> > +        #interrupt-cells = <2>;
> > +        gpio-ranges = <&tlmm 0 0 65>;
> > +
> > +        uart2-state {
> > +            pins = "gpio34", "gpio35";
> > +            function = "blsp2_uart";
> > +            drive-strength = <8>;
> > +            bias-pull-down;
> > +        };
> > +    };
Best Regards,
Devi Priya