[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20170204182733.GD12980@dtor-ws>
Date: Sat, 4 Feb 2017 10:27:33 -0800
From: Dmitry Torokhov <dmitry.torokhov@...il.com>
To: Mark Brown <broonie@...nel.org>
Cc: Liam Girdwood <lgirdwood@...il.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 4/5] regulator: core: simplify _regulator_get()
On Sat, Feb 04, 2017 at 11:34:58AM +0100, Mark Brown wrote:
> On Fri, Feb 03, 2017 at 01:56:03PM -0800, Dmitry Torokhov wrote:
> > The code in _regulator_get() got a bit confusing over time, with control
> > flow jumping to a label from couple of places. Let's untangle it a bit.
>
> This is quite hard to review without a concrete description of what the
> changes actually are...
>
> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@...il.com>
> > ---
> > drivers/regulator/core.c | 66 +++++++++++++++++++++++++-----------------------
> > 1 file changed, 34 insertions(+), 32 deletions(-)
>
> ...it's a fairly large change in core code which is as you say a little
> complicated.
OK, so the change is twofold:
1. We make handling of missing supplies and substituting them with dummy
regulators more explicit:
- check if we not have full constraints and refuse considering dummy
regulators with appropriate message
- use "switch (get_type)" to handle different types of request explicit
as well. "Normal" requests will get dummies, exclusive will not and
will notify user about that; optional will fail silently.
2. We will not be jumping to a label normal in the middle of the
function but instead have proper conditional flow. I believe jumps
should be reserved for error handling, breaking from inner loop, or
restarting a loop, but not for implementing normal conditional flow.
Do you need me to put this into commit description or this is
sufficient?
Thanks.
--
Dmitry
Powered by blists - more mailing lists