Re: [PATCH 2/6] PowerPC 440EPx: Sequoia DTS
From: Segher Boessenkool <hidden>
Date: 2007-08-06 20:12:07
quoted
+ - compatible : should contain the specific model of flash chip(s) used + followed by either "cfi-flash" or "jedec-flash"This "compatible" prop (and the node in whole) doesn't say a thing about how the flash is mapped into the CPU address space.
...and it shouldn't. That's what "ranges" properties in all the various (grand-)parents of the node are for.
I strongly disagree that this node provides enough info to select a driver. :-/
If Linux needs more information than what the device _is_, but also needs information about how it is _used_ on some particular hardware, to select a driver; then it can bloody well do so. Nowhere is it said that an OS can _only_ use "compatible" properties to do its driver selection.
quoted
+ - 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 "interleave" is overly complicated and still doesn't handle all cases. Also, it's a more confusing name. The goal here is to handle 98% (or just 90% even) of all cases in as simple a way as possible; everything else can get special handling later.
quoted
+ Flash partitions + - reg : + - read-only : (optional)All that would look nice but partition is even less of a device than the original "rom" node was... well, who cares now? :-)
Some partitions can be useful for the firmware itself, or for early boot stages; those should be described in the device tree in some way. And yes, you certainly can consider an (aligned) flash partition to be a subdevice of the whole flash bank.
Oh, I see that the new partition representation have really simplified parsing -- this function is hardly shorter than the old one... :-P
Neither simplifying machine-parsing nor compacting the kernel code are a goal at all, I don't see why you bring this up.
quoted
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
No, it wouldn't.
quoted
large-flash@2,0 {Hmm... what does @2,0 mean? :-Oquoted
reg = <2 0 400000>;
"2,0" is the text representation for the unit address <2 0> on this bus. Segher