diff mbox

[v2] rtc: fixes and new functionality for fm3130

Message ID 20100517230918.464a6db0@realm
State Superseded
Headers show

Commit Message

Sergey Matyukevich May 17, 2010, 7:09 p.m. UTC 
Hello Alessandro,

Could you please consider the following patch for fm3130 driver.
It is marked as 'v2' but there is no actual differences between
'v2' and 'v1'. The first version was lost deep in the rtc mail-list
with no comments/replies. So I repost that patch after rebase with
respect to the current upstream kernel.

This patch contains the following fixes and enhancements:

	* add sanity check for alarm data in fm3130_probe

	* fix fm3130_set_alarm:
		according to datasheet, setting match bit '0' indicates
		that the corresponding alarm field will be used in the
		match process

	* add operation alarm_irq_enable operation which is responsible for
		handling RTC_AIE_ON, RTC_AIE_OFF ioctls

	* remove clearing of AF bit after reading rtc/alarm control register:
		according to datasheet this bit is cleared
		anyway when rtc/alarm control register is read

Signed-off-by: Sergey Matyukevich <geomatsi@gmail.com>
---
 drivers/rtc/rtc-fm3130.c |  179 +++++++++++++++++++++++++++++++++-------------
 1 files changed, 128 insertions(+), 51 deletions(-)

Comments

Sergey Lapin May 17, 2010, 7:24 p.m. UTC | #1 
On Mon, May 17, 2010 at 11:09:18PM +0400, Sergey Matyukevich wrote:
> Hello Alessandro,
> 
> Could you please consider the following patch for fm3130 driver.
> It is marked as 'v2' but there is no actual differences between
> 'v2' and 'v1'. The first version was lost deep in the rtc mail-list
> with no comments/replies. So I repost that patch after rebase with
> respect to the current upstream kernel.
> 
> This patch contains the following fixes and enhancements:
> 
> 	* add sanity check for alarm data in fm3130_probe
> 
> 	* fix fm3130_set_alarm:
> 		according to datasheet, setting match bit '0' indicates
> 		that the corresponding alarm field will be used in the
> 		match process
> 
> 	* add operation alarm_irq_enable operation which is responsible for
> 		handling RTC_AIE_ON, RTC_AIE_OFF ioctls
> 
> 	* remove clearing of AF bit after reading rtc/alarm control register:
> 		according to datasheet this bit is cleared
> 		anyway when rtc/alarm control register is read
> 
> Signed-off-by: Sergey Matyukevich <geomatsi@gmail.com>
Acked-by: Sergey Lapin <slapin@ossfans.org>
Sergey Matyukevich June 2, 2010, 2:24 p.m. UTC | #2 
Hello Alessandro,

Any update regarding this patch ?
We have already got an ACK from author.

Thanks,
Sergey

2010/5/17 Sergey Matyukevich <geomatsi@gmail.com>

> Hello Alessandro,
>
> Could you please consider the following patch for fm3130 driver.
> It is marked as 'v2' but there is no actual differences between
> 'v2' and 'v1'. The first version was lost deep in the rtc mail-list
> with no comments/replies. So I repost that patch after rebase with
> respect to the current upstream kernel.
>
> This patch contains the following fixes and enhancements:
>
>        * add sanity check for alarm data in fm3130_probe
>
>        * fix fm3130_set_alarm:
>                according to datasheet, setting match bit '0' indicates
>                that the corresponding alarm field will be used in the
>                match process
>
>        * add operation alarm_irq_enable operation which is responsible for
>                handling RTC_AIE_ON, RTC_AIE_OFF ioctls
>
>        * remove clearing of AF bit after reading rtc/alarm control
> register:
>                according to datasheet this bit is cleared
>                anyway when rtc/alarm control register is read
>
> Signed-off-by: Sergey Matyukevich <geomatsi@gmail.com>
> ---
>  drivers/rtc/rtc-fm3130.c |  179
> +++++++++++++++++++++++++++++++++-------------
>  1 files changed, 128 insertions(+), 51 deletions(-)
>
>
Wan ZongShun June 2, 2010, 2:44 p.m. UTC | #3 
2010/6/2 Sergey Matyukevich <geomatsi@gmail.com>:
> Hello Alessandro,
>
> Any update regarding this patch ?
> We have already got an ACK from author.
>

It looks good to me too.

Acked-by: Wan ZongShun <mcuos.com@gmail.com>

Please resend your patch to Andrew, his email:

 "Andrew Morton" <akpm@linux-foundation.org>

Hi Andrew ,
Could you help merge this patch? thanks!


> Thanks,
> Sergey
>
> 2010/5/17 Sergey Matyukevich <geomatsi@gmail.com>
>>
>> Hello Alessandro,
>>
>> Could you please consider the following patch for fm3130 driver.
>> It is marked as 'v2' but there is no actual differences between
>> 'v2' and 'v1'. The first version was lost deep in the rtc mail-list
>> with no comments/replies. So I repost that patch after rebase with
>> respect to the current upstream kernel.
>>
>> This patch contains the following fixes and enhancements:
>>
>>        * add sanity check for alarm data in fm3130_probe
>>
>>        * fix fm3130_set_alarm:
>>                according to datasheet, setting match bit '0' indicates
>>                that the corresponding alarm field will be used in the
>>                match process
>>
>>        * add operation alarm_irq_enable operation which is responsible for
>>                handling RTC_AIE_ON, RTC_AIE_OFF ioctls
>>
>>        * remove clearing of AF bit after reading rtc/alarm control
>> register:
>>                according to datasheet this bit is cleared
>>                anyway when rtc/alarm control register is read
>>
>> Signed-off-by: Sergey Matyukevich <geomatsi@gmail.com>
>> ---
>>  drivers/rtc/rtc-fm3130.c |  179
>> +++++++++++++++++++++++++++++++++-------------
>>  1 files changed, 128 insertions(+), 51 deletions(-)
>>
> --
> 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.
diff mbox

Patch

diff --git a/drivers/rtc/rtc-fm3130.c b/drivers/rtc/rtc-fm3130.c
index ff6fce6..44ffbf2 100644
--- a/drivers/rtc/rtc-fm3130.c
+++ b/drivers/rtc/rtc-fm3130.c
@@ -52,8 +52,8 @@  struct fm3130 {
 	struct i2c_msg		msg[4];
 	struct i2c_client	*client;
 	struct rtc_device	*rtc;
+	int			alarm_valid;
 	int			data_valid;
-	int			alarm;
 };
 static const struct i2c_device_id fm3130_id[] = {
 	{ "fm3130", 0 },
@@ -87,11 +87,7 @@  static void fm3130_rtc_mode(struct device *dev, int mode)
 		dev_dbg(dev, "invalid mode %d\n", mode);
 		break;
 	}
-	/* Checking for alarm */
-	if (fm3130->regs[FM3130_RTC_CONTROL] & FM3130_RTC_CONTROL_BIT_AF) {
-		fm3130->alarm = 1;
-		fm3130->regs[FM3130_RTC_CONTROL] &= ~FM3130_RTC_CONTROL_BIT_AF;
-	}
+
 	i2c_smbus_write_byte_data(fm3130->client,
 		 FM3130_RTC_CONTROL, fm3130->regs[FM3130_RTC_CONTROL]);
 }
@@ -208,6 +204,15 @@  static int fm3130_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
 	struct fm3130 *fm3130 = dev_get_drvdata(dev);
 	int tmp;
 	struct rtc_time *tm = &alrm->time;
+
+	if (!fm3130->alarm_valid) {
+		/* We have invalid alarm in RTC, probably due
+		to battery faults or other problems. Return EIO
+		for now, it will allow us to set alarm value later
+		instead of error during probing which disables device */
+		return -EIO;
+	}
+
 	/* read the RTC alarm registers all at once */
 	tmp = i2c_transfer(to_i2c_adapter(fm3130->client->dev.parent),
 			&fm3130->msg[2], 2);
@@ -222,20 +227,31 @@  static int fm3130_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
 			fm3130->regs[FM3130_ALARM_DATE],
 			fm3130->regs[FM3130_ALARM_MONTHS]);
 
-
 	tm->tm_sec	= bcd2bin(fm3130->regs[FM3130_ALARM_SECONDS] & 0x7F);
 	tm->tm_min	= bcd2bin(fm3130->regs[FM3130_ALARM_MINUTES] & 0x7F);
 	tm->tm_hour	= bcd2bin(fm3130->regs[FM3130_ALARM_HOURS] & 0x3F);
 	tm->tm_mday	= bcd2bin(fm3130->regs[FM3130_ALARM_DATE] & 0x3F);
 	tm->tm_mon	= bcd2bin(fm3130->regs[FM3130_ALARM_MONTHS] & 0x1F);
