2013/11/29 Hauke Mehrtens <hauke@xxxxxxxxxx>:
> On 11/29/2013 05:09 PM, Rafał Miłecki wrote:
>> Input GPIO changes can generate interrupts, but we need kind of ACK for
>> them by changing IRQ polarity. This is required to stop hardware from
>> keep generating interrupts and generate another one on the next GPIO
>> state change.
>> This code allows using GPIOs with standard interrupts and add for
>> example GPIO buttons support.
>
> As far as I know IRQs for GPIOs are only support on SoCs and not on PCIe
> wifi cards like the BCM43224. I think the dependency to IRQ_DOMAIN
> should only be added when BCMA_HOST_SOC is set.
Sounds sane.
>> + for_each_set_bit(gpio, (unsigned long *)&irqs, cc->gpio.ngpio) {
>> + generic_handle_irq(bcma_gpio_to_irq(&cc->gpio, gpio));
>> + bcma_chipco_gpio_polarity(cc, BIT(gpio),
>> + (val & BIT(gpio)) ? BIT(gpio) : 0);
>
> What about setting this once for all irqs at the end of this function?
> bcma_chipco_gpio_polarity() takes an lock.
OK
>> int bcma_gpio_init(struct bcma_drv_cc *cc)
>> {
>> struct gpio_chip *chip = &cc->gpio;
>> + unsigned int hwirq, gpio;
>> + int err;
>>
>> + /*
>> + * GPIO chip
>> + */
>> chip->label = "bcma_gpio";
>> chip->owner = THIS_MODULE;
>> chip->request = bcma_gpio_request;
>> @@ -104,8 +152,48 @@ int bcma_gpio_init(struct bcma_drv_cc *cc)
>> chip->base = 0;
>> else
>> chip->base = -1;
>> + err = gpiochip_add(chip);
>> + if (err)
>> + goto err_gpiochip_add;
>
> Shouldn't the gpio chip be registered after we set up the irq handling
> so no one uses it before?
I don't know.
gpio-tegra.c calls functions in this order:
1) gpiochip_add
2) irq_create_mapping3
3) irq_set_chip_and_handler
4) irq_set_chained_handler
gpio-msm-v2:
1) gpiochip_add
2) irq_domain_add_linear
3) irq_set_chained_handler
gpio-mpc8xxx.c: like above
gpio-adnp.c on the other hand:
1) irq_domain_add_linear
2) request_threaded_irq
3) gpiochip_add
gpio-grgpio.c:
1) irq_domain_add_linear
2) gpiochip_add
So that differs pretty much between the drivers.
But I think calling "gpiochip_add" at the end makes some sense. I
guess I was looking at gpio-tegra.c too much.
>> + for (gpio = 0; gpio < chip->ngpio; gpio++) {
>> + int irq = irq_create_mapping(cc->irq_domain, gpio);
>> +
>> + irq_set_chip_data(irq, cc);
>> + irq_set_chip_and_handler(irq, &bcma_gpio_irq_chip,
>> + handle_simple_irq);
>> + }
>
> Is there some method needed to free or unregister these functions? This
> could be build as an module when it is not used on a SoC.
Yes: irq_dispose_mapping (it calls irq_set_chip_and_handler with NULL
and NULL for us). I'll use it.
--
Rafał
