[PATCH 0/8] the clean-up for gpmi

[PATCH 0/8] the clean-up for gpmi

[Huang Shijie]

This patch set does some clean-up to the gpmi driver.

Huang Shijie (8):
  mtd: gpmi: do not use the local array to do the DMA transfer
  mtd: gpmi: delete the gpmi_pre_bbt_scan
  mtd: gpmi: remove the unused line
  mtd: gpmi: rename the functions from gpmi_nfc_* to gpmi_nand_*
  mtd: gpmi: use devm_ioremap_resource
  mtd: gpmi: use devm_request_irq
  mtd: gpmi: change pr_err to dev_err
  mtd: gpmi: change pr_debug to dev_dbg

 drivers/mtd/nand/gpmi-nand/gpmi-lib.c  |   76 +++++++++++-------
 drivers/mtd/nand/gpmi-nand/gpmi-nand.c |  140 ++++++++++---------------------
 drivers/mtd/nand/gpmi-nand/gpmi-nand.h |    2 -
 3 files changed, 92 insertions(+), 126 deletions(-)

-- 
1.7.2.rc3

[PATCH 1/8] mtd: gpmi: do not use the local array to do the DMA transfer

[Huang Shijie]

The local array feature[] is in the stack. We can see the warning
when we enable the CONFIG_DMA_API_DEBUG:
----------------------------------------------------------
WARNING: at lib/dma-debug.c:950 check_for_stack+0xac/0xf8()
gpmi-nand 112000.gpmi-nand: DMA-API: device driver maps memory fromstack [addr=dc05be34]
Modules linked in:
CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.10.17-16851-g2414a73 #1324
[<80014cbc>] (unwind_backtrace+0x0/0x138) from [<8001251c>] (show_stack+0x10/0x14)
[<8001251c>] (show_stack+0x10/0x14) from [<8002699c>] (warn_slowpath_common+0x4c/0x68)
[<8002699c>] (warn_slowpath_common+0x4c/0x68) from [<80026a4c>] (warn_slowpath_fmt+0x30/0x40)
[<80026a4c>] (warn_slowpath_fmt+0x30/0x40) from [<8028e2f8>] (check_for_stack+0xac/0xf8)
[<8028e2f8>] (check_for_stack+0xac/0xf8) from [<8028e438>] (debug_dma_map_sg+0xf4/0x188)
[<8028e438>] (debug_dma_map_sg+0xf4/0x188) from [<803968d0>] (prepare_data_dma+0xb8/0x1a8)
[<803968d0>] (prepare_data_dma+0xb8/0x1a8) from [<80397b20>] (gpmi_send_data+0x84/0xfc)
[<80397b20>] (gpmi_send_data+0x84/0xfc) from [<8038c2b4>] (nand_onfi_set_features+0x50/0x74)
[<8038c2b4>] (nand_onfi_set_features+0x50/0x74) from [<80397198>] (gpmi_extra_init+0x90/0x170)
[<80397198>] (gpmi_extra_init+0x90/0x170) from [<8039520c>] (gpmi_nand_probe+0x2f8/0xb3c)
[<8039520c>] (gpmi_nand_probe+0x2f8/0xb3c) from [<8031b974>] (platform_drv_probe+0x18/0x1c)
----------------------------------------------------------

The patch uses the kzalloc to allocate the buffer, and free it when
we do not use it anymore.

Signed-off-by: Huang Shijie <b32955@freescale.com>
---
 drivers/mtd/nand/gpmi-nand/gpmi-lib.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-lib.c b/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
index c7a578c..10a6f07 100644
--- a/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
@@ -20,6 +20,7 @@
  */
 #include <linux/delay.h>
 #include <linux/clk.h>
+#include <linux/slab.h>
 
 #include "gpmi-nand.h"
 #include "gpmi-regs.h"
@@ -911,10 +912,14 @@ static int enable_edo_mode(struct gpmi_nand_data *this, int mode)
 	struct resources  *r = &this->resources;
 	struct nand_chip *nand = &this->nand;
 	struct mtd_info	 *mtd = &this->mtd;
-	uint8_t feature[ONFI_SUBFEATURE_PARAM_LEN] = {};
+	uint8_t *feature;
 	unsigned long rate;
 	int ret;
 
+	feature = kzalloc(ONFI_SUBFEATURE_PARAM_LEN, GFP_KERNEL);
+	if (!feature)
+		return -ENOMEM;
+
 	nand->select_chip(mtd, 0);
 
 	/* [1] send SET FEATURE commond to NAND */
@@ -942,11 +947,13 @@ static int enable_edo_mode(struct gpmi_nand_data *this, int mode)
 
 	this->flags |= GPMI_ASYNC_EDO_ENABLED;
 	this->timing_mode = mode;
+	kfree(feature);
 	dev_info(this->dev, "enable the asynchronous EDO mode %d\n", mode);
 	return 0;
 
 err_out:
 	nand->select_chip(mtd, -1);
+	kfree(feature);
 	dev_err(this->dev, "mode:%d ,failed in set feature.\n", mode);
 	return -EINVAL;
 }
-- 
1.7.2.rc3

Re: [PATCH 1/8] mtd: gpmi: do not use the local array to do the DMA transfer

[Ezequiel Garcia]

