From mboxrd@z Thu Jan  1 00:00:00 1970
From: Eric Nelson <eric.nelson@boundarydevices.com>
Date: Wed, 07 Mar 2012 08:53:09 -0700
Subject: [U-Boot] CONFIG_REVISION (was i.MX6: mx6qsabrelite: allow use
 with Freescale 2.6.38 kernels)
In-Reply-To: <4F571FC3.3070100@denx.de>
References: <1330728909-12203-1-git-send-email-eric.nelson@boundarydevices.com>	<20120302232545.464F212A7F30@gemini.denx.de>	<4F51BBA9.4090608@googlemail.com>	<20120303093826.C3054126F3B0@gemini.denx.de>	<4F52015A.2080003@googlemail.com>	<20120303133050.690AA82301@gemini.denx.de>
	<4F534D67.3010709@denx.de> <4F53C64D.4040407@boundarydevices.com>
	<4F571FC3.3070100@denx.de>
Message-ID: <4F578465.6030503@boundarydevices.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

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
>