public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
* [U-Boot] [PATCH 0/5] fix apalis-tk1 pcie gigabit ethernet operation
@ 2017-08-04 16:09 Marcel Ziswiler
  2017-08-04 16:10 ` [U-Boot] [PATCH 1/5] configs: apalis-tk1: fix boot failure using ext4 rootfs Marcel Ziswiler
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Marcel Ziswiler @ 2017-08-04 16:09 UTC (permalink / raw)
  To: u-boot


This series addresses a gigabit Ethernet reliability issue as observed
on Apalis TK1 related to a PCIe reset timing violation.

This series depends on Simon's work available at u-boot-dm/master plus
my previous series "move apalis t30/tk1, colibri t20/t30 to livetree".

This series is available at http://git.toradex.com/cgit/u-boot-toradex.git/log/?h=for-next


Marcel Ziswiler (4):
  apalis-tk1: add missing as3722 gpio0 configuration
  pci: tegra: make tegra_pcie_port_reset weak function with explicit
    index
  power: as3722: add as3722_ldo_set_voltage signature to header file
  apalis-tk1: fix pcie reset for reliable gigabit ethernet operation

Sanchayan Maity (1):
  configs: apalis-tk1: fix boot failure using ext4 rootfs

 arch/arm/dts/tegra124-apalis.dts      |   6 +-
 board/toradex/apalis-tk1/apalis-tk1.c | 241 ++++++++++++++++++++--------------
 drivers/pci/pci_tegra.c               |   7 +-
 include/configs/apalis-tk1.h          |   4 +-
 include/power/as3722.h                |   1 +
 5 files changed, 156 insertions(+), 103 deletions(-)

-- 
2.9.4

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [U-Boot] [PATCH 1/5] configs: apalis-tk1: fix boot failure using ext4 rootfs
  2017-08-04 16:09 [U-Boot] [PATCH 0/5] fix apalis-tk1 pcie gigabit ethernet operation Marcel Ziswiler
@ 2017-08-04 16:10 ` Marcel Ziswiler
  2017-08-04 16:10 ` [U-Boot] [PATCH 2/5] apalis-tk1: add missing as3722 gpio0 configuration Marcel Ziswiler
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Marcel Ziswiler @ 2017-08-04 16:10 UTC (permalink / raw)
  To: u-boot

From: Sanchayan Maity <maitysanchayan@gmail.com>

Trying to boot from an ext4 rootfs fails due to us defaulting to ext3.
While the downstream T20/T30 L4T kernel has issues with ext4 later TK1
L4T should work just fine with it. Hence enable ext4 for sdboot and
usbboot on TK1.

Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
Acked-by: Marcel Ziswiler <marcel.ziswiler@toradex.com>
---

 include/configs/apalis-tk1.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/configs/apalis-tk1.h b/include/configs/apalis-tk1.h
index d6b226c..bb46768 100644
--- a/include/configs/apalis-tk1.h
+++ b/include/configs/apalis-tk1.h
@@ -87,7 +87,7 @@
 		"&& setenv dtbparam ${fdt_addr_r}\0"
 
 #define SD_BOOTCMD \
-	"sdargs=ip=off root=/dev/mmcblk1p2 rw rootfstype=ext3 rootwait\0" \
+	"sdargs=ip=off root=/dev/mmcblk1p2 rw rootfstype=ext4 rootwait\0" \
 	"sdboot=run setup; setenv bootargs ${defargs} ${sdargs} ${setupargs} " \
 		"${vidargs}; echo Booting from SD card in 8bit slot...; " \
 		"run sddtbload; load mmc 1:1 ${kernel_addr_r} " \
@@ -98,7 +98,7 @@
 		"&& setenv dtbparam ${fdt_addr_r}\0"
 
 #define USB_BOOTCMD \
-	"usbargs=ip=off root=/dev/sda2 rw rootfstype=ext3 rootwait\0" \
+	"usbargs=ip=off root=/dev/sda2 rw rootfstype=ext4 rootwait\0" \
 	"usbboot=run setup; setenv bootargs ${defargs} ${setupargs} " \
 		"${usbargs} ${vidargs}; echo Booting from USB stick...; " \
 		"usb start && run usbdtbload; load usb 0:1 ${kernel_addr_r} " \
-- 
2.9.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [U-Boot] [PATCH 2/5] apalis-tk1: add missing as3722 gpio0 configuration
  2017-08-04 16:09 [U-Boot] [PATCH 0/5] fix apalis-tk1 pcie gigabit ethernet operation Marcel Ziswiler
  2017-08-04 16:10 ` [U-Boot] [PATCH 1/5] configs: apalis-tk1: fix boot failure using ext4 rootfs Marcel Ziswiler
