* [U-Boot] Fwd: Re: [PATCH 2/3] env_mmc: support env partition setup in runtime [not found] <53AA7D53.4030405@compulab.co.il> @ 2014-07-03 8:36 ` Igor Grinberg 2014-07-15 8:00 ` Igor Grinberg 0 siblings, 1 reply; 2+ messages in thread From: Igor Grinberg @ 2014-07-03 8:36 UTC (permalink / raw) To: u-boot Hi Pantelis, Tom, Apparently, Dmitry has sent the message in html format... Resending now... Sorry for that... -------- Original Message -------- Subject: Re: [U-Boot] [PATCH 2/3] env_mmc: support env partition setup in runtime Date: Wed, 25 Jun 2014 10:42:11 +0300 From: Dmitry Lifshitz <lifshitz@compulab.co.il> To: Pantelis Antoniou <pantelis.antoniou@gmail.com> CC: u-boot at lists.denx.de, Tom Rini <trini@ti.com>, Igor Grinberg <grinberg@compulab.co.il> Hi Pantelis, On 06/12/2014 06:10 PM, Pantelis Antoniou wrote: > Hi Dmitry, > > I took a good look at the patch and there's a problem. > > On Apr 27, 2014, at 1:18 PM, Dmitry Lifshitz wrote: > >> Add callback with __weak annotation to allow setup of environment >> partition number in runtime from a board file. >> >> Signed-off-by: Dmitry Lifshitz <lifshitz@compulab.co.il> >> Signed-off-by: Igor Grinberg <grinberg@compulab.co.il> >> --- >> common/env_mmc.c | 35 ++++++++++++++++++++++++++--------- >> 1 files changed, 26 insertions(+), 9 deletions(-) >> >> diff --git a/common/env_mmc.c b/common/env_mmc.c >> index 045428c..5d4b5f4 100644 >> --- a/common/env_mmc.c >> +++ b/common/env_mmc.c >> @@ -62,6 +62,30 @@ int env_init(void) >> return 0; >> } >> >> + >> +#ifdef CONFIG_SYS_MMC_ENV_PART >> +__weak uint mmc_get_env_part(struct mmc *mmc) >> +{ >> + return CONFIG_SYS_MMC_ENV_PART; >> +} >> + >> +static int mmc_set_env_part(struct mmc *mmc) >> +{ >> + uint part = mmc_get_env_part(mmc); >> + >> + if (part != mmc->part_num) { >> + if (mmc_switch_part(CONFIG_SYS_MMC_ENV_DEV, part)) { >> + puts("MMC partition switch failed\n"); >> + return -1; >> + } >> + } >> + >> + return 0; >> +} >> +#else >> +static inline int mmc_set_env_part(struct mmc *mmc) {return 0; }; >> +#endif >> + >> static int init_mmc_for_env(struct mmc *mmc) >> { >> if (!mmc) { >> @@ -74,15 +98,8 @@ static int init_mmc_for_env(struct mmc *mmc) >> return -1; >> } >> > Just before this hunk, we have this: > >> #ifdef CONFIG_SYS_MMC_ENV_PART >> int dev = CONFIG_SYS_MMC_ENV_DEV; >> >> #ifdef CONFIG_SPL_BUILD >> dev = 0; >> #endif >> #endif >> > This appears to be broken for SPL in case that CONFIG_SYS_MMC_ENV_DEV is not 0. Exactly as it was broken before the patch, right? > > SPL hardcoded dev to 0, while mmc_switch_part is implicitly operating on > CONFIG_SYS_MMC_ENV_DEV. > > Please rework it so that the SPL case is unaffected. This patch does not change the behavior and its purpose is not fixing the current code. The bug describe can be fixed by later patch and possibly will require additional review and testing. Given that this patch does not change the behavior, can it be accepted please ? Regards, Dmitry >> -#ifdef CONFIG_SYS_MMC_ENV_PART >> - if (CONFIG_SYS_MMC_ENV_PART != mmc->part_num) { >> - if (mmc_switch_part(CONFIG_SYS_MMC_ENV_DEV, >> - CONFIG_SYS_MMC_ENV_PART)) { >> - puts("MMC partition switch failed\n"); >> - return -1; >> - } >> - } >> -#endif >> + if (mmc_set_env_part(mmc)) >> + return -1; >> >> return 0; >> } >> -- >> 1.7.5.4 > > ^ permalink raw reply [flat|nested] 2+ messages in thread
* [U-Boot] Fwd: Re: [PATCH 2/3] env_mmc: support env partition setup in runtime 2014-07-03 8:36 ` [U-Boot] Fwd: Re: [PATCH 2/3] env_mmc: support env partition setup in runtime Igor Grinberg @ 2014-07-15 8:00 ` Igor Grinberg 0 siblings, 0 replies; 2+ messages in thread From: Igor Grinberg @ 2014-07-15 8:00 UTC (permalink / raw) To: u-boot ping. On 07/03/14 11:36, Igor Grinberg wrote: > Hi Pantelis, Tom, > > Apparently, Dmitry has sent the message in html format... > > Resending now... > Sorry for that... > > > -------- Original Message -------- > Subject: Re: [U-Boot] [PATCH 2/3] env_mmc: support env partition setup in runtime > Date: Wed, 25 Jun 2014 10:42:11 +0300 > From: Dmitry Lifshitz <lifshitz@compulab.co.il> > To: Pantelis Antoniou <pantelis.antoniou@gmail.com> > CC: u-boot at lists.denx.de, Tom Rini <trini@ti.com>, Igor Grinberg <grinberg@compulab.co.il> > > > > Hi Pantelis, > > On 06/12/2014 06:10 PM, Pantelis Antoniou wrote: >> Hi Dmitry, >> >> I took a good look at the patch and there's a problem. >> >> On Apr 27, 2014, at 1:18 PM, Dmitry Lifshitz wrote: >> >>> Add callback with __weak annotation to allow setup of environment >>> partition number in runtime from a board file. >>> >>> Signed-off-by: Dmitry Lifshitz <lifshitz@compulab.co.il> >>> Signed-off-by: Igor Grinberg <grinberg@compulab.co.il> >>> --- >>> common/env_mmc.c | 35 ++++++++++++++++++++++++++--------- >>> 1 files changed, 26 insertions(+), 9 deletions(-) >>> >>> diff --git a/common/env_mmc.c b/common/env_mmc.c >>> index 045428c..5d4b5f4 100644 >>> --- a/common/env_mmc.c >>> +++ b/common/env_mmc.c >>> @@ -62,6 +62,30 @@ int env_init(void) >>> return 0; >>> } >>> >>> + >>> +#ifdef CONFIG_SYS_MMC_ENV_PART >>> +__weak uint mmc_get_env_part(struct mmc *mmc) >>> +{ >>> + return CONFIG_SYS_MMC_ENV_PART; >>> +} >>> + >>> +static int mmc_set_env_part(struct mmc *mmc) >>> +{ >>> + uint part = mmc_get_env_part(mmc); >>> + >>> + if (part != mmc->part_num) { >>> + if (mmc_switch_part(CONFIG_SYS_MMC_ENV_DEV, part)) { >>> + puts("MMC partition switch failed\n"); >>> + return -1; >>> + } >>> + } >>> + >>> + return 0; >>> +} >>> +#else >>> +static inline int mmc_set_env_part(struct mmc *mmc) {return 0; }; >>> +#endif >>> + >>> static int init_mmc_for_env(struct mmc *mmc) >>> { >>> if (!mmc) { >>> @@ -74,15 +98,8 @@ static int init_mmc_for_env(struct mmc *mmc) >>> return -1; >>> } >>> >> Just before this hunk, we have this: >> >>> #ifdef CONFIG_SYS_MMC_ENV_PART >>> int dev = CONFIG_SYS_MMC_ENV_DEV; >>> >>> #ifdef CONFIG_SPL_BUILD >>> dev = 0; >>> #endif >>> #endif >>> >> This appears to be broken for SPL in case that CONFIG_SYS_MMC_ENV_DEV is not 0. > > Exactly as it was broken before the patch, right? > >> >> SPL hardcoded dev to 0, while mmc_switch_part is implicitly operating on >> CONFIG_SYS_MMC_ENV_DEV. >> >> Please rework it so that the SPL case is unaffected. > > This patch does not change the behavior and its purpose > is not fixing the current code. > > The bug describe can be fixed by later patch and possibly > will require additional review and testing. > > Given that this patch does not change the behavior, can it be accepted please ? > > Regards, > > Dmitry > >>> -#ifdef CONFIG_SYS_MMC_ENV_PART >>> - if (CONFIG_SYS_MMC_ENV_PART != mmc->part_num) { >>> - if (mmc_switch_part(CONFIG_SYS_MMC_ENV_DEV, >>> - CONFIG_SYS_MMC_ENV_PART)) { >>> - puts("MMC partition switch failed\n"); >>> - return -1; >>> - } >>> - } >>> -#endif >>> + if (mmc_set_env_part(mmc)) >>> + return -1; >>> >>> return 0; >>> } >>> -- >>> 1.7.5.4 >> >> > > > > _______________________________________________ > U-Boot mailing list > U-Boot at lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot > -- Regards, Igor. ^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2014-07-15 8:00 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <53AA7D53.4030405@compulab.co.il>
2014-07-03 8:36 ` [U-Boot] Fwd: Re: [PATCH 2/3] env_mmc: support env partition setup in runtime Igor Grinberg
2014-07-15 8:00 ` Igor Grinberg
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox