[PATCH 0/4] Add EPSON RX8035 RTC support

[PATCH 0/4] Add EPSON RX8035 RTC support

[Mathew McBride]

This patchset adds support for the EPSON RX8035 real time
clock to the rx8025 driver. The two devices are very similar
and with only minor differences for basic real time clock functions.

Some issues have been identified with the handling of date
setting and clock reset with the U-Boot driver which are resolved
with these patches.

It appears only one board (socrates) in the U-Boot tree uses this driver
at present, the other users either not in tree or removed under
previous cleanups, so take this opportunity to remove non_DM-RTC
support as well, and vestiges of previous board specific hacks.

The Traverse Technologies Ten64 board will be introduced in the 
near future and it will use this driver.

Mathew McBride (4):
  rtc: rx8025: drop non-DM support
  rtc: rx8025: add support for EPSON RX8035.
  rtc: rx8025: set date in a single i2c transaction
  rtc: rx8025: revise single register write to use offset

 drivers/rtc/Kconfig  |   1 +
 drivers/rtc/rx8025.c | 166 ++++++++++++++++---------------------------
 2 files changed, 63 insertions(+), 104 deletions(-)

-- 
2.30.1

[PATCH 1/4] rtc: rx8025: drop non-DM support

[Mathew McBride]

A search of the tree showed there is only one user
of this driver (soon to be two) - board/socrates

The second user will be the Traverse Ten64 board.

Both these boards have DM_RTC.

Signed-off-by: Mathew McBride <matt@traverse.com.au>
---
 drivers/rtc/Kconfig  |  1 +
 drivers/rtc/rx8025.c | 80 +++-----------------------------------------
 2 files changed, 6 insertions(+), 75 deletions(-)

diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index b6692e62df..71777cdd05 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -136,6 +136,7 @@ config RTC_RX8010SJ
 
 config RTC_RX8025
 	bool "Enable RX8025 driver"
+	depends on DM_RTC
 	help
 	  Support for Epson RX8025 Real Time Clock devices.
 
diff --git a/drivers/rtc/rx8025.c b/drivers/rtc/rx8025.c
index e717dcbbfe..36e5b0122c 100644
--- a/drivers/rtc/rx8025.c
+++ b/drivers/rtc/rx8025.c
@@ -24,22 +24,6 @@
 #endif
 /*---------------------------------------------------------------------*/
 
-#ifndef CONFIG_SYS_I2C_RTC_ADDR
-# define CONFIG_SYS_I2C_RTC_ADDR	0x32
-#endif
-
-#ifdef CONFIG_DM_RTC
-#define DEV_TYPE struct udevice
-#else
-/* Local udevice */
-struct ludevice {
-	u8 chip;
-};
-
-#define DEV_TYPE struct ludevice
-
-#endif
-
 /*
  * RTC register addresses
  */
