diff mbox series

[1/2] mtd: rawnand: gpmi: Fix the driver only sense CS0 R/B issue

Message ID 20201205063004.3099-1-han.xu@nxp.com
State Superseded
Delegated to: Miquel Raynal
Headers show
Series [1/2] mtd: rawnand: gpmi: Fix the driver only sense CS0 R/B issue | expand

Commit Message

Han Xu Dec. 5, 2020, 6:30 a.m. UTC 
set the GPMI CTRL1 GANGED_RDYBUSY bit so dirver can sense the R/B signal
from all CS.

For the NAND chip MT29F64G08AFAAAWP, only the first chip detected
without the patch.

[    3.764118] nand: device found, Manufacturer ID: 0x2c, Chip ID: 0x68
[    3.770613] nand: Micron MT29F64G08AFAAAWP
[    3.774752] nand: 4096 MiB, SLC, erase size: 1024 KiB, page size: 8192, OOB size: 448
[    3.786421] Bad block table found at page 524160, version 0x01
[    3.792730] Bad block table found at page 524032, version 0x01

After applying the patch

[    3.764445] nand: device found, Manufacturer ID: 0x2c, Chip ID: 0x68
[    3.770941] nand: Micron MT29F64G08AFAAAWP
[    3.775080] nand: 4096 MiB, SLC, erase size: 1024 KiB, page size: 8192, OOB size: 448
[    3.784390] nand: 2 chips detected
[    3.790900] Bad block table found at page 524160, version 0x01
[    3.796776] Bad block table found at page 1048448, version 0x01

Fixes: 3045f8e36963 ("mtd: rawnand: gpmi: move all driver code into single file")
Signed-off-by: Han Xu <han.xu@nxp.com>
---
 drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c | 6 ++++--
 drivers/mtd/nand/raw/gpmi-nand/gpmi-regs.h | 1 +
 2 files changed, 5 insertions(+), 2 deletions(-)

Comments

Sascha Hauer Dec. 7, 2020, 9:50 a.m. UTC | #1 
On Sat, Dec 05, 2020 at 12:30:03AM -0600, Han Xu wrote:
> set the GPMI CTRL1 GANGED_RDYBUSY bit so dirver can sense the R/B signal
> from all CS.
> 
> For the NAND chip MT29F64G08AFAAAWP, only the first chip detected
> without the patch.
> 
> [    3.764118] nand: device found, Manufacturer ID: 0x2c, Chip ID: 0x68
> [    3.770613] nand: Micron MT29F64G08AFAAAWP
> [    3.774752] nand: 4096 MiB, SLC, erase size: 1024 KiB, page size: 8192, OOB size: 448
> [    3.786421] Bad block table found at page 524160, version 0x01
> [    3.792730] Bad block table found at page 524032, version 0x01
> 
> After applying the patch
> 
> [    3.764445] nand: device found, Manufacturer ID: 0x2c, Chip ID: 0x68
> [    3.770941] nand: Micron MT29F64G08AFAAAWP
> [    3.775080] nand: 4096 MiB, SLC, erase size: 1024 KiB, page size: 8192, OOB size: 448
> [    3.784390] nand: 2 chips detected
> [    3.790900] Bad block table found at page 524160, version 0x01
> [    3.796776] Bad block table found at page 1048448, version 0x01
> 
> Fixes: 3045f8e36963 ("mtd: rawnand: gpmi: move all driver code into single file")

I don't see how 3045f8e36963 changes his behaviour. Are you sure it
worked without this patch?

Sascha
Han Xu Dec. 7, 2020, 4:14 p.m. UTC | #2 
> -----Original Message-----
> From: Sascha Hauer <s.hauer@pengutronix.de>
> Sent: Monday, December 7, 2020 3:50 AM
> To: Han Xu <han.xu@nxp.com>
> Cc: miquel.raynal@bootlin.com; linux-mtd@lists.infradead.org
> Subject: [EXT] Re: [PATCH 1/2] mtd: rawnand: gpmi: Fix the driver only sense
> CS0 R/B issue
> 
> Caution: EXT Email
> 
> On Sat, Dec 05, 2020 at 12:30:03AM -0600, Han Xu wrote:
> > set the GPMI CTRL1 GANGED_RDYBUSY bit so dirver can sense the R/B
> > signal from all CS.
> >
> > For the NAND chip MT29F64G08AFAAAWP, only the first chip detected
> > without the patch.
> >
> > [    3.764118] nand: device found, Manufacturer ID: 0x2c, Chip ID: 0x68
> > [    3.770613] nand: Micron MT29F64G08AFAAAWP
> > [    3.774752] nand: 4096 MiB, SLC, erase size: 1024 KiB, page size: 8192, OOB
> size: 448
> > [    3.786421] Bad block table found at page 524160, version 0x01
> > [    3.792730] Bad block table found at page 524032, version 0x01
> >
> > After applying the patch
> >
> > [    3.764445] nand: device found, Manufacturer ID: 0x2c, Chip ID: 0x68
> > [    3.770941] nand: Micron MT29F64G08AFAAAWP
> > [    3.775080] nand: 4096 MiB, SLC, erase size: 1024 KiB, page size: 8192, OOB
> size: 448
> > [    3.784390] nand: 2 chips detected
> > [    3.790900] Bad block table found at page 524160, version 0x01
> > [    3.796776] Bad block table found at page 1048448, version 0x01
> >
> > Fixes: 3045f8e36963 ("mtd: rawnand: gpmi: move all driver code into
> > single file")
> 
> I don't see how 3045f8e36963 changes his behaviour. Are you sure it worked
> without this patch?

After several rounds files merge and code move, it's hard to find when this issue first involved, the driver still works for single CS NAND but CTRL1 GANGED_RDYBUSY must be set for multi-CS NAND chips.

> 
> Sascha
> 
> 
> --
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       |
> https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.peng
> utronix.de%2F&amp;data=04%7C01%7Chan.xu%40nxp.com%7Cfeed58dd05004a
> 8f81f208d89a957af4%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637
> 429314087020152%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJ
> QIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=ZyW3svz
> HvL1soAA%2BUi6FCe%2B8ZUm2gubwEK6P%2BO9T2%2FI%3D&amp;reserved=0
> |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
Sascha Hauer Dec. 7, 2020, 4:43 p.m. UTC | #3 
On Mon, Dec 07, 2020 at 04:14:00PM +0000, Han Xu wrote:
> 
> 
> > -----Original Message-----
> > From: Sascha Hauer <s.hauer@pengutronix.de>
> > Sent: Monday, December 7, 2020 3:50 AM
> > To: Han Xu <han.xu@nxp.com>
> > Cc: miquel.raynal@bootlin.com; linux-mtd@lists.infradead.org
> > Subject: [EXT] Re: [PATCH 1/2] mtd: rawnand: gpmi: Fix the driver only sense
> > CS0 R/B issue
> > 
> > Caution: EXT Email
> > 
> > On Sat, Dec 05, 2020 at 12:30:03AM -0600, Han Xu wrote:
> > > set the GPMI CTRL1 GANGED_RDYBUSY bit so dirver can sense the R/B
> > > signal from all CS.
> > >
> > > For the NAND chip MT29F64G08AFAAAWP, only the first chip detected
> > > without the patch.
> > >
> > > [    3.764118] nand: device found, Manufacturer ID: 0x2c, Chip ID: 0x68
> > > [    3.770613] nand: Micron MT29F64G08AFAAAWP
> > > [    3.774752] nand: 4096 MiB, SLC, erase size: 1024 KiB, page size: 8192, OOB
> > size: 448
> > > [    3.786421] Bad block table found at page 524160, version 0x01
> > > [    3.792730] Bad block table found at page 524032, version 0x01
> > >
> > > After applying the patch
> > >
> > > [    3.764445] nand: device found, Manufacturer ID: 0x2c, Chip ID: 0x68
> > > [    3.770941] nand: Micron MT29F64G08AFAAAWP
> > > [    3.775080] nand: 4096 MiB, SLC, erase size: 1024 KiB, page size: 8192, OOB
> > size: 448
> > > [    3.784390] nand: 2 chips detected
> > > [    3.790900] Bad block table found at page 524160, version 0x01
> > > [    3.796776] Bad block table found at page 1048448, version 0x01
> > >
> > > Fixes: 3045f8e36963 ("mtd: rawnand: gpmi: move all driver code into
> > > single file")
> > 
> > I don't see how 3045f8e36963 changes his behaviour. Are you sure it worked
> > without this patch?
> 
> After several rounds files merge and code move, it's hard to find when
> this issue first involved, the driver still works for single CS NAND
> but CTRL1 GANGED_RDYBUSY must be set for multi-CS NAND chips.

3045f8e36963 only re-arranges the code without any functional change, so
I strongly doubt that a commit writing a register with a newly
introduced register bit ixes it.

Please drop this "Fixes:" tag.

Sascha
Miquel Raynal Dec. 8, 2020, 9:53 p.m. UTC | #4 
Hello,

Sascha Hauer <s.hauer@pengutronix.de> wrote on Mon, 7 Dec 2020 17:43:46
+0100:

> On Mon, Dec 07, 2020 at 04:14:00PM +0000, Han Xu wrote:
> > 
> >   
> > > -----Original Message-----
> > > From: Sascha Hauer <s.hauer@pengutronix.de>
> > > Sent: Monday, December 7, 2020 3:50 AM
> > > To: Han Xu <han.xu@nxp.com>
> > > Cc: miquel.raynal@bootlin.com; linux-mtd@lists.infradead.org
> > > Subject: [EXT] Re: [PATCH 1/2] mtd: rawnand: gpmi: Fix the driver only sense
> > > CS0 R/B issue
> > > 
> > > Caution: EXT Email
> > > 
> > > On Sat, Dec 05, 2020 at 12:30:03AM -0600, Han Xu wrote:  
> > > > set the GPMI CTRL1 GANGED_RDYBUSY bit so dirver can sense the R/B
> > > > signal from all CS.
> > > >
> > > > For the NAND chip MT29F64G08AFAAAWP, only the first chip detected
> > > > without the patch.
> > > >
> > > > [    3.764118] nand: device found, Manufacturer ID: 0x2c, Chip ID: 0x68
> > > > [    3.770613] nand: Micron MT29F64G08AFAAAWP
> > > > [    3.774752] nand: 4096 MiB, SLC, erase size: 1024 KiB, page size: 8192, OOB  
> > > size: 448  
> > > > [    3.786421] Bad block table found at page 524160, version 0x01
> > > > [    3.792730] Bad block table found at page 524032, version 0x01
> > > >
> > > > After applying the patch
> > > >
> > > > [    3.764445] nand: device found, Manufacturer ID: 0x2c, Chip ID: 0x68
> > > > [    3.770941] nand: Micron MT29F64G08AFAAAWP
> > > > [    3.775080] nand: 4096 MiB, SLC, erase size: 1024 KiB, page size: 8192, OOB  
> > > size: 448  
> > > > [    3.784390] nand: 2 chips detected
> > > > [    3.790900] Bad block table found at page 524160, version 0x01
> > > > [    3.796776] Bad block table found at page 1048448, version 0x01
> > > >
> > > > Fixes: 3045f8e36963 ("mtd: rawnand: gpmi: move all driver code into
> > > > single file")  
> > > 
> > > I don't see how 3045f8e36963 changes his behaviour. Are you sure it worked
> > > without this patch?  
> > 
> > After several rounds files merge and code move, it's hard to find when
> > this issue first involved, the driver still works for single CS NAND
> > but CTRL1 GANGED_RDYBUSY must be set for multi-CS NAND chips.  
> 
> 3045f8e36963 only re-arranges the code without any functional change, so
> I strongly doubt that a commit writing a register with a newly
> introduced register bit ixes it.
> 
> Please drop this "Fixes:" tag.

I will drop the tag when applying.

> 
> Sascha
> 

Thanks,
Miquèl
diff mbox series

Patch

diff --git a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
index b5f46f214a58..793a8e27ce66 100644
--- a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
+++ b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
@@ -181,9 +181,11 @@  static int gpmi_init(struct gpmi_nand_data *this)
 
 	/*
 	 * Decouple the chip select from dma channel. We use dma0 for all
-	 * the chips.
+	 * the chips, force all NAND RDY_BUSY inputs to be sourced from
+	 * RDY_BUSY0.
 	 */
-	writel(BM_GPMI_CTRL1_DECOUPLE_CS, r->gpmi_regs + HW_GPMI_CTRL1_SET);
+	writel(BM_GPMI_CTRL1_DECOUPLE_CS | BM_GPMI_CTRL1_GANGED_RDYBUSY,
+	       r->gpmi_regs + HW_GPMI_CTRL1_SET);
 
 err_out:
 	pm_runtime_mark_last_busy(this->dev);
diff --git a/drivers/mtd/nand/raw/gpmi-nand/gpmi-regs.h b/drivers/mtd/nand/raw/gpmi-nand/gpmi-regs.h
index f5e4f26c34da..fc31fd084dcf 100644
--- a/drivers/mtd/nand/raw/gpmi-nand/gpmi-regs.h
+++ b/drivers/mtd/nand/raw/gpmi-nand/gpmi-regs.h
@@ -107,6 +107,7 @@ 
 #define BV_GPMI_CTRL1_WRN_DLY_SEL_7_TO_12NS		0x2
 #define BV_GPMI_CTRL1_WRN_DLY_SEL_NO_DELAY		0x3
 
+#define BM_GPMI_CTRL1_GANGED_RDYBUSY			(1 << 19)
 #define BM_GPMI_CTRL1_BCH_MODE				(1 << 18)
 
 #define BP_GPMI_CTRL1_DLL_ENABLE			17