| Message ID | 1513857247-15821-4-git-send-email-jjhiblot@ti.com |
|---|---|
| State | Superseded |
| Delegated to: | Jaehoon Chung |
| Headers | show |
| Series | reduce the size of the mmc core | expand |
On 12/21/2017 08:53 PM, Jean-Jacques Hiblot wrote: > mmc_set_card_speed() reads the ext csd to check if switch has been OK. > But it does it only for MMC_HS and MMC_HS_52. Moreover this check is not > really required as there will be a ext csd reading later in the > initialization process to make sure that it succeeded. > So remove this partial verification and save space. Yes, it's saved the space, but it may be affected with the driver strength value. It needs to consider more whether we can remove it. > > Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com> > --- > > drivers/mmc/mmc.c | 22 +++------------------- > 1 file changed, 3 insertions(+), 19 deletions(-) > > diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c > index 67d05c5..7a92718 100644 > --- a/drivers/mmc/mmc.c > +++ b/drivers/mmc/mmc.c > @@ -788,11 +788,8 @@ int mmc_switch(struct mmc *mmc, u8 set, u8 index, u8 value) > > static int mmc_set_card_speed(struct mmc *mmc, enum bus_mode mode) > { > - int err; > int speed_bits; > > - ALLOC_CACHE_ALIGN_BUFFER(u8, test_csd, MMC_MAX_BLOCK_LEN); > - > switch (mode) { > case MMC_HS: > case MMC_HS_52: > @@ -808,23 +805,8 @@ static int mmc_set_card_speed(struct mmc *mmc, enum bus_mode mode) > default: > return -EINVAL; > } > - err = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_HS_TIMING, > + return mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_HS_TIMING, > speed_bits); > - if (err) > - return err; > - > - if ((mode == MMC_HS) || (mode == MMC_HS_52)) { > - /* Now check to see that it worked */ > - err = mmc_send_ext_csd(mmc, test_csd); > - if (err) > - return err; > - > - /* No high-speed support */ > - if (!test_csd[EXT_CSD_HS_TIMING]) > - return -ENOTSUPP; > - } > - > - return 0; > } > > static int mmc_get_capabilities(struct mmc *mmc) > @@ -851,10 +833,12 @@ static int mmc_get_capabilities(struct mmc *mmc) > cardtype = ext_csd[EXT_CSD_CARD_TYPE] & 0x3f; > mmc->cardtype = cardtype; > > +#if CONFIG_IS_ENABLED(MMC_HS200_SUPPORT) > if (cardtype & (EXT_CSD_CARD_TYPE_HS200_1_2V | > EXT_CSD_CARD_TYPE_HS200_1_8V)) { > mmc->card_caps |= MMC_MODE_HS200; > } > +#endif It's not related with your subject.? Best Regards, Jaehoon Chung > if (cardtype & EXT_CSD_CARD_TYPE_52) { > if (cardtype & EXT_CSD_CARD_TYPE_DDR_52) > mmc->card_caps |= MMC_MODE_DDR_52MHz; >
Hi Jaehoon, On 26/12/2017 11:36, Jaehoon Chung wrote: > On 12/21/2017 08:53 PM, Jean-Jacques Hiblot wrote: >> mmc_set_card_speed() reads the ext csd to check if switch has been OK. >> But it does it only for MMC_HS and MMC_HS_52. Moreover this check is not >> really required as there will be a ext csd reading later in the >> initialization process to make sure that it succeeded. >> So remove this partial verification and save space. > Yes, it's saved the space, but it may be affected with the driver strength value. > It needs to consider more whether we can remove it. I'll drop the patch from the series > >> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com> >> --- >> >> drivers/mmc/mmc.c | 22 +++------------------- >> 1 file changed, 3 insertions(+), 19 deletions(-) >> >> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c >> index 67d05c5..7a92718 100644 >> --- a/drivers/mmc/mmc.c >> +++ b/drivers/mmc/mmc.c >> @@ -788,11 +788,8 @@ int mmc_switch(struct mmc *mmc, u8 set, u8 index, u8 value) >> >> static int mmc_set_card_speed(struct mmc *mmc, enum bus_mode mode) >> { >> - int err; >> int speed_bits; >> >> - ALLOC_CACHE_ALIGN_BUFFER(u8, test_csd, MMC_MAX_BLOCK_LEN); >> - >> switch (mode) { >> case MMC_HS: >> case MMC_HS_52: >> @@ -808,23 +805,8 @@ static int mmc_set_card_speed(struct mmc *mmc, enum bus_mode mode) >> default: >> return -EINVAL; >> } >> - err = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_HS_TIMING, >> + return mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_HS_TIMING, >> speed_bits); >> - if (err) >> - return err; >> - >> - if ((mode == MMC_HS) || (mode == MMC_HS_52)) { >> - /* Now check to see that it worked */ >> - err = mmc_send_ext_csd(mmc, test_csd); >> - if (err) >> - return err; >> - >> - /* No high-speed support */ >> - if (!test_csd[EXT_CSD_HS_TIMING]) >> - return -ENOTSUPP; >> - } >> - >> - return 0; >> } >> >> static int mmc_get_capabilities(struct mmc *mmc) >> @@ -851,10 +833,12 @@ static int mmc_get_capabilities(struct mmc *mmc) >> cardtype = ext_csd[EXT_CSD_CARD_TYPE] & 0x3f; >> mmc->cardtype = cardtype; >> >> +#if CONFIG_IS_ENABLED(MMC_HS200_SUPPORT) >> if (cardtype & (EXT_CSD_CARD_TYPE_HS200_1_2V | >> EXT_CSD_CARD_TYPE_HS200_1_8V)) { >> mmc->card_caps |= MMC_MODE_HS200; >> } >> +#endif > It's not related with your subject.? You are right. I'll put this in the right patch: 'mmc: compile out more code if support for UHS and HS200 is not enabled'. Thanks, Jean-Jacques > > > Best Regards, > Jaehoon Chung > >> if (cardtype & EXT_CSD_CARD_TYPE_52) { >> if (cardtype & EXT_CSD_CARD_TYPE_DDR_52) >> mmc->card_caps |= MMC_MODE_DDR_52MHz; >> >
diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c index 67d05c5..7a92718 100644 --- a/drivers/mmc/mmc.c +++ b/drivers/mmc/mmc.c @@ -788,11 +788,8 @@ int mmc_switch(struct mmc *mmc, u8 set, u8 index, u8 value) static int mmc_set_card_speed(struct mmc *mmc, enum bus_mode mode) { - int err; int speed_bits; - ALLOC_CACHE_ALIGN_BUFFER(u8, test_csd, MMC_MAX_BLOCK_LEN); - switch (mode) { case MMC_HS: case MMC_HS_52: @@ -808,23 +805,8 @@ static int mmc_set_card_speed(struct mmc *mmc, enum bus_mode mode) default: return -EINVAL; } - err = mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_HS_TIMING, + return mmc_switch(mmc, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_HS_TIMING, speed_bits); - if (err) - return err; - - if ((mode == MMC_HS) || (mode == MMC_HS_52)) { - /* Now check to see that it worked */ - err = mmc_send_ext_csd(mmc, test_csd); - if (err) - return err; - - /* No high-speed support */ - if (!test_csd[EXT_CSD_HS_TIMING]) - return -ENOTSUPP; - } - - return 0; } static int mmc_get_capabilities(struct mmc *mmc) @@ -851,10 +833,12 @@ static int mmc_get_capabilities(struct mmc *mmc) cardtype = ext_csd[EXT_CSD_CARD_TYPE] & 0x3f; mmc->cardtype = cardtype; +#if CONFIG_IS_ENABLED(MMC_HS200_SUPPORT) if (cardtype & (EXT_CSD_CARD_TYPE_HS200_1_2V | EXT_CSD_CARD_TYPE_HS200_1_8V)) { mmc->card_caps |= MMC_MODE_HS200; } +#endif if (cardtype & EXT_CSD_CARD_TYPE_52) { if (cardtype & EXT_CSD_CARD_TYPE_DDR_52) mmc->card_caps |= MMC_MODE_DDR_52MHz;
mmc_set_card_speed() reads the ext csd to check if switch has been OK. But it does it only for MMC_HS and MMC_HS_52. Moreover this check is not really required as there will be a ext csd reading later in the initialization process to make sure that it succeeded. So remove this partial verification and save space. Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com> --- drivers/mmc/mmc.c | 22 +++------------------- 1 file changed, 3 insertions(+), 19 deletions(-)