[linux-sunxi] Re: [PATCH v3 4/9] of: mtd: add documentation for the ONFI NAND timing mode property

2014-07-09 Thread Brian Norris
Hi Boris,

Looking back at this thread, there's at least one or two things I forgot
to answer. Sorry.

On Tue, May 20, 2014 at 11:32:04PM +0200, Boris BREZILLON wrote:
 On 20/05/2014 21:52, Brian Norris wrote:
[...]
 If the ECC bindings don't encode the minimum required ECC strength but
 rather the ECC config on a specific board then I guess minimum
 required ECC strength for non-ONFI chips should be defined somewhere
 else (stored in the device ID table ?).

They are. See nand_flash_dev::ecc, which holds fields for
ecc_strength_ds and step_ds. If we have to, we can add a timing mode
field to this struct.

  So you're saying that even though the chip actually specifies a single
  set of timings, you would describe this as a bitmask of several
  supported ONFI timing modes, up to the max performance?
 
  Is there ever a case where (for instance) a non-ONFI flash supports the
  equivalent of timing mode 3, but it does not support mode 2 or 1?
 
 I don't think so.

OK, then I don't think the mask approach is necessary, if we do ever
settle on using a DT binding here. (I hope we can avoid this.)

  But I can modify the bindings to just encode the maximum supported
  timing mode.
  AIUI, the non-ONFI datasheets really only specify a single timing mode,
  so I think we should only specify the max. And as a bonus, this
  actually makes the binding easier to use. A driver does not care about
  how many different modes are supported; it only needs to know what the
  max is.
 
 Agreed, actually my first binding was defining it this way.

Was there a good reason for changing it?

Thanks,
Brian

-- 
You received this message because you are subscribed to the Google Groups 
linux-sunxi group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[linux-sunxi] Re: [PATCH v3 4/9] of: mtd: add documentation for the ONFI NAND timing mode property

2014-05-20 Thread Boris BREZILLON
Hi Brian,

On 20/05/2014 20:25, Brian Norris wrote:
 Hi Boris,

 On Wed, Mar 12, 2014 at 07:07:39PM +0100, Boris BREZILLON wrote:
 Add documentation for the ONFI NAND timing mode property.

 Signed-off-by: Boris BREZILLON b.brezillon@gmail.com
 ---
  Documentation/devicetree/bindings/mtd/nand.txt |8 
  1 file changed, 8 insertions(+)

 diff --git a/Documentation/devicetree/bindings/mtd/nand.txt 
 b/Documentation/devicetree/bindings/mtd/nand.txt
 index b53f92e..2046027 100644
 --- a/Documentation/devicetree/bindings/mtd/nand.txt
 +++ b/Documentation/devicetree/bindings/mtd/nand.txt
 @@ -19,3 +19,11 @@ errors per {size} bytes.
  The interpretation of these parameters is implementation-defined, so not all
  implementations must support all possible combinations. However, 
 implementations
  are encouraged to further specify the value(s) they support.
 +
 +- onfi,nand-timing-mode: an integer encoding the supported ONFI timing 
 modes of
 +  the NAND chip. Each supported mode is represented as a bit position (i.e. 
 :
 +  mode 0 and 1 = (1  0) | (1  1) = 0x3).
 +  This is only used when the chip does not support the ONFI standard.
 +  The last bit set represent the closest mode fulfilling the NAND chip 
 timings.
 +  For a full description of the different timing modes see this document:
 +  www.onfi.org/~/media/ONFI/specs/onfi_3_1_spec.pdf
 I'm not 100% convinced this property should go in the device tree. With
 most other flash properties (device size, page size, and even minimum
 ECC requirements), we try to auto-detect these parameters to some
 extent. ONFI makes it easy for some class of chips, but for others, we
 typically rely on an in-kernel device ID table or ID decoding heuristic
 -- we don't require a DT description of every property of the flash. So
 what makes this property different?

AFAICT nothing, but the same goes for the ECC requirements, and we've
recently added DT bindings to define these requirements.
I'm not telling we should drop these ECC requirements bindings (actually
I'm using them :-)), but what's different with the timings requirements ?

Moreover, we will end up with a lot of new entries in the device ID
table if we decide to put these informations in this table.


 I realize that we may not include device ID entries for every flash that
 you need in the ID table (although we still are able to detect the
 important properties accurately, like page and block size). But would it
 suffice to default these flash to a lowest common timing mode, like mode
 0?

IMHO this is not a good solution: you'll end up with lower perfomances
on most of the supported NAND chips and I'm not sure this is what we want.


 If no other option works well, then I am still open to describing the
 supported timing modes in the DT.

 BTW, this bitfield property looks kinda strange to me. Do non-ONFI flash
 typically support multiple timing modes? And if so, how are we supposed
 to *change* modes? AFAIK, ONFI provides the only standard for
 configuring the flash's timing mode. So maybe you're really only wanting
 a default timing mode property that is a single integer, not a
 bitfield.

Indeed, I based it on the ONFI NAND timings mode model, but AFAIK (tell
me if I'm wrong), it should work because most of the timings are min
requirements.
This means, even if you provide slower signals transitions, the NAND
will work as expected.

But I can modify the bindings to just encode the maximum supported
timing mode.

Thanks for your feedback.

Best Regards,

Boris


-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

-- 
You received this message because you are subscribed to the Google Groups 
linux-sunxi group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[linux-sunxi] Re: [PATCH v3 4/9] of: mtd: add documentation for the ONFI NAND timing mode property

2014-05-20 Thread Jason Gunthorpe
On Tue, May 20, 2014 at 09:30:33PM +0200, Boris BREZILLON wrote:

 AFAICT nothing, but the same goes for the ECC requirements, and we've
 recently added DT bindings to define these requirements.
 I'm not telling we should drop these ECC requirements bindings (actually
 I'm using them :-)), but what's different with the timings requirements ?

ECC requirements are almost always something that has to be matched to
the bootloader (since the bootloader typicaly reads the NAND to boot),
so it is sensible to put that in the DT

The timings are a property of the chip, and if they can be detected
they should be. IMHO, the main purpose of a DT property would be to
lower the speed if, for some reason, the board cannot support the
device's full speed.

 Indeed, I based it on the ONFI NAND timings mode model, but AFAIK
 (tell me if I'm wrong), it should work because most of the timings
 are min requirements.  This means, even if you provide slower
 signals transitions, the NAND will work as expected.

IIRC for ONFI a device must always work in the mode 0 timings, without
requiring a command?

Jason

-- 
You received this message because you are subscribed to the Google Groups 
linux-sunxi group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[linux-sunxi] Re: [PATCH v3 4/9] of: mtd: add documentation for the ONFI NAND timing mode property

2014-05-20 Thread Brian Norris
On Tue, May 20, 2014 at 09:30:33PM +0200, Boris BREZILLON wrote:
 Hi Brian,
 
 On 20/05/2014 20:25, Brian Norris wrote:
  Hi Boris,
 
  On Wed, Mar 12, 2014 at 07:07:39PM +0100, Boris BREZILLON wrote:
  Add documentation for the ONFI NAND timing mode property.
 
  Signed-off-by: Boris BREZILLON b.brezillon@gmail.com
  ---
   Documentation/devicetree/bindings/mtd/nand.txt |8 
   1 file changed, 8 insertions(+)
 
  diff --git a/Documentation/devicetree/bindings/mtd/nand.txt 
  b/Documentation/devicetree/bindings/mtd/nand.txt
  index b53f92e..2046027 100644
  --- a/Documentation/devicetree/bindings/mtd/nand.txt
  +++ b/Documentation/devicetree/bindings/mtd/nand.txt
  @@ -19,3 +19,11 @@ errors per {size} bytes.
   The interpretation of these parameters is implementation-defined, so not 
  all
   implementations must support all possible combinations. However, 
  implementations
   are encouraged to further specify the value(s) they support.
  +
  +- onfi,nand-timing-mode: an integer encoding the supported ONFI timing 
  modes of
  +  the NAND chip. Each supported mode is represented as a bit position 
  (i.e. :
  +  mode 0 and 1 = (1  0) | (1  1) = 0x3).
  +  This is only used when the chip does not support the ONFI standard.
  +  The last bit set represent the closest mode fulfilling the NAND chip 
  timings.
  +  For a full description of the different timing modes see this document:
  +  www.onfi.org/~/media/ONFI/specs/onfi_3_1_spec.pdf
  I'm not 100% convinced this property should go in the device tree. With
  most other flash properties (device size, page size, and even minimum
  ECC requirements), we try to auto-detect these parameters to some
  extent. ONFI makes it easy for some class of chips, but for others, we
  typically rely on an in-kernel device ID table or ID decoding heuristic
  -- we don't require a DT description of every property of the flash. So
  what makes this property different?
 
 AFAICT nothing, but the same goes for the ECC requirements, and we've
 recently added DT bindings to define these requirements.
 I'm not telling we should drop these ECC requirements bindings (actually
 I'm using them :-)), but what's different with the timings requirements ?

ECC selection is not quite as scientific; with ECC, there are external
factors that influence the ECC mode that you should use, since any data
read/written from Linux has to be compatible with any data read/written
with another entity (e.g., bootloader). Note that the ECC bindings do
not represent a property of the flash chip itself (i.e., they don't hold
the minimum required ECC strength), but of the entire flash system
(i.e., what ECC must I use to play nicely with the rest of the world).

With timing modes, this is purely a property of the flash chip, and we
do not have to synchronize it with the bootloader. We don't exactly care
if a bootloader and Linux use slightly different timing modes.

 Moreover, we will end up with a lot of new entries in the device ID
 table if we decide to put these informations in this table.

Yes, that could be a problem.

What sort of non-ONFI flash chips do you have that need this property?
And what timing mode(s) do they use? Is there, for instance, a pattern
such that all Hynix MLC of a certain generation use a particular timing
mode?

  I realize that we may not include device ID entries for every flash that
  you need in the ID table (although we still are able to detect the
  important properties accurately, like page and block size). But would it
  suffice to default these flash to a lowest common timing mode, like mode
  0?
 
 IMHO this is not a good solution: you'll end up with lower perfomances
 on most of the supported NAND chips and I'm not sure this is what we want.

No, we wouldn't want to always use mode 0. But it's possible we can get
good enough heuristics for most flash, if we can integrate timing modes
into the current extended ID decoding. Not sure.

I'm also concerned here that this kind of binding will be difficult to
use properly. A user/developer/board-designer would have to read the
datasheet and compare all its values to the ONFI spec to find the
closest match, and they would have to do this for each new flash they
use. If we can help them by autodetecting this, that would be great.

  If no other option works well, then I am still open to describing the
  supported timing modes in the DT.
 
  BTW, this bitfield property looks kinda strange to me. Do non-ONFI flash
  typically support multiple timing modes? And if so, how are we supposed
  to *change* modes? AFAIK, ONFI provides the only standard for
  configuring the flash's timing mode. So maybe you're really only wanting
  a default timing mode property that is a single integer, not a
  bitfield.
 
 Indeed, I based it on the ONFI NAND timings mode model, but AFAIK (tell
 me if I'm wrong), it should work because most of the timings are min
 requirements.
 This means, even if you provide slower signals 

[linux-sunxi] Re: [PATCH v3 4/9] of: mtd: add documentation for the ONFI NAND timing mode property

2014-05-20 Thread Boris BREZILLON

On 20/05/2014 21:52, Brian Norris wrote:
 On Tue, May 20, 2014 at 09:30:33PM +0200, Boris BREZILLON wrote:
 Hi Brian,

 On 20/05/2014 20:25, Brian Norris wrote:
 Hi Boris,

 On Wed, Mar 12, 2014 at 07:07:39PM +0100, Boris BREZILLON wrote:
 Add documentation for the ONFI NAND timing mode property.

 Signed-off-by: Boris BREZILLON b.brezillon@gmail.com
 ---
  Documentation/devicetree/bindings/mtd/nand.txt |8 
  1 file changed, 8 insertions(+)

 diff --git a/Documentation/devicetree/bindings/mtd/nand.txt 
 b/Documentation/devicetree/bindings/mtd/nand.txt
 index b53f92e..2046027 100644
 --- a/Documentation/devicetree/bindings/mtd/nand.txt
 +++ b/Documentation/devicetree/bindings/mtd/nand.txt
 @@ -19,3 +19,11 @@ errors per {size} bytes.
  The interpretation of these parameters is implementation-defined, so not 
 all
  implementations must support all possible combinations. However, 
 implementations
  are encouraged to further specify the value(s) they support.
 +
 +- onfi,nand-timing-mode: an integer encoding the supported ONFI timing 
 modes of
 +  the NAND chip. Each supported mode is represented as a bit position 
 (i.e. :
 +  mode 0 and 1 = (1  0) | (1  1) = 0x3).
 +  This is only used when the chip does not support the ONFI standard.
 +  The last bit set represent the closest mode fulfilling the NAND chip 
 timings.
 +  For a full description of the different timing modes see this document:
 +  www.onfi.org/~/media/ONFI/specs/onfi_3_1_spec.pdf
 I'm not 100% convinced this property should go in the device tree. With
 most other flash properties (device size, page size, and even minimum
 ECC requirements), we try to auto-detect these parameters to some
 extent. ONFI makes it easy for some class of chips, but for others, we
 typically rely on an in-kernel device ID table or ID decoding heuristic
 -- we don't require a DT description of every property of the flash. So
 what makes this property different?
 AFAICT nothing, but the same goes for the ECC requirements, and we've
 recently added DT bindings to define these requirements.
 I'm not telling we should drop these ECC requirements bindings (actually
 I'm using them :-)), but what's different with the timings requirements ?
 ECC selection is not quite as scientific; with ECC, there are external
 factors that influence the ECC mode that you should use, since any data
 read/written from Linux has to be compatible with any data read/written
 with another entity (e.g., bootloader). Note that the ECC bindings do
 not represent a property of the flash chip itself (i.e., they don't hold
 the minimum required ECC strength), but of the entire flash system
 (i.e., what ECC must I use to play nicely with the rest of the world).

If the ECC bindings don't encode the minimum required ECC strength but
rather the ECC config on a specific board then I guess minimum
required ECC strength for non-ONFI chips should be defined somewhere
else (stored in the device ID table ?).

Actually, in the sunxi NAND controller driver, I'm using the DT defined
ECC config when the NAND does not support ONFI timings retrieval.


 With timing modes, this is purely a property of the flash chip, and we
 do not have to synchronize it with the bootloader. We don't exactly care
 if a bootloader and Linux use slightly different timing modes.

Agreed.



 Moreover, we will end up with a lot of new entries in the device ID
 table if we decide to put these informations in this table.
 Yes, that could be a problem.

 What sort of non-ONFI flash chips do you have that need this property?

I only have the Hynix one defined in my patch series.
Other people tested my driver on different boards but I don't recall
exactly which NAND they had (a samsung one IIRC).

 And what timing mode(s) do they use? Is there, for instance, a pattern
 such that all Hynix MLC of a certain generation use a particular timing
 mode?

I'll take a look.


 I realize that we may not include device ID entries for every flash that
 you need in the ID table (although we still are able to detect the
 important properties accurately, like page and block size). But would it
 suffice to default these flash to a lowest common timing mode, like mode
 0?
 IMHO this is not a good solution: you'll end up with lower perfomances
 on most of the supported NAND chips and I'm not sure this is what we want.
 No, we wouldn't want to always use mode 0. But it's possible we can get
 good enough heuristics for most flash, if we can integrate timing modes
 into the current extended ID decoding. Not sure.

 I'm also concerned here that this kind of binding will be difficult to
 use properly. A user/developer/board-designer would have to read the
 datasheet and compare all its values to the ONFI spec to find the
 closest match, and they would have to do this for each new flash they
 use. If we can help them by autodetecting this, that would be great.
 


 If no other option works well, then I am still open to describing the
 supported 

[linux-sunxi] Re: [PATCH v3 4/9] of: mtd: add documentation for the ONFI NAND timing mode property

2014-03-12 Thread Boris BREZILLON

Le 12/03/2014 19:27, Warner Losh a écrit :

On Mar 12, 2014, at 12:07 PM, Boris BREZILLON b.brezillon@gmail.com wrote:


Add documentation for the ONFI NAND timing mode property.

I don't see a Toggle/JEDEC mode timing property. Will that be defined for 
Toshiba, Samsung
and San Disk flash? Or will this be limited to Micron, Intel and Hynix (the 
only ones
supporting ONFI)?


There is currently no Toggle/JEDEC timing mode support, and I don't know 
what these

timing modes describe.
But I guess they can be converted to standard timings described in 
nand_sdr_timings

struct (or in your case in the future nand_ddr_timings struct).

Could you check that the timings described by these modes use the same 
naming convention
as those defined in the ONFI specification 
(www.onfi.org/~/media/ONFI/specs/onfi_3_1_spec.pdf

 chapter 4.15.4) ?

If this is the case, we can define a new DT property and new converters.

Best Regards,

Boris


Warner



Signed-off-by: Boris BREZILLON b.brezillon@gmail.com
---
Documentation/devicetree/bindings/mtd/nand.txt |8 
1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/mtd/nand.txt 
b/Documentation/devicetree/bindings/mtd/nand.txt
index b53f92e..2046027 100644
--- a/Documentation/devicetree/bindings/mtd/nand.txt
+++ b/Documentation/devicetree/bindings/mtd/nand.txt
@@ -19,3 +19,11 @@ errors per {size} bytes.
The interpretation of these parameters is implementation-defined, so not all
implementations must support all possible combinations. However, implementations
are encouraged to further specify the value(s) they support.
+
+- onfi,nand-timing-mode: an integer encoding the supported ONFI timing modes of
+  the NAND chip. Each supported mode is represented as a bit position (i.e. :
+  mode 0 and 1 = (1  0) | (1  1) = 0x3).
+  This is only used when the chip does not support the ONFI standard.
+  The last bit set represent the closest mode fulfilling the NAND chip timings.
+  For a full description of the different timing modes see this document:
+  www.onfi.org/~/media/ONFI/specs/onfi_3_1_spec.pdf
--
1.7.9.5

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


--
You received this message because you are subscribed to the Google Groups 
linux-sunxi group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[linux-sunxi] Re: [PATCH v3 4/9] of: mtd: add documentation for the ONFI NAND timing mode property

2014-03-12 Thread Warner Losh

On Mar 12, 2014, at 12:07 PM, Boris BREZILLON b.brezillon@gmail.com wrote:

 Add documentation for the ONFI NAND timing mode property.

I don't see a Toggle/JEDEC mode timing property. Will that be defined for 
Toshiba, Samsung
and San Disk flash? Or will this be limited to Micron, Intel and Hynix (the 
only ones
supporting ONFI)?

Warner


 Signed-off-by: Boris BREZILLON b.brezillon@gmail.com
 ---
 Documentation/devicetree/bindings/mtd/nand.txt |8 
 1 file changed, 8 insertions(+)
 
 diff --git a/Documentation/devicetree/bindings/mtd/nand.txt 
 b/Documentation/devicetree/bindings/mtd/nand.txt
 index b53f92e..2046027 100644
 --- a/Documentation/devicetree/bindings/mtd/nand.txt
 +++ b/Documentation/devicetree/bindings/mtd/nand.txt
 @@ -19,3 +19,11 @@ errors per {size} bytes.
 The interpretation of these parameters is implementation-defined, so not all
 implementations must support all possible combinations. However, 
 implementations
 are encouraged to further specify the value(s) they support.
 +
 +- onfi,nand-timing-mode: an integer encoding the supported ONFI timing modes 
 of
 +  the NAND chip. Each supported mode is represented as a bit position (i.e. :
 +  mode 0 and 1 = (1  0) | (1  1) = 0x3).
 +  This is only used when the chip does not support the ONFI standard.
 +  The last bit set represent the closest mode fulfilling the NAND chip 
 timings.
 +  For a full description of the different timing modes see this document:
 +  www.onfi.org/~/media/ONFI/specs/onfi_3_1_spec.pdf
 -- 
 1.7.9.5
 
 --
 To unsubscribe from this list: send the line unsubscribe devicetree in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
You received this message because you are subscribed to the Google Groups 
linux-sunxi group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.