Re: [PATCH v4] gpio: add Intel WhiskeyCove GPIO driver

2016-07-11 Thread Bin Gao 
On Wed, Jul 06, 2016 at 10:57:19AM +0200, Linus Walleij wrote:
> > +   gpiochip_irqchip_add(>chip, _irqchip, 0,
> > +handle_simple_irq, IRQ_TYPE_NONE);
> 
> Reexamine the use of handle_simple_irq() here. We have two kinds of
> irq hardware: those with one register for ACKing and reading the status
> of an IRQ, and those with two registers for it: one where you ACK the
> IRQ (so it can immediately re-trigger) and one to read the status of
> whether it happened. Sometimes different handling is needed for
> levek and edge IRQs even (c.f. gpio-pl061.c).
> 
> Only the hardware with just one register for both things should use
> handle_simple_irq(). This seems to be the case here but I want you
> to verify.

Yes, our case is handle_simple_irq(), not handle_edge_irq(), handle_level_irq() 
or
handle_fasteoi_irq(), etc. because there is no ACK mechanism inside the
GPIO controller's interrupt logic - all we need to do is read the status
register to get the status and write-to-clear the status register so that
a new interrupt can be triggered, i.e. there is only one register for both.

> 
> Yours,
> Linus Walleij

Re: [PATCH v4] gpio: add Intel WhiskeyCove GPIO driver

2016-07-11 Thread Bin Gao 
On Wed, Jul 06, 2016 at 10:57:19AM +0200, Linus Walleij wrote:
> > +   gpiochip_irqchip_add(>chip, _irqchip, 0,
> > +handle_simple_irq, IRQ_TYPE_NONE);
> 
> Reexamine the use of handle_simple_irq() here. We have two kinds of
> irq hardware: those with one register for ACKing and reading the status
> of an IRQ, and those with two registers for it: one where you ACK the
> IRQ (so it can immediately re-trigger) and one to read the status of
> whether it happened. Sometimes different handling is needed for
> levek and edge IRQs even (c.f. gpio-pl061.c).
> 
> Only the hardware with just one register for both things should use
> handle_simple_irq(). This seems to be the case here but I want you
> to verify.

Yes, our case is handle_simple_irq(), not handle_edge_irq(), handle_level_irq() 
or
handle_fasteoi_irq(), etc. because there is no ACK mechanism inside the
GPIO controller's interrupt logic - all we need to do is read the status
register to get the status and write-to-clear the status register so that
a new interrupt can be triggered, i.e. there is only one register for both.

> 
> Yours,
> Linus Walleij

Re: [PATCH v4] gpio: add Intel WhiskeyCove GPIO driver

