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

[Dirk Behme <dirk.behme@de.bosch.com>]

On 27.03.2013 09:02, Dirk Behme wrote:
> On 26.03.2013 18:04, Fabio Estevam wrote:
>> Hi Dirk,
>>
>> On Tue, Mar 26, 2013 at 12:43 PM, Dirk Behme <dirk.behme@de.bosch.com> 
>> wrote:
>>> Hi Fabio,
>>>
>>>
>>> On 26.03.2013 13:54, Fabio Estevam wrote:
>>>> Currently when booting a mx6 solo processor get_cpu_rev() returns 
>>>> 0x62xxx,
>>>> which
>>>> is an invalid mx6 CPU revision.
>>>
>>> Do you have somewhere a list of valid CPU revisions? From two points of
>>> view:
>>>
>>> a) the i.MX6 hardware spec
>>>
>>> b) the VPU library
>>
>> Sorry, I don't. I am basing the CPU revision numbers from FSL U-boot:
>> http://git.freescale.com/git/cgit.cgi/imx/uboot-imx.git/tree/board/freescale/common/fsl_sys_rev.c?h=imx_v2009.08_3.0.0 
>>
>>
>> Adding Jason, in case he could clarify it.
>>
>>> You remove Troy's code here introduced with
>>>
>>> http://git.denx.de/cgi-bin/gitweb.cgi?p=u-boot.git;a=commitdiff;h=20332a066aff98f39419495821e14edd10b2a3f8 
>>>
>>>
>>> Troy's detection you remove here intentionally distinguishes between
>>> DualLite and Solo. You now re-introduce a common DL_S, again.
>>>
>>> Additionally, you completely seem to drop checking for scu->config. I've
>>> already seen some (broken?) i.MX6Solo where this check was essential.
>>>
>>> I can't talk about the "problems when trying to use VPU library in the
>>> kernel" (btw, which problems?) and the invalid 0x62xxx, but we used 
>>> Troy's
>>> version of the detection successfully.
>>
>> Passing 0x62xxx as cpu_rev on a mx6solo caused the VPU issues 
>> described here:
>> https://community.freescale.com/thread/305396
>>
>> Which cpu_rev value is returned with your mx6solo? Are you able to use 
>> VPU lib?
> 
> I'll check this.
> 
> Rethinking about the issue here, my recent understanding is:
> 
> a) We have a VPU library which only understands 0x63 (Quad) and 0x61 
> (DualLite/Solo)
> 
> b) We have Troy's existing get_cpu_rev() [1] which seems to correctly 
> decode the CPU revision (at least this is my impression from testing ;) 
> ). But reports 0x62 for the Solo which then isn't understood by the VPU 
> library (to be checked).

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.

In this case I'd propose to just keep Troy's version of get_cpu_rev() as 
it is [1].

Sorry for the confusion, hopefully this is correct now ;)

Best regards

Dirk

(*) There might be U-Boot/Kernel combinations out there, where U-Boot 
exports the CPU revision via ATAGs to the kernel. But hopefully this 
doesn't affect us here (?)

> I wonder if we could find a way to combine both parts without breaking 
> the other? I.e. using Troy's get_cpu_rev() to correctly report the CPU 
> revision (in U-Boot), but let the VPU library get the revision it 
> understands?
> 
> Best regards
> 
> Dirk
> 
> [1] 
> http://git.denx.de/cgi-bin/gitweb.cgi?p=u-boot.git;a=commitdiff;h=20332a066aff98f39419495821e14edd10b2a3f8