Re: [PATCH v8 1/2] mmc: core: Add infrastructure for undervoltage handling

[Ulf Hansson <ulf.hansson@linaro.org>]

[...]

> diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
> index f14671ea5716..5f0ec23aeff5 100644
> --- a/drivers/mmc/core/host.c
> +++ b/drivers/mmc/core/host.c
> @@ -564,6 +564,8 @@ struct mmc_host *mmc_alloc_host(int extra, struct device *dev)
>         INIT_WORK(&host->sdio_irq_work, sdio_irq_work);
>         timer_setup(&host->retune_timer, mmc_retune_timer, 0);
>
> +       INIT_WORK(&host->supply.uv_work, mmc_undervoltage_workfn);
> +
>         /*
>          * By default, hosts do not support SGIO or large requests.
>          * They have to set these according to their abilities.
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index 5be9b42d5057..7dc0a9339c5e 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -2370,6 +2370,7 @@ int mmc_attach_mmc(struct mmc_host *host)
>                 goto remove_card;
>
>         mmc_claim_host(host);
> +

White space. Please drop it.

>         return 0;
>
>  remove_card:

[...]

> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> index 68f09a955a90..0d2685365c6b 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -11,6 +11,7 @@
>  #include <linux/device.h>
>  #include <linux/fault-inject.h>
>  #include <linux/debugfs.h>
> +#include <linux/workqueue.h>

We should not include header-files from header-files, unless it's
actually needed. In this case, we can do a forward declaration
instead.

struct notifier_block;
struct work_struct;

>
>  #include <linux/mmc/core.h>
>  #include <linux/mmc/card.h>
> @@ -342,6 +343,9 @@ struct mmc_supply {
>         struct regulator *vmmc;         /* Card power supply */
>         struct regulator *vqmmc;        /* Optional Vccq supply */
>         struct regulator *vqmmc2;       /* Optional supply for phy */
> +
> +       struct notifier_block vmmc_nb;  /* Notifier for vmmc */
> +       struct work_struct uv_work;     /* Undervoltage work */
>  };
>
>  struct mmc_ctx {
> @@ -494,6 +498,13 @@ struct mmc_host {
>         unsigned int            can_dma_map_merge:1; /* merging can be used */
>         unsigned int            vqmmc_enabled:1; /* vqmmc regulator is enabled */
>
> +       /*
> +        * Indicates if an undervoltage event has already been handled.
> +        * This prevents repeated regulator notifiers from triggering
> +        * multiple REGULATOR_EVENT_UNDER_VOLTAGE events.
> +        */
> +       unsigned int            undervoltage:1;  /* Undervoltage state */
> +
>         int                     rescan_disable; /* disable card detection */
>         int                     rescan_entered; /* used with nonremovable devices */
>
> @@ -659,6 +670,9 @@ static inline int mmc_regulator_set_vqmmc2(struct mmc_host *mmc,
>  int mmc_regulator_get_supply(struct mmc_host *mmc);
>  int mmc_regulator_enable_vqmmc(struct mmc_host *mmc);
>  void mmc_regulator_disable_vqmmc(struct mmc_host *mmc);
> +void mmc_regulator_register_undervoltage_notifier(struct mmc_host *host);
> +void mmc_regulator_unregister_undervoltage_notifier(struct mmc_host *host);
> +void mmc_undervoltage_workfn(struct work_struct *work);

We don't need these to be available outside the mmc core, hence please
move these three to drivers/mmc/core/core.h instead.

>
>  static inline int mmc_card_is_removable(struct mmc_host *host)
>  {
> --
> 2.39.5
>

Kind regards
Uffe