attached.
ron
ron minnich wrote:
I misunderstood the dts when I wrote this code. The @ values are implicitly hex. Correctly emit hex constants for all cases, including the pci a,b syntax.
Tested on dbe62. Note that before this patch goes in we need to change ALL dts files to be hex.
Also a small buglet: fix the strcmp on ioport to match 6, not 3, chars.
Signed-off-by: Ronald G. Minnich rminnich@gmail.com
Acked-by: Stefan Reinauer stepan@coresystems.de
On 29.07.2008 18:21, ron minnich wrote:
attached.
ron
I misunderstood the dts when I wrote this code. The @ values are implicitly hex. Correctly emit hex constants for all cases, including the pci a,b syntax.
Tested on dbe62. Note that before this patch goes in we need to change ALL dts files to be hex.
Also a small buglet: fix the strcmp on ioport to match 6, not 3, chars.
Signed-off-by: Ronald G. Minnich rminnich@gmail.com
AFAICS this will break all targets unless each dts is fixed as well. If you include the fixed dts for each board, the patch is Acked-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
./mainboard/adl/msm800sev/dts ./mainboard/amd/db800/dts ./mainboard/amd/norwich/dts ./mainboard/artecgroup/dbe61/dts ./mainboard/artecgroup/dbe62/dts ./mainboard/emulation/qemu-x86/dts ./mainboard/pcengines/alix1c/dts ./mainboard/pcengines/alix2c3/dts
Regards, Carl-Daniel
Carl-Daniel Hailfinger wrote:
AFAICS this will break all targets unless each dts is fixed as well. If you include the fixed dts for each board, the patch is Acked-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
./mainboard/adl/msm800sev/dts ./mainboard/amd/db800/dts ./mainboard/amd/norwich/dts ./mainboard/artecgroup/dbe61/dts ./mainboard/artecgroup/dbe62/dts ./mainboard/emulation/qemu-x86/dts ./mainboard/pcengines/alix1c/dts ./mainboard/pcengines/alix2c3/dts
and possibly a few more files like apic, ide and whatever they're called..
To make clear what those files are, we should rename them...
dts -> mainboard.dts ide -> ide.dts apic -> apic.dts
Stefan
On Tue, Jul 29, 2008 at 09:21:17AM -0700, ron minnich wrote:
I misunderstood the dts when I wrote this code. The @ values are implicitly hex. Correctly emit hex constants for all cases, including the pci a,b syntax.
Great patch! I just have one general thought..
fprintf(f, "\t.path = {.type=DEVICE_PATH_CPU,.u={.cpu={ .id = %s }}},\n",
fprintf(f, "\t.path = {.type=DEVICE_PATH_CPU,.u={.cpu={ .id = 0x%s }}},\n",
Shouldn't dtc actually try to convert these into scalars rather than storing them as strings, so that errors are caught earlier and better error messages can be output?
On Tue, Jul 29, 2008 at 07:58:22PM +0200, Stefan Reinauer wrote:
and possibly a few more files like apic, ide and whatever they're called..
To make clear what those files are, we should rename them...
dts -> mainboard.dts ide -> ide.dts apic -> apic.dts
Yes, I argued strongly for this when they first appeared and even sent a patch. The problem is that the filenames are tied hard into the struct names generated by dtc.
//Peter
On Tue, Jul 29, 2008 at 12:50 PM, Peter Stuge peter@stuge.se wrote:
On Tue, Jul 29, 2008 at 09:21:17AM -0700, ron minnich wrote:
I misunderstood the dts when I wrote this code. The @ values are implicitly hex. Correctly emit hex constants for all cases, including the pci a,b syntax.
Great patch! I just have one general thought..
fprintf(f, "\t.path = {.type=DEVICE_PATH_CPU,.u={.cpu={ .id = %s }}},\n",
fprintf(f, "\t.path = {.type=DEVICE_PATH_CPU,.u={.cpu={ .id = 0x%s }}},\n",Shouldn't dtc actually try to convert these into scalars rather than storing them as strings, so that errors are caught earlier and better error messages can be output?
yes it probably should. That's a future fix.
rest assured I won't commit this unless it includes the commit for the dts changes.
ron
On Tue, Jul 29, 2008 at 12:52:57PM -0700, ron minnich wrote:
rest assured I won't commit this unless it includes the commit for the dts changes.
Go for it! :)
If you need one more;
Acked-by: Peter Stuge peter@stuge.se
Peter Stuge wrote:
On Tue, Jul 29, 2008 at 09:21:17AM -0700, ron minnich wrote:
I misunderstood the dts when I wrote this code. The @ values are implicitly hex. Correctly emit hex constants for all cases, including the pci a,b syntax.
Great patch! I just have one general thought..
fprintf(f, "\t.path = {.type=DEVICE_PATH_CPU,.u={.cpu={ .id = %s }}},\n",
fprintf(f, "\t.path = {.type=DEVICE_PATH_CPU,.u={.cpu={ .id = 0x%s }}},\n",Shouldn't dtc actually try to convert these into scalars rather than storing them as strings, so that errors are caught earlier and better error messages can be output?
On Tue, Jul 29, 2008 at 07:58:22PM +0200, Stefan Reinauer wrote:
and possibly a few more files like apic, ide and whatever they're called..
To make clear what those files are, we should rename them...
dts -> mainboard.dts ide -> ide.dts apic -> apic.dts
Yes, I argued strongly for this when they first appeared and even sent a patch. The problem is that the filenames are tied hard into the struct names generated by dtc.
Hm. I definitely want to support your idea here then.
Adding .dts to the filename is about as hard as not doing that. Also, in 2 out of 3 dts files I see struct names.
If we'd really autocreate something, we should drop that behavior.
Writing a decent device tree got a lot more complicated than it was in v2. And we did not even start adding new features.
v3 ought to be simpler for the user and for the developer of new systems..
we need to focus on that again.
On Tue, Jul 29, 2008 at 1:05 PM, Stefan Reinauer stepan@coresystems.de wrote:
Peter Stuge wrote:
Yes, I argued strongly for this when they first appeared and even sent a patch. The problem is that the filenames are tied hard into the struct names generated by dtc.
Hm. I definitely want to support your idea here then.
Adding .dts to the filename is about as hard as not doing that. Also, in 2 out of 3 dts files I see struct names.
If we'd really autocreate something, we should drop that behavior.
Writing a decent device tree got a lot more complicated than it was in v2. And we did not even start adding new features.
oh no! it's harder! we blew it!
Tell me what to change and let's fix it. V3 is still the Department of Exploding Things. We can get it right.
ron
On 29.07.2008 22:10, ron minnich wrote:
On Tue, Jul 29, 2008 at 1:05 PM, Stefan Reinauer stepan@coresystems.de wrote:
Peter Stuge wrote:
Yes, I argued strongly for this when they first appeared and even sent a patch. The problem is that the filenames are tied hard into the struct names generated by dtc.
Hm. I definitely want to support your idea here then.
Adding .dts to the filename is about as hard as not doing that. Also, in 2 out of 3 dts files I see struct names.
And what about files which are named dts right now? Do we call them dts.dts (ugly) or .dts (hidden file)? Stripping a given file suffix in dtc before creating the struct name is easy.
If we'd really autocreate something, we should drop that behavior.
struct name autocreation is a feature I really like.
Writing a decent device tree got a lot more complicated than it was in v2. And we did not even start adding new features.
oh no! it's harder! we blew it!
I'd like to disagree. I still haven't fully understood the v2 device tree, while the v3 device tree seems obvious and simple to me. The
Tell me what to change and let's fix it. V3 is still the Department of Exploding Things. We can get it right.
v3 has a few perceived problems and a few real problems. The problem is that everybody has his own idea about which problems are real. I'm not claiming that my version of the story is the absolute truth(tm). Do we have a list of v3 design/implementation problems on the wiki somewhere?
Regards, Carl-Daniel
Carl-Daniel Hailfinger wrote:
On 29.07.2008 22:10, ron minnich wrote:
On Tue, Jul 29, 2008 at 1:05 PM, Stefan Reinauer stepan@coresystems.de wrote:
Peter Stuge wrote:
Yes, I argued strongly for this when they first appeared and even sent a patch. The problem is that the filenames are tied hard into the struct names generated by dtc.
Hm. I definitely want to support your idea here then.
Adding .dts to the filename is about as hard as not doing that. Also, in 2 out of 3 dts files I see struct names.
And what about files which are named dts right now? Do we call them dts.dts (ugly) or .dts (hidden file)? Stripping a given file suffix in dtc before creating the struct name is easy.
Read my initial proposal.
If we'd really autocreate something, we should drop that behavior.
struct name autocreation is a feature I really like.
Absolutely. "That behavior" meant mentioning the struct name manually in the dts files.
oh no! it's harder! we blew it!
I'd like to disagree. I still haven't fully understood the v2 device tree, while the v3 device tree seems obvious and simple to me.
On a code level beyound the dts they're 100% the same. Now go compare a mainboard Config.lb (minus the makefile stuff) to the scattered dts mentioning struct names for components etc etc. It is really much more complex than in v2. Yet, it does not have more features.
v3 has a few perceived problems and a few real problems. The problem is that everybody has his own idea about which problems are real. I'm not claiming that my version of the story is the absolute truth(tm)
What exactly are you trying to say? What is your version of the story anyways?
On 30.07.2008 01:53, Stefan Reinauer wrote:
Carl-Daniel Hailfinger wrote:
On 29.07.2008 22:10, ron minnich wrote:
On Tue, Jul 29, 2008 at 1:05 PM, Stefan Reinauer stepan@coresystems.de wrote:
Peter Stuge wrote:
Yes, I argued strongly for this when they first appeared and even sent a patch. The problem is that the filenames are tied hard into the struct names generated by dtc.
Hm. I definitely want to support your idea here then.
Adding .dts to the filename is about as hard as not doing that. Also, in 2 out of 3 dts files I see struct names.
And what about files which are named dts right now? Do we call them dts.dts (ugly) or .dts (hidden file)? Stripping a given file suffix in dtc before creating the struct name is easy.
Read my initial proposal.
That proposal said:
To make clear what those files are, we should rename them...
dts -> mainboard.dts ide -> ide.dts apic -> apic.dts
I can either take your words literally ( southbridge/amd/cs5536/dts becomes southbridge/amd/cs5536/mainboard.dts ) or I take their intent and conclude the initial proposal was incomplete.
If we'd really autocreate something, we should drop that behavior.
struct name autocreation is a feature I really like.
Absolutely. "That behavior" meant mentioning the struct name manually in the dts files.
Indeed. I'll take a look at those files in the next few months. I'd be happy if someone could tackle this before.
oh no! it's harder! we blew it!
I'd like to disagree. I still haven't fully understood the v2 device tree, while the v3 device tree seems obvious and simple to me.
On a code level beyound the dts they're 100% the same. Now go compare a mainboard Config.lb (minus the makefile stuff) to the scattered dts mentioning struct names for components etc etc. It is really much more complex than in v2. Yet, it does not have more features.
v3 has a few perceived problems and a few real problems. The problem is that everybody has his own idea about which problems are real. I'm not claiming that my version of the story is the absolute truth(tm)
What exactly are you trying to say?
It's like flashrom. Agreeing on features and roadmap is probably more difficult than coding stuff up.
What is your version of the story anyways?
The v2 config files are completely unreadable. In v3, the situation is a lot better. Maybe not optimal, but orders of magnitude better than v2. One thing I see as a problem in both versions is how I can specify different settings for each instance of a chip appearing multiple times on a board. (I may be misinterpreting struct name generation...)
Regards, Carl-Daniel
On Tue, Jul 29, 2008 at 6:32 PM, Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net wrote:
The v2 config files are completely unreadable. In v3, the situation is a lot better. Maybe not optimal, but orders of magnitude better than v2. One thing I see as a problem in both versions is how I can specify different settings for each instance of a chip appearing multiple times on a board. (I may be misinterpreting struct name generation...)
I think you are. I put some work into that part and it ought to be ok. If not, it's fixable. I even thought I tested it, I might be wrong. But it's an easy fix.
ron
On Tue, Jul 29, 2008 at 6:32 PM, Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net wrote:
One thing I see as a problem in both versions is how I can specify different settings for each instance of a chip appearing multiple times on a board. (I may be misinterpreting struct name generation...)
I checked and the path is part of the instance. So if you have superio@2e{ etc. } superio@4e { etc. }
and they're the same part, you still get two structs, one which looks like: struct superio_winbond_hf86whatever_config ioport_2e and struct superio_winbond_hf86whatever_config ioport_4e
i.e you get the struct (the kind) and the path is flattened into the name to make it unique (the instance).
ron
On 30.07.2008 06:12, ron minnich wrote:
On Tue, Jul 29, 2008 at 6:32 PM, Carl-Daniel Hailfinger wrote:
One thing I see as a problem in both versions is how I can specify different settings for each instance of a chip appearing multiple times on a board. (I may be misinterpreting struct name generation...)
I checked and the path is part of the instance. So if you have superio@2e{ etc. } superio@4e { etc. }
and they're the same part, you still get two structs, one which looks like: struct superio_winbond_hf86whatever_config ioport_2e and struct superio_winbond_hf86whatever_config ioport_4e
i.e you get the struct (the kind) and the path is flattened into the name to make it unique (the instance).
Thanks for clearing this up. I had only looked at the struct type name, not the struct instance name. My apologies.
Regards, Carl-Daniel
Committed revision 700.
On Tue, Jul 29, 2008 at 4:53 PM, Stefan Reinauer stepan@coresystems.de wrote:
On a code level beyound the dts they're 100% the same. Now go compare a mainboard Config.lb (minus the makefile stuff) to the scattered dts mentioning struct names for components etc etc. It is really much more complex than in v2. Yet, it does not have more features.
There are some improvements. We got rid of chip.h and now generate structs from dts. We also have default init values, not just 0. These two improvements make my life easier.
ron
On 29.07.2008 19:58, Stefan Reinauer wrote:
Carl-Daniel Hailfinger wrote:
AFAICS this will break all targets unless each dts is fixed as well. If you include the fixed dts for each board, the patch is Acked-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
./mainboard/adl/msm800sev/dts ./mainboard/amd/db800/dts ./mainboard/amd/norwich/dts ./mainboard/artecgroup/dbe61/dts ./mainboard/artecgroup/dbe62/dts ./mainboard/emulation/qemu-x86/dts ./mainboard/pcengines/alix1c/dts ./mainboard/pcengines/alix2c3/dts
and possibly a few more files like apic, ide and whatever they're called..
I already went though these files and they don't have any statements which would be affected by the change AFAICS.
Regards, Carl-Daniel
second round trip, one last verification.
Attached, dts files now fixed.
ron p.s. I'm sorry that there is unhappiness with dts, let's fix it :-) I'm happy to do what's needed. I'm about to start dragging in k8 support, so I want to get it right.
ron
On 30.07.2008 05:51, ron minnich wrote:
second round trip, one last verification.
Attached, dts files now fixed.
ron p.s. I'm sorry that there is unhappiness with dts, let's fix it :-) I'm happy to do what's needed. I'm about to start dragging in k8 support, so I want to get it right.
Thanks for updating the patch.
Acked-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Regards, Carl-Daniel
-
Carl-Daniel Hailfinger -
Peter Stuge -
ron minnich -
Stefan Reinauer