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.c...@freescale.com>
> > >> Acked-by: Alan Stern <st...@rowland.harvard.edu>
> > >> Signed-off-by: Marc Kleine-Budde <m...@pengutronix.de>
> > >> Tested-by: Marc Kleine-Budde <m...@pengutronix.de>
> > >> ---
> > >>  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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to