[PATCH] rtc: stm32: Handle single EXTI IRQ as wake up source

[Marek Vasut]

On 5/30/23 17:18, Amelie Delaunay wrote:
> On 5/30/23 16:13, Marek Vasut wrote:
>> On 5/30/23 16:02, Amelie Delaunay wrote:
>>> Hi Marek,
>>
>> Hello Amelie,
>>
>>> On 5/18/23 02:33, Marek Vasut wrote:
>>>> The arch/arm/boot/dts/stm32mp151.dtsi specifies one interrupt for the
>>>> RTC node, while the expectation of the RTC driver is two interrupts on
>>>> STM32MP15xx SoC, one connected to GIC interrupt controller and another
>>>> one to EXTI. Per STM32MP15xx reference manual, the two interrupts serve
>>>> the same purpose, except the EXTI one can also wake the system up. The
>>>> EXTI driver already internally handles this GIC and EXTI duality and
>>>> maps the EXTI interrupt onto GIC during runtime, and only uses the EXTI
>>>> for wake up functionality.
>>>>
>>>> Therefore, fix the driver such that if on STM32MP15xx there is only one
>>>> interrupt specified in the DT, use that interrupt as EXTI interrupt and
>>>> set it as wake up source.
>>>>
>>>> This fixes the following warning in the kernel log on STM32MP15xx:
>>>> "
>>>> stm32_rtc 5c004000.rtc: error -ENXIO: IRQ index 1 not found
>>>> stm32_rtc 5c004000.rtc: alarm can't wake up the system: -6
>>>> "
>>>>
>>>> This also fixes the system wake up via built-in RTC using e.g.:
>>>> $ rtcwake -s 5 -m mem
>>>>
>>>> Fixes: b72252b6580c ("rtc: stm32: add stm32mp1 rtc support")
>>>> Signed-off-by: Marek Vasut <marex at denx.de>
>>>> ---
>>>> Cc: Alessandro Zummo <a.zummo at towertech.it>
>>>> Cc: Alexandre Belloni <alexandre.belloni at bootlin.com>
>>>> Cc: Alexandre Torgue <alexandre.torgue at foss.st.com>
>>>> Cc: Amelie DELAUNAY <amelie.delaunay at foss.st.com>
>>>> Cc: Maxime Coquelin <mcoquelin.stm32 at gmail.com>
>>>> Cc: linux-arm-kernel at lists.infradead.org
>>>> Cc: linux-rtc at vger.kernel.org
>>>> Cc: linux-stm32 at st-md-mailman.stormreply.com
>>>> ---
>>>>   drivers/rtc/rtc-stm32.c | 9 +++++----
>>>>   1 file changed, 5 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/rtc/rtc-stm32.c b/drivers/rtc/rtc-stm32.c
>>>> index 229cb2847cc48..72994b9f95319 100644
>>>> --- a/drivers/rtc/rtc-stm32.c
>>>> +++ b/drivers/rtc/rtc-stm32.c
>>>> @@ -780,14 +780,15 @@ static int stm32_rtc_probe(struct 
>>>> platform_device *pdev)
>>>>       ret = device_init_wakeup(&pdev->dev, true);
>>>>       if (rtc->data->has_wakeirq) {
>>>> -        rtc->wakeirq_alarm = platform_get_irq(pdev, 1);
>>>> +        rtc->wakeirq_alarm = platform_get_irq_optional(pdev, 1);
>>>>           if (rtc->wakeirq_alarm > 0) {
>>>>               ret = dev_pm_set_dedicated_wake_irq(&pdev->dev,
>>>>                                   rtc->wakeirq_alarm);
>>>> -        } else {
>>>> +        } else if (rtc->wakeirq_alarm == -EPROBE_DEFER) {
>>>>               ret = rtc->wakeirq_alarm;
>>>> -            if (rtc->wakeirq_alarm == -EPROBE_DEFER)
>>>> -                goto err;
>>>> +            goto err;
>>>> +        } else {
>>>> +            ret = dev_pm_set_wake_irq(&pdev->dev, rtc->irq_alarm);
>>>>           }
>>>>       }
>>>>       if (ret)
>>>
>>> In our downstream [1], dedicated wakeup irq management is dropped: it 
>>> is neither described in st,stm32-rtc bindings nor used in STM32Fx, 
>>> STM32Hx, STM32MP1x device trees.
>>> The pointed patch is going to be upstreamed.
>>>
>>> [1] 
>>> https://github.com/STMicroelectronics/linux/commit/5a45e1f100d59c33b6b50fe98c0f62862bd6f3d2
>>
>> Won't that break compatibility with DTs which do use two interrupts 
>> entries ?
> 
> I didn't find any DTs using STM32 RTC with two interrupts. Did I miss 
> something?

I am not aware of any either (I also did check a couple of repositories 
to be sure) . However, the DT is an ABI , so there might be users we do 
not know about, who might be unable to update their DTs , and who would 
be broken by dropping the support for two interrupts.

On the other hand, I only use one interrupt anyway, so I am not affected 
and I don't have a strong preference regarding which patch gets in.