[PATCH v8 12/14] dt-bindings: arm: airoha: Add binding for Airoha GPIO controller

[PATCH v8 12/14] dt-bindings: arm: airoha: Add binding for Airoha GPIO controller

[Felix Fietkau]

From: John Crispin <john@phrozen.org>

Airoha's GPIO controller on their ARM EN7523 SoCs consists of two banks of 32
GPIOs

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: John Crispin <john@phrozen.org>
Signed-off-by: Felix Fietkau <nbd@nbd.name>
---
 .../bindings/gpio/airoha,en7523-gpio.yaml     | 67 +++++++++++++++++++
 1 file changed, 67 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/airoha,en7523-gpio.yaml

diff --git a/Documentation/devicetree/bindings/gpio/airoha,en7523-gpio.yaml b/Documentation/devicetree/bindings/gpio/airoha,en7523-gpio.yaml
new file mode 100644
index 000000000000..66c00ec85731
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/airoha,en7523-gpio.yaml
@@ -0,0 +1,67 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/gpio/airoha,en7523-gpio.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Airoha EN7523 GPIO controller
+
+maintainers:
+  - John Crispin <john@phrozen.org>
+
+description: |
+  Airoha's GPIO controller on their ARM EN7523 SoCs consists of two banks of 32
+  GPIOs.
+
+properties:
+  $nodename:
+    pattern: "^gpio@[0-9a-f]+$"
+
+  compatible:
+    items:
+      - const: airoha,en7523-gpio
+
+  reg:
+    description: |
+      The first tuple points to the input register.
+      The second and third tuple point to the direction registers
+      The fourth tuple points to the output register
+    maxItems: 4
+
+  "#gpio-cells":
+    const: 2
+
+  gpio-controller: true
+
+required:
+  - compatible
+  - reg
+  - "#gpio-cells"
+  - gpio-controller
+
+additionalProperties: false
+
+examples:
+  - |
+    gpio0: gpio@1fbf0200 {
+        compatible = "airoha,en7523-gpio";
+        reg = <0x1fbf0204 0x4>,
+              <0x1fbf0200 0x4>,
+              <0x1fbf0220 0x4>,
+              <0x1fbf0214 0x4>;
+        gpio-controller;
+        #gpio-cells = <2>;
+    };
+
+    gpio1: gpio@1fbf0270 {
+        compatible = "airoha,en7523-gpio";
+        reg = <0x1fbf0270 0x4>,
+              <0x1fbf0260 0x4>,
+              <0x1fbf0264 0x4>,
+              <0x1fbf0278 0x4>;
+        gpio-controller;
+        #gpio-cells = <2>;
+    };
+
+
+...
-- 
2.34.1

[PATCH v8 13/14] gpio: Add support for Airoha EN7523 GPIO controller

[Felix Fietkau]

From: John Crispin <john@phrozen.org>

Airoha's GPIO controller on their ARM EN7523 SoCs consists of two banks of 32
GPIOs. Each instance in DT is for a single bank.

Signed-off-by: John Crispin <john@phrozen.org>
Signed-off-by: Felix Fietkau <nbd@nbd.name>
---
 drivers/gpio/Kconfig       |   9 +++
 drivers/gpio/Makefile      |   1 +
 drivers/gpio/gpio-en7523.c | 134 +++++++++++++++++++++++++++++++++++++
 3 files changed, 144 insertions(+)
 create mode 100644 drivers/gpio/gpio-en7523.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 072ed610f9c6..e4a34272504f 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -247,6 +247,15 @@ config GPIO_EM
 	help
 	  Say yes here to support GPIO on Renesas Emma Mobile SoCs.
 
+config GPIO_EN7523
+	tristate "Airoha GPIO support"
+	depends on ARCH_AIROHA
+	default ARCH_AIROHA
+	select GPIO_GENERIC
+	select GPIOLIB_IRQCHIP
+	help
+	  Say yes here to support the GPIO controller on Airoha EN7523.
+
 config GPIO_EP93XX
 	def_bool y
 	depends on ARCH_EP93XX
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 71ee9fc2ff83..d2269ee0948e 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -56,6 +56,7 @@ obj-$(CONFIG_GPIO_DLN2)			+= gpio-dln2.o
 obj-$(CONFIG_GPIO_DWAPB)		+= gpio-dwapb.o
 obj-$(CONFIG_GPIO_EIC_SPRD)		+= gpio-eic-sprd.o
 obj-$(CONFIG_GPIO_EM)			+= gpio-em.o