On Thu, Nov 14, 2013 at 02:25:44PM +0800, Huang Shijie wrote:
> The local array feature[] is in the stack. We can see the warning
> when we enable the CONFIG_DMA_API_DEBUG:
> ----------------------------------------------------------
> WARNING: at lib/dma-debug.c:950 check_for_stack+0xac/0xf8()
> gpmi-nand 112000.gpmi-nand: DMA-API: device driver maps memory fromstack [addr=dc05be34]
> Modules linked in:
> CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.10.17-16851-g2414a73 #1324
> [<80014cbc>] (unwind_backtrace+0x0/0x138) from [<8001251c>] (show_stack+0x10/0x14)
> [<8001251c>] (show_stack+0x10/0x14) from [<8002699c>] (warn_slowpath_common+0x4c/0x68)
> [<8002699c>] (warn_slowpath_common+0x4c/0x68) from [<80026a4c>] (warn_slowpath_fmt+0x30/0x40)
> [<80026a4c>] (warn_slowpath_fmt+0x30/0x40) from [<8028e2f8>] (check_for_stack+0xac/0xf8)
> [<8028e2f8>] (check_for_stack+0xac/0xf8) from [<8028e438>] (debug_dma_map_sg+0xf4/0x188)
> [<8028e438>] (debug_dma_map_sg+0xf4/0x188) from [<803968d0>] (prepare_data_dma+0xb8/0x1a8)
> [<803968d0>] (prepare_data_dma+0xb8/0x1a8) from [<80397b20>] (gpmi_send_data+0x84/0xfc)
> [<80397b20>] (gpmi_send_data+0x84/0xfc) from [<8038c2b4>] (nand_onfi_set_features+0x50/0x74)
> [<8038c2b4>] (nand_onfi_set_features+0x50/0x74) from [<80397198>] (gpmi_extra_init+0x90/0x170)
> [<80397198>] (gpmi_extra_init+0x90/0x170) from [<8039520c>] (gpmi_nand_probe+0x2f8/0xb3c)
> [<8039520c>] (gpmi_nand_probe+0x2f8/0xb3c) from [<8031b974>] (platform_drv_probe+0x18/0x1c)
> ----------------------------------------------------------
> 
> The patch uses the kzalloc to allocate the buffer, and free it when
> we do not use it anymore.
> 
> Signed-off-by: Huang Shijie <b32955@freescale.com>
> ---
>  drivers/mtd/nand/gpmi-nand/gpmi-lib.c |    9 ++++++++-
>  1 files changed, 8 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-lib.c b/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
> index c7a578c..10a6f07 100644
> --- a/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
> @@ -20,6 +20,7 @@
>   */
>  #include <linux/delay.h>
>  #include <linux/clk.h>
> +#include <linux/slab.h>
>  
>  #include "gpmi-nand.h"
>  #include "gpmi-regs.h"
> @@ -911,10 +912,14 @@ static int enable_edo_mode(struct gpmi_nand_data *this, int mode)
>  	struct resources  *r = &this->resources;
>  	struct nand_chip *nand = &this->nand;
>  	struct mtd_info	 *mtd = &this->mtd;
> -	uint8_t feature[ONFI_SUBFEATURE_PARAM_LEN] = {};
> +	uint8_t *feature;
>  	unsigned long rate;
>  	int ret;
>  

ONFI_SUBFEATURE_PARAM_LEN is only 4, so the most natural choice is to
allocate that in the stack. I think you should add a comment here
explaining that this buffer needs to be kmallocated for dma purposes.

Just a nitpick, to avoid people asking why you're doing the allocation
for just 4 bytes.

> +	feature = kzalloc(ONFI_SUBFEATURE_PARAM_LEN, GFP_KERNEL);
> +	if (!feature)
> +		return -ENOMEM;
> +
>  	nand->select_chip(mtd, 0);
>  
>  	/* [1] send SET FEATURE commond to NAND */
> @@ -942,11 +947,13 @@ static int enable_edo_mode(struct gpmi_nand_data *this, int mode)
>  
>  	this->flags |= GPMI_ASYNC_EDO_ENABLED;
>  	this->timing_mode = mode;
> +	kfree(feature);
>  	dev_info(this->dev, "enable the asynchronous EDO mode %d\n", mode);
>  	return 0;
>  
>  err_out:
>  	nand->select_chip(mtd, -1);
> +	kfree(feature);
>  	dev_err(this->dev, "mode:%d ,failed in set feature.\n", mode);
>  	return -EINVAL;
>  }
> -- 
> 1.7.2.rc3
> 
> 
> 
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/

-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

Re: [PATCH 1/8] mtd: gpmi: do not use the local array to do the DMA transfer

[Huang Shijie]

On Thu, Nov 14, 2013 at 10:46:31AM -0300, Ezequiel Garcia wrote:
> On Thu, Nov 14, 2013 at 02:25:44PM +0800, Huang Shijie wrote:
> 
> ONFI_SUBFEATURE_PARAM_LEN is only 4, so the most natural choice is to
> allocate that in the stack. I think you should add a comment here
> explaining that this buffer needs to be kmallocated for dma purposes.
> 
> Just a nitpick, to avoid people asking why you're doing the allocation
> for just 4 bytes.
i just do not like the warning. 

thanks
Huang Shijie

Re: [PATCH 1/8] mtd: gpmi: do not use the local array to do the DMA transfer

[David Woodhouse]

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

On Thu, 2013-11-14 at 22:53 -0500, Huang Shijie wrote:
> ...

Huang, your system clock is *still* set incorrectly, and your messages
are claiming to have been sent from 11 hours in the future.

Unfortunately, a number of email clients sort on the time which a
message claims to have been sent, rather than the time it was actually
received (which would make much more sense). So your clock
misconfiguration makes these threads appear permanently at the bottom of
a mailbox... to the detriment of other active conversations. It confuses
the Android mailer so much that it keeps telling me I have *new* mail
from you, over and over again.

Please could you fix your clock?

You *didn't* actually send that message through a time machine from
22:53 US Eastern Time as it claims, did you?

-- 
dwmw2


[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5745 bytes --]

Re: [PATCH 1/8] mtd: gpmi: do not use the local array to do the DMA transfer

[Huang Shijie]

于 2013年11月15日 00:23, David Woodhouse 写道:
> Please could you fix your clock?
ok, i will fix the clock.

thanks
Huang Shijie

[PATCH 2/8] mtd: gpmi: delete the gpmi_pre_bbt_scan

[Huang Shijie]

We do not scan the BBT after we call the gpmi_pre_bbt_scan,
so it has lost the meaning of existence.

This patch merges this function into gpmi_init_last, and delete it.
This patch does not change any logic.

Signed-off-by: Huang Shijie <b32955@freescale.com>
---
 drivers/mtd/nand/gpmi-nand/gpmi-nand.c |   20 +++++---------------
 1 files changed, 5 insertions(+), 15 deletions(-)

diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
index 739f131..9bce04b 100644
--- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
@@ -1562,19 +1562,6 @@ static int gpmi_set_geometry(struct gpmi_nand_data *this)
 	return gpmi_alloc_dma_buffer(this);
 }
 
