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

[Alexandre Belloni <alexandre.belloni@bootlin.com>]

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
https://bootlin.com