Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1298554 > unrolled thread

[PATCH] [media] tuners: One check less in m88rs6000t_get_rf_strength() after error detection

Started bySF Markus Elfring <elfring@users.sourceforge.net>
First post2015-12-28 10:20 +0100
Last post2015-12-28 15:50 +0100
Articles 10 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  [PATCH] [media] tuners: One check less in  m88rs6000t_get_rf_strength() after error detection SF Markus Elfring <elfring@users.sourceforge.net> - 2015-12-28 10:20 +0100
    Re: [PATCH] [media] tuners: One check less in m88rs6000t_get_rf_strength()  after error detection Julia Lawall <julia.lawall@lip6.fr> - 2015-12-28 10:30 +0100
      Re: [media] tuners: One check less in m88rs6000t_get_rf_strength()  after error detection SF Markus Elfring <elfring@users.sourceforge.net> - 2015-12-28 11:40 +0100
        Re: [media] tuners: One check less in m88rs6000t_get_rf_strength()  after error detection Julia Lawall <julia.lawall@lip6.fr> - 2015-12-28 11:40 +0100
          [PATCH 0/2] [media] m88rs6000t: Fine-tuning for some function  implementations SF Markus Elfring <elfring@users.sourceforge.net> - 2015-12-28 15:40 +0100
            [PATCH 1/2] [media] m88rs6000t: Better exception handling in five  functions SF Markus Elfring <elfring@users.sourceforge.net> - 2015-12-28 15:40 +0100
              Re: [PATCH 1/2] [media] m88rs6000t: Better exception handling in  five functions Julia Lawall <julia.lawall@lip6.fr> - 2015-12-28 15:50 +0100
                Re: [media] m88rs6000t: Better exception handling in five functions SF Markus Elfring <elfring@users.sourceforge.net> - 2015-12-28 16:10 +0100
                  Re: [media] m88rs6000t: Better exception handling in five  functions Julia Lawall <julia.lawall@lip6.fr> - 2015-12-28 16:20 +0100
            [PATCH 2/2] [media] tuners: Refactoring for m88rs6000t_sleep() SF Markus Elfring <elfring@users.sourceforge.net> - 2015-12-28 15:50 +0100

#1298554 — [PATCH] [media] tuners: One check less in m88rs6000t_get_rf_strength() after error detection

FromSF Markus Elfring <elfring@users.sourceforge.net>
Date2015-12-28 10:20 +0100
Subject[PATCH] [media] tuners: One check less in m88rs6000t_get_rf_strength() after error detection
Message-ID<qKC89-8ir-5@gated-at.bofh.it>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 28 Dec 2015 10:10:34 +0100

This issue was detected by using the Coccinelle software.

Move the jump label directly before the desired log statement
so that the variable "ret" will not be checked once more
after it was determined that a function call failed.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/media/tuners/m88rs6000t.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/media/tuners/m88rs6000t.c b/drivers/media/tuners/m88rs6000t.c
index 504bfbc..b45594e 100644
--- a/drivers/media/tuners/m88rs6000t.c
+++ b/drivers/media/tuners/m88rs6000t.c
@@ -510,27 +510,27 @@ static int m88rs6000t_get_rf_strength(struct dvb_frontend *fe, u16 *strength)
 
 	ret = regmap_read(dev->regmap, 0x5A, &val);
 	if (ret)
-		goto err;
+		goto report_failure;
 	RF_GC = val & 0x0f;
 
 	ret = regmap_read(dev->regmap, 0x5F, &val);
 	if (ret)
-		goto err;
+		goto report_failure;
 	IF_GC = val & 0x0f;
 
 	ret = regmap_read(dev->regmap, 0x3F, &val);
 	if (ret)
-		goto err;
+		goto report_failure;
 	TIA_GC = (val >> 4) & 0x07;
 
 	ret = regmap_read(dev->regmap, 0x77, &val);
 	if (ret)
-		goto err;
+		goto report_failure;
 	BB_GC = (val >> 4) & 0x0f;
 
 	ret = regmap_read(dev->regmap, 0x76, &val);
 	if (ret)
-		goto err;
+		goto report_failure;
 	PGA2_GC = val & 0x3f;
 	PGA2_cri = PGA2_GC >> 2;
 	PGA2_crf = PGA2_GC & 0x03;
@@ -562,9 +562,11 @@ static int m88rs6000t_get_rf_strength(struct dvb_frontend *fe, u16 *strength)
 	/* scale value to 0x0000-0xffff */
 	gain = clamp_val(gain, 1000U, 10500U);
 	*strength = (10500 - gain) * 0xffff / (10500 - 1000);
-err:
-	if (ret)
+
+	if (ret) {
+report_failure:
 		dev_dbg(&dev->client->dev, "failed=%d\n", ret);
+	}
 	return ret;
 }
 
-- 
2.6.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]


#1298556 — Re: [PATCH] [media] tuners: One check less in m88rs6000t_get_rf_strength() after error detection

FromJulia Lawall <julia.lawall@lip6.fr>
Date2015-12-28 10:30 +0100
SubjectRe: [PATCH] [media] tuners: One check less in m88rs6000t_get_rf_strength() after error detection
Message-ID<qKChP-8lQ-3@gated-at.bofh.it>
In reply to#1298554
On Mon, 28 Dec 2015, SF Markus Elfring wrote:

> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Mon, 28 Dec 2015 10:10:34 +0100
>
> This issue was detected by using the Coccinelle software.
>
> Move the jump label directly before the desired log statement
> so that the variable "ret" will not be checked once more
> after it was determined that a function call failed.

Why not avoid both unnecessary ifs and the enormous ugliness of a label
inside an if by making two returns: a return 0 for success and a dev_dbg
and return ret for failure?

