Re: usb: dwc2: regression on MyBook Live Duo / Canyonlands since 4.3.0-rc4

[Benjamin Herrenschmidt]

On Mon, 2016-05-09 at 12:36 +0200, Arnd Bergmann wrote:
>Â
> I think we can simply make this set of accessors architecture-
> dependent
> (MIPS vs. the rest of the world) to revert ARM and PowerPC back to
> the working version.

Or use writel_be which mips seems to support...

Really, make it a BE vs. LE device test is a much better solution.

For now, sinceÂdwc2_readl() and writel don't take the device as an
argument, you can make it a function of a compile time #define, or
maybe a driver global, but the right way is really something like

	if (device_is_be())
		return readl_be(...)
	else
		return readl(...)

With the device_is_be() being temporarily set to true for MIPS for
example, and later, a second pass, add the device argument and make it
a device-flag initialized from the probe routine, possibly from the DT.

Cheers,
Ben.

> Signed-off-by: Arnd Bergmann <arnd@xxxxxxxx>
> 
> diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
> index 3c58d633ce80..1f8ed149a40f 100644
> --- a/drivers/usb/dwc2/core.h
> +++ b/drivers/usb/dwc2/core.h
> @@ -64,12 +64,24 @@
> Â	DWC2_TRACE_SCHEDULER_VB(pr_fmt("%s: SCH: " fmt),		
> \
> Â				dev_name(hsotg->dev), ##__VA_ARGS__)
> Â
> +
> +#ifdef CONFIG_MIPS
> +/*
> + * There are some MIPS machines that can run in either big-endian
> + * or little-endian mode and that use the dwc2 register without
> + * a byteswap in both ways.
> + * Unlike other architectures, MIPS does not require a barrier
> + * before the __raw_writel() to synchronize with DMA but does
> + * require the barrier after the writel() to serialize a series
> + * of writes. This set of operations was added specifically for
> + * MIPS and should only be used there.
> + */
> Âstatic inline u32 dwc2_readl(const void __iomem *addr)
> Â{
> Â	u32 value = __raw_readl(addr);
> Â
> -	/* In order to preserve endianness __raw_* operation is
> used. Therefore
> -	Â* a barrier is needed to ensure IO access is not re-ordered 
> across
> +	/* in order to preserve endianness __raw_* operation is
> used. therefore
> +	Â* a barrier is needed to ensure io access is not re-ordered 
> across
> Â	Â* reads or writes
> Â	Â*/
> Â	mb();
> @@ -81,15 +93,32 @@ static inline void dwc2_writel(u32 value, void
> __iomem *addr)
> Â	__raw_writel(value, addr);
> Â
> Â	/*
> -	Â* In order to preserve endianness __raw_* operation is
> used. Therefore
> -	Â* a barrier is needed to ensure IO access is not re-ordered 
> across
> +	Â* in order to preserve endianness __raw_* operation is
> used. therefore
> +	Â* a barrier is needed to ensure io access is not re-ordered 
> across
> Â	Â* reads or writes
> Â	Â*/
> Â	mb();
> -#ifdef DWC2_LOG_WRITES
> -	pr_info("INFO:: wrote %08x to %p\n", value, addr);
> +#ifdef dwc2_log_writes
> +	pr_info("info:: wrote %08x to %p\n", value, addr);
> Â#endif
> Â}
> +#else
> +/* Normal architectures just use readl/write */
> +static inline u32 dwc2_readl(const void __iomem *addr)
> +{
> +	u32 value = readl(addr);
> +	return value;
> +}
> +
> +static inline void dwc2_writel(u32 value, void __iomem *addr)
> +{
> +	writel(value, addr);
> +
> +#ifdef dwc2_log_writes
> +	pr_info("info:: wrote %08x to %p\n", value, addr);
> +#endif
> +}
> +#endif
> Â
> Â/* Maximum number of Endpoints/HostChannels */
> Â#define MAX_EPS_CHANNELS	16