[PATCH RESEND] mtd: nand: allow NAND_NO_SUBPAGE_WRITE to be set from driver
Marek Vasut
marex at denx.de
Tue Jul 31 10:12:27 EDT 2012
Dear Matthieu CASTET,
> Marek Vasut a écrit :
> > Dear Matthieu CASTET,
> >
> >> Hi Marek,
> >>
> >> Marek Vasut a écrit :
> >>> Dear Matthieu CASTET,
> >>>
> >>>> Hi,
> >>>>
> >>>> for ONFI flash (like this micron one) the information should be
> >>>> extracted form the ONFI table (programs_per_page IIRC)
> >>>>
> >>>> This should be better than relying on the SOC driver for setting this
> >>>> flags.
> >>>>
> >>>> Does the gpmi driver set this flag because it do not support partial
> >>>> write ?
> >>>
> >>> Yes
> >>>
> >>>> In this case why it doesn't set chip->ecc.steps to 1 ?
> >>>
> >>> Can you elabore how exactly will that help please?
> >>
> >> If you look at the nand_base.c, you will see that mtd->subpage_sft = 0
> >> if NAND_NO_SUBPAGE_WRITE flags is set or chip->ecc.steps == 1 [1].
> >
> > Ok, this is what I saw coming ... this is yet another hole in the design
> > and I see only undefined behavior. So if default: branch started
> > returning an error, this whole code will break again.
>
> Do you see any reason why chip->ecc.steps == 1 will return an error ?
> This will break drivers.
It'd be enough if mtd->subpage_sft was inited to some other value. So either add
"case 1: mtd->subpage_sft = 0; break;" or go with this patch. Either way is fine
by me.
> The behavior match the comment : "Allow subpage writes up to ecc.steps"
>
> Matthieu
>
> >> [1]
> >>
> >> /* Allow subpage writes up to ecc.steps. Not possible for MLC flash
> >> */ if (!(chip->options & NAND_NO_SUBPAGE_WRITE) &&
> >>
> >> !(chip->cellinfo & NAND_CI_CELLTYPE_MSK)) {
> >> switch (chip->ecc.steps) {
> >>
> >> case 2:
> >> mtd->subpage_sft = 1;
> >> break;
> >>
> >> case 4:
> >> case 8:
> >>
> >> case 16:
> >> mtd->subpage_sft = 2;
> >> break;
> >>
> >> }
> >>
> >> }
> >> chip->subpagesize = mtd->writesize >> mtd->subpage_sft;
Best regards,
Marek Vasut
More information about the linux-mtd
mailing list