linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] rtc: rs5c372: add offset correction support
@ 2021-11-30  9:50 Camel Guo
  2021-11-30 14:55 ` Camel Guo
  0 siblings, 1 reply; 4+ messages in thread
From: Camel Guo @ 2021-11-30  9:50 UTC (permalink / raw)
  To: Alessandro Zummo, Alexandre Belloni
  Cc: kernel, Camel Guo, linux-rtc, linux-kernel

From: Camel Guo <camelg@axis.com>

This commit adds support of offset correction by configuring the
oscillation adjustment register of rs5c372.

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

diff --git a/drivers/rtc/rtc-rs5c372.c b/drivers/rtc/rtc-rs5c372.c
index 80980414890c..77027498cad5 100644
--- a/drivers/rtc/rtc-rs5c372.c
+++ b/drivers/rtc/rtc-rs5c372.c
@@ -30,6 +30,8 @@
 #define RS5C372_REG_TRIM	7
 #	define RS5C372_TRIM_XSL		0x80
 #	define RS5C372_TRIM_MASK	0x7F
+#	define RS5C372_TRIM_DEV		(1 << 7)
+#	define RS5C372_TRIM_DECR	(1 << 6)
 
 #define RS5C_REG_ALARM_A_MIN	8			/* or ALARM_W */
 #define RS5C_REG_ALARM_A_HOURS	9
@@ -485,6 +487,82 @@ static int rs5c372_rtc_proc(struct device *dev, struct seq_file *seq)
 #define	rs5c372_rtc_proc	NULL
 #endif
 
+static int rs5c372_read_offset(struct device *dev, long *offset)
+{
+	struct rs5c372 *rs5c = i2c_get_clientdata(to_i2c_client(dev));
+	int addr = RS5C_ADDR(RS5C372_REG_TRIM);
+	unsigned char val = i2c_smbus_read_byte_data(rs5c->client, addr);
+	long ppb_per_step = (val & RS5C372_TRIM_DEV) ? 1017 : 3052;
+	unsigned char decr = val & RS5C372_TRIM_DECR;
+
+	/* Only bits[0:5] repsents the time counts */
+	val &= 0x3F;
+
+	/* If bits[1:5] are all 0, it means no increment or decrement */
+	if (!(val & 0x3E)) {
+		*offset = 0;
+	} else {
+		if (decr)
+			*offset = -(((~val) & 0x3F) + 1) * ppb_per_step;
+		else
+			*offset = (val - 1) * ppb_per_step;
+	}
+
+	return 0;
+}
+
+static int rs5c372_set_offset(struct device *dev, long offset)
+{
+	struct rs5c372 *rs5c = i2c_get_clientdata(to_i2c_client(dev));
+	int addr = RS5C_ADDR(RS5C372_REG_TRIM);
+	unsigned char val = RS5C372_TRIM_DEV;
+	long steps = 0;
+
+	/*
+	 * Check if it is possible to use high resolution mode (DEV=1). In this
+	 * mode, the minimum resolution is 2 / (32768 * 20 * 3), which is about
+	 * 1017 ppb
+	 */
+	steps = DIV_ROUND_CLOSEST(offset, 1017);
+	if (steps > 0x3E || steps < -0x3E) {
+		/*
+		 * offset is out of the range of high resolution mode. Try to
+		 * use low resolution mode (DEV=0). In this mode, the minimum
+		 * resolution is 2 / (32768 * 20), which is about 3052 ppb.
+		 */
+		val &= ~RS5C372_TRIM_DEV;
+		steps = DIV_ROUND_CLOSEST(offset, 3052);
+
+		if (steps > 0x3E || steps < -0x3E)
+			return -ERANGE;
+	}
+
+	if (steps > 0) {
+		val |= steps + 1;
+	} else {
+		val |= RS5C372_TRIM_DECR;
+		val |= (~(-steps - 1)) & 0x3F;
+	}
+
+	if (!steps || !(val & 0x3E)) {
+		/*
+		 * if offset is too small, set oscillation adjustment register
+		 * with the default values, which means no increment or
+		 * decrement.
+		 */
+		val = 0;
+	}
+
+	dev_dbg(&rs5c->client->dev, "write 0x%x for offset %ld\n", val, offset);
+
+	if (i2c_smbus_write_byte_data(rs5c->client, addr, val) < 0) {
+		dev_err(&rs5c->client->dev, "failed to write 0x%x to reg %d\n", val, addr);
+		return -EIO;
+	}
+
+	return 0;
+}
+
 static const struct rtc_class_ops rs5c372_rtc_ops = {
 	.proc		= rs5c372_rtc_proc,
 	.read_time	= rs5c372_rtc_read_time,
@@ -492,6 +570,8 @@ 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,
+	.read_offset    = rs5c372_read_offset,
+	.set_offset     = rs5c372_set_offset,
 };
 
 #if IS_ENABLED(CONFIG_RTC_INTF_SYSFS)
-- 
2.20.1


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

* Re: [PATCH] rtc: rs5c372: add offset correction support
  2021-11-30  9:50 [PATCH] rtc: rs5c372: add offset correction support Camel Guo
@ 2021-11-30 14:55 ` Camel Guo
  2021-12-01  8:43   ` Vincent Whitchurch
  2021-12-02 15:25   ` Camel Guo
  0 siblings, 2 replies; 4+ messages in thread
From: Camel Guo @ 2021-11-30 14:55 UTC (permalink / raw)
  To: Camel Guo, Alessandro Zummo, Alexandre Belloni
  Cc: kernel, linux-rtc, linux-kernel

On 11/30/21 10:50 AM, Camel Guo wrote:
> From: Camel Guo <camelg@axis.com>
> 
> This commit adds support of offset correction by configuring the
> oscillation adjustment register of rs5c372.
> 
> Signed-off-by: Camel Guo <camelg@axis.com>
> ---
>   drivers/rtc/rtc-rs5c372.c | 80 +++++++++++++++++++++++++++++++++++++++
>   1 file changed, 80 insertions(+)
> 
> diff --git a/drivers/rtc/rtc-rs5c372.c b/drivers/rtc/rtc-rs5c372.c
> index 80980414890c..77027498cad5 100644
> --- a/drivers/rtc/rtc-rs5c372.c
> +++ b/drivers/rtc/rtc-rs5c372.c
> @@ -30,6 +30,8 @@
>   #define RS5C372_REG_TRIM        7
>   #       define RS5C372_TRIM_XSL         0x80
>   #       define RS5C372_TRIM_MASK        0x7F
> +#      define RS5C372_TRIM_DEV         (1 << 7)
> +#      define RS5C372_TRIM_DECR        (1 << 6)
> 
>   #define RS5C_REG_ALARM_A_MIN    8                       /* or ALARM_W */
>   #define RS5C_REG_ALARM_A_HOURS  9
> @@ -485,6 +487,82 @@ static int rs5c372_rtc_proc(struct device *dev, 
> struct seq_file *seq)
>   #define rs5c372_rtc_proc        NULL
>   #endif
> 
> +static int rs5c372_read_offset(struct device *dev, long *offset)
> +{
> +       struct rs5c372 *rs5c = i2c_get_clientdata(to_i2c_client(dev));
> +       int addr = RS5C_ADDR(RS5C372_REG_TRIM);
> +       unsigned char val = i2c_smbus_read_byte_data(rs5c->client, addr);
> +       long ppb_per_step = (val & RS5C372_TRIM_DEV) ? 1017 : 3052;
> +       unsigned char decr = val & RS5C372_TRIM_DECR;
> +
> +       /* Only bits[0:5] repsents the time counts */
> +       val &= 0x3F;
> +
> +       /* If bits[1:5] are all 0, it means no increment or decrement */
> +       if (!(val & 0x3E)) {
> +               *offset = 0;
> +       } else {
> +               if (decr)
> +                       *offset = -(((~val) & 0x3F) + 1) * ppb_per_step;
> +               else
> +                       *offset = (val - 1) * ppb_per_step;
> +       }
> +
> +       return 0;
> +}
> +
> +static int rs5c372_set_offset(struct device *dev, long offset)
> +{
> +       struct rs5c372 *rs5c = i2c_get_clientdata(to_i2c_client(dev));
> +       int addr = RS5C_ADDR(RS5C372_REG_TRIM);
> +       unsigned char val = RS5C372_TRIM_DEV;
> +       long steps = 0;
> +
> +       /*
> +        * Check if it is possible to use high resolution mode (DEV=1). 
> In this
> +        * mode, the minimum resolution is 2 / (32768 * 20 * 3), which 
> is about
> +        * 1017 ppb
> +        */
> +       steps = DIV_ROUND_CLOSEST(offset, 1017);
> +       if (steps > 0x3E || steps < -0x3E) {
> +               /*
> +                * offset is out of the range of high resolution mode. 
> Try to
> +                * use low resolution mode (DEV=0). In this mode, the 
> minimum
> +                * resolution is 2 / (32768 * 20), which is about 3052 ppb.
> +                */
> +               val &= ~RS5C372_TRIM_DEV;
> +               steps = DIV_ROUND_CLOSEST(offset, 3052);
> +
> +               if (steps > 0x3E || steps < -0x3E)
> +                       return -ERANGE;
> +       }
> +
> +       if (steps > 0) {
> +               val |= steps + 1;
> +       } else {
> +               val |= RS5C372_TRIM_DECR;
> +               val |= (~(-steps - 1)) & 0x3F;
> +       }
> +
> +       if (!steps || !(val & 0x3E)) {
> +               /*
> +                * if offset is too small, set oscillation adjustment 
> register
> +                * with the default values, which means no increment or
> +                * decrement.
> +                */
> +               val = 0;
> +       }
> +
> +       dev_dbg(&rs5c->client->dev, "write 0x%x for offset %ld\n", val, 
> offset);
> +
> +       if (i2c_smbus_write_byte_data(rs5c->client, addr, val) < 0) {
> +               dev_err(&rs5c->client->dev, "failed to write 0x%x to reg 
> %d\n", val, addr);
> +               return -EIO;
> +       }
> +
> +       return 0;
> +}
> +
>   static const struct rtc_class_ops rs5c372_rtc_ops = {
>           .proc           = rs5c372_rtc_proc,
>           .read_time      = rs5c372_rtc_read_time,
> @@ -492,6 +570,8 @@ 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,
> +       .read_offset    = rs5c372_read_offset,
> +       .set_offset     = rs5c372_set_offset,
>   };
> 
>   #if IS_ENABLED(CONFIG_RTC_INTF_SYSFS)
> -- 
> 2.20.1
> 

Just realize that the oscillation adjustment registers of r2*, rs5c* are 
not totally same and all of these differences need to be handled in 
different way. Hence this patch needs to be updated to cover all cases.

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

* Re: [PATCH] rtc: rs5c372: add offset correction support
  2021-11-30 14:55 ` Camel Guo
@ 2021-12-01  8:43   ` Vincent Whitchurch
  2021-12-02 15:25   ` Camel Guo
  1 sibling, 0 replies; 4+ messages in thread
From: Vincent Whitchurch @ 2021-12-01  8:43 UTC (permalink / raw)
  To: Camel Guo
  Cc: Alessandro Zummo, Alexandre Belloni, linux-rtc, kernel, linux-kernel

On Tue, Nov 30, 2021 at 03:55:43PM +0100, Camel Guo wrote:
> Just realize that the oscillation adjustment registers of r2*, rs5c* are 
> not totally same and all of these differences need to be handled in 
> different way. Hence this patch needs to be updated to cover all cases.

Unless you have the hardware to actually test the other variants,
perhaps it would be safer to just reject them in ->set_offset /
->get_offset for now?  Returning -EINVAL for unsupported types should
make rtc_set_offset() and rtc_get_offset() have the same behaviour as
not implementing the callbacks for these variants.

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

* Re: [PATCH] rtc: rs5c372: add offset correction support
  2021-11-30 14:55 ` Camel Guo
  2021-12-01  8:43   ` Vincent Whitchurch
@ 2021-12-02 15:25   ` Camel Guo
  1 sibling, 0 replies; 4+ messages in thread
From: Camel Guo @ 2021-12-02 15:25 UTC (permalink / raw)
  To: Camel Guo, Alessandro Zummo, Alexandre Belloni
  Cc: kernel, linux-rtc, linux-kernel

On 11/30/21 3:55 PM, Camel Guo wrote:
> On 11/30/21 10:50 AM, Camel Guo wrote:
>> From: Camel Guo <camelg@axis.com>
>>
>> This commit adds support of offset correction by configuring the
>> oscillation adjustment register of rs5c372.
>>
>> Signed-off-by: Camel Guo <camelg@axis.com>
>> ---
>>   drivers/rtc/rtc-rs5c372.c | 80 +++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 80 insertions(+)
>>
>> diff --git a/drivers/rtc/rtc-rs5c372.c b/drivers/rtc/rtc-rs5c372.c
>> index 80980414890c..77027498cad5 100644
>> --- a/drivers/rtc/rtc-rs5c372.c
>> +++ b/drivers/rtc/rtc-rs5c372.c
>> @@ -30,6 +30,8 @@
>>   #define RS5C372_REG_TRIM        7
>>   #       define RS5C372_TRIM_XSL         0x80
>>   #       define RS5C372_TRIM_MASK        0x7F
>> +#      define RS5C372_TRIM_DEV         (1 << 7)
>> +#      define RS5C372_TRIM_DECR        (1 << 6)
>>
>>   #define RS5C_REG_ALARM_A_MIN    8                       /* or 
>> ALARM_W */
>>   #define RS5C_REG_ALARM_A_HOURS  9
>> @@ -485,6 +487,82 @@ static int rs5c372_rtc_proc(struct device *dev, 
>> struct seq_file *seq)
>>   #define rs5c372_rtc_proc        NULL
>>   #endif
>>
>> +static int rs5c372_read_offset(struct device *dev, long *offset)
>> +{
>> +       struct rs5c372 *rs5c = i2c_get_clientdata(to_i2c_client(dev));
>> +       int addr = RS5C_ADDR(RS5C372_REG_TRIM);
>> +       unsigned char val = i2c_smbus_read_byte_data(rs5c->client, addr);
>> +       long ppb_per_step = (val & RS5C372_TRIM_DEV) ? 1017 : 3052;
>> +       unsigned char decr = val & RS5C372_TRIM_DECR;
>> +
>> +       /* Only bits[0:5] repsents the time counts */
>> +       val &= 0x3F;
>> +
>> +       /* If bits[1:5] are all 0, it means no increment or decrement */
>> +       if (!(val & 0x3E)) {
>> +               *offset = 0;
>> +       } else {
>> +               if (decr)
>> +                       *offset = -(((~val) & 0x3F) + 1) * ppb_per_step;
>> +               else
>> +                       *offset = (val - 1) * ppb_per_step;
>> +       }
>> +
>> +       return 0;
>> +}
>> +
>> +static int rs5c372_set_offset(struct device *dev, long offset)
>> +{
>> +       struct rs5c372 *rs5c = i2c_get_clientdata(to_i2c_client(dev));
>> +       int addr = RS5C_ADDR(RS5C372_REG_TRIM);
>> +       unsigned char val = RS5C372_TRIM_DEV;
>> +       long steps = 0;
>> +
>> +       /*
>> +        * Check if it is possible to use high resolution mode 
>> (DEV=1). In this
>> +        * mode, the minimum resolution is 2 / (32768 * 20 * 3), which 
>> is about
>> +        * 1017 ppb
>> +        */
>> +       steps = DIV_ROUND_CLOSEST(offset, 1017);
>> +       if (steps > 0x3E || steps < -0x3E) {
>> +               /*
>> +                * offset is out of the range of high resolution mode. 
>> Try to
>> +                * use low resolution mode (DEV=0). In this mode, the 
>> minimum
>> +                * resolution is 2 / (32768 * 20), which is about 3052 
>> ppb.
>> +                */
>> +               val &= ~RS5C372_TRIM_DEV;
>> +               steps = DIV_ROUND_CLOSEST(offset, 3052);
>> +
>> +               if (steps > 0x3E || steps < -0x3E)
>> +                       return -ERANGE;
>> +       }
>> +
>> +       if (steps > 0) {
>> +               val |= steps + 1;
>> +       } else {
>> +               val |= RS5C372_TRIM_DECR;
>> +               val |= (~(-steps - 1)) & 0x3F;
>> +       }
>> +
>> +       if (!steps || !(val & 0x3E)) {
>> +               /*
>> +                * if offset is too small, set oscillation adjustment 
>> register
>> +                * with the default values, which means no increment or
>> +                * decrement.
>> +                */
>> +               val = 0;
>> +       }
>> +
>> +       dev_dbg(&rs5c->client->dev, "write 0x%x for offset %ld\n", 
>> val, offset);
>> +
>> +       if (i2c_smbus_write_byte_data(rs5c->client, addr, val) < 0) {
>> +               dev_err(&rs5c->client->dev, "failed to write 0x%x to 
>> reg %d\n", val, addr);
>> +               return -EIO;
>> +       }
>> +
>> +       return 0;
>> +}
>> +
>>   static const struct rtc_class_ops rs5c372_rtc_ops = {
>>           .proc           = rs5c372_rtc_proc,
>>           .read_time      = rs5c372_rtc_read_time,
>> @@ -492,6 +570,8 @@ 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,
>> +       .read_offset    = rs5c372_read_offset,
>> +       .set_offset     = rs5c372_set_offset,
>>   };
>>
>>   #if IS_ENABLED(CONFIG_RTC_INTF_SYSFS)
>> -- 
>> 2.20.1
>>
> 
> Just realize that the oscillation adjustment registers of r2*, rs5c* are 
> not totally same and all of these differences need to be handled in 
> different way. Hence this patch needs to be updated to cover all cases.

Patch V2 has been uploaded. Please review that one instead.

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

end of thread, other threads:[~2021-12-02 15:25 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-30  9:50 [PATCH] rtc: rs5c372: add offset correction support Camel Guo
2021-11-30 14:55 ` Camel Guo
2021-12-01  8:43   ` Vincent Whitchurch
2021-12-02 15:25   ` 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).