[rtc-linux] [PATCH 0/2] Improve low voltage or invalid time detection

[rtc-linux] [PATCH 0/2] Improve low voltage or invalid time detection

[Fabien Lahoudere]

This patchset implements feature to detect RTC low voltage or power off.

Fabien Lahoudere (2):
  RTC: s35390a: handle invalid RTC time
  RTC: s35390a: implement ioctls

 drivers/rtc/rtc-s35390a.c | 49 +++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 47 insertions(+), 2 deletions(-)

-- 
1.8.3.1

-- 
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 1/2] RTC: s35390a: handle invalid RTC time

[Fabien Lahoudere]

If RTC time have been altered by low voltage, we notify users
that RTC time is invalid by returning -EINVAL.
The RTC time needs to be set correctly to clear the invalid flag.
If the RTC is not set before restarting, the information will be lost.

Signed-off-by: Fabien Lahoudere <fabien.lahoudere@collabora.co.uk>
---
 drivers/rtc/rtc-s35390a.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/rtc/rtc-s35390a.c b/drivers/rtc/rtc-s35390a.c
index 5dab466..ef4ada9 100644
--- a/drivers/rtc/rtc-s35390a.c
+++ b/drivers/rtc/rtc-s35390a.c
@@ -62,6 +62,7 @@ struct s35390a {
 	struct i2c_client *client[8];
 	struct rtc_device *rtc;
 	int twentyfourhour;
+	int isinvalid;
 };
 
 static int s35390a_set_reg(struct s35390a *s35390a, int reg, char *buf, int len)
@@ -135,6 +136,8 @@ static int s35390a_reset(struct s35390a *s35390a, char *status1)
 	 * The 24H bit is kept over reset, so set it already here.
 	 */
 initialize:
+	/* set the RTC time as invalid */
+	s35390a->isinvalid = 1;
 	*status1 = S35390A_FLAG_24H;
 	buf = S35390A_FLAG_RESET | S35390A_FLAG_24H;
 	ret = s35390a_set_reg(s35390a, S35390A_CMD_STATUS1, &buf, 1);
@@ -221,6 +224,8 @@ static int s35390a_set_datetime(struct i2c_client *client, struct rtc_time *tm)
 		buf[i] = bitrev8(buf[i]);
 
 	err = s35390a_set_reg(s35390a, S35390A_CMD_TIME1, buf, sizeof(buf));
+	if (err >= 0)
+		s35390a->isinvalid = 0;
 
 	return err;
 }
@@ -231,6 +236,9 @@ static int s35390a_get_datetime(struct i2c_client *client, struct rtc_time *tm)
 	char buf[7];
 	int i, err;
 
+	if (s35390a->isinvalid)
+		return -EINVAL;
+
 	err = s35390a_get_reg(s35390a, S35390A_CMD_TIME1, buf, sizeof(buf));
 	if (err < 0)
 		return err;
@@ -418,6 +426,7 @@ static int s35390a_probe(struct i2c_client *client,
 
 	s35390a->client[0] = client;
 	i2c_set_clientdata(client, s35390a);
+	s35390a->isinvalid = 0;
 
 	/* This chip uses multiple addresses, use dummy devices for them */
 	for (i = 1; i < 8; ++i) {
-- 
1.8.3.1

-- 
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 2/2] RTC: s35390a: implement ioctls

[Fabien Lahoudere]

Implements RTC_VL_READ and RTC_VL_CLR ioctls.

Signed-off-by: Fabien Lahoudere <fabien.lahoudere@collabora.co.uk>
---
 drivers/rtc/rtc-s35390a.c | 40 ++++++++++++++++++++++++++++++++++++++--
 1 file changed, 38 insertions(+), 2 deletions(-)

diff --git a/drivers/rtc/rtc-s35390a.c b/drivers/rtc/rtc-s35390a.c
index ef4ada9..2bd8301 100644
--- a/drivers/rtc/rtc-s35390a.c
+++ b/drivers/rtc/rtc-s35390a.c
@@ -63,6 +63,7 @@ struct s35390a {
 	struct rtc_device *rtc;
 	int twentyfourhour;
 	int isinvalid;
+	int lowvoltage;
 };
 
 static int s35390a_set_reg(struct s35390a *s35390a, int reg, char *buf, int len)
@@ -121,7 +122,9 @@ static int s35390a_reset(struct s35390a *s35390a, char *status1)
 		 * detection circuit is in operation.
 		 */
 		msleep(500);
-	else if (!(*status1 & S35390A_FLAG_BLD))
+	else if (*status1 & S35390A_FLAG_BLD)
+		s35390a->lowvoltage = 1;
+	else
 		/*
 		 * If both POC and BLD are unset everything is fine.
 		 */
@@ -393,12 +396,44 @@ static int s35390a_rtc_set_time(struct device *dev, struct rtc_time *tm)
 	return s35390a_set_datetime(to_i2c_client(dev), tm);
 }
 
