[rtc-linux] [PATCH v3 1/8] rtc: ds3232: rename rtc-ds3232.c to rtc-ds3232-core.c for preparation

[rtc-linux] [PATCH v3 1/8] rtc: ds3232: rename rtc-ds3232.c to rtc-ds3232-core.c for preparation

[Akinobu Mita]

This is preparation for splitting into core module and i2c driver.
This will make it a lot easier to review the real changes.

Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: Alexandre Belloni <alexandre.belloni@free-electrons.com>
Cc: Dennis Aberilla <denzzzhome@yahoo.com>
---
* No changes since v1

 drivers/rtc/Makefile          |   1 +
 drivers/rtc/rtc-ds3232-core.c | 517 ++++++++++++++++++++++++++++++++++++++++++
 drivers/rtc/rtc-ds3232.c      | 517 ------------------------------------------
 3 files changed, 518 insertions(+), 517 deletions(-)
 create mode 100644 drivers/rtc/rtc-ds3232-core.c
 delete mode 100644 drivers/rtc/rtc-ds3232.c

diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
index 62d61b2..0b8cb6c 100644
--- a/drivers/rtc/Makefile
+++ b/drivers/rtc/Makefile
@@ -59,6 +59,7 @@ obj-$(CONFIG_RTC_DRV_DS1685_FAMILY)	+= rtc-ds1685.o
 obj-$(CONFIG_RTC_DRV_DS1742)	+= rtc-ds1742.o
 obj-$(CONFIG_RTC_DRV_DS2404)	+= rtc-ds2404.o
 obj-$(CONFIG_RTC_DRV_DS3232)	+= rtc-ds3232.o
+rtc-ds3232-objs			:= rtc-ds3232-core.o
 obj-$(CONFIG_RTC_DRV_DS3234)	+= rtc-ds3234.o
 obj-$(CONFIG_RTC_DRV_EFI)	+= rtc-efi.o
 obj-$(CONFIG_RTC_DRV_EM3027)	+= rtc-em3027.o
