On Fri, 5 Apr 2019 at 10:35, Andrea Merello <andrea.merello@xxxxxxxxx> wrote:
>
> pwrseq_emmc.c implements a HW reset procedure for eMMC chip by driving a
> GPIO line.
>
> It registers the .reset() cb on mmc_pwrseq_ops and it registers a system
> restart notification handler; both of them perform reset by unconditionally
> calling gpiod_set_value().
>
> If the eMMC reset line is tied to a GPIO controller whose driver can sleep
> (i.e. I2C GPIO controller), then the kernel would spit warnings when trying
> to reset the eMMC chip by means of .reset() mmc_pwrseq_ops cb (that is
> exactly what I'm seeing during boot).
>
> Furthermore, on system reset we would gets to the system restart
> notification handler with disabled interrupts - local_irq_disable() is
> called in machine_restart() at least on ARM/ARM64 - and we would be in
> trouble when the GPIO driver tries to sleep (which indeed doesn't happen
> here, likely because in my case the machine specific code doesn't call
> do_kernel_restart(), I guess..).
>
> This patch fixes the .reset() cb to make use of gpiod_set_value_cansleep(),
> so that the eMMC gets reset on boot without complaints, while, since there
> isn't that much we can do, we avoid register the restart handler if the
> GPIO controller has a sleepy driver (and we spit a dev_notice() message to
> let people know)..
>
> This had been tested on a downstream 4.9 kernel with backported
> commit 83f37ee7ba33 ("mmc: pwrseq: Add reset callback to the struct
> mmc_pwrseq_ops") and commit ae60fb031cf2 ("mmc: core: Don't do eMMC HW
> reset when resuming the eMMC card"), because I couldn't boot my board
> otherwise. Maybe worth to RFT.
>
> Signed-off-by: Andrea Merello <andrea.merello@xxxxxxxxx>
Applied for next, thanks!
Perhaps we want this to go stable as well?
Kind regards
Uffe
>
> diff --git a/drivers/mmc/core/pwrseq_emmc.c b/drivers/mmc/core/pwrseq_emmc.c
> index efb8a7965dd4..154f4204d58c 100644
> --- a/drivers/mmc/core/pwrseq_emmc.c
> +++ b/drivers/mmc/core/pwrseq_emmc.c
> @@ -30,19 +30,14 @@ struct mmc_pwrseq_emmc {
>
> #define to_pwrseq_emmc(p) container_of(p, struct mmc_pwrseq_emmc, pwrseq)
>
> -static void __mmc_pwrseq_emmc_reset(struct mmc_pwrseq_emmc *pwrseq)
> -{
> - gpiod_set_value(pwrseq->reset_gpio, 1);
> - udelay(1);
> - gpiod_set_value(pwrseq->reset_gpio, 0);
> - udelay(200);
> -}
> -
> static void mmc_pwrseq_emmc_reset(struct mmc_host *host)
> {
> struct mmc_pwrseq_emmc *pwrseq = to_pwrseq_emmc(host->pwrseq);
>
> - __mmc_pwrseq_emmc_reset(pwrseq);
> + gpiod_set_value_cansleep(pwrseq->reset_gpio, 1);
> + udelay(1);
> + gpiod_set_value_cansleep(pwrseq->reset_gpio, 0);
> + udelay(200);
> }
>
> static int mmc_pwrseq_emmc_reset_nb(struct notifier_block *this,
> @@ -50,8 +45,11 @@ static int mmc_pwrseq_emmc_reset_nb(struct notifier_block *this,
> {
> struct mmc_pwrseq_emmc *pwrseq = container_of(this,
> struct mmc_pwrseq_emmc, reset_nb);
> + gpiod_set_value(pwrseq->reset_gpio, 1);
> + udelay(1);
> + gpiod_set_value(pwrseq->reset_gpio, 0);
> + udelay(200);
>
> - __mmc_pwrseq_emmc_reset(pwrseq);
> return NOTIFY_DONE;
> }
>
> @@ -72,14 +70,18 @@ static int mmc_pwrseq_emmc_probe(struct platform_device *pdev)
> if (IS_ERR(pwrseq->reset_gpio))
> return PTR_ERR(pwrseq->reset_gpio);
>
> - /*
> - * register reset handler to ensure emmc reset also from
> - * emergency_reboot(), priority 255 is the highest priority
> - * so it will be executed before any system reboot handler.
> - */
> - pwrseq->reset_nb.notifier_call = mmc_pwrseq_emmc_reset_nb;
> - pwrseq->reset_nb.priority = 255;
> - register_restart_handler(&pwrseq->reset_nb);
> + if (!gpiod_cansleep(pwrseq->reset_gpio)) {
> + /*
> + * register reset handler to ensure emmc reset also from
> + * emergency_reboot(), priority 255 is the highest priority
> + * so it will be executed before any system reboot handler.
> + */
> + pwrseq->reset_nb.notifier_call = mmc_pwrseq_emmc_reset_nb;
> + pwrseq->reset_nb.priority = 255;
> + register_restart_handler(&pwrseq->reset_nb);
> + } else {
> + dev_notice(dev, "EMMC reset pin tied to a sleepy GPIO driver; reset on emergency-reboot disabled\n");
> + }
>
> pwrseq->pwrseq.ops = &mmc_pwrseq_emmc_ops;
> pwrseq->pwrseq.dev = dev;
> --
> 2.17.1
>
