Re: [RESEND PATCH v2 3/3] rtc: Add driver for Nuvoton ma35d1 rtc controller

From: Jacky Huang
Date: Thu Aug 10 2023 - 04:26:39 EST




On 2023/8/10 下午 03:30, Alexandre Belloni wrote:
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.

OK, I got it. I will drop the TICKINT and use ALMIEN only.

MA35D1 RTC has an alarm mask register which supports alarm mask for seconds, minutes, and hours.
We will use the alarm mask to have RTC generate an alarm interrupt per second, and make sure
the driver can pass rtctest.

        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;
}



Best Regards,
Jacky Huang