[PATCH v5 01/17] powerpc/fsl-pci: improve clock API use

Gerhard Sittig gsi at denx.de
Sun Nov 17 18:06:01 EST 2013


make the Freescale PCI driver get, prepare and enable the PCI clock
during probe(); the clock gets put upon device shutdown by the devm
approach

clock lookup is non-fatal as not all platforms may provide clock specs
in their device tree or implement a device tree based clock provider,
but failure to enable clocks after successful lookup is fatal

the driver appears to not have a remove() routine, so no reference to
the clock is kept during use, and the clock isn't released (the devm
approach will put the clock, but it won't get disabled or unprepared)

the 85xx/86xx platforms go through the probe() routine, where clock
lookup occurs and the clock gets acquired if one was specified; the
512x/83xx platforms don't pass through probe() but instead directly call
the add_bridge() routine at a point in time where the clock provider has
not been setup yet even if the platform implements one -- add comments
to the code paths as a reminder for the potential need of a workaround
in the platform's clock driver, and to keep awareness if code should get
re-arranged or moved

Cc: Benjamin Herrenschmidt <benh at kernel.crashing.org>
Cc: Paul Mackerras <paulus at samba.org>
Cc: Kumar Gala <galak at kernel.crashing.org>
Cc: linuxppc-dev at lists.ozlabs.org
Signed-off-by: Gerhard Sittig <gsi at denx.de>
---
 arch/powerpc/sysdev/fsl_pci.c |   52 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/arch/powerpc/sysdev/fsl_pci.c b/arch/powerpc/sysdev/fsl_pci.c
index ccfb50ddfe38..efa0916f61b6 100644
--- a/arch/powerpc/sysdev/fsl_pci.c
+++ b/arch/powerpc/sysdev/fsl_pci.c
@@ -17,6 +17,8 @@
  * Free Software Foundation;  either version 2 of the  License, or (at your
  * option) any later version.
  */
+
+#include <linux/clk.h>
 #include <linux/kernel.h>
 #include <linux/pci.h>
 #include <linux/delay.h>
@@ -755,6 +757,32 @@ int __init mpc83xx_add_bridge(struct device_node *dev)
 	const int *bus_range;
 	int primary;
 
+	/*
+	 * 85xx/86xx platforms take the path through the probe() routine
+	 * as one would expect, PCI related clocks get acquired there if
+	 * specified
+	 *
+	 * 83xx/512x _don't_ pass through probe(), this add_bridge()
+	 * routine instead is called from within .setup_arch() at a
+	 * point in time where clock providers haven't been setup yet;
+	 * so clocks cannot get acquired here -- lookup would always
+	 * fail even on those platforms which implement the provider
+	 *
+	 * there is no counterpart for add_bridge() just like there is
+	 * no remove() counterpart for probe(), so in either case the
+	 * PCI related clock won't get released, and all of the
+	 * 512x/83xx/85xx/86xx platforms behave in identical ways
+	 *
+	 * this comment is here to "keep the balance" against the
+	 * probe() routine, and as a reminder to acquire clocks if the
+	 * add_bridge() call should move to some later point in time
+	 *
+	 * until then clock providers are expected to work around the
+	 * peripheral driver's not acquiring the PCI clock on those
+	 * platforms where clock providers exist, while nothing needs to
+	 * be done for those platforms without a clock provider
+	 */
+
 	is_mpc83xx_pci = 1;
 
 	if (!of_device_is_available(dev)) {
@@ -1086,9 +1114,33 @@ void fsl_pci_assign_primary(void)
 
 static int fsl_pci_probe(struct platform_device *pdev)
 {
+	struct clk *clk;
 	int ret;
 	struct device_node *node;
 
+	/*
+	 * clock lookup is non-fatal since the driver is shared among
+	 * platforms and not all of them provide clocks specs in their
+	 * device tree, but failure to enable a specified clock is
+	 * considered fatal
+	 *
+	 * note that only the 85xx and 86xx platforms pass through this
+	 * probe() routine, while 83xx and 512x directly invoke the
+	 * mpc83xx_add_bridge() routine from within .setup_arch() code
+	 */
+	clk = devm_clk_get(&pdev->dev, "ipg");
+	if (!IS_ERR(clk)) {
+		ret = clk_prepare_enable(clk);
+		if (ret) {
+			dev_err(&pdev->dev, "Could not enable PCI clock\n");
+			return ret;
+		}
+		/*
+		 * TODO where to store the 'clk' reference?  there appears
+		 * to be no remove() routine which undoes what probe() does
+		 */
+	}
+
 	node = pdev->dev.of_node;
 	ret = fsl_add_bridge(pdev, fsl_pci_primary == node);
 
-- 
1.7.10.4




More information about the linux-arm-kernel mailing list