Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1396415 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: usb: dwc2: regression on MyBook Live Duo / Canyonlands since 4.3.0-rc4 Christian Lamparter <chunkeey@googlemail.com> - 2016-05-08 13:50 +0200
    Re: usb: dwc2: regression on MyBook Live Duo / Canyonlands since  4.3.0-rc4 Benjamin Herrenschmidt <benh@au1.ibm.com> - 2016-05-09 02:30 +0200
      Re: usb: dwc2: regression on MyBook Live Duo / Canyonlands since 4.3.0-rc4 Arnd Bergmann <arnd@arndb.de> - 2016-05-09 12:40 +0200
        Re: usb: dwc2: regression on MyBook Live Duo / Canyonlands since 4.3.0-rc4 Felipe Balbi <felipe.balbi@linux.intel.com> - 2016-05-09 12:50 +0200
          Re: usb: dwc2: regression on MyBook Live Duo / Canyonlands since 4.3.0-rc4 Arnd Bergmann <arnd@arndb.de> - 2016-05-09 17:10 +0200
            Re: usb: dwc2: regression on MyBook Live Duo / Canyonlands since 4.3.0-rc4 Arnd Bergmann <arnd@arndb.de> - 2016-05-09 22:20 +0200
            Re: usb: dwc2: regression on MyBook Live Duo / Canyonlands since  4.3.0-rc4 Benjamin Herrenschmidt <benh@kernel.crashing.org> - 2016-05-10 00:40 +0200
              Re: usb: dwc2: regression on MyBook Live Duo / Canyonlands since 4.3.0-rc4 Arnd Bergmann <arnd@arndb.de> - 2016-05-10 09:30 +0200
                Re: usb: dwc2: regression on MyBook Live Duo / Canyonlands since 4.3.0-rc4 Christian Lamparter <chunkeey@googlemail.com> - 2016-05-12 12:00 +0200
                  Re: usb: dwc2: regression on MyBook Live Duo / Canyonlands since 4.3.0-rc4 Arnd Bergmann <arnd@arndb.de> - 2016-05-12 14:00 +0200
                    Re: usb: dwc2: regression on MyBook Live Duo / Canyonlands since 4.3.0-rc4 Christian Lamparter <chunkeey@googlemail.com> - 2016-05-12 15:40 +0200
                      Re: usb: dwc2: regression on MyBook Live Duo / Canyonlands since  4.3.0-rc4 John Youn <John.Youn@synopsys.com> - 2016-05-12 20:50 +0200
                        Re: usb: dwc2: regression on MyBook Live Duo / Canyonlands since 4.3.0-rc4 Christian Lamparter <chunkeey@googlemail.com> - 2016-05-12 22:40 +0200
                          Re: usb: dwc2: regression on MyBook Live Duo / Canyonlands since 4.3.0-rc4 Arnd Bergmann <arnd@arndb.de> - 2016-05-12 23:00 +0200
                          Re: usb: dwc2: regression on MyBook Live Duo / Canyonlands since  4.3.0-rc4 John Youn <John.Youn@synopsys.com> - 2016-05-12 23:00 +0200
                        Re: usb: dwc2: regression on MyBook Live Duo / Canyonlands since 4.3.0-rc4 Arnd Bergmann <arnd@arndb.de> - 2016-05-14 21:50 +0200
                        Re: usb: dwc2: regression on MyBook Live Duo / Canyonlands since  4.3.0-rc4 John Youn <John.Youn@synopsys.com> - 2016-05-18 02:00 +0200
                          Re: usb: dwc2: regression on MyBook Live Duo / Canyonlands since 4.3.0-rc4 Christian Lamparter <chunkeey@googlemail.com> - 2016-05-18 21:20 +0200
                            Re: usb: dwc2: regression on MyBook Live Duo / Canyonlands since 4.3.0-rc4 Arnd Bergmann <arnd@arndb.de> - 2016-05-18 23:10 +0200
                            Re: usb: dwc2: regression on MyBook Live Duo / Canyonlands since  4.3.0-rc4 John Youn <John.Youn@synopsys.com> - 2016-05-19 02:40 +0200
                  Re: usb: dwc2: regression on MyBook Live Duo / Canyonlands since  4.3.0-rc4 Benjamin Herrenschmidt <benh@kernel.crashing.org> - 2016-05-13 00:20 +0200
            Re: usb: dwc2: regression on MyBook Live Duo / Canyonlands since  4.3.0-rc4 Benjamin Herrenschmidt <benh@kernel.crashing.org> - 2016-05-10 00:50 +0200
          Re: usb: dwc2: regression on MyBook Live Duo / Canyonlands since  4.3.0-rc4 Benjamin Herrenschmidt <benh@kernel.crashing.org> - 2016-05-10 00:40 +0200
        Re: usb: dwc2: regression on MyBook Live Duo / Canyonlands since  4.3.0-rc4 Benjamin Herrenschmidt <benh@au1.ibm.com> - 2016-05-09 16:10 +0200
        Re: usb: dwc2: regression on MyBook Live Duo / Canyonlands since  4.3.0-rc4 John Youn <John.Youn@synopsys.com> - 2016-05-09 22:30 +0200
          Re: usb: dwc2: regression on MyBook Live Duo / Canyonlands since 4.3.0-rc4 Arnd Bergmann <arnd@arndb.de> - 2016-05-09 22:40 +0200
            Re: usb: dwc2: regression on MyBook Live Duo / Canyonlands since  4.3.0-rc4 John Youn <John.Youn@synopsys.com> - 2016-05-09 23:20 +0200
              Re: usb: dwc2: regression on MyBook Live Duo / Canyonlands since 4.3.0-rc4 Arnd Bergmann <arnd@arndb.de> - 2016-05-09 23:40 +0200

