Skip to contentSkip to conversation

booting-without-of: add more bindings for FSL UPM driver

PATCH In review Wolfgang Grandegger 2 participants 6 messages Jun 9, 2008 – Jun 26, 2008

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 booting-without-of: add more bindings for FSL UPM driver

This patch adds the bindings for the property "chip-delay" used by the patch "[NAND] driver extension to support NAND on TQM85xx modules" posted to this list a few minutes ago:

      http://ozlabs.org/pipermail/linuxppc-dev/2008-June/057632.html

Note: this patch is based on the following patches from Anton

      Vorontsov posted to this list some time ago:

      http://ozlabs.org/pipermail/linuxppc-dev/2008-May/056531.html
      http://ozlabs.org/pipermail/linuxppc-dev/2008-June/057244.html
---
 Documentation/powerpc/booting-without-of.txt |    1 +
 1 file changed, 1 insertion(+)
Documentation/powerpc/booting-without-of.txt +1 -0
Index: linux-2.6-galak/Documentation/powerpc/booting-without-of.txt
===================================================================
@@ -3007,6 +3007,7 @@ platforms are moved over to use the flat
       - reg : should specify localbus chip select and size used for the chip.
       - fsl,upm-addr-offset : UPM pattern offset for the address latch.
       - fsl,upm-cmd-offset : UPM pattern offset for the command latch.
+      - chip-delay : may specify a delay value in milliseconds.
       - gpios : may specify optional GPIO connected to the Ready-Not-Busy pin.
 
       Example:
#1 lore raw
Quoted 1 line

+ - chip-delay : may specify a delay value in milliseconds.

Delay for what? The binding should say. "chip-delay" is a bit too generic name as well, it could be more descriptive perhaps.

Shouldn't this be a property of the NAND device anyway, not the NAND controller?

Segher

#2 lore raw
Quoted 4 lines · Segher Boessenkool
Quoted 1 line

+ - chip-delay : may specify a delay value in milliseconds.

Delay for what? The binding should say. "chip-delay" is a bit too generic name as well, it could be more descriptive perhaps.

The chip-delay property defines an appropriate maximum delay time (tR) required for read operations if the R/B pin is not connected. It's used as shown below:

+ /* Note: NAND support needs to be enabled in U-Boot */
+ upm@3,0 {
+ #address-cells = <0>;
+ #size-cells = <0>;
+ compatible = "fsl,upm-nand";
+ reg = <3 0x0 0x800>;
+ fsl,upm-addr-offset = <0x10>;
+ fsl,upm-cmd-offset = <0x08>;
+ chip-delay = <25>; // in micro-seconds
+
+ nand@0 {
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ partition@0 {
+ label = "fs";
+ reg = <0x00000000 0x01000000>;
+ };
+ };
+ };
+ };
+

Quoted 2 lines

Shouldn't this be a property of the NAND device anyway, not the NAND controller?

Strictly speaking, it's a property of the NAND device. Therefore it should be inside the node nand@0, I thhink:

+ nand@0 {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ chip-delay = <25>; // in micro-seconds
+
+ partition@0 {
+ label = "fs";
+ reg = <0x00000000 0x01000000>;
+ };
+ };

Where should that be documented?

Wolfgang.

#3 lore raw
Quoted 8 lines
Quoted 4 lines
Quoted 1 line

+ - chip-delay : may specify a delay value in milliseconds.

Delay for what? The binding should say. "chip-delay" is a bit too generic name as well, it could be more descriptive perhaps.

The chip-delay property defines an appropriate maximum delay time (tR) required for read operations if the R/B pin is not connected.

Yeah. So please put that in the binding.

Review

Quoted 10 lines
Quoted 2 lines

Shouldn't this be a property of the NAND device anyway, not the NAND controller?

Strictly speaking, it's a property of the NAND device. Therefore it should be inside the node nand@0, I thhink:

