diff mbox series

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

Message ID 3b031bd957f9bce96992c45b063999ec8eec946a.1512906844.git.baruch@tkos.co.il
State Superseded
Delegated to: Stefan Roese
Headers show
Series [U-Boot] arm64: mvebu: armada-8k: support SD card environment | expand

Commit Message

Baruch Siach Dec. 10, 2017, 11:54 a.m. UTC
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@marvell.com>
Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
 include/configs/mvebu_armada-8k.h | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Kostya Porotchkin Dec. 14, 2017, 11:28 a.m. UTC | #1
> -----Original Message-----
> From: Baruch Siach [mailto:baruch@tkos.co.il]
> Sent: Sunday, December 10, 2017 13:54
> To: Stefan Roese
> Cc: u-boot@lists.denx.de; Sergey Matyukevich; Baruch Siach; Kostya
> Porotchkin
> Subject: [EXT] [PATCH] arm64: mvebu: armada-8k: support SD card
> environment
> 
> External Email
> 
> ----------------------------------------------------------------------
> 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@marvell.com>
> Signed-off-by: Baruch Siach <baruch@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
[Konstantin Porotchkin] 
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.

Kosta

>  #define CONFIG_ENV_SIZE			(64 << 10) /* 64KiB */
>  #define CONFIG_ENV_SECT_SIZE		(64 << 10) /* 64KiB sectors */
> 
> --
> 2.15.0
Baruch Siach Dec. 19, 2017, 6:43 a.m. UTC | #2
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@marvell.com>
> > Signed-off-by: Baruch Siach <baruch@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?

baruch
Kostya Porotchkin Dec. 19, 2017, 8:42 a.m. UTC | #3
Hi, Baruch,

> -----Original Message-----
> From: Baruch Siach [mailto:baruch@tkos.co.il]
> Sent: Tuesday, December 19, 2017 08:43
> To: Kostya Porotchkin
> Cc: Stefan Roese; u-boot@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@marvell.com>
> > > Signed-off-by: Baruch Siach <baruch@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.

Kosta
> 
> baruch
> 
> --
>      http://baruch.siach.name/blog/                  ~. .~   Tk Open
> Systems
> =}------------------------------------------------ooO--U--Ooo-----------
> -{=
>    - baruch@tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -
Stefan Roese Jan. 9, 2018, 3:40 p.m. UTC | #4
Hi Baruch, Hi Kosta,

On 19.12.2017 09:42, Kostya Porotchkin wrote:
>> -----Original Message-----
>> From: Baruch Siach [mailto:baruch@tkos.co.il]
>> Sent: Tuesday, December 19, 2017 08:43
>> To: Kostya Porotchkin
>> Cc: Stefan Roese; u-boot@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@marvell.com>
>>>> Signed-off-by: Baruch Siach <baruch@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.

Thanks,
Stefan
Kostya Porotchkin Jan. 9, 2018, 3:43 p.m. UTC | #5
Hi, Stefan,

> -----Original Message-----

> From: Stefan Roese [mailto:sr@denx.de]

> Sent: Tuesday, January 09, 2018 17:40

> To: Kostya Porotchkin; Baruch Siach

> Cc: u-boot@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@tkos.co.il]

> >> Sent: Tuesday, December 19, 2017 08:43

> >> To: Kostya Porotchkin

> >> Cc: Stefan Roese; u-boot@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@marvell.com>

> >>>> Signed-off-by: Baruch Siach <baruch@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
Baruch Siach Jan. 9, 2018, 4:36 p.m. UTC | #6
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@tkos.co.il]
> > > Sent: Tuesday, December 19, 2017 08:43
> > > To: Kostya Porotchkin
> > > Cc: Stefan Roese; u-boot@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@marvell.com>
> > > > > Signed-off-by: Baruch Siach <baruch@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
diff mbox series

Patch

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
 #define CONFIG_ENV_SIZE			(64 << 10) /* 64KiB */
 #define CONFIG_ENV_SECT_SIZE		(64 << 10) /* 64KiB sectors */