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 16:15:50 +0000	[thread overview]
Message-ID: <20201113161550.GC4828@sirena.org.uk> (raw)
In-Reply-To: <7f066805-97d9-088f-e89d-a554fe478574@gmail.com>

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

On Fri, Nov 13, 2020 at 06:55:27PM +0300, Dmitry Osipenko wrote:
> 13.11.2020 17:29, Mark Brown пишет:

> > It's not clear if it matters - it's more a policy decision on the part
> > of the driver about what it thinks safe error handling is.  If it's not

> If regulator_get() returns a dummy regulator, then this means that
> regulator isn't specified in a device-tree. But then the only way for a
> consumer driver to check whether regulator is dummy, is to check
> presence of the supply property in a device-tree.

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

> We want to emit error messages when something goes wrong, for example
> when regulator voltage fails to change. It's fine that voltage changes
> are failing for a dummy regulator, but then consumer driver shouldn't
> recognize it as a error condition.

If you're fine with that you should also be fine with any other
regulator for which you failed to enumerate any voltages which you can
set.

> The regulator_get_optional() provides a more consistent and
> straightforward way for consumer drivers to check presence of a physical
> voltage regulator in comparison to dealing with a regulator_get(). The
> dummy regulator is nice to use when there is no need to change
> regulator's voltage, which doesn't work for a dummy regulator.

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.

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