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

[Fang Hongjie(æææ)]

> 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.

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.

> >
> > 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.

> > +       }
> >
> >         /* Switch card to HS mode */
> >         err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
> > --
> > 1.9.1
> >
> 
> Kind regards
> Uffe


B&R
Hongjie