Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20200719145028.3370-3-miguelborgesdefreitas@gmail.com>
 <20200720112401.4620-1-miguelborgesdefreitas@gmail.com> <20200720112401.4620-2-miguelborgesdefreitas@gmail.com>
 <20200723174905.GA596242@bogus> <20200723195755.GV3428@piout.net>
 <20200727094553.GH1551@shell.armlinux.org.uk> <20200727144938.GC239143@piout.net>
 <20200727152439.GK1551@shell.armlinux.org.uk> <20200727154104.GE239143@piout.net>
 <20200727154335.GL1551@shell.armlinux.org.uk>
In-Reply-To: <20200727154335.GL1551@shell.armlinux.org.uk>
From:   Jon Nettleton <jon@solid-run.com>
Date:   Mon, 27 Jul 2020 17:55:50 +0200
Message-ID: <CABdtJHuVaTa5T0-KdQ-wZQrmFQ6HO3FvgnTgSo3aOi+=SPzDZA@mail.gmail.com>
Subject: Re: [PATCH v2 1/3] dt-bindings: rtc: pcf8523: add DSM pm option for
 battery switch-over
To:     Russell King - ARM Linux admin <linux@armlinux.org.uk>
Cc:     Alexandre Belloni <alexandre.belloni@bootlin.com>,
        Rob Herring <robh@kernel.org>,
        Miguel Borges de Freitas <miguelborgesdefreitas@gmail.com>,
        a.zummo@towertech.it, Baruch Siach <baruch@tkos.co.il>,
        Shawn Guo <shawnguo@kernel.org>,
        Sascha Hauer <s.hauer@pengutronix.de>,
        Sascha Hauer <kernel@pengutronix.de>,
        Fabio Estevam <festevam@gmail.com>,
        NXP Linux Team <linux-imx@nxp.com>,
        devicetree <devicetree@vger.kernel.org>,
        linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Mon, Jul 27, 2020 at 5:43 PM Russell King - ARM Linux admin
<linux@armlinux.org.uk> wrote:
>
> On Mon, Jul 27, 2020 at 05:41:04PM +0200, Alexandre Belloni wrote:
> > On 27/07/2020 16:24:39+0100, Russell King - ARM Linux admin wrote:
> > > On Mon, Jul 27, 2020 at 04:49:38PM +0200, Alexandre Belloni wrote:
> > > > On 27/07/2020 10:45:53+0100, Russell King - ARM Linux admin wrote:
> > > > > > 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?
> > > >
> > > > For RTCs with a standby mode, you want to be able to return to standby
> > > > mode.
> > > >
> > > > That would happen for example after factory flashing in that common use
> > > > case:
> > > >  - the board is manufactured
> > > >  - Vbackup is installed, the RTC switches to standby mode
> > > >  - the board is then booted to flash a system, Vprimary is now present,
> > > >    the RTC switches to DSM.
> > > >
> > > > At this point, if the board is simply shut down, the RTC will start
> > > > draining Vbackup before leaving the factory. Instead, we want to be able
> > > > to return to standby mode until the final user switches the product on
> > > > for the first time.
> > >
> > > I don't think you're understanding what's going on with this proposed
> > > patch.  The cubox-i does work today, and the RTC does survive most
> > > power-downs. There are situations where it doesn't.
> > >
> > > So, let's take your process above.
> > >
> > > - the board is manufactured
> > > - Vbackup is installed, the RTC switches to standby mode
> > > - the board is then booted to flash a system, Vprimary is now present
> > > - the board is powered down.  the RTC _might_ switch over to battery
> > >   if it notices the power failure in time, or it might not.  A random
> > >   sample of units leaving the factory have the RTC in standby mode.
> > >   Others are draining the battery.
> > >
> > > I'm not saying what you propose isn't a good idea.  I'm questioning
> > > why we should expose this in the generic kernel on platforms where
> > > it's likely to end up with the RTC being corrupted.
> > >
> >
> > Note that I didn't say we should expose settings that are not working
> > but it is a different discussion.
>
> It isn't a different discussion - that is exactly what the point of
> my emails to you all along have been!
>
> So, can we please have that discussion, it is pertinent to this patch.
>

Thinking about this some more, I believe whether or not an IOCTL
interface is in the works or needed is irrelevant.  This patch
describes the hardware and how it is designed and the topic of
discussion is if we need a simple boolean state, or if we need
something that could be used to support dynamic configuration in the
future.  I would rather make this decision now rather than keep
tacking on boolean config options, or revisit a bunch of device-tree
changes.