All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] mmc: sdhci-of-at91: Support external regulators
@ 2017-03-09 15:18 ` Romain Izard
  0 siblings, 0 replies; 11+ messages in thread
From: Romain Izard @ 2017-03-09 15:18 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel, linux-mmc, Ludovic Desroches,
	Adrian Hunter, Ulf Hansson
  Cc: Romain Izard

The SDHCI controller in the SAMA5D2 chip requires a valid voltage set
in the power control register, otherwise commands will fail with a
timeout error.

When using the regulator framework to specify the regulator used by the
mmc device, the voltage is not configured, and it is not possible to use
the connected device.

Implement a custom 'set_power' function for this specific hardware, that
configures the voltage in the register in all cases.

Signed-off-by: Romain Izard <romain.izard.pro@gmail.com>
---
 drivers/mmc/host/sdhci-of-at91.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/mmc/host/sdhci-of-at91.c b/drivers/mmc/host/sdhci-of-at91.c
index 2f9ad213377a..291a74955a4c 100644
--- a/drivers/mmc/host/sdhci-of-at91.c
+++ b/drivers/mmc/host/sdhci-of-at91.c
@@ -85,11 +85,30 @@ static void sdhci_at91_set_clock(struct sdhci_host *host, unsigned int clock)
 	sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
 }
 
+/*
+ * In this specific implementation of the SDHCI contoller, the power register
+ * needs to have a valid voltage set even when the power supply is managed by
+ * an external regulator.
+ */
+static void sdhci_at91_set_power(struct sdhci_host *host, unsigned char mode,
+		     unsigned short vdd)
+{
+	if (!IS_ERR(host->mmc->supply.vmmc)) {
+		struct mmc_host *mmc = host->mmc;
+
+		spin_unlock_irq(&host->lock);
+		mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd);
+		spin_lock_irq(&host->lock);
+	}
+	sdhci_set_power_noreg(host, mode, vdd);
+}
+
 static const struct sdhci_ops sdhci_at91_sama5d2_ops = {
 	.set_clock		= sdhci_at91_set_clock,
 	.set_bus_width		= sdhci_set_bus_width,
 	.reset			= sdhci_reset,
 	.set_uhs_signaling	= sdhci_set_uhs_signaling,
+	.set_power		= sdhci_at91_set_power,
 };
 
 static const struct sdhci_pltfm_data soc_data_sama5d2 = {
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH] mmc: sdhci-of-at91: Support external regulators
@ 2017-03-09 15:18 ` Romain Izard
  0 siblings, 0 replies; 11+ messages in thread
From: Romain Izard @ 2017-03-09 15:18 UTC (permalink / raw)
  To: linux-arm-kernel

The SDHCI controller in the SAMA5D2 chip requires a valid voltage set
in the power control register, otherwise commands will fail with a
timeout error.

When using the regulator framework to specify the regulator used by the
mmc device, the voltage is not configured, and it is not possible to use
the connected device.

Implement a custom 'set_power' function for this specific hardware, that
configures the voltage in the register in all cases.

Signed-off-by: Romain Izard <romain.izard.pro@gmail.com>
---
 drivers/mmc/host/sdhci-of-at91.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/mmc/host/sdhci-of-at91.c b/drivers/mmc/host/sdhci-of-at91.c
index 2f9ad213377a..291a74955a4c 100644
--- a/drivers/mmc/host/sdhci-of-at91.c
+++ b/drivers/mmc/host/sdhci-of-at91.c
@@ -85,11 +85,30 @@ static void sdhci_at91_set_clock(struct sdhci_host *host, unsigned int clock)
 	sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
 }
 
