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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <dd6434a7-aff1-94ec-2fdf-51374c695ada@st.com>
Date:   Tue, 11 Feb 2020 11:08:29 +0100
From:   Alexandre Torgue <alexandre.torgue@...com>
To:     Marek Vasut <marex@...x.de>, Thomas Gleixner <tglx@...utronix.de>,
        Jason Cooper <jason@...edaemon.net>,
        Marc Zyngier <marc.zyngier@....com>,
        Linus Walleij <linus.walleij@...aro.org>
CC:     <linux-arm-kernel@...ts.infradead.org>,
        <linux-kernel@...r.kernel.org>, <linux-gpio@...r.kernel.org>
Subject: Re: [PATCH 2/2] pinctrl: stm32: Add level interrupt support to gpio
 irq chip

Hi Marek

On 2/10/20 7:39 PM, Marek Vasut wrote:
> 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@...com>
>>
>> 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.

I agree

> 
>>   	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?

Yes. It'll be fixed in v2.

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

Thanks