Re: [PATCH 2/2] pinctrl: stm32: Add level interrupt support to gpio irq chip

From: Marek Vasut
Date: Mon Feb 10 2020 - 13:39:11 EST


On 2/10/20 2:49 PM, Alexandre Torgue wrote:
> This patch adds level interrupt support to gpio irq chip.
>
> GPIO hardware block is directly linked to EXTI block but EXTI handles
> external interrupts only on edge. To be able to handle GPIO interrupt on
> level a "hack" is done in gpio irq chip: parent interrupt (exti irq chip)
> is retriggered following interrupt type and gpio line value.
>
> Signed-off-by: Alexandre Torgue <alexandre.torgue@xxxxxx>
>
> diff --git a/drivers/pinctrl/stm32/pinctrl-stm32.c b/drivers/pinctrl/stm32/pinctrl-stm32.c
> index 2d5e0435af0a..04e1b062c20e 100644
> --- a/drivers/pinctrl/stm32/pinctrl-stm32.c
> +++ b/drivers/pinctrl/stm32/pinctrl-stm32.c
> @@ -89,6 +89,7 @@ struct stm32_gpio_bank {
> struct pinctrl_gpio_range range;
> struct fwnode_handle *fwnode;
> struct irq_domain *domain;
> + u32 irq_type[STM32_GPIO_PINS_PER_BANK];

You might want reverse xmas tree order here.

> u32 bank_nr;
> u32 bank_ioport_nr;
> u32 pin_backup[STM32_GPIO_PINS_PER_BANK];
> @@ -303,6 +304,48 @@ static const struct gpio_chip stm32_gpio_template = {
> .get_direction = stm32_gpio_get_direction,
> };
>
> +void stm32_gpio_irq_eoi(struct irq_data *d)
> +{
> + struct stm32_gpio_bank *bank = d->domain->host_data;
> + int line;
> +
> + irq_chip_eoi_parent(d);
> +
> + /* If level interrupt type then retrig */
> + line = stm32_gpio_get(&bank->gpio_chip, d->hwirq);
> + if ((line == 0 && bank->irq_type[d->hwirq] == IRQ_TYPE_LEVEL_LOW) ||
> + (line == 1 && bank->irq_type[d->hwirq] == IRQ_TYPE_LEVEL_HIGH))
> + irq_chip_retrigger_hierarchy(d);
> +};
> +
> +static int stm32_gpio_set_type(struct irq_data *d, unsigned int type)
> +{
> + struct stm32_gpio_bank *bank = d->domain->host_data;
> + u32 parent_type;
> +
> + bank->irq_type[d->hwirq] = type;
> +
> + switch (type) {
> + case IRQ_TYPE_EDGE_RISING:
> + case IRQ_TYPE_EDGE_FALLING:
> + case IRQ_TYPE_EDGE_BOTH:
> + parent_type = type;
> + break;
> + case IRQ_TYPE_LEVEL_HIGH:
> + parent_type = IRQ_TYPE_EDGE_RISING;
> + break;
> + case IRQ_TYPE_LEVEL_LOW:
> + parent_type = IRQ_TYPE_EDGE_FALLING;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + irq_chip_set_type_parent(d, parent_type);

irq_chip_set_type_parent() returns error code, shouldn't that be handled?

Otherwise, tested on STM32MP1 with KSZ8851-16MLL NIC.