[U-Boot] [EXT] [PATCH] arm64: mvebu: armada-8k: support SD card environment

[Baruch Siach]

HI Stefan,

On Tue, Jan 09, 2018 at 04:40:05PM +0100, Stefan Roese wrote:
> On 19.12.2017 09:42, Kostya Porotchkin wrote:
> > > -----Original Message-----
> > > From: Baruch Siach [mailto:baruch at tkos.co.il]
> > > Sent: Tuesday, December 19, 2017 08:43
> > > To: Kostya Porotchkin
> > > Cc: Stefan Roese; u-boot at lists.denx.de; Sergey Matyukevich
> > > Subject: Re: [EXT] [PATCH] arm64: mvebu: armada-8k: support SD card
> > > environment
> > > 
> > > Hi Kostya,
> > > 
> > > Thanks for reviewing.
> > > 
> > > On Thu, Dec 14, 2017 at 11:28:18AM +0000, Kostya Porotchkin wrote:
> > > > > Allow storing the environment on the Macchiatobin SD card. This is
> > > > > useful for distribution of SD card software images. Currently, the
> > > > > environment is always loaded from the SPI flash whose content might
> > > > > be incompatible with SD card kernel loading.
> > > > > 
> > > > > Use the last 64KB of the 32MB boot partition as per the instructions
> > > > > in the Macchiatobin wiki:
> > > > > 
> > > > >    http://wiki.macchiatobin.net/tiki-
> > > > > index.php?page=Setup+alternative+boot+sources
> > > > > 
> > > > > Cc: Konstantin Porotchkin <kostap at marvell.com>
> > > > > Signed-off-by: Baruch Siach <baruch at tkos.co.il>
> > > > > ---
> > > > >   include/configs/mvebu_armada-8k.h | 5 +++++
> > > > >   1 file changed, 5 insertions(+)
> > > > > 
> > > > > diff --git a/include/configs/mvebu_armada-8k.h
> > > > > b/include/configs/mvebu_armada-8k.h
> > > > > index d85527434a0a..0457a72e353b 100644
> > > > > --- a/include/configs/mvebu_armada-8k.h
> > > > > +++ b/include/configs/mvebu_armada-8k.h
> > > > > @@ -73,7 +73,12 @@
> > > > >   #define CONFIG_SF_DEFAULT_MODE		SPI_MODE_0
> > > > >   #define CONFIG_ENV_SPI_MODE		CONFIG_SF_DEFAULT_MODE
> > > > > 
> > > > > +#if defined(CONFIG_ENV_IS_IN_SPI_FLASH)
> > > > >   #define CONFIG_ENV_OFFSET		0x180000 /* as Marvell U-Boot
> > > version */
> > > > > +#elif defined(CONFIG_ENV_IS_IN_MMC)
> > > > > +#define CONFIG_SYS_MMC_ENV_DEV		1
> > > > > +#define CONFIG_ENV_OFFSET		0x21f0000
> > > > > +#endif
> > > > 
> > > > I think this will break boot from the on-board eMMC device.
> > > > Maybe the environment offset should be connected to the MMC device ID.
> > > > So it will be different for SD and eMMC.
> > > > The eMMC boot partition is not that big as the space on SD allocated
> > > for the boot images.
> > > > Additionally, the SD pre-allocated boot space is not really a constant
> > > value.
> > > 
> > > So what would you suggest? How can we detect the location of the SD/eMMC
> > > stored environment? Would a per-board 'u-boot,mmc-env-offset' DT
> > > property help? Maybe move to CONFIG_ENV_IS_IN_FAT?
> > [Konstantin Porotchkin]
> > What we know for sure is that partition 0 is always the data partition for both SD and eMMC.
> > This is the only partition in case of SD and it less likely to be used with eMMC at all.
> > So we can, for example, use your environment offset when the CONFIG_SYS_MMC_ENV_PART=0
> > As for the value of CONFIG_SYS_MMC_ENV_DEV, we cannot assume that it will have the same meaning for all Marvell A8K platforms. On some boards the device 1 may be the eMMC as well.
> > Since this header is shared across all boards, I would not hard code the device 1 as SD.
> > Using file for the environment storage when we boot from the data partition is also possible.
> > In this case we do not have the dependency from the size of the empty space allocated before the first partition entry.
> 
> So whats the current state of this patch? As I see it (please correct
> me, if I'm wrong), its not clear to go in this way. So I'll not
> push it upstream with my upcoming pull request.

I agree that a DT based approach would be better.

baruch

-- 
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -