linux-kernel - Re: [PATCH v11 2/8] mfd: bd70528: Support ROHM bd70528 PMIC

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190403112559.GP11301@dell>
Date:   Wed, 3 Apr 2019 12:25:59 +0100
From:   Lee Jones <lee.jones@...aro.org>
To:     Matti Vaittinen <matti.vaittinen@...rohmeurope.com>
Cc:     mazziesaccount@...il.com, Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Michael Turquette <mturquette@...libre.com>,
        Stephen Boyd <sboyd@...nel.org>,
        Linus Walleij <linus.walleij@...aro.org>,
        Bartosz Golaszewski <bgolaszewski@...libre.com>,
        Sebastian Reichel <sre@...nel.org>,
        Liam Girdwood <lgirdwood@...il.com>,
        Mark Brown <broonie@...nel.org>,
        Alessandro Zummo <a.zummo@...ertech.it>,
        Alexandre Belloni <alexandre.belloni@...tlin.com>,
        Wim Van Sebroeck <wim@...ux-watchdog.org>,
        Guenter Roeck <linux@...ck-us.net>, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-clk@...r.kernel.org,
        linux-gpio@...r.kernel.org, linux-pm@...r.kernel.org,
        linux-rtc@...r.kernel.org, linux-watchdog@...r.kernel.org,
        heikki.haikola@...rohmeurope.com, mikko.mutanen@...rohmeurope.com
Subject: Re: [PATCH v11 2/8] mfd: bd70528: Support ROHM bd70528 PMIC - core

On Wed, 03 Apr 2019, Matti Vaittinen wrote:

> On Wed, Apr 03, 2019 at 10:30:15AM +0100, Lee Jones wrote:
> > On Wed, 03 Apr 2019, Matti Vaittinen wrote:
> > 
> > > Hello Lee,
> > > 
> > > Thanks for taking a look on this again =) I agree with most of the
> > > comments and correct them at next version.
> > > 
> > > On Wed, Apr 03, 2019 at 08:31:52AM +0100, Lee Jones wrote:
> > > > On Mon, 25 Mar 2019, Matti Vaittinen wrote:
> > > > 
> > > > > ROHM BD70528MWV is an ultra-low quiescent current general
> > > > > purpose single-chip power management IC for battery-powered
> > > > > portable devices.
> > > > > 
> > > > > Add MFD core which enables chip access for following subdevices:
> > > > > 	- regulators/LED drivers
> > > > > 	- battery-charger
> > > > > 	- gpios
> > > > > 	- 32.768kHz clk
> > > > > 	- RTC
> > > > > 	- watchdog
> > > > > 
> > > > > Signed-off-by: Matti Vaittinen <matti.vaittinen@...rohmeurope.com>
> > > > > + * Mapping of main IRQ register bits to sub irq register offsets so
> > > > 
> > > > "sub-IRQ"
> > > > 
> > > > > + * that we can access corect sub IRQ registers based on bits that
> > > > 
> > > > "sub IRQ" is also fine, but please standardise.
> > > > 
> > > > I do prefer "sub-IRQ" though.
> > > 
> > > I'll go with "sub-IRQ" then
> > > 
> > > > > +
> > > > > +#define WD_CTRL_MAGIC1 0x55
> > > > > +#define WD_CTRL_MAGIC2 0xAA
> > > > > +/**
> > > > > + * bd70528_wdt_set - arm or disarm watchdog timer
> > > > > + *
> > > > > + * @data:	device data for the PMIC instance we want to operate on
> > > > > + * @enable:	new state of WDT. zero to disable, non zero to enable
> > > > > + * @old_state:	previous state of WDT will be filled here
> > > > > + *
> > > > > + * Arm or disarm WDT on BD70528 PMIC. Expected to be called only by
> > > > > + * BD70528 RTC and BD70528 WDT drivers. The rtc_timer_lock must be taken
> > > > > + * by calling bd70528_wdt_lock before calling bd70528_wdt_set.
> > > > > + */
> > > > > +int bd70528_wdt_set(struct rohm_regmap_dev *data, int enable, int *old_state)
> > > > 
> > > > Why doesn't this reside in the watchdog driver?
> > > 
> > > If my memory serves me right we shortly discussed this already during v8
> > > review ;) Cant blame you though as I have seen some of the mail traffic
> > > going through your inbox :D
> > > 
> > > The motivation to have the functions exported from MFD is to not create
> > > sirect dependency between RTC and WDT. There may be cases where we want
> > > to leave either RTC or WDT out of compilation. MFD is always needed so
> > > the dependency from MFD to RTC/WDT does not harm.
> > > 
> > > (Here's some discussion necromancy if you are interested in re-reading
> > > how we did end up with this implementation:
> > > https://lore.kernel.org/lkml/20190212091723.GZ20638@dell/)
> > > 
> > > I hope you are still Ok with having the WDT control functions in MFD.
> > 
> > OOI, why does the RTC need to control the WDT?
> 
> I thought I had a comment about this somewhere in code... O_o Must have
> been in some development branch I had :/
> 
> Anyways, setting the RTC counter may cause watchdog to trigger. It is not
> further explained why but I would guess watchdog uses RTC counter to check
> if it should've been pinged already. So RTC needs to disable watch dog for
> the duration of hwclock setting and enable it again after the new time is
> set. I can add a comment about this to MFD driver if it helps :)

How does the user select between using the RTC and the WDT?

Or are the generally both enabled at the same time?

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog