Dear Uffe,
Thank you for your review comment. :)
I'll split this patch into two, and re-submit this as your request.
> -----Original Message-----
> From: Ulf Hansson <ulf.hansson@xxxxxxxxxx>
> Sent: Tuesday, July 12, 2022 7:57 PM
> To: Seunghui Lee <sh043.lee@xxxxxxxxxxx>
> Cc: linux-mmc@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> axboe@xxxxxxxxx; Avri.Altman@xxxxxxx; shawn.lin@xxxxxxxxxxxxxx;
> adrian.hunter@xxxxxxxxx; grant.jung@xxxxxxxxxxx; jt77.jang@xxxxxxxxxxx;
> dh0421.hwang@xxxxxxxxxxx; junwoo80.lee@xxxxxxxxxxx; jangsub.yi@xxxxxxxxxxx;
> cw9316.lee@xxxxxxxxxxx; sh8267.baek@xxxxxxxxxxx; wkon.kim@xxxxxxxxxxx;
> seunghwan.hyun@xxxxxxxxxxx
> Subject: Re: [PATCH] mmc: use mmc_card_* macro and add it for sd_combo
>
> On Tue, 5 Jul 2022 at 03:25, Seunghui Lee <sh043.lee@xxxxxxxxxxx> wrote:
> >
> > add mmc_card_sd_combo() macro for sd combo type card and use
> > mmc_card_* macro to simplify instead of comparing card->type
> >
> > Signed-off-by: Seunghui Lee <sh043.lee@xxxxxxxxxxx>
>
> Nice cleanup! I minor thing though, see below.
>
> > ---
> > drivers/mmc/core/block.c | 4 ++--
> > drivers/mmc/core/bus.c | 4 ++--
> > drivers/mmc/core/sd.c | 2 +-
> > drivers/mmc/core/sdio.c | 16 ++++++++--------
> > drivers/mmc/host/mxcmmc.c | 2 +-
>
> Please split this patch into two. One for the core and one for the mxcmmc
> driver.
>
> Otherwise this looks good to me!
>
> Kind regards
> Uffe
>
> > include/linux/mmc/card.h | 1 +
> > 6 files changed, 15 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c index
> > bda6c67ce93f..4d7ae8fc2901 100644
> > --- a/drivers/mmc/core/block.c
> > +++ b/drivers/mmc/core/block.c
> > @@ -2987,7 +2987,7 @@ static int mmc_blk_probe(struct mmc_card *card)
> > * Don't enable runtime PM for SD-combo cards here. Leave that
> > * decision to be taken during the SDIO init sequence instead.
> > */
> > - if (card->type != MMC_TYPE_SD_COMBO) {
> > + if (!mmc_card_sd_combo(card)) {
> > pm_runtime_set_active(&card->dev);
> > pm_runtime_enable(&card->dev);
> > }
> > @@ -3014,7 +3014,7 @@ static void mmc_blk_remove(struct mmc_card *card)
> > mmc_blk_part_switch(card, md->part_type);
> > mmc_release_host(card->host);
> > }
> > - if (card->type != MMC_TYPE_SD_COMBO)
> > + if (!mmc_card_sd_combo(card))
> > pm_runtime_disable(&card->dev);
> > pm_runtime_put_noidle(&card->dev);
> > mmc_blk_remove_req(md);
> > diff --git a/drivers/mmc/core/bus.c b/drivers/mmc/core/bus.c index
> > 58a60afa650b..d8762fa3d5cd 100644
> > --- a/drivers/mmc/core/bus.c
> > +++ b/drivers/mmc/core/bus.c
> > @@ -85,7 +85,7 @@ mmc_bus_uevent(struct device *dev, struct
> kobj_uevent_env *env)
> > return retval;
> > }
> >
> > - if (card->type == MMC_TYPE_SDIO || card->type ==
> MMC_TYPE_SD_COMBO) {
> > + if (mmc_card_sdio(card) || mmc_card_sd_combo(card)) {
> > retval = add_uevent_var(env, "SDIO_ID=%04X:%04X",
> > card->cis.vendor, card->cis.device);
> > if (retval)
> > @@ -107,7 +107,7 @@ mmc_bus_uevent(struct device *dev, struct
> kobj_uevent_env *env)
> > * SDIO (non-combo) cards are not handled by mmc_block driver and
> do not
> > * have accessible CID register which used by mmc_card_name()
> function.
> > */
> > - if (card->type == MMC_TYPE_SDIO)
> > + if (mmc_card_sdio(card))
> > return 0;
> >
> > retval = add_uevent_var(env, "MMC_NAME=%s",
> > mmc_card_name(card)); diff --git a/drivers/mmc/core/sd.c
> > b/drivers/mmc/core/sd.c index c5f1df6ce4c0..f0186bdf2025 100644
> > --- a/drivers/mmc/core/sd.c
> > +++ b/drivers/mmc/core/sd.c
> > @@ -793,7 +793,7 @@ static umode_t sd_std_is_visible(struct kobject
> *kobj, struct attribute *attr,
> > attr == &dev_attr_info2.attr ||
> > attr == &dev_attr_info3.attr ||
> > attr == &dev_attr_info4.attr
> > - ) && card->type != MMC_TYPE_SD_COMBO)
> > + ) &&!mmc_card_sd_combo(card))
> > return 0;
> >
> > return attr->mode;
> > diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c index
> > 25799accf8a0..b589df1c35e0 100644
> > --- a/drivers/mmc/core/sdio.c
> > +++ b/drivers/mmc/core/sdio.c
> > @@ -335,7 +335,7 @@ static int sdio_disable_4bit_bus(struct mmc_card
> > *card) {
> > int err;
> >
> > - if (card->type == MMC_TYPE_SDIO)
> > + if (mmc_card_sdio(card))
> > goto out;
> >
> > if (!(card->host->caps & MMC_CAP_4_BIT_DATA)) @@ -360,7 +360,7
> > @@ static int sdio_enable_4bit_bus(struct mmc_card *card)
> > err = sdio_enable_wide(card);
> > if (err <= 0)
> > return err;
> > - if (card->type == MMC_TYPE_SDIO)
> > + if (mmc_card_sdio(card))
> > goto out;
> >
> > if (card->scr.bus_widths & SD_SCR_BUS_WIDTH_4) { @@ -415,7
> > +415,7 @@ static int sdio_enable_hs(struct mmc_card *card)
> > int ret;
> >
> > ret = mmc_sdio_switch_hs(card, true);
> > - if (ret <= 0 || card->type == MMC_TYPE_SDIO)
> > + if (ret <= 0 || mmc_card_sdio(card))
> > return ret;
> >
> > ret = mmc_sd_switch_hs(card);
> > @@ -441,7 +441,7 @@ static unsigned mmc_sdio_get_max_clock(struct
> mmc_card *card)
> > max_dtr = card->cis.max_dtr;
> > }
> >
> > - if (card->type == MMC_TYPE_SD_COMBO)
> > + if (mmc_card_sd_combo(card))
> > max_dtr = min(max_dtr, mmc_sd_get_max_clock(card));
> >
> > return max_dtr;
> > @@ -689,7 +689,7 @@ static int mmc_sdio_init_card(struct mmc_host *host,
> u32 ocr,
> > mmc_sd_get_cid(host, ocr & rocr, card->raw_cid, NULL) == 0) {
> > card->type = MMC_TYPE_SD_COMBO;
> >
> > - if (oldcard && (oldcard->type != MMC_TYPE_SD_COMBO ||
> > + if (oldcard && (!mmc_card_sd_combo(oldcard) ||
> > memcmp(card->raw_cid, oldcard->raw_cid, sizeof(card-
> >raw_cid)) != 0)) {
> > err = -ENOENT;
> > goto mismatch; @@ -697,7 +697,7 @@ static int
> > mmc_sdio_init_card(struct mmc_host *host, u32 ocr,
> > } else {
> > card->type = MMC_TYPE_SDIO;
> >
> > - if (oldcard && oldcard->type != MMC_TYPE_SDIO) {
> > + if (oldcard && !mmc_card_sdio(oldcard)) {
> > err = -ENOENT;
> > goto mismatch;
> > }
> > @@ -754,7 +754,7 @@ static int mmc_sdio_init_card(struct mmc_host *host,
> u32 ocr,
> > /*
> > * Read CSD, before selecting the card
> > */
> > - if (!oldcard && card->type == MMC_TYPE_SD_COMBO) {
> > + if (!oldcard && mmc_card_sd_combo(card)) {
> > err = mmc_sd_get_csd(card);
> > if (err)
> > goto remove;
> > @@ -827,7 +827,7 @@ static int mmc_sdio_init_card(struct mmc_host
> > *host, u32 ocr,
> >
> > mmc_fixup_device(card, sdio_fixup_methods);
> >
> > - if (card->type == MMC_TYPE_SD_COMBO) {
> > + if (mmc_card_sd_combo(card)) {
> > err = mmc_sd_setup_card(host, card, oldcard != NULL);
> > /* handle as SDIO-only card if memory init failed */
> > if (err) {
> > diff --git a/drivers/mmc/host/mxcmmc.c b/drivers/mmc/host/mxcmmc.c
> > index 613f13306433..2cf0413407ea 100644
> > --- a/drivers/mmc/host/mxcmmc.c
> > +++ b/drivers/mmc/host/mxcmmc.c
> > @@ -923,7 +923,7 @@ static void mxcmci_init_card(struct mmc_host *host,
> struct mmc_card *card)
> > * One way to prevent this is to only allow 1-bit transfers.
> > */
> >
> > - if (is_imx31_mmc(mxcmci) && card->type == MMC_TYPE_SDIO)
> > + if (is_imx31_mmc(mxcmci) && mmc_card_sdio(card))
> > host->caps &= ~MMC_CAP_4_BIT_DATA;
> > else
> > host->caps |= MMC_CAP_4_BIT_DATA; diff --git
> > a/include/linux/mmc/card.h b/include/linux/mmc/card.h index
> > 37f975875102..156a7b673a28 100644
> > --- a/include/linux/mmc/card.h
> > +++ b/include/linux/mmc/card.h
> > @@ -348,5 +348,6 @@ bool mmc_card_is_blockaddr(struct mmc_card *card);
> > #define mmc_card_mmc(c) ((c)->type == MMC_TYPE_MMC)
> > #define mmc_card_sd(c) ((c)->type == MMC_TYPE_SD)
> > #define mmc_card_sdio(c) ((c)->type == MMC_TYPE_SDIO)
> > +#define mmc_card_sd_combo(c) ((c)->type == MMC_TYPE_SD_COMBO)
> >
> > #endif /* LINUX_MMC_CARD_H */
> > --
> > 2.29.0
> >