@@ -74,39 +58,22 @@ struct ludevice {
  * address in a first cycle that is terminated by
  * a STOP condition. The chips needs a 'restart'
  * sequence (start sequence without a prior stop).
- * This driver has been written for a 4xx board.
- * U-Boot's 4xx i2c driver is currently not capable
- * to generate such cycles to some work arounds
- * are used.
  */
 
-/* static uchar rtc_read (uchar reg); */
-#ifdef CONFIG_DM_RTC
-/*
- * on mpc85xx based board with DM and offset len 1
- * accessing rtc works fine. May we can drop this ?
- */
 #define rtc_read(reg) buf[(reg) & 0xf]
-#else
-#define rtc_read(reg) buf[((reg) + 1) & 0xf]
-#endif
 
-static int rtc_write(DEV_TYPE *dev, uchar reg, uchar val);
+static int rtc_write(struct udevice *dev, uchar reg, uchar val);
 
 /*
  * Get the current time from the RTC
  */
-static int rx8025_rtc_get(DEV_TYPE *dev, struct rtc_time *tmp)
+static int rx8025_rtc_get(struct udevice *dev, struct rtc_time *tmp)
 {
 	int rel = 0;
 	uchar sec, min, hour, mday, wday, mon, year, ctl2;
 	uchar buf[16];
 
-#ifdef CONFIG_DM_RTC
 	if (dm_i2c_read(dev, 0, buf, sizeof(buf))) {
-#else
-	if (i2c_read(dev->chip, 0, 0, buf, 16)) {
-#endif
 		printf("Error reading from RTC\n");
 		return -EIO;
 	}
@@ -165,7 +132,7 @@ static int rx8025_rtc_get(DEV_TYPE *dev, struct rtc_time *tmp)
 /*
  * Set the RTC
  */
-static int rx8025_rtc_set(DEV_TYPE *dev, const struct rtc_time *tmp)
+static int rx8025_rtc_set(struct udevice *dev, const struct rtc_time *tmp)
 {
 	DEBUGR("Set DATE: %4d-%02d-%02d (wday=%d)  TIME: %2d:%02d:%02d\n",
 	       tmp->tm_year, tmp->tm_mon, tmp->tm_mday, tmp->tm_wday,
@@ -201,16 +168,12 @@ static int rx8025_rtc_set(DEV_TYPE *dev, const struct rtc_time *tmp)
 /*
  * Reset the RTC
  */
-static int rx8025_rtc_reset(DEV_TYPE *dev)
+static int rx8025_rtc_reset(struct udevice *dev)
 {
 	uchar buf[16];
 	uchar ctl2;
 
-#ifdef CONFIG_DM_RTC
 	if (dm_i2c_read(dev, 0, buf, sizeof(buf))) {
-#else
-	if (i2c_read(dev->chip, 0, 0, buf, 16)) {
-#endif
 		printf("Error reading from RTC\n");
 		return -EIO;
 	}
@@ -225,17 +188,13 @@ static int rx8025_rtc_reset(DEV_TYPE *dev)
 /*
  * Helper functions
  */
-static int rtc_write(DEV_TYPE *dev, uchar reg, uchar val)
+static int rtc_write(struct udevice *dev, uchar reg, uchar val)
 {
 	uchar buf[2];
 	buf[0] = reg << 4;
 	buf[1] = val;
 
-#ifdef CONFIG_DM_RTC
 	if (dm_i2c_write(dev, 0, buf, 2)) {
-#else
-	if (i2c_write(dev->chip, 0, 0, buf, 2) != 0) {
-#endif
 		printf("Error writing to RTC\n");
 		return -EIO;
 	}
@@ -243,7 +202,6 @@ static int rtc_write(DEV_TYPE *dev, uchar reg, uchar val)
 	return 0;
 }
 
-#ifdef CONFIG_DM_RTC
 static int rx8025_probe(struct udevice *dev)
 {
 	uchar buf[16];
@@ -276,31 +234,3 @@ U_BOOT_DRIVER(rx8010sj_rtc) = {
 	.of_match = rx8025_rtc_ids,
 	.ops	  = &rx8025_rtc_ops,
 };
-#else
-int rtc_get(struct rtc_time *tm)
-{
-	struct ludevice dev = {
-		.chip = CONFIG_SYS_I2C_RTC_ADDR,
-	};
-
-	return rx8025_rtc_get(&dev, tm);
-}
-
-int rtc_set(struct rtc_time *tm)
-{
-	struct ludevice dev = {
-		.chip = CONFIG_SYS_I2C_RTC_ADDR,
-	};
-
-	return rx8025_rtc_set(&dev, tm);
-}
-
-void rtc_reset(void)
-{
-	struct ludevice dev = {
-		.chip = CONFIG_SYS_I2C_RTC_ADDR,
-	};
-
-	rx8025_rtc_reset(&dev);
-}
-#endif
-- 
2.30.1

[PATCH 2/4] rtc: rx8025: add support for EPSON RX8035.

[Mathew McBride]

The RX8035 is a newer model from EPSON which is
very similar in operation to the RX8025.

The changes mirror similar ones that will be
in Linux 5.15:
https://lore.kernel.org/all/20210709044518.28769-2-matt@traverse.com.au/

The UBOOT_DRIVER ID has also been corrected, previously
it declared itself as rx8010sj_rtc which is a different driver.

Signed-off-by: Mathew McBride <matt@traverse.com.au>
---
 drivers/rtc/rx8025.c | 33 ++++++++++++++++++++++++++++-----
 1 file changed, 28 insertions(+), 5 deletions(-)

diff --git a/drivers/rtc/rx8025.c b/drivers/rtc/rx8025.c
index 36e5b0122c..09bf365f63 100644
--- a/drivers/rtc/rx8025.c
+++ b/drivers/rtc/rx8025.c
@@ -24,6 +24,11 @@
 #endif
 /*---------------------------------------------------------------------*/
 
+enum rx_model {
+	model_rx_8025,
+	model_rx_8035,
+};
+
 /*
  * RTC register addresses
  */
@@ -64,6 +69,20 @@
 
 static int rtc_write(struct udevice *dev, uchar reg, uchar val);
 
+static int rx8025_is_osc_stopped(enum rx_model model, int ctrl2)
+{
+	int xstp = ctrl2 & RTC_CTL2_BIT_XST;
+	/* XSTP bit has different polarity on RX-8025 vs RX-8035.
+	 * RX-8025: 0 == oscillator stopped
+	 * RX-8035: 1 == oscillator stopped
+	 */
+
+	if (model == model_rx_8025)
+		xstp = !xstp;
+
+	return xstp;
+}
+
 /*
  * Get the current time from the RTC
  */
@@ -101,8 +120,7 @@ static int rx8025_rtc_get(struct udevice *dev, struct rtc_time *tmp)
 		printf("RTC: voltage drop detected\n");
 		rel = -1;
 	}
-
-	if (!(ctl2 & RTC_CTL2_BIT_XST)) {
+	if (rx8025_is_osc_stopped(dev->driver_data, ctl2)) {
 		printf("RTC: oscillator stop detected\n");
 		rel = -1;
 	}
@@ -180,7 +198,11 @@ static int rx8025_rtc_reset(struct udevice *dev)
 
 	ctl2 = rtc_read(RTC_CTL2_REG_ADDR);
 	ctl2 &= ~(RTC_CTL2_BIT_PON | RTC_CTL2_BIT_VDET);
-	ctl2 |= RTC_CTL2_BIT_XST | RTC_CTL2_BIT_VDSL;
+
+	if (dev->driver_data == model_rx_8035)
+		ctl2 &= ~(RTC_CTL2_BIT_XST);
+	else
+		ctl2 |= RTC_CTL2_BIT_XST;
 
 	return rtc_write(dev, RTC_CTL2_REG_ADDR, ctl2);
 }
@@ -223,11 +245,12 @@ static const struct rtc_ops rx8025_rtc_ops = {
 };
 
 static const struct udevice_id rx8025_rtc_ids[] = {
-	{ .compatible = "epson,rx8025" },
+	{ .compatible = "epson,rx8025", .data = model_rx_8025 },
+	{ .compatible = "epson,rx8035", .data = model_rx_8035 },
 	{ }
 };
 
-U_BOOT_DRIVER(rx8010sj_rtc) = {
+U_BOOT_DRIVER(rx8025_rtc) = {
 	.name	  = "rx8025_rtc",
 	.id	      = UCLASS_RTC,
 	.probe    = rx8025_probe,
-- 
2.30.1

[PATCH 3/4] rtc: rx8025: set date in a single i2c transaction

[Mathew McBride]

The RX8025/RX8035 does not like having it's time registers
set byte-by-byte in separate I2C transactions.

From the note at the top of the file, it appears
target-dependent workarounds have been used in the
past for this.

Resolve this by setting the time registers in a single
I2C transaction.

As part of this, also ensure the '24/12' flag in the RTC
is reset before writing the date (instead of after), otherwise
the RX8035 will clear the seconds and minutes registers.

Tested on Traverse Ten64 (NXP LS1088A) with RX8035.

Signed-off-by: Mathew McBride <matt@traverse.com.au>
---
 drivers/rtc/rx8025.c | 44 +++++++++++++++++++++++---------------------
 1 file changed, 23 insertions(+), 21 deletions(-)

diff --git a/drivers/rtc/rx8025.c b/drivers/rtc/rx8025.c
index 09bf365f63..9423a1bb82 100644
--- a/drivers/rtc/rx8025.c
+++ b/drivers/rtc/rx8025.c
@@ -39,6 +39,7 @@ enum rx_model {
 #define RTC_DATE_REG_ADDR	0x04
 #define RTC_MON_REG_ADDR	0x05
 #define RTC_YR_REG_ADDR		0x06
+#define RTC_OFFSET_REG_ADDR	0x07
 
 #define RTC_CTL1_REG_ADDR	0x0e
 #define RTC_CTL2_REG_ADDR	0x0f
@@ -152,6 +153,19 @@ static int rx8025_rtc_get(struct udevice *dev, struct rtc_time *tmp)
  */
 static int rx8025_rtc_set(struct udevice *dev, const struct rtc_time *tmp)
 {
+	/* To work around the read/write cycle issue mentioned
+	 * at the top of this file, write all the time registers
+	 * in one I2C transaction
+	 */
+	u8 write_op[8];
+
+	/* 2412 flag must be set before doing a RTC write,
+	 * otherwise the seconds and minute register
+	 * will be cleared when the flag is set
+	 */
+	if (rtc_write(dev, RTC_CTL1_REG_ADDR, RTC_CTL1_BIT_2412))
+		return -EIO;
+
 	DEBUGR("Set DATE: %4d-%02d-%02d (wday=%d)  TIME: %2d:%02d:%02d\n",
 	       tmp->tm_year, tmp->tm_mon, tmp->tm_mday, tmp->tm_wday,
 	       tmp->tm_hour, tmp->tm_min, tmp->tm_sec);
@@ -159,28 +173,16 @@ static int rx8025_rtc_set(struct udevice *dev, const struct rtc_time *tmp)
 	if (tmp->tm_year < 1970 || tmp->tm_year > 2069)
 		printf("WARNING: year should be between 1970 and 2069!\n");
 
-	if (rtc_write(dev, RTC_YR_REG_ADDR, bin2bcd(tmp->tm_year % 100)))
-		return -EIO;
-
-	if (rtc_write(dev, RTC_MON_REG_ADDR, bin2bcd(tmp->tm_mon)))
-		return -EIO;
-
-	if (rtc_write(dev, RTC_DAY_REG_ADDR, bin2bcd(tmp->tm_wday)))
-		return -EIO;
-
-	if (rtc_write(dev, RTC_DATE_REG_ADDR, bin2bcd(tmp->tm_mday)))
-		return -EIO;
-
-	if (rtc_write(dev, RTC_HR_REG_ADDR, bin2bcd(tmp->tm_hour)))
-		return -EIO;
-
-	if (rtc_write(dev, RTC_MIN_REG_ADDR, bin2bcd(tmp->tm_min)))
-		return -EIO;
-
-	if (rtc_write(dev, RTC_SEC_REG_ADDR, bin2bcd(tmp->tm_sec)))
-		return -EIO;
+	write_op[RTC_SEC_REG_ADDR]  = bin2bcd(tmp->tm_sec);
+	write_op[RTC_MIN_REG_ADDR]  = bin2bcd(tmp->tm_min);
+	write_op[RTC_HR_REG_ADDR]   = bin2bcd(tmp->tm_hour);
+	write_op[RTC_DAY_REG_ADDR]	= bin2bcd(tmp->tm_wday);
+	write_op[RTC_DATE_REG_ADDR]	= bin2bcd(tmp->tm_mday);
+	write_op[RTC_MON_REG_ADDR]  = bin2bcd(tmp->tm_mon);
+	write_op[RTC_YR_REG_ADDR]	= bin2bcd(tmp->tm_year % 100);
+	write_op[RTC_OFFSET_REG_ADDR] = 0;
 
-	return rtc_write(dev, RTC_CTL1_REG_ADDR, RTC_CTL1_BIT_2412);
+	return dm_i2c_write(dev, 0, &write_op[0], 8);
 }
 
 /*
-- 
2.30.1

[PATCH 4/4] rtc: rx8025: revise single register write to use offset

[Mathew McBride]

Writing of individual registers was not functioning
correctly as a 0 'offset' byte under DM-managed
I2C was being appended in front of register we
wanted to access.

Signed-off-by: Mathew McBride <matt@traverse.com.au>
---
 drivers/rtc/rx8025.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/rtc/rx8025.c b/drivers/rtc/rx8025.c
index 9423a1bb82..1394c2306a 100644
--- a/drivers/rtc/rx8025.c
+++ b/drivers/rtc/rx8025.c
@@ -214,11 +214,14 @@ static int rx8025_rtc_reset(struct udevice *dev)
  */
 static int rtc_write(struct udevice *dev, uchar reg, uchar val)
 {
-	uchar buf[2];
-	buf[0] = reg << 4;
-	buf[1] = val;
+	/* The RX8025/RX8035 uses the top 4 bits of the
+	 * 'offset' byte as the start register address,
+	 * and the bottom 4 bits as a 'transfer' mode setting
+	 * (only applicable for reads)
+	 */
+	u8 offset = (reg << 4);
 
-	if (dm_i2c_write(dev, 0, buf, 2)) {
+	if (dm_i2c_reg_write(dev, offset, val)) {
 		printf("Error writing to RTC\n");
 		return -EIO;
 	}
-- 
2.30.1

Re: [PATCH 1/4] rtc: rx8025: drop non-DM support

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 375 bytes --]

On Fri, Sep 17, 2021 at 06:46:01AM +0000, Mathew McBride wrote:

> A search of the tree showed there is only one user
> of this driver (soon to be two) - board/socrates
> 
> The second user will be the Traverse Ten64 board.
> 
> Both these boards have DM_RTC.
> 
> Signed-off-by: Mathew McBride <matt@traverse.com.au>

Applied to u-boot/next, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 2/4] rtc: rx8025: add support for EPSON RX8035.

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 545 bytes --]

On Fri, Sep 17, 2021 at 06:46:02AM +0000, Mathew McBride wrote:

> The RX8035 is a newer model from EPSON which is
> very similar in operation to the RX8025.
> 
> The changes mirror similar ones that will be
> in Linux 5.15:
> https://lore.kernel.org/all/20210709044518.28769-2-matt@traverse.com.au/
> 
> The UBOOT_DRIVER ID has also been corrected, previously
> it declared itself as rx8010sj_rtc which is a different driver.
> 
> Signed-off-by: Mathew McBride <matt@traverse.com.au>

Applied to u-boot/next, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 3/4] rtc: rx8025: set date in a single i2c transaction

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 745 bytes --]

On Fri, Sep 17, 2021 at 06:46:03AM +0000, Mathew McBride wrote:

> The RX8025/RX8035 does not like having it's time registers
> set byte-by-byte in separate I2C transactions.
> 
> From the note at the top of the file, it appears
> target-dependent workarounds have been used in the
> past for this.
> 
> Resolve this by setting the time registers in a single
> I2C transaction.
> 
> As part of this, also ensure the '24/12' flag in the RTC
> is reset before writing the date (instead of after), otherwise
> the RX8035 will clear the seconds and minutes registers.
> 
> Tested on Traverse Ten64 (NXP LS1088A) with RX8035.
> 
> Signed-off-by: Mathew McBride <matt@traverse.com.au>

Applied to u-boot/next, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 4/4] rtc: rx8025: revise single register write to use offset

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 351 bytes --]

On Fri, Sep 17, 2021 at 06:46:04AM +0000, Mathew McBride wrote:

> Writing of individual registers was not functioning
> correctly as a 0 'offset' byte under DM-managed
> I2C was being appended in front of register we
> wanted to access.
> 
> Signed-off-by: Mathew McBride <matt@traverse.com.au>

Applied to u-boot/next, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]