Re: [PATCH RESEND] mtd: nand: allow NAND_NO_SUBPAGE_WRITE to be set from driver

[Marek Vasut <marex@denx.de>]

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