All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mark Brown <broonie@kernel.org>
To: Sumit Semwal <sumit.semwal@linaro.org>
Cc: agross@kernel.org, Bjorn Andersson <bjorn.andersson@linaro.org>,
	lgirdwood@gmail.com, robh+dt@kernel.org,
	Nisha Kumari <nishakumari@codeaurora.org>,
	linux-arm-msm@vger.kernel.org,
	LKML <linux-kernel@vger.kernel.org>,
	devicetree@vger.kernel.org, kgunda@codeaurora.org,
	Rajendra Nayak <rnayak@codeaurora.org>
Subject: Re: [PATCH v4 4/5] regulator: qcom: Add labibb driver
Date: Wed, 17 Jun 2020 13:06:01 +0100	[thread overview]
Message-ID: <20200617120601.GE4613@sirena.org.uk> (raw)
In-Reply-To: <CAO_48GF9pKZCCof170TvB0ubOkecDzcGhtUUuY_Td78L1J338A@mail.gmail.com>

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

On Wed, Jun 17, 2020 at 05:27:12PM +0530, Sumit Semwal wrote:
> On Wed, 17 Jun 2020 at 17:17, Mark Brown <broonie@kernel.org> wrote:
> > On Wed, Jun 17, 2020 at 05:12:35PM +0530, Sumit Semwal wrote:

> > > I understand from a pure regulators' correctness point of view,
> > > ENABLE_CTL should be the one checked there, so I can change the patch
> > > as you suggested, but there seems to be some performance penalty
> > > there.

> > I thought the goal was to have the performance penalty to ensure that
> > the regulator had actually started?

> IMHO, with the poll_enabled_time mechanism added, we would not need to
> wait for the full enabled_time time for the regulator to get enabled,
> but we could poll (and potentially know earlier) if the regulator is
> enabled.
> The performance penalty I was talking, is about how should we check if
> the regulator is really enabled or not - via reading the STATUS1
> register, which seems to tell the status a bit faster, or via reading
> the ENABLE_CTL register which we also use to enable/disable the
> regulator, but which seems to be slower in updating the status.

That seems...  interesting.  Are you sure the regulator has fully ramped
when STATUS1 starts flagging?

> > > > > The WARN_ON? This was suggested by Bjorn to catch the case where the
> > > > > DT binding for a PMIC instantiates only one of the regulators.

> > > > No, this whole loop - why this whole match and get child stuff?

> > > This loop mechanism is what I saw in the other qcom regulators
> > > upstream, so thought it was an acceptable way.
> > > For the two children nodes, do you recommend another mechanism to get
> > > and validate both nodes?

> > I don't understand what you mean by "two children nodes" here?

> The two 'lab' and 'ibb' regulator nodes that are part of the labibb node.

Use of_match and regulators_node like other regulator drivers.

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

  reply	other threads:[~2020-06-17 12:06 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-02 10:09 [PATCH v4 0/5] Qualcomm labibb regulator driver Sumit Semwal
2020-06-02 10:09 ` [PATCH v4 1/5] regulator: Allow regulators to verify enabled during enable() Sumit Semwal
2020-06-02 11:24   ` Mark Brown
2020-06-02 11:57     ` Sumit Semwal
2020-06-18 23:44   ` Bjorn Andersson
2020-06-02 10:09 ` [PATCH v4 2/5] dt-bindings: regulator: Add labibb regulator Sumit Semwal
2020-06-09 22:52   ` Rob Herring
2020-06-02 10:09 ` [PATCH v4 3/5] arm64: dts: qcom: pmi8998: Add nodes for LAB and IBB regulators Sumit Semwal
2020-06-02 10:09 ` [PATCH v4 4/5] regulator: qcom: Add labibb driver Sumit Semwal
2020-06-02 11:32   ` Mark Brown
2020-06-02 12:10     ` Sumit Semwal
2020-06-02 12:25       ` Mark Brown
2020-06-17 11:42         ` Sumit Semwal
2020-06-17 11:47           ` Mark Brown
2020-06-17 11:57             ` Sumit Semwal
2020-06-17 12:06               ` Mark Brown [this message]
2020-06-17 12:09                 ` Sumit Semwal
2020-06-17 12:40                   ` Mark Brown
2020-06-02 10:09 ` [PATCH v4 5/5] regulator: qcom: labibb: Add SC interrupt handling Sumit Semwal
2020-06-02 12:22   ` Mark Brown
2020-06-17 12:06     ` Sumit Semwal
2020-06-17 12:38       ` Mark Brown

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=20200617120601.GE4613@sirena.org.uk \
    --to=broonie@kernel.org \
    --cc=agross@kernel.org \
    --cc=bjorn.andersson@linaro.org \
    --cc=devicetree@vger.kernel.org \
    --cc=kgunda@codeaurora.org \
    --cc=lgirdwood@gmail.com \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nishakumari@codeaurora.org \
    --cc=rnayak@codeaurora.org \
    --cc=robh+dt@kernel.org \
    --cc=sumit.semwal@linaro.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.