[U-Boot] [PATCH 1/2] nitrogen6x: Pass the correct CPU revision to the kernel

[U-Boot] [PATCH 1/2] nitrogen6x: Pass the correct CPU revision to the kernel

[Fabio Estevam]

From: Fabio Estevam <fabio.estevam@freescale.com>

As nitrogen6x boards support different i.MX6 flavors (quad, dual-lite and solo)
the correct CPU revision needs to passed to the kernel, so call get_cpu_rev()
instead of hardcoding it.

Freescale 3.0.35 kernel assumes that the CPU revision is passed passed from the 
bootloader.

Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
 board/boundary/nitrogen6x/nitrogen6x.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/board/boundary/nitrogen6x/nitrogen6x.c b/board/boundary/nitrogen6x/nitrogen6x.c
index 229c237..fec0e3a 100644
--- a/board/boundary/nitrogen6x/nitrogen6x.c
+++ b/board/boundary/nitrogen6x/nitrogen6x.c
@@ -330,7 +330,7 @@ int board_mmc_init(bd_t *bis)
 
 u32 get_board_rev(void)
 {
-	return 0x63000;
+	return get_cpu_rev();
 }
 
 #ifdef CONFIG_MXC_SPI
-- 
1.7.9.5

[U-Boot] [PATCH 2/2] mx6qsabrelite: Do not hardcode the CPU revision

[Fabio Estevam]

From: Fabio Estevam <fabio.estevam@freescale.com>

Instead of hardcoding the CPU revision, it is better to use get_cpu_rev().

Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
 board/freescale/mx6qsabrelite/mx6qsabrelite.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/board/freescale/mx6qsabrelite/mx6qsabrelite.c b/board/freescale/mx6qsabrelite/mx6qsabrelite.c
index 5b69a6d..9bd444e 100644
--- a/board/freescale/mx6qsabrelite/mx6qsabrelite.c
+++ b/board/freescale/mx6qsabrelite/mx6qsabrelite.c
@@ -26,6 +26,7 @@
 #include <asm/arch/imx-regs.h>
 #include <asm/arch/iomux.h>
 #include <asm/arch/mx6q_pins.h>
+#include <asm/arch/sys_proto.h>
 #include <asm/errno.h>
 #include <asm/gpio.h>
 #include <asm/imx-common/iomux-v3.h>
@@ -300,7 +301,7 @@ int board_mmc_init(bd_t *bis)
 
 u32 get_board_rev(void)
 {
-	return 0x63000 ;
+	return get_cpu_rev();
 }
 
 #ifdef CONFIG_MXC_SPI
-- 
1.7.9.5

[U-Boot] [PATCH 1/2] nitrogen6x: Pass the correct CPU revision to the kernel

[Eric Nelson]

On 03/15/2013 02:06 PM, Fabio Estevam wrote:
> From: Fabio Estevam <fabio.estevam@freescale.com>
>
> As nitrogen6x boards support different i.MX6 flavors (quad, dual-lite and solo)
> the correct CPU revision needs to passed to the kernel, so call get_cpu_rev()
> instead of hardcoding it.
>
> Freescale 3.0.35 kernel assumes that the CPU revision is passed passed from the
> bootloader.
>
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> ---
>   board/boundary/nitrogen6x/nitrogen6x.c |    2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/board/boundary/nitrogen6x/nitrogen6x.c b/board/boundary/nitrogen6x/nitrogen6x.c
> index 229c237..fec0e3a 100644
> --- a/board/boundary/nitrogen6x/nitrogen6x.c
> +++ b/board/boundary/nitrogen6x/nitrogen6x.c
> @@ -330,7 +330,7 @@ int board_mmc_init(bd_t *bis)
>
>   u32 get_board_rev(void)
>   {
> -	return 0x63000;
> +	return get_cpu_rev();
>   }
>
>   #ifdef CONFIG_MXC_SPI
>

This is the **board** revision, right?

At first glance, the kernel seems to be getting the silicon revision
from the same place as get_cpu_rev():
	https://github.com/boundarydevices/linux-imx6/blob/boundary-imx_3.0.35_1.1.1/arch/arm/mach-mx6/cpu.c#L51
	http://git.denx.de/u-boot.git/?p=u-boot.git;a=blob;f=arch/arm/cpu/armv7/mx6/soc.c;h=a8aad5dd0a6c8548277021ebe8f6e159dbf31b9b;hb=HEAD#l42

