[PATCH 2/3] rtc: s3c: support for exynosautov9 on-chip RTC

Devang Tailor posted 3 patches 3 months, 1 week ago
There is a newer version of this series
[PATCH 2/3] rtc: s3c: support for exynosautov9 on-chip RTC
Posted by Devang Tailor 3 months, 1 week ago
The on-chip RTC of this SoC is almost similar to the previous
versions of SoC. Hence re-use the existing driver with platform specific
change to enable RTC.

This has been tested with 'hwclock' & 'date' utilities

Signed-off-by: Devang Tailor <dev.tailor@samsung.com>
---
 drivers/rtc/rtc-s3c.c | 26 ++++++++++++++++++++++++++
 drivers/rtc/rtc-s3c.h |  4 ++++
 2 files changed, 30 insertions(+)

diff --git a/drivers/rtc/rtc-s3c.c b/drivers/rtc/rtc-s3c.c
index 5dd575865adf..00686aa805f2 100644
--- a/drivers/rtc/rtc-s3c.c
+++ b/drivers/rtc/rtc-s3c.c
@@ -384,6 +384,23 @@ static void s3c6410_rtc_disable(struct s3c_rtc *info)
 	writew(con, info->base + S3C2410_RTCCON);
 }
 
+static void exynosautov9_rtc_disable(struct s3c_rtc *info)
+{
+	unsigned int con;
+
+	con = readb(info->base + S3C2410_RTCCON);
+	con &= ~S3C2410_RTCCON_RTCEN;
+	writeb(con, info->base + S3C2410_RTCCON);
+
+	con = readb(info->base + EXYNOSAUTOV9_TICCON0);
+	con &= ~EXYNOSAUTOV9_TICCON_TICEN;
+	writeb(con, info->base + EXYNOSAUTOV9_TICCON0);
+
+	con = readb(info->base + EXYNOSAUTOV9_TICCON1);
+	con &= ~EXYNOSAUTOV9_TICCON_TICEN;
+	writeb(con, info->base + EXYNOSAUTOV9_TICCON1);
+}
+
 static void s3c_rtc_remove(struct platform_device *pdev)
 {
 	struct s3c_rtc *info = platform_get_drvdata(pdev);
@@ -574,6 +591,12 @@ static struct s3c_rtc_data const s3c6410_rtc_data = {
 	.disable		= s3c6410_rtc_disable,
 };
 
+static struct s3c_rtc_data const exynosautov9_rtc_data = {
+	.irq_handler		= s3c6410_rtc_irq,
+	.enable			= s3c24xx_rtc_enable,
+	.disable		= exynosautov9_rtc_disable,
+};
+
 static const __maybe_unused struct of_device_id s3c_rtc_dt_match[] = {
 	{
 		.compatible = "samsung,s3c2410-rtc",
@@ -590,6 +613,9 @@ static const __maybe_unused struct of_device_id s3c_rtc_dt_match[] = {
 	}, {
 		.compatible = "samsung,exynos3250-rtc",
 		.data = &s3c6410_rtc_data,
+	}, {
+		.compatible = "samsung,exynosautov9-rtc",
+		.data = &exynosautov9_rtc_data,
 	},
 	{ /* sentinel */ },
 };
diff --git a/drivers/rtc/rtc-s3c.h b/drivers/rtc/rtc-s3c.h
index 3552914aa611..ed57ebe06b41 100644
--- a/drivers/rtc/rtc-s3c.h
+++ b/drivers/rtc/rtc-s3c.h
@@ -14,6 +14,10 @@
 #define S3C2410_INTP_ALM	(1 << 1)
 #define S3C2410_INTP_TIC	(1 << 0)
 
+#define	EXYNOSAUTOV9_TICCON0	S3C2410_RTCREG(0x38)
+#define	EXYNOSAUTOV9_TICCON1	S3C2410_RTCREG(0x3C)
+#define	EXYNOSAUTOV9_TICCON_TICEN	(1 << 0)
+
 #define S3C2410_RTCCON		S3C2410_RTCREG(0x40)
 #define S3C2410_RTCCON_RTCEN	(1 << 0)
 #define S3C2410_RTCCON_CNTSEL	(1 << 2)
-- 
2.34.1
Re: [PATCH 2/3] rtc: s3c: support for exynosautov9 on-chip RTC
Posted by Krzysztof Kozlowski 3 months ago
On 02/07/2025 07:24, Devang Tailor wrote:
> The on-chip RTC of this SoC is almost similar to the previous
> versions of SoC. Hence re-use the existing driver with platform specific
> change to enable RTC.
> 
> This has been tested with 'hwclock' & 'date' utilities
> 
> Signed-off-by: Devang Tailor <dev.tailor@samsung.com>
> ---
>  drivers/rtc/rtc-s3c.c | 26 ++++++++++++++++++++++++++
>  drivers/rtc/rtc-s3c.h |  4 ++++
>  2 files changed, 30 insertions(+)
> 
> diff --git a/drivers/rtc/rtc-s3c.c b/drivers/rtc/rtc-s3c.c
> index 5dd575865adf..00686aa805f2 100644
> --- a/drivers/rtc/rtc-s3c.c
> +++ b/drivers/rtc/rtc-s3c.c
> @@ -384,6 +384,23 @@ static void s3c6410_rtc_disable(struct s3c_rtc *info)
>  	writew(con, info->base + S3C2410_RTCCON);
>  }
>  
> +static void exynosautov9_rtc_disable(struct s3c_rtc *info)
> +{
> +	unsigned int con;
> +
> +	con = readb(info->base + S3C2410_RTCCON);
> +	con &= ~S3C2410_RTCCON_RTCEN;
> +	writeb(con, info->base + S3C2410_RTCCON);
> +
> +	con = readb(info->base + EXYNOSAUTOV9_TICCON0);
> +	con &= ~EXYNOSAUTOV9_TICCON_TICEN;
> +	writeb(con, info->base + EXYNOSAUTOV9_TICCON0);
> +
> +	con = readb(info->base + EXYNOSAUTOV9_TICCON1);
> +	con &= ~EXYNOSAUTOV9_TICCON_TICEN;
> +	writeb(con, info->base + EXYNOSAUTOV9_TICCON1);

You clear these bits during disable, but why aren't they set during
enable? Why is this asymmetric? This should be clearly explained, but
both commit msg and code is completely silent.

> +}
> +
>  static void s3c_rtc_remove(struct platform_device *pdev)
>  {
>  	struct s3c_rtc *info = platform_get_drvdata(pdev);
> @@ -574,6 +591,12 @@ static struct s3c_rtc_data const s3c6410_rtc_data = {
>  	.disable		= s3c6410_rtc_disable,
>  };
>  
> +static struct s3c_rtc_data const exynosautov9_rtc_data = {

Please put const after static.



Best regards,
Krzysztof
RE: [PATCH 2/3] rtc: s3c: support for exynosautov9 on-chip RTC
Posted by Devang Tailor 3 months ago
Hi Krzysztof,


> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: 07 July 2025 14:54
> To: Devang Tailor <dev.tailor@samsung.com>;
> alexandre.belloni@bootlin.com; robh@kernel.org; krzk+dt@kernel.org;
> conor+dt@kernel.org; alim.akhtar@samsung.com; linux-rtc@vger.kernel.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; inux-arm-
> kernel@lists.infradead.org; linux-samsung-soc@vger.kernel.org;
> faraz.ata@samsung.com
> Subject: Re: [PATCH 2/3] rtc: s3c: support for exynosautov9 on-chip RTC
> 
> On 02/07/2025 07:24, Devang Tailor wrote:
> > The on-chip RTC of this SoC is almost similar to the previous versions
> > of SoC. Hence re-use the existing driver with platform specific change
> > to enable RTC.
> >
> > This has been tested with 'hwclock' & 'date' utilities
> >
> > Signed-off-by: Devang Tailor <dev.tailor@samsung.com>
> > ---
> >  drivers/rtc/rtc-s3c.c | 26 ++++++++++++++++++++++++++
> > drivers/rtc/rtc-s3c.h |  4 ++++
> >  2 files changed, 30 insertions(+)
> >
> > diff --git a/drivers/rtc/rtc-s3c.c b/drivers/rtc/rtc-s3c.c index
> > 5dd575865adf..00686aa805f2 100644
> > --- a/drivers/rtc/rtc-s3c.c
> > +++ b/drivers/rtc/rtc-s3c.c
> > @@ -384,6 +384,23 @@ static void s3c6410_rtc_disable(struct s3c_rtc
> *info)
> >  	writew(con, info->base + S3C2410_RTCCON);  }
> >
> > +static void exynosautov9_rtc_disable(struct s3c_rtc *info) {
> > +	unsigned int con;
> > +
> > +	con = readb(info->base + S3C2410_RTCCON);
> > +	con &= ~S3C2410_RTCCON_RTCEN;
> > +	writeb(con, info->base + S3C2410_RTCCON);
> > +
> > +	con = readb(info->base + EXYNOSAUTOV9_TICCON0);
> > +	con &= ~EXYNOSAUTOV9_TICCON_TICEN;
> > +	writeb(con, info->base + EXYNOSAUTOV9_TICCON0);
> > +
> > +	con = readb(info->base + EXYNOSAUTOV9_TICCON1);
> > +	con &= ~EXYNOSAUTOV9_TICCON_TICEN;
> > +	writeb(con, info->base + EXYNOSAUTOV9_TICCON1);
> 
> You clear these bits during disable, but why aren't they set during enable?
> Why is this asymmetric? This should be clearly explained, but both commit
> msg and code is completely silent.

OK. I will correct in V2 patch

> 
> > +}
> > +
> >  static void s3c_rtc_remove(struct platform_device *pdev)  {
> >  	struct s3c_rtc *info = platform_get_drvdata(pdev); @@ -574,6 +591,12
> > @@ static struct s3c_rtc_data const s3c6410_rtc_data = {
> >  	.disable		= s3c6410_rtc_disable,
> >  };
> >
> > +static struct s3c_rtc_data const exynosautov9_rtc_data = {
> 
> Please put const after static.

I tried to keep it similar to the existing format, I will correct it in V2 patch.

> 
> 
> 
> Best regards,
> Krzysztof