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

[Hans de Goede]

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 at redhat.com> wrote:
>>>> Hi,
>>>>
>>>> On 23-09-15 23:43, Ulf Hansson wrote:
>>>>>
>>>>> On 22 September 2015 at 17:30, Hans de Goede <hdegoede at 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.

Regards,

Hans



>
> Best Regards,
> Jaehoon Chung
>
>>
>> And if you're right that we should wait for all data commands, then
>> I wonder if this is a designware thing (I believe the allwinner
>> mmc controller is designware derived) or a generic mmc / sdio thing ?
>>
>>> The Designware Databook
>>> makes no reference to only needing the wait for SDIO commands.
>>
>> Yet your commit message references problems with sdio wifi cards, and
>> on sunxi we've only been seeing this problem with sdio wifi cards / sdio
>> commands.
>>
>>> ...of course, it's always possible that some of the things I saw above
>>> will no longer happen with all the other fixes we've done in the
>>> meantime (turning on voltages at the right time, adding the right
>>> delays, etc).
>>>
>>>
>>> Note that I've hardly looked at sdhci at all, but on SDHCI is this
>>> handled by the "SDHCI_DATA_INHIBIT" bits?
>>
>> Regards,
>>
>> Hans
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>> the body of a message to majordomo at vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>