* [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
* 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
* [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
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 an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.