+/*
+ * In this specific implementation of the SDHCI contoller, the power register
+ * needs to have a valid voltage set even when the power supply is managed by
+ * an external regulator.
+ */
+static void sdhci_at91_set_power(struct sdhci_host *host, unsigned char mode,
+		     unsigned short vdd)
+{
+	if (!IS_ERR(host->mmc->supply.vmmc)) {
+		struct mmc_host *mmc = host->mmc;
+
+		spin_unlock_irq(&host->lock);
+		mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd);
+		spin_lock_irq(&host->lock);
+	}
+	sdhci_set_power_noreg(host, mode, vdd);
+}
+
 static const struct sdhci_ops sdhci_at91_sama5d2_ops = {
 	.set_clock		= sdhci_at91_set_clock,
 	.set_bus_width		= sdhci_set_bus_width,
 	.reset			= sdhci_reset,
 	.set_uhs_signaling	= sdhci_set_uhs_signaling,
+	.set_power		= sdhci_at91_set_power,
 };
 
 static const struct sdhci_pltfm_data soc_data_sama5d2 = {
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] mmc: sdhci-of-at91: Support external regulators
  2017-03-09 15:18 ` Romain Izard
  (?)
@ 2017-03-10 13:29   ` Ludovic Desroches
  -1 siblings, 0 replies; 11+ messages in thread
From: Ludovic Desroches @ 2017-03-10 13:29 UTC (permalink / raw)
  To: Romain Izard, linux-kernel, linux-arm-kernel, linux-mmc,
	Adrian Hunter, Ulf Hansson

Hi Romain,

On 03/09/2017 04:18 PM, Romain Izard wrote:
> The SDHCI controller in the SAMA5D2 chip requires a valid voltage set
> in the power control register, otherwise commands will fail with a
> timeout error.
>
> When using the regulator framework to specify the regulator used by the
> mmc device, the voltage is not configured, and it is not possible to use
> the connected device.
>

It's true, I proposed some patches long time ago to change a bit the 
behavior of the sdhci_set_power function but we didn't agree and so I 
didn't declare the regulator.

> Implement a custom 'set_power' function for this specific hardware, that
> configures the voltage in the register in all cases.
>

I don't remember if the set_power ops was available at that time. If 
yes, I would have thought about it. IMO it's a nice solution.

> Signed-off-by: Romain Izard <romain.izard.pro@gmail.com>
Acked-by: Ludovic Desroches <ludovic.desroches@microchip.com>

> ---
>  drivers/mmc/host/sdhci-of-at91.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-of-at91.c b/drivers/mmc/host/sdhci-of-at91.c
> index 2f9ad213377a..291a74955a4c 100644
> --- a/drivers/mmc/host/sdhci-of-at91.c
> +++ b/drivers/mmc/host/sdhci-of-at91.c
> @@ -85,11 +85,30 @@ static void sdhci_at91_set_clock(struct sdhci_host *host, unsigned int clock)
>  	sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
>  }
>
> +/*
> + * In this specific implementation of the SDHCI contoller, the power register
controller

Regards

Ludovic

> + * needs to have a valid voltage set even when the power supply is managed by
> + * an external regulator.
> + */
> +static void sdhci_at91_set_power(struct sdhci_host *host, unsigned char mode,
> +		     unsigned short vdd)
> +{
> +	if (!IS_ERR(host->mmc->supply.vmmc)) {
> +		struct mmc_host *mmc = host->mmc;
> +
> +		spin_unlock_irq(&host->lock);
> +		mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd);
> +		spin_lock_irq(&host->lock);
> +	}
> +	sdhci_set_power_noreg(host, mode, vdd);
> +}
> +
>  static const struct sdhci_ops sdhci_at91_sama5d2_ops = {
>  	.set_clock		= sdhci_at91_set_clock,
>  	.set_bus_width		= sdhci_set_bus_width,
>  	.reset			= sdhci_reset,
>  	.set_uhs_signaling	= sdhci_set_uhs_signaling,
> +	.set_power		= sdhci_at91_set_power,
>  };
>
>  static const struct sdhci_pltfm_data soc_data_sama5d2 = {
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] mmc: sdhci-of-at91: Support external regulators
@ 2017-03-10 13:29   ` Ludovic Desroches
  0 siblings, 0 replies; 11+ messages in thread
From: Ludovic Desroches @ 2017-03-10 13:29 UTC (permalink / raw)
  To: Romain Izard, linux-kernel, linux-arm-kernel, linux-mmc,
	Adrian Hunter, Ulf Hansson

Hi Romain,

On 03/09/2017 04:18 PM, Romain Izard wrote:
> The SDHCI controller in the SAMA5D2 chip requires a valid voltage set
> in the power control register, otherwise commands will fail with a
> timeout error.
>
> When using the regulator framework to specify the regulator used by the
> mmc device, the voltage is not configured, and it is not possible to use
> the connected device.
>

It's true, I proposed some patches long time ago to change a bit the 
behavior of the sdhci_set_power function but we didn't agree and so I 
didn't declare the regulator.

> Implement a custom 'set_power' function for this specific hardware, that
> configures the voltage in the register in all cases.
>

I don't remember if the set_power ops was available at that time. If 
yes, I would have thought about it. IMO it's a nice solution.

> Signed-off-by: Romain Izard <romain.izard.pro@gmail.com>
Acked-by: Ludovic Desroches <ludovic.desroches@microchip.com>

> ---
>  drivers/mmc/host/sdhci-of-at91.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-of-at91.c b/drivers/mmc/host/sdhci-of-at91.c
> index 2f9ad213377a..291a74955a4c 100644
> --- a/drivers/mmc/host/sdhci-of-at91.c
> +++ b/drivers/mmc/host/sdhci-of-at91.c
> @@ -85,11 +85,30 @@ static void sdhci_at91_set_clock(struct sdhci_host *host, unsigned int clock)
>  	sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
>  }
>
> +/*
> + * In this specific implementation of the SDHCI contoller, the power register
controller

Regards

Ludovic

> + * needs to have a valid voltage set even when the power supply is managed by
> + * an external regulator.
> + */
> +static void sdhci_at91_set_power(struct sdhci_host *host, unsigned char mode,
> +		     unsigned short vdd)
> +{
> +	if (!IS_ERR(host->mmc->supply.vmmc)) {
> +		struct mmc_host *mmc = host->mmc;
> +
> +		spin_unlock_irq(&host->lock);
> +		mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd);
> +		spin_lock_irq(&host->lock);
> +	}
> +	sdhci_set_power_noreg(host, mode, vdd);
> +}
> +
>  static const struct sdhci_ops sdhci_at91_sama5d2_ops = {
>  	.set_clock		= sdhci_at91_set_clock,
>  	.set_bus_width		= sdhci_set_bus_width,
>  	.reset			= sdhci_reset,
>  	.set_uhs_signaling	= sdhci_set_uhs_signaling,
> +	.set_power		= sdhci_at91_set_power,
>  };
>
>  static const struct sdhci_pltfm_data soc_data_sama5d2 = {
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH] mmc: sdhci-of-at91: Support external regulators
@ 2017-03-10 13:29   ` Ludovic Desroches
  0 siblings, 0 replies; 11+ messages in thread
From: Ludovic Desroches @ 2017-03-10 13:29 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Romain,

On 03/09/2017 04:18 PM, Romain Izard wrote:
> The SDHCI controller in the SAMA5D2 chip requires a valid voltage set
> in the power control register, otherwise commands will fail with a
> timeout error.
>
> When using the regulator framework to specify the regulator used by the
> mmc device, the voltage is not configured, and it is not possible to use
> the connected device.
>

It's true, I proposed some patches long time ago to change a bit the 
behavior of the sdhci_set_power function but we didn't agree and so I 
didn't declare the regulator.

> Implement a custom 'set_power' function for this specific hardware, that
> configures the voltage in the register in all cases.
>

I don't remember if the set_power ops was available at that time. If 
yes, I would have thought about it. IMO it's a nice solution.

> Signed-off-by: Romain Izard <romain.izard.pro@gmail.com>
Acked-by: Ludovic Desroches <ludovic.desroches@microchip.com>

> ---
>  drivers/mmc/host/sdhci-of-at91.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-of-at91.c b/drivers/mmc/host/sdhci-of-at91.c
> index 2f9ad213377a..291a74955a4c 100644
> --- a/drivers/mmc/host/sdhci-of-at91.c
> +++ b/drivers/mmc/host/sdhci-of-at91.c
> @@ -85,11 +85,30 @@ static void sdhci_at91_set_clock(struct sdhci_host *host, unsigned int clock)
>  	sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
>  }
>
> +/*
> + * In this specific implementation of the SDHCI contoller, the power register
controller

Regards

Ludovic

> + * needs to have a valid voltage set even when the power supply is managed by
> + * an external regulator.
> + */
> +static void sdhci_at91_set_power(struct sdhci_host *host, unsigned char mode,
> +		     unsigned short vdd)
> +{
> +	if (!IS_ERR(host->mmc->supply.vmmc)) {
> +		struct mmc_host *mmc = host->mmc;
> +
> +		spin_unlock_irq(&host->lock);
> +		mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd);
> +		spin_lock_irq(&host->lock);
> +	}
> +	sdhci_set_power_noreg(host, mode, vdd);
> +}
> +
>  static const struct sdhci_ops sdhci_at91_sama5d2_ops = {
>  	.set_clock		= sdhci_at91_set_clock,
>  	.set_bus_width		= sdhci_set_bus_width,
>  	.reset			= sdhci_reset,
>  	.set_uhs_signaling	= sdhci_set_uhs_signaling,
> +	.set_power		= sdhci_at91_set_power,
>  };
>
>  static const struct sdhci_pltfm_data soc_data_sama5d2 = {
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] mmc: sdhci-of-at91: Support external regulators
  2017-03-09 15:18 ` Romain Izard
  (?)
@ 2017-03-14 13:06   ` Ludovic Desroches
  -1 siblings, 0 replies; 11+ messages in thread
From: Ludovic Desroches @ 2017-03-14 13:06 UTC (permalink / raw)
  To: Romain Izard
  Cc: linux-kernel, linux-arm-kernel, linux-mmc, Ludovic Desroches,
	Adrian Hunter, Ulf Hansson

On Thu, Mar 09, 2017 at 04:18:20PM +0100, Romain Izard wrote:
> The SDHCI controller in the SAMA5D2 chip requires a valid voltage set
> in the power control register, otherwise commands will fail with a
> timeout error.
> 
> When using the regulator framework to specify the regulator used by the
> mmc device, the voltage is not configured, and it is not possible to use
> the connected device.
> 
> Implement a custom 'set_power' function for this specific hardware, that
> configures the voltage in the register in all cases.
> 
> Signed-off-by: Romain Izard <romain.izard.pro@gmail.com>

Cc: stable@vger.kernel.org #4.9+

Can be added since sdhci_set_power_noreg was introduced in 4.9.

> ---
>  drivers/mmc/host/sdhci-of-at91.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/drivers/mmc/host/sdhci-of-at91.c b/drivers/mmc/host/sdhci-of-at91.c
> index 2f9ad213377a..291a74955a4c 100644
> --- a/drivers/mmc/host/sdhci-of-at91.c
> +++ b/drivers/mmc/host/sdhci-of-at91.c
> @@ -85,11 +85,30 @@ static void sdhci_at91_set_clock(struct sdhci_host *host, unsigned int clock)
>  	sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
>  }
>  
> +/*
> + * In this specific implementation of the SDHCI contoller, the power register
> + * needs to have a valid voltage set even when the power supply is managed by
> + * an external regulator.
> + */
> +static void sdhci_at91_set_power(struct sdhci_host *host, unsigned char mode,
> +		     unsigned short vdd)
> +{
> +	if (!IS_ERR(host->mmc->supply.vmmc)) {
> +		struct mmc_host *mmc = host->mmc;
> +
> +		spin_unlock_irq(&host->lock);
> +		mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd);
> +		spin_lock_irq(&host->lock);
> +	}
> +	sdhci_set_power_noreg(host, mode, vdd);
> +}
> +
>  static const struct sdhci_ops sdhci_at91_sama5d2_ops = {
>  	.set_clock		= sdhci_at91_set_clock,
>  	.set_bus_width		= sdhci_set_bus_width,
>  	.reset			= sdhci_reset,
>  	.set_uhs_signaling	= sdhci_set_uhs_signaling,
> +	.set_power		= sdhci_at91_set_power,
>  };
>  
>  static const struct sdhci_pltfm_data soc_data_sama5d2 = {
> -- 
> 2.9.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] mmc: sdhci-of-at91: Support external regulators
@ 2017-03-14 13:06   ` Ludovic Desroches
  0 siblings, 0 replies; 11+ messages in thread
From: Ludovic Desroches @ 2017-03-14 13:06 UTC (permalink / raw)
  To: Romain Izard
  Cc: linux-kernel, linux-arm-kernel, linux-mmc, Ludovic Desroches,
	Adrian Hunter, Ulf Hansson

On Thu, Mar 09, 2017 at 04:18:20PM +0100, Romain Izard wrote:
> The SDHCI controller in the SAMA5D2 chip requires a valid voltage set
> in the power control register, otherwise commands will fail with a
> timeout error.
> 
> When using the regulator framework to specify the regulator used by the
> mmc device, the voltage is not configured, and it is not possible to use
> the connected device.
> 
> Implement a custom 'set_power' function for this specific hardware, that
> configures the voltage in the register in all cases.
> 
> Signed-off-by: Romain Izard <romain.izard.pro@gmail.com>

Cc: stable@vger.kernel.org #4.9+

Can be added since sdhci_set_power_noreg was introduced in 4.9.

> ---
>  drivers/mmc/host/sdhci-of-at91.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/drivers/mmc/host/sdhci-of-at91.c b/drivers/mmc/host/sdhci-of-at91.c
> index 2f9ad213377a..291a74955a4c 100644
> --- a/drivers/mmc/host/sdhci-of-at91.c
> +++ b/drivers/mmc/host/sdhci-of-at91.c
> @@ -85,11 +85,30 @@ static void sdhci_at91_set_clock(struct sdhci_host *host, unsigned int clock)
>  	sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
>  }
>  
> +/*
> + * In this specific implementation of the SDHCI contoller, the power register
> + * needs to have a valid voltage set even when the power supply is managed by
> + * an external regulator.
> + */
> +static void sdhci_at91_set_power(struct sdhci_host *host, unsigned char mode,
> +		     unsigned short vdd)
> +{
> +	if (!IS_ERR(host->mmc->supply.vmmc)) {
> +		struct mmc_host *mmc = host->mmc;
> +
> +		spin_unlock_irq(&host->lock);
> +		mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd);
> +		spin_lock_irq(&host->lock);
> +	}
> +	sdhci_set_power_noreg(host, mode, vdd);
> +}
> +
>  static const struct sdhci_ops sdhci_at91_sama5d2_ops = {
>  	.set_clock		= sdhci_at91_set_clock,
>  	.set_bus_width		= sdhci_set_bus_width,
>  	.reset			= sdhci_reset,
>  	.set_uhs_signaling	= sdhci_set_uhs_signaling,
> +	.set_power		= sdhci_at91_set_power,
>  };
>  
>  static const struct sdhci_pltfm_data soc_data_sama5d2 = {
> -- 
> 2.9.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH] mmc: sdhci-of-at91: Support external regulators
@ 2017-03-14 13:06   ` Ludovic Desroches
  0 siblings, 0 replies; 11+ messages in thread
From: Ludovic Desroches @ 2017-03-14 13:06 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Mar 09, 2017 at 04:18:20PM +0100, Romain Izard wrote:
> The SDHCI controller in the SAMA5D2 chip requires a valid voltage set
> in the power control register, otherwise commands will fail with a
> timeout error.
> 
> When using the regulator framework to specify the regulator used by the
> mmc device, the voltage is not configured, and it is not possible to use
> the connected device.
> 
> Implement a custom 'set_power' function for this specific hardware, that
> configures the voltage in the register in all cases.
> 
> Signed-off-by: Romain Izard <romain.izard.pro@gmail.com>

Cc: stable at vger.kernel.org #4.9+

Can be added since sdhci_set_power_noreg was introduced in 4.9.

