All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] pwm: sti: Avoid conditional gotos
@ 2020-11-11 19:14 Thierry Reding
  2020-11-11 19:14 ` [PATCH 2/2] pwm: sti: Remove unnecessary blank line Thierry Reding
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Thierry Reding @ 2020-11-11 19:14 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Uwe Kleine-König, Lee Jones, Ajit Pal Singh, linux-pwm

Using gotos for conditional code complicates this code significantly.
Convert the code to simple conditional blocks to increase readability.

Suggested-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Signed-off-by: Thierry Reding <thierry.reding@gmail.com>
---
 drivers/pwm/pwm-sti.c | 53 ++++++++++++++++++++-----------------------
 1 file changed, 24 insertions(+), 29 deletions(-)

diff --git a/drivers/pwm/pwm-sti.c b/drivers/pwm/pwm-sti.c
index a1392255955f..c6c82724d327 100644
--- a/drivers/pwm/pwm-sti.c
+++ b/drivers/pwm/pwm-sti.c
@@ -590,48 +590,43 @@ static int sti_pwm_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	if (!cdata->pwm_num_devs)
-		goto skip_pwm;
-
-	pc->pwm_clk = of_clk_get_by_name(dev->of_node, "pwm");
-	if (IS_ERR(pc->pwm_clk)) {
-		dev_err(dev, "failed to get PWM clock\n");
-		return PTR_ERR(pc->pwm_clk);
-	}
+	if (cdata->pwm_num_devs) {
+		pc->pwm_clk = of_clk_get_by_name(dev->of_node, "pwm");
+		if (IS_ERR(pc->pwm_clk)) {
+			dev_err(dev, "failed to get PWM clock\n");
+			return PTR_ERR(pc->pwm_clk);
+		}
 
-	ret = clk_prepare(pc->pwm_clk);
-	if (ret) {
-		dev_err(dev, "failed to prepare clock\n");
-		goto put_pwm_clk;
+		ret = clk_prepare(pc->pwm_clk);
+		if (ret) {
+			dev_err(dev, "failed to prepare clock\n");
+			goto put_pwm_clk;
+		}
 	}
 
-skip_pwm:
-	if (!cdata->cpt_num_devs)
-		goto skip_cpt;
-
-	pc->cpt_clk = of_clk_get_by_name(dev->of_node, "capture");
-	if (IS_ERR(pc->cpt_clk)) {
-		dev_err(dev, "failed to get PWM capture clock\n");
-		ret = PTR_ERR(pc->cpt_clk);
-		goto unprepare_pwm_clk;
-	}
+	if (cdata->cpt_num_devs) {
+		pc->cpt_clk = of_clk_get_by_name(dev->of_node, "capture");
+		if (IS_ERR(pc->cpt_clk)) {
+			dev_err(dev, "failed to get PWM capture clock\n");
+			ret = PTR_ERR(pc->cpt_clk);
+			goto unprepare_pwm_clk;
+		}
 
-	ret = clk_prepare(pc->cpt_clk);
-	if (ret) {
-		dev_err(dev, "failed to prepare clock\n");
-		goto put_cpt_clk;
+		ret = clk_prepare(pc->cpt_clk);
+		if (ret) {
+			dev_err(dev, "failed to prepare clock\n");
+			goto put_cpt_clk;
+		}
 	}
 
-skip_cpt:
 	pc->chip.dev = dev;
 	pc->chip.ops = &sti_pwm_ops;
 	pc->chip.base = -1;
 	pc->chip.npwm = pc->cdata->pwm_num_devs;
 
 	ret = pwmchip_add(&pc->chip);
-	if (ret < 0) {
+	if (ret < 0)
 		goto unprepare_cpt_clk;
-	}
 
 	for (i = 0; i < cdata->cpt_num_devs; i++) {
 		struct sti_cpt_ddata *ddata;
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/2] pwm: sti: Remove unnecessary blank line
  2020-11-11 19:14 [PATCH 1/2] pwm: sti: Avoid conditional gotos Thierry Reding
@ 2020-11-11 19:14 ` Thierry Reding
  2020-11-12  8:59   ` Lee Jones
  2020-11-12 11:04   ` Uwe Kleine-König
  2020-11-12  7:56 ` [PATCH 1/2] pwm: sti: Avoid conditional gotos Uwe Kleine-König
  2020-11-12  8:59 ` Lee Jones
  2 siblings, 2 replies; 6+ messages in thread
From: Thierry Reding @ 2020-11-11 19:14 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Uwe Kleine-König, Lee Jones, Ajit Pal Singh, linux-pwm

A single blank line is enough to separate logical code blocks.

Signed-off-by: Thierry Reding <thierry.reding@gmail.com>
---
 drivers/pwm/pwm-sti.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/pwm/pwm-sti.c b/drivers/pwm/pwm-sti.c
index c6c82724d327..ec667b31d7ec 100644
--- a/drivers/pwm/pwm-sti.c
+++ b/drivers/pwm/pwm-sti.c
@@ -505,7 +505,6 @@ static int sti_pwm_probe_dt(struct sti_pwm_chip *pc)
 	if (IS_ERR(pc->prescale_high))
 		return PTR_ERR(pc->prescale_high);
 
-
 	pc->pwm_out_en = devm_regmap_field_alloc(dev, pc->regmap,
 						 reg_fields[PWM_OUT_EN]);
 	if (IS_ERR(pc->pwm_out_en))
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] pwm: sti: Avoid conditional gotos
  2020-11-11 19:14 [PATCH 1/2] pwm: sti: Avoid conditional gotos Thierry Reding
  2020-11-11 19:14 ` [PATCH 2/2] pwm: sti: Remove unnecessary blank line Thierry Reding
@ 2020-11-12  7:56 ` Uwe Kleine-König
  2020-11-12  8:59 ` Lee Jones
  2 siblings, 0 replies; 6+ messages in thread
From: Uwe Kleine-König @ 2020-11-12  7:56 UTC (permalink / raw)
  To: Thierry Reding; +Cc: Lee Jones, Ajit Pal Singh, linux-pwm

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

Hello Thierry,

On Wed, Nov 11, 2020 at 08:14:48PM +0100, Thierry Reding wrote:
> Using gotos for conditional code complicates this code significantly.
> Convert the code to simple conditional blocks to increase readability.
> 
> Suggested-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> Signed-off-by: Thierry Reding <thierry.reding@gmail.com>

Great you picked that up, thanks.

Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] pwm: sti: Avoid conditional gotos
  2020-11-11 19:14 [PATCH 1/2] pwm: sti: Avoid conditional gotos Thierry Reding
  2020-11-11 19:14 ` [PATCH 2/2] pwm: sti: Remove unnecessary blank line Thierry Reding
  2020-11-12  7:56 ` [PATCH 1/2] pwm: sti: Avoid conditional gotos Uwe Kleine-König
@ 2020-11-12  8:59 ` Lee Jones
  2 siblings, 0 replies; 6+ messages in thread
From: Lee Jones @ 2020-11-12  8:59 UTC (permalink / raw)
  To: Thierry Reding; +Cc: Uwe Kleine-König, Ajit Pal Singh, linux-pwm

On Wed, 11 Nov 2020, Thierry Reding wrote:

