[PATCH v5 00/17] add COMMON_CLK support for PowerPC MPC512x

[PATCH v5 00/17] add COMMON_CLK support for PowerPC MPC512x

[Gerhard Sittig]

this series introduces support for the common clock framework (CCF,
COMMON_CLK Kconfig option) in the PowerPC based MPC512x platform, which
brings device tree based clock lookup as well

at subsystem maintainers:

this series was streamlined for conflict free application through the
subsystems' individual trees, and consists of the following phases
- peripheral driver cleanup (1/17) (not essential, in fact comments only
  and code which is a NOP)
- introduction of CCF support including migration workarounds and
  backwards compatibility, device tree updates (2/17 - 7/17)
  (nevertheless I suggest to take the .dts/.dtsi updates through the
  PowerPC tree, the extensions are straight forward and strictly are
  clock related, and complement the CCF platform support)
- peripheral driver adjustment to the CCF approach (8/17 - 16/17)
- removal of migration workarounds (17/17)

at device tree maintainers:

- the series does not introduce new bindings, but implements the
  existing clock binding (OF clock provider, DT based clock lookup) and
  so adjusts and extends DTS files
- the code is backwards compatible, and keeps working with device trees
  which don't contain clock related information


the series is based on v3.12, but I'll rebase against v3.13-rc1 (when
available) or any other subtree upon request