@ 2017-08-04 16:10 ` Marcel Ziswiler
  2017-08-06  5:16   ` Simon Glass
  2017-08-04 16:10 ` [U-Boot] [PATCH 3/5] pci: tegra: make tegra_pcie_port_reset weak function with explicit index Marcel Ziswiler
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 10+ messages in thread
From: Marcel Ziswiler @ 2017-08-04 16:10 UTC (permalink / raw)
  To: u-boot

From: Marcel Ziswiler <marcel.ziswiler@toradex.com>

As the AS3722 GPIO0 is also a not connected on our Apalis TK1 module
explicitly configure it to high-impedance as well.

Signed-off-by: Marcel Ziswiler <marcel.ziswiler@toradex.com>
---

 arch/arm/dts/tegra124-apalis.dts | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/dts/tegra124-apalis.dts b/arch/arm/dts/tegra124-apalis.dts
index 2fc0384..fe08d3e 100644
--- a/arch/arm/dts/tegra124-apalis.dts
+++ b/arch/arm/dts/tegra124-apalis.dts
@@ -1683,9 +1683,9 @@
 					bias-pull-up;
 				};
 
-				gpio1_3_4_5_6 {
-					pins = "gpio1", "gpio3", "gpio4",
-					       "gpio5", "gpio6";
+				gpio0_1_3_4_5_6 {
+					pins = "gpio0", "gpio1", "gpio3",
+					       "gpio4", "gpio5", "gpio6";
 					bias-high-impedance;
 				};
 			};
-- 
2.9.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [U-Boot] [PATCH 3/5] pci: tegra: make tegra_pcie_port_reset weak function with explicit index
  2017-08-04 16:09 [U-Boot] [PATCH 0/5] fix apalis-tk1 pcie gigabit ethernet operation Marcel Ziswiler
  2017-08-04 16:10 ` [U-Boot] [PATCH 1/5] configs: apalis-tk1: fix boot failure using ext4 rootfs Marcel Ziswiler
  2017-08-04 16:10 ` [U-Boot] [PATCH 2/5] apalis-tk1: add missing as3722 gpio0 configuration Marcel Ziswiler
@ 2017-08-04 16:10 ` Marcel Ziswiler
  2017-08-04 16:33   ` Stephen Warren
  2017-08-04 16:10 ` [U-Boot] [PATCH 4/5] power: as3722: add as3722_ldo_set_voltage signature to header file Marcel Ziswiler
  2017-08-04 16:10 ` [U-Boot] [PATCH 5/5] apalis-tk1: fix pcie reset for reliable gigabit ethernet operation Marcel Ziswiler
  4 siblings, 1 reply; 10+ messages in thread
From: Marcel Ziswiler @ 2017-08-04 16:10 UTC (permalink / raw)
  To: u-boot

From: Marcel Ziswiler <marcel.ziswiler@toradex.com>

Make tegra_pcie_port_reset() a weak function with an explicit index
parameter. This allows overriding the PCIe port reset functionality
from board specific code as e.g. required for Apalis TK1.

Signed-off-by: Marcel Ziswiler <marcel.ziswiler@toradex.com>
---

 drivers/pci/pci_tegra.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/pci_tegra.c b/drivers/pci/pci_tegra.c
index cb5cf8b..d373a88 100644
--- a/drivers/pci/pci_tegra.c
+++ b/drivers/pci/pci_tegra.c
@@ -893,7 +893,8 @@ static unsigned long tegra_pcie_port_get_pex_ctrl(struct tegra_pcie_port *port)
 	return ret;
 }
 
-static void tegra_pcie_port_reset(struct tegra_pcie_port *port)
+void __weak tegra_pcie_port_reset(struct tegra_pcie_port *port,
+				  unsigned int index)
 {
 	unsigned long ctrl = tegra_pcie_port_get_pex_ctrl(port);
 	unsigned long value;
@@ -928,7 +929,7 @@ static void tegra_pcie_port_enable(struct tegra_pcie_port *port)
 
 	afi_writel(pcie, value, ctrl);
 
-	tegra_pcie_port_reset(port);
+	tegra_pcie_port_reset(port, port->index);
 
 	if (soc->force_pca_enable) {
 		value = rp_readl(port, RP_VEND_CTL2);
@@ -979,7 +980,7 @@ static bool tegra_pcie_port_check_link(struct tegra_pcie_port *port)
 		} while (--timeout);
 
 retry:
-		tegra_pcie_port_reset(port);
+		tegra_pcie_port_reset(port, port->index);
 	} while (--retries);
 
 	return false;
-- 
2.9.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [U-Boot] [PATCH 4/5] power: as3722: add as3722_ldo_set_voltage signature to header file
  2017-08-04 16:09 [U-Boot] [PATCH 0/5] fix apalis-tk1 pcie gigabit ethernet operation Marcel Ziswiler
                   ` (2 preceding siblings ...)
  2017-08-04 16:10 ` [U-Boot] [PATCH 3/5] pci: tegra: make tegra_pcie_port_reset weak function with explicit index Marcel Ziswiler
@ 2017-08-04 16:10 ` Marcel Ziswiler
  2017-08-06  5:16   ` Simon Glass
  2017-08-04 16:10 ` [U-Boot] [PATCH 5/5] apalis-tk1: fix pcie reset for reliable gigabit ethernet operation Marcel Ziswiler
  4 siblings, 1 reply; 10+ messages in thread
From: Marcel Ziswiler @ 2017-08-04 16:10 UTC (permalink / raw)
  To: u-boot

From: Marcel Ziswiler <marcel.ziswiler@toradex.com>

Just like the already present as3722_sd_set_voltage() add the currently
missing signature of the as3722_ldo_set_voltage() function to its header
file.

Signed-off-by: Marcel Ziswiler <marcel.ziswiler@toradex.com>
---

 include/power/as3722.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/power/as3722.h b/include/power/as3722.h
index cb4b188..43af9b2 100644
--- a/include/power/as3722.h
+++ b/include/power/as3722.h
@@ -25,5 +25,6 @@
 #define AS3722_GPIO_CONTROL_INVERT (1 << 7)
 
 int as3722_sd_set_voltage(struct udevice *dev, unsigned int sd, u8 value);
+int as3722_ldo_set_voltage(struct udevice *dev, unsigned int ldo, u8 value);
 
 #endif /* __POWER_AS3722_H__ */
-- 
2.9.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [U-Boot] [PATCH 5/5] apalis-tk1: fix pcie reset for reliable gigabit ethernet operation
  2017-08-04 16:09 [U-Boot] [PATCH 0/5] fix apalis-tk1 pcie gigabit ethernet operation Marcel Ziswiler
                   ` (3 preceding siblings ...)
  2017-08-04 16:10 ` [U-Boot] [PATCH 4/5] power: as3722: add as3722_ldo_set_voltage signature to header file Marcel Ziswiler
