* [PATCH 0/2] *** Ricoh RS5C372: add support for Xtal trimming configuration *** @ 2021-10-30 22:50 Pavel Modilaynen 2021-10-30 22:50 ` [PATCH 1/2] rtc: rs5c372: Add support for trim configuration Pavel Modilaynen 2021-10-30 22:50 ` [PATCH 2/2] dt-bindings: rtc: Add bindings for Ricoh rs5c372 Pavel Modilaynen 0 siblings, 2 replies; 8+ messages in thread From: Pavel Modilaynen @ 2021-10-30 22:50 UTC (permalink / raw) To: a.zummo, alexandre.belloni, robh+dt Cc: linux-rtc, devicetree, linux-kernel, lkml, kernel, Pavel Modilaynen From: Pavel Modilaynen <pavelmn@axis.com> *** BLURB HERE *** Add support for Xtal trimming configuration with new DT property: ricoh,trim. This uint8 property is to contain raw value to setup Oscillation Adjustment register (0x7) of Ricoh RS5C372 and similar RTC controllers. This raw value has device model specific format and is used to adjust (subtract or add) time counts per second (normally 32768) on regular intervals (e.g. every 20th second). Pavel Modilaynen (2): rtc: rs5c372: Add support for trim configuration dt-bindings: rtc: Add bindings for Ricoh rs5c372 .../bindings/rtc/ricoh,rs5c372.yaml | 58 +++++++++++++++++++ .../devicetree/bindings/rtc/trivial-rtc.yaml | 12 ---- drivers/rtc/rtc-rs5c372.c | 18 +++++- 3 files changed, 75 insertions(+), 13 deletions(-) create mode 100644 Documentation/devicetree/bindings/rtc/ricoh,rs5c372.yaml -- 2.20.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2] rtc: rs5c372: Add support for trim configuration 2021-10-30 22:50 [PATCH 0/2] *** Ricoh RS5C372: add support for Xtal trimming configuration *** Pavel Modilaynen @ 2021-10-30 22:50 ` Pavel Modilaynen 2021-10-30 22:57 ` Alexandre Belloni 2021-10-30 22:50 ` [PATCH 2/2] dt-bindings: rtc: Add bindings for Ricoh rs5c372 Pavel Modilaynen 1 sibling, 1 reply; 8+ messages in thread From: Pavel Modilaynen @ 2021-10-30 22:50 UTC (permalink / raw) To: a.zummo, alexandre.belloni, robh+dt Cc: linux-rtc, devicetree, linux-kernel, lkml, kernel, Pavel Modilaynen From: Pavel Modilaynen <pavelmn@axis.com> Add support for oscillation adjustment register RS5C372_REG_TRIM setting that is needed to accommodate for effective crystal capacitance. Use optional property ricoh,trim that should contain raw value to setup this register. According to datasheets for RS5C372, R2025S/D, RV5C38[67] and R222[13] the value will be converted to a number of ticks that is to be subtracted or added when the second digits read 00, 20 or 40 seconds. Signed-off-by: Pavel Modilaynen <pavelmn@axis.com> --- drivers/rtc/rtc-rs5c372.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/drivers/rtc/rtc-rs5c372.c b/drivers/rtc/rtc-rs5c372.c index 80980414890c..3a2db0326669 100644 --- a/drivers/rtc/rtc-rs5c372.c +++ b/drivers/rtc/rtc-rs5c372.c @@ -13,6 +13,7 @@ #include <linux/slab.h> #include <linux/module.h> #include <linux/of_device.h> +#include <linux/of.h> /* * Ricoh has a family of I2C based RTCs, which differ only slightly from @@ -560,6 +561,8 @@ static int rs5c_oscillator_setup(struct rs5c372 *rs5c372) { unsigned char buf[2]; int addr, i, ret = 0; + struct i2c_client *client = rs5c372->client; + u8 trim = 0; addr = RS5C_ADDR(RS5C_REG_CTRL1); buf[0] = rs5c372->regs[RS5C_REG_CTRL1]; @@ -599,9 +602,22 @@ static int rs5c_oscillator_setup(struct rs5c372 *rs5c372) break; } + /* optional setup of xtal trimming */ + if (!of_property_read_u8(client->dev.of_node, "ricoh,trim", &trim)) { + if (rs5c372->type != rtc_r2221tl && (trim & ~RS5C372_TRIM_MASK)) { + dev_warn(&client->dev, "Erroneous setting for ricoh,trim in devicetree\n"); + } else { + int addr = RS5C_ADDR(RS5C372_REG_TRIM); + int ret = i2c_smbus_write_byte_data(client, addr, trim); + + if (unlikely(ret < 0)) + return ret; + } + } + for (i = 0; i < sizeof(buf); i++) { addr = RS5C_ADDR(RS5C_REG_CTRL1 + i); - ret = i2c_smbus_write_byte_data(rs5c372->client, addr, buf[i]); + ret = i2c_smbus_write_byte_data(client, addr, buf[i]); if (unlikely(ret < 0)) return ret; } -- 2.20.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] rtc: rs5c372: Add support for trim configuration 2021-10-30 22:50 ` [PATCH 1/2] rtc: rs5c372: Add support for trim configuration Pavel Modilaynen @ 2021-10-30 22:57 ` Alexandre Belloni 2021-10-31 10:29 ` Pavel Modilaynen 0 siblings, 1 reply; 8+ messages in thread From: Alexandre Belloni @ 2021-10-30 22:57 UTC (permalink / raw) To: Pavel Modilaynen Cc: a.zummo, robh+dt, linux-rtc, devicetree, linux-kernel, lkml, kernel, Pavel Modilaynen Hello, Please use the proper RTC interface by implementing .set_offset and .read_offset. On 31/10/2021 00:50:53+0200, Pavel Modilaynen wrote: > From: Pavel Modilaynen <pavelmn@axis.com> > > Add support for oscillation adjustment register RS5C372_REG_TRIM > setting that is needed to accommodate for effective crystal > capacitance. > > Use optional property ricoh,trim that should contain > raw value to setup this register. According to > datasheets for RS5C372, R2025S/D, RV5C38[67] and R222[13] > the value will be converted to a number of ticks that > is to be subtracted or added when the second digits read > 00, 20 or 40 seconds. > > Signed-off-by: Pavel Modilaynen <pavelmn@axis.com> > --- > drivers/rtc/rtc-rs5c372.c | 18 +++++++++++++++++- > 1 file changed, 17 insertions(+), 1 deletion(-) > > diff --git a/drivers/rtc/rtc-rs5c372.c b/drivers/rtc/rtc-rs5c372.c > index 80980414890c..3a2db0326669 100644 > --- a/drivers/rtc/rtc-rs5c372.c > +++ b/drivers/rtc/rtc-rs5c372.c > @@ -13,6 +13,7 @@ > #include <linux/slab.h> > #include <linux/module.h> > #include <linux/of_device.h> > +#include <linux/of.h> > > /* > * Ricoh has a family of I2C based RTCs, which differ only slightly from > @@ -560,6 +561,8 @@ static int rs5c_oscillator_setup(struct rs5c372 *rs5c372) > { > unsigned char buf[2]; > int addr, i, ret = 0; > + struct i2c_client *client = rs5c372->client; > + u8 trim = 0; > > addr = RS5C_ADDR(RS5C_REG_CTRL1); > buf[0] = rs5c372->regs[RS5C_REG_CTRL1]; > @@ -599,9 +602,22 @@ static int rs5c_oscillator_setup(struct rs5c372 *rs5c372) > break; > } > > + /* optional setup of xtal trimming */ > + if (!of_property_read_u8(client->dev.of_node, "ricoh,trim", &trim)) { > + if (rs5c372->type != rtc_r2221tl && (trim & ~RS5C372_TRIM_MASK)) { > + dev_warn(&client->dev, "Erroneous setting for ricoh,trim in devicetree\n"); > + } else { > + int addr = RS5C_ADDR(RS5C372_REG_TRIM); > + int ret = i2c_smbus_write_byte_data(client, addr, trim); > + > + if (unlikely(ret < 0)) > + return ret; > + } > + } > + > for (i = 0; i < sizeof(buf); i++) { > addr = RS5C_ADDR(RS5C_REG_CTRL1 + i); > - ret = i2c_smbus_write_byte_data(rs5c372->client, addr, buf[i]); > + ret = i2c_smbus_write_byte_data(client, addr, buf[i]); > if (unlikely(ret < 0)) > return ret; > } > -- > 2.20.1 > -- Alexandre Belloni, co-owner and COO, Bootlin Embedded Linux and Kernel engineering https://bootlin.com ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] rtc: rs5c372: Add support for trim configuration 2021-10-30 22:57 ` Alexandre Belloni @ 2021-10-31 10:29 ` Pavel Modilaynen 2021-11-01 18:23 ` Alexandre Belloni 0 siblings, 1 reply; 8+ messages in thread From: Pavel Modilaynen @ 2021-10-31 10:29 UTC (permalink / raw) To: Alexandre Belloni, Pavel Modilaynen Cc: a.zummo@towertech.it, robh+dt@kernel.org, linux-rtc@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, lkml@axis.com, kernel Hi Alexandre, On 10/31/21 12:57 AM, Alexandre Belloni wrote: > Hello, > > Please use the proper RTC interface by implementing .set_offset and > .read_offset. I am not sure about .set/read_offset. It looks as runtime adjustment interface, however this Xtal trimming parameter is based on schematics and Xtal capacitance (datasheet parameter). It is found by calibration procedure based on RTC clock output (the procedure and calculation of trimming parameter is described in datasheets). So, I would like to say that this parameter is functionally close to "quartz-load-femtofarads" for rtc-pcf8523/pcf85063. > > On 31/10/2021 00:50:53+0200, Pavel Modilaynen wrote: >> From: Pavel Modilaynen <pavelmn@axis.com> >> >> Add support for oscillation adjustment register RS5C372_REG_TRIM >> setting that is needed to accommodate for effective crystal >> capacitance. >> >> Use optional property ricoh,trim that should contain >> raw value to setup this register. According to >> datasheets for RS5C372, R2025S/D, RV5C38[67] and R222[13] >> the value will be converted to a number of ticks that >> is to be subtracted or added when the second digits read >> 00, 20 or 40 seconds. >> >> Signed-off-by: Pavel Modilaynen <pavelmn@axis.com> >> --- >> drivers/rtc/rtc-rs5c372.c | 18 +++++++++++++++++- >> 1 file changed, 17 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/rtc/rtc-rs5c372.c b/drivers/rtc/rtc-rs5c372.c >> index 80980414890c..3a2db0326669 100644 >> --- a/drivers/rtc/rtc-rs5c372.c >> +++ b/drivers/rtc/rtc-rs5c372.c >> @@ -13,6 +13,7 @@ >> #include <linux/slab.h> >> #include <linux/module.h> >> #include <linux/of_device.h> >> +#include <linux/of.h> >> >> /* >> * Ricoh has a family of I2C based RTCs, which differ only slightly from >> @@ -560,6 +561,8 @@ static int rs5c_oscillator_setup(struct rs5c372 *rs5c372) >> { >> unsigned char buf[2]; >> int addr, i, ret = 0; >> + struct i2c_client *client = rs5c372->client; >> + u8 trim = 0; >> >> addr = RS5C_ADDR(RS5C_REG_CTRL1); >> buf[0] = rs5c372->regs[RS5C_REG_CTRL1]; >> @@ -599,9 +602,22 @@ static int rs5c_oscillator_setup(struct rs5c372 *rs5c372) >> break; >> } >> >> + /* optional setup of xtal trimming */ >> + if (!of_property_read_u8(client->dev.of_node, "ricoh,trim", &trim)) { >> + if (rs5c372->type != rtc_r2221tl && (trim & ~RS5C372_TRIM_MASK)) { >> + dev_warn(&client->dev, "Erroneous setting for ricoh,trim in devicetree\n"); >> + } else { >> + int addr = RS5C_ADDR(RS5C372_REG_TRIM); >> + int ret = i2c_smbus_write_byte_data(client, addr, trim); >> + >> + if (unlikely(ret < 0)) >> + return ret; >> + } >> + } >> + >> for (i = 0; i < sizeof(buf); i++) { >> addr = RS5C_ADDR(RS5C_REG_CTRL1 + i); >> - ret = i2c_smbus_write_byte_data(rs5c372->client, addr, buf[i]); >> + ret = i2c_smbus_write_byte_data(client, addr, buf[i]); >> if (unlikely(ret < 0)) >> return ret; >> } >> -- >> 2.20.1 >> > > -- > Alexandre Belloni, co-owner and COO, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com <https://bootlin.com> ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] rtc: rs5c372: Add support for trim configuration 2021-10-31 10:29 ` Pavel Modilaynen @ 2021-11-01 18:23 ` Alexandre Belloni 2021-11-01 23:14 ` Pavel Modilaynen 0 siblings, 1 reply; 8+ messages in thread From: Alexandre Belloni @ 2021-11-01 18:23 UTC (permalink / raw) To: Pavel Modilaynen Cc: Pavel Modilaynen, a.zummo@towertech.it, robh+dt@kernel.org, linux-rtc@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, lkml@axis.com, kernel On 31/10/2021 11:29:12+0100, Pavel Modilaynen wrote: > On 10/31/21 12:57 AM, Alexandre Belloni wrote: > > Hello, > > > > Please use the proper RTC interface by implementing .set_offset and > > .read_offset. > > I am not sure about .set/read_offset. It looks as runtime adjustment > interface, > however this Xtal trimming parameter is based on schematics and Xtal > capacitance (datasheet parameter). > It is found by calibration procedure based on RTC clock output (the > procedure and calculation of trimming parameter is described in datasheets). > So, I would like to say that this parameter is functionally close to > "quartz-load-femtofarads" for rtc-pcf8523/pcf85063. > quartz-load-femtofarads is for analog trimming which this RTC doesn't have, both CD and CG are set to 10pF. .set/read_offset are for digital trimming which is what you are configuring here. You definitively want to be able to do that at runtime as you need to adjust for temperature and ageing of the crystal (datasheet, page 14: "For those systems that have temperature detection precision of clock function may be increased by correcting clock error according to temperature fluctuations.") > > > > On 31/10/2021 00:50:53+0200, Pavel Modilaynen wrote: > > > From: Pavel Modilaynen <pavelmn@axis.com> > > > > > > Add support for oscillation adjustment register RS5C372_REG_TRIM > > > setting that is needed to accommodate for effective crystal > > > capacitance. > > > > > > Use optional property ricoh,trim that should contain > > > raw value to setup this register. According to > > > datasheets for RS5C372, R2025S/D, RV5C38[67] and R222[13] > > > the value will be converted to a number of ticks that > > > is to be subtracted or added when the second digits read > > > 00, 20 or 40 seconds. > > > > > > Signed-off-by: Pavel Modilaynen <pavelmn@axis.com> > > > --- > > > drivers/rtc/rtc-rs5c372.c | 18 +++++++++++++++++- > > > 1 file changed, 17 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/rtc/rtc-rs5c372.c b/drivers/rtc/rtc-rs5c372.c > > > index 80980414890c..3a2db0326669 100644 > > > --- a/drivers/rtc/rtc-rs5c372.c > > > +++ b/drivers/rtc/rtc-rs5c372.c > > > @@ -13,6 +13,7 @@ > > > #include <linux/slab.h> > > > #include <linux/module.h> > > > #include <linux/of_device.h> > > > +#include <linux/of.h> > > > /* > > > * Ricoh has a family of I2C based RTCs, which differ only slightly from > > > @@ -560,6 +561,8 @@ static int rs5c_oscillator_setup(struct rs5c372 *rs5c372) > > > { > > > unsigned char buf[2]; > > > int addr, i, ret = 0; > > > + struct i2c_client *client = rs5c372->client; > > > + u8 trim = 0; > > > addr = RS5C_ADDR(RS5C_REG_CTRL1); > > > buf[0] = rs5c372->regs[RS5C_REG_CTRL1]; > > > @@ -599,9 +602,22 @@ static int rs5c_oscillator_setup(struct rs5c372 *rs5c372) > > > break; > > > } > > > + /* optional setup of xtal trimming */ > > > + if (!of_property_read_u8(client->dev.of_node, "ricoh,trim", &trim)) { > > > + if (rs5c372->type != rtc_r2221tl && (trim & ~RS5C372_TRIM_MASK)) { > > > + dev_warn(&client->dev, "Erroneous setting for ricoh,trim in devicetree\n"); > > > + } else { > > > + int addr = RS5C_ADDR(RS5C372_REG_TRIM); > > > + int ret = i2c_smbus_write_byte_data(client, addr, trim); > > > + > > > + if (unlikely(ret < 0)) > > > + return ret; > > > + } > > > + } > > > + > > > for (i = 0; i < sizeof(buf); i++) { > > > addr = RS5C_ADDR(RS5C_REG_CTRL1 + i); > > > - ret = i2c_smbus_write_byte_data(rs5c372->client, addr, buf[i]); > > > + ret = i2c_smbus_write_byte_data(client, addr, buf[i]); > > > if (unlikely(ret < 0)) > > > return ret; > > > } > > > -- > > > 2.20.1 > > > > > > > -- > > Alexandre Belloni, co-owner and COO, Bootlin > > Embedded Linux and Kernel engineering > > https://bootlin.com <https://bootlin.com> -- Alexandre Belloni, co-owner and COO, Bootlin Embedded Linux and Kernel engineering https://bootlin.com ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] rtc: rs5c372: Add support for trim configuration 2021-11-01 18:23 ` Alexandre Belloni @ 2021-11-01 23:14 ` Pavel Modilaynen 2021-11-05 15:26 ` Alexandre Belloni 0 siblings, 1 reply; 8+ messages in thread From: Pavel Modilaynen @ 2021-11-01 23:14 UTC (permalink / raw) To: Alexandre Belloni, Pavel Modilaynen Cc: a.zummo@towertech.it, robh+dt@kernel.org, linux-rtc@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, kernel Hi Alexandre, On 11/1/21 7:23 PM, Alexandre Belloni wrote: > On 31/10/2021 11:29:12+0100, Pavel Modilaynen wrote: >> On 10/31/21 12:57 AM, Alexandre Belloni wrote: >> > Hello, >> > >> > Please use the proper RTC interface by implementing .set_offset and >> > .read_offset. >> >> I am not sure about .set/read_offset. It looks as runtime adjustment >> interface, >> however this Xtal trimming parameter is based on schematics and Xtal >> capacitance (datasheet parameter). >> It is found by calibration procedure based on RTC clock output (the >> procedure and calculation of trimming parameter is described in datasheets). >> So, I would like to say that this parameter is functionally close to >> "quartz-load-femtofarads" for rtc-pcf8523/pcf85063. >> > > quartz-load-femtofarads is for analog trimming which this RTC doesn't > have, both CD and CG are set to 10pF. .set/read_offset are for digital > trimming which is what you are configuring here. You definitively want > to be able to do that at runtime as you need to adjust for temperature > and ageing of the crystal (datasheet, page 14: "For those systems that > have temperature detection precision of clock function may be increased > by correcting clock error according to temperature fluctuations.") > Thank you for reply. I am not denying the need in runtime adjustment related to temperature, aging and precision, which you are referring by excerpt from p.14. I would like to make a point that Xtal trimming is for coarse grained adjustment (pages 36-39), primarily related to Xtal capacitance CL (not CD/CG). Our goal is to keep a reasonable drift of ,say, <1 second per day and for Xtal that we use with 12.5pF RTC manufacturer recommends using 0x23 value for adjustment. In this case we act according to (A) course from page 38: "Adjustment of clock is not made for IC (no adjustment) and any CL value may be used for the crystal oscillator. Precision fluctuations of a crystal oscillator may be selected as long as clock precision allows. Obtain the central frequency as described in section 2.2 using several crystal oscillator and ICs, determine an adjustment value as described in “2.4 Time Trimming Circuit” which shall be set to the RS5C372A/B." >> > >> > On 31/10/2021 00:50:53+0200, Pavel Modilaynen wrote: >> > > From: Pavel Modilaynen <pavelmn@axis.com> >> > > >> > > Add support for oscillation adjustment register RS5C372_REG_TRIM >> > > setting that is needed to accommodate for effective crystal >> > > capacitance. >> > > >> > > Use optional property ricoh,trim that should contain >> > > raw value to setup this register. According to >> > > datasheets for RS5C372, R2025S/D, RV5C38[67] and R222[13] >> > > the value will be converted to a number of ticks that >> > > is to be subtracted or added when the second digits read >> > > 00, 20 or 40 seconds. >> > > >> > > Signed-off-by: Pavel Modilaynen <pavelmn@axis.com> >> > > --- >> > > drivers/rtc/rtc-rs5c372.c | 18 +++++++++++++++++- >> > > 1 file changed, 17 insertions(+), 1 deletion(-) >> > > >> > > diff --git a/drivers/rtc/rtc-rs5c372.c b/drivers/rtc/rtc-rs5c372.c >> > > index 80980414890c..3a2db0326669 100644 >> > > --- a/drivers/rtc/rtc-rs5c372.c >> > > +++ b/drivers/rtc/rtc-rs5c372.c >> > > @@ -13,6 +13,7 @@ >> > > #include <linux/slab.h> >> > > #include <linux/module.h> >> > > #include <linux/of_device.h> >> > > +#include <linux/of.h> >> > > /* >> > > * Ricoh has a family of I2C based RTCs, which differ only slightly from >> > > @@ -560,6 +561,8 @@ static int rs5c_oscillator_setup(struct rs5c372 *rs5c372) >> > > { >> > > unsigned char buf[2]; >> > > int addr, i, ret = 0; >> > > + struct i2c_client *client = rs5c372->client; >> > > + u8 trim = 0; >> > > addr = RS5C_ADDR(RS5C_REG_CTRL1); >> > > buf[0] = rs5c372->regs[RS5C_REG_CTRL1]; >> > > @@ -599,9 +602,22 @@ static int rs5c_oscillator_setup(struct rs5c372 *rs5c372) >> > > break; >> > > } >> > > + /* optional setup of xtal trimming */ >> > > + if (!of_property_read_u8(client->dev.of_node, "ricoh,trim", &trim)) { >> > > + if (rs5c372->type != rtc_r2221tl && (trim & ~RS5C372_TRIM_MASK)) { >> > > + dev_warn(&client->dev, "Erroneous setting for ricoh,trim in devicetree\n"); >> > > + } else { >> > > + int addr = RS5C_ADDR(RS5C372_REG_TRIM); >> > > + int ret = i2c_smbus_write_byte_data(client, addr, trim); >> > > + >> > > + if (unlikely(ret < 0)) >> > > + return ret; >> > > + } >> > > + } >> > > + >> > > for (i = 0; i < sizeof(buf); i++) { >> > > addr = RS5C_ADDR(RS5C_REG_CTRL1 + i); >> > > - ret = i2c_smbus_write_byte_data(rs5c372->client, addr, buf[i]); >> > > + ret = i2c_smbus_write_byte_data(client, addr, buf[i]); >> > > if (unlikely(ret < 0)) >> > > return ret; >> > > } >> > > -- >> > > 2.20.1 >> > > >> > >> > -- >> > Alexandre Belloni, co-owner and COO, Bootlin >> > Embedded Linux and Kernel engineering >> > https://bootlin.com <https://bootlin.com> <https://bootlin.com > <https://bootlin.com>> > > -- > Alexandre Belloni, co-owner and COO, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com <https://bootlin.com> ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] rtc: rs5c372: Add support for trim configuration 2021-11-01 23:14 ` Pavel Modilaynen @ 2021-11-05 15:26 ` Alexandre Belloni 0 siblings, 0 replies; 8+ messages in thread From: Alexandre Belloni @ 2021-11-05 15:26 UTC (permalink / raw) To: Pavel Modilaynen Cc: Pavel Modilaynen, a.zummo@towertech.it, robh+dt@kernel.org, linux-rtc@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, kernel On 02/11/2021 00:14:39+0100, Pavel Modilaynen wrote: > Hi Alexandre, > > On 11/1/21 7:23 PM, Alexandre Belloni wrote: > > On 31/10/2021 11:29:12+0100, Pavel Modilaynen wrote: > > > On 10/31/21 12:57 AM, Alexandre Belloni wrote: > > > > Hello, > > > > > Please use the proper RTC interface by implementing .set_offset > > > and > > > > .read_offset. > > > > > > I am not sure about .set/read_offset. It looks as runtime adjustment > > > interface, > > > however this Xtal trimming parameter is based on schematics and Xtal > > > capacitance (datasheet parameter). > > > It is found by calibration procedure based on RTC clock output (the > > > procedure and calculation of trimming parameter is described in datasheets). > > > So, I would like to say that this parameter is functionally close to > > > "quartz-load-femtofarads" for rtc-pcf8523/pcf85063. > > > > > > > quartz-load-femtofarads is for analog trimming which this RTC doesn't > > have, both CD and CG are set to 10pF. .set/read_offset are for digital > > trimming which is what you are configuring here. You definitively want > > to be able to do that at runtime as you need to adjust for temperature > > and ageing of the crystal (datasheet, page 14: "For those systems that > > have temperature detection precision of clock function may be increased > > by correcting clock error according to temperature fluctuations.") > > > > Thank you for reply. > > I am not denying the need in runtime adjustment related to > temperature, aging and precision, which you are referring by excerpt from > p.14. > > I would like to make a point that Xtal trimming is for coarse grained > adjustment (pages 36-39), primarily related to Xtal capacitance CL (not > CD/CG). Our goal is to keep a reasonable drift of ,say, <1 second per day > and for Xtal that we use with 12.5pF RTC manufacturer recommends using 0x23 > value for adjustment. In this case we act according to (A) course from page > 38: > > "Adjustment of clock is not made for IC (no adjustment) and any CL value may > be used for the crystal oscillator. Precision fluctuations of a crystal > oscillator may be selected as long as clock precision allows. Obtain the > central frequency as described in section 2.2 using several crystal > oscillator and ICs, determine an adjustment value as > described in “2.4 Time Trimming Circuit” which shall be set to the > RS5C372A/B." > This doesn't prevent you from using the .set/read_offset interface. Simply have a startup script that forcibly set the hardcoded value if this is what you want. However, I'm pretty sure your products have NTP which allows to precisely set a calculated value instead of an hardcoded one. > > > > > > On 31/10/2021 00:50:53+0200, Pavel Modilaynen wrote: > > > > > From: Pavel Modilaynen <pavelmn@axis.com> > > > > > > > Add support for oscillation adjustment register > > > RS5C372_REG_TRIM > > > > > setting that is needed to accommodate for effective crystal > > > > > capacitance. > > > > > > > Use optional property ricoh,trim that should contain > > > > > raw value to setup this register. According to > > > > > datasheets for RS5C372, R2025S/D, RV5C38[67] and R222[13] > > > > > the value will be converted to a number of ticks that > > > > > is to be subtracted or added when the second digits read > > > > > 00, 20 or 40 seconds. > > > > > > > Signed-off-by: Pavel Modilaynen <pavelmn@axis.com> > > > > > --- > > > > > drivers/rtc/rtc-rs5c372.c | 18 +++++++++++++++++- > > > > > 1 file changed, 17 insertions(+), 1 deletion(-) > > > > > > > diff --git a/drivers/rtc/rtc-rs5c372.c > > > b/drivers/rtc/rtc-rs5c372.c > > > > > index 80980414890c..3a2db0326669 100644 > > > > > --- a/drivers/rtc/rtc-rs5c372.c > > > > > +++ b/drivers/rtc/rtc-rs5c372.c > > > > > @@ -13,6 +13,7 @@ > > > > > #include <linux/slab.h> > > > > > #include <linux/module.h> > > > > > #include <linux/of_device.h> > > > > > +#include <linux/of.h> > > > > > /* > > > > > * Ricoh has a family of I2C based RTCs, which differ only slightly from > > > > > @@ -560,6 +561,8 @@ static int rs5c_oscillator_setup(struct rs5c372 *rs5c372) > > > > > { > > > > > unsigned char buf[2]; > > > > > int addr, i, ret = 0; > > > > > + struct i2c_client *client = rs5c372->client; > > > > > + u8 trim = 0; > > > > > addr = RS5C_ADDR(RS5C_REG_CTRL1); > > > > > buf[0] = rs5c372->regs[RS5C_REG_CTRL1]; > > > > > @@ -599,9 +602,22 @@ static int rs5c_oscillator_setup(struct rs5c372 *rs5c372) > > > > > break; > > > > > } > > > > > + /* optional setup of xtal trimming */ > > > > > + if (!of_property_read_u8(client->dev.of_node, "ricoh,trim", &trim)) { > > > > > + if (rs5c372->type != rtc_r2221tl && (trim & ~RS5C372_TRIM_MASK)) { > > > > > + dev_warn(&client->dev, "Erroneous setting for ricoh,trim in devicetree\n"); > > > > > + } else { > > > > > + int addr = RS5C_ADDR(RS5C372_REG_TRIM); > > > > > + int ret = i2c_smbus_write_byte_data(client, addr, trim); > > > > > + > > > > > + if (unlikely(ret < 0)) > > > > > + return ret; > > > > > + } > > > > > + } > > > > > + > > > > > for (i = 0; i < sizeof(buf); i++) { > > > > > addr = RS5C_ADDR(RS5C_REG_CTRL1 + i); > > > > > - ret = i2c_smbus_write_byte_data(rs5c372->client, addr, buf[i]); > > > > > + ret = i2c_smbus_write_byte_data(client, addr, buf[i]); > > > > > if (unlikely(ret < 0)) > > > > > return ret; > > > > > } > > > > > -- > > 2.20.1 > > > > > > > -- > Alexandre Belloni, co-owner and COO, Bootlin > > > > Embedded Linux and Kernel engineering > > > > https://bootlin.com <https://bootlin.com> <https://bootlin.com > > <https://bootlin.com>> > > > > -- > > Alexandre Belloni, co-owner and COO, Bootlin > > Embedded Linux and Kernel engineering > > https://bootlin.com <https://bootlin.com> -- Alexandre Belloni, co-owner and COO, Bootlin Embedded Linux and Kernel engineering https://bootlin.com ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/2] dt-bindings: rtc: Add bindings for Ricoh rs5c372 2021-10-30 22:50 [PATCH 0/2] *** Ricoh RS5C372: add support for Xtal trimming configuration *** Pavel Modilaynen 2021-10-30 22:50 ` [PATCH 1/2] rtc: rs5c372: Add support for trim configuration Pavel Modilaynen @ 2021-10-30 22:50 ` Pavel Modilaynen 1 sibling, 0 replies; 8+ messages in thread From: Pavel Modilaynen @ 2021-10-30 22:50 UTC (permalink / raw) To: a.zummo, alexandre.belloni, robh+dt Cc: linux-rtc, devicetree, linux-kernel, lkml, kernel, Pavel Modilaynen From: Pavel Modilaynen <pavelmn@axis.com> Create new DT bindings yaml file for Ricoh rs5c372 driver since adding property support naturally deserves it. Place a description of this property: ricoh,trim. Signed-off-by: Pavel Modilaynen <pavelmn@axis.com> --- .../bindings/rtc/ricoh,rs5c372.yaml | 58 +++++++++++++++++++ .../devicetree/bindings/rtc/trivial-rtc.yaml | 12 ---- 2 files changed, 58 insertions(+), 12 deletions(-) create mode 100644 Documentation/devicetree/bindings/rtc/ricoh,rs5c372.yaml diff --git a/Documentation/devicetree/bindings/rtc/ricoh,rs5c372.yaml b/Documentation/devicetree/bindings/rtc/ricoh,rs5c372.yaml new file mode 100644 index 000000000000..6cb6a97db5d1 --- /dev/null +++ b/Documentation/devicetree/bindings/rtc/ricoh,rs5c372.yaml @@ -0,0 +1,58 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/rtc/ricoh,rs5c372.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Ricoh RS5C372, R2025S/D, RV5C38[67] and R222[13] Real Time Clock + +maintainers: + - Alexandre Belloni <alexandre.belloni@bootlin.com> + +allOf: + - $ref: rtc.yaml# + +properties: + compatible: + enum: + - ricoh,r2025sd + - ricoh,r2221tl + - ricoh,rs5c372a + - ricoh,rs5c372b + - ricoh,rv5c386 + - ricoh,rv5c387a + + reg: + maxItems: 1 + + ricoh,trim: + $ref: /schemas/types.yaml#/definitions/uint8 + description: | + Raw value of Oscillation Adjustment Register (0x7) which changes + time counts of 1 second. Normally, the second counter is incremented + once per 32768 clock pulses. The value causes to increment + decrement time counts when second digits read 00, 20, 40 seconds + (R222[13] can operate on 00 seconds if MSB set). + See datasheets for details. + + start-year: true + +required: + - compatible + - reg + +additionalProperties: false + +examples: + - | + i2c { + #address-cells = <1>; + #size-cells = <0>; + + rtc@32 { + compatible = "ricoh,r2221tl"; + reg = <0x32>; + ricoh,trim = /bits/ 8 <0x23>; + }; + }; +... diff --git a/Documentation/devicetree/bindings/rtc/trivial-rtc.yaml b/Documentation/devicetree/bindings/rtc/trivial-rtc.yaml index 13925bb78ec7..aeabede89654 100644 --- a/Documentation/devicetree/bindings/rtc/trivial-rtc.yaml +++ b/Documentation/devicetree/bindings/rtc/trivial-rtc.yaml @@ -55,18 +55,6 @@ properties: - nxp,pcf2129 # Real-time Clock Module - pericom,pt7c4338 - # I2C bus SERIAL INTERFACE REAL-TIME CLOCK IC - - ricoh,r2025sd - # I2C bus SERIAL INTERFACE REAL-TIME CLOCK IC - - ricoh,r2221tl - # I2C bus SERIAL INTERFACE REAL-TIME CLOCK IC - - ricoh,rs5c372a - # I2C bus SERIAL INTERFACE REAL-TIME CLOCK IC - - ricoh,rs5c372b - # I2C bus SERIAL INTERFACE REAL-TIME CLOCK IC - - ricoh,rv5c386 - # I2C bus SERIAL INTERFACE REAL-TIME CLOCK IC - - ricoh,rv5c387a # 2-wire CMOS real-time clock - sii,s35390a # I2C bus SERIAL INTERFACE REAL-TIME CLOCK IC -- 2.20.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-11-05 15:26 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2021-10-30 22:50 [PATCH 0/2] *** Ricoh RS5C372: add support for Xtal trimming configuration *** Pavel Modilaynen 2021-10-30 22:50 ` [PATCH 1/2] rtc: rs5c372: Add support for trim configuration Pavel Modilaynen 2021-10-30 22:57 ` Alexandre Belloni 2021-10-31 10:29 ` Pavel Modilaynen 2021-11-01 18:23 ` Alexandre Belloni 2021-11-01 23:14 ` Pavel Modilaynen 2021-11-05 15:26 ` Alexandre Belloni 2021-10-30 22:50 ` [PATCH 2/2] dt-bindings: rtc: Add bindings for Ricoh rs5c372 Pavel Modilaynen
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).