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

Stefano Babic
2012-03-07T16:32:31Z

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