Hi,
here's a first patch (in a row of a bunch of other, similar planned patches) which reduces the amount of crappy, duplicated mainboard code in the repository.
(it also adds support for a new target, the Advantech PCM-5820, btw.)
This common code supports three different GX1-boards, more will follow. We have at least 7 of them in the repo right now (and I'll add more), duplicating the same crap in each mainboard/* directory again and again is definately not what we want to do.
This is build-tested, and runtime-tested on hardware on two boards. Erwan Velu erwan@seanodes.com will likely be able to test on the Advantech PCM-5820 soon.
I'm planning similar patches for unifying the code for CK804 based boards, and others. E.g. we can easily support the
- ASUS A8NE-FM/S - ASUS A8N-E - ASUS A85NX - and probably lots more very similar boards
from one common code base (and we should!).
Note: I'm not saying that we _must_ do this with _all_ boards in the repository. If the differences are too big, this doesn't make too much sense. But I have a very strong opinion that we should indeed do it for all boards which are reasonably similar.
The savings even in this (GX1) case are pretty large, but if you look at some CK804/MCP55 boards, you'll see that it makes a _dramatic_ difference in the amount of code whether you copy-paste or use a generic common code base.
Comments welcome, Uwe.
Uwe Hermann wrote:
Hi,
here's a first patch (in a row of a bunch of other, similar planned patches) which reduces the amount of crappy, duplicated mainboard code in the repository.
(it also adds support for a new target, the Advantech PCM-5820, btw.)
This common code supports three different GX1-boards, more will follow. We have at least 7 of them in the repo right now (and I'll add more), duplicating the same crap in each mainboard/* directory again and again is definately not what we want to do.
Uwe, Good stuff! If you are interested, it would be good to do this for the LX/5536 platforms too. What do you think?
Marc
On Thu, Oct 18, 2007 at 06:19:58PM -0600, Marc Jones wrote:
here's a first patch (in a row of a bunch of other, similar planned patches) which reduces the amount of crappy, duplicated mainboard code in the repository. (it also adds support for a new target, the Advantech PCM-5820, btw.) This common code supports three different GX1-boards, more will follow. We have at least 7 of them in the repo right now (and I'll add more), duplicating the same crap in each mainboard/* directory again and again is definately not what we want to do.
Uwe, Good stuff! If you are interested, it would be good to do this for the LX/5536 platforms too. What do you think?
Yes, definately. IMO we can do this for pretty much all chipsets, e.g.
src/common/gx1-base src/common/lx-base src/common/i440bx-base src/common/ck804-base src/common/mcp55-base etc. etc.
My next patch will be for the Intel 440BX though, as I can personally test those boards.
Uwe.
Quoting Uwe Hermann uwe@hermann-uwe.de:
On Thu, Oct 18, 2007 at 06:19:58PM -0600, Marc Jones wrote:
here's a first patch (in a row of a bunch of other, similar planned patches) which reduces the amount of crappy, duplicated mainboard code in the repository. (it also adds support for a new target, the Advantech PCM-5820, btw.) This common code supports three different GX1-boards, more will follow. We have at least 7 of them in the repo right now (and I'll add more), duplicating the same crap in each mainboard/* directory again and again is definately not what we want to do.
Uwe, Good stuff! If you are interested, it would be good to do this for the LX/5536 platforms too. What do you think?
Yes, definately. IMO we can do this for pretty much all chipsets, e.g.
src/common/gx1-base src/common/lx-base src/common/i440bx-base src/common/ck804-base src/common/mcp55-base etc. etc.
My next patch will be for the Intel 440BX though, as I can personally test those boards.
Alot of the intel chipsets have common functions. Maybe some other ones can be included? I think this is a good Idea. Eliminating duplicated code and adding it globaly can simplify things when adding support for a new chip.
Thanks - Joe
Uwe Hermann a écrit :
Hi,
here's a first patch (in a row of a bunch of other, similar planned patches) which reduces the amount of crappy, duplicated mainboard code in the repository.
(it also adds support for a new target, the Advantech PCM-5820, btw.)
This common code supports three different GX1-boards, more will follow. We have at least 7 of them in the repo right now (and I'll add more), duplicating the same crap in each mainboard/* directory again and again is definately not what we want to do.
This is build-tested, and runtime-tested on hardware on two boards. Erwan Velu erwan@seanodes.com will likely be able to test on the Advantech PCM-5820 soon.
That patch is working except the link but you already knew that. Thanks a lot for you help uwe.
-------------------------------------------------------------------------------- Les opinions et prises de position emises par le signataire du present message lui sont propres et ne sauraient engager la responsabilite de la societe SEANODES.
Ce message ainsi que les eventuelles pieces jointes constituent une correspondance privee et confidentielle a l'attention exclusive du destinataire designe ci-dessus. Si vous n'etes pas le destinataire du present message ou une personne susceptible de pouvoir le lui delivrer, il vous est signifie que toute divulgation, distribution ou copie de cette transmission est strictement interdite. Si vous avez recu ce message par erreur, nous vous remercions d'en informer l'expediteur par telephone ou de lui retourner le present message, puis d'effacer immediatement ce message de votre systeme.
The views and opinions expressed by the author of this message are personal. SEANODES shall assume no liability, express or implied for such message.
This e-mail and any attachments is a confidential correspondence intended only for use of the individual or entity named above. If you are not the intended recipient or the agent responsible for delivering the message to the intended recipient, you are hereby notified that any disclosure, distribution or copying of this communication is strictly prohibited. If you have received this communication in error, please notify the sender by phone or by replying this message, and then delete this message from your system.
Hi Uwe
I just applied your patch and looked at the common/gx1-base/*. How do I add my board to the code tree?
It seems board specific codes which used to be in independent files are now placed in #ifdef chain. Should I add another #elif...? That sounds not right. Is there other way to refactor?
Regards, Kenji Noguchi
2007/10/18, Uwe Hermann uwe@hermann-uwe.de:
Hi,
here's a first patch (in a row of a bunch of other, similar planned patches) which reduces the amount of crappy, duplicated mainboard code in the repository.
(it also adds support for a new target, the Advantech PCM-5820, btw.)
This common code supports three different GX1-boards, more will follow. We have at least 7 of them in the repo right now (and I'll add more), duplicating the same crap in each mainboard/* directory again and again is definately not what we want to do.
This is build-tested, and runtime-tested on hardware on two boards. Erwan Velu erwan@seanodes.com will likely be able to test on the Advantech PCM-5820 soon.
I'm planning similar patches for unifying the code for CK804 based boards, and others. E.g. we can easily support the
- ASUS A8NE-FM/S
- ASUS A8N-E
- ASUS A85NX
- and probably lots more very similar boards
from one common code base (and we should!).
Note: I'm not saying that we _must_ do this with _all_ boards in the repository. If the differences are too big, this doesn't make too much sense. But I have a very strong opinion that we should indeed do it for all boards which are reasonably similar.
The savings even in this (GX1) case are pretty large, but if you look at some CK804/MCP55 boards, you'll see that it makes a _dramatic_ difference in the amount of code whether you copy-paste or use a generic common code base.
Comments welcome, Uwe.
http://www.hermann-uwe.de | http://www.holsham-traders.de http://www.crazy-hacks.org | http://www.unmaintained-free-software.org
-----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.6 (GNU/Linux)
iD8DBQFHF/F/XdVoV3jWIbQRAm8WAKCIvs1FF6a8spNDKwFhzvvPJQP/uQCeOApS Ztstiz7PtF5HAR/mxi801EY= =vVrH -----END PGP SIGNATURE-----
-- linuxbios mailing list linuxbios@linuxbios.org http://www.linuxbios.org/mailman/listinfo/linuxbios
On Fri, Oct 19, 2007 at 12:21:00PM -0700, Kenji Noguchi wrote:
Hi Uwe
I just applied your patch and looked at the common/gx1-base/*. How do I add my board to the code tree?
It seems board specific codes which used to be in independent files are now placed in #ifdef chain. Should I add another #elif...?
Yes, basically.
And you need a symlink from
src/mainboard/XXXX/YYYYY -> src/mainboard/common/gx1-base
That sounds not right. Is there other way to refactor?
I can't think of a better way to do it, but I'm open to suggestions. The #elif's are a cheap price when we can get rid of all the duplicated files we had before.
There are just 2-3 places where you have to add your board in the #elif.
On the long run (in v3 maybe) we could make the framework a bit more generic, to allow to plug in different Super I/Os etc., without having to #ifdef all of them...
Uwe.
On Fri, Oct 19, 2007 at 01:51:28AM +0200, Uwe Hermann wrote:
Comments welcome, Uwe.
On Fri, Oct 19, 2007 at 09:33:21PM +0200, Uwe Hermann wrote:
That sounds not right. Is there other way to refactor?
I can't think of a better way to do it, but I'm open to suggestions.
I think this is too much work for v2.
The #elif's are a cheap price when we can get rid of all the duplicated files we had before.
For v2, I think I'm with Ron. Duplicated code and files isn't that bad. Yes, it's dirty, but rather than cleaning it up piece by piece I'd like to work on v3.
There are just 2-3 places where you have to add your board in the #elif.
On the long run (in v3 maybe)
How long is "long" ?
we could make the framework a bit more generic, to allow to plug in different Super I/Os etc., without having to #ifdef all of them...
Isn't this the case already?
Bottom line:
I would much rather see this effort go into v3 and a bonus is that many of the boards supported by v2 would be working also with v3.
And:
#ifdef/#elif is really ugly. At least do #include instead?
//Peter
On Tue, Oct 23, 2007 at 04:39:00PM +0200, Peter Stuge wrote:
The #elif's are a cheap price when we can get rid of all the duplicated files we had before.
For v2, I think I'm with Ron. Duplicated code and files isn't that bad. Yes, it's dirty, but rather than cleaning it up piece by piece I'd like to work on v3.
Sure, but working on v3 doesn't mean we necessarily have to let v2 bitrot.
I think it's pretty clear that I totally dislike duplicated code (_the_ single biggest problem of the v2 codebase if you ask me), but I understand that it may not be _that_ dramatic with the GX1 or 440BX boards (which are -- or can be made -- very small and clean anyway).
The bigger problem is more recent boards. Look at m57sli, a8n_e, etc. Those not only contain board-specific config items (which would be ideal), but rather board-specific _code_ (which is not nice and we should avoid that in v3), and worse they contain (contrary to GX1/440BX) a _lot_ of duplicated or near-duplicated code.
Maybe we don't even have to fully merge the boards in v2, some simple cleanups in the current code base would also be helpful, I think.
Config.lb:
- Can easily be made totally generic to work for _every_ board we use. The only really specific part in there is the static device tree, all the other blurb could as well be in a common file.
Hm, do Config.lb/Option.lb files support an #include-like mechanisms?
Options.lb:
- Not needed at all, IMHO. We could just drop them all, I think. 95% of those files is the same for each board, the rest can be easily set to some sane default and overridden in targets/.../Config.lb as needed.
Other files, such as get_bus_conf.c, irq_tables.c, mptable.c, resourcemap.c etc. are twistly little mazes. They partly hardcode information such as the number of certain devices in C code etc. This is where it gets really ugly. I'm not sure I can be bothered to create patches to fix _that_ in v2, but I'd really appreciate it if v3 would have that kind of information as a simple config value part of the dts.
There are just 2-3 places where you have to add your board in the #elif.
On the long run (in v3 maybe)
How long is "long" ?
No idea. As short as possible, of course, but just like anyone else I have no release date.
I would much rather see this effort go into v3 and a bonus is that
Sure, I'm willing to also provide patches to eliminate any uselessly duplicated code from v3.
many of the boards supported by v2 would be working also with v3.
Can you elaborate? How does eliminating duplicated code help or not help in supporting v2 boards in v3? Those issues are not really related IMO.
#ifdef/#elif is really ugly. At least do #include instead?
Could be done, yes. Would such a patch be accepted?
Uwe.
* Uwe Hermann uwe@hermann-uwe.de [071024 00:08]:
Sure, but working on v3 doesn't mean we necessarily have to let v2 bitrot.
Unfortunately it's v3 that bitrots.. :(
The bigger problem is more recent boards. Look at m57sli, a8n_e, etc. Those not only contain board-specific config items (which would be ideal), but rather board-specific _code_ (which is not nice and we should avoid that in v3), and worse they contain (contrary to GX1/440BX) a _lot_ of duplicated or near-duplicated code.
The right answer is to advance the structure of LinuxBIOS in a way that no code is needed anymore per board. stuff like pirq, mptable, acpi could be derived from a well written config.lb/dtc.
Config.lb:
Can easily be made totally generic to work for _every_ board we use. The only really specific part in there is the static device tree, all the other blurb could as well be in a common file.
Hm, do Config.lb/Option.lb files support an #include-like mechanisms?
Yes, and in v3 we cleaned this up. Let's not put our v3 effort in v2. This will make v3 die and v2 will be a half hearted solution..
Options.lb:
- Not needed at all, IMHO. We could just drop them all, I think. 95% of those files is the same for each board, the rest can be easily set to some sane default and overridden in targets/.../Config.lb as needed.
Forget about this stuff for v2. It's not nice, I agree. But it's a design issue. You're at best fighting windmills here.
Other files, such as get_bus_conf.c, irq_tables.c, mptable.c, resourcemap.c etc. are twistly little mazes. They partly hardcode information such as the number of certain devices in C code etc. This is where it gets really ugly. I'm not sure I can be bothered to create patches to fix _that_ in v2, but I'd really appreciate it if v3 would have that kind of information as a simple config value part of the dts.
Yes, in v3 we should, no, we must work on autocreating all those.
dtc is the only mainboard specific information source in the end.
If you have ideas, please make that happen for v3, rather than "wasting time" on v2.
Because once we got that done, porting new boards will be so much easier than now, and no long cleanups are required.
#ifdef/#elif is really ugly. At least do #include instead?
Could be done, yes. Would such a patch be accepted?
... Patches to v3, doing the "right thing(tm)" are definitely preferred.
Stefan
On Wed, Oct 24, 2007 at 12:08:58AM +0200, Uwe Hermann wrote:
I think it's pretty clear that I totally dislike duplicated code
Me too, but it's not worth the effort to fix that in v2.
The bigger problem is more recent boards.
..
worse they contain a _lot_ of duplicated or near-duplicated code.
Yes, likely because of copy-paste development.
Maybe we don't even have to fully merge the boards in v2, some simple cleanups in the current code base would also be helpful, I think.
Disagree. Not for v2.
get_bus_conf.c, irq_tables.c, mptable.c, resourcemap.c etc.
..
create patches to fix _that_ in v2, but I'd really appreciate it if v3 would have that kind of information as a simple config value part of the dts.
This is indeed my idea for v3 too.
There are just 2-3 places where you have to add your board in the #elif.
On the long run (in v3 maybe)
How long is "long" ?
No idea. As short as possible, of course, but just like anyone else I have no release date.
Understood. What I was getting at was that it may be quicker to just forget v2 and work with new shinyness in v3.
I would much rather see this effort go into v3 and a bonus is that
Sure, I'm willing to also provide patches to eliminate any uselessly duplicated code from v3.
My point was that there is little if any duplicate code in v3 and that the v3 design is intended to minimize it - working on v3 is much more interesting, since it solves many of the problems in v2.
many of the boards supported by v2 would be working also with v3.
Can you elaborate? How does eliminating duplicated code help or not help in supporting v2 boards in v3? Those issues are not really related IMO.
That's right. But I meant that moving one v2 port to v3 (part of which is structuring the code a bit better) should make it very easy to add new boards with similar hardware - just like we want.
And, as Stefan pointed out, putting effort into v2 means v3 becomes stale.
I still expect people to work on v2 to some degree since it's much easier to do copy-paste-development there, but I don't think we should encourage any efforts in v2 except as a testbed now that v3 is starting to run on hardware.
//Peter
On Thu, Oct 25, 2007 at 05:43:58AM +0200, Peter Stuge wrote:
Understood. What I was getting at was that it may be quicker to just forget v2 and work with new shinyness in v3.
Yeah, good point. I'm playing with CAR for Intel CPUs right now, if it canbe made to work soonish in v3, I'll port over the 440BX boards ASAP.
Sure, I'm willing to also provide patches to eliminate any uselessly duplicated code from v3.
My point was that there is little if any duplicate code in v3 and that the v3 design is intended to minimize it - working on v3 is much more interesting, since it solves many of the problems in v2.
Yes, I fully agree, but I still don't want to let v2 bitrot. _Most_ work should indeed rather go into v3, but v2 should not be completely ignored (yet).
Uwe.
Hi Kenji,
On Fri, Oct 19, 2007 at 12:21:00PM -0700, Kenji Noguchi wrote:
I just applied your patch and looked at the common/gx1-base/*. How do I add my board to the code tree?
It seems board specific codes which used to be in independent files are now placed in #ifdef chain. Should I add another #elif...? That sounds not right. Is there other way to refactor?
Any news on the TC7020 board? We decided to not use the merged gx1-base code for now, so please feel free to post a "normal" patch for your board. I suggest to start with the BCOM Winnet100 as an example.
Thanks, Uwe.