Linux-ARM-MSM Archive on lore.kernel.org
 help / color / Atom feed
From: Jeffrey Hugo <jeffrey.l.hugo@gmail.com>
To: Mark Brown <broonie@kernel.org>
Cc: lgirdwood@gmail.com, Andy Gross <agross@kernel.org>,
	Bjorn Andersson <bjorn.andersson@linaro.org>,
	Rob Herring <robh+dt@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	MSM <linux-arm-msm@vger.kernel.org>,
	lkml <linux-kernel@vger.kernel.org>,
	devicetree@vger.kernel.org
Subject: Re: [PATCH v4 4/7] regulator: qcom_spmi: Add support for PM8005
Date: Mon, 17 Jun 2019 13:41:24 -0600
Message-ID: <CAOCk7NpwMzhbbvwp-vy3TvbadTHqxqVau6yfds+pSOPG=2Qb7g@mail.gmail.com> (raw)
In-Reply-To: <20190617192413.GI5316@sirena.org.uk>

On Mon, Jun 17, 2019 at 1:24 PM Mark Brown <broonie@kernel.org> wrote:
>
> On Mon, Jun 17, 2019 at 01:06:42PM -0600, Jeffrey Hugo wrote:
> > On Mon, Jun 17, 2019 at 12:37 PM Mark Brown <broonie@kernel.org> wrote:
>
> > > Something really weird is going on with the word wrapping in your mail,
> > > it looks like you're writing lines longer than 80 characters (120?) and
> > > they're getting a newline added in the middle of the line to wrap them
> > > without reflowing the paragraph.
>
> > Doh.  Hopefully this one is better.
>
> Yes, thanks!  Though now it's off-list :/

Doh.  Added everyone back.

>
> > > Well, it doesn't *have* to be the raw register value, more accurately
> > > it's some token which is useful for passing to and from the hardware;
> > > The documentation such as it
> > > is is in the documentation of the list_voltage() operation (which is
> > > what defines the selector values for a given driver).
>
> > Ok, so this one bit -
> > Selectors range from zero to one less than regulator_desc.n_voltages.
> > Voltages may be reported in any order.
>
> Yes.
>
> > So, I understand that.  Its an indexing of the supported voltages.
> > From my perspective, that has nothing to do with hardware.  Its just a
> > remapping of the values to a different set.  Voltages X, Y, and Z map
> > to 0, 1, and 2.  Its a token so that the driver and the framework can
> > use a common value.
>
> > I really think we are on the same page here, its just I was getting
> > confused by how you were describing it in your replies.
>
> Yes, I was just coming from the perspective that for almost all hardware
> the selectors are chosen to be the values that are in the bitfield that
> the hardware uses to specify the voltage since that's the most common
> thing and tends to make things simpler for people, it's also the primary
> place where the concept came from.
>
> > > Your idea of very basic implementations is how the overwhelming majority
> > > of hardware is implemented.  Regulators with register maps will tend to
> > > just have a bitfield where you set the voltage with each valid value in
> > > that bitfield mapped to a single voltage, the exceptions tend to use
> > > direct voltage values instead (and not support enumeration at all).
>
> > > Looking at the driver I think it's got what the helpers are terming
> > > pickable linear ranges (naming is hard) and could use those helpers.
>
> > I'm pretty sure Stephen Boyd and Bjorn just investigated that a few
> > weeks ago, and came to the conclusion that it can't because of things
> > like the hardware really wants to stay in the same range if at all
> > possible, which is not behavior the pickable linear ranges seems to
> > support.
>
> Sounds like it just needs a custom map_voltage() function?  Though
> thinking about it it's possibly worth just making the standard map try
> to keep things in the same range as that will if nothing else reduce the
> number of I/O operations.  Probably also reduce glitches if there's
> overlapping ranges.

I didn't think so when I was paying attention to their discussion.
However maybe I missed something.  I think I'll take another look.
Might make for a nice cleanup, but if its just in the driver, or if it
involves updating the framework, it seems to be outside scope for this
series of changes.

  parent reply index

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-13 21:24 [PATCH v4 0/7] PM8005 and PMS405 regulator support Jeffrey Hugo
2019-06-13 21:25 ` [PATCH v4 1/7] regulator: qcom_spmi: enable linear range info Jeffrey Hugo
2019-06-13 21:25   ` [PATCH v4 2/7] regulator: qcom_spmi: Refactor get_mode/set_mode Jeffrey Hugo
2019-06-13 21:32     ` Bjorn Andersson
2019-06-17 15:24     ` Applied "regulator: qcom_spmi: Refactor get_mode/set_mode" to the regulator tree Mark Brown
2019-06-17 15:24   ` Applied "regulator: qcom_spmi: enable linear range info" " Mark Brown
2019-06-13 21:25 ` [PATCH v4 3/7] dt-bindings: qcom_spmi: Document PM8005 regulators Jeffrey Hugo
2019-06-13 21:25   ` [PATCH v4 4/7] regulator: qcom_spmi: Add support for PM8005 Jeffrey Hugo
2019-06-17 15:05     ` Mark Brown
2019-06-17 15:17       ` Jeffrey Hugo
2019-06-17 16:03         ` Mark Brown
2019-06-17 17:07           ` Jeffrey Hugo
2019-06-17 18:37             ` Mark Brown
     [not found]               ` <CAOCk7NpbZwAreGpVCvF2yFBDJKbAxBZ23oncfF_SyEwoiC2+PQ@mail.gmail.com>
     [not found]                 ` <20190617192413.GI5316@sirena.org.uk>
2019-06-17 19:41                   ` Jeffrey Hugo [this message]
2019-06-13 21:26 ` [PATCH v4 5/7] arm64: dts: msm8998-mtp: Add pm8005_s1 regulator Jeffrey Hugo
2019-06-13 21:27 ` [PATCH v4 6/7] dt-bindings: qcom_spmi: Document pms405 support Jeffrey Hugo
2019-06-13 21:27   ` [PATCH v4 7/7] regulator: qcom_spmi: add PMS405 SPMI regulator Jeffrey Hugo
2019-06-13 21:37     ` Bjorn Andersson
2019-06-13 21:37   ` [PATCH v4 6/7] dt-bindings: qcom_spmi: Document pms405 support Bjorn Andersson
2019-06-17 14:58 ` [PATCH v4 0/7] PM8005 and PMS405 regulator support Mark Brown
2019-06-17 15:04   ` Jeffrey Hugo
2019-06-17 15:12     ` Mark Brown

Reply instructions:

You may reply publically 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='CAOCk7NpwMzhbbvwp-vy3TvbadTHqxqVau6yfds+pSOPG=2Qb7g@mail.gmail.com' \
    --to=jeffrey.l.hugo@gmail.com \
    --cc=agross@kernel.org \
    --cc=bjorn.andersson@linaro.org \
    --cc=broonie@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=lgirdwood@gmail.com \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=robh+dt@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

Linux-ARM-MSM Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-arm-msm/0 linux-arm-msm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-arm-msm linux-arm-msm/ https://lore.kernel.org/linux-arm-msm \
		linux-arm-msm@vger.kernel.org linux-arm-msm@archiver.kernel.org
	public-inbox-index linux-arm-msm


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-arm-msm


AGPL code for this site: git clone https://public-inbox.org/ public-inbox