Update and Cut down mach types
Subsystems (2)
Paulraj
Wolfgang, Albert,
Russell King sent some updates to the linux kernel for mach-types.
He also removed a lot of entries which never made it to mainline.
I have a patch and it is the branch below
http://git.denx.de/?p=u-boot/u-boot-ti.git;a=shortlog;h=refs/heads/update-mach-types
Please review and if it is acceptable to everyone then we should apply this patch to remain in sync with the mainline kernel.
Regards,
Sandeep
Hello Sandeep, Wolfgang
Am 19.04.2011 14:42, schrieb Paulraj, Sandeep:
Quoted 6 lines
Wolfgang, Albert,
Russell King sent some updates to the linux kernel for mach-types.
Quoted 8 lines
He also removed a lot of entries which never made it to mainline.
I have a patch and it is the branch below
http://git.denx.de/?p=u-boot/u-boot-ti.git;a=shortlog;h=refs/heads/update-mach-types
Please review and if it is acceptable to everyone then we should apply this patch to remain in sync with the mainline kernel.
This will break a least jadecpu. We don't use Linux on this board. When porting I was requested to reserve an MACH_ID just in case the board will ever be used with Linux. This has not been the case for this board. But I would like to have this board in the u-boot tree. What will be the solution for ARM but non-Linux u-boot ports then? What should be passed to gd->bd->bi_arch_number?
Regards,
Matthias
Paulraj
Quoted 20 lines
Hello Sandeep, Wolfgang
Am 19.04.2011 14:42, schrieb Paulraj, Sandeep:
Quoted 5 lines
Wolfgang, Albert,
Russell King sent some updates to the linux kernel for mach-types.
2.6.git;a=commitdiff;h=6f82f4db80189281a8ac42f2e72396accb719b57
Quoted 6 lines
He also removed a lot of entries which never made it to mainline.
I have a patch and it is the branch below
ti.git;a=shortlog;h=refs/heads/update-mach-types
Quoted 1 line
Quoted 8 lines
Quoted 2 lines
Please review and if it is acceptable to everyone then we should apply this patch to remain in sync with the mainline kernel.
This will break a least jadecpu.
We don't use Linux on this board. When
porting I was requested to reserve anMACH_IDjust in case the board will ever be used with Linux. This has not been the case for this board. But I would like to have this board in the u-boot tree.
We are not removing this board from u-boot
Quoted 3 lines
What will be the
solution for ARM but non-Linux u-boot ports then? What should be passed togd->bd->bi_arch_number?
I'll defer to Wolfgang and Albert on this issue.
There are new boards and hence mach types being added.
Russell sent a patch that both added new mach IDs and removed many others.
--Sandeep
Le 19/04/2011 15:45, Paulraj, Sandeep a ?crit :
Quoted 35 lines
Quoted 20 lines
Hello Sandeep, Wolfgang
Am 19.04.2011 14:42, schrieb Paulraj, Sandeep:
Quoted 5 lines
Wolfgang, Albert,
Russell King sent some updates to the linux kernel for mach-types.
2.6.git;a=commitdiff;h=6f82f4db80189281a8ac42f2e72396accb719b57
Quoted 6 lines
He also removed a lot of entries which never made it to mainline.
I have a patch and it is the branch below
ti.git;a=shortlog;h=refs/heads/update-mach-types
Quoted 1 line
Quoted 8 lines
Quoted 2 lines
Please review and if it is acceptable to everyone then we should apply this patch to remain in sync with the mainline kernel.
This will break a least jadecpu.
We don't use Linux on this board. When
porting I was requested to reserve anMACH_IDjust in case the board will ever be used with Linux. This has not been the case for this board. But I would like to have this board in the u-boot tree.We are not removing this board from u-boot
Quoted 3 lines
What will be the
solution for ARM but non-Linux u-boot ports then? What should be passed togd->bd->bi_arch_number?
Quoted 7 lines
I'll defer to Wolfgang and Albert on this issue.
There are new boards and hence mach types being added.
Russell sent a patch that both added new mach IDs and removed many others.--Sandeep
If the jadecpu board does not require a machine ID because it does not run Linux, then it should be able to build without a machine ID. So:
- if the need for a machine ID lies in the jadecpu U-Boot code, then the jadecpu code should be fixed.
- if the need for a machine ID is general to the U-Boot ARM arch, than the ARM arch must be fixed.
Now somebody must find out which is which, and submit a patch copying either the jadecpu maintainer, the ARM custodian, or better yet, both. :)
Amicalement,
Signature · 1 line
Albert.
Dear =?ISO-8859-1?Q?Matthias_Wei=DFer?=,
Quoted 7 lines · In message <[email protected]> you wrote:
This will break a least jadecpu. We don't use Linux on this board. When porting I was requested to reserve an
MACH_IDjust in case the board will ever be used with Linux. This has not been the case for this board. But I would like to have this board in the u-boot tree. What will be the solution for ARM but non-Linux u-boot ports then? What should be passed togd->bd->bi_arch_number?
I think you have two options:
1) Complain with RMK about the removal of yoru MACH_ID. Explain to him that you use this elsewhere and ash to re-add it.
2) If you don;t use Linux on that board, there will be no code who ever looks at bi_arch_number, so just pass a random number (like 0) there.
Best regards,
Wolfgang Denk
Signature · 6 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de It became apparent that one reason why the Ice Giants were known as the Ice Giants was because they were, well, giants. The other was that they were made of ice. -Terry Pratchett, _Sourcery_
Hello Wolfgang
Am 19.04.2011 16:21, schrieb Wolfgang Denk:
Quoted 8 lines
Dear =?ISO-8859-1?Q?Matthias_Wei=DFer?=,
Quoted 2 lines
solution for ARM but non-Linux u-boot ports then? What should be passed to
gd->bd->bi_arch_number?I think you have two options:
1) Complain with RMK about the removal of yoru
MACH_ID. Explain to him that you use this elsewhere and ash to re-add it.
The point it that it makes no sense to add an MACH_ID to the database for a board that probably will never run Linux.
Quoted 3 lines
2) If you don;t use Linux on that board, there will be no code who ever looks at
bi_arch_number, so just pass a random number (like 0) there.
This approach was rejected in 2009 when I initially send patches for this board. But I would like to go that way (adding a comment so that it is clear what is going on) So, I expect that this is OK and I send a patch then when time permits.
Thanks,
Matthias
Am 04/19/2011 04:21 PM, schrieb Wolfgang Denk:
Quoted 9 lines
Dear =?ISO-8859-1?Q?Matthias_Wei=DFer?=,
Quoted 6 lines · In message <[email protected]> you wrote:
This will break a least jadecpu. We don't use Linux on this board. When porting I was requested to reserve an
MACH_IDjust in case the board will ever be used with Linux. This has not been the case for this board. But I would like to have this board in the u-boot tree. What will be the solution for ARM but non-Linux u-boot ports then? What should be passed togd->bd->bi_arch_number?
Quoted 4 lines
I think you have two options:
1) Complain with RMK about the removal of yoru
MACH_ID. Explain to him that you use this elsewhere and ash to re-add it.
I did that and got the following reply (without quotes due to cut-and-paste)
cu
Michael
From: Russell King - ARM Linux
<[email protected]>
To: Michael Schwingen
<[email protected]>
Subject: Re: [U-Boot] Update and Cut down mach types - ACTUX* and DVLHOST
machines
removed
What makes it into the kernel is determined by a script which is based
upon a couple of simple
rules:
1. If the entry appears in a MACHINE_START() declaration in Linus' kernel
tree (evaluated on a daily basis), it is kept in the
file.
2. If the entry was created or modified in the database within the last
12
months.
(2) gives people a way to refresh their entry to ensure that it stays in
the list in the mainline
kernel.
[...]
The normal URL which you fetch the file from (as contained within the file)
will give you the full listing rather than the cut-down version. There
really is no need for uboot to go picking the copy up from the mainline
kernel.
Hi Michael,
[...]
Quoted 1 line
I did that and got the following reply (without quotes due to cut-and-paste)
[...]
Quoted 4 lines
The normal URL which you fetch the file from (as contained within the file) will give you the full listing rather than the cut-down version. There really is no need for uboot to go picking the copy up from the mainline kernel.
So we can just use that version and everything remains "sane"? Then let's do that and move on to other topics ;)
Cheers
Detlev
Signature · 6 lines
If we knew what it was we were doing, we wouldn't call it research.
-- Einstein
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.deHi Sandeep, Albert, Wolfgang,
Quoted 7 lines · Sandeep
Wolfgang, Albert,
Russell King sent some updates to the linux kernel for mach-types.
He also removed a lot of entries which never made it to mainline.
Well, as I understood from Russell, the main purpose of this "cut down" is to make "du -s linux/arch/arm" smaller, because there is no real need in all those boards listed in mach-types.h unless there is a support for them in mainline Linux kernel.
Nevertheless the real ARM registry remains untouched - meaning that all board ids remain the same and no board is removed from the registry.
This is the place where U-Boot board support diverges from Linux...
Are we obliged to follow the Linux mach-types.h?
Can't we just adopt Russell's "cut down" script to boards supported by U-Boot? Or will it harden the mach-types.h future updates?
Have you thought of getting rid of mach-types.h completely? Making every board define its ARM registry id can work and will eliminate the need for mach-types.h update every couple of months.
Quoted 3 lines
I have a patch and it is the branch below
http://git.denx.de/?p=u-boot/u-boot-ti.git;a=shortlog;h=refs/heads/update-mach-types
Have you checked that none of the removed boards are in U-Boot tree? Because if there are some, then their build will be broken... And have to be fixed by... say a #define MACH_TYPE_* <id> in a board_config file.
Quoted 2 lines
Please review and if it is acceptable to everyone then we should apply this patch to remain in sync with the mainline kernel.
[...]
Signature · 2 lines
Regards, Igor.
Quoted 9 lines · Igor Grinberg
Hi Sandeep, Albert, Wolfgang,
Quoted 6 lines · Sandeep
Wolfgang, Albert,
Russell King sent some updates to the linux kernel for mach-types.
Quoted 7 lines
Quoted 1 line
He also removed a lot of entries which never made it to mainline.
Well, as I understood from Russell, the main purpose of this "cut down" is to make "du -s linux/arch/arm" smaller, because there is no real need in all those boards listed in
mach-types.hunless there is a support for them in mainline Linux kernel.
Nevertheless the real ARM registry remains untouched - meaning that all board ids remain the same and no board is removed from the registry.
Correct.
Quoted 9 lines
This is the place where U-Boot board support diverges from Linux...
Are we obliged to follow the Linux
mach-types.h?
Can't we just adopt Russell's "cut down" script to boards supported by U-Boot? Or will it harden themach-types.hfuture updates?Have you thought of getting rid of
mach-types.hcompletely? Making every board define its ARM registry id can work and will eliminate the need formach-types.hupdate every couple of months.
Also, why do we need to pull mach-types.h from Linux at all?
Why don't we pull the original master mach-types file, and generate the required .h file(s) during make using the same (or a similar) script Linux uses?
Quoted 5 lines
Quoted 3 lines
I have a patch and it is the branch below
http://git.denx.de/?p=u-boot/u-boot-ti.git;a=shortlog;h=refs/heads/update-mach-types
Have you checked that none of the removed boards are in U-Boot tree? Because if there are some, then their build will be broken...
It will break ACTUX1-ACTUX4 (which are in-tree, and work fine as soon as the relocation-breakage-patch is accepted), plus DVLHOST, for which I have patches submitted to add support.
For my own boards, I can go to the ARM machine database, touch the entry, and wait until the define re-emerges in Linux, and await until that is marged back to u-boot, but this is plain silly. However, for DVLHOST, I am not the registered maintainer in the machine database, so I would have to create a duplicate entry for this to work.
cu
Michael
Le 20/04/2011 19:15, Michael Schwingen a ?crit :
Quoted 3 lines
Why don't we pull the original master mach-types file, and generate the required .h file(s) during make using the same (or a similar) script Linux uses?
Hmm, because it would mean maintaining the same script as Linux uses. With the current solution, there's work to be done on mach-types only when someone needs new machine IDs.
Quoted 11 lines
Quoted 2 lines
Have you checked that none of the removed boards are in U-Boot tree? Because if there are some, then their build will be broken...
It will break ACTUX1-ACTUX4 (which are in-tree, and work fine as soon as the relocation-breakage-patch is accepted), plus DVLHOST, for which I have patches submitted to add support.
For my own boards, I can go to the ARM machine database, touch the entry, and wait until the define re-emerges in Linux, and await until that is marged back to u-boot, but this is plain silly. However, for DVLHOST, I am not the registered maintainer in the machine database, so I would have to create a duplicate entry for this to work.
IIUC the machines that would disappear are those for which the is no actual mainline Linux support and which have not been touched in over a year, right? Do ACTUX* and DVLHOST boards fit in this description?
Quoted 2 lines
cu
Michael
Amicalement,
Signature · 1 line
Albert.
Quoted 8 lines · Albert ARIBAUD
Le 20/04/2011 19:15, Michael Schwingen a ?crit :
Quoted 3 lines
Why don't we pull the original master mach-types file, and generate the required .h file(s) during make using the same (or a similar) script Linux uses?
Hmm, because it would mean maintaining the same script as Linux uses. With the current solution, there's work to be done on mach-types only when someone needs new machine IDs.
I don't see how much maintaining the script would need - if the input format does not change, the script does not need changes, and if changes are needed, the can be copied 1:1 from the Linux version.
On the plus side: the mach-types file is much more terse than the generated headers, so updates that pull in new machines would generate diffs that are a lot smaller than they are now.
Quoted 6 lines
Quoted 6 lines
Quoted 2 lines
Have you checked that none of the removed boards are in U-Boot tree? Because if there are some, then their build will be broken...
It will break ACTUX1-ACTUX4 (which are in-tree, and work fine as soon as the relocation-breakage-patch is accepted), plus DVLHOST, for which I have patches submitted to add support.
Quoted 8 lines
Quoted 5 lines
For my own boards, I can go to the ARM machine database, touch the entry, and wait until the define re-emerges in Linux, and await until that is marged back to u-boot, but this is plain silly. However, for DVLHOST, I am not the registered maintainer in the machine database, so I would have to create a duplicate entry for this to work.
IIUC the machines that would disappear are those for which the is no actual mainline Linux support and which have not been touched in over a year, right? Do ACTUX* and DVLHOST boards fit in this description?
Yes. The ACTUX board ports are by me, while the DVLHOST machine type seems to be allocated by the manufacturer, Devolo, who never mainlined their Linux adaptions, so my goal is to get an independent port up. However, that means I can't update the machine type to get it back in mainline Linux by the 12-month-rule.
cu
Michael
Le 20/04/2011 21:26, Michael Schwingen a ?crit :
Quoted 32 lines · Albert ARIBAUD
Quoted 8 lines
Le 20/04/2011 19:15, Michael Schwingen a ?crit :
Quoted 3 lines
Why don't we pull the original master mach-types file, and generate the required .h file(s) during make using the same (or a similar) script Linux uses?
Hmm, because it would mean maintaining the same script as Linux uses. With the current solution, there's work to be done on mach-types only when someone needs new machine IDs.
I don't see how much maintaining the script would need - if the input format does not change, the script does not need changes, and if changes are needed, the can be copied 1:1 from the Linux version.
On the plus side: the mach-types file is much more terse than the generated headers, so updates that pull in new machines would generate diffs that are a lot smaller than they are now.
Quoted 6 lines
Quoted 6 lines
Quoted 2 lines
Have you checked that none of the removed boards are in U-Boot tree? Because if there are some, then their build will be broken...
It will break ACTUX1-ACTUX4 (which are in-tree, and work fine as soon as the relocation-breakage-patch is accepted), plus DVLHOST, for which I have patches submitted to add support.
Quoted 8 lines
Quoted 5 lines
For my own boards, I can go to the ARM machine database, touch the entry, and wait until the define re-emerges in Linux, and await until that is marged back to u-boot, but this is plain silly. However, for DVLHOST, I am not the registered maintainer in the machine database, so I would have to create a duplicate entry for this to work.
IIUC the machines that would disappear are those for which the is no actual mainline Linux support and which have not been touched in over a year, right? Do ACTUX* and DVLHOST boards fit in this description?
Quoted 8 lines
Yes. The ACTUX board ports are by me, while the DVLHOST machine type seems to be allocated by the manufacturer, Devolo, who never mainlined their Linux adaptions, so my goal is to get an independent port up. However, that means I can't update the machine type to get it back in mainline Linux by the 12-month-rule.
cu
Michael
Michael, for the time being, can you provide a patch over Sandeep's update to reintroduce ACTUX* and DVLHOST? I'll consider it as a bugfix and apply it before my pull request.
For the longer term, let us keep on weighting the pros and cons of the options we have for handling machine types.
Amicalement,
Signature · 1 line
Albert.
Am 04/21/2011 01:39 PM, schrieb Albert ARIBAUD:
Quoted 8 lines
Le 20/04/2011 21:26, Michael Schwingen a ?crit :
Quoted 7 lines
year, right? Do ACTUX* and DVLHOST boards fit in this description? Yes. The ACTUX board ports are by me, while the DVLHOST machine type seems to be allocated by the manufacturer, Devolo, who never mainlined their Linux adaptions, so my goal is to get an independent port up. However, that means I can't update the machine type to get it back in mainline Linux by the 12-month-rule.
Quoted 5 lines
Quoted 2 lines
cu
MichaelMichael, for the time being, can you provide a patch over Sandeep's update to reintroduce ACTUX* and DVLHOST? I'll consider it as a bugfix and apply it before my pull request.
I guess this is not required after your NAK of the original patch?
Nevertheless, we need a method so that we no not need to patch the mach-types.h after every cacle where the Linux version is pulled in.
cu
Michael
Dear Michael Schwingen,
Quoted 3 lines · In message <[email protected]> you wrote:
Nevertheless, we need a method so that we no not need to patch the
mach-types.hafter every cacle where the Linux version is pulled in.
Detlev Zundel suggested we might maintain a U-Boot local file "obsoleted-mach-types.h" which would contain just the MACH_ID definitions that 1) are being actively used in U-Boot and 2) have been removed from the official "mach-types.h" file. This way we just need to add a "#include <obsoleted-mach-types.h>" to "mach-types.h" to maintain compatibility with the existing code, and we make it very obvious that these are somewhat special MACH_IDs.
Comments?
Best regards,
Wolfgang Denk
Signature · 5 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de "A verbal contract isn't worth the paper it's printed on." - Samuel Goldwyn
Le 26/04/2011 21:40, Wolfgang Denk a ?crit :
Quoted 16 lines
Dear Michael Schwingen,
Quoted 3 lines · In message<[email protected]> you wrote:
Nevertheless, we need a method so that we no not need to patch the
mach-types.hafter every cacle where the Linux version is pulled in.Detlev Zundel suggested we might maintain a U-Boot local file "
obsoleted-mach-types.h" which would contain just theMACH_IDdefinitions that 1) are being actively used in U-Boot and 2) have been removed from the official "mach-types.h" file. This way we just need to add a "#include<obsoleted-mach-types.h>" to "mach-types.h" to maintain compatibility with the existing code, and we make it very obvious that these are somewhat specialMACH_IDs.
Quoted 1 line
Comments?
Well, as you stated yourself recently, why would/should we maintain mach-types that are apparently not going to be used? Do machine types have other uses than for Linux? No code in U-Boot should worry about the mach-id if not for Linux.
Also, if we still decide to maintain our own list of mach-types, we will need some rule to decide when to remove mach-types from this special list eventually. Otherwise, it'll become asymptotically identical to the full lits that is also availabe, and then, what would be the point of maintaining our own?
So IMO, if we have mach-types in U-Boot for supporting Linux, then we should keep using a (reasonably) up-to-date Linux machine ID list just like we do now -- mach-types that disappear from the list mean Linux support has become useless for that machine in U-Boot. And if we have our own mach-type policy, different from "has linux support", then we need to specify what this policy is and how it is implemented.
Quoted 3 lines
Best regards,
Wolfgang Denk
Amicalement,
Signature · 1 line
Albert.
Dear Albert ARIBAUD,
Quoted 5 lines · In message <[email protected]> you wrote:
Well, as you stated yourself recently, why would/should we maintain mach-types that are apparently not going to be used? Do machine types have other uses than for Linux? No code in U-Boot should worry about the mach-id if not for Linux.
Well, in principle you are of course right.
But I am well aware that there is a ton of Linux BSPs out there which have never been pushed upstream into mainline by their respective creators for some reason or another. Also I see a chance that other uses of the mach-ids might exist - the Linux ARM folks have, fro a very long time, always explained what a clever idea this is to describe hardware features.
I hesitate to cut off all these exitisting or even potential users lightly, when there is a solution that works reasonably well for them and, at the same time, brings only minimal maintenance burdon for us.
Quoted 5 lines
Also, if we still decide to maintain our own list of mach-types, we will need some rule to decide when to remove mach-types from this special list eventually. Otherwise, it'll become asymptotically identical to the full lits that is also availabe, and then, what would be the point of maintaining our own?
That rule can be simple: we will only allow to add the now existing (in U-Boot mainline code) mach-ids, so this list should not grow further after the initial creation. OK, ther eis a slight chance that any newly added boards (to U-Boot) will get removed from the Linux master file later, but I consider this a small risk - especially as I expect to see more and ore device-tree based ARM ports quickly, so the whole mach-id thing becomes less and less of a pain.
Quoted 6 lines
So IMO, if we have mach-types in U-Boot for supporting Linux, then we should keep using a (reasonably) up-to-date Linux machine ID list just like we do now -- mach-types that disappear from the list mean Linux support has become useless for that machine in U-Boot. And if we have our own mach-type policy, different from "has linux support", then we need to specify what this policy is and how it is implemented.
I think we should be gentle to users of existing code and avoid breaking it. From now on, we could establish a policy that a mach-id can only be referenced when and as long mainline Linux support for this board exists.
I'm open for suggestions.
Best regards,
Wolfgang Denk
Signature · 5 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de There is a time in the tides of men, Which, taken at its flood, leads on to success. On the other hand, don't count on it. - T. K. Lawson
Quoted 43 lines · Wolfgang Denk
Dear Albert ARIBAUD,
Quoted 5 lines · In message<[email protected]> you wrote:
Well, as you stated yourself recently, why would/should we maintain mach-types that are apparently not going to be used? Do machine types have other uses than for Linux? No code in U-Boot should worry about the mach-id if not for Linux.
Well, in principle you are of course right.
But I am well aware that there is a ton of Linux BSPs out there which have never been pushed upstream into mainline by their respective creators for some reason or another. Also I see a chance that other uses of the mach-ids might exist - the Linux ARM folks have, fro a very long time, always explained what a clever idea this is to describe hardware features.
I hesitate to cut off all these exitisting or even potential users lightly, when there is a solution that works reasonably well for them and, at the same time, brings only minimal maintenance burdon for us.
Quoted 5 lines
Also, if we still decide to maintain our own list of mach-types, we will need some rule to decide when to remove mach-types from this special list eventually. Otherwise, it'll become asymptotically identical to the full lits that is also availabe, and then, what would be the point of maintaining our own?
That rule can be simple: we will only allow to add the now existing (in U-Boot mainline code) mach-ids, so this list should not grow further after the initial creation. OK, ther eis a slight chance that any newly added boards (to U-Boot) will get removed from the Linux master file later, but I consider this a small risk - especially as I expect to see more and ore device-tree based ARM ports quickly, so the whole mach-id thing becomes less and less of a pain.
Quoted 6 lines
So IMO, if we have mach-types in U-Boot for supporting Linux, then we should keep using a (reasonably) up-to-date Linux machine ID list just like we do now -- mach-types that disappear from the list mean Linux support has become useless for that machine in U-Boot. And if we have our own mach-type policy, different from "has linux support", then we need to specify what this policy is and how it is implemented.
Quoted 6 lines
I think we should be gentle to users of existing code and avoid breaking it. From now on, we could establish a policy that a mach-id can only be referenced when and as long mainline Linux support for this board exists.
I'm open for suggestions.
Hi Wolfgang, Albert,
why don't we just create the #define MACH_xxx lines directly from the "http://www.arm.linux.org.uk/developer/machines/download.php". We don't need all the *_is_* macros in u-boot anyway. Then we would have just a few 1000 lines of #define MACH_*
Reinhard
Am 04/26/2011 11:38 PM, schrieb Reinhard Meyer:
Quoted 16 lines
Quoted 8 lines
Quoted 7 lines
So IMO, if we have mach-types in U-Boot for supporting Linux, then we should keep using a (reasonably) up-to-date Linux machine ID list just like we do now -- mach-types that disappear from the list mean Linux support has become useless for that machine in U-Boot. And if we have our own mach-type policy, different from "has linux support", then we need to specify what this policy is and how it is implemented.
Quoted 6 lines
I think we should be gentle to users of existing code and avoid breaking it. From now on, we could establish a policy that a mach-id can only be referenced when and as long mainline Linux support for this board exists.
I'm open for suggestions.
Quoted 8 lines
Hi Wolfgang, Albert,
why don't we just create the #define
MACH_xxxlines directly from the "http://www.arm.linux.org.uk/developer/machines/download.php". We don't need all the *_is_* macros in u-boot anyway. Then we would have just a few 1000
lines of #define MACH_*
I had already proposed that - after all, that is the way Linux does it as well: the mach-types.h file is auto-generated from that list (or now from a cut-down version of that list), so directly using the original list to generate the .h file in u-boot would completely cut out the middle man.
This would have multiple advantages IMHO:
- the downloaded file is terse: only one line per machine, compared
with the current mach-types.h where one added machine generates lots of lines (most of which we do not need at all!). Reviewing a patch that pulls in a new upstream version would be easier with the original file instead of the .h file.
- Newly added machines turn up much earlier. When bringing up a new
board, you will usually work on u-boot first. Having to wait until the machine ID trickles down into the Linux kernel, and *then* gets pulled into u-boot at some later time, makes for a substantial delay until board patches can be submitted to u-boot.
- It would actually save space:
138803 Apr 27 12:12 mach-types (freshly downloaded, complete
unfiltered list)
1177444 Apr 5 20:55 ./arch/arm/include/asm/mach-types.h (from u-boot master, before the patch that removes boards)
- There would be no problem with removing boards that are supported in u-boot, but not in Linux mainline - we would have the IDs for all known boards, while still saving space.
I do think the maintenance effort when using the original mach-types file would be lower than with the current system. However, when I proposed this before,
cu
Michael
Quoted 48 lines · Michael Schwingen
Am 04/26/2011 11:38 PM, schrieb Reinhard Meyer:
Quoted 13 lines
Quoted 6 lines
Quoted 6 lines
So IMO, if we have mach-types in U-Boot for supporting Linux, then we should keep using a (reasonably) up-to-date Linux machine ID list just like we do now -- mach-types that disappear from the list mean Linux support has become useless for that machine in U-Boot. And if we have our own mach-type policy, different from "has linux support", then we need to specify what this policy is and how it is implemented.
Quoted 6 lines
I think we should be gentle to users of existing code and avoid breaking it. From now on, we could establish a policy that a mach-id can only be referenced when and as long mainline Linux support for this board exists.
I'm open for suggestions.
Quoted 8 lines
Hi Wolfgang, Albert,
why don't we just create the #define
MACH_xxxlines directly from the "http://www.arm.linux.org.uk/developer/machines/download.php". We don't need all the *_is_* macros in u-boot anyway. Then we would have just a few 1000
lines of #define MACH_*I had already proposed that - after all, that is the way Linux does it as well: the
mach-types.hfile is auto-generated from that list (or now from a cut-down version of that list), so directly using the original list to generate the .h file in u-boot would completely cut out the middle man.This would have multiple advantages IMHO:
- the downloaded file is terse: only one line per machine, compared
with the currentmach-types.hwhere one added machine generates lots of lines (most of which we do not need at all!). Reviewing a patch that pulls in a new upstream version would be easier with the original file instead of the .h file.- Newly added machines turn up much earlier. When bringing up a new
board, you will usually work on u-boot first. Having to wait until the machine ID trickles down into the Linux kernel, and *then* gets pulled into u-boot at some later time, makes for a substantial delay until board patches can be submitted to u-boot.- It would actually save space:
138803 Apr 27 12:12 mach-types (freshly downloaded, complete
unfiltered list)
1177444 Apr 5 20:55./arch/arm/include/asm/mach-types.h(from u-boot master, before the patch that removes boards)
Quoted 7 lines
- There would be no problem with removing boards that are supported in u-boot, but not in Linux mainline - we would have the IDs for all known boards, while still saving space.
I do think the maintenance effort when using the original mach-types file would be lower than with the current system. However, when I proposed this before,
+1 to all said above, though some minor patching should be done:
u-boot $ grep -rn machine_is_ --exclude=mach-types.h *
arch/arm/cpu/arm920t/at91rm9200/ether.c:204: if (machine_is_csb337()) {
board/ti/omap1610inn/omap1610innovator.c:66: if (machine_is_omap_h2())
board/ti/omap1610inn/omap1610innovator.c:68: else if (machine_is_omap_innovator())Signature · 2 lines
Regards, Igor.
Hi Igor,
[...]
Quoted 6 lines
+1 to all said above, though some minor patching should be done:
u-boot $ grep -rn
machine_is_--exclude=mach-types.h *arch/arm/cpu/arm920t/at91rm9200/ether.c:204: if (machine_is_csb337()) { board/ti/omap1610inn/omap1610innovator.c:66: if (machine_is_omap_h2()) board/ti/omap1610inn/omap1610innovator.c:68: else if (machine_is_omap_innovator())
Thanks for finding those spots. Care to send a patch for these?
Cheers
Detlev
Signature · 8 lines
We have a live-manual. It's called emacs-devel at gnu.org.
You can stick to just reading it, but you can skip to a specific chapter
by simply sending an email asking for it ;-)
-- Stefan Monnier
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.dePATCH 1/3 arm: omap: innovator: fix compilation error
CONFIG_SYS_SDRAM_BASE and CONFIG_SYS_INIT_SP_ADDR were not defined for this board thus breaking its build.
--- include/configs/omap1610h2.h | 7 +++++++ include/configs/omap1610inn.h | 7 +++++++ 2 files changed, 14 insertions(+), 0 deletions(-)
include/configs/omap1610h2.h +7 -0
@@ -152,6 +152,13 @@#define PHYS_SDRAM_1 0x10000000/* SDRAM Bank #1 */ #define PHYS_SDRAM_1_SIZE 0x02000000/* 32 MB */ +#define CONFIG_SYS_SDRAM_BASE PHYS_SDRAM_1 +#define CONFIG_SYS_INIT_RAM_SIZE 0x1000 +/* FIXME: the value below is taken from davinci which uses ARM926EJS cpu */ +#define CONFIG_SYS_INIT_SP_ADDR (CONFIG_SYS_SDRAM_BASE + \ + CONFIG_SYS_INIT_RAM_SIZE - \ + GENERATED_GBL_DATA_SIZE) +#define PHYS_FLASH_1_BM1 0x00000000/* Flash Bank #1 if booting from flash */ #define PHYS_FLASH_1_BM0 0x0C000000/* Flash Bank #1 if booting from RAM */
include/configs/omap1610inn.h +7 -0
@@ -157,6 +157,13 @@#define PHYS_SDRAM_1 0x10000000/* SDRAM Bank #1 */ #define PHYS_SDRAM_1_SIZE 0x02000000/* 32 MB */ +#define CONFIG_SYS_SDRAM_BASE PHYS_SDRAM_1 +#define CONFIG_SYS_INIT_RAM_SIZE 0x1000 +/* FIXME: the value below is taken from davinci which uses ARM926EJS cpu */ +#define CONFIG_SYS_INIT_SP_ADDR (CONFIG_SYS_SDRAM_BASE + \ + CONFIG_SYS_INIT_RAM_SIZE - \ + GENERATED_GBL_DATA_SIZE) +#define PHYS_FLASH_1_BM1 0x00000000/* Flash Bank #1 if booting from flash */ #define PHYS_FLASH_1_BM0 0x0C000000/* Flash Bank #1 if booting from RAM */
Signature · 1 line
1.7.3.4
PATCH 1/3 arm: omap: innovator: fix compilation error
Sandeep,
Any comments?
Quoted 48 lines · Igor Grinberg
CONFIG_SYS_SDRAM_BASEandCONFIG_SYS_INIT_SP_ADDRwere not defined for this board thus breaking its build.Signed-off-by Igor Grinberg <[email protected]>Cc Sandeep Paulraj <[email protected]>Cc Nishant Kamat <[email protected]>Cc Kshitij Gupta <[email protected]>--- include/configs/omap1610h2.h | 7 +++++++ include/configs/omap1610inn.h | 7 +++++++ 2 files changed, 14 insertions(+), 0 deletions(-)include/configs/omap1610h2.h +7
-0@@ -152,6 +152,13 @@#define PHYS_SDRAM_1 0x10000000/* SDRAM Bank #1 */ #define PHYS_SDRAM_1_SIZE 0x02000000/* 32 MB */ +#define CONFIG_SYS_SDRAM_BASE PHYS_SDRAM_1 +#define CONFIG_SYS_INIT_RAM_SIZE 0x1000 +/* FIXME: the value below is taken from davinci which uses ARM926EJS cpu */ +#define CONFIG_SYS_INIT_SP_ADDR (CONFIG_SYS_SDRAM_BASE + \ + CONFIG_SYS_INIT_RAM_SIZE - \ + GENERATED_GBL_DATA_SIZE) +#define PHYS_FLASH_1_BM1 0x00000000/* Flash Bank #1 if booting from flash */ #define PHYS_FLASH_1_BM0 0x0C000000/* Flash Bank #1 if booting from RAM */ include/configs/omap1610inn.h +7
-0@@ -157,6 +157,13 @@#define PHYS_SDRAM_1 0x10000000/* SDRAM Bank #1 */ #define PHYS_SDRAM_1_SIZE 0x02000000/* 32 MB */ +#define CONFIG_SYS_SDRAM_BASE PHYS_SDRAM_1 +#define CONFIG_SYS_INIT_RAM_SIZE 0x1000 +/* FIXME: the value below is taken from davinci which uses ARM926EJS cpu */ +#define CONFIG_SYS_INIT_SP_ADDR (CONFIG_SYS_SDRAM_BASE + \ + CONFIG_SYS_INIT_RAM_SIZE - \ + GENERATED_GBL_DATA_SIZE) +#define PHYS_FLASH_1_BM1 0x00000000/* Flash Bank #1 if booting from flash */ #define PHYS_FLASH_1_BM0 0x0C000000/* Flash Bank #1 if booting from RAM */
Signature · 2 lines
Regards, Igor.
Paulraj
PATCH 1/3 arm: omap: innovator: fix compilation error
Quoted 5 lines
Sandeep,
Any comments?
I'm on vacation.
i'll apply it soon.
Regards,
Sandeep
PATCH 2/3 arm: omap: innovator: Prepare for mach-types.h changes
In reply to Detlev ZundelThis board used machine_is_* macros for identifying the arch number. Use compile time defines instead.
---
This has been compile tested after the 1/3 patch is applied.
board/ti/omap1610inn/omap1610innovator.c | 13 +++++++------ 1 files changed, 7 insertions(+), 6 deletions(-)
board/ti/omap1610inn/omap1610innovator.c +7 -6
@@ -63,12 +63,13 @@ static inline void delay (unsigned long loops)int board_init (void ) { -if (machine_is_omap_h2 ()) - gd->bd->bi_arch_number =MACH_TYPE_OMAP_H2 ; -else if (machine_is_omap_innovator ()) - gd->bd->bi_arch_number =MACH_TYPE_OMAP_INNOVATOR ; -else - gd->bd->bi_arch_number =MACH_TYPE_OMAP_GENERIC ; +#if defined(CONFIG_MACH_OMAP_H2 ) + gd->bd->bi_arch_number =MACH_TYPE_OMAP_H2 ; +#elif defined(CONFIG_MACH_OMAP_INNOVATOR ) + gd->bd->bi_arch_number =MACH_TYPE_OMAP_INNOVATOR ; +#else + gd->bd->bi_arch_number =MACH_TYPE_OMAP_GENERIC ; +#endif /* adress of boot parameters */ gd->bd->bi_boot_params =0x10000100 ;
Signature · 1 line
1.7.3.4
PATCH 2/3 arm: omap: innovator: Prepare for mach-types.h changes
I'm sorry for sounding rude, it's not my intention.
I didn't follow closely the discussion about mach_types.h, but I think we are heading in the wrong direction.
For example, this patch:
board/ti/omap1610inn/omap1610innovator.c static inline void delay (unsigned long loops) · line 72
Quoted 13 lines
board/ti/omap1610inn/omap1610innovator.c +7
-6-if (machine_is_omap_h2 ()) - gd->bd->bi_arch_number =MACH_TYPE_OMAP_H2 ; -else if (machine_is_omap_innovator ()) - gd->bd->bi_arch_number =MACH_TYPE_OMAP_INNOVATOR ; -else - gd->bd->bi_arch_number =MACH_TYPE_OMAP_GENERIC ; +#if defined(CONFIG_MACH_OMAP_H2 ) + gd->bd->bi_arch_number =MACH_TYPE_OMAP_H2 ; +#elif defined(CONFIG_MACH_OMAP_INNOVATOR ) + gd->bd->bi_arch_number =MACH_TYPE_OMAP_INNOVATOR ; +#else + gd->bd->bi_arch_number =MACH_TYPE_OMAP_GENERIC ; +#endif
Since when turning if into ifdef has been a wise move for maintainability?
The commis says:
Quoted 2 lines
This board used machine_is_* macros for identifying the arch number. Use compile time defines instead.
But this already was compile-time: no code generated. But even if it generated code, I prefer 3 run-time comparisons than 3 compile-time ifdefs.
Note that mach_types.h, as designed by Russell King, already had compile time selection, becuase if you selected one machine only (like in u-boot), one of the "if" becomes compile-time-true and the other ones become "0".
I see a lot of discussion about checkpatch compliance and cleanup-only patches are being accepted; this goes in the opposite direction, for no reason apparent to me.
thanks for your patience
/alessandro
PATCH 2/3 arm: omap: innovator: Prepare for mach-types.h changes
Quoted 4 lines · Alessandro Rubini
I'm sorry for sounding rude, it's not my intention.
I didn't follow closely the discussion about
mach_types.h, but I think we are heading in the wrong direction.
Exactly, this is the problem...
Please, read:
http://www.mail-archive.com/u-boot at lists.denx.de/msg51265.html
Quoted 17 lines
For example, this patch:
board/ti/omap1610inn/omap1610innovator.c static inline void delay (unsigned long loops) · line 72
Quoted 13 lines
board/ti/omap1610inn/omap1610innovator.c +7
-6-if (machine_is_omap_h2 ()) - gd->bd->bi_arch_number =MACH_TYPE_OMAP_H2 ; -else if (machine_is_omap_innovator ()) - gd->bd->bi_arch_number =MACH_TYPE_OMAP_INNOVATOR ; -else - gd->bd->bi_arch_number =MACH_TYPE_OMAP_GENERIC ; +#if defined(CONFIG_MACH_OMAP_H2 ) + gd->bd->bi_arch_number =MACH_TYPE_OMAP_H2 ; +#elif defined(CONFIG_MACH_OMAP_INNOVATOR ) + gd->bd->bi_arch_number =MACH_TYPE_OMAP_INNOVATOR ; +#else + gd->bd->bi_arch_number =MACH_TYPE_OMAP_GENERIC ; +#endif Since when turning if into ifdef has been a wise move for maintainability?
This never was...
I agree, but it at least the board won't break when the mach-types.h is cut down the way explained in this thread, so please, read the thread.
I can make another patch which will not convert the if into ifdef (either way is compile time expanded), but introduce machine_is_omap_h2() and machine_is_omap_innovator() macros definition in a board specific .h file. Do you think it will be a better solution?
Quoted 7 lines
The commis says:
Quoted 2 lines
This board used machine_is_* macros for identifying the arch number. Use compile time defines instead.
But this already was compile-time: no code generated. But even if it generated code, I prefer 3 run-time comparisons than 3 compile-time ifdefs.
This is a compile time expanded macros.
Quoted 4 lines
Note that
mach_types.h, as designed by Russell King, already had compile time selection, becuase if you selected one machine only (like in u-boot), one of the "if" becomes compile-time-true and the other ones become "0".
That is the problem... We want to move away from Russell's mach-types.h as it gets cut down to only machines supported by mainline Linux kernel and apparently does not suit U-Boot needs.
Quoted 3 lines
I see a lot of discussion about checkpatch compliance and cleanup-only patches are being accepted; this goes in the opposite direction, for no reason apparent to me.
Please, read the thread...
Signature · 2 lines
Regards, Igor.
PATCH v2 2/3 arm: omap: innovator: Prepare for mach-types.h changes
This board used machine_is_* macros for identifying the arch number. Fix this by introducing a board specific configuration variable.
---
v2: remove the ifdeferry by introducing config variable,
Alessandro, what about this one?
board/ti/omap1610inn/omap1610innovator.c | 7 +------ include/configs/omap1610h2.h | 4 +++- include/configs/omap1610inn.h | 4 +++- 3 files changed, 7 insertions(+), 8 deletions(-)
board/ti/omap1610inn/omap1610innovator.c +1 -6
@@ -63,12 +63,7 @@ static inline void delay (unsigned long loops)int board_init (void ) { -if (machine_is_omap_h2 ()) - gd->bd->bi_arch_number =MACH_TYPE_OMAP_H2 ; -else if (machine_is_omap_innovator ()) - gd->bd->bi_arch_number =MACH_TYPE_OMAP_INNOVATOR ; -else - gd->bd->bi_arch_number =MACH_TYPE_OMAP_GENERIC ; + gd->bd->bi_arch_number =CONFIG_BOARD_MACH_TYPE ;/* adress of boot parameters */ gd->bd->bi_boot_params =0x10000100 ;
include/configs/omap1610h2.h +3 -1
@@ -34,7 +34,9 @@#define CONFIG_OMAP 1/* in a TI OMAP core */ #define CONFIG_OMAP1610 1/* which is in a 1610 */ #define CONFIG_H2_OMAP1610 1/* on an H2 Board */ -#define CONFIG_MACH_OMAP_H2 /* Select board mach-type */ + +/* Select board mach-type */ +#define CONFIG_BOARD_MACH_TYPE MACH_TYPE_OMAP_H2/* input clock of PLL */ /* the OMAP1610 H2 has 12MHz input clock */
include/configs/omap1610inn.h +3 -1
@@ -34,7 +34,9 @@#define CONFIG_OMAP 1/* in a TI OMAP core */ #define CONFIG_OMAP1610 1/* which is in a 1610 */ #define CONFIG_INNOVATOROMAP1610 1/* a Innovator Board */ -#define CONFIG_MACH_OMAP_INNOVATOR /* Select board mach-type */ + +/* Select board mach-type */ +#define CONFIG_BOARD_MACH_TYPE MACH_TYPE_OMAP_INNOVATOR/* input clock of PLL */ /* the OMAP1610 Innovator has 12MHz input clock */
Signature · 1 line
1.7.3.4
PATCH v2 2/3 arm: omap: innovator: Prepare for mach-types.h changes
Dear Igor Grinberg,
Quoted 7 lines · In message <[email protected]> you…
This board used machine_is_* macros for identifying the arch number. Fix this by introducing a board specific configuration variable.
Signed-off-by Igor Grinberg <[email protected]>---
v2: remove the ifdeferry by introducing config variable,
Alessandro, what about this one?
...
board/ti/omap1610inn/omap1610innovator.c static inline void delay (unsigned long loops) · line 66
Quoted 1 line
board/ti/omap1610inn/omap1610innovator.c +1
-0+ gd->bd->bi_arch_number =CONFIG_BOARD_MACH_TYPE ;
In principle this is OK, but why do you invent yet another new CONFIG_ variable (and without documenting it) ?
We have a number of boards that already use a similar construct with CONFIG_MACH_TYPE, so I suggest you do the same.
And while being there, could you please also add a description forCONFIG_MACH_TYPE to the README? Thanks!
Best regards,
Wolfgang Denk
Signature · 6 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de If the car industry behaved like the computer industry over the last 30 years, a Rolls-Royce would cost $5, get 300 miles per gallon, and blow up once a year killing all passengers inside.
PATCH v2 2/3 arm: omap: innovator: Prepare for mach-types.h changes
Hi Wolfgang,
Quoted 6 lines · Wolfgang Denk
Dear Igor Grinberg,
Quoted 3 lines · In message <[email protected]> you…
This board used machine_is_* macros for identifying the arch number. Fix this by introducing a board specific configuration variable.
Quoted 8 lines
Quoted 4 lines
Signed-off-by Igor Grinberg <[email protected]>---
v2: remove the ifdeferry by introducing config variable,
Alessandro, what about this one?...
board/ti/omap1610inn/omap1610innovator.c static inline void delay (unsigned long loops) · line 66
Quoted 1 line
board/ti/omap1610inn/omap1610innovator.c +1
-0+ gd->bd->bi_arch_number =CONFIG_BOARD_MACH_TYPE ;In principle this is OK, but why do you invent yet another new
CONFIG_variable (and without documenting it) ?
Well, it was meant to be board specific, so local documentation in config file should be enough.
Quoted 2 lines
We have a number of boards that already use a similar construct with
CONFIG_MACH_TYPE, so I suggest you do the same.
Didn't know that, I though it is new... silly me... ;) Thanks for pointing.
Quoted 2 lines
And while being there, could you please also add a description for
CONFIG_MACH_TYPEto the README? Thanks!
I'll try my best, but this will take a while...
Other patches in this series are not affected by this one, so can be easily applied.
Signature · 2 lines
Regards, Igor.
PATCH v3 2/3 arm: omap: innovator: Prepare for mach-types.h changes
This board used machine_is_* macros for identifying the arch number. Fix this by using CONFIG_MACH_TYPE configuration variable.
---
v2: remove the ifdeferry by introducing config variable
v3: do not introduce yet another CONFIG_<var>, use existing
CONFIG_MACH_TYPE variable
board/ti/omap1610inn/omap1610innovator.c | 7 +------ include/configs/omap1610h2.h | 4 +++- include/configs/omap1610inn.h | 4 +++- 3 files changed, 7 insertions(+), 8 deletions(-)
board/ti/omap1610inn/omap1610innovator.c +1 -6
@@ -63,12 +63,7 @@ static inline void delay (unsigned long loops)int board_init (void ) { -if (machine_is_omap_h2 ()) - gd->bd->bi_arch_number =MACH_TYPE_OMAP_H2 ; -else if (machine_is_omap_innovator ()) - gd->bd->bi_arch_number =MACH_TYPE_OMAP_INNOVATOR ; -else - gd->bd->bi_arch_number =MACH_TYPE_OMAP_GENERIC ; + gd->bd->bi_arch_number =CONFIG_MACH_TYPE ;/* adress of boot parameters */ gd->bd->bi_boot_params =0x10000100 ;
include/configs/omap1610h2.h +3 -1
@@ -34,7 +34,9 @@#define CONFIG_OMAP 1/* in a TI OMAP core */ #define CONFIG_OMAP1610 1/* which is in a 1610 */ #define CONFIG_H2_OMAP1610 1/* on an H2 Board */ -#define CONFIG_MACH_OMAP_H2 /* Select board mach-type */ + +/* Select board mach-type */ +#define CONFIG_MACH_TYPE MACH_TYPE_OMAP_H2/* input clock of PLL */ /* the OMAP1610 H2 has 12MHz input clock */
include/configs/omap1610inn.h +3 -1
@@ -34,7 +34,9 @@#define CONFIG_OMAP 1/* in a TI OMAP core */ #define CONFIG_OMAP1610 1/* which is in a 1610 */ #define CONFIG_INNOVATOROMAP1610 1/* a Innovator Board */ -#define CONFIG_MACH_OMAP_INNOVATOR /* Select board mach-type */ + +/* Select board mach-type */ +#define CONFIG_MACH_TYPE MACH_TYPE_OMAP_INNOVATOR/* input clock of PLL */ /* the OMAP1610 Innovator has 12MHz input clock */
Signature · 1 line
1.7.3.4
PATCH 3/3 arm: at91: ether: Prepare for mach-types.h changes
In reply to Detlev Zundelat91 ethernet module used machine_is_cbs337() macro for board specific Linux compatibility issue.
Use compile time defines instead.
--- arch/arm/cpu/arm920t/at91rm9200/ether.c | 18 +++++++++--------- 1 files changed, 9 insertions(+), 9 deletions(-)
arch/arm/cpu/arm920t/at91rm9200/ether.c +9 -9
@@ -201,15 +201,15 @@ int eth_init (bd_t * bd)* that MicroMonitor behavior so we avoid needing to make such OS code * care about which bootloader was used. */ -if (machine_is_csb337 ()) { - p_mac->EMAC_SA2H = (enetaddr[0 ] <<8 ) | (enetaddr[1 ]); - p_mac->EMAC_SA2L = (enetaddr[2 ] <<24 ) | (enetaddr[3 ] <<16 ) - | (enetaddr[4 ] <<8 ) | (enetaddr[5 ]); - }else { - p_mac->EMAC_SA2L = (enetaddr[3 ] <<24 ) | (enetaddr[2 ] <<16 ) - | (enetaddr[1 ] <<8 ) | (enetaddr[0 ]); - p_mac->EMAC_SA2H = (enetaddr[5 ] <<8 ) | (enetaddr[4 ]); - } +#ifdef CONFIG_MACH_CSB337 + p_mac->EMAC_SA2H = (enetaddr[0 ] <<8 ) | (enetaddr[1 ]); + p_mac->EMAC_SA2L = (enetaddr[2 ] <<24 ) | (enetaddr[3 ] <<16 ) + | (enetaddr[4 ] <<8 ) | (enetaddr[5 ]); +#else + p_mac->EMAC_SA2L = (enetaddr[3 ] <<24 ) | (enetaddr[2 ] <<16 ) + | (enetaddr[1 ] <<8 ) | (enetaddr[0 ]); + p_mac->EMAC_SA2H = (enetaddr[5 ] <<8 ) | (enetaddr[4 ]); +#endif p_mac->EMAC_RBQP = (long ) (&rbfdt[0 ]); p_mac->EMAC_RSR &= ~(AT91C_EMAC_RSR_OVR |AT91C_EMAC_REC |AT91C_EMAC_BNA );
Signature · 1 line
1.7.3.4
PATCH 3/3 arm: at91: ether: Prepare for mach-types.h changes
Dear Igor Grinberg,
Quoted 28 lines
at91 ethernet module used
machine_is_cbs337()macro for board specific Linux compatibility issue.
Use compile time defines instead.Signed-off-by Igor Grinberg<[email protected]>--- arch/arm/cpu/arm920t/at91rm9200/ether.c | 18 +++++++++--------- 1 files changed, 9 insertions(+), 9 deletions(-)arch/arm/cpu/arm920t/at91rm9200/ether.c +2
-9@@ -201,15 +201,15 @@ int eth_init (bd_t * bd)* that MicroMonitor behavior so we avoid needing to make such OS code * care about which bootloader was used. */ -if (machine_is_csb337 ()) { - p_mac->EMAC_SA2H = (enetaddr[0 ]<<8 ) | (enetaddr[1 ]); - p_mac->EMAC_SA2L = (enetaddr[2 ]<<24 ) | (enetaddr[3 ]<<16 ) - | (enetaddr[4 ]<<8 ) | (enetaddr[5 ]); - }else { - p_mac->EMAC_SA2L = (enetaddr[3 ]<<24 ) | (enetaddr[2 ]<<16 ) - | (enetaddr[1 ]<<8 ) | (enetaddr[0 ]); - p_mac->EMAC_SA2H = (enetaddr[5 ]<<8 ) | (enetaddr[4 ]); - } +#ifdef CONFIG_MACH_CSB337 + p_mac->EMAC_SA2H = (enetaddr[0 ]<<8 ) | (enetaddr[1 ]);
arch/arm/cpu/arm920t/at91rm9200/ether.c int eth_init (bd_t * bd) · line 215
Quoted 10 lines
arch/arm/cpu/arm920t/at91rm9200/ether.c +7
-0+ p_mac->EMAC_SA2L = (enetaddr[2 ]<<24 ) | (enetaddr[3 ]<<16 ) + | (enetaddr[4 ]<<8 ) | (enetaddr[5 ]); +#else + p_mac->EMAC_SA2L = (enetaddr[3 ]<<24 ) | (enetaddr[2 ]<<16 ) + | (enetaddr[1 ]<<8 ) | (enetaddr[0 ]); + p_mac->EMAC_SA2H = (enetaddr[5 ]<<8 ) | (enetaddr[4 ]); +#endif p_mac->EMAC_RBQP = (long ) (&rbfdt[0 ]); p_mac->EMAC_RSR&= ~(AT91C_EMAC_RSR_OVR |AT91C_EMAC_REC |AT91C_EMAC_BNA );
There is nothing wrong with your patch itself, but it let me to take a closer look at the reasoning of why there is a machine dependency. The full code at this section is:
eth_getenv_enetaddr("ethaddr", enetaddr);
/* The CSB337 originally used a version of the MicroMonitor bootloader
* which saved Ethernet addresses in the "wrong" order. Operating
* systems (like Linux) know this, and apply a workaround. Replicate
* that MicroMonitor behavior so we avoid needing to make such OS code
* care about which bootloader was used.
*/
if (machine_is_csb337()) {
p_mac->EMAC_SA2H = (enetaddr[0] << 8) | (enetaddr[1]);
p_mac->EMAC_SA2L = (enetaddr[2] << 24) | (enetaddr[3] << 16)
| (enetaddr[4] << 8) | (enetaddr[5]);
} else {
p_mac->EMAC_SA2L = (enetaddr[3] << 24) | (enetaddr[2] << 16)
| (enetaddr[1] << 8) | (enetaddr[0]);
p_mac->EMAC_SA2H = (enetaddr[5] << 8) | (enetaddr[4]);
}So, for the sake of a(nother) broken bootloader and a workaround in Linux we store the MAC address in the wrong order? What if U-Boot itself is used to make LAN accesses?
Apart from that, it feels entirely wrong to do so. Fix the kernel to NOT do a workaround instead should be the better approach.
Any opinions by Ben or Wolfgang on this?
Best Regards,
Reinhard
PATCH 3/3 arm: at91: ether: Prepare for mach-types.h changes
Quoted 62 lines · Reinhard Meyer
Dear Igor Grinberg,
Quoted 28 lines
at91 ethernet module used
machine_is_cbs337()macro for board specific Linux compatibility issue.
Use compile time defines instead.Signed-off-by Igor Grinberg<[email protected]>--- arch/arm/cpu/arm920t/at91rm9200/ether.c | 18 +++++++++--------- 1 files changed, 9 insertions(+), 9 deletions(-)arch/arm/cpu/arm920t/at91rm9200/ether.c +2
-9@@ -201,15 +201,15 @@ int eth_init (bd_t * bd)* that MicroMonitor behavior so we avoid needing to make such OS code * care about which bootloader was used. */ -if (machine_is_csb337 ()) { - p_mac->EMAC_SA2H = (enetaddr[0 ]<<8 ) | (enetaddr[1 ]); - p_mac->EMAC_SA2L = (enetaddr[2 ]<<24 ) | (enetaddr[3 ]<<16 ) - | (enetaddr[4 ]<<8 ) | (enetaddr[5 ]); - }else { - p_mac->EMAC_SA2L = (enetaddr[3 ]<<24 ) | (enetaddr[2 ]<<16 ) - | (enetaddr[1 ]<<8 ) | (enetaddr[0 ]); - p_mac->EMAC_SA2H = (enetaddr[5 ]<<8 ) | (enetaddr[4 ]); - } +#ifdef CONFIG_MACH_CSB337 + p_mac->EMAC_SA2H = (enetaddr[0 ]<<8 ) | (enetaddr[1 ]);arch/arm/cpu/arm920t/at91rm9200/ether.c int eth_init (bd_t * bd) · line 215
Quoted 10 lines
arch/arm/cpu/arm920t/at91rm9200/ether.c +7
-0+ p_mac->EMAC_SA2L = (enetaddr[2 ]<<24 ) | (enetaddr[3 ]<<16 ) + | (enetaddr[4 ]<<8 ) | (enetaddr[5 ]); +#else + p_mac->EMAC_SA2L = (enetaddr[3 ]<<24 ) | (enetaddr[2 ]<<16 ) + | (enetaddr[1 ]<<8 ) | (enetaddr[0 ]); + p_mac->EMAC_SA2H = (enetaddr[5 ]<<8 ) | (enetaddr[4 ]); +#endif p_mac->EMAC_RBQP = (long ) (&rbfdt[0 ]); p_mac->EMAC_RSR&= ~(AT91C_EMAC_RSR_OVR |AT91C_EMAC_REC |AT91C_EMAC_BNA );There is nothing wrong with your patch itself, but it let me to take a closer look at the reasoning of why there is a machine dependency. The full code at this section is:
eth_getenv_enetaddr("ethaddr", enetaddr); /* The CSB337 originally used a version of the MicroMonitor bootloader * which saved Ethernet addresses in the "wrong" order. Operating * systems (like Linux) know this, and apply a workaround. Replicate * that MicroMonitor behavior so we avoid needing to make such OS code * care about which bootloader was used. */ if (machine_is_csb337()) { p_mac->EMAC_SA2H = (enetaddr[0] << 8) | (enetaddr[1]); p_mac->EMAC_SA2L = (enetaddr[2] << 24) | (enetaddr[3] << 16) | (enetaddr[4] << 8) | (enetaddr[5]); } else { p_mac->EMAC_SA2L = (enetaddr[3] << 24) | (enetaddr[2] << 16) | (enetaddr[1] << 8) | (enetaddr[0]); p_mac->EMAC_SA2H = (enetaddr[5] << 8) | (enetaddr[4]); }
Quoted 3 lines
So, for the sake of a(nother) broken bootloader and a workaround in Linux we store the MAC address in the wrong order? What if U-Boot itself is used to make LAN accesses?
Well, I've read the comment before preparing the patch. Actually, I felt like: "this should be thrown away!". Also, I haven't found csb337 board in the tree...
I didn't want to decide for you (If I'm not mistaken, you are the maintainer of Atmel) what to do with it, so I left it. Do you think we should remove this?
I would love to send another patch to remove this completely.
Quoted 3 lines
Apart from that, it feels entirely wrong to do so. Fix the kernel to NOT do a workaround instead should be the better approach.
Yep, I totally agree...
Quoted 3 lines
Any opinions by Ben or Wolfgang on this?
Signature · 2 lines
Regards, Igor.
PATCH 3/3 arm: at91: ether: Prepare for mach-types.h changes
Hi Igor,
Quoted 68 lines · Reinhard Meyer
Quoted 63 lines
Dear Igor Grinberg,
Quoted 28 lines
at91 ethernet module used
machine_is_cbs337()macro for board specific Linux compatibility issue.
Use compile time defines instead.Signed-off-by Igor Grinberg<[email protected]>--- arch/arm/cpu/arm920t/at91rm9200/ether.c | 18 +++++++++--------- 1 files changed, 9 insertions(+), 9 deletions(-)arch/arm/cpu/arm920t/at91rm9200/ether.c +2
-9@@ -201,15 +201,15 @@ int eth_init (bd_t * bd)* that MicroMonitor behavior so we avoid needing to make such OS code * care about which bootloader was used. */ -if (machine_is_csb337 ()) { - p_mac->EMAC_SA2H = (enetaddr[0 ]<<8 ) | (enetaddr[1 ]); - p_mac->EMAC_SA2L = (enetaddr[2 ]<<24 ) | (enetaddr[3 ]<<16 ) - | (enetaddr[4 ]<<8 ) | (enetaddr[5 ]); - }else { - p_mac->EMAC_SA2L = (enetaddr[3 ]<<24 ) | (enetaddr[2 ]<<16 ) - | (enetaddr[1 ]<<8 ) | (enetaddr[0 ]); - p_mac->EMAC_SA2H = (enetaddr[5 ]<<8 ) | (enetaddr[4 ]); - } +#ifdef CONFIG_MACH_CSB337 + p_mac->EMAC_SA2H = (enetaddr[0 ]<<8 ) | (enetaddr[1 ]);arch/arm/cpu/arm920t/at91rm9200/ether.c int eth_init (bd_t * bd) · line 215
Quoted 10 lines
arch/arm/cpu/arm920t/at91rm9200/ether.c +7
-0+ p_mac->EMAC_SA2L = (enetaddr[2 ]<<24 ) | (enetaddr[3 ]<<16 ) + | (enetaddr[4 ]<<8 ) | (enetaddr[5 ]); +#else + p_mac->EMAC_SA2L = (enetaddr[3 ]<<24 ) | (enetaddr[2 ]<<16 ) + | (enetaddr[1 ]<<8 ) | (enetaddr[0 ]); + p_mac->EMAC_SA2H = (enetaddr[5 ]<<8 ) | (enetaddr[4 ]); +#endif p_mac->EMAC_RBQP = (long ) (&rbfdt[0 ]); p_mac->EMAC_RSR&= ~(AT91C_EMAC_RSR_OVR |AT91C_EMAC_REC |AT91C_EMAC_BNA );There is nothing wrong with your patch itself, but it let me to take a closer look at the reasoning of why there is a machine dependency. The full code at this section is:
eth_getenv_enetaddr("ethaddr", enetaddr); /* The CSB337 originally used a version of the MicroMonitor bootloader * which saved Ethernet addresses in the "wrong" order. Operating * systems (like Linux) know this, and apply a workaround. Replicate * that MicroMonitor behavior so we avoid needing to make such OS code * care about which bootloader was used. */ if (machine_is_csb337()) { p_mac->EMAC_SA2H = (enetaddr[0] << 8) | (enetaddr[1]); p_mac->EMAC_SA2L = (enetaddr[2] << 24) | (enetaddr[3] << 16) | (enetaddr[4] << 8) | (enetaddr[5]); } else { p_mac->EMAC_SA2L = (enetaddr[3] << 24) | (enetaddr[2] << 16) | (enetaddr[1] << 8) | (enetaddr[0]); p_mac->EMAC_SA2H = (enetaddr[5] << 8) | (enetaddr[4]); }Quoted 3 lines
So, for the sake of a(nother) broken bootloader and a workaround in Linux we store the MAC address in the wrong order? What if U-Boot itself is used to make LAN accesses?
Quoted 7 lines
Well, I've read the comment before preparing the patch. Actually, I felt like: "this should be thrown away!". Also, I haven't found csb337 board in the tree...
I didn't want to decide for you (If I'm not mistaken, you are the maintainer of Atmel) what to do with it, so I left it. Do you think we should remove this?
I would love to send another patch to remove this completely.
I'd say remove it. Why do I say that?
[dzu at pollux u-boot-testing (master)]$ make csb337_config
make: *** No rule to make target `csb337_config'. Stop.
make: *** [csb337_config] Error 1
[dzu at pollux u-boot-testing (master)]$ grep -i csb337 Makefile [dzu at pollux u-boot-testing (master)]$ grep -i csb337 boards.cfg [dzu at pollux u-boot-testing (master)]$
Cheers
Detlev
Signature · 7 lines
Indeed, the author firmly believes that the best serious work is also
good fun. We needn't apologize if we enjoy doing research.
-- Donald Knuth
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.dePATCH v2 3/3 arm: at91: ether: Prepare for mach-types.h changes
at91 ethernet module used machine_is_cbs337() macro for board specific Linux compatibility issue.
Remove this, as no such board exist in current U-Boot tree.
--- arch/arm/cpu/arm920t/at91rm9200/ether.c | 18 +++--------------- 1 files changed, 3 insertions(+), 15 deletions(-)
arch/arm/cpu/arm920t/at91rm9200/ether.c +3 -15
@@ -195,21 +195,9 @@ int eth_init (bd_t * bd)eth_getenv_enetaddr ("ethaddr" , enetaddr); -/* The CSB337 originally used a version of the MicroMonitor bootloader -* which saved Ethernet addresses in the "wrong" order. Operating -* systems (like Linux) know this, and apply a workaround. Replicate -* that MicroMonitor behavior so we avoid needing to make such OS code -* care about which bootloader was used. -*/ -if (machine_is_csb337 ()) { - p_mac->EMAC_SA2H = (enetaddr[0 ] <<8 ) | (enetaddr[1 ]); - p_mac->EMAC_SA2L = (enetaddr[2 ] <<24 ) | (enetaddr[3 ] <<16 ) - | (enetaddr[4 ] <<8 ) | (enetaddr[5 ]); - }else { - p_mac->EMAC_SA2L = (enetaddr[3 ] <<24 ) | (enetaddr[2 ] <<16 ) - | (enetaddr[1 ] <<8 ) | (enetaddr[0 ]); - p_mac->EMAC_SA2H = (enetaddr[5 ] <<8 ) | (enetaddr[4 ]); - } + p_mac->EMAC_SA2L = (enetaddr[3 ] <<24 ) | (enetaddr[2 ] <<16 ) + | (enetaddr[1 ] <<8 ) | (enetaddr[0 ]); + p_mac->EMAC_SA2H = (enetaddr[5 ] <<8 ) | (enetaddr[4 ]); p_mac->EMAC_RBQP = (long ) (&rbfdt[0 ]); p_mac->EMAC_RSR &= ~(AT91C_EMAC_RSR_OVR |AT91C_EMAC_REC |AT91C_EMAC_BNA );
Signature · 1 line
1.7.3.4
PATCH v2 3/3 arm: at91: ether: Prepare for mach-types.h changes
ping!
It has been two weeks...
Quoted 38 lines · Igor Grinberg
at91 ethernet module used
machine_is_cbs337()macro for board specific Linux compatibility issue.
Remove this, as no such board exist in current U-Boot tree.Signed-off-by Igor Grinberg <[email protected]>--- arch/arm/cpu/arm920t/at91rm9200/ether.c | 18 +++--------------- 1 files changed, 3 insertions(+), 15 deletions(-)arch/arm/cpu/arm920t/at91rm9200/ether.c +3
-15@@ -195,21 +195,9 @@ int eth_init (bd_t * bd)eth_getenv_enetaddr ("ethaddr" , enetaddr); -/* The CSB337 originally used a version of the MicroMonitor bootloader -* which saved Ethernet addresses in the "wrong" order. Operating -* systems (like Linux) know this, and apply a workaround. Replicate -* that MicroMonitor behavior so we avoid needing to make such OS code -* care about which bootloader was used. -*/ -if (machine_is_csb337 ()) { - p_mac->EMAC_SA2H = (enetaddr[0 ] <<8 ) | (enetaddr[1 ]); - p_mac->EMAC_SA2L = (enetaddr[2 ] <<24 ) | (enetaddr[3 ] <<16 ) - | (enetaddr[4 ] <<8 ) | (enetaddr[5 ]); - }else { - p_mac->EMAC_SA2L = (enetaddr[3 ] <<24 ) | (enetaddr[2 ] <<16 ) - | (enetaddr[1 ] <<8 ) | (enetaddr[0 ]); - p_mac->EMAC_SA2H = (enetaddr[5 ] <<8 ) | (enetaddr[4 ]); - } + p_mac->EMAC_SA2L = (enetaddr[3 ] <<24 ) | (enetaddr[2 ] <<16 ) + | (enetaddr[1 ] <<8 ) | (enetaddr[0 ]); + p_mac->EMAC_SA2H = (enetaddr[5 ] <<8 ) | (enetaddr[4 ]); p_mac->EMAC_RBQP = (long ) (&rbfdt[0 ]); p_mac->EMAC_RSR &= ~(AT91C_EMAC_RSR_OVR |AT91C_EMAC_REC |AT91C_EMAC_BNA );
Signature · 2 lines
Regards, Igor.
Hi Reinhard,
[...]
Quoted 4 lines
why don't we just create the #define
MACH_xxxlines directly from the "http://www.arm.linux.org.uk/developer/machines/download.php". We don't need all the *_is_* macros in u-boot anyway. Then we would have just a few 1000 lines of #define MACH_*
... and we could update whenever we feel like it and decouple from the linux interval. This looks like a nice clean backwards compatible solution.
Cheers
Detlev
Signature · 6 lines
"Win32 sucks so hard it could pull matter out of a Black Hole."
-- Pohl Longsine
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de