@ 2017-08-04 16:10 ` Marcel Ziswiler
  4 siblings, 0 replies; 10+ messages in thread
From: Marcel Ziswiler @ 2017-08-04 16:10 UTC (permalink / raw)
  To: u-boot

From: Marcel Ziswiler <marcel.ziswiler@toradex.com>

It turns out that the current PCIe reset implementation in the PCIe
board init function is not quite working reliably due to PCIe reset
timing violations. Fix this by overriding the tegra_pcie_port_reset()
function.

Also allow optionally bringing up the PCIe switch as found on the Apalis
Evaluation board. Note however that the Apalis PCIe port is also left
disabled in the device tree by default.

Signed-off-by: Marcel Ziswiler <marcel.ziswiler@toradex.com>
---

 board/toradex/apalis-tk1/apalis-tk1.c | 241 ++++++++++++++++++++--------------
 1 file changed, 146 insertions(+), 95 deletions(-)

diff --git a/board/toradex/apalis-tk1/apalis-tk1.c b/board/toradex/apalis-tk1/apalis-tk1.c
index 5de61e7..13449ce 100644
--- a/board/toradex/apalis-tk1/apalis-tk1.c
+++ b/board/toradex/apalis-tk1/apalis-tk1.c
@@ -5,17 +5,25 @@
  */
 
 #include <common.h>
+#include <dm.h>
 #include <asm/arch-tegra/ap.h>
 #include <asm/gpio.h>
 #include <asm/io.h>
 #include <asm/arch/gpio.h>
 #include <asm/arch/pinmux.h>
 #include <power/as3722.h>
+#include <power/pmic.h>
 
 #include "../common/tdx-common.h"
 #include "pinmux-config-apalis-tk1.h"
 
-#define LAN_RESET_N TEGRA_GPIO(S, 2)
+#define LAN_DEV_OFF_N	TEGRA_GPIO(O, 6)
+#define LAN_RESET_N	TEGRA_GPIO(S, 2)
+#define LAN_WAKE_N	TEGRA_GPIO(O, 5)
+#ifdef APALIS_TK1_PCIE_EVALBOARD_INIT
+#define PEX_PERST_N	TEGRA_GPIO(DD, 1) /* Apalis GPIO7 */
+#define RESET_MOCI_CTRL	TEGRA_GPIO(U, 4)
+#endif /* APALIS_TK1_PCIE_EVALBOARD_INIT */
 
 int arch_misc_init(void)
 {
@@ -59,123 +67,166 @@ void pinmux_init(void)
 }
 
 #ifdef CONFIG_PCI_TEGRA
-int tegra_pcie_board_init(void)
+/* TODO: Convert to driver model */
+static int as3722_sd_enable(struct udevice *pmic, unsigned int sd)
 {
-	/* TODO: Convert to driver model
-	struct udevice *pmic;
 	int err;
 
-	err = as3722_init(&pmic);
-	if (err) {
-		error("failed to initialize AS3722 PMIC: %d\n", err);
-		return err;
-	}
+	if (sd > 6)
+		return -EINVAL;
 
-	err = as3722_sd_enable(pmic, 4);
-	if (err < 0) {
-		error("failed to enable SD4: %d\n", err);
+	err = pmic_clrsetbits(pmic, AS3722_SD_CONTROL, 0, 1 << sd);
+	if (err) {
+		error("failed to update SD control register: %d", err);
 		return err;
 	}
 
-	err = as3722_sd_set_voltage(pmic, 4, 0x24);
-	if (err < 0) {
-		error("failed to set SD4 voltage: %d\n", err);
-		return err;
-	}
+	return 0;
+}
 
-	err = as3722_gpio_configure(pmic, 1, AS3722_GPIO_OUTPUT_VDDH |
-					     AS3722_GPIO_INVERT);
-	if (err < 0) {
-		error("failed to configure GPIO#1 as output: %d\n", err);
-		return err;
-	}
+/* TODO: Convert to driver model */
+static int as3722_ldo_enable(struct udevice *pmic, unsigned int ldo)
+{
+	int err;
 
-	err = as3722_gpio_direction_output(pmic, 2, 1);
-	if (err < 0) {
-		error("failed to set GPIO#2 high: %d\n", err);
-		return err;
-	}
-	*/
+	if (ldo > 11)
+		return -EINVAL;
 
-	/* Reset I210 Gigabit Ethernet Controller */
-	gpio_request(LAN_RESET_N, "LAN_RESET_N");
-	gpio_direction_output(LAN_RESET_N, 0);
-
-	/*
-	 * Make sure we don't get any back feeding from LAN_WAKE_N resp.
-	 * DEV_OFF_N
-	 */
-	gpio_request(TEGRA_GPIO(O, 5), "LAN_WAKE_N");
-	gpio_direction_output(TEGRA_GPIO(O, 5), 0);
-
-	gpio_request(TEGRA_GPIO(O, 6), "LAN_DEV_OFF_N");
-	gpio_direction_output(TEGRA_GPIO(O, 6), 0);
-
-	/* Make sure LDO9 and LDO10 are initially enabled @ 0V */
-	/* TODO: Convert to driver model
-	err = as3722_ldo_enable(pmic, 9);
-	if (err < 0) {
-		error("failed to enable LDO9: %d\n", err);
-		return err;
-	}
-	err = as3722_ldo_enable(pmic, 10);
-	if (err < 0) {
-		error("failed to enable LDO10: %d\n", err);
-		return err;
-	}
-	err = as3722_ldo_set_voltage(pmic, 9, 0x80);
-	if (err < 0) {
-		error("failed to set LDO9 voltage: %d\n", err);
-		return err;
-	}
-	err = as3722_ldo_set_voltage(pmic, 10, 0x80);
-	if (err < 0) {
-		error("failed to set LDO10 voltage: %d\n", err);
+	err = pmic_clrsetbits(pmic, AS3722_LDO_CONTROL, 0, 1 << ldo);
+	if (err) {
+		error("failed to update LDO control register: %d", err);
 		return err;
 	}
-	*/
 
-	mdelay(100);
+	return 0;
+}
 
-	/* Make sure controller gets enabled by disabling DEV_OFF_N */
-	gpio_set_value(TEGRA_GPIO(O, 6), 1);
+int tegra_pcie_board_init(void)
+{
+	struct udevice *dev;
+	int ret;
+
+	ret = uclass_get_device_by_driver(UCLASS_PMIC,
+					  DM_GET_DRIVER(pmic_as3722), &dev);
+	if (ret) {
+		debug("%s: Failed to find PMIC\n", __func__);
+		return ret;
+	}
 
-	/* Enable LDO9 and LDO10 for +V3.3_ETH on patched prototypes */
-	/* TODO: Convert to driver model
-	err = as3722_ldo_set_voltage(pmic, 9, 0xff);
-	if (err < 0) {
-		error("failed to set LDO9 voltage: %d\n", err);
-		return err;
+	ret = as3722_sd_enable(dev, 4);
+	if (ret < 0) {
+		error("failed to enable SD4: %d\n", ret);
+		return ret;
 	}
-	err = as3722_ldo_set_voltage(pmic, 10, 0xff);
-	if (err < 0) {
-		error("failed to set LDO10 voltage: %d\n", err);
-		return err;
+
+	ret = as3722_sd_set_voltage(dev, 4, 0x24);
+	if (ret < 0) {
+		error("failed to set SD4 voltage: %d\n", ret);
+		return ret;
 	}
-	*/
 
-	mdelay(100);
-	gpio_set_value(LAN_RESET_N, 1);
+	gpio_request(LAN_DEV_OFF_N, "LAN_DEV_OFF_N");
+	gpio_request(LAN_RESET_N, "LAN_RESET_N");
+	gpio_request(LAN_WAKE_N, "LAN_WAKE_N");
 
 #ifdef APALIS_TK1_PCIE_EVALBOARD_INIT
-#define PEX_PERST_N	TEGRA_GPIO(DD, 1) /* Apalis GPIO7 */
-#define RESET_MOCI_CTRL	TEGRA_GPIO(U, 4)
-
-	/* Reset PLX PEX 8605 PCIe Switch plus PCIe devices on Apalis Evaluation
-	   Board */
 	gpio_request(PEX_PERST_N, "PEX_PERST_N");
 	gpio_request(RESET_MOCI_CTRL, "RESET_MOCI_CTRL");
-	gpio_direction_output(PEX_PERST_N, 0);
-	gpio_direction_output(RESET_MOCI_CTRL, 0);
-	/* Must be asserted for 100 ms after power and clocks are stable */
-	mdelay(100);
-	gpio_set_value(PEX_PERST_N, 1);
-	/* Err_5: PEX_REFCLK_OUTpx/nx Clock Outputs is not Guaranteed Until
-	   900 us After PEX_PERST# De-assertion */
-	mdelay(1);
-	gpio_set_value(RESET_MOCI_CTRL, 1);
-#endif /* APALIS_T30_PCIE_EVALBOARD_INIT */
+#endif /* APALIS_TK1_PCIE_EVALBOARD_INIT */
 
 	return 0;
 }
+
+void tegra_pcie_port_reset(void *port, unsigned int index)
+{
+	if (index == 1) { /* I210 Gigabit Ethernet Controller (On-module) */
+		struct udevice *dev;
+		int ret;
+
+		ret = uclass_get_device_by_driver(UCLASS_PMIC,
+						  DM_GET_DRIVER(pmic_as3722),
+						  &dev);
+		if (ret) {
+			debug("%s: Failed to find PMIC\n", __func__);
+			return;
+		}
+
+		/* Reset I210 Gigabit Ethernet Controller */
+		gpio_direction_output(LAN_RESET_N, 0);
+
+		/*
+		 * Make sure we don't get any back feeding from DEV_OFF_N resp.
+		 * LAN_WAKE_N
+		 */
+		gpio_direction_output(LAN_DEV_OFF_N, 0);
+		gpio_direction_output(LAN_WAKE_N, 0);
+
+		/* Make sure LDO9 and LDO10 are initially enabled @ 0V */
+		ret = as3722_ldo_enable(dev, 9);
+		if (ret < 0) {
+			error("failed to enable LDO9: %d\n", ret);
+			return;
+		}
+		ret = as3722_ldo_enable(dev, 10);
+		if (ret < 0) {
+			error("failed to enable LDO10: %d\n", ret);
+			return;
+		}
+		ret = as3722_ldo_set_voltage(dev, 9, 0x80);
+		if (ret < 0) {
+			error("failed to set LDO9 voltage: %d\n", ret);
+			return;
+		}
+		ret = as3722_ldo_set_voltage(dev, 10, 0x80);
+		if (ret < 0) {
+			error("failed to set LDO10 voltage: %d\n", ret);
+			return;
+		}
+
+		/* Make sure controller gets enabled by disabling DEV_OFF_N */
+		gpio_set_value(LAN_DEV_OFF_N, 1);
+
+		/* Enable LDO9 and LDO10 for +V3.3_ETH on patched prototypes */
+		ret = as3722_ldo_set_voltage(dev, 9, 0xff);
+		if (ret < 0) {
+			error("failed to set LDO9 voltage: %d\n", ret);
+			return;
+		}
+		ret = as3722_ldo_set_voltage(dev, 10, 0xff);
+		if (ret < 0) {
+			error("failed to set LDO10 voltage: %d\n", ret);
+			return;
+		}
+
+		/*
+		 * Must be asserted for 100 ms after power and clocks are stable
+		 */
+		mdelay(100);
+
+		gpio_set_value(LAN_RESET_N, 1);
+
+	} else if (index == 0) { /* Apalis PCIe */
+#ifdef APALIS_TK1_PCIE_EVALBOARD_INIT
+		/*
+		 * Reset PLX PEX 8605 PCIe Switch plus PCIe devices on Apalis
+		 * Evaluation Board
+		 */
+		gpio_direction_output(PEX_PERST_N, 0);
+		gpio_direction_output(RESET_MOCI_CTRL, 0);
+
+		/*
+		 * Must be asserted for 100 ms after power and clocks are stable
+		 */
+		mdelay(100);
+
+		gpio_set_value(PEX_PERST_N, 1);
+		/*
+		 * Err_5: PEX_REFCLK_OUTpx/nx Clock Outputs is not Guaranteed
+		 * Until 900 us After PEX_PERST# De-assertion
+		 */
+		mdelay(1);
+		gpio_set_value(RESET_MOCI_CTRL, 1);
+#endif /* APALIS_TK1_PCIE_EVALBOARD_INIT */
+	}
+}
 #endif /* CONFIG_PCI_TEGRA */
-- 
2.9.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [U-Boot] [PATCH 3/5] pci: tegra: make tegra_pcie_port_reset weak function with explicit index
  2017-08-04 16:10 ` [U-Boot] [PATCH 3/5] pci: tegra: make tegra_pcie_port_reset weak function with explicit index Marcel Ziswiler
@ 2017-08-04 16:33   ` Stephen Warren
  2017-08-07 12:47     ` Marcel Ziswiler
  0 siblings, 1 reply; 10+ messages in thread
From: Stephen Warren @ 2017-08-04 16:33 UTC (permalink / raw)
  To: u-boot

On 08/04/2017 10:10 AM, Marcel Ziswiler wrote:
> From: Marcel Ziswiler <marcel.ziswiler@toradex.com>
> 
> Make tegra_pcie_port_reset() a weak function with an explicit index
> parameter. This allows overriding the PCIe port reset functionality
> from board specific code as e.g. required for Apalis TK1.

I think this change should be implemented differently.

Does the patch description mean that:

a) This change allows the board code to know which port is being reset.

If so, simply have the board-specific implementation access port->index 
since it's already being passed port, and all callers are passing 
port->index as the index parameter. If the type isn't available, then 
you can add a tegra_pcie_port_index_of_port() function to retrieve it 
through the opaque type.

or:

b) That by changing the function signature code is able to choose 
between calling the board-specific reset override function and the PCIe 
driver low-level reset function?

If so, let's just have two different function names, have all callers 
call the board-specific function only, and have the board-specific 
function call the driver function. There can be a weak default 
board-specific implementation that simply calls the driver function.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [U-Boot] [PATCH 2/5] apalis-tk1: add missing as3722 gpio0 configuration
  2017-08-04 16:10 ` [U-Boot] [PATCH 2/5] apalis-tk1: add missing as3722 gpio0 configuration Marcel Ziswiler
@ 2017-08-06  5:16   ` Simon Glass
  0 siblings, 0 replies; 10+ messages in thread
