diff mbox

Revert "backlight: pwm: Handle EPROBE_DEFER while requesting the PWM"

Message ID 1443295482-18687-1-git-send-email-robert.jarzmik@free.fr
State Superseded
Headers show

Commit Message

Robert Jarzmik Sept. 26, 2015, 7:24 p.m. UTC
This reverts commit 68feaca0b13e453aa14ee064c1736202b48b342f.
This commit breaks legacy platforms, for which :
 (a) no pwm table is added (legacy platforms)
 (b) in this case, in pwm_get(), pmw_lookup_list is empty, and therefore
     chosen == NULL, and therefore pwm_get() returns NULL, and pwm_get()
     returns -EPROBE_DEFER
 (c) as a consequence, this code is unreachable in pwm_bl.c :
     if (IS_ERR(pb->pwm)) {
	ret = PTR_ERR(pb->pwm);
 	dev_info(&pdev->dev, "%s:%d(): %d\n", __func__, __LINE__, ret);
 	if (ret == -EPROBE_DEFER)
 		goto err_alloc;

 	dev_err(&pdev->dev, "unable to request PWM, trying legacy API\n");
 	pb->legacy = true;
 	pb->pwm = pwm_request(data->pwm_id, "pwm-backlight");

As this code is unreachable, all legacy platforms relying on pwm_id are
broken, amongst which pxa have been tested as broken.

Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
---
 drivers/video/backlight/pwm_bl.c | 4 ----
 1 file changed, 4 deletions(-)

Comments

Nicolas Ferre Sept. 28, 2015, 8:57 a.m. UTC | #1
Robert Jarzmik <robert.jarzmik <at> free.fr> writes:

> 
> This reverts commit 68feaca0b13e453aa14ee064c1736202b48b342f.
> This commit breaks legacy platforms, for which :
>  (a) no pwm table is added (legacy platforms)
>  (b) in this case, in pwm_get(), pmw_lookup_list is empty, and therefore
>      chosen == NULL, and therefore pwm_get() returns NULL, and pwm_get()
>      returns -EPROBE_DEFER
>  (c) as a consequence, this code is unreachable in pwm_bl.c :
>      if (IS_ERR(pb->pwm)) {
> 	ret = PTR_ERR(pb->pwm);
>  	dev_info(&pdev->dev, "%s:%d(): %d\n", __func__, __LINE__, ret);
>  	if (ret == -EPROBE_DEFER)
>  		goto err_alloc;
> 
>  	dev_err(&pdev->dev, "unable to request PWM, trying legacy API\n");
>  	pb->legacy = true;
>  	pb->pwm = pwm_request(data->pwm_id, "pwm-backlight");
> 
> As this code is unreachable, all legacy platforms relying on pwm_id are
> broken, amongst which pxa have been tested as broken.

Well, why don't you add the needed pwm table to the platforms so that you
comply with the pwm subsystem behaviour.

Otherwise, we may need to test if the platform uses the DT or instantiates
correctly the pwm which may lead to other platforms breaking.

So I would advise to not revert this patch and properly fix the existing
legacy platforms.

> Signed-off-by: Robert Jarzmik <robert.jarzmik <at> free.fr>

So to be clear: NACK for me.
(Can you please add me in CC of the discussion).

Best regards,

> ---
>  drivers/video/backlight/pwm_bl.c | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/drivers/video/backlight/pwm_bl.c
b/drivers/video/backlight/pwm_bl.c
> index eff379b234cc..57cb9ec8be43 100644
> --- a/drivers/video/backlight/pwm_bl.c
> +++ b/drivers/video/backlight/pwm_bl.c
>  <at>  <at>  -272,10 +272,6  <at>  <at>  static int
pwm_backlight_probe(struct platform_device *pdev)
> 
>  	pb->pwm = devm_pwm_get(&pdev->dev, NULL);
>  	if (IS_ERR(pb->pwm)) {
> -		ret = PTR_ERR(pb->pwm);
> -		if (ret == -EPROBE_DEFER)
> -			goto err_alloc;
> -
>  		dev_err(&pdev->dev, "unable to request PWM, trying legacy API\n");
>  		pb->legacy = true;
>  		pb->pwm = pwm_request(data->pwm_id, "pwm-backlight");




--
To unsubscribe from this list: send the line "unsubscribe linux-pwm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Robert Jarzmik Sept. 30, 2015, 7:29 p.m. UTC | #2
Robert Jarzmik <robert.jarzmik@free.fr> writes:

> This reverts commit 68feaca0b13e453aa14ee064c1736202b48b342f.
> This commit breaks legacy platforms, for which :
>  (a) no pwm table is added (legacy platforms)
>  (b) in this case, in pwm_get(), pmw_lookup_list is empty, and therefore
>      chosen == NULL, and therefore pwm_get() returns NULL, and pwm_get()
>      returns -EPROBE_DEFER
>  (c) as a consequence, this code is unreachable in pwm_bl.c :
>      if (IS_ERR(pb->pwm)) {
> 	ret = PTR_ERR(pb->pwm);
>  	dev_info(&pdev->dev, "%s:%d(): %d\n", __func__, __LINE__, ret);
>  	if (ret == -EPROBE_DEFER)
>  		goto err_alloc;
>
>  	dev_err(&pdev->dev, "unable to request PWM, trying legacy API\n");
>  	pb->legacy = true;
>  	pb->pwm = pwm_request(data->pwm_id, "pwm-backlight");
>
> As this code is unreachable, all legacy platforms relying on pwm_id are
> broken, amongst which pxa have been tested as broken.
>
> Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
Thierry, would you have a look please ?
As I said before, all legacy platform relying on pwm_id are broken. I'd like to
be sure this lands in the next -rc series.

Cheers.

--
Robert
--
To unsubscribe from this list: send the line "unsubscribe linux-pwm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
index eff379b234cc..57cb9ec8be43 100644
--- a/drivers/video/backlight/pwm_bl.c
+++ b/drivers/video/backlight/pwm_bl.c
@@ -272,10 +272,6 @@  static int pwm_backlight_probe(struct platform_device *pdev)
 
 	pb->pwm = devm_pwm_get(&pdev->dev, NULL);
 	if (IS_ERR(pb->pwm)) {
-		ret = PTR_ERR(pb->pwm);
-		if (ret == -EPROBE_DEFER)
-			goto err_alloc;
-
 		dev_err(&pdev->dev, "unable to request PWM, trying legacy API\n");
 		pb->legacy = true;
 		pb->pwm = pwm_request(data->pwm_id, "pwm-backlight");