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

[Magnus Lilja]

Hi,

On 22 December 2016 at 02:08, Vladimir Zapolskiy <vz at mleia.com> wrote:
> On 12/21/2016 10:28 PM, Fabio Estevam wrote:
>> On Wed, Dec 21, 2016 at 6:24 PM, Vladimir Zapolskiy <vz at mleia.com> wrote:
>>
>>> Correct, I would recommend to postpone adding any extensions to the driver
>>> platform data, which by the way is found in include/linux/mfd/mc13xxx.h
>>>
>>> The extension can be added only when it becomes needed.
>>
>> Yes, I agree.
>>
>
> So, for reference here is a snippet from the i.MX31 Lite board DTS:
>
> &spi2 {
>         pinctrl-names = "default";
>         pinctrl-0 = <&pinctrl_cspi2>;
>
>         /*
>          * Note that intentionally there is no GPIO CS, i.MX31 CSPI
>          * controller does not support GPIO CS and this must be specially
>          * accounted in the driver, which currently fails to register
>          * without a provided "cs-gpios" property.
>          */
>         fsl,spi-num-chipselects = <1>;
>         status = "okay";
>
>         pmic: mc13783 at 0 {
>                 compatible = "fsl,mc13783";
>                 reg = <0>;
>                 spi-cs-high;
>                 spi-max-frequency = <1000000>;
>                 interrupt-parent = <&gpio1>;
>                 interrupts = <3 IRQ_TYPE_EDGE_RISING>;
>
>                 fsl,mc13xxx-uses-rtc;
>         };
> };
>
> I'm running the current pre-rc1 v4.10. The MFD device is registered and
> MC13783 primary interrupt connected to GPIO1_3 is fired as expected
> independently if the fixup under discussion is applied or not:
>
> root at mx31lite:~# dmesg | grep spi
> [    2.017217] mc13xxx spi32766.0: mc13783: rev: 3.3, fin: 0, fab: 0, icid: 2/0
> [    2.072029] spi_imx 50010000.cspi: probed
>
> root at mx31lite:~# dmesg | grep rtc0
> [    2.682459] mc13xxx-rtc mc13783-rtc: rtc core: registered mc13783-rtc as rtc0
>
> root at mx31lite:~# cat /proc/interrupts | grep mc13xxx-rtc
> 176:          0  spi32766.0  31 Edge      mc13xxx-rtc
> 177:          0  spi32766.0  25 Edge      mc13xxx-rtc
>
> root at mx31lite:~# echo +5 > /sys/class/rtc/rtc0/wakealarm ; sleep 5
>
> root at mx31lite:~# cat /proc/interrupts | grep mc13xxx-rtc
> 176:          0  spi32766.0  31 Edge      mc13xxx-rtc
> 177:          1  spi32766.0  25 Edge      mc13xxx-rtc
>
> So the change at least does not break i.MX31 Lite board with DTS support,
> however I'm not sure if the change is still valid for any board with DTS
> if the type of the interrupt is specified as IRQ_TYPE_EDGE_FALLING.

So, have we reached a conclusion that a solution based on the code
below is good enough for now? Further device tree development would
make it possible to support other triggers on i.MX31 boards. And any
board that already has dt support can override TRIGGER_HIGH since
irqd_get_trigger_type() will return something else than IRQ_TYPE_NONE.

> 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, ...);

The above code works on both 3.18.x and 4.9.

Regards, Magnus