julia


> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/media/tuners/m88rs6000t.c | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/media/tuners/m88rs6000t.c b/drivers/media/tuners/m88rs6000t.c
> index 504bfbc..b45594e 100644
> --- a/drivers/media/tuners/m88rs6000t.c
> +++ b/drivers/media/tuners/m88rs6000t.c
> @@ -510,27 +510,27 @@ static int m88rs6000t_get_rf_strength(struct dvb_frontend *fe, u16 *strength)
>
>  	ret = regmap_read(dev->regmap, 0x5A, &val);
>  	if (ret)
> -		goto err;
> +		goto report_failure;
>  	RF_GC = val & 0x0f;
>
>  	ret = regmap_read(dev->regmap, 0x5F, &val);
>  	if (ret)
> -		goto err;
> +		goto report_failure;
>  	IF_GC = val & 0x0f;
>
>  	ret = regmap_read(dev->regmap, 0x3F, &val);
>  	if (ret)
> -		goto err;
> +		goto report_failure;
>  	TIA_GC = (val >> 4) & 0x07;
>
>  	ret = regmap_read(dev->regmap, 0x77, &val);
>  	if (ret)
> -		goto err;
> +		goto report_failure;
>  	BB_GC = (val >> 4) & 0x0f;
>
>  	ret = regmap_read(dev->regmap, 0x76, &val);
>  	if (ret)
> -		goto err;
> +		goto report_failure;
>  	PGA2_GC = val & 0x3f;
>  	PGA2_cri = PGA2_GC >> 2;
>  	PGA2_crf = PGA2_GC & 0x03;
> @@ -562,9 +562,11 @@ static int m88rs6000t_get_rf_strength(struct dvb_frontend *fe, u16 *strength)
>  	/* scale value to 0x0000-0xffff */
>  	gain = clamp_val(gain, 1000U, 10500U);
>  	*strength = (10500 - gain) * 0xffff / (10500 - 1000);
> -err:
> -	if (ret)
> +
> +	if (ret) {
> +report_failure:
>  		dev_dbg(&dev->client->dev, "failed=%d\n", ret);
> +	}
>  	return ret;
>  }
>
> --
> 2.6.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1298602 — Re: [media] tuners: One check less in m88rs6000t_get_rf_strength() after error detection

FromSF Markus Elfring <elfring@users.sourceforge.net>
Date2015-12-28 11:40 +0100
SubjectRe: [media] tuners: One check less in m88rs6000t_get_rf_strength() after error detection
Message-ID<qKDnz-AY-1@gated-at.bofh.it>
In reply to#1298556
>> Move the jump label directly before the desired log statement
>> so that the variable "ret" will not be checked once more
>> after it was determined that a function call failed.
> 
> Why not avoid both unnecessary ifs

I would find such a fine-tuning also nice in principle at more source code places.


> and the enormous ugliness of a label inside an if by making two returns:
> a return 0 for success and a dev_dbg and return ret for failure?

How should your suggestion finally work when the desired execution success
can be determined for such functions only after several other calls succeeded?

Is consistent checking of failure predicates usually required?

Regards,
Markus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1298603 — Re: [media] tuners: One check less in m88rs6000t_get_rf_strength() after error detection

FromJulia Lawall <julia.lawall@lip6.fr>
Date2015-12-28 11:40 +0100
SubjectRe: [media] tuners: One check less in m88rs6000t_get_rf_strength() after error detection
Message-ID<qKDnA-AY-5@gated-at.bofh.it>
In reply to#1298602

On Mon, 28 Dec 2015, SF Markus Elfring wrote:

> >> Move the jump label directly before the desired log statement
> >> so that the variable "ret" will not be checked once more
> >> after it was determined that a function call failed.
> >
> > Why not avoid both unnecessary ifs
>
> I would find such a fine-tuning also nice in principle at more source code places.
>
>
> > and the enormous ugliness of a label inside an if by making two returns:
> > a return 0 for success and a dev_dbg and return ret for failure?
>
> How should your suggestion finally work when the desired execution success
> can be determined for such functions only after several other calls succeeded?

Not idea what this means, but immediate return 0 followed by various code
for reacting to an error is very common, so it looks like it should be
possible here.

julia
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1298658 — [PATCH 0/2] [media] m88rs6000t: Fine-tuning for some function implementations

FromSF Markus Elfring <elfring@users.sourceforge.net>
Date2015-12-28 15:40 +0100
Subject[PATCH 0/2] [media] m88rs6000t: Fine-tuning for some function implementations
Message-ID<qKH7Q-3lo-1@gated-at.bofh.it>
In reply to#1298603
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 28 Dec 2015 15:32:20 +0100

A few update suggestions were taken into account
from static source code analysis.

Markus Elfring (2):
  Better exception handling in five functions
  Refactoring for m88rs6000t_sleep()

 drivers/media/tuners/m88rs6000t.c | 165 +++++++++++++++++++-------------------
 1 file changed, 83 insertions(+), 82 deletions(-)

-- 
2.6.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1298659 — [PATCH 1/2] [media] m88rs6000t: Better exception handling in five functions

FromSF Markus Elfring <elfring@users.sourceforge.net>
Date2015-12-28 15:40 +0100
Subject[PATCH 1/2] [media] m88rs6000t: Better exception handling in five functions
Message-ID<qKH7Q-3lo-7@gated-at.bofh.it>
In reply to#1298658
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 28 Dec 2015 15:10:30 +0100

This issue was detected by using the Coccinelle software.

Move the jump label directly before the desired log statement
so that the variable "ret" will not be checked once more
after a function call.
Use the identifier "report_failure" instead of "err".

Suggested-by: Julia Lawall <julia.lawall@lip6.fr>
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/media/tuners/m88rs6000t.c | 154 +++++++++++++++++++-------------------
 1 file changed, 78 insertions(+), 76 deletions(-)

diff --git a/drivers/media/tuners/m88rs6000t.c b/drivers/media/tuners/m88rs6000t.c
index 504bfbc..7e59a9f 100644
--- a/drivers/media/tuners/m88rs6000t.c
+++ b/drivers/media/tuners/m88rs6000t.c
@@ -44,7 +44,7 @@ static int m88rs6000t_set_demod_mclk(struct dvb_frontend *fe)
 	/* select demod main mclk */
 	ret = regmap_read(dev->regmap, 0x15, &utmp);
 	if (ret)
-		goto err;
+		goto report_failure;
 	reg15 = utmp;
 	if (c->symbol_rate > 45010000) {
 		reg11 = 0x0E;
@@ -106,7 +106,7 @@ static int m88rs6000t_set_demod_mclk(struct dvb_frontend *fe)
 
 	ret = regmap_read(dev->regmap, 0x1D, &utmp);
 	if (ret)
-		goto err;
+		goto report_failure;
 	reg1D = utmp;
 	reg1D &= ~0x03;
 	reg1D |= N - 1;
@@ -116,42 +116,42 @@ static int m88rs6000t_set_demod_mclk(struct dvb_frontend *fe)
 	/* program and recalibrate demod PLL */
 	ret = regmap_write(dev->regmap, 0x05, 0x40);
 	if (ret)
-		goto err;
+		goto report_failure;
 	ret = regmap_write(dev->regmap, 0x11, 0x08);
 	if (ret)
-		goto err;
+		goto report_failure;
 	ret = regmap_write(dev->regmap, 0x15, reg15);
 	if (ret)
-		goto err;
+		goto report_failure;
 	ret = regmap_write(dev->regmap, 0x16, reg16);
 	if (ret)
-		goto err;
+		goto report_failure;
 	ret = regmap_write(dev->regmap, 0x1D, reg1D);
 	if (ret)
-		goto err;
+		goto report_failure;
 	ret = regmap_write(dev->regmap, 0x1E, reg1E);
 	if (ret)
-		goto err;
+		goto report_failure;
 	ret = regmap_write(dev->regmap, 0x1F, reg1F);
 	if (ret)
-		goto err;
+		goto report_failure;
 	ret = regmap_write(dev->regmap, 0x17, 0xc1);
 	if (ret)
-		goto err;
+		goto report_failure;
 	ret = regmap_write(dev->regmap, 0x17, 0x81);
 	if (ret)
-		goto err;
+		goto report_failure;
 	usleep_range(5000, 50000);
 	ret = regmap_write(dev->regmap, 0x05, 0x00);
 	if (ret)
-		goto err;
+		goto report_failure;
 	ret = regmap_write(dev->regmap, 0x11, reg11);
 	if (ret)
-		goto err;
+		goto report_failure;
 	usleep_range(5000, 50000);
-err:
-	if (ret)
-		dev_dbg(&dev->client->dev, "failed=%d\n", ret);
+	return 0;
+report_failure:
+	dev_dbg(&dev->client->dev, "failed=%d\n", ret);
 	return ret;
 }
 
