Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1318449 > unrolled thread
| Started by | Mark Brown <broonie@kernel.org> |
|---|---|
| First post | 2016-01-26 23:50 +0100 |
| Last post | 2016-01-27 20:00 +0100 |
| Articles | 7 — 5 participants |
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.
[PATCH RFC 2/2] MIPS: dt: Explicitly specify native endian behaviour for syscon Mark Brown <broonie@kernel.org> - 2016-01-26 23:50 +0100
Re: [PATCH RFC 2/2] MIPS: dt: Explicitly specify native endian behaviour for syscon Florian Fainelli <f.fainelli@gmail.com> - 2016-01-27 00:20 +0100
Re: [PATCH RFC 2/2] MIPS: dt: Explicitly specify native endian behaviour for syscon Ralf Baechle <ralf@linux-mips.org> - 2016-01-27 10:40 +0100
Re: [PATCH RFC 2/2] MIPS: dt: Explicitly specify native endian behaviour for syscon Jonas Gorski <jogo@openwrt.org> - 2016-01-27 11:40 +0100
Re: [PATCH RFC 2/2] MIPS: dt: Explicitly specify native endian behaviour for syscon Johannes Berg <johannes@sipsolutions.net> - 2016-01-27 11:50 +0100
Re: [PATCH RFC 2/2] MIPS: dt: Explicitly specify native endian behaviour for syscon Mark Brown <broonie@kernel.org> - 2016-01-27 12:20 +0100
Re: [PATCH RFC 2/2] MIPS: dt: Explicitly specify native endian behaviour for syscon Mark Brown <broonie@kernel.org> - 2016-01-27 20:00 +0100
| From | Mark Brown <broonie@kernel.org> |
|---|---|
| Date | 2016-01-26 23:50 +0100 |
| Subject | [PATCH RFC 2/2] MIPS: dt: Explicitly specify native endian behaviour for syscon |
| Message-ID | <qVkAW-2yI-15@gated-at.bofh.it> |
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@sipsolutions.net>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
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 +
arch/mips/boot/dts/brcm/bcm7125.dtsi | 1 +
arch/mips/boot/dts/brcm/bcm7346.dtsi | 1 +
arch/mips/boot/dts/brcm/bcm7358.dtsi | 1 +
arch/mips/boot/dts/brcm/bcm7360.dtsi | 1 +
arch/mips/boot/dts/brcm/bcm7362.dtsi | 1 +
arch/mips/boot/dts/brcm/bcm7420.dtsi | 1 +
arch/mips/boot/dts/brcm/bcm7425.dtsi | 1 +
arch/mips/boot/dts/brcm/bcm7435.dtsi | 1 +
9 files changed, 9 insertions(+)
diff --git a/arch/mips/boot/dts/brcm/bcm6328.dtsi b/arch/mips/boot/dts/brcm/bcm6328.dtsi
index d52ce3d07f16..04867035ea0e 100644
--- a/arch/mips/boot/dts/brcm/bcm6328.dtsi
+++ b/arch/mips/boot/dts/brcm/bcm6328.dtsi
@@ -73,6 +73,7 @@
timer: timer@10000040 {
compatible = "syscon";
reg = <0x10000040 0x2c>;
+ native-endian;
};
reboot {
diff --git a/arch/mips/boot/dts/brcm/bcm7125.dtsi b/arch/mips/boot/dts/brcm/bcm7125.dtsi
index 4fc7ecee273c..3ae16053a0c9 100644
--- a/arch/mips/boot/dts/brcm/bcm7125.dtsi
+++ b/arch/mips/boot/dts/brcm/bcm7125.dtsi
@@ -98,6 +98,7 @@
sun_top_ctrl: syscon@404000 {
compatible = "brcm,bcm7125-sun-top-ctrl", "syscon";
reg = <0x404000 0x60c>;
+ native-endian;
};
reboot {
diff --git a/arch/mips/boot/dts/brcm/bcm7346.dtsi b/arch/mips/boot/dts/brcm/bcm7346.dtsi
index a3039bb53477..be7991917d29 100644
--- a/arch/mips/boot/dts/brcm/bcm7346.dtsi
+++ b/arch/mips/boot/dts/brcm/bcm7346.dtsi
@@ -118,6 +118,7 @@
sun_top_ctrl: syscon@404000 {
compatible = "brcm,bcm7346-sun-top-ctrl", "syscon";
reg = <0x404000 0x51c>;
+ native-endian;
};
reboot {
diff --git a/arch/mips/boot/dts/brcm/bcm7358.dtsi b/arch/mips/boot/dts/brcm/bcm7358.dtsi
index 4274ff41ec21..060805be619a 100644
--- a/arch/mips/boot/dts/brcm/bcm7358.dtsi
+++ b/arch/mips/boot/dts/brcm/bcm7358.dtsi
@@ -112,6 +112,7 @@
sun_top_ctrl: syscon@404000 {
compatible = "brcm,bcm7358-sun-top-ctrl", "syscon";
reg = <0x404000 0x51c>;
+ native-endian;
};
reboot {
diff --git a/arch/mips/boot/dts/brcm/bcm7360.dtsi b/arch/mips/boot/dts/brcm/bcm7360.dtsi
index 0dcc9163c27b..bcdb09bfe07b 100644
--- a/arch/mips/boot/dts/brcm/bcm7360.dtsi
+++ b/arch/mips/boot/dts/brcm/bcm7360.dtsi
@@ -112,6 +112,7 @@
sun_top_ctrl: syscon@404000 {
compatible = "brcm,bcm7360-sun-top-ctrl", "syscon";
reg = <0x404000 0x51c>;
+ native-endian;
};
reboot {
diff --git a/arch/mips/boot/dts/brcm/bcm7362.dtsi b/arch/mips/boot/dts/brcm/bcm7362.dtsi
index 2f3f9fc2c478..d3b1b762e6c3 100644
--- a/arch/mips/boot/dts/brcm/bcm7362.dtsi
+++ b/arch/mips/boot/dts/brcm/bcm7362.dtsi
@@ -118,6 +118,7 @@
sun_top_ctrl: syscon@404000 {
compatible = "brcm,bcm7362-sun-top-ctrl", "syscon";
reg = <0x404000 0x51c>;
+ native-endian;
};
reboot {
diff --git a/arch/mips/boot/dts/brcm/bcm7420.dtsi b/arch/mips/boot/dts/brcm/bcm7420.dtsi
index bee221b3b568..3302a1b8a5c9 100644
--- a/arch/mips/boot/dts/brcm/bcm7420.dtsi
+++ b/arch/mips/boot/dts/brcm/bcm7420.dtsi
@@ -99,6 +99,7 @@
sun_top_ctrl: syscon@404000 {
compatible = "brcm,bcm7420-sun-top-ctrl", "syscon";
reg = <0x404000 0x60c>;
+ native-endian;
};
reboot {
diff --git a/arch/mips/boot/dts/brcm/bcm7425.dtsi b/arch/mips/boot/dts/brcm/bcm7425.dtsi
index 571f30f52e3f..15b27aae15a9 100644
--- a/arch/mips/boot/dts/brcm/bcm7425.dtsi
+++ b/arch/mips/boot/dts/brcm/bcm7425.dtsi
@@ -100,6 +100,7 @@
sun_top_ctrl: syscon@404000 {
compatible = "brcm,bcm7425-sun-top-ctrl", "syscon";
reg = <0x404000 0x51c>;
+ native-endian;
};
reboot {
diff --git a/arch/mips/boot/dts/brcm/bcm7435.dtsi b/arch/mips/boot/dts/brcm/bcm7435.dtsi
index 614ee211f71a..adb33e355043 100644
--- a/arch/mips/boot/dts/brcm/bcm7435.dtsi
+++ b/arch/mips/boot/dts/brcm/bcm7435.dtsi
@@ -114,6 +114,7 @@
sun_top_ctrl: syscon@404000 {
compatible = "brcm,bcm7425-sun-top-ctrl", "syscon";
reg = <0x404000 0x51c>;
+ native-endian;
};
reboot {
--
2.7.0.rc3
[toc] | [next] | [standalone]
| From | Florian Fainelli <f.fainelli@gmail.com> |
|---|---|
| Date | 2016-01-27 00:20 +0100 |
| Subject | Re: [PATCH RFC 2/2] MIPS: dt: Explicitly specify native endian behaviour for syscon |
| Message-ID | <qVl3Y-2ZP-19@gated-at.bofh.it> |
| In reply to | #1318449 |
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@sipsolutions.net>
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
>
> 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;
};
reboot: syscon-reboot@10000008 {
--
Florian
[toc] | [prev] | [next] | [standalone]
| From | Ralf Baechle <ralf@linux-mips.org> |
|---|---|
| Date | 2016-01-27 10:40 +0100 |
| Subject | Re: [PATCH RFC 2/2] MIPS: dt: Explicitly specify native endian behaviour for syscon |
| Message-ID | <qVuJY-1sr-11@gated-at.bofh.it> |
| In reply to | #1318462 |
On Tue, Jan 26, 2016 at 03:16:15PM -0800, Florian Fainelli wrote:
> 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;
> };
>
> reboot: syscon-reboot@10000008 {
Acked-by: Ralf Baechle <ralf@linux-mips.org>
for the combined patch.
Ralf
[toc] | [prev] | [next] | [standalone]
| From | Jonas Gorski <jogo@openwrt.org> |
|---|---|
| Date | 2016-01-27 11:40 +0100 |
| Subject | Re: [PATCH RFC 2/2] MIPS: dt: Explicitly specify native endian behaviour for syscon |
| Message-ID | <qVvG2-2ab-15@gated-at.bofh.it> |
| In reply to | #1318462 |
Hi,
On 27 January 2016 at 00:16, Florian Fainelli <f.fainelli@gmail.com> 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@sipsolutions.net>
>> Signed-off-by: Mark Brown <broonie@kernel.org>
>> ---
>>
>> 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
[toc] | [prev] | [next] | [standalone]
| From | Johannes Berg <johannes@sipsolutions.net> |
|---|---|
| Date | 2016-01-27 11:50 +0100 |
| Subject | Re: [PATCH RFC 2/2] MIPS: dt: Explicitly specify native endian behaviour for syscon |
| Message-ID | <qVvPI-2dw-17@gated-at.bofh.it> |
| In reply to | #1318840 |
On Wed, 2016-01-27 at 11:33 +0100, Jonas Gorski wrote:
>
> > +++ 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?
>
It's complicated :)
These were originally specified as little-endian because it _worked_,
but that was only because of an issue with the code - it was getting
byteswapped twice.
This was "fixed" in commit 29bb45f25ff3051354ed330c0d0f10418a2b8c7c,
and I assume this DT file was created/copied before that commit and
didn't get that update due to merge delays. So after that commit, this
file couldn't have worked, but that's how it got to this point.
johannes
[toc] | [prev] | [next] | [standalone]
| From | Mark Brown <broonie@kernel.org> |
|---|---|
| Date | 2016-01-27 12:20 +0100 |
| Subject | Re: [PATCH RFC 2/2] MIPS: dt: Explicitly specify native endian behaviour for syscon |
| Message-ID | <qVwiK-2Fz-19@gated-at.bofh.it> |
| In reply to | #1318840 |
[Multipart message — attachments visible in raw view] — view raw
On Wed, Jan 27, 2016 at 11:33:37AM +0100, Jonas Gorski wrote: > > - little-endian; > > + native-endian; > But native-endian == big-endian usually for bcm63xx, so is this > actually a bugfix? Yes, it's buggy - in v4.5-rc1 we respect the endianness so big endian just won't work, prior to that this was a hack around another bug which was done rather than reporting it so it "just" results in a DT that's obviously incorrect. This local hack has been causing a lot of problems.
[toc] | [prev] | [next] | [standalone]
| From | Mark Brown <broonie@kernel.org> |
|---|---|
| Date | 2016-01-27 20:00 +0100 |
| Subject | Re: [PATCH RFC 2/2] MIPS: dt: Explicitly specify native endian behaviour for syscon |
| Message-ID | <qVDtX-7VF-69@gated-at.bofh.it> |
| In reply to | #1318462 |
[Multipart message — attachments visible in raw view] — view raw
On Tue, Jan 26, 2016 at 03:16:15PM -0800, Florian Fainelli wrote: > 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: Folded that in, thanks.
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web