diff mbox series

[02/12] pinctrl: add a pincontrol driver for BCM6328

Message ID 20210225164216.21124-3-noltari@gmail.com
State New
Headers show
Series pinctrl: add BCM63XX pincontrol support | expand

Commit Message

Álvaro Fernández Rojas Feb. 25, 2021, 4:42 p.m. UTC 
Add a pincontrol driver for BCM6328. BCM628 supports muxing 32 pins as
GPIOs, as LEDs for the integrated LED controller, or various other
functions. Its pincontrol mux registers also control other aspects, like
switching the second USB port between host and device mode.

Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com>
---
 drivers/pinctrl/bcm/Kconfig           |  11 +
 drivers/pinctrl/bcm/Makefile          |   1 +
 drivers/pinctrl/bcm/pinctrl-bcm6328.c | 581 ++++++++++++++++++++++++++
 3 files changed, 593 insertions(+)
 create mode 100644 drivers/pinctrl/bcm/pinctrl-bcm6328.c

Comments

Linus Walleij March 2, 2021, 3:20 p.m. UTC | #1 
Hi Álvaro,

thanks for your patch!

On Thu, Feb 25, 2021 at 5:42 PM Álvaro Fernández Rojas
<noltari@gmail.com> wrote:

> Add a pincontrol driver for BCM6328. BCM628 supports muxing 32 pins as
> GPIOs, as LEDs for the integrated LED controller, or various other
> functions. Its pincontrol mux registers also control other aspects, like
> switching the second USB port between host and device mode.
>
> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
> Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com>

Thanks for working on this. This SoC definitely need to come upstream.

I think this driver can be simplified a bit and reuse some core infrastructure
to make it more maintainable. It might be a bit of challenge but definitely
worth it!

> +config PINCTRL_BCM6328
> +       bool "Broadcom BCM6328 GPIO driver"
> +       depends on OF_GPIO && (BMIPS_GENERIC || COMPILE_TEST)
> +       select PINMUX
> +       select PINCONF
> +       select GENERIC_PINCONF
> +       select MFD_SYSCON
> +       default BMIPS_GENERIC
> +       help
> +          Say Y here to enable the Broadcom BCM6328 GPIO driver.

I suggest

select GPIO_REGMAP
select GPIOLIB_IRQCHIP
select IRQ_DOMAIN_HIERARCHY

see below.

(...)
> +#include <linux/bitops.h>

Just <linux/bits.h> maybe, if you only use BIT() and GENMASK().

> +#include <linux/gpio.h>
> +#include <linux/of_gpio.h>

Do not include these, just:
#include <linux/gpio/driver.h>

> +#define BCM6328_DIROUT_REG     0x04
> +#define BCM6328_DATA_REG       0x0c
> +#define BCM6328_MODE_REG       0x18

This looks very much like it could use GPIO_REGMAP.
Can you look at:
drivers/gpio/gpio-regmap.c
drivers/gpio/gpio-sl28cpld.c

And see if you can do what that driver is doing and reuse
this core infrastructure?

> +static inline unsigned int bcm6328_bank_pin(unsigned int pin)
> +{
> +       return pin % PINS_PER_BANK;
> +}

I am generally reluctant about registering several banks/instances
of the GPIO if it is possible to just use more devices in the
device tree, like one for each instance.

> +static inline unsigned int bcm6328_reg_off(unsigned int reg, unsigned int pin)
> +{
> +       return reg - (pin / PINS_PER_BANK) * BANK_SIZE;
> +}

Because it leads to this kind of weirdness to split out the devices
from the main device in practice.

