Linux-RTC Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] rtc: abx80x: Add support for autocalibration filter capacitor
@ 2020-05-30 12:32 Kevin P. Fleming
  2020-05-30 12:50 ` Kevin P. Fleming
  2020-06-09 22:14 ` Rob Herring
  0 siblings, 2 replies; 7+ messages in thread
From: Kevin P. Fleming @ 2020-05-30 12:32 UTC (permalink / raw)
  To: linux-rtc, devicetree
  Cc: Kevin P. Fleming, Alessandro Zummo, Alexandre Belloni, Rob Herring

All of the parts supported by this driver can make use of a
small capacitor to improve the accuracy of the autocalibration
process for their RC oscillators. If a capacitor is connected,
a configuration register must be set to enable its use, so a
new Device Tree property has been added for that purpose.

Signed-off-by: Kevin P. Fleming <kevin+linux@km6g.us>
Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: Rob Herring <robh+dt@kernel.org>
To: linux-rtc@vger.kernel.org
To: devicetree@vger.kernel.org
---
 .../bindings/rtc/abracon,abx80x.txt           |  6 ++++
 drivers/rtc/rtc-abx80x.c                      | 34 +++++++++++++++++++
 2 files changed, 40 insertions(+)

diff --git a/Documentation/devicetree/bindings/rtc/abracon,abx80x.txt b/Documentation/devicetree/bindings/rtc/abracon,abx80x.txt
index 2405e35a1bc0..ad5d59ed6f24 100644
--- a/Documentation/devicetree/bindings/rtc/abracon,abx80x.txt
+++ b/Documentation/devicetree/bindings/rtc/abracon,abx80x.txt
@@ -29,3 +29,9 @@ and valid to enable charging:
  - "abracon,tc-diode": should be "standard" (0.6V) or "schottky" (0.3V)
  - "abracon,tc-resistor": should be <0>, <3>, <6> or <11>. 0 disables the output
                           resistor, the other values are in kOhm.
+
+All of the devices can have a 47pf capacitor attached to increase the
+autocalibration accuracy of their RC oscillators. To enable usage of the
+capacitor the following property has to be defined:
+
+ - "abracon,autocal-filter"
diff --git a/drivers/rtc/rtc-abx80x.c b/drivers/rtc/rtc-abx80x.c
index 3521d8e8dc38..be5a814e8c0b 100644
--- a/drivers/rtc/rtc-abx80x.c
+++ b/drivers/rtc/rtc-abx80x.c
@@ -76,6 +76,9 @@
 #define ABX8XX_CFG_KEY_OSC	0xa1
 #define ABX8XX_CFG_KEY_MISC	0x9d
 
+#define ABX8XX_REG_AFCTRL	0x26
+#define ABX8XX_AUTOCAL_FILTER_ENABLE	0xa0
+
 #define ABX8XX_REG_ID0		0x28
 
 #define ABX8XX_REG_OUT_CTRL	0x30
@@ -130,6 +133,31 @@ static int abx80x_is_rc_mode(struct i2c_client *client)
 	return (flags & ABX8XX_OSS_OMODE) ? 1 : 0;
 }
 
