| Message ID | 1330728909-12203-2-git-send-email-eric.nelson@boundarydevices.com |
|---|---|
| State | Superseded |
| Headers | show |
> Freescale 2.6.38 (Non-DT) kernels require the revision atag to > enable the VPU. > > Signed-off-by: Eric Nelson <eric.nelson@boundarydevices.com> > --- > board/freescale/mx6qsabrelite/mx6qsabrelite.c | 7 +++++++ > include/configs/mx6qsabrelite.h | 1 + > 2 files changed, 8 insertions(+), 0 deletions(-) > > diff --git a/board/freescale/mx6qsabrelite/mx6qsabrelite.c > b/board/freescale/mx6qsabrelite/mx6qsabrelite.c index db1bea9..590030b > 100644 > --- a/board/freescale/mx6qsabrelite/mx6qsabrelite.c > +++ b/board/freescale/mx6qsabrelite/mx6qsabrelite.c > @@ -215,6 +215,13 @@ int board_mmc_init(bd_t *bis) > } > #endif > > +#ifdef CONFIG_REVISION_TAG > +u32 get_board_rev(void) > +{ > + return 0x63000 ; > +} > +#endif > + > #ifdef CONFIG_MXC_SPI > iomux_v3_cfg_t ecspi1_pads[] = { > /* SS1 */ > diff --git a/include/configs/mx6qsabrelite.h > b/include/configs/mx6qsabrelite.h index 93000f0..85f6f7a 100644 > --- a/include/configs/mx6qsabrelite.h > +++ b/include/configs/mx6qsabrelite.h > @@ -33,6 +33,7 @@ > #define CONFIG_CMDLINE_TAG > #define CONFIG_SETUP_MEMORY_TAGS > #define CONFIG_INITRD_TAG > +#define CONFIG_REVISION_TAG I think you can avoid this define altogether, you're using it anyway. M
On 02/03/2012 23:55, Eric Nelson wrote: > Freescale 2.6.38 (Non-DT) kernels require the revision atag to > enable the VPU. > > Signed-off-by: Eric Nelson <eric.nelson@boundarydevices.com> > --- > board/freescale/mx6qsabrelite/mx6qsabrelite.c | 7 +++++++ > include/configs/mx6qsabrelite.h | 1 + > 2 files changed, 8 insertions(+), 0 deletions(-) > > diff --git a/board/freescale/mx6qsabrelite/mx6qsabrelite.c b/board/freescale/mx6qsabrelite/mx6qsabrelite.c > index db1bea9..590030b 100644 > --- a/board/freescale/mx6qsabrelite/mx6qsabrelite.c > +++ b/board/freescale/mx6qsabrelite/mx6qsabrelite.c > @@ -215,6 +215,13 @@ int board_mmc_init(bd_t *bis) > } > #endif > > +#ifdef CONFIG_REVISION_TAG You add CONFIG_REVISION_TAG, then you can get rid of unneeded #ifdef Best regards, Stefano Babic
On 03/02/2012 03:59 PM, Marek Vasut wrote: >> Freescale 2.6.38 (Non-DT) kernels require the revision atag to >> enable the VPU. >> >> Signed-off-by: Eric Nelson<eric.nelson@boundarydevices.com> >> --- >> board/freescale/mx6qsabrelite/mx6qsabrelite.c | 7 +++++++ >> include/configs/mx6qsabrelite.h | 1 + >> 2 files changed, 8 insertions(+), 0 deletions(-) >> >> diff --git a/board/freescale/mx6qsabrelite/mx6qsabrelite.c >> b/board/freescale/mx6qsabrelite/mx6qsabrelite.c index db1bea9..590030b >> 100644 >> --- a/board/freescale/mx6qsabrelite/mx6qsabrelite.c >> +++ b/board/freescale/mx6qsabrelite/mx6qsabrelite.c >> @@ -215,6 +215,13 @@ int board_mmc_init(bd_t *bis) >> } >> #endif >> >> +#ifdef CONFIG_REVISION_TAG >> +u32 get_board_rev(void) >> +{ >> + return 0x63000 ; >> +} >> +#endif >> + >> #ifdef CONFIG_MXC_SPI >> iomux_v3_cfg_t ecspi1_pads[] = { >> /* SS1 */ >> diff --git a/include/configs/mx6qsabrelite.h >> b/include/configs/mx6qsabrelite.h index 93000f0..85f6f7a 100644 >> --- a/include/configs/mx6qsabrelite.h >> +++ b/include/configs/mx6qsabrelite.h >> @@ -33,6 +33,7 @@ >> #define CONFIG_CMDLINE_TAG >> #define CONFIG_SETUP_MEMORY_TAGS >> #define CONFIG_INITRD_TAG >> +#define CONFIG_REVISION_TAG > > I think you can avoid this define altogether, you're using it anyway. > Hi Marek, I'm not sure I understand. I need to define CONFIG_REVISION_TAG in order to get bootm to add the tag: http://git.denx.de/?p=u-boot.git;a=blob;f=arch/arm/lib/bootm.c;h=afa0093df7620606ee12b4dc1dd2ee37adee347c;hb=master#l137 I made get_board_rev() conditional so we can get rid of it if (once) we're only using DT kernels.
> On 03/02/2012 03:59 PM, Marek Vasut wrote: > >> Freescale 2.6.38 (Non-DT) kernels require the revision atag to > >> enable the VPU. > >> > >> Signed-off-by: Eric Nelson<eric.nelson@boundarydevices.com> > >> --- > >> > >> board/freescale/mx6qsabrelite/mx6qsabrelite.c | 7 +++++++ > >> include/configs/mx6qsabrelite.h | 1 + > >> 2 files changed, 8 insertions(+), 0 deletions(-) > >> > >> diff --git a/board/freescale/mx6qsabrelite/mx6qsabrelite.c > >> b/board/freescale/mx6qsabrelite/mx6qsabrelite.c index db1bea9..590030b > >> 100644 > >> --- a/board/freescale/mx6qsabrelite/mx6qsabrelite.c > >> +++ b/board/freescale/mx6qsabrelite/mx6qsabrelite.c > >> @@ -215,6 +215,13 @@ int board_mmc_init(bd_t *bis) > >> > >> } > >> #endif > >> > >> +#ifdef CONFIG_REVISION_TAG > >> +u32 get_board_rev(void) > >> +{ > >> + return 0x63000 ; > >> +} > >> +#endif > >> + > >> > >> #ifdef CONFIG_MXC_SPI > >> iomux_v3_cfg_t ecspi1_pads[] = { > >> > >> /* SS1 */ > >> > >> diff --git a/include/configs/mx6qsabrelite.h > >> b/include/configs/mx6qsabrelite.h index 93000f0..85f6f7a 100644 > >> --- a/include/configs/mx6qsabrelite.h > >> +++ b/include/configs/mx6qsabrelite.h > >> @@ -33,6 +33,7 @@ > >> > >> #define CONFIG_CMDLINE_TAG > >> #define CONFIG_SETUP_MEMORY_TAGS > >> #define CONFIG_INITRD_TAG > >> > >> +#define CONFIG_REVISION_TAG > > > > I think you can avoid this define altogether, you're using it anyway. > > Hi Marek, > > I'm not sure I understand. > > I need to define CONFIG_REVISION_TAG in order to get bootm to add > the tag: > http://git.denx.de/?p=u- boot.git;a=blob;f=arch/arm/lib/bootm.c;h=afa0093df > 7620606ee12b4dc1dd2ee37adee347c;hb=master#l137 > > I made get_board_rev() conditional so we can get rid of it if (once) > we're only using DT kernels. You have CONFIG_REVISION_TAG set unconditionally, so you don't need the macro around that get_revision() function. I might be wrong. M
On 03/04/2012 01:59 PM, Marek Vasut wrote: >> On 03/02/2012 03:59 PM, Marek Vasut wrote: >>>> Freescale 2.6.38 (Non-DT) kernels require the revision atag to >>>> enable the VPU. >>>> >>>> Signed-off-by: Eric Nelson<eric.nelson@boundarydevices.com> >>>> --- >>>> >>>> board/freescale/mx6qsabrelite/mx6qsabrelite.c | 7 +++++++ >>>> include/configs/mx6qsabrelite.h | 1 + >>>> 2 files changed, 8 insertions(+), 0 deletions(-) >>>> >>>> diff --git a/board/freescale/mx6qsabrelite/mx6qsabrelite.c >>>> b/board/freescale/mx6qsabrelite/mx6qsabrelite.c index db1bea9..590030b >>>> 100644 >>>> --- a/board/freescale/mx6qsabrelite/mx6qsabrelite.c >>>> +++ b/board/freescale/mx6qsabrelite/mx6qsabrelite.c >>>> @@ -215,6 +215,13 @@ int board_mmc_init(bd_t *bis) >>>> >>>> } >>>> #endif >>>> >>>> +#ifdef CONFIG_REVISION_TAG >>>> +u32 get_board_rev(void) >>>> +{ >>>> + return 0x63000 ; >>>> +} >>>> +#endif >>>> + >>>> >>>> #ifdef CONFIG_MXC_SPI >>>> iomux_v3_cfg_t ecspi1_pads[] = { >>>> >>>> /* SS1 */ >>>> >>>> diff --git a/include/configs/mx6qsabrelite.h >>>> b/include/configs/mx6qsabrelite.h index 93000f0..85f6f7a 100644 >>>> --- a/include/configs/mx6qsabrelite.h >>>> +++ b/include/configs/mx6qsabrelite.h >>>> @@ -33,6 +33,7 @@ >>>> >>>> #define CONFIG_CMDLINE_TAG >>>> #define CONFIG_SETUP_MEMORY_TAGS >>>> #define CONFIG_INITRD_TAG >>>> >>>> +#define CONFIG_REVISION_TAG >>> >>> I think you can avoid this define altogether, you're using it anyway. >> >> Hi Marek, >> >> I'm not sure I understand. >> >> I need to define CONFIG_REVISION_TAG in order to get bootm to add >> the tag: >> http://git.denx.de/?p=u- > boot.git;a=blob;f=arch/arm/lib/bootm.c;h=afa0093df >> 7620606ee12b4dc1dd2ee37adee347c;hb=master#l137 >> >> I made get_board_rev() conditional so we can get rid of it if (once) >> we're only using DT kernels. > > You have CONFIG_REVISION_TAG set unconditionally, so you don't need the macro > around that get_revision() function. I might be wrong. > You're right. Its just that I did that deliberately so you can save the (admittedly) small amount of code by editing mx6qsabrelite.h. Since U-Boot doesn't really have a configuration step, I find that folks tend to tweak the board configuration file quite a bit based on their needs. Though it's a bit of a hack, we even formalized it on our i.MX5x products to allow customers to keep their git repositories clean: http://boundarydevices.com/blogs/customizing-u-boot-on-i-mx
> On 03/04/2012 01:59 PM, Marek Vasut wrote: > >> On 03/02/2012 03:59 PM, Marek Vasut wrote: > >>>> Freescale 2.6.38 (Non-DT) kernels require the revision atag to > >>>> enable the VPU. > >>>> > >>>> Signed-off-by: Eric Nelson<eric.nelson@boundarydevices.com> > >>>> --- > >>>> > >>>> board/freescale/mx6qsabrelite/mx6qsabrelite.c | 7 +++++++ > >>>> include/configs/mx6qsabrelite.h | 1 + > >>>> 2 files changed, 8 insertions(+), 0 deletions(-) > >>>> > >>>> diff --git a/board/freescale/mx6qsabrelite/mx6qsabrelite.c > >>>> b/board/freescale/mx6qsabrelite/mx6qsabrelite.c index db1bea9..590030b > >>>> 100644 > >>>> --- a/board/freescale/mx6qsabrelite/mx6qsabrelite.c > >>>> +++ b/board/freescale/mx6qsabrelite/mx6qsabrelite.c > >>>> @@ -215,6 +215,13 @@ int board_mmc_init(bd_t *bis) > >>>> > >>>> } > >>>> #endif > >>>> > >>>> +#ifdef CONFIG_REVISION_TAG > >>>> +u32 get_board_rev(void) > >>>> +{ > >>>> + return 0x63000 ; > >>>> +} > >>>> +#endif > >>>> + > >>>> > >>>> #ifdef CONFIG_MXC_SPI > >>>> iomux_v3_cfg_t ecspi1_pads[] = { > >>>> > >>>> /* SS1 */ > >>>> > >>>> diff --git a/include/configs/mx6qsabrelite.h > >>>> b/include/configs/mx6qsabrelite.h index 93000f0..85f6f7a 100644 > >>>> --- a/include/configs/mx6qsabrelite.h > >>>> +++ b/include/configs/mx6qsabrelite.h > >>>> @@ -33,6 +33,7 @@ > >>>> > >>>> #define CONFIG_CMDLINE_TAG > >>>> #define CONFIG_SETUP_MEMORY_TAGS > >>>> #define CONFIG_INITRD_TAG > >>>> > >>>> +#define CONFIG_REVISION_TAG > >>> > >>> I think you can avoid this define altogether, you're using it anyway. > >> > >> Hi Marek, > >> > >> I'm not sure I understand. > >> > >> I need to define CONFIG_REVISION_TAG in order to get bootm to add > >> > >> the tag: > >> http://git.denx.de/?p=u- > > > > boot.git;a=blob;f=arch/arm/lib/bootm.c;h=afa0093df > > > >> 7620606ee12b4dc1dd2ee37adee347c;hb=master#l137 > >> > >> I made get_board_rev() conditional so we can get rid of it if (once) > >> we're only using DT kernels. > > > > You have CONFIG_REVISION_TAG set unconditionally, so you don't need the > > macro around that get_revision() function. I might be wrong. > > You're right. Its just that I did that deliberately so you can save the > (admittedly) small amount of code by editing mx6qsabrelite.h. Shouldn't that be removed by the compiler anyway? > > Since U-Boot doesn't really have a configuration step, I find that > folks tend to tweak the board configuration file quite a bit based > on their needs. This needs to be fixed, we need kbuild. Who's up to doing it ? ;-D > > Though it's a bit of a hack, we even formalized it on our i.MX5x products > to allow customers to keep their git repositories clean: > http://boundarydevices.com/blogs/customizing-u-boot-on-i-mx That's good, thank you :) Cheers! M
Dear Eric Nelson, In message <4F53D8E6.4000408@boundarydevices.com> you wrote: > > Since U-Boot doesn't really have a configuration step, I find that > folks tend to tweak the board configuration file quite a bit based > on their needs. Yes, peaople do that a lot, nd it has always been a very good idea to at least detect such modifications of the code. The worst thing to happen is when you receive a bug report for version FOO, and you have no chance of detecting that somebody actually messed with the configuration and/or the code. > Though it's a bit of a hack, we even formalized it on our i.MX5x products > to allow customers to keep their git repositories clean: > .... [Link deleted to not further distribute this horrible stuff.] Frankly, this "keep their git repositories clean" is a really, really bad idea. We pay special care to add the git commit ID to the U-Boot version string so you can easily detect if somebody messed with the code, and you come up and invent methods of circumventing that. This is totally counterproductive. If you use this, that's your business. But please do me the favour and do not recommend this to others. Thanks. Wolfgang Denk
On 03/04/2012 03:06 PM, Wolfgang Denk wrote: > Dear Eric Nelson, > > In message<4F53D8E6.4000408@boundarydevices.com> you wrote: >> >> Since U-Boot doesn't really have a configuration step, I find that >> folks tend to tweak the board configuration file quite a bit based >> on their needs. > > Yes, peaople do that a lot, nd it has always been a very good idea to > at least detect such modifications of the code. The worst thing to > happen is when you receive a bug report for version FOO, and you have > no chance of detecting that somebody actually messed with the > configuration and/or the code. > >> Though it's a bit of a hack, we even formalized it on our i.MX5x products >> to allow customers to keep their git repositories clean: >> .... > [Link deleted to not further distribute this horrible stuff.] > > Frankly, this "keep their git repositories clean" is a really, really > bad idea. We pay special care to add the git commit ID to the U-Boot > version string so you can easily detect if somebody messed with the > code, and you come up and invent methods of circumventing that. > > This is totally counterproductive. > > If you use this, that's your business. But please do me the favour and > do not recommend this to others. > We don't. Sorry if I implied otherwise. We recommend that customers keep their own repositories with changes from our "standard" releases, but we have a number of customers whose complete set of differences amounts to default environment variables and/or choices of serial consoles for which we don't want to carry and publish an entire configuration. This hack was borne out of frustration when getting a 'revision-dirty' reference back from a customer because they've changed something simple (usually environment settings).
diff --git a/board/freescale/mx6qsabrelite/mx6qsabrelite.c b/board/freescale/mx6qsabrelite/mx6qsabrelite.c index db1bea9..590030b 100644 --- a/board/freescale/mx6qsabrelite/mx6qsabrelite.c +++ b/board/freescale/mx6qsabrelite/mx6qsabrelite.c @@ -215,6 +215,13 @@ int board_mmc_init(bd_t *bis) } #endif +#ifdef CONFIG_REVISION_TAG +u32 get_board_rev(void) +{ + return 0x63000 ; +} +#endif + #ifdef CONFIG_MXC_SPI iomux_v3_cfg_t ecspi1_pads[] = { /* SS1 */ diff --git a/include/configs/mx6qsabrelite.h b/include/configs/mx6qsabrelite.h index 93000f0..85f6f7a 100644 --- a/include/configs/mx6qsabrelite.h +++ b/include/configs/mx6qsabrelite.h @@ -33,6 +33,7 @@ #define CONFIG_CMDLINE_TAG #define CONFIG_SETUP_MEMORY_TAGS #define CONFIG_INITRD_TAG +#define CONFIG_REVISION_TAG /* Size of malloc() pool */ #define CONFIG_SYS_MALLOC_LEN (CONFIG_ENV_SIZE + 2 * 1024 * 1024)
Freescale 2.6.38 (Non-DT) kernels require the revision atag to enable the VPU. Signed-off-by: Eric Nelson <eric.nelson@boundarydevices.com> --- board/freescale/mx6qsabrelite/mx6qsabrelite.c | 7 +++++++ include/configs/mx6qsabrelite.h | 1 + 2 files changed, 8 insertions(+), 0 deletions(-)