-static int gpmi_pre_bbt_scan(struct gpmi_nand_data  *this)
-{
-	/* Set up swap_block_mark, must be set before the gpmi_set_geometry() */
-	if (GPMI_IS_MX23(this))
-		this->swap_block_mark = false;
-	else
-		this->swap_block_mark = true;
-
-	/* Set up the medium geometry */
-	return gpmi_set_geometry(this);
-
-}
-
 static void gpmi_nfc_exit(struct gpmi_nand_data *this)
 {
 	nand_release(&this->mtd);
@@ -1589,8 +1576,11 @@ static int gpmi_init_last(struct gpmi_nand_data *this)
 	struct bch_geometry *bch_geo = &this->bch_geometry;
 	int ret;
 
-	/* Prepare for the BBT scan. */
-	ret = gpmi_pre_bbt_scan(this);
+	/* Set up swap_block_mark, must be set before the gpmi_set_geometry() */
+	this->swap_block_mark = !GPMI_IS_MX23(this);
+
+	/* Set up the medium geometry */
+	ret = gpmi_set_geometry(this);
 	if (ret)
 		return ret;
 
-- 
1.7.2.rc3

[PATCH 3/8] mtd: gpmi: remove the unused line

[Huang Shijie]

We do not use the chip->oob_poi in the mx23_write_transcription_stamp.
So remove the unused line.

Signed-off-by: Huang Shijie <b32955@freescale.com>
---
 drivers/mtd/nand/gpmi-nand/gpmi-nand.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
index 9bce04b..1faf198 100644
--- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
@@ -1439,7 +1439,6 @@ static int mx23_write_transcription_stamp(struct gpmi_nand_data *this)
 
 	/* Write the NCB fingerprint into the page buffer. */
 	memset(buffer, ~0, mtd->writesize);
-	memset(chip->oob_poi, ~0, mtd->oobsize);
 	memcpy(buffer + 12, fingerprint, strlen(fingerprint));
 
 	/* Loop through the first search area, writing NCB fingerprints. */
-- 
1.7.2.rc3

[PATCH 4/8] mtd: gpmi: rename the functions from gpmi_nfc_* to gpmi_nand_*

[Huang Shijie]

The gpmi_nfc_* is the legacy name. In order to avoid the confusion,
The patch renames the gpmi_nfc_* functions to gpmi_nand_*.

Signed-off-by: Huang Shijie <b32955@freescale.com>
---
 drivers/mtd/nand/gpmi-nand/gpmi-nand.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
index 1faf198..36ef60a 100644
--- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
@@ -1561,7 +1561,7 @@ static int gpmi_set_geometry(struct gpmi_nand_data *this)
 	return gpmi_alloc_dma_buffer(this);
 }
 
-static void gpmi_nfc_exit(struct gpmi_nand_data *this)
+static void gpmi_nand_exit(struct gpmi_nand_data *this)
 {
 	nand_release(&this->mtd);
 	gpmi_free_dma_buffer(this);
@@ -1604,7 +1604,7 @@ static int gpmi_init_last(struct gpmi_nand_data *this)
 	return 0;
 }
 
-static int gpmi_nfc_init(struct gpmi_nand_data *this)
+static int gpmi_nand_init(struct gpmi_nand_data *this)
 {
 	struct mtd_info  *mtd = &this->mtd;
 	struct nand_chip *chip = &this->nand;
@@ -1668,7 +1668,7 @@ static int gpmi_nfc_init(struct gpmi_nand_data *this)
 	return 0;
 
 err_out:
-	gpmi_nfc_exit(this);
+	gpmi_nand_exit(this);
 	return ret;
 }
 
@@ -1725,7 +1725,7 @@ static int gpmi_nand_probe(struct platform_device *pdev)
 	if (ret)
 		goto exit_nfc_init;
 
-	ret = gpmi_nfc_init(this);
+	ret = gpmi_nand_init(this);
 	if (ret)
 		goto exit_nfc_init;
 
@@ -1745,7 +1745,7 @@ static int gpmi_nand_remove(struct platform_device *pdev)
 {
 	struct gpmi_nand_data *this = platform_get_drvdata(pdev);
 
-	gpmi_nfc_exit(this);
+	gpmi_nand_exit(this);
 	release_resources(this);
 	return 0;
 }
-- 
1.7.2.rc3

[PATCH 5/8] mtd: gpmi: use devm_ioremap_resource

[Huang Shijie]

Use the devm_ioremap_resource to simplify the code.

Signed-off-by: Huang Shijie <b32955@freescale.com>
---
 drivers/mtd/nand/gpmi-nand/gpmi-nand.c |   26 +++-----------------------
 1 files changed, 3 insertions(+), 23 deletions(-)

diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
index 36ef60a..1f99038 100644
--- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
@@ -486,16 +486,9 @@ static int acquire_register_block(struct gpmi_nand_data *this,
 	void __iomem *p;
 
 	r = platform_get_resource_byname(pdev, IORESOURCE_MEM, res_name);
-	if (!r) {
-		pr_err("Can't get resource for %s\n", res_name);
-		return -ENODEV;
-	}
-
-	p = ioremap(r->start, resource_size(r));
-	if (!p) {
-		pr_err("Can't remap %s\n", res_name);
-		return -ENOMEM;
-	}
+	p = devm_ioremap_resource(&pdev->dev, r);
+	if (IS_ERR(p))
+		return PTR_ERR(p);
 
 	if (!strcmp(res_name, GPMI_NAND_GPMI_REGS_ADDR_RES_NAME))
 		res->gpmi_regs = p;
@@ -507,17 +500,6 @@ static int acquire_register_block(struct gpmi_nand_data *this,
 	return 0;
 }
 
-static void release_register_block(struct gpmi_nand_data *this)
-{
-	struct resources *res = &this->resources;
-	if (res->gpmi_regs)
-		iounmap(res->gpmi_regs);
-	if (res->bch_regs)
-		iounmap(res->bch_regs);
-	res->gpmi_regs = NULL;
-	res->bch_regs = NULL;
-}
-
 static int acquire_bch_irq(struct gpmi_nand_data *this, irq_handler_t irq_h)
 {
 	struct platform_device *pdev = this->pdev;
@@ -665,13 +647,11 @@ exit_clock:
 exit_dma_channels:
 	release_bch_irq(this);
 exit_regs:
-	release_register_block(this);
 	return ret;
 }
 
 static void release_resources(struct gpmi_nand_data *this)
 {
-	release_register_block(this);
 	release_bch_irq(this);
 	release_dma_channels(this);
 }
-- 
1.7.2.rc3

Re: [PATCH 5/8] mtd: gpmi: use devm_ioremap_resource

[Ezequiel Garcia]

On Thu, Nov 14, 2013 at 02:25:48PM +0800, Huang Shijie wrote:
> Use the devm_ioremap_resource to simplify the code.
> 
> Signed-off-by: Huang Shijie <b32955@freescale.com>
> ---
>  drivers/mtd/nand/gpmi-nand/gpmi-nand.c |   26 +++-----------------------
>  1 files changed, 3 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> index 36ef60a..1f99038 100644
> --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> @@ -486,16 +486,9 @@ static int acquire_register_block(struct gpmi_nand_data *this,
>  	void __iomem *p;
>  
>  	r = platform_get_resource_byname(pdev, IORESOURCE_MEM, res_name);
> -	if (!r) {
> -		pr_err("Can't get resource for %s\n", res_name);
> -		return -ENODEV;
> -	}
> -
> -	p = ioremap(r->start, resource_size(r));
> -	if (!p) {
> -		pr_err("Can't remap %s\n", res_name);
> -		return -ENOMEM;
> -	}
> +	p = devm_ioremap_resource(&pdev->dev, r);
> +	if (IS_ERR(p))
> +		return PTR_ERR(p);

Hm... devm_ioremap_resource() is not "pin-to-pin" replacement for the
combination of platform_get_resource + ioremap. Instead, you're adding
a request_memory() call.

So, this is an improvement, and it's fine to do so. I'm just warning
about a change in the driver's behavior.
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

Re: [PATCH 5/8] mtd: gpmi: use devm_ioremap_resource

[Huang Shijie]

On Thu, Nov 14, 2013 at 10:50:34AM -0300, Ezequiel Garcia wrote:
> So, this is an improvement, and it's fine to do so. I'm just warning
> about a change in the driver's behavior.
thanks, got it.


Huang Shijie

[PATCH 6/8] mtd: gpmi: use devm_request_irq

[Huang Shijie]

Use devm_request_irq to simplify the code.
Also remove the unused fields of structure resources{}.

Signed-off-by: Huang Shijie <b32955@freescale.com>
---
 drivers/mtd/nand/gpmi-nand/gpmi-nand.c |   27 +++++----------------------
 drivers/mtd/nand/gpmi-nand/gpmi-nand.h |    2 --
 2 files changed, 5 insertions(+), 24 deletions(-)

diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
index 1f99038..5617876 100644
--- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
@@ -503,7 +503,6 @@ static int acquire_register_block(struct gpmi_nand_data *this,
 static int acquire_bch_irq(struct gpmi_nand_data *this, irq_handler_t irq_h)
 {
 	struct platform_device *pdev = this->pdev;
-	struct resources *res = &this->resources;
 	const char *res_name = GPMI_NAND_BCH_INTERRUPT_RES_NAME;
 	struct resource *r;
 	int err;
@@ -514,24 +513,11 @@ static int acquire_bch_irq(struct gpmi_nand_data *this, irq_handler_t irq_h)
 		return -ENODEV;
 	}
 
-	err = request_irq(r->start, irq_h, 0, res_name, this);
-	if (err) {
-		pr_err("Can't own %s\n", res_name);
-		return err;
-	}
-
-	res->bch_low_interrupt = r->start;
-	res->bch_high_interrupt = r->end;
-	return 0;
-}
-
-static void release_bch_irq(struct gpmi_nand_data *this)
-{
-	struct resources *res = &this->resources;
-	int i = res->bch_low_interrupt;
+	err = devm_request_irq(this->dev, r->start, irq_h, 0, res_name, this);
+	if (err)
+		dev_err(this->dev, "error requesting BCH IRQ\n");
 
-	for (; i <= res->bch_high_interrupt; i++)
-		free_irq(i, this);
+	return err;
 }
 
 static void release_dma_channels(struct gpmi_nand_data *this)
@@ -635,7 +621,7 @@ static int acquire_resources(struct gpmi_nand_data *this)
 
 	ret = acquire_dma_channels(this);
 	if (ret)
-		goto exit_dma_channels;
+		goto exit_regs;
 
 	ret = gpmi_get_clks(this);
 	if (ret)
@@ -644,15 +630,12 @@ static int acquire_resources(struct gpmi_nand_data *this)
 
 exit_clock:
 	release_dma_channels(this);
-exit_dma_channels:
-	release_bch_irq(this);
 exit_regs:
 	return ret;
 }
 
 static void release_resources(struct gpmi_nand_data *this)
 {
-	release_bch_irq(this);
 	release_dma_channels(this);
 }
 
diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.h b/drivers/mtd/nand/gpmi-nand/gpmi-nand.h
index a7685e3..4c801fa 100644
--- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.h
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.h
@@ -26,8 +26,6 @@
 struct resources {
 	void __iomem  *gpmi_regs;
 	void __iomem  *bch_regs;
-	unsigned int  bch_low_interrupt;
-	unsigned int  bch_high_interrupt;
 	unsigned int  dma_low_channel;
 	unsigned int  dma_high_channel;
 	struct clk    *clock[GPMI_CLK_MAX];
-- 
1.7.2.rc3

[PATCH 7/8] mtd: gpmi: change pr_err to dev_err

[Huang Shijie]

There are pr_err and dev_err in the gpmi driver now.
It makes people confused.

This patch changes all the pr_err to dev_err except the one
in the gpmi_reset_block().

Signed-off-by: Huang Shijie <b32955@freescale.com>
---
 drivers/mtd/nand/gpmi-nand/gpmi-lib.c  |   67 ++++++++++++++++++-------------
 drivers/mtd/nand/gpmi-nand/gpmi-nand.c |   43 ++++++++++----------
 2 files changed, 61 insertions(+), 49 deletions(-)

diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-lib.c b/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
index 10a6f07..82ac4a3 100644
--- a/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
@@ -208,30 +208,41 @@ void gpmi_dump_info(struct gpmi_nand_data *this)
 	u32 reg;
 	int i;
 
-	pr_err("Show GPMI registers :\n");
+	dev_err(this->dev, "Show GPMI registers :\n");
 	for (i = 0; i <= HW_GPMI_DEBUG / 0x10 + 1; i++) {
 		reg = readl(r->gpmi_regs + i * 0x10);
-		pr_err("offset 0x%.3x : 0x%.8x\n", i * 0x10, reg);
+		dev_err(this->dev, "offset 0x%.3x : 0x%.8x\n", i * 0x10, reg);
 	}
 
 	/* start to print out the BCH info */
-	pr_err("Show BCH registers :\n");
+	dev_err(this->dev, "Show BCH registers :\n");
 	for (i = 0; i <= HW_BCH_VERSION / 0x10 + 1; i++) {
 		reg = readl(r->bch_regs + i * 0x10);
-		pr_err("offset 0x%.3x : 0x%.8x\n", i * 0x10, reg);
+		dev_err(this->dev, "offset 0x%.3x : 0x%.8x\n", i * 0x10, reg);
 	}
-	pr_err("BCH Geometry :\n");
-	pr_err("GF length              : %u\n", geo->gf_len);
-	pr_err("ECC Strength           : %u\n", geo->ecc_strength);
-	pr_err("Page Size in Bytes     : %u\n", geo->page_size);
-	pr_err("Metadata Size in Bytes : %u\n", geo->metadata_size);
-	pr_err("ECC Chunk Size in Bytes: %u\n", geo->ecc_chunk_size);
-	pr_err("ECC Chunk Count        : %u\n", geo->ecc_chunk_count);
-	pr_err("Payload Size in Bytes  : %u\n", geo->payload_size);
-	pr_err("Auxiliary Size in Bytes: %u\n", geo->auxiliary_size);
-	pr_err("Auxiliary Status Offset: %u\n", geo->auxiliary_status_offset);
-	pr_err("Block Mark Byte Offset : %u\n", geo->block_mark_byte_offset);
-	pr_err("Block Mark Bit Offset  : %u\n", geo->block_mark_bit_offset);
+	dev_err(this->dev, "BCH Geometry :\n"
+		"GF length              : %u\n"
+		"ECC Strength           : %u\n"
+		"Page Size in Bytes     : %u\n"
+		"Metadata Size in Bytes : %u\n"
+		"ECC Chunk Size in Bytes: %u\n"
+		"ECC Chunk Count        : %u\n"
+		"Payload Size in Bytes  : %u\n"
+		"Auxiliary Size in Bytes: %u\n"
+		"Auxiliary Status Offset: %u\n"
+		"Block Mark Byte Offset : %u\n"
+		"Block Mark Bit Offset  : %u\n",
+		geo->gf_len,
+		geo->ecc_strength,
+		geo->page_size,
+		geo->metadata_size,
+		geo->ecc_chunk_size,
+		geo->ecc_chunk_count,
+		geo->payload_size,
+		geo->auxiliary_size,
+		geo->auxiliary_status_offset,
+		geo->block_mark_byte_offset,
+		geo->block_mark_bit_offset);
 }
 
 /* Configures the geometry for BCH.  */
@@ -993,7 +1004,7 @@ void gpmi_begin(struct gpmi_nand_data *this)
 	/* Enable the clock. */
 	ret = gpmi_enable_clk(this);
 	if (ret) {
-		pr_err("We failed in enable the clk\n");
+		dev_err(this->dev, "We failed in enable the clk\n");
 		goto err_out;
 	}
 
@@ -1097,7 +1108,7 @@ int gpmi_is_ready(struct gpmi_nand_data *this, unsigned chip)
 		mask = MX28_BF_GPMI_STAT_READY_BUSY(1 << chip);
 		reg = readl(r->gpmi_regs + HW_GPMI_STAT);
 	} else
