linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: Wolfgang Grandegger <wg@grandegger.com>
To: Segher Boessenkool <segher@kernel.crashing.org>
Cc: Linuxppc-dev@ozlabs.org, linux-mtd@lists.infradead.org
Subject: Re: [PATCH] booting-without-of: add more bindings for FSL UPM driver
Date: Thu, 12 Jun 2008 10:10:02 +0200	[thread overview]
Message-ID: <4850D9DA.7000207@grandegger.com> (raw)
In-Reply-To: <a9f4227722f169bc9c2e712df774b748@kernel.crashing.org>

Segher Boessenkool wrote:
>>>> +      - 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.
> 
>>> 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
> 
> Something like that, yes.  You wrote milliseconds before; which is it?

Oops, it's in micro-seconds.

> 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.

>> 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.

  reply	other threads:[~2008-06-12  8:10 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-06-09  8:42 [PATCH] booting-without-of: add more bindings for FSL UPM driver Wolfgang Grandegger
2008-06-09 10:30 ` Segher Boessenkool
2008-06-09 15:19   ` Wolfgang Grandegger
2008-06-09 23:47     ` Segher Boessenkool
2008-06-12  8:10       ` Wolfgang Grandegger [this message]
2008-06-26 13:07       ` Wolfgang Grandegger

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4850D9DA.7000207@grandegger.com \
    --to=wg@grandegger.com \
    --cc=Linuxppc-dev@ozlabs.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=segher@kernel.crashing.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).