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.
