RE: [PATCH v2 3/3] devicetree: bindings: pinctrl: Add bindings doc for Sunplus SP7021.

["Wells Lu 呂芳騰" <wells.lu@sunplus.com>]

Hi,

Thanks for your review.


> On Mon, Nov 01, 2021 at 04:11:17PM +0800, Wells Lu wrote:
> > Add bindings documentation for Sunplus SP7021.
> 
> Patch 2 and 3 can be combined. Use consistent subjects. Patch 2 is good.
> This one is not.

Yes, I'll combine patch 2 and 3 into a single patch in next patch series.


> >
> > Signed-off-by: Wells Lu <wells.lu@sunplus.com>
> > ---
> > Changes in v2:
> >  - None
> >
> >  .../bindings/pinctrl/sunplus,sp7021-pinctrl.yaml   | 277 +++++++++++++++++++++
> >  MAINTAINERS                                        |   1 +
> >  2 files changed, 278 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/pinctrl/sunplus,sp7021-pinctrl.yaml
> >
> > diff --git
> > a/Documentation/devicetree/bindings/pinctrl/sunplus,sp7021-pinctrl.yam
> > l
> > b/Documentation/devicetree/bindings/pinctrl/sunplus,sp7021-pinctrl.yam
> > l
> > new file mode 100644
> > index 0000000..7cfa0ce
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pinctrl/sunplus,sp7021-pinctrl
> > +++ .yaml
> > @@ -0,0 +1,277 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) # Copyright
> > +(C) Sunplus Co., Ltd. 2021 %YAML 1.2
> > +---
> > +$id:
> > +http://devicetree.org/schemas/pinctrl/sunplus,sp7021-pinctrl.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Sunplus SP7021 Pin Controller Device Tree Bindings
> > +
> > +maintainers:
> > +  - Dvorkin Dmitry <dvorkin@tibbo.com>
> > +  - Wells Lu <wells.lu@sunplus.com>
> > +
> > +description: |
> > +  The Sunplus SP7021 pin controller is used to control SoC pins.
> > +Please
> > +  refer to pinctrl-bindings.txt in this directory for details of the
> > +common
> > +  pinctrl bindings used by client devices.
> > +
> > +  Refer to https://sunplus-tibbo.atlassian.net/wiki/spaces/doc/pages/
> > +  1443495991/How+to+setup+pins+of+SP7021+in+device-tree+source
> > +
> > +  The device node of pin controller of Sunplus SP7021 has following
> > + properties.
> > +
> > +properties:
> > +  compatible:
> > +    const: sunplus,sp7021-pctl
> > +
> > +  gpio-controller: true
> > +
> > +  '#gpio-cells':
> > +    const: 2
> > +
> > +  reg:
> > +    items:
> > +      - description: Base address and length of the MOON2 registers.
> > +      - description: Base address and length of the GPIOXT registers.
> > +      - description: Base address and length of the GPIOXT2 registers.
> > +      - description: Base address and length of the FIRST registers.
> > +      - description: Base address and length of the MOON1 registers.
> > +
> > +  clocks:
> > +    maxItems: 1
> > +
> > +  resets:
> > +    maxItems: 1
> > +
> > +patternProperties:
> > +  '^.*$':
> > +    if:
> > +      type: object
> > +    then:
> 
> For new bindings, don't use this hack. Make the node name something you can match on (e.g.
> '-pins$').

Yes, I'll modify the regular expression to '-pins$' in next patch.
Sub-nodes of pinctrl node will look like:

	uart0-pins {
		...
	};

	emmc-pins {
		...
	};

> > +      description: |
> > +        A pinctrl node should contain at least one subnodes representing the
> > +        pins or function-pins group available on the machine. Each subnode
> > +        will list the pins it needs, and how they should be configured.
> > +
> > +        Pinctrl node's client devices use subnodes for desired pin
> > +        configuration. Client device subnodes use below standard properties.
> > +
> > +      properties:
> > +        pins:
> > +          description: |
> > +            Define pins which are used by pinctrl node's client device.
> > +
> > +            It consists of one or more integers which represents the config
> > +            setting for corresponding pin. Please use macro SPPCTL_IOPAD to
> > +            define the integers for pins.
> > +
> > +            The first argument of the macro is pin number, the second is pin
> > +            type, the third is type of GPIO, the last is default output state
> > +            of GPIO.
> > +          $ref: /schemas/types.yaml#/definitions/uint32-array
> > +
> > +        function:
> > +          description: |
> > +            Define pin-function which is used by pinctrl node's client device.
> > +            The name should be one of string in the following enumeration.
> > +          $ref: "/schemas/types.yaml#/definitions/string"
> > +          enum: [ SPI_FLASH, SPI_FLASH_4BIT, SPI_NAND, CARD0_EMMC, SD_CARD,
> > +                  UA0, FPGA_IFX, HDMI_TX, LCDIF, USB0_OTG, USB1_OTG ]
> > +
> > +        groups:
> > +          description: |
> > +            Define pin-group in a specified pin-function.
> > +            The name should be one of string in the following enumeration.
> > +          $ref: "/schemas/types.yaml#/definitions/string"
> > +          enum: [ SPI_FLASH1, SPI_FLASH2, SPI_FLASH_4BIT1, SPI_FLASH_4BIT2,
> > +                  SPI_NAND, CARD0_EMMC, SD_CARD, UA0, FPGA_IFX, HDMI_TX1,
> > +                  HDMI_TX2, HDMI_TX3, LCDIF, USB0_OTG, USB1_OTG ]
> > +
> > +        zero_func:
> > +          description: |
> > +            Disabled pins which are not used by pinctrl node's client device.
> > +          $ref: /schemas/types.yaml#/definitions/uint32-array
> > +
> > +      additionalProperties: false
> > +
> > +      allOf:
> > +        - if:
> > +            properties:
> > +              function:
> > +                enum:
> > +                  - SPI_FLASH
> > +          then:
> > +            properties:
> > +              groups:
> > +                enum:
> > +                  - SPI_FLASH1
> > +                  - SPI_FLASH2
> > +        - if:
> > +            properties:
> > +              function:
> > +                enum:
> > +                  - SPI_FLASH_4BIT
> > +          then:
> > +            properties:
> > +              groups:
> > +                enum:
> > +                  - SPI_FLASH_4BIT1
> > +                  - SPI_FLASH_4BIT2
> > +        - if:
> > +            properties:
> > +              function:
> > +                enum:
> > +                  - SPI_NAND
> > +          then:
> > +            properties:
> > +              groups:
> > +                enum:
> > +                  - SPI_NAND
> > +        - if:
> > +            properties:
> > +              function:
> > +                enum:
> > +                  - CARD0_EMMC
> > +          then:
> > +            properties:
> > +              groups:
> > +                enum:
> > +                  - CARD0_EMMC
> > +        - if:
> > +            properties:
> > +              function:
> > +                enum:
> > +                  - SD_CARD
> > +          then:
> > +            properties:
> > +              groups:
> > +                enum:
> > +                  - SD_CARD
> > +        - if:
> > +            properties:
> > +              function:
> > +                enum:
> > +                  - UA0
> > +          then:
> > +            properties:
> > +              groups:
> > +                enum:
> > +                  - UA0
> > +        - if:
> > +            properties:
> > +              function:
> > +                enum:
> > +                  - FPGA_IFX
> > +          then:
> > +            properties:
> > +              groups:
> > +                enum:
> > +                  - FPGA_IFX
> > +        - if:
> > +            properties:
> > +              function:
> > +                enum:
> > +                  - HDMI_TX
> > +          then:
> > +            properties:
> > +              groups:
> > +                enum:
> > +                  - HDMI_TX1
> > +                  - HDMI_TX2
> > +                  - HDMI_TX3
> > +        - if:
> > +            properties:
> > +              function:
> > +                enum:
> > +                  - LCDIF
> > +          then:
> > +            properties:
> > +              groups:
> > +                enum:
> > +                  - LCDIF
> > +        - if:
> > +            properties:
> > +              function:
> > +                enum:
> > +                  - USB0_OTG
> > +          then:
> > +            properties:
> > +              groups:
> > +                enum:
> > +                  - USB0_OTG
> > +        - if:
> > +            properties:
> > +              function:
> > +                enum:
> > +                  - USB1_OTG
> > +          then:
> > +            properties:
> > +              groups:
> > +                enum:
> > +                  - USB1_OTG
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - "#gpio-cells"
> > +  - gpio-controller
> > +  - clocks
> > +  - resets
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/clock/sp-sp7021.h>
> > +    #include <dt-bindings/reset/sp-sp7021.h>
> > +    #include <dt-bindings/pinctrl/sppctl-sp7021.h>
> > +
> > +    pctl: pctl@9C000100 {
> 
> pinctl@9c000100

Yes, I'll modify node name of pin-ctrl to 'pinctl@9c000100' in next patch.


> > +        compatible = "sunplus,sp7021-pctl";
> > +        reg = <0x9C000100 0x100>, <0x9C000300 0x80>, <0x9C000380 0x80>,
> > +              <0x9C0032e4 0x1C>, <0x9C000080 0x20>;
> > +        gpio-controller;
> > +        #gpio-cells = <2>;
> > +        clocks = <&clkc GPIO>;
> > +        resets = <&rstc RST_GPIO>;
> > +
> > +        pins_uart0: pins_uart0 {
> > +            function = "UA0";
> > +            groups = "UA0";
> > +        };
> > +
> > +        pins_uart1: pins_uart1 {
> > +            pins = <
> > +                SPPCTL_IOPAD(11,SPPCTL_PCTL_G_PMUX,MUXF_UA1_TX,0)
> > +                SPPCTL_IOPAD(10,SPPCTL_PCTL_G_PMUX,MUXF_UA1_RX,0)
> > +                SPPCTL_IOPAD(7,SPPCTL_PCTL_G_GPIO,0,SPPCTL_PCTL_L_OUT)
> > +            >;
> > +        };
> > +
> > +        emmc_mux: emmc_mux {
> > +            function = "CARD0_EMMC";
> > +            groups = "CARD0_EMMC";
> > +        };
> > +
> > +        mmc1_mux: mmc1_mux {
> > +            function = "SD_CARD";
> > +            groups = "SD_CARD";
> > +            pins = < SPPCTL_IOPAD(91,SPPCTL_PCTL_G_GPIO,0,0) >;
> > +        };
> > +
> > +        hdmi_A_tx1: hdmi_A_tx1_pins {
> > +            function = "HDMI_TX";
> > +            groups = "HDMI_TX1";
> > +        };
> > +        hdmi_A_tx2: hdmi_A_tx2_pins {
> > +            function = "HDMI_TX";
> > +            groups = "HDMI_TX2";
> > +        };
> > +        hdmi_A_tx3: hdmi_A_tx3_pins {
> > +            function = "HDMI_TX";
> > +            groups = "HDMI_TX3";
> > +        };
> > +    };
> > +...
> > diff --git a/MAINTAINERS b/MAINTAINERS index da6378f..11835e7 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -14872,6 +14872,7 @@ M:	Wells Lu <wells.lu@sunplus.com>
> >  L:	linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
> >  S:	Maintained
> >  W:	https://sunplus-tibbo.atlassian.net/wiki/spaces/doc/overview
> > +F:	Documentation/devicetree/bindings/pinctrl/sunplus,*
> >  F:	drivers/pinctrl/sunplus/
> >  F:	include/dt-bindings/pinctrl/sppctl*
> >
> > --
> > 2.7.4
> >
> >