-		pr_err("unknow arch.\n");
+		dev_err(this->dev, "unknow arch.\n");
 	return reg & mask;
 }
 
@@ -1129,7 +1140,7 @@ int gpmi_send_command(struct gpmi_nand_data *this)
 					(struct scatterlist *)pio,
 					ARRAY_SIZE(pio), DMA_TRANS_NONE, 0);
 	if (!desc) {
-		pr_err("step 1 error\n");
+		dev_err(this->dev, "step 1 error\n");
 		return -1;
 	}
 
@@ -1143,7 +1154,7 @@ int gpmi_send_command(struct gpmi_nand_data *this)
 				DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
 
 	if (!desc) {
-		pr_err("step 2 error\n");
+		dev_err(this->dev, "step 2 error\n");
 		return -1;
 	}
 
@@ -1175,7 +1186,7 @@ int gpmi_send_data(struct gpmi_nand_data *this)
 	desc = dmaengine_prep_slave_sg(channel, (struct scatterlist *)pio,
 					ARRAY_SIZE(pio), DMA_TRANS_NONE, 0);
 	if (!desc) {
-		pr_err("step 1 error\n");
+		dev_err(this->dev, "step 1 error\n");
 		return -1;
 	}
 
@@ -1185,7 +1196,7 @@ int gpmi_send_data(struct gpmi_nand_data *this)
 					1, DMA_MEM_TO_DEV,
 					DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
 	if (!desc) {
-		pr_err("step 2 error\n");
+		dev_err(this->dev, "step 2 error\n");
 		return -1;
 	}
 	/* [3] submit the DMA */
@@ -1212,7 +1223,7 @@ int gpmi_read_data(struct gpmi_nand_data *this)
 					(struct scatterlist *)pio,
 					ARRAY_SIZE(pio), DMA_TRANS_NONE, 0);
 	if (!desc) {
-		pr_err("step 1 error\n");
+		dev_err(this->dev, "step 1 error\n");
 		return -1;
 	}
 
@@ -1222,7 +1233,7 @@ int gpmi_read_data(struct gpmi_nand_data *this)
 					1, DMA_DEV_TO_MEM,
 					DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
 	if (!desc) {
-		pr_err("step 2 error\n");
+		dev_err(this->dev, "step 2 error\n");
 		return -1;
 	}
 
