public inbox for linuxppc-dev@ozlabs.org 
 help / color / mirror / Atom feed
From: Sergei Shtylyov <sshtylyov@ru•mvista.com>
To: David Gibson <david@gibson•dropbear.id.au>
Cc: linuxppc-dev@ozlabs•org, linux-mtd@lists•infradead.org
Subject: Re: [PATCH 2/6] PowerPC 440EPx: Sequoia DTS
Date: Sun, 19 Aug 2007 16:59:45 +0400	[thread overview]
Message-ID: <46C83EC1.7050304@ru.mvista.com> (raw)
In-Reply-To: <20070807032806.GE15619@localhost.localdomain>

Hello.

    I'm having sort of vacation, mostly away from computer, hence my belated 
reply...

David Gibson wrote:

>>>>>Can you describe some of the options for *not* direct mapped flash
>>>>>chips - I can't reasonably come up with a way of describing the
>>>>>distinction when I've never seen NOR flash other than direct mapped.

>>>>   You're lucky to live in the single-endian world.  Once you're in bi-endian 
>>>>one, all kinds of strange mappings become possible.  I've seen the MIPS 

>>    Well, not necessarily -- 16-bit only accesses are always possibly (no 
>>dount this would be a strange mapping however)...

> I have no idea what you're getting at here.

    The code fragment that I've posted before -- it used readw() in read8() 
method of the mapping.

>>>>mapping driver which required swapping flash bytes in s/w in BE mode, i.e. 
>>>>couldn't be served by physmap, yet that's not all...  here's the code of its 
>>>>map read*() methods:

>>>Aha!  Ok, now I understand the sorts of situations you're talking
>>>about.  By "not direct mapped", I thought you were talking about some
>>>kind of access via address/data registers on some indirect bus
>>>controller, rather than weird variations on endianness and
>>>bit-swizzling.

>>    No, that would be just too ridiculous for a NOR flash -- I hope. :-)

> Heh.  In my experience, very little is so ridiculous that some
> embedded vendor won't do it.

    Right. :-)

>>>Hrm.. this is a property of how the flash is wired onto the bus,
>>>rather than of the flash chips themselves, 

>>    Indeed.

>>>so I'm not entirely sure where description of it belongs.

>>    So, you're saying that the 1:1 address correspondence rule stops to apply 
>>here?

> Well.. it all depends what exactly you consider the address space of
> the flash bank.  By which I mean the whole shmozzle represented by the
> device node, not the individual flash chips.  It's not immediately
> obvious whether or not that should include any swizzling done by the
> bus wiring.

> It would be possible, I guess, to define a 'swizzled-ranges' property
> or something which allows child devices to be embedded in the parent's
> address range in a not-direct way.  However, the swizzling on the
> flash bank is really a property of the flash bank, not of the parent
> bus - requiring it to be encoded in the parent is pretty yucky -
> especially if the flash bank is just part of a larger chunk of bus
> address space, defined by a single large ranges entry in the parent.

[...]

>>>>>>>+     - reg : Address range of the flash chip
>>>>>>>+     - bank-width : Width (in bytes) of the flash bank.  Equal to the device width
>>>>>>>+       times the number of interleaved chips.
>>>>>>>+     - device-width : (optional) Width of a single flash chip.  If omitted,
>>>>>>>+       assumed to be equal to 'bank-width'.

>>>>>>  Why then not just introduce the "interleave" prop and obsolete the
>>>>>>"bank-width"?

>>>>>Because they're equivalent information, and bank-width is what the
>>>>>code ends up actually caring about.  I don't see any reason to prefer
>>>>>giving the interleave.

>>>>   Well, "device-width" is not the thing that we care about either. ;-)

>>>Well, yes but we need to encode it somehow.  Segher preferred
>>>device-width to interleave, because it's closer to a description of
>>>the actual hardware, rather than an abstration decribing its wiring.

>>>In other words I *still* don't see any reason to prefer giving the
>>>interleave.

    Well, for example, enable "Flash chip driver advanced configuration 
options" (appers in the "RAM/ROM/Flash chip drivers menu" when you enable CFI 
probe, and it has a choice of bus widths 8 to 256 bits (here buswidth == bank 
width) and *interleave factors* 1 to 8...

>>    I wasn't talking of "interleave" preference over "device-width", I was 
>>talking about obsoleting "bank-width" with this pair.

> Whatever.  You haven't given any argument to prefer interleave over
> either device-width or bank-width.

    Consistency? The bank width is a product of the chip width and interleave 
factor.  However, I don't insist. The MTD core itself has a different view on 
the CFI flash geometry: bank width (which is a property of a mapping driver) 
and interleave (which is a property of a chip driver).

>>>>>>>Index: working-2.6/drivers/mtd/maps/physmap_of.c
>>>>>>>===================================================================
>>>>>>>--- working-2.6.orig/drivers/mtd/maps/physmap_of.c	2007-07-30 17:07:11.000000000 +1000
>>>>>>>+++ working-2.6/drivers/mtd/maps/physmap_of.c	2007-07-30 17:07:14.000000000 +1000

>>>>[...]

>>>>>>>@@ -221,6 +297,14 @@ err_out:
>>>>>>>
>>>>>>>static struct of_device_id of_physmap_match[] = {
>>>>>>>	{
>>>>>>>+		.compatible	= "cfi-flash",
>>>>>>>+		.data		= (void *)"cfi_probe",
>>>>>>>+	},
>>>>>>>+	{
>>>>>>>+		.compatible	= "jedec-flash",
>>>>>>>+		.data		= (void *)"jedec_probe",
>>>>>>>+	},
>>>>>>>+	{

>>>>>>  This would also trigger on non-linearly mapped CFI or JEDEC
>>>>>>flashes which is not a good idea -- however, as we're using the MTD
>>>>>>probing code anyway, it will just fail, so it's not luckily is not a
>>>>>>fatal design flaw.

>>>>>Well, if there's nothing else to distinguish them.  Which there isn't
>>>>>yet, but will need to be: see above about incomplete - helpful
>>>>>suggestions about how to describe the mapping would be more useful
>>>>>than merely pointing out the lack.

>>>>   I was thinking about adding "direct-mapped" prop... but maybe adding 
>>>>"ranges" to the parent node (that's "ebc") would indeed ensure that the flash 
>>>>is mapped 1:1 to the EBC's parent bus also.

>>>The ebc already has a ranges property.  But that can't describe the

>>    Not in the device tree that started that thread -- I haven't seen another.

> Ah sorry.  The ranges property isn't in the dts, it's added by the
> bootwrapper based on the EBC register contents.

    Cool. :-)

>>>sorts of bit-swizzling you're talking about.

>>    Let's hear what Segher says (if he's not yet tired of all this :-).

>>>>>>>Index: working-2.6/arch/powerpc/boot/dts/ebony.dts
>>>>>>>===================================================================
>>>>>>>--- working-2.6.orig/arch/powerpc/boot/dts/ebony.dts	2007-07-30 17:07:14.000000000 +1000
>>>>>>>+++ working-2.6/arch/powerpc/boot/dts/ebony.dts	2007-07-30 17:07:14.000000000 +1000

>>>>>>[...]

>>>>>>>@@ -158,14 +161,20 @@
>>>>>>>				};

>>>>>>>				large-flash@2,0 {

>>>>>>  Hmm... what does @2,0 mean? :-O

>>>>>EBC chip select 2, offset 0.

>>>>   Well, so this node is under some kind of local bus node -- that's good.
>>>>Didn't know that the spec allows commas after @...

>>>Well, now you do.  I believe USB usually uses that format, too.

>>    USB what, hosts or devices?

> Devices.

    And what the comma-separated numbers mean there?

[...]

WBR, Sergei

  parent reply	other threads:[~2007-08-19 12:57 UTC|newest]

Thread overview: 54+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-07-30 15:06 [PATCH 2/6] PowerPC 440EPx: Sequoia DTS Valentine Barshak
2007-08-01  2:08 ` David Gibson
2007-08-01  4:57   ` Segher Boessenkool
2007-08-01  5:04     ` David Gibson
2007-08-01  5:47       ` David Gibson
2007-08-02 15:23         ` Sergei Shtylyov
2007-08-03  3:13           ` David Gibson
2007-08-03 15:47             ` Sergei Shtylyov
2007-08-06  4:21               ` David Gibson
2007-08-06 18:37                 ` Sergei Shtylyov
2007-08-06 21:03                   ` Segher Boessenkool
2007-08-06 22:15                   ` Benjamin Herrenschmidt
2007-08-06 23:09                     ` Segher Boessenkool
2007-08-07  3:29                     ` David Gibson
2007-08-07  3:28                   ` David Gibson
2007-08-07 15:43                     ` Scott Wood
2007-08-07 17:01                       ` Segher Boessenkool
2007-08-07 16:43                     ` Segher Boessenkool
2007-08-08  0:35                       ` David Gibson
2007-08-19 12:59                     ` Sergei Shtylyov [this message]
2007-08-06 20:54                 ` Segher Boessenkool
2007-08-07  4:12                   ` David Gibson
2007-08-07 16:51                     ` Segher Boessenkool
2007-08-08  1:13                       ` David Gibson
2007-08-09 19:53                         ` Segher Boessenkool
2007-08-10  1:07                           ` David Gibson
2007-08-10 20:48                             ` Segher Boessenkool
2007-08-24 19:10                       ` Sergei Shtylyov
2007-08-24 20:43                         ` Segher Boessenkool
2007-08-06 20:35               ` Segher Boessenkool
2007-08-07  4:09                 ` David Gibson
2007-08-07 16:58                   ` Segher Boessenkool
2007-08-08  0:48                     ` David Gibson
2007-08-06 20:20             ` Segher Boessenkool
2007-08-07  3:35               ` David Gibson
2007-08-06 20:12           ` Segher Boessenkool
2007-08-02 20:18         ` Josh Boyer
2007-08-03  0:49           ` David Gibson
2007-08-03 16:29         ` Sergei Shtylyov
2007-08-06  4:31           ` David Gibson
2007-08-06 20:55             ` Segher Boessenkool
2007-08-06 20:41           ` Segher Boessenkool
2007-08-06 19:59         ` Segher Boessenkool
2007-08-07  3:41           ` David Gibson
2007-08-07 16:33             ` Segher Boessenkool
2007-08-08  1:51               ` David Gibson
2007-08-09 20:00                 ` Segher Boessenkool
2007-08-10  1:11                   ` David Gibson
2007-08-02 20:16       ` Josh Boyer
2007-08-01 14:13   ` Valentine Barshak
2007-08-02  1:00     ` David Gibson
2007-08-02 20:15   ` Josh Boyer
2007-08-06 20:15     ` Segher Boessenkool
2007-08-07  4:11       ` David Gibson

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=46C83EC1.7050304@ru.mvista.com \
    --to=sshtylyov@ru$(echo .)mvista.com \
    --cc=david@gibson$(echo .)dropbear.id.au \
    --cc=linux-mtd@lists$(echo .)infradead.org \
    --cc=linuxppc-dev@ozlabs$(echo .)org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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