On 21/02/2018 15:11, Alexander Stein wrote:
On Tuesday, February 20, 2018, 6:15:46 PM CET Jan Kundrát wrote:
I was told that the appropriate way forward are device drivers which do not
specify the IRQ polarity. Apparently, people are supposed to do that in
their DT.
So, in this context:
- pinctrl-mcp23s08.c should only specify IRQF_ONESHOT | IRQF_SHARED
- the DT should use an appropriate IRQ_TYPE_LEVEL_LOW or
IRQ_TYPE_LEVEL_HIGH based on what the CPU expects to see on its IRQ pins
- the DT must also set a property to configure the MCP23xxx device to
*generate* an IRQ by the active-high flag, or another flag for an
open-drain active-low IRQ output suitable for connecting directly to an
interrupt line which gets shared by several open-drain IRQ producers
- whoever supplies the DT must now check that their settings "make sense"
Yes, this means that people might have to update their DTs. To me, that
makes sense. If you ask me, the DT is already sort-of broken because it's
using IRQF_SHARED with a push-pull IRQ output. Yes, one can fix that with
extra transistors, but that sounds quite ugly, doesn't it.
Well, that is the exact situation on the board I had to deal with. The MCP was
attached to a transisitor inverting the signal. The output on MCP has to be
push-pull as there was no pull-up oder -down. But the line connecting the
inverter to the CPU was an open-drain one and this line was actually a shared
IRQ line. So of course the line bewteen MCP and inverter cannot be shared, but
the IRQ used on CPU is actually shared. How can this be respresented in DT?
G'day Alexandar,
It can't at the moment. Linux W. proposed modelling a inverter. Basically a irqchip driver
that flips the polarity of the interrupt. I was going to give it a go when I found time.
But it's not a high priority for me.
In your patch at: https://www.spinics.net/lists/devicetree/msg58208.html
This bit in mcp23s08_irq_setup
+ if (mcp->irq_active_high)
+ irqflags |= IRQF_TRIGGER_HIGH;
+ else
+ irqflags |= IRQF_TRIGGER_LOW;
is causing me problems.
It overrides the setting from the dt for me.
Probably due to this bit in __setup_irq:
/*
* If the trigger type is not specified by the caller,
* then use the default for this interrupt.
*/
if (!(new->flags & IRQF_TRIGGER_MASK))
new->flags |= irqd_get_trigger_type(&desc->irq_data);
Based on your description I would have thought the same.
With a transisitor inverting the line you'd want to register irq active_low,
while driving active_high to turn the transitor on.
Was this the case?
--
Regards
Phil Reid
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
