diff mbox

mmc: tmio: add barriers to IO operations

Message ID Pine.LNX.4.64.1301231745130.26301@axis700.grange (mailing list archive)
State Superseded
Headers show

Commit Message

Guennadi Liakhovetski Jan. 23, 2013, 4:46 p.m. UTC
Without barriers SDIO operations fail with runtime PM enabled.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/mmc/host/tmio_mmc.h |   14 ++++++++------
 1 files changed, 8 insertions(+), 6 deletions(-)

Comments

Paul Mundt Jan. 24, 2013, 11:10 a.m. UTC | #1
On Wed, Jan 23, 2013 at 05:46:18PM +0100, Guennadi Liakhovetski wrote:
> Without barriers SDIO operations fail with runtime PM enabled.
> 
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> ---
>  drivers/mmc/host/tmio_mmc.h |   14 ++++++++------
>  1 files changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
> index d857f5c..ad1a1c6 100644
> --- a/drivers/mmc/host/tmio_mmc.h
> +++ b/drivers/mmc/host/tmio_mmc.h
> @@ -159,19 +159,20 @@ int tmio_mmc_host_runtime_resume(struct device *dev);
>  
>  static inline u16 sd_ctrl_read16(struct tmio_mmc_host *host, int addr)
>  {
> -	return readw(host->ctl + (addr << host->bus_shift));
> +	return ioread16(host->ctl + (addr << host->bus_shift));
>  }
>  
>  static inline void sd_ctrl_read16_rep(struct tmio_mmc_host *host, int addr,
>  		u16 *buf, int count)
>  {
> +	wmb();
>  	readsw(host->ctl + (addr << host->bus_shift), buf, count);
>  }
>  
Some reason to not use ioread16_rep() and friends?
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Guennadi Liakhovetski Jan. 24, 2013, 11:20 a.m. UTC | #2
On Thu, 24 Jan 2013, Paul Mundt wrote:

> On Wed, Jan 23, 2013 at 05:46:18PM +0100, Guennadi Liakhovetski wrote:
> > Without barriers SDIO operations fail with runtime PM enabled.
> > 
> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > ---
> >  drivers/mmc/host/tmio_mmc.h |   14 ++++++++------
> >  1 files changed, 8 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
> > index d857f5c..ad1a1c6 100644
> > --- a/drivers/mmc/host/tmio_mmc.h
> > +++ b/drivers/mmc/host/tmio_mmc.h
> > @@ -159,19 +159,20 @@ int tmio_mmc_host_runtime_resume(struct device *dev);
> >  
> >  static inline u16 sd_ctrl_read16(struct tmio_mmc_host *host, int addr)
> >  {
> > -	return readw(host->ctl + (addr << host->bus_shift));
> > +	return ioread16(host->ctl + (addr << host->bus_shift));
> >  }
> >  
> >  static inline void sd_ctrl_read16_rep(struct tmio_mmc_host *host, int addr,
> >  		u16 *buf, int count)
> >  {
> > +	wmb();
> >  	readsw(host->ctl + (addr << host->bus_shift), buf, count);
> >  }
> >  
> Some reason to not use ioread16_rep() and friends?

Aren't they "raw," i.e. without barriers:

#define ioread16_rep(p,d,c)	__raw_readsw(p,d,c)

?

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Paul Mundt Jan. 24, 2013, 11:42 a.m. UTC | #3
On Thu, Jan 24, 2013 at 12:20:19PM +0100, Guennadi Liakhovetski wrote:
> On Thu, 24 Jan 2013, Paul Mundt wrote:
> 
> > On Wed, Jan 23, 2013 at 05:46:18PM +0100, Guennadi Liakhovetski wrote:
> > > Without barriers SDIO operations fail with runtime PM enabled.
> > > 
> > > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > > ---
> > >  drivers/mmc/host/tmio_mmc.h |   14 ++++++++------
> > >  1 files changed, 8 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
> > > index d857f5c..ad1a1c6 100644
> > > --- a/drivers/mmc/host/tmio_mmc.h
> > > +++ b/drivers/mmc/host/tmio_mmc.h
> > > @@ -159,19 +159,20 @@ int tmio_mmc_host_runtime_resume(struct device *dev);
> > >  
> > >  static inline u16 sd_ctrl_read16(struct tmio_mmc_host *host, int addr)
> > >  {
> > > -	return readw(host->ctl + (addr << host->bus_shift));
> > > +	return ioread16(host->ctl + (addr << host->bus_shift));
> > >  }
> > >  
> > >  static inline void sd_ctrl_read16_rep(struct tmio_mmc_host *host, int addr,
> > >  		u16 *buf, int count)
> > >  {
> > > +	wmb();
> > >  	readsw(host->ctl + (addr << host->bus_shift), buf, count);
> > >  }
> > >  
> > Some reason to not use ioread16_rep() and friends?
> 
> Aren't they "raw," i.e. without barriers:
> 
> #define ioread16_rep(p,d,c)	__raw_readsw(p,d,c)

I wasn't talking about the barrier semantics, I meant for consistency, as
you are converting to the ioread/write API for everything else.
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Guennadi Liakhovetski Jan. 24, 2013, 11:52 a.m. UTC | #4
On Thu, 24 Jan 2013, Paul Mundt wrote:

> On Thu, Jan 24, 2013 at 12:20:19PM +0100, Guennadi Liakhovetski wrote:
> > On Thu, 24 Jan 2013, Paul Mundt wrote:
> > 
> > > On Wed, Jan 23, 2013 at 05:46:18PM +0100, Guennadi Liakhovetski wrote:
> > > > Without barriers SDIO operations fail with runtime PM enabled.
> > > > 
> > > > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > > > ---
> > > >  drivers/mmc/host/tmio_mmc.h |   14 ++++++++------
> > > >  1 files changed, 8 insertions(+), 6 deletions(-)
> > > > 
> > > > diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
> > > > index d857f5c..ad1a1c6 100644
> > > > --- a/drivers/mmc/host/tmio_mmc.h
> > > > +++ b/drivers/mmc/host/tmio_mmc.h
> > > > @@ -159,19 +159,20 @@ int tmio_mmc_host_runtime_resume(struct device *dev);
> > > >  
> > > >  static inline u16 sd_ctrl_read16(struct tmio_mmc_host *host, int addr)
> > > >  {
> > > > -	return readw(host->ctl + (addr << host->bus_shift));
> > > > +	return ioread16(host->ctl + (addr << host->bus_shift));
> > > >  }
> > > >  
> > > >  static inline void sd_ctrl_read16_rep(struct tmio_mmc_host *host, int addr,
> > > >  		u16 *buf, int count)
> > > >  {
> > > > +	wmb();
> > > >  	readsw(host->ctl + (addr << host->bus_shift), buf, count);
> > > >  }
> > > >  
> > > Some reason to not use ioread16_rep() and friends?
> > 
> > Aren't they "raw," i.e. without barriers:
> > 
> > #define ioread16_rep(p,d,c)	__raw_readsw(p,d,c)
> 
> I wasn't talking about the barrier semantics, I meant for consistency, as
> you are converting to the ioread/write API for everything else.

Ok, I see. But the reason to replace readw() / writew() ops with io* was 
to add barriers, whereas this change wouldn't actually do any good. So, 
yeah, we could do that for consistency, but otherwise it wouldn't bring 
any more benefits and explicit barriers would have to be kept anyway.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" 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/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index d857f5c..ad1a1c6 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -159,19 +159,20 @@  int tmio_mmc_host_runtime_resume(struct device *dev);
 
 static inline u16 sd_ctrl_read16(struct tmio_mmc_host *host, int addr)
 {
-	return readw(host->ctl + (addr << host->bus_shift));
+	return ioread16(host->ctl + (addr << host->bus_shift));
 }
 
 static inline void sd_ctrl_read16_rep(struct tmio_mmc_host *host, int addr,
 		u16 *buf, int count)
 {
+	wmb();
 	readsw(host->ctl + (addr << host->bus_shift), buf, count);
 }
 
 static inline u32 sd_ctrl_read32(struct tmio_mmc_host *host, int addr)
 {
-	return readw(host->ctl + (addr << host->bus_shift)) |
-	       readw(host->ctl + ((addr + 2) << host->bus_shift)) << 16;
+	return ioread16(host->ctl + (addr << host->bus_shift)) |
+	       ioread16(host->ctl + ((addr + 2) << host->bus_shift)) << 16;
 }
 
 static inline void sd_ctrl_write16(struct tmio_mmc_host *host, int addr, u16 val)
@@ -181,19 +182,20 @@  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))
 		return;
-	writew(val, host->ctl + (addr << host->bus_shift));
+	iowrite16(val, host->ctl + (addr << host->bus_shift));
 }
 
 static inline void sd_ctrl_write16_rep(struct tmio_mmc_host *host, int addr,
 		u16 *buf, int count)
 {
 	writesw(host->ctl + (addr << host->bus_shift), buf, count);
+	wmb();
 }
 
 static inline void sd_ctrl_write32(struct tmio_mmc_host *host, int addr, u32 val)
 {
-	writew(val, host->ctl + (addr << host->bus_shift));
-	writew(val >> 16, host->ctl + ((addr + 2) << host->bus_shift));
+	iowrite16(val, host->ctl + (addr << host->bus_shift));
+	iowrite16(val >> 16, host->ctl + ((addr + 2) << host->bus_shift));
 }