Re: [PATCH] Revert "configs: stm32mp1: only support SD card after NOR in bootcmd_stm32mp"
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"
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"
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"
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