Hi Quentin, sorry for delays!

On Wed, Jul 25, 2018 at 2:27 PM Quentin Schulz
[off-list ref] wrote:

This GPIO controller can serve as an interrupt controller as well on the
GPIOs it handles.

An interrupt is generated whenever a GPIO line changes and the
interrupt for this GPIO line is enabled. This means that both the
changes from low to high and high to low generate an interrupt.

For some use cases, it makes sense to ignore the high to low change and
not generate an interrupt. Such a use case is a line that is hold in a
level high/low manner until the event holding the line gets acked.
This can be achieved by making sure the interrupt on the GPIO controller
side gets acked and masked only after the line gets hold in its default
state, this is what's done with the fasteoi functions.

Only IRQ_TYPE_EDGE_BOTH and IRQ_TYPE_LEVEL_HIGH are supported for now.

Signed-off-by: Quentin Schulz <redacted>

Patch applied, it's such a pretty and straight-forward patch.
Also IRQ is probably very nice to have, so let's get this in and
supported.

Please consider addressing the following in follow-up patch(es):

+static int ocelot_irq_set_type(struct irq_data *data, unsigned int type);

Can't you just move the function above so you don't have to forward-declare
this?

+static struct irq_chip ocelot_eoi_irqchip = {
+       .name           = "gpio",
+       .irq_mask       = ocelot_irq_mask,
+       .irq_eoi        = ocelot_irq_ack,
+       .irq_unmask     = ocelot_irq_unmask,
+       .flags          = IRQCHIP_EOI_THREADED | IRQCHIP_EOI_IF_HANDLED,

As you see the latter part of the define is "IF_HANDLED".

+       .irq_set_type   = ocelot_irq_set_type,
+};
+
+static struct irq_chip ocelot_irqchip = {
+       .name           = "gpio",
+       .irq_mask       = ocelot_irq_mask,
+       .irq_ack        = ocelot_irq_ack,
+       .irq_unmask     = ocelot_irq_unmask,
+       .irq_set_type   = ocelot_irq_set_type,
+};

Is it really neccessary to have two irqchips?

Is this to separate ACK and EOI because the EOI version
doesn't survive an ACK?

Yours,
Linus Walleij