Page 2 of 2 — ← Prev page 1 [2]

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

On Thu, 2016-05-12 at 11:58 +0200, Christian Lamparter wrote:
> 
> > http://www.linuxplumbersconf.org/2012/wp-content/uploads/2012/09/2012-lpc-ref-big-little-endian-herrenschmidt.odp
> > 
> > but there are at least two more twists that you completely missed here:
> > 
> > - Some architectures (e.g. ARMv5 "BE32" mode in IXP4xx, surely some others)
> >   do not implement big-endian mode by wiring up the data lines between the
> >   bus and the CPU differently between big- and little-endian mode like
> >   powerpc and armv7 "BE8" do, but instead they swizzle the *address* lines
> >   on 8-bit and 16-bit addresses. The effect of that is that normal RAM
> >   accesses work as expected both ways, and devices that are accessed using
> >   32-bit MMIO ops never need any byteswap (you actually get "native
> >   endian") while MMIO with 8 and 16 bit width does something completely
> >   unexpected and touches the wrong register. Having an explicit byteswap
> >   on the PCI host bridge gets you the expected addresses again for 8-bit
> >   cycles but it also means that readl()/writel() again need to swap the
> >   data.

Right. Old PowerPC did that too and it's completely stupid. Thankfully
most vendors grew a clue since then and this practice has slowly fallen
into oblivion.

> > - Some other architectures (e.g. Broadcom MIPS) apparently are even fancier
> >   and use a strapping pin on the SoC flips the endianess of the CPU core
> >   at the same time as all the peripheral MMIO registers, with the intention
> >   of never requiring any byte swaps. I believe they are implemented careful
> >   enough to actually get this right, but it confuses the heck out of
> >   Linux drivers that don't expect this.

Right. Drivers like that will need an explicit test in the accessors.

Cheers,
Ben.

[toc] | [prev] | [next] | [standalone]

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

On Mon, 2016-05-09 at 21:06 +0200, Christian Lamparter via Linuxppc-dev wrote:
> 
> I ran into the following issues:
>         - gadget.c uses ioread32_rep [0] & iowrite32_rep [1].
>           This is interesting because both of these functions actually use
>           the __raw_io* on powerpc. This is because powerpc uses the default
>           defines of include/asm-generic/io.h [2].
> 
>           Ideally, this should be done by sth like a writesl_be or writesl(e)
>           function. But I found none so for now: Let's make a ugly hack:
>           to_correct_endian that will work for testing, but will be replaced.

No. The _rep variants always must use native endian. If for some reason
your device requires something different then the device is more broken
than I thought. IE. even with a BE core and an LE device, we use non-
byeswap _rep versions, this isn't just because we use "defaults" on
powerpc for example, it's necessary to work.

Here too, I could suggest you google for my talk on the subject :-) But
if you think closely about it, you'll figure out that FIFOs don't have
an endian per-se, thus what matters is which byte is first in ascending
address order, and that byte must land in memory in the first address
as well.

Interpreting the resulting data might require endian swaps, but
actually transferring to/from the fifo doesn't.

>    - dwc2_readl, dwc2_writel. I have no insight in the craziness that's
>           going on with MIPS and the memory barrier. But it got me thinking
>           rather than CONFIG_MIPS, isn't there a umbrella
>           "ARCH_HAS_NEED_MEMORY_BARRIER_FOR_MMIO" config symbol?
> 
>         - is_little_endian (do we want a separate is_big_endian?)
>           Also, do we want to be able to overwrite the detection code
>           if the endian setting was set in the device tree?. For now
>           it always does auto detection (see dwc2_detect_endiannes() ).

If auto-detect always work, no need to bother with the device-tree. A
single flag is enough, either is_big or is_little, doesn't matter.

"readl" should always be little, readl_be should always be big, though
the latter isn't supported on all archs. However the new accessors in
iomap.h should provide a "be" variant on all archs so switching to
these might be a good idea.

>         ( - 80 character per line issues, is it possible to drop the
>                  hsotg->reg + REGISTER from the dwc2_readl/writel since we
>                  pass the hsotg now anyway and do the reg + REGISTER
>                  calculation in the accessor? I played around with macros
>                  as most functions calling the accessors have the hsotg
>                  variable anyway )

Or just use a temp variable, the compiler should deal with it the same
way.

Ben.
 
> Regards,
> Christian

[toc] | [prev] | [next] | [standalone]

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

On Mon, 2016-05-09 at 13:39 +0300, Felipe Balbi wrote:
> and patch all drivers similarly? Shouldn't arch/mips itself deal with
> it and hide it from drivers ?

Not sure what you mean, but we never had "endian neutral" accessors. It
would be a bit of an endeavour and we already have so many accessors
that adding more need a very strong justification.

Most IP blocks have a fixed endian...

Ben.

[toc] | [prev] | [next] | [standalone]

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

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@arndb.de>
> 
> 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

[toc] | [prev] | [next] | [standalone]

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

On 5/9/2016 3:36 AM, Arnd Bergmann wrote:
> On Monday 09 May 2016 10:23:22 Benjamin Herrenschmidt wrote:
>> On Sun, 2016-05-08 at 13:44 +0200, Christian Lamparter wrote:
>>> On Sunday, May 08, 2016 08:40:55 PM Benjamin Herrenschmidt wrote:
>>>>
>>>> On Sun, 2016-05-08 at 00:54 +0200, Christian Lamparter via Linuxppc-dev 
>>>> wrote:
>>>>>
>>>>> I've been looking in getting the MyBook Live Duo's USB OTG port
>>>>> to function. The SoC is a APM82181. Which has a PowerPC 464 core
>>>>> and related to the supported canyonlands architecture in
>>>>> arch/powerpc/.
>>>>>
>>>>> Currently in -next the dwc2 module doesn't load: 
>>>> Smells like the APM implementation is little endian. You might need to
>>>> use a flag to indicate what endian to use instead and set it
>>>> appropriately based on some DT properties.
>>> I tried. As per common-properties[0], I added little-endian; but it has no
>>> effect. I looked in dwc2_driver_probe and found no way of specifying the
>>> endian of the device. It all comes down to the dwc2_readl & dwc2_writel
>>> accessors. These - sadly - have been hardwired to use __raw_readl and
>>> __raw_writel. So, it's always "native-endian". While common-properties
>>> says little-endian should be preferred.
>>
>> Right, I meant, you should produce a patch adding a runtime test inside
>> those functions based on a device-tree property, a bit like we do for
>> some of the HCDs like OHCI, EHCI etc...
>>
>>
> 
> The patch that caused the problem had multiple issues:
> 
> - it broke big-endian ARM kernels: any machine that was working
>   correctly with a little-endian kernel is no longer using byteswaps
>   on big-endian kernels, which clearly breaks them.


I'm a bit confused about how this is supposed to work. My
understanding was that the readl() and writel() are defined as little
endian. So byte-swapping was performed if the architecture is big
endian. And the raw versions never swapped, always using the "native"
endianness.

dwc2 is always treating the result of readl/writel as if it was read
in native endian. So it needs to read the registers in big-endian on
big-endian systems.

This was the premise on which this patch was made.

So for big endian systems, isn't what we want is to read in big-endian
without any byteswapping to little-endian? But your saying this breaks
big-endian ARM systems as well. Am I missing something?

The rest of the feedback makes sense. Just confused about this bit.

Regards,
John

[toc] | [prev] | [next] | [standalone]

#1397374

On Monday 09 May 2016 13:22:48 John Youn wrote:
> On 5/9/2016 3:36 AM, Arnd Bergmann wrote:
> > On Monday 09 May 2016 10:23:22 Benjamin Herrenschmidt wrote:
> >> On Sun, 2016-05-08 at 13:44 +0200, Christian Lamparter wrote:
> >>> On Sunday, May 08, 2016 08:40:55 PM Benjamin Herrenschmidt wrote:
> >>>>
> >>>> On Sun, 2016-05-08 at 00:54 +0200, Christian Lamparter via Linuxppc-dev 
> >>>> wrote:
> >>>>>
> >>>>> I've been looking in getting the MyBook Live Duo's USB OTG port
> >>>>> to function. The SoC is a APM82181. Which has a PowerPC 464 core
> >>>>> and related to the supported canyonlands architecture in
> >>>>> arch/powerpc/.
> >>>>>
> >>>>> Currently in -next the dwc2 module doesn't load: 
> >>>> Smells like the APM implementation is little endian. You might need to
> >>>> use a flag to indicate what endian to use instead and set it
> >>>> appropriately based on some DT properties.
> >>> I tried. As per common-properties[0], I added little-endian; but it has no
> >>> effect. I looked in dwc2_driver_probe and found no way of specifying the
> >>> endian of the device. It all comes down to the dwc2_readl & dwc2_writel
> >>> accessors. These - sadly - have been hardwired to use __raw_readl and
> >>> __raw_writel. So, it's always "native-endian". While common-properties
> >>> says little-endian should be preferred.
> >>
> >> Right, I meant, you should produce a patch adding a runtime test inside
> >> those functions based on a device-tree property, a bit like we do for
> >> some of the HCDs like OHCI, EHCI etc...
> >>
> >>
> > 
> > The patch that caused the problem had multiple issues:
> > 
> > - it broke big-endian ARM kernels: any machine that was working
> >   correctly with a little-endian kernel is no longer using byteswaps
> >   on big-endian kernels, which clearly breaks them.
> 
> 
> I'm a bit confused about how this is supposed to work. My
> understanding was that the readl() and writel() are defined as little
> endian. So byte-swapping was performed if the architecture is big
> endian. And the raw versions never swapped, always using the "native"
> endianness.
> 
> dwc2 is always treating the result of readl/writel as if it was read
> in native endian. So it needs to read the registers in big-endian on
> big-endian systems.

The hardware has no idea of what endianess the CPU uses at any
given time, it's fixed by the SoC design, so there is no such
thing as "native" endianess for a random IP block.

The readl/writel accessors accomodate for that by swapping the
data on big-endian kernels, because most SoC designers tend to
pick little-endian device registers by default.

> This was the premise on which this patch was made.
> 
> So for big endian systems, isn't what we want is to read in big-endian
> without any byteswapping to little-endian? But your saying this breaks
> big-endian ARM systems as well. Am I missing something?

The systems are not a particular endianess, only the current state
of the CPU is, and that may change independent of the way the
hardware block got synthesized. We don't support swapping endianess
at runtime in Linux, but the system normally doesn't care what we
run.

The normal behavior is for the register contents to be read as
little-endian, and then swapped on big-endian kernel builds to
match what the kernel expects.

MIPS is a special case here, because the endianess of the CPU
core is fixed in hardware (or using a strapping pin) and is often
tied to the endianess of all the IP blocks. There are a couple
of other architectures like this (e.g. ARM ixp4xx, but none of the
modern ARM systems).

	Arnd

[toc] | [prev] | [next] | [standalone]

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

