Re: [PATCH 0/1] Fix OMAP2 NAND ONFI device detection

2013-11-30 Thread Brian Norris 
Hi Ezequiel,

Dragging up an old piece of the conversation, but I think this
highlights some of the difficulty we're still having. Perhaps I should
have headed this off a month ago...

On Wed, Oct 30, 2013 at 08:19:57PM -0300, Ezequiel Garcia wrote:
 On Wed, Oct 30, 2013 at 09:18:53PM +, Gupta, Pekon wrote:
   
   I'm not sure, of course, but I don't see why not. It's more likely to
   break for x16 than it is for x8.
   
  Another question here is ..
  The above patch assumes that user has configured GPMC bus-width
  correctly, so if user is already providing GPMC bus-width information
  via DT, then do we really need to detect NAND device bus-width again
  using NAND_BUSWIDTH_AUTO ?
 
 
 Hm.. I think you're forgetting the original motivation for this patch,
 which was initially explained by you :-) The problem is ONFI doesn't work
 in 16-bit mode.

So fix our ONFI detection code! Uwe's patch has helped reinforce what I
believe (but can't test yet, as I have no x16 hardware): that we can
*fix* the ONFI code so that it doesn't matter what bus width mode we
are in, but we can *always* identify the lower 8 bits of the bus.

 Let me clarify. Since GPMC and NAND widths *must* match (otherwise, it
 wouldn't make much sense, right?) we don't really need to autodetect the
 NAND width.

The rest of your argument should be trimmed here. Your following
comments are seemingly a hack to get around the fact that we don't
support ONFI correctly. But I think we should take a look at Uwe's
approach before going this far on a hack.

 However, since ONFI doesn't work in 16-bit mode, we would have to do
 something like this (untested pseudo code):
 
   nand_scan_ident(...);
 
   if (platform_data-devsize == 16)
   nand_chip-options |= NAND_BUSWIDTH_16;
 
 And in some cases we might even get away with such solution. However,
 we can't guarantee nand_scan_ident() doesn't need to swith to 16-bit mode
 inside to perform other commands (maybe some ONFI extended parameter page).

(ONFI extended parameter page is only on the lower 8 bits too. So no
16-bit mode there.)

 I must admit I'm a bit puzzled by seeing most drivers setting 16-bit
 before calling nand_scan_ident(). I guess none of them relies on ONFI?

I'm fairly confident that most of those drivers have never been used
with a 16-bit bus width since the introduction of ONFI. I believe x16 is
much less common than x8, and there are probably several drivers that
get little or no use anyway. I think the closest we got to testing
results was when a developer complained about this line in the ONFI
code:

WARN_ON(chip-options  NAND_BUSWIDTH_16);

resulting in commit 0ce82b7f7b7373b16ecf7b5725e21e2975204500. (It's
notable Paul was using similar hardware to you and Pekon...)

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 0/1] Fix OMAP2 NAND ONFI device detection

2013-11-06 Thread Gupta, Pekon 
 From: Ezequiel Garcia [mailto:ezequiel.gar...@free-electrons.com]
  On Wed, Oct 30, 2013 at 09:18:53PM +, Gupta, Pekon wrote:
  
   I'm not sure, of course, but I don't see why not. It's more likely to
   break for x16 than it is for x8.
  
  Another question here is ..
  The above patch assumes that user has configured GPMC bus-width
  correctly, so if user is already providing GPMC bus-width information
  via DT, then do we really need to detect NAND device bus-width again
  using NAND_BUSWIDTH_AUTO ?
 
 
 Hm.. I think you're forgetting the original motivation for this patch,
 which was initially explained by you :-) The problem is ONFI doesn't work
 in 16-bit mode.
 
 Let me clarify. Since GPMC and NAND widths *must* match (otherwise, it
 wouldn't make much sense, right?) we don't really need to autodetect the
 NAND width.
 
 However, since ONFI doesn't work in 16-bit mode, we would have to do
 something like this (untested pseudo code):
 
   nand_scan_ident(...);
 
   if (platform_data-devsize == 16)
   nand_chip-options |= NAND_BUSWIDTH_16;
 
 And in some cases we might even get away with such solution. However,
 we can't guarantee nand_scan_ident() doesn't need to swith to 16-bit mode
 inside to perform other commands (maybe some ONFI extended parameter
 page).
 
 Therefore, and given the width can be autodetected in any case (array
 or ONFI based carry such information) it's much cleaner to simply do:
 
   nand_chip-options |= NAND_BUSWIDTH_AUTOMAGIC;
   nand_scan_ident(...);
 
 See? Much cleaner, no?
 
but still dependent on DT property for GPMC configurations...
I preferred NAND bus-width detection to be completely independent
of 'any' DT property.


 And remember: the fact that we must know the device width to configure
 the GPMC correctly (which being a hardware parameter belongs to the DT)
 is OMAP specific. Other SoCs might not have such memory controller
 and thus won't need such knowledge before hand, although that sounds
 unlikely to me.
 
 The outcome of this discussion seems to be that no driver should ever
 need to specify the 'nand-bus-width' DT property, as Brian
 previously suggested, right?
 
Right.. And this is where I'm suggesting that in-order to completely
eliminate the dependency on 'nand-bus-width' DT property, you need
to also update GPMC driver, so that its registers can be re-configured with
correct NAND bus-width after nand_scan_ident().


 I must admit I'm a bit puzzled by seeing most drivers setting 16-bit
 before calling nand_scan_ident(). I guess none of them relies on ONFI?
 
May be, but going forward we should make NAND_BUSWIDTH_AUTO
as a mandatory option, because most of the NAND devices have adopted
to ONFI, and remaining legacy (if any) are part of nand_flash_id[] table.
So either way the device would get detected, if you start with x8
(lowest minimum capability bus-width). Thus there is no need for user
to specify NAND device bus-width property via DT.


with regards, pekon

Re: [PATCH 0/1] Fix OMAP2 NAND ONFI device detection

2013-11-06 Thread Ezequiel Garcia 
On Wed, Nov 06, 2013 at 08:54:56PM +, Gupta, Pekon wrote:
  From: Ezequiel Garcia [mailto:ezequiel.gar...@free-electrons.com]
   On Wed, Oct 30, 2013 at 09:18:53PM +, Gupta, Pekon wrote:
   
I'm not sure, of course, but I don't see why not. It's more likely to
break for x16 than it is for x8.
   
   Another question here is ..
   The above patch assumes that user has configured GPMC bus-width
   correctly, so if user is already providing GPMC bus-width information
   via DT, then do we really need to detect NAND device bus-width again
   using NAND_BUSWIDTH_AUTO ?
  
  
  Hm.. I think you're forgetting the original motivation for this patch,
  which was initially explained by you :-) The problem is ONFI doesn't work
  in 16-bit mode.
  
  Let me clarify. Since GPMC and NAND widths *must* match (otherwise, it
  wouldn't make much sense, right?) we don't really need to autodetect the
  NAND width.
  
  However, since ONFI doesn't work in 16-bit mode, we would have to do
  something like this (untested pseudo code):
  
nand_scan_ident(...);
  
if (platform_data-devsize == 16)
  nand_chip-options |= NAND_BUSWIDTH_16;
  
  And in some cases we might even get away with such solution. However,
  we can't guarantee nand_scan_ident() doesn't need to swith to 16-bit mode
  inside to perform other commands (maybe some ONFI extended parameter
  page).
  
  Therefore, and given the width can be autodetected in any case (array
  or ONFI based carry such information) it's much cleaner to simply do:
  
nand_chip-options |= NAND_BUSWIDTH_AUTOMAGIC;
nand_scan_ident(...);
  
  See? Much cleaner, no?
  
 but still dependent on DT property for GPMC configurations...
 I preferred NAND bus-width detection to be completely independent
 of 'any' DT property.
 

I think we're still mixing GPMC and NAND, and I don't think it's sane.

  And remember: the fact that we must know the device width to configure
  the GPMC correctly (which being a hardware parameter belongs to the DT)
  is OMAP specific. Other SoCs might not have such memory controller
  and thus won't need such knowledge before hand, although that sounds
  unlikely to me.
  
  The outcome of this discussion seems to be that no driver should ever
  need to specify the 'nand-bus-width' DT property, as Brian
  previously suggested, right?
  
 Right.. And this is where I'm suggesting that in-order to completely
 eliminate the dependency on 'nand-bus-width' DT property, you need
 to also update GPMC driver, so that its registers can be re-configured with
 correct NAND bus-width after nand_scan_ident().
 


Well, as I said: I don't think there's any technical reason why you
can't just export a GPMC function to re-configure it from the NAND
driver. I just think it's ugly, and not useful.

So, I think you (or me, or anybody else) can push an RFC/PATCH to see
how ugly would that really be. Maybe it's not so bad.

But: on the other hand, I'd really like you to convince me as to
why is it so bad to require the DTB to have the proper GPMC bus width.

Once again:
1. the NAND devices aren't hot-pluggable
2. the user (who is actually an engineer, not some regular dummy user)
knows perfectly well the width of the device.

What's the problem with describing the hardware in the DT and saving us
lots of runtime re-configuration trouble?
-- 
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 0/1] Fix OMAP2 NAND ONFI device detection

2013-11-06 Thread Gupta, Pekon 
 From: Ezequiel Garcia
[...]

 But: on the other hand, I'd really like you to convince me as to
 why is it so bad to require the DTB to have the proper GPMC bus width.
 
No its not at all bad, all I want is either of the one way (not mixture of 
both).
- Either depend on DT completely (which is current case for all drivers)
- OR depend on ONFI and nand_flash_id[] for bus-width detection.


 Once again:
 1. the NAND devices aren't hot-pluggable
 2. the user (who is actually an engineer, not some regular dummy user)
 knows perfectly well the width of the device.
 
 What's the problem with describing the hardware in the DT and saving us
 lots of runtime re-configuration trouble?

I agree with both your arguments above.
So shouldn't we kill NAND_BUSWIDTH_AUTO ?
And probably therefore  NAND_BUSWIDTH_AUTO isn't that popular.

Now what remains is ONFI probe, which should always happen in x8 mode.
So for that below patch should be sufficient ..

--
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index ec1db1e..d1220fb 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -2942,14 +2942,8 @@ static int nand_flash_detect_onfi(struct mtd_info *mtd, 
struct nand_chip *chip,
chip-read_byte(mtd) != 'F' || chip-read_byte(mtd) != 'I')
return 0;

-   /*
-* ONFI must be probed in 8-bit mode or with NAND_BUSWIDTH_AUTO, not
-* with NAND_BUSWIDTH_16
-*/
-   if (chip-options  NAND_BUSWIDTH_16) {
-   pr_err(ONFI cannot be probed in 16-bit mode; aborting\n);
-   return 0;
-   }
+   /* ONFI must be probed in 8-bit mode only */
+   nand_set_defaults(chip, 0);

