linux-kernel - Re: [PATCH 2/2] regulator: provide regulator_set_voltage_sel_regmap

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

Message-ID: <CAMRc=MfC-E9DpMhDSXSQ-o5xG1ONccbT35=0rHsZy6oLt9j8cA@mail.gmail.com>
Date:   Tue, 11 Dec 2018 11:55:37 +0100
From:   Bartosz Golaszewski <brgl@...ev.pl>
To:     Mark Brown <broonie@...nel.org>
Cc:     Liam Girdwood <lgirdwood@...il.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Bartosz Golaszewski <bgolaszewski@...libre.com>
Subject: Re: [PATCH 2/2] regulator: provide regulator_set_voltage_sel_regmap_step()
 helper

pon., 10 gru 2018 o 16:41 Mark Brown <broonie@...nel.org> napisał(a):
>
> 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.
>

This sounds like a good improvement to the core regulator code indeed.
I'll give it a try but I'm not sure I'll have something before the
next merge window, so for now I'll put this logic into my driver that
I'd like to get merged for 4.22 and we can factor it out later on once
we agree on the right approach.

> > +     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.

Sure.

Thanks,
Bartosz