| Message ID | 1409761938-27902-1-git-send-email-pab@pabigot.com |
|---|---|
| State | Superseded |
| Headers | show |
On 09/03/2014 10:32 AM, Peter A. Bigot wrote: > The code to set the MMC partition uses an weak function to obtain the > correct partition number. Use that instead of the compile-time default > when deciding whether it needs to switch back. Yes, this clearly fixes a bug. Can you also please add a Fixes: tag that refers to the commit which introduced the problem (i.e. which updated mmc_set_env_part() to call mmc_get_env_part(), but forgot to update fini_mmc_for_env() to match.
On 09/03/2014 11:52 AM, Stephen Warren wrote: > On 09/03/2014 10:32 AM, Peter A. Bigot wrote: >> The code to set the MMC partition uses an weak function to obtain the >> correct partition number. Use that instead of the compile-time default >> when deciding whether it needs to switch back. > > Yes, this clearly fixes a bug. > > Can you also please add a Fixes: tag that refers to the commit which > introduced the problem (i.e. which updated mmc_set_env_part() to call > mmc_get_env_part(), but forgot to update fini_mmc_for_env() to match. Done. If this tag is important enough to ask people to add it and resubmit their patches with no other changes, it should probably be described at http://www.denx.de/wiki/view/U-Boot/Patches#Review_Process_Git_Tags and suggested in the section on general patch submission rules, so the poor contributor might have a chance of being able to avoid the rework. Peter
On 09/03/2014 11:30 AM, Peter A. Bigot wrote: > On 09/03/2014 11:52 AM, Stephen Warren wrote: >> On 09/03/2014 10:32 AM, Peter A. Bigot wrote: >>> The code to set the MMC partition uses an weak function to obtain the >>> correct partition number. Use that instead of the compile-time default >>> when deciding whether it needs to switch back. >> >> Yes, this clearly fixes a bug. >> >> Can you also please add a Fixes: tag that refers to the commit which >> introduced the problem (i.e. which updated mmc_set_env_part() to call >> mmc_get_env_part(), but forgot to update fini_mmc_for_env() to match. > > Done. > > If this tag is important enough to ask people to add it and resubmit > their patches with no other changes, it should probably be described at > http://www.denx.de/wiki/view/U-Boot/Patches#Review_Process_Git_Tags and > suggested in the section on general patch submission rules, so the poor > contributor might have a chance of being able to avoid the rework. I'd expect that if the only issue was a patch was a missing fixes line, the person applying the patch could manually edit it in when applying the patch, so all the contributor would have to do is reply to the email with the desired content. Still, different committers have different levels of tolerance for this, so YMMV!
On 09/03/2014 12:46 PM, Stephen Warren wrote: > On 09/03/2014 11:30 AM, Peter A. Bigot wrote: >> On 09/03/2014 11:52 AM, Stephen Warren wrote: >>> On 09/03/2014 10:32 AM, Peter A. Bigot wrote: >>>> The code to set the MMC partition uses an weak function to obtain the >>>> correct partition number. Use that instead of the compile-time >>>> default >>>> when deciding whether it needs to switch back. >>> >>> Yes, this clearly fixes a bug. >>> >>> Can you also please add a Fixes: tag that refers to the commit which >>> introduced the problem (i.e. which updated mmc_set_env_part() to call >>> mmc_get_env_part(), but forgot to update fini_mmc_for_env() to match. >> >> Done. >> >> If this tag is important enough to ask people to add it and resubmit >> their patches with no other changes, it should probably be described at >> http://www.denx.de/wiki/view/U-Boot/Patches#Review_Process_Git_Tags and >> suggested in the section on general patch submission rules, so the poor >> contributor might have a chance of being able to avoid the rework. > > I'd expect that if the only issue was a patch was a missing fixes > line, the person applying the patch could manually edit it in when > applying the patch, so all the contributor would have to do is reply > to the email with the desired content. Still, different committers > have different levels of tolerance for this, so YMMV! Ah; I'd forgotten that patchwork collects that sort of thing. The need to provide the tag should still be described in the patch process wiki pages, but replying would have been an easier solution if I'd known that doing so would have satisfied your request to add it. Peter
diff --git a/common/env_mmc.c b/common/env_mmc.c index a7621a8..14648e3 100644 --- a/common/env_mmc.c +++ b/common/env_mmc.c @@ -113,7 +113,7 @@ static void fini_mmc_for_env(struct mmc *mmc) #ifdef CONFIG_SPL_BUILD dev = 0; #endif - if (CONFIG_SYS_MMC_ENV_PART != mmc->part_num) + if (mmc_get_env_part(mmc) != mmc->part_num) mmc_switch_part(dev, mmc->part_num); #endif }
The code to set the MMC partition uses an weak function to obtain the correct partition number. Use that instead of the compile-time default when deciding whether it needs to switch back. Signed-off-by: Peter A. Bigot <pab@pabigot.com> --- V2: * Preserve desired behavior of avoiding diagnostic when no HW partition supported * Supersedes https://patchwork.ozlabs.org/patch/385355/ common/env_mmc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)