Linux ARM-MSM sub-architecture
 help / color / mirror / Atom feed
From: Stephan Gerhold <stephan.gerhold@kernkonzept.com>
To: Mark Brown <broonie@kernel.org>
Cc: Liam Girdwood <lgirdwood@gmail.com>,
	Andy Gross <agross@kernel.org>,
	Bjorn Andersson <andersson@kernel.org>,
	Konrad Dybcio <konrad.dybcio@linaro.org>,
	linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org,
	Stephan Gerhold <stephan@gerhold.net>
Subject: Re: [PATCH RFC 1/2] regulator: core: Disable unused regulators with unknown status
Date: Tue, 24 Oct 2023 10:57:38 +0200	[thread overview]
Message-ID: <ZTeHAqL5QB2w33RN@kernkonzept.com> (raw)
In-Reply-To: <80307316-f55e-4540-9c5f-655844c3b3f4@sirena.org.uk>

On Mon, Oct 23, 2023 at 01:09:11PM +0100, Mark Brown wrote:
> On Wed, Oct 04, 2023 at 04:17:17PM +0200, Stephan Gerhold wrote:
> 
> > Instead of -EINVAL we could also use a different return code to indicate
> > the initial status is unknown. Or maybe there is some other option that
> > would be easier? This is working for me but I'm sending it as RFC to get
> > more feedback. :)
> 
> The more normal thing here would be -EBUSY I think - -EINVAL kind of
> indicates that the operation will never work while in reality it could
> possibly work in future.  Though for the RPMH it's not really the case
> that it ever supports readback, what it does is have it's own reference
> counting in the driver.  Rather than doing this we should probably have
> logic in the core which sees that the driver has a write operation but
> no read operation and implements appropriate behaviour.

Yep, I agree that it would be nicer to handle this case in the core,
rather than duplicating the logic in all the RPM-related drivers.

I think it does not change much for this patch, though. Even when
implemented in the core we still need to represent this situation
somehow for regulator_is_enabled(). Simply returning 0 (disabled) or
1 (enabled) would be wrong. Do you think returning -EBUSY would be
appropriate for that?

The second challenge I see on a quick look is that both
qcom_smd-regulator.c and qcom-rpmh-regulator.c use their reference
counter internally in other function (e.g. to decide if a voltage change
should be sent, see "vreg->enabled" checks). I think we would also need
to add some rdev_is_enabled() function that would expose the core
reference counter to the driver?

Tracking the enable state in the driver (the way it is right now) is not
that much code, so I'm not entirely sure if we might actually end up
with more code/complexity when moving this to the core.

Thanks,
-- 
Stephan Gerhold <stephan.gerhold@kernkonzept.com>
Kernkonzept GmbH at Dresden, Germany, HRB 31129, CEO Dr.-Ing. Michael Hohmuth

  parent reply	other threads:[~2023-10-24  8:57 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-04 14:17 [PATCH RFC 0/2] regulator: qcom_smd: Disable unused regulators Stephan Gerhold
2023-10-04 14:17 ` [PATCH RFC 1/2] regulator: core: Disable unused regulators with unknown status Stephan Gerhold
2023-10-06 21:11   ` Konrad Dybcio
2023-10-09 20:21     ` Stephan Gerhold
2023-10-10 12:14       ` Konrad Dybcio
2023-10-23 12:09   ` Mark Brown
2023-10-23 23:11     ` Bjorn Andersson
2023-10-24  8:57     ` Stephan Gerhold [this message]
2023-10-25 17:49       ` Mark Brown
2023-10-25 19:51         ` Stephan Gerhold
2023-10-25 20:07           ` Mark Brown
2023-10-04 14:17 ` [PATCH RFC 2/2] regulator: qcom_smd: Disable unused regulators Stephan Gerhold
2023-10-06 21:15   ` Konrad Dybcio
2023-10-09 20:23     ` Stephan Gerhold
2023-10-09 21:00       ` Konrad Dybcio

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=ZTeHAqL5QB2w33RN@kernkonzept.com \
    --to=stephan.gerhold@kernkonzept.com \
    --cc=agross@kernel.org \
    --cc=andersson@kernel.org \
    --cc=broonie@kernel.org \
    --cc=konrad.dybcio@linaro.org \
    --cc=lgirdwood@gmail.com \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=stephan@gerhold.net \
    /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