[PATCH 0/2] Re: [git:v4l-dvb/for_v3.3] [media] dvb-bt8xx: handle errors from dvb_net_init

[PATCH 0/2] Re: [git:v4l-dvb/for_v3.3] [media] dvb-bt8xx: handle errors from dvb_net_init

[Jonathan Nieder]

Hi Mauro,

Mauro Carvalho Chehab wrote:

> Subject: [media] dvb-bt8xx: handle errors from dvb_net_init
[...]
> [mchehab.redhat.com: codingstyle fix: printk() should include KERN_ facility level]
[...]
> --- a/drivers/media/dvb/bt8xx/dvb-bt8xx.c
> +++ b/drivers/media/dvb/bt8xx/dvb-bt8xx.c
> @@ -782,7 +782,12 @@ static int __devinit dvb_bt8xx_load_card(struct dvb_bt8xx_card *card, u32 type)
>  		goto err_remove_mem_frontend;
>  	}
>  
> -	dvb_net_init(&card->dvb_adapter, &card->dvbnet, &card->demux.dmx);
> +	result = dvb_net_init(&card->dvb_adapter, &card->dvbnet, &card->demux.dmx);
> +	if (result < 0) {
> +		printk(KERN_ERR,
> +		       "dvb_bt8xx: dvb_net_init failed (errno = %d)\n", result);

I think there is an extra comma here:

	$ make drivers/media/dvb/bt8xx/dvb-bt8xx.o
	  CHK     include/linux/version.h
	  CHK     include/generated/utsrelease.h
	  CALL    scripts/checksyscalls.sh
	  CC [M]  drivers/media/dvb/bt8xx/dvb-bt8xx.o
	drivers/media/dvb/bt8xx/dvb-bt8xx.c: In function ‘dvb_bt8xx_load_card’:
	drivers/media/dvb/bt8xx/dvb-bt8xx.c:788:10: warning: too many arguments for format [-Wformat-extra-args]

Perhaps it would be better to add the KERN_ levels throughout the file
with a separate patch.  Like this:

Jonathan Nieder (2):
  [media] dvb-bt8xx: use dprintk for debug statements
  [media] dvb-bt8xx: convert printks to pr_err()

 drivers/media/dvb/bt8xx/dvb-bt8xx.c |   41 +++++++++++++++++------------------
 1 files changed, 20 insertions(+), 21 deletions(-)

[PATCH 1/2] [media] dvb-bt8xx: use dprintk for debug statements

[Jonathan Nieder]

This way, the messages will be tagged with KERN_DEBUG and not clutter
the log from dmesg unless the "debug" module parameter is set.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 drivers/media/dvb/bt8xx/dvb-bt8xx.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/dvb/bt8xx/dvb-bt8xx.c b/drivers/media/dvb/bt8xx/dvb-bt8xx.c
index b9d927270318..2f38cca7604b 100644
--- a/drivers/media/dvb/bt8xx/dvb-bt8xx.c
+++ b/drivers/media/dvb/bt8xx/dvb-bt8xx.c
@@ -205,7 +205,7 @@ static int cx24108_tuner_set_params(struct dvb_frontend *fe)
 		0x00120000,0x00140000};
 
 	#define XTAL 1011100 /* Hz, really 1.0111 MHz and a /10 prescaler */
-	printk("cx24108 debug: entering SetTunerFreq, freq=%d\n",freq);
+	dprintk("cx24108 debug: entering SetTunerFreq, freq=%d\n",freq);
 
 	/* This is really the bit driving the tuner chip cx24108 */
 
@@ -216,7 +216,7 @@ static int cx24108_tuner_set_params(struct dvb_frontend *fe)
 
 	/* decide which VCO to use for the input frequency */
 	for(i = 1; (i < ARRAY_SIZE(osci) - 1) && (osci[i] < freq); i++);
-	printk("cx24108 debug: select vco #%d (f=%d)\n",i,freq);
+	dprintk("cx24108 debug: select vco #%d (f=%d)\n",i,freq);
 	band=bandsel[i];
 	/* the gain values must be set by SetSymbolrate */
 	/* compute the pll divider needed, from Conexant data sheet,
@@ -232,7 +232,7 @@ static int cx24108_tuner_set_params(struct dvb_frontend *fe)
 	    ((a&0x1f)<<11);
 	/* everything is shifted left 11 bits to left-align the bits in the
 	   32bit word. Output to the tuner goes MSB-aligned, after all */
-	printk("cx24108 debug: pump=%d, n=%d, a=%d\n",pump,n,a);
+	dprintk("cx24108 debug: pump=%d, n=%d, a=%d\n",pump,n,a);
 	cx24110_pll_write(fe,band);
 	/* set vga and vca to their widest-band settings, as a precaution.
 	   SetSymbolrate might not be called to set this up */
-- 
1.7.8.2

[PATCH 2/2] [media] dvb-bt8xx: convert printks to pr_err()

[Jonathan Nieder]

This module does some printks with the loglevel missing.

pr_err() takes care of adding the KERN_ERR tag and the module name.
So we can simplify the code and add the missing printk loglevel by
using it.

