diff mbox

[3/5] mmc: sdhi: Add write16_hook

Message ID 1308550015-15717-4-git-send-email-horms@verge.net.au (mailing list archive)
State New, archived
Headers show

Commit Message

Simon Horman June 20, 2011, 6:06 a.m. UTC
Some controllers require waiting for the bus to become idle
before writing to some registers. I have implemented this
by adding a hook to sd_ctrl_write16() and implementing
a hook for SDHI which waits for the bus to become idle.

Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Magnus Damm <magnus.damm@gmail.com>
Signed-off-by: Simon Horman <horms@verge.net.au>

---

Dependenvies: "mmc: tmio: Share register access functions"
---
 drivers/mmc/host/sh_mobile_sdhi.c |   34 ++++++++++++++++++++++++++++++++++
 drivers/mmc/host/tmio_mmc.h       |    2 ++
 include/linux/mfd/tmio.h          |    8 ++++++++
 include/linux/mmc/tmio.h          |    1 +
 4 files changed, 45 insertions(+), 0 deletions(-)

Comments

Paul Mundt June 20, 2011, 6:25 a.m. UTC | #1
On Mon, Jun 20, 2011 at 03:06:53PM +0900, Simon Horman wrote:
> diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
> index b365429..6c56453 100644
> --- a/drivers/mmc/host/sh_mobile_sdhi.c
> +++ b/drivers/mmc/host/sh_mobile_sdhi.c
> @@ -27,6 +27,8 @@
>  #include <linux/mfd/tmio.h>
>  #include <linux/sh_dma.h>
>  
> +#include <asm/delay.h>
> +
>  #include "tmio_mmc.h"
>  
linux/delay.h will suffice.

> +static void sh_mobile_sdhi_wait_idle(struct tmio_mmc_host *host)
> +{
> +	int timeout = 1000;
> +
> +	while (--timeout && !(sd_ctrl_read16(host, CTL_STATUS2) & (1 << 13)))
> +		udelay(1);
> +
> +	if (!timeout)
> +		dev_warn(host->pdata->dev, "timeout waiting for SD bus idle\n");
> +
> +}
> +
> +static void sh_mobile_sdhi_write16_hook(struct tmio_mmc_host *host, int addr)
> +{
> +	if (!(host->pdata->flags & TMIO_MMC_HAS_IDLE_WAIT))
> +		return;
> +
> +	switch (addr)
> +	{
> +	case CTL_SD_CMD:
> +	case CTL_STOP_INTERNAL_ACTION:
> +	case CTL_XFER_BLK_COUNT:
> +	case CTL_SD_CARD_CLK_CTL:
> +	case CTL_SD_XFER_LEN:
> +	case CTL_SD_MEM_CARD_OPT:
> +	case CTL_TRANSACTION_CTL:
> +	case CTL_DMA_ENABLE:
> +		sh_mobile_sdhi_wait_idle(host);
> +	}
> +}
> +
The timeout error should really be handed down.

> diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
> index 0c22df0..5912cf3 100644
> --- a/drivers/mmc/host/tmio_mmc.h
> +++ b/drivers/mmc/host/tmio_mmc.h
> @@ -153,6 +153,8 @@ static inline u32 sd_ctrl_read32(struct tmio_mmc_host *host, int addr)
>  
>  static inline void sd_ctrl_write16(struct tmio_mmc_host *host, int addr, u16 val)
>  {
> +	if (host->pdata->write16_hook)
> +		host->pdata->write16_hook(host, addr);
>  	writew(val, host->ctl + (addr << host->bus_shift));
>  }
>  
If it times out you are unlikely to want to proceed with the write. At
best you end up with a lost write cycle, at worst potentially a bus
lockup you can't easily recover from.
--
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
Guennadi Liakhovetski June 20, 2011, 6:29 a.m. UTC | #2
On Mon, 20 Jun 2011, Simon Horman wrote:

