[PATCH v3 0/5] pinctrl/GPIO driver for Apple SoCs

[PATCH v3 0/5] pinctrl/GPIO driver for Apple SoCs

[Joey Gouly]

Hi all,

Here is the v3 patchset for the Apple pinctrl/GPIO driver.

Changes since v2: [1]
  - Document apple,npins property (passes dt_binding_check)
  - Add #interrupt-cells to bindings, asked for by Sven
  - Redo macro defines, add register bits (that aren't currently used)
  - use regmap-mmio, to remove NIH shadowing
  - remove weird INPUT_ENABLE dance in apple_gpio_set_reg

I split the MAINTAINERS change into a separate commit, Hector Martin has been
collecting those in his tree, to avoid merge conflicts.

There is a branch here with the driver:
  https://gitlab.arm.com/linux-arm/jg-open/-/tree/pinctrl_apple_v3

Thanks,
Joey

note: For those that have been testing this with PCIe etc, you will
probably want to also include the last commit in the following branch, as I
dropped the clock references in the code (due to the switch to power domains):
https://gitlab.arm.com/linux-arm/jg-open/-/commits/pinctrl_apple_v3_clock

[1]
https://lore.kernel.org/linux-gpio/20211001191209.29988-1-joey.gouly@arm.com/

Joey Gouly (4):
  dt-bindings: pinctrl: add #interrupt-cells to apple,pinctrl
  dt-bindings: pinctrl: Add apple,npins property to apple,pinctrl
  pinctrl: add pinctrl/GPIO driver for Apple SoCs
  MAINTAINERS: add pinctrl-apple-gpio to ARM/APPLE MACHINE

Marc Zyngier (1):
  gpio: Allow per-parent interrupt data

 .../bindings/pinctrl/apple,pinctrl.yaml       |   9 +
 MAINTAINERS                                   |   1 +
 drivers/gpio/gpiolib.c                        |   9 +-
 drivers/pinctrl/Kconfig                       |  16 +
 drivers/pinctrl/Makefile                      |   1 +
 drivers/pinctrl/pinctrl-apple-gpio.c          | 564 ++++++++++++++++++
 include/linux/gpio/driver.h                   |  19 +-
 7 files changed, 615 insertions(+), 4 deletions(-)
 create mode 100644 drivers/pinctrl/pinctrl-apple-gpio.c

-- 
2.17.1

[PATCH v3 1/5] gpio: Allow per-parent interrupt data

[Joey Gouly]

From: Marc Zyngier <maz@kernel.org>

The core gpiolib code is able to deal with multiple interrupt parents
for a single gpio irqchip. It however only allows a single piece
of data to be conveyed to all flow handlers (either the gpio_chip
or some other, driver-specific data).

This means that drivers have to go through some interesting dance
to find the correct context, something that isn't great in interrupt
context (see aebdc8abc9db86e2bd33070fc2f961012fff74b4 for a prime
example).

Instead, offer an optional way for a pinctrl/gpio driver to provide
an array of pointers which gets used to provide the correct context
to the flow handler.

Signed-off-by: Marc Zyngier <maz@kernel.org>
Signed-off-by: Joey Gouly <joey.gouly@arm.com>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/gpio/gpiolib.c      |  9 +++++++--
 include/linux/gpio/driver.h | 19 +++++++++++++++++--
 2 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index d1b9b721218f..abfbf546d159 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1534,9 +1534,14 @@ static int gpiochip_add_irqchip(struct gpio_chip *gc,
 	}
 
 	if (gc->irq.parent_handler) {
-		void *data = gc->irq.parent_handler_data ?: gc;
-
 		for (i = 0; i < gc->irq.num_parents; i++) {
+			void *data;
+
+			if (gc->irq.per_parent_data)
+				data = gc->irq.parent_handler_data_array[i];
+			else
+				data = gc->irq.parent_handler_data ?: gc;
+
 			/*
 			 * The parent IRQ chip is already using the chip_data
 			 * for this IRQ chip, so our callbacks simply use the
diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h
index a0f9901dcae6..a673a359e20b 100644
--- a/include/linux/gpio/driver.h
+++ b/include/linux/gpio/driver.h
@@ -168,11 +168,18 @@ struct gpio_irq_chip {
 
 	/**
 	 * @parent_handler_data:
+	 * @parent_handler_data_array:
 	 *
 	 * Data associated, and passed to, the handler for the parent
-	 * interrupt.
+	 * interrupt. Can either be a single pointer if @per_parent_data
+	 * is false, or an array of @num_parents pointers otherwise.  If
+	 * @per_parent_data is true, @parent_handler_data_array cannot be
+	 * NULL.
 	 */
-	void *parent_handler_data;
+	union {
+		void *parent_handler_data;
+		void **parent_handler_data_array;
+	};
 
 	/**
 	 * @num_parents:
@@ -203,6 +210,14 @@ struct gpio_irq_chip {
 	 */
 	bool threaded;
 
+	/**
+	 * @per_parent_data:
+	 *
+	 * True if parent_handler_data_array describes a @num_parents
+	 * sized array to be used as parent data.
+	 */
+	bool per_parent_data;
+
 	/**
 	 * @init_hw: optional routine to initialize hardware before
 	 * an IRQ chip will be added. This is quite useful when
-- 
2.17.1

Re: [PATCH v3 1/5] gpio: Allow per-parent interrupt data

[Linus Walleij]

Top-posting because I need a nod from Bartosz that I can
merge this patch with the rest in the pinctrl tree.

Bartosz: I can also offer this one patch in an immutable branch
as well so you can pull it in as well.

Yours,
Linus Walleij

On Sat, Oct 16, 2021 at 4:19 PM Joey Gouly <joey.gouly@arm.com> wrote:

> From: Marc Zyngier <maz@kernel.org>
>
> The core gpiolib code is able to deal with multiple interrupt parents
> for a single gpio irqchip. It however only allows a single piece
> of data to be conveyed to all flow handlers (either the gpio_chip
> or some other, driver-specific data).
>
> This means that drivers have to go through some interesting dance
> to find the correct context, something that isn't great in interrupt
> context (see aebdc8abc9db86e2bd33070fc2f961012fff74b4 for a prime
> example).
>
> Instead, offer an optional way for a pinctrl/gpio driver to provide
> an array of pointers which gets used to provide the correct context
> to the flow handler.
>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> Signed-off-by: Joey Gouly <joey.gouly@arm.com>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  drivers/gpio/gpiolib.c      |  9 +++++++--
>  include/linux/gpio/driver.h | 19 +++++++++++++++++--
>  2 files changed, 24 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index d1b9b721218f..abfbf546d159 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -1534,9 +1534,14 @@ static int gpiochip_add_irqchip(struct gpio_chip *gc,
>         }
>
>         if (gc->irq.parent_handler) {
> -               void *data = gc->irq.parent_handler_data ?: gc;
> -
>                 for (i = 0; i < gc->irq.num_parents; i++) {
> +                       void *data;
> +
> +                       if (gc->irq.per_parent_data)
> +                               data = gc->irq.parent_handler_data_array[i];
> +                       else
> +                               data = gc->irq.parent_handler_data ?: gc;
> +
>                         /*
>                          * The parent IRQ chip is already using the chip_data
>                          * for this IRQ chip, so our callbacks simply use the
> diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h
> index a0f9901dcae6..a673a359e20b 100644
> --- a/include/linux/gpio/driver.h
> +++ b/include/linux/gpio/driver.h
> @@ -168,11 +168,18 @@ struct gpio_irq_chip {
>
>         /**
>          * @parent_handler_data:
> +        * @parent_handler_data_array:
>          *
>          * Data associated, and passed to, the handler for the parent
> -        * interrupt.
> +        * interrupt. Can either be a single pointer if @per_parent_data
> +        * is false, or an array of @num_parents pointers otherwise.  If
> +        * @per_parent_data is true, @parent_handler_data_array cannot be
> +        * NULL.
>          */
> -       void *parent_handler_data;
> +       union {
> +               void *parent_handler_data;
> +               void **parent_handler_data_array;
> +       };
>
>         /**
>          * @num_parents:
> @@ -203,6 +210,14 @@ struct gpio_irq_chip {
>          */
>         bool threaded;
>
> +       /**
> +        * @per_parent_data:
> +        *
> +        * True if parent_handler_data_array describes a @num_parents
> +        * sized array to be used as parent data.
> +        */
> +       bool per_parent_data;
> +
>         /**
>          * @init_hw: optional routine to initialize hardware before
>          * an IRQ chip will be added. This is quite useful when
> --
> 2.17.1
>

Re: [PATCH v3 1/5] gpio: Allow per-parent interrupt data

[Bartosz Golaszewski]

On Sun, Oct 17, 2021 at 12:37 AM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> Top-posting because I need a nod from Bartosz that I can
> merge this patch with the rest in the pinctrl tree.
>
> Bartosz: I can also offer this one patch in an immutable branch
> as well so you can pull it in as well.
>
> Yours,
> Linus Walleij

Hey! Sorry, didn't see it. Yes please take it.

Acked-by: Bartosz Golaszewski <brgl@bgdev.pl>

[PATCH v3 2/5] dt-bindings: pinctrl: add #interrupt-cells to apple,pinctrl

[Joey Gouly]

The GPIO/pinctrl hardware can act as an interrupt-controller, so add
the #interrupt-cells property to the binding.

Signed-off-by: Joey Gouly <joey.gouly@arm.com>
---
 Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml
index d50571affd1f..340be4eabf44 100644
--- a/Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml
@@ -43,6 +43,9 @@ properties:
 
   interrupt-controller: true
 
+  '#interrupt-cells':
+    const: 2
+
 patternProperties:
   '-pins$':
     type: object
-- 
2.17.1

Re: [PATCH v3 2/5] dt-bindings: pinctrl: add #interrupt-cells to apple,pinctrl

[Sven Peter]

On Sat, Oct 16, 2021, at 16:18, Joey Gouly wrote:
> The GPIO/pinctrl hardware can act as an interrupt-controller, so add
> the #interrupt-cells property to the binding.
>
> Signed-off-by: Joey Gouly <joey.gouly@arm.com>
> ---
>  Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git 
> a/Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml 
> b/Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml
> index d50571affd1f..340be4eabf44 100644
> --- a/Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml
> +++ b/Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml
> @@ -43,6 +43,9 @@ properties:
> 
>    interrupt-controller: true
> 
> +  '#interrupt-cells':
> +    const: 2
> +

thanks for adding this! small nit: could you also add it to the example below?

Reviewed-by: Sven Peter <sven@svenpeter.dev>


Thanks,

Sven

Re: [PATCH v3 2/5] dt-bindings: pinctrl: add #interrupt-cells to apple,pinctrl

[Rob Herring]

On Sat, 16 Oct 2021 15:18:36 +0100, Joey Gouly wrote:
> The GPIO/pinctrl hardware can act as an interrupt-controller, so add
> the #interrupt-cells property to the binding.
> 
> Signed-off-by: Joey Gouly <joey.gouly@arm.com>
> ---
>  Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml | 3 +++
>  1 file changed, 3 insertions(+)
> 

Acked-by: Rob Herring <robh@kernel.org>

[PATCH v3 3/5] dt-bindings: pinctrl: Add apple,npins property to apple,pinctrl

[Joey Gouly]

This property is used to describe the total number of pins on this
particular pinctrl hardware block.

Signed-off-by: Joey Gouly <joey.gouly@arm.com>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
---
 .../devicetree/bindings/pinctrl/apple,pinctrl.yaml          | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml
index 340be4eabf44..5f3baa918ed7 100644
--- a/Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml
@@ -34,6 +34,10 @@ properties:
   gpio-ranges:
     maxItems: 1
 
+  apple,npins:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: The number of pins in this GPIO controller.
+
   interrupts:
     description: One interrupt for each of the (up to 7) interrupt
       groups supported by the controller sorted by interrupt group
@@ -69,6 +73,7 @@ required:
   - gpio-controller
   - '#gpio-cells'
   - gpio-ranges
+  - apple,npins
 
 additionalProperties: false
 
@@ -89,6 +94,7 @@ examples:
         gpio-controller;
         #gpio-cells = <2>;
         gpio-ranges = <&pinctrl 0 0 212>;
+        apple,npins = <212>;
 
         interrupt-controller;
         interrupt-parent = <&aic>;
-- 
2.17.1

Re: [PATCH v3 3/5] dt-bindings: pinctrl: Add apple,npins property to apple,pinctrl

[Rob Herring]

On Sat, 16 Oct 2021 15:18:37 +0100, Joey Gouly wrote:
> This property is used to describe the total number of pins on this
> particular pinctrl hardware block.
> 
> Signed-off-by: Joey Gouly <joey.gouly@arm.com>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  .../devicetree/bindings/pinctrl/apple,pinctrl.yaml          | 6 ++++++
>  1 file changed, 6 insertions(+)
> 

Acked-by: Rob Herring <robh@kernel.org>

[PATCH v3 4/5] pinctrl: add pinctrl/GPIO driver for Apple SoCs

[Joey Gouly]

This driver adds support for the pinctrl / GPIO hardware found
on some Apple SoCs.

Co-developed-by: Stan Skowronek <stan@corellium.com>
Signed-off-by: Stan Skowronek <stan@corellium.com>
Signed-off-by: Joey Gouly <joey.gouly@arm.com>
---
 drivers/pinctrl/Kconfig              |  16 +
 drivers/pinctrl/Makefile             |   1 +
 drivers/pinctrl/pinctrl-apple-gpio.c | 564 +++++++++++++++++++++++++++
 3 files changed, 581 insertions(+)
 create mode 100644 drivers/pinctrl/pinctrl-apple-gpio.c

diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index 31921108e456..6a961d5f8726 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -31,6 +31,22 @@ config DEBUG_PINCTRL
 	help
 	  Say Y here to add some extra checks and diagnostics to PINCTRL calls.
 
+config PINCTRL_APPLE_GPIO
+	tristate "Apple SoC GPIO pin controller driver"
+	depends on ARCH_APPLE
+	select PINMUX
+	select GPIOLIB
+	select GPIOLIB_IRQCHIP
+	select GENERIC_PINCTRL_GROUPS
+	select GENERIC_PINMUX_FUNCTIONS
+	select OF_GPIO
+	help
+	  This is the driver for the GPIO controller found on Apple ARM SoCs,
+	  including M1.
+
+	  This driver can also be built as a module. If so, the module
+	  will be called pinctrl-apple-gpio.
+
 config PINCTRL_ARTPEC6
 	bool "Axis ARTPEC-6 pin controller driver"
 	depends on MACH_ARTPEC6
diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
index 200073bcc2c1..5e63de2ffcf4 100644
--- a/drivers/pinctrl/Makefile
+++ b/drivers/pinctrl/Makefile
@@ -8,6 +8,7 @@ obj-$(CONFIG_PINMUX)		+= pinmux.o
 obj-$(CONFIG_PINCONF)		+= pinconf.o
 obj-$(CONFIG_OF)		+= devicetree.o
 obj-$(CONFIG_GENERIC_PINCONF)	+= pinconf-generic.o
+obj-$(CONFIG_PINCTRL_APPLE_GPIO) += pinctrl-apple-gpio.o
 obj-$(CONFIG_PINCTRL_ARTPEC6)	+= pinctrl-artpec6.o
 obj-$(CONFIG_PINCTRL_AS3722)	+= pinctrl-as3722.o
 obj-$(CONFIG_PINCTRL_AXP209)	+= pinctrl-axp209.o
diff --git a/drivers/pinctrl/pinctrl-apple-gpio.c b/drivers/pinctrl/pinctrl-apple-gpio.c
new file mode 100644
index 000000000000..df93eb12069d
--- /dev/null
+++ b/drivers/pinctrl/pinctrl-apple-gpio.c
@@ -0,0 +1,564 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Apple SoC pinctrl+GPIO+external IRQ driver
+ *
+ * Copyright (C) 2021 The Asahi Linux Contributors
+ * Copyright (C) 2020 Corellium LLC
+ *
+ * Based on: pinctrl-pistachio.c
+ * Copyright (C) 2014 Imagination Technologies Ltd.
+ * Copyright (C) 2014 Google, Inc.
+ */
+
+#include <dt-bindings/pinctrl/apple.h>
+#include <linux/gpio/driver.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_irq.h>
+#include <linux/pinctrl/pinctrl.h>
+#include <linux/pinctrl/pinmux.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#include "pinctrl-utils.h"
+#include "core.h"
+#include "pinmux.h"
+
+struct apple_gpio_irq_data {
+	struct apple_gpio_pinctrl *pctl;
+	int irqgrp;
+};
+
+struct apple_gpio_pinctrl {
+	struct device *dev;
+	struct pinctrl_dev *pctldev;
+
+	void __iomem *base;
+	struct regmap *map;
+	unsigned int nirqgrps;
+
+	struct pinctrl_desc pinctrl_desc;
+	struct gpio_chip gpio_chip;
+	struct irq_chip irq_chip;
+};
+
+#define REG_GPIO(x)          (4 * (x))
+#define REG_GPIOx_DATA       BIT(0)
+#define REG_GPIOx_MODE       GENMASK(3, 1)
+#define REG_GPIOx_OUT        1
+#define REG_GPIOx_IN_IRQ_HI  2
+#define REG_GPIOx_IN_IRQ_LO  3
+#define REG_GPIOx_IN_IRQ_UP  4
+#define REG_GPIOx_IN_IRQ_DN  5
+#define REG_GPIOx_IN_IRQ_ANY 6
+#define REG_GPIOx_IN_IRQ_OFF 7
+#define REG_GPIOx_PERIPH     GENMASK(6, 5)
+#define REG_GPIOx_PULL       GENMASK(8, 7)
+#define REG_GPIOx_PULL_OFF   0
+#define REG_GPIOx_PULL_DOWN  1
+#define REG_GPIOx_PULL_UP_STRONG 2
+#define REG_GPIOx_PULL_UP    3
+#define REG_GPIOx_INPUT_ENABLE BIT(9)
+#define REG_GPIOx_DRIVE_STRENGTH0 GENMASK(11, 10)
+#define REG_GPIOx_SCHMITT    BIT(15)
+#define REG_GPIOx_GRP        GENMASK(18, 16)
+#define REG_GPIOx_LOCK       BIT(21)
+#define REG_GPIOx_DRIVE_STRENGTH1 GENMASK(23, 22)
+#define REG_IRQ(g, x)        (0x800 + 0x40 * (g) + 4 * ((x) >> 5))
+
+struct regmap_config regmap_config = {
+	.reg_bits = 32,
+	.val_bits = 32,
+	.reg_stride = 4,
+	.cache_type = REGCACHE_FLAT,
+	.max_register = 512 * sizeof(uint32_t),
+	.num_reg_defaults_raw = 512,
+	.use_relaxed_mmio = true
+};
+
+// No locking needed to mask/unmask IRQs as the interrupt mode is per pin-register.
+static void apple_gpio_set_reg(struct apple_gpio_pinctrl *pctl,
+			       unsigned int pin, uint32_t mask, uint32_t value)
+{
+	regmap_update_bits(pctl->map, REG_GPIO(pin), mask, value);
+}
+
+static uint32_t apple_gpio_get_reg(struct apple_gpio_pinctrl *pctl,
+				   unsigned int pin)
+{
+	uint32_t val = 0;
+
+	regmap_read(pctl->map, REG_GPIO(pin), &val);
+	return val;
+}
+
+/* Pin controller functions */
+
+static int apple_gpio_dt_node_to_map(struct pinctrl_dev *pctldev,
+				     struct device_node *node,
+				     struct pinctrl_map **map,
+				     unsigned *num_maps)
+{
+	unsigned reserved_maps;
+	struct apple_gpio_pinctrl *pctl;
+	u32 pinfunc, pin, func;
+	int num_pins, i, ret;
+	const char *group_name;
+	const char *function_name;
+
+	*map = NULL;
+	*num_maps = 0;
+	reserved_maps = 0;
+
+	pctl = pinctrl_dev_get_drvdata(pctldev);
+
+	ret = of_property_count_u32_elems(node, "pinmux");
+	if (ret <= 0) {
+		dev_err(pctl->dev,
+			"missing or empty pinmux property in node %pOFn.\n",
+			node);
+		return -EINVAL;
+	}
+
+	num_pins = ret;
+
+	ret = pinctrl_utils_reserve_map(pctldev, map, &reserved_maps, num_maps,
+					num_pins);
+	if (ret)
+		return ret;
+
+	for (i = 0; i < num_pins; i++) {
+		ret = of_property_read_u32_index(node, "pinmux", i, &pinfunc);
+		if (ret)
+			goto free_map;
+
+		pin = APPLE_PIN(pinfunc);
+		func = APPLE_FUNC(pinfunc);
+
+		if (func >= pinmux_generic_get_function_count(pctldev)) {
+			ret = -EINVAL;
+			goto free_map;
+		}
+
+		group_name = pinctrl_generic_get_group_name(pctldev, pin);
+		function_name =
+			pinmux_generic_get_function_name(pctl->pctldev, func);
+		ret = pinctrl_utils_add_map_mux(pctl->pctldev, map,
+						&reserved_maps, num_maps,
+						group_name, function_name);
+		if (ret)
+			goto free_map;
+	}
+
+free_map:
+	if (ret < 0) {
+		pinctrl_utils_free_map(pctldev, *map, *num_maps);
+		return ret;
+	}
+
+	return 0;
+}
+
+static const struct pinctrl_ops apple_gpio_pinctrl_ops = {
+	.get_groups_count = pinctrl_generic_get_group_count,
+	.get_group_name = pinctrl_generic_get_group_name,
+	.get_group_pins = pinctrl_generic_get_group_pins,
+	.dt_node_to_map = apple_gpio_dt_node_to_map,
+	.dt_free_map = pinctrl_utils_free_map,
+};
+
+/* Pin multiplexer functions */
+
+static int apple_gpio_pinmux_enable(struct pinctrl_dev *pctldev, unsigned func,
+				    unsigned group)
+{
+	struct apple_gpio_pinctrl *pctl = pinctrl_dev_get_drvdata(pctldev);
+
+	apple_gpio_set_reg(
+		pctl, group, REG_GPIOx_PERIPH | REG_GPIOx_INPUT_ENABLE,
+		FIELD_PREP(REG_GPIOx_PERIPH, func) | REG_GPIOx_INPUT_ENABLE);
+
+	return 0;
+}
+
+static const struct pinmux_ops apple_gpio_pinmux_ops = {
+	.get_functions_count = pinmux_generic_get_function_count,
+	.get_function_name = pinmux_generic_get_function_name,
+	.get_function_groups = pinmux_generic_get_function_groups,
+	.set_mux = apple_gpio_pinmux_enable,
+	.strict = true,
+};
+
+/* GPIO chip functions */
+
+static int apple_gpio_gpio_get_direction(struct gpio_chip *chip,
+					 unsigned int offset)
+{
+	struct apple_gpio_pinctrl *pctl = gpiochip_get_data(chip);
+	unsigned int reg = apple_gpio_get_reg(pctl, offset);
+
+	return (FIELD_GET(REG_GPIOx_MODE, reg) == REG_GPIOx_OUT) ?
+		       GPIO_LINE_DIRECTION_OUT :
+			     GPIO_LINE_DIRECTION_IN;
+}
+
+static int apple_gpio_gpio_get(struct gpio_chip *chip, unsigned offset)
+{
+	struct apple_gpio_pinctrl *pctl = gpiochip_get_data(chip);
+	unsigned int reg = apple_gpio_get_reg(pctl, offset);
+
+	// If this is an input GPIO, read the actual value (not the cached regmap value)
+	if (FIELD_GET(REG_GPIOx_MODE, reg) != REG_GPIOx_OUT)
+		reg = readl_relaxed(pctl->base + REG_GPIO(offset));
+
+	return !!(reg & REG_GPIOx_DATA);
+}
+
+static void apple_gpio_gpio_set(struct gpio_chip *chip, unsigned int offset,
+				int value)
+{
+	struct apple_gpio_pinctrl *pctl = gpiochip_get_data(chip);
+
+	apple_gpio_set_reg(pctl, offset, REG_GPIOx_DATA,
+			   value ? REG_GPIOx_DATA : 0);
+}
+
+static int apple_gpio_gpio_direction_input(struct gpio_chip *chip,
+					   unsigned int offset)
+{
+	struct apple_gpio_pinctrl *pctl = gpiochip_get_data(chip);
+
+	apple_gpio_set_reg(pctl, offset,
+			   REG_GPIOx_PERIPH | REG_GPIOx_MODE | REG_GPIOx_DATA |
+				   REG_GPIOx_INPUT_ENABLE,
+			   FIELD_PREP(REG_GPIOx_MODE, REG_GPIOx_IN_IRQ_OFF) |
+				   REG_GPIOx_INPUT_ENABLE);
+	return 0;
+}
+
+static int apple_gpio_gpio_direction_output(struct gpio_chip *chip,
+					    unsigned int offset, int value)
+{
+	struct apple_gpio_pinctrl *pctl = gpiochip_get_data(chip);
+
+	apple_gpio_set_reg(pctl, offset,
+			   REG_GPIOx_PERIPH | REG_GPIOx_MODE | REG_GPIOx_DATA,
+			   FIELD_PREP(REG_GPIOx_MODE, REG_GPIOx_OUT) |
+				   (value ? REG_GPIOx_DATA : 0));
+	return 0;
+}
+
+/* IRQ chip functions */
+
+static void apple_gpio_gpio_irq_ack(struct irq_data *data)
+{
+	struct apple_gpio_pinctrl *pctl =
+		gpiochip_get_data(irq_data_get_irq_chip_data(data));
+	unsigned int irqgrp =
+		FIELD_GET(REG_GPIOx_GRP, apple_gpio_get_reg(pctl, data->hwirq));
+
+	writel(BIT(data->hwirq & 31),
+	       pctl->base + REG_IRQ(irqgrp, data->hwirq));
+}
+
+static int apple_gpio_irq_type(unsigned int type)
+{
+	switch (type & IRQ_TYPE_SENSE_MASK) {
+	case IRQ_TYPE_EDGE_RISING:
+		return REG_GPIOx_IN_IRQ_UP;
+	case IRQ_TYPE_EDGE_FALLING:
+		return REG_GPIOx_IN_IRQ_DN;
+	case IRQ_TYPE_EDGE_BOTH:
+		return REG_GPIOx_IN_IRQ_ANY;
+	case IRQ_TYPE_LEVEL_HIGH:
+		return REG_GPIOx_IN_IRQ_HI;
+	case IRQ_TYPE_LEVEL_LOW:
+		return REG_GPIOx_IN_IRQ_LO;
+	default:
+		return -EINVAL;
+	}
+}
+
+static void apple_gpio_gpio_irq_mask(struct irq_data *data)
+{
+	struct apple_gpio_pinctrl *pctl =
+		gpiochip_get_data(irq_data_get_irq_chip_data(data));
+	apple_gpio_set_reg(pctl, data->hwirq, REG_GPIOx_MODE,
+			   FIELD_PREP(REG_GPIOx_MODE, REG_GPIOx_IN_IRQ_OFF));
+}
+
+static void apple_gpio_gpio_irq_unmask(struct irq_data *data)
+{
+	struct apple_gpio_pinctrl *pctl =
+		gpiochip_get_data(irq_data_get_irq_chip_data(data));
+	int irqtype = apple_gpio_irq_type(irqd_get_trigger_type(data));
+
+	if (WARN_ON(irqtype < 0))
+		return;
+	apple_gpio_set_reg(pctl, data->hwirq, REG_GPIOx_MODE,
+			   FIELD_PREP(REG_GPIOx_MODE, irqtype));
+}
+
+static unsigned int apple_gpio_gpio_irq_startup(struct irq_data *data)
+{
+	struct gpio_chip *chip = irq_data_get_irq_chip_data(data);
+	struct apple_gpio_pinctrl *pctl = gpiochip_get_data(chip);
+
+	apple_gpio_set_reg(pctl, data->hwirq, REG_GPIOx_GRP,
+			   FIELD_PREP(REG_GPIOx_GRP, 0));
+
+	apple_gpio_gpio_direction_input(chip, data->hwirq);
+	apple_gpio_gpio_irq_unmask(data);
+
+	return 0;
+}
+
+static int apple_gpio_gpio_irq_set_type(struct irq_data *data,
+					unsigned int type)
+{
+	struct apple_gpio_pinctrl *pctl =
+		gpiochip_get_data(irq_data_get_irq_chip_data(data));
+	int irqtype = apple_gpio_irq_type(type);
+
+	if (irqtype < 0)
+		return irqtype;
+
+	irqd_set_trigger_type(data, type);
+
+	apple_gpio_set_reg(pctl, data->hwirq, REG_GPIOx_MODE,
+			   FIELD_PREP(REG_GPIOx_MODE, irqtype));
+
+	if (type & IRQ_TYPE_LEVEL_MASK)
+		irq_set_handler_locked(data, handle_level_irq);
+	else
+		irq_set_handler_locked(data, handle_edge_irq);
+	return 0;
+}
+
+static void apple_gpio_gpio_irq_handler(struct irq_desc *desc)
+{
+	struct apple_gpio_irq_data *data = irq_desc_get_handler_data(desc);
+	struct apple_gpio_pinctrl *pctl = data->pctl;
+	struct gpio_chip *gc = &pctl->gpio_chip;
+	struct irq_chip *chip = irq_desc_get_chip(desc);
+	unsigned int irqgrp = data->irqgrp;
+	unsigned int pinh, pinl;
+	unsigned long pending;
+
+	chained_irq_enter(chip, desc);
+	for (pinh = 0; pinh < gc->ngpio; pinh += 32) {
+		pending = readl(pctl->base + REG_IRQ(irqgrp, pinh));
+		for_each_set_bit(pinl, &pending, 32)
+			generic_handle_domain_irq(gc->irq.domain, pinh + pinl);
+	}
+	chained_irq_exit(chip, desc);
+}
+
+/* Probe & register */
+
+static int apple_gpio_gpio_register(struct apple_gpio_pinctrl *pctl)
+{
+	struct device_node *node = pctl->dev->of_node;
+	struct gpio_irq_chip *girq;
+	struct apple_gpio_irq_data **irq_data;
+	int i;
+	int ret;
+
+	if (!of_find_property(node, "gpio-controller", NULL)) {
+		dev_err(pctl->dev,
+			"Apple GPIO must have 'gpio-controller' property.\n");
+		return -ENODEV;
+	}
+
+	pctl->gpio_chip.label = dev_name(pctl->dev);
+	pctl->gpio_chip.request = gpiochip_generic_request;
+	pctl->gpio_chip.free = gpiochip_generic_free;
+	pctl->gpio_chip.get_direction = apple_gpio_gpio_get_direction;
+	pctl->gpio_chip.direction_input = apple_gpio_gpio_direction_input;
+	pctl->gpio_chip.direction_output = apple_gpio_gpio_direction_output;
+	pctl->gpio_chip.get = apple_gpio_gpio_get;
+	pctl->gpio_chip.set = apple_gpio_gpio_set;
+	pctl->gpio_chip.base = -1;
+	pctl->gpio_chip.ngpio = pctl->pinctrl_desc.npins;
+	pctl->gpio_chip.parent = pctl->dev;
+	pctl->gpio_chip.of_node = node;
+
+	if (of_property_read_bool(node, "interrupt-controller")) {
+		ret = platform_irq_count(to_platform_device(pctl->dev));
+		if (ret < 0)
+			return ret;
+
+		pctl->nirqgrps = ret;
+
+		pctl->irq_chip.name = "Apple-GPIO";
+		pctl->irq_chip.irq_startup = apple_gpio_gpio_irq_startup;
+		pctl->irq_chip.irq_ack = apple_gpio_gpio_irq_ack;
+		pctl->irq_chip.irq_mask = apple_gpio_gpio_irq_mask;
+		pctl->irq_chip.irq_unmask = apple_gpio_gpio_irq_unmask;
+		pctl->irq_chip.irq_set_type = apple_gpio_gpio_irq_set_type;
+
+		girq = &pctl->gpio_chip.irq;
+		girq->chip = &pctl->irq_chip;
+		girq->parent_handler = apple_gpio_gpio_irq_handler;
+		girq->num_parents = pctl->nirqgrps;
+
+		girq->parents = kmalloc_array(
+			pctl->nirqgrps, sizeof(*girq->parents), GFP_KERNEL);
+		if (!girq->parents)
+			return -ENOMEM;
+
+		for (i = 0; i < pctl->nirqgrps; i++) {
+			ret = platform_get_irq(to_platform_device(pctl->dev),
+					       i);
+			if (ret < 0) {
+				return dev_err_probe(pctl->dev, ret,
+						     "Failed to map IRQ %d\n",
+						     i);
+			}
+			girq->parents[i] = ret;
+		}
+
+		irq_data = kmalloc_array(pctl->nirqgrps, sizeof(*irq_data),
+					 GFP_KERNEL);
+		if (!irq_data) {
+			kfree(girq->parents);
+			return -ENOMEM;
+		}
+		for (i = 0; i < pctl->nirqgrps; i++) {
+			irq_data[i] = devm_kzalloc(
+				pctl->dev, sizeof(*irq_data[i]), GFP_KERNEL);
+			irq_data[i]->pctl = pctl;
+			irq_data[i]->irqgrp = i;
+		}
+
+		girq->parent_handler_data_array = (void **)irq_data;
+		girq->per_parent_data = true;
+		girq->default_type = IRQ_TYPE_NONE;
+		girq->handler = handle_level_irq;
+	}
+
+	ret = devm_gpiochip_add_data(pctl->dev, &pctl->gpio_chip, pctl);
+
+	if (of_property_read_bool(node, "interrupt-controller")) {
+		kfree(girq->parents);
+		kfree(irq_data);
+	}
+
+	return ret;
+}
+
+static int apple_gpio_pinctrl_probe(struct platform_device *pdev)
+{
+	struct apple_gpio_pinctrl *pctl;
+	struct pinctrl_pin_desc *pins;
+	unsigned int npins;
+	const char **pin_names;
+	unsigned int *pin_nums;
+	unsigned int i;
+	int res;
+
+	pctl = devm_kzalloc(&pdev->dev, sizeof(*pctl), GFP_KERNEL);
+	if (!pctl)
+		return -ENOMEM;
+	pctl->dev = &pdev->dev;
+	dev_set_drvdata(&pdev->dev, pctl);
+
+	if (of_property_read_u32(pdev->dev.of_node, "apple,npins", &npins)) {
+		dev_err(&pdev->dev, "apple,npins property not found\n");
+		return -EINVAL;
+	}
+
+	pins = devm_kmalloc_array(&pdev->dev, npins, sizeof(pins[0]),
+				  GFP_KERNEL);
+	if (!pins)
+		return -ENOMEM;
+	pin_names = devm_kmalloc_array(&pdev->dev, npins, sizeof(pin_names[0]),
+				       GFP_KERNEL);
+	if (!pin_names)
+		return -ENOMEM;
+	pin_nums = devm_kmalloc_array(&pdev->dev, npins, sizeof(pin_nums[0]),
+				      GFP_KERNEL);
+	if (!pin_nums)
+		return -ENOMEM;
+
+	pctl->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(pctl->base))
+		return PTR_ERR(pctl->base);
+
+	pctl->map =
+		devm_regmap_init_mmio(&pdev->dev, pctl->base, &regmap_config);
+	if (IS_ERR(pctl->map)) {
+		dev_err(&pdev->dev, "Failed to create regmap\n");
+		return PTR_ERR(pctl->map);
+	}
+
+	for (i = 0; i < npins; i++) {
+		pins[i].number = i;
+		pins[i].name =
+			devm_kasprintf(&pdev->dev, GFP_KERNEL, "PIN%u", i);
+		pins[i].drv_data = pctl;
+		pin_names[i] = pins[i].name;
+		pin_nums[i] = i;
+	}
+
+	pctl->pinctrl_desc.name = dev_name(pctl->dev);
+	pctl->pinctrl_desc.pins = pins;
+	pctl->pinctrl_desc.npins = npins;
+	pctl->pinctrl_desc.pctlops = &apple_gpio_pinctrl_ops;
+	pctl->pinctrl_desc.pmxops = &apple_gpio_pinmux_ops;
+
+	pctl->pctldev =
+		devm_pinctrl_register(&pdev->dev, &pctl->pinctrl_desc, pctl);
+	if (IS_ERR(pctl->pctldev)) {
+		dev_err(&pdev->dev, "Failed to register pinctrl device.\n");
+		return PTR_ERR(pctl->pctldev);
+	}
+
+	for (i = 0; i < npins; i++) {
+		res = pinctrl_generic_add_group(pctl->pctldev, pins[i].name,
+						pin_nums + i, 1, pctl);
+		if (res < 0) {
+			dev_err(pctl->dev, "Failed to register group.");
+			return res;
+		}
+	}
+
+	res = pinmux_generic_add_function(pctl->pctldev, "gpio", pin_names,
+					  npins, pctl);
+	if (res < 0) {
+		dev_err(pctl->dev, "Failed to register function.");
+		return res;
+	}
+
+	res = pinmux_generic_add_function(pctl->pctldev, "periph", pin_names,
+					  npins, pctl);
+	if (res < 0) {
+		dev_err(pctl->dev, "Failed to register function.");
+		return res;
+	}
+
+	return apple_gpio_gpio_register(pctl);
+}
+
+static const struct of_device_id apple_gpio_pinctrl_of_match[] = {
+	{ .compatible = "apple,t8103-pinctrl", },
+	{ .compatible = "apple,pinctrl", },
+	{ }
+};
+
+static struct platform_driver apple_gpio_pinctrl_driver = {
+	.driver = {
+		.name = "apple-gpio-pinctrl",
+		.of_match_table = apple_gpio_pinctrl_of_match,
+		.suppress_bind_attrs = true,
+	},
+	.probe = apple_gpio_pinctrl_probe,
+};
+module_platform_driver(apple_gpio_pinctrl_driver);
+
+MODULE_DESCRIPTION("Apple pinctrl/GPIO driver");
+MODULE_AUTHOR("Stan Skowronek <stan@corellium.com>");
+MODULE_AUTHOR("Joey Gouly <joey.gouly@arm.com>");
+MODULE_LICENSE("GPL v2");
-- 
2.17.1

Re: [PATCH v3 4/5] pinctrl: add pinctrl/GPIO driver for Apple SoCs

[Alyssa Rosenzweig]

> +static int apple_gpio_gpio_get_direction(struct gpio_chip *chip,
> +					 unsigned int offset)
> +{
> +	struct apple_gpio_pinctrl *pctl = gpiochip_get_data(chip);
> +	unsigned int reg = apple_gpio_get_reg(pctl, offset);
> +
> +	return (FIELD_GET(REG_GPIOx_MODE, reg) == REG_GPIOx_OUT) ?
> +		       GPIO_LINE_DIRECTION_OUT :
> +			     GPIO_LINE_DIRECTION_IN;
> +}

Nit: weird spacing.

Re: [PATCH v3 4/5] pinctrl: add pinctrl/GPIO driver for Apple SoCs

[Mark Kettenis]

> From: Joey Gouly <joey.gouly@arm.com>
> Date: Sat, 16 Oct 2021 15:18:38 +0100
> 
> This driver adds support for the pinctrl / GPIO hardware found
> on some Apple SoCs.

Hi Joey,

> Co-developed-by: Stan Skowronek <stan@corellium.com>
> Signed-off-by: Stan Skowronek <stan@corellium.com>
> Signed-off-by: Joey Gouly <joey.gouly@arm.com>
> ---
>  drivers/pinctrl/Kconfig              |  16 +
>  drivers/pinctrl/Makefile             |   1 +
>  drivers/pinctrl/pinctrl-apple-gpio.c | 564 +++++++++++++++++++++++++++
>  3 files changed, 581 insertions(+)
>  create mode 100644 drivers/pinctrl/pinctrl-apple-gpio.c
> 
> diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
> index 31921108e456..6a961d5f8726 100644
> --- a/drivers/pinctrl/Kconfig
> +++ b/drivers/pinctrl/Kconfig
> @@ -31,6 +31,22 @@ config DEBUG_PINCTRL
>  	help
>  	  Say Y here to add some extra checks and diagnostics to PINCTRL calls.
>  
> +config PINCTRL_APPLE_GPIO
> +	tristate "Apple SoC GPIO pin controller driver"
> +	depends on ARCH_APPLE
> +	select PINMUX
> +	select GPIOLIB
> +	select GPIOLIB_IRQCHIP
> +	select GENERIC_PINCTRL_GROUPS
> +	select GENERIC_PINMUX_FUNCTIONS
> +	select OF_GPIO
> +	help
> +	  This is the driver for the GPIO controller found on Apple ARM SoCs,
> +	  including M1.
> +
> +	  This driver can also be built as a module. If so, the module
> +	  will be called pinctrl-apple-gpio.
> +
>  config PINCTRL_ARTPEC6
>  	bool "Axis ARTPEC-6 pin controller driver"
>  	depends on MACH_ARTPEC6
> diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
> index 200073bcc2c1..5e63de2ffcf4 100644
> --- a/drivers/pinctrl/Makefile
> +++ b/drivers/pinctrl/Makefile
> @@ -8,6 +8,7 @@ obj-$(CONFIG_PINMUX)		+= pinmux.o
>  obj-$(CONFIG_PINCONF)		+= pinconf.o
>  obj-$(CONFIG_OF)		+= devicetree.o
>  obj-$(CONFIG_GENERIC_PINCONF)	+= pinconf-generic.o
> +obj-$(CONFIG_PINCTRL_APPLE_GPIO) += pinctrl-apple-gpio.o
>  obj-$(CONFIG_PINCTRL_ARTPEC6)	+= pinctrl-artpec6.o
>  obj-$(CONFIG_PINCTRL_AS3722)	+= pinctrl-as3722.o
>  obj-$(CONFIG_PINCTRL_AXP209)	+= pinctrl-axp209.o
> diff --git a/drivers/pinctrl/pinctrl-apple-gpio.c b/drivers/pinctrl/pinctrl-apple-gpio.c
> new file mode 100644
> index 000000000000..df93eb12069d
> --- /dev/null
> +++ b/drivers/pinctrl/pinctrl-apple-gpio.c
> @@ -0,0 +1,564 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Apple SoC pinctrl+GPIO+external IRQ driver
> + *
> + * Copyright (C) 2021 The Asahi Linux Contributors
> + * Copyright (C) 2020 Corellium LLC
> + *
> + * Based on: pinctrl-pistachio.c
> + * Copyright (C) 2014 Imagination Technologies Ltd.
> + * Copyright (C) 2014 Google, Inc.
> + */
> +
> +#include <dt-bindings/pinctrl/apple.h>
> +#include <linux/gpio/driver.h>
> +#include <linux/interrupt.h>
> +#include <linux/irq.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_irq.h>
> +#include <linux/pinctrl/pinctrl.h>
> +#include <linux/pinctrl/pinmux.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +
> +#include "pinctrl-utils.h"
> +#include "core.h"
> +#include "pinmux.h"
> +
> +struct apple_gpio_irq_data {
> +	struct apple_gpio_pinctrl *pctl;
> +	int irqgrp;
> +};
> +
> +struct apple_gpio_pinctrl {
> +	struct device *dev;
> +	struct pinctrl_dev *pctldev;
> +
> +	void __iomem *base;
> +	struct regmap *map;
> +	unsigned int nirqgrps;
> +
> +	struct pinctrl_desc pinctrl_desc;
> +	struct gpio_chip gpio_chip;
> +	struct irq_chip irq_chip;
> +};
> +
> +#define REG_GPIO(x)          (4 * (x))
> +#define REG_GPIOx_DATA       BIT(0)
> +#define REG_GPIOx_MODE       GENMASK(3, 1)
> +#define REG_GPIOx_OUT        1
> +#define REG_GPIOx_IN_IRQ_HI  2
> +#define REG_GPIOx_IN_IRQ_LO  3
> +#define REG_GPIOx_IN_IRQ_UP  4
> +#define REG_GPIOx_IN_IRQ_DN  5
> +#define REG_GPIOx_IN_IRQ_ANY 6
> +#define REG_GPIOx_IN_IRQ_OFF 7
> +#define REG_GPIOx_PERIPH     GENMASK(6, 5)
> +#define REG_GPIOx_PULL       GENMASK(8, 7)
> +#define REG_GPIOx_PULL_OFF   0
> +#define REG_GPIOx_PULL_DOWN  1
> +#define REG_GPIOx_PULL_UP_STRONG 2
> +#define REG_GPIOx_PULL_UP    3
> +#define REG_GPIOx_INPUT_ENABLE BIT(9)
> +#define REG_GPIOx_DRIVE_STRENGTH0 GENMASK(11, 10)
> +#define REG_GPIOx_SCHMITT    BIT(15)
> +#define REG_GPIOx_GRP        GENMASK(18, 16)
> +#define REG_GPIOx_LOCK       BIT(21)
> +#define REG_GPIOx_DRIVE_STRENGTH1 GENMASK(23, 22)
> +#define REG_IRQ(g, x)        (0x800 + 0x40 * (g) + 4 * ((x) >> 5))
> +
> +struct regmap_config regmap_config = {
> +	.reg_bits = 32,
> +	.val_bits = 32,
> +	.reg_stride = 4,
> +	.cache_type = REGCACHE_FLAT,
> +	.max_register = 512 * sizeof(uint32_t),
> +	.num_reg_defaults_raw = 512,
> +	.use_relaxed_mmio = true
> +};
> +
> +// No locking needed to mask/unmask IRQs as the interrupt mode is per pin-register.
> +static void apple_gpio_set_reg(struct apple_gpio_pinctrl *pctl,
> +			       unsigned int pin, uint32_t mask, uint32_t value)
> +{
> +	regmap_update_bits(pctl->map, REG_GPIO(pin), mask, value);
> +}
> +
> +static uint32_t apple_gpio_get_reg(struct apple_gpio_pinctrl *pctl,
> +				   unsigned int pin)
> +{
> +	uint32_t val = 0;
> +
> +	regmap_read(pctl->map, REG_GPIO(pin), &val);
> +	return val;
> +}
> +
> +/* Pin controller functions */
> +
> +static int apple_gpio_dt_node_to_map(struct pinctrl_dev *pctldev,
> +				     struct device_node *node,
> +				     struct pinctrl_map **map,
> +				     unsigned *num_maps)
> +{
> +	unsigned reserved_maps;
> +	struct apple_gpio_pinctrl *pctl;
> +	u32 pinfunc, pin, func;
> +	int num_pins, i, ret;
> +	const char *group_name;
> +	const char *function_name;
> +
> +	*map = NULL;
> +	*num_maps = 0;
> +	reserved_maps = 0;
> +
> +	pctl = pinctrl_dev_get_drvdata(pctldev);
> +
> +	ret = of_property_count_u32_elems(node, "pinmux");
> +	if (ret <= 0) {
> +		dev_err(pctl->dev,
> +			"missing or empty pinmux property in node %pOFn.\n",
> +			node);
> +		return -EINVAL;
> +	}
> +
> +	num_pins = ret;
> +
> +	ret = pinctrl_utils_reserve_map(pctldev, map, &reserved_maps, num_maps,
> +					num_pins);
> +	if (ret)
> +		return ret;
> +
> +	for (i = 0; i < num_pins; i++) {
> +		ret = of_property_read_u32_index(node, "pinmux", i, &pinfunc);
> +		if (ret)
> +			goto free_map;
> +
> +		pin = APPLE_PIN(pinfunc);
> +		func = APPLE_FUNC(pinfunc);
> +
> +		if (func >= pinmux_generic_get_function_count(pctldev)) {
> +			ret = -EINVAL;
> +			goto free_map;
> +		}
> +
> +		group_name = pinctrl_generic_get_group_name(pctldev, pin);
> +		function_name =
> +			pinmux_generic_get_function_name(pctl->pctldev, func);
> +		ret = pinctrl_utils_add_map_mux(pctl->pctldev, map,
> +						&reserved_maps, num_maps,
> +						group_name, function_name);
> +		if (ret)
> +			goto free_map;
> +	}
> +
> +free_map:
> +	if (ret < 0) {
> +		pinctrl_utils_free_map(pctldev, *map, *num_maps);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct pinctrl_ops apple_gpio_pinctrl_ops = {
> +	.get_groups_count = pinctrl_generic_get_group_count,
> +	.get_group_name = pinctrl_generic_get_group_name,
> +	.get_group_pins = pinctrl_generic_get_group_pins,
> +	.dt_node_to_map = apple_gpio_dt_node_to_map,
> +	.dt_free_map = pinctrl_utils_free_map,
> +};
> +
> +/* Pin multiplexer functions */
> +
> +static int apple_gpio_pinmux_enable(struct pinctrl_dev *pctldev, unsigned func,
> +				    unsigned group)
> +{
> +	struct apple_gpio_pinctrl *pctl = pinctrl_dev_get_drvdata(pctldev);
> +
> +	apple_gpio_set_reg(
> +		pctl, group, REG_GPIOx_PERIPH | REG_GPIOx_INPUT_ENABLE,
> +		FIELD_PREP(REG_GPIOx_PERIPH, func) | REG_GPIOx_INPUT_ENABLE);
> +
> +	return 0;
> +}
> +
> +static const struct pinmux_ops apple_gpio_pinmux_ops = {
> +	.get_functions_count = pinmux_generic_get_function_count,
> +	.get_function_name = pinmux_generic_get_function_name,
> +	.get_function_groups = pinmux_generic_get_function_groups,
> +	.set_mux = apple_gpio_pinmux_enable,
> +	.strict = true,
> +};
> +
> +/* GPIO chip functions */
> +
> +static int apple_gpio_gpio_get_direction(struct gpio_chip *chip,
> +					 unsigned int offset)
> +{
> +	struct apple_gpio_pinctrl *pctl = gpiochip_get_data(chip);
> +	unsigned int reg = apple_gpio_get_reg(pctl, offset);
> +
> +	return (FIELD_GET(REG_GPIOx_MODE, reg) == REG_GPIOx_OUT) ?
> +		       GPIO_LINE_DIRECTION_OUT :
> +			     GPIO_LINE_DIRECTION_IN;
> +}
> +
> +static int apple_gpio_gpio_get(struct gpio_chip *chip, unsigned offset)
> +{
> +	struct apple_gpio_pinctrl *pctl = gpiochip_get_data(chip);
> +	unsigned int reg = apple_gpio_get_reg(pctl, offset);
> +
> +	// If this is an input GPIO, read the actual value (not the cached regmap value)
> +	if (FIELD_GET(REG_GPIOx_MODE, reg) != REG_GPIOx_OUT)
> +		reg = readl_relaxed(pctl->base + REG_GPIO(offset));
> +
> +	return !!(reg & REG_GPIOx_DATA);
> +}
> +
> +static void apple_gpio_gpio_set(struct gpio_chip *chip, unsigned int offset,
> +				int value)
> +{
> +	struct apple_gpio_pinctrl *pctl = gpiochip_get_data(chip);
> +
> +	apple_gpio_set_reg(pctl, offset, REG_GPIOx_DATA,
> +			   value ? REG_GPIOx_DATA : 0);
> +}
> +
> +static int apple_gpio_gpio_direction_input(struct gpio_chip *chip,
> +					   unsigned int offset)
> +{
> +	struct apple_gpio_pinctrl *pctl = gpiochip_get_data(chip);
> +
> +	apple_gpio_set_reg(pctl, offset,
> +			   REG_GPIOx_PERIPH | REG_GPIOx_MODE | REG_GPIOx_DATA |
> +				   REG_GPIOx_INPUT_ENABLE,
> +			   FIELD_PREP(REG_GPIOx_MODE, REG_GPIOx_IN_IRQ_OFF) |
> +				   REG_GPIOx_INPUT_ENABLE);
> +	return 0;
> +}
> +
> +static int apple_gpio_gpio_direction_output(struct gpio_chip *chip,
> +					    unsigned int offset, int value)
> +{
> +	struct apple_gpio_pinctrl *pctl = gpiochip_get_data(chip);
> +
> +	apple_gpio_set_reg(pctl, offset,
> +			   REG_GPIOx_PERIPH | REG_GPIOx_MODE | REG_GPIOx_DATA,
> +			   FIELD_PREP(REG_GPIOx_MODE, REG_GPIOx_OUT) |
> +				   (value ? REG_GPIOx_DATA : 0));
> +	return 0;
> +}
> +
> +/* IRQ chip functions */
> +
> +static void apple_gpio_gpio_irq_ack(struct irq_data *data)
> +{
> +	struct apple_gpio_pinctrl *pctl =
> +		gpiochip_get_data(irq_data_get_irq_chip_data(data));
> +	unsigned int irqgrp =
> +		FIELD_GET(REG_GPIOx_GRP, apple_gpio_get_reg(pctl, data->hwirq));
> +
> +	writel(BIT(data->hwirq & 31),
> +	       pctl->base + REG_IRQ(irqgrp, data->hwirq));
> +}
> +
> +static int apple_gpio_irq_type(unsigned int type)
> +{
> +	switch (type & IRQ_TYPE_SENSE_MASK) {
> +	case IRQ_TYPE_EDGE_RISING:
> +		return REG_GPIOx_IN_IRQ_UP;
> +	case IRQ_TYPE_EDGE_FALLING:
> +		return REG_GPIOx_IN_IRQ_DN;
> +	case IRQ_TYPE_EDGE_BOTH:
> +		return REG_GPIOx_IN_IRQ_ANY;
> +	case IRQ_TYPE_LEVEL_HIGH:
> +		return REG_GPIOx_IN_IRQ_HI;
> +	case IRQ_TYPE_LEVEL_LOW:
> +		return REG_GPIOx_IN_IRQ_LO;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static void apple_gpio_gpio_irq_mask(struct irq_data *data)
> +{
> +	struct apple_gpio_pinctrl *pctl =
> +		gpiochip_get_data(irq_data_get_irq_chip_data(data));
> +	apple_gpio_set_reg(pctl, data->hwirq, REG_GPIOx_MODE,
> +			   FIELD_PREP(REG_GPIOx_MODE, REG_GPIOx_IN_IRQ_OFF));
> +}
> +
> +static void apple_gpio_gpio_irq_unmask(struct irq_data *data)
> +{
> +	struct apple_gpio_pinctrl *pctl =
> +		gpiochip_get_data(irq_data_get_irq_chip_data(data));
> +	int irqtype = apple_gpio_irq_type(irqd_get_trigger_type(data));
> +
> +	if (WARN_ON(irqtype < 0))
> +		return;
> +	apple_gpio_set_reg(pctl, data->hwirq, REG_GPIOx_MODE,
> +			   FIELD_PREP(REG_GPIOx_MODE, irqtype));
> +}
> +
> +static unsigned int apple_gpio_gpio_irq_startup(struct irq_data *data)
> +{
> +	struct gpio_chip *chip = irq_data_get_irq_chip_data(data);
> +	struct apple_gpio_pinctrl *pctl = gpiochip_get_data(chip);
> +
> +	apple_gpio_set_reg(pctl, data->hwirq, REG_GPIOx_GRP,
> +			   FIELD_PREP(REG_GPIOx_GRP, 0));
> +
> +	apple_gpio_gpio_direction_input(chip, data->hwirq);
> +	apple_gpio_gpio_irq_unmask(data);
> +
> +	return 0;
> +}
> +
> +static int apple_gpio_gpio_irq_set_type(struct irq_data *data,
> +					unsigned int type)
> +{
> +	struct apple_gpio_pinctrl *pctl =
> +		gpiochip_get_data(irq_data_get_irq_chip_data(data));
> +	int irqtype = apple_gpio_irq_type(type);
> +
> +	if (irqtype < 0)
> +		return irqtype;
> +
> +	irqd_set_trigger_type(data, type);
> +
> +	apple_gpio_set_reg(pctl, data->hwirq, REG_GPIOx_MODE,
> +			   FIELD_PREP(REG_GPIOx_MODE, irqtype));
> +
> +	if (type & IRQ_TYPE_LEVEL_MASK)
> +		irq_set_handler_locked(data, handle_level_irq);
> +	else
> +		irq_set_handler_locked(data, handle_edge_irq);
> +	return 0;
> +}
> +
> +static void apple_gpio_gpio_irq_handler(struct irq_desc *desc)
> +{
> +	struct apple_gpio_irq_data *data = irq_desc_get_handler_data(desc);
> +	struct apple_gpio_pinctrl *pctl = data->pctl;
> +	struct gpio_chip *gc = &pctl->gpio_chip;
> +	struct irq_chip *chip = irq_desc_get_chip(desc);
> +	unsigned int irqgrp = data->irqgrp;
> +	unsigned int pinh, pinl;
> +	unsigned long pending;
> +
> +	chained_irq_enter(chip, desc);
> +	for (pinh = 0; pinh < gc->ngpio; pinh += 32) {
> +		pending = readl(pctl->base + REG_IRQ(irqgrp, pinh));
> +		for_each_set_bit(pinl, &pending, 32)
> +			generic_handle_domain_irq(gc->irq.domain, pinh + pinl);
> +	}
> +	chained_irq_exit(chip, desc);
> +}
> +
> +/* Probe & register */
> +
> +static int apple_gpio_gpio_register(struct apple_gpio_pinctrl *pctl)
> +{
> +	struct device_node *node = pctl->dev->of_node;
> +	struct gpio_irq_chip *girq;
> +	struct apple_gpio_irq_data **irq_data;
> +	int i;
> +	int ret;
> +
> +	if (!of_find_property(node, "gpio-controller", NULL)) {
> +		dev_err(pctl->dev,
> +			"Apple GPIO must have 'gpio-controller' property.\n");
> +		return -ENODEV;
> +	}
> +
> +	pctl->gpio_chip.label = dev_name(pctl->dev);
> +	pctl->gpio_chip.request = gpiochip_generic_request;
> +	pctl->gpio_chip.free = gpiochip_generic_free;
> +	pctl->gpio_chip.get_direction = apple_gpio_gpio_get_direction;
> +	pctl->gpio_chip.direction_input = apple_gpio_gpio_direction_input;
> +	pctl->gpio_chip.direction_output = apple_gpio_gpio_direction_output;
> +	pctl->gpio_chip.get = apple_gpio_gpio_get;
> +	pctl->gpio_chip.set = apple_gpio_gpio_set;
> +	pctl->gpio_chip.base = -1;
> +	pctl->gpio_chip.ngpio = pctl->pinctrl_desc.npins;
> +	pctl->gpio_chip.parent = pctl->dev;
> +	pctl->gpio_chip.of_node = node;
> +
> +	if (of_property_read_bool(node, "interrupt-controller")) {
> +		ret = platform_irq_count(to_platform_device(pctl->dev));
> +		if (ret < 0)
> +			return ret;
> +
> +		pctl->nirqgrps = ret;
> +
> +		pctl->irq_chip.name = "Apple-GPIO";
> +		pctl->irq_chip.irq_startup = apple_gpio_gpio_irq_startup;
> +		pctl->irq_chip.irq_ack = apple_gpio_gpio_irq_ack;
> +		pctl->irq_chip.irq_mask = apple_gpio_gpio_irq_mask;
> +		pctl->irq_chip.irq_unmask = apple_gpio_gpio_irq_unmask;
> +		pctl->irq_chip.irq_set_type = apple_gpio_gpio_irq_set_type;
> +
> +		girq = &pctl->gpio_chip.irq;
> +		girq->chip = &pctl->irq_chip;
> +		girq->parent_handler = apple_gpio_gpio_irq_handler;
> +		girq->num_parents = pctl->nirqgrps;
> +
> +		girq->parents = kmalloc_array(
> +			pctl->nirqgrps, sizeof(*girq->parents), GFP_KERNEL);
> +		if (!girq->parents)
> +			return -ENOMEM;
> +
> +		for (i = 0; i < pctl->nirqgrps; i++) {
> +			ret = platform_get_irq(to_platform_device(pctl->dev),
> +					       i);
> +			if (ret < 0) {
> +				return dev_err_probe(pctl->dev, ret,
> +						     "Failed to map IRQ %d\n",
> +						     i);
> +			}
> +			girq->parents[i] = ret;
> +		}
> +
> +		irq_data = kmalloc_array(pctl->nirqgrps, sizeof(*irq_data),
> +					 GFP_KERNEL);
> +		if (!irq_data) {
> +			kfree(girq->parents);
> +			return -ENOMEM;
> +		}
> +		for (i = 0; i < pctl->nirqgrps; i++) {
> +			irq_data[i] = devm_kzalloc(
> +				pctl->dev, sizeof(*irq_data[i]), GFP_KERNEL);
> +			irq_data[i]->pctl = pctl;
> +			irq_data[i]->irqgrp = i;
> +		}
> +
> +		girq->parent_handler_data_array = (void **)irq_data;
> +		girq->per_parent_data = true;
> +		girq->default_type = IRQ_TYPE_NONE;
> +		girq->handler = handle_level_irq;
> +	}
> +
> +	ret = devm_gpiochip_add_data(pctl->dev, &pctl->gpio_chip, pctl);
> +
> +	if (of_property_read_bool(node, "interrupt-controller")) {
> +		kfree(girq->parents);
> +		kfree(irq_data);
> +	}
> +
> +	return ret;
> +}
> +
> +static int apple_gpio_pinctrl_probe(struct platform_device *pdev)
> +{
> +	struct apple_gpio_pinctrl *pctl;
> +	struct pinctrl_pin_desc *pins;
> +	unsigned int npins;
> +	const char **pin_names;
> +	unsigned int *pin_nums;
> +	unsigned int i;
> +	int res;
> +
> +	pctl = devm_kzalloc(&pdev->dev, sizeof(*pctl), GFP_KERNEL);
> +	if (!pctl)
> +		return -ENOMEM;
> +	pctl->dev = &pdev->dev;
> +	dev_set_drvdata(&pdev->dev, pctl);
> +
> +	if (of_property_read_u32(pdev->dev.of_node, "apple,npins", &npins)) {
> +		dev_err(&pdev->dev, "apple,npins property not found\n");
> +		return -EINVAL;
> +	}
> +
> +	pins = devm_kmalloc_array(&pdev->dev, npins, sizeof(pins[0]),
> +				  GFP_KERNEL);
> +	if (!pins)
> +		return -ENOMEM;
> +	pin_names = devm_kmalloc_array(&pdev->dev, npins, sizeof(pin_names[0]),
> +				       GFP_KERNEL);
> +	if (!pin_names)
> +		return -ENOMEM;
> +	pin_nums = devm_kmalloc_array(&pdev->dev, npins, sizeof(pin_nums[0]),
> +				      GFP_KERNEL);
> +	if (!pin_nums)
> +		return -ENOMEM;
> +
> +	pctl->base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(pctl->base))
> +		return PTR_ERR(pctl->base);
> +
> +	pctl->map =
> +		devm_regmap_init_mmio(&pdev->dev, pctl->base, &regmap_config);
> +	if (IS_ERR(pctl->map)) {
> +		dev_err(&pdev->dev, "Failed to create regmap\n");
> +		return PTR_ERR(pctl->map);
> +	}
> +
> +	for (i = 0; i < npins; i++) {
> +		pins[i].number = i;
> +		pins[i].name =
> +			devm_kasprintf(&pdev->dev, GFP_KERNEL, "PIN%u", i);
> +		pins[i].drv_data = pctl;
> +		pin_names[i] = pins[i].name;
> +		pin_nums[i] = i;
> +	}
> +
> +	pctl->pinctrl_desc.name = dev_name(pctl->dev);
> +	pctl->pinctrl_desc.pins = pins;
> +	pctl->pinctrl_desc.npins = npins;
> +	pctl->pinctrl_desc.pctlops = &apple_gpio_pinctrl_ops;
> +	pctl->pinctrl_desc.pmxops = &apple_gpio_pinmux_ops;
> +
> +	pctl->pctldev =
> +		devm_pinctrl_register(&pdev->dev, &pctl->pinctrl_desc, pctl);
> +	if (IS_ERR(pctl->pctldev)) {
> +		dev_err(&pdev->dev, "Failed to register pinctrl device.\n");
> +		return PTR_ERR(pctl->pctldev);
> +	}
> +
> +	for (i = 0; i < npins; i++) {
> +		res = pinctrl_generic_add_group(pctl->pctldev, pins[i].name,
> +						pin_nums + i, 1, pctl);
> +		if (res < 0) {
> +			dev_err(pctl->dev, "Failed to register group.");
> +			return res;
> +		}
> +	}
> +
> +	res = pinmux_generic_add_function(pctl->pctldev, "gpio", pin_names,
> +					  npins, pctl);
> +	if (res < 0) {
> +		dev_err(pctl->dev, "Failed to register function.");
> +		return res;
> +	}
> +
> +	res = pinmux_generic_add_function(pctl->pctldev, "periph", pin_names,
> +					  npins, pctl);
> +	if (res < 0) {
> +		dev_err(pctl->dev, "Failed to register function.");
> +		return res;
> +	}
> +
> +	return apple_gpio_gpio_register(pctl);
> +}
> +
> +static const struct of_device_id apple_gpio_pinctrl_of_match[] = {
> +	{ .compatible = "apple,t8103-pinctrl", },
> +	{ .compatible = "apple,pinctrl", },

No need to list both.  Just "apple,pinctrl" should be enough.

> +	{ }
> +};
> +
> +static struct platform_driver apple_gpio_pinctrl_driver = {
> +	.driver = {
> +		.name = "apple-gpio-pinctrl",
> +		.of_match_table = apple_gpio_pinctrl_of_match,
> +		.suppress_bind_attrs = true,
> +	},
> +	.probe = apple_gpio_pinctrl_probe,
> +};
> +module_platform_driver(apple_gpio_pinctrl_driver);
> +
> +MODULE_DESCRIPTION("Apple pinctrl/GPIO driver");
> +MODULE_AUTHOR("Stan Skowronek <stan@corellium.com>");
> +MODULE_AUTHOR("Joey Gouly <joey.gouly@arm.com>");
> +MODULE_LICENSE("GPL v2");
> -- 
> 2.17.1
> 
> 

Re: [PATCH v3 4/5] pinctrl: add pinctrl/GPIO driver for Apple SoCs

[Linus Walleij]

On Sat, Oct 16, 2021 at 4:18 PM Joey Gouly <joey.gouly@arm.com> wrote:

> This driver adds support for the pinctrl / GPIO hardware found
> on some Apple SoCs.
>
> Co-developed-by: Stan Skowronek <stan@corellium.com>
> Signed-off-by: Stan Skowronek <stan@corellium.com>
> Signed-off-by: Joey Gouly <joey.gouly@arm.com>

Hi Joey, this looks really good. I started to write a reply but
noticed that all my comments are already covered by
Marc Z so I scrapped it: do what Marc says.

This is interesting:

> +#define REG_GPIOx_PULL_OFF   0
> +#define REG_GPIOx_PULL_DOWN  1
> +#define REG_GPIOx_PULL_UP_STRONG 2
> +#define REG_GPIOx_PULL_UP    3

Pull-up strong! Nice that you found these details. We don't have a
generic pinconf binding for that but possibly the bias-pull-up
argument can be used if we know how many ohms each is
(preferred).

But this is no problem for the moment because pin config is
for implementing later (I assume). I bet this is going to be important
to get right as well as it usually affects the power dissipation in
suspend-to-RAM.

> +static int apple_gpio_pinmux_enable(struct pinctrl_dev *pctldev, unsigned func,
> +                                   unsigned group)
(...)
> +       .set_mux = apple_gpio_pinmux_enable,

What about just naming it apple_gpio_pinmux_set()?

Next iteration should be good to go I guess!

Yours,
Linus Walleij

[PATCH v3 5/5] MAINTAINERS: add pinctrl-apple-gpio to ARM/APPLE MACHINE

[Joey Gouly]

Add the Apple SoC pinctrl driver to the ARM/APPLE MACHINE entry
in MAINTAINERS.

Signed-off-by: Joey Gouly <joey.gouly@arm.com>
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index ca6d6fde85cf..e83e992b8ada 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1722,6 +1722,7 @@ F:	Documentation/devicetree/bindings/interrupt-controller/apple,aic.yaml
 F:	Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml
 F:	arch/arm64/boot/dts/apple/
 F:	drivers/irqchip/irq-apple-aic.c
+F:	drivers/pinctrl/pinctrl-apple-gpio.c
 F:	include/dt-bindings/interrupt-controller/apple-aic.h
 F:	include/dt-bindings/pinctrl/apple.h
 
-- 
2.17.1