[PATCH v2] rtc: add Abracon ABx80x driver

[PATCH v2] rtc: add Abracon ABx80x driver

[Alexandre Belloni]

Add support for the i2c RTC from Abracon.

Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
---
Changes in v2:
 - corrected style according to checkpatch --strict
 - renamed functions to abx80x_*
 - reordered makefile and kconfig additions

 drivers/rtc/Kconfig      |   9 +++
 drivers/rtc/Makefile     |   1 +
 drivers/rtc/rtc-abx80x.c | 193 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 203 insertions(+)
 create mode 100644 drivers/rtc/rtc-abx80x.c

diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index b5b5c3d485d6..f66b94332992 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -164,6 +164,15 @@ 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
+	  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..4179fa624e7e
--- /dev/null
+++ b/drivers/rtc/rtc-abx80x.c
@@ -0,0 +1,193 @@
+/*
+ * An I2C driver for the Abracon AB08xx
+ *
+ * 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/module.h>
+#include <linux/i2c.h>
+#include <linux/bcd.h>
+#include <linux/rtc.h>
+#include <linux/log2.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_REG_PART0	0x28
+#define ABX8XX_REG_PART1	0x29
+
+#define ABX8XX_CTRL_WRITE	BIT(1)
+#define ABX8XX_CTRL_12_24	BIT(6)
+
+enum abx80x_chip {ABX80X, AB0801, AB0802, AB0803, AB0804, AB0805,
+	AB1801, AB1802, AB1803, AB1804, AB1805};
+
+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,
+					    8, 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]);
+	if (tm->tm_year < 70)
+		tm->tm_year += 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);
+	int data, err;
+	unsigned char buf[8];
+
+	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] = (0x1 << tm->tm_wday);
+
+	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_WRITE));
+	if (err < 0) {
+		dev_err(&client->dev, "Unable to write control register\n");
+		return -EIO;
+	}
+
+	err = i2c_smbus_write_i2c_block_data(client, ABX8XX_REG_SC, 7,
+					     &buf[ABX8XX_REG_SC]);
+	if (err < 0) {
+		dev_err(&client->dev, "Unable to write to date registers\n");
+		return -EIO;
+	}
+
+	err = i2c_smbus_write_byte_data(client, ABX8XX_REG_CTRL1,
+					(data & ~ABX8XX_CTRL_WRITE));
+	if (err < 0) {
+		dev_err(&client->dev, "Unable to write control register\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_probe(struct i2c_client *client,
+			const struct i2c_device_id *id)
+{
+	struct rtc_device *rtc;
+	int data, err, part0, part1;
+
+	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
+		return -ENODEV;
+
+	part0 = i2c_smbus_read_byte_data(client, ABX8XX_REG_PART0);
+	part1 = i2c_smbus_read_byte_data(client, ABX8XX_REG_PART1);
+	if ((part0 < 0) || (part1 < 0)) {
+		dev_err(&client->dev, "Unable to read part number\n");
+		return -EIO;
+	}
+	dev_info(&client->dev, "chip found %02x%02x\n", part0, part1);
+
+	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));
+	if (err < 0) {
+		dev_err(&client->dev, "Unable to write control register\n");
+		return -EIO;
+	}
+
+	rtc = devm_rtc_device_register(&client->dev, "rtc-abx80x",
+				       &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 },
+	{ "ab0802", AB0802 },
+	{ "ab0803", AB0803 },
+	{ "ab0804", AB0804 },
+	{ "ab0805", AB0805 },
+	{ "ab1801", AB1801 },
+	{ "ab1802", AB1802 },
+	{ "ab1803", AB1803 },
+	{ "ab1804", AB1804 },
+	{ "ab1805", AB1805 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, abx80x_id);
+
+static struct i2c_driver abx80x_driver = {
+	.driver		= {
+		.name	= "rtc-abx80x",
+		.owner	= THIS_MODULE,
+	},
+	.probe		= abx80x_probe,
+	.remove		= abx80x_remove,
+	.id_table	= abx80x_id,
+};
+
+module_i2c_driver(abx80x_driver);
+
+MODULE_AUTHOR("Alexandre Belloni <alexandre.belloni@free-electrons.com>");
+MODULE_DESCRIPTION("Abracon ABX80X RTC driver");
+MODULE_LICENSE("GPL");
-- 
2.1.0

Re: [PATCH v2] rtc: add Abracon ABx80x driver

[Paul Bolle]

On Sun, 2015-03-01 at 11:27 +0100, Alexandre Belloni wrote:
> --- /dev/null
> +++ b/drivers/rtc/rtc-abx80x.c
> @@ -0,0 +1,193 @@
> +/*
> + * An I2C driver for the Abracon AB08xx
> + *
> + * 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.
> + *
> + */

This states the license is plain GPL v2

[...]

> +MODULE_LICENSE("GPL");

So you probably want
    MODULE_LICENSE("GPL v2");

here.


Paul Bolle

Re: [PATCH v2] rtc: add Abracon ABx80x driver

