Re: [PATCH v2 1/3] dt-bindings: rtc: pcf8523: add DSM pm option for battery switch-over

[Jon Nettleton <jon@solid-run.com>]

On Mon, Jul 27, 2020 at 4:17 PM Russell King - ARM Linux admin
<linux@armlinux.org.uk> wrote:
>
> On Mon, Jul 27, 2020 at 03:33:17PM +0200, Jon Nettleton wrote:
> > On Mon, Jul 27, 2020 at 11:46 AM Russell King - ARM Linux admin
> > <linux@armlinux.org.uk> wrote:
> > >
> > > On Thu, Jul 23, 2020 at 09:57:55PM +0200, Alexandre Belloni wrote:
> > > > On 23/07/2020 11:49:05-0600, Rob Herring wrote:
> > > > > On Mon, Jul 20, 2020 at 12:23:59PM +0100, miguelborgesdefreitas@gmail.com wrote:
> > > > > > From: Miguel Borges de Freitas <miguelborgesdefreitas@gmail.com>
> > > > > >
> > > > > > This adds direct-switching mode as a configurable DT flag for
> > > > > > RTC modules supporting it (e.g. nxp pcf8523).
> > > > > > DSM switches the power source to the battery supply whenever the
> > > > > > VDD drops below VBAT. The option is recommended for hw designs
> > > > > > where VDD is always expected to be higher than VBAT.
> > > > > >
> > > > > > Signed-off-by: Miguel Borges de Freitas <miguelborgesdefreitas@gmail.com>
> > > > > > ---
> > > > > > Changes in v2:
> > > > > > - Added extended commit message for git history
> > > > > > - Separate dt bindings documentation into a single patch
> > > > > >
> > > > > >  Documentation/devicetree/bindings/rtc/nxp,pcf8523.txt | 7 ++++++-
> > > > > >  Documentation/devicetree/bindings/rtc/rtc.yaml        | 7 +++++++
> > > > > >  2 files changed, 13 insertions(+), 1 deletion(-)
> > > > > >
> > > > > > diff --git a/Documentation/devicetree/bindings/rtc/nxp,pcf8523.txt b/Documentation/devicetree/bindings/rtc/nxp,pcf8523.txt
> > > > > > index 0b1080c..f715a8f 100644
> > > > > > --- a/Documentation/devicetree/bindings/rtc/nxp,pcf8523.txt
> > > > > > +++ b/Documentation/devicetree/bindings/rtc/nxp,pcf8523.txt
> > > > > > @@ -4,10 +4,14 @@ Required properties:
> > > > > >  - compatible: Should contain "nxp,pcf8523".
> > > > > >  - reg: I2C address for chip.
> > > > > >
> > > > > > -Optional property:
> > > > > > +Optional properties:
> > > > > >  - quartz-load-femtofarads: The capacitive load of the quartz(x-tal),
> > > > > >    expressed in femto Farad (fF). Valid values are 7000 and 12500.
> > > > > >    Default value (if no value is specified) is 12500fF.
> > > > > > +- pm-enable-dsm: battery switch-over function is enabled in direct
> > > > > > +  switching mode. The power failure condition happens when VDD < VBAT,
> > > > > > +  without requiring VDD to drop below Vth(sw)bat.
> > > > > > +  Default value (if not provided) is the standard mode.
> > > > > >
> > > > > >  Example:
> > > > > >
> > > > > > @@ -15,4 +19,5 @@ pcf8523: rtc@68 {
> > > > > >   compatible = "nxp,pcf8523";
> > > > > >   reg = <0x68>;
> > > > > >   quartz-load-femtofarads = <7000>;
> > > > > > + pm-enable-dsm;
> > > > > >  };
> > > > > > diff --git a/Documentation/devicetree/bindings/rtc/rtc.yaml b/Documentation/devicetree/bindings/rtc/rtc.yaml
> > > > > > index ee237b2..a0048f4 100644
> > > > > > --- a/Documentation/devicetree/bindings/rtc/rtc.yaml
> > > > > > +++ b/Documentation/devicetree/bindings/rtc/rtc.yaml
> > > > > > @@ -47,4 +47,11 @@ properties:
> > > > > >      description:
> > > > > >        Enables wake up of host system on alarm.
> > > > > >
> > > > > > +  pm-enable-dsm:
> > > > > > +    $ref: /schemas/types.yaml#/definitions/flag
> > > > > > +    description:
> > > > > > +      Enables the battery switch-over function in direct switching
> > > > > > +      mode. Should be set in systems where VDD is higher than VBAT
> > > > > > +      at all times.
> > > > >
> > > > > I'm all for common properties, but is this common across vendors?
> > > > >
> > > >
> > > > This is but this shouldn't be a DT property as it has to be changed
> > > > dynamically. I'm working on an ioctl interface to change this
> > > > configuration.
> > >
> > > Why does it need to be changed dynamically?  If the hardware components
> > > are not fitted to allow the RTC to be safely used without DSM, then
> > > why should userspace be able to disable DSM?
> > >
> >
> > My presumption would be if you had a system that ran at different
> > system voltages depending if it is plugged in to mains or running on a
> > battery.
>
> Yes, but we're not talking about that case with the Cubox-i.
>
> Should a platform like the Cubox-i allow the user to disable DSM?
>
> There needs to be a way to block the ability to dynamically change
> this mode if the hardware is not up to operating without DSM.
>

Agreed.  Do we need a modes supported device-tree property?  That
would actually describe the hardware as device-tree is supposed to do.