linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Mark Brown <broonie@kernel.org>
To: Bartosz Golaszewski <brgl@bgdev.pl>
Cc: Liam Girdwood <lgirdwood@gmail.com>,
	linux-kernel@vger.kernel.org,
	Bartosz Golaszewski <bgolaszewski@baylibre.com>
Subject: Re: [PATCH 2/2] regulator: provide regulator_set_voltage_sel_regmap_step() helper
Date: Mon, 10 Dec 2018 15:41:25 +0000	[thread overview]
Message-ID: <20181210154125.GE6307@sirena.org.uk> (raw)
In-Reply-To: <20181210151024.3906-3-brgl@bgdev.pl>

[-- Attachment #1: Type: text/plain, Size: 860 bytes --]

On Mon, Dec 10, 2018 at 04:10:24PM +0100, Bartosz Golaszewski wrote:

> On some devices we need to manually ramp the regulators to desired
> voltage one step at a time. This patch adds a helper routine for
> regmap users that checks if the regulator is enabled and, if so,

My first question would be why do this at the regmap helper level, why
not do this at the core level by stepping through selectors?  I'd also
expect to be programming the step size here, while some regulators
aren't able to regulate well over larger step changes I'd expect that
they wouldn't need to step through every single possible voltage value,
that's potentially extremely slow.

> +	asc = diff > 0 ? false : true;
> +	end = asc ? sel + 1 : sel - 1;
> +	asc ? curr++ : curr--;

Please just write normal conditional statements so the code is more
readable and hence maintainable.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

  reply	other threads:[~2018-12-10 15:41 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-10 15:10 [PATCH 0/2] regulator: stepping set voltage for regmap users Bartosz Golaszewski
2018-12-10 15:10 ` [PATCH 1/2] regulator: make _regulator_is_enabled() available in internal.h Bartosz Golaszewski
2018-12-10 15:10 ` [PATCH 2/2] regulator: provide regulator_set_voltage_sel_regmap_step() helper Bartosz Golaszewski
2018-12-10 15:41   ` Mark Brown [this message]
2018-12-11 10:55     ` Bartosz Golaszewski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181210154125.GE6307@sirena.org.uk \
    --to=broonie@kernel.org \
    --cc=bgolaszewski@baylibre.com \
    --cc=brgl@bgdev.pl \
    --cc=lgirdwood@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).