* mtd/NAND:Fix issues with freescale IFC to support NAND 2K @ 2011-12-29 4:59 Prabhakar Kushwaha 2011-12-29 4:59 ` [PATCH 1/2] mtd/nand:Fix wrong address read in is_blank() Prabhakar Kushwaha 2011-12-29 4:59 ` [PATCH 2/2] mtd/nand: Fix IFC driver to support 2K NAND page Prabhakar Kushwaha 0 siblings, 2 replies; 7+ messages in thread From: Prabhakar Kushwaha @ 2011-12-29 4:59 UTC (permalink / raw) To: linuxppc-dev, linux-mtd; +Cc: scottwood This Patch series takes care issues with Freescale IFC driver for supporting 2K page size NAND with ECC enabled. [PATCH 1/2] mtd/nand:Fix wrong address read in is_blank() Fix driver issue when ECC enabled. [PATCH 2/2] mtd/nand: Fix IFC driver to support 2K NAND page Fix driver during OOB updation ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] mtd/nand:Fix wrong address read in is_blank() 2011-12-29 4:59 mtd/NAND:Fix issues with freescale IFC to support NAND 2K Prabhakar Kushwaha @ 2011-12-29 4:59 ` Prabhakar Kushwaha 2012-01-03 20:24 ` Scott Wood 2011-12-29 4:59 ` [PATCH 2/2] mtd/nand: Fix IFC driver to support 2K NAND page Prabhakar Kushwaha 1 sibling, 1 reply; 7+ messages in thread From: Prabhakar Kushwaha @ 2011-12-29 4:59 UTC (permalink / raw) To: linuxppc-dev, linux-mtd; +Cc: scottwood, Poonam Aggrwal, Prabhakar Kushwaha IFC NAND Machine calculates ECC on 512byte sector. Same is taken care in fsl_ifc_run_command() while ECC status verification. Here buffer number is calculated assuming 512byte sector and same is passed to is_blank. However in is_blank() buffer address is calculated using mdt->writesize which is wrong. It should be calculated on basis of ecc sector size. Also, in fsl_ifc_run_command() bufferpage is calculated on the basis of ecc sector size instead of hard coded value. Signed-off-by: Poonam Aggrwal <poonam.aggrwal@freescale.com> Signed-off-by: Prabhakar Kushwaha <prabhakar@freescale.com> --- git://git.kernel.org/pub/scm/linux/kernel/git/galak/powerpc.git (branch next) Tested on P1010RDB drivers/mtd/nand/fsl_ifc_nand.c | 6 ++++-- 1 files changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/mtd/nand/fsl_ifc_nand.c b/drivers/mtd/nand/fsl_ifc_nand.c index 8475b88..2df7206 100644 --- a/drivers/mtd/nand/fsl_ifc_nand.c +++ b/drivers/mtd/nand/fsl_ifc_nand.c @@ -191,7 +191,9 @@ static int is_blank(struct mtd_info *mtd, unsigned int bufnum) { struct nand_chip *chip = mtd->priv; struct fsl_ifc_mtd *priv = chip->priv; - u8 __iomem *addr = priv->vbase + bufnum * (mtd->writesize * 2); + int bufperpage = mtd->writesize / chip->ecc.size; + u8 __iomem *addr = priv->vbase + bufnum / bufperpage + * (mtd->writesize * 2); u32 __iomem *mainarea = (u32 *)addr; u8 __iomem *oob = addr + mtd->writesize; int i; @@ -273,7 +275,7 @@ static void fsl_ifc_run_command(struct mtd_info *mtd) dev_err(priv->dev, "NAND Flash Write Protect Error\n"); if (nctrl->eccread) { - int bufperpage = mtd->writesize / 512; + int bufperpage = mtd->writesize / chip->ecc.size; int bufnum = (nctrl->page & priv->bufnum_mask) * bufperpage; int bufnum_end = bufnum + bufperpage - 1; -- 1.7.5.4 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] mtd/nand:Fix wrong address read in is_blank() 2011-12-29 4:59 ` [PATCH 1/2] mtd/nand:Fix wrong address read in is_blank() Prabhakar Kushwaha @ 2012-01-03 20:24 ` Scott Wood 2012-01-04 4:35 ` Prabhakar 0 siblings, 1 reply; 7+ messages in thread From: Scott Wood @ 2012-01-03 20:24 UTC (permalink / raw) To: Prabhakar Kushwaha; +Cc: linux-mtd, linuxppc-dev, Poonam Aggrwal On 12/28/2011 10:59 PM, Prabhakar Kushwaha wrote: > IFC NAND Machine calculates ECC on 512byte sector. Same is taken care in > fsl_ifc_run_command() while ECC status verification. Here buffer number is > calculated assuming 512byte sector and same is passed to is_blank. > However in is_blank() buffer address is calculated using mdt->writesize which is > wrong. It should be calculated on basis of ecc sector size. > > Also, in fsl_ifc_run_command() bufferpage is calculated on the basis of ecc sector > size instead of hard coded value. > > Signed-off-by: Poonam Aggrwal <poonam.aggrwal@freescale.com> > Signed-off-by: Prabhakar Kushwaha <prabhakar@freescale.com> > --- > git://git.kernel.org/pub/scm/linux/kernel/git/galak/powerpc.git (branch next) > > Tested on P1010RDB > > drivers/mtd/nand/fsl_ifc_nand.c | 6 ++++-- > 1 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/mtd/nand/fsl_ifc_nand.c b/drivers/mtd/nand/fsl_ifc_nand.c > index 8475b88..2df7206 100644 > --- a/drivers/mtd/nand/fsl_ifc_nand.c > +++ b/drivers/mtd/nand/fsl_ifc_nand.c > @@ -191,7 +191,9 @@ static int is_blank(struct mtd_info *mtd, unsigned int bufnum) > { > struct nand_chip *chip = mtd->priv; > struct fsl_ifc_mtd *priv = chip->priv; > - u8 __iomem *addr = priv->vbase + bufnum * (mtd->writesize * 2); > + int bufperpage = mtd->writesize / chip->ecc.size; > + u8 __iomem *addr = priv->vbase + bufnum / bufperpage > + * (mtd->writesize * 2); > u32 __iomem *mainarea = (u32 *)addr; > u8 __iomem *oob = addr + mtd->writesize; > int i; This function should only be checking one ECC block, not the entire page. The caller is responsible for passing in the appropriate buffer numbers. I think what the current code needs is for (mtd->writesize * 2) to be replaced with chip->ecc.size, and for the calling code to multiply the starting bufnum by two. > @@ -273,7 +275,7 @@ static void fsl_ifc_run_command(struct mtd_info *mtd) > dev_err(priv->dev, "NAND Flash Write Protect Error\n"); > > if (nctrl->eccread) { > - int bufperpage = mtd->writesize / 512; > + int bufperpage = mtd->writesize / chip->ecc.size; > int bufnum = (nctrl->page & priv->bufnum_mask) * bufperpage; > int bufnum_end = bufnum + bufperpage - 1; > Currently this driver always sets chip->ecc.size to 512. If we want to support other ECC block sizes that future versions of IFC may have, can we calculate bufperpage during chip init (similar to bufnum_mask) to avoid the runtime division? It's probably not huge overhead compared to everything else we do per NAND page transfer, but still... -Scott ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] mtd/nand:Fix wrong address read in is_blank() 2012-01-03 20:24 ` Scott Wood @ 2012-01-04 4:35 ` Prabhakar 0 siblings, 0 replies; 7+ messages in thread From: Prabhakar @ 2012-01-04 4:35 UTC (permalink / raw) To: Scott Wood; +Cc: linux-mtd, linuxppc-dev, Poonam Aggrwal On Wednesday 04 January 2012 01:54 AM, Scott Wood wrote: > On 12/28/2011 10:59 PM, Prabhakar Kushwaha wrote: >> IFC NAND Machine calculates ECC on 512byte sector. Same is taken care in >> fsl_ifc_run_command() while ECC status verification. Here buffer number is >> calculated assuming 512byte sector and same is passed to is_blank. >> However in is_blank() buffer address is calculated using mdt->writesize which is >> wrong. It should be calculated on basis of ecc sector size. >> >> Also, in fsl_ifc_run_command() bufferpage is calculated on the basis of ecc sector >> size instead of hard coded value. >> >> Signed-off-by: Poonam Aggrwal<poonam.aggrwal@freescale.com> >> Signed-off-by: Prabhakar Kushwaha<prabhakar@freescale.com> >> --- >> git://git.kernel.org/pub/scm/linux/kernel/git/galak/powerpc.git (branch next) >> >> Tested on P1010RDB >> >> drivers/mtd/nand/fsl_ifc_nand.c | 6 ++++-- >> 1 files changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/mtd/nand/fsl_ifc_nand.c b/drivers/mtd/nand/fsl_ifc_nand.c >> index 8475b88..2df7206 100644 >> --- a/drivers/mtd/nand/fsl_ifc_nand.c >> +++ b/drivers/mtd/nand/fsl_ifc_nand.c >> @@ -191,7 +191,9 @@ static int is_blank(struct mtd_info *mtd, unsigned int bufnum) >> { >> struct nand_chip *chip = mtd->priv; >> struct fsl_ifc_mtd *priv = chip->priv; >> - u8 __iomem *addr = priv->vbase + bufnum * (mtd->writesize * 2); >> + int bufperpage = mtd->writesize / chip->ecc.size; >> + u8 __iomem *addr = priv->vbase + bufnum / bufperpage >> + * (mtd->writesize * 2); >> u32 __iomem *mainarea = (u32 *)addr; >> u8 __iomem *oob = addr + mtd->writesize; >> int i; > This function should only be checking one ECC block, not the entire > page. The caller is responsible for passing in the appropriate buffer > numbers. > > I think what the current code needs is for (mtd->writesize * 2) to be > replaced with chip->ecc.size, and for the calling code to multiply the > starting bufnum by two. Got your point :). I will take care in next patch version. >> @@ -273,7 +275,7 @@ static void fsl_ifc_run_command(struct mtd_info *mtd) >> dev_err(priv->dev, "NAND Flash Write Protect Error\n"); >> >> if (nctrl->eccread) { >> - int bufperpage = mtd->writesize / 512; >> + int bufperpage = mtd->writesize / chip->ecc.size; >> int bufnum = (nctrl->page& priv->bufnum_mask) * bufperpage; >> int bufnum_end = bufnum + bufperpage - 1; >> > Currently this driver always sets chip->ecc.size to 512. If we want to > support other ECC block sizes that future versions of IFC may have, can > we calculate bufperpage during chip init (similar to bufnum_mask) to > avoid the runtime division? It's probably not huge overhead compared to > everything else we do per NAND page transfer, but still... > Yes. I agree. We are working on this in order to support new controller version. --Prabhakar ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2] mtd/nand: Fix IFC driver to support 2K NAND page 2011-12-29 4:59 mtd/NAND:Fix issues with freescale IFC to support NAND 2K Prabhakar Kushwaha 2011-12-29 4:59 ` [PATCH 1/2] mtd/nand:Fix wrong address read in is_blank() Prabhakar Kushwaha @ 2011-12-29 4:59 ` Prabhakar Kushwaha 2012-01-03 19:49 ` Scott Wood 1 sibling, 1 reply; 7+ messages in thread From: Prabhakar Kushwaha @ 2011-12-29 4:59 UTC (permalink / raw) To: linuxppc-dev, linux-mtd; +Cc: scottwood, Poonam Aggrwal, Prabhakar Kushwaha 1) OOB area should be updated irrespective of NAND page size. Earlier it was updated only for 512byte NAND page. 2) During OOB update fbcr should be equal to OOB size. Signed-off-by: Poonam Aggrwal <poonam.aggrwal@freescale.com> Signed-off-by: Prabhakar Kushwaha <prabhakar@freescale.com> --- git://git.kernel.org/pub/scm/linux/kernel/git/galak/powerpc.git (branch next) Tested on P1010RDB drivers/mtd/nand/fsl_ifc_nand.c | 20 ++++++++------------ 1 files changed, 8 insertions(+), 12 deletions(-) diff --git a/drivers/mtd/nand/fsl_ifc_nand.c b/drivers/mtd/nand/fsl_ifc_nand.c index 2df7206..2c02168 100644 --- a/drivers/mtd/nand/fsl_ifc_nand.c +++ b/drivers/mtd/nand/fsl_ifc_nand.c @@ -439,20 +439,16 @@ static void fsl_ifc_cmdfunc(struct mtd_info *mtd, unsigned int command, out_be32(&ifc->ifc_nand.nand_fir1, (IFC_FIR_OP_CW1 << IFC_NAND_FIR1_OP5_SHIFT)); - if (column >= mtd->writesize) { - /* OOB area --> READOOB */ - column -= mtd->writesize; - nand_fcr0 |= NAND_CMD_READOOB << - IFC_NAND_FCR0_CMD0_SHIFT; - ifc_nand_ctrl->oob = 1; - } else if (column < 256) + if (column < 256) /* First 256 bytes --> READ0 */ nand_fcr0 |= NAND_CMD_READ0 << IFC_NAND_FCR0_CMD0_SHIFT; - else - /* Second 256 bytes --> READ1 */ - nand_fcr0 |= - NAND_CMD_READ1 << IFC_NAND_FCR0_CMD0_SHIFT; + } + + if (column >= mtd->writesize) { + /* OOB area --> READOOB */ + column -= mtd->writesize; + ifc_nand_ctrl->oob = 1; } out_be32(&ifc->ifc_nand.nand_fcr0, nand_fcr0); @@ -465,7 +461,7 @@ static void fsl_ifc_cmdfunc(struct mtd_info *mtd, unsigned int command, int full_page; if (ifc_nand_ctrl->oob) { out_be32(&ifc->ifc_nand.nand_fbcr, - ifc_nand_ctrl->index); + ifc_nand_ctrl->index - ifc_nand_ctrl->column); full_page = 0; } else { out_be32(&ifc->ifc_nand.nand_fbcr, 0); -- 1.7.5.4 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] mtd/nand: Fix IFC driver to support 2K NAND page 2011-12-29 4:59 ` [PATCH 2/2] mtd/nand: Fix IFC driver to support 2K NAND page Prabhakar Kushwaha @ 2012-01-03 19:49 ` Scott Wood 2012-01-04 4:58 ` Prabhakar 0 siblings, 1 reply; 7+ messages in thread From: Scott Wood @ 2012-01-03 19:49 UTC (permalink / raw) To: Prabhakar Kushwaha; +Cc: linux-mtd, linuxppc-dev, Poonam Aggrwal On 12/28/2011 10:59 PM, Prabhakar Kushwaha wrote: > 1) OOB area should be updated irrespective of NAND page size. Earlier it was > updated only for 512byte NAND page. > > 2) During OOB update fbcr should be equal to OOB size. > > Signed-off-by: Poonam Aggrwal <poonam.aggrwal@freescale.com> > Signed-off-by: Prabhakar Kushwaha <prabhakar@freescale.com> > --- > git://git.kernel.org/pub/scm/linux/kernel/git/galak/powerpc.git (branch next) The IFC driver hasn't been merged into that tree that I can see. > Tested on P1010RDB > > drivers/mtd/nand/fsl_ifc_nand.c | 20 ++++++++------------ > 1 files changed, 8 insertions(+), 12 deletions(-) > > diff --git a/drivers/mtd/nand/fsl_ifc_nand.c b/drivers/mtd/nand/fsl_ifc_nand.c > index 2df7206..2c02168 100644 > --- a/drivers/mtd/nand/fsl_ifc_nand.c > +++ b/drivers/mtd/nand/fsl_ifc_nand.c > @@ -439,20 +439,16 @@ static void fsl_ifc_cmdfunc(struct mtd_info *mtd, unsigned int command, > out_be32(&ifc->ifc_nand.nand_fir1, > (IFC_FIR_OP_CW1 << IFC_NAND_FIR1_OP5_SHIFT)); > > - if (column >= mtd->writesize) { > - /* OOB area --> READOOB */ > - column -= mtd->writesize; > - nand_fcr0 |= NAND_CMD_READOOB << > - IFC_NAND_FCR0_CMD0_SHIFT; > - ifc_nand_ctrl->oob = 1; > - } else if (column < 256) > + if (column < 256) > /* First 256 bytes --> READ0 */ > nand_fcr0 |= > NAND_CMD_READ0 << IFC_NAND_FCR0_CMD0_SHIFT; > - else > - /* Second 256 bytes --> READ1 */ > - nand_fcr0 |= > - NAND_CMD_READ1 << IFC_NAND_FCR0_CMD0_SHIFT; > + } > + > + if (column >= mtd->writesize) { > + /* OOB area --> READOOB */ > + column -= mtd->writesize; > + ifc_nand_ctrl->oob = 1; > } Where is NAND_CMD_READOOB going to be set in the small-page case? The small-page code should read something like: if (column >= mtd->writesize) { nand_fcr0 |= NAND_CMD_READOOB << IFC_NAND_FCR0_CMD0_SHIFT; } else { nand_fcr0 |= NAND_CMD_READ0 << IFC_NAND_FCR0_CMD0_SHIFT; } It looks like we can get rid of ctrl->column, BTW. -Scott ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] mtd/nand: Fix IFC driver to support 2K NAND page 2012-01-03 19:49 ` Scott Wood @ 2012-01-04 4:58 ` Prabhakar 0 siblings, 0 replies; 7+ messages in thread From: Prabhakar @ 2012-01-04 4:58 UTC (permalink / raw) To: Scott Wood; +Cc: linux-mtd, linuxppc-dev, Poonam Aggrwal On Wednesday 04 January 2012 01:19 AM, Scott Wood wrote: > On 12/28/2011 10:59 PM, Prabhakar Kushwaha wrote: >> 1) OOB area should be updated irrespective of NAND page size. Earlier it was >> updated only for 512byte NAND page. >> >> 2) During OOB update fbcr should be equal to OOB size. >> >> Signed-off-by: Poonam Aggrwal<poonam.aggrwal@freescale.com> >> Signed-off-by: Prabhakar Kushwaha<prabhakar@freescale.com> >> --- >> git://git.kernel.org/pub/scm/linux/kernel/git/galak/powerpc.git (branch next) > The IFC driver hasn't been merged into that tree that I can see. This patch is created on top of IFC driver patch (already floated in mailing list). Please find there link: http://patchwork.ozlabs.org/patch/133315/ http://patchwork.ozlabs.org/patch/133316/ >> Tested on P1010RDB >> >> drivers/mtd/nand/fsl_ifc_nand.c | 20 ++++++++------------ >> 1 files changed, 8 insertions(+), 12 deletions(-) >> >> diff --git a/drivers/mtd/nand/fsl_ifc_nand.c b/drivers/mtd/nand/fsl_ifc_nand.c >> index 2df7206..2c02168 100644 >> --- a/drivers/mtd/nand/fsl_ifc_nand.c >> +++ b/drivers/mtd/nand/fsl_ifc_nand.c >> @@ -439,20 +439,16 @@ static void fsl_ifc_cmdfunc(struct mtd_info *mtd, unsigned int command, >> out_be32(&ifc->ifc_nand.nand_fir1, >> (IFC_FIR_OP_CW1<< IFC_NAND_FIR1_OP5_SHIFT)); >> >> - if (column>= mtd->writesize) { >> - /* OOB area --> READOOB */ >> - column -= mtd->writesize; >> - nand_fcr0 |= NAND_CMD_READOOB<< >> - IFC_NAND_FCR0_CMD0_SHIFT; >> - ifc_nand_ctrl->oob = 1; >> - } else if (column< 256) >> + if (column< 256) >> /* First 256 bytes --> READ0 */ >> nand_fcr0 |= >> NAND_CMD_READ0<< IFC_NAND_FCR0_CMD0_SHIFT; >> - else >> - /* Second 256 bytes --> READ1 */ >> - nand_fcr0 |= >> - NAND_CMD_READ1<< IFC_NAND_FCR0_CMD0_SHIFT; >> + } >> + >> + if (column>= mtd->writesize) { >> + /* OOB area --> READOOB */ >> + column -= mtd->writesize; >> + ifc_nand_ctrl->oob = 1; >> } > Where is NAND_CMD_READOOB going to be set in the small-page case? 2K NAND flash does not require NAND_CMD_READOOB. So i thought same should be applied to 512byte NAND. but i am wrong. Thanks for pointing it out :) > > The small-page code should read something like: > > if (column>= mtd->writesize) { > nand_fcr0 |= > NAND_CMD_READOOB<< IFC_NAND_FCR0_CMD0_SHIFT; > } else { > nand_fcr0 |= > NAND_CMD_READ0<< IFC_NAND_FCR0_CMD0_SHIFT; > } > > It looks like we can get rid of ctrl->column, BTW. > I will take care this in next patch release --Prabhakar ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2012-01-04 4:59 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-12-29 4:59 mtd/NAND:Fix issues with freescale IFC to support NAND 2K Prabhakar Kushwaha 2011-12-29 4:59 ` [PATCH 1/2] mtd/nand:Fix wrong address read in is_blank() Prabhakar Kushwaha 2012-01-03 20:24 ` Scott Wood 2012-01-04 4:35 ` Prabhakar 2011-12-29 4:59 ` [PATCH 2/2] mtd/nand: Fix IFC driver to support 2K NAND page Prabhakar Kushwaha 2012-01-03 19:49 ` Scott Wood 2012-01-04 4:58 ` Prabhakar
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).