mmc: core: Set card as removed in mmc_remove_card()
diff mbox

Message ID 9674ff5c-ac9a-6f76-0e60-be2fe70784bb@intel.com
State New
Headers show

Commit Message

Adrian Hunter May 16, 2018, 10:57 a.m. UTC
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:


--
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

Comments

Shawn Lin May 17, 2018, 2:10 a.m. UTC | #1
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
Adrian Hunter May 17, 2018, 6:43 a.m. UTC | #2
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

Patch
diff mbox

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);