> +static int bcm6328_gpio_direction_input(struct gpio_chip *chip,
> +                                       unsigned int pin)
> +{
(...)
> +       /*
> +        * Check with the pinctrl driver whether this pin is usable as
> +        * an input GPIO
> +        */
> +       ret = pinctrl_gpio_direction_input(chip->base + pin);
> +       if (ret)
> +               return ret;

This is very nice.

> +static int bcm6328_gpio_to_irq(struct gpio_chip *chip, unsigned gpio)
> +{
> +       char irq_name[7];
> +
> +       sprintf(irq_name, "gpio%d", gpio);
> +
> +       return of_irq_get_byname(chip->of_node, irq_name);
> +}

This is a clear indication that we are dealing with a hierarchical irqchip.

My assumption is that you have one IRQ per GPIO line, so each
GPIO has a dedicated IRQ on the interrupt controller. Correct?

This means:

- Do not add all the interrupts into the device tree by name.

- In Kconfig select GPIOLIB_IRQCHIP, select IRQ_DOMAIN_HIERARCHY

- Populate a simple struct gpio_irq_chip, if no local registers need
  updating on interrupts, just pass interrupts through
        .irq_mask       = irq_chip_mask_parent,
        .irq_unmask     = irq_chip_unmask_parent,
  etc.

- Implement bcm6328_gpio_child_to_parent_hwirq() for this chip
  with hardcoded mappings between the hardware GPIO and interrupt
  lines, using the parent interrupt controller hierarchically. This mapping
  is determined from the compatible-string, and part of the property
  of how the GPIO block is integrated with the SoC. If need be to
  tell different chips apart, more precise compatible strings are needed.

- Examples:
  drivers/gpio/gpio-ixp4xx.c
  drivers/gpio/gpio-sifive.c

If you do this you will notice the core is more helpful to cut down on the
code.

Yours,
Linus Walleij
Álvaro Fernández Rojas March 2, 2021, 4:59 p.m. UTC | #2 
Hi Linus,

El 02/03/2021 a las 16:20, Linus Walleij escribió:
> Hi Álvaro,
> 
> thanks for your patch!
> 
> On Thu, Feb 25, 2021 at 5:42 PM Álvaro Fernández Rojas
> <noltari@gmail.com> wrote:
> 
>> Add a pincontrol driver for BCM6328. BCM628 supports muxing 32 pins as
>> GPIOs, as LEDs for the integrated LED controller, or various other
>> functions. Its pincontrol mux registers also control other aspects, like
>> switching the second USB port between host and device mode.
>>
>> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
>> Signed-off-by: Jonas Gorski <jonas.gorski@gmail.com>
> 
> Thanks for working on this. This SoC definitely need to come upstream.

I will try my best :).

> 
> I think this driver can be simplified a bit and reuse some core infrastructure
> to make it more maintainable. It might be a bit of challenge but definitely
> worth it!
> 
>> +config PINCTRL_BCM6328
>> +       bool "Broadcom BCM6328 GPIO driver"
>> +       depends on OF_GPIO && (BMIPS_GENERIC || COMPILE_TEST)
>> +       select PINMUX
>> +       select PINCONF
>> +       select GENERIC_PINCONF
>> +       select MFD_SYSCON
>> +       default BMIPS_GENERIC
>> +       help
>> +          Say Y here to enable the Broadcom BCM6328 GPIO driver.
> 
> I suggest
> 
> select GPIO_REGMAP
> select GPIOLIB_IRQCHIP
> select IRQ_DOMAIN_HIERARCHY
> 
> see below.
> 
> (...)
>> +#include <linux/bitops.h>
> 
> Just <linux/bits.h> maybe, if you only use BIT() and GENMASK().
> 
>> +#include <linux/gpio.h>
>> +#include <linux/of_gpio.h>
> 
> Do not include these, just:
> #include <linux/gpio/driver.h>
> 
>> +#define BCM6328_DIROUT_REG     0x04
>> +#define BCM6328_DATA_REG       0x0c
>> +#define BCM6328_MODE_REG       0x18
> 
> This looks very much like it could use GPIO_REGMAP.
> Can you look at:
> drivers/gpio/gpio-regmap.c
> drivers/gpio/gpio-sl28cpld.c
> 
> And see if you can do what that driver is doing and reuse
> this core infrastructure?

I've just checked drivers/gpio/gpio-regmap.c and it seems that "struct 
gpio_regmap" should be declared in include/linux/gpio/regmap.h.
Right now devm_gpio_regmap_register() is returning a pointer to a 
structure which none except gpio-regmap knows. Does that make any sense?
I need to access gpio_regmap->gpio_chip in order to gather 
gpio_chip.base and pass it to pinctrl_add_gpio_range().

> 
>> +static inline unsigned int bcm6328_bank_pin(unsigned int pin)
>> +{
>> +       return pin % PINS_PER_BANK;
>> +}
> 
> I am generally reluctant about registering several banks/instances
> of the GPIO if it is possible to just use more devices in the
> device tree, like one for each instance.
> 
>> +static inline unsigned int bcm6328_reg_off(unsigned int reg, unsigned int pin)
>> +{
>> +       return reg - (pin / PINS_PER_BANK) * BANK_SIZE;
>> +}
> 
> Because it leads to this kind of weirdness to split out the devices
> from the main device in practice.
> 
>> +static int bcm6328_gpio_direction_input(struct gpio_chip *chip,
>> +                                       unsigned int pin)
>> +{
> (...)
>> +       /*
>> +        * Check with the pinctrl driver whether this pin is usable as
>> +        * an input GPIO
>> +        */
>> +       ret = pinctrl_gpio_direction_input(chip->base + pin);
>> +       if (ret)
>> +               return ret;
> 
> This is very nice.
> 
>> +static int bcm6328_gpio_to_irq(struct gpio_chip *chip, unsigned gpio)
>> +{
>> +       char irq_name[7];
>> +
>> +       sprintf(irq_name, "gpio%d", gpio);
>> +
>> +       return of_irq_get_byname(chip->of_node, irq_name);
>> +}
> 
> This is a clear indication that we are dealing with a hierarchical irqchip.
> 
> My assumption is that you have one IRQ per GPIO line, so each
> GPIO has a dedicated IRQ on the interrupt controller. Correct?
> 
> This means:
> 
> - Do not add all the interrupts into the device tree by name.
> 
> - In Kconfig select GPIOLIB_IRQCHIP, select IRQ_DOMAIN_HIERARCHY
> 
> - Populate a simple struct gpio_irq_chip, if no local registers need
>    updating on interrupts, just pass interrupts through
>          .irq_mask       = irq_chip_mask_parent,
>          .irq_unmask     = irq_chip_unmask_parent,
>    etc.
> 
> - Implement bcm6328_gpio_child_to_parent_hwirq() for this chip
>    with hardcoded mappings between the hardware GPIO and interrupt
>    lines, using the parent interrupt controller hierarchically. This mapping
>    is determined from the compatible-string, and part of the property
>    of how the GPIO block is integrated with the SoC. If need be to
>    tell different chips apart, more precise compatible strings are needed.
> 
> - Examples:
>    drivers/gpio/gpio-ixp4xx.c
>    drivers/gpio/gpio-sifive.c
> 
> If you do this you will notice the core is more helpful to cut down on the
> code.
> 
> Yours,
> Linus Walleij
> 

Best regards,
Álvaro.
diff mbox series

Patch

diff --git a/drivers/pinctrl/bcm/Kconfig b/drivers/pinctrl/bcm/Kconfig
index 0ed14de0134c..4c6e41cf7a32 100644
--- a/drivers/pinctrl/bcm/Kconfig
+++ b/drivers/pinctrl/bcm/Kconfig
@@ -29,6 +29,17 @@  config PINCTRL_BCM2835
 	help
 	   Say Y here to enable the Broadcom BCM2835 GPIO driver.
 
+config PINCTRL_BCM6328
+	bool "Broadcom BCM6328 GPIO driver"
+	depends on OF_GPIO && (BMIPS_GENERIC || COMPILE_TEST)
+	select PINMUX
+	select PINCONF
+	select GENERIC_PINCONF
+	select MFD_SYSCON
+	default BMIPS_GENERIC
+	help
+	   Say Y here to enable the Broadcom BCM6328 GPIO driver.
+
 config PINCTRL_IPROC_GPIO
 	bool "Broadcom iProc GPIO (with PINCONF) driver"
 	depends on OF_GPIO && (ARCH_BCM_IPROC || COMPILE_TEST)
diff --git a/drivers/pinctrl/bcm/Makefile b/drivers/pinctrl/bcm/Makefile
index 79d5e49fdd9a..7e7c6e25b26d 100644
--- a/drivers/pinctrl/bcm/Makefile
+++ b/drivers/pinctrl/bcm/Makefile
@@ -3,6 +3,7 @@ 
 
 obj-$(CONFIG_PINCTRL_BCM281XX)		+= pinctrl-bcm281xx.o
 obj-$(CONFIG_PINCTRL_BCM2835)		+= pinctrl-bcm2835.o
+obj-$(CONFIG_PINCTRL_BCM6328)		+= pinctrl-bcm6328.o
 obj-$(CONFIG_PINCTRL_IPROC_GPIO)	+= pinctrl-iproc-gpio.o
 obj-$(CONFIG_PINCTRL_CYGNUS_MUX)	+= pinctrl-cygnus-mux.o
 obj-$(CONFIG_PINCTRL_NS)		+= pinctrl-ns.o
diff --git a/drivers/pinctrl/bcm/pinctrl-bcm6328.c b/drivers/pinctrl/bcm/pinctrl-bcm6328.c
new file mode 100644
index 000000000000..0b1e45145fdd
--- /dev/null
+++ b/drivers/pinctrl/bcm/pinctrl-bcm6328.c
@@ -0,0 +1,581 @@ 
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Driver for BCM6328 GPIO unit (pinctrl + GPIO)
+ *
+ * Copyright (C) 2021 Álvaro Fernández Rojas <noltari@gmail.com>
+ * Copyright (C) 2016 Jonas Gorski <jonas.gorski@gmail.com>
+ */
+
+#include <linux/bitops.h>
+#include <linux/gpio.h>
+#include <linux/kernel.h>
+#include <linux/mfd/syscon.h>
+#include <linux/of.h>
+#include <linux/of_gpio.h>
+#include <linux/of_irq.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#include <linux/pinctrl/machine.h>
+#include <linux/pinctrl/pinconf.h>
+#include <linux/pinctrl/pinconf-generic.h>
+#include <linux/pinctrl/pinmux.h>
+
+#include "../core.h"
+#include "../pinctrl-utils.h"
+
+#define MODULE_NAME		"bcm6328-pinctrl"
+#define BCM6328_NUM_GPIOS	32
+
+#define BANK_SIZE		sizeof(uint32_t)
+#define PINS_PER_BANK		(BANK_SIZE * BITS_PER_BYTE)
+
+#define BCM6328_DIROUT_REG	0x04
+#define BCM6328_DATA_REG	0x0c
+#define BCM6328_MODE_REG	0x18
+#define BCM6328_MUX_HI_REG	0x1c
+#define BCM6328_MUX_LO_REG	0x20
+#define BCM6328_MUX_OTHER_REG	0x24
+
+struct bcm6328_pingroup {
+	const char *name;
+	const unsigned * const pins;
+	const unsigned num_pins;
+};
+
+struct bcm6328_function {
+	const char *name;
+	const char * const *groups;
+	const unsigned num_groups;
+
+	unsigned mode_val:1;
+	unsigned mux_val:2;
+};
+
+struct bcm6328_pinctrl {
+	struct device *dev;
+	struct regmap *regs;
+
+	struct pinctrl_dev *pctl_dev;
+	struct gpio_chip gpio_chip;
+	struct pinctrl_desc pctl_desc;
+	struct pinctrl_gpio_range gpio_range;
+};
+
+static const struct pinctrl_pin_desc bcm6328_pins[] = {
+	PINCTRL_PIN(0, "gpio0"),
+	PINCTRL_PIN(1, "gpio1"),
+	PINCTRL_PIN(2, "gpio2"),
+	PINCTRL_PIN(3, "gpio3"),
+	PINCTRL_PIN(4, "gpio4"),
+	PINCTRL_PIN(5, "gpio5"),
+	PINCTRL_PIN(6, "gpio6"),
+	PINCTRL_PIN(7, "gpio7"),
+	PINCTRL_PIN(8, "gpio8"),
+	PINCTRL_PIN(9, "gpio9"),
+	PINCTRL_PIN(10, "gpio10"),
+	PINCTRL_PIN(11, "gpio11"),
+	PINCTRL_PIN(12, "gpio12"),
+	PINCTRL_PIN(13, "gpio13"),
+	PINCTRL_PIN(14, "gpio14"),
+	PINCTRL_PIN(15, "gpio15"),
+	PINCTRL_PIN(16, "gpio16"),
+	PINCTRL_PIN(17, "gpio17"),
+	PINCTRL_PIN(18, "gpio18"),
+	PINCTRL_PIN(19, "gpio19"),
+	PINCTRL_PIN(20, "gpio20"),
+	PINCTRL_PIN(21, "gpio21"),
+	PINCTRL_PIN(22, "gpio22"),
+	PINCTRL_PIN(23, "gpio23"),
+	PINCTRL_PIN(24, "gpio24"),
+	PINCTRL_PIN(25, "gpio25"),
+	PINCTRL_PIN(26, "gpio26"),
+	PINCTRL_PIN(27, "gpio27"),
+	PINCTRL_PIN(28, "gpio28"),
+	PINCTRL_PIN(29, "gpio29"),
+	PINCTRL_PIN(30, "gpio30"),
+	PINCTRL_PIN(31, "gpio31"),
+
+	/*
+	 * No idea where they really are; so let's put them according
+	 * to their mux offsets.
+	 */
+	PINCTRL_PIN(36, "hsspi_cs1"),
+	PINCTRL_PIN(38, "usb_p2"),
+};
+
+static unsigned gpio0_pins[] = { 0 };
+static unsigned gpio1_pins[] = { 1 };
+static unsigned gpio2_pins[] = { 2 };
+static unsigned gpio3_pins[] = { 3 };
+static unsigned gpio4_pins[] = { 4 };
+static unsigned gpio5_pins[] = { 5 };
+static unsigned gpio6_pins[] = { 6 };
+static unsigned gpio7_pins[] = { 7 };
+static unsigned gpio8_pins[] = { 8 };
+static unsigned gpio9_pins[] = { 9 };
+static unsigned gpio10_pins[] = { 10 };
+static unsigned gpio11_pins[] = { 11 };
+static unsigned gpio12_pins[] = { 12 };
+static unsigned gpio13_pins[] = { 13 };
+static unsigned gpio14_pins[] = { 14 };
+static unsigned gpio15_pins[] = { 15 };
+static unsigned gpio16_pins[] = { 16 };
+static unsigned gpio17_pins[] = { 17 };
+static unsigned gpio18_pins[] = { 18 };
+static unsigned gpio19_pins[] = { 19 };
+static unsigned gpio20_pins[] = { 20 };
+static unsigned gpio21_pins[] = { 21 };
+static unsigned gpio22_pins[] = { 22 };
+static unsigned gpio23_pins[] = { 23 };
+static unsigned gpio24_pins[] = { 24 };
+static unsigned gpio25_pins[] = { 25 };
+static unsigned gpio26_pins[] = { 26 };
+static unsigned gpio27_pins[] = { 27 };
+static unsigned gpio28_pins[] = { 28 };
+static unsigned gpio29_pins[] = { 29 };
+static unsigned gpio30_pins[] = { 30 };
+static unsigned gpio31_pins[] = { 31 };
+
+static unsigned hsspi_cs1_pins[] = { 36 };
+static unsigned usb_port1_pins[] = { 38 };
+
+#define BCM6328_GROUP(n)					\
+	{							\
+		.name = #n,					\
+		.pins = n##_pins,				\
+		.num_pins = ARRAY_SIZE(n##_pins),		\
+	}
+
+static struct bcm6328_pingroup bcm6328_groups[] = {
+	BCM6328_GROUP(gpio0),
+	BCM6328_GROUP(gpio1),
+	BCM6328_GROUP(gpio2),
+	BCM6328_GROUP(gpio3),
+	BCM6328_GROUP(gpio4),
+	BCM6328_GROUP(gpio5),
+	BCM6328_GROUP(gpio6),
+	BCM6328_GROUP(gpio7),
+	BCM6328_GROUP(gpio8),
+	BCM6328_GROUP(gpio9),
+	BCM6328_GROUP(gpio10),
+	BCM6328_GROUP(gpio11),
+	BCM6328_GROUP(gpio12),
+	BCM6328_GROUP(gpio13),
+	BCM6328_GROUP(gpio14),
+	BCM6328_GROUP(gpio15),
+	BCM6328_GROUP(gpio16),
+	BCM6328_GROUP(gpio17),
+	BCM6328_GROUP(gpio18),
+	BCM6328_GROUP(gpio19),
+	BCM6328_GROUP(gpio20),
+	BCM6328_GROUP(gpio21),
+	BCM6328_GROUP(gpio22),
+	BCM6328_GROUP(gpio23),
+	BCM6328_GROUP(gpio24),
+	BCM6328_GROUP(gpio25),
+	BCM6328_GROUP(gpio26),
+	BCM6328_GROUP(gpio27),
+	BCM6328_GROUP(gpio28),
+	BCM6328_GROUP(gpio29),
+	BCM6328_GROUP(gpio30),
+	BCM6328_GROUP(gpio31),
+
+	BCM6328_GROUP(hsspi_cs1),
+	BCM6328_GROUP(usb_port1),
+};
+
+/* GPIO_MODE */
+static const char * const led_groups[] = {
+	"gpio0",
+	"gpio1",
+	"gpio2",
+	"gpio3",
+	"gpio4",
+	"gpio5",
+	"gpio6",
+	"gpio7",
+	"gpio8",
+	"gpio9",
+	"gpio10",
+	"gpio11",
+	"gpio12",
+	"gpio13",
+	"gpio14",
+	"gpio15",
+	"gpio16",
+	"gpio17",
+	"gpio18",
+	"gpio19",
+	"gpio20",
+	"gpio21",
+	"gpio22",
+	"gpio23",
+};
+
+/* PINMUX_SEL */
+static const char * const serial_led_data_groups[] = {
+	"gpio6",
+};
+
+static const char * const serial_led_clk_groups[] = {
+	"gpio7",
+};
+
+static const char * const inet_act_led_groups[] = {
+	"gpio11",
+};
+
+static const char * const pcie_clkreq_groups[] = {
+	"gpio16",
+};
+
+static const char * const ephy0_act_led_groups[] = {
+	"gpio25",
+};
+
+static const char * const ephy1_act_led_groups[] = {
+	"gpio26",
+};
+
+static const char * const ephy2_act_led_groups[] = {
+	"gpio27",
+};
+
+static const char * const ephy3_act_led_groups[] = {
+	"gpio28",
+};
+
+static const char * const hsspi_cs1_groups[] = {
+	"hsspi_cs1"
+};
+
+static const char * const usb_host_port_groups[] = {
+	"usb_port1",
+};
+
+static const char * const usb_device_port_groups[] = {
+	"usb_port1",
+};
+
+#define BCM6328_MODE_FUN(n)				\
+	{						\
+		.name = #n,				\
+		.groups = n##_groups,			\
+		.num_groups = ARRAY_SIZE(n##_groups),	\
+		.mode_val = 1,				\
+	}
+
+#define BCM6328_MUX_FUN(n, mux)				\
+	{						\
+		.name = #n,				\
+		.groups = n##_groups,			\
+		.num_groups = ARRAY_SIZE(n##_groups),	\
+		.mux_val = mux,				\
+	}
+
+static const struct bcm6328_function bcm6328_funcs[] = {
+	BCM6328_MODE_FUN(led),
+	BCM6328_MUX_FUN(serial_led_data, 2),
+	BCM6328_MUX_FUN(serial_led_clk, 2),
+	BCM6328_MUX_FUN(inet_act_led, 1),
+	BCM6328_MUX_FUN(pcie_clkreq, 2),
+	BCM6328_MUX_FUN(ephy0_act_led, 1),
+	BCM6328_MUX_FUN(ephy1_act_led, 1),
+	BCM6328_MUX_FUN(ephy2_act_led, 1),
+	BCM6328_MUX_FUN(ephy3_act_led, 1),
+	BCM6328_MUX_FUN(hsspi_cs1, 2),
+	BCM6328_MUX_FUN(usb_host_port, 1),
+	BCM6328_MUX_FUN(usb_device_port, 2),
+};
+
+static inline unsigned int bcm6328_bank_pin(unsigned int pin)
+{
+	return pin % PINS_PER_BANK;
+}
+
+static inline unsigned int bcm6318_mux_off(unsigned int pin)
+{
+	static const unsigned int bcm6328_mux[] = {
+		BCM6328_MUX_LO_REG,
+		BCM6328_MUX_HI_REG,
+		BCM6328_MUX_OTHER_REG
+	};
+
+	return bcm6328_mux[pin / 16];
+}
+
+static inline unsigned int bcm6328_reg_off(unsigned int reg, unsigned int pin)
+{
+	return reg - (pin / PINS_PER_BANK) * BANK_SIZE;
+}
+
+static int bcm6328_gpio_direction_input(struct gpio_chip *chip,
+					unsigned int pin)
+{
+	struct bcm6328_pinctrl *pc = gpiochip_get_data(chip);
+	unsigned int dirout = bcm6328_reg_off(BCM6328_DIROUT_REG, pin);
+	unsigned int bank_pin = bcm6328_bank_pin(pin);
+	int ret;
+
+	/*
+	 * Check with the pinctrl driver whether this pin is usable as
+	 * an input GPIO
+	 */
+	ret = pinctrl_gpio_direction_input(chip->base + pin);
+	if (ret)
+		return ret;
+
+	regmap_update_bits(pc->regs, dirout, BIT(bank_pin), 0);
+
+	return 0;
+}
+
+static int bcm6328_gpio_direction_output(struct gpio_chip *chip,
+					 unsigned int pin, int value)
+{
+	struct bcm6328_pinctrl *pc = gpiochip_get_data(chip);
+	unsigned int data = bcm6328_reg_off(BCM6328_DATA_REG, pin);
+	unsigned int dirout = bcm6328_reg_off(BCM6328_DIROUT_REG, pin);
+	unsigned int bank_pin = bcm6328_bank_pin(pin);
+	unsigned int val = value ? BIT(bank_pin) : 0;
+	int ret;
+
+	/*
+	 * Check with the pinctrl driver whether this pin is usable as
+	 * an output GPIO
+	 */
+	ret = pinctrl_gpio_direction_output(chip->base + pin);
+	if (ret)
+		return ret;
+
+	regmap_update_bits(pc->regs, dirout, BIT(bank_pin), BIT(bank_pin));
+	regmap_update_bits(pc->regs, data, BIT(bank_pin), val);
+
+	return 0;
+}
+
+static int bcm6328_gpio_get(struct gpio_chip *chip, unsigned int pin)
+{
+	struct bcm6328_pinctrl *pc = gpiochip_get_data(chip);
+	unsigned int data = bcm6328_reg_off(BCM6328_DATA_REG, pin);
+	unsigned int bank_pin = bcm6328_bank_pin(pin);
+	unsigned int val;
+
+	regmap_read(pc->regs, data, &val);
+
+	return !!(val & BIT(bank_pin));
+}
+
+static int bcm6328_gpio_get_direction(struct gpio_chip *chip, unsigned int pin)
+{
+	struct bcm6328_pinctrl *pc = gpiochip_get_data(chip);
+	unsigned int dirout = bcm6328_reg_off(BCM6328_DIROUT_REG, pin);
+	unsigned int bank_pin = bcm6328_bank_pin(pin);
+	unsigned int val;
+
+	regmap_read(pc->regs, dirout, &val);
+
+	if (val & BIT(bank_pin))
+		return GPIO_LINE_DIRECTION_OUT;
+
+	return GPIO_LINE_DIRECTION_IN;
+}
+
+static void bcm6328_gpio_set(struct gpio_chip *chip, unsigned int pin,
+			     int value)
+{
+	struct bcm6328_pinctrl *pc = gpiochip_get_data(chip);
+	unsigned int data = bcm6328_reg_off(BCM6328_DATA_REG, pin);
+	unsigned int bank_pin = bcm6328_bank_pin(pin);
+	unsigned int val = value ? BIT(bank_pin) : 0;
+
+	regmap_update_bits(pc->regs, data, BIT(bank_pin), val);
+}
+
+static int bcm6328_gpio_to_irq(struct gpio_chip *chip, unsigned gpio)
+{
+	char irq_name[7];
+
+	sprintf(irq_name, "gpio%d", gpio);
+
+	return of_irq_get_byname(chip->of_node, irq_name);
+}
+
+static int bcm6328_pinctrl_get_group_count(struct pinctrl_dev *pctldev)
+{
+	return ARRAY_SIZE(bcm6328_groups);
+}
+
+static const char *bcm6328_pinctrl_get_group_name(struct pinctrl_dev *pctldev,
+						  unsigned group)
+{
+	return bcm6328_groups[group].name;
+}
+
+static int bcm6328_pinctrl_get_group_pins(struct pinctrl_dev *pctldev,
+					  unsigned group, const unsigned **pins,
+					  unsigned *num_pins)
+{
+	*pins = bcm6328_groups[group].pins;
+	*num_pins = bcm6328_groups[group].num_pins;
+
+	return 0;
+}
+
+static int bcm6328_pinctrl_get_func_count(struct pinctrl_dev *pctldev)
+{
+	return ARRAY_SIZE(bcm6328_funcs);
+}
+
+static const char *bcm6328_pinctrl_get_func_name(struct pinctrl_dev *pctldev,
+						 unsigned selector)
+{
+	return bcm6328_funcs[selector].name;
+}
+
+static int bcm6328_pinctrl_get_groups(struct pinctrl_dev *pctldev,
+				      unsigned selector,
+				      const char * const **groups,
+				      unsigned * const num_groups)
+{
+	*groups = bcm6328_funcs[selector].groups;
+	*num_groups = bcm6328_funcs[selector].num_groups;
+
+	return 0;
+}
+
+static void bcm6328_rmw_mux(struct bcm6328_pinctrl *pc, unsigned pin,
+			    unsigned int mode, unsigned int mux)
+{
+	if (pin < BCM6328_NUM_GPIOS)
+		regmap_update_bits(pc->regs, BCM6328_MODE_REG, BIT(pin),
+				   mode ? BIT(pin) : 0);
+
+	regmap_update_bits(pc->regs, bcm6318_mux_off(pin),
+			   3UL << ((pin % 16) * 2),
+			   mux << ((pin % 16) * 2));
+}
+
+static int bcm6328_pinctrl_set_mux(struct pinctrl_dev *pctldev,
+				   unsigned selector, unsigned group)
+{
+	struct bcm6328_pinctrl *pc = pinctrl_dev_get_drvdata(pctldev);
+	const struct bcm6328_pingroup *pg = &bcm6328_groups[group];
+	const struct bcm6328_function *f = &bcm6328_funcs[selector];
+
+	bcm6328_rmw_mux(pc, pg->pins[0], f->mode_val, f->mux_val);
+
+	return 0;
+}
+
+static int bcm6328_gpio_request_enable(struct pinctrl_dev *pctldev,
+				       struct pinctrl_gpio_range *range,
+				       unsigned offset)
+{
+	struct bcm6328_pinctrl *pc = pinctrl_dev_get_drvdata(pctldev);
+
+	/* disable all functions using this pin */
+	bcm6328_rmw_mux(pc, offset, 0, 0);
+
+	return 0;
+}
+
+static struct pinctrl_ops bcm6328_pctl_ops = {
+	.get_groups_count = bcm6328_pinctrl_get_group_count,
+	.get_group_name = bcm6328_pinctrl_get_group_name,
+	.get_group_pins = bcm6328_pinctrl_get_group_pins,
+	.dt_node_to_map = pinconf_generic_dt_node_to_map_pin,
+	.dt_free_map = pinctrl_utils_free_map,
+};
+
+static struct pinmux_ops bcm6328_pmx_ops = {
+	.get_functions_count = bcm6328_pinctrl_get_func_count,
+	.get_function_name = bcm6328_pinctrl_get_func_name,
+	.get_function_groups = bcm6328_pinctrl_get_groups,
+	.set_mux = bcm6328_pinctrl_set_mux,
+	.gpio_request_enable = bcm6328_gpio_request_enable,
+	.strict = true,
+};
+
+static int bcm6328_pinctrl_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	struct bcm6328_pinctrl *pc;
+	int err;
+
+	pc = devm_kzalloc(dev, sizeof(*pc), GFP_KERNEL);
+	if (!pc)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, pc);
+	pc->dev = dev;
+
+	pc->regs = syscon_node_to_regmap(dev->parent->of_node);
+	if (IS_ERR(pc->regs))
+		return PTR_ERR(pc->regs);
+
+	pc->gpio_chip.label = MODULE_NAME;
+	pc->gpio_chip.owner = THIS_MODULE;
+	pc->gpio_chip.request = gpiochip_generic_request;
+	pc->gpio_chip.free = gpiochip_generic_free;
+	pc->gpio_chip.direction_input = bcm6328_gpio_direction_input;
+	pc->gpio_chip.direction_output = bcm6328_gpio_direction_output;
+	pc->gpio_chip.get_direction = bcm6328_gpio_get_direction;
+	pc->gpio_chip.get = bcm6328_gpio_get;
+	pc->gpio_chip.set = bcm6328_gpio_set;
+	pc->gpio_chip.set_config = gpiochip_generic_config;
+	pc->gpio_chip.base = -1;
+	pc->gpio_chip.ngpio = BCM6328_NUM_GPIOS;
+	pc->gpio_chip.can_sleep = false;
+	pc->gpio_chip.parent = dev;
+	pc->gpio_chip.of_node = np;
+
+	if (of_get_property(np, "interrupt-names", NULL))
+		pc->gpio_chip.to_irq = bcm6328_gpio_to_irq;
+
+	err = gpiochip_add_data(&pc->gpio_chip, pc);
+	if (err) {
+		dev_err(dev, "could not add GPIO chip\n");
+		return err;
+	}
+
+	pc->pctl_desc.name = MODULE_NAME,
+	pc->pctl_desc.pins = bcm6328_pins,
+	pc->pctl_desc.npins = ARRAY_SIZE(bcm6328_pins),
+	pc->pctl_desc.pctlops = &bcm6328_pctl_ops,
+	pc->pctl_desc.pmxops = &bcm6328_pmx_ops,
+	pc->pctl_desc.owner = THIS_MODULE,
+
+	pc->pctl_dev = devm_pinctrl_register(dev, &pc->pctl_desc, pc);
+	if (IS_ERR(pc->pctl_dev)) {
+		gpiochip_remove(&pc->gpio_chip);
+		return PTR_ERR(pc->pctl_dev);
+	}
+
+	pc->gpio_range.name = MODULE_NAME;
+	pc->gpio_range.npins = BCM6328_NUM_GPIOS;
+	pc->gpio_range.base = pc->gpio_chip.base;
+	pc->gpio_range.gc = &pc->gpio_chip;
+	pinctrl_add_gpio_range(pc->pctl_dev, &pc->gpio_range);
+
+	dev_info(dev, "registered\n");
+
+	return 0;
+}
+
+static const struct of_device_id bcm6328_pinctrl_match[] = {
+	{ .compatible = "brcm,bcm6328-pinctrl", },
+	{ },
+};
+
+static struct platform_driver bcm6328_pinctrl_driver = {
+	.probe = bcm6328_pinctrl_probe,
+	.driver = {
+		.name = MODULE_NAME,
+		.of_match_table = bcm6328_pinctrl_match,
+	},
+};
+
+builtin_platform_driver(bcm6328_pinctrl_driver);