[PATCH v5 12/14] mtd: nand: pxa3xx: Introduce multiple page I/O support
Brian Norris
computersforpeace at gmail.com
Thu Nov 14 18:07:03 EST 2013
On Thu, Nov 14, 2013 at 08:02:50PM -0300, Ezequiel Garcia wrote:
> On Thu, Nov 14, 2013 at 02:40:56PM -0800, Brian Norris wrote:
> > On Thu, Nov 14, 2013 at 06:25:37PM -0300, Ezequiel Garcia wrote:
> > > --- a/drivers/mtd/nand/pxa3xx_nand.c
> > > +++ b/drivers/mtd/nand/pxa3xx_nand.c
> > > @@ -1151,7 +1288,28 @@ static int armada370_ecc_init(struct pxa3xx_nand_info *info,
> > > struct nand_ecc_ctrl *ecc,
> > > int strength, int page_size)
> > > {
> > > - /* Unimplemented yet */
> > > + if (strength == 4 && page_size == 4096) {
> >
> > I still think this sort of check can be improved just a bit. Can you
> > comment on my additional diff, pasted below? (Not even compile-tested)
> >
> > > + info->ecc_bch = 1;
> > > + info->chunk_size = 2048;
> > > + info->spare_size = 32;
> > > + info->ecc_size = 32;
> > > + ecc->mode = NAND_ECC_HW;
> > > + ecc->size = info->chunk_size;
> > > + ecc->layout = &ecc_layout_4KB_bch4bit;
> > > + ecc->strength = 16;
> > > + return 1;
> > > +
> > > + } else if (strength == 8 && page_size == 4096) {
> > > + info->ecc_bch = 1;
> > > + info->chunk_size = 1024;
> > > + info->spare_size = 0;
> > > + info->ecc_size = 32;
> > > + ecc->mode = NAND_ECC_HW;
> > > + ecc->size = info->chunk_size;
> > > + ecc->layout = &ecc_layout_4KB_bch8bit;
> > > + ecc->strength = 16;
> > > + return 1;
> > > + }
> > > return 0;
> > > }
> > >
> >
> > Brian
> >
> > diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
> > index 8cb6640..e219d75 100644
> > --- a/drivers/mtd/nand/pxa3xx_nand.c
> > +++ b/drivers/mtd/nand/pxa3xx_nand.c
> > @@ -1363,9 +1363,13 @@ static int pxa_ecc_init(struct pxa3xx_nand_info *info,
> >
> > static int armada370_ecc_init(struct pxa3xx_nand_info *info,
> > struct nand_ecc_ctrl *ecc,
> > - int strength, int page_size)
> > + int strength, int ecc_stepsize, int page_size)
> > {
> > - if (strength == 4 && page_size == 4096) {
> > + /*
> > + * Required ECC: 4-bit correction per 512 bytes
> > + * Select: 16-bit correction per 2048 bytes
> > + */
> > + if (strength == 4 && ecc_stepsize == 512 && page_size == 4096) {
> > info->ecc_bch = 1;
> > info->chunk_size = 2048;
> > info->spare_size = 32;
> > @@ -1376,7 +1380,11 @@ static int armada370_ecc_init(struct pxa3xx_nand_info *info,
> > ecc->strength = 16;
> > return 1;
> >
> > - } else if (strength == 8 && page_size == 4096) {
> > + /*
> > + * Required ECC: 8-bit correction per 512 bytes
> > + * Select: 16-bit correction per 1024 bytes
> > + */
> > + } else if (strength == 8 && ecc_stepsize == 512 && page_size == 4096) {
> > info->ecc_bch = 1;
> > info->chunk_size = 1024;
> > info->spare_size = 0;
> > @@ -1484,6 +1492,7 @@ KEEP_CONFIG:
> > if (info->variant == PXA3XX_NAND_VARIANT_ARMADA370)
> > ret = armada370_ecc_init(info, &chip->ecc,
> > chip->ecc_strength_ds,
> > + chip->ecc_step_ds,
> > mtd->writesize);
> > else
> > ret = pxa_ecc_init(info, &chip->ecc,
>
> Looks good, but let me see if I'm getting this straight.
>
> Your point is that an ECC "strength" is meaningless without
> the _step_ definition.
>
> So, I was assuming the step to be 512, and your diff is making such
> assumption an explicit check.
>
> Correct me if I didn't understand.
Exactly correct. I'll put a description on this and send it separately.
Brian
More information about the linux-arm-kernel
mailing list