linux-rtc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] rtc: pcf2127: add default battery-related power-on values
@ 2023-08-02 19:11 Hugo Villeneuve
  2023-08-02 19:11 ` [PATCH 1/2] dt-bindings: rtc: add properties to set battery-related functions Hugo Villeneuve
  2023-08-02 19:11 ` [PATCH 2/2] rtc: pcf2127: add support for battery-related DT properties Hugo Villeneuve
  0 siblings, 2 replies; 14+ messages in thread
From: Hugo Villeneuve @ 2023-08-02 19:11 UTC (permalink / raw)
  To: a.zummo, alexandre.belloni, robh+dt, krzysztof.kozlowski+dt, conor+dt
  Cc: linux-rtc, devicetree, linux-kernel, hugo, bruno.thomsen,
	Hugo Villeneuve

From: Hugo Villeneuve <hvilleneuve@dimonoff.com>

Hello,
this patch series adds support for setting default battery-related
functions for RTC devices.

This evolved from previous discussions about PCF2127 and also when reviewing
PCF2131 driver:

Link: https://lore.kernel.org/linux-rtc/20190910143945.9364-1-bruno.thomsen@gmail.com/
Link: https://lore.kernel.org/linux-rtc/20191211163354.GC1463890@piout.net/
Link: https://lore.kernel.org/linux-rtc/20230123170731.6064430c50f5fb7b484d8734@hugovil.com/

I decided to add these two new DT properties as generic RTC properties, in the
hope that they can be reused by other RTC drivers if needed.

Patch 1 adds two new DT properties to set battery-related functions. These
properties are generic for all RTC devices.

Patch 2 adds support for these two new DT properties to the PCF2127 driver.
This is especially important for PCF2131 devices which have default PWRMNG
values which disable battery-related functions.

Thank you.

Hugo Villeneuve (2):
  dt-bindings: rtc: add properties to set battery-related functions
  rtc: pcf2127: add support for battery-related DT properties

 .../devicetree/bindings/rtc/rtc.yaml          | 19 ++++++
 drivers/rtc/rtc-pcf2127.c                     | 59 +++++++++++++++++++
 2 files changed, 78 insertions(+)


base-commit: 3c87b351809f220294aec3c0df7b078ff5c5b15b
-- 
2.30.2


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

* [PATCH 1/2] dt-bindings: rtc: add properties to set battery-related functions
  2023-08-02 19:11 [PATCH 0/2] rtc: pcf2127: add default battery-related power-on values Hugo Villeneuve
@ 2023-08-02 19:11 ` Hugo Villeneuve
  2023-08-08 11:21   ` Conor Dooley
  2023-08-02 19:11 ` [PATCH 2/2] rtc: pcf2127: add support for battery-related DT properties Hugo Villeneuve
  1 sibling, 1 reply; 14+ messages in thread
From: Hugo Villeneuve @ 2023-08-02 19:11 UTC (permalink / raw)
  To: a.zummo, alexandre.belloni, robh+dt, krzysztof.kozlowski+dt, conor+dt
  Cc: linux-rtc, devicetree, linux-kernel, hugo, bruno.thomsen,
	Hugo Villeneuve

From: Hugo Villeneuve <hvilleneuve@dimonoff.com>

These properties can be defined in the board's device tree to set the
default power-on values for battery-related functions.

Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
---
 .../devicetree/bindings/rtc/rtc.yaml          | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/Documentation/devicetree/bindings/rtc/rtc.yaml b/Documentation/devicetree/bindings/rtc/rtc.yaml
index efb66df82782..0217d229e3fa 100644
--- a/Documentation/devicetree/bindings/rtc/rtc.yaml
+++ b/Documentation/devicetree/bindings/rtc/rtc.yaml
@@ -26,6 +26,25 @@ properties:
       0: not chargeable
       1: chargeable
 
+  battery-low-detect:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [0, 1]
+    description: |
+      For RTC devices supporting a backup battery/supercap, this flag can be
+      used to configure the battery low detection reporting function:
+      0: disabled
+      1: enabled
+
+  battery-switch-over:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [0, 1]
+    description: |
+      For RTC devices supporting a backup battery/supercap, this flag can be
+      used to configure the battery switch over when the main voltage source is
+      turned off:
+      0: disabled
+      1: enabled
+
   quartz-load-femtofarads:
     description:
       The capacitive load of the quartz(x-tal), expressed in femto
-- 
2.30.2


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

* [PATCH 2/2] rtc: pcf2127: add support for battery-related DT properties
  2023-08-02 19:11 [PATCH 0/2] rtc: pcf2127: add default battery-related power-on values Hugo Villeneuve
  2023-08-02 19:11 ` [PATCH 1/2] dt-bindings: rtc: add properties to set battery-related functions Hugo Villeneuve
@ 2023-08-02 19:11 ` Hugo Villeneuve
  1 sibling, 0 replies; 14+ messages in thread
From: Hugo Villeneuve @ 2023-08-02 19:11 UTC (permalink / raw)
  To: a.zummo, alexandre.belloni, robh+dt, krzysztof.kozlowski+dt, conor+dt
  Cc: linux-rtc, devicetree, linux-kernel, hugo, bruno.thomsen,
	Hugo Villeneuve

From: Hugo Villeneuve <hvilleneuve@dimonoff.com>

Add support for "battery-switch-over-enable" DT property which can be
used to enable/disable the battery switch over function.

Also add support for "battery-low-detect-enable" DT property which can
be used to enable/disable the battery low detection function.

If any of these properties is not defined, then no alteration to the
PWRMNG field will occur.

These properties can be used to change the default power-on values
(PWRMNG) for battery-related functions. It is especially useful for
the PCF2131 where the default PWRMNG power-on values disable by
default the battery-related functions (contrary to the PCF2127).

Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
---
 drivers/rtc/rtc-pcf2127.c | 59 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 59 insertions(+)

diff --git a/drivers/rtc/rtc-pcf2127.c b/drivers/rtc/rtc-pcf2127.c
index 78141bb06ab0..3bb3ad95c67e 100644
--- a/drivers/rtc/rtc-pcf2127.c
+++ b/drivers/rtc/rtc-pcf2127.c
@@ -20,6 +20,7 @@
 #include <linux/i2c.h>
 #include <linux/spi/spi.h>
 #include <linux/bcd.h>
+#include <linux/bitfield.h>
 #include <linux/rtc.h>
 #include <linux/slab.h>
 #include <linux/module.h>
@@ -48,6 +49,7 @@
 #define PCF2127_BIT_CTRL3_BLF			BIT(2)
 #define PCF2127_BIT_CTRL3_BF			BIT(3)
 #define PCF2127_BIT_CTRL3_BTSE			BIT(4)
+#define PCF2127_CTRL3_PWRMNG_MASK		GENMASK(7, 5)
 /* Time and date registers */
 #define PCF2127_REG_TIME_BASE		0x03
 #define PCF2127_BIT_SC_OSF			BIT(7)
@@ -1080,6 +1082,57 @@ static int pcf2127_enable_ts(struct device *dev, int ts_id)
 	return ret;
 }
 
+/*
+ * By default, do not reconfigure or set default power management mode,
+ * unless explicitly requested via DT properties:
+ *   battery-switch-over
+ *   battery-low-detect
+ */
+static int pcf2127_configure_power_management(struct device *dev)
+{
+	struct pcf2127 *pcf2127 = dev_get_drvdata(dev);
+	int ret;
+	u8 pwrmng;
+	u32 bat_sw_over, bat_low_detect;
+
+	/*
+	 * The PWRMNG field is defined in a peculiar way for PCF21XX
+	 * devices: there is no individual bit defined for the
+	 * battery-switch-over or battery-low-detect functions.
+	 * Therefore, we require that both properties must be defined
+	 * to alter the PWRMNG field.
+	 */
+	if (device_property_read_u32(dev, "battery-switch-over", &bat_sw_over))
+		return 0;
+
+	if (device_property_read_u32(dev, "battery-low-detect",
+				     &bat_low_detect))
+		return 0;
+
+	if (!bat_sw_over) {
+		/*
+		 * If battery-switch-over is disabled, then the
+		 * battery-low-detect function is always disabled.
+		 */
+		pwrmng = BIT(2) | BIT(1) | BIT(0);
+	} else {
+		if (bat_low_detect)
+			pwrmng = 0;
+		else
+			pwrmng = BIT(0);
+	}
+
+	ret = regmap_update_bits(pcf2127->regmap, PCF2127_REG_CTRL3,
+				 PCF2127_CTRL3_PWRMNG_MASK,
+				 FIELD_PREP(PCF2127_CTRL3_PWRMNG_MASK, pwrmng));
+	if (ret < 0) {
+		dev_dbg(dev, "PWRMNG config failed\n");
+		return ret;
+	}
+
+	return 0;
+}
+
 /* Route all interrupt sources to INT A pin. */
 static int pcf2127_configure_interrupt_pins(struct device *dev)
 {
@@ -1163,6 +1216,12 @@ static int pcf2127_probe(struct device *dev, struct regmap *regmap,
 		pcf2127->irq_enabled = true;
 	}
 
+	ret = pcf2127_configure_power_management(dev);
+	if (ret) {
+		dev_err(dev, "failed to configure power management\n");
+		return ret;
+	}
+
 	if (alarm_irq > 0 || device_property_read_bool(dev, "wakeup-source")) {
 		device_init_wakeup(dev, true);
 		set_bit(RTC_FEATURE_ALARM, pcf2127->rtc->features);
-- 
2.30.2


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

* Re: [PATCH 1/2] dt-bindings: rtc: add properties to set battery-related functions
  2023-08-02 19:11 ` [PATCH 1/2] dt-bindings: rtc: add properties to set battery-related functions Hugo Villeneuve
