Re: [U-Boot] [RFC PATCH 0/19] Create generic board init and move ARM and x86 to it
Dear Simon Glass, In message CAPnjgZ2jS8kzCt06LbYnXSyEKm6ck8apm=p1yqqdtnz0+q9...@mail.gmail.com you wrote: Well let's see how we go with the incremental approach - hopefully we can get the same result with less pain and risk, and not too much work. Did you miss my proposal not to change the existing coe for all boards, but to provide anew implementation and convert boards one by one? What do you think about such an approach? 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: w...@denx.de The X11 source code style is ATROCIOUS and should not be used as a model. - Doug Gwyn ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [RFC PATCH 0/19] Create generic board init and move ARM and x86 to it
Hi Wolfgang, On 03/01/12 19:12, Wolfgang Denk wrote: Dear Simon Glass, In message CAPnjgZ2jS8kzCt06LbYnXSyEKm6ck8apm=p1yqqdtnz0+q9...@mail.gmail.com you wrote: Well let's see how we go with the incremental approach - hopefully we can get the same result with less pain and risk, and not too much work. Did you miss my proposal not to change the existing coe for all boards, but to provide anew implementation and convert boards one by one? What do you think about such an approach? My idea was to migrate each arch to the 'pure processing loops' implementation I have just posted for x86. We can then factor out the processing loop code into common code leaving just the init function arrays in arch/foo/lib/board.c We can then look at how to deal with the disparate init sequences - I'm going to have a look at my new 'initcall' proposal to see if that is viable - If so, we auto generate init_sequence.c Regards, Graeme ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [RFC PATCH 0/19] Create generic board init and move ARM and x86 to it
Hi Wolfgang, On Tue, Jan 3, 2012 at 12:12 AM, Wolfgang Denk w...@denx.de wrote: Dear Simon Glass, In message CAPnjgZ2jS8kzCt06LbYnXSyEKm6ck8apm=p1yqqdtnz0+q9...@mail.gmail.com you wrote: Well let's see how we go with the incremental approach - hopefully we can get the same result with less pain and risk, and not too much work. Did you miss my proposal not to change the existing coe for all boards, but to provide anew implementation and convert boards one by one? What do you think about such an approach? Sorry I must have misread this as 'architecture' instead of 'board'. I suppose this would work since there is a simple CONFIG switch to move over. The concern with doing this for a whole architecture was the board breakages, and this idea would avoid that. I wonder what would be the incentive for boards to move over? Perhaps eventually it could be the default, or we could discourage patches to the old code. I have brought in PPC to board_f.c and board_r.c but not dealt with relocation. Once I have done that, gone through the new x86 series again and MAKEALL doesn't scream too loudly I'll do another RFC along these lines. If Graham can move his initcall stuff along too then we can also get rid of the #ifdefs, which it IMO the main ugliness with my RFC. Regards, Simon 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: w...@denx.de The X11 source code style is ATROCIOUS and should not be used as a model. - Doug Gwyn ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [RFC PATCH 0/19] Create generic board init and move ARM and x86 to it
Dear Simon Glass, In message CAPnjgZ3592m_m=tmafptwzo+_o94veykvrffdgpp599igef...@mail.gmail.com you wrote: I have brought in PPC to board_f.c and board_r.c but not dealt with relocation. Once I have done that, gone through the new x86 series again and MAKEALL doesn't scream too loudly I'll do another RFC along these lines. I'm seriously concerned about you insisting to include relocation here. This has nothign to do with it. Leave that out. 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: w...@denx.de When the only tool you have is a hammer, you tend to treat everything as if it were a nail.- Abraham Maslow ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [RFC PATCH 0/19] Create generic board init and move ARM and x86 to it
Hi Wolfgang, On Tue, Jan 3, 2012 at 2:35 PM, Wolfgang Denk w...@denx.de wrote: Dear Simon Glass, In message CAPnjgZ3592m_m=tmafptwzo+_o94veykvrffdgpp599igef...@mail.gmail.com you wrote: I have brought in PPC to board_f.c and board_r.c but not dealt with relocation. Once I have done that, gone through the new x86 series again and MAKEALL doesn't scream too loudly I'll do another RFC along these lines. I'm seriously concerned about you insisting to include relocation here. This has nothign to do with it. Leave that out. I'm not insisting, only that I have already done a series for generic relocation so figured this new series would come second. I did signal this right at the beginning when suggesting the approach: http://lists.denx.de/pipermail/u-boot/2011-November/109987.html Anyway, leaving out relocation is easier so I will rebase this series on top of master instead. Regards, Simon 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: w...@denx.de When the only tool you have is a hammer, you tend to treat everything as if it were a nail. - Abraham Maslow ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [RFC PATCH 0/19] Create generic board init and move ARM and x86 to it
Hi Simon, In case you haven't noticed (I did not Cc you or Wolfgang, sorry 'bout that) I've posted the cleaned up version of my x86 init refactoring... On 02/01/12 10:48, Simon Glass wrote: Hi Graeme, On Sat, Dec 31, 2011 at 3:52 AM, Graeme Russ graeme.r...@gmail.com wrote: Hi Simon, On 31/12/11 13:02, Simon Glass wrote: Hi Graeme, On Fri, Dec 30, 2011 at 7:49 AM, Graeme Russ graeme.r...@gmail.com wrote: Hi Simon, [snip] Ah yes, looking a bit closer I see this now in patches 5 6. However, I think if you look at my patches 9, 10, 12 and 13 I'm doing essentially the same thing for x86. The difference is, I'm moving live code, rather than creating dead code then switching over. I personally do not think your approach is very safe - If there is a breakage, it is spread over multiple patches - With the 'move code around' approach, all breakages are confined to a single patch See my other email on this topic though - I feel that one test/fix cycle is better than testing every board for every little patch over an extended period. The problem is not one of how sparsely the test/fix cycles are spread over time, it is one of spreading the breakage over multiple patches - If you are replacing functionality then add the new functionality, add the hooks to use it and delete the old in a single patch. That way, if you change breaks something, the revert is trivial. If you multi-patch approach breaks something, the revert becomes more difficult. I don't think it is a huge job to do this for PowerPC also, and that seems to be the most feature-full architecture. I agree - The init architecture is the same, but the sequence is bigger (and not in the same order) Yes that's what I'm finding. Also it does depend on expectations. I would hope that moving an architecture over would be a fairly small task: - getting generic relocation working - adding functions for anything that is missing from board init code - removing things which don't work on the architecture? - worrying about differences in ordering between functions I see it as: 1) Rework the arch init sequence to be a pure list of 'int foo(void)' functions, adding helpers and wrappers where necessary 2) Switch over to use the generic init processing loop 3) Factor out common helpers and wrappers across all arches 4) Factor out common functionality (e.g. relocation) 5) After all arches are switched, remove wrappers by changing the function signature of the wrapped function 6) Move helpers into relevant driver/common source file I think 1 2 need to be done for at least ARM, x86 and PPC before starting 3 4 and 1-4 needs to be done for all arches before doing 5 6 OK yes we could do this. But I think the end result is different. I am keen to create a single common/board_f.c implementation for pre-reloc init and common/board_r.c for post-reloc init. I believe where your approach ends up with with separate arch/xxx/board.c files still, with their own initcall list, and a number of private Correct, but as a first step, the processing loop code is made common, leaving only the init arrays per-arch functions. Now this is better than where we are now, but my original complaint was that I want to be able to add features to a single place and have them work across all architectures. With your end-result I still need to figure out where to put the new feature in each initcall list and create 10 patches, one for each arch, to plumb it in. Agree, and my ultimate end goal (as per my previous initcall patch series) is to remove the per-arch init list as well... IMO the ordering differences between architectures are mostly unnecessary - an artifact of forking the code - and we should aim to minimise these differences. Putting everything in one file helps to clarify exactly what the differences are between archs, and discussions can take place as to why and whether this differences can be removed. If we accept that PPC is the one true way (in terms of U-Boot code base), then the job is to make other arch's code more like PPC. This is best done in one place I think. I see two issues with your approach: 1) You want to take a 'big bang' approach and change every arch over in one set of patches. I admire your vision, but question you realism ;) I took one look across all the init sequences and fled in horror. Such a big-bang approach is sure to cause some _major_ board breakage 2) As Wolfgang has eluded to, a unified set of init arrays will be more #ifdef than code After a bit of thought, I worry also that your approach will create an extended period of time where U-Boot is unstable, as patch are patch How so - Take a look at my x86 patch series. It only impacts x86 and it can be easily tested. Other arches are going to be even easier as the most delicate part of my series was getting global data to behave properly. For the other arches, it will be a simple
Re: [U-Boot] [RFC PATCH 0/19] Create generic board init and move ARM and x86 to it
Dear Graeme Russ, In message 4f019473.8000...@gmail.com you wrote: The problem is not one of how sparsely the test/fix cycles are spread over time, it is one of spreading the breakage over multiple patches - If you are replacing functionality then add the new functionality, add the hooks to use it and delete the old in a single patch. That way, if you change breaks something, the revert is trivial. If you multi-patch approach breaks something, the revert becomes more difficult. True. Especially as it's likely that different patches will break different boards, so there will not even be a chance so revert. Well I don't propose to create things which are not bisect-able. I But you have - You create new functionality in one patch, add a number of patches, then finally use that new functionality in a later patch. This is what I fear as well. 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: w...@denx.de Disc space - the final frontier! ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [RFC PATCH 0/19] Create generic board init and move ARM and x86 to it
Hi Graham, On Mon, Jan 2, 2012 at 3:26 AM, Graeme Russ graeme.r...@gmail.com wrote: Hi Simon, In case you haven't noticed (I did not Cc you or Wolfgang, sorry 'bout that) I've posted the cleaned up version of my x86 init refactoring... Yes I see it - thats great thanks. I will go through it and compare. [big snip] functions. Now this is better than where we are now, but my original complaint was that I want to be able to add features to a single place and have them work across all architectures. With your end-result I still need to figure out where to put the new feature in each initcall list and create 10 patches, one for each arch, to plumb it in. Agree, and my ultimate end goal (as per my previous initcall patch series) is to remove the per-arch init list as well... Well since it seems we are going for the same goal, and you are actively submitting patches towards it, I'm very happy. I mostly agree with your points, so will not respond bit by bit. The easiest route to the goal is the right one, so let's run with what you propose and see how we go. Thanks for taking it up. [another big snip] I had a go at bringing PPC into the generic board patches. Apart from the cloud of #ifdefs as expected it is not too bad. And those can be dealt with by initcalls. Am hoping we can get there soon! Regards, Simon Regards, Graeme ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [RFC PATCH 0/19] Create generic board init and move ARM and x86 to it
Hi Wolfgang, On Mon, Jan 2, 2012 at 6:46 AM, Wolfgang Denk w...@denx.de wrote: Dear Graeme Russ, In message 4f019473.8000...@gmail.com you wrote: The problem is not one of how sparsely the test/fix cycles are spread over time, it is one of spreading the breakage over multiple patches - If you are replacing functionality then add the new functionality, add the hooks to use it and delete the old in a single patch. That way, if you change breaks something, the revert is trivial. If you multi-patch approach breaks something, the revert becomes more difficult. True. Especially as it's likely that different patches will break different boards, so there will not even be a chance so revert. The only option would be to have a way to use the legacy code for particular boards (say a CONFIG option) until they are fixed. Not entirely satisfactory. Of course this could happen with either approach, and reverts become impossible when further patches are layered on top. Well I don't propose to create things which are not bisect-able. I But you have - You create new functionality in one patch, add a number of patches, then finally use that new functionality in a later patch. This is what I fear as well. Well let's see how we go with the incremental approach - hopefully we can get the same result with less pain and risk, and not too much work. Regards, Simon 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: w...@denx.de Disc space - the final frontier! ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [RFC PATCH 0/19] Create generic board init and move ARM and x86 to it
Hi Wolfgang, On Sat, Dec 31, 2011 at 5:54 PM, Wolfgang Denk w...@denx.de wrote: Dear Simon Glass, In message CAPnjgZ1Zim6=u3rlcnake-0bw3so_hry+u67jkpknoni8g7...@mail.gmail.com you wrote: Also it does depend on expectations. I would hope that moving an architecture over would be a fairly small task: - getting generic relocation working I don;t see why this would be a direct prerequisite here. We want to have that, no boubt about that. But it appears to be an unrelated change to me. I don't think it's essential, but it is desirable, which I why it is on the list. - adding functions for anything that is missing from board init code anything that is missing from board init code ? By that I mean that if the architecture has its own little things which aren't supported by generic board init, then these would need to be worked into the generic board init somehow, perhaps initially with a new function in the initcall list. - removing things which don't work on the architecture? That would probably reander systems broken that need these things? Sorry for being so vague. What I mean is that if an arch does not support a feature, then we don't want to call the init functions for it on that architecture. Sometimes this is handled automatically - e.g. if an arch does not support CONFIG_POST then the generic board init code will skip this anyway. But there may be other cases where features available in the generic board init cannot be supported in all archs. - worrying about differences in ordering between functions This is indeed the biggest issue. Keep in mind that my original ida of providing a function call table was to allow to define this table in a board specific way (i. e. in the board config file). [Not that this idea found many friends...] With this series I have moved over to purely having a function call table, with no actual init code in board_init_f() and board_init_r(). Having done this, I have found that the list of functions is very long, So we probably don't want boards to duplicate this list and #ifdefs may be the lesser of two evils. If we can move to Graham' initcall idea (basically using the linker to create the list of initcalls for pre- and post-reloc) then it becomes possible to boards to insert things in the init sequence without #ifdefs in commong/board.f/r.c. For now I have tried to simply move the code into separate functions, since this makes it easier to compare against the original code in the arch/xxx/lib/board.c files, to make sure nothing is left out and the ordering is OK. pull out common init code like init_baudrate() and hang() and change the function signatures of the functions that require wrappers and move some #ifdef's into more appropriate locations - One example in board.c: Well it seems like a lot of work to refactor each arch/xxx/board.c file into functions with a function pointer list, then later remove this code function by function. Would that really be a clever approach? I'm not convinced it is the best idea. A trivial change of just removing duplicated code from all the archs and inserting it into generic board should be safe enough. But is that really a common case? I have found lots of subtle differences that need to be worked out, and there is the ordering to worry about. Every time there is a patch we need to re-test all boards. It might be easier to just create a generic board implementation one, and then test it once on each board, rather than testing on each board with every patch. Of course every time you patch generic board you might introduce a breakage in an existing (previously working) board, but that is the normal situation with development so I don't see a problem there. Perhaps the biggest problem is the difficulty of testing, and if we can avoid months of little patches (each one potentially breaking lots of boards at run-time) then I would prefer that. My feeling is that with a generic board, it should hopefully be a fairly small amount of work for someone familiar with an architecture to find the bugs and patch the generic code to suit their architecture. It is something that needs to be done once, not every time there is a new patch removing (almost) common code. It is definitely not that easy. You fix one thing here, and break another board there. Ideally you would have to re-test any change on _all_ boards. Well I suppose that's what I'm saying. Doing this once, accepting that some boards will thereby be broken, and then accepting patches to fix them feels easier than trying to keep everything working through a long series of little changes. From previous discussions, if something is optional then the switch will never happen. The code you are talking about is sometimes identical, sometimes slightly different. In some cases the order is different. I see many ways of introducing breakages. I do agree that And I bet most of them _will_ introduce breakages. doing
Re: [U-Boot] [RFC PATCH 0/19] Create generic board init and move ARM and x86 to it
Hi Graeme, On Sat, Dec 31, 2011 at 3:52 AM, Graeme Russ graeme.r...@gmail.com wrote: Hi Simon, On 31/12/11 13:02, Simon Glass wrote: Hi Graeme, On Fri, Dec 30, 2011 at 7:49 AM, Graeme Russ graeme.r...@gmail.com wrote: Hi Simon, [snip] However, I think this approach is not the right one - and I think the CFI driver backs me up. Your plan is to create generic code which you want ALL arches to cross over to, but you only look to migrate two initially and migrate the rest 'later'. This is similar to what happened with the CFI driver, and there are still boards with custom flash.c files which are completely redundant. But, creating a single patch-set to migrate everyone in one go is going to be too massive a job to do in one go, and too prone to introducing breakage. Yes to some extent. However, my patch basically splits the two board_init_x() functions into parts, and puts them in a function table. Ah yes, looking a bit closer I see this now in patches 5 6. However, I think if you look at my patches 9, 10, 12 and 13 I'm doing essentially the same thing for x86. The difference is, I'm moving live code, rather than creating dead code then switching over. I personally do not think your approach is very safe - If there is a breakage, it is spread over multiple patches - With the 'move code around' approach, all breakages are confined to a single patch See my other email on this topic though - I feel that one test/fix cycle is better than testing every board for every little patch over an extended period. I don't think it is a huge job to do this for PowerPC also, and that seems to be the most feature-full architecture. I agree - The init architecture is the same, but the sequence is bigger (and not in the same order) Yes that's what I'm finding. Also it does depend on expectations. I would hope that moving an architecture over would be a fairly small task: - getting generic relocation working - adding functions for anything that is missing from board init code - removing things which don't work on the architecture? - worrying about differences in ordering between functions I see it as: 1) Rework the arch init sequence to be a pure list of 'int foo(void)' functions, adding helpers and wrappers where necessary 2) Switch over to use the generic init processing loop 3) Factor out common helpers and wrappers across all arches 4) Factor out common functionality (e.g. relocation) 5) After all arches are switched, remove wrappers by changing the function signature of the wrapped function 6) Move helpers into relevant driver/common source file I think 1 2 need to be done for at least ARM, x86 and PPC before starting 3 4 and 1-4 needs to be done for all arches before doing 5 6 OK yes we could do this. But I think the end result is different. I am keen to create a single common/board_f.c implementation for pre-reloc init and common/board_r.c for post-reloc init. I believe where your approach ends up with with separate arch/xxx/board.c files still, with their own initcall list, and a number of private functions. Now this is better than where we are now, but my original complaint was that I want to be able to add features to a single place and have them work across all architectures. With your end-result I still need to figure out where to put the new feature in each initcall list and create 10 patches, one for each arch, to plumb it in. IMO the ordering differences between architectures are mostly unnecessary - an artifact of forking the code - and we should aim to minimise these differences. Putting everything in one file helps to clarify exactly what the differences are between archs, and discussions can take place as to why and whether this differences can be removed. If we accept that PPC is the one true way (in terms of U-Boot code base), then the job is to make other arch's code more like PPC. This is best done in one place I think. After a bit of thought, I worry also that your approach will create an extended period of time where U-Boot is unstable, as patch are patch is added to refactor the code. Granted, many of these patches are small and the risk is small. But why bother? At the end we still have 10 different board files, lots of differences and no one willing to change anything because it requires an entire test cycle again. Maybe I am being too ambitious here, but since everything started with PPC's board.c, I don't see why we shouldn't head back in that direction and just create a single file again. It does create a big test problem, but only once :-) Anyway if we do go with what you propose I still think it is better than what we have. [snip] If we work each individual arch to use a generic init sequence (like the proposed x86 code) then the init processing naturally falls out as common code and the patch to realise this is trivial. From there, we can start to pull out common init code like init_baudrate() and
Re: [U-Boot] [RFC PATCH 0/19] Create generic board init and move ARM and x86 to it
Dear Simon Glass, In message CAPnjgZ1eutEboyvEtYcxcZcv=1z_3tkq_45wbrgndpsnxrt...@mail.gmail.com you wrote: - getting generic relocation working I don;t see why this would be a direct prerequisite here. =A0We want to have that, no boubt about that. =A0But it appears to be an unrelated change to me. I don't think it's essential, but it is desirable, which I why it is on the list. I recommend to put this on a different list. Please keep changes orthogonal and restict this discussion and patch series to things dealing with the normalization of the board init code. This is complicated enough, and it does not good to include additional, unrelated tasks. - adding functions for anything that is missing from board init code anything that is missing from board init code ? By that I mean that if the architecture has its own little things which aren't supported by generic board init, then these would need to be worked into the generic board init somehow, perhaps initially with a new function in the initcall list. Such an approach will most probable result in a mess of code that is even more dificult to maintain than the current split we have now. - removing things which don't work on the architecture? That would probably reander systems broken that need these things? Sorry for being so vague. What I mean is that if an arch does not support a feature, then we don't want to call the init functions for it on that architecture. Sometimes this is handled automatically - e.g. if an arch does not support CONFIG_POST then the generic board init code will skip this anyway. But there may be other cases where features available in the generic board init cannot be supported in all archs. This is the same problem as above, right? Each architecture, and actually each board may or may not require certain initializations, and actually may require them to happen in a certain order (which may be different from other boards). Keep in mind that my original ida of providing a function call table was to allow to define this table in a board specific way (i. e. in the board config file). [Not that this idea found many friends...] With this series I have moved over to purely having a function call table, with no actual init code in board_init_f() and board_init_r(). This was the original plan. Having done this, I have found that the list of functions is very long, So we probably don't want boards to duplicate this list and #ifdefs may be the lesser of two evils. If we can move to Graham' I don't really see this as an practical approach. Did you look at the PPC code? init_sequence[] has a total of 29 entries, decorated with 21 conditionals. If we merge in ARM and x86, we will have a mess nobody is able to understand. For now I have tried to simply move the code into separate functions, since this makes it easier to compare against the original code in the arch/xxx/lib/board.c files, to make sure nothing is left out and the ordering is OK. Agreed. I'm not convinced it is the best idea. A trivial change of just removing duplicated code from all the archs and inserting it into generic board should be safe enough. But is that really a common case? I have found lots of subtle differences that need to be worked out, and there is the ordering to worry about. Every time there is a patch we need to re-test all boards. It might be easier to just create a generic board implementation one, and then test it once on each board, rather than testing on each board with every patch. Of course every time you patch generic board you might introduce a breakage in an existing (previously working) board, but that is the normal situation with development so I don't see a problem there. Perhaps the biggest problem is the difficulty of testing, and if we can avoid months of little patches (each one potentially breaking lots of boards at run-time) then I would prefer that. I am not sure that the whole approach makes sense. The resulkting mess of #ifdef's is not there because people didn't care, but because of very specific requirements of specific hardware, that - at least given the tools of the respective point of time - could not be solved more efficiently. I see two options here: - We can take the existing code and try to unify it, and make all boards (including a large number of EOLed and unmaintained ones) to that new code base. This will be a lot of work, and the resulting code will probably messy again. - We could as well try to come up with a new, cleaned up implementa- tion that maybe doesn't fit all, but only 80 or 90% of the existing boards. We could then (1) convert existing code step by step to the new implementation and (2) accept new submissions only when using the new code. There may be a number of boards where conversion is non trivial, or where nobody is willing or able to perform the changes and / or the testing. My gut
Re: [U-Boot] [RFC PATCH 0/19] Create generic board init and move ARM and x86 to it
Hi Simon, On 31/12/11 13:02, Simon Glass wrote: Hi Graeme, On Fri, Dec 30, 2011 at 7:49 AM, Graeme Russ graeme.r...@gmail.com wrote: Hi Simon, [snip] However, I think this approach is not the right one - and I think the CFI driver backs me up. Your plan is to create generic code which you want ALL arches to cross over to, but you only look to migrate two initially and migrate the rest 'later'. This is similar to what happened with the CFI driver, and there are still boards with custom flash.c files which are completely redundant. But, creating a single patch-set to migrate everyone in one go is going to be too massive a job to do in one go, and too prone to introducing breakage. Yes to some extent. However, my patch basically splits the two board_init_x() functions into parts, and puts them in a function table. Ah yes, looking a bit closer I see this now in patches 5 6. However, I think if you look at my patches 9, 10, 12 and 13 I'm doing essentially the same thing for x86. The difference is, I'm moving live code, rather than creating dead code then switching over. I personally do not think your approach is very safe - If there is a breakage, it is spread over multiple patches - With the 'move code around' approach, all breakages are confined to a single patch I don't think it is a huge job to do this for PowerPC also, and that seems to be the most feature-full architecture. I agree - The init architecture is the same, but the sequence is bigger (and not in the same order) Also it does depend on expectations. I would hope that moving an architecture over would be a fairly small task: - getting generic relocation working - adding functions for anything that is missing from board init code - removing things which don't work on the architecture? - worrying about differences in ordering between functions I see it as: 1) Rework the arch init sequence to be a pure list of 'int foo(void)' functions, adding helpers and wrappers where necessary 2) Switch over to use the generic init processing loop 3) Factor out common helpers and wrappers across all arches 4) Factor out common functionality (e.g. relocation) 5) After all arches are switched, remove wrappers by changing the function signature of the wrapped function 6) Move helpers into relevant driver/common source file I think 1 2 need to be done for at least ARM, x86 and PPC before starting 3 4 and 1-4 needs to be done for all arches before doing 5 6 [snip] If we work each individual arch to use a generic init sequence (like the proposed x86 code) then the init processing naturally falls out as common code and the patch to realise this is trivial. From there, we can start to pull out common init code like init_baudrate() and hang() and change the function signatures of the functions that require wrappers and move some #ifdef's into more appropriate locations - One example in board.c: Well it seems like a lot of work to refactor each arch/xxx/board.c file into functions with a function pointer list, then later remove this code function by function. Yes, it is more work, but it is intrinsically safer (and git bisect-able) My feeling is that with a generic board, it should hopefully be a fairly small amount of work for someone familiar with an architecture to find the bugs and patch the generic code to suit their You seem to admit that your approach can introduce bugs architecture. It is something that needs to be done once, not every time there is a new patch removing (almost) common code. The safer approach is to tackle each arch independently - Convert each board.c to something similar to what I have shown with x86 and test those arch-specific changes to make sure nothing has been broken. Then it is a trivial matter of checking that there are no incompatibilities between the arch and the common case (and adjusting as required) and switching over to the common case [snip] The other big benefit is that you only touch one architecture at a time up until you 'pull the switch'. And when you do pull the switch, you should be factoring out identical code so the chances of breaking something should be vastly reduced. Take a look at the history of ARM relocation for example - that was constrained to one arch but still the amount of breakage was massive. From previous discussions, if something is optional then the switch will never happen. The code you are talking about is sometimes identical, sometimes slightly different. In some cases the order is different. I see many ways of introducing breakages. I do agree that The init sequence order is dictated arch-specifically through the init arrays which will (for the time being) remain in the arch specific board.c. But that is all arch/foo/lib/board.c will contain - init sequence arrays, no 'code' doing one architecture at a time is best - with the proviso that we need to pick archs that have the most features (so ARM and
Re: [U-Boot] [RFC PATCH 0/19] Create generic board init and move ARM and x86 to it
Dear Simon Glass, In message CAPnjgZ1Zim6=u3rlcnake-0bw3so_hry+u67jkpknoni8g7...@mail.gmail.com you wrote: Also it does depend on expectations. I would hope that moving an architecture over would be a fairly small task: - getting generic relocation working I don;t see why this would be a direct prerequisite here. We want to have that, no boubt about that. But it appears to be an unrelated change to me. - adding functions for anything that is missing from board init code anything that is missing from board init code ? - removing things which don't work on the architecture? That would probably reander systems broken that need these things? - worrying about differences in ordering between functions This is indeed the biggest issue. Keep in mind that my original ida of providing a function call table was to allow to define this table in a board specific way (i. e. in the board config file). [Not that this idea found many friends...] pull out common init code like init_baudrate() and hang() and change the function signatures of the functions that require wrappers and move some #ifdef's into more appropriate locations - One example in board.c: Well it seems like a lot of work to refactor each arch/xxx/board.c file into functions with a function pointer list, then later remove this code function by function. Would that really be a clever approach? My feeling is that with a generic board, it should hopefully be a fairly small amount of work for someone familiar with an architecture to find the bugs and patch the generic code to suit their architecture. It is something that needs to be done once, not every time there is a new patch removing (almost) common code. It is definitely not that easy. You fix one thing here, and break another board there. Ideally you would have to re-test any change on _all_ boards. From previous discussions, if something is optional then the switch will never happen. The code you are talking about is sometimes identical, sometimes slightly different. In some cases the order is different. I see many ways of introducing breakages. I do agree that And I bet most of them _will_ introduce breakages. doing one architecture at a time is best - with the proviso that we need to pick archs that have the most features (so ARM and PowerPC I suppose) to make sure we are not deluding ourselves as to the simplicity of the task. I would suggest to attempt to merge ARM into PPC. So perhaps a generic board init that is the default can be switched off on board-by-board basic would be the right approach. Then we can flick the switch while providing for those affected to still get work done until bugs are reported and found? Heh! Board specific init tables! Note that not all arches need and/or use ELF relocation - Attacking this first does not move towards unity of board.c It is a feature used by about half, and it does include the complexity of jumping from pre-reloc to post-reloc init. I think it was a reasonable first target. What exactly is the problem here? Per Wolfgang's request to go with PPC as an early-adopter, this is No. It's the other way round. PPC is what should be adopted to. Can anyone recommend a PowerPC board with a quick U-Boot program-run cycle that I can easily get that will let me try out things there? Define get easily. 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: w...@denx.de People have one thing in common: they are all different. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [RFC PATCH 0/19] Create generic board init and move ARM and x86 to it
Hi Simon,
Sorry for the delay in reviewing this - I've been doing a lot of work on
the x86 side of things. I now have a working solution to the
board_init_f_r() / global data clobbering problem which involves having the
gd 'variable' as a register like all other arch's. The solution is
non-trivial and gd access is slightly more expensive than the vanilla
variable approach, but it makes this a lot cleaner cross-arch wise...
Here's a hint ;)
static inline gd_t *get_fs_gd_ptr(void)
{
gd_t *gd_ptr;
asm volatile(fs movl 0, %0\n : =r (gd_ptr));
return gd_ptr;
}
#define gd get_fs_gd_ptr()
On 28/12/11 17:35, Simon Glass wrote:
This series creates a generic board.c implementation which contains
the essential functions of the various arch/xxx/lib/board.c files.
What is the motivation for this change?
[snip]
I think that we can all agree that there is strong motivation for change.
However, I think this approach is not the right one - and I think the CFI
driver backs me up. Your plan is to create generic code which you want ALL
arches to cross over to, but you only look to migrate two initially and
migrate the rest 'later'. This is similar to what happened with the CFI
driver, and there are still boards with custom flash.c files which are
completely redundant.
But, creating a single patch-set to migrate everyone in one go is going to
be too massive a job to do in one go, and too prone to introducing breakage.
All the functions of board_init_f() and board_init_r() are broken into
separate function calls so that they can easily be included or excluded
for a particular architecture. It also makes it easier to adopt Graeme's
initcall proposal later if desired.
I think we should look at this sooner rather than later. I've shown with
x86 that the init sequence has three distinct phases:
1) Execute in flash using temporary RAM
2) Copy U-Boot from flash to RAM
3) Execute in RAM
My latest work now has all init functions having the same signature (i.e.
void parameter returning int). For x86, there is a little 'magic' that
needs to be done as gd is copied from temporary RAM to SDRAM, but for other
arches using a dedicated register, this is otherwise trivial.
So instead of trying to pluck out something (relocation in this case) from
the molasses of ten different board.c files and having to perform
open-heart surgery on them all to get the code grafted in, I think we
should approach it from the generic init sequence angle.
If we work each individual arch to use a generic init sequence (like the
proposed x86 code) then the init processing naturally falls out as common
code and the patch to realise this is trivial. From there, we can start to
pull out common init code like init_baudrate() and hang() and change the
function signatures of the functions that require wrappers and move some
#ifdef's into more appropriate locations - One example in board.c:
#ifdef CONFIG_BITBANGMII
int bb_miiphy_init_r(void)
{
bb_miiphy_init();
return 0;
}
#endif
Ouch!
The other big benefit is that you only touch one architecture at a time up
until you 'pull the switch'. And when you do pull the switch, you should be
factoring out identical code so the chances of breaking something should be
vastly reduced. Take a look at the history of ARM relocation for example -
that was constrained to one arch but still the amount of breakage was massive.
Generic relocation is used (see previous series) but now rather than
calling relocate_code() to do everything, we call the individual
relocation steps one by one. Again this makes it easier to leave things
out, particularly for SPL.
Note that not all arches need and/or use ELF relocation - Attacking this
first does not move towards unity of board.c
ARM is a relatively large board.c file and one which I can test, therefore
I think it is a good target for this series. On the other hand, x86 is
relatively small and simple, but different enough that it introduces a
few issues to be solved. So I have chosen both ARM and x86 for this series.
The next target should probably be PowerPC, since it is large and has
some additional features. I suspect we may want to leave some of these
very architecture-specific functions in arch/powerpc/lib/board.c, taking
out only the generic code. I haven't felt a strong need to do this for
ARM/x86, but we could even go as far as putting the initcall list into
the architecture-specific board file if the architecture adds a lot of
unusual calls.
A generic global_data structure is also required. This might upset a few
people. Here is my basic reasoning: most fields are the same, all
architectures include and need it, most global_data.h files already have
#ifdefs to select fields for a particular SOC, so it is hard to
see why architecures are different in this area. We can perhaps add a
way to put architecture-specific fields into a separate header file, but
for now I have judged that to be
Re: [U-Boot] [RFC PATCH 0/19] Create generic board init and move ARM and x86 to it
Dear Graeme Russ, In message 4efddd7d.50...@gmail.com you wrote: I honestly think we should get the x86 init sequence patches finalised first for several reasons: - Because x86 is so small, it provides a good test-bed - ELF relocation was first finalised on x86 (it came and went with varying levels of success previously) - They bring x86 in line with other arches re: global data - They are now fully run-tested I agree with that. 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: w...@denx.de The game of life is a game of boomerangs. Our thoughts, deeds and words return to us sooner or later with astounding accuracy. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [RFC PATCH 0/19] Create generic board init and move ARM and x86 to it
Hi Graeme,
On Fri, Dec 30, 2011 at 7:49 AM, Graeme Russ graeme.r...@gmail.com wrote:
Hi Simon,
Sorry for the delay in reviewing this - I've been doing a lot of work on
the x86 side of things. I now have a working solution to the
board_init_f_r() / global data clobbering problem which involves having the
gd 'variable' as a register like all other arch's. The solution is
non-trivial and gd access is slightly more expensive than the vanilla
variable approach, but it makes this a lot cleaner cross-arch wise...
Here's a hint ;)
static inline gd_t *get_fs_gd_ptr(void)
{
gd_t *gd_ptr;
asm volatile(fs movl 0, %0\n : =r (gd_ptr));
return gd_ptr;
}
#define gd get_fs_gd_ptr()
On 28/12/11 17:35, Simon Glass wrote:
This series creates a generic board.c implementation which contains
the essential functions of the various arch/xxx/lib/board.c files.
What is the motivation for this change?
[snip]
I think that we can all agree that there is strong motivation for change.
However, I think this approach is not the right one - and I think the CFI
driver backs me up. Your plan is to create generic code which you want ALL
arches to cross over to, but you only look to migrate two initially and
migrate the rest 'later'. This is similar to what happened with the CFI
driver, and there are still boards with custom flash.c files which are
completely redundant.
But, creating a single patch-set to migrate everyone in one go is going to
be too massive a job to do in one go, and too prone to introducing breakage.
Yes to some extent. However, my patch basically splits the two
board_init_x() functions into parts, and puts them in a function
table.
I don't think it is a huge job to do this for PowerPC also, and that
seems to be the most feature-full architecture.
Also it does depend on expectations. I would hope that moving an
architecture over would be a fairly small task:
- getting generic relocation working
- adding functions for anything that is missing from board init code
- removing things which don't work on the architecture?
- worrying about differences in ordering between functions
All the functions of board_init_f() and board_init_r() are broken into
separate function calls so that they can easily be included or excluded
for a particular architecture. It also makes it easier to adopt Graeme's
initcall proposal later if desired.
I think we should look at this sooner rather than later. I've shown with
x86 that the init sequence has three distinct phases:
1) Execute in flash using temporary RAM
2) Copy U-Boot from flash to RAM
3) Execute in RAM
My latest work now has all init functions having the same signature (i.e.
void parameter returning int). For x86, there is a little 'magic' that
needs to be done as gd is copied from temporary RAM to SDRAM, but for other
arches using a dedicated register, this is otherwise trivial.
So instead of trying to pluck out something (relocation in this case) from
the molasses of ten different board.c files and having to perform
open-heart surgery on them all to get the code grafted in, I think we
should approach it from the generic init sequence angle.
If we work each individual arch to use a generic init sequence (like the
proposed x86 code) then the init processing naturally falls out as common
code and the patch to realise this is trivial. From there, we can start to
pull out common init code like init_baudrate() and hang() and change the
function signatures of the functions that require wrappers and move some
#ifdef's into more appropriate locations - One example in board.c:
Well it seems like a lot of work to refactor each arch/xxx/board.c
file into functions with a function pointer list, then later remove
this code function by function.
My feeling is that with a generic board, it should hopefully be a
fairly small amount of work for someone familiar with an architecture
to find the bugs and patch the generic code to suit their
architecture. It is something that needs to be done once, not every
time there is a new patch removing (almost) common code.
#ifdef CONFIG_BITBANGMII
int bb_miiphy_init_r(void)
{
bb_miiphy_init();
return 0;
}
#endif
Ouch!
The other big benefit is that you only touch one architecture at a time up
until you 'pull the switch'. And when you do pull the switch, you should be
factoring out identical code so the chances of breaking something should be
vastly reduced. Take a look at the history of ARM relocation for example -
that was constrained to one arch but still the amount of breakage was massive.
From previous discussions, if something is optional then the switch
will never happen. The code you are talking about is sometimes
identical, sometimes slightly different. In some cases the order is
different. I see many ways of introducing breakages. I do agree that
doing one architecture at a time is best - with the proviso that we
need to pick
Re: [U-Boot] [RFC PATCH 0/19] Create generic board init and move ARM and x86 to it
Dear Simon, In message 1325054160-24894-1-git-send-email-...@chromium.org you wrote: This series creates a generic board.c implementation which contains the essential functions of the various arch/xxx/lib/board.c files. I'll be loking into this deeper when I'm back at work (i. e. starting with week 2 next year), but I'd like to make this comment as early as possible: When making such changes, I _strongly_ recommend to use the Power architecture implementation as reference. There are a number of reasons for this, which I don't intend to explain in detail here and now (lack of time). But PPC is the most complete, longest living and most thoroughly designed architecture. Things that other architectures aquire only over time (relocation, cache support, FDT support, multiple busses [I2C, PCI, ...]) have always been supported by PPC. I fear that unifying any architectures and not including PPC rightr from the beginning will result in additional efforts later, so please have a look is this can be changed. 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: w...@denx.de Cigarette, n.: A fire at one end, a fool at the other, and a bit of tobacco in between. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [RFC PATCH 0/19] Create generic board init and move ARM and x86 to it
Hi Wolfgang, On Wed, Dec 28, 2011 at 9:30 AM, Wolfgang Denk w...@denx.de wrote: Dear Simon, In message 1325054160-24894-1-git-send-email-...@chromium.org you wrote: This series creates a generic board.c implementation which contains the essential functions of the various arch/xxx/lib/board.c files. I'll be loking into this deeper when I'm back at work (i. e. starting with week 2 next year), but I'd like to make this comment as early as possible: When making such changes, I _strongly_ recommend to use the Power architecture implementation as reference. There are a number of reasons for this, which I don't intend to explain in detail here and now (lack of time). But PPC is the most complete, longest living and most thoroughly designed architecture. Things that other architectures aquire only over time (relocation, cache support, FDT support, multiple busses [I2C, PCI, ...]) have always been supported by PPC. I fear that unifying any architectures and not including PPC rightr from the beginning will result in additional efforts later, so please have a look is this can be changed. Thanks. Yes that has been in the back of my mind. I have had a look through this but have not started a serious effort on it. There are quite a few extra bits. My main concern is that I cannot test PPC, but I will see if I can put together a patch in any case. Regards, Simon 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: w...@denx.de Cigarette, n.: A fire at one end, a fool at the other, and a bit of tobacco in between. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
