Re: [PATCH 0/3] mmc: Wait for card_busy before starting sdio requests

[Jaehoon Chung <jh80.chung@samsung.com>]

On 09/25/2015 07:06 PM, Hans de Goede wrote:
> Hi,
> 
> On 25-09-15 11:58, Jaehoon Chung wrote:
>> On 09/25/2015 06:41 PM, Hans de Goede wrote:
>>> Hi,
>>>
>>> On 25-09-15 11:37, Jaehoon Chung wrote:
>>>> Hi, Hans.
>>>>
>>>> On 09/25/2015 04:53 PM, Hans de Goede wrote:
>>>>> Hi,
>>>>>
>>>>> On 24-09-15 18:04, Doug Anderson wrote:
>>>>>> Hi,
>>>>>>
>>>>>> On Thu, Sep 24, 2015 at 2:19 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> On 23-09-15 23:43, Ulf Hansson wrote:
>>>>>>>>
>>>>>>>> On 22 September 2015 at 17:30, Hans de Goede <hdegoede@redhat.com> wrote:
>>>>>>>>>
>>>>>>>>> Hi Ulf,
>>>>>>>>>
>>>>>>>>> Here is a non RFC version of my patch-set to wait for card_busy before
>>>>>>>>> starting sdio requests. It is the same as the RFC version of the set,
>>>>>>>>> but this time it has been tested no hardware which actually needs this
>>>>>>>>> and I can confirm now that this fixes wifi on that hardware.
>>>>>>>>
>>>>>>>>
>>>>>>>> Great! Thanks, applied for next!
>>>>>>>
>>>>>>>
>>>>>>> Great, thanks, I guess it is too late for this to go as a fix into
>>>>>>> 4.3-rcX (no worries if it is) ?
>>>>>>>
>>>>>>>>> This patch-set should also allow removing this dw_mmc specific fix:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/mmc/host/dw_mmc.c?id=0bdbd0e88cf6b603a2196418672715b0890fb040
>>>>>>>>>
>>>>>>>>> As this patch-set fixes this problem in a generic manner.
>>>>>>>>
>>>>>>>>
>>>>>>>> Care to send a patch to remove the above hack/fix?
>>>>>>>
>>>>>>>
>>>>>>> I do not have any hardware to test this.
>>>>>>>
>>>>>>> I've added Doug the original author of that patch to the Cc.
>>>>>>>
>>>>>>> Dough, can you test if with the patch set from this mail thread
>>>>>>> (merged into mmc/next) this patch:
>>>>>>>
>>>>>>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/mmc/host/dw_mmc.c?id=0bdbd0e88cf6b603a2196418672715b0890fb040
>>>>>>>
>>>>>>> Is still necessary ? Since this patch-set fixes the same issue
>>>>>>> in the mmc core I believe that this commit can be reverted now.
>>>>>>
>>>>>> I'll try to find some time in the next few days to test, but I'm not
>>>>>> terribly hopeful we can just revert the patch because:
>>>>>>
>>>>>> 1. Only one of the two callers of dw_mci_wait_while_busy() is handled
>>>>>> by your patch.  mci_send_cmd() is used internally in dw_mmc to throw
>>>>>> something in the CMD register without going through the normal MMC
>>>>>> path.  This is used exclusively to update the clock registers in
>>>>>> dw_mmc.  I'm pretty sure this needs the wait, too.  It's always seemed
>>>>>> weird / awkward to me that you need to use the CMD register to update
>>>>>> clock settings in dw_mmc, but c'est la vie.
>>>>>
>>>>> I would not expect the card to signal busy when trying to change clocks
>>>>> though, so I do not think this will really be a problem.
>>>>
>>>> No. It shouldn't be occurred any problem.
>>>> But according to designware TRM, it needs to check whether card is busy or not, before updating clock.
>>>> I think even if problem will not occur, it doesn't mean this code is useless.
>>>>
>>>>>
>>>>>> 2. If I remember correctly, we ran into other instances where non-SDIO
>>>>>> cards needed the busy check.  It wasn't terribly common, but I think I
>>>>>> ran into this when stress testing, but only on a few cards.
>>>>>
>>>>> Hmm, that would be a problem yes.
>>>>>
>>>>>> The patch referenced here only seems to check for SDIO commands.  As I
>>>>>> understand it, to be correct, it should check for all data commands
>>>>>> (other than stop or voltage change commands).
>>>>>
>>>>> But that is not what the patch does, it actually waits for all commands,
>>>>> including non data commands. An earlier attempt of mine to fix the sdio
>>>>> wifi issues with the sunxi driver copied your approach, and I actually
>>>>> got reports of regressions with using normal micro-sd memory cards
>>>>> from several people testing that patch.
>>>>
>>>> I can't see any problem reported at mailing list.
>>>> Could you share more information what regressions issue?
>>>
>>> IIRC people where hitting the timeout in the code to wait for the card-busy.
>>>
>>> Now that I think about this, this may have been caused by waiting
>>> for card-busy while sending a stop.
>>
>> I understood what problem you said.
>> Well, but i don't accept your opinion yet.
>> Is that case reproduced with dwmmc controller? otherwise...sunxi driver?
>> (It seems that you mentioned the case of sunxi driver.)
> 
> I was talking about the sunxi driver only. A few weeks back I did a
> patch for the sunxi driver which copied the dw-mmc fix, and some users
> testing that saw issues accessing certain micro-sd memory cards.

Designware needs to check other MMC/SD card before sending command. :)
I will maintain Doug's patch for dwmmc controller.
If similar issue is occurred, then i will analyze codes at that time.
Maybe, your comment helps to me at that time.

Best Regards,
Jaehoon Chung

> 
> Regards,
> 
> Hans
> -- 
> 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
>