@@ -169,13 +169,13 @@ static int m88rs6000t_set_pll_freq(struct m88rs6000t_dev *dev,
 
 	ret = regmap_write(dev->regmap, 0x36, (refDiv - 8));
 	if (ret)
-		goto err;
+		goto report_failure;
 	ret = regmap_write(dev->regmap, 0x31, 0x00);
 	if (ret)
-		goto err;
+		goto report_failure;
 	ret = regmap_write(dev->regmap, 0x2c, 0x02);
 	if (ret)
-		goto err;
+		goto report_failure;
 
 	if (tuner_freq_MHz >= 1550) {
 		ucLoDiv1 = 2;
@@ -227,105 +227,105 @@ static int m88rs6000t_set_pll_freq(struct m88rs6000t_dev *dev,
 	reg27 = (((ulNDiv1 >> 8) & 0x0F) + ucLomod1) & 0x7F;
 	ret = regmap_write(dev->regmap, 0x27, reg27);
 	if (ret)
-		goto err;
+		goto report_failure;
 	ret = regmap_write(dev->regmap, 0x28, (u8)(ulNDiv1 & 0xFF));
 	if (ret)
-		goto err;
+		goto report_failure;
 	reg29 = (((ulNDiv2 >> 8) & 0x0F) + ucLomod2) & 0x7f;
 	ret = regmap_write(dev->regmap, 0x29, reg29);
 	if (ret)
-		goto err;
+		goto report_failure;
 	ret = regmap_write(dev->regmap, 0x2a, (u8)(ulNDiv2 & 0xFF));
 	if (ret)
-		goto err;
+		goto report_failure;
 	ret = regmap_write(dev->regmap, 0x2F, 0xf5);
 	if (ret)
-		goto err;
+		goto report_failure;
 	ret = regmap_write(dev->regmap, 0x30, 0x05);
 	if (ret)
-		goto err;
+		goto report_failure;
 	ret = regmap_write(dev->regmap, 0x08, 0x1f);
 	if (ret)
-		goto err;
+		goto report_failure;
 	ret = regmap_write(dev->regmap, 0x08, 0x3f);
 	if (ret)
-		goto err;
+		goto report_failure;
 	ret = regmap_write(dev->regmap, 0x09, 0x20);
 	if (ret)
-		goto err;
+		goto report_failure;
 	ret = regmap_write(dev->regmap, 0x09, 0x00);
 	if (ret)
-		goto err;
+		goto report_failure;
 	ret = regmap_write(dev->regmap, 0x3e, 0x11);
 	if (ret)
-		goto err;
+		goto report_failure;
 	ret = regmap_write(dev->regmap, 0x08, 0x2f);
 	if (ret)
-		goto err;
+		goto report_failure;
 	ret = regmap_write(dev->regmap, 0x08, 0x3f);
 	if (ret)
-		goto err;
+		goto report_failure;
 	ret = regmap_write(dev->regmap, 0x09, 0x10);
 	if (ret)
-		goto err;
+		goto report_failure;
 	ret = regmap_write(dev->regmap, 0x09, 0x00);
 	if (ret)
-		goto err;
+		goto report_failure;
 	usleep_range(2000, 50000);
 
 	ret = regmap_read(dev->regmap, 0x42, &utmp);
 	if (ret)
-		goto err;
+		goto report_failure;
 	reg42 = utmp;
 
 	ret = regmap_write(dev->regmap, 0x3e, 0x10);
 	if (ret)
-		goto err;
+		goto report_failure;
 	ret = regmap_write(dev->regmap, 0x08, 0x2f);
 	if (ret)
-		goto err;
+		goto report_failure;
 	ret = regmap_write(dev->regmap, 0x08, 0x3f);
 	if (ret)
-		goto err;
+		goto report_failure;
 	ret = regmap_write(dev->regmap, 0x09, 0x10);
 	if (ret)
-		goto err;
+		goto report_failure;
 	ret = regmap_write(dev->regmap, 0x09, 0x00);
 	if (ret)
-		goto err;
+		goto report_failure;
 	usleep_range(2000, 50000);
 
 	ret = regmap_read(dev->regmap, 0x42, &utmp);
 	if (ret)
-		goto err;
+		goto report_failure;
 	reg42buf = utmp;
 	if (reg42buf < reg42) {
 		ret = regmap_write(dev->regmap, 0x3e, 0x11);
 		if (ret)
-			goto err;
+			goto report_failure;
 	}
 	usleep_range(5000, 50000);
 
 	ret = regmap_read(dev->regmap, 0x2d, &utmp);
 	if (ret)
-		goto err;
+		goto report_failure;
 	ret = regmap_write(dev->regmap, 0x2d, utmp);
 	if (ret)
-		goto err;
+		goto report_failure;
 	ret = regmap_read(dev->regmap, 0x2e, &utmp);
 	if (ret)
-		goto err;
+		goto report_failure;
 	ret = regmap_write(dev->regmap, 0x2e, utmp);
 	if (ret)
-		goto err;
+		goto report_failure;
 
 	ret = regmap_read(dev->regmap, 0x27, &utmp);
 	if (ret)
-		goto err;
+		goto report_failure;
 	reg27 = utmp & 0x70;
 	ret = regmap_read(dev->regmap, 0x83, &utmp);
 	if (ret)
-		goto err;
+		goto report_failure;
 	if (reg27 == (utmp & 0x70)) {
 		ucLoDiv	= ucLoDiv1;
 		ulNDiv = ulNDiv1;
@@ -340,7 +340,7 @@ static int m88rs6000t_set_pll_freq(struct m88rs6000t_dev *dev,
 		refDiv = 18;
 		ret = regmap_write(dev->regmap, 0x36, (refDiv - 8));
 		if (ret)
-			goto err;
+			goto report_failure;
 		ulNDiv = ((tuner_freq_MHz * ucLoDiv * 1000) * refDiv
 				/ fcry_KHz - 1024) / 2;
 	}
@@ -349,16 +349,16 @@ static int m88rs6000t_set_pll_freq(struct m88rs6000t_dev *dev,
 			+ ((ulNDiv >> 8) & 0x0F)) & 0xFF;
 	ret = regmap_write(dev->regmap, 0x27, reg27);
 	if (ret)
-		goto err;
+		goto report_failure;
 	ret = regmap_write(dev->regmap, 0x28, (u8)(ulNDiv & 0xFF));
 	if (ret)
-		goto err;
+		goto report_failure;
 	ret = regmap_write(dev->regmap, 0x29, 0x80);
 	if (ret)
-		goto err;
+		goto report_failure;
 	ret = regmap_write(dev->regmap, 0x31, 0x03);
 	if (ret)
-		goto err;
+		goto report_failure;
 
 	if (ucLoDiv == 3)
 		utmp = 0xCE;
@@ -366,15 +366,15 @@ static int m88rs6000t_set_pll_freq(struct m88rs6000t_dev *dev,
 		utmp = 0x8A;
 	ret = regmap_write(dev->regmap, 0x3b, utmp);
 	if (ret)
-		goto err;
+		goto report_failure;
 
 	dev->frequency_khz = fcry_KHz * (ulNDiv * 2 + 1024) / refDiv / ucLoDiv;
 
 	dev_dbg(&dev->client->dev,
 		"actual tune frequency=%d\n", dev->frequency_khz);
-err:
-	if (ret)
-		dev_dbg(&dev->client->dev, "failed=%d\n", ret);
+	return 0;
+report_failure:
+	dev_dbg(&dev->client->dev, "failed=%d\n", ret);
 	return ret;
 }
 
@@ -413,21 +413,23 @@ static int m88rs6000t_set_params(struct dvb_frontend *fe)
 	freq_MHz = (realFreq + 500) / 1000;
 	ret = m88rs6000t_set_pll_freq(dev, freq_MHz);
 	if (ret)
-		goto err;
+		goto report_failure;
 	ret = m88rs6000t_set_bb(dev, c->symbol_rate / 1000, lpf_offset_KHz);
 	if (ret)
-		goto err;
+		goto report_failure;
 	ret = regmap_write(dev->regmap, 0x00, 0x01);
 	if (ret)
-		goto err;
+		goto report_failure;
 	ret = regmap_write(dev->regmap, 0x00, 0x00);
 	if (ret)
-		goto err;
+		goto report_failure;
 	/* set demod mlck */
 	ret = m88rs6000t_set_demod_mclk(fe);
-err:
 	if (ret)
-		dev_dbg(&dev->client->dev, "failed=%d\n", ret);
+		goto report_failure;
+	return 0;
+report_failure:
+	dev_dbg(&dev->client->dev, "failed=%d\n", ret);
 	return ret;
 }
 
@@ -440,16 +442,16 @@ static int m88rs6000t_init(struct dvb_frontend *fe)
 
 	ret = regmap_update_bits(dev->regmap, 0x11, 0x08, 0x08);
 	if (ret)
-		goto err;
+		goto report_failure;
 	usleep_range(5000, 50000);
 	ret = regmap_update_bits(dev->regmap, 0x10, 0x01, 0x01);
 	if (ret)
-		goto err;
+		goto report_failure;
 	usleep_range(10000, 50000);
 	ret = regmap_write(dev->regmap, 0x07, 0x7d);
-err:
-	if (ret)
-		dev_dbg(&dev->client->dev, "failed=%d\n", ret);
+	return 0;
+report_failure:
+	dev_dbg(&dev->client->dev, "failed=%d\n", ret);
 	return ret;
 }
 
@@ -510,27 +512,27 @@ static int m88rs6000t_get_rf_strength(struct dvb_frontend *fe, u16 *strength)
 
 	ret = regmap_read(dev->regmap, 0x5A, &val);
 	if (ret)
-		goto err;
+		goto report_failure;
 	RF_GC = val & 0x0f;
 
 	ret = regmap_read(dev->regmap, 0x5F, &val);
 	if (ret)
-		goto err;
+		goto report_failure;
 	IF_GC = val & 0x0f;
 
 	ret = regmap_read(dev->regmap, 0x3F, &val);
 	if (ret)
-		goto err;
+		goto report_failure;
 	TIA_GC = (val >> 4) & 0x07;
 
 	ret = regmap_read(dev->regmap, 0x77, &val);
 	if (ret)
-		goto err;
+		goto report_failure;
 	BB_GC = (val >> 4) & 0x0f;
 
 	ret = regmap_read(dev->regmap, 0x76, &val);
 	if (ret)
-		goto err;
+		goto report_failure;
 	PGA2_GC = val & 0x3f;
 	PGA2_cri = PGA2_GC >> 2;
 	PGA2_crf = PGA2_GC & 0x03;
@@ -562,9 +564,9 @@ static int m88rs6000t_get_rf_strength(struct dvb_frontend *fe, u16 *strength)
 	/* scale value to 0x0000-0xffff */
 	gain = clamp_val(gain, 1000U, 10500U);
 	*strength = (10500 - gain) * 0xffff / (10500 - 1000);
-err:
-	if (ret)
-		dev_dbg(&dev->client->dev, "failed=%d\n", ret);
+	return 0;
+report_failure:
+	dev_dbg(&dev->client->dev, "failed=%d\n", ret);
 	return ret;
 }
 
-- 
2.6.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1298663 — Re: [PATCH 1/2] [media] m88rs6000t: Better exception handling in five functions

FromJulia Lawall <julia.lawall@lip6.fr>
Date2015-12-28 15:50 +0100
SubjectRe: [PATCH 1/2] [media] m88rs6000t: Better exception handling in five functions
Message-ID<qKHhv-3pQ-11@gated-at.bofh.it>
In reply to#1298659

On Mon, 28 Dec 2015, SF Markus Elfring wrote:

> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Mon, 28 Dec 2015 15:10:30 +0100
>
> This issue was detected by using the Coccinelle software.
>
> Move the jump label directly before the desired log statement
> so that the variable "ret" will not be checked once more
> after a function call.

This commit message fits with the previous change.

It could be nice to put a blank line before the error handling code.  See
what is done elsewhere in the file.

julia

>
> Suggested-by: Julia Lawall <julia.lawall@lip6.fr>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/media/tuners/m88rs6000t.c | 154 +++++++++++++++++++-------------------
>  1 file changed, 78 insertions(+), 76 deletions(-)
>
> diff --git a/drivers/media/tuners/m88rs6000t.c b/drivers/media/tuners/m88rs6000t.c
> index 504bfbc..7e59a9f 100644
> --- a/drivers/media/tuners/m88rs6000t.c
> +++ b/drivers/media/tuners/m88rs6000t.c
> @@ -44,7 +44,7 @@ static int m88rs6000t_set_demod_mclk(struct dvb_frontend *fe)
>  	/* select demod main mclk */
>  	ret = regmap_read(dev->regmap, 0x15, &utmp);
>  	if (ret)
> -		goto err;
> +		goto report_failure;
>  	reg15 = utmp;
>  	if (c->symbol_rate > 45010000) {
>  		reg11 = 0x0E;
> @@ -106,7 +106,7 @@ static int m88rs6000t_set_demod_mclk(struct dvb_frontend *fe)
>
>  	ret = regmap_read(dev->regmap, 0x1D, &utmp);
>  	if (ret)
> -		goto err;
> +		goto report_failure;
>  	reg1D = utmp;
>  	reg1D &= ~0x03;
>  	reg1D |= N - 1;
> @@ -116,42 +116,42 @@ static int m88rs6000t_set_demod_mclk(struct dvb_frontend *fe)
>  	/* program and recalibrate demod PLL */
>  	ret = regmap_write(dev->regmap, 0x05, 0x40);
>  	if (ret)
> -		goto err;
> +		goto report_failure;
>  	ret = regmap_write(dev->regmap, 0x11, 0x08);
>  	if (ret)
> -		goto err;
> +		goto report_failure;
>  	ret = regmap_write(dev->regmap, 0x15, reg15);
>  	if (ret)
> -		goto err;
> +		goto report_failure;
>  	ret = regmap_write(dev->regmap, 0x16, reg16);
>  	if (ret)
> -		goto err;
> +		goto report_failure;
>  	ret = regmap_write(dev->regmap, 0x1D, reg1D);
>  	if (ret)
> -		goto err;
> +		goto report_failure;
>  	ret = regmap_write(dev->regmap, 0x1E, reg1E);
>  	if (ret)
> -		goto err;
> +		goto report_failure;
>  	ret = regmap_write(dev->regmap, 0x1F, reg1F);
>  	if (ret)
> -		goto err;
> +		goto report_failure;
>  	ret = regmap_write(dev->regmap, 0x17, 0xc1);
>  	if (ret)
> -		goto err;
> +		goto report_failure;
>  	ret = regmap_write(dev->regmap, 0x17, 0x81);
>  	if (ret)
> -		goto err;
> +		goto report_failure;
>  	usleep_range(5000, 50000);
>  	ret = regmap_write(dev->regmap, 0x05, 0x00);
>  	if (ret)
> -		goto err;
> +		goto report_failure;
>  	ret = regmap_write(dev->regmap, 0x11, reg11);
>  	if (ret)
> -		goto err;
> +		goto report_failure;
>  	usleep_range(5000, 50000);
> -err:
> -	if (ret)
> -		dev_dbg(&dev->client->dev, "failed=%d\n", ret);
> +	return 0;
> +report_failure:
> +	dev_dbg(&dev->client->dev, "failed=%d\n", ret);
>  	return ret;
>  }
>
> @@ -169,13 +169,13 @@ static int m88rs6000t_set_pll_freq(struct m88rs6000t_dev *dev,
>
>  	ret = regmap_write(dev->regmap, 0x36, (refDiv - 8));
>  	if (ret)
> -		goto err;
> +		goto report_failure;
>  	ret = regmap_write(dev->regmap, 0x31, 0x00);
>  	if (ret)
> -		goto err;
> +		goto report_failure;
>  	ret = regmap_write(dev->regmap, 0x2c, 0x02);
>  	if (ret)
> -		goto err;
> +		goto report_failure;
>
>  	if (tuner_freq_MHz >= 1550) {
>  		ucLoDiv1 = 2;
> @@ -227,105 +227,105 @@ static int m88rs6000t_set_pll_freq(struct m88rs6000t_dev *dev,
>  	reg27 = (((ulNDiv1 >> 8) & 0x0F) + ucLomod1) & 0x7F;
>  	ret = regmap_write(dev->regmap, 0x27, reg27);
>  	if (ret)
> -		goto err;
> +		goto report_failure;
>  	ret = regmap_write(dev->regmap, 0x28, (u8)(ulNDiv1 & 0xFF));
>  	if (ret)
> -		goto err;
> +		goto report_failure;
>  	reg29 = (((ulNDiv2 >> 8) & 0x0F) + ucLomod2) & 0x7f;
>  	ret = regmap_write(dev->regmap, 0x29, reg29);
>  	if (ret)
> -		goto err;
> +		goto report_failure;
>  	ret = regmap_write(dev->regmap, 0x2a, (u8)(ulNDiv2 & 0xFF));
>  	if (ret)
> -		goto err;
> +		goto report_failure;
>  	ret = regmap_write(dev->regmap, 0x2F, 0xf5);
>  	if (ret)
> -		goto err;
> +		goto report_failure;
>  	ret = regmap_write(dev->regmap, 0x30, 0x05);
>  	if (ret)
> -		goto err;
> +		goto report_failure;
>  	ret = regmap_write(dev->regmap, 0x08, 0x1f);
>  	if (ret)
> -		goto err;
> +		goto report_failure;
>  	ret = regmap_write(dev->regmap, 0x08, 0x3f);
>  	if (ret)
> -		goto err;
> +		goto report_failure;
>  	ret = regmap_write(dev->regmap, 0x09, 0x20);
>  	if (ret)
> -		goto err;
> +		goto report_failure;
>  	ret = regmap_write(dev->regmap, 0x09, 0x00);
>  	if (ret)
> -		goto err;
> +		goto report_failure;
>  	ret = regmap_write(dev->regmap, 0x3e, 0x11);
>  	if (ret)
> -		goto err;
> +		goto report_failure;
>  	ret = regmap_write(dev->regmap, 0x08, 0x2f);
>  	if (ret)
> -		goto err;
> +		goto report_failure;
>  	ret = regmap_write(dev->regmap, 0x08, 0x3f);
>  	if (ret)
> -		goto err;
> +		goto report_failure;
>  	ret = regmap_write(dev->regmap, 0x09, 0x10);
>  	if (ret)
> -		goto err;
> +		goto report_failure;
>  	ret = regmap_write(dev->regmap, 0x09, 0x00);
>  	if (ret)
> -		goto err;
> +		goto report_failure;
>  	usleep_range(2000, 50000);
>
>  	ret = regmap_read(dev->regmap, 0x42, &utmp);
>  	if (ret)
> -		goto err;
> +		goto report_failure;
>  	reg42 = utmp;
>
>  	ret = regmap_write(dev->regmap, 0x3e, 0x10);
>  	if (ret)
> -		goto err;
> +		goto report_failure;
>  	ret = regmap_write(dev->regmap, 0x08, 0x2f);
>  	if (ret)
> -		goto err;
> +		goto report_failure;
>  	ret = regmap_write(dev->regmap, 0x08, 0x3f);
>  	if (ret)
> -		goto err;
> +		goto report_failure;
>  	ret = regmap_write(dev->regmap, 0x09, 0x10);
>  	if (ret)
> -		goto err;
> +		goto report_failure;
>  	ret = regmap_write(dev->regmap, 0x09, 0x00);
>  	if (ret)
> -		goto err;
> +		goto report_failure;
>  	usleep_range(2000, 50000);
>
>  	ret = regmap_read(dev->regmap, 0x42, &utmp);
>  	if (ret)
> -		goto err;
> +		goto report_failure;
>  	reg42buf = utmp;
>  	if (reg42buf < reg42) {
>  		ret = regmap_write(dev->regmap, 0x3e, 0x11);
>  		if (ret)
> -			goto err;
> +			goto report_failure;
>  	}
>  	usleep_range(5000, 50000);
>
>  	ret = regmap_read(dev->regmap, 0x2d, &utmp);
>  	if (ret)
> -		goto err;
> +		goto report_failure;
>  	ret = regmap_write(dev->regmap, 0x2d, utmp);
>  	if (ret)
> -		goto err;
> +		goto report_failure;
>  	ret = regmap_read(dev->regmap, 0x2e, &utmp);
>  	if (ret)
> -		goto err;
> +		goto report_failure;
>  	ret = regmap_write(dev->regmap, 0x2e, utmp);
>  	if (ret)
> -		goto err;
> +		goto report_failure;
>
>  	ret = regmap_read(dev->regmap, 0x27, &utmp);
>  	if (ret)
> -		goto err;
> +		goto report_failure;
>  	reg27 = utmp & 0x70;
>  	ret = regmap_read(dev->regmap, 0x83, &utmp);
>  	if (ret)
> -		goto err;
> +		goto report_failure;
>  	if (reg27 == (utmp & 0x70)) {
>  		ucLoDiv	= ucLoDiv1;
>  		ulNDiv = ulNDiv1;
> @@ -340,7 +340,7 @@ static int m88rs6000t_set_pll_freq(struct m88rs6000t_dev *dev,
>  		refDiv = 18;
>  		ret = regmap_write(dev->regmap, 0x36, (refDiv - 8));
>  		if (ret)
> -			goto err;
> +			goto report_failure;
>  		ulNDiv = ((tuner_freq_MHz * ucLoDiv * 1000) * refDiv
>  				/ fcry_KHz - 1024) / 2;
>  	}
> @@ -349,16 +349,16 @@ static int m88rs6000t_set_pll_freq(struct m88rs6000t_dev *dev,
>  			+ ((ulNDiv >> 8) & 0x0F)) & 0xFF;
>  	ret = regmap_write(dev->regmap, 0x27, reg27);
>  	if (ret)
> -		goto err;
> +		goto report_failure;
>  	ret = regmap_write(dev->regmap, 0x28, (u8)(ulNDiv & 0xFF));
>  	if (ret)
> -		goto err;
> +		goto report_failure;
>  	ret = regmap_write(dev->regmap, 0x29, 0x80);
>  	if (ret)
> -		goto err;
> +		goto report_failure;
>  	ret = regmap_write(dev->regmap, 0x31, 0x03);
>  	if (ret)
> -		goto err;
> +		goto report_failure;
>
>  	if (ucLoDiv == 3)
>  		utmp = 0xCE;
> @@ -366,15 +366,15 @@ static int m88rs6000t_set_pll_freq(struct m88rs6000t_dev *dev,
>  		utmp = 0x8A;
>  	ret = regmap_write(dev->regmap, 0x3b, utmp);
>  	if (ret)
> -		goto err;
> +		goto report_failure;
>
>  	dev->frequency_khz = fcry_KHz * (ulNDiv * 2 + 1024) / refDiv / ucLoDiv;
>
>  	dev_dbg(&dev->client->dev,
>  		"actual tune frequency=%d\n", dev->frequency_khz);
> -err:
> -	if (ret)
> -		dev_dbg(&dev->client->dev, "failed=%d\n", ret);
> +	return 0;
> +report_failure:
> +	dev_dbg(&dev->client->dev, "failed=%d\n", ret);
>  	return ret;
>  }
>
> @@ -413,21 +413,23 @@ static int m88rs6000t_set_params(struct dvb_frontend *fe)
>  	freq_MHz = (realFreq + 500) / 1000;
>  	ret = m88rs6000t_set_pll_freq(dev, freq_MHz);
>  	if (ret)
> -		goto err;
> +		goto report_failure;
>  	ret = m88rs6000t_set_bb(dev, c->symbol_rate / 1000, lpf_offset_KHz);
>  	if (ret)
> -		goto err;
> +		goto report_failure;
>  	ret = regmap_write(dev->regmap, 0x00, 0x01);
>  	if (ret)
> -		goto err;
> +		goto report_failure;
>  	ret = regmap_write(dev->regmap, 0x00, 0x00);
>  	if (ret)
> -		goto err;
> +		goto report_failure;
>  	/* set demod mlck */
>  	ret = m88rs6000t_set_demod_mclk(fe);
> -err:
>  	if (ret)
> -		dev_dbg(&dev->client->dev, "failed=%d\n", ret);
> +		goto report_failure;
> +	return 0;
> +report_failure:
> +	dev_dbg(&dev->client->dev, "failed=%d\n", ret);
>  	return ret;
>  }
>
> @@ -440,16 +442,16 @@ static int m88rs6000t_init(struct dvb_frontend *fe)
>
>  	ret = regmap_update_bits(dev->regmap, 0x11, 0x08, 0x08);
>  	if (ret)
> -		goto err;
> +		goto report_failure;
>  	usleep_range(5000, 50000);
>  	ret = regmap_update_bits(dev->regmap, 0x10, 0x01, 0x01);
>  	if (ret)
> -		goto err;
> +		goto report_failure;
>  	usleep_range(10000, 50000);
>  	ret = regmap_write(dev->regmap, 0x07, 0x7d);
> -err:
> -	if (ret)
> -		dev_dbg(&dev->client->dev, "failed=%d\n", ret);
> +	return 0;
> +report_failure:
> +	dev_dbg(&dev->client->dev, "failed=%d\n", ret);
>  	return ret;
>  }
>
> @@ -510,27 +512,27 @@ static int m88rs6000t_get_rf_strength(struct dvb_frontend *fe, u16 *strength)
>
>  	ret = regmap_read(dev->regmap, 0x5A, &val);
>  	if (ret)
> -		goto err;
> +		goto report_failure;
>  	RF_GC = val & 0x0f;
>
>  	ret = regmap_read(dev->regmap, 0x5F, &val);
>  	if (ret)
> -		goto err;
> +		goto report_failure;
>  	IF_GC = val & 0x0f;
>
>  	ret = regmap_read(dev->regmap, 0x3F, &val);
>  	if (ret)
> -		goto err;
> +		goto report_failure;
>  	TIA_GC = (val >> 4) & 0x07;
>
>  	ret = regmap_read(dev->regmap, 0x77, &val);
>  	if (ret)
> -		goto err;
> +		goto report_failure;
>  	BB_GC = (val >> 4) & 0x0f;
>
>  	ret = regmap_read(dev->regmap, 0x76, &val);
>  	if (ret)
> -		goto err;
> +		goto report_failure;
>  	PGA2_GC = val & 0x3f;
>  	PGA2_cri = PGA2_GC >> 2;
>  	PGA2_crf = PGA2_GC & 0x03;
> @@ -562,9 +564,9 @@ static int m88rs6000t_get_rf_strength(struct dvb_frontend *fe, u16 *strength)
>  	/* scale value to 0x0000-0xffff */
>  	gain = clamp_val(gain, 1000U, 10500U);
>  	*strength = (10500 - gain) * 0xffff / (10500 - 1000);
> -err:
> -	if (ret)
> -		dev_dbg(&dev->client->dev, "failed=%d\n", ret);
> +	return 0;
> +report_failure:
> +	dev_dbg(&dev->client->dev, "failed=%d\n", ret);
>  	return ret;
>  }
>
> --
> 2.6.3
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1298674 — Re: [media] m88rs6000t: Better exception handling in five functions

FromSF Markus Elfring <elfring@users.sourceforge.net>
Date2015-12-28 16:10 +0100
SubjectRe: [media] m88rs6000t: Better exception handling in five functions
Message-ID<qKHAS-3Nt-15@gated-at.bofh.it>
In reply to#1298663
>> Move the jump label directly before the desired log statement
>> so that the variable "ret" will not be checked once more
>> after a function call.
> 
> This commit message fits with the previous change.

Do you prefer an other wording?


> It could be nice to put a blank line before the error handling code.

Is it really a coding style requirement to insert another blank line between
the suggested placement of the statement "return 0;" and the jump label?

Regards,
Markus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1298676 — Re: [media] m88rs6000t: Better exception handling in five functions

FromJulia Lawall <julia.lawall@lip6.fr>
Date2015-12-28 16:20 +0100
SubjectRe: [media] m88rs6000t: Better exception handling in five functions
Message-ID<qKHKx-3R8-1@gated-at.bofh.it>
In reply to#1298674
On Mon, 28 Dec 2015, SF Markus Elfring wrote:

> >> Move the jump label directly before the desired log statement
> >> so that the variable "ret" will not be checked once more
> >> after a function call.
> >
> > This commit message fits with the previous change.
>
> Do you prefer an other wording?

Something like "Split the return into success and error cases, to avoid
the need for testing before logging."

The concept of the return being duplicated didn't come across in your
message.

> > It could be nice to put a blank line before the error handling code.
>
> Is it really a coding style requirement to insert another blank line between
> the suggested placement of the statement "return 0;" and the jump label?

I don't think it is a requirement.  But some files do it, and if other
functions in this file do it, then it would be nice to do the same.

julia
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1298670 — [PATCH 2/2] [media] tuners: Refactoring for m88rs6000t_sleep()

FromSF Markus Elfring <elfring@users.sourceforge.net>
Date2015-12-28 15:50 +0100
Subject[PATCH 2/2] [media] tuners: Refactoring for m88rs6000t_sleep()
Message-ID<qKHhw-3pQ-31@gated-at.bofh.it>
In reply to#1298658
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 28 Dec 2015 15:20:45 +0100

This issue was detected by using the Coccinelle software.

1. Let us return directly if a call of the regmap_write() function failed.

2. Delete the jump label "err" then.

3. Return zero as a constant at the end.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/media/tuners/m88rs6000t.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/media/tuners/m88rs6000t.c b/drivers/media/tuners/m88rs6000t.c
index 7e59a9f..8d10798 100644
--- a/drivers/media/tuners/m88rs6000t.c
+++ b/drivers/media/tuners/m88rs6000t.c
@@ -463,13 +463,12 @@ static int m88rs6000t_sleep(struct dvb_frontend *fe)
 	dev_dbg(&dev->client->dev, "%s:\n", __func__);
 
 	ret = regmap_write(dev->regmap, 0x07, 0x6d);
-	if (ret)
-		goto err;
-	usleep_range(5000, 10000);
-err:
-	if (ret)
+	if (ret) {
 		dev_dbg(&dev->client->dev, "failed=%d\n", ret);
-	return ret;
+		return ret;
+	}
+	usleep_range(5000, 10000);
+	return 0;
 }
 
 static int m88rs6000t_get_frequency(struct dvb_frontend *fe, u32 *frequency)
-- 
2.6.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web