[PATCH v3 1/2] spl: mmc: Pass eMMC HW partition and SW partition to spl_mmc_get_uboot_raw_sector()
Pass both eMMC HW partition and software partition numbers to spl_mmc_get_uboot_raw_sector() so the function can better decide which offset within the partition to load payload from.
Signed-off-by: Fedor Ross fedor.ross@ifm.com Signed-off-by: Marek Vasut marex@denx.de --- Cc: Jaehoon Chung jh80.chung@samsung.com Cc: Peng Fan peng.fan@nxp.com --- V2: No change V3: Update on u-boot/master, update spl.h function prototype --- arch/arm/mach-imx/image-container.c | 2 ++ arch/arm/mach-sunxi/board.c | 2 ++ .../advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c | 4 +++- common/spl/spl_mmc.c | 15 +++++++++------ include/spl.h | 4 ++++ 5 files changed, 20 insertions(+), 7 deletions(-)
diff --git a/arch/arm/mach-imx/image-container.c b/arch/arm/mach-imx/image-container.c index 5b059a64292..2eb8d5f6b9d 100644 --- a/arch/arm/mach-imx/image-container.c +++ b/arch/arm/mach-imx/image-container.c @@ -216,6 +216,8 @@ unsigned long spl_spi_get_uboot_offs(struct spi_flash *flash)
#ifdef CONFIG_SPL_MMC unsigned long spl_mmc_get_uboot_raw_sector(struct mmc *mmc, + unsigned long hw_part, + unsigned long raw_part, unsigned long raw_sect) { int end; diff --git a/arch/arm/mach-sunxi/board.c b/arch/arm/mach-sunxi/board.c index 391a65a5495..3be444d84fe 100644 --- a/arch/arm/mach-sunxi/board.c +++ b/arch/arm/mach-sunxi/board.c @@ -324,6 +324,8 @@ uint32_t sunxi_get_spl_size(void) * immediately follow the SPL if that is bigger than that. */ unsigned long spl_mmc_get_uboot_raw_sector(struct mmc *mmc, + unsigned long hw_part, + unsigned long raw_part, unsigned long raw_sect) { unsigned long spl_size = sunxi_get_spl_size(); diff --git a/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c b/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c index 466174679e8..af9d0040261 100644 --- a/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c +++ b/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c @@ -194,7 +194,9 @@ int board_late_init(void) #ifdef CONFIG_SPL_MMC #define UBOOT_RAW_SECTOR_OFFSET 0x40 unsigned long spl_mmc_get_uboot_raw_sector(struct mmc *mmc, - unsigned long raw_sector) + unsigned long hw_part, + unsigned long raw_part, + unsigned long raw_sect); { u32 boot_dev = spl_boot_device();
diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c index bd5e6adf1ea..f1f87d78ba7 100644 --- a/common/spl/spl_mmc.c +++ b/common/spl/spl_mmc.c @@ -362,6 +362,8 @@ int __weak spl_mmc_boot_partition(const u32 boot_device) #endif
unsigned long __weak spl_mmc_get_uboot_raw_sector(struct mmc *mmc, + unsigned long hw_part, + unsigned long raw_part, unsigned long raw_sect) { return raw_sect; @@ -410,7 +412,7 @@ int spl_mmc_load(struct spl_image_info *spl_image, static struct mmc *mmc; u32 boot_mode; int err = 0; - __maybe_unused int part = 0; + __maybe_unused int hw_part = 0; int mmc_dev;
/* Perform peripheral init only once for an mmc device */ @@ -434,12 +436,12 @@ int spl_mmc_load(struct spl_image_info *spl_image, err = -EINVAL; switch (boot_mode) { case MMCSD_MODE_EMMCBOOT: - part = spl_mmc_emmc_boot_partition(mmc); + hw_part = spl_mmc_emmc_boot_partition(mmc);
if (CONFIG_IS_ENABLED(MMC_TINY)) - err = mmc_switch_part(mmc, part); + err = mmc_switch_part(mmc, hw_part); else - err = blk_dselect_hwpart(mmc_get_blk_desc(mmc), part); + err = blk_dselect_hwpart(mmc_get_blk_desc(mmc), hw_part);
if (err) { #ifdef CONFIG_SPL_LIBCOMMON_SUPPORT @@ -457,7 +459,8 @@ int spl_mmc_load(struct spl_image_info *spl_image, return err; }
- raw_sect = spl_mmc_get_uboot_raw_sector(mmc, raw_sect); + raw_sect = spl_mmc_get_uboot_raw_sector(mmc, hw_part, + raw_part, raw_sect);
#ifdef CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_PARTITION err = mmc_load_image_raw_partition(spl_image, bootdev, @@ -468,7 +471,7 @@ int spl_mmc_load(struct spl_image_info *spl_image, #endif #ifdef CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR err = mmc_load_image_raw_sector(spl_image, bootdev, mmc, - raw_sect + spl_mmc_raw_uboot_offset(part)); + raw_sect + spl_mmc_raw_uboot_offset(hw_part)); if (!err) return err; #endif diff --git a/include/spl.h b/include/spl.h index 7e0f5ac63b0..5e174547f26 100644 --- a/include/spl.h +++ b/include/spl.h @@ -473,10 +473,14 @@ void spl_set_bd(void); * where the start of the U-Boot image has been written to. * * @mmc: struct mmc that describes the devie where U-Boot resides + * @hw_part: The eMMC hardware partition where U-Boot is by default. + * @raw_part: The software partition where U-Boot is by default. * @raw_sect: The raw sector number where U-Boot is by default. * Return: The raw sector location that U-Boot resides at */ unsigned long spl_mmc_get_uboot_raw_sector(struct mmc *mmc, + unsigned long hw_part, + unsigned long raw_part, unsigned long raw_sect);
/**
The eMMC HW partition 0 and 7 both mean USER HW partition. Use this as a mean of propagating A/B copy selection within USER HW partition. The spl_mmc_get_uboot_raw_sector() can detect that a USER HW partition is in use and based on whether it is 0 or 7, select appropriate sector to load from.
Signed-off-by: Fedor Ross fedor.ross@ifm.com Signed-off-by: Marek Vasut marex@denx.de --- Cc: Jaehoon Chung jh80.chung@samsung.com Cc: Peng Fan peng.fan@nxp.com --- V2: Initialize the part variable, else it is passed uninitialized to mmc_load_image_raw_sector(..raw_sect + spl_mmc_raw_uboot_offset(part)); and that prevents pine64 from booting. Interestingly enough, there is no warning emitted, which sounds like a compiler bug. V3: Update on u-boot/master --- common/spl/spl_mmc.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c index f1f87d78ba7..865c395b9fe 100644 --- a/common/spl/spl_mmc.c +++ b/common/spl/spl_mmc.c @@ -412,7 +412,7 @@ int spl_mmc_load(struct spl_image_info *spl_image, static struct mmc *mmc; u32 boot_mode; int err = 0; - __maybe_unused int hw_part = 0; + __maybe_unused int part = 0, hw_part = 0; int mmc_dev;
/* Perform peripheral init only once for an mmc device */ @@ -437,11 +437,12 @@ int spl_mmc_load(struct spl_image_info *spl_image, switch (boot_mode) { case MMCSD_MODE_EMMCBOOT: hw_part = spl_mmc_emmc_boot_partition(mmc); + part = hw_part == 7 ? 0 : hw_part;
if (CONFIG_IS_ENABLED(MMC_TINY)) - err = mmc_switch_part(mmc, hw_part); + err = mmc_switch_part(mmc, part); else - err = blk_dselect_hwpart(mmc_get_blk_desc(mmc), hw_part); + err = blk_dselect_hwpart(mmc_get_blk_desc(mmc), part);
if (err) { #ifdef CONFIG_SPL_LIBCOMMON_SUPPORT @@ -471,7 +472,7 @@ int spl_mmc_load(struct spl_image_info *spl_image, #endif #ifdef CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR err = mmc_load_image_raw_sector(spl_image, bootdev, mmc, - raw_sect + spl_mmc_raw_uboot_offset(hw_part)); + raw_sect + spl_mmc_raw_uboot_offset(part)); if (!err) return err; #endif
On Tuesday 04 April 2023 20:05:15 Marek Vasut wrote:
The eMMC HW partition 0 and 7 both mean USER HW partition.
This is not truth!
Use this as a mean of propagating A/B copy selection within USER HW partition. The spl_mmc_get_uboot_raw_sector() can detect that a USER HW partition is in use and based on whether it is 0 or 7, select appropriate sector to load from.
Signed-off-by: Fedor Ross fedor.ross@ifm.com Signed-off-by: Marek Vasut marex@denx.de
Cc: Jaehoon Chung jh80.chung@samsung.com Cc: Peng Fan peng.fan@nxp.com
V2: Initialize the part variable, else it is passed uninitialized to mmc_load_image_raw_sector(..raw_sect + spl_mmc_raw_uboot_offset(part)); and that prevents pine64 from booting. Interestingly enough, there is no warning emitted, which sounds like a compiler bug. V3: Update on u-boot/master
common/spl/spl_mmc.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c index f1f87d78ba7..865c395b9fe 100644 --- a/common/spl/spl_mmc.c +++ b/common/spl/spl_mmc.c @@ -412,7 +412,7 @@ int spl_mmc_load(struct spl_image_info *spl_image, static struct mmc *mmc; u32 boot_mode; int err = 0;
- __maybe_unused int hw_part = 0;
__maybe_unused int part = 0, hw_part = 0; int mmc_dev;
/* Perform peripheral init only once for an mmc device */
@@ -437,11 +437,12 @@ int spl_mmc_load(struct spl_image_info *spl_image, switch (boot_mode) { case MMCSD_MODE_EMMCBOOT: hw_part = spl_mmc_emmc_boot_partition(mmc);
part = hw_part == 7 ? 0 : hw_part;
... and so this change will break accessing eMMC part 7 / GP4.
if (CONFIG_IS_ENABLED(MMC_TINY))
err = mmc_switch_part(mmc, hw_part);
err = mmc_switch_part(mmc, part);
err = blk_dselect_hwpart(mmc_get_blk_desc(mmc), hw_part);
err = blk_dselect_hwpart(mmc_get_blk_desc(mmc), part);if (err) {
#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT @@ -471,7 +472,7 @@ int spl_mmc_load(struct spl_image_info *spl_image, #endif #ifdef CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR err = mmc_load_image_raw_sector(spl_image, bootdev, mmc,
raw_sect + spl_mmc_raw_uboot_offset(hw_part));
raw_sect + spl_mmc_raw_uboot_offset(part));#endif
2.39.2
On 4/4/23 21:25, Pali Rohár wrote:
On Tuesday 04 April 2023 20:05:15 Marek Vasut wrote:
The eMMC HW partition 0 and 7 both mean USER HW partition.
This is not truth!
Can you please provide further details to back your claim ?
This kind of screaming feedback with zero additional information is worthless, sorry. Before you proceed with any further replies, please have a look at JEDEC JESD84-B51 section 7.4.69 PARTITION_CONFIG Bit[5:3] BOOT_PARTITION_ENABLE , this is what is being discussed here. Value 1/2 is either BOOT HW partition, value 0/7 is treated as USER HW partition by the boot code.
On Wednesday 05 April 2023 00:11:17 Marek Vasut wrote:
On 4/4/23 21:25, Pali Rohár wrote:
On Tuesday 04 April 2023 20:05:15 Marek Vasut wrote:
The eMMC HW partition 0 and 7 both mean USER HW partition.
This is not truth!
Can you please provide further details to back your claim ?
Yes, see a patch with explanation which I meanwhile sent: https://patchwork.ozlabs.org/project/uboot/patch/20230404202805.8523-1-pali@...
This kind of screaming feedback with zero additional information is worthless, sorry. Before you proceed with any further replies, please have a look at JEDEC JESD84-B51 section 7.4.69 PARTITION_CONFIG Bit[5:3] BOOT_PARTITION_ENABLE , this is what is being discussed here. Value 1/2 is either BOOT HW partition, value 0/7 is treated as USER HW partition by the boot code.
On 4/5/23 00:16, Pali Rohár wrote:
On Wednesday 05 April 2023 00:11:17 Marek Vasut wrote:
On 4/4/23 21:25, Pali Rohár wrote:
On Tuesday 04 April 2023 20:05:15 Marek Vasut wrote:
The eMMC HW partition 0 and 7 both mean USER HW partition.
This is not truth!
Can you please provide further details to back your claim ?
Yes, see a patch with explanation which I meanwhile sent: https://patchwork.ozlabs.org/project/uboot/patch/20230404202805.8523-1-pali@...
This very much adds a comment that looks like the content of the second paragraph of my reply, see below.
It is a good thing you mention the aforementioned patch, since exactly one line past the changes you implemented is the following piece of code:
https://source.denx.de/u-boot/u-boot/-/blob/master/common/spl/spl_mmc.c#L382
" int default_spl_mmc_emmc_boot_partition(struct mmc *mmc) ... part = (mmc->part_config >> 3) & PART_ACCESS_MASK; if (part == 7) part = 0; "
Which maps 7 to 0 .
This kind of screaming feedback with zero additional information is worthless, sorry. Before you proceed with any further replies, please have a look at JEDEC JESD84-B51 section 7.4.69 PARTITION_CONFIG Bit[5:3] BOOT_PARTITION_ENABLE , this is what is being discussed here. Value 1/2 is either BOOT HW partition, value 0/7 is treated as USER HW partition by the boot code.
Please read the paragraph above, I do not see any reply to it and I think that might put the conversation back on track.
On Wednesday 05 April 2023 00:33:02 Marek Vasut wrote:
On 4/5/23 00:16, Pali Rohár wrote:
On Wednesday 05 April 2023 00:11:17 Marek Vasut wrote:
On 4/4/23 21:25, Pali Rohár wrote:
On Tuesday 04 April 2023 20:05:15 Marek Vasut wrote:
The eMMC HW partition 0 and 7 both mean USER HW partition.
This is not truth!
Can you please provide further details to back your claim ?
Yes, see a patch with explanation which I meanwhile sent: https://patchwork.ozlabs.org/project/uboot/patch/20230404202805.8523-1-pali@...
This very much adds a comment that looks like the content of the second paragraph of my reply, see below.
It is a good thing you mention the aforementioned patch, since exactly one line past the changes you implemented is the following piece of code:
https://source.denx.de/u-boot/u-boot/-/blob/master/common/spl/spl_mmc.c#L382
" int default_spl_mmc_emmc_boot_partition(struct mmc *mmc) ... part = (mmc->part_config >> 3) & PART_ACCESS_MASK; if (part == 7) part = 0; "
Which maps 7 to 0 .
This kind of screaming feedback with zero additional information is worthless, sorry. Before you proceed with any further replies, please have a look at JEDEC JESD84-B51 section 7.4.69 PARTITION_CONFIG Bit[5:3] BOOT_PARTITION_ENABLE , this is what is being discussed here. Value 1/2 is either BOOT HW partition, value 0/7 is treated as USER HW partition by the boot code.
Please read the paragraph above, I do not see any reply to it and I think that might put the conversation back on track.
I have read it and the reply with explanation is already there in my first email and also in the linked email.
What you have written above in not truth.
On 4/5/23 00:44, Pali Rohár wrote:
On Wednesday 05 April 2023 00:33:02 Marek Vasut wrote:
On 4/5/23 00:16, Pali Rohár wrote:
On Wednesday 05 April 2023 00:11:17 Marek Vasut wrote:
On 4/4/23 21:25, Pali Rohár wrote:
On Tuesday 04 April 2023 20:05:15 Marek Vasut wrote:
The eMMC HW partition 0 and 7 both mean USER HW partition.
This is not truth!
Can you please provide further details to back your claim ?
Yes, see a patch with explanation which I meanwhile sent: https://patchwork.ozlabs.org/project/uboot/patch/20230404202805.8523-1-pali@...
This very much adds a comment that looks like the content of the second paragraph of my reply, see below.
It is a good thing you mention the aforementioned patch, since exactly one line past the changes you implemented is the following piece of code:
https://source.denx.de/u-boot/u-boot/-/blob/master/common/spl/spl_mmc.c#L382
" int default_spl_mmc_emmc_boot_partition(struct mmc *mmc) ... part = (mmc->part_config >> 3) & PART_ACCESS_MASK; if (part == 7) part = 0; "
Which maps 7 to 0 .
This kind of screaming feedback with zero additional information is worthless, sorry. Before you proceed with any further replies, please have a look at JEDEC JESD84-B51 section 7.4.69 PARTITION_CONFIG Bit[5:3] BOOT_PARTITION_ENABLE , this is what is being discussed here. Value 1/2 is either BOOT HW partition, value 0/7 is treated as USER HW partition by the boot code.
Please read the paragraph above, I do not see any reply to it and I think that might put the conversation back on track.
I have read it and the reply with explanation is already there in my first email and also in the linked email.
What you have written above in not truth.
Very well, I think I will defer further judgement to the MMC maintainer and stop participating in this thread. Constructive feedback is welcome.
On Wednesday 05 April 2023 01:03:49 Marek Vasut wrote:
On 4/5/23 00:44, Pali Rohár wrote:
On Wednesday 05 April 2023 00:33:02 Marek Vasut wrote:
On 4/5/23 00:16, Pali Rohár wrote:
On Wednesday 05 April 2023 00:11:17 Marek Vasut wrote:
On 4/4/23 21:25, Pali Rohár wrote:
On Tuesday 04 April 2023 20:05:15 Marek Vasut wrote: > The eMMC HW partition 0 and 7 both mean USER HW partition.
This is not truth!
Can you please provide further details to back your claim ?
Yes, see a patch with explanation which I meanwhile sent: https://patchwork.ozlabs.org/project/uboot/patch/20230404202805.8523-1-pali@...
This very much adds a comment that looks like the content of the second paragraph of my reply, see below.
It is a good thing you mention the aforementioned patch, since exactly one line past the changes you implemented is the following piece of code:
https://source.denx.de/u-boot/u-boot/-/blob/master/common/spl/spl_mmc.c#L382
" int default_spl_mmc_emmc_boot_partition(struct mmc *mmc) ... part = (mmc->part_config >> 3) & PART_ACCESS_MASK; if (part == 7) part = 0; "
Which maps 7 to 0 .
This kind of screaming feedback with zero additional information is worthless, sorry. Before you proceed with any further replies, please have a look at JEDEC JESD84-B51 section 7.4.69 PARTITION_CONFIG Bit[5:3] BOOT_PARTITION_ENABLE , this is what is being discussed here. Value 1/2 is either BOOT HW partition, value 0/7 is treated as USER HW partition by the boot code.
Please read the paragraph above, I do not see any reply to it and I think that might put the conversation back on track.
I have read it and the reply with explanation is already there in my first email and also in the linked email.
What you have written above in not truth.
Very well, I think I will defer further judgement to the MMC maintainer and stop participating in this thread. Constructive feedback is welcome.
I see that this is your standard trolling reaction for a very long time if you do not have any relevant argument; or when you are lazy to read replies from other people.
On Wed, Apr 05, 2023 at 01:09:35AM +0200, Pali Rohár wrote:
On Wednesday 05 April 2023 01:03:49 Marek Vasut wrote:
On 4/5/23 00:44, Pali Rohár wrote:
On Wednesday 05 April 2023 00:33:02 Marek Vasut wrote:
On 4/5/23 00:16, Pali Rohár wrote:
On Wednesday 05 April 2023 00:11:17 Marek Vasut wrote:
On 4/4/23 21:25, Pali Rohár wrote: > On Tuesday 04 April 2023 20:05:15 Marek Vasut wrote: > > The eMMC HW partition 0 and 7 both mean USER HW partition. > > This is not truth!
Can you please provide further details to back your claim ?
Yes, see a patch with explanation which I meanwhile sent: https://patchwork.ozlabs.org/project/uboot/patch/20230404202805.8523-1-pali@...
Please note that I'm not sure the comments in this patch are a correct reflection of what the code does, or is supposed to be doing. I need to go and study everything there again.
This very much adds a comment that looks like the content of the second paragraph of my reply, see below.
It is a good thing you mention the aforementioned patch, since exactly one line past the changes you implemented is the following piece of code:
https://source.denx.de/u-boot/u-boot/-/blob/master/common/spl/spl_mmc.c#L382
" int default_spl_mmc_emmc_boot_partition(struct mmc *mmc) ... part = (mmc->part_config >> 3) & PART_ACCESS_MASK; if (part == 7) part = 0; "
Which maps 7 to 0 .
This kind of screaming feedback with zero additional information is worthless, sorry. Before you proceed with any further replies, please have a look at JEDEC JESD84-B51 section 7.4.69 PARTITION_CONFIG Bit[5:3] BOOT_PARTITION_ENABLE , this is what is being discussed here. Value 1/2 is either BOOT HW partition, value 0/7 is treated as USER HW partition by the boot code.
Please read the paragraph above, I do not see any reply to it and I think that might put the conversation back on track.
I have read it and the reply with explanation is already there in my first email and also in the linked email.
What you have written above in not truth.
Very well, I think I will defer further judgement to the MMC maintainer and stop participating in this thread. Constructive feedback is welcome.
I see that this is your standard trolling reaction for a very long time if you do not have any relevant argument; or when you are lazy to read replies from other people.
That's uncalled for and rude, Pali. If you don't have anything further to add, technically, there's no need nor justification for such language.
participants (3)
-
Marek Vasut -
Pali Rohár -
Tom Rini