Re: [PATCH RFC 2/2] MIPS: dt: Explicitly specify native endian behaviour for syscon

[Jonas Gorski]

Hi,

On 27 January 2016 at 00:16, Florian Fainelli <f.fainelli@xxxxxxxxx> wrote:
> On 26/01/16 14:46, Mark Brown wrote:
>> On many MIPS systems the endianness of IP blocks is kept the same as
>> that of the CPU by the hardware.  This includes the system controllers
>> on these systems which are controlled via syscon which uses the regmap
>> API which used readl() and writel() to interact with the hardware,
>> meaning that all writes are converted to little endian when writing to
>> the hardware.  This caused a bad interaction with the regmap core in big
>> endian mode since it was not aware of the byte swapping and so ended up
>> performing little endian writes.
>>
>> Unfortunately when this issue was noticed it was addressed by updating
>> the DT for the affected devices to specify them as little endian.  This
>> happened to work since it resulted in two endianness swaps which
>> cancelled each other out and gave little endian behaviour but meant that
>> the DT was clearly not accurately describing the hardware.
>>
>> The intention of commit 29bb45f25ff305 (regmap-mmio: Use native
>> endianness for read/write) was to fix this by making regmap default to
>> native endianness but this breaks most other MMIO users where the
>> hardware has a fixed endianness and the implementation uses the __raw
>> accessors which are not intended to be used outside of architecture
>> code.  Instead use the newly added native-endian DT property to say
>> exactly what we want for these systems.
>>
>> Fixes: 29bb45f25ff305 (regmap-mmio: Use native endianness for read/write)
>> Reported-by: Johannes Berg <johannes@xxxxxxxxxxxxxxxx>
>> Signed-off-by: Mark Brown <broonie@xxxxxxxxxx>
>> ---
>>
>> Posted for review only, this will interact with some other patches
>> fixing the implementation of regmap-mmio and will probably need to be
>> merged along with them.
>>
>>  arch/mips/boot/dts/brcm/bcm6328.dtsi | 1 +
>
> v4.5-rc1 now contains an arch/mips/boot/dts/brcm/bcm6368.dtsi which
> copied the 6328.dtsi and therefore needs this hunk to be added to your
> patch series:
>
> diff --git a/arch/mips/boot/dts/brcm/bcm6368.dtsi
> b/arch/mips/boot/dts/brcm/bcm6368.dtsi
> index 9c8d3fe28b31..1f6b9b5cddb4 100644
> --- a/arch/mips/boot/dts/brcm/bcm6368.dtsi
> +++ b/arch/mips/boot/dts/brcm/bcm6368.dtsi
> @@ -54,7 +54,7 @@
>                 periph_cntl: syscon@10000000 {
>                         compatible = "syscon";
>                         reg = <0x10000000 0x14>;
> -                       little-endian;
> +                       native-endian;

But native-endian == big-endian usually for bcm63xx, so is this
actually a bugfix?


Jonas