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

From: Wells Lu 呂芳騰
Date: Thu Nov 18 2021 - 04:16:05 EST

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@xxxxxxxxxxx>
> > ---
> > 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@xxxxxxxxx>
> > + - Wells Lu <wells.lu@xxxxxxxxxxx>
> > +
> > +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@xxxxxxxxxxx>
> > L: linux-arm-kernel@xxxxxxxxxxxxxxxxxxx (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
> >
> >