Re: [PATCH] Revert "configs: stm32mp1: only support SD card after NOR in bootcmd_stm32mp"

2021-10-08 Thread Patrice CHOTARD
Hi Marek

On 10/4/21 1:48 PM, Marek Vasut wrote:
> This reverts commit d5d726d3cc47691ace3c68fa31147ad104aaf579,
> which breaks boards which ship with multiple SD/eMMC sockets.
> 
> This stm32mp1.h config is not used only by the ST reference
> boards, but all the other STM32MP1 based boards in U-Boot, so
> changes to this stm32mp1.h cannot break the other boards.
> 
> Signed-off-by: Marek Vasut 
> Cc: Patrice Chotard 
> Cc: Patrick Delaunay 
> ---
> NOTE: I think we might need to split out the env for different
>   boards into different headers instead. Thoughts ?
> ---
>  include/configs/stm32mp1.h | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/include/configs/stm32mp1.h b/include/configs/stm32mp1.h
> index 973a4f1d4b8..a75ed693f57 100644
> --- a/include/configs/stm32mp1.h
> +++ b/include/configs/stm32mp1.h
> @@ -120,7 +120,7 @@
>   * for serial/usb: execute the stm32prog command
>   * for mmc boot (eMMC, SD card), boot only on the same device
>   * for nand or spi-nand boot, boot with on ubifs partition on UBI partition
> - * for nor boot, use SD card = mmc0
> + * for nor boot, use the default order
>   */
>  #define STM32MP_BOOTCMD "bootcmd_stm32mp=" \
>   "echo \"Boot over ${boot_device}${boot_instance}!\";" \
> @@ -133,8 +133,6 @@
>   "if test ${boot_device} = nand ||" \
> " test ${boot_device} = spi-nand ;" \
>   "then env set boot_targets ubifs0; fi;" \
> - "if test ${boot_device} = nor;" \
> - "then env set boot_targets mmc0; fi;" \
>   "run distro_bootcmd;" \
>   "fi;\0"
>  
> 

Applied on u-boot-stm32 for next

Thanks
Patrice


RE: [PATCH] Revert "configs: stm32mp1: only support SD card after NOR in bootcmd_stm32mp"

2021-10-05 Thread patrick.delaunay
Hi,


ST Restricted

> -Original Message-
> From: Marek Vasut 
> Sent: lundi 4 octobre 2021 18:45
> To: Patrick DELAUNAY ; u-boot@lists.denx.de
> Cc: Patrice Chotard 
> Subject: Re: [PATCH] Revert "configs: stm32mp1: only support SD card after
> NOR in bootcmd_stm32mp"
> 
> On 10/4/21 6:34 PM, Patrick DELAUNAY wrote:
> > Hi,
> 
> Hi,
> 
> > On 10/4/21 1:48 PM, Marek Vasut wrote:
> >> This reverts commit d5d726d3cc47691ace3c68fa31147ad104aaf579,
> >> which breaks boards which ship with multiple SD/eMMC sockets.
> >>
> >> This stm32mp1.h config is not used only by the ST reference boards,
> >> but all the other STM32MP1 based boards in U-Boot, so changes to this
> >> stm32mp1.h cannot break the other boards.
> >>
> >> Signed-off-by: Marek Vasut 
> >> Cc: Patrice Chotard 
> >> Cc: Patrick Delaunay 
> >> ---
> >> NOTE: I think we might need to split out the env for different
> >>boards into different headers instead. Thoughts ?
> >> ---
> >>   include/configs/stm32mp1.h | 4 +---
> >>   1 file changed, 1 insertion(+), 3 deletions(-)
> >>
> >> diff --git a/include/configs/stm32mp1.h b/include/configs/stm32mp1.h
> >> index 973a4f1d4b8..a75ed693f57 100644
> >> --- a/include/configs/stm32mp1.h
> >> +++ b/include/configs/stm32mp1.h
> >> @@ -120,7 +120,7 @@
> >>* for serial/usb: execute the stm32prog command
> >>* for mmc boot (eMMC, SD card), boot only on the same device
> >>* for nand or spi-nand boot, boot with on ubifs partition on UBI
> >> partition
> >> - * for nor boot, use SD card = mmc0
> >> + * for nor boot, use the default order
> >>*/
> >>   #define STM32MP_BOOTCMD "bootcmd_stm32mp=" \
> >>   "echo \"Boot over ${boot_device}${boot_instance}!\";" \ @@
> >> -133,8 +133,6 @@
> >>   "if test ${boot_device} = nand ||" \
> >> " test ${boot_device} = spi-nand ;" \
> >>   "then env set boot_targets ubifs0; fi;" \
> >> -"if test ${boot_device} = nor;" \
> >> -"then env set boot_targets mmc0; fi;" \
> >>   "run distro_bootcmd;" \
> >>   "fi;\0"
> >
> >
> > Reviewed-by: Patrick Delaunay 
> >
> >
> > Sorry to break your board, but I assumed the "stm32mp1.h" was only the
> > default ST configuration
> 
> No worries really. No, it isn't only for the ST evalboards, and I don't know 
> whether
> this is the right approach.
> 
> > for ST boards and other boards need to be overridden it if it is not
> > align with their needs
> >
> > as I don't know the expected boot sequence.
> >
> > for example with:
> >
> >  CONFIG_BOOTCOMMAND="run bootcmd_dh_stm32mp"
> >
> > with bootcmd_dh_stm32mp to be defined
> >
> >
> > but today this file is a mix between SOC configuration (generic) and
> > ST boards needs.
> >
> >
> > So I will merge your revert and I will push a other solution to only
> > support SD card
> >
> > after NOR in bootcmd_stm32mp but only for STMicroelectronics boards
> >
> > (because the revert now break the EV1 boot from NOR).
> >
> > 1) stm32mp1.h = common for SOC STMP15x (as today)
> >
> > 2) st_stm32mp1.h = ST boards configuration (override common)
> 
> Maybe the naming should be the other way around, so we have some sort of
> namespacing, like this:
> 
> stm32mp1_common.h
> ... common stuff for all boards and SoM and all ...
> 
> stm32mp1_st_evalboard.h (or whatever you want to call it) #include
>  #define custom ST env ...
> 
> stm32mp1_dh_dhsom.h
> #include 
> #define custom DH env ...
> 
> That's what imx does , except for the namespacing, so the file names are a 
> mess.
> I think we can do better there. So anything related to stm32mp1 should have
> stm32mp1_* filename prefix, and then some vendor_ or board_ suffix.

Agree it is better and more coherent.

I wasn't want to change the dh files, 
but I will do it: 

configs/stm32mp1_common.h or
stm32mp15_common.h

configs/stm32mp1_dh_dhsom.h (as you proposed) or 
stm32mp15_dh_dhsom.h

configs/stm32mp1_st_stm32mp15.h or 
stm32mp15_st_common.h or
 stm32mp15_st_boards.h (name to be confirmed)

with
- stm32mp1_* filename prefix for STM32MP1 Series (Cortex arm v7)
- stm32mp15_* filename prefix for STM32MP15x lines
- stm32mp13_* filename prefix for STM32MP13x lines (coming soon => 
https://lwn.net/Articles/864174/)

Regards

Patrick



Re: [PATCH] Revert "configs: stm32mp1: only support SD card after NOR in bootcmd_stm32mp"

2021-10-04 Thread Marek Vasut

On 10/4/21 6:34 PM, Patrick DELAUNAY wrote:

Hi,


Hi,


On 10/4/21 1:48 PM, Marek Vasut wrote:

This reverts commit d5d726d3cc47691ace3c68fa31147ad104aaf579,
which breaks boards which ship with multiple SD/eMMC sockets.

This stm32mp1.h config is not used only by the ST reference
boards, but all the other STM32MP1 based boards in U-Boot, so
changes to this stm32mp1.h cannot break the other boards.

Signed-off-by: Marek Vasut 
Cc: Patrice Chotard 
Cc: Patrick Delaunay 
---
NOTE: I think we might need to split out the env for different
   boards into different headers instead. Thoughts ?
---
  include/configs/stm32mp1.h | 4 +---
  1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/include/configs/stm32mp1.h b/include/configs/stm32mp1.h
index 973a4f1d4b8..a75ed693f57 100644
--- a/include/configs/stm32mp1.h
+++ b/include/configs/stm32mp1.h
@@ -120,7 +120,7 @@
   * for serial/usb: execute the stm32prog command
   * for mmc boot (eMMC, SD card), boot only on the same device
   * for nand or spi-nand boot, boot with on ubifs partition on UBI 
partition

- * for nor boot, use SD card = mmc0
+ * for nor boot, use the default order
   */
  #define STM32MP_BOOTCMD "bootcmd_stm32mp=" \
  "echo \"Boot over ${boot_device}${boot_instance}!\";" \
@@ -133,8 +133,6 @@
  "if test ${boot_device} = nand ||" \
    " test ${boot_device} = spi-nand ;" \
  "then env set boot_targets ubifs0; fi;" \
-    "if test ${boot_device} = nor;" \
-    "then env set boot_targets mmc0; fi;" \
  "run distro_bootcmd;" \
  "fi;\0"



Reviewed-by: Patrick Delaunay 


Sorry to break your board, but I assumed the "stm32mp1.h" was only the 
default ST configuration


No worries really. No, it isn't only for the ST evalboards, and I don't 
know whether this is the right approach.


for ST boards and other boards need to be overridden it if it is not 
align with their needs


as I don't know the expected boot sequence.

for example with:

     CONFIG_BOOTCOMMAND="run bootcmd_dh_stm32mp"

with bootcmd_dh_stm32mp to be defined


but today this file is a mix between SOC configuration (generic) and ST 
boards needs.



So I will merge your revert and I will push a other solution to only 
support SD card


after NOR in bootcmd_stm32mp but only for STMicroelectronics boards

(because the revert now break the EV1 boot from NOR).

1) stm32mp1.h = common for SOC STMP15x (as today)

