linux-rtc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] rtc: rs5c372: Add RTC_VL_READ, RTC_VL_CLR ioctls
@ 2021-11-10 11:54 Camel Guo
  2021-11-10 13:54 ` Alexandre Belloni
  0 siblings, 1 reply; 7+ messages in thread
From: Camel Guo @ 2021-11-10 11:54 UTC (permalink / raw)
  To: Alessandro Zummo, Alexandre Belloni
  Cc: kernel, Camel Guo, linux-rtc, linux-kernel

From: Camel Guo <camelg@axis.com>

In order to make it possible to get battery voltage status, this commit
adds RTC_VL_READ, RTC_VL_CLR ioctl commands to rtc-rs5c372.

Signed-off-by: Camel Guo <camelg@axis.com>
---
 drivers/rtc/rtc-rs5c372.c | 66 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)

diff --git a/drivers/rtc/rtc-rs5c372.c b/drivers/rtc/rtc-rs5c372.c
index 80980414890c..5a96e5d3663a 100644
--- a/drivers/rtc/rtc-rs5c372.c
+++ b/drivers/rtc/rtc-rs5c372.c
@@ -485,6 +485,71 @@ static int rs5c372_rtc_proc(struct device *dev, struct seq_file *seq)
 #define	rs5c372_rtc_proc	NULL
 #endif
 
+#ifdef CONFIG_RTC_INTF_DEV
+static int rs5c372_ioctl(struct device *dev, unsigned int cmd, unsigned long arg)
+{
+	struct rs5c372	*rs5c = i2c_get_clientdata(to_i2c_client(dev));
+	unsigned char	ctrl2;
+	int		addr;
+	unsigned int	flags;
+
+	dev_dbg(dev, "%s: cmd=%x\n", __func__, cmd);
+
+	addr = RS5C_ADDR(RS5C_REG_CTRL2);
+	ctrl2 = i2c_smbus_read_byte_data(rs5c->client, addr);
+
+	switch (cmd) {
+	case RTC_VL_READ:
+		flags = 0;
+
+		switch (rs5c->type) {
+		case rtc_r2025sd:
+		case rtc_r2221tl:
+			if ((rs5c->type == rtc_r2025sd && !(ctrl2 & R2x2x_CTRL2_XSTP)) ||
+				(rs5c->type == rtc_r2221tl &&  (ctrl2 & R2x2x_CTRL2_XSTP))) {
+				flags |= RTC_VL_DATA_INVALID;
+			}
+			if (ctrl2 & R2x2x_CTRL2_VDET)
+				flags |= RTC_VL_ACCURACY_LOW;
+			break;
+		default:
+			if (ctrl2 & RS5C_CTRL2_XSTP)
+				flags |= RTC_VL_DATA_INVALID;
+			break;
+		}
+
+		return put_user(flags, (unsigned int __user *)arg);
+	case RTC_VL_CLR:
+		/* clear rtc VDET, PON and XSTP bits */
+		switch (rs5c->type) {
+		case rtc_r2025sd:
+		case rtc_r2221tl:
+			ctrl2 &= ~(R2x2x_CTRL2_VDET | R2x2x_CTRL2_PON);
+			if (rs5c->type == rtc_r2025sd)
+				ctrl2 |= R2x2x_CTRL2_XSTP;
+			else
+				ctrl2 &= ~R2x2x_CTRL2_XSTP;
+			break;
+		default:
+			ctrl2 &= ~RS5C_CTRL2_XSTP;
+			break;
+		}
+
+		if (i2c_smbus_write_byte_data(rs5c->client, addr, ctrl2) < 0) {
+			dev_dbg(&rs5c->client->dev, "%s: write error in line %i\n",
+					__func__, __LINE__);
+			return -EIO;
+		}
+		return 0;
+	default:
+		return -ENOIOCTLCMD;
+	}
+	return 0;
+}
+#else
+#define rs5c372_ioctl	NULL
+#endif
+
 static const struct rtc_class_ops rs5c372_rtc_ops = {
 	.proc		= rs5c372_rtc_proc,
 	.read_time	= rs5c372_rtc_read_time,
@@ -492,6 +557,7 @@ static const struct rtc_class_ops rs5c372_rtc_ops = {
 	.read_alarm	= rs5c_read_alarm,
 	.set_alarm	= rs5c_set_alarm,
 	.alarm_irq_enable = rs5c_rtc_alarm_irq_enable,
+	.ioctl		= rs5c372_ioctl,
 };
 
 #if IS_ENABLED(CONFIG_RTC_INTF_SYSFS)
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] rtc: rs5c372: Add RTC_VL_READ, RTC_VL_CLR ioctls
  2021-11-10 11:54 [PATCH] rtc: rs5c372: Add RTC_VL_READ, RTC_VL_CLR ioctls Camel Guo
@ 2021-11-10 13:54 ` Alexandre Belloni
  2021-11-10 14:03   ` Camel Guo
  0 siblings, 1 reply; 7+ messages in thread
From: Alexandre Belloni @ 2021-11-10 13:54 UTC (permalink / raw)
  To: Camel Guo; +Cc: Alessandro Zummo, kernel, Camel Guo, linux-rtc, linux-kernel

Hello,

On 10/11/2021 12:54:54+0100, Camel Guo wrote:
> From: Camel Guo <camelg@axis.com>
> 
> In order to make it possible to get battery voltage status, this commit
> adds RTC_VL_READ, RTC_VL_CLR ioctl commands to rtc-rs5c372.
> 
> Signed-off-by: Camel Guo <camelg@axis.com>
> ---
>  drivers/rtc/rtc-rs5c372.c | 66 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 66 insertions(+)
> 
> diff --git a/drivers/rtc/rtc-rs5c372.c b/drivers/rtc/rtc-rs5c372.c
> index 80980414890c..5a96e5d3663a 100644
> --- a/drivers/rtc/rtc-rs5c372.c
> +++ b/drivers/rtc/rtc-rs5c372.c
> @@ -485,6 +485,71 @@ static int rs5c372_rtc_proc(struct device *dev, struct seq_file *seq)
>  #define	rs5c372_rtc_proc	NULL
>  #endif
>  
> +#ifdef CONFIG_RTC_INTF_DEV
> +static int rs5c372_ioctl(struct device *dev, unsigned int cmd, unsigned long arg)
> +{
> +	struct rs5c372	*rs5c = i2c_get_clientdata(to_i2c_client(dev));
> +	unsigned char	ctrl2;
> +	int		addr;
> +	unsigned int	flags;
> +
> +	dev_dbg(dev, "%s: cmd=%x\n", __func__, cmd);
> +
> +	addr = RS5C_ADDR(RS5C_REG_CTRL2);
> +	ctrl2 = i2c_smbus_read_byte_data(rs5c->client, addr);
> +
> +	switch (cmd) {
> +	case RTC_VL_READ:
> +		flags = 0;
> +
> +		switch (rs5c->type) {
> +		case rtc_r2025sd:
> +		case rtc_r2221tl:
> +			if ((rs5c->type == rtc_r2025sd && !(ctrl2 & R2x2x_CTRL2_XSTP)) ||
> +				(rs5c->type == rtc_r2221tl &&  (ctrl2 & R2x2x_CTRL2_XSTP))) {
> +				flags |= RTC_VL_DATA_INVALID;
> +			}
> +			if (ctrl2 & R2x2x_CTRL2_VDET)
> +				flags |= RTC_VL_ACCURACY_LOW;

Shouldn't that be RTC_VL_BACKUP_LOW?

> +			break;
> +		default:
> +			if (ctrl2 & RS5C_CTRL2_XSTP)
> +				flags |= RTC_VL_DATA_INVALID;
> +			break;
> +		}
> +
> +		return put_user(flags, (unsigned int __user *)arg);
> +	case RTC_VL_CLR:
> +		/* clear rtc VDET, PON and XSTP bits */
> +		switch (rs5c->type) {
> +		case rtc_r2025sd:
> +		case rtc_r2221tl:
> +			ctrl2 &= ~(R2x2x_CTRL2_VDET | R2x2x_CTRL2_PON);
> +			if (rs5c->type == rtc_r2025sd)
> +				ctrl2 |= R2x2x_CTRL2_XSTP;
> +			else
> +				ctrl2 &= ~R2x2x_CTRL2_XSTP;
> +			break;
> +		default:
> +			ctrl2 &= ~RS5C_CTRL2_XSTP;

You can clear VDET but you must nt clear PON or XSTP as they are used to
know whether the time on the RTC has been set correctly. Clearing those
can only be done in .set_time.

> +			break;
> +		}
> +
> +		if (i2c_smbus_write_byte_data(rs5c->client, addr, ctrl2) < 0) {
> +			dev_dbg(&rs5c->client->dev, "%s: write error in line %i\n",
> +					__func__, __LINE__);
> +			return -EIO;
> +		}
> +		return 0;
> +	default:
> +		return -ENOIOCTLCMD;
> +	}
> +	return 0;
> +}
> +#else
> +#define rs5c372_ioctl	NULL
> +#endif
> +
>  static const struct rtc_class_ops rs5c372_rtc_ops = {
>  	.proc		= rs5c372_rtc_proc,
>  	.read_time	= rs5c372_rtc_read_time,
> @@ -492,6 +557,7 @@ static const struct rtc_class_ops rs5c372_rtc_ops = {
>  	.read_alarm	= rs5c_read_alarm,
>  	.set_alarm	= rs5c_set_alarm,
>  	.alarm_irq_enable = rs5c_rtc_alarm_irq_enable,
> +	.ioctl		= rs5c372_ioctl,
>  };
>  
>  #if IS_ENABLED(CONFIG_RTC_INTF_SYSFS)
> -- 
> 2.20.1
> 

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] rtc: rs5c372: Add RTC_VL_READ, RTC_VL_CLR ioctls
  2021-11-10 13:54 ` Alexandre Belloni
@ 2021-11-10 14:03   ` Camel Guo
  2021-11-10 14:27     ` Alexandre Belloni
  0 siblings, 1 reply; 7+ messages in thread
From: Camel Guo @ 2021-11-10 14:03 UTC (permalink / raw)
  To: Alexandre Belloni, Camel Guo
  Cc: Alessandro Zummo, kernel, linux-rtc, linux-kernel



On 11/10/21 2:54 PM, Alexandre Belloni wrote:
> Hello,
> 
> On 10/11/2021 12:54:54+0100, Camel Guo wrote:
>> From: Camel Guo <camelg@axis.com>
>> 
>> In order to make it possible to get battery voltage status, this commit
>> adds RTC_VL_READ, RTC_VL_CLR ioctl commands to rtc-rs5c372.
>> 
>> Signed-off-by: Camel Guo <camelg@axis.com>
>> ---
>>  drivers/rtc/rtc-rs5c372.c | 66 +++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 66 insertions(+)
>> 
>> diff --git a/drivers/rtc/rtc-rs5c372.c b/drivers/rtc/rtc-rs5c372.c
>> index 80980414890c..5a96e5d3663a 100644
>> --- a/drivers/rtc/rtc-rs5c372.c
>> +++ b/drivers/rtc/rtc-rs5c372.c
>> @@ -485,6 +485,71 @@ static int rs5c372_rtc_proc(struct device *dev, struct seq_file *seq)
>>  #define      rs5c372_rtc_proc        NULL
>>  #endif
>>  
>> +#ifdef CONFIG_RTC_INTF_DEV
>> +static int rs5c372_ioctl(struct device *dev, unsigned int cmd, unsigned long arg)
>> +{
>> +     struct rs5c372  *rs5c = i2c_get_clientdata(to_i2c_client(dev));
>> +     unsigned char   ctrl2;
>> +     int             addr;
>> +     unsigned int    flags;
>> +
>> +     dev_dbg(dev, "%s: cmd=%x\n", __func__, cmd);
>> +
>> +     addr = RS5C_ADDR(RS5C_REG_CTRL2);
>> +     ctrl2 = i2c_smbus_read_byte_data(rs5c->client, addr);
>> +
>> +     switch (cmd) {
>> +     case RTC_VL_READ:
>> +             flags = 0;
>> +
>> +             switch (rs5c->type) {
>> +             case rtc_r2025sd:
>> +             case rtc_r2221tl:
>> +                     if ((rs5c->type == rtc_r2025sd && !(ctrl2 & R2x2x_CTRL2_XSTP)) ||
>> +                             (rs5c->type == rtc_r2221tl &&  (ctrl2 & R2x2x_CTRL2_XSTP))) {
>> +                             flags |= RTC_VL_DATA_INVALID;
>> +                     }
>> +                     if (ctrl2 & R2x2x_CTRL2_VDET)
>> +                             flags |= RTC_VL_ACCURACY_LOW;
> 
> Shouldn't that be RTC_VL_BACKUP_LOW?

Some drivers (e.g: rv3029_ioctl and rv8803_ioctl) use 
RTC_VL_ACCURACY_LOW, but some other drivers (e.g: abx80x_ioctl, 
pcf2127_rtc_ioctl and pcf8523_rtc_ioctl) use RTC_VL_BACKUP_LOW instead. 
Is there any guideline or document telling the differences between them?

I can change it to RTC_VL_BACKUP_LOW of course.

> 
>> +                     break;
>> +             default:
>> +                     if (ctrl2 & RS5C_CTRL2_XSTP)
>> +                             flags |= RTC_VL_DATA_INVALID;
>> +                     break;
>> +             }
>> +
>> +             return put_user(flags, (unsigned int __user *)arg);
>> +     case RTC_VL_CLR:
>> +             /* clear rtc VDET, PON and XSTP bits */
>> +             switch (rs5c->type) {
>> +             case rtc_r2025sd:
>> +             case rtc_r2221tl:
>> +                     ctrl2 &= ~(R2x2x_CTRL2_VDET | R2x2x_CTRL2_PON);
>> +                     if (rs5c->type == rtc_r2025sd)
>> +                             ctrl2 |= R2x2x_CTRL2_XSTP;
>> +                     else
>> +                             ctrl2 &= ~R2x2x_CTRL2_XSTP;
>> +                     break;
>> +             default:
>> +                     ctrl2 &= ~RS5C_CTRL2_XSTP;
> 
> You can clear VDET but you must nt clear PON or XSTP as they are used to
> know whether the time on the RTC has been set correctly. Clearing those
> can only be done in .set_time.

Will update shortly.

> 
>> +                     break;
>> +             }
>> +
>> +             if (i2c_smbus_write_byte_data(rs5c->client, addr, ctrl2) < 0) {
>> +                     dev_dbg(&rs5c->client->dev, "%s: write error in line %i\n",
>> +                                     __func__, __LINE__);
>> +                     return -EIO;
>> +             }
>> +             return 0;
>> +     default:
>> +             return -ENOIOCTLCMD;
>> +     }
>> +     return 0;
>> +}
>> +#else
>> +#define rs5c372_ioctl        NULL
>> +#endif
>> +
>>  static const struct rtc_class_ops rs5c372_rtc_ops = {
>>        .proc           = rs5c372_rtc_proc,
>>        .read_time      = rs5c372_rtc_read_time,
>> @@ -492,6 +557,7 @@ static const struct rtc_class_ops rs5c372_rtc_ops = {
>>        .read_alarm     = rs5c_read_alarm,
>>        .set_alarm      = rs5c_set_alarm,
>>        .alarm_irq_enable = rs5c_rtc_alarm_irq_enable,
>> +     .ioctl          = rs5c372_ioctl,
>>  };
>>  
>>  #if IS_ENABLED(CONFIG_RTC_INTF_SYSFS)
>> -- 
>> 2.20.1
>> 
> 
> -- 
> Alexandre Belloni, co-owner and COO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] rtc: rs5c372: Add RTC_VL_READ, RTC_VL_CLR ioctls
  2021-11-10 14:03   ` Camel Guo
@ 2021-11-10 14:27     ` Alexandre Belloni
  2021-11-10 14:30       ` Camel Guo
  0 siblings, 1 reply; 7+ messages in thread
From: Alexandre Belloni @ 2021-11-10 14:27 UTC (permalink / raw)
  To: Camel Guo; +Cc: Camel Guo, Alessandro Zummo, kernel, linux-rtc, linux-kernel

On 10/11/2021 15:03:49+0100, Camel Guo wrote:
> > On 10/11/2021 12:54:54+0100, Camel Guo wrote:
> > > From: Camel Guo <camelg@axis.com>
> > > +     switch (cmd) {
> > > +     case RTC_VL_READ:
> > > +             flags = 0;
> > > +
> > > +             switch (rs5c->type) {
> > > +             case rtc_r2025sd:
> > > +             case rtc_r2221tl:
> > > +                     if ((rs5c->type == rtc_r2025sd && !(ctrl2 & R2x2x_CTRL2_XSTP)) ||
> > > +                             (rs5c->type == rtc_r2221tl &&  (ctrl2 & R2x2x_CTRL2_XSTP))) {
> > > +                             flags |= RTC_VL_DATA_INVALID;
> > > +                     }
> > > +                     if (ctrl2 & R2x2x_CTRL2_VDET)
> > > +                             flags |= RTC_VL_ACCURACY_LOW;
> > 
> > Shouldn't that be RTC_VL_BACKUP_LOW?
> 
> Some drivers (e.g: rv3029_ioctl and rv8803_ioctl) use RTC_VL_ACCURACY_LOW,
> but some other drivers (e.g: abx80x_ioctl, pcf2127_rtc_ioctl and
> pcf8523_rtc_ioctl) use RTC_VL_BACKUP_LOW instead. Is there any guideline or
> document telling the differences between them?
> 

RTC_VL_BACKUP_LOW: The backup voltage is low
RTC_VL_ACCURACY_LOW: the primary or backup voltage is low, temperature
compensation (or similar) has stopped

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] rtc: rs5c372: Add RTC_VL_READ, RTC_VL_CLR ioctls
  2021-11-10 14:27     ` Alexandre Belloni
@ 2021-11-10 14:30       ` Camel Guo
  2021-11-10 15:01         ` Camel Guo
  2021-11-11  8:38         ` Camel Guo
  0 siblings, 2 replies; 7+ messages in thread
From: Camel Guo @ 2021-11-10 14:30 UTC (permalink / raw)
  To: Alexandre Belloni, Camel Guo
  Cc: Alessandro Zummo, kernel, linux-rtc, linux-kernel

Hello,

On 11/10/21 3:27 PM, Alexandre Belloni wrote:
> On 10/11/2021 15:03:49+0100, Camel Guo wrote:
>> > On 10/11/2021 12:54:54+0100, Camel Guo wrote:
>> > > From: Camel Guo <camelg@axis.com>
>> > > +     switch (cmd) {
>> > > +     case RTC_VL_READ:
>> > > +             flags = 0;
>> > > +
>> > > +             switch (rs5c->type) {
>> > > +             case rtc_r2025sd:
>> > > +             case rtc_r2221tl:
>> > > +                     if ((rs5c->type == rtc_r2025sd && !(ctrl2 & R2x2x_CTRL2_XSTP)) ||
>> > > +                             (rs5c->type == rtc_r2221tl &&  (ctrl2 & R2x2x_CTRL2_XSTP))) {
>> > > +                             flags |= RTC_VL_DATA_INVALID;
>> > > +                     }
>> > > +                     if (ctrl2 & R2x2x_CTRL2_VDET)
>> > > +                             flags |= RTC_VL_ACCURACY_LOW;
>> > 
>> > Shouldn't that be RTC_VL_BACKUP_LOW?
>> 
>> Some drivers (e.g: rv3029_ioctl and rv8803_ioctl) use RTC_VL_ACCURACY_LOW,
>> but some other drivers (e.g: abx80x_ioctl, pcf2127_rtc_ioctl and
>> pcf8523_rtc_ioctl) use RTC_VL_BACKUP_LOW instead. Is there any guideline or
>> document telling the differences between them?
>> 
> 
> RTC_VL_BACKUP_LOW: The backup voltage is low
> RTC_VL_ACCURACY_LOW: the primary or backup voltage is low, temperature
> compensation (or similar) has stopped

Then I agree that we should go for RTC_VL_BACKUP_LOW.

> 
> -- 
> Alexandre Belloni, co-owner and COO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] rtc: rs5c372: Add RTC_VL_READ, RTC_VL_CLR ioctls
  2021-11-10 14:30       ` Camel Guo
@ 2021-11-10 15:01         ` Camel Guo
  2021-11-11  8:38         ` Camel Guo
  1 sibling, 0 replies; 7+ messages in thread
From: Camel Guo @ 2021-11-10 15:01 UTC (permalink / raw)
  To: Camel Guo, Alexandre Belloni
  Cc: Alessandro Zummo, kernel, linux-rtc, linux-kernel

Patch V2 has been uploaded. Please review patch v2 instead.

On 11/10/21 3:30 PM, Camel Guo wrote:
> Hello,
> 
> On 11/10/21 3:27 PM, Alexandre Belloni wrote:
>> On 10/11/2021 15:03:49+0100, Camel Guo wrote:
>>> > On 10/11/2021 12:54:54+0100, Camel Guo wrote:
>>> > > From: Camel Guo <camelg@axis.com>
>>> > > +     switch (cmd) {
>>> > > +     case RTC_VL_READ:
>>> > > +             flags = 0;
>>> > > +
>>> > > +             switch (rs5c->type) {
>>> > > +             case rtc_r2025sd:
>>> > > +             case rtc_r2221tl:
>>> > > +                     if ((rs5c->type == rtc_r2025sd && !(ctrl2 & R2x2x_CTRL2_XSTP)) ||
>>> > > +                             (rs5c->type == rtc_r2221tl &&  (ctrl2 & R2x2x_CTRL2_XSTP))) {
>>> > > +                             flags |= RTC_VL_DATA_INVALID;
>>> > > +                     }
>>> > > +                     if (ctrl2 & R2x2x_CTRL2_VDET)
>>> > > +                             flags |= RTC_VL_ACCURACY_LOW;
>>> > 
>>> > Shouldn't that be RTC_VL_BACKUP_LOW?
>>> 
>>> Some drivers (e.g: rv3029_ioctl and rv8803_ioctl) use RTC_VL_ACCURACY_LOW,
>>> but some other drivers (e.g: abx80x_ioctl, pcf2127_rtc_ioctl and
>>> pcf8523_rtc_ioctl) use RTC_VL_BACKUP_LOW instead. Is there any guideline or
>>> document telling the differences between them?
>>> 
>> 
>> RTC_VL_BACKUP_LOW: The backup voltage is low
>> RTC_VL_ACCURACY_LOW: the primary or backup voltage is low, temperature
>> compensation (or similar) has stopped
> 
> Then I agree that we should go for RTC_VL_BACKUP_LOW.
> 
>> 
>> -- 
>> Alexandre Belloni, co-owner and COO, Bootlin
>> Embedded Linux and Kernel engineering
>> https://bootlin.com

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] rtc: rs5c372: Add RTC_VL_READ, RTC_VL_CLR ioctls
  2021-11-10 14:30       ` Camel Guo
  2021-11-10 15:01         ` Camel Guo
@ 2021-11-11  8:38         ` Camel Guo
  1 sibling, 0 replies; 7+ messages in thread
From: Camel Guo @ 2021-11-11  8:38 UTC (permalink / raw)
  To: Camel Guo, Alexandre Belloni
  Cc: Alessandro Zummo, kernel, linux-rtc, linux-kernel

On 11/10/21 3:30 PM, Camel Guo wrote:
> Hello,
> 
> On 11/10/21 3:27 PM, Alexandre Belloni wrote:
>> On 10/11/2021 15:03:49+0100, Camel Guo wrote:
>>> > On 10/11/2021 12:54:54+0100, Camel Guo wrote:
>>> > > From: Camel Guo <camelg@axis.com>
>>> > > +     switch (cmd) {
>>> > > +     case RTC_VL_READ:
>>> > > +             flags = 0;
>>> > > +
>>> > > +             switch (rs5c->type) {
>>> > > +             case rtc_r2025sd:
>>> > > +             case rtc_r2221tl:
>>> > > +                     if ((rs5c->type == rtc_r2025sd && !(ctrl2 & R2x2x_CTRL2_XSTP)) ||
>>> > > +                             (rs5c->type == rtc_r2221tl &&  (ctrl2 & R2x2x_CTRL2_XSTP))) {
>>> > > +                             flags |= RTC_VL_DATA_INVALID;
>>> > > +                     }
>>> > > +                     if (ctrl2 & R2x2x_CTRL2_VDET)
>>> > > +                             flags |= RTC_VL_ACCURACY_LOW;
>>> > 
>>> > Shouldn't that be RTC_VL_BACKUP_LOW?

Fixed in V3.

>>> 
>>> Some drivers (e.g: rv3029_ioctl and rv8803_ioctl) use RTC_VL_ACCURACY_LOW,
>>> but some other drivers (e.g: abx80x_ioctl, pcf2127_rtc_ioctl and
>>> pcf8523_rtc_ioctl) use RTC_VL_BACKUP_LOW instead. Is there any guideline or
>>> document telling the differences between them?
>>> 
>> 
>> RTC_VL_BACKUP_LOW: The backup voltage is low
>> RTC_VL_ACCURACY_LOW: the primary or backup voltage is low, temperature
>> compensation (or similar) has stopped
> 
> Then I agree that we should go for RTC_VL_BACKUP_LOW.

Fixed in V3.

> 
>> 
>> -- 
>> Alexandre Belloni, co-owner and COO, Bootlin
>> Embedded Linux and Kernel engineering
>> https://bootlin.com

All comments are fixed in V3, please review it again.


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-11-11  8:38 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-10 11:54 [PATCH] rtc: rs5c372: Add RTC_VL_READ, RTC_VL_CLR ioctls Camel Guo
2021-11-10 13:54 ` Alexandre Belloni
2021-11-10 14:03   ` Camel Guo
2021-11-10 14:27     ` Alexandre Belloni
2021-11-10 14:30       ` Camel Guo
2021-11-10 15:01         ` Camel Guo
2021-11-11  8:38         ` Camel Guo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).