+static int s35390a_rtc_ioctl(struct device *dev, unsigned int cmd,
+			     unsigned long arg)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct s35390a *s35390a = i2c_get_clientdata(client);
+	char sts;
+	int err;
+
+	switch (cmd) {
+	case RTC_VL_READ:
+		/* s35390a_reset set lowvoltage flag and init RTC if needed */
+		err = s35390a_reset(s35390a, &sts);
+		if (err < 0)
+			return err;
+		if (copy_to_user((void __user *)arg, &s35390a->lowvoltage,
+				 sizeof(int)))
+			return -EFAULT;
+
+	case RTC_VL_CLR:
+		/* update flag and clear register */
+		err = s35390a_reset(s35390a, &sts);
+		if ((err == 1) || (err == 0))
+			s35390a->lowvoltage = 0;
+		else
+			return err;
+	default:
+		return -ENOIOCTLCMD;
+	}
+
+	return 0;
+}
+
 static const struct rtc_class_ops s35390a_rtc_ops = {
 	.read_time	= s35390a_rtc_read_time,
 	.set_time	= s35390a_rtc_set_time,
 	.set_alarm	= s35390a_rtc_set_alarm,
 	.read_alarm	= s35390a_rtc_read_alarm,
-
+	.ioctl          = s35390a_rtc_ioctl,
 };
 
 static struct i2c_driver s35390a_driver;