> Using gotos for conditional code complicates this code significantly.
> Convert the code to simple conditional blocks to increase readability.
> 
> Suggested-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> Signed-off-by: Thierry Reding <thierry.reding@gmail.com>
> ---
>  drivers/pwm/pwm-sti.c | 53 ++++++++++++++++++++-----------------------
>  1 file changed, 24 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/pwm/pwm-sti.c b/drivers/pwm/pwm-sti.c
> index a1392255955f..c6c82724d327 100644
> --- a/drivers/pwm/pwm-sti.c
> +++ b/drivers/pwm/pwm-sti.c
> @@ -590,48 +590,43 @@ static int sti_pwm_probe(struct platform_device *pdev)
>  	if (ret)
>  		return ret;
>  
> -	if (!cdata->pwm_num_devs)
> -		goto skip_pwm;
> -
> -	pc->pwm_clk = of_clk_get_by_name(dev->of_node, "pwm");
> -	if (IS_ERR(pc->pwm_clk)) {
> -		dev_err(dev, "failed to get PWM clock\n");
> -		return PTR_ERR(pc->pwm_clk);
> -	}
> +	if (cdata->pwm_num_devs) {
> +		pc->pwm_clk = of_clk_get_by_name(dev->of_node, "pwm");
> +		if (IS_ERR(pc->pwm_clk)) {
> +			dev_err(dev, "failed to get PWM clock\n");
> +			return PTR_ERR(pc->pwm_clk);
> +		}
>  
> -	ret = clk_prepare(pc->pwm_clk);
> -	if (ret) {
> -		dev_err(dev, "failed to prepare clock\n");
> -		goto put_pwm_clk;
> +		ret = clk_prepare(pc->pwm_clk);
> +		if (ret) {
> +			dev_err(dev, "failed to prepare clock\n");
> +			goto put_pwm_clk;
> +		}
>  	}
>  
> -skip_pwm:
> -	if (!cdata->cpt_num_devs)
> -		goto skip_cpt;
> -
> -	pc->cpt_clk = of_clk_get_by_name(dev->of_node, "capture");
> -	if (IS_ERR(pc->cpt_clk)) {
> -		dev_err(dev, "failed to get PWM capture clock\n");
> -		ret = PTR_ERR(pc->cpt_clk);
> -		goto unprepare_pwm_clk;
> -	}
> +	if (cdata->cpt_num_devs) {
> +		pc->cpt_clk = of_clk_get_by_name(dev->of_node, "capture");
> +		if (IS_ERR(pc->cpt_clk)) {
> +			dev_err(dev, "failed to get PWM capture clock\n");
> +			ret = PTR_ERR(pc->cpt_clk);
> +			goto unprepare_pwm_clk;
> +		}
>  
> -	ret = clk_prepare(pc->cpt_clk);
> -	if (ret) {
> -		dev_err(dev, "failed to prepare clock\n");
> -		goto put_cpt_clk;
> +		ret = clk_prepare(pc->cpt_clk);
> +		if (ret) {
> +			dev_err(dev, "failed to prepare clock\n");
> +			goto put_cpt_clk;
> +		}
>  	}
>  
> -skip_cpt:
>  	pc->chip.dev = dev;
>  	pc->chip.ops = &sti_pwm_ops;
>  	pc->chip.base = -1;
>  	pc->chip.npwm = pc->cdata->pwm_num_devs;
>  
>  	ret = pwmchip_add(&pc->chip);
> -	if (ret < 0) {
> +	if (ret < 0)
>  		goto unprepare_cpt_clk;
> -	}

Sneaky!

Acked-by: Lee Jones <lee.jones@linaro.org>

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] pwm: sti: Remove unnecessary blank line
  2020-11-11 19:14 ` [PATCH 2/2] pwm: sti: Remove unnecessary blank line Thierry Reding
@ 2020-11-12  8:59   ` Lee Jones
  2020-11-12 11:04   ` Uwe Kleine-König
  1 sibling, 0 replies; 6+ messages in thread
From: Lee Jones @ 2020-11-12  8:59 UTC (permalink / raw)
  To: Thierry Reding; +Cc: Uwe Kleine-König, Ajit Pal Singh, linux-pwm

On Wed, 11 Nov 2020, Thierry Reding wrote:

> A single blank line is enough to separate logical code blocks.
> 
> Signed-off-by: Thierry Reding <thierry.reding@gmail.com>
> ---
>  drivers/pwm/pwm-sti.c | 1 -
>  1 file changed, 1 deletion(-)

Acked-by: Lee Jones <lee.jones@linaro.org>

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] pwm: sti: Remove unnecessary blank line
  2020-11-11 19:14 ` [PATCH 2/2] pwm: sti: Remove unnecessary blank line Thierry Reding
  2020-11-12  8:59   ` Lee Jones
@ 2020-11-12 11:04   ` Uwe Kleine-König
  1 sibling, 0 replies; 6+ messages in thread
From: Uwe Kleine-König @ 2020-11-12 11:04 UTC (permalink / raw)
  To: Thierry Reding; +Cc: Lee Jones, linux-pwm

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

[dropped Ajit Pal Singh from Cc:]

On Wed, Nov 11, 2020 at 08:14:49PM +0100, Thierry Reding wrote:
> A single blank line is enough to separate logical code blocks.
> 
> Signed-off-by: Thierry Reding <thierry.reding@gmail.com>

I would consider this too minor to create a patch for, but as you
already invested the time:

Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Thanks
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-11-12 11:05 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-11 19:14 [PATCH 1/2] pwm: sti: Avoid conditional gotos Thierry Reding
2020-11-11 19:14 ` [PATCH 2/2] pwm: sti: Remove unnecessary blank line Thierry Reding
2020-11-12  8:59   ` Lee Jones
2020-11-12 11:04   ` Uwe Kleine-König
2020-11-12  7:56 ` [PATCH 1/2] pwm: sti: Avoid conditional gotos Uwe Kleine-König
2020-11-12  8:59 ` Lee Jones

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.