Re: [PATCH V3 1/3] mmc: sdhci: Allow platform controlled voltage switching

From: Evan Green
Date: Thu Nov 15 2018 - 18:17:43 EST


On Wed, Nov 14, 2018 at 6:36 AM Veerabhadrarao Badiganti
<vbadigan@xxxxxxxxxxxxxx> wrote:
>
> >>> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
> >>> index b001cf4..3c28152 100644
> >>> --- a/drivers/mmc/host/sdhci.h
> >>> +++ b/drivers/mmc/host/sdhci.h
> >>> @@ -524,6 +524,7 @@ struct sdhci_host {
> >>> bool pending_reset; /* Cmd/data reset is pending */
> >>> bool irq_wake_enabled; /* IRQ wakeup is enabled */
> >>> bool v4_mode; /* Host Version 4 Enable */
> >>> + bool vqmmc_enabled; /* Vqmmc is enabled */
> >> I still don't love this, since it doesn't mean what it says. Everyone
> >> else that has a vqmmc_enabled member uses it to actually mean "vqmmc
> >> is enabled", but this doesn't mean that. For example, you don't clear
> >> this when you disable the regulator in patch 3, so this would be set
> >> even if the regulator is disabled, and you don't set it when sdhci
> >> enables the regulator, so the regulator is on when this flag is not
> >> set.
> >>
> Hi Evan
>
> This flag is meant to say "disable vqmmc *only* if it is enabled by host
> driver (sdhci_host)".
> If host driver doesn't enable vqmmc (enabled by platfrm driver) or if it
> fails to enable it, then don't call disable vqmmc.
>
> Agree with you, the present name is not conveying its purpose.
> It must be something like "vqmmc_enabled_by_host".
>
> Please let me know if you have any suggestions on this name.

Yeah. Maybe vqmmc_pltfrm_controlled? Or vqmmc_enabled_by_platfrm as
you suggested?
-Evan