Patchwork [U-Boot] i.MX6: mx6qsabrelite: allow use with Freescale 2.6.38 kernels

login
register
mail settings
Submitter Dirk Behme
Date March 3, 2012, 11:32 a.m.
Message ID <4F52015A.2080003@googlemail.com>
Download mbox | patch
Permalink /patch/144417/
State Changes Requested
Headers show

Comments

Dirk Behme - March 3, 2012, 11:32 a.m.
Dear Wolfgang,

On 03.03.2012 10:38, Wolfgang Denk wrote:
> Dear Dirk,
>
> In message<4F51BBA9.4090608@googlemail.com>  you wrote:
>>
>> Having Freescale working on these quite old and unclean U-Boot and
>> Kernel versions is a pain. Kernel is an other topic, but with U-Boot,
>> thanks to the very good work of everybody, we are in a good position
>> to get rid of the old Freescale U-Boot, now. And get everybody to work
>> with mainline and create patches against it.
>
> ACK.
>
>> So if it helps to apply some backward compatibility to make it easier
>> for everybody, esp. Freescale, to switch to mainline U-Boot, I think
>> we should try it.
>
> 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'?

And which changes 'introduce forward incompatibilities', and what are 
these incompatibilities?

Many thanks and best regards

Dirk

[1]

http://lists.denx.de/pipermail/u-boot/2012-March/119207.html

http://lists.denx.de/pipermail/u-boot/2012-March/119208.html

(assuming this will be changed to

)
Wolfgang Denk - March 3, 2012, 1:30 p.m.
Dear Dirk Behme,

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.

The problems I mentioned are with # 2, which now would depend on
MACH_TYPE_MX6Q_SABRELITE, which may or may not exist.

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.

> 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, 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.

Best regards,

Wolfgang Denk
Troy Kisky - March 4, 2012, 1:19 a.m.
On 3/3/2012 6:30 AM, Wolfgang Denk wrote:
> Dear Dirk Behme,
>
> 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.
>
> The problems I mentioned are with # 2, which now would depend on
> MACH_TYPE_MX6Q_SABRELITE, which may or may not exist.
>
> 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.
>
>> 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.
>
>
Would you rather that I take RMK's cleaned up file, and undelete the 
machines that u-boot
uses?  That would be more simple than adding to the board's config file.
I can delete all of the mach_is_xxx macros in mach-types while I'm at it.


Thanks
Troy
Wolfgang Denk - March 4, 2012, 8:39 a.m.
Dear Troy Kisky,

In message <4F52C327.3080309@boundarydevices.com> you wrote:
>
> >> 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.
> >
> Would you rather that I take RMK's cleaned up file, and undelete the 
> machines that u-boot
> uses?  That would be more simple than adding to the board's config file.
> I can delete all of the mach_is_xxx macros in mach-types while I'm at it.

I think we had this discussion before (when RMK's changes hit us), and
it was decided not to do this.  IIRC we decided not to do this.

I have never understood why this mach_types thingy was needed (other
rchitectures worked fine without it, or better), and now we are on the
edge of obsoleting it. So all efforts trying to maintain this file are
futile, and we would have to redo these for any updates of the file.

I feel this is not a good investment of our time.

Best regards,

Wolfgang Denk
Stefano Babic - March 4, 2012, 11:09 a.m.
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
Igor Grinberg - March 4, 2012, 2:14 p.m.
Hi,

On 03/04/12 13:09, Stefano Babic wrote:
> 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.

Linux's system_rev global variable is set to that value.
You can check for it by "cat /proc/cpuinfo | grep Revision".

> 
>>
>> 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..

+1
making the mach-type local to a board moves the maintenance burden
to a board maintainer which is good because it can be decided on per
board basis.
Also, we should always raise the question if it is still needed
for a particular board and don't let it in, if it is not...

> 
>>
>> 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.

+1

> 
> 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.

+1

> And CONFIG_MACH_TYPE is well documented.

10x ;-)

> 
>> 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.

+1