2) st_stm32mp1.h = ST boards configuration (override common)


Maybe the naming should be the other way around, so we have some sort of 
namespacing, like this:


stm32mp1_common.h
... common stuff for all boards and SoM and all ...

stm32mp1_st_evalboard.h (or whatever you want to call it)
#include 
#define custom ST env ...

stm32mp1_dh_dhsom.h
#include 
#define custom DH env ...

That's what imx does , except for the namespacing, so the file names are 
a mess. I think we can do better there. So anything related to stm32mp1 
should have stm32mp1_* filename prefix, and then some vendor_ or board_ 
suffix.


Re: [PATCH] Revert "configs: stm32mp1: only support SD card after NOR in bootcmd_stm32mp"

2021-10-04 Thread Patrick DELAUNAY

Hi,

On 10/4/21 1:48 PM, Marek Vasut wrote:

This reverts commit d5d726d3cc47691ace3c68fa31147ad104aaf579,
which breaks boards which ship with multiple SD/eMMC sockets.

This stm32mp1.h config is not used only by the ST reference
boards, but all the other STM32MP1 based boards in U-Boot, so
changes to this stm32mp1.h cannot break the other boards.

Signed-off-by: Marek Vasut 
Cc: Patrice Chotard 
Cc: Patrick Delaunay 
---
NOTE: I think we might need to split out the env for different
   boards into different headers instead. Thoughts ?
---
  include/configs/stm32mp1.h | 4 +---
  1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/include/configs/stm32mp1.h b/include/configs/stm32mp1.h
index 973a4f1d4b8..a75ed693f57 100644
--- a/include/configs/stm32mp1.h
+++ b/include/configs/stm32mp1.h
@@ -120,7 +120,7 @@
   * for serial/usb: execute the stm32prog command
   * for mmc boot (eMMC, SD card), boot only on the same device
   * for nand or spi-nand boot, boot with on ubifs partition on UBI partition
- * for nor boot, use SD card = mmc0
+ * for nor boot, use the default order
   */
  #define STM32MP_BOOTCMD "bootcmd_stm32mp=" \
"echo \"Boot over ${boot_device}${boot_instance}!\";" \
@@ -133,8 +133,6 @@
"if test ${boot_device} = nand ||" \
  " test ${boot_device} = spi-nand ;" \
"then env set boot_targets ubifs0; fi;" \
-   "if test ${boot_device} = nor;" \
-   "then env set boot_targets mmc0; fi;" \
"run distro_bootcmd;" \
"fi;\0"
  



Reviewed-by: Patrick Delaunay 


Sorry to break your board, but I assumed the "stm32mp1.h" was only the 
default ST configuration


for ST boards and other boards need to be overridden it if it is not 
align with their needs


as I don't know the expected boot sequence.

for example with:

    CONFIG_BOOTCOMMAND="run bootcmd_dh_stm32mp"

with bootcmd_dh_stm32mp to be defined


but today this file is a mix between SOC configuration (generic) and ST 
boards needs.



So I will merge your revert and I will push a other solution to only 
support SD card


after NOR in bootcmd_stm32mp but only for STMicroelectronics boards

(because the revert now break the EV1 boot from NOR).

1) stm32mp1.h = common for SOC STMP15x (as today)

2) st_stm32mp1.h = ST boards configuration (override common)


Thanks
Patrick