@@ -1270,7 +1281,7 @@ int gpmi_send_page(struct gpmi_nand_data *this,
 					ARRAY_SIZE(pio), DMA_TRANS_NONE,
 					DMA_CTRL_ACK);
 	if (!desc) {
-		pr_err("step 2 error\n");
+		dev_err(this->dev, "step 2 error\n");
 		return -1;
 	}
 	set_dma_type(this, DMA_FOR_WRITE_ECC_PAGE);
@@ -1305,7 +1316,7 @@ int gpmi_read_page(struct gpmi_nand_data *this,
 				(struct scatterlist *)pio, 2,
 				DMA_TRANS_NONE, 0);
 	if (!desc) {
-		pr_err("step 1 error\n");
+		dev_err(this->dev, "step 1 error\n");
 		return -1;
 	}
 
@@ -1335,7 +1346,7 @@ int gpmi_read_page(struct gpmi_nand_data *this,
 					ARRAY_SIZE(pio), DMA_TRANS_NONE,
 					DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
 	if (!desc) {
-		pr_err("step 2 error\n");
+		dev_err(this->dev, "step 2 error\n");
 		return -1;
 	}
 
@@ -1356,7 +1367,7 @@ int gpmi_read_page(struct gpmi_nand_data *this,
 				DMA_TRANS_NONE,
 				DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
 	if (!desc) {
-		pr_err("step 3 error\n");
+		dev_err(this->dev, "step 3 error\n");
 		return -1;
 	}
 
diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
index 5617876..85d2be2 100644
--- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
@@ -385,7 +385,7 @@ void prepare_data_dma(struct gpmi_nand_data *this, enum dma_data_direction dr)
 
 		ret = dma_map_sg(this->dev, sgl, 1, dr);
 		if (ret == 0)
-			pr_err("DMA mapping failed.\n");
+			dev_err(this->dev, "DMA mapping failed.\n");
 
 		this->direct_dma_map_ok = false;
 	}
@@ -419,7 +419,7 @@ static void dma_irq_callback(void *param)
 		break;
 
 	default:
-		pr_err("in wrong DMA operation.\n");
+		dev_err(this->dev, "in wrong DMA operation.\n");
 	}
 
 	complete(dma_c);
@@ -441,7 +441,8 @@ int start_dma_without_bch_irq(struct gpmi_nand_data *this,
 	/* Wait for the interrupt from the DMA block. */
 	err = wait_for_completion_timeout(dma_c, msecs_to_jiffies(1000));
 	if (!err) {
-		pr_err("DMA timeout, last DMA :%d\n", this->last_dma_type);
+		dev_err(this->dev, "DMA timeout, last DMA :%d\n",
+			this->last_dma_type);
 		gpmi_dump_info(this);
 		return -ETIMEDOUT;
 	}
@@ -470,7 +471,8 @@ int start_dma_with_bch_irq(struct gpmi_nand_data *this,
 	/* Wait for the interrupt from the BCH block. */
 	err = wait_for_completion_timeout(bch_c, msecs_to_jiffies(1000));
 	if (!err) {
-		pr_err("BCH timeout, last DMA :%d\n", this->last_dma_type);
+		dev_err(this->dev, "BCH timeout, last DMA :%d\n",
+			this->last_dma_type);
 		gpmi_dump_info(this);
 		return -ETIMEDOUT;
 	}
@@ -495,7 +497,7 @@ static int acquire_register_block(struct gpmi_nand_data *this,
 	else if (!strcmp(res_name, GPMI_NAND_BCH_REGS_ADDR_RES_NAME))
 		res->bch_regs = p;
 	else
-		pr_err("unknown resource name : %s\n", res_name);
+		dev_err(this->dev, "unknown resource name : %s\n", res_name);
 
 	return 0;
 }
@@ -509,7 +511,7 @@ static int acquire_bch_irq(struct gpmi_nand_data *this, irq_handler_t irq_h)
 
 	r = platform_get_resource_byname(pdev, IORESOURCE_IRQ, res_name);
 	if (!r) {
-		pr_err("Can't get resource for %s\n", res_name);
+		dev_err(this->dev, "Can't get resource for %s\n", res_name);
 		return -ENODEV;
 	}
 
@@ -538,7 +540,7 @@ static int acquire_dma_channels(struct gpmi_nand_data *this)
 	/* request dma channel */
 	dma_chan = dma_request_slave_channel(&pdev->dev, "rx-tx");
 	if (!dma_chan) {
-		pr_err("Failed to request DMA channel.\n");
+		dev_err(this->dev, "Failed to request DMA channel.\n");
 		goto acquire_err;
 	}
 
@@ -681,8 +683,7 @@ static int read_page_prepare(struct gpmi_nand_data *this,
 						length, DMA_FROM_DEVICE);
 		if (dma_mapping_error(dev, dest_phys)) {
 			if (alt_size < length) {
-				pr_err("%s, Alternate buffer is too small\n",
-					__func__);
+				dev_err(dev, "Alternate buffer is too small\n");
 				return -ENOMEM;
 			}
 			goto map_failed;
@@ -732,8 +733,7 @@ static int send_page_prepare(struct gpmi_nand_data *this,
 						DMA_TO_DEVICE);
 		if (dma_mapping_error(dev, source_phys)) {
 			if (alt_size < length) {
-				pr_err("%s, Alternate buffer is too small\n",
-					__func__);
+				dev_err(dev, "Alternate buffer is too small\n");
 				return -ENOMEM;
 			}
 			goto map_failed;
@@ -821,7 +821,7 @@ static int gpmi_alloc_dma_buffer(struct gpmi_nand_data *this)
 
 error_alloc:
 	gpmi_free_dma_buffer(this);
-	pr_err("Error allocating DMA buffers!\n");
+	dev_err(dev, "Error allocating DMA buffers!\n");
 	return -ENOMEM;
 }
 
@@ -853,7 +853,8 @@ static void gpmi_cmd_ctrl(struct mtd_info *mtd, int data, unsigned int ctrl)
 
 	ret = gpmi_send_command(this);
 	if (ret)
-		pr_err("Chip: %u, Error %d\n", this->current_chip, ret);
+		dev_err(this->dev, "Chip: %u, Error %d\n",
+			this->current_chip, ret);
 
 	this->command_length = 0;
 }
@@ -981,7 +982,7 @@ static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
 					nfc_geo->payload_size,
 					&payload_virt, &payload_phys);
 	if (ret) {
-		pr_err("Inadequate DMA buffer\n");
+		dev_err(this->dev, "Inadequate DMA buffer\n");
 		ret = -ENOMEM;
 		return ret;
 	}
@@ -995,7 +996,7 @@ static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
 			nfc_geo->payload_size,
 			payload_virt, payload_phys);
 	if (ret) {
-		pr_err("Error in ECC-based read: %d\n", ret);
+		dev_err(this->dev, "Error in ECC-based read: %d\n", ret);
 		return ret;
 	}
 
