From: Camel Guo <[email protected]>
This commit adds support of offset correction by configuring the
oscillation adjustment register of rs5c372.
Signed-off-by: Camel Guo <[email protected]>
---
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
On 11/30/21 10:50 AM, Camel Guo wrote:
> From: Camel Guo <[email protected]>
>
> This commit adds support of offset correction by configuring the
> oscillation adjustment register of rs5c372.
>
> Signed-off-by: Camel Guo <[email protected]>
> ---
> ?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.
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.
On 11/30/21 3:55 PM, Camel Guo wrote:
> On 11/30/21 10:50 AM, Camel Guo wrote:
>> From: Camel Guo <[email protected]>
>>
>> This commit adds support of offset correction by configuring the
>> oscillation adjustment register of rs5c372.
>>
>> Signed-off-by: Camel Guo <[email protected]>
>> ---
>> ??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.