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

From: Alex Courbot
Date: Tue Mar 05 2013 - 23:56:52 EST


On 03/06/2013 01:20 PM, Stephen Warren wrote:
On 03/05/2013 07:18 PM, Alex Courbot wrote:
On 03/06/2013 08:51 AM, Andrew Chew wrote:
The backlight enable regulator is specified in the device tree node for
backlight.

diff --git a/include/linux/pwm_backlight.h

struct platform_pwm_backlight_data {
int pwm_id;
+ struct regulator *en_supply;

You should not have this here. Platform data is supposed to provide the
necessary information for the driver to resolve the resource - not the
resource itself.
...
There is one catch though: in case you don't want to use a regulator,
and thus have none defined, regulator_get() will return -EPROBE_DEFER,
so you cannot distinguish between "no regulator needed" and "supplier
not ready yet" and your driver will always *require* a regulator. So at
the end of the day you might still need a "use_enable_regulator" in the
platform data to explicitly ask for probe() to look for it. This
variable would also be set by parse_dt() if the "enable-supply" property
exists.

A driver that requires a regulator always requires that regulator. If a
particular board doesn't have SW control over the power source, you're
supposed to provide a dummy (fixed) regulator so that the driver doesn't
care about the difference.

That's good to know, thanks. So does this mean that Andrew should make the enable regulator mandatory and update current users to provide a dummy one?

Alex.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/