[PATCH v6 2/9] irqchip: add Amlogic Meson GPIO irqchip driver

Kevin Hilman khilman at baylibre.com
Fri Jun 9 14:15:45 PDT 2017


Jerome Brunet <jbrunet at baylibre.com> writes:

> On Thu, 2017-06-08 at 21:38 +0200, Heiner Kallweit wrote:
>> Add a driver supporting the GPIO interrupt controller on certain
>> Amlogic meson SoC's.
>> 
>> Signed-off-by: Heiner Kallweit <hkallweit1 at gmail.com>

[...]

>> +static unsigned int meson_irq_startup(struct irq_data *data)
>> +{
>> +	irq_chip_unmask_parent(data);
>> +	/*
>> +	 * An extra bit was added to allow having the same gpio hwirq twice
>> +	 * for handling IRQ_TYPE_EDGE_BOTH. Remove this bit to get the
>> +	 * gpio hwirq.
>> +	 */
>> +	meson_irq_set_hwirq(data, data->hwirq >> 1);
>
> Please keep in mind that any device can use this controller as irq parent.
> It has to make sense, even when not serving the gpio driver.
>
> This hack means that, in DT, we'd have to multiply by 2 the values given under
> section "22.3 GPIO interrupts" of the datasheet. This is an example Linux
> specific stuff in DT.
> It also means that the controller declares a lot more lines that it really has
> ... 
>
> This is all to accommodate your hack around IRQ_TYPE_BOTH and creating the
> mapping from the irq_set_type callback of the GPIO driver, which is still think
> should be dropped at this point.

+1

Please drop the hack for IRQ_TYPE_BOTH so we can reach agreement on the
basic design and functionality.  The gymnastics required to support this
hack (due to broken hardware) are getting in the way of getting basic
functionality merged.

Also, we already know from previous discussions between Jerome and the
IRQ maintainers that hacks for IRQ_TYPE_BOTH like this have been very
thoroughly rejected.  So while the IRQ maintainers haven't chimed in on
this series, we have a very good idea what they will say when they do.

So please, pretty please, let's avoid giving the IRQ maintainers a fun
reason to NAK, and drop the IRQ_TYPE_BOTH stuff until we have an agreed
upon way to support the hardware that actually works.

Thanks,

Kevin



More information about the linux-amlogic mailing list