[Andrew Morton]

On Sun,  1 Mar 2015 11:27:15 +0100 Alexandre Belloni <alexandre.belloni@free-electrons.com> wrote:

> Add support for the i2c RTC from Abracon.

What is the relationship between this patch and
http://ozlabs.org/~akpm/mmots/broken-out/rtc-add-rtc-abx805-a-driver-for-the-abracon-ab-1805-i2c-rtc.patch?


From: Alexandre Belloni <alexandre.belloni@free-electrons.com>
Subject: rtc: add Abracon ABx80x driver

Add support for the i2c RTC from Abracon.

Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
Cc: Philippe De Muyter <phdm@macqel.be>
Cc: Alessandro Zummo <a.zummo@towertech.it>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 drivers/rtc/Kconfig      |    9 +
 drivers/rtc/Makefile     |    1 
 drivers/rtc/rtc-abx80x.c |  193 +++++++++++++++++++++++++++++++++++++
 3 files changed, 203 insertions(+)

diff -puN drivers/rtc/Kconfig~rtc-add-abracon-abx80x-driver drivers/rtc/Kconfig
--- a/drivers/rtc/Kconfig~rtc-add-abracon-abx80x-driver
+++ a/drivers/rtc/Kconfig
@@ -164,6 +164,15 @@ 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
+	  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 -puN drivers/rtc/Makefile~rtc-add-abracon-abx80x-driver drivers/rtc/Makefile
--- a/drivers/rtc/Makefile~rtc-add-abracon-abx80x-driver
+++ a/drivers/rtc/Makefile
@@ -26,6 +26,7 @@ obj-$(CONFIG_RTC_DRV_AB3100)	+= rtc-ab31
 obj-$(CONFIG_RTC_DRV_AB8500)	+= rtc-ab8500.o
 obj-$(CONFIG_RTC_DRV_ABB5ZES3)	+= rtc-ab-b5ze-s3.o
 obj-$(CONFIG_RTC_DRV_ABX805)	+= rtc-abx805.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 -puN /dev/null drivers/rtc/rtc-abx80x.c
