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 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?

thanks,

greg k-h
--
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