Re: [U-Boot] [PATCH v0 1/5] pci: mvebu: Fix Armada 38x support

2015-11-18 Thread Dirk Eibach 
Hi Anton,

2015-11-17 13:55 GMT+01:00 Anton Schubert :
> Hi Dirk,
>
> 2015-10-28 16:44 GMT+01:00 :
>>
>> From: Dirk Eibach 
>>
>> @@ -344,7 +345,6 @@ void pci_init_board(void)
>>
>> /* Don't read at all from pci registers if port power is
>> down */
>> if (pcie->lane == 0 && SELECT(soc_ctrl, pcie->port) == 0)
>> {
>> -   i += 3;
>> debug("%s: skipping port %d\n", __func__,
>> pcie->port);
>> continue;
>> }
>
>
> Is there a specific reason why you removed this line or was it just by
> mistake? Because I think doing so would break Armada XP in certain Serdes
> Configurations, as it doesn't like it's PCI registers being read if the port
> is off.

I assume the idea is to go to the next port if the current port is
disabled. But adding 3 to the index does not seem to be the right
thing to do, since Armada XP has ports with 4 lanes, but also with
ports with one lane.
I assume that iterating over all lanes would not be a problem, but by
mistake the pcie->lane == 0  was left in the condition. So this should
perform better:

/* Don't read at all from pci registers if port power is down */
if (SELECT(soc_ctrl, pcie->port) == 0) {
if (pcie->lane == 0)
debug("%s: skipping port %d\n", __func__, pcie->port);
continue;
}

What do you think?

Cheers
Dirk
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v0 1/5] pci: mvebu: Fix Armada 38x support

2015-11-18 Thread Dirk Eibach 
2015-11-18 14:23 GMT+01:00 Anton Schubert :
> Am 18.11.2015 um 13:48 schrieb Dirk Eibach:
>> I assume the idea is to go to the next port if the current port is
>> disabled. But adding 3 to the index does not seem to be the right
>> thing to do, since Armada XP has ports with 4 lanes, but also with
>> ports with one lane.
>> I assume that iterating over all lanes would not be a problem, but by
>> mistake the pcie->lane == 0  was left in the condition.
> Yeah you are right. The additional condition was superfluous in the
> original version.
>
>> So this should perform better:
>>
>> /* Don't read at all from pci registers if port power is down */
>> if (SELECT(soc_ctrl, pcie->port) == 0) {
>> if (pcie->lane == 0)
>> debug("%s: skipping port %d\n", __func__, pcie->port);
>> continue;
>> }
> I agree.

Fine. I will add this to the v1 series.

Cheers
Dirk
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v0 1/5] pci: mvebu: Fix Armada 38x support

2015-11-18 Thread Anton Schubert 
Am 18.11.2015 um 13:48 schrieb Dirk Eibach:
> I assume the idea is to go to the next port if the current port is
> disabled. But adding 3 to the index does not seem to be the right
> thing to do, since Armada XP has ports with 4 lanes, but also with
> ports with one lane.
> I assume that iterating over all lanes would not be a problem, but by
> mistake the pcie->lane == 0  was left in the condition. 
Yeah you are right. The additional condition was superfluous in the
original version.

> So this should perform better:
>
> /* Don't read at all from pci registers if port power is down */
> if (SELECT(soc_ctrl, pcie->port) == 0) {
> if (pcie->lane == 0)
> debug("%s: skipping port %d\n", __func__, pcie->port);
> continue;
> }
I agree.

Regards,
Anton
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v0 1/5] pci: mvebu: Fix Armada 38x support

2015-11-17 Thread Anton Schubert 
Hi Dirk,

2015-10-28 16:44 GMT+01:00 :

> From: Dirk Eibach 
>
> @@ -344,7 +345,6 @@ void pci_init_board(void)
>
> /* Don't read at all from pci registers if port power is
> down */
> if (pcie->lane == 0 && SELECT(soc_ctrl, pcie->port) == 0) {
> -   i += 3;
> debug("%s: skipping port %d\n", __func__,
> pcie->port);
> continue;
> }
>

Is there a specific reason why you removed this line or was it just by
mistake? Because I think doing so would break Armada XP in certain Serdes
Configurations, as it doesn't like it's PCI registers being read if the
port is off.

Kind Regards,
Anton
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v0 1/5] pci: mvebu: Fix Armada 38x support

2015-10-28 Thread dirk . eibach 
From: Dirk Eibach 

Armada 38x has 4 pci ports, not 3.
The optimization in pci_init_board() seems to assume,
that every port has 3 lanes. This is obviously wrong
and breaks support for Armada 38x.

Signed-off-by: Dirk Eibach 
---

 arch/arm/mach-mvebu/include/mach/soc.h |  1 +
 drivers/pci/pci_mvebu.c| 22 +++---
 2 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/arch/arm/mach-mvebu/include/mach/soc.h 
b/arch/arm/mach-mvebu/include/mach/soc.h
index 02c21bc..a1014b3 100644
--- a/arch/arm/mach-mvebu/include/mach/soc.h
+++ b/arch/arm/mach-mvebu/include/mach/soc.h
@@ -67,6 +67,7 @@
 #define MVEBU_USB20_BASE   (MVEBU_REGISTER(0x58000))
 #define MVEBU_EGIGA0_BASE  (MVEBU_REGISTER(0x7))
 #define MVEBU_EGIGA1_BASE  (MVEBU_REGISTER(0x74000))
+#define MVEBU_REG_PCIE0_BASE   (MVEBU_REGISTER(0x8))
 #define MVEBU_AXP_SATA_BASE(MVEBU_REGISTER(0xa))
 #define MVEBU_SATA0_BASE   (MVEBU_REGISTER(0xa8000))
 #define MVEBU_NAND_BASE(MVEBU_REGISTER(0xd))
diff --git a/drivers/pci/pci_mvebu.c b/drivers/pci/pci_mvebu.c
index fd2744d..50e6419 100644
--- a/drivers/pci/pci_mvebu.c
+++ b/drivers/pci/pci_mvebu.c
@@ -90,26 +90,27 @@ static void __iomem *mvebu_pcie_membase = (void __iomem 
*)MBUS_PCI_MEM_BASE;
 
 #if defined(CONFIG_ARMADA_38X)
 #define PCIE_BASE(if)  \
-   ((if) == 0 ?\
-MVEBU_REG_PCIE_BASE + 0x4 :\
-MVEBU_REG_PCIE_BASE + 0x4000 * (if))
+   ((if) == 0 ?\
+   MVEBU_REG_PCIE0_BASE : \
+   (MVEBU_REG_PCIE_BASE + 0x4000 * (if - 1)))
 
 /*
  * On A38x MV6820 these PEX ports are supported:
  *  0 - Port 0.0
- *  1 - Port 0.1
- *  2 - Port 0.2
+ *  1 - Port 1.0
+ *  2 - Port 2.0
+ *  3 - Port 3.0
  */
-#define MAX_PEX 3
+#define MAX_PEX 4
 static struct mvebu_pcie pcie_bus[MAX_PEX];
 
 static void mvebu_get_port_lane(struct mvebu_pcie *pcie, int pex_idx,
int *mem_target, int *mem_attr)
 {
-   u8 port[] = { 0, 1, 2 };
-   u8 lane[] = { 0, 0, 0 };
-   u8 target[] = { 8, 4, 4 };
-   u8 attr[] = { 0xe8, 0xe8, 0xd8 };
+   u8 port[] = { 0, 1, 2, 3 };
+   u8 lane[] = { 0, 0, 0, 0 };
+   u8 target[] = { 8, 4, 4, 4 };
+   u8 attr[] = { 0xe8, 0xe8, 0xd8, 0xb8 };
 
pcie->port = port[pex_idx];
pcie->lane = lane[pex_idx];
@@ -344,7 +345,6 @@ void pci_init_board(void)
 
/* Don't read at all from pci registers if port power is down */
if (pcie->lane == 0 && SELECT(soc_ctrl, pcie->port) == 0) {
-   i += 3;
debug("%s: skipping port %d\n", __func__, pcie->port);
continue;
}
-- 
2.1.3

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v0 1/5] pci: mvebu: Fix Armada 38x support

2015-10-28 Thread Stefan Roese 

Hi Dirk,

On 28.10.2015 16:44, dirk.eib...@gdsys.cc wrote:

From: Dirk Eibach 

Armada 38x has 4 pci ports, not 3.
The optimization in pci_init_board() seems to assume,
that every port has 3 lanes. This is obviously wrong
and breaks support for Armada 38x.

Signed-off-by: Dirk Eibach 


Looks good, so:

Reviewed-by: Stefan Roese 

Thanks,
Stefan

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot