[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20140610105334.GF5099@sirena.org.uk>
Date: Tue, 10 Jun 2014 11:53:34 +0100
From: Mark Brown <broonie@...nel.org>
To: Javier Martinez Canillas <javier.martinez@...labora.co.uk>
Cc: Lee Jones <lee.jones@...aro.org>,
Samuel Ortiz <sameo@...ux.intel.com>,
Mike Turquette <mturquette@...aro.org>,
Liam Girdwood <lgirdwood@...il.com>,
Alessandro Zummo <a.zummo@...ertech.it>,
Kukjin Kim <kgene.kim@...sung.com>,
Doug Anderson <dianders@...omium.org>,
Olof Johansson <olof@...om.net>,
Sjoerd Simons <sjoerd.simons@...labora.co.uk>,
Daniel Stone <daniels@...labora.com>,
Tomeu Vizoso <tomeu.vizoso@...labora.com>,
linux-arm-kernel@...ts.infradead.org, devicetree@...r.kernel.org,
linux-samsung-soc@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/5] regulator: Add driver for Maxim 77802 PMIC regulators
On Tue, Jun 10, 2014 at 01:29:17AM +0200, Javier Martinez Canillas wrote:
> On 06/09/2014 09:38 PM, Mark Brown wrote:
> > On Mon, Jun 09, 2014 at 11:37:47AM +0200, Javier Martinez Canillas wrote:
> >> + case REGULATOR_MODE_STANDBY: /* switch off */
> >> + if (id != MAX77802_LDO1 && id != MAX77802_LDO20 &&
> >> + id != MAX77802_LDO21 && id != MAX77802_LDO3) {
> >> + val = MAX77802_OPMODE_STANDBY;
> >> + break;
> >> + }
> >> + /* no break */
> > This sounds very broken...
> The problem is that not all regulators supports the same operational modes. For
> instance regulators LDO 1, 20, 21 and 3 does not support REGULATOR_MODE_STANDBY
> so if the condition is not met a break is not needed since the default case is
> to warn that the mode is not supported.
No, it's the "switch off" comment...
> But I'll rework that logic on v2 to make it cleaner and have a break on each
> case and don't rely on case cascading.
...though you should also consider splitting things up so you have
separate ops for separate regulators if they behave differently.
Download attachment "signature.asc" of type "application/pgp-signature" (820 bytes)
Powered by blists - more mailing lists