+
 	if (tm->tm_mon > 0)
 		tm->tm_mon -= 1; /* RTC is 1-12, tm_mon is 0-11 */
+
 	dev_dbg(dev, "%s secs=%d, mins=%d, "
 		"hours=%d, mday=%d, mon=%d, year=%d, wday=%d\n",
 		"read alarm", tm->tm_sec, tm->tm_min,
 		tm->tm_hour, tm->tm_mday,
 		tm->tm_mon, tm->tm_year, tm->tm_wday);
 
+	/* check if alarm enabled */
+	fm3130->regs[FM3130_RTC_CONTROL] =
+		i2c_smbus_read_byte_data(fm3130->client, FM3130_RTC_CONTROL);
+
+	if ((fm3130->regs[FM3130_RTC_CONTROL] & FM3130_RTC_CONTROL_BIT_AEN) &&
+		(~fm3130->regs[FM3130_RTC_CONTROL] &
+			FM3130_RTC_CONTROL_BIT_CAL)) {
+		alrm->enabled = 1;
+	}
+
 	return 0;
 }
 
@@ -251,25 +267,20 @@  static int fm3130_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
 		tm->tm_hour, tm->tm_mday,
 		tm->tm_mon, tm->tm_year, tm->tm_wday);
 
-	if (tm->tm_sec != -1)
-		fm3130->regs[FM3130_ALARM_SECONDS] =
-			bin2bcd(tm->tm_sec) | 0x80;
+	fm3130->regs[FM3130_ALARM_SECONDS] =
+		(tm->tm_sec != -1) ? bin2bcd(tm->tm_sec) : 0x80;
 
-	if (tm->tm_min != -1)
-		fm3130->regs[FM3130_ALARM_MINUTES] =
-			bin2bcd(tm->tm_min) | 0x80;
+	fm3130->regs[FM3130_ALARM_MINUTES] =
+		(tm->tm_min != -1) ? bin2bcd(tm->tm_min) : 0x80;
 
-	if (tm->tm_hour != -1)
-		fm3130->regs[FM3130_ALARM_HOURS] =
-			bin2bcd(tm->tm_hour) | 0x80;
+	fm3130->regs[FM3130_ALARM_HOURS] =
+		(tm->tm_hour != -1) ? bin2bcd(tm->tm_hour) : 0x80;
 
-	if (tm->tm_mday != -1)
-		fm3130->regs[FM3130_ALARM_DATE] =
-			bin2bcd(tm->tm_mday) | 0x80;
+	fm3130->regs[FM3130_ALARM_DATE] =
+		(tm->tm_mday != -1) ? bin2bcd(tm->tm_mday) : 0x80;
 
-	if (tm->tm_mon != -1)
-		fm3130->regs[FM3130_ALARM_MONTHS] =
-			bin2bcd(tm->tm_mon + 1) | 0x80;
+	fm3130->regs[FM3130_ALARM_MONTHS] =
+		(tm->tm_mon != -1) ? bin2bcd(tm->tm_mon + 1) : 0x80;
 
 	dev_dbg(dev, "alarm write %02x %02x %02x %02x %02x\n",
 			fm3130->regs[FM3130_ALARM_SECONDS],
@@ -285,11 +296,8 @@  static int fm3130_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
 	}
 	fm3130->regs[FM3130_RTC_CONTROL] =
 		i2c_smbus_read_byte_data(fm3130->client, FM3130_RTC_CONTROL);
