Message ID | 1333463043-11495-1-git-send-email-l.majewski@samsung.com |
---|---|
State | Superseded, archived |
Delegated to: | Andy Fleming |
Headers | show |
Hi Lukasz, On 3 April 2012 23:24, Lukasz Majewski <l.majewski@samsung.com> wrote: > This code adds call to mmc_init(), for partition related commands (e.g. > fatls, fatinfo etc.). > > It is safe to call mmc_init() multiple times since mmc->has_init flag > prevents from multiple initialization. > > The FAT related code calls get_dev high level method and then uses > elements from mmc->block_dev, which is uninitialized until the mmc_init > (and thereof mmc_startup) is called. > > This problem appears on boards, which don't use mmc as the default > place for envs > > Signed-off-by: Lukasz Majewski <l.majewski@samsung.com> > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com> > Cc: Andy Fleming <afleming@gmail.com> > > --- > Test HW: > - GONI S5PC110 > - Universal C210 (Exynos4) > --- > drivers/mmc/mmc.c | 6 +++++- > 1 files changed, 5 insertions(+), 1 deletions(-) > > diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c > index 618960e..1fa90e7 100644 > --- a/drivers/mmc/mmc.c > +++ b/drivers/mmc/mmc.c > @@ -1305,8 +1305,12 @@ int mmc_register(struct mmc *mmc) > block_dev_desc_t *mmc_get_dev(int dev) > { > struct mmc *mmc = find_mmc_device(dev); > + if (mmc) { > + mmc_init(mmc); > + return &mmc->block_dev; > + } > > - return mmc ? &mmc->block_dev : NULL; > + return NULL; > } > #endif > I think if (!mmc) return NULL; mmc_init(mmc); return &mmc->block_dev; is better. How you think? Thanks. Minkyu Kang.
Hi, Minkyu > Hi Lukasz, > > On 3 April 2012 23:24, Lukasz Majewski <l.majewski@samsung.com> wrote: > > This code adds call to mmc_init(), for partition related commands > > (e.g. fatls, fatinfo etc.). > > > > It is safe to call mmc_init() multiple times since mmc->has_init > > flag prevents from multiple initialization. > > > > The FAT related code calls get_dev high level method and then uses > > elements from mmc->block_dev, which is uninitialized until the > > mmc_init (and thereof mmc_startup) is called. > > > > This problem appears on boards, which don't use mmc as the default > > place for envs > > > > Signed-off-by: Lukasz Majewski <l.majewski@samsung.com> > > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com> > > Cc: Andy Fleming <afleming@gmail.com> > > > > --- > > Test HW: > > - GONI S5PC110 > > - Universal C210 (Exynos4) > > --- > > drivers/mmc/mmc.c | 6 +++++- > > 1 files changed, 5 insertions(+), 1 deletions(-) > > > > diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c > > index 618960e..1fa90e7 100644 > > --- a/drivers/mmc/mmc.c > > +++ b/drivers/mmc/mmc.c > > @@ -1305,8 +1305,12 @@ int mmc_register(struct mmc *mmc) > > block_dev_desc_t *mmc_get_dev(int dev) > > { > > struct mmc *mmc = find_mmc_device(dev); > > + if (mmc) { > > + mmc_init(mmc); > > + return &mmc->block_dev; > > + } > > > > - return mmc ? &mmc->block_dev : NULL; > > + return NULL; > > } > > #endif > > > > I think > > if (!mmc) > return NULL; > > mmc_init(mmc); > return &mmc->block_dev; > > is better. > How you think? Yes, it seems also more readable for me. But anyway Andy shall express his opinion.
diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c index 618960e..1fa90e7 100644 --- a/drivers/mmc/mmc.c +++ b/drivers/mmc/mmc.c @@ -1305,8 +1305,12 @@ int mmc_register(struct mmc *mmc) block_dev_desc_t *mmc_get_dev(int dev) { struct mmc *mmc = find_mmc_device(dev); + if (mmc) { + mmc_init(mmc); + return &mmc->block_dev; + } - return mmc ? &mmc->block_dev : NULL; + return NULL; } #endif