From mboxrd@z Thu Jan  1 00:00:00 1970
From: Marek Vasut <marek.vasut@gmail.com>
Date: Sun, 6 Nov 2011 13:43:45 +0100
Subject: [U-Boot] [PATCH] NAND: Let NAND_NO_SUBPAGE_WRITE through
In-Reply-To: <4EB64CE2.4090702@denx.de>
References: <1320459844-18243-1-git-send-email-marek.vasut@gmail.com>
	<4EB64CE2.4090702@denx.de>
Message-ID: <201111061343.45155.marek.vasut@gmail.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

> On 11/05/2011 03:24 AM, Marek Vasut wrote:
> > There is a problem reported that the NAND_NO_SUBPAGE_WRITE, set by some
> > drivers, is silently ignored by NAND core. This causes UBI to
> > malfunction on these drivers, because UBI tries to use subpage writes.
> 
> Hi Marek,
> 
> > This was discussed already with no conclusion, see thread:
> > Message-Id: <1302372335-30232-6-git-send-email-sbabic@denx.de>
> 
> Right, there was some discussion also comparing what it is done on the
> Linux Kernel. There is a discrepancy for the usage of the
> NAND_NO_SUBPAGE_WRITE, because it belongs to the chip options, but in
> this case (and in the patch I submitted) it is used as "controller"
> option. Same case here.

Yes sure, but the controller driver sets it as a chip option as there's no 
"controller" option field and I don't want to introduce new definition consuming 
one bit in the top half of chip option.
> 
> > The bug was recently retriggered by Veli-Pekka Peltola, causing him
> > trouble with UBI on the MX28 CPU:
> > Message-ID: <4EB3E4EA.9080509@bluegiga.com>
> > 
> >  drivers/mtd/nand/nand_base.c |   11 +++++++++++
> >  1 files changed, 11 insertions(+), 0 deletions(-)
> > 
> > diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> > index 6aac6a2..7ecd5a3 100644
> > --- a/drivers/mtd/nand/nand_base.c
> > +++ b/drivers/mtd/nand/nand_base.c
> > @@ -2548,6 +2548,7 @@ static const struct nand_flash_dev
> > *nand_get_flash_type(struct mtd_info *mtd,
> > 
> >  {
> >  
> >  	int ret, maf_idx;
> >  	int tmp_id, tmp_manf;
> > 
> > +	int no_subpage = 0;
> > 
> >  	/* Select the device */
> >  	chip->select_chip(mtd, 0);
> > 
> > @@ -2612,10 +2613,20 @@ static const struct nand_flash_dev
> > *nand_get_flash_type(struct mtd_info *mtd,
> > 
> >  	if (!ret)
> >  	
> >  		nand_flash_detect_non_onfi(mtd, chip, type, &busw);
> > 
> > +	/*
> > +	 * If the controller is incapable of subpage writes, force no subpage
> > +	 * writes. This has to be done here, otherwise UBI will complain.
> > +	 */
> > +	if (chip->options & NAND_NO_SUBPAGE_WRITE)
> > +		no_subpage = 1;
> > +
> 
> The patch does not convince me. This relies only if the SUBPAGE_WRITE is
> supported by the chip, and this is half problem. What happens if we have
> a SLC (that generally supports subpage mode) connected to a controller
> that does not support it ? NAND_NO_SUBPAGE_WRITE is not set, but the
> controller can't manage it.
> I think your patch fix only a particular case, and it is not general
> enough.

No, the controller must set NAND_NO_SUBPAGE_WRITE as a chip option in its driver 
to force it so it should work for both cases.

Cheers