Also add a #define pr_fmt() to make this work, and remove a few
unnecessary periods at the end of messages and bump the loglevel of
"Unknown bttv card type" from KERN_WARNING to KERN_ERR while at it.

Inspired-by: Mauro Carvalho Chehab <mchehab@redhat.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 drivers/media/dvb/bt8xx/dvb-bt8xx.c |   35 +++++++++++++++++------------------
 1 files changed, 17 insertions(+), 18 deletions(-)

diff --git a/drivers/media/dvb/bt8xx/dvb-bt8xx.c b/drivers/media/dvb/bt8xx/dvb-bt8xx.c
index 2f38cca7604b..f9087a0f0cc3 100644
--- a/drivers/media/dvb/bt8xx/dvb-bt8xx.c
+++ b/drivers/media/dvb/bt8xx/dvb-bt8xx.c
@@ -19,6 +19,8 @@
  *
  */
 
+#define pr_fmt(fmt) "dvb_bt8xx: " fmt
+
 #include <linux/bitops.h>
 #include <linux/module.h>
 #include <linux/init.h>
@@ -666,7 +668,7 @@ static void frontend_init(struct dvb_bt8xx_card *card, u32 type)
 		/*	DST is not a frontend driver !!!		*/
 		state = kmalloc(sizeof (struct dst_state), GFP_KERNEL);
 		if (!state) {
-			printk("dvb_bt8xx: No memory\n");
+			pr_err("No memory\n");
 			break;
 		}
 		/*	Setup the Card					*/
@@ -676,7 +678,7 @@ static void frontend_init(struct dvb_bt8xx_card *card, u32 type)
 		state->dst_ca = NULL;
 		/*	DST is not a frontend, attaching the ASIC	*/
 		if (dvb_attach(dst_attach, state, &card->dvb_adapter) == NULL) {
-			printk("%s: Could not find a Twinhan DST.\n", __func__);
+			pr_err("%s: Could not find a Twinhan DST\n", __func__);
 			break;
 		}
 		/*	Attach other DST peripherals if any		*/
@@ -705,14 +707,14 @@ static void frontend_init(struct dvb_bt8xx_card *card, u32 type)
 	}
 
 	if (card->fe == NULL)
-		printk("dvb-bt8xx: A frontend driver was not found for device [%04x:%04x] subsystem [%04x:%04x]\n",
+		pr_err("A frontend driver was not found for device [%04x:%04x] subsystem [%04x:%04x]\n",
 		       card->bt->dev->vendor,
 		       card->bt->dev->device,
 		       card->bt->dev->subsystem_vendor,
 		       card->bt->dev->subsystem_device);
 	else
 		if (dvb_register_frontend(&card->dvb_adapter, card->fe)) {
-			printk("dvb-bt8xx: Frontend registration failed!\n");
+			pr_err("Frontend registration failed!\n");
 			dvb_frontend_detach(card->fe);
 			card->fe = NULL;
 		}
@@ -726,7 +728,7 @@ static int __devinit dvb_bt8xx_load_card(struct dvb_bt8xx_card *card, u32 type)
 				      THIS_MODULE, &card->bt->dev->dev,
 				      adapter_nr);
 	if (result < 0) {
-		printk("dvb_bt8xx: dvb_register_adapter failed (errno = %d)\n", result);
+		pr_err("dvb_register_adapter failed (errno = %d)\n", result);
 		return result;
 	}
 	card->dvb_adapter.priv = card;
@@ -746,7 +748,7 @@ static int __devinit dvb_bt8xx_load_card(struct dvb_bt8xx_card *card, u32 type)
 
 	result = dvb_dmx_init(&card->demux);
 	if (result < 0) {
-		printk("dvb_bt8xx: dvb_dmx_init failed (errno = %d)\n", result);
+		pr_err("dvb_dmx_init failed (errno = %d)\n", result);
 		goto err_unregister_adaptor;
 	}
 
@@ -756,7 +758,7 @@ static int __devinit dvb_bt8xx_load_card(struct dvb_bt8xx_card *card, u32 type)
 
 	result = dvb_dmxdev_init(&card->dmxdev, &card->dvb_adapter);
 	if (result < 0) {
-		printk("dvb_bt8xx: dvb_dmxdev_init failed (errno = %d)\n", result);
+		pr_err("dvb_dmxdev_init failed (errno = %d)\n", result);
 		goto err_dmx_release;
 	}
 
@@ -764,7 +766,7 @@ static int __devinit dvb_bt8xx_load_card(struct dvb_bt8xx_card *card, u32 type)
 
 	result = card->demux.dmx.add_frontend(&card->demux.dmx, &card->fe_hw);
 	if (result < 0) {
-		printk("dvb_bt8xx: dvb_dmx_init failed (errno = %d)\n", result);
+		pr_err("dvb_dmx_init failed (errno = %d)\n", result);
 		goto err_dmxdev_release;
 	}
 
