[PATCH v11 04/10] mtd: nand: omap: use DT specified bus-width only for scanning NAND device

[Pekon Gupta]

This patch:
- calls nand_scan_ident() using bus-width as passed by DT
- removes double calls to nand_scan_ident(), incase first call fails
  then omap_nand_probe just returns error.

Signed-off-by: Pekon Gupta pe...@ti.com
---
 drivers/mtd/nand/omap2.c | 21 +
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
index 5596368..f464321 100644
--- a/drivers/mtd/nand/omap2.c
+++ b/drivers/mtd/nand/omap2.c
@@ -1856,7 +1856,6 @@ static int omap_nand_probe(struct platform_device *pdev)
mtd-name   = dev_name(pdev-dev);
mtd-owner  = THIS_MODULE;
nand_chip   = info-nand;
-   nand_chip-options  = pdata-devsize;
nand_chip-options  |= NAND_SKIP_BBTSCAN;
 #ifdef CONFIG_MTD_NAND_OMAP_BCH
info-of_node   = pdata-of_node;
@@ -1904,6 +1903,15 @@ static int omap_nand_probe(struct platform_device *pdev)
nand_chip-chip_delay = 50;
}
 
+   /* scan NAND device connected to chip controller */
+   nand_chip-options |= pdata-devsize  NAND_BUSWIDTH_16;
+   if (nand_scan_ident(mtd, 1, NULL)) {
+   pr_err(nand device scan failed, may be bus-width mismatch\n);
+   err = -ENXIO;
+   goto out_release_mem_region;
+   }
+
+   /* re-populate low-level callbacks based on xfer modes */
switch (pdata-xfer_type) {
case NAND_OMAP_PREFETCH_POLLED:
nand_chip-read_buf   = omap_read_buf_pref;
@@ -2011,17 +2019,6 @@ static int omap_nand_probe(struct platform_device *pdev)
}
}
 
-   /* DIP switches on some boards change between 8 and 16 bit
-* bus widths for flash.  Try the other width if the first try fails.
-*/
-   if (nand_scan_ident(mtd, 1, NULL)) {
-   nand_chip-options ^= NAND_BUSWIDTH_16;
-   if (nand_scan_ident(mtd, 1, NULL)) {
-   err = -ENXIO;
-   goto out_release_mem_region;
-   }
-   }
-
/* rom code layout */
if (pdata-ecc_opt == OMAP_ECC_HAM1_CODE_HW) {
 
-- 
1.8.1

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v11 04/10] mtd: nand: omap: use DT specified bus-width only for scanning NAND device

[Ezequiel Garcia]

On Thu, Oct 24, 2013 at 06:20:20PM +0530, Pekon Gupta wrote:
 This patch:
 - calls nand_scan_ident() using bus-width as passed by DT
 - removes double calls to nand_scan_ident(), incase first call fails
   then omap_nand_probe just returns error.
 
 Signed-off-by: Pekon Gupta pe...@ti.com
 ---
  drivers/mtd/nand/omap2.c | 21 +
  1 file changed, 9 insertions(+), 12 deletions(-)
 
 diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
 index 5596368..f464321 100644
 --- a/drivers/mtd/nand/omap2.c
 +++ b/drivers/mtd/nand/omap2.c
 @@ -1856,7 +1856,6 @@ static int omap_nand_probe(struct platform_device *pdev)
   mtd-name   = dev_name(pdev-dev);
   mtd-owner  = THIS_MODULE;
   nand_chip   = info-nand;
 - nand_chip-options  = pdata-devsize;
   nand_chip-options  |= NAND_SKIP_BBTSCAN;
  #ifdef CONFIG_MTD_NAND_OMAP_BCH
   info-of_node   = pdata-of_node;
 @@ -1904,6 +1903,15 @@ static int omap_nand_probe(struct platform_device 
 *pdev)
   nand_chip-chip_delay = 50;
   }
  
 + /* scan NAND device connected to chip controller */
 + nand_chip-options |= pdata-devsize  NAND_BUSWIDTH_16;

Hm.. this only works if the device is listed in nand_flash_ids[] array,
so that ONFI detection is not used. To make ONFI detection work I think you
need to do as Brian suggested and use NAND_BUSWIDTH_AUTO.

(Odd: why is there no current user of that auto-width option?)

Anyway, I really think we should fix this now and independently
of the evolution of this ECC DT binding discussion.
That way you can keep sending a smaller ECC DT binding patchset and
make reviewers focus on what's really important in each case.

I have a few fixes (based on your work) and I'll send them now, after
I complete the tests. We can continue our discussion there.
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v11 04/10] mtd: nand: omap: use DT specified bus-width only for scanning NAND device

[Brian Norris]

On Thu, Oct 24, 2013 at 06:27:15PM -0300, Ezequiel Garcia wrote:
 On Thu, Oct 24, 2013 at 06:20:20PM +0530, Pekon Gupta wrote:
  This patch:
  - calls nand_scan_ident() using bus-width as passed by DT
  - removes double calls to nand_scan_ident(), incase first call fails
then omap_nand_probe just returns error.
  
  Signed-off-by: Pekon Gupta pe...@ti.com
  ---
   drivers/mtd/nand/omap2.c | 21 +
   1 file changed, 9 insertions(+), 12 deletions(-)
  
  diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
  index 5596368..f464321 100644
  --- a/drivers/mtd/nand/omap2.c
  +++ b/drivers/mtd/nand/omap2.c
  @@ -1856,7 +1856,6 @@ static int omap_nand_probe(struct platform_device 
  *pdev)
  mtd-name   = dev_name(pdev-dev);
  mtd-owner  = THIS_MODULE;
  nand_chip   = info-nand;
  -   nand_chip-options  = pdata-devsize;
  nand_chip-options  |= NAND_SKIP_BBTSCAN;
   #ifdef CONFIG_MTD_NAND_OMAP_BCH
  info-of_node   = pdata-of_node;
  @@ -1904,6 +1903,15 @@ static int omap_nand_probe(struct platform_device 
  *pdev)
  nand_chip-chip_delay = 50;
  }
   
  +   /* scan NAND device connected to chip controller */
  +   nand_chip-options |= pdata-devsize  NAND_BUSWIDTH_16;
 
 Hm.. this only works if the device is listed in nand_flash_ids[] array,
 so that ONFI detection is not used.

But this is no more broken than it used to be, no? I mean, you would
never properly detect an x16 ONFI flash with the old
double-nand_scan_ident() method, right?

 To make ONFI detection work I think you
 need to do as Brian suggested and use NAND_BUSWIDTH_AUTO.

I think that is the correct way forward. But Pekon seems to think that
will require more invasive changes to the GPMC code. But I'm not sure
why.

 (Odd: why is there no current user of that auto-width option?)

Hmm, I could have sworn somebody was using that... I know there was some
pending work on using it for GPIO NAND, but Alexander Shiyan never
followed up on the latest comments. It also seems like the original
author (Matthieu Castet) was working on OMAP support about a year ago,
but things stalled when there wasn't proper mainline support for much of
it:

  http://thread.gmane.org/gmane.linux.ports.arm.omap/88550/focus=44770

Personally, I've only ever used x8 NAND, so I don't have much to go on
here.

 Anyway, I really think we should fix this now and independently
 of the evolution of this ECC DT binding discussion.
 That way you can keep sending a smaller ECC DT binding patchset and
 make reviewers focus on what's really important in each case.

AFAIK, the ECC DT bindings were all approved, and the code looked OK to
my knowledge, except for this single patch. I had recommended either its
total removal or its simplification (i.e., this current patch).

I will be taking a last look and queueing this series up soon, I
believe.

 I have a few fixes (based on your work) and I'll send them now, after
 I complete the tests. We can continue our discussion there.

I'll take a look at those soon.

So am I to understand you have hardware for testing Pekon's work now,
Ezequiel? That will be great if we can have better Reviewed-by/Tested-by
results.

Brian
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v11 04/10] mtd: nand: omap: use DT specified bus-width only for scanning NAND device

[Ezequiel Garcia]

On Thu, Oct 24, 2013 at 03:43:00PM -0700, Brian Norris wrote:
 On Thu, Oct 24, 2013 at 06:27:15PM -0300, Ezequiel Garcia wrote:
  On Thu, Oct 24, 2013 at 06:20:20PM +0530, Pekon Gupta wrote:
   This patch:
   - calls nand_scan_ident() using bus-width as passed by DT
   - removes double calls to nand_scan_ident(), incase first call fails
 then omap_nand_probe just returns error.
   
   Signed-off-by: Pekon Gupta pe...@ti.com
   ---
drivers/mtd/nand/omap2.c | 21 +
1 file changed, 9 insertions(+), 12 deletions(-)
   
   diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
   index 5596368..f464321 100644
   --- a/drivers/mtd/nand/omap2.c
   +++ b/drivers/mtd/nand/omap2.c
   @@ -1856,7 +1856,6 @@ static int omap_nand_probe(struct platform_device 
   *pdev)
 mtd-name   = dev_name(pdev-dev);
 mtd-owner  = THIS_MODULE;
 nand_chip   = info-nand;
   - nand_chip-options  = pdata-devsize;
 nand_chip-options  |= NAND_SKIP_BBTSCAN;
#ifdef CONFIG_MTD_NAND_OMAP_BCH
 info-of_node   = pdata-of_node;
   @@ -1904,6 +1903,15 @@ static int omap_nand_probe(struct platform_device 
   *pdev)
 nand_chip-chip_delay = 50;
 }

   + /* scan NAND device connected to chip controller */
   + nand_chip-options |= pdata-devsize  NAND_BUSWIDTH_16;
  
  Hm.. this only works if the device is listed in nand_flash_ids[] array,
  so that ONFI detection is not used.
 
 But this is no more broken than it used to be, no? I mean, you would
 never properly detect an x16 ONFI flash with the old
 double-nand_scan_ident() method, right?
 

That's right. But the issue is not really fixed either.

  To make ONFI detection work I think you
  need to do as Brian suggested and use NAND_BUSWIDTH_AUTO.
 
 I think that is the correct way forward. But Pekon seems to think that
 will require more invasive changes to the GPMC code. But I'm not sure
 why.
 

Hm... not sure. AFAIK, the GPMC should be *already* configured prior to the
NAND driver being probed.

  (Odd: why is there no current user of that auto-width option?)
 
 Hmm, I could have sworn somebody was using that... I know there was some
 pending work on using it for GPIO NAND, but Alexander Shiyan never
 followed up on the latest comments. It also seems like the original
 author (Matthieu Castet) was working on OMAP support about a year ago,
 but things stalled when there wasn't proper mainline support for much of
 it:
 
   http://thread.gmane.org/gmane.linux.ports.arm.omap/88550/focus=44770
 
 Personally, I've only ever used x8 NAND, so I don't have much to go on
 here.
 
  Anyway, I really think we should fix this now and independently
  of the evolution of this ECC DT binding discussion.
  That way you can keep sending a smaller ECC DT binding patchset and
  make reviewers focus on what's really important in each case.
 
 AFAIK, the ECC DT bindings were all approved, and the code looked OK to
 my knowledge, except for this single patch. I had recommended either its
 total removal or its simplification (i.e., this current patch).
 

FWIW, I'm in favor of *completely* dropping whatever doesn't belong
to the ECC DT binding.

 I will be taking a last look and queueing this series up soon, I
 believe.
 
  I have a few fixes (based on your work) and I'll send them now, after
  I complete the tests. We can continue our discussion there.
 
 I'll take a look at those soon.
 

Ok, cool.

 So am I to understand you have hardware for testing Pekon's work now,
 Ezequiel? That will be great if we can have better Reviewed-by/Tested-by
 results.
 

Yup, I gave it quick test actually, but nothing deep. Let me test some
more maybe later today/tomorrow. I just wanted to sort this out first.
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html