Is there a reference to the ATAG that I'm not seeing somewhere?

Please advise,


Eric

[U-Boot] [PATCH 2/2] mx6qsabrelite: Do not hardcode the CPU revision

[Dirk Behme]

Am 15.03.2013 22:06, schrieb Fabio Estevam:
> From: Fabio Estevam <fabio.estevam@freescale.com>
>
> Instead of hardcoding the CPU revision, it is better to use get_cpu_rev().

I think to remember that there is a reason why it is hard coded this 
way. Have you tested this with the Vivante GPU driver? If I remember 
correctly they check for exactly the 0x63000 and if it's not there, it 
won't work (?).

Best regards

Dirk

> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> ---
>   board/freescale/mx6qsabrelite/mx6qsabrelite.c |    3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/board/freescale/mx6qsabrelite/mx6qsabrelite.c b/board/freescale/mx6qsabrelite/mx6qsabrelite.c
> index 5b69a6d..9bd444e 100644
> --- a/board/freescale/mx6qsabrelite/mx6qsabrelite.c
> +++ b/board/freescale/mx6qsabrelite/mx6qsabrelite.c
> @@ -26,6 +26,7 @@
>   #include <asm/arch/imx-regs.h>
>   #include <asm/arch/iomux.h>
>   #include <asm/arch/mx6q_pins.h>
> +#include <asm/arch/sys_proto.h>
>   #include <asm/errno.h>
>   #include <asm/gpio.h>
>   #include <asm/imx-common/iomux-v3.h>
> @@ -300,7 +301,7 @@ int board_mmc_init(bd_t *bis)
>
>   u32 get_board_rev(void)
>   {
> -	return 0x63000 ;
> +	return get_cpu_rev();
>   }
>
>   #ifdef CONFIG_MXC_SPI
>

[U-Boot] [PATCH 2/2] mx6qsabrelite: Do not hardcode the CPU revision

[Wolfgang Denk]

Dear Dirk Behme,

In message <51440A4D.5060005@gmail.com> you wrote:
>
> I think to remember that there is a reason why it is hard coded this 
> way. Have you tested this with the Vivante GPU driver? If I remember 
> correctly they check for exactly the 0x63000 and if it's not there, it 
> won't work (?).

Then this driver needs fixing?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Men of peace usually are [brave].
	-- Spock, "The Savage Curtain", stardate 5906.5

[U-Boot] [PATCH 2/2] mx6qsabrelite: Do not hardcode the CPU revision

[Fabio Estevam]

Hi Dirk,

On Sat, Mar 16, 2013 at 2:59 AM, Dirk Behme <dirk.behme@gmail.com> wrote:
> Am 15.03.2013 22:06, schrieb Fabio Estevam:
>
>> From: Fabio Estevam <fabio.estevam@freescale.com>
>>
>> Instead of hardcoding the CPU revision, it is better to use get_cpu_rev().
>
>
> I think to remember that there is a reason why it is hard coded this way.
> Have you tested this with the Vivante GPU driver? If I remember correctly
> they check for exactly the 0x63000 and if it's not there, it won't work (?).

Good point, maybe we can do:

return (get_cpu_rev() & ~0xfff);

Regards,

Fabio Estevam

[U-Boot] [PATCH 2/2] mx6qsabrelite: Do not hardcode the CPU revision

[Otavio Salvador]

On Sat, Mar 16, 2013 at 11:50 AM, Fabio Estevam <festevam@gmail.com> wrote:
> Hi Dirk,
>
> On Sat, Mar 16, 2013 at 2:59 AM, Dirk Behme <dirk.behme@gmail.com> wrote:
>> Am 15.03.2013 22:06, schrieb Fabio Estevam:
>>
>>> From: Fabio Estevam <fabio.estevam@freescale.com>
>>>
>>> Instead of hardcoding the CPU revision, it is better to use get_cpu_rev().
>>
>>
>> I think to remember that there is a reason why it is hard coded this way.
>> Have you tested this with the Vivante GPU driver? If I remember correctly
>> they check for exactly the 0x63000 and if it's not there, it won't work (?).
>
> Good point, maybe we can do:
>
> return (get_cpu_rev() & ~0xfff);