chip-cmdfunc(mtd, NAND_CMD_PARAM, 0, -1);
for (i = 0; i  3; i++) {
@@ -2962,7 +2956,7 @@ static int nand_flash_detect_onfi(struct mtd_info *mtd, 
struct nand_chip *chip,

if (i == 3) {
pr_err(Could not find valid ONFI parameter page; aborting\n);
-   return 0;
+   goto return_error;
}

/* Check version */
@@ -2980,7 +2974,7 @@ static int nand_flash_detect_onfi(struct mtd_info *mtd, 
struct nand_chip *chip,

if (!chip-onfi_version) {
pr_info(%s: unsupported ONFI version: %d\n, __func__, val);
-   return 0;
+   goto return_error;
}

sanitize_string(p-manufacturer, sizeof(p-manufacturer));
@@ -3033,6 +3027,12 @@ static int nand_flash_detect_onfi(struct mtd_info *mtd, 
struct nand_chip *chip,
}

return 1;
+
+return_error:
+   /* revert original bus-width */
+   nand_set_defaults( chip-options  NAND_BUSWIDTH_16);
+   return 0;
+
 }

 /*
- 


with regards, pekon

Re: [PATCH 0/1] Fix OMAP2 NAND ONFI device detection

2013-11-06 Thread Ezequiel Garcia 
On Wed, Nov 06, 2013 at 10:00:09PM +, Gupta, Pekon wrote:
  From: Ezequiel Garcia
 [...]
 
  But: on the other hand, I'd really like you to convince me as to
  why is it so bad to require the DTB to have the proper GPMC bus width.
  
 No its not at all bad, all I want is either of the one way (not mixture of 
 both).
 - Either depend on DT completely (which is current case for all drivers)
 - OR depend on ONFI and nand_flash_id[] for bus-width detection.
 
 
  Once again:
  1. the NAND devices aren't hot-pluggable
  2. the user (who is actually an engineer, not some regular dummy user)
  knows perfectly well the width of the device.
  
  What's the problem with describing the hardware in the DT and saving us
  lots of runtime re-configuration trouble?
 
 I agree with both your arguments above.
 So shouldn't we kill NAND_BUSWIDTH_AUTO ?
 And probably therefore  NAND_BUSWIDTH_AUTO isn't that popular.
 
 Now what remains is ONFI probe, which should always happen in x8 mode.
 So for that below patch should be sufficient ..
 

Hm.. that might work. Maybe you should submit this as RFC/PATCH to catch
the attention of some more people. And we can keep discussing on this
new idea...

I should get an 8-bit module for the BBB, so this means I'll be able
to run 8-bit and 16-bit tests.

 --
 diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
 index ec1db1e..d1220fb 100644
 --- a/drivers/mtd/nand/nand_base.c
 +++ b/drivers/mtd/nand/nand_base.c
 @@ -2942,14 +2942,8 @@ static int nand_flash_detect_onfi(struct mtd_info 
 *mtd, struct nand_chip *chip,
 chip-read_byte(mtd) != 'F' || chip-read_byte(mtd) != 'I')
 return 0;
 
 -   /*
 -* ONFI must be probed in 8-bit mode or with NAND_BUSWIDTH_AUTO, not
 -* with NAND_BUSWIDTH_16
 -*/
 -   if (chip-options  NAND_BUSWIDTH_16) {
 -   pr_err(ONFI cannot be probed in 16-bit mode; aborting\n);
 -   return 0;
 -   }
 +   /* ONFI must be probed in 8-bit mode only */
 +   nand_set_defaults(chip, 0);
 
 chip-cmdfunc(mtd, NAND_CMD_PARAM, 0, -1);
 for (i = 0; i  3; i++) {
 @@ -2962,7 +2956,7 @@ static int nand_flash_detect_onfi(struct mtd_info *mtd, 
 struct nand_chip *chip,
 
 if (i == 3) {
 pr_err(Could not find valid ONFI parameter page; 
 aborting\n);
 -   return 0;
 +   goto return_error;
 }
 
 /* Check version */
 @@ -2980,7 +2974,7 @@ static int nand_flash_detect_onfi(struct mtd_info *mtd, 
 struct nand_chip *chip,
 
 if (!chip-onfi_version) {
 pr_info(%s: unsupported ONFI version: %d\n, __func__, val);
 -   return 0;
 +   goto return_error;
 }
 
 sanitize_string(p-manufacturer, sizeof(p-manufacturer));
 @@ -3033,6 +3027,12 @@ static int nand_flash_detect_onfi(struct mtd_info 
 *mtd, struct nand_chip *chip,
 }
 
 return 1;
 +
 +return_error:
 +   /* revert original bus-width */
 +   nand_set_defaults( chip-options  NAND_BUSWIDTH_16);
 +   return 0;
 +
  }
 
  /*
 - 
 
 
 with regards, pekon

-- 
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 0/1] Fix OMAP2 NAND ONFI device detection

2013-10-30 Thread Brian Norris 
Hi,

On Wed, Oct 30, 2013 at 06:53:07AM -0300, Ezequiel Garcia wrote:
 As it was discussed recently in the mailing list, the omap2-nand driver 
 currently
 has an issue preventing proper ONFI detection of 16-bit devices (other drivers
 may suffer from this same issue).

First of all, thanks for clearly separating this discussion to its own
patch. It is an interesting issue by itself (without tying it up with
other driver-specific issues), and I think it has something useful to
teach other drivers, which may mostly be dodging issues with x16
devices.

 In an attempt to address such issue, this patch uses NAND_BUSWIDTH_AUTO
 (actually discarding any DT property) and leaves the bus width detection
 to the NAND core code.
 
 This has been tested in a Beaglebone black (AM335x) board with a 16-bit Micron
 NAND device, both ONFI and array-based detections work fine:
 
 [1.560945] omap-gpmc 5000.gpmc: GPMC revision 6.0
 [1.569328] NAND device: Manufacturer ID: 0x2c, Chip ID: 0xcc (Micron 
 MT29F4G16ABADAH4)
 [1.577853] NAND device: 512MiB, SLC, page size: 2048, OOB size: 64
 [1.584448] nand: using OMAP_ECC_BCH8_CODE_HW ECC scheme
 [1.591772] 8 ofpart partitions found on MTD device omap2-nand.0
 [1.598171] Creating 8 MTD partitions on omap2-nand.0:
 [1.603797] 0x-0x0002 : SPL1
 [1.612926] 0x0002-0x0004 : SPL2
 [1.620112] 0x0004-0x0006 : SPL3
 [1.627192] 0x0006-0x0008 : SPL4
 [1.634205] 0x0008-0x0026 : U-boot
 [1.642708] 0x0026-0x0028 : environment
 [1.650410] 0x0028-0x0078 : Kernel
 [1.661828] 0x0078-0x1000 : File-System
 
 Also, a quick run of nandtest ends successfully:
 
 # nandtest /dev/mtd6
 ECC corrections: 0
 ECC failures   : 0
 Bad blocks : 0
 BBT blocks : 0
 004e: checking...
 Finished pass 1 successfully

I suspected this was the case (in agreement with your comments on
previous threads): that BUSWIDTH_AUTO will discover the correct buswidth
*AND* will handle initial ONFI requests correctly (i.e., initially in x8
mode, then switching to x16 callbacks as appropriate).

I do have one curiosity here: omap2.c looks like it's essentially
defaulting to the NAND_OMAP_POLLED callbacks during nand_scan_ident(),
since omap2.c only initializes the read_buf callback after
nand_scan_ident(). Is this ever a problem? Or should omap2.c be
initializing its callbacks both before and after nand_scan_ident()
(similar to how nand_base calls nand_set_defaults() twice for the AUTO
case)?

What value do you use for ti,nand-xfer-type in your BeagleBone device
tree? Is the xfer type a hardware requirement, or just a software
configuration? IOW, does the hardware care if I simply use POLLED, even
temporarily? (A potential side issue: does ti,nand-xfer-type even
belong in the device tree, if it is not actually a hardware property?)

 This time I've decided to submit this patch alone, so we can focus
 the discussion on this issue. The other cleanups can wait.
 
 AFAICS, the remaining questions are:
 
  1. Does this work in the 8-bit case?
  (I'm not able to test such case for lack of hardware)

I'm not sure, of course, but I don't see why not. It's more likely to
break for x16 than it is for x8.

  2. Do we want to re-configure the GPMC one the NAND core detects the
 device bus width?

I'm not quite sure here, as I don't know if I know enough about the GPMC
to give an educated response here. Additionally, I'm not quite sure how
re-configurable GPMC really is. It seems like we would be restricted
physically if GPMC is hooked up as x8 for an x16 NAND (there are not
enough pins connected). So even if we detect the NAND, it cannot
function. I'm not sure about the reverse.

Now, this returns me to my rejection of Pekon's patch here:

  http://lists.infradead.org/pipermail/linux-mtd/2013-October/049154.html

(This patch addresses the question of checking the GPMC configuration,
not correcting it.)

I'll admit that I was underinformed regarding the need for this type of
patch. Since that time, I am less sure of my criticism. But really, I
just have more questions now.

Does the GPMC intrinsically (physically; according to its pin
configuration) restrict an attached device (e.g., NAND) to use x16 or
x8? Or does it simply specify a maximum data width that is wired up?
(I'm presuming that you could wire an x8 device to an x16 interface and
just leave the upper 8 unconnected...) Or is there some third option?

The DT entry says:

  gpmc,device-width Total width of device(s) connected to a GPMC
chip-select in bytes. The GPMC supports 8-bit
and 16-bit devices and so this property must be
1 or 2.

So, this *sounds* like it specifies the exact width. But as I read it,
this property could more optimally be specified as the *maximum*
width...

 Regarding this last

RE: [PATCH 0/1] Fix OMAP2 NAND ONFI device detection

2013-10-30 Thread Gupta, Pekon 
Hi Brian, Ezequiel Garcia,

Some replies to your queries...

 From: Brian Norris [mailto:computersforpe...@gmail.com]
  On Wed, Oct 30, 2013 at 06:53:07AM -0300, Ezequiel Garcia wrote:

[...]

 I do have one curiosity here: omap2.c looks like it's essentially
 defaulting to the NAND_OMAP_POLLED callbacks during nand_scan_ident(),
 since omap2.c only initializes the read_buf callback after
 nand_scan_ident(). Is this ever a problem? Or should omap2.c be
 initializing its callbacks both before and after nand_scan_ident()
 (similar to how nand_base calls nand_set_defaults() twice for the AUTO
 case)?
 
There is no issue if _default_ functions present in nand_base.c
are used for chip-read_byte() or chip-read_buf() while probing the device.
The different callbacks defined in omap2.c like
- omap_read_buf()
- omap_read_buf_pref()
- omap_read_buf_dma_pref()
- omap_read_buf_irq_pref()
Above are alternatives for having better throughput in different use-cases.
These are not tied to hardware. So it's okay if these callbacks are assigned
post nand_scan_ident().


 What value do you use for ti,nand-xfer-type in your BeagleBone device
 tree? Is the xfer type a hardware requirement, or just a software
 configuration? IOW, does the hardware care if I simply use POLLED, even
 temporarily? (A potential side issue: does ti,nand-xfer-type even
 belong in the device tree, if it is not actually a hardware property?)
 
DMA and IRQ based callbacks have better throughput numbers than
POLLED ones. Though its not related to hardware, but as it's there in DT
now so we should maintain it. Also considering that:
- some older platforms might not support xx_dma_pref().
- some related pieces of information (like IRQ number, etc) comes from DT.
It was added as part of following patch..
http://www.spinics.net/lists/linux-omap/msg90250.html


  This time I've decided to submit this patch alone, so we can focus
  the discussion on this issue. The other cleanups can wait.
 
  AFAICS, the remaining questions are:
 
   1. Does this work in the 8-bit case?
   (I'm not able to test such case for lack of hardware)
 
 I'm not sure, of course, but I don't see why not. It's more likely to
 break for x16 than it is for x8.
 
Another question here is ..
The above patch assumes that user has configured GPMC bus-width
correctly, so if user is already providing GPMC bus-width information
via DT, then do we really need to detect NAND device bus-width again
using NAND_BUSWIDTH_AUTO ?
 

   2. Do we want to re-configure the GPMC one the NAND core detects the
  device bus width?
 
 I'm not quite sure here, as I don't know if I know enough about the GPMC
 to give an educated response here. Additionally, I'm not quite sure how
 re-configurable GPMC really is. It seems like we would be restricted
 physically if GPMC is hooked up as x8 for an x16 NAND (there are not
 enough pins connected). So even if we detect the NAND, it cannot
 function. I'm not sure about the reverse.
 
 Now, this returns me to my rejection of Pekon's patch here:
 
   http://lists.infradead.org/pipermail/linux-mtd/2013-October/049154.html
 
 (This patch addresses the question of checking the GPMC configuration,
 not correcting it.)
 
 I'll admit that I was underinformed regarding the need for this type of
 patch. Since that time, I am less sure of my criticism. But really, I
 just have more questions now.
 
 Does the GPMC intrinsically (physically; according to its pin
 configuration) restrict an attached device (e.g., NAND) to use x16 or
 x8?
Yes.. 

  Or does it simply specify a maximum data width that is wired up?
 (I'm presuming that you could wire an x8 device to an x16 interface and
 just leave the upper 8 unconnected...) Or is there some third option?
 
GPMC engine uses bus-width info to drive its data-lines.
- If GPMC is configured as x8, then it will only perform I/O on D0.. D7,
   even if all D0 .. D15 were wired on board.
- If GPMC is configured as x16, then it will perform I/O on D0.. D15,
   even if only D0 .. D7 were wired on board. There by reading 0x0 or
   garbage on D8 .. D15 lines.

This does not affect the probe, because chip-read_byte() would return
only single byte whether it's a x8 device or x16 device. So READID_CMD
works fine and you can read maf_id and dev_id. And based on that
device parameters can be looked from nand_flash_id[].
However when it comes to reading or writing complete page, then the
mismatch between GPMC configuration and actual NAND device
bus-width is seen, because there chip-read_buf() or chip-write_buf()
is used.


 The DT entry says:
 
   gpmc,device-width Total width of device(s) connected to a GPMC
 chip-select in bytes. The GPMC supports 8-bit
 and 16-bit devices and so this property must be
 1 or 2.
 
 So, this *sounds* like it specifies the exact width. But as I read it,
 this property could more optimally be specified as the *maximum*
 width...
 
Yes, its exact

Re: [PATCH 0/1] Fix OMAP2 NAND ONFI device detection

2013-10-30 Thread Ezequiel Garcia 
Pekon,

Let me answer this one alone, given it's an important question.

On Wed, Oct 30, 2013 at 09:18:53PM +, Gupta, Pekon wrote:
  
  I'm not sure, of course, but I don't see why not. It's more likely to
  break for x16 than it is for x8.
  
 Another question here is ..
 The above patch assumes that user has configured GPMC bus-width
 correctly, so if user is already providing GPMC bus-width information
 via DT, then do we really need to detect NAND device bus-width again
 using NAND_BUSWIDTH_AUTO ?


Hm.. I think you're forgetting the original motivation for this patch,
which was initially explained by you :-) The problem is ONFI doesn't work
in 16-bit mode.

Let me clarify. Since GPMC and NAND widths *must* match (otherwise, it
wouldn't make much sense, right?) we don't really need to autodetect the
NAND width.

However, since ONFI doesn't work in 16-bit mode, we would have to do
something like this (untested pseudo code):

  nand_scan_ident(...);

  if (platform_data-devsize == 16)
nand_chip-options |= NAND_BUSWIDTH_16;

And in some cases we might even get away with such solution. However,
we can't guarantee nand_scan_ident() doesn't need to swith to 16-bit mode
inside to perform other commands (maybe some ONFI extended parameter page).

Therefore, and given the width can be autodetected in any case (array
or ONFI based carry such information) it's much cleaner to simply do:

  nand_chip-options |= NAND_BUSWIDTH_AUTOMAGIC;
  nand_scan_ident(...);

See? Much cleaner, no?

And remember: the fact that we must know the device width to configure
the GPMC correctly (which being a hardware parameter belongs to the DT)
is OMAP specific. Other SoCs might not have such memory controller
and thus won't need such knowledge before hand, although that sounds
unlikely to me.

The outcome of this discussion seems to be that no driver should ever
need to specify the 'nand-bus-width' DT property, as Brian
previously suggested, right?

I must admit I'm a bit puzzled by seeing most drivers setting 16-bit
before calling nand_scan_ident(). I guess none of them relies on ONFI?
-- 
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