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

[Eric Nelson <eric.nelson@boundarydevices.com>]

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
>