Or add a method to do that? as it'd need to be done in all i.MX6 boards.

-- 
Otavio Salvador                             O.S. Systems
E-mail: otavio at ossystems.com.br  http://www.ossystems.com.br
Mobile: +55 53 9981-7854              http://projetos.ossystems.com.br

[U-Boot] [PATCH 1/2] nitrogen6x: Pass the correct CPU revision to the kernel

[Fabio Estevam]

Hi Eric,

On Fri, Mar 15, 2013 at 9:20 PM, Eric Nelson
<eric.nelson@boundarydevices.com> wrote:

> This is the **board** revision, right?
>
> At first glance, the kernel seems to be getting the silicon revision
> from the same place as get_cpu_rev():
>
> https://github.com/boundarydevices/linux-imx6/blob/boundary-imx_3.0.35_1.1.1/arch/arm/mach-mx6/cpu.c#L51
>
> http://git.denx.de/u-boot.git/?p=u-boot.git;a=blob;f=arch/arm/cpu/armv7/mx6/soc.c;h=a8aad5dd0a6c8548277021ebe8f6e159dbf31b9b;hb=HEAD#l42
>
> Is there a reference to the ATAG that I'm not seeing somewhere?

Ok, so 3.0.35 treats cpu_rev correctly and do not assume this info to
be passed from the bootloader. I was confused with 2.6.35, where I had
issues with this on mx53.

So, it seems that nitrogen does not need to pass get_board_rev() at all then?

Regards,

Fabio Estevam

[U-Boot] [PATCH 2/2] mx6qsabrelite: Do not hardcode the CPU revision

[Fabio Estevam]

On Sat, Mar 16, 2013 at 11:52 AM, Otavio Salvador
<otavio@ossystems.com.br> wrote:

> Or add a method to do that? as it'd need to be done in all i.MX6 boards.

Not all boards need to pass their revision to the kernel. For
mx6sabresd/sabreauto we do need it.

I plan to work on it.

Regards,

Fabio Estevam

[U-Boot] [PATCH 1/2] nitrogen6x: Pass the correct CPU revision to the kernel

[Eric Nelson]

On 03/16/2013 07:58 AM, Fabio Estevam wrote:
> Hi Eric,
>
> On Fri, Mar 15, 2013 at 9:20 PM, Eric Nelson
> <eric.nelson@boundarydevices.com> wrote:
>
>> This is the **board** revision, right?
>>
>> At first glance, the kernel seems to be getting the silicon revision
>> from the same place as get_cpu_rev():
>>
>> https://github.com/boundarydevices/linux-imx6/blob/boundary-imx_3.0.35_1.1.1/arch/arm/mach-mx6/cpu.c#L51
>>
>> http://git.denx.de/u-boot.git/?p=u-boot.git;a=blob;f=arch/arm/cpu/armv7/mx6/soc.c;h=a8aad5dd0a6c8548277021ebe8f6e159dbf31b9b;hb=HEAD#l42
>>
>> Is there a reference to the ATAG that I'm not seeing somewhere?
>
> Ok, so 3.0.35 treats cpu_rev correctly and do not assume this info to
> be passed from the bootloader. I was confused with 2.6.35, where I had
> issues with this on mx53.
>
> So, it seems that nitrogen does not need to pass get_board_rev() at all then?
>
At the moment, it doesn't.

I would really like to see us (the i.MX6 community) standardize
the use of some fuses to specifically mean board revision.

We're contemplating some board changes such as switching the
ethernet PHY and having a convention for the use of a few
bits in OTP would allow us to implement get_board_rev() once in
a common place.

Over the lifetime of most boards, it's likely that at least
one board revision will have software implications and having
a common way to express/detect this could prevent some churn
in board-specific files.

Such a convention would need to have broad sign off though.

Let me know your thoughts on the subject.

Regards,


Eric

[U-Boot] [PATCH 2/2] mx6qsabrelite: Do not hardcode the CPU revision

[Otavio Salvador]

On Sat, Mar 16, 2013 at 12:01 PM, Fabio Estevam <festevam@gmail.com> wrote:
> On Sat, Mar 16, 2013 at 11:52 AM, Otavio Salvador
> <otavio@ossystems.com.br> wrote:
>
>> Or add a method to do that? as it'd need to be done in all i.MX6 boards.
>
> Not all boards need to pass their revision to the kernel. For
> mx6sabresd/sabreauto we do need it.

