linux-kernel - Re: [PATCH 2/3] gpio: dwapb: do not create the irq mapping upfront.

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]

Date:	Sat, 22 Mar 2014 13:37:05 +0100
From:	Sebastian Andrzej Siewior <bigeasy@...utronix.de>
To:	delicious quinoa <delicious.quinoa@...il.com>,
	Linus Walleij <linus.walleij@...aro.org>
CC:	Alan Tull <atull@...era.com>, Alexandre Courbot <gnurou@...il.com>,
	"linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>,
	linux-kernel <linux-kernel@...r.kernel.org>,
	Dinh Nguyen <dinguyen@...era.com>
Subject: Re: [PATCH 2/3] gpio: dwapb: do not create the irq mapping upfront.

On 03/20/2014 09:20 PM, delicious quinoa wrote:
>> diff --git a/drivers/gpio/gpio-dwapb.c b/drivers/gpio/gpio-dwapb.c
>> index 4d25a06b..0db0b94 100644
>> --- a/drivers/gpio/gpio-dwapb.c
>> +++ b/drivers/gpio/gpio-dwapb.c
>> @@ -263,9 +263,6 @@ static void dwapb_configure_irqs(struct dwapb_gpio *gpio,
>>         irq_set_chained_handler(irq, dwapb_irq_handler);
>>         irq_set_handler_data(irq, gpio);
>>
>> -       for (hwirq = 0 ; hwirq < ngpio ; hwirq++)
>> -               irq_create_mapping(gpio->domain, hwirq);
>> -
>>         port->bgc.gc.to_irq = dwapb_gpio_to_irq;
>>  }
> 
> 
> Hi Sebastian,
> 
> I think this functionality has changed at some point or maybe
> everybody copied the same bad code over and over.  I see a lot of
> legacy gpio drivers calling irq_create_mapping in their to_irq()
> functions.  Linus Walleij asked me to create the mappings at probe
> time.  Please see https://lkml.org/lkml/2014/2/10/154 and the
> subsequent discussion in https://lkml.org/lkml/2014/2/24/232

Linus, I don't understand why you need the mapping upfront. I looked at
those two links and you quote gpio_to_irq() which is not required.
The mapping is created once irq_of_parse_and_map() is invoked or a
platform_device is created from the DT entry. The latter creates the
warning on my 3.13 tree:

WARNING: CPU: 0 PID: 1 at drivers/of/platform.c:171
of_device_alloc+0x164/0x168()
(of_device_alloc+0x164/0x168) from [<8029fc9c>]
(of_platform_device_create_pdata+0x30/0x9c)
(of_platform_device_create_pdata+0x30/0x9c) from [<8029fde0>]
(of_platform_bus_create+0xd8/0x294)
(of_platform_bus_create+0xd8/0x294) from [<8029fe2c>]
(of_platform_bus_create+0x124/0x294)
(of_platform_bus_create+0x124/0x294) from [<8029fff8>]
(of_platform_populate+0x5c/0x9c)
(of_platform_populate+0x5c/0x9c) from [<8048d048>]
(socfpga_cyclone5_init+0x30/0x158)
(socfpga_cyclone5_init+0x30/0x158) from [<80488820>]
(customize_machine+0x20/0x40)

because at the time the device is created (struct resources is
populated) the gpio controller isn't probed yet.

With or without the upfront mapping, the struct irq is not present in
struct resource.

> 
> Alan Tull
> aka
> delicious quinoa
> 
Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/