Re: [PATCH v3 u-boot] mmc: spl: Make partition choice in default_spl_mmc_emmc_boot_partition() more explicit
On Saturday 08 July 2023 11:28:34 Tom Rini wrote: > On Sat, Jul 08, 2023 at 10:30:47AM +0200, Pali Rohár wrote: > > On Friday 07 July 2023 19:43:34 Tom Rini wrote: > > > On Sat, Jul 08, 2023 at 12:54:38AM +0200, Pali Rohár wrote: > > > > > > > To make eMMC partition choosing in default_spl_mmc_emmc_boot_partition() > > > > function better understandable, rewrite it via explicit switch-case code > > > > pattern. > > > > > > > > Also indicate an error when eMMC EXT_CSD[179] register is configured by > > > > user to value which is not suitable for eMMC booting and SPL do not know > > > > how to interpret it. > > > > > > > > Signed-off-by: Pali Rohár > > > > > > I did some quick local testing to check on the size impact and this is > > > indeed quite manageable, thanks for reworking things! > > > > > > Reviewed-by: Tom Rini > > > > Unfortunately no, sama5d2_xplained_emmc, sama5d2_xplained_mmc and > > sama5d2_xplained_qspiflash are failing with v3; but not with v2. > > > > Seems that these boards have their SPL at limit and adding any new > > puts() increase size over the limit. > > > > So I think that there is no other way than just adding a new config > > option to hide this new puts(). > > We should enable LTO on those platforms then and buy us some space. > > -- > Tom Ok, but then this is really out of what I have a free time to do...
Re: [PATCH v3 u-boot] mmc: spl: Make partition choice in default_spl_mmc_emmc_boot_partition() more explicit
On Sat, Jul 08, 2023 at 10:30:47AM +0200, Pali Rohár wrote: > On Friday 07 July 2023 19:43:34 Tom Rini wrote: > > On Sat, Jul 08, 2023 at 12:54:38AM +0200, Pali Rohár wrote: > > > > > To make eMMC partition choosing in default_spl_mmc_emmc_boot_partition() > > > function better understandable, rewrite it via explicit switch-case code > > > pattern. > > > > > > Also indicate an error when eMMC EXT_CSD[179] register is configured by > > > user to value which is not suitable for eMMC booting and SPL do not know > > > how to interpret it. > > > > > > Signed-off-by: Pali Rohár > > > > I did some quick local testing to check on the size impact and this is > > indeed quite manageable, thanks for reworking things! > > > > Reviewed-by: Tom Rini > > Unfortunately no, sama5d2_xplained_emmc, sama5d2_xplained_mmc and > sama5d2_xplained_qspiflash are failing with v3; but not with v2. > > Seems that these boards have their SPL at limit and adding any new > puts() increase size over the limit. > > So I think that there is no other way than just adding a new config > option to hide this new puts(). We should enable LTO on those platforms then and buy us some space. -- Tom signature.asc Description: PGP signature
Re: [PATCH v3 u-boot] mmc: spl: Make partition choice in default_spl_mmc_emmc_boot_partition() more explicit
On Friday 07 July 2023 19:43:34 Tom Rini wrote: > On Sat, Jul 08, 2023 at 12:54:38AM +0200, Pali Rohár wrote: > > > To make eMMC partition choosing in default_spl_mmc_emmc_boot_partition() > > function better understandable, rewrite it via explicit switch-case code > > pattern. > > > > Also indicate an error when eMMC EXT_CSD[179] register is configured by > > user to value which is not suitable for eMMC booting and SPL do not know > > how to interpret it. > > > > Signed-off-by: Pali Rohár > > I did some quick local testing to check on the size impact and this is > indeed quite manageable, thanks for reworking things! > > Reviewed-by: Tom Rini > > -- > Tom Unfortunately no, sama5d2_xplained_emmc, sama5d2_xplained_mmc and sama5d2_xplained_qspiflash are failing with v3; but not with v2. Seems that these boards have their SPL at limit and adding any new puts() increase size over the limit. So I think that there is no other way than just adding a new config option to hide this new puts().
Re: [PATCH v3 u-boot] mmc: spl: Make partition choice in default_spl_mmc_emmc_boot_partition() more explicit
On Sat, Jul 08, 2023 at 12:54:38AM +0200, Pali Rohár wrote: > To make eMMC partition choosing in default_spl_mmc_emmc_boot_partition() > function better understandable, rewrite it via explicit switch-case code > pattern. > > Also indicate an error when eMMC EXT_CSD[179] register is configured by > user to value which is not suitable for eMMC booting and SPL do not know > how to interpret it. > > Signed-off-by: Pali Rohár I did some quick local testing to check on the size impact and this is indeed quite manageable, thanks for reworking things! Reviewed-by: Tom Rini -- Tom signature.asc Description: PGP signature
[PATCH v3 u-boot] mmc: spl: Make partition choice in default_spl_mmc_emmc_boot_partition() more explicit
To make eMMC partition choosing in default_spl_mmc_emmc_boot_partition()
function better understandable, rewrite it via explicit switch-case code
pattern.
Also indicate an error when eMMC EXT_CSD[179] register is configured by
user to value which is not suitable for eMMC booting and SPL do not know
how to interpret it.
Signed-off-by: Pali Rohár
---
Changes in v3:
* Instead of showing (verbose) warning, make them as errors and
propagate them back to caller.
* Add comment with explanation what happened and how to fix possible
misconfigured configuration.
Changes in v2:
* Disable showing warning on sama5d2_xplained due to size restrictions
---
This patch depends on another patch:
mmc: spl: Add comments for default_spl_mmc_emmc_boot_partition()
https://patchwork.ozlabs.org/project/uboot/patch/20230404202805.8523-1-p...@kernel.org/
---
common/spl/spl_mmc.c | 56 +---
1 file changed, 48 insertions(+), 8 deletions(-)
diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c
index f7a42a11477d..656363a3b51a 100644
--- a/common/spl/spl_mmc.c
+++ b/common/spl/spl_mmc.c
@@ -408,15 +408,46 @@ int default_spl_mmc_emmc_boot_partition(struct mmc *mmc)
*
* Note: See difference between EXT_CSD_EXTRACT_PARTITION_ACCESS
* and EXT_CSD_EXTRACT_BOOT_PART, specially about User area value.
-*
-* FIXME: When booting from this eMMC device is explicitly
-* disabled then we use User area for booting. This is incorrect.
-* Probably we should skip this eMMC device and select the next
-* one for booting. Or at least throw warning about this fallback.
*/
- part = EXT_CSD_EXTRACT_BOOT_PART(mmc->part_config);
- if (part == 7)
- part = 0;
+ if (mmc->part_config == MMCPART_NOAVAILABLE)
+ part = 0; /* If partitions are not supported then we have only
User Area partition */
+ else {
+ switch(EXT_CSD_EXTRACT_BOOT_PART(mmc->part_config)) {
+ case 0: /* Booting from this eMMC device is disabled */
+ /*
+* This eMMC device has disabled booting.
+* This may happen because of misconfiguration of eMMC
device or
+* because user explicitly wanted to not boot from this
eMMC device.
+* eMMC boot configuration can be changed by "mmc
partconf" command.
+*/
+ part = -ENXIO; /* negative value indicates error */
+ /* Note that error message is printed by caller of this
function. */
+ break;
+ case 1: /* Boot partition 1 is used for booting */
+ part = 1;
+ break;
+ case 2: /* Boot partition 2 is used for booting */
+ part = 2;
+ break;
+ case 7: /* User area is used for booting */
+ part = 0;
+ break;
+ default: /* Other values are reserved */
+ /*
+* This eMMC device has configured booting from
reserved value.
+* This may happen because of misconfiguration of eMMC
device or
+* because this is newer eMMC device than what U-Boot
understand.
+* If newer eMMC specification defines meaning for some
reserved
+* values then switch above should be updated for new
cases.
+* At this stage we do not know how to interpret this
reserved
+* value so return error.
+* eMMC boot configuration can be changed by "mmc
partconf" command.
+*/
+ part = -EINVAL; /* negative value indicates error */
+ /* Note that error message is printed by caller of this
function. */
+ break;
+ }
+ }
#endif
return part;
}
@@ -471,6 +502,15 @@ int spl_mmc_load(struct spl_image_info *spl_image,
switch (boot_mode) {
case MMCSD_MODE_EMMCBOOT:
part = spl_mmc_emmc_boot_partition(mmc);
+ if (part < 0) {
+#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT
+ if (part == -ENXIO)
+ puts("spl: mmc booting disabled\n");
+ else
+ puts("spl: mmc misconfigured\n");
+#endif
+ return part;
+ }
if (CONFIG_IS_ENABLED(MMC_TINY))
err = mmc_switch_part(mmc, part);
--
2.20.1
