On Sat, Jul 25, 2020 at 01:38:12PM +0200, Stefan Roese wrote:
Hi Tom,
On 24.07.20 23:13, Tom Rini wrote:
The code around CONFIG_SYS_MMC_ENV_PART has been untested since merge. This can be seen by it referencing 'mmc->part_num' which was migrated elsewhere prior to this code being merged.
I'm seeing that CONFIG_SYS_MMC_ENV_PART is also mentioned in the documentation for this MVEBU cmd:
doc/mvebu/cmd/bubt.txt
So I hesitate a bit to remove it completely from this command (even though I personally have never used it). Could you perhaps send me a link a patch / commit, where 'mmc->part_num' has been migrated?
It was changed in: commit 873cc1d7775ed5de07e6722c7ff423080c2e8f71 Author: Stephen Warren swarren@nvidia.com AuthorDate: Mon Dec 7 11:38:49 2015 -0700 Commit: Tom Rini trini@konsulko.com CommitDate: Wed Jan 13 21:05:19 2016 -0500
mmc: store hwpart in the block device
This will allow us to have multiple block device structs each referring to the same eMMC device, yet different HW partitions.
For now, there is still a single block device per eMMC device. As before, this block device always accesses whichever HW partition was most recently selected. Clients wishing to make use of multiple block devices referring to different HW partitions can simply take a copy of this block device once it points at the correct HW partition, and use each one as they wish. This feature will be used by the next patch.
In the future, perhaps get_device() could be enhanced to return a dynamically allocated block device struct, to avoid the client needing to copy it in order to maintain multiple block devices. However, this would require all users to be updated to free those block device structs at some point, which is rather a large change.
Most callers of mmc_switch_part() wish to permanently switch the default MMC block device's HW partition. Enhance mmc_switch_part() so that it does this. This removes the need for callers to do this. However, common/env_mmc.c needs to save and restore the current HW partition. Make it do this more explicitly.
Replace use of mmc_switch_part() with mmc_select_hwpart() in order to remove duplicate code that skips the call if that HW partition is already selected.
Signed-off-by: Stephen Warren swarren@nvidia.com Reviewed-by: Tom Rini trini@konsulko.com
Which is before cmd/mvebu/bubt.c was added to mainline. So while I'm sure the command worked in the downstream tree, it wasn't ever tested in mainline. It also means that changing it to mmc->block_dev.hwpart instead of mmc->part_num would also work, but the code would remain untested. Note that I ran in to this in moving CONFIG_SYS_MMC_ENV_PART/DEV to Kconfig and that leading to us always having these variables set to at least 0, so if you're going to fix the code to build please keep that in mind as well and that would should probably drop the #if check and always change to the correct dev/part. Thanks!