On 10/08/2023 15:21:47+0800, Jacky Huang wrote:
> > > > > + return 0;
> > > > > +}
> > > > > +
> > > > > +static int ma35d1_rtc_suspend(struct platform_device *pdev, pm_message_t state)
> > > > > +{
> > > > > + struct ma35_rtc *rtc = platform_get_drvdata(pdev);
> > > > > + u32 regval;
> > > > > +
> > > > > + if (device_may_wakeup(&pdev->dev))
> > > > > + enable_irq_wake(rtc->irq_num);
> > > > > +
> > > > > + regval = rtc_reg_read(rtc, MA35_REG_RTC_INTEN);
> > > > > + regval &= ~RTC_INTEN_TICKIEN;
> > > > > + rtc_reg_write(rtc, MA35_REG_RTC_INTEN, regval);
> > > > This is not what the user is asking, don't do this. Also, how was this
> > > > tested?
> > > Sure, I will remove these three lines of code.
> > >
> > > We test it with "echo mem > /sys/power/state".
> > >
> > Yes, my point is that if UIE is enabled, then the user wants to be woken
> > up every second. If this is not what is wanted, then UIE has to be
> > disabled before going to suspend.
> >
> > My question is why are you enabling RTC_INTEN_TICKIEN in probe? I don't
> > expect anyone to use an actual hardware tick interrupt, unless the alarm
> > is broken and can't be set every second. This is why I questioned the
> > RTC_UF path because I don't expect it to be taken at all.
>
> Yes, we will remove TICKIEN from probe and modify ma35d1_alarm_irq_enable().
> TICKIEN will be enabled only if UIE is enabled.
>
> static int ma35d1_alarm_irq_enable(struct device *dev, unsigned int enabled)
> {
> struct ma35d1_rtc *rtc = dev_get_drvdata(dev);
>
> if (enabled) {
> if (rtc->rtc->uie_rtctimer.enabled)
> rtc_reg_write(rtc, NVT_RTC_INTEN,
> (rtc_reg_read(rtc,
> NVT_RTC_INTEN)|(RTC_INTEN_TICKIEN)));
Don't do that unless the regular alarm can't be set every second. Simply
always use ALMIEN, then check rtctest is passing properly.
> if (rtc->rtc->aie_timer.enabled)
> rtc_reg_write(rtc, NVT_RTC_INTEN,
> (rtc_reg_read(rtc,
> NVT_RTC_INTEN)|(RTC_INTEN_ALMIEN)));
> } else {
> if (rtc->rtc->uie_rtctimer.enabled)
> rtc_reg_write(rtc, NVT_RTC_INTEN,
> (rtc_reg_read(rtc, NVT_RTC_INTEN) &
> (~RTC_INTEN_TICKIEN)));
> if (rtc->rtc->aie_timer.enabled)
> rtc_reg_write(rtc, NVT_RTC_INTEN,
> (rtc_reg_read(rtc, NVT_RTC_INTEN) &
> (~RTC_INTEN_ALMIEN)));
> }
> return 0;
> }
>
--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
