On 03/03/2012 14:30, Wolfgang Denk wrote: > Dear Dirk Behme, > Hi,
> In message <4f52015a.2080...@googlemail.com> you wrote: >> >>> Agreed. If these patches were only for backward compatibility I would >>> not complain much. But they are known to introduce forward incompati- >>> bilities with all this MACH_ID stuff, and this is what I would like to >>> avoid. >> >> Now I'm just trying to learn something regarding [1]: >> >> Which changes would you accept in the category 'backward compatibility'? > > There are 3 commits in this series: > > [PATCH 1/3] i.MX6: mx6qsabrelite: add CONFIG_REVISION_TAG > [PATCH 2/3] i.MX6: mx6qsabrelite: add MACH_TYPE_MX6Q_SABRELITE > [PATCH 3/3] i.MX6: mx6qsabrelite: add ext2 support > > I dislike #1 because it uses the completely undocumented > CONFIG_REVISION_TAG, and I agree with Marek's and Stefano's comments. A lot of boards are currently set CONFIG_REVISION_TAG. Sure, it would be nice to document it. To be consistent we should drop this CONFIG_ from all boards, or add documentation for it. However, I am asking if this TAG is really needed. I have searched in 2.6.38 Kernel provided by Freescale if the TAG is really evaluated to set different revisions of the boards, but I have not found anything. Is it really needed ? If not, we should drop it. > > The problems I mentioned are with # 2, which now would depend on > MACH_TYPE_MX6Q_SABRELITE, which may or may not exist. Right, I agree. And we do not know (maybe it is not this case) if MACH_TYPE_MX6Q_SABRELITE will be dropped in the future. IMHO we should not use mach-types at all.. > > Also, I think we should not need this any more at all, as we now have > DT support in Linux on ARM, too. > > I see no issues with # 3. I will merge #3 into u-boot-imx > >> And which changes 'introduce forward incompatibilities', and what are >> these incompatibilities? > > See the recent problems that occurred when RMK decided to "clean up" > the machids file. > >> (assuming this will be changed to >> >> --- a/include/configs/mx6qsabrelite.h >> +++ b/include/configs/mx6qsabrelite.h >> @@ -27,6 +27,7 @@ >> #define CONFIG_SYS_MX6_CLK32 32768 >> #define CONFIG_DISPLAY_CPUINFO >> #define CONFIG_DISPLAY_BOARDINFO >> +#define CONFIG_MACH_TYPE 3769 > > Such a change would avoid breakages as the ones mentioned above, but > is this nice? Either we have a mach-types.h that can be used, or we > don't, Personally I vote for the second choice. U-boot does not use mach-types, and it is at the end only a parameter for the kernel. I think the solution in the patch is better as to try to maintain the mach-types file: not very nice, but setting its value is not different as several other setups inside the configuration file that are very board specifiv. And CONFIG_MACH_TYPE is well documented. > in which case we should stop using any definitions from it, > especially for new boards where it's not needed due to DT support in > the kernel. Agree completely - mach-types introduces a strong dependency with the kernel, and we do not need it. Best regards, Stefano Babic -- ===================================================================== DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: off...@denx.de ===================================================================== _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
