Re: [PATCH v7 2/4] rtc: s32g: add NXP S32G2/S32G3 SoC support

[Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>]

On 2/11/2025 7:07 PM, Frank Li wrote:
> On Tue, Feb 11, 2025 at 01:25:38PM +0200, Ciprian Marian Costea wrote:
>> On 2/7/2025 10:16 PM, Frank Li wrote:
>>> On Fri, Feb 07, 2025 at 06:38:06PM +0200, Ciprian Costea wrote:
>>>> From: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>
>>>>
>>>> Add a RTC driver for NXP S32G2/S32G3 SoCs.
>>>>
>>>> RTC tracks clock time during system suspend. It can be a wakeup source
>>>> for the S32G2/S32G3 SoC based boards.
>>>>
>>>> The RTC module from S32G2/S32G3 is not battery-powered and it is not kept
>>>> alive during system reset.
>>>>
>>>> Co-developed-by: Bogdan Hamciuc <bogdan.hamciuc@nxp.com>
>>>> Signed-off-by: Bogdan Hamciuc <bogdan.hamciuc@nxp.com>
>>>> Co-developed-by: Ghennadi Procopciuc <Ghennadi.Procopciuc@nxp.com>
>>>> Signed-off-by: Ghennadi Procopciuc <Ghennadi.Procopciuc@nxp.com>
>>>> Signed-off-by: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>
>>>> ---
>>>>    drivers/rtc/Kconfig    |  11 ++
>>>>    drivers/rtc/Makefile   |   1 +
>>>>    drivers/rtc/rtc-s32g.c | 383 +++++++++++++++++++++++++++++++++++++++++
>>>>    3 files changed, 395 insertions(+)
>>>>    create mode 100644 drivers/rtc/rtc-s32g.c
>>>>
>>>> diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
>>>> index 0bbbf778ecfa..510dc2db745d 100644
>>>> --- a/drivers/rtc/Kconfig
>>>> +++ b/drivers/rtc/Kconfig
>>>> @@ -2103,4 +2103,15 @@ config RTC_DRV_AMLOGIC_A4
>>>>    	  This driver can also be built as a module. If so, the module
>>>>    	  will be called "rtc-amlogic-a4".
>>>>
>>>> +config RTC_DRV_S32G
>>>> +	tristate "RTC driver for S32G2/S32G3 SoCs"
>>>> +	depends on ARCH_S32 || COMPILE_TEST
>>>> +	depends on COMMON_CLK
>>>> +	help
>>>> +	  Say yes to enable RTC driver for platforms based on the
>>>> +	  S32G2/S32G3 SoC family.
>>>> +
>>>> +	  This RTC module can be used as a wakeup source.
>>>> +	  Please note that it is not battery-powered.
>>>> +
>>>>    endif # RTC_CLASS
>>>> diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
>>>> index 489b4ab07068..e4b616ecd5ce 100644
>>>> --- a/drivers/rtc/Makefile
>>>> +++ b/drivers/rtc/Makefile
>>>> @@ -161,6 +161,7 @@ obj-$(CONFIG_RTC_DRV_RX8111)	+= rtc-rx8111.o
>>>>    obj-$(CONFIG_RTC_DRV_RX8581)	+= rtc-rx8581.o
>>>>    obj-$(CONFIG_RTC_DRV_RZN1)	+= rtc-rzn1.o
>>>>    obj-$(CONFIG_RTC_DRV_RENESAS_RTCA3)	+= rtc-renesas-rtca3.o
>>>> +obj-$(CONFIG_RTC_DRV_S32G)	+= rtc-s32g.o
>>>>    obj-$(CONFIG_RTC_DRV_S35390A)	+= rtc-s35390a.o
>>>>    obj-$(CONFIG_RTC_DRV_S3C)	+= rtc-s3c.o
>>>>    obj-$(CONFIG_RTC_DRV_S5M)	+= rtc-s5m.o
>>>> diff --git a/drivers/rtc/rtc-s32g.c b/drivers/rtc/rtc-s32g.c
>>>> new file mode 100644
>>>> index 000000000000..3244b23c533e
>>>> --- /dev/null
>>>> +++ b/drivers/rtc/rtc-s32g.c
>>>
>>> ...
>>>
>>>> +
>>>> +static int s32g_rtc_suspend(struct device *dev)
>>>> +{
>>>> +	struct rtc_priv *priv = dev_get_drvdata(dev);
>>>> +	u32 apival = readl(priv->rtc_base + APIVAL_OFFSET);
>>>> +
>>>> +	/* RTC registers are being reset in suspend.
>>>> +	 * Thus store standby time.
>>>> +	 */
>>>> +	if (check_add_overflow(priv->sleep_sec, div64_u64(apival, priv->rtc_hz),
>>>> +			       &priv->sleep_sec)) {
>>>> +		dev_warn(dev, "Overflow on sleep cycles occurred. Resetting to 0.\n");
>>>> +		priv->sleep_sec = 0;
>>>> +	}
>>>
>>> Strange. If RTC register are reset in suspend. How do it wake up system?
>>>
>>> Frank
>>>
>>
>> Hello Frank,
>>
>> I believe the transition between resume to run is a reset event. This would
>> lead to the RTC registers being reset after resume from suspend (triggered
>> via Suspend to RAM).
> 
> It is weird design. I suppose it should be design error. (any errata for
> it).
> 
> Frank
> 

Unfortunately I could not find any such errata. But this behaviour (RTC 
registers being reset) is what I see when resuming from Suspend to RAM 
operations.

Regards,
Ciprian

>>
>> On the other hand, the RTC is kept on during suspend for as long as its been
>> set up (for example via rtcwake -s), thus waking up the sistem via an API
>> interrupt.
>>
>> Regards,
>> Ciprian
>>
>>>> +
>>>> +	return 0;
>>>> +}
>>>> +
>>>> +static int s32g_rtc_resume(struct device *dev)
>>>> +{
>>>> +	struct rtc_priv *priv = dev_get_drvdata(dev);
>>>> +
>>>> +	return rtc_clk_src_setup(priv);
>>>> +}
>>>> +
>>>> +static const struct of_device_id rtc_dt_ids[] = {
>>>> +	{ .compatible = "nxp,s32g2-rtc", .data = &rtc_s32g2_data},
>>>> +	{ /* sentinel */ },
>>>> +};
>>>> +
>>>> +static DEFINE_SIMPLE_DEV_PM_OPS(s32g_rtc_pm_ops,
>>>> +			 s32g_rtc_suspend, s32g_rtc_resume);
>>>> +
>>>> +static struct platform_driver s32g_rtc_driver = {
>>>> +	.driver		= {
>>>> +		.name			= "s32g-rtc",
>>>> +		.pm				= pm_sleep_ptr(&s32g_rtc_pm_ops),
>>>> +		.of_match_table = rtc_dt_ids,
>>>> +	},
>>>> +	.probe		= s32g_rtc_probe,
>>>> +};
>>>> +module_platform_driver(s32g_rtc_driver);
>>>> +
>>>> +MODULE_AUTHOR("NXP");
>>>> +MODULE_DESCRIPTION("NXP RTC driver for S32G2/S32G3");
>>>> +MODULE_LICENSE("GPL");
>>>> --
>>>> 2.45.2
>>>>
>>