From: Simon Glass @ 2017-08-06  5:16 UTC (permalink / raw)
  To: u-boot

On 4 August 2017 at 10:10, Marcel Ziswiler <marcel@ziswiler.com> wrote:
> From: Marcel Ziswiler <marcel.ziswiler@toradex.com>
>
> As the AS3722 GPIO0 is also a not connected on our Apalis TK1 module
> explicitly configure it to high-impedance as well.
>
> Signed-off-by: Marcel Ziswiler <marcel.ziswiler@toradex.com>
> ---
>
>  arch/arm/dts/tegra124-apalis.dts | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [U-Boot] [PATCH 4/5] power: as3722: add as3722_ldo_set_voltage signature to header file
  2017-08-04 16:10 ` [U-Boot] [PATCH 4/5] power: as3722: add as3722_ldo_set_voltage signature to header file Marcel Ziswiler
@ 2017-08-06  5:16   ` Simon Glass
  0 siblings, 0 replies; 10+ messages in thread
From: Simon Glass @ 2017-08-06  5:16 UTC (permalink / raw)
  To: u-boot

On 4 August 2017 at 10:10, Marcel Ziswiler <marcel@ziswiler.com> wrote:
> From: Marcel Ziswiler <marcel.ziswiler@toradex.com>
>
> Just like the already present as3722_sd_set_voltage() add the currently
> missing signature of the as3722_ldo_set_voltage() function to its header
> file.
>
> Signed-off-by: Marcel Ziswiler <marcel.ziswiler@toradex.com>
> ---
>
>  include/power/as3722.h | 1 +
>  1 file changed, 1 insertion(+)

Reviewed-by: Simon Glass <sjg@chromium.org>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [U-Boot] [PATCH 3/5] pci: tegra: make tegra_pcie_port_reset weak function with explicit index
  2017-08-04 16:33   ` Stephen Warren
@ 2017-08-07 12:47     ` Marcel Ziswiler
  0 siblings, 0 replies; 10+ messages in thread
From: Marcel Ziswiler @ 2017-08-07 12:47 UTC (permalink / raw)
  To: u-boot

Hi Stephen

On Fri, 2017-08-04 at 10:33 -0600, Stephen Warren wrote:
> On 08/04/2017 10:10 AM, Marcel Ziswiler wrote:
> > From: Marcel Ziswiler <marcel.ziswiler@toradex.com>
> > 
> > Make tegra_pcie_port_reset() a weak function with an explicit index
> > parameter. This allows overriding the PCIe port reset functionality
> > from board specific code as e.g. required for Apalis TK1.
> 
> I think this change should be implemented differently.
> 
> Does the patch description mean that:
> 
> a) This change allows the board code to know which port is being
> reset.
> 
> If so, simply have the board-specific implementation access port-
> >index 
> since it's already being passed port, and all callers are passing 
> port->index as the index parameter. If the type isn't available,
> then 
> you can add a tegra_pcie_port_index_of_port() function to retrieve
> it 
> through the opaque type.

