[PATCH 2/2] mfd: mc13xxx: Pass the IRQF_TRIGGER_HIGH flag.

[Magnus Lilja]

Hi,

On 21 December 2016 at 07:00, Alexander Shiyan <shc_work at mail.ru> wrote:
>>Вторник, 20 декабря 2016, 22:35 +03:00 от Magnus Lilja <lilja.magnus at gmail.com>:
>>
>>On 20 December 2016 at 06:20, Alexander Shiyan < shc_work at mail.ru > wrote:
>>>>Вторник, 20 декабря 2016, 0:28 +03:00 от Magnus Lilja < lilja.magnus at gmail.com >:
>>>>
>>>>All supported mc13xxx devices have active high interrupt outputs. Make sure
>>>>to configure the interrupt as active high by passing the IRQF_TRIGGER_HIGH
>>>>flag. This is required at least on the i.MX31 PDK board.
>>>>
>>>>Signed-off-by: Magnus Lilja <  lilja.magnus at gmail.com >
>>>>---
>>>>
>>>> drivers/mfd/mc13xxx-core.c | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>>diff --git a/drivers/mfd/mc13xxx-core.c b/drivers/mfd/mc13xxx-core.c
>>>>index d7f54e4..4cbe6b7 100644
>>>>--- a/drivers/mfd/mc13xxx-core.c
>>>>+++ b/drivers/mfd/mc13xxx-core.c
>>>>@@ -440,7 +440,7 @@ int mc13xxx_common_init(struct device *dev)
>>>> mc13xxx->irq_chip.irqs = mc13xxx->irqs;
>>>> mc13xxx->irq_chip.num_irqs = ARRAY_SIZE(mc13xxx->irqs);
>>>>
>>>>-ret = regmap_add_irq_chip(mc13xxx->regmap, mc13xxx->irq, IRQF_ONESHOT,
>>>>+ret = regmap_add_irq_chip(mc13xxx->regmap, mc13xxx->irq, IRQF_ONESHOT | IRQF_TRIGGER_HIGH,
>>>>   0, &mc13xxx->irq_chip, &mc13xxx->irq_data);
>>>> if (ret)
>>>> return ret;
>>>
>>> IRQ line can be passed through inverter to IC.
>>> On my opinion the best way to handle all possible situations is parse
>>> devicetree IRQ flags and pass to the driver.
>>
>>My guess is that most boards follow the evaluation boards/kits and
>>don't have an inverter so I think the default should be TRIG_HIGH
>>since that will make most boards work automatically. But I can have a
>>look at making it configurable (with and without the use of device
>>tree), but for the device tree stuff I would need pointers to similar
>>code since my experience with that is none.
>>Any pointers to similar code?
>
> Hello.
>
> Perhaps I'm wrong and the desired active level has setup at the IRQ code level.

Don't think I understand what you mean here.

> Otherwise, I think you need to use something like the following:
>
> unsigned flags = irqd_get_trigger_type(irq_get_irq_data(irq));
> flags = (flags == IRQ_TYPE_NONE) ? IRQF_TRIGGER_HIGH : flags;
> ret = regmap_add_irq_chip(..., IRQF_ONESHOT | flags, ...);

That would work but I don't know how one would set the trigger type in
case of not using device trees. But that would only be a problem if a
board really has an inverter on the IRQ line so that can be solved
later, and might be not necessary to solve if mx31 boards are
converted to device tree usage.

I guess that get trigger_type can be set via the device tree magic
when using device trees.


Regards, Magnus