[PATCH v4 0/2] mmc: sdhci: Use mmc core regulator infrastucture

[PATCH v4 0/2] mmc: sdhci: Use mmc core regulator infrastucture

[Markus Mayer]

This series switches the common SDHCI code over to use mmc_host's
regulator pointers rather than keeping its own set.

In addition, we can now re-use the newly introduced local "mmc" pointer
in several function calls in lieu of using host->mmc.

The first patch in the series has been posted before
(https://lkml.org/lkml/2014/4/24/947). The follow-on patch has not.

This series is based on mainline (commit 16b9057804).

Changes from v3:
  - Squashed patches 2 & 3 together. The series now has only 2 parts,
    but is otherwise unchanged.

Changes from v2:
  - Replaced a few more host->mmc references with mmc in
    sdhci_add_host(). This change affects PATCH 3/3 only. The other two
    patches are unchanged from v2.
  - Rebased the series on mainline (rather than mmc-next). Doing so did
    not affect the patches themselves. They applied cleanly as-is.

Changes from v1:
  - Resolved merge conflicts resulting from RMK's MMC series. The most
    significant conflict was the move of regulator code from
    sdhci_do_set_ios() to sdhci_set_power().
  - Added follow-on patches 2 & 3 of the series.
  - Updated Tim Kryger's e-mail address.

Markus Mayer (1):
  mmc: sdhci: Replace host->mmc with mmc where possible

Tim Kryger (1):
  mmc: sdhci: Use mmc core regulator infrastucture

 drivers/mmc/host/sdhci.c  | 123 ++++++++++++++++++----------------------------
 include/linux/mmc/sdhci.h |   3 --
 2 files changed, 49 insertions(+), 77 deletions(-)

-- 
1.9.1

[PATCH v4 1/2] mmc: sdhci: Use mmc core regulator infrastucture

[Markus Mayer]

From: Tim Kryger <tim.kryger@gmail.com>

Switch the common SDHCI code over to use mmc_host's regulator pointers
and remove the ones in the sdhci_host structure.  Additionally, use the
common mmc_regulator_get_supply function to get the regulators and set
the ocr_avail mask.

This change sets the ocr_avail directly based upon the voltage ranges
supported which ensures ocr_avail is set correctly while allowing the
use of regulators that can't provide exactly 1.8v, 3.0v, or 3.3v.

Signed-off-by: Tim Kryger <tim.kryger@gmail.com>
Signed-off-by: Markus Mayer <markus.mayer@linaro.org>
Reviewed-by: Matt Porter <mporter@linaro.org>
---
 drivers/mmc/host/sdhci.c  | 97 ++++++++++++++++++-----------------------------
 include/linux/mmc/sdhci.h |  3 --
 2 files changed, 36 insertions(+), 64 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 47055f3..ee524b0 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1223,6 +1223,7 @@ EXPORT_SYMBOL_GPL(sdhci_set_clock);
 static void sdhci_set_power(struct sdhci_host *host, unsigned char mode,
 			    unsigned short vdd)
 {
+	struct mmc_host *mmc = host->mmc;
 	u8 pwr = 0;
 
 	if (mode != MMC_POWER_OFF) {
@@ -1284,9 +1285,9 @@ static void sdhci_set_power(struct sdhci_host *host, unsigned char mode,
 			mdelay(10);
 	}
 
-	if (host->vmmc) {
+	if (!IS_ERR(mmc->supply.vmmc)) {
 		spin_unlock_irq(&host->lock);
-		mmc_regulator_set_ocr(host->mmc, host->vmmc, vdd);
+		mmc_regulator_set_ocr(host->mmc, mmc->supply.vmmc, vdd);
 		spin_lock_irq(&host->lock);
 	}
 }
@@ -1440,13 +1441,15 @@ static void sdhci_do_set_ios(struct sdhci_host *host, struct mmc_ios *ios)
 {
 	unsigned long flags;
 	u8 ctrl;
+	struct mmc_host *mmc = host->mmc;
 
 	spin_lock_irqsave(&host->lock, flags);
 
 	if (host->flags & SDHCI_DEVICE_DEAD) {
 		spin_unlock_irqrestore(&host->lock, flags);
-		if (host->vmmc && ios->power_mode == MMC_POWER_OFF)
-			mmc_regulator_set_ocr(host->mmc, host->vmmc, 0);
+		if (!IS_ERR(mmc->supply.vmmc) &&
+		    ios->power_mode == MMC_POWER_OFF)
+			mmc_regulator_set_ocr(host->mmc, mmc->supply.vmmc, 0);
 		return;
 	}
 
@@ -1707,6 +1710,7 @@ static void sdhci_enable_sdio_irq(struct mmc_host *mmc, int enable)
 static int sdhci_do_start_signal_voltage_switch(struct sdhci_host *host,
 						struct mmc_ios *ios)
 {
+	struct mmc_host *mmc = host->mmc;
 	u16 ctrl;
 	int ret;
 
@@ -1725,8 +1729,9 @@ static int sdhci_do_start_signal_voltage_switch(struct sdhci_host *host,
 		ctrl &= ~SDHCI_CTRL_VDD_180;
 		sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2);
 
-		if (host->vqmmc) {
-			ret = regulator_set_voltage(host->vqmmc, 2700000, 3600000);
+		if (!IS_ERR(mmc->supply.vqmmc)) {
+			ret = regulator_set_voltage(mmc->supply.vqmmc, 2700000,
+						    3600000);
 			if (ret) {
 				pr_warning("%s: Switching to 3.3V signalling voltage "
 						" failed\n", mmc_hostname(host->mmc));
@@ -1746,8 +1751,8 @@ static int sdhci_do_start_signal_voltage_switch(struct sdhci_host *host,
 
 		return -EAGAIN;
 	case MMC_SIGNAL_VOLTAGE_180:
-		if (host->vqmmc) {
-			ret = regulator_set_voltage(host->vqmmc,
+		if (!IS_ERR(mmc->supply.vqmmc)) {
+			ret = regulator_set_voltage(mmc->supply.vqmmc,
 					1700000, 1950000);
 			if (ret) {
 				pr_warning("%s: Switching to 1.8V signalling voltage "
@@ -1776,8 +1781,9 @@ static int sdhci_do_start_signal_voltage_switch(struct sdhci_host *host,
 
 		return -EAGAIN;
 	case MMC_SIGNAL_VOLTAGE_120:
-		if (host->vqmmc) {
-			ret = regulator_set_voltage(host->vqmmc, 1100000, 1300000);
+		if (!IS_ERR(mmc->supply.vqmmc)) {
+			ret = regulator_set_voltage(mmc->supply.vqmmc, 1100000,
+						    1300000);
 			if (ret) {
 				pr_warning("%s: Switching to 1.2V signalling voltage "
 						" failed\n", mmc_hostname(host->mmc));
@@ -2962,25 +2968,22 @@ int sdhci_add_host(struct sdhci_host *host)
 	    !(host->mmc->caps & MMC_CAP_NONREMOVABLE))
 		mmc->caps |= MMC_CAP_NEEDS_POLL;
 
+	/* If there are external regulators, get them */
+	if (mmc_regulator_get_supply(mmc) == -EPROBE_DEFER)
+		return -EPROBE_DEFER;
+
 	/* If vqmmc regulator and no 1.8V signalling, then there's no UHS */
-	host->vqmmc = regulator_get_optional(mmc_dev(mmc), "vqmmc");
-	if (IS_ERR_OR_NULL(host->vqmmc)) {
-		if (PTR_ERR(host->vqmmc) < 0) {
-			pr_info("%s: no vqmmc regulator found\n",
-				mmc_hostname(mmc));
-			host->vqmmc = NULL;
-		}
-	} else {
-		ret = regulator_enable(host->vqmmc);
-		if (!regulator_is_supported_voltage(host->vqmmc, 1700000,
-			1950000))
+	if (!IS_ERR(mmc->supply.vqmmc)) {
+		ret = regulator_enable(mmc->supply.vqmmc);
+		if (!regulator_is_supported_voltage(mmc->supply.vqmmc, 1700000,
+						    1950000))
 			caps[1] &= ~(SDHCI_SUPPORT_SDR104 |
 					SDHCI_SUPPORT_SDR50 |
 					SDHCI_SUPPORT_DDR50);
 		if (ret) {
 			pr_warn("%s: Failed to enable vqmmc regulator: %d\n",
 				mmc_hostname(mmc), ret);
-			host->vqmmc = NULL;
+			mmc->supply.vqmmc = NULL;
 		}
 	}
 
@@ -3041,34 +3044,6 @@ int sdhci_add_host(struct sdhci_host *host)
 
 	ocr_avail = 0;
 
-	host->vmmc = regulator_get_optional(mmc_dev(mmc), "vmmc");
-	if (IS_ERR_OR_NULL(host->vmmc)) {
-		if (PTR_ERR(host->vmmc) < 0) {
-			pr_info("%s: no vmmc regulator found\n",
-				mmc_hostname(mmc));
-			host->vmmc = NULL;
-		}
-	}
-
-#ifdef CONFIG_REGULATOR
-	/*
-	 * Voltage range check makes sense only if regulator reports
-	 * any voltage value.
-	 */
-	if (host->vmmc && regulator_get_voltage(host->vmmc) > 0) {
-		ret = regulator_is_supported_voltage(host->vmmc, 2700000,
-			3600000);
-		if ((ret <= 0) || (!(caps[0] & SDHCI_CAN_VDD_330)))
-			caps[0] &= ~SDHCI_CAN_VDD_330;
-		if ((ret <= 0) || (!(caps[0] & SDHCI_CAN_VDD_300)))
-			caps[0] &= ~SDHCI_CAN_VDD_300;
-		ret = regulator_is_supported_voltage(host->vmmc, 1700000,
-			1950000);
-		if ((ret <= 0) || (!(caps[0] & SDHCI_CAN_VDD_180)))
-			caps[0] &= ~SDHCI_CAN_VDD_180;
-	}
-#endif /* CONFIG_REGULATOR */
-
 	/*
 	 * According to SD Host Controller spec v3.00, if the Host System
 	 * can afford more than 150mA, Host Driver should set XPC to 1. Also
@@ -3077,8 +3052,8 @@ int sdhci_add_host(struct sdhci_host *host)
 	 * value.
 	 */
 	max_current_caps = sdhci_readl(host, SDHCI_MAX_CURRENT);
-	if (!max_current_caps && host->vmmc) {
-		u32 curr = regulator_get_current_limit(host->vmmc);
+	if (!max_current_caps && !IS_ERR(mmc->supply.vmmc)) {
+		u32 curr = regulator_get_current_limit(mmc->supply.vmmc);
 		if (curr > 0) {
 
 			/* convert to SDHCI_MAX_CURRENT format */
@@ -3118,8 +3093,11 @@ int sdhci_add_host(struct sdhci_host *host)
 				   SDHCI_MAX_CURRENT_MULTIPLIER;
 	}
 
+	if (mmc->ocr_avail)
+		ocr_avail &= mmc->ocr_avail;
+
 	if (host->ocr_mask)
-		ocr_avail = host->ocr_mask;
+		ocr_avail &= host->ocr_mask;
 
 	mmc->ocr_avail = ocr_avail;
 	mmc->ocr_avail_sdio = ocr_avail;
@@ -3273,6 +3251,7 @@ EXPORT_SYMBOL_GPL(sdhci_add_host);
 
 void sdhci_remove_host(struct sdhci_host *host, int dead)
 {
+	struct mmc_host *mmc = host->mmc;
 	unsigned long flags;
 
 	if (dead) {
@@ -3310,15 +3289,11 @@ void sdhci_remove_host(struct sdhci_host *host, int dead)
 
 	tasklet_kill(&host->finish_tasklet);
 
-	if (host->vmmc) {
-		regulator_disable(host->vmmc);
-		regulator_put(host->vmmc);
-	}
+	if (!IS_ERR(mmc->supply.vmmc))
+		regulator_disable(mmc->supply.vmmc);
 
-	if (host->vqmmc) {
-		regulator_disable(host->vqmmc);
-		regulator_put(host->vqmmc);
-	}
+	if (!IS_ERR(mmc->supply.vqmmc))
+		regulator_disable(mmc->supply.vqmmc);
 
 	if (host->adma_desc)
 		dma_free_coherent(mmc_dev(host->mmc), ADMA_SIZE,
diff --git a/include/linux/mmc/sdhci.h b/include/linux/mmc/sdhci.h
index 08abe99..09ebe57 100644
--- a/include/linux/mmc/sdhci.h
+++ b/include/linux/mmc/sdhci.h
@@ -104,9 +104,6 @@ struct sdhci_host {
 
 	const struct sdhci_ops *ops;	/* Low level hw interface */
 
-	struct regulator *vmmc;		/* Power regulator (vmmc) */
-	struct regulator *vqmmc;	/* Signaling regulator (vccq) */
-
 	/* Internal data */
 	struct mmc_host *mmc;	/* MMC structure */
 	u64 dma_mask;		/* custom DMA mask */
-- 
1.9.1

Re: [PATCH v4 1/2] mmc: sdhci: Use mmc core regulator infrastucture

[Ulf Hansson]

On 13 June 2014 19:13, Markus Mayer <markus.mayer@linaro.org> wrote:
> From: Tim Kryger <tim.kryger@gmail.com>
>
> Switch the common SDHCI code over to use mmc_host's regulator pointers
> and remove the ones in the sdhci_host structure.  Additionally, use the
> common mmc_regulator_get_supply function to get the regulators and set
> the ocr_avail mask.
>
> This change sets the ocr_avail directly based upon the voltage ranges
> supported which ensures ocr_avail is set correctly while allowing the
> use of regulators that can't provide exactly 1.8v, 3.0v, or 3.3v.
>
> Signed-off-by: Tim Kryger <tim.kryger@gmail.com>
> Signed-off-by: Markus Mayer <markus.mayer@linaro.org>
> Reviewed-by: Matt Porter <mporter@linaro.org>

Thanks!

Applied for next.

Kind regards
Uffe

> ---
>  drivers/mmc/host/sdhci.c  | 97 ++++++++++++++++++-----------------------------
>  include/linux/mmc/sdhci.h |  3 --
>  2 files changed, 36 insertions(+), 64 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 47055f3..ee524b0 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -1223,6 +1223,7 @@ EXPORT_SYMBOL_GPL(sdhci_set_clock);
>  static void sdhci_set_power(struct sdhci_host *host, unsigned char mode,
>                             unsigned short vdd)
>  {
> +       struct mmc_host *mmc = host->mmc;
>         u8 pwr = 0;
>
>         if (mode != MMC_POWER_OFF) {
> @@ -1284,9 +1285,9 @@ static void sdhci_set_power(struct sdhci_host *host, unsigned char mode,
>                         mdelay(10);
>         }
>
> -       if (host->vmmc) {
> +       if (!IS_ERR(mmc->supply.vmmc)) {
>                 spin_unlock_irq(&host->lock);
> -               mmc_regulator_set_ocr(host->mmc, host->vmmc, vdd);
> +               mmc_regulator_set_ocr(host->mmc, mmc->supply.vmmc, vdd);
>                 spin_lock_irq(&host->lock);
>         }
>  }
> @@ -1440,13 +1441,15 @@ static void sdhci_do_set_ios(struct sdhci_host *host, struct mmc_ios *ios)
>  {
>         unsigned long flags;
>         u8 ctrl;
> +       struct mmc_host *mmc = host->mmc;
>
>         spin_lock_irqsave(&host->lock, flags);
>
>         if (host->flags & SDHCI_DEVICE_DEAD) {
>                 spin_unlock_irqrestore(&host->lock, flags);
> -               if (host->vmmc && ios->power_mode == MMC_POWER_OFF)
> -                       mmc_regulator_set_ocr(host->mmc, host->vmmc, 0);
> +               if (!IS_ERR(mmc->supply.vmmc) &&
> +                   ios->power_mode == MMC_POWER_OFF)
> +                       mmc_regulator_set_ocr(host->mmc, mmc->supply.vmmc, 0);
>                 return;
>         }
>
> @@ -1707,6 +1710,7 @@ static void sdhci_enable_sdio_irq(struct mmc_host *mmc, int enable)
>  static int sdhci_do_start_signal_voltage_switch(struct sdhci_host *host,
>                                                 struct mmc_ios *ios)
>  {
> +       struct mmc_host *mmc = host->mmc;
>         u16 ctrl;
>         int ret;
>
> @@ -1725,8 +1729,9 @@ static int sdhci_do_start_signal_voltage_switch(struct sdhci_host *host,
>                 ctrl &= ~SDHCI_CTRL_VDD_180;
>                 sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2);
>
> -               if (host->vqmmc) {
> -                       ret = regulator_set_voltage(host->vqmmc, 2700000, 3600000);
> +               if (!IS_ERR(mmc->supply.vqmmc)) {
> +                       ret = regulator_set_voltage(mmc->supply.vqmmc, 2700000,
> +                                                   3600000);
>                         if (ret) {
>                                 pr_warning("%s: Switching to 3.3V signalling voltage "
>                                                 " failed\n", mmc_hostname(host->mmc));
> @@ -1746,8 +1751,8 @@ static int sdhci_do_start_signal_voltage_switch(struct sdhci_host *host,
>
>                 return -EAGAIN;
>         case MMC_SIGNAL_VOLTAGE_180:
> -               if (host->vqmmc) {
> -                       ret = regulator_set_voltage(host->vqmmc,
> +               if (!IS_ERR(mmc->supply.vqmmc)) {
> +                       ret = regulator_set_voltage(mmc->supply.vqmmc,
>                                         1700000, 1950000);
>                         if (ret) {
>                                 pr_warning("%s: Switching to 1.8V signalling voltage "
> @@ -1776,8 +1781,9 @@ static int sdhci_do_start_signal_voltage_switch(struct sdhci_host *host,
>
>                 return -EAGAIN;
>         case MMC_SIGNAL_VOLTAGE_120:
> -               if (host->vqmmc) {
> -                       ret = regulator_set_voltage(host->vqmmc, 1100000, 1300000);
> +               if (!IS_ERR(mmc->supply.vqmmc)) {
> +                       ret = regulator_set_voltage(mmc->supply.vqmmc, 1100000,
> +                                                   1300000);
>                         if (ret) {
>                                 pr_warning("%s: Switching to 1.2V signalling voltage "
>                                                 " failed\n", mmc_hostname(host->mmc));
> @@ -2962,25 +2968,22 @@ int sdhci_add_host(struct sdhci_host *host)
>             !(host->mmc->caps & MMC_CAP_NONREMOVABLE))
>                 mmc->caps |= MMC_CAP_NEEDS_POLL;
>
> +       /* If there are external regulators, get them */
> +       if (mmc_regulator_get_supply(mmc) == -EPROBE_DEFER)
> +               return -EPROBE_DEFER;
> +
>         /* If vqmmc regulator and no 1.8V signalling, then there's no UHS */
> -       host->vqmmc = regulator_get_optional(mmc_dev(mmc), "vqmmc");
> -       if (IS_ERR_OR_NULL(host->vqmmc)) {
> -               if (PTR_ERR(host->vqmmc) < 0) {
> -                       pr_info("%s: no vqmmc regulator found\n",
> -                               mmc_hostname(mmc));
> -                       host->vqmmc = NULL;
> -               }
> -       } else {
> -               ret = regulator_enable(host->vqmmc);
> -               if (!regulator_is_supported_voltage(host->vqmmc, 1700000,
> -                       1950000))
> +       if (!IS_ERR(mmc->supply.vqmmc)) {
> +               ret = regulator_enable(mmc->supply.vqmmc);
> +               if (!regulator_is_supported_voltage(mmc->supply.vqmmc, 1700000,
> +                                                   1950000))
>                         caps[1] &= ~(SDHCI_SUPPORT_SDR104 |
>                                         SDHCI_SUPPORT_SDR50 |
>                                         SDHCI_SUPPORT_DDR50);
>                 if (ret) {
>                         pr_warn("%s: Failed to enable vqmmc regulator: %d\n",
>                                 mmc_hostname(mmc), ret);
> -                       host->vqmmc = NULL;
> +                       mmc->supply.vqmmc = NULL;
>                 }
>         }
>
> @@ -3041,34 +3044,6 @@ int sdhci_add_host(struct sdhci_host *host)
>
>         ocr_avail = 0;
>
> -       host->vmmc = regulator_get_optional(mmc_dev(mmc), "vmmc");
> -       if (IS_ERR_OR_NULL(host->vmmc)) {
> -               if (PTR_ERR(host->vmmc) < 0) {
> -                       pr_info("%s: no vmmc regulator found\n",
> -                               mmc_hostname(mmc));
> -                       host->vmmc = NULL;
> -               }
> -       }
> -
> -#ifdef CONFIG_REGULATOR
> -       /*
> -        * Voltage range check makes sense only if regulator reports
> -        * any voltage value.
> -        */
> -       if (host->vmmc && regulator_get_voltage(host->vmmc) > 0) {
> -               ret = regulator_is_supported_voltage(host->vmmc, 2700000,
> -                       3600000);
> -               if ((ret <= 0) || (!(caps[0] & SDHCI_CAN_VDD_330)))
> -                       caps[0] &= ~SDHCI_CAN_VDD_330;
> -               if ((ret <= 0) || (!(caps[0] & SDHCI_CAN_VDD_300)))
> -                       caps[0] &= ~SDHCI_CAN_VDD_300;
> -               ret = regulator_is_supported_voltage(host->vmmc, 1700000,
> -                       1950000);
> -               if ((ret <= 0) || (!(caps[0] & SDHCI_CAN_VDD_180)))
> -                       caps[0] &= ~SDHCI_CAN_VDD_180;
> -       }
> -#endif /* CONFIG_REGULATOR */
> -
>         /*
>          * According to SD Host Controller spec v3.00, if the Host System
>          * can afford more than 150mA, Host Driver should set XPC to 1. Also
> @@ -3077,8 +3052,8 @@ int sdhci_add_host(struct sdhci_host *host)
>          * value.
>          */
>         max_current_caps = sdhci_readl(host, SDHCI_MAX_CURRENT);
> -       if (!max_current_caps && host->vmmc) {
> -               u32 curr = regulator_get_current_limit(host->vmmc);
> +       if (!max_current_caps && !IS_ERR(mmc->supply.vmmc)) {
> +               u32 curr = regulator_get_current_limit(mmc->supply.vmmc);
>                 if (curr > 0) {
>
>                         /* convert to SDHCI_MAX_CURRENT format */
> @@ -3118,8 +3093,11 @@ int sdhci_add_host(struct sdhci_host *host)
>                                    SDHCI_MAX_CURRENT_MULTIPLIER;
>         }
>
> +       if (mmc->ocr_avail)
> +               ocr_avail &= mmc->ocr_avail;
> +
>         if (host->ocr_mask)
> -               ocr_avail = host->ocr_mask;
> +               ocr_avail &= host->ocr_mask;
>
>         mmc->ocr_avail = ocr_avail;
>         mmc->ocr_avail_sdio = ocr_avail;
> @@ -3273,6 +3251,7 @@ EXPORT_SYMBOL_GPL(sdhci_add_host);
>
>  void sdhci_remove_host(struct sdhci_host *host, int dead)
>  {
> +       struct mmc_host *mmc = host->mmc;
>         unsigned long flags;
>
>         if (dead) {
> @@ -3310,15 +3289,11 @@ void sdhci_remove_host(struct sdhci_host *host, int dead)
>
>         tasklet_kill(&host->finish_tasklet);
>
> -       if (host->vmmc) {
> -               regulator_disable(host->vmmc);
> -               regulator_put(host->vmmc);
> -       }
> +       if (!IS_ERR(mmc->supply.vmmc))
> +               regulator_disable(mmc->supply.vmmc);
>
> -       if (host->vqmmc) {
> -               regulator_disable(host->vqmmc);
> -               regulator_put(host->vqmmc);
> -       }
> +       if (!IS_ERR(mmc->supply.vqmmc))
> +               regulator_disable(mmc->supply.vqmmc);
>
>         if (host->adma_desc)
>                 dma_free_coherent(mmc_dev(host->mmc), ADMA_SIZE,
> diff --git a/include/linux/mmc/sdhci.h b/include/linux/mmc/sdhci.h
> index 08abe99..09ebe57 100644
> --- a/include/linux/mmc/sdhci.h
> +++ b/include/linux/mmc/sdhci.h
> @@ -104,9 +104,6 @@ struct sdhci_host {
>
>         const struct sdhci_ops *ops;    /* Low level hw interface */
>
> -       struct regulator *vmmc;         /* Power regulator (vmmc) */
> -       struct regulator *vqmmc;        /* Signaling regulator (vccq) */
> -
>         /* Internal data */
>         struct mmc_host *mmc;   /* MMC structure */
>         u64 dma_mask;           /* custom DMA mask */
> --
> 1.9.1
>

[PATCH v4 2/2] mmc: sdhci: Replace host->mmc with mmc where possible

[Markus Mayer]

After the switch to the MMC core regulator infrastucture, we already
have a local "mmc" pointer in various functions. There is no longer a
need to access the data structure via host->mmc.

Signed-off-by: Markus Mayer <markus.mayer@linaro.org>
Reviewed-by: Matt Porter <mporter@linaro.org>
---
 drivers/mmc/host/sdhci.c | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index ee524b0..54b5304 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1287,7 +1287,7 @@ static void sdhci_set_power(struct sdhci_host *host, unsigned char mode,
 
 	if (!IS_ERR(mmc->supply.vmmc)) {
 		spin_unlock_irq(&host->lock);
-		mmc_regulator_set_ocr(host->mmc, mmc->supply.vmmc, vdd);
+		mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd);
 		spin_lock_irq(&host->lock);
 	}
 }
@@ -1449,7 +1449,7 @@ static void sdhci_do_set_ios(struct sdhci_host *host, struct mmc_ios *ios)
 		spin_unlock_irqrestore(&host->lock, flags);
 		if (!IS_ERR(mmc->supply.vmmc) &&
 		    ios->power_mode == MMC_POWER_OFF)
-			mmc_regulator_set_ocr(host->mmc, mmc->supply.vmmc, 0);
+			mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, 0);
 		return;
 	}
 
@@ -1734,7 +1734,7 @@ static int sdhci_do_start_signal_voltage_switch(struct sdhci_host *host,
 						    3600000);
 			if (ret) {
 				pr_warning("%s: Switching to 3.3V signalling voltage "
-						" failed\n", mmc_hostname(host->mmc));
+						" failed\n", mmc_hostname(mmc));
 				return -EIO;
 			}
 		}
@@ -1747,7 +1747,7 @@ static int sdhci_do_start_signal_voltage_switch(struct sdhci_host *host,
 			return 0;
 
 		pr_warning("%s: 3.3V regulator output did not became stable\n",
-				mmc_hostname(host->mmc));
+				mmc_hostname(mmc));
 
 		return -EAGAIN;
 	case MMC_SIGNAL_VOLTAGE_180:
@@ -1756,7 +1756,7 @@ static int sdhci_do_start_signal_voltage_switch(struct sdhci_host *host,
 					1700000, 1950000);
 			if (ret) {
 				pr_warning("%s: Switching to 1.8V signalling voltage "
-						" failed\n", mmc_hostname(host->mmc));
+						" failed\n", mmc_hostname(mmc));
 				return -EIO;
 			}
 		}
@@ -1777,7 +1777,7 @@ static int sdhci_do_start_signal_voltage_switch(struct sdhci_host *host,
 			return 0;
 
 		pr_warning("%s: 1.8V regulator output did not became stable\n",
-				mmc_hostname(host->mmc));
+				mmc_hostname(mmc));
 
 		return -EAGAIN;
 	case MMC_SIGNAL_VOLTAGE_120:
@@ -1786,7 +1786,7 @@ static int sdhci_do_start_signal_voltage_switch(struct sdhci_host *host,
 						    1300000);
 			if (ret) {
 				pr_warning("%s: Switching to 1.2V signalling voltage "
-						" failed\n", mmc_hostname(host->mmc));
+						" failed\n", mmc_hostname(mmc));
 				return -EIO;
 			}
 		}
@@ -2826,12 +2826,12 @@ int sdhci_add_host(struct sdhci_host *host)
 		 * (128) and potentially one alignment transfer for
 		 * each of those entries.
 		 */
-		host->adma_desc = dma_alloc_coherent(mmc_dev(host->mmc),
+		host->adma_desc = dma_alloc_coherent(mmc_dev(mmc),
 						     ADMA_SIZE, &host->adma_addr,
 						     GFP_KERNEL);
 		host->align_buffer = kmalloc(128 * 4, GFP_KERNEL);
 		if (!host->adma_desc || !host->align_buffer) {
-			dma_free_coherent(mmc_dev(host->mmc), ADMA_SIZE,
+			dma_free_coherent(mmc_dev(mmc), ADMA_SIZE,
 					  host->adma_desc, host->adma_addr);
 			kfree(host->align_buffer);
 			pr_warning("%s: Unable to allocate ADMA "
@@ -2844,7 +2844,7 @@ int sdhci_add_host(struct sdhci_host *host)
 			pr_warning("%s: unable to allocate aligned ADMA descriptor\n",
 				   mmc_hostname(mmc));
 			host->flags &= ~SDHCI_USE_ADMA;
-			dma_free_coherent(mmc_dev(host->mmc), ADMA_SIZE,
+			dma_free_coherent(mmc_dev(mmc), ADMA_SIZE,
 					  host->adma_desc, host->adma_addr);
 			kfree(host->align_buffer);
 			host->adma_desc = NULL;
@@ -2859,7 +2859,7 @@ int sdhci_add_host(struct sdhci_host *host)
 	 */
 	if (!(host->flags & (SDHCI_USE_SDMA | SDHCI_USE_ADMA))) {
 		host->dma_mask = DMA_BIT_MASK(64);
-		mmc_dev(host->mmc)->dma_mask = &host->dma_mask;
+		mmc_dev(mmc)->dma_mask = &host->dma_mask;
 	}
 
 	if (host->version >= SDHCI_SPEC_300)
@@ -2965,7 +2965,7 @@ int sdhci_add_host(struct sdhci_host *host)
 		mmc->caps |= MMC_CAP_SD_HIGHSPEED | MMC_CAP_MMC_HIGHSPEED;
 
 	if ((host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION) &&
-	    !(host->mmc->caps & MMC_CAP_NONREMOVABLE))
+	    !(mmc->caps & MMC_CAP_NONREMOVABLE))
 		mmc->caps |= MMC_CAP_NEEDS_POLL;
 
 	/* If there are external regulators, get them */
@@ -3261,7 +3261,7 @@ void sdhci_remove_host(struct sdhci_host *host, int dead)
 
 		if (host->mrq) {
 			pr_err("%s: Controller removed during "
-				" transfer!\n", mmc_hostname(host->mmc));
+				" transfer!\n", mmc_hostname(mmc));
 
 			host->mrq->cmd->error = -ENOMEDIUM;
 			tasklet_schedule(&host->finish_tasklet);
@@ -3272,7 +3272,7 @@ void sdhci_remove_host(struct sdhci_host *host, int dead)
 
 	sdhci_disable_card_detection(host);
 
-	mmc_remove_host(host->mmc);
+	mmc_remove_host(mmc);
 
 #ifdef SDHCI_USE_LEDS_CLASS
 	led_classdev_unregister(&host->led);
@@ -3296,7 +3296,7 @@ void sdhci_remove_host(struct sdhci_host *host, int dead)
 		regulator_disable(mmc->supply.vqmmc);
 
 	if (host->adma_desc)
-		dma_free_coherent(mmc_dev(host->mmc), ADMA_SIZE,
+		dma_free_coherent(mmc_dev(mmc), ADMA_SIZE,
 				  host->adma_desc, host->adma_addr);
 	kfree(host->align_buffer);
 
-- 
1.9.1

Re: [PATCH v4 2/2] mmc: sdhci: Replace host->mmc with mmc where possible

[Ulf Hansson]

On 13 June 2014 19:13, Markus Mayer <markus.mayer@linaro.org> wrote:
> After the switch to the MMC core regulator infrastucture, we already
> have a local "mmc" pointer in various functions. There is no longer a
> need to access the data structure via host->mmc.
>
> Signed-off-by: Markus Mayer <markus.mayer@linaro.org>
> Reviewed-by: Matt Porter <mporter@linaro.org>

Hi Markus,

Could you run checkpatch? There were some warnings to handle.

Kind regards
Ulf Hansson

> ---
>  drivers/mmc/host/sdhci.c | 30 +++++++++++++++---------------
>  1 file changed, 15 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index ee524b0..54b5304 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -1287,7 +1287,7 @@ static void sdhci_set_power(struct sdhci_host *host, unsigned char mode,
>
>         if (!IS_ERR(mmc->supply.vmmc)) {
>                 spin_unlock_irq(&host->lock);
> -               mmc_regulator_set_ocr(host->mmc, mmc->supply.vmmc, vdd);
> +               mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd);
>                 spin_lock_irq(&host->lock);
>         }
>  }
> @@ -1449,7 +1449,7 @@ static void sdhci_do_set_ios(struct sdhci_host *host, struct mmc_ios *ios)
>                 spin_unlock_irqrestore(&host->lock, flags);
>                 if (!IS_ERR(mmc->supply.vmmc) &&
>                     ios->power_mode == MMC_POWER_OFF)
> -                       mmc_regulator_set_ocr(host->mmc, mmc->supply.vmmc, 0);
> +                       mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, 0);
>                 return;
>         }
>
> @@ -1734,7 +1734,7 @@ static int sdhci_do_start_signal_voltage_switch(struct sdhci_host *host,
>                                                     3600000);
>                         if (ret) {
>                                 pr_warning("%s: Switching to 3.3V signalling voltage "
> -                                               " failed\n", mmc_hostname(host->mmc));
> +                                               " failed\n", mmc_hostname(mmc));
>                                 return -EIO;
>                         }
>                 }
> @@ -1747,7 +1747,7 @@ static int sdhci_do_start_signal_voltage_switch(struct sdhci_host *host,
>                         return 0;
>
>                 pr_warning("%s: 3.3V regulator output did not became stable\n",
> -                               mmc_hostname(host->mmc));
> +                               mmc_hostname(mmc));
>
>                 return -EAGAIN;
>         case MMC_SIGNAL_VOLTAGE_180:
> @@ -1756,7 +1756,7 @@ static int sdhci_do_start_signal_voltage_switch(struct sdhci_host *host,
>                                         1700000, 1950000);
>                         if (ret) {
>                                 pr_warning("%s: Switching to 1.8V signalling voltage "
> -                                               " failed\n", mmc_hostname(host->mmc));
> +                                               " failed\n", mmc_hostname(mmc));
>                                 return -EIO;
>                         }
>                 }
> @@ -1777,7 +1777,7 @@ static int sdhci_do_start_signal_voltage_switch(struct sdhci_host *host,
>                         return 0;
>
>                 pr_warning("%s: 1.8V regulator output did not became stable\n",
> -                               mmc_hostname(host->mmc));
> +                               mmc_hostname(mmc));
>
>                 return -EAGAIN;
>         case MMC_SIGNAL_VOLTAGE_120:
> @@ -1786,7 +1786,7 @@ static int sdhci_do_start_signal_voltage_switch(struct sdhci_host *host,
>                                                     1300000);
>                         if (ret) {
>                                 pr_warning("%s: Switching to 1.2V signalling voltage "
> -                                               " failed\n", mmc_hostname(host->mmc));
> +                                               " failed\n", mmc_hostname(mmc));
>                                 return -EIO;
>                         }
>                 }
> @@ -2826,12 +2826,12 @@ int sdhci_add_host(struct sdhci_host *host)
>                  * (128) and potentially one alignment transfer for
>                  * each of those entries.
>                  */
> -               host->adma_desc = dma_alloc_coherent(mmc_dev(host->mmc),
> +               host->adma_desc = dma_alloc_coherent(mmc_dev(mmc),
>                                                      ADMA_SIZE, &host->adma_addr,
>                                                      GFP_KERNEL);
>                 host->align_buffer = kmalloc(128 * 4, GFP_KERNEL);
>                 if (!host->adma_desc || !host->align_buffer) {
> -                       dma_free_coherent(mmc_dev(host->mmc), ADMA_SIZE,
> +                       dma_free_coherent(mmc_dev(mmc), ADMA_SIZE,
>                                           host->adma_desc, host->adma_addr);
>                         kfree(host->align_buffer);
>                         pr_warning("%s: Unable to allocate ADMA "
> @@ -2844,7 +2844,7 @@ int sdhci_add_host(struct sdhci_host *host)
>                         pr_warning("%s: unable to allocate aligned ADMA descriptor\n",
>                                    mmc_hostname(mmc));
>                         host->flags &= ~SDHCI_USE_ADMA;
> -                       dma_free_coherent(mmc_dev(host->mmc), ADMA_SIZE,
> +                       dma_free_coherent(mmc_dev(mmc), ADMA_SIZE,
>                                           host->adma_desc, host->adma_addr);
>                         kfree(host->align_buffer);
>                         host->adma_desc = NULL;
> @@ -2859,7 +2859,7 @@ int sdhci_add_host(struct sdhci_host *host)
>          */
>         if (!(host->flags & (SDHCI_USE_SDMA | SDHCI_USE_ADMA))) {
>                 host->dma_mask = DMA_BIT_MASK(64);
> -               mmc_dev(host->mmc)->dma_mask = &host->dma_mask;
> +               mmc_dev(mmc)->dma_mask = &host->dma_mask;
>         }
>
>         if (host->version >= SDHCI_SPEC_300)
> @@ -2965,7 +2965,7 @@ int sdhci_add_host(struct sdhci_host *host)
>                 mmc->caps |= MMC_CAP_SD_HIGHSPEED | MMC_CAP_MMC_HIGHSPEED;
>
>         if ((host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION) &&
> -           !(host->mmc->caps & MMC_CAP_NONREMOVABLE))
> +           !(mmc->caps & MMC_CAP_NONREMOVABLE))
>                 mmc->caps |= MMC_CAP_NEEDS_POLL;
>
>         /* If there are external regulators, get them */
> @@ -3261,7 +3261,7 @@ void sdhci_remove_host(struct sdhci_host *host, int dead)
>
>                 if (host->mrq) {
>                         pr_err("%s: Controller removed during "
> -                               " transfer!\n", mmc_hostname(host->mmc));
> +                               " transfer!\n", mmc_hostname(mmc));
>
>                         host->mrq->cmd->error = -ENOMEDIUM;
>                         tasklet_schedule(&host->finish_tasklet);
> @@ -3272,7 +3272,7 @@ void sdhci_remove_host(struct sdhci_host *host, int dead)
>
>         sdhci_disable_card_detection(host);
>
> -       mmc_remove_host(host->mmc);
> +       mmc_remove_host(mmc);
>
>  #ifdef SDHCI_USE_LEDS_CLASS
>         led_classdev_unregister(&host->led);
> @@ -3296,7 +3296,7 @@ void sdhci_remove_host(struct sdhci_host *host, int dead)
>                 regulator_disable(mmc->supply.vqmmc);
>
>         if (host->adma_desc)
> -               dma_free_coherent(mmc_dev(host->mmc), ADMA_SIZE,
> +               dma_free_coherent(mmc_dev(mmc), ADMA_SIZE,
>                                   host->adma_desc, host->adma_addr);
>         kfree(host->align_buffer);
>
> --
> 1.9.1
>

Re: [PATCH v4 2/2] mmc: sdhci: Replace host->mmc with mmc where possible

[Markus Mayer]

On 16 June 2014 01:23, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> On 13 June 2014 19:13, Markus Mayer <markus.mayer@linaro.org> wrote:
>> After the switch to the MMC core regulator infrastucture, we already
>> have a local "mmc" pointer in various functions. There is no longer a
>> need to access the data structure via host->mmc.
>>
>> Signed-off-by: Markus Mayer <markus.mayer@linaro.org>
>> Reviewed-by: Matt Porter <mporter@linaro.org>
>
> Could you run checkpatch? There were some warnings to handle.

You're right. There are several warnings regarding wrapped strings.
However, those warnings are unrelated to my changes. The wrapped
strings exist in current code.

See for instance:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/mmc/host/sdhci.c?id=refs/tags/v3.16-rc1#n1730

I don't think I should intermix my "host->mmc to mmc" replacement
patch with changes to fix the wrapping of strings.

If you would like these string related checkpatch warnings fixed, I
can look into creating a follow-on patch to fix those up. There are
quite a few more instances in sdhci.c that wouldn't get addressed if I
simply fixed up my existing patch. And that would be the other
argument against intermixing 2 different fixes: I wouldn't be taking
care of all string wrapping issues if I just fixed the four instances
it complains about in my patch.

Some of those wrapped strings are actually quite long. Here's an example:

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/mmc/host/sdhci.c?id=refs/tags/v3.16-rc1#n1938

So, fixing those long messages may take a bit more thought than one
might initially expect.

Please let me know how you would like to proceed.

Thanks,
-Markus