From: masonccyang@mxic.com.tw To: "Boris Brezillon" <boris.brezillon@collabora.com> Cc: bbrezillon@kernel.org, broonie@kernel.org, christophe.kerello@st.com, computersforpeace@gmail.com, devicetree@vger.kernel.org, dwmw2@infradead.org, geert@linux-m68k.org, juliensu@mxic.com.tw, lee.jones@linaro.org, liang.yang@amlogic.com, linux-kernel@vger.kernel.org, linux-mtd@lists.infradead.org, linux-spi@vger.kernel.org, marcel.ziswiler@toradex.com, marek.vasut@gmail.com, mark.rutland@arm.com, "Miquel Raynal" <miquel.raynal@bootlin.com>, paul.burton@mips.com, richard@nod.at, robh+dt@kernel.org, stefan@agner.ch, zhengxunli@mxic.com.tw Subject: Re: [PATCH v3 2/4] mtd: rawnand: Add Macronix MX25F0A NAND controller Date: Mon, 24 Jun 2019 16:55:41 +0800 [thread overview] Message-ID: <OFC97D5691.B26644D3-ON48258423.002F8D47-48258423.00310B3E@mxic.com.tw> (raw) In-Reply-To: <20190619110643.523c1f56@collabora.com> 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. I think I have fixed the problem and the root cause is the our NFC's TX FIFO counter do a unnecessary reset in CS control function. Our NFC implement read-write buffer transfer to send command, address and data. A unnecessary reset to TX FIFO will send a command byte out first and this extra command will make something wrong to next operation. For now, doing CS# control in ->exec_op() is OK to me. thanks & best regards, Mason CONFIDENTIALITY NOTE: This e-mail and any attachments may contain confidential information and/or personal data, which is protected by applicable laws. Please be reminded that duplication, disclosure, distribution, or use of this e-mail (and/or its attachments) or any part thereof is prohibited. If you receive this e-mail in error, please notify us immediately and delete this mail as well as its attachment(s) from your system. In addition, please be informed that collection, processing, and/or use of personal data is prohibited unless expressly permitted by personal data protection laws. Thank you for your attention and cooperation. Macronix International Co., Ltd. ===================================================================== ============================================================================ CONFIDENTIALITY NOTE: This e-mail and any attachments may contain confidential information and/or personal data, which is protected by applicable laws. Please be reminded that duplication, disclosure, distribution, or use of this e-mail (and/or its attachments) or any part thereof is prohibited. If you receive this e-mail in error, please notify us immediately and delete this mail as well as its attachment(s) from your system. In addition, please be informed that collection, processing, and/or use of personal data is prohibited unless expressly permitted by personal data protection laws. Thank you for your attention and cooperation. Macronix International Co., Ltd. =====================================================================
WARNING: multiple messages have this Message-ID (diff)
From: masonccyang@mxic.com.tw To: "Boris Brezillon" <boris.brezillon@collabora.com> 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: Mon, 24 Jun 2019 16:55:41 +0800 [thread overview] Message-ID: <OFC97D5691.B26644D3-ON48258423.002F8D47-48258423.00310B3E@mxic.com.tw> (raw) In-Reply-To: <20190619110643.523c1f56@collabora.com> 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. I think I have fixed the problem and the root cause is the our NFC's TX FIFO counter do a unnecessary reset in CS control function. Our NFC implement read-write buffer transfer to send command, address and data. A unnecessary reset to TX FIFO will send a command byte out first and this extra command will make something wrong to next operation. For now, doing CS# control in ->exec_op() is OK to me. thanks & best regards, Mason CONFIDENTIALITY NOTE: This e-mail and any attachments may contain confidential information and/or personal data, which is protected by applicable laws. Please be reminded that duplication, disclosure, distribution, or use of this e-mail (and/or its attachments) or any part thereof is prohibited. If you receive this e-mail in error, please notify us immediately and delete this mail as well as its attachment(s) from your system. In addition, please be informed that collection, processing, and/or use of personal data is prohibited unless expressly permitted by personal data protection laws. Thank you for your attention and cooperation. Macronix International Co., Ltd. ===================================================================== ============================================================================ CONFIDENTIALITY NOTE: This e-mail and any attachments may contain confidential information and/or personal data, which is protected by applicable laws. Please be reminded that duplication, disclosure, distribution, or use of this e-mail (and/or its attachments) or any part thereof is prohibited. If you receive this e-mail in error, please notify us immediately and delete this mail as well as its attachment(s) from your system. In addition, please be informed that collection, processing, and/or use of personal data is prohibited unless expressly permitted by personal data protection laws. Thank you for your attention and cooperation. Macronix International Co., Ltd. ===================================================================== ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/
next prev parent reply other threads:[~2019-06-24 8:55 UTC|newest] Thread overview: 71+ 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 ` Mason Yang 2019-04-15 9:23 ` [PATCH v3 1/4] mfd: Add Macronix MX25F0A MFD controller driver Mason Yang 2019-04-15 9:23 ` Mason Yang 2019-05-12 13:08 ` Miquel Raynal 2019-05-12 13:08 ` Miquel Raynal 2019-05-15 6:46 ` masonccyang 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-04-15 9:23 ` Mason Yang 2019-05-12 13:18 ` Miquel Raynal 2019-05-12 13:18 ` Miquel Raynal 2019-05-15 8:48 ` masonccyang 2019-05-15 8:48 ` masonccyang 2019-05-15 12:08 ` Miquel Raynal 2019-05-15 12:08 ` Miquel Raynal [not found] ` <OF8A566F14.A2F0F576-ON482583FB.002E7E32-482583FB.003068B1@LocalDomain> 2019-05-15 9:18 ` masonccyang 2019-05-15 9:18 ` masonccyang 2019-05-15 9:18 ` masonccyang 2019-05-17 9:30 ` masonccyang 2019-05-17 9:30 ` masonccyang 2019-05-20 12:23 ` Miquel Raynal 2019-05-20 12:23 ` Miquel Raynal 2019-05-23 8:58 ` masonccyang 2019-05-23 8:58 ` masonccyang 2019-05-27 12:42 ` Miquel Raynal 2019-05-27 12:42 ` Miquel Raynal 2019-05-29 3:12 ` masonccyang 2019-05-29 3:12 ` masonccyang 2019-06-17 12:35 ` Miquel Raynal 2019-06-17 12:35 ` Miquel Raynal 2019-06-18 1:24 ` masonccyang 2019-06-18 1:24 ` masonccyang 2019-06-18 6:14 ` Boris Brezillon 2019-06-18 6:14 ` Boris Brezillon 2019-06-18 7:29 ` Boris Brezillon 2019-06-18 7:29 ` Boris Brezillon 2019-06-19 8:04 ` masonccyang 2019-06-19 8:04 ` masonccyang 2019-06-19 8:09 ` Miquel Raynal 2019-06-19 8:09 ` Miquel Raynal 2019-06-19 8:48 ` masonccyang 2019-06-19 8:48 ` masonccyang 2019-06-24 9:05 ` masonccyang 2019-06-24 9:05 ` masonccyang 2019-06-19 8:15 ` Boris Brezillon 2019-06-19 8:15 ` Boris Brezillon 2019-06-19 8:55 ` masonccyang 2019-06-19 8:55 ` masonccyang 2019-06-19 9:06 ` Boris Brezillon 2019-06-19 9:06 ` Boris Brezillon 2019-06-24 8:55 ` masonccyang [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-15 9:23 ` Mason Yang 2019-04-19 14:51 ` Mark Brown 2019-04-19 14:51 ` Mark Brown 2019-04-29 6:51 ` masonccyang 2019-04-30 10:23 ` masonccyang 2019-05-02 2:41 ` Mark Brown 2019-05-02 2:41 ` Mark Brown 2019-05-02 3:27 ` masonccyang 2019-04-15 9:23 ` [PATCH v3 4/4] dt-bindings: mfd: Document Macronix MX25F0A controller bindings Mason Yang 2019-04-15 9:23 ` Mason Yang 2019-04-26 22:41 ` Rob Herring 2019-04-26 22:41 ` Rob Herring 2019-04-29 8:00 ` masonccyang 2019-05-12 13:23 ` Miquel Raynal 2019-05-12 13:23 ` Miquel Raynal 2019-05-15 7:36 ` masonccyang 2019-05-15 7:36 ` masonccyang
Reply instructions: You may reply publicly 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=OFC97D5691.B26644D3-ON48258423.002F8D47-48258423.00310B3E@mxic.com.tw \ --to=masonccyang@mxic.com.tw \ --cc=bbrezillon@kernel.org \ --cc=boris.brezillon@collabora.com \ --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=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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.