> Some controllers require waiting for the bus to become idle
> before writing to some registers. I have implemented this
> by adding a hook to sd_ctrl_write16() and implementing
> a hook for SDHI which waits for the bus to become idle.
> 
> Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> Cc: Magnus Damm <magnus.damm@gmail.com>
> Signed-off-by: Simon Horman <horms@verge.net.au>
> 
> ---
> 
> Dependenvies: "mmc: tmio: Share register access functions"
> ---
>  drivers/mmc/host/sh_mobile_sdhi.c |   34 ++++++++++++++++++++++++++++++++++
>  drivers/mmc/host/tmio_mmc.h       |    2 ++
>  include/linux/mfd/tmio.h          |    8 ++++++++
>  include/linux/mmc/tmio.h          |    1 +
>  4 files changed, 45 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
> index b365429..6c56453 100644
> --- a/drivers/mmc/host/sh_mobile_sdhi.c
> +++ b/drivers/mmc/host/sh_mobile_sdhi.c
> @@ -27,6 +27,8 @@
>  #include <linux/mfd/tmio.h>
>  #include <linux/sh_dma.h>
>  
> +#include <asm/delay.h>

linux/delay.h

> +
>  #include "tmio_mmc.h"
>  
>  struct sh_mobile_sdhi {
> @@ -55,6 +57,37 @@ static int sh_mobile_sdhi_get_cd(struct platform_device *pdev)
>  		return -ENOSYS;
>  }
>  
> +static void sh_mobile_sdhi_wait_idle(struct tmio_mmc_host *host)
> +{
> +	int timeout = 1000;
> +
> +	while (--timeout && !(sd_ctrl_read16(host, CTL_STATUS2) & (1 << 13)))
> +		udelay(1);
> +
> +	if (!timeout)
> +		dev_warn(host->pdata->dev, "timeout waiting for SD bus idle\n");
> +
> +}
> +
> +static void sh_mobile_sdhi_write16_hook(struct tmio_mmc_host *host, int addr)
> +{
> +	if (!(host->pdata->flags & TMIO_MMC_HAS_IDLE_WAIT))
> +		return;
> +
> +	switch (addr)
> +	{
> +	case CTL_SD_CMD:
> +	case CTL_STOP_INTERNAL_ACTION:
> +	case CTL_XFER_BLK_COUNT:
> +	case CTL_SD_CARD_CLK_CTL:
> +	case CTL_SD_XFER_LEN:
> +	case CTL_SD_MEM_CARD_OPT:
> +	case CTL_TRANSACTION_CTL:
> +	case CTL_DMA_ENABLE:
> +		sh_mobile_sdhi_wait_idle(host);
> +	}
> +}
> +
>  static int __devinit sh_mobile_sdhi_probe(struct platform_device *pdev)
>  {
>  	struct sh_mobile_sdhi *priv;
> @@ -86,6 +119,7 @@ static int __devinit sh_mobile_sdhi_probe(struct platform_device *pdev)
>  	mmc_data->hclk = clk_get_rate(priv->clk);
>  	mmc_data->set_pwr = sh_mobile_sdhi_set_pwr;
>  	mmc_data->get_cd = sh_mobile_sdhi_get_cd;
> +	mmc_data->write16_hook = sh_mobile_sdhi_write16_hook;

Not sure I like the "write16_hook" name. You consider this callback as 
something the host might need to do, when writing to a 16-bit register. In 
this specific case it is actually waiting for the bus to become idle, 
which might also be needed in other locations. So, maybe it is better to 
call this callback "wait_idle" or something similar? Or you think, other 
hosts might need a write16 hook doing something different?...

Thanks
Guennadi

>  	mmc_data->capabilities = MMC_CAP_MMC_HIGHSPEED;
>  	if (p) {
>  		mmc_data->flags = p->tmio_flags;
> diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
> index 0c22df0..5912cf3 100644
> --- a/drivers/mmc/host/tmio_mmc.h
> +++ b/drivers/mmc/host/tmio_mmc.h
> @@ -153,6 +153,8 @@ static inline u32 sd_ctrl_read32(struct tmio_mmc_host *host, int addr)
>  
>  static inline void sd_ctrl_write16(struct tmio_mmc_host *host, int addr, u16 val)
>  {
> +	if (host->pdata->write16_hook)
> +		host->pdata->write16_hook(host, addr);
>  	writew(val, host->ctl + (addr << host->bus_shift));
>  }
>  
> diff --git a/include/linux/mfd/tmio.h b/include/linux/mfd/tmio.h
> index 5a90266..dc5db86 100644
> --- a/include/linux/mfd/tmio.h
> +++ b/include/linux/mfd/tmio.h
> @@ -68,6 +68,11 @@
>   * controller and report the event to the driver.
>   */
>  #define TMIO_MMC_HAS_COLD_CD		(1 << 3)
> +/*
> + * Some controllers require waiting for the SD bus to become
> + * idle before writing to some registers.
> + */
> +#define TMIO_MMC_HAS_IDLE_WAIT		(1 << 4)
>  
>  int tmio_core_mmc_enable(void __iomem *cnf, int shift, unsigned long base);
>  int tmio_core_mmc_resume(void __iomem *cnf, int shift, unsigned long base);
> @@ -80,6 +85,8 @@ struct tmio_mmc_dma {
>  	int alignment_shift;
>  };
>  
> +struct tmio_mmc_host;
> +
>  /*
>   * data for the MMC controller
>   */
> @@ -94,6 +101,7 @@ struct tmio_mmc_data {
>  	void (*set_pwr)(struct platform_device *host, int state);
>  	void (*set_clk_div)(struct platform_device *host, int state);
>  	int (*get_cd)(struct platform_device *host);
> +	void (*write16_hook)(struct tmio_mmc_host *host, int addr);
>  };
>  
>  static inline void tmio_mmc_cd_wakeup(struct tmio_mmc_data *pdata)
> diff --git a/include/linux/mmc/tmio.h b/include/linux/mmc/tmio.h
> index 0a6e40d..f235757 100644
> --- a/include/linux/mmc/tmio.h
> +++ b/include/linux/mmc/tmio.h
> @@ -21,6 +21,7 @@
>  #define CTL_XFER_BLK_COUNT 0xa
>  #define CTL_RESPONSE 0x0c
>  #define CTL_STATUS 0x1c
> +#define CTL_STATUS2 0x1e
>  #define CTL_IRQ_MASK 0x20
>  #define CTL_SD_CARD_CLK_CTL 0x24
>  #define CTL_SD_XFER_LEN 0x26
> -- 
> 1.7.4.4
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
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
Magnus Damm June 20, 2011, 7:04 a.m. UTC | #3
On Mon, Jun 20, 2011 at 3:06 PM, Simon Horman <horms@verge.net.au> wrote:
> Some controllers require waiting for the bus to become idle
> before writing to some registers. I have implemented this
> by adding a hook to sd_ctrl_write16() and implementing
> a hook for SDHI which waits for the bus to become idle.
>
> Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> Cc: Magnus Damm <magnus.damm@gmail.com>
> Signed-off-by: Simon Horman <horms@verge.net.au>
>
> ---

Hi Simon,

Thanks for your work on this!

> --- a/drivers/mmc/host/sh_mobile_sdhi.c
> +++ b/drivers/mmc/host/sh_mobile_sdhi.c
> @@ -55,6 +57,37 @@ static int sh_mobile_sdhi_get_cd(struct platform_device *pdev)
>                return -ENOSYS;
>  }
>
> +static void sh_mobile_sdhi_wait_idle(struct tmio_mmc_host *host)
> +{
> +       int timeout = 1000;
> +
> +       while (--timeout && !(sd_ctrl_read16(host, CTL_STATUS2) & (1 << 13)))
> +               udelay(1);
> +
> +       if (!timeout)
> +               dev_warn(host->pdata->dev, "timeout waiting for SD bus idle\n");
> +
> +}
> +
> +static void sh_mobile_sdhi_write16_hook(struct tmio_mmc_host *host, int addr)
> +{
> +       if (!(host->pdata->flags & TMIO_MMC_HAS_IDLE_WAIT))
> +               return;

and

> @@ -86,6 +119,7 @@ static int __devinit sh_mobile_sdhi_probe(struct platform_device *pdev)
>        mmc_data->hclk = clk_get_rate(priv->clk);
>        mmc_data->set_pwr = sh_mobile_sdhi_set_pwr;
>        mmc_data->get_cd = sh_mobile_sdhi_get_cd;
> +       mmc_data->write16_hook = sh_mobile_sdhi_write16_hook;
>        mmc_data->capabilities = MMC_CAP_MMC_HIGHSPEED;
>        if (p) {
>                mmc_data->flags = p->tmio_flags;

Doesn't it make more sense to do:

if (!(host->pdata->flags & TMIO_MMC_HAS_IDLE_WAIT))
  mmc_data->write16_hook = sh_mobile_sdhi_write16_hook;

and skip the check inside the sh_mobile_sdhi_write16_hook() function?

No need to do that check on every register access unless really needed.

Thanks,

/ magnus
--
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
Simon Horman June 20, 2011, 1:40 p.m. UTC | #4
On Mon, Jun 20, 2011 at 08:29:18AM +0200, Guennadi Liakhovetski wrote:
> On Mon, 20 Jun 2011, Simon Horman wrote:
> 
> > Some controllers require waiting for the bus to become idle
> > before writing to some registers. I have implemented this
> > by adding a hook to sd_ctrl_write16() and implementing
> > a hook for SDHI which waits for the bus to become idle.
> > 
> > Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > Cc: Magnus Damm <magnus.damm@gmail.com>
> > Signed-off-by: Simon Horman <horms@verge.net.au>
> > 
> > ---
> > 
> > Dependenvies: "mmc: tmio: Share register access functions"
> > ---
> >  drivers/mmc/host/sh_mobile_sdhi.c |   34 ++++++++++++++++++++++++++++++++++
> >  drivers/mmc/host/tmio_mmc.h       |    2 ++
> >  include/linux/mfd/tmio.h          |    8 ++++++++
> >  include/linux/mmc/tmio.h          |    1 +
> >  4 files changed, 45 insertions(+), 0 deletions(-)
> > 
> > diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
> > index b365429..6c56453 100644
> > --- a/drivers/mmc/host/sh_mobile_sdhi.c
> > +++ b/drivers/mmc/host/sh_mobile_sdhi.c
> > @@ -27,6 +27,8 @@
> >  #include <linux/mfd/tmio.h>
> >  #include <linux/sh_dma.h>
> >  
> > +#include <asm/delay.h>
> 
> linux/delay.h

Thanks.

> > +
> >  #include "tmio_mmc.h"
> >  
> >  struct sh_mobile_sdhi {
> > @@ -55,6 +57,37 @@ static int sh_mobile_sdhi_get_cd(struct platform_device *pdev)
> >  		return -ENOSYS;
> >  }
> >  
> > +static void sh_mobile_sdhi_wait_idle(struct tmio_mmc_host *host)
> > +{
> > +	int timeout = 1000;
> > +
> > +	while (--timeout && !(sd_ctrl_read16(host, CTL_STATUS2) & (1 << 13)))
> > +		udelay(1);
> > +
> > +	if (!timeout)
> > +		dev_warn(host->pdata->dev, "timeout waiting for SD bus idle\n");
> > +
> > +}
> > +
> > +static void sh_mobile_sdhi_write16_hook(struct tmio_mmc_host *host, int addr)
> > +{
> > +	if (!(host->pdata->flags & TMIO_MMC_HAS_IDLE_WAIT))
> > +		return;
> > +
> > +	switch (addr)
> > +	{
> > +	case CTL_SD_CMD:
> > +	case CTL_STOP_INTERNAL_ACTION:
> > +	case CTL_XFER_BLK_COUNT:
> > +	case CTL_SD_CARD_CLK_CTL:
> > +	case CTL_SD_XFER_LEN:
> > +	case CTL_SD_MEM_CARD_OPT:
> > +	case CTL_TRANSACTION_CTL:
> > +	case CTL_DMA_ENABLE:
> > +		sh_mobile_sdhi_wait_idle(host);
> > +	}
> > +}
> > +
> >  static int __devinit sh_mobile_sdhi_probe(struct platform_device *pdev)
> >  {
> >  	struct sh_mobile_sdhi *priv;
> > @@ -86,6 +119,7 @@ static int __devinit sh_mobile_sdhi_probe(struct platform_device *pdev)
> >  	mmc_data->hclk = clk_get_rate(priv->clk);
> >  	mmc_data->set_pwr = sh_mobile_sdhi_set_pwr;
> >  	mmc_data->get_cd = sh_mobile_sdhi_get_cd;
> > +	mmc_data->write16_hook = sh_mobile_sdhi_write16_hook;
> 
> Not sure I like the "write16_hook" name. You consider this callback as 
> something the host might need to do, when writing to a 16-bit register. In 
> this specific case it is actually waiting for the bus to become idle, 
> which might also be needed in other locations. So, maybe it is better to 
> call this callback "wait_idle" or something similar? Or you think, other 
> hosts might need a write16 hook doing something different?...

I'm not strongly attached to the name, and I do see your point.  But as it
stands the hook is currently only needed on 16bit register writes; the hook
is called from sd_ctrl_write16(): and sh_mobile_sdhi_write16_hook() works
by looking at addr, which is specific to register reads and writes.  So
while it isn't great, I think the current name isn't entirely horrible.

I think its a little hard to look into a crystal ball and see
where other TMIO hardware might need wait_idle(). Who knows
what we might need to do for the next batch of hardware? :-)

I had considered other approaches to this problem, such as
calling something_wait_idle() from within tmio_mmc_*.c as needed.
But the approach in the patch I posted turned out to be quite
a lot cleaner and have the advantage of concisely documenting
what is being done - that is, writes to which registers need
wait_idle treatment.

The down side is that the callback is obviously more specific
than perhaps a hook ought to be. But I think that it can evolve
as the need arises.

--
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
Simon Horman June 20, 2011, 1:40 p.m. UTC | #5
On Mon, Jun 20, 2011 at 04:04:04PM +0900, Magnus Damm wrote:
> On Mon, Jun 20, 2011 at 3:06 PM, Simon Horman <horms@verge.net.au> wrote:
> > Some controllers require waiting for the bus to become idle
> > before writing to some registers. I have implemented this
> > by adding a hook to sd_ctrl_write16() and implementing
> > a hook for SDHI which waits for the bus to become idle.
> >
> > Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > Cc: Magnus Damm <magnus.damm@gmail.com>
> > Signed-off-by: Simon Horman <horms@verge.net.au>
> >
> > ---
> 
> Hi Simon,
> 
> Thanks for your work on this!
> 
> > --- a/drivers/mmc/host/sh_mobile_sdhi.c
> > +++ b/drivers/mmc/host/sh_mobile_sdhi.c
> > @@ -55,6 +57,37 @@ static int sh_mobile_sdhi_get_cd(struct platform_device *pdev)
> >                return -ENOSYS;
> >  }
> >
> > +static void sh_mobile_sdhi_wait_idle(struct tmio_mmc_host *host)
> > +{
> > +       int timeout = 1000;
> > +
> > +       while (--timeout && !(sd_ctrl_read16(host, CTL_STATUS2) & (1 << 13)))
> > +               udelay(1);
> > +
> > +       if (!timeout)
> > +               dev_warn(host->pdata->dev, "timeout waiting for SD bus idle\n");
> > +
> > +}
> > +
> > +static void sh_mobile_sdhi_write16_hook(struct tmio_mmc_host *host, int addr)
> > +{
> > +       if (!(host->pdata->flags & TMIO_MMC_HAS_IDLE_WAIT))
> > +               return;
> 
> and
> 
> > @@ -86,6 +119,7 @@ static int __devinit sh_mobile_sdhi_probe(struct platform_device *pdev)
> >        mmc_data->hclk = clk_get_rate(priv->clk);
> >        mmc_data->set_pwr = sh_mobile_sdhi_set_pwr;
> >        mmc_data->get_cd = sh_mobile_sdhi_get_cd;
> > +       mmc_data->write16_hook = sh_mobile_sdhi_write16_hook;
> >        mmc_data->capabilities = MMC_CAP_MMC_HIGHSPEED;
> >        if (p) {
> >                mmc_data->flags = p->tmio_flags;
> 
> Doesn't it make more sense to do:
> 
> if (!(host->pdata->flags & TMIO_MMC_HAS_IDLE_WAIT))
>   mmc_data->write16_hook = sh_mobile_sdhi_write16_hook;
> 
> and skip the check inside the sh_mobile_sdhi_write16_hook() function?
> 
> No need to do that check on every register access unless really needed.

Sure, good idea.
--
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
Simon Horman June 20, 2011, 1:42 p.m. UTC | #6
On Mon, Jun 20, 2011 at 03:25:22PM +0900, Paul Mundt wrote:
> On Mon, Jun 20, 2011 at 03:06:53PM +0900, Simon Horman wrote:
> > diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
> > index b365429..6c56453 100644
> > --- a/drivers/mmc/host/sh_mobile_sdhi.c
> > +++ b/drivers/mmc/host/sh_mobile_sdhi.c
> > @@ -27,6 +27,8 @@
> >  #include <linux/mfd/tmio.h>
> >  #include <linux/sh_dma.h>
> >  
> > +#include <asm/delay.h>
> > +
> >  #include "tmio_mmc.h"
> >  
> linux/delay.h will suffice.

Thanks.

> > +static void sh_mobile_sdhi_wait_idle(struct tmio_mmc_host *host)
> > +{
> > +	int timeout = 1000;
> > +
> > +	while (--timeout && !(sd_ctrl_read16(host, CTL_STATUS2) & (1 << 13)))
> > +		udelay(1);
> > +
> > +	if (!timeout)
> > +		dev_warn(host->pdata->dev, "timeout waiting for SD bus idle\n");
> > +
> > +}
> > +
> > +static void sh_mobile_sdhi_write16_hook(struct tmio_mmc_host *host, int addr)
> > +{
> > +	if (!(host->pdata->flags & TMIO_MMC_HAS_IDLE_WAIT))
> > +		return;
> > +
> > +	switch (addr)
> > +	{
> > +	case CTL_SD_CMD:
> > +	case CTL_STOP_INTERNAL_ACTION:
> > +	case CTL_XFER_BLK_COUNT:
> > +	case CTL_SD_CARD_CLK_CTL:
> > +	case CTL_SD_XFER_LEN:
> > +	case CTL_SD_MEM_CARD_OPT:
> > +	case CTL_TRANSACTION_CTL:
> > +	case CTL_DMA_ENABLE:
> > +		sh_mobile_sdhi_wait_idle(host);
> > +	}
> > +}
> > +
> The timeout error should really be handed down.
> 
> > diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
> > index 0c22df0..5912cf3 100644
> > --- a/drivers/mmc/host/tmio_mmc.h
> > +++ b/drivers/mmc/host/tmio_mmc.h
> > @@ -153,6 +153,8 @@ static inline u32 sd_ctrl_read32(struct tmio_mmc_host *host, int addr)
> >  
> >  static inline void sd_ctrl_write16(struct tmio_mmc_host *host, int addr, u16 val)
> >  {
> > +	if (host->pdata->write16_hook)
> > +		host->pdata->write16_hook(host, addr);
> >  	writew(val, host->ctl + (addr << host->bus_shift));
> >  }
> >  
> If it times out you are unlikely to want to proceed with the write. At
> best you end up with a lost write cycle, at worst potentially a bus
> lockup you can't easily recover from.

As discussed off-line, I'll change this around so that
write16_hook() may return an error and the write will be
skipped if that occurs.

--
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
diff mbox

Patch

diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
index b365429..6c56453 100644
--- a/drivers/mmc/host/sh_mobile_sdhi.c
+++ b/drivers/mmc/host/sh_mobile_sdhi.c
@@ -27,6 +27,8 @@ 
 #include <linux/mfd/tmio.h>
 #include <linux/sh_dma.h>
 
+#include <asm/delay.h>
+
 #include "tmio_mmc.h"
 
 struct sh_mobile_sdhi {
@@ -55,6 +57,37 @@  static int sh_mobile_sdhi_get_cd(struct platform_device *pdev)
 		return -ENOSYS;
 }
 
+static void sh_mobile_sdhi_wait_idle(struct tmio_mmc_host *host)
+{
+	int timeout = 1000;
+
+	while (--timeout && !(sd_ctrl_read16(host, CTL_STATUS2) & (1 << 13)))
+		udelay(1);
+
+	if (!timeout)
+		dev_warn(host->pdata->dev, "timeout waiting for SD bus idle\n");
+
+}
+
+static void sh_mobile_sdhi_write16_hook(struct tmio_mmc_host *host, int addr)
+{
+	if (!(host->pdata->flags & TMIO_MMC_HAS_IDLE_WAIT))
+		return;
+
+	switch (addr)
+	{
+	case CTL_SD_CMD:
+	case CTL_STOP_INTERNAL_ACTION:
+	case CTL_XFER_BLK_COUNT:
+	case CTL_SD_CARD_CLK_CTL:
+	case CTL_SD_XFER_LEN:
+	case CTL_SD_MEM_CARD_OPT:
+	case CTL_TRANSACTION_CTL:
+	case CTL_DMA_ENABLE:
+		sh_mobile_sdhi_wait_idle(host);
+	}
+}
+
 static int __devinit sh_mobile_sdhi_probe(struct platform_device *pdev)
 {
 	struct sh_mobile_sdhi *priv;
@@ -86,6 +119,7 @@  static int __devinit sh_mobile_sdhi_probe(struct platform_device *pdev)
 	mmc_data->hclk = clk_get_rate(priv->clk);
 	mmc_data->set_pwr = sh_mobile_sdhi_set_pwr;
 	mmc_data->get_cd = sh_mobile_sdhi_get_cd;
+	mmc_data->write16_hook = sh_mobile_sdhi_write16_hook;
 	mmc_data->capabilities = MMC_CAP_MMC_HIGHSPEED;
 	if (p) {
 		mmc_data->flags = p->tmio_flags;
diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index 0c22df0..5912cf3 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -153,6 +153,8 @@  static inline u32 sd_ctrl_read32(struct tmio_mmc_host *host, int addr)
 
 static inline void sd_ctrl_write16(struct tmio_mmc_host *host, int addr, u16 val)
 {
+	if (host->pdata->write16_hook)
+		host->pdata->write16_hook(host, addr);
 	writew(val, host->ctl + (addr << host->bus_shift));
 }
 
diff --git a/include/linux/mfd/tmio.h b/include/linux/mfd/tmio.h
index 5a90266..dc5db86 100644
--- a/include/linux/mfd/tmio.h
+++ b/include/linux/mfd/tmio.h
@@ -68,6 +68,11 @@ 
  * controller and report the event to the driver.
  */
 #define TMIO_MMC_HAS_COLD_CD		(1 << 3)
+/*
+ * Some controllers require waiting for the SD bus to become
+ * idle before writing to some registers.
+ */
+#define TMIO_MMC_HAS_IDLE_WAIT		(1 << 4)
 
 int tmio_core_mmc_enable(void __iomem *cnf, int shift, unsigned long base);
 int tmio_core_mmc_resume(void __iomem *cnf, int shift, unsigned long base);
@@ -80,6 +85,8 @@  struct tmio_mmc_dma {
 	int alignment_shift;
 };
 
+struct tmio_mmc_host;
+
 /*
  * data for the MMC controller
  */
@@ -94,6 +101,7 @@  struct tmio_mmc_data {
 	void (*set_pwr)(struct platform_device *host, int state);
 	void (*set_clk_div)(struct platform_device *host, int state);
 	int (*get_cd)(struct platform_device *host);
+	void (*write16_hook)(struct tmio_mmc_host *host, int addr);
 };
 
 static inline void tmio_mmc_cd_wakeup(struct tmio_mmc_data *pdata)
diff --git a/include/linux/mmc/tmio.h b/include/linux/mmc/tmio.h
index 0a6e40d..f235757 100644
--- a/include/linux/mmc/tmio.h
+++ b/include/linux/mmc/tmio.h
@@ -21,6 +21,7 @@ 
 #define CTL_XFER_BLK_COUNT 0xa
 #define CTL_RESPONSE 0x0c
 #define CTL_STATUS 0x1c
+#define CTL_STATUS2 0x1e
 #define CTL_IRQ_MASK 0x20
 #define CTL_SD_CARD_CLK_CTL 0x24
 #define CTL_SD_XFER_LEN 0x26