Re: [RFC PATCH 2/2] usb: chipidea: use the generic ehci_writel override for i.MX28

[Peter Chen <peter.chen@kernel.org>]

On 26-07-13 21:22:05, Daniel Palmer wrote:
> From: Daniel Palmer <daniel@0x0f.com>
> 
> The EHCI core carries an imx28_write_fix flag baked into ehci_writel()
> that special-cases the i.MX28's need for a swp instruction when writing
> the EHCI registers.
> 
> Now that the EHCI core provides a generic ehci_writel override hook,
> convert the chipidea host glue to install its own writel via that hook
> and drop the i.MX28 special case from the core, keeping ehci_writel()
> free of SoC-specific knowledge.
> 
> Signed-off-by: Daniel Palmer <daniel@0x0f.com>
> ---
>  drivers/usb/chipidea/Kconfig |  1 +
>  drivers/usb/chipidea/host.c  | 17 ++++++++++++++++-
>  drivers/usb/host/ehci.h      | 19 ++-----------------
>  3 files changed, 19 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/usb/chipidea/Kconfig b/drivers/usb/chipidea/Kconfig
> index bab45bc62361..5bada84943e2 100644
> --- a/drivers/usb/chipidea/Kconfig
> +++ b/drivers/usb/chipidea/Kconfig
> @@ -29,6 +29,7 @@ config USB_CHIPIDEA_HOST
>  	bool "ChipIdea host controller"
>  	depends on USB_EHCI_HCD
>  	select USB_EHCI_ROOT_HUB_TT
> +	select HAVE_BROKEN_EHCI_HCD if SOC_IMX28

Is the SoC you are trying adding support imx28? If it is not, I could not
understand why you add this patch.

Peter

>  	help
>  	  Say Y here to enable host controller functionality of the
>  	  ChipIdea driver.
> diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c
> index ced6076a8248..820bdea5da12 100644
> --- a/drivers/usb/chipidea/host.c
> +++ b/drivers/usb/chipidea/host.c
> @@ -117,6 +117,18 @@ static irqreturn_t host_irq(struct ci_hdrc *ci)
>  	return usb_hcd_irq(ci->irq, ci->hcd);
>  }
>  
> +#ifdef CONFIG_SOC_IMX28
> +/*
> + * The i.MX28 needs a swp instruction rather than a normal write to the
> + * EHCI registers; provide it through the generic ehci_writel override.
> + */
> +static void ci_hdrc_ehci_writel(const struct ehci_hcd *ehci,
> +				const unsigned int val, __u32 __iomem *regs)
> +{
> +	asm("swp %0, %0, [%1]" : : "r"(val), "r"(regs));
> +}
> +#endif
> +
>  static int host_start(struct ci_hdrc *ci)
>  {
>  	struct usb_hcd *hcd;
> @@ -150,7 +162,10 @@ 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;
> +#ifdef CONFIG_SOC_IMX28
> +	if (ci->imx28_write_fix)
> +		ehci->ehci_writel = ci_hdrc_ehci_writel;
> +#endif
>  	ehci->has_ci_pec_bug = ci->has_portsc_pec_bug;
>  
>  	priv = (struct ehci_ci_priv *)ehci->priv;
> diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
> index f592cc26e494..616b01314bfe 100644
> --- a/drivers/usb/host/ehci.h
> +++ b/drivers/usb/host/ehci.h
> @@ -218,7 +218,6 @@ 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 */
>  	unsigned		spurious_oc:1;
>  	unsigned		is_aspeed:1;
>  	unsigned		zx_wakeup_clear_needed:1;
> @@ -778,18 +777,6 @@ 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)
>  {
> @@ -798,13 +785,11 @@ static inline void ehci_writel(const struct ehci_hcd *ehci,
>  		writel_be(val, regs) :
>  		writel(val, regs);
>  #else
> -	if (ehci->imx28_write_fix)
> -		imx28_ehci_writel(val, regs);
>  #ifdef CONFIG_HAVE_BROKEN_EHCI_HCD
> -	else if (ehci->ehci_writel)
> +	if (ehci->ehci_writel)
>  		ehci->ehci_writel(ehci, val, regs);
> -#endif
>  	else
> +#endif
>  		writel(val, regs);
>  #endif
>  }
> -- 
> 2.53.0
> 

-- 

Thanks,
Peter Chen