--- /dev/null
+++ a/drivers/rtc/rtc-abx80x.c
@@ -0,0 +1,193 @@
+/*
+ * An I2C driver for the Abracon AB08xx
+ *
+ * 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/module.h>
+#include <linux/i2c.h>
+#include <linux/bcd.h>
+#include <linux/rtc.h>
+#include <linux/log2.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_REG_PART0	0x28
+#define ABX8XX_REG_PART1	0x29
+
+#define ABX8XX_CTRL_WRITE	BIT(1)
+#define ABX8XX_CTRL_12_24	BIT(6)
+
+enum abx80x_chip {ABX80X, AB0801, AB0802, AB0803, AB0804, AB0805,
+	AB1801, AB1802, AB1803, AB1804, AB1805};
+
+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,
+					    8, 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]);
+	if (tm->tm_year < 70)
+		tm->tm_year += 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);
+	int data, err;
+	unsigned char buf[8];
+
+	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] = (0x1 << tm->tm_wday);
+
+	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_WRITE));
+	if (err < 0) {
+		dev_err(&client->dev, "Unable to write control register\n");
+		return -EIO;
+	}
+
+	err = i2c_smbus_write_i2c_block_data(client, ABX8XX_REG_SC, 7,
+					     &buf[ABX8XX_REG_SC]);
+	if (err < 0) {
+		dev_err(&client->dev, "Unable to write to date registers\n");
+		return -EIO;
+	}
+
+	err = i2c_smbus_write_byte_data(client, ABX8XX_REG_CTRL1,
+					(data & ~ABX8XX_CTRL_WRITE));
+	if (err < 0) {
+		dev_err(&client->dev, "Unable to write control register\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_probe(struct i2c_client *client,
+			const struct i2c_device_id *id)
+{
+	struct rtc_device *rtc;
+	int data, err, part0, part1;
+
+	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
+		return -ENODEV;
+
+	part0 = i2c_smbus_read_byte_data(client, ABX8XX_REG_PART0);
+	part1 = i2c_smbus_read_byte_data(client, ABX8XX_REG_PART1);
+	if ((part0 < 0) || (part1 < 0)) {
+		dev_err(&client->dev, "Unable to read part number\n");
+		return -EIO;
+	}
+	dev_info(&client->dev, "chip found %02x%02x\n", part0, part1);
+
+	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));
+	if (err < 0) {
+		dev_err(&client->dev, "Unable to write control register\n");
+		return -EIO;
+	}
+
+	rtc = devm_rtc_device_register(&client->dev, "rtc-abx80x",
+				       &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 },
+	{ "ab0802", AB0802 },
+	{ "ab0803", AB0803 },
+	{ "ab0804", AB0804 },
+	{ "ab0805", AB0805 },
+	{ "ab1801", AB1801 },
+	{ "ab1802", AB1802 },
+	{ "ab1803", AB1803 },
+	{ "ab1804", AB1804 },
+	{ "ab1805", AB1805 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, abx80x_id);
+
+static struct i2c_driver abx80x_driver = {
+	.driver		= {
+		.name	= "rtc-abx80x",
+		.owner	= THIS_MODULE,
+	},
+	.probe		= abx80x_probe,
+	.remove		= abx80x_remove,
+	.id_table	= abx80x_id,
+};
+
+module_i2c_driver(abx80x_driver);
+
+MODULE_AUTHOR("Alexandre Belloni <alexandre.belloni@free-electrons.com>");
+MODULE_DESCRIPTION("Abracon ABX80X RTC driver");
+MODULE_LICENSE("GPL");
_

Re: [PATCH v2] rtc: add Abracon ABx80x driver

[Alexandre Belloni]

On 02/03/2015 at 15:53:37 -0800, Andrew Morton wrote :
> On Sun,  1 Mar 2015 11:27:15 +0100 Alexandre Belloni <alexandre.belloni@free-electrons.com> wrote:
> 
> > Add support for the i2c RTC from Abracon.
> 
> What is the relationship between this patch and
> http://ozlabs.org/~akpm/mmots/broken-out/rtc-add-rtc-abx805-a-driver-for-the-abracon-ab-1805-i2c-rtc.patch?
> 

I'd say drop mine, I couldn't find the other one before writing it...

I'll try to build on Philippe's driver.


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

Re: [PATCH v2] rtc: add Abracon ABx80x driver

[Andrew Morton]

On Tue, 3 Mar 2015 02:11:16 +0100 Alexandre Belloni <alexandre.belloni@free-electrons.com> wrote:

> On 02/03/2015 at 15:53:37 -0800, Andrew Morton wrote :
> > On Sun,  1 Mar 2015 11:27:15 +0100 Alexandre Belloni <alexandre.belloni@free-electrons.com> wrote:
> > 
> > > Add support for the i2c RTC from Abracon.
> > 
> > What is the relationship between this patch and
> > http://ozlabs.org/~akpm/mmots/broken-out/rtc-add-rtc-abx805-a-driver-for-the-abracon-ab-1805-i2c-rtc.patch?
> > 
> 
> I'd say drop mine, I couldn't find the other one before writing it...
> 
> I'll try to build on Philippe's driver.

Which driver supports the most devices?  Your driver has

+static const struct i2c_device_id abx80x_id[] = {
+	{ "abx80x", ABX80X },
+	{ "ab0801", AB0801 },
+	{ "ab0802", AB0802 },
+	{ "ab0803", AB0803 },
+	{ "ab0804", AB0804 },
+	{ "ab0805", AB0805 },
+	{ "ab1801", AB1801 },
+	{ "ab1802", AB1802 },
+	{ "ab1803", AB1803 },
+	{ "ab1804", AB1804 },
+	{ "ab1805", AB1805 },
+	{ }
+};

And Philippe's has

+static struct i2c_device_id abx805_id[] = {
+	{ "abx805-rtc", 0 },
+	{ }
+};


And is the naming in Philippe's driver appropriate?  If it supports the
AB1801 (for example) then why is it described as an "abx805" driver?

Re: [PATCH v2] rtc: add Abracon ABx80x driver

[Alexandre Belloni]

On 03/03/2015 at 12:20:12 -0800, Andrew Morton wrote :
> On Tue, 3 Mar 2015 02:11:16 +0100 Alexandre Belloni <alexandre.belloni@free-electrons.com> wrote:
> 
> > On 02/03/2015 at 15:53:37 -0800, Andrew Morton wrote :
> > > On Sun,  1 Mar 2015 11:27:15 +0100 Alexandre Belloni <alexandre.belloni@free-electrons.com> wrote:
> > > 
> > > > Add support for the i2c RTC from Abracon.
> > > 
> > > What is the relationship between this patch and
> > > http://ozlabs.org/~akpm/mmots/broken-out/rtc-add-rtc-abx805-a-driver-for-the-abracon-ab-1805-i2c-rtc.patch?
> > > 
> > 
> > I'd say drop mine, I couldn't find the other one before writing it...
> > 
> > I'll try to build on Philippe's driver.
> 
> Which driver supports the most devices?  Your driver has
> 
> +static const struct i2c_device_id abx80x_id[] = {
> +	{ "abx80x", ABX80X },
> +	{ "ab0801", AB0801 },
> +	{ "ab0802", AB0802 },
> +	{ "ab0803", AB0803 },
> +	{ "ab0804", AB0804 },
> +	{ "ab0805", AB0805 },
> +	{ "ab1801", AB1801 },
> +	{ "ab1802", AB1802 },
> +	{ "ab1803", AB1803 },
> +	{ "ab1804", AB1804 },
> +	{ "ab1805", AB1805 },
> +	{ }
> +};
> 
> And Philippe's has
> 
> +static struct i2c_device_id abx805_id[] = {
> +	{ "abx805-rtc", 0 },
> +	{ }
> +};
> 
> 
> And is the naming in Philippe's driver appropriate?  If it supports the
> AB1801 (for example) then why is it described as an "abx805" driver?

The real naming is in the form ABx8yz. With:

x: 0 or 1, indicate the presence of the reset management
y: 0 or 1: 0 is i2c, 1 is spi
z: [1-5]: different amount of on chip SRAM. From what I understand, only
ABx8y5 are actually recommended for new designs.

They all share the same register set, apart from the reset management
that is only available on AB18yz.

>From my point of view, but I'm obviously biased, I'd take my patch
because it declares and supports all the available rtc and it also uses
the i2c_smbus_xxx API that is more robust than the raw i2c_transfer.

I also take care of the 12/24 mode bit and the write RTC bit which is
necessary to be able to write to the RTC.

What I like in Philippe's driver is the info printed at probe time and
the support for trickle charging. However, I wouldn't enable it
unconditionally.

To move forward, I can either send follow up patches based on what
Philippe did.

Or I can merge features from Philippe in my driver in a new patch,
keeping his authorship.

Regards,

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

Re: [PATCH v2] rtc: add Abracon ABx80x driver

[Andrew Morton]

On Tue, 3 Mar 2015 21:50:32 +0100 Alexandre Belloni <alexandre.belloni@free-electrons.com> wrote:

> To move forward, I can either send follow up patches based on what
> Philippe did.
> 
> Or I can merge features from Philippe in my driver in a new patch,
> keeping his authorship.

I'm easy either way ;) I guess we could put Philippe's name first
because I saw his patch first.  Or toss a coin.

Here's the latest version of Philippe's patch.  If you have the time
then I suggest you go ahead and create a new v3 patch along the lines
you suggest and we'll take it from there?



From: Philippe De Muyter <phdm@macqel.be>
Subject: rtc: add rtc-abx805, a driver for the Abracon AB 1805 i2c rtc

This is a basic driver for the ultra-low-power Abracon AB 1805 RTC chip. 
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>
Cc: 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      |    8 +
 drivers/rtc/Makefile     |    1 
 drivers/rtc/rtc-abx805.c |  223 +++++++++++++++++++++++++++++++++++++
 3 files changed, 232 insertions(+)

diff -puN drivers/rtc/Kconfig~rtc-add-rtc-abx805-a-driver-for-the-abracon-ab-1805-i2c-rtc drivers/rtc/Kconfig
--- a/drivers/rtc/Kconfig~rtc-add-rtc-abx805-a-driver-for-the-abracon-ab-1805-i2c-rtc
+++ a/drivers/rtc/Kconfig
@@ -1081,6 +1081,14 @@ config RTC_DRV_AB8500
 	  Select this to enable the ST-Ericsson AB8500 power management IC RTC
 	  support. This chip contains a battery- and capacitor-backed RTC.
 
+config RTC_DRV_ABX805
+	tristate "Abracon AB X805 RTC"
+	depends on I2C
+	help
+	  Select this to enable support for the Abracon AB X805 RTC.
+	  AB X805 is the i2c flavour of the AB 18X5 family of ultra-low-power
+	  battery- and capacitor-backed RTC..
+
 config RTC_DRV_NUC900
 	tristate "NUC910/NUC920 RTC driver"
 	depends on ARCH_W90X900
diff -puN drivers/rtc/Makefile~rtc-add-rtc-abx805-a-driver-for-the-abracon-ab-1805-i2c-rtc drivers/rtc/Makefile
--- a/drivers/rtc/Makefile~rtc-add-rtc-abx805-a-driver-for-the-abracon-ab-1805-i2c-rtc
+++ a/drivers/rtc/Makefile
@@ -25,6 +25,7 @@ obj-$(CONFIG_RTC_DRV_88PM80X)	+= rtc-88p
 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_ABX805)	+= rtc-abx805.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 -puN /dev/null drivers/rtc/rtc-abx805.c
--- /dev/null
+++ a/drivers/rtc/rtc-abx805.c
@@ -0,0 +1,223 @@
+/*
+ * A driver for the I2C members of the Abracon AB 18X5 RTC family,
+ * and compatible: AB 1805 and AB 0805
+ *
+ * Copyright 2014-2015 Macq S.A.
+ *
+ * Author: Philippe De Muyter <phdm@macqel.be>
+ *
+ * Based on rtc-em3027.c by Mike Rapoport <mike@compulab.co.il>
+ *
+ * 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/i2c.h>
+#include <linux/rtc.h>
+#include <linux/bcd.h>
+#include <linux/module.h>
+
+/* Registers */
+
+#define ABX805_REG_SECONDS		0x01
+#define ABX805_REG_CONFIGURATION_KEY	0x1f
+#define		KEY_ENABLE_MISC_REGISTERS_WRITE_ACCESS	0x90
+#define ABX805_REG_TRICKLE		0x20
+#define		TRICKLE_CHARGE_ENABLE		0xA0
+#define		TRICKLE_STANDARD_DIODE		0x8
+#define		TRICKLE_SCHOTTKY_DIODE		0x4
+#define		TRICKLE_OUTPUT_RESISTOR_3KOHM	0x1
+#define		TRICKLE_OUTPUT_RESISTOR_6KOHM	0x2
+#define		TRICKLE_OUTPUT_RESISTOR_11KOHM	0x3
+#define ABX805_REG_ID0			0x28
+
+static struct i2c_driver abx805_driver;
+
+static int abx805_read_multiple_regs(struct i2c_client *client,
+				     u8 *buf, u8 addr0, int len)
+{
+	u8 addr = addr0;
+	struct i2c_msg msgs[] = {
+		{/* setup read addr */
+			.addr = client->addr,
+			.len = 1,
+			.buf = &addr
+		},
+		{/* read into buf */
+			.addr = client->addr,
+			.flags = I2C_M_RD,
+			.len = len,
+			.buf = buf
+		},
+	};
+
+	if ((i2c_transfer(client->adapter, &msgs[0], 2)) != 2) {
+		dev_err(&client->dev, "%s: read error\n", __func__);
+		return -EIO;
+	}
+
+	return 0;
+}
+
+static int abx805_enable_trickle_charger(struct i2c_client *client)
+{
+	u8 buf[2];
+	struct i2c_msg msg = {
+		.addr = client->addr,
+		.len = 2,
+		.buf = buf,
+	};
+
+	/*
+	 * Write 0x90 in the configuration key register (0x1F) to enable
+	 * the access to the Trickle register
+	 */
+	buf[0] = ABX805_REG_CONFIGURATION_KEY;
+	buf[1] = 0x9D;
+
+	/* write register */
+	if ((i2c_transfer(client->adapter, &msg, 1)) != 1) {
+		dev_err(&client->dev, "%s: write error\n", __func__);
+		return -EIO;
+	}
+
+	buf[0] = ABX805_REG_TRICKLE;
+	buf[1] = TRICKLE_CHARGE_ENABLE | TRICKLE_SCHOTTKY_DIODE |
+		 TRICKLE_OUTPUT_RESISTOR_3KOHM;
+
+	/* write register */
+	if ((i2c_transfer(client->adapter, &msg, 1)) != 1) {
+		dev_err(&client->dev, "%s: write error\n", __func__);
+		return -EIO;
+	}
+	return 0;
+}
+
+static int abx805_get_time(struct device *dev, struct rtc_time *tm)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	u8 buf[7];
+	int err;
+
+	dev_dbg(dev, "abx805_get_time\n");
+	/* read time/date registers */
+	err = abx805_read_multiple_regs(client, buf, ABX805_REG_SECONDS,
+					sizeof(buf));
+	if (err) {
+		dev_err(&client->dev, "%s: read error\n", __func__);
+		return err;
+	}
+
+	tm->tm_sec	= bcd2bin(buf[0]);
+	tm->tm_min	= bcd2bin(buf[1]);
+	tm->tm_hour	= bcd2bin(buf[2]);
+	tm->tm_mday	= bcd2bin(buf[3]);
+	tm->tm_mon	= bcd2bin(buf[4]) - 1;
+	tm->tm_year	= bcd2bin(buf[5]) + 100;
+	tm->tm_wday	= bcd2bin(buf[6]);
+
+	return 0;
+}
+
+static int abx805_set_time(struct device *dev, struct rtc_time *tm)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	u8 buf[8];
+
+	struct i2c_msg msg = {
+		.addr = client->addr,
+		.len = 8,
+		.buf = buf,	/* write time/date */
+	};
+
+	dev_dbg(dev, "abx805_set_time\n");
+	buf[0] = ABX805_REG_SECONDS;
+	buf[1] = bin2bcd(tm->tm_sec);
+	buf[2] = bin2bcd(tm->tm_min);
+	buf[3] = bin2bcd(tm->tm_hour);
+	buf[4] = bin2bcd(tm->tm_mday);
+	buf[5] = bin2bcd(tm->tm_mon + 1);
+	buf[6] = bin2bcd(tm->tm_year % 100);
+	buf[7] = bin2bcd(tm->tm_wday);
+
+	/* write time/date registers */
+	if ((i2c_transfer(client->adapter, &msg, 1)) != 1) {
+		dev_err(&client->dev, "%s: write error\n", __func__);
+		return -EIO;
+	}
+
+	return 0;
+}
+
+static const struct rtc_class_ops abx805_rtc_ops = {
+	.read_time = abx805_get_time,
+	.set_time = abx805_set_time,
+};
+
+static int abx805_probe(struct i2c_client *client,
+			const struct i2c_device_id *id)
+{
+	int err;
+	struct rtc_device *rtc;
+	char buf[7];
+	unsigned int partnumber;
+	unsigned int majrev, minrev;
+	unsigned int lot;
+	unsigned int wafer;
+	unsigned int uid;
+
+	dev_info(&client->dev, "abx805_probe\n");
+	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
+		return -ENODEV;
+
+	err = abx805_read_multiple_regs(client, buf, ABX805_REG_ID0,
+					sizeof(buf));
+	if (err)
+		return err;
+
+	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);
+
+	abx805_enable_trickle_charger(client);
+
+	rtc = devm_rtc_device_register(&client->dev, abx805_driver.driver.name,
+				  &abx805_rtc_ops, THIS_MODULE);
+	if (IS_ERR(rtc))
+		return PTR_ERR(rtc);
+
+	i2c_set_clientdata(client, rtc);
+
+	return 0;
+}
+
+static int abx805_remove(struct i2c_client *client)
+{
+	return 0;
+}
+
+static struct i2c_device_id abx805_id[] = {
+	{ "abx805-rtc", 0 },
+	{ }
+};
+
+static struct i2c_driver abx805_driver = {
+	.driver = {
+		   .name = "abx805-rtc",
+	},
+	.probe = &abx805_probe,
+	.remove = &abx805_remove,
+	.id_table = abx805_id,
+};
+
+module_i2c_driver(abx805_driver);
+
+MODULE_AUTHOR("Philippe De Muyter <phdm@macqel.be>");
+MODULE_DESCRIPTION("Abracon AB X805 RTC driver");
+MODULE_LICENSE("GPL");
_

Re: [PATCH v2] rtc: add Abracon ABx80x driver

[Alexandre Belloni]

On 03/03/2015 at 14:32:07 -0800, Andrew Morton wrote :
> On Tue, 3 Mar 2015 21:50:32 +0100 Alexandre Belloni <alexandre.belloni@free-electrons.com> wrote:
> 
> > To move forward, I can either send follow up patches based on what
> > Philippe did.
> > 
> > Or I can merge features from Philippe in my driver in a new patch,
> > keeping his authorship.
> 
> I'm easy either way ;) I guess we could put Philippe's name first
> because I saw his patch first.  Or toss a coin.
> 
> Here's the latest version of Philippe's patch.  If you have the time
> then I suggest you go ahead and create a new v3 patch along the lines
> you suggest and we'll take it from there?
> 

I'll cook something up. What about the name?

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

Re: [PATCH v2] rtc: add Abracon ABx80x driver

[Philippe De Muyter]

On Tue, Mar 03, 2015 at 09:50:32PM +0100, Alexandre Belloni wrote:
> On 03/03/2015 at 12:20:12 -0800, Andrew Morton wrote :
> > On Tue, 3 Mar 2015 02:11:16 +0100 Alexandre Belloni <alexandre.belloni@free-electrons.com> wrote:
> > 
> > > On 02/03/2015 at 15:53:37 -0800, Andrew Morton wrote :
> > > > On Sun,  1 Mar 2015 11:27:15 +0100 Alexandre Belloni <alexandre.belloni@free-electrons.com> wrote:
> > > > 
> > > > > Add support for the i2c RTC from Abracon.
> > > > 
> > > > What is the relationship between this patch and
> > > > http://ozlabs.org/~akpm/mmots/broken-out/rtc-add-rtc-abx805-a-driver-for-the-abracon-ab-1805-i2c-rtc.patch?
> > > > 
> > > 
> > > I'd say drop mine, I couldn't find the other one before writing it...
> > > 
> > > I'll try to build on Philippe's driver.
> > 
> > Which driver supports the most devices?  Your driver has
> > 
> > +static const struct i2c_device_id abx80x_id[] = {
> > +	{ "abx80x", ABX80X },
> > +	{ "ab0801", AB0801 },
> > +	{ "ab0802", AB0802 },
> > +	{ "ab0803", AB0803 },
> > +	{ "ab0804", AB0804 },
> > +	{ "ab0805", AB0805 },
> > +	{ "ab1801", AB1801 },
> > +	{ "ab1802", AB1802 },
> > +	{ "ab1803", AB1803 },
> > +	{ "ab1804", AB1804 },
> > +	{ "ab1805", AB1805 },
> > +	{ }
> > +};
> > 
> > And Philippe's has
> > 
> > +static struct i2c_device_id abx805_id[] = {
> > +	{ "abx805-rtc", 0 },

The only part my driver is actually tested with, is the 'AB1805', but see
below.

> > +	{ }
> > +};
> > 
> > 
> > And is the naming in Philippe's driver appropriate?  If it supports the
> > AB1801 (for example) then why is it described as an "abx805" driver?
> 
> The real naming is in the form ABx8yz. With:
> 
> x: 0 or 1, indicate the presence of the reset management
> y: 0 or 1: 0 is i2c, 1 is spi
> z: [1-5]: different amount of on chip SRAM. From what I understand, only
> ABx8y5 are actually recommended for new designs.

My driver is based on the datashet called 'AB18X5 Real-Time Clock with
Power Management Family', which calls the parts 'AB18X5 family' with X
being '0' for I2c parts and '1' for SPI parts.

As the part I have and the driver I wrote are I2C, not SPI, I fixed the
'X' as 0 in the name of the driver.

The datasheet lists only one part as being 'software and pin compatible'
with the 'AB1805': the 'AB0805', hence the 'x' in the name I choose : abx805.

> 
> They all share the same register set, apart from the reset management
> that is only available on AB18yz.
> 
> >From my point of view, but I'm obviously biased, I'd take my patch
> because it declares and supports all the available rtc and it also uses
> the i2c_smbus_xxx API that is more robust than the raw i2c_transfer.

I have no opinion on that.  The driver I started from uses 'raw i2c_transfer'.

> 
> I also take care of the 12/24 mode bit and the write RTC bit which is
> necessary to be able to write to the RTC.

Actually, my driver is used in production and works fine, because the
default(reset) value of the 12/24 mode bit is '24 hour mode' and the
default value of the 'write RTC bit' enables writing.  There is
no real need to play with them.

> 
> What I like in Philippe's driver is the info printed at probe time and
> the support for trickle charging. However, I wouldn't enable it
> unconditionally.

My hardware colleagues told me that the only way to enable the 'ultra low-power'
functionality is enabling the trickle charger.  And the 'ultra low-power'
functionality is the reason we choose that chip, so I would at least
keep that as the default behaviour.

Regards

Philippe

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

Re: [PATCH v2] rtc: add Abracon ABx80x driver

[Alexandre Belloni]

On 04/03/2015 at 09:52:42 +0100, Philippe De Muyter wrote :
> > > And is the naming in Philippe's driver appropriate?  If it supports the
> > > AB1801 (for example) then why is it described as an "abx805" driver?
> > 
> > The real naming is in the form ABx8yz. With:
> > 
> > x: 0 or 1, indicate the presence of the reset management
> > y: 0 or 1: 0 is i2c, 1 is spi
> > z: [1-5]: different amount of on chip SRAM. From what I understand, only
> > ABx8y5 are actually recommended for new designs.
> 
> My driver is based on the datashet called 'AB18X5 Real-Time Clock with
> Power Management Family', which calls the parts 'AB18X5 family' with X
> being '0' for I2c parts and '1' for SPI parts.
> 
> As the part I have and the driver I wrote are I2C, not SPI, I fixed the
> 'X' as 0 in the name of the driver.
> 
> The datasheet lists only one part as being 'software and pin compatible'
> with the 'AB1805': the 'AB0805', hence the 'x' in the name I choose : abx805.
> 

The "AB08XX Real-Time Clock Family" document states that they are all
software and pin compatible (including the AB18xx).