+static int abx80x_enable_autocal_filter(struct i2c_client *client)
+{
+	int err;
+
+	/*
+	 * Write the configuration key register to enable access to the AFCTRL
+	 * register
+	 */
+	err = i2c_smbus_write_byte_data(client, ABX8XX_REG_CFG_KEY,
+					ABX8XX_CFG_KEY_MISC);
+	if (err < 0) {
+		dev_err(&client->dev, "Unable to write configuration key\n");
+		return -EIO;
+	}
+
+	err = i2c_smbus_write_byte_data(client, ABX8XX_REG_AFCTRL,
+					ABX8XX_AUTOCAL_FILTER_ENABLE);
+	if (err < 0) {
+		dev_err(&client->dev, "Unable to write autocal filter register\n");
+		return -EIO;
+	}
+
+	return 0;
+}
+
 static int abx80x_enable_trickle_charger(struct i2c_client *client,
 					 u8 trickle_cfg)
 {
@@ -825,6 +853,12 @@ static int abx80x_probe(struct i2c_client *client,
 			return err;
 	}
 
+	if (of_property_read_bool(np, "abracon,autocal_filter")) {
+		err = abx80x_enable_autocal_filter(client);
+		if (err)
+			return err;
+	}
+
 	if (client->irq > 0) {
 		dev_info(&client->dev, "IRQ %d supplied\n", client->irq);
 		err = devm_request_threaded_irq(&client->dev, client->irq, NULL,
-- 
2.26.2


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

* Re: [PATCH] rtc: abx80x: Add support for autocalibration filter capacitor
  2020-05-30 12:32 [PATCH] rtc: abx80x: Add support for autocalibration filter capacitor Kevin P. Fleming
@ 2020-05-30 12:50 ` Kevin P. Fleming
  2020-06-09 22:14 ` Rob Herring
  1 sibling, 0 replies; 7+ messages in thread
From: Kevin P. Fleming @ 2020-05-30 12:50 UTC (permalink / raw)
  To: Kevin P. Fleming
  Cc: linux-rtc, devicetree, Alessandro Zummo, Alexandre Belloni, Rob Herring

Sorry, this was sent prematurely and should have been split into two
patches. It has been resent properly.

On Sat, May 30, 2020 at 8:32 AM Kevin P. Fleming <kevin+linux@km6g.us> wrote:
>
> All of the parts supported by this driver can make use of a
> small capacitor to improve the accuracy of the autocalibration
> process for their RC oscillators. If a capacitor is connected,
> a configuration register must be set to enable its use, so a
> new Device Tree property has been added for that purpose.
>
> Signed-off-by: Kevin P. Fleming <kevin+linux@km6g.us>
> Cc: Alessandro Zummo <a.zummo@towertech.it>
> Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> To: linux-rtc@vger.kernel.org
> To: devicetree@vger.kernel.org
> ---
>  .../bindings/rtc/abracon,abx80x.txt           |  6 ++++
>  drivers/rtc/rtc-abx80x.c                      | 34 +++++++++++++++++++
>  2 files changed, 40 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/rtc/abracon,abx80x.txt b/Documentation/devicetree/bindings/rtc/abracon,abx80x.txt
> index 2405e35a1bc0..ad5d59ed6f24 100644
> --- a/Documentation/devicetree/bindings/rtc/abracon,abx80x.txt
> +++ b/Documentation/devicetree/bindings/rtc/abracon,abx80x.txt
> @@ -29,3 +29,9 @@ and valid to enable charging:
>   - "abracon,tc-diode": should be "standard" (0.6V) or "schottky" (0.3V)
>   - "abracon,tc-resistor": should be <0>, <3>, <6> or <11>. 0 disables the output
>                            resistor, the other values are in kOhm.
> +
> +All of the devices can have a 47pf capacitor attached to increase the
> +autocalibration accuracy of their RC oscillators. To enable usage of the
> +capacitor the following property has to be defined:
> +
> + - "abracon,autocal-filter"
> diff --git a/drivers/rtc/rtc-abx80x.c b/drivers/rtc/rtc-abx80x.c
> index 3521d8e8dc38..be5a814e8c0b 100644
> --- a/drivers/rtc/rtc-abx80x.c
> +++ b/drivers/rtc/rtc-abx80x.c
> @@ -76,6 +76,9 @@
>  #define ABX8XX_CFG_KEY_OSC     0xa1
>  #define ABX8XX_CFG_KEY_MISC    0x9d
>
> +#define ABX8XX_REG_AFCTRL      0x26
> +#define ABX8XX_AUTOCAL_FILTER_ENABLE   0xa0
> +
>  #define ABX8XX_REG_ID0         0x28
>
>  #define ABX8XX_REG_OUT_CTRL    0x30
> @@ -130,6 +133,31 @@ static int abx80x_is_rc_mode(struct i2c_client *client)
>         return (flags & ABX8XX_OSS_OMODE) ? 1 : 0;
>  }
>
> +static int abx80x_enable_autocal_filter(struct i2c_client *client)
> +{
> +       int err;
> +
> +       /*
> +        * Write the configuration key register to enable access to the AFCTRL
> +        * register
> +        */
> +       err = i2c_smbus_write_byte_data(client, ABX8XX_REG_CFG_KEY,
> +                                       ABX8XX_CFG_KEY_MISC);
> +       if (err < 0) {
> +               dev_err(&client->dev, "Unable to write configuration key\n");
> +               return -EIO;
> +       }
> +
> +       err = i2c_smbus_write_byte_data(client, ABX8XX_REG_AFCTRL,
> +                                       ABX8XX_AUTOCAL_FILTER_ENABLE);
> +       if (err < 0) {
> +               dev_err(&client->dev, "Unable to write autocal filter register\n");
> +               return -EIO;
> +       }
> +
> +       return 0;
> +}
> +
>  static int abx80x_enable_trickle_charger(struct i2c_client *client,
>                                          u8 trickle_cfg)
>  {
> @@ -825,6 +853,12 @@ static int abx80x_probe(struct i2c_client *client,
>                         return err;
>         }
>
> +       if (of_property_read_bool(np, "abracon,autocal_filter")) {
> +               err = abx80x_enable_autocal_filter(client);
> +               if (err)
> +                       return err;
> +       }
> +
>         if (client->irq > 0) {
>                 dev_info(&client->dev, "IRQ %d supplied\n", client->irq);
>                 err = devm_request_threaded_irq(&client->dev, client->irq, NULL,
> --
> 2.26.2
>

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

* Re: [PATCH] rtc: abx80x: Add support for autocalibration filter capacitor
  2020-05-30 12:32 [PATCH] rtc: abx80x: Add support for autocalibration filter capacitor Kevin P. Fleming
  2020-05-30 12:50 ` Kevin P. Fleming
@ 2020-06-09 22:14 ` Rob Herring
  2020-06-09 22:23   ` Kevin P. Fleming
  1 sibling, 1 reply; 7+ messages in thread
From: Rob Herring @ 2020-06-09 22:14 UTC (permalink / raw)
  To: Kevin P. Fleming
  Cc: linux-rtc, devicetree, Alessandro Zummo, Alexandre Belloni

On Sat, May 30, 2020 at 08:32:22AM -0400, Kevin P. Fleming wrote:
> All of the parts supported by this driver can make use of a
> small capacitor to improve the accuracy of the autocalibration
> process for their RC oscillators. If a capacitor is connected,
> a configuration register must be set to enable its use, so a
> new Device Tree property has been added for that purpose.
> 
> Signed-off-by: Kevin P. Fleming <kevin+linux@km6g.us>
> Cc: Alessandro Zummo <a.zummo@towertech.it>
> Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> To: linux-rtc@vger.kernel.org
> To: devicetree@vger.kernel.org
> ---
>  .../bindings/rtc/abracon,abx80x.txt           |  6 ++++
>  drivers/rtc/rtc-abx80x.c                      | 34 +++++++++++++++++++
>  2 files changed, 40 insertions(+)

Binding should be a separate patch?

> 
> diff --git a/Documentation/devicetree/bindings/rtc/abracon,abx80x.txt b/Documentation/devicetree/bindings/rtc/abracon,abx80x.txt
> index 2405e35a1bc0..ad5d59ed6f24 100644
> --- a/Documentation/devicetree/bindings/rtc/abracon,abx80x.txt
> +++ b/Documentation/devicetree/bindings/rtc/abracon,abx80x.txt
> @@ -29,3 +29,9 @@ and valid to enable charging:
>   - "abracon,tc-diode": should be "standard" (0.6V) or "schottky" (0.3V)
>   - "abracon,tc-resistor": should be <0>, <3>, <6> or <11>. 0 disables the output
>                            resistor, the other values are in kOhm.
> +
> +All of the devices can have a 47pf capacitor attached to increase the
> +autocalibration accuracy of their RC oscillators. To enable usage of the
> +capacitor the following property has to be defined:
> +
> + - "abracon,autocal-filter"

Can't the standard 'quartz-load-femtofarads' property be used here? You 
might not need to know the value, but presence of the property can 
enable the feature.

Rob

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

* Re: [PATCH] rtc: abx80x: Add support for autocalibration filter capacitor
  2020-06-09 22:14 ` Rob Herring
@ 2020-06-09 22:23   ` Kevin P. Fleming
  2020-06-10 15:16     ` Alexandre Belloni
  0 siblings, 1 reply; 7+ messages in thread
From: Kevin P. Fleming @ 2020-06-09 22:23 UTC (permalink / raw)
  To: Rob Herring
  Cc: Kevin P. Fleming, linux-rtc, devicetree, Alessandro Zummo,
	Alexandre Belloni

On Tue, Jun 9, 2020 at 6:14 PM Rob Herring <robh@kernel.org> wrote:
> > ---
> >  .../bindings/rtc/abracon,abx80x.txt           |  6 ++++
> >  drivers/rtc/rtc-abx80x.c                      | 34 +++++++++++++++++++
> >  2 files changed, 40 insertions(+)
>
> Binding should be a separate patch?

Indeed, it was re-sent with the patches separated.

> > +All of the devices can have a 47pf capacitor attached to increase the
> > +autocalibration accuracy of their RC oscillators. To enable usage of the
> > +capacitor the following property has to be defined:
> > +
> > + - "abracon,autocal-filter"
>
> Can't the standard 'quartz-load-femtofarads' property be used here? You
> might not need to know the value, but presence of the property can
> enable the feature.

On these devices the capacitor is connected to the RC oscillator, not
the crystal oscillator, so this property is controlling a different
function. I'm certainly open to suggestions for different names for
the property if that is desired.

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

* Re: [PATCH] rtc: abx80x: Add support for autocalibration filter capacitor
  2020-06-09 22:23   ` Kevin P. Fleming
@ 2020-06-10 15:16     ` Alexandre Belloni
  2020-06-12 11:48       ` Kevin P. Fleming
  0 siblings, 1 reply; 7+ messages in thread
From: Alexandre Belloni @ 2020-06-10 15:16 UTC (permalink / raw)
  To: Kevin P. Fleming; +Cc: Rob Herring, linux-rtc, devicetree, Alessandro Zummo

Hi,

On 09/06/2020 18:23:48-0400, Kevin P. Fleming wrote:
> On Tue, Jun 9, 2020 at 6:14 PM Rob Herring <robh@kernel.org> wrote:
> > > ---
> > >  .../bindings/rtc/abracon,abx80x.txt           |  6 ++++
> > >  drivers/rtc/rtc-abx80x.c                      | 34 +++++++++++++++++++
> > >  2 files changed, 40 insertions(+)
> >
> > Binding should be a separate patch?
> 
> Indeed, it was re-sent with the patches separated.
> 
> > > +All of the devices can have a 47pf capacitor attached to increase the
> > > +autocalibration accuracy of their RC oscillators. To enable usage of the
> > > +capacitor the following property has to be defined:
> > > +
> > > + - "abracon,autocal-filter"
> >
> > Can't the standard 'quartz-load-femtofarads' property be used here? You
> > might not need to know the value, but presence of the property can
> > enable the feature.
> 
> On these devices the capacitor is connected to the RC oscillator, not
> the crystal oscillator, so this property is controlling a different
> function. I'm certainly open to suggestions for different names for
> the property if that is desired.

I agree that this is something different, quartz-load-femtofarads is
about asking the RTC to put the correct load depending on the crystal
populated on the board. Here, you want to indicate the presence or
absence of a filter capacitor.

When working with RTCs, there is one issue though: boolean properties
are not working well because there is no way to express the 3 different
conditions:
 1/ the capacitor is present, set the register
 2/ the capacitor is absent, clear the register
 3/ the device tree didn't have this property until not and the register
   may have been set or cleared using another mean, don't touch it.

As your patch is written, it only handles 1 and 3 which is probably the
safest option but then we will never have a way to clear it from the
driver. I'd say that this is not an issue but it is also something we
will never be able to change without breaking some setups.


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

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

* Re: [PATCH] rtc: abx80x: Add support for autocalibration filter capacitor
  2020-06-10 15:16     ` Alexandre Belloni
@ 2020-06-12 11:48       ` Kevin P. Fleming
  2020-06-12 14:23         ` Rob Herring
  0 siblings, 1 reply; 7+ messages in thread
From: Kevin P. Fleming @ 2020-06-12 11:48 UTC (permalink / raw)
  To: Alexandre Belloni
  Cc: Kevin P. Fleming, Rob Herring, linux-rtc, devicetree, Alessandro Zummo

On Wed, Jun 10, 2020 at 11:16 AM Alexandre Belloni
<alexandre.belloni@bootlin.com> wrote:
> When working with RTCs, there is one issue though: boolean properties
> are not working well because there is no way to express the 3 different
> conditions:
>  1/ the capacitor is present, set the register
>  2/ the capacitor is absent, clear the register
>  3/ the device tree didn't have this property until not and the register
>    may have been set or cleared using another mean, don't touch it.
>
> As your patch is written, it only handles 1 and 3 which is probably the
> safest option but then we will never have a way to clear it from the
> driver. I'd say that this is not an issue but it is also something we
> will never be able to change without breaking some setups.

I agree. I could implement this as an enumerated string option which
accepts 'yes' or 'no'. Those would cover cases 1 and 2, and the
absence of the property would be case 3. I looked through the bindings
that exist and didn't see any examples of properties configured this
way, but I think it would be understandable to users.

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

* Re: [PATCH] rtc: abx80x: Add support for autocalibration filter capacitor
  2020-06-12 11:48       ` Kevin P. Fleming
@ 2020-06-12 14:23         ` Rob Herring
  0 siblings, 0 replies; 7+ messages in thread
From: Rob Herring @ 2020-06-12 14:23 UTC (permalink / raw)
  To: Kevin P. Fleming
  Cc: Alexandre Belloni, open list:REAL TIME CLOCK (RTC) SUBSYSTEM,
	devicetree, Alessandro Zummo

On Fri, Jun 12, 2020 at 5:48 AM Kevin P. Fleming <kevin+linux@km6g.us> wrote:
>
> On Wed, Jun 10, 2020 at 11:16 AM Alexandre Belloni
> <alexandre.belloni@bootlin.com> wrote:
> > When working with RTCs, there is one issue though: boolean properties
> > are not working well because there is no way to express the 3 different
> > conditions:
> >  1/ the capacitor is present, set the register
> >  2/ the capacitor is absent, clear the register
> >  3/ the device tree didn't have this property until not and the register
> >    may have been set or cleared using another mean, don't touch it.
> >
> > As your patch is written, it only handles 1 and 3 which is probably the
> > safest option but then we will never have a way to clear it from the
> > driver. I'd say that this is not an issue but it is also something we
> > will never be able to change without breaking some setups.
>
> I agree. I could implement this as an enumerated string option which
> accepts 'yes' or 'no'. Those would cover cases 1 and 2, and the
> absence of the property would be case 3. I looked through the bindings
> that exist and didn't see any examples of properties configured this
> way, but I think it would be understandable to users.

It's a solved problem:

foo-feature = 0; //disable
foo-feature = 1; //enable
<no prop> // Leave feature configured as-is

Rob

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

end of thread, back to index

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-30 12:32 [PATCH] rtc: abx80x: Add support for autocalibration filter capacitor Kevin P. Fleming
2020-05-30 12:50 ` Kevin P. Fleming
2020-06-09 22:14 ` Rob Herring
2020-06-09 22:23   ` Kevin P. Fleming
2020-06-10 15:16     ` Alexandre Belloni
2020-06-12 11:48       ` Kevin P. Fleming
2020-06-12 14:23         ` Rob Herring

Linux-RTC Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-rtc/0 linux-rtc/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-rtc linux-rtc/ https://lore.kernel.org/linux-rtc \
		linux-rtc@vger.kernel.org
	public-inbox-index linux-rtc

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-rtc


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git