| Message ID | 20230518003311.415018-1-marex@denx.de |
|---|---|
| State | Rejected |
| Headers | show |
| Series | rtc: stm32: Handle single EXTI IRQ as wake up source | expand |
Hi Marek, 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@denx.de> > --- > Cc: Alessandro Zummo <a.zummo@towertech.it> > Cc: Alexandre Belloni <alexandre.belloni@bootlin.com> > Cc: Alexandre Torgue <alexandre.torgue@foss.st.com> > Cc: Amelie DELAUNAY <amelie.delaunay@foss.st.com> > Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com> > Cc: linux-arm-kernel@lists.infradead.org > Cc: linux-rtc@vger.kernel.org > Cc: linux-stm32@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 Regards, Amelie
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@denx.de> >> --- >> Cc: Alessandro Zummo <a.zummo@towertech.it> >> Cc: Alexandre Belloni <alexandre.belloni@bootlin.com> >> Cc: Alexandre Torgue <alexandre.torgue@foss.st.com> >> Cc: Amelie DELAUNAY <amelie.delaunay@foss.st.com> >> Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com> >> Cc: linux-arm-kernel@lists.infradead.org >> Cc: linux-rtc@vger.kernel.org >> Cc: linux-stm32@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 ?
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@denx.de> >>> --- >>> Cc: Alessandro Zummo <a.zummo@towertech.it> >>> Cc: Alexandre Belloni <alexandre.belloni@bootlin.com> >>> Cc: Alexandre Torgue <alexandre.torgue@foss.st.com> >>> Cc: Amelie DELAUNAY <amelie.delaunay@foss.st.com> >>> Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com> >>> Cc: linux-arm-kernel@lists.infradead.org >>> Cc: linux-rtc@vger.kernel.org >>> Cc: linux-stm32@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?
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@denx.de> >>>> --- >>>> Cc: Alessandro Zummo <a.zummo@towertech.it> >>>> Cc: Alexandre Belloni <alexandre.belloni@bootlin.com> >>>> Cc: Alexandre Torgue <alexandre.torgue@foss.st.com> >>>> Cc: Amelie DELAUNAY <amelie.delaunay@foss.st.com> >>>> Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com> >>>> Cc: linux-arm-kernel@lists.infradead.org >>>> Cc: linux-rtc@vger.kernel.org >>>> Cc: linux-stm32@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.
Hi Marek On 5/30/23 21:54, Marek Vasut wrote: > 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@denx.de> >>>>> --- >>>>> Cc: Alessandro Zummo <a.zummo@towertech.it> >>>>> Cc: Alexandre Belloni <alexandre.belloni@bootlin.com> >>>>> Cc: Alexandre Torgue <alexandre.torgue@foss.st.com> >>>>> Cc: Amelie DELAUNAY <amelie.delaunay@foss.st.com> >>>>> Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com> >>>>> Cc: linux-arm-kernel@lists.infradead.org >>>>> Cc: linux-rtc@vger.kernel.org >>>>> Cc: linux-stm32@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. Currently if people use 2 interrupts in their DT with an up to date kernel I don't think it works fine. At the beginning of the MP1 story, 2 interrupts were needed to wakeup system from LPSTOP: one for GIC and the other one for EXTI. But since maybe 2 years, EXTI and GIC interrupts are directly linked inside the EXTI driver. So now, devices only need to take one interrupt. With this implementation if one device uses 2 interrupts in their DT then the GIC interrupt will be mapped two times. So I think that current implementation in RTC driver is broken and it should be aligned with "new" EXTI implementation. Note also that the use of 2 interrupts has never been documented in dt-bindings documentation. Above words are for STM32 MPU products For STM32 MCU products RTC is only mapped to the EXTI (not to the NVIC) so no needs to handle two interrupts. Alex > 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.
On 5/31/23 11:36, Alexandre TORGUE wrote: > Hi Marek Hi, > On 5/30/23 21:54, Marek Vasut wrote: >> 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@denx.de> >>>>>> --- >>>>>> Cc: Alessandro Zummo <a.zummo@towertech.it> >>>>>> Cc: Alexandre Belloni <alexandre.belloni@bootlin.com> >>>>>> Cc: Alexandre Torgue <alexandre.torgue@foss.st.com> >>>>>> Cc: Amelie DELAUNAY <amelie.delaunay@foss.st.com> >>>>>> Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com> >>>>>> Cc: linux-arm-kernel@lists.infradead.org >>>>>> Cc: linux-rtc@vger.kernel.org >>>>>> Cc: linux-stm32@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. > > Currently if people use 2 interrupts in their DT with an up to date > kernel I don't think it works fine. At the beginning of the MP1 story, 2 > interrupts were needed to wakeup system from LPSTOP: one for GIC and the > other one for EXTI. But since maybe 2 years, EXTI and GIC interrupts are > directly linked inside the EXTI driver. So now, devices only need to > take one interrupt. With this implementation if one device uses 2 > interrupts in their DT then the GIC interrupt will be mapped two times. > So I think that current implementation in RTC driver is broken and it > should be aligned with "new" EXTI implementation. Note also that the use > of 2 interrupts has never been documented in dt-bindings documentation. In that case, maybe add a warning into the driver if you detect two interrupts, notify user and avoid any unexpected surprises ? > Above words are for STM32 MPU products For STM32 MCU products RTC is > only mapped to the EXTI (not to the NVIC) so no needs to handle two > interrupts. Like I wrote before, I am fine either way. [...]
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)
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@denx.de> --- Cc: Alessandro Zummo <a.zummo@towertech.it> Cc: Alexandre Belloni <alexandre.belloni@bootlin.com> Cc: Alexandre Torgue <alexandre.torgue@foss.st.com> Cc: Amelie DELAUNAY <amelie.delaunay@foss.st.com> Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com> Cc: linux-arm-kernel@lists.infradead.org Cc: linux-rtc@vger.kernel.org Cc: linux-stm32@st-md-mailman.stormreply.com --- drivers/rtc/rtc-stm32.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)