Re: [PATCH v2 00/19] mtd: rawnand: cafe: Convert to exec_op() (and more)

[Boris Brezillon <boris.brezillon@xxxxxxxxxxxxx>]

On Sat, 16 May 2020 16:56:50 +0200
Lubomir Rintel <lkundrak@xxxxx> wrote:

> On Fri, May 15, 2020 at 09:25:43PM +0200, Lubomir Rintel wrote:
> > On Fri, May 15, 2020 at 04:47:07PM +0200, Lubomir Rintel wrote:  
> > > On Wed, May 13, 2020 at 07:10:38PM +0200, Boris Brezillon wrote:  
> > > > Hi Lubomir,
> > > > 
> > > > On Tue, 12 May 2020 18:40:57 +0200
> > > > Lubomir Rintel <lkundrak@xxxxx> wrote:
> > > >   
> > > > > I don't really mind the patch; I was just not sure why you removed the
> > > > > acks and re-set the mask and suspected that maybe it was not
> > > > > intentional.
> > > > > 
> > > > > That said, I've now disabled the camera and mmc and did my usual test
> > > > > of mounting the filesystem and I'm seeing zero interrupts.
> > > > > 
> > > > > I suppose it's safe to assume that contrary to what I was imagining,
> > > > > the masking works well and the interrupts I was seeing are indeed from
> > > > > elsewhere (I guess the MMC driver polling the controller or something).
> > > > > 
> > > > > Also, the re-set of the mask from the interrupt handler is not realy
> > > > > necessary (but I certainly wouldn't complain if you keep it in place).  
> > > > 
> > > > I pushed a new version to my nand/cafe-nand-exec-op-debug branch. This
> > > > time I get rid of the IRQ handler as it's not used anyway. Let me know
> > > > if that keeps working an I'll send a v3.  
> > > 
> > > I can confirm that your branch (as of 12ef1918985f "mtd: rawnand: cafe: Get
> > > rid of the last printk()") is working great here:
> > > 
> > >   bash-5.0# time mount -t jffs2 mtd0 /mnt
> > >   jffs2: notice: (101) jffs2_build_xattr_subsystem: complete building xattr subsystem, 0 of xdatum (0 unchecked, 0 orphan) and 0 of xref (0 dead, 0 orphan) .
> > >   
> > >   real    0m7.319s
> > >   user    0m0.000s
> > >   sys     0m7.294s
> > >   bash-5.0# uname -a
> > >   Linux (none) 5.7.0-rc3-00097-g12ef1918985f #114 PREEMPT Fri May 15 16:35:01 CEST 2020 i586 GNU/Linux
> > >   bash-5.0#   
> > 
> > Spoke too soon again. Seems like the writes don't make it to the device:
> > 
> >   bash-5.0# mount -t jffs2 /dev/mtdblock0 /mnt
> >   jffs2: Empty flash at 0x045a5798 ends at 0x045a5800
> >   jffs2: notice: (98) jffs2_build_xattr_subsystem: complete building xattr subsystem, 1 of xdatum (0 unchecked, 0 orphan) and 1 of xref (0 dead, 0 orphan) f.
> >   bash-5.0# touch /mnt/meh
> >   bash-5.0# sync 
> >   jffs2: jffs2_flush_wbuf(): Write failed with -5
> >   jffs2: jffs2_flush_wbuf(): Write failed with -5
> >   jffs2: jffs2_flush_wbuf(): Write failed with -5
> >   jffs2: jffs2_flush_wbuf(): Write failed with -5
> >   bash-5.0# 
> > 
> > I'll follow up once I know more; with some traces or hopefully a fix.  
> 
> Hi again. Hope you're having a good weekend.
> 
> Some good news and some bad news. Good news is that now I got around to
> building an image with mtd_debug and got around to making erase, write
> adn read work, withut the hassle of dealing with jffs2 (and hassle of
> dealing with corrupted image from eariler attempts).
> 
> The bad news is that I have very little idea about what I'm actually
> doing. Here's the branch with changes I've done, with my notes in
> the commit messages:
> 
>   git fetch git://git.kernel.org/pub/scm/linux/kernel/git/lkundrak/linux.git \
>       lr/nand/cafe-nand-exec-op-debug
> 
> The relevant changes:
> 
>   6ee7e0d11e19 fixup! mtd: rawnand: cafe: Demistify register fields

This one is valid.

>   565d5f126e86 fixup! mtd: rawnand: cafe: Demistify register fields

Here it should be

#define CAFE_NAND_ECC_SYN_REG(x)		((((x) / 2) * 4) + 0x50)

but maybe the loop is no longer valid after that.

>   064d381b4615 fixup! mtd: rawnand: cafe: Demistify register fields

This one looks good too.

> 
>     During the test with JFFS2 filesystem, I was seeing a metric ton of ECC
>     errors. I have no clue where they came from because they presumably happened
>     on an asynchronous writeback, but they no longer happen and I believe
>     some of the above was to blame. Unfortunely I didn't investigate
>     further, because in my setup it is hugely inconvenient to beat the image
>     back into shape.
> 
>   085a6ad43151 Revert "mtd: rawnand: cafe: Don't leave ECC enabled in the write path"
> 
>     This triggered a WARN_ON(). Probably not too important, because the
>     code in question gets thrown out afterwards.

Oops. Maybe I should move it after the exec_op() conversion then.

> 
>   4a9d95bf6d6b fixup! mtd: rawnand: cafe: Add exec_op() support
> 
>     An straightforward fix for page write op.

Yep, that one is valid, thanks for the fix.

> 
>   6959a724994d XXX add address to CMD_STATUS
> 
>     Tricky, and I have no idea what's going on. Perhaps you'll have an
>     idea how to either address this more reasonably. As usual, I'm happy
>     to apply patches with extra tracing if necessary.

Duh, that one is super weird. I wonder if it doesn't have to do with
the fact we're using DMA. Can you try with the DMA disabled? I see
there's also a non-memory read mode, but even the standard mode should
work according to the diagram page 37 of the spec [1].

[1]http://wiki.laptop.org/images/5/5c/88ALP01_Datasheet_July_2007.pdf

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/