* [U-Boot] [PATCH] mmc: support the revision check for eMMC4.5 [not found] <4F71647C.2030508@samsung.com> @ 2012-05-07 22:16 ` Andy Fleming 2012-05-08 4:19 ` Jaehoon Chung 0 siblings, 1 reply; 2+ messages in thread From: Andy Fleming @ 2012-05-07 22:16 UTC (permalink / raw) To: u-boot On Tue, Mar 27, 2012 at 1:55 AM, Jaehoon Chung <jh80.chung@samsung.com> wrote: > eMMC card is introduced the eMMC4.5. > But now eMMC card is checked up to eMMC4.0. > This patch is supported until eMMC4.5 > > Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com> > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com> > --- > ?common/cmd_mmc.c ?| ? ?5 ++++- > ?drivers/mmc/mmc.c | ? 25 +++++++++++++++++++++++++ > ?include/mmc.h ? ? | ? ?8 ++++++++ > ?3 files changed, 37 insertions(+), 1 deletions(-) > > diff --git a/common/cmd_mmc.c b/common/cmd_mmc.c > index 8f13c22..ff150ca 100644 > --- a/common/cmd_mmc.c > +++ b/common/cmd_mmc.c > @@ -106,7 +106,10 @@ static void print_mmcinfo(struct mmc *mmc) > ? ? ? ?printf("Rd Block Len: %d\n", mmc->read_bl_len); > > ? ? ? ?printf("%s version %d.%d\n", IS_SD(mmc) ? "SD" : "MMC", > - ? ? ? ? ? ? ? ? ? ? ? (mmc->version >> 4) & 0xf, mmc->version & 0xf); > + ? ? ? ? ? ? ? ? ? ? ? (mmc->version >> 4) & 0xf, > + ? ? ? ? ? ? ? ? ? ? ? (mmc->version & 0xf) == EXT_CSD_REV_1_5 ? > + ? ? ? ? ? ? ? ? ? ? ? 41 :((mmc->version & 0xf) > EXT_CSD_REV_1_5 ? > + ? ? ? ? ? ? ? ? ? ? ? (mmc->version & 0xf) - 1 : (mmc->version & 0xf))); This is very confusing. Let's pull some of that math and control code out of the printf invocation. > > ? ? ? ?printf("High Capacity: %s\n", mmc->high_capacity ? "Yes" : "No"); > ? ? ? ?puts("Capacity: "); > diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c > index 49c3349..e035012 100644 > --- a/drivers/mmc/mmc.c > +++ b/drivers/mmc/mmc.c > @@ -651,6 +651,31 @@ int mmc_change_freq(struct mmc *mmc) > ? ? ? ?if (err) > ? ? ? ? ? ? ? ?return err; > > + ? ? ? switch (ext_csd[EXT_CSD_REV]) { > + ? ? ? case EXT_CSD_REV_1_0: > + ? ? ? ? ? ? ? mmc->version |= EXT_CSD_REV_1_0; > + ? ? ? ? ? ? ? break; > + ? ? ? case EXT_CSD_REV_1_1: > + ? ? ? ? ? ? ? mmc->version |= EXT_CSD_REV_1_1; > + ? ? ? ? ? ? ? break; > + ? ? ? case EXT_CSD_REV_1_2: > + ? ? ? ? ? ? ? mmc->version |= EXT_CSD_REV_1_2; > + ? ? ? ? ? ? ? break; > + ? ? ? case EXT_CSD_REV_1_3: > + ? ? ? ? ? ? ? mmc->version |= EXT_CSD_REV_1_3; > + ? ? ? ? ? ? ? break; > + ? ? ? case EXT_CSD_REV_1_5: > + ? ? ? ? ? ? ? mmc->version |= EXT_CSD_REV_1_5; > + ? ? ? ? ? ? ? break; > + ? ? ? case EXT_CSD_REV_1_6: > + ? ? ? ? ? ? ? mmc->version |= EXT_CSD_REV_1_6; > + ? ? ? ? ? ? ? break; > + ? ? ? case EXT_CSD_REV_1_4: > + ? ? ? default: > + ? ? ? ? ? ? ? printf("Unknown revision - %x\n", ext_csd[EXT_CSD_REV]); > + ? ? ? ? ? ? ? return 0; > + ? ? ? } > + This seems broken, and the wrong place to do this. the mmc->version field will already be set based on the CSD, to values that overlap with the ones you are ORing, here. Also, the case statement appears to serve no function, other than to paradoxically identify that version 1.4 is unknown. I think that, if we want to do any version checking, we should do it in mmc_startup(). I suspect that the version information obtained here is only appropriate for certain versions of the spec, so that should be checked before adding in the information. Andy ^ permalink raw reply [flat|nested] 2+ messages in thread
* [U-Boot] [PATCH] mmc: support the revision check for eMMC4.5 2012-05-07 22:16 ` [U-Boot] [PATCH] mmc: support the revision check for eMMC4.5 Andy Fleming @ 2012-05-08 4:19 ` Jaehoon Chung 0 siblings, 0 replies; 2+ messages in thread From: Jaehoon Chung @ 2012-05-08 4:19 UTC (permalink / raw) To: u-boot Hi Andy. On 05/08/2012 07:16 AM, Andy Fleming wrote: > On Tue, Mar 27, 2012 at 1:55 AM, Jaehoon Chung <jh80.chung@samsung.com> wrote: >> eMMC card is introduced the eMMC4.5. >> But now eMMC card is checked up to eMMC4.0. >> This patch is supported until eMMC4.5 >> >> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com> >> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com> >> --- >> common/cmd_mmc.c | 5 ++++- >> drivers/mmc/mmc.c | 25 +++++++++++++++++++++++++ >> include/mmc.h | 8 ++++++++ >> 3 files changed, 37 insertions(+), 1 deletions(-) >> >> diff --git a/common/cmd_mmc.c b/common/cmd_mmc.c >> index 8f13c22..ff150ca 100644 >> --- a/common/cmd_mmc.c >> +++ b/common/cmd_mmc.c >> @@ -106,7 +106,10 @@ static void print_mmcinfo(struct mmc *mmc) >> printf("Rd Block Len: %d\n", mmc->read_bl_len); >> >> printf("%s version %d.%d\n", IS_SD(mmc) ? "SD" : "MMC", >> - (mmc->version >> 4) & 0xf, mmc->version & 0xf); >> + (mmc->version >> 4) & 0xf, >> + (mmc->version & 0xf) == EXT_CSD_REV_1_5 ? >> + 41 :((mmc->version & 0xf) > EXT_CSD_REV_1_5 ? >> + (mmc->version & 0xf) - 1 : (mmc->version & 0xf))); > > > This is very confusing. Let's pull some of that math and control code > out of the printf invocation. Ok..i will do it. > > >> >> printf("High Capacity: %s\n", mmc->high_capacity ? "Yes" : "No"); >> puts("Capacity: "); >> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c >> index 49c3349..e035012 100644 >> --- a/drivers/mmc/mmc.c >> +++ b/drivers/mmc/mmc.c >> @@ -651,6 +651,31 @@ int mmc_change_freq(struct mmc *mmc) >> if (err) >> return err; >> >> + switch (ext_csd[EXT_CSD_REV]) { >> + case EXT_CSD_REV_1_0: >> + mmc->version |= EXT_CSD_REV_1_0; >> + break; >> + case EXT_CSD_REV_1_1: >> + mmc->version |= EXT_CSD_REV_1_1; >> + break; >> + case EXT_CSD_REV_1_2: >> + mmc->version |= EXT_CSD_REV_1_2; >> + break; >> + case EXT_CSD_REV_1_3: >> + mmc->version |= EXT_CSD_REV_1_3; >> + break; >> + case EXT_CSD_REV_1_5: >> + mmc->version |= EXT_CSD_REV_1_5; >> + break; >> + case EXT_CSD_REV_1_6: >> + mmc->version |= EXT_CSD_REV_1_6; >> + break; >> + case EXT_CSD_REV_1_4: >> + default: >> + printf("Unknown revision - %x\n", ext_csd[EXT_CSD_REV]); >> + return 0; >> + } >> + > > > This seems broken, and the wrong place to do this. the mmc->version > field will already be set based on the CSD, to values that overlap > with the ones you are ORing, here. Yes..it's already set based on the CSD. but we can't check whether version4.1 or 4.2 or 4.3.. To check More exactly, we need to check CSD structure version in ext_csd register. And also check EXT_CSD_REV[192] in ext_csd register. > > Also, the case statement appears to serve no function, other than to > paradoxically identify that version 1.4 is unknown. > > I think that, if we want to do any version checking, we should do it > in mmc_startup(). I suspect that the version information obtained here > is only appropriate for certain versions of the spec, so that should > be checked before adding in the information. I will resend the patch. Best Regards, Jaehoon Chung > > Andy > _______________________________________________ > U-Boot mailing list > U-Boot at lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot > ^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2012-05-08 4:19 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <4F71647C.2030508@samsung.com>
2012-05-07 22:16 ` [U-Boot] [PATCH] mmc: support the revision check for eMMC4.5 Andy Fleming
2012-05-08 4:19 ` Jaehoon Chung
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.