@@ -427,6 +462,7 @@ static int s35390a_probe(struct i2c_client *client,
 	s35390a->client[0] = client;
 	i2c_set_clientdata(client, s35390a);
 	s35390a->isinvalid = 0;
+	s35390a->lowvoltage = 0;
 
 	/* This chip uses multiple addresses, use dummy devices for them */
 	for (i = 1; i < 8; ++i) {
-- 
1.8.3.1

-- 
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 2/2] RTC: s35390a: implement ioctls

[Alexandre Belloni]

On 12/01/2017 at 11:43:38 +0100, Fabien Lahoudere wrote :
> Implements RTC_VL_READ and RTC_VL_CLR ioctls.
> 
> Signed-off-by: Fabien Lahoudere <fabien.lahoudere@collabora.co.uk>
> ---
>  drivers/rtc/rtc-s35390a.c | 40 ++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 38 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/rtc/rtc-s35390a.c b/drivers/rtc/rtc-s35390a.c
> index ef4ada9..2bd8301 100644
> --- a/drivers/rtc/rtc-s35390a.c
> +++ b/drivers/rtc/rtc-s35390a.c
> @@ -121,7 +122,9 @@ static int s35390a_reset(struct s35390a *s35390a, char *status1)
>  		 * detection circuit is in operation.
>  		 */
>  		msleep(500);
> -	else if (!(*status1 & S35390A_FLAG_BLD))
> +	else if (*status1 & S35390A_FLAG_BLD)
> +		s35390a->lowvoltage = 1;
> +	else
>  		/*
>  		 * If both POC and BLD are unset everything is fine.
>  		 */
> @@ -393,12 +396,44 @@ static int s35390a_rtc_set_time(struct device *dev, struct rtc_time *tm)
>  	return s35390a_set_datetime(to_i2c_client(dev), tm);
>  }
>  
> +static int s35390a_rtc_ioctl(struct device *dev, unsigned int cmd,
> +			     unsigned long arg)
> +{
> +	struct i2c_client *client = to_i2c_client(dev);
> +	struct s35390a *s35390a = i2c_get_clientdata(client);
> +	char sts;
> +	int err;
> +
> +	switch (cmd) {
> +	case RTC_VL_READ:

It seems wasteful to call s35390a_reset() every time. If
s35390a->lowvoltage is already set to 1 then you already have the answer
you need (and you already resetted the RTC)

> +		/* s35390a_reset set lowvoltage flag and init RTC if needed */
> +		err = s35390a_reset(s35390a, &sts);
> +		if (err < 0)
> +			return err;
> +		if (copy_to_user((void __user *)arg, &s35390a->lowvoltage,
> +				 sizeof(int)))
> +			return -EFAULT;
> +
> +	case RTC_VL_CLR:
> +		/* update flag and clear register */
> +		err = s35390a_reset(s35390a, &sts);
> +		if ((err == 1) || (err == 0))

Whatever the error, once the first s35390a_get_reg(), the POC and BLD
flags are lost. I would reset lowvoltage in every cases.


-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel 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 1/2] RTC: s35390a: handle invalid RTC time

[Alexandre Belloni]

Hi,

On 12/01/2017 at 11:43:37 +0100, Fabien Lahoudere wrote :
> If RTC time have been altered by low voltage, we notify users
> that RTC time is invalid by returning -EINVAL.
> The RTC time needs to be set correctly to clear the invalid flag.
> If the RTC is not set before restarting, the information will be lost.
> 
> Signed-off-by: Fabien Lahoudere <fabien.lahoudere@collabora.co.uk>
> ---
>  drivers/rtc/rtc-s35390a.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/rtc/rtc-s35390a.c b/drivers/rtc/rtc-s35390a.c
> index 5dab466..ef4ada9 100644
> --- a/drivers/rtc/rtc-s35390a.c
> +++ b/drivers/rtc/rtc-s35390a.c
> @@ -62,6 +62,7 @@ struct s35390a {
>  	struct i2c_client *client[8];
>  	struct rtc_device *rtc;
>  	int twentyfourhour;
> +	int isinvalid;
>  };
>  
>  static int s35390a_set_reg(struct s35390a *s35390a, int reg, char *buf, int len)
> @@ -135,6 +136,8 @@ static int s35390a_reset(struct s35390a *s35390a, char *status1)
>  	 * The 24H bit is kept over reset, so set it already here.
>  	 */
>  initialize:
> +	/* set the RTC time as invalid */
> +	s35390a->isinvalid = 1;
>  	*status1 = S35390A_FLAG_24H;
>  	buf = S35390A_FLAG_RESET | S35390A_FLAG_24H;
>  	ret = s35390a_set_reg(s35390a, S35390A_CMD_STATUS1, &buf, 1);
> @@ -221,6 +224,8 @@ static int s35390a_set_datetime(struct i2c_client *client, struct rtc_time *tm)
>  		buf[i] = bitrev8(buf[i]);
>  
>  	err = s35390a_set_reg(s35390a, S35390A_CMD_TIME1, buf, sizeof(buf));
> +	if (err >= 0)
> +		s35390a->isinvalid = 0;
>  
>  	return err;
>  }
> @@ -231,6 +236,9 @@ static int s35390a_get_datetime(struct i2c_client *client, struct rtc_time *tm)
>  	char buf[7];
>  	int i, err;
>  
> +	if (s35390a->isinvalid)
> +		return -EINVAL;
> +