> ---
>  drivers/mmc/host/sdhci-of-at91.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/drivers/mmc/host/sdhci-of-at91.c b/drivers/mmc/host/sdhci-of-at91.c
> index 2f9ad213377a..291a74955a4c 100644
> --- a/drivers/mmc/host/sdhci-of-at91.c
> +++ b/drivers/mmc/host/sdhci-of-at91.c
> @@ -85,11 +85,30 @@ static void sdhci_at91_set_clock(struct sdhci_host *host, unsigned int clock)
>  	sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
>  }
>  
> +/*
> + * In this specific implementation of the SDHCI contoller, the power register
> + * needs to have a valid voltage set even when the power supply is managed by
> + * an external regulator.
> + */
> +static void sdhci_at91_set_power(struct sdhci_host *host, unsigned char mode,
> +		     unsigned short vdd)
> +{
> +	if (!IS_ERR(host->mmc->supply.vmmc)) {
> +		struct mmc_host *mmc = host->mmc;
> +
> +		spin_unlock_irq(&host->lock);
> +		mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd);
> +		spin_lock_irq(&host->lock);
> +	}
> +	sdhci_set_power_noreg(host, mode, vdd);
> +}
> +
>  static const struct sdhci_ops sdhci_at91_sama5d2_ops = {
>  	.set_clock		= sdhci_at91_set_clock,
>  	.set_bus_width		= sdhci_set_bus_width,
>  	.reset			= sdhci_reset,
>  	.set_uhs_signaling	= sdhci_set_uhs_signaling,
> +	.set_power		= sdhci_at91_set_power,
>  };
>  
>  static const struct sdhci_pltfm_data soc_data_sama5d2 = {
> -- 
> 2.9.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] mmc: sdhci-of-at91: Support external regulators
  2017-03-14 13:06   ` Ludovic Desroches
  (?)
@ 2017-03-16 14:46     ` Ulf Hansson
  -1 siblings, 0 replies; 11+ messages in thread
From: Ulf Hansson @ 2017-03-16 14:46 UTC (permalink / raw)
  To: Romain Izard
  Cc: Ludovic Desroches, linux-mmc, linux-arm-kernel, Adrian Hunter,
	linux-kernel, Ulf Hansson, # 4.0+

+ stable

On 14 March 2017 at 14:06, Ludovic Desroches
<ludovic.desroches@microchip.com> wrote:
> On Thu, Mar 09, 2017 at 04:18:20PM +0100, Romain Izard wrote:
>> The SDHCI controller in the SAMA5D2 chip requires a valid voltage set
>> in the power control register, otherwise commands will fail with a
>> timeout error.
>>
>> When using the regulator framework to specify the regulator used by the
>> mmc device, the voltage is not configured, and it is not possible to use
>> the connected device.
>>
>> Implement a custom 'set_power' function for this specific hardware, that
>> configures the voltage in the register in all cases.
>>
>> Signed-off-by: Romain Izard <romain.izard.pro@gmail.com>

Thanks, applied for fixes and by correcting a "contoller" to
"controller" as pointed out by Ludovic.

Kind regards
Uffe

>
> Cc: stable@vger.kernel.org #4.9+
>
> Can be added since sdhci_set_power_noreg was introduced in 4.9.
>
>> ---
>>  drivers/mmc/host/sdhci-of-at91.c | 19 +++++++++++++++++++
>>  1 file changed, 19 insertions(+)
>>
>> diff --git a/drivers/mmc/host/sdhci-of-at91.c b/drivers/mmc/host/sdhci-of-at91.c
>> index 2f9ad213377a..291a74955a4c 100644
>> --- a/drivers/mmc/host/sdhci-of-at91.c
>> +++ b/drivers/mmc/host/sdhci-of-at91.c
>> @@ -85,11 +85,30 @@ static void sdhci_at91_set_clock(struct sdhci_host *host, unsigned int clock)
>>       sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
>>  }
>>
>> +/*
>> + * In this specific implementation of the SDHCI contoller, the power register
>> + * needs to have a valid voltage set even when the power supply is managed by
>> + * an external regulator.
>> + */
>> +static void sdhci_at91_set_power(struct sdhci_host *host, unsigned char mode,
>> +                  unsigned short vdd)
>> +{
>> +     if (!IS_ERR(host->mmc->supply.vmmc)) {
>> +             struct mmc_host *mmc = host->mmc;
>> +
>> +             spin_unlock_irq(&host->lock);
>> +             mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd);
>> +             spin_lock_irq(&host->lock);
>> +     }
>> +     sdhci_set_power_noreg(host, mode, vdd);
>> +}
>> +
>>  static const struct sdhci_ops sdhci_at91_sama5d2_ops = {
>>       .set_clock              = sdhci_at91_set_clock,
>>       .set_bus_width          = sdhci_set_bus_width,
>>       .reset                  = sdhci_reset,
>>       .set_uhs_signaling      = sdhci_set_uhs_signaling,
>> +     .set_power              = sdhci_at91_set_power,
>>  };
>>
>>  static const struct sdhci_pltfm_data soc_data_sama5d2 = {
>> --
>> 2.9.3
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] mmc: sdhci-of-at91: Support external regulators
@ 2017-03-16 14:46     ` Ulf Hansson
  0 siblings, 0 replies; 11+ messages in thread
From: Ulf Hansson @ 2017-03-16 14:46 UTC (permalink / raw)
  To: Romain Izard
  Cc: Ludovic Desroches, linux-mmc, linux-arm-kernel, Adrian Hunter,
	linux-kernel, Ulf Hansson, # 4.0+

+ stable

On 14 March 2017 at 14:06, Ludovic Desroches
<ludovic.desroches@microchip.com> wrote:
> On Thu, Mar 09, 2017 at 04:18:20PM +0100, Romain Izard wrote:
>> The SDHCI controller in the SAMA5D2 chip requires a valid voltage set
>> in the power control register, otherwise commands will fail with a
>> timeout error.
>>
>> When using the regulator framework to specify the regulator used by the
>> mmc device, the voltage is not configured, and it is not possible to use
>> the connected device.
>>
>> Implement a custom 'set_power' function for this specific hardware, that
>> configures the voltage in the register in all cases.
>>
>> Signed-off-by: Romain Izard <romain.izard.pro@gmail.com>

Thanks, applied for fixes and by correcting a "contoller" to
"controller" as pointed out by Ludovic.

Kind regards
Uffe

>
> Cc: stable@vger.kernel.org #4.9+
>
> Can be added since sdhci_set_power_noreg was introduced in 4.9.
>
>> ---
>>  drivers/mmc/host/sdhci-of-at91.c | 19 +++++++++++++++++++
>>  1 file changed, 19 insertions(+)
>>
>> diff --git a/drivers/mmc/host/sdhci-of-at91.c b/drivers/mmc/host/sdhci-of-at91.c
>> index 2f9ad213377a..291a74955a4c 100644
>> --- a/drivers/mmc/host/sdhci-of-at91.c
>> +++ b/drivers/mmc/host/sdhci-of-at91.c
>> @@ -85,11 +85,30 @@ static void sdhci_at91_set_clock(struct sdhci_host *host, unsigned int clock)
>>       sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
>>  }
>>
>> +/*
>> + * In this specific implementation of the SDHCI contoller, the power register
>> + * needs to have a valid voltage set even when the power supply is managed by
>> + * an external regulator.
>> + */
>> +static void sdhci_at91_set_power(struct sdhci_host *host, unsigned char mode,
>> +                  unsigned short vdd)
>> +{
>> +     if (!IS_ERR(host->mmc->supply.vmmc)) {
>> +             struct mmc_host *mmc = host->mmc;
>> +
>> +             spin_unlock_irq(&host->lock);
>> +             mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd);
>> +             spin_lock_irq(&host->lock);
>> +     }
>> +     sdhci_set_power_noreg(host, mode, vdd);
>> +}
>> +
>>  static const struct sdhci_ops sdhci_at91_sama5d2_ops = {
>>       .set_clock              = sdhci_at91_set_clock,
>>       .set_bus_width          = sdhci_set_bus_width,
>>       .reset                  = sdhci_reset,
>>       .set_uhs_signaling      = sdhci_set_uhs_signaling,
>> +     .set_power              = sdhci_at91_set_power,
>>  };
>>
>>  static const struct sdhci_pltfm_data soc_data_sama5d2 = {
>> --
>> 2.9.3
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH] mmc: sdhci-of-at91: Support external regulators
@ 2017-03-16 14:46     ` Ulf Hansson
  0 siblings, 0 replies; 11+ messages in thread
From: Ulf Hansson @ 2017-03-16 14:46 UTC (permalink / raw)
  To: linux-arm-kernel

+ stable

On 14 March 2017 at 14:06, Ludovic Desroches
<ludovic.desroches@microchip.com> wrote:
> On Thu, Mar 09, 2017 at 04:18:20PM +0100, Romain Izard wrote:
>> The SDHCI controller in the SAMA5D2 chip requires a valid voltage set
>> in the power control register, otherwise commands will fail with a
>> timeout error.
>>
>> When using the regulator framework to specify the regulator used by the
>> mmc device, the voltage is not configured, and it is not possible to use
>> the connected device.
>>
>> Implement a custom 'set_power' function for this specific hardware, that
>> configures the voltage in the register in all cases.
>>
>> Signed-off-by: Romain Izard <romain.izard.pro@gmail.com>

Thanks, applied for fixes and by correcting a "contoller" to
"controller" as pointed out by Ludovic.

Kind regards
Uffe

>
> Cc: stable at vger.kernel.org #4.9+
>
> Can be added since sdhci_set_power_noreg was introduced in 4.9.
>
>> ---
>>  drivers/mmc/host/sdhci-of-at91.c | 19 +++++++++++++++++++
>>  1 file changed, 19 insertions(+)
>>
>> diff --git a/drivers/mmc/host/sdhci-of-at91.c b/drivers/mmc/host/sdhci-of-at91.c
>> index 2f9ad213377a..291a74955a4c 100644
>> --- a/drivers/mmc/host/sdhci-of-at91.c
>> +++ b/drivers/mmc/host/sdhci-of-at91.c
>> @@ -85,11 +85,30 @@ static void sdhci_at91_set_clock(struct sdhci_host *host, unsigned int clock)
>>       sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
>>  }
>>
>> +/*
>> + * In this specific implementation of the SDHCI contoller, the power register
>> + * needs to have a valid voltage set even when the power supply is managed by
>> + * an external regulator.
>> + */
>> +static void sdhci_at91_set_power(struct sdhci_host *host, unsigned char mode,
>> +                  unsigned short vdd)
>> +{
>> +     if (!IS_ERR(host->mmc->supply.vmmc)) {
>> +             struct mmc_host *mmc = host->mmc;
>> +
>> +             spin_unlock_irq(&host->lock);
>> +             mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd);
>> +             spin_lock_irq(&host->lock);
>> +     }
>> +     sdhci_set_power_noreg(host, mode, vdd);
>> +}
>> +
>>  static const struct sdhci_ops sdhci_at91_sama5d2_ops = {
>>       .set_clock              = sdhci_at91_set_clock,
>>       .set_bus_width          = sdhci_set_bus_width,
>>       .reset                  = sdhci_reset,
>>       .set_uhs_signaling      = sdhci_set_uhs_signaling,
>> +     .set_power              = sdhci_at91_set_power,
>>  };
>>
>>  static const struct sdhci_pltfm_data soc_data_sama5d2 = {
>> --
>> 2.9.3
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>> the body of a message to majordomo at vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2017-03-16 14:48 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-09 15:18 [PATCH] mmc: sdhci-of-at91: Support external regulators Romain Izard
2017-03-09 15:18 ` Romain Izard
2017-03-10 13:29 ` Ludovic Desroches
2017-03-10 13:29   ` Ludovic Desroches
2017-03-10 13:29   ` Ludovic Desroches
2017-03-14 13:06 ` Ludovic Desroches
2017-03-14 13:06   ` Ludovic Desroches
2017-03-14 13:06   ` Ludovic Desroches
2017-03-16 14:46   ` Ulf Hansson
2017-03-16 14:46     ` Ulf Hansson
2017-03-16 14:46     ` Ulf Hansson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.