Re: [PATCH v3 06/12] rtc: renesas-rtca3: Add driver for RTCA-3 available on Renesas RZ/G3S SoC

Alexandre Belloni
2024-08-30T22:25:50Z

On 30/08/2024 16:02:12+0300, Claudiu wrote:
> + priv->rtc_dev->range_min = mktime64(2000, 1, 1, 0, 0, 0);

RTC_TIMESTAMP_BEGIN_2000

> + priv->rtc_dev->range_max = mktime64(2099, 12, 31, 23, 59, 59);

RTC_TIMESTAMP_END_2099

> +
> + return devm_rtc_register_device(priv->rtc_dev);
> +}
> +
> +static void rtca3_remove(struct platform_device *pdev)
> +{
> + struct rtca3_priv *priv = platform_get_drvdata(pdev);
> +
> + guard(spinlock_irqsave)(&priv->lock);
> +
> + /* Disable alarm, periodic interrupts. */
> + rtca3_alarm_irq_set_helper(priv, RTCA3_RCR1_AIE | RTCA3_RCR1_PIE, 0);

Why do you disable alarms on driver remove? I think you need to add a
comment if this is because it can't system up, else this is a bad
practice.

> +
> +static int rtca3_clean_alarm(struct rtca3_priv *priv)
> +{
> + struct rtc_device *rtc_dev = priv->rtc_dev;
> + time64_t alarm_time, now;
> + struct rtc_wkalrm alarm;
> + struct rtc_time tm;
> + u8 pending;
> + int ret;
> +
> + ret = rtc_read_alarm(rtc_dev, &alarm);
> + if (ret)
> + return ret;
> +
> + if (!alarm.enabled)
> + return 0;
> +
> + ret = rtc_read_time(rtc_dev, &tm);
> + if (ret)
> + return ret;
> +
> + alarm_time = rtc_tm_to_time64(&alarm.time);
> + now = rtc_tm_to_time64(&tm);
> + if (alarm_time >= now)
> + return 0;
> +
> + /*
> + * Heuristically, it has been determined that when returning from deep
> + * sleep state the RTCA3_RSR.AF is zero even though the alarm expired.
> + * Call again the rtc_update_irq() if alarm helper detects this.
> + */
> +
> + guard(spinlock_irqsave)(&priv->lock);
> +
> + pending = rtca3_alarm_handler_helper(priv);
> + if (!pending)
> + rtc_update_irq(priv->rtc_dev, 1, RTC_AF | RTC_IRQF);
> +
> + return 0;
> +}
> +
> +static int rtca3_resume(struct device *dev)
> +{
> + struct rtca3_priv *priv = dev_get_drvdata(dev);
> +
> + if (!device_may_wakeup(dev))
> + return 0;
> +
> + disable_irq_wake(priv->wakeup_irq);
> +
> + /*
> + * According to the HW manual (section 22.6.4 Notes on writing to
> + * and reading from registers) we need to wait 1/128 seconds while
> + * RCR2.START = 1 to be able to read the counters after a return from low
> + * power consumption state.
> + */
> + mdelay(8);
> +
> + /*
> + * The alarm cannot wake the system from deep sleep states. In case
> + * we return from deep sleep states and the alarm expired we need
> + * to disable it to avoid failures when setting another alarm.
> + */
> + return rtca3_clean_alarm(priv);
> +}
> +
> +static DEFINE_SIMPLE_DEV_PM_OPS(rtca3_pm_ops, rtca3_suspend, rtca3_resume);
> +
> +static const struct of_device_id rtca3_of_match[] = {
> + { .compatible = "renesas,rz-rtca3", },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, rtca3_of_match);
> +
> +static struct platform_driver rtca3_platform_driver = {
> + .driver = {
> + .name = "rtc-rtca3",
> + .pm = pm_ptr(&rtca3_pm_ops),
> + .of_match_table = rtca3_of_match,
> + },
> + .probe = rtca3_probe,
> + .remove_new = rtca3_remove,
> +};
> +module_platform_driver(rtca3_platform_driver);
> +
> +MODULE_DESCRIPTION("Renesas RTCA-3 RTC driver");
> +MODULE_AUTHOR("Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>");
> +MODULE_LICENSE("GPL");
> --
> 2.39.2
>

--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering