1. Patchew
  2. linux
  3. [PATCH 0/2]
  4. View patch

[PATCH 1/2] rtc: m48t59: Accommodate chips that lack a century bit

Finn Thain posted 2 patches 1 month, 3 weeks ago
There is a newer version of this series
[PATCH 1/2] rtc: m48t59: Accommodate chips that lack a century bit
Posted by Finn Thain 1 month, 3 weeks ago 
The m48t59 driver is needed by both SPARC and MVME systems. Linux on
MVME uses 1970 as "year zero" rather than 1968 that's used on SPARC.
Add support for the MVME convention. Otherwise, the RTC on non-SPARC
systems can only read and write dates between 1900 and 1999.

Tested-by: Daniel Palmer <daniel@0x0f.com>
Signed-off-by: Finn Thain <fthain@linux-m68k.org>
---
 drivers/rtc/rtc-m48t59.c | 31 +++++++++++++++----------------
 1 file changed, 15 insertions(+), 16 deletions(-)

diff --git a/drivers/rtc/rtc-m48t59.c b/drivers/rtc/rtc-m48t59.c
index f0f6b9b6daec..e2d882ea5c2f 100644
--- a/drivers/rtc/rtc-m48t59.c
+++ b/drivers/rtc/rtc-m48t59.c
@@ -57,6 +57,17 @@ m48t59_mem_readb(struct device *dev, u32 ofs)
 	return readb(m48t59->ioaddr+ofs);
 }
 
+/*
+ * Sun SPARC machines count years since 1968. MVME machines running Linux
+ * count years since 1970.
+ */
+
+#ifdef CONFIG_SPARC
+#define YEAR0 68
+#else
+#define YEAR0 70
+#endif
+
 /*
  * NOTE: M48T59 only uses BCD mode
  */
@@ -82,10 +93,7 @@ static int m48t59_rtc_read_time(struct device *dev, struct rtc_time *tm)
 		dev_dbg(dev, "Century bit is enabled\n");
 		tm->tm_year += 100;	/* one century */
 	}
-#ifdef CONFIG_SPARC
-	/* Sun SPARC machines count years since 1968 */
-	tm->tm_year += 68;
-#endif
+	tm->tm_year += YEAR0;
 
 	tm->tm_wday	= bcd2bin(val & 0x07);
 	tm->tm_hour	= bcd2bin(M48T59_READ(M48T59_HOUR) & 0x3F);
@@ -108,10 +116,7 @@ static int m48t59_rtc_set_time(struct device *dev, struct rtc_time *tm)
 	u8 val = 0;
 	int year = tm->tm_year;
 
-#ifdef CONFIG_SPARC
-	/* Sun SPARC machines count years since 1968 */
-	year -= 68;
-#endif
+	year -= YEAR0;
 
 	dev_dbg(dev, "RTC set time %04d-%02d-%02d %02d/%02d/%02d\n",
 		year + 1900, tm->tm_mon, tm->tm_mday,
@@ -163,10 +168,7 @@ static int m48t59_rtc_readalarm(struct device *dev, struct rtc_wkalrm *alrm)
 	M48T59_SET_BITS(M48T59_CNTL_READ, M48T59_CNTL);
 
 	tm->tm_year = bcd2bin(M48T59_READ(M48T59_YEAR));
-#ifdef CONFIG_SPARC
-	/* Sun SPARC machines count years since 1968 */
-	tm->tm_year += 68;
-#endif
+	tm->tm_year += YEAR0;
 	/* tm_mon is 0-11 */
 	tm->tm_mon = bcd2bin(M48T59_READ(M48T59_MONTH)) - 1;
 
@@ -199,10 +201,7 @@ static int m48t59_rtc_setalarm(struct device *dev, struct rtc_wkalrm *alrm)
 	unsigned long flags;
 	int year = tm->tm_year;
 
-#ifdef CONFIG_SPARC
-	/* Sun SPARC machines count years since 1968 */
-	year -= 68;
-#endif
+	year -= YEAR0;
 
 	/* If no irq, we don't support ALARM */
 	if (m48t59->irq == NO_IRQ)
-- 
2.39.5
Re: [PATCH 1/2] rtc: m48t59: Accommodate chips that lack a century bit
Posted by Alexandre Belloni 1 month, 3 weeks ago 
Hello,

On 03/10/2024 13:23:22+1000, Finn Thain wrote:
> The m48t59 driver is needed by both SPARC and MVME systems. Linux on
> MVME uses 1970 as "year zero" rather than 1968 that's used on SPARC.
> Add support for the MVME convention. Otherwise, the RTC on non-SPARC
> systems can only read and write dates between 1900 and 1999.
> 
> Tested-by: Daniel Palmer <daniel@0x0f.com>
> Signed-off-by: Finn Thain <fthain@linux-m68k.org>
> ---
>  drivers/rtc/rtc-m48t59.c | 31 +++++++++++++++----------------
>  1 file changed, 15 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/rtc/rtc-m48t59.c b/drivers/rtc/rtc-m48t59.c
> index f0f6b9b6daec..e2d882ea5c2f 100644
> --- a/drivers/rtc/rtc-m48t59.c
> +++ b/drivers/rtc/rtc-m48t59.c
> @@ -57,6 +57,17 @@ m48t59_mem_readb(struct device *dev, u32 ofs)
>  	return readb(m48t59->ioaddr+ofs);
>  }
>  
> +/*
> + * Sun SPARC machines count years since 1968. MVME machines running Linux
> + * count years since 1970.
> + */
> +
> +#ifdef CONFIG_SPARC
> +#define YEAR0 68
> +#else
> +#define YEAR0 70
> +#endif
> +
>  /*
>   * NOTE: M48T59 only uses BCD mode
>   */
> @@ -82,10 +93,7 @@ static int m48t59_rtc_read_time(struct device *dev, struct rtc_time *tm)
>  		dev_dbg(dev, "Century bit is enabled\n");
>  		tm->tm_year += 100;	/* one century */
>  	}
> -#ifdef CONFIG_SPARC
> -	/* Sun SPARC machines count years since 1968 */
> -	tm->tm_year += 68;
> -#endif
> +	tm->tm_year += YEAR0;
>  

I'm super happy to see someone working on this, while you are it, can
you use m48t59->rtc->start_secs and m48t59->rtc->set_start_time in probe
instead of offsetting tm_year in read_time/set_time so we can later use
device tree or any other mechanism to extend the range?

It is super funny because I was telling Geert that I wanted to get rid
of this #ifdef CONFIG_SPARC two weeks ago at LPC. That could indeed then
come from platform data.

>  	tm->tm_wday	= bcd2bin(val & 0x07);
>  	tm->tm_hour	= bcd2bin(M48T59_READ(M48T59_HOUR) & 0x3F);
> @@ -108,10 +116,7 @@ static int m48t59_rtc_set_time(struct device *dev, struct rtc_time *tm)
>  	u8 val = 0;
>  	int year = tm->tm_year;
>  
> -#ifdef CONFIG_SPARC
> -	/* Sun SPARC machines count years since 1968 */
> -	year -= 68;
> -#endif
> +	year -= YEAR0;
>  
>  	dev_dbg(dev, "RTC set time %04d-%02d-%02d %02d/%02d/%02d\n",
>  		year + 1900, tm->tm_mon, tm->tm_mday,
> @@ -163,10 +168,7 @@ static int m48t59_rtc_readalarm(struct device *dev, struct rtc_wkalrm *alrm)
>  	M48T59_SET_BITS(M48T59_CNTL_READ, M48T59_CNTL);
>  
>  	tm->tm_year = bcd2bin(M48T59_READ(M48T59_YEAR));
> -#ifdef CONFIG_SPARC
> -	/* Sun SPARC machines count years since 1968 */
> -	tm->tm_year += 68;
> -#endif
> +	tm->tm_year += YEAR0;
>  	/* tm_mon is 0-11 */
>  	tm->tm_mon = bcd2bin(M48T59_READ(M48T59_MONTH)) - 1;
>  
> @@ -199,10 +201,7 @@ static int m48t59_rtc_setalarm(struct device *dev, struct rtc_wkalrm *alrm)
>  	unsigned long flags;
>  	int year = tm->tm_year;
>  
> -#ifdef CONFIG_SPARC
> -	/* Sun SPARC machines count years since 1968 */
> -	year -= 68;
> -#endif
> +	year -= YEAR0;
>  
>  	/* If no irq, we don't support ALARM */
>  	if (m48t59->irq == NO_IRQ)
> -- 
> 2.39.5
> 

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Re: [PATCH 1/2] rtc: m48t59: Accommodate chips that lack a century bit
Posted by Finn Thain 1 month, 3 weeks ago 
On Thu, 3 Oct 2024, Alexandre Belloni wrote:

> 
> ... while you are it, can you use m48t59->rtc->start_secs and 
> m48t59->rtc->set_start_time in probe instead of offsetting tm_year in 
> read_time/set_time so we can later use device tree or any other 
> mechanism to extend the range?
> 

That didn't work out as I'd hoped. I booted a patched kernel (diff below) 
under qemu-system-sparc64:

~ # for yyyy in 1970 1971 1999 2000 2024 2025 2068 2069 ; do 
date 01010101$yyyy ; hwclock --systohc --utc && hwclock --utc ; echo ; done
Thu Jan  1 01:01:00 UTC 1970
Thu Jan  1 01:01:00 1970  0.000000 seconds

Fri Jan  1 01:01:00 UTC 1971
Tue Nov 24 18:32:44 1998  0.000000 seconds

Fri Jan  1 01:01:00 UTC 1999
Tue Nov 24 18:32:44 2026  0.000000 seconds

Sat Jan  1 01:01:00 UTC 2000
Sun Jan  2 23:29:16 2000  0.000000 seconds

Mon Jan  1 01:01:00 UTC 2024
Tue Jan  2 23:29:16 2024  0.000000 seconds

Wed Jan  1 01:01:00 UTC 2025
Thu Jan  2 23:29:16 2025  0.000000 seconds

Sun Jan  1 01:01:00 UTC 2068
hwclock: RTC_SET_TIME: Numerical result out of range

Tue Jan  1 01:01:00 UTC 2069
hwclock: RTC_SET_TIME: Numerical result out of range

~ # 

Here's the result from an unpatched kernel (v6.11):

~ # for yyyy in 1970 1971 1999 2000 2024 2025 2068 2069 ; do 
date 01010101$yyyy ; hwclock --systohc --utc && hwclock --utc ; echo ; done
Thu Jan  1 01:01:00 UTC 1970
Thu Jan  1 01:01:00 1970  0.000000 seconds

Fri Jan  1 01:01:00 UTC 1971
Fri Jan  1 01:01:00 1971  0.000000 seconds

Fri Jan  1 01:01:00 UTC 1999
Fri Jan  1 01:01:01 1999  0.000000 seconds

Sat Jan  1 01:01:00 UTC 2000
Sat Jan  1 01:01:00 2000  0.000000 seconds

Mon Jan  1 01:01:00 UTC 2024
Mon Jan  1 01:01:00 2024  0.000000 seconds

Wed Jan  1 01:01:00 UTC 2025
Wed Jan  1 01:01:00 2025  0.000000 seconds

Sun Jan  1 01:01:00 UTC 2068
hwclock: RTC_RD_TIME: Invalid argument

Tue Jan  1 01:01:00 UTC 2069
hwclock: RTC_RD_TIME: Invalid argument

~ # 


I'm afraid I don't see how we might avoid adding/subtracting in 
read_time/set_time given that we must avoid messing up the present date 
when users boot into an upgraded kernel.


diff --git a/arch/sparc/kernel/time_32.c b/arch/sparc/kernel/time_32.c
index 08bbdc458596..41ae3d1aa12e 100644
--- a/arch/sparc/kernel/time_32.c
+++ b/arch/sparc/kernel/time_32.c
@@ -255,6 +255,7 @@ static void mostek_write_byte(struct device *dev, u32 ofs, u8 val)
 static struct m48t59_plat_data m48t59_data = {
 	.read_byte = mostek_read_byte,
 	.write_byte = mostek_write_byte,
+	.start_year = 1968,
 };
 
 /* resource is set at runtime */
diff --git a/arch/sparc/kernel/time_64.c b/arch/sparc/kernel/time_64.c
index 60f1c8cc5363..eceb3fadb71a 100644
--- a/arch/sparc/kernel/time_64.c
+++ b/arch/sparc/kernel/time_64.c
@@ -544,6 +544,7 @@ static void mostek_write_byte(struct device *dev, u32 ofs, u8 val)
 static struct m48t59_plat_data m48t59_data = {
 	.read_byte	= mostek_read_byte,
 	.write_byte	= mostek_write_byte,
+	.start_year	= 1968,
 };
 
 static struct platform_device m48t59_rtc = {
diff --git a/drivers/rtc/rtc-m48t59.c b/drivers/rtc/rtc-m48t59.c
index f0f6b9b6daec..d7e1f79cd52b 100644
--- a/drivers/rtc/rtc-m48t59.c
+++ b/drivers/rtc/rtc-m48t59.c
@@ -82,10 +82,6 @@ static int m48t59_rtc_read_time(struct device *dev, struct rtc_time *tm)
 		dev_dbg(dev, "Century bit is enabled\n");
 		tm->tm_year += 100;	/* one century */
 	}
-#ifdef CONFIG_SPARC
-	/* Sun SPARC machines count years since 1968 */
-	tm->tm_year += 68;
-#endif
 
 	tm->tm_wday	= bcd2bin(val & 0x07);
 	tm->tm_hour	= bcd2bin(M48T59_READ(M48T59_HOUR) & 0x3F);
@@ -108,11 +104,6 @@ static int m48t59_rtc_set_time(struct device *dev, struct rtc_time *tm)
 	u8 val = 0;
 	int year = tm->tm_year;
 
-#ifdef CONFIG_SPARC
-	/* Sun SPARC machines count years since 1968 */
-	year -= 68;
-#endif
-
 	dev_dbg(dev, "RTC set time %04d-%02d-%02d %02d/%02d/%02d\n",
 		year + 1900, tm->tm_mon, tm->tm_mday,
 		tm->tm_hour, tm->tm_min, tm->tm_sec);
@@ -163,10 +154,7 @@ static int m48t59_rtc_readalarm(struct device *dev, struct rtc_wkalrm *alrm)
 	M48T59_SET_BITS(M48T59_CNTL_READ, M48T59_CNTL);
 
 	tm->tm_year = bcd2bin(M48T59_READ(M48T59_YEAR));
-#ifdef CONFIG_SPARC
-	/* Sun SPARC machines count years since 1968 */
-	tm->tm_year += 68;
-#endif
+
 	/* tm_mon is 0-11 */
 	tm->tm_mon = bcd2bin(M48T59_READ(M48T59_MONTH)) - 1;
 
@@ -199,11 +187,6 @@ static int m48t59_rtc_setalarm(struct device *dev, struct rtc_wkalrm *alrm)
 	unsigned long flags;
 	int year = tm->tm_year;
 
-#ifdef CONFIG_SPARC
-	/* Sun SPARC machines count years since 1968 */
-	year -= 68;
-#endif
-
 	/* If no irq, we don't support ALARM */
 	if (m48t59->irq == NO_IRQ)
 		return -EIO;
@@ -458,6 +441,10 @@ static int m48t59_rtc_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, m48t59);
 
 	m48t59->rtc->ops = &m48t59_rtc_ops;
+	m48t59->rtc->range_min = mktime64(1900, 1, 1, 0, 0, 0);
+	m48t59->rtc->range_max = mktime64(1999, 12, 31, 23, 59, 59);
+	m48t59->rtc->start_secs = mktime64(pdata->start_year, 1, 1, 0, 0, 0);
+	m48t59->rtc->set_start_time = true;
 
 	nvmem_cfg.size = pdata->offset;
 	ret = devm_rtc_nvmem_register(m48t59->rtc, &nvmem_cfg);
diff --git a/include/linux/rtc/m48t59.h b/include/linux/rtc/m48t59.h
index 9465d5405fe2..b01c514d7079 100644
--- a/include/linux/rtc/m48t59.h
+++ b/include/linux/rtc/m48t59.h
@@ -56,6 +56,9 @@ struct m48t59_plat_data {
 	void __iomem *ioaddr;
 	/* offset to RTC registers, automatically set according to the type */
 	unsigned int offset;
+
+	/* value to be used to initialize rtc->start_secs */
+	time64_t start_year;
 };
 
 #endif /* _LINUX_RTC_M48T59_H_ */
Re: [PATCH 1/2] rtc: m48t59: Accommodate chips that lack a century bit
Posted by Alexandre Belloni 1 month, 1 week ago 
On 05/10/2024 14:23:28+1000, Finn Thain wrote:
> 
> On Thu, 3 Oct 2024, Alexandre Belloni wrote:
> 
> > 
> > ... while you are it, can you use m48t59->rtc->start_secs and 
> > m48t59->rtc->set_start_time in probe instead of offsetting tm_year in 
> > read_time/set_time so we can later use device tree or any other 
> > mechanism to extend the range?
> > 
> 
> That didn't work out as I'd hoped. I booted a patched kernel (diff below) 
> under qemu-system-sparc64:
> 
> ~ # for yyyy in 1970 1971 1999 2000 2024 2025 2068 2069 ; do 
> date 01010101$yyyy ; hwclock --systohc --utc && hwclock --utc ; echo ; done
> Thu Jan  1 01:01:00 UTC 1970
> Thu Jan  1 01:01:00 1970  0.000000 seconds
> 
> Fri Jan  1 01:01:00 UTC 1971
> Tue Nov 24 18:32:44 1998  0.000000 seconds
> 
> Fri Jan  1 01:01:00 UTC 1999
> Tue Nov 24 18:32:44 2026  0.000000 seconds
> 
> Sat Jan  1 01:01:00 UTC 2000
> Sun Jan  2 23:29:16 2000  0.000000 seconds
> 
> Mon Jan  1 01:01:00 UTC 2024
> Tue Jan  2 23:29:16 2024  0.000000 seconds
> 
> Wed Jan  1 01:01:00 UTC 2025
> Thu Jan  2 23:29:16 2025  0.000000 seconds
> 
> Sun Jan  1 01:01:00 UTC 2068
> hwclock: RTC_SET_TIME: Numerical result out of range
> 
> Tue Jan  1 01:01:00 UTC 2069
> hwclock: RTC_SET_TIME: Numerical result out of range
> 
> ~ # 
> 
> Here's the result from an unpatched kernel (v6.11):
> 
> ~ # for yyyy in 1970 1971 1999 2000 2024 2025 2068 2069 ; do 
> date 01010101$yyyy ; hwclock --systohc --utc && hwclock --utc ; echo ; done
> Thu Jan  1 01:01:00 UTC 1970
> Thu Jan  1 01:01:00 1970  0.000000 seconds
> 
> Fri Jan  1 01:01:00 UTC 1971
> Fri Jan  1 01:01:00 1971  0.000000 seconds
> 
> Fri Jan  1 01:01:00 UTC 1999
> Fri Jan  1 01:01:01 1999  0.000000 seconds
> 
> Sat Jan  1 01:01:00 UTC 2000
> Sat Jan  1 01:01:00 2000  0.000000 seconds
> 
> Mon Jan  1 01:01:00 UTC 2024
> Mon Jan  1 01:01:00 2024  0.000000 seconds
> 
> Wed Jan  1 01:01:00 UTC 2025
> Wed Jan  1 01:01:00 2025  0.000000 seconds
> 
> Sun Jan  1 01:01:00 UTC 2068
> hwclock: RTC_RD_TIME: Invalid argument
> 
> Tue Jan  1 01:01:00 UTC 2069
> hwclock: RTC_RD_TIME: Invalid argument
> 
> ~ # 
> 
> 
> I'm afraid I don't see how we might avoid adding/subtracting in 
> read_time/set_time given that we must avoid messing up the present date 
> when users boot into an upgraded kernel.

I'm pretty sure this is avoidable as this is exactly what the offset
mechanism is trying to achieve. I guess the issue is in the RTC core
because both range_min and start_secs are negative which has never been
tested. My plan was to have unit tests for this but this never
happened...


-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Re: [PATCH 1/2] rtc: m48t59: Accommodate chips that lack a century bit
Posted by Finn Thain 1 month, 3 weeks ago 
On Thu, 3 Oct 2024, Alexandre Belloni wrote:

> On 03/10/2024 13:23:22+1000, Finn Thain wrote:
> > The m48t59 driver is needed by both SPARC and MVME systems. Linux on
> > MVME uses 1970 as "year zero" rather than 1968 that's used on SPARC.
> > Add support for the MVME convention. Otherwise, the RTC on non-SPARC
> > systems can only read and write dates between 1900 and 1999.
> > 
> > Tested-by: Daniel Palmer <daniel@0x0f.com>
> > Signed-off-by: Finn Thain <fthain@linux-m68k.org>
> > ---
> >  drivers/rtc/rtc-m48t59.c | 31 +++++++++++++++----------------
> >  1 file changed, 15 insertions(+), 16 deletions(-)
> > 
> > diff --git a/drivers/rtc/rtc-m48t59.c b/drivers/rtc/rtc-m48t59.c
> > index f0f6b9b6daec..e2d882ea5c2f 100644
> > --- a/drivers/rtc/rtc-m48t59.c
> > +++ b/drivers/rtc/rtc-m48t59.c
> > @@ -57,6 +57,17 @@ m48t59_mem_readb(struct device *dev, u32 ofs)
> >  	return readb(m48t59->ioaddr+ofs);
> >  }
> >  
> > +/*
> > + * Sun SPARC machines count years since 1968. MVME machines running Linux
> > + * count years since 1970.
> > + */
> > +
> > +#ifdef CONFIG_SPARC
> > +#define YEAR0 68
> > +#else
> > +#define YEAR0 70
> > +#endif
> > +
> >  /*
> >   * NOTE: M48T59 only uses BCD mode
> >   */
> > @@ -82,10 +93,7 @@ static int m48t59_rtc_read_time(struct device *dev, struct rtc_time *tm)
> >  		dev_dbg(dev, "Century bit is enabled\n");
> >  		tm->tm_year += 100;	/* one century */
> >  	}
> > -#ifdef CONFIG_SPARC
> > -	/* Sun SPARC machines count years since 1968 */
> > -	tm->tm_year += 68;
> > -#endif
> > +	tm->tm_year += YEAR0;
> >  
> 
> I'm super happy to see someone working on this, while you are it, can 
> you use m48t59->rtc->start_secs and m48t59->rtc->set_start_time in probe 
> instead of offsetting tm_year in read_time/set_time so we can later use 
> device tree or any other mechanism to extend the range?
> 

Sure, I will look into it.

> It is super funny because I was telling Geert that I wanted to get rid 
> of this #ifdef CONFIG_SPARC two weeks ago at LPC. That could indeed then 
> come from platform data.
> 

I can't test any patches on SPARC, unless there's some way to do so using 
QEMU that would satisfy maintainers. (I rely on Daniel to test my patches 
on an MVME147 system.)
Re: [PATCH 1/2] rtc: m48t59: Accommodate chips that lack a century bit
Posted by Geert Uytterhoeven 1 month, 3 weeks ago 
Hi Finn,

On Thu, Oct 3, 2024 at 5:27 AM Finn Thain <fthain@linux-m68k.org> wrote:
> The m48t59 driver is needed by both SPARC and MVME systems. Linux on
> MVME uses 1970 as "year zero" rather than 1968 that's used on SPARC.
> Add support for the MVME convention. Otherwise, the RTC on non-SPARC
> systems can only read and write dates between 1900 and 1999.
>
> Tested-by: Daniel Palmer <daniel@0x0f.com>
> Signed-off-by: Finn Thain <fthain@linux-m68k.org>

Thanks for your patch!

> --- a/drivers/rtc/rtc-m48t59.c
> +++ b/drivers/rtc/rtc-m48t59.c
> @@ -57,6 +57,17 @@ m48t59_mem_readb(struct device *dev, u32 ofs)
>         return readb(m48t59->ioaddr+ofs);
>  }
>
> +/*
> + * Sun SPARC machines count years since 1968. MVME machines running Linux
> + * count years since 1970.
> + */
> +
> +#ifdef CONFIG_SPARC
> +#define YEAR0 68
> +#else
 +#define YEAR0 70
> +#endif

This causes a change in behavior on other non-SPARC platforms,
if any out-of-tree platform exists that uses this driver.

So I'd rather use:

    #elif defined(CONFIG_VME)
    #define YEAR0 70
    #else
    #define YEAR0 0
    #endif

> +
>  /*
>   * NOTE: M48T59 only uses BCD mode
>   */
> @@ -82,10 +93,7 @@ static int m48t59_rtc_read_time(struct device *dev, struct rtc_time *tm)
>                 dev_dbg(dev, "Century bit is enabled\n");
>                 tm->tm_year += 100;     /* one century */
>         }
> -#ifdef CONFIG_SPARC
> -       /* Sun SPARC machines count years since 1968 */
> -       tm->tm_year += 68;
> -#endif
> +       tm->tm_year += YEAR0;

Upon closer look, the driver uses platform data, so a better solution
would be to add the year0 offset to struct m48t59_plat_data.

Another suggestion for improvement, not related to this patch,
would be to differentiate among M48T59, M48T02, and M48T08 by using
platform_driver.id_table and platform_device_id.driver_data, instead
of m48t59_plat_data.type.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Re: [PATCH 1/2] rtc: m48t59: Accommodate chips that lack a century bit
Posted by Finn Thain 1 month, 3 weeks ago 
On Thu, 3 Oct 2024, Geert Uytterhoeven wrote:

> Thanks for your patch!
> 

Thanks for your review.

> > --- a/drivers/rtc/rtc-m48t59.c
> > +++ b/drivers/rtc/rtc-m48t59.c
> > @@ -57,6 +57,17 @@ m48t59_mem_readb(struct device *dev, u32 ofs)
> >         return readb(m48t59->ioaddr+ofs);
> >  }
> >
> > +/*
> > + * Sun SPARC machines count years since 1968. MVME machines running Linux
> > + * count years since 1970.
> > + */
> > +
> > +#ifdef CONFIG_SPARC
> > +#define YEAR0 68
> > +#else
>  +#define YEAR0 70
> > +#endif
> 
> This causes a change in behavior on other non-SPARC platforms,
> if any out-of-tree platform exists that uses this driver.
> 

I'm unaware of any need to support out-of-tree code. Do you see think such 
a requirement would be feasible somehow? Is this documented somewhere?

> So I'd rather use:
> 
>     #elif defined(CONFIG_VME)
>     #define YEAR0 70
>     #else
>     #define YEAR0 0
>     #endif
> 

That is a Y2K bug, right?

> > +
> >  /*
> >   * NOTE: M48T59 only uses BCD mode
> >   */
> > @@ -82,10 +93,7 @@ static int m48t59_rtc_read_time(struct device *dev, struct rtc_time *tm)
> >                 dev_dbg(dev, "Century bit is enabled\n");
> >                 tm->tm_year += 100;     /* one century */
> >         }
> > -#ifdef CONFIG_SPARC
> > -       /* Sun SPARC machines count years since 1968 */
> > -       tm->tm_year += 68;
> > -#endif
> > +       tm->tm_year += YEAR0;
> 
> Upon closer look, the driver uses platform data, so a better solution 
> would be to add the year0 offset to struct m48t59_plat_data.
> 

I agree.

> Another suggestion for improvement, not related to this patch, would be 
> to differentiate among M48T59, M48T02, and M48T08 by using 
> platform_driver.id_table and platform_device_id.driver_data, instead of 
> m48t59_plat_data.type.
> 

Yes, that's well out-of-scope I think.

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.