| Message ID | b5f36f547bb12ed4e5398401685523625ca50312.1383681717.git.arno@natisbad.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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+
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.
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
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+
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.
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
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@1,0 { device_type = "pci"; @@ -144,20 +144,20 @@ status = "disabled"; }; - pcie@9,0 { + pcie@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"; }; };
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(-)