Re: [RFC PATCH 1/2] usb: ehci: Add option to override ehci_readl()/ehci_writel()

[Alan Stern]

On Mon, Jul 13, 2026 at 09:22:04PM +0900, Daniel Palmer wrote:
> From: Daniel Palmer <daniel@xxxxxxxx>
> 
> There are EHCI-like hosts, like fotg210, out there that should
> be able to use the shared ehci code but can't because they need
> to override some parts of the ehci code to work properly.
> 
> Let ehci_readl() and ehci_writel() be overridden first.
> This allows the ehci code to be used with hosts that need
> to do special dances when accessing their registers.
> 
> The change is hidden behind a boolean kconfig option
> so that the extra function pointer NULL check and call
> only happens when a broken controller is enabled.
> 
> Signed-off-by: Daniel Palmer <daniel@xxxxxxxx>
> ---
>  drivers/usb/host/Kconfig |  3 +++
>  drivers/usb/host/ehci.h  | 16 ++++++++++++++++
>  2 files changed, 19 insertions(+)
> 
> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
> index b3b1ec696bf5..8c88354e91e5 100644
> --- a/drivers/usb/host/Kconfig
> +++ b/drivers/usb/host/Kconfig
> @@ -362,6 +362,9 @@ config USB_OCTEON_EHCI
>  	  USB 2.0 device support.  All CN6XXX based chips with USB are
>  	  supported.
>  
> +config HAVE_BROKEN_EHCI_HCD
> +	bool

That is not a good choice of name; it's far too generic.  EHCI 
controllers can be broken in so many different ways...

How about USB_EHCI_MMIO_OVERRIDES instead?

Also, the way this is implemented below implicitly assumes that the new 
CONFIG flag will never be set at the same time as 
USB_EHCI_BIG_ENDIAN_MMIO.  That should be enforced here.  Even though 
IMX28 doesn't do it, someone else might do it in the future.

> +
>  endif # USB_EHCI_HCD
>  
>  config USB_OXU210HP_HCD
> diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
> index d7a3c8d13f6b..f592cc26e494 100644
> --- a/drivers/usb/host/ehci.h
> +++ b/drivers/usb/host/ehci.h
> @@ -258,6 +258,14 @@ struct ehci_hcd {			/* one per controller */
>  						/* us budgeted per uframe */
>  	struct list_head	tt_list;
>  
> +#ifdef CONFIG_HAVE_BROKEN_EHCI_HCD
> +	/* Overrides to fix up broken implementations */
> +	/* Broken IO */

Similarly, the comment should be improved.  "Overrides to fix up broken 
MMIO implementations".

Alan Stern

> +	void (*ehci_writel)(const struct ehci_hcd *ehci,
> +			    const unsigned int val, __u32 __iomem *regs);
> +	unsigned int (*ehci_readl)(const struct ehci_hcd *ehci, __u32 __iomem *regs);
> +#endif
> +
>  	/* platform-specific data -- must come last */
>  	unsigned long		priv[] __aligned(sizeof(s64));
>  };
> @@ -762,6 +770,10 @@ static inline unsigned int ehci_readl(const struct ehci_hcd *ehci,
>  		readl_be(regs) :
>  		readl(regs);
>  #else
> +#ifdef CONFIG_HAVE_BROKEN_EHCI_HCD
> +	if (ehci->ehci_readl)
> +		return ehci->ehci_readl(ehci, regs);
> +#endif
>  	return readl(regs);
>  #endif
>  }
> @@ -788,6 +800,10 @@ static inline void ehci_writel(const struct ehci_hcd *ehci,
>  #else
>  	if (ehci->imx28_write_fix)
>  		imx28_ehci_writel(val, regs);
> +#ifdef CONFIG_HAVE_BROKEN_EHCI_HCD
> +	else if (ehci->ehci_writel)
> +		ehci->ehci_writel(ehci, val, regs);
> +#endif
>  	else
>  		writel(val, regs);
>  #endif
> -- 
> 2.53.0
>