[PATCH v2] mmc: tmio: Don't access hardware registers after stopping clocks

[PATCH v2] mmc: tmio: Don't access hardware registers after stopping clocks

[Laurent Pinchart]

The tmio_mmc_set_ios() function configures the MMC power, clock and bus
width. When the mmc core requests the driver to power off the card, we
inform runtime PM, that the controller can be suspended. This can lead
to the MSTP clock being turned off.

Writing to any 16-bit hardware registers with the MSTP clock off leads
to timeouts and errors being printed to the kernel log. This can occur
both when stopping the MMC clock and when configuring the bus width.

To fix this, stop the MMC clock before calling put_runtime_pm(), and
skip bus width configuration when power is off.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/mmc/host/tmio_mmc_pio.c |   18 ++++++++++--------
 1 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index 9a7996a..53d1de6 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -810,19 +810,21 @@ static void tmio_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 		if (host->set_pwr && ios->power_mode = MMC_POWER_OFF)
 			host->set_pwr(host->pdev, 0);
 		if (host->power) {
+			tmio_mmc_clk_stop(host);
 			host->power = false;
 			pm_runtime_put(dev);
 		}
-		tmio_mmc_clk_stop(host);
 	}
 
-	switch (ios->bus_width) {
-	case MMC_BUS_WIDTH_1:
-		sd_ctrl_write16(host, CTL_SD_MEM_CARD_OPT, 0x80e0);
-	break;
-	case MMC_BUS_WIDTH_4:
-		sd_ctrl_write16(host, CTL_SD_MEM_CARD_OPT, 0x00e0);
-	break;
+	if (host->power) {
+		switch (ios->bus_width) {
+		case MMC_BUS_WIDTH_1:
+			sd_ctrl_write16(host, CTL_SD_MEM_CARD_OPT, 0x80e0);
+		break;
+		case MMC_BUS_WIDTH_4:
+			sd_ctrl_write16(host, CTL_SD_MEM_CARD_OPT, 0x00e0);
+		break;
+		}
 	}
 
 	/* Let things settle. delay taken from winCE driver */
-- 
Regards,

Laurent Pinchart

Re: [PATCH v2] mmc: tmio: Don't access hardware registers after stopping clocks

[Simon Horman]

On Tue, Jun 12, 2012 at 11:29:35PM +0200, Laurent Pinchart wrote:
> The tmio_mmc_set_ios() function configures the MMC power, clock and bus
> width. When the mmc core requests the driver to power off the card, we
> inform runtime PM, that the controller can be suspended. This can lead
> to the MSTP clock being turned off.
> 
> Writing to any 16-bit hardware registers with the MSTP clock off leads
> to timeouts and errors being printed to the kernel log. This can occur
> both when stopping the MMC clock and when configuring the bus width.
> 
> To fix this, stop the MMC clock before calling put_runtime_pm(), and
> skip bus width configuration when power is off.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Hi Laurent,

could you let me know if there is a (simple :) way for me to exercise this?

> ---
>  drivers/mmc/host/tmio_mmc_pio.c |   18 ++++++++++--------
>  1 files changed, 10 insertions(+), 8 deletions(-)

Re: [PATCH v2] mmc: tmio: Don't access hardware registers after stopping clocks

[Laurent Pinchart]

Hi Simon,

On Wednesday 13 June 2012 10:17:12 Simon Horman wrote:
> On Tue, Jun 12, 2012 at 11:29:35PM +0200, Laurent Pinchart wrote:
> > The tmio_mmc_set_ios() function configures the MMC power, clock and bus
> > width. When the mmc core requests the driver to power off the card, we
> > inform runtime PM, that the controller can be suspended. This can lead
> > to the MSTP clock being turned off.
> > 
> > Writing to any 16-bit hardware registers with the MSTP clock off leads
> > to timeouts and errors being printed to the kernel log. This can occur
> > both when stopping the MMC clock and when configuring the bus width.
> > 
> > To fix this, stop the MMC clock before calling put_runtime_pm(), and
> > skip bus width configuration when power is off.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> Hi Laurent,
> 
> could you let me know if there is a (simple :) way for me to exercise this?

Without the patch suspending the Armadillo board to RAM produces a "timeout 
waiting for SD bus idle" message. Make sure you apply the "mmc: sh_mmcif: 
Support MMC_SLEEP_AWAKE command" patch first, or suspending the board will 
fail completely.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2] mmc: tmio: Don't access hardware registers after stopping clocks

[Guennadi Liakhovetski]

On Tue, 12 Jun 2012, Laurent Pinchart wrote:

> The tmio_mmc_set_ios() function configures the MMC power, clock and bus
> width. When the mmc core requests the driver to power off the card, we
> inform runtime PM, that the controller can be suspended. This can lead
> to the MSTP clock being turned off.
> 
> Writing to any 16-bit hardware registers with the MSTP clock off leads
> to timeouts and errors being printed to the kernel log. This can occur
> both when stopping the MMC clock and when configuring the bus width.
> 
> To fix this, stop the MMC clock before calling put_runtime_pm(), and
> skip bus width configuration when power is off.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Acked-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>

Thanks
Guennadi

> ---
>  drivers/mmc/host/tmio_mmc_pio.c |   18 ++++++++++--------
>  1 files changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
> index 9a7996a..53d1de6 100644
> --- a/drivers/mmc/host/tmio_mmc_pio.c
> +++ b/drivers/mmc/host/tmio_mmc_pio.c
> @@ -810,19 +810,21 @@ static void tmio_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
>  		if (host->set_pwr && ios->power_mode = MMC_POWER_OFF)
>  			host->set_pwr(host->pdev, 0);
>  		if (host->power) {
> +			tmio_mmc_clk_stop(host);
>  			host->power = false;
>  			pm_runtime_put(dev);
>  		}
> -		tmio_mmc_clk_stop(host);
>  	}
>  
> -	switch (ios->bus_width) {
> -	case MMC_BUS_WIDTH_1:
> -		sd_ctrl_write16(host, CTL_SD_MEM_CARD_OPT, 0x80e0);
> -	break;
> -	case MMC_BUS_WIDTH_4:
> -		sd_ctrl_write16(host, CTL_SD_MEM_CARD_OPT, 0x00e0);
> -	break;
> +	if (host->power) {
> +		switch (ios->bus_width) {
> +		case MMC_BUS_WIDTH_1:
> +			sd_ctrl_write16(host, CTL_SD_MEM_CARD_OPT, 0x80e0);
> +		break;
> +		case MMC_BUS_WIDTH_4:
> +			sd_ctrl_write16(host, CTL_SD_MEM_CARD_OPT, 0x00e0);
> +		break;
> +		}
>  	}
>  
>  	/* Let things settle. delay taken from winCE driver */
> -- 
> Regards,
> 
> Laurent Pinchart
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH v2] mmc: tmio: Don't access hardware registers after stopping clocks

[Chris Ball]

Hi,

On Wed, Jun 13 2012, Guennadi Liakhovetski wrote:
> On Tue, 12 Jun 2012, Laurent Pinchart wrote:
>
>> The tmio_mmc_set_ios() function configures the MMC power, clock and bus
>> width. When the mmc core requests the driver to power off the card, we
>> inform runtime PM, that the controller can be suspended. This can lead
>> to the MSTP clock being turned off.
>> 
>> Writing to any 16-bit hardware registers with the MSTP clock off leads
>> to timeouts and errors being printed to the kernel log. This can occur
>> both when stopping the MMC clock and when configuring the bus width.
>> 
>> To fix this, stop the MMC clock before calling put_runtime_pm(), and
>> skip bus width configuration when power is off.
>> 
>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> Acked-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>

Thanks, pushed to mmc-next for 3.6.

- Chris.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child

Re: [PATCH v2] mmc: tmio: Don't access hardware registers after stopping clocks

[Kuninori Morimoto]

Laurent Pinchart wrote:
> 
> The tmio_mmc_set_ios() function configures the MMC power, clock and bus
> width. When the mmc core requests the driver to power off the card, we
> inform runtime PM, that the controller can be suspended. This can lead
> to the MSTP clock being turned off.
> 
> Writing to any 16-bit hardware registers with the MSTP clock off leads
> to timeouts and errors being printed to the kernel log. This can occur
> both when stopping the MMC clock and when configuring the bus width.
> 
> To fix this, stop the MMC clock before calling put_runtime_pm(), and
> skip bus width configuration when power is off.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---

This patch solved kzm9g MicroSD hang-up issue.

Tested-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Best regards
---
Kuninori Morimoto

Re: [PATCH v2] mmc: tmio: Don't access hardware registers after stopping clocks

[Simon Horman]

On Wed, Jun 13, 2012 at 09:11:13PM -0700, Kuninori Morimoto wrote:
> 
> Laurent Pinchart wrote:
> > 
> > The tmio_mmc_set_ios() function configures the MMC power, clock and bus
> > width. When the mmc core requests the driver to power off the card, we
> > inform runtime PM, that the controller can be suspended. This can lead
> > to the MSTP clock being turned off.
> > 
> > Writing to any 16-bit hardware registers with the MSTP clock off leads
> > to timeouts and errors being printed to the kernel log. This can occur
> > both when stopping the MMC clock and when configuring the bus width.
> > 
> > To fix this, stop the MMC clock before calling put_runtime_pm(), and
> > skip bus width configuration when power is off.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> 
> This patch solved kzm9g MicroSD hang-up issue.
> 
> Tested-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Yes, it solves that issue for me too.

Tested-by: Simon Horman <horms@verge.net.au>