[PATCH 2/3] mmc: core: remove the invalid message in mmc_select_timing

Ulf Hansson ulf.hansson at linaro.org
Thu Apr 28 02:24:07 PDT 2016


On 20 April 2016 at 18:51, Dong Aisheng <aisheng.dong at nxp.com> wrote:
> mmc_select_hs200() and mmc_select_hs() will keep the timing
> as before if switch fails. So it's meaningless to print the
> failed switched mode outside based on the current host timing.
>
> Furthermore, the original print is wrong, it should be:
> pr_warn("%s: switch to %s failed\n",
>         mmc_hostname(card->host),
>         mmc_card_hs(card) ? "high-speed" :
>         (mmc_card_hs200(card) ? "hs200" : ""));
>
> Since we already have error message in mmc_select_hs200(),
> simply remove it outside.
>
> Signed-off-by: Dong Aisheng <aisheng.dong at nxp.com>
> ---
>  drivers/mmc/core/mmc.c | 10 +---------
>  1 file changed, 1 insertion(+), 9 deletions(-)
>
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index f6683a9..55c8201 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -1321,21 +1321,13 @@ static int mmc_select_timing(struct mmc_card *card)
>         if (err && err != -EBADMSG)
>                 return err;
>
> -       if (err) {
> -               pr_warn("%s: switch to %s failed\n",
> -                       mmc_card_hs(card) ? "high-speed" :
> -                       (mmc_card_hs200(card) ? "hs200" : ""),
> -                       mmc_hostname(card->host));
> -               err = 0;
> -       }
> -

No, this is not correct.

First, in patch1/3, you change to return an error code immediately
when __mmc_set_signal_voltage() fails, instead of using the "goto
err". Thus there will be no error printed for this case any more.

Second, mmc_select_hs() may fail and there is no print done within
that function which is why above print is also needed.

On the other hand I agree with you, it doesn't seems necessary to
print two messages when mmc_select_hs200() fails. One should be
enough. Can we change this so *only* mmc_select_timing() will deal
with the print at all errors?

>  bus_speed:
>         /*
>          * Set the bus speed to the selected bus timing.
>          * If timing is not selected, backward compatible is the default.
>          */
>         mmc_set_bus_speed(card);
> -       return err;
> +       return 0;
>  }
>
>  /*
> --
> 1.9.1
>

Kind regards
Uffe



More information about the linux-arm-kernel mailing list