| Message ID | 1364405816-14085-1-git-send-email-fabio.estevam@freescale.com |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Stefano Babic |
| Headers | show |
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
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
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
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
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");
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>
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
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");
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(-)