Message ID | 003501ceda2a$d7ae2980$870a7c80$%jun@samsung.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Hi Seungwon, On 5 November 2013 14:27, Seungwon Jeon <tgih.jun@samsung.com> wrote: > Card state related to speed mode should be in non-overlapped. > Consideration for all cases is required when being cleared. > Also, MMC_STATE_PRESENT and MMC_STATE_REMOVED are same. > It's exclusive state which cannot be set at the same time. > > Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com> > --- > drivers/mmc/core/core.c | 1 - > drivers/mmc/core/mmc.c | 4 ++-- > drivers/mmc/core/sd.c | 5 +++-- > drivers/mmc/core/sdio.c | 5 ++++- > include/linux/mmc/card.h | 37 +++++++++++++++++++++++++++++++------ > 5 files changed, 40 insertions(+), 12 deletions(-) > > diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c > index 57a2b40..b183d56 100644 > --- a/drivers/mmc/core/core.c > +++ b/drivers/mmc/core/core.c > @@ -2281,7 +2281,6 @@ static int mmc_do_hw_reset(struct mmc_host *host, int check) > } > } > > - host->card->state &= ~(MMC_STATE_HIGHSPEED | MMC_STATE_HIGHSPEED_DDR); > if (mmc_host_is_spi(host)) { > host->ios.chip_select = MMC_CS_HIGH; > host->ios.bus_mode = MMC_BUSMODE_PUSHPULL; > diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c > index f4f8991..1668ea4 100644 > --- a/drivers/mmc/core/mmc.c > +++ b/drivers/mmc/core/mmc.c > @@ -1597,11 +1597,11 @@ static int _mmc_suspend(struct mmc_host *host, bool is_suspend) > err = mmc_sleep(host); > else if (!mmc_host_is_spi(host)) > err = mmc_deselect_cards(host); > - host->card->state &= ~(MMC_STATE_HIGHSPEED | MMC_STATE_HIGHSPEED_200); > > if (!err) { > mmc_power_off(host); > mmc_card_set_suspended(host->card); > + mmc_card_set_ds(host->card); > } > out: > mmc_release_host(host); > @@ -1727,8 +1727,8 @@ static int mmc_power_restore(struct mmc_host *host) > { > int ret; > > - host->card->state &= ~(MMC_STATE_HIGHSPEED | MMC_STATE_HIGHSPEED_200); > mmc_claim_host(host); > + mmc_card_set_ds(host->card); > ret = mmc_init_card(host, host->card->ocr, host->card); > mmc_release_host(host); > > diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c > index 6f42050..b19a8f4 100644 > --- a/drivers/mmc/core/sd.c > +++ b/drivers/mmc/core/sd.c > @@ -1082,10 +1082,11 @@ static int _mmc_sd_suspend(struct mmc_host *host) > > if (!mmc_host_is_spi(host)) > err = mmc_deselect_cards(host); > - host->card->state &= ~MMC_STATE_HIGHSPEED; > + > if (!err) { > mmc_power_off(host); > mmc_card_set_suspended(host->card); > + mmc_card_set_ds(host->card); > } > > out: > @@ -1191,8 +1192,8 @@ static int mmc_sd_power_restore(struct mmc_host *host) > { > int ret; > > - host->card->state &= ~MMC_STATE_HIGHSPEED; > mmc_claim_host(host); > + mmc_card_set_ds(host->card); > ret = mmc_sd_init_card(host, host->card->ocr, host->card); > mmc_release_host(host); > > diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c > index 4d721c6..7c6c43c 100644 > --- a/drivers/mmc/core/sdio.c > +++ b/drivers/mmc/core/sdio.c > @@ -968,8 +968,10 @@ static int mmc_sdio_suspend(struct mmc_host *host) > mmc_release_host(host); > } > > - if (!err && !mmc_card_keep_power(host)) > + if (!err && !mmc_card_keep_power(host)) { > mmc_power_off(host); > + mmc_card_set_ds(host->card); > + } > > return err; > } > @@ -1075,6 +1077,7 @@ static int mmc_sdio_power_restore(struct mmc_host *host) > if (ret) > goto out; > > + mmc_card_set_ds(host->card); > ret = mmc_sdio_init_card(host, host->card->ocr, host->card, > mmc_card_keep_power(host)); > if (!ret && host->sdio_irqs) > diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h > index c119735..f2c2620 100644 > --- a/include/linux/mmc/card.h > +++ b/include/linux/mmc/card.h > @@ -260,6 +260,11 @@ struct mmc_card { > #define MMC_STATE_HIGHSPEED_200 (1<<8) /* card is in HS200 mode */ > #define MMC_STATE_DOING_BKOPS (1<<10) /* card is doing BKOPS */ > #define MMC_STATE_SUSPENDED (1<<11) /* card is suspended */ > +#define MMC_STATE_SPEED_MASK (MMC_STATE_HIGHSPEED | \ > + MMC_STATE_HIGHSPEED_DDR | \ > + MMC_STATE_ULTRAHIGHSPEED | \ > + MMC_STATE_HIGHSPEED_200) > + /* Mask for default speed(DS) */ > unsigned int quirks; /* card quirks */ > #define MMC_QUIRK_LENIENT_FN0 (1<<0) /* allow SDIO FN0 writes outside of the VS CCCR range */ > #define MMC_QUIRK_BLKSZ_FOR_BYTE_MODE (1<<1) /* use func->cur_blksize */ > @@ -431,19 +436,39 @@ static inline void __maybe_unused remove_quirk(struct mmc_card *card, int data) > #define mmc_card_doing_bkops(c) ((c)->state & MMC_STATE_DOING_BKOPS) > #define mmc_card_suspended(c) ((c)->state & MMC_STATE_SUSPENDED) > > -#define mmc_card_set_present(c) ((c)->state |= MMC_STATE_PRESENT) > + > #define mmc_card_set_readonly(c) ((c)->state |= MMC_STATE_READONLY) > -#define mmc_card_set_highspeed(c) ((c)->state |= MMC_STATE_HIGHSPEED) > -#define mmc_card_set_hs200(c) ((c)->state |= MMC_STATE_HIGHSPEED_200) > #define mmc_card_set_blockaddr(c) ((c)->state |= MMC_STATE_BLOCKADDR) > -#define mmc_card_set_ddr_mode(c) ((c)->state |= MMC_STATE_HIGHSPEED_DDR) > -#define mmc_card_set_uhs(c) ((c)->state |= MMC_STATE_ULTRAHIGHSPEED) > #define mmc_card_set_ext_capacity(c) ((c)->state |= MMC_CARD_SDXC) > -#define mmc_card_set_removed(c) ((c)->state |= MMC_CARD_REMOVED) > #define mmc_card_set_doing_bkops(c) ((c)->state |= MMC_STATE_DOING_BKOPS) > #define mmc_card_clr_doing_bkops(c) ((c)->state &= ~MMC_STATE_DOING_BKOPS) > #define mmc_card_set_suspended(c) ((c)->state |= MMC_STATE_SUSPENDED) > #define mmc_card_clr_suspended(c) ((c)->state &= ~MMC_STATE_SUSPENDED) > +#define mmc_card_set_highspeed(c) \ > + ((c)->state = \ > + ((c)->state & ~MMC_STATE_SPEED_MASK) | \ > + MMC_STATE_HIGHSPEED) > +#define mmc_card_set_ddr_mode(c) \ > + ((c)->state = \ > + ((c)->state & ~MMC_STATE_SPEED_MASK) | \ > + MMC_STATE_HIGHSPEED_DDR) > +#define mmc_card_set_hs200(c) \ > + ((c)->state = \ > + ((c)->state & ~MMC_STATE_SPEED_MASK) | \ > + MMC_STATE_HIGHSPEED_200) > +#define mmc_card_set_uhs(c) \ > + ((c)->state = \ > + ((c)->state & ~MMC_STATE_SPEED_MASK) | \ > + MMC_STATE_ULTRAHIGHSPEED) > +#define mmc_card_set_present(c) \ > + ((c)->state = \ > + ((c)->state & ~MMC_CARD_REMOVED) | \ > + MMC_STATE_PRESENT) > +#define mmc_card_set_removed(c) \ > + ((c)->state = \ > + ((c)->state & ~MMC_STATE_PRESENT) | \ > + MMC_CARD_REMOVED) > +#define mmc_card_set_ds(c) ((c)->state &= ~MMC_STATE_SPEED_MASK) I have a feeling of that this "card->state" has become a container for a lot of mixed stuff. So your clean up is definitely justified. I have a suggestion for how we can improve simplicity, how about dividing the state into two variables instead: -> One part are actually used in the mmc core code to track a state and to make certain decisions during execution, like MMC_STATE_SUSPENDED, MMC_STATE_PRESENT, MMC_CARD_REMOVED. -> The other part is more to be considered as the current operational state, for example the bus speed mode. The information in this "state variable" will be re-negotiated as a part of mmc_sd|sdio_init_card and can therefore always be reset in the beginning of these functions. Does it make sense? Kind regards Ulf Hansson > > /* > * Quirk add/remove for MMC products. > -- > 1.7.0.4 > > -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, November 05, 2013, Ulf Hansson wrote: > Hi Seungwon, > > > On 5 November 2013 14:27, Seungwon Jeon <tgih.jun@samsung.com> wrote: > > Card state related to speed mode should be in non-overlapped. > > Consideration for all cases is required when being cleared. > > Also, MMC_STATE_PRESENT and MMC_STATE_REMOVED are same. > > It's exclusive state which cannot be set at the same time. > > > > Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com> > > --- > > drivers/mmc/core/core.c | 1 - > > drivers/mmc/core/mmc.c | 4 ++-- > > drivers/mmc/core/sd.c | 5 +++-- > > drivers/mmc/core/sdio.c | 5 ++++- > > include/linux/mmc/card.h | 37 +++++++++++++++++++++++++++++++------ > > 5 files changed, 40 insertions(+), 12 deletions(-) > > > > diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c > > index 57a2b40..b183d56 100644 > > --- a/drivers/mmc/core/core.c > > +++ b/drivers/mmc/core/core.c > > @@ -2281,7 +2281,6 @@ static int mmc_do_hw_reset(struct mmc_host *host, int check) > > } > > } > > > > - host->card->state &= ~(MMC_STATE_HIGHSPEED | MMC_STATE_HIGHSPEED_DDR); > > if (mmc_host_is_spi(host)) { > > host->ios.chip_select = MMC_CS_HIGH; > > host->ios.bus_mode = MMC_BUSMODE_PUSHPULL; > > diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c > > index f4f8991..1668ea4 100644 > > --- a/drivers/mmc/core/mmc.c > > +++ b/drivers/mmc/core/mmc.c > > @@ -1597,11 +1597,11 @@ static int _mmc_suspend(struct mmc_host *host, bool is_suspend) > > err = mmc_sleep(host); > > else if (!mmc_host_is_spi(host)) > > err = mmc_deselect_cards(host); > > - host->card->state &= ~(MMC_STATE_HIGHSPEED | MMC_STATE_HIGHSPEED_200); > > > > if (!err) { > > mmc_power_off(host); > > mmc_card_set_suspended(host->card); > > + mmc_card_set_ds(host->card); > > } > > out: > > mmc_release_host(host); > > @@ -1727,8 +1727,8 @@ static int mmc_power_restore(struct mmc_host *host) > > { > > int ret; > > > > - host->card->state &= ~(MMC_STATE_HIGHSPEED | MMC_STATE_HIGHSPEED_200); > > mmc_claim_host(host); > > + mmc_card_set_ds(host->card); > > ret = mmc_init_card(host, host->card->ocr, host->card); > > mmc_release_host(host); > > > > diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c > > index 6f42050..b19a8f4 100644 > > --- a/drivers/mmc/core/sd.c > > +++ b/drivers/mmc/core/sd.c > > @@ -1082,10 +1082,11 @@ static int _mmc_sd_suspend(struct mmc_host *host) > > > > if (!mmc_host_is_spi(host)) > > err = mmc_deselect_cards(host); > > - host->card->state &= ~MMC_STATE_HIGHSPEED; > > + > > if (!err) { > > mmc_power_off(host); > > mmc_card_set_suspended(host->card); > > + mmc_card_set_ds(host->card); > > } > > > > out: > > @@ -1191,8 +1192,8 @@ static int mmc_sd_power_restore(struct mmc_host *host) > > { > > int ret; > > > > - host->card->state &= ~MMC_STATE_HIGHSPEED; > > mmc_claim_host(host); > > + mmc_card_set_ds(host->card); > > ret = mmc_sd_init_card(host, host->card->ocr, host->card); > > mmc_release_host(host); > > > > diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c > > index 4d721c6..7c6c43c 100644 > > --- a/drivers/mmc/core/sdio.c > > +++ b/drivers/mmc/core/sdio.c > > @@ -968,8 +968,10 @@ static int mmc_sdio_suspend(struct mmc_host *host) > > mmc_release_host(host); > > } > > > > - if (!err && !mmc_card_keep_power(host)) > > + if (!err && !mmc_card_keep_power(host)) { > > mmc_power_off(host); > > + mmc_card_set_ds(host->card); > > + } > > > > return err; > > } > > @@ -1075,6 +1077,7 @@ static int mmc_sdio_power_restore(struct mmc_host *host) > > if (ret) > > goto out; > > > > + mmc_card_set_ds(host->card); > > ret = mmc_sdio_init_card(host, host->card->ocr, host->card, > > mmc_card_keep_power(host)); > > if (!ret && host->sdio_irqs) > > diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h > > index c119735..f2c2620 100644 > > --- a/include/linux/mmc/card.h > > +++ b/include/linux/mmc/card.h > > @@ -260,6 +260,11 @@ struct mmc_card { > > #define MMC_STATE_HIGHSPEED_200 (1<<8) /* card is in HS200 mode */ > > #define MMC_STATE_DOING_BKOPS (1<<10) /* card is doing BKOPS */ > > #define MMC_STATE_SUSPENDED (1<<11) /* card is suspended */ > > +#define MMC_STATE_SPEED_MASK (MMC_STATE_HIGHSPEED | \ > > + MMC_STATE_HIGHSPEED_DDR | \ > > + MMC_STATE_ULTRAHIGHSPEED | \ > > + MMC_STATE_HIGHSPEED_200) > > + /* Mask for default speed(DS) */ > > unsigned int quirks; /* card quirks */ > > #define MMC_QUIRK_LENIENT_FN0 (1<<0) /* allow SDIO FN0 writes outside of the VS CCCR range > */ > > #define MMC_QUIRK_BLKSZ_FOR_BYTE_MODE (1<<1) /* use func->cur_blksize */ > > @@ -431,19 +436,39 @@ static inline void __maybe_unused remove_quirk(struct mmc_card *card, int data) > > #define mmc_card_doing_bkops(c) ((c)->state & MMC_STATE_DOING_BKOPS) > > #define mmc_card_suspended(c) ((c)->state & MMC_STATE_SUSPENDED) > > > > -#define mmc_card_set_present(c) ((c)->state |= MMC_STATE_PRESENT) > > + > > #define mmc_card_set_readonly(c) ((c)->state |= MMC_STATE_READONLY) > > -#define mmc_card_set_highspeed(c) ((c)->state |= MMC_STATE_HIGHSPEED) > > -#define mmc_card_set_hs200(c) ((c)->state |= MMC_STATE_HIGHSPEED_200) > > #define mmc_card_set_blockaddr(c) ((c)->state |= MMC_STATE_BLOCKADDR) > > -#define mmc_card_set_ddr_mode(c) ((c)->state |= MMC_STATE_HIGHSPEED_DDR) > > -#define mmc_card_set_uhs(c) ((c)->state |= MMC_STATE_ULTRAHIGHSPEED) > > #define mmc_card_set_ext_capacity(c) ((c)->state |= MMC_CARD_SDXC) > > -#define mmc_card_set_removed(c) ((c)->state |= MMC_CARD_REMOVED) > > #define mmc_card_set_doing_bkops(c) ((c)->state |= MMC_STATE_DOING_BKOPS) > > #define mmc_card_clr_doing_bkops(c) ((c)->state &= ~MMC_STATE_DOING_BKOPS) > > #define mmc_card_set_suspended(c) ((c)->state |= MMC_STATE_SUSPENDED) > > #define mmc_card_clr_suspended(c) ((c)->state &= ~MMC_STATE_SUSPENDED) > > +#define mmc_card_set_highspeed(c) \ > > + ((c)->state = \ > > + ((c)->state & ~MMC_STATE_SPEED_MASK) | \ > > + MMC_STATE_HIGHSPEED) > > +#define mmc_card_set_ddr_mode(c) \ > > + ((c)->state = \ > > + ((c)->state & ~MMC_STATE_SPEED_MASK) | \ > > + MMC_STATE_HIGHSPEED_DDR) > > +#define mmc_card_set_hs200(c) \ > > + ((c)->state = \ > > + ((c)->state & ~MMC_STATE_SPEED_MASK) | \ > > + MMC_STATE_HIGHSPEED_200) > > +#define mmc_card_set_uhs(c) \ > > + ((c)->state = \ > > + ((c)->state & ~MMC_STATE_SPEED_MASK) | \ > > + MMC_STATE_ULTRAHIGHSPEED) > > +#define mmc_card_set_present(c) \ > > + ((c)->state = \ > > + ((c)->state & ~MMC_CARD_REMOVED) | \ > > + MMC_STATE_PRESENT) > > +#define mmc_card_set_removed(c) \ > > + ((c)->state = \ > > + ((c)->state & ~MMC_STATE_PRESENT) | \ > > + MMC_CARD_REMOVED) > > +#define mmc_card_set_ds(c) ((c)->state &= ~MMC_STATE_SPEED_MASK) > > I have a feeling of that this "card->state" has become a container for > a lot of mixed stuff. So your clean up is definitely justified. > > I have a suggestion for how we can improve simplicity, how about > dividing the state into two variables instead: > > -> One part are actually used in the mmc core code to track a state > and to make certain decisions during execution, like > MMC_STATE_SUSPENDED, MMC_STATE_PRESENT, MMC_CARD_REMOVED. > > -> The other part is more to be considered as the current operational > state, for example the bus speed mode. The information in this "state > variable" will be re-negotiated as a part of mmc_sd|sdio_init_card and > can therefore always be reset in the beginning of these functions. > > Does it make sense? Thank you for suggestion. I also feel that. Hmm, but when considering there is no case we access 'card->sate' directly, splitting into two types of state doesn't seem to give much. So I guess being kept is not bad. Thanks, Seungwon Jeon > > Kind regards > Ulf Hansson > > > > > /* > > * Quirk add/remove for MMC products. > > -- > > 1.7.0.4 > > > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-mmc" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 6 November 2013 10:35, Seungwon Jeon <tgih.jun@samsung.com> wrote: > On Tue, November 05, 2013, Ulf Hansson wrote: >> Hi Seungwon, >> >> >> On 5 November 2013 14:27, Seungwon Jeon <tgih.jun@samsung.com> wrote: >> > Card state related to speed mode should be in non-overlapped. >> > Consideration for all cases is required when being cleared. >> > Also, MMC_STATE_PRESENT and MMC_STATE_REMOVED are same. >> > It's exclusive state which cannot be set at the same time. >> > >> > Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com> >> > --- >> > drivers/mmc/core/core.c | 1 - >> > drivers/mmc/core/mmc.c | 4 ++-- >> > drivers/mmc/core/sd.c | 5 +++-- >> > drivers/mmc/core/sdio.c | 5 ++++- >> > include/linux/mmc/card.h | 37 +++++++++++++++++++++++++++++++------ >> > 5 files changed, 40 insertions(+), 12 deletions(-) >> > >> > diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c >> > index 57a2b40..b183d56 100644 >> > --- a/drivers/mmc/core/core.c >> > +++ b/drivers/mmc/core/core.c >> > @@ -2281,7 +2281,6 @@ static int mmc_do_hw_reset(struct mmc_host *host, int check) >> > } >> > } >> > >> > - host->card->state &= ~(MMC_STATE_HIGHSPEED | MMC_STATE_HIGHSPEED_DDR); >> > if (mmc_host_is_spi(host)) { >> > host->ios.chip_select = MMC_CS_HIGH; >> > host->ios.bus_mode = MMC_BUSMODE_PUSHPULL; >> > diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c >> > index f4f8991..1668ea4 100644 >> > --- a/drivers/mmc/core/mmc.c >> > +++ b/drivers/mmc/core/mmc.c >> > @@ -1597,11 +1597,11 @@ static int _mmc_suspend(struct mmc_host *host, bool is_suspend) >> > err = mmc_sleep(host); >> > else if (!mmc_host_is_spi(host)) >> > err = mmc_deselect_cards(host); >> > - host->card->state &= ~(MMC_STATE_HIGHSPEED | MMC_STATE_HIGHSPEED_200); >> > >> > if (!err) { >> > mmc_power_off(host); >> > mmc_card_set_suspended(host->card); >> > + mmc_card_set_ds(host->card); >> > } >> > out: >> > mmc_release_host(host); >> > @@ -1727,8 +1727,8 @@ static int mmc_power_restore(struct mmc_host *host) >> > { >> > int ret; >> > >> > - host->card->state &= ~(MMC_STATE_HIGHSPEED | MMC_STATE_HIGHSPEED_200); >> > mmc_claim_host(host); >> > + mmc_card_set_ds(host->card); >> > ret = mmc_init_card(host, host->card->ocr, host->card); >> > mmc_release_host(host); >> > >> > diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c >> > index 6f42050..b19a8f4 100644 >> > --- a/drivers/mmc/core/sd.c >> > +++ b/drivers/mmc/core/sd.c >> > @@ -1082,10 +1082,11 @@ static int _mmc_sd_suspend(struct mmc_host *host) >> > >> > if (!mmc_host_is_spi(host)) >> > err = mmc_deselect_cards(host); >> > - host->card->state &= ~MMC_STATE_HIGHSPEED; >> > + >> > if (!err) { >> > mmc_power_off(host); >> > mmc_card_set_suspended(host->card); >> > + mmc_card_set_ds(host->card); >> > } >> > >> > out: >> > @@ -1191,8 +1192,8 @@ static int mmc_sd_power_restore(struct mmc_host *host) >> > { >> > int ret; >> > >> > - host->card->state &= ~MMC_STATE_HIGHSPEED; >> > mmc_claim_host(host); >> > + mmc_card_set_ds(host->card); >> > ret = mmc_sd_init_card(host, host->card->ocr, host->card); >> > mmc_release_host(host); >> > >> > diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c >> > index 4d721c6..7c6c43c 100644 >> > --- a/drivers/mmc/core/sdio.c >> > +++ b/drivers/mmc/core/sdio.c >> > @@ -968,8 +968,10 @@ static int mmc_sdio_suspend(struct mmc_host *host) >> > mmc_release_host(host); >> > } >> > >> > - if (!err && !mmc_card_keep_power(host)) >> > + if (!err && !mmc_card_keep_power(host)) { >> > mmc_power_off(host); >> > + mmc_card_set_ds(host->card); >> > + } >> > >> > return err; >> > } >> > @@ -1075,6 +1077,7 @@ static int mmc_sdio_power_restore(struct mmc_host *host) >> > if (ret) >> > goto out; >> > >> > + mmc_card_set_ds(host->card); >> > ret = mmc_sdio_init_card(host, host->card->ocr, host->card, >> > mmc_card_keep_power(host)); >> > if (!ret && host->sdio_irqs) >> > diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h >> > index c119735..f2c2620 100644 >> > --- a/include/linux/mmc/card.h >> > +++ b/include/linux/mmc/card.h >> > @@ -260,6 +260,11 @@ struct mmc_card { >> > #define MMC_STATE_HIGHSPEED_200 (1<<8) /* card is in HS200 mode */ >> > #define MMC_STATE_DOING_BKOPS (1<<10) /* card is doing BKOPS */ >> > #define MMC_STATE_SUSPENDED (1<<11) /* card is suspended */ >> > +#define MMC_STATE_SPEED_MASK (MMC_STATE_HIGHSPEED | \ >> > + MMC_STATE_HIGHSPEED_DDR | \ >> > + MMC_STATE_ULTRAHIGHSPEED | \ >> > + MMC_STATE_HIGHSPEED_200) >> > + /* Mask for default speed(DS) */ >> > unsigned int quirks; /* card quirks */ >> > #define MMC_QUIRK_LENIENT_FN0 (1<<0) /* allow SDIO FN0 writes outside of the VS CCCR range >> */ >> > #define MMC_QUIRK_BLKSZ_FOR_BYTE_MODE (1<<1) /* use func->cur_blksize */ >> > @@ -431,19 +436,39 @@ static inline void __maybe_unused remove_quirk(struct mmc_card *card, int data) >> > #define mmc_card_doing_bkops(c) ((c)->state & MMC_STATE_DOING_BKOPS) >> > #define mmc_card_suspended(c) ((c)->state & MMC_STATE_SUSPENDED) >> > >> > -#define mmc_card_set_present(c) ((c)->state |= MMC_STATE_PRESENT) >> > + >> > #define mmc_card_set_readonly(c) ((c)->state |= MMC_STATE_READONLY) >> > -#define mmc_card_set_highspeed(c) ((c)->state |= MMC_STATE_HIGHSPEED) >> > -#define mmc_card_set_hs200(c) ((c)->state |= MMC_STATE_HIGHSPEED_200) >> > #define mmc_card_set_blockaddr(c) ((c)->state |= MMC_STATE_BLOCKADDR) >> > -#define mmc_card_set_ddr_mode(c) ((c)->state |= MMC_STATE_HIGHSPEED_DDR) >> > -#define mmc_card_set_uhs(c) ((c)->state |= MMC_STATE_ULTRAHIGHSPEED) >> > #define mmc_card_set_ext_capacity(c) ((c)->state |= MMC_CARD_SDXC) >> > -#define mmc_card_set_removed(c) ((c)->state |= MMC_CARD_REMOVED) >> > #define mmc_card_set_doing_bkops(c) ((c)->state |= MMC_STATE_DOING_BKOPS) >> > #define mmc_card_clr_doing_bkops(c) ((c)->state &= ~MMC_STATE_DOING_BKOPS) >> > #define mmc_card_set_suspended(c) ((c)->state |= MMC_STATE_SUSPENDED) >> > #define mmc_card_clr_suspended(c) ((c)->state &= ~MMC_STATE_SUSPENDED) >> > +#define mmc_card_set_highspeed(c) \ >> > + ((c)->state = \ >> > + ((c)->state & ~MMC_STATE_SPEED_MASK) | \ >> > + MMC_STATE_HIGHSPEED) >> > +#define mmc_card_set_ddr_mode(c) \ >> > + ((c)->state = \ >> > + ((c)->state & ~MMC_STATE_SPEED_MASK) | \ >> > + MMC_STATE_HIGHSPEED_DDR) >> > +#define mmc_card_set_hs200(c) \ >> > + ((c)->state = \ >> > + ((c)->state & ~MMC_STATE_SPEED_MASK) | \ >> > + MMC_STATE_HIGHSPEED_200) >> > +#define mmc_card_set_uhs(c) \ >> > + ((c)->state = \ >> > + ((c)->state & ~MMC_STATE_SPEED_MASK) | \ >> > + MMC_STATE_ULTRAHIGHSPEED) >> > +#define mmc_card_set_present(c) \ >> > + ((c)->state = \ >> > + ((c)->state & ~MMC_CARD_REMOVED) | \ >> > + MMC_STATE_PRESENT) >> > +#define mmc_card_set_removed(c) \ >> > + ((c)->state = \ >> > + ((c)->state & ~MMC_STATE_PRESENT) | \ >> > + MMC_CARD_REMOVED) >> > +#define mmc_card_set_ds(c) ((c)->state &= ~MMC_STATE_SPEED_MASK) >> >> I have a feeling of that this "card->state" has become a container for >> a lot of mixed stuff. So your clean up is definitely justified. >> >> I have a suggestion for how we can improve simplicity, how about >> dividing the state into two variables instead: >> >> -> One part are actually used in the mmc core code to track a state >> and to make certain decisions during execution, like >> MMC_STATE_SUSPENDED, MMC_STATE_PRESENT, MMC_CARD_REMOVED. >> >> -> The other part is more to be considered as the current operational >> state, for example the bus speed mode. The information in this "state >> variable" will be re-negotiated as a part of mmc_sd|sdio_init_card and >> can therefore always be reset in the beginning of these functions. >> >> Does it make sense? > Thank you for suggestion. I also feel that. > Hmm, but when considering there is no case we access 'card->sate' directly, > splitting into two types of state doesn't seem to give much. Actually, on a second thought, why are we even caching the bus speeds in the card state? The bus speeds are already reflected in the "ios->timing" struct, which moreover also is available through debugfs. Can we remove the bus speeds entirely from the card state instead? Kind regards Ulf Hansson > So I guess being kept is not bad. > > Thanks, > Seungwon Jeon > >> >> Kind regards >> Ulf Hansson >> >> > >> > /* >> > * Quirk add/remove for MMC products. >> > -- >> > 1.7.0.4 >> > >> > >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, November 06, 2013, Ulf Hansson wrote: > On 6 November 2013 10:35, Seungwon Jeon <tgih.jun@samsung.com> wrote: > > On Tue, November 05, 2013, Ulf Hansson wrote: > >> Hi Seungwon, > >> > >> > >> On 5 November 2013 14:27, Seungwon Jeon <tgih.jun@samsung.com> wrote: > >> > Card state related to speed mode should be in non-overlapped. > >> > Consideration for all cases is required when being cleared. > >> > Also, MMC_STATE_PRESENT and MMC_STATE_REMOVED are same. > >> > It's exclusive state which cannot be set at the same time. > >> > > >> > Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com> > >> > --- > >> > drivers/mmc/core/core.c | 1 - > >> > drivers/mmc/core/mmc.c | 4 ++-- > >> > drivers/mmc/core/sd.c | 5 +++-- > >> > drivers/mmc/core/sdio.c | 5 ++++- > >> > include/linux/mmc/card.h | 37 +++++++++++++++++++++++++++++++------ > >> > 5 files changed, 40 insertions(+), 12 deletions(-) > >> > > >> > diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c > >> > index 57a2b40..b183d56 100644 > >> > --- a/drivers/mmc/core/core.c > >> > +++ b/drivers/mmc/core/core.c > >> > @@ -2281,7 +2281,6 @@ static int mmc_do_hw_reset(struct mmc_host *host, int check) > >> > } > >> > } > >> > > >> > - host->card->state &= ~(MMC_STATE_HIGHSPEED | MMC_STATE_HIGHSPEED_DDR); > >> > if (mmc_host_is_spi(host)) { > >> > host->ios.chip_select = MMC_CS_HIGH; > >> > host->ios.bus_mode = MMC_BUSMODE_PUSHPULL; > >> > diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c > >> > index f4f8991..1668ea4 100644 > >> > --- a/drivers/mmc/core/mmc.c > >> > +++ b/drivers/mmc/core/mmc.c > >> > @@ -1597,11 +1597,11 @@ static int _mmc_suspend(struct mmc_host *host, bool is_suspend) > >> > err = mmc_sleep(host); > >> > else if (!mmc_host_is_spi(host)) > >> > err = mmc_deselect_cards(host); > >> > - host->card->state &= ~(MMC_STATE_HIGHSPEED | MMC_STATE_HIGHSPEED_200); > >> > > >> > if (!err) { > >> > mmc_power_off(host); > >> > mmc_card_set_suspended(host->card); > >> > + mmc_card_set_ds(host->card); > >> > } > >> > out: > >> > mmc_release_host(host); > >> > @@ -1727,8 +1727,8 @@ static int mmc_power_restore(struct mmc_host *host) > >> > { > >> > int ret; > >> > > >> > - host->card->state &= ~(MMC_STATE_HIGHSPEED | MMC_STATE_HIGHSPEED_200); > >> > mmc_claim_host(host); > >> > + mmc_card_set_ds(host->card); > >> > ret = mmc_init_card(host, host->card->ocr, host->card); > >> > mmc_release_host(host); > >> > > >> > diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c > >> > index 6f42050..b19a8f4 100644 > >> > --- a/drivers/mmc/core/sd.c > >> > +++ b/drivers/mmc/core/sd.c > >> > @@ -1082,10 +1082,11 @@ static int _mmc_sd_suspend(struct mmc_host *host) > >> > > >> > if (!mmc_host_is_spi(host)) > >> > err = mmc_deselect_cards(host); > >> > - host->card->state &= ~MMC_STATE_HIGHSPEED; > >> > + > >> > if (!err) { > >> > mmc_power_off(host); > >> > mmc_card_set_suspended(host->card); > >> > + mmc_card_set_ds(host->card); > >> > } > >> > > >> > out: > >> > @@ -1191,8 +1192,8 @@ static int mmc_sd_power_restore(struct mmc_host *host) > >> > { > >> > int ret; > >> > > >> > - host->card->state &= ~MMC_STATE_HIGHSPEED; > >> > mmc_claim_host(host); > >> > + mmc_card_set_ds(host->card); > >> > ret = mmc_sd_init_card(host, host->card->ocr, host->card); > >> > mmc_release_host(host); > >> > > >> > diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c > >> > index 4d721c6..7c6c43c 100644 > >> > --- a/drivers/mmc/core/sdio.c > >> > +++ b/drivers/mmc/core/sdio.c > >> > @@ -968,8 +968,10 @@ static int mmc_sdio_suspend(struct mmc_host *host) > >> > mmc_release_host(host); > >> > } > >> > > >> > - if (!err && !mmc_card_keep_power(host)) > >> > + if (!err && !mmc_card_keep_power(host)) { > >> > mmc_power_off(host); > >> > + mmc_card_set_ds(host->card); > >> > + } > >> > > >> > return err; > >> > } > >> > @@ -1075,6 +1077,7 @@ static int mmc_sdio_power_restore(struct mmc_host *host) > >> > if (ret) > >> > goto out; > >> > > >> > + mmc_card_set_ds(host->card); > >> > ret = mmc_sdio_init_card(host, host->card->ocr, host->card, > >> > mmc_card_keep_power(host)); > >> > if (!ret && host->sdio_irqs) > >> > diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h > >> > index c119735..f2c2620 100644 > >> > --- a/include/linux/mmc/card.h > >> > +++ b/include/linux/mmc/card.h > >> > @@ -260,6 +260,11 @@ struct mmc_card { > >> > #define MMC_STATE_HIGHSPEED_200 (1<<8) /* card is in HS200 mode */ > >> > #define MMC_STATE_DOING_BKOPS (1<<10) /* card is doing BKOPS */ > >> > #define MMC_STATE_SUSPENDED (1<<11) /* card is suspended */ > >> > +#define MMC_STATE_SPEED_MASK (MMC_STATE_HIGHSPEED | \ > >> > + MMC_STATE_HIGHSPEED_DDR | \ > >> > + MMC_STATE_ULTRAHIGHSPEED | \ > >> > + MMC_STATE_HIGHSPEED_200) > >> > + /* Mask for default speed(DS) */ > >> > unsigned int quirks; /* card quirks */ > >> > #define MMC_QUIRK_LENIENT_FN0 (1<<0) /* allow SDIO FN0 writes outside of the VS CCCR > range > >> */ > >> > #define MMC_QUIRK_BLKSZ_FOR_BYTE_MODE (1<<1) /* use func->cur_blksize */ > >> > @@ -431,19 +436,39 @@ static inline void __maybe_unused remove_quirk(struct mmc_card *card, int > data) > >> > #define mmc_card_doing_bkops(c) ((c)->state & MMC_STATE_DOING_BKOPS) > >> > #define mmc_card_suspended(c) ((c)->state & MMC_STATE_SUSPENDED) > >> > > >> > -#define mmc_card_set_present(c) ((c)->state |= MMC_STATE_PRESENT) > >> > + > >> > #define mmc_card_set_readonly(c) ((c)->state |= MMC_STATE_READONLY) > >> > -#define mmc_card_set_highspeed(c) ((c)->state |= MMC_STATE_HIGHSPEED) > >> > -#define mmc_card_set_hs200(c) ((c)->state |= MMC_STATE_HIGHSPEED_200) > >> > #define mmc_card_set_blockaddr(c) ((c)->state |= MMC_STATE_BLOCKADDR) > >> > -#define mmc_card_set_ddr_mode(c) ((c)->state |= MMC_STATE_HIGHSPEED_DDR) > >> > -#define mmc_card_set_uhs(c) ((c)->state |= MMC_STATE_ULTRAHIGHSPEED) > >> > #define mmc_card_set_ext_capacity(c) ((c)->state |= MMC_CARD_SDXC) > >> > -#define mmc_card_set_removed(c) ((c)->state |= MMC_CARD_REMOVED) > >> > #define mmc_card_set_doing_bkops(c) ((c)->state |= MMC_STATE_DOING_BKOPS) > >> > #define mmc_card_clr_doing_bkops(c) ((c)->state &= ~MMC_STATE_DOING_BKOPS) > >> > #define mmc_card_set_suspended(c) ((c)->state |= MMC_STATE_SUSPENDED) > >> > #define mmc_card_clr_suspended(c) ((c)->state &= ~MMC_STATE_SUSPENDED) > >> > +#define mmc_card_set_highspeed(c) \ > >> > + ((c)->state = \ > >> > + ((c)->state & ~MMC_STATE_SPEED_MASK) | \ > >> > + MMC_STATE_HIGHSPEED) > >> > +#define mmc_card_set_ddr_mode(c) \ > >> > + ((c)->state = \ > >> > + ((c)->state & ~MMC_STATE_SPEED_MASK) | \ > >> > + MMC_STATE_HIGHSPEED_DDR) > >> > +#define mmc_card_set_hs200(c) \ > >> > + ((c)->state = \ > >> > + ((c)->state & ~MMC_STATE_SPEED_MASK) | \ > >> > + MMC_STATE_HIGHSPEED_200) > >> > +#define mmc_card_set_uhs(c) \ > >> > + ((c)->state = \ > >> > + ((c)->state & ~MMC_STATE_SPEED_MASK) | \ > >> > + MMC_STATE_ULTRAHIGHSPEED) > >> > +#define mmc_card_set_present(c) \ > >> > + ((c)->state = \ > >> > + ((c)->state & ~MMC_CARD_REMOVED) | \ > >> > + MMC_STATE_PRESENT) > >> > +#define mmc_card_set_removed(c) \ > >> > + ((c)->state = \ > >> > + ((c)->state & ~MMC_STATE_PRESENT) | \ > >> > + MMC_CARD_REMOVED) > >> > +#define mmc_card_set_ds(c) ((c)->state &= ~MMC_STATE_SPEED_MASK) > >> > >> I have a feeling of that this "card->state" has become a container for > >> a lot of mixed stuff. So your clean up is definitely justified. > >> > >> I have a suggestion for how we can improve simplicity, how about > >> dividing the state into two variables instead: > >> > >> -> One part are actually used in the mmc core code to track a state > >> and to make certain decisions during execution, like > >> MMC_STATE_SUSPENDED, MMC_STATE_PRESENT, MMC_CARD_REMOVED. > >> > >> -> The other part is more to be considered as the current operational > >> state, for example the bus speed mode. The information in this "state > >> variable" will be re-negotiated as a part of mmc_sd|sdio_init_card and > >> can therefore always be reset in the beginning of these functions. > >> > >> Does it make sense? > > Thank you for suggestion. I also feel that. > > Hmm, but when considering there is no case we access 'card->sate' directly, > > splitting into two types of state doesn't seem to give much. > > Actually, on a second thought, why are we even caching the bus speeds > in the card state? The bus speeds are already reflected in the > "ios->timing" struct, which moreover also is available through > debugfs. Can we remove the bus speeds entirely from the card state > instead? You point out redundancy of existing state for speed mode. Yes, I agree with you on that point. It could be replaced. I will consider for next. Thanks, Seungwon Jeon -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index 57a2b40..b183d56 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -2281,7 +2281,6 @@ static int mmc_do_hw_reset(struct mmc_host *host, int check) } } - host->card->state &= ~(MMC_STATE_HIGHSPEED | MMC_STATE_HIGHSPEED_DDR); if (mmc_host_is_spi(host)) { host->ios.chip_select = MMC_CS_HIGH; host->ios.bus_mode = MMC_BUSMODE_PUSHPULL; diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c index f4f8991..1668ea4 100644 --- a/drivers/mmc/core/mmc.c +++ b/drivers/mmc/core/mmc.c @@ -1597,11 +1597,11 @@ static int _mmc_suspend(struct mmc_host *host, bool is_suspend) err = mmc_sleep(host); else if (!mmc_host_is_spi(host)) err = mmc_deselect_cards(host); - host->card->state &= ~(MMC_STATE_HIGHSPEED | MMC_STATE_HIGHSPEED_200); if (!err) { mmc_power_off(host); mmc_card_set_suspended(host->card); + mmc_card_set_ds(host->card); } out: mmc_release_host(host); @@ -1727,8 +1727,8 @@ static int mmc_power_restore(struct mmc_host *host) { int ret; - host->card->state &= ~(MMC_STATE_HIGHSPEED | MMC_STATE_HIGHSPEED_200); mmc_claim_host(host); + mmc_card_set_ds(host->card); ret = mmc_init_card(host, host->card->ocr, host->card); mmc_release_host(host); diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c index 6f42050..b19a8f4 100644 --- a/drivers/mmc/core/sd.c +++ b/drivers/mmc/core/sd.c @@ -1082,10 +1082,11 @@ static int _mmc_sd_suspend(struct mmc_host *host) if (!mmc_host_is_spi(host)) err = mmc_deselect_cards(host); - host->card->state &= ~MMC_STATE_HIGHSPEED; + if (!err) { mmc_power_off(host); mmc_card_set_suspended(host->card); + mmc_card_set_ds(host->card); } out: @@ -1191,8 +1192,8 @@ static int mmc_sd_power_restore(struct mmc_host *host) { int ret; - host->card->state &= ~MMC_STATE_HIGHSPEED; mmc_claim_host(host); + mmc_card_set_ds(host->card); ret = mmc_sd_init_card(host, host->card->ocr, host->card); mmc_release_host(host); diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c index 4d721c6..7c6c43c 100644 --- a/drivers/mmc/core/sdio.c +++ b/drivers/mmc/core/sdio.c @@ -968,8 +968,10 @@ static int mmc_sdio_suspend(struct mmc_host *host) mmc_release_host(host); } - if (!err && !mmc_card_keep_power(host)) + if (!err && !mmc_card_keep_power(host)) { mmc_power_off(host); + mmc_card_set_ds(host->card); + } return err; } @@ -1075,6 +1077,7 @@ static int mmc_sdio_power_restore(struct mmc_host *host) if (ret) goto out; + mmc_card_set_ds(host->card); ret = mmc_sdio_init_card(host, host->card->ocr, host->card, mmc_card_keep_power(host)); if (!ret && host->sdio_irqs) diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h index c119735..f2c2620 100644 --- a/include/linux/mmc/card.h +++ b/include/linux/mmc/card.h @@ -260,6 +260,11 @@ struct mmc_card { #define MMC_STATE_HIGHSPEED_200 (1<<8) /* card is in HS200 mode */ #define MMC_STATE_DOING_BKOPS (1<<10) /* card is doing BKOPS */ #define MMC_STATE_SUSPENDED (1<<11) /* card is suspended */ +#define MMC_STATE_SPEED_MASK (MMC_STATE_HIGHSPEED | \ + MMC_STATE_HIGHSPEED_DDR | \ + MMC_STATE_ULTRAHIGHSPEED | \ + MMC_STATE_HIGHSPEED_200) + /* Mask for default speed(DS) */ unsigned int quirks; /* card quirks */ #define MMC_QUIRK_LENIENT_FN0 (1<<0) /* allow SDIO FN0 writes outside of the VS CCCR range */ #define MMC_QUIRK_BLKSZ_FOR_BYTE_MODE (1<<1) /* use func->cur_blksize */ @@ -431,19 +436,39 @@ static inline void __maybe_unused remove_quirk(struct mmc_card *card, int data) #define mmc_card_doing_bkops(c) ((c)->state & MMC_STATE_DOING_BKOPS) #define mmc_card_suspended(c) ((c)->state & MMC_STATE_SUSPENDED) -#define mmc_card_set_present(c) ((c)->state |= MMC_STATE_PRESENT) + #define mmc_card_set_readonly(c) ((c)->state |= MMC_STATE_READONLY) -#define mmc_card_set_highspeed(c) ((c)->state |= MMC_STATE_HIGHSPEED) -#define mmc_card_set_hs200(c) ((c)->state |= MMC_STATE_HIGHSPEED_200) #define mmc_card_set_blockaddr(c) ((c)->state |= MMC_STATE_BLOCKADDR) -#define mmc_card_set_ddr_mode(c) ((c)->state |= MMC_STATE_HIGHSPEED_DDR) -#define mmc_card_set_uhs(c) ((c)->state |= MMC_STATE_ULTRAHIGHSPEED) #define mmc_card_set_ext_capacity(c) ((c)->state |= MMC_CARD_SDXC) -#define mmc_card_set_removed(c) ((c)->state |= MMC_CARD_REMOVED) #define mmc_card_set_doing_bkops(c) ((c)->state |= MMC_STATE_DOING_BKOPS) #define mmc_card_clr_doing_bkops(c) ((c)->state &= ~MMC_STATE_DOING_BKOPS) #define mmc_card_set_suspended(c) ((c)->state |= MMC_STATE_SUSPENDED) #define mmc_card_clr_suspended(c) ((c)->state &= ~MMC_STATE_SUSPENDED) +#define mmc_card_set_highspeed(c) \ + ((c)->state = \ + ((c)->state & ~MMC_STATE_SPEED_MASK) | \ + MMC_STATE_HIGHSPEED) +#define mmc_card_set_ddr_mode(c) \ + ((c)->state = \ + ((c)->state & ~MMC_STATE_SPEED_MASK) | \ + MMC_STATE_HIGHSPEED_DDR) +#define mmc_card_set_hs200(c) \ + ((c)->state = \ + ((c)->state & ~MMC_STATE_SPEED_MASK) | \ + MMC_STATE_HIGHSPEED_200) +#define mmc_card_set_uhs(c) \ + ((c)->state = \ + ((c)->state & ~MMC_STATE_SPEED_MASK) | \ + MMC_STATE_ULTRAHIGHSPEED) +#define mmc_card_set_present(c) \ + ((c)->state = \ + ((c)->state & ~MMC_CARD_REMOVED) | \ + MMC_STATE_PRESENT) +#define mmc_card_set_removed(c) \ + ((c)->state = \ + ((c)->state & ~MMC_STATE_PRESENT) | \ + MMC_CARD_REMOVED) +#define mmc_card_set_ds(c) ((c)->state &= ~MMC_STATE_SPEED_MASK) /* * Quirk add/remove for MMC products.
Card state related to speed mode should be in non-overlapped. Consideration for all cases is required when being cleared. Also, MMC_STATE_PRESENT and MMC_STATE_REMOVED are same. It's exclusive state which cannot be set at the same time. Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com> --- drivers/mmc/core/core.c | 1 - drivers/mmc/core/mmc.c | 4 ++-- drivers/mmc/core/sd.c | 5 +++-- drivers/mmc/core/sdio.c | 5 ++++- include/linux/mmc/card.h | 37 +++++++++++++++++++++++++++++++------ 5 files changed, 40 insertions(+), 12 deletions(-)