> Actually, my driver is used in production and works fine, because the
> default(reset) value of the 12/24 mode bit is '24 hour mode' and the
> default value of the 'write RTC bit' enables writing.  There is
> no real need to play with them.
> 

I know this is unlikely to happen but what if someone messes with the
RTC in the bootloader?

> > 
> > What I like in Philippe's driver is the info printed at probe time and
> > the support for trickle charging. However, I wouldn't enable it
> > unconditionally.
> 
> My hardware colleagues told me that the only way to enable the 'ultra low-power'
> functionality is enabling the trickle charger.  And the 'ultra low-power'
> functionality is the reason we choose that chip, so I would at least
> keep that as the default behaviour.
> 

My concern is that you have a static configuration. I would expose a
sysfs interface to configure the diode, resistor and enable/disable the
trickle charger. Would that work for you?


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

Re: [PATCH v2] rtc: add Abracon ABx80x driver

[Philippe De Muyter]

Hi Alexandre,

On Wed, Mar 04, 2015 at 10:06:01AM +0100, Alexandre Belloni wrote:
> On 04/03/2015 at 09:52:42 +0100, Philippe De Muyter wrote :
> > > > And is the naming in Philippe's driver appropriate?  If it supports the
> > > > AB1801 (for example) then why is it described as an "abx805" driver?
> > > 
> > > The real naming is in the form ABx8yz. With:
> > > 
> > > x: 0 or 1, indicate the presence of the reset management
> > > y: 0 or 1: 0 is i2c, 1 is spi
> > > z: [1-5]: different amount of on chip SRAM. From what I understand, only
> > > ABx8y5 are actually recommended for new designs.
> > 
> > My driver is based on the datashet called 'AB18X5 Real-Time Clock with
> > Power Management Family', which calls the parts 'AB18X5 family' with X
> > being '0' for I2c parts and '1' for SPI parts.
> > 
> > As the part I have and the driver I wrote are I2C, not SPI, I fixed the
> > 'X' as 0 in the name of the driver.
> > 
> > The datasheet lists only one part as being 'software and pin compatible'
> > with the 'AB1805': the 'AB0805', hence the 'x' in the name I choose : abx805.
> > 
> 
> The "AB08XX Real-Time Clock Family" document states that they are all
> software and pin compatible (including the AB18xx).

Which part(s) do you really have ?  Mine is a 1805.  Do all the parts have
the trickle charger ?  As I don't know, I prefer not to pretend that the
driver supports those chips.

> 
> > Actually, my driver is used in production and works fine, because the
> > default(reset) value of the 12/24 mode bit is '24 hour mode' and the
> > default value of the 'write RTC bit' enables writing.  There is
> > no real need to play with them.
> > 
> 
> I know this is unlikely to happen but what if someone messes with the
> RTC in the bootloader?

