RE: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
Hi Miquel, > -Original Message- > From: Miquel Raynal > Sent: Thursday, June 27, 2019 9:58 PM > To: Naga Sureshkumar Relli > Cc: Naga Sureshkumar Relli ; r...@kernel.org; > martin.lund@keep-it- > simple.com; rich...@nod.at; linux-kernel@vger.kernel.org; Boris Brezillon > ; linux-...@lists.infradead.org; > nagasures...@gmail.com; > Michal Simek ; computersforpe...@gmail.com; > dw...@infradead.org; > marek.va...@gmail.com > Subject: Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for > Arasan NAND > Flash Controller > > Hi Naga, > > Naga Sureshkumar Relli wrote on Tue, > 18 Jun 2019 22:44:24 -0600: > > > On Mon, Jan 28, 2019 at 10:27:39AM +0100, Miquel Raynal wrote: > > Hi Miquel, > > > > > Hi Naga, > > > > > > Naga Sureshkumar Relli wrote on Mon, 28 Jan > > > 2019 > > > 06:04:53 +: > > > > > > > Hi Boris & Miquel, > > > > > > > > Could you please provide your thoughts on this driver to support HW-ECC? > > > > As I said previously, there is no way to detect errors beyond N bit. > > > > I am ok to update the driver based on your inputs. > > > > > > We won't support the ECC engine. It simply cannot be used reliably. > > > > > > I am working on a generic ECC engine object. It's gonna take a few > > > months until it gets merged but after that you could update the > > > controller driver to drop any ECC-related function. Although the ECC > > > > Could you please let me know that, when can we expect generic ECC > > engine update in mtd NAND? > > Based on that, i will plan to update the ARASAN NAND driver along with > > your comments mentioned above under this update, as you know there is > > a limiation in ARASAN NAND controller to detect ECC errors. > > i am following this series > > https://patchwork.kernel.org/patch/10838705/ > > It is gonna take more time than expected. You can stick to the software > engines for now. Ok. I will update the driver accordingly. Thanks, Naga Sureshkumar Relli > > Thanks, > Miquèl
Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
Hi Naga, Naga Sureshkumar Relli wrote on Tue, 18 Jun 2019 22:44:24 -0600: > On Mon, Jan 28, 2019 at 10:27:39AM +0100, Miquel Raynal wrote: > Hi Miquel, > > > Hi Naga, > > > > Naga Sureshkumar Relli wrote on Mon, 28 Jan 2019 > > 06:04:53 +: > > > > > Hi Boris & Miquel, > > > > > > Could you please provide your thoughts on this driver to support HW-ECC? > > > As I said previously, there is no way to detect errors beyond N bit. > > > I am ok to update the driver based on your inputs. > > > > We won't support the ECC engine. It simply cannot be used reliably. > > > > I am working on a generic ECC engine object. It's gonna take a few > > months until it gets merged but after that you could update the > > controller driver to drop any ECC-related function. Although the ECC > > Could you please let me know that, when can we expect generic ECC engine > update in mtd NAND? > Based on that, i will plan to update the ARASAN NAND driver along with your > comments mentioned above under this update, > as you know there is a limiation in ARASAN NAND controller to detect > ECC errors. > i am following this series https://patchwork.kernel.org/patch/10838705/ It is gonna take more time than expected. You can stick to the software engines for now. Thanks, Miquèl
Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
On Mon, Jan 28, 2019 at 10:27:39AM +0100, Miquel Raynal wrote: Hi Miquel, > Hi Naga, > > Naga Sureshkumar Relli wrote on Mon, 28 Jan 2019 > 06:04:53 +: > > > Hi Boris & Miquel, > > > > Could you please provide your thoughts on this driver to support HW-ECC? > > As I said previously, there is no way to detect errors beyond N bit. > > I am ok to update the driver based on your inputs. > > We won't support the ECC engine. It simply cannot be used reliably. > > I am working on a generic ECC engine object. It's gonna take a few > months until it gets merged but after that you could update the > controller driver to drop any ECC-related function. Although the ECC Could you please let me know that, when can we expect generic ECC engine update in mtd NAND? Based on that, i will plan to update the ARASAN NAND driver along with your comments mentioned above under this update, as you know there is a limiation in ARASAN NAND controller to detect ECC errors. i am following this series https://patchwork.kernel.org/patch/10838705/ Thanks, Naga Sureshkumar Relli > engine part is blocking, raw access still look wrong and the driver > still needs changes. > > > Thanks, > Miquèl > > __ > Linux MTD discussion mailing list > http://lists.infradead.org/mailman/listinfo/linux-mtd/
RE: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
Hi Miquel, > -Original Message- > From: Miquel Raynal [mailto:miquel.ray...@bootlin.com] > Sent: Monday, January 28, 2019 2:58 PM > To: Naga Sureshkumar Relli > Cc: r...@kernel.org; marek.va...@gmail.com; rich...@nod.at; > martin.lund@keep-it- > simple.com; linux-kernel@vger.kernel.org; Boris Brezillon > ; > linux-...@lists.infradead.org; nagasures...@gmail.com; Michal Simek > ; computersforpe...@gmail.com; dw...@infradead.org > Subject: Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for > Arasan > NAND Flash Controller > > Hi Naga, > > Naga Sureshkumar Relli wrote on Mon, 28 Jan 2019 > 06:04:53 +: > > > Hi Boris & Miquel, > > > > Could you please provide your thoughts on this driver to support HW-ECC? > > As I said previously, there is no way to detect errors beyond N bit. > > I am ok to update the driver based on your inputs. > > We won't support the ECC engine. It simply cannot be used reliably. > > I am working on a generic ECC engine object. It's gonna take a few months > until it gets > merged but after that you could update the controller driver to drop any > ECC-related > function. Although the ECC engine part is blocking, raw access still look > wrong and the > driver still needs changes. Thank you for the update. Yes, currently ECC engine part is blocking. Also whenever you have time, please provide your comments to the driver v12 patch. I will update those. Thanks, Naga Sureshkumar Relli > > > Thanks, > Miquèl
Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
Hi Naga, Naga Sureshkumar Relli wrote on Mon, 28 Jan 2019 06:04:53 +: > Hi Boris & Miquel, > > Could you please provide your thoughts on this driver to support HW-ECC? > As I said previously, there is no way to detect errors beyond N bit. > I am ok to update the driver based on your inputs. We won't support the ECC engine. It simply cannot be used reliably. I am working on a generic ECC engine object. It's gonna take a few months until it gets merged but after that you could update the controller driver to drop any ECC-related function. Although the ECC engine part is blocking, raw access still look wrong and the driver still needs changes. Thanks, Miquèl
RE: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
Hi Boris & Miquel, Could you please provide your thoughts on this driver to support HW-ECC? As I said previously, there is no way to detect errors beyond N bit. I am ok to update the driver based on your inputs. Thanks, Naga Sureshkumar Relli > -Original Message- > From: linux-mtd [mailto:linux-mtd-boun...@lists.infradead.org] On Behalf Of > Naga > Sureshkumar Relli > Sent: Friday, December 21, 2018 1:06 PM > To: Miquel Raynal > Cc: r...@kernel.org; marek.va...@gmail.com; rich...@nod.at; > martin.lund@keep-it- > simple.com; linux-kernel@vger.kernel.org; Boris Brezillon > ; > linux-...@lists.infradead.org; nagasures...@gmail.com; Michal Simek > ; computersforpe...@gmail.com; dw...@infradead.org > Subject: RE: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for > Arasan > NAND Flash Controller > > Hi Miquel, > > > -Original Message- > > From: Miquel Raynal [mailto:miquel.ray...@bootlin.com] > > Sent: Wednesday, December 19, 2018 7:57 PM > > To: Naga Sureshkumar Relli > > Cc: Boris Brezillon ; r...@kernel.org; > > rich...@nod.at; linux-kernel@vger.kernel.org; marek.va...@gmail.com; > > linux-...@lists.infradead.org; nagasures...@gmail.com; Michal Simek > > ; computersforpe...@gmail.com; > > dw...@infradead.org; martin.l...@keep-it-simple.com > > Subject: Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support > > for Arasan NAND Flash Controller > > > > Hi Naga, > > > > + Martin > > > > Naga Sureshkumar Relli wrote on Tue, 18 Dec 2018 > > 05:33:53 +: > > > > > Hi Miquel, > > > > > > > -Original Message- > > > > From: Miquel Raynal [mailto:miquel.ray...@bootlin.com] > > > > Sent: Monday, December 17, 2018 10:11 PM > > > > To: Naga Sureshkumar Relli > > > > Cc: Boris Brezillon ; > > > > r...@kernel.org; rich...@nod.at; linux- ker...@vger.kernel.org; > > > > marek.va...@gmail.com; linux-...@lists.infradead.org; > > > > nagasures...@gmail.com; Michal Simek ; > > > > computersforpe...@gmail.com; dw...@infradead.org > > > > Subject: Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add > > > > support for Arasan NAND Flash Controller > > > > > > > > Hi Naga, > > > > > > > > [...] > > > > > > > > > Inserted biterror @ 48/7 > > > > > Successfully corrected 25 bit errors per subpage Inserted > > > > > biterror @ > > > > > 50/7 ECC failure, invalid data despite read success > > > > > root@xilinx-zc1751-dc2-2018_1:~# > > > > > > > > > > But even in this case also, driver is saying ECC failure but read > > > > > success. > > > > > That means controller is able to detect errors on read page up to 24 > > > > > bit only. > > > > > After that there is no way to say to the upper layers that the > > > > > page is bad because of the > > > > limitation in the controller. > > > > > > > > This is more than a "limitation", the design is broken. I am not > > > > sure how to support such controller, and I am not sure if we even want > > > > to. > > > > > > The number of errors that are correctable is limited by a parameter > > > 't'(total number of errors), If there is a condition that the number > > > of errors greater than 't', > > then the controller won't be able to detect that. > > > I guess this concept is same for other controllers as well. > > > In Arasan it is limited to 24-bit. > > > > > > Even, in case of Hamming, it is 1-bit error correction and 2-bit error > > > detection. > > > What will happen if there are multiple errors(greater than 2-bit)? > > > > Ok let's use the Hamming comparison in your ECC engine case. > > > > -> hamming: > > * 0 bf: everything is fine > > * 1 bf: will be detected, corrected, signaled > > * 2 bf: will be detected, not corrected, signaled > > * 3+ bf: don't care > > > > -> BCH: > > * 0 bf: everything is fine > > * 1-24 bf: will be detected, corrected, signaled > > * 25 bf: everything is fine > > * 26+ bf: don't care > > > > Do you see the problem? > No. > > > > In the 25 bf case, the controller is reporting that everything went > > fine while it should report that it detected an uncorrectable situation. > > > > Here are two leads to solve this issue, please investigate them
RE: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
Hi Miquel, > -Original Message- > From: Miquel Raynal [mailto:miquel.ray...@bootlin.com] > Sent: Wednesday, December 19, 2018 7:57 PM > To: Naga Sureshkumar Relli > Cc: Boris Brezillon ; r...@kernel.org; > rich...@nod.at; > linux-kernel@vger.kernel.org; marek.va...@gmail.com; > linux-...@lists.infradead.org; > nagasures...@gmail.com; Michal Simek ; > computersforpe...@gmail.com; dw...@infradead.org; > martin.l...@keep-it-simple.com > Subject: Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for > Arasan > NAND Flash Controller > > Hi Naga, > > + Martin > > Naga Sureshkumar Relli wrote on Tue, 18 Dec 2018 > 05:33:53 +: > > > Hi Miquel, > > > > > -Original Message- > > > From: Miquel Raynal [mailto:miquel.ray...@bootlin.com] > > > Sent: Monday, December 17, 2018 10:11 PM > > > To: Naga Sureshkumar Relli > > > Cc: Boris Brezillon ; r...@kernel.org; > > > rich...@nod.at; linux- ker...@vger.kernel.org; > > > marek.va...@gmail.com; linux-...@lists.infradead.org; > > > nagasures...@gmail.com; Michal Simek ; > > > computersforpe...@gmail.com; dw...@infradead.org > > > Subject: Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support > > > for Arasan NAND Flash Controller > > > > > > Hi Naga, > > > > > > [...] > > > > > > > Inserted biterror @ 48/7 > > > > Successfully corrected 25 bit errors per subpage Inserted biterror > > > > @ > > > > 50/7 ECC failure, invalid data despite read success > > > > root@xilinx-zc1751-dc2-2018_1:~# > > > > > > > > But even in this case also, driver is saying ECC failure but read > > > > success. > > > > That means controller is able to detect errors on read page up to 24 > > > > bit only. > > > > After that there is no way to say to the upper layers that the > > > > page is bad because of the > > > limitation in the controller. > > > > > > This is more than a "limitation", the design is broken. I am not > > > sure how to support such controller, and I am not sure if we even want to. > > > > The number of errors that are correctable is limited by a parameter > > 't'(total number of errors), If there is a condition that the number of > > errors greater than 't', > then the controller won't be able to detect that. > > I guess this concept is same for other controllers as well. > > In Arasan it is limited to 24-bit. > > > > Even, in case of Hamming, it is 1-bit error correction and 2-bit error > > detection. > > What will happen if there are multiple errors(greater than 2-bit)? > > Ok let's use the Hamming comparison in your ECC engine case. > > -> hamming: > * 0 bf: everything is fine > * 1 bf: will be detected, corrected, signaled > * 2 bf: will be detected, not corrected, signaled > * 3+ bf: don't care > > -> BCH: > * 0 bf: everything is fine > * 1-24 bf: will be detected, corrected, signaled > * 25 bf: everything is fine > * 26+ bf: don't care > > Do you see the problem? No. > > In the 25 bf case, the controller is reporting that everything went fine > while it should report > that it detected an uncorrectable situation. > > Here are two leads to solve this issue, please investigate them both: > 1/ Talk to your colleagues that developed the RTL, ask if there is a >hidden/reserved bit for that purpose that is not documented. I spoke to RTL guys, there is nothing hidden/reserved bit for this purpose. I tried reading the status registers reserved bits, but they are raz(read as zero) > 2/ Search for a status in the registers that might indicate that an >error occurred, for instance "0 bf corrected" and "bf have been >detected". I tried reading status registers and other registers as well, but no luck. > > NB: I know that, with a BCH ECC engine, error detection at (strength + > 1) is not 100% sure but statistically it will almost always be detected and > in this case we > need the controller to warn the user! Ok, I understood now. Thanks, Naga Sureshkumar Relli > > > Thanks, > Miquèl
Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
Hi Naga, + Martin Naga Sureshkumar Relli wrote on Tue, 18 Dec 2018 05:33:53 +: > Hi Miquel, > > > -Original Message- > > From: Miquel Raynal [mailto:miquel.ray...@bootlin.com] > > Sent: Monday, December 17, 2018 10:11 PM > > To: Naga Sureshkumar Relli > > Cc: Boris Brezillon ; r...@kernel.org; > > rich...@nod.at; linux- > > ker...@vger.kernel.org; marek.va...@gmail.com; > > linux-...@lists.infradead.org; > > nagasures...@gmail.com; Michal Simek ; > > computersforpe...@gmail.com; dw...@infradead.org > > Subject: Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for > > Arasan > > NAND Flash Controller > > > > Hi Naga, > > > > [...] > > > > > Inserted biterror @ 48/7 > > > Successfully corrected 25 bit errors per subpage Inserted biterror @ > > > 50/7 ECC failure, invalid data despite read success > > > root@xilinx-zc1751-dc2-2018_1:~# > > > > > > But even in this case also, driver is saying ECC failure but read success. > > > That means controller is able to detect errors on read page up to 24 bit > > > only. > > > After that there is no way to say to the upper layers that the page is > > > bad because of the > > limitation in the controller. > > > > This is more than a "limitation", the design is broken. I am not sure how > > to support such > > controller, and I am not sure if we even want to. > > The number of errors that are correctable is limited by a parameter 't'(total > number of errors), > If there is a condition that the number of errors greater than 't', then the > controller won't be able to detect that. > I guess this concept is same for other controllers as well. > In Arasan it is limited to 24-bit. > > Even, in case of Hamming, it is 1-bit error correction and 2-bit error > detection. > What will happen if there are multiple errors(greater than 2-bit)? Ok let's use the Hamming comparison in your ECC engine case. -> hamming: * 0 bf: everything is fine * 1 bf: will be detected, corrected, signaled * 2 bf: will be detected, not corrected, signaled * 3+ bf: don't care -> BCH: * 0 bf: everything is fine * 1-24 bf: will be detected, corrected, signaled * 25 bf: everything is fine * 26+ bf: don't care Do you see the problem? In the 25 bf case, the controller is reporting that everything went fine while it should report that it detected an uncorrectable situation. Here are two leads to solve this issue, please investigate them both: 1/ Talk to your colleagues that developed the RTL, ask if there is a hidden/reserved bit for that purpose that is not documented. 2/ Search for a status in the registers that might indicate that an error occurred, for instance "0 bf corrected" and "bf have been detected". NB: I know that, with a BCH ECC engine, error detection at (strength + 1) is not 100% sure but statistically it will almost always be detected and in this case we need the controller to warn the user! Thanks, Miquèl
RE: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
Hi Miquel, > -Original Message- > From: Miquel Raynal [mailto:miquel.ray...@bootlin.com] > Sent: Monday, December 17, 2018 10:11 PM > To: Naga Sureshkumar Relli > Cc: Boris Brezillon ; r...@kernel.org; > rich...@nod.at; linux- > ker...@vger.kernel.org; marek.va...@gmail.com; linux-...@lists.infradead.org; > nagasures...@gmail.com; Michal Simek ; > computersforpe...@gmail.com; dw...@infradead.org > Subject: Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for > Arasan > NAND Flash Controller > > Hi Naga, > > [...] > > > Inserted biterror @ 48/7 > > Successfully corrected 25 bit errors per subpage Inserted biterror @ > > 50/7 ECC failure, invalid data despite read success > > root@xilinx-zc1751-dc2-2018_1:~# > > > > But even in this case also, driver is saying ECC failure but read success. > > That means controller is able to detect errors on read page up to 24 bit > > only. > > After that there is no way to say to the upper layers that the page is bad > > because of the > limitation in the controller. > > This is more than a "limitation", the design is broken. I am not sure how to > support such > controller, and I am not sure if we even want to. The number of errors that are correctable is limited by a parameter 't'(total number of errors), If there is a condition that the number of errors greater than 't', then the controller won't be able to detect that. I guess this concept is same for other controllers as well. In Arasan it is limited to 24-bit. Even, in case of Hamming, it is 1-bit error correction and 2-bit error detection. What will happen if there are multiple errors(greater than 2-bit)? Thanks, Naga Sureshkumar Relli > > > Could you please suggest any alternative to report the errors in that case? > > Shall we support the controller without the hw ECC engine? Boris, any > thoughts? > > > Thanks, > Miquèl
Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
Hi Naga, [...] > Inserted biterror @ 48/7 > Successfully corrected 25 bit errors per subpage > Inserted biterror @ 50/7 > ECC failure, invalid data despite read success > root@xilinx-zc1751-dc2-2018_1:~# > > But even in this case also, driver is saying ECC failure but read success. > That means controller is able to detect errors on read page up to 24 bit only. > After that there is no way to say to the upper layers that the page is bad > because of the limitation in the controller. This is more than a "limitation", the design is broken. I am not sure how to support such controller, and I am not sure if we even want to. > Could you please suggest any alternative to report the errors in that case? Shall we support the controller without the hw ECC engine? Boris, any thoughts? Thanks, Miquèl
RE: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
Hi Miquel, > -Original Message- > From: Miquel Raynal [mailto:miquel.ray...@bootlin.com] > Sent: Wednesday, December 12, 2018 6:48 PM > To: Naga Sureshkumar Relli > Cc: Boris Brezillon ; r...@kernel.org; > rich...@nod.at; linux- > ker...@vger.kernel.org; marek.va...@gmail.com; linux-...@lists.infradead.org; > nagasures...@gmail.com; Michal Simek ; > computersforpe...@gmail.com; dw...@infradead.org > Subject: Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for > Arasan > NAND Flash Controller > > Hi Naga, > > Naga Sureshkumar Relli wrote on Wed, 12 Dec 2018 > 13:07:42 +: > > > Hi Miquel, > > > > > -Original Message- > > > From: Miquel Raynal [mailto:miquel.ray...@bootlin.com] > > > Sent: Wednesday, December 12, 2018 2:40 PM > > > To: Naga Sureshkumar Relli > > > Cc: Boris Brezillon ; r...@kernel.org; > > > rich...@nod.at; linux- ker...@vger.kernel.org; > > > marek.va...@gmail.com; linux-...@lists.infradead.org; > > > nagasures...@gmail.com; Michal Simek ; > > > computersforpe...@gmail.com; dw...@infradead.org > > > Subject: Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support > > > for Arasan NAND Flash Controller > > > > > > Hi Naga, > > > > > > Naga Sureshkumar Relli wrote on Wed, 12 Dec > > > 2018 > > > 09:04:16 +: > > > > > > > Hi Miquel, > > > > > > > > > -Original Message- > > > > > From: Miquel Raynal [mailto:miquel.ray...@bootlin.com] > > > > > Sent: Wednesday, December 12, 2018 1:42 PM > > > > > To: Naga Sureshkumar Relli > > > > > Cc: Boris Brezillon ; > > > > > r...@kernel.org; rich...@nod.at; linux- ker...@vger.kernel.org; > > > > > marek.va...@gmail.com; linux-...@lists.infradead.org; > > > > > nagasures...@gmail.com; Michal Simek ; > > > > > computersforpe...@gmail.com; dw...@infradead.org > > > > > Subject: Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add > > > > > support for Arasan NAND Flash Controller > > > > > > > > > > Hi Naga, > > > > > > > > > > Naga Sureshkumar Relli wrote on Wed, 12 > > > > > Dec > > > > > 2018 > > > > > 05:27:03 +: > > > > > > > > > > > Hi Boris & Miquel, > > > > > > > > > > > > An update to my comments on thread > > > > > > https://lkml.org/lkml/2018/11/15/656. > > > > > > In this I said, will take a default error count value as 16 > > > > > > and during page read, will check the error count Register > > > > > > value with this and if it is equal to or greater than the > > > > > > default count(16) then I am checking for > > > Erased pages. > > > > > > But bit[7:0] in ECC_Error_Count_Register(0x38) will update for each > > > > > > error > occurred. > > > > > > Link: > > > > > > https://www.xilinx.com/html_docs/registers/ug1087/ug1087-zynq- > > > > > > ultr > > > > > > ascale- > > > > > registers.html and check for NAND module, ECC_Error_Count_Register. > > > > > > > > > > > > I mean previously I dependent on Total error count value > > > > > > (bit[16:8]), but we can simply check for bit[7:0] To see the error > > > > > > occurred or not. > > > > > > I tried with this approach and I don't see any issues with that. > > > > > > I ran ubifs with this and I am able to see the bit[7:0] count > > > > > > is updated for erased page read and then will Use > > > > > > nand_chech_erased_ecc_chunk() to see the > > > > > bitflips. > > > > > > > > > > > > If it is ok, I will update the driver and will send new patch, > > > > > > but do you have any other > > > > > comments on v12? > > > > > > > > > > Is 'nandbiterrs -i' running correctly now? > > > > Yes, but with some changes in driver. > > > > I have added the log and changes done in > > > > https://lkml.org/lkml/2018/11/23/705. > > > > > > No, I don't see a working nandbiterrs there, sorry. > > The log that I have attached is from mtd_nandbiterrs test So as per > > ARASAN controller ECC mechanism, it will correct upto 24-bit. Aft
Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
Hi Naga, Naga Sureshkumar Relli wrote on Wed, 12 Dec 2018 13:07:42 +: > Hi Miquel, > > > -Original Message- > > From: Miquel Raynal [mailto:miquel.ray...@bootlin.com] > > Sent: Wednesday, December 12, 2018 2:40 PM > > To: Naga Sureshkumar Relli > > Cc: Boris Brezillon ; r...@kernel.org; > > rich...@nod.at; linux- > > ker...@vger.kernel.org; marek.va...@gmail.com; > > linux-...@lists.infradead.org; > > nagasures...@gmail.com; Michal Simek ; > > computersforpe...@gmail.com; dw...@infradead.org > > Subject: Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for > > Arasan > > NAND Flash Controller > > > > Hi Naga, > > > > Naga Sureshkumar Relli wrote on Wed, 12 Dec 2018 > > 09:04:16 +: > > > > > Hi Miquel, > > > > > > > -Original Message- > > > > From: Miquel Raynal [mailto:miquel.ray...@bootlin.com] > > > > Sent: Wednesday, December 12, 2018 1:42 PM > > > > To: Naga Sureshkumar Relli > > > > Cc: Boris Brezillon ; r...@kernel.org; > > > > rich...@nod.at; linux- ker...@vger.kernel.org; > > > > marek.va...@gmail.com; linux-...@lists.infradead.org; > > > > nagasures...@gmail.com; Michal Simek ; > > > > computersforpe...@gmail.com; dw...@infradead.org > > > > Subject: Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support > > > > for Arasan NAND Flash Controller > > > > > > > > Hi Naga, > > > > > > > > Naga Sureshkumar Relli wrote on Wed, 12 Dec > > > > 2018 > > > > 05:27:03 +: > > > > > > > > > Hi Boris & Miquel, > > > > > > > > > > An update to my comments on thread > > > > > https://lkml.org/lkml/2018/11/15/656. > > > > > In this I said, will take a default error count value as 16 and > > > > > during page read, will check the error count Register value with > > > > > this and if it is equal to or greater than the default count(16) then > > > > > I am checking for > > Erased pages. > > > > > But bit[7:0] in ECC_Error_Count_Register(0x38) will update for each > > > > > error occurred. > > > > > Link: > > > > > https://www.xilinx.com/html_docs/registers/ug1087/ug1087-zynq-ultr > > > > > ascale- > > > > registers.html and check for NAND module, ECC_Error_Count_Register. > > > > > > > > > > I mean previously I dependent on Total error count value > > > > > (bit[16:8]), but we can simply check for bit[7:0] To see the error > > > > > occurred or not. > > > > > I tried with this approach and I don't see any issues with that. > > > > > I ran ubifs with this and I am able to see the bit[7:0] count is > > > > > updated for erased page read and then will Use > > > > > nand_chech_erased_ecc_chunk() to see the > > > > bitflips. > > > > > > > > > > If it is ok, I will update the driver and will send new patch, but > > > > > do you have any other > > > > comments on v12? > > > > > > > > Is 'nandbiterrs -i' running correctly now? > > > Yes, but with some changes in driver. > > > I have added the log and changes done in > > > https://lkml.org/lkml/2018/11/23/705. > > > > No, I don't see a working nandbiterrs there, sorry. > The log that I have attached is from mtd_nandbiterrs test > So as per ARASAN controller ECC mechanism, it will correct upto 24-bit. After > that the test will fail. There is a distinction between: 1/ The driver fails to correct more than 24-bit and advertise the caller that the page read is somehow corrupted. 2/ The driver fails to correct more than 24-bit but does not complain about it. In our case, the caller (the test tool) will compare the page written and read: if it do not match it means the driver is broken because the driver reported a successful operation despite the fact that it returned a corrupted page. You are in the second case, we expect the driver to behave like in 1/. > > I am running mtd-utils nandbiterr test now. Will let you know once I > completed that. Yes please, prefer using the userspace tools. Thanks, Miquèl
RE: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
Hi Miquel, > -Original Message- > From: Miquel Raynal [mailto:miquel.ray...@bootlin.com] > Sent: Wednesday, December 12, 2018 2:40 PM > To: Naga Sureshkumar Relli > Cc: Boris Brezillon ; r...@kernel.org; > rich...@nod.at; linux- > ker...@vger.kernel.org; marek.va...@gmail.com; linux-...@lists.infradead.org; > nagasures...@gmail.com; Michal Simek ; > computersforpe...@gmail.com; dw...@infradead.org > Subject: Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for > Arasan > NAND Flash Controller > > Hi Naga, > > Naga Sureshkumar Relli wrote on Wed, 12 Dec 2018 > 09:04:16 +: > > > Hi Miquel, > > > > > -Original Message- > > > From: Miquel Raynal [mailto:miquel.ray...@bootlin.com] > > > Sent: Wednesday, December 12, 2018 1:42 PM > > > To: Naga Sureshkumar Relli > > > Cc: Boris Brezillon ; r...@kernel.org; > > > rich...@nod.at; linux- ker...@vger.kernel.org; > > > marek.va...@gmail.com; linux-...@lists.infradead.org; > > > nagasures...@gmail.com; Michal Simek ; > > > computersforpe...@gmail.com; dw...@infradead.org > > > Subject: Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support > > > for Arasan NAND Flash Controller > > > > > > Hi Naga, > > > > > > Naga Sureshkumar Relli wrote on Wed, 12 Dec > > > 2018 > > > 05:27:03 +: > > > > > > > Hi Boris & Miquel, > > > > > > > > An update to my comments on thread https://lkml.org/lkml/2018/11/15/656. > > > > In this I said, will take a default error count value as 16 and > > > > during page read, will check the error count Register value with > > > > this and if it is equal to or greater than the default count(16) then I > > > > am checking for > Erased pages. > > > > But bit[7:0] in ECC_Error_Count_Register(0x38) will update for each > > > > error occurred. > > > > Link: > > > > https://www.xilinx.com/html_docs/registers/ug1087/ug1087-zynq-ultr > > > > ascale- > > > registers.html and check for NAND module, ECC_Error_Count_Register. > > > > > > > > I mean previously I dependent on Total error count value > > > > (bit[16:8]), but we can simply check for bit[7:0] To see the error > > > > occurred or not. > > > > I tried with this approach and I don't see any issues with that. > > > > I ran ubifs with this and I am able to see the bit[7:0] count is > > > > updated for erased page read and then will Use > > > > nand_chech_erased_ecc_chunk() to see the > > > bitflips. > > > > > > > > If it is ok, I will update the driver and will send new patch, but > > > > do you have any other > > > comments on v12? > > > > > > Is 'nandbiterrs -i' running correctly now? > > Yes, but with some changes in driver. > > I have added the log and changes done in > > https://lkml.org/lkml/2018/11/23/705. > > No, I don't see a working nandbiterrs there, sorry. The log that I have attached is from mtd_nandbiterrs test So as per ARASAN controller ECC mechanism, it will correct upto 24-bit. After that the test will fail. I am running mtd-utils nandbiterr test now. Will let you know once I completed that. Thanks, Naga Sureshkumar Relli > > > Thanks, > Miquèl
Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
Hi Naga, Naga Sureshkumar Relli wrote on Wed, 12 Dec 2018 09:04:16 +: > Hi Miquel, > > > -Original Message- > > From: Miquel Raynal [mailto:miquel.ray...@bootlin.com] > > Sent: Wednesday, December 12, 2018 1:42 PM > > To: Naga Sureshkumar Relli > > Cc: Boris Brezillon ; r...@kernel.org; > > rich...@nod.at; linux- > > ker...@vger.kernel.org; marek.va...@gmail.com; > > linux-...@lists.infradead.org; > > nagasures...@gmail.com; Michal Simek ; > > computersforpe...@gmail.com; dw...@infradead.org > > Subject: Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for > > Arasan > > NAND Flash Controller > > > > Hi Naga, > > > > Naga Sureshkumar Relli wrote on Wed, 12 Dec 2018 > > 05:27:03 +: > > > > > Hi Boris & Miquel, > > > > > > An update to my comments on thread https://lkml.org/lkml/2018/11/15/656. > > > In this I said, will take a default error count value as 16 and during > > > page read, will check the error count Register value with this and if > > > it is equal to or greater than the default count(16) then I am checking > > > for Erased pages. > > > But bit[7:0] in ECC_Error_Count_Register(0x38) will update for each error > > > occurred. > > > Link: > > > https://www.xilinx.com/html_docs/registers/ug1087/ug1087-zynq-ultrascale- > > > > > registers.html and check for NAND module, ECC_Error_Count_Register. > > > > > > I mean previously I dependent on Total error count value (bit[16:8]), > > > but we can simply check for bit[7:0] To see the error occurred or not. > > > I tried with this approach and I don't see any issues with that. > > > I ran ubifs with this and I am able to see the bit[7:0] count is > > > updated for erased page read and then will Use > > > nand_chech_erased_ecc_chunk() to see the > > bitflips. > > > > > > If it is ok, I will update the driver and will send new patch, but do you > > > have any other > > comments on v12? > > > > Is 'nandbiterrs -i' running correctly now? > Yes, but with some changes in driver. > I have added the log and changes done in https://lkml.org/lkml/2018/11/23/705. No, I don't see a working nandbiterrs there, sorry. Thanks, Miquèl
RE: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
Hi Miquel, > -Original Message- > From: Miquel Raynal [mailto:miquel.ray...@bootlin.com] > Sent: Wednesday, December 12, 2018 1:42 PM > To: Naga Sureshkumar Relli > Cc: Boris Brezillon ; r...@kernel.org; > rich...@nod.at; linux- > ker...@vger.kernel.org; marek.va...@gmail.com; linux-...@lists.infradead.org; > nagasures...@gmail.com; Michal Simek ; > computersforpe...@gmail.com; dw...@infradead.org > Subject: Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for > Arasan > NAND Flash Controller > > Hi Naga, > > Naga Sureshkumar Relli wrote on Wed, 12 Dec 2018 > 05:27:03 +: > > > Hi Boris & Miquel, > > > > An update to my comments on thread https://lkml.org/lkml/2018/11/15/656. > > In this I said, will take a default error count value as 16 and during > > page read, will check the error count Register value with this and if > > it is equal to or greater than the default count(16) then I am checking for > > Erased pages. > > But bit[7:0] in ECC_Error_Count_Register(0x38) will update for each error > > occurred. > > Link: > > https://www.xilinx.com/html_docs/registers/ug1087/ug1087-zynq-ultrascale- > registers.html and check for NAND module, ECC_Error_Count_Register. > > > > I mean previously I dependent on Total error count value (bit[16:8]), > > but we can simply check for bit[7:0] To see the error occurred or not. > > I tried with this approach and I don't see any issues with that. > > I ran ubifs with this and I am able to see the bit[7:0] count is > > updated for erased page read and then will Use > > nand_chech_erased_ecc_chunk() to see the > bitflips. > > > > If it is ok, I will update the driver and will send new patch, but do you > > have any other > comments on v12? > > Is 'nandbiterrs -i' running correctly now? Yes, but with some changes in driver. I have added the log and changes done in https://lkml.org/lkml/2018/11/23/705. Do you see any issues with that approach? Thanks, Naga Sureshkumar Relli.
Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
Hi Naga, Naga Sureshkumar Relli wrote on Wed, 12 Dec 2018 05:27:03 +: > Hi Boris & Miquel, > > An update to my comments on thread https://lkml.org/lkml/2018/11/15/656. > In this I said, will take a default error count value as 16 and during page > read, will check the error count > Register value with this and if it is equal to or greater than the default > count(16) then I am checking for > Erased pages. > But bit[7:0] in ECC_Error_Count_Register(0x38) will update for each error > occurred. > Link: > https://www.xilinx.com/html_docs/registers/ug1087/ug1087-zynq-ultrascale-registers.html > and > check for NAND module, ECC_Error_Count_Register. > > I mean previously I dependent on Total error count value (bit[16:8]), but we > can simply check for bit[7:0] > To see the error occurred or not. > I tried with this approach and I don't see any issues with that. > I ran ubifs with this and I am able to see the bit[7:0] count is updated for > erased page read and then will > Use nand_chech_erased_ecc_chunk() to see the bitflips. > > If it is ok, I will update the driver and will send new patch, but do you > have any other comments on v12? Is 'nandbiterrs -i' running correctly now? Thanks, Miquèl
RE: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
Hi Boris & Miquel, An update to my comments on thread https://lkml.org/lkml/2018/11/15/656. In this I said, will take a default error count value as 16 and during page read, will check the error count Register value with this and if it is equal to or greater than the default count(16) then I am checking for Erased pages. But bit[7:0] in ECC_Error_Count_Register(0x38) will update for each error occurred. Link: https://www.xilinx.com/html_docs/registers/ug1087/ug1087-zynq-ultrascale-registers.html and check for NAND module, ECC_Error_Count_Register. I mean previously I dependent on Total error count value (bit[16:8]), but we can simply check for bit[7:0] To see the error occurred or not. I tried with this approach and I don't see any issues with that. I ran ubifs with this and I am able to see the bit[7:0] count is updated for erased page read and then will Use nand_chech_erased_ecc_chunk() to see the bitflips. If it is ok, I will update the driver and will send new patch, but do you have any other comments on v12? Thanks, Naga Sureshkumar Relli > -Original Message- > From: linux-mtd [mailto:linux-mtd-boun...@lists.infradead.org] On Behalf Of > Naga > Sureshkumar Relli > Sent: Friday, November 23, 2018 7:24 PM > To: Miquel Raynal ; Boris Brezillon > > Cc: r...@kernel.org; rich...@nod.at; linux-kernel@vger.kernel.org; > marek.va...@gmail.com; > linux-...@lists.infradead.org; nagasures...@gmail.com; Michal Simek > ; computersforpe...@gmail.com; dw...@infradead.org > Subject: RE: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for > Arasan > NAND Flash Controller > > Hi Boris & Miquel, > > > -Original Message- > > From: Miquel Raynal [mailto:miquel.ray...@bootlin.com] > > Sent: Tuesday, November 20, 2018 6:06 PM > > To: Boris Brezillon > > Cc: Naga Sureshkumar Relli ; rich...@nod.at; > > dw...@infradead.org; computersforpe...@gmail.com; marek.va...@gmail.com; > > linux- > > m...@lists.infradead.org; linux-kernel@vger.kernel.org; > > nagasures...@gmail.com; > > r...@kernel.org; Michal Simek > > Subject: Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for > > Arasan > > NAND Flash Controller > > > > Hi Naga, > > > > Boris Brezillon wrote on Tue, 20 Nov 2018 > > 12:02:44 +0100: > > > > > On Tue, 20 Nov 2018 07:02:08 + > > > Naga Sureshkumar Relli wrote: > > > > > > > > > > > > > > > > Can you please run nandbiterrs (availaible in mtd-utils). I fear your > > > > > device won't pass the test. > > > > Yes, nandbiterror test is passing till 24bit, after that it is failing. > > > > > > Can you paste the output of nandbiterrs please? > > > > Apparently 'nandbiterrs -i 'just crashes the kernel because of a > > segmentation fault. Please > run > > this test (from the mtd-utils package) and fix this issue. Then we would > > like to see the > output. > Here is the output of mtd_nandbiterrs, > [ 1830.546807] mtd_nandbiterrs: verify_page > [ 1830.551924] mtd_nandbiterrs: Successfully corrected 8 bit errors per > subpage > [ 1830.558961] mtd_nandbiterrs: Inserted biterror @ 2/5 > [ 1830.563917] mtd_nandbiterrs: rewrite page > [ 1830.568216] mtd_nandbiterrs: read_page > [ 1830.572155] mtd_nandbiterrs: verify_page > [ 1830.576531] mtd_nandbiterrs: Successfully corrected 9 bit errors per > subpage > [ 1830.583568] mtd_nandbiterrs: Inserted biterror @ 2/2 > [ 1830.588527] mtd_nandbiterrs: rewrite page > [ 1830.592881] mtd_nandbiterrs: read_page > [ 1830.596825] mtd_nandbiterrs: verify_page > [ 1830.601197] mtd_nandbiterrs: Successfully corrected 10 bit errors per > subpage > [ 1830.608326] mtd_nandbiterrs: Inserted biterror @ 2/0 > [ 1830.613279] mtd_nandbiterrs: rewrite page > [ 1830.617585] mtd_nandbiterrs: read_page > [ 1830.621524] mtd_nandbiterrs: verify_page > [ 1830.625900] mtd_nandbiterrs: Successfully corrected 11 bit errors per > subpage > [ 1830.633027] mtd_nandbiterrs: Inserted biterror @ 3/7 > [ 1830.637984] mtd_nandbiterrs: rewrite page > [ 1830.642281] mtd_nandbiterrs: read_page > [ 1830.646223] mtd_nandbiterrs: verify_page > [ 1830.650595] mtd_nandbiterrs: Successfully corrected 12 bit errors per > subpage > [ 1830.657724] mtd_nandbiterrs: Inserted biterror @ 3/6 > [ 1830.662677] mtd_nandbiterrs: rewrite page > [ 1830.666983] mtd_nandbiterrs: read_page > [ 1830.670922] mtd_nandbiterrs: verify_page > [ 1830.675296] mtd_nandbiterrs: Successfully corrected 13 bit errors per > subpage > [ 1830.682417] mtd_nandbiterrs: Inserted biterror @ 3/5 > [ 1830.687373] mtd_nandbiterrs: rewrite page > [ 1830.691671] mtd_nand
RE: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
Hi Boris,
> -Original Message-
> From: Boris Brezillon [mailto:boris.brezil...@bootlin.com]
> Sent: Tuesday, November 20, 2018 9:55 PM
> To: Naga Sureshkumar Relli
> Cc: miquel.ray...@bootlin.com; rich...@nod.at; dw...@infradead.org;
> computersforpe...@gmail.com; marek.va...@gmail.com; Michal Simek
> ; nagasures...@gmail.com; linux-...@lists.infradead.org;
> linux-
> ker...@vger.kernel.org; r...@kernel.org
> Subject: Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for
> Arasan
> NAND Flash Controller
>
> On Fri, 9 Nov 2018 10:30:41 +0530
> Naga Sureshkumar Relli wrote:
>
> > +static int anfc_setup_data_interface(struct nand_chip *chip, int csline,
> > +const struct nand_data_interface *conf) {
> > + struct anfc_nand_controller *nfc = to_anfc(chip->controller);
> > + struct anfc_nand_chip *achip = to_anfc_nand(chip);
> > + int err;
> > + const struct nand_sdr_timings *sdr;
> > + u32 inftimeval;
> > + bool change_sdr_clk = false;
> > +
> > + if (csline == NAND_DATA_IFACE_CHECK_ONLY)
> > + return 0;
> > +
> > + /*
> > +* If the controller is already in the same mode as flash device
> > +* then no need to change the timing mode again.
> > +*/
> > + sdr = nand_get_sdr_timings(conf);
> > + if (IS_ERR(sdr))
> > + return PTR_ERR(sdr);
> > +
> > + if (sdr->mode < 0)
> > + return -ENOTSUPP;
> > +
> > + inftimeval = sdr->mode & 7;
> > + if (sdr->mode >= 2 && sdr->mode <= 5)
> > + change_sdr_clk = true;
> > + /*
> > +* SDR timing modes 2-5 will not work for the arasan nand when
> > +* freq > 90 MHz, so reduce the freq in SDR modes 2-5 to < 90Mhz
>
> What's the freq for mode 0 and 1?
It is 100MHz in SDR mode 0 and 1.
>
> > +*/
> > + if (change_sdr_clk) {
> > + clk_disable_unprepare(nfc->clk_sys);
> > + err = clk_set_rate(nfc->clk_sys, SDR_MODE_DEFLT_FREQ);
>
> You should not change the clk rate here. It should be done when the chip is
> selected, so that,
> if you ever have 2 different chips connected to the same controller, you can
> adapt the rate
> when they are accessed.
Ok, got it. I will update.
>
> > + if (err) {
> > + dev_err(nfc->dev, "Can't set the clock rate\n");
> > + return err;
> > + }
> > + err = clk_prepare_enable(nfc->clk_sys);
> > + if (err) {
> > + dev_err(nfc->dev, "Unable to enable sys clock.\n");
> > + clk_disable_unprepare(nfc->clk_sys);
> > + return err;
> > + }
> > + }
> > + achip->inftimeval = inftimeval;
> > +
> > + return 0;
> > +}
> > +
> > +static int anfc_nand_attach_chip(struct nand_chip *chip) {
> > + struct mtd_info *mtd = nand_to_mtd(chip);
> > + struct anfc_nand_chip *achip = to_anfc_nand(chip);
> > + u32 ret;
> > +
> > + if (mtd->writesize <= SZ_512)
> > + achip->spare_caddr_cycles = 1;
> > + else
> > + achip->spare_caddr_cycles = 2;
> > +
> > + chip->ecc.calc_buf = kmalloc(mtd->oobsize, GFP_KERNEL);
> > + chip->ecc.code_buf = kmalloc(mtd->oobsize, GFP_KERNEL);
>
> Those bufs are allocated but never freed (memleak). Also, are you sure you
> really need them.
These bufs are freed in nand_release(), which is calling from anfc_remove().
And chip->ecc.code_buf, is used in anfc_read_page_hwecc().
What we are doing here is, extract ECC code from OOB and place it in
ecv.code_buf, and passing this info to nand_check_ecc_chunk(buf,
chip->ecc.size, _code[i], eccbytes, NULL, 0,chip->ecc.strength).
i.e. just to store ECC code from OOB area.
And chip->ecc.calc_buf is no where used in the driver, I will remove it.
Thanks,
Naga Sureshkumar Relli.
RE: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
Hi Boris,
> -Original Message-
> From: Boris Brezillon [mailto:boris.brezil...@bootlin.com]
> Sent: Tuesday, November 20, 2018 9:55 PM
> To: Naga Sureshkumar Relli
> Cc: miquel.ray...@bootlin.com; rich...@nod.at; dw...@infradead.org;
> computersforpe...@gmail.com; marek.va...@gmail.com; Michal Simek
> ; nagasures...@gmail.com; linux-...@lists.infradead.org;
> linux-
> ker...@vger.kernel.org; r...@kernel.org
> Subject: Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for
> Arasan
> NAND Flash Controller
>
> On Fri, 9 Nov 2018 10:30:41 +0530
> Naga Sureshkumar Relli wrote:
>
> > +static int anfc_setup_data_interface(struct nand_chip *chip, int csline,
> > +const struct nand_data_interface *conf) {
> > + struct anfc_nand_controller *nfc = to_anfc(chip->controller);
> > + struct anfc_nand_chip *achip = to_anfc_nand(chip);
> > + int err;
> > + const struct nand_sdr_timings *sdr;
> > + u32 inftimeval;
> > + bool change_sdr_clk = false;
> > +
> > + if (csline == NAND_DATA_IFACE_CHECK_ONLY)
> > + return 0;
> > +
> > + /*
> > +* If the controller is already in the same mode as flash device
> > +* then no need to change the timing mode again.
> > +*/
> > + sdr = nand_get_sdr_timings(conf);
> > + if (IS_ERR(sdr))
> > + return PTR_ERR(sdr);
> > +
> > + if (sdr->mode < 0)
> > + return -ENOTSUPP;
> > +
> > + inftimeval = sdr->mode & 7;
> > + if (sdr->mode >= 2 && sdr->mode <= 5)
> > + change_sdr_clk = true;
> > + /*
> > +* SDR timing modes 2-5 will not work for the arasan nand when
> > +* freq > 90 MHz, so reduce the freq in SDR modes 2-5 to < 90Mhz
>
> What's the freq for mode 0 and 1?
It is 100MHz in SDR mode 0 and 1.
>
> > +*/
> > + if (change_sdr_clk) {
> > + clk_disable_unprepare(nfc->clk_sys);
> > + err = clk_set_rate(nfc->clk_sys, SDR_MODE_DEFLT_FREQ);
>
> You should not change the clk rate here. It should be done when the chip is
> selected, so that,
> if you ever have 2 different chips connected to the same controller, you can
> adapt the rate
> when they are accessed.
Ok, got it. I will update.
>
> > + if (err) {
> > + dev_err(nfc->dev, "Can't set the clock rate\n");
> > + return err;
> > + }
> > + err = clk_prepare_enable(nfc->clk_sys);
> > + if (err) {
> > + dev_err(nfc->dev, "Unable to enable sys clock.\n");
> > + clk_disable_unprepare(nfc->clk_sys);
> > + return err;
> > + }
> > + }
> > + achip->inftimeval = inftimeval;
> > +
> > + return 0;
> > +}
> > +
> > +static int anfc_nand_attach_chip(struct nand_chip *chip) {
> > + struct mtd_info *mtd = nand_to_mtd(chip);
> > + struct anfc_nand_chip *achip = to_anfc_nand(chip);
> > + u32 ret;
> > +
> > + if (mtd->writesize <= SZ_512)
> > + achip->spare_caddr_cycles = 1;
> > + else
> > + achip->spare_caddr_cycles = 2;
> > +
> > + chip->ecc.calc_buf = kmalloc(mtd->oobsize, GFP_KERNEL);
> > + chip->ecc.code_buf = kmalloc(mtd->oobsize, GFP_KERNEL);
>
> Those bufs are allocated but never freed (memleak). Also, are you sure you
> really need them.
These bufs are freed in nand_release(), which is calling from anfc_remove().
And chip->ecc.code_buf, is used in anfc_read_page_hwecc().
What we are doing here is, extract ECC code from OOB and place it in
ecv.code_buf, and passing this info to nand_check_ecc_chunk(buf,
chip->ecc.size, _code[i], eccbytes, NULL, 0,chip->ecc.strength).
i.e. just to store ECC code from OOB area.
And chip->ecc.calc_buf is no where used in the driver, I will remove it.
Thanks,
Naga Sureshkumar Relli.
RE: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
Hi Boris & Miquel, > -Original Message- > From: Miquel Raynal [mailto:miquel.ray...@bootlin.com] > Sent: Tuesday, November 20, 2018 6:06 PM > To: Boris Brezillon > Cc: Naga Sureshkumar Relli ; rich...@nod.at; > dw...@infradead.org; computersforpe...@gmail.com; marek.va...@gmail.com; > linux- > m...@lists.infradead.org; linux-kernel@vger.kernel.org; > nagasures...@gmail.com; > r...@kernel.org; Michal Simek > Subject: Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for > Arasan > NAND Flash Controller > > Hi Naga, > > Boris Brezillon wrote on Tue, 20 Nov 2018 > 12:02:44 +0100: > > > On Tue, 20 Nov 2018 07:02:08 + > > Naga Sureshkumar Relli wrote: > > > > > > > > > > > > Can you please run nandbiterrs (availaible in mtd-utils). I fear your > > > > device won't pass the test. > > > Yes, nandbiterror test is passing till 24bit, after that it is failing. > > > > Can you paste the output of nandbiterrs please? > > Apparently 'nandbiterrs -i 'just crashes the kernel because of a segmentation > fault. Please run > this test (from the mtd-utils package) and fix this issue. Then we would like > to see the output. Here is the output of mtd_nandbiterrs, [ 1830.546807] mtd_nandbiterrs: verify_page [ 1830.551924] mtd_nandbiterrs: Successfully corrected 8 bit errors per subpage [ 1830.558961] mtd_nandbiterrs: Inserted biterror @ 2/5 [ 1830.563917] mtd_nandbiterrs: rewrite page [ 1830.568216] mtd_nandbiterrs: read_page [ 1830.572155] mtd_nandbiterrs: verify_page [ 1830.576531] mtd_nandbiterrs: Successfully corrected 9 bit errors per subpage [ 1830.583568] mtd_nandbiterrs: Inserted biterror @ 2/2 [ 1830.588527] mtd_nandbiterrs: rewrite page [ 1830.592881] mtd_nandbiterrs: read_page [ 1830.596825] mtd_nandbiterrs: verify_page [ 1830.601197] mtd_nandbiterrs: Successfully corrected 10 bit errors per subpage [ 1830.608326] mtd_nandbiterrs: Inserted biterror @ 2/0 [ 1830.613279] mtd_nandbiterrs: rewrite page [ 1830.617585] mtd_nandbiterrs: read_page [ 1830.621524] mtd_nandbiterrs: verify_page [ 1830.625900] mtd_nandbiterrs: Successfully corrected 11 bit errors per subpage [ 1830.633027] mtd_nandbiterrs: Inserted biterror @ 3/7 [ 1830.637984] mtd_nandbiterrs: rewrite page [ 1830.642281] mtd_nandbiterrs: read_page [ 1830.646223] mtd_nandbiterrs: verify_page [ 1830.650595] mtd_nandbiterrs: Successfully corrected 12 bit errors per subpage [ 1830.657724] mtd_nandbiterrs: Inserted biterror @ 3/6 [ 1830.662677] mtd_nandbiterrs: rewrite page [ 1830.666983] mtd_nandbiterrs: read_page [ 1830.670922] mtd_nandbiterrs: verify_page [ 1830.675296] mtd_nandbiterrs: Successfully corrected 13 bit errors per subpage [ 1830.682417] mtd_nandbiterrs: Inserted biterror @ 3/5 [ 1830.687373] mtd_nandbiterrs: rewrite page [ 1830.691671] mtd_nandbiterrs: read_page [ 1830.695610] mtd_nandbiterrs: verify_page [ 1830.699983] mtd_nandbiterrs: Successfully corrected 14 bit errors per subpage [ 1830.707113] mtd_nandbiterrs: Inserted biterror @ 3/2 [ 1830.712067] mtd_nandbiterrs: rewrite page [ 1830.716494] mtd_nandbiterrs: read_page [ 1830.720459] mtd_nandbiterrs: verify_page [ 1830.724841] mtd_nandbiterrs: Successfully corrected 15 bit errors per subpage [ 1830.731963] mtd_nandbiterrs: Inserted biterror @ 3/0 [ 1830.736920] mtd_nandbiterrs: rewrite page [ 1830.741161] mtd_nandbiterrs: read_page [ 1830.745107] mtd_nandbiterrs: verify_page [ 1830.749478] mtd_nandbiterrs: Successfully corrected 16 bit errors per subpage [ 1830.756607] mtd_nandbiterrs: Inserted biterror @ 4/2 [ 1830.761564] mtd_nandbiterrs: rewrite page [ 1830.765924] mtd_nandbiterrs: read_page [ 1830.769858] mtd_nandbiterrs: verify_page [ 1830.774232] mtd_nandbiterrs: Successfully corrected 17 bit errors per subpage [ 1830.781362] mtd_nandbiterrs: Inserted biterror @ 4/0 [ 1830.786318] mtd_nandbiterrs: rewrite page [ 1830.790558] mtd_nandbiterrs: read_page [ 1830.794496] mtd_nandbiterrs: verify_page [ 1830.798867] mtd_nandbiterrs: Successfully corrected 18 bit errors per subpage [ 1830.805997] mtd_nandbiterrs: Inserted biterror @ 5/7 [ 1830.810949] mtd_nandbiterrs: rewrite page [ 1830.815249] mtd_nandbiterrs: read_page [ 1830.819189] mtd_nandbiterrs: verify_page [ 1830.823561] mtd_nandbiterrs: Successfully corrected 19 bit errors per subpage [ 1830.830690] mtd_nandbiterrs: Inserted biterror @ 5/2 [ 1830.835646] mtd_nandbiterrs: rewrite page [ 1830.839943] mtd_nandbiterrs: read_page [ 1830.843886] mtd_nandbiterrs: verify_page [ 1830.848252] mtd_nandbiterrs: Successfully corrected 20 bit errors per subpage [ 1830.855378] mtd_nandbiterrs: Inserted biterror @ 5/0 [ 1830.860331] mtd_nandbiterrs: rewrite page [ 1830.864580] mtd_nandbiterrs: read_page [ 1830.868522] mtd_nandbiterrs: verify_page [ 1830.872890] mtd_nandbiterrs: Successfully corrected 21 bit errors per subpage [ 1830.880023] mtd_nandbiterr
RE: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
Hi Boris & Miquel, > -Original Message- > From: Miquel Raynal [mailto:miquel.ray...@bootlin.com] > Sent: Tuesday, November 20, 2018 6:06 PM > To: Boris Brezillon > Cc: Naga Sureshkumar Relli ; rich...@nod.at; > dw...@infradead.org; computersforpe...@gmail.com; marek.va...@gmail.com; > linux- > m...@lists.infradead.org; linux-kernel@vger.kernel.org; > nagasures...@gmail.com; > r...@kernel.org; Michal Simek > Subject: Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for > Arasan > NAND Flash Controller > > Hi Naga, > > Boris Brezillon wrote on Tue, 20 Nov 2018 > 12:02:44 +0100: > > > On Tue, 20 Nov 2018 07:02:08 + > > Naga Sureshkumar Relli wrote: > > > > > > > > > > > > Can you please run nandbiterrs (availaible in mtd-utils). I fear your > > > > device won't pass the test. > > > Yes, nandbiterror test is passing till 24bit, after that it is failing. > > > > Can you paste the output of nandbiterrs please? > > Apparently 'nandbiterrs -i 'just crashes the kernel because of a segmentation > fault. Please run > this test (from the mtd-utils package) and fix this issue. Then we would like > to see the output. Here is the output of mtd_nandbiterrs, [ 1830.546807] mtd_nandbiterrs: verify_page [ 1830.551924] mtd_nandbiterrs: Successfully corrected 8 bit errors per subpage [ 1830.558961] mtd_nandbiterrs: Inserted biterror @ 2/5 [ 1830.563917] mtd_nandbiterrs: rewrite page [ 1830.568216] mtd_nandbiterrs: read_page [ 1830.572155] mtd_nandbiterrs: verify_page [ 1830.576531] mtd_nandbiterrs: Successfully corrected 9 bit errors per subpage [ 1830.583568] mtd_nandbiterrs: Inserted biterror @ 2/2 [ 1830.588527] mtd_nandbiterrs: rewrite page [ 1830.592881] mtd_nandbiterrs: read_page [ 1830.596825] mtd_nandbiterrs: verify_page [ 1830.601197] mtd_nandbiterrs: Successfully corrected 10 bit errors per subpage [ 1830.608326] mtd_nandbiterrs: Inserted biterror @ 2/0 [ 1830.613279] mtd_nandbiterrs: rewrite page [ 1830.617585] mtd_nandbiterrs: read_page [ 1830.621524] mtd_nandbiterrs: verify_page [ 1830.625900] mtd_nandbiterrs: Successfully corrected 11 bit errors per subpage [ 1830.633027] mtd_nandbiterrs: Inserted biterror @ 3/7 [ 1830.637984] mtd_nandbiterrs: rewrite page [ 1830.642281] mtd_nandbiterrs: read_page [ 1830.646223] mtd_nandbiterrs: verify_page [ 1830.650595] mtd_nandbiterrs: Successfully corrected 12 bit errors per subpage [ 1830.657724] mtd_nandbiterrs: Inserted biterror @ 3/6 [ 1830.662677] mtd_nandbiterrs: rewrite page [ 1830.666983] mtd_nandbiterrs: read_page [ 1830.670922] mtd_nandbiterrs: verify_page [ 1830.675296] mtd_nandbiterrs: Successfully corrected 13 bit errors per subpage [ 1830.682417] mtd_nandbiterrs: Inserted biterror @ 3/5 [ 1830.687373] mtd_nandbiterrs: rewrite page [ 1830.691671] mtd_nandbiterrs: read_page [ 1830.695610] mtd_nandbiterrs: verify_page [ 1830.699983] mtd_nandbiterrs: Successfully corrected 14 bit errors per subpage [ 1830.707113] mtd_nandbiterrs: Inserted biterror @ 3/2 [ 1830.712067] mtd_nandbiterrs: rewrite page [ 1830.716494] mtd_nandbiterrs: read_page [ 1830.720459] mtd_nandbiterrs: verify_page [ 1830.724841] mtd_nandbiterrs: Successfully corrected 15 bit errors per subpage [ 1830.731963] mtd_nandbiterrs: Inserted biterror @ 3/0 [ 1830.736920] mtd_nandbiterrs: rewrite page [ 1830.741161] mtd_nandbiterrs: read_page [ 1830.745107] mtd_nandbiterrs: verify_page [ 1830.749478] mtd_nandbiterrs: Successfully corrected 16 bit errors per subpage [ 1830.756607] mtd_nandbiterrs: Inserted biterror @ 4/2 [ 1830.761564] mtd_nandbiterrs: rewrite page [ 1830.765924] mtd_nandbiterrs: read_page [ 1830.769858] mtd_nandbiterrs: verify_page [ 1830.774232] mtd_nandbiterrs: Successfully corrected 17 bit errors per subpage [ 1830.781362] mtd_nandbiterrs: Inserted biterror @ 4/0 [ 1830.786318] mtd_nandbiterrs: rewrite page [ 1830.790558] mtd_nandbiterrs: read_page [ 1830.794496] mtd_nandbiterrs: verify_page [ 1830.798867] mtd_nandbiterrs: Successfully corrected 18 bit errors per subpage [ 1830.805997] mtd_nandbiterrs: Inserted biterror @ 5/7 [ 1830.810949] mtd_nandbiterrs: rewrite page [ 1830.815249] mtd_nandbiterrs: read_page [ 1830.819189] mtd_nandbiterrs: verify_page [ 1830.823561] mtd_nandbiterrs: Successfully corrected 19 bit errors per subpage [ 1830.830690] mtd_nandbiterrs: Inserted biterror @ 5/2 [ 1830.835646] mtd_nandbiterrs: rewrite page [ 1830.839943] mtd_nandbiterrs: read_page [ 1830.843886] mtd_nandbiterrs: verify_page [ 1830.848252] mtd_nandbiterrs: Successfully corrected 20 bit errors per subpage [ 1830.855378] mtd_nandbiterrs: Inserted biterror @ 5/0 [ 1830.860331] mtd_nandbiterrs: rewrite page [ 1830.864580] mtd_nandbiterrs: read_page [ 1830.868522] mtd_nandbiterrs: verify_page [ 1830.872890] mtd_nandbiterrs: Successfully corrected 21 bit errors per subpage [ 1830.880023] mtd_nandbiterr
Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
On Fri, 9 Nov 2018 10:30:41 +0530
Naga Sureshkumar Relli wrote:
> +static int anfc_setup_data_interface(struct nand_chip *chip, int csline,
> + const struct nand_data_interface *conf)
> +{
> + struct anfc_nand_controller *nfc = to_anfc(chip->controller);
> + struct anfc_nand_chip *achip = to_anfc_nand(chip);
> + int err;
> + const struct nand_sdr_timings *sdr;
> + u32 inftimeval;
> + bool change_sdr_clk = false;
> +
> + if (csline == NAND_DATA_IFACE_CHECK_ONLY)
> + return 0;
> +
> + /*
> + * If the controller is already in the same mode as flash device
> + * then no need to change the timing mode again.
> + */
> + sdr = nand_get_sdr_timings(conf);
> + if (IS_ERR(sdr))
> + return PTR_ERR(sdr);
> +
> + if (sdr->mode < 0)
> + return -ENOTSUPP;
> +
> + inftimeval = sdr->mode & 7;
> + if (sdr->mode >= 2 && sdr->mode <= 5)
> + change_sdr_clk = true;
> + /*
> + * SDR timing modes 2-5 will not work for the arasan nand when
> + * freq > 90 MHz, so reduce the freq in SDR modes 2-5 to < 90Mhz
What's the freq for mode 0 and 1?
> + */
> + if (change_sdr_clk) {
> + clk_disable_unprepare(nfc->clk_sys);
> + err = clk_set_rate(nfc->clk_sys, SDR_MODE_DEFLT_FREQ);
You should not change the clk rate here. It should be done when the
chip is selected, so that, if you ever have 2 different chips connected
to the same controller, you can adapt the rate when they are accessed.
> + if (err) {
> + dev_err(nfc->dev, "Can't set the clock rate\n");
> + return err;
> + }
> + err = clk_prepare_enable(nfc->clk_sys);
> + if (err) {
> + dev_err(nfc->dev, "Unable to enable sys clock.\n");
> + clk_disable_unprepare(nfc->clk_sys);
> + return err;
> + }
> + }
> + achip->inftimeval = inftimeval;
> +
> + return 0;
> +}
> +
> +static int anfc_nand_attach_chip(struct nand_chip *chip)
> +{
> + struct mtd_info *mtd = nand_to_mtd(chip);
> + struct anfc_nand_chip *achip = to_anfc_nand(chip);
> + u32 ret;
> +
> + if (mtd->writesize <= SZ_512)
> + achip->spare_caddr_cycles = 1;
> + else
> + achip->spare_caddr_cycles = 2;
> +
> + chip->ecc.calc_buf = kmalloc(mtd->oobsize, GFP_KERNEL);
> + chip->ecc.code_buf = kmalloc(mtd->oobsize, GFP_KERNEL);
Those bufs are allocated but never freed (memleak). Also, are you sure
you really need them.
Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
On Fri, 9 Nov 2018 10:30:41 +0530
Naga Sureshkumar Relli wrote:
> +static int anfc_setup_data_interface(struct nand_chip *chip, int csline,
> + const struct nand_data_interface *conf)
> +{
> + struct anfc_nand_controller *nfc = to_anfc(chip->controller);
> + struct anfc_nand_chip *achip = to_anfc_nand(chip);
> + int err;
> + const struct nand_sdr_timings *sdr;
> + u32 inftimeval;
> + bool change_sdr_clk = false;
> +
> + if (csline == NAND_DATA_IFACE_CHECK_ONLY)
> + return 0;
> +
> + /*
> + * If the controller is already in the same mode as flash device
> + * then no need to change the timing mode again.
> + */
> + sdr = nand_get_sdr_timings(conf);
> + if (IS_ERR(sdr))
> + return PTR_ERR(sdr);
> +
> + if (sdr->mode < 0)
> + return -ENOTSUPP;
> +
> + inftimeval = sdr->mode & 7;
> + if (sdr->mode >= 2 && sdr->mode <= 5)
> + change_sdr_clk = true;
> + /*
> + * SDR timing modes 2-5 will not work for the arasan nand when
> + * freq > 90 MHz, so reduce the freq in SDR modes 2-5 to < 90Mhz
What's the freq for mode 0 and 1?
> + */
> + if (change_sdr_clk) {
> + clk_disable_unprepare(nfc->clk_sys);
> + err = clk_set_rate(nfc->clk_sys, SDR_MODE_DEFLT_FREQ);
You should not change the clk rate here. It should be done when the
chip is selected, so that, if you ever have 2 different chips connected
to the same controller, you can adapt the rate when they are accessed.
> + if (err) {
> + dev_err(nfc->dev, "Can't set the clock rate\n");
> + return err;
> + }
> + err = clk_prepare_enable(nfc->clk_sys);
> + if (err) {
> + dev_err(nfc->dev, "Unable to enable sys clock.\n");
> + clk_disable_unprepare(nfc->clk_sys);
> + return err;
> + }
> + }
> + achip->inftimeval = inftimeval;
> +
> + return 0;
> +}
> +
> +static int anfc_nand_attach_chip(struct nand_chip *chip)
> +{
> + struct mtd_info *mtd = nand_to_mtd(chip);
> + struct anfc_nand_chip *achip = to_anfc_nand(chip);
> + u32 ret;
> +
> + if (mtd->writesize <= SZ_512)
> + achip->spare_caddr_cycles = 1;
> + else
> + achip->spare_caddr_cycles = 2;
> +
> + chip->ecc.calc_buf = kmalloc(mtd->oobsize, GFP_KERNEL);
> + chip->ecc.code_buf = kmalloc(mtd->oobsize, GFP_KERNEL);
Those bufs are allocated but never freed (memleak). Also, are you sure
you really need them.
Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
Hi Naga, Boris Brezillon wrote on Tue, 20 Nov 2018 12:02:44 +0100: > On Tue, 20 Nov 2018 07:02:08 + > Naga Sureshkumar Relli wrote: > > > > > > > > Can you please run nandbiterrs (availaible in mtd-utils). I fear your > > > device won't pass the test. > > Yes, nandbiterror test is passing till 24bit, after that it is failing. > > Can you paste the output of nandbiterrs please? Apparently 'nandbiterrs -i 'just crashes the kernel because of a segmentation fault. Please run this test (from the mtd-utils package) and fix this issue. Then we would like to see the output. > > > > > > > > But we are hitting this because of erased page reading(needed in case > > > > of ubifs). > > > > > > > > > > > > > > Don't you have a bit (or several bits) reporting when the ECC engine > > > > > was not able to > > > correct > > > > > data? I you do, you should base the "detect bitflips in erase pages" > > > > > logic on this information. > > > > Bit reporting for several bit errors is there only for Hamming(1bit > > > > correction and 2bit > > > detection) but not in BCH. > > > > > > > > > > Then I tend to agree with Miquel: your ECC engine is broken, and I'm > > > not even sure how to deal with that yet. > > So as per the Miquel's suggestion, can I proceed to add the below one? > > "you should re-read the page in raw mode and check for the number of > > bitflips manually (thanks to the helpers in the core). Again, if the number > > of BF is above 16, we can assume the page is bad and increment ->ecc.failed > > accordingly." > > But that's just partially fixing the problem. And you didn't answer my > previous question: what happens when you configure the ECC engine in, > say 12bit/1024 and you end up with uncorrectable errors (more than 12 > bitflips in a 1k block). What's the number reported ECC_ERR_CNT? Is it > set to 13? Please dump this register, and eventually what's the value of the Packet_bound_Err_count field ([0:7]) for each iteration of nandbiterrs -i. If there is no way, when the status bit is set, to discriminate if the data is reliable or was not corrected at all, it is gonna be a real issue and I don't think we want to support such engine. Thanks, Miquèl
Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
Hi Naga, Boris Brezillon wrote on Tue, 20 Nov 2018 12:02:44 +0100: > On Tue, 20 Nov 2018 07:02:08 + > Naga Sureshkumar Relli wrote: > > > > > > > > Can you please run nandbiterrs (availaible in mtd-utils). I fear your > > > device won't pass the test. > > Yes, nandbiterror test is passing till 24bit, after that it is failing. > > Can you paste the output of nandbiterrs please? Apparently 'nandbiterrs -i 'just crashes the kernel because of a segmentation fault. Please run this test (from the mtd-utils package) and fix this issue. Then we would like to see the output. > > > > > > > > But we are hitting this because of erased page reading(needed in case > > > > of ubifs). > > > > > > > > > > > > > > Don't you have a bit (or several bits) reporting when the ECC engine > > > > > was not able to > > > correct > > > > > data? I you do, you should base the "detect bitflips in erase pages" > > > > > logic on this information. > > > > Bit reporting for several bit errors is there only for Hamming(1bit > > > > correction and 2bit > > > detection) but not in BCH. > > > > > > > > > > Then I tend to agree with Miquel: your ECC engine is broken, and I'm > > > not even sure how to deal with that yet. > > So as per the Miquel's suggestion, can I proceed to add the below one? > > "you should re-read the page in raw mode and check for the number of > > bitflips manually (thanks to the helpers in the core). Again, if the number > > of BF is above 16, we can assume the page is bad and increment ->ecc.failed > > accordingly." > > But that's just partially fixing the problem. And you didn't answer my > previous question: what happens when you configure the ECC engine in, > say 12bit/1024 and you end up with uncorrectable errors (more than 12 > bitflips in a 1k block). What's the number reported ECC_ERR_CNT? Is it > set to 13? Please dump this register, and eventually what's the value of the Packet_bound_Err_count field ([0:7]) for each iteration of nandbiterrs -i. If there is no way, when the status bit is set, to discriminate if the data is reliable or was not corrected at all, it is gonna be a real issue and I don't think we want to support such engine. Thanks, Miquèl
Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
On Tue, 20 Nov 2018 07:02:08 + Naga Sureshkumar Relli wrote: > > > > Can you please run nandbiterrs (availaible in mtd-utils). I fear your > > device won't pass the test. > Yes, nandbiterror test is passing till 24bit, after that it is failing. Can you paste the output of nandbiterrs please? > > > > > But we are hitting this because of erased page reading(needed in case of > > > ubifs). > > > > > > > > > > > Don't you have a bit (or several bits) reporting when the ECC engine > > > > was not able to > > correct > > > > data? I you do, you should base the "detect bitflips in erase pages" > > > > logic on this information. > > > Bit reporting for several bit errors is there only for Hamming(1bit > > > correction and 2bit > > detection) but not in BCH. > > > > > > > Then I tend to agree with Miquel: your ECC engine is broken, and I'm > > not even sure how to deal with that yet. > So as per the Miquel's suggestion, can I proceed to add the below one? > "you should re-read the page in raw mode and check for the number of bitflips > manually (thanks to the helpers in the core). Again, if the number of BF is > above 16, we can assume the page is bad and increment ->ecc.failed > accordingly." But that's just partially fixing the problem. And you didn't answer my previous question: what happens when you configure the ECC engine in, say 12bit/1024 and you end up with uncorrectable errors (more than 12 bitflips in a 1k block). What's the number reported ECC_ERR_CNT? Is it set to 13?
Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
On Tue, 20 Nov 2018 07:02:08 + Naga Sureshkumar Relli wrote: > > > > Can you please run nandbiterrs (availaible in mtd-utils). I fear your > > device won't pass the test. > Yes, nandbiterror test is passing till 24bit, after that it is failing. Can you paste the output of nandbiterrs please? > > > > > But we are hitting this because of erased page reading(needed in case of > > > ubifs). > > > > > > > > > > > Don't you have a bit (or several bits) reporting when the ECC engine > > > > was not able to > > correct > > > > data? I you do, you should base the "detect bitflips in erase pages" > > > > logic on this information. > > > Bit reporting for several bit errors is there only for Hamming(1bit > > > correction and 2bit > > detection) but not in BCH. > > > > > > > Then I tend to agree with Miquel: your ECC engine is broken, and I'm > > not even sure how to deal with that yet. > So as per the Miquel's suggestion, can I proceed to add the below one? > "you should re-read the page in raw mode and check for the number of bitflips > manually (thanks to the helpers in the core). Again, if the number of BF is > above 16, we can assume the page is bad and increment ->ecc.failed > accordingly." But that's just partially fixing the problem. And you didn't answer my previous question: what happens when you configure the ECC engine in, say 12bit/1024 and you end up with uncorrectable errors (more than 12 bitflips in a 1k block). What's the number reported ECC_ERR_CNT? Is it set to 13?
RE: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
Hi Boris, > -Original Message- > From: Boris Brezillon [mailto:boris.brezil...@bootlin.com] > Sent: Monday, November 19, 2018 1:33 PM > To: Naga Sureshkumar Relli > Cc: miquel.ray...@bootlin.com; rich...@nod.at; dw...@infradead.org; > computersforpe...@gmail.com; marek.va...@gmail.com; > linux-...@lists.infradead.org; linux- > ker...@vger.kernel.org; nagasures...@gmail.com; r...@kernel.org; Michal Simek > > Subject: Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for > Arasan NAND > Flash Controller > > On Mon, 19 Nov 2018 06:20:28 + > Naga Sureshkumar Relli wrote: > > > H Boris, > > > > > -Original Message- > > > From: Boris Brezillon [mailto:boris.brezil...@bootlin.com] > > > Sent: Monday, November 19, 2018 1:13 AM > > > To: Naga Sureshkumar Relli > > > Cc: miquel.ray...@bootlin.com; rich...@nod.at; dw...@infradead.org; > > > computersforpe...@gmail.com; marek.va...@gmail.com; > > > linux-...@lists.infradead.org; linux- ker...@vger.kernel.org; > > > nagasures...@gmail.com; r...@kernel.org; Michal Simek > > > > > > Subject: Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support > > > for Arasan NAND Flash Controller > > > > > > On Thu, 15 Nov 2018 09:34:16 + > > > Naga Sureshkumar Relli wrote: > > > > > > > Hi Boris & Miquel, > > > > > > > > I am updating the driver by addressing your comments, and I have > > > > one concern, especially in anfc_read_page_hwecc(), there I am checking > > > > for erased pages > bit flips. > > > > Since Arasan NAND controller doesn't have multibit error detection > > > > beyond 24-bit( it can correct up to 24 bit), i.e. there is no > > > > indication from controller to detect > > > uncorrectable error beyond 24bit. > > > > > > Do you mean that you can't detect uncorrectable errors, or just that > > > it's not 100% sure to detect errors above max_strength? > > Yes, in Arasan NAND controller there is no way to detect uncorrectable > > errors beyond 24- > bit. > > So how do you detect uncorrectable errors when the strength is less than > 24bits? Below or equal to the level of ECC strength, controller will definitely correct. But beyond the level of ECC strength, it won't even detect the errors. > > > > > > > > So I took some error count as default value(MULTI_BIT_ERR_CNT 16, I > > > > put this based on the error count that I got while reading erased page > > > > on Micron device). > > > > And during a page read, will just read the error count register and > > > > compare this value with the default error count(16) and if it is more > > > > Than default then I > am > > > checking for erased page bit flips. > > > > > > Hm, that's wrong, especially if you set ecc_strength to something > 16. > > Ok > > > > > > > I am doubting that this will not work in all cases. > > > > > > It definitely doesn't. > > Ok > > > > > > > In my case it is just working because the error count that it got on an > > > > erased page is 16. > > > > Could you please suggest a way to do detect erased_page bit flips when > > > > reading a page > with > > > HW-ECC?. > > > > > > I'm a bit lost. Is the problem only about bitflips in erase pages, or is > > > it also impacting reads > of > > > written pages that lead to uncorrectable errors. > > Yes, it is for both. But in case of read errors that we can't detect beyond > > 24-bit, then the > answer from HW design team > > Is that the flash part is bad. > > Unfortunately till now we haven't ran into that situation(read errors of > > written pages beyond > 24-bit). > > Can you please run nandbiterrs (availaible in mtd-utils). I fear your > device won't pass the test. Yes, nandbiterror test is passing till 24bit, after that it is failing. > > > But we are hitting this because of erased page reading(needed in case of > > ubifs). > > > > > > > > Don't you have a bit (or several bits) reporting when the ECC engine was > > > not able to > correct > > > data? I you do, you should base the "detect bitflips in erase pages" > > > logic on this information. > > Bit reporting for several bit errors is there only for Hamming(1bit > > correction and 2bit > detection) but not in BCH. > > > > Then I tend to agree with Miquel: your ECC engine is broken, and I'm > not even sure how to deal with that yet. So as per the Miquel's suggestion, can I proceed to add the below one? "you should re-read the page in raw mode and check for the number of bitflips manually (thanks to the helpers in the core). Again, if the number of BF is above 16, we can assume the page is bad and increment ->ecc.failed accordingly." Thanks, Naga Sureshkumar Relli
RE: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
Hi Boris, > -Original Message- > From: Boris Brezillon [mailto:boris.brezil...@bootlin.com] > Sent: Monday, November 19, 2018 1:33 PM > To: Naga Sureshkumar Relli > Cc: miquel.ray...@bootlin.com; rich...@nod.at; dw...@infradead.org; > computersforpe...@gmail.com; marek.va...@gmail.com; > linux-...@lists.infradead.org; linux- > ker...@vger.kernel.org; nagasures...@gmail.com; r...@kernel.org; Michal Simek > > Subject: Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for > Arasan NAND > Flash Controller > > On Mon, 19 Nov 2018 06:20:28 + > Naga Sureshkumar Relli wrote: > > > H Boris, > > > > > -Original Message- > > > From: Boris Brezillon [mailto:boris.brezil...@bootlin.com] > > > Sent: Monday, November 19, 2018 1:13 AM > > > To: Naga Sureshkumar Relli > > > Cc: miquel.ray...@bootlin.com; rich...@nod.at; dw...@infradead.org; > > > computersforpe...@gmail.com; marek.va...@gmail.com; > > > linux-...@lists.infradead.org; linux- ker...@vger.kernel.org; > > > nagasures...@gmail.com; r...@kernel.org; Michal Simek > > > > > > Subject: Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support > > > for Arasan NAND Flash Controller > > > > > > On Thu, 15 Nov 2018 09:34:16 + > > > Naga Sureshkumar Relli wrote: > > > > > > > Hi Boris & Miquel, > > > > > > > > I am updating the driver by addressing your comments, and I have > > > > one concern, especially in anfc_read_page_hwecc(), there I am checking > > > > for erased pages > bit flips. > > > > Since Arasan NAND controller doesn't have multibit error detection > > > > beyond 24-bit( it can correct up to 24 bit), i.e. there is no > > > > indication from controller to detect > > > uncorrectable error beyond 24bit. > > > > > > Do you mean that you can't detect uncorrectable errors, or just that > > > it's not 100% sure to detect errors above max_strength? > > Yes, in Arasan NAND controller there is no way to detect uncorrectable > > errors beyond 24- > bit. > > So how do you detect uncorrectable errors when the strength is less than > 24bits? Below or equal to the level of ECC strength, controller will definitely correct. But beyond the level of ECC strength, it won't even detect the errors. > > > > > > > > So I took some error count as default value(MULTI_BIT_ERR_CNT 16, I > > > > put this based on the error count that I got while reading erased page > > > > on Micron device). > > > > And during a page read, will just read the error count register and > > > > compare this value with the default error count(16) and if it is more > > > > Than default then I > am > > > checking for erased page bit flips. > > > > > > Hm, that's wrong, especially if you set ecc_strength to something > 16. > > Ok > > > > > > > I am doubting that this will not work in all cases. > > > > > > It definitely doesn't. > > Ok > > > > > > > In my case it is just working because the error count that it got on an > > > > erased page is 16. > > > > Could you please suggest a way to do detect erased_page bit flips when > > > > reading a page > with > > > HW-ECC?. > > > > > > I'm a bit lost. Is the problem only about bitflips in erase pages, or is > > > it also impacting reads > of > > > written pages that lead to uncorrectable errors. > > Yes, it is for both. But in case of read errors that we can't detect beyond > > 24-bit, then the > answer from HW design team > > Is that the flash part is bad. > > Unfortunately till now we haven't ran into that situation(read errors of > > written pages beyond > 24-bit). > > Can you please run nandbiterrs (availaible in mtd-utils). I fear your > device won't pass the test. Yes, nandbiterror test is passing till 24bit, after that it is failing. > > > But we are hitting this because of erased page reading(needed in case of > > ubifs). > > > > > > > > Don't you have a bit (or several bits) reporting when the ECC engine was > > > not able to > correct > > > data? I you do, you should base the "detect bitflips in erase pages" > > > logic on this information. > > Bit reporting for several bit errors is there only for Hamming(1bit > > correction and 2bit > detection) but not in BCH. > > > > Then I tend to agree with Miquel: your ECC engine is broken, and I'm > not even sure how to deal with that yet. So as per the Miquel's suggestion, can I proceed to add the below one? "you should re-read the page in raw mode and check for the number of bitflips manually (thanks to the helpers in the core). Again, if the number of BF is above 16, we can assume the page is bad and increment ->ecc.failed accordingly." Thanks, Naga Sureshkumar Relli
Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
On Mon, 19 Nov 2018 06:20:28 + Naga Sureshkumar Relli wrote: > H Boris, > > > -Original Message- > > From: Boris Brezillon [mailto:boris.brezil...@bootlin.com] > > Sent: Monday, November 19, 2018 1:13 AM > > To: Naga Sureshkumar Relli > > Cc: miquel.ray...@bootlin.com; rich...@nod.at; dw...@infradead.org; > > computersforpe...@gmail.com; marek.va...@gmail.com; > > linux-...@lists.infradead.org; linux- > > ker...@vger.kernel.org; nagasures...@gmail.com; r...@kernel.org; Michal > > Simek > > > > Subject: Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for > > Arasan NAND > > Flash Controller > > > > On Thu, 15 Nov 2018 09:34:16 + > > Naga Sureshkumar Relli wrote: > > > > > Hi Boris & Miquel, > > > > > > I am updating the driver by addressing your comments, and I have one > > > concern, especially in anfc_read_page_hwecc(), there I am checking for > > > erased pages bit flips. > > > Since Arasan NAND controller doesn't have multibit error detection > > > beyond 24-bit( it can correct up to 24 bit), i.e. there is no indication > > > from controller to detect > > uncorrectable error beyond 24bit. > > > > Do you mean that you can't detect uncorrectable errors, or just that it's > > not 100% sure to detect > > errors above max_strength? > Yes, in Arasan NAND controller there is no way to detect uncorrectable errors > beyond 24-bit. So how do you detect uncorrectable errors when the strength is less than 24bits? > > > > > So I took some error count as default value(MULTI_BIT_ERR_CNT 16, I > > > put this based on the error count that I got while reading erased page on > > > Micron device). > > > And during a page read, will just read the error count register and > > > compare this value with the default error count(16) and if it is more > > > Than default then I am > > checking for erased page bit flips. > > > > Hm, that's wrong, especially if you set ecc_strength to something > 16. > Ok > > > > > I am doubting that this will not work in all cases. > > > > It definitely doesn't. > Ok > > > > > In my case it is just working because the error count that it got on an > > > erased page is 16. > > > Could you please suggest a way to do detect erased_page bit flips when > > > reading a page with > > HW-ECC?. > > > > I'm a bit lost. Is the problem only about bitflips in erase pages, or is it > > also impacting reads of > > written pages that lead to uncorrectable errors. > Yes, it is for both. But in case of read errors that we can't detect beyond > 24-bit, then the answer from HW design team > Is that the flash part is bad. > Unfortunately till now we haven't ran into that situation(read errors of > written pages beyond 24-bit). Can you please run nandbiterrs (availaible in mtd-utils). I fear your device won't pass the test. > But we are hitting this because of erased page reading(needed in case of > ubifs). > > > > > Don't you have a bit (or several bits) reporting when the ECC engine was > > not able to correct > > data? I you do, you should base the "detect bitflips in erase pages" logic > > on this information. > Bit reporting for several bit errors is there only for Hamming(1bit > correction and 2bit detection) but not in BCH. > Then I tend to agree with Miquel: your ECC engine is broken, and I'm not even sure how to deal with that yet.
Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
On Mon, 19 Nov 2018 06:20:28 + Naga Sureshkumar Relli wrote: > H Boris, > > > -Original Message- > > From: Boris Brezillon [mailto:boris.brezil...@bootlin.com] > > Sent: Monday, November 19, 2018 1:13 AM > > To: Naga Sureshkumar Relli > > Cc: miquel.ray...@bootlin.com; rich...@nod.at; dw...@infradead.org; > > computersforpe...@gmail.com; marek.va...@gmail.com; > > linux-...@lists.infradead.org; linux- > > ker...@vger.kernel.org; nagasures...@gmail.com; r...@kernel.org; Michal > > Simek > > > > Subject: Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for > > Arasan NAND > > Flash Controller > > > > On Thu, 15 Nov 2018 09:34:16 + > > Naga Sureshkumar Relli wrote: > > > > > Hi Boris & Miquel, > > > > > > I am updating the driver by addressing your comments, and I have one > > > concern, especially in anfc_read_page_hwecc(), there I am checking for > > > erased pages bit flips. > > > Since Arasan NAND controller doesn't have multibit error detection > > > beyond 24-bit( it can correct up to 24 bit), i.e. there is no indication > > > from controller to detect > > uncorrectable error beyond 24bit. > > > > Do you mean that you can't detect uncorrectable errors, or just that it's > > not 100% sure to detect > > errors above max_strength? > Yes, in Arasan NAND controller there is no way to detect uncorrectable errors > beyond 24-bit. So how do you detect uncorrectable errors when the strength is less than 24bits? > > > > > So I took some error count as default value(MULTI_BIT_ERR_CNT 16, I > > > put this based on the error count that I got while reading erased page on > > > Micron device). > > > And during a page read, will just read the error count register and > > > compare this value with the default error count(16) and if it is more > > > Than default then I am > > checking for erased page bit flips. > > > > Hm, that's wrong, especially if you set ecc_strength to something > 16. > Ok > > > > > I am doubting that this will not work in all cases. > > > > It definitely doesn't. > Ok > > > > > In my case it is just working because the error count that it got on an > > > erased page is 16. > > > Could you please suggest a way to do detect erased_page bit flips when > > > reading a page with > > HW-ECC?. > > > > I'm a bit lost. Is the problem only about bitflips in erase pages, or is it > > also impacting reads of > > written pages that lead to uncorrectable errors. > Yes, it is for both. But in case of read errors that we can't detect beyond > 24-bit, then the answer from HW design team > Is that the flash part is bad. > Unfortunately till now we haven't ran into that situation(read errors of > written pages beyond 24-bit). Can you please run nandbiterrs (availaible in mtd-utils). I fear your device won't pass the test. > But we are hitting this because of erased page reading(needed in case of > ubifs). > > > > > Don't you have a bit (or several bits) reporting when the ECC engine was > > not able to correct > > data? I you do, you should base the "detect bitflips in erase pages" logic > > on this information. > Bit reporting for several bit errors is there only for Hamming(1bit > correction and 2bit detection) but not in BCH. > Then I tend to agree with Miquel: your ECC engine is broken, and I'm not even sure how to deal with that yet.
RE: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
H Boris, > -Original Message- > From: Boris Brezillon [mailto:boris.brezil...@bootlin.com] > Sent: Monday, November 19, 2018 1:13 AM > To: Naga Sureshkumar Relli > Cc: miquel.ray...@bootlin.com; rich...@nod.at; dw...@infradead.org; > computersforpe...@gmail.com; marek.va...@gmail.com; > linux-...@lists.infradead.org; linux- > ker...@vger.kernel.org; nagasures...@gmail.com; r...@kernel.org; Michal Simek > > Subject: Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for > Arasan NAND > Flash Controller > > On Thu, 15 Nov 2018 09:34:16 + > Naga Sureshkumar Relli wrote: > > > Hi Boris & Miquel, > > > > I am updating the driver by addressing your comments, and I have one > > concern, especially in anfc_read_page_hwecc(), there I am checking for > > erased pages bit flips. > > Since Arasan NAND controller doesn't have multibit error detection > > beyond 24-bit( it can correct up to 24 bit), i.e. there is no indication > > from controller to detect > uncorrectable error beyond 24bit. > > Do you mean that you can't detect uncorrectable errors, or just that it's not > 100% sure to detect > errors above max_strength? Yes, in Arasan NAND controller there is no way to detect uncorrectable errors beyond 24-bit. > > > So I took some error count as default value(MULTI_BIT_ERR_CNT 16, I > > put this based on the error count that I got while reading erased page on > > Micron device). > > And during a page read, will just read the error count register and > > compare this value with the default error count(16) and if it is more Than > > default then I am > checking for erased page bit flips. > > Hm, that's wrong, especially if you set ecc_strength to something > 16. Ok > > > I am doubting that this will not work in all cases. > > It definitely doesn't. Ok > > > In my case it is just working because the error count that it got on an > > erased page is 16. > > Could you please suggest a way to do detect erased_page bit flips when > > reading a page with > HW-ECC?. > > I'm a bit lost. Is the problem only about bitflips in erase pages, or is it > also impacting reads of > written pages that lead to uncorrectable errors. Yes, it is for both. But in case of read errors that we can't detect beyond 24-bit, then the answer from HW design team Is that the flash part is bad. Unfortunately till now we haven't ran into that situation(read errors of written pages beyond 24-bit). But we are hitting this because of erased page reading(needed in case of ubifs). > > Don't you have a bit (or several bits) reporting when the ECC engine was not > able to correct > data? I you do, you should base the "detect bitflips in erase pages" logic on > this information. Bit reporting for several bit errors is there only for Hamming(1bit correction and 2bit detection) but not in BCH. Thanks, Naga Sureshkumar Relli
RE: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
H Boris, > -Original Message- > From: Boris Brezillon [mailto:boris.brezil...@bootlin.com] > Sent: Monday, November 19, 2018 1:13 AM > To: Naga Sureshkumar Relli > Cc: miquel.ray...@bootlin.com; rich...@nod.at; dw...@infradead.org; > computersforpe...@gmail.com; marek.va...@gmail.com; > linux-...@lists.infradead.org; linux- > ker...@vger.kernel.org; nagasures...@gmail.com; r...@kernel.org; Michal Simek > > Subject: Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for > Arasan NAND > Flash Controller > > On Thu, 15 Nov 2018 09:34:16 + > Naga Sureshkumar Relli wrote: > > > Hi Boris & Miquel, > > > > I am updating the driver by addressing your comments, and I have one > > concern, especially in anfc_read_page_hwecc(), there I am checking for > > erased pages bit flips. > > Since Arasan NAND controller doesn't have multibit error detection > > beyond 24-bit( it can correct up to 24 bit), i.e. there is no indication > > from controller to detect > uncorrectable error beyond 24bit. > > Do you mean that you can't detect uncorrectable errors, or just that it's not > 100% sure to detect > errors above max_strength? Yes, in Arasan NAND controller there is no way to detect uncorrectable errors beyond 24-bit. > > > So I took some error count as default value(MULTI_BIT_ERR_CNT 16, I > > put this based on the error count that I got while reading erased page on > > Micron device). > > And during a page read, will just read the error count register and > > compare this value with the default error count(16) and if it is more Than > > default then I am > checking for erased page bit flips. > > Hm, that's wrong, especially if you set ecc_strength to something > 16. Ok > > > I am doubting that this will not work in all cases. > > It definitely doesn't. Ok > > > In my case it is just working because the error count that it got on an > > erased page is 16. > > Could you please suggest a way to do detect erased_page bit flips when > > reading a page with > HW-ECC?. > > I'm a bit lost. Is the problem only about bitflips in erase pages, or is it > also impacting reads of > written pages that lead to uncorrectable errors. Yes, it is for both. But in case of read errors that we can't detect beyond 24-bit, then the answer from HW design team Is that the flash part is bad. Unfortunately till now we haven't ran into that situation(read errors of written pages beyond 24-bit). But we are hitting this because of erased page reading(needed in case of ubifs). > > Don't you have a bit (or several bits) reporting when the ECC engine was not > able to correct > data? I you do, you should base the "detect bitflips in erase pages" logic on > this information. Bit reporting for several bit errors is there only for Hamming(1bit correction and 2bit detection) but not in BCH. Thanks, Naga Sureshkumar Relli
Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
On Thu, 15 Nov 2018 09:34:16 + Naga Sureshkumar Relli wrote: > Hi Boris & Miquel, > > I am updating the driver by addressing your comments, and I have one concern, > especially in anfc_read_page_hwecc(), > there I am checking for erased pages bit flips. > Since Arasan NAND controller doesn't have multibit error detection beyond > 24-bit( it can correct up to 24 bit), > i.e. there is no indication from controller to detect uncorrectable error > beyond 24bit. Do you mean that you can't detect uncorrectable errors, or just that it's not 100% sure to detect errors above max_strength? > So I took some error count as default value(MULTI_BIT_ERR_CNT 16, I put this > based on the error count that > I got while reading erased page on Micron device). > And during a page read, will just read the error count register and compare > this value with the default error count(16) and if it is more > Than default then I am checking for erased page bit flips. Hm, that's wrong, especially if you set ecc_strength to something > 16. > I am doubting that this will not work in all cases. It definitely doesn't. > In my case it is just working because the error count that it got on an > erased page is 16. > Could you please suggest a way to do detect erased_page bit flips when > reading a page with HW-ECC?. I'm a bit lost. Is the problem only about bitflips in erase pages, or is it also impacting reads of written pages that lead to uncorrectable errors. Don't you have a bit (or several bits) reporting when the ECC engine was not able to correct data? I you do, you should base the "detect bitflips in erase pages" logic on this information.
Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
On Thu, 15 Nov 2018 09:34:16 + Naga Sureshkumar Relli wrote: > Hi Boris & Miquel, > > I am updating the driver by addressing your comments, and I have one concern, > especially in anfc_read_page_hwecc(), > there I am checking for erased pages bit flips. > Since Arasan NAND controller doesn't have multibit error detection beyond > 24-bit( it can correct up to 24 bit), > i.e. there is no indication from controller to detect uncorrectable error > beyond 24bit. Do you mean that you can't detect uncorrectable errors, or just that it's not 100% sure to detect errors above max_strength? > So I took some error count as default value(MULTI_BIT_ERR_CNT 16, I put this > based on the error count that > I got while reading erased page on Micron device). > And during a page read, will just read the error count register and compare > this value with the default error count(16) and if it is more > Than default then I am checking for erased page bit flips. Hm, that's wrong, especially if you set ecc_strength to something > 16. > I am doubting that this will not work in all cases. It definitely doesn't. > In my case it is just working because the error count that it got on an > erased page is 16. > Could you please suggest a way to do detect erased_page bit flips when > reading a page with HW-ECC?. I'm a bit lost. Is the problem only about bitflips in erase pages, or is it also impacting reads of written pages that lead to uncorrectable errors. Don't you have a bit (or several bits) reporting when the ECC engine was not able to correct data? I you do, you should base the "detect bitflips in erase pages" logic on this information.
Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
Hi Naga, Naga Sureshkumar Relli wrote on Thu, 15 Nov 2018 09:34:16 +: > Hi Boris & Miquel, > > I am updating the driver by addressing your comments, and I have one concern, > especially in anfc_read_page_hwecc(), > there I am checking for erased pages bit flips. > Since Arasan NAND controller doesn't have multibit error detection beyond > 24-bit( it can correct up to 24 bit), > i.e. there is no indication from controller to detect uncorrectable error > beyond 24bit. > So I took some error count as default value(MULTI_BIT_ERR_CNT 16, I put this > based on the error count that > I got while reading erased page on Micron device). > And during a page read, will just read the error count register and compare > this value with the default error count(16) and if it is more > Than default then I am checking for erased page bit flips. > I am doubting that this will not work in all cases. > In my case it is just working because the error count that it got on an > erased page is 16. > Could you please suggest a way to do detect erased_page bit flips when > reading a page with HW-ECC?. So the ECC engine is broken by design. I think you should determine a number of bitflips (16 looks nice to me) over which you declare the page bad anyway. Now, this is generic logic: anytime a page is declared bad, you should re-read the page in raw mode and check for the number of bitflips manually (thanks to the helpers in the core). Again, if the number of BF is above 16, we can assume the page is bad and increment ->ecc.failed accordingly. Thanks, Miquèl
Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
Hi Naga, Naga Sureshkumar Relli wrote on Thu, 15 Nov 2018 09:34:16 +: > Hi Boris & Miquel, > > I am updating the driver by addressing your comments, and I have one concern, > especially in anfc_read_page_hwecc(), > there I am checking for erased pages bit flips. > Since Arasan NAND controller doesn't have multibit error detection beyond > 24-bit( it can correct up to 24 bit), > i.e. there is no indication from controller to detect uncorrectable error > beyond 24bit. > So I took some error count as default value(MULTI_BIT_ERR_CNT 16, I put this > based on the error count that > I got while reading erased page on Micron device). > And during a page read, will just read the error count register and compare > this value with the default error count(16) and if it is more > Than default then I am checking for erased page bit flips. > I am doubting that this will not work in all cases. > In my case it is just working because the error count that it got on an > erased page is 16. > Could you please suggest a way to do detect erased_page bit flips when > reading a page with HW-ECC?. So the ECC engine is broken by design. I think you should determine a number of bitflips (16 looks nice to me) over which you declare the page bad anyway. Now, this is generic logic: anytime a page is declared bad, you should re-read the page in raw mode and check for the number of bitflips manually (thanks to the helpers in the core). Again, if the number of BF is above 16, we can assume the page is bad and increment ->ecc.failed accordingly. Thanks, Miquèl
Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
Hi Naga,
Thank you for the patch! Perhaps something to improve:
url:
https://github.com/0day-ci/linux/commits/Naga-Sureshkumar-Relli/dt-bindings-mtd-arasan-Add-device-tree-binding-documentation/20181110-034106
base: git://git.infradead.org/linux-mtd.git nand/next
smatch warnings:
drivers/mtd/nand/raw/arasan_nand.c:353 anfc_rw_dma_op() warn: right shifting
more than type allows 32 vs 32
drivers/mtd/nand/raw/arasan_nand.c:1032 anfc_setup_data_interface() warn:
unsigned 'sdr->mode' is never less than zero.
#
https://github.com/0day-ci/linux/commit/8db68ae6047a392d3e4092cbd6d3051eec1edd47
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout 8db68ae6047a392d3e4092cbd6d3051eec1edd47
vim +353 drivers/mtd/nand/raw/arasan_nand.c
8db68ae6 Naga Sureshkumar Relli 2018-11-09 325
8db68ae6 Naga Sureshkumar Relli 2018-11-09 326 static void
anfc_rw_dma_op(struct mtd_info *mtd, u8 *buf, int len,
8db68ae6 Naga Sureshkumar Relli 2018-11-09 327bool
do_read, u32 prog, int pktcount, int pktsize)
8db68ae6 Naga Sureshkumar Relli 2018-11-09 328 {
8db68ae6 Naga Sureshkumar Relli 2018-11-09 329 dma_addr_t paddr;
8db68ae6 Naga Sureshkumar Relli 2018-11-09 330 struct nand_chip *chip
= mtd_to_nand(mtd);
8db68ae6 Naga Sureshkumar Relli 2018-11-09 331 struct
anfc_nand_controller *nfc = to_anfc(chip->controller);
8db68ae6 Naga Sureshkumar Relli 2018-11-09 332 struct anfc_nand_chip
*achip = to_anfc_nand(chip);
8db68ae6 Naga Sureshkumar Relli 2018-11-09 333 u32 eccintr = 0, dir;
8db68ae6 Naga Sureshkumar Relli 2018-11-09 334
8db68ae6 Naga Sureshkumar Relli 2018-11-09 335 if (pktsize == 0)
8db68ae6 Naga Sureshkumar Relli 2018-11-09 336 pktsize = len;
8db68ae6 Naga Sureshkumar Relli 2018-11-09 337
8db68ae6 Naga Sureshkumar Relli 2018-11-09 338 anfc_setpktszcnt(nfc,
pktsize, pktcount);
8db68ae6 Naga Sureshkumar Relli 2018-11-09 339
8db68ae6 Naga Sureshkumar Relli 2018-11-09 340 if (!achip->strength)
8db68ae6 Naga Sureshkumar Relli 2018-11-09 341 eccintr =
MBIT_ERROR;
8db68ae6 Naga Sureshkumar Relli 2018-11-09 342
8db68ae6 Naga Sureshkumar Relli 2018-11-09 343 if (do_read)
8db68ae6 Naga Sureshkumar Relli 2018-11-09 344 dir =
DMA_FROM_DEVICE;
8db68ae6 Naga Sureshkumar Relli 2018-11-09 345 else
8db68ae6 Naga Sureshkumar Relli 2018-11-09 346 dir =
DMA_TO_DEVICE;
8db68ae6 Naga Sureshkumar Relli 2018-11-09 347 paddr =
dma_map_single(nfc->dev, buf, len, dir);
8db68ae6 Naga Sureshkumar Relli 2018-11-09 348 if
(dma_mapping_error(nfc->dev, paddr)) {
8db68ae6 Naga Sureshkumar Relli 2018-11-09 349
dev_err(nfc->dev, "Read buffer mapping error");
8db68ae6 Naga Sureshkumar Relli 2018-11-09 350 return;
8db68ae6 Naga Sureshkumar Relli 2018-11-09 351 }
8db68ae6 Naga Sureshkumar Relli 2018-11-09 352 writel(paddr, nfc->base
+ DMA_ADDR0_OFST);
8db68ae6 Naga Sureshkumar Relli 2018-11-09 @353 writel((paddr >> 32),
nfc->base + DMA_ADDR1_OFST);
^
This is zero. Which is probably intended and fine... I was hoping it
would have the other line 1032 warning in the email...
8db68ae6 Naga Sureshkumar Relli 2018-11-09 354 anfc_enable_intrs(nfc,
(XFER_COMPLETE | eccintr));
8db68ae6 Naga Sureshkumar Relli 2018-11-09 355 writel(prog, nfc->base
+ PROG_OFST);
8db68ae6 Naga Sureshkumar Relli 2018-11-09 356
anfc_wait_for_event(nfc);
8db68ae6 Naga Sureshkumar Relli 2018-11-09 357
dma_unmap_single(nfc->dev, paddr, len, dir);
8db68ae6 Naga Sureshkumar Relli 2018-11-09 358 }
8db68ae6 Naga Sureshkumar Relli 2018-11-09 359
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
Hi Naga,
Thank you for the patch! Perhaps something to improve:
url:
https://github.com/0day-ci/linux/commits/Naga-Sureshkumar-Relli/dt-bindings-mtd-arasan-Add-device-tree-binding-documentation/20181110-034106
base: git://git.infradead.org/linux-mtd.git nand/next
smatch warnings:
drivers/mtd/nand/raw/arasan_nand.c:353 anfc_rw_dma_op() warn: right shifting
more than type allows 32 vs 32
drivers/mtd/nand/raw/arasan_nand.c:1032 anfc_setup_data_interface() warn:
unsigned 'sdr->mode' is never less than zero.
#
https://github.com/0day-ci/linux/commit/8db68ae6047a392d3e4092cbd6d3051eec1edd47
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout 8db68ae6047a392d3e4092cbd6d3051eec1edd47
vim +353 drivers/mtd/nand/raw/arasan_nand.c
8db68ae6 Naga Sureshkumar Relli 2018-11-09 325
8db68ae6 Naga Sureshkumar Relli 2018-11-09 326 static void
anfc_rw_dma_op(struct mtd_info *mtd, u8 *buf, int len,
8db68ae6 Naga Sureshkumar Relli 2018-11-09 327bool
do_read, u32 prog, int pktcount, int pktsize)
8db68ae6 Naga Sureshkumar Relli 2018-11-09 328 {
8db68ae6 Naga Sureshkumar Relli 2018-11-09 329 dma_addr_t paddr;
8db68ae6 Naga Sureshkumar Relli 2018-11-09 330 struct nand_chip *chip
= mtd_to_nand(mtd);
8db68ae6 Naga Sureshkumar Relli 2018-11-09 331 struct
anfc_nand_controller *nfc = to_anfc(chip->controller);
8db68ae6 Naga Sureshkumar Relli 2018-11-09 332 struct anfc_nand_chip
*achip = to_anfc_nand(chip);
8db68ae6 Naga Sureshkumar Relli 2018-11-09 333 u32 eccintr = 0, dir;
8db68ae6 Naga Sureshkumar Relli 2018-11-09 334
8db68ae6 Naga Sureshkumar Relli 2018-11-09 335 if (pktsize == 0)
8db68ae6 Naga Sureshkumar Relli 2018-11-09 336 pktsize = len;
8db68ae6 Naga Sureshkumar Relli 2018-11-09 337
8db68ae6 Naga Sureshkumar Relli 2018-11-09 338 anfc_setpktszcnt(nfc,
pktsize, pktcount);
8db68ae6 Naga Sureshkumar Relli 2018-11-09 339
8db68ae6 Naga Sureshkumar Relli 2018-11-09 340 if (!achip->strength)
8db68ae6 Naga Sureshkumar Relli 2018-11-09 341 eccintr =
MBIT_ERROR;
8db68ae6 Naga Sureshkumar Relli 2018-11-09 342
8db68ae6 Naga Sureshkumar Relli 2018-11-09 343 if (do_read)
8db68ae6 Naga Sureshkumar Relli 2018-11-09 344 dir =
DMA_FROM_DEVICE;
8db68ae6 Naga Sureshkumar Relli 2018-11-09 345 else
8db68ae6 Naga Sureshkumar Relli 2018-11-09 346 dir =
DMA_TO_DEVICE;
8db68ae6 Naga Sureshkumar Relli 2018-11-09 347 paddr =
dma_map_single(nfc->dev, buf, len, dir);
8db68ae6 Naga Sureshkumar Relli 2018-11-09 348 if
(dma_mapping_error(nfc->dev, paddr)) {
8db68ae6 Naga Sureshkumar Relli 2018-11-09 349
dev_err(nfc->dev, "Read buffer mapping error");
8db68ae6 Naga Sureshkumar Relli 2018-11-09 350 return;
8db68ae6 Naga Sureshkumar Relli 2018-11-09 351 }
8db68ae6 Naga Sureshkumar Relli 2018-11-09 352 writel(paddr, nfc->base
+ DMA_ADDR0_OFST);
8db68ae6 Naga Sureshkumar Relli 2018-11-09 @353 writel((paddr >> 32),
nfc->base + DMA_ADDR1_OFST);
^
This is zero. Which is probably intended and fine... I was hoping it
would have the other line 1032 warning in the email...
8db68ae6 Naga Sureshkumar Relli 2018-11-09 354 anfc_enable_intrs(nfc,
(XFER_COMPLETE | eccintr));
8db68ae6 Naga Sureshkumar Relli 2018-11-09 355 writel(prog, nfc->base
+ PROG_OFST);
8db68ae6 Naga Sureshkumar Relli 2018-11-09 356
anfc_wait_for_event(nfc);
8db68ae6 Naga Sureshkumar Relli 2018-11-09 357
dma_unmap_single(nfc->dev, paddr, len, dir);
8db68ae6 Naga Sureshkumar Relli 2018-11-09 358 }
8db68ae6 Naga Sureshkumar Relli 2018-11-09 359
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
RE: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
Hi Boris & Miquel,
I am updating the driver by addressing your comments, and I have one concern,
especially in anfc_read_page_hwecc(),
there I am checking for erased pages bit flips.
Since Arasan NAND controller doesn't have multibit error detection beyond
24-bit( it can correct up to 24 bit),
i.e. there is no indication from controller to detect uncorrectable error
beyond 24bit.
So I took some error count as default value(MULTI_BIT_ERR_CNT 16, I put this
based on the error count that
I got while reading erased page on Micron device).
And during a page read, will just read the error count register and compare
this value with the default error count(16) and if it is more
Than default then I am checking for erased page bit flips.
I am doubting that this will not work in all cases.
In my case it is just working because the error count that it got on an erased
page is 16.
Could you please suggest a way to do detect erased_page bit flips when reading
a page with HW-ECC?.
> +
> +/*
> + * Arasan NAND controller can't detect errors beyond 24-bit in BCH
> + * For an erased page we observed that multibit error count as 16
> + * with 24-bit ECC. so if the count is equal to or greater than 16
> + * then we can say that its an uncorrectable ECC error.
> + */
> +#define MULTI_BIT_ERR_CNT 16
> +
>
> +}
> +
> +static int anfc_read_page_hwecc(struct nand_chip *chip, u8 *buf,
> + int oob_required, int page)
> +{
> + struct anfc_nand_controller *nfc = to_anfc(chip->controller);
> + struct anfc_nand_chip *achip = to_anfc_nand(chip);
> + struct mtd_info *mtd = nand_to_mtd(chip);
> + u8 *ecc_code = chip->ecc.code_buf;
> + u8 *p;
> + int eccsize = chip->ecc.size;
> + int eccbytes = chip->ecc.bytes;
> + int stat = 0, i;
> + u32 ret;
> + unsigned int max_bitflips = 0;
> + u32 eccsteps = chip->ecc.steps;
> + u32 one_bit_err = 0, multi_bit_err = 0;
> +
> + anfc_set_eccsparecmd(nfc, achip, NAND_CMD_RNDOUT,
> NAND_CMD_RNDOUTSTART);
> + anfc_config_ecc(nfc, true);
> +
> + ret = nand_read_page_op(chip, page, 0, buf, mtd->writesize);
> + if (ret)
> + return ret;
> +
> + anfc_config_ecc(nfc, false);
> + if (achip->strength) {
> + /*
> + * In BCH mode Arasan NAND controller can correct ECC upto
> + * 24-bit Beyond that, it can't even detect errors.
> + */
> + multi_bit_err = readl(nfc->base + ECC_ERR_CNT_OFST);
> + multi_bit_err = ((multi_bit_err & PAGE_ERR_CNT_MASK) >> 8);
> + } else {
> + /*
> + * In Hamming mode Arasan NAND controller can correct ECC upto
> + * 1-bit and can detect upto 2-bit errors.
> + */
> + one_bit_err = readl(nfc->base + ECC_ERR_CNT_1BIT_OFST);
> + multi_bit_err = readl(nfc->base + ECC_ERR_CNT_2BIT_OFST);
> + /* Clear ecc error count register 1Bit, 2Bit */
> + writel(0x0, nfc->base + ECC_ERR_CNT_1BIT_OFST);
> + writel(0x0, nfc->base + ECC_ERR_CNT_2BIT_OFST);
> + }
> +
> + if (oob_required)
> + chip->ecc.read_oob(chip, page);
> +
> + if (multi_bit_err >= MULTI_BIT_ERR_CNT) {
> + if (!oob_required)
> + chip->ecc.read_oob(chip, page);
> +
> + mtd_ooblayout_get_eccbytes(mtd, ecc_code, chip->oob_poi, 0,
> +chip->ecc.total);
> + p = buf;
> + for (i = 0; eccsteps; eccsteps--, i += eccbytes,
> + p += eccsize) {
> + stat = nand_check_erased_ecc_chunk(p,
> +chip->ecc.size,
> +_code[i],
> +eccbytes,
> +NULL, 0,
> +chip->ecc.strength);
> + if (stat < 0) {
> + mtd->ecc_stats.failed++;
> + } else {
> + mtd->ecc_stats.corrected += stat;
> + max_bitflips = max_t(unsigned int, max_bitflips,
> + stat);
> + }
> + }
> + }
> +
> + return max_bitflips;
> +}
> +
> +
Thanks,
Naga Sureshkumar Relli.
RE: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
Hi Boris & Miquel,
I am updating the driver by addressing your comments, and I have one concern,
especially in anfc_read_page_hwecc(),
there I am checking for erased pages bit flips.
Since Arasan NAND controller doesn't have multibit error detection beyond
24-bit( it can correct up to 24 bit),
i.e. there is no indication from controller to detect uncorrectable error
beyond 24bit.
So I took some error count as default value(MULTI_BIT_ERR_CNT 16, I put this
based on the error count that
I got while reading erased page on Micron device).
And during a page read, will just read the error count register and compare
this value with the default error count(16) and if it is more
Than default then I am checking for erased page bit flips.
I am doubting that this will not work in all cases.
In my case it is just working because the error count that it got on an erased
page is 16.
Could you please suggest a way to do detect erased_page bit flips when reading
a page with HW-ECC?.
> +
> +/*
> + * Arasan NAND controller can't detect errors beyond 24-bit in BCH
> + * For an erased page we observed that multibit error count as 16
> + * with 24-bit ECC. so if the count is equal to or greater than 16
> + * then we can say that its an uncorrectable ECC error.
> + */
> +#define MULTI_BIT_ERR_CNT 16
> +
>
> +}
> +
> +static int anfc_read_page_hwecc(struct nand_chip *chip, u8 *buf,
> + int oob_required, int page)
> +{
> + struct anfc_nand_controller *nfc = to_anfc(chip->controller);
> + struct anfc_nand_chip *achip = to_anfc_nand(chip);
> + struct mtd_info *mtd = nand_to_mtd(chip);
> + u8 *ecc_code = chip->ecc.code_buf;
> + u8 *p;
> + int eccsize = chip->ecc.size;
> + int eccbytes = chip->ecc.bytes;
> + int stat = 0, i;
> + u32 ret;
> + unsigned int max_bitflips = 0;
> + u32 eccsteps = chip->ecc.steps;
> + u32 one_bit_err = 0, multi_bit_err = 0;
> +
> + anfc_set_eccsparecmd(nfc, achip, NAND_CMD_RNDOUT,
> NAND_CMD_RNDOUTSTART);
> + anfc_config_ecc(nfc, true);
> +
> + ret = nand_read_page_op(chip, page, 0, buf, mtd->writesize);
> + if (ret)
> + return ret;
> +
> + anfc_config_ecc(nfc, false);
> + if (achip->strength) {
> + /*
> + * In BCH mode Arasan NAND controller can correct ECC upto
> + * 24-bit Beyond that, it can't even detect errors.
> + */
> + multi_bit_err = readl(nfc->base + ECC_ERR_CNT_OFST);
> + multi_bit_err = ((multi_bit_err & PAGE_ERR_CNT_MASK) >> 8);
> + } else {
> + /*
> + * In Hamming mode Arasan NAND controller can correct ECC upto
> + * 1-bit and can detect upto 2-bit errors.
> + */
> + one_bit_err = readl(nfc->base + ECC_ERR_CNT_1BIT_OFST);
> + multi_bit_err = readl(nfc->base + ECC_ERR_CNT_2BIT_OFST);
> + /* Clear ecc error count register 1Bit, 2Bit */
> + writel(0x0, nfc->base + ECC_ERR_CNT_1BIT_OFST);
> + writel(0x0, nfc->base + ECC_ERR_CNT_2BIT_OFST);
> + }
> +
> + if (oob_required)
> + chip->ecc.read_oob(chip, page);
> +
> + if (multi_bit_err >= MULTI_BIT_ERR_CNT) {
> + if (!oob_required)
> + chip->ecc.read_oob(chip, page);
> +
> + mtd_ooblayout_get_eccbytes(mtd, ecc_code, chip->oob_poi, 0,
> +chip->ecc.total);
> + p = buf;
> + for (i = 0; eccsteps; eccsteps--, i += eccbytes,
> + p += eccsize) {
> + stat = nand_check_erased_ecc_chunk(p,
> +chip->ecc.size,
> +_code[i],
> +eccbytes,
> +NULL, 0,
> +chip->ecc.strength);
> + if (stat < 0) {
> + mtd->ecc_stats.failed++;
> + } else {
> + mtd->ecc_stats.corrected += stat;
> + max_bitflips = max_t(unsigned int, max_bitflips,
> + stat);
> + }
> + }
> + }
> +
> + return max_bitflips;
> +}
> +
> +
Thanks,
Naga Sureshkumar Relli.
RE: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
Hi Boris & Martin,
> -Original Message-
> From: Boris Brezillon [mailto:boris.brezil...@bootlin.com]
> Sent: Monday, November 12, 2018 4:28 PM
> To: Martin Lund
> Cc: Naga Sureshkumar Relli ; miquel.ray...@bootlin.com;
> rich...@nod.at; dw...@infradead.org; computersforpe...@gmail.com;
> marek.va...@gmail.com; Michal Simek ;
> nagasures...@gmail.com;
> linux-...@lists.infradead.org; linux-kernel@vger.kernel.org; r...@kernel.org
> Subject: Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for
> Arasan NAND
> Flash Controller
>
> On Mon, 12 Nov 2018 11:55:36 +0100
> Martin Lund wrote:
>
> > Hi Naga,
> >
> > Just a few review comments for the v12 version.
> >
> > On Fri, Nov 9, 2018 at 6:00 AM Naga Sureshkumar Relli
> > wrote:
> > > +#define PKT_OFST 0x00
> > > +#define PKT_CNT_SHIFT 12
> > > +
> > > +#define MEM_ADDR1_OFST 0x04
> > > +#define MEM_ADDR2_OFST 0x08
> >
> > For the sake of readability I think *_OFFSET is preferred, especially
> > since the driver already includes short macro names. I think this is
> > similar to the EVNT vs EVENT point.
> > The majority of drivers in the Linux kernel do not shorten OFFSET -> OFST.
> >
> >
> > > +static void anfc_rw_pio_op(struct mtd_info *mtd, u8 *buf, int len,
> > > + bool do_read, int prog, int pktcount, int
> > > +pktsize) {
> > > + struct nand_chip *chip = mtd_to_nand(mtd);
> > > + struct anfc_nand_controller *nfc = to_anfc(chip->controller);
> > > + struct anfc_nand_chip *achip = to_anfc_nand(chip);
> > > + u32 *bufptr = (u32 *)buf;
> > > + u32 cnt = 0, intr = 0;
> > > +
> > > + anfc_config_dma(nfc, 0);
> > > +
> > > + if (pktsize == 0)
> > > + pktsize = len;
> > > +
> > > + anfc_setpktszcnt(nfc, pktsize, pktcount);
> > > +
> > > + if (!achip->strength)
> > > + intr = MBIT_ERROR;
> > > +
> > > + if (do_read)
> > > + intr |= READ_READY;
> > > + else
> > > + intr |= WRITE_READY;
> > > + anfc_enable_intrs(nfc, intr);
> > > + writel(prog, nfc->base + PROG_OFST);
> > > + while (cnt < pktcount) {
> > > + anfc_wait_for_event(nfc);
> > > + cnt++;
> > > + if (cnt == pktcount)
> > > + anfc_enable_intrs(nfc, XFER_COMPLETE);
> > > + if (do_read)
> > > + ioread32_rep(nfc->base + DATA_PORT_OFST, bufptr,
> > > +pktsize / 4);
> > > + else
> > > + iowrite32_rep(nfc->base + DATA_PORT_OFST, bufptr,
> > > + pktsize / 4);
> > > + bufptr += (pktsize / 4);
> > > + if (cnt < pktcount)
> > > + anfc_enable_intrs(nfc, intr);
> > > + }
> > > + anfc_wait_for_event(nfc);
> > > +}
> >
> > Throughout the driver all calls to anfc_wait_for_event() ignores the
> > timeout return value. It would be nice to see some error handling in
> > case it times out - at minimum consider printing out an error message
> > since timeout on NAND operations are fairly critical and should
> > generally not occur. Perhaps even an argument can be made for
> > returning -ETIMEDOUT in case of timeout.
>
> Yes please, check anfc_wait_for_event() return code and propagate the error
> to the upper layer.
Ok, I will update
Thanks,
Naga Sureshkumar Relli
RE: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
Hi Boris & Martin,
> -Original Message-
> From: Boris Brezillon [mailto:boris.brezil...@bootlin.com]
> Sent: Monday, November 12, 2018 4:28 PM
> To: Martin Lund
> Cc: Naga Sureshkumar Relli ; miquel.ray...@bootlin.com;
> rich...@nod.at; dw...@infradead.org; computersforpe...@gmail.com;
> marek.va...@gmail.com; Michal Simek ;
> nagasures...@gmail.com;
> linux-...@lists.infradead.org; linux-kernel@vger.kernel.org; r...@kernel.org
> Subject: Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for
> Arasan NAND
> Flash Controller
>
> On Mon, 12 Nov 2018 11:55:36 +0100
> Martin Lund wrote:
>
> > Hi Naga,
> >
> > Just a few review comments for the v12 version.
> >
> > On Fri, Nov 9, 2018 at 6:00 AM Naga Sureshkumar Relli
> > wrote:
> > > +#define PKT_OFST 0x00
> > > +#define PKT_CNT_SHIFT 12
> > > +
> > > +#define MEM_ADDR1_OFST 0x04
> > > +#define MEM_ADDR2_OFST 0x08
> >
> > For the sake of readability I think *_OFFSET is preferred, especially
> > since the driver already includes short macro names. I think this is
> > similar to the EVNT vs EVENT point.
> > The majority of drivers in the Linux kernel do not shorten OFFSET -> OFST.
> >
> >
> > > +static void anfc_rw_pio_op(struct mtd_info *mtd, u8 *buf, int len,
> > > + bool do_read, int prog, int pktcount, int
> > > +pktsize) {
> > > + struct nand_chip *chip = mtd_to_nand(mtd);
> > > + struct anfc_nand_controller *nfc = to_anfc(chip->controller);
> > > + struct anfc_nand_chip *achip = to_anfc_nand(chip);
> > > + u32 *bufptr = (u32 *)buf;
> > > + u32 cnt = 0, intr = 0;
> > > +
> > > + anfc_config_dma(nfc, 0);
> > > +
> > > + if (pktsize == 0)
> > > + pktsize = len;
> > > +
> > > + anfc_setpktszcnt(nfc, pktsize, pktcount);
> > > +
> > > + if (!achip->strength)
> > > + intr = MBIT_ERROR;
> > > +
> > > + if (do_read)
> > > + intr |= READ_READY;
> > > + else
> > > + intr |= WRITE_READY;
> > > + anfc_enable_intrs(nfc, intr);
> > > + writel(prog, nfc->base + PROG_OFST);
> > > + while (cnt < pktcount) {
> > > + anfc_wait_for_event(nfc);
> > > + cnt++;
> > > + if (cnt == pktcount)
> > > + anfc_enable_intrs(nfc, XFER_COMPLETE);
> > > + if (do_read)
> > > + ioread32_rep(nfc->base + DATA_PORT_OFST, bufptr,
> > > +pktsize / 4);
> > > + else
> > > + iowrite32_rep(nfc->base + DATA_PORT_OFST, bufptr,
> > > + pktsize / 4);
> > > + bufptr += (pktsize / 4);
> > > + if (cnt < pktcount)
> > > + anfc_enable_intrs(nfc, intr);
> > > + }
> > > + anfc_wait_for_event(nfc);
> > > +}
> >
> > Throughout the driver all calls to anfc_wait_for_event() ignores the
> > timeout return value. It would be nice to see some error handling in
> > case it times out - at minimum consider printing out an error message
> > since timeout on NAND operations are fairly critical and should
> > generally not occur. Perhaps even an argument can be made for
> > returning -ETIMEDOUT in case of timeout.
>
> Yes please, check anfc_wait_for_event() return code and propagate the error
> to the upper layer.
Ok, I will update
Thanks,
Naga Sureshkumar Relli
Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
Hi Naga,
Just a few review comments for the v12 version.
On Fri, Nov 9, 2018 at 6:00 AM Naga Sureshkumar Relli
wrote:
> +#define PKT_OFST 0x00
> +#define PKT_CNT_SHIFT 12
> +
> +#define MEM_ADDR1_OFST 0x04
> +#define MEM_ADDR2_OFST 0x08
For the sake of readability I think *_OFFSET is preferred, especially
since the driver already includes short macro names. I think this is
similar to the EVNT vs EVENT point.
The majority of drivers in the Linux kernel do not shorten OFFSET -> OFST.
> +static void anfc_rw_pio_op(struct mtd_info *mtd, u8 *buf, int len,
> + bool do_read, int prog, int pktcount, int pktsize)
> +{
> + struct nand_chip *chip = mtd_to_nand(mtd);
> + struct anfc_nand_controller *nfc = to_anfc(chip->controller);
> + struct anfc_nand_chip *achip = to_anfc_nand(chip);
> + u32 *bufptr = (u32 *)buf;
> + u32 cnt = 0, intr = 0;
> +
> + anfc_config_dma(nfc, 0);
> +
> + if (pktsize == 0)
> + pktsize = len;
> +
> + anfc_setpktszcnt(nfc, pktsize, pktcount);
> +
> + if (!achip->strength)
> + intr = MBIT_ERROR;
> +
> + if (do_read)
> + intr |= READ_READY;
> + else
> + intr |= WRITE_READY;
> + anfc_enable_intrs(nfc, intr);
> + writel(prog, nfc->base + PROG_OFST);
> + while (cnt < pktcount) {
> + anfc_wait_for_event(nfc);
> + cnt++;
> + if (cnt == pktcount)
> + anfc_enable_intrs(nfc, XFER_COMPLETE);
> + if (do_read)
> + ioread32_rep(nfc->base + DATA_PORT_OFST, bufptr,
> +pktsize / 4);
> + else
> + iowrite32_rep(nfc->base + DATA_PORT_OFST, bufptr,
> + pktsize / 4);
> + bufptr += (pktsize / 4);
> + if (cnt < pktcount)
> + anfc_enable_intrs(nfc, intr);
> + }
> + anfc_wait_for_event(nfc);
> +}
Throughout the driver all calls to anfc_wait_for_event() ignores the
timeout return value. It would be nice to see some error handling in
case it times out - at minimum consider printing out an error message
since timeout on NAND operations are fairly critical and should
generally not occur. Perhaps even an argument can be made for
returning -ETIMEDOUT in case of timeout.
I'm putting a focus on this because I see the original non-upstream
Arasan driver sometimes timeout on NAND operations when I stress test
it via the UBI stress test. Not sure what the cause for the timeout is
yet but either way an error message would have been helpful.
Br, Martin
Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
Hi Naga,
Just a few review comments for the v12 version.
On Fri, Nov 9, 2018 at 6:00 AM Naga Sureshkumar Relli
wrote:
> +#define PKT_OFST 0x00
> +#define PKT_CNT_SHIFT 12
> +
> +#define MEM_ADDR1_OFST 0x04
> +#define MEM_ADDR2_OFST 0x08
For the sake of readability I think *_OFFSET is preferred, especially
since the driver already includes short macro names. I think this is
similar to the EVNT vs EVENT point.
The majority of drivers in the Linux kernel do not shorten OFFSET -> OFST.
> +static void anfc_rw_pio_op(struct mtd_info *mtd, u8 *buf, int len,
> + bool do_read, int prog, int pktcount, int pktsize)
> +{
> + struct nand_chip *chip = mtd_to_nand(mtd);
> + struct anfc_nand_controller *nfc = to_anfc(chip->controller);
> + struct anfc_nand_chip *achip = to_anfc_nand(chip);
> + u32 *bufptr = (u32 *)buf;
> + u32 cnt = 0, intr = 0;
> +
> + anfc_config_dma(nfc, 0);
> +
> + if (pktsize == 0)
> + pktsize = len;
> +
> + anfc_setpktszcnt(nfc, pktsize, pktcount);
> +
> + if (!achip->strength)
> + intr = MBIT_ERROR;
> +
> + if (do_read)
> + intr |= READ_READY;
> + else
> + intr |= WRITE_READY;
> + anfc_enable_intrs(nfc, intr);
> + writel(prog, nfc->base + PROG_OFST);
> + while (cnt < pktcount) {
> + anfc_wait_for_event(nfc);
> + cnt++;
> + if (cnt == pktcount)
> + anfc_enable_intrs(nfc, XFER_COMPLETE);
> + if (do_read)
> + ioread32_rep(nfc->base + DATA_PORT_OFST, bufptr,
> +pktsize / 4);
> + else
> + iowrite32_rep(nfc->base + DATA_PORT_OFST, bufptr,
> + pktsize / 4);
> + bufptr += (pktsize / 4);
> + if (cnt < pktcount)
> + anfc_enable_intrs(nfc, intr);
> + }
> + anfc_wait_for_event(nfc);
> +}
Throughout the driver all calls to anfc_wait_for_event() ignores the
timeout return value. It would be nice to see some error handling in
case it times out - at minimum consider printing out an error message
since timeout on NAND operations are fairly critical and should
generally not occur. Perhaps even an argument can be made for
returning -ETIMEDOUT in case of timeout.
I'm putting a focus on this because I see the original non-upstream
Arasan driver sometimes timeout on NAND operations when I stress test
it via the UBI stress test. Not sure what the cause for the timeout is
yet but either way an error message would have been helpful.
Br, Martin
Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
On Mon, 12 Nov 2018 11:55:36 +0100
Martin Lund wrote:
> Hi Naga,
>
> Just a few review comments for the v12 version.
>
> On Fri, Nov 9, 2018 at 6:00 AM Naga Sureshkumar Relli
> wrote:
> > +#define PKT_OFST 0x00
> > +#define PKT_CNT_SHIFT 12
> > +
> > +#define MEM_ADDR1_OFST 0x04
> > +#define MEM_ADDR2_OFST 0x08
>
> For the sake of readability I think *_OFFSET is preferred, especially
> since the driver already includes short macro names. I think this is
> similar to the EVNT vs EVENT point.
> The majority of drivers in the Linux kernel do not shorten OFFSET -> OFST.
>
>
> > +static void anfc_rw_pio_op(struct mtd_info *mtd, u8 *buf, int len,
> > + bool do_read, int prog, int pktcount, int
> > pktsize)
> > +{
> > + struct nand_chip *chip = mtd_to_nand(mtd);
> > + struct anfc_nand_controller *nfc = to_anfc(chip->controller);
> > + struct anfc_nand_chip *achip = to_anfc_nand(chip);
> > + u32 *bufptr = (u32 *)buf;
> > + u32 cnt = 0, intr = 0;
> > +
> > + anfc_config_dma(nfc, 0);
> > +
> > + if (pktsize == 0)
> > + pktsize = len;
> > +
> > + anfc_setpktszcnt(nfc, pktsize, pktcount);
> > +
> > + if (!achip->strength)
> > + intr = MBIT_ERROR;
> > +
> > + if (do_read)
> > + intr |= READ_READY;
> > + else
> > + intr |= WRITE_READY;
> > + anfc_enable_intrs(nfc, intr);
> > + writel(prog, nfc->base + PROG_OFST);
> > + while (cnt < pktcount) {
> > + anfc_wait_for_event(nfc);
> > + cnt++;
> > + if (cnt == pktcount)
> > + anfc_enable_intrs(nfc, XFER_COMPLETE);
> > + if (do_read)
> > + ioread32_rep(nfc->base + DATA_PORT_OFST, bufptr,
> > +pktsize / 4);
> > + else
> > + iowrite32_rep(nfc->base + DATA_PORT_OFST, bufptr,
> > + pktsize / 4);
> > + bufptr += (pktsize / 4);
> > + if (cnt < pktcount)
> > + anfc_enable_intrs(nfc, intr);
> > + }
> > + anfc_wait_for_event(nfc);
> > +}
>
> Throughout the driver all calls to anfc_wait_for_event() ignores the
> timeout return value. It would be nice to see some error handling in
> case it times out - at minimum consider printing out an error message
> since timeout on NAND operations are fairly critical and should
> generally not occur. Perhaps even an argument can be made for
> returning -ETIMEDOUT in case of timeout.
Yes please, check anfc_wait_for_event() return code and propagate the
error to the upper layer.
Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
On Mon, 12 Nov 2018 11:55:36 +0100
Martin Lund wrote:
> Hi Naga,
>
> Just a few review comments for the v12 version.
>
> On Fri, Nov 9, 2018 at 6:00 AM Naga Sureshkumar Relli
> wrote:
> > +#define PKT_OFST 0x00
> > +#define PKT_CNT_SHIFT 12
> > +
> > +#define MEM_ADDR1_OFST 0x04
> > +#define MEM_ADDR2_OFST 0x08
>
> For the sake of readability I think *_OFFSET is preferred, especially
> since the driver already includes short macro names. I think this is
> similar to the EVNT vs EVENT point.
> The majority of drivers in the Linux kernel do not shorten OFFSET -> OFST.
>
>
> > +static void anfc_rw_pio_op(struct mtd_info *mtd, u8 *buf, int len,
> > + bool do_read, int prog, int pktcount, int
> > pktsize)
> > +{
> > + struct nand_chip *chip = mtd_to_nand(mtd);
> > + struct anfc_nand_controller *nfc = to_anfc(chip->controller);
> > + struct anfc_nand_chip *achip = to_anfc_nand(chip);
> > + u32 *bufptr = (u32 *)buf;
> > + u32 cnt = 0, intr = 0;
> > +
> > + anfc_config_dma(nfc, 0);
> > +
> > + if (pktsize == 0)
> > + pktsize = len;
> > +
> > + anfc_setpktszcnt(nfc, pktsize, pktcount);
> > +
> > + if (!achip->strength)
> > + intr = MBIT_ERROR;
> > +
> > + if (do_read)
> > + intr |= READ_READY;
> > + else
> > + intr |= WRITE_READY;
> > + anfc_enable_intrs(nfc, intr);
> > + writel(prog, nfc->base + PROG_OFST);
> > + while (cnt < pktcount) {
> > + anfc_wait_for_event(nfc);
> > + cnt++;
> > + if (cnt == pktcount)
> > + anfc_enable_intrs(nfc, XFER_COMPLETE);
> > + if (do_read)
> > + ioread32_rep(nfc->base + DATA_PORT_OFST, bufptr,
> > +pktsize / 4);
> > + else
> > + iowrite32_rep(nfc->base + DATA_PORT_OFST, bufptr,
> > + pktsize / 4);
> > + bufptr += (pktsize / 4);
> > + if (cnt < pktcount)
> > + anfc_enable_intrs(nfc, intr);
> > + }
> > + anfc_wait_for_event(nfc);
> > +}
>
> Throughout the driver all calls to anfc_wait_for_event() ignores the
> timeout return value. It would be nice to see some error handling in
> case it times out - at minimum consider printing out an error message
> since timeout on NAND operations are fairly critical and should
> generally not occur. Perhaps even an argument can be made for
> returning -ETIMEDOUT in case of timeout.
Yes please, check anfc_wait_for_event() return code and propagate the
error to the upper layer.
Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
Hi Naga,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on mtd/nand/next]
[also build test WARNING on v4.20-rc1 next-20181109]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Naga-Sureshkumar-Relli/dt-bindings-mtd-arasan-Add-device-tree-binding-documentation/20181110-034106
base: git://git.infradead.org/linux-mtd.git nand/next
config: sh-allyesconfig (attached as .config)
compiler: sh4-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.2.0 make.cross ARCH=sh
All warnings (new ones prefixed by >>):
In file included from include/linux/scatterlist.h:9:0,
from include/linux/dma-mapping.h:11,
from drivers/mtd//nand/raw/arasan_nand.c:12:
drivers/mtd//nand/raw/arasan_nand.c: In function 'anfc_rw_dma_op':
>> drivers/mtd//nand/raw/arasan_nand.c:353:16: warning: right shift count >=
>> width of type [-Wshift-count-overflow]
writel((paddr >> 32), nfc->base + DMA_ADDR1_OFST);
^
arch/sh/include/asm/io.h:31:77: note: in definition of macro '__raw_writel'
#define __raw_writel(v,a) (__chk_io_ptr(a), *(volatile u32 __force *)(a) =
(v))
^
arch/sh/include/asm/io.h:46:62: note: in expansion of macro 'ioswabl'
#define writel_relaxed(v,c) ((void)__raw_writel((__force u32)ioswabl(v),c))
^~~
arch/sh/include/asm/io.h:56:32: note: in expansion of macro 'writel_relaxed'
#define writel(v,a) ({ wmb(); writel_relaxed((v),(a)); })
^~
>> drivers/mtd//nand/raw/arasan_nand.c:353:2: note: in expansion of macro
>> 'writel'
writel((paddr >> 32), nfc->base + DMA_ADDR1_OFST);
^~
vim +353 drivers/mtd//nand/raw/arasan_nand.c
325
326 static void anfc_rw_dma_op(struct mtd_info *mtd, u8 *buf, int len,
327 bool do_read, u32 prog, int pktcount, int
pktsize)
328 {
329 dma_addr_t paddr;
330 struct nand_chip *chip = mtd_to_nand(mtd);
331 struct anfc_nand_controller *nfc = to_anfc(chip->controller);
332 struct anfc_nand_chip *achip = to_anfc_nand(chip);
333 u32 eccintr = 0, dir;
334
335 if (pktsize == 0)
336 pktsize = len;
337
338 anfc_setpktszcnt(nfc, pktsize, pktcount);
339
340 if (!achip->strength)
341 eccintr = MBIT_ERROR;
342
343 if (do_read)
344 dir = DMA_FROM_DEVICE;
345 else
346 dir = DMA_TO_DEVICE;
347 paddr = dma_map_single(nfc->dev, buf, len, dir);
348 if (dma_mapping_error(nfc->dev, paddr)) {
349 dev_err(nfc->dev, "Read buffer mapping error");
350 return;
351 }
352 writel(paddr, nfc->base + DMA_ADDR0_OFST);
> 353 writel((paddr >> 32), nfc->base + DMA_ADDR1_OFST);
354 anfc_enable_intrs(nfc, (XFER_COMPLETE | eccintr));
355 writel(prog, nfc->base + PROG_OFST);
356 anfc_wait_for_event(nfc);
357 dma_unmap_single(nfc->dev, paddr, len, dir);
358 }
359
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
.config.gz
Description: application/gzip
Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
Hi Naga,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on mtd/nand/next]
[also build test WARNING on v4.20-rc1 next-20181109]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Naga-Sureshkumar-Relli/dt-bindings-mtd-arasan-Add-device-tree-binding-documentation/20181110-034106
base: git://git.infradead.org/linux-mtd.git nand/next
config: sh-allyesconfig (attached as .config)
compiler: sh4-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.2.0 make.cross ARCH=sh
All warnings (new ones prefixed by >>):
In file included from include/linux/scatterlist.h:9:0,
from include/linux/dma-mapping.h:11,
from drivers/mtd//nand/raw/arasan_nand.c:12:
drivers/mtd//nand/raw/arasan_nand.c: In function 'anfc_rw_dma_op':
>> drivers/mtd//nand/raw/arasan_nand.c:353:16: warning: right shift count >=
>> width of type [-Wshift-count-overflow]
writel((paddr >> 32), nfc->base + DMA_ADDR1_OFST);
^
arch/sh/include/asm/io.h:31:77: note: in definition of macro '__raw_writel'
#define __raw_writel(v,a) (__chk_io_ptr(a), *(volatile u32 __force *)(a) =
(v))
^
arch/sh/include/asm/io.h:46:62: note: in expansion of macro 'ioswabl'
#define writel_relaxed(v,c) ((void)__raw_writel((__force u32)ioswabl(v),c))
^~~
arch/sh/include/asm/io.h:56:32: note: in expansion of macro 'writel_relaxed'
#define writel(v,a) ({ wmb(); writel_relaxed((v),(a)); })
^~
>> drivers/mtd//nand/raw/arasan_nand.c:353:2: note: in expansion of macro
>> 'writel'
writel((paddr >> 32), nfc->base + DMA_ADDR1_OFST);
^~
vim +353 drivers/mtd//nand/raw/arasan_nand.c
325
326 static void anfc_rw_dma_op(struct mtd_info *mtd, u8 *buf, int len,
327 bool do_read, u32 prog, int pktcount, int
pktsize)
328 {
329 dma_addr_t paddr;
330 struct nand_chip *chip = mtd_to_nand(mtd);
331 struct anfc_nand_controller *nfc = to_anfc(chip->controller);
332 struct anfc_nand_chip *achip = to_anfc_nand(chip);
333 u32 eccintr = 0, dir;
334
335 if (pktsize == 0)
336 pktsize = len;
337
338 anfc_setpktszcnt(nfc, pktsize, pktcount);
339
340 if (!achip->strength)
341 eccintr = MBIT_ERROR;
342
343 if (do_read)
344 dir = DMA_FROM_DEVICE;
345 else
346 dir = DMA_TO_DEVICE;
347 paddr = dma_map_single(nfc->dev, buf, len, dir);
348 if (dma_mapping_error(nfc->dev, paddr)) {
349 dev_err(nfc->dev, "Read buffer mapping error");
350 return;
351 }
352 writel(paddr, nfc->base + DMA_ADDR0_OFST);
> 353 writel((paddr >> 32), nfc->base + DMA_ADDR1_OFST);
354 anfc_enable_intrs(nfc, (XFER_COMPLETE | eccintr));
355 writel(prog, nfc->base + PROG_OFST);
356 anfc_wait_for_event(nfc);
357 dma_unmap_single(nfc->dev, paddr, len, dir);
358 }
359
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
.config.gz
Description: application/gzip
RE: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
Hi Boris,
> -Original Message-
> From: Boris Brezillon [mailto:boris.brezil...@bootlin.com]
> Sent: Friday, November 9, 2018 1:38 PM
> To: Naga Sureshkumar Relli
> Cc: miquel.ray...@bootlin.com; rich...@nod.at; dw...@infradead.org;
> computersforpe...@gmail.com; marek.va...@gmail.com; Michal Simek
> ; linux-...@lists.infradead.org;
> linux-kernel@vger.kernel.org;
> nagasures...@gmail.com; r...@kernel.org
> Subject: Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for
> Arasan NAND
> Flash Controller
>
> Hi Naga,
>
> Just a preliminary review. I still think we have problems with how you
> execute NAND
> operations. You seem to assume that read/write operation are always page
> write/read operation
> with a size aligned on a page size. This is wrong, either the controller is
> able to execute the
> exact operation that has been requested or it returns -ENOTSUPP.
Are you pointing the anfc_read_param_get_feature_sp_read_type_exec()?
Where I am reading a length of page size even for get_features op.
Is that you are pointing?
>
> On Fri, 9 Nov 2018 10:30:41 +0530
> Naga Sureshkumar Relli wrote:
>
> > +
> > +/**
> > + * struct anfc_nand_chip - Defines the nand chip related information
> > + * @node: Used to store NAND chips into a list.
> > + * @chip: NAND chip information structure.
> > + * @strength: Bch or Hamming mode enable/disable.
>
> The name is still confusing. BTW, can't you deduce Hamming vs BCH from the
> strength?
> Hamming strength is 1, while BCH strengths seem to start at 4.
Yes we can deduce. I will try that.
.
>
> > + * @ecc_strength: Ecc strength 4.8/12/16.
>
> ^/
>
> > + * @eccval:Ecc config value.
> > + * @spare_caddr_cycles:Column address cycle information for spare area.
> > + * @pktsize: Packet size for read / write operation.
> > + * @csnum: chipselect number to be used.
> > + * @spktsize: Packet size in ddr mode for status operation.
> > + * @inftimeval:Data interface and timing mode information
> > + */
> > +struct anfc_nand_chip {
> > + struct list_head node;
> > + struct nand_chip chip;
> > + bool strength;
> > + u32 ecc_strength;
> > + u32 eccval;
> > + u16 spare_caddr_cycles;
> > + u32 pktsize;
> > + int csnum;
> > + u32 spktsize;
> > + u32 inftimeval;
> > +};
> > +
> > +/**
> > + * struct anfc_nand_controller - Defines the Arasan NAND flash controller
> > + * driver instance
> > + * @controller:base controller structure.
> > + * @chips: list of all nand chips attached to the ctrler.
> > + * @dev: Pointer to the device structure.
> > + * @base: Virtual address of the NAND flash device.
> > + * @clk_sys: Pointer to the system clock.
> > + * @clk_flash: Pointer to the flash clock.
> > + * @dma: Dma enable/disable.
> > + * @buf: Buffer used for read/write byte operations.
> > + * @irq: irq number
>
> You should need this field. Just get the irq num in you probe path an
> register an irq handler
> with devm_request_irq().
You mean to say, instead of putting irq in anfc_nand_controller structure,
update the code like below snippet, right?
irq = platform_get_irq(pdev, 0);
if (irq < 0) {
dev_err(dev, "failed to retrieve irq\n");
return irq;
}
devm_request_irq(>dev, irq, anfc_irq_handler, 0, "arasannfc", nfc);
>
> > + * @bufshift: Variable used for indexing buffer operation
> > + * @csnum: Chip select number currently inuse.
>
>^ in use
>
> > + * @event: Completion event for nand status events.
> > + * @status:Status of the flash device.
> > + * @prog: Used to initiate controller operations.
> > + */
> > +struct anfc_nand_controller {
> > + struct nand_controller controller;
> > + struct list_head chips;
> > + struct device *dev;
> > + void __iomem *base;
> > + struct clk *clk_sys;
> > + struct clk *clk_flash;
> > + int irq;
> > + int csnum;
> > + struct completion event;
> > + int status;
> > + u8 buf[TEMP_BUF_SIZE];
>
> Allocate this buf dynamically.
Ok
>
> > + u32 eccval;
> > +};
>
> > +static int anfc_page_write_type_exec(struct nand_chip *chip,
> > +
RE: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
Hi Boris,
> -Original Message-
> From: Boris Brezillon [mailto:boris.brezil...@bootlin.com]
> Sent: Friday, November 9, 2018 1:38 PM
> To: Naga Sureshkumar Relli
> Cc: miquel.ray...@bootlin.com; rich...@nod.at; dw...@infradead.org;
> computersforpe...@gmail.com; marek.va...@gmail.com; Michal Simek
> ; linux-...@lists.infradead.org;
> linux-kernel@vger.kernel.org;
> nagasures...@gmail.com; r...@kernel.org
> Subject: Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for
> Arasan NAND
> Flash Controller
>
> Hi Naga,
>
> Just a preliminary review. I still think we have problems with how you
> execute NAND
> operations. You seem to assume that read/write operation are always page
> write/read operation
> with a size aligned on a page size. This is wrong, either the controller is
> able to execute the
> exact operation that has been requested or it returns -ENOTSUPP.
Are you pointing the anfc_read_param_get_feature_sp_read_type_exec()?
Where I am reading a length of page size even for get_features op.
Is that you are pointing?
>
> On Fri, 9 Nov 2018 10:30:41 +0530
> Naga Sureshkumar Relli wrote:
>
> > +
> > +/**
> > + * struct anfc_nand_chip - Defines the nand chip related information
> > + * @node: Used to store NAND chips into a list.
> > + * @chip: NAND chip information structure.
> > + * @strength: Bch or Hamming mode enable/disable.
>
> The name is still confusing. BTW, can't you deduce Hamming vs BCH from the
> strength?
> Hamming strength is 1, while BCH strengths seem to start at 4.
Yes we can deduce. I will try that.
.
>
> > + * @ecc_strength: Ecc strength 4.8/12/16.
>
> ^/
>
> > + * @eccval:Ecc config value.
> > + * @spare_caddr_cycles:Column address cycle information for spare area.
> > + * @pktsize: Packet size for read / write operation.
> > + * @csnum: chipselect number to be used.
> > + * @spktsize: Packet size in ddr mode for status operation.
> > + * @inftimeval:Data interface and timing mode information
> > + */
> > +struct anfc_nand_chip {
> > + struct list_head node;
> > + struct nand_chip chip;
> > + bool strength;
> > + u32 ecc_strength;
> > + u32 eccval;
> > + u16 spare_caddr_cycles;
> > + u32 pktsize;
> > + int csnum;
> > + u32 spktsize;
> > + u32 inftimeval;
> > +};
> > +
> > +/**
> > + * struct anfc_nand_controller - Defines the Arasan NAND flash controller
> > + * driver instance
> > + * @controller:base controller structure.
> > + * @chips: list of all nand chips attached to the ctrler.
> > + * @dev: Pointer to the device structure.
> > + * @base: Virtual address of the NAND flash device.
> > + * @clk_sys: Pointer to the system clock.
> > + * @clk_flash: Pointer to the flash clock.
> > + * @dma: Dma enable/disable.
> > + * @buf: Buffer used for read/write byte operations.
> > + * @irq: irq number
>
> You should need this field. Just get the irq num in you probe path an
> register an irq handler
> with devm_request_irq().
You mean to say, instead of putting irq in anfc_nand_controller structure,
update the code like below snippet, right?
irq = platform_get_irq(pdev, 0);
if (irq < 0) {
dev_err(dev, "failed to retrieve irq\n");
return irq;
}
devm_request_irq(>dev, irq, anfc_irq_handler, 0, "arasannfc", nfc);
>
> > + * @bufshift: Variable used for indexing buffer operation
> > + * @csnum: Chip select number currently inuse.
>
>^ in use
>
> > + * @event: Completion event for nand status events.
> > + * @status:Status of the flash device.
> > + * @prog: Used to initiate controller operations.
> > + */
> > +struct anfc_nand_controller {
> > + struct nand_controller controller;
> > + struct list_head chips;
> > + struct device *dev;
> > + void __iomem *base;
> > + struct clk *clk_sys;
> > + struct clk *clk_flash;
> > + int irq;
> > + int csnum;
> > + struct completion event;
> > + int status;
> > + u8 buf[TEMP_BUF_SIZE];
>
> Allocate this buf dynamically.
Ok
>
> > + u32 eccval;
> > +};
>
> > +static int anfc_page_write_type_exec(struct nand_chip *chip,
> > +
Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
Hi Naga,
Just a preliminary review. I still think we have problems with how you
execute NAND operations. You seem to assume that read/write operation
are always page write/read operation with a size aligned on a page
size. This is wrong, either the controller is able to execute the exact
operation that has been requested or it returns -ENOTSUPP.
On Fri, 9 Nov 2018 10:30:41 +0530
Naga Sureshkumar Relli wrote:
> +
> +/**
> + * struct anfc_nand_chip - Defines the nand chip related information
> + * @node:Used to store NAND chips into a list.
> + * @chip:NAND chip information structure.
> + * @strength:Bch or Hamming mode enable/disable.
The name is still confusing. BTW, can't you deduce Hamming vs BCH from
the strength? Hamming strength is 1, while BCH strengths seem to start
at 4.
> + * @ecc_strength:Ecc strength 4.8/12/16.
^/
> + * @eccval: Ecc config value.
> + * @spare_caddr_cycles: Column address cycle information for spare area.
> + * @pktsize: Packet size for read / write operation.
> + * @csnum: chipselect number to be used.
> + * @spktsize:Packet size in ddr mode for status operation.
> + * @inftimeval: Data interface and timing mode information
> + */
> +struct anfc_nand_chip {
> + struct list_head node;
> + struct nand_chip chip;
> + bool strength;
> + u32 ecc_strength;
> + u32 eccval;
> + u16 spare_caddr_cycles;
> + u32 pktsize;
> + int csnum;
> + u32 spktsize;
> + u32 inftimeval;
> +};
> +
> +/**
> + * struct anfc_nand_controller - Defines the Arasan NAND flash controller
> + *driver instance
> + * @controller: base controller structure.
> + * @chips: list of all nand chips attached to the ctrler.
> + * @dev: Pointer to the device structure.
> + * @base:Virtual address of the NAND flash device.
> + * @clk_sys: Pointer to the system clock.
> + * @clk_flash: Pointer to the flash clock.
> + * @dma: Dma enable/disable.
> + * @buf: Buffer used for read/write byte operations.
> + * @irq: irq number
You should need this field. Just get the irq num in you probe path an
register an irq handler with devm_request_irq().
> + * @bufshift:Variable used for indexing buffer operation
> + * @csnum: Chip select number currently inuse.
^ in use
> + * @event: Completion event for nand status events.
> + * @status: Status of the flash device.
> + * @prog:Used to initiate controller operations.
> + */
> +struct anfc_nand_controller {
> + struct nand_controller controller;
> + struct list_head chips;
> + struct device *dev;
> + void __iomem *base;
> + struct clk *clk_sys;
> + struct clk *clk_flash;
> + int irq;
> + int csnum;
> + struct completion event;
> + int status;
> + u8 buf[TEMP_BUF_SIZE];
Allocate this buf dynamically.
> + u32 eccval;
> +};
> +static int anfc_page_write_type_exec(struct nand_chip *chip,
> + const struct nand_subop *subop)
> +{
> + const struct nand_op_instr *instr;
> + struct anfc_nand_chip *achip = to_anfc_nand(chip);
> + struct anfc_nand_controller *nfc = to_anfc(chip->controller);
> + unsigned int op_id, len;
> + struct anfc_op nfc_op = {};
> + struct mtd_info *mtd = nand_to_mtd(chip);
> +
> + anfc_parse_instructions(chip, subop, _op);
> + instr = nfc_op.data_instr;
> + op_id = nfc_op.data_instr_idx;
> + anfc_prepare_cmd(chip, nfc_op.cmds[0], nfc_op.cmds[1], 1,
> + mtd->writesize, nfc_op.naddrcycles);
> + anfc_setpagecoladdr(nfc, nfc_op.row, nfc_op.col);
> + if (!nfc_op.data_instr)
> + return 0;
> +
> + len = nand_subop_get_data_len(subop, op_id);
> + anfc_write_data_op(chip, (char *)instr->ctx.data.buf.out,
^ extra white space here
and please drop the cast.
Can you please run checkpatch --strict prior to submitting patches?
> +mtd->writesize,
> +DIV_ROUND_UP(mtd->writesize, achip->pktsize),
No, that's wrong. You should use instr->ctx.data.len here, and the
DIV_ROUND_UP() thing is a bit scary. That means you might be writing
more data than requested.
> +achip->pktsize);
> +
> + return 0;
> +}
> +
> +
> +static int anfc_probe(struct platform_device *pdev)
> +{
> + struct anfc_nand_controller *nfc;
> + struct anfc_nand_chip *anand_chip;
> + struct device_node *np = pdev->dev.of_node, *child;
> + struct resource *res;
> + int err;
> +
> + nfc = devm_kzalloc(>dev, sizeof(*nfc), GFP_KERNEL);
> + if (!nfc)
> + return -ENOMEM;
> +
> +
Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
Hi Naga,
Just a preliminary review. I still think we have problems with how you
execute NAND operations. You seem to assume that read/write operation
are always page write/read operation with a size aligned on a page
size. This is wrong, either the controller is able to execute the exact
operation that has been requested or it returns -ENOTSUPP.
On Fri, 9 Nov 2018 10:30:41 +0530
Naga Sureshkumar Relli wrote:
> +
> +/**
> + * struct anfc_nand_chip - Defines the nand chip related information
> + * @node:Used to store NAND chips into a list.
> + * @chip:NAND chip information structure.
> + * @strength:Bch or Hamming mode enable/disable.
The name is still confusing. BTW, can't you deduce Hamming vs BCH from
the strength? Hamming strength is 1, while BCH strengths seem to start
at 4.
> + * @ecc_strength:Ecc strength 4.8/12/16.
^/
> + * @eccval: Ecc config value.
> + * @spare_caddr_cycles: Column address cycle information for spare area.
> + * @pktsize: Packet size for read / write operation.
> + * @csnum: chipselect number to be used.
> + * @spktsize:Packet size in ddr mode for status operation.
> + * @inftimeval: Data interface and timing mode information
> + */
> +struct anfc_nand_chip {
> + struct list_head node;
> + struct nand_chip chip;
> + bool strength;
> + u32 ecc_strength;
> + u32 eccval;
> + u16 spare_caddr_cycles;
> + u32 pktsize;
> + int csnum;
> + u32 spktsize;
> + u32 inftimeval;
> +};
> +
> +/**
> + * struct anfc_nand_controller - Defines the Arasan NAND flash controller
> + *driver instance
> + * @controller: base controller structure.
> + * @chips: list of all nand chips attached to the ctrler.
> + * @dev: Pointer to the device structure.
> + * @base:Virtual address of the NAND flash device.
> + * @clk_sys: Pointer to the system clock.
> + * @clk_flash: Pointer to the flash clock.
> + * @dma: Dma enable/disable.
> + * @buf: Buffer used for read/write byte operations.
> + * @irq: irq number
You should need this field. Just get the irq num in you probe path an
register an irq handler with devm_request_irq().
> + * @bufshift:Variable used for indexing buffer operation
> + * @csnum: Chip select number currently inuse.
^ in use
> + * @event: Completion event for nand status events.
> + * @status: Status of the flash device.
> + * @prog:Used to initiate controller operations.
> + */
> +struct anfc_nand_controller {
> + struct nand_controller controller;
> + struct list_head chips;
> + struct device *dev;
> + void __iomem *base;
> + struct clk *clk_sys;
> + struct clk *clk_flash;
> + int irq;
> + int csnum;
> + struct completion event;
> + int status;
> + u8 buf[TEMP_BUF_SIZE];
Allocate this buf dynamically.
> + u32 eccval;
> +};
> +static int anfc_page_write_type_exec(struct nand_chip *chip,
> + const struct nand_subop *subop)
> +{
> + const struct nand_op_instr *instr;
> + struct anfc_nand_chip *achip = to_anfc_nand(chip);
> + struct anfc_nand_controller *nfc = to_anfc(chip->controller);
> + unsigned int op_id, len;
> + struct anfc_op nfc_op = {};
> + struct mtd_info *mtd = nand_to_mtd(chip);
> +
> + anfc_parse_instructions(chip, subop, _op);
> + instr = nfc_op.data_instr;
> + op_id = nfc_op.data_instr_idx;
> + anfc_prepare_cmd(chip, nfc_op.cmds[0], nfc_op.cmds[1], 1,
> + mtd->writesize, nfc_op.naddrcycles);
> + anfc_setpagecoladdr(nfc, nfc_op.row, nfc_op.col);
> + if (!nfc_op.data_instr)
> + return 0;
> +
> + len = nand_subop_get_data_len(subop, op_id);
> + anfc_write_data_op(chip, (char *)instr->ctx.data.buf.out,
^ extra white space here
and please drop the cast.
Can you please run checkpatch --strict prior to submitting patches?
> +mtd->writesize,
> +DIV_ROUND_UP(mtd->writesize, achip->pktsize),
No, that's wrong. You should use instr->ctx.data.len here, and the
DIV_ROUND_UP() thing is a bit scary. That means you might be writing
more data than requested.
> +achip->pktsize);
> +
> + return 0;
> +}
> +
> +
> +static int anfc_probe(struct platform_device *pdev)
> +{
> + struct anfc_nand_controller *nfc;
> + struct anfc_nand_chip *anand_chip;
> + struct device_node *np = pdev->dev.of_node, *child;
> + struct resource *res;
> + int err;
> +
> + nfc = devm_kzalloc(>dev, sizeof(*nfc), GFP_KERNEL);
> + if (!nfc)
> + return -ENOMEM;
> +
> +
[LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
Add the basic driver for Arasan NAND Flash Controller used in Zynq UltraScale+ MPSoC. It supports HW ECC and upto 24bit correction Signed-off-by: Naga Sureshkumar Relli --- Changes in v12: - Rebased on top of 4.20 - As suggested by Boris, instead of checking the command using nfc_op.cmds[], use PROG_PGRD or PROG_PGPROG as appropriate in reads and writes. - Also use address cycles information provided by core instead of guessing it. Changes in v11: Fixed the below commits given by Boris - implemented separate hooks for each pattern - Changed EVNT_TIMEOUT_MSEC to EVENT_TIMEOUT_MSEC - Grouped register offsets with theri fields, previously there are defines at randome positions - changes cmnds to cmds and s32 to u32 - Removed unnecessary fields from struct anfc_op - Renamed bch and bchmode to strength and ecc_strength respectively - Passed nand_chip object direclty to all functions - Replace is_vmalloc_addr() with virt_addr_valid() - Use default routines for read/write_oob() - Added core support to get sdr timing mode value Changes in v10: - Implemented ->exec_op() interface. - Converted the driver to nand_scan(). Changes in v9: - Added the SPDX tags Changes in v8: - Implemented setup_data_interface hook - fixed checkpatch --strict warnings - Added anfc_config_ecc in read_page_hwecc - Fixed returning status value by reading flash status in read_byte() instead of reading previous value. Changes in v7: - Implemented Marek suggestions and comments - Corrected the acronyms those should be in caps - Modified kconfig/Make file to keep arasan entry in sorted order - Added is_vmlloc_addr check - Used ioread/write32_rep variants to avoid compilation error for intel platforms - separated PIO and DMA mode read/write functions - Minor cleanup Chnages in v6: - Addressed most of the Brian and Boris comments - Separated the nandchip from the nand controller - Removed the ecc lookup table from driver - Now use framework nand waitfunction and readoob - Fixed the compiler warning - Adapted the new frameowrk changes related to ecc and ooblayout - Disabled the clocks after the nand_reelase - Now using only one completion object - Boris suggessions like adapting cmd_ctrl and rework on read/write byte are not implemented and i will patch them later - Also check_erased_ecc_chunk for erase and check for is_vmalloc_addr will implement later once the basic driver is mainlined. Changes in v5: - Renamed the driver filei as arasan_nand.c - Fixed all comments relaqted coding style - Fixed comments related to propagating the errors - Modified the anfc_write_page_hwecc as per the write_page prototype Changes in v4: - Added support for onfi timing mode configuration - Added clock suort - Added support for multiple chipselects Changes in v3: - Removed unused variables - Avoided busy loop and used jifies based implementation - Fixed compiler warnings "right shift count >= width of type" - Removed unneeded codei and improved error reporting - Added onfi version check to ensure reading the valid address cycles Changes in v2: - Added missing of.h to avoid kbuild system report erro --- drivers/mtd/nand/raw/Kconfig |7 + drivers/mtd/nand/raw/Makefile |1 + drivers/mtd/nand/raw/arasan_nand.c | 1238 3 files changed, 1246 insertions(+) create mode 100644 drivers/mtd/nand/raw/arasan_nand.c diff --git a/drivers/mtd/nand/raw/Kconfig b/drivers/mtd/nand/raw/Kconfig index c7efc31..3f7ae73 100644 --- a/drivers/mtd/nand/raw/Kconfig +++ b/drivers/mtd/nand/raw/Kconfig @@ -541,4 +541,11 @@ config MTD_NAND_TEGRA is supported. Extra OOB bytes when using HW ECC are currently not supported. +config MTD_NAND_ARASAN + tristate "Support for Arasan Nand Flash controller" + depends on HAS_IOMEM && HAS_DMA + help + Enables the driver for the Arasan Nand Flash controller on + Zynq Ultrascale+ MPSoC. + endif # MTD_NAND diff --git a/drivers/mtd/nand/raw/Makefile b/drivers/mtd/nand/raw/Makefile index 57159b3..042d53d 100644 --- a/drivers/mtd/nand/raw/Makefile +++ b/drivers/mtd/nand/raw/Makefile @@ -56,6 +56,7 @@ obj-$(CONFIG_MTD_NAND_BRCMNAND) += brcmnand/ obj-$(CONFIG_MTD_NAND_QCOM)+= qcom_nandc.o obj-$(CONFIG_MTD_NAND_MTK) += mtk_ecc.o mtk_nand.o obj-$(CONFIG_MTD_NAND_TEGRA) += tegra_nand.o +obj-$(CONFIG_MTD_NAND_ARASAN) += arasan_nand.o nand-objs := nand_base.o nand_legacy.o nand_bbt.o nand_timings.o nand_ids.o nand-objs += nand_onfi.o diff --git a/drivers/mtd/nand/raw/arasan_nand.c b/drivers/mtd/nand/raw/arasan_nand.c new file mode 100644 index 000..b8f39c3 --- /dev/null +++ b/drivers/mtd/nand/raw/arasan_nand.c @@ -0,0 +1,1238 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Arasan NAND Flash Controller Driver + * + * Copyright (C) 2014 - 2017 Xilinx, Inc. + * Author: Punnaiah Choudary Kalluri + * Author: Naga Sureshkumar Relli + * + */
[LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
Add the basic driver for Arasan NAND Flash Controller used in Zynq UltraScale+ MPSoC. It supports HW ECC and upto 24bit correction Signed-off-by: Naga Sureshkumar Relli --- Changes in v12: - Rebased on top of 4.20 - As suggested by Boris, instead of checking the command using nfc_op.cmds[], use PROG_PGRD or PROG_PGPROG as appropriate in reads and writes. - Also use address cycles information provided by core instead of guessing it. Changes in v11: Fixed the below commits given by Boris - implemented separate hooks for each pattern - Changed EVNT_TIMEOUT_MSEC to EVENT_TIMEOUT_MSEC - Grouped register offsets with theri fields, previously there are defines at randome positions - changes cmnds to cmds and s32 to u32 - Removed unnecessary fields from struct anfc_op - Renamed bch and bchmode to strength and ecc_strength respectively - Passed nand_chip object direclty to all functions - Replace is_vmalloc_addr() with virt_addr_valid() - Use default routines for read/write_oob() - Added core support to get sdr timing mode value Changes in v10: - Implemented ->exec_op() interface. - Converted the driver to nand_scan(). Changes in v9: - Added the SPDX tags Changes in v8: - Implemented setup_data_interface hook - fixed checkpatch --strict warnings - Added anfc_config_ecc in read_page_hwecc - Fixed returning status value by reading flash status in read_byte() instead of reading previous value. Changes in v7: - Implemented Marek suggestions and comments - Corrected the acronyms those should be in caps - Modified kconfig/Make file to keep arasan entry in sorted order - Added is_vmlloc_addr check - Used ioread/write32_rep variants to avoid compilation error for intel platforms - separated PIO and DMA mode read/write functions - Minor cleanup Chnages in v6: - Addressed most of the Brian and Boris comments - Separated the nandchip from the nand controller - Removed the ecc lookup table from driver - Now use framework nand waitfunction and readoob - Fixed the compiler warning - Adapted the new frameowrk changes related to ecc and ooblayout - Disabled the clocks after the nand_reelase - Now using only one completion object - Boris suggessions like adapting cmd_ctrl and rework on read/write byte are not implemented and i will patch them later - Also check_erased_ecc_chunk for erase and check for is_vmalloc_addr will implement later once the basic driver is mainlined. Changes in v5: - Renamed the driver filei as arasan_nand.c - Fixed all comments relaqted coding style - Fixed comments related to propagating the errors - Modified the anfc_write_page_hwecc as per the write_page prototype Changes in v4: - Added support for onfi timing mode configuration - Added clock suort - Added support for multiple chipselects Changes in v3: - Removed unused variables - Avoided busy loop and used jifies based implementation - Fixed compiler warnings "right shift count >= width of type" - Removed unneeded codei and improved error reporting - Added onfi version check to ensure reading the valid address cycles Changes in v2: - Added missing of.h to avoid kbuild system report erro --- drivers/mtd/nand/raw/Kconfig |7 + drivers/mtd/nand/raw/Makefile |1 + drivers/mtd/nand/raw/arasan_nand.c | 1238 3 files changed, 1246 insertions(+) create mode 100644 drivers/mtd/nand/raw/arasan_nand.c diff --git a/drivers/mtd/nand/raw/Kconfig b/drivers/mtd/nand/raw/Kconfig index c7efc31..3f7ae73 100644 --- a/drivers/mtd/nand/raw/Kconfig +++ b/drivers/mtd/nand/raw/Kconfig @@ -541,4 +541,11 @@ config MTD_NAND_TEGRA is supported. Extra OOB bytes when using HW ECC are currently not supported. +config MTD_NAND_ARASAN + tristate "Support for Arasan Nand Flash controller" + depends on HAS_IOMEM && HAS_DMA + help + Enables the driver for the Arasan Nand Flash controller on + Zynq Ultrascale+ MPSoC. + endif # MTD_NAND diff --git a/drivers/mtd/nand/raw/Makefile b/drivers/mtd/nand/raw/Makefile index 57159b3..042d53d 100644 --- a/drivers/mtd/nand/raw/Makefile +++ b/drivers/mtd/nand/raw/Makefile @@ -56,6 +56,7 @@ obj-$(CONFIG_MTD_NAND_BRCMNAND) += brcmnand/ obj-$(CONFIG_MTD_NAND_QCOM)+= qcom_nandc.o obj-$(CONFIG_MTD_NAND_MTK) += mtk_ecc.o mtk_nand.o obj-$(CONFIG_MTD_NAND_TEGRA) += tegra_nand.o +obj-$(CONFIG_MTD_NAND_ARASAN) += arasan_nand.o nand-objs := nand_base.o nand_legacy.o nand_bbt.o nand_timings.o nand_ids.o nand-objs += nand_onfi.o diff --git a/drivers/mtd/nand/raw/arasan_nand.c b/drivers/mtd/nand/raw/arasan_nand.c new file mode 100644 index 000..b8f39c3 --- /dev/null +++ b/drivers/mtd/nand/raw/arasan_nand.c @@ -0,0 +1,1238 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Arasan NAND Flash Controller Driver + * + * Copyright (C) 2014 - 2017 Xilinx, Inc. + * Author: Punnaiah Choudary Kalluri + * Author: Naga Sureshkumar Relli + * + */
