Re: [PATCH v3 2/4] mtd: rawnand: Add Macronix MX25F0A NAND controller

[Boris Brezillon <boris.brezillon@xxxxxxxxxxxxx>]

On Wed, 19 Jun 2019 16:55:52 +0800
masonccyang@xxxxxxxxxxx wrote:

> Hi Boris,
> 
> > > > 
> > > > Re: [PATCH v3 2/4] mtd: rawnand: Add Macronix MX25F0A NAND   
> controller
> > > > 
> > > > On Tue, 18 Jun 2019 08:14:36 +0200
> > > > Boris Brezillon <boris.brezillon@xxxxxxxxxxxxx> wrote:
> > > >   
> > > > > > > > > > 
> > > > > > > > > > How to make all #CS keep high for NAND to enter 
> > > > > > > > > > low-power standby mode if driver don't use   
> > > "legacy.select_chip()"   
> > > > > > ?   
> > > > > > > > > 
> > > > > > > > > See commit 02b4a52604a4 ("mtd: rawnand: Make   
> ->select_chip()   
> > > > > > optional   
> > > > > > > > > when ->exec_op() is implemented") which states:
> > > > > > > > > 
> > > > > > > > >         "When [->select_chip() is] not implemented, the   
> core 
> > > is   
> > > > > > assuming   
> > > > > > > > >    the CS line is automatically asserted/deasserted by the   
>  
> > > driver   
> > > > > > > > >    ->exec_op() implementation."   
> > > > > > > > > 
> > > > > > > > > Of course, the above is right only when the controller   
> driver 
> > >   
> > > > > > supports   
> > > > > > > > > the ->exec_op() interface.   
> > > > > > > > 
> > > > > > > > Currently, it seems that we will get the incorrect data and    
> 
> > > error  
> > > > > > > > operation due to CS in error toggling if CS line is   
> controlled 
> > > in   
> > > > > > > > ->exec_op().   
> > > > 
> > > > Oh, and please provide the modifications you added on top of this   
> patch.
> > > > Right now we're speculating on what you've done which is definitely   
> not
> > > > an efficient way to debug this sort of issues.   
> > > 
> > > The patch is to add in beginning of ->exec_op() to control CS# low and   
> 
> > > before return from ->exec_op() to control CS# High.
> > > i.e,.
> > > static in mxic_nand_exec_op( )
> > > {
> > >  cs_to_low();
> > > 
> > > 
> > > 
> > >  cs_to_high();
> > >  return;
> > > }
> > > 
> > > But for nand_onfi_detect(), 
> > > it calls nand_read_param_page_op() and then nand_read_data_op().
> > > mxic_nand_exec_op() be called twice for nand_onfi_detect() and
> > > driver will get incorrect ONFI parameter table data from 
> > > nand_read_data_op().  
> > 
> > And I think it's valid to release the CE pin between
> > read_param_page_op() (CMD(0xEC)+ADDR(0x0)) and read_data_op() (data
> > cycles) if your chip is CE-dont-care compliant. So, either you have a
> > problem with your controller driver (CS-related timings are incorrect)
> > or your chip is not CE-dont-care compliant.  
> 
> Understood, I will try to fix it on my NFC driver.

Before you do that, can you please try to understand where the problem
comes from and explain it to us? Hacking the NFC driver is only
meaningful if the problem is on the NFC side. If your NAND chip does
not support when the CS pin goes high between read_param_page_op() and
read_data_op() the problem should be fixed in the core.