* [U-Boot] [PATCH v2] mx6: Fix get_board_rev() for the mx6 solo case
2013-03-27 17:36 [U-Boot] [PATCH v2] mx6: Fix get_board_rev() for the mx6 solo case Fabio Estevam
@ 2013-03-27 18:56 ` Dirk Behme
2013-03-27 19:36 ` Fabio Estevam
2013-03-28 7:04 ` Dirk Behme
` (2 subsequent siblings)
3 siblings, 1 reply; 8+ messages in thread
From: Dirk Behme @ 2013-03-27 18:56 UTC (permalink / raw)
To: u-boot
Hi Fabio,
On 27.03.2013 18:36, Fabio Estevam wrote:
> When booting a Freescale kernel 3.0.35 on a Wandboard solo, the get_board_rev()
> returns 0x62xxx, which is not a value understood by the VPU
> (Video Processing Unit) library in the kernel and causes the video playback to
> fail.
>
> The expected values for get_board_rev are:
> 0x63xxx: For mx6quad/dual
> 0x61xxx: For mx6dual-lite/solo
>
> So adjust get_board_rev() accordingly and make it as weak function, so that we
> do not need to define it in every mx6 board file.
>
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> ---
> Changes since v1:
> - Avoid extra call to get_cpu_rev()
>
> arch/arm/cpu/armv7/mx6/soc.c | 12 ++++++++++++
> board/boundary/nitrogen6x/nitrogen6x.c | 5 -----
> board/freescale/mx6qsabrelite/mx6qsabrelite.c | 5 -----
> board/freescale/mx6qsabresd/mx6qsabresd.c | 5 -----
> board/wandboard/wandboard.c | 5 -----
> 5 files changed, 12 insertions(+), 20 deletions(-)
>
> diff --git a/arch/arm/cpu/armv7/mx6/soc.c b/arch/arm/cpu/armv7/mx6/soc.c
> index 193ba12..4f3cd14 100644
> --- a/arch/arm/cpu/armv7/mx6/soc.c
> +++ b/arch/arm/cpu/armv7/mx6/soc.c
> @@ -61,6 +61,18 @@ u32 get_cpu_rev(void)
> return (type << 12) | (reg + 0x10);
> }
>
> +#ifdef CONFIG_REVISION_TAG
> +u32 __weak get_board_rev(void)
> +{
> + u32 cpurev = get_cpu_rev();
> + u32 type = ((cpurev >> 12) & 0xff);
> + if (type == MXC_CPU_MX6SOLO)
> + cpurev = (MXC_CPU_MX6DL) << 12 | (cpurev & 0xFFF);
> +
> + return cpurev;
> +}
> +#endif
This is much better than changing Troy's get_cpu_rev(). Thanks :)
But what's about to add this code to
http://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git/tree/arch/arm/kernel/setup.c?h=imx_3.0.35_1.1.0#n655
?
Best regards
Dirk
^ permalink raw reply [flat|nested] 8+ messages in thread* [U-Boot] [PATCH v2] mx6: Fix get_board_rev() for the mx6 solo case
2013-03-27 18:56 ` Dirk Behme
@ 2013-03-27 19:36 ` Fabio Estevam
2013-03-27 19:47 ` Dirk Behme
0 siblings, 1 reply; 8+ messages in thread
From: Fabio Estevam @ 2013-03-27 19:36 UTC (permalink / raw)
To: u-boot
Hi Dirk,
On Wed, Mar 27, 2013 at 3:56 PM, Dirk Behme <dirk.behme@gmail.com> wrote:
> This is much better than changing Troy's get_cpu_rev(). Thanks :)
I am glad you like it :-)
>
> But what's about to add this code to
>
> http://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git/tree/arch/arm/kernel/setup.c?h=imx_3.0.35_1.1.0#n655
Because I cannot post a patch in the U-boot list to fix an internal
FSL kernel :-)
I can certainly suggest this (and yes, I fully agree that the kernel
should handle the CPU detection without relying at the bootloader at
all), but from a U-boot perspective we cannot really know which kernel
version the user will deploy.
If we want mainline U-boot to work with this particular kernel version
on mx6 solo, then we need to adjust ATAGS as proposed by my patch.
Regards,
Fabio Estevam
^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH v2] mx6: Fix get_board_rev() for the mx6 solo case
2013-03-27 19:36 ` Fabio Estevam
@ 2013-03-27 19:47 ` Dirk Behme
2013-03-27 20:02 ` Eric Nelson
0 siblings, 1 reply; 8+ messages in thread
From: Dirk Behme @ 2013-03-27 19:47 UTC (permalink / raw)
To: u-boot
Am 27.03.2013 20:36, schrieb Fabio Estevam:
> Hi Dirk,
>
> On Wed, Mar 27, 2013 at 3:56 PM, Dirk Behme <dirk.behme@gmail.com> wrote:
>
>> This is much better than changing Troy's get_cpu_rev(). Thanks :)
>
> I am glad you like it :-)
>
>>
>> But what's about to add this code to
>>
>> http://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git/tree/arch/arm/kernel/setup.c?h=imx_3.0.35_1.1.0#n655
>
> Because I cannot post a patch in the U-boot list to fix an internal
> FSL kernel :-)
>
> I can certainly suggest this (and yes, I fully agree that the kernel
> should handle the CPU detection without relying at the bootloader at
> all), but from a U-boot perspective we cannot really know which kernel
> version the user will deploy.
>
> If we want mainline U-boot to work with this particular kernel version
> on mx6 solo, then we need to adjust ATAGS as proposed by my patch.
Ok, sounds reasonable, agreed.
Even if someone might complain that this is off-topic here, again ( ;)
): Do you like to check if the recent _mainline_ kernel wants
something similar?
Best regards
Dirk
^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH v2] mx6: Fix get_board_rev() for the mx6 solo case
2013-03-27 19:47 ` Dirk Behme
@ 2013-03-27 20:02 ` Eric Nelson
0 siblings, 0 replies; 8+ messages in thread
From: Eric Nelson @ 2013-03-27 20:02 UTC (permalink / raw)
To: u-boot
Hi Dirk,
On 03/27/2013 12:47 PM, Dirk Behme wrote:
> Am 27.03.2013 20:36, schrieb Fabio Estevam:
>> Hi Dirk,
>>
>> On Wed, Mar 27, 2013 at 3:56 PM, Dirk Behme <dirk.behme@gmail.com> wrote:
>>
>>> This is much better than changing Troy's get_cpu_rev(). Thanks :)
>>
>> I am glad you like it :-)
>>
>>>
>>> But what's about to add this code to
>>>
>>> http://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git/tree/arch/arm/kernel/setup.c?h=imx_3.0.35_1.1.0#n655
>>>
>>
>> Because I cannot post a patch in the U-boot list to fix an internal
>> FSL kernel :-)
>>
>> I can certainly suggest this (and yes, I fully agree that the kernel
>> should handle the CPU detection without relying at the bootloader at
>> all), but from a U-boot perspective we cannot really know which kernel
>> version the user will deploy.
>>
>> If we want mainline U-boot to work with this particular kernel version
>> on mx6 solo, then we need to adjust ATAGS as proposed by my patch.
>
> Ok, sounds reasonable, agreed.
>
> Even if someone might complain that this is off-topic here, again ( ;)
> ): Do you like to check if the recent _mainline_ kernel wants something
> similar?
>
It wasn't a complaint from me, but I can assure you that the
offending bit of code that I found (in imx-lib) won't function
against a main-line kernel without some serious re-work.
Regards,
Eric
^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH v2] mx6: Fix get_board_rev() for the mx6 solo case
2013-03-27 17:36 [U-Boot] [PATCH v2] mx6: Fix get_board_rev() for the mx6 solo case Fabio Estevam
2013-03-27 18:56 ` Dirk Behme
@ 2013-03-28 7:04 ` Dirk Behme
2013-03-28 13:58 ` Eric Nelson
2013-04-03 9:36 ` Stefano Babic
3 siblings, 0 replies; 8+ messages in thread
From: Dirk Behme @ 2013-03-28 7:04 UTC (permalink / raw)
To: u-boot
On 27.03.2013 18:36, Fabio Estevam wrote:
> When booting a Freescale kernel 3.0.35 on a Wandboard solo, the get_board_rev()
> returns 0x62xxx, which is not a value understood by the VPU
> (Video Processing Unit) library in the kernel and causes the video playback to
> fail.
>
> The expected values for get_board_rev are:
> 0x63xxx: For mx6quad/dual
> 0x61xxx: For mx6dual-lite/solo
>
> So adjust get_board_rev() accordingly and make it as weak function, so that we
> do not need to define it in every mx6 board file.
>
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
Acked-by: Dirk Behme <dirk.behme@de.bosch.com>
Thanks
Dirk
> ---
> Changes since v1:
> - Avoid extra call to get_cpu_rev()
>
> arch/arm/cpu/armv7/mx6/soc.c | 12 ++++++++++++
> board/boundary/nitrogen6x/nitrogen6x.c | 5 -----
> board/freescale/mx6qsabrelite/mx6qsabrelite.c | 5 -----
> board/freescale/mx6qsabresd/mx6qsabresd.c | 5 -----
> board/wandboard/wandboard.c | 5 -----
> 5 files changed, 12 insertions(+), 20 deletions(-)
>
> diff --git a/arch/arm/cpu/armv7/mx6/soc.c b/arch/arm/cpu/armv7/mx6/soc.c
> index 193ba12..4f3cd14 100644
> --- a/arch/arm/cpu/armv7/mx6/soc.c
> +++ b/arch/arm/cpu/armv7/mx6/soc.c
> @@ -61,6 +61,18 @@ u32 get_cpu_rev(void)
> return (type << 12) | (reg + 0x10);
> }
>
> +#ifdef CONFIG_REVISION_TAG
> +u32 __weak get_board_rev(void)
> +{
> + u32 cpurev = get_cpu_rev();
> + u32 type = ((cpurev >> 12) & 0xff);
> + if (type == MXC_CPU_MX6SOLO)
> + cpurev = (MXC_CPU_MX6DL) << 12 | (cpurev & 0xFFF);
> +
> + return cpurev;
> +}
> +#endif
> +
> void init_aips(void)
> {
> struct aipstz_regs *aips1, *aips2;
> diff --git a/board/boundary/nitrogen6x/nitrogen6x.c b/board/boundary/nitrogen6x/nitrogen6x.c
> index 229c237..1b1bedf 100644
> --- a/board/boundary/nitrogen6x/nitrogen6x.c
> +++ b/board/boundary/nitrogen6x/nitrogen6x.c
> @@ -328,11 +328,6 @@ int board_mmc_init(bd_t *bis)
> }
> #endif
>
> -u32 get_board_rev(void)
> -{
> - return 0x63000;
> -}
> -
> #ifdef CONFIG_MXC_SPI
> iomux_v3_cfg_t const ecspi1_pads[] = {
> /* SS1 */
> diff --git a/board/freescale/mx6qsabrelite/mx6qsabrelite.c b/board/freescale/mx6qsabrelite/mx6qsabrelite.c
> index 5b69a6d..782e5ba 100644
> --- a/board/freescale/mx6qsabrelite/mx6qsabrelite.c
> +++ b/board/freescale/mx6qsabrelite/mx6qsabrelite.c
> @@ -298,11 +298,6 @@ int board_mmc_init(bd_t *bis)
> }
> #endif
>
> -u32 get_board_rev(void)
> -{
> - return 0x63000 ;
> -}
> -
> #ifdef CONFIG_MXC_SPI
> iomux_v3_cfg_t const ecspi1_pads[] = {
> /* SS1 */
> diff --git a/board/freescale/mx6qsabresd/mx6qsabresd.c b/board/freescale/mx6qsabresd/mx6qsabresd.c
> index 2b3926a..806769f 100644
> --- a/board/freescale/mx6qsabresd/mx6qsabresd.c
> +++ b/board/freescale/mx6qsabresd/mx6qsabresd.c
> @@ -239,11 +239,6 @@ int board_eth_init(bd_t *bis)
> return 0;
> }
>
> -u32 get_board_rev(void)
> -{
> - return 0x63000;
> -}
> -
> int board_early_init_f(void)
> {
> setup_iomux_uart();
> diff --git a/board/wandboard/wandboard.c b/board/wandboard/wandboard.c
> index d95189f..1e379fb 100644
> --- a/board/wandboard/wandboard.c
> +++ b/board/wandboard/wandboard.c
> @@ -168,11 +168,6 @@ int board_init(void)
> return 0;
> }
>
> -u32 get_board_rev(void)
> -{
> - return get_cpu_rev();
> -}
> -
> int checkboard(void)
> {
> puts("Board: Wandboard\n");
^ permalink raw reply [flat|nested] 8+ messages in thread* [U-Boot] [PATCH v2] mx6: Fix get_board_rev() for the mx6 solo case
2013-03-27 17:36 [U-Boot] [PATCH v2] mx6: Fix get_board_rev() for the mx6 solo case Fabio Estevam
2013-03-27 18:56 ` Dirk Behme
2013-03-28 7:04 ` Dirk Behme
@ 2013-03-28 13:58 ` Eric Nelson
2013-04-03 9:36 ` Stefano Babic
3 siblings, 0 replies; 8+ messages in thread
From: Eric Nelson @ 2013-03-28 13:58 UTC (permalink / raw)
To: u-boot
On 03/27/2013 10:36 AM, Fabio Estevam wrote:
> When booting a Freescale kernel 3.0.35 on a Wandboard solo, the get_board_rev()
> returns 0x62xxx, which is not a value understood by the VPU
> (Video Processing Unit) library in the kernel and causes the video playback to
> fail.
>
> The expected values for get_board_rev are:
> 0x63xxx: For mx6quad/dual
> 0x61xxx: For mx6dual-lite/solo
>
> So adjust get_board_rev() accordingly and make it as weak function, so that we
> do not need to define it in every mx6 board file.
>
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> ---
> Changes since v1:
> - Avoid extra call to get_cpu_rev()
>
> arch/arm/cpu/armv7/mx6/soc.c | 12 ++++++++++++
> board/boundary/nitrogen6x/nitrogen6x.c | 5 -----
> board/freescale/mx6qsabrelite/mx6qsabrelite.c | 5 -----
> board/freescale/mx6qsabresd/mx6qsabresd.c | 5 -----
> board/wandboard/wandboard.c | 5 -----
> 5 files changed, 12 insertions(+), 20 deletions(-)
>
Acked-by: Eric Nelson <eric.nelson@boundarydevices.com>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH v2] mx6: Fix get_board_rev() for the mx6 solo case
2013-03-27 17:36 [U-Boot] [PATCH v2] mx6: Fix get_board_rev() for the mx6 solo case Fabio Estevam
` (2 preceding siblings ...)
2013-03-28 13:58 ` Eric Nelson
@ 2013-04-03 9:36 ` Stefano Babic
3 siblings, 0 replies; 8+ messages in thread
From: Stefano Babic @ 2013-04-03 9:36 UTC (permalink / raw)
To: u-boot
On 27/03/2013 18:36, Fabio Estevam wrote:
> When booting a Freescale kernel 3.0.35 on a Wandboard solo, the get_board_rev()
> returns 0x62xxx, which is not a value understood by the VPU
> (Video Processing Unit) library in the kernel and causes the video playback to
> fail.
>
> The expected values for get_board_rev are:
> 0x63xxx: For mx6quad/dual
> 0x61xxx: For mx6dual-lite/solo
>
> So adjust get_board_rev() accordingly and make it as weak function, so that we
> do not need to define it in every mx6 board file.
>
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> ---
Hi Fabio,
> Changes since v1:
> - Avoid extra call to get_cpu_rev()
>
> arch/arm/cpu/armv7/mx6/soc.c | 12 ++++++++++++
> board/boundary/nitrogen6x/nitrogen6x.c | 5 -----
> board/freescale/mx6qsabrelite/mx6qsabrelite.c | 5 -----
> board/freescale/mx6qsabresd/mx6qsabresd.c | 5 -----
> board/wandboard/wandboard.c | 5 -----
> 5 files changed, 12 insertions(+), 20 deletions(-)
>
> diff --git a/arch/arm/cpu/armv7/mx6/soc.c b/arch/arm/cpu/armv7/mx6/soc.c
> index 193ba12..4f3cd14 100644
> --- a/arch/arm/cpu/armv7/mx6/soc.c
> +++ b/arch/arm/cpu/armv7/mx6/soc.c
> @@ -61,6 +61,18 @@ u32 get_cpu_rev(void)
> return (type << 12) | (reg + 0x10);
> }
>
> +#ifdef CONFIG_REVISION_TAG
> +u32 __weak get_board_rev(void)
> +{
> + u32 cpurev = get_cpu_rev();
> + u32 type = ((cpurev >> 12) & 0xff);
> + if (type == MXC_CPU_MX6SOLO)
> + cpurev = (MXC_CPU_MX6DL) << 12 | (cpurev & 0xFFF);
> +
> + return cpurev;
> +}
> +#endif
> +
We use a cpu revision as board revision, and this seems quite perverse.
Anyway, I understand the reasons, and this seems a good way to work
around bugs in other (FSL) code without breaking U-Boot.
Applied to u-boot-imx, thanks.
Best regards,
Stefano Babic
--
=====================================================================
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================
^ permalink raw reply [flat|nested] 8+ messages in thread