linux-kernel - Re: [PATCH 09/10] pwm-backlight: Use an optional power supply

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20131001213120.GG9201@ulmo.nvidia.com>
Date:	Tue, 1 Oct 2013 23:31:27 +0200
From:	Thierry Reding <thierry.reding@...il.com>
To:	Stephen Warren <swarren@...dotorg.org>
Cc:	Rob Herring <rob.herring@...xeda.com>,
	Pawel Moll <pawel.moll@....com>,
	Mark Rutland <mark.rutland@....com>,
	Ian Campbell <ijc+devicetree@...lion.org.uk>,
	Tony Lindgren <tony@...mide.com>,
	Eric Miao <eric.y.miao@...il.com>,
	Haojian Zhuang <haojian.zhuang@...il.com>,
	Ben Dooks <ben-linux@...ff.org>,
	Kukjin Kim <kgene.kim@...sung.com>,
	Simon Horman <horms@...ge.net.au>,
	Magnus Damm <magnus.damm@...il.com>,
	Guan Xuetao <gxt@...c.pku.edu.cn>, devicetree@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org, linux-omap@...r.kernel.org,
	openezx-devel@...ts.openezx.org, linux-samsung-soc@...r.kernel.org,
	linux-sh@...r.kernel.org, linux-pwm@...r.kernel.org,
	linux-kernel@...r.kernel.org, Mark Brown <broonie@...nel.org>
Subject: Re: [PATCH 09/10] pwm-backlight: Use an optional power supply

On Tue, Oct 01, 2013 at 02:59:43PM -0600, Stephen Warren wrote:
> On 10/01/2013 02:53 PM, Thierry Reding wrote:
> > On Tue, Oct 01, 2013 at 12:43:57PM -0600, Stephen Warren wrote:
> >> On 09/23/2013 03:41 PM, Thierry Reding wrote:
> >>> Many backlights require a power supply to work properly. This
> >>> commit uses a power-supply regulator, if available, to power up
> >>> and power down the panel.
> >> 
> >> I think that all backlights require a power supply, albeit the
> >> supply may not be SW-controllable. Hence, shouldn't the regulator
> >> be mandatory in the binding, yet the driver be defensively coded
> >> such that if one isn't specified, the driver continues to work?
> > 
> > That has already changed in my local version of this patch.
> > 
> >>> diff --git a/drivers/video/backlight/pwm_bl.c
> >>> b/drivers/video/backlight/pwm_bl.c
> >> 
> >>> @@ -253,6 +264,16 @@ static int pwm_backlight_probe(struct
> >>> platform_device *pdev) } }
> >>> 
> >>> +	pb->power_supply = devm_regulator_get_optional(&pdev->dev,
> >>> "power");
> >> 
> >> ... so I think that should be devm_regulator_get(), since the
> >> regulator isn't really optional.
> >> 
> >>> +	if (IS_ERR(pb->power_supply)) { +		if
> >>> (PTR_ERR(pb->power_supply) != -ENODEV) { +			ret =
> >>> PTR_ERR(pb->power_supply); +			goto err_gpio; +		} + +
> >>> pb->power_supply = NULL;
> >> 
> >> If devm_regulator_get_optional() returns an error value or a
> >> valid value, then I don't think that this driver should transmute
> >> error values into NULL; NULL might be a perfectly valid regulator
> >> value. Related, I think the if (pb->power_supply) tests should be
> >> replaced with if (!IS_ERR(pb->power_supply)) instead.
> > 
> > All of that is already done in my local tree. This actually turns
> > out to work rather smoothly with the new support for optional
> > regulators. The regulator core will give you a dummy regulator
> > (assuming it's there physically but hasn't been wired up in
> > software) that's always on, so the driver doesn't even have to
> > special case it anymore.
> 
> OK, hopefully it (the regulator core) complains about the missing DT
> property though; I assume you're using regulator_get() not
> regulator_get_optional(), since the supply really is not optional.

It doesn't always. There's a pr_warn() in _regulator_get(), but that's
only for non-DT (since for DT, has_full_constraints is set to true in
regulator_init_complete()). Actually that would mean that the regulator
core will complain as long as init isn't complete. But since, like many
other drivers, pwm-backlight could use deferred probing it's likely to
end up being probed after init.

Cc'ing Mark Brown.

Thierry

Content of type "application/pgp-signature" skipped