I did of course know that the port struct already contained that
information but did not think of any such elegant way to do it as you
describe outside of adding an explicit index. So I agree and will just
implement and make use of such a new tegra_pcie_port_index_of_port()
function.

> or:
> 
> b) That by changing the function signature code is able to choose 
> between calling the board-specific reset override function and the
> PCIe 
> driver low-level reset function?
> 
> If so, let's just have two different function names, have all
> callers 
> call the board-specific function only, and have the board-specific 
> function call the driver function. There can be a weak default 
> board-specific implementation that simply calls the driver function.

Initially that was not really my intention but I believe you bring up a
 very valid point and it could be useful to use the default reset
behaviour on certain ports while using a custom one on others. This
would e.g. be the case on Apalis T30 where the on-module PCIe gigabit
Ethernet chip is using the standard out-of-band signalling while the
PCIe switch on the Apalis Evaluation board requires the same special
reset work-around as optionally implemented for Apalis TK1. I will
therefore implement it this way as well leaving it completely up to
board code which way to go.

Thanks again for your valuable input.

Cheers

Marcel

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2017-08-07 12:47 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-08-04 16:09 [U-Boot] [PATCH 0/5] fix apalis-tk1 pcie gigabit ethernet operation Marcel Ziswiler
2017-08-04 16:10 ` [U-Boot] [PATCH 1/5] configs: apalis-tk1: fix boot failure using ext4 rootfs Marcel Ziswiler
2017-08-04 16:10 ` [U-Boot] [PATCH 2/5] apalis-tk1: add missing as3722 gpio0 configuration Marcel Ziswiler
2017-08-06  5:16   ` Simon Glass
2017-08-04 16:10 ` [U-Boot] [PATCH 3/5] pci: tegra: make tegra_pcie_port_reset weak function with explicit index Marcel Ziswiler
2017-08-04 16:33   ` Stephen Warren
2017-08-07 12:47     ` Marcel Ziswiler
2017-08-04 16:10 ` [U-Boot] [PATCH 4/5] power: as3722: add as3722_ldo_set_voltage signature to header file Marcel Ziswiler
2017-08-06  5:16   ` Simon Glass
2017-08-04 16:10 ` [U-Boot] [PATCH 5/5] apalis-tk1: fix pcie reset for reliable gigabit ethernet operation Marcel Ziswiler

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox