Re: [PATCH 1/2] backlight: pwm: don't call legacy pwm request for device defined in dt

[Thierry Reding <thierry.reding@gmail.com>]

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

On Sat, Oct 11, 2014 at 04:46:25PM +0300, Vladimir Zapolskiy wrote:
> Platform PWM backlight data provided by board's device tree should be
> complete enough to successfully request a pwm device using pwm_get() API.
> 
> Based on initial implementation done by Dmitry Eremin-Solenikov.
> 
> Reported-by: Dmitry Eremin-Solenikov <dmitry_eremin@mentor.com>
> Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
> Cc: Thierry Reding <thierry.reding@gmail.com>
> Cc: Jingoo Han <jg1.han@samsung.com>
> Cc: Bryan Wu <cooloney@gmail.com>
> Cc: Lee Jones <lee.jones@linaro.org>
> ---
>  drivers/video/backlight/pwm_bl.c |   14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)

This fell off my radar, but I think it's good. I used to have a local
patch somewhere that solved the same problem by initializing the pwm_id
field of platform_pwm_backlight_data to -1 in pwm_backlight_parse_dt(),
but I like this variant better because it's more explicit and doesn't
even attempt to request using the legacy API (which will inevitably fail
in the DT case anyway).

Vladimir, do you think you'd have the time to rebase this patch on top
of something recent and perhaps extend the commit message with some of
the arguments that you brought forth in this thread? Specifically it'd
be useful to mention that this enforces the DT binding and fixes a real
bug where the legacy path would try to request a PWM that's not
necessarily the right one.

Thierry

> diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
> index cb5ae4c..dd7aaf7 100644
> --- a/drivers/video/backlight/pwm_bl.c
> +++ b/drivers/video/backlight/pwm_bl.c
> @@ -272,15 +272,15 @@ static int pwm_backlight_probe(struct platform_device *pdev)
>  	}
>  
>  	pb->pwm = devm_pwm_get(&pdev->dev, NULL);
> -	if (IS_ERR(pb->pwm)) {
> +	if (IS_ERR(pb->pwm) && !pdev->dev.of_node) {
>  		dev_err(&pdev->dev, "unable to request PWM, trying legacy API\n");
> -
>  		pb->pwm = pwm_request(data->pwm_id, "pwm-backlight");
> -		if (IS_ERR(pb->pwm)) {
> -			dev_err(&pdev->dev, "unable to request legacy PWM\n");
> -			ret = PTR_ERR(pb->pwm);
> -			goto err_alloc;
> -		}
> +	}
> +
> +	if (IS_ERR(pb->pwm)) {
> +		dev_err(&pdev->dev, "unable to request PWM\n");
> +		ret = PTR_ERR(pb->pwm);
> +		goto err_alloc;
>  	}
>  
>  	dev_dbg(&pdev->dev, "got pwm for backlight\n");
> -- 
> 1.7.10.4
> 

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