Yes, you may unconditionnaly rewrite this register if you want.

> 
> > > 
> > > What I like in Philippe's driver is the info printed at probe time and
> > > the support for trickle charging. However, I wouldn't enable it
> > > unconditionally.
> > 
> > My hardware colleagues told me that the only way to enable the 'ultra low-power'
> > functionality is enabling the trickle charger.  And the 'ultra low-power'
> > functionality is the reason we choose that chip, so I would at least
> > keep that as the default behaviour.
> > 
> 
> My concern is that you have a static configuration. I would expose a
> sysfs interface to configure the diode, resistor and enable/disable the
> trickle charger. Would that work for you?

I think a 'of_xxx' dts/dtb description is the way to go, but I did not want to
introduce unnecessaty complexity without knowing other usages.  My hardware
colleagues followed the recommended design from Abracon.

Philippe

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

Re: [PATCH v2] rtc: add Abracon ABx80x driver

[Alexandre Belloni]

On 04/03/2015 at 10:55:56 +0100, Philippe De Muyter wrote :
> > The "AB08XX Real-Time Clock Family" document states that they are all
> > software and pin compatible (including the AB18xx).
> 
> Which part(s) do you really have ?  Mine is a 1805.  Do all the parts have
> the trickle charger ?  As I don't know, I prefer not to pretend that the
> driver supports those chips.
> 

I have the AB0805. The trickle charger is not available on ABx8x1 and
ABx8x3.

> > > My hardware colleagues told me that the only way to enable the 'ultra low-power'
> > > functionality is enabling the trickle charger.  And the 'ultra low-power'
> > > functionality is the reason we choose that chip, so I would at least
> > > keep that as the default behaviour.
> > > 
> > 
> > My concern is that you have a static configuration. I would expose a
> > sysfs interface to configure the diode, resistor and enable/disable the
> > trickle charger. Would that work for you?
> 
> I think a 'of_xxx' dts/dtb description is the way to go, but I did not want to
> introduce unnecessaty complexity without knowing other usages.  My hardware
> colleagues followed the recommended design from Abracon.
> 

Yeah, I'm not sure about the DT description, some may argue this is
configuration. But I guess it actually depends on the battery you are
using so that could count as hardware description.

I'll use:
 abracon,tc-diode = "(standard|schottky)"
 abracon,tc-resistor = <(0|3|6|11)>

If both are defined and valid, I'll enable the trickle charger with the
provided configuration in probe().

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

Re: [PATCH v2] rtc: add Abracon ABx80x driver

[Philippe De Muyter]

Hi Alexandre,

On Wed, Mar 04, 2015 at 11:38:14AM +0100, Alexandre Belloni wrote:
> On 04/03/2015 at 10:55:56 +0100, Philippe De Muyter wrote :
> > > The "AB08XX Real-Time Clock Family" document states that they are all
> > > software and pin compatible (including the AB18xx).
> > 
> > Which part(s) do you really have ?  Mine is a 1805.  Do all the parts have
> > the trickle charger ?  As I don't know, I prefer not to pretend that the
> > driver supports those chips.
> > 
> 
> I have the AB0805. The trickle charger is not available on ABx8x1 and
> ABx8x3.
> 
> > > > My hardware colleagues told me that the only way to enable the 'ultra low-power'
> > > > functionality is enabling the trickle charger.  And the 'ultra low-power'
> > > > functionality is the reason we choose that chip, so I would at least
> > > > keep that as the default behaviour.
> > > > 
> > > 
> > > My concern is that you have a static configuration. I would expose a
> > > sysfs interface to configure the diode, resistor and enable/disable the
> > > trickle charger. Would that work for you?
> > 
> > I think a 'of_xxx' dts/dtb description is the way to go, but I did not want to
> > introduce unnecessaty complexity without knowing other usages.  My hardware
> > colleagues followed the recommended design from Abracon.
> > 
> 
> Yeah, I'm not sure about the DT description, some may argue this is
> configuration. But I guess it actually depends on the battery you are
> using so that could count as hardware description.

As far as I know, it's purely hardware related.

> 
> I'll use:
>  abracon,tc-diode = "(standard|schottky)"
>  abracon,tc-resistor = <(0|3|6|11)>
> 
> If both are defined and valid, I'll enable the trickle charger with the
> provided configuration in probe().

My current entry is :

&i2c2 {
	...
        ab1805@69 {
                compatible = "abracon,abx805-rtc";
                reg = <0x69>;
                position = <3>;
        };
};

It would then become :

&i2c2 {
	...
        ab1805@69 {
                compatible = "abracon,abx805-rtc";
                reg = <0x69>;
                position = <3>;
		abracon,tc-diode = "schottky";
		abracon,tc-resistor = <3>;
        };
};

or if you change the driver's name to "abx80x-rtc"

                compatible = "abracon,abx80x-rtc";

That's fine for me.  Thanks !

Philippe

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