[PATCH v2 0/2] rtc: add Abracon ABx80x

[PATCH v2 0/2] rtc: add Abracon ABx80x

[Alexandre Belloni]

Hi,

As discussed, I reworked the abracon abx80x driver, merging Philippe's and mine.

I have added the Device Tree properties to enable the trickle charger and the
driver will detect the partnumber before doing that so that it won't try to
enable the charger when it is not available.

Andrew, when taking this series, you can drop:
rtc-add-rtc-abx805-a-driver-for-the-abracon-ab-1805-i2c-rtc.patch
rtc-add-abracon-abx80x-driver.patch
rtc-add-abracon-abx80x-driver-fix.patch

Changes in v2:
 - corrected MODULE_LICENSE
 - enforce ABX8XX_CTRL_WRITE at probe time instead of at write time
 - changed the range of supported dates to be 2000-2099
 - reworked the write function to be clearer

Alexandre Belloni (1):
  Documentation: bindings: add abracon,abx80x

Philippe De Muyter (1):
  rtc: add rtc-abx80x, a driver for the Abracon AB x80x i2c rtc

 .../devicetree/bindings/rtc/abracon,abx80x.txt     |  32 +++
 drivers/rtc/Kconfig                                |  10 +
 drivers/rtc/Makefile                               |   1 +
 drivers/rtc/rtc-abx80x.c                           | 307 +++++++++++++++++++++
 4 files changed, 350 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/rtc/abracon,abx80x.txt
 create mode 100644 drivers/rtc/rtc-abx80x.c

-- 
2.1.0

[PATCH v2 1/2] Documentation: bindings: add abracon,abx80x

[Alexandre Belloni]

Document the bindings for abracon,abx80x and related compatibles.

Cc: devicetree-discuss@lists.ozlabs.org
Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
---
 .../devicetree/bindings/rtc/abracon,abx80x.txt     | 32 ++++++++++++++++++++++
 1 file changed, 32 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/rtc/abracon,abx80x.txt

diff --git a/Documentation/devicetree/bindings/rtc/abracon,abx80x.txt b/Documentation/devicetree/bindings/rtc/abracon,abx80x.txt
new file mode 100644
index 000000000000..6f0d3b2cdd89
--- /dev/null
+++ b/Documentation/devicetree/bindings/rtc/abracon,abx80x.txt
@@ -0,0 +1,32 @@
+Abracon ABX80X I2C ultra low power RTC/Alarm chip
+
+The Abracon ABX80X family consist of the ab0801, ab0803, ab0804, ab0805, ab1801,
+ab1803, ab1804 and ab1805. The ab0805 is the superset of ab080x and the ab1805
+is the superset of ab180x.
+
+Required properties:
+
+ - "compatible": should one of:
+        "abracon,abx80x"
+        "abracon,ab0801"
+        "abracon,ab0803"
+        "abracon,ab0804"
+        "abracon,ab0805"
+        "abracon,ab1801"
+        "abracon,ab1803"
+        "abracon,ab1804"
+        "abracon,ab1805"
+	Using "abracon,abx80x" will enable chip autodetection.
+ - "reg": I2C bus address of the device
+
+Optional properties:
+
+The abx804 and abx805 have a trickle charger that is able to charge the
+connected battery or supercap. Both the following properties have to be defined
+and valid to enable charging:
+
+ - "abracon,tc-diode": should be "standard" (0.6V) or "schottky" (0.3V)
+ - "abracon,tc-resistor": should be <0>, <3>, <6> or <11>. 0 disables the output
+                          resistor, the other values are in ohm.
+
+
-- 
2.1.0

[PATCH v2 2/2] rtc: add rtc-abx80x, a driver for the Abracon AB x80x i2c rtc

[Alexandre Belloni]

From: Philippe De Muyter <phdm@macqel.be>

