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

[Krzysztof Kozlowski <krzk@kernel.org>]

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