-	/* Checking for alarm */
-	if (fm3130->regs[FM3130_RTC_CONTROL] & FM3130_RTC_CONTROL_BIT_AF) {
-		fm3130->alarm = 1;
-		fm3130->regs[FM3130_RTC_CONTROL] &= ~FM3130_RTC_CONTROL_BIT_AF;
-	}
+
+	/* enable or disable alarm */
 	if (alrm->enabled) {
 		i2c_smbus_write_byte_data(fm3130->client, FM3130_RTC_CONTROL,
 			(fm3130->regs[FM3130_RTC_CONTROL] &
@@ -298,16 +306,55 @@  static int fm3130_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
 	} else {
 		i2c_smbus_write_byte_data(fm3130->client, FM3130_RTC_CONTROL,
 			fm3130->regs[FM3130_RTC_CONTROL] &
-				~(FM3130_RTC_CONTROL_BIT_AEN));
+				~(FM3130_RTC_CONTROL_BIT_CAL) &
+					~(FM3130_RTC_CONTROL_BIT_AEN));
 	}
+
+	/* We assume here that data is valid once written */
+	if (!fm3130->alarm_valid)
+		fm3130->alarm_valid = 1;
+
 	return 0;
 }
 
+int fm3130_alarm_irq_enable(struct device *dev, unsigned int enabled)
+{
+	struct fm3130 *fm3130 = dev_get_drvdata(dev);
+	int ret = 0;
+
+	fm3130->regs[FM3130_RTC_CONTROL] =
+		i2c_smbus_read_byte_data(fm3130->client, FM3130_RTC_CONTROL);
+
+	dev_dbg(dev, "alarm_irq_enable: enable=%d, FM3130_RTC_CONTROL=%02x\n",
+		enabled, fm3130->regs[FM3130_RTC_CONTROL]);
+
+	switch (enabled) {
+	case 0:		/* alarm off */
+		ret = i2c_smbus_write_byte_data(fm3130->client,
+			FM3130_RTC_CONTROL, fm3130->regs[FM3130_RTC_CONTROL] &
+				~(FM3130_RTC_CONTROL_BIT_CAL) &
+					~(FM3130_RTC_CONTROL_BIT_AEN));
+		break;
+	case 1:		/* alarm on */
+		ret = i2c_smbus_write_byte_data(fm3130->client,
+			FM3130_RTC_CONTROL, (fm3130->regs[FM3130_RTC_CONTROL] &
+				~(FM3130_RTC_CONTROL_BIT_CAL)) |
+					FM3130_RTC_CONTROL_BIT_AEN);
+		break;
+	default:
+		ret = -EINVAL;
+		break;
+	}
+
+	return ret;
+}
+
 static const struct rtc_class_ops fm3130_rtc_ops = {
 	.read_time	= fm3130_get_time,
 	.set_time	= fm3130_set_time,
 	.read_alarm	= fm3130_read_alarm,
 	.set_alarm	= fm3130_set_alarm,
+	.alarm_irq_enable = fm3130_alarm_irq_enable,
 };
 
 static struct i2c_driver fm3130_driver;