This is a basic driver for the ultra-low-power Abracon AB x80x series of RTC
chips. It supports in particular, the supersets AB0805 and AB1805.
It allows reading and writing the time, and enables the supercapacitor/
battery charger.

[arnd@arndb.de: abx805 depends on i2c]
Signed-off-by: Philippe De Muyter <phdm@macqel.be>
Cc: Alessandro Zummo <a.zummo@towertech.it>
Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---
 drivers/rtc/Kconfig      |  10 ++
 drivers/rtc/Makefile     |   1 +
 drivers/rtc/rtc-abx80x.c | 307 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 318 insertions(+)
 create mode 100644 drivers/rtc/rtc-abx80x.c

diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index b5b5c3d485d6..89507c1858ec 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -164,6 +164,16 @@ config RTC_DRV_ABB5ZES3
 	  This driver can also be built as a module. If so, the module
 	  will be called rtc-ab-b5ze-s3.
 
+config RTC_DRV_ABX80X
+	tristate "Abracon ABx80x"
+	help
+	  If you say yes here you get support for Abracon AB080X and AB180X
+	  families of ultra-low-power  battery- and capacitor-backed real-time
+	  clock chips.
+
+	  This driver can also be built as a module. If so, the module
+	  will be called rtc-abx80x.
+
 config RTC_DRV_AS3722
 	tristate "ams AS3722 RTC driver"
 	depends on MFD_AS3722
diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
index 69c87062b098..7b20b0462cb6 100644
--- a/drivers/rtc/Makefile
+++ b/drivers/rtc/Makefile
@@ -25,6 +25,7 @@ obj-$(CONFIG_RTC_DRV_88PM80X)	+= rtc-88pm80x.o
 obj-$(CONFIG_RTC_DRV_AB3100)	+= rtc-ab3100.o
 obj-$(CONFIG_RTC_DRV_AB8500)	+= rtc-ab8500.o
 obj-$(CONFIG_RTC_DRV_ABB5ZES3)	+= rtc-ab-b5ze-s3.o
+obj-$(CONFIG_RTC_DRV_ABX80X)	+= rtc-abx80x.o
 obj-$(CONFIG_RTC_DRV_ARMADA38X)	+= rtc-armada38x.o
 obj-$(CONFIG_RTC_DRV_AS3722)	+= rtc-as3722.o
 obj-$(CONFIG_RTC_DRV_AT32AP700X)+= rtc-at32ap700x.o
