Re: [PATCH v6 00/15] A SPI NAND framework under generic NAND framework

[Boris Brezillon <boris.brezillon@free-electrons.com>]

On Fri, 22 Dec 2017 14:37:06 +0800
Peter Pan <peterpansjtu@gmail.com> wrote:

> Hi Boris and Frieder
> 
> On Fri, Dec 22, 2017 at 8:49 AM, Peter Pan <peterpansjtu@gmail.com> wrote:
> > Hi Frieder,
> >
> > On Thu, Dec 21, 2017 at 7:48 PM, Frieder Schrempf
> > <frieder.schrempf@exceet.de> wrote:  
> >> Hello Boris,
> >>  
> >>>>>> So shouldn't there be an spinand_die_select_op in the SPI NAND core
> >>>>>> that
> >>>>>> is executed when necessary and skipped if there's only one die.  
> >>>>>
> >>>>>
> >>>>> Sure, I was arguing against a ->select_chip() at the generic NAND
> >>>>> level. That's something you can add in the SPI NAND framework.  
> >>
> >>
> >> I added an op to send the die selection command and call it, where I think
> >> it is needed: [1]  
> >
> > I think we should add die_select_op in vendor's file and call a hook in core.c
> > since the die select command implementation is different by vendors.
> >
> > Thanks
> > Peter Pan
> >
> >  
> >>
> >> Accessing both dies on the Winbond SPI NAND works fine like this.
> >>
> >> While running tests I came across some problems:
> >>
> >> * On accessing the BBT in RAM via nanddev_bbt_[set/get]_block_status, the
> >> calculation of position and offset seems to be wrong.
> >> My fix is here: [2]
> >>
> >> * On calculating the row address for read/program/erase via
> >> nanddev_pos_to_row, it seems like the eraseblock_addr_shift is wrong.
> >>
> >> * Also, I'm not sure if the LUN should be taken into account when
> >> calculating the row address. At least when you select the LUN by issuing a
> >> separate command, the row address sent to the chip should only contain the
> >> page address.
> >> But I'm not sure if that's the case in general, or if some code is needed to
> >> differentiate.
> >>
> >> See my changes of nanddev_pos_to_row here: [3]
> >>
> >> * I run into a mutex deadlock, when spinand_write_page fails (e.g. because
> >> of a bad block) as the lock is not released in such cases. See my fix here:
> >> [4]
> >>
> >> With these fixes applied and as far as I can tell everything seems to work
> >> fine. I'll do some tests with UBI next and look into the ECC topic.  
> 
> UBI attach failed since missing mtd->writebufsize assignment in nanddev_init()
> After fixing it and with Frieder's fixing, UBIFS can be mounted successfully on
> Zedboard with generic spi controller and Micron SPI NAND 2Gb device.
> 
> Also, the code now can pass mtd read and page test. 1 error found in oob test
> since we don't have "past end of partition" check in part_write_oob() which I
> mentioned in earlier mail. Since we don't support ECC right now, I didn't try
> nandbiterror test.
> 

Peter, Frider, I just pushed a new version to my nand/spi-nand branch
[1] fixing the authorship, adding/fixing/removing comments where needed.

Can you please have a look at those changes (everything I changed
should appear as !fixup commits) and let me know if I did something
wrong.

We still need to make commit messages a bit more verbose.

This afternoon I'll have a look at the ->select_die() feature.

Regards,

Boris

[1]https://github.com/bbrezillon/linux-0day/commits/nand/spi-nand