| Message ID | 1399325340-20416-1-git-send-email-b.galvani@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
On Tue, May 6, 2014 at 2:59 AM, Beniamino Galvani <b.galvani@gmail.com> wrote: > pwmchip_add() returns zero on success and a negative value on error, > so the condition of the check must be inverted. > > Signed-off-by: Beniamino Galvani <b.galvani@gmail.com> > --- > drivers/pwm/pwm-spear.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/pwm/pwm-spear.c b/drivers/pwm/pwm-spear.c > index cb2d4f0..945556d 100644 > --- a/drivers/pwm/pwm-spear.c > +++ b/drivers/pwm/pwm-spear.c > @@ -222,7 +222,7 @@ static int spear_pwm_probe(struct platform_device *pdev) > } > > ret = pwmchip_add(&pc->chip); > - if (!ret) { > + if (ret < 0) { > clk_unprepare(pc->clk); > dev_err(&pdev->dev, "pwmchip_add() failed: %d\n", ret); > } Acked-by: Viresh Kumar <viresh.kumar@linaro.org> -- 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
On Wed, May 07, 2014 at 11:37:25AM +0530, Viresh Kumar wrote: > On Tue, May 6, 2014 at 2:59 AM, Beniamino Galvani <b.galvani@gmail.com> wrote: > > pwmchip_add() returns zero on success and a negative value on error, > > so the condition of the check must be inverted. > > > > Signed-off-by: Beniamino Galvani <b.galvani@gmail.com> > > --- > > drivers/pwm/pwm-spear.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/pwm/pwm-spear.c b/drivers/pwm/pwm-spear.c > > index cb2d4f0..945556d 100644 > > --- a/drivers/pwm/pwm-spear.c > > +++ b/drivers/pwm/pwm-spear.c > > @@ -222,7 +222,7 @@ static int spear_pwm_probe(struct platform_device *pdev) > > } > > > > ret = pwmchip_add(&pc->chip); > > - if (!ret) { > > + if (ret < 0) { > > clk_unprepare(pc->clk); > > dev_err(&pdev->dev, "pwmchip_add() failed: %d\n", ret); > > } So the current code would run clk_unprepare() on success, but not on failure. Does that cause any harm? Is the device still usable after this? I'm asking because I'm not sure if this is linux-next material or should be a fix for 3.15 (and possibly backported to stable). Thierry
On 7 May 2014 13:54, Thierry Reding <thierry.reding@gmail.com> wrote: > So the current code would run clk_unprepare() on success, but not on > failure. Does that cause any harm? Yeah, that would make the device unusable as clk_enable will always fail. > Is the device still usable after this? Yes, this will fix the bug. > I'm asking because I'm not sure if this is linux-next material > or should be a fix for 3.15 (and possibly backported to stable). IMO, it should be sent for stable: v3.8+ -- 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
On Wed, May 07, 2014 at 02:02:24PM +0530, Viresh Kumar wrote: > On 7 May 2014 13:54, Thierry Reding <thierry.reding@gmail.com> wrote: > > So the current code would run clk_unprepare() on success, but not on > > failure. Does that cause any harm? > > Yeah, that would make the device unusable as clk_enable will always fail. Hmm... so ever since this driver was merged in 3.8 it was completely broken and nobody noticed? Interesting. > > Is the device still usable after this? > > Yes, this will fix the bug. > > > I'm asking because I'm not sure if this is linux-next material > > or should be a fix for 3.15 (and possibly backported to stable). > > IMO, it should be sent for stable: v3.8+ Since this was obviously broken since 3.8 and nobody noticed I wonder if there's really an urgent need for this in stable. Thierry
On 7 May 2014 14:19, Thierry Reding <thierry.reding@gmail.com> wrote: > Since this was obviously broken since 3.8 and nobody noticed I wonder if > there's really an urgent need for this in stable. I don't think there is any urgent need of this, in case somebody wants we can add it later. :) -- 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 --git a/drivers/pwm/pwm-spear.c b/drivers/pwm/pwm-spear.c index cb2d4f0..945556d 100644 --- a/drivers/pwm/pwm-spear.c +++ b/drivers/pwm/pwm-spear.c @@ -222,7 +222,7 @@ static int spear_pwm_probe(struct platform_device *pdev) } ret = pwmchip_add(&pc->chip); - if (!ret) { + if (ret < 0) { clk_unprepare(pc->clk); dev_err(&pdev->dev, "pwmchip_add() failed: %d\n", ret); }
pwmchip_add() returns zero on success and a negative value on error, so the condition of the check must be inverted. Signed-off-by: Beniamino Galvani <b.galvani@gmail.com> --- drivers/pwm/pwm-spear.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)