diff mbox

[V7,2/4] MTD: pxa3xx_nand: convert all printk into dev_*

Message ID 1310481336-13230-3-git-send-email-leiwen@marvell.com (mailing list archive)
State New, archived
Headers show

Commit Message

Lei Wen July 12, 2011, 2:35 p.m. UTC
Signed-off-by: Lei Wen <leiwen@marvell.com>
---
 drivers/mtd/nand/pxa3xx_nand.c |   27 ++++++++++++++++-----------
 1 files changed, 16 insertions(+), 11 deletions(-)

Comments

Sergei Shtylyov July 13, 2011, 10:57 a.m. UTC | #1
Hello.

On 12-07-2011 18:35, Lei Wen wrote:

> Signed-off-by: Lei Wen<leiwen@marvell.com>
> ---
>   drivers/mtd/nand/pxa3xx_nand.c |   27 ++++++++++++++++-----------
>   1 files changed, 16 insertions(+), 11 deletions(-)

> diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
> index 1efecf7..afe14bd 100644
> --- a/drivers/mtd/nand/pxa3xx_nand.c
> +++ b/drivers/mtd/nand/pxa3xx_nand.c
[...]
> @@ -774,11 +774,15 @@ static int pxa3xx_nand_config_flash(struct pxa3xx_nand_info *info,
>   	struct pxa3xx_nand_platform_data *pdata = pdev->dev.platform_data;
>   	uint32_t ndcr = 0x0; /* enable all interrupts */
>
> -	if (f->page_size != 2048 && f->page_size != 512)
> +	if (f->page_size != 2048 && f->page_size != 512) {
> +		dev_err(&pdev->dev, "Current only support 2048 and 512 size\n");
>   		return -EINVAL;
> +	}
>
> -	if (f->flash_width != 16 && f->flash_width != 8)
> +	if (f->flash_width != 16 && f->flash_width != 8) {
> +		dev_err(&pdev->dev, "Only support 8bit and 16 bit!\n");

    So this patch not only converts printk() to dev_*() calls, it also adds 
some dev_err() calls? This should have been mentioned in the change long at 
least...

>   		return -EINVAL;
> +	}
>
>   	/* calculate flash information */
>   	info->cmdset =&default_cmdset;
> @@ -898,7 +902,7 @@ static int pxa3xx_nand_scan(struct mtd_info *mtd)
>   	if (!ret) {
>   		kfree(mtd);
>   		info->mtd = NULL;
> -		printk(KERN_INFO "There is no nand chip on cs 0!\n");
> +		dev_info(&info->pdev->dev, "There is no nand chip on cs 0!\n");
>
>   		return -EINVAL;
>   	}
> @@ -906,11 +910,12 @@ static int pxa3xx_nand_scan(struct mtd_info *mtd)
>   	chip->cmdfunc(mtd, NAND_CMD_READID, 0, 0);
>   	id = *((uint16_t *)(info->data_buff));
>   	if (id != 0)
> -		printk(KERN_INFO "Detect a flash id %x\n", id);
> +		dev_info(&info->pdev->dev, "Detect a flash id %x\n", id);
>   	else {
>   		kfree(mtd);
>   		info->mtd = NULL;
> -		printk(KERN_WARNING "Read out ID 0, potential timing set wrong!!\n");
> +		dev_warn(&info->pdev->dev, "Read out ID 0, "
> +				"potential timing set wrong!!\n");

    Don't break the string -- this prevents grepping for it.

WBR, Sergei
Lei Wen July 13, 2011, 12:41 p.m. UTC | #2
Hi Sergei,

On Wed, Jul 13, 2011 at 6:57 PM, Sergei Shtylyov <sshtylyov@mvista.com> wrote:
> Hello.
>
> On 12-07-2011 18:35, Lei Wen wrote:
>
>> Signed-off-by: Lei Wen<leiwen@marvell.com>
>> ---
>>  drivers/mtd/nand/pxa3xx_nand.c |   27 ++++++++++++++++-----------
>>  1 files changed, 16 insertions(+), 11 deletions(-)
>
>> diff --git a/drivers/mtd/nand/pxa3xx_nand.c
>> b/drivers/mtd/nand/pxa3xx_nand.c
>> index 1efecf7..afe14bd 100644
>> --- a/drivers/mtd/nand/pxa3xx_nand.c
>> +++ b/drivers/mtd/nand/pxa3xx_nand.c
>
> [...]
>>
>> @@ -774,11 +774,15 @@ static int pxa3xx_nand_config_flash(struct
>> pxa3xx_nand_info *info,
>>        struct pxa3xx_nand_platform_data *pdata = pdev->dev.platform_data;
>>        uint32_t ndcr = 0x0; /* enable all interrupts */
>>
>> -       if (f->page_size != 2048 && f->page_size != 512)
>> +       if (f->page_size != 2048 && f->page_size != 512) {
>> +               dev_err(&pdev->dev, "Current only support 2048 and 512
>> size\n");
>>                return -EINVAL;
>> +       }
>>
>> -       if (f->flash_width != 16 && f->flash_width != 8)
>> +       if (f->flash_width != 16 && f->flash_width != 8) {
>> +               dev_err(&pdev->dev, "Only support 8bit and 16 bit!\n");
>
>   So this patch not only converts printk() to dev_*() calls, it also adds
> some dev_err() calls? This should have been mentioned in the change long at
> least...

What do you mean by "chang long"? Do you mean the cover letter or the
patch name?
Certainly it is a good suggest to show it in the change version list.

>
>>                return -EINVAL;
>> +       }
>>
>>        /* calculate flash information */
>>        info->cmdset =&default_cmdset;
>> @@ -898,7 +902,7 @@ static int pxa3xx_nand_scan(struct mtd_info *mtd)
>>        if (!ret) {
>>                kfree(mtd);
>>                info->mtd = NULL;
>> -               printk(KERN_INFO "There is no nand chip on cs 0!\n");
>> +               dev_info(&info->pdev->dev, "There is no nand chip on cs
>> 0!\n");
>>
>>                return -EINVAL;
>>        }
>> @@ -906,11 +910,12 @@ static int pxa3xx_nand_scan(struct mtd_info *mtd)
>>        chip->cmdfunc(mtd, NAND_CMD_READID, 0, 0);
>>        id = *((uint16_t *)(info->data_buff));
>>        if (id != 0)
>> -               printk(KERN_INFO "Detect a flash id %x\n", id);
>> +               dev_info(&info->pdev->dev, "Detect a flash id %x\n", id);
>>        else {
>>                kfree(mtd);
>>                info->mtd = NULL;
>> -               printk(KERN_WARNING "Read out ID 0, potential timing set
>> wrong!!\n");
>> +               dev_warn(&info->pdev->dev, "Read out ID 0, "
>> +                               "potential timing set wrong!!\n");
>
>   Don't break the string -- this prevents grepping for it.

Thanks, I would fix it.

Best regards,
Lei
Sergei Shtylyov July 13, 2011, 2:35 p.m. UTC | #3
Hello.

Lei Wen wrote:

>>> Signed-off-by: Lei Wen<leiwen@marvell.com>
>>> ---
>>>  drivers/mtd/nand/pxa3xx_nand.c |   27 ++++++++++++++++-----------
>>>  1 files changed, 16 insertions(+), 11 deletions(-)
>>> diff --git a/drivers/mtd/nand/pxa3xx_nand.c
>>> b/drivers/mtd/nand/pxa3xx_nand.c
>>> index 1efecf7..afe14bd 100644
>>> --- a/drivers/mtd/nand/pxa3xx_nand.c
>>> +++ b/drivers/mtd/nand/pxa3xx_nand.c
>> [...]
>>> @@ -774,11 +774,15 @@ static int pxa3xx_nand_config_flash(struct
>>> pxa3xx_nand_info *info,
>>>        struct pxa3xx_nand_platform_data *pdata = pdev->dev.platform_data;
>>>        uint32_t ndcr = 0x0; /* enable all interrupts */
>>>
>>> -       if (f->page_size != 2048 && f->page_size != 512)
>>> +       if (f->page_size != 2048 && f->page_size != 512) {
>>> +               dev_err(&pdev->dev, "Current only support 2048 and 512
>>> size\n");
>>>                return -EINVAL;
>>> +       }
>>>
>>> -       if (f->flash_width != 16 && f->flash_width != 8)
>>> +       if (f->flash_width != 16 && f->flash_width != 8) {
>>> +               dev_err(&pdev->dev, "Only support 8bit and 16 bit!\n");

>>   So this patch not only converts printk() to dev_*() calls, it also adds
>> some dev_err() calls? This should have been mentioned in the change long at
>> least...

> What do you mean by "chang long"? Do you mean the cover letter or the
> patch name?

    I mean the patch description (which in this patch is missing).

> Certainly it is a good suggest to show it in the change version list.

    Don't quite understand what you mean here...

> Best regards,
> Lei

WBR, Sergei
diff mbox

Patch

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index 1efecf7..afe14bd 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -359,7 +359,7 @@  static void handle_data_pio(struct pxa3xx_nand_info *info)
 					DIV_ROUND_UP(info->oob_size, 4));
 		break;
 	default:
-		printk(KERN_ERR "%s: invalid state %d\n", __func__,
+		dev_err(&info->pdev->dev, "%s: invalid state %d\n", __func__,
 				info->state);
 		BUG();
 	}
@@ -385,7 +385,7 @@  static void start_data_dma(struct pxa3xx_nand_info *info)
 		desc->dcmd |= DCMD_INCTRGADDR | DCMD_FLOWSRC;
 		break;
 	default:
-		printk(KERN_ERR "%s: invalid state %d\n", __func__,
+		dev_err(&info->pdev->dev, "%s: invalid state %d\n", __func__,
 				info->state);
 		BUG();
 	}
@@ -616,8 +616,8 @@  static int prepare_command_pool(struct pxa3xx_nand_info *info, int command,
 
 	default:
 		exec_cmd = 0;
-		printk(KERN_ERR "pxa3xx-nand: non-supported"
-			" command %x\n", command);
+		dev_err(&info->pdev->dev, "non-supported command %x\n",
+				command);
 		break;
 	}
 
@@ -646,7 +646,7 @@  static void pxa3xx_nand_cmdfunc(struct mtd_info *mtd, unsigned command,
 		ret = wait_for_completion_timeout(&info->cmd_complete,
 				CHIP_DELAY_TIMEOUT);
 		if (!ret) {
-			printk(KERN_ERR "Wait time out!!!\n");
+			dev_err(&info->pdev->dev, "Wait time out!!!\n");
 			/* Stop State Machine for next command cycle */
 			pxa3xx_nand_stop(info);
 		}
@@ -774,11 +774,15 @@  static int pxa3xx_nand_config_flash(struct pxa3xx_nand_info *info,
 	struct pxa3xx_nand_platform_data *pdata = pdev->dev.platform_data;
 	uint32_t ndcr = 0x0; /* enable all interrupts */
 
-	if (f->page_size != 2048 && f->page_size != 512)
+	if (f->page_size != 2048 && f->page_size != 512) {
+		dev_err(&pdev->dev, "Current only support 2048 and 512 size\n");
 		return -EINVAL;
+	}
 
-	if (f->flash_width != 16 && f->flash_width != 8)
+	if (f->flash_width != 16 && f->flash_width != 8) {
+		dev_err(&pdev->dev, "Only support 8bit and 16 bit!\n");
 		return -EINVAL;
+	}
 
 	/* calculate flash information */
 	info->cmdset = &default_cmdset;
@@ -898,7 +902,7 @@  static int pxa3xx_nand_scan(struct mtd_info *mtd)
 	if (!ret) {
 		kfree(mtd);
 		info->mtd = NULL;
-		printk(KERN_INFO "There is no nand chip on cs 0!\n");
+		dev_info(&info->pdev->dev, "There is no nand chip on cs 0!\n");
 
 		return -EINVAL;
 	}
@@ -906,11 +910,12 @@  static int pxa3xx_nand_scan(struct mtd_info *mtd)
 	chip->cmdfunc(mtd, NAND_CMD_READID, 0, 0);
 	id = *((uint16_t *)(info->data_buff));
 	if (id != 0)
-		printk(KERN_INFO "Detect a flash id %x\n", id);
+		dev_info(&info->pdev->dev, "Detect a flash id %x\n", id);
 	else {
 		kfree(mtd);
 		info->mtd = NULL;
-		printk(KERN_WARNING "Read out ID 0, potential timing set wrong!!\n");
+		dev_warn(&info->pdev->dev, "Read out ID 0, "
+				"potential timing set wrong!!\n");
 
 		return -EINVAL;
 	}
@@ -930,7 +935,7 @@  static int pxa3xx_nand_scan(struct mtd_info *mtd)
 	if (i >= (ARRAY_SIZE(builtin_flash_types) + pdata->num_flash - 1)) {
 		kfree(mtd);
 		info->mtd = NULL;
-		printk(KERN_ERR "ERROR!! flash not defined!!!\n");
+		dev_err(&info->pdev->dev, "ERROR!! flash not defined!!!\n");
 
 		return -EINVAL;
 	}