Message ID | 1397730174-4763-1-git-send-email-yamada.m@jp.panasonic.com |
---|---|
State | Deferred |
Headers | show |
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
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
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);
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(-)