* [U-Boot] Update and Cut down mach types @ 2011-04-19 12:42 Paulraj, Sandeep 2011-04-19 13:39 ` Matthias Weißer 2011-04-20 8:58 ` Igor Grinberg 0 siblings, 2 replies; 38+ messages in thread From: Paulraj, Sandeep @ 2011-04-19 12:42 UTC (permalink / raw) To: u-boot Wolfgang, Albert, Russell King sent some updates to the linux kernel for mach-types. http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=6f82f4db80189281a8ac42f2e72396accb719b57 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 ^ permalink raw reply [flat|nested] 38+ messages in thread
* [U-Boot] Update and Cut down mach types 2011-04-19 12:42 [U-Boot] Update and Cut down mach types Paulraj, Sandeep @ 2011-04-19 13:39 ` Matthias Weißer 2011-04-19 13:45 ` Paulraj, Sandeep 2011-04-19 14:21 ` Wolfgang Denk 2011-04-20 8:58 ` Igor Grinberg 1 sibling, 2 replies; 38+ messages in thread From: Matthias Weißer @ 2011-04-19 13:39 UTC (permalink / raw) To: u-boot Hello Sandeep, Wolfgang Am 19.04.2011 14:42, schrieb Paulraj, Sandeep: > Wolfgang, Albert, > > Russell King sent some updates to the linux kernel for mach-types. > > http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=6f82f4db80189281a8ac42f2e72396accb719b57 > > 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 ^ permalink raw reply [flat|nested] 38+ messages in thread
* [U-Boot] Update and Cut down mach types 2011-04-19 13:39 ` Matthias Weißer @ 2011-04-19 13:45 ` Paulraj, Sandeep 2011-04-20 8:44 ` Albert ARIBAUD 2011-04-19 14:21 ` Wolfgang Denk 1 sibling, 1 reply; 38+ messages in thread From: Paulraj, Sandeep @ 2011-04-19 13:45 UTC (permalink / raw) To: u-boot > > Hello Sandeep, Wolfgang > > Am 19.04.2011 14:42, schrieb Paulraj, Sandeep: > > Wolfgang, Albert, > > > > Russell King sent some updates to the linux kernel for mach-types. > > > > http://git.kernel.org/?p=linux/kernel/git/torvalds/linux- > 2.6.git;a=commitdiff;h=6f82f4db80189281a8ac42f2e72396accb719b57 > > > > 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. We are not removing this board from u-boot > What will be the > solution for ARM but non-Linux u-boot ports then? What should be passed > to gd->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 ^ permalink raw reply [flat|nested] 38+ messages in thread
* [U-Boot] Update and Cut down mach types 2011-04-19 13:45 ` Paulraj, Sandeep @ 2011-04-20 8:44 ` Albert ARIBAUD 0 siblings, 0 replies; 38+ messages in thread From: Albert ARIBAUD @ 2011-04-20 8:44 UTC (permalink / raw) To: u-boot Le 19/04/2011 15:45, Paulraj, Sandeep a ?crit : > > >> >> Hello Sandeep, Wolfgang >> >> Am 19.04.2011 14:42, schrieb Paulraj, Sandeep: >>> Wolfgang, Albert, >>> >>> Russell King sent some updates to the linux kernel for mach-types. >>> >>> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux- >> 2.6.git;a=commitdiff;h=6f82f4db80189281a8ac42f2e72396accb719b57 >>> >>> 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. > > We are not removing this board from u-boot > >> What will be the >> solution for ARM but non-Linux u-boot ports then? What should be passed >> to gd->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 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, -- Albert. ^ permalink raw reply [flat|nested] 38+ messages in thread
* [U-Boot] Update and Cut down mach types 2011-04-19 13:39 ` Matthias Weißer 2011-04-19 13:45 ` Paulraj, Sandeep @ 2011-04-19 14:21 ` Wolfgang Denk 2011-04-19 18:42 ` Matthias Weisser 2011-04-19 18:44 ` Michael Schwingen 1 sibling, 2 replies; 38+ messages in thread From: Wolfgang Denk @ 2011-04-19 14:21 UTC (permalink / raw) To: u-boot Dear =?ISO-8859-1?Q?Matthias_Wei=DFer?=, In message <4DAD90AF.2080105@arcor.de> 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_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? 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 -- 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_ ^ permalink raw reply [flat|nested] 38+ messages in thread
* [U-Boot] Update and Cut down mach types 2011-04-19 14:21 ` Wolfgang Denk @ 2011-04-19 18:42 ` Matthias Weisser 2011-04-19 18:44 ` Michael Schwingen 1 sibling, 0 replies; 38+ messages in thread From: Matthias Weisser @ 2011-04-19 18:42 UTC (permalink / raw) To: u-boot Hello Wolfgang Am 19.04.2011 16:21, schrieb Wolfgang Denk: > Dear =?ISO-8859-1?Q?Matthias_Wei=DFer?=, >> 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. > 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 ^ permalink raw reply [flat|nested] 38+ messages in thread
* [U-Boot] Update and Cut down mach types 2011-04-19 14:21 ` Wolfgang Denk 2011-04-19 18:42 ` Matthias Weisser @ 2011-04-19 18:44 ` Michael Schwingen 2011-04-20 8:15 ` Detlev Zundel 1 sibling, 1 reply; 38+ messages in thread From: Michael Schwingen @ 2011-04-19 18:44 UTC (permalink / raw) To: u-boot Am 04/19/2011 04:21 PM, schrieb Wolfgang Denk: > Dear =?ISO-8859-1?Q?Matthias_Wei=DFer?=, > > In message <4DAD90AF.2080105@arcor.de> 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_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? > 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 <linux@arm.linux.org.uk> To: Michael Schwingen <rincewind@discworld.dascon.de> 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. ^ permalink raw reply [flat|nested] 38+ messages in thread
* [U-Boot] Update and Cut down mach types 2011-04-19 18:44 ` Michael Schwingen @ 2011-04-20 8:15 ` Detlev Zundel 0 siblings, 0 replies; 38+ messages in thread From: Detlev Zundel @ 2011-04-20 8:15 UTC (permalink / raw) To: u-boot Hi Michael, [...] > I did that and got the following reply (without quotes due to cut-and-paste) [...] > 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 -- 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.de ^ permalink raw reply [flat|nested] 38+ messages in thread
* [U-Boot] Update and Cut down mach types 2011-04-19 12:42 [U-Boot] Update and Cut down mach types Paulraj, Sandeep 2011-04-19 13:39 ` Matthias Weißer @ 2011-04-20 8:58 ` Igor Grinberg 2011-04-20 17:15 ` Michael Schwingen 1 sibling, 1 reply; 38+ messages in thread From: Igor Grinberg @ 2011-04-20 8:58 UTC (permalink / raw) To: u-boot Hi Sandeep, Albert, Wolfgang, On 04/19/11 15:42, Paulraj, Sandeep wrote: > Wolfgang, Albert, > > Russell King sent some updates to the linux kernel for mach-types. > > http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=6f82f4db80189281a8ac42f2e72396accb719b57 > > 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. > 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. > > Please review and if it is acceptable to everyone then we should apply this patch to remain in sync with the mainline kernel. [...] -- Regards, Igor. ^ permalink raw reply [flat|nested] 38+ messages in thread
* [U-Boot] Update and Cut down mach types 2011-04-20 8:58 ` Igor Grinberg @ 2011-04-20 17:15 ` Michael Schwingen 2011-04-20 17:49 ` Albert ARIBAUD 0 siblings, 1 reply; 38+ messages in thread From: Michael Schwingen @ 2011-04-20 17:15 UTC (permalink / raw) To: u-boot On 04/20/2011 10:58 AM, Igor Grinberg wrote: > Hi Sandeep, Albert, Wolfgang, > > On 04/19/11 15:42, Paulraj, Sandeep wrote: >> Wolfgang, Albert, >> >> Russell King sent some updates to the linux kernel for mach-types. >> >> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=6f82f4db80189281a8ac42f2e72396accb719b57 >> >> 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. Correct. > 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. 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? >> 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 ^ permalink raw reply [flat|nested] 38+ messages in thread
* [U-Boot] Update and Cut down mach types 2011-04-20 17:15 ` Michael Schwingen @ 2011-04-20 17:49 ` Albert ARIBAUD 2011-04-20 19:26 ` Michael Schwingen 0 siblings, 1 reply; 38+ messages in thread From: Albert ARIBAUD @ 2011-04-20 17:49 UTC (permalink / raw) To: u-boot Le 20/04/2011 19:15, Michael Schwingen a ?crit : > 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. >> 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? > cu > Michael Amicalement, -- Albert. ^ permalink raw reply [flat|nested] 38+ messages in thread
* [U-Boot] Update and Cut down mach types 2011-04-20 17:49 ` Albert ARIBAUD @ 2011-04-20 19:26 ` Michael Schwingen 2011-04-21 11:39 ` Albert ARIBAUD 0 siblings, 1 reply; 38+ messages in thread From: Michael Schwingen @ 2011-04-20 19:26 UTC (permalink / raw) To: u-boot On 04/20/2011 07:49 PM, Albert ARIBAUD wrote: > Le 20/04/2011 19:15, Michael Schwingen a ?crit : > >> 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. >>> 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? 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 ^ permalink raw reply [flat|nested] 38+ messages in thread
* [U-Boot] Update and Cut down mach types 2011-04-20 19:26 ` Michael Schwingen @ 2011-04-21 11:39 ` Albert ARIBAUD 2011-04-26 18:14 ` Michael Schwingen 0 siblings, 1 reply; 38+ messages in thread From: Albert ARIBAUD @ 2011-04-21 11:39 UTC (permalink / raw) To: u-boot Le 20/04/2011 21:26, Michael Schwingen a ?crit : > On 04/20/2011 07:49 PM, Albert ARIBAUD wrote: >> Le 20/04/2011 19:15, Michael Schwingen a ?crit : >> >>> 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. > > >>>> 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? > 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, -- Albert. ^ permalink raw reply [flat|nested] 38+ messages in thread
* [U-Boot] Update and Cut down mach types 2011-04-21 11:39 ` Albert ARIBAUD @ 2011-04-26 18:14 ` Michael Schwingen 2011-04-26 19:40 ` Wolfgang Denk 0 siblings, 1 reply; 38+ messages in thread From: Michael Schwingen @ 2011-04-26 18:14 UTC (permalink / raw) To: u-boot Am 04/21/2011 01:39 PM, schrieb Albert ARIBAUD: > Le 20/04/2011 21:26, Michael Schwingen a ?crit : >> 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 > 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. 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 ^ permalink raw reply [flat|nested] 38+ messages in thread
* [U-Boot] Update and Cut down mach types 2011-04-26 18:14 ` Michael Schwingen @ 2011-04-26 19:40 ` Wolfgang Denk 2011-04-26 20:38 ` Albert ARIBAUD 0 siblings, 1 reply; 38+ messages in thread From: Wolfgang Denk @ 2011-04-26 19:40 UTC (permalink / raw) To: u-boot Dear Michael Schwingen, In message <4DB70B98.6040208@discworld.dascon.de> you wrote: > > 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. 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 -- 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 ^ permalink raw reply [flat|nested] 38+ messages in thread
* [U-Boot] Update and Cut down mach types 2011-04-26 19:40 ` Wolfgang Denk @ 2011-04-26 20:38 ` Albert ARIBAUD 2011-04-26 21:32 ` Wolfgang Denk 0 siblings, 1 reply; 38+ messages in thread From: Albert ARIBAUD @ 2011-04-26 20:38 UTC (permalink / raw) To: u-boot Le 26/04/2011 21:40, Wolfgang Denk a ?crit : > Dear Michael Schwingen, > > In message<4DB70B98.6040208@discworld.dascon.de> you wrote: >> >> 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. > > 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? 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. > Best regards, > > Wolfgang Denk Amicalement, -- Albert. ^ permalink raw reply [flat|nested] 38+ messages in thread
* [U-Boot] Update and Cut down mach types 2011-04-26 20:38 ` Albert ARIBAUD @ 2011-04-26 21:32 ` Wolfgang Denk 2011-04-26 21:38 ` Reinhard Meyer 0 siblings, 1 reply; 38+ messages in thread From: Wolfgang Denk @ 2011-04-26 21:32 UTC (permalink / raw) To: u-boot Dear Albert ARIBAUD, In message <4DB72D4A.5070102@aribaud.net> 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. > 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. > 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 -- 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 ^ permalink raw reply [flat|nested] 38+ messages in thread
* [U-Boot] Update and Cut down mach types 2011-04-26 21:32 ` Wolfgang Denk @ 2011-04-26 21:38 ` Reinhard Meyer 2011-04-27 10:19 ` Michael Schwingen 2011-04-27 11:44 ` [U-Boot] Update and Cut down mach types Detlev Zundel 0 siblings, 2 replies; 38+ messages in thread From: Reinhard Meyer @ 2011-04-26 21:38 UTC (permalink / raw) To: u-boot On 26.04.2011 23:32, Wolfgang Denk wrote: > Dear Albert ARIBAUD, > > In message<4DB72D4A.5070102@aribaud.net> 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. > >> 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. > >> 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. 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 ^ permalink raw reply [flat|nested] 38+ messages in thread
* [U-Boot] Update and Cut down mach types 2011-04-26 21:38 ` Reinhard Meyer @ 2011-04-27 10:19 ` Michael Schwingen 2011-04-28 6:20 ` Igor Grinberg 2011-04-27 11:44 ` [U-Boot] Update and Cut down mach types Detlev Zundel 1 sibling, 1 reply; 38+ messages in thread From: Michael Schwingen @ 2011-04-27 10:19 UTC (permalink / raw) To: u-boot Am 04/26/2011 11:38 PM, schrieb Reinhard Meyer: >> >>> 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. > > > 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_* > 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 ^ permalink raw reply [flat|nested] 38+ messages in thread
* [U-Boot] Update and Cut down mach types 2011-04-27 10:19 ` Michael Schwingen @ 2011-04-28 6:20 ` Igor Grinberg 2011-04-29 8:58 ` Detlev Zundel 0 siblings, 1 reply; 38+ messages in thread From: Igor Grinberg @ 2011-04-28 6:20 UTC (permalink / raw) To: u-boot On 04/27/11 13:19, Michael Schwingen wrote: > Am 04/26/2011 11:38 PM, schrieb Reinhard Meyer: >>>> 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. >> >> 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_* >> > 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, +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()) -- Regards, Igor. ^ permalink raw reply [flat|nested] 38+ messages in thread
* [U-Boot] Update and Cut down mach types 2011-04-28 6:20 ` Igor Grinberg @ 2011-04-29 8:58 ` Detlev Zundel 2011-05-01 10:10 ` [U-Boot] [PATCH 1/3] arm: omap: innovator: fix compilation error Igor Grinberg ` (2 more replies) 0 siblings, 3 replies; 38+ messages in thread From: Detlev Zundel @ 2011-04-29 8:58 UTC (permalink / raw) To: u-boot Hi Igor, [...] > +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 -- 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.de ^ permalink raw reply [flat|nested] 38+ messages in thread
* [U-Boot] [PATCH 1/3] arm: omap: innovator: fix compilation error 2011-04-29 8:58 ` Detlev Zundel @ 2011-05-01 10:10 ` Igor Grinberg 2011-05-17 12:40 ` Igor Grinberg 2011-05-01 10:10 ` [U-Boot] [PATCH 2/3] arm: omap: innovator: Prepare for mach-types.h changes Igor Grinberg 2011-05-01 10:10 ` [U-Boot] [PATCH 3/3] arm: at91: ether: " Igor Grinberg 2 siblings, 1 reply; 38+ messages in thread From: Igor Grinberg @ 2011-05-01 10:10 UTC (permalink / raw) To: u-boot CONFIG_SYS_SDRAM_BASE and CONFIG_SYS_INIT_SP_ADDR were not defined for this board thus breaking its build. Signed-off-by: Igor Grinberg <grinberg@compulab.co.il> Cc: Sandeep Paulraj <s-paulraj@ti.com> Cc: Nishant Kamat <nskamat@ti.com> Cc: Kshitij Gupta <kshitij@ti.com> --- include/configs/omap1610h2.h | 7 +++++++ include/configs/omap1610inn.h | 7 +++++++ 2 files changed, 14 insertions(+), 0 deletions(-) diff --git a/include/configs/omap1610h2.h b/include/configs/omap1610h2.h index 2936dcc..7e53ae6 100644 --- a/include/configs/omap1610h2.h +++ b/include/configs/omap1610h2.h @@ -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 */ diff --git a/include/configs/omap1610inn.h b/include/configs/omap1610inn.h index 0b41c46..be569a3 100644 --- a/include/configs/omap1610inn.h +++ b/include/configs/omap1610inn.h @@ -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 */ -- 1.7.3.4 ^ permalink raw reply related [flat|nested] 38+ messages in thread
* [U-Boot] [PATCH 1/3] arm: omap: innovator: fix compilation error 2011-05-01 10:10 ` [U-Boot] [PATCH 1/3] arm: omap: innovator: fix compilation error Igor Grinberg @ 2011-05-17 12:40 ` Igor Grinberg 2011-05-21 21:40 ` Paulraj, Sandeep 0 siblings, 1 reply; 38+ messages in thread From: Igor Grinberg @ 2011-05-17 12:40 UTC (permalink / raw) To: u-boot Sandeep, Any comments? On 05/01/11 13:10, Igor Grinberg wrote: > CONFIG_SYS_SDRAM_BASE and CONFIG_SYS_INIT_SP_ADDR were not defined for > this board thus breaking its build. > > Signed-off-by: Igor Grinberg <grinberg@compulab.co.il> > Cc: Sandeep Paulraj <s-paulraj@ti.com> > Cc: Nishant Kamat <nskamat@ti.com> > Cc: Kshitij Gupta <kshitij@ti.com> > --- > include/configs/omap1610h2.h | 7 +++++++ > include/configs/omap1610inn.h | 7 +++++++ > 2 files changed, 14 insertions(+), 0 deletions(-) > > diff --git a/include/configs/omap1610h2.h b/include/configs/omap1610h2.h > index 2936dcc..7e53ae6 100644 > --- a/include/configs/omap1610h2.h > +++ b/include/configs/omap1610h2.h > @@ -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 */ > > diff --git a/include/configs/omap1610inn.h b/include/configs/omap1610inn.h > index 0b41c46..be569a3 100644 > --- a/include/configs/omap1610inn.h > +++ b/include/configs/omap1610inn.h > @@ -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 */ > -- Regards, Igor. ^ permalink raw reply [flat|nested] 38+ messages in thread
* [U-Boot] [PATCH 1/3] arm: omap: innovator: fix compilation error 2011-05-17 12:40 ` Igor Grinberg @ 2011-05-21 21:40 ` Paulraj, Sandeep 0 siblings, 0 replies; 38+ messages in thread From: Paulraj, Sandeep @ 2011-05-21 21:40 UTC (permalink / raw) To: u-boot >Sandeep, > >Any comments? I'm on vacation. i'll apply it soon. Regards, Sandeep ^ permalink raw reply [flat|nested] 38+ messages in thread
* [U-Boot] [PATCH 2/3] arm: omap: innovator: Prepare for mach-types.h changes 2011-04-29 8:58 ` Detlev Zundel 2011-05-01 10:10 ` [U-Boot] [PATCH 1/3] arm: omap: innovator: fix compilation error Igor Grinberg @ 2011-05-01 10:10 ` Igor Grinberg 2011-05-01 20:28 ` Alessandro Rubini 2011-05-01 10:10 ` [U-Boot] [PATCH 3/3] arm: at91: ether: " Igor Grinberg 2 siblings, 1 reply; 38+ messages in thread From: Igor Grinberg @ 2011-05-01 10:10 UTC (permalink / raw) To: u-boot This board used machine_is_* macros for identifying the arch number. Use compile time defines instead. Signed-off-by: Igor Grinberg <grinberg@compulab.co.il> Cc: Sandeep Paulraj <s-paulraj@ti.com> Cc: Nishant Kamat <nskamat@ti.com> Cc: Kshitij Gupta <kshitij@ti.com> --- 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(-) diff --git a/board/ti/omap1610inn/omap1610innovator.c b/board/ti/omap1610inn/omap1610innovator.c index 44818bb..01b9f53 100644 --- a/board/ti/omap1610inn/omap1610innovator.c +++ b/board/ti/omap1610inn/omap1610innovator.c @@ -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; -- 1.7.3.4 ^ permalink raw reply related [flat|nested] 38+ messages in thread
* [U-Boot] [PATCH 2/3] arm: omap: innovator: Prepare for mach-types.h changes 2011-05-01 10:10 ` [U-Boot] [PATCH 2/3] arm: omap: innovator: Prepare for mach-types.h changes Igor Grinberg @ 2011-05-01 20:28 ` Alessandro Rubini 2011-05-02 7:18 ` Igor Grinberg 0 siblings, 1 reply; 38+ messages in thread From: Alessandro Rubini @ 2011-05-01 20:28 UTC (permalink / raw) To: u-boot 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: > - 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: > 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 ^ permalink raw reply [flat|nested] 38+ messages in thread
* [U-Boot] [PATCH 2/3] arm: omap: innovator: Prepare for mach-types.h changes 2011-05-01 20:28 ` Alessandro Rubini @ 2011-05-02 7:18 ` Igor Grinberg 2011-05-03 10:08 ` [U-Boot] [PATCH v2 " Igor Grinberg 0 siblings, 1 reply; 38+ messages in thread From: Igor Grinberg @ 2011-05-02 7:18 UTC (permalink / raw) To: u-boot On 05/01/11 23:28, Alessandro Rubini wrote: > 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 > For example, this patch: > >> - 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? > The commis says: > >> 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. > 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. > 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... -- Regards, Igor. ^ permalink raw reply [flat|nested] 38+ messages in thread
* [U-Boot] [PATCH v2 2/3] arm: omap: innovator: Prepare for mach-types.h changes 2011-05-02 7:18 ` Igor Grinberg @ 2011-05-03 10:08 ` Igor Grinberg 2011-05-03 12:29 ` Wolfgang Denk 0 siblings, 1 reply; 38+ messages in thread From: Igor Grinberg @ 2011-05-03 10:08 UTC (permalink / raw) To: u-boot 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 <grinberg@compulab.co.il> --- 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(-) diff --git a/board/ti/omap1610inn/omap1610innovator.c b/board/ti/omap1610inn/omap1610innovator.c index 44818bb..a071f63 100644 --- a/board/ti/omap1610inn/omap1610innovator.c +++ b/board/ti/omap1610inn/omap1610innovator.c @@ -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; diff --git a/include/configs/omap1610h2.h b/include/configs/omap1610h2.h index 7e53ae6..0770d9d 100644 --- a/include/configs/omap1610h2.h +++ b/include/configs/omap1610h2.h @@ -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 */ diff --git a/include/configs/omap1610inn.h b/include/configs/omap1610inn.h index be569a3..1859780 100644 --- a/include/configs/omap1610inn.h +++ b/include/configs/omap1610inn.h @@ -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 */ -- 1.7.3.4 ^ permalink raw reply related [flat|nested] 38+ messages in thread
* [U-Boot] [PATCH v2 2/3] arm: omap: innovator: Prepare for mach-types.h changes 2011-05-03 10:08 ` [U-Boot] [PATCH v2 " Igor Grinberg @ 2011-05-03 12:29 ` Wolfgang Denk 2011-05-03 13:00 ` Igor Grinberg 0 siblings, 1 reply; 38+ messages in thread From: Wolfgang Denk @ 2011-05-03 12:29 UTC (permalink / raw) To: u-boot Dear Igor Grinberg, In message <1304417333-30745-1-git-send-email-grinberg@compulab.co.il> you wrote: > 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 <grinberg@compulab.co.il> > --- > v2: remove the ifdeferry by introducing config variable, > Alessandro, what about this one? ... > + 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 for CONFIG_MACH_TYPE to the README? Thanks! Best regards, Wolfgang Denk -- 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. ^ permalink raw reply [flat|nested] 38+ messages in thread
* [U-Boot] [PATCH v2 2/3] arm: omap: innovator: Prepare for mach-types.h changes 2011-05-03 12:29 ` Wolfgang Denk @ 2011-05-03 13:00 ` Igor Grinberg 2011-05-04 7:13 ` [U-Boot] [PATCH v3 " Igor Grinberg 0 siblings, 1 reply; 38+ messages in thread From: Igor Grinberg @ 2011-05-03 13:00 UTC (permalink / raw) To: u-boot Hi Wolfgang, On 05/03/11 15:29, Wolfgang Denk wrote: > Dear Igor Grinberg, > > In message <1304417333-30745-1-git-send-email-grinberg@compulab.co.il> you wrote: >> 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 <grinberg@compulab.co.il> >> --- >> v2: remove the ifdeferry by introducing config variable, >> Alessandro, what about this one? > ... >> + 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. > 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. > And while being there, could you please also add a description for > CONFIG_MACH_TYPE to 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. -- Regards, Igor. ^ permalink raw reply [flat|nested] 38+ messages in thread
* [U-Boot] [PATCH v3 2/3] arm: omap: innovator: Prepare for mach-types.h changes 2011-05-03 13:00 ` Igor Grinberg @ 2011-05-04 7:13 ` Igor Grinberg 0 siblings, 0 replies; 38+ messages in thread From: Igor Grinberg @ 2011-05-04 7:13 UTC (permalink / raw) To: u-boot This board used machine_is_* macros for identifying the arch number. Fix this by using CONFIG_MACH_TYPE configuration variable. Signed-off-by: Igor Grinberg <grinberg@compulab.co.il> --- 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(-) diff --git a/board/ti/omap1610inn/omap1610innovator.c b/board/ti/omap1610inn/omap1610innovator.c index 44818bb..2dbb9e5 100644 --- a/board/ti/omap1610inn/omap1610innovator.c +++ b/board/ti/omap1610inn/omap1610innovator.c @@ -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; diff --git a/include/configs/omap1610h2.h b/include/configs/omap1610h2.h index 7e53ae6..9cda6c9 100644 --- a/include/configs/omap1610h2.h +++ b/include/configs/omap1610h2.h @@ -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 */ diff --git a/include/configs/omap1610inn.h b/include/configs/omap1610inn.h index be569a3..f32975d 100644 --- a/include/configs/omap1610inn.h +++ b/include/configs/omap1610inn.h @@ -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 */ -- 1.7.3.4 ^ permalink raw reply related [flat|nested] 38+ messages in thread
* [U-Boot] [PATCH 3/3] arm: at91: ether: Prepare for mach-types.h changes 2011-04-29 8:58 ` Detlev Zundel 2011-05-01 10:10 ` [U-Boot] [PATCH 1/3] arm: omap: innovator: fix compilation error Igor Grinberg 2011-05-01 10:10 ` [U-Boot] [PATCH 2/3] arm: omap: innovator: Prepare for mach-types.h changes Igor Grinberg @ 2011-05-01 10:10 ` Igor Grinberg 2011-05-01 19:38 ` Reinhard Meyer 2 siblings, 1 reply; 38+ messages in thread From: Igor Grinberg @ 2011-05-01 10:10 UTC (permalink / raw) To: u-boot at91 ethernet module used machine_is_cbs337() macro for board specific Linux compatibility issue. Use compile time defines instead. Signed-off-by: Igor Grinberg <grinberg@compulab.co.il> --- arch/arm/cpu/arm920t/at91rm9200/ether.c | 18 +++++++++--------- 1 files changed, 9 insertions(+), 9 deletions(-) diff --git a/arch/arm/cpu/arm920t/at91rm9200/ether.c b/arch/arm/cpu/arm920t/at91rm9200/ether.c index e1cdeba..4aeb883 100644 --- a/arch/arm/cpu/arm920t/at91rm9200/ether.c +++ b/arch/arm/cpu/arm920t/at91rm9200/ether.c @@ -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); -- 1.7.3.4 ^ permalink raw reply related [flat|nested] 38+ messages in thread
* [U-Boot] [PATCH 3/3] arm: at91: ether: Prepare for mach-types.h changes 2011-05-01 10:10 ` [U-Boot] [PATCH 3/3] arm: at91: ether: " Igor Grinberg @ 2011-05-01 19:38 ` Reinhard Meyer 2011-05-02 7:29 ` Igor Grinberg 0 siblings, 1 reply; 38+ messages in thread From: Reinhard Meyer @ 2011-05-01 19:38 UTC (permalink / raw) To: u-boot Dear Igor Grinberg, > at91 ethernet module used machine_is_cbs337() macro for board specific > Linux compatibility issue. > Use compile time defines instead. > > Signed-off-by: Igor Grinberg<grinberg@compulab.co.il> > --- > arch/arm/cpu/arm920t/at91rm9200/ether.c | 18 +++++++++--------- > 1 files changed, 9 insertions(+), 9 deletions(-) > > diff --git a/arch/arm/cpu/arm920t/at91rm9200/ether.c b/arch/arm/cpu/arm920t/at91rm9200/ether.c > index e1cdeba..4aeb883 100644 > --- a/arch/arm/cpu/arm920t/at91rm9200/ether.c > +++ b/arch/arm/cpu/arm920t/at91rm9200/ether.c > @@ -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); 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 ^ permalink raw reply [flat|nested] 38+ messages in thread
* [U-Boot] [PATCH 3/3] arm: at91: ether: Prepare for mach-types.h changes 2011-05-01 19:38 ` Reinhard Meyer @ 2011-05-02 7:29 ` Igor Grinberg 2011-05-02 10:09 ` Detlev Zundel 0 siblings, 1 reply; 38+ messages in thread From: Igor Grinberg @ 2011-05-02 7:29 UTC (permalink / raw) To: u-boot On 05/01/11 22:38, Reinhard Meyer wrote: > Dear Igor Grinberg, > >> at91 ethernet module used machine_is_cbs337() macro for board specific >> Linux compatibility issue. >> Use compile time defines instead. >> >> Signed-off-by: Igor Grinberg<grinberg@compulab.co.il> >> --- >> arch/arm/cpu/arm920t/at91rm9200/ether.c | 18 +++++++++--------- >> 1 files changed, 9 insertions(+), 9 deletions(-) >> >> diff --git a/arch/arm/cpu/arm920t/at91rm9200/ether.c b/arch/arm/cpu/arm920t/at91rm9200/ether.c >> index e1cdeba..4aeb883 100644 >> --- a/arch/arm/cpu/arm920t/at91rm9200/ether.c >> +++ b/arch/arm/cpu/arm920t/at91rm9200/ether.c >> @@ -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); > > 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? 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. > > 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... > > Any opinions by Ben or Wolfgang on this? > -- Regards, Igor. ^ permalink raw reply [flat|nested] 38+ messages in thread
* [U-Boot] [PATCH 3/3] arm: at91: ether: Prepare for mach-types.h changes 2011-05-02 7:29 ` Igor Grinberg @ 2011-05-02 10:09 ` Detlev Zundel 2011-05-02 12:49 ` [U-Boot] [PATCH v2 " Igor Grinberg 0 siblings, 1 reply; 38+ messages in thread From: Detlev Zundel @ 2011-05-02 10:09 UTC (permalink / raw) To: u-boot Hi Igor, > On 05/01/11 22:38, Reinhard Meyer wrote: > >> Dear Igor Grinberg, >> >>> at91 ethernet module used machine_is_cbs337() macro for board specific >>> Linux compatibility issue. >>> Use compile time defines instead. >>> >>> Signed-off-by: Igor Grinberg<grinberg@compulab.co.il> >>> --- >>> arch/arm/cpu/arm920t/at91rm9200/ether.c | 18 +++++++++--------- >>> 1 files changed, 9 insertions(+), 9 deletions(-) >>> >>> diff --git a/arch/arm/cpu/arm920t/at91rm9200/ether.c b/arch/arm/cpu/arm920t/at91rm9200/ether.c >>> index e1cdeba..4aeb883 100644 >>> --- a/arch/arm/cpu/arm920t/at91rm9200/ether.c >>> +++ b/arch/arm/cpu/arm920t/at91rm9200/ether.c >>> @@ -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); >> >> 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? > > 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 -- 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.de ^ permalink raw reply [flat|nested] 38+ messages in thread
* [U-Boot] [PATCH v2 3/3] arm: at91: ether: Prepare for mach-types.h changes 2011-05-02 10:09 ` Detlev Zundel @ 2011-05-02 12:49 ` Igor Grinberg 2011-05-16 13:31 ` Igor Grinberg 0 siblings, 1 reply; 38+ messages in thread From: Igor Grinberg @ 2011-05-02 12:49 UTC (permalink / raw) To: u-boot 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 <grinberg@compulab.co.il> --- arch/arm/cpu/arm920t/at91rm9200/ether.c | 18 +++--------------- 1 files changed, 3 insertions(+), 15 deletions(-) diff --git a/arch/arm/cpu/arm920t/at91rm9200/ether.c b/arch/arm/cpu/arm920t/at91rm9200/ether.c index e1cdeba..2015e13 100644 --- a/arch/arm/cpu/arm920t/at91rm9200/ether.c +++ b/arch/arm/cpu/arm920t/at91rm9200/ether.c @@ -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); -- 1.7.3.4 ^ permalink raw reply related [flat|nested] 38+ messages in thread
* [U-Boot] [PATCH v2 3/3] arm: at91: ether: Prepare for mach-types.h changes 2011-05-02 12:49 ` [U-Boot] [PATCH v2 " Igor Grinberg @ 2011-05-16 13:31 ` Igor Grinberg 0 siblings, 0 replies; 38+ messages in thread From: Igor Grinberg @ 2011-05-16 13:31 UTC (permalink / raw) To: u-boot ping! It has been two weeks... On 05/02/11 15:49, Igor Grinberg wrote: > 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 <grinberg@compulab.co.il> > --- > arch/arm/cpu/arm920t/at91rm9200/ether.c | 18 +++--------------- > 1 files changed, 3 insertions(+), 15 deletions(-) > > diff --git a/arch/arm/cpu/arm920t/at91rm9200/ether.c b/arch/arm/cpu/arm920t/at91rm9200/ether.c > index e1cdeba..2015e13 100644 > --- a/arch/arm/cpu/arm920t/at91rm9200/ether.c > +++ b/arch/arm/cpu/arm920t/at91rm9200/ether.c > @@ -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); -- Regards, Igor. ^ permalink raw reply [flat|nested] 38+ messages in thread
* [U-Boot] Update and Cut down mach types 2011-04-26 21:38 ` Reinhard Meyer 2011-04-27 10:19 ` Michael Schwingen @ 2011-04-27 11:44 ` Detlev Zundel 1 sibling, 0 replies; 38+ messages in thread From: Detlev Zundel @ 2011-04-27 11:44 UTC (permalink / raw) To: u-boot Hi Reinhard, [...] > 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_* ... 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 -- "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 ^ permalink raw reply [flat|nested] 38+ messages in thread
end of thread, other threads:[~2011-05-21 21:40 UTC | newest] Thread overview: 38+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-04-19 12:42 [U-Boot] Update and Cut down mach types Paulraj, Sandeep 2011-04-19 13:39 ` Matthias Weißer 2011-04-19 13:45 ` Paulraj, Sandeep 2011-04-20 8:44 ` Albert ARIBAUD 2011-04-19 14:21 ` Wolfgang Denk 2011-04-19 18:42 ` Matthias Weisser 2011-04-19 18:44 ` Michael Schwingen 2011-04-20 8:15 ` Detlev Zundel 2011-04-20 8:58 ` Igor Grinberg 2011-04-20 17:15 ` Michael Schwingen 2011-04-20 17:49 ` Albert ARIBAUD 2011-04-20 19:26 ` Michael Schwingen 2011-04-21 11:39 ` Albert ARIBAUD 2011-04-26 18:14 ` Michael Schwingen 2011-04-26 19:40 ` Wolfgang Denk 2011-04-26 20:38 ` Albert ARIBAUD 2011-04-26 21:32 ` Wolfgang Denk 2011-04-26 21:38 ` Reinhard Meyer 2011-04-27 10:19 ` Michael Schwingen 2011-04-28 6:20 ` Igor Grinberg 2011-04-29 8:58 ` Detlev Zundel 2011-05-01 10:10 ` [U-Boot] [PATCH 1/3] arm: omap: innovator: fix compilation error Igor Grinberg 2011-05-17 12:40 ` Igor Grinberg 2011-05-21 21:40 ` Paulraj, Sandeep 2011-05-01 10:10 ` [U-Boot] [PATCH 2/3] arm: omap: innovator: Prepare for mach-types.h changes Igor Grinberg 2011-05-01 20:28 ` Alessandro Rubini 2011-05-02 7:18 ` Igor Grinberg 2011-05-03 10:08 ` [U-Boot] [PATCH v2 " Igor Grinberg 2011-05-03 12:29 ` Wolfgang Denk 2011-05-03 13:00 ` Igor Grinberg 2011-05-04 7:13 ` [U-Boot] [PATCH v3 " Igor Grinberg 2011-05-01 10:10 ` [U-Boot] [PATCH 3/3] arm: at91: ether: " Igor Grinberg 2011-05-01 19:38 ` Reinhard Meyer 2011-05-02 7:29 ` Igor Grinberg 2011-05-02 10:09 ` Detlev Zundel 2011-05-02 12:49 ` [U-Boot] [PATCH v2 " Igor Grinberg 2011-05-16 13:31 ` Igor Grinberg 2011-04-27 11:44 ` [U-Boot] Update and Cut down mach types Detlev Zundel
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox