All of lore.kernel.org
 help / color / mirror / Atom feed
From: Rob Herring <robh@kernel.org>
To: "Jonathan Neuschäfer" <j.neuschaefer@gmx.net>
Cc: linux-gpio@vger.kernel.org, devicetree@vger.kernel.org,
	Linus Walleij <linus.walleij@linaro.org>,
	openbmc@lists.ozlabs.org, Tomer Maimon <tmaimon77@gmail.com>,
	Joel Stanley <joel@jms.id.au>,
	linux-kernel@vger.kernel.org,
	Andy Shevchenko <andy.shevchenko@gmail.com>,
	Avi Fishman <avifishman70@gmail.com>,
	Tali Perry <tali.perry1@gmail.com>,
	Patrick Venture <venture@google.com>,
	Nancy Yuen <yuenn@google.com>,
	Benjamin Fair <benjaminfair@google.com>
Subject: Re: [PATCH v3 4/9] dt-bindings: pinctrl: Add Nuvoton WPCM450
Date: Tue, 4 Jan 2022 16:12:21 -0600	[thread overview]
Message-ID: <YdTGRY+n9XY522jg@robh.at.kernel.org> (raw)
In-Reply-To: <20211224200935.93817-5-j.neuschaefer@gmx.net>

On Fri, Dec 24, 2021 at 09:09:30PM +0100, Jonathan Neuschäfer wrote:
> This binding is heavily based on the one for NPCM7xx, because the
> hardware is similar. There are some notable differences, however:
> 
> - The addresses of GPIO banks are not physical addresses but simple
>   indices (0 to 7), because the GPIO registers are not laid out in
>   convenient blocks.
> - Pinmux settings can explicitly specify that the GPIO mode is used.
> 
> Certain pins support blink patterns in hardware. This is currently not
> modelled in the DT binding.
> 
> Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
> 
> 
> ---
> v3:
> - Make changes suggested by Rob Herring
> - Fix lint errors
> - Simplify child node patterns
> - Remove if/type=object/then trick
> - Reduce interrupts.maxItems to 3: 4 aren't necessary
> - Replace list of gpio0/1/2/etc. with pattern
> - Remove nuvoton,interrupt-map again, to simplify the binding
> - Make tuples clearer
> 
> v2:
> - https://lore.kernel.org/lkml/20211207210823.1975632-5-j.neuschaefer@gmx.net/
> - Move GPIO into subnodes
> - Improve use of quotes
> - Remove unnecessary minItems/maxItems lines
> - Remove "phandle: true"
> - Use separate prefixes for pinmux and pincfg nodes
> - Add nuvoton,interrupt-map property
> - Make it possible to set pinmux to GPIO explicitly
> 
> v1:
> - https://lore.kernel.org/lkml/20210602120329.2444672-5-j.neuschaefer@gmx.net/
> ---
>  .../pinctrl/nuvoton,wpcm450-pinctrl.yaml      | 159 ++++++++++++++++++
>  1 file changed, 159 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pinctrl/nuvoton,wpcm450-pinctrl.yaml
> 
> diff --git a/Documentation/devicetree/bindings/pinctrl/nuvoton,wpcm450-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/nuvoton,wpcm450-pinctrl.yaml
> new file mode 100644
> index 0000000000000..2d15737b5815e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pinctrl/nuvoton,wpcm450-pinctrl.yaml
> @@ -0,0 +1,159 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pinctrl/nuvoton,wpcm450-pinctrl.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Nuvoton WPCM450 pin control and GPIO
> +
> +maintainers:
> +  - Jonathan Neuschäfer <j.neuschaefer@gmx.net>
> +
> +properties:
> +  compatible:
> +    const: nuvoton,wpcm450-pinctrl
> +
> +  reg:
> +    maxItems: 1
> +
> +  '#address-cells':
> +    const: 1
> +
> +  '#size-cells':
> +    const: 0
> +
> +patternProperties:
> +  # There are three kinds of subnodes:
> +  # 1. a GPIO controller node for each GPIO bank
> +  # 2. a pinmux node configures pin muxing for a group of pins (e.g. rmii2)
> +  # 3. a pinconf node configures properties of a single pin
> +
> +  "^gpio":

'^gpio@[0-7]$'

> +    type: object
> +
> +    description:
> +      Eight GPIO banks (gpio@0 to gpio@7), that each contain between 14 and 18
> +      GPIOs. Some GPIOs support interrupts.
> +
> +    properties:
> +      reg:
> +        description: GPIO bank number (0-7)

reg:
  minimum: 0
  maximum: 7

But there's not an actual register address range you could use instead?

> +
> +      gpio-controller: true
> +
> +      "#gpio-cells":
> +        const: 2
> +
> +      interrupt-controller: true
> +
> +      "#interrupt-cells":
> +        const: 2
> +
> +      interrupts:
> +        maxItems: 3
> +        description:
> +          The interrupts associated with this GPIO bank
> +
> +    required:
> +      - reg
> +      - gpio-controller
> +      - '#gpio-cells'
> +
> +  "^mux-":
> +    $ref: pinmux-node.yaml#
> +
> +    properties:
> +      groups:
> +        description:
> +          One or more groups of pins to mux to a certain function
> +        items:
> +          enum: [ smb3, smb4, smb5, scs1, scs2, scs3, smb0, smb1, smb2, bsp,
> +                  hsp1, hsp2, r1err, r1md, rmii2, r2err, r2md, kbcc, dvo,
> +                  clko, smi, uinc, gspi, mben, xcs2, xcs1, sdio, sspi, fi0,
> +                  fi1, fi2, fi3, fi4, fi5, fi6, fi7, fi8, fi9, fi10, fi11,
> +                  fi12, fi13, fi14, fi15, pwm0, pwm1, pwm2, pwm3, pwm4, pwm5,
> +                  pwm6, pwm7, hg0, hg1, hg2, hg3, hg4, hg5, hg6, hg7 ]
> +      function:
> +        description:
> +          The function that a group of pins is muxed to
> +        enum: [ smb3, smb4, smb5, scs1, scs2, scs3, smb0, smb1, smb2, bsp,
> +                hsp1, hsp2, r1err, r1md, rmii2, r2err, r2md, kbcc, dvo0,
> +                dvo1, dvo2, dvo3, dvo4, dvo5, dvo6, dvo7, clko, smi, uinc,
> +                gspi, mben, xcs2, xcs1, sdio, sspi, fi0, fi1, fi2, fi3, fi4,
> +                fi5, fi6, fi7, fi8, fi9, fi10, fi11, fi12, fi13, fi14, fi15,
> +                pwm0, pwm1, pwm2, pwm3, pwm4, pwm5, pwm6, pwm7, hg0, hg1,
> +                hg2, hg3, hg4, hg5, hg6, hg7, gpio ]
> +
> +    dependencies:
> +      groups: [ function ]
> +      function: [ groups ]
> +
> +    additionalProperties: false
> +
> +  "^cfg-":
> +    $ref: pincfg-node.yaml#
> +
> +    properties:
> +      pins:
> +        description:
> +          A list of pins to configure in certain ways, such as enabling
> +          debouncing
> +        items:
> +          pattern: "^gpio1?[0-9]{1,2}$"
> +
> +      input-debounce: true
> +
> +    additionalProperties: false
> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    #include <dt-bindings/gpio/gpio.h>
> +    pinctrl: pinctrl@b8003000 {
> +      compatible = "nuvoton,wpcm450-pinctrl";
> +      reg = <0xb8003000 0x1000>;
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +
> +      gpio0: gpio@0 {
> +        reg = <0>;
> +        gpio-controller;
> +        #gpio-cells = <2>;
> +        interrupts = <2 IRQ_TYPE_LEVEL_HIGH>,
> +                     <3 IRQ_TYPE_LEVEL_HIGH>,
> +                     <4 IRQ_TYPE_LEVEL_HIGH>;
> +      };
> +
> +      mux-rmii2 {
> +        groups = "rmii2";
> +        function = "rmii2";
> +      };
> +
> +      pinmux_uid: mux-uid {
> +        groups = "gspi", "sspi";
> +        function = "gpio";
> +      };
> +
> +      pinctrl_uid: cfg-uid {
> +        pins = "gpio14";
> +        input-debounce = <1>;
> +      };
> +    };
> +
> +    gpio-keys {
> +      compatible = "gpio-keys";
> +      pinctrl-names = "default";
> +      pinctrl-0 = <&pinctrl_uid>, <&pinmux_uid>;
> +
> +      uid {
> +        label = "UID";
> +        linux,code = <102>;
> +        gpios = <&gpio0 14 GPIO_ACTIVE_HIGH>;
> +      };
> +    };
> --
> 2.30.2
> 
> 

