diff mbox

[U-Boot,v2,4/6] pci: tegra: introduce weak tegra_pcie_board_port_reset() function

Message ID 20170808124228.5001-5-marcel@ziswiler.com
State Superseded
Delegated to: Tom Warren
Headers show

Commit Message

Marcel Ziswiler Aug. 8, 2017, 12:42 p.m. UTC
From: Marcel Ziswiler <marcel.ziswiler@toradex.com>

Introduce a weak tegra_pcie_board_port_reset() function by default
calling the existing tegra_pcie_port_reset() function. Additionally add
a tegra_pcie_port_index_of_port() function to retrieve the specific PCIe
port index if required. This allows overriding the PCIe port reset
functionality from board specific code as e.g. required for Apalis T30
and Apalis TK1.

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

Changes in v2:
- Incorporate Stephen's review feedback by introducing a
  tegra_pcie_port_index_of_port() function as well as a board-specific
  reset override function.

 drivers/pci/pci_tegra.c | 28 ++++++++++++++++++++--------
 include/pci_tegra.h     |  9 +++++++++
 2 files changed, 29 insertions(+), 8 deletions(-)
 create mode 100644 include/pci_tegra.h

Comments

Stephen Warren Aug. 8, 2017, 4:09 p.m. UTC | #1
On 08/08/2017 06:42 AM, Marcel Ziswiler wrote:
> From: Marcel Ziswiler <marcel.ziswiler@toradex.com>
> 
> Introduce a weak tegra_pcie_board_port_reset() function by default
> calling the existing tegra_pcie_port_reset() function. Additionally add
> a tegra_pcie_port_index_of_port() function to retrieve the specific PCIe
> port index if required. This allows overriding the PCIe port reset
> functionality from board specific code as e.g. required for Apalis T30
> and Apalis TK1.

> diff --git a/include/pci_tegra.h b/include/pci_tegra.h

> +int tegra_pcie_port_index_of_port(void *port);
> +
> +void tegra_pcie_port_reset(void *port);

The parameter to these functions should be "struct tegra_pcie_port 
*port" for type-safety and to avoid all the casts in the implementation. 
Since this is a pointer, it doesn't matter that the board-level callers 
don't have the full type defined since they don't dereference the 
pointer, just pass it around. All you need to do is add the following to 
the header before the function prototypes:

struct tegra_pcie_port;
Marcel Ziswiler Aug. 9, 2017, 8:18 a.m. UTC | #2
On Tue, 2017-08-08 at 10:09 -0600, Stephen Warren wrote:
> On 08/08/2017 06:42 AM, Marcel Ziswiler wrote:

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

> > 

> > Introduce a weak tegra_pcie_board_port_reset() function by default

> > calling the existing tegra_pcie_port_reset() function. Additionally

> > add

> > a tegra_pcie_port_index_of_port() function to retrieve the specific

> > PCIe

> > port index if required. This allows overriding the PCIe port reset

> > functionality from board specific code as e.g. required for Apalis

> > T30

> > and Apalis TK1.

> > diff --git a/include/pci_tegra.h b/include/pci_tegra.h

> > +int tegra_pcie_port_index_of_port(void *port);

> > +

> > +void tegra_pcie_port_reset(void *port);

> 

> The parameter to these functions should be "struct tegra_pcie_port 

> *port" for type-safety and to avoid all the casts in the

> implementation. 

> Since this is a pointer, it doesn't matter that the board-level

> callers 

> don't have the full type defined since they don't dereference the 

> pointer, just pass it around. All you need to do is add the following

> to 

> the header before the function prototypes:

> 

> struct tegra_pcie_port;


Yes, thanks. Of course that makes more sense. I will do that in a v3.
diff mbox

Patch

diff --git a/drivers/pci/pci_tegra.c b/drivers/pci/pci_tegra.c
index cb5cf8b..cb0a30c 100644
--- a/drivers/pci/pci_tegra.c
+++ b/drivers/pci/pci_tegra.c
@@ -18,6 +18,7 @@ 
 #include <errno.h>
 #include <malloc.h>
 #include <pci.h>
+#include <pci_tegra.h>
 #include <power-domain.h>
 #include <reset.h>
 
@@ -893,21 +894,32 @@  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 tegra_pcie_port_reset(void *port)
 {
-	unsigned long ctrl = tegra_pcie_port_get_pex_ctrl(port);
+	unsigned long ctrl = tegra_pcie_port_get_pex_ctrl(
+			(struct tegra_pcie_port *)port);
 	unsigned long value;
 
 	/* pulse reset signel */
-	value = afi_readl(port->pcie, ctrl);
+	value = afi_readl(((struct tegra_pcie_port *)port)->pcie, ctrl);
 	value &= ~AFI_PEX_CTRL_RST;
-	afi_writel(port->pcie, value, ctrl);
+	afi_writel(((struct tegra_pcie_port *)port)->pcie, value, ctrl);
 
 	udelay(2000);
 
-	value = afi_readl(port->pcie, ctrl);
+	value = afi_readl(((struct tegra_pcie_port *)port)->pcie, ctrl);
 	value |= AFI_PEX_CTRL_RST;
-	afi_writel(port->pcie, value, ctrl);
+	afi_writel(((struct tegra_pcie_port *)port)->pcie, value, ctrl);
+}
+
+int tegra_pcie_port_index_of_port(void *port)
+{
+	return ((struct tegra_pcie_port *)port)->index;
+}
+
+void __weak tegra_pcie_board_port_reset(void *port)
+{
+	tegra_pcie_port_reset(port);
 }
 
 static void tegra_pcie_port_enable(struct tegra_pcie_port *port)
@@ -928,7 +940,7 @@  static void tegra_pcie_port_enable(struct tegra_pcie_port *port)
 
 	afi_writel(pcie, value, ctrl);
 
-	tegra_pcie_port_reset(port);
+	tegra_pcie_board_port_reset(port);
 
 	if (soc->force_pca_enable) {
 		value = rp_readl(port, RP_VEND_CTL2);
@@ -979,7 +991,7 @@  static bool tegra_pcie_port_check_link(struct tegra_pcie_port *port)
 		} while (--timeout);
 
 retry:
-		tegra_pcie_port_reset(port);
+		tegra_pcie_board_port_reset(port);
 	} while (--retries);
 
 	return false;
diff --git a/include/pci_tegra.h b/include/pci_tegra.h
new file mode 100644
index 0000000..13ee908
--- /dev/null
+++ b/include/pci_tegra.h
@@ -0,0 +1,9 @@ 
+/*
+ * Copyright (c) 2017 Toradex, Inc.
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+int tegra_pcie_port_index_of_port(void *port);
+
+void tegra_pcie_port_reset(void *port);