2016-07-06 Thread Bin Gao 
On Wed, Jul 06, 2016 at 01:07:15PM +0300, Mika Westerberg wrote:
> On Wed, Jul 06, 2016 at 10:57:19AM +0200, Linus Walleij wrote:
> > On Tue, Jun 28, 2016 at 1:56 AM, Bin Gao  wrote:
> > 
> > > This patch introduces a separate GPIO driver for Intel WhiskeyCove PMIC.
> > > This driver is based on gpio-crystalcove.c.
> > >
> > > Signed-off-by: Ajay Thomas 
> > > Signed-off-by: Bin Gao 
> > > ---
> > > Changes in v4:
> > >  - Converted CTLI_INTCNT_XX macros to less verbose ones INT_DETECT_XX.
> > >  - Add comments about why there is no .pm for the driver.
> > >  - Header files re-ordered.
> > >  - Various coding style change to address Andy's comments.
> > 
> > Mika can I have your ACK/review tag on this driver so I can merge it?
> > I prefer to have all Intel stuff bearing your seal of approval.
> 
> Thanks for your trust :)
> 
> I don't have much comments in addition to what you already pointed out.
> I'll just wait for the next revision and give my ack then.
> 
> > > +static irqreturn_t wcove_gpio_irq_handler(int irq, void *data)
> > > +{
> > > +   int pending;
> > > +   unsigned int p0, p1, virq, gpio;
> > > +   struct wcove_gpio *wg = data;
> 
> Bin,
> 
> Since you are going to make another iteration, please arrange the
> declarations like:
> 
>   unsigned int p0, p1, virq, gpio;
>   struct wcove_gpio *wg = data;
>   int pending;

Yes, will do. Thanks.

-Bin

Re: [PATCH v4] gpio: add Intel WhiskeyCove GPIO driver

2016-07-06 Thread Bin Gao 
On Wed, Jul 06, 2016 at 01:07:15PM +0300, Mika Westerberg wrote:
> On Wed, Jul 06, 2016 at 10:57:19AM +0200, Linus Walleij wrote:
> > On Tue, Jun 28, 2016 at 1:56 AM, Bin Gao  wrote:
> > 
> > > This patch introduces a separate GPIO driver for Intel WhiskeyCove PMIC.
> > > This driver is based on gpio-crystalcove.c.
> > >
> > > Signed-off-by: Ajay Thomas 
> > > Signed-off-by: Bin Gao 
> > > ---
> > > Changes in v4:
> > >  - Converted CTLI_INTCNT_XX macros to less verbose ones INT_DETECT_XX.
> > >  - Add comments about why there is no .pm for the driver.
> > >  - Header files re-ordered.
> > >  - Various coding style change to address Andy's comments.
> > 
> > Mika can I have your ACK/review tag on this driver so I can merge it?
> > I prefer to have all Intel stuff bearing your seal of approval.
> 
> Thanks for your trust :)
> 
> I don't have much comments in addition to what you already pointed out.
> I'll just wait for the next revision and give my ack then.
> 
> > > +static irqreturn_t wcove_gpio_irq_handler(int irq, void *data)
> > > +{
> > > +   int pending;
> > > +   unsigned int p0, p1, virq, gpio;
> > > +   struct wcove_gpio *wg = data;
> 
> Bin,
> 
> Since you are going to make another iteration, please arrange the
> declarations like:
> 
>   unsigned int p0, p1, virq, gpio;
>   struct wcove_gpio *wg = data;
>   int pending;

Yes, will do. Thanks.

-Bin

Re: [PATCH v4] gpio: add Intel WhiskeyCove GPIO driver

2016-07-06 Thread Bin Gao 
On Wed, Jul 06, 2016 at 10:57:19AM +0200, Linus Walleij wrote:
> > +static irqreturn_t wcove_gpio_irq_handler(int irq, void *data)
> > +{
> > +   int pending;
> > +   unsigned int p0, p1, virq, gpio;
> > +   struct wcove_gpio *wg = data;
> > +
> > +   if (regmap_read(wg->regmap, IRQ_STATUS_OFFSET + 0, ) ||
> > +   regmap_read(wg->regmap, IRQ_STATUS_OFFSET + 1, )) {
> 
> Why can't you use regmap_bulk_read() here?

Will fix this in v5.

> 
> > +   dev_err(wg->chip.parent, "%s(): regmap_read() failed.\n",
> > +   __func__);
> > +   return IRQ_NONE;
> > +   }
> > +
> > +   pending = p0 | (p1 << 8);
> > +
> > +   for (gpio = 0; gpio < WCOVE_GPIO_NUM; gpio++) {
> > +   if (pending & BIT(gpio)) {
> > +   virq = irq_find_mapping(wg->chip.irqdomain, gpio);
> > +   handle_nested_irq(virq);
> > +   }
> > +   }
> > +
> > +   regmap_write(wg->regmap, IRQ_STATUS_OFFSET + 0, p0);
> > +   regmap_write(wg->regmap, IRQ_STATUS_OFFSET + 1, p1);
> 
> Use regmap_bulk_write()?

Will fix this in v5.

> 
> Also you're ignoring the return error code. Check it and dev_err() if
> it fails.

Yes, will fix.

> 
> This loop seems like it could miss interrupts happening while
> processing. Especially edge interrupts, and thatr will lead to serious
> bugs later.
> 
> Please consider the following construction:
> 
> 1. read status register
> 2. Any IRQs active?
>   2.1 No IRQs active: if this is the FIRST iteration, exit with IRQ_NONE
>   2.2 No IRQs active If this the second iteration or later, exit with
> IRQ_HANDLED
>   2.3 IRQs active, continue
> 2. Find first active IRQ
> 3. Handle first active IRQ
> 4. ACK the first active IRQ by writing the status register
> 5. Reiterate from 1
> 
> This way, if two IRQs happen at the same time, or if a new IRQ appears
> while you're inside the interrupt handler, it gets served.

I agree. Writing to status register should be done bit by bit, instead of
one write for all bits. Will fix this in v5.

> 
> > +static void wcove_gpio_dbg_show(struct seq_file *s, struct gpio_chip *chip)
> > +{
> > +   struct wcove_gpio *wg = gpiochip_get_data(chip);
> > +   int gpio, offset, group;
> > +   unsigned int ctlo, ctli, irq_mask, irq_status;
> > +
> > +   for (gpio = 0; gpio < WCOVE_GPIO_NUM; gpio++) {
> > +   group = gpio < GROUP0_NR_IRQS ? 0 : 1;
> > +   regmap_read(wg->regmap, to_reg(gpio, CTRL_OUT), );
> > +   regmap_read(wg->regmap, to_reg(gpio, CTRL_IN), );
> > +   regmap_read(wg->regmap, IRQ_MASK_OFFSET + group, _mask);
> > +   regmap_read(wg->regmap, IRQ_STATUS_OFFSET + group, 
> > _status);
> 
> Ignoring error codes. Fix this.

Will Fix in v5.

> 
> > +   gpiochip_irqchip_add(>chip, _irqchip, 0,
> > +handle_simple_irq, IRQ_TYPE_NONE);
> 
> Reexamine the use of handle_simple_irq() here. We have two kinds of
> irq hardware: those with one register for ACKing and reading the status
> of an IRQ, and those with two registers for it: one where you ACK the
> IRQ (so it can immediately re-trigger) and one to read the status of
> whether it happened. Sometimes different handling is needed for
> levek and edge IRQs even (c.f. gpio-pl061.c).
> 
> Only the hardware with just one register for both things should use
> handle_simple_irq(). This seems to be the case here but I want you
> to verify.

I will check and fix if it's needed.

> 
> Yours,
> Linus Walleij

Thanks for your review.

Re: [PATCH v4] gpio: add Intel WhiskeyCove GPIO driver

2016-07-06 Thread Bin Gao 
On Wed, Jul 06, 2016 at 10:57:19AM +0200, Linus Walleij wrote:
> > +static irqreturn_t wcove_gpio_irq_handler(int irq, void *data)
> > +{
> > +   int pending;
> > +   unsigned int p0, p1, virq, gpio;
> > +   struct wcove_gpio *wg = data;
> > +
> > +   if (regmap_read(wg->regmap, IRQ_STATUS_OFFSET + 0, ) ||
> > +   regmap_read(wg->regmap, IRQ_STATUS_OFFSET + 1, )) {
> 
> Why can't you use regmap_bulk_read() here?

Will fix this in v5.

> 
> > +   dev_err(wg->chip.parent, "%s(): regmap_read() failed.\n",
> > +   __func__);
> > +   return IRQ_NONE;
> > +   }
> > +
> > +   pending = p0 | (p1 << 8);
> > +
> > +   for (gpio = 0; gpio < WCOVE_GPIO_NUM; gpio++) {
> > +   if (pending & BIT(gpio)) {
> > +   virq = irq_find_mapping(wg->chip.irqdomain, gpio);
> > +   handle_nested_irq(virq);
> > +   }
> > +   }
> > +
> > +   regmap_write(wg->regmap, IRQ_STATUS_OFFSET + 0, p0);
> > +   regmap_write(wg->regmap, IRQ_STATUS_OFFSET + 1, p1);
> 
> Use regmap_bulk_write()?

Will fix this in v5.

> 
> Also you're ignoring the return error code. Check it and dev_err() if
> it fails.

Yes, will fix.

> 
> This loop seems like it could miss interrupts happening while
> processing. Especially edge interrupts, and thatr will lead to serious
> bugs later.
> 
> Please consider the following construction:
> 
> 1. read status register
> 2. Any IRQs active?
>   2.1 No IRQs active: if this is the FIRST iteration, exit with IRQ_NONE
>   2.2 No IRQs active If this the second iteration or later, exit with
> IRQ_HANDLED
>   2.3 IRQs active, continue
> 2. Find first active IRQ
> 3. Handle first active IRQ
> 4. ACK the first active IRQ by writing the status register
> 5. Reiterate from 1
> 
> This way, if two IRQs happen at the same time, or if a new IRQ appears
> while you're inside the interrupt handler, it gets served.

I agree. Writing to status register should be done bit by bit, instead of
one write for all bits. Will fix this in v5.

> 
> > +static void wcove_gpio_dbg_show(struct seq_file *s, struct gpio_chip *chip)
> > +{
> > +   struct wcove_gpio *wg = gpiochip_get_data(chip);
> > +   int gpio, offset, group;
> > +   unsigned int ctlo, ctli, irq_mask, irq_status;
> > +
> > +   for (gpio = 0; gpio < WCOVE_GPIO_NUM; gpio++) {
> > +   group = gpio < GROUP0_NR_IRQS ? 0 : 1;
> > +   regmap_read(wg->regmap, to_reg(gpio, CTRL_OUT), );
> > +   regmap_read(wg->regmap, to_reg(gpio, CTRL_IN), );
> > +   regmap_read(wg->regmap, IRQ_MASK_OFFSET + group, _mask);
> > +   regmap_read(wg->regmap, IRQ_STATUS_OFFSET + group, 
> > _status);
> 
> Ignoring error codes. Fix this.

Will Fix in v5.

> 
> > +   gpiochip_irqchip_add(>chip, _irqchip, 0,
> > +handle_simple_irq, IRQ_TYPE_NONE);
> 
> Reexamine the use of handle_simple_irq() here. We have two kinds of
> irq hardware: those with one register for ACKing and reading the status
> of an IRQ, and those with two registers for it: one where you ACK the
> IRQ (so it can immediately re-trigger) and one to read the status of
> whether it happened. Sometimes different handling is needed for
> levek and edge IRQs even (c.f. gpio-pl061.c).
> 
> Only the hardware with just one register for both things should use
> handle_simple_irq(). This seems to be the case here but I want you
> to verify.

I will check and fix if it's needed.

> 
> Yours,
> Linus Walleij

Thanks for your review.

Re: [PATCH v4] gpio: add Intel WhiskeyCove GPIO driver

2016-07-06 Thread Mika Westerberg 
On Wed, Jul 06, 2016 at 10:57:19AM +0200, Linus Walleij wrote:
> On Tue, Jun 28, 2016 at 1:56 AM, Bin Gao  wrote:
> 
> > This patch introduces a separate GPIO driver for Intel WhiskeyCove PMIC.
> > This driver is based on gpio-crystalcove.c.
> >
> > Signed-off-by: Ajay Thomas 
> > Signed-off-by: Bin Gao 
> > ---
> > Changes in v4:
> >  - Converted CTLI_INTCNT_XX macros to less verbose ones INT_DETECT_XX.
> >  - Add comments about why there is no .pm for the driver.
> >  - Header files re-ordered.
> >  - Various coding style change to address Andy's comments.
> 
> Mika can I have your ACK/review tag on this driver so I can merge it?
> I prefer to have all Intel stuff bearing your seal of approval.

Thanks for your trust :)

I don't have much comments in addition to what you already pointed out.
I'll just wait for the next revision and give my ack then.

> > +static irqreturn_t wcove_gpio_irq_handler(int irq, void *data)
> > +{
> > +   int pending;
> > +   unsigned int p0, p1, virq, gpio;
> > +   struct wcove_gpio *wg = data;

Bin,

Since you are going to make another iteration, please arrange the
declarations like:

unsigned int p0, p1, virq, gpio;
struct wcove_gpio *wg = data;
int pending;

Re: [PATCH v4] gpio: add Intel WhiskeyCove GPIO driver

2016-07-06 Thread Mika Westerberg 
On Wed, Jul 06, 2016 at 10:57:19AM +0200, Linus Walleij wrote:
> On Tue, Jun 28, 2016 at 1:56 AM, Bin Gao  wrote:
> 
> > This patch introduces a separate GPIO driver for Intel WhiskeyCove PMIC.
> > This driver is based on gpio-crystalcove.c.
> >
> > Signed-off-by: Ajay Thomas 
> > Signed-off-by: Bin Gao 
> > ---
> > Changes in v4:
> >  - Converted CTLI_INTCNT_XX macros to less verbose ones INT_DETECT_XX.
> >  - Add comments about why there is no .pm for the driver.
> >  - Header files re-ordered.
> >  - Various coding style change to address Andy's comments.
> 
> Mika can I have your ACK/review tag on this driver so I can merge it?
> I prefer to have all Intel stuff bearing your seal of approval.

Thanks for your trust :)

I don't have much comments in addition to what you already pointed out.
I'll just wait for the next revision and give my ack then.

> > +static irqreturn_t wcove_gpio_irq_handler(int irq, void *data)
> > +{
> > +   int pending;
> > +   unsigned int p0, p1, virq, gpio;
> > +   struct wcove_gpio *wg = data;

Bin,

Since you are going to make another iteration, please arrange the
declarations like:

unsigned int p0, p1, virq, gpio;
struct wcove_gpio *wg = data;
int pending;

Re: [PATCH v4] gpio: add Intel WhiskeyCove GPIO driver

2016-07-06 Thread Linus Walleij 
On Tue, Jun 28, 2016 at 1:56 AM, Bin Gao  wrote:

> This patch introduces a separate GPIO driver for Intel WhiskeyCove PMIC.
> This driver is based on gpio-crystalcove.c.
>
> Signed-off-by: Ajay Thomas 
> Signed-off-by: Bin Gao 
> ---
> Changes in v4:
>  - Converted CTLI_INTCNT_XX macros to less verbose ones INT_DETECT_XX.
>  - Add comments about why there is no .pm for the driver.
>  - Header files re-ordered.
>  - Various coding style change to address Andy's comments.

Mika can I have your ACK/review tag on this driver so I can merge it?
I prefer to have all Intel stuff bearing your seal of approval.

> +static irqreturn_t wcove_gpio_irq_handler(int irq, void *data)
> +{
> +   int pending;
> +   unsigned int p0, p1, virq, gpio;
> +   struct wcove_gpio *wg = data;
> +
> +   if (regmap_read(wg->regmap, IRQ_STATUS_OFFSET + 0, ) ||
> +   regmap_read(wg->regmap, IRQ_STATUS_OFFSET + 1, )) {

Why can't you use regmap_bulk_read() here?

> +   dev_err(wg->chip.parent, "%s(): regmap_read() failed.\n",
> +   __func__);
> +   return IRQ_NONE;
> +   }
> +
> +   pending = p0 | (p1 << 8);
> +
> +   for (gpio = 0; gpio < WCOVE_GPIO_NUM; gpio++) {
> +   if (pending & BIT(gpio)) {
> +   virq = irq_find_mapping(wg->chip.irqdomain, gpio);
> +   handle_nested_irq(virq);
> +   }
> +   }
> +
> +   regmap_write(wg->regmap, IRQ_STATUS_OFFSET + 0, p0);
> +   regmap_write(wg->regmap, IRQ_STATUS_OFFSET + 1, p1);

Use regmap_bulk_write()?

Also you're ignoring the return error code. Check it and dev_err() if
it fails.

This loop seems like it could miss interrupts happening while
processing. Especially edge interrupts, and thatr will lead to serious
bugs later.

Please consider the following construction:

1. read status register
2. Any IRQs active?
  2.1 No IRQs active: if this is the FIRST iteration, exit with IRQ_NONE
  2.2 No IRQs active If this the second iteration or later, exit with
IRQ_HANDLED
  2.3 IRQs active, continue
2. Find first active IRQ
3. Handle first active IRQ
4. ACK the first active IRQ by writing the status register
5. Reiterate from 1

This way, if two IRQs happen at the same time, or if a new IRQ appears
while you're inside the interrupt handler, it gets served.

> +static void wcove_gpio_dbg_show(struct seq_file *s, struct gpio_chip *chip)
> +{
> +   struct wcove_gpio *wg = gpiochip_get_data(chip);
> +   int gpio, offset, group;
> +   unsigned int ctlo, ctli, irq_mask, irq_status;
> +
> +   for (gpio = 0; gpio < WCOVE_GPIO_NUM; gpio++) {
> +   group = gpio < GROUP0_NR_IRQS ? 0 : 1;
> +   regmap_read(wg->regmap, to_reg(gpio, CTRL_OUT), );
> +   regmap_read(wg->regmap, to_reg(gpio, CTRL_IN), );
> +   regmap_read(wg->regmap, IRQ_MASK_OFFSET + group, _mask);
> +   regmap_read(wg->regmap, IRQ_STATUS_OFFSET + group, 
> _status);

Ignoring error codes. Fix this.

> +   gpiochip_irqchip_add(>chip, _irqchip, 0,
> +handle_simple_irq, IRQ_TYPE_NONE);

Reexamine the use of handle_simple_irq() here. We have two kinds of
irq hardware: those with one register for ACKing and reading the status
of an IRQ, and those with two registers for it: one where you ACK the
IRQ (so it can immediately re-trigger) and one to read the status of
whether it happened. Sometimes different handling is needed for
levek and edge IRQs even (c.f. gpio-pl061.c).

Only the hardware with just one register for both things should use
handle_simple_irq(). This seems to be the case here but I want you
to verify.

Yours,
Linus Walleij

Re: [PATCH v4] gpio: add Intel WhiskeyCove GPIO driver

2016-07-06 Thread Linus Walleij 
On Tue, Jun 28, 2016 at 1:56 AM, Bin Gao  wrote:

> This patch introduces a separate GPIO driver for Intel WhiskeyCove PMIC.
> This driver is based on gpio-crystalcove.c.
>
> Signed-off-by: Ajay Thomas 
> Signed-off-by: Bin Gao 
> ---
> Changes in v4:
>  - Converted CTLI_INTCNT_XX macros to less verbose ones INT_DETECT_XX.
>  - Add comments about why there is no .pm for the driver.
>  - Header files re-ordered.
>  - Various coding style change to address Andy's comments.

Mika can I have your ACK/review tag on this driver so I can merge it?
I prefer to have all Intel stuff bearing your seal of approval.

> +static irqreturn_t wcove_gpio_irq_handler(int irq, void *data)
> +{
> +   int pending;
> +   unsigned int p0, p1, virq, gpio;
> +   struct wcove_gpio *wg = data;
> +
> +   if (regmap_read(wg->regmap, IRQ_STATUS_OFFSET + 0, ) ||
> +   regmap_read(wg->regmap, IRQ_STATUS_OFFSET + 1, )) {

Why can't you use regmap_bulk_read() here?

> +   dev_err(wg->chip.parent, "%s(): regmap_read() failed.\n",
> +   __func__);
> +   return IRQ_NONE;
> +   }
> +
> +   pending = p0 | (p1 << 8);
> +
> +   for (gpio = 0; gpio < WCOVE_GPIO_NUM; gpio++) {
> +   if (pending & BIT(gpio)) {
> +   virq = irq_find_mapping(wg->chip.irqdomain, gpio);
> +   handle_nested_irq(virq);
> +   }
> +   }
> +
> +   regmap_write(wg->regmap, IRQ_STATUS_OFFSET + 0, p0);
> +   regmap_write(wg->regmap, IRQ_STATUS_OFFSET + 1, p1);

Use regmap_bulk_write()?

Also you're ignoring the return error code. Check it and dev_err() if
it fails.

This loop seems like it could miss interrupts happening while
processing. Especially edge interrupts, and thatr will lead to serious
bugs later.

Please consider the following construction:

1. read status register
2. Any IRQs active?
  2.1 No IRQs active: if this is the FIRST iteration, exit with IRQ_NONE
  2.2 No IRQs active If this the second iteration or later, exit with
IRQ_HANDLED
  2.3 IRQs active, continue
2. Find first active IRQ
3. Handle first active IRQ
4. ACK the first active IRQ by writing the status register
5. Reiterate from 1

This way, if two IRQs happen at the same time, or if a new IRQ appears
while you're inside the interrupt handler, it gets served.

> +static void wcove_gpio_dbg_show(struct seq_file *s, struct gpio_chip *chip)
> +{
> +   struct wcove_gpio *wg = gpiochip_get_data(chip);
> +   int gpio, offset, group;
> +   unsigned int ctlo, ctli, irq_mask, irq_status;
> +
> +   for (gpio = 0; gpio < WCOVE_GPIO_NUM; gpio++) {
> +   group = gpio < GROUP0_NR_IRQS ? 0 : 1;
> +   regmap_read(wg->regmap, to_reg(gpio, CTRL_OUT), );
> +   regmap_read(wg->regmap, to_reg(gpio, CTRL_IN), );
> +   regmap_read(wg->regmap, IRQ_MASK_OFFSET + group, _mask);
> +   regmap_read(wg->regmap, IRQ_STATUS_OFFSET + group, 
> _status);

Ignoring error codes. Fix this.

> +   gpiochip_irqchip_add(>chip, _irqchip, 0,
> +handle_simple_irq, IRQ_TYPE_NONE);

Reexamine the use of handle_simple_irq() here. We have two kinds of
irq hardware: those with one register for ACKing and reading the status
of an IRQ, and those with two registers for it: one where you ACK the
IRQ (so it can immediately re-trigger) and one to read the status of
whether it happened. Sometimes different handling is needed for
levek and edge IRQs even (c.f. gpio-pl061.c).

Only the hardware with just one register for both things should use
handle_simple_irq(). This seems to be the case here but I want you
to verify.

Yours,
Linus Walleij

Re: [PATCH v4] gpio: add Intel WhiskeyCove GPIO driver

2016-07-01 Thread Andy Shevchenko 
On Tue, Jun 28, 2016 at 2:56 AM, Bin Gao  wrote:
> This patch introduces a separate GPIO driver for Intel WhiskeyCove PMIC.
> This driver is based on gpio-crystalcove.c.

There are still minors, though they would be fixed later.
FWIW:
Reviewed-by: Andy Shevchenko 

>
> Signed-off-by: Ajay Thomas 
> Signed-off-by: Bin Gao 
> ---
> Changes in v4:
>  - Converted CTLI_INTCNT_XX macros to less verbose ones INT_DETECT_XX.
>  - Add comments about why there is no .pm for the driver.
>  - Header files re-ordered.
>  - Various coding style change to address Andy's comments.
> Changes in v3:
>  - Fixed the year in copyright line(2015-->2016).
>  - Removed DRV_NAME macro.
>  - Added kernel-doc for regmap_irq_chip of the wcove_gpio structure.
>  - Line length fix.
> Changes in v2:
>  - Typo fix (Whsikey --> Whiskey).
>  - Included linux/gpio/driver.h instead of linux/gpio.h
>  - Implemented .set_single_ended().
>  - Added GPIO register description.
>  - Replaced container_of() with gpiochip_get_data().
>  - Removed unnecessary "if (gpio > WCOVE_VGPIO_NUM" check.
>  - Removed the device id table and added MODULE_ALIAS().
>  drivers/gpio/Kconfig  |  13 ++
>  drivers/gpio/Makefile |   1 +
>  drivers/gpio/gpio-wcove.c | 433 
> ++
>  3 files changed, 447 insertions(+)
>  create mode 100644 drivers/gpio/gpio-wcove.c
>
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index cebcb40..ac74299 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -806,6 +806,19 @@ config GPIO_CRYSTAL_COVE
>   This driver can also be built as a module. If so, the module will be
>   called gpio-crystalcove.
>
> +config GPIO_WHISKEY_COVE
> +   tristate "GPIO support for Whiskey Cove PMIC"
> +   depends on INTEL_SOC_PMIC
> +   select GPIOLIB_IRQCHIP
> +   help
> + Support for GPIO pins on Whiskey Cove PMIC.
> +
> + Say Yes if you have a Intel SoC based tablet with Whiskey Cove PMIC
> + inside.
> +
> + This driver can also be built as a module. If so, the module will be
> + called gpio-wcove.
> +
>  config GPIO_CS5535
> tristate "AMD CS5535/CS5536 GPIO support"
> depends on MFD_CS5535
> diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
> index 991598e..fff6914 100644
> --- a/drivers/gpio/Makefile
> +++ b/drivers/gpio/Makefile
> @@ -34,6 +34,7 @@ obj-$(CONFIG_GPIO_BT8XX)  += gpio-bt8xx.o
>  obj-$(CONFIG_GPIO_CLPS711X)+= gpio-clps711x.o
>  obj-$(CONFIG_GPIO_CS5535)  += gpio-cs5535.o
>  obj-$(CONFIG_GPIO_CRYSTAL_COVE)+= gpio-crystalcove.o
> +obj-$(CONFIG_GPIO_WHISKEY_COVE)+= gpio-wcove.o
>  obj-$(CONFIG_GPIO_DA9052)  += gpio-da9052.o
>  obj-$(CONFIG_GPIO_DA9055)  += gpio-da9055.o
>  obj-$(CONFIG_GPIO_DAVINCI) += gpio-davinci.o
> diff --git a/drivers/gpio/gpio-wcove.c b/drivers/gpio/gpio-wcove.c
> new file mode 100644
> index 000..9f1b25a
> --- /dev/null
> +++ b/drivers/gpio/gpio-wcove.c
> @@ -0,0 +1,433 @@
> +/*
> + * gpio-wcove.c - Intel Whiskey Cove GPIO Driver
> + *
> + * This driver is written based on gpio-crystalcove.c
> + *
> + * Copyright (C) 2016 Intel Corporation. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License version
> + * 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +/*
> + * Whiskey Cove PMIC has 13 physical GPIO pins divided into 3 banks:
> + * Bank 0: Pin 0 - 6
> + * Bank 1: Pin 7 - 10
> + * Bank 2: Pin 11 -12
> + * Each pin has one output control register and one input control register.
> + */
> +#define BANK0_NR_PINS  7
> +#define BANK1_NR_PINS  4
> +#define BANK2_NR_PINS  2
> +#define WCOVE_GPIO_NUM (BANK0_NR_PINS + BANK1_NR_PINS + 
> BANK2_NR_PINS)
> +#define WCOVE_VGPIO_NUM94
> +/* GPIO output control registers(one per pin): 0x4e44 - 0x4e50 */
> +#define OUT_CTRL_OFFSET0x4e44
> +/* GPIO input control registers(one per pin): 0x4e51 - 0x4e5d */
> +#define IN_CTRL_OFFSET 0x4e51
> +
> +/*
> + * GPIO interrupts are organized in two groups:
> + * Group 0: Bank 0 pins (Pin 0 - 6)
> + * Group 1: Bank 1 and Bank 2 pins (Pin 7 - 12)
> + * Each group has two registers(one bit per pin): status and mask.
> + */
> +#define GROUP0_NR_IRQS 7
> +#define GROUP1_NR_IRQS 6
> +#define IRQ_MASK_OFFSET0x4e19
> +#define IRQ_STATUS_OFFSET

Re: [PATCH v4] gpio: add Intel WhiskeyCove GPIO driver

2016-07-01 Thread Andy Shevchenko 
On Tue, Jun 28, 2016 at 2:56 AM, Bin Gao  wrote:
> This patch introduces a separate GPIO driver for Intel WhiskeyCove PMIC.
> This driver is based on gpio-crystalcove.c.

There are still minors, though they would be fixed later.
FWIW:
Reviewed-by: Andy Shevchenko 

>
> Signed-off-by: Ajay Thomas 
> Signed-off-by: Bin Gao 
> ---
> Changes in v4:
>  - Converted CTLI_INTCNT_XX macros to less verbose ones INT_DETECT_XX.
>  - Add comments about why there is no .pm for the driver.
>  - Header files re-ordered.
>  - Various coding style change to address Andy's comments.
> Changes in v3:
>  - Fixed the year in copyright line(2015-->2016).
>  - Removed DRV_NAME macro.
>  - Added kernel-doc for regmap_irq_chip of the wcove_gpio structure.
>  - Line length fix.
> Changes in v2:
>  - Typo fix (Whsikey --> Whiskey).
>  - Included linux/gpio/driver.h instead of linux/gpio.h
>  - Implemented .set_single_ended().
>  - Added GPIO register description.
>  - Replaced container_of() with gpiochip_get_data().
>  - Removed unnecessary "if (gpio > WCOVE_VGPIO_NUM" check.
>  - Removed the device id table and added MODULE_ALIAS().
>  drivers/gpio/Kconfig  |  13 ++
>  drivers/gpio/Makefile |   1 +
>  drivers/gpio/gpio-wcove.c | 433 
> ++
>  3 files changed, 447 insertions(+)
>  create mode 100644 drivers/gpio/gpio-wcove.c
>
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index cebcb40..ac74299 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -806,6 +806,19 @@ config GPIO_CRYSTAL_COVE
>   This driver can also be built as a module. If so, the module will be
>   called gpio-crystalcove.
>
> +config GPIO_WHISKEY_COVE
> +   tristate "GPIO support for Whiskey Cove PMIC"
> +   depends on INTEL_SOC_PMIC
> +   select GPIOLIB_IRQCHIP
> +   help
> + Support for GPIO pins on Whiskey Cove PMIC.
> +
> + Say Yes if you have a Intel SoC based tablet with Whiskey Cove PMIC
> + inside.
> +
> + This driver can also be built as a module. If so, the module will be
> + called gpio-wcove.
> +
>  config GPIO_CS5535
> tristate "AMD CS5535/CS5536 GPIO support"
> depends on MFD_CS5535
> diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
> index 991598e..fff6914 100644
> --- a/drivers/gpio/Makefile
> +++ b/drivers/gpio/Makefile
> @@ -34,6 +34,7 @@ obj-$(CONFIG_GPIO_BT8XX)  += gpio-bt8xx.o
>  obj-$(CONFIG_GPIO_CLPS711X)+= gpio-clps711x.o
>  obj-$(CONFIG_GPIO_CS5535)  += gpio-cs5535.o
>  obj-$(CONFIG_GPIO_CRYSTAL_COVE)+= gpio-crystalcove.o
> +obj-$(CONFIG_GPIO_WHISKEY_COVE)+= gpio-wcove.o
>  obj-$(CONFIG_GPIO_DA9052)  += gpio-da9052.o
>  obj-$(CONFIG_GPIO_DA9055)  += gpio-da9055.o
>  obj-$(CONFIG_GPIO_DAVINCI) += gpio-davinci.o
> diff --git a/drivers/gpio/gpio-wcove.c b/drivers/gpio/gpio-wcove.c
> new file mode 100644
> index 000..9f1b25a
> --- /dev/null
> +++ b/drivers/gpio/gpio-wcove.c
> @@ -0,0 +1,433 @@
> +/*
> + * gpio-wcove.c - Intel Whiskey Cove GPIO Driver
> + *
> + * This driver is written based on gpio-crystalcove.c
> + *
> + * Copyright (C) 2016 Intel Corporation. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License version
> + * 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +/*
> + * Whiskey Cove PMIC has 13 physical GPIO pins divided into 3 banks:
> + * Bank 0: Pin 0 - 6
> + * Bank 1: Pin 7 - 10
> + * Bank 2: Pin 11 -12
> + * Each pin has one output control register and one input control register.
> + */
> +#define BANK0_NR_PINS  7
> +#define BANK1_NR_PINS  4
> +#define BANK2_NR_PINS  2
> +#define WCOVE_GPIO_NUM (BANK0_NR_PINS + BANK1_NR_PINS + 
> BANK2_NR_PINS)
> +#define WCOVE_VGPIO_NUM94
> +/* GPIO output control registers(one per pin): 0x4e44 - 0x4e50 */
> +#define OUT_CTRL_OFFSET0x4e44
> +/* GPIO input control registers(one per pin): 0x4e51 - 0x4e5d */
> +#define IN_CTRL_OFFSET 0x4e51
> +
> +/*
> + * GPIO interrupts are organized in two groups:
> + * Group 0: Bank 0 pins (Pin 0 - 6)
> + * Group 1: Bank 1 and Bank 2 pins (Pin 7 - 12)
> + * Each group has two registers(one bit per pin): status and mask.
> + */
> +#define GROUP0_NR_IRQS 7
> +#define GROUP1_NR_IRQS 6
> +#define IRQ_MASK_OFFSET0x4e19
> +#define IRQ_STATUS_OFFSET  0x4e0b
> +#define UPDATE_IRQ_TYPEBIT(0)
> +#define UPDATE_IRQ_MASKBIT(1)
> +
>

[PATCH v4] gpio: add Intel WhiskeyCove GPIO driver

2016-06-27 Thread Bin Gao 
This patch introduces a separate GPIO driver for Intel WhiskeyCove PMIC.
This driver is based on gpio-crystalcove.c.

Signed-off-by: Ajay Thomas 
Signed-off-by: Bin Gao 
---
Changes in v4:
 - Converted CTLI_INTCNT_XX macros to less verbose ones INT_DETECT_XX.
 - Add comments about why there is no .pm for the driver.
 - Header files re-ordered.
 - Various coding style change to address Andy's comments.
Changes in v3:
 - Fixed the year in copyright line(2015-->2016).
 - Removed DRV_NAME macro.
 - Added kernel-doc for regmap_irq_chip of the wcove_gpio structure.
 - Line length fix.
Changes in v2:
 - Typo fix (Whsikey --> Whiskey).
 - Included linux/gpio/driver.h instead of linux/gpio.h
 - Implemented .set_single_ended().
 - Added GPIO register description.
 - Replaced container_of() with gpiochip_get_data().
 - Removed unnecessary "if (gpio > WCOVE_VGPIO_NUM" check.
 - Removed the device id table and added MODULE_ALIAS().
 drivers/gpio/Kconfig  |  13 ++
 drivers/gpio/Makefile |   1 +
 drivers/gpio/gpio-wcove.c | 433 ++
 3 files changed, 447 insertions(+)
 create mode 100644 drivers/gpio/gpio-wcove.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index cebcb40..ac74299 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -806,6 +806,19 @@ config GPIO_CRYSTAL_COVE
  This driver can also be built as a module. If so, the module will be
  called gpio-crystalcove.
 
+config GPIO_WHISKEY_COVE
+   tristate "GPIO support for Whiskey Cove PMIC"
+   depends on INTEL_SOC_PMIC
+   select GPIOLIB_IRQCHIP
+   help
+ Support for GPIO pins on Whiskey Cove PMIC.
+
+ Say Yes if you have a Intel SoC based tablet with Whiskey Cove PMIC
+ inside.
+
+ This driver can also be built as a module. If so, the module will be
+ called gpio-wcove.
+
 config GPIO_CS5535
tristate "AMD CS5535/CS5536 GPIO support"
depends on MFD_CS5535
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 991598e..fff6914 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -34,6 +34,7 @@ obj-$(CONFIG_GPIO_BT8XX)  += gpio-bt8xx.o
 obj-$(CONFIG_GPIO_CLPS711X)+= gpio-clps711x.o
 obj-$(CONFIG_GPIO_CS5535)  += gpio-cs5535.o
 obj-$(CONFIG_GPIO_CRYSTAL_COVE)+= gpio-crystalcove.o
+obj-$(CONFIG_GPIO_WHISKEY_COVE)+= gpio-wcove.o
 obj-$(CONFIG_GPIO_DA9052)  += gpio-da9052.o
 obj-$(CONFIG_GPIO_DA9055)  += gpio-da9055.o
 obj-$(CONFIG_GPIO_DAVINCI) += gpio-davinci.o
diff --git a/drivers/gpio/gpio-wcove.c b/drivers/gpio/gpio-wcove.c
new file mode 100644
index 000..9f1b25a
--- /dev/null
+++ b/drivers/gpio/gpio-wcove.c
@@ -0,0 +1,433 @@
+/*
+ * gpio-wcove.c - Intel Whiskey Cove GPIO Driver
+ *
+ * This driver is written based on gpio-crystalcove.c
+ *
+ * Copyright (C) 2016 Intel Corporation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License version
+ * 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/*
+ * Whiskey Cove PMIC has 13 physical GPIO pins divided into 3 banks:
+ * Bank 0: Pin 0 - 6
+ * Bank 1: Pin 7 - 10
+ * Bank 2: Pin 11 -12
+ * Each pin has one output control register and one input control register.
+ */
+#define BANK0_NR_PINS  7
+#define BANK1_NR_PINS  4
+#define BANK2_NR_PINS  2
+#define WCOVE_GPIO_NUM (BANK0_NR_PINS + BANK1_NR_PINS + BANK2_NR_PINS)
+#define WCOVE_VGPIO_NUM94
+/* GPIO output control registers(one per pin): 0x4e44 - 0x4e50 */
+#define OUT_CTRL_OFFSET0x4e44
+/* GPIO input control registers(one per pin): 0x4e51 - 0x4e5d */
+#define IN_CTRL_OFFSET 0x4e51
+
+/*
+ * GPIO interrupts are organized in two groups:
+ * Group 0: Bank 0 pins (Pin 0 - 6)
+ * Group 1: Bank 1 and Bank 2 pins (Pin 7 - 12)
+ * Each group has two registers(one bit per pin): status and mask.
+ */
+#define GROUP0_NR_IRQS 7
+#define GROUP1_NR_IRQS 6
+#define IRQ_MASK_OFFSET0x4e19
+#define IRQ_STATUS_OFFSET  0x4e0b
+#define UPDATE_IRQ_TYPEBIT(0)
+#define UPDATE_IRQ_MASKBIT(1)
+
+#define LSHIFT_ONE(x)  ((x) << 1)
+
+/* GPIO Input Pin Interrupt Dectection */
+#define INT_DETECT_DISABLE LSHIFT_ONE(0) /* Disable interrupt input */
+#define INT_DETECT_FALLING LSHIFT_ONE(1) /* Detect falling edge */
+#define INT_DETECT_RISING  LSHIFT_ONE(2) /* Detect rising edge */
+#define INT_DETECT_BOTH

[PATCH v4] gpio: add Intel WhiskeyCove GPIO driver

2016-06-27 Thread Bin Gao 
This patch introduces a separate GPIO driver for Intel WhiskeyCove PMIC.
This driver is based on gpio-crystalcove.c.

Signed-off-by: Ajay Thomas 
Signed-off-by: Bin Gao 
---
Changes in v4:
 - Converted CTLI_INTCNT_XX macros to less verbose ones INT_DETECT_XX.
 - Add comments about why there is no .pm for the driver.
 - Header files re-ordered.
 - Various coding style change to address Andy's comments.
Changes in v3:
 - Fixed the year in copyright line(2015-->2016).
 - Removed DRV_NAME macro.
 - Added kernel-doc for regmap_irq_chip of the wcove_gpio structure.
 - Line length fix.
Changes in v2:
 - Typo fix (Whsikey --> Whiskey).
 - Included linux/gpio/driver.h instead of linux/gpio.h
 - Implemented .set_single_ended().
 - Added GPIO register description.
 - Replaced container_of() with gpiochip_get_data().
 - Removed unnecessary "if (gpio > WCOVE_VGPIO_NUM" check.
 - Removed the device id table and added MODULE_ALIAS().
 drivers/gpio/Kconfig  |  13 ++
 drivers/gpio/Makefile |   1 +
 drivers/gpio/gpio-wcove.c | 433 ++
 3 files changed, 447 insertions(+)
 create mode 100644 drivers/gpio/gpio-wcove.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index cebcb40..ac74299 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -806,6 +806,19 @@ config GPIO_CRYSTAL_COVE
  This driver can also be built as a module. If so, the module will be
  called gpio-crystalcove.
 
+config GPIO_WHISKEY_COVE
+   tristate "GPIO support for Whiskey Cove PMIC"
+   depends on INTEL_SOC_PMIC
+   select GPIOLIB_IRQCHIP
+   help
+ Support for GPIO pins on Whiskey Cove PMIC.
+
+ Say Yes if you have a Intel SoC based tablet with Whiskey Cove PMIC
+ inside.
+
+ This driver can also be built as a module. If so, the module will be
+ called gpio-wcove.
+
 config GPIO_CS5535
tristate "AMD CS5535/CS5536 GPIO support"
depends on MFD_CS5535
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 991598e..fff6914 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -34,6 +34,7 @@ obj-$(CONFIG_GPIO_BT8XX)  += gpio-bt8xx.o
 obj-$(CONFIG_GPIO_CLPS711X)+= gpio-clps711x.o
 obj-$(CONFIG_GPIO_CS5535)  += gpio-cs5535.o
 obj-$(CONFIG_GPIO_CRYSTAL_COVE)+= gpio-crystalcove.o
+obj-$(CONFIG_GPIO_WHISKEY_COVE)+= gpio-wcove.o
 obj-$(CONFIG_GPIO_DA9052)  += gpio-da9052.o
 obj-$(CONFIG_GPIO_DA9055)  += gpio-da9055.o
 obj-$(CONFIG_GPIO_DAVINCI) += gpio-davinci.o
diff --git a/drivers/gpio/gpio-wcove.c b/drivers/gpio/gpio-wcove.c
new file mode 100644
index 000..9f1b25a
--- /dev/null
+++ b/drivers/gpio/gpio-wcove.c
@@ -0,0 +1,433 @@
+/*
+ * gpio-wcove.c - Intel Whiskey Cove GPIO Driver
+ *
+ * This driver is written based on gpio-crystalcove.c
+ *
+ * Copyright (C) 2016 Intel Corporation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License version
+ * 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/*
+ * Whiskey Cove PMIC has 13 physical GPIO pins divided into 3 banks:
+ * Bank 0: Pin 0 - 6
+ * Bank 1: Pin 7 - 10
+ * Bank 2: Pin 11 -12
+ * Each pin has one output control register and one input control register.
+ */
+#define BANK0_NR_PINS  7
+#define BANK1_NR_PINS  4
+#define BANK2_NR_PINS  2
+#define WCOVE_GPIO_NUM (BANK0_NR_PINS + BANK1_NR_PINS + BANK2_NR_PINS)
+#define WCOVE_VGPIO_NUM94
+/* GPIO output control registers(one per pin): 0x4e44 - 0x4e50 */
+#define OUT_CTRL_OFFSET0x4e44
+/* GPIO input control registers(one per pin): 0x4e51 - 0x4e5d */
+#define IN_CTRL_OFFSET 0x4e51
+
+/*
+ * GPIO interrupts are organized in two groups:
+ * Group 0: Bank 0 pins (Pin 0 - 6)
+ * Group 1: Bank 1 and Bank 2 pins (Pin 7 - 12)
+ * Each group has two registers(one bit per pin): status and mask.
+ */
+#define GROUP0_NR_IRQS 7
+#define GROUP1_NR_IRQS 6
+#define IRQ_MASK_OFFSET0x4e19
+#define IRQ_STATUS_OFFSET  0x4e0b
+#define UPDATE_IRQ_TYPEBIT(0)
+#define UPDATE_IRQ_MASKBIT(1)
+
+#define LSHIFT_ONE(x)  ((x) << 1)
+
+/* GPIO Input Pin Interrupt Dectection */
+#define INT_DETECT_DISABLE LSHIFT_ONE(0) /* Disable interrupt input */
+#define INT_DETECT_FALLING LSHIFT_ONE(1) /* Detect falling edge */
+#define INT_DETECT_RISING  LSHIFT_ONE(2) /* Detect rising edge */
+#define INT_DETECT_BOTHLSHIFT_ONE(3) /* Detect both edges */
+
+#define CTLO_DIR_IN