[v4,1/2] gpio: Add support for IDT 79RC3243x GPIO controller

Message ID 20210426095426.118356-1-tsbogend@alpha.franken.de
State New
Headers show
Series
  • [v4,1/2] gpio: Add support for IDT 79RC3243x GPIO controller
Related show

Commit Message

Thomas Bogendoerfer April 26, 2021, 9:54 a.m.
IDT 79RC3243x SoCs integrated a gpio controller, which handles up
to 32 gpios. All gpios could be used as an interrupt source.

Signed-off-by: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
---
Changes in v4:
 - added spinlock to serialize access to irq registers
 - reworked checking of irq sense bits
 - start with handle_bad_irq and set handle_level_irq in idt_gpio_irq_set_type
 - cleaned up #includes
 - use platform_get_irq

Changes in v3:
 - changed compatible string to idt,32434-gpio
 - registers now start with gpio direction register and leaves
   out alternate function register for pinmux/pinctrl driver

Changes in v2:
 - made driver buildable as module
 - use for_each_set_bit() in irq dispatch handler
 - use gpiochip_get_data instead of own container_of helper
 - use module_platform_driver() instead of arch_initcall
 - don't default y for Mikrotik RB532

 drivers/gpio/Kconfig         |  12 ++
 drivers/gpio/Makefile        |   1 +
 drivers/gpio/gpio-idt3243x.c | 209 +++++++++++++++++++++++++++++++++++
 3 files changed, 222 insertions(+)
 create mode 100644 drivers/gpio/gpio-idt3243x.c

Comments

Michael Walle April 27, 2021, 10:51 p.m. | #1
Hi,

Am 2021-04-26 12:29, schrieb Andy Shevchenko:
> On Mon, Apr 26, 2021 at 12:55 PM Thomas Bogendoerfer

> <tsbogend@alpha.franken.de> wrote:

> 

> 2) there is gpio-regmap generic code, that may be worth

> considering.


This driver uses memory mapped registers. While that is
also possible with gpio-regmap, there is one drawback:
it assumes gpiochip->can_sleep = true for now, see [1].
Unfortunately, there is no easy way to ask the regmap
if its mmio/fastio.

-michael

[1] 
https://elixir.bootlin.com/linux/v5.12/source/drivers/gpio/gpio-regmap.c#L257
Andy Shevchenko April 28, 2021, 11:07 a.m. | #2
On Wed, Apr 28, 2021 at 1:51 AM Michael Walle <michael@walle.cc> wrote:
> Am 2021-04-26 12:29, schrieb Andy Shevchenko:

> > On Mon, Apr 26, 2021 at 12:55 PM Thomas Bogendoerfer

> > <tsbogend@alpha.franken.de> wrote:

> >

> > 2) there is gpio-regmap generic code, that may be worth

> > considering.

>

> This driver uses memory mapped registers. While that is

> also possible with gpio-regmap, there is one drawback:

> it assumes gpiochip->can_sleep = true for now, see [1].

> Unfortunately, there is no easy way to ask the regmap

> if its mmio/fastio.


I don't see how it is an impediment.
Prerequisite patch?

> [1]

> https://elixir.bootlin.com/linux/v5.12/source/drivers/gpio/gpio-regmap.c#L257




-- 
With Best Regards,
Andy Shevchenko
Michael Walle April 28, 2021, 11:57 a.m. | #3
Am 2021-04-28 13:07, schrieb Andy Shevchenko:
> On Wed, Apr 28, 2021 at 1:51 AM Michael Walle <michael@walle.cc> wrote:

>> Am 2021-04-26 12:29, schrieb Andy Shevchenko:

>> > On Mon, Apr 26, 2021 at 12:55 PM Thomas Bogendoerfer

>> > <tsbogend@alpha.franken.de> wrote:

>> >

>> > 2) there is gpio-regmap generic code, that may be worth

>> > considering.

>> 

>> This driver uses memory mapped registers. While that is

>> also possible with gpio-regmap, there is one drawback:

>> it assumes gpiochip->can_sleep = true for now, see [1].

>> Unfortunately, there is no easy way to ask the regmap

