[PATCH 1/6] rtc: rv8803: Kconfig: Indicate rx8900 support

[PATCH 1/6] rtc: rv8803: Kconfig: Indicate rx8900 support

[Benoît Thébaudeau]

This driver supports the Epson RX8900, but this was not indicated in
Kconfig.

Signed-off-by: Benoît Thébaudeau <benoit@wsystem.com>
---
 drivers/rtc/Kconfig | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 8526f1c..e215f50 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -578,10 +578,10 @@ config RTC_DRV_EM3027
 	  will be called rtc-em3027.
 
 config RTC_DRV_RV8803
-	tristate "Micro Crystal RV8803"
+	tristate "Micro Crystal RV8803, Epson RX8900"
 	help
-	  If you say yes here you get support for the Micro Crystal
-	  RV8803 RTC chips.
+	  If you say yes here you get support for the Micro Crystal RV8803 and
+	  Epson RX8900 RTC chips.
 
 	  This driver can also be built as a module. If so, the module
 	  will be called rtc-rv8803.
-- 
2.5.0

[rtc-linux] [PATCH 1/6] rtc: rv8803: Kconfig: Indicate rx8900 support

[Benoît Thébaudeau]

This driver supports the Epson RX8900, but this was not indicated in
Kconfig.

Signed-off-by: Beno=C3=AEt Th=C3=A9baudeau <benoit@wsystem.com>
---
 drivers/rtc/Kconfig | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 8526f1c..e215f50 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -578,10 +578,10 @@ config RTC_DRV_EM3027
 	  will be called rtc-em3027.
=20
 config RTC_DRV_RV8803
-	tristate "Micro Crystal RV8803"
+	tristate "Micro Crystal RV8803, Epson RX8900"
 	help
-	  If you say yes here you get support for the Micro Crystal
-	  RV8803 RTC chips.
+	  If you say yes here you get support for the Micro Crystal RV8803 and
+	  Epson RX8900 RTC chips.
=20
 	  This driver can also be built as a module. If so, the module
 	  will be called rtc-rv8803.
--=20
2.5.0

--=20
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
---=20
You received this message because you are subscribed to the Google Groups "=
rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an e=
mail to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[PATCH 2/6] rtc: rv8803: Remove the check for valid time

[Benoît Thébaudeau]

The RTC core always calls rtc_valid_tm() after ->read_time() in case of
success (in __rtc_read_time()), so do not call it twice.

Signed-off-by: Benoît Thébaudeau <benoit@wsystem.com>
---
 drivers/rtc/rtc-rv8803.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/rtc/rtc-rv8803.c b/drivers/rtc/rtc-rv8803.c
index 05c79c3..e740c83 100644
--- a/drivers/rtc/rtc-rv8803.c
+++ b/drivers/rtc/rtc-rv8803.c
@@ -150,7 +150,7 @@ static int rv8803_get_time(struct device *dev, struct rtc_time *tm)
 	tm->tm_mon  = bcd2bin(date[RV8803_MONTH] & 0x1f) - 1;
 	tm->tm_year = bcd2bin(date[RV8803_YEAR]) + 100;
 
-	return rtc_valid_tm(tm);
+	return 0;
 }
 
 static int rv8803_set_time(struct device *dev, struct rtc_time *tm)
-- 
2.5.0

[rtc-linux] [PATCH 2/6] rtc: rv8803: Remove the check for valid time

[Benoît Thébaudeau]

The RTC core always calls rtc_valid_tm() after ->read_time() in case of
success (in __rtc_read_time()), so do not call it twice.

Signed-off-by: Beno=C3=AEt Th=C3=A9baudeau <benoit@wsystem.com>
---
 drivers/rtc/rtc-rv8803.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/rtc/rtc-rv8803.c b/drivers/rtc/rtc-rv8803.c
index 05c79c3..e740c83 100644
--- a/drivers/rtc/rtc-rv8803.c
+++ b/drivers/rtc/rtc-rv8803.c
@@ -150,7 +150,7 @@ static int rv8803_get_time(struct device *dev, struct r=
tc_time *tm)
 	tm->tm_mon  =3D bcd2bin(date[RV8803_MONTH] & 0x1f) - 1;
 	tm->tm_year =3D bcd2bin(date[RV8803_YEAR]) + 100;
=20
-	return rtc_valid_tm(tm);
+	return 0;
 }
=20
 static int rv8803_set_time(struct device *dev, struct rtc_time *tm)
--=20
2.5.0

--=20
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
---=20
You received this message because you are subscribed to the Google Groups "=
rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an e=
mail to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[PATCH 3/6] rtc: rv8803: Fix read day of week

[Benoît Thébaudeau]

The Weekday register is encoded as 2^tm_wday, with tm_wday in 0..6, so
using tm_wday = ffs(reg) to fill tm_wday from the register value is
wrong because this gives the expected value + 1. This could be fixed as
tm_wday = ffs(reg) - 1, but tm_wday = ilog2(reg) works as well and is
more direct.

Signed-off-by: Benoît Thébaudeau <benoit@wsystem.com>
---
 drivers/rtc/rtc-rv8803.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/rtc/rtc-rv8803.c b/drivers/rtc/rtc-rv8803.c
index e740c83..aa1d6b6 100644
--- a/drivers/rtc/rtc-rv8803.c
+++ b/drivers/rtc/rtc-rv8803.c
@@ -13,6 +13,7 @@
 
 #include <linux/bcd.h>
 #include <linux/bitops.h>
+#include <linux/log2.h>
 #include <linux/i2c.h>
 #include <linux/interrupt.h>
 #include <linux/kernel.h>
@@ -145,7 +146,7 @@ static int rv8803_get_time(struct device *dev, struct rtc_time *tm)
 	tm->tm_sec  = bcd2bin(date[RV8803_SEC] & 0x7f);
 	tm->tm_min  = bcd2bin(date[RV8803_MIN] & 0x7f);
 	tm->tm_hour = bcd2bin(date[RV8803_HOUR] & 0x3f);
-	tm->tm_wday = ffs(date[RV8803_WEEK] & 0x7f);
+	tm->tm_wday = ilog2(date[RV8803_WEEK] & 0x7f);
 	tm->tm_mday = bcd2bin(date[RV8803_DAY] & 0x3f);
 	tm->tm_mon  = bcd2bin(date[RV8803_MONTH] & 0x1f) - 1;
 	tm->tm_year = bcd2bin(date[RV8803_YEAR]) + 100;
-- 
2.5.0

[rtc-linux] [PATCH 3/6] rtc: rv8803: Fix read day of week

[Benoît Thébaudeau]

The Weekday register is encoded as 2^tm_wday, with tm_wday in 0..6, so
using tm_wday =3D ffs(reg) to fill tm_wday from the register value is
wrong because this gives the expected value + 1. This could be fixed as
tm_wday =3D ffs(reg) - 1, but tm_wday =3D ilog2(reg) works as well and is
more direct.

Signed-off-by: Beno=C3=AEt Th=C3=A9baudeau <benoit@wsystem.com>
---
 drivers/rtc/rtc-rv8803.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/rtc/rtc-rv8803.c b/drivers/rtc/rtc-rv8803.c
index e740c83..aa1d6b6 100644
--- a/drivers/rtc/rtc-rv8803.c
+++ b/drivers/rtc/rtc-rv8803.c
@@ -13,6 +13,7 @@
=20
 #include <linux/bcd.h>
 #include <linux/bitops.h>
+#include <linux/log2.h>
 #include <linux/i2c.h>
 #include <linux/interrupt.h>
 #include <linux/kernel.h>
@@ -145,7 +146,7 @@ static int rv8803_get_time(struct device *dev, struct r=
tc_time *tm)
 	tm->tm_sec  =3D bcd2bin(date[RV8803_SEC] & 0x7f);
 	tm->tm_min  =3D bcd2bin(date[RV8803_MIN] & 0x7f);
 	tm->tm_hour =3D bcd2bin(date[RV8803_HOUR] & 0x3f);
-	tm->tm_wday =3D ffs(date[RV8803_WEEK] & 0x7f);
+	tm->tm_wday =3D ilog2(date[RV8803_WEEK] & 0x7f);
 	tm->tm_mday =3D bcd2bin(date[RV8803_DAY] & 0x3f);
 	tm->tm_mon  =3D bcd2bin(date[RV8803_MONTH] & 0x1f) - 1;
 	tm->tm_year =3D bcd2bin(date[RV8803_YEAR]) + 100;
--=20
2.5.0

--=20
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
---=20
You received this message because you are subscribed to the Google Groups "=
rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an e=
mail to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[PATCH 4/6] rtc: rv8803: Always apply the I²C workaround

[Benoît Thébaudeau]

The I²C NACK issue of the RV-8803 may occur after any I²C START
condition, depending on the timings. Consequently, the workaround must
be applied for all the I²C transfers.

This commit abstracts the I²C transfer code into register access
functions. This avoids duplicating the I²C workaround everywhere. This
also avoids the duplication of the code handling the return value of
i2c_smbus_read_i2c_block_data(). Error messages are issued in case of
definitive register access failures (if the workaround fails). This
change also makes the I²C transfer return value checks consistent.

Signed-off-by: Benoît Thébaudeau <benoit@wsystem.com>
---
 drivers/rtc/rtc-rv8803.c | 179 ++++++++++++++++++++++++++++++-----------------
 1 file changed, 113 insertions(+), 66 deletions(-)

diff --git a/drivers/rtc/rtc-rv8803.c b/drivers/rtc/rtc-rv8803.c
index aa1d6b6..09ab5cb 100644
--- a/drivers/rtc/rtc-rv8803.c
+++ b/drivers/rtc/rtc-rv8803.c
@@ -20,6 +20,8 @@
 #include <linux/module.h>
 #include <linux/rtc.h>
 
+#define RV8803_I2C_TRY_COUNT		4
+
 #define RV8803_SEC			0x00
 #define RV8803_MIN			0x01
 #define RV8803_HOUR			0x02
@@ -57,19 +59,85 @@ struct rv8803_data {
 	u8 ctrl;
 };
 
+static int rv8803_read_reg(const struct i2c_client *client, u8 reg)
+{
+	int try = RV8803_I2C_TRY_COUNT;
+	s32 ret;
+
+	/*
+	 * There is a 61µs window during which the RTC does not acknowledge I2C
+	 * transfers. In that case, ensure that there are multiple attempts.
+	 */
+	do
+		ret = i2c_smbus_read_byte_data(client, reg);
+	while ((ret == -ENXIO || ret == -EIO) && --try);
+	if (ret < 0)
+		dev_err(&client->dev, "Unable to read register 0x%02x\n", reg);
+
+	return ret;
+}
+
+static int rv8803_read_regs(const struct i2c_client *client,
+			    u8 reg, u8 count, u8 *values)
+{
+	int try = RV8803_I2C_TRY_COUNT;
+	s32 ret;
+
+	do
+		ret = i2c_smbus_read_i2c_block_data(client, reg, count, values);
+	while ((ret == -ENXIO || ret == -EIO) && --try);
+	if (ret != count) {
+		dev_err(&client->dev,
+			"Unable to read registers 0x%02x..0x%02x\n",
+			reg, reg + count - 1);
+		return ret < 0 ? ret : -EIO;
+	}
+
+	return 0;
+}
+
+static int rv8803_write_reg(const struct i2c_client *client, u8 reg, u8 value)
+{
+	int try = RV8803_I2C_TRY_COUNT;
+	s32 ret;
+
+	do
+		ret = i2c_smbus_write_byte_data(client, reg, value);
+	while ((ret == -ENXIO || ret == -EIO) && --try);
+	if (ret)
+		dev_err(&client->dev, "Unable to write register 0x%02x\n", reg);
+
+	return ret;
+}
+
+static int rv8803_write_regs(const struct i2c_client *client,
+			     u8 reg, u8 count, const u8 *values)
+{
+	int try = RV8803_I2C_TRY_COUNT;
+	s32 ret;
+
+	do
+		ret = i2c_smbus_write_i2c_block_data(client, reg, count,
+						     values);
+	while ((ret == -ENXIO || ret == -EIO) && --try);
+	if (ret)
+		dev_err(&client->dev,
+			"Unable to write registers 0x%02x..0x%02x\n",
+			reg, reg + count - 1);
+
+	return ret;
+}
+
 static irqreturn_t rv8803_handle_irq(int irq, void *dev_id)
 {
 	struct i2c_client *client = dev_id;
 	struct rv8803_data *rv8803 = i2c_get_clientdata(client);
 	unsigned long events = 0;
-	int flags, try = 0;
+	int flags;
 
 	mutex_lock(&rv8803->flags_lock);
 
-	do {
-		flags = i2c_smbus_read_byte_data(client, RV8803_FLAG);
-		try++;
-	} while (((flags == -ENXIO) || (flags == -EIO)) && (try < 4));
+	flags = rv8803_read_reg(client, RV8803_FLAG);
 	if (flags <= 0) {
 		mutex_unlock(&rv8803->flags_lock);
 		return IRQ_NONE;
@@ -101,9 +169,8 @@ static irqreturn_t rv8803_handle_irq(int irq, void *dev_id)
 
 	if (events) {
 		rtc_update_irq(rv8803->rtc, 1, events);
-		i2c_smbus_write_byte_data(client, RV8803_FLAG, flags);
-		i2c_smbus_write_byte_data(rv8803->client, RV8803_CTRL,
-					  rv8803->ctrl);
+		rv8803_write_reg(client, RV8803_FLAG, flags);
+		rv8803_write_reg(rv8803->client, RV8803_CTRL, rv8803->ctrl);
 	}
 
 	mutex_unlock(&rv8803->flags_lock);
@@ -119,7 +186,7 @@ static int rv8803_get_time(struct device *dev, struct rtc_time *tm)
 	u8 *date = date1;
 	int ret, flags;
 
-	flags = i2c_smbus_read_byte_data(rv8803->client, RV8803_FLAG);
+	flags = rv8803_read_reg(rv8803->client, RV8803_FLAG);
 	if (flags < 0)
 		return flags;
 
@@ -128,16 +195,14 @@ static int rv8803_get_time(struct device *dev, struct rtc_time *tm)
 		return -EINVAL;
 	}
 
-	ret = i2c_smbus_read_i2c_block_data(rv8803->client, RV8803_SEC,
-					    7, date);
-	if (ret != 7)
-		return ret < 0 ? ret : -EIO;
+	ret = rv8803_read_regs(rv8803->client, RV8803_SEC, 7, date);
+	if (ret)
+		return ret;
 
 	if ((date1[RV8803_SEC] & 0x7f) == bin2bcd(59)) {
-		ret = i2c_smbus_read_i2c_block_data(rv8803->client, RV8803_SEC,
-						    7, date2);
-		if (ret != 7)
-			return ret < 0 ? ret : -EIO;
+		ret = rv8803_read_regs(rv8803->client, RV8803_SEC, 7, date2);
+		if (ret)
+			return ret;
 
 		if ((date2[RV8803_SEC] & 0x7f) != bin2bcd(59))
 			date = date2;
@@ -171,21 +236,20 @@ static int rv8803_set_time(struct device *dev, struct rtc_time *tm)
 	date[RV8803_MONTH] = bin2bcd(tm->tm_mon + 1);
 	date[RV8803_YEAR]  = bin2bcd(tm->tm_year - 100);
 
-	ret = i2c_smbus_write_i2c_block_data(rv8803->client, RV8803_SEC,
-					     7, date);
-	if (ret < 0)
+	ret = rv8803_write_regs(rv8803->client, RV8803_SEC, 7, date);
+	if (ret)
 		return ret;
 
 	mutex_lock(&rv8803->flags_lock);
 
-	flags = i2c_smbus_read_byte_data(rv8803->client, RV8803_FLAG);
+	flags = rv8803_read_reg(rv8803->client, RV8803_FLAG);
 	if (flags < 0) {
 		mutex_unlock(&rv8803->flags_lock);
 		return flags;
 	}
 
-	ret = i2c_smbus_write_byte_data(rv8803->client, RV8803_FLAG,
-					flags & ~RV8803_FLAG_V2F);
+	ret = rv8803_write_reg(rv8803->client, RV8803_FLAG,
+			       flags & ~RV8803_FLAG_V2F);
 
 	mutex_unlock(&rv8803->flags_lock);
 
@@ -199,12 +263,11 @@ static int rv8803_get_alarm(struct device *dev, struct rtc_wkalrm *alrm)
 	u8 alarmvals[3];
 	int flags, ret;
 
-	ret = i2c_smbus_read_i2c_block_data(client, RV8803_ALARM_MIN,
-					    3, alarmvals);
-	if (ret != 3)
-		return ret < 0 ? ret : -EIO;
+	ret = rv8803_read_regs(client, RV8803_ALARM_MIN, 3, alarmvals);
+	if (ret)
+		return ret;
 
-	flags = i2c_smbus_read_byte_data(client, RV8803_FLAG);
+	flags = rv8803_read_reg(client, RV8803_FLAG);
 	if (flags < 0)
 		return flags;
 
@@ -237,10 +300,10 @@ static int rv8803_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
 
 	mutex_lock(&rv8803->flags_lock);
 
-	ret = i2c_smbus_read_i2c_block_data(client, RV8803_FLAG, 2, ctrl);
-	if (ret != 2) {
+	ret = rv8803_read_regs(client, RV8803_FLAG, 2, ctrl);
+	if (ret) {
 		mutex_unlock(&rv8803->flags_lock);
-		return ret < 0 ? ret : -EIO;
+		return ret;
 	}
 
 	alarmvals[0] = bin2bcd(alrm->time.tm_min);
@@ -249,8 +312,8 @@ static int rv8803_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
 
 	if (rv8803->ctrl & (RV8803_CTRL_AIE | RV8803_CTRL_UIE)) {
 		rv8803->ctrl &= ~(RV8803_CTRL_AIE | RV8803_CTRL_UIE);
-		err = i2c_smbus_write_byte_data(rv8803->client, RV8803_CTRL,
-						rv8803->ctrl);
+		err = rv8803_write_reg(rv8803->client, RV8803_CTRL,
+				       rv8803->ctrl);
 		if (err) {
 			mutex_unlock(&rv8803->flags_lock);
 			return err;
@@ -258,13 +321,12 @@ static int rv8803_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
 	}
 
 	ctrl[1] &= ~RV8803_FLAG_AF;
-	err = i2c_smbus_write_byte_data(rv8803->client, RV8803_FLAG, ctrl[1]);
+	err = rv8803_write_reg(rv8803->client, RV8803_FLAG, ctrl[1]);
 	mutex_unlock(&rv8803->flags_lock);
 	if (err)
 		return err;
 
-	err = i2c_smbus_write_i2c_block_data(rv8803->client, RV8803_ALARM_MIN,
-					     3, alarmvals);
+	err = rv8803_write_regs(rv8803->client, RV8803_ALARM_MIN, 3, alarmvals);
 	if (err)
 		return err;
 
@@ -274,8 +336,8 @@ static int rv8803_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
 		if (rv8803->rtc->aie_timer.enabled)
 			rv8803->ctrl |= RV8803_CTRL_AIE;
 
-		err = i2c_smbus_write_byte_data(rv8803->client, RV8803_CTRL,
-						rv8803->ctrl);
+		err = rv8803_write_reg(rv8803->client, RV8803_CTRL,
+				       rv8803->ctrl);
 		if (err)
 			return err;
 	}
@@ -304,21 +366,20 @@ static int rv8803_alarm_irq_enable(struct device *dev, unsigned int enabled)
 	}
 
 	mutex_lock(&rv8803->flags_lock);
-	flags = i2c_smbus_read_byte_data(client, RV8803_FLAG);
+	flags = rv8803_read_reg(client, RV8803_FLAG);
 	if (flags < 0) {
 		mutex_unlock(&rv8803->flags_lock);
 		return flags;
 	}
 	flags &= ~(RV8803_FLAG_AF | RV8803_FLAG_UF);
-	err = i2c_smbus_write_byte_data(client, RV8803_FLAG, flags);
+	err = rv8803_write_reg(client, RV8803_FLAG, flags);
 	mutex_unlock(&rv8803->flags_lock);
 	if (err)
 		return err;
 
 	if (ctrl != rv8803->ctrl) {
 		rv8803->ctrl = ctrl;
-		err = i2c_smbus_write_byte_data(client, RV8803_CTRL,
-						rv8803->ctrl);
+		err = rv8803_write_reg(client, RV8803_CTRL, rv8803->ctrl);
 		if (err)
 			return err;
 	}
@@ -334,7 +395,7 @@ static int rv8803_ioctl(struct device *dev, unsigned int cmd, unsigned long arg)
 
 	switch (cmd) {
 	case RTC_VL_READ:
-		flags = i2c_smbus_read_byte_data(client, RV8803_FLAG);
+		flags = rv8803_read_reg(client, RV8803_FLAG);
 		if (flags < 0)
 			return flags;
 
@@ -353,16 +414,16 @@ static int rv8803_ioctl(struct device *dev, unsigned int cmd, unsigned long arg)
 
 	case RTC_VL_CLR:
 		mutex_lock(&rv8803->flags_lock);
-		flags = i2c_smbus_read_byte_data(client, RV8803_FLAG);
+		flags = rv8803_read_reg(client, RV8803_FLAG);
 		if (flags < 0) {
 			mutex_unlock(&rv8803->flags_lock);
 			return flags;
 		}
 
 		flags &= ~(RV8803_FLAG_V1F | RV8803_FLAG_V2F);
-		ret = i2c_smbus_write_byte_data(client, RV8803_FLAG, flags);
+		ret = rv8803_write_reg(client, RV8803_FLAG, flags);
 		mutex_unlock(&rv8803->flags_lock);
-		if (ret < 0)
+		if (ret)
 			return ret;
 
 		return 0;
@@ -380,8 +441,8 @@ static ssize_t rv8803_nvram_write(struct file *filp, struct kobject *kobj,
 	struct i2c_client *client = to_i2c_client(dev);
 	int ret;
 
-	ret = i2c_smbus_write_byte_data(client, RV8803_RAM, buf[0]);
-	if (ret < 0)
+	ret = rv8803_write_reg(client, RV8803_RAM, buf[0]);
+	if (ret)
 		return ret;
 
 	return 1;
@@ -395,7 +456,7 @@ static ssize_t rv8803_nvram_read(struct file *filp, struct kobject *kobj,
 	struct i2c_client *client = to_i2c_client(dev);
 	int ret;
 
-	ret = i2c_smbus_read_byte_data(client, RV8803_RAM);
+	ret = rv8803_read_reg(client, RV8803_RAM);
 	if (ret < 0)
 		return ret;
 
@@ -425,7 +486,7 @@ static int rv8803_probe(struct i2c_client *client,
 {
 	struct i2c_adapter *adapter = to_i2c_adapter(client->dev.parent);
 	struct rv8803_data *rv8803;
-	int err, flags, try = 0;
+	int err, flags;
 
 	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA |
 				     I2C_FUNC_SMBUS_I2C_BLOCK)) {
@@ -442,16 +503,7 @@ static int rv8803_probe(struct i2c_client *client,
 	rv8803->client = client;
 	i2c_set_clientdata(client, rv8803);
 
-	/*
-	 * There is a 60µs window where the RTC may not reply on the i2c bus in
-	 * that case, the transfer is not ACKed. In that case, ensure there are
-	 * multiple attempts.
-	 */
-	do {
-		flags = i2c_smbus_read_byte_data(client, RV8803_FLAG);
-		try++;
-	} while (((flags == -ENXIO) || (flags == -EIO)) && (try < 4));
-
+	flags = rv8803_read_reg(client, RV8803_FLAG);
 	if (flags < 0)
 		return flags;
 
@@ -486,12 +538,7 @@ static int rv8803_probe(struct i2c_client *client,
 		return PTR_ERR(rv8803->rtc);
 	}
 
-	try = 0;
-	do {
-		err = i2c_smbus_write_byte_data(rv8803->client, RV8803_EXT,
-						RV8803_EXT_WADA);
-		try++;
-	} while (((err == -ENXIO) || (flags == -EIO)) && (try < 4));
+	err = rv8803_write_reg(rv8803->client, RV8803_EXT, RV8803_EXT_WADA);
 	if (err)
 		return err;
 
-- 
2.5.0

[rtc-linux] [PATCH 4/6] rtc: rv8803: Always apply the I²C workaround

[Benoît Thébaudeau]

The I=C2=B2C NACK issue of the RV-8803 may occur after any I=C2=B2C START
condition, depending on the timings. Consequently, the workaround must
be applied for all the I=C2=B2C transfers.

This commit abstracts the I=C2=B2C transfer code into register access
functions. This avoids duplicating the I=C2=B2C workaround everywhere. This
also avoids the duplication of the code handling the return value of
i2c_smbus_read_i2c_block_data(). Error messages are issued in case of
definitive register access failures (if the workaround fails). This
change also makes the I=C2=B2C transfer return value checks consistent.

Signed-off-by: Beno=C3=AEt Th=C3=A9baudeau <benoit@wsystem.com>
---
 drivers/rtc/rtc-rv8803.c | 179 ++++++++++++++++++++++++++++++-------------=
----
 1 file changed, 113 insertions(+), 66 deletions(-)

diff --git a/drivers/rtc/rtc-rv8803.c b/drivers/rtc/rtc-rv8803.c
index aa1d6b6..09ab5cb 100644
--- a/drivers/rtc/rtc-rv8803.c
+++ b/drivers/rtc/rtc-rv8803.c
@@ -20,6 +20,8 @@
 #include <linux/module.h>
 #include <linux/rtc.h>
=20
+#define RV8803_I2C_TRY_COUNT		4
+
 #define RV8803_SEC			0x00
 #define RV8803_MIN			0x01
 #define RV8803_HOUR			0x02
@@ -57,19 +59,85 @@ struct rv8803_data {
 	u8 ctrl;
 };
=20
+static int rv8803_read_reg(const struct i2c_client *client, u8 reg)
+{
+	int try =3D RV8803_I2C_TRY_COUNT;
+	s32 ret;
+
+	/*
+	 * There is a 61=C2=B5s window during which the RTC does not acknowledge =
I2C
+	 * transfers. In that case, ensure that there are multiple attempts.
+	 */
+	do
+		ret =3D i2c_smbus_read_byte_data(client, reg);
+	while ((ret =3D=3D -ENXIO || ret =3D=3D -EIO) && --try);
+	if (ret < 0)
+		dev_err(&client->dev, "Unable to read register 0x%02x\n", reg);
+
+	return ret;
+}
+
+static int rv8803_read_regs(const struct i2c_client *client,
+			    u8 reg, u8 count, u8 *values)
+{
+	int try =3D RV8803_I2C_TRY_COUNT;
+	s32 ret;
+
+	do
+		ret =3D i2c_smbus_read_i2c_block_data(client, reg, count, values);
+	while ((ret =3D=3D -ENXIO || ret =3D=3D -EIO) && --try);
+	if (ret !=3D count) {
+		dev_err(&client->dev,
+			"Unable to read registers 0x%02x..0x%02x\n",
+			reg, reg + count - 1);
+		return ret < 0 ? ret : -EIO;
+	}
+
+	return 0;
+}
+
+static int rv8803_write_reg(const struct i2c_client *client, u8 reg, u8 va=
lue)
+{
+	int try =3D RV8803_I2C_TRY_COUNT;
+	s32 ret;
+
+	do
+		ret =3D i2c_smbus_write_byte_data(client, reg, value);
+	while ((ret =3D=3D -ENXIO || ret =3D=3D -EIO) && --try);
+	if (ret)
+		dev_err(&client->dev, "Unable to write register 0x%02x\n", reg);
+
+	return ret;
+}
+
+static int rv8803_write_regs(const struct i2c_client *client,
+			     u8 reg, u8 count, const u8 *values)
+{
+	int try =3D RV8803_I2C_TRY_COUNT;
+	s32 ret;
+
+	do
+		ret =3D i2c_smbus_write_i2c_block_data(client, reg, count,
+						     values);
+	while ((ret =3D=3D -ENXIO || ret =3D=3D -EIO) && --try);
+	if (ret)
+		dev_err(&client->dev,
+			"Unable to write registers 0x%02x..0x%02x\n",
+			reg, reg + count - 1);
+
+	return ret;
+}
+
 static irqreturn_t rv8803_handle_irq(int irq, void *dev_id)
 {
 	struct i2c_client *client =3D dev_id;
 	struct rv8803_data *rv8803 =3D i2c_get_clientdata(client);
 	unsigned long events =3D 0;
-	int flags, try =3D 0;
+	int flags;
=20
 	mutex_lock(&rv8803->flags_lock);
=20
-	do {
-		flags =3D i2c_smbus_read_byte_data(client, RV8803_FLAG);
-		try++;
-	} while (((flags =3D=3D -ENXIO) || (flags =3D=3D -EIO)) && (try < 4));
+	flags =3D rv8803_read_reg(client, RV8803_FLAG);
 	if (flags <=3D 0) {
 		mutex_unlock(&rv8803->flags_lock);
 		return IRQ_NONE;
@@ -101,9 +169,8 @@ static irqreturn_t rv8803_handle_irq(int irq, void *dev=
_id)
=20
 	if (events) {
 		rtc_update_irq(rv8803->rtc, 1, events);
-		i2c_smbus_write_byte_data(client, RV8803_FLAG, flags);
-		i2c_smbus_write_byte_data(rv8803->client, RV8803_CTRL,
-					  rv8803->ctrl);
+		rv8803_write_reg(client, RV8803_FLAG, flags);
+		rv8803_write_reg(rv8803->client, RV8803_CTRL, rv8803->ctrl);
 	}
=20
 	mutex_unlock(&rv8803->flags_lock);
@@ -119,7 +186,7 @@ static int rv8803_get_time(struct device *dev, struct r=
tc_time *tm)
 	u8 *date =3D date1;
 	int ret, flags;
=20
-	flags =3D i2c_smbus_read_byte_data(rv8803->client, RV8803_FLAG);
+	flags =3D rv8803_read_reg(rv8803->client, RV8803_FLAG);
 	if (flags < 0)
 		return flags;
=20
@@ -128,16 +195,14 @@ static int rv8803_get_time(struct device *dev, struct=
 rtc_time *tm)
 		return -EINVAL;
 	}
=20
-	ret =3D i2c_smbus_read_i2c_block_data(rv8803->client, RV8803_SEC,
-					    7, date);
-	if (ret !=3D 7)
-		return ret < 0 ? ret : -EIO;
+	ret =3D rv8803_read_regs(rv8803->client, RV8803_SEC, 7, date);
+	if (ret)
+		return ret;
=20
 	if ((date1[RV8803_SEC] & 0x7f) =3D=3D bin2bcd(59)) {
-		ret =3D i2c_smbus_read_i2c_block_data(rv8803->client, RV8803_SEC,
-						    7, date2);
-		if (ret !=3D 7)
-			return ret < 0 ? ret : -EIO;
+		ret =3D rv8803_read_regs(rv8803->client, RV8803_SEC, 7, date2);
+		if (ret)
+			return ret;
=20
 		if ((date2[RV8803_SEC] & 0x7f) !=3D bin2bcd(59))
 			date =3D date2;
@@ -171,21 +236,20 @@ static int rv8803_set_time(struct device *dev, struct=
 rtc_time *tm)
 	date[RV8803_MONTH] =3D bin2bcd(tm->tm_mon + 1);
 	date[RV8803_YEAR]  =3D bin2bcd(tm->tm_year - 100);
=20
-	ret =3D i2c_smbus_write_i2c_block_data(rv8803->client, RV8803_SEC,
-					     7, date);
-	if (ret < 0)
+	ret =3D rv8803_write_regs(rv8803->client, RV8803_SEC, 7, date);
+	if (ret)
 		return ret;
=20
 	mutex_lock(&rv8803->flags_lock);
=20
-	flags =3D i2c_smbus_read_byte_data(rv8803->client, RV8803_FLAG);
+	flags =3D rv8803_read_reg(rv8803->client, RV8803_FLAG);
 	if (flags < 0) {
 		mutex_unlock(&rv8803->flags_lock);
 		return flags;
 	}
=20
-	ret =3D i2c_smbus_write_byte_data(rv8803->client, RV8803_FLAG,
-					flags & ~RV8803_FLAG_V2F);
+	ret =3D rv8803_write_reg(rv8803->client, RV8803_FLAG,
+			       flags & ~RV8803_FLAG_V2F);
=20
 	mutex_unlock(&rv8803->flags_lock);
=20
@@ -199,12 +263,11 @@ static int rv8803_get_alarm(struct device *dev, struc=
t rtc_wkalrm *alrm)
 	u8 alarmvals[3];
 	int flags, ret;
=20
-	ret =3D i2c_smbus_read_i2c_block_data(client, RV8803_ALARM_MIN,
-					    3, alarmvals);
-	if (ret !=3D 3)
-		return ret < 0 ? ret : -EIO;
+	ret =3D rv8803_read_regs(client, RV8803_ALARM_MIN, 3, alarmvals);
+	if (ret)
+		return ret;
=20
-	flags =3D i2c_smbus_read_byte_data(client, RV8803_FLAG);
+	flags =3D rv8803_read_reg(client, RV8803_FLAG);
 	if (flags < 0)
 		return flags;
=20
@@ -237,10 +300,10 @@ static int rv8803_set_alarm(struct device *dev, struc=
t rtc_wkalrm *alrm)
=20
 	mutex_lock(&rv8803->flags_lock);
=20
-	ret =3D i2c_smbus_read_i2c_block_data(client, RV8803_FLAG, 2, ctrl);
-	if (ret !=3D 2) {
+	ret =3D rv8803_read_regs(client, RV8803_FLAG, 2, ctrl);
+	if (ret) {
 		mutex_unlock(&rv8803->flags_lock);
-		return ret < 0 ? ret : -EIO;
+		return ret;
 	}
=20
 	alarmvals[0] =3D bin2bcd(alrm->time.tm_min);
@@ -249,8 +312,8 @@ static int rv8803_set_alarm(struct device *dev, struct =
rtc_wkalrm *alrm)
=20
 	if (rv8803->ctrl & (RV8803_CTRL_AIE | RV8803_CTRL_UIE)) {
 		rv8803->ctrl &=3D ~(RV8803_CTRL_AIE | RV8803_CTRL_UIE);
-		err =3D i2c_smbus_write_byte_data(rv8803->client, RV8803_CTRL,
-						rv8803->ctrl);
+		err =3D rv8803_write_reg(rv8803->client, RV8803_CTRL,
+				       rv8803->ctrl);
 		if (err) {
 			mutex_unlock(&rv8803->flags_lock);
 			return err;
@@ -258,13 +321,12 @@ static int rv8803_set_alarm(struct device *dev, struc=
t rtc_wkalrm *alrm)
 	}
=20
 	ctrl[1] &=3D ~RV8803_FLAG_AF;
-	err =3D i2c_smbus_write_byte_data(rv8803->client, RV8803_FLAG, ctrl[1]);
+	err =3D rv8803_write_reg(rv8803->client, RV8803_FLAG, ctrl[1]);
 	mutex_unlock(&rv8803->flags_lock);
 	if (err)
 		return err;
=20
-	err =3D i2c_smbus_write_i2c_block_data(rv8803->client, RV8803_ALARM_MIN,
-					     3, alarmvals);
+	err =3D rv8803_write_regs(rv8803->client, RV8803_ALARM_MIN, 3, alarmvals)=
;
 	if (err)
 		return err;
=20
@@ -274,8 +336,8 @@ static int rv8803_set_alarm(struct device *dev, struct =
rtc_wkalrm *alrm)
 		if (rv8803->rtc->aie_timer.enabled)
 			rv8803->ctrl |=3D RV8803_CTRL_AIE;
=20
-		err =3D i2c_smbus_write_byte_data(rv8803->client, RV8803_CTRL,
-						rv8803->ctrl);
+		err =3D rv8803_write_reg(rv8803->client, RV8803_CTRL,
+				       rv8803->ctrl);
 		if (err)
 			return err;
 	}
@@ -304,21 +366,20 @@ static int rv8803_alarm_irq_enable(struct device *dev=
, unsigned int enabled)
 	}
=20
 	mutex_lock(&rv8803->flags_lock);
-	flags =3D i2c_smbus_read_byte_data(client, RV8803_FLAG);
+	flags =3D rv8803_read_reg(client, RV8803_FLAG);
 	if (flags < 0) {
 		mutex_unlock(&rv8803->flags_lock);
 		return flags;
 	}
 	flags &=3D ~(RV8803_FLAG_AF | RV8803_FLAG_UF);
-	err =3D i2c_smbus_write_byte_data(client, RV8803_FLAG, flags);
+	err =3D rv8803_write_reg(client, RV8803_FLAG, flags);
 	mutex_unlock(&rv8803->flags_lock);
 	if (err)
 		return err;
=20
 	if (ctrl !=3D rv8803->ctrl) {
 		rv8803->ctrl =3D ctrl;
-		err =3D i2c_smbus_write_byte_data(client, RV8803_CTRL,
-						rv8803->ctrl);
+		err =3D rv8803_write_reg(client, RV8803_CTRL, rv8803->ctrl);
 		if (err)
 			return err;
 	}
@@ -334,7 +395,7 @@ static int rv8803_ioctl(struct device *dev, unsigned in=
t cmd, unsigned long arg)
=20
 	switch (cmd) {
 	case RTC_VL_READ:
-		flags =3D i2c_smbus_read_byte_data(client, RV8803_FLAG);
+		flags =3D rv8803_read_reg(client, RV8803_FLAG);
 		if (flags < 0)
 			return flags;
=20
@@ -353,16 +414,16 @@ static int rv8803_ioctl(struct device *dev, unsigned =
int cmd, unsigned long arg)
=20
 	case RTC_VL_CLR:
 		mutex_lock(&rv8803->flags_lock);
-		flags =3D i2c_smbus_read_byte_data(client, RV8803_FLAG);
+		flags =3D rv8803_read_reg(client, RV8803_FLAG);
 		if (flags < 0) {
 			mutex_unlock(&rv8803->flags_lock);
 			return flags;
 		}
=20
 		flags &=3D ~(RV8803_FLAG_V1F | RV8803_FLAG_V2F);
-		ret =3D i2c_smbus_write_byte_data(client, RV8803_FLAG, flags);
+		ret =3D rv8803_write_reg(client, RV8803_FLAG, flags);
 		mutex_unlock(&rv8803->flags_lock);
-		if (ret < 0)
+		if (ret)
 			return ret;
=20
 		return 0;
@@ -380,8 +441,8 @@ static ssize_t rv8803_nvram_write(struct file *filp, st=
ruct kobject *kobj,
 	struct i2c_client *client =3D to_i2c_client(dev);
 	int ret;
=20
-	ret =3D i2c_smbus_write_byte_data(client, RV8803_RAM, buf[0]);
-	if (ret < 0)
+	ret =3D rv8803_write_reg(client, RV8803_RAM, buf[0]);
+	if (ret)
 		return ret;
=20
 	return 1;
@@ -395,7 +456,7 @@ static ssize_t rv8803_nvram_read(struct file *filp, str=
uct kobject *kobj,
 	struct i2c_client *client =3D to_i2c_client(dev);
 	int ret;
=20
-	ret =3D i2c_smbus_read_byte_data(client, RV8803_RAM);
+	ret =3D rv8803_read_reg(client, RV8803_RAM);
 	if (ret < 0)
 		return ret;
=20
@@ -425,7 +486,7 @@ static int rv8803_probe(struct i2c_client *client,
 {
 	struct i2c_adapter *adapter =3D to_i2c_adapter(client->dev.parent);
 	struct rv8803_data *rv8803;
-	int err, flags, try =3D 0;
+	int err, flags;
=20
 	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA |
 				     I2C_FUNC_SMBUS_I2C_BLOCK)) {
@@ -442,16 +503,7 @@ static int rv8803_probe(struct i2c_client *client,
 	rv8803->client =3D client;
 	i2c_set_clientdata(client, rv8803);
=20
-	/*
-	 * There is a 60=C2=B5s window where the RTC may not reply on the i2c bus=
 in
-	 * that case, the transfer is not ACKed. In that case, ensure there are
-	 * multiple attempts.
-	 */
-	do {
-		flags =3D i2c_smbus_read_byte_data(client, RV8803_FLAG);
-		try++;
-	} while (((flags =3D=3D -ENXIO) || (flags =3D=3D -EIO)) && (try < 4));
-
+	flags =3D rv8803_read_reg(client, RV8803_FLAG);
 	if (flags < 0)
 		return flags;
=20
@@ -486,12 +538,7 @@ static int rv8803_probe(struct i2c_client *client,
 		return PTR_ERR(rv8803->rtc);
 	}
=20
-	try =3D 0;
-	do {
-		err =3D i2c_smbus_write_byte_data(rv8803->client, RV8803_EXT,
-						RV8803_EXT_WADA);
-		try++;
-	} while (((err =3D=3D -ENXIO) || (flags =3D=3D -EIO)) && (try < 4));
+	err =3D rv8803_write_reg(rv8803->client, RV8803_EXT, RV8803_EXT_WADA);
 	if (err)
 		return err;
=20
--=20
2.5.0

--=20
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
---=20
You received this message because you are subscribed to the Google Groups "=
rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an e=
mail to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [PATCH 4/6] rtc: rv8803: Always apply the I²C workaround

[Alexandre Belloni]

On 21/07/2016 at 12:41:30 +0200, Benoît Thébaudeau wrote :
> The I²C NACK issue of the RV-8803 may occur after any I²C START
> condition, depending on the timings. Consequently, the workaround must
> be applied for all the I²C transfers.
> 
> This commit abstracts the I²C transfer code into register access
> functions. This avoids duplicating the I²C workaround everywhere. This
> also avoids the duplication of the code handling the return value of
> i2c_smbus_read_i2c_block_data(). Error messages are issued in case of
> definitive register access failures (if the workaround fails). This
> change also makes the I²C transfer return value checks consistent.
> 

Well, my initial idea was that the workaround is actually needed only
for operations that are not restartable from userspace.

Did you it that bug? On which RTC?

-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[rtc-linux] Re: [PATCH 4/6] rtc: rv8803: Always apply the I²C workaround

[Alexandre Belloni]

On 21/07/2016 at 12:41:30 +0200, Beno=C3=AEt Th=C3=A9baudeau wrote :
> The I=C2=B2C NACK issue of the RV-8803 may occur after any I=C2=B2C START
> condition, depending on the timings. Consequently, the workaround must
> be applied for all the I=C2=B2C transfers.
>=20
> This commit abstracts the I=C2=B2C transfer code into register access
> functions. This avoids duplicating the I=C2=B2C workaround everywhere. Th=
is
> also avoids the duplication of the code handling the return value of
> i2c_smbus_read_i2c_block_data(). Error messages are issued in case of
> definitive register access failures (if the workaround fails). This
> change also makes the I=C2=B2C transfer return value checks consistent.
>=20

Well, my initial idea was that the workaround is actually needed only
for operations that are not restartable from userspace.

Did you it that bug? On which RTC?

--=20
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

--=20
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
---=20
You received this message because you are subscribed to the Google Groups "=
rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an e=
mail to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [PATCH 4/6] rtc: rv8803: Always apply the I²C workaround

[Benoît Thébaudeau]

On 21/07/2016 at 13:10, Alexandre Belloni wrote:
> On 21/07/2016 at 12:41:30 +0200, Benoît Thébaudeau wrote :
>> The I²C NACK issue of the RV-8803 may occur after any I²C START
>> condition, depending on the timings. Consequently, the workaround must
>> be applied for all the I²C transfers.
>>
>> This commit abstracts the I²C transfer code into register access
>> functions. This avoids duplicating the I²C workaround everywhere. This
>> also avoids the duplication of the code handling the return value of
>> i2c_smbus_read_i2c_block_data(). Error messages are issued in case of
>> definitive register access failures (if the workaround fails). This
>> change also makes the I²C transfer return value checks consistent.
>>
> 
> Well, my initial idea was that the workaround is actually needed only
> for operations that are not restartable from userspace.
> 
> Did you it that bug? On which RTC?

No, I've not seen that bug. However, the errata sheet says that this issue may
occur if two consecutive I²C START conditions are slightly more than 950 ms
apart. It does not even say that this is restricted to I²C transfers addressing
this RTC. Moreover, if the time is read or set from userspace, this might occur
at any time relatively to an RTC interrupt or to a previous time read/set
operation, so possibly 950 ms afterwards.

All in all, it's safer and easier to always apply this workaround, all the more
it is applied at almost no cost when there is no issue.

Best regards,
Benoît

[rtc-linux] Re: [PATCH 4/6] rtc: rv8803: Always apply the I²C workaround

[Benoît Thébaudeau]

On 21/07/2016 at 13:10, Alexandre Belloni wrote:
> On 21/07/2016 at 12:41:30 +0200, Beno=C3=AEt Th=C3=A9baudeau wrote :
>> The I=C2=B2C NACK issue of the RV-8803 may occur after any I=C2=B2C STAR=
T
>> condition, depending on the timings. Consequently, the workaround must
>> be applied for all the I=C2=B2C transfers.
>>
>> This commit abstracts the I=C2=B2C transfer code into register access
>> functions. This avoids duplicating the I=C2=B2C workaround everywhere. T=
his
>> also avoids the duplication of the code handling the return value of
>> i2c_smbus_read_i2c_block_data(). Error messages are issued in case of
>> definitive register access failures (if the workaround fails). This
>> change also makes the I=C2=B2C transfer return value checks consistent.
>>
>=20
> Well, my initial idea was that the workaround is actually needed only
> for operations that are not restartable from userspace.
>=20
> Did you it that bug? On which RTC?

No, I've not seen that bug. However, the errata sheet says that this issue =
may
occur if two consecutive I=C2=B2C START conditions are slightly more than 9=
50 ms
apart. It does not even say that this is restricted to I=C2=B2C transfers a=
ddressing
this RTC. Moreover, if the time is read or set from userspace, this might o=
ccur
at any time relatively to an RTC interrupt or to a previous time read/set
operation, so possibly 950 ms afterwards.

All in all, it's safer and easier to always apply this workaround, all the =
more
it is applied at almost no cost when there is no issue.

Best regards,
Beno=C3=AEt

--=20
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
---=20
You received this message because you are subscribed to the Google Groups "=
rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an e=
mail to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [PATCH 4/6] rtc: rv8803: Always apply the I²C workaround

[Benoît Thébaudeau]

On 21/07/2016 at 14:34, Benoît Thébaudeau wrote:
> On 21/07/2016 at 13:10, Alexandre Belloni wrote:
>> On 21/07/2016 at 12:41:30 +0200, Benoît Thébaudeau wrote :
>>> The I²C NACK issue of the RV-8803 may occur after any I²C START
>>> condition, depending on the timings. Consequently, the workaround must
>>> be applied for all the I²C transfers.
>>>
>>> This commit abstracts the I²C transfer code into register access
>>> functions. This avoids duplicating the I²C workaround everywhere. This
>>> also avoids the duplication of the code handling the return value of
>>> i2c_smbus_read_i2c_block_data(). Error messages are issued in case of
>>> definitive register access failures (if the workaround fails). This
>>> change also makes the I²C transfer return value checks consistent.
>>>
>>
>> Well, my initial idea was that the workaround is actually needed only
>> for operations that are not restartable from userspace.
>>
>> Did you it that bug? On which RTC?
>
> No, I've not seen that bug. However, the errata sheet says that this issue may
> occur if two consecutive I²C START conditions are slightly more than 950 ms
> apart. It does not even say that this is restricted to I²C transfers
> addressing
> this RTC. Moreover, if the time is read or set from userspace, this might
> occur
> at any time relatively to an RTC interrupt or to a previous time read/set
> operation, so possibly 950 ms afterwards.
>
> All in all, it's safer and easier to always apply this workaround, all the
> more
> it is applied at almost no cost when there is no issue.

To fully answer your questions, it's the job of the driver to execute the
userspace requests properly without returning spurious errors. Userspace might
consider that such errors are definitive and that there is no reason to try and
restart the operation, which could cause all sorts of issues. In other words,
the driver should not rely on userspace operation restarts as a kind of
workaround if it can easily handle this itself.

Also, the kernel itself (not only userspace) may read the RTC time (with
CONFIG_RTC_HCTOSYS), without userspace caring.

I'm rather working with the RX8900.

Best regards,
Benoît

[rtc-linux] Re: [PATCH 4/6] rtc: rv8803: Always apply the I²C workaround

[Benoît Thébaudeau]

On 21/07/2016 at 14:34, Beno=C3=AEt Th=C3=A9baudeau wrote:
> On 21/07/2016 at 13:10, Alexandre Belloni wrote:
>> On 21/07/2016 at 12:41:30 +0200, Beno=C3=AEt Th=C3=A9baudeau wrote :
>>> The I=C2=B2C NACK issue of the RV-8803 may occur after any I=C2=B2C STA=
RT
>>> condition, depending on the timings. Consequently, the workaround must
>>> be applied for all the I=C2=B2C transfers.
>>>
>>> This commit abstracts the I=C2=B2C transfer code into register access
>>> functions. This avoids duplicating the I=C2=B2C workaround everywhere. =
This
>>> also avoids the duplication of the code handling the return value of
>>> i2c_smbus_read_i2c_block_data(). Error messages are issued in case of
>>> definitive register access failures (if the workaround fails). This
>>> change also makes the I=C2=B2C transfer return value checks consistent.
>>>
>>
>> Well, my initial idea was that the workaround is actually needed only
>> for operations that are not restartable from userspace.
>>
>> Did you it that bug? On which RTC?
>
> No, I've not seen that bug. However, the errata sheet says that this issu=
e may
> occur if two consecutive I=C2=B2C START conditions are slightly more than=
 950 ms
> apart. It does not even say that this is restricted to I=C2=B2C transfers
> addressing
> this RTC. Moreover, if the time is read or set from userspace, this might
> occur
> at any time relatively to an RTC interrupt or to a previous time read/set
> operation, so possibly 950 ms afterwards.
>
> All in all, it's safer and easier to always apply this workaround, all th=
e
> more
> it is applied at almost no cost when there is no issue.

To fully answer your questions, it's the job of the driver to execute the
userspace requests properly without returning spurious errors. Userspace mi=
ght
consider that such errors are definitive and that there is no reason to try=
 and
restart the operation, which could cause all sorts of issues. In other word=
s,
the driver should not rely on userspace operation restarts as a kind of
workaround if it can easily handle this itself.

Also, the kernel itself (not only userspace) may read the RTC time (with
CONFIG_RTC_HCTOSYS), without userspace caring.

I'm rather working with the RX8900.

Best regards,
Beno=C3=AEt

--=20
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
---=20
You received this message because you are subscribed to the Google Groups "=
rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an e=
mail to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[PATCH 5/6] rtc: rv8803: Stop the clock while setting the time

[Benoît Thébaudeau]

According to the application manual of the RX8900, the RESET bit must be
set to 1 to prevent a timer update while setting the time. This also
resets the subsecond counter. The application manual of the RV-8803 does
not mention such a requirement, and it says that the 100th Seconds
register is cleared when writing to the Seconds register, but using the
RESET bit for the RV-8803 too should not be an issue and is probably
safer.

This change also ensures that the RESET bit is initialized properly in
all cases. Indeed, all the registers must be initialized if the voltage
has been lower than VLOW2 (triggering V2F), but not low enough to
trigger a POR.

Signed-off-by: Benoît Thébaudeau <benoit@wsystem.com>
---
 drivers/rtc/rtc-rv8803.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/rtc/rtc-rv8803.c b/drivers/rtc/rtc-rv8803.c
index 09ab5cb..24c688e 100644
--- a/drivers/rtc/rtc-rv8803.c
+++ b/drivers/rtc/rtc-rv8803.c
@@ -223,11 +223,21 @@ static int rv8803_set_time(struct device *dev, struct rtc_time *tm)
 {
 	struct rv8803_data *rv8803 = dev_get_drvdata(dev);
 	u8 date[7];
-	int flags, ret;
+	int ctrl, flags, ret;
 
 	if ((tm->tm_year < 100) || (tm->tm_year > 199))
 		return -EINVAL;
 
+	ctrl = rv8803_read_reg(rv8803->client, RV8803_CTRL);
+	if (ctrl < 0)
+		return ctrl;
+
+	/* Stop the clock */
+	ret = rv8803_write_reg(rv8803->client, RV8803_CTRL,
+			       ctrl | RV8803_CTRL_RESET);
+	if (ret)
+		return ret;
+
 	date[RV8803_SEC]   = bin2bcd(tm->tm_sec);
 	date[RV8803_MIN]   = bin2bcd(tm->tm_min);
 	date[RV8803_HOUR]  = bin2bcd(tm->tm_hour);
@@ -240,6 +250,12 @@ static int rv8803_set_time(struct device *dev, struct rtc_time *tm)
 	if (ret)
 		return ret;
 
+	/* Restart the clock */
+	ret = rv8803_write_reg(rv8803->client, RV8803_CTRL,
+			       ctrl & ~RV8803_CTRL_RESET);
+	if (ret)
+		return ret;
+
 	mutex_lock(&rv8803->flags_lock);
 
 	flags = rv8803_read_reg(rv8803->client, RV8803_FLAG);
-- 
2.5.0

[rtc-linux] [PATCH 5/6] rtc: rv8803: Stop the clock while setting the time

[Benoît Thébaudeau]

According to the application manual of the RX8900, the RESET bit must be
set to 1 to prevent a timer update while setting the time. This also
resets the subsecond counter. The application manual of the RV-8803 does
not mention such a requirement, and it says that the 100th Seconds
register is cleared when writing to the Seconds register, but using the
RESET bit for the RV-8803 too should not be an issue and is probably
safer.

This change also ensures that the RESET bit is initialized properly in
all cases. Indeed, all the registers must be initialized if the voltage
has been lower than VLOW2 (triggering V2F), but not low enough to
trigger a POR.

Signed-off-by: Beno=C3=AEt Th=C3=A9baudeau <benoit@wsystem.com>
---
 drivers/rtc/rtc-rv8803.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/rtc/rtc-rv8803.c b/drivers/rtc/rtc-rv8803.c
index 09ab5cb..24c688e 100644
--- a/drivers/rtc/rtc-rv8803.c
+++ b/drivers/rtc/rtc-rv8803.c
@@ -223,11 +223,21 @@ static int rv8803_set_time(struct device *dev, struct=
 rtc_time *tm)
 {
 	struct rv8803_data *rv8803 =3D dev_get_drvdata(dev);
 	u8 date[7];
-	int flags, ret;
+	int ctrl, flags, ret;
=20
 	if ((tm->tm_year < 100) || (tm->tm_year > 199))
 		return -EINVAL;
=20
+	ctrl =3D rv8803_read_reg(rv8803->client, RV8803_CTRL);
+	if (ctrl < 0)
+		return ctrl;
+
+	/* Stop the clock */
+	ret =3D rv8803_write_reg(rv8803->client, RV8803_CTRL,
+			       ctrl | RV8803_CTRL_RESET);
+	if (ret)
+		return ret;
+
 	date[RV8803_SEC]   =3D bin2bcd(tm->tm_sec);
 	date[RV8803_MIN]   =3D bin2bcd(tm->tm_min);
 	date[RV8803_HOUR]  =3D bin2bcd(tm->tm_hour);
@@ -240,6 +250,12 @@ static int rv8803_set_time(struct device *dev, struct =
rtc_time *tm)
 	if (ret)
 		return ret;
=20
+	/* Restart the clock */
+	ret =3D rv8803_write_reg(rv8803->client, RV8803_CTRL,
+			       ctrl & ~RV8803_CTRL_RESET);
+	if (ret)
+		return ret;
+
 	mutex_lock(&rv8803->flags_lock);
=20
 	flags =3D rv8803_read_reg(rv8803->client, RV8803_FLAG);
--=20
2.5.0

--=20
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
---=20
You received this message because you are subscribed to the Google Groups "=
rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an e=
mail to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[PATCH 6/6] rtc: rv8803: Clear V1F when setting the time

[Benoît Thébaudeau]

V1F indicates that the time accuracy may have been compromised because
of a voltage drop (possibly only temporary) below VLOW1, which stops the
temperature compensation. When the time is set, the accuracy is
restored, so V1F should be cleared in order to indicate this and to be
able to detect the next temperature compensation loss. This is the same
principle as for V2F, which is cleared when the time is set to indicate
that the time is no longer invalid and to be able to detect the next
data loss.

Signed-off-by: Benoît Thébaudeau <benoit@wsystem.com>
---
 drivers/rtc/rtc-rv8803.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/rtc/rtc-rv8803.c b/drivers/rtc/rtc-rv8803.c
index 24c688e..9a2f6a9 100644
--- a/drivers/rtc/rtc-rv8803.c
+++ b/drivers/rtc/rtc-rv8803.c
@@ -265,7 +265,7 @@ static int rv8803_set_time(struct device *dev, struct rtc_time *tm)
 	}
 
 	ret = rv8803_write_reg(rv8803->client, RV8803_FLAG,
-			       flags & ~RV8803_FLAG_V2F);
+			       flags & ~(RV8803_FLAG_V1F | RV8803_FLAG_V2F));
 
 	mutex_unlock(&rv8803->flags_lock);
 
-- 
2.5.0

[rtc-linux] [PATCH 6/6] rtc: rv8803: Clear V1F when setting the time

[Benoît Thébaudeau]

V1F indicates that the time accuracy may have been compromised because
of a voltage drop (possibly only temporary) below VLOW1, which stops the
temperature compensation. When the time is set, the accuracy is
restored, so V1F should be cleared in order to indicate this and to be
able to detect the next temperature compensation loss. This is the same
principle as for V2F, which is cleared when the time is set to indicate
that the time is no longer invalid and to be able to detect the next
data loss.

Signed-off-by: Beno=C3=AEt Th=C3=A9baudeau <benoit@wsystem.com>
---
 drivers/rtc/rtc-rv8803.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/rtc/rtc-rv8803.c b/drivers/rtc/rtc-rv8803.c
index 24c688e..9a2f6a9 100644
--- a/drivers/rtc/rtc-rv8803.c
+++ b/drivers/rtc/rtc-rv8803.c
@@ -265,7 +265,7 @@ static int rv8803_set_time(struct device *dev, struct r=
tc_time *tm)
 	}
=20
 	ret =3D rv8803_write_reg(rv8803->client, RV8803_FLAG,
-			       flags & ~RV8803_FLAG_V2F);
+			       flags & ~(RV8803_FLAG_V1F | RV8803_FLAG_V2F));
=20
 	mutex_unlock(&rv8803->flags_lock);
=20
--=20
2.5.0

--=20
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
---=20
You received this message because you are subscribed to the Google Groups "=
rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an e=
mail to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [PATCH 1/6] rtc: rv8803: Kconfig: Indicate rx8900 support

[Alexandre Belloni]

On 21/07/2016 at 12:41:27 +0200, Benoît Thébaudeau wrote :
> This driver supports the Epson RX8900, but this was not indicated in
> Kconfig.
> 
> Signed-off-by: Benoît Thébaudeau <benoit@wsystem.com>
> ---
>  drivers/rtc/Kconfig | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 

All applied, thanks.

-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[rtc-linux] Re: [PATCH 1/6] rtc: rv8803: Kconfig: Indicate rx8900 support

[Alexandre Belloni]

On 21/07/2016 at 12:41:27 +0200, Beno=C3=AEt Th=C3=A9baudeau wrote :
> This driver supports the Epson RX8900, but this was not indicated in
> Kconfig.
>=20
> Signed-off-by: Beno=C3=AEt Th=C3=A9baudeau <benoit@wsystem.com>
> ---
>  drivers/rtc/Kconfig | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>=20

All applied, thanks.

--=20
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

--=20
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
---=20
You received this message because you are subscribed to the Google Groups "=
rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an e=
mail to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.