WARNING: multiple messages have this Message-ID (diff)
From: Rob Herring <robh@kernel.org>
To: "Jonathan Neuschäfer" <j.neuschaefer@gmx.net>
Cc: devicetree@vger.kernel.org, Tomer Maimon <tmaimon77@gmail.com>,
	Avi Fishman <avifishman70@gmail.com>,
	Patrick Venture <venture@google.com>,
	Linus Walleij <linus.walleij@linaro.org>,
	linux-kernel@vger.kernel.org, Tali Perry <tali.perry1@gmail.com>,
	linux-gpio@vger.kernel.org,
	Andy Shevchenko <andy.shevchenko@gmail.com>,
	openbmc@lists.ozlabs.org, Benjamin Fair <benjaminfair@google.com>
Subject: Re: [PATCH v3 4/9] dt-bindings: pinctrl: Add Nuvoton WPCM450
Date: Tue, 4 Jan 2022 16:12:21 -0600	[thread overview]
Message-ID: <YdTGRY+n9XY522jg@robh.at.kernel.org> (raw)
In-Reply-To: <20211224200935.93817-5-j.neuschaefer@gmx.net>

On Fri, Dec 24, 2021 at 09:09:30PM +0100, Jonathan Neuschäfer wrote:
> This binding is heavily based on the one for NPCM7xx, because the
> hardware is similar. There are some notable differences, however:
> 
> - The addresses of GPIO banks are not physical addresses but simple
>   indices (0 to 7), because the GPIO registers are not laid out in
>   convenient blocks.
> - Pinmux settings can explicitly specify that the GPIO mode is used.
> 
> Certain pins support blink patterns in hardware. This is currently not
> modelled in the DT binding.
> 
> Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
> 
> 
> ---
> v3:
> - Make changes suggested by Rob Herring
> - Fix lint errors
> - Simplify child node patterns
> - Remove if/type=object/then trick
> - Reduce interrupts.maxItems to 3: 4 aren't necessary
> - Replace list of gpio0/1/2/etc. with pattern
> - Remove nuvoton,interrupt-map again, to simplify the binding
> - Make tuples clearer
> 
> v2:
> - https://lore.kernel.org/lkml/20211207210823.1975632-5-j.neuschaefer@gmx.net/
> - Move GPIO into subnodes
> - Improve use of quotes
> - Remove unnecessary minItems/maxItems lines
> - Remove "phandle: true"
> - Use separate prefixes for pinmux and pincfg nodes
> - Add nuvoton,interrupt-map property
> - Make it possible to set pinmux to GPIO explicitly
> 
> v1:
> - https://lore.kernel.org/lkml/20210602120329.2444672-5-j.neuschaefer@gmx.net/
> ---
>  .../pinctrl/nuvoton,wpcm450-pinctrl.yaml      | 159 ++++++++++++++++++
>  1 file changed, 159 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pinctrl/nuvoton,wpcm450-pinctrl.yaml
> 
> diff --git a/Documentation/devicetree/bindings/pinctrl/nuvoton,wpcm450-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/nuvoton,wpcm450-pinctrl.yaml
> new file mode 100644
> index 0000000000000..2d15737b5815e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pinctrl/nuvoton,wpcm450-pinctrl.yaml
> @@ -0,0 +1,159 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pinctrl/nuvoton,wpcm450-pinctrl.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Nuvoton WPCM450 pin control and GPIO
> +
> +maintainers:
> +  - Jonathan Neuschäfer <j.neuschaefer@gmx.net>
> +
> +properties:
> +  compatible:
> +    const: nuvoton,wpcm450-pinctrl
> +
> +  reg:
> +    maxItems: 1
> +
> +  '#address-cells':
> +    const: 1
> +
> +  '#size-cells':
> +    const: 0
> +
> +patternProperties:
> +  # There are three kinds of subnodes:
> +  # 1. a GPIO controller node for each GPIO bank
> +  # 2. a pinmux node configures pin muxing for a group of pins (e.g. rmii2)
> +  # 3. a pinconf node configures properties of a single pin
> +
> +  "^gpio":