+obj-$(CONFIG_GPIO_EN7523)		+= gpio-en7523.o
 obj-$(CONFIG_GPIO_EP93XX)		+= gpio-ep93xx.o
 obj-$(CONFIG_GPIO_EXAR)			+= gpio-exar.o
 obj-$(CONFIG_GPIO_F7188X)		+= gpio-f7188x.o
diff --git a/drivers/gpio/gpio-en7523.c b/drivers/gpio/gpio-en7523.c
new file mode 100644
index 000000000000..67631396cd93
--- /dev/null
+++ b/drivers/gpio/gpio-en7523.c
@@ -0,0 +1,134 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <linux/gpio/driver.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+
+#define AIROHA_GPIO_MAX		32
+
+/**
+ * airoha_gpio_ctrl - Airoha GPIO driver data
+ * @gc: Associated gpio_chip instance.
+ * @data: The data register.
+ * @dir0: The direction register for the lower 16 pins.
+ * @dir1: The direction register for the higher 16 pins.
+ * @output: The output enable register.
+ */
+struct airoha_gpio_ctrl {
+	struct gpio_chip gc;
+	void __iomem *data;
+	void __iomem *dir[2];
+	void __iomem *output;
+};
+
+static struct airoha_gpio_ctrl *gc_to_ctrl(struct gpio_chip *gc)
+{
+	return container_of(gc, struct airoha_gpio_ctrl, gc);
+}
+
+static int airoha_dir_set(struct gpio_chip *gc, unsigned int gpio,
+			  int val, int out)
+{
+	struct airoha_gpio_ctrl *ctrl = gc_to_ctrl(gc);
+	u32 dir = ioread32(ctrl->dir[gpio / 16]);
+	u32 output = ioread32(ctrl->output);
+	u32 mask = BIT((gpio % 16) * 2);
+
+	if (out) {
+		dir |= mask;
+		output |= BIT(gpio);
+	} else {
+		dir &= ~mask;
+		output &= ~BIT(gpio);
+	}
+
+	iowrite32(dir, ctrl->dir[gpio / 16]);
+
+	if (out)
+		gc->set(gc, gpio, val);
+
+	iowrite32(output, ctrl->output);
+
+	return 0;
+}
+
+static int airoha_dir_out(struct gpio_chip *gc, unsigned int gpio,
+			  int val)
+{
+	return airoha_dir_set(gc, gpio, val, 1);
+}
+
+static int airoha_dir_in(struct gpio_chip *gc, unsigned int gpio)
+{
+	return airoha_dir_set(gc, gpio, 0, 0);
+}
+
+static int airoha_get_dir(struct gpio_chip *gc, unsigned int gpio)
+{
+	struct airoha_gpio_ctrl *ctrl = gc_to_ctrl(gc);
+	u32 dir = ioread32(ctrl->dir[gpio / 16]);
+	u32 mask = BIT((gpio % 16) * 2);
+
+	return (dir & mask) ? GPIO_LINE_DIRECTION_OUT : GPIO_LINE_DIRECTION_IN;
+}
+
+static const struct of_device_id airoha_gpio_of_match[] = {
+	{ .compatible = "airoha,en7523-gpio" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, airoha_gpio_of_match);
+
+static int airoha_gpio_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct airoha_gpio_ctrl *ctrl;
+	int err;
+
+	ctrl = devm_kzalloc(dev, sizeof(*ctrl), GFP_KERNEL);
+	if (!ctrl)
+		return -ENOMEM;
+
+	ctrl->data = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(ctrl->data))
+		return PTR_ERR(ctrl->data);
+
+	ctrl->dir[0] = devm_platform_ioremap_resource(pdev, 1);
+	if (IS_ERR(ctrl->dir[0]))
+		return PTR_ERR(ctrl->dir[0]);
+
+	ctrl->dir[1] = devm_platform_ioremap_resource(pdev, 2);
+	if (IS_ERR(ctrl->dir[1]))
+		return PTR_ERR(ctrl->dir[1]);
+
+	ctrl->output = devm_platform_ioremap_resource(pdev, 3);
+	if (IS_ERR(ctrl->output))
+		return PTR_ERR(ctrl->output);
+
+	err = bgpio_init(&ctrl->gc, dev, 4, ctrl->data, NULL,
+			 NULL, NULL, NULL, 0);
+	if (err)
+		return dev_err_probe(dev, err, "unable to init generic GPIO");
+
+	ctrl->gc.ngpio = AIROHA_GPIO_MAX;
+	ctrl->gc.owner = THIS_MODULE;
+	ctrl->gc.direction_output = airoha_dir_out;
+	ctrl->gc.direction_input = airoha_dir_in;
+	ctrl->gc.get_direction = airoha_get_dir;
+
+	return devm_gpiochip_add_data(dev, &ctrl->gc, ctrl);
+}
+
+static struct platform_driver airoha_gpio_driver = {
+	.driver = {
+		.name = "airoha-gpio",
+		.of_match_table	= airoha_gpio_of_match,
+	},
+	.probe = airoha_gpio_probe,
+};
+module_platform_driver(airoha_gpio_driver);
+
+MODULE_DESCRIPTION("Airoha GPIO support");
+MODULE_AUTHOR("John Crispin <john@phrozen.org>");
+MODULE_LICENSE("GPL v2");
-- 
2.34.1

