[U-Boot] [PATCH] mx6: Fix the reading of CPU revision

[Dirk Behme]

Hi Eric,

On 27.03.2013 15:00, Eric Nelson wrote:
> Hi Fabio,
> 
> On 03/27/2013 06:37 AM, Fabio Estevam wrote:
>> On Wed, Mar 27, 2013 at 5:57 AM, Dirk Behme <dirk.behme at de.bosch.com> wrote:
>>
>>> Some additional rethinking: I missed that we have a Linux kernel, too ;)
>>>
>>> c) It's the job of the Linux kernel to export the CPU revision to the VPU
>>> library. In case the Linux kernel completely ignores what we are doing in
>>> U-Boot and calculates the CPU revision itself (*), e.g. by something like
>>>
>>> http://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git/tree/arch/arm/mach-mx6/mm.c?h=imx_3.0.35_1.1.0&id=207f80453c77bc32e04b5fef863f6fe50a7fd1a8#n60
>>>
>>> we can do anything in U-Boot. Independent of the VPU library.
>> Unfortunately VPU library relies on the bootloader to pass the correct
>> silicon revision.
>>
> 
> The VPU library relies on the output of /proc/cpuinfo (specifically
> the line beginning with "Revision".
> 
> The snippet (from vpu_io.h) is:
> 
> 	tmp = strstr(buf, "Revision");
> 	if (tmp != NULL) {
> 		rev = index(tmp, ':');
> 		if (rev != NULL) {
> 			rev++;
> 			system_rev = strtoul(rev, NULL, 16);
> 			ret = 0;
> 		}
> 	}
> 
> This code should really be changed, 

Yes :)

> so we don't have to carry this
> data all the way from boot loader to /proc/cpuinfo.

As mentioned in my previous mail, I have some doubts that *all* kernels 
pick the version from the boot loader. It's my understanding that this 
strongly depends on the kernel? I.e. there are kernels which get the 
version from the boot loader, e.g. via ATAGs. But there are kernels 
which are independent from the boot loader and calculate it on their 
own? E.g.

http://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git/tree/arch/arm/mach-mx6/mm.c?h=imx_3.0.35_1.1.0&id=207f80453c77bc32e04b5fef863f6fe50a7fd1a8#n60

?

Best regards

Dirk

> Similar (but different) code is present in mxc_ipu_hl_lib.c
> for the IPU.
> 
> In the case of the VPU library, it seems more sane to have the
> VPU driver expose the particular IP revision present on the
> system.
> 
>> Eric's tested passing 0 as get_cpu_rev and showed that VPU simply
>> cannot work on this case.
>>
>>> In this case I'd propose to just keep Troy's version of get_cpu_rev() as it
>>> is [1].
>> This is proven to not to work with mx6solo and VPU, so we need the fix
>> I proposed.
>>
>> Here is what I am planning to do:
>>
>> 1. Send a v2 of this patch with the small correction pointed out by Eric
>> 2. Include a weak function to pass get_cpu_rev in common mx6 code
>>
>> Then on top of that, one can send a patch that prints the mx6 silicon
>> strings by differentiating between a mx6dual-lite and mx6solo, if it
>> is worth.
>>
> 
> It seems a reasonable interim solution to provide backward
> compatibility until the kernel driver(s) and userspace can be
> fixed.
> 
> Another way of doing this that prevents get_cpu_rev() from
> hiding the precise CPU is to do this in the "weak" version
> of get_board_rev().
> 
> Regards,