Re: [PATCH 1/1] ARM: OMAP: board-4430sdp: Provide regulator to pwm-backlight

From: Thierry Reding <thierry.reding@avionic-design.de>
To: Andrew Chew <achew@nvidia.com>
Cc: peter.ujfalusi@ti.com, acourbot@nvidia.com, linux-omap@vger.kernel.org
Subject: Re: [PATCH 1/1] ARM: OMAP: board-4430sdp: Provide regulator to pwm-backlight
Date: Tue, 12 Mar 2013 08:01:04 +0100	[thread overview]
Message-ID: <20130312070104.GB6467@avionic-0098.mockup.avionic-design.de> (raw)
In-Reply-To: <1363053266-515-1-git-send-email-achew@nvidia.com>

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

On Mon, Mar 11, 2013 at 06:54:26PM -0700, Andrew Chew wrote:
> The pwm-backlight driver now takes a mandatory regulator that is gotten
> during driver probe.  Initialize a dummy regulator to satisfy this
> requirement.
> 
> Signed-off-by: Andrew Chew <achew@nvidia.com>
> ---
> This patch, along with many more soon to follow, attempts to satisfy the new
> mandatory regulator that pwm-backlight will grab during probe.  The only
> board in mach-omap2 to use the pwm-backlight appears to be the 4430sdp.
> 
> I thought I'd start small and use this board as an example.
> 
> I tested similar code in my Tegra board, so it should be okay.  Of course,
> I don't have a 4430sdp to test with.

Hi Andrew,

This looks good, one minor comment below. I think it might make sense to
post this patch as part of a series that includes the change to the
pwm-backlight driver. This will make things easier on potential testers.
You can later extend that series to include all the other users and
eventually the whole series can be merged.

>  arch/arm/mach-omap2/board-4430sdp.c |    5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/arch/arm/mach-omap2/board-4430sdp.c b/arch/arm/mach-omap2/board-4430sdp.c
> index 35f3ad0..62022c0 100644
> --- a/arch/arm/mach-omap2/board-4430sdp.c
> +++ b/arch/arm/mach-omap2/board-4430sdp.c
> @@ -291,6 +291,10 @@ static struct platform_device sdp4430_leds_pwm = {
>  	},
>  };
>  
> +/* Dummy regulator for pwm-backlight driver */
> +static struct regulator_consumer_supply backlight_supply =
> +	REGULATOR_SUPPLY("enable", NULL);
> +
>  static struct platform_pwm_backlight_data sdp4430_backlight_data = {
>  	.max_brightness = 127,
>  	.dft_brightness = 127,
> @@ -718,6 +722,7 @@ static void __init omap_4430sdp_init(void)
>  
>  	omap4_i2c_init();
>  	omap_sfh7741prox_init();
> +	regulator_register_always_on(-1, "bl-enable", &backlight_supply, 1, 0);

I think I'd prefer to spell out "backlight-enable". I know it's rather
long but I find it is much easier on the eye to look at the various
outputs where the string is used and see "backlight" instead of "bl".
But it's really up to the OMAP crowd, if everybody else is fine with
"bl-enable" I'll shut up.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]