[PATCH 0/2] rtc: pcf2127: add default battery-related power-on values

[PATCH 0/2] rtc: pcf2127: add default battery-related power-on values

[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

[PATCH 1/2] dt-bindings: rtc: add properties to set battery-related functions

[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

[PATCH 2/2] rtc: pcf2127: add support for battery-related DT properties

[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

Re: [PATCH 1/2] dt-bindings: rtc: add properties to set battery-related functions

[Conor Dooley]

[-- 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 --]

Re: [PATCH 1/2] dt-bindings: rtc: add properties to set battery-related functions

[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
> > 

Re: [PATCH 1/2] dt-bindings: rtc: add properties to set battery-related functions

[Alexandre Belloni]

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

Re: [PATCH 1/2] dt-bindings: rtc: add properties to set battery-related functions

[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

Re: [PATCH 1/2] dt-bindings: rtc: add properties to set battery-related functions

[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

Re: [PATCH 1/2] dt-bindings: rtc: add properties to set battery-related functions

[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
> 

Re: [PATCH 1/2] dt-bindings: rtc: add properties to set battery-related functions

[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
> 

Re: [PATCH 1/2] dt-bindings: rtc: add properties to set battery-related functions

[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
> > 

Re: [PATCH 1/2] dt-bindings: rtc: add properties to set battery-related functions

[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
> > 

Re: [PATCH 1/2] dt-bindings: rtc: add properties to set battery-related functions

[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
> > > 

Re: [PATCH 1/2] dt-bindings: rtc: add properties to set battery-related functions

[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
> > >