the series passes 'checkpatch.pl --strict' except for one warning which
cannot get resolved, since the <linux/clk-provider.h> API dictates the
data type, "fixing" the checkpatch warning would break compilation

  WARNING: static const char * array should probably be static const char * const
  #420: FILE: arch/powerpc/platforms/512x/clock-commonclk.c:342:
  +static const char *parent_names_mux0[] = {

  total: 0 errors, 1 warnings, 0 checks, 834 lines checked

checkpatch appears to choke on the NODE_CHK() macro in the backwards
compat patch, while I cannot see why since it groks the FOR_NODES() and
NODE_PREP() macros

  Use of uninitialized value $c in pattern match (m//) at ./scripts/checkpatch.pl line 3280.
  Use of uninitialized value in substr at ./scripts/checkpatch.pl line 3287.
  Use of uninitialized value $s in substr at ./scripts/checkpatch.pl line 3287.


the series has been build tested (each step on PowerPC 512x, 52xx, 5xxx
multi platform, 83xx, 85xx, 86xx, and on ARM v6/v7), run tested (each
step on 512x, the switch to CCF on 52xx), and tested for backwards
compatibility (each step on 512x with a v3.11 dtb)


changes in v5:
- extend comments in the PCI driver cleanup (probe() vs setup_arch()
  discussion, no code change); all other peripheral driver cleanup from
  v4 was taken into mainline
- concentrate migration support in a separate routine for improved
  maintainability
- fix the oscillator frequency lookup ('osc' reference) in the CCF
  platform support code which creates the clock tree
- add backwards compatibility with device trees that lack clock specs,
  concentrate compat support in a separate routine for improved
  maintainability, add it in a separate patch for easier review
- consistent use of the 'ipg' name in DTS files for the register access
  clock item of peripherals
- switch from PPC_CLOCK to COMMON_CLK at the same time for 512x and 52xx
  (keep multi-platform setups operational), in a separate patch
- move removal of migration support to the very end of the series, it's
  no longer intertwined with peripheral driver adjustment
- SPI and UART: get 'mclk' and 'ipg' clock items in a more consistent
  order (less obfuscation in the diff)
- add/adjust Cc: and Acked-By: entries, rework commit messages and
  comments where appropriate

changes in v4:
- remove explicit devm_clk_put() calls as these will occur implicitly
  upon device release (01/31, 02/31, 03/31, 04/31, 05/31, 06/31, 08/31,
  09/31, 27/31)
- split the PSC (SPI, UART) and MSCAN (CAN) related MCLK subtrees into
  separate 'ipg'/'bdlc' gated clock items for register access as well as
  the 'mclk' clock subtrees that apply to bitrates -- this eliminates
  the need for "shared gates" and further reduces clock pre-enable
  workarounds (11/31, 15/31, 17/31, 18/31, 20/31, 21/31, 22/31, 27/31)
- further adjust the CAN clock driver, fix an incomplete error code path
  in the network device open callback (11/31), only enable the bitrate
  clock when the network device is open (27/31)
- remove debug output in the clock tree setup when introducing the
  platform's clock driver, there already is CONFIG_COMMON_CLK_DEBUG to
  retrieve more complete information (17/31)
- remove an "enums don't work here" comment in the dt-bindings header
  file (15/31)
- reword and update commit messages (body and/or subject) where
  appropriate (03/31, 04/31, 05/31, 06/31, 08/31, 09/31, 11/31, 12/31,
  17/31, 20/31, 21/31, 22/31, 27/31, 28/31, 30/31, 31/31)
- add 'Reviewed-By' attributes which were received for v3

changes in v3:
- rebase the series against v3.11-rc2
- re-ordered the series to first address all general clock handling
  concerns in existing drivers, before introducing common clock support
  in the platform's clock driver
- slightly rework the SPI (01/31), UART (02/31), and PSC FIFO (23/31)
  clock handling in comparison to v2 which introduced those fixes
  (devm_{get,put}_clk() calls, fewer goto labels in error paths)
- fix and improve clock handling (balance allocation and release of
  clocks, check for errors during setup) in all of the other drivers
  which this series has touched before in naive ways: USB (03/31), NAND
  flash (04/31), video capture (05/31), I2C (06/31), ethernet (08/31),
  PCI (09/31), CAN (11/31)
- silence a build warning in the ethernet driver (07/31)
- eliminate all PPC_CLOCK references, use 'per' clock names for NAND
  flash (25/31) and VIU (26/31) as well
- unbreak CAN operation for the period between introducing common clock
  support in the platform's clock driver and introducing common clock
  support in the CAN peripheral driver as well as providing clock specs
  in the device tree (provide clkdev aliases for SYS and REF)
- improve common clock support for CAN (devm_{get,put}_clk() calls,
  check enable() errors, keep a reference to used clocks, disable and
  put clocks after use)
- reworded several commit messages to better reflect the kind of change
  and because fixes were applied before adding common infrastructure
  support
- point to individual numbered patches of the series in the list of
  changes for v2 as well

changes in v2:
- cleanup of the UART (02/24) and SPI (01/24) clock handling before the
  introduction of common clock support for the platform, as incomplete
  clock handling becomes fatal or more dangerous later (which in turn
  changes the context of the "device tree lookup only" followup patch
  later)
- reordered the sequence of patches to keep the serial communication
  related parts together (UART, SPI, and PSC FIFO changes after common
  clock support was introduced, which have become 11-14/24 now)
- updated commit messages for the clock API use cleanup in the serial
  communication drivers, updated comments and reworded commit messages
  in the core clock driver to expand on the pre-enable workaround and
  clkdev registration (09/24)
- keep a reference to the PSC FIFO clock during use instead of looking
  up the clock again in the uninit() routine (14/24)
- remove the clkdev.h header file inclusion directive with the removal
  of the clkdev registration call (13/24)


Gerhard Sittig (17):
  powerpc/fsl-pci: improve clock API use
  dts: mpc512x: introduce dt-bindings/clock/ header
  dts: mpc512x: add clock related device tree specs
  clk: mpc512x: introduce COMMON_CLK for MPC512x (disabled)
  clk: mpc512x: add backwards compat to the CCF code
  dts: mpc512x: add clock specs for client lookups
  clk: mpc5xxx: switch to COMMON_CLK, retire PPC_CLOCK
  spi: mpc512x: adjust to OF based clock lookup
  serial: mpc512x: adjust for OF based clock lookup
  serial: mpc512x: setup the PSC FIFO clock as well
  USB: fsl-mph-dr-of: adjust for OF based clock lookup
  mtd: mpc5121_nfc: adjust for OF based clock lookup
  [media] fsl-viu: adjust for OF based clock lookup
  net: can: mscan: adjust to common clock support for mpc512x
  net: can: mscan: remove non-CCF code for MPC512x
  powerpc/mpc512x: improve DIU related clock setup
  clk: mpc512x: remove migration support workarounds

 arch/powerpc/Kconfig                          |    5 -
 arch/powerpc/boot/dts/ac14xx.dts              |    7 +
 arch/powerpc/boot/dts/mpc5121.dtsi            |  113 ++-
 arch/powerpc/kernel/Makefile                  |    1 -
 arch/powerpc/kernel/clock.c                   |   82 ---
 arch/powerpc/platforms/512x/Kconfig           |    2 +-
 arch/powerpc/platforms/512x/Makefile          |    3 +-
 arch/powerpc/platforms/512x/clock-commonclk.c |  950 +++++++++++++++++++++++++
 arch/powerpc/platforms/512x/clock.c           |  753 --------------------
 arch/powerpc/platforms/512x/mpc512x_shared.c  |  169 +++--
 arch/powerpc/platforms/52xx/Kconfig           |    2 +-
 arch/powerpc/sysdev/fsl_pci.c                 |   52 ++
 drivers/media/platform/fsl-viu.c              |    2 +-
 drivers/mtd/nand/mpc5121_nfc.c                |    2 +-
 drivers/net/can/mscan/mpc5xxx_can.c           |  270 ++++---
 drivers/spi/spi-mpc512x-psc.c                 |   26 +-
 drivers/tty/serial/mpc52xx_uart.c             |   90 ++-
 drivers/usb/host/fsl-mph-dr-of.c              |   13 +-
 include/dt-bindings/clock/mpc512x-clock.h     |   69 ++
 include/linux/clk-provider.h                  |   16 +
 20 files changed, 1556 insertions(+), 1071 deletions(-)
 delete mode 100644 arch/powerpc/kernel/clock.c
 create mode 100644 arch/powerpc/platforms/512x/clock-commonclk.c
 delete mode 100644 arch/powerpc/platforms/512x/clock.c
 create mode 100644 include/dt-bindings/clock/mpc512x-clock.h

-- 
1.7.10.4

[PATCH v5 09/17] serial: mpc512x: adjust for OF based clock lookup

[Gerhard Sittig]

after device tree based clock lookup became available, the peripheral
driver need no longer construct clock names which include the PSC index,
remove the "psc%d_mclk" template and unconditionally use 'mclk'

acquire and release the "ipg" clock item for register access as well

Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jiri Slaby <jslaby@suse.cz>
Cc: linux-serial@vger.kernel.org
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> # for v4
Signed-off-by: Gerhard Sittig <gsi@denx.de>
---
Greg, the difference between v4 and v5 of this patch is that v4
took the 'mclk' and 'ipg' clock items in reverse order, and thus
potentially obfuscated the adjusted name for 'mclk' -- v5 of the
patch is identical in content but cleaner diff-wise

---
 drivers/tty/serial/mpc52xx_uart.c |   40 ++++++++++++++++++++++++++++++++-----
 1 file changed, 35 insertions(+), 5 deletions(-)

diff --git a/drivers/tty/serial/mpc52xx_uart.c b/drivers/tty/serial/mpc52xx_uart.c
index 5be1df39f9f5..1d1dbe9d5af2 100644
--- a/drivers/tty/serial/mpc52xx_uart.c
+++ b/drivers/tty/serial/mpc52xx_uart.c
@@ -619,29 +619,55 @@ static irqreturn_t mpc512x_psc_handle_irq(struct uart_port *port)
 }
 
 static struct clk *psc_mclk_clk[MPC52xx_PSC_MAXNUM];
+static struct clk *psc_ipg_clk[MPC52xx_PSC_MAXNUM];
 
 /* called from within the .request_port() callback (allocation) */
 static int mpc512x_psc_alloc_clock(struct uart_port *port)
 {
 	int psc_num;
-	char clk_name[16];
 	struct clk *clk;
 	int err;
 
 	psc_num = (port->mapbase & 0xf00) >> 8;
-	snprintf(clk_name, sizeof(clk_name), "psc%d_mclk", psc_num);
-	clk = devm_clk_get(port->dev, clk_name);
+
+	clk = devm_clk_get(port->dev, "mclk");
 	if (IS_ERR(clk)) {
 		dev_err(port->dev, "Failed to get MCLK!\n");
-		return PTR_ERR(clk);
+		err = PTR_ERR(clk);
+		goto out_err;
 	}
 	err = clk_prepare_enable(clk);
 	if (err) {
 		dev_err(port->dev, "Failed to enable MCLK!\n");
-		return err;
+		goto out_err;
 	}
 	psc_mclk_clk[psc_num] = clk;
+
+	clk = devm_clk_get(port->dev, "ipg");
+	if (IS_ERR(clk)) {
+		dev_err(port->dev, "Failed to get IPG clock!\n");
+		err = PTR_ERR(clk);
+		goto out_err;
+	}
+	err = clk_prepare_enable(clk);
+	if (err) {
+		dev_err(port->dev, "Failed to enable IPG clock!\n");
+		goto out_err;
+	}
+	psc_ipg_clk[psc_num] = clk;
+
 	return 0;
+
+out_err:
+	if (psc_mclk_clk[psc_num]) {
+		clk_disable_unprepare(psc_mclk_clk[psc_num]);
+		psc_mclk_clk[psc_num] = NULL;
+	}
+	if (psc_ipg_clk[psc_num]) {
+		clk_disable_unprepare(psc_ipg_clk[psc_num]);
+		psc_ipg_clk[psc_num] = NULL;
+	}
+	return err;
 }
 
 /* called from within the .release_port() callback (release) */
@@ -656,6 +682,10 @@ static void mpc512x_psc_relse_clock(struct uart_port *port)
 		clk_disable_unprepare(clk);
 		psc_mclk_clk[psc_num] = NULL;
 	}
