[U-Boot] Fix fsl_elbc_nand driver

[Scott Wood]

On Wed, 2015-05-20 at 18:27 -0700, Andrei Yakimov wrote:
> I will make a patch with 1536.
> 
> Where should I send linux patch?
> They have bunch of mail lists for different subsystems.
> Andrei

The MAINTAINERS file shows where to send patches for different
subsystems.  In this case it would be the "MEMORY TECHNOLOGY DEVICES
(MTD)" section.

> On Wed, 2015-05-20 at 17:25 -0500, Scott Wood wrote:
> > On Tue, 2015-05-19 at 16:42 -0700, Andrei Yakimov wrote:
> > > On Tue, 2015-05-19 at 17:38 -0500, Scott Wood wrote:
> > > > On Tue, 2015-05-19 at 15:29 -0700, Andrei Yakimov wrote:
> > > > > I did not compiling latest, I still in 2011.9 and 2.6.38.
> > > > > I have go over latest kernel and can see they using 
> > > > > NAND_CMD_PARAM with sub command  0x40 - to get JEDEC
> > > > > information, it is 3 mandatory copy by 512 bytes.
> > > > 
> > > > 3x512 or 3x256?
> > > ONFI - 3x256 sub command 0x0
> > > JEDEC - 3x512 sub command 0x40
> > 
> > So then we want 1536 bytes, not 768 (or 786) if we go with the simple
> > fix?
> Yes
> > 
> > > > > Going over kernel divers, figure out some read whole
> > > > > page some 256 bytes.
> > > > > Reading whole page (set fcbr = 0) have some sense - you do not need
> > > > > to know anything about flash, but what to put in to read_bytes ?
> > > > 
> > > > You don't want fbcr = 0 here because that will enable ECC which isn't
> > > > there.
> > >  Is it correcting or just generating syndrome? It is working on
> > >  my board, I would say it only generate or ignored for this command
> > > (8313). It should corrupt data if it correcting but it does not.
> > 
> > Correcting.  Perhaps it's working because it's reporting an
> > uncorrectable error (thus not correcting anything) and you're ignoring
> > it?
> may be.
> >  
> > > > > It looks like for universal patch 2K should be read.
> > > > 
> > > > Again, if we're going to do anything beyond s/256/768/ it should be a
> > > > higher level function where the caller says how much it wants.
> > > It is not normal nand  flow:  READ_ID and PARAM assuming it know the
> > > size.
> > 
> > I'm not sure what you're trying to say here.
> I meant normal nand flow read/write - propagate size how much to read
> or write, READ_ID and PARAM regular nand flow assume driver would know
> how much to read.

No, they don't assume the driver knows how much to read.  They assume
the driver doesn't need to tell the hardware how much to read, which
isn't true with this controller.

> > > > > I have also check other vendor controllers like tegra,
> > > > > there continuous data read trigger additional data transfer from
> > > > > chip.
> > > Can we do  (NOP CWO UA RWB RS RS RS RS) 
> > > wait ltesr (cc) and after that 
> > > next  read_buffer ( RB or RS)
> > > all command have to start with NOP,
> > > this will effectively terminate previous command.
> > > And we do not care about locks in u-boot. kernel will be different
> > > store, but again this code executed only during start up - so who care
> > > holding CS to long.
> > 
> > You won't be holding CS that long.  It will drop as soon as the current
> > operation completes.  And I'm not interested in a solution that only
> > works in U-Boot's single-tasking environment, given that this code is
> > more or less shared with Linux.
> Are you saying elbc will drop CS even last fir instruction not 0?
> You are at Freescale - you should know or can check :).

That is my understanding of how the hardware works, yes (though I
haven't tested or asked someone who knows the implementation).  The bit
about NOP ending the operation sequence just means that the hardware
won't look at any subsequent fields in FIR once it finds a NOP.

> About lock, I was only saying linux will might need a lock for this
> sequences, depend on nand flash detection can or can not run in parallel
> if you have multiple chips - but I do not think it can - it is early
> boot an it is not how nand initializes. MTD doing it at once.	

What if the user inserts a NAND module after boot, while NOR activity is
going on in the background?

In any case, I really don't want to do such things in this driver.

> > I don't see what the objection is to adding a replaceable read_param()
> > method that is not so hostile to high-level controllers.
> Sorry, I has not understand you completely.
> Are you suggesting add read_param() method to whole nand infrastructure,
> for NAND_CMD_PARAM method?

Yes.

>  It is huge changes

It's not.  The default implementation would contain the more or less the
same code that runs today.

> and this will not change fact some how we should get information about read size.

The size to be read would be a parameter to read_param().

> For elbc and imx due to we reading all at once, it can not be stateless,
> we need to read more and more each time

Why do we need to?  Why can't we read all three copies at once?

> reissuing command or relay on different way to ID chip - and this make
> it pointless if it can not be done universally.

Or, we can reissue the command.  I don't see any big problem either way.
This is not performance critical.

-Scott