* [PATCH 0/8] rtc: rx8010: use regmap instead of i2c smbus API @ 2020-09-04 15:21 Bartosz Golaszewski 2020-09-04 15:21 ` [PATCH 1/8] rtc: rx8010: remove unnecessary parentheses Bartosz Golaszewski ` (7 more replies) 0 siblings, 8 replies; 16+ messages in thread From: Bartosz Golaszewski @ 2020-09-04 15:21 UTC (permalink / raw) To: Alessandro Zummo, Alexandre Belloni Cc: linux-rtc, linux-kernel, Bartosz Golaszewski From: Bartosz Golaszewski <bgolaszewski@baylibre.com> 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. First 7 patches are just cleanups and some refactoring. The actual conversion happens in patch 8. Bartosz Golaszewski (8): rtc: rx8010: remove unnecessary parentheses rtc: rx8010: consolidate local variables of the same type rtc: rx8010: use tabs for 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: fix indentation in probe() rtc: rx8010: convert to using regmap drivers/rtc/rtc-rx8010.c | 284 +++++++++++++++++---------------------- 1 file changed, 120 insertions(+), 164 deletions(-) -- 2.26.1 ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 1/8] rtc: rx8010: remove unnecessary parentheses 2020-09-04 15:21 [PATCH 0/8] rtc: rx8010: use regmap instead of i2c smbus API Bartosz Golaszewski @ 2020-09-04 15:21 ` Bartosz Golaszewski 2020-09-04 15:32 ` Alexandre Belloni 2020-09-04 15:21 ` [PATCH 2/8] rtc: rx8010: consolidate local variables of the same type Bartosz Golaszewski ` (6 subsequent siblings) 7 siblings, 1 reply; 16+ messages in thread From: Bartosz Golaszewski @ 2020-09-04 15:21 UTC (permalink / raw) To: Alessandro Zummo, Alexandre Belloni Cc: linux-rtc, linux-kernel, Bartosz Golaszewski From: Bartosz Golaszewski <bgolaszewski@baylibre.com> Remove parentheses whenever 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 fe010151ec8f..2faf5357a3a5 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] 16+ messages in thread
* Re: [PATCH 1/8] rtc: rx8010: remove unnecessary parentheses 2020-09-04 15:21 ` [PATCH 1/8] rtc: rx8010: remove unnecessary parentheses Bartosz Golaszewski @ 2020-09-04 15:32 ` Alexandre Belloni 0 siblings, 0 replies; 16+ messages in thread From: Alexandre Belloni @ 2020-09-04 15:32 UTC (permalink / raw) To: Bartosz Golaszewski Cc: Alessandro Zummo, linux-rtc, linux-kernel, Bartosz Golaszewski On 04/09/2020 17:21:09+0200, Bartosz Golaszewski wrote: > From: Bartosz Golaszewski <bgolaszewski@baylibre.com> > > Remove parentheses whenever they guard a single line. Those would be braces or curly brackets, not parentheses ;) > > 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 fe010151ec8f..2faf5357a3a5 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 > -- Alexandre Belloni, Bootlin Embedded Linux and Kernel engineering https://bootlin.com ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 2/8] rtc: rx8010: consolidate local variables of the same type 2020-09-04 15:21 [PATCH 0/8] rtc: rx8010: use regmap instead of i2c smbus API Bartosz Golaszewski 2020-09-04 15:21 ` [PATCH 1/8] rtc: rx8010: remove unnecessary parentheses Bartosz Golaszewski @ 2020-09-04 15:21 ` Bartosz Golaszewski 2020-09-04 15:21 ` [PATCH 3/8] rtc: rx8010: use tabs for instead of spaces for code formatting Bartosz Golaszewski ` (5 subsequent siblings) 7 siblings, 0 replies; 16+ messages in thread From: Bartosz Golaszewski @ 2020-09-04 15:21 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 2faf5357a3a5..4c790d33f589 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] 16+ messages in thread
* [PATCH 3/8] rtc: rx8010: use tabs for instead of spaces for code formatting 2020-09-04 15:21 [PATCH 0/8] rtc: rx8010: use regmap instead of i2c smbus API Bartosz Golaszewski 2020-09-04 15:21 ` [PATCH 1/8] rtc: rx8010: remove unnecessary parentheses Bartosz Golaszewski 2020-09-04 15:21 ` [PATCH 2/8] rtc: rx8010: consolidate local variables of the same type Bartosz Golaszewski @ 2020-09-04 15:21 ` Bartosz Golaszewski 2020-09-04 15:21 ` [PATCH 4/8] rtc: rx8010: rename ret to err in rx8010_set_time() Bartosz Golaszewski ` (4 subsequent siblings) 7 siblings, 0 replies; 16+ messages in thread From: Bartosz Golaszewski @ 2020-09-04 15:21 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 4c790d33f589..2038700a3e8e 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] 16+ messages in thread
* [PATCH 4/8] rtc: rx8010: rename ret to err in rx8010_set_time() 2020-09-04 15:21 [PATCH 0/8] rtc: rx8010: use regmap instead of i2c smbus API Bartosz Golaszewski ` (2 preceding siblings ...) 2020-09-04 15:21 ` [PATCH 3/8] rtc: rx8010: use tabs for instead of spaces for code formatting Bartosz Golaszewski @ 2020-09-04 15:21 ` Bartosz Golaszewski 2020-09-04 15:21 ` [PATCH 5/8] rtc: rx8010: don't use magic values for time buffer length Bartosz Golaszewski ` (3 subsequent siblings) 7 siblings, 0 replies; 16+ messages in thread From: Bartosz Golaszewski @ 2020-09-04 15:21 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 2038700a3e8e..67ff06a76629 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] 16+ messages in thread
* [PATCH 5/8] rtc: rx8010: don't use magic values for time buffer length 2020-09-04 15:21 [PATCH 0/8] rtc: rx8010: use regmap instead of i2c smbus API Bartosz Golaszewski ` (3 preceding siblings ...) 2020-09-04 15:21 ` [PATCH 4/8] rtc: rx8010: rename ret to err in rx8010_set_time() Bartosz Golaszewski @ 2020-09-04 15:21 ` Bartosz Golaszewski 2020-09-04 15:39 ` Alexandre Belloni 2020-09-04 15:21 ` [PATCH 6/8] rtc: rx8010: drop unnecessary initialization Bartosz Golaszewski ` (2 subsequent siblings) 7 siblings, 1 reply; 16+ messages in thread From: Bartosz Golaszewski @ 2020-09-04 15:21 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 a constant. Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com> --- drivers/rtc/rtc-rx8010.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/drivers/rtc/rtc-rx8010.c b/drivers/rtc/rtc-rx8010.c index 67ff06a76629..f3bed7be2533 100644 --- a/drivers/rtc/rtc-rx8010.c +++ b/drivers/rtc/rtc-rx8010.c @@ -48,6 +48,8 @@ #define RX8010_ALARM_AE BIT(7) +#define RX8010_TIME_BUF_LEN 7 + static const struct i2c_device_id rx8010_id[] = { { "rx8010", 0 }, { } @@ -108,7 +110,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_TIME_BUF_LEN]; int flagreg, err; flagreg = i2c_smbus_read_byte_data(rx8010->client, RX8010_FLAG); @@ -121,8 +123,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) + RX8010_TIME_BUF_LEN, date); + if (err != RX8010_TIME_BUF_LEN) return err < 0 ? err : -EIO; dt->tm_sec = bcd2bin(date[RX8010_SEC - RX8010_SEC] & 0x7f); @@ -139,7 +141,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_TIME_BUF_LEN]; int ctrl, flagreg, err; if ((dt->tm_year < 100) || (dt->tm_year > 199)) @@ -164,7 +166,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, RX8010_TIME_BUF_LEN, + date); if (err < 0) return err; -- 2.26.1 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 5/8] rtc: rx8010: don't use magic values for time buffer length 2020-09-04 15:21 ` [PATCH 5/8] rtc: rx8010: don't use magic values for time buffer length Bartosz Golaszewski @ 2020-09-04 15:39 ` Alexandre Belloni 0 siblings, 0 replies; 16+ messages in thread From: Alexandre Belloni @ 2020-09-04 15:39 UTC (permalink / raw) To: Bartosz Golaszewski Cc: Alessandro Zummo, linux-rtc, linux-kernel, Bartosz Golaszewski On 04/09/2020 17:21:13+0200, Bartosz Golaszewski wrote: > From: Bartosz Golaszewski <bgolaszewski@baylibre.com> > > The time buffer len is used directly in this driver. For readability > it's better to define a constant. > > Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com> > --- > drivers/rtc/rtc-rx8010.c | 13 ++++++++----- > 1 file changed, 8 insertions(+), 5 deletions(-) > > diff --git a/drivers/rtc/rtc-rx8010.c b/drivers/rtc/rtc-rx8010.c > index 67ff06a76629..f3bed7be2533 100644 > --- a/drivers/rtc/rtc-rx8010.c > +++ b/drivers/rtc/rtc-rx8010.c > @@ -48,6 +48,8 @@ > > #define RX8010_ALARM_AE BIT(7) > > +#define RX8010_TIME_BUF_LEN 7 > + > static const struct i2c_device_id rx8010_id[] = { > { "rx8010", 0 }, > { } > @@ -108,7 +110,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]; I'm usually fine with a magic value here... > + u8 date[RX8010_TIME_BUF_LEN]; > int flagreg, err; > > flagreg = i2c_smbus_read_byte_data(rx8010->client, RX8010_FLAG); > @@ -121,8 +123,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) > + RX8010_TIME_BUF_LEN, date); ..as long as sizeof(date) is used here. Even better, you could have date[RX8010_YEAR - RX8010_SEC + 1] and then use sizeof. Or also have #define RX8010_TIME_BUF_LEN (RX8010_YEAR - RX8010_SEC + 1) which would be safer than 7. -- Alexandre Belloni, Bootlin Embedded Linux and Kernel engineering https://bootlin.com ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 6/8] rtc: rx8010: drop unnecessary initialization 2020-09-04 15:21 [PATCH 0/8] rtc: rx8010: use regmap instead of i2c smbus API Bartosz Golaszewski ` (4 preceding siblings ...) 2020-09-04 15:21 ` [PATCH 5/8] rtc: rx8010: don't use magic values for time buffer length Bartosz Golaszewski @ 2020-09-04 15:21 ` Bartosz Golaszewski 2020-09-04 15:21 ` [PATCH 7/8] rtc: rx8010: fix indentation in probe() Bartosz Golaszewski 2020-09-04 15:21 ` [PATCH 8/8] rtc: rx8010: convert to using regmap Bartosz Golaszewski 7 siblings, 0 replies; 16+ messages in thread From: Bartosz Golaszewski @ 2020-09-04 15:21 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 f3bed7be2533..181fc21cefa8 100644 --- a/drivers/rtc/rtc-rx8010.c +++ b/drivers/rtc/rtc-rx8010.c @@ -196,7 +196,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] 16+ messages in thread
* [PATCH 7/8] rtc: rx8010: fix indentation in probe() 2020-09-04 15:21 [PATCH 0/8] rtc: rx8010: use regmap instead of i2c smbus API Bartosz Golaszewski ` (5 preceding siblings ...) 2020-09-04 15:21 ` [PATCH 6/8] rtc: rx8010: drop unnecessary initialization Bartosz Golaszewski @ 2020-09-04 15:21 ` Bartosz Golaszewski 2020-09-04 15:41 ` Alexandre Belloni 2020-09-04 15:21 ` [PATCH 8/8] rtc: rx8010: convert to using regmap Bartosz Golaszewski 7 siblings, 1 reply; 16+ messages in thread From: Bartosz Golaszewski @ 2020-09-04 15:21 UTC (permalink / raw) To: Alessandro Zummo, Alexandre Belloni Cc: linux-rtc, linux-kernel, Bartosz Golaszewski From: Bartosz Golaszewski <bgolaszewski@baylibre.com> Align the arguments passed to devm_rtc_device_register() with the upper line. 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 181fc21cefa8..ed8ba38b4991 100644 --- a/drivers/rtc/rtc-rx8010.c +++ b/drivers/rtc/rtc-rx8010.c @@ -450,7 +450,7 @@ static int rx8010_probe(struct i2c_client *client, } rx8010->rtc = devm_rtc_device_register(&client->dev, client->name, - &rx8010_rtc_ops, THIS_MODULE); + &rx8010_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] 16+ messages in thread
* Re: [PATCH 7/8] rtc: rx8010: fix indentation in probe() 2020-09-04 15:21 ` [PATCH 7/8] rtc: rx8010: fix indentation in probe() Bartosz Golaszewski @ 2020-09-04 15:41 ` Alexandre Belloni 2020-09-07 9:34 ` Bartosz Golaszewski 0 siblings, 1 reply; 16+ messages in thread From: Alexandre Belloni @ 2020-09-04 15:41 UTC (permalink / raw) To: Bartosz Golaszewski Cc: Alessandro Zummo, linux-rtc, linux-kernel, Bartosz Golaszewski On 04/09/2020 17:21:15+0200, Bartosz Golaszewski wrote: > From: Bartosz Golaszewski <bgolaszewski@baylibre.com> > > Align the arguments passed to devm_rtc_device_register() with the upper > line. > > 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 181fc21cefa8..ed8ba38b4991 100644 > --- a/drivers/rtc/rtc-rx8010.c > +++ b/drivers/rtc/rtc-rx8010.c > @@ -450,7 +450,7 @@ static int rx8010_probe(struct i2c_client *client, > } > > rx8010->rtc = devm_rtc_device_register(&client->dev, client->name, > - &rx8010_rtc_ops, THIS_MODULE); > + &rx8010_rtc_ops, THIS_MODULE); > You have bonus points if you replace that patch by switching from devm_rtc_device_register to devm_rtc_allocate_device and rtc_register_device. More bonus points if you also set range_min and range_max and then get rid of the range checking in set_time. -- Alexandre Belloni, Bootlin Embedded Linux and Kernel engineering https://bootlin.com ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 7/8] rtc: rx8010: fix indentation in probe() 2020-09-04 15:41 ` Alexandre Belloni @ 2020-09-07 9:34 ` Bartosz Golaszewski 2020-09-11 12:28 ` Alexandre Belloni 0 siblings, 1 reply; 16+ messages in thread From: Bartosz Golaszewski @ 2020-09-07 9:34 UTC (permalink / raw) To: Alexandre Belloni; +Cc: Bartosz Golaszewski, Alessandro Zummo, linux-rtc, LKML On Fri, Sep 4, 2020 at 5:41 PM Alexandre Belloni <alexandre.belloni@bootlin.com> wrote: > > On 04/09/2020 17:21:15+0200, Bartosz Golaszewski wrote: > > From: Bartosz Golaszewski <bgolaszewski@baylibre.com> > > > > Align the arguments passed to devm_rtc_device_register() with the upper > > line. > > > > 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 181fc21cefa8..ed8ba38b4991 100644 > > --- a/drivers/rtc/rtc-rx8010.c > > +++ b/drivers/rtc/rtc-rx8010.c > > @@ -450,7 +450,7 @@ static int rx8010_probe(struct i2c_client *client, > > } > > > > rx8010->rtc = devm_rtc_device_register(&client->dev, client->name, > > - &rx8010_rtc_ops, THIS_MODULE); > > + &rx8010_rtc_ops, THIS_MODULE); > > > > You have bonus points if you replace that patch by switching from > devm_rtc_device_register to devm_rtc_allocate_device and > rtc_register_device. > > More bonus points if you also set range_min and range_max and then get > rid of the range checking in set_time. > Hi Alexandre! I've just looked at the code and wondered why there's no devm counterpart for rtc_register_device(). Then I noticed that the release callback for devm_rtc_allocate_device() takes care of unregistering the device. This looks like serious devres abuse to me. In general the idea is for the release callback to only undo whatever the devres function did and this should be opaque to the concerned resources. In this case I believe there's no need for the 'registered' field in struct rtc_device - this structure should *not* care about this - and there should be devm_rtc_register_device() whose release callback would take care of the unregistering. Since this function would be called after devm_rtc_allocate_device(), it would be released before so the ordering should be fine. Let me know your thoughts. Best regards, Bartosz Golaszewski ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 7/8] rtc: rx8010: fix indentation in probe() 2020-09-07 9:34 ` Bartosz Golaszewski @ 2020-09-11 12:28 ` Alexandre Belloni 2020-09-11 12:33 ` Bartosz Golaszewski 0 siblings, 1 reply; 16+ messages in thread From: Alexandre Belloni @ 2020-09-11 12:28 UTC (permalink / raw) To: Bartosz Golaszewski Cc: Bartosz Golaszewski, Alessandro Zummo, linux-rtc, LKML On 07/09/2020 11:34:59+0200, Bartosz Golaszewski wrote: > On Fri, Sep 4, 2020 at 5:41 PM Alexandre Belloni > <alexandre.belloni@bootlin.com> wrote: > > > > On 04/09/2020 17:21:15+0200, Bartosz Golaszewski wrote: > > > From: Bartosz Golaszewski <bgolaszewski@baylibre.com> > > > > > > Align the arguments passed to devm_rtc_device_register() with the upper > > > line. > > > > > > 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 181fc21cefa8..ed8ba38b4991 100644 > > > --- a/drivers/rtc/rtc-rx8010.c > > > +++ b/drivers/rtc/rtc-rx8010.c > > > @@ -450,7 +450,7 @@ static int rx8010_probe(struct i2c_client *client, > > > } > > > > > > rx8010->rtc = devm_rtc_device_register(&client->dev, client->name, > > > - &rx8010_rtc_ops, THIS_MODULE); > > > + &rx8010_rtc_ops, THIS_MODULE); > > > > > > > You have bonus points if you replace that patch by switching from > > devm_rtc_device_register to devm_rtc_allocate_device and > > rtc_register_device. > > > > More bonus points if you also set range_min and range_max and then get > > rid of the range checking in set_time. > > > > Hi Alexandre! > > I've just looked at the code and wondered why there's no devm > counterpart for rtc_register_device(). Then I noticed that the release > callback for devm_rtc_allocate_device() takes care of unregistering > the device. This looks like serious devres abuse to me. In general the > idea is for the release callback to only undo whatever the devres > function did and this should be opaque to the concerned resources. > > In this case I believe there's no need for the 'registered' field in > struct rtc_device - this structure should *not* care about this - and > there should be devm_rtc_register_device() whose release callback > would take care of the unregistering. Since this function would be > called after devm_rtc_allocate_device(), it would be released before > so the ordering should be fine. > Note that the input subsystem is also doing it that way which is probably not a good reason alone to do it like that. But, IIRC, there was an actual reason this was done this way and it was the ordering of the rtc_nvmem_register/rtc_nvmem_unregister with rtc_device_unregister. I'm not sure this is still necessary though. -- Alexandre Belloni, Bootlin Embedded Linux and Kernel engineering https://bootlin.com ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 7/8] rtc: rx8010: fix indentation in probe() 2020-09-11 12:28 ` Alexandre Belloni @ 2020-09-11 12:33 ` Bartosz Golaszewski 2020-09-11 12:44 ` Alexandre Belloni 0 siblings, 1 reply; 16+ messages in thread From: Bartosz Golaszewski @ 2020-09-11 12:33 UTC (permalink / raw) To: Alexandre Belloni; +Cc: Bartosz Golaszewski, Alessandro Zummo, linux-rtc, LKML On Fri, Sep 11, 2020 at 2:28 PM Alexandre Belloni <alexandre.belloni@bootlin.com> wrote: > > On 07/09/2020 11:34:59+0200, Bartosz Golaszewski wrote: > > On Fri, Sep 4, 2020 at 5:41 PM Alexandre Belloni > > <alexandre.belloni@bootlin.com> wrote: > > > > > > On 04/09/2020 17:21:15+0200, Bartosz Golaszewski wrote: > > > > From: Bartosz Golaszewski <bgolaszewski@baylibre.com> > > > > > > > > Align the arguments passed to devm_rtc_device_register() with the upper > > > > line. > > > > > > > > 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 181fc21cefa8..ed8ba38b4991 100644 > > > > --- a/drivers/rtc/rtc-rx8010.c > > > > +++ b/drivers/rtc/rtc-rx8010.c > > > > @@ -450,7 +450,7 @@ static int rx8010_probe(struct i2c_client *client, > > > > } > > > > > > > > rx8010->rtc = devm_rtc_device_register(&client->dev, client->name, > > > > - &rx8010_rtc_ops, THIS_MODULE); > > > > + &rx8010_rtc_ops, THIS_MODULE); > > > > > > > > > > You have bonus points if you replace that patch by switching from > > > devm_rtc_device_register to devm_rtc_allocate_device and > > > rtc_register_device. > > > > > > More bonus points if you also set range_min and range_max and then get > > > rid of the range checking in set_time. > > > > > > > Hi Alexandre! > > > > I've just looked at the code and wondered why there's no devm > > counterpart for rtc_register_device(). Then I noticed that the release > > callback for devm_rtc_allocate_device() takes care of unregistering > > the device. This looks like serious devres abuse to me. In general the > > idea is for the release callback to only undo whatever the devres > > function did and this should be opaque to the concerned resources. > > > > In this case I believe there's no need for the 'registered' field in > > struct rtc_device - this structure should *not* care about this - and > > there should be devm_rtc_register_device() whose release callback > > would take care of the unregistering. Since this function would be > > called after devm_rtc_allocate_device(), it would be released before > > so the ordering should be fine. > > > > Note that the input subsystem is also doing it that way which is > probably not a good reason alone to do it like that. But, IIRC, there I'm seeing this pattern elsewhere in the kernel too and I just recently fixed this for MDIO. I think it's just a matter of people copy-pasting a bad implementation. > was an actual reason this was done this way and it was the ordering of > the rtc_nvmem_register/rtc_nvmem_unregister with rtc_device_unregister. > I'm not sure this is still necessary though. > To me - each of these should have their own 'unregister' function and appropriate devres helpers *OR* RTC-related nvmem structures could be set up and assigned to struct rtc_device after devm_rtc_allocate_device() and picked up by the registration function (and also undone by rtc_unregister_device()). I'll try to allocate some time to look into this but it's not like it's urgent or anything - it's just a potential improvement. Bartosz ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 7/8] rtc: rx8010: fix indentation in probe() 2020-09-11 12:33 ` Bartosz Golaszewski @ 2020-09-11 12:44 ` Alexandre Belloni 0 siblings, 0 replies; 16+ messages in thread From: Alexandre Belloni @ 2020-09-11 12:44 UTC (permalink / raw) To: Bartosz Golaszewski Cc: Bartosz Golaszewski, Alessandro Zummo, linux-rtc, LKML On 11/09/2020 14:33:46+0200, Bartosz Golaszewski wrote: > I'm seeing this pattern elsewhere in the kernel too and I just > recently fixed this for MDIO. I think it's just a matter of people > copy-pasting a bad implementation. > > > was an actual reason this was done this way and it was the ordering of > > the rtc_nvmem_register/rtc_nvmem_unregister with rtc_device_unregister. > > I'm not sure this is still necessary though. > > > > To me - each of these should have their own 'unregister' function and > appropriate devres helpers *OR* RTC-related nvmem structures could be > set up and assigned to struct rtc_device after > devm_rtc_allocate_device() and picked up by the registration function > (and also undone by rtc_unregister_device()). > > I'll try to allocate some time to look into this but it's not like > it's urgent or anything - it's just a potential improvement. > Well, this could simply be done by adding a devres_add in __rtc_register_device. I'm planning to remove rtc_nvmem_unregister after the next LTS which will make that even easier. -- Alexandre Belloni, Bootlin Embedded Linux and Kernel engineering https://bootlin.com ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 8/8] rtc: rx8010: convert to using regmap 2020-09-04 15:21 [PATCH 0/8] rtc: rx8010: use regmap instead of i2c smbus API Bartosz Golaszewski ` (6 preceding siblings ...) 2020-09-04 15:21 ` [PATCH 7/8] rtc: rx8010: fix indentation in probe() Bartosz Golaszewski @ 2020-09-04 15:21 ` Bartosz Golaszewski 7 siblings, 0 replies; 16+ messages in thread From: Bartosz Golaszewski @ 2020-09-04 15:21 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 | 200 ++++++++++++++++----------------------- 1 file changed, 81 insertions(+), 119 deletions(-) diff --git a/drivers/rtc/rtc-rx8010.c b/drivers/rtc/rtc-rx8010.c index ed8ba38b4991..9be3ea88e86d 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 @@ -63,7 +64,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; }; @@ -72,13 +73,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; } @@ -101,10 +101,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) @@ -113,19 +112,19 @@ static int rx8010_get_time(struct device *dev, struct rtc_time *dt) u8 date[RX8010_TIME_BUF_LEN]; 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, - RX8010_TIME_BUF_LEN, date); - if (err != RX8010_TIME_BUF_LEN) - return err < 0 ? err : -EIO; + err = regmap_bulk_read(rx8010->regs, RX8010_SEC, + date, RX8010_TIME_BUF_LEN); + if (err) + return err; dt->tm_sec = bcd2bin(date[RX8010_SEC - RX8010_SEC] & 0x7f); dt->tm_min = bcd2bin(date[RX8010_MIN - RX8010_SEC] & 0x7f); @@ -142,19 +141,14 @@ static int rx8010_set_time(struct device *dev, struct rtc_time *dt) { struct rx8010_data *rx8010 = dev_get_drvdata(dev); u8 date[RX8010_TIME_BUF_LEN]; - 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); @@ -165,66 +159,55 @@ 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, RX8010_TIME_BUF_LEN, - date); - if (err < 0) + err = regmap_bulk_write(rx8010->regs, RX8010_SEC, + date, RX8010_TIME_BUF_LEN); + 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; } @@ -236,8 +219,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; } @@ -249,17 +232,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); @@ -276,52 +258,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) { @@ -331,9 +299,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; } @@ -343,9 +310,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; @@ -362,20 +328,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; } @@ -385,13 +345,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); @@ -407,28 +367,30 @@ static struct rtc_class_ops rx8010_rtc_ops = { .ioctl = rx8010_ioctl, }; +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, const struct i2c_device_id *id) { - struct i2c_adapter *adapter = client->adapter; 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(&client->dev, sizeof(struct rx8010_data), 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(&client->dev); if (err) return err; -- 2.26.1 ^ permalink raw reply related [flat|nested] 16+ messages in thread
end of thread, other threads:[~2020-09-11 17:45 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-09-04 15:21 [PATCH 0/8] rtc: rx8010: use regmap instead of i2c smbus API Bartosz Golaszewski 2020-09-04 15:21 ` [PATCH 1/8] rtc: rx8010: remove unnecessary parentheses Bartosz Golaszewski 2020-09-04 15:32 ` Alexandre Belloni 2020-09-04 15:21 ` [PATCH 2/8] rtc: rx8010: consolidate local variables of the same type Bartosz Golaszewski 2020-09-04 15:21 ` [PATCH 3/8] rtc: rx8010: use tabs for instead of spaces for code formatting Bartosz Golaszewski 2020-09-04 15:21 ` [PATCH 4/8] rtc: rx8010: rename ret to err in rx8010_set_time() Bartosz Golaszewski 2020-09-04 15:21 ` [PATCH 5/8] rtc: rx8010: don't use magic values for time buffer length Bartosz Golaszewski 2020-09-04 15:39 ` Alexandre Belloni 2020-09-04 15:21 ` [PATCH 6/8] rtc: rx8010: drop unnecessary initialization Bartosz Golaszewski 2020-09-04 15:21 ` [PATCH 7/8] rtc: rx8010: fix indentation in probe() Bartosz Golaszewski 2020-09-04 15:41 ` Alexandre Belloni 2020-09-07 9:34 ` Bartosz Golaszewski 2020-09-11 12:28 ` Alexandre Belloni 2020-09-11 12:33 ` Bartosz Golaszewski 2020-09-11 12:44 ` Alexandre Belloni 2020-09-04 15:21 ` [PATCH 8/8] rtc: rx8010: convert to using regmap Bartosz Golaszewski
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).