From: David Gibson <hidden> Date: 2007-05-01 05:18:04
powerpc_flash_init(), the only function in arch/powerpc/sysdev/rom.c,
goes through the device tree finding anything with device_type=="rom"
and creating of_platform devices for them, which will be picked up by
the physmap_of mtd driver. This has two serious conceptual errors and
one bad implementation error which is quite an accomplishment for 15
lines of code.
Most seriously, this "find all roms" approach to probing is
fundamentally incompatible with the normal way of probing for
of_platform devices, to wit, using of_platform_bus_probe(). If a
flash is on a bus probed with of_platform_bus_probe()
powerpc_flash_init() will create a duplicate of_platform device for
it. powerpc_flash_init() could also mistakenly probe roms which
appear on other random busses which should use their own probe logic
instead of going straight off the device tree (admittedly flash is
unlikely to appear on such a bus).
Also, it uses the device node's name without unit address as the
of_platform device's name. So if a bus somewhere has two flash
devices named, say "flash@0" and "flash@800000", the device code will
give a stack dump during boot as powerpc_flash_init() attempts to
register them both under the name "flash".
I observe that none of the dts files actually present in the kernel
tree use physmap_of's format for describing flash devices (and
therefore don't use this code). I'm therefore rather tempted to
simply blow arch/powerpc/sysdev/rom.c away, and anyone out-of-tree
relying on this code will have to fix their platform probing code to
create the flash of_platform devices properly.
Unless someone who actually knows how this code was intended to be
used can suggest a more polite way of fixing it.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
On Tue, 1 May 2007 15:18:04 +1000
David Gibson wrote:
powerpc_flash_init(), the only function in arch/powerpc/sysdev/rom.c,
goes through the device tree finding anything with device_type=="rom"
and creating of_platform devices for them, which will be picked up by
the physmap_of mtd driver. This has two serious conceptual errors and
one bad implementation error which is quite an accomplishment for 15
lines of code.
It should be rewritten then - the way it does init is obsoleted by
the of_platform_bus_probe() now and is confusion-prone. Have to admit I missed
this part while reviewing that rom of_device patch.
[snip]
Unless someone who actually knows how this code was intended to be
used can suggest a more polite way of fixing it.
I guess, the idea was for this stuff to be updated once one of the dts inside boot/ would have physmap nodes
added. I have rom/physmap[dts] rehaul in my TODO list, but it has (so far at least) little chance to happen during this merge window. Yet, if someone has suggestions and/or some interest for this to be cured, it will gain priority. Otherwise, I'll replace actual erroneous code with kind of rant that it's up to BSP code to take care of of_devices to be registered, using of_platform_bus_probe() or other way.
--
Sincerely, Vitaly
From: David Gibson <hidden> Date: 2007-05-03 07:03:58
On Thu, May 03, 2007 at 10:35:34AM +0400, Vitaly Bordug wrote:
On Tue, 1 May 2007 15:18:04 +1000
David Gibson wrote:
quoted
powerpc_flash_init(), the only function in arch/powerpc/sysdev/rom.c,
goes through the device tree finding anything with device_type=="rom"
and creating of_platform devices for them, which will be picked up by
the physmap_of mtd driver. This has two serious conceptual errors and
one bad implementation error which is quite an accomplishment for 15
lines of code.
It should be rewritten then - the way it does init is obsoleted by
the of_platform_bus_probe() now and is confusion-prone. Have to
admit I missed this part while reviewing that rom of_device patch.
Well, I don't see that it needs rewriting as a unit at all. The
correct place for probing is in the platform code, no extra rom.c
necessary.
[snip]
quoted
Unless someone who actually knows how this code was intended to be
used can suggest a more polite way of fixing it.
I guess, the idea was for this stuff to be updated once one of the
dts inside boot/ would have physmap nodes added. I have
rom/physmap[dts] rehaul in my TODO list, but it has (so far at
least) little chance to happen during this merge window. Yet, if
someone has suggestions and/or some interest for this to be cured,
it will gain priority. Otherwise, I'll replace actual erroneous code
with kind of rant that it's up to BSP code to take care of
of_devices to be registered, using of_platform_bus_probe() or other
way.
I'm having some trouble parsing that paragraph. At this stage I don't
see any reason to hold off on tearing out arch/powerpc/sysdev/rom.c,
any necessary changes to replace it will go in the platform code or
other places.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
From: Sergei Shtylyov <hidden> Date: 2007-05-03 11:46:01
Hello.
David Gibson wrote:
powerpc_flash_init(), the only function in arch/powerpc/sysdev/rom.c,
goes through the device tree finding anything with device_type=="rom"
and creating of_platform devices for them, which will be picked up by
the physmap_of mtd driver. This has two serious conceptual errors and
one bad implementation error which is quite an accomplishment for 15
lines of code.
Most seriously, this "find all roms" approach to probing is
fundamentally incompatible with the normal way of probing for
of_platform devices, to wit, using of_platform_bus_probe(). If a
We weren't aware of the of_platform.c work when writing the MTD support.
Note that this function usually probes only the specified set of (SoC)
busses, none of which usully contains NOR flash (which is located at the root
level).
flash is on a bus probed with of_platform_bus_probe()
powerpc_flash_init() will create a duplicate of_platform device for it.
Flash on a SoC bus? Well, that's more likely to happen for NAND.
But generally, I'd agree with you.
powerpc_flash_init() could also mistakenly probe roms which
appear on other random busses which should use their own probe logic
instead of going straight off the device tree (admittedly flash is
unlikely to appear on such a bus).
Well, if you consider NAND...
Also, it uses the device node's name without unit address as the
of_platform device's name. So if a bus somewhere has two flash
devices named, say "flash@0" and "flash@800000", the device code will
give a stack dump during boot as powerpc_flash_init() attempts to
register them both under the name "flash".
Well, we didn't think about 2 flashes named the same way. :-/
I observe that none of the dts files actually present in the kernel
tree use physmap_of's format for describing flash devices (and
therefore don't use this code). I'm therefore rather tempted to
Which means I still haven't submitted the patch. :-<
simply blow arch/powerpc/sysdev/rom.c away, and anyone out-of-tree
relying on this code will have to fix their platform probing code to
create the flash of_platform devices properly.
You mean creating the "rom" devices from the platform-specific code?
I doubt that it's really a flexible approach...
Unless someone who actually knows how this code was intended to be
used can suggest a more polite way of fixing it.
Well, probably it needs to only look up the root bus...
WBR, Sergei
From: Sergei Shtylyov <hidden> Date: 2007-05-03 12:00:36
Hello.
David Gibson wrote:
quoted
quoted
Unless someone who actually knows how this code was intended to be
used can suggest a more polite way of fixing it.
quoted
I guess, the idea was for this stuff to be updated once one of the
dts inside boot/ would have physmap nodes added. I have
rom/physmap[dts] rehaul in my TODO list, but it has (so far at
least) little chance to happen during this merge window. Yet, if
someone has suggestions and/or some interest for this to be cured,
it will gain priority. Otherwise, I'll replace actual erroneous code
with kind of rant that it's up to BSP code to take care of
of_devices to be registered, using of_platform_bus_probe() or other
way.
I'm having some trouble parsing that paragraph. At this stage I don't
see any reason to hold off on tearing out arch/powerpc/sysdev/rom.c,
any necessary changes to replace it will go in the platform code or
It doesn't seem a flexible enough approach. We could continue using
platform devices then.
From: David Gibson <hidden> Date: 2007-05-03 12:22:13
On Thu, May 03, 2007 at 04:02:12PM +0400, Sergei Shtylyov wrote:
Hello.
David Gibson wrote:
quoted
quoted
quoted
Unless someone who actually knows how this code was intended to be
used can suggest a more polite way of fixing it.
quoted
quoted
I guess, the idea was for this stuff to be updated once one of the
dts inside boot/ would have physmap nodes added. I have
rom/physmap[dts] rehaul in my TODO list, but it has (so far at
least) little chance to happen during this merge window. Yet, if
someone has suggestions and/or some interest for this to be cured,
it will gain priority. Otherwise, I'll replace actual erroneous code
with kind of rant that it's up to BSP code to take care of
of_devices to be registered, using of_platform_bus_probe() or other
way.
quoted
I'm having some trouble parsing that paragraph. At this stage I don't
see any reason to hold off on tearing out arch/powerpc/sysdev/rom.c,
any necessary changes to replace it will go in the platform code or
It doesn't seem a flexible enough approach. We could continue using
platform devices then.
Sorry, I don't follow you.
quoted
other places.
Any ideas where else?
Not really. I don't immediately see a case where doing it from the
platform code wouldn't be right.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: 2007-05-03 12:29:13
It doesn't seem a flexible enough approach. We could continue using
platform devices then.
The problem is trivial enough tho... who ends up creating an
of_platform_device for that rom node ... to be picked up by the driver.
Creating of_platform devices is platform code responsibility... either
by calling of_platform_bus_probe() (or whatever I called it ...) to
generate them from known bus type or by creating them directly, that
doesn't matter, that's still platform code business.
Ben.
From: David Gibson <hidden> Date: 2007-05-03 12:30:55
On Thu, May 03, 2007 at 03:47:36PM +0400, Sergei Shtylyov wrote:
Hello.
David Gibson wrote:
quoted
powerpc_flash_init(), the only function in arch/powerpc/sysdev/rom.c,
goes through the device tree finding anything with device_type=="rom"
and creating of_platform devices for them, which will be picked up by
the physmap_of mtd driver. This has two serious conceptual errors and
one bad implementation error which is quite an accomplishment for 15
lines of code.
quoted
Most seriously, this "find all roms" approach to probing is
fundamentally incompatible with the normal way of probing for
of_platform devices, to wit, using of_platform_bus_probe(). If a
We weren't aware of the of_platform.c work when writing the MTD support.
Note that this function usually probes only the specified set of (SoC)
busses, none of which usully contains NOR flash (which is located at the
root level).
The root level? Um... I don't think so...
quoted
flash is on a bus probed with of_platform_bus_probe()
powerpc_flash_init() will create a duplicate of_platform device for it.
Flash on a SoC bus? Well, that's more likely to happen for NAND.
But generally, I'd agree with you.
Well, on Ebony, the (NOR) flash is on the bus controlled by the
440gp's external bus controller (EBC). So it's not an SoC bus as
such, but it's still a "dumb bus" (to use BenH's terminology) which
can be suitably probed by of_platform_bus_probe().
I believe the arrangement is similar for most other 4xx systems. More
PC or desktop like systems sometimes have boot flash connected to the
south bridge, which I believe puts it on the ISA bus, topologically
speaking.
quoted
powerpc_flash_init() could also mistakenly probe roms which
appear on other random busses which should use their own probe logic
instead of going straight off the device tree (admittedly flash is
unlikely to appear on such a bus).
Well, if you consider NAND...
quoted
Also, it uses the device node's name without unit address as the
of_platform device's name. So if a bus somewhere has two flash
devices named, say "flash@0" and "flash@800000", the device code will
give a stack dump during boot as powerpc_flash_init() attempts to
register them both under the name "flash".
Well, we didn't think about 2 flashes named the same way. :-/
quoted
I observe that none of the dts files actually present in the kernel
tree use physmap_of's format for describing flash devices (and
therefore don't use this code). I'm therefore rather tempted to
Which means I still haven't submitted the patch. :-<
quoted
simply blow arch/powerpc/sysdev/rom.c away, and anyone out-of-tree
relying on this code will have to fix their platform probing code to
create the flash of_platform devices properly.
You mean creating the "rom" devices from the platform-specific code?
I doubt that it's really a flexible approach...
Since it's handled on a per-platform basis, it's more-or-less by
definition more flexible than the current broken approach.
quoted
Unless someone who actually knows how this code was intended to be
used can suggest a more polite way of fixing it.
Well, probably it needs to only look up the root bus...
Really, truly on the root bus? Even so I don't think such a probe
should be conducted as an initcall whenever CONFIG_MTD is set. A
helper function invoked from the platform code might be reasonable.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
From: Sergei Shtylyov <hidden> Date: 2007-05-03 13:02:58
Hello.
David Gibson wrote:
quoted
quoted
powerpc_flash_init(), the only function in arch/powerpc/sysdev/rom.c,
goes through the device tree finding anything with device_type=="rom"
and creating of_platform devices for them, which will be picked up by
the physmap_of mtd driver. This has two serious conceptual errors and
one bad implementation error which is quite an accomplishment for 15
lines of code.
quoted
quoted
Most seriously, this "find all roms" approach to probing is
fundamentally incompatible with the normal way of probing for
of_platform devices, to wit, using of_platform_bus_probe(). If a
quoted
We weren't aware of the of_platform.c work when writing the MTD support.
Note that this function usually probes only the specified set of (SoC)
busses, none of which usully contains NOR flash (which is located at the
root level).
The root level? Um... I don't think so...
"Trust me". :-)
NOR flashes are at the same level as the "memory" node (where else you
expect them to appear I wonder?).
quoted
quoted
flash is on a bus probed with of_platform_bus_probe()
powerpc_flash_init() will create a duplicate of_platform device for it.
quoted
Flash on a SoC bus? Well, that's more likely to happen for NAND.
But generally, I'd agree with you.
Well, on Ebony, the (NOR) flash is on the bus controlled by the
440gp's external bus controller (EBC). So it's not an SoC bus as
such, but it's still a "dumb bus" (to use BenH's terminology) which
can be suitably probed by of_platform_bus_probe().
Interesting...
I believe the arrangement is similar for most other 4xx systems. More
PC or desktop like systems sometimes have boot flash connected to the
south bridge, which I believe puts it on the ISA bus, topologically
speaking.
Not exactly. Boot flash is mapped beyond ISA address space on 386+ -- at
the top of 4GB (where the "reset vector" is). Although it may be dual mapped
below 1MB as well (I'm starting to forget x86 :-).
quoted
quoted
powerpc_flash_init() could also mistakenly probe roms which
appear on other random busses which should use their own probe logic
instead of going straight off the device tree (admittedly flash is
unlikely to appear on such a bus).
quoted
Well, if you consider NAND...
quoted
quoted
Also, it uses the device node's name without unit address as the
of_platform device's name. So if a bus somewhere has two flash
devices named, say "flash@0" and "flash@800000", the device code will
give a stack dump during boot as powerpc_flash_init() attempts to
register them both under the name "flash".
quoted
Well, we didn't think about 2 flashes named the same way. :-/
quoted
quoted
I observe that none of the dts files actually present in the kernel
tree use physmap_of's format for describing flash devices (and
therefore don't use this code). I'm therefore rather tempted to
quoted
Which means I still haven't submitted the patch. :-<
I hope to post a patch soon.
quoted
quoted
simply blow arch/powerpc/sysdev/rom.c away, and anyone out-of-tree
relying on this code will have to fix their platform probing code to
create the flash of_platform devices properly.
quoted
You mean creating the "rom" devices from the platform-specific code?
I doubt that it's really a flexible approach...
Since it's handled on a per-platform basis, it's more-or-less by
definition more flexible than the current broken approach.
I'm worried about the code duplication.
quoted
quoted
Unless someone who actually knows how this code was intended to be
used can suggest a more polite way of fixing it.
quoted
Well, probably it needs to only look up the root bus...
Really, truly on the root bus? Even so I don't think such a probe
should be conducted as an initcall whenever CONFIG_MTD is set. A
helper function invoked from the platform code might be reasonable.
Yeah, I agree. Probably doesn't even worth a function since for most
cases there's only one flash.
WBR, Sergei
From: Sergei Shtylyov <hidden> Date: 2007-05-03 13:27:08
Hello.
David Gibson wrote:
quoted
quoted
quoted
quoted
Unless someone who actually knows how this code was intended to be
used can suggest a more polite way of fixing it.
quoted
quoted
quoted
I guess, the idea was for this stuff to be updated once one of the
dts inside boot/ would have physmap nodes added. I have
rom/physmap[dts] rehaul in my TODO list, but it has (so far at
least) little chance to happen during this merge window. Yet, if
someone has suggestions and/or some interest for this to be cured,
it will gain priority. Otherwise, I'll replace actual erroneous code
with kind of rant that it's up to BSP code to take care of
of_devices to be registered, using of_platform_bus_probe() or other
way.
quoted
quoted
I'm having some trouble parsing that paragraph. At this stage I don't
see any reason to hold off on tearing out arch/powerpc/sysdev/rom.c,
any necessary changes to replace it will go in the platform code or
quoted
It doesn't seem a flexible enough approach. We could continue using
platform devices then.
Sorry, I don't follow you.
Well, IIUC, the idea behind the device is to free the platform code of as
much burden of registering the platform devices itself as possible, isn't it?
quoted
quoted
other places.
quoted
Any ideas where else?
Not really. I don't immediately see a case where doing it from the
platform code wouldn't be right.
BTW, is it legal/appropriate to specify device (not bus) types for
of_platform_bus_probe()?
WBR, Sergei
We weren't aware of the of_platform.c work when writing the MTD
support.
Note that this function usually probes only the specified set of
(SoC)
busses, none of which usully contains NOR flash (which is located at
the
root level).
quoted
The root level? Um... I don't think so...
"Trust me". :-)
NOR flashes are at the same level as the "memory" node (where else
you
expect them to appear I wonder?).
The "memory" node doesn't describe the RAM devices;
it describes the RAM address space, instead. You can
have separate nodes for the actual devices.
Now for ROM/flash/NVRAM, nodes _can_ appear directly
under the root, but only if that is where they belong
on your platform (i.e., they sit directly on the "system
bus" (whatever that means on your platform); on most
platforms though, such devices are connected via some
I/O busses, so the nodes should appear under their
respective controllers.
quoted
I believe the arrangement is similar for most other 4xx systems. More
PC or desktop like systems sometimes have boot flash connected to the
south bridge, which I believe puts it on the ISA bus, topologically
speaking.
Some have it on the LPC bus as an LPC device, some
have it on the LPC bus but accessed with a separate
protocol, some have it attached to another LPC device
(some "superio" typically), some have it attached
directly to the "south bridge".
Not exactly. Boot flash is mapped beyond ISA address space on 386+
-- at
the top of 4GB (where the "reset vector" is). Although it may be dual
mapped
below 1MB as well (I'm starting to forget x86 :-).
Most "north bridges" have some bits that enable
translation of accesses in the "low bios" area to
the 4GB-minus-a-bit area. There are many variations
and it all is a big mess :-)
Now, back to the case at hand -- it would be nice to
have a platform-independent way to probe the simple
case -- a single direct-mapped device -- but it isn't
obvious how to make that not clash with the not-so-simple
cases. A helper function that does the work but is
only called by the platforms that want it would do, I
suppose?
Segher
From: Sergei Shtylyov <hidden> Date: 2007-05-03 17:16:18
Hello.
Segher Boessenkool wrote:
quoted
quoted
quoted
We weren't aware of the of_platform.c work when writing the MTD
support.
Note that this function usually probes only the specified set of
(SoC)
busses, none of which usully contains NOR flash (which is located at
the
root level).
quoted
quoted
The root level? Um... I don't think so...
quoted
"Trust me". :-)
NOR flashes are at the same level as the "memory" node (where else
you
expect them to appear I wonder?).
The "memory" node doesn't describe the RAM devices;
it describes the RAM address space, instead. You can
have separate nodes for the actual devices.
If you can remember our prior discussion, the "rom" nodes don't describe
"the actual devices" as well, only their mapping into the address space. ;-)
Now for ROM/flash/NVRAM, nodes _can_ appear directly
under the root, but only if that is where they belong
on your platform (i.e., they sit directly on the "system
bus" (whatever that means on your platform); on most
platforms though, such devices are connected via some
I/O busses, so the nodes should appear under their
respective controllers.
Yeah, you're right here, and I've probably misunderstood what "memory"
node was. In fact, the flash in my system resides on the same local bus as
RAM, so the proper place would be behind the "lbc" (or whatever -- it doesn't
exist as yet) node on the "soc" bus. Do you think I need to go and document
it as well for such cause? :-]
quoted
quoted
I believe the arrangement is similar for most other 4xx systems. More
PC or desktop like systems sometimes have boot flash connected to the
south bridge, which I believe puts it on the ISA bus, topologically
speaking.
Some have it on the LPC bus as an LPC device, some
have it on the LPC bus but accessed with a separate
protocol, some have it attached to another LPC device
(some "superio" typically), some have it attached
directly to the "south bridge".
quoted
Not exactly. Boot flash is mapped beyond ISA address space on 386+
-- at
the top of 4GB (where the "reset vector" is). Although it may be dual
mapped
below 1MB as well (I'm starting to forget x86 :-).
Most "north bridges" have some bits that enable
translation of accesses in the "low bios" area to
the 4GB-minus-a-bit area. There are many variations
and it all is a big mess :-)
Human perversion knows no limits. O:-)
Now, back to the case at hand -- it would be nice to
have a platform-independent way to probe the simple
case -- a single direct-mapped device -- but it isn't
obvious how to make that not clash with the not-so-simple
cases. A helper function that does the work but is
only called by the platforms that want it would do, I
suppose?
It probably doesn't even worth a helper (since out of those 15 lines, 6
were pretty useless anyway)
BTW, is it legal/appropriate to specify device (not bus) types
for
of_platform_bus_probe()?
quoted
In almost all cases you should probe on "name"/"compatible",
Probing on name is not a good idea, since those are mostly generic.
The OF standard tells you to probe on "name" before
probing on "compatible". If the "name" in a certain
node is generic, that doesn't harm at all, since you
probe for the more specific names only.
quoted
and not use "device_type" at all.
Heh, SPARC device trees I saw seem to not have this "useless" prop
at all. :-)
Well they have it in the nodes for which they _should_
have it, but not anywhere else no.
Segher
From: Sergei Shtylyov <hidden> Date: 2007-05-03 17:51:46
Hello, I wrote:
quoted
Now for ROM/flash/NVRAM, nodes _can_ appear directly
under the root, but only if that is where they belong
on your platform (i.e., they sit directly on the "system
bus" (whatever that means on your platform); on most
platforms though, such devices are connected via some
I/O busses, so the nodes should appear under their
respective controllers.
Yeah, you're right here, and I've probably misunderstood what "memory"
node was. In fact, the flash in my system resides on the same local bus as
RAM, so the proper place would be behind the "lbc" (or whatever -- it doesn't
exist as yet) node on the "soc" bus. Do you think I need to go and document
it as well for such cause? :-]
No, that probably won't do. MPC85xx SoC bus has ranges = <e0000000
00100000> and the NOR flash is mapped at 0xff000000, so it seems that it can't
be located under the "soc" bus (unless that latter has "ranges" prop extended?).
Yeah, you're right here, and I've probably misunderstood what
"memory" node was. In fact, the flash in my system resides on the
same local bus as RAM, so the proper place would be behind the "lbc"
(or whatever -- it doesn't exist as yet) node on the "soc" bus. Do
you think I need to go and document it as well for such cause? :-]
No, that probably won't do. MPC85xx SoC bus has ranges = <e0000000
00100000> and the NOR flash is mapped at 0xff000000, so it seems that
it can't be located under the "soc" bus (unless that latter has
"ranges" prop extended?).
If the RAM and/or ROM sit on the SoC bus, the "ranges"
property in the SoC node should be able to translate
their addresses, yes. You could opt for having the
memory controller a separate device node, as a sibling
of the "soc" node, if that agrees better with your
SoC architecture. "It all depends".
Segher
NOR flashes are at the same level as the "memory" node (where
else you
expect them to appear I wonder?).
quoted
The "memory" node doesn't describe the RAM devices;
it describes the RAM address space, instead. You can
have separate nodes for the actual devices.
If you can remember our prior discussion, the "rom" nodes don't
describe "the actual devices" as well, only their mapping into the
address space. ;-)
I don't remember that no. And having a node for the
"ROM address space" isn't useful in the same way as
having one for the "RAM address space" is -- flash
memory is not a resource you randomly hand out to
anyone who wants a piece. You also need to know some
_specifics_ about a certain ROM device before you can
map it into CPU address space properly.
quoted
Now for ROM/flash/NVRAM, nodes _can_ appear directly
under the root, but only if that is where they belong
on your platform (i.e., they sit directly on the "system
bus" (whatever that means on your platform); on most
platforms though, such devices are connected via some
I/O busses, so the nodes should appear under their
respective controllers.
Yeah, you're right here, and I've probably misunderstood what
"memory" node was. In fact, the flash in my system resides on the same
local bus as RAM, so the proper place would be behind the "lbc" (or
whatever -- it doesn't exist as yet) node on the "soc" bus. Do you
think I need to go and document it as well for such cause? :-]
If the "lbc" isn't software visible, you can/should put
the RAM/ROM nodes directly under the SoC node.
This is all just standard considerations, so I don't think
you need to document it separately no. An example device
tree will help other implementors using your SoC create a
proper device tree, of course.
quoted
Most "north bridges" have some bits that enable
translation of accesses in the "low bios" area to
the 4GB-minus-a-bit area. There are many variations
and it all is a big mess :-)
Human perversion knows no limits. O:-)
Well it's note like there aren't any groovy things on
some PowerPC systems, but x86 definitely wins :-)
quoted
Now, back to the case at hand -- it would be nice to
have a platform-independent way to probe the simple
case -- a single direct-mapped device -- but it isn't
obvious how to make that not clash with the not-so-simple
cases. A helper function that does the work but is
only called by the platforms that want it would do, I
suppose?
It probably doesn't even worth a helper (since out of those 15
lines, 6 were pretty useless anyway)
Sure -- but since it is such a common device to have (a
simple NOR boot flash), it would be nice to avoid any
code duplication. Compare to the serial port and RTC
situation.
Segher
From: Sergei Shtylyov <hidden> Date: 2007-05-03 18:17:43
Hello.
Segher Boessenkool wrote:
quoted
quoted
quoted
NOR flashes are at the same level as the "memory" node (where
else you
expect them to appear I wonder?).
quoted
quoted
The "memory" node doesn't describe the RAM devices;
it describes the RAM address space, instead. You can
have separate nodes for the actual devices.
quoted
If you can remember our prior discussion, the "rom" nodes don't
describe "the actual devices" as well, only their mapping into the
address space. ;-)
I don't remember that no. And having a node for the
That's a pity. :-)
"ROM address space" isn't useful in the same way as
having one for the "RAM address space" is -- flash
memory is not a resource you randomly hand out to
anyone who wants a piece. You also need to know some
_specifics_ about a certain ROM device before you can
map it into CPU address space properly.
Almost all of that is handled by MTD subsys transparently by probing.
What one *must* supply are the bank width and the address mapping (may
optionally supply a probe type).
quoted
quoted
Now for ROM/flash/NVRAM, nodes _can_ appear directly
under the root, but only if that is where they belong
on your platform (i.e., they sit directly on the "system
bus" (whatever that means on your platform); on most
platforms though, such devices are connected via some
I/O busses, so the nodes should appear under their
respective controllers.
quoted
Yeah, you're right here, and I've probably misunderstood what
"memory" node was. In fact, the flash in my system resides on the same
local bus as RAM, so the proper place would be behind the "lbc" (or
whatever -- it doesn't exist as yet) node on the "soc" bus. Do you
think I need to go and document it as well for such cause? :-]
If the "lbc" isn't software visible, you can/should put
the RAM/ROM nodes directly under the SoC node.
It has a register set of its own.
quoted
quoted
Now, back to the case at hand -- it would be nice to
have a platform-independent way to probe the simple
case -- a single direct-mapped device -- but it isn't
obvious how to make that not clash with the not-so-simple
cases. A helper function that does the work but is
only called by the platforms that want it would do, I
suppose?
quoted
It probably doesn't even worth a helper (since out of those 15
lines, 6 were pretty useless anyway)
Sure -- but since it is such a common device to have (a
simple NOR boot flash), it would be nice to avoid any
code duplication. Compare to the serial port and RTC
situation.
UARTs should be registered as of_device by the same bus probing mechanism
(and there was an attempt at OF based driver, IIRC).
arch/powerpc/kernel/legacy_serial.c only facilitates the old, platform device
based approach.
From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: 2007-05-03 21:44:33
UARTs should be registered as of_device by the same bus probing mechanism
(and there was an attempt at OF based driver, IIRC).
There is more than an "attempt". There is one and it works :-)
arch/powerpc/kernel/legacy_serial.c only facilitates the old, platform device
based approach.
This is a compromise for legacy port (ok, it did grow a bit beyond
legacy stuff but still...) mostly to allow for very early initialisation
of serial ports.
Ultimately, if we generalize of_platform_devices for serial ports,
however, we will not need to create platform devices there anymore.
Ben.
From: David Gibson <hidden> Date: 2007-05-03 23:49:19
On Thu, May 03, 2007 at 06:21:35PM +0200, Segher Boessenkool wrote:
quoted
BTW, is it legal/appropriate to specify device (not bus) types for
of_platform_bus_probe()?
In almost all cases you should probe on "name"/"compatible",
and not use "device_type" at all.
I don't think that's what he's getting at. "compatible" is what we
should use for the device <-> driver matching, but
of_platform_bus_probe() is earlier than that, creating the devices in
the first place.
And no, I don't think it is appropriate to specify device types rather
than bus types there. Probing devices independent of the bus type
they sit on will always run the risk of conflicting with the bus
type's specific probe logic.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
From: David Gibson <hidden> Date: 2007-05-03 23:56:20
On Thu, May 03, 2007 at 08:07:27PM +0200, Segher Boessenkool wrote:
quoted
quoted
Yeah, you're right here, and I've probably misunderstood what
"memory" node was. In fact, the flash in my system resides on the
same local bus as RAM, so the proper place would be behind the "lbc"
(or whatever -- it doesn't exist as yet) node on the "soc" bus. Do
you think I need to go and document it as well for such cause? :-]
No, that probably won't do. MPC85xx SoC bus has ranges = <e0000000
00100000> and the NOR flash is mapped at 0xff000000, so it seems that
it can't be located under the "soc" bus (unless that latter has
"ranges" prop extended?).
If the RAM and/or ROM sit on the SoC bus, the "ranges"
property in the SoC node should be able to translate
their addresses, yes. You could opt for having the
memory controller a separate device node, as a sibling
of the "soc" node, if that agrees better with your
SoC architecture. "It all depends".
But if the flash really is on an external bus controlled by a bus
controller on the SoC, it sounds like it should go under that bus
bridge. In which case the SoC would need another range in its ranges
property.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
On Thu, 03 May 2007 22:29:13 +1000
Benjamin Herrenschmidt wrote:
quoted
It doesn't seem a flexible enough approach. We could continue
using platform devices then.
The problem is trivial enough tho... who ends up creating an
of_platform_device for that rom node ... to be picked up by the
driver.
Creating of_platform devices is platform code responsibility... either
by calling of_platform_bus_probe() (or whatever I called it ...) to
generate them from known bus type or by creating them directly, that
doesn't matter, that's still platform code business.
Still, there is code duplication problem.
So, assuming we'd need 6 lines of code to do it properly in platform code,
we may face tens of different targets that require exactly the same process to
provide mtd of_devices stuff.
The hard part of it is apparently how the respective flash space should be remapped
(b/c it is depending on HW design), but since we really care only about things that may be
encoded into device tree, there is really an option to utilize devtree power and effectiveness,
I think.
--
Sincerely, Vitaly
From: David Gibson <hidden> Date: 2007-05-04 01:28:35
On Fri, May 04, 2007 at 04:30:56AM +0400, Vitaly Bordug wrote:
On Thu, 03 May 2007 22:29:13 +1000
Benjamin Herrenschmidt wrote:
quoted
quoted
It doesn't seem a flexible enough approach. We could continue
using platform devices then.
The problem is trivial enough tho... who ends up creating an
of_platform_device for that rom node ... to be picked up by the
driver.
Creating of_platform devices is platform code responsibility... either
by calling of_platform_bus_probe() (or whatever I called it ...) to
generate them from known bus type or by creating them directly, that
doesn't matter, that's still platform code business.
Still, there is code duplication problem.
So, assuming we'd need 6 lines of code to do it properly in platform
code, we may face tens of different targets that require exactly the
same process to provide mtd of_devices stuff.
So make a helper function.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
If the RAM and/or ROM sit on the SoC bus, the "ranges"
property in the SoC node should be able to translate
their addresses, yes. You could opt for having the
memory controller a separate device node, as a sibling
of the "soc" node, if that agrees better with your
SoC architecture. "It all depends".
But if the flash really is on an external bus controlled by a bus
controller on the SoC, it sounds like it should go under that bus
bridge. In which case the SoC would need another range in its ranges
property.
Yeah, it all depends on the specific SoC device.
I don't know which one we're talking about here
so I can't give any more specific advice.
Segher
From: Sergei Shtylyov <hidden> Date: 2007-05-05 17:35:03
Hello.
David Gibson wrote:
quoted
quoted
quoted
Yeah, you're right here, and I've probably misunderstood what
"memory" node was. In fact, the flash in my system resides on the
same local bus as RAM, so the proper place would be behind the "lbc"
(or whatever -- it doesn't exist as yet) node on the "soc" bus. Do
you think I need to go and document it as well for such cause? :-]
quoted
quoted
No, that probably won't do. MPC85xx SoC bus has ranges = <e0000000
00100000> and the NOR flash is mapped at 0xff000000, so it seems that
it can't be located under the "soc" bus (unless that latter has
"ranges" prop extended?).
quoted
If the RAM and/or ROM sit on the SoC bus, the "ranges"
property in the SoC node should be able to translate
their addresses, yes. You could opt for having the
memory controller a separate device node, as a sibling
of the "soc" node, if that agrees better with your
SoC architecture. "It all depends".
But if the flash really is on an external bus controlled by a bus
controller on the SoC, it sounds like it should go under that bus
bridge. In which case the SoC would need another range in its ranges
property.
Erm, how multiple memory ranges are supposed to work? Aren't the addresses
in the "reg" property of subnodes relative to the "ranges" property?
WBR, Sergei
But if the flash really is on an external bus controlled by a bus
controller on the SoC, it sounds like it should go under that bus
bridge. In which case the SoC would need another range in its ranges
property.
Erm, how multiple memory ranges are supposed to work? Aren't the
addresses in the "reg" property of subnodes relative to the "ranges"
property?
If the address space of a child bus is a subset (perhaps
after some mapping) of the address space of the parent bus,
the "ranges" property in the parent node describes that
subset (and mapping). If the address spaces are identical,
this is expressed by an empty "ranges" property. If there
is no such mapping (indirect register access, for example),
this is expressed by the absence of a "ranges" property.
There can be multiple ranges in a "ranges" property, that's
why it's called "ranges" and not "range". A common example
is the "ranges" property in PCI bus nodes (if those have
both a memory and legacy I/O range; for host bridges, this
means that both those ranges are direct mapped).
I hope this answered your question, it was a bit vague.
Segher