On 5/9/2016 1:39 PM, Arnd Bergmann wrote:
> On Monday 09 May 2016 13:22:48 John Youn wrote:
>> On 5/9/2016 3:36 AM, Arnd Bergmann wrote:
>>> On Monday 09 May 2016 10:23:22 Benjamin Herrenschmidt wrote:
>>>> On Sun, 2016-05-08 at 13:44 +0200, Christian Lamparter wrote:
>>>>> On Sunday, May 08, 2016 08:40:55 PM Benjamin Herrenschmidt wrote:
>>>>>>
>>>>>> On Sun, 2016-05-08 at 00:54 +0200, Christian Lamparter via Linuxppc-dev 
>>>>>> wrote:
>>>>>>>
>>>>>>> I've been looking in getting the MyBook Live Duo's USB OTG port
>>>>>>> to function. The SoC is a APM82181. Which has a PowerPC 464 core
>>>>>>> and related to the supported canyonlands architecture in
>>>>>>> arch/powerpc/.
>>>>>>>
>>>>>>> Currently in -next the dwc2 module doesn't load: 
>>>>>> Smells like the APM implementation is little endian. You might need to
>>>>>> use a flag to indicate what endian to use instead and set it
>>>>>> appropriately based on some DT properties.
>>>>> I tried. As per common-properties[0], I added little-endian; but it has no
>>>>> effect. I looked in dwc2_driver_probe and found no way of specifying the
>>>>> endian of the device. It all comes down to the dwc2_readl & dwc2_writel
>>>>> accessors. These - sadly - have been hardwired to use __raw_readl and
>>>>> __raw_writel. So, it's always "native-endian". While common-properties
>>>>> says little-endian should be preferred.
>>>>
>>>> Right, I meant, you should produce a patch adding a runtime test inside
>>>> those functions based on a device-tree property, a bit like we do for
>>>> some of the HCDs like OHCI, EHCI etc...
>>>>
>>>>
>>>
>>> The patch that caused the problem had multiple issues:
>>>
>>> - it broke big-endian ARM kernels: any machine that was working
>>>   correctly with a little-endian kernel is no longer using byteswaps
>>>   on big-endian kernels, which clearly breaks them.
>>
>>
>> I'm a bit confused about how this is supposed to work. My
>> understanding was that the readl() and writel() are defined as little
>> endian. So byte-swapping was performed if the architecture is big
>> endian. And the raw versions never swapped, always using the "native"
>> endianness.
>>
>> dwc2 is always treating the result of readl/writel as if it was read
>> in native endian. So it needs to read the registers in big-endian on
>> big-endian systems.
> 
> The hardware has no idea of what endianess the CPU uses at any
> given time, it's fixed by the SoC design, so there is no such
> thing as "native" endianess for a random IP block.
> 
> The readl/writel accessors accomodate for that by swapping the
> data on big-endian kernels, because most SoC designers tend to
> pick little-endian device registers by default.
> 
>> This was the premise on which this patch was made.
>>
>> So for big endian systems, isn't what we want is to read in big-endian
>> without any byteswapping to little-endian? But your saying this breaks
>> big-endian ARM systems as well. Am I missing something?
> 
> The systems are not a particular endianess, only the current state
> of the CPU is, and that may change independent of the way the
> hardware block got synthesized. We don't support swapping endianess
> at runtime in Linux, but the system normally doesn't care what we
> run.
> 
> The normal behavior is for the register contents to be read as
> little-endian, and then swapped on big-endian kernel builds to
> match what the kernel expects.
> 
> MIPS is a special case here, because the endianess of the CPU
> core is fixed in hardware (or using a strapping pin) and is often
> tied to the endianess of all the IP blocks. There are a couple
> of other architectures like this (e.g. ARM ixp4xx, but none of the
> modern ARM systems).

Ok thanks. What you're saying is clear now.

Is there a standard way to handle this? Must all drivers either check
some endianness configuration or do a runtime check?

Regards,
John

[toc] | [prev] | [next] | [standalone]

#1397415

On Monday 09 May 2016 14:11:23 John Youn wrote:
> On 5/9/2016 1:39 PM, Arnd Bergmann wrote:
>
> > The systems are not a particular endianess, only the current state
> > of the CPU is, and that may change independent of the way the
> > hardware block got synthesized. We don't support swapping endianess
> > at runtime in Linux, but the system normally doesn't care what we
> > run.
> > 
> > The normal behavior is for the register contents to be read as
> > little-endian, and then swapped on big-endian kernel builds to
> > match what the kernel expects.
> > 
> > MIPS is a special case here, because the endianess of the CPU
> > core is fixed in hardware (or using a strapping pin) and is often
> > tied to the endianess of all the IP blocks. There are a couple
> > of other architectures like this (e.g. ARM ixp4xx, but none of the
> > modern ARM systems).
> 
> Ok thanks. What you're saying is clear now.
> 
> Is there a standard way to handle this? Must all drivers either check
> some endianness configuration or do a runtime check?

There are four common approaches:

1 hardcode a particular endianess because an IP block is always
  used the same way

2 pick the right endianess at compile-time because we know
  what we are building for (typically by CPU architecture, but
  there are some other ways)

3 do runtime configuration based on a DT property or platform data

4 do automatic runtime configuration based on well-known register
  contents

Approach 1 is the most common, in particular as most IP blocks
are not configurable and are not used on big-endian MIPS machines.

Approach 4 as implemented by Christian Lamparter's patch is the
most reliable way, and probably makes most sense for DesignWare
IP blocks in general, because they are configurable and may
get used in all kinds of systems.

Approaches 2 and 3 are somewhat inferior because you have to
rely on everyone else doing it the way you planned, but 2
is particularly easy and 3 is used when there is no obvious
way to detect endianess of the device.

The patch I suggested would be approach 2, and as Felipe correctly
mentioned, it is not ideal, but it may be more appropriate for
backports to the 4.4-longterm, 4.5-stable and 4.6-stable kernels
than the more elaborate patch we are now discussing.

	Arnd

[toc] | [prev] | [standalone]

Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | linux.kernel

csiph-web