On 03/03/2012 14:30, Wolfgang Denk wrote:
Dear Dirk Behme,
Hi,
In message 4F52015A.2080003@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