If Dirk is right we need to pass it to make GPU work, no?

-- 
Otavio Salvador                             O.S. Systems
E-mail: otavio at ossystems.com.br  http://www.ossystems.com.br
Mobile: +55 53 9981-7854              http://projetos.ossystems.com.br

[U-Boot] [PATCH 1/2] nitrogen6x: Pass the correct CPU revision to the kernel

[Dirk Behme]

Am 16.03.2013 17:13, schrieb Eric Nelson:
> On 03/16/2013 07:58 AM, Fabio Estevam wrote:
>> Hi Eric,
>>
>> On Fri, Mar 15, 2013 at 9:20 PM, Eric Nelson
>> <eric.nelson@boundarydevices.com> wrote:
>>
>>> This is the **board** revision, right?
>>>
>>> At first glance, the kernel seems to be getting the silicon revision
>>> from the same place as get_cpu_rev():
>>>
>>> https://github.com/boundarydevices/linux-imx6/blob/boundary-imx_3.0.35_1.1.1/arch/arm/mach-mx6/cpu.c#L51
>>>
>>>
>>> http://git.denx.de/u-boot.git/?p=u-boot.git;a=blob;f=arch/arm/cpu/armv7/mx6/soc.c;h=a8aad5dd0a6c8548277021ebe8f6e159dbf31b9b;hb=HEAD#l42
>>>
>>>
>>> Is there a reference to the ATAG that I'm not seeing somewhere?
>>
>> Ok, so 3.0.35 treats cpu_rev correctly and do not assume this info to
>> be passed from the bootloader. I was confused with 2.6.35, where I had
>> issues with this on mx53.
>>
>> So, it seems that nitrogen does not need to pass get_board_rev() at
>> all then?
>>
> At the moment, it doesn't.
>
> I would really like to see us (the i.MX6 community) standardize
> the use of some fuses to specifically mean board revision.
>
> We're contemplating some board changes such as switching the
> ethernet PHY and having a convention for the use of a few
> bits in OTP would allow us to implement get_board_rev() once in
> a common place.
>
> Over the lifetime of most boards, it's likely that at least
> one board revision will have software implications and having
> a common way to express/detect this could prevent some churn
> in board-specific files.
>
> Such a convention would need to have broad sign off though.
>
> Let me know your thoughts on the subject.

I think the OMAP/Beagle community introduced serial EEPROMs to 
identify their (add on) boards.

Best regards

Dirk

[U-Boot] [PATCH 2/2] mx6qsabrelite: Do not hardcode the CPU revision

[Fabio Estevam]

Hi Dirk,

On Sat, Mar 16, 2013 at 2:59 AM, Dirk Behme <dirk.behme@gmail.com> wrote:
> Am 15.03.2013 22:06, schrieb Fabio Estevam:
>
>> From: Fabio Estevam <fabio.estevam@freescale.com>
>>
>> Instead of hardcoding the CPU revision, it is better to use get_cpu_rev().
>
>
> I think to remember that there is a reason why it is hard coded this way.
> Have you tested this with the Vivante GPU driver? If I remember correctly
> they check for exactly the 0x63000 and if it's not there, it won't work (?).

mx6qsabresd/sabreauto do not pass 0x63000 exactly and they are able to
run the GPU driver.

They also encode the board type and revision.

Regards,

Fabio Estevam

[U-Boot] [PATCH 1/2] nitrogen6x: Pass the correct CPU revision to the kernel

[Fabio Estevam]

Hi Eric,

On Sat, Mar 16, 2013 at 1:13 PM, Eric Nelson
<eric.nelson@boundarydevices.com> wrote:

> At the moment, it doesn't.
>
> I would really like to see us (the i.MX6 community) standardize
> the use of some fuses to specifically mean board revision.
>
> We're contemplating some board changes such as switching the
> ethernet PHY and having a convention for the use of a few
> bits in OTP would allow us to implement get_board_rev() once in
> a common place.
>
> Over the lifetime of most boards, it's likely that at least
> one board revision will have software implications and having
> a common way to express/detect this could prevent some churn
> in board-specific files.
>
> Such a convention would need to have broad sign off though.
>
> Let me know your thoughts on the subject.

