Re: [PATCH v2] drivers: irqchip: add irq-type-changer

[Nikita Yushchenko]

Hi

> I also don't see why you model this as the actual device that triggers
> the interrupt.

Well, that somehow matches the physical reality. In the case of wl18xx on KF, physically the interrupt 
signal indeed originates from the intermediate chip - the inverting level-shifter.

I remember your suggestion to configure interrupt source's node with interrupt-parent set to inverter 
and interrupt details for inverter's parent. But this looks hacky and inconsistent for me.

In contrary, an abstraction of intermediate entity that does a static conversion of the trigger type and 
does not need any software control, looks sane. And, hardware designers do strange things sometimes, I 
won't be surprised observing a change from level to edge one day. Thus it looked a good idea not to 
limit the conversion to inversion, but support arbitrary one. Then, irqspec inside the node for the 
intermediate entity obtains a meaning, making the entire model consistent.

I don't strongly insist on this approach, it just looks cleaner for me.


> I'm also pretty sure that with the current code,
> you end-up with *two* interrupts (one for the inverter and one for the
> end-point).

In driver's init, I only call of_irq_parse_one() for interrupt defined in the changer's node. This does 
not create a mapping for it. The mapping is only created when changer's "interrupt-child" creates a 
mapping for their interrupt - then changer's alloc() routine calls irq_domain_alloc_irqs_parent() in the 
same way as all other hierarchical irqchips do.

I don't see where double mapping can appear here. Please explain.


> You need a proper DT binding...
> Geert commented on why this is wrong...
> Use struct_size()...

Will fix all that after we negotiate the basic approach.

Thanks,

Nikita