Patchwork [U-Boot,v2] mmc: Update "mmc->part_num" when performing a partition switch

login
register
mail settings
Submitter Fabio Estevam
Date June 4, 2013, 7:17 p.m.
Message ID <1370373446-14025-1-git-send-email-fabio.estevam@freescale.com>
Download mbox | patch
Permalink /patch/248847/
State Superseded
Delegated to: Andy Fleming
Headers show

Comments

Fabio Estevam - June 4, 2013, 7:17 p.m.
When running the "save" command several times on a mx6qsabresd we see:

U-Boot > save
Saving Environment to MMC...
Writing to MMC(1)... done
U-Boot > save
Saving Environment to MMC...
MMC partition switch failed
U-Boot > save
Saving Environment to MMC...
Writing to MMC(1)... done
U-Boot > save
Saving Environment to MMC...
MMC partition switch failed
U-Boot > save
Saving Environment to MMC...
Writing to MMC(1)... done
U-Boot > save
Saving Environment to MMC...
MMC partition switch failed

Fix this by updating mmc->part_num inside mmc_switch_part() after a succesful
mmc partition switch.

After this fix, we no longer see the error after the "save" command on a 
mx6qsabresd. Also tested on a mx53loco.

Reported-by: Jason Liu <r64343@freescale.com>

Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
Changes since v1:
- Do the change inside the mmc core

 drivers/mmc/mmc.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)
Stephen Warren - June 4, 2013, 7:21 p.m.
On 06/04/2013 01:17 PM, Fabio Estevam wrote:
> When running the "save" command several times on a mx6qsabresd we see:
> 
> U-Boot > save
> Saving Environment to MMC...
> Writing to MMC(1)... done
> U-Boot > save
> Saving Environment to MMC...
> MMC partition switch failed
> U-Boot > save
> Saving Environment to MMC...
> Writing to MMC(1)... done
> U-Boot > save
> Saving Environment to MMC...
> MMC partition switch failed
> U-Boot > save
> Saving Environment to MMC...
> Writing to MMC(1)... done
> U-Boot > save
> Saving Environment to MMC...
> MMC partition switch failed
> 
> Fix this by updating mmc->part_num inside mmc_switch_part() after a succesful
> mmc partition switch.

This surely has exactly the same issue as the previous version, where
the update of part_num was done inside common/env_mmc.c?
Fabio Estevam - June 4, 2013, 7:30 p.m.
On Tue, Jun 4, 2013 at 4:21 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:

> This surely has exactly the same issue as the previous version, where
> the update of part_num was done inside common/env_mmc.c?

My understading is that the mmc core should update mmc->part_num and I
don't see such update, or am I missing something?

Thanks,

Fabio Estevam
Stephen Warren - June 4, 2013, 7:36 p.m.
On 06/04/2013 01:30 PM, Fabio Estevam wrote:
> On Tue, Jun 4, 2013 at 4:21 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> 
>> This surely has exactly the same issue as the previous version, where
>> the update of part_num was done inside common/env_mmc.c?
> 
> My understading is that the mmc core should update mmc->part_num and I
> don't see such update, or am I missing something?

That seems like a reasonable way for the code to work. However, you'd
need to modify common/env_mmc.c:init_mmc_for_env() so that it saves off
mmc->part_num before switching MMC partitions, so that
fini_mmc_for_env() knows which partition to switch back to. Right now,
it relies on the fact that mmc_switch_part() does not update
mmc->part_num, and hence uses that value to save the previously selected
partition ID.

Also, if you make this change, I think you can update
common/cmd_mmc.c:do_mmcops(), since it will no longer need to update
mmc->part_num after a successful switch.

Patch

diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index 2590f1b..7f568ed 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -680,14 +680,20 @@  static int mmc_change_freq(struct mmc *mmc)
 
 int mmc_switch_part(int dev_num, unsigned int part_num)
 {
+	int ret;
 	struct mmc *mmc = find_mmc_device(dev_num);
 
 	if (!mmc)
 		return -1;
 
-	return mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_PART_CONF,
+	ret = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_PART_CONF,
 			  (mmc->part_config & ~PART_ACCESS_MASK)
 			  | (part_num & PART_ACCESS_MASK));
+
+	if (!ret)
+		mmc->part_num = part_num;
+
+	return ret;
 }
 
 int mmc_getcd(struct mmc *mmc)