[PATCH v2 02/17] RTC: ds1307: Disable square wave and timers as default

[PATCH v2 02/17] RTC: ds1307: Disable square wave and timers as default

[Andrey Smirnov]

Disable square wave and timers as default for DS1337/39/41 and
DS3231. The rationale being that configuring a chip this way puts it
into a known state with lower power consumption. While it is not very
likely it is still possible that the code controlling RTCs that ran
before this driver configured it to produce square wave and left it in
such a state.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 drivers/rtc/rtc-ds1307.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
index 7e65e2e..c618c22 100644
--- a/drivers/rtc/rtc-ds1307.c
+++ b/drivers/rtc/rtc-ds1307.c
@@ -1320,19 +1320,17 @@ static int ds1307_probe(struct i2c_client *client,
 			ds1307->regs[0] &= ~DS1337_BIT_nEOSC;
 
 		/*
-		 * Using IRQ or defined as wakeup-source?
 		 * Disable the square wave and both alarms.
 		 * For some variants, be sure alarms can trigger when we're
 		 * running on Vbackup (BBSQI/BBSQW)
 		 */
-		if (chip->alarm && (ds1307->client->irq > 0 ||
-						ds1307_can_wakeup_device)) {
-			ds1307->regs[0] |= DS1337_BIT_INTCN
-					| bbsqi_bitpos[ds1307->type];
-			ds1307->regs[0] &= ~(DS1337_BIT_A2IE | DS1337_BIT_A1IE);
+		ds1307->regs[0] |= DS1337_BIT_INTCN
+			| bbsqi_bitpos[ds1307->type];
+		ds1307->regs[0] &= ~(DS1337_BIT_A2IE | DS1337_BIT_A1IE);
 
+		if (chip->alarm && (ds1307->client->irq > 0 ||
+				    ds1307_can_wakeup_device))
 			want_irq = true;
-		}
 
 		i2c_smbus_write_byte_data(client, DS1337_REG_CONTROL,
 							ds1307->regs[0]);
-- 
2.5.5

[PATCH v2 03/17] RTC: ds1307: Add devicetree bindings for DS1341

[Andrey Smirnov]

Add devicetree bindings for DS1341 that allow to control settings
affecting power consumption of the chip.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 .../devicetree/bindings/rtc/dallas,ds1341.txt      | 23 ++++++++++++++++++++++
 1 file changed, 23 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/rtc/dallas,ds1341.txt

diff --git a/Documentation/devicetree/bindings/rtc/dallas,ds1341.txt b/Documentation/devicetree/bindings/rtc/dallas,ds1341.txt
new file mode 100644
index 0000000..7942dd0
--- /dev/null
+++ b/Documentation/devicetree/bindings/rtc/dallas,ds1341.txt
@@ -0,0 +1,23 @@
+* Dallas DS1341		I2C Serial Real-Time Clock
+
+Required properties:
+
+- compatible: Should contain "dallas,ds1341".
+
+- reg: I2C address for chip
+
+Optional properties:
+
+- dallas,disable-oscillator-stop-flag : Boolean. Configure chip to disable oscillator
+  fault detection circuitry. Setting this flag is beneficial for power saving.
+
+- dallas,enable-glitch-filter : Boolean. Configure chip to enable crystal oscillator
+  output glitch filtering. Setting this flag will increase power consumption.
+
+Example:
+	ds1341: rtc@68 {
+		compatible = "dallas,ds1341";
+		dallas,disable-oscillator-stop-flag;
+		dallas,enable-glitch-filter;
+		reg = <0x68>;
+	};
-- 
2.5.5

[PATCH v2 04/17] RTC: ds1307: Add DS1341 specific power-saving options

[Andrey Smirnov]

Add DS1341 specific power-saving options that allow to disable certain
functional aspects of the chip in order to minimize its power
consumption.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 drivers/rtc/rtc-ds1307.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
index c618c22..b21ea5d 100644
--- a/drivers/rtc/rtc-ds1307.c
+++ b/drivers/rtc/rtc-ds1307.c
@@ -78,6 +78,7 @@ enum ds_type {
 #define DS1337_REG_CONTROL	0x0e
 #	define DS1337_BIT_nEOSC		0x80
 #	define DS1339_BIT_BBSQI		0x20
+#	define DS1341_BIT_EGFIL		0x20
 #	define DS3231_BIT_BBSQW		0x40 /* same as BBSQI */
 #	define DS1337_BIT_RS2		0x10
 #	define DS1337_BIT_RS1		0x08
@@ -93,7 +94,9 @@ enum ds_type {
 #	define DS1340_BIT_OSF		0x80
 #define DS1337_REG_STATUS	0x0f
 #	define DS1337_BIT_OSF		0x80
+#	define DS1341_BIT_DOSF		0x40
 #	define DS3231_BIT_EN32KHZ	0x08
+#	define DS1341_BIT_ECLK		0x04
 #	define DS1337_BIT_A2I		0x02
 #	define DS1337_BIT_A1I		0x01
 #define DS1339_REG_ALARM1_SECS	0x07
@@ -1319,6 +1322,31 @@ static int ds1307_probe(struct i2c_client *client,
 		if (ds1307->regs[0] & DS1337_BIT_nEOSC)
 			ds1307->regs[0] &= ~DS1337_BIT_nEOSC;
 
+		if (ds1307->type == ds_1341) {
+			/* Make sure we are not generating square wave
+			 * output */
+			ds1307->regs[1] &= ~DS1341_BIT_ECLK;
+
+			if (of_property_read_bool(client->dev.of_node,
+						  "dallas,disable-oscillator-stop-flag"))
+				ds1307->regs[1] |= DS1341_BIT_DOSF;
+			else
+				ds1307->regs[1] &= ~DS1341_BIT_DOSF;
+
+			if (of_property_read_bool(client->dev.of_node,
+						  "dallas,enable-glitch-filter"))
+				ds1307->regs[0] |= DS1341_BIT_EGFIL;
+			else
+				ds1307->regs[0] &= ~DS1341_BIT_EGFIL;
+
+			/*
+			 * Write status register. Control register
+			 * would be set by the code below
+			 */
+			i2c_smbus_write_byte_data(client, DS1337_REG_STATUS,
+						  ds1307->regs[1]);
+		}
+
 		/*
 		 * Disable the square wave and both alarms.
 		 * For some variants, be sure alarms can trigger when we're
-- 
2.5.5

[PATCH v2 05/17] RTC: ds1307: Convert ds1307_can_wakeup_device into a predicate

[Andrey Smirnov]

Convert ds1307_can_wakeup_device in ds1307_probe into a predicate in
order to make various chunks of code in that function less connected to
each other.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 drivers/rtc/rtc-ds1307.c | 35 ++++++++++++++++++-----------------
 1 file changed, 18 insertions(+), 17 deletions(-)

diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
index b21ea5d..98e1f81 100644
--- a/drivers/rtc/rtc-ds1307.c
+++ b/drivers/rtc/rtc-ds1307.c
@@ -1232,6 +1232,21 @@ static void ds1307_clks_register(struct ds1307 *ds1307)
 
 #endif /* CONFIG_COMMON_CLK */
 
+static bool ds1307_can_wakeup_device(const struct ds1307 *ds1307)
+{
+
+/*
+ * For devices with no IRQ directly connected to the SoC, the RTC chip
+ * can be forced as a wakeup source by stating that explicitly in
+ * the device's .dts file using the "wakeup-source" boolean property.
+ * If the "wakeup-source" property is set, don't request an IRQ.
+ * This will guarantee the 'wakealarm' sysfs entry is available on the device,
+ * if supported by the RTC.
+ */
+	return of_property_read_bool(ds1307->client->dev.of_node,
+				     "wakeup-source");
+}
+
 static int ds1307_probe(struct i2c_client *client,
 			const struct i2c_device_id *id)
 {
@@ -1241,7 +1256,6 @@ static int ds1307_probe(struct i2c_client *client,
 	struct chip_desc	*chip = &chips[id->driver_data];
 	struct i2c_adapter	*adapter = to_i2c_adapter(client->dev.parent);
 	bool			want_irq = false;
-	bool			ds1307_can_wakeup_device = false;
 	unsigned char		*buf;
 	struct ds1307_platform_data *pdata = dev_get_platdata(&client->dev);
 	irq_handler_t	irq_handler = ds1307_irq;
@@ -1290,20 +1304,6 @@ static int ds1307_probe(struct i2c_client *client,
 		ds1307->write_block_data = ds1307_write_block_data;
 	}
 
-#ifdef CONFIG_OF
-/*
- * For devices with no IRQ directly connected to the SoC, the RTC chip
- * can be forced as a wakeup source by stating that explicitly in
- * the device's .dts file using the "wakeup-source" boolean property.
- * If the "wakeup-source" property is set, don't request an IRQ.
- * This will guarantee the 'wakealarm' sysfs entry is available on the device,
- * if supported by the RTC.
- */
-	if (of_property_read_bool(client->dev.of_node, "wakeup-source")) {
-		ds1307_can_wakeup_device = true;
-	}
-#endif
-
 	switch (ds1307->type) {
 	case ds_1337:
 	case ds_1339:
@@ -1357,7 +1357,7 @@ static int ds1307_probe(struct i2c_client *client,
 		ds1307->regs[0] &= ~(DS1337_BIT_A2IE | DS1337_BIT_A1IE);
 
 		if (chip->alarm && (ds1307->client->irq > 0 ||
-				    ds1307_can_wakeup_device))
+				    ds1307_can_wakeup_device(ds1307)))
 			want_irq = true;
 
 		i2c_smbus_write_byte_data(client, DS1337_REG_CONTROL,
@@ -1567,7 +1567,8 @@ read_rtc:
 		return PTR_ERR(ds1307->rtc);
 	}
 
-	if (ds1307_can_wakeup_device && ds1307->client->irq <= 0) {
+	if (ds1307_can_wakeup_device(ds1307) &&
+	    ds1307->client->irq <= 0) {
 		/* Disable request for an IRQ */
 		want_irq = false;
 		dev_info(&client->dev, "'wakeup-source' is set, request for an IRQ is disabled!\n");
-- 
2.5.5

[PATCH v2 06/17] RTC: ds1307: Convert want_irq into a predicate

[Andrey Smirnov]

Convert want_irq variable into a predicate in order to detangle various
independent chunks of ds1307_probe().

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 drivers/rtc/rtc-ds1307.c | 44 +++++++++++++++++++++++++++++---------------
 1 file changed, 29 insertions(+), 15 deletions(-)

diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
index 98e1f81..ccfaba4 100644
--- a/drivers/rtc/rtc-ds1307.c
+++ b/drivers/rtc/rtc-ds1307.c
@@ -1247,6 +1247,30 @@ static bool ds1307_can_wakeup_device(const struct ds1307 *ds1307)
 				     "wakeup-source");
 }
 
+static bool ds1307_want_irq(const struct ds1307 *ds1307,
+			    const struct chip_desc *chip)
+{
+
+
+	if (chip->alarm) {
+		switch (ds1307->type) {
+		case ds_1337:
+		case ds_1339:
+		case ds_3231:
+		case ds_1341:
+			return (ds1307->client->irq > 0 ||
+				ds1307_can_wakeup_device(ds1307));
+
+		case mcp794xx:
+			return (ds1307->client->irq > 0);
+		default:
+			break;
+		}
+	}
+
+	return false;
+}
+
 static int ds1307_probe(struct i2c_client *client,
 			const struct i2c_device_id *id)
 {
@@ -1255,7 +1279,6 @@ static int ds1307_probe(struct i2c_client *client,
 	int			tmp;
 	struct chip_desc	*chip = &chips[id->driver_data];
 	struct i2c_adapter	*adapter = to_i2c_adapter(client->dev.parent);
-	bool			want_irq = false;
 	unsigned char		*buf;
 	struct ds1307_platform_data *pdata = dev_get_platdata(&client->dev);
 	irq_handler_t	irq_handler = ds1307_irq;
@@ -1356,10 +1379,6 @@ static int ds1307_probe(struct i2c_client *client,
 			| bbsqi_bitpos[ds1307->type];
 		ds1307->regs[0] &= ~(DS1337_BIT_A2IE | DS1337_BIT_A1IE);
 
-		if (chip->alarm && (ds1307->client->irq > 0 ||
-				    ds1307_can_wakeup_device(ds1307)))
-			want_irq = true;
-
 		i2c_smbus_write_byte_data(client, DS1337_REG_CONTROL,
 							ds1307->regs[0]);
 
@@ -1441,10 +1460,7 @@ static int ds1307_probe(struct i2c_client *client,
 		break;
 	case mcp794xx:
 		rtc_ops = &mcp794xx_rtc_ops;
-		if (ds1307->client->irq > 0 && chip->alarm) {
-			irq_handler = mcp794xx_irq;
-			want_irq = true;
-		}
+		irq_handler = mcp794xx_irq;
 		break;
 	default:
 		break;
@@ -1557,7 +1573,7 @@ read_rtc:
 				bin2bcd(tmp));
 	}
 
-	if (want_irq) {
+	if (ds1307_want_irq(ds1307, chip)) {
 		device_set_wakeup_capable(&client->dev, true);
 		set_bit(HAS_ALARM, &ds1307->flags);
 	}
@@ -1570,13 +1586,11 @@ read_rtc:
 	if (ds1307_can_wakeup_device(ds1307) &&
 	    ds1307->client->irq <= 0) {
 		/* Disable request for an IRQ */
-		want_irq = false;
-		dev_info(&client->dev, "'wakeup-source' is set, request for an IRQ is disabled!\n");
+		dev_info(&client->dev,
+			 "'wakeup-source' is set, request for an IRQ is disabled!\n");
 		/* We cannot support UIE mode if we do not have an IRQ line */
 		ds1307->rtc->uie_unsupported = 1;
-	}
-
-	if (want_irq) {
+	} else if (ds1307_want_irq(ds1307, chip)) {
 		err = devm_request_threaded_irq(&client->dev,
 						client->irq, NULL, irq_handler,
 						IRQF_SHARED | IRQF_ONESHOT,
-- 
2.5.5

[PATCH v2 07/17] RTC: ds1307: Move chip configuration into a separate routine

[Andrey Smirnov]

Move chip configuration into a separate routine to improve readablity of
the code.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 drivers/rtc/rtc-ds1307.c | 172 ++++++++++++++++++++++++++---------------------
 1 file changed, 97 insertions(+), 75 deletions(-)

diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
index ccfaba4..4fa09dc 100644
--- a/drivers/rtc/rtc-ds1307.c
+++ b/drivers/rtc/rtc-ds1307.c
@@ -1271,74 +1271,32 @@ static bool ds1307_want_irq(const struct ds1307 *ds1307,
 	return false;
 }
 
-static int ds1307_probe(struct i2c_client *client,
-			const struct i2c_device_id *id)
+static int ds1307_chip_configure(struct ds1307 *ds1307)
 {
-	struct ds1307		*ds1307;
-	int			err = -ENODEV;
-	int			tmp;
-	struct chip_desc	*chip = &chips[id->driver_data];
-	struct i2c_adapter	*adapter = to_i2c_adapter(client->dev.parent);
-	unsigned char		*buf;
-	struct ds1307_platform_data *pdata = dev_get_platdata(&client->dev);
-	irq_handler_t	irq_handler = ds1307_irq;
-
-	static const int	bbsqi_bitpos[] = {
-		[ds_1337] = 0,
-		[ds_1341] = 0,
-		[ds_1339] = DS1339_BIT_BBSQI,
-		[ds_3231] = DS3231_BIT_BBSQW,
-	};
-	const struct rtc_class_ops *rtc_ops = &ds13xx_rtc_ops;
-
-	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA)
-	    && !i2c_check_functionality(adapter, I2C_FUNC_SMBUS_I2C_BLOCK))
-		return -EIO;
-
-	ds1307 = devm_kzalloc(&client->dev, sizeof(struct ds1307), GFP_KERNEL);
-	if (!ds1307)
-		return -ENOMEM;
-
-	i2c_set_clientdata(client, ds1307);
-
-	ds1307->client	= client;
-	ds1307->type	= id->driver_data;
-
-	if (!pdata && client->dev.of_node)
-		ds1307_trickle_of_init(client, chip);
-	else if (pdata && pdata->trickle_charger_setup)
-		chip->trickle_charger_setup = pdata->trickle_charger_setup;
-
-	if (chip->trickle_charger_setup && chip->trickle_charger_reg) {
-		dev_dbg(&client->dev, "writing trickle charger info 0x%x to 0x%x\n",
-		    DS13XX_TRICKLE_CHARGER_MAGIC | chip->trickle_charger_setup,
-		    chip->trickle_charger_reg);
-		i2c_smbus_write_byte_data(client, chip->trickle_charger_reg,
-		    DS13XX_TRICKLE_CHARGER_MAGIC |
-		    chip->trickle_charger_setup);
-	}
+	int tmp;
+	unsigned char *buf;
+	struct i2c_client *client = ds1307->client;
 
 	buf = ds1307->regs;
-	if (i2c_check_functionality(adapter, I2C_FUNC_SMBUS_I2C_BLOCK)) {
-		ds1307->read_block_data = ds1307_native_smbus_read_block_data;
-		ds1307->write_block_data = ds1307_native_smbus_write_block_data;
-	} else {
-		ds1307->read_block_data = ds1307_read_block_data;
-		ds1307->write_block_data = ds1307_write_block_data;
-	}
 
 	switch (ds1307->type) {
 	case ds_1337:
 	case ds_1339:
 	case ds_3231:
-	case ds_1341:
+	case ds_1341: {
+		static const int bbsqi_bitpos[] = {
+			[ds_1337] = 0,
+			[ds_1341] = 0,
+			[ds_1339] = DS1339_BIT_BBSQI,
+			[ds_3231] = DS3231_BIT_BBSQW,
+		};
+
 		/* get registers that the "rtc" read below won't read... */
-		tmp = ds1307->read_block_data(ds1307->client,
-				DS1337_REG_CONTROL, 2, buf);
+		tmp = ds1307->read_block_data(client,
+					      DS1337_REG_CONTROL, 2, buf);
 		if (tmp != 2) {
-			dev_dbg(&client->dev, "read error %d\n", tmp);
-			err = -EIO;
-			goto exit;
+			dev_dbg(&ds1307->client->dev, "read error %d\n", tmp);
+			return -EIO;
 		}
 
 		/* oscillator off?  turn it on, so clock can tick. */
@@ -1366,7 +1324,8 @@ static int ds1307_probe(struct i2c_client *client,
 			 * Write status register. Control register
 			 * would be set by the code below
 			 */
-			i2c_smbus_write_byte_data(client, DS1337_REG_STATUS,
+			i2c_smbus_write_byte_data(client,
+						  DS1337_REG_STATUS,
 						  ds1307->regs[1]);
 		}
 
@@ -1379,24 +1338,26 @@ static int ds1307_probe(struct i2c_client *client,
 			| bbsqi_bitpos[ds1307->type];
 		ds1307->regs[0] &= ~(DS1337_BIT_A2IE | DS1337_BIT_A1IE);
 
-		i2c_smbus_write_byte_data(client, DS1337_REG_CONTROL,
-							ds1307->regs[0]);
+		i2c_smbus_write_byte_data(client,
+					  DS1337_REG_CONTROL,
+					  ds1307->regs[0]);
 
 		/* oscillator fault?  clear flag, and warn */
 		if (ds1307->regs[1] & DS1337_BIT_OSF) {
-			i2c_smbus_write_byte_data(client, DS1337_REG_STATUS,
-				ds1307->regs[1] & ~DS1337_BIT_OSF);
-			dev_warn(&client->dev, "SET TIME!\n");
+			i2c_smbus_write_byte_data(client,
+						  DS1337_REG_STATUS,
+						  ds1307->regs[1] & ~DS1337_BIT_OSF);
+			dev_warn(&ds1307->client->dev, "SET TIME!\n");
 		}
 		break;
-
+	}
 	case rx_8025:
-		tmp = i2c_smbus_read_i2c_block_data(ds1307->client,
-				RX8025_REG_CTRL1 << 4 | 0x08, 2, buf);
+		tmp = i2c_smbus_read_i2c_block_data(client,
+						    RX8025_REG_CTRL1 << 4 | 0x08,
+						    2, buf);
 		if (tmp != 2) {
 			dev_dbg(&client->dev, "read error %d\n", tmp);
-			err = -EIO;
-			goto exit;
+			return -EIO;
 		}
 
 		/* oscillator off?  turn it on, so clock can tick. */
@@ -1432,15 +1393,14 @@ static int ds1307_probe(struct i2c_client *client,
 			/* switch to 24 hour mode */
 			i2c_smbus_write_byte_data(client,
 						  RX8025_REG_CTRL1 << 4 | 0x08,
-						  ds1307->regs[0] |
-						  RX8025_BIT_2412);
+						  ds1307->regs[0] | RX8025_BIT_2412);
 
-			tmp = i2c_smbus_read_i2c_block_data(ds1307->client,
-					RX8025_REG_CTRL1 << 4 | 0x08, 2, buf);
+			tmp = i2c_smbus_read_i2c_block_data(client,
+							    RX8025_REG_CTRL1 << 4 | 0x08,
+							    2, buf);
 			if (tmp != 2) {
 				dev_dbg(&client->dev, "read error %d\n", tmp);
-				err = -EIO;
-				goto exit;
+				return -EIO;
 			}
 
 			/* correct hour */
@@ -1455,6 +1415,68 @@ static int ds1307_probe(struct i2c_client *client,
 						  hour);
 		}
 		break;
+	default:
+		break;
+	}
+
+	return 0;
+}
+
+static int ds1307_probe(struct i2c_client *client,
+			const struct i2c_device_id *id)
+{
+	struct ds1307		*ds1307;
+	int			err = -ENODEV;
+	int			tmp;
+	struct chip_desc	*chip = &chips[id->driver_data];
+	struct i2c_adapter	*adapter = to_i2c_adapter(client->dev.parent);
+	unsigned char		*buf;
+	struct ds1307_platform_data *pdata = dev_get_platdata(&client->dev);
+	irq_handler_t	irq_handler = ds1307_irq;
+
+	const struct rtc_class_ops *rtc_ops = &ds13xx_rtc_ops;
+
+	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA)
+	    && !i2c_check_functionality(adapter, I2C_FUNC_SMBUS_I2C_BLOCK))
+		return -EIO;
+
+	ds1307 = devm_kzalloc(&client->dev, sizeof(struct ds1307), GFP_KERNEL);
+	if (!ds1307)
+		return -ENOMEM;
+
+	i2c_set_clientdata(client, ds1307);
+
+	ds1307->client	= client;
+	ds1307->type	= id->driver_data;
+
+	if (!pdata && client->dev.of_node)
+		ds1307_trickle_of_init(client, chip);
+	else if (pdata && pdata->trickle_charger_setup)
+		chip->trickle_charger_setup = pdata->trickle_charger_setup;
+
+	if (chip->trickle_charger_setup && chip->trickle_charger_reg) {
+		dev_dbg(&client->dev, "writing trickle charger info 0x%x to 0x%x\n",
+		    DS13XX_TRICKLE_CHARGER_MAGIC | chip->trickle_charger_setup,
+		    chip->trickle_charger_reg);
+		i2c_smbus_write_byte_data(client, chip->trickle_charger_reg,
+		    DS13XX_TRICKLE_CHARGER_MAGIC |
+		    chip->trickle_charger_setup);
+	}
+
+	buf = ds1307->regs;
+	if (i2c_check_functionality(adapter, I2C_FUNC_SMBUS_I2C_BLOCK)) {
+		ds1307->read_block_data = ds1307_native_smbus_read_block_data;
+		ds1307->write_block_data = ds1307_native_smbus_write_block_data;
+	} else {
+		ds1307->read_block_data = ds1307_read_block_data;
+		ds1307->write_block_data = ds1307_write_block_data;
+	}
+
+	err = ds1307_chip_configure(ds1307);
+	if (err < 0)
+		return err;
+
+	switch (ds1307->type) {
 	case ds_1388:
 		ds1307->offset = 1; /* Seconds starts at 1 */
 		break;
-- 
2.5.5

[PATCH v2 08/17] RTC: ds1307: Move chip sanity checking into a separate routine

[Andrey Smirnov]

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 drivers/rtc/rtc-ds1307.c | 148 +++++++++++++++++++++++++----------------------
 1 file changed, 79 insertions(+), 69 deletions(-)

diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
index 4fa09dc..d262db5 100644
--- a/drivers/rtc/rtc-ds1307.c
+++ b/drivers/rtc/rtc-ds1307.c
@@ -1422,79 +1422,20 @@ static int ds1307_chip_configure(struct ds1307 *ds1307)
 	return 0;
 }
 
-static int ds1307_probe(struct i2c_client *client,
-			const struct i2c_device_id *id)
+static int ds1307_chip_sanity_check(struct ds1307 *ds1307)
 {
-	struct ds1307		*ds1307;
-	int			err = -ENODEV;
-	int			tmp;
-	struct chip_desc	*chip = &chips[id->driver_data];
-	struct i2c_adapter	*adapter = to_i2c_adapter(client->dev.parent);
-	unsigned char		*buf;
-	struct ds1307_platform_data *pdata = dev_get_platdata(&client->dev);
-	irq_handler_t	irq_handler = ds1307_irq;
-
-	const struct rtc_class_ops *rtc_ops = &ds13xx_rtc_ops;
-
-	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA)
-	    && !i2c_check_functionality(adapter, I2C_FUNC_SMBUS_I2C_BLOCK))
-		return -EIO;
-
-	ds1307 = devm_kzalloc(&client->dev, sizeof(struct ds1307), GFP_KERNEL);
-	if (!ds1307)
-		return -ENOMEM;
-
-	i2c_set_clientdata(client, ds1307);
-
-	ds1307->client	= client;
-	ds1307->type	= id->driver_data;
-
-	if (!pdata && client->dev.of_node)
-		ds1307_trickle_of_init(client, chip);
-	else if (pdata && pdata->trickle_charger_setup)
-		chip->trickle_charger_setup = pdata->trickle_charger_setup;
-
-	if (chip->trickle_charger_setup && chip->trickle_charger_reg) {
-		dev_dbg(&client->dev, "writing trickle charger info 0x%x to 0x%x\n",
-		    DS13XX_TRICKLE_CHARGER_MAGIC | chip->trickle_charger_setup,
-		    chip->trickle_charger_reg);
-		i2c_smbus_write_byte_data(client, chip->trickle_charger_reg,
-		    DS13XX_TRICKLE_CHARGER_MAGIC |
-		    chip->trickle_charger_setup);
-	}
+	int tmp;
+	unsigned char *buf;
+	struct i2c_client *client = ds1307->client;
 
 	buf = ds1307->regs;
-	if (i2c_check_functionality(adapter, I2C_FUNC_SMBUS_I2C_BLOCK)) {
-		ds1307->read_block_data = ds1307_native_smbus_read_block_data;
-		ds1307->write_block_data = ds1307_native_smbus_write_block_data;
-	} else {
-		ds1307->read_block_data = ds1307_read_block_data;
-		ds1307->write_block_data = ds1307_write_block_data;
-	}
-
-	err = ds1307_chip_configure(ds1307);
-	if (err < 0)
-		return err;
-
-	switch (ds1307->type) {
-	case ds_1388:
-		ds1307->offset = 1; /* Seconds starts at 1 */
-		break;
-	case mcp794xx:
-		rtc_ops = &mcp794xx_rtc_ops;
-		irq_handler = mcp794xx_irq;
-		break;
-	default:
-		break;
-	}
 
 read_rtc:
 	/* read RTC registers */
 	tmp = ds1307->read_block_data(ds1307->client, ds1307->offset, 8, buf);
 	if (tmp != 8) {
 		dev_dbg(&client->dev, "read error %d\n", tmp);
-		err = -EIO;
-		goto exit;
+		return -EIO;
 	}
 
 	/*
@@ -1535,8 +1476,7 @@ read_rtc:
 		tmp = i2c_smbus_read_byte_data(client, DS1340_REG_FLAG);
 		if (tmp < 0) {
 			dev_dbg(&client->dev, "read error %d\n", tmp);
-			err = -EIO;
-			goto exit;
+			return -EIO;
 		}
 
 		/* oscillator fault?  clear flag, and warn */
@@ -1566,6 +1506,79 @@ read_rtc:
 		break;
 	}
 
+	return 0;
+}
+
+static int ds1307_probe(struct i2c_client *client,
+			const struct i2c_device_id *id)
+{
+	struct ds1307		*ds1307;
+	int			err = -ENODEV;
+	int			tmp;
+	struct chip_desc	*chip = &chips[id->driver_data];
+	struct i2c_adapter	*adapter = to_i2c_adapter(client->dev.parent);
+	unsigned char		*buf;
+	struct ds1307_platform_data *pdata = dev_get_platdata(&client->dev);
+	irq_handler_t	irq_handler = ds1307_irq;
+
+	const struct rtc_class_ops *rtc_ops = &ds13xx_rtc_ops;
+
+	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA)
+	    && !i2c_check_functionality(adapter, I2C_FUNC_SMBUS_I2C_BLOCK))
+		return -EIO;
+
+	ds1307 = devm_kzalloc(&client->dev, sizeof(struct ds1307), GFP_KERNEL);
+	if (!ds1307)
+		return -ENOMEM;
+
+	i2c_set_clientdata(client, ds1307);
+
+	ds1307->client	= client;
+	ds1307->type	= id->driver_data;
+
+	if (!pdata && client->dev.of_node)
+		ds1307_trickle_of_init(client, chip);
+	else if (pdata && pdata->trickle_charger_setup)
+		chip->trickle_charger_setup = pdata->trickle_charger_setup;
+
+	if (chip->trickle_charger_setup && chip->trickle_charger_reg) {
+		dev_dbg(&client->dev, "writing trickle charger info 0x%x to 0x%x\n",
+		    DS13XX_TRICKLE_CHARGER_MAGIC | chip->trickle_charger_setup,
+		    chip->trickle_charger_reg);
+		i2c_smbus_write_byte_data(client, chip->trickle_charger_reg,
+		    DS13XX_TRICKLE_CHARGER_MAGIC |
+		    chip->trickle_charger_setup);
+	}
+
+	buf = ds1307->regs;
+	if (i2c_check_functionality(adapter, I2C_FUNC_SMBUS_I2C_BLOCK)) {
+		ds1307->read_block_data = ds1307_native_smbus_read_block_data;
+		ds1307->write_block_data = ds1307_native_smbus_write_block_data;
+	} else {
+		ds1307->read_block_data = ds1307_read_block_data;
+		ds1307->write_block_data = ds1307_write_block_data;
+	}
+
+	err = ds1307_chip_configure(ds1307);
+	if (err < 0)
+		return err;
+
+	switch (ds1307->type) {
+	case ds_1388:
+		ds1307->offset = 1; /* Seconds starts at 1 */
+		break;
+	case mcp794xx:
+		rtc_ops = &mcp794xx_rtc_ops;
+		irq_handler = mcp794xx_irq;
+		break;
+	default:
+		break;
+	}
+
+	err = ds1307_chip_sanity_check(ds1307);
+	if (err < 0)
+		return err;
+
 	tmp = ds1307->regs[DS1307_REG_HOUR];
 	switch (ds1307->type) {
 	case ds_1340:
@@ -1663,9 +1676,6 @@ read_rtc:
 	ds1307_clks_register(ds1307);
 
 	return 0;
-
-exit:
-	return err;
 }
 
 static int ds1307_remove(struct i2c_client *client)
-- 
2.5.5

[PATCH v2 09/17] RTC: ds1307: Remove register "cache"

[Andrey Smirnov]

Remove shared area used by many subroutines to store values of RTC's
registers. There wasn't very much caching or sharing going on in the
code and that register cache, being a semi-global variable, only created
additional implicit dependencies between function and made code more
confusing (there were a number of functions that defined a convenience
variable pointing to ds1307->regs, but failed to use it in the code
consistently).

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 drivers/rtc/rtc-ds1307.c | 229 ++++++++++++++++++++++++-----------------------
 1 file changed, 116 insertions(+), 113 deletions(-)

diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
index d262db5..66e7168 100644
--- a/drivers/rtc/rtc-ds1307.c
+++ b/drivers/rtc/rtc-ds1307.c
@@ -110,10 +110,10 @@ enum ds_type {
 #	define RX8025_BIT_VDET		0x40
 #	define RX8025_BIT_XST		0x20
 
+#define DS1307_REG_COUNT	11
 
 struct ds1307 {
 	u8			offset; /* register's offset */
-	u8			regs[11];
 	u16			nvram_offset;
 	struct bin_attribute	*nvram;
 	enum ds_type		type;
@@ -366,30 +366,31 @@ out:
 
 static int ds1307_get_time(struct device *dev, struct rtc_time *t)
 {
+	u8              regs[DS1307_REG_COUNT];
 	struct ds1307	*ds1307 = dev_get_drvdata(dev);
 	int		tmp;
 
 	/* read the RTC date and time registers all at once */
 	tmp = ds1307->read_block_data(ds1307->client,
-		ds1307->offset, 7, ds1307->regs);
+		ds1307->offset, 7, regs);
 	if (tmp != 7) {
 		dev_err(dev, "%s error %d\n", "read", tmp);
 		return -EIO;
 	}
 
-	dev_dbg(dev, "%s: %7ph\n", "read", ds1307->regs);
+	dev_dbg(dev, "%s: %7ph\n", "read", regs);
 
-	t->tm_sec = bcd2bin(ds1307->regs[DS1307_REG_SECS] & 0x7f);
-	t->tm_min = bcd2bin(ds1307->regs[DS1307_REG_MIN] & 0x7f);
-	tmp = ds1307->regs[DS1307_REG_HOUR] & 0x3f;
+	t->tm_sec = bcd2bin(regs[DS1307_REG_SECS] & 0x7f);
+	t->tm_min = bcd2bin(regs[DS1307_REG_MIN] & 0x7f);
+	tmp = regs[DS1307_REG_HOUR] & 0x3f;
 	t->tm_hour = bcd2bin(tmp);
-	t->tm_wday = bcd2bin(ds1307->regs[DS1307_REG_WDAY] & 0x07) - 1;
-	t->tm_mday = bcd2bin(ds1307->regs[DS1307_REG_MDAY] & 0x3f);
-	tmp = ds1307->regs[DS1307_REG_MONTH] & 0x1f;
+	t->tm_wday = bcd2bin(regs[DS1307_REG_WDAY] & 0x07) - 1;
+	t->tm_mday = bcd2bin(regs[DS1307_REG_MDAY] & 0x3f);
+	tmp = regs[DS1307_REG_MONTH] & 0x1f;
 	t->tm_mon = bcd2bin(tmp) - 1;
 
 	/* assume 20YY not 19YY, and ignore DS1337_BIT_CENTURY */
-	t->tm_year = bcd2bin(ds1307->regs[DS1307_REG_YEAR]) + 100;
+	t->tm_year = bcd2bin(regs[DS1307_REG_YEAR]) + 100;
 
 	dev_dbg(dev, "%s secs=%d, mins=%d, "
 		"hours=%d, mday=%d, mon=%d, year=%d, wday=%d\n",
@@ -406,7 +407,7 @@ static int ds1307_set_time(struct device *dev, struct rtc_time *t)
 	struct ds1307	*ds1307 = dev_get_drvdata(dev);
 	int		result;
 	int		tmp;
-	u8		*buf = ds1307->regs;
+	u8		regs[DS1307_REG_COUNT];
 
 	dev_dbg(dev, "%s secs=%d, mins=%d, "
 		"hours=%d, mday=%d, mon=%d, year=%d, wday=%d\n",
@@ -414,26 +415,26 @@ static int ds1307_set_time(struct device *dev, struct rtc_time *t)
 		t->tm_hour, t->tm_mday,
 		t->tm_mon, t->tm_year, t->tm_wday);
 
-	buf[DS1307_REG_SECS] = bin2bcd(t->tm_sec);
-	buf[DS1307_REG_MIN] = bin2bcd(t->tm_min);
-	buf[DS1307_REG_HOUR] = bin2bcd(t->tm_hour);
-	buf[DS1307_REG_WDAY] = bin2bcd(t->tm_wday + 1);
-	buf[DS1307_REG_MDAY] = bin2bcd(t->tm_mday);
-	buf[DS1307_REG_MONTH] = bin2bcd(t->tm_mon + 1);
+	regs[DS1307_REG_SECS]  = bin2bcd(t->tm_sec);
+	regs[DS1307_REG_MIN]   = bin2bcd(t->tm_min);
+	regs[DS1307_REG_HOUR]  = bin2bcd(t->tm_hour);
+	regs[DS1307_REG_WDAY]  = bin2bcd(t->tm_wday + 1);
+	regs[DS1307_REG_MDAY]  = bin2bcd(t->tm_mday);
+	regs[DS1307_REG_MONTH] = bin2bcd(t->tm_mon + 1);
 
 	/* assume 20YY not 19YY */
 	tmp = t->tm_year - 100;
-	buf[DS1307_REG_YEAR] = bin2bcd(tmp);
+	regs[DS1307_REG_YEAR] = bin2bcd(tmp);
 
 	switch (ds1307->type) {
 	case ds_1337:
 	case ds_1339:
 	case ds_3231:
 	case ds_1341:
-		buf[DS1307_REG_MONTH] |= DS1337_BIT_CENTURY;
+		regs[DS1307_REG_MONTH] |= DS1337_BIT_CENTURY;
 		break;
 	case ds_1340:
-		buf[DS1307_REG_HOUR] |= DS1340_BIT_CENTURY_EN
+		regs[DS1307_REG_HOUR] |= DS1340_BIT_CENTURY_EN
 				| DS1340_BIT_CENTURY;
 		break;
 	case mcp794xx:
@@ -442,17 +443,17 @@ static int ds1307_set_time(struct device *dev, struct rtc_time *t)
 		 * values and need to be set again before writing the
 		 * buffer out to the device.
 		 */
-		buf[DS1307_REG_SECS] |= MCP794XX_BIT_ST;
-		buf[DS1307_REG_WDAY] |= MCP794XX_BIT_VBATEN;
+		regs[DS1307_REG_SECS] |= MCP794XX_BIT_ST;
+		regs[DS1307_REG_WDAY] |= MCP794XX_BIT_VBATEN;
 		break;
 	default:
 		break;
 	}
 
-	dev_dbg(dev, "%s: %7ph\n", "write", buf);
+	dev_dbg(dev, "%s: %7ph\n", "write", regs);
 
 	result = ds1307->write_block_data(ds1307->client,
-		ds1307->offset, 7, buf);
+		ds1307->offset, 7, regs);
 	if (result < 0) {
 		dev_err(dev, "%s error %d\n", "write", result);
 		return result;
@@ -465,29 +466,30 @@ static int ds1337_read_alarm(struct device *dev, struct rtc_wkalrm *t)
 	struct i2c_client       *client = to_i2c_client(dev);
 	struct ds1307		*ds1307 = i2c_get_clientdata(client);
 	int			ret;
+	u8			regs[DS1307_REG_COUNT];
 
 	if (!test_bit(HAS_ALARM, &ds1307->flags))
 		return -EINVAL;
 
 	/* read all ALARM1, ALARM2, and status registers at once */
 	ret = ds1307->read_block_data(client,
-			DS1339_REG_ALARM1_SECS, 9, ds1307->regs);
+			DS1339_REG_ALARM1_SECS, 9, regs);
 	if (ret != 9) {
 		dev_err(dev, "%s error %d\n", "alarm read", ret);
 		return -EIO;
 	}
 
 	dev_dbg(dev, "%s: %4ph, %3ph, %2ph\n", "alarm read",
-		&ds1307->regs[0], &ds1307->regs[4], &ds1307->regs[7]);
+		&regs[0], &regs[4], &regs[7]);
 
 	/*
 	 * report alarm time (ALARM1); assume 24 hour and day-of-month modes,
 	 * and that all four fields are checked matches
 	 */
-	t->time.tm_sec = bcd2bin(ds1307->regs[0] & 0x7f);
-	t->time.tm_min = bcd2bin(ds1307->regs[1] & 0x7f);
-	t->time.tm_hour = bcd2bin(ds1307->regs[2] & 0x3f);
-	t->time.tm_mday = bcd2bin(ds1307->regs[3] & 0x3f);
+	t->time.tm_sec = bcd2bin(regs[0] & 0x7f);
+	t->time.tm_min = bcd2bin(regs[1] & 0x7f);
+	t->time.tm_hour = bcd2bin(regs[2] & 0x3f);
+	t->time.tm_mday = bcd2bin(regs[3] & 0x3f);
 	t->time.tm_mon = -1;
 	t->time.tm_year = -1;
 	t->time.tm_wday = -1;
@@ -495,8 +497,8 @@ static int ds1337_read_alarm(struct device *dev, struct rtc_wkalrm *t)
 	t->time.tm_isdst = -1;
 
 	/* ... and status */
-	t->enabled = !!(ds1307->regs[7] & DS1337_BIT_A1IE);
-	t->pending = !!(ds1307->regs[8] & DS1337_BIT_A1I);
+	t->enabled = !!(regs[7] & DS1337_BIT_A1IE);
+	t->pending = !!(regs[8] & DS1337_BIT_A1I);
 
 	dev_dbg(dev, "%s secs=%d, mins=%d, "
 		"hours=%d, mday=%d, enabled=%d, pending=%d\n",
@@ -511,7 +513,7 @@ static int ds1337_set_alarm(struct device *dev, struct rtc_wkalrm *t)
 {
 	struct i2c_client	*client = to_i2c_client(dev);
 	struct ds1307		*ds1307 = i2c_get_clientdata(client);
-	unsigned char		*buf = ds1307->regs;
+	u8			regs[DS1307_REG_COUNT];
 	u8			control, status;
 	int			ret;
 
@@ -526,34 +528,34 @@ static int ds1337_set_alarm(struct device *dev, struct rtc_wkalrm *t)
 
 	/* read current status of both alarms and the chip */
 	ret = ds1307->read_block_data(client,
-			DS1339_REG_ALARM1_SECS, 9, buf);
+			DS1339_REG_ALARM1_SECS, 9, regs);
 	if (ret != 9) {
 		dev_err(dev, "%s error %d\n", "alarm write", ret);
 		return -EIO;
 	}
-	control = ds1307->regs[7];
-	status = ds1307->regs[8];
+	control = regs[7];
+	status  = regs[8];
 
 	dev_dbg(dev, "%s: %4ph, %3ph, %02x %02x\n", "alarm set (old status)",
-		&ds1307->regs[0], &ds1307->regs[4], control, status);
+		&regs[0], &regs[4], control, status);
 
 	/* set ALARM1, using 24 hour and day-of-month modes */
-	buf[0] = bin2bcd(t->time.tm_sec);
-	buf[1] = bin2bcd(t->time.tm_min);
-	buf[2] = bin2bcd(t->time.tm_hour);
-	buf[3] = bin2bcd(t->time.tm_mday);
+	regs[0] = bin2bcd(t->time.tm_sec);
+	regs[1] = bin2bcd(t->time.tm_min);
+	regs[2] = bin2bcd(t->time.tm_hour);
+	regs[3] = bin2bcd(t->time.tm_mday);
 
 	/* set ALARM2 to non-garbage */
-	buf[4] = 0;
-	buf[5] = 0;
-	buf[6] = 0;
+	regs[4] = 0;
+	regs[5] = 0;
+	regs[6] = 0;
 
 	/* disable alarms */
-	buf[7] = control & ~(DS1337_BIT_A1IE | DS1337_BIT_A2IE);
-	buf[8] = status & ~(DS1337_BIT_A1I | DS1337_BIT_A2I);
+	regs[7] = control & ~(DS1337_BIT_A1IE | DS1337_BIT_A2IE);
+	regs[8] = status & ~(DS1337_BIT_A1I | DS1337_BIT_A2I);
 
 	ret = ds1307->write_block_data(client,
-			DS1339_REG_ALARM1_SECS, 9, buf);
+			DS1339_REG_ALARM1_SECS, 9, regs);
 	if (ret < 0) {
 		dev_err(dev, "can't set alarm time\n");
 		return ret;
@@ -562,8 +564,8 @@ static int ds1337_set_alarm(struct device *dev, struct rtc_wkalrm *t)
 	/* optionally enable ALARM1 */
 	if (t->enabled) {
 		dev_dbg(dev, "alarm IRQ armed\n");
-		buf[7] |= DS1337_BIT_A1IE;	/* only ALARM1 is used */
-		i2c_smbus_write_byte_data(client, DS1337_REG_CONTROL, buf[7]);
+		regs[7] |= DS1337_BIT_A1IE;	/* only ALARM1 is used */
+		i2c_smbus_write_byte_data(client, DS1337_REG_CONTROL, regs[7]);
 	}
 
 	return 0;
@@ -665,7 +667,7 @@ static int mcp794xx_read_alarm(struct device *dev, struct rtc_wkalrm *t)
 {
 	struct i2c_client *client = to_i2c_client(dev);
 	struct ds1307 *ds1307 = i2c_get_clientdata(client);
-	u8 *regs = ds1307->regs;
+	u8  regs[DS1307_REG_COUNT];
 	int ret;
 
 	if (!test_bit(HAS_ALARM, &ds1307->flags))
@@ -679,12 +681,12 @@ static int mcp794xx_read_alarm(struct device *dev, struct rtc_wkalrm *t)
 	t->enabled = !!(regs[0] & MCP794XX_BIT_ALM0_EN);
 
 	/* Report alarm 0 time assuming 24-hour and day-of-month modes. */
-	t->time.tm_sec = bcd2bin(ds1307->regs[3] & 0x7f);
-	t->time.tm_min = bcd2bin(ds1307->regs[4] & 0x7f);
-	t->time.tm_hour = bcd2bin(ds1307->regs[5] & 0x3f);
-	t->time.tm_wday = bcd2bin(ds1307->regs[6] & 0x7) - 1;
-	t->time.tm_mday = bcd2bin(ds1307->regs[7] & 0x3f);
-	t->time.tm_mon = bcd2bin(ds1307->regs[8] & 0x1f) - 1;
+	t->time.tm_sec = bcd2bin(regs[3] & 0x7f);
+	t->time.tm_min = bcd2bin(regs[4] & 0x7f);
+	t->time.tm_hour = bcd2bin(regs[5] & 0x3f);
+	t->time.tm_wday = bcd2bin(regs[6] & 0x7) - 1;
+	t->time.tm_mday = bcd2bin(regs[7] & 0x3f);
+	t->time.tm_mon = bcd2bin(regs[8] & 0x1f) - 1;
 	t->time.tm_year = -1;
 	t->time.tm_yday = -1;
 	t->time.tm_isdst = -1;
@@ -693,9 +695,9 @@ static int mcp794xx_read_alarm(struct device *dev, struct rtc_wkalrm *t)
 		"enabled=%d polarity=%d irq=%d match=%d\n", __func__,
 		t->time.tm_sec, t->time.tm_min, t->time.tm_hour,
 		t->time.tm_wday, t->time.tm_mday, t->time.tm_mon, t->enabled,
-		!!(ds1307->regs[6] & MCP794XX_BIT_ALMX_POL),
-		!!(ds1307->regs[6] & MCP794XX_BIT_ALMX_IF),
-		(ds1307->regs[6] & MCP794XX_MSK_ALMX_MATCH) >> 4);
+		!!(regs[6] & MCP794XX_BIT_ALMX_POL),
+		!!(regs[6] & MCP794XX_BIT_ALMX_IF),
+		(regs[6] & MCP794XX_MSK_ALMX_MATCH) >> 4);
 
 	return 0;
 }
@@ -704,7 +706,7 @@ static int mcp794xx_set_alarm(struct device *dev, struct rtc_wkalrm *t)
 {
 	struct i2c_client *client = to_i2c_client(dev);
 	struct ds1307 *ds1307 = i2c_get_clientdata(client);
-	unsigned char *regs = ds1307->regs;
+	u8  regs[DS1307_REG_COUNT];
 	int ret;
 
 	if (!test_bit(HAS_ALARM, &ds1307->flags))
@@ -1274,11 +1276,9 @@ static bool ds1307_want_irq(const struct ds1307 *ds1307,
 static int ds1307_chip_configure(struct ds1307 *ds1307)
 {
 	int tmp;
-	unsigned char *buf;
+	u8  regs[DS1307_REG_COUNT];
 	struct i2c_client *client = ds1307->client;
 
-	buf = ds1307->regs;
-
 	switch (ds1307->type) {
 	case ds_1337:
 	case ds_1339:
@@ -1293,32 +1293,32 @@ static int ds1307_chip_configure(struct ds1307 *ds1307)
 
 		/* get registers that the "rtc" read below won't read... */
 		tmp = ds1307->read_block_data(client,
-					      DS1337_REG_CONTROL, 2, buf);
+					      DS1337_REG_CONTROL, 2, regs);
 		if (tmp != 2) {
 			dev_dbg(&ds1307->client->dev, "read error %d\n", tmp);
 			return -EIO;
 		}
 
 		/* oscillator off?  turn it on, so clock can tick. */
-		if (ds1307->regs[0] & DS1337_BIT_nEOSC)
-			ds1307->regs[0] &= ~DS1337_BIT_nEOSC;
+		if (regs[0] & DS1337_BIT_nEOSC)
+			regs[0] &= ~DS1337_BIT_nEOSC;
 
 		if (ds1307->type == ds_1341) {
 			/* Make sure we are not generating square wave
 			 * output */
-			ds1307->regs[1] &= ~DS1341_BIT_ECLK;
+			regs[1] &= ~DS1341_BIT_ECLK;
 
 			if (of_property_read_bool(client->dev.of_node,
 						  "dallas,disable-oscillator-stop-flag"))
-				ds1307->regs[1] |= DS1341_BIT_DOSF;
+				regs[1] |= DS1341_BIT_DOSF;
 			else
-				ds1307->regs[1] &= ~DS1341_BIT_DOSF;
+				regs[1] &= ~DS1341_BIT_DOSF;
 
 			if (of_property_read_bool(client->dev.of_node,
 						  "dallas,enable-glitch-filter"))
-				ds1307->regs[0] |= DS1341_BIT_EGFIL;
+				regs[0] |= DS1341_BIT_EGFIL;
 			else
-				ds1307->regs[0] &= ~DS1341_BIT_EGFIL;
+				regs[0] &= ~DS1341_BIT_EGFIL;
 
 			/*
 			 * Write status register. Control register
@@ -1326,7 +1326,7 @@ static int ds1307_chip_configure(struct ds1307 *ds1307)
 			 */
 			i2c_smbus_write_byte_data(client,
 						  DS1337_REG_STATUS,
-						  ds1307->regs[1]);
+						  regs[1]);
 		}
 
 		/*
@@ -1334,19 +1334,19 @@ static int ds1307_chip_configure(struct ds1307 *ds1307)
 		 * For some variants, be sure alarms can trigger when we're
 		 * running on Vbackup (BBSQI/BBSQW)
 		 */
-		ds1307->regs[0] |= DS1337_BIT_INTCN
+		regs[0] |= DS1337_BIT_INTCN
 			| bbsqi_bitpos[ds1307->type];
-		ds1307->regs[0] &= ~(DS1337_BIT_A2IE | DS1337_BIT_A1IE);
+		regs[0] &= ~(DS1337_BIT_A2IE | DS1337_BIT_A1IE);
 
 		i2c_smbus_write_byte_data(client,
 					  DS1337_REG_CONTROL,
-					  ds1307->regs[0]);
+					  regs[0]);
 
 		/* oscillator fault?  clear flag, and warn */
-		if (ds1307->regs[1] & DS1337_BIT_OSF) {
+		if (regs[1] & DS1337_BIT_OSF) {
 			i2c_smbus_write_byte_data(client,
 						  DS1337_REG_STATUS,
-						  ds1307->regs[1] & ~DS1337_BIT_OSF);
+						  regs[1] & ~DS1337_BIT_OSF);
 			dev_warn(&ds1307->client->dev, "SET TIME!\n");
 		}
 		break;
@@ -1354,60 +1354,60 @@ static int ds1307_chip_configure(struct ds1307 *ds1307)
 	case rx_8025:
 		tmp = i2c_smbus_read_i2c_block_data(client,
 						    RX8025_REG_CTRL1 << 4 | 0x08,
-						    2, buf);
+						    2, regs);
 		if (tmp != 2) {
 			dev_dbg(&client->dev, "read error %d\n", tmp);
 			return -EIO;
 		}
 
 		/* oscillator off?  turn it on, so clock can tick. */
-		if (!(ds1307->regs[1] & RX8025_BIT_XST)) {
-			ds1307->regs[1] |= RX8025_BIT_XST;
+		if (!(regs[1] & RX8025_BIT_XST)) {
+			regs[1] |= RX8025_BIT_XST;
 			i2c_smbus_write_byte_data(client,
 						  RX8025_REG_CTRL2 << 4 | 0x08,
-						  ds1307->regs[1]);
+						  regs[1]);
 			dev_warn(&client->dev,
 				 "oscillator stop detected - SET TIME!\n");
 		}
 
-		if (ds1307->regs[1] & RX8025_BIT_PON) {
-			ds1307->regs[1] &= ~RX8025_BIT_PON;
+		if (regs[1] & RX8025_BIT_PON) {
+			regs[1] &= ~RX8025_BIT_PON;
 			i2c_smbus_write_byte_data(client,
 						  RX8025_REG_CTRL2 << 4 | 0x08,
-						  ds1307->regs[1]);
+						  regs[1]);
 			dev_warn(&client->dev, "power-on detected\n");
 		}
 
-		if (ds1307->regs[1] & RX8025_BIT_VDET) {
-			ds1307->regs[1] &= ~RX8025_BIT_VDET;
+		if (regs[1] & RX8025_BIT_VDET) {
+			regs[1] &= ~RX8025_BIT_VDET;
 			i2c_smbus_write_byte_data(client,
 						  RX8025_REG_CTRL2 << 4 | 0x08,
-						  ds1307->regs[1]);
+						  regs[1]);
 			dev_warn(&client->dev, "voltage drop detected\n");
 		}
 
 		/* make sure we are running in 24hour mode */
-		if (!(ds1307->regs[0] & RX8025_BIT_2412)) {
+		if (!(regs[0] & RX8025_BIT_2412)) {
 			u8 hour;
 
 			/* switch to 24 hour mode */
 			i2c_smbus_write_byte_data(client,
 						  RX8025_REG_CTRL1 << 4 | 0x08,
-						  ds1307->regs[0] | RX8025_BIT_2412);
+						  regs[0] | RX8025_BIT_2412);
 
 			tmp = i2c_smbus_read_i2c_block_data(client,
 							    RX8025_REG_CTRL1 << 4 | 0x08,
-							    2, buf);
+							    2, regs);
 			if (tmp != 2) {
 				dev_dbg(&client->dev, "read error %d\n", tmp);
 				return -EIO;
 			}
 
 			/* correct hour */
-			hour = bcd2bin(ds1307->regs[DS1307_REG_HOUR]);
+			hour = bcd2bin(regs[DS1307_REG_HOUR]);
 			if (hour == 12)
 				hour = 0;
-			if (ds1307->regs[DS1307_REG_HOUR] & DS1307_BIT_PM)
+			if (regs[DS1307_REG_HOUR] & DS1307_BIT_PM)
 				hour += 12;
 
 			i2c_smbus_write_byte_data(client,
@@ -1425,14 +1425,12 @@ static int ds1307_chip_configure(struct ds1307 *ds1307)
 static int ds1307_chip_sanity_check(struct ds1307 *ds1307)
 {
 	int tmp;
-	unsigned char *buf;
+	u8  regs[DS1307_REG_COUNT];
 	struct i2c_client *client = ds1307->client;
 
-	buf = ds1307->regs;
-
 read_rtc:
 	/* read RTC registers */
-	tmp = ds1307->read_block_data(ds1307->client, ds1307->offset, 8, buf);
+	tmp = ds1307->read_block_data(ds1307->client, ds1307->offset, 8, regs);
 	if (tmp != 8) {
 		dev_dbg(&client->dev, "read error %d\n", tmp);
 		return -EIO;
@@ -1443,7 +1441,7 @@ read_rtc:
 	 * specify the extra bits as must-be-zero, but there are
 	 * still a few values that are clearly out-of-range.
 	 */
-	tmp = ds1307->regs[DS1307_REG_SECS];
+	tmp = regs[DS1307_REG_SECS];
 	switch (ds1307->type) {
 	case ds_1307:
 	case m41t00:
@@ -1460,9 +1458,9 @@ read_rtc:
 			i2c_smbus_write_byte_data(client, DS1307_REG_SECS, 0);
 
 		/* oscillator fault?  clear flag, and warn */
-		if (ds1307->regs[DS1307_REG_CONTROL] & DS1338_BIT_OSF) {
+		if (regs[DS1307_REG_CONTROL] & DS1338_BIT_OSF) {
 			i2c_smbus_write_byte_data(client, DS1307_REG_CONTROL,
-					ds1307->regs[DS1307_REG_CONTROL]
+					regs[DS1307_REG_CONTROL]
 					& ~DS1338_BIT_OSF);
 			dev_warn(&client->dev, "SET TIME!\n");
 			goto read_rtc;
@@ -1487,9 +1485,9 @@ read_rtc:
 		break;
 	case mcp794xx:
 		/* make sure that the backup battery is enabled */
-		if (!(ds1307->regs[DS1307_REG_WDAY] & MCP794XX_BIT_VBATEN)) {
+		if (!(regs[DS1307_REG_WDAY] & MCP794XX_BIT_VBATEN)) {
 			i2c_smbus_write_byte_data(client, DS1307_REG_WDAY,
-					ds1307->regs[DS1307_REG_WDAY]
+					regs[DS1307_REG_WDAY]
 					| MCP794XX_BIT_VBATEN);
 		}
 
@@ -1514,10 +1512,10 @@ static int ds1307_probe(struct i2c_client *client,
 {
 	struct ds1307		*ds1307;
 	int			err = -ENODEV;
-	int			tmp;
+	int			reg;
+	u8			hour;
 	struct chip_desc	*chip = &chips[id->driver_data];
 	struct i2c_adapter	*adapter = to_i2c_adapter(client->dev.parent);
-	unsigned char		*buf;
 	struct ds1307_platform_data *pdata = dev_get_platdata(&client->dev);
 	irq_handler_t	irq_handler = ds1307_irq;
 
@@ -1550,7 +1548,6 @@ static int ds1307_probe(struct i2c_client *client,
 		    chip->trickle_charger_setup);
 	}
 
-	buf = ds1307->regs;
 	if (i2c_check_functionality(adapter, I2C_FUNC_SMBUS_I2C_BLOCK)) {
 		ds1307->read_block_data = ds1307_native_smbus_read_block_data;
 		ds1307->write_block_data = ds1307_native_smbus_write_block_data;
@@ -1579,7 +1576,14 @@ static int ds1307_probe(struct i2c_client *client,
 	if (err < 0)
 		return err;
 
-	tmp = ds1307->regs[DS1307_REG_HOUR];
+	reg = i2c_smbus_read_byte_data(client,
+				       ds1307->offset + DS1307_REG_HOUR);
+	if (reg < 0) {
+		dev_err(&client->dev,
+			"failed to read HOUR register\n");
+		return reg;
+	}
+
 	switch (ds1307->type) {
 	case ds_1340:
 	case m41t00:
@@ -1591,21 +1595,21 @@ static int ds1307_probe(struct i2c_client *client,
 	case rx_8025:
 		break;
 	default:
-		if (!(tmp & DS1307_BIT_12HR))
+		if (!(reg & DS1307_BIT_12HR))
 			break;
 
 		/*
 		 * Be sure we're in 24 hour mode.  Multi-master systems
 		 * take note...
 		 */
-		tmp = bcd2bin(tmp & 0x1f);
-		if (tmp == 12)
-			tmp = 0;
-		if (ds1307->regs[DS1307_REG_HOUR] & DS1307_BIT_PM)
-			tmp += 12;
+		hour = bcd2bin(reg & 0x1f);
+		if (hour == 12)
+			hour = 0;
+		if (reg & DS1307_BIT_PM)
+			hour += 12;
 		i2c_smbus_write_byte_data(client,
 				ds1307->offset + DS1307_REG_HOUR,
-				bin2bcd(tmp));
+				bin2bcd(hour));
 	}
 
 	if (ds1307_want_irq(ds1307, chip)) {
@@ -1620,7 +1624,6 @@ static int ds1307_probe(struct i2c_client *client,
 
 	if (ds1307_can_wakeup_device(ds1307) &&
 	    ds1307->client->irq <= 0) {
-		/* Disable request for an IRQ */
 		dev_info(&client->dev,
 			 "'wakeup-source' is set, request for an IRQ is disabled!\n");
 		/* We cannot support UIE mode if we do not have an IRQ line */
-- 
2.5.5

[PATCH v2 10/17] RTC: ds1307: Constify struct ds1307 where possible

[Andrey Smirnov]

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 drivers/rtc/rtc-ds1307.c | 50 ++++++++++++++++++++++++------------------------
 1 file changed, 25 insertions(+), 25 deletions(-)

diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
index 66e7168..67907f1 100644
--- a/drivers/rtc/rtc-ds1307.c
+++ b/drivers/rtc/rtc-ds1307.c
@@ -333,7 +333,7 @@ static s32 ds1307_native_smbus_read_block_data(const struct i2c_client *client,
 static irqreturn_t ds1307_irq(int irq, void *dev_id)
 {
 	struct i2c_client	*client = dev_id;
-	struct ds1307		*ds1307 = i2c_get_clientdata(client);
+	const struct ds1307	*ds1307 = i2c_get_clientdata(client);
 	struct mutex		*lock = &ds1307->rtc->ops_lock;
 	int			stat, control;
 
@@ -367,8 +367,8 @@ out:
 static int ds1307_get_time(struct device *dev, struct rtc_time *t)
 {
 	u8              regs[DS1307_REG_COUNT];
-	struct ds1307	*ds1307 = dev_get_drvdata(dev);
 	int		tmp;
+	const struct ds1307 *ds1307 = dev_get_drvdata(dev);
 
 	/* read the RTC date and time registers all at once */
 	tmp = ds1307->read_block_data(ds1307->client,
@@ -404,7 +404,7 @@ static int ds1307_get_time(struct device *dev, struct rtc_time *t)
 
 static int ds1307_set_time(struct device *dev, struct rtc_time *t)
 {
-	struct ds1307	*ds1307 = dev_get_drvdata(dev);
+	const struct ds1307 *ds1307 = dev_get_drvdata(dev);
 	int		result;
 	int		tmp;
 	u8		regs[DS1307_REG_COUNT];
@@ -464,7 +464,7 @@ static int ds1307_set_time(struct device *dev, struct rtc_time *t)
 static int ds1337_read_alarm(struct device *dev, struct rtc_wkalrm *t)
 {
 	struct i2c_client       *client = to_i2c_client(dev);
-	struct ds1307		*ds1307 = i2c_get_clientdata(client);
+	const struct ds1307	*ds1307 = i2c_get_clientdata(client);
 	int			ret;
 	u8			regs[DS1307_REG_COUNT];
 
@@ -512,7 +512,7 @@ static int ds1337_read_alarm(struct device *dev, struct rtc_wkalrm *t)
 static int ds1337_set_alarm(struct device *dev, struct rtc_wkalrm *t)
 {
 	struct i2c_client	*client = to_i2c_client(dev);
-	struct ds1307		*ds1307 = i2c_get_clientdata(client);
+	const struct ds1307	*ds1307 = i2c_get_clientdata(client);
 	u8			regs[DS1307_REG_COUNT];
 	u8			control, status;
 	int			ret;
@@ -574,7 +574,7 @@ static int ds1337_set_alarm(struct device *dev, struct rtc_wkalrm *t)
 static int ds1307_alarm_irq_enable(struct device *dev, unsigned int enabled)
 {
 	struct i2c_client	*client = to_i2c_client(dev);
-	struct ds1307		*ds1307 = i2c_get_clientdata(client);
+	const struct ds1307	*ds1307 = i2c_get_clientdata(client);
 	int			ret;
 
 	if (!test_bit(HAS_ALARM, &ds1307->flags))
@@ -629,7 +629,7 @@ static const struct rtc_class_ops ds13xx_rtc_ops = {
 static irqreturn_t mcp794xx_irq(int irq, void *dev_id)
 {
 	struct i2c_client       *client = dev_id;
-	struct ds1307           *ds1307 = i2c_get_clientdata(client);
+	const struct ds1307     *ds1307 = i2c_get_clientdata(client);
 	struct mutex            *lock = &ds1307->rtc->ops_lock;
 	int reg, ret;
 
@@ -666,7 +666,7 @@ out:
 static int mcp794xx_read_alarm(struct device *dev, struct rtc_wkalrm *t)
 {
 	struct i2c_client *client = to_i2c_client(dev);
-	struct ds1307 *ds1307 = i2c_get_clientdata(client);
+	const struct ds1307 *ds1307 = i2c_get_clientdata(client);
 	u8  regs[DS1307_REG_COUNT];
 	int ret;
 
@@ -705,7 +705,7 @@ static int mcp794xx_read_alarm(struct device *dev, struct rtc_wkalrm *t)
 static int mcp794xx_set_alarm(struct device *dev, struct rtc_wkalrm *t)
 {
 	struct i2c_client *client = to_i2c_client(dev);
-	struct ds1307 *ds1307 = i2c_get_clientdata(client);
+	const struct ds1307 *ds1307 = i2c_get_clientdata(client);
 	u8  regs[DS1307_REG_COUNT];
 	int ret;
 
@@ -751,7 +751,7 @@ static int mcp794xx_set_alarm(struct device *dev, struct rtc_wkalrm *t)
 static int mcp794xx_alarm_irq_enable(struct device *dev, unsigned int enabled)
 {
 	struct i2c_client *client = to_i2c_client(dev);
-	struct ds1307 *ds1307 = i2c_get_clientdata(client);
+	const struct ds1307 *ds1307 = i2c_get_clientdata(client);
 	int reg;
 
 	if (!test_bit(HAS_ALARM, &ds1307->flags))
@@ -785,7 +785,7 @@ ds1307_nvram_read(struct file *filp, struct kobject *kobj,
 		char *buf, loff_t off, size_t count)
 {
 	struct i2c_client	*client;
-	struct ds1307		*ds1307;
+	const struct ds1307	*ds1307;
 	int			result;
 
 	client = kobj_to_i2c_client(kobj);
@@ -804,7 +804,7 @@ ds1307_nvram_write(struct file *filp, struct kobject *kobj,
 		char *buf, loff_t off, size_t count)
 {
 	struct i2c_client	*client;
-	struct ds1307		*ds1307;
+	const struct ds1307	*ds1307;
 	int			result;
 
 	client = kobj_to_i2c_client(kobj);
@@ -880,7 +880,7 @@ out:
  */
 static int ds3231_hwmon_read_temp(struct device *dev, s32 *mC)
 {
-	struct ds1307 *ds1307 = dev_get_drvdata(dev);
+	const struct ds1307 *ds1307 = dev_get_drvdata(dev);
 	u8 temp_buf[2];
 	s16 temp;
 	int ret;
@@ -973,7 +973,7 @@ static int ds3231_clk_sqw_rates[] = {
 	8192,
 };
 
-static int ds1337_write_control(struct ds1307 *ds1307, u8 mask, u8 value)
+static int ds1337_write_control(const struct ds1307 *ds1307, u8 mask, u8 value)
 {
 	struct i2c_client *client = ds1307->client;
 	struct mutex *lock = &ds1307->rtc->ops_lock;
@@ -1001,7 +1001,7 @@ out:
 static unsigned long ds3231_clk_sqw_recalc_rate(struct clk_hw *hw,
 						unsigned long parent_rate)
 {
-	struct ds1307 *ds1307 = clk_sqw_to_ds1307(hw);
+	const struct ds1307 *ds1307 = clk_sqw_to_ds1307(hw);
 	int control;
 	int rate_sel = 0;
 
@@ -1032,7 +1032,7 @@ static long ds3231_clk_sqw_round_rate(struct clk_hw *hw, unsigned long rate,
 static int ds3231_clk_sqw_set_rate(struct clk_hw *hw, unsigned long rate,
 					unsigned long parent_rate)
 {
-	struct ds1307 *ds1307 = clk_sqw_to_ds1307(hw);
+	const struct ds1307 *ds1307 = clk_sqw_to_ds1307(hw);
 	int control = 0;
 	int rate_sel;
 
@@ -1056,21 +1056,21 @@ static int ds3231_clk_sqw_set_rate(struct clk_hw *hw, unsigned long rate,
 
 static int ds3231_clk_sqw_prepare(struct clk_hw *hw)
 {
-	struct ds1307 *ds1307 = clk_sqw_to_ds1307(hw);
+	const struct ds1307 *ds1307 = clk_sqw_to_ds1307(hw);
 
 	return ds1337_write_control(ds1307, DS1337_BIT_INTCN, 0);
 }
 
 static void ds3231_clk_sqw_unprepare(struct clk_hw *hw)
 {
-	struct ds1307 *ds1307 = clk_sqw_to_ds1307(hw);
+	const struct ds1307 *ds1307 = clk_sqw_to_ds1307(hw);
 
 	ds1337_write_control(ds1307, DS1337_BIT_INTCN, DS1337_BIT_INTCN);
 }
 
 static int ds3231_clk_sqw_is_prepared(struct clk_hw *hw)
 {
-	struct ds1307 *ds1307 = clk_sqw_to_ds1307(hw);
+	const struct ds1307 *ds1307 = clk_sqw_to_ds1307(hw);
 	int control;
 
 	control = i2c_smbus_read_byte_data(ds1307->client, DS1337_REG_CONTROL);
@@ -1095,7 +1095,7 @@ static unsigned long ds3231_clk_32khz_recalc_rate(struct clk_hw *hw,
 	return 32768;
 }
 
-static int ds3231_clk_32khz_control(struct ds1307 *ds1307, bool enable)
+static int ds3231_clk_32khz_control(const struct ds1307 *ds1307, bool enable)
 {
 	struct i2c_client *client = ds1307->client;
 	struct mutex *lock = &ds1307->rtc->ops_lock;
@@ -1124,21 +1124,21 @@ out:
 
 static int ds3231_clk_32khz_prepare(struct clk_hw *hw)
 {
-	struct ds1307 *ds1307 = clk_32khz_to_ds1307(hw);
+	const struct ds1307 *ds1307 = clk_32khz_to_ds1307(hw);
 
 	return ds3231_clk_32khz_control(ds1307, true);
 }
 
 static void ds3231_clk_32khz_unprepare(struct clk_hw *hw)
 {
-	struct ds1307 *ds1307 = clk_32khz_to_ds1307(hw);
+	const struct ds1307 *ds1307 = clk_32khz_to_ds1307(hw);
 
 	ds3231_clk_32khz_control(ds1307, false);
 }
 
 static int ds3231_clk_32khz_is_prepared(struct clk_hw *hw)
 {
-	struct ds1307 *ds1307 = clk_32khz_to_ds1307(hw);
+	const struct ds1307 *ds1307 = clk_32khz_to_ds1307(hw);
 	int status;
 
 	status = i2c_smbus_read_byte_data(ds1307->client, DS1337_REG_STATUS);
@@ -1273,7 +1273,7 @@ static bool ds1307_want_irq(const struct ds1307 *ds1307,
 	return false;
 }
 
-static int ds1307_chip_configure(struct ds1307 *ds1307)
+static int ds1307_chip_configure(const struct ds1307 *ds1307)
 {
 	int tmp;
 	u8  regs[DS1307_REG_COUNT];
@@ -1422,7 +1422,7 @@ static int ds1307_chip_configure(struct ds1307 *ds1307)
 	return 0;
 }
 
-static int ds1307_chip_sanity_check(struct ds1307 *ds1307)
+static int ds1307_chip_sanity_check(const struct ds1307 *ds1307)
 {
 	int tmp;
 	u8  regs[DS1307_REG_COUNT];
-- 
2.5.5

[PATCH v2 11/17] RTC: ds1307: Convert goto to a loop

[Andrey Smirnov]

Convert goto to a loop and set a hard upper limit on the number of times
driver would try to make RTC work (as opposed to having an infinite
retry loop).

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 drivers/rtc/rtc-ds1307.c | 151 +++++++++++++++++++++++++----------------------
 1 file changed, 82 insertions(+), 69 deletions(-)

diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
index 67907f1..cbbe9f0 100644
--- a/drivers/rtc/rtc-ds1307.c
+++ b/drivers/rtc/rtc-ds1307.c
@@ -1424,87 +1424,100 @@ static int ds1307_chip_configure(const struct ds1307 *ds1307)
 
 static int ds1307_chip_sanity_check(const struct ds1307 *ds1307)
 {
-	int tmp;
+	int tmp, retries;
 	u8  regs[DS1307_REG_COUNT];
 	struct i2c_client *client = ds1307->client;
 
-read_rtc:
-	/* read RTC registers */
-	tmp = ds1307->read_block_data(ds1307->client, ds1307->offset, 8, regs);
-	if (tmp != 8) {
-		dev_dbg(&client->dev, "read error %d\n", tmp);
-		return -EIO;
-	}
-
-	/*
-	 * minimal sanity checking; some chips (like DS1340) don't
-	 * specify the extra bits as must-be-zero, but there are
-	 * still a few values that are clearly out-of-range.
-	 */
-	tmp = regs[DS1307_REG_SECS];
-	switch (ds1307->type) {
-	case ds_1307:
-	case m41t00:
-		/* clock halted?  turn it on, so clock can tick. */
-		if (tmp & DS1307_BIT_CH) {
-			i2c_smbus_write_byte_data(client, DS1307_REG_SECS, 0);
-			dev_warn(&client->dev, "SET TIME!\n");
-			goto read_rtc;
-		}
-		break;
-	case ds_1338:
-		/* clock halted?  turn it on, so clock can tick. */
-		if (tmp & DS1307_BIT_CH)
-			i2c_smbus_write_byte_data(client, DS1307_REG_SECS, 0);
-
-		/* oscillator fault?  clear flag, and warn */
-		if (regs[DS1307_REG_CONTROL] & DS1338_BIT_OSF) {
-			i2c_smbus_write_byte_data(client, DS1307_REG_CONTROL,
-					regs[DS1307_REG_CONTROL]
-					& ~DS1338_BIT_OSF);
-			dev_warn(&client->dev, "SET TIME!\n");
-			goto read_rtc;
-		}
-		break;
-	case ds_1340:
-		/* clock halted?  turn it on, so clock can tick. */
-		if (tmp & DS1340_BIT_nEOSC)
-			i2c_smbus_write_byte_data(client, DS1307_REG_SECS, 0);
-
-		tmp = i2c_smbus_read_byte_data(client, DS1340_REG_FLAG);
-		if (tmp < 0) {
+	for (retries = 0; retries < 5; retries++) {
+		/* read RTC registers */
+		tmp = ds1307->read_block_data(ds1307->client,
+					      ds1307->offset, 8, regs);
+		if (tmp != 8) {
 			dev_dbg(&client->dev, "read error %d\n", tmp);
 			return -EIO;
 		}
 
-		/* oscillator fault?  clear flag, and warn */
-		if (tmp & DS1340_BIT_OSF) {
-			i2c_smbus_write_byte_data(client, DS1340_REG_FLAG, 0);
-			dev_warn(&client->dev, "SET TIME!\n");
-		}
-		break;
-	case mcp794xx:
-		/* make sure that the backup battery is enabled */
-		if (!(regs[DS1307_REG_WDAY] & MCP794XX_BIT_VBATEN)) {
-			i2c_smbus_write_byte_data(client, DS1307_REG_WDAY,
-					regs[DS1307_REG_WDAY]
-					| MCP794XX_BIT_VBATEN);
-		}
+		/*
+		 * minimal sanity checking; some chips (like DS1340)
+		 * don't specify the extra bits as must-be-zero, but
+		 * there are still a few values that are clearly
+		 * out-of-range.
+		 */
+		tmp = regs[DS1307_REG_SECS];
+		switch (ds1307->type) {
+		case ds_1307:
+		case m41t00:
+			/* clock halted?  turn it on, so clock can tick. */
+			if (tmp & DS1307_BIT_CH) {
+				i2c_smbus_write_byte_data(client,
+							  DS1307_REG_SECS, 0);
+				dev_warn(&client->dev, "SET TIME!\n");
+				continue;
+			}
+			break;
+		case ds_1338:
+			/* clock halted?  turn it on, so clock can tick. */
+			if (tmp & DS1307_BIT_CH)
+				i2c_smbus_write_byte_data(client,
+							  DS1307_REG_SECS, 0);
+
+			/* oscillator fault?  clear flag, and warn */
+			if (regs[DS1307_REG_CONTROL] & DS1338_BIT_OSF) {
+				i2c_smbus_write_byte_data(client,
+							  DS1307_REG_CONTROL,
+							  regs[DS1307_REG_CONTROL]
+							  & ~DS1338_BIT_OSF);
+				dev_warn(&client->dev, "SET TIME!\n");
+				continue;
+			}
+			break;
+		case ds_1340:
+			/* clock halted?  turn it on, so clock can tick. */
+			if (tmp & DS1340_BIT_nEOSC)
+				i2c_smbus_write_byte_data(client,
+							  DS1307_REG_SECS, 0);
+
+			tmp = i2c_smbus_read_byte_data(client, DS1340_REG_FLAG);
+			if (tmp < 0) {
+				dev_dbg(&client->dev, "read error %d\n", tmp);
+				return -EIO;
+			}
+
+			/* oscillator fault?  clear flag, and warn */
+			if (tmp & DS1340_BIT_OSF) {
+				i2c_smbus_write_byte_data(client,
+							  DS1340_REG_FLAG, 0);
+				dev_warn(&client->dev, "SET TIME!\n");
+			}
+			return 0;
 
-		/* clock halted?  turn it on, so clock can tick. */
-		if (!(tmp & MCP794XX_BIT_ST)) {
-			i2c_smbus_write_byte_data(client, DS1307_REG_SECS,
-					MCP794XX_BIT_ST);
-			dev_warn(&client->dev, "SET TIME!\n");
-			goto read_rtc;
+		case mcp794xx:
+			/* make sure that the backup battery is enabled */
+			if (!(regs[DS1307_REG_WDAY] & MCP794XX_BIT_VBATEN)) {
+				i2c_smbus_write_byte_data(client,
+							  DS1307_REG_WDAY,
+							  regs[DS1307_REG_WDAY]
+							  | MCP794XX_BIT_VBATEN);
+			}
+
+			/* clock halted?  turn it on, so clock can tick. */
+			if (!(tmp & MCP794XX_BIT_ST)) {
+				i2c_smbus_write_byte_data(client,
+							  DS1307_REG_SECS,
+							  MCP794XX_BIT_ST);
+				dev_warn(&client->dev, "SET TIME!\n");
+				continue;
+			}
+
+			break;
+		default:
+			break;
 		}
 
-		break;
-	default:
-		break;
+		return 0;
 	}
 
-	return 0;
+	return -EIO;
 }
 
 static int ds1307_probe(struct i2c_client *client,
-- 
2.5.5

[PATCH v2 12/17] RTC: ds1307: Redefine RX8025_REG_* to minimize extra code

[Andrey Smirnov]

The only place in the driver where RX8025_REG_* are used they are always
shifted and ORed the same way, so instead of repeating that idiom make
it a part of symbolic constant.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 drivers/rtc/rtc-ds1307.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
index cbbe9f0..2af9c00 100644
--- a/drivers/rtc/rtc-ds1307.c
+++ b/drivers/rtc/rtc-ds1307.c
@@ -103,9 +103,11 @@ enum ds_type {
 
 #define DS13XX_TRICKLE_CHARGER_MAGIC	0xa0
 
-#define RX8025_REG_CTRL1	0x0e
+#define RX8025_REG_CTRL1_	0x0e
+#define RX8025_REG_CTRL1	((RX8025_REG_CTRL1_ << 4) | 0x08)
 #	define RX8025_BIT_2412		0x20
-#define RX8025_REG_CTRL2	0x0f
+#define RX8025_REG_CTRL2_	0x0f
+#define RX8025_REG_CTRL2	((RX8025_REG_CTRL2_ << 4) | 0x08)
 #	define RX8025_BIT_PON		0x10
 #	define RX8025_BIT_VDET		0x40
 #	define RX8025_BIT_XST		0x20
@@ -1353,7 +1355,7 @@ static int ds1307_chip_configure(const struct ds1307 *ds1307)
 	}
 	case rx_8025:
 		tmp = i2c_smbus_read_i2c_block_data(client,
-						    RX8025_REG_CTRL1 << 4 | 0x08,
+						    RX8025_REG_CTRL1,
 						    2, regs);
 		if (tmp != 2) {
 			dev_dbg(&client->dev, "read error %d\n", tmp);
@@ -1364,7 +1366,7 @@ static int ds1307_chip_configure(const struct ds1307 *ds1307)
 		if (!(regs[1] & RX8025_BIT_XST)) {
 			regs[1] |= RX8025_BIT_XST;
 			i2c_smbus_write_byte_data(client,
-						  RX8025_REG_CTRL2 << 4 | 0x08,
+						  RX8025_REG_CTRL2,
 						  regs[1]);
 			dev_warn(&client->dev,
 				 "oscillator stop detected - SET TIME!\n");
@@ -1373,7 +1375,7 @@ static int ds1307_chip_configure(const struct ds1307 *ds1307)
 		if (regs[1] & RX8025_BIT_PON) {
 			regs[1] &= ~RX8025_BIT_PON;
 			i2c_smbus_write_byte_data(client,
-						  RX8025_REG_CTRL2 << 4 | 0x08,
+						  RX8025_REG_CTRL2,
 						  regs[1]);
 			dev_warn(&client->dev, "power-on detected\n");
 		}
@@ -1381,7 +1383,7 @@ static int ds1307_chip_configure(const struct ds1307 *ds1307)
 		if (regs[1] & RX8025_BIT_VDET) {
 			regs[1] &= ~RX8025_BIT_VDET;
 			i2c_smbus_write_byte_data(client,
-						  RX8025_REG_CTRL2 << 4 | 0x08,
+						  RX8025_REG_CTRL2,
 						  regs[1]);
 			dev_warn(&client->dev, "voltage drop detected\n");
 		}
@@ -1392,11 +1394,11 @@ static int ds1307_chip_configure(const struct ds1307 *ds1307)
 
 			/* switch to 24 hour mode */
 			i2c_smbus_write_byte_data(client,
-						  RX8025_REG_CTRL1 << 4 | 0x08,
+						  RX8025_REG_CTRL1,
 						  regs[0] | RX8025_BIT_2412);
 
 			tmp = i2c_smbus_read_i2c_block_data(client,
-							    RX8025_REG_CTRL1 << 4 | 0x08,
+							    RX8025_REG_CTRL1,
 							    2, regs);
 			if (tmp != 2) {
 				dev_dbg(&client->dev, "read error %d\n", tmp);
-- 
2.5.5

[PATCH v2 13/17] RTC: ds1307: Report oscillator problems more intelligently

[Andrey Smirnov]

Report oscillator problems more intelligently, by printing more
information about what cause the issue and not yelling "SET TIME!" at
the user.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 drivers/rtc/rtc-ds1307.c | 28 +++++++++++++++++++++++-----
 1 file changed, 23 insertions(+), 5 deletions(-)

diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
index 2af9c00..2169e5b 100644
--- a/drivers/rtc/rtc-ds1307.c
+++ b/drivers/rtc/rtc-ds1307.c
@@ -1424,6 +1424,19 @@ static int ds1307_chip_configure(const struct ds1307 *ds1307)
 	return 0;
 }
 
+static void ds1307_report_clock_halt(const struct ds1307 *ds1307)
+{
+	dev_warn(&ds1307->client->dev, "RTC's oscillator is turned off. "
+		 "Turning it on. Please set time");
+}
+
+static void ds1307_report_oscillator_fault(const struct ds1307 *ds1307)
+{
+	dev_warn(&ds1307->client->dev, "RTC's reported oscillator fault. "
+		 "Clearing the fault flag and re-reading RTC status. "
+		 "Please set time");
+}
+
 static int ds1307_chip_sanity_check(const struct ds1307 *ds1307)
 {
 	int tmp, retries;
@@ -1453,7 +1466,7 @@ static int ds1307_chip_sanity_check(const struct ds1307 *ds1307)
 			if (tmp & DS1307_BIT_CH) {
 				i2c_smbus_write_byte_data(client,
 							  DS1307_REG_SECS, 0);
-				dev_warn(&client->dev, "SET TIME!\n");
+				ds1307_report_clock_halt(ds1307);
 				continue;
 			}
 			break;
@@ -1469,15 +1482,17 @@ static int ds1307_chip_sanity_check(const struct ds1307 *ds1307)
 							  DS1307_REG_CONTROL,
 							  regs[DS1307_REG_CONTROL]
 							  & ~DS1338_BIT_OSF);
-				dev_warn(&client->dev, "SET TIME!\n");
+				ds1307_report_oscillator_fault(ds1307);
 				continue;
 			}
 			break;
 		case ds_1340:
 			/* clock halted?  turn it on, so clock can tick. */
-			if (tmp & DS1340_BIT_nEOSC)
+			if (tmp & DS1340_BIT_nEOSC) {
 				i2c_smbus_write_byte_data(client,
 							  DS1307_REG_SECS, 0);
+				ds1307_report_clock_halt(ds1307);
+			}
 
 			tmp = i2c_smbus_read_byte_data(client, DS1340_REG_FLAG);
 			if (tmp < 0) {
@@ -1489,7 +1504,7 @@ static int ds1307_chip_sanity_check(const struct ds1307 *ds1307)
 			if (tmp & DS1340_BIT_OSF) {
 				i2c_smbus_write_byte_data(client,
 							  DS1340_REG_FLAG, 0);
-				dev_warn(&client->dev, "SET TIME!\n");
+				ds1307_report_oscillator_fault(ds1307);
 			}
 			return 0;
 
@@ -1500,6 +1515,9 @@ static int ds1307_chip_sanity_check(const struct ds1307 *ds1307)
 							  DS1307_REG_WDAY,
 							  regs[DS1307_REG_WDAY]
 							  | MCP794XX_BIT_VBATEN);
+				dev_warn(&client->dev,
+					 "battery backup was disabled. "
+					 "Re-enabling it\n");
 			}
 
 			/* clock halted?  turn it on, so clock can tick. */
@@ -1507,7 +1525,7 @@ static int ds1307_chip_sanity_check(const struct ds1307 *ds1307)
 				i2c_smbus_write_byte_data(client,
 							  DS1307_REG_SECS,
 							  MCP794XX_BIT_ST);
-				dev_warn(&client->dev, "SET TIME!\n");
+				ds1307_report_clock_halt(ds1307);
 				continue;
 			}
 
-- 
2.5.5

Re: [PATCH v2 13/17] RTC: ds1307: Report oscillator problems more intelligently

[Alexandre Belloni]

On 21/06/2016 at 00:22:46 -0700, Andrey Smirnov wrote :
> Report oscillator problems more intelligently, by printing more
> information about what cause the issue and not yelling "SET TIME!" at
> the user.
> 

Well, the proper way of doing that is to ensure that -EINVAL is returned
when reading the time until it has been set once instead of starting the
oscillator and forgetting about that useful information.

> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> ---
>  drivers/rtc/rtc-ds1307.c | 28 +++++++++++++++++++++++-----
>  1 file changed, 23 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
> index 2af9c00..2169e5b 100644
> --- a/drivers/rtc/rtc-ds1307.c
> +++ b/drivers/rtc/rtc-ds1307.c
> @@ -1424,6 +1424,19 @@ static int ds1307_chip_configure(const struct ds1307 *ds1307)
>  	return 0;
>  }
>  
> +static void ds1307_report_clock_halt(const struct ds1307 *ds1307)
> +{
> +	dev_warn(&ds1307->client->dev, "RTC's oscillator is turned off. "
> +		 "Turning it on. Please set time");
> +}
> +
> +static void ds1307_report_oscillator_fault(const struct ds1307 *ds1307)
> +{
> +	dev_warn(&ds1307->client->dev, "RTC's reported oscillator fault. "
> +		 "Clearing the fault flag and re-reading RTC status. "
> +		 "Please set time");
> +}
> +
>  static int ds1307_chip_sanity_check(const struct ds1307 *ds1307)
>  {
>  	int tmp, retries;
> @@ -1453,7 +1466,7 @@ static int ds1307_chip_sanity_check(const struct ds1307 *ds1307)
>  			if (tmp & DS1307_BIT_CH) {
>  				i2c_smbus_write_byte_data(client,
>  							  DS1307_REG_SECS, 0);
> -				dev_warn(&client->dev, "SET TIME!\n");
> +				ds1307_report_clock_halt(ds1307);
>  				continue;
>  			}
>  			break;
> @@ -1469,15 +1482,17 @@ static int ds1307_chip_sanity_check(const struct ds1307 *ds1307)
>  							  DS1307_REG_CONTROL,
>  							  regs[DS1307_REG_CONTROL]
>  							  & ~DS1338_BIT_OSF);
> -				dev_warn(&client->dev, "SET TIME!\n");
> +				ds1307_report_oscillator_fault(ds1307);
>  				continue;
>  			}
>  			break;
>  		case ds_1340:
>  			/* clock halted?  turn it on, so clock can tick. */
> -			if (tmp & DS1340_BIT_nEOSC)
> +			if (tmp & DS1340_BIT_nEOSC) {
>  				i2c_smbus_write_byte_data(client,
>  							  DS1307_REG_SECS, 0);
> +				ds1307_report_clock_halt(ds1307);
> +			}
>  
>  			tmp = i2c_smbus_read_byte_data(client, DS1340_REG_FLAG);
>  			if (tmp < 0) {
> @@ -1489,7 +1504,7 @@ static int ds1307_chip_sanity_check(const struct ds1307 *ds1307)
>  			if (tmp & DS1340_BIT_OSF) {
>  				i2c_smbus_write_byte_data(client,
>  							  DS1340_REG_FLAG, 0);
> -				dev_warn(&client->dev, "SET TIME!\n");
> +				ds1307_report_oscillator_fault(ds1307);
>  			}
>  			return 0;
>  
> @@ -1500,6 +1515,9 @@ static int ds1307_chip_sanity_check(const struct ds1307 *ds1307)
>  							  DS1307_REG_WDAY,
>  							  regs[DS1307_REG_WDAY]
>  							  | MCP794XX_BIT_VBATEN);
> +				dev_warn(&client->dev,
> +					 "battery backup was disabled. "
> +					 "Re-enabling it\n");
>  			}
>  
>  			/* clock halted?  turn it on, so clock can tick. */
> @@ -1507,7 +1525,7 @@ static int ds1307_chip_sanity_check(const struct ds1307 *ds1307)
>  				i2c_smbus_write_byte_data(client,
>  							  DS1307_REG_SECS,
>  							  MCP794XX_BIT_ST);
> -				dev_warn(&client->dev, "SET TIME!\n");
> +				ds1307_report_clock_halt(ds1307);
>  				continue;
>  			}
>  
> -- 
> 2.5.5
> 

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

Re: [PATCH v2 13/17] RTC: ds1307: Report oscillator problems more intelligently

[Andrey Smirnov]

On Tue, Jun 21, 2016 at 2:22 PM, Alexandre Belloni
<alexandre.belloni@free-electrons.com> wrote:
> On 21/06/2016 at 00:22:46 -0700, Andrey Smirnov wrote :
>> Report oscillator problems more intelligently, by printing more
>> information about what cause the issue and not yelling "SET TIME!" at
>> the user.
>>
>
> Well, the proper way of doing that is to ensure that -EINVAL is returned
> when reading the time until it has been set once instead of starting the
> oscillator and forgetting about that useful information.

Agreed, will change in v3.

[PATCH v2 14/17] RTC: ds1307: Move last bits of sanity checking out of chip_configure

[Andrey Smirnov]

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 drivers/rtc/rtc-ds1307.c | 106 ++++++++++++++++++++++++++---------------------
 1 file changed, 59 insertions(+), 47 deletions(-)

diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
index 2169e5b..45b2adb 100644
--- a/drivers/rtc/rtc-ds1307.c
+++ b/drivers/rtc/rtc-ds1307.c
@@ -1344,66 +1344,23 @@ static int ds1307_chip_configure(const struct ds1307 *ds1307)
 					  DS1337_REG_CONTROL,
 					  regs[0]);
 
-		/* oscillator fault?  clear flag, and warn */
-		if (regs[1] & DS1337_BIT_OSF) {
-			i2c_smbus_write_byte_data(client,
-						  DS1337_REG_STATUS,
-						  regs[1] & ~DS1337_BIT_OSF);
-			dev_warn(&ds1307->client->dev, "SET TIME!\n");
-		}
 		break;
 	}
 	case rx_8025:
-		tmp = i2c_smbus_read_i2c_block_data(client,
-						    RX8025_REG_CTRL1,
-						    2, regs);
-		if (tmp != 2) {
+		tmp = i2c_smbus_read_byte_data(client, RX8025_REG_CTRL1);
+		if (tmp < 0) {
 			dev_dbg(&client->dev, "read error %d\n", tmp);
 			return -EIO;
 		}
 
-		/* oscillator off?  turn it on, so clock can tick. */
-		if (!(regs[1] & RX8025_BIT_XST)) {
-			regs[1] |= RX8025_BIT_XST;
-			i2c_smbus_write_byte_data(client,
-						  RX8025_REG_CTRL2,
-						  regs[1]);
-			dev_warn(&client->dev,
-				 "oscillator stop detected - SET TIME!\n");
-		}
-
-		if (regs[1] & RX8025_BIT_PON) {
-			regs[1] &= ~RX8025_BIT_PON;
-			i2c_smbus_write_byte_data(client,
-						  RX8025_REG_CTRL2,
-						  regs[1]);
-			dev_warn(&client->dev, "power-on detected\n");
-		}
-
-		if (regs[1] & RX8025_BIT_VDET) {
-			regs[1] &= ~RX8025_BIT_VDET;
-			i2c_smbus_write_byte_data(client,
-						  RX8025_REG_CTRL2,
-						  regs[1]);
-			dev_warn(&client->dev, "voltage drop detected\n");
-		}
-
 		/* make sure we are running in 24hour mode */
-		if (!(regs[0] & RX8025_BIT_2412)) {
+		if (!(tmp & RX8025_BIT_2412)) {
 			u8 hour;
 
 			/* switch to 24 hour mode */
 			i2c_smbus_write_byte_data(client,
 						  RX8025_REG_CTRL1,
-						  regs[0] | RX8025_BIT_2412);
-
-			tmp = i2c_smbus_read_i2c_block_data(client,
-							    RX8025_REG_CTRL1,
-							    2, regs);
-			if (tmp != 2) {
-				dev_dbg(&client->dev, "read error %d\n", tmp);
-				return -EIO;
-			}
+						  tmp | RX8025_BIT_2412);
 
 			/* correct hour */
 			hour = bcd2bin(regs[DS1307_REG_HOUR]);
@@ -1486,6 +1443,27 @@ static int ds1307_chip_sanity_check(const struct ds1307 *ds1307)
 				continue;
 			}
 			break;
+
+		case ds_1337:
+		case ds_1339:
+		case ds_3231:
+		case ds_1341:
+			tmp = i2c_smbus_read_byte_data(client,
+						       DS1337_REG_STATUS);
+			if (tmp < 0) {
+				dev_dbg(&client->dev, "read error %d\n", tmp);
+				return -EIO;
+			}
+
+			/* oscillator fault?  clear flag, and warn */
+			if (tmp & DS1337_BIT_OSF) {
+				i2c_smbus_write_byte_data(client,
+							  DS1337_REG_STATUS,
+							  regs[1] & ~DS1337_BIT_OSF);
+				ds1307_report_oscillator_fault(ds1307);
+			}
+			return 0;
+
 		case ds_1340:
 			/* clock halted?  turn it on, so clock can tick. */
 			if (tmp & DS1340_BIT_nEOSC) {
@@ -1530,6 +1508,40 @@ static int ds1307_chip_sanity_check(const struct ds1307 *ds1307)
 			}
 
 			break;
+
+		case rx_8025:
+			tmp = i2c_smbus_read_byte_data(client,
+						       RX8025_REG_CTRL2);
+			if (tmp < 0) {
+				dev_dbg(&client->dev, "read error %d\n", tmp);
+				return -EIO;
+			}
+
+			/* oscillator off?  turn it on, so clock can tick. */
+			if (!(tmp & RX8025_BIT_XST)) {
+				tmp |= RX8025_BIT_XST;
+				i2c_smbus_write_byte_data(client,
+							  RX8025_REG_CTRL2,
+							  tmp);
+				ds1307_report_clock_halt(ds1307);
+			}
+
+			if (tmp & RX8025_BIT_PON) {
+				tmp &= ~RX8025_BIT_PON;
+				i2c_smbus_write_byte_data(client,
+							  RX8025_REG_CTRL2,
+							  tmp);
+				dev_warn(&client->dev, "power-on detected\n");
+			}
+
+			if (tmp & RX8025_BIT_VDET) {
+				tmp &= ~RX8025_BIT_VDET;
+				i2c_smbus_write_byte_data(client,
+							  RX8025_REG_CTRL2,
+							  tmp);
+				dev_warn(&client->dev, "voltage drop detected\n");
+			}
+			break;
 		default:
 			break;
 		}
-- 
2.5.5

[PATCH v2 15/17] RTC: rtctest: Change alarm IRQ support detection

[Andrey Smirnov]

For old style drivers, call a call to ioctl(..., RTC_ALM_SET, ...):

    - char/ds1302.c will always return -EINVAL
    - char/genrtc.c: will always return -EINVAL
    - char/rtc.c will succeed regardless if IRQs are supported or not
    - char/efirtc.c will always return -EINVAL
    - input/misc/hp_sdc_rtc.c ... that ioctl code is a good lesson about
      ifdefing code out and punting implementation ... and it will
      always return -EINVAL

For new style rtc drivers, a call to ioctl(..., RTC_ALM_SET, ...) never
results in a call to __rtc_set_alarm, since struct rtc_wkalarm passed to
rtc_set_alarm has 'enabled' field set to 0. This means that
rtc->ops->set_alarm driver hook is never called in that ioctl. Since no
driver code interaction happens as a part of that call, using its
results to ascertain properties of the driver is not going to work. To
remedy this - use the result of RTC_AIE_ON to make the judgement.

This patch also changes ENOTTY to EINVAL as a error code value that
would tell us that IRQs are not supported. There are three reason for
this:

 - As mentioned above old style driver never retunr ENOTTY for this
   ioctl

 - In it's code __rtc_set_alarm() returns -EINVAL if rtc->ops->set_alarm
   method is not provided by the driver, so one reason for change is to
   be consistent with that code path.

 - A call to ioctl(..., RTC_UIE_ON, ...) will result in a call to
   rtc_update_irq_enable() and then __rtc_set_alarm(), which, if IRQs
   are not supported by the driver, will result in a non-zero error
   code. Returning ENOTTY in that case would:

   	 a) Not be consistent with other codepaths of
   	 rtc_update_irq_enable, for example the check of
   	 rtc->uie_unsupported

	 b) Would break update IRQ emulation code since that codpath
	 expects EINVAL

	 c) Would break test's logic for feature support detection in
	 the case of RTC_UIE_ON ioctl

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 tools/testing/selftests/timers/rtctest.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/timers/rtctest.c b/tools/testing/selftests/timers/rtctest.c
index 624bce5..0cb4628 100644
--- a/tools/testing/selftests/timers/rtctest.c
+++ b/tools/testing/selftests/timers/rtctest.c
@@ -144,11 +144,12 @@ test_READ:
 
 	retval = ioctl(fd, RTC_ALM_SET, &rtc_tm);
 	if (retval == -1) {
-		if (errno == ENOTTY) {
+		if (errno == EINVAL) {
 			fprintf(stderr,
 				"\n...Alarm IRQs not supported.\n");
 			goto test_PIE;
 		}
+
 		perror("RTC_ALM_SET ioctl");
 		exit(errno);
 	}
@@ -166,6 +167,12 @@ test_READ:
 	/* Enable alarm interrupts */
 	retval = ioctl(fd, RTC_AIE_ON, 0);
 	if (retval == -1) {
+		if (errno == EINVAL) {
+			fprintf(stderr,
+				"\n...Alarm IRQs not supported.\n");
+			goto test_PIE;
+		}
+
 		perror("RTC_AIE_ON ioctl");
 		exit(errno);
 	}
-- 
2.5.5

[PATCH v2 16/17] RTC: rtctest: Change no IRQ detection for RTC_IRQP_READ

[Andrey Smirnov]

A call to ioctl(..., RTC_IRQP_READ, ...) should never result in
ENOTTY. All new style RTC drivers implement it and all of the old style
drivers return EINVAL when they don't support periodic IRQs.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 tools/testing/selftests/timers/rtctest.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/timers/rtctest.c b/tools/testing/selftests/timers/rtctest.c
index 0cb4628..97beadf 100644
--- a/tools/testing/selftests/timers/rtctest.c
+++ b/tools/testing/selftests/timers/rtctest.c
@@ -200,7 +200,7 @@ test_PIE:
 	retval = ioctl(fd, RTC_IRQP_READ, &tmp);
 	if (retval == -1) {
 		/* not all RTCs support periodic IRQs */
-		if (errno == ENOTTY) {
+		if (errno == EINVAL) {
 			fprintf(stderr, "\nNo periodic IRQ support\n");
 			goto done;
 		}
-- 
2.5.5

[PATCH v2 17/17] RTC: rtctest: Change no IRQ detection for RTC_IRQP_SET

[Andrey Smirnov]

A call to ioctl(..., RTC_IRQP_SET, ...) should never result in
ENOTTY. All new style RTC drivers implement it and all of the old style
drivers return EINVAL when they don't support periodic IRQs.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 tools/testing/selftests/timers/rtctest.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/timers/rtctest.c b/tools/testing/selftests/timers/rtctest.c
index 97beadf..4230d30 100644
--- a/tools/testing/selftests/timers/rtctest.c
+++ b/tools/testing/selftests/timers/rtctest.c
@@ -218,7 +218,7 @@ test_PIE:
 		retval = ioctl(fd, RTC_IRQP_SET, tmp);
 		if (retval == -1) {
 			/* not all RTCs can change their periodic IRQ rate */
-			if (errno == ENOTTY) {
+			if (errno == EINVAL) {
 				fprintf(stderr,
 					"\n...Periodic IRQ rate is fixed\n");
 				goto done;
-- 
2.5.5

Re: [PATCH v2 02/17] RTC: ds1307: Disable square wave and timers as default

[Alexandre Belloni]

On 21/06/2016 at 00:22:35 -0700, Andrey Smirnov wrote :
> Disable square wave and timers as default for DS1337/39/41 and
> DS3231. The rationale being that configuring a chip this way puts it
> into a known state with lower power consumption. While it is not very
> likely it is still possible that the code controlling RTCs that ran
> before this driver configured it to produce square wave and left it in
> such a state.
> 

The main issue being that if it has been configured that way (from the
bootloader for example). It is probably necessary for the board. There
may be users of that clock and this patch definitively breaks them.
The proper way of doing that is to add CCF support in the driver. See:
http://patchwork.ozlabs.org/patch/576201/

> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> ---
>  drivers/rtc/rtc-ds1307.c | 12 +++++-------
>  1 file changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
> index 7e65e2e..c618c22 100644
> --- a/drivers/rtc/rtc-ds1307.c
> +++ b/drivers/rtc/rtc-ds1307.c
> @@ -1320,19 +1320,17 @@ static int ds1307_probe(struct i2c_client *client,
>  			ds1307->regs[0] &= ~DS1337_BIT_nEOSC;
>  
>  		/*
> -		 * Using IRQ or defined as wakeup-source?
>  		 * Disable the square wave and both alarms.
>  		 * For some variants, be sure alarms can trigger when we're
>  		 * running on Vbackup (BBSQI/BBSQW)
>  		 */
> -		if (chip->alarm && (ds1307->client->irq > 0 ||
> -						ds1307_can_wakeup_device)) {
> -			ds1307->regs[0] |= DS1337_BIT_INTCN
> -					| bbsqi_bitpos[ds1307->type];
> -			ds1307->regs[0] &= ~(DS1337_BIT_A2IE | DS1337_BIT_A1IE);
> +		ds1307->regs[0] |= DS1337_BIT_INTCN
> +			| bbsqi_bitpos[ds1307->type];
> +		ds1307->regs[0] &= ~(DS1337_BIT_A2IE | DS1337_BIT_A1IE);
>  
> +		if (chip->alarm && (ds1307->client->irq > 0 ||
> +				    ds1307_can_wakeup_device))
>  			want_irq = true;
> -		}
>  
>  		i2c_smbus_write_byte_data(client, DS1337_REG_CONTROL,
>  							ds1307->regs[0]);
> -- 
> 2.5.5
> 

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

Re: [PATCH v2 02/17] RTC: ds1307: Disable square wave and timers as default

[Andrey Smirnov]

On Tue, Jun 21, 2016 at 2:17 PM, Alexandre Belloni
<alexandre.belloni@free-electrons.com> wrote:
> On 21/06/2016 at 00:22:35 -0700, Andrey Smirnov wrote :
>> Disable square wave and timers as default for DS1337/39/41 and
>> DS3231. The rationale being that configuring a chip this way puts it
>> into a known state with lower power consumption. While it is not very
>> likely it is still possible that the code controlling RTCs that ran
>> before this driver configured it to produce square wave and left it in
>> such a state.
>>
>
> The main issue being that if it has been configured that way (from the
> bootloader for example). It is probably necessary for the board. There
> may be users of that clock and this patch definitively breaks them.
> The proper way of doing that is to add CCF support in the driver. See:
> http://patchwork.ozlabs.org/patch/576201/

Implementing CCF support would still break users who rely on this
particular behavior and they would still have to spend effort
modifying their boards' device tree blob, so doing it the way you
propose wouldn't really save those users from pain, it would just give
them a way out.

Unfortunately, I don't have a bandwidth to develop and test a feature
that I don't have a use-case for, so I'll drop this patch from v3.

Andrey