I think, some more time is needed for the transition to DT.
We should let the boards use non-DT configurations, but move the
maintenance to the board maintainer (i.e. set the CONFIG_MACH_TYPE
in the board config file).
Also, soon there will be no non-DT boards accepted in the mainline kernel
(probably, also, there will be no new machine types accepted in the machine
types registry), so the need for the mach-type will just go away, no?
Eric Nelson - March 4, 2012, 7:45 p.m.
On 03/04/2012 04:09 AM, Stefano Babic wrote:
> On 03/03/2012 14:30, Wolfgang Denk wrote:
>> Dear Dirk Behme,
>>
>> <snip>
>>
>> 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 linkage is really indirect. The ATAG item is still supported in the
main-line kernel for ARM:
	http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=blob;f=arch/arm/kernel/setup.c;h=a255c39612ca3cfa10bddb7c7728216efeeb04d5;hb=HEAD#l704

The breakage I noticed was in the VPU driver, which refused to load
with a zero-value in system_rev. The net result was no video playback
in the Freescale Android ICS release.
Stefano Babic - March 7, 2012, 8:43 a.m.
On 04/03/2012 20:45, Eric Nelson wrote:

> The linkage is really indirect. The ATAG item is still supported in the
> main-line kernel for ARM:
>     http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=blob;f=arch/arm/kernel/setup.c;h=a255c39612ca3cfa10bddb7c7728216efeeb04d5;hb=HEAD#l704
> 
> 
> The breakage I noticed was in the VPU driver, which refused to load
> with a zero-value in system_rev. The net result was no video playback
> in the Freescale Android ICS release.

However, I have not found a statement in FSL's kernel where system_rev
in VPU driver is checked, directly or indirectly - neither in VPU driver
nor in the board initialization code, nor in another MXC driver.
It seems to me a side-effect in FSL's kernel, and for some not yet known
reasons the problem disappears with this tag - or can reappear later,
because we do not know the cause.

And if I am not wrong, there are two macros in FSL's kernel checking the
system_rev (arch/arm/plat-mxc/include/mach/mxc.h):

#define imx_cpu_ver()		(system_rev & 0xFF)
#define board_is_rev(rev)	(((system_rev & 0x0F00) == rev) ? 1 : 0)

But you defines board_rev as:
> +u32 get_board_rev(void)
> +{
> +	return 0x63000 ;

Both macro still return 0x00...there is no change if the ATAG is not
set. Sure that the problem you report is really bound to system_rev ? I
am quite OT here, we are investigating an issue in FSL kernel on the
U-boot ML.

Anyway, the ATAG is supported and as I already said quite common for
U-Boot boards. The commit message " Freescale 2.6.38 (Non-DT) kernels
require the revision atag to enable the VPU." should be extended
explaining the real cause (if known) or changed dropping VPU because
there is no clear relationship between the ATAG and the issue.

Best regards,
Stefano Babic
Albert ARIBAUD - March 7, 2012, 9:37 a.m.
Hi Wolfgang,

Le 04/03/2012 09:39, Wolfgang Denk a écrit :
> Dear Troy Kisky,
>
> In message<4F52C327.3080309@boundarydevices.com>  you wrote:
>>
>>>> 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.
>>>
>> Would you rather that I take RMK's cleaned up file, and undelete the
>> machines that u-boot
>> uses?  That would be more simple than adding to the board's config file.
>> I can delete all of the mach_is_xxx macros in mach-types while I'm at it.
>
> I think we had this discussion before (when RMK's changes hit us), and
> it was decided not to do this.  IIRC we decided not to do this.

YRC. :)

The retained strategy is to not fiddle with the Linux mach-type.h and to 
complement (and lampshade) any discrepancy between Linux and U-Boot 
support in the board config files.

> I have never understood why this mach_types thingy was needed (other
> rchitectures worked fine without it, or better), and now we are on the
> edge of obsoleting it. So all efforts trying to maintain this file are
> futile, and we would have to redo these for any updates of the file.
>
> I feel this is not a good investment of our time.

Hopefully FDT will make mach-types obsolete, except for the rare boards 
which will want to keep support for it, at which point we'll decide to 
either maintain or own, reduced and mostly legacy, mach-type file, or to 
move mach-type declarations in the board's config files.

> Best regards,
>
> Wolfgang Denk

Amicalement,
Eric Nelson - March 7, 2012, 3:53 p.m.
Hi Stefano,

Thanks for the feedback and the prod.

On 03/07/2012 01:43 AM, Stefano Babic wrote:
> On 04/03/2012 20:45, Eric Nelson wrote:
>
>> The linkage is really indirect. The ATAG item is still supported in the
>> main-line kernel for ARM:
>>      http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=blob;f=arch/arm/kernel/setup.c;h=a255c39612ca3cfa10bddb7c7728216efeeb04d5;hb=HEAD#l704
>>
>> The breakage I noticed was in the VPU driver, which refused to load
>> with a zero-value in system_rev. The net result was no video playback
>> in the Freescale Android ICS release.
>
> However, I have not found a statement in FSL's kernel where system_rev
> in VPU driver is checked, directly or indirectly - neither in VPU driver
> nor in the board initialization code, nor in another MXC driver.
> It seems to me a side-effect in FSL's kernel, and for some not yet known
> reasons the problem disappears with this tag - or can reappear later,
> because we do not know the cause.
>

I did the same search but was apparently not as persistent as you were.
The symptom is simple: Video won't play back on Android (R13.1 ICS)
without the system revision but play nicely with it.

> And if I am not wrong, there are two macros in FSL's kernel checking the
> system_rev (arch/arm/plat-mxc/include/mach/mxc.h):
>
> #define imx_cpu_ver()		(system_rev&  0xFF)
> #define board_is_rev(rev)	(((system_rev&  0x0F00) == rev) ? 1 : 0)
>
> But you defines board_rev as:
>> +u32 get_board_rev(void)
>> +{
>> +	return 0x63000 ;
>
> Both macro still return 0x00...there is no change if the ATAG is not
> set. Sure that the problem you report is really bound to system_rev ? I
> am quite OT here, we are investigating an issue in FSL kernel on the
> U-boot ML.
>

I think I just found the culprit, and it's in userspace, not in the
kernel. In package imx-lib-11.11.01, file vpu/vpu.c, there's this
routine that sets a global system_rev based on /proc/cpuinfo:

static int get_system_rev(void)
{
	FILE *fp;
	char buf[1024];
	int nread;
	char *tmp, *rev;
	int ret = -1;

	fp = fopen("/proc/cpuinfo", "r");
	if (fp == NULL) {
		perror("/proc/cpuinfo\n");
		return ret;
	}

	nread = fread(buf, 1, sizeof(buf), fp);
	fclose(fp);
	if ((nread == 0) || (nread == sizeof(buf))) {
		fclose(fp);
		return ret;
	}

	buf[nread] = '\0';

	tmp = strstr(buf, "Revision");
	if (tmp != NULL) {
		rev = index(tmp, ':');
		if (rev != NULL) {
			rev++;
			system_rev = strtoul(rev, NULL, 16);
			ret = 0;
		}
	}

	return ret;
}

The global is then exported via macros:
	vpu/vpu_io.c:unsigned int system_rev;
	vpu/vpu_io.c:static int get_system_rev(void)
	vpu/vpu_io.c:			system_rev = strtoul(rev, NULL, 16);
	vpu/vpu_io.c:	ret = get_system_rev();
	vpu/vpu_lib.h:extern unsigned int system_rev;
	vpu/vpu_lib.h:#define mxc_cpu()               (system_rev >> 12)
	vpu/vpu_lib.h:#define mxc_cpu_rev()           (system_rev & 0xFF)

and used to find the firmware file:
	vpu/vpu_util.c:	sprintf(temp_str, "vpu_fw_imx%2x.bin", mxc_cpu());

...all to support the userspace I/O for the VPU.

We are way off topic here, but I certainly hope we can address this in the
future and get a real driver written for the VPU.

> Anyway, the ATAG is supported and as I already said quite common for
> U-Boot boards. The commit message " Freescale 2.6.38 (Non-DT) kernels
> require the revision atag to enable the VPU." should be extended
> explaining the real cause (if known) or changed dropping VPU because
> there is no clear relationship between the ATAG and the issue.
>

How about something more generic like this?
	"Freescale Linux distributions depend on system_rev".

> Best regards,
> Stefano Babic
>
Stefano Babic - March 7, 2012, 4:32 p.m.
On 07/03/2012 16:53, Eric Nelson wrote:
> Hi Stefano,
> 

Hi Eric,

> 
> I did the same search but was apparently not as persistent as you were.
> The symptom is simple: Video won't play back on Android (R13.1 ICS)
> without the system revision but play nicely with it.

Ok, we know the symptoms...

> I think I just found the culprit, and it's in userspace, not in the
> kernel. In package imx-lib-11.11.01, file vpu/vpu.c, there's this
> routine that sets a global system_rev based on /proc/cpuinfo:
> 
> static int get_system_rev(void)
> {
>     FILE *fp;
>     char buf[1024];
>     int nread;
>     char *tmp, *rev;
>     int ret = -1;
> 
>     fp = fopen("/proc/cpuinfo", "r");
>     if (fp == NULL) {
>         perror("/proc/cpuinfo\n");
>         return ret;
>     }
> 
>     nread = fread(buf, 1, sizeof(buf), fp);
>     fclose(fp);
>     if ((nread == 0) || (nread == sizeof(buf))) {
>         fclose(fp);
>         return ret;
>     }
> 
>     buf[nread] = '\0';
> 
>     tmp = strstr(buf, "Revision");
>     if (tmp != NULL) {
>         rev = index(tmp, ':');
>         if (rev != NULL) {
>             rev++;
>             system_rev = strtoul(rev, NULL, 16);
>             ret = 0;
>         }
>     }
> 
>     return ret;
> }
> 
> The global is then exported via macros:
>     vpu/vpu_io.c:unsigned int system_rev;
>     vpu/vpu_io.c:static int get_system_rev(void)
>     vpu/vpu_io.c:            system_rev = strtoul(rev, NULL, 16);
>     vpu/vpu_io.c:    ret = get_system_rev();
>     vpu/vpu_lib.h:extern unsigned int system_rev;
>     vpu/vpu_lib.h:#define mxc_cpu()               (system_rev >> 12)
>     vpu/vpu_lib.h:#define mxc_cpu_rev()           (system_rev & 0xFF)
> 
> and used to find the firmware file:
>     vpu/vpu_util.c:    sprintf(temp_str, "vpu_fw_imx%2x.bin", mxc_cpu());
> 
> ...all to support the userspace I/O for the VPU.

Understood - this is really crappy, because it makes so absurd
dependencies that is very easy to break - and when it happens, nobody
knows why, as we find now. Really this is a problem neither in u-boot
nor in kernel...

> 
> We are way off topic here,

Well, we have now the cause...

> but I certainly hope we can address this in the
> future and get a real driver written for the VPU.

..in the mainline kernel...

> 
>> Anyway, the ATAG is supported and as I already said quite common for
>> U-Boot boards. The commit message " Freescale 2.6.38 (Non-DT) kernels
>> require the revision atag to enable the VPU." should be extended
>> explaining the real cause (if known) or changed dropping VPU because
>> there is no clear relationship between the ATAG and the issue.
>>
> 
> How about something more generic like this?
>     "Freescale Linux distributions depend on system_rev".

Because you deeply investigated and found the reason, I propose you add
a full description indicating that the imx lib libraries depend on the
system_rev in kernel to transfer the correct firmware. So we know it is
neither a problem in u-boot nor in kernel, but we as u-bootlers are fair
with some bad implemented libraries....

Normally I would say that the fix should be done where the bug is - we
are introducing a work-around for a problem in user space. But as I
stated previously, the revision tag is used on a lot of ARM boards, and
there is no reason to reject it only on this board.

Best regards,
Stefano Babic

Patch

--- 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

  #include <asm/arch/imx-regs.h>