Re: [PATCH v1 11/30] drm/tegra: dc: Support OPP and SoC core voltage scaling

From: Mark Brown <broonie@kernel.org>
To: Dmitry Osipenko <digetx@gmail.com>
Cc: "Thierry Reding" <thierry.reding@gmail.com>,
	"Jonathan Hunter" <jonathanh@nvidia.com>,
	"Alan Stern" <stern@rowland.harvard.edu>,
	"Peter Chen" <Peter.Chen@nxp.com>,
	"Liam Girdwood" <lgirdwood@gmail.com>,
	"Adrian Hunter" <adrian.hunter@intel.com>,
	"Krzysztof Kozlowski" <krzk@kernel.org>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"Lee Jones" <lee.jones@linaro.org>,
	"Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>,
	"Ulf Hansson" <ulf.hansson@linaro.org>,
	"Mauro Carvalho Chehab" <mchehab@kernel.org>,
	"Rob Herring" <robh+dt@kernel.org>,
	"Marek Szyprowski" <m.szyprowski@samsung.com>,
	"Peter Geis" <pgwipeout@gmail.com>,
	"Nicolas Chauvet" <kwizart@gmail.com>,
	linux-samsung-soc@vger.kernel.org, devel@driverdev.osuosl.org,
	linux-usb@vger.kernel.org, linux-pwm@vger.kernel.org,
	linux-mmc@vger.kernel.org, linux-kernel@vger.kernel.org,
	devicetree@vger.kernel.org, dri-devel@lists.freedesktop.org,
	linux-media@vger.kernel.org, linux-tegra@vger.kernel.org
Subject: Re: [PATCH v1 11/30] drm/tegra: dc: Support OPP and SoC core voltage scaling
Date: Fri, 13 Nov 2020 17:28:59 +0000	[thread overview]
Message-ID: <20201113172859.GF4828@sirena.org.uk> (raw)
In-Reply-To: <3beaa12b-4a50-a3b6-fc43-ebb5ce7a8db7@gmail.com>

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

On Fri, Nov 13, 2020 at 08:13:49PM +0300, Dmitry Osipenko wrote:
> 13.11.2020 19:15, Mark Brown пишет:

> > My point here is that the driver shouldn't be checking for a dummy
> > regulator, the driver should be checking the features that are provided
> > to it by the regulator and handling those.

> I understand yours point, but then we need dummy regulator to provide
> all the features, and currently it does the opposite.

As could any other regulator?

> > It doesn't matter if this is
> > a dummy regulator or an actual regulator with limited features, the
> > effect is the same and the handling should be the same.  If the driver
> > is doing anything to handle dummy regulators explicitly as dummy
> > regulators it is doing it wrong.

> It matters because dummy regulator errors out all checks and changes
> other than enable/disable, instead of accepting them. If we could add an
> option for dummy regulator to succeed all the checks and accept all the
> values, then it could become more usable.

I'm a bit confused here TBH - I'm not sure I see a substantial
difference between a consumer detecting that it can't set any voltages
at all and the handling for an optional regulator.  Either way if it's
going to carry on and assume that whatever voltage is there works for
everything it boils down to setting a flag saying to skip the set
voltage operation.  I think you are too focused on the specific
implementation you currently have here.

We obviously can't just accept voltage change operations when we've no
idea what the current voltage of the device is.

> > To repeat you should *only* be using regulator_get_optional() in the
> > case where the supply may be physically absent which is not the case
> > here.
> 
> Alright, but then we either need to improve regulator core to make dummy
> regulator a bit more usable, or continue to work around it in drivers.
> What should we do?

As I keep saying the consumer driver should be enumerating the voltages
it can set, if it can't find any and wants to continue then it can just
skip setting voltages later on.  If only some are unavailable then it
probably wants to eliminate those specific OPPs instead.

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