Would this approach work?

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

Regards,

Fabio Estevam

[U-Boot] [PATCH 1/2] nitrogen6x: Pass the correct CPU revision to the kernel

[Wolfgang Denk]

Dear Eric Nelson,

In message <51449A34.7080902@boundarydevices.com> you wrote:
>
> At the moment, it doesn't.
> 
> I would really like to see us (the i.MX6 community) standardize
> the use of some fuses to specifically mean board revision.

No.  This is a very bad idea.  We've been working long enough with a
number of board manufacturers; many of these provides SOMs (Systems on
Module) that get then used by many different customers for many
different purposes - and the use of things like fuse settings should
be left to these end users.

> We're contemplating some board changes such as switching the
> ethernet PHY and having a convention for the use of a few
> bits in OTP would allow us to implement get_board_rev() once in
> a common place.
> 
> Over the lifetime of most boards, it's likely that at least
> one board revision will have software implications and having
> a common way to express/detect this could prevent some churn
> in board-specific files.
> 
> Such a convention would need to have broad sign off though.

You seem to forget that there is a standardized, well documented way
to pass all kind of hardware related information to the Linux kernel.
If you need any such information, add it to the device tree.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
If you're not part of the solution, you're part of the problem.

[U-Boot] [PATCH 1/2] nitrogen6x: Pass the correct CPU revision to the kernel

[Wolfgang Denk]

Dear Dirk Behme,

In message <5144A401.9020209@gmail.com> you wrote:
>
> I think the OMAP/Beagle community introduced serial EEPROMs to 
> identify their (add on) boards.

There are many such ad-hoc approaches, and most of them are just a
PITA.  If you are trying to optimize boot times, it is really a pain
if you have to wait for inherently slow devives like EEPROMs on a I2C
bus etc.

Also, this addresses only the first half of the problem - the source
of the information. The other half is how to pass the information to
the kernel (-> DT).

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
The only solution is ... a balance of power. We  arm  our  side  with
exactly  that  much  more.  A balance of power -- the trickiest, most
difficult, dirtiest game of them all. But the only one that preserves
both sides.
	-- Kirk, "A Private Little War", stardate 4211.8

[U-Boot] [PATCH 1/2] nitrogen6x: Pass the correct CPU revision to the kernel

[Eric Nelson]

Hi Fabio,

On 03/16/2013 12:48 PM, Fabio Estevam wrote:
> Hi Eric,
>
> On Sat, Mar 16, 2013 at 1:13 PM, Eric Nelson
> <eric.nelson@boundarydevices.com> wrote:
>
>> At the moment, it doesn't.
>>
>> I would really like to see us (the i.MX6 community) standardize
>> the use of some fuses to specifically mean board revision.
>>
>> We're contemplating some board changes such as switching the
>> ethernet PHY and having a convention for the use of a few
>> bits in OTP would allow us to implement get_board_rev() once in
>> a common place.
>>
>> Over the lifetime of most boards, it's likely that at least
>> one board revision will have software implications and having
>> a common way to express/detect this could prevent some churn
>> in board-specific files.
>>
>> Such a convention would need to have broad sign off though.
>>
>> Let me know your thoughts on the subject.
>
> Would this approach work?
>
> 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
>

I think this mixes apples and oranges a bit.

	/* Get Board ID information from OCOTP_GP1[15:8]
	 * bit 12-15: Board type
	 * 0x0 : Unknown
	 * 0x1 : Sabre-AI (ARD)
	 * 0x2 : Smart Device (SD)
	 * 0x3 : Quick-Start Board (QSB)
	 * 0x4 : SoloLite EVK (SL-EVK)
	 *
	 * bit 8-11: Board Revision ID
	 * 0x0 : Unknown or latest revision
	 * 0x1 : RevA board
	 * 0x2 : RevB
	 * 0x3 : RevC
	 *
	 * exp:
	 * i.MX6Q ARD RevA:     0x11
	 * i.MX6Q ARD RevB:     0x12
	 * i.MX6Solo ARD RevA:  0x11
	 * i.MX6Solo ARD RevB:  0x12
	 */

Bits 8-11 seem reasonable, though the comment for zero
is probably bad. It seems that as soon as a board needs
to make a decision based on board revision, it will add
a requirement for a non-zero (programmed) fuse, so the
"latest" will be non-zero by definition.

