On Sat, Jun 09 2018, Sergio Paracuellos wrote:
> This chip support high level and low level interrupts. Those
> have to be implemented also to get a complete and clean driver.
>
> Signed-off-by: Sergio Paracuellos <sergio.paracuellos@xxxxxxxxx>
> ---
> drivers/staging/mt7621-gpio/gpio-mt7621.c | 57 +++++++++++++++++++++++++------
> 1 file changed, 46 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/staging/mt7621-gpio/gpio-mt7621.c b/drivers/staging/mt7621-gpio/gpio-mt7621.c
> index 5d082c8..b0cbb30 100644
> --- a/drivers/staging/mt7621-gpio/gpio-mt7621.c
> +++ b/drivers/staging/mt7621-gpio/gpio-mt7621.c
> @@ -37,6 +37,8 @@
> * @bank: gpio bank number for the chip
> * @rising: mask for rising irqs
> * @falling: mask for falling irqs
> + * @hlevel: mask for high level irqs
> + * @llevel: mask for low level irqs
> */
> struct mtk_gc {
> struct gpio_chip chip;
> @@ -44,6 +46,8 @@ struct mtk_gc {
> int bank;
> u32 rising;
> u32 falling;
> + u32 hlevel;
> + u32 llevel;
> };
>
> /**
> @@ -114,7 +118,7 @@ mediatek_gpio_irq_unmask(struct irq_data *d)
> int bank = pin / MTK_BANK_WIDTH;
> struct mtk_gc *rg = &gpio_data->gc_map[bank];
> unsigned long flags;
> - u32 rise, fall;
> + u32 rise, fall, high, low;
>
> if (!rg)
> return;
> @@ -122,10 +126,16 @@ mediatek_gpio_irq_unmask(struct irq_data *d)
> spin_lock_irqsave(&rg->lock, flags);
> rise = mtk_gpio_r32(rg, GPIO_REG_REDGE(bank));
> fall = mtk_gpio_r32(rg, GPIO_REG_FEDGE(bank));
> + high = mtk_gpio_r32(rg, GPIO_REG_HLVL(bank));
> + low = mtk_gpio_r32(rg, GPIO_REG_LLVL(bank));
> mtk_gpio_w32(rg, GPIO_REG_REDGE(bank),
> rise | (PIN_MASK(pin) & rg->rising));
> mtk_gpio_w32(rg, GPIO_REG_FEDGE(bank),
> fall | (PIN_MASK(pin) & rg->falling));
> + mtk_gpio_w32(rg, GPIO_REG_HLVL(bank),
> + high | (PIN_MASK(pin) & rg->hlevel));
> + mtk_gpio_w32(rg, GPIO_REG_LLVL(bank),
> + low | (PIN_MASK(pin) & rg->llevel));
> spin_unlock_irqrestore(&rg->lock, flags);
> }
>
> @@ -137,7 +147,7 @@ mediatek_gpio_irq_mask(struct irq_data *d)
> int bank = pin / MTK_BANK_WIDTH;
> struct mtk_gc *rg = &gpio_data->gc_map[bank];
> unsigned long flags;
> - u32 rise, fall;
> + u32 rise, fall, high, low;
>
> if (!rg)
> return;
> @@ -145,8 +155,12 @@ mediatek_gpio_irq_mask(struct irq_data *d)
> spin_lock_irqsave(&rg->lock, flags);
> rise = mtk_gpio_r32(rg, GPIO_REG_REDGE(bank));
> fall = mtk_gpio_r32(rg, GPIO_REG_FEDGE(bank));
> + high = mtk_gpio_r32(rg, GPIO_REG_HLVL(bank));
> + low = mtk_gpio_r32(rg, GPIO_REG_LLVL(bank));
> mtk_gpio_w32(rg, GPIO_REG_FEDGE(bank), fall & ~PIN_MASK(pin));
> mtk_gpio_w32(rg, GPIO_REG_REDGE(bank), rise & ~PIN_MASK(pin));
> + mtk_gpio_w32(rg, GPIO_REG_REDGE(bank), high & ~PIN_MASK(pin));
> + mtk_gpio_w32(rg, GPIO_REG_REDGE(bank), low & ~PIN_MASK(pin));
> spin_unlock_irqrestore(&rg->lock, flags);
> }
>
> @@ -163,21 +177,42 @@ mediatek_gpio_irq_type(struct irq_data *d, unsigned int type)
> return -1;
>
> if (type == IRQ_TYPE_PROBE) {
> - if ((rg->rising | rg->falling) & mask)
> + if ((rg->rising | rg->falling |
> + rg->hlevel | rg->llevel) & mask)
> return 0;
>
> - type = IRQ_TYPE_EDGE_RISING | IRQ_TYPE_EDGE_FALLING;
> + type = (IRQ_TYPE_EDGE_RISING | IRQ_TYPE_EDGE_FALLING |
> + IRQ_TYPE_LEVEL_HIGH | IRQ_TYPE_LEVEL_LOW);
> }
>
> - if (type & IRQ_TYPE_EDGE_RISING)
> + switch (type) {
> + case IRQ_TYPE_EDGE_RISING:
> rg->rising |= mask;
> - else
> - rg->rising &= ~mask;
> -
> - if (type & IRQ_TYPE_EDGE_FALLING)
> - rg->falling |= mask;
> - else
> rg->falling &= ~mask;
> + rg->hlevel &= ~mask;
> + rg->llevel &= ~mask;
> + break;
> + case IRQ_TYPE_EDGE_FALLING:
> + rg->falling |= mask;
> + rg->rising &= ~mask;
> + rg->hlevel &= ~mask;
> + rg->llevel &= ~mask;
> + break;
> + case IRQ_TYPE_LEVEL_HIGH:
> + rg->hlevel |= mask;
> + rg->rising &= ~mask;
> + rg->falling &= mask;
> + rg->llevel &= ~mask;
> + break;
> + case IRQ_TYPE_LEVEL_LOW:
> + rg->llevel |= mask;
> + rg->rising &= ~mask;
> + rg->falling &= mask;
> + rg->hlevel &= ~mask;
> + break;
Changing this to a switch statement is definitely the wrong thing to do.
Various combinations of bits are possible.
It only makes sense to have HIGH *or* LOW, but it makes lots of sense to
have both RISING and FALLING.
It probably doesn't make sense to have both a LEVEL and an EDGE, but I'm
not certain.
Thanks,
NeilBrown
> + default:
> + return -EINVAL;
> + }
>
> return 0;
> }
> --
> 2.7.4
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