'^gpio@[0-7]$'

> +    type: object
> +
> +    description:
> +      Eight GPIO banks (gpio@0 to gpio@7), that each contain between 14 and 18
> +      GPIOs. Some GPIOs support interrupts.
> +
> +    properties:
> +      reg:
> +        description: GPIO bank number (0-7)

reg:
  minimum: 0
  maximum: 7

But there's not an actual register address range you could use instead?

> +
> +      gpio-controller: true
> +
> +      "#gpio-cells":
> +        const: 2
> +
> +      interrupt-controller: true
> +
> +      "#interrupt-cells":
> +        const: 2
> +
> +      interrupts:
> +        maxItems: 3
> +        description:
> +          The interrupts associated with this GPIO bank
> +
> +    required:
> +      - reg
> +      - gpio-controller
> +      - '#gpio-cells'
> +
> +  "^mux-":
> +    $ref: pinmux-node.yaml#
> +
> +    properties:
> +      groups:
> +        description:
> +          One or more groups of pins to mux to a certain function
> +        items:
> +          enum: [ smb3, smb4, smb5, scs1, scs2, scs3, smb0, smb1, smb2, bsp,
> +                  hsp1, hsp2, r1err, r1md, rmii2, r2err, r2md, kbcc, dvo,
> +                  clko, smi, uinc, gspi, mben, xcs2, xcs1, sdio, sspi, fi0,
> +                  fi1, fi2, fi3, fi4, fi5, fi6, fi7, fi8, fi9, fi10, fi11,
> +                  fi12, fi13, fi14, fi15, pwm0, pwm1, pwm2, pwm3, pwm4, pwm5,
> +                  pwm6, pwm7, hg0, hg1, hg2, hg3, hg4, hg5, hg6, hg7 ]
> +      function:
> +        description:
> +          The function that a group of pins is muxed to
> +        enum: [ smb3, smb4, smb5, scs1, scs2, scs3, smb0, smb1, smb2, bsp,
> +                hsp1, hsp2, r1err, r1md, rmii2, r2err, r2md, kbcc, dvo0,
> +                dvo1, dvo2, dvo3, dvo4, dvo5, dvo6, dvo7, clko, smi, uinc,
> +                gspi, mben, xcs2, xcs1, sdio, sspi, fi0, fi1, fi2, fi3, fi4,
> +                fi5, fi6, fi7, fi8, fi9, fi10, fi11, fi12, fi13, fi14, fi15,
> +                pwm0, pwm1, pwm2, pwm3, pwm4, pwm5, pwm6, pwm7, hg0, hg1,
> +                hg2, hg3, hg4, hg5, hg6, hg7, gpio ]
> +
> +    dependencies:
> +      groups: [ function ]
> +      function: [ groups ]
> +
> +    additionalProperties: false
> +
> +  "^cfg-":
> +    $ref: pincfg-node.yaml#
> +
> +    properties:
> +      pins:
> +        description:
> +          A list of pins to configure in certain ways, such as enabling
> +          debouncing
> +        items:
> +          pattern: "^gpio1?[0-9]{1,2}$"
> +
> +      input-debounce: true
> +
> +    additionalProperties: false
> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    #include <dt-bindings/gpio/gpio.h>
> +    pinctrl: pinctrl@b8003000 {
> +      compatible = "nuvoton,wpcm450-pinctrl";
> +      reg = <0xb8003000 0x1000>;
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +
> +      gpio0: gpio@0 {
> +        reg = <0>;
> +        gpio-controller;
> +        #gpio-cells = <2>;
> +        interrupts = <2 IRQ_TYPE_LEVEL_HIGH>,
> +                     <3 IRQ_TYPE_LEVEL_HIGH>,
> +                     <4 IRQ_TYPE_LEVEL_HIGH>;
> +      };
> +
> +      mux-rmii2 {
> +        groups = "rmii2";
> +        function = "rmii2";
> +      };
> +
> +      pinmux_uid: mux-uid {
> +        groups = "gspi", "sspi";
> +        function = "gpio";
> +      };
> +
> +      pinctrl_uid: cfg-uid {
> +        pins = "gpio14";
> +        input-debounce = <1>;
> +      };
> +    };
> +
> +    gpio-keys {
> +      compatible = "gpio-keys";
> +      pinctrl-names = "default";
> +      pinctrl-0 = <&pinctrl_uid>, <&pinmux_uid>;
> +
> +      uid {
> +        label = "UID";
> +        linux,code = <102>;
> +        gpios = <&gpio0 14 GPIO_ACTIVE_HIGH>;
> +      };
> +    };
> --
> 2.30.2
> 
> 

  reply	other threads:[~2022-01-04 22:12 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-24 20:09 [PATCH v3 0/9] Nuvoton WPCM450 pinctrl and GPIO driver Jonathan Neuschäfer
2021-12-24 20:09 ` Jonathan Neuschäfer
2021-12-24 20:09 ` [PATCH v3 1/9] dt-bindings: arm/npcm: Add binding for global control registers (GCR) Jonathan Neuschäfer
2021-12-24 20:09   ` Jonathan Neuschäfer
2022-01-04 22:07   ` Rob Herring
2022-01-04 22:07     ` Rob Herring
2021-12-24 20:09 ` [PATCH v3 2/9] MAINTAINERS: Match all of bindings/arm/npcm/ as part of NPCM architecture Jonathan Neuschäfer
2021-12-24 20:09   ` Jonathan Neuschäfer
2021-12-24 20:09 ` [PATCH v3 3/9] ARM: dts: wpcm450: Add global control registers (GCR) node Jonathan Neuschäfer
2021-12-24 20:09   ` Jonathan Neuschäfer
2021-12-24 20:09 ` [PATCH v3 4/9] dt-bindings: pinctrl: Add Nuvoton WPCM450 Jonathan Neuschäfer
2021-12-24 20:09   ` Jonathan Neuschäfer
2022-01-04 22:12   ` Rob Herring [this message]
2022-01-04 22:12     ` Rob Herring
2022-01-05 14:40     ` Jonathan Neuschäfer
2022-01-05 14:40       ` Jonathan Neuschäfer
2021-12-24 20:09 ` [PATCH v3 5/9] pinctrl: nuvoton: Add driver for WPCM450 Jonathan Neuschäfer
2021-12-24 20:09   ` Jonathan Neuschäfer
2021-12-24 21:15   ` Andy Shevchenko
2021-12-24 21:15     ` Andy Shevchenko
2022-01-05 14:24     ` Jonathan Neuschäfer
2022-01-05 14:24       ` Jonathan Neuschäfer
2021-12-24 20:09 ` [PATCH v3 6/9] ARM: dts: wpcm450: Add pinctrl and GPIO nodes Jonathan Neuschäfer
2021-12-24 20:09   ` Jonathan Neuschäfer
2021-12-24 20:09 ` [PATCH v3 7/9] ARM: dts: wpcm450: Add pin functions Jonathan Neuschäfer
2021-12-24 20:09   ` Jonathan Neuschäfer
2021-12-24 20:09 ` [PATCH v3 8/9] ARM: dts: wpcm450-supermicro-x9sci-ln4f: Add GPIO LEDs and buttons Jonathan Neuschäfer
2021-12-24 20:09   ` Jonathan Neuschäfer
2021-12-24 20:09 ` [PATCH v3 9/9] ARM: dts: wpcm450: Add pinmux information to UART0 Jonathan Neuschäfer
2021-12-24 20:09   ` Jonathan Neuschäfer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YdTGRY+n9XY522jg@robh.at.kernel.org \
    --to=robh@kernel.org \
    --cc=andy.shevchenko@gmail.com \
    --cc=avifishman70@gmail.com \
    --cc=benjaminfair@google.com \
    --cc=devicetree@vger.kernel.org \
    --cc=j.neuschaefer@gmx.net \
    --cc=joel@jms.id.au \
    --cc=linus.walleij@linaro.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=openbmc@lists.ozlabs.org \
    --cc=tali.perry1@gmail.com \
    --cc=tmaimon77@gmail.com \
    --cc=venture@google.com \
    --cc=yuenn@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.