@@ -356,6 +403,7 @@  static int __devinit fm3130_probe(struct i2c_client *client,
 	fm3130->msg[3].len = FM3130_ALARM_REGS;
 	fm3130->msg[3].buf = &fm3130->regs[FM3130_ALARM_SECONDS];
 
+	fm3130->alarm_valid = 0;
 	fm3130->data_valid = 0;
 
 	tmp = i2c_transfer(adapter, fm3130->msg, 4);
@@ -370,12 +418,6 @@  static int __devinit fm3130_probe(struct i2c_client *client,
 	fm3130->regs[FM3130_CAL_CONTROL] =
 		i2c_smbus_read_byte_data(client, FM3130_CAL_CONTROL);
 
-	/* Checking for alarm */
-	if (fm3130->regs[FM3130_RTC_CONTROL] & FM3130_RTC_CONTROL_BIT_AF) {
-		fm3130->alarm = 1;
-		fm3130->regs[FM3130_RTC_CONTROL] &= ~FM3130_RTC_CONTROL_BIT_AF;
-	}
-
 	/* Disabling calibration mode */
 	if (fm3130->regs[FM3130_RTC_CONTROL] & FM3130_RTC_CONTROL_BIT_CAL) {
 		i2c_smbus_write_byte_data(client, FM3130_RTC_CONTROL,
@@ -400,44 +442,79 @@  static int __devinit fm3130_probe(struct i2c_client *client,
 			fm3130->regs[FM3130_CAL_CONTROL] &
 				~(FM3130_CAL_CONTROL_BIT_nOSCEN));
 
-	/* oscillator fault?  clear flag, and warn */
-	if (fm3130->regs[FM3130_RTC_CONTROL] & FM3130_RTC_CONTROL_BIT_LB)
+	/* low battery?  clear flag, and warn */
+	if (fm3130->regs[FM3130_RTC_CONTROL] & FM3130_RTC_CONTROL_BIT_LB) {
+		i2c_smbus_write_byte_data(client, FM3130_RTC_CONTROL,
+			fm3130->regs[FM3130_RTC_CONTROL] &
+				~(FM3130_RTC_CONTROL_BIT_LB));
 		dev_warn(&client->dev, "Low battery!\n");
+	}
 
-	/* oscillator fault?  clear flag, and warn */
+	/* check if Power On Reset bit is set */
 	if (fm3130->regs[FM3130_RTC_CONTROL] & FM3130_RTC_CONTROL_BIT_POR) {
 		i2c_smbus_write_byte_data(client, FM3130_RTC_CONTROL,
 			fm3130->regs[FM3130_RTC_CONTROL] &
 				~FM3130_RTC_CONTROL_BIT_POR);
-		dev_warn(&client->dev, "SET TIME!\n");
+		dev_dbg(&client->dev, "POR bit is set\n");
 	}
 	/* ACS is controlled by alarm */
 	i2c_smbus_write_byte_data(client, FM3130_ALARM_WP_CONTROL, 0x80);
 
-	/* TODO */
-	/* TODO need to sanity check alarm */
-	tmp = fm3130->regs[FM3130_RTC_SECONDS];
-	tmp = bcd2bin(tmp & 0x7f);
-	if (tmp > 60)
-		goto exit_bad;
+	/* alarm registers sanity check */
+	tmp = bcd2bin(fm3130->regs[FM3130_RTC_SECONDS] & 0x7f);
+	if (tmp > 59)
+		goto bad_alarm;
+
 	tmp = bcd2bin(fm3130->regs[FM3130_RTC_MINUTES] & 0x7f);
-	if (tmp > 60)
-		goto exit_bad;
+	if (tmp > 59)
+		goto bad_alarm;
+
+	tmp = bcd2bin(fm3130->regs[FM3130_RTC_HOURS] & 0x3f);
+	if (tmp > 23)
+		goto bad_alarm;
 
 	tmp = bcd2bin(fm3130->regs[FM3130_RTC_DATE] & 0x3f);
 	if (tmp == 0 || tmp > 31)
-		goto exit_bad;
+		goto bad_alarm;
 
 	tmp = bcd2bin(fm3130->regs[FM3130_RTC_MONTHS] & 0x1f);
 	if (tmp == 0 || tmp > 12)
-		goto exit_bad;
+		goto bad_alarm;
 
-	tmp = fm3130->regs[FM3130_RTC_HOURS];
+	fm3130->alarm_valid = 1;
+
+bad_alarm:
+
+	/* clock registers sanity chek */
+	tmp = bcd2bin(fm3130->regs[FM3130_RTC_SECONDS] & 0x7f);
+	if (tmp > 59)
+		goto bad_clock;
+
+	tmp = bcd2bin(fm3130->regs[FM3130_RTC_MINUTES] & 0x7f);
+	if (tmp > 59)
+		goto bad_clock;
+
+	tmp = bcd2bin(fm3130->regs[FM3130_RTC_HOURS] & 0x3f);
+	if (tmp > 23)
+		goto bad_clock;
+
+	tmp = bcd2bin(fm3130->regs[FM3130_RTC_DAY] & 0x7);
+	if (tmp == 0 || tmp > 7)
+		goto bad_clock;
+
+	tmp = bcd2bin(fm3130->regs[FM3130_RTC_DATE] & 0x3f);
+	if (tmp == 0 || tmp > 31)
+		goto bad_clock;
+
+	tmp = bcd2bin(fm3130->regs[FM3130_RTC_MONTHS] & 0x1f);
+	if (tmp == 0 || tmp > 12)
+		goto bad_clock;
 
 	fm3130->data_valid = 1;
 
-exit_bad:
-	if (!fm3130->data_valid)
+bad_clock:
+
+	if (!fm3130->data_valid || !fm3130->alarm_valid)
 		dev_dbg(&client->dev,
 				"%s: %02x %02x %02x %02x %02x %02x %02x %02x"
 				"%02x %02x %02x %02x %02x %02x %02x\n",