> From: Peter Chen
...
> > > >> +#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

If you replace the above with:
#define imx28_ehci_writel(v, a) writel(v, a)

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

I think the compiler might optimise out the conditional (because the
then and else clauses are identical.

Also the earlier #if - for big-endian - can be removed.
The compiler already collapses it down.

        David



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