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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 10 Jul 2018 10:14:48 +0200
From:   Ulf Hansson <ulf.hansson@...aro.org>
To:     Fang Hongjie(方洪杰) 
        <hongjiefang@...micro.com>
Cc:     Adrian Hunter <adrian.hunter@...el.com>,
        Shawn Lin <shawn.lin@...k-chips.com>,
        Linus Walleij <linus.walleij@...aro.org>,
        Bastian Stender <bst@...gutronix.de>,
        Wolfram Sang <wsa+renesas@...g-engineering.com>,
        Chanho Min <chanho.min@....com>,
        Kyle Roeschley <kyle.roeschley@...com>,
        "linux-mmc@...r.kernel.org" <linux-mmc@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] mmc: core: improve rationality of bus width setting for HS400es

On 10 July 2018 at 07:04, Fang Hongjie(方洪杰) <hongjiefang@...micro.com> wrote:
>> On 9 July 2018 at 08:47, Hongjie Fang <hongjiefang@...micro.com> 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@...micro.com>
>> > ---
>> >  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