Re: [U-Boot] CONFIG_REVISION (was i.MX6: mx6qsabrelite: allow use with Freescale 2.6.38 kernels)

7 Mar 2012


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