Re: [PATCH 4/8] usb: ehci: add freescale imx28 special write register method

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Jan 08, 2014 at 06:25:01AM -0800, Greg KH wrote:
> On Wed, Jan 08, 2014 at 12:54:25PM +0100, Marc Kleine-Budde wrote:
> > On 01/08/2014 01:23 AM, Greg KH wrote:
> > > On Mon, Jan 06, 2014 at 10:10:41AM +0800, 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 a special ehci_write
> > >> for imx28.
> > >>
> > >> Discussion for it at below:
> > >> http://marc.info/?l=linux-usb&m=137996395529294&w=2
> > >>
> > >> Signed-off-by: Peter Chen <peter.chen@xxxxxxxxxxxxx>
> > >> Acked-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
> > >> Signed-off-by: Marc Kleine-Budde <mkl@xxxxxxxxxxxxxx>
> > >> Tested-by: Marc Kleine-Budde <mkl@xxxxxxxxxxxxxx>
> > >> ---
> > >>  drivers/usb/host/ehci.h |   18 +++++++++++++++++-
> > >>  1 files changed, 17 insertions(+), 1 deletions(-)
> > >>
> > >> diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
> > >> index c35a6e2b..e26099b 100644
> > >> --- a/drivers/usb/host/ehci.h
> > >> +++ b/drivers/usb/host/ehci.h
> > >> @@ -225,6 +225,7 @@ struct ehci_hcd {			/* one per controller */
> > >>  	unsigned		has_synopsys_hc_bug:1; /* Synopsys HC */
> > >>  	unsigned		frame_index_bug:1; /* MosChip (AKA NetMos) */
> > >>  	unsigned		need_oc_pp_cycle:1; /* MPC834X port power */
> > >> +	unsigned		imx28_write_fix:1; /* For Freescale i.MX28 */
> > >>  
> > >>  	/* required for usb32 quirk */
> > >>  	#define OHCI_CTRL_HCFS          (3 << 6)
> > >> @@ -728,6 +729,18 @@ static inline unsigned int ehci_readl(const struct ehci_hcd *ehci,
> > >>  #endif
> > >>  }
> > >>  
> > >> +#ifdef CONFIG_SOC_IMX28
> > >> +static inline void imx28_ehci_writel(const unsigned int val,
> > >> +		volatile __u32 __iomem *addr)
> > >> +{
> > >> +	__asm__ ("swp %0, %0, [%1]" : : "r"(val), "r"(addr));
> > >> +}
> > >> +#else
> > >> +static inline void imx28_ehci_writel(const unsigned int val,
> > >> +		volatile __u32 __iomem *addr)
> > >> +{
> > >> +}
> > >> +#endif
> > >>  static inline void ehci_writel(const struct ehci_hcd *ehci,
> > >>  		const unsigned int val, __u32 __iomem *regs)
> > >>  {
> > >> @@ -736,7 +749,10 @@ static inline void ehci_writel(const struct ehci_hcd *ehci,
> > >>  		writel_be(val, regs) :
> > >>  		writel(val, regs);
> > >>  #else
> > >> -	writel(val, regs);
> > >> +	if (IS_ENABLED(CONFIG_SOC_IMX28) && ehci->imx28_write_fix)
> > >> +		imx28_ehci_writel(val, regs);
> > >> +	else
> > >> +		writel(val, regs);
> > >>  #endif
> > > 
> > > This IS_ENABLED() isn't needed at all, so please remove it.
> > 
> > It's an optimisation for the hot path, if kernel isn't build for a mx28
> > the newly added if() is completely optimized out. The same argument
> > applies to the next patch.
> 
> If the kernel isn't built for mx28 then the if statment goes away
> without the IS_ENABLED() call as well, due to the empty inline
> function, right?
> 

With IS_ENABLED(), the non-imx28 platform don't need to judge the
condition of ehci->imx28_write_fix, if you don't think we need
to care one or two instructions for every other platforms, I can
delete it.

-- 

Best Regards,
Peter Chen

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux