linux-kernel - Re: [PATCH 1/1 v3] pwm_bl: Add support for backlight enable regulator

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: <20130306070019.GA2436@avionic-0098.mockup.avionic-design.de>
Date:	Wed, 6 Mar 2013 08:00:19 +0100
From:	Thierry Reding <thierry.reding@...onic-design.de>
To:	Alex Courbot <acourbot@...dia.com>
Cc:	Andrew Chew <AChew@...dia.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/1 v3] pwm_bl: Add support for backlight enable regulator

On Wed, Mar 06, 2013 at 01:53:27PM +0900, Alex Courbot wrote:
> On 03/06/2013 11:41 AM, Andrew Chew wrote:
> >>>   struct pwm_bl_data {
> >>>   	struct pwm_device	*pwm;
> >>>   	struct device		*dev;
> >>>+	struct regulator	*en_supply;
> >>>+	bool			en_supply_enabled;
> >>
> >>Couldn't you use regulator_is_enabled() and get rid of en_supply_enabled?
> >>It would also ensure the driver performs correctly no matter what the initial
> >>state of the regulator is.
> >
> >Are you sure this works?  I'm concerned about the (bizarre and unlikely) case
> >where this supply is shared with another driver, so I use en_supply_enabled
> >to track the state of the supply such that I can ignore that case.
> 
> You're right, consumers can share regulators and the calls to
> enable/disable need to be balanced. Also there is no way to check
> the intensity of the backlight prior to the change to detect a
> transition, so I guess your approach is indeed the most appropriate
> here.

I think the right thing to do here is just enable the regulator when
the pwm-backlight driver needs it. If it is shared with other devices
they'll have to do the same and the reference counting should only
disable the regulator when there are no users.

Tracking this via platform data won't work because platform data is
statically defined at compile time. So if indeed there was another user
of the regulator it enable/disable the regulator at any time and your
en_supply_enabled would be wrong.

Thierry

Content of type "application/pgp-signature" skipped