diff mbox

[v3,2/5] usb: chipidea: add freescale imx28 special write register method

Message ID 1382680943-9258-2-git-send-email-peter.chen@freescale.com (mailing list archive)
State New, archived
Headers show

Commit Message

Peter Chen Oct. 25, 2013, 6:02 a.m. UTC
According to Freescale imx28 Errata, "ENGR119653 USB: ARM to USB
register error issue", All USB register write operations must
use the ARM SWP instruction. So, we implement special hw_write
and hw_test_and_clear for imx28.

Discussion for it at below:
http://marc.info/?l=linux-usb&m=137996395529294&w=2

Signed-off-by: Peter Chen <peter.chen@freescale.com>
---
Changes for v2:
- Rebase to latest usb-next tree

 drivers/usb/chipidea/ci.h    |   23 +++++++++++++++++++++++
 drivers/usb/chipidea/core.c  |    2 ++
 drivers/usb/chipidea/host.c  |    1 +
 include/linux/usb/chipidea.h |    1 +
 4 files changed, 27 insertions(+), 0 deletions(-)

Comments

Hector Palacios Oct. 28, 2013, 10:36 a.m. UTC | #1
Dear Peter,

On 10/25/2013 08:02 AM, Peter Chen wrote:
> According to Freescale imx28 Errata, "ENGR119653 USB: ARM to USB
> register error issue", All USB register write operations must
> use the ARM SWP instruction. So, we implement special hw_write
> and hw_test_and_clear for imx28.
>
> Discussion for it at below:
> http://marc.info/?l=linux-usb&m=137996395529294&w=2
>
> Signed-off-by: Peter Chen <peter.chen-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> ---
> Changes for v2:
> - Rebase to latest usb-next tree
>
>   drivers/usb/chipidea/ci.h    |   23 +++++++++++++++++++++++
>   drivers/usb/chipidea/core.c  |    2 ++
>   drivers/usb/chipidea/host.c  |    1 +
>   include/linux/usb/chipidea.h |    1 +
>   4 files changed, 27 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
> index 1c94fc5..4eb61d0 100644
> --- a/drivers/usb/chipidea/ci.h
> +++ b/drivers/usb/chipidea/ci.h
> @@ -173,6 +173,8 @@ struct ci_hdrc {
>   	struct dentry			*debugfs;
>   	bool				id_event;
>   	bool				b_sess_valid_event;
> +	/* imx28 needs swp instruction for writing */
> +	bool				imx28_write_fix;
>   };
>
>   static inline struct ci_role_driver *ci_role(struct ci_hdrc *ci)
> @@ -253,6 +255,13 @@ static inline u32 hw_read(struct ci_hdrc *ci, enum ci_hw_regs reg, u32 mask)
>   	return ioread32(ci->hw_bank.regmap[reg]) & mask;
>   }
>
> +#ifdef CONFIG_SOC_IMX28
> +static inline void imx28_ci_writel(u32 val32, volatile u32 *addr)
> +{
> +	__asm__ ("swp %0, %0, [%1]" : : "r"(val32), "r"(addr));
> +}
> +#endif
> +
>   /**
>    * hw_write: writes to a hw register
>    * @reg:  register index
> @@ -266,7 +275,14 @@ static inline void hw_write(struct ci_hdrc *ci, enum ci_hw_regs reg,
>   		data = (ioread32(ci->hw_bank.regmap[reg]) & ~mask)
>   			| (data & mask);
>
> +#ifdef CONFIG_SOC_IMX28
> +	if (ci->imx28_write_fix)
> +		imx28_ci_writel(data, ci->hw_bank.regmap[reg]);
> +	else
> +		iowrite32(data, ci->hw_bank.regmap[reg]);
> +#else
>   	iowrite32(data, ci->hw_bank.regmap[reg]);
> +#endif
>   }
>
>   /**
> @@ -281,7 +297,14 @@ static inline u32 hw_test_and_clear(struct ci_hdrc *ci, enum ci_hw_regs reg,
>   {
>   	u32 val = ioread32(ci->hw_bank.regmap[reg]) & mask;
>
> +#ifdef CONFIG_SOC_IMX28
> +	if (ci->imx28_write_fix)
> +		imx28_ci_writel(val, ci->hw_bank.regmap[reg]);
> +	else
> +		iowrite32(val, ci->hw_bank.regmap[reg]);
> +#else
>   	iowrite32(val, ci->hw_bank.regmap[reg]);
> +#endif
>   	return val;
>   }

Can't we remove the #ifdefs CONFIG_SOC_IMX28 all around?
The check is done on the new flag ci->imx28_write_fix which exists for all platforms, 
isn't it?.

Best regards,
Marek Vasut Oct. 28, 2013, 10:47 a.m. UTC | #2
Dear Hector Palacios,

> Dear Peter,
> 
> On 10/25/2013 08:02 AM, Peter Chen wrote:
> > According to Freescale imx28 Errata, "ENGR119653 USB: ARM to USB
> > register error issue", All USB register write operations must
> > use the ARM SWP instruction. So, we implement special hw_write
> > and hw_test_and_clear for imx28.
> > 
> > Discussion for it at below:
> > http://marc.info/?l=linux-usb&m=137996395529294&w=2
> > 
> > Signed-off-by: Peter Chen
> > <peter.chen-KZfg59tc24xl57MIdRCFDg@public.gmane.org> ---
> > Changes for v2:
> > - Rebase to latest usb-next tree
> > 
> >   drivers/usb/chipidea/ci.h    |   23 +++++++++++++++++++++++
> >   drivers/usb/chipidea/core.c  |    2 ++
> >   drivers/usb/chipidea/host.c  |    1 +
> >   include/linux/usb/chipidea.h |    1 +
> >   4 files changed, 27 insertions(+), 0 deletions(-)
> > 
> > diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
> > index 1c94fc5..4eb61d0 100644
> > --- a/drivers/usb/chipidea/ci.h
> > +++ b/drivers/usb/chipidea/ci.h
> > @@ -173,6 +173,8 @@ struct ci_hdrc {
> > 
> >   	struct dentry			*debugfs;
> >   	bool				id_event;
> >   	bool				b_sess_valid_event;
> > 
> > +	/* imx28 needs swp instruction for writing */
> > +	bool				imx28_write_fix;
> > 
> >   };
> >   
> >   static inline struct ci_role_driver *ci_role(struct ci_hdrc *ci)
> > 
> > @@ -253,6 +255,13 @@ static inline u32 hw_read(struct ci_hdrc *ci, enum
> > ci_hw_regs reg, u32 mask)
> > 
> >   	return ioread32(ci->hw_bank.regmap[reg]) & mask;
> >   
> >   }
> > 
> > +#ifdef CONFIG_SOC_IMX28
> > +static inline void imx28_ci_writel(u32 val32, volatile u32 *addr)
> > +{
> > +	__asm__ ("swp %0, %0, [%1]" : : "r"(val32), "r"(addr));
> > +}
> > +#endif
> > +
> > 
> >   /**
> >   
> >    * hw_write: writes to a hw register
> >    * @reg:  register index
> > 
> > @@ -266,7 +275,14 @@ static inline void hw_write(struct ci_hdrc *ci, enum
> > ci_hw_regs reg,
> > 
> >   		data = (ioread32(ci->hw_bank.regmap[reg]) & ~mask)
> >   		
> >   			| (data & mask);
> > 
> > +#ifdef CONFIG_SOC_IMX28
> > +	if (ci->imx28_write_fix)
> > +		imx28_ci_writel(data, ci->hw_bank.regmap[reg]);
> > +	else
> > +		iowrite32(data, ci->hw_bank.regmap[reg]);
> > +#else
> > 
> >   	iowrite32(data, ci->hw_bank.regmap[reg]);
> > 
> > +#endif
> > 
> >   }
> >   
> >   /**
> > 
> > @@ -281,7 +297,14 @@ static inline u32 hw_test_and_clear(struct ci_hdrc
> > *ci, enum ci_hw_regs reg,
> > 
> >   {
> >   
> >   	u32 val = ioread32(ci->hw_bank.regmap[reg]) & mask;
> > 
> > +#ifdef CONFIG_SOC_IMX28
> > +	if (ci->imx28_write_fix)
> > +		imx28_ci_writel(val, ci->hw_bank.regmap[reg]);
> > +	else
> > +		iowrite32(val, ci->hw_bank.regmap[reg]);
> > +#else
> > 
> >   	iowrite32(val, ci->hw_bank.regmap[reg]);
> > 
> > +#endif
> > 
> >   	return val;
> >   
> >   }
> 
> Can't we remove the #ifdefs CONFIG_SOC_IMX28 all around?
> The check is done on the new flag ci->imx28_write_fix which exists for all
> platforms, isn't it?.

The SWP instruction is specific to ARM, so you'd need to stub-out the 
imx28_ci_writel() with ifdef then.
Greg Kroah-Hartman Oct. 29, 2013, 11:54 p.m. UTC | #3
On Mon, Oct 28, 2013 at 11:47:53AM +0100, Marek Vasut wrote:
> Dear Hector Palacios,
> 
> > Dear Peter,
> > 
> > On 10/25/2013 08:02 AM, Peter Chen wrote:
> > > According to Freescale imx28 Errata, "ENGR119653 USB: ARM to USB
> > > register error issue", All USB register write operations must
> > > use the ARM SWP instruction. So, we implement special hw_write
> > > and hw_test_and_clear for imx28.
> > > 
> > > Discussion for it at below:
> > > http://marc.info/?l=linux-usb&m=137996395529294&w=2
> > > 
> > > Signed-off-by: Peter Chen
> > > <peter.chen-KZfg59tc24xl57MIdRCFDg@public.gmane.org> ---
> > > Changes for v2:
> > > - Rebase to latest usb-next tree
> > > 
> > >   drivers/usb/chipidea/ci.h    |   23 +++++++++++++++++++++++
> > >   drivers/usb/chipidea/core.c  |    2 ++
> > >   drivers/usb/chipidea/host.c  |    1 +
> > >   include/linux/usb/chipidea.h |    1 +
> > >   4 files changed, 27 insertions(+), 0 deletions(-)
> > > 
> > > diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
> > > index 1c94fc5..4eb61d0 100644
> > > --- a/drivers/usb/chipidea/ci.h
> > > +++ b/drivers/usb/chipidea/ci.h
> > > @@ -173,6 +173,8 @@ struct ci_hdrc {
> > > 
> > >   	struct dentry			*debugfs;
> > >   	bool				id_event;
> > >   	bool				b_sess_valid_event;
> > > 
> > > +	/* imx28 needs swp instruction for writing */
> > > +	bool				imx28_write_fix;
> > > 
> > >   };
> > >   
> > >   static inline struct ci_role_driver *ci_role(struct ci_hdrc *ci)
> > > 
> > > @@ -253,6 +255,13 @@ static inline u32 hw_read(struct ci_hdrc *ci, enum
> > > ci_hw_regs reg, u32 mask)
> > > 
> > >   	return ioread32(ci->hw_bank.regmap[reg]) & mask;
> > >   
> > >   }
> > > 
> > > +#ifdef CONFIG_SOC_IMX28
> > > +static inline void imx28_ci_writel(u32 val32, volatile u32 *addr)
> > > +{
> > > +	__asm__ ("swp %0, %0, [%1]" : : "r"(val32), "r"(addr));
> > > +}
> > > +#endif
> > > +
> > > 
> > >   /**
> > >   
> > >    * hw_write: writes to a hw register
> > >    * @reg:  register index
> > > 
> > > @@ -266,7 +275,14 @@ static inline void hw_write(struct ci_hdrc *ci, enum
> > > ci_hw_regs reg,
> > > 
> > >   		data = (ioread32(ci->hw_bank.regmap[reg]) & ~mask)
> > >   		
> > >   			| (data & mask);
> > > 
> > > +#ifdef CONFIG_SOC_IMX28
> > > +	if (ci->imx28_write_fix)
> > > +		imx28_ci_writel(data, ci->hw_bank.regmap[reg]);
> > > +	else
> > > +		iowrite32(data, ci->hw_bank.regmap[reg]);
> > > +#else
> > > 
> > >   	iowrite32(data, ci->hw_bank.regmap[reg]);
> > > 
> > > +#endif
> > > 
> > >   }
> > >   
> > >   /**
> > > 
> > > @@ -281,7 +297,14 @@ static inline u32 hw_test_and_clear(struct ci_hdrc
> > > *ci, enum ci_hw_regs reg,
> > > 
> > >   {
> > >   
> > >   	u32 val = ioread32(ci->hw_bank.regmap[reg]) & mask;
> > > 
> > > +#ifdef CONFIG_SOC_IMX28
> > > +	if (ci->imx28_write_fix)
> > > +		imx28_ci_writel(val, ci->hw_bank.regmap[reg]);
> > > +	else
> > > +		iowrite32(val, ci->hw_bank.regmap[reg]);
> > > +#else
> > > 
> > >   	iowrite32(val, ci->hw_bank.regmap[reg]);
> > > 
> > > +#endif
> > > 
> > >   	return val;
> > >   
> > >   }
> > 
> > Can't we remove the #ifdefs CONFIG_SOC_IMX28 all around?
> > The check is done on the new flag ci->imx28_write_fix which exists for all
> > platforms, isn't it?.
> 
> The SWP instruction is specific to ARM, so you'd need to stub-out the 
> imx28_ci_writel() with ifdef then.

That's better than the mess of #ifdefs this patch adds, which isn't ok
at all :(
Peter Chen Oct. 30, 2013, 12:53 a.m. UTC | #4
On Tue, Oct 29, 2013 at 04:54:47PM -0700, gregkh@linuxfoundation.org wrote:
> On Mon, Oct 28, 2013 at 11:47:53AM +0100, Marek Vasut wrote:
> > Dear Hector Palacios,
> > 
> > > Dear Peter,
> > > 
> > > On 10/25/2013 08:02 AM, Peter Chen wrote:
> > > > According to Freescale imx28 Errata, "ENGR119653 USB: ARM to USB
> > > > register error issue", All USB register write operations must
> > > > use the ARM SWP instruction. So, we implement special hw_write
> > > > and hw_test_and_clear for imx28.
> > > > 
> > > > Discussion for it at below:
> > > > http://marc.info/?l=linux-usb&m=137996395529294&w=2
> > > > 
> > > > Signed-off-by: Peter Chen
> > > > <peter.chen-KZfg59tc24xl57MIdRCFDg@public.gmane.org> ---
> > > > Changes for v2:
> > > > - Rebase to latest usb-next tree
> > > > 
> > > >   drivers/usb/chipidea/ci.h    |   23 +++++++++++++++++++++++
> > > >   drivers/usb/chipidea/core.c  |    2 ++
> > > >   drivers/usb/chipidea/host.c  |    1 +
> > > >   include/linux/usb/chipidea.h |    1 +
> > > >   4 files changed, 27 insertions(+), 0 deletions(-)
> > > > 
> > > > diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
> > > > index 1c94fc5..4eb61d0 100644
> > > > --- a/drivers/usb/chipidea/ci.h
> > > > +++ b/drivers/usb/chipidea/ci.h
> > > > @@ -173,6 +173,8 @@ struct ci_hdrc {
> > > > 
> > > >   	struct dentry			*debugfs;
> > > >   	bool				id_event;
> > > >   	bool				b_sess_valid_event;
> > > > 
> > > > +	/* imx28 needs swp instruction for writing */
> > > > +	bool				imx28_write_fix;
> > > > 
> > > >   };
> > > >   
> > > >   static inline struct ci_role_driver *ci_role(struct ci_hdrc *ci)
> > > > 
> > > > @@ -253,6 +255,13 @@ static inline u32 hw_read(struct ci_hdrc *ci, enum
> > > > ci_hw_regs reg, u32 mask)
> > > > 
> > > >   	return ioread32(ci->hw_bank.regmap[reg]) & mask;
> > > >   
> > > >   }
> > > > 
> > > > +#ifdef CONFIG_SOC_IMX28
> > > > +static inline void imx28_ci_writel(u32 val32, volatile u32 *addr)
> > > > +{
> > > > +	__asm__ ("swp %0, %0, [%1]" : : "r"(val32), "r"(addr));
> > > > +}
> > > > +#endif
> > > > +
> > > > 
> > > >   /**
> > > >   
> > > >    * hw_write: writes to a hw register
> > > >    * @reg:  register index
> > > > 
> > > > @@ -266,7 +275,14 @@ static inline void hw_write(struct ci_hdrc *ci, enum
> > > > ci_hw_regs reg,
> > > > 
> > > >   		data = (ioread32(ci->hw_bank.regmap[reg]) & ~mask)
> > > >   		
> > > >   			| (data & mask);
> > > > 
> > > > +#ifdef CONFIG_SOC_IMX28
> > > > +	if (ci->imx28_write_fix)
> > > > +		imx28_ci_writel(data, ci->hw_bank.regmap[reg]);
> > > > +	else
> > > > +		iowrite32(data, ci->hw_bank.regmap[reg]);
> > > > +#else
> > > > 
> > > >   	iowrite32(data, ci->hw_bank.regmap[reg]);
> > > > 
> > > > +#endif
> > > > 
> > > >   }
> > > >   
> > > >   /**
> > > > 
> > > > @@ -281,7 +297,14 @@ static inline u32 hw_test_and_clear(struct ci_hdrc
> > > > *ci, enum ci_hw_regs reg,
> > > > 
> > > >   {
> > > >   
> > > >   	u32 val = ioread32(ci->hw_bank.regmap[reg]) & mask;
> > > > 
> > > > +#ifdef CONFIG_SOC_IMX28
> > > > +	if (ci->imx28_write_fix)
> > > > +		imx28_ci_writel(val, ci->hw_bank.regmap[reg]);
> > > > +	else
> > > > +		iowrite32(val, ci->hw_bank.regmap[reg]);
> > > > +#else
> > > > 
> > > >   	iowrite32(val, ci->hw_bank.regmap[reg]);
> > > > 
> > > > +#endif
> > > > 
> > > >   	return val;
> > > >   
> > > >   }
> > > 
> > > Can't we remove the #ifdefs CONFIG_SOC_IMX28 all around?
> > > The check is done on the new flag ci->imx28_write_fix which exists for all
> > > platforms, isn't it?.
> > 
> > The SWP instruction is specific to ARM, so you'd need to stub-out the 
> > imx28_ci_writel() with ifdef then.
> 
> That's better than the mess of #ifdefs this patch adds, which isn't ok
> at all :(
> 

You mean try to reduce the number of #ifdef?
Greg Kroah-Hartman Oct. 30, 2013, 3:09 a.m. UTC | #5
On Wed, Oct 30, 2013 at 08:53:42AM +0800, Peter Chen wrote:
> On Tue, Oct 29, 2013 at 04:54:47PM -0700, gregkh@linuxfoundation.org wrote:
> > On Mon, Oct 28, 2013 at 11:47:53AM +0100, Marek Vasut wrote:
> > > Dear Hector Palacios,
> > > 
> > > > Dear Peter,
> > > > 
> > > > On 10/25/2013 08:02 AM, Peter Chen wrote:
> > > > > According to Freescale imx28 Errata, "ENGR119653 USB: ARM to USB
> > > > > register error issue", All USB register write operations must
> > > > > use the ARM SWP instruction. So, we implement special hw_write
> > > > > and hw_test_and_clear for imx28.
> > > > > 
> > > > > Discussion for it at below:
> > > > > http://marc.info/?l=linux-usb&m=137996395529294&w=2
> > > > > 
> > > > > Signed-off-by: Peter Chen
> > > > > <peter.chen-KZfg59tc24xl57MIdRCFDg@public.gmane.org> ---
> > > > > Changes for v2:
> > > > > - Rebase to latest usb-next tree
> > > > > 
> > > > >   drivers/usb/chipidea/ci.h    |   23 +++++++++++++++++++++++
> > > > >   drivers/usb/chipidea/core.c  |    2 ++
> > > > >   drivers/usb/chipidea/host.c  |    1 +
> > > > >   include/linux/usb/chipidea.h |    1 +
> > > > >   4 files changed, 27 insertions(+), 0 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
> > > > > index 1c94fc5..4eb61d0 100644
> > > > > --- a/drivers/usb/chipidea/ci.h
> > > > > +++ b/drivers/usb/chipidea/ci.h
> > > > > @@ -173,6 +173,8 @@ struct ci_hdrc {
> > > > > 
> > > > >   	struct dentry			*debugfs;
> > > > >   	bool				id_event;
> > > > >   	bool				b_sess_valid_event;
> > > > > 
> > > > > +	/* imx28 needs swp instruction for writing */
> > > > > +	bool				imx28_write_fix;
> > > > > 
> > > > >   };
> > > > >   
> > > > >   static inline struct ci_role_driver *ci_role(struct ci_hdrc *ci)
> > > > > 
> > > > > @@ -253,6 +255,13 @@ static inline u32 hw_read(struct ci_hdrc *ci, enum
> > > > > ci_hw_regs reg, u32 mask)
> > > > > 
> > > > >   	return ioread32(ci->hw_bank.regmap[reg]) & mask;
> > > > >   
> > > > >   }
> > > > > 
> > > > > +#ifdef CONFIG_SOC_IMX28
> > > > > +static inline void imx28_ci_writel(u32 val32, volatile u32 *addr)
> > > > > +{
> > > > > +	__asm__ ("swp %0, %0, [%1]" : : "r"(val32), "r"(addr));
> > > > > +}
> > > > > +#endif
> > > > > +
> > > > > 
> > > > >   /**
> > > > >   
> > > > >    * hw_write: writes to a hw register
> > > > >    * @reg:  register index
> > > > > 
> > > > > @@ -266,7 +275,14 @@ static inline void hw_write(struct ci_hdrc *ci, enum
> > > > > ci_hw_regs reg,
> > > > > 
> > > > >   		data = (ioread32(ci->hw_bank.regmap[reg]) & ~mask)
> > > > >   		
> > > > >   			| (data & mask);
> > > > > 
> > > > > +#ifdef CONFIG_SOC_IMX28
> > > > > +	if (ci->imx28_write_fix)
> > > > > +		imx28_ci_writel(data, ci->hw_bank.regmap[reg]);
> > > > > +	else
> > > > > +		iowrite32(data, ci->hw_bank.regmap[reg]);
> > > > > +#else
> > > > > 
> > > > >   	iowrite32(data, ci->hw_bank.regmap[reg]);
> > > > > 
> > > > > +#endif
> > > > > 
> > > > >   }
> > > > >   
> > > > >   /**
> > > > > 
> > > > > @@ -281,7 +297,14 @@ static inline u32 hw_test_and_clear(struct ci_hdrc
> > > > > *ci, enum ci_hw_regs reg,
> > > > > 
> > > > >   {
> > > > >   
> > > > >   	u32 val = ioread32(ci->hw_bank.regmap[reg]) & mask;
> > > > > 
> > > > > +#ifdef CONFIG_SOC_IMX28
> > > > > +	if (ci->imx28_write_fix)
> > > > > +		imx28_ci_writel(val, ci->hw_bank.regmap[reg]);
> > > > > +	else
> > > > > +		iowrite32(val, ci->hw_bank.regmap[reg]);
> > > > > +#else
> > > > > 
> > > > >   	iowrite32(val, ci->hw_bank.regmap[reg]);
> > > > > 
> > > > > +#endif
> > > > > 
> > > > >   	return val;
> > > > >   
> > > > >   }
> > > > 
> > > > Can't we remove the #ifdefs CONFIG_SOC_IMX28 all around?
> > > > The check is done on the new flag ci->imx28_write_fix which exists for all
> > > > platforms, isn't it?.
> > > 
> > > The SWP instruction is specific to ARM, so you'd need to stub-out the 
> > > imx28_ci_writel() with ifdef then.
> > 
> > That's better than the mess of #ifdefs this patch adds, which isn't ok
> > at all :(
> > 
> 
> You mean try to reduce the number of #ifdef?

Of course you should.
diff mbox

Patch

diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
index 1c94fc5..4eb61d0 100644
--- a/drivers/usb/chipidea/ci.h
+++ b/drivers/usb/chipidea/ci.h
@@ -173,6 +173,8 @@  struct ci_hdrc {
 	struct dentry			*debugfs;
 	bool				id_event;
 	bool				b_sess_valid_event;
+	/* imx28 needs swp instruction for writing */
+	bool				imx28_write_fix;
 };
 
 static inline struct ci_role_driver *ci_role(struct ci_hdrc *ci)
@@ -253,6 +255,13 @@  static inline u32 hw_read(struct ci_hdrc *ci, enum ci_hw_regs reg, u32 mask)
 	return ioread32(ci->hw_bank.regmap[reg]) & mask;
 }
 
+#ifdef CONFIG_SOC_IMX28
+static inline void imx28_ci_writel(u32 val32, volatile u32 *addr)
+{
+	__asm__ ("swp %0, %0, [%1]" : : "r"(val32), "r"(addr));
+}
+#endif
+
 /**
  * hw_write: writes to a hw register
  * @reg:  register index
@@ -266,7 +275,14 @@  static inline void hw_write(struct ci_hdrc *ci, enum ci_hw_regs reg,
 		data = (ioread32(ci->hw_bank.regmap[reg]) & ~mask)
 			| (data & mask);
 
+#ifdef CONFIG_SOC_IMX28
+	if (ci->imx28_write_fix)
+		imx28_ci_writel(data, ci->hw_bank.regmap[reg]);
+	else
+		iowrite32(data, ci->hw_bank.regmap[reg]);
+#else
 	iowrite32(data, ci->hw_bank.regmap[reg]);
+#endif
 }
 
 /**
@@ -281,7 +297,14 @@  static inline u32 hw_test_and_clear(struct ci_hdrc *ci, enum ci_hw_regs reg,
 {
 	u32 val = ioread32(ci->hw_bank.regmap[reg]) & mask;
 
+#ifdef CONFIG_SOC_IMX28
+	if (ci->imx28_write_fix)
+		imx28_ci_writel(val, ci->hw_bank.regmap[reg]);
+	else
+		iowrite32(val, ci->hw_bank.regmap[reg]);
+#else
 	iowrite32(val, ci->hw_bank.regmap[reg]);
+#endif
 	return val;
 }
 
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 06204b7..357a059 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -551,6 +551,8 @@  static int ci_hdrc_probe(struct platform_device *pdev)
 
 	ci->dev = dev;
 	ci->platdata = dev->platform_data;
+	ci->imx28_write_fix = !!(ci->platdata->flags &
+		CI_HDRC_IMX28_WRITE_FIX);
 
 	ret = hw_device_init(ci, base);
 	if (ret < 0) {
diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c
index 59e6020..06fd042 100644
--- a/drivers/usb/chipidea/host.c
+++ b/drivers/usb/chipidea/host.c
@@ -65,6 +65,7 @@  static int host_start(struct ci_hdrc *ci)
 	ehci->caps = ci->hw_bank.cap;
 	ehci->has_hostpc = ci->hw_bank.lpm;
 	ehci->has_tdi_phy_lpm = ci->hw_bank.lpm;
+	ehci->imx28_write_fix = ci->imx28_write_fix;
 
 	if (ci->platdata->reg_vbus) {
 		ret = regulator_enable(ci->platdata->reg_vbus);
diff --git a/include/linux/usb/chipidea.h b/include/linux/usb/chipidea.h
index 7d39967..708bd11 100644
--- a/include/linux/usb/chipidea.h
+++ b/include/linux/usb/chipidea.h
@@ -24,6 +24,7 @@  struct ci_hdrc_platform_data {
 	 * but otg is not supported (no register otgsc).
 	 */
 #define CI_HDRC_DUAL_ROLE_NOT_OTG	BIT(4)
+#define CI_HDRC_IMX28_WRITE_FIX		BIT(5)
 	enum usb_dr_mode	dr_mode;
 #define CI_HDRC_CONTROLLER_RESET_EVENT		0
 #define CI_HDRC_CONTROLLER_STOPPED_EVENT	1