[PATCH v2 0/4] rtc: s35390a: uie_unsupported and minor fixes

[PATCH v2 0/4] rtc: s35390a: uie_unsupported and minor fixes

[Richard Leitner]

As the s35390a does only support per-minute based alarms we have to
set the uie_unsupported flag. Otherwise it delays for 10sec and 
fails afterwards with modern hwclock versions.

Furthermore some other minor changes are made.

All patches were tested on an i.MX6 platform.

CHANGES v2:
 - use BIT in "clarify INT2 pin output modes"
 - add "change FLAG defines to use BIT macro"

Richard Leitner (4):
  rtc: s35390a: clarify INT2 pin output modes
  rtc: s35390a: set uie_unsupported
  rtc: s35390a: introduce struct device in probe
  rtc: s35390a: change FLAG defines to use BIT macro

 drivers/rtc/rtc-s35390a.c | 55 +++++++++++++++++++++------------------
 1 file changed, 29 insertions(+), 26 deletions(-)

-- 
2.20.1

[PATCH v2 1/4] rtc: s35390a: clarify INT2 pin output modes

[Richard Leitner]

Fix the INT2 mode mask to not include the "TEST" flag. Furthermore
remove the not needed reversion of bits when parsing the INT2 modes.
Instead reverse the INT2_MODE defines to match the bit order from the
datasheet.

Additionally mention the flag names from the datasheet for the different
modes in the comments.

Signed-off-by: Richard Leitner <richard.leitner@skidata.com>
---
 drivers/rtc/rtc-s35390a.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/drivers/rtc/rtc-s35390a.c b/drivers/rtc/rtc-s35390a.c
index 3c64dbb08109..fb795c454077 100644
--- a/drivers/rtc/rtc-s35390a.c
+++ b/drivers/rtc/rtc-s35390a.c
@@ -45,12 +45,13 @@
 /* flag for STATUS2 */
 #define S35390A_FLAG_TEST	0x01
 
-#define S35390A_INT2_MODE_MASK		0xF0
-
+/* INT2 pin output mode */
+#define S35390A_INT2_MODE_MASK		0x0E
 #define S35390A_INT2_MODE_NOINTR	0x00
-#define S35390A_INT2_MODE_FREQ		0x10
-#define S35390A_INT2_MODE_ALARM		0x40
-#define S35390A_INT2_MODE_PMIN_EDG	0x20
+#define S35390A_INT2_MODE_ALARM		BIT(1) /* INT2AE */
+#define S35390A_INT2_MODE_PMIN_EDG	BIT(2) /* INT2ME */
+#define S35390A_INT2_MODE_FREQ		BIT(3) /* INT2FE */
+#define S35390A_INT2_MODE_PMIN		(BIT(3) | BIT(2)) /* INT2FE | INT2ME */
 
 static const struct i2c_device_id s35390a_id[] = {
 	{ "s35390a", 0 },
@@ -303,9 +304,6 @@ static int s35390a_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alm)
 	else
 		sts = S35390A_INT2_MODE_NOINTR;
 
-	/* This chip expects the bits of each byte to be in reverse order */
-	sts = bitrev8(sts);
-
 	/* set interupt mode*/
 	err = s35390a_set_reg(s35390a, S35390A_CMD_STATUS2, &sts, sizeof(sts));
 	if (err < 0)
@@ -343,7 +341,7 @@ static int s35390a_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alm)
 	if (err < 0)
 		return err;
 
-	if ((bitrev8(sts) & S35390A_INT2_MODE_MASK) != S35390A_INT2_MODE_ALARM) {
+	if ((sts & S35390A_INT2_MODE_MASK) != S35390A_INT2_MODE_ALARM) {
 		/*
 		 * When the alarm isn't enabled, the register to configure
 		 * the alarm time isn't accessible.
-- 
2.20.1

[PATCH v2 2/4] rtc: s35390a: set uie_unsupported

[Richard Leitner]

Alarms are only supported on a per minute basis. This is why
uie_unsupported is set. Furthermore issue a warning when a second based
alarm is requested.

Signed-off-by: Richard Leitner <richard.leitner@skidata.com>
---
 drivers/rtc/rtc-s35390a.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/rtc/rtc-s35390a.c b/drivers/rtc/rtc-s35390a.c
index fb795c454077..4ca37f281ed9 100644
--- a/drivers/rtc/rtc-s35390a.c
+++ b/drivers/rtc/rtc-s35390a.c
@@ -289,6 +289,9 @@ static int s35390a_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alm)
 		alm->time.tm_min, alm->time.tm_hour, alm->time.tm_mday,
 		alm->time.tm_mon, alm->time.tm_year, alm->time.tm_wday);
 
+	if (alm->time.tm_sec != 0)
+		dev_warn(&client->dev, "Alarms are only supported on a per minute basis!\n");
+
 	/* disable interrupt (which deasserts the irq line) */
 	err = s35390a_set_reg(s35390a, S35390A_CMD_STATUS2, &sts, sizeof(sts));
 	if (err < 0)
@@ -500,6 +503,9 @@ static int s35390a_probe(struct i2c_client *client,
 		goto exit_dummy;
 	}
 
+	/* supports per-minute alarms only, therefore set uie_unsupported */
+	s35390a->rtc->uie_unsupported = 1;
+
 	if (status1 & S35390A_FLAG_INT2)
 		rtc_update_irq(s35390a->rtc, 1, RTC_AF);
 
-- 
2.20.1

[PATCH v2 3/4] rtc: s35390a: introduce struct device in probe

[Richard Leitner]

To simplify access and shorten code introduce a struct device pointer in
the s35390a probe function.

Signed-off-by: Richard Leitner <richard.leitner@skidata.com>
---
 drivers/rtc/rtc-s35390a.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/rtc/rtc-s35390a.c b/drivers/rtc/rtc-s35390a.c
index 4ca37f281ed9..7293dcdea692 100644
--- a/drivers/rtc/rtc-s35390a.c
+++ b/drivers/rtc/rtc-s35390a.c
@@ -436,14 +436,14 @@ static int s35390a_probe(struct i2c_client *client,
 	unsigned int i;
 	struct s35390a *s35390a;
 	char buf, status1;
+	struct device *dev = &client->dev;
 
 	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
 		err = -ENODEV;
 		goto exit;
 	}
 
-	s35390a = devm_kzalloc(&client->dev, sizeof(struct s35390a),
-				GFP_KERNEL);
+	s35390a = devm_kzalloc(dev, sizeof(struct s35390a), GFP_KERNEL);
 	if (!s35390a) {
 		err = -ENOMEM;
 		goto exit;
@@ -457,8 +457,8 @@ static int s35390a_probe(struct i2c_client *client,
 		s35390a->client[i] = i2c_new_dummy(client->adapter,
 					client->addr + i);
 		if (!s35390a->client[i]) {
-			dev_err(&client->dev, "Address %02x unavailable\n",
-						client->addr + i);
+			dev_err(dev, "Address %02x unavailable\n",
+				client->addr + i);
 			err = -EBUSY;
 			goto exit_dummy;
 		}
@@ -467,7 +467,7 @@ static int s35390a_probe(struct i2c_client *client,
 	err_read = s35390a_read_status(s35390a, &status1);
 	if (err_read < 0) {
 		err = err_read;
-		dev_err(&client->dev, "error resetting chip\n");
+		dev_err(dev, "error resetting chip\n");
 		goto exit_dummy;
 	}
 
@@ -481,22 +481,21 @@ static int s35390a_probe(struct i2c_client *client,
 		buf = 0;
 		err = s35390a_set_reg(s35390a, S35390A_CMD_STATUS2, &buf, 1);
 		if (err < 0) {
-			dev_err(&client->dev, "error disabling alarm");
+			dev_err(dev, "error disabling alarm");
 			goto exit_dummy;
 		}
 	} else {
 		err = s35390a_disable_test_mode(s35390a);
 		if (err < 0) {
-			dev_err(&client->dev, "error disabling test mode\n");
+			dev_err(dev, "error disabling test mode\n");
 			goto exit_dummy;
 		}
 	}
 
-	device_set_wakeup_capable(&client->dev, 1);
+	device_set_wakeup_capable(dev, 1);
 
-	s35390a->rtc = devm_rtc_device_register(&client->dev,
-					s35390a_driver.driver.name,
-					&s35390a_rtc_ops, THIS_MODULE);
+	s35390a->rtc = devm_rtc_device_register(dev, s35390a_driver.driver.name,
+						&s35390a_rtc_ops, THIS_MODULE);
 
 	if (IS_ERR(s35390a->rtc)) {
 		err = PTR_ERR(s35390a->rtc);
-- 
2.20.1

[PATCH v2 4/4] rtc: s35390a: change FLAG defines to use BIT macro

[Richard Leitner]

To be consistent change the S35390A_FLAG defines to use the BIT
macro (like the S35390A_INT2_MODE defines).

Signed-off-by: Richard Leitner <richard.leitner@skidata.com>
---
 drivers/rtc/rtc-s35390a.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/rtc/rtc-s35390a.c b/drivers/rtc/rtc-s35390a.c
index 7293dcdea692..b18ce5610a94 100644
--- a/drivers/rtc/rtc-s35390a.c
+++ b/drivers/rtc/rtc-s35390a.c
@@ -36,14 +36,14 @@
 #define S35390A_ALRM_BYTE_MINS	2
 
 /* flags for STATUS1 */
-#define S35390A_FLAG_POC	0x01
-#define S35390A_FLAG_BLD	0x02
-#define S35390A_FLAG_INT2	0x04
-#define S35390A_FLAG_24H	0x40
-#define S35390A_FLAG_RESET	0x80
+#define S35390A_FLAG_POC	BIT(0)
+#define S35390A_FLAG_BLD	BIT(1)
+#define S35390A_FLAG_INT2	BIT(2)
+#define S35390A_FLAG_24H	BIT(6)
+#define S35390A_FLAG_RESET	BIT(7)
 
 /* flag for STATUS2 */
-#define S35390A_FLAG_TEST	0x01
+#define S35390A_FLAG_TEST	BIT(0)
 
 /* INT2 pin output mode */
 #define S35390A_INT2_MODE_MASK		0x0E
-- 
2.20.1

Re: [PATCH v2 0/4] rtc: s35390a: uie_unsupported and minor fixes

[Alexandre Belloni]

On 23/05/2019 13:54:47+0200, Richard Leitner wrote:
> As the s35390a does only support per-minute based alarms we have to
> set the uie_unsupported flag. Otherwise it delays for 10sec and 
> fails afterwards with modern hwclock versions.
> 
> Furthermore some other minor changes are made.
> 
> All patches were tested on an i.MX6 platform.
> 
> CHANGES v2:
>  - use BIT in "clarify INT2 pin output modes"
>  - add "change FLAG defines to use BIT macro"
> 
> Richard Leitner (4):
>   rtc: s35390a: clarify INT2 pin output modes
>   rtc: s35390a: set uie_unsupported
>   rtc: s35390a: introduce struct device in probe
>   rtc: s35390a: change FLAG defines to use BIT macro
> 
>  drivers/rtc/rtc-s35390a.c | 55 +++++++++++++++++++++------------------
>  1 file changed, 29 insertions(+), 26 deletions(-)
> 

Applied, thanks.

-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com