Re: [PATCH V3 3/7] of: Document {little,big,native}-endian bindings

[Peter Hurley <peter@hurleysoftware.com>]

On 03/02/2015 11:28 AM, Kevin Cernekee wrote:
> On Mon, Mar 2, 2015 at 8:08 AM, Peter Hurley <peter@hurleysoftware.com> wrote:
>> On 03/02/2015 09:56 AM, Kevin Cernekee wrote:
>>> On Mon, Mar 2, 2015 at 5:14 AM, Peter Hurley <peter@hurleysoftware.com> wrote:
>>>> On 11/24/2014 06:36 PM, Kevin Cernekee wrote:
>>>>> These apply to newly converted drivers, like serial8250/libahci/...
>>>>> The examples were adapted from the regmap bindings document.
>>>>>
>>>>> Signed-off-by: Kevin Cernekee <cernekee@gmail.com>
>>>>> ---
>>>>>  .../devicetree/bindings/common-properties.txt      | 60 ++++++++++++++++++++++
>>>>>  1 file changed, 60 insertions(+)
>>>>>  create mode 100644 Documentation/devicetree/bindings/common-properties.txt
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/common-properties.txt b/Documentation/devicetree/bindings/common-properties.txt
>>>>> new file mode 100644
>>>>> index 0000000..21044a4
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/common-properties.txt
>>>>> @@ -0,0 +1,60 @@
>>>>> +Common properties
>>>>> +
>>>>> +The ePAPR specification does not define any properties related to hardware
>>>>> +byteswapping, but endianness issues show up frequently in porting Linux to
>>>>> +different machine types.  This document attempts to provide a consistent
>>>>> +way of handling byteswapping across drivers.
>>>>> +
>>>>> +Optional properties:
>>>>> + - big-endian: Boolean; force big endian register accesses
>>>>> +   unconditionally (e.g. ioread32be/iowrite32be).  Use this if you
>>>>> +   know the peripheral always needs to be accessed in BE mode.
>>>>> + - little-endian: Boolean; force little endian register accesses
>>>>> +   unconditionally (e.g. readl/writel).  Use this if you know the
>>>>> +   peripheral always needs to be accessed in LE mode.  This is the
>>>>> +   default.
>>>>
>>>> There is a fundamental problem with specifying the default in DT bindings.
>>>> How can drivers which are currently native-endian support big-endian?
>>>>
>>>> If the driver is converted to support big-endian, every previous
>>>> devicetree will be invalid with the new kernel (because those devicetrees
>>>> don't specify 'native-endian').
>>>>
>>>> IOW, consider if the default were 'native-endian'. How would the 8250
>>>> driver support existing devicetrees?
>>>
>>> Correct.  This scheme is intended for drivers like 8250 and libahci
>>> which currently default to little-endian by virtue of using
>>> readl/writel for MMIO accesses.  Drivers that default to native-endian
>>> should specify that in their bindings documents, similar to
>>> Documentation/devicetree/bindings/regmap/regmap.txt.
>>
>> Which effectively means if a user can't upgrade their devicetree, they
>> can't upgrade their kernel. I don't think that flies.
> 
> This doesn't change the behavior of pre-existing drivers that
> implement the *-endian properties in a different way.  There are not
> many of these drivers and they can be documented as special cases.

Yeah, ok, as long as there's no expectation that existing drivers
meet this criteria when they add big-endian support.

>> It's exactly this kind of stuff that prompted Jonathan Corbet's article,
>> "Device trees as ABI"  http://lwn.net/Articles/561462
>>
>> Why not leave the default unspecified?
> 
> The document aims to provide a consistent way of handling DT
> endianness properties across (compliant) drivers.  It is confusing if
> one new driver defaults to little-endian, and another new driver
> defaults to native-endian.

Ok. How many 4.0 driver + DT submissions that are native-endian are
declaring this binding?


> And since most of the commonly used drivers already implement
> little-endian MMIO accesses, that is the default.  My personal
> preference would have been native-endian since that seems more common
> on the hardware side, but defaulting to little-endian prevents
> breaking the device tree "ABI" on existing systems.

That was basically my point; there's no way to meet these goals
for existing, native-endian drivers without breakage (just as there
would have been no way if native-endian had been the default).

Regards,
Peter Hurley