[PATCH] mtd: denali: fix write_oob_data() function

[PATCH] mtd: denali: fix write_oob_data() function

[Masahiro Yamada]

Since write_oob_data is calling denali_send_pipeline_cmd()
with access_type=SPARE_ACCESS, this might has no impact.

But calling it with tranfser_spare=false looks weird because
write_oob_data() is the function for transferring the spare area.

Signed-off-by: Masahiro Yamada <yamada.m@jp.panasonic.com>
---
 drivers/mtd/nand/denali.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
index c07cd57..a55b9d4 100644
--- a/drivers/mtd/nand/denali.c
+++ b/drivers/mtd/nand/denali.c
@@ -841,7 +841,7 @@ static int write_oob_data(struct mtd_info *mtd, uint8_t *buf, int page)
 
 	denali->page = page;
 
-	if (denali_send_pipeline_cmd(denali, false, false, SPARE_ACCESS,
+	if (denali_send_pipeline_cmd(denali, false, true, SPARE_ACCESS,
 							DENALI_WRITE) == PASS) {
 		write_data_to_flash_mem(denali, buf, mtd->oobsize);
 
-- 
1.8.3.2

Re: [PATCH] mtd: denali: fix write_oob_data() function

[Brian Norris]

On Thu, Apr 17, 2014 at 07:22:54PM +0900, Masahiro Yamada wrote:
> Since write_oob_data is calling denali_send_pipeline_cmd()
> with access_type=SPARE_ACCESS, this might has no impact.

"Might"? Are you sure? Do you have any test results? I don't want to
change this code based on a whim...

> But calling it with tranfser_spare=false looks weird because
> write_oob_data() is the function for transferring the spare area.
> 
> Signed-off-by: Masahiro Yamada <yamada.m@jp.panasonic.com>
> ---
>  drivers/mtd/nand/denali.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
> index c07cd57..a55b9d4 100644
> --- a/drivers/mtd/nand/denali.c
> +++ b/drivers/mtd/nand/denali.c
> @@ -841,7 +841,7 @@ static int write_oob_data(struct mtd_info *mtd, uint8_t *buf, int page)
>  
>  	denali->page = page;
>  
> -	if (denali_send_pipeline_cmd(denali, false, false, SPARE_ACCESS,
> +	if (denali_send_pipeline_cmd(denali, false, true, SPARE_ACCESS,
>  							DENALI_WRITE) == PASS) {
>  		write_data_to_flash_mem(denali, buf, mtd->oobsize);
>  

Brian

Re: [PATCH] mtd: denali: fix write_oob_data() function

[Masahiro Yamada]

Hi Brian,


On Fri, 9 May 2014 19:12:26 -0700
Brian Norris <computersforpeace@gmail.com> wrote:

> On Thu, Apr 17, 2014 at 07:22:54PM +0900, Masahiro Yamada wrote:
> > Since write_oob_data is calling denali_send_pipeline_cmd()
> > with access_type=SPARE_ACCESS, this might has no impact.
> 
> "Might"? Are you sure? Do you have any test results? I don't want to
> change this code based on a whim...

Sorry, I could not make myself understood because of my bad English..

What I wanted to say:
I know this patch has no impact,
but calling denali_send_pipeline_cmd() with transfer_spare=false looks weird.
So I want to fix it just in case.


I referred Denali(Cadence) NAND Flash Memory Controller User's Guide
and actually I am sure this patch has no impact.

The 3rd argument (transfer_spare) of denali_send_pipeline_cmd()
is don't care  if  its 4th argument to  is access_type=SPARE_ACCESS.

read_oob_data() function calls 
  denali_send_pipeline_cmd(denali, false, true, SPARE_ACCESS, DENALI_READ)

So, write_oob_data() function should call
  denali_send_pipeline_cmd(denali, false, true, SPARE_ACCESS, DENALI_WRITE)
for consistency.


Best Regards
Masahiro Yamada