On Tue, Jun 29, 2021 at 12:52 AM Vincent Pelletier <plr.vincent@xxxxxxxxx> wrote: > While on this topic, there is a bullet point in > Documentation/driver-api/gpio/driver.rst which I fail to understand: > > | - Nominally set all handlers to handle_bad_irq() in the setup call and pass > | handle_bad_irq() as flow handler parameter in > gpiochip_irqchip_add() if it is > | expected for GPIO driver that irqchip .set_type() callback will be called > | before using/enabling each GPIO IRQ. Then set the handler to > | handle_level_irq() and/or handle_edge_irq() in the irqchip .set_type() > | callback depending on what your controller supports and what is requested > | by the consumer. > > - why the plural in "set all handlers to handle_bad_irq()" ? Isn't > there only a single handler in struct gpio_irq_chip ? girq->handler will be applied (inside gpiolib) successively to all the irq lines on the irqchip, as many as you have GPIO lines. It will then be set to something usable when we get to ->set_type() when an IRQ is being requested. > - I do not find a function named gpiochip_irqchip_add(), only > gpiochip_irqchip_add_domain() This refers to the old API that didn't add the irqchip with the gpiochip. Needs updating. Patches welcome ;) > - "Then set the handler to [...] in the irqchip .set_type() callback" > Isn't set_type per-pin, and isn't the interrupt handler chip-level ? Those are two different things. The ->set_type() callback is per-line (call it lines, not pins, because terminology can be confusing otherwise), these are the interrupt handlers per-line apart from of course the handler the consumer provides in e.g. request_[threaded_]irq(). There is usually another cascading interrupt handler for the GPIO irqchip itself, usually what is passed in girq->parent_handler *BUT* you are looking at an hierarchical interrupt controller with one line per gpio line to the next level of irq controller, so you do not see this. You don't have a chip-level interrupt handler in the sifive driver, all falls through upward in the hierarchy to the next overarching interrupt controller. Yours, Linus Walleij