@@ -1081,7 +1082,7 @@ static int gpmi_ecc_write_page(struct mtd_info *mtd, struct nand_chip *chip,
 				nfc_geo->payload_size,
 				&payload_virt, &payload_phys);
 		if (ret) {
-			pr_err("Inadequate payload DMA buffer\n");
+			dev_err(this->dev, "Inadequate payload DMA buffer\n");
 			return 0;
 		}
 
@@ -1091,7 +1092,7 @@ static int gpmi_ecc_write_page(struct mtd_info *mtd, struct nand_chip *chip,
 				nfc_geo->auxiliary_size,
 				&auxiliary_virt, &auxiliary_phys);
 		if (ret) {
-			pr_err("Inadequate auxiliary DMA buffer\n");
+			dev_err(this->dev, "Inadequate auxiliary DMA buffer\n");
 			goto exit_auxiliary;
 		}
 	}
@@ -1099,7 +1100,7 @@ static int gpmi_ecc_write_page(struct mtd_info *mtd, struct nand_chip *chip,
 	/* Ask the NFC. */
 	ret = gpmi_send_page(this, payload_phys, auxiliary_phys);
 	if (ret)
-		pr_err("Error in ECC-based write: %d\n", ret);
+		dev_err(this->dev, "Error in ECC-based write: %d\n", ret);
 
 	if (!this->swap_block_mark) {
 		send_page_end(this, chip->oob_poi, mtd->oobsize,
@@ -1516,7 +1517,7 @@ static int gpmi_set_geometry(struct gpmi_nand_data *this)
 	/* Set up the NFC geometry which is used by BCH. */
 	ret = bch_set_geometry(this);
 	if (ret) {
-		pr_err("Error setting BCH geometry : %d\n", ret);
+		dev_err(this->dev, "Error setting BCH geometry : %d\n", ret);
 		return ret;
 	}
 
@@ -1666,13 +1667,13 @@ static int gpmi_nand_probe(struct platform_device *pdev)
 	if (of_id) {
 		pdev->id_entry = of_id->data;
 	} else {
-		pr_err("Failed to find the right device id.\n");
+		dev_err(&pdev->dev, "Failed to find the right device id.\n");
 		return -ENODEV;
 	}
 
 	this = devm_kzalloc(&pdev->dev, sizeof(*this), GFP_KERNEL);
 	if (!this) {
-		pr_err("Failed to allocate per-device memory\n");
+		dev_err(&pdev->dev, "Failed to allocate per-device memory\n");
 		return -ENOMEM;
 	}
 
-- 
1.7.2.rc3

Re: [PATCH 7/8] mtd: gpmi: change pr_err to dev_err

[Brian Norris]

On Thu, Nov 14, 2013 at 02:25:50PM +0800, Huang Shijie wrote:
> There are pr_err and dev_err in the gpmi driver now.
> It makes people confused.
> 
> This patch changes all the pr_err to dev_err except the one
> in the gpmi_reset_block().

Hmm, many of these pr_err() are wholely uninformative. They are only
useful for someone who is doing low-level debugging fo the driver, not
for users. At most, they should be {dev_dbg,pr_debug} messages, but
really, they should be removed.

> Signed-off-by: Huang Shijie <b32955@freescale.com>
> ---
>  drivers/mtd/nand/gpmi-nand/gpmi-lib.c  |   67 ++++++++++++++++++-------------
>  drivers/mtd/nand/gpmi-nand/gpmi-nand.c |   43 ++++++++++----------
>  2 files changed, 61 insertions(+), 49 deletions(-)
> 
> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-lib.c b/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
> index 10a6f07..82ac4a3 100644
> --- a/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-lib.c
> @@ -1129,7 +1140,7 @@ int gpmi_send_command(struct gpmi_nand_data *this)
>  					(struct scatterlist *)pio,
>  					ARRAY_SIZE(pio), DMA_TRANS_NONE, 0);
>  	if (!desc) {
> -		pr_err("step 1 error\n");
> +		dev_err(this->dev, "step 1 error\n");

All of these "step 1/2/etc." error messages should not be here. They
should just return a proper error code (not -1) and let the caller
display an error as appropriate. (Most of the callers already handle
this fine.)

>  		return -1;
>  	}
>  
> @@ -1143,7 +1154,7 @@ int gpmi_send_command(struct gpmi_nand_data *this)
>  				DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
>  
>  	if (!desc) {
> -		pr_err("step 2 error\n");
> +		dev_err(this->dev, "step 2 error\n");

Same here.

>  		return -1;
>  	}
>  
> @@ -1175,7 +1186,7 @@ int gpmi_send_data(struct gpmi_nand_data *this)
>  	desc = dmaengine_prep_slave_sg(channel, (struct scatterlist *)pio,
>  					ARRAY_SIZE(pio), DMA_TRANS_NONE, 0);
>  	if (!desc) {
> -		pr_err("step 1 error\n");
> +		dev_err(this->dev, "step 1 error\n");

Same here.

>  		return -1;
>  	}
>  
> @@ -1185,7 +1196,7 @@ int gpmi_send_data(struct gpmi_nand_data *this)
>  					1, DMA_MEM_TO_DEV,
>  					DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
>  	if (!desc) {
> -		pr_err("step 2 error\n");
> +		dev_err(this->dev, "step 2 error\n");

Ditto.

>  		return -1;
>  	}
>  	/* [3] submit the DMA */
> @@ -1212,7 +1223,7 @@ int gpmi_read_data(struct gpmi_nand_data *this)
>  					(struct scatterlist *)pio,
>  					ARRAY_SIZE(pio), DMA_TRANS_NONE, 0);
>  	if (!desc) {
> -		pr_err("step 1 error\n");
> +		dev_err(this->dev, "step 1 error\n");

Ditto.

>  		return -1;
>  	}
>  
> @@ -1222,7 +1233,7 @@ int gpmi_read_data(struct gpmi_nand_data *this)
>  					1, DMA_DEV_TO_MEM,
>  					DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
>  	if (!desc) {
> -		pr_err("step 2 error\n");
> +		dev_err(this->dev, "step 2 error\n");

Ditto.

>  		return -1;
>  	}
>  
> @@ -1270,7 +1281,7 @@ int gpmi_send_page(struct gpmi_nand_data *this,
>  					ARRAY_SIZE(pio), DMA_TRANS_NONE,
>  					DMA_CTRL_ACK);
>  	if (!desc) {
> -		pr_err("step 2 error\n");
> +		dev_err(this->dev, "step 2 error\n");

Ditto.

>  		return -1;
>  	}
>  	set_dma_type(this, DMA_FOR_WRITE_ECC_PAGE);
> @@ -1305,7 +1316,7 @@ int gpmi_read_page(struct gpmi_nand_data *this,
>  				(struct scatterlist *)pio, 2,
>  				DMA_TRANS_NONE, 0);
>  	if (!desc) {
> -		pr_err("step 1 error\n");
> +		dev_err(this->dev, "step 1 error\n");

Ditto.

>  		return -1;
>  	}
>  
> @@ -1335,7 +1346,7 @@ int gpmi_read_page(struct gpmi_nand_data *this,
>  					ARRAY_SIZE(pio), DMA_TRANS_NONE,
>  					DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
>  	if (!desc) {
> -		pr_err("step 2 error\n");
> +		dev_err(this->dev, "step 2 error\n");

Ditto.

>  		return -1;
>  	}
>  
> @@ -1356,7 +1367,7 @@ int gpmi_read_page(struct gpmi_nand_data *this,
>  				DMA_TRANS_NONE,
>  				DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
>  	if (!desc) {
> -		pr_err("step 3 error\n");
> +		dev_err(this->dev, "step 3 error\n");

Ditto.

>  		return -1;
>  	}
>  

I'm not taking this patch for now.

Brian

Re: [PATCH 7/8] mtd: gpmi: change pr_err to dev_err

[Huang Shijie]

于 2013年11月19日 04:23, Brian Norris 写道:
> Hmm, many of these pr_err() are wholely uninformative. They are only
> useful for someone who is doing low-level debugging fo the driver, not
> for users. At most, they should be {dev_dbg,pr_debug} messages, but
> really, they should be removed.
ok. I will remove these lines in the next patch.

thanks
Huang Shijie

[PATCH 8/8] mtd: gpmi: change pr_debug to dev_dbg

[Huang Shijie]

Change all the pr_debug to dev_dbg.
Remove the pr_fmt too, since we do not need it.

Signed-off-by: Huang Shijie <b32955@freescale.com>
---
 drivers/mtd/nand/gpmi-nand/gpmi-nand.c |   13 +++++--------
 1 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
index 85d2be2..4b8df4a 100644
--- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
@@ -18,9 +18,6 @@
  * with this program; if not, write to the Free Software Foundation, Inc.,
  * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
  */
-
-#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
-
 #include <linux/clk.h>
 #include <linux/slab.h>
 #include <linux/interrupt.h>
@@ -885,7 +882,7 @@ static void gpmi_read_buf(struct mtd_info *mtd, uint8_t *buf, int len)
 	struct nand_chip *chip = mtd->priv;
 	struct gpmi_nand_data *this = chip->priv;
 
-	pr_debug("len is %d\n", len);
+	dev_dbg(this->dev, "len is %d\n", len);
 	this->upper_buf	= buf;
 	this->upper_len	= len;
 
@@ -897,7 +894,7 @@ static void gpmi_write_buf(struct mtd_info *mtd, const uint8_t *buf, int len)
 	struct nand_chip *chip = mtd->priv;
 	struct gpmi_nand_data *this = chip->priv;
 
-	pr_debug("len is %d\n", len);
+	dev_dbg(this->dev, "len is %d\n", len);
 	this->upper_buf	= (uint8_t *)buf;
 	this->upper_len	= len;
 
@@ -976,7 +973,7 @@ static int gpmi_ecc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
 	unsigned int  max_bitflips = 0;
 	int           ret;
 
-	pr_debug("page number is : %d\n", page);
+	dev_dbg(this->dev, "page number is : %d\n", page);
 	ret = read_page_prepare(this, buf, mtd->writesize,
 					this->payload_virt, this->payload_phys,
 					nfc_geo->payload_size,
@@ -1052,7 +1049,7 @@ static int gpmi_ecc_write_page(struct mtd_info *mtd, struct nand_chip *chip,
 	dma_addr_t auxiliary_phys;
 	int        ret;
 
-	pr_debug("ecc write page.\n");
+	dev_dbg(this->dev, "ecc write page.\n");
 	if (this->swap_block_mark) {
 		/*
 		 * If control arrives here, we're doing block mark swapping.
@@ -1190,7 +1187,7 @@ static int gpmi_ecc_read_oob(struct mtd_info *mtd, struct nand_chip *chip,
 {
 	struct gpmi_nand_data *this = chip->priv;
 
-	pr_debug("page number is %d\n", page);
+	dev_dbg(this->dev, "page number is %d\n", page);
 	/* clear the OOB buffer */
 	memset(chip->oob_poi, ~0, mtd->oobsize);
 
-- 
1.7.2.rc3

Re: [PATCH 8/8] mtd: gpmi: change pr_debug to dev_dbg

[Brian Norris]

On Thu, Nov 14, 2013 at 02:25:51PM +0800, Huang Shijie wrote:
> Change all the pr_debug to dev_dbg.
> Remove the pr_fmt too, since we do not need it.

Not taking this patch yet, since I believe the pr_fmt() removal is
dependent on the previous patch.

Brian

Re: [PATCH 0/8] the clean-up for gpmi

[Brian Norris]

On Thu, Nov 14, 2013 at 02:25:43PM +0800, Huang Shijie wrote:
> This patch set does some clean-up to the gpmi driver.

Pushed patches 1-6 to l2-mtd.git/next. See comments on patch 7.

Brian