Re: [PATCH v8 13/14] gpio: Add support for Airoha EN7523 GPIO controller

[Arnd Bergmann]

On Mon, Dec 20, 2021 at 10:18 PM Felix Fietkau <nbd@nbd.name> wrote:
>
> From: John Crispin <john@phrozen.org>
>
> Airoha's GPIO controller on their ARM EN7523 SoCs consists of two banks of 32
> GPIOs. Each instance in DT is for a single bank.
>
> Signed-off-by: John Crispin <john@phrozen.org>
> Signed-off-by: Felix Fietkau <nbd@nbd.name>

This looks ok to me. If you want to merge the entire series through
the SoC tree,
it does need to be reviewed by the GPIO maintainers though. If you want to
go through individual subsystem trees, I would suggest you post this patch
and the DT binding separately from the rest.

      Arnd

Re: [PATCH v8 13/14] gpio: Add support for Airoha EN7523 GPIO controller

[Linus Walleij]

On Tue, Dec 21, 2021 at 2:50 PM Arnd Bergmann <arnd@arndb.de> wrote:
> On Mon, Dec 20, 2021 at 10:18 PM Felix Fietkau <nbd@nbd.name> wrote:
> >
> > From: John Crispin <john@phrozen.org>
> >
> > Airoha's GPIO controller on their ARM EN7523 SoCs consists of two banks of 32
> > GPIOs. Each instance in DT is for a single bank.
> >
> > Signed-off-by: John Crispin <john@phrozen.org>
> > Signed-off-by: Felix Fietkau <nbd@nbd.name>
>
> This looks ok to me. If you want to merge the entire series through
> the SoC tree,
> it does need to be reviewed by the GPIO maintainers though. If you want to
> go through individual subsystem trees, I would suggest you post this patch
> and the DT binding separately from the rest.

I reviewed a v7 version I think, anyways:
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCH v8 13/14] gpio: Add support for Airoha EN7523 GPIO controller

[Bartosz Golaszewski]

On Mon, Dec 20, 2021 at 10:24 PM Felix Fietkau <nbd@nbd.name> wrote:
>
> From: John Crispin <john@phrozen.org>
>
> Airoha's GPIO controller on their ARM EN7523 SoCs consists of two banks of 32
> GPIOs. Each instance in DT is for a single bank.
>
> Signed-off-by: John Crispin <john@phrozen.org>
> Signed-off-by: Felix Fietkau <nbd@nbd.name>
> ---

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

Arnd: Please go ahead and take it through the arm-soc tree.

Bart

Re: [PATCH v8 13/14] gpio: Add support for Airoha EN7523 GPIO controller

[Andy Shevchenko]

On Tue, Dec 21, 2021 at 12:02 AM Felix Fietkau <nbd@nbd.name> wrote:
>
> From: John Crispin <john@phrozen.org>
>
> Airoha's GPIO controller on their ARM EN7523 SoCs consists of two banks of 32
> GPIOs. Each instance in DT is for a single bank.

FWIW,
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

A couple of minor comments and one about headers below.

> Signed-off-by: John Crispin <john@phrozen.org>
> Signed-off-by: Felix Fietkau <nbd@nbd.name>
> ---
>  drivers/gpio/Kconfig       |   9 +++
>  drivers/gpio/Makefile      |   1 +
>  drivers/gpio/gpio-en7523.c | 134 +++++++++++++++++++++++++++++++++++++
>  3 files changed, 144 insertions(+)
>  create mode 100644 drivers/gpio/gpio-en7523.c
>
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index 072ed610f9c6..e4a34272504f 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -247,6 +247,15 @@ config GPIO_EM
>         help
>           Say yes here to support GPIO on Renesas Emma Mobile SoCs.
>
> +config GPIO_EN7523
> +       tristate "Airoha GPIO support"
> +       depends on ARCH_AIROHA
> +       default ARCH_AIROHA
> +       select GPIO_GENERIC
> +       select GPIOLIB_IRQCHIP
> +       help
> +         Say yes here to support the GPIO controller on Airoha EN7523.

Checkpatch nowadays wants a better description. Also add a paragraph
at the end, like many other drivers do, what the module would be if
selected as M.

>  config GPIO_EP93XX
>         def_bool y
>         depends on ARCH_EP93XX
> diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
> index 71ee9fc2ff83..d2269ee0948e 100644
> --- a/drivers/gpio/Makefile
> +++ b/drivers/gpio/Makefile
> @@ -56,6 +56,7 @@ obj-$(CONFIG_GPIO_DLN2)                       += gpio-dln2.o
>  obj-$(CONFIG_GPIO_DWAPB)               += gpio-dwapb.o
>  obj-$(CONFIG_GPIO_EIC_SPRD)            += gpio-eic-sprd.o
>  obj-$(CONFIG_GPIO_EM)                  += gpio-em.o
> +obj-$(CONFIG_GPIO_EN7523)              += gpio-en7523.o
>  obj-$(CONFIG_GPIO_EP93XX)              += gpio-ep93xx.o
>  obj-$(CONFIG_GPIO_EXAR)                        += gpio-exar.o
>  obj-$(CONFIG_GPIO_F7188X)              += gpio-f7188x.o
> diff --git a/drivers/gpio/gpio-en7523.c b/drivers/gpio/gpio-en7523.c
> new file mode 100644
> index 000000000000..67631396cd93
> --- /dev/null
> +++ b/drivers/gpio/gpio-en7523.c
> @@ -0,0 +1,134 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#include <linux/gpio/driver.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/property.h>

Missed headers
types.h for _iomem, etc.
io.h for I/O accessors.
bits.h for BIT(), etc.

> +#define AIROHA_GPIO_MAX                32
> +
> +/**
> + * airoha_gpio_ctrl - Airoha GPIO driver data
> + * @gc: Associated gpio_chip instance.
> + * @data: The data register.
> + * @dir0: The direction register for the lower 16 pins.
> + * @dir1: The direction register for the higher 16 pins.
> + * @output: The output enable register.
> + */
> +struct airoha_gpio_ctrl {
> +       struct gpio_chip gc;
> +       void __iomem *data;
> +       void __iomem *dir[2];
> +       void __iomem *output;
> +};
> +
> +static struct airoha_gpio_ctrl *gc_to_ctrl(struct gpio_chip *gc)
> +{
> +       return container_of(gc, struct airoha_gpio_ctrl, gc);
> +}
> +
> +static int airoha_dir_set(struct gpio_chip *gc, unsigned int gpio,
> +                         int val, int out)
> +{
> +       struct airoha_gpio_ctrl *ctrl = gc_to_ctrl(gc);
> +       u32 dir = ioread32(ctrl->dir[gpio / 16]);
> +       u32 output = ioread32(ctrl->output);
> +       u32 mask = BIT((gpio % 16) * 2);
> +
> +       if (out) {
> +               dir |= mask;
> +               output |= BIT(gpio);
> +       } else {
> +               dir &= ~mask;
> +               output &= ~BIT(gpio);
> +       }
> +
> +       iowrite32(dir, ctrl->dir[gpio / 16]);
> +
> +       if (out)
> +               gc->set(gc, gpio, val);
> +
> +       iowrite32(output, ctrl->output);
> +
> +       return 0;
> +}
> +
> +static int airoha_dir_out(struct gpio_chip *gc, unsigned int gpio,
> +                         int val)
> +{
> +       return airoha_dir_set(gc, gpio, val, 1);
> +}
> +
> +static int airoha_dir_in(struct gpio_chip *gc, unsigned int gpio)
> +{
> +       return airoha_dir_set(gc, gpio, 0, 0);
> +}
> +
> +static int airoha_get_dir(struct gpio_chip *gc, unsigned int gpio)
> +{
> +       struct airoha_gpio_ctrl *ctrl = gc_to_ctrl(gc);
> +       u32 dir = ioread32(ctrl->dir[gpio / 16]);
> +       u32 mask = BIT((gpio % 16) * 2);
> +
> +       return (dir & mask) ? GPIO_LINE_DIRECTION_OUT : GPIO_LINE_DIRECTION_IN;
> +}

> +static const struct of_device_id airoha_gpio_of_match[] = {
> +       { .compatible = "airoha,en7523-gpio" },
> +       { }
> +};
> +MODULE_DEVICE_TABLE(of, airoha_gpio_of_match);

Move this below, closer to its user.

> +static int airoha_gpio_probe(struct platform_device *pdev)
> +{
> +       struct device *dev = &pdev->dev;
> +       struct airoha_gpio_ctrl *ctrl;
> +       int err;
> +
> +       ctrl = devm_kzalloc(dev, sizeof(*ctrl), GFP_KERNEL);
> +       if (!ctrl)
> +               return -ENOMEM;
> +
> +       ctrl->data = devm_platform_ioremap_resource(pdev, 0);
> +       if (IS_ERR(ctrl->data))
> +               return PTR_ERR(ctrl->data);
> +
> +       ctrl->dir[0] = devm_platform_ioremap_resource(pdev, 1);
> +       if (IS_ERR(ctrl->dir[0]))
> +               return PTR_ERR(ctrl->dir[0]);
> +
> +       ctrl->dir[1] = devm_platform_ioremap_resource(pdev, 2);
> +       if (IS_ERR(ctrl->dir[1]))
> +               return PTR_ERR(ctrl->dir[1]);
> +
> +       ctrl->output = devm_platform_ioremap_resource(pdev, 3);
> +       if (IS_ERR(ctrl->output))
> +               return PTR_ERR(ctrl->output);
> +
> +       err = bgpio_init(&ctrl->gc, dev, 4, ctrl->data, NULL,
> +                        NULL, NULL, NULL, 0);
> +       if (err)
> +               return dev_err_probe(dev, err, "unable to init generic GPIO");
> +
> +       ctrl->gc.ngpio = AIROHA_GPIO_MAX;
> +       ctrl->gc.owner = THIS_MODULE;
> +       ctrl->gc.direction_output = airoha_dir_out;
> +       ctrl->gc.direction_input = airoha_dir_in;
> +       ctrl->gc.get_direction = airoha_get_dir;
> +
> +       return devm_gpiochip_add_data(dev, &ctrl->gc, ctrl);
> +}
> +
> +static struct platform_driver airoha_gpio_driver = {
> +       .driver = {
> +               .name = "airoha-gpio",
> +               .of_match_table = airoha_gpio_of_match,
> +       },
> +       .probe = airoha_gpio_probe,
> +};
> +module_platform_driver(airoha_gpio_driver);
> +
> +MODULE_DESCRIPTION("Airoha GPIO support");
> +MODULE_AUTHOR("John Crispin <john@phrozen.org>");
> +MODULE_LICENSE("GPL v2");
> --
> 2.34.1
>


-- 
With Best Regards,
Andy Shevchenko