* [PATCH 1/2] ARM: mvebu: second PCIe unit of Armada XP mv78230 is only x1 capable
2013-11-05 20:45 [PATCH 0/2] ARM: mvebu: fix DT def. of PCIe units for mv78230 and mv78260 Arnaud Ebalard
@ 2013-11-05 20:45 ` Arnaud Ebalard
2013-11-06 14:43 ` Thomas Petazzoni
2013-11-05 20:46 ` [PATCH 2/2] ARM: mvebu: fix second and third PCIe unit of Armada XP mv78260 Arnaud Ebalard
` (3 subsequent siblings)
4 siblings, 1 reply; 20+ messages in thread
From: Arnaud Ebalard @ 2013-11-05 20:45 UTC (permalink / raw)
To: linux-arm-kernel
Various Marvell datasheets advertise second PCIe unit of mv78230
flavour of Armada XP as x4/quad x1 capable. This second unit is in
fact only x1 capable. This patch fixes current mv78230 .dtsi to
reflect that, i.e. makes 1.0 the second interface (instead of 2.0
at the moment). This was successfully tested on a mv78230-based
ReadyNAS 2120 platform with a x1 device (FL1009 XHCI controller)
connected to this second interface.
Signed-off-by: Arnaud Ebalard <arno@natisbad.org>
---
arch/arm/boot/dts/armada-xp-mv78230.dtsi | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/arch/arm/boot/dts/armada-xp-mv78230.dtsi b/arch/arm/boot/dts/armada-xp-mv78230.dtsi
index 0358a33..9dc7381 100644
--- a/arch/arm/boot/dts/armada-xp-mv78230.dtsi
+++ b/arch/arm/boot/dts/armada-xp-mv78230.dtsi
@@ -47,7 +47,7 @@
/*
* MV78230 has 2 PCIe units Gen2.0: One unit can be
* configured as x4 or quad x1 lanes. One unit is
- * x4/x1.
+ * x1 only.
*/
pcie-controller {
compatible = "marvell,armada-xp-pcie";
@@ -61,10 +61,10 @@
ranges =
<0x82000000 0 0x40000 MBUS_ID(0xf0, 0x01) 0x40000 0 0x00002000 /* Port 0.0 registers */
- 0x82000000 0 0x42000 MBUS_ID(0xf0, 0x01) 0x42000 0 0x00002000 /* Port 2.0 registers */
0x82000000 0 0x44000 MBUS_ID(0xf0, 0x01) 0x44000 0 0x00002000 /* Port 0.1 registers */
0x82000000 0 0x48000 MBUS_ID(0xf0, 0x01) 0x48000 0 0x00002000 /* Port 0.2 registers */
0x82000000 0 0x4c000 MBUS_ID(0xf0, 0x01) 0x4c000 0 0x00002000 /* Port 0.3 registers */
+ 0x82000000 0 0x80000 MBUS_ID(0xf0, 0x01) 0x80000 0 0x00002000 /* Port 1.0 registers */
0x82000000 0x1 0 MBUS_ID(0x04, 0xe8) 0 1 0 /* Port 0.0 MEM */
0x81000000 0x1 0 MBUS_ID(0x04, 0xe0) 0 1 0 /* Port 0.0 IO */
0x82000000 0x2 0 MBUS_ID(0x04, 0xd8) 0 1 0 /* Port 0.1 MEM */
@@ -73,8 +73,8 @@
0x81000000 0x3 0 MBUS_ID(0x04, 0xb0) 0 1 0 /* Port 0.2 IO */
0x82000000 0x4 0 MBUS_ID(0x04, 0x78) 0 1 0 /* Port 0.3 MEM */
0x81000000 0x4 0 MBUS_ID(0x04, 0x70) 0 1 0 /* Port 0.3 IO */
- 0x82000000 0x9 0 MBUS_ID(0x04, 0xf8) 0 1 0 /* Port 2.0 MEM */
- 0x81000000 0x9 0 MBUS_ID(0x04, 0xf0) 0 1 0 /* Port 2.0 IO */>;
+ 0x82000000 0x5 0 MBUS_ID(0x08, 0xe8) 0 1 0 /* Port 1.0 MEM */
+ 0x81000000 0x5 0 MBUS_ID(0x08, 0xe0) 0 1 0 /* Port 1.0 IO */>;
pcie at 1,0 {
device_type = "pci";
@@ -144,20 +144,20 @@
status = "disabled";
};
- pcie at 9,0 {
+ pcie at 5,0 {
device_type = "pci";
- assigned-addresses = <0x82000800 0 0x42000 0 0x2000>;
- reg = <0x4800 0 0 0 0>;
+ assigned-addresses = <0x82000800 0 0x80000 0 0x2000>;
+ reg = <0x2800 0 0 0 0>;
#address-cells = <3>;
#size-cells = <2>;
#interrupt-cells = <1>;
- ranges = <0x82000000 0 0 0x82000000 0x9 0 1 0
- 0x81000000 0 0 0x81000000 0x9 0 1 0>;
+ ranges = <0x82000000 0 0 0x82000000 0x5 0 1 0
+ 0x81000000 0 0 0x81000000 0x5 0 1 0>;
interrupt-map-mask = <0 0 0 0>;
- interrupt-map = <0 0 0 0 &mpic 99>;
- marvell,pcie-port = <2>;
+ interrupt-map = <0 0 0 0 &mpic 62>;
+ marvell,pcie-port = <1>;
marvell,pcie-lane = <0>;
- clocks = <&gateclk 26>;
+ clocks = <&gateclk 9>;
status = "disabled";
};
};
--
1.8.4.rc3
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 1/2] ARM: mvebu: second PCIe unit of Armada XP mv78230 is only x1 capable
2013-11-05 20:45 ` [PATCH 1/2] ARM: mvebu: second PCIe unit of Armada XP mv78230 is only x1 capable Arnaud Ebalard
@ 2013-11-06 14:43 ` Thomas Petazzoni
2013-11-06 18:08 ` Arnaud Ebalard
0 siblings, 1 reply; 20+ messages in thread
From: Thomas Petazzoni @ 2013-11-06 14:43 UTC (permalink / raw)
To: linux-arm-kernel
Dear Arnaud Ebalard,
On Tue, 05 Nov 2013 21:45:48 +0100, Arnaud Ebalard wrote:
> Various Marvell datasheets advertise second PCIe unit of mv78230
> flavour of Armada XP as x4/quad x1 capable. This second unit is in
> fact only x1 capable. This patch fixes current mv78230 .dtsi to
> reflect that, i.e. makes 1.0 the second interface (instead of 2.0
> at the moment). This was successfully tested on a mv78230-based
> ReadyNAS 2120 platform with a x1 device (FL1009 XHCI controller)
> connected to this second interface.
>
> Signed-off-by: Arnaud Ebalard <arno@natisbad.org>
> ---
> arch/arm/boot/dts/armada-xp-mv78230.dtsi | 24 ++++++++++++------------
> 1 file changed, 12 insertions(+), 12 deletions(-)
Acked-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
This is actually a bug fix, and the problem exists since commit
9d8f44f02d4a5f6e7b8d138ea8f8c6e30ae6e1a3, and therefore in 3.10, 3.11
and 3.12.
However, while the PCIe DT stuff was merged in 3.10, the actual driver
itself was only merged in 3.11, so in 3.10, there is no chance to hit
the bug.
Therefore, having this fix pushed to the stable 3.11.x and 3.12.x trees
would be good.
Thanks Arnaud for following up on this!
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 1/2] ARM: mvebu: second PCIe unit of Armada XP mv78230 is only x1 capable
2013-11-06 14:43 ` Thomas Petazzoni
@ 2013-11-06 18:08 ` Arnaud Ebalard
2013-11-06 18:12 ` Jason Cooper
0 siblings, 1 reply; 20+ messages in thread
From: Arnaud Ebalard @ 2013-11-06 18:08 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
Thomas Petazzoni <thomas.petazzoni@free-electrons.com> writes:
> On Tue, 05 Nov 2013 21:45:48 +0100, Arnaud Ebalard wrote:
>> Various Marvell datasheets advertise second PCIe unit of mv78230
>> flavour of Armada XP as x4/quad x1 capable. This second unit is in
>> fact only x1 capable. This patch fixes current mv78230 .dtsi to
>> reflect that, i.e. makes 1.0 the second interface (instead of 2.0
>> at the moment). This was successfully tested on a mv78230-based
>> ReadyNAS 2120 platform with a x1 device (FL1009 XHCI controller)
>> connected to this second interface.
>>
>> Signed-off-by: Arnaud Ebalard <arno@natisbad.org>
>> ---
>> arch/arm/boot/dts/armada-xp-mv78230.dtsi | 24 ++++++++++++------------
>> 1 file changed, 12 insertions(+), 12 deletions(-)
>
> Acked-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Thanks for the review of both patches, Thomas. And no relation, but
thanks for 714086029116b6 ;-)
> This is actually a bug fix, and the problem exists since commit
> 9d8f44f02d4a5f6e7b8d138ea8f8c6e30ae6e1a3, and therefore in 3.10, 3.11
> and 3.12.
>
> However, while the PCIe DT stuff was merged in 3.10, the actual driver
> itself was only merged in 3.11, so in 3.10, there is no chance to hit
> the bug.
>
> Therefore, having this fix pushed to the stable 3.11.x and 3.12.x trees
> would be good.
Jason, any action required on my side regarding the relay of the two
patches to stable team or will you handle that once they are in your
tree (or Linus one)?
Cheers,
a+
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 1/2] ARM: mvebu: second PCIe unit of Armada XP mv78230 is only x1 capable
2013-11-06 18:08 ` Arnaud Ebalard
@ 2013-11-06 18:12 ` Jason Cooper
2013-11-06 18:20 ` Thomas Petazzoni
2013-11-21 23:28 ` Arnaud Ebalard
0 siblings, 2 replies; 20+ messages in thread
From: Jason Cooper @ 2013-11-06 18:12 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Nov 06, 2013 at 07:08:04PM +0100, Arnaud Ebalard wrote:
> Hi,
>
> Thomas Petazzoni <thomas.petazzoni@free-electrons.com> writes:
>
> > On Tue, 05 Nov 2013 21:45:48 +0100, Arnaud Ebalard wrote:
> >> Various Marvell datasheets advertise second PCIe unit of mv78230
> >> flavour of Armada XP as x4/quad x1 capable. This second unit is in
> >> fact only x1 capable. This patch fixes current mv78230 .dtsi to
> >> reflect that, i.e. makes 1.0 the second interface (instead of 2.0
> >> at the moment). This was successfully tested on a mv78230-based
> >> ReadyNAS 2120 platform with a x1 device (FL1009 XHCI controller)
> >> connected to this second interface.
> >>
> >> Signed-off-by: Arnaud Ebalard <arno@natisbad.org>
> >> ---
> >> arch/arm/boot/dts/armada-xp-mv78230.dtsi | 24 ++++++++++++------------
> >> 1 file changed, 12 insertions(+), 12 deletions(-)
> >
> > Acked-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
>
> Thanks for the review of both patches, Thomas. And no relation, but
> thanks for 714086029116b6 ;-)
>
> > This is actually a bug fix, and the problem exists since commit
> > 9d8f44f02d4a5f6e7b8d138ea8f8c6e30ae6e1a3, and therefore in 3.10, 3.11
> > and 3.12.
> >
> > However, while the PCIe DT stuff was merged in 3.10, the actual driver
> > itself was only merged in 3.11, so in 3.10, there is no chance to hit
> > the bug.
> >
> > Therefore, having this fix pushed to the stable 3.11.x and 3.12.x trees
> > would be good.
oops, I forgot to reply to this. I disagree here. We shouldn't assume
that the dts file used will be from the same commit and the kernel
built. Additionally, it doesn't hurt to backport the change the whole
way.
> Jason, any action required on my side regarding the relay of the two
> patches to stable team or will you handle that once they are in your
> tree (or Linus one)?
I'll take care of it.
thx,
Jason.
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 1/2] ARM: mvebu: second PCIe unit of Armada XP mv78230 is only x1 capable
2013-11-06 18:12 ` Jason Cooper
@ 2013-11-06 18:20 ` Thomas Petazzoni
2013-11-21 23:28 ` Arnaud Ebalard
1 sibling, 0 replies; 20+ messages in thread
From: Thomas Petazzoni @ 2013-11-06 18:20 UTC (permalink / raw)
To: linux-arm-kernel
Dear Jason Cooper,
On Wed, 6 Nov 2013 13:12:41 -0500, Jason Cooper wrote:
> > > However, while the PCIe DT stuff was merged in 3.10, the actual driver
> > > itself was only merged in 3.11, so in 3.10, there is no chance to hit
> > > the bug.
> > >
> > > Therefore, having this fix pushed to the stable 3.11.x and 3.12.x trees
> > > would be good.
>
> oops, I forgot to reply to this. I disagree here. We shouldn't assume
> that the dts file used will be from the same commit and the kernel
> built. Additionally, it doesn't hurt to backport the change the whole
> way.
Well, I believe anyone trying to use mvebu .dts from one kernel version
with a kernel image of another version will hit numerous problems. But
ok, in principle I agree with you :)
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 1/2] ARM: mvebu: second PCIe unit of Armada XP mv78230 is only x1 capable
2013-11-06 18:12 ` Jason Cooper
2013-11-06 18:20 ` Thomas Petazzoni
@ 2013-11-21 23:28 ` Arnaud Ebalard
2013-11-22 13:44 ` Jason Cooper
1 sibling, 1 reply; 20+ messages in thread
From: Arnaud Ebalard @ 2013-11-21 23:28 UTC (permalink / raw)
To: linux-arm-kernel
Hi Jason,
Jason Cooper <jason@lakedaemon.net> writes:
> On Wed, Nov 06, 2013 at 07:08:04PM +0100, Arnaud Ebalard wrote:
>> Hi,
>>
>> Thomas Petazzoni <thomas.petazzoni@free-electrons.com> writes:
>>
>> > On Tue, 05 Nov 2013 21:45:48 +0100, Arnaud Ebalard wrote:
>> >> Various Marvell datasheets advertise second PCIe unit of mv78230
>> >> flavour of Armada XP as x4/quad x1 capable. This second unit is in
>> >> fact only x1 capable. This patch fixes current mv78230 .dtsi to
>> >> reflect that, i.e. makes 1.0 the second interface (instead of 2.0
>> >> at the moment). This was successfully tested on a mv78230-based
>> >> ReadyNAS 2120 platform with a x1 device (FL1009 XHCI controller)
>> >> connected to this second interface.
>> >>
>> >> Signed-off-by: Arnaud Ebalard <arno@natisbad.org>
>> >> ---
>> >> arch/arm/boot/dts/armada-xp-mv78230.dtsi | 24 ++++++++++++------------
>> >> 1 file changed, 12 insertions(+), 12 deletions(-)
>> >
>> > Acked-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
>>
>> Thanks for the review of both patches, Thomas. And no relation, but
>> thanks for 714086029116b6 ;-)
>>
>> > This is actually a bug fix, and the problem exists since commit
>> > 9d8f44f02d4a5f6e7b8d138ea8f8c6e30ae6e1a3, and therefore in 3.10, 3.11
>> > and 3.12.
>> >
>> > However, while the PCIe DT stuff was merged in 3.10, the actual driver
>> > itself was only merged in 3.11, so in 3.10, there is no chance to hit
>> > the bug.
>> >
>> > Therefore, having this fix pushed to the stable 3.11.x and 3.12.x trees
>> > would be good.
>
> oops, I forgot to reply to this. I disagree here. We shouldn't assume
> that the dts file used will be from the same commit and the kernel
> built. Additionally, it doesn't hurt to backport the change the whole
> way.
>
>> Jason, any action required on my side regarding the relay of the two
>> patches to stable team or will you handle that once they are in your
>> tree (or Linus one)?
>
> I'll take care of it.
I may have missed something but I expected those two patches to hit
Linus tree after 3.12 to be part of 3.13-rc1. Am I just too impatient?
Cheers,
a+
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 1/2] ARM: mvebu: second PCIe unit of Armada XP mv78230 is only x1 capable
2013-11-21 23:28 ` Arnaud Ebalard
@ 2013-11-22 13:44 ` Jason Cooper
2013-11-22 14:28 ` Thomas Petazzoni
0 siblings, 1 reply; 20+ messages in thread
From: Jason Cooper @ 2013-11-22 13:44 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Nov 22, 2013 at 12:28:01AM +0100, Arnaud Ebalard wrote:
> Hi Jason,
>
> Jason Cooper <jason@lakedaemon.net> writes:
>
> > On Wed, Nov 06, 2013 at 07:08:04PM +0100, Arnaud Ebalard wrote:
> >> Hi,
> >>
> >> Thomas Petazzoni <thomas.petazzoni@free-electrons.com> writes:
> >>
> >> > On Tue, 05 Nov 2013 21:45:48 +0100, Arnaud Ebalard wrote:
> >> >> Various Marvell datasheets advertise second PCIe unit of mv78230
> >> >> flavour of Armada XP as x4/quad x1 capable. This second unit is in
> >> >> fact only x1 capable. This patch fixes current mv78230 .dtsi to
> >> >> reflect that, i.e. makes 1.0 the second interface (instead of 2.0
> >> >> at the moment). This was successfully tested on a mv78230-based
> >> >> ReadyNAS 2120 platform with a x1 device (FL1009 XHCI controller)
> >> >> connected to this second interface.
> >> >>
> >> >> Signed-off-by: Arnaud Ebalard <arno@natisbad.org>
> >> >> ---
> >> >> arch/arm/boot/dts/armada-xp-mv78230.dtsi | 24 ++++++++++++------------
> >> >> 1 file changed, 12 insertions(+), 12 deletions(-)
> >> >
> >> > Acked-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> >>
> >> Thanks for the review of both patches, Thomas. And no relation, but
> >> thanks for 714086029116b6 ;-)
> >>
> >> > This is actually a bug fix, and the problem exists since commit
> >> > 9d8f44f02d4a5f6e7b8d138ea8f8c6e30ae6e1a3, and therefore in 3.10, 3.11
> >> > and 3.12.
> >> >
> >> > However, while the PCIe DT stuff was merged in 3.10, the actual driver
> >> > itself was only merged in 3.11, so in 3.10, there is no chance to hit
> >> > the bug.
> >> >
> >> > Therefore, having this fix pushed to the stable 3.11.x and 3.12.x trees
> >> > would be good.
> >
> > oops, I forgot to reply to this. I disagree here. We shouldn't assume
> > that the dts file used will be from the same commit and the kernel
> > built. Additionally, it doesn't hurt to backport the change the whole
> > way.
> >
> >> Jason, any action required on my side regarding the relay of the two
> >> patches to stable team or will you handle that once they are in your
> >> tree (or Linus one)?
> >
> > I'll take care of it.
>
> I may have missed something but I expected those two patches to hit
> Linus tree after 3.12 to be part of 3.13-rc1. Am I just too impatient?
It is a fix, but as (iirc) Thomas mentioned, there is the risk of
breaking existing PCIe boards. So, I was sitting on it till after
v3.13-rc1 drops. I'd like for it to have a run in -next and get a few
more Tested-by's before pushing it.
thx,
Jason.
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 1/2] ARM: mvebu: second PCIe unit of Armada XP mv78230 is only x1 capable
2013-11-22 13:44 ` Jason Cooper
@ 2013-11-22 14:28 ` Thomas Petazzoni
0 siblings, 0 replies; 20+ messages in thread
From: Thomas Petazzoni @ 2013-11-22 14:28 UTC (permalink / raw)
To: linux-arm-kernel
Dear Jason Cooper,
On Fri, 22 Nov 2013 08:44:49 -0500, Jason Cooper wrote:
> On Fri, Nov 22, 2013 at 12:28:01AM +0100, Arnaud Ebalard wrote:
> > Hi Jason,
> >
> > Jason Cooper <jason@lakedaemon.net> writes:
> >
> > > On Wed, Nov 06, 2013 at 07:08:04PM +0100, Arnaud Ebalard wrote:
> > >> Hi,
> > >>
> > >> Thomas Petazzoni <thomas.petazzoni@free-electrons.com> writes:
> > >>
> > >> > On Tue, 05 Nov 2013 21:45:48 +0100, Arnaud Ebalard wrote:
> > >> >> Various Marvell datasheets advertise second PCIe unit of mv78230
> > >> >> flavour of Armada XP as x4/quad x1 capable. This second unit is in
> > >> >> fact only x1 capable. This patch fixes current mv78230 .dtsi to
> > >> >> reflect that, i.e. makes 1.0 the second interface (instead of 2.0
> > >> >> at the moment). This was successfully tested on a mv78230-based
> > >> >> ReadyNAS 2120 platform with a x1 device (FL1009 XHCI controller)
> > >> >> connected to this second interface.
> > >> >>
> > >> >> Signed-off-by: Arnaud Ebalard <arno@natisbad.org>
> > >> >> ---
> > >> >> arch/arm/boot/dts/armada-xp-mv78230.dtsi | 24 ++++++++++++------------
> > >> >> 1 file changed, 12 insertions(+), 12 deletions(-)
> > >> >
> > >> > Acked-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> > >>
> > >> Thanks for the review of both patches, Thomas. And no relation, but
> > >> thanks for 714086029116b6 ;-)
> > >>
> > >> > This is actually a bug fix, and the problem exists since commit
> > >> > 9d8f44f02d4a5f6e7b8d138ea8f8c6e30ae6e1a3, and therefore in 3.10, 3.11
> > >> > and 3.12.
> > >> >
> > >> > However, while the PCIe DT stuff was merged in 3.10, the actual driver
> > >> > itself was only merged in 3.11, so in 3.10, there is no chance to hit
> > >> > the bug.
> > >> >
> > >> > Therefore, having this fix pushed to the stable 3.11.x and 3.12.x trees
> > >> > would be good.
> > >
> > > oops, I forgot to reply to this. I disagree here. We shouldn't assume
> > > that the dts file used will be from the same commit and the kernel
> > > built. Additionally, it doesn't hurt to backport the change the whole
> > > way.
> > >
> > >> Jason, any action required on my side regarding the relay of the two
> > >> patches to stable team or will you handle that once they are in your
> > >> tree (or Linus one)?
> > >
> > > I'll take care of it.
> >
> > I may have missed something but I expected those two patches to hit
> > Linus tree after 3.12 to be part of 3.13-rc1. Am I just too impatient?
>
> It is a fix, but as (iirc) Thomas mentioned, there is the risk of
> breaking existing PCIe boards. So, I was sitting on it till after
> v3.13-rc1 drops. I'd like for it to have a run in -next and get a few
> more Tested-by's before pushing it.
Hum, did I say that it could break existing PCIe boards? I might have
lost the context, but I don't quite see how it could break existing
PCIe boards. The patches from Arnaud can only make *more* PCIe cards
detected, without affecting the ones that were already correctly
detected.
So I don't think there should be any "suspicion" of potential breakage
with those two patches, there are only improving things without any
risk of breaking existing things, as far as I can remember.
Best regards,
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 2/2] ARM: mvebu: fix second and third PCIe unit of Armada XP mv78260
2013-11-05 20:45 [PATCH 0/2] ARM: mvebu: fix DT def. of PCIe units for mv78230 and mv78260 Arnaud Ebalard
2013-11-05 20:45 ` [PATCH 1/2] ARM: mvebu: second PCIe unit of Armada XP mv78230 is only x1 capable Arnaud Ebalard
@ 2013-11-05 20:46 ` Arnaud Ebalard
2013-11-06 14:54 ` Thomas Petazzoni
2013-11-06 18:14 ` [PATCH 0/2] ARM: mvebu: fix DT def. of PCIe units for mv78230 and mv78260 Arnaud Ebalard
` (2 subsequent siblings)
4 siblings, 1 reply; 20+ messages in thread
From: Arnaud Ebalard @ 2013-11-05 20:46 UTC (permalink / raw)
To: linux-arm-kernel
mv78260 flavour of Marvell Armada XP SoC has 3 PCIe units. The
two first units are both x4 and quad x1 capable. The third unit
is only x4 capable. This patch fixes mv78260 .dtsi to reflect
those capabilities.
Signed-off-by: Arnaud Ebalard <arno@natisbad.org>
---
arch/arm/boot/dts/armada-xp-mv78260.dtsi | 109 ++++++++++++++++++++++++-------
1 file changed, 85 insertions(+), 24 deletions(-)
diff --git a/arch/arm/boot/dts/armada-xp-mv78260.dtsi b/arch/arm/boot/dts/armada-xp-mv78260.dtsi
index 0e82c50..a598ce9 100644
--- a/arch/arm/boot/dts/armada-xp-mv78260.dtsi
+++ b/arch/arm/boot/dts/armada-xp-mv78260.dtsi
@@ -48,7 +48,7 @@
/*
* MV78260 has 3 PCIe units Gen2.0: Two units can be
* configured as x4 or quad x1 lanes. One unit is
- * x4/x1.
+ * x4 only.
*/
pcie-controller {
compatible = "marvell,armada-xp-pcie";
@@ -67,7 +67,9 @@
0x82000000 0 0x48000 MBUS_ID(0xf0, 0x01) 0x48000 0 0x00002000 /* Port 0.2 registers */
0x82000000 0 0x4c000 MBUS_ID(0xf0, 0x01) 0x4c000 0 0x00002000 /* Port 0.3 registers */
0x82000000 0 0x80000 MBUS_ID(0xf0, 0x01) 0x80000 0 0x00002000 /* Port 1.0 registers */
- 0x82000000 0 0x82000 MBUS_ID(0xf0, 0x01) 0x82000 0 0x00002000 /* Port 3.0 registers */
+ 0x82000000 0 0x84000 MBUS_ID(0xf0, 0x01) 0x84000 0 0x00002000 /* Port 1.1 registers */
+ 0x82000000 0 0x88000 MBUS_ID(0xf0, 0x01) 0x88000 0 0x00002000 /* Port 1.2 registers */
+ 0x82000000 0 0x8c000 MBUS_ID(0xf0, 0x01) 0x8c000 0 0x00002000 /* Port 1.3 registers */
0x82000000 0x1 0 MBUS_ID(0x04, 0xe8) 0 1 0 /* Port 0.0 MEM */
0x81000000 0x1 0 MBUS_ID(0x04, 0xe0) 0 1 0 /* Port 0.0 IO */
0x82000000 0x2 0 MBUS_ID(0x04, 0xd8) 0 1 0 /* Port 0.1 MEM */
@@ -76,10 +78,18 @@
0x81000000 0x3 0 MBUS_ID(0x04, 0xb0) 0 1 0 /* Port 0.2 IO */
0x82000000 0x4 0 MBUS_ID(0x04, 0x78) 0 1 0 /* Port 0.3 MEM */
0x81000000 0x4 0 MBUS_ID(0x04, 0x70) 0 1 0 /* Port 0.3 IO */
- 0x82000000 0x9 0 MBUS_ID(0x08, 0xe8) 0 1 0 /* Port 1.0 MEM */
- 0x81000000 0x9 0 MBUS_ID(0x08, 0xe0) 0 1 0 /* Port 1.0 IO */
- 0x82000000 0xa 0 MBUS_ID(0x08, 0xf8) 0 1 0 /* Port 3.0 MEM */
- 0x81000000 0xa 0 MBUS_ID(0x08, 0xf0) 0 1 0 /* Port 3.0 IO */>;
+
+ 0x82000000 0x5 0 MBUS_ID(0x08, 0xe8) 0 1 0 /* Port 1.0 MEM */
+ 0x81000000 0x5 0 MBUS_ID(0x08, 0xe0) 0 1 0 /* Port 1.0 IO */
+ 0x82000000 0x6 0 MBUS_ID(0x08, 0xd8) 0 1 0 /* Port 1.1 MEM */
+ 0x81000000 0x6 0 MBUS_ID(0x08, 0xd0) 0 1 0 /* Port 1.1 IO */
+ 0x82000000 0x7 0 MBUS_ID(0x08, 0xb8) 0 1 0 /* Port 1.2 MEM */
+ 0x81000000 0x7 0 MBUS_ID(0x08, 0xb0) 0 1 0 /* Port 1.2 IO */
+ 0x82000000 0x8 0 MBUS_ID(0x08, 0x78) 0 1 0 /* Port 1.3 MEM */
+ 0x81000000 0x8 0 MBUS_ID(0x08, 0x70) 0 1 0 /* Port 1.3 IO */
+
+ 0x82000000 0x9 0 MBUS_ID(0x04, 0xf8) 0 1 0 /* Port 2.0 MEM */
+ 0x81000000 0x9 0 MBUS_ID(0x04, 0xf0) 0 1 0 /* Port 2.0 IO */>;
pcie at 1,0 {
device_type = "pci";
@@ -105,8 +115,8 @@
#address-cells = <3>;
#size-cells = <2>;
#interrupt-cells = <1>;
- ranges = <0x82000000 0 0 0x82000000 0x2 0 1 0
- 0x81000000 0 0 0x81000000 0x2 0 1 0>;
+ ranges = <0x82000000 0 0 0x82000000 0x2 0 1 0
+ 0x81000000 0 0 0x81000000 0x2 0 1 0>;
interrupt-map-mask = <0 0 0 0>;
interrupt-map = <0 0 0 0 &mpic 59>;
marvell,pcie-port = <0>;
@@ -149,37 +159,88 @@
status = "disabled";
};
- pcie at 9,0 {
+ pcie at 5,0 {
device_type = "pci";
- assigned-addresses = <0x82000800 0 0x42000 0 0x2000>;
- reg = <0x4800 0 0 0 0>;
+ assigned-addresses = <0x82000800 0 0x80000 0 0x2000>;
+ reg = <0x2800 0 0 0 0>;
#address-cells = <3>;
#size-cells = <2>;
#interrupt-cells = <1>;
- ranges = <0x82000000 0 0 0x82000000 0x9 0 1 0
- 0x81000000 0 0 0x81000000 0x9 0 1 0>;
+ ranges = <0x82000000 0 0 0x82000000 0x5 0 1 0
+ 0x81000000 0 0 0x81000000 0x5 0 1 0>;
interrupt-map-mask = <0 0 0 0>;
- interrupt-map = <0 0 0 0 &mpic 99>;
- marvell,pcie-port = <2>;
+ interrupt-map = <0 0 0 0 &mpic 62>;
+ marvell,pcie-port = <1>;
marvell,pcie-lane = <0>;
- clocks = <&gateclk 26>;
+ clocks = <&gateclk 9>;
status = "disabled";
};
- pcie at 10,0 {
+ pcie at 6,0 {
device_type = "pci";
- assigned-addresses = <0x82000800 0 0x82000 0 0x2000>;
- reg = <0x5000 0 0 0 0>;
+ assigned-addresses = <0x82000800 0 0x84000 0 0x2000>;
+ reg = <0x3000 0 0 0 0>;
#address-cells = <3>;
#size-cells = <2>;
#interrupt-cells = <1>;
- ranges = <0x82000000 0 0 0x82000000 0xa 0 1 0
- 0x81000000 0 0 0x81000000 0xa 0 1 0>;
+ ranges = <0x82000000 0 0 0x82000000 0x6 0 1 0
+ 0x81000000 0 0 0x81000000 0x6 0 1 0>;
interrupt-map-mask = <0 0 0 0>;
- interrupt-map = <0 0 0 0 &mpic 103>;
- marvell,pcie-port = <3>;
+ interrupt-map = <0 0 0 0 &mpic 63>;
+ marvell,pcie-port = <1>;
+ marvell,pcie-lane = <1>;
+ clocks = <&gateclk 10>;
+ status = "disabled";
+ };
+
+ pcie at 7,0 {
+ device_type = "pci";
+ assigned-addresses = <0x82000800 0 0x88000 0 0x2000>;
+ reg = <0x3800 0 0 0 0>;
+ #address-cells = <3>;
+ #size-cells = <2>;
+ #interrupt-cells = <1>;
+ ranges = <0x82000000 0 0 0x82000000 0x7 0 1 0
+ 0x81000000 0 0 0x81000000 0x7 0 1 0>;
+ interrupt-map-mask = <0 0 0 0>;
+ interrupt-map = <0 0 0 0 &mpic 64>;
+ marvell,pcie-port = <1>;
+ marvell,pcie-lane = <2>;
+ clocks = <&gateclk 11>;
+ status = "disabled";
+ };
+
+ pcie at 8,0 {
+ device_type = "pci";
+ assigned-addresses = <0x82000800 0 0x8c000 0 0x2000>;
+ reg = <0x4000 0 0 0 0>;
+ #address-cells = <3>;
+ #size-cells = <2>;
+ #interrupt-cells = <1>;
+ ranges = <0x82000000 0 0 0x82000000 0x8 0 1 0
+ 0x81000000 0 0 0x81000000 0x8 0 1 0>;
+ interrupt-map-mask = <0 0 0 0>;
+ interrupt-map = <0 0 0 0 &mpic 65>;
+ marvell,pcie-port = <1>;
+ marvell,pcie-lane = <3>;
+ clocks = <&gateclk 12>;
+ status = "disabled";
+ };
+
+ pcie at 9,0 {
+ device_type = "pci";
+ assigned-addresses = <0x82000800 0 0x42000 0 0x2000>;
+ reg = <0x4800 0 0 0 0>;
+ #address-cells = <3>;
+ #size-cells = <2>;
+ #interrupt-cells = <1>;
+ ranges = <0x82000000 0 0 0x82000000 0x9 0 1 0
+ 0x81000000 0 0 0x81000000 0x9 0 1 0>;
+ interrupt-map-mask = <0 0 0 0>;
+ interrupt-map = <0 0 0 0 &mpic 99>;
+ marvell,pcie-port = <2>;
marvell,pcie-lane = <0>;
- clocks = <&gateclk 27>;
+ clocks = <&gateclk 26>;
status = "disabled";
};
};
--
1.8.4.rc3
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 2/2] ARM: mvebu: fix second and third PCIe unit of Armada XP mv78260
2013-11-05 20:46 ` [PATCH 2/2] ARM: mvebu: fix second and third PCIe unit of Armada XP mv78260 Arnaud Ebalard
@ 2013-11-06 14:54 ` Thomas Petazzoni
0 siblings, 0 replies; 20+ messages in thread
From: Thomas Petazzoni @ 2013-11-06 14:54 UTC (permalink / raw)
To: linux-arm-kernel
Dear Arnaud Ebalard,
On Tue, 05 Nov 2013 21:46:02 +0100, Arnaud Ebalard wrote:
> mv78260 flavour of Marvell Armada XP SoC has 3 PCIe units. The
> two first units are both x4 and quad x1 capable. The third unit
> is only x4 capable. This patch fixes mv78260 .dtsi to reflect
> those capabilities.
>
> Signed-off-by: Arnaud Ebalard <arno@natisbad.org>
> ---
> arch/arm/boot/dts/armada-xp-mv78260.dtsi | 109 ++++++++++++++++++++++++-------
> 1 file changed, 85 insertions(+), 24 deletions(-)
Acked-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Same thing: this has been buggy since 3.10, but only 3.11 onwards have
the PCIe driver that uses this DT information. Therefore, pushing this
to 3.11.x and 3.12.x would be fine.
Best regards,
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 0/2] ARM: mvebu: fix DT def. of PCIe units for mv78230 and mv78260
2013-11-05 20:45 [PATCH 0/2] ARM: mvebu: fix DT def. of PCIe units for mv78230 and mv78260 Arnaud Ebalard
2013-11-05 20:45 ` [PATCH 1/2] ARM: mvebu: second PCIe unit of Armada XP mv78230 is only x1 capable Arnaud Ebalard
2013-11-05 20:46 ` [PATCH 2/2] ARM: mvebu: fix second and third PCIe unit of Armada XP mv78260 Arnaud Ebalard
@ 2013-11-06 18:14 ` Arnaud Ebalard
2013-11-06 18:22 ` Thomas Petazzoni
2013-11-22 15:04 ` Jason Cooper
2013-11-23 15:36 ` Jason Cooper
4 siblings, 1 reply; 20+ messages in thread
From: Arnaud Ebalard @ 2013-11-06 18:14 UTC (permalink / raw)
To: linux-arm-kernel
Hi Thomas,
arno at natisbad.org (Arnaud Ebalard) writes:
> As a side note, Thomas, I noticed one more thing, this time in mv78460
> .dtsi when comparing it w/ mv78230 and mv78260 ones. The first address
> of "assigned-address" property varies in the former for each pcie
> node:
>
> $ grep assigned-address armada-xp-mv78230.dtsi
> assigned-addresses = <0x82000800 0 0x40000 0 0x2000>;
> assigned-addresses = <0x82000800 0 0x44000 0 0x2000>;
> assigned-addresses = <0x82000800 0 0x48000 0 0x2000>;
> assigned-addresses = <0x82000800 0 0x4c000 0 0x2000>;
> assigned-addresses = <0x82000800 0 0x80000 0 0x2000>;
> $ grep assigned-address armada-xp-mv78260.dtsi
> assigned-addresses = <0x82000800 0 0x40000 0 0x2000>;
> assigned-addresses = <0x82000800 0 0x44000 0 0x2000>;
> assigned-addresses = <0x82000800 0 0x48000 0 0x2000>;
> assigned-addresses = <0x82000800 0 0x4c000 0 0x2000>;
> assigned-addresses = <0x82000800 0 0x80000 0 0x2000>;
> assigned-addresses = <0x82000800 0 0x84000 0 0x2000>;
> assigned-addresses = <0x82000800 0 0x88000 0 0x2000>;
> assigned-addresses = <0x82000800 0 0x8c000 0 0x2000>;
> assigned-addresses = <0x82000800 0 0x42000 0 0x2000>;
> $ grep assigned-address armada-xp-mv78460.dtsi
> assigned-addresses = <0x82000800 0 0x40000 0 0x2000>;
> assigned-addresses = <0x82001000 0 0x44000 0 0x2000>;
> assigned-addresses = <0x82001800 0 0x48000 0 0x2000>;
> assigned-addresses = <0x82002000 0 0x4c000 0 0x2000>;
> assigned-addresses = <0x82002800 0 0x80000 0 0x2000>;
> assigned-addresses = <0x82003000 0 0x84000 0 0x2000>;
> assigned-addresses = <0x82003800 0 0x88000 0 0x2000>;
> assigned-addresses = <0x82004000 0 0x8c000 0 0x2000>;
> assigned-addresses = <0x82004800 0 0x42000 0 0x2000>;
> assigned-addresses = <0x82005000 0 0x82000 0 0x2000>;
>
> I took at Documentation/devicetree/bindings/pci/mvebu-pci.txt but
> failed to find an answer. Can you explain where the difference comes
> from?
Sorry to bother you again with the one above (it was in the cover
letter) but the difference looks quite odd for someone not familiar
with the syntax and the semantic.
Cheers,
a+
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 0/2] ARM: mvebu: fix DT def. of PCIe units for mv78230 and mv78260
2013-11-06 18:14 ` [PATCH 0/2] ARM: mvebu: fix DT def. of PCIe units for mv78230 and mv78260 Arnaud Ebalard
@ 2013-11-06 18:22 ` Thomas Petazzoni
2013-11-06 18:37 ` Jason Gunthorpe
0 siblings, 1 reply; 20+ messages in thread
From: Thomas Petazzoni @ 2013-11-06 18:22 UTC (permalink / raw)
To: linux-arm-kernel
Dear Arnaud Ebalard,
On Wed, 06 Nov 2013 19:14:42 +0100, Arnaud Ebalard wrote:
> > As a side note, Thomas, I noticed one more thing, this time in mv78460
> > .dtsi when comparing it w/ mv78230 and mv78260 ones. The first address
> > of "assigned-address" property varies in the former for each pcie
> > node:
> >
> > $ grep assigned-address armada-xp-mv78230.dtsi
> > assigned-addresses = <0x82000800 0 0x40000 0 0x2000>;
> > assigned-addresses = <0x82000800 0 0x44000 0 0x2000>;
> > assigned-addresses = <0x82000800 0 0x48000 0 0x2000>;
> > assigned-addresses = <0x82000800 0 0x4c000 0 0x2000>;
> > assigned-addresses = <0x82000800 0 0x80000 0 0x2000>;
> > $ grep assigned-address armada-xp-mv78260.dtsi
> > assigned-addresses = <0x82000800 0 0x40000 0 0x2000>;
> > assigned-addresses = <0x82000800 0 0x44000 0 0x2000>;
> > assigned-addresses = <0x82000800 0 0x48000 0 0x2000>;
> > assigned-addresses = <0x82000800 0 0x4c000 0 0x2000>;
> > assigned-addresses = <0x82000800 0 0x80000 0 0x2000>;
> > assigned-addresses = <0x82000800 0 0x84000 0 0x2000>;
> > assigned-addresses = <0x82000800 0 0x88000 0 0x2000>;
> > assigned-addresses = <0x82000800 0 0x8c000 0 0x2000>;
> > assigned-addresses = <0x82000800 0 0x42000 0 0x2000>;
> > $ grep assigned-address armada-xp-mv78460.dtsi
> > assigned-addresses = <0x82000800 0 0x40000 0 0x2000>;
> > assigned-addresses = <0x82001000 0 0x44000 0 0x2000>;
> > assigned-addresses = <0x82001800 0 0x48000 0 0x2000>;
> > assigned-addresses = <0x82002000 0 0x4c000 0 0x2000>;
> > assigned-addresses = <0x82002800 0 0x80000 0 0x2000>;
> > assigned-addresses = <0x82003000 0 0x84000 0 0x2000>;
> > assigned-addresses = <0x82003800 0 0x88000 0 0x2000>;
> > assigned-addresses = <0x82004000 0 0x8c000 0 0x2000>;
> > assigned-addresses = <0x82004800 0 0x42000 0 0x2000>;
> > assigned-addresses = <0x82005000 0 0x82000 0 0x2000>;
> >
> > I took at Documentation/devicetree/bindings/pci/mvebu-pci.txt but
> > failed to find an answer. Can you explain where the difference comes
> > from?
>
> Sorry to bother you again with the one above (it was in the cover
> letter) but the difference looks quite odd for someone not familiar
> with the syntax and the semantic.
I do still have this question in mind, I wanted to reply to it today
but just hadn't had the time to do it. I'm adding Jason Gunthorpe in Cc
because he knows better than me the precise meaning of
assigned-addresses. If it doesn't reply, I'll try to have a look
tomorrow. This is the kind of stuff I can't answer without
concentrating for a bit of time to re-understand again how it all fits
together :-)
Best regards,
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 0/2] ARM: mvebu: fix DT def. of PCIe units for mv78230 and mv78260
2013-11-06 18:22 ` Thomas Petazzoni
@ 2013-11-06 18:37 ` Jason Gunthorpe
2013-11-06 20:08 ` Arnaud Ebalard
2013-11-06 20:55 ` Jason Cooper
0 siblings, 2 replies; 20+ messages in thread
From: Jason Gunthorpe @ 2013-11-06 18:37 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Nov 06, 2013 at 07:22:33PM +0100, Thomas Petazzoni wrote:
> Dear Arnaud Ebalard,
>
> On Wed, 06 Nov 2013 19:14:42 +0100, Arnaud Ebalard wrote:
>
> > > As a side note, Thomas, I noticed one more thing, this time in mv78460
> > > .dtsi when comparing it w/ mv78230 and mv78260 ones. The first address
> > > of "assigned-address" property varies in the former for each pcie
> > > node:
> > >
> > > $ grep assigned-address armada-xp-mv78230.dtsi
> > > assigned-addresses = <0x82000800 0 0x40000 0 0x2000>;
> > > assigned-addresses = <0x82000800 0 0x44000 0 0x2000>;
> > > assigned-addresses = <0x82000800 0 0x48000 0 0x2000>;
> > > assigned-addresses = <0x82000800 0 0x4c000 0 0x2000>;
> > > assigned-addresses = <0x82000800 0 0x80000 0 0x2000>;
> > > $ grep assigned-address armada-xp-mv78260.dtsi
> > > assigned-addresses = <0x82000800 0 0x40000 0 0x2000>;
> > > assigned-addresses = <0x82000800 0 0x44000 0 0x2000>;
> > > assigned-addresses = <0x82000800 0 0x48000 0 0x2000>;
> > > assigned-addresses = <0x82000800 0 0x4c000 0 0x2000>;
> > > assigned-addresses = <0x82000800 0 0x80000 0 0x2000>;
> > > assigned-addresses = <0x82000800 0 0x84000 0 0x2000>;
> > > assigned-addresses = <0x82000800 0 0x88000 0 0x2000>;
> > > assigned-addresses = <0x82000800 0 0x8c000 0 0x2000>;
> > > assigned-addresses = <0x82000800 0 0x42000 0 0x2000>;
> > > $ grep assigned-address armada-xp-mv78460.dtsi
> > > assigned-addresses = <0x82000800 0 0x40000 0 0x2000>;
> > > assigned-addresses = <0x82001000 0 0x44000 0 0x2000>;
> > > assigned-addresses = <0x82001800 0 0x48000 0 0x2000>;
> > > assigned-addresses = <0x82002000 0 0x4c000 0 0x2000>;
> > > assigned-addresses = <0x82002800 0 0x80000 0 0x2000>;
> > > assigned-addresses = <0x82003000 0 0x84000 0 0x2000>;
> > > assigned-addresses = <0x82003800 0 0x88000 0 0x2000>;
> > > assigned-addresses = <0x82004000 0 0x8c000 0 0x2000>;
> > > assigned-addresses = <0x82004800 0 0x42000 0 0x2000>;
> > > assigned-addresses = <0x82005000 0 0x82000 0 0x2000>;
> > >
> > > I took at Documentation/devicetree/bindings/pci/mvebu-pci.txt but
> > > failed to find an answer. Can you explain where the difference comes
> > > from?
> >
> > Sorry to bother you again with the one above (it was in the cover
> > letter) but the difference looks quite odd for someone not familiar
> > with the syntax and the semantic.
>
> I do still have this question in mind, I wanted to reply to it today
> but just hadn't had the time to do it. I'm adding Jason Gunthorpe in Cc
> because he knows better than me the precise meaning of
> assigned-addresses. If it doesn't reply, I'll try to have a look
> tomorrow. This is the kind of stuff I can't answer without
> concentrating for a bit of time to re-understand again how it all fits
> together :-)
In this context the assigned-addresses encodes the PCI device bus
function as well as the BAR #.
This PCI stuff would be 100% more readable with some macros ..
#define PCI_REG_BDF(bus,device,function) (....)
#define PCI_MEM_BAR_BDF(bus,device,function,bar) \
(0x82000000 | PCI_REG_BDF(bus,device,function) + bar*XX) 0)
So that:
reg = <PCI_REG_BDF(0,1,0) 0 0>
assigned-addresses = <PCI_MEM_BAR_BDF(0,1,0,0) 0x40000 0 0x2000>
When done properly the assigned-address and reg should have the same
BDF. As you've noticed there is a copy and paste mistake in some of
the dts's:
eg:
pcie at 2,0 {
assigned-addresses = <0x82000800 0 0x44000 0 0x2000>;
reg = <0x1000 0 0 0 0>;
The assigned-addresses in that instance should be 0x82001000 to match
the reg.
It probably works because the Linux machinery doesn't check the BDF
value?
The purpose of assigned-address in this specific context is to tell
the driver how to find its control registers. So the 0x44000 above is
the base for the PEX register. The "pci at 3,0", assigned-addresses, and
reg are encoding this statement: "Create a PCI bridge at B:D.F using
the PEX registers at 0x44000" The B:D.F in all three places should be
the same.
Does that sound right Thomas?
I recommend adding some macros and constants derived from the OF PCI
spec. It will save us all headaches :)
Jason
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH 0/2] ARM: mvebu: fix DT def. of PCIe units for mv78230 and mv78260
2013-11-06 18:37 ` Jason Gunthorpe
@ 2013-11-06 20:08 ` Arnaud Ebalard
2013-11-06 20:55 ` Jason Cooper
1 sibling, 0 replies; 20+ messages in thread
From: Arnaud Ebalard @ 2013-11-06 20:08 UTC (permalink / raw)
To: linux-arm-kernel
Hi Jason,
Jason Gunthorpe <jgunthorpe@obsidianresearch.com> writes:
> On Wed, Nov 06, 2013 at 07:22:33PM +0100, Thomas Petazzoni wrote:
>> Dear Arnaud Ebalard,
>>
>> On Wed, 06 Nov 2013 19:14:42 +0100, Arnaud Ebalard wrote:
>>
>> > > As a side note, Thomas, I noticed one more thing, this time in mv78460
>> > > .dtsi when comparing it w/ mv78230 and mv78260 ones. The first address
>> > > of "assigned-address" property varies in the former for each pcie
>> > > node:
>> > >
>> > > $ grep assigned-address armada-xp-mv78230.dtsi
>> > > assigned-addresses = <0x82000800 0 0x40000 0 0x2000>;
>> > > assigned-addresses = <0x82000800 0 0x44000 0 0x2000>;
>> > > assigned-addresses = <0x82000800 0 0x48000 0 0x2000>;
>> > > assigned-addresses = <0x82000800 0 0x4c000 0 0x2000>;
>> > > assigned-addresses = <0x82000800 0 0x80000 0 0x2000>;
>> > > $ grep assigned-address armada-xp-mv78260.dtsi
>> > > assigned-addresses = <0x82000800 0 0x40000 0 0x2000>;
>> > > assigned-addresses = <0x82000800 0 0x44000 0 0x2000>;
>> > > assigned-addresses = <0x82000800 0 0x48000 0 0x2000>;
>> > > assigned-addresses = <0x82000800 0 0x4c000 0 0x2000>;
>> > > assigned-addresses = <0x82000800 0 0x80000 0 0x2000>;
>> > > assigned-addresses = <0x82000800 0 0x84000 0 0x2000>;
>> > > assigned-addresses = <0x82000800 0 0x88000 0 0x2000>;
>> > > assigned-addresses = <0x82000800 0 0x8c000 0 0x2000>;
>> > > assigned-addresses = <0x82000800 0 0x42000 0 0x2000>;
>> > > $ grep assigned-address armada-xp-mv78460.dtsi
>> > > assigned-addresses = <0x82000800 0 0x40000 0 0x2000>;
>> > > assigned-addresses = <0x82001000 0 0x44000 0 0x2000>;
>> > > assigned-addresses = <0x82001800 0 0x48000 0 0x2000>;
>> > > assigned-addresses = <0x82002000 0 0x4c000 0 0x2000>;
>> > > assigned-addresses = <0x82002800 0 0x80000 0 0x2000>;
>> > > assigned-addresses = <0x82003000 0 0x84000 0 0x2000>;
>> > > assigned-addresses = <0x82003800 0 0x88000 0 0x2000>;
>> > > assigned-addresses = <0x82004000 0 0x8c000 0 0x2000>;
>> > > assigned-addresses = <0x82004800 0 0x42000 0 0x2000>;
>> > > assigned-addresses = <0x82005000 0 0x82000 0 0x2000>;
>> > >
>> > > I took at Documentation/devicetree/bindings/pci/mvebu-pci.txt but
>> > > failed to find an answer. Can you explain where the difference comes
>> > > from?
>> >
>> > Sorry to bother you again with the one above (it was in the cover
>> > letter) but the difference looks quite odd for someone not familiar
>> > with the syntax and the semantic.
>>
>> I do still have this question in mind, I wanted to reply to it today
>> but just hadn't had the time to do it. I'm adding Jason Gunthorpe in Cc
>> because he knows better than me the precise meaning of
>> assigned-addresses. If it doesn't reply, I'll try to have a look
>> tomorrow. This is the kind of stuff I can't answer without
>> concentrating for a bit of time to re-understand again how it all fits
>> together :-)
>
> In this context the assigned-addresses encodes the PCI device bus
> function as well as the BAR #.
>
> This PCI stuff would be 100% more readable with some macros ..
>
> #define PCI_REG_BDF(bus,device,function) (....)
> #define PCI_MEM_BAR_BDF(bus,device,function,bar) \
> (0x82000000 | PCI_REG_BDF(bus,device,function) + bar*XX) 0)
>
> So that:
> reg = <PCI_REG_BDF(0,1,0) 0 0>
> assigned-addresses = <PCI_MEM_BAR_BDF(0,1,0,0) 0x40000 0 0x2000>
>
> When done properly the assigned-address and reg should have the same
> BDF. As you've noticed there is a copy and paste mistake in some of
> the dts's:
>
> eg:
> pcie at 2,0 {
> assigned-addresses = <0x82000800 0 0x44000 0 0x2000>;
> reg = <0x1000 0 0 0 0>;
>
> The assigned-addresses in that instance should be 0x82001000 to match
> the reg.
>
> It probably works because the Linux machinery doesn't check the BDF
> value?
ok.
> The purpose of assigned-address in this specific context is to tell
> the driver how to find its control registers. So the 0x44000 above is
> the base for the PEX register. The "pci at 3,0", assigned-addresses, and
> reg are encoding this statement: "Create a PCI bridge at B:D.F using
> the PEX registers at 0x44000" The B:D.F in all three places should be
> the same.
Thanks for the explanation.
> I recommend adding some macros and constants derived from the OF PCI
> spec. It will save us all headaches :)
Too bad it was not that clear in mvebu-pci.txt; I could have taken the
opportunity to fix mv782{30,60} .dtsi in two previous patches.
Cheers,
a+
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH 0/2] ARM: mvebu: fix DT def. of PCIe units for mv78230 and mv78260
2013-11-06 18:37 ` Jason Gunthorpe
2013-11-06 20:08 ` Arnaud Ebalard
@ 2013-11-06 20:55 ` Jason Cooper
2013-11-06 21:32 ` Arnaud Ebalard
1 sibling, 1 reply; 20+ messages in thread
From: Jason Cooper @ 2013-11-06 20:55 UTC (permalink / raw)
To: linux-arm-kernel
Jason,
On Wed, Nov 06, 2013 at 11:37:24AM -0700, Jason Gunthorpe wrote:
...
> This PCI stuff would be 100% more readable with some macros ..
>
> #define PCI_REG_BDF(bus,device,function) (....)
> #define PCI_MEM_BAR_BDF(bus,device,function,bar) \
> (0x82000000 | PCI_REG_BDF(bus,device,function) + bar*XX) 0)
>
> So that:
> reg = <PCI_REG_BDF(0,1,0) 0 0>
> assigned-addresses = <PCI_MEM_BAR_BDF(0,1,0,0) 0x40000 0 0x2000>
Do I smell a patch brewing? :)
Arnaud, would you like to take a crack at it?
thx,
Jason.
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 0/2] ARM: mvebu: fix DT def. of PCIe units for mv78230 and mv78260
2013-11-06 20:55 ` Jason Cooper
@ 2013-11-06 21:32 ` Arnaud Ebalard
0 siblings, 0 replies; 20+ messages in thread
From: Arnaud Ebalard @ 2013-11-06 21:32 UTC (permalink / raw)
To: linux-arm-kernel
Hi Jason,
Jason Cooper <jason@lakedaemon.net> writes:
> On Wed, Nov 06, 2013 at 11:37:24AM -0700, Jason Gunthorpe wrote:
> ...
>> This PCI stuff would be 100% more readable with some macros ..
>>
>> #define PCI_REG_BDF(bus,device,function) (....)
>> #define PCI_MEM_BAR_BDF(bus,device,function,bar) \
>> (0x82000000 | PCI_REG_BDF(bus,device,function) + bar*XX) 0)
>>
>> So that:
>> reg = <PCI_REG_BDF(0,1,0) 0 0>
>> assigned-addresses = <PCI_MEM_BAR_BDF(0,1,0,0) 0x40000 0 0x2000>
>
> Do I smell a patch brewing? :)
>
> Arnaud, would you like to take a crack at it?
My understanding of the topic being in the end limited to what I learnt
from recent discussions, I'll pass on that one but I'll be happy to test
on a mv78230.
Cheers,
a+
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 0/2] ARM: mvebu: fix DT def. of PCIe units for mv78230 and mv78260
2013-11-05 20:45 [PATCH 0/2] ARM: mvebu: fix DT def. of PCIe units for mv78230 and mv78260 Arnaud Ebalard
` (2 preceding siblings ...)
2013-11-06 18:14 ` [PATCH 0/2] ARM: mvebu: fix DT def. of PCIe units for mv78230 and mv78260 Arnaud Ebalard
@ 2013-11-22 15:04 ` Jason Cooper
2013-11-22 15:29 ` Arnaud Ebalard
2013-11-23 15:36 ` Jason Cooper
4 siblings, 1 reply; 20+ messages in thread
From: Jason Cooper @ 2013-11-22 15:04 UTC (permalink / raw)
To: linux-arm-kernel
Arnaud, Thomas,
On Tue, Nov 05, 2013 at 09:45:33PM +0100, Arnaud Ebalard wrote:
> Second patch fixes mv78260 .dtsi to reflect the fact that the two first
> units are x4 and quad x1 capable, and third (and last) interface is x4
> only. This patch has only been compiled and not tested on real hardware.
> As associated changes are quite error-prone, I think it needs some
> careful review.
This, in addition to changing the .dtsi files is where my "let's take
time merging this" came from.
...
> Arnaud Ebalard (2):
> ARM: mvebu: second PCIe unit of Armada XP mv78230 is only x1 capable
> ARM: mvebu: fix second and third PCIe units of Armada XP mv78260
>
> arch/arm/boot/dts/armada-xp-mv78230.dtsi | 24 +++----
> arch/arm/boot/dts/armada-xp-mv78260.dtsi | 109 ++++++++++++++++++++++++-------
> 2 files changed, 97 insertions(+), 36 deletions(-)
At any rate, since these are going to -stable anyway, there's no need to
rush it. These are currently queued up as fixes for v3.13-rc1.
thx,
Jason.
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH 0/2] ARM: mvebu: fix DT def. of PCIe units for mv78230 and mv78260
2013-11-22 15:04 ` Jason Cooper
@ 2013-11-22 15:29 ` Arnaud Ebalard
0 siblings, 0 replies; 20+ messages in thread
From: Arnaud Ebalard @ 2013-11-22 15:29 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
Jason Cooper <jason@lakedaemon.net> writes:
> On Tue, Nov 05, 2013 at 09:45:33PM +0100, Arnaud Ebalard wrote:
>> Second patch fixes mv78260 .dtsi to reflect the fact that the two first
>> units are x4 and quad x1 capable, and third (and last) interface is x4
>> only. This patch has only been compiled and not tested on real hardware.
>> As associated changes are quite error-prone, I think it needs some
>> careful review.
>
> This, in addition to changing the .dtsi files is where my "let's take
> time merging this" came from.
>
> ...
>> Arnaud Ebalard (2):
>> ARM: mvebu: second PCIe unit of Armada XP mv78230 is only x1 capable
>> ARM: mvebu: fix second and third PCIe units of Armada XP mv78260
>>
>> arch/arm/boot/dts/armada-xp-mv78230.dtsi | 24 +++----
>> arch/arm/boot/dts/armada-xp-mv78260.dtsi | 109 ++++++++++++++++++++++++-------
>> 2 files changed, 97 insertions(+), 36 deletions(-)
>
> At any rate, since these are going to -stable anyway, there's no need to
> rush it. These are currently queued up as fixes for v3.13-rc1.
fine with me.
Cheers,
a+
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 0/2] ARM: mvebu: fix DT def. of PCIe units for mv78230 and mv78260
2013-11-05 20:45 [PATCH 0/2] ARM: mvebu: fix DT def. of PCIe units for mv78230 and mv78260 Arnaud Ebalard
` (3 preceding siblings ...)
2013-11-22 15:04 ` Jason Cooper
@ 2013-11-23 15:36 ` Jason Cooper
4 siblings, 0 replies; 20+ messages in thread
From: Jason Cooper @ 2013-11-23 15:36 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Nov 05, 2013 at 09:45:33PM +0100, Arnaud Ebalard wrote:
> Arnaud Ebalard (2):
> ARM: mvebu: second PCIe unit of Armada XP mv78230 is only x1 capable
> ARM: mvebu: fix second and third PCIe units of Armada XP mv78260
>
> arch/arm/boot/dts/armada-xp-mv78230.dtsi | 24 +++----
> arch/arm/boot/dts/armada-xp-mv78260.dtsi | 109 ++++++++++++++++++++++++-------
> 2 files changed, 97 insertions(+), 36 deletions(-)
Whole series applied to mvebu/dt-fixes with Thomas' Ack and Cc'd to
stable for v3.10.x and up.
thx,
Jason.
^ permalink raw reply [flat|nested] 20+ messages in thread