@ 2023-08-08 11:21   ` Conor Dooley
  2023-08-08 12:25     ` Hugo Villeneuve
  0 siblings, 1 reply; 14+ messages in thread
From: Conor Dooley @ 2023-08-08 11:21 UTC (permalink / raw)
  To: Hugo Villeneuve
  Cc: a.zummo, alexandre.belloni, robh+dt, krzysztof.kozlowski+dt,
	conor+dt, linux-rtc, devicetree, linux-kernel, bruno.thomsen,
	Hugo Villeneuve

[-- Attachment #1: Type: text/plain, Size: 1918 bytes --]

Hey Hugo,

On Wed, Aug 02, 2023 at 03:11:52PM -0400, Hugo Villeneuve wrote:
> From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> 
> These properties can be defined in the board's device tree to set the
> default power-on values for battery-related functions.
> 
> Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> ---
>  .../devicetree/bindings/rtc/rtc.yaml          | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/rtc/rtc.yaml b/Documentation/devicetree/bindings/rtc/rtc.yaml
> index efb66df82782..0217d229e3fa 100644
> --- a/Documentation/devicetree/bindings/rtc/rtc.yaml
> +++ b/Documentation/devicetree/bindings/rtc/rtc.yaml
> @@ -26,6 +26,25 @@ properties:
>        0: not chargeable
>        1: chargeable
>  
> +  battery-low-detect:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [0, 1]
> +    description: |
> +      For RTC devices supporting a backup battery/supercap, this flag can be
> +      used to configure the battery low detection reporting function:
> +      0: disabled
> +      1: enabled
> +
> +  battery-switch-over:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [0, 1]
> +    description: |
> +      For RTC devices supporting a backup battery/supercap, this flag can be
> +      used to configure the battery switch over when the main voltage source is
> +      turned off:
> +      0: disabled
> +      1: enabled

Why are these implemented as enums? This seems to fall into the category
of using DT to determine software policy - why's it not sufficient to
have boolean properties that indicate hardware support and let the software
decide what to do with them?

Thanks,
Conor.

> +
>    quartz-load-femtofarads:
>      description:
>        The capacitive load of the quartz(x-tal), expressed in femto
> -- 
> 2.30.2
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

* Re: [PATCH 1/2] dt-bindings: rtc: add properties to set battery-related functions
  2023-08-08 11:21   ` Conor Dooley
@ 2023-08-08 12:25     ` Hugo Villeneuve
  2023-08-08 12:32       ` Alexandre Belloni
  0 siblings, 1 reply; 14+ messages in thread
From: Hugo Villeneuve @ 2023-08-08 12:25 UTC (permalink / raw)
  To: Conor Dooley
  Cc: a.zummo, alexandre.belloni, robh+dt, krzysztof.kozlowski+dt,
	conor+dt, linux-rtc, devicetree, linux-kernel, bruno.thomsen,
	Hugo Villeneuve

On Tue, 8 Aug 2023 12:21:24 +0100
Conor Dooley <conor@kernel.org> wrote:

> Hey Hugo,
> 
> On Wed, Aug 02, 2023 at 03:11:52PM -0400, Hugo Villeneuve wrote:
> > From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > 
> > These properties can be defined in the board's device tree to set the
> > default power-on values for battery-related functions.
> > 
> > Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > ---
> >  .../devicetree/bindings/rtc/rtc.yaml          | 19 +++++++++++++++++++
> >  1 file changed, 19 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/rtc/rtc.yaml b/Documentation/devicetree/bindings/rtc/rtc.yaml
> > index efb66df82782..0217d229e3fa 100644
> > --- a/Documentation/devicetree/bindings/rtc/rtc.yaml
> > +++ b/Documentation/devicetree/bindings/rtc/rtc.yaml
> > @@ -26,6 +26,25 @@ properties:
> >        0: not chargeable
> >        1: chargeable
> >  
> > +  battery-low-detect:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    enum: [0, 1]
> > +    description: |
> > +      For RTC devices supporting a backup battery/supercap, this flag can be
> > +      used to configure the battery low detection reporting function:
> > +      0: disabled
> > +      1: enabled
> > +
> > +  battery-switch-over:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    enum: [0, 1]
> > +    description: |
> > +      For RTC devices supporting a backup battery/supercap, this flag can be
> > +      used to configure the battery switch over when the main voltage source is
> > +      turned off:
> > +      0: disabled
> > +      1: enabled
> 
> Why are these implemented as enums? This seems to fall into the category
> of using DT to determine software policy - why's it not sufficient to
> have boolean properties that indicate hardware support and let the software
> decide what to do with them?

Hi Conor,
the reason is that I based the new properties on the existing property
"aux-voltage-chargeable":

-------------------
 aux-voltage-chargeable:
    $ref: /schemas/types.yaml#/definitions/uint32
    enum: [0, 1]
    description: |
      Tells whether the battery/supercap of the RTC (if any) is
      chargeable or not:
      0: not chargeable
      1: chargeable
-------------------

I agree with you that a boolean would be more appropriate. Should I
also submit a (separate) patch to fix the "aux-voltage-chargeable"
property to a boolean?

Hugo.


> Thanks,
> Conor.
> 
> > +
> >    quartz-load-femtofarads:
> >      description:
> >        The capacitive load of the quartz(x-tal), expressed in femto
> > -- 
> > 2.30.2
> > 

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

* Re: [PATCH 1/2] dt-bindings: rtc: add properties to set battery-related functions
  2023-08-08 12:25     ` Hugo Villeneuve
@ 2023-08-08 12:32       ` Alexandre Belloni
  2023-08-08 12:44         ` Hugo Villeneuve
  0 siblings, 1 reply; 14+ messages in thread
From: Alexandre Belloni @ 2023-08-08 12:32 UTC (permalink / raw)
  To: Hugo Villeneuve
  Cc: Conor Dooley, a.zummo, robh+dt, krzysztof.kozlowski+dt, conor+dt,
	linux-rtc, devicetree, linux-kernel, bruno.thomsen,
	Hugo Villeneuve

On 08/08/2023 08:25:33-0400, Hugo Villeneuve wrote:
> On Tue, 8 Aug 2023 12:21:24 +0100
> Conor Dooley <conor@kernel.org> wrote:
> 
> > Hey Hugo,
> > 
> > On Wed, Aug 02, 2023 at 03:11:52PM -0400, Hugo Villeneuve wrote:
> > > From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > > 
> > > These properties can be defined in the board's device tree to set the
> > > default power-on values for battery-related functions.
> > > 
> > > Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > > ---
> > >  .../devicetree/bindings/rtc/rtc.yaml          | 19 +++++++++++++++++++
> > >  1 file changed, 19 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/rtc/rtc.yaml b/Documentation/devicetree/bindings/rtc/rtc.yaml
> > > index efb66df82782..0217d229e3fa 100644
> > > --- a/Documentation/devicetree/bindings/rtc/rtc.yaml
> > > +++ b/Documentation/devicetree/bindings/rtc/rtc.yaml
> > > @@ -26,6 +26,25 @@ properties:
> > >        0: not chargeable
> > >        1: chargeable
> > >  
> > > +  battery-low-detect:
> > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > +    enum: [0, 1]
> > > +    description: |
> > > +      For RTC devices supporting a backup battery/supercap, this flag can be
> > > +      used to configure the battery low detection reporting function:
> > > +      0: disabled
> > > +      1: enabled
> > > +
> > > +  battery-switch-over:
> > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > +    enum: [0, 1]
> > > +    description: |
> > > +      For RTC devices supporting a backup battery/supercap, this flag can be
> > > +      used to configure the battery switch over when the main voltage source is
> > > +      turned off:
> > > +      0: disabled
> > > +      1: enabled
> > 
> > Why are these implemented as enums? This seems to fall into the category
> > of using DT to determine software policy - why's it not sufficient to
> > have boolean properties that indicate hardware support and let the software
> > decide what to do with them?
> 
> Hi Conor,
> the reason is that I based the new properties on the existing property
> "aux-voltage-chargeable":
> 
> -------------------
>  aux-voltage-chargeable:
>     $ref: /schemas/types.yaml#/definitions/uint32
>     enum: [0, 1]
>     description: |
>       Tells whether the battery/supercap of the RTC (if any) is
>       chargeable or not:
>       0: not chargeable
>       1: chargeable
> -------------------
> 
> I agree with you that a boolean would be more appropriate. Should I
> also submit a (separate) patch to fix the "aux-voltage-chargeable"
> property to a boolean?
> 

No, this is an enum on purpose.
I will not take battery switch over related properties, this is not
hardware description but software configuration. There is an ioctl for
this.

> Hugo.
> 
> 
> > Thanks,
> > Conor.
> > 
> > > +
> > >    quartz-load-femtofarads:
> > >      description:
> > >        The capacitive load of the quartz(x-tal), expressed in femto
> > > -- 
> > > 2.30.2
> > > 

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

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

* Re: [PATCH 1/2] dt-bindings: rtc: add properties to set battery-related functions
  2023-08-08 12:32       ` Alexandre Belloni
@ 2023-08-08 12:44         ` Hugo Villeneuve
  2023-09-05 15:30           ` Hugo Villeneuve
  0 siblings, 1 reply; 14+ messages in thread
From: Hugo Villeneuve @ 2023-08-08 12:44 UTC (permalink / raw)
  To: Alexandre Belloni
  Cc: Conor Dooley, a.zummo, robh+dt, krzysztof.kozlowski+dt, conor+dt,
	linux-rtc, devicetree, linux-kernel, bruno.thomsen,
	Hugo Villeneuve

On Tue, 8 Aug 2023 14:32:26 +0200
Alexandre Belloni <alexandre.belloni@bootlin.com> wrote:

> On 08/08/2023 08:25:33-0400, Hugo Villeneuve wrote:
> > On Tue, 8 Aug 2023 12:21:24 +0100
> > Conor Dooley <conor@kernel.org> wrote:
> > 
> > > Hey Hugo,
> > > 
> > > On Wed, Aug 02, 2023 at 03:11:52PM -0400, Hugo Villeneuve wrote:
> > > > From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > > > 
> > > > These properties can be defined in the board's device tree to set the
> > > > default power-on values for battery-related functions.
> > > > 
> > > > Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > > > ---
> > > >  .../devicetree/bindings/rtc/rtc.yaml          | 19 +++++++++++++++++++
> > > >  1 file changed, 19 insertions(+)
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/rtc/rtc.yaml b/Documentation/devicetree/bindings/rtc/rtc.yaml
> > > > index efb66df82782..0217d229e3fa 100644
> > > > --- a/Documentation/devicetree/bindings/rtc/rtc.yaml
> > > > +++ b/Documentation/devicetree/bindings/rtc/rtc.yaml
> > > > @@ -26,6 +26,25 @@ properties:
> > > >        0: not chargeable
> > > >        1: chargeable
> > > >  
> > > > +  battery-low-detect:
> > > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > > +    enum: [0, 1]
> > > > +    description: |
> > > > +      For RTC devices supporting a backup battery/supercap, this flag can be
> > > > +      used to configure the battery low detection reporting function:
> > > > +      0: disabled
> > > > +      1: enabled
> > > > +
> > > > +  battery-switch-over:
> > > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > > +    enum: [0, 1]
> > > > +    description: |
> > > > +      For RTC devices supporting a backup battery/supercap, this flag can be
> > > > +      used to configure the battery switch over when the main voltage source is
> > > > +      turned off:
> > > > +      0: disabled
> > > > +      1: enabled
> > > 
> > > Why are these implemented as enums? This seems to fall into the category
> > > of using DT to determine software policy - why's it not sufficient to
> > > have boolean properties that indicate hardware support and let the software
> > > decide what to do with them?
> > 
> > Hi Conor,
> > the reason is that I based the new properties on the existing property
> > "aux-voltage-chargeable":
> > 
> > -------------------
> >  aux-voltage-chargeable:
> >     $ref: /schemas/types.yaml#/definitions/uint32
> >     enum: [0, 1]
> >     description: |
> >       Tells whether the battery/supercap of the RTC (if any) is
> >       chargeable or not:
> >       0: not chargeable
> >       1: chargeable
> > -------------------
> > 
> > I agree with you that a boolean would be more appropriate. Should I
> > also submit a (separate) patch to fix the "aux-voltage-chargeable"
> > property to a boolean?
> > 
> 
> No, this is an enum on purpose.
> I will not take battery switch over related properties, this is not
> hardware description but software configuration. There is an ioctl for
> this.

Hi Alexandre,
can you suggest then how we can set default PWRMNG values for the
PCF2131 then?

I looked at Documentation/ABI/testing/rtc-cdev but couldn't find an
ioctl to activate the battery switch over function, nor one to activate
the battery-low detection...

Thank you,
Hugo.


> 
> > Hugo.
> > 
> > 
> > > Thanks,
> > > Conor.
> > > 
> > > > +
> > > >    quartz-load-femtofarads:
> > > >      description:
> > > >        The capacitive load of the quartz(x-tal), expressed in femto
> > > > -- 
> > > > 2.30.2
> > > > 
> 
> -- 
> Alexandre Belloni, co-owner and COO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com

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

* Re: [PATCH 1/2] dt-bindings: rtc: add properties to set battery-related functions
  2023-08-08 12:44         ` Hugo Villeneuve
@ 2023-09-05 15:30           ` Hugo Villeneuve
  2023-09-19 15:32             ` Hugo Villeneuve
  2023-09-19 15:34             ` Hugo Villeneuve
  0 siblings, 2 replies; 14+ messages in thread
From: Hugo Villeneuve @ 2023-09-05 15:30 UTC (permalink / raw)
  To: Hugo Villeneuve
  Cc: Alexandre Belloni, Conor Dooley, a.zummo, robh+dt,
	krzysztof.kozlowski+dt, conor+dt, linux-rtc, devicetree,
	linux-kernel, bruno.thomsen, Hugo Villeneuve

On Tue, 8 Aug 2023 08:44:26 -0400
Hugo Villeneuve <hugo@hugovil.com> wrote:

> On Tue, 8 Aug 2023 14:32:26 +0200
> Alexandre Belloni <alexandre.belloni@bootlin.com> wrote:
> 
> > On 08/08/2023 08:25:33-0400, Hugo Villeneuve wrote:
> > > On Tue, 8 Aug 2023 12:21:24 +0100
> > > Conor Dooley <conor@kernel.org> wrote:
> > > 
> > > > Hey Hugo,
> > > > 
> > > > On Wed, Aug 02, 2023 at 03:11:52PM -0400, Hugo Villeneuve wrote:
> > > > > From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > > > > 
> > > > > These properties can be defined in the board's device tree to set the
> > > > > default power-on values for battery-related functions.
> > > > > 
> > > > > Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > > > > ---
> > > > >  .../devicetree/bindings/rtc/rtc.yaml          | 19 +++++++++++++++++++
> > > > >  1 file changed, 19 insertions(+)
> > > > > 
> > > > > diff --git a/Documentation/devicetree/bindings/rtc/rtc.yaml b/Documentation/devicetree/bindings/rtc/rtc.yaml
> > > > > index efb66df82782..0217d229e3fa 100644
> > > > > --- a/Documentation/devicetree/bindings/rtc/rtc.yaml
> > > > > +++ b/Documentation/devicetree/bindings/rtc/rtc.yaml
> > > > > @@ -26,6 +26,25 @@ properties:
> > > > >        0: not chargeable
> > > > >        1: chargeable
> > > > >  
> > > > > +  battery-low-detect:
> > > > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > > > +    enum: [0, 1]
> > > > > +    description: |
> > > > > +      For RTC devices supporting a backup battery/supercap, this flag can be
> > > > > +      used to configure the battery low detection reporting function:
> > > > > +      0: disabled
> > > > > +      1: enabled
> > > > > +
> > > > > +  battery-switch-over:
> > > > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > > > +    enum: [0, 1]
> > > > > +    description: |
> > > > > +      For RTC devices supporting a backup battery/supercap, this flag can be
> > > > > +      used to configure the battery switch over when the main voltage source is
> > > > > +      turned off:
> > > > > +      0: disabled
> > > > > +      1: enabled
> > > > 
> > > > Why are these implemented as enums? This seems to fall into the category
> > > > of using DT to determine software policy - why's it not sufficient to
> > > > have boolean properties that indicate hardware support and let the software
> > > > decide what to do with them?
> > > 
> > > Hi Conor,
> > > the reason is that I based the new properties on the existing property
> > > "aux-voltage-chargeable":
> > > 
> > > -------------------
> > >  aux-voltage-chargeable:
> > >     $ref: /schemas/types.yaml#/definitions/uint32
> > >     enum: [0, 1]
> > >     description: |
> > >       Tells whether the battery/supercap of the RTC (if any) is
> > >       chargeable or not:
> > >       0: not chargeable
> > >       1: chargeable
> > > -------------------
> > > 
> > > I agree with you that a boolean would be more appropriate. Should I
> > > also submit a (separate) patch to fix the "aux-voltage-chargeable"
> > > property to a boolean?
> > > 
> > 
> > No, this is an enum on purpose.
> > I will not take battery switch over related properties, this is not
> > hardware description but software configuration. There is an ioctl for
> > this.
> 
> Hi Alexandre,
> can you suggest then how we can set default PWRMNG values for the
> PCF2131 then?
> 
> I looked at Documentation/ABI/testing/rtc-cdev but couldn't find an
> ioctl to activate the battery switch over function, nor one to activate
> the battery-low detection...

Ping...


> Thank you,
> Hugo.
> 
> 
> > 
> > > Hugo.
> > > 
> > > 
> > > > Thanks,
> > > > Conor.
> > > > 
> > > > > +
> > > > >    quartz-load-femtofarads:
> > > > >      description:
> > > > >        The capacitive load of the quartz(x-tal), expressed in femto
> > > > > -- 
> > > > > 2.30.2
> > > > > 
> > 
> > -- 
> > Alexandre Belloni, co-owner and COO, Bootlin
> > Embedded Linux and Kernel engineering
> > https://bootlin.com

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

* Re: [PATCH 1/2] dt-bindings: rtc: add properties to set battery-related functions
  2023-09-05 15:30           ` Hugo Villeneuve
@ 2023-09-19 15:32             ` Hugo Villeneuve
  2023-09-19 15:34             ` Hugo Villeneuve
  1 sibling, 0 replies; 14+ messages in thread
From: Hugo Villeneuve @ 2023-09-19 15:32 UTC (permalink / raw)
  To: Hugo Villeneuve
  Cc: Alexandre Belloni, Conor Dooley, a.zummo, robh+dt,
	krzysztof.kozlowski+dt, conor+dt, linux-rtc, devicetree,
	linux-kernel, bruno.thomsen, Hugo Villeneuve

On Tue, 5 Sep 2023 11:30:58 -0400
Hugo Villeneuve <hugo@hugovil.com> wrote:

> On Tue, 8 Aug 2023 08:44:26 -0400
> Hugo Villeneuve <hugo@hugovil.com> wrote:
> 
> > On Tue, 8 Aug 2023 14:32:26 +0200
> > Alexandre Belloni <alexandre.belloni@bootlin.com> wrote:
> > 
> > > On 08/08/2023 08:25:33-0400, Hugo Villeneuve wrote:
> > > > On Tue, 8 Aug 2023 12:21:24 +0100
> > > > Conor Dooley <conor@kernel.org> wrote:
> > > > 
> > > > > Hey Hugo,
> > > > > 
> > > > > On Wed, Aug 02, 2023 at 03:11:52PM -0400, Hugo Villeneuve wrote:
> > > > > > From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > > > > > 
> > > > > > These properties can be defined in the board's device tree to set the
> > > > > > default power-on values for battery-related functions.
> > > > > > 
> > > > > > Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > > > > > ---
> > > > > >  .../devicetree/bindings/rtc/rtc.yaml          | 19 +++++++++++++++++++
> > > > > >  1 file changed, 19 insertions(+)
> > > > > > 
> > > > > > diff --git a/Documentation/devicetree/bindings/rtc/rtc.yaml b/Documentation/devicetree/bindings/rtc/rtc.yaml
> > > > > > index efb66df82782..0217d229e3fa 100644
> > > > > > --- a/Documentation/devicetree/bindings/rtc/rtc.yaml
> > > > > > +++ b/Documentation/devicetree/bindings/rtc/rtc.yaml
> > > > > > @@ -26,6 +26,25 @@ properties:
> > > > > >        0: not chargeable
> > > > > >        1: chargeable
> > > > > >  
> > > > > > +  battery-low-detect:
> > > > > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > > > > +    enum: [0, 1]
> > > > > > +    description: |
> > > > > > +      For RTC devices supporting a backup battery/supercap, this flag can be
> > > > > > +      used to configure the battery low detection reporting function:
> > > > > > +      0: disabled
> > > > > > +      1: enabled
> > > > > > +
> > > > > > +  battery-switch-over:
> > > > > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > > > > +    enum: [0, 1]
> > > > > > +    description: |
> > > > > > +      For RTC devices supporting a backup battery/supercap, this flag can be
> > > > > > +      used to configure the battery switch over when the main voltage source is
> > > > > > +      turned off:
> > > > > > +      0: disabled
> > > > > > +      1: enabled
> > > > > 
> > > > > Why are these implemented as enums? This seems to fall into the category
> > > > > of using DT to determine software policy - why's it not sufficient to
> > > > > have boolean properties that indicate hardware support and let the software
> > > > > decide what to do with them?
> > > > 
> > > > Hi Conor,
> > > > the reason is that I based the new properties on the existing property
> > > > "aux-voltage-chargeable":
> > > > 
> > > > -------------------
> > > >  aux-voltage-chargeable:
> > > >     $ref: /schemas/types.yaml#/definitions/uint32
> > > >     enum: [0, 1]
> > > >     description: |
> > > >       Tells whether the battery/supercap of the RTC (if any) is
> > > >       chargeable or not:
> > > >       0: not chargeable
> > > >       1: chargeable
> > > > -------------------
> > > > 
> > > > I agree with you that a boolean would be more appropriate. Should I
> > > > also submit a (separate) patch to fix the "aux-voltage-chargeable"
> > > > property to a boolean?
> > > > 
> > > 
> > > No, this is an enum on purpose.
> > > I will not take battery switch over related properties, this is not
> > > hardware description but software configuration. There is an ioctl for
> > > this.
> > 
> > Hi Alexandre,
> > can you suggest then how we can set default PWRMNG values for the
> > PCF2131 then?
> > 
> > I looked at Documentation/ABI/testing/rtc-cdev but couldn't find an
> > ioctl to activate the battery switch over function, nor one to activate
> > the battery-low detection...
> 
> Ping...

Second ping...

Hugo.


> > Thank you,
> > Hugo.
> > 
> > 
> > > 
> > > > Hugo.
> > > > 
> > > > 
> > > > > Thanks,
> > > > > Conor.
> > > > > 
> > > > > > +
> > > > > >    quartz-load-femtofarads:
> > > > > >      description:
> > > > > >        The capacitive load of the quartz(x-tal), expressed in femto
> > > > > > -- 
> > > > > > 2.30.2
> > > > > > 
> > > 
> > > -- 
> > > Alexandre Belloni, co-owner and COO, Bootlin
> > > Embedded Linux and Kernel engineering
> > > https://bootlin.com
> 

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

* Re: [PATCH 1/2] dt-bindings: rtc: add properties to set battery-related functions
  2023-09-05 15:30           ` Hugo Villeneuve
  2023-09-19 15:32             ` Hugo Villeneuve
@ 2023-09-19 15:34             ` Hugo Villeneuve
  2023-10-11 13:49               ` Hugo Villeneuve
  2023-10-11 22:23               ` Hugo Villeneuve
  1 sibling, 2 replies; 14+ messages in thread
From: Hugo Villeneuve @ 2023-09-19 15:34 UTC (permalink / raw)
  To: Alexandre Belloni
  Cc: Conor Dooley, a.zummo, robh+dt, krzysztof.kozlowski+dt, conor+dt,
	linux-rtc, devicetree, linux-kernel, bruno.thomsen,
	Hugo Villeneuve

On Tue, 5 Sep 2023 11:30:58 -0400
Hugo Villeneuve <hugo@hugovil.com> wrote:

> On Tue, 8 Aug 2023 08:44:26 -0400
> Hugo Villeneuve <hugo@hugovil.com> wrote:
> 
> > On Tue, 8 Aug 2023 14:32:26 +0200
> > Alexandre Belloni <alexandre.belloni@bootlin.com> wrote:
> > 
> > > On 08/08/2023 08:25:33-0400, Hugo Villeneuve wrote:
> > > > On Tue, 8 Aug 2023 12:21:24 +0100
> > > > Conor Dooley <conor@kernel.org> wrote:
> > > > 
> > > > > Hey Hugo,
> > > > > 
> > > > > On Wed, Aug 02, 2023 at 03:11:52PM -0400, Hugo Villeneuve wrote:
> > > > > > From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > > > > > 
> > > > > > These properties can be defined in the board's device tree to set the
> > > > > > default power-on values for battery-related functions.
> > > > > > 
> > > > > > Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > > > > > ---
> > > > > >  .../devicetree/bindings/rtc/rtc.yaml          | 19 +++++++++++++++++++
> > > > > >  1 file changed, 19 insertions(+)
> > > > > > 
> > > > > > diff --git a/Documentation/devicetree/bindings/rtc/rtc.yaml b/Documentation/devicetree/bindings/rtc/rtc.yaml
> > > > > > index efb66df82782..0217d229e3fa 100644
> > > > > > --- a/Documentation/devicetree/bindings/rtc/rtc.yaml
> > > > > > +++ b/Documentation/devicetree/bindings/rtc/rtc.yaml
> > > > > > @@ -26,6 +26,25 @@ properties:
> > > > > >        0: not chargeable
> > > > > >        1: chargeable
> > > > > >  
> > > > > > +  battery-low-detect:
> > > > > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > > > > +    enum: [0, 1]
> > > > > > +    description: |
> > > > > > +      For RTC devices supporting a backup battery/supercap, this flag can be
> > > > > > +      used to configure the battery low detection reporting function:
> > > > > > +      0: disabled
> > > > > > +      1: enabled
> > > > > > +
> > > > > > +  battery-switch-over:
> > > > > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > > > > +    enum: [0, 1]
> > > > > > +    description: |
> > > > > > +      For RTC devices supporting a backup battery/supercap, this flag can be
> > > > > > +      used to configure the battery switch over when the main voltage source is
> > > > > > +      turned off:
> > > > > > +      0: disabled
> > > > > > +      1: enabled
> > > > > 
> > > > > Why are these implemented as enums? This seems to fall into the category
> > > > > of using DT to determine software policy - why's it not sufficient to
> > > > > have boolean properties that indicate hardware support and let the software
> > > > > decide what to do with them?
> > > > 
> > > > Hi Conor,
> > > > the reason is that I based the new properties on the existing property
> > > > "aux-voltage-chargeable":
> > > > 
> > > > -------------------
> > > >  aux-voltage-chargeable:
> > > >     $ref: /schemas/types.yaml#/definitions/uint32
> > > >     enum: [0, 1]
> > > >     description: |
> > > >       Tells whether the battery/supercap of the RTC (if any) is
> > > >       chargeable or not:
> > > >       0: not chargeable
> > > >       1: chargeable
> > > > -------------------
> > > > 
> > > > I agree with you that a boolean would be more appropriate. Should I
> > > > also submit a (separate) patch to fix the "aux-voltage-chargeable"
> > > > property to a boolean?
> > > > 
> > > 
> > > No, this is an enum on purpose.
> > > I will not take battery switch over related properties, this is not
> > > hardware description but software configuration. There is an ioctl for
> > > this.
> > 
> > Hi Alexandre,
> > can you suggest then how we can set default PWRMNG values for the
> > PCF2131 then?
> > 
> > I looked at Documentation/ABI/testing/rtc-cdev but couldn't find an
> > ioctl to activate the battery switch over function, nor one to activate
> > the battery-low detection...
> 
> Ping...

Second ping...

Hugo.


> > Thank you,
> > Hugo.
> > 
> > 
> > > 
> > > > Hugo.
> > > > 
> > > > 
> > > > > Thanks,
> > > > > Conor.
> > > > > 
> > > > > > +
> > > > > >    quartz-load-femtofarads:
> > > > > >      description:
> > > > > >        The capacitive load of the quartz(x-tal), expressed in femto
> > > > > > -- 
> > > > > > 2.30.2
> > > > > > 
> > > 
> > > -- 
> > > Alexandre Belloni, co-owner and COO, Bootlin
> > > Embedded Linux and Kernel engineering
> > > https://bootlin.com
> 

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

* Re: [PATCH 1/2] dt-bindings: rtc: add properties to set battery-related functions
  2023-09-19 15:34             ` Hugo Villeneuve
@ 2023-10-11 13:49               ` Hugo Villeneuve
  2023-10-11 22:23               ` Hugo Villeneuve
  1 sibling, 0 replies; 14+ messages in thread
From: Hugo Villeneuve @ 2023-10-11 13:49 UTC (permalink / raw)
  To: Hugo Villeneuve
  Cc: Alexandre Belloni, Conor Dooley, a.zummo, robh+dt,
	krzysztof.kozlowski+dt, conor+dt, linux-rtc, devicetree,
	linux-kernel, bruno.thomsen, Hugo Villeneuve

On Tue, 19 Sep 2023 11:34:23 -0400
Hugo Villeneuve <hugo@hugovil.com> wrote:

> On Tue, 5 Sep 2023 11:30:58 -0400
> Hugo Villeneuve <hugo@hugovil.com> wrote:
> 
> > On Tue, 8 Aug 2023 08:44:26 -0400
> > Hugo Villeneuve <hugo@hugovil.com> wrote:
> > 
> > > On Tue, 8 Aug 2023 14:32:26 +0200
> > > Alexandre Belloni <alexandre.belloni@bootlin.com> wrote:
> > > 
> > > > On 08/08/2023 08:25:33-0400, Hugo Villeneuve wrote:
> > > > > On Tue, 8 Aug 2023 12:21:24 +0100
> > > > > Conor Dooley <conor@kernel.org> wrote:
> > > > > 
> > > > > > Hey Hugo,
> > > > > > 
> > > > > > On Wed, Aug 02, 2023 at 03:11:52PM -0400, Hugo Villeneuve wrote:
> > > > > > > From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > > > > > > 
> > > > > > > These properties can be defined in the board's device tree to set the
> > > > > > > default power-on values for battery-related functions.
> > > > > > > 
> > > > > > > Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > > > > > > ---
> > > > > > >  .../devicetree/bindings/rtc/rtc.yaml          | 19 +++++++++++++++++++
> > > > > > >  1 file changed, 19 insertions(+)
> > > > > > > 
> > > > > > > diff --git a/Documentation/devicetree/bindings/rtc/rtc.yaml b/Documentation/devicetree/bindings/rtc/rtc.yaml
> > > > > > > index efb66df82782..0217d229e3fa 100644
> > > > > > > --- a/Documentation/devicetree/bindings/rtc/rtc.yaml
> > > > > > > +++ b/Documentation/devicetree/bindings/rtc/rtc.yaml
> > > > > > > @@ -26,6 +26,25 @@ properties:
> > > > > > >        0: not chargeable
> > > > > > >        1: chargeable
> > > > > > >  
> > > > > > > +  battery-low-detect:
> > > > > > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > > > > > +    enum: [0, 1]
> > > > > > > +    description: |
> > > > > > > +      For RTC devices supporting a backup battery/supercap, this flag can be
> > > > > > > +      used to configure the battery low detection reporting function:
> > > > > > > +      0: disabled
> > > > > > > +      1: enabled
> > > > > > > +
> > > > > > > +  battery-switch-over:
> > > > > > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > > > > > +    enum: [0, 1]
> > > > > > > +    description: |
> > > > > > > +      For RTC devices supporting a backup battery/supercap, this flag can be
> > > > > > > +      used to configure the battery switch over when the main voltage source is
> > > > > > > +      turned off:
> > > > > > > +      0: disabled
> > > > > > > +      1: enabled
> > > > > > 
> > > > > > Why are these implemented as enums? This seems to fall into the category
> > > > > > of using DT to determine software policy - why's it not sufficient to
> > > > > > have boolean properties that indicate hardware support and let the software
> > > > > > decide what to do with them?
> > > > > 
> > > > > Hi Conor,
> > > > > the reason is that I based the new properties on the existing property
> > > > > "aux-voltage-chargeable":
> > > > > 
> > > > > -------------------
> > > > >  aux-voltage-chargeable:
> > > > >     $ref: /schemas/types.yaml#/definitions/uint32
> > > > >     enum: [0, 1]
> > > > >     description: |
> > > > >       Tells whether the battery/supercap of the RTC (if any) is
> > > > >       chargeable or not:
> > > > >       0: not chargeable
> > > > >       1: chargeable
> > > > > -------------------
> > > > > 
> > > > > I agree with you that a boolean would be more appropriate. Should I
> > > > > also submit a (separate) patch to fix the "aux-voltage-chargeable"
> > > > > property to a boolean?
> > > > > 
> > > > 
> > > > No, this is an enum on purpose.
> > > > I will not take battery switch over related properties, this is not
> > > > hardware description but software configuration. There is an ioctl for
> > > > this.
> > > 
> > > Hi Alexandre,
> > > can you suggest then how we can set default PWRMNG values for the
> > > PCF2131 then?
> > > 
> > > I looked at Documentation/ABI/testing/rtc-cdev but couldn't find an
> > > ioctl to activate the battery switch over function, nor one to activate
> > > the battery-low detection...
> > 
> > Ping...
> 
> Second ping...
> 
> Hugo.

Third ping...

Hugo.


> 
> 
> > > Thank you,
> > > Hugo.
> > > 
> > > 
> > > > 
> > > > > Hugo.
> > > > > 
> > > > > 
> > > > > > Thanks,
> > > > > > Conor.
> > > > > > 
> > > > > > > +
> > > > > > >    quartz-load-femtofarads:
> > > > > > >      description:
> > > > > > >        The capacitive load of the quartz(x-tal), expressed in femto
> > > > > > > -- 
> > > > > > > 2.30.2
> > > > > > > 
> > > > 
> > > > -- 
> > > > Alexandre Belloni, co-owner and COO, Bootlin
> > > > Embedded Linux and Kernel engineering
> > > > https://bootlin.com
> > 

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

* Re: [PATCH 1/2] dt-bindings: rtc: add properties to set battery-related functions
  2023-09-19 15:34             ` Hugo Villeneuve
  2023-10-11 13:49               ` Hugo Villeneuve
@ 2023-10-11 22:23               ` Hugo Villeneuve
  2023-11-01 14:19                 ` Hugo Villeneuve
  2023-11-01 14:21                 ` Hugo Villeneuve
  1 sibling, 2 replies; 14+ messages in thread
From: Hugo Villeneuve @ 2023-10-11 22:23 UTC (permalink / raw)
  To: Alexandre Belloni
  Cc: Conor Dooley, a.zummo, robh+dt, krzysztof.kozlowski+dt, conor+dt,
	linux-rtc, devicetree, linux-kernel, bruno.thomsen,
	Hugo Villeneuve

On Tue, 19 Sep 2023 11:34:23 -0400
Hugo Villeneuve <hugo@hugovil.com> wrote:

> On Tue, 5 Sep 2023 11:30:58 -0400
> Hugo Villeneuve <hugo@hugovil.com> wrote:
> 
> > On Tue, 8 Aug 2023 08:44:26 -0400
> > Hugo Villeneuve <hugo@hugovil.com> wrote:
> > 
> > > On Tue, 8 Aug 2023 14:32:26 +0200
> > > Alexandre Belloni <alexandre.belloni@bootlin.com> wrote:
> > > 
> > > > On 08/08/2023 08:25:33-0400, Hugo Villeneuve wrote:
> > > > > On Tue, 8 Aug 2023 12:21:24 +0100
> > > > > Conor Dooley <conor@kernel.org> wrote:
> > > > > 
> > > > > > Hey Hugo,
> > > > > > 
> > > > > > On Wed, Aug 02, 2023 at 03:11:52PM -0400, Hugo Villeneuve wrote:
> > > > > > > From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > > > > > > 
> > > > > > > These properties can be defined in the board's device tree to set the
> > > > > > > default power-on values for battery-related functions.
> > > > > > > 
> > > > > > > Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > > > > > > ---
> > > > > > >  .../devicetree/bindings/rtc/rtc.yaml          | 19 +++++++++++++++++++
> > > > > > >  1 file changed, 19 insertions(+)
> > > > > > > 
> > > > > > > diff --git a/Documentation/devicetree/bindings/rtc/rtc.yaml b/Documentation/devicetree/bindings/rtc/rtc.yaml
> > > > > > > index efb66df82782..0217d229e3fa 100644
> > > > > > > --- a/Documentation/devicetree/bindings/rtc/rtc.yaml
> > > > > > > +++ b/Documentation/devicetree/bindings/rtc/rtc.yaml
> > > > > > > @@ -26,6 +26,25 @@ properties:
> > > > > > >        0: not chargeable
> > > > > > >        1: chargeable
> > > > > > >  
> > > > > > > +  battery-low-detect:
> > > > > > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > > > > > +    enum: [0, 1]
> > > > > > > +    description: |
> > > > > > > +      For RTC devices supporting a backup battery/supercap, this flag can be
> > > > > > > +      used to configure the battery low detection reporting function:
> > > > > > > +      0: disabled
> > > > > > > +      1: enabled
> > > > > > > +
> > > > > > > +  battery-switch-over:
> > > > > > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > > > > > +    enum: [0, 1]
> > > > > > > +    description: |
> > > > > > > +      For RTC devices supporting a backup battery/supercap, this flag can be
> > > > > > > +      used to configure the battery switch over when the main voltage source is
> > > > > > > +      turned off:
> > > > > > > +      0: disabled
> > > > > > > +      1: enabled
> > > > > > 
> > > > > > Why are these implemented as enums? This seems to fall into the category
> > > > > > of using DT to determine software policy - why's it not sufficient to
> > > > > > have boolean properties that indicate hardware support and let the software
> > > > > > decide what to do with them?
> > > > > 
> > > > > Hi Conor,
> > > > > the reason is that I based the new properties on the existing property
> > > > > "aux-voltage-chargeable":
> > > > > 
> > > > > -------------------
> > > > >  aux-voltage-chargeable:
> > > > >     $ref: /schemas/types.yaml#/definitions/uint32
> > > > >     enum: [0, 1]
> > > > >     description: |
> > > > >       Tells whether the battery/supercap of the RTC (if any) is
> > > > >       chargeable or not:
> > > > >       0: not chargeable
> > > > >       1: chargeable
> > > > > -------------------
> > > > > 
> > > > > I agree with you that a boolean would be more appropriate. Should I
> > > > > also submit a (separate) patch to fix the "aux-voltage-chargeable"
> > > > > property to a boolean?
> > > > > 
> > > > 
> > > > No, this is an enum on purpose.
> > > > I will not take battery switch over related properties, this is not
> > > > hardware description but software configuration. There is an ioctl for
> > > > this.
> > > 
> > > Hi Alexandre,
> > > can you suggest then how we can set default PWRMNG values for the
> > > PCF2131 then?
> > > 
> > > I looked at Documentation/ABI/testing/rtc-cdev but couldn't find an
> > > ioctl to activate the battery switch over function, nor one to activate
> > > the battery-low detection...
> > 
> > Ping...
> 
> Second ping...
> 
> Hugo.

Third ping...

Hugo.


> 
> 
> > > Thank you,
> > > Hugo.
> > > 
> > > 
> > > > 
> > > > > Hugo.
> > > > > 
> > > > > 
> > > > > > Thanks,
> > > > > > Conor.
> > > > > > 
> > > > > > > +
> > > > > > >    quartz-load-femtofarads:
> > > > > > >      description:
> > > > > > >        The capacitive load of the quartz(x-tal), expressed in femto
> > > > > > > -- 
> > > > > > > 2.30.2
> > > > > > > 
> > > > 
> > > > -- 
> > > > Alexandre Belloni, co-owner and COO, Bootlin
> > > > Embedded Linux and Kernel engineering
> > > > https://bootlin.com
> > 

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

* Re: [PATCH 1/2] dt-bindings: rtc: add properties to set battery-related functions
  2023-10-11 22:23               ` Hugo Villeneuve
@ 2023-11-01 14:19                 ` Hugo Villeneuve
  2023-11-01 14:21                 ` Hugo Villeneuve
  1 sibling, 0 replies; 14+ messages in thread
From: Hugo Villeneuve @ 2023-11-01 14:19 UTC (permalink / raw)
  To: Hugo Villeneuve
  Cc: Alexandre Belloni, Conor Dooley, a.zummo, robh+dt,
	krzysztof.kozlowski+dt, conor+dt, linux-rtc, devicetree,
	linux-kernel, bruno.thomsen, Hugo Villeneuve

On Wed, 11 Oct 2023 18:23:30 -0400
Hugo Villeneuve <hugo@hugovil.com> wrote:

> On Tue, 19 Sep 2023 11:34:23 -0400
> Hugo Villeneuve <hugo@hugovil.com> wrote:
> 
> > On Tue, 5 Sep 2023 11:30:58 -0400
> > Hugo Villeneuve <hugo@hugovil.com> wrote:
> > 
> > > On Tue, 8 Aug 2023 08:44:26 -0400
> > > Hugo Villeneuve <hugo@hugovil.com> wrote:
> > > 
> > > > On Tue, 8 Aug 2023 14:32:26 +0200
> > > > Alexandre Belloni <alexandre.belloni@bootlin.com> wrote:
> > > > 
> > > > > On 08/08/2023 08:25:33-0400, Hugo Villeneuve wrote:
> > > > > > On Tue, 8 Aug 2023 12:21:24 +0100
> > > > > > Conor Dooley <conor@kernel.org> wrote:
> > > > > > 
> > > > > > > Hey Hugo,
> > > > > > > 
> > > > > > > On Wed, Aug 02, 2023 at 03:11:52PM -0400, Hugo Villeneuve wrote:
> > > > > > > > From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > > > > > > > 
> > > > > > > > These properties can be defined in the board's device tree to set the
> > > > > > > > default power-on values for battery-related functions.
> > > > > > > > 
> > > > > > > > Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > > > > > > > ---
> > > > > > > >  .../devicetree/bindings/rtc/rtc.yaml          | 19 +++++++++++++++++++
> > > > > > > >  1 file changed, 19 insertions(+)
> > > > > > > > 
> > > > > > > > diff --git a/Documentation/devicetree/bindings/rtc/rtc.yaml b/Documentation/devicetree/bindings/rtc/rtc.yaml
> > > > > > > > index efb66df82782..0217d229e3fa 100644
> > > > > > > > --- a/Documentation/devicetree/bindings/rtc/rtc.yaml
> > > > > > > > +++ b/Documentation/devicetree/bindings/rtc/rtc.yaml
> > > > > > > > @@ -26,6 +26,25 @@ properties:
> > > > > > > >        0: not chargeable
> > > > > > > >        1: chargeable
> > > > > > > >  
> > > > > > > > +  battery-low-detect:
> > > > > > > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > > > > > > +    enum: [0, 1]
> > > > > > > > +    description: |
> > > > > > > > +      For RTC devices supporting a backup battery/supercap, this flag can be
> > > > > > > > +      used to configure the battery low detection reporting function:
> > > > > > > > +      0: disabled
> > > > > > > > +      1: enabled
> > > > > > > > +
> > > > > > > > +  battery-switch-over:
> > > > > > > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > > > > > > +    enum: [0, 1]
> > > > > > > > +    description: |
> > > > > > > > +      For RTC devices supporting a backup battery/supercap, this flag can be
> > > > > > > > +      used to configure the battery switch over when the main voltage source is
> > > > > > > > +      turned off:
> > > > > > > > +      0: disabled
> > > > > > > > +      1: enabled
> > > > > > > 
> > > > > > > Why are these implemented as enums? This seems to fall into the category
> > > > > > > of using DT to determine software policy - why's it not sufficient to
> > > > > > > have boolean properties that indicate hardware support and let the software
> > > > > > > decide what to do with them?
> > > > > > 
> > > > > > Hi Conor,
> > > > > > the reason is that I based the new properties on the existing property
> > > > > > "aux-voltage-chargeable":
> > > > > > 
> > > > > > -------------------
> > > > > >  aux-voltage-chargeable:
> > > > > >     $ref: /schemas/types.yaml#/definitions/uint32
> > > > > >     enum: [0, 1]
> > > > > >     description: |
> > > > > >       Tells whether the battery/supercap of the RTC (if any) is
> > > > > >       chargeable or not:
> > > > > >       0: not chargeable
> > > > > >       1: chargeable
> > > > > > -------------------
> > > > > > 
> > > > > > I agree with you that a boolean would be more appropriate. Should I
> > > > > > also submit a (separate) patch to fix the "aux-voltage-chargeable"
> > > > > > property to a boolean?
> > > > > > 
> > > > > 
> > > > > No, this is an enum on purpose.
> > > > > I will not take battery switch over related properties, this is not
> > > > > hardware description but software configuration. There is an ioctl for
> > > > > this.
> > > > 
> > > > Hi Alexandre,
> > > > can you suggest then how we can set default PWRMNG values for the
> > > > PCF2131 then?
> > > > 
> > > > I looked at Documentation/ABI/testing/rtc-cdev but couldn't find an
> > > > ioctl to activate the battery switch over function, nor one to activate
> > > > the battery-low detection...
> > > 
> > > Ping...
> > 
> > Second ping...
> > 
> > Hugo.
> 
> Third ping...

Hi Alexandre,
Fourth ping...

People are writing to me off the mailing
list to indicate that there is a "bug" with the PCF2131 driver relating
to PWRMNG incorrect values.

Can you answer my question so that we can find a solution to this
problem?

Hugo.


> > > > > > > > +
> > > > > > > >    quartz-load-femtofarads:
> > > > > > > >      description:
> > > > > > > >        The capacitive load of the quartz(x-tal), expressed in femto
> > > > > > > > -- 
> > > > > > > > 2.30.2
> > > > > > > > 
> > > > > 
> > > > > -- 
> > > > > Alexandre Belloni, co-owner and COO, Bootlin
> > > > > Embedded Linux and Kernel engineering
> > > > > https://bootlin.com
> > > 

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

* Re: [PATCH 1/2] dt-bindings: rtc: add properties to set battery-related functions
  2023-10-11 22:23               ` Hugo Villeneuve
  2023-11-01 14:19                 ` Hugo Villeneuve
@ 2023-11-01 14:21                 ` Hugo Villeneuve
  1 sibling, 0 replies; 14+ messages in thread
From: Hugo Villeneuve @ 2023-11-01 14:21 UTC (permalink / raw)
  To: Alexandre Belloni
  Cc: Conor Dooley, a.zummo, robh+dt, krzysztof.kozlowski+dt, conor+dt,
	linux-rtc, devicetree, linux-kernel, bruno.thomsen,
	Hugo Villeneuve

On Wed, 11 Oct 2023 18:23:30 -0400
Hugo Villeneuve <hugo@hugovil.com> wrote:

> On Tue, 19 Sep 2023 11:34:23 -0400
> Hugo Villeneuve <hugo@hugovil.com> wrote:
> 
> > On Tue, 5 Sep 2023 11:30:58 -0400
> > Hugo Villeneuve <hugo@hugovil.com> wrote:
> > 
> > > On Tue, 8 Aug 2023 08:44:26 -0400
> > > Hugo Villeneuve <hugo@hugovil.com> wrote:
> > > 
> > > > On Tue, 8 Aug 2023 14:32:26 +0200
> > > > Alexandre Belloni <alexandre.belloni@bootlin.com> wrote:
> > > > 
> > > > > On 08/08/2023 08:25:33-0400, Hugo Villeneuve wrote:
> > > > > > On Tue, 8 Aug 2023 12:21:24 +0100
> > > > > > Conor Dooley <conor@kernel.org> wrote:
> > > > > > 
> > > > > > > Hey Hugo,
> > > > > > > 
> > > > > > > On Wed, Aug 02, 2023 at 03:11:52PM -0400, Hugo Villeneuve wrote:
> > > > > > > > From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > > > > > > > 
> > > > > > > > These properties can be defined in the board's device tree to set the
> > > > > > > > default power-on values for battery-related functions.
> > > > > > > > 
> > > > > > > > Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > > > > > > > ---
> > > > > > > >  .../devicetree/bindings/rtc/rtc.yaml          | 19 +++++++++++++++++++
> > > > > > > >  1 file changed, 19 insertions(+)
> > > > > > > > 
> > > > > > > > diff --git a/Documentation/devicetree/bindings/rtc/rtc.yaml b/Documentation/devicetree/bindings/rtc/rtc.yaml
> > > > > > > > index efb66df82782..0217d229e3fa 100644
> > > > > > > > --- a/Documentation/devicetree/bindings/rtc/rtc.yaml
> > > > > > > > +++ b/Documentation/devicetree/bindings/rtc/rtc.yaml
> > > > > > > > @@ -26,6 +26,25 @@ properties:
> > > > > > > >        0: not chargeable
> > > > > > > >        1: chargeable
> > > > > > > >  
> > > > > > > > +  battery-low-detect:
> > > > > > > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > > > > > > +    enum: [0, 1]
> > > > > > > > +    description: |
> > > > > > > > +      For RTC devices supporting a backup battery/supercap, this flag can be
> > > > > > > > +      used to configure the battery low detection reporting function:
> > > > > > > > +      0: disabled
> > > > > > > > +      1: enabled
> > > > > > > > +
> > > > > > > > +  battery-switch-over:
> > > > > > > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > > > > > > +    enum: [0, 1]
> > > > > > > > +    description: |
> > > > > > > > +      For RTC devices supporting a backup battery/supercap, this flag can be
> > > > > > > > +      used to configure the battery switch over when the main voltage source is
> > > > > > > > +      turned off:
> > > > > > > > +      0: disabled
> > > > > > > > +      1: enabled
> > > > > > > 
> > > > > > > Why are these implemented as enums? This seems to fall into the category
> > > > > > > of using DT to determine software policy - why's it not sufficient to
> > > > > > > have boolean properties that indicate hardware support and let the software
> > > > > > > decide what to do with them?
> > > > > > 
> > > > > > Hi Conor,
> > > > > > the reason is that I based the new properties on the existing property
> > > > > > "aux-voltage-chargeable":
> > > > > > 
> > > > > > -------------------
> > > > > >  aux-voltage-chargeable:
> > > > > >     $ref: /schemas/types.yaml#/definitions/uint32
> > > > > >     enum: [0, 1]
> > > > > >     description: |
> > > > > >       Tells whether the battery/supercap of the RTC (if any) is
> > > > > >       chargeable or not:
> > > > > >       0: not chargeable
> > > > > >       1: chargeable
> > > > > > -------------------
> > > > > > 
> > > > > > I agree with you that a boolean would be more appropriate. Should I
> > > > > > also submit a (separate) patch to fix the "aux-voltage-chargeable"
> > > > > > property to a boolean?
> > > > > > 
> > > > > 
> > > > > No, this is an enum on purpose.
> > > > > I will not take battery switch over related properties, this is not
> > > > > hardware description but software configuration. There is an ioctl for
> > > > > this.
> > > > 
> > > > Hi Alexandre,
> > > > can you suggest then how we can set default PWRMNG values for the
> > > > PCF2131 then?
> > > > 
> > > > I looked at Documentation/ABI/testing/rtc-cdev but couldn't find an
> > > > ioctl to activate the battery switch over function, nor one to activate
> > > > the battery-low detection...
> > > 
> > > Ping...
> > 
> > Second ping...
> > 
> > Hugo.
> 
> Third ping...

Hi Alexandre,
Fourth ping...

People are writing to me off the mailing
list to indicate that there is a "bug" with the PCF2131 driver relating
to PWRMNG incorrect values.

Can you answer my question so that we can find a solution to this
problem?

Hugo.


> > > > > > > > +
> > > > > > > >    quartz-load-femtofarads:
> > > > > > > >      description:
> > > > > > > >        The capacitive load of the quartz(x-tal), expressed in femto
> > > > > > > > -- 
> > > > > > > > 2.30.2
> > > > > > > > 
> > > > > 
> > > > > -- 
> > > > > Alexandre Belloni, co-owner and COO, Bootlin
> > > > > Embedded Linux and Kernel engineering
> > > > > https://bootlin.com
> > > 

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

end of thread, other threads:[~2023-11-01 14:21 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-02 19:11 [PATCH 0/2] rtc: pcf2127: add default battery-related power-on values Hugo Villeneuve
2023-08-02 19:11 ` [PATCH 1/2] dt-bindings: rtc: add properties to set battery-related functions Hugo Villeneuve
2023-08-08 11:21   ` Conor Dooley
2023-08-08 12:25     ` Hugo Villeneuve
2023-08-08 12:32       ` Alexandre Belloni
2023-08-08 12:44         ` Hugo Villeneuve
2023-09-05 15:30           ` Hugo Villeneuve
2023-09-19 15:32             ` Hugo Villeneuve
2023-09-19 15:34             ` Hugo Villeneuve
2023-10-11 13:49               ` Hugo Villeneuve
2023-10-11 22:23               ` Hugo Villeneuve
2023-11-01 14:19                 ` Hugo Villeneuve
2023-11-01 14:21                 ` Hugo Villeneuve
2023-08-02 19:11 ` [PATCH 2/2] rtc: pcf2127: add support for battery-related DT properties Hugo Villeneuve

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).