Re: [PATCH] mmc: core: improve rationality of bus width setting for HS400es

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux