[PATCH 00/23] Rework Nomadik GPIO to add Mobileye EyeQ5 support

[PATCH 00/23] Rework Nomadik GPIO to add Mobileye EyeQ5 support

[Théo Lebrun]

Hi,

This patch series reworks the Nomadik GPIO driver to bring it up to date
to current kernel standards. We then add Mobileye EyeQ5 support that
uses the same IP block but with limited functionality. We also add
features required by our newly supported platform:

 - Dynamic GPIO ID allocation;
 - Make clock optional;
 - Shared IRQ (usecase: EyeQ5 has two banks using the same IRQ);
 - Handle variadic GPIO counts (usecase: EyeQ5 has <32 GPIOs per bank);
 - Grab optional reset at probe (usecase: EyeQ5 has a reset available).

This GPIO platform driver was previously declared & registered inside
drivers/pinctrl/nomadik/pinctrl-nomadik.c, side-by-side with the
pinctrl driver. Both are tightly integrated, mostly for muxing reasons.
Now that gpio-nomadik is used for another platform, we loosen the
relationship. The behavior should not change on already supported
hardware but I do not have Nomadik hardware to test for that.

We have some dependencies, kept neatly to the end. Those are:
- The base platform support series from Grégory [1]. This relates to the
  last four patches (20 thru 23), ie defconfig and devicetree.
- The OLB syscon support series [0]. It provides reset and pinctrl nodes
  inside the devicetree. This relates to the last two patches (22 and
  23), ie resets and gpio-ranges DT props. GPIO works fine without it
  if patches 22 and 23 are dropped.

This has been tested on the EyeQ5 hardware, with the two parent series
applied. It also works fine without the OLB syscon series when our last
two patches are removed. It has been built on both Arm defconfigs that
rely on pinctrl-nomadik: nhk8815_defconfig and u8500_defconfig. I don't
have any Nomadik hardware to test though.

Have a nice day,
Théo

[0]: https://lore.kernel.org/lkml/20240212-mbly-clk-v6-0-c46fa1f93839@bootlin.com/
[1]: https://lore.kernel.org/lkml/20240205153503.574468-1-gregory.clement@bootlin.com/

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
Théo Lebrun (23):
      dt-bindings: gpio: nomadik: convert into yaml format
      dt-bindings: gpio: nomadik: add optional ngpios property
      dt-bindings: gpio: nomadik: add mobileye,eyeq5-gpio compatible
      dt-bindings: gpio: nomadik: add optional reset property
      gpio: nomadik: extract GPIO platform driver from drivers/pinctrl/nomadik/
      pinctrl: nomadik: fix build warning (-Wformat)
      pinctrl: nomadik: fix build warning (-Wpointer-to-int-cast)
      pinctrl: nomadik: minimise indentation in probe
      pinctrl: nomadik: follow type-system kernel coding conventions
      pinctrl: nomadik: follow whitespace kernel coding conventions
      pinctrl: nomadik: follow conditional kernel coding conventions
      gpio: nomadik: request dynamic ID allocation
      gpio: nomadik: fix offset bug in nmk_pmx_set()
      gpio: nomadik: make clock optional
      gpio: nomadik: change driver name from gpio to gpio-nomadik
      gpio: nomadik: support shared GPIO IRQs
      gpio: nomadik: handle variadic GPIO count
      gpio: nomadik: support mobileye,eyeq5-gpio
      gpio: nomadik: grab optional reset control and deassert it at probe
      MIPS: eyeq5_defconfig: enable GPIO by default
      MIPS: mobileye: eyeq5: add two GPIO bank nodes
      MIPS: mobileye: eyeq5: add resets to GPIO banks
      MIPS: mobileye: eyeq5: map GPIOs to pins using gpio-ranges

 .../devicetree/bindings/gpio/gpio-nmk.txt          |  31 -
 .../devicetree/bindings/gpio/st,nomadik-gpio.yaml  |  96 +++
 MAINTAINERS                                        |   2 +
 arch/mips/boot/dts/mobileye/eyeq5.dtsi             |  30 +
 arch/mips/configs/eyeq5_defconfig                  |   2 +
 drivers/gpio/Kconfig                               |  13 +
 drivers/gpio/Makefile                              |   1 +
 drivers/gpio/gpio-nomadik.c                        | 725 ++++++++++++++++
 drivers/pinctrl/nomadik/Kconfig                    |   5 +-
 drivers/pinctrl/nomadik/pinctrl-nomadik-db8500.c   |   3 +-
 drivers/pinctrl/nomadik/pinctrl-nomadik-stn8815.c  |   3 +-
 drivers/pinctrl/nomadik/pinctrl-nomadik.c          | 938 +++------------------
 .../linux/gpio/gpio-nomadik.h                      | 124 ++-
 13 files changed, 1118 insertions(+), 855 deletions(-)
---
base-commit: d55aa725e32849f709b61eab3b7a50b810a71a84
change-id: 20231023-mbly-gpio-a30571ec3283

Best regards,
-- 
Théo Lebrun <theo.lebrun@bootlin.com>

[PATCH 01/23] dt-bindings: gpio: nomadik: convert into yaml format

[Théo Lebrun]

Create gpio/st,nomadik-gpio.yaml json-schema dt-bindings file as a
direct translation from gpio-nmk.txt. Remove the txt file.

Add clocks and gpio-ranges properties which were missing and are being
used in Nomadik devicetrees.

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 .../devicetree/bindings/gpio/gpio-nmk.txt          | 31 ---------
 .../devicetree/bindings/gpio/st,nomadik-gpio.yaml  | 75 ++++++++++++++++++++++
 MAINTAINERS                                        |  1 +
 3 files changed, 76 insertions(+), 31 deletions(-)

diff --git a/Documentation/devicetree/bindings/gpio/gpio-nmk.txt b/Documentation/devicetree/bindings/gpio/gpio-nmk.txt
deleted file mode 100644
index 8315ac7780ef..000000000000
--- a/Documentation/devicetree/bindings/gpio/gpio-nmk.txt
+++ /dev/null
@@ -1,31 +0,0 @@
-Nomadik GPIO controller
-
-Required properties:
-- compatible            : Should be "st,nomadik-gpio".
-- reg                   : Physical base address and length of the controller's registers.
-- interrupts            : The interrupt outputs from the controller.
-- #gpio-cells           : Should be two:
-                            The first cell is the pin number.
-                            The second cell is used to specify optional parameters:
-                              - bits[3:0] trigger type and level flags:
-                                  1 = low-to-high edge triggered.
-                                  2 = high-to-low edge triggered.
-                                  4 = active high level-sensitive.
-                                  8 = active low level-sensitive.
-- gpio-controller       : Marks the device node as a GPIO controller.
-- interrupt-controller  : Marks the device node as an interrupt controller.
-- gpio-bank             : Specifies which bank a controller owns.
-- st,supports-sleepmode : Specifies whether controller can sleep or not
-
-Example:
-
-                gpio1: gpio@8012e080 {
-                        compatible = "st,nomadik-gpio";
-                        reg =  <0x8012e080 0x80>;
-                        interrupts = <0 120 0x4>;
-                        #gpio-cells = <2>;
-                        gpio-controller;
-                        interrupt-controller;
-                        st,supports-sleepmode;
-                        gpio-bank = <1>;
-                };
diff --git a/Documentation/devicetree/bindings/gpio/st,nomadik-gpio.yaml b/Documentation/devicetree/bindings/gpio/st,nomadik-gpio.yaml
new file mode 100644
index 000000000000..a999908173d2
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/st,nomadik-gpio.yaml
@@ -0,0 +1,75 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/gpio/st,nomadik-gpio.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Nomadik GPIO controller
+
+description:
+  The Nomadik GPIO driver handles Nomadik SoC GPIO blocks. This block has also
+  been called ST STA2X11. On the Nomadik platform, this driver is intertwined
+  with pinctrl-nomadik.
+
+maintainers:
+  - Linus Walleij <linus.walleij@linaro.org>
+
+properties:
+  $nodename:
+    pattern: "^gpio@[0-9a-f]+$"
+
+  compatible:
+    const: st,nomadik-gpio
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  "#gpio-cells":
+    const: 2
+
+  gpio-controller: true
+
+  interrupt-controller: true
+
+  "#interrupt-cells":
+    const: 2
+
+  gpio-bank:
+    $ref: /schemas/types.yaml#/definitions/uint32
+
+  st,supports-sleepmode:
+    description: Whether the controller can sleep or not.
+    $ref: /schemas/types.yaml#/definitions/flag
+
+  clocks:
+    maxItems: 1
+
+  gpio-ranges:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - "#gpio-cells"
+  - gpio-controller
+  - interrupt-controller
+  - gpio-bank
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    gpio1: gpio@8012e080 {
+        compatible = "st,nomadik-gpio";
+        reg =  <0x8012e080 0x80>;
+        interrupts = <0 120 0x4>;
+        #gpio-cells = <2>;
+        gpio-controller;
+        interrupt-controller;
+        st,supports-sleepmode;
+        gpio-bank = <1>;
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index 9134384171fe..0cb2c459d1cf 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2465,6 +2465,7 @@ T:	git git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-nomadik.git
 F:	Documentation/devicetree/bindings/arm/ste-*
 F:	Documentation/devicetree/bindings/arm/ux500.yaml
 F:	Documentation/devicetree/bindings/arm/ux500/
+F:	Documentation/devicetree/bindings/gpio/st,nomadik-gpio.yaml
 F:	Documentation/devicetree/bindings/i2c/st,nomadik-i2c.yaml
 F:	arch/arm/boot/dts/st/ste-*
 F:	arch/arm/mach-nomadik/

-- 
2.43.1

Re: [PATCH 01/23] dt-bindings: gpio: nomadik: convert into yaml format

[Krzysztof Kozlowski]

On 14/02/2024 17:23, Théo Lebrun wrote:
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  "#gpio-cells":
> +    const: 2
> +
> +  gpio-controller: true
> +
> +  interrupt-controller: true
> +
> +  "#interrupt-cells":
> +    const: 2
> +
> +  gpio-bank:
> +    $ref: /schemas/types.yaml#/definitions/uint32

Missing description, unless it is a common property (but then where do I
find it?).

> +
> +  st,supports-sleepmode:
> +    description: Whether the controller can sleep or not.
> +    $ref: /schemas/types.yaml#/definitions/flag
> +
> +  clocks:
> +    maxItems: 1
> +
> +  gpio-ranges:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - "#gpio-cells"
> +  - gpio-controller
> +  - interrupt-controller
> +  - gpio-bank


Best regards,
Krzysztof

Re: [PATCH 01/23] dt-bindings: gpio: nomadik: convert into yaml format

[Théo Lebrun]

Hello,

On Thu Feb 15, 2024 at 10:11 AM CET, Krzysztof Kozlowski wrote:
> On 14/02/2024 17:23, Théo Lebrun wrote:
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  "#gpio-cells":
> > +    const: 2
> > +
> > +  gpio-controller: true
> > +
> > +  interrupt-controller: true
> > +
> > +  "#interrupt-cells":
> > +    const: 2
> > +
> > +  gpio-bank:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
>
> Missing description, unless it is a common property (but then where do I
> find it?).

Missing indeed. Will become:

  gpio-bank:
    description: System-wide GPIO bank index.
    $ref: /schemas/types.yaml#/definitions/uint32

Thanks,

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH 01/23] dt-bindings: gpio: nomadik: convert into yaml format

[Linus Walleij]

On Wed, Feb 14, 2024 at 5:24 PM Théo Lebrun <theo.lebrun@bootlin.com> wrote:

> Create gpio/st,nomadik-gpio.yaml json-schema dt-bindings file as a
> direct translation from gpio-nmk.txt. Remove the txt file.
>
> Add clocks and gpio-ranges properties which were missing and are being
> used in Nomadik devicetrees.
>
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>

Thanks for doing this.

With the fix pointed out by Krzysztof folded in:
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[PATCH 02/23] dt-bindings: gpio: nomadik: add optional ngpios property

[Théo Lebrun]

This GPIO controller can support a lesser number of GPIOs than 32.
Express that in devicetree using an optional, generic property.

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 Documentation/devicetree/bindings/gpio/st,nomadik-gpio.yaml | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/gpio/st,nomadik-gpio.yaml b/Documentation/devicetree/bindings/gpio/st,nomadik-gpio.yaml
index a999908173d2..bbd23daed229 100644
--- a/Documentation/devicetree/bindings/gpio/st,nomadik-gpio.yaml
+++ b/Documentation/devicetree/bindings/gpio/st,nomadik-gpio.yaml
@@ -50,6 +50,10 @@ properties:
   gpio-ranges:
     maxItems: 1
 
+  ngpios:
+    minimum: 0
+    maximum: 32
+
 required:
   - compatible
   - reg

-- 
2.43.1

Re: [PATCH 02/23] dt-bindings: gpio: nomadik: add optional ngpios property

[Krzysztof Kozlowski]

On 14/02/2024 17:23, Théo Lebrun wrote:
> This GPIO controller can support a lesser number of GPIOs than 32.
> Express that in devicetree using an optional, generic property.
> 
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> ---

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH 02/23] dt-bindings: gpio: nomadik: add optional ngpios property

[Linus Walleij]

On Wed, Feb 14, 2024 at 5:24 PM Théo Lebrun <theo.lebrun@bootlin.com> wrote:

> This GPIO controller can support a lesser number of GPIOs than 32.
> Express that in devicetree using an optional, generic property.
>
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> ---
>  Documentation/devicetree/bindings/gpio/st,nomadik-gpio.yaml | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/gpio/st,nomadik-gpio.yaml b/Documentation/devicetree/bindings/gpio/st,nomadik-gpio.yaml
> index a999908173d2..bbd23daed229 100644
> --- a/Documentation/devicetree/bindings/gpio/st,nomadik-gpio.yaml
> +++ b/Documentation/devicetree/bindings/gpio/st,nomadik-gpio.yaml
> @@ -50,6 +50,10 @@ properties:
>    gpio-ranges:
>      maxItems: 1
>
> +  ngpios:
> +    minimum: 0
> +    maximum: 32

I can't help but wonder what good is ngpios = <0>; but OK.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCH 02/23] dt-bindings: gpio: nomadik: add optional ngpios property

[Théo Lebrun]

Hello,

On Mon Feb 19, 2024 at 3:50 PM CET, Linus Walleij wrote:
> On Wed, Feb 14, 2024 at 5:24 PM Théo Lebrun <theo.lebrun@bootlin.com> wrote:
>
> > This GPIO controller can support a lesser number of GPIOs than 32.
> > Express that in devicetree using an optional, generic property.
> >
> > Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> > ---
> >  Documentation/devicetree/bindings/gpio/st,nomadik-gpio.yaml | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/gpio/st,nomadik-gpio.yaml b/Documentation/devicetree/bindings/gpio/st,nomadik-gpio.yaml
> > index a999908173d2..bbd23daed229 100644
> > --- a/Documentation/devicetree/bindings/gpio/st,nomadik-gpio.yaml
> > +++ b/Documentation/devicetree/bindings/gpio/st,nomadik-gpio.yaml
> > @@ -50,6 +50,10 @@ properties:
> >    gpio-ranges:
> >      maxItems: 1
> >
> > +  ngpios:
> > +    minimum: 0
> > +    maximum: 32
>
> I can't help but wonder what good is ngpios = <0>; but OK.
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

A min value is required, else it is equal to the max. There was no
reason to pick anything bigger than zero.

I'll admit it is not a setup I have tested so the driver might have
issues with the ngpios==0 edge-case. Of course I do not expect anyone
to care.

Regards,

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[PATCH 03/23] dt-bindings: gpio: nomadik: add mobileye,eyeq5-gpio compatible

[Théo Lebrun]

This GPIO controller is used on the Mobileye EyeQ5 SoC. Add its
compatible to the dt-bindings. One difference is that the block as
integrated on EyeQ5 does not support sleep-mode.

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 .../devicetree/bindings/gpio/st,nomadik-gpio.yaml        | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/gpio/st,nomadik-gpio.yaml b/Documentation/devicetree/bindings/gpio/st,nomadik-gpio.yaml
index bbd23daed229..e44cf292bc6d 100644
--- a/Documentation/devicetree/bindings/gpio/st,nomadik-gpio.yaml
+++ b/Documentation/devicetree/bindings/gpio/st,nomadik-gpio.yaml
@@ -19,7 +19,9 @@ properties:
     pattern: "^gpio@[0-9a-f]+$"
 
   compatible:
-    const: st,nomadik-gpio
+    enum:
+      - st,nomadik-gpio
+      - mobileye,eyeq5-gpio
 
   reg:
     maxItems: 1
@@ -65,6 +67,18 @@ required:
 
 unevaluatedProperties: false
 
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: st,nomadik-gpio
+    then:
+      properties:
+        st,supports-sleepmode:
+          description: Whether the controller can sleep or not.
+          $ref: /schemas/types.yaml#/definitions/flag
+
 examples:
   - |
     gpio1: gpio@8012e080 {

-- 
2.43.1

Re: [PATCH 03/23] dt-bindings: gpio: nomadik: add mobileye,eyeq5-gpio compatible

[Krzysztof Kozlowski]

On 14/02/2024 17:23, Théo Lebrun wrote:
> This GPIO controller is used on the Mobileye EyeQ5 SoC. Add its
> compatible to the dt-bindings. One difference is that the block as
> integrated on EyeQ5 does not support sleep-mode.
> 
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> ---
>  .../devicetree/bindings/gpio/st,nomadik-gpio.yaml        | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/gpio/st,nomadik-gpio.yaml b/Documentation/devicetree/bindings/gpio/st,nomadik-gpio.yaml
> index bbd23daed229..e44cf292bc6d 100644
> --- a/Documentation/devicetree/bindings/gpio/st,nomadik-gpio.yaml
> +++ b/Documentation/devicetree/bindings/gpio/st,nomadik-gpio.yaml
> @@ -19,7 +19,9 @@ properties:
>      pattern: "^gpio@[0-9a-f]+$"
>  
>    compatible:
> -    const: st,nomadik-gpio
> +    enum:
> +      - st,nomadik-gpio
> +      - mobileye,eyeq5-gpio
>  
>    reg:
>      maxItems: 1
> @@ -65,6 +67,18 @@ required:
>  
>  unevaluatedProperties: false
>  
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: st,nomadik-gpio
> +    then:
> +      properties:
> +        st,supports-sleepmode:
> +          description: Whether the controller can sleep or not.
> +          $ref: /schemas/types.yaml#/definitions/flag

You already have such property, so you are defining it second time. You
want instead if: for eyeq5-gpio making it:
then:
  properties:
    st,supports-sleepmode: false

Best regards,
Krzysztof

Re: [PATCH 03/23] dt-bindings: gpio: nomadik: add mobileye,eyeq5-gpio compatible

[Théo Lebrun]

Hello,

On Thu Feb 15, 2024 at 10:13 AM CET, Krzysztof Kozlowski wrote:
> On 14/02/2024 17:23, Théo Lebrun wrote:
> > This GPIO controller is used on the Mobileye EyeQ5 SoC. Add its
> > compatible to the dt-bindings. One difference is that the block as
> > integrated on EyeQ5 does not support sleep-mode.
> > 
> > Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> > ---
> >  .../devicetree/bindings/gpio/st,nomadik-gpio.yaml        | 16 +++++++++++++++-
> >  1 file changed, 15 insertions(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/gpio/st,nomadik-gpio.yaml b/Documentation/devicetree/bindings/gpio/st,nomadik-gpio.yaml
> > index bbd23daed229..e44cf292bc6d 100644
> > --- a/Documentation/devicetree/bindings/gpio/st,nomadik-gpio.yaml
> > +++ b/Documentation/devicetree/bindings/gpio/st,nomadik-gpio.yaml
> > @@ -19,7 +19,9 @@ properties:
> >      pattern: "^gpio@[0-9a-f]+$"
> >  
> >    compatible:
> > -    const: st,nomadik-gpio
> > +    enum:
> > +      - st,nomadik-gpio
> > +      - mobileye,eyeq5-gpio
> >  
> >    reg:
> >      maxItems: 1
> > @@ -65,6 +67,18 @@ required:
> >  
> >  unevaluatedProperties: false
> >  
> > +allOf:
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            const: st,nomadik-gpio
> > +    then:
> > +      properties:
> > +        st,supports-sleepmode:
> > +          description: Whether the controller can sleep or not.
> > +          $ref: /schemas/types.yaml#/definitions/flag
>
> You already have such property, so you are defining it second time. You
> want instead if: for eyeq5-gpio making it:
> then:
>   properties:
>     st,supports-sleepmode: false

Good catch. I hesitated between declaring the property inside an
if-statement or disabling it inside an if-statement as you show here. I
ended-up mixing those two approaches.

Wil pick the if-eyeq5-then-disable-prop solution.

Thanks for the review Krzysztof!

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[PATCH 04/23] dt-bindings: gpio: nomadik: add optional reset property

[Théo Lebrun]

Add optional reset device-tree property to the Nomadik GPIO controller.

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 Documentation/devicetree/bindings/gpio/st,nomadik-gpio.yaml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/gpio/st,nomadik-gpio.yaml b/Documentation/devicetree/bindings/gpio/st,nomadik-gpio.yaml
index e44cf292bc6d..d104662ec445 100644
--- a/Documentation/devicetree/bindings/gpio/st,nomadik-gpio.yaml
+++ b/Documentation/devicetree/bindings/gpio/st,nomadik-gpio.yaml
@@ -56,6 +56,9 @@ properties:
     minimum: 0
     maximum: 32
 
+  resets:
+    maxItems: 1
+
 required:
   - compatible
   - reg

-- 
2.43.1

Re: [PATCH 04/23] dt-bindings: gpio: nomadik: add optional reset property

[Krzysztof Kozlowski]

On 14/02/2024 17:23, Théo Lebrun wrote:
> Add optional reset device-tree property to the Nomadik GPIO controller.
> 
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> ---
>  Documentation/devicetree/bindings/gpio/st,nomadik-gpio.yaml | 3 +++
>  1 file changed, 3 insertions(+)
> 

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH 04/23] dt-bindings: gpio: nomadik: add optional reset property

[Linus Walleij]

On Wed, Feb 14, 2024 at 5:24 PM Théo Lebrun <theo.lebrun@bootlin.com> wrote:

> Add optional reset device-tree property to the Nomadik GPIO controller.
>
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[PATCH 05/23] gpio: nomadik: extract GPIO platform driver from drivers/pinctrl/nomadik/

[Théo Lebrun]

Previously, drivers/pinctrl/nomadik/pinctrl-nomadik.c registered two
platform drivers: pinctrl & GPIO. Move the GPIO aspect to the
drivers/gpio/ folder, as would be expected.

Both drivers are intertwined for a reason; pinctrl requires access to
GPIO registers for pinmuxing, pull-disable, disabling interrupts while
setting the muxing and wakeup control. Information sharing is done
through a shared array containing GPIO chips and a few helper
functions. That shared array is not touched from gpio-nomadik when
CONFIG_PINCTRL_NOMADIK is not defined.

Make no change to the code that moved into gpio-nomadik; there should be
no behavior change following. A few functions are shared and header
comments are added. Checkpatch warnings are addressed. NUM_BANKS is
renamed to NMK_MAX_BANKS.

It is supported to compile gpio-nomadik without pinctrl-nomadik. The
opposite is not true.

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 MAINTAINERS                                        |   1 +
 drivers/gpio/Kconfig                               |  12 +
 drivers/gpio/Makefile                              |   1 +
 drivers/gpio/gpio-nomadik.c                        | 660 +++++++++++++++++++
 drivers/pinctrl/nomadik/Kconfig                    |   5 +-
 drivers/pinctrl/nomadik/pinctrl-nomadik-db8500.c   |   3 +-
 drivers/pinctrl/nomadik/pinctrl-nomadik-stn8815.c  |   3 +-
 drivers/pinctrl/nomadik/pinctrl-nomadik.c          | 722 +--------------------
 .../linux/gpio/gpio-nomadik.h                      | 122 +++-
 9 files changed, 804 insertions(+), 725 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 0cb2c459d1cf..3f864e773267 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2474,6 +2474,7 @@ F:	drivers/clk/clk-nomadik.c
 F:	drivers/clocksource/clksrc-dbx500-prcmu.c
 F:	drivers/dma/ste_dma40*
 F:	drivers/pmdomain/st/ste-ux500-pm-domain.c
+F:	drivers/gpio/gpio-nomadik.c
 F:	drivers/hwspinlock/u8500_hsem.c
 F:	drivers/i2c/busses/i2c-nomadik.c
 F:	drivers/iio/adc/ab8500-gpadc.c
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 1301cec94f12..ff83371251c1 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -478,6 +478,18 @@ config GPIO_MXS
 	select GPIO_GENERIC
 	select GENERIC_IRQ_CHIP
 
+config GPIO_NOMADIK
+	bool "Nomadik GPIO driver"
+	depends on ARCH_U8500 || ARCH_NOMADIK || COMPILE_TEST
+	select OF_GPIO
+	select GPIOLIB_IRQCHIP
+	help
+	  Say yes here to support the Nomadik SoC GPIO block.
+
+	  It handles up to 32 GPIOs per bank, that can all be interrupt sources.
+	  It is deeply interconnected with the associated pinctrl driver as GPIO
+	  registers handle muxing ("alternate functions") as well.
+
 config GPIO_NPCM_SGPIO
 	bool "Nuvoton SGPIO support"
 	depends on ARCH_NPCM || COMPILE_TEST
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 9e40af196aae..9fc2f5931b22 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -116,6 +116,7 @@ obj-$(CONFIG_GPIO_MT7621)		+= gpio-mt7621.o
 obj-$(CONFIG_GPIO_MVEBU)		+= gpio-mvebu.o
 obj-$(CONFIG_GPIO_MXC)			+= gpio-mxc.o
 obj-$(CONFIG_GPIO_MXS)			+= gpio-mxs.o
+obj-$(CONFIG_GPIO_NOMADIK)		+= gpio-nomadik.o
 obj-$(CONFIG_GPIO_NPCM_SGPIO)		+= gpio-npcm-sgpio.o
 obj-$(CONFIG_GPIO_OCTEON)		+= gpio-octeon.o
 obj-$(CONFIG_GPIO_OMAP)			+= gpio-omap.o
diff --git a/drivers/gpio/gpio-nomadik.c b/drivers/gpio/gpio-nomadik.c
new file mode 100644
index 000000000000..e39477e1a58f
--- /dev/null
+++ b/drivers/gpio/gpio-nomadik.c
@@ -0,0 +1,660 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * GPIO driver for the IP block found in the Nomadik SoC; it is an AMBA device,
+ * managing 32 pins with alternate functions. It can also handle the STA2X11
+ * block from ST.
+ *
+ * The GPIO chips are shared with pinctrl-nomadik if used; it needs access for
+ * pinmuxing functionality and others.
+ *
+ * Copyright (C) 2008,2009 STMicroelectronics
+ * Copyright (C) 2009 Alessandro Rubini <rubini@unipv.it>
+ *   Rewritten based on work by Prafulla WADASKAR <prafulla.wadaskar@st.com>
+ * Copyright (C) 2011-2013 Linus Walleij <linus.walleij@linaro.org>
+ */
+#include <linux/cleanup.h>
+#include <linux/clk.h>
+#include <linux/gpio/driver.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/of_device.h>
+#include <linux/of_platform.h>
+#include <linux/pinctrl/pinctrl.h>
+#include <linux/platform_device.h>
+#include <linux/reset.h>
+#include <linux/seq_file.h>
+#include <linux/types.h>
+
+#include <linux/gpio/gpio-nomadik.h>
+
+#ifndef CONFIG_PINCTRL_NOMADIK
+static DEFINE_SPINLOCK(nmk_gpio_slpm_lock);
+#endif
+
+void __nmk_gpio_set_slpm(struct nmk_gpio_chip *nmk_chip, unsigned int offset,
+			 enum nmk_gpio_slpm mode)
+{
+	u32 slpm;
+
+	slpm = readl(nmk_chip->addr + NMK_GPIO_SLPC);
+	if (mode == NMK_GPIO_SLPM_NOCHANGE)
+		slpm |= BIT(offset);
+	else
+		slpm &= ~BIT(offset);
+	writel(slpm, nmk_chip->addr + NMK_GPIO_SLPC);
+}
+
+static void __nmk_gpio_set_output(struct nmk_gpio_chip *nmk_chip,
+				  unsigned int offset, int val)
+{
+	if (val)
+		writel(BIT(offset), nmk_chip->addr + NMK_GPIO_DATS);
+	else
+		writel(BIT(offset), nmk_chip->addr + NMK_GPIO_DATC);
+}
+
+void __nmk_gpio_make_output(struct nmk_gpio_chip *nmk_chip,
+			    unsigned int offset, int val)
+{
+	writel(BIT(offset), nmk_chip->addr + NMK_GPIO_DIRS);
+	__nmk_gpio_set_output(nmk_chip, offset, val);
+}
+
+/* IRQ functions */
+
+static void nmk_gpio_irq_ack(struct irq_data *d)
+{
+	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+	struct nmk_gpio_chip *nmk_chip = gpiochip_get_data(gc);
+
+	clk_enable(nmk_chip->clk);
+	writel(BIT(d->hwirq), nmk_chip->addr + NMK_GPIO_IC);
+	clk_disable(nmk_chip->clk);
+}
+
+enum nmk_gpio_irq_type {
+	NORMAL,
+	WAKE,
+};
+
+static void __nmk_gpio_irq_modify(struct nmk_gpio_chip *nmk_chip,
+				  int offset, enum nmk_gpio_irq_type which,
+				  bool enable)
+{
+	u32 *rimscval;
+	u32 *fimscval;
+	u32 rimscreg;
+	u32 fimscreg;
+
+	if (which == NORMAL) {
+		rimscreg = NMK_GPIO_RIMSC;
+		fimscreg = NMK_GPIO_FIMSC;
+		rimscval = &nmk_chip->rimsc;
+		fimscval = &nmk_chip->fimsc;
+	} else  {
+		rimscreg = NMK_GPIO_RWIMSC;
+		fimscreg = NMK_GPIO_FWIMSC;
+		rimscval = &nmk_chip->rwimsc;
+		fimscval = &nmk_chip->fwimsc;
+	}
+
+	/* we must individually set/clear the two edges */
+	if (nmk_chip->edge_rising & BIT(offset)) {
+		if (enable)
+			*rimscval |= BIT(offset);
+		else
+			*rimscval &= ~BIT(offset);
+		writel(*rimscval, nmk_chip->addr + rimscreg);
+	}
+	if (nmk_chip->edge_falling & BIT(offset)) {
+		if (enable)
+			*fimscval |= BIT(offset);
+		else
+			*fimscval &= ~BIT(offset);
+		writel(*fimscval, nmk_chip->addr + fimscreg);
+	}
+}
+
+static void __nmk_gpio_set_wake(struct nmk_gpio_chip *nmk_chip,
+				int offset, bool on)
+{
+	/*
+	 * Ensure WAKEUP_ENABLE is on.  No need to disable it if wakeup is
+	 * disabled, since setting SLPM to 1 increases power consumption, and
+	 * wakeup is anyhow controlled by the RIMSC and FIMSC registers.
+	 */
+	if (nmk_chip->sleepmode && on) {
+		__nmk_gpio_set_slpm(nmk_chip, offset,
+				    NMK_GPIO_SLPM_WAKEUP_ENABLE);
+	}
+
+	__nmk_gpio_irq_modify(nmk_chip, offset, WAKE, on);
+}
+
+static void nmk_gpio_irq_maskunmask(struct nmk_gpio_chip *nmk_chip,
+				    struct irq_data *d, bool enable)
+{
+	unsigned long flags;
+
+	clk_enable(nmk_chip->clk);
+	spin_lock_irqsave(&nmk_gpio_slpm_lock, flags);
+	spin_lock(&nmk_chip->lock);
+
+	__nmk_gpio_irq_modify(nmk_chip, d->hwirq, NORMAL, enable);
+
+	if (!(nmk_chip->real_wake & BIT(d->hwirq)))
+		__nmk_gpio_set_wake(nmk_chip, d->hwirq, enable);
+
+	spin_unlock(&nmk_chip->lock);
+	spin_unlock_irqrestore(&nmk_gpio_slpm_lock, flags);
+	clk_disable(nmk_chip->clk);
+}
+
+static void nmk_gpio_irq_mask(struct irq_data *d)
+{
+	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+	struct nmk_gpio_chip *nmk_chip = gpiochip_get_data(gc);
+
+	nmk_gpio_irq_maskunmask(nmk_chip, d, false);
+	gpiochip_disable_irq(gc, irqd_to_hwirq(d));
+}
+
+static void nmk_gpio_irq_unmask(struct irq_data *d)
+{
+	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+	struct nmk_gpio_chip *nmk_chip = gpiochip_get_data(gc);
+
+	gpiochip_enable_irq(gc, irqd_to_hwirq(d));
+	nmk_gpio_irq_maskunmask(nmk_chip, d, true);
+}
+
+static int nmk_gpio_irq_set_wake(struct irq_data *d, unsigned int on)
+{
+	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+	struct nmk_gpio_chip *nmk_chip = gpiochip_get_data(gc);
+	unsigned long flags;
+
+	clk_enable(nmk_chip->clk);
+	spin_lock_irqsave(&nmk_gpio_slpm_lock, flags);
+	spin_lock(&nmk_chip->lock);
+
+	if (irqd_irq_disabled(d))
+		__nmk_gpio_set_wake(nmk_chip, d->hwirq, on);
+
+	if (on)
+		nmk_chip->real_wake |= BIT(d->hwirq);
+	else
+		nmk_chip->real_wake &= ~BIT(d->hwirq);
+
+	spin_unlock(&nmk_chip->lock);
+	spin_unlock_irqrestore(&nmk_gpio_slpm_lock, flags);
+	clk_disable(nmk_chip->clk);
+
+	return 0;
+}
+
+static int nmk_gpio_irq_set_type(struct irq_data *d, unsigned int type)
+{
+	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+	struct nmk_gpio_chip *nmk_chip = gpiochip_get_data(gc);
+	bool enabled = !irqd_irq_disabled(d);
+	bool wake = irqd_is_wakeup_set(d);
+	unsigned long flags;
+
+	if (type & IRQ_TYPE_LEVEL_HIGH)
+		return -EINVAL;
+	if (type & IRQ_TYPE_LEVEL_LOW)
+		return -EINVAL;
+
+	clk_enable(nmk_chip->clk);
+	spin_lock_irqsave(&nmk_chip->lock, flags);
+
+	if (enabled)
+		__nmk_gpio_irq_modify(nmk_chip, d->hwirq, NORMAL, false);
+
+	if (enabled || wake)
+		__nmk_gpio_irq_modify(nmk_chip, d->hwirq, WAKE, false);
+
+	nmk_chip->edge_rising &= ~BIT(d->hwirq);
+	if (type & IRQ_TYPE_EDGE_RISING)
+		nmk_chip->edge_rising |= BIT(d->hwirq);
+
+	nmk_chip->edge_falling &= ~BIT(d->hwirq);
+	if (type & IRQ_TYPE_EDGE_FALLING)
+		nmk_chip->edge_falling |= BIT(d->hwirq);
+
+	if (enabled)
+		__nmk_gpio_irq_modify(nmk_chip, d->hwirq, NORMAL, true);
+
+	if (enabled || wake)
+		__nmk_gpio_irq_modify(nmk_chip, d->hwirq, WAKE, true);
+
+	spin_unlock_irqrestore(&nmk_chip->lock, flags);
+	clk_disable(nmk_chip->clk);
+
+	return 0;
+}
+
+static unsigned int nmk_gpio_irq_startup(struct irq_data *d)
+{
+	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+	struct nmk_gpio_chip *nmk_chip = gpiochip_get_data(gc);
+
+	clk_enable(nmk_chip->clk);
+	nmk_gpio_irq_unmask(d);
+	return 0;
+}
+
+static void nmk_gpio_irq_shutdown(struct irq_data *d)
+{
+	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+	struct nmk_gpio_chip *nmk_chip = gpiochip_get_data(gc);
+
+	nmk_gpio_irq_mask(d);
+	clk_disable(nmk_chip->clk);
+}
+
+static void nmk_gpio_irq_handler(struct irq_desc *desc)
+{
+	struct irq_chip *host_chip = irq_desc_get_chip(desc);
+	struct gpio_chip *chip = irq_desc_get_handler_data(desc);
+	struct nmk_gpio_chip *nmk_chip = gpiochip_get_data(chip);
+	u32 status;
+
+	chained_irq_enter(host_chip, desc);
+
+	clk_enable(nmk_chip->clk);
+	status = readl(nmk_chip->addr + NMK_GPIO_IS);
+	clk_disable(nmk_chip->clk);
+
+	while (status) {
+		int bit = __ffs(status);
+
+		generic_handle_domain_irq(chip->irq.domain, bit);
+		status &= ~BIT(bit);
+	}
+
+	chained_irq_exit(host_chip, desc);
+}
+
+/* I/O Functions */
+
+static int nmk_gpio_get_dir(struct gpio_chip *chip, unsigned int offset)
+{
+	struct nmk_gpio_chip *nmk_chip = gpiochip_get_data(chip);
+	int dir;
+
+	clk_enable(nmk_chip->clk);
+
+	dir = readl(nmk_chip->addr + NMK_GPIO_DIR) & BIT(offset);
+
+	clk_disable(nmk_chip->clk);
+
+	if (dir)
+		return GPIO_LINE_DIRECTION_OUT;
+
+	return GPIO_LINE_DIRECTION_IN;
+}
+
+static int nmk_gpio_make_input(struct gpio_chip *chip, unsigned int offset)
+{
+	struct nmk_gpio_chip *nmk_chip = gpiochip_get_data(chip);
+
+	clk_enable(nmk_chip->clk);
+
+	writel(BIT(offset), nmk_chip->addr + NMK_GPIO_DIRC);
+
+	clk_disable(nmk_chip->clk);
+
+	return 0;
+}
+
+static int nmk_gpio_get_input(struct gpio_chip *chip, unsigned int offset)
+{
+	struct nmk_gpio_chip *nmk_chip = gpiochip_get_data(chip);
+	int value;
+
+	clk_enable(nmk_chip->clk);
+
+	value = !!(readl(nmk_chip->addr + NMK_GPIO_DAT) & BIT(offset));
+
+	clk_disable(nmk_chip->clk);
+
+	return value;
+}
+
+static void nmk_gpio_set_output(struct gpio_chip *chip, unsigned int offset,
+				int val)
+{
+	struct nmk_gpio_chip *nmk_chip = gpiochip_get_data(chip);
+
+	clk_enable(nmk_chip->clk);
+
+	__nmk_gpio_set_output(nmk_chip, offset, val);
+
+	clk_disable(nmk_chip->clk);
+}
+
+static int nmk_gpio_make_output(struct gpio_chip *chip, unsigned int offset,
+				int val)
+{
+	struct nmk_gpio_chip *nmk_chip = gpiochip_get_data(chip);
+
+	clk_enable(nmk_chip->clk);
+
+	__nmk_gpio_make_output(nmk_chip, offset, val);
+
+	clk_disable(nmk_chip->clk);
+
+	return 0;
+}
+
+#ifdef CONFIG_DEBUG_FS
+
+static int nmk_gpio_get_mode(struct nmk_gpio_chip *nmk_chip, int offset)
+{
+	u32 afunc, bfunc;
+
+	clk_enable(nmk_chip->clk);
+
+	afunc = readl(nmk_chip->addr + NMK_GPIO_AFSLA) & BIT(offset);
+	bfunc = readl(nmk_chip->addr + NMK_GPIO_AFSLB) & BIT(offset);
+
+	clk_disable(nmk_chip->clk);
+
+	return (afunc ? NMK_GPIO_ALT_A : 0) | (bfunc ? NMK_GPIO_ALT_B : 0);
+}
+
+void nmk_gpio_dbg_show_one(struct seq_file *s, struct pinctrl_dev *pctldev,
+			   struct gpio_chip *chip, unsigned int offset,
+			   unsigned int gpio)
+{
+	struct nmk_gpio_chip *nmk_chip = gpiochip_get_data(chip);
+	int mode;
+	bool is_out;
+	bool data_out;
+	bool pull;
+	static const char * const modes[] = {
+		[NMK_GPIO_ALT_GPIO]	= "gpio",
+		[NMK_GPIO_ALT_A]	= "altA",
+		[NMK_GPIO_ALT_B]	= "altB",
+		[NMK_GPIO_ALT_C]	= "altC",
+		[NMK_GPIO_ALT_C + 1]	= "altC1",
+		[NMK_GPIO_ALT_C + 2]	= "altC2",
+		[NMK_GPIO_ALT_C + 3]	= "altC3",
+		[NMK_GPIO_ALT_C + 4]	= "altC4",
+	};
+
+	char *label = gpiochip_dup_line_label(chip, offset);
+	if (IS_ERR(label))
+		return;
+
+	clk_enable(nmk_chip->clk);
+	is_out = !!(readl(nmk_chip->addr + NMK_GPIO_DIR) & BIT(offset));
+	pull = !(readl(nmk_chip->addr + NMK_GPIO_PDIS) & BIT(offset));
+	data_out = !!(readl(nmk_chip->addr + NMK_GPIO_DAT) & BIT(offset));
+	mode = nmk_gpio_get_mode(nmk_chip, offset);
+#ifdef CONFIG_PINCTRL_NOMADIK
+	if (mode == NMK_GPIO_ALT_C && pctldev)
+		mode = nmk_prcm_gpiocr_get_mode(pctldev, gpio);
+#endif
+
+	if (is_out) {
+		seq_printf(s, " gpio-%-3d (%-20.20s) out %s           %s",
+			   gpio,
+			   label ?: "(none)",
+			   data_out ? "hi" : "lo",
+			   (mode < 0) ? "unknown" : modes[mode]);
+	} else {
+		int irq = chip->to_irq(chip, offset);
+		const int pullidx = pull ? 1 : 0;
+		int val;
+		static const char * const pulls[] = {
+			"none        ",
+			"pull enabled",
+		};
+
+		seq_printf(s, " gpio-%-3d (%-20.20s) in  %s %s",
+			   gpio,
+			   label ?: "(none)",
+			   pulls[pullidx],
+			   (mode < 0) ? "unknown" : modes[mode]);
+
+		val = nmk_gpio_get_input(chip, offset);
+		seq_printf(s, " VAL %d", val);
+
+		/*
+		 * This races with request_irq(), set_irq_type(),
+		 * and set_irq_wake() ... but those are "rare".
+		 */
+		if (irq > 0 && irq_has_action(irq)) {
+			char *trigger;
+			bool wake;
+
+			if (nmk_chip->edge_rising & BIT(offset))
+				trigger = "edge-rising";
+			else if (nmk_chip->edge_falling & BIT(offset))
+				trigger = "edge-falling";
+			else
+				trigger = "edge-undefined";
+
+			wake = !!(nmk_chip->real_wake & BIT(offset));
+
+			seq_printf(s, " irq-%d %s%s",
+				   irq, trigger, wake ? " wakeup" : "");
+		}
+	}
+	clk_disable(nmk_chip->clk);
+}
+
+static void nmk_gpio_dbg_show(struct seq_file *s, struct gpio_chip *chip)
+{
+	unsigned int i, gpio = chip->base;
+
+	for (i = 0; i < chip->ngpio; i++, gpio++) {
+		nmk_gpio_dbg_show_one(s, NULL, chip, i, gpio);
+		seq_puts(s, "\n");
+	}
+}
+
+#else
+
+static inline void nmk_gpio_dbg_show_one(struct seq_file *s,
+					 struct pinctrl_dev *pctldev,
+					 struct gpio_chip *chip,
+					 unsigned int offset,
+					 unsigned int gpio)
+{
+}
+
+#define nmk_gpio_dbg_show	NULL
+
+#endif
+
+/*
+ * We will allocate memory for the state container using devm* allocators
+ * binding to the first device reaching this point, it doesn't matter if
+ * it is the pin controller or GPIO driver. However we need to use the right
+ * platform device when looking up resources so pay attention to pdev.
+ */
+struct nmk_gpio_chip *nmk_gpio_populate_chip(struct device_node *np,
+					     struct platform_device *pdev)
+{
+	struct nmk_gpio_chip *nmk_chip;
+	struct platform_device *gpio_pdev;
+	struct gpio_chip *chip;
+	struct resource *res;
+	struct clk *clk;
+	void __iomem *base;
+	u32 id;
+
+	gpio_pdev = of_find_device_by_node(np);
+	if (!gpio_pdev) {
+		pr_err("populate \"%pOFn\": device not found\n", np);
+		return ERR_PTR(-ENODEV);
+	}
+	if (of_property_read_u32(np, "gpio-bank", &id)) {
+		dev_err(&pdev->dev, "populate: gpio-bank property not found\n");
+		platform_device_put(gpio_pdev);
+		return ERR_PTR(-EINVAL);
+	}
+
+#ifdef CONFIG_PINCTRL_NOMADIK
+	/* Already populated? */
+	nmk_chip = nmk_gpio_chips[id];
+	if (nmk_chip) {
+		platform_device_put(gpio_pdev);
+		return nmk_chip;
+	}
+#endif
+
+	nmk_chip = devm_kzalloc(&pdev->dev, sizeof(*nmk_chip), GFP_KERNEL);
+	if (!nmk_chip) {
+		platform_device_put(gpio_pdev);
+		return ERR_PTR(-ENOMEM);
+	}
+
+	nmk_chip->bank = id;
+	chip = &nmk_chip->chip;
+	chip->base = id * NMK_GPIO_PER_CHIP;
+	chip->ngpio = NMK_GPIO_PER_CHIP;
+	chip->label = dev_name(&gpio_pdev->dev);
+	chip->parent = &gpio_pdev->dev;
+
+	res = platform_get_resource(gpio_pdev, IORESOURCE_MEM, 0);
+	base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(base)) {
+		platform_device_put(gpio_pdev);
+		return ERR_CAST(base);
+	}
+	nmk_chip->addr = base;
+
+	clk = clk_get(&gpio_pdev->dev, NULL);
+	if (IS_ERR(clk)) {
+		platform_device_put(gpio_pdev);
+		return (void *)clk;
+	}
+	clk_prepare(clk);
+	nmk_chip->clk = clk;
+
+#ifdef CONFIG_PINCTRL_NOMADIK
+	BUG_ON(nmk_chip->bank >= ARRAY_SIZE(nmk_gpio_chips));
+	nmk_gpio_chips[id] = nmk_chip;
+#endif
+	return nmk_chip;
+}
+
+static void nmk_gpio_irq_print_chip(struct irq_data *d, struct seq_file *p)
+{
+	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+	struct nmk_gpio_chip *nmk_chip = gpiochip_get_data(gc);
+
+	seq_printf(p, "nmk%u-%u-%u", nmk_chip->bank,
+		   gc->base, gc->base + gc->ngpio - 1);
+}
+
+static const struct irq_chip nmk_irq_chip = {
+	.irq_ack = nmk_gpio_irq_ack,
+	.irq_mask = nmk_gpio_irq_mask,
+	.irq_unmask = nmk_gpio_irq_unmask,
+	.irq_set_type = nmk_gpio_irq_set_type,
+	.irq_set_wake = nmk_gpio_irq_set_wake,
+	.irq_startup = nmk_gpio_irq_startup,
+	.irq_shutdown = nmk_gpio_irq_shutdown,
+	.irq_print_chip = nmk_gpio_irq_print_chip,
+	.flags = IRQCHIP_MASK_ON_SUSPEND | IRQCHIP_IMMUTABLE,
+	GPIOCHIP_IRQ_RESOURCE_HELPERS,
+};
+
+static int nmk_gpio_probe(struct platform_device *dev)
+{
+	struct device_node *np = dev->dev.of_node;
+	struct nmk_gpio_chip *nmk_chip;
+	struct gpio_chip *chip;
+	struct gpio_irq_chip *girq;
+	bool supports_sleepmode;
+	int irq;
+	int ret;
+
+	nmk_chip = nmk_gpio_populate_chip(np, dev);
+	if (IS_ERR(nmk_chip)) {
+		dev_err(&dev->dev, "could not populate nmk chip struct\n");
+		return PTR_ERR(nmk_chip);
+	}
+
+	supports_sleepmode =
+		of_property_read_bool(np, "st,supports-sleepmode");
+
+	/* Correct platform device ID */
+	dev->id = nmk_chip->bank;
+
+	irq = platform_get_irq(dev, 0);
+	if (irq < 0)
+		return irq;
+
+	/*
+	 * The virt address in nmk_chip->addr is in the nomadik register space,
+	 * so we can simply convert the resource address, without remapping
+	 */
+	nmk_chip->sleepmode = supports_sleepmode;
+	spin_lock_init(&nmk_chip->lock);
+
+	chip = &nmk_chip->chip;
+	chip->parent = &dev->dev;
+	chip->request = gpiochip_generic_request;
+	chip->free = gpiochip_generic_free;
+	chip->get_direction = nmk_gpio_get_dir;
+	chip->direction_input = nmk_gpio_make_input;
+	chip->get = nmk_gpio_get_input;
+	chip->direction_output = nmk_gpio_make_output;
+	chip->set = nmk_gpio_set_output;
+	chip->dbg_show = nmk_gpio_dbg_show;
+	chip->can_sleep = false;
+	chip->owner = THIS_MODULE;
+
+	girq = &chip->irq;
+	gpio_irq_chip_set_chip(girq, &nmk_irq_chip);
+	girq->parent_handler = nmk_gpio_irq_handler;
+	girq->num_parents = 1;
+	girq->parents = devm_kcalloc(&dev->dev, 1,
+				     sizeof(*girq->parents),
+				     GFP_KERNEL);
+	if (!girq->parents)
+		return -ENOMEM;
+	girq->parents[0] = irq;
+	girq->default_type = IRQ_TYPE_NONE;
+	girq->handler = handle_edge_irq;
+
+	clk_enable(nmk_chip->clk);
+	nmk_chip->lowemi = readl_relaxed(nmk_chip->addr + NMK_GPIO_LOWEMI);
+	clk_disable(nmk_chip->clk);
+
+	ret = gpiochip_add_data(chip, nmk_chip);
+	if (ret)
+		return ret;
+
+	platform_set_drvdata(dev, nmk_chip);
+
+	dev_info(&dev->dev, "chip registered\n");
+
+	return 0;
+}
+
+static const struct of_device_id nmk_gpio_match[] = {
+	{ .compatible = "st,nomadik-gpio", },
+	{}
+};
+
+static struct platform_driver nmk_gpio_driver = {
+	.driver = {
+		.name = "gpio",
+		.of_match_table = nmk_gpio_match,
+	},
+	.probe = nmk_gpio_probe,
+};
+
+static int __init nmk_gpio_init(void)
+{
+	return platform_driver_register(&nmk_gpio_driver);
+}
+subsys_initcall(nmk_gpio_init);
diff --git a/drivers/pinctrl/nomadik/Kconfig b/drivers/pinctrl/nomadik/Kconfig
index 0fea167c283f..f47f0755a835 100644
--- a/drivers/pinctrl/nomadik/Kconfig
+++ b/drivers/pinctrl/nomadik/Kconfig
@@ -22,11 +22,10 @@ if (ARCH_U8500 || ARCH_NOMADIK)
 
 config PINCTRL_NOMADIK
 	bool "Nomadik pin controller driver"
