| Message ID | 9674ff5c-ac9a-6f76-0e60-be2fe70784bb@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Adrian, On 2018/5/16 18:57, Adrian Hunter wrote: > On 26/04/18 14:28, Shawn Lin wrote: >> On 2018/4/26 18:09, Adrian Hunter wrote: >>> On 24/04/18 12:13, Shawn Lin wrote: >>>> >>>> On 2018/4/13 15:55, Adrian Hunter wrote: >>>>> On 13/04/18 10:21, Shawn Lin wrote: >>>> >>>>> >>>>> No we always remove children (card) before the parent (host controller). >>>>> >>>> >>>> Ok, I agree with this. One thing now come to my mind is could we allow >>>> mmc_remove_host() to help remove children in advance or is it okay to >>>> add a new helper for host drivers to remove children in their >>>> ->remove()? >>> >>> Lets' step back a bit. At what point does the remove end up waiting? Does >>> it get to mmc_blk_remove()? >> >> I guess the remove might get to mmc_blk_remove for sd/(e)MMC cards. > > Yes, this works for me: > > diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c This alternative approach works for me , as it wisely never start pointless cycled claiming host, as well as avoid unnecessary switching part type, when removing the cards. Would you mind posting a separate patch for that or want me to prepare it in my v2? Thanks. > index 03e3d48b083e..54457d8745cd 100644 > --- a/drivers/mmc/core/block.c > +++ b/drivers/mmc/core/block.c > @@ -2968,9 +2968,11 @@ static void mmc_blk_remove(struct mmc_card *card) > mmc_blk_remove_debugfs(card, md); > mmc_blk_remove_parts(card, md); > pm_runtime_get_sync(&card->dev); > - mmc_claim_host(card->host); > - mmc_blk_part_switch(card, md->part_type); > - mmc_release_host(card->host); > + if (md->part_curr != md->part_type) { > + mmc_claim_host(card->host); > + mmc_blk_part_switch(card, md->part_type); > + mmc_release_host(card->host); > + } > if (card->type != MMC_TYPE_SD_COMBO) > pm_runtime_disable(&card->dev); > pm_runtime_put_noidle(&card->dev); > > -- > 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 17/05/18 05:10, Shawn Lin wrote: > Hi Adrian, > > On 2018/5/16 18:57, Adrian Hunter wrote: >> On 26/04/18 14:28, Shawn Lin wrote: >>> On 2018/4/26 18:09, Adrian Hunter wrote: >>>> On 24/04/18 12:13, Shawn Lin wrote: >>>>> >>>>> On 2018/4/13 15:55, Adrian Hunter wrote: >>>>>> On 13/04/18 10:21, Shawn Lin wrote: >>>>> >>>>>> >>>>>> No we always remove children (card) before the parent (host controller). >>>>>> >>>>> >>>>> Ok, I agree with this. One thing now come to my mind is could we allow >>>>> mmc_remove_host() to help remove children in advance or is it okay to >>>>> add a new helper for host drivers to remove children in their >>>>> ->remove()? >>>> >>>> Lets' step back a bit. At what point does the remove end up waiting? Does >>>> it get to mmc_blk_remove()? >>> >>> I guess the remove might get to mmc_blk_remove for sd/(e)MMC cards. >> >> Yes, this works for me: >> >> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c > > This alternative approach works for me , as it wisely never start > pointless cycled claiming host, as well as avoid unnecessary switching > part type, when removing the cards. > > Would you mind posting a separate patch for that or want me to prepare > it in my v2? I don't have time. But I also saw I/O errors upon unbind: [ 118.357491] Buffer I/O error on dev mmcblk0, logical block 50560, async page read [ 118.365903] Buffer I/O error on dev mmcblk0, logical block 50560, async page read It is not a big thing, but maybe they could be silenced. > > Thanks. > >> index 03e3d48b083e..54457d8745cd 100644 >> --- a/drivers/mmc/core/block.c >> +++ b/drivers/mmc/core/block.c >> @@ -2968,9 +2968,11 @@ static void mmc_blk_remove(struct mmc_card *card) >> mmc_blk_remove_debugfs(card, md); >> mmc_blk_remove_parts(card, md); >> pm_runtime_get_sync(&card->dev); >> - mmc_claim_host(card->host); >> - mmc_blk_part_switch(card, md->part_type); >> - mmc_release_host(card->host); >> + if (md->part_curr != md->part_type) { >> + mmc_claim_host(card->host); >> + mmc_blk_part_switch(card, md->part_type); >> + mmc_release_host(card->host); >> + } >> if (card->type != MMC_TYPE_SD_COMBO) >> pm_runtime_disable(&card->dev); >> pm_runtime_put_noidle(&card->dev); >> >> -- >> 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
diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c index 03e3d48b083e..54457d8745cd 100644 --- a/drivers/mmc/core/block.c +++ b/drivers/mmc/core/block.c @@ -2968,9 +2968,11 @@ static void mmc_blk_remove(struct mmc_card *card) mmc_blk_remove_debugfs(card, md); mmc_blk_remove_parts(card, md); pm_runtime_get_sync(&card->dev); - mmc_claim_host(card->host); - mmc_blk_part_switch(card, md->part_type); - mmc_release_host(card->host); + if (md->part_curr != md->part_type) { + mmc_claim_host(card->host); + mmc_blk_part_switch(card, md->part_type); + mmc_release_host(card->host); + } if (card->type != MMC_TYPE_SD_COMBO) pm_runtime_disable(&card->dev); pm_runtime_put_noidle(&card->dev);