diff --git a/drivers/rtc/rtc-ds3232-core.c b/drivers/rtc/rtc-ds3232-core.c
new file mode 100644
index 0000000..4e99ace
--- /dev/null
+++ b/drivers/rtc/rtc-ds3232-core.c
@@ -0,0 +1,517 @@
+/*
+ * RTC client/driver for the Maxim/Dallas DS3232 Real-Time Clock over I2C
+ *
+ * Copyright (C) 2009-2011 Freescale Semiconductor.
+ * Author: Jack Lan <jack.lan@freescale.com>
+ *
+ * This program is free software; you can redistribute  it and/or modify it
+ * under  the terms of  the GNU General  Public License as published by the
+ * Free Software Foundation;  either version 2 of the  License, or (at your
+ * option) any later version.
+ */
+/*
+ * It would be more efficient to use i2c msgs/i2c_transfer directly but, as
+ * recommened in .../Documentation/i2c/writing-clients section
+ * "Sending and receiving", using SMBus level communication is preferred.
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/interrupt.h>
+#include <linux/i2c.h>
+#include <linux/rtc.h>
+#include <linux/bcd.h>
+#include <linux/workqueue.h>
+#include <linux/slab.h>
+
+#define DS3232_REG_SECONDS	0x00
+#define DS3232_REG_MINUTES	0x01
+#define DS3232_REG_HOURS	0x02
+#define DS3232_REG_AMPM		0x02
+#define DS3232_REG_DAY		0x03
+#define DS3232_REG_DATE		0x04
+#define DS3232_REG_MONTH	0x05
+#define DS3232_REG_CENTURY	0x05
+#define DS3232_REG_YEAR		0x06
+#define DS3232_REG_ALARM1         0x07	/* Alarm 1 BASE */
+#define DS3232_REG_ALARM2         0x0B	/* Alarm 2 BASE */
+#define DS3232_REG_CR		0x0E	/* Control register */
+#	define DS3232_REG_CR_nEOSC        0x80
+#       define DS3232_REG_CR_INTCN        0x04
+#       define DS3232_REG_CR_A2IE        0x02
+#       define DS3232_REG_CR_A1IE        0x01
+
+#define DS3232_REG_SR	0x0F	/* control/status register */
+#	define DS3232_REG_SR_OSF   0x80
+#       define DS3232_REG_SR_BSY   0x04
+#       define DS3232_REG_SR_A2F   0x02
+#       define DS3232_REG_SR_A1F   0x01
+
+struct ds3232 {
+	struct i2c_client *client;
+	struct rtc_device *rtc;
+	struct work_struct work;
+
+	/* The mutex protects alarm operations, and prevents a race
+	 * between the enable_irq() in the workqueue and the free_irq()
+	 * in the remove function.
+	 */
+	struct mutex mutex;
+	bool suspended;
+	int exiting;
+};
+
+static struct i2c_driver ds3232_driver;
+
+static int ds3232_check_rtc_status(struct i2c_client *client)
+{
+	int ret = 0;
+	int control, stat;
+
+	stat = i2c_smbus_read_byte_data(client, DS3232_REG_SR);
+	if (stat < 0)
+		return stat;
+
+	if (stat & DS3232_REG_SR_OSF)
+		dev_warn(&client->dev,
+				"oscillator discontinuity flagged, "
+				"time unreliable\n");
+
+	stat &= ~(DS3232_REG_SR_OSF | DS3232_REG_SR_A1F | DS3232_REG_SR_A2F);
+
+	ret = i2c_smbus_write_byte_data(client, DS3232_REG_SR, stat);
+	if (ret < 0)
+		return ret;
+
+	/* If the alarm is pending, clear it before requesting
+	 * the interrupt, so an interrupt event isn't reported
+	 * before everything is initialized.
+	 */
+
+	control = i2c_smbus_read_byte_data(client, DS3232_REG_CR);
+	if (control < 0)
+		return control;
+
+	control &= ~(DS3232_REG_CR_A1IE | DS3232_REG_CR_A2IE);
+	control |= DS3232_REG_CR_INTCN;
+
+	return i2c_smbus_write_byte_data(client, DS3232_REG_CR, control);
+}
+
+static int ds3232_read_time(struct device *dev, struct rtc_time *time)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	int ret;
+	u8 buf[7];
+	unsigned int year, month, day, hour, minute, second;
+	unsigned int week, twelve_hr, am_pm;
+	unsigned int century, add_century = 0;
+
+	ret = i2c_smbus_read_i2c_block_data(client, DS3232_REG_SECONDS, 7, buf);
+
+	if (ret < 0)
+		return ret;
+	if (ret < 7)
+		return -EIO;
+
+	second = buf[0];
+	minute = buf[1];
+	hour = buf[2];
+	week = buf[3];
+	day = buf[4];
+	month = buf[5];
+	year = buf[6];
+
+	/* Extract additional information for AM/PM and century */
+
+	twelve_hr = hour & 0x40;
+	am_pm = hour & 0x20;
+	century = month & 0x80;
+
+	/* Write to rtc_time structure */
+
+	time->tm_sec = bcd2bin(second);
+	time->tm_min = bcd2bin(minute);
+	if (twelve_hr) {
+		/* Convert to 24 hr */
+		if (am_pm)
+			time->tm_hour = bcd2bin(hour & 0x1F) + 12;
+		else
+			time->tm_hour = bcd2bin(hour & 0x1F);
+	} else {
+		time->tm_hour = bcd2bin(hour);
+	}
+
+	/* Day of the week in linux range is 0~6 while 1~7 in RTC chip */
+	time->tm_wday = bcd2bin(week) - 1;
+	time->tm_mday = bcd2bin(day);
+	/* linux tm_mon range:0~11, while month range is 1~12 in RTC chip */
+	time->tm_mon = bcd2bin(month & 0x7F) - 1;
+	if (century)
+		add_century = 100;
+
+	time->tm_year = bcd2bin(year) + add_century;
+
+	return rtc_valid_tm(time);
+}
+
+static int ds3232_set_time(struct device *dev, struct rtc_time *time)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	u8 buf[7];
+
+	/* Extract time from rtc_time and load into ds3232*/
+
+	buf[0] = bin2bcd(time->tm_sec);
+	buf[1] = bin2bcd(time->tm_min);
+	buf[2] = bin2bcd(time->tm_hour);
+	/* Day of the week in linux range is 0~6 while 1~7 in RTC chip */
+	buf[3] = bin2bcd(time->tm_wday + 1);
+	buf[4] = bin2bcd(time->tm_mday); /* Date */
+	/* linux tm_mon range:0~11, while month range is 1~12 in RTC chip */
+	buf[5] = bin2bcd(time->tm_mon + 1);
+	if (time->tm_year >= 100) {
+		buf[5] |= 0x80;
+		buf[6] = bin2bcd(time->tm_year - 100);
+	} else {
+		buf[6] = bin2bcd(time->tm_year);
+	}
+
+	return i2c_smbus_write_i2c_block_data(client,
+					      DS3232_REG_SECONDS, 7, buf);
+}
+
+/*
+ * DS3232 has two alarm, we only use alarm1
+ * According to linux specification, only support one-shot alarm
+ * no periodic alarm mode
+ */
+static int ds3232_read_alarm(struct device *dev, struct rtc_wkalrm *alarm)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct ds3232 *ds3232 = i2c_get_clientdata(client);
+	int control, stat;
+	int ret;
+	u8 buf[4];
+
+	mutex_lock(&ds3232->mutex);
+
+	ret = i2c_smbus_read_byte_data(client, DS3232_REG_SR);
+	if (ret < 0)
+		goto out;
+	stat = ret;
+	ret = i2c_smbus_read_byte_data(client, DS3232_REG_CR);
+	if (ret < 0)
+		goto out;
+	control = ret;
+	ret = i2c_smbus_read_i2c_block_data(client, DS3232_REG_ALARM1, 4, buf);
+	if (ret < 0)
+		goto out;
+
+	alarm->time.tm_sec = bcd2bin(buf[0] & 0x7F);
+	alarm->time.tm_min = bcd2bin(buf[1] & 0x7F);
+	alarm->time.tm_hour = bcd2bin(buf[2] & 0x7F);
+	alarm->time.tm_mday = bcd2bin(buf[3] & 0x7F);
+
+	alarm->time.tm_mon = -1;
+	alarm->time.tm_year = -1;
+	alarm->time.tm_wday = -1;
+	alarm->time.tm_yday = -1;
+	alarm->time.tm_isdst = -1;
+
+	alarm->enabled = !!(control & DS3232_REG_CR_A1IE);
+	alarm->pending = !!(stat & DS3232_REG_SR_A1F);
+
+	ret = 0;
+out:
+	mutex_unlock(&ds3232->mutex);
+	return ret;
+}
+
+/*
+ * linux rtc-module does not support wday alarm
+ * and only 24h time mode supported indeed
+ */
+static int ds3232_set_alarm(struct device *dev, struct rtc_wkalrm *alarm)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct ds3232 *ds3232 = i2c_get_clientdata(client);
+	int control, stat;
+	int ret;
+	u8 buf[4];
+
+	if (client->irq <= 0)
+		return -EINVAL;
+
+	mutex_lock(&ds3232->mutex);
+
+	buf[0] = bin2bcd(alarm->time.tm_sec);
+	buf[1] = bin2bcd(alarm->time.tm_min);
+	buf[2] = bin2bcd(alarm->time.tm_hour);
+	buf[3] = bin2bcd(alarm->time.tm_mday);
+
+	/* clear alarm interrupt enable bit */
+	ret = i2c_smbus_read_byte_data(client, DS3232_REG_CR);
+	if (ret < 0)
+		goto out;
+	control = ret;
+	control &= ~(DS3232_REG_CR_A1IE | DS3232_REG_CR_A2IE);
+	ret = i2c_smbus_write_byte_data(client, DS3232_REG_CR, control);
+	if (ret < 0)
+		goto out;
+
+	/* clear any pending alarm flag */
+	ret = i2c_smbus_read_byte_data(client, DS3232_REG_SR);
+	if (ret < 0)
+		goto out;
+	stat = ret;
+	stat &= ~(DS3232_REG_SR_A1F | DS3232_REG_SR_A2F);
+	ret = i2c_smbus_write_byte_data(client, DS3232_REG_SR, stat);
+	if (ret < 0)
+		goto out;
+
+	ret = i2c_smbus_write_i2c_block_data(client, DS3232_REG_ALARM1, 4, buf);
+
+	if (alarm->enabled) {
+		control |= DS3232_REG_CR_A1IE;
+		ret = i2c_smbus_write_byte_data(client, DS3232_REG_CR, control);
+	}
+out:
+	mutex_unlock(&ds3232->mutex);
+	return ret;
+}
+
+static void ds3232_update_alarm(struct i2c_client *client)
+{
+	struct ds3232 *ds3232 = i2c_get_clientdata(client);
+	int control;
+	int ret;
+	u8 buf[4];
+
+	mutex_lock(&ds3232->mutex);
+
+	ret = i2c_smbus_read_i2c_block_data(client, DS3232_REG_ALARM1, 4, buf);
+	if (ret < 0)
+		goto unlock;
+
+	buf[0] = bcd2bin(buf[0]) < 0 || (ds3232->rtc->irq_data & RTC_UF) ?
+								0x80 : buf[0];
+	buf[1] = bcd2bin(buf[1]) < 0 || (ds3232->rtc->irq_data & RTC_UF) ?
+								0x80 : buf[1];
+	buf[2] = bcd2bin(buf[2]) < 0 || (ds3232->rtc->irq_data & RTC_UF) ?
+								0x80 : buf[2];
+	buf[3] = bcd2bin(buf[3]) < 0 || (ds3232->rtc->irq_data & RTC_UF) ?
+								0x80 : buf[3];
+
+	ret = i2c_smbus_write_i2c_block_data(client, DS3232_REG_ALARM1, 4, buf);
+	if (ret < 0)
+		goto unlock;
+
+	control = i2c_smbus_read_byte_data(client, DS3232_REG_CR);
+	if (control < 0)
+		goto unlock;
+
+	if (ds3232->rtc->irq_data & (RTC_AF | RTC_UF))
+		/* enable alarm1 interrupt */
+		control |= DS3232_REG_CR_A1IE;
+	else
+		/* disable alarm1 interrupt */
+		control &= ~(DS3232_REG_CR_A1IE);
+	i2c_smbus_write_byte_data(client, DS3232_REG_CR, control);
+
+unlock:
+	mutex_unlock(&ds3232->mutex);
+}
+
+static int ds3232_alarm_irq_enable(struct device *dev, unsigned int enabled)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct ds3232 *ds3232 = i2c_get_clientdata(client);
+
+	if (client->irq <= 0)
+		return -EINVAL;
+
+	if (enabled)
+		ds3232->rtc->irq_data |= RTC_AF;
+	else
+		ds3232->rtc->irq_data &= ~RTC_AF;
+
+	ds3232_update_alarm(client);
+	return 0;
+}
+
+static irqreturn_t ds3232_irq(int irq, void *dev_id)
+{
+	struct i2c_client *client = dev_id;
+	struct ds3232 *ds3232 = i2c_get_clientdata(client);
+
+	disable_irq_nosync(irq);
+
+	/*
+	 * If rtc as a wakeup source, can't schedule the work
+	 * at system resume flow, because at this time the i2c bus
+	 * has not been resumed.
+	 */
+	if (!ds3232->suspended)
+		schedule_work(&ds3232->work);
+
+	return IRQ_HANDLED;
+}
+
+static void ds3232_work(struct work_struct *work)
+{
+	struct ds3232 *ds3232 = container_of(work, struct ds3232, work);
+	struct i2c_client *client = ds3232->client;
+	int stat, control;
+
+	mutex_lock(&ds3232->mutex);
+
+	stat = i2c_smbus_read_byte_data(client, DS3232_REG_SR);
+	if (stat < 0)
+		goto unlock;
+
+	if (stat & DS3232_REG_SR_A1F) {
+		control = i2c_smbus_read_byte_data(client, DS3232_REG_CR);
+		if (control < 0) {
+			pr_warn("Read Control Register error - Disable IRQ%d\n",
+				client->irq);
+		} else {
+			/* disable alarm1 interrupt */
+			control &= ~(DS3232_REG_CR_A1IE);
+			i2c_smbus_write_byte_data(client, DS3232_REG_CR,
+						control);
+
+			/* clear the alarm pend flag */
+			stat &= ~DS3232_REG_SR_A1F;
+			i2c_smbus_write_byte_data(client, DS3232_REG_SR, stat);
+
+			rtc_update_irq(ds3232->rtc, 1, RTC_AF | RTC_IRQF);
+
+			if (!ds3232->exiting)
+				enable_irq(client->irq);
+		}
+	}
+
+unlock:
+	mutex_unlock(&ds3232->mutex);
+}
+
+static const struct rtc_class_ops ds3232_rtc_ops = {
+	.read_time = ds3232_read_time,
+	.set_time = ds3232_set_time,
+	.read_alarm = ds3232_read_alarm,
+	.set_alarm = ds3232_set_alarm,
+	.alarm_irq_enable = ds3232_alarm_irq_enable,
+};
+
+static int ds3232_probe(struct i2c_client *client,
+			const struct i2c_device_id *id)
+{
+	struct ds3232 *ds3232;
+	int ret;
+
+	ds3232 = devm_kzalloc(&client->dev, sizeof(struct ds3232), GFP_KERNEL);
+	if (!ds3232)
+		return -ENOMEM;
+
+	ds3232->client = client;
+	i2c_set_clientdata(client, ds3232);
+
+	INIT_WORK(&ds3232->work, ds3232_work);
+	mutex_init(&ds3232->mutex);
+
+	ret = ds3232_check_rtc_status(client);
+	if (ret)
+		return ret;
+
+	if (client->irq > 0) {
+		ret = devm_request_irq(&client->dev, client->irq, ds3232_irq,
+				       IRQF_SHARED, "ds3232", client);
+		if (ret) {
+			dev_err(&client->dev, "unable to request IRQ\n");
+		}
+		device_init_wakeup(&client->dev, 1);
+	}
+	ds3232->rtc = devm_rtc_device_register(&client->dev, client->name,
+					  &ds3232_rtc_ops, THIS_MODULE);
+	return PTR_ERR_OR_ZERO(ds3232->rtc);
+}
+
+static int ds3232_remove(struct i2c_client *client)
+{
+	struct ds3232 *ds3232 = i2c_get_clientdata(client);
+
+	if (client->irq > 0) {
+		mutex_lock(&ds3232->mutex);
+		ds3232->exiting = 1;
+		mutex_unlock(&ds3232->mutex);
+
+		devm_free_irq(&client->dev, client->irq, client);
+		cancel_work_sync(&ds3232->work);
+	}
+
+	return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int ds3232_suspend(struct device *dev)
+{
+	struct ds3232 *ds3232 = dev_get_drvdata(dev);
+	struct i2c_client *client = to_i2c_client(dev);
+
+	if (device_can_wakeup(dev)) {
+		ds3232->suspended = true;
+		if (irq_set_irq_wake(client->irq, 1)) {
+			dev_warn_once(dev, "Cannot set wakeup source\n");
+			ds3232->suspended = false;
+		}
+	}
+
+	return 0;
+}
+
+static int ds3232_resume(struct device *dev)
+{
+	struct ds3232 *ds3232 = dev_get_drvdata(dev);
+	struct i2c_client *client = to_i2c_client(dev);
+
+	if (ds3232->suspended) {
+		ds3232->suspended = false;
+
+		/* Clear the hardware alarm pend flag */
+		schedule_work(&ds3232->work);
+
+		irq_set_irq_wake(client->irq, 0);
+	}
+
+	return 0;
+}
+#endif
+
+static const struct dev_pm_ops ds3232_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(ds3232_suspend, ds3232_resume)
+};
+
+static const struct i2c_device_id ds3232_id[] = {
+	{ "ds3232", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, ds3232_id);
+
+static struct i2c_driver ds3232_driver = {
+	.driver = {
+		.name = "rtc-ds3232",
+		.pm	= &ds3232_pm_ops,
+	},
+	.probe = ds3232_probe,
+	.remove = ds3232_remove,
+	.id_table = ds3232_id,
+};
+
+module_i2c_driver(ds3232_driver);
+
+MODULE_AUTHOR("Srikanth Srinivasan <srikanth.srinivasan@freescale.com>");
+MODULE_DESCRIPTION("Maxim/Dallas DS3232 RTC Driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/rtc/rtc-ds3232.c b/drivers/rtc/rtc-ds3232.c
deleted file mode 100644
index 4e99ace..0000000
--- a/drivers/rtc/rtc-ds3232.c
+++ /dev/null
@@ -1,517 +0,0 @@
-/*
- * RTC client/driver for the Maxim/Dallas DS3232 Real-Time Clock over I2C
- *
- * Copyright (C) 2009-2011 Freescale Semiconductor.
- * Author: Jack Lan <jack.lan@freescale.com>
- *
- * This program is free software; you can redistribute  it and/or modify it
- * under  the terms of  the GNU General  Public License as published by the
- * Free Software Foundation;  either version 2 of the  License, or (at your
- * option) any later version.
- */
-/*
- * It would be more efficient to use i2c msgs/i2c_transfer directly but, as
- * recommened in .../Documentation/i2c/writing-clients section
- * "Sending and receiving", using SMBus level communication is preferred.
- */
-
-#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
-
-#include <linux/kernel.h>
-#include <linux/module.h>
-#include <linux/interrupt.h>
-#include <linux/i2c.h>
-#include <linux/rtc.h>
-#include <linux/bcd.h>
-#include <linux/workqueue.h>
-#include <linux/slab.h>
-
-#define DS3232_REG_SECONDS	0x00
-#define DS3232_REG_MINUTES	0x01
-#define DS3232_REG_HOURS	0x02
-#define DS3232_REG_AMPM		0x02
-#define DS3232_REG_DAY		0x03
-#define DS3232_REG_DATE		0x04
-#define DS3232_REG_MONTH	0x05
-#define DS3232_REG_CENTURY	0x05
-#define DS3232_REG_YEAR		0x06
-#define DS3232_REG_ALARM1         0x07	/* Alarm 1 BASE */
-#define DS3232_REG_ALARM2         0x0B	/* Alarm 2 BASE */
-#define DS3232_REG_CR		0x0E	/* Control register */
-#	define DS3232_REG_CR_nEOSC        0x80
-#       define DS3232_REG_CR_INTCN        0x04
-#       define DS3232_REG_CR_A2IE        0x02
-#       define DS3232_REG_CR_A1IE        0x01
-
-#define DS3232_REG_SR	0x0F	/* control/status register */
-#	define DS3232_REG_SR_OSF   0x80
-#       define DS3232_REG_SR_BSY   0x04
-#       define DS3232_REG_SR_A2F   0x02
-#       define DS3232_REG_SR_A1F   0x01
-
-struct ds3232 {
-	struct i2c_client *client;
-	struct rtc_device *rtc;
-	struct work_struct work;
-
-	/* The mutex protects alarm operations, and prevents a race
-	 * between the enable_irq() in the workqueue and the free_irq()
-	 * in the remove function.
-	 */
-	struct mutex mutex;
-	bool suspended;
-	int exiting;
-};
-
-static struct i2c_driver ds3232_driver;
-
-static int ds3232_check_rtc_status(struct i2c_client *client)
-{
-	int ret = 0;
-	int control, stat;
-
-	stat = i2c_smbus_read_byte_data(client, DS3232_REG_SR);
-	if (stat < 0)
-		return stat;
-
-	if (stat & DS3232_REG_SR_OSF)
-		dev_warn(&client->dev,
-				"oscillator discontinuity flagged, "
-				"time unreliable\n");
-
-	stat &= ~(DS3232_REG_SR_OSF | DS3232_REG_SR_A1F | DS3232_REG_SR_A2F);
-
-	ret = i2c_smbus_write_byte_data(client, DS3232_REG_SR, stat);
-	if (ret < 0)
-		return ret;
-
-	/* If the alarm is pending, clear it before requesting
-	 * the interrupt, so an interrupt event isn't reported
-	 * before everything is initialized.
-	 */
-
-	control = i2c_smbus_read_byte_data(client, DS3232_REG_CR);
-	if (control < 0)
-		return control;
-
-	control &= ~(DS3232_REG_CR_A1IE | DS3232_REG_CR_A2IE);
-	control |= DS3232_REG_CR_INTCN;
-
-	return i2c_smbus_write_byte_data(client, DS3232_REG_CR, control);
-}
-
-static int ds3232_read_time(struct device *dev, struct rtc_time *time)
-{
-	struct i2c_client *client = to_i2c_client(dev);
-	int ret;
-	u8 buf[7];
-	unsigned int year, month, day, hour, minute, second;
-	unsigned int week, twelve_hr, am_pm;
-	unsigned int century, add_century = 0;
-
-	ret = i2c_smbus_read_i2c_block_data(client, DS3232_REG_SECONDS, 7, buf);
-
-	if (ret < 0)
-		return ret;
-	if (ret < 7)
-		return -EIO;
-
-	second = buf[0];
-	minute = buf[1];
-	hour = buf[2];
-	week = buf[3];
-	day = buf[4];
-	month = buf[5];
-	year = buf[6];
-
-	/* Extract additional information for AM/PM and century */
-
-	twelve_hr = hour & 0x40;
-	am_pm = hour & 0x20;
-	century = month & 0x80;
-
-	/* Write to rtc_time structure */
-
-	time->tm_sec = bcd2bin(second);
-	time->tm_min = bcd2bin(minute);
-	if (twelve_hr) {
-		/* Convert to 24 hr */
-		if (am_pm)
-			time->tm_hour = bcd2bin(hour & 0x1F) + 12;
-		else
-			time->tm_hour = bcd2bin(hour & 0x1F);
-	} else {
-		time->tm_hour = bcd2bin(hour);
-	}
-
-	/* Day of the week in linux range is 0~6 while 1~7 in RTC chip */
-	time->tm_wday = bcd2bin(week) - 1;
-	time->tm_mday = bcd2bin(day);
-	/* linux tm_mon range:0~11, while month range is 1~12 in RTC chip */
-	time->tm_mon = bcd2bin(month & 0x7F) - 1;
-	if (century)
-		add_century = 100;
-
-	time->tm_year = bcd2bin(year) + add_century;
-
-	return rtc_valid_tm(time);
-}
-
-static int ds3232_set_time(struct device *dev, struct rtc_time *time)
-{
-	struct i2c_client *client = to_i2c_client(dev);
-	u8 buf[7];
-
-	/* Extract time from rtc_time and load into ds3232*/
-
-	buf[0] = bin2bcd(time->tm_sec);
-	buf[1] = bin2bcd(time->tm_min);
-	buf[2] = bin2bcd(time->tm_hour);
-	/* Day of the week in linux range is 0~6 while 1~7 in RTC chip */
-	buf[3] = bin2bcd(time->tm_wday + 1);
-	buf[4] = bin2bcd(time->tm_mday); /* Date */
-	/* linux tm_mon range:0~11, while month range is 1~12 in RTC chip */
-	buf[5] = bin2bcd(time->tm_mon + 1);
-	if (time->tm_year >= 100) {
-		buf[5] |= 0x80;
-		buf[6] = bin2bcd(time->tm_year - 100);
-	} else {
-		buf[6] = bin2bcd(time->tm_year);
-	}
-
-	return i2c_smbus_write_i2c_block_data(client,
-					      DS3232_REG_SECONDS, 7, buf);
-}
-
-/*
- * DS3232 has two alarm, we only use alarm1
- * According to linux specification, only support one-shot alarm
- * no periodic alarm mode
- */
-static int ds3232_read_alarm(struct device *dev, struct rtc_wkalrm *alarm)
-{
-	struct i2c_client *client = to_i2c_client(dev);
-	struct ds3232 *ds3232 = i2c_get_clientdata(client);
-	int control, stat;
-	int ret;
-	u8 buf[4];
-
-	mutex_lock(&ds3232->mutex);
-
-	ret = i2c_smbus_read_byte_data(client, DS3232_REG_SR);
-	if (ret < 0)
-		goto out;
-	stat = ret;
-	ret = i2c_smbus_read_byte_data(client, DS3232_REG_CR);
-	if (ret < 0)
-		goto out;
-	control = ret;
-	ret = i2c_smbus_read_i2c_block_data(client, DS3232_REG_ALARM1, 4, buf);
-	if (ret < 0)
-		goto out;
-
-	alarm->time.tm_sec = bcd2bin(buf[0] & 0x7F);
-	alarm->time.tm_min = bcd2bin(buf[1] & 0x7F);
-	alarm->time.tm_hour = bcd2bin(buf[2] & 0x7F);
-	alarm->time.tm_mday = bcd2bin(buf[3] & 0x7F);
-
-	alarm->time.tm_mon = -1;
-	alarm->time.tm_year = -1;
-	alarm->time.tm_wday = -1;
-	alarm->time.tm_yday = -1;
-	alarm->time.tm_isdst = -1;
-
-	alarm->enabled = !!(control & DS3232_REG_CR_A1IE);
-	alarm->pending = !!(stat & DS3232_REG_SR_A1F);
-
-	ret = 0;
-out:
-	mutex_unlock(&ds3232->mutex);
-	return ret;
-}
-
-/*
- * linux rtc-module does not support wday alarm
- * and only 24h time mode supported indeed
- */
-static int ds3232_set_alarm(struct device *dev, struct rtc_wkalrm *alarm)
-{
-	struct i2c_client *client = to_i2c_client(dev);
-	struct ds3232 *ds3232 = i2c_get_clientdata(client);
-	int control, stat;
-	int ret;
-	u8 buf[4];
-
-	if (client->irq <= 0)
-		return -EINVAL;
-
-	mutex_lock(&ds3232->mutex);
-
-	buf[0] = bin2bcd(alarm->time.tm_sec);
-	buf[1] = bin2bcd(alarm->time.tm_min);
-	buf[2] = bin2bcd(alarm->time.tm_hour);
-	buf[3] = bin2bcd(alarm->time.tm_mday);
-
-	/* clear alarm interrupt enable bit */
-	ret = i2c_smbus_read_byte_data(client, DS3232_REG_CR);
-	if (ret < 0)
-		goto out;
-	control = ret;
-	control &= ~(DS3232_REG_CR_A1IE | DS3232_REG_CR_A2IE);
-	ret = i2c_smbus_write_byte_data(client, DS3232_REG_CR, control);
-	if (ret < 0)
-		goto out;
-
-	/* clear any pending alarm flag */
-	ret = i2c_smbus_read_byte_data(client, DS3232_REG_SR);
-	if (ret < 0)
-		goto out;
-	stat = ret;
-	stat &= ~(DS3232_REG_SR_A1F | DS3232_REG_SR_A2F);
-	ret = i2c_smbus_write_byte_data(client, DS3232_REG_SR, stat);
-	if (ret < 0)
-		goto out;
-
-	ret = i2c_smbus_write_i2c_block_data(client, DS3232_REG_ALARM1, 4, buf);
-
-	if (alarm->enabled) {
-		control |= DS3232_REG_CR_A1IE;
-		ret = i2c_smbus_write_byte_data(client, DS3232_REG_CR, control);
-	}
-out:
-	mutex_unlock(&ds3232->mutex);
-	return ret;
-}
-
-static void ds3232_update_alarm(struct i2c_client *client)
-{
-	struct ds3232 *ds3232 = i2c_get_clientdata(client);
-	int control;
-	int ret;
-	u8 buf[4];
-
-	mutex_lock(&ds3232->mutex);
-
-	ret = i2c_smbus_read_i2c_block_data(client, DS3232_REG_ALARM1, 4, buf);
-	if (ret < 0)
-		goto unlock;
-
-	buf[0] = bcd2bin(buf[0]) < 0 || (ds3232->rtc->irq_data & RTC_UF) ?
-								0x80 : buf[0];
-	buf[1] = bcd2bin(buf[1]) < 0 || (ds3232->rtc->irq_data & RTC_UF) ?
-								0x80 : buf[1];
-	buf[2] = bcd2bin(buf[2]) < 0 || (ds3232->rtc->irq_data & RTC_UF) ?
-								0x80 : buf[2];
-	buf[3] = bcd2bin(buf[3]) < 0 || (ds3232->rtc->irq_data & RTC_UF) ?
-								0x80 : buf[3];
-
-	ret = i2c_smbus_write_i2c_block_data(client, DS3232_REG_ALARM1, 4, buf);
-	if (ret < 0)
-		goto unlock;
-
-	control = i2c_smbus_read_byte_data(client, DS3232_REG_CR);
-	if (control < 0)
-		goto unlock;
-
-	if (ds3232->rtc->irq_data & (RTC_AF | RTC_UF))
-		/* enable alarm1 interrupt */
-		control |= DS3232_REG_CR_A1IE;
-	else
-		/* disable alarm1 interrupt */
-		control &= ~(DS3232_REG_CR_A1IE);
-	i2c_smbus_write_byte_data(client, DS3232_REG_CR, control);
-
-unlock:
-	mutex_unlock(&ds3232->mutex);
-}
-
-static int ds3232_alarm_irq_enable(struct device *dev, unsigned int enabled)
-{
-	struct i2c_client *client = to_i2c_client(dev);
-	struct ds3232 *ds3232 = i2c_get_clientdata(client);
-
-	if (client->irq <= 0)
-		return -EINVAL;
-
-	if (enabled)
-		ds3232->rtc->irq_data |= RTC_AF;
-	else
-		ds3232->rtc->irq_data &= ~RTC_AF;
-
-	ds3232_update_alarm(client);
-	return 0;
-}
-
-static irqreturn_t ds3232_irq(int irq, void *dev_id)
-{
-	struct i2c_client *client = dev_id;
-	struct ds3232 *ds3232 = i2c_get_clientdata(client);
-
-	disable_irq_nosync(irq);
-
-	/*
-	 * If rtc as a wakeup source, can't schedule the work
-	 * at system resume flow, because at this time the i2c bus
-	 * has not been resumed.
-	 */
-	if (!ds3232->suspended)
-		schedule_work(&ds3232->work);
-
-	return IRQ_HANDLED;
-}
-
-static void ds3232_work(struct work_struct *work)
-{
-	struct ds3232 *ds3232 = container_of(work, struct ds3232, work);
-	struct i2c_client *client = ds3232->client;
-	int stat, control;
-
-	mutex_lock(&ds3232->mutex);
-
-	stat = i2c_smbus_read_byte_data(client, DS3232_REG_SR);
-	if (stat < 0)
-		goto unlock;
-
-	if (stat & DS3232_REG_SR_A1F) {
-		control = i2c_smbus_read_byte_data(client, DS3232_REG_CR);
-		if (control < 0) {
-			pr_warn("Read Control Register error - Disable IRQ%d\n",
-				client->irq);
-		} else {
-			/* disable alarm1 interrupt */
-			control &= ~(DS3232_REG_CR_A1IE);
-			i2c_smbus_write_byte_data(client, DS3232_REG_CR,
-						control);
-
-			/* clear the alarm pend flag */
-			stat &= ~DS3232_REG_SR_A1F;
-			i2c_smbus_write_byte_data(client, DS3232_REG_SR, stat);
-
-			rtc_update_irq(ds3232->rtc, 1, RTC_AF | RTC_IRQF);
-
-			if (!ds3232->exiting)
-				enable_irq(client->irq);
-		}
-	}
-
-unlock:
-	mutex_unlock(&ds3232->mutex);
-}
-
-static const struct rtc_class_ops ds3232_rtc_ops = {
-	.read_time = ds3232_read_time,
-	.set_time = ds3232_set_time,
-	.read_alarm = ds3232_read_alarm,
-	.set_alarm = ds3232_set_alarm,
-	.alarm_irq_enable = ds3232_alarm_irq_enable,
-};
-
-static int ds3232_probe(struct i2c_client *client,
-			const struct i2c_device_id *id)
-{
-	struct ds3232 *ds3232;
-	int ret;
-
-	ds3232 = devm_kzalloc(&client->dev, sizeof(struct ds3232), GFP_KERNEL);
-	if (!ds3232)
-		return -ENOMEM;
-
-	ds3232->client = client;
-	i2c_set_clientdata(client, ds3232);
-
-	INIT_WORK(&ds3232->work, ds3232_work);
-	mutex_init(&ds3232->mutex);
-
-	ret = ds3232_check_rtc_status(client);
-	if (ret)
-		return ret;
-
-	if (client->irq > 0) {
-		ret = devm_request_irq(&client->dev, client->irq, ds3232_irq,
-				       IRQF_SHARED, "ds3232", client);
-		if (ret) {
-			dev_err(&client->dev, "unable to request IRQ\n");
-		}
-		device_init_wakeup(&client->dev, 1);
-	}
-	ds3232->rtc = devm_rtc_device_register(&client->dev, client->name,
-					  &ds3232_rtc_ops, THIS_MODULE);
-	return PTR_ERR_OR_ZERO(ds3232->rtc);
-}
-
-static int ds3232_remove(struct i2c_client *client)
-{
-	struct ds3232 *ds3232 = i2c_get_clientdata(client);
-
-	if (client->irq > 0) {
-		mutex_lock(&ds3232->mutex);
-		ds3232->exiting = 1;
-		mutex_unlock(&ds3232->mutex);
-
-		devm_free_irq(&client->dev, client->irq, client);
-		cancel_work_sync(&ds3232->work);
-	}
-
-	return 0;
-}
-
-#ifdef CONFIG_PM_SLEEP
-static int ds3232_suspend(struct device *dev)
-{
-	struct ds3232 *ds3232 = dev_get_drvdata(dev);
-	struct i2c_client *client = to_i2c_client(dev);
-
-	if (device_can_wakeup(dev)) {
-		ds3232->suspended = true;
-		if (irq_set_irq_wake(client->irq, 1)) {
-			dev_warn_once(dev, "Cannot set wakeup source\n");
-			ds3232->suspended = false;
-		}
-	}
-
-	return 0;
-}
-
-static int ds3232_resume(struct device *dev)
-{
-	struct ds3232 *ds3232 = dev_get_drvdata(dev);
-	struct i2c_client *client = to_i2c_client(dev);
-
-	if (ds3232->suspended) {
-		ds3232->suspended = false;
-
-		/* Clear the hardware alarm pend flag */
-		schedule_work(&ds3232->work);
-
-		irq_set_irq_wake(client->irq, 0);
-	}
-
-	return 0;
-}
-#endif
-
-static const struct dev_pm_ops ds3232_pm_ops = {
-	SET_SYSTEM_SLEEP_PM_OPS(ds3232_suspend, ds3232_resume)
-};
-
-static const struct i2c_device_id ds3232_id[] = {
-	{ "ds3232", 0 },
-	{ }
-};
-MODULE_DEVICE_TABLE(i2c, ds3232_id);
-
-static struct i2c_driver ds3232_driver = {
-	.driver = {
-		.name = "rtc-ds3232",
-		.pm	= &ds3232_pm_ops,
-	},
-	.probe = ds3232_probe,
-	.remove = ds3232_remove,
-	.id_table = ds3232_id,
-};
-
-module_i2c_driver(ds3232_driver);
-
-MODULE_AUTHOR("Srikanth Srinivasan <srikanth.srinivasan@freescale.com>");
-MODULE_DESCRIPTION("Maxim/Dallas DS3232 RTC Driver");
-MODULE_LICENSE("GPL");
-- 
2.5.0

-- 
-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[rtc-linux] [PATCH v3 2/8] rtc: ds3232: split into core module and i2c driver

[Akinobu Mita]

According to "Feature Comparison of the DS323x Real-Time Clocks"
(http://pdfserv.maximintegrated.com/en/an/AN5143.pdf), DS3232 and
DS3234 are very similar.

In order to share common code between rtc-ds3232 and rtc-ds3234,
this splits rtc-ds3232 driver into core module and i2c driver.

Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: Alexandre Belloni <alexandre.belloni@free-electrons.com>
Cc: Dennis Aberilla <denzzzhome@yahoo.com>
---
* v3
- convert to use regmap

 drivers/rtc/Kconfig           |   4 +
 drivers/rtc/Makefile          |   2 +-
 drivers/rtc/rtc-ds3232-core.c | 215 ++++++++++++++++++------------------------
 drivers/rtc/rtc-ds3232.c      |  63 +++++++++++++
 drivers/rtc/rtc-ds3232.h      |  23 +++++
 5 files changed, 185 insertions(+), 122 deletions(-)
 create mode 100644 drivers/rtc/rtc-ds3232.c
 create mode 100644 drivers/rtc/rtc-ds3232.h

diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index a9a2185..a489d89 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -139,6 +139,9 @@ config RTC_DRV_TEST
 	  This driver can also be built as a module. If so, the module
 	  will be called rtc-test.
 
+config RTC_DRV_DS3232_CORE
+	tristate
+
 comment "I2C RTC drivers"
 	depends on I2C
 
@@ -250,6 +253,7 @@ config RTC_DRV_DS1672
 
 config RTC_DRV_DS3232
 	tristate "Dallas/Maxim DS3232"
+	select RTC_DRV_DS3232_CORE
 	help
 	  If you say yes here you get support for Dallas Semiconductor
 	  DS3232 real-time clock chips. If an interrupt is associated
diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
index 0b8cb6c..f2d236f 100644
--- a/drivers/rtc/Makefile
+++ b/drivers/rtc/Makefile
@@ -58,8 +58,8 @@ obj-$(CONFIG_RTC_DRV_DS1672)	+= rtc-ds1672.o
 obj-$(CONFIG_RTC_DRV_DS1685_FAMILY)	+= rtc-ds1685.o
 obj-$(CONFIG_RTC_DRV_DS1742)	+= rtc-ds1742.o
 obj-$(CONFIG_RTC_DRV_DS2404)	+= rtc-ds2404.o
+obj-$(CONFIG_RTC_DRV_DS3232_CORE)	+= rtc-ds3232-core.o
 obj-$(CONFIG_RTC_DRV_DS3232)	+= rtc-ds3232.o
-rtc-ds3232-objs			:= rtc-ds3232-core.o
 obj-$(CONFIG_RTC_DRV_DS3234)	+= rtc-ds3234.o
 obj-$(CONFIG_RTC_DRV_EFI)	+= rtc-efi.o
 obj-$(CONFIG_RTC_DRV_EM3027)	+= rtc-em3027.o
diff --git a/drivers/rtc/rtc-ds3232-core.c b/drivers/rtc/rtc-ds3232-core.c
index 4e99ace..d44c84e 100644
--- a/drivers/rtc/rtc-ds3232-core.c
+++ b/drivers/rtc/rtc-ds3232-core.c
@@ -1,5 +1,5 @@
 /*
- * RTC client/driver for the Maxim/Dallas DS3232 Real-Time Clock over I2C
+ * RTC client/driver for the Maxim/Dallas DS3232/DS3234 Real-Time Clock
  *
  * Copyright (C) 2009-2011 Freescale Semiconductor.
  * Author: Jack Lan <jack.lan@freescale.com>
@@ -9,22 +9,19 @@
  * Free Software Foundation;  either version 2 of the  License, or (at your
  * option) any later version.
  */
-/*
- * It would be more efficient to use i2c msgs/i2c_transfer directly but, as
- * recommened in .../Documentation/i2c/writing-clients section
- * "Sending and receiving", using SMBus level communication is preferred.
- */
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/interrupt.h>
-#include <linux/i2c.h>
 #include <linux/rtc.h>
 #include <linux/bcd.h>
 #include <linux/workqueue.h>
 #include <linux/slab.h>
+#include <linux/regmap.h>
+
+#include "rtc-ds3232.h"
 
 #define DS3232_REG_SECONDS	0x00
 #define DS3232_REG_MINUTES	0x01
@@ -50,7 +47,9 @@
 #       define DS3232_REG_SR_A1F   0x01
 
 struct ds3232 {
-	struct i2c_client *client;
+	struct device *dev;
+	struct regmap *regmap;
+	int irq;
 	struct rtc_device *rtc;
 	struct work_struct work;
 
@@ -63,26 +62,25 @@ struct ds3232 {
 	int exiting;
 };
 
-static struct i2c_driver ds3232_driver;
-
-static int ds3232_check_rtc_status(struct i2c_client *client)
+static int ds3232_check_rtc_status(struct device *dev)
 {
+	struct ds3232 *ds3232 = dev_get_drvdata(dev);
 	int ret = 0;
 	int control, stat;
 
-	stat = i2c_smbus_read_byte_data(client, DS3232_REG_SR);
-	if (stat < 0)
-		return stat;
+	ret = regmap_read(ds3232->regmap, DS3232_REG_SR, &stat);
+	if (ret)
+		return ret;
 
 	if (stat & DS3232_REG_SR_OSF)
-		dev_warn(&client->dev,
+		dev_warn(dev,
 				"oscillator discontinuity flagged, "
 				"time unreliable\n");
 
 	stat &= ~(DS3232_REG_SR_OSF | DS3232_REG_SR_A1F | DS3232_REG_SR_A2F);
 
-	ret = i2c_smbus_write_byte_data(client, DS3232_REG_SR, stat);
-	if (ret < 0)
+	ret = regmap_write(ds3232->regmap, DS3232_REG_SR, stat);
+	if (ret)
 		return ret;
 
 	/* If the alarm is pending, clear it before requesting
@@ -90,31 +88,28 @@ static int ds3232_check_rtc_status(struct i2c_client *client)
 	 * before everything is initialized.
 	 */
 
-	control = i2c_smbus_read_byte_data(client, DS3232_REG_CR);
-	if (control < 0)
-		return control;
+	ret = regmap_read(ds3232->regmap, DS3232_REG_CR, &control);
+	if (ret)
+		return ret;
 
 	control &= ~(DS3232_REG_CR_A1IE | DS3232_REG_CR_A2IE);
 	control |= DS3232_REG_CR_INTCN;
 
-	return i2c_smbus_write_byte_data(client, DS3232_REG_CR, control);
+	return regmap_write(ds3232->regmap, DS3232_REG_CR, control);
 }
 
 static int ds3232_read_time(struct device *dev, struct rtc_time *time)
 {
-	struct i2c_client *client = to_i2c_client(dev);
+	struct ds3232 *ds3232 = dev_get_drvdata(dev);
 	int ret;
 	u8 buf[7];
 	unsigned int year, month, day, hour, minute, second;
 	unsigned int week, twelve_hr, am_pm;
 	unsigned int century, add_century = 0;
 
-	ret = i2c_smbus_read_i2c_block_data(client, DS3232_REG_SECONDS, 7, buf);
-
-	if (ret < 0)
+	ret = regmap_bulk_read(ds3232->regmap, DS3232_REG_SECONDS, buf, 7);
+	if (ret)
 		return ret;
-	if (ret < 7)
-		return -EIO;
 
 	second = buf[0];
 	minute = buf[1];
@@ -159,7 +154,7 @@ static int ds3232_read_time(struct device *dev, struct rtc_time *time)
 
 static int ds3232_set_time(struct device *dev, struct rtc_time *time)
 {
-	struct i2c_client *client = to_i2c_client(dev);
+	struct ds3232 *ds3232 = dev_get_drvdata(dev);
 	u8 buf[7];
 
 	/* Extract time from rtc_time and load into ds3232*/
@@ -179,8 +174,7 @@ static int ds3232_set_time(struct device *dev, struct rtc_time *time)
 		buf[6] = bin2bcd(time->tm_year);
 	}
 
-	return i2c_smbus_write_i2c_block_data(client,
-					      DS3232_REG_SECONDS, 7, buf);
+	return regmap_bulk_write(ds3232->regmap, DS3232_REG_SECONDS, buf, 7);
 }
 
 /*
@@ -190,24 +184,21 @@ static int ds3232_set_time(struct device *dev, struct rtc_time *time)
  */
 static int ds3232_read_alarm(struct device *dev, struct rtc_wkalrm *alarm)
 {
-	struct i2c_client *client = to_i2c_client(dev);
-	struct ds3232 *ds3232 = i2c_get_clientdata(client);
+	struct ds3232 *ds3232 = dev_get_drvdata(dev);
 	int control, stat;
 	int ret;
 	u8 buf[4];
 
 	mutex_lock(&ds3232->mutex);
 
-	ret = i2c_smbus_read_byte_data(client, DS3232_REG_SR);
-	if (ret < 0)
+	ret = regmap_read(ds3232->regmap, DS3232_REG_SR, &stat);
+	if (ret)
 		goto out;
-	stat = ret;
-	ret = i2c_smbus_read_byte_data(client, DS3232_REG_CR);
-	if (ret < 0)
+	ret = regmap_read(ds3232->regmap, DS3232_REG_CR, &control);
+	if (ret)
 		goto out;
-	control = ret;
-	ret = i2c_smbus_read_i2c_block_data(client, DS3232_REG_ALARM1, 4, buf);
-	if (ret < 0)
+	ret = regmap_bulk_read(ds3232->regmap, DS3232_REG_ALARM1, buf, 4);
+	if (ret)
 		goto out;
 
 	alarm->time.tm_sec = bcd2bin(buf[0] & 0x7F);
@@ -236,13 +227,12 @@ out:
  */
 static int ds3232_set_alarm(struct device *dev, struct rtc_wkalrm *alarm)
 {
-	struct i2c_client *client = to_i2c_client(dev);
-	struct ds3232 *ds3232 = i2c_get_clientdata(client);
+	struct ds3232 *ds3232 = dev_get_drvdata(dev);
 	int control, stat;
 	int ret;
 	u8 buf[4];
 
-	if (client->irq <= 0)
+	if (ds3232->irq <= 0)
 		return -EINVAL;
 
 	mutex_lock(&ds3232->mutex);
@@ -253,47 +243,45 @@ static int ds3232_set_alarm(struct device *dev, struct rtc_wkalrm *alarm)
 	buf[3] = bin2bcd(alarm->time.tm_mday);
 
 	/* clear alarm interrupt enable bit */
-	ret = i2c_smbus_read_byte_data(client, DS3232_REG_CR);
-	if (ret < 0)
+	ret = regmap_read(ds3232->regmap, DS3232_REG_CR, &control);
+	if (ret)
 		goto out;
-	control = ret;
 	control &= ~(DS3232_REG_CR_A1IE | DS3232_REG_CR_A2IE);
-	ret = i2c_smbus_write_byte_data(client, DS3232_REG_CR, control);
-	if (ret < 0)
+	ret = regmap_write(ds3232->regmap, DS3232_REG_CR, control);
+	if (ret)
 		goto out;
 
 	/* clear any pending alarm flag */
-	ret = i2c_smbus_read_byte_data(client, DS3232_REG_SR);
-	if (ret < 0)
+	ret = regmap_read(ds3232->regmap, DS3232_REG_SR, &stat);
+	if (ret)
 		goto out;
-	stat = ret;
 	stat &= ~(DS3232_REG_SR_A1F | DS3232_REG_SR_A2F);
-	ret = i2c_smbus_write_byte_data(client, DS3232_REG_SR, stat);
-	if (ret < 0)
+	ret = regmap_write(ds3232->regmap, DS3232_REG_SR, stat);
+	if (ret)
 		goto out;
 
-	ret = i2c_smbus_write_i2c_block_data(client, DS3232_REG_ALARM1, 4, buf);
+	ret = regmap_bulk_write(ds3232->regmap, DS3232_REG_ALARM1, buf, 4);
 
 	if (alarm->enabled) {
 		control |= DS3232_REG_CR_A1IE;
-		ret = i2c_smbus_write_byte_data(client, DS3232_REG_CR, control);
+		ret = regmap_write(ds3232->regmap, DS3232_REG_CR, control);
 	}
 out:
 	mutex_unlock(&ds3232->mutex);
 	return ret;
 }
 
-static void ds3232_update_alarm(struct i2c_client *client)
+static void ds3232_update_alarm(struct device *dev)
 {
-	struct ds3232 *ds3232 = i2c_get_clientdata(client);
+	struct ds3232 *ds3232 = dev_get_drvdata(dev);
 	int control;
 	int ret;
 	u8 buf[4];
 
 	mutex_lock(&ds3232->mutex);
 
-	ret = i2c_smbus_read_i2c_block_data(client, DS3232_REG_ALARM1, 4, buf);
-	if (ret < 0)
+	ret = regmap_bulk_read(ds3232->regmap, DS3232_REG_ALARM1, buf, 4);
+	if (ret)
 		goto unlock;
 
 	buf[0] = bcd2bin(buf[0]) < 0 || (ds3232->rtc->irq_data & RTC_UF) ?
@@ -305,12 +293,12 @@ static void ds3232_update_alarm(struct i2c_client *client)
 	buf[3] = bcd2bin(buf[3]) < 0 || (ds3232->rtc->irq_data & RTC_UF) ?
 								0x80 : buf[3];
 
-	ret = i2c_smbus_write_i2c_block_data(client, DS3232_REG_ALARM1, 4, buf);
-	if (ret < 0)
+	ret = regmap_bulk_write(ds3232->regmap, DS3232_REG_ALARM1, buf, 4);
+	if (ret)
 		goto unlock;
 
-	control = i2c_smbus_read_byte_data(client, DS3232_REG_CR);
-	if (control < 0)
+	ret = regmap_read(ds3232->regmap, DS3232_REG_CR, &control);
+	if (ret)
 		goto unlock;
 
 	if (ds3232->rtc->irq_data & (RTC_AF | RTC_UF))
@@ -319,7 +307,7 @@ static void ds3232_update_alarm(struct i2c_client *client)
 	else
 		/* disable alarm1 interrupt */
 		control &= ~(DS3232_REG_CR_A1IE);
-	i2c_smbus_write_byte_data(client, DS3232_REG_CR, control);
+	regmap_write(ds3232->regmap, DS3232_REG_CR, control);
 
 unlock:
 	mutex_unlock(&ds3232->mutex);
@@ -327,10 +315,9 @@ unlock:
 
 static int ds3232_alarm_irq_enable(struct device *dev, unsigned int enabled)
 {
-	struct i2c_client *client = to_i2c_client(dev);
-	struct ds3232 *ds3232 = i2c_get_clientdata(client);
+	struct ds3232 *ds3232 = dev_get_drvdata(dev);
 
-	if (client->irq <= 0)
+	if (ds3232->irq <= 0)
 		return -EINVAL;
 
 	if (enabled)
@@ -338,14 +325,14 @@ static int ds3232_alarm_irq_enable(struct device *dev, unsigned int enabled)
 	else
 		ds3232->rtc->irq_data &= ~RTC_AF;
 
-	ds3232_update_alarm(client);
+	ds3232_update_alarm(dev);
 	return 0;
 }
 
 static irqreturn_t ds3232_irq(int irq, void *dev_id)
 {
-	struct i2c_client *client = dev_id;
-	struct ds3232 *ds3232 = i2c_get_clientdata(client);
+	struct device *dev = dev_id;
+	struct ds3232 *ds3232 = dev_get_drvdata(dev);
 
 	disable_irq_nosync(irq);
 
@@ -363,34 +350,33 @@ static irqreturn_t ds3232_irq(int irq, void *dev_id)
 static void ds3232_work(struct work_struct *work)
 {
 	struct ds3232 *ds3232 = container_of(work, struct ds3232, work);
-	struct i2c_client *client = ds3232->client;
+	int ret;
 	int stat, control;
 
 	mutex_lock(&ds3232->mutex);
 
-	stat = i2c_smbus_read_byte_data(client, DS3232_REG_SR);
-	if (stat < 0)
+	ret = regmap_read(ds3232->regmap, DS3232_REG_SR, &stat);
+	if (ret)
 		goto unlock;
 
 	if (stat & DS3232_REG_SR_A1F) {
-		control = i2c_smbus_read_byte_data(client, DS3232_REG_CR);
-		if (control < 0) {
+		ret = regmap_read(ds3232->regmap, DS3232_REG_CR, &control);
+		if (ret) {
 			pr_warn("Read Control Register error - Disable IRQ%d\n",
-				client->irq);
+				ds3232->irq);
 		} else {
 			/* disable alarm1 interrupt */
 			control &= ~(DS3232_REG_CR_A1IE);
-			i2c_smbus_write_byte_data(client, DS3232_REG_CR,
-						control);
+			regmap_write(ds3232->regmap, DS3232_REG_CR, control);
 
 			/* clear the alarm pend flag */
 			stat &= ~DS3232_REG_SR_A1F;
-			i2c_smbus_write_byte_data(client, DS3232_REG_SR, stat);
+			regmap_write(ds3232->regmap, DS3232_REG_SR, stat);
 
 			rtc_update_irq(ds3232->rtc, 1, RTC_AF | RTC_IRQF);
 
 			if (!ds3232->exiting)
-				enable_irq(client->irq);
+				enable_irq(ds3232->irq);
 		}
 	}
 
@@ -406,64 +392,69 @@ static const struct rtc_class_ops ds3232_rtc_ops = {
 	.alarm_irq_enable = ds3232_alarm_irq_enable,
 };
 
-static int ds3232_probe(struct i2c_client *client,
-			const struct i2c_device_id *id)
+int __ds3232_probe(struct device *dev, struct regmap *regmap, int irq,
+		const char *name, struct module *owner)
 {
 	struct ds3232 *ds3232;
 	int ret;
 
-	ds3232 = devm_kzalloc(&client->dev, sizeof(struct ds3232), GFP_KERNEL);
+	ds3232 = devm_kzalloc(dev, sizeof(*ds3232), GFP_KERNEL);
 	if (!ds3232)
 		return -ENOMEM;
 
-	ds3232->client = client;
-	i2c_set_clientdata(client, ds3232);
+	ds3232->regmap = regmap;
+	ds3232->irq = irq;
+	ds3232->dev = dev;
+	dev_set_drvdata(dev, ds3232);
 
 	INIT_WORK(&ds3232->work, ds3232_work);
 	mutex_init(&ds3232->mutex);
 
-	ret = ds3232_check_rtc_status(client);
+	ret = ds3232_check_rtc_status(dev);
 	if (ret)
 		return ret;
 
-	if (client->irq > 0) {
-		ret = devm_request_irq(&client->dev, client->irq, ds3232_irq,
-				       IRQF_SHARED, "ds3232", client);
+	if (ds3232->irq > 0) {
+		ret = devm_request_irq(dev, ds3232->irq, ds3232_irq,
+				       IRQF_SHARED, name, dev);
 		if (ret) {
-			dev_err(&client->dev, "unable to request IRQ\n");
-		}
-		device_init_wakeup(&client->dev, 1);
+			ds3232->irq = 0;
+			dev_err(dev, "unable to request IRQ\n");
+		} else
+			device_init_wakeup(dev, 1);
 	}
-	ds3232->rtc = devm_rtc_device_register(&client->dev, client->name,
-					  &ds3232_rtc_ops, THIS_MODULE);
+	ds3232->rtc = devm_rtc_device_register(dev, name, &ds3232_rtc_ops,
+						owner);
+
 	return PTR_ERR_OR_ZERO(ds3232->rtc);
 }
+EXPORT_SYMBOL_GPL(__ds3232_probe);
 
-static int ds3232_remove(struct i2c_client *client)
+int ds3232_remove(struct device *dev)
 {
-	struct ds3232 *ds3232 = i2c_get_clientdata(client);
+	struct ds3232 *ds3232 = dev_get_drvdata(dev);
 
-	if (client->irq > 0) {
+	if (ds3232->irq > 0) {
 		mutex_lock(&ds3232->mutex);
 		ds3232->exiting = 1;
 		mutex_unlock(&ds3232->mutex);
 
-		devm_free_irq(&client->dev, client->irq, client);
+		devm_free_irq(dev, ds3232->irq, dev);
 		cancel_work_sync(&ds3232->work);
 	}
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(ds3232_remove);
 
 #ifdef CONFIG_PM_SLEEP
 static int ds3232_suspend(struct device *dev)
 {
 	struct ds3232 *ds3232 = dev_get_drvdata(dev);
-	struct i2c_client *client = to_i2c_client(dev);
 
 	if (device_can_wakeup(dev)) {
 		ds3232->suspended = true;
-		if (irq_set_irq_wake(client->irq, 1)) {
+		if (irq_set_irq_wake(ds3232->irq, 1)) {
 			dev_warn_once(dev, "Cannot set wakeup source\n");
 			ds3232->suspended = false;
 		}
@@ -475,7 +466,6 @@ static int ds3232_suspend(struct device *dev)
 static int ds3232_resume(struct device *dev)
 {
 	struct ds3232 *ds3232 = dev_get_drvdata(dev);
-	struct i2c_client *client = to_i2c_client(dev);
 
 	if (ds3232->suspended) {
 		ds3232->suspended = false;
@@ -483,34 +473,17 @@ static int ds3232_resume(struct device *dev)
 		/* Clear the hardware alarm pend flag */
 		schedule_work(&ds3232->work);
 
-		irq_set_irq_wake(client->irq, 0);
+		irq_set_irq_wake(ds3232->irq, 0);
 	}
 
 	return 0;
 }
 #endif
 
-static const struct dev_pm_ops ds3232_pm_ops = {
+const struct dev_pm_ops ds3232_pm_ops = {
 	SET_SYSTEM_SLEEP_PM_OPS(ds3232_suspend, ds3232_resume)
 };
-
-static const struct i2c_device_id ds3232_id[] = {
-	{ "ds3232", 0 },
-	{ }
-};
-MODULE_DEVICE_TABLE(i2c, ds3232_id);
-
-static struct i2c_driver ds3232_driver = {
-	.driver = {
-		.name = "rtc-ds3232",
-		.pm	= &ds3232_pm_ops,
-	},
-	.probe = ds3232_probe,
-	.remove = ds3232_remove,
-	.id_table = ds3232_id,
-};
-
-module_i2c_driver(ds3232_driver);
+EXPORT_SYMBOL_GPL(ds3232_pm_ops);
 
 MODULE_AUTHOR("Srikanth Srinivasan <srikanth.srinivasan@freescale.com>");
 MODULE_DESCRIPTION("Maxim/Dallas DS3232 RTC Driver");
diff --git a/drivers/rtc/rtc-ds3232.c b/drivers/rtc/rtc-ds3232.c
new file mode 100644
index 0000000..72e6805
--- /dev/null
+++ b/drivers/rtc/rtc-ds3232.c
@@ -0,0 +1,63 @@
+/*
+ * RTC client/driver for the Maxim/Dallas DS3232 Real-Time Clock over I2C
+ *
+ * Copyright (C) 2009-2011 Freescale Semiconductor.
+ * Author: Jack Lan <jack.lan@freescale.com>
+ *
+ * This program is free software; you can redistribute  it and/or modify it
+ * under  the terms of  the GNU General  Public License as published by the
+ * Free Software Foundation;  either version 2 of the  License, or (at your
+ * option) any later version.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/i2c.h>
+#include <linux/regmap.h>
+
+#include "rtc-ds3232.h"
+
+static int ds3232_i2c_probe(struct i2c_client *client,
+			const struct i2c_device_id *id)
+{
+	struct regmap *regmap;
+	static const struct regmap_config config = {
+		.reg_bits = 8,
+		.val_bits = 8,
+	};
+
+	regmap = devm_regmap_init_i2c(client, &config);
+	if (IS_ERR(regmap)) {
+		dev_err(&client->dev, "%s: regmap allocation failed: %ld\n",
+			__func__, PTR_ERR(regmap));
+		return PTR_ERR(regmap);
+	}
+
+	return ds3232_probe(&client->dev, regmap, client->irq, client->name);
+}
+
+static int ds3232_i2c_remove(struct i2c_client *client)
+{
+	return ds3232_remove(&client->dev);
+}
+
+static const struct i2c_device_id ds3232_id[] = {
+	{ "ds3232", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, ds3232_id);
+
+static struct i2c_driver ds3232_driver = {
+	.driver = {
+		.name = "rtc-ds3232",
+		.pm	= &ds3232_pm_ops,
+	},
+	.probe = ds3232_i2c_probe,
+	.remove = ds3232_i2c_remove,
+	.id_table = ds3232_id,
+};
+module_i2c_driver(ds3232_driver);
+
+MODULE_AUTHOR("Srikanth Srinivasan <srikanth.srinivasan@freescale.com>");
+MODULE_DESCRIPTION("Maxim/Dallas DS3232 RTC Driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/rtc/rtc-ds3232.h b/drivers/rtc/rtc-ds3232.h
new file mode 100644
index 0000000..94bec04
--- /dev/null
+++ b/drivers/rtc/rtc-ds3232.h
@@ -0,0 +1,23 @@
+/*
+ * RTC client/driver for the Maxim/Dallas DS3232/DS3234 Real-Time Clock
+ *
+ * Copyright (C) 2009-2011 Freescale Semiconductor.
+ * Author: Jack Lan <jack.lan@freescale.com>
+ *
+ * This program is free software; you can redistribute  it and/or modify it
+ * under  the terms of  the GNU General  Public License as published by the
+ * Free Software Foundation;  either version 2 of the  License, or (at your
+ * option) any later version.
+ */
+#ifndef __RTC_DS3232_H__
+#define __RTC_DS3232_H__
+
+int __ds3232_probe(struct device *dev, struct regmap *regmap, int irq,
+		const char *name, struct module *owner);
+#define ds3232_probe(dev, regmap, irq, name)	\
+	__ds3232_probe(dev, regmap, irq, name, THIS_MODULE)
+
+int ds3232_remove(struct device *dev);
+extern const struct dev_pm_ops ds3232_pm_ops;
+
+#endif
-- 
2.5.0

-- 
-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[rtc-linux] [PATCH v3 3/8] rtc: ds3234: use rtc-ds3232 core and support alarm

[Akinobu Mita]

Use rtc-ds3232 core module.  This change enables to
support alarm for rtc-ds3234.

Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: Alexandre Belloni <alexandre.belloni@free-electrons.com>
Cc: Dennis Aberilla <denzzzhome@yahoo.com>
---
* v3
- convert to use regmap

 drivers/rtc/Kconfig      |   1 +
 drivers/rtc/rtc-ds3234.c | 141 ++++++++++++++---------------------------------
 2 files changed, 43 insertions(+), 99 deletions(-)

diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index a489d89..cb27ace 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -733,6 +733,7 @@ config RTC_DRV_RS5C348
 
 config RTC_DRV_DS3234
 	tristate "Maxim/Dallas DS3234"
+	select RTC_DRV_DS3232_CORE
 	help
 	  If you say yes here you get support for the
 	  Maxim/Dallas DS3234 SPI RTC chip.
diff --git a/drivers/rtc/rtc-ds3234.c b/drivers/rtc/rtc-ds3234.c
index 570ab28..fb92919 100644
--- a/drivers/rtc/rtc-ds3234.c
+++ b/drivers/rtc/rtc-ds3234.c
@@ -18,105 +18,37 @@
 #include <linux/rtc.h>
 #include <linux/spi/spi.h>
 #include <linux/bcd.h>
+#include <linux/regmap.h>
 
-#define DS3234_REG_SECONDS	0x00
-#define DS3234_REG_MINUTES	0x01
-#define DS3234_REG_HOURS	0x02
-#define DS3234_REG_DAY		0x03
-#define DS3234_REG_DATE		0x04
-#define DS3234_REG_MONTH	0x05
-#define DS3234_REG_YEAR		0x06
-#define DS3234_REG_CENTURY	(1 << 7) /* Bit 7 of the Month register */
+#include "rtc-ds3232.h"
 
 #define DS3234_REG_CONTROL	0x0E
 #define DS3234_REG_CONT_STAT	0x0F
 
-static int ds3234_set_reg(struct device *dev, unsigned char address,
-				unsigned char data)
-{
-	struct spi_device *spi = to_spi_device(dev);
-	unsigned char buf[2];
-
-	/* MSB must be '1' to indicate write */
-	buf[0] = address | 0x80;
-	buf[1] = data;
-
-	return spi_write_then_read(spi, buf, 2, NULL, 0);
-}
-
-static int ds3234_get_reg(struct device *dev, unsigned char address,
-				unsigned char *data)
-{
-	struct spi_device *spi = to_spi_device(dev);
-
-	*data = address & 0x7f;
-
-	return spi_write_then_read(spi, data, 1, data, 1);
-}
-
-static int ds3234_read_time(struct device *dev, struct rtc_time *dt)
-{
-	int err;
-	unsigned char buf[8];
-	struct spi_device *spi = to_spi_device(dev);
-
-	buf[0] = 0x00; /* Start address */
-
-	err = spi_write_then_read(spi, buf, 1, buf, 8);
-	if (err != 0)
-		return err;
-
-	/* Seconds, Minutes, Hours, Day, Date, Month, Year */
-	dt->tm_sec	= bcd2bin(buf[0]);
-	dt->tm_min	= bcd2bin(buf[1]);
-	dt->tm_hour	= bcd2bin(buf[2] & 0x3f);
-	dt->tm_wday	= bcd2bin(buf[3]) - 1; /* 0 = Sun */
-	dt->tm_mday	= bcd2bin(buf[4]);
-	dt->tm_mon	= bcd2bin(buf[5] & 0x1f) - 1; /* 0 = Jan */
-	dt->tm_year	= bcd2bin(buf[6] & 0xff) + 100; /* Assume 20YY */
-
-	return rtc_valid_tm(dt);
-}
-
-static int ds3234_set_time(struct device *dev, struct rtc_time *dt)
-{
-	ds3234_set_reg(dev, DS3234_REG_SECONDS, bin2bcd(dt->tm_sec));
-	ds3234_set_reg(dev, DS3234_REG_MINUTES, bin2bcd(dt->tm_min));
-	ds3234_set_reg(dev, DS3234_REG_HOURS, bin2bcd(dt->tm_hour) & 0x3f);
-
-	/* 0 = Sun */
-	ds3234_set_reg(dev, DS3234_REG_DAY, bin2bcd(dt->tm_wday + 1));
-	ds3234_set_reg(dev, DS3234_REG_DATE, bin2bcd(dt->tm_mday));
-
-	/* 0 = Jan */
-	ds3234_set_reg(dev, DS3234_REG_MONTH, bin2bcd(dt->tm_mon + 1));
-
-	/* Assume 20YY although we just want to make sure not to go negative. */
-	if (dt->tm_year > 100)
-		dt->tm_year -= 100;
-
-	ds3234_set_reg(dev, DS3234_REG_YEAR, bin2bcd(dt->tm_year));
-
-	return 0;
-}
-
-static const struct rtc_class_ops ds3234_rtc_ops = {
-	.read_time	= ds3234_read_time,
-	.set_time	= ds3234_set_time,
-};
-
 static int ds3234_probe(struct spi_device *spi)
 {
-	struct rtc_device *rtc;
-	unsigned char tmp;
 	int res;
+	unsigned int tmp;
+	static const struct regmap_config config = {
+		.reg_bits = 8,
+		.val_bits = 8,
+		.write_flag_mask = 0x80,
+	};
+	struct regmap *regmap;
+
+	regmap = devm_regmap_init_spi(spi, &config);
+	if (IS_ERR(regmap)) {
+		dev_err(&spi->dev, "%s: regmap allocation failed: %ld\n",
+			__func__, PTR_ERR(regmap));
+		return PTR_ERR(regmap);
+	}
 
 	spi->mode = SPI_MODE_3;
 	spi->bits_per_word = 8;
 	spi_setup(spi);
 
-	res = ds3234_get_reg(&spi->dev, DS3234_REG_SECONDS, &tmp);
-	if (res != 0)
+	res = regmap_read(regmap, 0, &tmp);
+	if (res)
 		return res;
 
 	/* Control settings
@@ -133,27 +65,38 @@ static int ds3234_probe(struct spi_device *spi)
 	 *
 	 *     1	0	0	0	1	0	0	0
 	 */
-	ds3234_get_reg(&spi->dev, DS3234_REG_CONTROL, &tmp);
-	ds3234_set_reg(&spi->dev, DS3234_REG_CONTROL, tmp & 0x1c);
+	res = regmap_read(regmap, DS3234_REG_CONTROL, &tmp);
+	if (res)
+		return res;
+	res = regmap_write(regmap, DS3234_REG_CONTROL, tmp & 0x1c);
+	if (res)
+		return res;
 
-	ds3234_get_reg(&spi->dev, DS3234_REG_CONT_STAT, &tmp);
-	ds3234_set_reg(&spi->dev, DS3234_REG_CONT_STAT, tmp & 0x88);
+	res = regmap_read(regmap, DS3234_REG_CONT_STAT, &tmp);
+	if (res)
+		return res;
+	res = regmap_write(regmap, DS3234_REG_CONT_STAT, tmp & 0x88);
+	if (res)
+		return res;
 
 	/* Print our settings */
-	ds3234_get_reg(&spi->dev, DS3234_REG_CONTROL, &tmp);
+	res = regmap_read(regmap, DS3234_REG_CONTROL, &tmp);
+	if (res)
+		return res;
 	dev_info(&spi->dev, "Control Reg: 0x%02x\n", tmp);
 
-	ds3234_get_reg(&spi->dev, DS3234_REG_CONT_STAT, &tmp);
+	res = regmap_read(regmap, DS3234_REG_CONT_STAT, &tmp);
+	if (res)
+		return res;
 	dev_info(&spi->dev, "Ctrl/Stat Reg: 0x%02x\n", tmp);
 
-	rtc = devm_rtc_device_register(&spi->dev, "ds3234",
-				&ds3234_rtc_ops, THIS_MODULE);
-	if (IS_ERR(rtc))
-		return PTR_ERR(rtc);
+	return ds3232_probe(&spi->dev, regmap, spi->irq, "ds3234");
 
-	spi_set_drvdata(spi, rtc);
+}
 
-	return 0;
+static int ds3234_remove(struct spi_device *spi)
+{
+	return ds3232_remove(&spi->dev);
 }
 
 static struct spi_driver ds3234_driver = {
@@ -161,8 +104,8 @@ static struct spi_driver ds3234_driver = {
 		.name	 = "ds3234",
 	},
 	.probe	 = ds3234_probe,
+	.remove = ds3234_remove,
 };
-
 module_spi_driver(ds3234_driver);
 
 MODULE_DESCRIPTION("DS3234 SPI RTC driver");
-- 
2.5.0

-- 
-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[rtc-linux] [PATCH v3 4/8] rtc: ds3232-core: fix read on /dev/rtc after RTC_AIE_ON

[Akinobu Mita]

The rtctest (tools/testing/selftests/timers/rtctest.c) found that
reading ds3232 rtc device immediately return the value 0x20 (RTC_AF)
without waiting alarm interrupt.

This is because alarm_irq_enable() of ds3232 driver changes RTC_AF
flag in rtc->irq_data.  So calling ioctl with RTC_AIE_ON generates
invalid value in rtc device.

The lower-level driver should not touch rtc->irq_data directly.

Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: Alexandre Belloni <alexandre.belloni@free-electrons.com>
Cc: Dennis Aberilla <denzzzhome@yahoo.com>
---
* No changes since v1

 drivers/rtc/rtc-ds3232-core.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/rtc/rtc-ds3232-core.c b/drivers/rtc/rtc-ds3232-core.c
index d44c84e..7de30b3 100644
--- a/drivers/rtc/rtc-ds3232-core.c
+++ b/drivers/rtc/rtc-ds3232-core.c
@@ -271,7 +271,7 @@ out:
 	return ret;
 }
 
-static void ds3232_update_alarm(struct device *dev)
+static void ds3232_update_alarm(struct device *dev, unsigned int enabled)
 {
 	struct ds3232 *ds3232 = dev_get_drvdata(dev);
 	int control;
@@ -301,7 +301,7 @@ static void ds3232_update_alarm(struct device *dev)
 	if (ret)
 		goto unlock;
 
-	if (ds3232->rtc->irq_data & (RTC_AF | RTC_UF))
+	if (enabled || (ds3232->rtc->irq_data & RTC_UF))
 		/* enable alarm1 interrupt */
 		control |= DS3232_REG_CR_A1IE;
 	else
@@ -320,12 +320,8 @@ static int ds3232_alarm_irq_enable(struct device *dev, unsigned int enabled)
 	if (ds3232->irq <= 0)
 		return -EINVAL;
 
-	if (enabled)
-		ds3232->rtc->irq_data |= RTC_AF;
-	else
-		ds3232->rtc->irq_data &= ~RTC_AF;
+	ds3232_update_alarm(dev, enabled);
 
-	ds3232_update_alarm(dev);
 	return 0;
 }
 
-- 
2.5.0

-- 
-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[rtc-linux] [PATCH v3 5/8] rtc: ds3232-core: add register access error checks

[Akinobu Mita]

Add missing register access error checks and make it return error code
or print error message.

Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: Alexandre Belloni <alexandre.belloni@free-electrons.com>
Cc: Dennis Aberilla <denzzzhome@yahoo.com>
---
* New patch from this version

 drivers/rtc/rtc-ds3232-core.c | 29 ++++++++++++++++++++++-------
 1 file changed, 22 insertions(+), 7 deletions(-)

diff --git a/drivers/rtc/rtc-ds3232-core.c b/drivers/rtc/rtc-ds3232-core.c
index 7de30b3..bcce75b 100644
--- a/drivers/rtc/rtc-ds3232-core.c
+++ b/drivers/rtc/rtc-ds3232-core.c
@@ -261,6 +261,8 @@ static int ds3232_set_alarm(struct device *dev, struct rtc_wkalrm *alarm)
 		goto out;
 
 	ret = regmap_bulk_write(ds3232->regmap, DS3232_REG_ALARM1, buf, 4);
+	if (ret)
+		goto out;
 
 	if (alarm->enabled) {
 		control |= DS3232_REG_CR_A1IE;
@@ -271,7 +273,7 @@ out:
 	return ret;
 }
 
-static void ds3232_update_alarm(struct device *dev, unsigned int enabled)
+static int ds3232_update_alarm(struct device *dev, unsigned int enabled)
 {
 	struct ds3232 *ds3232 = dev_get_drvdata(dev);
 	int control;
@@ -307,10 +309,12 @@ static void ds3232_update_alarm(struct device *dev, unsigned int enabled)
 	else
 		/* disable alarm1 interrupt */
 		control &= ~(DS3232_REG_CR_A1IE);
-	regmap_write(ds3232->regmap, DS3232_REG_CR, control);
+	ret = regmap_write(ds3232->regmap, DS3232_REG_CR, control);
 
 unlock:
 	mutex_unlock(&ds3232->mutex);
+
+	return ret;
 }
 
 static int ds3232_alarm_irq_enable(struct device *dev, unsigned int enabled)
@@ -320,9 +324,7 @@ static int ds3232_alarm_irq_enable(struct device *dev, unsigned int enabled)
 	if (ds3232->irq <= 0)
 		return -EINVAL;
 
-	ds3232_update_alarm(dev, enabled);
-
-	return 0;
+	return ds3232_update_alarm(dev, enabled);
 }
 
 static irqreturn_t ds3232_irq(int irq, void *dev_id)
@@ -363,11 +365,24 @@ static void ds3232_work(struct work_struct *work)
 		} else {
 			/* disable alarm1 interrupt */
 			control &= ~(DS3232_REG_CR_A1IE);
-			regmap_write(ds3232->regmap, DS3232_REG_CR, control);
+			ret = regmap_write(ds3232->regmap, DS3232_REG_CR,
+						control);
+			if (ret) {
+				dev_warn(ds3232->dev,
+					"Write Control Register error %d\n",
+					ret);
+				goto unlock;
+			}
 
 			/* clear the alarm pend flag */
 			stat &= ~DS3232_REG_SR_A1F;
-			regmap_write(ds3232->regmap, DS3232_REG_SR, stat);
+			ret = regmap_write(ds3232->regmap, DS3232_REG_SR, stat);
+			if (ret) {
+				dev_warn(ds3232->dev,
+					"Write Status Register error %d\n",
+					ret);
+				goto unlock;
+			}
 
 			rtc_update_irq(ds3232->rtc, 1, RTC_AF | RTC_IRQF);
 
-- 
2.5.0

-- 
-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[rtc-linux] [PATCH v3 6/8] rtc: ds3232-core: remove unused UIE code

[Akinobu Mita]

UIE mode irqs are handled by the generic rtc core now.  But there are
remaining unused code fragments for it.

Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: Alexandre Belloni <alexandre.belloni@free-electrons.com>
Cc: Dennis Aberilla <denzzzhome@yahoo.com>
---
* New patch from this version

 drivers/rtc/rtc-ds3232-core.c | 20 +-------------------
 1 file changed, 1 insertion(+), 19 deletions(-)

diff --git a/drivers/rtc/rtc-ds3232-core.c b/drivers/rtc/rtc-ds3232-core.c
index bcce75b..be8dfe0 100644
--- a/drivers/rtc/rtc-ds3232-core.c
+++ b/drivers/rtc/rtc-ds3232-core.c
@@ -278,32 +278,14 @@ static int ds3232_update_alarm(struct device *dev, unsigned int enabled)
 	struct ds3232 *ds3232 = dev_get_drvdata(dev);
 	int control;
 	int ret;
-	u8 buf[4];
 
 	mutex_lock(&ds3232->mutex);
 
-	ret = regmap_bulk_read(ds3232->regmap, DS3232_REG_ALARM1, buf, 4);
-	if (ret)
-		goto unlock;
-
-	buf[0] = bcd2bin(buf[0]) < 0 || (ds3232->rtc->irq_data & RTC_UF) ?
-								0x80 : buf[0];
-	buf[1] = bcd2bin(buf[1]) < 0 || (ds3232->rtc->irq_data & RTC_UF) ?
-								0x80 : buf[1];
-	buf[2] = bcd2bin(buf[2]) < 0 || (ds3232->rtc->irq_data & RTC_UF) ?
-								0x80 : buf[2];
-	buf[3] = bcd2bin(buf[3]) < 0 || (ds3232->rtc->irq_data & RTC_UF) ?
-								0x80 : buf[3];
-
-	ret = regmap_bulk_write(ds3232->regmap, DS3232_REG_ALARM1, buf, 4);
-	if (ret)
-		goto unlock;
-
 	ret = regmap_read(ds3232->regmap, DS3232_REG_CR, &control);
 	if (ret)
 		goto unlock;
 
-	if (enabled || (ds3232->rtc->irq_data & RTC_UF))
+	if (enabled)
 		/* enable alarm1 interrupt */
 		control |= DS3232_REG_CR_A1IE;
 	else
-- 
2.5.0

-- 
-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[rtc-linux] [PATCH v3 7/8] rtc: ds3232-core: fix issue when irq is shared several devices

[Akinobu Mita]

ds3232-core requests irq with IRQF_SHARED, so irq can be shared by
several devices.  But the irq handler for ds3232 unconditionally
disables the irq at first and the irq is re-enabled only when the
interrupt source was the ds3232's alarm.  This behaviour breaks the
devices sharing the same irq in the various scenarios.

Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: Alexandre Belloni <alexandre.belloni@free-electrons.com>
Cc: Dennis Aberilla <denzzzhome@yahoo.com>
---
* New patch from this version

 drivers/rtc/rtc-ds3232-core.c | 14 ++------------
 1 file changed, 2 insertions(+), 12 deletions(-)

diff --git a/drivers/rtc/rtc-ds3232-core.c b/drivers/rtc/rtc-ds3232-core.c
index be8dfe0..6489380 100644
--- a/drivers/rtc/rtc-ds3232-core.c
+++ b/drivers/rtc/rtc-ds3232-core.c
@@ -59,7 +59,6 @@ struct ds3232 {
 	 */
 	struct mutex mutex;
 	bool suspended;
-	int exiting;
 };
 
 static int ds3232_check_rtc_status(struct device *dev)
@@ -314,8 +313,6 @@ static irqreturn_t ds3232_irq(int irq, void *dev_id)
 	struct device *dev = dev_id;
 	struct ds3232 *ds3232 = dev_get_drvdata(dev);
 
-	disable_irq_nosync(irq);
-
 	/*
 	 * If rtc as a wakeup source, can't schedule the work
 	 * at system resume flow, because at this time the i2c bus
@@ -342,8 +339,8 @@ static void ds3232_work(struct work_struct *work)
 	if (stat & DS3232_REG_SR_A1F) {
 		ret = regmap_read(ds3232->regmap, DS3232_REG_CR, &control);
 		if (ret) {
-			pr_warn("Read Control Register error - Disable IRQ%d\n",
-				ds3232->irq);
+			dev_warn(ds3232->dev,
+				"Read Control Register error %d\n", ret);
 		} else {
 			/* disable alarm1 interrupt */
 			control &= ~(DS3232_REG_CR_A1IE);
@@ -367,9 +364,6 @@ static void ds3232_work(struct work_struct *work)
 			}
 
 			rtc_update_irq(ds3232->rtc, 1, RTC_AF | RTC_IRQF);
-
-			if (!ds3232->exiting)
-				enable_irq(ds3232->irq);
 		}
 	}
 
@@ -428,10 +422,6 @@ int ds3232_remove(struct device *dev)
 	struct ds3232 *ds3232 = dev_get_drvdata(dev);
 
 	if (ds3232->irq > 0) {
-		mutex_lock(&ds3232->mutex);
-		ds3232->exiting = 1;
-		mutex_unlock(&ds3232->mutex);
-
 		devm_free_irq(dev, ds3232->irq, dev);
 		cancel_work_sync(&ds3232->work);
 	}
-- 
2.5.0

-- 
-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[rtc-linux] [PATCH v3 8/8] rtc: use rtc->ops_lock to protect alarm operations

[Akinobu Mita]

ds3232->mutex is used to protect for alarm operations which
need to access status and control registers.

But we can use rtc->ops_lock instead.  rtc->ops_lock is held when most
of rtc_class_ops methods are called, so we only need to explicitly
acquire it from irq handler in order to protect form concurrent
accesses.

Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: Alexandre Belloni <alexandre.belloni@free-electrons.com>
Cc: Dennis Aberilla <denzzzhome@yahoo.com>
---
* New patch from this version

 drivers/rtc/rtc-ds3232-core.c | 24 ++++--------------------
 1 file changed, 4 insertions(+), 20 deletions(-)

diff --git a/drivers/rtc/rtc-ds3232-core.c b/drivers/rtc/rtc-ds3232-core.c
index 6489380..fbb1b9a 100644
--- a/drivers/rtc/rtc-ds3232-core.c
+++ b/drivers/rtc/rtc-ds3232-core.c
@@ -53,11 +53,6 @@ struct ds3232 {
 	struct rtc_device *rtc;
 	struct work_struct work;
 
-	/* The mutex protects alarm operations, and prevents a race
-	 * between the enable_irq() in the workqueue and the free_irq()
-	 * in the remove function.
-	 */
-	struct mutex mutex;
 	bool suspended;
 };
 
@@ -188,8 +183,6 @@ static int ds3232_read_alarm(struct device *dev, struct rtc_wkalrm *alarm)
 	int ret;
 	u8 buf[4];
 
-	mutex_lock(&ds3232->mutex);
-
 	ret = regmap_read(ds3232->regmap, DS3232_REG_SR, &stat);
 	if (ret)
 		goto out;
@@ -216,7 +209,6 @@ static int ds3232_read_alarm(struct device *dev, struct rtc_wkalrm *alarm)
 
 	ret = 0;
 out:
-	mutex_unlock(&ds3232->mutex);
 	return ret;
 }
 
@@ -234,8 +226,6 @@ static int ds3232_set_alarm(struct device *dev, struct rtc_wkalrm *alarm)
 	if (ds3232->irq <= 0)
 		return -EINVAL;
 
-	mutex_lock(&ds3232->mutex);
-
 	buf[0] = bin2bcd(alarm->time.tm_sec);
 	buf[1] = bin2bcd(alarm->time.tm_min);
 	buf[2] = bin2bcd(alarm->time.tm_hour);
@@ -268,7 +258,6 @@ static int ds3232_set_alarm(struct device *dev, struct rtc_wkalrm *alarm)
 		ret = regmap_write(ds3232->regmap, DS3232_REG_CR, control);
 	}
 out:
-	mutex_unlock(&ds3232->mutex);
 	return ret;
 }
 
@@ -278,11 +267,9 @@ static int ds3232_update_alarm(struct device *dev, unsigned int enabled)
 	int control;
 	int ret;
 
-	mutex_lock(&ds3232->mutex);
-
 	ret = regmap_read(ds3232->regmap, DS3232_REG_CR, &control);
 	if (ret)
-		goto unlock;
+		return ret;
 
 	if (enabled)
 		/* enable alarm1 interrupt */
@@ -292,9 +279,6 @@ static int ds3232_update_alarm(struct device *dev, unsigned int enabled)
 		control &= ~(DS3232_REG_CR_A1IE);
 	ret = regmap_write(ds3232->regmap, DS3232_REG_CR, control);
 
-unlock:
-	mutex_unlock(&ds3232->mutex);
-
 	return ret;
 }
 
@@ -327,10 +311,11 @@ static irqreturn_t ds3232_irq(int irq, void *dev_id)
 static void ds3232_work(struct work_struct *work)
 {
 	struct ds3232 *ds3232 = container_of(work, struct ds3232, work);
+	struct mutex *lock = &ds3232->rtc->ops_lock;
 	int ret;
 	int stat, control;
 
-	mutex_lock(&ds3232->mutex);
+	mutex_lock(lock);
 
 	ret = regmap_read(ds3232->regmap, DS3232_REG_SR, &stat);
 	if (ret)
@@ -368,7 +353,7 @@ static void ds3232_work(struct work_struct *work)
 	}
 
 unlock:
-	mutex_unlock(&ds3232->mutex);
+	mutex_unlock(lock);
 }
 
 static const struct rtc_class_ops ds3232_rtc_ops = {
@@ -395,7 +380,6 @@ int __ds3232_probe(struct device *dev, struct regmap *regmap, int irq,
 	dev_set_drvdata(dev, ds3232);
 
 	INIT_WORK(&ds3232->work, ds3232_work);
-	mutex_init(&ds3232->mutex);
 
 	ret = ds3232_check_rtc_status(dev);
 	if (ret)
-- 
2.5.0

-- 
-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[rtc-linux] Re: [PATCH v3 7/8] rtc: ds3232-core: fix issue when irq is shared several devices

[Akinobu Mita]

2016-03-01 0:13 GMT+09:00 Akinobu Mita <akinobu.mita@gmail.com>:
> ds3232-core requests irq with IRQF_SHARED, so irq can be shared by
> several devices.  But the irq handler for ds3232 unconditionally
> disables the irq at first and the irq is re-enabled only when the
> interrupt source was the ds3232's alarm.  This behaviour breaks the
> devices sharing the same irq in the various scenarios.
>
> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
> Cc: Alessandro Zummo <a.zummo@towertech.it>
> Cc: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> Cc: Dennis Aberilla <denzzzhome@yahoo.com>
> ---
> * New patch from this version
>
>  drivers/rtc/rtc-ds3232-core.c | 14 ++------------
>  1 file changed, 2 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/rtc/rtc-ds3232-core.c b/drivers/rtc/rtc-ds3232-core.c
> index be8dfe0..6489380 100644
> --- a/drivers/rtc/rtc-ds3232-core.c
> +++ b/drivers/rtc/rtc-ds3232-core.c
> @@ -59,7 +59,6 @@ struct ds3232 {
>          */
>         struct mutex mutex;
>         bool suspended;
> -       int exiting;
>  };
>
>  static int ds3232_check_rtc_status(struct device *dev)
> @@ -314,8 +313,6 @@ static irqreturn_t ds3232_irq(int irq, void *dev_id)
>         struct device *dev = dev_id;
>         struct ds3232 *ds3232 = dev_get_drvdata(dev);
>
> -       disable_irq_nosync(irq);
> -
>         /*
>          * If rtc as a wakeup source, can't schedule the work
>          * at system resume flow, because at this time the i2c bus

I found that this patch is not correct because interrupt is re-enabled
after this hardirq handler finished without clearing Alarm Flag or
Alarm Interrupt Enable in RTC register.

Maybe we should drop IRQF_SHARED flag.

-- 
-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[rtc-linux] Re: [PATCH v3 7/8] rtc: ds3232-core: fix issue when irq is shared several devices

[Alexandre Belloni]

On 03/03/2016 at 23:10:35 +0900, Akinobu Mita wrote :
> 2016-03-01 0:13 GMT+09:00 Akinobu Mita <akinobu.mita@gmail.com>:
> > ds3232-core requests irq with IRQF_SHARED, so irq can be shared by
> > several devices.  But the irq handler for ds3232 unconditionally
> > disables the irq at first and the irq is re-enabled only when the
> > interrupt source was the ds3232's alarm.  This behaviour breaks the
> > devices sharing the same irq in the various scenarios.
> >
> > Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
> > Cc: Alessandro Zummo <a.zummo@towertech.it>
> > Cc: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> > Cc: Dennis Aberilla <denzzzhome@yahoo.com>
> > ---
> > * New patch from this version
> >
> >  drivers/rtc/rtc-ds3232-core.c | 14 ++------------
> >  1 file changed, 2 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/rtc/rtc-ds3232-core.c b/drivers/rtc/rtc-ds3232-core.c
> > index be8dfe0..6489380 100644
> > --- a/drivers/rtc/rtc-ds3232-core.c
> > +++ b/drivers/rtc/rtc-ds3232-core.c
> > @@ -59,7 +59,6 @@ struct ds3232 {
> >          */
> >         struct mutex mutex;
> >         bool suspended;
> > -       int exiting;
> >  };
> >
> >  static int ds3232_check_rtc_status(struct device *dev)
> > @@ -314,8 +313,6 @@ static irqreturn_t ds3232_irq(int irq, void *dev_id)
> >         struct device *dev = dev_id;
> >         struct ds3232 *ds3232 = dev_get_drvdata(dev);
> >
> > -       disable_irq_nosync(irq);
> > -
> >         /*
> >          * If rtc as a wakeup source, can't schedule the work
> >          * at system resume flow, because at this time the i2c bus
> 
> I found that this patch is not correct because interrupt is re-enabled
> after this hardirq handler finished without clearing Alarm Flag or
> Alarm Interrupt Enable in RTC register.
> 
> Maybe we should drop IRQF_SHARED flag.

I think the best is to completely reworke the irq handling. It dates
from many many year ago and can be replace by a simple threaded irq.
This will solve your issue.


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

-- 
-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[rtc-linux] Re: [PATCH v3 1/8] rtc: ds3232: rename rtc-ds3232.c to rtc-ds3232-core.c for preparation

[Alexandre Belloni]

Hi,

I'm fine with the patches (apart from patch 7/8) but I'd like to request
one more change. Could actually support both the ds3232 and the ds3234
in the same file? That will nicely reduce the number of lines and remove
the need for an include.

For an example, you can have a look at sound/soc/codecs/wm8750.c

Thank you for your work!

On 01/03/2016 at 00:13:45 +0900, Akinobu Mita wrote :
> This is preparation for splitting into core module and i2c driver.
> This will make it a lot easier to review the real changes.
> 
> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
> Cc: Alessandro Zummo <a.zummo@towertech.it>
> Cc: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> Cc: Dennis Aberilla <denzzzhome@yahoo.com>
> ---
> * No changes since v1
> 
>  drivers/rtc/Makefile          |   1 +
>  drivers/rtc/rtc-ds3232-core.c | 517 ++++++++++++++++++++++++++++++++++++++++++
>  drivers/rtc/rtc-ds3232.c      | 517 ------------------------------------------
>  3 files changed, 518 insertions(+), 517 deletions(-)
>  create mode 100644 drivers/rtc/rtc-ds3232-core.c
>  delete mode 100644 drivers/rtc/rtc-ds3232.c
> 
> diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
> index 62d61b2..0b8cb6c 100644
> --- a/drivers/rtc/Makefile
> +++ b/drivers/rtc/Makefile
> @@ -59,6 +59,7 @@ obj-$(CONFIG_RTC_DRV_DS1685_FAMILY)	+= rtc-ds1685.o
>  obj-$(CONFIG_RTC_DRV_DS1742)	+= rtc-ds1742.o
>  obj-$(CONFIG_RTC_DRV_DS2404)	+= rtc-ds2404.o
>  obj-$(CONFIG_RTC_DRV_DS3232)	+= rtc-ds3232.o
> +rtc-ds3232-objs			:= rtc-ds3232-core.o
>  obj-$(CONFIG_RTC_DRV_DS3234)	+= rtc-ds3234.o
>  obj-$(CONFIG_RTC_DRV_EFI)	+= rtc-efi.o
>  obj-$(CONFIG_RTC_DRV_EM3027)	+= rtc-em3027.o
> diff --git a/drivers/rtc/rtc-ds3232-core.c b/drivers/rtc/rtc-ds3232-core.c
> new file mode 100644
> index 0000000..4e99ace
> --- /dev/null
> +++ b/drivers/rtc/rtc-ds3232-core.c
> @@ -0,0 +1,517 @@
> +/*
> + * RTC client/driver for the Maxim/Dallas DS3232 Real-Time Clock over I2C
> + *
> + * Copyright (C) 2009-2011 Freescale Semiconductor.
> + * Author: Jack Lan <jack.lan@freescale.com>
> + *
> + * This program is free software; you can redistribute  it and/or modify it
> + * under  the terms of  the GNU General  Public License as published by the
> + * Free Software Foundation;  either version 2 of the  License, or (at your
> + * option) any later version.
> + */
> +/*
> + * It would be more efficient to use i2c msgs/i2c_transfer directly but, as
> + * recommened in .../Documentation/i2c/writing-clients section
> + * "Sending and receiving", using SMBus level communication is preferred.
> + */
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/interrupt.h>
> +#include <linux/i2c.h>
> +#include <linux/rtc.h>
> +#include <linux/bcd.h>
> +#include <linux/workqueue.h>
> +#include <linux/slab.h>
> +
> +#define DS3232_REG_SECONDS	0x00
> +#define DS3232_REG_MINUTES	0x01
> +#define DS3232_REG_HOURS	0x02
> +#define DS3232_REG_AMPM		0x02
> +#define DS3232_REG_DAY		0x03
> +#define DS3232_REG_DATE		0x04
> +#define DS3232_REG_MONTH	0x05
> +#define DS3232_REG_CENTURY	0x05
> +#define DS3232_REG_YEAR		0x06
> +#define DS3232_REG_ALARM1         0x07	/* Alarm 1 BASE */
> +#define DS3232_REG_ALARM2         0x0B	/* Alarm 2 BASE */
> +#define DS3232_REG_CR		0x0E	/* Control register */
> +#	define DS3232_REG_CR_nEOSC        0x80
> +#       define DS3232_REG_CR_INTCN        0x04
> +#       define DS3232_REG_CR_A2IE        0x02
> +#       define DS3232_REG_CR_A1IE        0x01
> +
> +#define DS3232_REG_SR	0x0F	/* control/status register */
> +#	define DS3232_REG_SR_OSF   0x80
> +#       define DS3232_REG_SR_BSY   0x04
> +#       define DS3232_REG_SR_A2F   0x02
> +#       define DS3232_REG_SR_A1F   0x01
> +
> +struct ds3232 {
> +	struct i2c_client *client;
> +	struct rtc_device *rtc;
> +	struct work_struct work;
> +
> +	/* The mutex protects alarm operations, and prevents a race
> +	 * between the enable_irq() in the workqueue and the free_irq()
> +	 * in the remove function.
> +	 */
> +	struct mutex mutex;
> +	bool suspended;
> +	int exiting;
> +};
> +
> +static struct i2c_driver ds3232_driver;
> +
> +static int ds3232_check_rtc_status(struct i2c_client *client)
> +{
> +	int ret = 0;
> +	int control, stat;
> +
> +	stat = i2c_smbus_read_byte_data(client, DS3232_REG_SR);
> +	if (stat < 0)
> +		return stat;
> +
> +	if (stat & DS3232_REG_SR_OSF)
> +		dev_warn(&client->dev,
> +				"oscillator discontinuity flagged, "
> +				"time unreliable\n");
> +
> +	stat &= ~(DS3232_REG_SR_OSF | DS3232_REG_SR_A1F | DS3232_REG_SR_A2F);
> +
> +	ret = i2c_smbus_write_byte_data(client, DS3232_REG_SR, stat);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* If the alarm is pending, clear it before requesting
> +	 * the interrupt, so an interrupt event isn't reported
> +	 * before everything is initialized.
> +	 */
> +
> +	control = i2c_smbus_read_byte_data(client, DS3232_REG_CR);
> +	if (control < 0)
> +		return control;
> +
> +	control &= ~(DS3232_REG_CR_A1IE | DS3232_REG_CR_A2IE);
> +	control |= DS3232_REG_CR_INTCN;
> +
> +	return i2c_smbus_write_byte_data(client, DS3232_REG_CR, control);
> +}
> +
> +static int ds3232_read_time(struct device *dev, struct rtc_time *time)
> +{
> +	struct i2c_client *client = to_i2c_client(dev);
> +	int ret;
> +	u8 buf[7];
> +	unsigned int year, month, day, hour, minute, second;
> +	unsigned int week, twelve_hr, am_pm;
> +	unsigned int century, add_century = 0;
> +
> +	ret = i2c_smbus_read_i2c_block_data(client, DS3232_REG_SECONDS, 7, buf);
> +
> +	if (ret < 0)
> +		return ret;
> +	if (ret < 7)
> +		return -EIO;
> +
> +	second = buf[0];
> +	minute = buf[1];
> +	hour = buf[2];
> +	week = buf[3];
> +	day = buf[4];
> +	month = buf[5];
> +	year = buf[6];
> +
> +	/* Extract additional information for AM/PM and century */
> +
> +	twelve_hr = hour & 0x40;
> +	am_pm = hour & 0x20;
> +	century = month & 0x80;
> +
> +	/* Write to rtc_time structure */
> +
> +	time->tm_sec = bcd2bin(second);
> +	time->tm_min = bcd2bin(minute);
> +	if (twelve_hr) {
> +		/* Convert to 24 hr */
> +		if (am_pm)
> +			time->tm_hour = bcd2bin(hour & 0x1F) + 12;
> +		else
> +			time->tm_hour = bcd2bin(hour & 0x1F);
> +	} else {
> +		time->tm_hour = bcd2bin(hour);
> +	}
> +
> +	/* Day of the week in linux range is 0~6 while 1~7 in RTC chip */
> +	time->tm_wday = bcd2bin(week) - 1;
> +	time->tm_mday = bcd2bin(day);
> +	/* linux tm_mon range:0~11, while month range is 1~12 in RTC chip */
> +	time->tm_mon = bcd2bin(month & 0x7F) - 1;
> +	if (century)
> +		add_century = 100;
> +
> +	time->tm_year = bcd2bin(year) + add_century;
> +
> +	return rtc_valid_tm(time);
> +}
> +
> +static int ds3232_set_time(struct device *dev, struct rtc_time *time)
> +{
> +	struct i2c_client *client = to_i2c_client(dev);
> +	u8 buf[7];
> +
> +	/* Extract time from rtc_time and load into ds3232*/
> +
> +	buf[0] = bin2bcd(time->tm_sec);
> +	buf[1] = bin2bcd(time->tm_min);
> +	buf[2] = bin2bcd(time->tm_hour);
> +	/* Day of the week in linux range is 0~6 while 1~7 in RTC chip */
> +	buf[3] = bin2bcd(time->tm_wday + 1);
> +	buf[4] = bin2bcd(time->tm_mday); /* Date */
> +	/* linux tm_mon range:0~11, while month range is 1~12 in RTC chip */
> +	buf[5] = bin2bcd(time->tm_mon + 1);
> +	if (time->tm_year >= 100) {
> +		buf[5] |= 0x80;
> +		buf[6] = bin2bcd(time->tm_year - 100);
> +	} else {
> +		buf[6] = bin2bcd(time->tm_year);
> +	}
> +
> +	return i2c_smbus_write_i2c_block_data(client,
> +					      DS3232_REG_SECONDS, 7, buf);
> +}
> +
> +/*
> + * DS3232 has two alarm, we only use alarm1
> + * According to linux specification, only support one-shot alarm
> + * no periodic alarm mode
> + */
> +static int ds3232_read_alarm(struct device *dev, struct rtc_wkalrm *alarm)
> +{
> +	struct i2c_client *client = to_i2c_client(dev);
> +	struct ds3232 *ds3232 = i2c_get_clientdata(client);
> +	int control, stat;
> +	int ret;
> +	u8 buf[4];
> +
> +	mutex_lock(&ds3232->mutex);
> +
> +	ret = i2c_smbus_read_byte_data(client, DS3232_REG_SR);
> +	if (ret < 0)
> +		goto out;
> +	stat = ret;
> +	ret = i2c_smbus_read_byte_data(client, DS3232_REG_CR);
> +	if (ret < 0)
> +		goto out;
> +	control = ret;
> +	ret = i2c_smbus_read_i2c_block_data(client, DS3232_REG_ALARM1, 4, buf);
> +	if (ret < 0)
> +		goto out;
> +
> +	alarm->time.tm_sec = bcd2bin(buf[0] & 0x7F);
> +	alarm->time.tm_min = bcd2bin(buf[1] & 0x7F);
> +	alarm->time.tm_hour = bcd2bin(buf[2] & 0x7F);
> +	alarm->time.tm_mday = bcd2bin(buf[3] & 0x7F);
> +
> +	alarm->time.tm_mon = -1;
> +	alarm->time.tm_year = -1;
> +	alarm->time.tm_wday = -1;
> +	alarm->time.tm_yday = -1;
> +	alarm->time.tm_isdst = -1;
> +
> +	alarm->enabled = !!(control & DS3232_REG_CR_A1IE);
> +	alarm->pending = !!(stat & DS3232_REG_SR_A1F);
> +
> +	ret = 0;
> +out:
> +	mutex_unlock(&ds3232->mutex);
> +	return ret;
> +}
> +
> +/*
> + * linux rtc-module does not support wday alarm
> + * and only 24h time mode supported indeed
> + */
> +static int ds3232_set_alarm(struct device *dev, struct rtc_wkalrm *alarm)
> +{
> +	struct i2c_client *client = to_i2c_client(dev);
> +	struct ds3232 *ds3232 = i2c_get_clientdata(client);
> +	int control, stat;
> +	int ret;
> +	u8 buf[4];
> +
> +	if (client->irq <= 0)
> +		return -EINVAL;
> +
> +	mutex_lock(&ds3232->mutex);
> +
> +	buf[0] = bin2bcd(alarm->time.tm_sec);
> +	buf[1] = bin2bcd(alarm->time.tm_min);
> +	buf[2] = bin2bcd(alarm->time.tm_hour);
> +	buf[3] = bin2bcd(alarm->time.tm_mday);
> +
> +	/* clear alarm interrupt enable bit */
> +	ret = i2c_smbus_read_byte_data(client, DS3232_REG_CR);
> +	if (ret < 0)
> +		goto out;
> +	control = ret;
> +	control &= ~(DS3232_REG_CR_A1IE | DS3232_REG_CR_A2IE);
> +	ret = i2c_smbus_write_byte_data(client, DS3232_REG_CR, control);
> +	if (ret < 0)
> +		goto out;
> +
> +	/* clear any pending alarm flag */
> +	ret = i2c_smbus_read_byte_data(client, DS3232_REG_SR);
> +	if (ret < 0)
> +		goto out;
> +	stat = ret;
> +	stat &= ~(DS3232_REG_SR_A1F | DS3232_REG_SR_A2F);
> +	ret = i2c_smbus_write_byte_data(client, DS3232_REG_SR, stat);
> +	if (ret < 0)
> +		goto out;
> +
> +	ret = i2c_smbus_write_i2c_block_data(client, DS3232_REG_ALARM1, 4, buf);
> +
> +	if (alarm->enabled) {
> +		control |= DS3232_REG_CR_A1IE;
> +		ret = i2c_smbus_write_byte_data(client, DS3232_REG_CR, control);
> +	}
> +out:
> +	mutex_unlock(&ds3232->mutex);
> +	return ret;
> +}
> +
> +static void ds3232_update_alarm(struct i2c_client *client)
> +{
> +	struct ds3232 *ds3232 = i2c_get_clientdata(client);
> +	int control;
> +	int ret;
> +	u8 buf[4];
> +
> +	mutex_lock(&ds3232->mutex);
> +
> +	ret = i2c_smbus_read_i2c_block_data(client, DS3232_REG_ALARM1, 4, buf);
> +	if (ret < 0)
> +		goto unlock;
> +
> +	buf[0] = bcd2bin(buf[0]) < 0 || (ds3232->rtc->irq_data & RTC_UF) ?
> +								0x80 : buf[0];
> +	buf[1] = bcd2bin(buf[1]) < 0 || (ds3232->rtc->irq_data & RTC_UF) ?
> +								0x80 : buf[1];
> +	buf[2] = bcd2bin(buf[2]) < 0 || (ds3232->rtc->irq_data & RTC_UF) ?
> +								0x80 : buf[2];
> +	buf[3] = bcd2bin(buf[3]) < 0 || (ds3232->rtc->irq_data & RTC_UF) ?
> +								0x80 : buf[3];
> +
> +	ret = i2c_smbus_write_i2c_block_data(client, DS3232_REG_ALARM1, 4, buf);
> +	if (ret < 0)
> +		goto unlock;
> +
> +	control = i2c_smbus_read_byte_data(client, DS3232_REG_CR);
> +	if (control < 0)
> +		goto unlock;
> +
> +	if (ds3232->rtc->irq_data & (RTC_AF | RTC_UF))
> +		/* enable alarm1 interrupt */
> +		control |= DS3232_REG_CR_A1IE;
> +	else
> +		/* disable alarm1 interrupt */
> +		control &= ~(DS3232_REG_CR_A1IE);
> +	i2c_smbus_write_byte_data(client, DS3232_REG_CR, control);
> +
> +unlock:
> +	mutex_unlock(&ds3232->mutex);
> +}
> +
> +static int ds3232_alarm_irq_enable(struct device *dev, unsigned int enabled)
> +{
> +	struct i2c_client *client = to_i2c_client(dev);
> +	struct ds3232 *ds3232 = i2c_get_clientdata(client);
> +
> +	if (client->irq <= 0)
> +		return -EINVAL;
> +
> +	if (enabled)
> +		ds3232->rtc->irq_data |= RTC_AF;
> +	else
> +		ds3232->rtc->irq_data &= ~RTC_AF;
> +
> +	ds3232_update_alarm(client);
> +	return 0;
> +}
> +
> +static irqreturn_t ds3232_irq(int irq, void *dev_id)
> +{
> +	struct i2c_client *client = dev_id;
> +	struct ds3232 *ds3232 = i2c_get_clientdata(client);
> +
> +	disable_irq_nosync(irq);
> +
> +	/*
> +	 * If rtc as a wakeup source, can't schedule the work
> +	 * at system resume flow, because at this time the i2c bus
> +	 * has not been resumed.
> +	 */
> +	if (!ds3232->suspended)
> +		schedule_work(&ds3232->work);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static void ds3232_work(struct work_struct *work)
> +{
> +	struct ds3232 *ds3232 = container_of(work, struct ds3232, work);
> +	struct i2c_client *client = ds3232->client;
> +	int stat, control;
> +
> +	mutex_lock(&ds3232->mutex);
> +
> +	stat = i2c_smbus_read_byte_data(client, DS3232_REG_SR);
> +	if (stat < 0)
> +		goto unlock;
> +
> +	if (stat & DS3232_REG_SR_A1F) {
> +		control = i2c_smbus_read_byte_data(client, DS3232_REG_CR);
> +		if (control < 0) {
> +			pr_warn("Read Control Register error - Disable IRQ%d\n",
> +				client->irq);
> +		} else {
> +			/* disable alarm1 interrupt */
> +			control &= ~(DS3232_REG_CR_A1IE);
> +			i2c_smbus_write_byte_data(client, DS3232_REG_CR,
> +						control);
> +
> +			/* clear the alarm pend flag */
> +			stat &= ~DS3232_REG_SR_A1F;
> +			i2c_smbus_write_byte_data(client, DS3232_REG_SR, stat);
> +
> +			rtc_update_irq(ds3232->rtc, 1, RTC_AF | RTC_IRQF);
> +
> +			if (!ds3232->exiting)
> +				enable_irq(client->irq);
> +		}
> +	}
> +
> +unlock:
> +	mutex_unlock(&ds3232->mutex);
> +}
> +
> +static const struct rtc_class_ops ds3232_rtc_ops = {
> +	.read_time = ds3232_read_time,
> +	.set_time = ds3232_set_time,
> +	.read_alarm = ds3232_read_alarm,
> +	.set_alarm = ds3232_set_alarm,
> +	.alarm_irq_enable = ds3232_alarm_irq_enable,
> +};
> +
> +static int ds3232_probe(struct i2c_client *client,
> +			const struct i2c_device_id *id)
> +{
> +	struct ds3232 *ds3232;
> +	int ret;
> +
> +	ds3232 = devm_kzalloc(&client->dev, sizeof(struct ds3232), GFP_KERNEL);
> +	if (!ds3232)
> +		return -ENOMEM;
> +
> +	ds3232->client = client;
> +	i2c_set_clientdata(client, ds3232);
> +
> +	INIT_WORK(&ds3232->work, ds3232_work);
> +	mutex_init(&ds3232->mutex);
> +
> +	ret = ds3232_check_rtc_status(client);
> +	if (ret)
> +		return ret;
> +
> +	if (client->irq > 0) {
> +		ret = devm_request_irq(&client->dev, client->irq, ds3232_irq,
> +				       IRQF_SHARED, "ds3232", client);
> +		if (ret) {
> +			dev_err(&client->dev, "unable to request IRQ\n");
> +		}
> +		device_init_wakeup(&client->dev, 1);
> +	}
> +	ds3232->rtc = devm_rtc_device_register(&client->dev, client->name,
> +					  &ds3232_rtc_ops, THIS_MODULE);
> +	return PTR_ERR_OR_ZERO(ds3232->rtc);
> +}
> +
> +static int ds3232_remove(struct i2c_client *client)
> +{
> +	struct ds3232 *ds3232 = i2c_get_clientdata(client);
> +
> +	if (client->irq > 0) {
> +		mutex_lock(&ds3232->mutex);
> +		ds3232->exiting = 1;
> +		mutex_unlock(&ds3232->mutex);
> +
> +		devm_free_irq(&client->dev, client->irq, client);
> +		cancel_work_sync(&ds3232->work);
> +	}
> +
> +	return 0;
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int ds3232_suspend(struct device *dev)
> +{
> +	struct ds3232 *ds3232 = dev_get_drvdata(dev);
> +	struct i2c_client *client = to_i2c_client(dev);
> +
> +	if (device_can_wakeup(dev)) {
> +		ds3232->suspended = true;
> +		if (irq_set_irq_wake(client->irq, 1)) {
> +			dev_warn_once(dev, "Cannot set wakeup source\n");
> +			ds3232->suspended = false;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static int ds3232_resume(struct device *dev)
> +{
> +	struct ds3232 *ds3232 = dev_get_drvdata(dev);
> +	struct i2c_client *client = to_i2c_client(dev);
> +
> +	if (ds3232->suspended) {
> +		ds3232->suspended = false;
> +
> +		/* Clear the hardware alarm pend flag */
> +		schedule_work(&ds3232->work);
> +
> +		irq_set_irq_wake(client->irq, 0);
> +	}
> +
> +	return 0;
> +}
> +#endif
> +
> +static const struct dev_pm_ops ds3232_pm_ops = {
> +	SET_SYSTEM_SLEEP_PM_OPS(ds3232_suspend, ds3232_resume)
> +};
> +
> +static const struct i2c_device_id ds3232_id[] = {
> +	{ "ds3232", 0 },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, ds3232_id);
> +
> +static struct i2c_driver ds3232_driver = {
> +	.driver = {
> +		.name = "rtc-ds3232",
> +		.pm	= &ds3232_pm_ops,
> +	},
> +	.probe = ds3232_probe,
> +	.remove = ds3232_remove,
> +	.id_table = ds3232_id,
> +};
> +
> +module_i2c_driver(ds3232_driver);
> +
> +MODULE_AUTHOR("Srikanth Srinivasan <srikanth.srinivasan@freescale.com>");
> +MODULE_DESCRIPTION("Maxim/Dallas DS3232 RTC Driver");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/rtc/rtc-ds3232.c b/drivers/rtc/rtc-ds3232.c
> deleted file mode 100644
> index 4e99ace..0000000
> --- a/drivers/rtc/rtc-ds3232.c
> +++ /dev/null
> @@ -1,517 +0,0 @@
> -/*
> - * RTC client/driver for the Maxim/Dallas DS3232 Real-Time Clock over I2C
> - *
> - * Copyright (C) 2009-2011 Freescale Semiconductor.
> - * Author: Jack Lan <jack.lan@freescale.com>
> - *
> - * This program is free software; you can redistribute  it and/or modify it
> - * under  the terms of  the GNU General  Public License as published by the
> - * Free Software Foundation;  either version 2 of the  License, or (at your
> - * option) any later version.
> - */
> -/*
> - * It would be more efficient to use i2c msgs/i2c_transfer directly but, as
> - * recommened in .../Documentation/i2c/writing-clients section
> - * "Sending and receiving", using SMBus level communication is preferred.
> - */
> -
> -#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> -
> -#include <linux/kernel.h>
> -#include <linux/module.h>
> -#include <linux/interrupt.h>
> -#include <linux/i2c.h>
> -#include <linux/rtc.h>
> -#include <linux/bcd.h>
> -#include <linux/workqueue.h>
> -#include <linux/slab.h>
> -
> -#define DS3232_REG_SECONDS	0x00
> -#define DS3232_REG_MINUTES	0x01
> -#define DS3232_REG_HOURS	0x02
> -#define DS3232_REG_AMPM		0x02
> -#define DS3232_REG_DAY		0x03
> -#define DS3232_REG_DATE		0x04
> -#define DS3232_REG_MONTH	0x05
> -#define DS3232_REG_CENTURY	0x05
> -#define DS3232_REG_YEAR		0x06
> -#define DS3232_REG_ALARM1         0x07	/* Alarm 1 BASE */
> -#define DS3232_REG_ALARM2         0x0B	/* Alarm 2 BASE */
> -#define DS3232_REG_CR		0x0E	/* Control register */
> -#	define DS3232_REG_CR_nEOSC        0x80
> -#       define DS3232_REG_CR_INTCN        0x04
> -#       define DS3232_REG_CR_A2IE        0x02
> -#       define DS3232_REG_CR_A1IE        0x01
> -
> -#define DS3232_REG_SR	0x0F	/* control/status register */
> -#	define DS3232_REG_SR_OSF   0x80
> -#       define DS3232_REG_SR_BSY   0x04
> -#       define DS3232_REG_SR_A2F   0x02
> -#       define DS3232_REG_SR_A1F   0x01
> -
> -struct ds3232 {
> -	struct i2c_client *client;
> -	struct rtc_device *rtc;
> -	struct work_struct work;
> -
> -	/* The mutex protects alarm operations, and prevents a race
> -	 * between the enable_irq() in the workqueue and the free_irq()
> -	 * in the remove function.
> -	 */
> -	struct mutex mutex;
> -	bool suspended;
> -	int exiting;
> -};
> -
> -static struct i2c_driver ds3232_driver;
> -
> -static int ds3232_check_rtc_status(struct i2c_client *client)
> -{
> -	int ret = 0;
> -	int control, stat;
> -
> -	stat = i2c_smbus_read_byte_data(client, DS3232_REG_SR);
> -	if (stat < 0)
> -		return stat;
> -
> -	if (stat & DS3232_REG_SR_OSF)
> -		dev_warn(&client->dev,
> -				"oscillator discontinuity flagged, "
> -				"time unreliable\n");
> -
> -	stat &= ~(DS3232_REG_SR_OSF | DS3232_REG_SR_A1F | DS3232_REG_SR_A2F);
> -
> -	ret = i2c_smbus_write_byte_data(client, DS3232_REG_SR, stat);
> -	if (ret < 0)
> -		return ret;
> -
> -	/* If the alarm is pending, clear it before requesting
> -	 * the interrupt, so an interrupt event isn't reported
> -	 * before everything is initialized.
> -	 */
> -
> -	control = i2c_smbus_read_byte_data(client, DS3232_REG_CR);
> -	if (control < 0)
> -		return control;
> -
> -	control &= ~(DS3232_REG_CR_A1IE | DS3232_REG_CR_A2IE);
> -	control |= DS3232_REG_CR_INTCN;
> -
> -	return i2c_smbus_write_byte_data(client, DS3232_REG_CR, control);
> -}
> -
> -static int ds3232_read_time(struct device *dev, struct rtc_time *time)
> -{
> -	struct i2c_client *client = to_i2c_client(dev);
> -	int ret;
> -	u8 buf[7];
> -	unsigned int year, month, day, hour, minute, second;
> -	unsigned int week, twelve_hr, am_pm;
> -	unsigned int century, add_century = 0;
> -
> -	ret = i2c_smbus_read_i2c_block_data(client, DS3232_REG_SECONDS, 7, buf);
> -
> -	if (ret < 0)
> -		return ret;
> -	if (ret < 7)
> -		return -EIO;
> -
> -	second = buf[0];
> -	minute = buf[1];
> -	hour = buf[2];
> -	week = buf[3];
> -	day = buf[4];
> -	month = buf[5];
> -	year = buf[6];
> -
> -	/* Extract additional information for AM/PM and century */
> -
> -	twelve_hr = hour & 0x40;
> -	am_pm = hour & 0x20;
> -	century = month & 0x80;
> -
> -	/* Write to rtc_time structure */
> -
> -	time->tm_sec = bcd2bin(second);
> -	time->tm_min = bcd2bin(minute);
> -	if (twelve_hr) {
> -		/* Convert to 24 hr */
> -		if (am_pm)
> -			time->tm_hour = bcd2bin(hour & 0x1F) + 12;
> -		else
> -			time->tm_hour = bcd2bin(hour & 0x1F);
> -	} else {
> -		time->tm_hour = bcd2bin(hour);
> -	}
> -
> -	/* Day of the week in linux range is 0~6 while 1~7 in RTC chip */
> -	time->tm_wday = bcd2bin(week) - 1;
> -	time->tm_mday = bcd2bin(day);
> -	/* linux tm_mon range:0~11, while month range is 1~12 in RTC chip */
> -	time->tm_mon = bcd2bin(month & 0x7F) - 1;
> -	if (century)
> -		add_century = 100;
> -
> -	time->tm_year = bcd2bin(year) + add_century;
> -
> -	return rtc_valid_tm(time);
> -}
> -
> -static int ds3232_set_time(struct device *dev, struct rtc_time *time)
> -{
> -	struct i2c_client *client = to_i2c_client(dev);
> -	u8 buf[7];
> -
> -	/* Extract time from rtc_time and load into ds3232*/
> -
> -	buf[0] = bin2bcd(time->tm_sec);
> -	buf[1] = bin2bcd(time->tm_min);
> -	buf[2] = bin2bcd(time->tm_hour);
> -	/* Day of the week in linux range is 0~6 while 1~7 in RTC chip */
> -	buf[3] = bin2bcd(time->tm_wday + 1);
> -	buf[4] = bin2bcd(time->tm_mday); /* Date */
> -	/* linux tm_mon range:0~11, while month range is 1~12 in RTC chip */
> -	buf[5] = bin2bcd(time->tm_mon + 1);
> -	if (time->tm_year >= 100) {
> -		buf[5] |= 0x80;
> -		buf[6] = bin2bcd(time->tm_year - 100);
> -	} else {
> -		buf[6] = bin2bcd(time->tm_year);
> -	}
> -
> -	return i2c_smbus_write_i2c_block_data(client,
> -					      DS3232_REG_SECONDS, 7, buf);
> -}
> -
> -/*
> - * DS3232 has two alarm, we only use alarm1
> - * According to linux specification, only support one-shot alarm
> - * no periodic alarm mode
> - */
> -static int ds3232_read_alarm(struct device *dev, struct rtc_wkalrm *alarm)
> -{
> -	struct i2c_client *client = to_i2c_client(dev);
> -	struct ds3232 *ds3232 = i2c_get_clientdata(client);
> -	int control, stat;
> -	int ret;
> -	u8 buf[4];
> -
> -	mutex_lock(&ds3232->mutex);
> -
> -	ret = i2c_smbus_read_byte_data(client, DS3232_REG_SR);
> -	if (ret < 0)
> -		goto out;
> -	stat = ret;
> -	ret = i2c_smbus_read_byte_data(client, DS3232_REG_CR);
> -	if (ret < 0)
> -		goto out;
> -	control = ret;
> -	ret = i2c_smbus_read_i2c_block_data(client, DS3232_REG_ALARM1, 4, buf);
> -	if (ret < 0)
> -		goto out;
> -
> -	alarm->time.tm_sec = bcd2bin(buf[0] & 0x7F);
> -	alarm->time.tm_min = bcd2bin(buf[1] & 0x7F);
> -	alarm->time.tm_hour = bcd2bin(buf[2] & 0x7F);
> -	alarm->time.tm_mday = bcd2bin(buf[3] & 0x7F);
> -
> -	alarm->time.tm_mon = -1;
> -	alarm->time.tm_year = -1;
> -	alarm->time.tm_wday = -1;
> -	alarm->time.tm_yday = -1;
> -	alarm->time.tm_isdst = -1;
> -
> -	alarm->enabled = !!(control & DS3232_REG_CR_A1IE);
> -	alarm->pending = !!(stat & DS3232_REG_SR_A1F);
> -
> -	ret = 0;
> -out:
> -	mutex_unlock(&ds3232->mutex);
> -	return ret;
> -}
> -
> -/*
> - * linux rtc-module does not support wday alarm
> - * and only 24h time mode supported indeed
> - */
> -static int ds3232_set_alarm(struct device *dev, struct rtc_wkalrm *alarm)
> -{
> -	struct i2c_client *client = to_i2c_client(dev);
> -	struct ds3232 *ds3232 = i2c_get_clientdata(client);
> -	int control, stat;
> -	int ret;
> -	u8 buf[4];
> -
> -	if (client->irq <= 0)
> -		return -EINVAL;
> -
> -	mutex_lock(&ds3232->mutex);
> -
> -	buf[0] = bin2bcd(alarm->time.tm_sec);
> -	buf[1] = bin2bcd(alarm->time.tm_min);
> -	buf[2] = bin2bcd(alarm->time.tm_hour);
> -	buf[3] = bin2bcd(alarm->time.tm_mday);
> -
> -	/* clear alarm interrupt enable bit */
> -	ret = i2c_smbus_read_byte_data(client, DS3232_REG_CR);
> -	if (ret < 0)
> -		goto out;
> -	control = ret;
> -	control &= ~(DS3232_REG_CR_A1IE | DS3232_REG_CR_A2IE);
> -	ret = i2c_smbus_write_byte_data(client, DS3232_REG_CR, control);
> -	if (ret < 0)
> -		goto out;
> -
> -	/* clear any pending alarm flag */
> -	ret = i2c_smbus_read_byte_data(client, DS3232_REG_SR);
> -	if (ret < 0)
> -		goto out;
> -	stat = ret;
> -	stat &= ~(DS3232_REG_SR_A1F | DS3232_REG_SR_A2F);
> -	ret = i2c_smbus_write_byte_data(client, DS3232_REG_SR, stat);
> -	if (ret < 0)
> -		goto out;
> -
> -	ret = i2c_smbus_write_i2c_block_data(client, DS3232_REG_ALARM1, 4, buf);
> -
> -	if (alarm->enabled) {
> -		control |= DS3232_REG_CR_A1IE;
> -		ret = i2c_smbus_write_byte_data(client, DS3232_REG_CR, control);
> -	}
> -out:
> -	mutex_unlock(&ds3232->mutex);
> -	return ret;
> -}
> -
> -static void ds3232_update_alarm(struct i2c_client *client)
> -{
> -	struct ds3232 *ds3232 = i2c_get_clientdata(client);
> -	int control;
> -	int ret;
> -	u8 buf[4];
> -
> -	mutex_lock(&ds3232->mutex);
> -
> -	ret = i2c_smbus_read_i2c_block_data(client, DS3232_REG_ALARM1, 4, buf);
> -	if (ret < 0)
> -		goto unlock;
> -
> -	buf[0] = bcd2bin(buf[0]) < 0 || (ds3232->rtc->irq_data & RTC_UF) ?
> -								0x80 : buf[0];
> -	buf[1] = bcd2bin(buf[1]) < 0 || (ds3232->rtc->irq_data & RTC_UF) ?
> -								0x80 : buf[1];
> -	buf[2] = bcd2bin(buf[2]) < 0 || (ds3232->rtc->irq_data & RTC_UF) ?
> -								0x80 : buf[2];
> -	buf[3] = bcd2bin(buf[3]) < 0 || (ds3232->rtc->irq_data & RTC_UF) ?
> -								0x80 : buf[3];
> -
> -	ret = i2c_smbus_write_i2c_block_data(client, DS3232_REG_ALARM1, 4, buf);
> -	if (ret < 0)
> -		goto unlock;
> -
> -	control = i2c_smbus_read_byte_data(client, DS3232_REG_CR);
> -	if (control < 0)
> -		goto unlock;
> -
> -	if (ds3232->rtc->irq_data & (RTC_AF | RTC_UF))
> -		/* enable alarm1 interrupt */
> -		control |= DS3232_REG_CR_A1IE;
> -	else
> -		/* disable alarm1 interrupt */
> -		control &= ~(DS3232_REG_CR_A1IE);
> -	i2c_smbus_write_byte_data(client, DS3232_REG_CR, control);
> -
> -unlock:
> -	mutex_unlock(&ds3232->mutex);
> -}
> -
> -static int ds3232_alarm_irq_enable(struct device *dev, unsigned int enabled)
> -{
> -	struct i2c_client *client = to_i2c_client(dev);
> -	struct ds3232 *ds3232 = i2c_get_clientdata(client);
> -
> -	if (client->irq <= 0)
> -		return -EINVAL;
> -
> -	if (enabled)
> -		ds3232->rtc->irq_data |= RTC_AF;
> -	else
> -		ds3232->rtc->irq_data &= ~RTC_AF;
> -
> -	ds3232_update_alarm(client);
> -	return 0;
> -}
> -
> -static irqreturn_t ds3232_irq(int irq, void *dev_id)
> -{
> -	struct i2c_client *client = dev_id;
> -	struct ds3232 *ds3232 = i2c_get_clientdata(client);
> -
> -	disable_irq_nosync(irq);
> -
> -	/*
> -	 * If rtc as a wakeup source, can't schedule the work
> -	 * at system resume flow, because at this time the i2c bus
> -	 * has not been resumed.
> -	 */
> -	if (!ds3232->suspended)
> -		schedule_work(&ds3232->work);
> -
> -	return IRQ_HANDLED;
> -}
> -
> -static void ds3232_work(struct work_struct *work)
> -{
> -	struct ds3232 *ds3232 = container_of(work, struct ds3232, work);
> -	struct i2c_client *client = ds3232->client;
> -	int stat, control;
> -
> -	mutex_lock(&ds3232->mutex);
> -
> -	stat = i2c_smbus_read_byte_data(client, DS3232_REG_SR);
> -	if (stat < 0)
> -		goto unlock;
> -
> -	if (stat & DS3232_REG_SR_A1F) {
> -		control = i2c_smbus_read_byte_data(client, DS3232_REG_CR);
> -		if (control < 0) {
> -			pr_warn("Read Control Register error - Disable IRQ%d\n",
> -				client->irq);
> -		} else {
> -			/* disable alarm1 interrupt */
> -			control &= ~(DS3232_REG_CR_A1IE);
> -			i2c_smbus_write_byte_data(client, DS3232_REG_CR,
> -						control);
> -
> -			/* clear the alarm pend flag */
> -			stat &= ~DS3232_REG_SR_A1F;
> -			i2c_smbus_write_byte_data(client, DS3232_REG_SR, stat);
> -
> -			rtc_update_irq(ds3232->rtc, 1, RTC_AF | RTC_IRQF);
> -
> -			if (!ds3232->exiting)
> -				enable_irq(client->irq);
> -		}
> -	}
> -
> -unlock:
> -	mutex_unlock(&ds3232->mutex);
> -}
> -
> -static const struct rtc_class_ops ds3232_rtc_ops = {
> -	.read_time = ds3232_read_time,
> -	.set_time = ds3232_set_time,
> -	.read_alarm = ds3232_read_alarm,
> -	.set_alarm = ds3232_set_alarm,
> -	.alarm_irq_enable = ds3232_alarm_irq_enable,
> -};
> -
> -static int ds3232_probe(struct i2c_client *client,
> -			const struct i2c_device_id *id)
> -{
> -	struct ds3232 *ds3232;
> -	int ret;
> -
> -	ds3232 = devm_kzalloc(&client->dev, sizeof(struct ds3232), GFP_KERNEL);
> -	if (!ds3232)
> -		return -ENOMEM;
> -
> -	ds3232->client = client;
> -	i2c_set_clientdata(client, ds3232);
> -
> -	INIT_WORK(&ds3232->work, ds3232_work);
> -	mutex_init(&ds3232->mutex);
> -
> -	ret = ds3232_check_rtc_status(client);
> -	if (ret)
> -		return ret;
> -
> -	if (client->irq > 0) {
> -		ret = devm_request_irq(&client->dev, client->irq, ds3232_irq,
> -				       IRQF_SHARED, "ds3232", client);
> -		if (ret) {
> -			dev_err(&client->dev, "unable to request IRQ\n");
> -		}
> -		device_init_wakeup(&client->dev, 1);
> -	}
> -	ds3232->rtc = devm_rtc_device_register(&client->dev, client->name,
> -					  &ds3232_rtc_ops, THIS_MODULE);
> -	return PTR_ERR_OR_ZERO(ds3232->rtc);
> -}
> -
> -static int ds3232_remove(struct i2c_client *client)
> -{
> -	struct ds3232 *ds3232 = i2c_get_clientdata(client);
> -
> -	if (client->irq > 0) {
> -		mutex_lock(&ds3232->mutex);
> -		ds3232->exiting = 1;
> -		mutex_unlock(&ds3232->mutex);
> -
> -		devm_free_irq(&client->dev, client->irq, client);
> -		cancel_work_sync(&ds3232->work);
> -	}
> -
> -	return 0;
> -}
> -
> -#ifdef CONFIG_PM_SLEEP
> -static int ds3232_suspend(struct device *dev)
> -{
> -	struct ds3232 *ds3232 = dev_get_drvdata(dev);
> -	struct i2c_client *client = to_i2c_client(dev);
> -
> -	if (device_can_wakeup(dev)) {
> -		ds3232->suspended = true;
> -		if (irq_set_irq_wake(client->irq, 1)) {
> -			dev_warn_once(dev, "Cannot set wakeup source\n");
> -			ds3232->suspended = false;
> -		}
> -	}
> -
> -	return 0;
> -}
> -
> -static int ds3232_resume(struct device *dev)
> -{
> -	struct ds3232 *ds3232 = dev_get_drvdata(dev);
> -	struct i2c_client *client = to_i2c_client(dev);
> -
> -	if (ds3232->suspended) {
> -		ds3232->suspended = false;
> -
> -		/* Clear the hardware alarm pend flag */
> -		schedule_work(&ds3232->work);
> -
> -		irq_set_irq_wake(client->irq, 0);
> -	}
> -
> -	return 0;
> -}
> -#endif
> -
> -static const struct dev_pm_ops ds3232_pm_ops = {
> -	SET_SYSTEM_SLEEP_PM_OPS(ds3232_suspend, ds3232_resume)
> -};
> -
> -static const struct i2c_device_id ds3232_id[] = {
> -	{ "ds3232", 0 },
> -	{ }
> -};
> -MODULE_DEVICE_TABLE(i2c, ds3232_id);
> -
> -static struct i2c_driver ds3232_driver = {
> -	.driver = {
> -		.name = "rtc-ds3232",
> -		.pm	= &ds3232_pm_ops,
> -	},
> -	.probe = ds3232_probe,
> -	.remove = ds3232_remove,
> -	.id_table = ds3232_id,
> -};
> -
> -module_i2c_driver(ds3232_driver);
> -
> -MODULE_AUTHOR("Srikanth Srinivasan <srikanth.srinivasan@freescale.com>");
> -MODULE_DESCRIPTION("Maxim/Dallas DS3232 RTC Driver");
> -MODULE_LICENSE("GPL");
> -- 
> 2.5.0
> 

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

-- 
-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[rtc-linux] Re: [PATCH v3 1/8] rtc: ds3232: rename rtc-ds3232.c to rtc-ds3232-core.c for preparation

[Akinobu Mita]

2016-03-04 8:08 GMT+09:00 Alexandre Belloni
<alexandre.belloni@free-electrons.com>:
>
> Hi,
>
> I'm fine with the patches (apart from patch 7/8) but I'd like to request
> one more change. Could actually support both the ds3232 and the ds3234
> in the same file? That will nicely reduce the number of lines and remove
> the need for an include.
>
> For an example, you can have a look at sound/soc/codecs/wm8750.c

Sounds good.  I'll try.
Maybe we also need a trick like CONFIG_SND_SOC_I2C_AND_SPI for rtc.

-- 
-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[rtc-linux] Re: [PATCH v3 7/8] rtc: ds3232-core: fix issue when irq is shared several devices

[Akinobu Mita]

2016-03-04 7:57 GMT+09:00 Alexandre Belloni
<alexandre.belloni@free-electrons.com>:
> On 03/03/2016 at 23:10:35 +0900, Akinobu Mita wrote :
>> 2016-03-01 0:13 GMT+09:00 Akinobu Mita <akinobu.mita@gmail.com>:
>> > ds3232-core requests irq with IRQF_SHARED, so irq can be shared by
>> > several devices.  But the irq handler for ds3232 unconditionally
>> > disables the irq at first and the irq is re-enabled only when the
>> > interrupt source was the ds3232's alarm.  This behaviour breaks the
>> > devices sharing the same irq in the various scenarios.
>> >
>> > Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
>> > Cc: Alessandro Zummo <a.zummo@towertech.it>
>> > Cc: Alexandre Belloni <alexandre.belloni@free-electrons.com>
>> > Cc: Dennis Aberilla <denzzzhome@yahoo.com>
>> > ---
>> > * New patch from this version
>> >
>> >  drivers/rtc/rtc-ds3232-core.c | 14 ++------------
>> >  1 file changed, 2 insertions(+), 12 deletions(-)
>> >
>> > diff --git a/drivers/rtc/rtc-ds3232-core.c b/drivers/rtc/rtc-ds3232-core.c
>> > index be8dfe0..6489380 100644
>> > --- a/drivers/rtc/rtc-ds3232-core.c
>> > +++ b/drivers/rtc/rtc-ds3232-core.c
>> > @@ -59,7 +59,6 @@ struct ds3232 {
>> >          */
>> >         struct mutex mutex;
>> >         bool suspended;
>> > -       int exiting;
>> >  };
>> >
>> >  static int ds3232_check_rtc_status(struct device *dev)
>> > @@ -314,8 +313,6 @@ static irqreturn_t ds3232_irq(int irq, void *dev_id)
>> >         struct device *dev = dev_id;
>> >         struct ds3232 *ds3232 = dev_get_drvdata(dev);
>> >
>> > -       disable_irq_nosync(irq);
>> > -
>> >         /*
>> >          * If rtc as a wakeup source, can't schedule the work
>> >          * at system resume flow, because at this time the i2c bus
>>
>> I found that this patch is not correct because interrupt is re-enabled
>> after this hardirq handler finished without clearing Alarm Flag or
>> Alarm Interrupt Enable in RTC register.
>>
>> Maybe we should drop IRQF_SHARED flag.
>
> I think the best is to completely reworke the irq handling. It dates
> from many many year ago and can be replace by a simple threaded irq.
> This will solve your issue.

I tried converting to threaded irq.  But it needs to deal with suspend
resume paths. (i.e. the following comment in ds3232_irq)

        /*
         * If rtc as a wakeup source, can't schedule the work
         * at system resume flow, because at this time the i2c bus
         * has not been resumed.
         */

Unfortunately, I don't have the computer that can use RTC on i2c-bus
as wake up source.  So I think dropping IRQF_SHARED is safe for now.

-- 
-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[rtc-linux] Re: [PATCH v3 7/8] rtc: ds3232-core: fix issue when irq is shared several devices

[Alexandre Belloni]

On 04/03/2016 at 22:56:01 +0900, Akinobu Mita wrote :
> 2016-03-04 7:57 GMT+09:00 Alexandre Belloni
> <alexandre.belloni@free-electrons.com>:
> > On 03/03/2016 at 23:10:35 +0900, Akinobu Mita wrote :
> >> 2016-03-01 0:13 GMT+09:00 Akinobu Mita <akinobu.mita@gmail.com>:
> >> > ds3232-core requests irq with IRQF_SHARED, so irq can be shared by
> >> > several devices.  But the irq handler for ds3232 unconditionally
> >> > disables the irq at first and the irq is re-enabled only when the
> >> > interrupt source was the ds3232's alarm.  This behaviour breaks the
> >> > devices sharing the same irq in the various scenarios.
> >> >
> >> > Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
> >> > Cc: Alessandro Zummo <a.zummo@towertech.it>
> >> > Cc: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> >> > Cc: Dennis Aberilla <denzzzhome@yahoo.com>
> >> > ---
> >> > * New patch from this version
> >> >
> >> >  drivers/rtc/rtc-ds3232-core.c | 14 ++------------
> >> >  1 file changed, 2 insertions(+), 12 deletions(-)
> >> >
> >> > diff --git a/drivers/rtc/rtc-ds3232-core.c b/drivers/rtc/rtc-ds3232-core.c
> >> > index be8dfe0..6489380 100644
> >> > --- a/drivers/rtc/rtc-ds3232-core.c
> >> > +++ b/drivers/rtc/rtc-ds3232-core.c
> >> > @@ -59,7 +59,6 @@ struct ds3232 {
> >> >          */
> >> >         struct mutex mutex;
> >> >         bool suspended;
> >> > -       int exiting;
> >> >  };
> >> >
> >> >  static int ds3232_check_rtc_status(struct device *dev)
> >> > @@ -314,8 +313,6 @@ static irqreturn_t ds3232_irq(int irq, void *dev_id)
> >> >         struct device *dev = dev_id;
> >> >         struct ds3232 *ds3232 = dev_get_drvdata(dev);
> >> >
> >> > -       disable_irq_nosync(irq);
> >> > -
> >> >         /*
> >> >          * If rtc as a wakeup source, can't schedule the work
> >> >          * at system resume flow, because at this time the i2c bus
> >>
> >> I found that this patch is not correct because interrupt is re-enabled
> >> after this hardirq handler finished without clearing Alarm Flag or
> >> Alarm Interrupt Enable in RTC register.
> >>
> >> Maybe we should drop IRQF_SHARED flag.
> >
> > I think the best is to completely reworke the irq handling. It dates
> > from many many year ago and can be replace by a simple threaded irq.
> > This will solve your issue.
> 
> I tried converting to threaded irq.  But it needs to deal with suspend
> resume paths. (i.e. the following comment in ds3232_irq)
> 
>         /*
>          * If rtc as a wakeup source, can't schedule the work
>          * at system resume flow, because at this time the i2c bus
>          * has not been resumed.
>          */
> 
> Unfortunately, I don't have the computer that can use RTC on i2c-bus
> as wake up source.  So I think dropping IRQF_SHARED is safe for now.

That comment is bogus, you can drop it as well. It may have been the
case a few years ago but that is not true anymore (else a lot of other
RTC would fail).

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

-- 
-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[rtc-linux] Re: [PATCH v3 1/8] rtc: ds3232: rename rtc-ds3232.c to rtc-ds3232-core.c for preparation

[Alexandre Belloni]

On 04/03/2016 at 22:54:39 +0900, Akinobu Mita wrote :
> 2016-03-04 8:08 GMT+09:00 Alexandre Belloni
> <alexandre.belloni@free-electrons.com>:
> >
> > Hi,
> >
> > I'm fine with the patches (apart from patch 7/8) but I'd like to request
> > one more change. Could actually support both the ds3232 and the ds3234
> > in the same file? That will nicely reduce the number of lines and remove
> > the need for an include.
> >
> > For an example, you can have a look at sound/soc/codecs/wm8750.c
> 
> Sounds good.  I'll try.
> Maybe we also need a trick like CONFIG_SND_SOC_I2C_AND_SPI for rtc.

Yes, feel free to add that in the Kconfig and create a new section for
RTCs that are both I2C and SPI if this is needed.

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

-- 
-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.