| Message ID | 1326838190-13746-7-git-send-email-eric.nelson@boundarydevices.com |
|---|---|
| State | Superseded |
| Headers | show |
On Wed, Jan 18, 2012 at 6:09 AM, Eric Nelson <eric.nelson@boundarydevices.com> wrote: > Signed-off-by: Eric Nelson <eric.nelson@boundarydevices.com> > --- > include/configs/mx6qsabrelite.h | 12 +++++++++++- > 1 files changed, 11 insertions(+), 1 deletions(-) > > diff --git a/include/configs/mx6qsabrelite.h b/include/configs/mx6qsabrelite.h > index 44b028a..160894c 100644 > --- a/include/configs/mx6qsabrelite.h > +++ b/include/configs/mx6qsabrelite.h > @@ -174,10 +174,20 @@ > /* FLASH and environment organization */ > #define CONFIG_SYS_NO_FLASH > > -#define CONFIG_ENV_OFFSET (6 * 64 * 1024) > #define CONFIG_ENV_SIZE (8 * 1024) > + > #define CONFIG_ENV_IS_IN_MMC > +/* #define CONFIG_ENV_IS_IN_SPI_FLASH */ From the commit log, it says the default is in serial flash, but apparently in the code the env is default to MMC, which mismatch. please fix it. > + > +#if defined(CONFIG_ENV_IS_IN_MMC) > +#define CONFIG_ENV_OFFSET (6 * 64 * 1024) > #define CONFIG_SYS_MMC_ENV_DEV 0 > +#elif defined(CONFIG_ENV_IS_IN_SPI_FLASH) > +#define CONFIG_ENV_OFFSET (768 * 1024) > +#define CONFIG_ENV_SECT_SIZE (8 * 1024) > +#define CONFIG_ENV_SPI_CS 0x5300 I'm wondering how the CONFIG_ENV_SPI_CS could be 0x5300? Vague? > +#define CONFIG_ENV_SPI_MODE SPI_MODE_0 > +#endif > > #define CONFIG_OF_LIBFDT > > -- > 1.7.1 >
On 20.01.2012 04:27, Jason Hui wrote: > On Wed, Jan 18, 2012 at 6:09 AM, Eric Nelson > <eric.nelson@boundarydevices.com> wrote: >> Signed-off-by: Eric Nelson <eric.nelson@boundarydevices.com> >> --- >> include/configs/mx6qsabrelite.h | 12 +++++++++++- >> 1 files changed, 11 insertions(+), 1 deletions(-) >> >> diff --git a/include/configs/mx6qsabrelite.h b/include/configs/mx6qsabrelite.h >> index 44b028a..160894c 100644 >> --- a/include/configs/mx6qsabrelite.h >> +++ b/include/configs/mx6qsabrelite.h >> @@ -174,10 +174,20 @@ >> /* FLASH and environment organization */ >> #define CONFIG_SYS_NO_FLASH >> >> -#define CONFIG_ENV_OFFSET (6 * 64 * 1024) >> #define CONFIG_ENV_SIZE (8 * 1024) >> + >> #define CONFIG_ENV_IS_IN_MMC >> +/* #define CONFIG_ENV_IS_IN_SPI_FLASH */ > >>From the commit log, it says the default is in serial flash, No, the commit log doesn't say this. It doesn't say 'it is'. It says it 'provides the defaults'. But it doesn't say that it actually uses these defaults. > but > apparently in the code > the env is default to MMC, which mismatch. please fix it. As mentioned above, I understand this differently. While I reviewed it some days ago, I found the description and the doing here quite fine. It enables the MMC env and additionally _provides_ everything needed to easily switch to SPI env by uncommenting one switch. This is fine and quite helpful, see e.g. [1]. I like this, please keep it as is. Best regards Dirk [1] http://lists.denx.de/pipermail/u-boot/2012-January/116266.html "you can place the environment in SPI-NOR as well by commenting out CONFIG_ENV_IS_IN_MMC, and un-commenting ..._IN_SPI_FLASH in include/configs/mx6qsabrelite.h." >> + >> +#if defined(CONFIG_ENV_IS_IN_MMC) >> +#define CONFIG_ENV_OFFSET (6 * 64 * 1024) >> #define CONFIG_SYS_MMC_ENV_DEV 0 >> +#elif defined(CONFIG_ENV_IS_IN_SPI_FLASH) >> +#define CONFIG_ENV_OFFSET (768 * 1024) >> +#define CONFIG_ENV_SECT_SIZE (8 * 1024) >> +#define CONFIG_ENV_SPI_CS 0x5300 > > I'm wondering how the CONFIG_ENV_SPI_CS could be 0x5300? Vague? > >> +#define CONFIG_ENV_SPI_MODE SPI_MODE_0 >> +#endif >> >> #define CONFIG_OF_LIBFDT >> >> -- >> 1.7.1 >> >
On Fri, Jan 20, 2012 at 3:06 PM, Dirk Behme <dirk.behme@de.bosch.com> wrote: > On 20.01.2012 04:27, Jason Hui wrote: >> >> On Wed, Jan 18, 2012 at 6:09 AM, Eric Nelson >> <eric.nelson@boundarydevices.com> wrote: >>> >>> Signed-off-by: Eric Nelson <eric.nelson@boundarydevices.com> >>> --- >>> include/configs/mx6qsabrelite.h | 12 +++++++++++- >>> 1 files changed, 11 insertions(+), 1 deletions(-) >>> >>> diff --git a/include/configs/mx6qsabrelite.h >>> b/include/configs/mx6qsabrelite.h >>> index 44b028a..160894c 100644 >>> --- a/include/configs/mx6qsabrelite.h >>> +++ b/include/configs/mx6qsabrelite.h >>> @@ -174,10 +174,20 @@ >>> /* FLASH and environment organization */ >>> #define CONFIG_SYS_NO_FLASH >>> >>> -#define CONFIG_ENV_OFFSET (6 * 64 * 1024) >>> #define CONFIG_ENV_SIZE (8 * 1024) >>> + >>> #define CONFIG_ENV_IS_IN_MMC >>> +/* #define CONFIG_ENV_IS_IN_SPI_FLASH */ >> >> >>> From the commit log, it says the default is in serial flash, > > > No, the commit log doesn't say this. It doesn't say 'it is'. It says it > 'provides the defaults'. But it doesn't say that it actually uses these > defaults. > > >> but >> apparently in the code >> the env is default to MMC, which mismatch. please fix it. > > > As mentioned above, I understand this differently. While I reviewed it some > days ago, I found the description and the doing here quite fine. OK, I get it, then I'm fine with it too. > > It enables the MMC env and additionally _provides_ everything needed to > easily switch to SPI env by uncommenting one switch. This is fine and quite > helpful, see e.g. [1]. > > I like this, please keep it as is. > [...] >>> +#define CONFIG_ENV_SPI_CS 0x5300 >> >> >> I'm wondering how the CONFIG_ENV_SPI_CS could be 0x5300? Vague? Then the left open question is only above one. Jason
On 20/01/2012 08:48, Jason Hui wrote: >>> >>> I'm wondering how the CONFIG_ENV_SPI_CS could be 0x5300? Vague? > > Then the left open question is only above one. The SPI driver can take as chip select the controller's chip selects as well as an external GPIO. The LSB byte has the value of the internal chip select, the highest (thought as 16-bit value) contains the GPIO number. Reading this configuration, the GPIO used on this board should be the number 83 (0x53). Stefano
On 01/20/2012 01:47 AM, Stefano Babic wrote: > On 20/01/2012 08:48, Jason Hui wrote: > >>>> >>>> I'm wondering how the CONFIG_ENV_SPI_CS could be 0x5300? Vague? >> >> Then the left open question is only above one. > > The SPI driver can take as chip select the controller's chip selects as > well as an external GPIO. The LSB byte has the value of the internal > chip select, the highest (thought as 16-bit value) contains the GPIO > number. Reading this configuration, the GPIO used on this board should > be the number 83 (0x53). > > Stefano > Thanks Stefano, I like your description better than the one I just wrote... I should have scanned all of my e-mails before drafting my earlier commit msg ;)
diff --git a/include/configs/mx6qsabrelite.h b/include/configs/mx6qsabrelite.h index 44b028a..160894c 100644 --- a/include/configs/mx6qsabrelite.h +++ b/include/configs/mx6qsabrelite.h @@ -174,10 +174,20 @@ /* FLASH and environment organization */ #define CONFIG_SYS_NO_FLASH -#define CONFIG_ENV_OFFSET (6 * 64 * 1024) #define CONFIG_ENV_SIZE (8 * 1024) + #define CONFIG_ENV_IS_IN_MMC +/* #define CONFIG_ENV_IS_IN_SPI_FLASH */ + +#if defined(CONFIG_ENV_IS_IN_MMC) +#define CONFIG_ENV_OFFSET (6 * 64 * 1024) #define CONFIG_SYS_MMC_ENV_DEV 0 +#elif defined(CONFIG_ENV_IS_IN_SPI_FLASH) +#define CONFIG_ENV_OFFSET (768 * 1024) +#define CONFIG_ENV_SECT_SIZE (8 * 1024) +#define CONFIG_ENV_SPI_CS 0x5300 +#define CONFIG_ENV_SPI_MODE SPI_MODE_0 +#endif #define CONFIG_OF_LIBFDT
Signed-off-by: Eric Nelson <eric.nelson@boundarydevices.com> --- include/configs/mx6qsabrelite.h | 12 +++++++++++- 1 files changed, 11 insertions(+), 1 deletions(-)