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

[Stefano Babic]

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

-- 
=====================================================================
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: office at denx.de
=====================================================================