| Message ID | 20250516-rtc-uie-irq-fixes-v2-4-3de8e530a39e@geanix.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | rtc: Fix problems with missing UIE irqs | expand |
Hi Esben, On 16/05/2025 08:23, Esben Haabendal wrote: > Interrupts are automatically enabled when requested, so we need to > initialize irq_en accordingly to avoid causing an unbalanced enable > warning. > > Signed-off-by: Esben Haabendal <esben@geanix.com> > --- > drivers/rtc/rtc-tps6586x.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/rtc/rtc-tps6586x.c b/drivers/rtc/rtc-tps6586x.c > index 54c8429b16bfcc692b1f4d5404f0c42f720e93b4..76ecf7b798f0de22aa89a552a263b473ab3065ef 100644 > --- a/drivers/rtc/rtc-tps6586x.c > +++ b/drivers/rtc/rtc-tps6586x.c > @@ -258,6 +258,7 @@ static int tps6586x_rtc_probe(struct platform_device *pdev) > > irq_set_status_flags(rtc->irq, IRQ_NOAUTOEN); > > + rtc->irq_en = true; > ret = devm_request_threaded_irq(&pdev->dev, rtc->irq, NULL, > tps6586x_rtc_irq, > IRQF_ONESHOT, > I have bisected a suspend regression on one of our Tegra20 boards (that uses this driver) and bisect is pointing to this commit. Reverting this commit fixes the problem. Looking at the above I see that the flag IRQ_NOAUTOEN is being set and so now with your change we never enable the interrupt. Hence, the wake-ups are now broken and suspend testing fails. So it would seem best to revert this. BTW, I looked at the change to the CPCAP driver and that driver actively disables the IRQ after requesting it and so I am wondering if that will also have alarm issues? I don't have a board with CPCAP to test. Thanks Jon
On 29/10/2025 09:20:17+0000, Jon Hunter wrote: > Hi Esben, > > On 16/05/2025 08:23, Esben Haabendal wrote: > > Interrupts are automatically enabled when requested, so we need to > > initialize irq_en accordingly to avoid causing an unbalanced enable > > warning. > > > > Signed-off-by: Esben Haabendal <esben@geanix.com> > > --- > > drivers/rtc/rtc-tps6586x.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/rtc/rtc-tps6586x.c b/drivers/rtc/rtc-tps6586x.c > > index 54c8429b16bfcc692b1f4d5404f0c42f720e93b4..76ecf7b798f0de22aa89a552a263b473ab3065ef 100644 > > --- a/drivers/rtc/rtc-tps6586x.c > > +++ b/drivers/rtc/rtc-tps6586x.c > > @@ -258,6 +258,7 @@ static int tps6586x_rtc_probe(struct platform_device *pdev) > > irq_set_status_flags(rtc->irq, IRQ_NOAUTOEN); > > + rtc->irq_en = true; > > ret = devm_request_threaded_irq(&pdev->dev, rtc->irq, NULL, > > tps6586x_rtc_irq, > > IRQF_ONESHOT, > > > > I have bisected a suspend regression on one of our Tegra20 boards (that uses > this driver) and bisect is pointing to this commit. Reverting this commit > fixes the problem. > > Looking at the above I see that the flag IRQ_NOAUTOEN is being set and so > now with your change we never enable the interrupt. Hence, the wake-ups are > now broken and suspend testing fails. So it would seem best to revert this. > > BTW, I looked at the change to the CPCAP driver and that driver actively > disables the IRQ after requesting it and so I am wondering if that will also > have alarm issues? I don't have a board with CPCAP to test. > That's right, I guess you can send reverts.
On 29/10/2025 10:39, Alexandre Belloni wrote: > On 29/10/2025 09:20:17+0000, Jon Hunter wrote: >> Hi Esben, >> >> On 16/05/2025 08:23, Esben Haabendal wrote: >>> Interrupts are automatically enabled when requested, so we need to >>> initialize irq_en accordingly to avoid causing an unbalanced enable >>> warning. >>> >>> Signed-off-by: Esben Haabendal <esben@geanix.com> >>> --- >>> drivers/rtc/rtc-tps6586x.c | 1 + >>> 1 file changed, 1 insertion(+) >>> >>> diff --git a/drivers/rtc/rtc-tps6586x.c b/drivers/rtc/rtc-tps6586x.c >>> index 54c8429b16bfcc692b1f4d5404f0c42f720e93b4..76ecf7b798f0de22aa89a552a263b473ab3065ef 100644 >>> --- a/drivers/rtc/rtc-tps6586x.c >>> +++ b/drivers/rtc/rtc-tps6586x.c >>> @@ -258,6 +258,7 @@ static int tps6586x_rtc_probe(struct platform_device *pdev) >>> irq_set_status_flags(rtc->irq, IRQ_NOAUTOEN); >>> + rtc->irq_en = true; >>> ret = devm_request_threaded_irq(&pdev->dev, rtc->irq, NULL, >>> tps6586x_rtc_irq, >>> IRQF_ONESHOT, >>> >> >> I have bisected a suspend regression on one of our Tegra20 boards (that uses >> this driver) and bisect is pointing to this commit. Reverting this commit >> fixes the problem. >> >> Looking at the above I see that the flag IRQ_NOAUTOEN is being set and so >> now with your change we never enable the interrupt. Hence, the wake-ups are >> now broken and suspend testing fails. So it would seem best to revert this. >> >> BTW, I looked at the change to the CPCAP driver and that driver actively >> disables the IRQ after requesting it and so I am wondering if that will also >> have alarm issues? I don't have a board with CPCAP to test. >> > > That's right, I guess you can send reverts. You guessed right! I certainly can. I am happy to send a revert for this because I can clearly test and verify this. I am not sure if you and Esben want to review the rest of the series. Jon
diff --git a/drivers/rtc/rtc-tps6586x.c b/drivers/rtc/rtc-tps6586x.c index 54c8429b16bfcc692b1f4d5404f0c42f720e93b4..76ecf7b798f0de22aa89a552a263b473ab3065ef 100644 --- a/drivers/rtc/rtc-tps6586x.c +++ b/drivers/rtc/rtc-tps6586x.c @@ -258,6 +258,7 @@ static int tps6586x_rtc_probe(struct platform_device *pdev) irq_set_status_flags(rtc->irq, IRQ_NOAUTOEN); + rtc->irq_en = true; ret = devm_request_threaded_irq(&pdev->dev, rtc->irq, NULL, tps6586x_rtc_irq, IRQF_ONESHOT,
Interrupts are automatically enabled when requested, so we need to initialize irq_en accordingly to avoid causing an unbalanced enable warning. Signed-off-by: Esben Haabendal <esben@geanix.com> --- drivers/rtc/rtc-tps6586x.c | 1 + 1 file changed, 1 insertion(+)