On 08/08/2017 06:43 AM, Marcel Ziswiler wrote:
From: Marcel Ziswiler <marcel.ziswi...@toradex.com>

Allow optionally bringing up the Apalis type specific 4 lane PCIe port
as well as the PCIe switch as found on the Apalis Evaluation board. In
order to avoid violating the PCIe reset timing do this by overriding the
tegra_pcie_board_port_reset() function. Note however that both the
Apalis type specific 4 lane PCIe port as well as the regular Apalis PCIe
port are also left disabled in the device tree by default.

diff --git a/board/toradex/apalis_t30/apalis_t30.c 

+#define PEX_PERST_N    TEGRA_GPIO(S, 7) /* Apalis GPIO7 */
+#define RESET_MOCI_CTRL        TEGRA_GPIO(I, 4)

Shouldn't that be a CONFIG_xxx option (defined in Kconfig) rather than something new added to the old-style config header?

+void tegra_pcie_board_port_reset(void *port)
+       int index = tegra_pcie_port_index_of_port(port);
+       if (index == 2) { /* I210 Gigabit Ethernet Controller (On-module) */
+               tegra_pcie_port_reset(port);

I think it'd read better if the } were here rather than wrapping it onto the else line; a special case where ifdefs are involved.

+       } else if (index == 1) { /* Apalis PCIe */
+               /*
+                * As port 0 and 1 share the same RESET_MOCI only assert it once
+                * for both ports below to avoid loosing the previously brought
+                * up port again.
+                */

How do you know that port 0 gets reset first then port 1? Also, how do you know that the user isn't going to initialize PCIe multiple times and expect the HW to get reset when they do? This HW design seems fragile, but I suppose it's rather difficult to change already-shipped HW:-) To address some of the issues, does it make sense to keep state in this function and do the reset the first time either index 0 or 1 is reset, and not the second time, so this code doesn't care about the order? To account for re-initialization, you could perhaps do the reset every even (second) time the function is called for index 0 or 1, not just the first.

Aside from that, this series seems fine.
U-Boot mailing list

Reply via email to