diff mbox series

[v2,3/3] pinctrl: add pinctrl/GPIO driver for Apple SoCs

Message ID 20211001191209.29988-4-joey.gouly@arm.com
State New
Headers show
Series pinctrl/GPIO driver for Apple SoCs | expand

Commit Message

Joey Gouly Oct. 1, 2021, 7:12 p.m. UTC
From: Stan Skowronek <stan@corellium.com>

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

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

Comments

Linus Walleij Oct. 3, 2021, 10:35 p.m. UTC | #1
Hi Joey!

over all this driver is much improved and using a lot of stock functions
in the pin control core and getting really clean and compact.
I have one major nit below:

On Fri, Oct 1, 2021 at 9:12 PM Joey Gouly <joey.gouly@arm.com> wrote:

> +struct apple_gpio_pinctrl {
> +       // Shadow the pin values, the REG_GPIOx_DATA bit can read back stale values.
> +       u32 *pin_shadow;
(...)
> +// 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 clr, uint32_t set)
> +{
> +       void __iomem *ppin = pctl->base + REG_GPIO(pin);
> +       uint32_t prev, cfg;
> +
> +       prev = pctl->pin_shadow[pin];
> +       cfg = (prev & ~clr) | set;
> +
> +       if (!(prev & REG_GPIOx_CFG_DONE))
> +               writel_relaxed(cfg & ~REG_GPIOx_CFG_DONE, ppin);
> +       writel_relaxed(cfg, ppin);
> +       pctl->pin_shadow[pin] = cfg;
> +}

Are you not simply reinventing regmap-mmio here?

Keeping shadows of registers including write-only registers
is exactly what regmap does.

Check it out, if in doubt consult Mark Brown, I'm pretty
sure we can add what you need to regmap if it is missing.

> +static uint32_t apple_gpio_get_reg(struct apple_gpio_pinctrl *pctl,
> +                                  unsigned int pin)
> +{
> +       return readl_relaxed(pctl->base + REG_GPIO(pin));
> +}

If you use regmap-mmio I am pretty sure this will also
return the right value for the shadowed registers which it
currently does not IIUC.

> +               apple_gpio_set_reg(pctl, group, 0,
> +                                  REG_GPIOx_PERIPH | REG_GPIOx_CFG_DONE);
> +       else
> +               apple_gpio_set_reg(pctl, group, REG_GPIOx_PERIPH,
> +                                  REG_GPIOx_CFG_DONE);

I think all calls to apple_gpio_set_reg() could be replaced with
regmap_update_bits() or similar.

Yours,
Linus Walleij
Hector Martin Oct. 4, 2021, 3:33 a.m. UTC | #2
On 02/10/2021 04.12, Joey Gouly wrote:
> +#define REG_GPIO(x)          (4 * (x))
> +#define REG_GPIOx_DATA       BIT(0)
> +#define REG_GPIOx_MODE_MASK  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     BIT(5)
> +#define REG_GPIOx_CFG_DONE   BIT(9)
> +#define REG_GPIOx_GRP_MASK   GENMASK(18, 16)
> +#define REG_IRQ(g, x)        (0x800 + 0x40 * (g) + 4 * ((x) >> 5))

Can we update these defines with the correct definitions and names we 
figured out the other day and add the missing ones? We now know a bunch 
of these are wrong (e.g. CFG_DONE is INPUT_ENABLE, PERIPH should be two 
bits, we're missing pull-up control, drive strength, schmitt trigger and 
lock bits). Even if we don't implement all the features in the driver 
yet, we should have all the register bit defines for documentation 
purposes at least.

> +	if (!(prev & REG_GPIOx_CFG_DONE))
> +		writel_relaxed(cfg & ~REG_GPIOx_CFG_DONE, ppin);
> +	writel_relaxed(cfg, ppin);

We already determined this dance doesn't make any sense; if we want to 
change the pin config before enabling the input buffer (whether this 
serves any purpose at all is an open question) then that should be 
handled in the upper code responsible for enabling/disabling the input 
buffer, not in the core register wrappers.

> +	if (func)
> +		apple_gpio_set_reg(pctl, group, 0,
> +				   REG_GPIOx_PERIPH | REG_GPIOx_CFG_DONE);
> +	else
> +		apple_gpio_set_reg(pctl, group, REG_GPIOx_PERIPH,
> +				   REG_GPIOx_CFG_DONE);

Func is two bits (4 functions) :)

> +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,
> +			   REG_GPIOx_CFG_DONE | (value & REG_GPIOx_DATA));
> +}

`value ? REG_GPIOx_DATA : 0` please, otherwise this makes assumptions 
about value always being 1 and REG_GPIOx_DATA being the LSB.

Also as we now know, REG_GPIOx_CFG_DONE is nonsense and doesn't belong 
here. Let's drop the cargo cult and drive the hardware based on how it 
works, not how macOS or Corellium decided to do things.

> +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_MODE_MASK | REG_GPIOx_DATA,
> +			   FIELD_PREP(REG_GPIOx_MODE_MASK,
> +				      REG_GPIOx_IN_IRQ_OFF) |
> +				   REG_GPIOx_CFG_DONE);

Is hardcoding IRQ_OFF correct here? Shouldn't this be getting the 
intended IRQ state from somewhere, or is it always guaranteed that that 
gets set later?

> +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_DATA,
> +			   FIELD_PREP(REG_GPIOx_MODE_MASK, REG_GPIOx_OUT) |
> +				   (value & REG_GPIOx_DATA) |
> +				   REG_GPIOx_CFG_DONE);

I actually wonder if we should even bother turning on the input buffer 
for output pins, given we're shadowing the value anyway. Seems 
unnecessary and might save a few nanowatts.

Also, why is this clearing the peripheral (yet direction_input isn't)?

> +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_MASK,
> +			   FIELD_PREP(REG_GPIOx_MODE_MASK,
> +				      REG_GPIOx_IN_IRQ_OFF) |
> +				   REG_GPIOx_CFG_DONE);
> +}

-REG_GPIOx_CFG_DONE please

> +
> +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));
> +	u32 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_MASK,
> +			   FIELD_PREP(REG_GPIOx_MODE_MASK, irqtype) |
> +				   REG_GPIOx_CFG_DONE);

Ditto

> +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_MASK,
> +			   FIELD_PREP(REG_GPIOx_GRP_MASK, 0));

I guess we're only using a single IRQ group right now?

The driver structure looks good (though see the regmap suggestion from 
Linus). Let's just get the actual hardware part right. I didn't spend a 
couple hours poking register bits with a multimeter, a scope, and a 
breadboard for nothing ;)
Joey Gouly Oct. 14, 2021, 8:24 p.m. UTC | #3
Hi!

On Mon, Oct 04, 2021 at 12:33:15PM +0900, Hector Martin wrote:
> On 02/10/2021 04.12, Joey Gouly wrote:
> > +#define REG_GPIO(x)          (4 * (x))
> > +#define REG_GPIOx_DATA       BIT(0)
> > +#define REG_GPIOx_MODE_MASK  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     BIT(5)
> > +#define REG_GPIOx_CFG_DONE   BIT(9)
> > +#define REG_GPIOx_GRP_MASK   GENMASK(18, 16)
> > +#define REG_IRQ(g, x)        (0x800 + 0x40 * (g) + 4 * ((x) >> 5))
> 
> Can we update these defines with the correct definitions and names we
> figured out the other day and add the missing ones? We now know a bunch of
> these are wrong (e.g. CFG_DONE is INPUT_ENABLE, PERIPH should be two bits,
> we're missing pull-up control, drive strength, schmitt trigger and lock
> bits). Even if we don't implement all the features in the driver yet, we
> should have all the register bit defines for documentation purposes at
> least.

Done.

> 
> > +	if (!(prev & REG_GPIOx_CFG_DONE))
> > +		writel_relaxed(cfg & ~REG_GPIOx_CFG_DONE, ppin);
> > +	writel_relaxed(cfg, ppin);
> 
> We already determined this dance doesn't make any sense; if we want to
> change the pin config before enabling the input buffer (whether this serves
> any purpose at all is an open question) then that should be handled in the
> upper code responsible for enabling/disabling the input buffer, not in the
> core register wrappers.

Removed this.

[...]

> > +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,
> > +			   REG_GPIOx_CFG_DONE | (value & REG_GPIOx_DATA));
> > +}
> 
> `value ? REG_GPIOx_DATA : 0` please, otherwise this makes assumptions about
> value always being 1 and REG_GPIOx_DATA being the LSB.

I like that.

> > +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_MODE_MASK | REG_GPIOx_DATA,
> > +			   FIELD_PREP(REG_GPIOx_MODE_MASK,
> > +				      REG_GPIOx_IN_IRQ_OFF) |
> > +				   REG_GPIOx_CFG_DONE);
> 
> Is hardcoding IRQ_OFF correct here? Shouldn't this be getting the intended
> IRQ state from somewhere, or is it always guaranteed that that gets set
> later?

As far as I can tell, the only way to setup an IRQ on a pin/gpio goes
via the apple_gpio_irq_startup function, which calls
`apple_gpio_gpio_direction_input` and then `apple_gpio_gpio_irq_unmask`
which sets the correct IRQ_ value.

> 
> > +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_DATA,
> > +			   FIELD_PREP(REG_GPIOx_MODE_MASK, REG_GPIOx_OUT) |
> > +				   (value & REG_GPIOx_DATA) |
> > +				   REG_GPIOx_CFG_DONE);
> 
> I actually wonder if we should even bother turning on the input buffer for
> output pins, given we're shadowing the value anyway. Seems unnecessary and
> might save a few nanowatts.
> 
> Also, why is this clearing the peripheral (yet direction_input isn't)?

I've removed setting the input buffer here (CFG_DONE, or INPUT_ENABLE as
it is now named in v3).

That was just an oversight, I'm clearing it in direction_input too now.

[...]

> > +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_MASK,
> > +			   FIELD_PREP(REG_GPIOx_GRP_MASK, 0));
> 
> I guess we're only using a single IRQ group right now?
Yep. The irq handler should already work with different groups. So to
support different groups we'd have to get the group number here somehow.

Thanks,
Joey
diff mbox series

Patch

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
 
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..cfec37e3627a
--- /dev/null
+++ b/drivers/pinctrl/pinctrl-apple-gpio.c
@@ -0,0 +1,561 @@ 
+// 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/clk.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 "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;
+
+	// Shadow the pin values, the REG_GPIOx_DATA bit can read back stale values.
+	u32 *pin_shadow;
+
+	void __iomem *base;
+	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_MASK  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     BIT(5)
+#define REG_GPIOx_CFG_DONE   BIT(9)
+#define REG_GPIOx_GRP_MASK   GENMASK(18, 16)
+#define REG_IRQ(g, x)        (0x800 + 0x40 * (g) + 4 * ((x) >> 5))
+
+// 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 clr, uint32_t set)
+{
+	void __iomem *ppin = pctl->base + REG_GPIO(pin);
+	uint32_t prev, cfg;
+
+	prev = pctl->pin_shadow[pin];
+	cfg = (prev & ~clr) | set;
+
+	if (!(prev & REG_GPIOx_CFG_DONE))
+		writel_relaxed(cfg & ~REG_GPIOx_CFG_DONE, ppin);
+	writel_relaxed(cfg, ppin);
+	pctl->pin_shadow[pin] = cfg;
+}
+
+static uint32_t apple_gpio_get_reg(struct apple_gpio_pinctrl *pctl,
+				   unsigned int pin)
+{
+	return readl_relaxed(pctl->base + REG_GPIO(pin));
+}
+
+/* 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);
+
+	if (func)
+		apple_gpio_set_reg(pctl, group, 0,
+				   REG_GPIOx_PERIPH | REG_GPIOx_CFG_DONE);
+	else
+		apple_gpio_set_reg(pctl, group, REG_GPIOx_PERIPH,
+				   REG_GPIOx_CFG_DONE);
+
+	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);
+
+	return (FIELD_GET(REG_GPIOx_MODE_MASK, pctl->pin_shadow[offset]) ==
+		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);
+	uint32_t reg;
+
+	if (FIELD_GET(REG_GPIOx_MODE_MASK, pctl->pin_shadow[offset]) ==
+	    REG_GPIOx_OUT) {
+		return pctl->pin_shadow[offset] & REG_GPIOx_DATA;
+	}
+
+	reg = apple_gpio_get_reg(pctl, 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,
+			   REG_GPIOx_CFG_DONE | (value & REG_GPIOx_DATA));
+}
+
+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_MODE_MASK | REG_GPIOx_DATA,
+			   FIELD_PREP(REG_GPIOx_MODE_MASK,
+				      REG_GPIOx_IN_IRQ_OFF) |
+				   REG_GPIOx_CFG_DONE);
+	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_DATA,
+			   FIELD_PREP(REG_GPIOx_MODE_MASK, REG_GPIOx_OUT) |
+				   (value & REG_GPIOx_DATA) |
+				   REG_GPIOx_CFG_DONE);
+	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_MASK,
+					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_MASK,
+			   FIELD_PREP(REG_GPIOx_MODE_MASK,
+				      REG_GPIOx_IN_IRQ_OFF) |
+				   REG_GPIOx_CFG_DONE);
+}
+
+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));
+	u32 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_MASK,
+			   FIELD_PREP(REG_GPIOx_MODE_MASK, irqtype) |
+				   REG_GPIOx_CFG_DONE);
+}
+
+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_MASK,
+			   FIELD_PREP(REG_GPIOx_GRP_MASK, 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));
+	u32 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_MASK,
+			   FIELD_PREP(REG_GPIOx_MODE_MASK, irqtype) |
+				   REG_GPIOx_CFG_DONE);
+
+	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, reg;
+	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, "gpio-ranges property not found\n");
+		npins = 512;
+		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->pin_shadow = devm_kmalloc_array(
+		&pdev->dev, npins, sizeof(pctl->pin_shadow[0]), GFP_KERNEL);
+	if (!pctl->pin_shadow)
+		return -ENOMEM;
+
+	pctl->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(pctl->base))
+		return PTR_ERR(pctl->base);
+
+	for (i = 0; i < npins; i++) {
+		reg = apple_gpio_get_reg(pctl, i);
+		pctl->pin_shadow[i] = reg;
+		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");