Add alarm/wakeup support to rtc isl1208 driver
Signed-off-by: Ryan Mallon <[email protected]>
---
diff --git a/drivers/rtc/rtc-isl1208.c b/drivers/rtc/rtc-isl1208.c
index 468200c..94aa4f5 100644
--- a/drivers/rtc/rtc-isl1208.c
+++ b/drivers/rtc/rtc-isl1208.c
@@ -39,6 +39,8 @@
#define ISL1208_REG_SR_BAT (1<<1) /* battery */
#define ISL1208_REG_SR_RTCF (1<<0) /* rtc fail */
#define ISL1208_REG_INT 0x08
+#define ISL1208_REG_INT_ALME (1<<6) /* alarm enable */
+#define ISL1208_REG_INT_IM (1<<7) /* interrupt/alarm mode */
#define ISL1208_REG_09 0x09 /* reserved */
#define ISL1208_REG_ATR 0x0a
#define ISL1208_REG_DTR 0x0b
@@ -202,6 +204,31 @@ isl1208_i2c_set_usr(struct i2c_client *client, u16 usr)
}
static int
+isl1208_rtc_toggle_alarm(struct i2c_client *client, int enable)
+{
+ int icr;
+
+ icr = i2c_smbus_read_byte_data(client, ISL1208_REG_INT);
+ if (icr < 0) {
+ dev_err(&client->dev, "%s: reading INT failed\n", __func__);
+ return icr;
+ }
+
+ if (enable)
+ icr |= ISL1208_REG_INT_ALME | ISL1208_REG_INT_IM;
+ else
+ icr &= ~(ISL1208_REG_INT_ALME | ISL1208_REG_INT_IM);
+
+ icr = i2c_smbus_write_byte_data(client, ISL1208_REG_INT, icr);
+ if (icr < 0) {
+ dev_err(&client->dev, "%s: writing INT failed\n", __func__);
+ return icr;
+ }
+
+ return 0;
+}
+
+static int
isl1208_rtc_proc(struct device *dev, struct seq_file *seq)
{
struct i2c_client *const client = to_i2c_client(dev);
@@ -288,7 +315,7 @@ isl1208_i2c_read_alarm(struct i2c_client *client, struct rtc_wkalrm *alarm)
{
struct rtc_time *const tm = &alarm->time;
u8 regs[ISL1208_ALARM_SECTION_LEN] = { 0, };
- int sr;
+ int icr, yr, sr;
sr = isl1208_i2c_get_sr(client);
if (sr < 0) {
@@ -313,6 +340,73 @@ isl1208_i2c_read_alarm(struct i2c_client *client, struct rtc_wkalrm *alarm)
bcd2bin(regs[ISL1208_REG_MOA - ISL1208_REG_SCA] & 0x1f) - 1;
tm->tm_wday = bcd2bin(regs[ISL1208_REG_DWA - ISL1208_REG_SCA] & 0x03);
+ /* The alarm doesn't store the year so get it from the rtc section */
+ yr = i2c_smbus_read_byte_data(client, ISL1208_REG_YR);
+ if (yr < 0) {
+ dev_err(&client->dev, "%s: reading RTC YR failed\n", __func__);
+ return yr;
+ }
+ tm->tm_year = bcd2bin(yr) + 100;
+
+ icr = i2c_smbus_read_byte_data(client, ISL1208_REG_INT);
+ if (icr < 0) {
+ dev_err(&client->dev, "%s: reading INT failed\n", __func__);
+ return icr;
+ }
+ alarm->enabled = !!(icr & ISL1208_REG_INT_ALME);
+
+ return 0;
+}
+
+static int
+isl1208_i2c_set_alarm(struct i2c_client *client, struct rtc_wkalrm *alarm)
+{
+ struct rtc_time *alarm_tm = &alarm->time;
+ u8 regs[ISL1208_ALARM_SECTION_LEN] = { 0, };
+ const int offs = ISL1208_REG_SCA;
+ unsigned long rtc_secs, alarm_secs;
+ struct rtc_time rtc_tm;
+ int err, enable;
+
+ err = isl1208_i2c_read_time(client, &rtc_tm);
+ if (err)
+ return err;
+ err = rtc_tm_to_time(&rtc_tm, &rtc_secs);
+ if (err)
+ return err;
+ err = rtc_tm_to_time(alarm_tm, &alarm_secs);
+ if (err)
+ return err;
+
+ /* If the alarm time is before the current time disable the alarm */
+ if (!alarm->enabled || alarm_secs <= rtc_secs)
+ enable = 0x00;
+ else
+ enable = 0x80;
+
+ /* Program the alarm and enable it for each setting */
+ regs[ISL1208_REG_SCA - offs] = bin2bcd(alarm_tm->tm_sec) | enable;
+ regs[ISL1208_REG_MNA - offs] = bin2bcd(alarm_tm->tm_min) | enable;
+ regs[ISL1208_REG_HRA - offs] = bin2bcd(alarm_tm->tm_hour) |
+ ISL1208_REG_HR_MIL | enable;
+
+ regs[ISL1208_REG_DTA - offs] = bin2bcd(alarm_tm->tm_mday) | enable;
+ regs[ISL1208_REG_MOA - offs] = bin2bcd(alarm_tm->tm_mon + 1) | enable;
+ regs[ISL1208_REG_DWA - offs] = bin2bcd(alarm_tm->tm_wday & 7) | enable;
+
+ /* write ALARM registers */
+ err = isl1208_i2c_set_regs(client, offs, regs,
+ ISL1208_ALARM_SECTION_LEN);
+ if (err < 0) {
+ dev_err(&client->dev, "%s: writing ALARM section failed\n",
+ __func__);
+ return err;
+ }
+
+ err = isl1208_rtc_toggle_alarm(client, enable);
+ if (err)
+ return err;
+
return 0;
}
@@ -391,12 +485,63 @@ isl1208_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alarm)
return isl1208_i2c_read_alarm(to_i2c_client(dev), alarm);
}
+static int
+isl1208_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alarm)
+{
+ return isl1208_i2c_set_alarm(to_i2c_client(dev), alarm);
+}
+
+static irqreturn_t
+isl1208_rtc_interrupt(int irq, void *data)
+{
+ unsigned long timeout = jiffies + msecs_to_jiffies(1000);
+ struct i2c_client *client = data;
+ int handled = 0, sr, err;
+
+ /*
+ * I2C reads get NAK'ed if we read straight away after an interrupt?
+ * Using a mdelay/msleep didn't seem to help either, so we work around
+ * this by continually trying to read the register for a short time.
+ */
+ while (1) {
+ sr = isl1208_i2c_get_sr(client);
+ if (sr >= 0)
+ break;
+
+ if (time_after(jiffies, timeout)) {
+ dev_dbg(&client->dev, "%s: reading SR failed\n",
+ __func__);
+ return sr;
+ }
+ }
+
+ if (sr & ISL1208_REG_SR_ALM) {
+ dev_dbg(&client->dev, "alarm!\n");
+
+ /* Clear the alarm */
+ sr &= ~ISL1208_REG_SR_ALM;
+ sr = i2c_smbus_write_byte_data(client, ISL1208_REG_SR, sr);
+ if (sr < 0)
+ dev_err(&client->dev, "%s: writing SR failed\n",
+ __func__);
+ else
+ handled = 1;
+
+ /* Disable the alarm */
+ err = isl1208_rtc_toggle_alarm(client, 0);
+ if (err)
+ return err;
+ }
+
+ return handled ? IRQ_HANDLED : IRQ_NONE;
+}
+
static const struct rtc_class_ops isl1208_rtc_ops = {
.proc = isl1208_rtc_proc,
.read_time = isl1208_rtc_read_time,
.set_time = isl1208_rtc_set_time,
.read_alarm = isl1208_rtc_read_alarm,
- /*.set_alarm = isl1208_rtc_set_alarm, */
+ .set_alarm = isl1208_rtc_set_alarm,
};
/* sysfs interface */
@@ -488,11 +633,29 @@ isl1208_probe(struct i2c_client *client, const struct i2c_device_id *id)
dev_info(&client->dev,
"chip found, driver version " DRV_VERSION "\n");
+ if (client->irq > 0) {
+ rc = request_threaded_irq(client->irq, NULL,
+ isl1208_rtc_interrupt,
+ IRQF_SHARED,
+ isl1208_driver.driver.name, client);
+ if (!rc) {
+ device_init_wakeup(&client->dev, 1);
+ enable_irq_wake(client->irq);
+ } else {
+ dev_err(&client->dev,
+ "Unable to request irq %d, no alarm support\n",
+ client->irq);
+ client->irq = 0;
+ }
+ }
+
rtc = rtc_device_register(isl1208_driver.driver.name,
&client->dev, &isl1208_rtc_ops,
THIS_MODULE);
- if (IS_ERR(rtc))
- return PTR_ERR(rtc);
+ if (IS_ERR(rtc)) {
+ rc = PTR_ERR(rtc);
+ goto exit_free_irq;
+ }
i2c_set_clientdata(client, rtc);
@@ -514,6 +677,9 @@ isl1208_probe(struct i2c_client *client, const struct i2c_device_id *id)
exit_unregister:
rtc_device_unregister(rtc);
+exit_free_irq:
+ if (client->irq)
+ free_irq(client->irq, client);
return rc;
}
@@ -525,6 +691,8 @@ isl1208_remove(struct i2c_client *client)
sysfs_remove_group(&client->dev.kobj, &isl1208_rtc_sysfs_files);
rtc_device_unregister(rtc);
+ if (client->irq)
+ free_irq(client->irq, client);
return 0;
}
On Mon, 21 Feb 2011, Ryan Mallon wrote:
> Add alarm/wakeup support to rtc isl1208 driver
>
A few (very minor) comments below.
> Signed-off-by: Ryan Mallon <[email protected]>
> ---
>
> diff --git a/drivers/rtc/rtc-isl1208.c b/drivers/rtc/rtc-isl1208.c
> index 468200c..94aa4f5 100644
> --- a/drivers/rtc/rtc-isl1208.c
> +++ b/drivers/rtc/rtc-isl1208.c
> @@ -39,6 +39,8 @@
> #define ISL1208_REG_SR_BAT (1<<1) /* battery */
> #define ISL1208_REG_SR_RTCF (1<<0) /* rtc fail */
> #define ISL1208_REG_INT 0x08
> +#define ISL1208_REG_INT_ALME (1<<6) /* alarm enable */
> +#define ISL1208_REG_INT_IM (1<<7) /* interrupt/alarm mode */
> #define ISL1208_REG_09 0x09 /* reserved */
> #define ISL1208_REG_ATR 0x0a
> #define ISL1208_REG_DTR 0x0b
> @@ -202,6 +204,31 @@ isl1208_i2c_set_usr(struct i2c_client *client, u16 usr)
> }
>
> static int
> +isl1208_rtc_toggle_alarm(struct i2c_client *client, int enable)
> +{
> + int icr;
> +
> + icr = i2c_smbus_read_byte_data(client, ISL1208_REG_INT);
int icr = i2c_smbus_read_byte_data(client, ISL1208_REG_INT);
??
No functional change, but cuts down on source size.
> + if (icr < 0) {
> + dev_err(&client->dev, "%s: reading INT failed\n", __func__);
> + return icr;
> + }
> +
> + if (enable)
> + icr |= ISL1208_REG_INT_ALME | ISL1208_REG_INT_IM;
> + else
> + icr &= ~(ISL1208_REG_INT_ALME | ISL1208_REG_INT_IM);
> +
> + icr = i2c_smbus_write_byte_data(client, ISL1208_REG_INT, icr);
> + if (icr < 0) {
> + dev_err(&client->dev, "%s: writing INT failed\n", __func__);
> + return icr;
> + }
> +
> + return 0;
> +}
> +
> +static int
> isl1208_rtc_proc(struct device *dev, struct seq_file *seq)
> {
> struct i2c_client *const client = to_i2c_client(dev);
> @@ -288,7 +315,7 @@ isl1208_i2c_read_alarm(struct i2c_client *client, struct rtc_wkalrm *alarm)
> {
> struct rtc_time *const tm = &alarm->time;
> u8 regs[ISL1208_ALARM_SECTION_LEN] = { 0, };
> - int sr;
> + int icr, yr, sr;
>
> sr = isl1208_i2c_get_sr(client);
Maybe it's just me, but I'd have written this as
int icr, yr;
int sr = isl1208_i2c_get_sr(client);
> if (sr < 0) {
> @@ -313,6 +340,73 @@ isl1208_i2c_read_alarm(struct i2c_client *client, struct rtc_wkalrm *alarm)
> bcd2bin(regs[ISL1208_REG_MOA - ISL1208_REG_SCA] & 0x1f) - 1;
> tm->tm_wday = bcd2bin(regs[ISL1208_REG_DWA - ISL1208_REG_SCA] & 0x03);
>
> + /* The alarm doesn't store the year so get it from the rtc section */
> + yr = i2c_smbus_read_byte_data(client, ISL1208_REG_YR);
> + if (yr < 0) {
> + dev_err(&client->dev, "%s: reading RTC YR failed\n", __func__);
> + return yr;
> + }
> + tm->tm_year = bcd2bin(yr) + 100;
> +
> + icr = i2c_smbus_read_byte_data(client, ISL1208_REG_INT);
> + if (icr < 0) {
> + dev_err(&client->dev, "%s: reading INT failed\n", __func__);
> + return icr;
> + }
> + alarm->enabled = !!(icr & ISL1208_REG_INT_ALME);
> +
> + return 0;
> +}
> +
> +static int
> +isl1208_i2c_set_alarm(struct i2c_client *client, struct rtc_wkalrm *alarm)
> +{
> + struct rtc_time *alarm_tm = &alarm->time;
> + u8 regs[ISL1208_ALARM_SECTION_LEN] = { 0, };
> + const int offs = ISL1208_REG_SCA;
> + unsigned long rtc_secs, alarm_secs;
> + struct rtc_time rtc_tm;
> + int err, enable;
> +
> + err = isl1208_i2c_read_time(client, &rtc_tm);
> + if (err)
> + return err;
> + err = rtc_tm_to_time(&rtc_tm, &rtc_secs);
> + if (err)
> + return err;
> + err = rtc_tm_to_time(alarm_tm, &alarm_secs);
> + if (err)
> + return err;
> +
> + /* If the alarm time is before the current time disable the alarm */
> + if (!alarm->enabled || alarm_secs <= rtc_secs)
> + enable = 0x00;
> + else
> + enable = 0x80;
> +
> + /* Program the alarm and enable it for each setting */
> + regs[ISL1208_REG_SCA - offs] = bin2bcd(alarm_tm->tm_sec) | enable;
> + regs[ISL1208_REG_MNA - offs] = bin2bcd(alarm_tm->tm_min) | enable;
> + regs[ISL1208_REG_HRA - offs] = bin2bcd(alarm_tm->tm_hour) |
> + ISL1208_REG_HR_MIL | enable;
> +
> + regs[ISL1208_REG_DTA - offs] = bin2bcd(alarm_tm->tm_mday) | enable;
> + regs[ISL1208_REG_MOA - offs] = bin2bcd(alarm_tm->tm_mon + 1) | enable;
> + regs[ISL1208_REG_DWA - offs] = bin2bcd(alarm_tm->tm_wday & 7) | enable;
> +
> + /* write ALARM registers */
> + err = isl1208_i2c_set_regs(client, offs, regs,
> + ISL1208_ALARM_SECTION_LEN);
> + if (err < 0) {
> + dev_err(&client->dev, "%s: writing ALARM section failed\n",
> + __func__);
> + return err;
> + }
> +
> + err = isl1208_rtc_toggle_alarm(client, enable);
> + if (err)
> + return err;
> +
> return 0;
> }
>
> @@ -391,12 +485,63 @@ isl1208_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alarm)
> return isl1208_i2c_read_alarm(to_i2c_client(dev), alarm);
> }
>
> +static int
> +isl1208_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alarm)
> +{
> + return isl1208_i2c_set_alarm(to_i2c_client(dev), alarm);
> +}
> +
> +static irqreturn_t
> +isl1208_rtc_interrupt(int irq, void *data)
> +{
> + unsigned long timeout = jiffies + msecs_to_jiffies(1000);
> + struct i2c_client *client = data;
> + int handled = 0, sr, err;
> +
> + /*
> + * I2C reads get NAK'ed if we read straight away after an interrupt?
> + * Using a mdelay/msleep didn't seem to help either, so we work around
> + * this by continually trying to read the register for a short time.
> + */
> + while (1) {
> + sr = isl1208_i2c_get_sr(client);
> + if (sr >= 0)
> + break;
> +
> + if (time_after(jiffies, timeout)) {
> + dev_dbg(&client->dev, "%s: reading SR failed\n",
> + __func__);
> + return sr;
> + }
> + }
> +
> + if (sr & ISL1208_REG_SR_ALM) {
> + dev_dbg(&client->dev, "alarm!\n");
> +
> + /* Clear the alarm */
> + sr &= ~ISL1208_REG_SR_ALM;
> + sr = i2c_smbus_write_byte_data(client, ISL1208_REG_SR, sr);
> + if (sr < 0)
> + dev_err(&client->dev, "%s: writing SR failed\n",
> + __func__);
> + else
> + handled = 1;
> +
> + /* Disable the alarm */
> + err = isl1208_rtc_toggle_alarm(client, 0);
> + if (err)
> + return err;
> + }
> +
> + return handled ? IRQ_HANDLED : IRQ_NONE;
> +}
> +
> static const struct rtc_class_ops isl1208_rtc_ops = {
> .proc = isl1208_rtc_proc,
> .read_time = isl1208_rtc_read_time,
> .set_time = isl1208_rtc_set_time,
> .read_alarm = isl1208_rtc_read_alarm,
> - /*.set_alarm = isl1208_rtc_set_alarm, */
> + .set_alarm = isl1208_rtc_set_alarm,
> };
>
> /* sysfs interface */
> @@ -488,11 +633,29 @@ isl1208_probe(struct i2c_client *client, const struct i2c_device_id *id)
> dev_info(&client->dev,
> "chip found, driver version " DRV_VERSION "\n");
>
> + if (client->irq > 0) {
> + rc = request_threaded_irq(client->irq, NULL,
> + isl1208_rtc_interrupt,
> + IRQF_SHARED,
> + isl1208_driver.driver.name, client);
> + if (!rc) {
> + device_init_wakeup(&client->dev, 1);
> + enable_irq_wake(client->irq);
> + } else {
> + dev_err(&client->dev,
> + "Unable to request irq %d, no alarm support\n",
> + client->irq);
> + client->irq = 0;
> + }
> + }
> +
> rtc = rtc_device_register(isl1208_driver.driver.name,
> &client->dev, &isl1208_rtc_ops,
> THIS_MODULE);
> - if (IS_ERR(rtc))
> - return PTR_ERR(rtc);
> + if (IS_ERR(rtc)) {
> + rc = PTR_ERR(rtc);
> + goto exit_free_irq;
> + }
>
> i2c_set_clientdata(client, rtc);
>
> @@ -514,6 +677,9 @@ isl1208_probe(struct i2c_client *client, const struct i2c_device_id *id)
>
> exit_unregister:
> rtc_device_unregister(rtc);
> +exit_free_irq:
> + if (client->irq)
> + free_irq(client->irq, client);
I didn't dig deep into this, but from a cursory look it seems that
free_irq() should be fine being handed NULL. If that's right, then the
condifional "if" is not needed here..
>
> return rc;
> }
> @@ -525,6 +691,8 @@ isl1208_remove(struct i2c_client *client)
>
> sysfs_remove_group(&client->dev.kobj, &isl1208_rtc_sysfs_files);
> rtc_device_unregister(rtc);
> + if (client->irq)
> + free_irq(client->irq, client);
>
> return 0;
> }
>
--
Jesper Juhl <[email protected]> http://www.chaosbits.net/
Plain text mails only, please.
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
On 02/21/2011 12:52 PM, Jesper Juhl wrote:
> On Mon, 21 Feb 2011, Ryan Mallon wrote:
>
>> Add alarm/wakeup support to rtc isl1208 driver
>>
>
> A few (very minor) comments below.
Hi Jepser. Thanks for the review. See answers below.
I'm unsure where this patch is meant to be applied. There is a list and
a maintainer (but no git tree?) for the rtc subsystem, but there doesn't
seem to be much activity from either lately?
>
>
>> Signed-off-by: Ryan Mallon <[email protected]>
>> ---
>>
>> diff --git a/drivers/rtc/rtc-isl1208.c b/drivers/rtc/rtc-isl1208.c
>> static int
>> +isl1208_rtc_toggle_alarm(struct i2c_client *client, int enable)
>> +{
>> + int icr;
>> +
>> + icr = i2c_smbus_read_byte_data(client, ISL1208_REG_INT);
>
> int icr = i2c_smbus_read_byte_data(client, ISL1208_REG_INT);
>
> ??
>
> No functional change, but cuts down on source size.
Sure, I can change this.
>> +static int
>> isl1208_rtc_proc(struct device *dev, struct seq_file *seq)
>> {
>> struct i2c_client *const client = to_i2c_client(dev);
>> @@ -288,7 +315,7 @@ isl1208_i2c_read_alarm(struct i2c_client *client, struct rtc_wkalrm *alarm)
>> {
>> struct rtc_time *const tm = &alarm->time;
>> u8 regs[ISL1208_ALARM_SECTION_LEN] = { 0, };
>> - int sr;
>> + int icr, yr, sr;
>>
>> sr = isl1208_i2c_get_sr(client);
>
> Maybe it's just me, but I'd have written this as
>
> int icr, yr;
> int sr = isl1208_i2c_get_sr(client);
Same here. I'm not hugely fussed either way.
>> @@ -514,6 +677,9 @@ isl1208_probe(struct i2c_client *client, const struct i2c_device_id *id)
>>
>> exit_unregister:
>> rtc_device_unregister(rtc);
>> +exit_free_irq:
>> + if (client->irq)
>> + free_irq(client->irq, client);
>
> I didn't dig deep into this, but from a cursory look it seems that
> free_irq() should be fine being handed NULL. If that's right, then the
> condifional "if" is not needed here..
I think having the if is a bit clearer, since it will only try and free
the irq in the case where we have actually allocated it. This is on the
error exit path so performance is not exactly critical.
~Ryan
--
Bluewater Systems Ltd - ARM Technology Solution Centre
Ryan Mallon 5 Amuri Park, 404 Barbadoes St
[email protected] PO Box 13 889, Christchurch 8013
http://www.bluewatersys.com New Zealand
Phone: +64 3 3779127 Freecall: Australia 1800 148 751
Fax: +64 3 3779135 USA 1800 261 2934