linux-rtc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 00/14] rtc: rx8010: use regmap instead of i2c smbus API
@ 2020-09-14 15:45 Bartosz Golaszewski
  2020-09-14 15:45 ` [PATCH v3 01/14] rtc: rx8010: don't modify the global rtc ops Bartosz Golaszewski
                   ` (14 more replies)
  0 siblings, 15 replies; 18+ messages in thread
From: Bartosz Golaszewski @ 2020-09-14 15:45 UTC (permalink / raw)
  To: Alessandro Zummo, Alexandre Belloni
  Cc: linux-rtc, linux-kernel, Bartosz Golaszewski

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

This series gets bigger and bigger but I noticed a problem with this
driver that looks like stable material so I fixed it as the first patch
in the series to make backporting easy.

Other than that, there are new refactoring patches and I removed the
unnecessary error messages.

--

I want to use this driver on a platform where the i2c controller doesn't
speak SMBUS. This series converts the driver to i2c regmap which can
figure out the correct protocol to use.

The actual conversion happens in patch 13. The rest are bugfixes and
refactoring.

v1 -> v2:
- s/parentheses/brackets/g
- add a patch switching the driver to using the preferred RTC API
- rework the patch removing magic values
- use range_max and range_min instead of manual range checks
- add a patch adding a helper variable in probe() for client->dev
- add a patch using sizeof(*rx8010) instead of sizeof(struct rx8010_data)

v2 -> v3:
- remove unnecessary error messages when registering the RTC device
- add a patch switching to using the preferred probe_new callback in
  the I2C driver
- add a patch fixing an issue where the global rtc operations struct
  is modified depending on the presence of the interrupt
- add a patch removing a stray newline

Bartosz Golaszewski (14):
  rtc: rx8010: don't modify the global rtc ops
  rtc: rx8010: remove a stray newline
  rtc: rx8010: remove unnecessary brackets
  rtc: rx8010: consolidate local variables of the same type
  rtc: rx8010: use tabs instead of spaces for code formatting
  rtc: rx8010: rename ret to err in rx8010_set_time()
  rtc: rx8010: don't use magic values for time buffer length
  rtc: rx8010: drop unnecessary initialization
  rtc: rx8010: use a helper variable for client->dev in probe()
  rtc: rx8010: prefer sizeof(*val) over sizeof(struct type_of_val)
  rtc: rx8010: switch to using the preferred RTC API
  rtc: rx8010: switch to using the preferred i2c API
  rtc: rx8010: convert to using regmap
  rtc: rx8010: use range checking provided by core RTC code

 drivers/rtc/rtc-rx8010.c | 332 +++++++++++++++++----------------------
 1 file changed, 143 insertions(+), 189 deletions(-)

-- 
2.26.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v3 01/14] rtc: rx8010: don't modify the global rtc ops
  2020-09-14 15:45 [PATCH v3 00/14] rtc: rx8010: use regmap instead of i2c smbus API Bartosz Golaszewski
@ 2020-09-14 15:45 ` Bartosz Golaszewski
  2020-09-17 15:53   ` Sasha Levin
  2020-09-14 15:45 ` [PATCH v3 02/14] rtc: rx8010: remove a stray newline Bartosz Golaszewski
                   ` (13 subsequent siblings)
  14 siblings, 1 reply; 18+ messages in thread
From: Bartosz Golaszewski @ 2020-09-14 15:45 UTC (permalink / raw)
  To: Alessandro Zummo, Alexandre Belloni
  Cc: linux-rtc, linux-kernel, Bartosz Golaszewski, stable

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

The way the driver is implemented is buggy for the (admittedly unlikely)
use case where there are two RTCs with one having an interrupt configured
and the second not. This is caused by the fact that we use a global
rtc_class_ops struct which we modify depending on whether the irq number
is present or not.

Fix it by using two const ops structs with and without alarm operations.
While at it: not being able to request a configured interrupt is an error
so don't ignore it and bail out of probe().

Fixes: ed13d89b08e3 ("rtc: Add Epson RX8010SJ RTC driver")
Cc: stable@vger.kernel.org
Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 drivers/rtc/rtc-rx8010.c | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/drivers/rtc/rtc-rx8010.c b/drivers/rtc/rtc-rx8010.c
index fe010151ec8f..08c93d492494 100644
--- a/drivers/rtc/rtc-rx8010.c
+++ b/drivers/rtc/rtc-rx8010.c
@@ -407,16 +407,26 @@ static int rx8010_ioctl(struct device *dev, unsigned int cmd, unsigned long arg)
 	}
 }
 
-static struct rtc_class_ops rx8010_rtc_ops = {
+static const struct rtc_class_ops rx8010_rtc_ops_default = {
 	.read_time = rx8010_get_time,
 	.set_time = rx8010_set_time,
 	.ioctl = rx8010_ioctl,
 };
 
+static const struct rtc_class_ops rx8010_rtc_ops_alarm = {
+	.read_time = rx8010_get_time,
+	.set_time = rx8010_set_time,
+	.ioctl = rx8010_ioctl,
+	.read_alarm = rx8010_read_alarm,
+	.set_alarm = rx8010_set_alarm,
+	.alarm_irq_enable = rx8010_alarm_irq_enable,
+};
+
 static int rx8010_probe(struct i2c_client *client,
 			const struct i2c_device_id *id)
 {
 	struct i2c_adapter *adapter = client->adapter;
+	const struct rtc_class_ops *rtc_ops;
 	struct rx8010_data *rx8010;
 	int err = 0;
 
@@ -447,16 +457,16 @@ static int rx8010_probe(struct i2c_client *client,
 
 		if (err) {
 			dev_err(&client->dev, "unable to request IRQ\n");
-			client->irq = 0;
-		} else {
-			rx8010_rtc_ops.read_alarm = rx8010_read_alarm;
-			rx8010_rtc_ops.set_alarm = rx8010_set_alarm;
-			rx8010_rtc_ops.alarm_irq_enable = rx8010_alarm_irq_enable;
+			return err;
 		}
+
+		rtc_ops = &rx8010_rtc_ops_alarm;
+	} else {
+		rtc_ops = &rx8010_rtc_ops_default;
 	}
 
 	rx8010->rtc = devm_rtc_device_register(&client->dev, client->name,
-		&rx8010_rtc_ops, THIS_MODULE);
+					       rtc_ops, THIS_MODULE);
 
 	if (IS_ERR(rx8010->rtc)) {
 		dev_err(&client->dev, "unable to register the class device\n");
-- 
2.26.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v3 02/14] rtc: rx8010: remove a stray newline
  2020-09-14 15:45 [PATCH v3 00/14] rtc: rx8010: use regmap instead of i2c smbus API Bartosz Golaszewski
  2020-09-14 15:45 ` [PATCH v3 01/14] rtc: rx8010: don't modify the global rtc ops Bartosz Golaszewski
@ 2020-09-14 15:45 ` Bartosz Golaszewski
  2020-09-14 15:45 ` [PATCH v3 03/14] rtc: rx8010: remove unnecessary brackets Bartosz Golaszewski
                   ` (12 subsequent siblings)
  14 siblings, 0 replies; 18+ messages in thread
From: Bartosz Golaszewski @ 2020-09-14 15:45 UTC (permalink / raw)
  To: Alessandro Zummo, Alexandre Belloni
  Cc: linux-rtc, linux-kernel, Bartosz Golaszewski

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Remove an unnecessary newline after requesting the interrupt.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 drivers/rtc/rtc-rx8010.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/rtc/rtc-rx8010.c b/drivers/rtc/rtc-rx8010.c
index 08c93d492494..c6797ec0aba1 100644
--- a/drivers/rtc/rtc-rx8010.c
+++ b/drivers/rtc/rtc-rx8010.c
@@ -454,7 +454,6 @@ static int rx8010_probe(struct i2c_client *client,
 						rx8010_irq_1_handler,
 						IRQF_TRIGGER_LOW | IRQF_ONESHOT,
 						"rx8010", client);
-
 		if (err) {
 			dev_err(&client->dev, "unable to request IRQ\n");
 			return err;
-- 
2.26.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v3 03/14] rtc: rx8010: remove unnecessary brackets
  2020-09-14 15:45 [PATCH v3 00/14] rtc: rx8010: use regmap instead of i2c smbus API Bartosz Golaszewski
  2020-09-14 15:45 ` [PATCH v3 01/14] rtc: rx8010: don't modify the global rtc ops Bartosz Golaszewski
  2020-09-14 15:45 ` [PATCH v3 02/14] rtc: rx8010: remove a stray newline Bartosz Golaszewski
@ 2020-09-14 15:45 ` Bartosz Golaszewski
  2020-09-14 15:45 ` [PATCH v3 04/14] rtc: rx8010: consolidate local variables of the same type Bartosz Golaszewski
                   ` (11 subsequent siblings)
  14 siblings, 0 replies; 18+ messages in thread
From: Bartosz Golaszewski @ 2020-09-14 15:45 UTC (permalink / raw)
  To: Alessandro Zummo, Alexandre Belloni
  Cc: linux-rtc, linux-kernel, Bartosz Golaszewski

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Remove brackets wherever they guard a single line.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 drivers/rtc/rtc-rx8010.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/rtc/rtc-rx8010.c b/drivers/rtc/rtc-rx8010.c
index c6797ec0aba1..79a3d90d2c43 100644
--- a/drivers/rtc/rtc-rx8010.c
+++ b/drivers/rtc/rtc-rx8010.c
@@ -181,9 +181,8 @@ static int rx8010_set_time(struct device *dev, struct rtc_time *dt)
 		return ret;
 
 	flagreg = i2c_smbus_read_byte_data(rx8010->client, RX8010_FLAG);
-	if (flagreg < 0) {
+	if (flagreg < 0)
 		return flagreg;
-	}
 
 	if (flagreg & RX8010_FLAG_VLF)
 		ret = i2c_smbus_write_byte_data(rx8010->client, RX8010_FLAG,
@@ -284,17 +283,15 @@ static int rx8010_set_alarm(struct device *dev, struct rtc_wkalrm *t)
 	int err;
 
 	flagreg = i2c_smbus_read_byte_data(client, RX8010_FLAG);
-	if (flagreg < 0) {
+	if (flagreg < 0)
 		return flagreg;
-	}
 
 	if (rx8010->ctrlreg & (RX8010_CTRL_AIE | RX8010_CTRL_UIE)) {
 		rx8010->ctrlreg &= ~(RX8010_CTRL_AIE | RX8010_CTRL_UIE);
 		err = i2c_smbus_write_byte_data(rx8010->client, RX8010_CTRL,
 						rx8010->ctrlreg);
-		if (err < 0) {
+		if (err < 0)
 			return err;
-		}
 	}
 
 	flagreg &= ~RX8010_FLAG_AF;
-- 
2.26.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v3 04/14] rtc: rx8010: consolidate local variables of the same type
  2020-09-14 15:45 [PATCH v3 00/14] rtc: rx8010: use regmap instead of i2c smbus API Bartosz Golaszewski
                   ` (2 preceding siblings ...)
  2020-09-14 15:45 ` [PATCH v3 03/14] rtc: rx8010: remove unnecessary brackets Bartosz Golaszewski
@ 2020-09-14 15:45 ` Bartosz Golaszewski
  2020-09-14 15:45 ` [PATCH v3 05/14] rtc: rx8010: use tabs instead of spaces for code formatting Bartosz Golaszewski
                   ` (10 subsequent siblings)
  14 siblings, 0 replies; 18+ messages in thread
From: Bartosz Golaszewski @ 2020-09-14 15:45 UTC (permalink / raw)
  To: Alessandro Zummo, Alexandre Belloni
  Cc: linux-rtc, linux-kernel, Bartosz Golaszewski

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Move local variables of the same type into a single line for better
readability.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 drivers/rtc/rtc-rx8010.c | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/drivers/rtc/rtc-rx8010.c b/drivers/rtc/rtc-rx8010.c
index 79a3d90d2c43..153fa58f0365 100644
--- a/drivers/rtc/rtc-rx8010.c
+++ b/drivers/rtc/rtc-rx8010.c
@@ -109,8 +109,7 @@ static int rx8010_get_time(struct device *dev, struct rtc_time *dt)
 {
 	struct rx8010_data *rx8010 = dev_get_drvdata(dev);
 	u8 date[7];
-	int flagreg;
-	int err;
+	int flagreg, err;
 
 	flagreg = i2c_smbus_read_byte_data(rx8010->client, RX8010_FLAG);
 	if (flagreg < 0)
@@ -141,8 +140,7 @@ static int rx8010_set_time(struct device *dev, struct rtc_time *dt)
 {
 	struct rx8010_data *rx8010 = dev_get_drvdata(dev);
 	u8 date[7];
-	int ctrl, flagreg;
-	int ret;
+	int ctrl, flagreg, ret;
 
 	if ((dt->tm_year < 100) || (dt->tm_year > 199))
 		return -EINVAL;
@@ -250,8 +248,7 @@ static int rx8010_read_alarm(struct device *dev, struct rtc_wkalrm *t)
 	struct rx8010_data *rx8010 = dev_get_drvdata(dev);
 	struct i2c_client *client = rx8010->client;
 	u8 alarmvals[3];
-	int flagreg;
-	int err;
+	int flagreg, err;
 
 	err = i2c_smbus_read_i2c_block_data(client, RX8010_ALMIN, 3, alarmvals);
 	if (err != 3)
@@ -279,8 +276,7 @@ static int rx8010_set_alarm(struct device *dev, struct rtc_wkalrm *t)
 	struct i2c_client *client = to_i2c_client(dev);
 	struct rx8010_data *rx8010 = dev_get_drvdata(dev);
 	u8 alarmvals[3];
-	int extreg, flagreg;
-	int err;
+	int extreg, flagreg, err;
 
 	flagreg = i2c_smbus_read_byte_data(client, RX8010_FLAG);
 	if (flagreg < 0)
@@ -346,9 +342,8 @@ static int rx8010_alarm_irq_enable(struct device *dev,
 {
 	struct i2c_client *client = to_i2c_client(dev);
 	struct rx8010_data *rx8010 = dev_get_drvdata(dev);
-	int flagreg;
+	int flagreg, err;
 	u8 ctrl;
-	int err;
 
 	ctrl = rx8010->ctrlreg;
 
@@ -387,8 +382,7 @@ static int rx8010_alarm_irq_enable(struct device *dev,
 static int rx8010_ioctl(struct device *dev, unsigned int cmd, unsigned long arg)
 {
 	struct rx8010_data *rx8010 = dev_get_drvdata(dev);
-	int tmp;
-	int flagreg;
+	int tmp, flagreg;
 
 	switch (cmd) {
 	case RTC_VL_READ:
-- 
2.26.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v3 05/14] rtc: rx8010: use tabs instead of spaces for code formatting
  2020-09-14 15:45 [PATCH v3 00/14] rtc: rx8010: use regmap instead of i2c smbus API Bartosz Golaszewski
                   ` (3 preceding siblings ...)
  2020-09-14 15:45 ` [PATCH v3 04/14] rtc: rx8010: consolidate local variables of the same type Bartosz Golaszewski
@ 2020-09-14 15:45 ` Bartosz Golaszewski
  2020-09-14 15:45 ` [PATCH v3 06/14] rtc: rx8010: rename ret to err in rx8010_set_time() Bartosz Golaszewski
                   ` (9 subsequent siblings)
  14 siblings, 0 replies; 18+ messages in thread
From: Bartosz Golaszewski @ 2020-09-14 15:45 UTC (permalink / raw)
  To: Alessandro Zummo, Alexandre Belloni
  Cc: linux-rtc, linux-kernel, Bartosz Golaszewski

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

The define values in this driver are close to their names and they are
separated by spaces. Use tabs instead and align all defines.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 drivers/rtc/rtc-rx8010.c | 58 ++++++++++++++++++++--------------------
 1 file changed, 29 insertions(+), 29 deletions(-)

diff --git a/drivers/rtc/rtc-rx8010.c b/drivers/rtc/rtc-rx8010.c
index 153fa58f0365..51ac4fac8f19 100644
--- a/drivers/rtc/rtc-rx8010.c
+++ b/drivers/rtc/rtc-rx8010.c
@@ -13,40 +13,40 @@
 #include <linux/module.h>
 #include <linux/rtc.h>
 
-#define RX8010_SEC     0x10
-#define RX8010_MIN     0x11
-#define RX8010_HOUR    0x12
-#define RX8010_WDAY    0x13
-#define RX8010_MDAY    0x14
-#define RX8010_MONTH   0x15
-#define RX8010_YEAR    0x16
-#define RX8010_RESV17  0x17
-#define RX8010_ALMIN   0x18
-#define RX8010_ALHOUR  0x19
-#define RX8010_ALWDAY  0x1A
-#define RX8010_TCOUNT0 0x1B
-#define RX8010_TCOUNT1 0x1C
-#define RX8010_EXT     0x1D
-#define RX8010_FLAG    0x1E
-#define RX8010_CTRL    0x1F
+#define RX8010_SEC		0x10
+#define RX8010_MIN		0x11
+#define RX8010_HOUR		0x12
+#define RX8010_WDAY		0x13
+#define RX8010_MDAY		0x14
+#define RX8010_MONTH		0x15
+#define RX8010_YEAR		0x16
+#define RX8010_RESV17		0x17
+#define RX8010_ALMIN		0x18
+#define RX8010_ALHOUR		0x19
+#define RX8010_ALWDAY		0x1A
+#define RX8010_TCOUNT0		0x1B
+#define RX8010_TCOUNT1		0x1C
+#define RX8010_EXT		0x1D
+#define RX8010_FLAG		0x1E
+#define RX8010_CTRL		0x1F
 /* 0x20 to 0x2F are user registers */
-#define RX8010_RESV30  0x30
-#define RX8010_RESV31  0x31
-#define RX8010_IRQ     0x32
+#define RX8010_RESV30		0x30
+#define RX8010_RESV31		0x31
+#define RX8010_IRQ		0x32
 
-#define RX8010_EXT_WADA  BIT(3)
+#define RX8010_EXT_WADA		BIT(3)
 
-#define RX8010_FLAG_VLF  BIT(1)
-#define RX8010_FLAG_AF   BIT(3)
-#define RX8010_FLAG_TF   BIT(4)
-#define RX8010_FLAG_UF   BIT(5)
+#define RX8010_FLAG_VLF		BIT(1)
+#define RX8010_FLAG_AF		BIT(3)
+#define RX8010_FLAG_TF		BIT(4)
+#define RX8010_FLAG_UF		BIT(5)
 
-#define RX8010_CTRL_AIE  BIT(3)
-#define RX8010_CTRL_UIE  BIT(5)
-#define RX8010_CTRL_STOP BIT(6)
-#define RX8010_CTRL_TEST BIT(7)
+#define RX8010_CTRL_AIE		BIT(3)
+#define RX8010_CTRL_UIE		BIT(5)
+#define RX8010_CTRL_STOP	BIT(6)
+#define RX8010_CTRL_TEST	BIT(7)
 
-#define RX8010_ALARM_AE  BIT(7)
+#define RX8010_ALARM_AE		BIT(7)
 
 static const struct i2c_device_id rx8010_id[] = {
 	{ "rx8010", 0 },
-- 
2.26.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v3 06/14] rtc: rx8010: rename ret to err in rx8010_set_time()
  2020-09-14 15:45 [PATCH v3 00/14] rtc: rx8010: use regmap instead of i2c smbus API Bartosz Golaszewski
                   ` (4 preceding siblings ...)
  2020-09-14 15:45 ` [PATCH v3 05/14] rtc: rx8010: use tabs instead of spaces for code formatting Bartosz Golaszewski
@ 2020-09-14 15:45 ` Bartosz Golaszewski
  2020-09-14 15:45 ` [PATCH v3 07/14] rtc: rx8010: don't use magic values for time buffer length Bartosz Golaszewski
                   ` (8 subsequent siblings)
  14 siblings, 0 replies; 18+ messages in thread
From: Bartosz Golaszewski @ 2020-09-14 15:45 UTC (permalink / raw)
  To: Alessandro Zummo, Alexandre Belloni
  Cc: linux-rtc, linux-kernel, Bartosz Golaszewski

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

All other functions in this driver use 'err' for integer return values.
Do the same in rx8010_set_time() for consistency.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 drivers/rtc/rtc-rx8010.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/rtc/rtc-rx8010.c b/drivers/rtc/rtc-rx8010.c
index 51ac4fac8f19..300314ab7b6d 100644
--- a/drivers/rtc/rtc-rx8010.c
+++ b/drivers/rtc/rtc-rx8010.c
@@ -140,7 +140,7 @@ static int rx8010_set_time(struct device *dev, struct rtc_time *dt)
 {
 	struct rx8010_data *rx8010 = dev_get_drvdata(dev);
 	u8 date[7];
-	int ctrl, flagreg, ret;
+	int ctrl, flagreg, err;
 
 	if ((dt->tm_year < 100) || (dt->tm_year > 199))
 		return -EINVAL;
@@ -150,10 +150,10 @@ static int rx8010_set_time(struct device *dev, struct rtc_time *dt)
 	if (ctrl < 0)
 		return ctrl;
 	rx8010->ctrlreg = ctrl | RX8010_CTRL_STOP;
-	ret = i2c_smbus_write_byte_data(rx8010->client, RX8010_CTRL,
+	err = i2c_smbus_write_byte_data(rx8010->client, RX8010_CTRL,
 					rx8010->ctrlreg);
-	if (ret < 0)
-		return ret;
+	if (err < 0)
+		return err;
 
 	date[RX8010_SEC - RX8010_SEC] = bin2bcd(dt->tm_sec);
 	date[RX8010_MIN - RX8010_SEC] = bin2bcd(dt->tm_min);
@@ -163,27 +163,27 @@ static int rx8010_set_time(struct device *dev, struct rtc_time *dt)
 	date[RX8010_YEAR - RX8010_SEC] = bin2bcd(dt->tm_year - 100);
 	date[RX8010_WDAY - RX8010_SEC] = bin2bcd(1 << dt->tm_wday);
 
-	ret = i2c_smbus_write_i2c_block_data(rx8010->client,
+	err = i2c_smbus_write_i2c_block_data(rx8010->client,
 					     RX8010_SEC, 7, date);
-	if (ret < 0)
-		return ret;
+	if (err < 0)
+		return err;
 
 	/* clear STOP bit after changing clock/calendar */
 	ctrl = i2c_smbus_read_byte_data(rx8010->client, RX8010_CTRL);
 	if (ctrl < 0)
 		return ctrl;
 	rx8010->ctrlreg = ctrl & ~RX8010_CTRL_STOP;
-	ret = i2c_smbus_write_byte_data(rx8010->client, RX8010_CTRL,
+	err = i2c_smbus_write_byte_data(rx8010->client, RX8010_CTRL,
 					rx8010->ctrlreg);
-	if (ret < 0)
-		return ret;
+	if (err < 0)
+		return err;
 
 	flagreg = i2c_smbus_read_byte_data(rx8010->client, RX8010_FLAG);
 	if (flagreg < 0)
 		return flagreg;
 
 	if (flagreg & RX8010_FLAG_VLF)
-		ret = i2c_smbus_write_byte_data(rx8010->client, RX8010_FLAG,
+		err = i2c_smbus_write_byte_data(rx8010->client, RX8010_FLAG,
 						flagreg & ~RX8010_FLAG_VLF);
 
 	return 0;
-- 
2.26.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v3 07/14] rtc: rx8010: don't use magic values for time buffer length
  2020-09-14 15:45 [PATCH v3 00/14] rtc: rx8010: use regmap instead of i2c smbus API Bartosz Golaszewski
                   ` (5 preceding siblings ...)
  2020-09-14 15:45 ` [PATCH v3 06/14] rtc: rx8010: rename ret to err in rx8010_set_time() Bartosz Golaszewski
@ 2020-09-14 15:45 ` Bartosz Golaszewski
  2020-09-14 15:45 ` [PATCH v3 08/14] rtc: rx8010: drop unnecessary initialization Bartosz Golaszewski
                   ` (7 subsequent siblings)
  14 siblings, 0 replies; 18+ messages in thread
From: Bartosz Golaszewski @ 2020-09-14 15:45 UTC (permalink / raw)
  To: Alessandro Zummo, Alexandre Belloni
  Cc: linux-rtc, linux-kernel, Bartosz Golaszewski

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

The time buffer len is used directly in this driver. For readability
it's better to define it as the difference between the date register
offsets and use sizeof() whenever referencing it.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 drivers/rtc/rtc-rx8010.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/rtc/rtc-rx8010.c b/drivers/rtc/rtc-rx8010.c
index 300314ab7b6d..2c894e7aab6d 100644
--- a/drivers/rtc/rtc-rx8010.c
+++ b/drivers/rtc/rtc-rx8010.c
@@ -108,7 +108,7 @@ static irqreturn_t rx8010_irq_1_handler(int irq, void *dev_id)
 static int rx8010_get_time(struct device *dev, struct rtc_time *dt)
 {
 	struct rx8010_data *rx8010 = dev_get_drvdata(dev);
-	u8 date[7];
+	u8 date[RX8010_YEAR - RX8010_SEC + 1];
 	int flagreg, err;
 
 	flagreg = i2c_smbus_read_byte_data(rx8010->client, RX8010_FLAG);
@@ -121,8 +121,8 @@ static int rx8010_get_time(struct device *dev, struct rtc_time *dt)
 	}
 
 	err = i2c_smbus_read_i2c_block_data(rx8010->client, RX8010_SEC,
-					    7, date);
-	if (err != 7)
+					    sizeof(date), date);
+	if (err != sizeof(date))
 		return err < 0 ? err : -EIO;
 
 	dt->tm_sec = bcd2bin(date[RX8010_SEC - RX8010_SEC] & 0x7f);
@@ -139,7 +139,7 @@ static int rx8010_get_time(struct device *dev, struct rtc_time *dt)
 static int rx8010_set_time(struct device *dev, struct rtc_time *dt)
 {
 	struct rx8010_data *rx8010 = dev_get_drvdata(dev);
-	u8 date[7];
+	u8 date[RX8010_YEAR - RX8010_SEC + 1];
 	int ctrl, flagreg, err;
 
 	if ((dt->tm_year < 100) || (dt->tm_year > 199))
@@ -164,7 +164,8 @@ static int rx8010_set_time(struct device *dev, struct rtc_time *dt)
 	date[RX8010_WDAY - RX8010_SEC] = bin2bcd(1 << dt->tm_wday);
 
 	err = i2c_smbus_write_i2c_block_data(rx8010->client,
-					     RX8010_SEC, 7, date);
+					     RX8010_SEC, sizeof(date),
+					     date);
 	if (err < 0)
 		return err;
 
-- 
2.26.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v3 08/14] rtc: rx8010: drop unnecessary initialization
  2020-09-14 15:45 [PATCH v3 00/14] rtc: rx8010: use regmap instead of i2c smbus API Bartosz Golaszewski
                   ` (6 preceding siblings ...)
  2020-09-14 15:45 ` [PATCH v3 07/14] rtc: rx8010: don't use magic values for time buffer length Bartosz Golaszewski
@ 2020-09-14 15:45 ` Bartosz Golaszewski
  2020-09-14 15:45 ` [PATCH v3 09/14] rtc: rx8010: use a helper variable for client->dev in probe() Bartosz Golaszewski
                   ` (6 subsequent siblings)
  14 siblings, 0 replies; 18+ messages in thread
From: Bartosz Golaszewski @ 2020-09-14 15:45 UTC (permalink / raw)
  To: Alessandro Zummo, Alexandre Belloni
  Cc: linux-rtc, linux-kernel, Bartosz Golaszewski

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

The 'err' local variable in rx8010_init_client() doesn't need to be
initialized.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 drivers/rtc/rtc-rx8010.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/rtc/rtc-rx8010.c b/drivers/rtc/rtc-rx8010.c
index 2c894e7aab6d..64a9964eb5e0 100644
--- a/drivers/rtc/rtc-rx8010.c
+++ b/drivers/rtc/rtc-rx8010.c
@@ -194,7 +194,7 @@ static int rx8010_init_client(struct i2c_client *client)
 {
 	struct rx8010_data *rx8010 = i2c_get_clientdata(client);
 	u8 ctrl[2];
-	int need_clear = 0, err = 0;
+	int need_clear = 0, err;
 
 	/* Initialize reserved registers as specified in datasheet */
 	err = i2c_smbus_write_byte_data(client, RX8010_RESV17, 0xD8);
-- 
2.26.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v3 09/14] rtc: rx8010: use a helper variable for client->dev in probe()
  2020-09-14 15:45 [PATCH v3 00/14] rtc: rx8010: use regmap instead of i2c smbus API Bartosz Golaszewski
                   ` (7 preceding siblings ...)
  2020-09-14 15:45 ` [PATCH v3 08/14] rtc: rx8010: drop unnecessary initialization Bartosz Golaszewski
@ 2020-09-14 15:45 ` Bartosz Golaszewski
  2020-09-14 15:45 ` [PATCH v3 10/14] rtc: rx8010: prefer sizeof(*val) over sizeof(struct type_of_val) Bartosz Golaszewski
                   ` (5 subsequent siblings)
  14 siblings, 0 replies; 18+ messages in thread
From: Bartosz Golaszewski @ 2020-09-14 15:45 UTC (permalink / raw)
  To: Alessandro Zummo, Alexandre Belloni
  Cc: linux-rtc, linux-kernel, Bartosz Golaszewski

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Simple 'dev' looks better then repeated &client->dev and has the added
benefit of avoiding unnecessary line breaks.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 drivers/rtc/rtc-rx8010.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/rtc/rtc-rx8010.c b/drivers/rtc/rtc-rx8010.c
index 64a9964eb5e0..dba7c3f87d9e 100644
--- a/drivers/rtc/rtc-rx8010.c
+++ b/drivers/rtc/rtc-rx8010.c
@@ -419,6 +419,7 @@ static int rx8010_probe(struct i2c_client *client,
 {
 	struct i2c_adapter *adapter = client->adapter;
 	const struct rtc_class_ops *rtc_ops;
+	struct device *dev = &client->dev;
 	struct rx8010_data *rx8010;
 	int err = 0;
 
@@ -428,8 +429,7 @@ static int rx8010_probe(struct i2c_client *client,
 		return -EIO;
 	}
 
-	rx8010 = devm_kzalloc(&client->dev, sizeof(struct rx8010_data),
-			      GFP_KERNEL);
+	rx8010 = devm_kzalloc(dev, sizeof(struct rx8010_data), GFP_KERNEL);
 	if (!rx8010)
 		return -ENOMEM;
 
@@ -441,13 +441,13 @@ static int rx8010_probe(struct i2c_client *client,
 		return err;
 
 	if (client->irq > 0) {
-		dev_info(&client->dev, "IRQ %d supplied\n", client->irq);
-		err = devm_request_threaded_irq(&client->dev, client->irq, NULL,
+		dev_info(dev, "IRQ %d supplied\n", client->irq);
+		err = devm_request_threaded_irq(dev, client->irq, NULL,
 						rx8010_irq_1_handler,
 						IRQF_TRIGGER_LOW | IRQF_ONESHOT,
 						"rx8010", client);
 		if (err) {
-			dev_err(&client->dev, "unable to request IRQ\n");
+			dev_err(dev, "unable to request IRQ\n");
 			return err;
 		}
 
@@ -456,11 +456,10 @@ static int rx8010_probe(struct i2c_client *client,
 		rtc_ops = &rx8010_rtc_ops_default;
 	}
 
-	rx8010->rtc = devm_rtc_device_register(&client->dev, client->name,
+	rx8010->rtc = devm_rtc_device_register(dev, client->name,
 					       rtc_ops, THIS_MODULE);
-
 	if (IS_ERR(rx8010->rtc)) {
-		dev_err(&client->dev, "unable to register the class device\n");
+		dev_err(dev, "unable to register the class device\n");
 		return PTR_ERR(rx8010->rtc);
 	}
 
-- 
2.26.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v3 10/14] rtc: rx8010: prefer sizeof(*val) over sizeof(struct type_of_val)
  2020-09-14 15:45 [PATCH v3 00/14] rtc: rx8010: use regmap instead of i2c smbus API Bartosz Golaszewski
                   ` (8 preceding siblings ...)
  2020-09-14 15:45 ` [PATCH v3 09/14] rtc: rx8010: use a helper variable for client->dev in probe() Bartosz Golaszewski
@ 2020-09-14 15:45 ` Bartosz Golaszewski
  2020-09-14 15:45 ` [PATCH v3 11/14] rtc: rx8010: switch to using the preferred RTC API Bartosz Golaszewski
                   ` (4 subsequent siblings)
  14 siblings, 0 replies; 18+ messages in thread
From: Bartosz Golaszewski @ 2020-09-14 15:45 UTC (permalink / raw)
  To: Alessandro Zummo, Alexandre Belloni
  Cc: linux-rtc, linux-kernel, Bartosz Golaszewski

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Using the size of the variable is preferred over using the size of its
type when allocating memory. Convert the call to devm_kzalloc() in
probe().

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 drivers/rtc/rtc-rx8010.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/rtc/rtc-rx8010.c b/drivers/rtc/rtc-rx8010.c
index dba7c3f87d9e..aa357f800ad4 100644
--- a/drivers/rtc/rtc-rx8010.c
+++ b/drivers/rtc/rtc-rx8010.c
@@ -429,7 +429,7 @@ static int rx8010_probe(struct i2c_client *client,
 		return -EIO;
 	}
 
-	rx8010 = devm_kzalloc(dev, sizeof(struct rx8010_data), GFP_KERNEL);
+	rx8010 = devm_kzalloc(dev, sizeof(*rx8010), GFP_KERNEL);
 	if (!rx8010)
 		return -ENOMEM;
 
-- 
2.26.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v3 11/14] rtc: rx8010: switch to using the preferred RTC API
  2020-09-14 15:45 [PATCH v3 00/14] rtc: rx8010: use regmap instead of i2c smbus API Bartosz Golaszewski
                   ` (9 preceding siblings ...)
  2020-09-14 15:45 ` [PATCH v3 10/14] rtc: rx8010: prefer sizeof(*val) over sizeof(struct type_of_val) Bartosz Golaszewski
@ 2020-09-14 15:45 ` Bartosz Golaszewski
  2020-09-14 15:45 ` [PATCH v3 12/14] rtc: rx8010: switch to using the preferred i2c API Bartosz Golaszewski
                   ` (3 subsequent siblings)
  14 siblings, 0 replies; 18+ messages in thread
From: Bartosz Golaszewski @ 2020-09-14 15:45 UTC (permalink / raw)
  To: Alessandro Zummo, Alexandre Belloni
  Cc: linux-rtc, linux-kernel, Bartosz Golaszewski

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Use devm_rtc_allocate_device() + rtc_register_device() instead of the
deprecated devm_rtc_device_register().

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 drivers/rtc/rtc-rx8010.c | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/drivers/rtc/rtc-rx8010.c b/drivers/rtc/rtc-rx8010.c
index aa357f800ad4..6aeed3802670 100644
--- a/drivers/rtc/rtc-rx8010.c
+++ b/drivers/rtc/rtc-rx8010.c
@@ -418,7 +418,6 @@ static int rx8010_probe(struct i2c_client *client,
 			const struct i2c_device_id *id)
 {
 	struct i2c_adapter *adapter = client->adapter;
-	const struct rtc_class_ops *rtc_ops;
 	struct device *dev = &client->dev;
 	struct rx8010_data *rx8010;
 	int err = 0;
@@ -440,6 +439,10 @@ static int rx8010_probe(struct i2c_client *client,
 	if (err)
 		return err;
 
+	rx8010->rtc = devm_rtc_allocate_device(dev);
+	if (IS_ERR(rx8010->rtc))
+		return PTR_ERR(rx8010->rtc);
+
 	if (client->irq > 0) {
 		dev_info(dev, "IRQ %d supplied\n", client->irq);
 		err = devm_request_threaded_irq(dev, client->irq, NULL,
@@ -451,21 +454,14 @@ static int rx8010_probe(struct i2c_client *client,
 			return err;
 		}
 
-		rtc_ops = &rx8010_rtc_ops_alarm;
+		rx8010->rtc->ops = &rx8010_rtc_ops_alarm;
 	} else {
-		rtc_ops = &rx8010_rtc_ops_default;
-	}
-
-	rx8010->rtc = devm_rtc_device_register(dev, client->name,
-					       rtc_ops, THIS_MODULE);
-	if (IS_ERR(rx8010->rtc)) {
-		dev_err(dev, "unable to register the class device\n");
-		return PTR_ERR(rx8010->rtc);
+		rx8010->rtc->ops = &rx8010_rtc_ops_default;
 	}
 
 	rx8010->rtc->max_user_freq = 1;
 
-	return 0;
+	return rtc_register_device(rx8010->rtc);
 }
 
 static struct i2c_driver rx8010_driver = {
-- 
2.26.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v3 12/14] rtc: rx8010: switch to using the preferred i2c API
  2020-09-14 15:45 [PATCH v3 00/14] rtc: rx8010: use regmap instead of i2c smbus API Bartosz Golaszewski
                   ` (10 preceding siblings ...)
  2020-09-14 15:45 ` [PATCH v3 11/14] rtc: rx8010: switch to using the preferred RTC API Bartosz Golaszewski
@ 2020-09-14 15:45 ` Bartosz Golaszewski
  2020-09-14 15:46 ` [PATCH v3 13/14] rtc: rx8010: convert to using regmap Bartosz Golaszewski
                   ` (2 subsequent siblings)
  14 siblings, 0 replies; 18+ messages in thread
From: Bartosz Golaszewski @ 2020-09-14 15:45 UTC (permalink / raw)
  To: Alessandro Zummo, Alexandre Belloni
  Cc: linux-rtc, linux-kernel, Bartosz Golaszewski

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

We should generally use probe_new instead of probe when registering i2c
drivers. Convert rx8010 to using it.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 drivers/rtc/rtc-rx8010.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/rtc/rtc-rx8010.c b/drivers/rtc/rtc-rx8010.c
index 6aeed3802670..3c82f7d48a65 100644
--- a/drivers/rtc/rtc-rx8010.c
+++ b/drivers/rtc/rtc-rx8010.c
@@ -414,8 +414,7 @@ static const struct rtc_class_ops rx8010_rtc_ops_alarm = {
 	.alarm_irq_enable = rx8010_alarm_irq_enable,
 };
 
-static int rx8010_probe(struct i2c_client *client,
-			const struct i2c_device_id *id)
+static int rx8010_probe(struct i2c_client *client)
 {
 	struct i2c_adapter *adapter = client->adapter;
 	struct device *dev = &client->dev;
@@ -469,7 +468,7 @@ static struct i2c_driver rx8010_driver = {
 		.name = "rtc-rx8010",
 		.of_match_table = of_match_ptr(rx8010_of_match),
 	},
-	.probe		= rx8010_probe,
+	.probe_new	= rx8010_probe,
 	.id_table	= rx8010_id,
 };
 
-- 
2.26.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v3 13/14] rtc: rx8010: convert to using regmap
  2020-09-14 15:45 [PATCH v3 00/14] rtc: rx8010: use regmap instead of i2c smbus API Bartosz Golaszewski
                   ` (11 preceding siblings ...)
  2020-09-14 15:45 ` [PATCH v3 12/14] rtc: rx8010: switch to using the preferred i2c API Bartosz Golaszewski
@ 2020-09-14 15:46 ` Bartosz Golaszewski
  2020-09-14 15:46 ` [PATCH v3 14/14] rtc: rx8010: use range checking provided by core RTC code Bartosz Golaszewski
  2020-09-15  9:20 ` [PATCH v3 00/14] rtc: rx8010: use regmap instead of i2c smbus API Alexandre Belloni
  14 siblings, 0 replies; 18+ messages in thread
From: Bartosz Golaszewski @ 2020-09-14 15:46 UTC (permalink / raw)
  To: Alessandro Zummo, Alexandre Belloni
  Cc: linux-rtc, linux-kernel, Bartosz Golaszewski

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

This driver requires SMBUS to work. We can relax this requirement if we
switch to using i2c regmap and let the regmap sub-system figure out how
to talk to the bus.

This also has the advantage of shrinking the code for register updates.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 drivers/rtc/rtc-rx8010.c | 198 ++++++++++++++++-----------------------
 1 file changed, 79 insertions(+), 119 deletions(-)

diff --git a/drivers/rtc/rtc-rx8010.c b/drivers/rtc/rtc-rx8010.c
index 3c82f7d48a65..b8aa98fb62de 100644
--- a/drivers/rtc/rtc-rx8010.c
+++ b/drivers/rtc/rtc-rx8010.c
@@ -11,6 +11,7 @@
 #include <linux/i2c.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/regmap.h>
 #include <linux/rtc.h>
 
 #define RX8010_SEC		0x10
@@ -61,7 +62,7 @@ static const struct of_device_id rx8010_of_match[] = {
 MODULE_DEVICE_TABLE(of, rx8010_of_match);
 
 struct rx8010_data {
-	struct i2c_client *client;
+	struct regmap *regs;
 	struct rtc_device *rtc;
 	u8 ctrlreg;
 };
@@ -70,13 +71,12 @@ static irqreturn_t rx8010_irq_1_handler(int irq, void *dev_id)
 {
 	struct i2c_client *client = dev_id;
 	struct rx8010_data *rx8010 = i2c_get_clientdata(client);
-	int flagreg;
+	int flagreg, err;
 
 	mutex_lock(&rx8010->rtc->ops_lock);
 
-	flagreg = i2c_smbus_read_byte_data(client, RX8010_FLAG);
-
-	if (flagreg <= 0) {
+	err = regmap_read(rx8010->regs, RX8010_FLAG, &flagreg);
+	if (err) {
 		mutex_unlock(&rx8010->rtc->ops_lock);
 		return IRQ_NONE;
 	}
@@ -99,10 +99,9 @@ static irqreturn_t rx8010_irq_1_handler(int irq, void *dev_id)
 		rtc_update_irq(rx8010->rtc, 1, RTC_UF | RTC_IRQF);
 	}
 
-	i2c_smbus_write_byte_data(client, RX8010_FLAG, flagreg);
-
+	err = regmap_write(rx8010->regs, RX8010_FLAG, flagreg);
 	mutex_unlock(&rx8010->rtc->ops_lock);
-	return IRQ_HANDLED;
+	return err ? IRQ_NONE : IRQ_HANDLED;
 }
 
 static int rx8010_get_time(struct device *dev, struct rtc_time *dt)
@@ -111,19 +110,18 @@ static int rx8010_get_time(struct device *dev, struct rtc_time *dt)
 	u8 date[RX8010_YEAR - RX8010_SEC + 1];
 	int flagreg, err;
 
-	flagreg = i2c_smbus_read_byte_data(rx8010->client, RX8010_FLAG);
-	if (flagreg < 0)
-		return flagreg;
+	err = regmap_read(rx8010->regs, RX8010_FLAG, &flagreg);
+	if (err)
+		return err;
 
 	if (flagreg & RX8010_FLAG_VLF) {
 		dev_warn(dev, "Frequency stop detected\n");
 		return -EINVAL;
 	}
 
-	err = i2c_smbus_read_i2c_block_data(rx8010->client, RX8010_SEC,
-					    sizeof(date), date);
-	if (err != sizeof(date))
-		return err < 0 ? err : -EIO;
+	err = regmap_bulk_read(rx8010->regs, RX8010_SEC, date, sizeof(date));
+	if (err)
+		return err;
 
 	dt->tm_sec = bcd2bin(date[RX8010_SEC - RX8010_SEC] & 0x7f);
 	dt->tm_min = bcd2bin(date[RX8010_MIN - RX8010_SEC] & 0x7f);
@@ -140,19 +138,14 @@ static int rx8010_set_time(struct device *dev, struct rtc_time *dt)
 {
 	struct rx8010_data *rx8010 = dev_get_drvdata(dev);
 	u8 date[RX8010_YEAR - RX8010_SEC + 1];
-	int ctrl, flagreg, err;
+	int err;
 
 	if ((dt->tm_year < 100) || (dt->tm_year > 199))
 		return -EINVAL;
 
 	/* set STOP bit before changing clock/calendar */
-	ctrl = i2c_smbus_read_byte_data(rx8010->client, RX8010_CTRL);
-	if (ctrl < 0)
-		return ctrl;
-	rx8010->ctrlreg = ctrl | RX8010_CTRL_STOP;
-	err = i2c_smbus_write_byte_data(rx8010->client, RX8010_CTRL,
-					rx8010->ctrlreg);
-	if (err < 0)
+	err = regmap_set_bits(rx8010->regs, RX8010_CTRL, RX8010_CTRL_STOP);
+	if (err)
 		return err;
 
 	date[RX8010_SEC - RX8010_SEC] = bin2bcd(dt->tm_sec);
@@ -163,66 +156,54 @@ static int rx8010_set_time(struct device *dev, struct rtc_time *dt)
 	date[RX8010_YEAR - RX8010_SEC] = bin2bcd(dt->tm_year - 100);
 	date[RX8010_WDAY - RX8010_SEC] = bin2bcd(1 << dt->tm_wday);
 
-	err = i2c_smbus_write_i2c_block_data(rx8010->client,
-					     RX8010_SEC, sizeof(date),
-					     date);
-	if (err < 0)
+	err = regmap_bulk_write(rx8010->regs, RX8010_SEC, date, sizeof(date));
+	if (err)
 		return err;
 
 	/* clear STOP bit after changing clock/calendar */
-	ctrl = i2c_smbus_read_byte_data(rx8010->client, RX8010_CTRL);
-	if (ctrl < 0)
-		return ctrl;
-	rx8010->ctrlreg = ctrl & ~RX8010_CTRL_STOP;
-	err = i2c_smbus_write_byte_data(rx8010->client, RX8010_CTRL,
-					rx8010->ctrlreg);
-	if (err < 0)
+	err = regmap_clear_bits(rx8010->regs, RX8010_CTRL, RX8010_CTRL_STOP);
+	if (err)
 		return err;
 
-	flagreg = i2c_smbus_read_byte_data(rx8010->client, RX8010_FLAG);
-	if (flagreg < 0)
-		return flagreg;
-
-	if (flagreg & RX8010_FLAG_VLF)
-		err = i2c_smbus_write_byte_data(rx8010->client, RX8010_FLAG,
-						flagreg & ~RX8010_FLAG_VLF);
+	err = regmap_clear_bits(rx8010->regs, RX8010_FLAG, RX8010_FLAG_VLF);
+	if (err)
+		return err;
 
 	return 0;
 }
 
-static int rx8010_init_client(struct i2c_client *client)
+static int rx8010_init_client(struct device *dev)
 {
-	struct rx8010_data *rx8010 = i2c_get_clientdata(client);
+	struct rx8010_data *rx8010 = dev_get_drvdata(dev);
 	u8 ctrl[2];
 	int need_clear = 0, err;
 
 	/* Initialize reserved registers as specified in datasheet */
-	err = i2c_smbus_write_byte_data(client, RX8010_RESV17, 0xD8);
-	if (err < 0)
+	err = regmap_write(rx8010->regs, RX8010_RESV17, 0xD8);
+	if (err)
 		return err;
 
-	err = i2c_smbus_write_byte_data(client, RX8010_RESV30, 0x00);
-	if (err < 0)
+	err = regmap_write(rx8010->regs, RX8010_RESV30, 0x00);
+	if (err)
 		return err;
 
-	err = i2c_smbus_write_byte_data(client, RX8010_RESV31, 0x08);
-	if (err < 0)
+	err = regmap_write(rx8010->regs, RX8010_RESV31, 0x08);
+	if (err)
 		return err;
 
-	err = i2c_smbus_write_byte_data(client, RX8010_IRQ, 0x00);
-	if (err < 0)
+	err = regmap_write(rx8010->regs, RX8010_IRQ, 0x00);
+	if (err)
 		return err;
 
-	err = i2c_smbus_read_i2c_block_data(rx8010->client, RX8010_FLAG,
-					    2, ctrl);
-	if (err != 2)
-		return err < 0 ? err : -EIO;
+	err = regmap_bulk_read(rx8010->regs, RX8010_FLAG, ctrl, 2);
+	if (err)
+		return err;
 
 	if (ctrl[0] & RX8010_FLAG_VLF)
-		dev_warn(&client->dev, "Frequency stop was detected\n");
+		dev_warn(dev, "Frequency stop was detected\n");
 
 	if (ctrl[0] & RX8010_FLAG_AF) {
-		dev_warn(&client->dev, "Alarm was detected\n");
+		dev_warn(dev, "Alarm was detected\n");
 		need_clear = 1;
 	}
 
@@ -234,8 +215,8 @@ static int rx8010_init_client(struct i2c_client *client)
 
 	if (need_clear) {
 		ctrl[0] &= ~(RX8010_FLAG_AF | RX8010_FLAG_TF | RX8010_FLAG_UF);
-		err = i2c_smbus_write_byte_data(client, RX8010_FLAG, ctrl[0]);
-		if (err < 0)
+		err = regmap_write(rx8010->regs, RX8010_FLAG, ctrl[0]);
+		if (err)
 			return err;
 	}
 
@@ -247,17 +228,16 @@ static int rx8010_init_client(struct i2c_client *client)
 static int rx8010_read_alarm(struct device *dev, struct rtc_wkalrm *t)
 {
 	struct rx8010_data *rx8010 = dev_get_drvdata(dev);
-	struct i2c_client *client = rx8010->client;
 	u8 alarmvals[3];
 	int flagreg, err;
 
-	err = i2c_smbus_read_i2c_block_data(client, RX8010_ALMIN, 3, alarmvals);
-	if (err != 3)
-		return err < 0 ? err : -EIO;
+	err = regmap_bulk_read(rx8010->regs, RX8010_ALMIN, alarmvals, 3);
+	if (err)
+		return err;
 
-	flagreg = i2c_smbus_read_byte_data(client, RX8010_FLAG);
-	if (flagreg < 0)
-		return flagreg;
+	err = regmap_read(rx8010->regs, RX8010_FLAG, &flagreg);
+	if (err)
+		return err;
 
 	t->time.tm_sec = 0;
 	t->time.tm_min = bcd2bin(alarmvals[0] & 0x7f);
@@ -274,52 +254,38 @@ static int rx8010_read_alarm(struct device *dev, struct rtc_wkalrm *t)
 
 static int rx8010_set_alarm(struct device *dev, struct rtc_wkalrm *t)
 {
-	struct i2c_client *client = to_i2c_client(dev);
 	struct rx8010_data *rx8010 = dev_get_drvdata(dev);
 	u8 alarmvals[3];
-	int extreg, flagreg, err;
-
-	flagreg = i2c_smbus_read_byte_data(client, RX8010_FLAG);
-	if (flagreg < 0)
-		return flagreg;
+	int err;
 
 	if (rx8010->ctrlreg & (RX8010_CTRL_AIE | RX8010_CTRL_UIE)) {
 		rx8010->ctrlreg &= ~(RX8010_CTRL_AIE | RX8010_CTRL_UIE);
-		err = i2c_smbus_write_byte_data(rx8010->client, RX8010_CTRL,
-						rx8010->ctrlreg);
-		if (err < 0)
+		err = regmap_write(rx8010->regs, RX8010_CTRL, rx8010->ctrlreg);
+		if (err)
 			return err;
 	}
 
-	flagreg &= ~RX8010_FLAG_AF;
-	err = i2c_smbus_write_byte_data(rx8010->client, RX8010_FLAG, flagreg);
-	if (err < 0)
+	err = regmap_clear_bits(rx8010->regs, RX8010_FLAG, RX8010_FLAG_AF);
+	if (err)
 		return err;
 
 	alarmvals[0] = bin2bcd(t->time.tm_min);
 	alarmvals[1] = bin2bcd(t->time.tm_hour);
 	alarmvals[2] = bin2bcd(t->time.tm_mday);
 
-	err = i2c_smbus_write_i2c_block_data(rx8010->client, RX8010_ALMIN,
-					     2, alarmvals);
-	if (err < 0)
+	err = regmap_bulk_write(rx8010->regs, RX8010_ALMIN, alarmvals, 2);
+	if (err)
 		return err;
 
-	extreg = i2c_smbus_read_byte_data(client, RX8010_EXT);
-	if (extreg < 0)
-		return extreg;
-
-	extreg |= RX8010_EXT_WADA;
-	err = i2c_smbus_write_byte_data(rx8010->client, RX8010_EXT, extreg);
-	if (err < 0)
+	err = regmap_clear_bits(rx8010->regs, RX8010_EXT, RX8010_EXT_WADA);
+	if (err)
 		return err;
 
 	if (alarmvals[2] == 0)
 		alarmvals[2] |= RX8010_ALARM_AE;
 
-	err = i2c_smbus_write_byte_data(rx8010->client, RX8010_ALWDAY,
-					alarmvals[2]);
-	if (err < 0)
+	err = regmap_write(rx8010->regs, RX8010_ALWDAY, alarmvals[2]);
+	if (err)
 		return err;
 
 	if (t->enabled) {
@@ -329,9 +295,8 @@ static int rx8010_set_alarm(struct device *dev, struct rtc_wkalrm *t)
 			rx8010->ctrlreg |=
 				(RX8010_CTRL_AIE | RX8010_CTRL_UIE);
 
-		err = i2c_smbus_write_byte_data(rx8010->client, RX8010_CTRL,
-						rx8010->ctrlreg);
-		if (err < 0)
+		err = regmap_write(rx8010->regs, RX8010_CTRL, rx8010->ctrlreg);
+		if (err)
 			return err;
 	}
 
@@ -341,9 +306,8 @@ static int rx8010_set_alarm(struct device *dev, struct rtc_wkalrm *t)
 static int rx8010_alarm_irq_enable(struct device *dev,
 				   unsigned int enabled)
 {
-	struct i2c_client *client = to_i2c_client(dev);
 	struct rx8010_data *rx8010 = dev_get_drvdata(dev);
-	int flagreg, err;
+	int err;
 	u8 ctrl;
 
 	ctrl = rx8010->ctrlreg;
@@ -360,20 +324,14 @@ static int rx8010_alarm_irq_enable(struct device *dev,
 			ctrl &= ~RX8010_CTRL_AIE;
 	}
 
-	flagreg = i2c_smbus_read_byte_data(client, RX8010_FLAG);
-	if (flagreg < 0)
-		return flagreg;
-
-	flagreg &= ~RX8010_FLAG_AF;
-	err = i2c_smbus_write_byte_data(rx8010->client, RX8010_FLAG, flagreg);
-	if (err < 0)
+	err = regmap_clear_bits(rx8010->regs, RX8010_FLAG, RX8010_FLAG_AF);
+	if (err)
 		return err;
 
 	if (ctrl != rx8010->ctrlreg) {
 		rx8010->ctrlreg = ctrl;
-		err = i2c_smbus_write_byte_data(rx8010->client, RX8010_CTRL,
-						rx8010->ctrlreg);
-		if (err < 0)
+		err = regmap_write(rx8010->regs, RX8010_CTRL, rx8010->ctrlreg);
+		if (err)
 			return err;
 	}
 
@@ -383,13 +341,13 @@ static int rx8010_alarm_irq_enable(struct device *dev,
 static int rx8010_ioctl(struct device *dev, unsigned int cmd, unsigned long arg)
 {
 	struct rx8010_data *rx8010 = dev_get_drvdata(dev);
-	int tmp, flagreg;
+	int tmp, flagreg, err;
 
 	switch (cmd) {
 	case RTC_VL_READ:
-		flagreg = i2c_smbus_read_byte_data(rx8010->client, RX8010_FLAG);
-		if (flagreg < 0)
-			return flagreg;
+		err = regmap_read(rx8010->regs, RX8010_FLAG, &flagreg);
+		if (err)
+			return err;
 
 		tmp = flagreg & RX8010_FLAG_VLF ? RTC_VL_DATA_INVALID : 0;
 		return put_user(tmp, (unsigned int __user *)arg);
@@ -414,27 +372,29 @@ static const struct rtc_class_ops rx8010_rtc_ops_alarm = {
 	.alarm_irq_enable = rx8010_alarm_irq_enable,
 };
 
+static const struct regmap_config rx8010_regmap_config = {
+	.name = "rx8010-rtc",
+	.reg_bits = 8,
+	.val_bits = 8,
+};
+
 static int rx8010_probe(struct i2c_client *client)
 {
-	struct i2c_adapter *adapter = client->adapter;
 	struct device *dev = &client->dev;
 	struct rx8010_data *rx8010;
 	int err = 0;
 
-	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA
-		| I2C_FUNC_SMBUS_I2C_BLOCK)) {
-		dev_err(&adapter->dev, "doesn't support required functionality\n");
-		return -EIO;
-	}
-
 	rx8010 = devm_kzalloc(dev, sizeof(*rx8010), GFP_KERNEL);
 	if (!rx8010)
 		return -ENOMEM;
 
-	rx8010->client = client;
 	i2c_set_clientdata(client, rx8010);
 
-	err = rx8010_init_client(client);
+	rx8010->regs = devm_regmap_init_i2c(client, &rx8010_regmap_config);
+	if (IS_ERR(rx8010->regs))
+		return PTR_ERR(rx8010->regs);
+
+	err = rx8010_init_client(dev);
 	if (err)
 		return err;
 
-- 
2.26.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v3 14/14] rtc: rx8010: use range checking provided by core RTC code
  2020-09-14 15:45 [PATCH v3 00/14] rtc: rx8010: use regmap instead of i2c smbus API Bartosz Golaszewski
                   ` (12 preceding siblings ...)
  2020-09-14 15:46 ` [PATCH v3 13/14] rtc: rx8010: convert to using regmap Bartosz Golaszewski
@ 2020-09-14 15:46 ` Bartosz Golaszewski
  2020-09-15  9:20 ` [PATCH v3 00/14] rtc: rx8010: use regmap instead of i2c smbus API Alexandre Belloni
  14 siblings, 0 replies; 18+ messages in thread
From: Bartosz Golaszewski @ 2020-09-14 15:46 UTC (permalink / raw)
  To: Alessandro Zummo, Alexandre Belloni
  Cc: linux-rtc, linux-kernel, Bartosz Golaszewski

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

We don't need to check the time range manually in set_time(), we can
use range_min and range_max exposed by struct rtc_device.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 drivers/rtc/rtc-rx8010.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/rtc/rtc-rx8010.c b/drivers/rtc/rtc-rx8010.c
index b8aa98fb62de..01e9017d4025 100644
--- a/drivers/rtc/rtc-rx8010.c
+++ b/drivers/rtc/rtc-rx8010.c
@@ -140,9 +140,6 @@ static int rx8010_set_time(struct device *dev, struct rtc_time *dt)
 	u8 date[RX8010_YEAR - RX8010_SEC + 1];
 	int err;
 
-	if ((dt->tm_year < 100) || (dt->tm_year > 199))
-		return -EINVAL;
-
 	/* set STOP bit before changing clock/calendar */
 	err = regmap_set_bits(rx8010->regs, RX8010_CTRL, RX8010_CTRL_STOP);
 	if (err)
@@ -419,6 +416,8 @@ static int rx8010_probe(struct i2c_client *client)
 	}
 
 	rx8010->rtc->max_user_freq = 1;
+	rx8010->rtc->range_min = RTC_TIMESTAMP_BEGIN_2000;
+	rx8010->rtc->range_max = RTC_TIMESTAMP_END_2099;
 
 	return rtc_register_device(rx8010->rtc);
 }
-- 
2.26.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 00/14] rtc: rx8010: use regmap instead of i2c smbus API
  2020-09-14 15:45 [PATCH v3 00/14] rtc: rx8010: use regmap instead of i2c smbus API Bartosz Golaszewski
                   ` (13 preceding siblings ...)
  2020-09-14 15:46 ` [PATCH v3 14/14] rtc: rx8010: use range checking provided by core RTC code Bartosz Golaszewski
@ 2020-09-15  9:20 ` Alexandre Belloni
  14 siblings, 0 replies; 18+ messages in thread
From: Alexandre Belloni @ 2020-09-15  9:20 UTC (permalink / raw)
  To: Bartosz Golaszewski, Alessandro Zummo
  Cc: Alexandre Belloni, linux-rtc, linux-kernel, Bartosz Golaszewski

On Mon, 14 Sep 2020 17:45:47 +0200, Bartosz Golaszewski wrote:
> This series gets bigger and bigger but I noticed a problem with this
> driver that looks like stable material so I fixed it as the first patch
> in the series to make backporting easy.
> 
> Other than that, there are new refactoring patches and I removed the
> unnecessary error messages.
> 
> [...]

Applied, thanks!

[01/14] rtc: rx8010: don't modify the global rtc ops
        commit: d3b14296da69adb7825022f3224ac6137eb30abf
[02/14] rtc: rx8010: remove a stray newline
        commit: 2e0ce569102ccb1ca9bacc499c8411fb8fa53069
[03/14] rtc: rx8010: remove unnecessary brackets
        commit: 28c86f30c979f9d4460dd7680610c3470b4d009b
[04/14] rtc: rx8010: consolidate local variables of the same type
        commit: 75677971991940581e76bcd5176ea40d0baf8fcd
[05/14] rtc: rx8010: use tabs instead of spaces for code formatting
        commit: e9e4c2dae4313b88c62ee9df9d177a71c23121b2
[06/14] rtc: rx8010: rename ret to err in rx8010_set_time()
        commit: 13952c9e35384fd7f63a5ce8261108695491bb56
[07/14] rtc: rx8010: don't use magic values for time buffer length
        commit: f702699c67d315e4a232c64801b2de9af87fd9f4
[08/14] rtc: rx8010: drop unnecessary initialization
        commit: b3ff7fd68d925de2159a5312f28dcd178d0d3715
[09/14] rtc: rx8010: use a helper variable for client->dev in probe()
        commit: 955a123c14906e3adc43d43281f8fde91f631f7f
[10/14] rtc: rx8010: prefer sizeof(*val) over sizeof(struct type_of_val)
        commit: 666f21413b881e159efaf862f119d4d058fa2c4a
[11/14] rtc: rx8010: switch to using the preferred RTC API
        commit: 0ce627785afa730d8f6568eb8738d1700cbc4569
[12/14] rtc: rx8010: switch to using the preferred i2c API
        commit: cee015d90d96495d8376871af0f1a33027303d5e
[13/14] rtc: rx8010: convert to using regmap
        commit: 9868bc1ce272dc0387488e779c585e7a12cf7a1b
[14/14] rtc: rx8010: use range checking provided by core RTC code
        commit: 2fc1af3095af5cbcd8fc406610dc196b62e3ed21

Best regards,
-- 
Alexandre Belloni <alexandre.belloni@bootlin.com>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 01/14] rtc: rx8010: don't modify the global rtc ops
  2020-09-14 15:45 ` [PATCH v3 01/14] rtc: rx8010: don't modify the global rtc ops Bartosz Golaszewski
@ 2020-09-17 15:53   ` Sasha Levin
  2020-09-17 16:16     ` Bartosz Golaszewski
  0 siblings, 1 reply; 18+ messages in thread
From: Sasha Levin @ 2020-09-17 15:53 UTC (permalink / raw)
  To: Sasha Levin, Bartosz Golaszewski, Bartosz Golaszewski, Alessandro Zummo
  Cc: linux-rtc, linux-kernel, stable, stable

Hi

[This is an automated email]

This commit has been processed because it contains a "Fixes:" tag
fixing commit: ed13d89b08e3 ("rtc: Add Epson RX8010SJ RTC driver").

The bot has tested the following trees: v5.8.9, v5.4.65, v4.19.145, v4.14.198, v4.9.236.

v5.8.9: Build OK!
v5.4.65: Build OK!
v4.19.145: Failed to apply! Possible dependencies:
    9d085c54202d ("rtc: rx8010: simplify getting the adapter of a client")

v4.14.198: Failed to apply! Possible dependencies:
    9d085c54202d ("rtc: rx8010: simplify getting the adapter of a client")

v4.9.236: Failed to apply! Possible dependencies:
    9d085c54202d ("rtc: rx8010: simplify getting the adapter of a client")


NOTE: The patch will not be queued to stable trees until it is upstream.

How should we proceed with this patch?

-- 
Thanks
Sasha

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 01/14] rtc: rx8010: don't modify the global rtc ops
  2020-09-17 15:53   ` Sasha Levin
@ 2020-09-17 16:16     ` Bartosz Golaszewski
  0 siblings, 0 replies; 18+ messages in thread
From: Bartosz Golaszewski @ 2020-09-17 16:16 UTC (permalink / raw)
  To: Sasha Levin
  Cc: Bartosz Golaszewski, Alessandro Zummo, linux-rtc, LKML, Stable # 4 . 20+

On Thu, Sep 17, 2020 at 5:53 PM Sasha Levin <sashal@kernel.org> wrote:
>
> Hi
>
> [This is an automated email]
>
> This commit has been processed because it contains a "Fixes:" tag
> fixing commit: ed13d89b08e3 ("rtc: Add Epson RX8010SJ RTC driver").
>
> The bot has tested the following trees: v5.8.9, v5.4.65, v4.19.145, v4.14.198, v4.9.236.
>
> v5.8.9: Build OK!
> v5.4.65: Build OK!
> v4.19.145: Failed to apply! Possible dependencies:
>     9d085c54202d ("rtc: rx8010: simplify getting the adapter of a client")
>
> v4.14.198: Failed to apply! Possible dependencies:
>     9d085c54202d ("rtc: rx8010: simplify getting the adapter of a client")
>
> v4.9.236: Failed to apply! Possible dependencies:
>     9d085c54202d ("rtc: rx8010: simplify getting the adapter of a client")
>
>
> NOTE: The patch will not be queued to stable trees until it is upstream.
>
> How should we proceed with this patch?
>
> --
> Thanks
> Sasha

I sent out a backport for v4.X branches.

Bartosz

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2020-09-17 19:42 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-14 15:45 [PATCH v3 00/14] rtc: rx8010: use regmap instead of i2c smbus API Bartosz Golaszewski
2020-09-14 15:45 ` [PATCH v3 01/14] rtc: rx8010: don't modify the global rtc ops Bartosz Golaszewski
2020-09-17 15:53   ` Sasha Levin
2020-09-17 16:16     ` Bartosz Golaszewski
2020-09-14 15:45 ` [PATCH v3 02/14] rtc: rx8010: remove a stray newline Bartosz Golaszewski
2020-09-14 15:45 ` [PATCH v3 03/14] rtc: rx8010: remove unnecessary brackets Bartosz Golaszewski
2020-09-14 15:45 ` [PATCH v3 04/14] rtc: rx8010: consolidate local variables of the same type Bartosz Golaszewski
2020-09-14 15:45 ` [PATCH v3 05/14] rtc: rx8010: use tabs instead of spaces for code formatting Bartosz Golaszewski
2020-09-14 15:45 ` [PATCH v3 06/14] rtc: rx8010: rename ret to err in rx8010_set_time() Bartosz Golaszewski
2020-09-14 15:45 ` [PATCH v3 07/14] rtc: rx8010: don't use magic values for time buffer length Bartosz Golaszewski
2020-09-14 15:45 ` [PATCH v3 08/14] rtc: rx8010: drop unnecessary initialization Bartosz Golaszewski
2020-09-14 15:45 ` [PATCH v3 09/14] rtc: rx8010: use a helper variable for client->dev in probe() Bartosz Golaszewski
2020-09-14 15:45 ` [PATCH v3 10/14] rtc: rx8010: prefer sizeof(*val) over sizeof(struct type_of_val) Bartosz Golaszewski
2020-09-14 15:45 ` [PATCH v3 11/14] rtc: rx8010: switch to using the preferred RTC API Bartosz Golaszewski
2020-09-14 15:45 ` [PATCH v3 12/14] rtc: rx8010: switch to using the preferred i2c API Bartosz Golaszewski
2020-09-14 15:46 ` [PATCH v3 13/14] rtc: rx8010: convert to using regmap Bartosz Golaszewski
2020-09-14 15:46 ` [PATCH v3 14/14] rtc: rx8010: use range checking provided by core RTC code Bartosz Golaszewski
2020-09-15  9:20 ` [PATCH v3 00/14] rtc: rx8010: use regmap instead of i2c smbus API Alexandre Belloni

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).