On 10 July 2018 at 07:04, Fang Hongjie(方洪杰) <hongjiefang@xxxxxxxxxxxx> wrote:
>> On 9 July 2018 at 08:47, Hongjie Fang <hongjiefang@xxxxxxxxxxxx> wrote:
>> > mmc_select_hs400es() calls mmc_select_bus_width() which will try to
>> > set 4bit transfer mode if fail to set 8bit mode. The problem is that
>> > the bus width should not be set to 4bit in HS400es mode.
>>
>> I guess it fails because there is something wrong. Can you please
>> elaborate under what circumstance the problem occurs?
>>
> This problem very occasionally occurred when system resume from
> suspend state. The card stuck in the programming state after setting
> 8bit mode. After that, mmc_select_bus_width() will continue to set
> 4bit mode and return without errors. Of course, the following R/W
> requests will be all failing.
I see. So it's the mmc_switch() operation that fails, when trying 8-bit. Hmm.
>
> The problem scene is hard to reproduce. Maybe it happened
> because there is something wrong about bus clock or core voltage
> stability when system do resume.
Yeah, that needs further debugging.
Don't know what host driver you are using, but potentially you could
play with a couple host specific mechanism. The ->host->card_busy()
callback, host->max_busy_timeout and MMC_CAP_WAIT_WHILE_BUSY.
Depending on these, __mmc_switch() changes its policy when polling for
busy. Have a look at __mmc_switch() and mmc_poll_for_busy(), to see
what goes on.
>
>> >
>> > Signed-off-by: Hongjie Fang <hongjiefang@xxxxxxxxxxxx>
>> > ---
>> > drivers/mmc/core/mmc.c | 23 +++++++++++++++++++++--
>> > 1 file changed, 21 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>> > index 4466f5d..94c3cc5 100644
>> > --- a/drivers/mmc/core/mmc.c
>> > +++ b/drivers/mmc/core/mmc.c
>> > @@ -1337,9 +1337,28 @@ static int mmc_select_hs400es(struct mmc_card *card)
>> > if (err)
>> > goto out_err;
>> >
>> > - err = mmc_select_bus_width(card);
>> > - if (err < 0)
>> > + /* Switch card to 8 bit mode */
>> > + err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>> > + EXT_CSD_BUS_WIDTH,
>> > + EXT_CSD_BUS_WIDTH_8,
>> > + card->ext_csd.generic_cmd6_time);
>> > + if (err) {
>> > + pr_err("%s: switch to 8 bit mode for hs400es failed, err:%d\n",
>> > + mmc_hostname(host), err);
>> > + goto out_err;
>> > + }
>> > +
>> > + mmc_set_bus_width(host, MMC_BUS_WIDTH_8);
>> > +
>> > + if (!(host->caps & MMC_CAP_BUS_WIDTH_TEST))
>> > + err = mmc_compare_ext_csds(card, MMC_BUS_WIDTH_8);
>> > + else
>> > + err = mmc_bus_test(card, MMC_BUS_WIDTH_8);
>> > + if (err) {
>> > + pr_err("%s: test 8 bit mode for hs400es failed, err:%d\n",
>> > + mmc_hostname(host), err);
>> > goto out_err;
>>
>> Lot's open coding, no I don't like it.
>>
>> Wouldn't it instead be possible to check the return value from
>> mmc_select_bus_width(), as it should tell what width it manage to
>> select. Then if it isn't 8-bit, then we should bail out. Does that
>> work?
>>
> After setting 4bit mode, it can't work because it cannot back to
> HS200 mode like the HS400 mode.
> For HS400es, if fail to set 8bit mode, maybe it is more reasonable
> that return error directly.
Yeah. Care to send a patch for that?
[...]
Kind regards
Uffe
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
