Re: [PATCH V2 4/8] dt-bindings: pinctrl: qcom: Add ipq5018 pinctrl bindings

[Sricharan Ramabadhran]

On 6/22/2022 8:43 PM, Krzysztof Kozlowski wrote:
> On 21/06/2022 18:11, Sricharan R wrote:
> > From: Varadarajan Narayanan <quic_varada@xxxxxxxxxxx>
> >
> > Add device tree binding Documentation details for ipq5018
> > pinctrl driver.
> >
> > Co-developed-by: Nitheesh Sekar <quic_nsekar@xxxxxxxxxxx>
> > Co-developed-by: Sricharan R <quic_srichara@xxxxxxxxxxx>
> > Signed-off-by: Sricharan R <quic_srichara@xxxxxxxxxxx>
> > Signed-off-by: Nitheesh Sekar <quic_nsekar@xxxxxxxxxxx>
> > Signed-off-by: Varadarajan Narayanan <quic_varada@xxxxxxxxxxx>
> SoB should go after Co-developed.
>
> https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L473

 ok, will follow this.


> > ---
> >   .../pinctrl/qcom,ipq5018-pinctrl.yaml         | 145 ++++++++++++++++++
> >   1 file changed, 145 insertions(+)
> >   create mode 100644 Documentation/devicetree/bindings/pinctrl/qcom,ipq5018-pinctrl.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/pinctrl/qcom,ipq5018-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/qcom,ipq5018-pinctrl.yaml
> > new file mode 100644
> > index 000000000000..9b16c08bd127
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pinctrl/qcom,ipq5018-pinctrl.yaml
> > @@ -0,0 +1,145 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/pinctrl/qcom,ipq5018-pinctrl.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Qualcomm Technologies, Inc. IPQ5018 TLMM block
> > +
> > +maintainers:
> > +  - Varadarajan Narayanan <quic_varada@xxxxxxxxxxx>
> > +  - Sricharan R <quic_srichara@xxxxxxxxxxx>
> > +  - Nitheesh Sekar <quic_nsekar@xxxxxxxxxxx>
> > +
> > +description: |
> > +  This binding describes the Top Level Mode Multiplexer block found in the
> > +  IPQ5018 platform.
> > +
> > +properties:
> > +  compatible:
> > +    const: qcom,ipq5018-pinctrl
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    description: Specifies the TLMM summary IRQ
> > +    maxItems: 1
> > +
> > +  interrupt-controller: true
> > +
> > +  '#interrupt-cells':
> > +    description:
> > +      Specifies the PIN numbers and Flags, as defined in defined in
> > +      include/dt-bindings/interrupt-controller/irq.h
> > +    const: 2
> > +
> > +  gpio-controller: true
> > +
> > +  '#gpio-cells':
> > +    description: Specifying the pin number and flags, as defined in
> > +      include/dt-bindings/gpio/gpio.h
> > +    const: 2
> > +
> > +  gpio-ranges:
> > +    maxItems: 1
> > +
> > +#PIN CONFIGURATION NODES
> > +patternProperties:
> > +  '-pinmux$':
> > +    type: object
> > +    description:
> > +      Pinctrl node's client devices use subnodes for desired pin configuration.
> > +      Client device subnodes use below standard properties.
> > +    $ref: "/schemas/pinctrl/pincfg-node.yaml"
> No need for quotes

 ok.


> > +
> > +    properties:
> > +      pins:
> > +        description:
> > +          List of gpio pins affected by the properties specified in this
> > +          subnode.
> > +        items:
> > +          oneOf:
> > +            - pattern: "^gpio([1-9]|[1-7][0-9]|80)$"
> Use consistent quotes in the patch: either ' or ". Don't mix.

 ok.


> > +        minItems: 1
> > +        maxItems: 4
> > +
> > +      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_rxbclk, audio_rxd, audio_rxfsync,
> > +          audio_rxmclk, audio_txbclk, audio_txd, audio_txfsync, audio_txmclk,
> > +          blsp0_i2c, blsp0_spi, blsp0_uart0, blsp0_uart1, blsp1_i2c0,
> > +          blsp1_i2c1, blsp1_spi0, blsp1_spi1, blsp1_uart0, blsp1_uart1,
> > +          blsp1_uart2, blsp2_i2c0, blsp2_i2c1, blsp2_spi, blsp2_spi0,
> > +          blsp2_spi1, btss0, btss1, btss10, btss11, btss12, btss13, btss2,
> > +          btss3, btss4, btss5, btss6, btss7, btss8, btss9, burn0, burn1,
> > +          cri_trng, cri_trng0, cri_trng1, cxc_clk, cxc_data, dbg_out, eud_gpio,
> > +          gcc_plltest, gcc_tlmm, gpio, mac0, mac1, mdc, mdio, pcie0_clk,
> > +          pcie0_wake, pcie1_clk, pcie1_wake, pll_test, prng_rosc, pwm0, pwm1,
> > +          pwm2, pwm3, 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, qspi_clk, qspi_cs, qspi0, qspi1,
> > +          qspi2, qspi3, reset_out, sdc1_clk, sdc1_cmd, sdc10, sdc11, sdc12,
> > +          sdc13, wci0, wci1, wci2, wci3, wci4, wci5, wci6, wci7, wsa_swrm,
> > +          wsi_clk3, wsi_data3, wsis_reset, xfem0, xfem1, xfem2, xfem3, xfem4,
> > +          xfem5, xfem6, xfem7 ]
> > +
> > +      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
> > +
> > +    required:
> > +      - pins
> > +      - function
> > +
> > +    additionalProperties: false
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - interrupts
> > +  - interrupt-controller
> > +  - '#interrupt-cells'
> > +  - gpio-controller
> > +  - '#gpio-cells'
> > +  - gpio-ranges
> Missing allOf with reference to pinctrl schema.

  ok, will fix this.

Regards,
  Sricharan