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: <20190403093015.GJ11301@dell>
Date:   Wed, 3 Apr 2019 10:30:15 +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:

> 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?

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