diff mbox

[v2,2/3] RTC: new functionality for fm3130

Message ID 1256408743-6644-2-git-send-email-geomatsi@gmail.com
State Changes Requested, archived
Headers show

Commit Message

Sergey Matyukevich Oct. 24, 2009, 6:25 p.m. UTC 
This patch contains the following fixes and enhancements
for fm3130 rtc driver:

* 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 ioctls for RTC_AIE_ON, RTC_AIE_OFF

* 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
---
 drivers/rtc/rtc-fm3130.c |  179 +++++++++++++++++++++++++++++++++-------------
 1 files changed, 128 insertions(+), 51 deletions(-)

Comments

Sergey Lapin Oct. 24, 2009, 7:31 p.m. UTC | #1 
On Sat, Oct 24, 2009 at 10:25:42PM +0400, Sergey Matyukevich wrote:
> 
> This patch contains the following fixes and enhancements
> for fm3130 rtc driver:
> 
> * 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 ioctls for RTC_AIE_ON, RTC_AIE_OFF
> 
> * 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
This patch is mostly OK, but see below.
Anyway, I think it can go in as is.
Acked-by: Sergey Lapin <slapin@ossfans.org>
> ---
>  drivers/rtc/rtc-fm3130.c |  179 +++++++++++++++++++++++++++++++++-------------
>  1 files changed, 128 insertions(+), 51 deletions(-)
> 
> diff --git a/drivers/rtc/rtc-fm3130.c b/drivers/rtc/rtc-fm3130.c
> index 812c667..5df991a 100644
> --- a/drivers/rtc/rtc-fm3130.c
> +++ b/drivers/rtc/rtc-fm3130.c
> @@ -51,8 +51,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 },
> @@ -86,11 +86,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]);
>  }
> @@ -207,6 +203,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);
> @@ -221,20 +226,30 @@ static int fm3130_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
>  			fm3130->regs[FM3130_ALARM_DATE],
>  			fm3130->regs[FM3130_ALARM_MONTHS]);
>  
> -
I think that dosn't  belong here....

>  	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);
> +
Ditto.

>  	if (tm->tm_mon > 0)
>  		tm->tm_mon -= 1; /* RTC is 1-12, tm_mon is 0-11 */
> +
And so on.
>  	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;
>  }
>  
> @@ -250,25 +265,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],
> @@ -284,11 +294,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] &
> @@ -297,12 +304,52 @@ 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;
>  }
>  
> +static int fm3130_ioctl(struct device *dev, unsigned int cmd,
> +			unsigned long arg)
> +{
> +	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, "ioctl: cmd=%08x, FM3130_RTC_CONTROL=%08x\n",
> +			cmd, fm3130->regs[FM3130_RTC_CONTROL]);
> +
> +	switch (cmd) {
> +	case RTC_AIE_OFF:		/* 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 RTC_AIE_ON:		/* 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 = -ENOIOCTLCMD;
> +		break;
> +	}
> +
> +	return ret;
> +}
> +
>  static const struct rtc_class_ops fm3130_rtc_ops = {
> +	.ioctl		= fm3130_ioctl,
>  	.read_time	= fm3130_get_time,
>  	.set_time	= fm3130_set_time,
>  	.read_alarm	= fm3130_read_alarm,
> @@ -355,6 +402,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);
> @@ -369,12 +417,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,
> @@ -399,44 +441,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",
> -- 
> 1.6.2.5
> 
> 
> 

--~--~---------~--~----~------------~-------~--~----~
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 812c667..5df991a 100644
--- a/drivers/rtc/rtc-fm3130.c
+++ b/drivers/rtc/rtc-fm3130.c
@@ -51,8 +51,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 },
@@ -86,11 +86,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]);
 }
@@ -207,6 +203,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);
@@ -221,20 +226,30 @@  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;
 }
 
@@ -250,25 +265,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],
@@ -284,11 +294,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] &
@@ -297,12 +304,52 @@  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;
 }
 
+static int fm3130_ioctl(struct device *dev, unsigned int cmd,
+			unsigned long arg)
+{
+	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, "ioctl: cmd=%08x, FM3130_RTC_CONTROL=%08x\n",
+			cmd, fm3130->regs[FM3130_RTC_CONTROL]);
+
+	switch (cmd) {
+	case RTC_AIE_OFF:		/* 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 RTC_AIE_ON:		/* 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 = -ENOIOCTLCMD;
+		break;
+	}
+
+	return ret;
+}
+
 static const struct rtc_class_ops fm3130_rtc_ops = {
+	.ioctl		= fm3130_ioctl,
 	.read_time	= fm3130_get_time,
 	.set_time	= fm3130_set_time,
 	.read_alarm	= fm3130_read_alarm,
@@ -355,6 +402,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);
@@ -369,12 +417,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,
@@ -399,44 +441,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",