@@ -772,19 +774,19 @@ static int __devinit dvb_bt8xx_load_card(struct dvb_bt8xx_card *card, u32 type)
 
 	result = card->demux.dmx.add_frontend(&card->demux.dmx, &card->fe_mem);
 	if (result < 0) {
-		printk("dvb_bt8xx: dvb_dmx_init failed (errno = %d)\n", result);
+		pr_err("dvb_dmx_init failed (errno = %d)\n", result);
 		goto err_remove_hw_frontend;
 	}
 
 	result = card->demux.dmx.connect_frontend(&card->demux.dmx, &card->fe_hw);
 	if (result < 0) {
-		printk("dvb_bt8xx: dvb_dmx_init failed (errno = %d)\n", result);
+		pr_err("dvb_dmx_init failed (errno = %d)\n", result);
 		goto err_remove_mem_frontend;
 	}
 
 	result = dvb_net_init(&card->dvb_adapter, &card->dvbnet, &card->demux.dmx);
 	if (result < 0) {
-		printk("dvb_bt8xx: dvb_net_init failed (errno = %d)\n", result);
+		pr_err("dvb_net_init failed (errno = %d)\n", result);
 		goto err_disconnect_frontend;
 	}
 
@@ -887,8 +889,7 @@ static int __devinit dvb_bt8xx_probe(struct bttv_sub_device *sub)
 		break;
 
 	default:
-		printk(KERN_WARNING "dvb_bt8xx: Unknown bttv card type: %d.\n",
-				sub->core->type);
+		pr_err("Unknown bttv card type: %d\n", sub->core->type);
 		kfree(card);
 		return -ENODEV;
 	}
@@ -896,16 +897,14 @@ static int __devinit dvb_bt8xx_probe(struct bttv_sub_device *sub)
 	dprintk("dvb_bt8xx: identified card%d as %s\n", card->bttv_nr, card->card_name);
 
 	if (!(bttv_pci_dev = bttv_get_pcidev(card->bttv_nr))) {
-		printk("dvb_bt8xx: no pci device for card %d\n", card->bttv_nr);
+		pr_err("no pci device for card %d\n", card->bttv_nr);
 		kfree(card);
 		return -ENODEV;
 	}
 
 	if (!(card->bt = dvb_bt8xx_878_match(card->bttv_nr, bttv_pci_dev))) {
-		printk("dvb_bt8xx: unable to determine DMA core of card %d,\n",
-		       card->bttv_nr);
-		printk("dvb_bt8xx: if you have the ALSA bt87x audio driver "
-		       "installed, try removing it.\n");
+		pr_err("unable to determine DMA core of card %d,\n", card->bttv_nr);
+		pr_err("if you have the ALSA bt87x audio driver installed, try removing it.\n");
 
 		kfree(card);
 		return -ENODEV;
-- 
1.7.8.2

Re: [PATCH 0/2] Re: [git:v4l-dvb/for_v3.3] [media] dvb-bt8xx: handle errors from dvb_net_init

[Mauro Carvalho Chehab]

On 06-01-2012 16:25, Jonathan Nieder wrote:
> Hi Mauro,
> 
> Mauro Carvalho Chehab wrote:
> 
>> Subject: [media] dvb-bt8xx: handle errors from dvb_net_init
> [...]
>> [mchehab.redhat.com: codingstyle fix: printk() should include KERN_ facility level]
> [...]
>> --- a/drivers/media/dvb/bt8xx/dvb-bt8xx.c
>> +++ b/drivers/media/dvb/bt8xx/dvb-bt8xx.c
>> @@ -782,7 +782,12 @@ static int __devinit dvb_bt8xx_load_card(struct dvb_bt8xx_card *card, u32 type)
>>  		goto err_remove_mem_frontend;
>>  	}
>>  
>> -	dvb_net_init(&card->dvb_adapter, &card->dvbnet, &card->demux.dmx);
>> +	result = dvb_net_init(&card->dvb_adapter, &card->dvbnet, &card->demux.dmx);
>> +	if (result < 0) {
>> +		printk(KERN_ERR,
>> +		       "dvb_bt8xx: dvb_net_init failed (errno = %d)\n", result);
> 
> I think there is an extra comma here:

Yeah, I noticed it, but only after adding it at the main repo :(
> 
> 	$ make drivers/media/dvb/bt8xx/dvb-bt8xx.o
> 	  CHK     include/linux/version.h
> 	  CHK     include/generated/utsrelease.h
> 	  CALL    scripts/checksyscalls.sh
> 	  CC [M]  drivers/media/dvb/bt8xx/dvb-bt8xx.o
> 	drivers/media/dvb/bt8xx/dvb-bt8xx.c: In function ‘dvb_bt8xx_load_card’:
> 	drivers/media/dvb/bt8xx/dvb-bt8xx.c:788:10: warning: too many arguments for format [-Wformat-extra-args]
> 
> Perhaps it would be better to add the KERN_ levels throughout the file
> with a separate patch.  Like this:
> 
> Jonathan Nieder (2):
>   [media] dvb-bt8xx: use dprintk for debug statements
>   [media] dvb-bt8xx: convert printks to pr_err()
> 
>  drivers/media/dvb/bt8xx/dvb-bt8xx.c |   41 +++++++++++++++++------------------
>  1 files changed, 20 insertions(+), 21 deletions(-)