+	if (psc_ipg_clk[psc_num]) {
+		clk_disable_unprepare(psc_ipg_clk[psc_num]);
+		psc_ipg_clk[psc_num] = NULL;
+	}
 }
 
 /* implementation of the .clock() callback (enable/disable) */
-- 
1.7.10.4

[PATCH v5 10/17] serial: mpc512x: setup the PSC FIFO clock as well

[Gerhard Sittig]

prepare and enable the FIFO clock upon PSC FIFO initialization,
check for and propagage errors when enabling the PSC FIFO clock,
disable and unprepare the FIFO clock upon PSC FIFO uninitialization

devm_{get,put}_clk() doesn't apply here, as the SoC provides a
single FIFO component which is shared among several PSC components,
thus the FIFO isn't associated with a device (while the PSCs are)

provide a fallback clock lookup approach in case the OF based clock
lookup for the PSC FIFO fails, this allows for successful operation in
the presence of an outdated device tree which lacks clock specs

Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jiri Slaby <jslaby@suse.cz>
Cc: linux-serial@vger.kernel.org
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> # for v4
Signed-off-by: Gerhard Sittig <gsi@denx.de>
---
Greg, the addition since v4 is the clk_get_sys() call for the 'ipg'
clock item (backwards compat for device trees w/o clock specs)

---
 drivers/tty/serial/mpc52xx_uart.c |   50 ++++++++++++++++++++++++++++++++-----
 1 file changed, 44 insertions(+), 6 deletions(-)

diff --git a/drivers/tty/serial/mpc52xx_uart.c b/drivers/tty/serial/mpc52xx_uart.c
index 1d1dbe9d5af2..78c8cbadc776 100644
--- a/drivers/tty/serial/mpc52xx_uart.c
+++ b/drivers/tty/serial/mpc52xx_uart.c
@@ -421,6 +421,7 @@ struct psc_fifoc {
 
 static struct psc_fifoc __iomem *psc_fifoc;
 static unsigned int psc_fifoc_irq;
+static struct clk *psc_fifoc_clk;
 
 static void mpc512x_psc_fifo_init(struct uart_port *port)
 {
@@ -568,36 +569,73 @@ static unsigned int mpc512x_psc_set_baudrate(struct uart_port *port,
 /* Init PSC FIFO Controller */
 static int __init mpc512x_psc_fifoc_init(void)
 {
+	int err;
 	struct device_node *np;
+	struct clk *clk;
+
+	/* default error code, potentially overwritten by clock calls */
+	err = -ENODEV;
 
 	np = of_find_compatible_node(NULL, NULL,
 				     "fsl,mpc5121-psc-fifo");
 	if (!np) {
 		pr_err("%s: Can't find FIFOC node\n", __func__);
-		return -ENODEV;
+		goto out_err;
+	}
+
+	clk = of_clk_get(np, 0);
+	if (IS_ERR(clk)) {
+		/* backwards compat with device trees that lack clock specs */
+		clk = clk_get_sys(np->name, "ipg");
+	}
+	if (IS_ERR(clk)) {
+		pr_err("%s: Can't lookup FIFO clock\n", __func__);
+		err = PTR_ERR(clk);
+		goto out_ofnode_put;
+	}
+	if (clk_prepare_enable(clk)) {
+		pr_err("%s: Can't enable FIFO clock\n", __func__);
+		clk_put(clk);
+		goto out_ofnode_put;
 	}
+	psc_fifoc_clk = clk;
 
 	psc_fifoc = of_iomap(np, 0);
 	if (!psc_fifoc) {
 		pr_err("%s: Can't map FIFOC\n", __func__);
-		of_node_put(np);
-		return -ENODEV;
+		goto out_clk_disable;
 	}
 
 	psc_fifoc_irq = irq_of_parse_and_map(np, 0);
-	of_node_put(np);
 	if (psc_fifoc_irq == 0) {
 		pr_err("%s: Can't get FIFOC irq\n", __func__);
-		iounmap(psc_fifoc);
-		return -ENODEV;
+		goto out_unmap;
 	}
 
+	of_node_put(np);
 	return 0;
+
+out_unmap:
+	iounmap(psc_fifoc);
+out_clk_disable:
+	clk_disable_unprepare(psc_fifoc_clk);
+	clk_put(psc_fifoc_clk);
+out_ofnode_put:
+	of_node_put(np);
+out_err:
+	return err;
 }
 
 static void __exit mpc512x_psc_fifoc_uninit(void)
 {
 	iounmap(psc_fifoc);
+
+	/* disable the clock, errors are not fatal */
+	if (psc_fifoc_clk) {
+		clk_disable_unprepare(psc_fifoc_clk);
+		clk_put(psc_fifoc_clk);
+		psc_fifoc_clk = NULL;
+	}
 }
 
 /* 512x specific interrupt handler. The caller holds the port lock */
-- 
1.7.10.4

Re: [PATCH v5 00/17] add COMMON_CLK support for PowerPC MPC512x

[Gerhard Sittig]

On Mon, Nov 18, 2013 at 00:06 +0100, Gerhard Sittig wrote:
> 
> the series is based on v3.12, but I'll rebase against v3.13-rc1
> (when available) or any other subtree upon request

Now that v3.13-rc1 is out, I noticed that the series no longer
applies cleanly (minor context changes and conflicts) and needs
minor adjustment.

Compilation of 4/17 requires <linux/of_address.h> which no longer
is included implicitly.

PPC_CLOCK removal in 7/17 should remove <asm/clk_interface.h> as
well after all references to this header file have gone.  (And
the context of the patch has changed.)

The context of 16/17 for DIU initialization has changed.

I'll wait for a few more days whether there is more feedback for
v5 (especially on the device tree backwards compat approach),
then will rebase and send out v6 of the series.


virtually yours
Gerhard Sittig
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr. 5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office@denx.de