-	depends on OF && GPIOLIB
+	depends on OF
 	select PINMUX
 	select PINCONF
-	select OF_GPIO
-	select GPIOLIB_IRQCHIP
+	select GPIO_NOMADIK
 
 config PINCTRL_STN8815
 	bool "STN8815 pin controller driver"
diff --git a/drivers/pinctrl/nomadik/pinctrl-nomadik-db8500.c b/drivers/pinctrl/nomadik/pinctrl-nomadik-db8500.c
index 490e0959e8be..0b4a3dd9d8c7 100644
--- a/drivers/pinctrl/nomadik/pinctrl-nomadik-db8500.c
+++ b/drivers/pinctrl/nomadik/pinctrl-nomadik-db8500.c
@@ -3,8 +3,9 @@
 #include <linux/types.h>
 
 #include <linux/pinctrl/pinctrl.h>
+#include <linux/gpio/driver.h>
 
-#include "pinctrl-nomadik.h"
+#include <linux/gpio/gpio-nomadik.h>
 
 /* All the pins that can be used for GPIO and some other functions */
 #define _GPIO(offset)		(offset)
diff --git a/drivers/pinctrl/nomadik/pinctrl-nomadik-stn8815.c b/drivers/pinctrl/nomadik/pinctrl-nomadik-stn8815.c
index 1552222ac68e..c5a52fcaba30 100644
--- a/drivers/pinctrl/nomadik/pinctrl-nomadik-stn8815.c
+++ b/drivers/pinctrl/nomadik/pinctrl-nomadik-stn8815.c
@@ -3,8 +3,9 @@
 #include <linux/types.h>
 
 #include <linux/pinctrl/pinctrl.h>
+#include <linux/gpio/driver.h>
 
-#include "pinctrl-nomadik.h"
+#include <linux/gpio/gpio-nomadik.h>
 
 /* All the pins that can be used for GPIO and some other functions */
 #define _GPIO(offset)		(offset)
diff --git a/drivers/pinctrl/nomadik/pinctrl-nomadik.c b/drivers/pinctrl/nomadik/pinctrl-nomadik.c
index 7911353ac97d..f3897dbfa2c3 100644
--- a/drivers/pinctrl/nomadik/pinctrl-nomadik.c
+++ b/drivers/pinctrl/nomadik/pinctrl-nomadik.c
@@ -1,6 +1,8 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /*
- * Generic GPIO driver for logic cells found in the Nomadik SoC
+ * Pinmux & pinconf driver for the IP block found in the Nomadik SoC. This
+ * depends on gpio-nomadik and some handling is intertwined; see nmk_gpio_chips
+ * which is used by this driver to access the GPIO banks array.
  *
  * Copyright (C) 2008,2009 STMicroelectronics
  * Copyright (C) 2009 Alessandro Rubini <rubini@unipv.it>
@@ -25,6 +27,7 @@
 #include <linux/seq_file.h>
 #include <linux/slab.h>
 #include <linux/spinlock.h>
+#include <linux/types.h>
 
 /* Since we request GPIOs from ourself */
 #include <linux/pinctrl/consumer.h>
@@ -36,15 +39,7 @@
 #include "../core.h"
 #include "../pinctrl-utils.h"
 
-#include "pinctrl-nomadik.h"
-
-/*
- * The GPIO module in the Nomadik family of Systems-on-Chip is an
- * AMBA device, managing 32 pins and alternate functions.  The logic block
- * is currently used in the Nomadik and ux500.
- *
- * Symbols in this file are called "nmk_gpio" for "nomadik gpio"
- */
+#include <linux/gpio/gpio-nomadik.h>
 
 /*
  * pin configurations are represented by 32-bit integers:
@@ -200,75 +195,6 @@ typedef unsigned long pin_cfg_t;
 	(PIN_CFG_DEFAULT |\
 	 (PIN_NUM(num) | PIN_##alt | PIN_OUTPUT_##val))
 
-/*
- * "nmk_gpio" and "NMK_GPIO" stand for "Nomadik GPIO", leaving
- * the "gpio" namespace for generic and cross-machine functions
- */
-
-#define GPIO_BLOCK_SHIFT 5
-#define NMK_GPIO_PER_CHIP (1 << GPIO_BLOCK_SHIFT)
-#define NMK_MAX_BANKS DIV_ROUND_UP(512, NMK_GPIO_PER_CHIP)
-
-/* Register in the logic block */
-#define NMK_GPIO_DAT	0x00
-#define NMK_GPIO_DATS	0x04
-#define NMK_GPIO_DATC	0x08
-#define NMK_GPIO_PDIS	0x0c
-#define NMK_GPIO_DIR	0x10
-#define NMK_GPIO_DIRS	0x14
-#define NMK_GPIO_DIRC	0x18
-#define NMK_GPIO_SLPC	0x1c
-#define NMK_GPIO_AFSLA	0x20
-#define NMK_GPIO_AFSLB	0x24
-#define NMK_GPIO_LOWEMI	0x28
-
-#define NMK_GPIO_RIMSC	0x40
-#define NMK_GPIO_FIMSC	0x44
-#define NMK_GPIO_IS	0x48
-#define NMK_GPIO_IC	0x4c
-#define NMK_GPIO_RWIMSC	0x50
-#define NMK_GPIO_FWIMSC	0x54
-#define NMK_GPIO_WKS	0x58
-/* These appear in DB8540 and later ASICs */
-#define NMK_GPIO_EDGELEVEL 0x5C
-#define NMK_GPIO_LEVEL	0x60
-
-
-/* Pull up/down values */
-enum nmk_gpio_pull {
-	NMK_GPIO_PULL_NONE,
-	NMK_GPIO_PULL_UP,
-	NMK_GPIO_PULL_DOWN,
-};
-
-/* Sleep mode */
-enum nmk_gpio_slpm {
-	NMK_GPIO_SLPM_INPUT,
-	NMK_GPIO_SLPM_WAKEUP_ENABLE = NMK_GPIO_SLPM_INPUT,
-	NMK_GPIO_SLPM_NOCHANGE,
-	NMK_GPIO_SLPM_WAKEUP_DISABLE = NMK_GPIO_SLPM_NOCHANGE,
-};
-
-struct nmk_gpio_chip {
-	struct gpio_chip chip;
-	void __iomem *addr;
-	struct clk *clk;
-	unsigned int bank;
-	void (*set_ioforce)(bool enable);
-	spinlock_t lock;
-	bool sleepmode;
-	/* Keep track of configured edges */
-	u32 edge_rising;
-	u32 edge_falling;
-	u32 real_wake;
-	u32 rwimsc;
-	u32 fwimsc;
-	u32 rimsc;
-	u32 fimsc;
-	u32 pull_up;
-	u32 lowemi;
-};
-
 /**
  * struct nmk_pinctrl - state container for the Nomadik pin controller
  * @dev: containing device pointer
@@ -283,11 +209,10 @@ struct nmk_pinctrl {
 	void __iomem *prcm_base;
 };
 
-static struct nmk_gpio_chip *nmk_gpio_chips[NMK_MAX_BANKS];
+/* See nmk_gpio_populate_chip() that fills this array. */
+struct nmk_gpio_chip *nmk_gpio_chips[NMK_MAX_BANKS];
 
-static DEFINE_SPINLOCK(nmk_gpio_slpm_lock);
-
-#define NUM_BANKS ARRAY_SIZE(nmk_gpio_chips)
+DEFINE_SPINLOCK(nmk_gpio_slpm_lock);
 
 static void __nmk_gpio_set_mode(struct nmk_gpio_chip *nmk_chip,
 				unsigned offset, int gpio_mode)
@@ -304,19 +229,6 @@ static void __nmk_gpio_set_mode(struct nmk_gpio_chip *nmk_chip,
 	writel(bfunc, nmk_chip->addr + NMK_GPIO_AFSLB);
 }
 
-static void __nmk_gpio_set_slpm(struct nmk_gpio_chip *nmk_chip,
-				unsigned offset, enum nmk_gpio_slpm mode)
-{
-	u32 slpm;
-
-	slpm = readl(nmk_chip->addr + NMK_GPIO_SLPC);
-	if (mode == NMK_GPIO_SLPM_NOCHANGE)
-		slpm |= BIT(offset);
-	else
-		slpm &= ~BIT(offset);
-	writel(slpm, nmk_chip->addr + NMK_GPIO_SLPC);
-}
-
 static void __nmk_gpio_set_pull(struct nmk_gpio_chip *nmk_chip,
 				unsigned offset, enum nmk_gpio_pull pull)
 {
@@ -364,22 +276,6 @@ static void __nmk_gpio_make_input(struct nmk_gpio_chip *nmk_chip,
 	writel(BIT(offset), nmk_chip->addr + NMK_GPIO_DIRC);
 }
 
-static void __nmk_gpio_set_output(struct nmk_gpio_chip *nmk_chip,
-				  unsigned offset, int val)
-{
-	if (val)
-		writel(BIT(offset), nmk_chip->addr + NMK_GPIO_DATS);
-	else
-		writel(BIT(offset), nmk_chip->addr + NMK_GPIO_DATC);
-}
-
-static void __nmk_gpio_make_output(struct nmk_gpio_chip *nmk_chip,
-				  unsigned offset, int val)
-{
-	writel(BIT(offset), nmk_chip->addr + NMK_GPIO_DIRS);
-	__nmk_gpio_set_output(nmk_chip, offset, val);
-}
-
 static void __nmk_gpio_set_mode_safe(struct nmk_gpio_chip *nmk_chip,
 				     unsigned offset, int gpio_mode,
 				     bool glitch)
@@ -548,7 +444,7 @@ static void nmk_gpio_glitch_slpm_init(unsigned int *slpm)
 {
 	int i;
 
-	for (i = 0; i < NUM_BANKS; i++) {
+	for (i = 0; i < NMK_MAX_BANKS; i++) {
 		struct nmk_gpio_chip *chip = nmk_gpio_chips[i];
 		unsigned int temp = slpm[i];
 
@@ -566,7 +462,7 @@ static void nmk_gpio_glitch_slpm_restore(unsigned int *slpm)
 {
 	int i;
 
-	for (i = 0; i < NUM_BANKS; i++) {
+	for (i = 0; i < NMK_MAX_BANKS; i++) {
 		struct nmk_gpio_chip *chip = nmk_gpio_chips[i];
 
 		if (!chip)
@@ -578,7 +474,8 @@ static void nmk_gpio_glitch_slpm_restore(unsigned int *slpm)
 	}
 }
 
-static int __maybe_unused nmk_prcm_gpiocr_get_mode(struct pinctrl_dev *pctldev, int gpio)
+/* Only called by gpio-nomadik but requires knowledge of struct nmk_pinctrl. */
+int __maybe_unused nmk_prcm_gpiocr_get_mode(struct pinctrl_dev *pctldev, int gpio)
 {
 	int i;
 	u16 reg;
@@ -610,576 +507,6 @@ static int __maybe_unused nmk_prcm_gpiocr_get_mode(struct pinctrl_dev *pctldev,
 	return NMK_GPIO_ALT_C;
 }
 
-/* IRQ functions */
-
-static void nmk_gpio_irq_ack(struct irq_data *d)
-{
-	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
-	struct nmk_gpio_chip *nmk_chip = gpiochip_get_data(gc);
-
-	clk_enable(nmk_chip->clk);
-	writel(BIT(d->hwirq), nmk_chip->addr + NMK_GPIO_IC);
-	clk_disable(nmk_chip->clk);
-}
-
-enum nmk_gpio_irq_type {
-	NORMAL,
-	WAKE,
-};
-
-static void __nmk_gpio_irq_modify(struct nmk_gpio_chip *nmk_chip,
-				  int offset, enum nmk_gpio_irq_type which,
-				  bool enable)
-{
-	u32 *rimscval;
-	u32 *fimscval;
-	u32 rimscreg;
-	u32 fimscreg;
-
-	if (which == NORMAL) {
-		rimscreg = NMK_GPIO_RIMSC;
-		fimscreg = NMK_GPIO_FIMSC;
-		rimscval = &nmk_chip->rimsc;
-		fimscval = &nmk_chip->fimsc;
-	} else  {
-		rimscreg = NMK_GPIO_RWIMSC;
-		fimscreg = NMK_GPIO_FWIMSC;
-		rimscval = &nmk_chip->rwimsc;
-		fimscval = &nmk_chip->fwimsc;
-	}
-
-	/* we must individually set/clear the two edges */
-	if (nmk_chip->edge_rising & BIT(offset)) {
-		if (enable)
-			*rimscval |= BIT(offset);
-		else
-			*rimscval &= ~BIT(offset);
-		writel(*rimscval, nmk_chip->addr + rimscreg);
-	}
-	if (nmk_chip->edge_falling & BIT(offset)) {
-		if (enable)
-			*fimscval |= BIT(offset);
-		else
-			*fimscval &= ~BIT(offset);
-		writel(*fimscval, nmk_chip->addr + fimscreg);
-	}
-}
-
-static void __nmk_gpio_set_wake(struct nmk_gpio_chip *nmk_chip,
-				int offset, bool on)
-{
-	/*
-	 * Ensure WAKEUP_ENABLE is on.  No need to disable it if wakeup is
-	 * disabled, since setting SLPM to 1 increases power consumption, and
-	 * wakeup is anyhow controlled by the RIMSC and FIMSC registers.
-	 */
-	if (nmk_chip->sleepmode && on) {
-		__nmk_gpio_set_slpm(nmk_chip, offset,
-				    NMK_GPIO_SLPM_WAKEUP_ENABLE);
-	}
-
-	__nmk_gpio_irq_modify(nmk_chip, offset, WAKE, on);
-}
-
-static void nmk_gpio_irq_maskunmask(struct nmk_gpio_chip *nmk_chip,
-				    struct irq_data *d, bool enable)
-{
-	unsigned long flags;
-
-	clk_enable(nmk_chip->clk);
-	spin_lock_irqsave(&nmk_gpio_slpm_lock, flags);
-	spin_lock(&nmk_chip->lock);
-
-	__nmk_gpio_irq_modify(nmk_chip, d->hwirq, NORMAL, enable);
-
-	if (!(nmk_chip->real_wake & BIT(d->hwirq)))
-		__nmk_gpio_set_wake(nmk_chip, d->hwirq, enable);
-
-	spin_unlock(&nmk_chip->lock);
-	spin_unlock_irqrestore(&nmk_gpio_slpm_lock, flags);
-	clk_disable(nmk_chip->clk);
-}
-
-static void nmk_gpio_irq_mask(struct irq_data *d)
-{
-	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
-	struct nmk_gpio_chip *nmk_chip = gpiochip_get_data(gc);
-
-	nmk_gpio_irq_maskunmask(nmk_chip, d, false);
-	gpiochip_disable_irq(gc, irqd_to_hwirq(d));
-}
-
-static void nmk_gpio_irq_unmask(struct irq_data *d)
-{
-	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
-	struct nmk_gpio_chip *nmk_chip = gpiochip_get_data(gc);
-
-	gpiochip_enable_irq(gc, irqd_to_hwirq(d));
-	nmk_gpio_irq_maskunmask(nmk_chip, d, true);
-}
-
-static int nmk_gpio_irq_set_wake(struct irq_data *d, unsigned int on)
-{
-	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
-	struct nmk_gpio_chip *nmk_chip = gpiochip_get_data(gc);
-	unsigned long flags;
-
-	clk_enable(nmk_chip->clk);
-	spin_lock_irqsave(&nmk_gpio_slpm_lock, flags);
-	spin_lock(&nmk_chip->lock);
-
-	if (irqd_irq_disabled(d))
-		__nmk_gpio_set_wake(nmk_chip, d->hwirq, on);
-
-	if (on)
-		nmk_chip->real_wake |= BIT(d->hwirq);
-	else
-		nmk_chip->real_wake &= ~BIT(d->hwirq);
-
-	spin_unlock(&nmk_chip->lock);
-	spin_unlock_irqrestore(&nmk_gpio_slpm_lock, flags);
-	clk_disable(nmk_chip->clk);
-
-	return 0;
-}
-
-static int nmk_gpio_irq_set_type(struct irq_data *d, unsigned int type)
-{
-	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
-	struct nmk_gpio_chip *nmk_chip = gpiochip_get_data(gc);
-	bool enabled = !irqd_irq_disabled(d);
-	bool wake = irqd_is_wakeup_set(d);
-	unsigned long flags;
-
-	if (type & IRQ_TYPE_LEVEL_HIGH)
-		return -EINVAL;
-	if (type & IRQ_TYPE_LEVEL_LOW)
-		return -EINVAL;
-
-	clk_enable(nmk_chip->clk);
-	spin_lock_irqsave(&nmk_chip->lock, flags);
-
-	if (enabled)
-		__nmk_gpio_irq_modify(nmk_chip, d->hwirq, NORMAL, false);
-
-	if (enabled || wake)
-		__nmk_gpio_irq_modify(nmk_chip, d->hwirq, WAKE, false);
-
-	nmk_chip->edge_rising &= ~BIT(d->hwirq);
-	if (type & IRQ_TYPE_EDGE_RISING)
-		nmk_chip->edge_rising |= BIT(d->hwirq);
-
-	nmk_chip->edge_falling &= ~BIT(d->hwirq);
-	if (type & IRQ_TYPE_EDGE_FALLING)
-		nmk_chip->edge_falling |= BIT(d->hwirq);
-
-	if (enabled)
-		__nmk_gpio_irq_modify(nmk_chip, d->hwirq, NORMAL, true);
-
-	if (enabled || wake)
-		__nmk_gpio_irq_modify(nmk_chip, d->hwirq, WAKE, true);
-
-	spin_unlock_irqrestore(&nmk_chip->lock, flags);
-	clk_disable(nmk_chip->clk);
-
-	return 0;
-}
-
-static unsigned int nmk_gpio_irq_startup(struct irq_data *d)
-{
-	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
-	struct nmk_gpio_chip *nmk_chip = gpiochip_get_data(gc);
-
-	clk_enable(nmk_chip->clk);
-	nmk_gpio_irq_unmask(d);
-	return 0;
-}
-
-static void nmk_gpio_irq_shutdown(struct irq_data *d)
-{
-	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
-	struct nmk_gpio_chip *nmk_chip = gpiochip_get_data(gc);
-
-	nmk_gpio_irq_mask(d);
-	clk_disable(nmk_chip->clk);
-}
-
-static void nmk_gpio_irq_handler(struct irq_desc *desc)
-{
-	struct irq_chip *host_chip = irq_desc_get_chip(desc);
-	struct gpio_chip *chip = irq_desc_get_handler_data(desc);
-	struct nmk_gpio_chip *nmk_chip = gpiochip_get_data(chip);
-	u32 status;
-
-	chained_irq_enter(host_chip, desc);
-
-	clk_enable(nmk_chip->clk);
-	status = readl(nmk_chip->addr + NMK_GPIO_IS);
-	clk_disable(nmk_chip->clk);
-
-	while (status) {
-		int bit = __ffs(status);
-
-		generic_handle_domain_irq(chip->irq.domain, bit);
-		status &= ~BIT(bit);
-	}
-
-	chained_irq_exit(host_chip, desc);
-}
-
-/* I/O Functions */
-
-static int nmk_gpio_get_dir(struct gpio_chip *chip, unsigned offset)
-{
-	struct nmk_gpio_chip *nmk_chip = gpiochip_get_data(chip);
-	int dir;
-
-	clk_enable(nmk_chip->clk);
-
-	dir = readl(nmk_chip->addr + NMK_GPIO_DIR) & BIT(offset);
-
-	clk_disable(nmk_chip->clk);
-
-	if (dir)
-		return GPIO_LINE_DIRECTION_OUT;
-
-	return GPIO_LINE_DIRECTION_IN;
-}
-
-static int nmk_gpio_make_input(struct gpio_chip *chip, unsigned offset)
-{
-	struct nmk_gpio_chip *nmk_chip = gpiochip_get_data(chip);
-
-	clk_enable(nmk_chip->clk);
-
-	writel(BIT(offset), nmk_chip->addr + NMK_GPIO_DIRC);
-
-	clk_disable(nmk_chip->clk);
-
-	return 0;
-}
-
-static int nmk_gpio_get_input(struct gpio_chip *chip, unsigned offset)
-{
-	struct nmk_gpio_chip *nmk_chip = gpiochip_get_data(chip);
-	int value;
-
-	clk_enable(nmk_chip->clk);
-
-	value = !!(readl(nmk_chip->addr + NMK_GPIO_DAT) & BIT(offset));
-
-	clk_disable(nmk_chip->clk);
-
-	return value;
-}
-
-static void nmk_gpio_set_output(struct gpio_chip *chip, unsigned offset,
-				int val)
-{
-	struct nmk_gpio_chip *nmk_chip = gpiochip_get_data(chip);
-
-	clk_enable(nmk_chip->clk);
-
-	__nmk_gpio_set_output(nmk_chip, offset, val);
-
-	clk_disable(nmk_chip->clk);
-}
-
-static int nmk_gpio_make_output(struct gpio_chip *chip, unsigned offset,
-				int val)
-{
-	struct nmk_gpio_chip *nmk_chip = gpiochip_get_data(chip);
-
-	clk_enable(nmk_chip->clk);
-
-	__nmk_gpio_make_output(nmk_chip, offset, val);
-
-	clk_disable(nmk_chip->clk);
-
-	return 0;
-}
-
-#ifdef CONFIG_DEBUG_FS
-static int nmk_gpio_get_mode(struct nmk_gpio_chip *nmk_chip, int offset)
-{
-	u32 afunc, bfunc;
-
-	clk_enable(nmk_chip->clk);
-
-	afunc = readl(nmk_chip->addr + NMK_GPIO_AFSLA) & BIT(offset);
-	bfunc = readl(nmk_chip->addr + NMK_GPIO_AFSLB) & BIT(offset);
-
-	clk_disable(nmk_chip->clk);
-
-	return (afunc ? NMK_GPIO_ALT_A : 0) | (bfunc ? NMK_GPIO_ALT_B : 0);
-}
-
-static void nmk_gpio_dbg_show_one(struct seq_file *s,
-	struct pinctrl_dev *pctldev, struct gpio_chip *chip,
-	unsigned offset, unsigned gpio)
-{
-	struct nmk_gpio_chip *nmk_chip = gpiochip_get_data(chip);
-	int mode;
-	bool is_out;
-	bool data_out;
-	bool pull;
-	const char *modes[] = {
-		[NMK_GPIO_ALT_GPIO]	= "gpio",
-		[NMK_GPIO_ALT_A]	= "altA",
-		[NMK_GPIO_ALT_B]	= "altB",
-		[NMK_GPIO_ALT_C]	= "altC",
-		[NMK_GPIO_ALT_C+1]	= "altC1",
-		[NMK_GPIO_ALT_C+2]	= "altC2",
-		[NMK_GPIO_ALT_C+3]	= "altC3",
-		[NMK_GPIO_ALT_C+4]	= "altC4",
-	};
-
-	char *label = gpiochip_dup_line_label(chip, offset);
-	if (IS_ERR(label))
-		return;
-
-	clk_enable(nmk_chip->clk);
-	is_out = !!(readl(nmk_chip->addr + NMK_GPIO_DIR) & BIT(offset));
-	pull = !(readl(nmk_chip->addr + NMK_GPIO_PDIS) & BIT(offset));
-	data_out = !!(readl(nmk_chip->addr + NMK_GPIO_DAT) & BIT(offset));
-	mode = nmk_gpio_get_mode(nmk_chip, offset);
-	if ((mode == NMK_GPIO_ALT_C) && pctldev)
-		mode = nmk_prcm_gpiocr_get_mode(pctldev, gpio);
-
-	if (is_out) {
-		seq_printf(s, " gpio-%-3d (%-20.20s) out %s           %s",
-			   gpio,
-			   label ?: "(none)",
-			   data_out ? "hi" : "lo",
-			   (mode < 0) ? "unknown" : modes[mode]);
-	} else {
-		int irq = chip->to_irq(chip, offset);
-		const int pullidx = pull ? 1 : 0;
-		int val;
-		static const char * const pulls[] = {
-			"none        ",
-			"pull enabled",
-		};
-
-		seq_printf(s, " gpio-%-3d (%-20.20s) in  %s %s",
-			   gpio,
-			   label ?: "(none)",
-			   pulls[pullidx],
-			   (mode < 0) ? "unknown" : modes[mode]);
-
-		val = nmk_gpio_get_input(chip, offset);
-		seq_printf(s, " VAL %d", val);
-
-		/*
-		 * This races with request_irq(), set_irq_type(),
-		 * and set_irq_wake() ... but those are "rare".
-		 */
-		if (irq > 0 && irq_has_action(irq)) {
-			char *trigger;
-			bool wake;
-
-			if (nmk_chip->edge_rising & BIT(offset))
-				trigger = "edge-rising";
-			else if (nmk_chip->edge_falling & BIT(offset))
-				trigger = "edge-falling";
-			else
-				trigger = "edge-undefined";
-
-			wake = !!(nmk_chip->real_wake & BIT(offset));
-
-			seq_printf(s, " irq-%d %s%s",
-				   irq, trigger, wake ? " wakeup" : "");
-		}
-	}
-	clk_disable(nmk_chip->clk);
-}
-
-static void nmk_gpio_dbg_show(struct seq_file *s, struct gpio_chip *chip)
-{
-	unsigned		i;
-	unsigned		gpio = chip->base;
-
-	for (i = 0; i < chip->ngpio; i++, gpio++) {
-		nmk_gpio_dbg_show_one(s, NULL, chip, i, gpio);
-		seq_printf(s, "\n");
-	}
-}
-
-#else
-static inline void nmk_gpio_dbg_show_one(struct seq_file *s,
-					 struct pinctrl_dev *pctldev,
-					 struct gpio_chip *chip,
-					 unsigned offset, unsigned gpio)
-{
-}
-#define nmk_gpio_dbg_show	NULL
-#endif
-
-/*
- * We will allocate memory for the state container using devm* allocators
- * binding to the first device reaching this point, it doesn't matter if
- * it is the pin controller or GPIO driver. However we need to use the right
- * platform device when looking up resources so pay attention to pdev.
- */
-static struct nmk_gpio_chip *nmk_gpio_populate_chip(struct device_node *np,
-						struct platform_device *pdev)
-{
-	struct nmk_gpio_chip *nmk_chip;
-	struct platform_device *gpio_pdev;
-	struct gpio_chip *chip;
-	struct resource *res;
-	struct clk *clk;
-	void __iomem *base;
-	u32 id;
-
-	gpio_pdev = of_find_device_by_node(np);
-	if (!gpio_pdev) {
-		pr_err("populate \"%pOFn\": device not found\n", np);
-		return ERR_PTR(-ENODEV);
-	}
-	if (of_property_read_u32(np, "gpio-bank", &id)) {
-		dev_err(&pdev->dev, "populate: gpio-bank property not found\n");
-		platform_device_put(gpio_pdev);
-		return ERR_PTR(-EINVAL);
-	}
-
-	/* Already populated? */
-	nmk_chip = nmk_gpio_chips[id];
-	if (nmk_chip) {
-		platform_device_put(gpio_pdev);
-		return nmk_chip;
-	}
-
-	nmk_chip = devm_kzalloc(&pdev->dev, sizeof(*nmk_chip), GFP_KERNEL);
-	if (!nmk_chip) {
-		platform_device_put(gpio_pdev);
-		return ERR_PTR(-ENOMEM);
-	}
-
-	nmk_chip->bank = id;
-	chip = &nmk_chip->chip;
-	chip->base = id * NMK_GPIO_PER_CHIP;
-	chip->ngpio = NMK_GPIO_PER_CHIP;
-	chip->label = dev_name(&gpio_pdev->dev);
-	chip->parent = &gpio_pdev->dev;
-
-	res = platform_get_resource(gpio_pdev, IORESOURCE_MEM, 0);
-	base = devm_ioremap_resource(&pdev->dev, res);
-	if (IS_ERR(base)) {
-		platform_device_put(gpio_pdev);
-		return ERR_CAST(base);
-	}
-	nmk_chip->addr = base;
-
-	clk = clk_get(&gpio_pdev->dev, NULL);
-	if (IS_ERR(clk)) {
-		platform_device_put(gpio_pdev);
-		return (void *) clk;
-	}
-	clk_prepare(clk);
-	nmk_chip->clk = clk;
-
-	BUG_ON(nmk_chip->bank >= ARRAY_SIZE(nmk_gpio_chips));
-	nmk_gpio_chips[id] = nmk_chip;
-	return nmk_chip;
-}
-
-static void nmk_gpio_irq_print_chip(struct irq_data *d, struct seq_file *p)
-{
-	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
-	struct nmk_gpio_chip *nmk_chip = gpiochip_get_data(gc);
-
-	seq_printf(p, "nmk%u-%u-%u", nmk_chip->bank,
-		   gc->base, gc->base + gc->ngpio - 1);
-}
-
-static const struct irq_chip nmk_irq_chip = {
-	.irq_ack = nmk_gpio_irq_ack,
-	.irq_mask = nmk_gpio_irq_mask,
-	.irq_unmask = nmk_gpio_irq_unmask,
-	.irq_set_type = nmk_gpio_irq_set_type,
-	.irq_set_wake = nmk_gpio_irq_set_wake,
-	.irq_startup = nmk_gpio_irq_startup,
-	.irq_shutdown = nmk_gpio_irq_shutdown,
-	.irq_print_chip = nmk_gpio_irq_print_chip,
-	.flags = IRQCHIP_MASK_ON_SUSPEND | IRQCHIP_IMMUTABLE,
-	GPIOCHIP_IRQ_RESOURCE_HELPERS,
-};
-
-static int nmk_gpio_probe(struct platform_device *dev)
-{
-	struct device_node *np = dev->dev.of_node;
-	struct nmk_gpio_chip *nmk_chip;
-	struct gpio_chip *chip;
-	struct gpio_irq_chip *girq;
-	bool supports_sleepmode;
-	int irq;
-	int ret;
-
-	nmk_chip = nmk_gpio_populate_chip(np, dev);
-	if (IS_ERR(nmk_chip)) {
-		dev_err(&dev->dev, "could not populate nmk chip struct\n");
-		return PTR_ERR(nmk_chip);
-	}
-
-	supports_sleepmode =
-		of_property_read_bool(np, "st,supports-sleepmode");
-
-	/* Correct platform device ID */
-	dev->id = nmk_chip->bank;
-
-	irq = platform_get_irq(dev, 0);
-	if (irq < 0)
-		return irq;
-
-	/*
-	 * The virt address in nmk_chip->addr is in the nomadik register space,
-	 * so we can simply convert the resource address, without remapping
-	 */
-	nmk_chip->sleepmode = supports_sleepmode;
-	spin_lock_init(&nmk_chip->lock);
-
-	chip = &nmk_chip->chip;
-	chip->parent = &dev->dev;
-	chip->request = gpiochip_generic_request;
-	chip->free = gpiochip_generic_free;
-	chip->get_direction = nmk_gpio_get_dir;
-	chip->direction_input = nmk_gpio_make_input;
-	chip->get = nmk_gpio_get_input;
-	chip->direction_output = nmk_gpio_make_output;
-	chip->set = nmk_gpio_set_output;
-	chip->dbg_show = nmk_gpio_dbg_show;
-	chip->can_sleep = false;
-	chip->owner = THIS_MODULE;
-
-	girq = &chip->irq;
-	gpio_irq_chip_set_chip(girq, &nmk_irq_chip);
-	girq->parent_handler = nmk_gpio_irq_handler;
-	girq->num_parents = 1;
-	girq->parents = devm_kcalloc(&dev->dev, 1,
-				     sizeof(*girq->parents),
-				     GFP_KERNEL);
-	if (!girq->parents)
-		return -ENOMEM;
-	girq->parents[0] = irq;
-	girq->default_type = IRQ_TYPE_NONE;
-	girq->handler = handle_edge_irq;
-
-	clk_enable(nmk_chip->clk);
-	nmk_chip->lowemi = readl_relaxed(nmk_chip->addr + NMK_GPIO_LOWEMI);
-	clk_disable(nmk_chip->clk);
-
-	ret = gpiochip_add_data(chip, nmk_chip);
-	if (ret)
-		return ret;
-
-	platform_set_drvdata(dev, nmk_chip);
-
-	dev_info(&dev->dev, "chip registered\n");
-
-	return 0;
-}
-
 static int nmk_get_groups_cnt(struct pinctrl_dev *pctldev)
 {
 	struct nmk_pinctrl *npct = pinctrl_dev_get_drvdata(pctldev);
@@ -1197,12 +524,12 @@ static const char *nmk_get_group_name(struct pinctrl_dev *pctldev,
 
 static int nmk_get_group_pins(struct pinctrl_dev *pctldev, unsigned selector,
 			      const unsigned **pins,
-			      unsigned *npins)
+			      unsigned int *num_pins)
 {
 	struct nmk_pinctrl *npct = pinctrl_dev_get_drvdata(pctldev);
 
 	*pins = npct->soc->groups[selector].grp.pins;
-	*npins = npct->soc->groups[selector].grp.npins;
+	*num_pins = npct->soc->groups[selector].grp.npins;
 	return 0;
 }
 
@@ -1533,7 +860,7 @@ static int nmk_pmx_set(struct pinctrl_dev *pctldev, unsigned function,
 {
 	struct nmk_pinctrl *npct = pinctrl_dev_get_drvdata(pctldev);
 	const struct nmk_pingroup *g;
-	static unsigned int slpm[NUM_BANKS];
+	static unsigned int slpm[NMK_MAX_BANKS];
 	unsigned long flags = 0;
 	bool glitch;
 	int ret = -EINVAL;
@@ -1919,19 +1246,6 @@ static int nmk_pinctrl_probe(struct platform_device *pdev)
 	return 0;
 }
 
-static const struct of_device_id nmk_gpio_match[] = {
-	{ .compatible = "st,nomadik-gpio", },
-	{}
-};
-
-static struct platform_driver nmk_gpio_driver = {
-	.driver = {
-		.name = "gpio",
-		.of_match_table = nmk_gpio_match,
-	},
-	.probe = nmk_gpio_probe,
-};
-
 static SIMPLE_DEV_PM_OPS(nmk_pinctrl_pm_ops,
 			nmk_pinctrl_suspend,
 			nmk_pinctrl_resume);
@@ -1945,12 +1259,6 @@ static struct platform_driver nmk_pinctrl_driver = {
 	.probe = nmk_pinctrl_probe,
 };
 
-static int __init nmk_gpio_init(void)
-{
-	return platform_driver_register(&nmk_gpio_driver);
-}
-subsys_initcall(nmk_gpio_init);
-
 static int __init nmk_pinctrl_init(void)
 {
 	return platform_driver_register(&nmk_pinctrl_driver);
diff --git a/drivers/pinctrl/nomadik/pinctrl-nomadik.h b/include/linux/gpio/gpio-nomadik.h
similarity index 61%
rename from drivers/pinctrl/nomadik/pinctrl-nomadik.h
rename to include/linux/gpio/gpio-nomadik.h
index 1ef2559bc571..0166ddb71f43 100644
--- a/drivers/pinctrl/nomadik/pinctrl-nomadik.h
+++ b/include/linux/gpio/gpio-nomadik.h
@@ -1,16 +1,74 @@
 /* SPDX-License-Identifier: GPL-2.0 */
-#ifndef PINCTRL_PINCTRL_NOMADIK_H
-#define PINCTRL_PINCTRL_NOMADIK_H
-
-#include <linux/kernel.h>
-#include <linux/types.h>
-
-#include <linux/pinctrl/pinctrl.h>
+#ifndef __LINUX_GPIO_NOMADIK_H
+#define __LINUX_GPIO_NOMADIK_H
 
 /* Package definitions */
 #define PINCTRL_NMK_STN8815	0
 #define PINCTRL_NMK_DB8500	1
 
+#define GPIO_BLOCK_SHIFT 5
+#define NMK_GPIO_PER_CHIP BIT(GPIO_BLOCK_SHIFT)
+#define NMK_MAX_BANKS DIV_ROUND_UP(512, NMK_GPIO_PER_CHIP)
+
+/* Register in the logic block */
+#define NMK_GPIO_DAT	0x00
+#define NMK_GPIO_DATS	0x04
+#define NMK_GPIO_DATC	0x08
+#define NMK_GPIO_PDIS	0x0c
+#define NMK_GPIO_DIR	0x10
+#define NMK_GPIO_DIRS	0x14
+#define NMK_GPIO_DIRC	0x18
+#define NMK_GPIO_SLPC	0x1c
+#define NMK_GPIO_AFSLA	0x20
+#define NMK_GPIO_AFSLB	0x24
+#define NMK_GPIO_LOWEMI	0x28
+
+#define NMK_GPIO_RIMSC	0x40
+#define NMK_GPIO_FIMSC	0x44
+#define NMK_GPIO_IS	0x48
+#define NMK_GPIO_IC	0x4c
+#define NMK_GPIO_RWIMSC	0x50
+#define NMK_GPIO_FWIMSC	0x54
+#define NMK_GPIO_WKS	0x58
+/* These appear in DB8540 and later ASICs */
+#define NMK_GPIO_EDGELEVEL 0x5C
+#define NMK_GPIO_LEVEL	0x60
+
+/* Pull up/down values */
+enum nmk_gpio_pull {
+	NMK_GPIO_PULL_NONE,
+	NMK_GPIO_PULL_UP,
+	NMK_GPIO_PULL_DOWN,
+};
+
+/* Sleep mode */
+enum nmk_gpio_slpm {
+	NMK_GPIO_SLPM_INPUT,
+	NMK_GPIO_SLPM_WAKEUP_ENABLE = NMK_GPIO_SLPM_INPUT,
+	NMK_GPIO_SLPM_NOCHANGE,
+	NMK_GPIO_SLPM_WAKEUP_DISABLE = NMK_GPIO_SLPM_NOCHANGE,
+};
+
+struct nmk_gpio_chip {
+	struct gpio_chip chip;
+	void __iomem *addr;
+	struct clk *clk;
+	unsigned int bank;
+	void (*set_ioforce)(bool enable);
+	spinlock_t lock;
+	bool sleepmode;
+	/* Keep track of configured edges */
+	u32 edge_rising;
+	u32 edge_falling;
+	u32 real_wake;
+	u32 rwimsc;
+	u32 fwimsc;
+	u32 rimsc;
+	u32 fimsc;
+	u32 pull_up;
+	u32 lowemi;
+};
+
 /* Alternate functions: function C is set in hw by setting both A and B */
 #define NMK_GPIO_ALT_GPIO	0
 #define NMK_GPIO_ALT_A	1
@@ -104,7 +162,7 @@ struct prcm_gpiocr_altcx_pin_desc {
 struct nmk_function {
 	const char *name;
 	const char * const *groups;
-	unsigned ngroups;
+	unsigned int ngroups;
 };
 
 /**
@@ -141,13 +199,13 @@ struct nmk_pingroup {
  */
 struct nmk_pinctrl_soc_data {
 	const struct pinctrl_pin_desc *pins;
-	unsigned npins;
+	unsigned int npins;
 	const struct nmk_function *functions;
-	unsigned nfunctions;
+	unsigned int nfunctions;
 	const struct nmk_pingroup *groups;
-	unsigned ngroups;
+	unsigned int ngroups;
 	const struct prcm_gpiocr_altcx_pin_desc *altcx_pins;
-	unsigned npins_altcx;
+	unsigned int npins_altcx;
 	const u16 *prcm_gpiocr_registers;
 };
 
@@ -177,4 +235,42 @@ nmk_pinctrl_db8500_init(const struct nmk_pinctrl_soc_data **soc)
 
 #endif
 
-#endif /* PINCTRL_PINCTRL_NOMADIK_H */
+#ifdef CONFIG_PINCTRL_DB8540
+
+void nmk_pinctrl_db8540_init(const struct nmk_pinctrl_soc_data **soc);
+
+#else
+
+static inline void
+nmk_pinctrl_db8540_init(const struct nmk_pinctrl_soc_data **soc)
+{
+}
+
+#endif
+
+struct platform_device;
+
+/*
+ * Symbols declared in gpio-nomadik used by pinctrl-nomadik. If pinctrl-nomadik
+ * is enabled, then gpio-nomadik is enabled as well; the reverse if not always
+ * true.
+ */
+void nmk_gpio_dbg_show_one(struct seq_file *s, struct pinctrl_dev *pctldev,
+			   struct gpio_chip *chip, unsigned int offset,
+			   unsigned int gpio);
+void __nmk_gpio_make_output(struct nmk_gpio_chip *nmk_chip,
+			    unsigned int offset, int val);
+void __nmk_gpio_set_slpm(struct nmk_gpio_chip *nmk_chip, unsigned int offset,
+			 enum nmk_gpio_slpm mode);
+struct nmk_gpio_chip *nmk_gpio_populate_chip(struct device_node *np,
+					     struct platform_device *pdev);
+
+/* Symbols declared in pinctrl-nomadik used by gpio-nomadik. */
+#ifdef CONFIG_PINCTRL_NOMADIK
+extern struct nmk_gpio_chip *nmk_gpio_chips[NMK_MAX_BANKS];
+extern spinlock_t nmk_gpio_slpm_lock;
+int __maybe_unused nmk_prcm_gpiocr_get_mode(struct pinctrl_dev *pctldev,
+					    int gpio);
+#endif
+
+#endif /* __LINUX_GPIO_NOMADIK_H */

-- 
2.43.1

Re: [PATCH 05/23] gpio: nomadik: extract GPIO platform driver from drivers/pinctrl/nomadik/

[Philipp Zabel]

On Mi, 2024-02-14 at 17:23 +0100, Théo Lebrun wrote:
[...]
> diff --git a/drivers/gpio/gpio-nomadik.c b/drivers/gpio/gpio-nomadik.c
> new file mode 100644
> index 000000000000..e39477e1a58f
> --- /dev/null
> +++ b/drivers/gpio/gpio-nomadik.c
> @@ -0,0 +1,660 @@
[...]
> +static int nmk_gpio_probe(struct platform_device *dev)
> +{
[...]
> +	ret = gpiochip_add_data(chip, nmk_chip);

Use devm_gpiochip_add_data() to cleanup on unbind, before nmk_chip goes
away. Or make the driver un-unbindable via suppress_bind_attrs. In that
case you could drop devm_ prefixes everywhere for consistency.

regards
Philipp

Re: [PATCH 05/23] gpio: nomadik: extract GPIO platform driver from drivers/pinctrl/nomadik/

[Théo Lebrun]

Hello,

On Thu Feb 15, 2024 at 11:03 AM CET, Philipp Zabel wrote:
> On Mi, 2024-02-14 at 17:23 +0100, Théo Lebrun wrote:
> [...]
> > diff --git a/drivers/gpio/gpio-nomadik.c b/drivers/gpio/gpio-nomadik.c
> > new file mode 100644
> > index 000000000000..e39477e1a58f
> > --- /dev/null
> > +++ b/drivers/gpio/gpio-nomadik.c
> > @@ -0,0 +1,660 @@
> [...]
> > +static int nmk_gpio_probe(struct platform_device *dev)
> > +{
> [...]
> > +	ret = gpiochip_add_data(chip, nmk_chip);
>
> Use devm_gpiochip_add_data() to cleanup on unbind, before nmk_chip goes
> away. Or make the driver un-unbindable via suppress_bind_attrs. In that
> case you could drop devm_ prefixes everywhere for consistency.

Disabling unbind sounds like the best option. Will do so in next
revision, in a separate patch to keep this one as close of a copy-paste
as possible.

Thanks,

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH 05/23] gpio: nomadik: extract GPIO platform driver from drivers/pinctrl/nomadik/

[Bartosz Golaszewski]

On Thu, Feb 15, 2024 at 11:03 AM Philipp Zabel <p.zabel@pengutronix.de> wrote:
>
> On Mi, 2024-02-14 at 17:23 +0100, Théo Lebrun wrote:
> [...]
> > diff --git a/drivers/gpio/gpio-nomadik.c b/drivers/gpio/gpio-nomadik.c
> > new file mode 100644
> > index 000000000000..e39477e1a58f
> > --- /dev/null
> > +++ b/drivers/gpio/gpio-nomadik.c
> > @@ -0,0 +1,660 @@
> [...]
> > +static int nmk_gpio_probe(struct platform_device *dev)
> > +{
> [...]
> > +     ret = gpiochip_add_data(chip, nmk_chip);
>
> Use devm_gpiochip_add_data() to cleanup on unbind, before nmk_chip goes
> away. Or make the driver un-unbindable via suppress_bind_attrs. In that
> case you could drop devm_ prefixes everywhere for consistency.
>

No! Why? What about error paths in probe() where you want to undo everything?

Bart

> regards
> Philipp

Re: [PATCH 05/23] gpio: nomadik: extract GPIO platform driver from drivers/pinctrl/nomadik/

[Philipp Zabel]

Hi Bartosz,

On Mo, 2024-02-19 at 16:33 +0100, Bartosz Golaszewski wrote:
> On Thu, Feb 15, 2024 at 11:03 AM Philipp Zabel <p.zabel@pengutronix.de> wrote:
> > 
> > On Mi, 2024-02-14 at 17:23 +0100, Théo Lebrun wrote:
> > [...]
> > > diff --git a/drivers/gpio/gpio-nomadik.c b/drivers/gpio/gpio-nomadik.c
> > > new file mode 100644
> > > index 000000000000..e39477e1a58f
> > > --- /dev/null
> > > +++ b/drivers/gpio/gpio-nomadik.c
> > > @@ -0,0 +1,660 @@
> > [...]
> > > +static int nmk_gpio_probe(struct platform_device *dev)
> > > +{
> > [...]
> > > +     ret = gpiochip_add_data(chip, nmk_chip);
> > 
> > Use devm_gpiochip_add_data() to cleanup on unbind, before nmk_chip goes
> > away. Or make the driver un-unbindable via suppress_bind_attrs. In that
> > case you could drop devm_ prefixes everywhere for consistency.
> > 
> 
> No! Why? What about error paths in probe() where you want to undo everything?

Brain fog moment. I was triggered by the mixture of devm_ and non-devm_
calls and jumped to the wrong conclusion.

Yes, keeping devm_ for error cleanup is of course correct, and with
suppress_bind_attrs it'd even be ok to use non-devm_
gpiochip_add_data(), as long as there can be no error return
afterwards.

regards
Philipp

Re: [PATCH 05/23] gpio: nomadik: extract GPIO platform driver from drivers/pinctrl/nomadik/

[Bartosz Golaszewski]

On Wed, Feb 14, 2024 at 5:24 PM Théo Lebrun <theo.lebrun@bootlin.com> wrote:
>
> Previously, drivers/pinctrl/nomadik/pinctrl-nomadik.c registered two
> platform drivers: pinctrl & GPIO. Move the GPIO aspect to the
> drivers/gpio/ folder, as would be expected.
>
> Both drivers are intertwined for a reason; pinctrl requires access to
> GPIO registers for pinmuxing, pull-disable, disabling interrupts while
> setting the muxing and wakeup control. Information sharing is done
> through a shared array containing GPIO chips and a few helper
> functions. That shared array is not touched from gpio-nomadik when
> CONFIG_PINCTRL_NOMADIK is not defined.
>
> Make no change to the code that moved into gpio-nomadik; there should be
> no behavior change following. A few functions are shared and header
> comments are added. Checkpatch warnings are addressed. NUM_BANKS is
> renamed to NMK_MAX_BANKS.
>
> It is supported to compile gpio-nomadik without pinctrl-nomadik. The
> opposite is not true.
>
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> ---
>  MAINTAINERS                                        |   1 +
>  drivers/gpio/Kconfig                               |  12 +
>  drivers/gpio/Makefile                              |   1 +
>  drivers/gpio/gpio-nomadik.c                        | 660 +++++++++++++++++++
>  drivers/pinctrl/nomadik/Kconfig                    |   5 +-
>  drivers/pinctrl/nomadik/pinctrl-nomadik-db8500.c   |   3 +-
>  drivers/pinctrl/nomadik/pinctrl-nomadik-stn8815.c  |   3 +-
>  drivers/pinctrl/nomadik/pinctrl-nomadik.c          | 722 +--------------------
>  .../linux/gpio/gpio-nomadik.h                      | 122 +++-
>  9 files changed, 804 insertions(+), 725 deletions(-)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 0cb2c459d1cf..3f864e773267 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2474,6 +2474,7 @@ F:        drivers/clk/clk-nomadik.c
>  F:     drivers/clocksource/clksrc-dbx500-prcmu.c
>  F:     drivers/dma/ste_dma40*
>  F:     drivers/pmdomain/st/ste-ux500-pm-domain.c
> +F:     drivers/gpio/gpio-nomadik.c
>  F:     drivers/hwspinlock/u8500_hsem.c
>  F:     drivers/i2c/busses/i2c-nomadik.c
>  F:     drivers/iio/adc/ab8500-gpadc.c
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index 1301cec94f12..ff83371251c1 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -478,6 +478,18 @@ config GPIO_MXS
>         select GPIO_GENERIC
>         select GENERIC_IRQ_CHIP
>
> +config GPIO_NOMADIK
> +       bool "Nomadik GPIO driver"
> +       depends on ARCH_U8500 || ARCH_NOMADIK || COMPILE_TEST
> +       select OF_GPIO
> +       select GPIOLIB_IRQCHIP
> +       help
> +         Say yes here to support the Nomadik SoC GPIO block.
> +
> +         It handles up to 32 GPIOs per bank, that can all be interrupt sources.
> +         It is deeply interconnected with the associated pinctrl driver as GPIO
> +         registers handle muxing ("alternate functions") as well.
> +
>  config GPIO_NPCM_SGPIO
>         bool "Nuvoton SGPIO support"
>         depends on ARCH_NPCM || COMPILE_TEST
> diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
> index 9e40af196aae..9fc2f5931b22 100644
> --- a/drivers/gpio/Makefile
> +++ b/drivers/gpio/Makefile
> @@ -116,6 +116,7 @@ obj-$(CONFIG_GPIO_MT7621)           += gpio-mt7621.o
>  obj-$(CONFIG_GPIO_MVEBU)               += gpio-mvebu.o
>  obj-$(CONFIG_GPIO_MXC)                 += gpio-mxc.o
>  obj-$(CONFIG_GPIO_MXS)                 += gpio-mxs.o
> +obj-$(CONFIG_GPIO_NOMADIK)             += gpio-nomadik.o
>  obj-$(CONFIG_GPIO_NPCM_SGPIO)          += gpio-npcm-sgpio.o
>  obj-$(CONFIG_GPIO_OCTEON)              += gpio-octeon.o
>  obj-$(CONFIG_GPIO_OMAP)                        += gpio-omap.o
> diff --git a/drivers/gpio/gpio-nomadik.c b/drivers/gpio/gpio-nomadik.c
> new file mode 100644
> index 000000000000..e39477e1a58f
> --- /dev/null
> +++ b/drivers/gpio/gpio-nomadik.c
> @@ -0,0 +1,660 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * GPIO driver for the IP block found in the Nomadik SoC; it is an AMBA device,
> + * managing 32 pins with alternate functions. It can also handle the STA2X11
> + * block from ST.
> + *
> + * The GPIO chips are shared with pinctrl-nomadik if used; it needs access for
> + * pinmuxing functionality and others.
> + *
> + * Copyright (C) 2008,2009 STMicroelectronics
> + * Copyright (C) 2009 Alessandro Rubini <rubini@unipv.it>
> + *   Rewritten based on work by Prafulla WADASKAR <prafulla.wadaskar@st.com>
> + * Copyright (C) 2011-2013 Linus Walleij <linus.walleij@linaro.org>
> + */

Add a newline here.

> +#include <linux/cleanup.h>
> +#include <linux/clk.h>
> +#include <linux/gpio/driver.h>
> +#include <linux/interrupt.h>
> +#include <linux/kernel.h>
> +#include <linux/of_device.h>
> +#include <linux/of_platform.h>
> +#include <linux/pinctrl/pinctrl.h>
> +#include <linux/platform_device.h>
> +#include <linux/reset.h>
> +#include <linux/seq_file.h>
> +#include <linux/types.h>

You need linux/slab.h for GFP flags.

> +
> +#include <linux/gpio/gpio-nomadik.h>
> +
> +#ifndef CONFIG_PINCTRL_NOMADIK
> +static DEFINE_SPINLOCK(nmk_gpio_slpm_lock);
> +#endif
> +
> +void __nmk_gpio_set_slpm(struct nmk_gpio_chip *nmk_chip, unsigned int offset,
> +                        enum nmk_gpio_slpm mode)
> +{
> +       u32 slpm;
> +
> +       slpm = readl(nmk_chip->addr + NMK_GPIO_SLPC);
> +       if (mode == NMK_GPIO_SLPM_NOCHANGE)
> +               slpm |= BIT(offset);
> +       else
> +               slpm &= ~BIT(offset);
> +       writel(slpm, nmk_chip->addr + NMK_GPIO_SLPC);
> +}
> +
> +static void __nmk_gpio_set_output(struct nmk_gpio_chip *nmk_chip,
> +                                 unsigned int offset, int val)
> +{
> +       if (val)
> +               writel(BIT(offset), nmk_chip->addr + NMK_GPIO_DATS);
> +       else
> +               writel(BIT(offset), nmk_chip->addr + NMK_GPIO_DATC);
> +}
> +
> +void __nmk_gpio_make_output(struct nmk_gpio_chip *nmk_chip,
> +                           unsigned int offset, int val)
> +{
> +       writel(BIT(offset), nmk_chip->addr + NMK_GPIO_DIRS);
> +       __nmk_gpio_set_output(nmk_chip, offset, val);
> +}
> +
> +/* IRQ functions */
> +
> +static void nmk_gpio_irq_ack(struct irq_data *d)
> +{
> +       struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
> +       struct nmk_gpio_chip *nmk_chip = gpiochip_get_data(gc);
> +
> +       clk_enable(nmk_chip->clk);
> +       writel(BIT(d->hwirq), nmk_chip->addr + NMK_GPIO_IC);
> +       clk_disable(nmk_chip->clk);
> +}
> +
> +enum nmk_gpio_irq_type {
> +       NORMAL,
> +       WAKE,
> +};
> +
> +static void __nmk_gpio_irq_modify(struct nmk_gpio_chip *nmk_chip,
> +                                 int offset, enum nmk_gpio_irq_type which,
> +                                 bool enable)
> +{
> +       u32 *rimscval;
> +       u32 *fimscval;
> +       u32 rimscreg;
> +       u32 fimscreg;
> +
> +       if (which == NORMAL) {
> +               rimscreg = NMK_GPIO_RIMSC;
> +               fimscreg = NMK_GPIO_FIMSC;
> +               rimscval = &nmk_chip->rimsc;
> +               fimscval = &nmk_chip->fimsc;
> +       } else  {
> +               rimscreg = NMK_GPIO_RWIMSC;
> +               fimscreg = NMK_GPIO_FWIMSC;
> +               rimscval = &nmk_chip->rwimsc;
> +               fimscval = &nmk_chip->fwimsc;
> +       }
> +
> +       /* we must individually set/clear the two edges */
> +       if (nmk_chip->edge_rising & BIT(offset)) {
> +               if (enable)
> +                       *rimscval |= BIT(offset);
> +               else
> +                       *rimscval &= ~BIT(offset);
> +               writel(*rimscval, nmk_chip->addr + rimscreg);
> +       }
> +       if (nmk_chip->edge_falling & BIT(offset)) {
> +               if (enable)
> +                       *fimscval |= BIT(offset);
> +               else
> +                       *fimscval &= ~BIT(offset);
> +               writel(*fimscval, nmk_chip->addr + fimscreg);
> +       }
> +}
> +
> +static void __nmk_gpio_set_wake(struct nmk_gpio_chip *nmk_chip,
> +                               int offset, bool on)
> +{
> +       /*
> +        * Ensure WAKEUP_ENABLE is on.  No need to disable it if wakeup is
> +        * disabled, since setting SLPM to 1 increases power consumption, and
> +        * wakeup is anyhow controlled by the RIMSC and FIMSC registers.
> +        */
> +       if (nmk_chip->sleepmode && on) {
> +               __nmk_gpio_set_slpm(nmk_chip, offset,
> +                                   NMK_GPIO_SLPM_WAKEUP_ENABLE);
> +       }
> +
> +       __nmk_gpio_irq_modify(nmk_chip, offset, WAKE, on);
> +}
> +
> +static void nmk_gpio_irq_maskunmask(struct nmk_gpio_chip *nmk_chip,
> +                                   struct irq_data *d, bool enable)
> +{
> +       unsigned long flags;
> +
> +       clk_enable(nmk_chip->clk);
> +       spin_lock_irqsave(&nmk_gpio_slpm_lock, flags);
> +       spin_lock(&nmk_chip->lock);
> +
> +       __nmk_gpio_irq_modify(nmk_chip, d->hwirq, NORMAL, enable);
> +
> +       if (!(nmk_chip->real_wake & BIT(d->hwirq)))
> +               __nmk_gpio_set_wake(nmk_chip, d->hwirq, enable);
> +
> +       spin_unlock(&nmk_chip->lock);
> +       spin_unlock_irqrestore(&nmk_gpio_slpm_lock, flags);
> +       clk_disable(nmk_chip->clk);
> +}
> +
> +static void nmk_gpio_irq_mask(struct irq_data *d)
> +{
> +       struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
> +       struct nmk_gpio_chip *nmk_chip = gpiochip_get_data(gc);
> +
> +       nmk_gpio_irq_maskunmask(nmk_chip, d, false);
> +       gpiochip_disable_irq(gc, irqd_to_hwirq(d));
> +}
> +
> +static void nmk_gpio_irq_unmask(struct irq_data *d)
> +{
> +       struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
> +       struct nmk_gpio_chip *nmk_chip = gpiochip_get_data(gc);
> +
> +       gpiochip_enable_irq(gc, irqd_to_hwirq(d));
> +       nmk_gpio_irq_maskunmask(nmk_chip, d, true);
> +}
> +
> +static int nmk_gpio_irq_set_wake(struct irq_data *d, unsigned int on)
> +{
> +       struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
> +       struct nmk_gpio_chip *nmk_chip = gpiochip_get_data(gc);
> +       unsigned long flags;
> +
> +       clk_enable(nmk_chip->clk);
> +       spin_lock_irqsave(&nmk_gpio_slpm_lock, flags);
> +       spin_lock(&nmk_chip->lock);
> +
> +       if (irqd_irq_disabled(d))
> +               __nmk_gpio_set_wake(nmk_chip, d->hwirq, on);
> +
> +       if (on)
> +               nmk_chip->real_wake |= BIT(d->hwirq);
> +       else
> +               nmk_chip->real_wake &= ~BIT(d->hwirq);
> +
> +       spin_unlock(&nmk_chip->lock);
> +       spin_unlock_irqrestore(&nmk_gpio_slpm_lock, flags);
> +       clk_disable(nmk_chip->clk);
> +
> +       return 0;
> +}
> +
> +static int nmk_gpio_irq_set_type(struct irq_data *d, unsigned int type)
> +{
> +       struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
> +       struct nmk_gpio_chip *nmk_chip = gpiochip_get_data(gc);
> +       bool enabled = !irqd_irq_disabled(d);
> +       bool wake = irqd_is_wakeup_set(d);
> +       unsigned long flags;
> +
> +       if (type & IRQ_TYPE_LEVEL_HIGH)
> +               return -EINVAL;
> +       if (type & IRQ_TYPE_LEVEL_LOW)
> +               return -EINVAL;
> +
> +       clk_enable(nmk_chip->clk);
> +       spin_lock_irqsave(&nmk_chip->lock, flags);
> +
> +       if (enabled)
> +               __nmk_gpio_irq_modify(nmk_chip, d->hwirq, NORMAL, false);
> +
> +       if (enabled || wake)
> +               __nmk_gpio_irq_modify(nmk_chip, d->hwirq, WAKE, false);
> +
> +       nmk_chip->edge_rising &= ~BIT(d->hwirq);
> +       if (type & IRQ_TYPE_EDGE_RISING)
> +               nmk_chip->edge_rising |= BIT(d->hwirq);
> +
> +       nmk_chip->edge_falling &= ~BIT(d->hwirq);
> +       if (type & IRQ_TYPE_EDGE_FALLING)
> +               nmk_chip->edge_falling |= BIT(d->hwirq);
> +
> +       if (enabled)
> +               __nmk_gpio_irq_modify(nmk_chip, d->hwirq, NORMAL, true);
> +
> +       if (enabled || wake)
> +               __nmk_gpio_irq_modify(nmk_chip, d->hwirq, WAKE, true);
> +
> +       spin_unlock_irqrestore(&nmk_chip->lock, flags);
> +       clk_disable(nmk_chip->clk);
> +
> +       return 0;
> +}
> +
> +static unsigned int nmk_gpio_irq_startup(struct irq_data *d)
> +{
> +       struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
> +       struct nmk_gpio_chip *nmk_chip = gpiochip_get_data(gc);
> +
> +       clk_enable(nmk_chip->clk);
> +       nmk_gpio_irq_unmask(d);
> +       return 0;
> +}
> +
> +static void nmk_gpio_irq_shutdown(struct irq_data *d)
> +{
> +       struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
> +       struct nmk_gpio_chip *nmk_chip = gpiochip_get_data(gc);
> +
> +       nmk_gpio_irq_mask(d);
> +       clk_disable(nmk_chip->clk);
> +}
> +
> +static void nmk_gpio_irq_handler(struct irq_desc *desc)
> +{
> +       struct irq_chip *host_chip = irq_desc_get_chip(desc);
> +       struct gpio_chip *chip = irq_desc_get_handler_data(desc);
> +       struct nmk_gpio_chip *nmk_chip = gpiochip_get_data(chip);
> +       u32 status;
> +
> +       chained_irq_enter(host_chip, desc);
> +
> +       clk_enable(nmk_chip->clk);
> +       status = readl(nmk_chip->addr + NMK_GPIO_IS);
> +       clk_disable(nmk_chip->clk);
> +
> +       while (status) {
> +               int bit = __ffs(status);
> +
> +               generic_handle_domain_irq(chip->irq.domain, bit);
> +               status &= ~BIT(bit);
> +       }
> +
> +       chained_irq_exit(host_chip, desc);
> +}
> +
> +/* I/O Functions */
> +
> +static int nmk_gpio_get_dir(struct gpio_chip *chip, unsigned int offset)
> +{
> +       struct nmk_gpio_chip *nmk_chip = gpiochip_get_data(chip);
> +       int dir;
> +
> +       clk_enable(nmk_chip->clk);
> +
> +       dir = readl(nmk_chip->addr + NMK_GPIO_DIR) & BIT(offset);
> +
> +       clk_disable(nmk_chip->clk);
> +
> +       if (dir)
> +               return GPIO_LINE_DIRECTION_OUT;
> +
> +       return GPIO_LINE_DIRECTION_IN;
> +}
> +
> +static int nmk_gpio_make_input(struct gpio_chip *chip, unsigned int offset)
> +{
> +       struct nmk_gpio_chip *nmk_chip = gpiochip_get_data(chip);
> +
> +       clk_enable(nmk_chip->clk);
> +
> +       writel(BIT(offset), nmk_chip->addr + NMK_GPIO_DIRC);
> +
> +       clk_disable(nmk_chip->clk);
> +
> +       return 0;
> +}
> +
> +static int nmk_gpio_get_input(struct gpio_chip *chip, unsigned int offset)
> +{
> +       struct nmk_gpio_chip *nmk_chip = gpiochip_get_data(chip);
> +       int value;
> +
> +       clk_enable(nmk_chip->clk);
> +
> +       value = !!(readl(nmk_chip->addr + NMK_GPIO_DAT) & BIT(offset));
> +
> +       clk_disable(nmk_chip->clk);
> +
> +       return value;
> +}
> +
> +static void nmk_gpio_set_output(struct gpio_chip *chip, unsigned int offset,
> +                               int val)
> +{
> +       struct nmk_gpio_chip *nmk_chip = gpiochip_get_data(chip);
> +
> +       clk_enable(nmk_chip->clk);
> +
> +       __nmk_gpio_set_output(nmk_chip, offset, val);
> +
> +       clk_disable(nmk_chip->clk);
> +}
> +
> +static int nmk_gpio_make_output(struct gpio_chip *chip, unsigned int offset,
> +                               int val)
> +{
> +       struct nmk_gpio_chip *nmk_chip = gpiochip_get_data(chip);
> +
> +       clk_enable(nmk_chip->clk);
> +
> +       __nmk_gpio_make_output(nmk_chip, offset, val);
> +
> +       clk_disable(nmk_chip->clk);
> +
> +       return 0;
> +}
> +
> +#ifdef CONFIG_DEBUG_FS
> +
> +static int nmk_gpio_get_mode(struct nmk_gpio_chip *nmk_chip, int offset)
> +{
> +       u32 afunc, bfunc;
> +
> +       clk_enable(nmk_chip->clk);
> +
> +       afunc = readl(nmk_chip->addr + NMK_GPIO_AFSLA) & BIT(offset);
> +       bfunc = readl(nmk_chip->addr + NMK_GPIO_AFSLB) & BIT(offset);
> +
> +       clk_disable(nmk_chip->clk);
> +
> +       return (afunc ? NMK_GPIO_ALT_A : 0) | (bfunc ? NMK_GPIO_ALT_B : 0);
> +}
> +
> +void nmk_gpio_dbg_show_one(struct seq_file *s, struct pinctrl_dev *pctldev,
> +                          struct gpio_chip *chip, unsigned int offset,
> +                          unsigned int gpio)
> +{
> +       struct nmk_gpio_chip *nmk_chip = gpiochip_get_data(chip);
> +       int mode;
> +       bool is_out;
> +       bool data_out;
> +       bool pull;
> +       static const char * const modes[] = {
> +               [NMK_GPIO_ALT_GPIO]     = "gpio",
> +               [NMK_GPIO_ALT_A]        = "altA",
> +               [NMK_GPIO_ALT_B]        = "altB",
> +               [NMK_GPIO_ALT_C]        = "altC",
> +               [NMK_GPIO_ALT_C + 1]    = "altC1",
> +               [NMK_GPIO_ALT_C + 2]    = "altC2",
> +               [NMK_GPIO_ALT_C + 3]    = "altC3",
> +               [NMK_GPIO_ALT_C + 4]    = "altC4",
> +       };
> +
> +       char *label = gpiochip_dup_line_label(chip, offset);
> +       if (IS_ERR(label))
> +               return;
> +
> +       clk_enable(nmk_chip->clk);
> +       is_out = !!(readl(nmk_chip->addr + NMK_GPIO_DIR) & BIT(offset));
> +       pull = !(readl(nmk_chip->addr + NMK_GPIO_PDIS) & BIT(offset));
> +       data_out = !!(readl(nmk_chip->addr + NMK_GPIO_DAT) & BIT(offset));
> +       mode = nmk_gpio_get_mode(nmk_chip, offset);
> +#ifdef CONFIG_PINCTRL_NOMADIK
> +       if (mode == NMK_GPIO_ALT_C && pctldev)
> +               mode = nmk_prcm_gpiocr_get_mode(pctldev, gpio);
> +#endif
> +
> +       if (is_out) {
> +               seq_printf(s, " gpio-%-3d (%-20.20s) out %s           %s",
> +                          gpio,
> +                          label ?: "(none)",
> +                          data_out ? "hi" : "lo",
> +                          (mode < 0) ? "unknown" : modes[mode]);
> +       } else {
> +               int irq = chip->to_irq(chip, offset);
> +               const int pullidx = pull ? 1 : 0;
> +               int val;
> +               static const char * const pulls[] = {
> +                       "none        ",
> +                       "pull enabled",
> +               };
> +
> +               seq_printf(s, " gpio-%-3d (%-20.20s) in  %s %s",
> +                          gpio,
> +                          label ?: "(none)",
> +                          pulls[pullidx],
> +                          (mode < 0) ? "unknown" : modes[mode]);
> +
> +               val = nmk_gpio_get_input(chip, offset);
> +               seq_printf(s, " VAL %d", val);
> +
> +               /*
> +                * This races with request_irq(), set_irq_type(),
> +                * and set_irq_wake() ... but those are "rare".
> +                */
> +               if (irq > 0 && irq_has_action(irq)) {
> +                       char *trigger;
> +                       bool wake;
> +
> +                       if (nmk_chip->edge_rising & BIT(offset))
> +                               trigger = "edge-rising";
> +                       else if (nmk_chip->edge_falling & BIT(offset))
> +                               trigger = "edge-falling";
> +                       else
> +                               trigger = "edge-undefined";
> +
> +                       wake = !!(nmk_chip->real_wake & BIT(offset));
> +
> +                       seq_printf(s, " irq-%d %s%s",
> +                                  irq, trigger, wake ? " wakeup" : "");
> +               }
> +       }
> +       clk_disable(nmk_chip->clk);
> +}
> +
> +static void nmk_gpio_dbg_show(struct seq_file *s, struct gpio_chip *chip)
> +{
> +       unsigned int i, gpio = chip->base;
> +
> +       for (i = 0; i < chip->ngpio; i++, gpio++) {
> +               nmk_gpio_dbg_show_one(s, NULL, chip, i, gpio);
> +               seq_puts(s, "\n");
> +       }
> +}
> +
> +#else
> +
> +static inline void nmk_gpio_dbg_show_one(struct seq_file *s,
> +                                        struct pinctrl_dev *pctldev,
> +                                        struct gpio_chip *chip,
> +                                        unsigned int offset,
> +                                        unsigned int gpio)
> +{
> +}
> +
> +#define nmk_gpio_dbg_show      NULL
> +
> +#endif
> +
> +/*
> + * We will allocate memory for the state container using devm* allocators
> + * binding to the first device reaching this point, it doesn't matter if
> + * it is the pin controller or GPIO driver. However we need to use the right
> + * platform device when looking up resources so pay attention to pdev.
> + */
> +struct nmk_gpio_chip *nmk_gpio_populate_chip(struct device_node *np,
> +                                            struct platform_device *pdev)
> +{
> +       struct nmk_gpio_chip *nmk_chip;
> +       struct platform_device *gpio_pdev;
> +       struct gpio_chip *chip;
> +       struct resource *res;
> +       struct clk *clk;
> +       void __iomem *base;
> +       u32 id;
> +
> +       gpio_pdev = of_find_device_by_node(np);

Any way we can avoid using OF APIs here? How about bus_find_device_by_fwnode()?

In general I'd like to avoid adding new calls to OF functions and use
generic device properties everywhere, so all of_property_* in the
series would ideally be replaced with device_property_* alternatives.

> +       if (!gpio_pdev) {
> +               pr_err("populate \"%pOFn\": device not found\n", np);
> +               return ERR_PTR(-ENODEV);
> +       }
> +       if (of_property_read_u32(np, "gpio-bank", &id)) {
> +               dev_err(&pdev->dev, "populate: gpio-bank property not found\n");
> +               platform_device_put(gpio_pdev);
> +               return ERR_PTR(-EINVAL);
> +       }
> +
> +#ifdef CONFIG_PINCTRL_NOMADIK
> +       /* Already populated? */
> +       nmk_chip = nmk_gpio_chips[id];
> +       if (nmk_chip) {
> +               platform_device_put(gpio_pdev);
> +               return nmk_chip;
> +       }
> +#endif
> +
> +       nmk_chip = devm_kzalloc(&pdev->dev, sizeof(*nmk_chip), GFP_KERNEL);
> +       if (!nmk_chip) {
> +               platform_device_put(gpio_pdev);
> +               return ERR_PTR(-ENOMEM);
> +       }
> +
> +       nmk_chip->bank = id;
> +       chip = &nmk_chip->chip;
> +       chip->base = id * NMK_GPIO_PER_CHIP;
> +       chip->ngpio = NMK_GPIO_PER_CHIP;
> +       chip->label = dev_name(&gpio_pdev->dev);
> +       chip->parent = &gpio_pdev->dev;
> +
> +       res = platform_get_resource(gpio_pdev, IORESOURCE_MEM, 0);
> +       base = devm_ioremap_resource(&pdev->dev, res);

devm_platform_ioremap_resource()?

> +       if (IS_ERR(base)) {
> +               platform_device_put(gpio_pdev);
> +               return ERR_CAST(base);
> +       }
> +       nmk_chip->addr = base;
> +
> +       clk = clk_get(&gpio_pdev->dev, NULL);

devm_clk_get()?

> +       if (IS_ERR(clk)) {
> +               platform_device_put(gpio_pdev);
> +               return (void *)clk;
> +       }
> +       clk_prepare(clk);
> +       nmk_chip->clk = clk;
> +
> +#ifdef CONFIG_PINCTRL_NOMADIK
> +       BUG_ON(nmk_chip->bank >= ARRAY_SIZE(nmk_gpio_chips));
> +       nmk_gpio_chips[id] = nmk_chip;
> +#endif
> +       return nmk_chip;
> +}
> +
> +static void nmk_gpio_irq_print_chip(struct irq_data *d, struct seq_file *p)
> +{
> +       struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
> +       struct nmk_gpio_chip *nmk_chip = gpiochip_get_data(gc);
> +
> +       seq_printf(p, "nmk%u-%u-%u", nmk_chip->bank,
> +                  gc->base, gc->base + gc->ngpio - 1);
> +}
> +
> +static const struct irq_chip nmk_irq_chip = {
> +       .irq_ack = nmk_gpio_irq_ack,
> +       .irq_mask = nmk_gpio_irq_mask,
> +       .irq_unmask = nmk_gpio_irq_unmask,
> +       .irq_set_type = nmk_gpio_irq_set_type,
> +       .irq_set_wake = nmk_gpio_irq_set_wake,
> +       .irq_startup = nmk_gpio_irq_startup,
> +       .irq_shutdown = nmk_gpio_irq_shutdown,
> +       .irq_print_chip = nmk_gpio_irq_print_chip,
> +       .flags = IRQCHIP_MASK_ON_SUSPEND | IRQCHIP_IMMUTABLE,
> +       GPIOCHIP_IRQ_RESOURCE_HELPERS,
> +};
> +
> +static int nmk_gpio_probe(struct platform_device *dev)
> +{
> +       struct device_node *np = dev->dev.of_node;
> +       struct nmk_gpio_chip *nmk_chip;
> +       struct gpio_chip *chip;
> +       struct gpio_irq_chip *girq;
> +       bool supports_sleepmode;
> +       int irq;
> +       int ret;
> +
> +       nmk_chip = nmk_gpio_populate_chip(np, dev);
> +       if (IS_ERR(nmk_chip)) {
> +               dev_err(&dev->dev, "could not populate nmk chip struct\n");
> +               return PTR_ERR(nmk_chip);
> +       }
> +
> +       supports_sleepmode =
> +               of_property_read_bool(np, "st,supports-sleepmode");
> +
> +       /* Correct platform device ID */
> +       dev->id = nmk_chip->bank;
> +
> +       irq = platform_get_irq(dev, 0);
> +       if (irq < 0)
> +               return irq;
> +
> +       /*
> +        * The virt address in nmk_chip->addr is in the nomadik register space,
> +        * so we can simply convert the resource address, without remapping
> +        */
> +       nmk_chip->sleepmode = supports_sleepmode;
> +       spin_lock_init(&nmk_chip->lock);
> +
> +       chip = &nmk_chip->chip;
> +       chip->parent = &dev->dev;
> +       chip->request = gpiochip_generic_request;
> +       chip->free = gpiochip_generic_free;
> +       chip->get_direction = nmk_gpio_get_dir;
> +       chip->direction_input = nmk_gpio_make_input;
> +       chip->get = nmk_gpio_get_input;
> +       chip->direction_output = nmk_gpio_make_output;
> +       chip->set = nmk_gpio_set_output;
> +       chip->dbg_show = nmk_gpio_dbg_show;
> +       chip->can_sleep = false;
> +       chip->owner = THIS_MODULE;
> +
> +       girq = &chip->irq;
> +       gpio_irq_chip_set_chip(girq, &nmk_irq_chip);
> +       girq->parent_handler = nmk_gpio_irq_handler;
> +       girq->num_parents = 1;
> +       girq->parents = devm_kcalloc(&dev->dev, 1,
> +                                    sizeof(*girq->parents),
> +                                    GFP_KERNEL);
> +       if (!girq->parents)
> +               return -ENOMEM;
> +       girq->parents[0] = irq;
> +       girq->default_type = IRQ_TYPE_NONE;
> +       girq->handler = handle_edge_irq;
> +
> +       clk_enable(nmk_chip->clk);
> +       nmk_chip->lowemi = readl_relaxed(nmk_chip->addr + NMK_GPIO_LOWEMI);
> +       clk_disable(nmk_chip->clk);
> +
> +       ret = gpiochip_add_data(chip, nmk_chip);
> +       if (ret)
> +               return ret;
> +
> +       platform_set_drvdata(dev, nmk_chip);
> +
> +       dev_info(&dev->dev, "chip registered\n");
> +
> +       return 0;
> +}
> +
> +static const struct of_device_id nmk_gpio_match[] = {
> +       { .compatible = "st,nomadik-gpio", },
> +       {}
> +};
> +
> +static struct platform_driver nmk_gpio_driver = {
> +       .driver = {
> +               .name = "gpio",
> +               .of_match_table = nmk_gpio_match,
> +       },
> +       .probe = nmk_gpio_probe,
> +};
> +
> +static int __init nmk_gpio_init(void)
> +{
> +       return platform_driver_register(&nmk_gpio_driver);
> +}
> +subsys_initcall(nmk_gpio_init);
> diff --git a/drivers/pinctrl/nomadik/Kconfig b/drivers/pinctrl/nomadik/Kconfig
> index 0fea167c283f..f47f0755a835 100644
> --- a/drivers/pinctrl/nomadik/Kconfig
> +++ b/drivers/pinctrl/nomadik/Kconfig
> @@ -22,11 +22,10 @@ if (ARCH_U8500 || ARCH_NOMADIK)
>
>  config PINCTRL_NOMADIK
>         bool "Nomadik pin controller driver"
> -       depends on OF && GPIOLIB
> +       depends on OF
>         select PINMUX
>         select PINCONF
> -       select OF_GPIO
> -       select GPIOLIB_IRQCHIP
> +       select GPIO_NOMADIK
>
>  config PINCTRL_STN8815
>         bool "STN8815 pin controller driver"
> diff --git a/drivers/pinctrl/nomadik/pinctrl-nomadik-db8500.c b/drivers/pinctrl/nomadik/pinctrl-nomadik-db8500.c
> index 490e0959e8be..0b4a3dd9d8c7 100644
> --- a/drivers/pinctrl/nomadik/pinctrl-nomadik-db8500.c
> +++ b/drivers/pinctrl/nomadik/pinctrl-nomadik-db8500.c
> @@ -3,8 +3,9 @@
>  #include <linux/types.h>
>
>  #include <linux/pinctrl/pinctrl.h>
> +#include <linux/gpio/driver.h>
>
> -#include "pinctrl-nomadik.h"
> +#include <linux/gpio/gpio-nomadik.h>
>
>  /* All the pins that can be used for GPIO and some other functions */
>  #define _GPIO(offset)          (offset)
> diff --git a/drivers/pinctrl/nomadik/pinctrl-nomadik-stn8815.c b/drivers/pinctrl/nomadik/pinctrl-nomadik-stn8815.c
> index 1552222ac68e..c5a52fcaba30 100644
> --- a/drivers/pinctrl/nomadik/pinctrl-nomadik-stn8815.c
> +++ b/drivers/pinctrl/nomadik/pinctrl-nomadik-stn8815.c
> @@ -3,8 +3,9 @@
>  #include <linux/types.h>
>
>  #include <linux/pinctrl/pinctrl.h>
> +#include <linux/gpio/driver.h>
>
> -#include "pinctrl-nomadik.h"
> +#include <linux/gpio/gpio-nomadik.h>
>
>  /* All the pins that can be used for GPIO and some other functions */
>  #define _GPIO(offset)          (offset)
> diff --git a/drivers/pinctrl/nomadik/pinctrl-nomadik.c b/drivers/pinctrl/nomadik/pinctrl-nomadik.c
> index 7911353ac97d..f3897dbfa2c3 100644
> --- a/drivers/pinctrl/nomadik/pinctrl-nomadik.c
> +++ b/drivers/pinctrl/nomadik/pinctrl-nomadik.c
> @@ -1,6 +1,8 @@
>  // SPDX-License-Identifier: GPL-2.0-only
>  /*
> - * Generic GPIO driver for logic cells found in the Nomadik SoC
> + * Pinmux & pinconf driver for the IP block found in the Nomadik SoC. This
> + * depends on gpio-nomadik and some handling is intertwined; see nmk_gpio_chips
> + * which is used by this driver to access the GPIO banks array.
>   *
>   * Copyright (C) 2008,2009 STMicroelectronics
>   * Copyright (C) 2009 Alessandro Rubini <rubini@unipv.it>
> @@ -25,6 +27,7 @@
>  #include <linux/seq_file.h>
>  #include <linux/slab.h>
>  #include <linux/spinlock.h>
> +#include <linux/types.h>
>
>  /* Since we request GPIOs from ourself */
>  #include <linux/pinctrl/consumer.h>
> @@ -36,15 +39,7 @@
>  #include "../core.h"
>  #include "../pinctrl-utils.h"
>
> -#include "pinctrl-nomadik.h"
> -
> -/*
> - * The GPIO module in the Nomadik family of Systems-on-Chip is an
> - * AMBA device, managing 32 pins and alternate functions.  The logic block
> - * is currently used in the Nomadik and ux500.
> - *
> - * Symbols in this file are called "nmk_gpio" for "nomadik gpio"
> - */
> +#include <linux/gpio/gpio-nomadik.h>
>
>  /*
>   * pin configurations are represented by 32-bit integers:
> @@ -200,75 +195,6 @@ typedef unsigned long pin_cfg_t;
>         (PIN_CFG_DEFAULT |\
>          (PIN_NUM(num) | PIN_##alt | PIN_OUTPUT_##val))
>
> -/*
> - * "nmk_gpio" and "NMK_GPIO" stand for "Nomadik GPIO", leaving
> - * the "gpio" namespace for generic and cross-machine functions
> - */
> -
> -#define GPIO_BLOCK_SHIFT 5
> -#define NMK_GPIO_PER_CHIP (1 << GPIO_BLOCK_SHIFT)
> -#define NMK_MAX_BANKS DIV_ROUND_UP(512, NMK_GPIO_PER_CHIP)
> -
> -/* Register in the logic block */
> -#define NMK_GPIO_DAT   0x00
> -#define NMK_GPIO_DATS  0x04
> -#define NMK_GPIO_DATC  0x08
> -#define NMK_GPIO_PDIS  0x0c
> -#define NMK_GPIO_DIR   0x10
> -#define NMK_GPIO_DIRS  0x14
> -#define NMK_GPIO_DIRC  0x18
> -#define NMK_GPIO_SLPC  0x1c
> -#define NMK_GPIO_AFSLA 0x20
> -#define NMK_GPIO_AFSLB 0x24
> -#define NMK_GPIO_LOWEMI        0x28
> -
> -#define NMK_GPIO_RIMSC 0x40
> -#define NMK_GPIO_FIMSC 0x44
> -#define NMK_GPIO_IS    0x48
> -#define NMK_GPIO_IC    0x4c
> -#define NMK_GPIO_RWIMSC        0x50
> -#define NMK_GPIO_FWIMSC        0x54
> -#define NMK_GPIO_WKS   0x58
> -/* These appear in DB8540 and later ASICs */
> -#define NMK_GPIO_EDGELEVEL 0x5C
> -#define NMK_GPIO_LEVEL 0x60
> -
> -
> -/* Pull up/down values */
> -enum nmk_gpio_pull {
> -       NMK_GPIO_PULL_NONE,
> -       NMK_GPIO_PULL_UP,
> -       NMK_GPIO_PULL_DOWN,
> -};
> -
> -/* Sleep mode */
> -enum nmk_gpio_slpm {
> -       NMK_GPIO_SLPM_INPUT,
> -       NMK_GPIO_SLPM_WAKEUP_ENABLE = NMK_GPIO_SLPM_INPUT,
> -       NMK_GPIO_SLPM_NOCHANGE,
> -       NMK_GPIO_SLPM_WAKEUP_DISABLE = NMK_GPIO_SLPM_NOCHANGE,
> -};
> -
> -struct nmk_gpio_chip {
> -       struct gpio_chip chip;
> -       void __iomem *addr;
> -       struct clk *clk;
> -       unsigned int bank;
> -       void (*set_ioforce)(bool enable);
> -       spinlock_t lock;
> -       bool sleepmode;
> -       /* Keep track of configured edges */
> -       u32 edge_rising;
> -       u32 edge_falling;
> -       u32 real_wake;
> -       u32 rwimsc;
> -       u32 fwimsc;
> -       u32 rimsc;
> -       u32 fimsc;
> -       u32 pull_up;
> -       u32 lowemi;
> -};
> -
>  /**
>   * struct nmk_pinctrl - state container for the Nomadik pin controller
>   * @dev: containing device pointer
> @@ -283,11 +209,10 @@ struct nmk_pinctrl {
>         void __iomem *prcm_base;
>  };
>
> -static struct nmk_gpio_chip *nmk_gpio_chips[NMK_MAX_BANKS];
> +/* See nmk_gpio_populate_chip() that fills this array. */
> +struct nmk_gpio_chip *nmk_gpio_chips[NMK_MAX_BANKS];
>
> -static DEFINE_SPINLOCK(nmk_gpio_slpm_lock);
> -
> -#define NUM_BANKS ARRAY_SIZE(nmk_gpio_chips)
> +DEFINE_SPINLOCK(nmk_gpio_slpm_lock);
>
>  static void __nmk_gpio_set_mode(struct nmk_gpio_chip *nmk_chip,
>                                 unsigned offset, int gpio_mode)
> @@ -304,19 +229,6 @@ static void __nmk_gpio_set_mode(struct nmk_gpio_chip *nmk_chip,
>         writel(bfunc, nmk_chip->addr + NMK_GPIO_AFSLB);
>  }
>
> -static void __nmk_gpio_set_slpm(struct nmk_gpio_chip *nmk_chip,
> -                               unsigned offset, enum nmk_gpio_slpm mode)
> -{
> -       u32 slpm;
> -
> -       slpm = readl(nmk_chip->addr + NMK_GPIO_SLPC);
> -       if (mode == NMK_GPIO_SLPM_NOCHANGE)
> -               slpm |= BIT(offset);
> -       else
> -               slpm &= ~BIT(offset);
> -       writel(slpm, nmk_chip->addr + NMK_GPIO_SLPC);
> -}
> -
>  static void __nmk_gpio_set_pull(struct nmk_gpio_chip *nmk_chip,
>                                 unsigned offset, enum nmk_gpio_pull pull)
>  {
> @@ -364,22 +276,6 @@ static void __nmk_gpio_make_input(struct nmk_gpio_chip *nmk_chip,
>         writel(BIT(offset), nmk_chip->addr + NMK_GPIO_DIRC);
>  }
>
> -static void __nmk_gpio_set_output(struct nmk_gpio_chip *nmk_chip,
> -                                 unsigned offset, int val)
> -{
> -       if (val)
> -               writel(BIT(offset), nmk_chip->addr + NMK_GPIO_DATS);
> -       else
> -               writel(BIT(offset), nmk_chip->addr + NMK_GPIO_DATC);
> -}
> -
> -static void __nmk_gpio_make_output(struct nmk_gpio_chip *nmk_chip,
> -                                 unsigned offset, int val)
> -{
> -       writel(BIT(offset), nmk_chip->addr + NMK_GPIO_DIRS);
> -       __nmk_gpio_set_output(nmk_chip, offset, val);
> -}
> -
>  static void __nmk_gpio_set_mode_safe(struct nmk_gpio_chip *nmk_chip,
>                                      unsigned offset, int gpio_mode,
>                                      bool glitch)
> @@ -548,7 +444,7 @@ static void nmk_gpio_glitch_slpm_init(unsigned int *slpm)
>  {
>         int i;
>
> -       for (i = 0; i < NUM_BANKS; i++) {
> +       for (i = 0; i < NMK_MAX_BANKS; i++) {
>                 struct nmk_gpio_chip *chip = nmk_gpio_chips[i];
>                 unsigned int temp = slpm[i];
>
> @@ -566,7 +462,7 @@ static void nmk_gpio_glitch_slpm_restore(unsigned int *slpm)
>  {
>         int i;
>
> -       for (i = 0; i < NUM_BANKS; i++) {
> +       for (i = 0; i < NMK_MAX_BANKS; i++) {
>                 struct nmk_gpio_chip *chip = nmk_gpio_chips[i];
>
>                 if (!chip)
> @@ -578,7 +474,8 @@ static void nmk_gpio_glitch_slpm_restore(unsigned int *slpm)
>         }
>  }
>
> -static int __maybe_unused nmk_prcm_gpiocr_get_mode(struct pinctrl_dev *pctldev, int gpio)
> +/* Only called by gpio-nomadik but requires knowledge of struct nmk_pinctrl. */
> +int __maybe_unused nmk_prcm_gpiocr_get_mode(struct pinctrl_dev *pctldev, int gpio)
>  {
>         int i;
>         u16 reg;
> @@ -610,576 +507,6 @@ static int __maybe_unused nmk_prcm_gpiocr_get_mode(struct pinctrl_dev *pctldev,
>         return NMK_GPIO_ALT_C;
>  }
>
> -/* IRQ functions */
> -
> -static void nmk_gpio_irq_ack(struct irq_data *d)
> -{
> -       struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
> -       struct nmk_gpio_chip *nmk_chip = gpiochip_get_data(gc);
> -
> -       clk_enable(nmk_chip->clk);
> -       writel(BIT(d->hwirq), nmk_chip->addr + NMK_GPIO_IC);
> -       clk_disable(nmk_chip->clk);
> -}
> -
> -enum nmk_gpio_irq_type {
> -       NORMAL,
> -       WAKE,
> -};
> -
> -static void __nmk_gpio_irq_modify(struct nmk_gpio_chip *nmk_chip,
> -                                 int offset, enum nmk_gpio_irq_type which,
> -                                 bool enable)
> -{
> -       u32 *rimscval;
> -       u32 *fimscval;
> -       u32 rimscreg;
> -       u32 fimscreg;
> -
> -       if (which == NORMAL) {
> -               rimscreg = NMK_GPIO_RIMSC;
> -               fimscreg = NMK_GPIO_FIMSC;
> -               rimscval = &nmk_chip->rimsc;
> -               fimscval = &nmk_chip->fimsc;
> -       } else  {
> -               rimscreg = NMK_GPIO_RWIMSC;
> -               fimscreg = NMK_GPIO_FWIMSC;
> -               rimscval = &nmk_chip->rwimsc;
> -               fimscval = &nmk_chip->fwimsc;
> -       }
> -
> -       /* we must individually set/clear the two edges */
> -       if (nmk_chip->edge_rising & BIT(offset)) {
> -               if (enable)
> -                       *rimscval |= BIT(offset);
> -               else
> -                       *rimscval &= ~BIT(offset);
> -               writel(*rimscval, nmk_chip->addr + rimscreg);
> -       }
> -       if (nmk_chip->edge_falling & BIT(offset)) {
> -               if (enable)
> -                       *fimscval |= BIT(offset);
> -               else
> -                       *fimscval &= ~BIT(offset);
> -               writel(*fimscval, nmk_chip->addr + fimscreg);
> -       }
> -}
> -
> -static void __nmk_gpio_set_wake(struct nmk_gpio_chip *nmk_chip,
> -                               int offset, bool on)
> -{
> -       /*
> -        * Ensure WAKEUP_ENABLE is on.  No need to disable it if wakeup is
> -        * disabled, since setting SLPM to 1 increases power consumption, and
> -        * wakeup is anyhow controlled by the RIMSC and FIMSC registers.
> -        */
> -       if (nmk_chip->sleepmode && on) {
> -               __nmk_gpio_set_slpm(nmk_chip, offset,
> -                                   NMK_GPIO_SLPM_WAKEUP_ENABLE);
> -       }
> -
> -       __nmk_gpio_irq_modify(nmk_chip, offset, WAKE, on);
> -}
> -
> -static void nmk_gpio_irq_maskunmask(struct nmk_gpio_chip *nmk_chip,
> -                                   struct irq_data *d, bool enable)
> -{
> -       unsigned long flags;
> -
> -       clk_enable(nmk_chip->clk);
> -       spin_lock_irqsave(&nmk_gpio_slpm_lock, flags);
> -       spin_lock(&nmk_chip->lock);
> -
> -       __nmk_gpio_irq_modify(nmk_chip, d->hwirq, NORMAL, enable);
> -
> -       if (!(nmk_chip->real_wake & BIT(d->hwirq)))
> -               __nmk_gpio_set_wake(nmk_chip, d->hwirq, enable);
> -
> -       spin_unlock(&nmk_chip->lock);
> -       spin_unlock_irqrestore(&nmk_gpio_slpm_lock, flags);
> -       clk_disable(nmk_chip->clk);
> -}
> -
> -static void nmk_gpio_irq_mask(struct irq_data *d)
> -{
> -       struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
> -       struct nmk_gpio_chip *nmk_chip = gpiochip_get_data(gc);
> -
> -       nmk_gpio_irq_maskunmask(nmk_chip, d, false);
> -       gpiochip_disable_irq(gc, irqd_to_hwirq(d));
> -}
> -
> -static void nmk_gpio_irq_unmask(struct irq_data *d)
> -{
> -       struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
> -       struct nmk_gpio_chip *nmk_chip = gpiochip_get_data(gc);
> -
> -       gpiochip_enable_irq(gc, irqd_to_hwirq(d));
> -       nmk_gpio_irq_maskunmask(nmk_chip, d, true);
> -}
> -
> -static int nmk_gpio_irq_set_wake(struct irq_data *d, unsigned int on)
> -{
> -       struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
> -       struct nmk_gpio_chip *nmk_chip = gpiochip_get_data(gc);
> -       unsigned long flags;
> -
> -       clk_enable(nmk_chip->clk);
> -       spin_lock_irqsave(&nmk_gpio_slpm_lock, flags);
> -       spin_lock(&nmk_chip->lock);
> -
> -       if (irqd_irq_disabled(d))
> -               __nmk_gpio_set_wake(nmk_chip, d->hwirq, on);
> -
> -       if (on)
> -               nmk_chip->real_wake |= BIT(d->hwirq);
> -       else
> -               nmk_chip->real_wake &= ~BIT(d->hwirq);
> -
> -       spin_unlock(&nmk_chip->lock);
> -       spin_unlock_irqrestore(&nmk_gpio_slpm_lock, flags);
> -       clk_disable(nmk_chip->clk);
> -
> -       return 0;
> -}
> -
> -static int nmk_gpio_irq_set_type(struct irq_data *d, unsigned int type)
> -{
> -       struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
> -       struct nmk_gpio_chip *nmk_chip = gpiochip_get_data(gc);
> -       bool enabled = !irqd_irq_disabled(d);
> -       bool wake = irqd_is_wakeup_set(d);
> -       unsigned long flags;
> -
> -       if (type & IRQ_TYPE_LEVEL_HIGH)
> -               return -EINVAL;
> -       if (type & IRQ_TYPE_LEVEL_LOW)
> -               return -EINVAL;
> -
> -       clk_enable(nmk_chip->clk);
> -       spin_lock_irqsave(&nmk_chip->lock, flags);
> -
> -       if (enabled)
> -               __nmk_gpio_irq_modify(nmk_chip, d->hwirq, NORMAL, false);
> -
> -       if (enabled || wake)
> -               __nmk_gpio_irq_modify(nmk_chip, d->hwirq, WAKE, false);
> -
> -       nmk_chip->edge_rising &= ~BIT(d->hwirq);
> -       if (type & IRQ_TYPE_EDGE_RISING)
> -               nmk_chip->edge_rising |= BIT(d->hwirq);
> -
> -       nmk_chip->edge_falling &= ~BIT(d->hwirq);
> -       if (type & IRQ_TYPE_EDGE_FALLING)
> -               nmk_chip->edge_falling |= BIT(d->hwirq);
> -
> -       if (enabled)
> -               __nmk_gpio_irq_modify(nmk_chip, d->hwirq, NORMAL, true);
> -
> -       if (enabled || wake)
> -               __nmk_gpio_irq_modify(nmk_chip, d->hwirq, WAKE, true);
> -
> -       spin_unlock_irqrestore(&nmk_chip->lock, flags);
> -       clk_disable(nmk_chip->clk);
> -
> -       return 0;
> -}
> -
> -static unsigned int nmk_gpio_irq_startup(struct irq_data *d)
> -{
> -       struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
> -       struct nmk_gpio_chip *nmk_chip = gpiochip_get_data(gc);
> -
> -       clk_enable(nmk_chip->clk);
> -       nmk_gpio_irq_unmask(d);
> -       return 0;
> -}
> -
> -static void nmk_gpio_irq_shutdown(struct irq_data *d)
> -{
> -       struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
> -       struct nmk_gpio_chip *nmk_chip = gpiochip_get_data(gc);
> -
> -       nmk_gpio_irq_mask(d);
> -       clk_disable(nmk_chip->clk);
> -}
> -
> -static void nmk_gpio_irq_handler(struct irq_desc *desc)
> -{
> -       struct irq_chip *host_chip = irq_desc_get_chip(desc);
> -       struct gpio_chip *chip = irq_desc_get_handler_data(desc);
> -       struct nmk_gpio_chip *nmk_chip = gpiochip_get_data(chip);
> -       u32 status;
> -
> -       chained_irq_enter(host_chip, desc);
> -
> -       clk_enable(nmk_chip->clk);
> -       status = readl(nmk_chip->addr + NMK_GPIO_IS);
> -       clk_disable(nmk_chip->clk);
> -
> -       while (status) {
> -               int bit = __ffs(status);
> -
> -               generic_handle_domain_irq(chip->irq.domain, bit);
> -               status &= ~BIT(bit);
> -       }
> -
> -       chained_irq_exit(host_chip, desc);
> -}
> -
> -/* I/O Functions */
> -
> -static int nmk_gpio_get_dir(struct gpio_chip *chip, unsigned offset)
> -{
> -       struct nmk_gpio_chip *nmk_chip = gpiochip_get_data(chip);
> -       int dir;
> -
> -       clk_enable(nmk_chip->clk);
> -
> -       dir = readl(nmk_chip->addr + NMK_GPIO_DIR) & BIT(offset);
> -
> -       clk_disable(nmk_chip->clk);
> -
> -       if (dir)
> -               return GPIO_LINE_DIRECTION_OUT;
> -
> -       return GPIO_LINE_DIRECTION_IN;
> -}
> -
> -static int nmk_gpio_make_input(struct gpio_chip *chip, unsigned offset)
> -{
> -       struct nmk_gpio_chip *nmk_chip = gpiochip_get_data(chip);
> -
> -       clk_enable(nmk_chip->clk);
> -
> -       writel(BIT(offset), nmk_chip->addr + NMK_GPIO_DIRC);
> -
> -       clk_disable(nmk_chip->clk);
> -
> -       return 0;
> -}
> -
> -static int nmk_gpio_get_input(struct gpio_chip *chip, unsigned offset)
> -{
> -       struct nmk_gpio_chip *nmk_chip = gpiochip_get_data(chip);
> -       int value;
> -
> -       clk_enable(nmk_chip->clk);
> -
> -       value = !!(readl(nmk_chip->addr + NMK_GPIO_DAT) & BIT(offset));
> -
> -       clk_disable(nmk_chip->clk);
> -
> -       return value;
> -}
> -
> -static void nmk_gpio_set_output(struct gpio_chip *chip, unsigned offset,
> -                               int val)
> -{
> -       struct nmk_gpio_chip *nmk_chip = gpiochip_get_data(chip);
> -
> -       clk_enable(nmk_chip->clk);
> -
> -       __nmk_gpio_set_output(nmk_chip, offset, val);
> -
> -       clk_disable(nmk_chip->clk);
> -}
> -
> -static int nmk_gpio_make_output(struct gpio_chip *chip, unsigned offset,
> -                               int val)
> -{
> -       struct nmk_gpio_chip *nmk_chip = gpiochip_get_data(chip);
> -
> -       clk_enable(nmk_chip->clk);
> -
> -       __nmk_gpio_make_output(nmk_chip, offset, val);
> -
> -       clk_disable(nmk_chip->clk);
> -
> -       return 0;
> -}
> -
> -#ifdef CONFIG_DEBUG_FS
> -static int nmk_gpio_get_mode(struct nmk_gpio_chip *nmk_chip, int offset)
> -{
> -       u32 afunc, bfunc;
> -
> -       clk_enable(nmk_chip->clk);
> -
> -       afunc = readl(nmk_chip->addr + NMK_GPIO_AFSLA) & BIT(offset);
> -       bfunc = readl(nmk_chip->addr + NMK_GPIO_AFSLB) & BIT(offset);
> -
> -       clk_disable(nmk_chip->clk);
> -
> -       return (afunc ? NMK_GPIO_ALT_A : 0) | (bfunc ? NMK_GPIO_ALT_B : 0);
> -}
> -
> -static void nmk_gpio_dbg_show_one(struct seq_file *s,
> -       struct pinctrl_dev *pctldev, struct gpio_chip *chip,
> -       unsigned offset, unsigned gpio)
> -{
> -       struct nmk_gpio_chip *nmk_chip = gpiochip_get_data(chip);
> -       int mode;
> -       bool is_out;
> -       bool data_out;
> -       bool pull;
> -       const char *modes[] = {
> -               [NMK_GPIO_ALT_GPIO]     = "gpio",
> -               [NMK_GPIO_ALT_A]        = "altA",
> -               [NMK_GPIO_ALT_B]        = "altB",
> -               [NMK_GPIO_ALT_C]        = "altC",
> -               [NMK_GPIO_ALT_C+1]      = "altC1",
> -               [NMK_GPIO_ALT_C+2]      = "altC2",
> -               [NMK_GPIO_ALT_C+3]      = "altC3",
> -               [NMK_GPIO_ALT_C+4]      = "altC4",
> -       };
> -
> -       char *label = gpiochip_dup_line_label(chip, offset);
> -       if (IS_ERR(label))
> -               return;
> -
> -       clk_enable(nmk_chip->clk);
> -       is_out = !!(readl(nmk_chip->addr + NMK_GPIO_DIR) & BIT(offset));
> -       pull = !(readl(nmk_chip->addr + NMK_GPIO_PDIS) & BIT(offset));
> -       data_out = !!(readl(nmk_chip->addr + NMK_GPIO_DAT) & BIT(offset));
> -       mode = nmk_gpio_get_mode(nmk_chip, offset);
> -       if ((mode == NMK_GPIO_ALT_C) && pctldev)
> -               mode = nmk_prcm_gpiocr_get_mode(pctldev, gpio);
> -
> -       if (is_out) {
> -               seq_printf(s, " gpio-%-3d (%-20.20s) out %s           %s",
> -                          gpio,
> -                          label ?: "(none)",
> -                          data_out ? "hi" : "lo",
> -                          (mode < 0) ? "unknown" : modes[mode]);
> -       } else {
> -               int irq = chip->to_irq(chip, offset);
> -               const int pullidx = pull ? 1 : 0;
> -               int val;
> -               static const char * const pulls[] = {
> -                       "none        ",
> -                       "pull enabled",
> -               };
> -
> -               seq_printf(s, " gpio-%-3d (%-20.20s) in  %s %s",
> -                          gpio,
> -                          label ?: "(none)",
> -                          pulls[pullidx],
> -                          (mode < 0) ? "unknown" : modes[mode]);
> -
> -               val = nmk_gpio_get_input(chip, offset);
> -               seq_printf(s, " VAL %d", val);
> -
> -               /*
> -                * This races with request_irq(), set_irq_type(),
> -                * and set_irq_wake() ... but those are "rare".
> -                */
> -               if (irq > 0 && irq_has_action(irq)) {
> -                       char *trigger;
> -                       bool wake;
> -
> -                       if (nmk_chip->edge_rising & BIT(offset))
> -                               trigger = "edge-rising";
> -                       else if (nmk_chip->edge_falling & BIT(offset))
> -                               trigger = "edge-falling";
> -                       else
> -                               trigger = "edge-undefined";
> -
> -                       wake = !!(nmk_chip->real_wake & BIT(offset));
> -
> -                       seq_printf(s, " irq-%d %s%s",
> -                                  irq, trigger, wake ? " wakeup" : "");
> -               }
> -       }
> -       clk_disable(nmk_chip->clk);
> -}
> -
> -static void nmk_gpio_dbg_show(struct seq_file *s, struct gpio_chip *chip)
> -{
> -       unsigned                i;
> -       unsigned                gpio = chip->base;
> -
> -       for (i = 0; i < chip->ngpio; i++, gpio++) {
> -               nmk_gpio_dbg_show_one(s, NULL, chip, i, gpio);
> -               seq_printf(s, "\n");
> -       }
> -}
> -
> -#else
> -static inline void nmk_gpio_dbg_show_one(struct seq_file *s,
> -                                        struct pinctrl_dev *pctldev,
> -                                        struct gpio_chip *chip,
> -                                        unsigned offset, unsigned gpio)
> -{
> -}
> -#define nmk_gpio_dbg_show      NULL
> -#endif
> -
> -/*
> - * We will allocate memory for the state container using devm* allocators
> - * binding to the first device reaching this point, it doesn't matter if
> - * it is the pin controller or GPIO driver. However we need to use the right
> - * platform device when looking up resources so pay attention to pdev.
> - */
> -static struct nmk_gpio_chip *nmk_gpio_populate_chip(struct device_node *np,
> -                                               struct platform_device *pdev)
> -{
> -       struct nmk_gpio_chip *nmk_chip;
> -       struct platform_device *gpio_pdev;
> -       struct gpio_chip *chip;
> -       struct resource *res;
> -       struct clk *clk;
> -       void __iomem *base;
> -       u32 id;
> -
> -       gpio_pdev = of_find_device_by_node(np);
> -       if (!gpio_pdev) {
> -               pr_err("populate \"%pOFn\": device not found\n", np);
> -               return ERR_PTR(-ENODEV);
> -       }
> -       if (of_property_read_u32(np, "gpio-bank", &id)) {
> -               dev_err(&pdev->dev, "populate: gpio-bank property not found\n");
> -               platform_device_put(gpio_pdev);
> -               return ERR_PTR(-EINVAL);
> -       }
> -
> -       /* Already populated? */
> -       nmk_chip = nmk_gpio_chips[id];
> -       if (nmk_chip) {
> -               platform_device_put(gpio_pdev);
> -               return nmk_chip;
> -       }
> -
> -       nmk_chip = devm_kzalloc(&pdev->dev, sizeof(*nmk_chip), GFP_KERNEL);
> -       if (!nmk_chip) {
> -               platform_device_put(gpio_pdev);
> -               return ERR_PTR(-ENOMEM);
> -       }
> -
> -       nmk_chip->bank = id;
> -       chip = &nmk_chip->chip;
> -       chip->base = id * NMK_GPIO_PER_CHIP;
> -       chip->ngpio = NMK_GPIO_PER_CHIP;
> -       chip->label = dev_name(&gpio_pdev->dev);
> -       chip->parent = &gpio_pdev->dev;
> -
> -       res = platform_get_resource(gpio_pdev, IORESOURCE_MEM, 0);
> -       base = devm_ioremap_resource(&pdev->dev, res);
> -       if (IS_ERR(base)) {
> -               platform_device_put(gpio_pdev);
> -               return ERR_CAST(base);
> -       }
> -       nmk_chip->addr = base;
> -
> -       clk = clk_get(&gpio_pdev->dev, NULL);
> -       if (IS_ERR(clk)) {
> -               platform_device_put(gpio_pdev);
> -               return (void *) clk;
> -       }
> -       clk_prepare(clk);
> -       nmk_chip->clk = clk;
> -
> -       BUG_ON(nmk_chip->bank >= ARRAY_SIZE(nmk_gpio_chips));
> -       nmk_gpio_chips[id] = nmk_chip;
> -       return nmk_chip;
> -}
> -
> -static void nmk_gpio_irq_print_chip(struct irq_data *d, struct seq_file *p)
> -{
> -       struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
> -       struct nmk_gpio_chip *nmk_chip = gpiochip_get_data(gc);
> -
> -       seq_printf(p, "nmk%u-%u-%u", nmk_chip->bank,
> -                  gc->base, gc->base + gc->ngpio - 1);
> -}
> -
> -static const struct irq_chip nmk_irq_chip = {
> -       .irq_ack = nmk_gpio_irq_ack,
> -       .irq_mask = nmk_gpio_irq_mask,
> -       .irq_unmask = nmk_gpio_irq_unmask,
> -       .irq_set_type = nmk_gpio_irq_set_type,
> -       .irq_set_wake = nmk_gpio_irq_set_wake,
> -       .irq_startup = nmk_gpio_irq_startup,
> -       .irq_shutdown = nmk_gpio_irq_shutdown,
> -       .irq_print_chip = nmk_gpio_irq_print_chip,
> -       .flags = IRQCHIP_MASK_ON_SUSPEND | IRQCHIP_IMMUTABLE,
> -       GPIOCHIP_IRQ_RESOURCE_HELPERS,
> -};
> -
> -static int nmk_gpio_probe(struct platform_device *dev)
> -{
> -       struct device_node *np = dev->dev.of_node;
> -       struct nmk_gpio_chip *nmk_chip;
> -       struct gpio_chip *chip;
> -       struct gpio_irq_chip *girq;
> -       bool supports_sleepmode;
> -       int irq;
> -       int ret;
> -
> -       nmk_chip = nmk_gpio_populate_chip(np, dev);
> -       if (IS_ERR(nmk_chip)) {
> -               dev_err(&dev->dev, "could not populate nmk chip struct\n");
> -               return PTR_ERR(nmk_chip);
> -       }
> -
> -       supports_sleepmode =
> -               of_property_read_bool(np, "st,supports-sleepmode");
> -
> -       /* Correct platform device ID */
> -       dev->id = nmk_chip->bank;
> -
> -       irq = platform_get_irq(dev, 0);
> -       if (irq < 0)
> -               return irq;
> -
> -       /*
> -        * The virt address in nmk_chip->addr is in the nomadik register space,
> -        * so we can simply convert the resource address, without remapping
> -        */
> -       nmk_chip->sleepmode = supports_sleepmode;
> -       spin_lock_init(&nmk_chip->lock);
> -
> -       chip = &nmk_chip->chip;
> -       chip->parent = &dev->dev;
> -       chip->request = gpiochip_generic_request;
> -       chip->free = gpiochip_generic_free;
> -       chip->get_direction = nmk_gpio_get_dir;
> -       chip->direction_input = nmk_gpio_make_input;
> -       chip->get = nmk_gpio_get_input;
> -       chip->direction_output = nmk_gpio_make_output;
> -       chip->set = nmk_gpio_set_output;
> -       chip->dbg_show = nmk_gpio_dbg_show;
> -       chip->can_sleep = false;
> -       chip->owner = THIS_MODULE;
> -
> -       girq = &chip->irq;
> -       gpio_irq_chip_set_chip(girq, &nmk_irq_chip);
> -       girq->parent_handler = nmk_gpio_irq_handler;
> -       girq->num_parents = 1;
> -       girq->parents = devm_kcalloc(&dev->dev, 1,
> -                                    sizeof(*girq->parents),
> -                                    GFP_KERNEL);
> -       if (!girq->parents)
> -               return -ENOMEM;
> -       girq->parents[0] = irq;
> -       girq->default_type = IRQ_TYPE_NONE;
> -       girq->handler = handle_edge_irq;
> -
> -       clk_enable(nmk_chip->clk);
> -       nmk_chip->lowemi = readl_relaxed(nmk_chip->addr + NMK_GPIO_LOWEMI);
> -       clk_disable(nmk_chip->clk);
> -
> -       ret = gpiochip_add_data(chip, nmk_chip);
> -       if (ret)
> -               return ret;
> -
> -       platform_set_drvdata(dev, nmk_chip);
> -
> -       dev_info(&dev->dev, "chip registered\n");
> -
> -       return 0;
> -}
> -
>  static int nmk_get_groups_cnt(struct pinctrl_dev *pctldev)
>  {
>         struct nmk_pinctrl *npct = pinctrl_dev_get_drvdata(pctldev);
> @@ -1197,12 +524,12 @@ static const char *nmk_get_group_name(struct pinctrl_dev *pctldev,
>
>  static int nmk_get_group_pins(struct pinctrl_dev *pctldev, unsigned selector,
>                               const unsigned **pins,
> -                             unsigned *npins)
> +                             unsigned int *num_pins)
>  {
>         struct nmk_pinctrl *npct = pinctrl_dev_get_drvdata(pctldev);
>
>         *pins = npct->soc->groups[selector].grp.pins;
> -       *npins = npct->soc->groups[selector].grp.npins;
> +       *num_pins = npct->soc->groups[selector].grp.npins;
>         return 0;
>  }
>
> @@ -1533,7 +860,7 @@ static int nmk_pmx_set(struct pinctrl_dev *pctldev, unsigned function,
>  {
>         struct nmk_pinctrl *npct = pinctrl_dev_get_drvdata(pctldev);
>         const struct nmk_pingroup *g;
> -       static unsigned int slpm[NUM_BANKS];
> +       static unsigned int slpm[NMK_MAX_BANKS];
>         unsigned long flags = 0;
>         bool glitch;
>         int ret = -EINVAL;
> @@ -1919,19 +1246,6 @@ static int nmk_pinctrl_probe(struct platform_device *pdev)
>         return 0;
>  }
>
> -static const struct of_device_id nmk_gpio_match[] = {
> -       { .compatible = "st,nomadik-gpio", },
> -       {}
> -};
> -
> -static struct platform_driver nmk_gpio_driver = {
> -       .driver = {
> -               .name = "gpio",
> -               .of_match_table = nmk_gpio_match,
> -       },
> -       .probe = nmk_gpio_probe,
> -};
> -
>  static SIMPLE_DEV_PM_OPS(nmk_pinctrl_pm_ops,
>                         nmk_pinctrl_suspend,
>                         nmk_pinctrl_resume);
> @@ -1945,12 +1259,6 @@ static struct platform_driver nmk_pinctrl_driver = {
>         .probe = nmk_pinctrl_probe,
>  };
>
> -static int __init nmk_gpio_init(void)
> -{
> -       return platform_driver_register(&nmk_gpio_driver);
> -}
> -subsys_initcall(nmk_gpio_init);
> -
>  static int __init nmk_pinctrl_init(void)
>  {
>         return platform_driver_register(&nmk_pinctrl_driver);
> diff --git a/drivers/pinctrl/nomadik/pinctrl-nomadik.h b/include/linux/gpio/gpio-nomadik.h
> similarity index 61%
> rename from drivers/pinctrl/nomadik/pinctrl-nomadik.h
> rename to include/linux/gpio/gpio-nomadik.h
> index 1ef2559bc571..0166ddb71f43 100644
> --- a/drivers/pinctrl/nomadik/pinctrl-nomadik.h
> +++ b/include/linux/gpio/gpio-nomadik.h
> @@ -1,16 +1,74 @@
>  /* SPDX-License-Identifier: GPL-2.0 */
> -#ifndef PINCTRL_PINCTRL_NOMADIK_H
> -#define PINCTRL_PINCTRL_NOMADIK_H
> -
> -#include <linux/kernel.h>
> -#include <linux/types.h>
> -
> -#include <linux/pinctrl/pinctrl.h>
> +#ifndef __LINUX_GPIO_NOMADIK_H
> +#define __LINUX_GPIO_NOMADIK_H
>
>  /* Package definitions */
>  #define PINCTRL_NMK_STN8815    0
>  #define PINCTRL_NMK_DB8500     1
>
> +#define GPIO_BLOCK_SHIFT 5
> +#define NMK_GPIO_PER_CHIP BIT(GPIO_BLOCK_SHIFT)
> +#define NMK_MAX_BANKS DIV_ROUND_UP(512, NMK_GPIO_PER_CHIP)
> +
> +/* Register in the logic block */
> +#define NMK_GPIO_DAT   0x00
> +#define NMK_GPIO_DATS  0x04
> +#define NMK_GPIO_DATC  0x08
> +#define NMK_GPIO_PDIS  0x0c
> +#define NMK_GPIO_DIR   0x10
> +#define NMK_GPIO_DIRS  0x14
> +#define NMK_GPIO_DIRC  0x18
> +#define NMK_GPIO_SLPC  0x1c
> +#define NMK_GPIO_AFSLA 0x20
> +#define NMK_GPIO_AFSLB 0x24
> +#define NMK_GPIO_LOWEMI        0x28
> +
> +#define NMK_GPIO_RIMSC 0x40
> +#define NMK_GPIO_FIMSC 0x44
> +#define NMK_GPIO_IS    0x48
> +#define NMK_GPIO_IC    0x4c
> +#define NMK_GPIO_RWIMSC        0x50
> +#define NMK_GPIO_FWIMSC        0x54
> +#define NMK_GPIO_WKS   0x58
> +/* These appear in DB8540 and later ASICs */
> +#define NMK_GPIO_EDGELEVEL 0x5C
> +#define NMK_GPIO_LEVEL 0x60
> +
> +/* Pull up/down values */
> +enum nmk_gpio_pull {
> +       NMK_GPIO_PULL_NONE,
> +       NMK_GPIO_PULL_UP,
> +       NMK_GPIO_PULL_DOWN,
> +};
> +
> +/* Sleep mode */
> +enum nmk_gpio_slpm {
> +       NMK_GPIO_SLPM_INPUT,
> +       NMK_GPIO_SLPM_WAKEUP_ENABLE = NMK_GPIO_SLPM_INPUT,
> +       NMK_GPIO_SLPM_NOCHANGE,
> +       NMK_GPIO_SLPM_WAKEUP_DISABLE = NMK_GPIO_SLPM_NOCHANGE,
> +};
> +
> +struct nmk_gpio_chip {
> +       struct gpio_chip chip;
> +       void __iomem *addr;
> +       struct clk *clk;
> +       unsigned int bank;
> +       void (*set_ioforce)(bool enable);
> +       spinlock_t lock;
> +       bool sleepmode;
> +       /* Keep track of configured edges */
> +       u32 edge_rising;
> +       u32 edge_falling;
> +       u32 real_wake;
> +       u32 rwimsc;
> +       u32 fwimsc;
> +       u32 rimsc;
> +       u32 fimsc;
> +       u32 pull_up;
> +       u32 lowemi;
> +};
> +
>  /* Alternate functions: function C is set in hw by setting both A and B */
>  #define NMK_GPIO_ALT_GPIO      0
>  #define NMK_GPIO_ALT_A 1
> @@ -104,7 +162,7 @@ struct prcm_gpiocr_altcx_pin_desc {
>  struct nmk_function {
>         const char *name;
>         const char * const *groups;
> -       unsigned ngroups;
> +       unsigned int ngroups;
>  };
>
>  /**
> @@ -141,13 +199,13 @@ struct nmk_pingroup {
>   */
>  struct nmk_pinctrl_soc_data {
>         const struct pinctrl_pin_desc *pins;
> -       unsigned npins;
> +       unsigned int npins;
>         const struct nmk_function *functions;
> -       unsigned nfunctions;
> +       unsigned int nfunctions;
>         const struct nmk_pingroup *groups;
> -       unsigned ngroups;
> +       unsigned int ngroups;
>         const struct prcm_gpiocr_altcx_pin_desc *altcx_pins;
> -       unsigned npins_altcx;
> +       unsigned int npins_altcx;
>         const u16 *prcm_gpiocr_registers;
>  };
>
> @@ -177,4 +235,42 @@ nmk_pinctrl_db8500_init(const struct nmk_pinctrl_soc_data **soc)
>
>  #endif
>
> -#endif /* PINCTRL_PINCTRL_NOMADIK_H */
> +#ifdef CONFIG_PINCTRL_DB8540
> +
> +void nmk_pinctrl_db8540_init(const struct nmk_pinctrl_soc_data **soc);
> +
> +#else
> +
> +static inline void
> +nmk_pinctrl_db8540_init(const struct nmk_pinctrl_soc_data **soc)
> +{
> +}
> +
> +#endif
> +
> +struct platform_device;
> +
> +/*
> + * Symbols declared in gpio-nomadik used by pinctrl-nomadik. If pinctrl-nomadik
> + * is enabled, then gpio-nomadik is enabled as well; the reverse if not always
> + * true.
> + */
> +void nmk_gpio_dbg_show_one(struct seq_file *s, struct pinctrl_dev *pctldev,
> +                          struct gpio_chip *chip, unsigned int offset,
> +                          unsigned int gpio);
> +void __nmk_gpio_make_output(struct nmk_gpio_chip *nmk_chip,
> +                           unsigned int offset, int val);
> +void __nmk_gpio_set_slpm(struct nmk_gpio_chip *nmk_chip, unsigned int offset,
> +                        enum nmk_gpio_slpm mode);
> +struct nmk_gpio_chip *nmk_gpio_populate_chip(struct device_node *np,
> +                                            struct platform_device *pdev);
> +
> +/* Symbols declared in pinctrl-nomadik used by gpio-nomadik. */
> +#ifdef CONFIG_PINCTRL_NOMADIK
> +extern struct nmk_gpio_chip *nmk_gpio_chips[NMK_MAX_BANKS];
> +extern spinlock_t nmk_gpio_slpm_lock;
> +int __maybe_unused nmk_prcm_gpiocr_get_mode(struct pinctrl_dev *pctldev,
> +                                           int gpio);
> +#endif
> +
> +#endif /* __LINUX_GPIO_NOMADIK_H */
>
> --
> 2.43.1
>

Bart

Re: [PATCH 05/23] gpio: nomadik: extract GPIO platform driver from drivers/pinctrl/nomadik/

[Théo Lebrun]

Hello Bartosz,

On Mon Feb 19, 2024 at 5:08 PM CET, Bartosz Golaszewski wrote:
> On Wed, Feb 14, 2024 at 5:24 PM Théo Lebrun <theo.lebrun@bootlin.com> wrote:
> >
> > Previously, drivers/pinctrl/nomadik/pinctrl-nomadik.c registered two
> > platform drivers: pinctrl & GPIO. Move the GPIO aspect to the
> > drivers/gpio/ folder, as would be expected.
> >
> > Both drivers are intertwined for a reason; pinctrl requires access to
> > GPIO registers for pinmuxing, pull-disable, disabling interrupts while
> > setting the muxing and wakeup control. Information sharing is done
> > through a shared array containing GPIO chips and a few helper
> > functions. That shared array is not touched from gpio-nomadik when
> > CONFIG_PINCTRL_NOMADIK is not defined.
> >
> > Make no change to the code that moved into gpio-nomadik; there should be
> > no behavior change following. A few functions are shared and header
> > comments are added. Checkpatch warnings are addressed. NUM_BANKS is
> > renamed to NMK_MAX_BANKS.
> >
> > It is supported to compile gpio-nomadik without pinctrl-nomadik. The
> > opposite is not true.
> >
> > Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> > ---
> >  MAINTAINERS                                        |   1 +
> >  drivers/gpio/Kconfig                               |  12 +
> >  drivers/gpio/Makefile                              |   1 +
> >  drivers/gpio/gpio-nomadik.c                        | 660 +++++++++++++++++++
> >  drivers/pinctrl/nomadik/Kconfig                    |   5 +-
> >  drivers/pinctrl/nomadik/pinctrl-nomadik-db8500.c   |   3 +-
> >  drivers/pinctrl/nomadik/pinctrl-nomadik-stn8815.c  |   3 +-
> >  drivers/pinctrl/nomadik/pinctrl-nomadik.c          | 722 +--------------------
> >  .../linux/gpio/gpio-nomadik.h                      | 122 +++-
> >  9 files changed, 804 insertions(+), 725 deletions(-)
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 0cb2c459d1cf..3f864e773267 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -2474,6 +2474,7 @@ F:        drivers/clk/clk-nomadik.c
> >  F:     drivers/clocksource/clksrc-dbx500-prcmu.c
> >  F:     drivers/dma/ste_dma40*
> >  F:     drivers/pmdomain/st/ste-ux500-pm-domain.c
> > +F:     drivers/gpio/gpio-nomadik.c
> >  F:     drivers/hwspinlock/u8500_hsem.c
> >  F:     drivers/i2c/busses/i2c-nomadik.c
> >  F:     drivers/iio/adc/ab8500-gpadc.c
> > diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> > index 1301cec94f12..ff83371251c1 100644
> > --- a/drivers/gpio/Kconfig
> > +++ b/drivers/gpio/Kconfig
> > @@ -478,6 +478,18 @@ config GPIO_MXS
> >         select GPIO_GENERIC
> >         select GENERIC_IRQ_CHIP
> >
> > +config GPIO_NOMADIK
> > +       bool "Nomadik GPIO driver"
> > +       depends on ARCH_U8500 || ARCH_NOMADIK || COMPILE_TEST
> > +       select OF_GPIO
> > +       select GPIOLIB_IRQCHIP
> > +       help
> > +         Say yes here to support the Nomadik SoC GPIO block.
> > +
> > +         It handles up to 32 GPIOs per bank, that can all be interrupt sources.
> > +         It is deeply interconnected with the associated pinctrl driver as GPIO
> > +         registers handle muxing ("alternate functions") as well.
> > +
> >  config GPIO_NPCM_SGPIO
> >         bool "Nuvoton SGPIO support"
> >         depends on ARCH_NPCM || COMPILE_TEST
> > diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
> > index 9e40af196aae..9fc2f5931b22 100644
> > --- a/drivers/gpio/Makefile
> > +++ b/drivers/gpio/Makefile
> > @@ -116,6 +116,7 @@ obj-$(CONFIG_GPIO_MT7621)           += gpio-mt7621.o
> >  obj-$(CONFIG_GPIO_MVEBU)               += gpio-mvebu.o
> >  obj-$(CONFIG_GPIO_MXC)                 += gpio-mxc.o
> >  obj-$(CONFIG_GPIO_MXS)                 += gpio-mxs.o
> > +obj-$(CONFIG_GPIO_NOMADIK)             += gpio-nomadik.o
> >  obj-$(CONFIG_GPIO_NPCM_SGPIO)          += gpio-npcm-sgpio.o
> >  obj-$(CONFIG_GPIO_OCTEON)              += gpio-octeon.o
> >  obj-$(CONFIG_GPIO_OMAP)                        += gpio-omap.o
> > diff --git a/drivers/gpio/gpio-nomadik.c b/drivers/gpio/gpio-nomadik.c
> > new file mode 100644
> > index 000000000000..e39477e1a58f
> > --- /dev/null
> > +++ b/drivers/gpio/gpio-nomadik.c
> > @@ -0,0 +1,660 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * GPIO driver for the IP block found in the Nomadik SoC; it is an AMBA device,
> > + * managing 32 pins with alternate functions. It can also handle the STA2X11
> > + * block from ST.
> > + *
> > + * The GPIO chips are shared with pinctrl-nomadik if used; it needs access for
> > + * pinmuxing functionality and others.
> > + *
> > + * Copyright (C) 2008,2009 STMicroelectronics
> > + * Copyright (C) 2009 Alessandro Rubini <rubini@unipv.it>
> > + *   Rewritten based on work by Prafulla WADASKAR <prafulla.wadaskar@st.com>
> > + * Copyright (C) 2011-2013 Linus Walleij <linus.walleij@linaro.org>
> > + */
>
> Add a newline here.

This commit tries its best to not modify the file too much. It mostly is
a copy-and-paste. The goal is to have a sensible diff between old
drivers/pinctrl/nomadik/pinctrl-nomadik.c and new
drivers/gpio/gpio-nomadik.c. We wait until later commits to fix stuff.

Should below comments "avoid new calls to X" still be taken into
account, knowing that this is old code being moved around?

Thanks,

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

------------------------------------------------------------------------

Re: [PATCH 05/23] gpio: nomadik: extract GPIO platform driver from drivers/pinctrl/nomadik/

[Linus Walleij]

Hi Theo,

just a quick note here:

On Wed, Feb 14, 2024 at 5:24 PM Théo Lebrun <theo.lebrun@bootlin.com> wrote:

> +config GPIO_NOMADIK
> +       bool "Nomadik GPIO driver"
> +       depends on ARCH_U8500 || ARCH_NOMADIK || COMPILE_TEST
> +       select OF_GPIO
> +       select GPIOLIB_IRQCHIP

Could you add:

default PINCTRL_NOMADIK

so it is turned on by default when we have that, since they are jitted together
so closely.

Yours,
Linus Walleij

Re: [PATCH 05/23] gpio: nomadik: extract GPIO platform driver from drivers/pinctrl/nomadik/

[Théo Lebrun]

Hello,

On Wed Feb 21, 2024 at 3:37 PM CET, Linus Walleij wrote:
> just a quick note here:
>
> On Wed, Feb 14, 2024 at 5:24 PM Théo Lebrun <theo.lebrun@bootlin.com> wrote:
>
> > +config GPIO_NOMADIK
> > +       bool "Nomadik GPIO driver"
> > +       depends on ARCH_U8500 || ARCH_NOMADIK || COMPILE_TEST
> > +       select OF_GPIO
> > +       select GPIOLIB_IRQCHIP
>
> Could you add:
>
> default PINCTRL_NOMADIK
>
> so it is turned on by default when we have that, since they are jitted together
> so closely.

Would that bring something more than what is currently present? I've set
PINCTRL_NOMADIK to select GPIO_NOMADIK. This means that if
PINCTRL_NOMADIK=y then GPIO_NOMADIK=y. If PINCTRL_NOMADIK=n then
GPIO_NOMADIK is free to be whatever.

This behavior sounds similar to what would happen if adding "default
PINCTRL_NOMADIK".

Thanks,

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH 05/23] gpio: nomadik: extract GPIO platform driver from drivers/pinctrl/nomadik/

[Linus Walleij]

On Wed, Feb 21, 2024 at 5:20 PM Théo Lebrun <theo.lebrun@bootlin.com> wrote:

> > Could you add:
> >
> > default PINCTRL_NOMADIK
> >
> > so it is turned on by default when we have that, since they are jitted together
> > so closely.
>
> Would that bring something more than what is currently present? I've set
> PINCTRL_NOMADIK to select GPIO_NOMADIK.

I missed that!

OK no problem, all works fine.

Yours,
Linus Walleij

[PATCH 06/23] pinctrl: nomadik: fix build warning (-Wformat)

[Théo Lebrun]

Fix compiler warning found in the pinctrl-nomadik platform driver. GCC
message is as such:

drivers/pinctrl/nomadik/pinctrl-nomadik.c:855:21: warning:
format ‘%u’ expects argument of type ‘unsigned int’, but argument 5
has type ‘size_t {aka const long unsigned int}’ [-Wformat=]

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 drivers/pinctrl/nomadik/pinctrl-nomadik.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pinctrl/nomadik/pinctrl-nomadik.c b/drivers/pinctrl/nomadik/pinctrl-nomadik.c
index f3897dbfa2c3..d2a40c3dd0f6 100644
--- a/drivers/pinctrl/nomadik/pinctrl-nomadik.c
+++ b/drivers/pinctrl/nomadik/pinctrl-nomadik.c
@@ -871,7 +871,7 @@ static int nmk_pmx_set(struct pinctrl_dev *pctldev, unsigned function,
 	if (g->altsetting < 0)
 		return -EINVAL;
 
-	dev_dbg(npct->dev, "enable group %s, %u pins\n", g->grp.name, g->grp.npins);
+	dev_dbg(npct->dev, "enable group %s, %zu pins\n", g->grp.name, g->grp.npins);
 
 	/*
 	 * If we're setting altfunc C by setting both AFSLA and AFSLB to 1,

-- 
2.43.1

[PATCH 07/23] pinctrl: nomadik: fix build warning (-Wpointer-to-int-cast)

[Théo Lebrun]

Fix compiler warning found in the pinctrl-nomadik platform driver. GCC
message is as such:

drivers/pinctrl/nomadik/pinctrl-nomadik.c:1169:12: warning: cast from
pointer to integer of different size [-Wpointer-to-int-cast]

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 drivers/pinctrl/nomadik/pinctrl-nomadik.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pinctrl/nomadik/pinctrl-nomadik.c b/drivers/pinctrl/nomadik/pinctrl-nomadik.c
index d2a40c3dd0f6..60443de439fd 100644
--- a/drivers/pinctrl/nomadik/pinctrl-nomadik.c
+++ b/drivers/pinctrl/nomadik/pinctrl-nomadik.c
@@ -1174,14 +1174,14 @@ static int nmk_pinctrl_probe(struct platform_device *pdev)
 	struct device_node *np = pdev->dev.of_node;
 	struct device_node *prcm_np;
 	struct nmk_pinctrl *npct;
-	unsigned int version = 0;
+	uintptr_t version = 0;
 	int i;
 
 	npct = devm_kzalloc(&pdev->dev, sizeof(*npct), GFP_KERNEL);
 	if (!npct)
 		return -ENOMEM;
 
-	version = (unsigned int)device_get_match_data(&pdev->dev);
+	version = (uintptr_t)device_get_match_data(&pdev->dev);
 
 	/* Poke in other ASIC variants here */
 	if (version == PINCTRL_NMK_STN8815)

-- 
2.43.1

[PATCH 08/23] pinctrl: nomadik: minimise indentation in probe

[Théo Lebrun]

nmk_pinctrl_probe() iterates over each GPIO block. Use an early
conditional continue to skip to the next iteration rather than indent
all the loop code block.

Do not change code logic. The block is changed from:

	for (i = 0; i < NMK_MAX_BANKS; i++) {
		x = of_parse_phandle(...);
		if (x) {
			... do work ...
		}
	}

To:

	for (i = 0; i < NMK_MAX_BANKS; i++) {
		x = of_parse_phandle(...);
		if (!x)
			continue;

		... do work ...
	}

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 drivers/pinctrl/nomadik/pinctrl-nomadik.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/pinctrl/nomadik/pinctrl-nomadik.c b/drivers/pinctrl/nomadik/pinctrl-nomadik.c
index 60443de439fd..c1cb3a363692 100644
--- a/drivers/pinctrl/nomadik/pinctrl-nomadik.c
+++ b/drivers/pinctrl/nomadik/pinctrl-nomadik.c
@@ -1201,17 +1201,16 @@ static int nmk_pinctrl_probe(struct platform_device *pdev)
 		struct nmk_gpio_chip *nmk_chip;
 
 		gpio_np = of_parse_phandle(np, "nomadik-gpio-chips", i);
-		if (gpio_np) {
-			dev_info(&pdev->dev,
-				 "populate NMK GPIO %d \"%pOFn\"\n",
-				 i, gpio_np);
-			nmk_chip = nmk_gpio_populate_chip(gpio_np, pdev);
-			if (IS_ERR(nmk_chip))
-				dev_err(&pdev->dev,
-					"could not populate nmk chip struct "
-					"- continue anyway\n");
-			of_node_put(gpio_np);
-		}
+		if (!gpio_np)
+			continue;
+
+		dev_info(&pdev->dev, "populate NMK GPIO %d \"%pOFn\"\n",
+			 i, gpio_np);
+		nmk_chip = nmk_gpio_populate_chip(gpio_np, pdev);
+		if (IS_ERR(nmk_chip))
+			dev_err(&pdev->dev,
+				"could not populate nmk chip struct - continue anyway\n");
+		of_node_put(gpio_np);
 	}
 
 	prcm_np = of_parse_phandle(np, "prcm", 0);

-- 
2.43.1

[PATCH 09/23] pinctrl: nomadik: follow type-system kernel coding conventions

[Théo Lebrun]

Fix strict checkpatch warnings relative to types. Warning types
addressed:

   WARNING: do not add new typedefs
   WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
   WARNING: static const char * array should probably be static const
            char * const

Total messages before: 1 errors, 40 warnings, 39 checks.
Total messages after:  1 errors,  2 warnings, 38 checks.

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 drivers/pinctrl/nomadik/pinctrl-nomadik.c | 78 +++++++++++++++----------------
 1 file changed, 38 insertions(+), 40 deletions(-)

diff --git a/drivers/pinctrl/nomadik/pinctrl-nomadik.c b/drivers/pinctrl/nomadik/pinctrl-nomadik.c
index c1cb3a363692..94338a3c8d4f 100644
--- a/drivers/pinctrl/nomadik/pinctrl-nomadik.c
+++ b/drivers/pinctrl/nomadik/pinctrl-nomadik.c
@@ -71,8 +71,6 @@
  * PIN_CFG	   - default config with alternate function
  */
 
-typedef unsigned long pin_cfg_t;
-
 #define PIN_NUM_MASK		0x1ff
 #define PIN_NUM(x)		((x) & PIN_NUM_MASK)
 
@@ -215,7 +213,7 @@ struct nmk_gpio_chip *nmk_gpio_chips[NMK_MAX_BANKS];
 DEFINE_SPINLOCK(nmk_gpio_slpm_lock);
 
 static void __nmk_gpio_set_mode(struct nmk_gpio_chip *nmk_chip,
-				unsigned offset, int gpio_mode)
+				unsigned int offset, int gpio_mode)
 {
 	u32 afunc, bfunc;
 
@@ -230,7 +228,7 @@ static void __nmk_gpio_set_mode(struct nmk_gpio_chip *nmk_chip,
 }
 
 static void __nmk_gpio_set_pull(struct nmk_gpio_chip *nmk_chip,
-				unsigned offset, enum nmk_gpio_pull pull)
+				unsigned int offset, enum nmk_gpio_pull pull)
 {
 	u32 pdis;
 
@@ -254,7 +252,7 @@ static void __nmk_gpio_set_pull(struct nmk_gpio_chip *nmk_chip,
 }
 
 static void __nmk_gpio_set_lowemi(struct nmk_gpio_chip *nmk_chip,
-				  unsigned offset, bool lowemi)
+				  unsigned int offset, bool lowemi)
 {
 	bool enabled = nmk_chip->lowemi & BIT(offset);
 
@@ -271,13 +269,13 @@ static void __nmk_gpio_set_lowemi(struct nmk_gpio_chip *nmk_chip,
 }
 
 static void __nmk_gpio_make_input(struct nmk_gpio_chip *nmk_chip,
-				  unsigned offset)
+				  unsigned int offset)
 {
 	writel(BIT(offset), nmk_chip->addr + NMK_GPIO_DIRC);
 }
 
 static void __nmk_gpio_set_mode_safe(struct nmk_gpio_chip *nmk_chip,
-				     unsigned offset, int gpio_mode,
+				     unsigned int offset, int gpio_mode,
 				     bool glitch)
 {
 	u32 rwimsc = nmk_chip->rwimsc;
@@ -304,7 +302,7 @@ static void __nmk_gpio_set_mode_safe(struct nmk_gpio_chip *nmk_chip,
 }
 
 static void
-nmk_gpio_disable_lazy_irq(struct nmk_gpio_chip *nmk_chip, unsigned offset)
+nmk_gpio_disable_lazy_irq(struct nmk_gpio_chip *nmk_chip, unsigned int offset)
 {
 	u32 falling = nmk_chip->fimsc & BIT(offset);
 	u32 rising = nmk_chip->rimsc & BIT(offset);
@@ -343,7 +341,7 @@ static void nmk_write_masked(void __iomem *reg, u32 mask, u32 value)
 }
 
 static void nmk_prcm_altcx_set_mode(struct nmk_pinctrl *npct,
-	unsigned offset, unsigned alt_num)
+	unsigned int offset, unsigned int alt_num)
 {
 	int i;
 	u16 reg;
@@ -515,15 +513,15 @@ static int nmk_get_groups_cnt(struct pinctrl_dev *pctldev)
 }
 
 static const char *nmk_get_group_name(struct pinctrl_dev *pctldev,
-				       unsigned selector)
+				       unsigned int selector)
 {
 	struct nmk_pinctrl *npct = pinctrl_dev_get_drvdata(pctldev);
 
 	return npct->soc->groups[selector].grp.name;
 }
 
-static int nmk_get_group_pins(struct pinctrl_dev *pctldev, unsigned selector,
-			      const unsigned **pins,
+static int nmk_get_group_pins(struct pinctrl_dev *pctldev, unsigned int selector,
+			      const unsigned int **pins,
 			      unsigned int *num_pins)
 {
 	struct nmk_pinctrl *npct = pinctrl_dev_get_drvdata(pctldev);
@@ -533,7 +531,7 @@ static int nmk_get_group_pins(struct pinctrl_dev *pctldev, unsigned selector,
 	return 0;
 }
 
-static struct nmk_gpio_chip *find_nmk_gpio_from_pin(unsigned pin)
+static struct nmk_gpio_chip *find_nmk_gpio_from_pin(unsigned int pin)
 {
 	int i;
 	struct nmk_gpio_chip *nmk_gpio;
@@ -549,7 +547,7 @@ static struct nmk_gpio_chip *find_nmk_gpio_from_pin(unsigned pin)
 	return NULL;
 }
 
-static struct gpio_chip *find_gc_from_pin(unsigned pin)
+static struct gpio_chip *find_gc_from_pin(unsigned int pin)
 {
 	struct nmk_gpio_chip *nmk_gpio = find_nmk_gpio_from_pin(pin);
 
@@ -559,7 +557,7 @@ static struct gpio_chip *find_gc_from_pin(unsigned pin)
 }
 
 static void nmk_pin_dbg_show(struct pinctrl_dev *pctldev, struct seq_file *s,
-		   unsigned offset)
+		   unsigned int offset)
 {
 	struct gpio_chip *chip = find_gc_from_pin(offset);
 
@@ -570,8 +568,8 @@ static void nmk_pin_dbg_show(struct pinctrl_dev *pctldev, struct seq_file *s,
 	nmk_gpio_dbg_show_one(s, pctldev, chip, offset - chip->base, offset);
 }
 
-static int nmk_dt_add_map_mux(struct pinctrl_map **map, unsigned *reserved_maps,
-		unsigned *num_maps, const char *group,
+static int nmk_dt_add_map_mux(struct pinctrl_map **map, unsigned int *reserved_maps,
+		unsigned int *num_maps, const char *group,
 		const char *function)
 {
 	if (*num_maps == *reserved_maps)
@@ -586,9 +584,9 @@ static int nmk_dt_add_map_mux(struct pinctrl_map **map, unsigned *reserved_maps,
 }
 
 static int nmk_dt_add_map_configs(struct pinctrl_map **map,
-		unsigned *reserved_maps,
-		unsigned *num_maps, const char *group,
-		unsigned long *configs, unsigned num_configs)
+		unsigned int *reserved_maps,
+		unsigned int *num_maps, const char *group,
+		unsigned long *configs, unsigned int num_configs)
 {
 	unsigned long *dup_configs;
 
@@ -727,8 +725,8 @@ static bool nmk_pinctrl_dt_get_config(struct device_node *np,
 static int nmk_pinctrl_dt_subnode_to_map(struct pinctrl_dev *pctldev,
 		struct device_node *np,
 		struct pinctrl_map **map,
-		unsigned *reserved_maps,
-		unsigned *num_maps)
+		unsigned int *reserved_maps,
+		unsigned int *num_maps)
 {
 	int ret;
 	const char *function = NULL;
@@ -795,9 +793,9 @@ static int nmk_pinctrl_dt_subnode_to_map(struct pinctrl_dev *pctldev,
 
 static int nmk_pinctrl_dt_node_to_map(struct pinctrl_dev *pctldev,
 				 struct device_node *np_config,
-				 struct pinctrl_map **map, unsigned *num_maps)
+				 struct pinctrl_map **map, unsigned int *num_maps)
 {
-	unsigned reserved_maps;
+	unsigned int reserved_maps;
 	struct device_node *np;
 	int ret;
 
@@ -835,7 +833,7 @@ static int nmk_pmx_get_funcs_cnt(struct pinctrl_dev *pctldev)
 }
 
 static const char *nmk_pmx_get_func_name(struct pinctrl_dev *pctldev,
-					 unsigned function)
+					 unsigned int function)
 {
 	struct nmk_pinctrl *npct = pinctrl_dev_get_drvdata(pctldev);
 
@@ -843,7 +841,7 @@ static const char *nmk_pmx_get_func_name(struct pinctrl_dev *pctldev,
 }
 
 static int nmk_pmx_get_func_groups(struct pinctrl_dev *pctldev,
-				   unsigned function,
+				   unsigned int function,
 				   const char * const **groups,
 				   unsigned * const num_groups)
 {
@@ -855,8 +853,8 @@ static int nmk_pmx_get_func_groups(struct pinctrl_dev *pctldev,
 	return 0;
 }
 
-static int nmk_pmx_set(struct pinctrl_dev *pctldev, unsigned function,
-		       unsigned group)
+static int nmk_pmx_set(struct pinctrl_dev *pctldev, unsigned int function,
+		       unsigned int group)
 {
 	struct nmk_pinctrl *npct = pinctrl_dev_get_drvdata(pctldev);
 	const struct nmk_pingroup *g;
@@ -913,7 +911,7 @@ static int nmk_pmx_set(struct pinctrl_dev *pctldev, unsigned function,
 
 	for (i = 0; i < g->grp.npins; i++) {
 		struct nmk_gpio_chip *nmk_chip;
-		unsigned bit;
+		unsigned int bit;
 
 		nmk_chip = find_nmk_gpio_from_pin(g->grp.pins[i]);
 		if (!nmk_chip) {
@@ -966,12 +964,12 @@ static int nmk_pmx_set(struct pinctrl_dev *pctldev, unsigned function,
 
 static int nmk_gpio_request_enable(struct pinctrl_dev *pctldev,
 				   struct pinctrl_gpio_range *range,
-				   unsigned offset)
+				   unsigned int offset)
 {
 	struct nmk_pinctrl *npct = pinctrl_dev_get_drvdata(pctldev);
 	struct nmk_gpio_chip *nmk_chip;
 	struct gpio_chip *chip;
-	unsigned bit;
+	unsigned int bit;
 
 	if (!range) {
 		dev_err(npct->dev, "invalid range\n");
@@ -997,7 +995,7 @@ static int nmk_gpio_request_enable(struct pinctrl_dev *pctldev,
 
 static void nmk_gpio_disable_free(struct pinctrl_dev *pctldev,
 				  struct pinctrl_gpio_range *range,
-				  unsigned offset)
+				  unsigned int offset)
 {
 	struct nmk_pinctrl *npct = pinctrl_dev_get_drvdata(pctldev);
 
@@ -1015,30 +1013,30 @@ static const struct pinmux_ops nmk_pinmux_ops = {
 	.strict = true,
 };
 
-static int nmk_pin_config_get(struct pinctrl_dev *pctldev, unsigned pin,
+static int nmk_pin_config_get(struct pinctrl_dev *pctldev, unsigned int pin,
 			      unsigned long *config)
 {
 	/* Not implemented */
 	return -EINVAL;
 }
 
-static int nmk_pin_config_set(struct pinctrl_dev *pctldev, unsigned pin,
-			      unsigned long *configs, unsigned num_configs)
+static int nmk_pin_config_set(struct pinctrl_dev *pctldev, unsigned int pin,
+			      unsigned long *configs, unsigned int num_configs)
 {
-	static const char *pullnames[] = {
+	static const char * const pullnames[] = {
 		[NMK_GPIO_PULL_NONE]	= "none",
 		[NMK_GPIO_PULL_UP]	= "up",
 		[NMK_GPIO_PULL_DOWN]	= "down",
 		[3] /* illegal */	= "??"
 	};
-	static const char *slpmnames[] = {
+	static const char * const slpmnames[] = {
 		[NMK_GPIO_SLPM_INPUT]		= "input/wakeup",
 		[NMK_GPIO_SLPM_NOCHANGE]	= "no-change/no-wakeup",
 	};
 	struct nmk_pinctrl *npct = pinctrl_dev_get_drvdata(pctldev);
 	struct nmk_gpio_chip *nmk_chip;
-	unsigned bit;
-	pin_cfg_t cfg;
+	unsigned int bit;
+	unsigned long cfg;
 	int pull, slpm, output, val, i;
 	bool lowemi, gpiomode, sleep;
 
@@ -1055,7 +1053,7 @@ static int nmk_pin_config_set(struct pinctrl_dev *pctldev, unsigned pin,
 		 * here we just ignore that part. It's being handled by the
 		 * framework and pinmux callback respectively.
 		 */
-		cfg = (pin_cfg_t) configs[i];
+		cfg = configs[i];
 		pull = PIN_PULL(cfg);
 		slpm = PIN_SLPM(cfg);
 		output = PIN_DIR(cfg);

-- 
2.43.1

[PATCH 10/23] pinctrl: nomadik: follow whitespace kernel coding conventions

[Théo Lebrun]

Fix strict checkpatch warnings relative to whitespace. Message types
addressed:

   ERROR: space required before the open parenthesis '('
   WARNING: quoted string split across lines
   CHECK: Alignment should match open parenthesis
   CHECK: Please don't use multiple blank lines
   CHECK: line length of 103 exceeds 100 columns
   CHECK: spaces preferred around that '+' (ctx:VxV)

Before: 1 errors, 2 warnings, 38 checks.
After:  0 errors, 1 warnings, 16 checks.

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 drivers/pinctrl/nomadik/pinctrl-nomadik.c | 65 +++++++++++++++----------------
 1 file changed, 32 insertions(+), 33 deletions(-)

diff --git a/drivers/pinctrl/nomadik/pinctrl-nomadik.c b/drivers/pinctrl/nomadik/pinctrl-nomadik.c
index 94338a3c8d4f..83e7868bc1d7 100644
--- a/drivers/pinctrl/nomadik/pinctrl-nomadik.c
+++ b/drivers/pinctrl/nomadik/pinctrl-nomadik.c
@@ -165,7 +165,6 @@
 #define PIN_SLEEPMODE_DISABLED	(0 << PIN_SLEEPMODE_SHIFT)
 #define PIN_SLEEPMODE_ENABLED	(1 << PIN_SLEEPMODE_SHIFT)
 
-
 /* Shortcuts.  Use these instead of separate DIR, PULL, and VAL.  */
 #define PIN_INPUT_PULLDOWN	(PIN_DIR_INPUT | PIN_PULL_DOWN)
 #define PIN_INPUT_PULLUP	(PIN_DIR_INPUT | PIN_PULL_UP)
@@ -341,7 +340,7 @@ static void nmk_write_masked(void __iomem *reg, u32 mask, u32 value)
 }
 
 static void nmk_prcm_altcx_set_mode(struct nmk_pinctrl *npct,
-	unsigned int offset, unsigned int alt_num)
+				    unsigned int offset, unsigned int alt_num)
 {
 	int i;
 	u16 reg;
@@ -385,7 +384,7 @@ static void nmk_prcm_altcx_set_mode(struct nmk_pinctrl *npct,
 					nmk_write_masked(npct->prcm_base + reg, BIT(bit), 0);
 					dev_dbg(npct->dev,
 						"PRCM GPIOCR: pin %i: alternate-C%i has been disabled\n",
-						offset, i+1);
+						offset, i + 1);
 				}
 			}
 		}
@@ -395,8 +394,8 @@ static void nmk_prcm_altcx_set_mode(struct nmk_pinctrl *npct,
 	alt_index = alt_num - 1;
 	if (pin_desc->altcx[alt_index].used == false) {
 		dev_warn(npct->dev,
-			"PRCM GPIOCR: pin %i: alternate-C%i does not exist\n",
-			offset, alt_num);
+			 "PRCM GPIOCR: pin %i: alternate-C%i does not exist\n",
+			 offset, alt_num);
 		return;
 	}
 
@@ -414,7 +413,7 @@ static void nmk_prcm_altcx_set_mode(struct nmk_pinctrl *npct,
 				nmk_write_masked(npct->prcm_base + reg, BIT(bit), 0);
 				dev_dbg(npct->dev,
 					"PRCM GPIOCR: pin %i: alternate-C%i has been disabled\n",
-					offset, i+1);
+					offset, i + 1);
 			}
 		}
 	}
@@ -422,7 +421,7 @@ static void nmk_prcm_altcx_set_mode(struct nmk_pinctrl *npct,
 	reg = gpiocr_regs[pin_desc->altcx[alt_index].reg_index];
 	bit = pin_desc->altcx[alt_index].control_bit;
 	dev_dbg(npct->dev, "PRCM GPIOCR: pin %i: alternate-C%i has been selected\n",
-		offset, alt_index+1);
+		offset, alt_index + 1);
 	nmk_write_masked(npct->prcm_base + reg, BIT(bit), BIT(bit));
 }
 
@@ -499,7 +498,7 @@ int __maybe_unused nmk_prcm_gpiocr_get_mode(struct pinctrl_dev *pctldev, int gpi
 			reg = gpiocr_regs[pin_desc->altcx[i].reg_index];
 			bit = pin_desc->altcx[i].control_bit;
 			if (readl(npct->prcm_base + reg) & BIT(bit))
-				return NMK_GPIO_ALT_C+i+1;
+				return NMK_GPIO_ALT_C + i + 1;
 		}
 	}
 	return NMK_GPIO_ALT_C;
@@ -513,7 +512,7 @@ static int nmk_get_groups_cnt(struct pinctrl_dev *pctldev)
 }
 
 static const char *nmk_get_group_name(struct pinctrl_dev *pctldev,
-				       unsigned int selector)
+				      unsigned int selector)
 {
 	struct nmk_pinctrl *npct = pinctrl_dev_get_drvdata(pctldev);
 
@@ -536,12 +535,12 @@ static struct nmk_gpio_chip *find_nmk_gpio_from_pin(unsigned int pin)
 	int i;
 	struct nmk_gpio_chip *nmk_gpio;
 
-	for(i = 0; i < NMK_MAX_BANKS; i++) {
+	for (i = 0; i < NMK_MAX_BANKS; i++) {
 		nmk_gpio = nmk_gpio_chips[i];
 		if (!nmk_gpio)
 			continue;
 		if (pin >= nmk_gpio->chip.base &&
-			pin < nmk_gpio->chip.base + nmk_gpio->chip.ngpio)
+		    pin < nmk_gpio->chip.base + nmk_gpio->chip.ngpio)
 			return nmk_gpio;
 	}
 	return NULL;
@@ -557,7 +556,7 @@ static struct gpio_chip *find_gc_from_pin(unsigned int pin)
 }
 
 static void nmk_pin_dbg_show(struct pinctrl_dev *pctldev, struct seq_file *s,
-		   unsigned int offset)
+			     unsigned int offset)
 {
 	struct gpio_chip *chip = find_gc_from_pin(offset);
 
@@ -569,8 +568,8 @@ static void nmk_pin_dbg_show(struct pinctrl_dev *pctldev, struct seq_file *s,
 }
 
 static int nmk_dt_add_map_mux(struct pinctrl_map **map, unsigned int *reserved_maps,
-		unsigned int *num_maps, const char *group,
-		const char *function)
+			      unsigned int *num_maps, const char *group,
+			      const char *function)
 {
 	if (*num_maps == *reserved_maps)
 		return -ENOSPC;
@@ -584,9 +583,9 @@ static int nmk_dt_add_map_mux(struct pinctrl_map **map, unsigned int *reserved_m
 }
 
 static int nmk_dt_add_map_configs(struct pinctrl_map **map,
-		unsigned int *reserved_maps,
-		unsigned int *num_maps, const char *group,
-		unsigned long *configs, unsigned int num_configs)
+				  unsigned int *reserved_maps,
+				  unsigned int *num_maps, const char *group,
+				  unsigned long *configs, unsigned int num_configs)
 {
 	unsigned long *dup_configs;
 
@@ -702,15 +701,14 @@ static const char *nmk_find_pin_name(struct pinctrl_dev *pctldev, const char *pi
 }
 
 static bool nmk_pinctrl_dt_get_config(struct device_node *np,
-		unsigned long *configs)
+				      unsigned long *configs)
 {
 	bool has_config = 0;
 	unsigned long cfg = 0;
 	int i, val, ret;
 
 	for (i = 0; i < ARRAY_SIZE(nmk_cfg_params); i++) {
-		ret = of_property_read_u32(np,
-				nmk_cfg_params[i].property, &val);
+		ret = of_property_read_u32(np, nmk_cfg_params[i].property, &val);
 		if (ret != -EINVAL) {
 			if (nmk_dt_pin_config(i, val, &cfg) == 0) {
 				*configs |= cfg;
@@ -723,10 +721,10 @@ static bool nmk_pinctrl_dt_get_config(struct device_node *np,
 }
 
 static int nmk_pinctrl_dt_subnode_to_map(struct pinctrl_dev *pctldev,
-		struct device_node *np,
-		struct pinctrl_map **map,
-		unsigned int *reserved_maps,
-		unsigned int *num_maps)
+					 struct device_node *np,
+					 struct pinctrl_map **map,
+					 unsigned int *reserved_maps,
+					 unsigned int *num_maps)
 {
 	int ret;
 	const char *function = NULL;
@@ -751,7 +749,7 @@ static int nmk_pinctrl_dt_subnode_to_map(struct pinctrl_dev *pctldev,
 
 		of_property_for_each_string(np, "groups", prop, group) {
 			ret = nmk_dt_add_map_mux(map, reserved_maps, num_maps,
-					  group, function);
+						 group, function);
 			if (ret < 0)
 				goto exit;
 		}
@@ -792,8 +790,9 @@ static int nmk_pinctrl_dt_subnode_to_map(struct pinctrl_dev *pctldev,
 }
 
 static int nmk_pinctrl_dt_node_to_map(struct pinctrl_dev *pctldev,
-				 struct device_node *np_config,
-				 struct pinctrl_map **map, unsigned int *num_maps)
+				      struct device_node *np_config,
+				      struct pinctrl_map **map,
+				      unsigned int *num_maps)
 {
 	unsigned int reserved_maps;
 	struct device_node *np;
@@ -805,7 +804,7 @@ static int nmk_pinctrl_dt_node_to_map(struct pinctrl_dev *pctldev,
 
 	for_each_child_of_node(np_config, np) {
 		ret = nmk_pinctrl_dt_subnode_to_map(pctldev, np, map,
-				&reserved_maps, num_maps);
+						    &reserved_maps, num_maps);
 		if (ret < 0) {
 			pinctrl_utils_free_map(pctldev, *map, *num_maps);
 			of_node_put(np);
@@ -920,7 +919,8 @@ static int nmk_pmx_set(struct pinctrl_dev *pctldev, unsigned int function,
 				g->grp.pins[i], g->grp.name, i);
 			goto out_glitch;
 		}
-		dev_dbg(npct->dev, "setting pin %d to altsetting %d\n", g->grp.pins[i], g->altsetting);
+		dev_dbg(npct->dev, "setting pin %d to altsetting %d\n",
+			g->grp.pins[i], g->altsetting);
 
 		clk_enable(nmk_chip->clk);
 		bit = g->grp.pins[i] % NMK_GPIO_PER_CHIP;
@@ -934,7 +934,7 @@ static int nmk_pmx_set(struct pinctrl_dev *pctldev, unsigned int function,
 		nmk_gpio_disable_lazy_irq(nmk_chip, bit);
 
 		__nmk_gpio_set_mode_safe(nmk_chip, bit,
-			(g->altsetting & NMK_GPIO_ALT_C), glitch);
+					 (g->altsetting & NMK_GPIO_ALT_C), glitch);
 		clk_disable(nmk_chip->clk);
 
 		/*
@@ -947,7 +947,7 @@ static int nmk_pmx_set(struct pinctrl_dev *pctldev, unsigned int function,
 		 */
 		if ((g->altsetting & NMK_GPIO_ALT_C) == NMK_GPIO_ALT_C)
 			nmk_prcm_altcx_set_mode(npct, g->grp.pins[i],
-				g->altsetting >> NMK_GPIO_ALT_CX_SHIFT);
+						g->altsetting >> NMK_GPIO_ALT_CX_SHIFT);
 	}
 
 	/* When all pins are successfully reconfigured we get here */
@@ -1219,8 +1219,7 @@ static int nmk_pinctrl_probe(struct platform_device *pdev)
 	if (!npct->prcm_base) {
 		if (version == PINCTRL_NMK_STN8815) {
 			dev_info(&pdev->dev,
-				 "No PRCM base, "
-				 "assuming no ALT-Cx control is available\n");
+				 "No PRCM base, assuming no ALT-Cx control is available\n");
 		} else {
 			dev_err(&pdev->dev, "missing PRCM base address\n");
 			return -EINVAL;

-- 
2.43.1

[PATCH 11/23] pinctrl: nomadik: follow conditional kernel coding conventions

[Théo Lebrun]

Fix strict checkpatch warnings relative to if-else blocks and bool
expressions. Message types addressed:

   CHECK: Comparison to NULL could be written "!nmk_cfg_params[index].choice"
   CHECK: Unbalanced braces around else statement
   CHECK: Using comparison to false is error prone
   CHECK: Using comparison to true is error prone
   CHECK: braces {} should be used on all arms of this statement

Before: 0 errors, 1 warnings, 16 checks.
After:  0 errors, 1 warnings,  7 checks.

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 drivers/pinctrl/nomadik/pinctrl-nomadik.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/pinctrl/nomadik/pinctrl-nomadik.c b/drivers/pinctrl/nomadik/pinctrl-nomadik.c
index 83e7868bc1d7..8099dd947701 100644
--- a/drivers/pinctrl/nomadik/pinctrl-nomadik.c
+++ b/drivers/pinctrl/nomadik/pinctrl-nomadik.c
@@ -377,7 +377,7 @@ static void nmk_prcm_altcx_set_mode(struct nmk_pinctrl *npct,
 	 */
 	if (!alt_num) {
 		for (i = 0 ; i < PRCM_IDX_GPIOCR_ALTC_MAX ; i++) {
-			if (pin_desc->altcx[i].used == true) {
+			if (pin_desc->altcx[i].used) {
 				reg = gpiocr_regs[pin_desc->altcx[i].reg_index];
 				bit = pin_desc->altcx[i].control_bit;
 				if (readl(npct->prcm_base + reg) & BIT(bit)) {
@@ -392,7 +392,7 @@ static void nmk_prcm_altcx_set_mode(struct nmk_pinctrl *npct,
 	}
 
 	alt_index = alt_num - 1;
-	if (pin_desc->altcx[alt_index].used == false) {
+	if (!pin_desc->altcx[alt_index].used) {
 		dev_warn(npct->dev,
 			 "PRCM GPIOCR: pin %i: alternate-C%i does not exist\n",
 			 offset, alt_num);
@@ -406,7 +406,7 @@ static void nmk_prcm_altcx_set_mode(struct nmk_pinctrl *npct,
 	for (i = 0 ; i < PRCM_IDX_GPIOCR_ALTC_MAX ; i++) {
 		if (i == alt_index)
 			continue;
-		if (pin_desc->altcx[i].used == true) {
+		if (pin_desc->altcx[i].used) {
 			reg = gpiocr_regs[pin_desc->altcx[i].reg_index];
 			bit = pin_desc->altcx[i].control_bit;
 			if (readl(npct->prcm_base + reg) & BIT(bit)) {
@@ -494,7 +494,7 @@ int __maybe_unused nmk_prcm_gpiocr_get_mode(struct pinctrl_dev *pctldev, int gpi
 	pin_desc = npct->soc->altcx_pins + i;
 	gpiocr_regs = npct->soc->prcm_gpiocr_registers;
 	for (i = 0; i < PRCM_IDX_GPIOCR_ALTC_MAX; i++) {
-		if (pin_desc->altcx[i].used == true) {
+		if (pin_desc->altcx[i].used) {
 			reg = gpiocr_regs[pin_desc->altcx[i].reg_index];
 			bit = pin_desc->altcx[i].control_bit;
 			if (readl(npct->prcm_base + reg) & BIT(bit))
@@ -676,9 +676,9 @@ static const struct nmk_cfg_param nmk_cfg_params[] = {
 
 static int nmk_dt_pin_config(int index, int val, unsigned long *config)
 {
-	if (nmk_cfg_params[index].choice == NULL)
+	if (!nmk_cfg_params[index].choice) {
 		*config = nmk_cfg_params[index].config;
-	else {
+	} else {
 		/* test if out of range */
 		if  (val < nmk_cfg_params[index].size) {
 			*config = nmk_cfg_params[index].config |
@@ -1102,9 +1102,9 @@ static int nmk_pin_config_set(struct pinctrl_dev *pctldev, unsigned int pin,
 		if (gpiomode)
 			/* No glitch when going to GPIO mode */
 			__nmk_gpio_set_mode(nmk_chip, bit, NMK_GPIO_ALT_GPIO);
-		if (output)
+		if (output) {
 			__nmk_gpio_make_output(nmk_chip, bit, val);
-		else {
+		} else {
 			__nmk_gpio_make_input(nmk_chip, bit);
 			__nmk_gpio_set_pull(nmk_chip, bit, pull);
 		}

-- 
2.43.1

[PATCH 12/23] gpio: nomadik: request dynamic ID allocation

[Théo Lebrun]

Move away from statically allocated GPIO IDs. Switch to dynamic ID
allocation. Static IDs are deprecated because they cause issues when
multiple GPIO controllers are to be found on the same platform.

Add a bit of complexity to do pin number -> GPIO chip + offset.
Previously, bank number and offsets were retrieved using division and
remainder (bank size being constant 32). Now, to get the pin number
matching a bank base, we must know the sum of ngpios of previous banks.
This is done in find_nmk_gpio_from_pin() which also exposes the offset
inside the bank.

Also remove the assumption that bank sizes are constant. Instead of
using NMK_GPIO_PER_CHIP as bank size, use nmk_gpio_chips[i]->ngpio.

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 drivers/gpio/gpio-nomadik.c               |  2 +-
 drivers/pinctrl/nomadik/pinctrl-nomadik.c | 55 +++++++++++++++++++++----------
 2 files changed, 38 insertions(+), 19 deletions(-)

diff --git a/drivers/gpio/gpio-nomadik.c b/drivers/gpio/gpio-nomadik.c
index e39477e1a58f..8d47aef8d2c4 100644
--- a/drivers/gpio/gpio-nomadik.c
+++ b/drivers/gpio/gpio-nomadik.c
@@ -516,7 +516,7 @@ struct nmk_gpio_chip *nmk_gpio_populate_chip(struct device_node *np,
 
 	nmk_chip->bank = id;
 	chip = &nmk_chip->chip;
-	chip->base = id * NMK_GPIO_PER_CHIP;
+	chip->base = -1;
 	chip->ngpio = NMK_GPIO_PER_CHIP;
 	chip->label = dev_name(&gpio_pdev->dev);
 	chip->parent = &gpio_pdev->dev;
diff --git a/drivers/pinctrl/nomadik/pinctrl-nomadik.c b/drivers/pinctrl/nomadik/pinctrl-nomadik.c
index 8099dd947701..c7693fbc0484 100644
--- a/drivers/pinctrl/nomadik/pinctrl-nomadik.c
+++ b/drivers/pinctrl/nomadik/pinctrl-nomadik.c
@@ -530,25 +530,33 @@ static int nmk_get_group_pins(struct pinctrl_dev *pctldev, unsigned int selector
 	return 0;
 }
 
-static struct nmk_gpio_chip *find_nmk_gpio_from_pin(unsigned int pin)
+/* This makes the mapping from pin number to a GPIO chip. We also return the pin
+ * offset in the GPIO chip for convenience (and to avoid a second loop).
+ */
+static struct nmk_gpio_chip *find_nmk_gpio_from_pin(unsigned int pin,
+						    unsigned int *offset)
 {
-	int i;
+	int i, j = 0;
 	struct nmk_gpio_chip *nmk_gpio;
 
+	/* We assume that pins are allocated in bank order. */
 	for (i = 0; i < NMK_MAX_BANKS; i++) {
 		nmk_gpio = nmk_gpio_chips[i];
 		if (!nmk_gpio)
 			continue;
-		if (pin >= nmk_gpio->chip.base &&
-		    pin < nmk_gpio->chip.base + nmk_gpio->chip.ngpio)
+		if (pin >= j && pin < j + nmk_gpio->chip.ngpio) {
+			if (offset)
+				*offset = pin - j;
 			return nmk_gpio;
+		}
+		j += nmk_gpio->chip.ngpio;
 	}
 	return NULL;
 }
 
 static struct gpio_chip *find_gc_from_pin(unsigned int pin)
 {
-	struct nmk_gpio_chip *nmk_gpio = find_nmk_gpio_from_pin(pin);
+	struct nmk_gpio_chip *nmk_gpio = find_nmk_gpio_from_pin(pin, NULL);
 
 	if (nmk_gpio)
 		return &nmk_gpio->chip;
@@ -903,8 +911,19 @@ static int nmk_pmx_set(struct pinctrl_dev *pctldev, unsigned int function,
 		 * Then mask the pins that need to be sleeping now when we're
 		 * switching to the ALT C function.
 		 */
-		for (i = 0; i < g->grp.npins; i++)
-			slpm[g->grp.pins[i] / NMK_GPIO_PER_CHIP] &= ~BIT(g->grp.pins[i]);
+		for (i = 0; i < g->grp.npins; i++) {
+			struct nmk_gpio_chip *nmk_chip;
+
+			nmk_chip = find_nmk_gpio_from_pin(g->grp.pins[i], NULL);
+			if (!nmk_chip) {
+				dev_err(npct->dev,
+					"invalid pin offset %d in group %s at index %d\n",
+					g->grp.pins[i], g->grp.name, i);
+				goto out_pre_slpm_init;
+			}
+
+			slpm[nmk_chip->bank] &= ~BIT(g->grp.pins[i]);
+		}
 		nmk_gpio_glitch_slpm_init(slpm);
 	}
 
@@ -912,7 +931,7 @@ static int nmk_pmx_set(struct pinctrl_dev *pctldev, unsigned int function,
 		struct nmk_gpio_chip *nmk_chip;
 		unsigned int bit;
 
-		nmk_chip = find_nmk_gpio_from_pin(g->grp.pins[i]);
+		nmk_chip = find_nmk_gpio_from_pin(g->grp.pins[i], &bit);
 		if (!nmk_chip) {
 			dev_err(npct->dev,
 				"invalid pin offset %d in group %s at index %d\n",
@@ -923,7 +942,6 @@ static int nmk_pmx_set(struct pinctrl_dev *pctldev, unsigned int function,
 			g->grp.pins[i], g->altsetting);
 
 		clk_enable(nmk_chip->clk);
-		bit = g->grp.pins[i] % NMK_GPIO_PER_CHIP;
 		/*
 		 * If the pin is switching to altfunc, and there was an
 		 * interrupt installed on it which has been lazy disabled,
@@ -954,17 +972,18 @@ static int nmk_pmx_set(struct pinctrl_dev *pctldev, unsigned int function,
 	ret = 0;
 
 out_glitch:
-	if (glitch) {
+	if (glitch)
 		nmk_gpio_glitch_slpm_restore(slpm);
+out_pre_slpm_init:
+	if (glitch)
 		spin_unlock_irqrestore(&nmk_gpio_slpm_lock, flags);
-	}
 
 	return ret;
 }
 
 static int nmk_gpio_request_enable(struct pinctrl_dev *pctldev,
 				   struct pinctrl_gpio_range *range,
-				   unsigned int offset)
+				   unsigned int pin)
 {
 	struct nmk_pinctrl *npct = pinctrl_dev_get_drvdata(pctldev);
 	struct nmk_gpio_chip *nmk_chip;
@@ -982,10 +1001,11 @@ static int nmk_gpio_request_enable(struct pinctrl_dev *pctldev,
 	chip = range->gc;
 	nmk_chip = gpiochip_get_data(chip);
 
-	dev_dbg(npct->dev, "enable pin %u as GPIO\n", offset);
+	dev_dbg(npct->dev, "enable pin %u as GPIO\n", pin);
+
+	find_nmk_gpio_from_pin(pin, &bit);
 
 	clk_enable(nmk_chip->clk);
-	bit = offset % NMK_GPIO_PER_CHIP;
 	/* There is no glitch when converting any pin to GPIO */
 	__nmk_gpio_set_mode(nmk_chip, bit, NMK_GPIO_ALT_GPIO);
 	clk_disable(nmk_chip->clk);
@@ -995,11 +1015,11 @@ static int nmk_gpio_request_enable(struct pinctrl_dev *pctldev,
 
 static void nmk_gpio_disable_free(struct pinctrl_dev *pctldev,
 				  struct pinctrl_gpio_range *range,
-				  unsigned int offset)
+				  unsigned int pin)
 {
 	struct nmk_pinctrl *npct = pinctrl_dev_get_drvdata(pctldev);
 
-	dev_dbg(npct->dev, "disable pin %u as GPIO\n", offset);
+	dev_dbg(npct->dev, "disable pin %u as GPIO\n", pin);
 	/* Set the pin to some default state, GPIO is usually default */
 }
 
@@ -1040,7 +1060,7 @@ static int nmk_pin_config_set(struct pinctrl_dev *pctldev, unsigned int pin,
 	int pull, slpm, output, val, i;
 	bool lowemi, gpiomode, sleep;
 
-	nmk_chip = find_nmk_gpio_from_pin(pin);
+	nmk_chip = find_nmk_gpio_from_pin(pin, &bit);
 	if (!nmk_chip) {
 		dev_err(npct->dev,
 			"invalid pin offset %d\n", pin);
@@ -1098,7 +1118,6 @@ static int nmk_pin_config_set(struct pinctrl_dev *pctldev, unsigned int pin,
 			lowemi ? "on" : "off");
 
 		clk_enable(nmk_chip->clk);
-		bit = pin % NMK_GPIO_PER_CHIP;
 		if (gpiomode)
 			/* No glitch when going to GPIO mode */
 			__nmk_gpio_set_mode(nmk_chip, bit, NMK_GPIO_ALT_GPIO);

-- 
2.43.1

[PATCH 13/23] gpio: nomadik: fix offset bug in nmk_pmx_set()

[Théo Lebrun]

Previously, the statement looked like:

    slpm[x] &= ~BIT(g->pins[i]);

Where:
 - slpm is a unsigned int pointer;
 - g->pins[i] is a pin number which can grow to more than 32.

The expected shift amount is a pin bank offset.

This bug does not occur on every group or pin: the altsetting must be
NMK_GPIO_ALT_C and the pin must be 32 or above. It is possible that it
occurred. For example, in pinctrl-nomadik-db8500.c, pin group i2c3_c_2
has the right altsetting and has pins 229 and 230.

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 drivers/pinctrl/nomadik/pinctrl-nomadik.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/pinctrl/nomadik/pinctrl-nomadik.c b/drivers/pinctrl/nomadik/pinctrl-nomadik.c
index c7693fbc0484..99bdb25f358d 100644
--- a/drivers/pinctrl/nomadik/pinctrl-nomadik.c
+++ b/drivers/pinctrl/nomadik/pinctrl-nomadik.c
@@ -913,8 +913,9 @@ static int nmk_pmx_set(struct pinctrl_dev *pctldev, unsigned int function,
 		 */
 		for (i = 0; i < g->grp.npins; i++) {
 			struct nmk_gpio_chip *nmk_chip;
+			unsigned int bit;
 
-			nmk_chip = find_nmk_gpio_from_pin(g->grp.pins[i], NULL);
+			nmk_chip = find_nmk_gpio_from_pin(g->grp.pins[i], &bit);
 			if (!nmk_chip) {
 				dev_err(npct->dev,
 					"invalid pin offset %d in group %s at index %d\n",
@@ -922,7 +923,7 @@ static int nmk_pmx_set(struct pinctrl_dev *pctldev, unsigned int function,
 				goto out_pre_slpm_init;
 			}
 
-			slpm[nmk_chip->bank] &= ~BIT(g->grp.pins[i]);
+			slpm[nmk_chip->bank] &= ~BIT(bit);
 		}
 		nmk_gpio_glitch_slpm_init(slpm);
 	}

-- 
2.43.1

Re: [PATCH 13/23] gpio: nomadik: fix offset bug in nmk_pmx_set()

[Bartosz Golaszewski]

On Wed, Feb 14, 2024 at 5:24 PM Théo Lebrun <theo.lebrun@bootlin.com> wrote:
>
> Previously, the statement looked like:
>
>     slpm[x] &= ~BIT(g->pins[i]);
>
> Where:
>  - slpm is a unsigned int pointer;
>  - g->pins[i] is a pin number which can grow to more than 32.
>
> The expected shift amount is a pin bank offset.
>
> This bug does not occur on every group or pin: the altsetting must be
> NMK_GPIO_ALT_C and the pin must be 32 or above. It is possible that it
> occurred. For example, in pinctrl-nomadik-db8500.c, pin group i2c3_c_2
> has the right altsetting and has pins 229 and 230.
>
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>

Maybe add a Fixes: tag and put it at the beginning of the series so
that it can go upstream earlier as a fix?

Bart

> ---
>  drivers/pinctrl/nomadik/pinctrl-nomadik.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pinctrl/nomadik/pinctrl-nomadik.c b/drivers/pinctrl/nomadik/pinctrl-nomadik.c
> index c7693fbc0484..99bdb25f358d 100644
> --- a/drivers/pinctrl/nomadik/pinctrl-nomadik.c
> +++ b/drivers/pinctrl/nomadik/pinctrl-nomadik.c
> @@ -913,8 +913,9 @@ static int nmk_pmx_set(struct pinctrl_dev *pctldev, unsigned int function,
>                  */
>                 for (i = 0; i < g->grp.npins; i++) {
>                         struct nmk_gpio_chip *nmk_chip;
> +                       unsigned int bit;
>
> -                       nmk_chip = find_nmk_gpio_from_pin(g->grp.pins[i], NULL);
> +                       nmk_chip = find_nmk_gpio_from_pin(g->grp.pins[i], &bit);
>                         if (!nmk_chip) {
>                                 dev_err(npct->dev,
>                                         "invalid pin offset %d in group %s at index %d\n",
> @@ -922,7 +923,7 @@ static int nmk_pmx_set(struct pinctrl_dev *pctldev, unsigned int function,
>                                 goto out_pre_slpm_init;
>                         }
>
> -                       slpm[nmk_chip->bank] &= ~BIT(g->grp.pins[i]);
> +                       slpm[nmk_chip->bank] &= ~BIT(bit);
>                 }
>                 nmk_gpio_glitch_slpm_init(slpm);
>         }
>
> --
> 2.43.1
>

Re: [PATCH 13/23] gpio: nomadik: fix offset bug in nmk_pmx_set()

[Théo Lebrun]

Hello,

On Mon Feb 19, 2024 at 4:54 PM CET, Bartosz Golaszewski wrote:
> On Wed, Feb 14, 2024 at 5:24 PM Théo Lebrun <theo.lebrun@bootlin.com> wrote:
> >
> > Previously, the statement looked like:
> >
> >     slpm[x] &= ~BIT(g->pins[i]);
> >
> > Where:
> >  - slpm is a unsigned int pointer;
> >  - g->pins[i] is a pin number which can grow to more than 32.
> >
> > The expected shift amount is a pin bank offset.
> >
> > This bug does not occur on every group or pin: the altsetting must be
> > NMK_GPIO_ALT_C and the pin must be 32 or above. It is possible that it
> > occurred. For example, in pinctrl-nomadik-db8500.c, pin group i2c3_c_2
> > has the right altsetting and has pins 229 and 230.
> >
> > Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
>
> Maybe add a Fixes: tag and put it at the beginning of the series so
> that it can go upstream earlier as a fix?

I'll see how it works out because the fix depends on helpers added in
this series. I'll be reworking that.

Regards,

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH 13/23] gpio: nomadik: fix offset bug in nmk_pmx_set()

[Linus Walleij]

On Wed, Feb 14, 2024 at 5:24 PM Théo Lebrun <theo.lebrun@bootlin.com> wrote:

> Previously, the statement looked like:
>
>     slpm[x] &= ~BIT(g->pins[i]);
>
> Where:
>  - slpm is a unsigned int pointer;
>  - g->pins[i] is a pin number which can grow to more than 32.
>
> The expected shift amount is a pin bank offset.
>
> This bug does not occur on every group or pin: the altsetting must be
> NMK_GPIO_ALT_C and the pin must be 32 or above. It is possible that it
> occurred. For example, in pinctrl-nomadik-db8500.c, pin group i2c3_c_2
> has the right altsetting and has pins 229 and 230.
>
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>

Ah good catch!
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

I this something I could just apply as a fix or are there
dependencies on other patches?

Yours,
Linus Walleij

Re: [PATCH 13/23] gpio: nomadik: fix offset bug in nmk_pmx_set()

[Théo Lebrun]

Hello,

On Mon Feb 19, 2024 at 10:56 PM CET, Linus Walleij wrote:
> On Wed, Feb 14, 2024 at 5:24 PM Théo Lebrun <theo.lebrun@bootlin.com> wrote:
>
> > Previously, the statement looked like:
> >
> >     slpm[x] &= ~BIT(g->pins[i]);
> >
> > Where:
> >  - slpm is a unsigned int pointer;
> >  - g->pins[i] is a pin number which can grow to more than 32.
> >
> > The expected shift amount is a pin bank offset.
> >
> > This bug does not occur on every group or pin: the altsetting must be
> > NMK_GPIO_ALT_C and the pin must be 32 or above. It is possible that it
> > occurred. For example, in pinctrl-nomadik-db8500.c, pin group i2c3_c_2
> > has the right altsetting and has pins 229 and 230.
> >
> > Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
>
> Ah good catch!
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>
> I this something I could just apply as a fix or are there
> dependencies on other patches?

There are dependencies. As Bartosz and you asked, I'll try my best to
avoid them and move this patch up in the series, with an appropriate
Fixes: trailer (to the initial driver commit I believe).

Thanks,

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[PATCH 14/23] gpio: nomadik: make clock optional

[Théo Lebrun]

Not all platforms using this platform driver expose a clock for this
GPIO controller. Turn clk_get() into clk_get_optional() to avoid
failing when no clocks are provided.

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 drivers/gpio/gpio-nomadik.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpio/gpio-nomadik.c b/drivers/gpio/gpio-nomadik.c
index 8d47aef8d2c4..2e7d3840b714 100644
--- a/drivers/gpio/gpio-nomadik.c
+++ b/drivers/gpio/gpio-nomadik.c
@@ -529,7 +529,7 @@ struct nmk_gpio_chip *nmk_gpio_populate_chip(struct device_node *np,
 	}
 	nmk_chip->addr = base;
 
-	clk = clk_get(&gpio_pdev->dev, NULL);
+	clk = clk_get_optional(&gpio_pdev->dev, NULL);
 	if (IS_ERR(clk)) {
 		platform_device_put(gpio_pdev);
 		return (void *)clk;

-- 
2.43.1

[PATCH 15/23] gpio: nomadik: change driver name from gpio to gpio-nomadik

[Théo Lebrun]

This GPIO driver is historically related to the Nomadik platform. It
however can be used by others as it implements the ST STA2X11 IP block.
Pick a less ambiguous name for it.

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 drivers/gpio/gpio-nomadik.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpio/gpio-nomadik.c b/drivers/gpio/gpio-nomadik.c
index 2e7d3840b714..25c8490aa1b6 100644
--- a/drivers/gpio/gpio-nomadik.c
+++ b/drivers/gpio/gpio-nomadik.c
@@ -647,7 +647,7 @@ static const struct of_device_id nmk_gpio_match[] = {
 
 static struct platform_driver nmk_gpio_driver = {
 	.driver = {
-		.name = "gpio",
+		.name = "nomadik-gpio",
 		.of_match_table = nmk_gpio_match,
 	},
 	.probe = nmk_gpio_probe,

-- 
2.43.1

[PATCH 16/23] gpio: nomadik: support shared GPIO IRQs

[Théo Lebrun]

Support a single IRQs used by multiple GPIO banks. Change the IRQ
handler type from a chained handler (as used by gpiolib
for ->parent_handler) to a threaded IRQ.

Use a fake raw spinlock to ensure generic_handle_irq() is called in a
no-irq context. See Documentation/driver-api/gpio/driver.rst, "CHAINED
CASCADED GPIO IRQCHIPS" for additional information.

The Mobileye EyeQ5 platform uses this GPIO controller and share an IRQ
for its two banks.

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 drivers/gpio/gpio-nomadik.c       | 67 +++++++++++++++++++++------------------
 include/linux/gpio/gpio-nomadik.h |  1 +
 2 files changed, 38 insertions(+), 30 deletions(-)

diff --git a/drivers/gpio/gpio-nomadik.c b/drivers/gpio/gpio-nomadik.c
index 25c8490aa1b6..5b1e3b3efcff 100644
--- a/drivers/gpio/gpio-nomadik.c
+++ b/drivers/gpio/gpio-nomadik.c
@@ -254,27 +254,31 @@ static void nmk_gpio_irq_shutdown(struct irq_data *d)
 	clk_disable(nmk_chip->clk);
 }
 
-static void nmk_gpio_irq_handler(struct irq_desc *desc)
+static irqreturn_t nmk_gpio_irq_handler(int irq, void *dev_id)
 {
-	struct irq_chip *host_chip = irq_desc_get_chip(desc);
-	struct gpio_chip *chip = irq_desc_get_handler_data(desc);
-	struct nmk_gpio_chip *nmk_chip = gpiochip_get_data(chip);
-	u32 status;
-
-	chained_irq_enter(host_chip, desc);
+	struct nmk_gpio_chip *nmk_chip = dev_id;
+	struct gpio_chip *chip = &nmk_chip->chip;
+	unsigned long mask = GENMASK(chip->ngpio - 1, 0);
+	unsigned long flags, status;
+	int bit;
 
 	clk_enable(nmk_chip->clk);
+
 	status = readl(nmk_chip->addr + NMK_GPIO_IS);
-	clk_disable(nmk_chip->clk);
 
-	while (status) {
-		int bit = __ffs(status);
+	/* Ensure we cannot leave pending bits; this should never occur. */
+	if (unlikely(status & ~mask))
+		writel(status & ~mask, nmk_chip->addr + NMK_GPIO_IC);
+
+	clk_disable(nmk_chip->clk);
 
+	for_each_set_bit(bit, &status, chip->ngpio) {
+		raw_spin_lock_irqsave(&nmk_chip->fake_lock, flags);
 		generic_handle_domain_irq(chip->irq.domain, bit);
-		status &= ~BIT(bit);
+		raw_spin_unlock_irqrestore(&nmk_chip->fake_lock, flags);
 	}
 
-	chained_irq_exit(host_chip, desc);
+	return IRQ_RETVAL((status & mask) != 0);
 }
 
 /* I/O Functions */
@@ -566,19 +570,20 @@ static const struct irq_chip nmk_irq_chip = {
 	GPIOCHIP_IRQ_RESOURCE_HELPERS,
 };
 
-static int nmk_gpio_probe(struct platform_device *dev)
+static int nmk_gpio_probe(struct platform_device *pdev)
 {
-	struct device_node *np = dev->dev.of_node;
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
 	struct nmk_gpio_chip *nmk_chip;
-	struct gpio_chip *chip;
 	struct gpio_irq_chip *girq;
 	bool supports_sleepmode;
+	struct gpio_chip *chip;
 	int irq;
 	int ret;
 
-	nmk_chip = nmk_gpio_populate_chip(np, dev);
+	nmk_chip = nmk_gpio_populate_chip(np, pdev);
 	if (IS_ERR(nmk_chip)) {
-		dev_err(&dev->dev, "could not populate nmk chip struct\n");
+		dev_err(dev, "could not populate nmk chip struct\n");
 		return PTR_ERR(nmk_chip);
 	}
 
@@ -586,9 +591,9 @@ static int nmk_gpio_probe(struct platform_device *dev)
 		of_property_read_bool(np, "st,supports-sleepmode");
 
 	/* Correct platform device ID */
-	dev->id = nmk_chip->bank;
+	pdev->id = nmk_chip->bank;
 
-	irq = platform_get_irq(dev, 0);
+	irq = platform_get_irq(pdev, 0);
 	if (irq < 0)
 		return irq;
 
@@ -600,7 +605,7 @@ static int nmk_gpio_probe(struct platform_device *dev)
 	spin_lock_init(&nmk_chip->lock);
 
 	chip = &nmk_chip->chip;
-	chip->parent = &dev->dev;
+	chip->parent = dev;
 	chip->request = gpiochip_generic_request;
 	chip->free = gpiochip_generic_free;
 	chip->get_direction = nmk_gpio_get_dir;
@@ -614,17 +619,19 @@ static int nmk_gpio_probe(struct platform_device *dev)
 
 	girq = &chip->irq;
 	gpio_irq_chip_set_chip(girq, &nmk_irq_chip);
-	girq->parent_handler = nmk_gpio_irq_handler;
-	girq->num_parents = 1;
-	girq->parents = devm_kcalloc(&dev->dev, 1,
-				     sizeof(*girq->parents),
-				     GFP_KERNEL);
-	if (!girq->parents)
-		return -ENOMEM;
-	girq->parents[0] = irq;
+	girq->parent_handler = NULL;
+	girq->num_parents = 0;
+	girq->parents = NULL;
 	girq->default_type = IRQ_TYPE_NONE;
 	girq->handler = handle_edge_irq;
 
+	ret = devm_request_irq(dev, irq, nmk_gpio_irq_handler, IRQF_SHARED,
+			       dev_name(dev), nmk_chip);
+	if (ret) {
+		dev_err(dev, "failed requesting IRQ\n");
+		return ret;
+	}
+
 	clk_enable(nmk_chip->clk);
 	nmk_chip->lowemi = readl_relaxed(nmk_chip->addr + NMK_GPIO_LOWEMI);
 	clk_disable(nmk_chip->clk);
@@ -633,9 +640,9 @@ static int nmk_gpio_probe(struct platform_device *dev)
 	if (ret)
 		return ret;
 
-	platform_set_drvdata(dev, nmk_chip);
+	platform_set_drvdata(pdev, nmk_chip);
 
-	dev_info(&dev->dev, "chip registered\n");
+	dev_info(dev, "chip registered\n");
 
 	return 0;
 }
diff --git a/include/linux/gpio/gpio-nomadik.h b/include/linux/gpio/gpio-nomadik.h
index 0166ddb71f43..8f7bb756ad35 100644
--- a/include/linux/gpio/gpio-nomadik.h
+++ b/include/linux/gpio/gpio-nomadik.h
@@ -56,6 +56,7 @@ struct nmk_gpio_chip {
 	unsigned int bank;
 	void (*set_ioforce)(bool enable);
 	spinlock_t lock;
+	raw_spinlock_t fake_lock;
 	bool sleepmode;
 	/* Keep track of configured edges */
 	u32 edge_rising;

-- 
2.43.1

Re: [PATCH 16/23] gpio: nomadik: support shared GPIO IRQs

[Bartosz Golaszewski]

On Wed, Feb 14, 2024 at 5:24 PM Théo Lebrun <theo.lebrun@bootlin.com> wrote:
>
> Support a single IRQs used by multiple GPIO banks. Change the IRQ
> handler type from a chained handler (as used by gpiolib
> for ->parent_handler) to a threaded IRQ.
>
> Use a fake raw spinlock to ensure generic_handle_irq() is called in a
> no-irq context. See Documentation/driver-api/gpio/driver.rst, "CHAINED
> CASCADED GPIO IRQCHIPS" for additional information.
>

Any reason for not using preempt_disable()?

Bart

[snip]

Re: [PATCH 16/23] gpio: nomadik: support shared GPIO IRQs

[Théo Lebrun]

Hello,

On Mon Feb 19, 2024 at 4:48 PM CET, Bartosz Golaszewski wrote:
> On Wed, Feb 14, 2024 at 5:24 PM Théo Lebrun <theo.lebrun@bootlin.com> wrote:
> >
> > Support a single IRQs used by multiple GPIO banks. Change the IRQ
> > handler type from a chained handler (as used by gpiolib
> > for ->parent_handler) to a threaded IRQ.
> >
> > Use a fake raw spinlock to ensure generic_handle_irq() is called in a
> > no-irq context. See Documentation/driver-api/gpio/driver.rst, "CHAINED
> > CASCADED GPIO IRQCHIPS" for additional information.
> >
>
> Any reason for not using preempt_disable()?

I did what the doc recommended:

> The generic_handle_irq() is expected to be called with IRQ disabled,
> so the IRQ core will complain if it is called from an IRQ handler which is
> forced to a thread. The "fake?" raw lock can be used to work around this
> problem::
>
>   raw_spinlock_t wa_lock;
>   static irqreturn_t omap_gpio_irq_handler(int irq, void *gpiobank)
>       unsigned long wa_lock_flags;
>       raw_spin_lock_irqsave(&bank->wa_lock, wa_lock_flags);
>       generic_handle_irq(irq_find_mapping(bank->chip.irq.domain, bit));
>       raw_spin_unlock_irqrestore(&bank->wa_lock, wa_lock_flags);

If you confirm I should be using preempt_disable() that's what I'll do
in the next revision. I could even throw in a documentation patch if
the advice is outdated.

Thanks Bartosz,

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH 16/23] gpio: nomadik: support shared GPIO IRQs

[Bartosz Golaszewski]

On Mon, Feb 19, 2024 at 4:54 PM Théo Lebrun <theo.lebrun@bootlin.com> wrote:
>
> Hello,
>
> On Mon Feb 19, 2024 at 4:48 PM CET, Bartosz Golaszewski wrote:
> > On Wed, Feb 14, 2024 at 5:24 PM Théo Lebrun <theo.lebrun@bootlin.com> wrote:
> > >
> > > Support a single IRQs used by multiple GPIO banks. Change the IRQ
> > > handler type from a chained handler (as used by gpiolib
> > > for ->parent_handler) to a threaded IRQ.
> > >
> > > Use a fake raw spinlock to ensure generic_handle_irq() is called in a
> > > no-irq context. See Documentation/driver-api/gpio/driver.rst, "CHAINED
> > > CASCADED GPIO IRQCHIPS" for additional information.
> > >
> >
> > Any reason for not using preempt_disable()?
>
> I did what the doc recommended:
>
> > The generic_handle_irq() is expected to be called with IRQ disabled,
> > so the IRQ core will complain if it is called from an IRQ handler which is
> > forced to a thread. The "fake?" raw lock can be used to work around this
> > problem::
> >
> >   raw_spinlock_t wa_lock;
> >   static irqreturn_t omap_gpio_irq_handler(int irq, void *gpiobank)
> >       unsigned long wa_lock_flags;
> >       raw_spin_lock_irqsave(&bank->wa_lock, wa_lock_flags);
> >       generic_handle_irq(irq_find_mapping(bank->chip.irq.domain, bit));
> >       raw_spin_unlock_irqrestore(&bank->wa_lock, wa_lock_flags);
>
> If you confirm I should be using preempt_disable() that's what I'll do
> in the next revision. I could even throw in a documentation patch if
> the advice is outdated.
>
> Thanks Bartosz,

This was added 9 years ago:

commit c307b002548590c5d8c32b964831de671ad4affe
Author: Grygorii Strashko <grygorii.strashko@ti.com>
Date:   Tue Oct 20 17:22:15 2015 +0300

    gpio: add a real time compliance notes

    Put in a compliance checklist.

    Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
    Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

I'm Cc'ing Grygorii - maybe he can remember if there was any reason
for using a spinlock over preempt_disable(). But for now I'd go with
the latter.

Bart

>
> --
> Théo Lebrun, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com

Re: [PATCH 16/23] gpio: nomadik: support shared GPIO IRQs

[Linus Walleij]

On Mon, Feb 19, 2024 at 4:48 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> On Wed, Feb 14, 2024 at 5:24 PM Théo Lebrun <theo.lebrun@bootlin.com> wrote:
> >
> > Support a single IRQs used by multiple GPIO banks. Change the IRQ
> > handler type from a chained handler (as used by gpiolib
> > for ->parent_handler) to a threaded IRQ.
> >
> > Use a fake raw spinlock to ensure generic_handle_irq() is called in a
> > no-irq context. See Documentation/driver-api/gpio/driver.rst, "CHAINED
> > CASCADED GPIO IRQCHIPS" for additional information.
> >
>
> Any reason for not using preempt_disable()?

I think this needs to be discussed with tglx if Grygorii is not available.

Yours,
Linus Walleij

[PATCH 17/23] gpio: nomadik: handle variadic GPIO count

[Théo Lebrun]

Read the "ngpios" device-tree property to determine the number of GPIOs
for a bank. If not available, fallback to NMK_GPIO_PER_CHIP ie 32 ie
the current behavior.

The IP block always supports 32 GPIOs, but platforms can expose a lesser
amount. The Mobileye EyeQ5 is in this case; one bank is 29 GPIOs and
the other is 23.

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 drivers/gpio/gpio-nomadik.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/gpio/gpio-nomadik.c b/drivers/gpio/gpio-nomadik.c
index 5b1e3b3efcff..02b53c58adf7 100644
--- a/drivers/gpio/gpio-nomadik.c
+++ b/drivers/gpio/gpio-nomadik.c
@@ -490,7 +490,7 @@ struct nmk_gpio_chip *nmk_gpio_populate_chip(struct device_node *np,
 	struct resource *res;
 	struct clk *clk;
 	void __iomem *base;
-	u32 id;
+	u32 id, ngpio;
 
 	gpio_pdev = of_find_device_by_node(np);
 	if (!gpio_pdev) {
@@ -518,10 +518,15 @@ struct nmk_gpio_chip *nmk_gpio_populate_chip(struct device_node *np,
 		return ERR_PTR(-ENOMEM);
 	}
 
+	if (of_property_read_u32(np, "ngpios", &ngpio)) {
+		ngpio = NMK_GPIO_PER_CHIP;
+		dev_dbg(&pdev->dev, "populate: using default ngpio (%d)\n", ngpio);
+	}
+
 	nmk_chip->bank = id;
 	chip = &nmk_chip->chip;
 	chip->base = -1;
-	chip->ngpio = NMK_GPIO_PER_CHIP;
+	chip->ngpio = ngpio;
 	chip->label = dev_name(&gpio_pdev->dev);
 	chip->parent = &gpio_pdev->dev;
 

-- 
2.43.1

Re: [PATCH 17/23] gpio: nomadik: handle variadic GPIO count

[Bartosz Golaszewski]

On Wed, Feb 14, 2024 at 5:24 PM Théo Lebrun <theo.lebrun@bootlin.com> wrote:
>
> Read the "ngpios" device-tree property to determine the number of GPIOs
> for a bank. If not available, fallback to NMK_GPIO_PER_CHIP ie 32 ie
> the current behavior.
>
> The IP block always supports 32 GPIOs, but platforms can expose a lesser
> amount. The Mobileye EyeQ5 is in this case; one bank is 29 GPIOs and
> the other is 23.
>
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> ---
>  drivers/gpio/gpio-nomadik.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpio/gpio-nomadik.c b/drivers/gpio/gpio-nomadik.c
> index 5b1e3b3efcff..02b53c58adf7 100644
> --- a/drivers/gpio/gpio-nomadik.c
> +++ b/drivers/gpio/gpio-nomadik.c
> @@ -490,7 +490,7 @@ struct nmk_gpio_chip *nmk_gpio_populate_chip(struct device_node *np,
>         struct resource *res;
>         struct clk *clk;
>         void __iomem *base;
> -       u32 id;
> +       u32 id, ngpio;
>
>         gpio_pdev = of_find_device_by_node(np);
>         if (!gpio_pdev) {
> @@ -518,10 +518,15 @@ struct nmk_gpio_chip *nmk_gpio_populate_chip(struct device_node *np,
>                 return ERR_PTR(-ENOMEM);
>         }
>
> +       if (of_property_read_u32(np, "ngpios", &ngpio)) {

As commented elsewhere - please try to use device_property_read_u32().

Bart

> +               ngpio = NMK_GPIO_PER_CHIP;
> +               dev_dbg(&pdev->dev, "populate: using default ngpio (%d)\n", ngpio);
> +       }
> +
>         nmk_chip->bank = id;
>         chip = &nmk_chip->chip;
>         chip->base = -1;
> -       chip->ngpio = NMK_GPIO_PER_CHIP;
> +       chip->ngpio = ngpio;
>         chip->label = dev_name(&gpio_pdev->dev);
>         chip->parent = &gpio_pdev->dev;
>
>
> --
> 2.43.1
>

Re: [PATCH 17/23] gpio: nomadik: handle variadic GPIO count

[Linus Walleij]

On Wed, Feb 14, 2024 at 5:24 PM Théo Lebrun <theo.lebrun@bootlin.com> wrote:

> Read the "ngpios" device-tree property to determine the number of GPIOs
> for a bank. If not available, fallback to NMK_GPIO_PER_CHIP ie 32 ie
> the current behavior.
>
> The IP block always supports 32 GPIOs, but platforms can expose a lesser
> amount. The Mobileye EyeQ5 is in this case; one bank is 29 GPIOs and
> the other is 23.
>
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>

LGTM, with Bartosz' comments addressed:
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[PATCH 18/23] gpio: nomadik: support mobileye,eyeq5-gpio

[Théo Lebrun]

We create a custom compatible for the STA2X11 IP block as integrated
into the Mobileye EyeQ5 platform. Its wake and alternate functions have
been disabled, we want to avoid touching those registers.

We both do: (1) early return in functions that do not support the
platform, but with warnings, and (2) avoid calling those functions in
the first place.

We ensure that pinctrl-nomadik is not used with this STA2X11 variant.

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 drivers/gpio/Kconfig                      |  5 +--
 drivers/gpio/gpio-nomadik.c               | 52 ++++++++++++++++++++++++++-----
 drivers/pinctrl/nomadik/pinctrl-nomadik.c |  2 ++
 include/linux/gpio/gpio-nomadik.h         |  1 +
 4 files changed, 51 insertions(+), 9 deletions(-)

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index ff83371251c1..fe6112abb73a 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -480,11 +480,12 @@ config GPIO_MXS
 
 config GPIO_NOMADIK
 	bool "Nomadik GPIO driver"
-	depends on ARCH_U8500 || ARCH_NOMADIK || COMPILE_TEST
+	depends on ARCH_U8500 || ARCH_NOMADIK || MACH_EYEQ5 || COMPILE_TEST
 	select OF_GPIO
 	select GPIOLIB_IRQCHIP
 	help
-	  Say yes here to support the Nomadik SoC GPIO block.
+	  Say yes here to support the Nomadik SoC GPIO block. This block is also
+	  used by the Mobileye EyeQ5 SoC.
 
 	  It handles up to 32 GPIOs per bank, that can all be interrupt sources.
 	  It is deeply interconnected with the associated pinctrl driver as GPIO
diff --git a/drivers/gpio/gpio-nomadik.c b/drivers/gpio/gpio-nomadik.c
index 02b53c58adf7..21bb6d6363fc 100644
--- a/drivers/gpio/gpio-nomadik.c
+++ b/drivers/gpio/gpio-nomadik.c
@@ -7,6 +7,12 @@
  * The GPIO chips are shared with pinctrl-nomadik if used; it needs access for
  * pinmuxing functionality and others.
  *
+ * This driver also handles the mobileye,eyeq5-gpio compatible. It is an STA2X11
+ * but with only data, direction and interrupts register active. We want to
+ * avoid touching SLPM, RWIMSC, FWIMSC, AFSLA and AFSLB registers; that is,
+ * wake and alternate function registers. It is NOT compatible with
+ * pinctrl-nomadik.
+ *
  * Copyright (C) 2008,2009 STMicroelectronics
  * Copyright (C) 2009 Alessandro Rubini <rubini@unipv.it>
  *   Rewritten based on work by Prafulla WADASKAR <prafulla.wadaskar@st.com>
@@ -31,11 +37,17 @@
 static DEFINE_SPINLOCK(nmk_gpio_slpm_lock);
 #endif
 
+#define NMK_GPIO_FLAG_QUIRK_MBLY	BIT(0)
+
 void __nmk_gpio_set_slpm(struct nmk_gpio_chip *nmk_chip, unsigned int offset,
 			 enum nmk_gpio_slpm mode)
 {
 	u32 slpm;
 
+	/* We should NOT have been called. */
+	if (WARN_ON(nmk_chip->quirk_mbly))
+		return;
+
 	slpm = readl(nmk_chip->addr + NMK_GPIO_SLPC);
 	if (mode == NMK_GPIO_SLPM_NOCHANGE)
 		slpm |= BIT(offset);
@@ -92,6 +104,9 @@ static void __nmk_gpio_irq_modify(struct nmk_gpio_chip *nmk_chip,
 		rimscval = &nmk_chip->rimsc;
 		fimscval = &nmk_chip->fimsc;
 	} else  {
+		/* We should NOT have been called. */
+		if (WARN_ON(nmk_chip->quirk_mbly))
+			return;
 		rimscreg = NMK_GPIO_RWIMSC;
 		fimscreg = NMK_GPIO_FWIMSC;
 		rimscval = &nmk_chip->rwimsc;
@@ -118,6 +133,10 @@ static void __nmk_gpio_irq_modify(struct nmk_gpio_chip *nmk_chip,
 static void __nmk_gpio_set_wake(struct nmk_gpio_chip *nmk_chip,
 				int offset, bool on)
 {
+	/* We should NOT have been called. */
+	if (WARN_ON(nmk_chip->quirk_mbly))
+		return;
+
 	/*
 	 * Ensure WAKEUP_ENABLE is on.  No need to disable it if wakeup is
 	 * disabled, since setting SLPM to 1 increases power consumption, and
@@ -142,7 +161,7 @@ static void nmk_gpio_irq_maskunmask(struct nmk_gpio_chip *nmk_chip,
 
 	__nmk_gpio_irq_modify(nmk_chip, d->hwirq, NORMAL, enable);
 
-	if (!(nmk_chip->real_wake & BIT(d->hwirq)))
+	if (!nmk_chip->quirk_mbly && !(nmk_chip->real_wake & BIT(d->hwirq)))
 		__nmk_gpio_set_wake(nmk_chip, d->hwirq, enable);
 
 	spin_unlock(&nmk_chip->lock);
@@ -174,6 +193,10 @@ static int nmk_gpio_irq_set_wake(struct irq_data *d, unsigned int on)
 	struct nmk_gpio_chip *nmk_chip = gpiochip_get_data(gc);
 	unsigned long flags;
 
+	/* Handler is registered in all cases. */
+	if (nmk_chip->quirk_mbly)
+		return -ENXIO;
+
 	clk_enable(nmk_chip->clk);
 	spin_lock_irqsave(&nmk_gpio_slpm_lock, flags);
 	spin_lock(&nmk_chip->lock);
@@ -212,7 +235,7 @@ static int nmk_gpio_irq_set_type(struct irq_data *d, unsigned int type)
 	if (enabled)
 		__nmk_gpio_irq_modify(nmk_chip, d->hwirq, NORMAL, false);
 
-	if (enabled || wake)
+	if (!nmk_chip->quirk_mbly && (enabled || wake))
 		__nmk_gpio_irq_modify(nmk_chip, d->hwirq, WAKE, false);
 
 	nmk_chip->edge_rising &= ~BIT(d->hwirq);
@@ -226,7 +249,7 @@ static int nmk_gpio_irq_set_type(struct irq_data *d, unsigned int type)
 	if (enabled)
 		__nmk_gpio_irq_modify(nmk_chip, d->hwirq, NORMAL, true);
 
-	if (enabled || wake)
+	if (!nmk_chip->quirk_mbly && (enabled || wake))
 		__nmk_gpio_irq_modify(nmk_chip, d->hwirq, WAKE, true);
 
 	spin_unlock_irqrestore(&nmk_chip->lock, flags);
@@ -359,6 +382,10 @@ static int nmk_gpio_get_mode(struct nmk_gpio_chip *nmk_chip, int offset)
 {
 	u32 afunc, bfunc;
 
+	/* We don't support modes. */
+	if (nmk_chip->quirk_mbly)
+		return NMK_GPIO_ALT_GPIO;
+
 	clk_enable(nmk_chip->clk);
 
 	afunc = readl(nmk_chip->addr + NMK_GPIO_AFSLA) & BIT(offset);
@@ -490,6 +517,7 @@ struct nmk_gpio_chip *nmk_gpio_populate_chip(struct device_node *np,
 	struct resource *res;
 	struct clk *clk;
 	void __iomem *base;
+	uintptr_t flags;
 	u32 id, ngpio;
 
 	gpio_pdev = of_find_device_by_node(np);
@@ -523,6 +551,8 @@ struct nmk_gpio_chip *nmk_gpio_populate_chip(struct device_node *np,
 		dev_dbg(&pdev->dev, "populate: using default ngpio (%d)\n", ngpio);
 	}
 
+	flags = (uintptr_t)of_device_get_match_data(&gpio_pdev->dev);
+	nmk_chip->quirk_mbly = !!(flags & NMK_GPIO_FLAG_QUIRK_MBLY);
 	nmk_chip->bank = id;
 	chip = &nmk_chip->chip;
 	chip->base = -1;
@@ -637,9 +667,11 @@ static int nmk_gpio_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	clk_enable(nmk_chip->clk);
-	nmk_chip->lowemi = readl_relaxed(nmk_chip->addr + NMK_GPIO_LOWEMI);
-	clk_disable(nmk_chip->clk);
+	if (!nmk_chip->quirk_mbly) {
+		clk_enable(nmk_chip->clk);
+		nmk_chip->lowemi = readl_relaxed(nmk_chip->addr + NMK_GPIO_LOWEMI);
+		clk_disable(nmk_chip->clk);
+	}
 
 	ret = gpiochip_add_data(chip, nmk_chip);
 	if (ret)
@@ -653,7 +685,13 @@ static int nmk_gpio_probe(struct platform_device *pdev)
 }
 
 static const struct of_device_id nmk_gpio_match[] = {
-	{ .compatible = "st,nomadik-gpio", },
+	{
+		.compatible = "st,nomadik-gpio",
+	},
+	{
+		.compatible = "mobileye,eyeq5-gpio",
+		.data = (void*)NMK_GPIO_FLAG_QUIRK_MBLY,
+	},
 	{}
 };
 
diff --git a/drivers/pinctrl/nomadik/pinctrl-nomadik.c b/drivers/pinctrl/nomadik/pinctrl-nomadik.c
index 99bdb25f358d..d1c5f353c6a9 100644
--- a/drivers/pinctrl/nomadik/pinctrl-nomadik.c
+++ b/drivers/pinctrl/nomadik/pinctrl-nomadik.c
@@ -1229,6 +1229,8 @@ static int nmk_pinctrl_probe(struct platform_device *pdev)
 			dev_err(&pdev->dev,
 				"could not populate nmk chip struct - continue anyway\n");
 		of_node_put(gpio_np);
+		/* We are NOT compatible with mobileye,eyeq5-gpio. */
+		BUG_ON(nmk_chip->quirk_mbly);
 	}
 
 	prcm_np = of_parse_phandle(np, "prcm", 0);
diff --git a/include/linux/gpio/gpio-nomadik.h b/include/linux/gpio/gpio-nomadik.h
index 8f7bb756ad35..8d0134dd3771 100644
--- a/include/linux/gpio/gpio-nomadik.h
+++ b/include/linux/gpio/gpio-nomadik.h
@@ -58,6 +58,7 @@ struct nmk_gpio_chip {
 	spinlock_t lock;
 	raw_spinlock_t fake_lock;
 	bool sleepmode;
+	bool quirk_mbly;
 	/* Keep track of configured edges */
 	u32 edge_rising;
 	u32 edge_falling;

-- 
2.43.1

Re: [PATCH 18/23] gpio: nomadik: support mobileye,eyeq5-gpio

[Linus Walleij]

Hi Theo,

thanks for your patch!

On Wed, Feb 14, 2024 at 5:24 PM Théo Lebrun <theo.lebrun@bootlin.com> wrote:

> We create a custom compatible for the STA2X11 IP block as integrated
> into the Mobileye EyeQ5 platform. Its wake and alternate functions have
> been disabled, we want to avoid touching those registers.
>
> We both do: (1) early return in functions that do not support the
> platform, but with warnings, and (2) avoid calling those functions in
> the first place.
>
> We ensure that pinctrl-nomadik is not used with this STA2X11 variant.
>
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
(...)
>+       bool quirk_mbly;

Compulsive abbreviation? I would just rename it:

bool is_mobileye_soc;

Nevermind the long name, it makes it crystal clear for readers
what is going on. (Rusty Russell's API naming guidelines.)

With that changed:
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCH 18/23] gpio: nomadik: support mobileye,eyeq5-gpio

[Théo Lebrun]

Hello,

On Wed Feb 21, 2024 at 2:45 PM CET, Linus Walleij wrote:
> On Wed, Feb 14, 2024 at 5:24 PM Théo Lebrun <theo.lebrun@bootlin.com> wrote:
> > We create a custom compatible for the STA2X11 IP block as integrated
> > into the Mobileye EyeQ5 platform. Its wake and alternate functions have
> > been disabled, we want to avoid touching those registers.
> >
> > We both do: (1) early return in functions that do not support the
> > platform, but with warnings, and (2) avoid calling those functions in
> > the first place.
> >
> > We ensure that pinctrl-nomadik is not used with this STA2X11 variant.
> >
> > Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> (...)
> >+       bool quirk_mbly;
>
> Compulsive abbreviation? I would just rename it:
>
> bool is_mobileye_soc;
>
> Nevermind the long name, it makes it crystal clear for readers
> what is going on. (Rusty Russell's API naming guidelines.)
>
> With that changed:
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Makes complete sense. This is old heritage from my initial prototype
that should have long gone disappeared.

Thanks for your feedback & reviews!

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH 18/23] gpio: nomadik: support mobileye,eyeq5-gpio

[Linus Walleij]

On Wed, Feb 14, 2024 at 5:24 PM Théo Lebrun <theo.lebrun@bootlin.com> wrote:

> We create a custom compatible for the STA2X11 IP block as integrated
> into the Mobileye EyeQ5 platform. Its wake and alternate functions have
> been disabled, we want to avoid touching those registers.
>
> We both do: (1) early return in functions that do not support the
> platform, but with warnings, and (2) avoid calling those functions in
> the first place.
>
> We ensure that pinctrl-nomadik is not used with this STA2X11 variant.
>
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>

When testing I noticed that this patch breaks Ux500 (up until patch 17
all works fine!).

But I don't know why.

Trying to figure it out...

Yours,
Linus Walleij

Re: [PATCH 18/23] gpio: nomadik: support mobileye,eyeq5-gpio

[Théo Lebrun]

Hello,

On Wed Feb 21, 2024 at 3:31 PM CET, Linus Walleij wrote:
> On Wed, Feb 14, 2024 at 5:24 PM Théo Lebrun <theo.lebrun@bootlin.com> wrote:
>
> > We create a custom compatible for the STA2X11 IP block as integrated
> > into the Mobileye EyeQ5 platform. Its wake and alternate functions have
> > been disabled, we want to avoid touching those registers.
> >
> > We both do: (1) early return in functions that do not support the
> > platform, but with warnings, and (2) avoid calling those functions in
> > the first place.
> >
> > We ensure that pinctrl-nomadik is not used with this STA2X11 variant.
> >
> > Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
>
> When testing I noticed that this patch breaks Ux500 (up until patch 17
> all works fine!).
>
> But I don't know why.
>
> Trying to figure it out...

Can I help in the debugging process?

Reading the code once again I'd guess
of_device_get_match_data(&gpio_pdev->dev) could be the root cause. We
are accessing match data for the GPIO device while probing the pinctrl
device. Maybe something isn't initialised properly yet? The rest looks
rather harmless, I've checked all conditional expressions.

Thanks,

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH 18/23] gpio: nomadik: support mobileye,eyeq5-gpio

[Linus Walleij]

On Wed, Feb 21, 2024 at 5:16 PM Théo Lebrun <theo.lebrun@bootlin.com> wrote:

> > Trying to figure it out...
>
> Can I help in the debugging process?

Nah, I found it :)

> Reading the code once again I'd guess
> of_device_get_match_data(&gpio_pdev->dev) could be the root cause. We
> are accessing match data for the GPIO device while probing the pinctrl
> device. Maybe something isn't initialised properly yet? The rest looks
> rather harmless, I've checked all conditional expressions.

Yep spot on. The nmk_gpio_populate_chip() is sometimes called from
the pinctrl driver before the gpio probe() has been called, so the match
data is NULL and we crash.

This looks like it does for historical reasons and there could be better
ways to fix it now that Saravana Kannan has fixed up the probe ordering
code.

The following is one way to fix it for now using device_is_compatible()
(illustrating some other changes I did as well):

diff --git a/drivers/gpio/gpio-nomadik.c b/drivers/gpio/gpio-nomadik.c
index 21bb6d6363fc..11071a982ebb 100644
--- a/drivers/gpio/gpio-nomadik.c
+++ b/drivers/gpio/gpio-nomadik.c
@@ -27,6 +27,7 @@
 #include <linux/of_platform.h>
 #include <linux/pinctrl/pinctrl.h>
 #include <linux/platform_device.h>
+#include <linux/property.h>
 #include <linux/reset.h>
 #include <linux/seq_file.h>
 #include <linux/types.h>
@@ -37,15 +38,13 @@
 static DEFINE_SPINLOCK(nmk_gpio_slpm_lock);
 #endif

-#define NMK_GPIO_FLAG_QUIRK_MBLY    BIT(0)
-
 void __nmk_gpio_set_slpm(struct nmk_gpio_chip *nmk_chip, unsigned int offset,
              enum nmk_gpio_slpm mode)
 {
     u32 slpm;

     /* We should NOT have been called. */
-    if (WARN_ON(nmk_chip->quirk_mbly))
+    if (WARN_ON(nmk_chip->is_mobileye_soc))
         return;

     slpm = readl(nmk_chip->addr + NMK_GPIO_SLPC);
@@ -105,7 +104,7 @@ static void __nmk_gpio_irq_modify(struct
nmk_gpio_chip *nmk_chip,
         fimscval = &nmk_chip->fimsc;
     } else  {
         /* We should NOT have been called. */
-        if (WARN_ON(nmk_chip->quirk_mbly))
+        if (WARN_ON(nmk_chip->is_mobileye_soc))
             return;
         rimscreg = NMK_GPIO_RWIMSC;
         fimscreg = NMK_GPIO_FWIMSC;
@@ -134,7 +133,7 @@ static void __nmk_gpio_set_wake(struct
nmk_gpio_chip *nmk_chip,
                 int offset, bool on)
 {
     /* We should NOT have been called. */
-    if (WARN_ON(nmk_chip->quirk_mbly))
+    if (WARN_ON(nmk_chip->is_mobileye_soc))
         return;

     /*
@@ -161,7 +160,7 @@ static void nmk_gpio_irq_maskunmask(struct
nmk_gpio_chip *nmk_chip,

     __nmk_gpio_irq_modify(nmk_chip, d->hwirq, NORMAL, enable);

-    if (!nmk_chip->quirk_mbly && !(nmk_chip->real_wake & BIT(d->hwirq)))
+    if (!nmk_chip->is_mobileye_soc && !(nmk_chip->real_wake & BIT(d->hwirq)))
         __nmk_gpio_set_wake(nmk_chip, d->hwirq, enable);

     spin_unlock(&nmk_chip->lock);
@@ -194,7 +193,7 @@ static int nmk_gpio_irq_set_wake(struct irq_data
*d, unsigned int on)
     unsigned long flags;

     /* Handler is registered in all cases. */
-    if (nmk_chip->quirk_mbly)
+    if (nmk_chip->is_mobileye_soc)
         return -ENXIO;

     clk_enable(nmk_chip->clk);
@@ -235,7 +234,7 @@ static int nmk_gpio_irq_set_type(struct irq_data
*d, unsigned int type)
     if (enabled)
         __nmk_gpio_irq_modify(nmk_chip, d->hwirq, NORMAL, false);

-    if (!nmk_chip->quirk_mbly && (enabled || wake))
+    if (!nmk_chip->is_mobileye_soc && (enabled || wake))
         __nmk_gpio_irq_modify(nmk_chip, d->hwirq, WAKE, false);

     nmk_chip->edge_rising &= ~BIT(d->hwirq);
@@ -249,7 +248,7 @@ static int nmk_gpio_irq_set_type(struct irq_data
*d, unsigned int type)
     if (enabled)
         __nmk_gpio_irq_modify(nmk_chip, d->hwirq, NORMAL, true);

-    if (!nmk_chip->quirk_mbly && (enabled || wake))
+    if (!nmk_chip->is_mobileye_soc && (enabled || wake))
         __nmk_gpio_irq_modify(nmk_chip, d->hwirq, WAKE, true);

     spin_unlock_irqrestore(&nmk_chip->lock, flags);
@@ -383,7 +382,7 @@ static int nmk_gpio_get_mode(struct nmk_gpio_chip
*nmk_chip, int offset)
     u32 afunc, bfunc;

     /* We don't support modes. */
-    if (nmk_chip->quirk_mbly)
+    if (nmk_chip->is_mobileye_soc)
         return NMK_GPIO_ALT_GPIO;

     clk_enable(nmk_chip->clk);
@@ -517,7 +516,6 @@ struct nmk_gpio_chip
*nmk_gpio_populate_chip(struct device_node *np,
     struct resource *res;
     struct clk *clk;
     void __iomem *base;
-    uintptr_t flags;
     u32 id, ngpio;

     gpio_pdev = of_find_device_by_node(np);
@@ -551,8 +549,7 @@ struct nmk_gpio_chip
*nmk_gpio_populate_chip(struct device_node *np,
         dev_dbg(&pdev->dev, "populate: using default ngpio (%d)\n", ngpio);
     }

-    flags = (uintptr_t)of_device_get_match_data(&gpio_pdev->dev);
-    nmk_chip->quirk_mbly = !!(flags & NMK_GPIO_FLAG_QUIRK_MBLY);
+    nmk_chip->is_mobileye_soc = device_is_compatible(&gpio_pdev->dev,
"mobileye,eyeq5-gpio");
     nmk_chip->bank = id;
     chip = &nmk_chip->chip;
     chip->base = -1;
@@ -667,7 +664,7 @@ static int nmk_gpio_probe(struct platform_device *pdev)
         return ret;
     }

-    if (!nmk_chip->quirk_mbly) {
+    if (!nmk_chip->is_mobileye_soc) {
         clk_enable(nmk_chip->clk);
         nmk_chip->lowemi = readl_relaxed(nmk_chip->addr + NMK_GPIO_LOWEMI);
         clk_disable(nmk_chip->clk);
@@ -690,7 +687,6 @@ static const struct of_device_id nmk_gpio_match[] = {
     },
     {
         .compatible = "mobileye,eyeq5-gpio",
-        .data = (void*)NMK_GPIO_FLAG_QUIRK_MBLY,
     },
     {}
 };
diff --git a/include/linux/gpio/gpio-nomadik.h
b/include/linux/gpio/gpio-nomadik.h
index 8d0134dd3771..ede16cdaa920 100644
--- a/include/linux/gpio/gpio-nomadik.h
+++ b/include/linux/gpio/gpio-nomadik.h
@@ -51,6 +51,7 @@ enum nmk_gpio_slpm {

 struct nmk_gpio_chip {
     struct gpio_chip chip;
+    bool is_mobileye_soc;
     void __iomem *addr;
     struct clk *clk;
     unsigned int bank;

Yours,
Linus Walleij

Re: [PATCH 18/23] gpio: nomadik: support mobileye,eyeq5-gpio

[Théo Lebrun]

Hello,

On Wed Feb 21, 2024 at 8:36 PM CET, Linus Walleij wrote:
> On Wed, Feb 21, 2024 at 5:16 PM Théo Lebrun <theo.lebrun@bootlin.com> wrote:
>
> > > Trying to figure it out...
> >
> > Can I help in the debugging process?
>
> Nah, I found it :)
>
> > Reading the code once again I'd guess
> > of_device_get_match_data(&gpio_pdev->dev) could be the root cause. We
> > are accessing match data for the GPIO device while probing the pinctrl
> > device. Maybe something isn't initialised properly yet? The rest looks
> > rather harmless, I've checked all conditional expressions.
>
> Yep spot on. The nmk_gpio_populate_chip() is sometimes called from
> the pinctrl driver before the gpio probe() has been called, so the match
> data is NULL and we crash.
>
> This looks like it does for historical reasons and there could be better
> ways to fix it now that Saravana Kannan has fixed up the probe ordering
> code.
>
> The following is one way to fix it for now using device_is_compatible()
> (illustrating some other changes I did as well):

Thanks for the debugging and the proposed fix. Indeed matching on
compatible to avoid match data for a bool makes sense.

Thanks,

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[PATCH 19/23] gpio: nomadik: grab optional reset control and deassert it at probe

[Théo Lebrun]

Fetch a reference to the optional shared reset control and deassert it
if it exists.

Optional because not all platforms that use this driver have a reset
attached to the reset block. Shared because some platforms that use the
reset (at least Mobileye EyeQ5) share the reset across banks.

Do not keep a reference to the reset control as it is not needed
afterwards; the driver does not handle suspend, does not use runtime PM
and does not register a remove callback.

The operation is done in nmk_gpio_populate_chip(). This function is
called by either gpio-nomadik or pinctrl-nomadik, whoever comes first.
This is here for historic reasons and could probably be removed now; it
seems gpio-ranges enforces the ordering to be pinctrl-first. It is not
the topic of the present patch however.

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 drivers/gpio/gpio-nomadik.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/gpio/gpio-nomadik.c b/drivers/gpio/gpio-nomadik.c
index 21bb6d6363fc..b623c093b54d 100644
--- a/drivers/gpio/gpio-nomadik.c
+++ b/drivers/gpio/gpio-nomadik.c
@@ -513,12 +513,14 @@ struct nmk_gpio_chip *nmk_gpio_populate_chip(struct device_node *np,
 {
 	struct nmk_gpio_chip *nmk_chip;
 	struct platform_device *gpio_pdev;
+	struct reset_control *reset;
 	struct gpio_chip *chip;
 	struct resource *res;
 	struct clk *clk;
 	void __iomem *base;
 	uintptr_t flags;
 	u32 id, ngpio;
+	int ret;
 
 	gpio_pdev = of_find_device_by_node(np);
 	if (!gpio_pdev) {
@@ -576,6 +578,19 @@ struct nmk_gpio_chip *nmk_gpio_populate_chip(struct device_node *np,
 	clk_prepare(clk);
 	nmk_chip->clk = clk;
 
+	reset = devm_reset_control_get_optional_shared(&gpio_pdev->dev, NULL);
+	if (IS_ERR(reset)) {
+		dev_err(&pdev->dev, "failed getting reset control: %ld\n",
+			PTR_ERR(reset));
+		return ERR_CAST(reset);
+	}
+
+	ret = reset_control_deassert(reset);
+	if (ret) {
+		dev_err(&pdev->dev, "failed reset deassert: %d\n", ret);
+		return ERR_PTR(ret);
+	}
+
 #ifdef CONFIG_PINCTRL_NOMADIK
 	BUG_ON(nmk_chip->bank >= ARRAY_SIZE(nmk_gpio_chips));
 	nmk_gpio_chips[id] = nmk_chip;

-- 
2.43.1

Re: [PATCH 19/23] gpio: nomadik: grab optional reset control and deassert it at probe

[Philipp Zabel]

On Mi, 2024-02-14 at 17:24 +0100, Théo Lebrun wrote:
> Fetch a reference to the optional shared reset control and deassert it
> if it exists.
> 
> Optional because not all platforms that use this driver have a reset
> attached to the reset block. Shared because some platforms that use the
> reset (at least Mobileye EyeQ5) share the reset across banks.
> 
> Do not keep a reference to the reset control as it is not needed
> afterwards; the driver does not handle suspend, does not use runtime PM
> and does not register a remove callback.

I suppose you don't care that the reset is only ever deasserted once
and never asserted again on this hardware, but for shared reset
controls the expectation is that deassert/assert calls are balanced:

https://docs.kernel.org/driver-api/reset.html?highlight=balanced#assertion-and-deassertion

So maybe this warrants a comment in the code. Or do you mean to
suppress unbind via suppress_bind_attrs to explain away any missing
cleanup?

> The operation is done in nmk_gpio_populate_chip(). This function is
> called by either gpio-nomadik or pinctrl-nomadik, whoever comes first.
> This is here for historic reasons and could probably be removed now; it
> seems gpio-ranges enforces the ordering to be pinctrl-first. It is not
> the topic of the present patch however.
> 
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> ---
>  drivers/gpio/gpio-nomadik.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/drivers/gpio/gpio-nomadik.c b/drivers/gpio/gpio-nomadik.c
> index 21bb6d6363fc..b623c093b54d 100644
> --- a/drivers/gpio/gpio-nomadik.c
> +++ b/drivers/gpio/gpio-nomadik.c
> @@ -513,12 +513,14 @@ struct nmk_gpio_chip *nmk_gpio_populate_chip(struct device_node *np,
>  {
>  	struct nmk_gpio_chip *nmk_chip;
>  	struct platform_device *gpio_pdev;
> +	struct reset_control *reset;
>  	struct gpio_chip *chip;
>  	struct resource *res;
>  	struct clk *clk;
>  	void __iomem *base;
>  	uintptr_t flags;
>  	u32 id, ngpio;
> +	int ret;
>  
>  	gpio_pdev = of_find_device_by_node(np);
>  	if (!gpio_pdev) {
> @@ -576,6 +578,19 @@ struct nmk_gpio_chip *nmk_gpio_populate_chip(struct device_node *np,
>  	clk_prepare(clk);
>  	nmk_chip->clk = clk;
>  
> +	reset = devm_reset_control_get_optional_shared(&gpio_pdev->dev, NULL);
> +	if (IS_ERR(reset)) {
> +		dev_err(&pdev->dev, "failed getting reset control: %ld\n",
> +			PTR_ERR(reset));
> +		return ERR_CAST(reset);

Consider using dev_err_probe() here.

regards
Philipp

Re: [PATCH 19/23] gpio: nomadik: grab optional reset control and deassert it at probe

[Théo Lebrun]

Hello,

On Thu Feb 15, 2024 at 11:19 AM CET, Philipp Zabel wrote:
> On Mi, 2024-02-14 at 17:24 +0100, Théo Lebrun wrote:
> > Fetch a reference to the optional shared reset control and deassert it
> > if it exists.
> > 
> > Optional because not all platforms that use this driver have a reset
> > attached to the reset block. Shared because some platforms that use the
> > reset (at least Mobileye EyeQ5) share the reset across banks.
> > 
> > Do not keep a reference to the reset control as it is not needed
> > afterwards; the driver does not handle suspend, does not use runtime PM
> > and does not register a remove callback.
>
> I suppose you don't care that the reset is only ever deasserted once
> and never asserted again on this hardware, but for shared reset
> controls the expectation is that deassert/assert calls are balanced:
>
> https://docs.kernel.org/driver-api/reset.html?highlight=balanced#assertion-and-deassertion
>
> So maybe this warrants a comment in the code. Or do you mean to
> suppress unbind via suppress_bind_attrs to explain away any missing
> cleanup?

Those resets are shared only across GPIO banks which have no reason to
be unbind-able. I'll add a comment and disable unbind.

> > The operation is done in nmk_gpio_populate_chip(). This function is
> > called by either gpio-nomadik or pinctrl-nomadik, whoever comes first.
> > This is here for historic reasons and could probably be removed now; it
> > seems gpio-ranges enforces the ordering to be pinctrl-first. It is not
> > the topic of the present patch however.
> > 
> > Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> > ---
> >  drivers/gpio/gpio-nomadik.c | 15 +++++++++++++++
> >  1 file changed, 15 insertions(+)
> > 
> > diff --git a/drivers/gpio/gpio-nomadik.c b/drivers/gpio/gpio-nomadik.c
> > index 21bb6d6363fc..b623c093b54d 100644
> > --- a/drivers/gpio/gpio-nomadik.c
> > +++ b/drivers/gpio/gpio-nomadik.c
> > @@ -513,12 +513,14 @@ struct nmk_gpio_chip *nmk_gpio_populate_chip(struct device_node *np,
> >  {
> >  	struct nmk_gpio_chip *nmk_chip;
> >  	struct platform_device *gpio_pdev;
> > +	struct reset_control *reset;
> >  	struct gpio_chip *chip;
> >  	struct resource *res;
> >  	struct clk *clk;
> >  	void __iomem *base;
> >  	uintptr_t flags;
> >  	u32 id, ngpio;
> > +	int ret;
> >  
> >  	gpio_pdev = of_find_device_by_node(np);
> >  	if (!gpio_pdev) {
> > @@ -576,6 +578,19 @@ struct nmk_gpio_chip *nmk_gpio_populate_chip(struct device_node *np,
> >  	clk_prepare(clk);
> >  	nmk_chip->clk = clk;
> >  
> > +	reset = devm_reset_control_get_optional_shared(&gpio_pdev->dev, NULL);
> > +	if (IS_ERR(reset)) {
> > +		dev_err(&pdev->dev, "failed getting reset control: %ld\n",
> > +			PTR_ERR(reset));
> > +		return ERR_CAST(reset);
>
> Consider using dev_err_probe() here.

Makes sense, will do so for next revision.

Thanks for the review Philipp,

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[PATCH 20/23] MIPS: eyeq5_defconfig: enable GPIO by default

[Théo Lebrun]

Add both CONFIG_GPIOLIB=y and CONFIG_GPIO_NOMADIK=y to our platform
defconfig. Default config is CONFIG_GPIOLIB=n.

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 arch/mips/configs/eyeq5_defconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/mips/configs/eyeq5_defconfig b/arch/mips/configs/eyeq5_defconfig
index c35c29a4d479..6f052d4c5e58 100644
--- a/arch/mips/configs/eyeq5_defconfig
+++ b/arch/mips/configs/eyeq5_defconfig
@@ -66,6 +66,8 @@ CONFIG_SERIAL_AMBA_PL011_CONSOLE=y
 CONFIG_HW_RANDOM=y
 # CONFIG_PTP_1588_CLOCK is not set
 CONFIG_PINCTRL=y
+CONFIG_GPIOLIB=y
+CONFIG_GPIO_NOMADIK=y
 CONFIG_MFD_SYSCON=y
 CONFIG_HID_A4TECH=y
 CONFIG_HID_BELKIN=y

-- 
2.43.1

[PATCH 21/23] MIPS: mobileye: eyeq5: add two GPIO bank nodes

[Théo Lebrun]

This platform uses the same GPIO controller as Nomadik. It however has
its own pinconf & pinmux controller. EyeQ5 is the first platform to use
gpio-nomadik independently from pinctrl-nomadik.

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 arch/mips/boot/dts/mobileye/eyeq5.dtsi | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/arch/mips/boot/dts/mobileye/eyeq5.dtsi b/arch/mips/boot/dts/mobileye/eyeq5.dtsi
index 8d4f65ec912d..0f18ac73620b 100644
--- a/arch/mips/boot/dts/mobileye/eyeq5.dtsi
+++ b/arch/mips/boot/dts/mobileye/eyeq5.dtsi
@@ -158,6 +158,32 @@ timer {
 				clocks = <&core0_clk>;
 			};
 		};
+
+		gpio0: gpio@1400000 {
+			compatible = "mobileye,eyeq5-gpio";
+			reg = <0x0 0x1400000 0x0 0x1000>;
+			gpio-bank = <0>;
+			ngpios = <29>;
+			interrupt-parent = <&gic>;
+			interrupts = <GIC_SHARED 14 IRQ_TYPE_LEVEL_HIGH>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+		};
+
+		gpio1: gpio@1500000 {
+			compatible = "mobileye,eyeq5-gpio";
+			reg = <0x0 0x1500000 0x0 0x1000>;
+			gpio-bank = <1>;
+			ngpios = <23>;
+			interrupt-parent = <&gic>;
+			interrupts = <GIC_SHARED 14 IRQ_TYPE_LEVEL_HIGH>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+		};
 	};
 };
 

-- 
2.43.1

[PATCH 22/23] MIPS: mobileye: eyeq5: add resets to GPIO banks

[Théo Lebrun]

The two GPIO banks share a single reset line.

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 arch/mips/boot/dts/mobileye/eyeq5.dtsi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/mips/boot/dts/mobileye/eyeq5.dtsi b/arch/mips/boot/dts/mobileye/eyeq5.dtsi
index 0f18ac73620b..5f00d129c057 100644
--- a/arch/mips/boot/dts/mobileye/eyeq5.dtsi
+++ b/arch/mips/boot/dts/mobileye/eyeq5.dtsi
@@ -170,6 +170,7 @@ gpio0: gpio@1400000 {
 			#gpio-cells = <2>;
 			interrupt-controller;
 			#interrupt-cells = <2>;
+			resets = <&reset 0 26>;
 		};
 
 		gpio1: gpio@1500000 {
@@ -183,6 +184,7 @@ gpio1: gpio@1500000 {
 			#gpio-cells = <2>;
 			interrupt-controller;
 			#interrupt-cells = <2>;
+			resets = <&reset 0 26>;
 		};
 	};
 };

-- 
2.43.1

[PATCH 23/23] MIPS: mobileye: eyeq5: map GPIOs to pins using gpio-ranges

[Théo Lebrun]

Create a mapping between the GPIO controllers and the pin controllers.
GPIO is handled in a one-instance-per-bank manner while pinctrl is done
with a single instance for both pin banks.

See gpio-ranges documentation:
Documentation/devicetree/bindings/gpio/gpio.txt

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 arch/mips/boot/dts/mobileye/eyeq5.dtsi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/mips/boot/dts/mobileye/eyeq5.dtsi b/arch/mips/boot/dts/mobileye/eyeq5.dtsi
index 5f00d129c057..68f6c81331d7 100644
--- a/arch/mips/boot/dts/mobileye/eyeq5.dtsi
+++ b/arch/mips/boot/dts/mobileye/eyeq5.dtsi
@@ -168,6 +168,7 @@ gpio0: gpio@1400000 {
 			interrupts = <GIC_SHARED 14 IRQ_TYPE_LEVEL_HIGH>;
 			gpio-controller;
 			#gpio-cells = <2>;
+			gpio-ranges = <&pinctrl 0 0 29>;
 			interrupt-controller;
 			#interrupt-cells = <2>;
 			resets = <&reset 0 26>;
@@ -182,6 +183,7 @@ gpio1: gpio@1500000 {
 			interrupts = <GIC_SHARED 14 IRQ_TYPE_LEVEL_HIGH>;
 			gpio-controller;
 			#gpio-cells = <2>;
+			gpio-ranges = <&pinctrl 0 29 23>;
 			interrupt-controller;
 			#interrupt-cells = <2>;
 			resets = <&reset 0 26>;

-- 
2.43.1

Re: [PATCH 00/23] Rework Nomadik GPIO to add Mobileye EyeQ5 support

[Bartosz Golaszewski]

On Wed, Feb 14, 2024 at 5:24 PM Théo Lebrun <theo.lebrun@bootlin.com> wrote:
>
> Hi,
>
> This patch series reworks the Nomadik GPIO driver to bring it up to date
> to current kernel standards. We then add Mobileye EyeQ5 support that
> uses the same IP block but with limited functionality. We also add
> features required by our newly supported platform:
>
>  - Dynamic GPIO ID allocation;
>  - Make clock optional;
>  - Shared IRQ (usecase: EyeQ5 has two banks using the same IRQ);
>  - Handle variadic GPIO counts (usecase: EyeQ5 has <32 GPIOs per bank);
>  - Grab optional reset at probe (usecase: EyeQ5 has a reset available).
>
> This GPIO platform driver was previously declared & registered inside
> drivers/pinctrl/nomadik/pinctrl-nomadik.c, side-by-side with the
> pinctrl driver. Both are tightly integrated, mostly for muxing reasons.
> Now that gpio-nomadik is used for another platform, we loosen the
> relationship. The behavior should not change on already supported
> hardware but I do not have Nomadik hardware to test for that.
>

I hope Linus can leave his Tested-by under this series then.

> We have some dependencies, kept neatly to the end. Those are:
> - The base platform support series from Grégory [1]. This relates to the
>   last four patches (20 thru 23), ie defconfig and devicetree.
> - The OLB syscon support series [0]. It provides reset and pinctrl nodes
>   inside the devicetree. This relates to the last two patches (22 and
>   23), ie resets and gpio-ranges DT props. GPIO works fine without it
>   if patches 22 and 23 are dropped.
>
> This has been tested on the EyeQ5 hardware, with the two parent series
> applied. It also works fine without the OLB syscon series when our last
> two patches are removed. It has been built on both Arm defconfigs that
> rely on pinctrl-nomadik: nhk8815_defconfig and u8500_defconfig. I don't
> have any Nomadik hardware to test though.
>
> Have a nice day,
> Théo
>

Are you targeting the GPIO branch with this or pinctrl? I guess GPIO
so I'll need Linus' Acks under the pinctrl patches.

> [0]: https://lore.kernel.org/lkml/20240212-mbly-clk-v6-0-c46fa1f93839@bootlin.com/
> [1]: https://lore.kernel.org/lkml/20240205153503.574468-1-gregory.clement@bootlin.com/

Please advise the relevant maintainers that they should provide an
immutable branch for these series once they're queued in their
respective trees.

Bart

[snip]