>> if its mmio/fastio.

> 

> I don't see how it is an impediment.


You'd have to use the *_cansleep() variants with the gpios,
which cannot be used everywhere, no?

-michael
Andy Shevchenko April 28, 2021, 1:44 p.m. | #4
On Wed, Apr 28, 2021 at 2:57 PM Michael Walle <michael@walle.cc> wrote:
>

> Am 2021-04-28 13:07, schrieb Andy Shevchenko:

> > On Wed, Apr 28, 2021 at 1:51 AM Michael Walle <michael@walle.cc> wrote:

> >> Am 2021-04-26 12:29, schrieb Andy Shevchenko:

> >> > On Mon, Apr 26, 2021 at 12:55 PM Thomas Bogendoerfer

> >> > <tsbogend@alpha.franken.de> wrote:

> >> >

> >> > 2) there is gpio-regmap generic code, that may be worth

> >> > considering.

> >>

> >> This driver uses memory mapped registers. While that is

> >> also possible with gpio-regmap, there is one drawback:

> >> it assumes gpiochip->can_sleep = true for now, see [1].

> >> Unfortunately, there is no easy way to ask the regmap

> >> if its mmio/fastio.

> >

> > I don't see how it is an impediment.

>

> You'd have to use the *_cansleep() variants with the gpios,

> which cannot be used everywhere, no?


*can* sleep means that it requires a sleeping context to run, if your
controller is fine with that, there are no worries. OTOH if you want
to run this in an atomic context, then consumers can't do with that
kind of controller. What I meant above (and you stripped it here) is
to add a patch that will fix that and set it based on
gpio_regmap_config.


-- 
With Best Regards,
Andy Shevchenko
Michael Walle April 28, 2021, 2:04 p.m. | #5
Am 2021-04-28 15:44, schrieb Andy Shevchenko:
> On Wed, Apr 28, 2021 at 2:57 PM Michael Walle <michael@walle.cc> wrote:

>> 

>> Am 2021-04-28 13:07, schrieb Andy Shevchenko:

>> > On Wed, Apr 28, 2021 at 1:51 AM Michael Walle <michael@walle.cc> wrote:

>> >> Am 2021-04-26 12:29, schrieb Andy Shevchenko:

>> >> > On Mon, Apr 26, 2021 at 12:55 PM Thomas Bogendoerfer

>> >> > <tsbogend@alpha.franken.de> wrote:

>> >> >

>> >> > 2) there is gpio-regmap generic code, that may be worth

>> >> > considering.

>> >>

>> >> This driver uses memory mapped registers. While that is

>> >> also possible with gpio-regmap, there is one drawback:

>> >> it assumes gpiochip->can_sleep = true for now, see [1].

>> >> Unfortunately, there is no easy way to ask the regmap

>> >> if its mmio/fastio.

>> >

>> > I don't see how it is an impediment.

>> 

>> You'd have to use the *_cansleep() variants with the gpios,

>> which cannot be used everywhere, no?

> 

> *can* sleep means that it requires a sleeping context to run, if your

> controller is fine with that, there are no worries. OTOH if you want

> to run this in an atomic context, then consumers can't do with that

> kind of controller.


Ok, then we are on the same track.

> What I meant above (and you stripped it here) is

> to add a patch that will fix that and set it based on

> gpio_regmap_config.


Yes, but ideally, it would ask the regmap. Otherwise that
information is redundant and might mismatch, i.e. gpio_regmap_config
tell can_sleep=false but the regmap is an I2C type for example. Also
if a driver wants to support both regmap types, we are no step
further.

-michael
Andy Shevchenko April 28, 2021, 2:32 p.m. | #6
On Wed, Apr 28, 2021 at 5:04 PM Michael Walle <michael@walle.cc> wrote:
> Am 2021-04-28 15:44, schrieb Andy Shevchenko:

> > On Wed, Apr 28, 2021 at 2:57 PM Michael Walle <michael@walle.cc> wrote:

> >>

> >> Am 2021-04-28 13:07, schrieb Andy Shevchenko:

> >> > On Wed, Apr 28, 2021 at 1:51 AM Michael Walle <michael@walle.cc> wrote:

> >> >> Am 2021-04-26 12:29, schrieb Andy Shevchenko:

> >> >> > On Mon, Apr 26, 2021 at 12:55 PM Thomas Bogendoerfer

> >> >> > <tsbogend@alpha.franken.de> wrote:

> >> >> >

> >> >> > 2) there is gpio-regmap generic code, that may be worth

> >> >> > considering.

> >> >>

> >> >> This driver uses memory mapped registers. While that is

> >> >> also possible with gpio-regmap, there is one drawback:

> >> >> it assumes gpiochip->can_sleep = true for now, see [1].

> >> >> Unfortunately, there is no easy way to ask the regmap

> >> >> if its mmio/fastio.

> >> >

> >> > I don't see how it is an impediment.

> >>

> >> You'd have to use the *_cansleep() variants with the gpios,

> >> which cannot be used everywhere, no?

> >

> > *can* sleep means that it requires a sleeping context to run, if your

> > controller is fine with that, there are no worries. OTOH if you want

> > to run this in an atomic context, then consumers can't do with that

> > kind of controller.

>

> Ok, then we are on the same track.

>

> > What I meant above (and you stripped it here) is

> > to add a patch that will fix that and set it based on

> > gpio_regmap_config.

>

> Yes, but ideally, it would ask the regmap. Otherwise that

> information is redundant and might mismatch, i.e. gpio_regmap_config

> tell can_sleep=false but the regmap is an I2C type for example. Also

> if a driver wants to support both regmap types, we are no step

> further.


Yeah, I agree that is a band aid, but you are free to fix it actually
on regmap level.
I don't think it will require an enormous amount of work there.

We have time :-)

-- 
With Best Regards,
Andy Shevchenko
Michael Walle April 28, 2021, 2:48 p.m. | #7
[Adding Mark here, too]

Am 2021-04-28 16:32, schrieb Andy Shevchenko:
> On Wed, Apr 28, 2021 at 5:04 PM Michael Walle <michael@walle.cc> wrote:

>> Am 2021-04-28 15:44, schrieb Andy Shevchenko:

>> > On Wed, Apr 28, 2021 at 2:57 PM Michael Walle <michael@walle.cc> wrote:

>> >>

>> >> Am 2021-04-28 13:07, schrieb Andy Shevchenko:

>> >> > On Wed, Apr 28, 2021 at 1:51 AM Michael Walle <michael@walle.cc> wrote:

>> >> >> Am 2021-04-26 12:29, schrieb Andy Shevchenko:

>> >> >> > On Mon, Apr 26, 2021 at 12:55 PM Thomas Bogendoerfer

>> >> >> > <tsbogend@alpha.franken.de> wrote:

>> >> >> >

>> >> >> > 2) there is gpio-regmap generic code, that may be worth

>> >> >> > considering.

>> >> >>

>> >> >> This driver uses memory mapped registers. While that is

>> >> >> also possible with gpio-regmap, there is one drawback:

>> >> >> it assumes gpiochip->can_sleep = true for now, see [1].

>> >> >> Unfortunately, there is no easy way to ask the regmap

>> >> >> if its mmio/fastio.

>> >> >

>> >> > I don't see how it is an impediment.

>> >>

>> >> You'd have to use the *_cansleep() variants with the gpios,

>> >> which cannot be used everywhere, no?

>> >

>> > *can* sleep means that it requires a sleeping context to run, if your

>> > controller is fine with that, there are no worries. OTOH if you want

>> > to run this in an atomic context, then consumers can't do with that

>> > kind of controller.

>> 

>> Ok, then we are on the same track.

>> 

>> > What I meant above (and you stripped it here) is

>> > to add a patch that will fix that and set it based on

>> > gpio_regmap_config.

>> 

>> Yes, but ideally, it would ask the regmap. Otherwise that

>> information is redundant and might mismatch, i.e. gpio_regmap_config

>> tell can_sleep=false but the regmap is an I2C type for example. Also

>> if a driver wants to support both regmap types, we are no step

>> further.

> 

> Yeah, I agree that is a band aid, but you are free to fix it actually

> on regmap level.

> I don't think it will require an enormous amount of work there.


I'd love to fix that, but Mark was against exposing that property
outside of regmap. So it it what it is for now ;) Maybe he'll change
his mind or someone has another idea.

-michael
Andy Shevchenko April 28, 2021, 3:02 p.m. | #8
On Wed, Apr 28, 2021 at 5:48 PM Michael Walle <michael@walle.cc> wrote:
>

> [Adding Mark here, too]

>

> Am 2021-04-28 16:32, schrieb Andy Shevchenko:

> > On Wed, Apr 28, 2021 at 5:04 PM Michael Walle <michael@walle.cc> wrote:

> >> Am 2021-04-28 15:44, schrieb Andy Shevchenko:

> >> > On Wed, Apr 28, 2021 at 2:57 PM Michael Walle <michael@walle.cc> wrote:

> >> >>

> >> >> Am 2021-04-28 13:07, schrieb Andy Shevchenko:

> >> >> > On Wed, Apr 28, 2021 at 1:51 AM Michael Walle <michael@walle.cc> wrote:

> >> >> >> Am 2021-04-26 12:29, schrieb Andy Shevchenko:

> >> >> >> > On Mon, Apr 26, 2021 at 12:55 PM Thomas Bogendoerfer

> >> >> >> > <tsbogend@alpha.franken.de> wrote:

> >> >> >> >

> >> >> >> > 2) there is gpio-regmap generic code, that may be worth

> >> >> >> > considering.

> >> >> >>

> >> >> >> This driver uses memory mapped registers. While that is

> >> >> >> also possible with gpio-regmap, there is one drawback:

> >> >> >> it assumes gpiochip->can_sleep = true for now, see [1].

> >> >> >> Unfortunately, there is no easy way to ask the regmap

> >> >> >> if its mmio/fastio.

> >> >> >

> >> >> > I don't see how it is an impediment.

> >> >>

> >> >> You'd have to use the *_cansleep() variants with the gpios,

> >> >> which cannot be used everywhere, no?

> >> >

> >> > *can* sleep means that it requires a sleeping context to run, if your

> >> > controller is fine with that, there are no worries. OTOH if you want

> >> > to run this in an atomic context, then consumers can't do with that

> >> > kind of controller.

> >>

> >> Ok, then we are on the same track.

> >>

> >> > What I meant above (and you stripped it here) is

> >> > to add a patch that will fix that and set it based on

> >> > gpio_regmap_config.

> >>

> >> Yes, but ideally, it would ask the regmap. Otherwise that

> >> information is redundant and might mismatch, i.e. gpio_regmap_config

> >> tell can_sleep=false but the regmap is an I2C type for example. Also

> >> if a driver wants to support both regmap types, we are no step

> >> further.

> >

> > Yeah, I agree that is a band aid, but you are free to fix it actually

> > on regmap level.

> > I don't think it will require an enormous amount of work there.

>

> I'd love to fix that, but Mark was against exposing that property

> outside of regmap. So it it what it is for now ;) Maybe he'll change

> his mind or someone has another idea.


Then let's go to ugly variant with duplicating it in gpio-regmap
config. with a FIXME note or so. I don't think we should allow new
drivers be based on bgpio_init().

-- 
With Best Regards,
Andy Shevchenko
Michael Walle April 28, 2021, 3:07 p.m. | #9
Am 2021-04-28 17:02, schrieb Andy Shevchenko:
> On Wed, Apr 28, 2021 at 5:48 PM Michael Walle <michael@walle.cc> wrote:

>> 

>> [Adding Mark here, too]

>> 

>> Am 2021-04-28 16:32, schrieb Andy Shevchenko:

>> > On Wed, Apr 28, 2021 at 5:04 PM Michael Walle <michael@walle.cc> wrote:

>> >> Am 2021-04-28 15:44, schrieb Andy Shevchenko:

>> >> > On Wed, Apr 28, 2021 at 2:57 PM Michael Walle <michael@walle.cc> wrote:

>> >> >>

>> >> >> Am 2021-04-28 13:07, schrieb Andy Shevchenko:

>> >> >> > On Wed, Apr 28, 2021 at 1:51 AM Michael Walle <michael@walle.cc> wrote:

>> >> >> >> Am 2021-04-26 12:29, schrieb Andy Shevchenko:

>> >> >> >> > On Mon, Apr 26, 2021 at 12:55 PM Thomas Bogendoerfer

>> >> >> >> > <tsbogend@alpha.franken.de> wrote:

>> >> >> >> >

>> >> >> >> > 2) there is gpio-regmap generic code, that may be worth

>> >> >> >> > considering.

>> >> >> >>

>> >> >> >> This driver uses memory mapped registers. While that is

>> >> >> >> also possible with gpio-regmap, there is one drawback:

>> >> >> >> it assumes gpiochip->can_sleep = true for now, see [1].

>> >> >> >> Unfortunately, there is no easy way to ask the regmap

>> >> >> >> if its mmio/fastio.

>> >> >> >

>> >> >> > I don't see how it is an impediment.

>> >> >>

>> >> >> You'd have to use the *_cansleep() variants with the gpios,

>> >> >> which cannot be used everywhere, no?

>> >> >

>> >> > *can* sleep means that it requires a sleeping context to run, if your

>> >> > controller is fine with that, there are no worries. OTOH if you want

>> >> > to run this in an atomic context, then consumers can't do with that

>> >> > kind of controller.

>> >>

>> >> Ok, then we are on the same track.

>> >>

>> >> > What I meant above (and you stripped it here) is

>> >> > to add a patch that will fix that and set it based on

>> >> > gpio_regmap_config.

>> >>

>> >> Yes, but ideally, it would ask the regmap. Otherwise that

>> >> information is redundant and might mismatch, i.e. gpio_regmap_config

>> >> tell can_sleep=false but the regmap is an I2C type for example. Also

>> >> if a driver wants to support both regmap types, we are no step

>> >> further.

>> >

>> > Yeah, I agree that is a band aid, but you are free to fix it actually

>> > on regmap level.

>> > I don't think it will require an enormous amount of work there.

>> 

>> I'd love to fix that, but Mark was against exposing that property

>> outside of regmap. So it it what it is for now ;) Maybe he'll change

>> his mind or someone has another idea.

> 

> Then let's go to ugly variant with duplicating it in gpio-regmap

> config. with a FIXME note or so. I don't think we should allow new

> drivers be based on bgpio_init().


Agreed, given that a possible fix should be easy enough later.

-michael

Patch

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index e3607ec4c2e8..90543a95dbb8 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -770,6 +770,18 @@  config GPIO_MSC313
 	  Say Y here to support the main GPIO block on MStar/SigmaStar
 	  ARMv7 based SoCs.
 
+config GPIO_IDT3243X
+	tristate "IDT 79RC3243X GPIO support"
+	depends on MIKROTIK_RB532 || COMPILE_TEST
+	select GPIO_GENERIC
+	select GPIOLIB_IRQCHIP
+	help
+	  Select this option to enable GPIO driver for
+	  IDT 79RC3243X based devices like Mikrotik RB532.
+
+	  To compile this driver as a module, choose M here: the module will
+	  be called gpio-idt3243x.
+
 endmenu
 
 menu "Port-mapped I/O GPIO drivers"
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index c58a90a3c3b1..75dd9c5665c5 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -67,6 +67,7 @@  obj-$(CONFIG_GPIO_HISI)                 += gpio-hisi.o
 obj-$(CONFIG_GPIO_HLWD)			+= gpio-hlwd.o
 obj-$(CONFIG_HTC_EGPIO)			+= gpio-htc-egpio.o
 obj-$(CONFIG_GPIO_ICH)			+= gpio-ich.o
+obj-$(CONFIG_GPIO_IDT3243X)		+= gpio-idt3243x.o
 obj-$(CONFIG_GPIO_IOP)			+= gpio-iop.o
 obj-$(CONFIG_GPIO_IT87)			+= gpio-it87.o
 obj-$(CONFIG_GPIO_IXP4XX)		+= gpio-ixp4xx.o
diff --git a/drivers/gpio/gpio-idt3243x.c b/drivers/gpio/gpio-idt3243x.c
new file mode 100644
index 000000000000..62e5643a0228
--- /dev/null
+++ b/drivers/gpio/gpio-idt3243x.c
@@ -0,0 +1,209 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/* Driver for IDT/Renesas 79RC3243x Interrupt Controller  */
+
+#include <linux/gpio/driver.h>
+#include <linux/irq.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/platform_device.h>
+
+#define IDT_PIC_IRQ_PEND	0x00
+#define IDT_PIC_IRQ_MASK	0x08
+
+#define IDT_GPIO_DIR		0x00
+#define IDT_GPIO_DATA		0x04
+#define IDT_GPIO_ILEVEL		0x08
+#define IDT_GPIO_ISTAT		0x0C
+
+struct idt_gpio_ctrl {
+	struct gpio_chip gc;
+	void __iomem *pic;
+	void __iomem *gpio;
+	u32 mask_cache;
+	spinlock_t irq_lock; /* serialize access to irq registers */
+};
+
+static void idt_gpio_dispatch(struct irq_desc *desc)
+{
+	struct gpio_chip *gc = irq_desc_get_handler_data(desc);
+	struct idt_gpio_ctrl *ctrl = gpiochip_get_data(gc);
+	struct irq_chip *host_chip = irq_desc_get_chip(desc);
+	unsigned int bit, virq;
+	unsigned long pending;
+
+	chained_irq_enter(host_chip, desc);
+
+	pending = readl(ctrl->pic + IDT_PIC_IRQ_PEND);
+	pending &= ~ctrl->mask_cache;
+	for_each_set_bit(bit, &pending, gc->ngpio) {
+		virq = irq_linear_revmap(gc->irq.domain, bit);
+		if (virq)
+			generic_handle_irq(virq);
+	}
+
+	chained_irq_exit(host_chip, desc);
+}
+
+static int idt_gpio_irq_set_type(struct irq_data *d, unsigned int flow_type)
+{
+	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+	struct idt_gpio_ctrl *ctrl = gpiochip_get_data(gc);
+	unsigned int sense = flow_type & IRQ_TYPE_SENSE_MASK;
+	unsigned long flags;
+	u32 ilevel;
+
+	/* hardware only supports level triggered */
+	if (sense & IRQ_TYPE_EDGE_BOTH)
+		return -EINVAL;
+
+	if (sense == 0 || sense == IRQ_TYPE_LEVEL_MASK)
+		return -EINVAL;
+
+	spin_lock_irqsave(&ctrl->irq_lock, flags);
+
+	ilevel = readl(ctrl->gpio + IDT_GPIO_ILEVEL);
+	if (sense & IRQ_TYPE_LEVEL_HIGH)
+		ilevel |= BIT(d->hwirq);
+	else if (sense & IRQ_TYPE_LEVEL_LOW)
+		ilevel &= ~BIT(d->hwirq);
+
+	writel(ilevel, ctrl->gpio + IDT_GPIO_ILEVEL);
+	irq_set_handler_locked(d, handle_level_irq);
+
+	spin_unlock_irqrestore(&ctrl->irq_lock, flags);
+	return 0;
+}
+
+static void idt_gpio_ack(struct irq_data *d)
+{
+	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+	struct idt_gpio_ctrl *ctrl = gpiochip_get_data(gc);
+
+	writel(~BIT(d->hwirq), ctrl->gpio + IDT_GPIO_ISTAT);
+}
+
+static void idt_gpio_mask(struct irq_data *d)
+{
+	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+	struct idt_gpio_ctrl *ctrl = gpiochip_get_data(gc);
+	unsigned long flags;
+
+	spin_lock_irqsave(&ctrl->irq_lock, flags);
+
+	ctrl->mask_cache |= BIT(d->hwirq);
+	writel(ctrl->mask_cache, ctrl->pic + IDT_PIC_IRQ_MASK);
+
+	spin_unlock_irqrestore(&ctrl->irq_lock, flags);
+}
+
+static void idt_gpio_unmask(struct irq_data *d)
+{
+	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+	struct idt_gpio_ctrl *ctrl = gpiochip_get_data(gc);
+	unsigned long flags;
+
+	spin_lock_irqsave(&ctrl->irq_lock, flags);
+
+	ctrl->mask_cache &= ~BIT(d->hwirq);
+	writel(ctrl->mask_cache, ctrl->pic + IDT_PIC_IRQ_MASK);
+
+	spin_unlock_irqrestore(&ctrl->irq_lock, flags);
+}
+
+static int idt_gpio_irq_init_hw(struct gpio_chip *gc)
+{
+	struct idt_gpio_ctrl *ctrl = gpiochip_get_data(gc);
+
+	/* Mask interrupts. */
+	ctrl->mask_cache = 0xffffffff;
+	writel(ctrl->mask_cache, ctrl->pic + IDT_PIC_IRQ_MASK);
+
+	return 0;
+}
+
+static struct irq_chip idt_gpio_irqchip = {
+	.name = "IDTGPIO",
+	.irq_mask = idt_gpio_mask,
+	.irq_ack = idt_gpio_ack,
+	.irq_unmask = idt_gpio_unmask,
+	.irq_set_type = idt_gpio_irq_set_type
+};
+
+static int idt_gpio_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct gpio_irq_chip *girq;
+	struct idt_gpio_ctrl *ctrl;
+	unsigned int parent_irq;
+	int ngpios;
+	int ret;
+
+	ret = device_property_read_u32(dev, "ngpios", &ngpios);
+	if (ret) {
+		dev_err(dev, "ngpios property is not valid\n");
+		return ret;
+	}
+
+	ctrl = devm_kzalloc(dev, sizeof(*ctrl), GFP_KERNEL);
+	if (!ctrl)
+		return -ENOMEM;
+
+	ctrl->gpio = devm_platform_ioremap_resource_byname(pdev, "gpio");
+	if (!ctrl->gpio)
+		return -ENOMEM;
+
+	ctrl->gc.parent = dev;
+
+	ret = bgpio_init(&ctrl->gc, &pdev->dev, 4, ctrl->gpio + IDT_GPIO_DATA,
+			 NULL, NULL, ctrl->gpio + IDT_GPIO_DIR, NULL, 0);
+	if (ret) {
+		dev_err(dev, "bgpio_init failed\n");
+		return ret;
+	}
+	ctrl->gc.ngpio = ngpios;
+
+	ctrl->pic = devm_platform_ioremap_resource_byname(pdev, "pic");
+	if (!ctrl->pic)
+		return -ENOMEM;
+
+	parent_irq = platform_get_irq(pdev, 0);
+	if (!parent_irq)
+		return -EINVAL;
+
+	girq = &ctrl->gc.irq;
+	girq->chip = &idt_gpio_irqchip;
+	girq->init_hw = idt_gpio_irq_init_hw;
+	girq->parent_handler = idt_gpio_dispatch;
+	girq->num_parents = 1;
+	girq->parents = devm_kcalloc(dev, girq->num_parents,
+				     sizeof(*girq->parents), GFP_KERNEL);
+	if (!girq->parents)
+		return -ENOMEM;
+
+	girq->parents[0] = parent_irq;
+	girq->default_type = IRQ_TYPE_NONE;
+	girq->handler = handle_bad_irq;
+
+	spin_lock_init(&ctrl->irq_lock);
+
+	return devm_gpiochip_add_data(&pdev->dev, &ctrl->gc, ctrl);
+}
+
+static const struct of_device_id idt_gpio_of_match[] = {
+	{ .compatible = "idt,32434-gpio" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, idt_gpio_of_match);
+
+static struct platform_driver idt_gpio_driver = {
+	.probe = idt_gpio_probe,
+	.driver = {
+		.name = "idt3243x-gpio",
+		.of_match_table = idt_gpio_of_match,
+	},
+};
+module_platform_driver(idt_gpio_driver);
+
+MODULE_DESCRIPTION("IDT 79RC3243x GPIO/PIC Driver");
+MODULE_AUTHOR("Thomas Bogendoerfer <tsbogend@alpha.franken.de>");
+MODULE_LICENSE("GPL");