That's fine but what happens if it became invalid between probe and
s35390a_get_datetime()? (This is particularly relevant after patch 2/2.

>  	err = s35390a_get_reg(s35390a, S35390A_CMD_TIME1, buf, sizeof(buf));
>  	if (err < 0)
>  		return err;
> @@ -418,6 +426,7 @@ static int s35390a_probe(struct i2c_client *client,
>  
>  	s35390a->client[0] = client;
>  	i2c_set_clientdata(client, s35390a);
> +	s35390a->isinvalid = 0;
>  
>  	/* This chip uses multiple addresses, use dummy devices for them */
>  	for (i = 1; i < 8; ++i) {
> -- 
> 1.8.3.1
> 

-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel 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 1/2] RTC: s35390a: handle invalid RTC time

[Fabien Lahoudere]

On Mon, 2017-01-16 at 18:50 +0100, Alexandre Belloni wrote:
> Hi,
> 
> On 12/01/2017 at 11:43:37 +0100, Fabien Lahoudere wrote :
> > If RTC time have been altered by low voltage, we notify users
> > that RTC time is invalid by returning -EINVAL.
> > The RTC time needs to be set correctly to clear the invalid flag.
> > If the RTC is not set before restarting, the information will be lost.
> > 
> > Signed-off-by: Fabien Lahoudere <fabien.lahoudere@collabora.co.uk>
> > ---
> >  drivers/rtc/rtc-s35390a.c | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/drivers/rtc/rtc-s35390a.c b/drivers/rtc/rtc-s35390a.c
> > index 5dab466..ef4ada9 100644
> > --- a/drivers/rtc/rtc-s35390a.c
> > +++ b/drivers/rtc/rtc-s35390a.c
> > @@ -62,6 +62,7 @@ struct s35390a {
> >  	struct i2c_client *client[8];
> >  	struct rtc_device *rtc;
> >  	int twentyfourhour;
> > +	int isinvalid;
> >  };
> >  
> >  static int s35390a_set_reg(struct s35390a *s35390a, int reg, char *buf, int len)
> > @@ -135,6 +136,8 @@ static int s35390a_reset(struct s35390a *s35390a, char *status1)
> >  	 * The 24H bit is kept over reset, so set it already here.
> >  	 */
> >  initialize:
> > +	/* set the RTC time as invalid */
> > +	s35390a->isinvalid = 1;
> >  	*status1 = S35390A_FLAG_24H;
> >  	buf = S35390A_FLAG_RESET | S35390A_FLAG_24H;
> >  	ret = s35390a_set_reg(s35390a, S35390A_CMD_STATUS1, &buf, 1);
> > @@ -221,6 +224,8 @@ static int s35390a_set_datetime(struct i2c_client *client, struct rtc_time *tm)
> >  		buf[i] = bitrev8(buf[i]);
> >  
> >  	err = s35390a_set_reg(s35390a, S35390A_CMD_TIME1, buf, sizeof(buf));
> > +	if (err >= 0)
> > +		s35390a->isinvalid = 0;
> >  
> >  	return err;
> >  }
> > @@ -231,6 +236,9 @@ static int s35390a_get_datetime(struct i2c_client *client, struct rtc_time *tm)
> >  	char buf[7];
> >  	int i, err;
> >  
> > +	if (s35390a->isinvalid)
> > +		return -EINVAL;
> > +
> 
> That's fine but what happens if it became invalid between probe and
> s35390a_get_datetime()? (This is particularly relevant after patch 2/2.

This is not possible with our design. When the system is on, its power
supply replace the backup battery. (See p38 Figure 49).

If you prefer I can call s35390a_reset if s35390a->isinvalid is set to
0?

> 
> >  	err = s35390a_get_reg(s35390a, S35390A_CMD_TIME1, buf, sizeof(buf));
> >  	if (err < 0)
> >  		return err;
> > @@ -418,6 +426,7 @@ static int s35390a_probe(struct i2c_client *client,
> >  
> >  	s35390a->client[0] = client;
> >  	i2c_set_clientdata(client, s35390a);
> > +	s35390a->isinvalid = 0;
> >  
> >  	/* This chip uses multiple addresses, use dummy devices for them */
> >  	for (i = 1; i < 8; ++i) {
> > -- 
> > 1.8.3.1
> > 
> 


-- 
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 1/2] RTC: s35390a: handle invalid RTC time

[Alexandre Belloni]

On 17/01/2017 at 09:24:17 +0100, Fabien Lahoudere wrote :
> On Mon, 2017-01-16 at 18:50 +0100, Alexandre Belloni wrote:
> > Hi,
> > 
> > On 12/01/2017 at 11:43:37 +0100, Fabien Lahoudere wrote :
> > > If RTC time have been altered by low voltage, we notify users
> > > that RTC time is invalid by returning -EINVAL.
> > > The RTC time needs to be set correctly to clear the invalid flag.
> > > If the RTC is not set before restarting, the information will be lost.
> > > 
> > > Signed-off-by: Fabien Lahoudere <fabien.lahoudere@collabora.co.uk>
> > > ---
> > >  drivers/rtc/rtc-s35390a.c | 9 +++++++++
> > >  1 file changed, 9 insertions(+)
> > > 
> > > diff --git a/drivers/rtc/rtc-s35390a.c b/drivers/rtc/rtc-s35390a.c
> > > index 5dab466..ef4ada9 100644
> > > --- a/drivers/rtc/rtc-s35390a.c
> > > +++ b/drivers/rtc/rtc-s35390a.c
> > > @@ -62,6 +62,7 @@ struct s35390a {
> > >  	struct i2c_client *client[8];
> > >  	struct rtc_device *rtc;
> > >  	int twentyfourhour;
> > > +	int isinvalid;
> > >  };
> > >  
> > >  static int s35390a_set_reg(struct s35390a *s35390a, int reg, char *buf, int len)
> > > @@ -135,6 +136,8 @@ static int s35390a_reset(struct s35390a *s35390a, char *status1)
> > >  	 * The 24H bit is kept over reset, so set it already here.
> > >  	 */
> > >  initialize:
> > > +	/* set the RTC time as invalid */
> > > +	s35390a->isinvalid = 1;
> > >  	*status1 = S35390A_FLAG_24H;
> > >  	buf = S35390A_FLAG_RESET | S35390A_FLAG_24H;
> > >  	ret = s35390a_set_reg(s35390a, S35390A_CMD_STATUS1, &buf, 1);
> > > @@ -221,6 +224,8 @@ static int s35390a_set_datetime(struct i2c_client *client, struct rtc_time *tm)
> > >  		buf[i] = bitrev8(buf[i]);
> > >  
> > >  	err = s35390a_set_reg(s35390a, S35390A_CMD_TIME1, buf, sizeof(buf));
> > > +	if (err >= 0)
> > > +		s35390a->isinvalid = 0;
> > >  
> > >  	return err;
> > >  }
> > > @@ -231,6 +236,9 @@ static int s35390a_get_datetime(struct i2c_client *client, struct rtc_time *tm)
> > >  	char buf[7];
> > >  	int i, err;
> > >  
> > > +	if (s35390a->isinvalid)
> > > +		return -EINVAL;
> > > +
> > 
> > That's fine but what happens if it became invalid between probe and
> > s35390a_get_datetime()? (This is particularly relevant after patch 2/2.
> 
> This is not possible with our design. When the system is on, its power
> supply replace the backup battery. (See p38 Figure 49).
> 

Well, it depends on the tolerances of each components (but yeah, I doubt
your SoC is more tolerant than the RTC).


> If you prefer I can call s35390a_reset if s35390a->isinvalid is set to
> 0?
> 

Actually, after reading the datasheet, I realize it is only POC that is
reset to 0 after reading so isinvalid is not needed. Just read status1
and look for BLD instead of caching it.

I think it is probably worth separating s35390a_reset() into two
functions. One that does the initialization and another one that reads
status1 and immediately doest the initialization when POC is set. If BLD
is set, then we can wait for set_time to happen before initializing.

-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel 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 1/2] RTC: s35390a: handle invalid RTC time

[Fabien Lahoudere]

On Tue, 2017-01-17 at 12:00 +0100, Alexandre Belloni wrote:
> On 17/01/2017 at 09:24:17 +0100, Fabien Lahoudere wrote :
> > On Mon, 2017-01-16 at 18:50 +0100, Alexandre Belloni wrote:
> > > Hi,
> > >=20
> > > On 12/01/2017 at 11:43:37 +0100, Fabien Lahoudere wrote :
> > > > If RTC time have been altered by low voltage, we notify users
> > > > that RTC time is invalid by returning -EINVAL.
> > > > The RTC time needs to be set correctly to clear the invalid flag.
> > > > If the RTC is not set before restarting, the information will be lo=
st.
> > > >=20
> > > > Signed-off-by: Fabien Lahoudere <fabien.lahoudere@collabora.co.uk>
> > > > ---
> > > > =C2=A0drivers/rtc/rtc-s35390a.c | 9 +++++++++
> > > > =C2=A01 file changed, 9 insertions(+)
> > > >=20
> > > > diff --git a/drivers/rtc/rtc-s35390a.c b/drivers/rtc/rtc-s35390a.c
> > > > index 5dab466..ef4ada9 100644
> > > > --- a/drivers/rtc/rtc-s35390a.c
> > > > +++ b/drivers/rtc/rtc-s35390a.c
> > > > @@ -62,6 +62,7 @@ struct s35390a {
> > > > =C2=A0	struct i2c_client *client[8];
> > > > =C2=A0	struct rtc_device *rtc;
> > > > =C2=A0	int twentyfourhour;
> > > > +	int isinvalid;
> > > > =C2=A0};
> > > > =C2=A0
> > > > =C2=A0static int s35390a_set_reg(struct s35390a *s35390a, int reg, =
char *buf, int len)
> > > > @@ -135,6 +136,8 @@ static int s35390a_reset(struct s35390a *s35390=
a, char *status1)
> > > > =C2=A0	=C2=A0* The 24H bit is kept over reset, so set it already he=
re.
> > > > =C2=A0	=C2=A0*/
> > > > =C2=A0initialize:
> > > > +	/* set the RTC time as invalid */
> > > > +	s35390a->isinvalid =3D 1;
> > > > =C2=A0	*status1 =3D S35390A_FLAG_24H;
> > > > =C2=A0	buf =3D S35390A_FLAG_RESET | S35390A_FLAG_24H;
> > > > =C2=A0	ret =3D s35390a_set_reg(s35390a, S35390A_CMD_STATUS1, &buf, =
1);
> > > > @@ -221,6 +224,8 @@ static int s35390a_set_datetime(struct i2c_clie=
nt *client, struct
> > > > rtc_time *tm)
> > > > =C2=A0		buf[i] =3D bitrev8(buf[i]);
> > > > =C2=A0
> > > > =C2=A0	err =3D s35390a_set_reg(s35390a, S35390A_CMD_TIME1, buf, siz=
eof(buf));
> > > > +	if (err >=3D 0)
> > > > +		s35390a->isinvalid =3D 0;
> > > > =C2=A0
> > > > =C2=A0	return err;
> > > > =C2=A0}
> > > > @@ -231,6 +236,9 @@ static int s35390a_get_datetime(struct i2c_clie=
nt *client, struct
> > > > rtc_time *tm)
> > > > =C2=A0	char buf[7];
> > > > =C2=A0	int i, err;
> > > > =C2=A0
> > > > +	if (s35390a->isinvalid)
> > > > +		return -EINVAL;
> > > > +
> > >=20
> > > That's fine but what happens if it became invalid between probe and
> > > s35390a_get_datetime()? (This is particularly relevant after patch 2/=
2.
> >=20
> > This is not possible with our design. When the system is on, its power
> > supply replace the backup battery. (See p38 Figure 49).
> >=20
>=20
> Well, it depends on the tolerances of each components (but yeah, I doubt
> your SoC is more tolerant than the RTC).
>=20
>=20
> > If you prefer I can call s35390a_reset if s35390a->isinvalid is set to
> > 0?
> >=20
>=20
> Actually, after reading the datasheet, I realize it is only POC that is
> reset to 0 after reading so isinvalid is not needed. Just read status1
> and look for BLD instead of caching it.
>=20

isinvalid is also used in s35390a_set_datetime. So if I remove it how can I=
 detect that time setting
failed?

> I think it is probably worth separating s35390a_reset() into two
> functions. One that does the initialization and another one that reads
> status1 and immediately doest the initialization when POC is set. If BLD
> is set, then we can wait for set_time to happen before initializing.
>=20

--=20
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.
---=20
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 e=
mail to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[rtc-linux] Re: [PATCH 1/2] RTC: s35390a: handle invalid RTC time

[Alexandre Belloni]

On 26/06/2017 at 11:51:13 +0200, Fabien Lahoudere wrote:
> > Actually, after reading the datasheet, I realize it is only POC that is
> > reset to 0 after reading so isinvalid is not needed. Just read status1
> > and look for BLD instead of caching it.
> > 
> 
> isinvalid is also used in s35390a_set_datetime. So if I remove it how can I detect that time setting
> failed?
> 

If it fails, simply don't reset BLD so it is still set when reading the
time.

> > I think it is probably worth separating s35390a_reset() into two
> > functions. One that does the initialization and another one that reads
> > status1 and immediately doest the initialization when POC is set. If BLD
> > is set, then we can wait for set_time to happen before initializing.
> > 

-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel 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 1/2] RTC: s35390a: handle invalid RTC time

[Fabien Lahoudere]

On Thu, 2017-06-29 at 09:56 +0200, Alexandre Belloni wrote:
> On 26/06/2017 at 11:51:13 +0200, Fabien Lahoudere wrote:
> > > Actually, after reading the datasheet, I realize it is only POC that is
> > > reset to 0 after reading so isinvalid is not needed. Just read status1
> > > and look for BLD instead of caching it.
> > > 
> > 
> > isinvalid is also used in s35390a_set_datetime. So if I remove it how can I detect that time
> > setting
> > failed?
> > 
> 
> If it fails, simply don't reset BLD so it is still set when reading the
> time.
> 

In BLD section, datasheet says "When this flag is "1", be sure to initialize."
So I think we need those flags.

> > > I think it is probably worth separating s35390a_reset() into two
> > > functions. One that does the initialization and another one that reads
> > > status1 and immediately doest the initialization when POC is set. If BLD
> > > is set, then we can wait for set_time to happen before initializing.
> > > 
> 
> 

-- 
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 1/2] RTC: s35390a: handle invalid RTC time

[Alexandre Belloni]

On 04/07/2017 at 10:55:52 +0200, Fabien Lahoudere wrote:
> On Thu, 2017-06-29 at 09:56 +0200, Alexandre Belloni wrote:
> > On 26/06/2017 at 11:51:13 +0200, Fabien Lahoudere wrote:
> > > > Actually, after reading the datasheet, I realize it is only POC that is
> > > > reset to 0 after reading so isinvalid is not needed. Just read status1
> > > > and look for BLD instead of caching it.
> > > > 
> > > 
> > > isinvalid is also used in s35390a_set_datetime. So if I remove it how can I detect that time
> > > setting
> > > failed?
> > > 
> > 
> > If it fails, simply don't reset BLD so it is still set when reading the
> > time.
> > 
> 
> In BLD section, datasheet says "When this flag is "1", be sure to initialize."
> So I think we need those flags.
> 

No, you don't. If BLD is set, return -EINVAL in read_time, initialize
the RTC in set_time and that is it. You'll always know when power failed
and you'll always initialize at the correct time (i.e before really
caring about the date/time).

> > > > I think it is probably worth separating s35390a_reset() into two
> > > > functions. One that does the initialization and another one that reads
> > > > status1 and immediately doest the initialization when POC is set. If BLD
> > > > is set, then we can wait for set_time to happen before initializing.
> > > > 
> > 
> > 

-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel 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.