All of lore.kernel.org
 help / color / mirror / Atom feed
From: Gerhard Sittig <gsi@denx.de>
To: linuxppc-dev@lists.ozlabs.org,
	linux-arm-kernel@lists.infradead.org,
	Anatolij Gustschin <agust@denx.de>,
	Mike Turquette <mturquette@linaro.org>
Cc: Detlev Zundel <dzu@denx.de>, Gerhard Sittig <gsi@denx.de>,
	Paul Mackerras <paulus@samba.org>,
	Scott Wood <scottwood@freescale.com>
Subject: [PATCH v5 01/17] powerpc/fsl-pci: improve clock API use
Date: Mon, 18 Nov 2013 00:06:01 +0100	[thread overview]
Message-ID: <1384729577-7336-2-git-send-email-gsi@denx.de> (raw)
In-Reply-To: <1384729577-7336-1-git-send-email-gsi@denx.de>

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@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Kumar Gala <galak@kernel.crashing.org>
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Gerhard Sittig <gsi@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

WARNING: multiple messages have this Message-ID (diff)
From: gsi@denx.de (Gerhard Sittig)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v5 01/17] powerpc/fsl-pci: improve clock API use
Date: Mon, 18 Nov 2013 00:06:01 +0100	[thread overview]
Message-ID: <1384729577-7336-2-git-send-email-gsi@denx.de> (raw)
In-Reply-To: <1384729577-7336-1-git-send-email-gsi@denx.de>

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@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Kumar Gala <galak@kernel.crashing.org>
Cc: linuxppc-dev at lists.ozlabs.org
Signed-off-by: Gerhard Sittig <gsi@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

  reply	other threads:[~2013-11-17 23:07 UTC|newest]

Thread overview: 64+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-11-17 23:06 [PATCH v5 00/17] add COMMON_CLK support for PowerPC MPC512x Gerhard Sittig
2013-11-17 23:06 ` Gerhard Sittig
2013-11-17 23:06 ` Gerhard Sittig
2013-11-17 23:06 ` Gerhard Sittig
2013-11-17 23:06 ` Gerhard Sittig
2013-11-17 23:06 ` Gerhard Sittig
2013-11-17 23:06 ` Gerhard Sittig [this message]
2013-11-17 23:06   ` [PATCH v5 01/17] powerpc/fsl-pci: improve clock API use Gerhard Sittig
2013-11-19 22:41   ` Scott Wood
2013-11-19 22:41     ` Scott Wood
2013-11-21  9:21     ` Gerhard Sittig
2013-11-21  9:21       ` Gerhard Sittig
2013-11-17 23:06 ` [PATCH v5 04/17] clk: mpc512x: introduce COMMON_CLK for MPC512x (disabled) Gerhard Sittig
2013-11-17 23:06   ` Gerhard Sittig
2013-11-17 23:06 ` [PATCH v5 05/17] clk: mpc512x: add backwards compat to the CCF code Gerhard Sittig
2013-11-17 23:06   ` Gerhard Sittig
2013-11-17 23:06 ` [PATCH v5 07/17] clk: mpc5xxx: switch to COMMON_CLK, retire PPC_CLOCK Gerhard Sittig
2013-11-17 23:06   ` Gerhard Sittig
     [not found] ` <1384729577-7336-1-git-send-email-gsi-ynQEQJNshbs@public.gmane.org>
2013-11-17 23:06   ` [PATCH v5 02/17] dts: mpc512x: introduce dt-bindings/clock/ header Gerhard Sittig
2013-11-17 23:06     ` Gerhard Sittig
2013-11-17 23:06     ` Gerhard Sittig
2013-11-17 23:06   ` [PATCH v5 03/17] dts: mpc512x: add clock related device tree specs Gerhard Sittig
2013-11-17 23:06     ` Gerhard Sittig
2013-11-17 23:06     ` Gerhard Sittig
2013-11-17 23:06   ` [PATCH v5 06/17] dts: mpc512x: add clock specs for client lookups Gerhard Sittig
2013-11-17 23:06     ` Gerhard Sittig
2013-11-17 23:06     ` Gerhard Sittig
2013-11-17 23:06   ` [PATCH v5 08/17] spi: mpc512x: adjust to OF based clock lookup Gerhard Sittig
2013-11-17 23:06     ` Gerhard Sittig
2013-11-17 23:06     ` Gerhard Sittig
2013-11-25 17:30     ` Mark Brown
2013-11-25 17:30       ` Mark Brown
2013-11-25 17:30       ` Mark Brown
     [not found]       ` <20131125173008.GQ14725-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
2013-11-25 19:06         ` Gerhard Sittig
2013-11-25 19:06           ` Gerhard Sittig
2013-11-25 19:06           ` Gerhard Sittig
2013-11-17 23:06 ` [PATCH v5 09/17] serial: mpc512x: adjust for " Gerhard Sittig
2013-11-17 23:06   ` Gerhard Sittig
2013-11-17 23:06   ` Gerhard Sittig
2013-11-17 23:06 ` [PATCH v5 10/17] serial: mpc512x: setup the PSC FIFO clock as well Gerhard Sittig
2013-11-17 23:06   ` Gerhard Sittig
2013-11-17 23:06   ` Gerhard Sittig
2013-11-17 23:06 ` [PATCH v5 11/17] USB: fsl-mph-dr-of: adjust for OF based clock lookup Gerhard Sittig
2013-11-17 23:06   ` Gerhard Sittig
2013-11-17 23:06 ` [PATCH v5 12/17] mtd: mpc5121_nfc: " Gerhard Sittig
2013-11-17 23:06   ` Gerhard Sittig
2013-11-17 23:06 ` [PATCH v5 13/17] [media] fsl-viu: " Gerhard Sittig
2013-11-17 23:06   ` Gerhard Sittig
2013-11-17 23:06   ` Gerhard Sittig
2013-11-17 23:06 ` [PATCH v5 14/17] net: can: mscan: adjust to common clock support for mpc512x Gerhard Sittig
2013-11-17 23:06   ` Gerhard Sittig
2013-11-17 23:06   ` Gerhard Sittig
2013-11-17 23:06 ` [PATCH v5 15/17] net: can: mscan: remove non-CCF code for MPC512x Gerhard Sittig
2013-11-17 23:06   ` Gerhard Sittig
2013-11-17 23:06   ` Gerhard Sittig
2013-11-17 23:06 ` [PATCH v5 16/17] powerpc/mpc512x: improve DIU related clock setup Gerhard Sittig
2013-11-17 23:06   ` Gerhard Sittig
2013-11-17 23:06 ` [PATCH v5 17/17] clk: mpc512x: remove migration support workarounds Gerhard Sittig
2013-11-17 23:06   ` Gerhard Sittig
2013-11-24 18:39 ` [PATCH v5 00/17] add COMMON_CLK support for PowerPC MPC512x Gerhard Sittig
2013-11-24 18:39   ` Gerhard Sittig
2013-11-24 18:39   ` Gerhard Sittig
2013-11-24 18:39   ` Gerhard Sittig
2013-11-24 18:39   ` Gerhard Sittig

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1384729577-7336-2-git-send-email-gsi@denx.de \
    --to=gsi@denx.de \
    --cc=agust@denx.de \
    --cc=dzu@denx.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=mturquette@linaro.org \
    --cc=paulus@samba.org \
    --cc=scottwood@freescale.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.