linux-kernel - RE: [PATCH] drivers: mfd: da9063: Add restart notifier implementation

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Fri, 15 Oct 2021 08:47:27 +0000
From:   Adam Thomson <Adam.Thomson.Opensource@...semi.com>
To:     Alexandre Ghiti <alexandre.ghiti@...onical.com>,
        Adam Thomson <Adam.Thomson.Opensource@...semi.com>
CC:     David Abdurachmanov <david.abdurachmanov@...il.com>,
        Support Opensource <Support.Opensource@...semi.com>,
        Lee Jones <lee.jones@...aro.org>,
        "linux-riscv@...ts.infradead.org" <linux-riscv@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH] drivers: mfd: da9063: Add restart notifier implementation

On 14 October 2021 16:51, Alexandre Ghiti wrote:

> > So having examined your sequence again it's now clearer as to what is
> happening.
> > With the sequence you provided this is only a partial reset whereby all of the
> > output rails are sequenced down then up again and restored to OTP voltages.
> > However the remainder of the chip settings aren't reset as this isn't a true
> > reset of the device going through full reload from OTP, so for example settings
> > of regulator mode GPIO states, or IRQ mask bits would persist on the restart,
> > which could have implications on system operation.
> 
> Ok, it's not perfect but I think those are settings that will get
> reinitialized by the corresponding drivers while booting, contrary to
> the RTC registers which are clobbered by the other method.

Well I guess that depends on which DA9063 drivers are being loaded and what is
populated in DT. This is something you'll need to consider now and with future
updates around this board to make sure nothing untoward is happening as a side
effect.

> 
> >
> > In addition the only bits of interest for you should be:
> >
> >  - CONTROL_F (0x13)
> >         WAKE_UP (BIT 2) = 1
> >  - CONTROL_A (0x0E)
> >         SYSTEM_EN (BIT 0) = 0
> >
> > Setting those two bits should be enough to trigger the partial reset sequence.
> > The other bits you had in your sequence don't seem to be necessary or
> relevant.
> >
> > One final caveat to this approach is that there is a 16s internal timer (as you
> > noted before, VDD_START) which is started when the device moves to ACTIVE
> mode.
> > When that 16s timer expires the device will clear the WAKE_UP bit
> automatically.
> > This means there's the outside chance that you could try the reset command
> > sequence above around the same time, and that could mean you set the
> WAKE_UP
> > bit, but it's immediately cleared again by this timer expiry before the
> > SYSTEM_EN bit is set low. In that case there would be a need for an external
> > event (e.g. ONKEY) to kick the system to start again.
> 
> Ok, the risk exists but the window is quite small.
> 
> After all, the solution I first proposed is not perfect, but now we
> know why it works and IMO it has less drawbacks than using the RTC
> registers, so I think we should go for this solution. I'll see if I
> can help Nikita implement this directly in openSBI.

Personally, if it was possible I think the RTC approach would be best as it's a
full reset and to me is far safer with regards to potential side effects, but as
that's not on the table then this seems the only other approach in your case.

> @Adam Thomson I had migrated the DA9063 device tree bindings to yaml,
> I'll push that soon. Thanks for all your help, much appreciated.

No problem and thanks for making that update. I'll take a look when the
changes are available.