diff --git a/drivers/rtc/rtc-abx80x.c b/drivers/rtc/rtc-abx80x.c
new file mode 100644
index 000000000000..3d627f16cbac
--- /dev/null
+++ b/drivers/rtc/rtc-abx80x.c
@@ -0,0 +1,307 @@
+/*
+ * A driver for the I2C members of the Abracon AB x8xx RTC family,
+ * and compatible: AB 1805 and AB 0805
+ *
+ * Copyright 2014-2015 Macq S.A.
+ *
+ * Author: Philippe De Muyter <phdm@macqel.be>
+ * Author: Alexandre Belloni <alexandre.belloni@free-electrons.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/bcd.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/rtc.h>
+
+#define ABX8XX_REG_HTH		0x00
+#define ABX8XX_REG_SC		0x01
+#define ABX8XX_REG_MN		0x02
+#define ABX8XX_REG_HR		0x03
+#define ABX8XX_REG_DA		0x04
+#define ABX8XX_REG_MO		0x05
+#define ABX8XX_REG_YR		0x06
+#define ABX8XX_REG_WD		0x07
+
+#define ABX8XX_REG_CTRL1	0x10
+#define ABX8XX_CTRL_WRITE	BIT(1)
+#define ABX8XX_CTRL_12_24	BIT(6)
+
+#define ABX8XX_REG_CFG_KEY	0x1f
+#define ABX8XX_CFG_KEY_MISC	0x9d
+
+#define ABX8XX_REG_ID0		0x28
+
+#define ABX8XX_REG_TRICKLE	0x20
+#define ABX8XX_TRICKLE_CHARGE_ENABLE	0xa0
+#define ABX8XX_TRICKLE_STANDARD_DIODE	0x8
+#define ABX8XX_TRICKLE_SCHOTTKY_DIODE	0x4
+
+static u8 trickle_resistors[] = {0, 3, 6, 11};
+
+enum abx80x_chip {AB0801, AB0803, AB0804, AB0805,
+	AB1801, AB1803, AB1804, AB1805, ABX80X};
+
+struct abx80x_cap {
+	u16 pn;
+	bool has_tc;
+};
+
+static struct abx80x_cap abx80x_caps[] = {
+	[AB0801] = {.pn = 0x0801},
+	[AB0803] = {.pn = 0x0803},
+	[AB0804] = {.pn = 0x0804, .has_tc = true},
+	[AB0805] = {.pn = 0x0805, .has_tc = true},
+	[AB1801] = {.pn = 0x1801},
+	[AB1803] = {.pn = 0x1803},
+	[AB1804] = {.pn = 0x1804, .has_tc = true},
+	[AB1805] = {.pn = 0x1805, .has_tc = true},
+	[ABX80X] = {.pn = 0}
+};
+
+static struct i2c_driver abx80x_driver;
+
+static int abx80x_enable_trickle_charger(struct i2c_client *client,
+					 u8 trickle_cfg)
+{
+	int err;
+
+	/*
+	 * Write the configuration key register to enable access to the Trickle
+	 * register
+	 */
+	err = i2c_smbus_write_byte_data(client, ABX8XX_REG_CFG_KEY,
+					ABX8XX_CFG_KEY_MISC);
+	if (err < 0) {
+		dev_err(&client->dev, "Unable to write configuration key\n");
+		return -EIO;
+	}
+
+	err = i2c_smbus_write_byte_data(client, ABX8XX_REG_TRICKLE,
+					ABX8XX_TRICKLE_CHARGE_ENABLE |
+					trickle_cfg);
+	if (err < 0) {
+		dev_err(&client->dev, "Unable to write trickle register\n");
+		return -EIO;
+	}
+
+	return 0;
+}
+
+static int abx80x_rtc_read_time(struct device *dev, struct rtc_time *tm)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	unsigned char date[8];
+	int err;
+
+	err = i2c_smbus_read_i2c_block_data(client, ABX8XX_REG_HTH,
+					    sizeof(date), date);
+	if (err < 0) {
+		dev_err(&client->dev, "Unable to read date\n");
+		return -EIO;
+	}
+
+	tm->tm_sec = bcd2bin(date[ABX8XX_REG_SC] & 0x7F);
+	tm->tm_min = bcd2bin(date[ABX8XX_REG_MN] & 0x7F);
+	tm->tm_hour = bcd2bin(date[ABX8XX_REG_HR] & 0x3F);
+	tm->tm_wday = date[ABX8XX_REG_WD] & 0x7;
+	tm->tm_mday = bcd2bin(date[ABX8XX_REG_DA] & 0x3F);
+	tm->tm_mon = bcd2bin(date[ABX8XX_REG_MO] & 0x1F) - 1;
+	tm->tm_year = bcd2bin(date[ABX8XX_REG_YR]) + 100;
+
+	err = rtc_valid_tm(tm);
+	if (err < 0)
+		dev_err(&client->dev, "retrieved date/time is not valid.\n");
+
+	return err;
+}
+
+static int abx80x_rtc_set_time(struct device *dev, struct rtc_time *tm)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	unsigned char buf[8];
+	int err;
+
+	if (tm->tm_year < 100)
+		return -EINVAL;
+
+	buf[ABX8XX_REG_HTH] = 0;
+	buf[ABX8XX_REG_SC] = bin2bcd(tm->tm_sec);
+	buf[ABX8XX_REG_MN] = bin2bcd(tm->tm_min);
+	buf[ABX8XX_REG_HR] = bin2bcd(tm->tm_hour);
+	buf[ABX8XX_REG_DA] = bin2bcd(tm->tm_mday);
+	buf[ABX8XX_REG_MO] = bin2bcd(tm->tm_mon + 1);
+	buf[ABX8XX_REG_YR] = bin2bcd(tm->tm_year - 100);
+	buf[ABX8XX_REG_WD] = tm->tm_wday;
+
+	err = i2c_smbus_write_i2c_block_data(client, ABX8XX_REG_HTH,
+					     sizeof(buf), buf);
+	if (err < 0) {
+		dev_err(&client->dev, "Unable to write to date registers\n");
+		return -EIO;
+	}
+
+	return 0;
+}
+
+static const struct rtc_class_ops abx80x_rtc_ops = {
+	.read_time	= abx80x_rtc_read_time,
+	.set_time	= abx80x_rtc_set_time,
+};
+
+static int abx80x_dt_trickle_cfg(struct device_node *np)
+{
+	const char *diode;
+	int trickle_cfg = 0;
+	int i, ret;
+	u32 tmp;
+
+	ret = of_property_read_string(np, "abracon,tc-diode", &diode);
+	if (ret)
+		return ret;
+
+	if (!strcmp(diode, "standard"))
+		trickle_cfg |= ABX8XX_TRICKLE_STANDARD_DIODE;
+	else if (!strcmp(diode, "schottky"))
+		trickle_cfg |= ABX8XX_TRICKLE_SCHOTTKY_DIODE;
+	else
+		return -EINVAL;
+
+	ret = of_property_read_u32(np, "abracon,tc-resistor", &tmp);
+	if (ret)
+		return ret;
+
+	for (i = 0; i < sizeof(trickle_resistors); i++)
+		if (trickle_resistors[i] == tmp)
+			break;
+
+	if (i == sizeof(trickle_resistors))
+		return -EINVAL;
+
+	return (trickle_cfg | i);
+}
+
+static int abx80x_probe(struct i2c_client *client,
+			const struct i2c_device_id *id)
+{
+	struct device_node *np = client->dev.of_node;
+	struct rtc_device *rtc;
+	int i, data, err, trickle_cfg = -EINVAL;
+	char buf[7];
+	unsigned int part = id->driver_data;
+	unsigned int partnumber;
+	unsigned int majrev, minrev;
+	unsigned int lot;
+	unsigned int wafer;
+	unsigned int uid;
+
+	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
+		return -ENODEV;
+
+	err = i2c_smbus_read_i2c_block_data(client, ABX8XX_REG_ID0,
+					    sizeof(buf), buf);
+	if (err < 0) {
+		dev_err(&client->dev, "Unable to read partnumber\n");
+		return -EIO;
+	}
+
+	partnumber = (buf[0] << 8) | buf[1];
+	majrev = buf[2] >> 3;
+	minrev = buf[2] & 0x7;
+	lot = ((buf[4] & 0x80) << 2) | ((buf[6] & 0x80) << 1) | buf[3];
+	uid = ((buf[4] & 0x7f) << 8) | buf[5];
+	wafer = (buf[6] & 0x7c) >> 2;
+	dev_info(&client->dev, "model %04x, revision %u.%u, lot %x, wafer %x, uid %x\n",
+		 partnumber, majrev, minrev, lot, wafer, uid);
+
+	data = i2c_smbus_read_byte_data(client, ABX8XX_REG_CTRL1);
+	if (data < 0) {
+		dev_err(&client->dev, "Unable to read control register\n");
+		return -EIO;
+	}
+
+	err = i2c_smbus_write_byte_data(client, ABX8XX_REG_CTRL1,
+					((data & ~ABX8XX_CTRL_12_24) |
+					 ABX8XX_CTRL_WRITE));
+	if (err < 0) {
+		dev_err(&client->dev, "Unable to write control register\n");
+		return -EIO;
+	}
+
+	/* part autodetection */
+	if (part == ABX80X) {
+		for (i = 0; abx80x_caps[i].pn; i++)
+			if (partnumber == abx80x_caps[i].pn)
+				break;
+		if (abx80x_caps[i].pn == 0) {
+			dev_err(&client->dev, "Unknown part: %04x\n",
+				partnumber);
+			return -EINVAL;
+		}
+		part = i;
+	}
+
+	if (partnumber != abx80x_caps[part].pn) {
+		dev_err(&client->dev, "partnumber mismatch %04x != %04x\n",
+			partnumber, abx80x_caps[part].pn);
+		return -EINVAL;
+	}
+
+	if (np && abx80x_caps[part].has_tc)
+		trickle_cfg = abx80x_dt_trickle_cfg(np);
+
+	if (trickle_cfg > 0) {
+		dev_info(&client->dev, "Enabling trickle charger: %02x\n",
+			 trickle_cfg);
+		abx80x_enable_trickle_charger(client, trickle_cfg);
+	}
+
+	rtc = devm_rtc_device_register(&client->dev, abx80x_driver.driver.name,
+				       &abx80x_rtc_ops, THIS_MODULE);
+
+	if (IS_ERR(rtc))
+		return PTR_ERR(rtc);
+
+	i2c_set_clientdata(client, rtc);
+
+	return 0;
+}
+
+static int abx80x_remove(struct i2c_client *client)
+{
+	return 0;
+}
+
+static const struct i2c_device_id abx80x_id[] = {
+	{ "abx80x", ABX80X },
+	{ "ab0801", AB0801 },
+	{ "ab0803", AB0803 },
+	{ "ab0804", AB0804 },
+	{ "ab0805", AB0805 },
+	{ "ab1801", AB1801 },
+	{ "ab1803", AB1803 },
+	{ "ab1804", AB1804 },
+	{ "ab1805", AB1805 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, abx80x_id);
+
+static struct i2c_driver abx80x_driver = {
+	.driver		= {
+		.name	= "rtc-abx80x",
+	},
+	.probe		= abx80x_probe,
+	.remove		= abx80x_remove,
+	.id_table	= abx80x_id,
+};
+
+module_i2c_driver(abx80x_driver);
+
+MODULE_AUTHOR("Philippe De Muyter <phdm@macqel.be>");
+MODULE_AUTHOR("Alexandre Belloni <alexandre.belloni@free-electrons.com>");
+MODULE_DESCRIPTION("Abracon ABX80X RTC driver");
+MODULE_LICENSE("GPL v2");
-- 
2.1.0

Re: [PATCH v2 1/2] Documentation: bindings: add abracon,abx80x

[Philippe De Muyter]

Hi Alexandre,

when I look into Documentation/devicetree/bindings/rtc, it seems that
all rtc but a few (mainly old ones) have the "-rtc" suffix in the
"compatible" property.

Would it not be better to keep it here also, like what I had written ?

Best regards

Philippe


On Wed, Apr 01, 2015 at 04:52:22AM +0200, Alexandre Belloni wrote:
> Document the bindings for abracon,abx80x and related compatibles.
> 
> Cc: devicetree-discuss@lists.ozlabs.org
> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> ---
>  .../devicetree/bindings/rtc/abracon,abx80x.txt     | 32 ++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/rtc/abracon,abx80x.txt
> 
> diff --git a/Documentation/devicetree/bindings/rtc/abracon,abx80x.txt b/Documentation/devicetree/bindings/rtc/abracon,abx80x.txt
> new file mode 100644
> index 000000000000..6f0d3b2cdd89
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/rtc/abracon,abx80x.txt
> @@ -0,0 +1,32 @@
> +Abracon ABX80X I2C ultra low power RTC/Alarm chip
> +
> +The Abracon ABX80X family consist of the ab0801, ab0803, ab0804, ab0805, ab1801,
> +ab1803, ab1804 and ab1805. The ab0805 is the superset of ab080x and the ab1805
> +is the superset of ab180x.
> +
> +Required properties:
> +
> + - "compatible": should one of:
> +        "abracon,abx80x"
> +        "abracon,ab0801"
> +        "abracon,ab0803"
> +        "abracon,ab0804"
> +        "abracon,ab0805"
> +        "abracon,ab1801"
> +        "abracon,ab1803"
> +        "abracon,ab1804"
> +        "abracon,ab1805"
> +	Using "abracon,abx80x" will enable chip autodetection.
> + - "reg": I2C bus address of the device
> +
> +Optional properties:
> +
> +The abx804 and abx805 have a trickle charger that is able to charge the
> +connected battery or supercap. Both the following properties have to be defined
> +and valid to enable charging:
> +
> + - "abracon,tc-diode": should be "standard" (0.6V) or "schottky" (0.3V)
> + - "abracon,tc-resistor": should be <0>, <3>, <6> or <11>. 0 disables the output
> +                          resistor, the other values are in ohm.
> +
> +
> -- 
> 2.1.0

-- 
Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles

Re: [PATCH v2 2/2] rtc: add rtc-abx80x, a driver for the Abracon AB x80x i2c rtc

[Philippe De Muyter]

Hi Alexandre,

On Wed, Apr 01, 2015 at 04:52:23AM +0200, Alexandre Belloni wrote:
> From: Philippe De Muyter <phdm@macqel.be>
> 
> This is a basic driver for the ultra-low-power Abracon AB x80x series of RTC
> chips. It supports in particular, the supersets AB0805 and AB1805.
> It allows reading and writing the time, and enables the supercapacitor/
> battery charger.
> 
> [arnd@arndb.de: abx805 depends on i2c]
> Signed-off-by: Philippe De Muyter <phdm@macqel.be>
> Cc: Alessandro Zummo <a.zummo@towertech.it>
> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> ---
...
> +static int abx80x_rtc_read_time(struct device *dev, struct rtc_time *tm)
> +{
> +	struct i2c_client *client = to_i2c_client(dev);
> +	unsigned char date[8];
> +	int err;
> +
> +	err = i2c_smbus_read_i2c_block_data(client, ABX8XX_REG_HTH,
> +					    sizeof(date), date);
> +	if (err < 0) {
> +		dev_err(&client->dev, "Unable to read date\n");
> +		return -EIO;
> +	}
> +
> +	tm->tm_sec = bcd2bin(date[ABX8XX_REG_SC] & 0x7F);
> +	tm->tm_min = bcd2bin(date[ABX8XX_REG_MN] & 0x7F);
> +	tm->tm_hour = bcd2bin(date[ABX8XX_REG_HR] & 0x3F);
> +	tm->tm_wday = date[ABX8XX_REG_WD] & 0x7;
> +	tm->tm_mday = bcd2bin(date[ABX8XX_REG_DA] & 0x3F);
> +	tm->tm_mon = bcd2bin(date[ABX8XX_REG_MO] & 0x1F) - 1;
> +	tm->tm_year = bcd2bin(date[ABX8XX_REG_YR]) + 100;

Just a minor nit: I would rather not name the buffer 'date', but 'buf' like
in abx80x_rtc_set_time.

> +
> +	err = rtc_valid_tm(tm);
> +	if (err < 0)
> +		dev_err(&client->dev, "retrieved date/time is not valid.\n");
> +
> +	return err;
> +}
> +
> +static int abx80x_rtc_set_time(struct device *dev, struct rtc_time *tm)
> +{
> +	struct i2c_client *client = to_i2c_client(dev);
> +	unsigned char buf[8];
> +	int err;
> +
...
> +static const struct i2c_device_id abx80x_id[] = {
> +	{ "abx80x", ABX80X },

should become
	{ "abx80x-rtc", ABX80X },
> +	{ "ab0801", AB0801 },
> +	{ "ab0803", AB0803 },
> +	{ "ab0804", AB0804 },
> +	{ "ab0805", AB0805 },
> +	{ "ab1801", AB1801 },
> +	{ "ab1803", AB1803 },
> +	{ "ab1804", AB1804 },
> +	{ "ab1805", AB1805 },
> +	{ }
and likewise

Best regards

Philippe

-- 
Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles

Re: [rtc-linux] Re: [PATCH v2 1/2] Documentation: bindings: add abracon,abx80x

[Alexandre Belloni]

Hi,

On 09/04/2015 at 15:25:23 +0200, Philippe De Muyter wrote :
> Hi Alexandre,
> 
> when I look into Documentation/devicetree/bindings/rtc, it seems that
> all rtc but a few (mainly old ones) have the "-rtc" suffix in the
> "compatible" property.
> 
> Would it not be better to keep it here also, like what I had written ?
> 

No, they have that suffix because they are RTC IPs part of a SoC so the
chosen compatible is <soc>-rtc. In the case of the abracon RTC, it would
be redundant.

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

Re: [rtc-linux] Re: [PATCH v2 2/2] rtc: add rtc-abx80x, a driver for the Abracon AB x80x i2c rtc

[Alexandre Belloni]

On 09/04/2015 at 15:31:22 +0200, Philippe De Muyter wrote :
> > +static int abx80x_rtc_read_time(struct device *dev, struct rtc_time *tm)
> > +{
> > +	struct i2c_client *client = to_i2c_client(dev);
> > +	unsigned char date[8];
> > +	int err;
> > +
> > +	err = i2c_smbus_read_i2c_block_data(client, ABX8XX_REG_HTH,
> > +					    sizeof(date), date);
> > +	if (err < 0) {
> > +		dev_err(&client->dev, "Unable to read date\n");
> > +		return -EIO;
> > +	}
> > +
> > +	tm->tm_sec = bcd2bin(date[ABX8XX_REG_SC] & 0x7F);
> > +	tm->tm_min = bcd2bin(date[ABX8XX_REG_MN] & 0x7F);
> > +	tm->tm_hour = bcd2bin(date[ABX8XX_REG_HR] & 0x3F);
> > +	tm->tm_wday = date[ABX8XX_REG_WD] & 0x7;
> > +	tm->tm_mday = bcd2bin(date[ABX8XX_REG_DA] & 0x3F);
> > +	tm->tm_mon = bcd2bin(date[ABX8XX_REG_MO] & 0x1F) - 1;
> > +	tm->tm_year = bcd2bin(date[ABX8XX_REG_YR]) + 100;
> 
> Just a minor nit: I would rather not name the buffer 'date', but 'buf' like
> in abx80x_rtc_set_time.
> 

Ok but I won't have time to respin before the merge window, can you send
a v3 with that change?

> > +
> > +	err = rtc_valid_tm(tm);
> > +	if (err < 0)
> > +		dev_err(&client->dev, "retrieved date/time is not valid.\n");
> > +
> > +	return err;
> > +}
> > +
> > +static int abx80x_rtc_set_time(struct device *dev, struct rtc_time *tm)
> > +{
> > +	struct i2c_client *client = to_i2c_client(dev);
> > +	unsigned char buf[8];
> > +	int err;
> > +
> ...
> > +static const struct i2c_device_id abx80x_id[] = {
> > +	{ "abx80x", ABX80X },
> 
> should become
> 	{ "abx80x-rtc", ABX80X },
> > +	{ "ab0801", AB0801 },
> > +	{ "ab0803", AB0803 },
> > +	{ "ab0804", AB0804 },
> > +	{ "ab0805", AB0805 },
> > +	{ "ab1801", AB1801 },
> > +	{ "ab1803", AB1803 },
> > +	{ "ab1804", AB1804 },
> > +	{ "ab1805", AB1805 },
> > +	{ }
> and likewise

See my other answer, it would be redundant.

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