linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Frank Schäfer" <fschaefer.oss@googlemail.com>
To: Mauro Carvalho Chehab <mchehab@redhat.com>
Cc: Linux Media Mailing List <linux-media@vger.kernel.org>
Subject: Re: [PATCH v2 01/11] em28xx-i2c: replace printk() with the corresponding em28xx macros
Date: Mon, 04 Mar 2013 19:27:12 +0100	[thread overview]
Message-ID: <5134E780.4000700@googlemail.com> (raw)
In-Reply-To: <20130304150931.050dd815@redhat.com>

Am 04.03.2013 19:09, schrieb Mauro Carvalho Chehab:
> Em Sun,  3 Mar 2013 20:37:34 +0100
> Frank Schäfer <fschaefer.oss@googlemail.com> escreveu:
>
>> Reduces the number of characters/lines, unifies the code and improves readability.
> Had you actually test this patch? The reason why printk() is used on some
> places is because dev->name is not available early.

The em28xx-specific macros are using printk, too.
They are actually just an abbreviation of the current printks (saving us
the KERN_XY and the the dev->name parameter).
See em28xx.h.

> That's said, it makes sense to replace all those em28xx-specific printk
> functions by the standard pr_fmt-based ones (pr_err, pr_info, pr_debug, etc).

Yeah, I agree.
But that would be a separate patch series... ;)
I can do that (later) if you want.

Regards,
Frank

>
> Regards,
> Mauro
>
>> Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com>
>> ---
>>  drivers/media/usb/em28xx/em28xx-i2c.c |   55 ++++++++++++++-------------------
>>  1 Datei geändert, 24 Zeilen hinzugefügt(+), 31 Zeilen entfernt(-)
>>
>> diff --git a/drivers/media/usb/em28xx/em28xx-i2c.c b/drivers/media/usb/em28xx/em28xx-i2c.c
>> index 8532c1d..8819b54 100644
>> --- a/drivers/media/usb/em28xx/em28xx-i2c.c
>> +++ b/drivers/media/usb/em28xx/em28xx-i2c.c
>> @@ -399,7 +399,7 @@ static int em28xx_i2c_eeprom(struct em28xx *dev, unsigned char *eedata, int len)
>>  	/* Check if board has eeprom */
>>  	err = i2c_master_recv(&dev->i2c_client, &buf, 0);
>>  	if (err < 0) {
>> -		em28xx_errdev("board has no eeprom\n");
>> +		em28xx_info("board has no eeprom\n");
>>  		memset(eedata, 0, len);
>>  		return -ENODEV;
>>  	}
>> @@ -408,8 +408,7 @@ static int em28xx_i2c_eeprom(struct em28xx *dev, unsigned char *eedata, int len)
>>  
>>  	err = i2c_master_send(&dev->i2c_client, &buf, 1);
>>  	if (err != 1) {
>> -		printk(KERN_INFO "%s: Huh, no eeprom present (err=%d)?\n",
>> -		       dev->name, err);
>> +		em28xx_errdev("failed to read eeprom (err=%d)\n", err);
>>  		return err;
>>  	}
>>  
>> @@ -426,9 +425,7 @@ static int em28xx_i2c_eeprom(struct em28xx *dev, unsigned char *eedata, int len)
>>  
>>  		if (block !=
>>  		    (err = i2c_master_recv(&dev->i2c_client, p, block))) {
>> -			printk(KERN_WARNING
>> -			       "%s: i2c eeprom read error (err=%d)\n",
>> -			       dev->name, err);
>> +			em28xx_errdev("i2c eeprom read error (err=%d)\n", err);
>>  			return err;
>>  		}
>>  		size -= block;
>> @@ -436,7 +433,7 @@ static int em28xx_i2c_eeprom(struct em28xx *dev, unsigned char *eedata, int len)
>>  	}
>>  	for (i = 0; i < len; i++) {
>>  		if (0 == (i % 16))
>> -			printk(KERN_INFO "%s: i2c eeprom %02x:", dev->name, i);
>> +			em28xx_info("i2c eeprom %02x:", i);
>>  		printk(" %02x", eedata[i]);
>>  		if (15 == (i % 16))
>>  			printk("\n");
>> @@ -445,55 +442,51 @@ static int em28xx_i2c_eeprom(struct em28xx *dev, unsigned char *eedata, int len)
>>  	if (em_eeprom->id == 0x9567eb1a)
>>  		dev->hash = em28xx_hash_mem(eedata, len, 32);
>>  
>> -	printk(KERN_INFO "%s: EEPROM ID= 0x%08x, EEPROM hash = 0x%08lx\n",
>> -	       dev->name, em_eeprom->id, dev->hash);
>> +	em28xx_info("EEPROM ID = 0x%08x, EEPROM hash = 0x%08lx\n",
>> +		    em_eeprom->id, dev->hash);
>>  
>> -	printk(KERN_INFO "%s: EEPROM info:\n", dev->name);
>> +	em28xx_info("EEPROM info:\n");
>>  
>>  	switch (em_eeprom->chip_conf >> 4 & 0x3) {
>>  	case 0:
>> -		printk(KERN_INFO "%s:\tNo audio on board.\n", dev->name);
>> +		em28xx_info("\tNo audio on board.\n");
>>  		break;
>>  	case 1:
>> -		printk(KERN_INFO "%s:\tAC97 audio (5 sample rates)\n",
>> -				 dev->name);
>> +		em28xx_info("\tAC97 audio (5 sample rates)\n");
>>  		break;
>>  	case 2:
>> -		printk(KERN_INFO "%s:\tI2S audio, sample rate=32k\n",
>> -				 dev->name);
>> +		em28xx_info("\tI2S audio, sample rate=32k\n");
>>  		break;
>>  	case 3:
>> -		printk(KERN_INFO "%s:\tI2S audio, 3 sample rates\n",
>> -				 dev->name);
>> +		em28xx_info("\tI2S audio, 3 sample rates\n");
>>  		break;
>>  	}
>>  
>>  	if (em_eeprom->chip_conf & 1 << 3)
>> -		printk(KERN_INFO "%s:\tUSB Remote wakeup capable\n", dev->name);
>> +		em28xx_info("\tUSB Remote wakeup capable\n");
>>  
>>  	if (em_eeprom->chip_conf & 1 << 2)
>> -		printk(KERN_INFO "%s:\tUSB Self power capable\n", dev->name);
>> +		em28xx_info("\tUSB Self power capable\n");
>>  
>>  	switch (em_eeprom->chip_conf & 0x3) {
>>  	case 0:
>> -		printk(KERN_INFO "%s:\t500mA max power\n", dev->name);
>> +		em28xx_info("\t500mA max power\n");
>>  		break;
>>  	case 1:
>> -		printk(KERN_INFO "%s:\t400mA max power\n", dev->name);
>> +		em28xx_info("\t400mA max power\n");
>>  		break;
>>  	case 2:
>> -		printk(KERN_INFO "%s:\t300mA max power\n", dev->name);
>> +		em28xx_info("\t300mA max power\n");
>>  		break;
>>  	case 3:
>> -		printk(KERN_INFO "%s:\t200mA max power\n", dev->name);
>> +		em28xx_info("\t200mA max power\n");
>>  		break;
>>  	}
>> -	printk(KERN_INFO "%s:\tTable at 0x%02x, strings=0x%04x, 0x%04x, 0x%04x\n",
>> -				dev->name,
>> -				em_eeprom->string_idx_table,
>> -				em_eeprom->string1,
>> -				em_eeprom->string2,
>> -				em_eeprom->string3);
>> +	em28xx_info("\tTable at offset 0x%02x, strings=0x%04x, 0x%04x, 0x%04x\n",
>> +		    em_eeprom->string_idx_table,
>> +		    em_eeprom->string1,
>> +		    em_eeprom->string2,
>> +		    em_eeprom->string3);
>>  
>>  	return 0;
>>  }
>> @@ -570,8 +563,8 @@ void em28xx_do_i2c_scan(struct em28xx *dev)
>>  		if (rc < 0)
>>  			continue;
>>  		i2c_devicelist[i] = i;
>> -		printk(KERN_INFO "%s: found i2c device @ 0x%x [%s]\n",
>> -		       dev->name, i << 1, i2c_devs[i] ? i2c_devs[i] : "???");
>> +		em28xx_info("found i2c device @ 0x%x [%s]\n",
>> +			    i << 1, i2c_devs[i] ? i2c_devs[i] : "???");
>>  	}
>>  
>>  	dev->i2c_hash = em28xx_hash_mem(i2c_devicelist,
>


  reply	other threads:[~2013-03-04 18:34 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-03-03 19:37 [PATCH v2 00/11] em28xx: i2c debugging cleanups and support for newer eeproms Frank Schäfer
2013-03-03 19:37 ` [PATCH v2 01/11] em28xx-i2c: replace printk() with the corresponding em28xx macros Frank Schäfer
2013-03-04 18:09   ` Mauro Carvalho Chehab
2013-03-04 18:27     ` Frank Schäfer [this message]
2013-03-04 18:46       ` Mauro Carvalho Chehab
2013-03-03 19:37 ` [PATCH v2 02/11] em28xx-i2c: get rid of the dprintk2 macro Frank Schäfer
2013-03-03 19:37 ` [PATCH v2 03/11] em28xx-i2c: also print debug messages at debug level 1 Frank Schäfer
2013-03-03 19:37 ` [PATCH v2 04/11] em28xx: do not interpret eeprom content if eeprom key is invalid Frank Schäfer
2013-03-03 19:37 ` [PATCH v2 05/11] em28xx: fix eeprom data endianess Frank Schäfer
2013-03-03 19:37 ` [PATCH v2 06/11] em28xx: make sure we are at i2c bus A when calling em28xx_i2c_register() Frank Schäfer
2013-03-04 19:14   ` Mauro Carvalho Chehab
2013-03-04 21:24     ` Frank Schäfer
2013-03-05  2:31       ` Mauro Carvalho Chehab
2013-03-03 19:37 ` [PATCH v2 07/11] em28xx: add basic support for eeproms with 16 bit address width Frank Schäfer
2013-03-03 19:37 ` [PATCH v2 08/11] em28xx: add helper function for reading data blocks from i2c clients Frank Schäfer
2013-03-03 19:37 ` [PATCH v2 09/11] em28xx: do not store eeprom content permanently Frank Schäfer
2013-03-03 19:37 ` [PATCH v2 10/11] em28xx: extract the device configuration dataset from eeproms with 16 bit address width Frank Schäfer
2013-03-03 19:37 ` [PATCH v2 11/11] em28xx: enable tveeprom for device Hauppauge HVR-930C Frank Schäfer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5134E780.4000700@googlemail.com \
    --to=fschaefer.oss@googlemail.com \
    --cc=linux-media@vger.kernel.org \
    --cc=mchehab@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).