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

Eric Nelson
2012-03-07T15:53:09Z

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
>