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

[Kostya Porotchkin]

Hi, Stefan,

> -----Original Message-----
> From: Stefan Roese [mailto:sr at denx.de]
> Sent: Tuesday, January 09, 2018 17:40
> To: Kostya Porotchkin; Baruch Siach
> Cc: u-boot at lists.denx.de; Sergey Matyukevich
> Subject: Re: [EXT] [PATCH] arm64: mvebu: armada-8k: support SD card
> environment
> 
> Hi Baruch, Hi Kosta,
> 
> 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.
[Konstantin Porotchkin] yes, I agree, it should not be merged as-is.
Kosta
> 
> Thanks,
> Stefan