Linux-mtd Archive on lore.kernel.org
 help / color / Atom feed
From: Boris Brezillon <boris.brezillon@collabora.com>
To: masonccyang@mxic.com.tw
Cc: mark.rutland@arm.com, christophe.kerello@st.com,
	marcel.ziswiler@toradex.com, stefan@agner.ch,
	liang.yang@amlogic.com, linux-mtd@lists.infradead.org,
	Miquel Raynal <miquel.raynal@bootlin.com>,
	lee.jones@linaro.org, richard@nod.at, marek.vasut@gmail.com,
	geert@linux-m68k.org, devicetree@vger.kernel.org,
	robh+dt@kernel.org, bbrezillon@kernel.org, juliensu@mxic.com.tw,
	linux-kernel@vger.kernel.org, linux-spi@vger.kernel.org,
	paul.burton@mips.com, broonie@kernel.org,
	computersforpeace@gmail.com, dwmw2@infradead.org,
	zhengxunli@mxic.com.tw
Subject: Re: [PATCH v3 2/4] mtd: rawnand: Add Macronix MX25F0A NAND controller
Date: Wed, 19 Jun 2019 11:06:43 +0200
Message-ID: <20190619110643.523c1f56@collabora.com> (raw)
In-Reply-To: <OF9EFE4BDE.3166D82B-ON4825841E.00307F51-4825841E.00310FB5@mxic.com.tw>

On Wed, 19 Jun 2019 16:55:52 +0800
masonccyang@mxic.com.tw 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@collabora.com> 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.

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

  reply index

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-15  9:23 [PATCH v3 0/4] Add Macronix MX25F0A MFD driver for raw nand and spi Mason Yang
2019-04-15  9:23 ` [PATCH v3 1/4] mfd: Add Macronix MX25F0A MFD controller driver Mason Yang
2019-05-12 13:08   ` Miquel Raynal
2019-05-15  6:46     ` masonccyang
2019-04-15  9:23 ` [PATCH v3 2/4] mtd: rawnand: Add Macronix MX25F0A NAND controller Mason Yang
2019-05-12 13:18   ` Miquel Raynal
2019-05-15  8:48     ` masonccyang
2019-05-15 12:08       ` Miquel Raynal
     [not found]     ` <OF8A566F14.A2F0F576-ON482583FB.002E7E32-482583FB.003068B1@LocalDomain>
2019-05-15  9:18       ` masonccyang
2019-05-17  9:30     ` masonccyang
2019-05-20 12:23       ` Miquel Raynal
2019-05-23  8:58         ` masonccyang
2019-05-27 12:42           ` Miquel Raynal
2019-05-29  3:12             ` masonccyang
2019-06-17 12:35               ` Miquel Raynal
2019-06-18  1:24                 ` masonccyang
2019-06-18  6:14                   ` Boris Brezillon
2019-06-18  7:29                     ` Boris Brezillon
2019-06-19  8:04                       ` masonccyang
2019-06-19  8:09                         ` Miquel Raynal
2019-06-19  8:48                           ` masonccyang
2019-06-24  9:05                           ` masonccyang
2019-06-19  8:15                         ` Boris Brezillon
2019-06-19  8:55                           ` masonccyang
2019-06-19  9:06                             ` Boris Brezillon [this message]
2019-06-24  8:55                               ` masonccyang
2019-04-15  9:23 ` [PATCH v3 3/4] spi: Patch Macronix SPI controller driver according to MX25F0A MFD driver Mason Yang
2019-04-19 14:51   ` Mark Brown
     [not found]     ` <OF7742B4A9.445066F6-ON482583EC.0037E377-482583EC.0039125B@mxic.com.tw>
2019-05-02  2:41       ` Mark Brown
2019-04-15  9:23 ` [PATCH v3 4/4] dt-bindings: mfd: Document Macronix MX25F0A controller bindings Mason Yang
2019-04-26 22:41   ` Rob Herring
2019-05-12 13:23   ` Miquel Raynal
2019-05-15  7:36     ` masonccyang

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190619110643.523c1f56@collabora.com \
    --to=boris.brezillon@collabora.com \
    --cc=bbrezillon@kernel.org \
    --cc=broonie@kernel.org \
    --cc=christophe.kerello@st.com \
    --cc=computersforpeace@gmail.com \
    --cc=devicetree@vger.kernel.org \
    --cc=dwmw2@infradead.org \
    --cc=geert@linux-m68k.org \
    --cc=juliensu@mxic.com.tw \
    --cc=lee.jones@linaro.org \
    --cc=liang.yang@amlogic.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=linux-spi@vger.kernel.org \
    --cc=marcel.ziswiler@toradex.com \
    --cc=marek.vasut@gmail.com \
    --cc=mark.rutland@arm.com \
    --cc=masonccyang@mxic.com.tw \
    --cc=miquel.raynal@bootlin.com \
    --cc=paul.burton@mips.com \
    --cc=richard@nod.at \
    --cc=robh+dt@kernel.org \
    --cc=stefan@agner.ch \
    --cc=zhengxunli@mxic.com.tw \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-mtd Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-mtd/0 linux-mtd/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-mtd linux-mtd/ https://lore.kernel.org/linux-mtd \
		linux-mtd@lists.infradead.org linux-mtd@archiver.kernel.org
	public-inbox-index linux-mtd


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.infradead.lists.linux-mtd


AGPL code for this site: git clone https://public-inbox.org/ public-inbox