Four bits also seems like plenty for a revision of a
given board type.

The board type bits above are a bit parochial. You may be able
to list Freescale boards with only four bits, but I would expect the
number of down-stream board types to be in the hundreds, so
it seems this is better left in CONFIG_MACH_TYPE.

The CPU and silicon revision is already available in other
ways and programmed at the Freescale factory.

So for the purposes of get_board_rev(), I think we just
need to identify some bits in an OTP register, define them
as the standard and have get_board_rev() return their value.

That said, I don't think any of this can or should be done
without identifying the down-stream code that might break.

I've seen code that scrapes /proc/cpuinfo for the "Revision:"
line and uses that.

My memory is hazy, but I think it was in either or both the
gstreamer plugins or an "FSL Power Monitor" applet in Android.

I don't recall seeing it in any VPU-related code. Dirk, do you
have a reference there?

I'll try to do some tests of different userspaces with
get_board_rev() returning zero and see what breaks.

Regards,


Eric

[U-Boot] [PATCH 1/2] nitrogen6x: Pass the correct CPU revision to the kernel

[Wolfgang Denk]

Dear Fabio Estevam,

In message <CAOMZO5C0Wi8qf82KDspM0=ZyMn8DBh3b215e-PmWJjDu7=sMUQ@mail.gmail.com> you wrote:
> 
> Would this approach work?
> 
> 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

I bet there is a to of existing ways to encode and pass such
information - in NOR flash, EEPROM, encoded in the serial number
and/or MAC addresses, whatever.  I would expect that each major board
vendor has his preferred style, and I don't see how this could be
changed.

Also, pleas ekeep inmind that there are two sides of the issue:

- storage device and format for the related information

- method and for mat of passing this on to the kernel

The Subject: addresses only the second part of this.  And as mentioned
before, there is a standardized method of passing such infoirmation to
the kernel - through the device tree.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Always leave room to add an explanation if it doesn't work out.

[U-Boot] [PATCH 1/2] nitrogen6x: Pass the correct CPU revision to the kernel

[Eric Nelson]

Hi Fabio,

On 03/16/2013 01:27 PM, Eric Nelson wrote:
> Hi Fabio,
>
> <snip>
 >
> That said, I don't think any of this can or should be done
> without identifying the down-stream code that might break.
>
> I've seen code that scrapes /proc/cpuinfo for the "Revision:"
> line and uses that.
>
> My memory is hazy, but I think it was in either or both the
> gstreamer plugins or an "FSL Power Monitor" applet in Android.
>
> I don't recall seeing it in any VPU-related code. Dirk, do you
> have a reference there?
>
> I'll try to do some tests of different userspaces with
> get_board_rev() returning zero and see what breaks.
>

The Gstreamer VPU plugin breaks on Solo and Quad if get_board_rev()
doesn't return 0x61xxx or 0x63xxx. Oddly, the code in vpu_lib.c
loads firmware based on the 61 or 63 reference and yet still
nominally works on a Solo with a hard-coded value of 0x63000.

The VPU seems fine. It runs Vivante demos on Solo and Quad
with a board rev of zero.

Regards,


Eric

[U-Boot] [PATCH 1/2] nitrogen6x: Pass the correct CPU revision to the kernel

[Fabio Estevam]

Hi Eric,

On Mon, Mar 25, 2013 at 4:14 PM, Eric Nelson
<eric.nelson@boundarydevices.com> wrote:

> The Gstreamer VPU plugin breaks on Solo and Quad if get_board_rev()
> doesn't return 0x61xxx or 0x63xxx. Oddly, the code in vpu_lib.c
> loads firmware based on the 61 or 63 reference and yet still
> nominally works on a Solo with a hard-coded value of 0x63000.

Thanks for testing, I will fix get_cpu_rev() tomorrow.

Regards,

Fabio Estevam

[U-Boot] [PATCH 1/2] nitrogen6x: Pass the correct CPU revision to the kernel

[Fabio Estevam]

On Mon, Mar 25, 2013 at 11:25 PM, Fabio Estevam <festevam@gmail.com> wrote:
> Hi Eric,
>
> On Mon, Mar 25, 2013 at 4:14 PM, Eric Nelson
> <eric.nelson@boundarydevices.com> wrote:
>
>> The Gstreamer VPU plugin breaks on Solo and Quad if get_board_rev()
>> doesn't return 0x61xxx or 0x63xxx. Oddly, the code in vpu_lib.c
>> loads firmware based on the 61 or 63 reference and yet still
>> nominally works on a Solo with a hard-coded value of 0x63000.
>
> Thanks for testing, I will fix get_cpu_rev() tomorrow.

Eric, please try the attached patch if you have a chance.

I don't have my mx6dl or solo to test it, but the attached patch plus
the original one of this thread should make things work.

Thanks,

Fabio Estevam
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-rev.patch
Type: application/octet-stream
Size: 3621 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20130326/19b5bb6b/attachment.obj>

[U-Boot] [PATCH 1/2] nitrogen6x: Pass the correct CPU revision to the kernel

[Eric Nelson]

Hi Fabio,

On 03/15/2013 02:06 PM, Fabio Estevam wrote:
> From: Fabio Estevam <fabio.estevam@freescale.com>
>
> As nitrogen6x boards support different i.MX6 flavors (quad, dual-lite and solo)
> the correct CPU revision needs to passed to the kernel, so call get_cpu_rev()
> instead of hardcoding it.
>
> Freescale 3.0.35 kernel assumes that the CPU revision is passed passed from the
> bootloader.
>
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> ---
>   board/boundary/nitrogen6x/nitrogen6x.c |    2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/board/boundary/nitrogen6x/nitrogen6x.c b/board/boundary/nitrogen6x/nitrogen6x.c
> index 229c237..fec0e3a 100644
> --- a/board/boundary/nitrogen6x/nitrogen6x.c
> +++ b/board/boundary/nitrogen6x/nitrogen6x.c
> @@ -330,7 +330,7 @@ int board_mmc_init(bd_t *bis)
>
>   u32 get_board_rev(void)
>   {
> -	return 0x63000;
> +	return get_cpu_rev();
>   }
>
>   #ifdef CONFIG_MXC_SPI
>

Since this convention is shared among at least SABRE Lite, SABRE SD,
Nitrogen6x and Wandboard, wouldn't a weak function in imx-common/cpu.c
be a better choice?

+#ifdef CONFIG_REVISION_TAG
+u32 __weak get_board_rev(void)
+{
+       return get_cpu_rev();
+}
+#endif

Then boards could override things, but will do the reasonable thing
without the extra code.

[U-Boot] [PATCH 1/2] nitrogen6x: Pass the correct CPU revision to the kernel

[Eric Nelson]

On 03/26/2013 08:24 AM, Eric Nelson wrote:
> Hi Fabio,
>
 > <snip>
 >
>> --- a/board/boundary/nitrogen6x/nitrogen6x.c
>> +++ b/board/boundary/nitrogen6x/nitrogen6x.c
>> @@ -330,7 +330,7 @@ int board_mmc_init(bd_t *bis)
>>
>>   u32 get_board_rev(void)
>>   {
>> -    return 0x63000;
>> +    return get_cpu_rev();
>>   }
>>
>>   #ifdef CONFIG_MXC_SPI
>>
>
> Since this convention is shared among at least SABRE Lite, SABRE SD,
> Nitrogen6x and Wandboard, wouldn't a weak function in imx-common/cpu.c
> be a better choice?
>

Oops.

I meant to say arch/arm/cpu/armv7/mx6/soc.c, not imx-common/cpu.c.

[U-Boot] [PATCH 1/2] nitrogen6x: Pass the correct CPU revision to the kernel

[Fabio Estevam]

Hi Eric,

On Tue, Mar 26, 2013 at 12:24 PM, Eric Nelson
<eric.nelson@boundarydevices.com> wrote:

> Since this convention is shared among at least SABRE Lite, SABRE SD,
> Nitrogen6x and Wandboard, wouldn't a weak function in imx-common/cpu.c
> be a better choice?
>
> +#ifdef CONFIG_REVISION_TAG
> +u32 __weak get_board_rev(void)
> +{
> +       return get_cpu_rev();
> +}
> +#endif
>
> Then boards could override things, but will do the reasonable thing
> without the extra code.

I like this idea. Will submit this change with the get_cpu_rev()
tomorrow, after I get more comments on the proposed get_cpu_rev patch.

Thanks,

Fabio Estevam