(quoted diff) +4 -0
+			nand@0 {
+				#address-cells = <1>;
+				#size-cells = <1>;
+			        chip-delay = <25>; // in micro-seconds

Something like that, yes. You wrote milliseconds before; which is it?

And, a better property name, please.

Quoted 1 line

Where should that be documented?

In the binding for nand devices. If there isn't any yet, it might be best to include that with the binding for your nand controller (i.e., describe the whole sub node there).

Segher

#4 lore raw
Quoted 17 lines · Segher Boessenkool
Quoted 8 lines
Quoted 4 lines
Quoted 1 line

+ - chip-delay : may specify a delay value in milliseconds.

Delay for what? The binding should say. "chip-delay" is a bit too generic name as well, it could be more descriptive perhaps.

The chip-delay property defines an appropriate maximum delay time (tR) required for read operations if the R/B pin is not connected.

Yeah. So please put that in the binding.

Quoted 6 lines
Quoted 2 lines

Shouldn't this be a property of the NAND device anyway, not the NAND controller?

Strictly speaking, it's a property of the NAND device. Therefore it should be inside the node nand@0, I thhink:

Quoted 6 lines

Review

Quoted 4 lines
(quoted diff) +4 -0
+            nand@0 {
+                #address-cells = <1>;
+                #size-cells = <1>;
+                    chip-delay = <25>; // in micro-seconds

Something like that, yes. You wrote milliseconds before; which is it?

Oops, it's in micro-seconds.

Quoted 1 line

And, a better property name, please.

Here is what we speak about (from the NAND chip manual):

"After a READ command is sent to the memory device, data is
transferred from the memory array to the data register in tR. Typically tR is 25us. The READ STATUS (70h) command or the R/B# signal can be used to determine when the device is ready."

And in the NAND header file "chip-delay" is documented as shown below:

"chip_delay: [BOARDSPECIFIC] chip dependent delay for transfering data
from array to read regs (tR)"

The name "read-fetch-time" sounds more reasonable too me, but English is not my native language.

Quoted 5 lines
Quoted 1 line

Where should that be documented?

In the binding for nand devices. If there isn't any yet, it might be best to include that with the binding for your nand controller (i.e., describe the whole sub node there).

OK, here is a my proposal:

x) Freescale Localbus UPM programmed to work with NAND flash

Required properties:
- compatible : "fsl,upm-nand".
- reg : should specify localbus chip select and size used for the chip.
- fsl,upm-addr-offset : UPM pattern offset for the address latch.
- fsl,upm-cmd-offset : UPM pattern offset for the command latch.
- gpios : may specify optional GPIO connected to the Ready-Not-Busy pin.

Each NAND flash device is represented as a sub-node of the UPM node. The nodes's name represents the name of the corresponding device.

NAND flash properties:
- compatible: not yet used.
- read-fetch-delay: chip dependent delay for transfering data from array to read regs (tR) im micro-seconds.

Each partition is represented as a sub-node of the NAND flash device. Each node's name represents the name of the corresponding partition of the NAND flash device.

NAND flash partitions:
- reg : The partition's offset and size within the flash bank.
- label : (optional) The label / name for this flash partition. If omitted, the label is taken from the node name (excluding the unit address).
- read-only : (optional) This parameter, if present, is a hint to Linux that this flash partition should only be mounted read-only. This is usually used for flash partitions containing early-boot firmware images or data which should not be clobbered.

Example:

upm@1,0 {
compatible = "fsl,upm-nand";
reg = <1 0 1>;
fsl,upm-addr-offset = <16>;
fsl,upm-cmd-offset = <8>;
gpios = <&qe_pio_e 18 0>;

flash {
#address-cells = <1>; #size-cells = <1>;
compatible = "stmicro,NAND512W3A2BN6E"; read-fetch-delay = 25

fs@0 {
label = "fs";
reg = <0 f80000>;
};

firmware@f80000 {
label ="firmware"; reg = <f80000 80000>; read-only;
};
};
};

What do you think?

Wolfgang.

#5 lore raw
In reply to Segher Boessenkool

Hi Segher,

Quoted 24 lines · Segher Boessenkool
Quoted 8 lines
Quoted 4 lines
Quoted 1 line

+ - chip-delay : may specify a delay value in milliseconds.

Delay for what? The binding should say. "chip-delay" is a bit too generic name as well, it could be more descriptive perhaps.

The chip-delay property defines an appropriate maximum delay time (tR) required for read operations if the R/B pin is not connected.

Yeah. So please put that in the binding.

Review

Quoted 10 lines
Quoted 2 lines

Shouldn't this be a property of the NAND device anyway, not the NAND controller?

Strictly speaking, it's a property of the NAND device. Therefore it should be inside the node nand@0, I thhink:

(quoted diff) +4 -0
+            nand@0 {
+                #address-cells = <1>;
+                #size-cells = <1>;
+                    chip-delay = <25>; // in micro-seconds

Something like that, yes. You wrote milliseconds before; which is it?

Quoted 7 lines

And, a better property name, please.

Quoted 1 line

Where should that be documented?

In the binding for nand devices. If there isn't any yet, it might be best to include that with the binding for your nand controller (i.e., describe the whole sub node there).

This is another try to get this binding accepted. Below is my revised patch introducing the proper "read-fetch-delay" for the sub-nodes of the FSL UPM nodes and documenting all other options as well.

What do you think?

Wolfgang.

From: Wolfgang Grandegger <[email protected]>
Subject: booting-without-of: add more bindings for FSL UPM driver

This patch adds the bindings for the property "read-fetch-delay" to the sub-nodes of the "fsl,upm-nand" compatible nodes. It is used by the patch "[NAND] driver extension to support NAND on TQM85xx modules" posted to this list some time ago:

     http://ozlabs.org/pipermail/linuxppc-dev/2008-June/057632.html

Furthermore it documents all other properties of the sub-nodes.

Note: this patch is based on the following patches from Anton

     Vorontsov posted to this list some time ago:

     http://ozlabs.org/pipermail/linuxppc-dev/2008-May/056531.html
     http://ozlabs.org/pipermail/linuxppc-dev/2008-June/057244.html

---
Documentation/powerpc/booting-without-of.txt | 34 +++++++++++++++++++++++++--

1 file changed, 32 insertions(+), 2 deletions(-)
Documentation/powerpc/booting-without-of.txt +22 -0
Index: linux-2.6-galak/Documentation/powerpc/booting-without-of.txt
===================================================================
@@ -3036,6 +3036,28 @@ platforms are moved over to use the flat
      - fsl,upm-cmd-offset : UPM pattern offset for the command latch.
      - gpios : may specify optional GPIO connected to the Ready-Not-Busy pin.
 
+      Each NAND flash is represented as a sub-node of the UPM node.
+      The nodes's name represents the name of the corresponding device.
+
+      NAND flash properties:
+      - compatible : not yet used.
+      - read-fetch-delay : chip dependent delay for transfering data
+        from array to read regs (tR).
+
+      Each partition is represented as a sub-node of the NAND flash device.
+      Each node's name represents the name of the corresponding partition
+      of the flash device.
+
+      NAND flash partitions:
+      - reg : The partition's offset and size within the flash bank.
+      - label : (optional) The label / name for this flash partition. If
+        omitted, the label is taken from the node name (excluding the
+        unit address).
+      - read-only : (optional) This parameter, if present, is a hint to
+        Linux that this flash partition should only be mounted read-only.
+        This is usually used for flash partitions containing early-boot
+        firmware images or data which should not be clobbered.
+
      Example:
 
	upm@1,0 {
@@ -3049,9 +3071,17 @@ platforms are moved over to use the flat
			#address-cells = <1>;
			#size-cells = <1>;
			compatible = "stmicro,NAND512W3A2BN6E";

+ read-fetch-delay = 25
+
+ fs@0 {
+ label = "fs";
+ reg = <0 f80000>;
+ };

- partition@0 {
- ...
+ firmware@f80000 {
+ label ="firmware";
+ reg = <f80000 80000>;

+				read-only;
			};
		};
	};