add COMMON_CLK support for PowerPC MPC512x

this series introduces support for the common clock framework (CCF, COMMON_CLK Kconfig option) in the PowerPC based MPC512x platform, a resulting debugfs clk_summary is at the end of the message after the stats

although the series does touch several subsystems -- serial, spi, net (can, fs_enet), mtd (nfc), usb, i2c, media (viu), and dts -- all of the patches are strictly clock related, changes outside of the clock driver in the mpc512x platform are rather small and local to probe and setup

the distribution was chosen such to not spam too many lists, yet reach all the relevant maintainers (there appears to be no clk related list, it's assumed that the clk API is ARM centric, which is why I send the PowerPC related changes to the ARM list as well), but I'd be happy to add more participants if necessary

it appears most appropriate to take this series through either the clk or the powerpc trees after it has passed review and other subsystem maintainers ACKed the clock setup related driver modifications -- splitting the series and taking the parts through several trees might reorder the set (make drivers fail to see the device tree specs) or would create unnecessary dependencies which need to get tracked upon merges and might defer propagation (unless I missed something)

the series passes 'checkpatch.pl --strict' except for one warning which cannot get resolved, since that either breaks compilation (the data type is preset by the clk-provider.h API) or requires a cast which shadows real mismatches:

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

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

Gerhard Sittig (24):
spi: mpc512x: prepare clocks before enabling them
serial: mpc512x: prepare clocks before enabling them
mtd: mpc5121_nfc: prepare clocks before enabling them
powerpc: mpc512x: array decl for MCLK registers in CCM
clk: wrap I/O access for improved portability
dts: mpc512x: prepare for preprocessor support
dts: mpc512x: introduce dt-bindings/clock/ header
dts: mpc512x: add clock related device tree specs
clk: mpc512x: introduce COMMON_CLK for MPC512x
dts: mpc512x: add clock specs for client lookups
net: can: mscan: add a comment on reg to idx mapping
net: can: mscan: make mpc512x code use common clock
spi: mpc512x: OF clock lookup, use the 'mclk' name powerpc/mpc512x: improve DIU related clock setup
serial: mpc512x: OF clock lookup, use the 'mclk' name
clk: mpc512x: remove now obsolete clkdev registration
serial: mpc512x: setup the PSC FIFO clock as well
i2c: mpc: OF clock lookup for MPC512x
USB: fsl-mph-dr-of: OF clock lookup, prepare and enable
fs_enet: OF clock lookup (non-fatal), prepare and enable [media] fsl-viu: OF clock lookup, prepare before enable powerpc/fsl-pci: OF clock lookup, prepare before enable
clk: mpc512x: switch to COMMON_CLK, remove PPC_CLOCK
net: can: mscan: remove MPC512x non-COMMON_CLK code path

 arch/powerpc/boot/dts/ac14xx.dts                   |    9 +-
 arch/powerpc/boot/dts/include/dt-bindings          |    1 +
 arch/powerpc/boot/dts/mpc5121.dtsi                 |   94 ++-
 arch/powerpc/boot/dts/mpc5121ads.dts               |    2 +-
 arch/powerpc/boot/dts/pdm360ng.dts                 |    2 +-
 arch/powerpc/include/asm/mpc5121.h                 |   18 +-
 arch/powerpc/platforms/512x/Kconfig                |    2 +-
 arch/powerpc/platforms/512x/Makefile               |    3 +-
 arch/powerpc/platforms/512x/clock-commonclk.c      |  762 ++++++++++++++++++++
 arch/powerpc/platforms/512x/clock.c                |  753 -------------------
 arch/powerpc/platforms/512x/mpc512x_shared.c       |  165 +++--
 arch/powerpc/sysdev/fsl_pci.c                      |   15 +
 drivers/clk/clk-divider.c                          |    6 +-
 drivers/clk/clk-gate.c                             |    6 +-
 drivers/clk/clk-mux.c                              |    6 +-
 drivers/i2c/busses/i2c-mpc.c                       |    9 +
 drivers/media/platform/fsl-viu.c                   |   12 +-
 drivers/mtd/nand/mpc5121_nfc.c                     |    4 +-
 drivers/net/can/mscan/mpc5xxx_can.c                |  234 +++---
 .../net/ethernet/freescale/fs_enet/fs_enet-main.c  |   27 +-
 drivers/spi/spi-mpc512x-psc.c                      |    6 +-
 drivers/tty/serial/mpc52xx_uart.c                  |   59 +-
 drivers/usb/host/fsl-mph-dr-of.c                   |   24 +-
 include/dt-bindings/clock/mpc512x-clock.h          |   59 ++
 include/linux/clk-provider.h                       |   33 +
 include/linux/fs_enet_pd.h                         |    3 +
 26 files changed, 1299 insertions(+), 1015 deletions(-)
 create mode 120000 arch/powerpc/boot/dts/include/dt-bindings
 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

$ uname -srm
Linux 3.11.0-rc1-00024-g25dc674 ppc
$ cat /sys/kernel/debug/clk/clk_summary
   clock                        enable_cnt  prepare_cnt  rate
---------------------------------------------------------------------
 ac97                           0           0            24567000
 spdif_rx_in                    0           0            0
 spdif_tx_in                    0           0            0
 psc_mclk_in                    0           0            25000000
 dummy                          1           1            0
 osc                            1           1            25000000
    ref                         1           1            25000000
       sys                      9           9            320000000
          spdif-mux0            0           0            320000000
             spdif-en0          0           0            320000000
                spdif_mclk_div  0           0            80000000
                   spdif_mclk_out 0           0            80000000
                      spdif_mclk 0           0            80000000
          mscan3-mux0           1           1            320000000
             mscan3-en0         1           1            320000000
                mscan3_mclk_div 1           1            80000000
                   mscan3_mclk_out 1           1            80000000
                      mscan3_mclk 1           1            80000000
          mscan2-mux0           1           1            320000000
             mscan2-en0         1           1            320000000
                mscan2_mclk_div 1           1            80000000
                   mscan2_mclk_out 1           1            80000000
                      mscan2_mclk 1           1            80000000
          mscan1-mux0           1           1            320000000
             mscan1-en0         1           1            320000000
                mscan1_mclk_div 1           1            16000000
                   mscan1_mclk_out 1           1            16000000
                      mscan1_mclk 2           2            16000000
          mscan0-mux0           1           1            320000000
             mscan0-en0         1           1            320000000
                mscan0_mclk_div 1           1            16000000
                   mscan0_mclk_out 1           1            16000000
                      mscan0_mclk 2           2            16000000
          psc11-mux0            0           0            320000000
             psc11-en0          0           0            320000000
                psc11_mclk_div  0           0            80000000
                   psc11_mclk_out 0           0            80000000
                      psc11_mclk 0           0            80000000
          psc10-mux0            0           0            320000000
             psc10-en0          0           0            320000000
                psc10_mclk_div  0           0            80000000
                   psc10_mclk_out 0           0            80000000
                      psc10_mclk 0           0            80000000
          psc9-mux0             0           0            320000000
             psc9-en0           0           0            320000000
                psc9_mclk_div   0           0            80000000
                   psc9_mclk_out 0           0            80000000
                      psc9_mclk 0           0            80000000
          psc8-mux0             0           0            320000000
             psc8-en0           0           0            320000000
                psc8_mclk_div   0           0            80000000
                   psc8_mclk_out 0           0            80000000
                      psc8_mclk 0           0            80000000
          psc7-mux0             0           0            320000000
             psc7-en0           0           0            320000000
                psc7_mclk_div   0           0            80000000
                   psc7_mclk_out 0           0            80000000
                      psc7_mclk 0           0            80000000
          psc6-mux0             0           0            320000000
             psc6-en0           0           0            320000000
                psc6_mclk_div   0           0            80000000
                   psc6_mclk_out 0           0            80000000
                      psc6_mclk 0           0            80000000
          psc5-mux0             1           1            320000000
             psc5-en0           1           1            320000000
                psc5_mclk_div   1           1            80000000
                   psc5_mclk_out 1           1            80000000
                      psc5_mclk 1           1            80000000
          psc4-mux0             1           1            320000000
             psc4-en0           1           1            320000000
                psc4_mclk_div   1           1            80000000
                   psc4_mclk_out 1           1            80000000
                      psc4_mclk 1           1            80000000
          psc3-mux0             1           1            320000000
             psc3-en0           1           1            320000000
                psc3_mclk_div   1           1            80000000
                   psc3_mclk_out 1           1            80000000
                      psc3_mclk 1           1            80000000
          psc2-mux0             0           0            320000000
             psc2-en0           0           0            320000000
                psc2_mclk_div   0           0            80000000
                   psc2_mclk_out 0           0            80000000
                      psc2_mclk 0           0            80000000
          psc1-mux0             0           0            320000000
             psc1-en0           0           0            320000000
                psc1_mclk_div   0           0            80000000
                   psc1_mclk_out 0           0            80000000
                      psc1_mclk 0           0            80000000
          psc0-mux0             0           0            320000000
             psc0-en0           0           0            320000000
                psc0_mclk_div   0           0            80000000
                   psc0_mclk_out 0           0            80000000
                      psc0_mclk 0           0            80000000
          ddr-ug                1           1            160000000
             ddr                1           1            160000000
          csb                   4           4            160000000
             viu                0           0            160000000
             iim                0           0            160000000
             usb2               0           0            160000000
             usb1               0           0            160000000
             axe                0           0            160000000
             pci-ug             0           0            26666666
                pci             0           0            26666666
             mbx-bus-ug         0           0            80000000
                mbx-bus         0           0            80000000
                mbx-ug          0           0            80000000
                   mbx          0           0            80000000
                   mbx-3d-ug    0           0            80000000
                      mbx-3d    0           0            80000000
             e300               1           1            400000000
             diu-x4             1           1            640000000
                diu-ug          1           1            2509803
                   diu          2           2            2509803
             sdhc-x4            1           1            640000000
                sdhc-ug         1           1            53333333
                   sdhc-2       0           0            53333333
                   sdhc         1           1            53333333
             ips                7           7            80000000
                i2c             3           3            80000000
                mem             1           1            80000000
                fec             1           1            80000000
                sata            0           0            80000000
                psc-fifo        1           1            80000000
                pata            0           0            80000000
                lpc-ug          1           1            40000000
                   lpc          1           1            40000000
                nfc-ug          0           0            40000000
                   nfc          0           0            40000000

1.7.10.4

clocks need to get prepared before they can get enabled, fix the MPC512x PSC SPI master's initialization

---
 drivers/spi/spi-mpc512x-psc.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index 29fce6a..76b20ea 100644
@@ -395,7 +395,7 @@ static int mpc512x_psc_spi_port_config(struct spi_master *master,
 
 	sprintf(name, "psc%d_mclk", master->bus_num);
 	spiclk = clk_get(&master->dev, name);
-	clk_enable(spiclk);
+	clk_prepare_enable(spiclk);
 	mps->mclk = clk_get_rate(spiclk);
 	clk_put(spiclk);
 

1.7.10.4

"This code" refers to the driver's original condition, right? I agree that the driver has been suffering from incomplete clock handling since it was born three years ago. And that this issue shall get addressed. The question is just whether it needs to be part of this series which has a different focus.

What the above patch addresses is prevention of an immediate and fatal breakage, when common clock gets used but clocks aren't prepared before getting enabled. So I consider it appropriate as a step in preparation before introducing support for common clock on the platform. (It's funny that I had a comment in this spirit in the commit message, but trimmed it to not be overly verbose.)

What the patch does not address is to fix any other deficiencies in the driver which might have been lurking there for ages. This would be the scope of a different patch or series, as addressing it here as well would mix orthogonal issues within one series, and would complicate review and test (and would delay or even potentially kill the introduction of desirable support for common infrastructure just because other legacy non-fatal issues aren't addressed as well in bypassing).

I will look into what I can do to address your concerns about proper general clock handling in this specific driver. But I'm unable to do this for all other drivers as well which I happen to pass by as I work on platform code (partially due to lack of knowledge). In any case I won't be able to test all of these changes in all subsystems (mostly due to lack of hardware and test setups).

So I suggest to leave these general cleanup activities for a separate series. The current series certainly improves general platform code, transparently brings new drivers into successful operation, and keeps the quality of existing code (doesn't break anything, does even actively prevent breakage). So it shall be acceptable despite its not being perfect (like addressing each and every other aspect which may arise elsewhere). Where would I stop then?

Sorry for the lengthy reply, but I guess it's about just one general aspect which equally applies to other parts of the series, not just the specific SPI driver which the feedback was provided for. And I do appreciate your looking at the submission.

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 at denx.de

This is a pretty long e-mail. It'd probably have taken less time to fix the issues than to reply to the e-mail... but anyway.

A big part of the issue with the state of the driver is that there's
obvious clock API abuse going on that isn't corrected here - the main
one is that the sprintf() for the clock name is a fairly clear warning
sign, the driver should be using a fixed value there. This raises a
warning flag for me about the quality of the common clock API
implementation that's being sent and/or the potential for bugs to be
noticed with the common clock framework. I'd not expect this code to
actually work, and looking at the rest of the series I don't see how it
does since I can't see what forces the name.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130717/245057e2/attachment-0001.sig>

[ this is meta stuff, technical details are after the next quotation ]

Not quite. Please consider that careful submission is as expensive as thorough review is, and that a lot of work is done before v1 gets submitted, which you may not always notice from looking at a concentrated series that may no longer suggest how much it took to get there (especially when prepared carefully).

As mentioned before I did not question the need to fix issues as they get identified. I just asked whether reworking the serial driver is in the scope of this series which provides a different clock driver implementation for the platform. To me these two things are orthogonal, while I did promise to see how I can address your concerns.

Let's provide the technical information you need to judge the quality of the submission and see why it shall be acceptable:

Why the code does work:

OK, here is why the driver keeps its state throughout the set and is operational as good or as bad as it was before:

The sprintf() in the SPI driver used to construct a name which includes the PSC index. This applies to the UART driver as well, as they both use the PSC controller hardware to implement their serial communication.

The corresponding clock name was found in the previous PPC_CLOCK implementation since the clock driver provided those names which include the PSC index (fixed strings in a table).

The initial COMMON_CLK implementation in part 09/24 registers clkdev items for compatibility during migration. The string isn't greppable since it's constructed by the preprocessor in the MCLK data setup, see the MCLK_SETUP_DATA_PSC() macro.

Part 13/24 adjusts the SPI driver to no longer construct a name, but instead use a fixed string after OF based clock lookup became available. This shall meet your expectation and simplifies the client side. Part 15/24 does the same for the UART driver.

Part 16/24 removes the clkdev registration in the clock driver after both the UART and SPI clients switched to OF clock lookups.

At this stage things are the way you would expect: The client uses a fixed string in the lookup, while lookup occurs via device tree, and instances are supported without the clients' constructing something based on a component index.

Remaining issues are not with the common clock support of the platform, but in the serial driver and are identical to the previous (actually: the initial) status. While we agree on the existance of the remaining issue and the desire to address it. Just not on which context that fix shall be done in.

The series' status and perspective:

The UART and SPI drivers did work before, while it's true that clock handling wasn't complete (in non-fatal ways). This has been the case in the past, and has been identified just now.

The serial drivers are kept operational during migration, and still are operational after the series. But now they use common clock and OF lookups, which is an improvement for the platform in my eyes.

It's true that the status of the serial driver isn't improved with the series -- but it isn't degraded either, while (additional) breakage actively gets prevented.

Now let me see how I can improve the SPI driver with regard to overall clock handling and beyond mere operation by coincidence in the absence of errors. But please understand that I don't want to stall the common clock support for a whole platform just because some of the drivers it needs to touch to keep them operational have other issues that weren't addressed yet, while these issues aren't introduced with the series but preceed it.

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 at denx.de

This is rather undermined the more time and effort gets spent pushing back against doing trival fixes, of course... besides, the issues here are all really simple to fix and test. It's not a major or risky rewrite of the driver or anything like that - most of this can be tested by just probing the driver.

OK, so send patches then.

Ugh, right - it didn't show up in searches due to being hidden by the macro.

Again, you're not being asked to implement substantial new functionality here. From my point of view I can't test this at all so I'm looking at code that's obviously not good for the standard clock API and wondering if it even works or how robust it's going to be going forward as the common clock code changes which makes me relucatant to say it'll be OK.

The fact that you're switching over to use generic code is itself a
reason to make sure that the API usage is sane, dodgy API usage against
open coded clock implementations is less risky than against the common
code.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130717/df54aed2/attachment.sig>

clocks need to get prepared before they can get enabled, and after disabling them they can get unprepared

---
 drivers/tty/serial/mpc52xx_uart.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

index e1280a2..53c1093 100644
@@ -636,9 +636,9 @@ static int mpc512x_psc_clock(struct uart_port *port, int enable)
 	dev_dbg(port->dev, "%s %sable\n", clk_name, enable ? "en" : "dis");
 
 	if (enable)
-		clk_enable(psc_clk);
+		clk_prepare_enable(psc_clk);
 	else
-		clk_disable(psc_clk);
+		clk_disable_unprepare(psc_clk);
 
 	return 0;
 }

1.7.10.4

must prepare clocks before enabling them, unprepare after disable

---
 drivers/mtd/nand/mpc5121_nfc.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

index 3c9cdcb..eb7771d 100644
@@ -618,7 +618,7 @@ static void mpc5121_nfc_free(struct device *dev, struct mtd_info *mtd)
 	struct mpc5121_nfc_prv *prv = chip->priv;
 
 	if (prv->clk) {
-		clk_disable(prv->clk);
+		clk_disable_unprepare(prv->clk);
 		clk_put(prv->clk);
 	}
 

@@ -737,7 +737,7 @@ static int mpc5121_nfc_probe(struct platform_device *op)
 		goto error;
 	}
 
-	clk_enable(prv->clk);
+	clk_prepare_enable(prv->clk);
 
 	/* Reset NAND Flash controller */
 	nfc_set(mtd, NFC_CONFIG1, NFC_RESET);

1.7.10.4

reword the clock control module's registers declaration such that the MCLK related registers form an array and get indexed by PSC number

this change is in preparation to COMMON_CLK support for the MPC512x platform, the changed declaration remains neutral to existing code since the PSC and MSCAN CCR fields declared here aren't referenced anywhere

---
 arch/powerpc/include/asm/mpc5121.h |   18 ++----------------
 1 file changed, 2 insertions(+), 16 deletions(-)

index 8ae133e..887d3d6 100644
@@ -32,25 +32,11 @@ struct mpc512x_ccm {
 	u32	scfr2;	/* System Clock Frequency Register 2 */
 	u32	scfr2s;	/* System Clock Frequency Shadow Register 2 */
 	u32	bcr;	/* Bread Crumb Register */
-	u32	p0ccr;	/* PSC0 Clock Control Register */
-	u32	p1ccr;	/* PSC1 CCR */
-	u32	p2ccr;	/* PSC2 CCR */
-	u32	p3ccr;	/* PSC3 CCR */
-	u32	p4ccr;	/* PSC4 CCR */
-	u32	p5ccr;	/* PSC5 CCR */
-	u32	p6ccr;	/* PSC6 CCR */
-	u32	p7ccr;	/* PSC7 CCR */
-	u32	p8ccr;	/* PSC8 CCR */
-	u32	p9ccr;	/* PSC9 CCR */
-	u32	p10ccr;	/* PSC10 CCR */
-	u32	p11ccr;	/* PSC11 CCR */
+	u32	psc_ccr[12];	/* PSC Clock Control Registers */
 	u32	spccr;	/* SPDIF Clock Control Register */
 	u32	cccr;	/* CFM Clock Control Register */
 	u32	dccr;	/* DIU Clock Control Register */
-	u32	m1ccr;	/* MSCAN1 CCR */
-	u32	m2ccr;	/* MSCAN2 CCR */
-	u32	m3ccr;	/* MSCAN3 CCR */
-	u32	m4ccr;	/* MSCAN4 CCR */
+	u32	mscan_ccr[4];	/* MSCAN Clock Control Registers */
 	u8	res[0x98]; /* Reserved */
 };
 

1.7.10.4

the common clock drivers were motivated/initiated by ARM development and apparently assume little endian peripherals

wrap register/peripherals access in the common code (div, gate, mux) in preparation of adding COMMON_CLK support for other platforms

---
 drivers/clk/clk-divider.c    |    6 +++---
 drivers/clk/clk-gate.c       |    6 +++---
 drivers/clk/clk-mux.c        |    6 +++---
 include/linux/clk-provider.h |   17 +++++++++++++++++
 4 files changed, 26 insertions(+), 9 deletions(-)

index 6d55eb2..2c07061 100644
@@ -104,7 +104,7 @@ static unsigned long clk_divider_recalc_rate(struct clk_hw *hw,
 	struct clk_divider *divider = to_clk_divider(hw);
 	unsigned int div, val;
 
-	val = readl(divider->reg) >> divider->shift;
+	val = clk_readl(divider->reg) >> divider->shift;
 	val &= div_mask(divider);
 
 	div = _get_div(divider, val);

@@ -230,11 +230,11 @@ static int clk_divider_set_rate(struct clk_hw *hw, unsigned long rate,
 	if (divider->flags & CLK_DIVIDER_HIWORD_MASK) {
 		val = div_mask(divider) << (divider->shift + 16);
 	} else {
-		val = readl(divider->reg);
+		val = clk_readl(divider->reg);
 		val &= ~(div_mask(divider) << divider->shift);
 	}
 	val |= value << divider->shift;
-	writel(val, divider->reg);
+	clk_writel(val, divider->reg);
 
 	if (divider->lock)
 		spin_unlock_irqrestore(divider->lock, flags);

index 790306e..b7fbd96 100644
@@ -58,7 +58,7 @@ static void clk_gate_endisable(struct clk_hw *hw, int enable)
 		if (set)
 			reg |= BIT(gate->bit_idx);
 	} else {
-		reg = readl(gate->reg);
+		reg = clk_readl(gate->reg);
 
 		if (set)
 			reg |= BIT(gate->bit_idx);

@@ -66,7 +66,7 @@ static void clk_gate_endisable(struct clk_hw *hw, int enable)
 			reg &= ~BIT(gate->bit_idx);
 	}
 
-	writel(reg, gate->reg);
+	clk_writel(reg, gate->reg);
 
 	if (gate->lock)
 		spin_unlock_irqrestore(gate->lock, flags);

@@ -89,7 +89,7 @@ static int clk_gate_is_enabled(struct clk_hw *hw)
 	u32 reg;
 	struct clk_gate *gate = to_clk_gate(hw);
 
-	reg = readl(gate->reg);
+	reg = clk_readl(gate->reg);
 
 	/* if a set bit disables this clk, flip it before masking */
 	if (gate->flags & CLK_GATE_SET_TO_DISABLE)

index 614444c..02ef506 100644
@@ -42,7 +42,7 @@ static u8 clk_mux_get_parent(struct clk_hw *hw)
 	 * OTOH, pmd_trace_clk_mux_ck uses a separate bit for each clock, so
 	 * val = 0x4 really means "bit 2, index starts at bit 0"
 	 */
-	val = readl(mux->reg) >> mux->shift;
+	val = clk_readl(mux->reg) >> mux->shift;
 	val &= mux->mask;
 
 	if (mux->table) {

@@ -89,11 +89,11 @@ static int clk_mux_set_parent(struct clk_hw *hw, u8 index)
 	if (mux->flags & CLK_MUX_HIWORD_MASK) {
 		val = mux->mask << (mux->shift + 16);
 	} else {
-		val = readl(mux->reg);
+		val = clk_readl(mux->reg);
 		val &= ~(mux->mask << mux->shift);
 	}
 	val |= index << mux->shift;
-	writel(val, mux->reg);
+	clk_writel(val, mux->reg);
 
 	if (mux->lock)
 		spin_unlock_irqrestore(mux->lock, flags);

index 1ec14a7..c4f7799 100644
@@ -12,6 +12,7 @@
 #define __LINUX_CLK_PROVIDER_H
 
 #include <linux/clk.h>
+#include <linux/io.h>
 
 #ifdef CONFIG_COMMON_CLK
 

@@ -490,5 +491,21 @@ static inline const char *of_clk_get_parent_name(struct device_node *np,
 #define of_clk_init(matches) \
 	{ while (0); }
 #endif /* CONFIG_OF */
+
+/*
+ * wrap access to peripherals in accessor routines
+ * for improved portability across platforms
+ */
+
+static inline u32 clk_readl(u32 __iomem *reg)
+{
+	return readl(reg);
+}
+
+static inline void clk_writel(u32 val, u32 __iomem *reg)
+{
+	writel(val, reg);
+}
+
 #endif /* CONFIG_COMMON_CLK */
 #endif /* CLK_PROVIDER_H */

1.7.10.4

Does regmap assume that those registers form a (dense) array of equal sized items? Does it introduce unconditional indirection and requirement for explicit setup beyond mere mapping? What's the overhead compared to the readl/inbe32 approach that's currently resolved at compile time?

Neither of the above needs to be a blocker, just needs to be acceptable after consideration. So far nobody appears to have felt pain with the LE32 register assumption.

BTW does the common clock support that I introduce for MPC512x only cover those parts from crystal over internal busses to internal peripherals (register files).

It does not include bitrate generation within the peripherals -- this appears to remain the domain of individual drivers, which keep manipulating register fields to derive wire bitrates from internal references. Sharing drivers between platforms with and without common clock support forbids the switch anyway, or both CCF abstraction as well as local register manipulation need to get implemented in parallel, as I did for the mscan(4) driver.

Some of these bitrate related registers aren't 32bit entities and thus could not get managed by the common clock primitives in their current form. Some of the IP blocks may even spread integer values across several non-adjacent bit fields for legacy reasons, but I guess that these aren't in the scope of the shared primitives either (and they may not be popular either).

While we are at improvements for the common clock primitives:

What I missed was support for fractional dividers, i.e. dividers with a register bitfield backed divider part but a fixed factor multiplier part. Currently there's only dividers (bitfield factor or bitfield with table lookup for the factor) and fixed-factor (multiplier and divider, but both of them fixed and not adjustable by register manipulation). This was addressed by "intermediate" clock items ("ungated", "x4")

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 at denx.de

Some other thought crossed my mind regarding access to clock control registers that reside behind some communication channel like I2C:

The common clock API assumes (it's part of the contract) that there are potentially expensive operations like get, put, prepare and unprepare, as well as swift and non-blocking operations like enable and disable.

Would the regmap abstraction hide the potentially blocking nature of a register access (I understand that you can implement "local" as well as "remote" register sets by this mechanism)? Or could you still meet the assumptions or requirements of the common clock API?

It might as well be the responsibility of the clock driver's implementor to arrange for the availability of non-blocking enable/disable operations, just as it is today. Such that expensive register access need not be forbidden in general.

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 at denx.de

regmap for mmio uses a spinlock for read/modify/write operations, just like you have to use a spinlock in the common clk dividers/gates. This part wouldn't change with regmap.

For i2c connected clocks where a spinlock doesn't work due to the nonatomic nature of i2c devices we would have to move the enable/disble stuff to prepare/unprepare in the common gate code. This can be left for someone who works on i2c clocks though.

I think regmap has the potential to solve a number of issues like the hardcoded readl/writel in the common clock blocks, issues with i2c clocks and your endianess issue. The biggest question probably is how to get there without putting too much of a burden on you. It's probably not an option to convert all users to regmap, so it seems additional functions like clk_register_gate_regmap are better to handle.

Sascha

Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

That's basically what regulator does. There's versions of the ops that
all the drivers can use.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130718/43ffb513/attachment.sig>

Let's get something straight here, because what you've said above is wrong.

1. clk_get() and clk_put() are NOT part of the common clock API. They're separate - they're part of the clk API, and the infrastructure behind that is clkdev, which is a separately owned thing (by me.)

2. The "contract" of the clk API is defined by the clk API, not by some random implementation like the common clock API. The clk API is maintained by myself, and is described in include/linux/clk.h

3. clk_prepare() and clk_unprepare() are functions MUST only be called from contexts where sleeping is permitted. These functions MAY sleep for whatever reason they require to, and as long as they require to. (This is the whole reason these two functions were created in the first place.)

4. clk_enable() and clk_disable() MAY be called from any context, but MUST never sleep. If you need to talk over a non-atomic bus for these, then these functions should be no-ops, and the code which does that must be executed from the clk_prepare()/clk_unprepare() operations.

That is the "clk API" contract. The CCF has no bearing on this; if it disagrees, then the CCF is buggy and is non-conformant.

Could the above be included in some form in Documentation/clk.txt (this is likely one of the first location people look for information) and elsewhere if appropriate please?

A *lot* of people are confused by the prepare-enable-disable-unprepare sequence and when I try to find some rational for the prepare/enable split I can only direct them to mail archive posts since this is nowhere to be found in the kernel.

The comments in include/linux/clk.h, while correct, are very terse and don't provide any insight to the reason why there is a split in the API.

The content of Documentation/clk.txt does refer to prepare and enable (and their counterparts) but again doesn't provide any clue about the reason for their existence.

Since there've been several good posts with usage example now buried into list archives, I think this would go a long way helping people get it right if those were part of the kernel documentation as well.

Nicolas

Quoting Nicolas Pitre (2013-07-18 10:47:22)

I'll update Documentation/clk.txt with more verbosity. The document was originally intended as a "porting guide" to help migrate from legacy frameworks to the common struct clk implementation. However the scope of the document should probably be generalized a bit more.

Regards,
Mike

prepare C preprocessor support when processing MPC512x DTS files
- switch from DTS syntax to CPP syntax for include specs
- create a symlink such that DTS processing can reference includes

---
 arch/powerpc/boot/dts/ac14xx.dts          |    2 +-
 arch/powerpc/boot/dts/include/dt-bindings |    1 +
 arch/powerpc/boot/dts/mpc5121ads.dts      |    2 +-
 arch/powerpc/boot/dts/pdm360ng.dts        |    2 +-
 4 files changed, 4 insertions(+), 3 deletions(-)
 create mode 120000 arch/powerpc/boot/dts/include/dt-bindings

index a27a460..a543c40 100644
@@ -10,7 +10,7 @@
  */
 
 
-/include/ "mpc5121.dtsi"
+#include <mpc5121.dtsi>
 
 / {
 	model = "ac14xx";

new file mode 120000
index 0000000..08c00e4
@@ -0,0 +1 @@
+../../../../../include/dt-bindings
\ No newline at end of file

index 7d3cb79..c228a0a 100644
@@ -9,7 +9,7 @@
  * option) any later version.
  */
 
-/include/ "mpc5121.dtsi"
+#include <mpc5121.dtsi>
 
 / {
 	model = "mpc5121ads";

index 7433740..871c16d 100644
@@ -13,7 +13,7 @@
  * option) any later version.
  */
 
-/include/ "mpc5121.dtsi"
+#include <mpc5121.dtsi>
 
 / {
 	model = "pdm360ng";

1.7.10.4

introduce a dt-bindings/ header file for MPC512x clocks, providing symbolic identifiers for those SoC clocks which clients will reference from their device tree nodes

---
 include/dt-bindings/clock/mpc512x-clock.h |   59 +++++++++++++++++++++++++++++
 1 file changed, 59 insertions(+)
 create mode 100644 include/dt-bindings/clock/mpc512x-clock.h

new file mode 100644
index 0000000..46c560e
@@ -0,0 +1,59 @@
+/*
+ * This header provides constants for MPC512x clock specs in DT bindings.
+ *
+ * Unfortunately the clock number declaration cannot be an enum but
+ * needs to be a list of #define directives since when referenced from
+ * within DTS files they need to get resolved "at compile time".
+ */
+
+#ifndef _DT_BINDINGS_CLOCK_MPC512x_CLOCK_H
+#define _DT_BINDINGS_CLOCK_MPC512x_CLOCK_H
+
+#define MPC512x_CLK_DUMMY		0
+#define MPC512x_CLK_REF			1
+#define MPC512x_CLK_SYS			2
+#define MPC512x_CLK_DIU			3
+#define MPC512x_CLK_VIU			4
+#define MPC512x_CLK_CSB			5
+#define MPC512x_CLK_E300		6
+#define MPC512x_CLK_IPS			7
+#define MPC512x_CLK_FEC			8
+#define MPC512x_CLK_SATA		9
+#define MPC512x_CLK_PATA		10
+#define MPC512x_CLK_NFC			11
+#define MPC512x_CLK_LPC			12
+#define MPC512x_CLK_MBX_BUS		13
+#define MPC512x_CLK_MBX			14
+#define MPC512x_CLK_MBX_3D		15
+#define MPC512x_CLK_AXE			16
+#define MPC512x_CLK_USB1		17
+#define MPC512x_CLK_USB2		18
+#define MPC512x_CLK_I2C			19
+#define MPC512x_CLK_MSCAN0_MCLK		20
+#define MPC512x_CLK_MSCAN1_MCLK		21
+#define MPC512x_CLK_MSCAN2_MCLK		22
+#define MPC512x_CLK_MSCAN3_MCLK		23
+#define MPC512x_CLK_SDHC		24
+#define MPC512x_CLK_PCI			25
+#define MPC512x_CLK_PSC_MCLK_IN		26
+#define MPC512x_CLK_SPDIF_TX		27
+#define MPC512x_CLK_SPDIF_RX		28
+#define MPC512x_CLK_SPDIF_MCLK		29
+#define MPC512x_CLK_AC97		30
+#define MPC512x_CLK_PSC0_MCLK		31
+#define MPC512x_CLK_PSC1_MCLK		32
+#define MPC512x_CLK_PSC2_MCLK		33
+#define MPC512x_CLK_PSC3_MCLK		34
+#define MPC512x_CLK_PSC4_MCLK		35
+#define MPC512x_CLK_PSC5_MCLK		36
+#define MPC512x_CLK_PSC6_MCLK		37
+#define MPC512x_CLK_PSC7_MCLK		38
+#define MPC512x_CLK_PSC8_MCLK		39
+#define MPC512x_CLK_PSC9_MCLK		40
+#define MPC512x_CLK_PSC10_MCLK		41
+#define MPC512x_CLK_PSC11_MCLK		42
+#define MPC512x_CLK_PSC_FIFO		43
+
+#define MPC512x_CLK_LAST_PUBLIC		43
+
+#endif

1.7.10.4

this addresses the clock driver aka provider's side of clocks
- prepare for future '<&clks ID>' phandle references for device tree based clock lookup in client drivers
- introduce a 'clocks' subtree with an 'osc' node for the crystal or oscillator SoC input (fixed frequency)
- provide default values with 33MHz oscillator frequency in the common include (the 66MHz IPS bus already was there), add override values for the ifm AC14xx board which deviates from the reference design (25MHz xtal, 80MHz IPS bus)

---
 arch/powerpc/boot/dts/ac14xx.dts   |    7 +++++++
 arch/powerpc/boot/dts/mpc5121.dtsi |   15 ++++++++++++++-
 2 files changed, 21 insertions(+), 1 deletion(-)

index a543c40..a1b8837 100644
@@ -139,7 +139,14 @@
 		};
 	};
 
+	clocks {
+		osc {
+			clock-frequency = <25000000>;
+		};
+	};
+
 	soc at 80000000 {
+		bus-frequency = <80000000>;	/* 80 MHz ips bus */
 
 		clock at f00 {
 			compatible = "fsl,mpc5121rev2-clock", "fsl,mpc5121-clock";

index bd14c00..8f4cba0 100644
@@ -9,6 +9,8 @@
  * option) any later version.
  */
 
+#include <dt-bindings/clock/mpc512x-clock.h>
+
 /dts-v1/;
 
 / {

@@ -73,6 +75,16 @@
 		ranges = <0x0 0x0 0xfc000000 0x04000000>;
 	};
 
+	clocks {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		osc {
+			compatible = "fsl,mpc512x-osc", "fixed-clock";
+			clock-frequency = <33000000>;
+		};
+	};
+
 	soc at 80000000 {
 		compatible = "fsl,mpc5121-immr";
 		#address-cells = <1>;

@@ -118,9 +130,10 @@
 		};
 
 		/* Clock control */
-		clock at f00 {
+		clks: clock at f00 {
 			compatible = "fsl,mpc5121-clock";
 			reg = <0xf00 0x100>;
+			#clock-cells = <1>;
 		};
 
 		/* Power Management Controller */

1.7.10.4

this change introduces a clock infrastructure implementation for the MPC512x PowerPC platform which follows the COMMON_CLK approach and uses common clock drivers shared with other platforms

this driver implements the publicly announced set of clocks (which can get referenced by means of symbolic identifiers from the dt-bindings header file), as well as generates additional 'struct clk' items where the SoC hardware cannot easily get mapped to the common primitives of the clock API, or requires "intermediate" clock nodes to represent clocks that have both gates and dividers

the previous PPC_CLOCK implementation is kept in place and resides in parallel to the common clock implementation for test and comparison during migration, a compile time option picks one of the two alternatives (Kconfig switch, common clock used by default)

---
 arch/powerpc/platforms/512x/Kconfig           |   14 +-
 arch/powerpc/platforms/512x/Makefile          |    4 +-
 arch/powerpc/platforms/512x/clock-commonclk.c |  777 +++++++++++++++++++++++++
 include/linux/clk-provider.h                  |   16 +
 4 files changed, 809 insertions(+), 2 deletions(-)
 create mode 100644 arch/powerpc/platforms/512x/clock-commonclk.c

index fc9c1cb..c5fcdd0 100644
@@ -1,9 +1,21 @@
+config MPC512x_COMMON_CLK
+	bool "MPC512x platform uses COMMON_CLK"
+	default y
+	depends on PPC_MPC512x
+	help
+	  This option is only here to support tests and comparison
+	  during development and migration.  This option will get
+	  removed after the COMMON_CLK support for MPC512x has become
+	  fully operational and all drivers were adjusted to explicitly
+	  acquire their required clocks.
+
 config PPC_MPC512x
 	bool "512x-based boards"
 	depends on 6xx
 	select FSL_SOC
 	select IPIC
-	select PPC_CLOCK
+	select PPC_CLOCK if !MPC512x_COMMON_CLK
+	select COMMON_CLK if MPC512x_COMMON_CLK
 	select PPC_PCI_CHOICE
 	select FSL_PCI if PCI
 	select ARCH_WANT_OPTIONAL_GPIOLIB

index 72fb934..1e05f9d 100644
@@ -1,7 +1,9 @@
 #
 # Makefile for the Freescale PowerPC 512x linux kernel.
 #
-obj-y				+= clock.o mpc512x_shared.o
+obj-$(CONFIG_PPC_CLOCK)		+= clock.o
+obj-$(CONFIG_COMMON_CLK)	+= clock-commonclk.o
+obj-y				+= mpc512x_shared.o
 obj-$(CONFIG_MPC5121_ADS)	+= mpc5121_ads.o mpc5121_ads_cpld.o
 obj-$(CONFIG_MPC512x_GENERIC)	+= mpc512x_generic.o
 obj-$(CONFIG_PDM360NG)		+= pdm360ng.o

index c4f7799..7f8fc64 100644
@@ -497,6 +497,20 @@ static inline const char *of_clk_get_parent_name(struct device_node *np,
  * for improved portability across platforms
  */
 
+#if IS_ENABLED(CONFIG_PPC)
+
+static inline u32 clk_readl(u32 __iomem *reg)
+{
+	return ioread32be(reg);
+}
+
+static inline void clk_writel(u32 val, u32 __iomem *reg)
+{
+	iowrite32be(val, reg);
+}
+
+#else	/* platform dependent I/O accessors */
+
 static inline u32 clk_readl(u32 __iomem *reg)
 {
 	return readl(reg);

@@ -507,5 +521,7 @@ static inline void clk_writel(u32 val, u32 __iomem *reg)
 	writel(val, reg);
 }
 
+#endif	/* platform dependent I/O accessors */
+
 #endif /* CONFIG_COMMON_CLK */
 #endif /* CLK_PROVIDER_H */

1.7.10.4

this addresses the client side of device tree based clock lookups:
add clock specifiers to the mbx, nfc, mscan, sdhc, i2c, axe, diu, viu, mdio, fec, usb, pata, psc, psc fifo, and pci nodes in the shared mpc5121.dtsi include

these specs map 'clock-names' encoded in drivers to their respective 'struct clk' items in the platform's clock driver; some drivers still need to learn how to lookup OF clocks, other drivers will transparently probe successfully on MPC512x as well (sdhc)

few 'clock-names' (nfc, viu) use strings that are encoded in their respective drivers to keep up compatibility, an alternative approach would be to register clkdev items in the platform's clock driver for the migration period

---
 arch/powerpc/boot/dts/mpc5121.dtsi |   79 ++++++++++++++++++++++++++++++++++++
 1 file changed, 79 insertions(+)

index 8f4cba0..983da9c 100644
@@ -51,6 +51,10 @@
 		compatible = "fsl,mpc5121-mbx";
 		reg = <0x20000000 0x4000>;
 		interrupts = <66 0x8>;
+		clocks = <&clks MPC512x_CLK_MBX_BUS>,
+			 <&clks MPC512x_CLK_MBX_3D>,
+			 <&clks MPC512x_CLK_MBX>;
+		clock-names = "mbx-bus", "mbx-3d", "mbx";
 	};
 
 	sram at 30000000 {

@@ -64,6 +68,8 @@
 		interrupts = <6 8>;
 		#address-cells = <1>;
 		#size-cells = <1>;
+		clocks = <&clks MPC512x_CLK_NFC>;
+		clock-names = "nfc_clk";	/* backwards compat */
 	};
 
 	localbus at 80000020 {

@@ -153,12 +159,22 @@
 			compatible = "fsl,mpc5121-mscan";
 			reg = <0x1300 0x80>;
 			interrupts = <12 0x8>;
+			clocks = <&clks MPC512x_CLK_IPS>,
+				 <&clks MPC512x_CLK_SYS>,
+				 <&clks MPC512x_CLK_REF>,
+				 <&clks MPC512x_CLK_MSCAN0_MCLK>;
+			clock-names = "ips", "sys", "ref", "mclk";
 		};
 
 		can at 1380 {
 			compatible = "fsl,mpc5121-mscan";
 			reg = <0x1380 0x80>;
 			interrupts = <13 0x8>;
+			clocks = <&clks MPC512x_CLK_IPS>,
+				 <&clks MPC512x_CLK_SYS>,
+				 <&clks MPC512x_CLK_REF>,
+				 <&clks MPC512x_CLK_MSCAN1_MCLK>;
+			clock-names = "ips", "sys", "ref", "mclk";
 		};
 
 		sdhc at 1500 {

@@ -167,6 +183,9 @@
 			interrupts = <8 0x8>;
 			dmas = <&dma0 30>;
 			dma-names = "rx-tx";
+			clocks = <&clks MPC512x_CLK_IPS>,
+				 <&clks MPC512x_CLK_SDHC>;
+			clock-names = "ipg", "per";
 		};
 
 		i2c at 1700 {

@@ -175,6 +194,8 @@
 			compatible = "fsl,mpc5121-i2c", "fsl-i2c";
 			reg = <0x1700 0x20>;
 			interrupts = <9 0x8>;
+			clocks = <&clks MPC512x_CLK_I2C>;
+			clock-names = "per";
 		};
 
 		i2c at 1720 {

@@ -183,6 +204,8 @@
 			compatible = "fsl,mpc5121-i2c", "fsl-i2c";
 			reg = <0x1720 0x20>;
 			interrupts = <10 0x8>;
+			clocks = <&clks MPC512x_CLK_I2C>;
+			clock-names = "per";
 		};
 
 		i2c at 1740 {

@@ -191,6 +214,8 @@
 			compatible = "fsl,mpc5121-i2c", "fsl-i2c";
 			reg = <0x1740 0x20>;
 			interrupts = <11 0x8>;
+			clocks = <&clks MPC512x_CLK_I2C>;
+			clock-names = "per";
 		};
 
 		i2ccontrol at 1760 {

@@ -202,30 +227,46 @@
 			compatible = "fsl,mpc5121-axe";
 			reg = <0x2000 0x100>;
 			interrupts = <42 0x8>;
+			clocks = <&clks MPC512x_CLK_AXE>;
+			clock-names = "per";
 		};
 
 		display at 2100 {
 			compatible = "fsl,mpc5121-diu";
 			reg = <0x2100 0x100>;
 			interrupts = <64 0x8>;
+			clocks = <&clks MPC512x_CLK_DIU>;
+			clock-names = "per";
 		};
 
 		can at 2300 {
 			compatible = "fsl,mpc5121-mscan";
 			reg = <0x2300 0x80>;
 			interrupts = <90 0x8>;
+			clocks = <&clks MPC512x_CLK_IPS>,
+				 <&clks MPC512x_CLK_SYS>,
+				 <&clks MPC512x_CLK_REF>,
+				 <&clks MPC512x_CLK_MSCAN2_MCLK>;
+			clock-names = "ips", "sys", "ref", "mclk";
 		};
 
 		can at 2380 {
 			compatible = "fsl,mpc5121-mscan";
 			reg = <0x2380 0x80>;
 			interrupts = <91 0x8>;
+			clocks = <&clks MPC512x_CLK_IPS>,
+				 <&clks MPC512x_CLK_SYS>,
+				 <&clks MPC512x_CLK_REF>,
+				 <&clks MPC512x_CLK_MSCAN3_MCLK>;
+			clock-names = "ips", "sys", "ref", "mclk";
 		};
 
 		viu at 2400 {
 			compatible = "fsl,mpc5121-viu";
 			reg = <0x2400 0x400>;
 			interrupts = <67 0x8>;
+			clocks = <&clks MPC512x_CLK_VIU>;
+			clock-names = "viu_clk";	/* backwards compat */
 		};
 
 		mdio at 2800 {

@@ -233,6 +274,8 @@
 			reg = <0x2800 0x800>;
 			#address-cells = <1>;
 			#size-cells = <0>;
+			clocks = <&clks MPC512x_CLK_FEC>;
+			clock-names = "per";
 		};
 
 		eth0: ethernet at 2800 {

@@ -241,6 +284,8 @@
 			reg = <0x2800 0x800>;
 			local-mac-address = [ 00 00 00 00 00 00 ];
 			interrupts = <4 0x8>;
+			clocks = <&clks MPC512x_CLK_FEC>;
+			clock-names = "per";
 		};
 
 		/* USB1 using external ULPI PHY */

@@ -252,6 +297,8 @@
 			interrupts = <43 0x8>;
 			dr_mode = "otg";
 			phy_type = "ulpi";
+			clocks = <&clks MPC512x_CLK_USB1>;
+			clock-names = "per";
 		};
 
 		/* USB0 using internal UTMI PHY */

@@ -263,6 +310,8 @@
 			interrupts = <44 0x8>;
 			dr_mode = "otg";
 			phy_type = "utmi_wide";
+			clocks = <&clks MPC512x_CLK_USB2>;
+			clock-names = "per";
 		};
 
 		/* IO control */

@@ -281,6 +330,8 @@
 			compatible = "fsl,mpc5121-pata";
 			reg = <0x10200 0x100>;
 			interrupts = <5 0x8>;
+			clocks = <&clks MPC512x_CLK_PATA>;
+			clock-names = "per";
 		};
 
 		/* 512x PSCs are not 52xx PSC compatible */

@@ -292,6 +343,8 @@
 			interrupts = <40 0x8>;
 			fsl,rx-fifo-size = <16>;
 			fsl,tx-fifo-size = <16>;
+			clocks = <&clks MPC512x_CLK_PSC0_MCLK>;
+			clock-names = "mclk";
 		};
 
 		/* PSC1 */

@@ -301,6 +354,8 @@
 			interrupts = <40 0x8>;
 			fsl,rx-fifo-size = <16>;
 			fsl,tx-fifo-size = <16>;
+			clocks = <&clks MPC512x_CLK_PSC1_MCLK>;
+			clock-names = "mclk";
 		};
 
 		/* PSC2 */

@@ -310,6 +365,8 @@
 			interrupts = <40 0x8>;
 			fsl,rx-fifo-size = <16>;
 			fsl,tx-fifo-size = <16>;
+			clocks = <&clks MPC512x_CLK_PSC2_MCLK>;
+			clock-names = "mclk";
 		};
 
 		/* PSC3 */

@@ -319,6 +376,8 @@
 			interrupts = <40 0x8>;
 			fsl,rx-fifo-size = <16>;
 			fsl,tx-fifo-size = <16>;
+			clocks = <&clks MPC512x_CLK_PSC3_MCLK>;
+			clock-names = "mclk";
 		};
 
 		/* PSC4 */

@@ -328,6 +387,8 @@
 			interrupts = <40 0x8>;
 			fsl,rx-fifo-size = <16>;
 			fsl,tx-fifo-size = <16>;
+			clocks = <&clks MPC512x_CLK_PSC4_MCLK>;
+			clock-names = "mclk";
 		};
 
 		/* PSC5 */

@@ -337,6 +398,8 @@
 			interrupts = <40 0x8>;
 			fsl,rx-fifo-size = <16>;
 			fsl,tx-fifo-size = <16>;
+			clocks = <&clks MPC512x_CLK_PSC5_MCLK>;
+			clock-names = "mclk";
 		};
 
 		/* PSC6 */

@@ -346,6 +409,8 @@
 			interrupts = <40 0x8>;
 			fsl,rx-fifo-size = <16>;
 			fsl,tx-fifo-size = <16>;
+			clocks = <&clks MPC512x_CLK_PSC6_MCLK>;
+			clock-names = "mclk";
 		};
 
 		/* PSC7 */

@@ -355,6 +420,8 @@
 			interrupts = <40 0x8>;
 			fsl,rx-fifo-size = <16>;
 			fsl,tx-fifo-size = <16>;
+			clocks = <&clks MPC512x_CLK_PSC7_MCLK>;
+			clock-names = "mclk";
 		};
 
 		/* PSC8 */

@@ -364,6 +431,8 @@
 			interrupts = <40 0x8>;
 			fsl,rx-fifo-size = <16>;
 			fsl,tx-fifo-size = <16>;
+			clocks = <&clks MPC512x_CLK_PSC8_MCLK>;
+			clock-names = "mclk";
 		};
 
 		/* PSC9 */

@@ -373,6 +442,8 @@
 			interrupts = <40 0x8>;
 			fsl,rx-fifo-size = <16>;
 			fsl,tx-fifo-size = <16>;
+			clocks = <&clks MPC512x_CLK_PSC9_MCLK>;
+			clock-names = "mclk";
 		};
 
 		/* PSC10 */

@@ -382,6 +453,8 @@
 			interrupts = <40 0x8>;
 			fsl,rx-fifo-size = <16>;
 			fsl,tx-fifo-size = <16>;
+			clocks = <&clks MPC512x_CLK_PSC10_MCLK>;
+			clock-names = "mclk";
 		};
 
 		/* PSC11 */

@@ -391,12 +464,16 @@
 			interrupts = <40 0x8>;
 			fsl,rx-fifo-size = <16>;
 			fsl,tx-fifo-size = <16>;
+			clocks = <&clks MPC512x_CLK_PSC11_MCLK>;
+			clock-names = "mclk";
 		};
 
 		pscfifo at 11f00 {
 			compatible = "fsl,mpc5121-psc-fifo";
 			reg = <0x11f00 0x100>;
 			interrupts = <40 0x8>;
+			clocks = <&clks MPC512x_CLK_PSC_FIFO>;
+			clock-names = "per";
 		};
 
 		dma0: dma at 14000 {

@@ -414,6 +491,8 @@
 		#address-cells = <3>;
 		#size-cells = <2>;
 		#interrupt-cells = <1>;
+		clocks = <&clks MPC512x_CLK_PCI>;
+		clock-names = "per";
 
 		reg = <0x80008500 0x100	/* internal registers */
 		       0x80008300 0x8>;	/* config space access registers */

1.7.10.4

add a comment about the magic of deriving an MSCAN component index from the peripheral's physical address / register offset

---
 drivers/net/can/mscan/mpc5xxx_can.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

index 5b0ee8e..bc422ba 100644
@@ -148,7 +148,10 @@ static u32 mpc512x_can_get_clock(struct platform_device *ofdev,
 		goto exit_put;
 	}
 
-	/* Determine the MSCAN device index from the physical address */
+	/* Determine the MSCAN device index from the peripheral's
+	 * physical address. Register address offsets against the
+	 * IMMR base are:  0x1300, 0x1380, 0x2300, 0x2380
+	 */
 	pval = of_get_property(ofdev->dev.of_node, "reg", &plen);
 	BUG_ON(!pval || plen < sizeof(*pval));
 	clockidx = (*pval & 0x80) ? 1 : 0;

1.7.10.4

extend the mscan(4) driver with alternative support for the COMMON_CLK approach which is an option in the MPC512x platform, keep the existing clock support implementation in place since the driver is shared with other MPC5xxx SoCs which don't have common clock support

one byproduct of this change is that the CAN driver no longer needs to access the SoC's clock control registers, which shall be the domain of the platform's clock driver

---
 drivers/net/can/mscan/mpc5xxx_can.c |  139 +++++++++++++++++++++++++++++++++++
 1 file changed, 139 insertions(+)

index bc422ba..dd26ab6 100644
@@ -108,6 +108,143 @@ static u32 mpc52xx_can_get_clock(struct platform_device *ofdev,
 #endif /* CONFIG_PPC_MPC52xx */
 
 #ifdef CONFIG_PPC_MPC512x
+
+#if IS_ENABLED(CONFIG_COMMON_CLK)
+
+static u32 mpc512x_can_get_clock(struct platform_device *ofdev,
+				 const char *clock_source, int *mscan_clksrc)
+{
+	struct device_node *np;
+	u32 clockdiv;
+	enum {
+		CLK_FROM_AUTO,
+		CLK_FROM_IPS,
+		CLK_FROM_SYS,
+		CLK_FROM_REF,
+	} clk_from;
+	struct clk *clk_in, *clk_can;
+	unsigned long freq_calc;
+
+	/* the caller passed in the clock source spec that was read from
+	 * the device tree, get the optional clock divider as well
+	 */
+	np = ofdev->dev.of_node;
+	clockdiv = 1;
+	of_property_read_u32(np, "fsl,mscan-clock-divider", &clockdiv);
+	dev_dbg(&ofdev->dev, "device tree specs: clk src[%s] div[%d]\n",
+		clock_source ? clock_source : "<NULL>", clockdiv);
+
+	/* when clock-source is 'ip', the CANCTL1[CLKSRC] bit needs to
+	 * get set, and the 'ips' clock is the input to the MSCAN
+	 * component
+	 *
+	 * for clock-source values of 'ref' or 'sys' the CANCTL1[CLKSRC]
+	 * bit needs to get cleared, an optional clock-divider may have
+	 * been specified (the default value is 1), the appropriate
+	 * MSCAN related MCLK is the input to the MSCAN component
+	 *
+	 * in the absence of a clock-source spec, first an optimal clock
+	 * gets determined based on the 'sys' clock, if that fails the
+	 * 'ref' clock is used
+	 */
+	clk_from = CLK_FROM_AUTO;
+	if (clock_source) {
+		/* interpret the device tree's spec for the clock source */
+		if (!strcmp(clock_source, "ip"))
+			clk_from = CLK_FROM_IPS;
+		else if (!strcmp(clock_source, "sys"))
+			clk_from = CLK_FROM_SYS;
+		else if (!strcmp(clock_source, "ref"))
+			clk_from = CLK_FROM_REF;
+		else
+			goto err_invalid;
+		dev_dbg(&ofdev->dev, "got a clk source spec[%d]\n", clk_from);
+	}
+	if (clk_from == CLK_FROM_AUTO) {
+		/* no spec so far, try the 'sys' clock; round to the
+		 * next MHz and see if we can get a multiple of 16MHz
+		 */
+		dev_dbg(&ofdev->dev, "no clk source spec, trying SYS\n");
+		clk_in = clk_get(&ofdev->dev, "sys");
+		if (IS_ERR(clk_in))
+			goto err_notavail;
+		freq_calc = clk_get_rate(clk_in);
+		freq_calc +=  499999;
+		freq_calc /= 1000000;
+		freq_calc *= 1000000;
+		if ((freq_calc % 16000000) == 0) {
+			clk_from = CLK_FROM_SYS;
+			clockdiv = freq_calc / 16000000;
+			dev_dbg(&ofdev->dev,
+				"clk fit, sys[%lu] div[%d] freq[%lu]\n",
+				freq_calc, clockdiv, freq_calc / clockdiv);
+		}
+	}
+	if (clk_from == CLK_FROM_AUTO) {
+		/* no spec so far, use the 'ref' clock */
+		dev_dbg(&ofdev->dev, "no clk source spec, trying REF\n");
+		clk_in = clk_get(&ofdev->dev, "ref");
+		if (IS_ERR(clk_in))
+			goto err_notavail;
+		clk_from = CLK_FROM_REF;
+		freq_calc = clk_get_rate(clk_in);
+		dev_dbg(&ofdev->dev,
+			"clk fit, ref[%lu] (no div) freq[%lu]\n",
+			freq_calc, freq_calc);
+	}
+
+	/* select IPS or MCLK as the MSCAN input (returned to the caller),
+	 * setup the MCLK mux source and rate if applicable, apply the
+	 * optionally specified or derived above divider, and determine
+	 * the actual resulting clock rate to return to the caller
+	 */
+	switch (clk_from) {
+	case CLK_FROM_IPS:
+		clk_can = clk_get(&ofdev->dev, "ips");
+		if (IS_ERR(clk_can))
+			goto err_notavail;
+		clk_prepare_enable(clk_can);
+		freq_calc = clk_get_rate(clk_can);
+		*mscan_clksrc = MSCAN_CLKSRC_IPS;
+		dev_dbg(&ofdev->dev, "clk from IPS, clksrc[%d] freq[%lu]\n",
+			*mscan_clksrc, freq_calc);
+		break;
+	case CLK_FROM_SYS:
+	case CLK_FROM_REF:
+		clk_can = clk_get(&ofdev->dev, "mclk");
+		if (IS_ERR(clk_can))
+			goto err_notavail;
+		clk_prepare_enable(clk_can);
+		if (clk_from == CLK_FROM_SYS)
+			clk_in = clk_get(&ofdev->dev, "sys");
+		if (clk_from == CLK_FROM_REF)
+			clk_in = clk_get(&ofdev->dev, "ref");
+		if (IS_ERR(clk_in))
+			goto err_notavail;
+		clk_set_parent(clk_can, clk_in);
+		freq_calc = clk_get_rate(clk_in);
+		freq_calc /= clockdiv;
+		clk_set_rate(clk_can, freq_calc);
+		freq_calc = clk_get_rate(clk_can);
+		*mscan_clksrc = MSCAN_CLKSRC_BUS;
+		dev_dbg(&ofdev->dev, "clk from MCLK, clksrc[%d] freq[%lu]\n",
+			*mscan_clksrc, freq_calc);
+		break;
+	default:
+		goto err_invalid;
+	}
+
+	return freq_calc;
+
+err_invalid:
+	dev_err(&ofdev->dev, "invalid clock source specification\n");
+	return 0;
+
+err_notavail:
+	dev_err(&ofdev->dev, "cannot acquire or setup clock source\n");
+	return 0;
+}
+#else	/* COMMON_CLK */
 struct mpc512x_clockctl {
 	u32 spmr;		/* System PLL Mode Reg */
 	u32 sccr[2];		/* System Clk Ctrl Reg 1 & 2 */

@@ -238,6 +375,8 @@ exit_put:
 	of_node_put(np_clock);
 	return freq;
 }
+#endif	/* COMMON_CLK */
+
 #else /* !CONFIG_PPC_MPC512x */
 static u32 mpc512x_can_get_clock(struct platform_device *ofdev,
 				 const char *clock_name, int *mscan_clksrc)

1.7.10.4

after PSC related clock specifiers were added to the device tree, the former 'psc%d_mclk' isn't needed any longer to lookup clock items

---
 drivers/spi/spi-mpc512x-psc.c |    4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

index 76b20ea..3e30e52 100644
@@ -388,13 +388,11 @@ static int mpc512x_psc_spi_port_config(struct spi_master *master,
 	struct mpc512x_psc_fifo __iomem *fifo = mps->fifo;
 	struct clk *spiclk;
 	int ret = 0;
-	char name[32];
 	u32 sicr;
 	u32 ccr;
 	u16 bclkdiv;
 
-	sprintf(name, "psc%d_mclk", master->bus_num);
-	spiclk = clk_get(&master->dev, name);
+	spiclk = clk_get(&master->dev, "mclk");
 	clk_prepare_enable(spiclk);
 	mps->mclk = clk_get_rate(spiclk);
 	clk_put(spiclk);

1.7.10.4

adapt the DIU clock initialization to the COMMON_CLK approach: device tree based clock lookup, prepare and unprepare for clocks, work with frequencies not dividers, call the appropriate clk_*() routines and don't access CCM registers, remove the pre-enable workaround in the platform's clock driver

the "best clock" determination now completely relies on the platform's clock driver to pick a frequency close to what the caller requests, and merely checks whether the desired frequency was met (is acceptable since it meets the tolerance of the monitor) -- this approach shall succeed upon first try in the usual case, will test a few less desirable yet acceptable frequencies in edge cases, and will fallback to "best effort" if none of the previously tried frequencies pass the test

---
 arch/powerpc/platforms/512x/clock-commonclk.c |    1 -
 arch/powerpc/platforms/512x/mpc512x_shared.c  |  165 +++++++++++++------------
 2 files changed, 88 insertions(+), 78 deletions(-)

index 218f55b..f31f895 100644
@@ -705,7 +705,6 @@ static void mpc512x_clk_setup_clock_tree(int busfreq)
 	/* some are not yet acquired by their respective drivers */
 	clk_prepare_enable(clks[MPC512x_CLK_PSC3_MCLK]);/* serial console */
 	clk_prepare_enable(clks[MPC512x_CLK_FEC]);	/* network, NFS */
-	clk_prepare_enable(clks[MPC512x_CLK_DIU]);	/* display */
 	clk_prepare_enable(clks[MPC512x_CLK_I2C]);
 	/*
 	 * some have their individual clock subtree with separate clock

1.7.10.4

with device tree based clock lookup, the MCLK name no longer depends on the PSC index

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

index 53c1093..221fb89 100644
@@ -619,21 +619,17 @@ static irqreturn_t mpc512x_psc_handle_irq(struct uart_port *port)
 static int mpc512x_psc_clock(struct uart_port *port, int enable)
 {
 	struct clk *psc_clk;
-	int psc_num;
-	char clk_name[10];
 
 	if (uart_console(port))
 		return 0;
 
-	psc_num = (port->mapbase & 0xf00) >> 8;
-	snprintf(clk_name, sizeof(clk_name), "psc%d_mclk", psc_num);
-	psc_clk = clk_get(port->dev, clk_name);
+	psc_clk = clk_get(port->dev, "mclk");
 	if (IS_ERR(psc_clk)) {
 		dev_err(port->dev, "Failed to get PSC clock entry!\n");
 		return -ENODEV;
 	}
 
-	dev_dbg(port->dev, "%s %sable\n", clk_name, enable ? "en" : "dis");
+	dev_dbg(port->dev, "mclk %sable\n", enable ? "en" : "dis");
 
 	if (enable)
 		clk_prepare_enable(psc_clk);

1.7.10.4

So I'd like to respond in the same way as I did for the SPI driver. :)

The scope of this series is the introduction of support for the common clock framework. Addressing other (legacy non-fatal and previously accepted) issues as they get identified in bypassing would be the scope of a separate patch or series.

I'm not questioning the need to fix other additionally identified issues. I'm just asking whether they shall be in the scope of this very 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 at denx.de

after the PSC drivers for UART and SPI mode got converted to device tree based clock lookup, the former MCLK name which depends on the PSC index isn't needed any longer -- remove the clk_register_clkdev() invocation

---
 arch/powerpc/platforms/512x/clock-commonclk.c |   10 ----------
 1 file changed, 10 deletions(-)

index f31f895..31707b5 100644
@@ -529,16 +529,6 @@ static void mpc512x_clk_setup_mclk(struct mclk_setup_data *entry)
 				entry->name_mclk,
 				entry->name_mux1, 1, 1);
 	}
-
-	/*
-	 * without this "clock device" registration, "simple" lookups in
-	 * the SPI master initialization and serial port setup will fail
-	 *
-	 * those drivers need to get adjusted to lookup their required
-	 * clocks from OF specs, before this clkdev registration becomes
-	 * obsolete
-	 */
-	clk_register_clkdev(clks[clks_idx_pub], entry->name_mclk, NULL);
 }
 
 static void mpc512x_clk_setup_mclks(struct mclk_setup_data *table, size_t count)

1.7.10.4

prepare and enable the FIFO clock upon PSC FIFO initialization, disable and unprepare the FIFO clock upon PSC FIFO uninitialization, remove the pre-enable workaround from the platform's clock driver

---
 arch/powerpc/platforms/512x/clock-commonclk.c |    2 --
 drivers/tty/serial/mpc52xx_uart.c             |   47 +++++++++++++++++++++----
 2 files changed, 41 insertions(+), 8 deletions(-)

index 31707b5..2c6da07 100644
@@ -690,8 +690,6 @@ static void mpc512x_clk_setup_clock_tree(int busfreq)
 	clk_prepare_enable(clks[MPC512x_CLK_MEM]);	/* SRAM */
 	clk_prepare_enable(clks[MPC512x_CLK_IPS]);	/* SoC periph */
 	clk_prepare_enable(clks[MPC512x_CLK_LPC]);	/* boot media */
-	/* some are required yet no dependencies were declared */
-	clk_prepare_enable(clks[MPC512x_CLK_PSC_FIFO]);
 	/* some are not yet acquired by their respective drivers */
 	clk_prepare_enable(clks[MPC512x_CLK_PSC3_MCLK]);/* serial console */
 	clk_prepare_enable(clks[MPC512x_CLK_FEC]);	/* network, NFS */

1.7.10.4

make the MPC I2C driver prepare and enable the peripheral clock ('per' for register access) in the MPC512x setup routine, make this clock setup non-fatal to allow for a migration period, remove the pre-enabling hack in the platform's clock driver

---
 arch/powerpc/platforms/512x/clock-commonclk.c |    1 -
 drivers/i2c/busses/i2c-mpc.c                  |    9 +++++++++
 2 files changed, 9 insertions(+), 1 deletion(-)

index 2c6da07..29c51a5 100644
@@ -693,7 +693,6 @@ static void mpc512x_clk_setup_clock_tree(int busfreq)
 	/* some are not yet acquired by their respective drivers */
 	clk_prepare_enable(clks[MPC512x_CLK_PSC3_MCLK]);/* serial console */
 	clk_prepare_enable(clks[MPC512x_CLK_FEC]);	/* network, NFS */
-	clk_prepare_enable(clks[MPC512x_CLK_I2C]);
 	/*
 	 * some have their individual clock subtree with separate clock
 	 * items and their individual enable counters, yet share a

index 7607dc0..13d6822 100644
@@ -21,6 +21,7 @@
 #include <linux/of_i2c.h>
 #include <linux/slab.h>
 
+#include <linux/clk.h>
 #include <linux/io.h>
 #include <linux/fsl_devices.h>
 #include <linux/i2c.h>

@@ -264,11 +265,19 @@ static void mpc_i2c_setup_512x(struct device_node *node,
 					 struct mpc_i2c *i2c,
 					 u32 clock, u32 prescaler)
 {
+	struct clk *clk;
 	struct device_node *node_ctrl;
 	void __iomem *ctrl;
 	const u32 *pval;
 	u32 idx;
 
+	/* enable clock for the I2C peripheral (non fatal) */
+	clk = of_clk_get_by_name(node, "per");
+	if (!IS_ERR(clk)) {
+		clk_prepare_enable(clk);
+		clk_put(clk);
+	}
+
 	/* Enable I2C interrupts for mpc5121 */
 	node_ctrl = of_find_compatible_node(NULL, NULL,
 					    "fsl,mpc5121-i2c-ctrl");

1.7.10.4

device tree based clock lookup in the MPC512x initialization (lookup 'per' for register access), add error check in the clock setup, must prepare clocks before they can get enabled, unprepare after disable

---
 drivers/usb/host/fsl-mph-dr-of.c |   24 +++++++++---------------
 1 file changed, 9 insertions(+), 15 deletions(-)

index 11e0b79..eb564f3 100644
@@ -260,25 +260,19 @@ int fsl_usb2_mpc5121_init(struct platform_device *pdev)
 {
 	struct fsl_usb2_platform_data *pdata = pdev->dev.platform_data;
 	struct clk *clk;
-	char clk_name[10];
-	int base, clk_num;
-
-	base = pdev->resource->start & 0xf000;
-	if (base == 0x3000)
-		clk_num = 1;
-	else if (base == 0x4000)
-		clk_num = 2;
-	else
-		return -ENODEV;
+	int err;
 
-	snprintf(clk_name, sizeof(clk_name), "usb%d_clk", clk_num);
-	clk = clk_get(&pdev->dev, clk_name);
+	clk = clk_get(&pdev->dev, "per");
 	if (IS_ERR(clk)) {
 		dev_err(&pdev->dev, "failed to get clk\n");
 		return PTR_ERR(clk);
 	}
-
-	clk_enable(clk);
+	err = clk_prepare_enable(clk);
+	if (err) {
+		dev_err(&pdev->dev, "failed to enable clk\n");
+		clk_put(clk);
+		return err;
+	}
 	pdata->clk = clk;
 
 	if (pdata->phy_mode == FSL_USB2_PHY_UTMI_WIDE) {

@@ -303,7 +297,7 @@ static void fsl_usb2_mpc5121_exit(struct platform_device *pdev)
 	pdata->regs = NULL;
 
 	if (pdata->clk) {
-		clk_disable(pdata->clk);
+		clk_disable_unprepare(pdata->clk);
 		clk_put(pdata->clk);
 	}
 }

1.7.10.4

device tree based clock lookup, must prepare clocks before enabling them, unprepare after disable, error check in the clock setup, remove the pre-enable workaround in the MPC512x platform's clock driver

this change implements non-fatal clock lookup since not all platforms provide device tree specs for clocks, but failure to enable a specified clock is considered fatal

---
 arch/powerpc/platforms/512x/clock-commonclk.c      |    1 -
 .../net/ethernet/freescale/fs_enet/fs_enet-main.c  |   27 +++++++++++++++++++-
 include/linux/fs_enet_pd.h                         |    3 +++
 3 files changed, 29 insertions(+), 2 deletions(-)

index 29c51a5..31d0cf3 100644
@@ -692,7 +692,6 @@ static void mpc512x_clk_setup_clock_tree(int busfreq)
 	clk_prepare_enable(clks[MPC512x_CLK_LPC]);	/* boot media */
 	/* some are not yet acquired by their respective drivers */
 	clk_prepare_enable(clks[MPC512x_CLK_PSC3_MCLK]);/* serial console */
-	clk_prepare_enable(clks[MPC512x_CLK_FEC]);	/* network, NFS */
 	/*
 	 * some have their individual clock subtree with separate clock
 	 * items and their individual enable counters, yet share a

index 8de53a1..df92e12 100644
@@ -1020,6 +1020,21 @@ static int fs_enet_probe(struct platform_device *ofdev)
 		fpi->cp_command = *data;
 	}
 
+	/* make clock lookup non-fatal (the driver is shared among platforms),
+	 * but require enable to succeed when a clock was specified/found
+	 */
+	fpi->clk_per = clk_get(&ofdev->dev, "per");
+	if (IS_ERR(fpi->clk_per))
+		fpi->clk_per = NULL;
+	if (fpi->clk_per) {
+		int err;
+		err = clk_prepare_enable(fpi->clk_per);
+		if (err) {
+			ret = err;
+			goto out_clk_put;
+		}
+	}
+
 	fpi->rx_ring = 32;
 	fpi->tx_ring = 32;
 	fpi->rx_copybreak = 240;

@@ -1028,7 +1043,7 @@ static int fs_enet_probe(struct platform_device *ofdev)
 	fpi->phy_node = of_parse_phandle(ofdev->dev.of_node, "phy-handle", 0);
 	if ((!fpi->phy_node) && (!of_get_property(ofdev->dev.of_node, "fixed-link",
 						  NULL)))
-		goto out_free_fpi;
+		goto out_clk_dis;
 
 	if (of_device_is_compatible(ofdev->dev.of_node, "fsl,mpc5125-fec")) {
 		phy_connection_type = of_get_property(ofdev->dev.of_node,

@@ -1108,6 +1123,12 @@ out_free_dev:
 	free_netdev(ndev);
 out_put:
 	of_node_put(fpi->phy_node);
+out_clk_dis:
+	if (fpi->clk_per)
+		clk_disable_unprepare(fpi->clk_per);
+out_clk_put:
+	if (fpi->clk_per)
+		clk_put(fpi->clk_per);
 out_free_fpi:
 	kfree(fpi);
 	return ret;

@@ -1124,6 +1145,10 @@ static int fs_enet_remove(struct platform_device *ofdev)
 	fep->ops->cleanup_data(ndev);
 	dev_set_drvdata(fep->dev, NULL);
 	of_node_put(fep->fpi->phy_node);
+	if (fep->fpi->clk_per) {
+		clk_disable_unprepare(fep->fpi->clk_per);
+		clk_put(fep->fpi->clk_per);
+	}
 	free_netdev(ndev);
 	return 0;
 }

index 51b7934..a978d0d 100644
@@ -16,6 +16,7 @@
 #ifndef FS_ENET_PD_H
 #define FS_ENET_PD_H
 
+#include <linux/clk.h>
 #include <linux/string.h>
 #include <linux/of_mdio.h>
 #include <asm/types.h>

@@ -142,6 +143,8 @@ struct fs_platform_info {
 
 	int use_rmii;		/* use RMII mode 	       */
 	int has_phy;            /* if the network is phy container as well...*/
+
+	struct clk *clk_per;	/* 'per' clock for register access */
 };
 struct fs_mii_fec_platform_info {
 	u32 irq[32];

1.7.10.4

device tree based clock lookup, must prepare clocks before enabling them, unprepare after disable, error check in the clock setup

---
 drivers/media/platform/fsl-viu.c |   12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

index 221ec42..b95aa43 100644
@@ -1582,8 +1582,11 @@ static int viu_of_probe(struct platform_device *op)
 		dev_err(&op->dev, "failed to find the clock module!\n");
 		ret = -ENODEV;
 		goto err_clk;
-	} else {
-		clk_enable(viu_dev->clk);
+	}
+	ret = clk_prepare_enable(viu_dev->clk);
+	if (ret) {
+		dev_err(&op->dev, "failed to enable the clock!\n");
+		goto err_clk_put;
 	}
 
 	/* reset VIU module */

@@ -1602,7 +1605,8 @@ static int viu_of_probe(struct platform_device *op)
 	return ret;
 
 err_irq:
-	clk_disable(viu_dev->clk);
+	clk_disable_unprepare(viu_dev->clk);
+err_clk_put:
 	clk_put(viu_dev->clk);
 err_clk:
 	video_unregister_device(viu_dev->vdev);

@@ -1626,7 +1630,7 @@ static int viu_of_remove(struct platform_device *op)
 	free_irq(dev->irq, (void *)dev);
 	irq_dispose_mapping(dev->irq);
 
-	clk_disable(dev->clk);
+	clk_disable_unprepare(dev->clk);
 	clk_put(dev->clk);
 
 	video_unregister_device(dev->vdev);

1.7.10.4

device tree based clock lookup, must prepare clocks before enabling them, error check in the clock setup

this change implements non-fatal clock lookup for compatibility with platforms that don't provide OF clock specs, but failure to enable a specified clock is considered fatal

---
 arch/powerpc/sysdev/fsl_pci.c |   15 +++++++++++++++
 1 file changed, 15 insertions(+)

index 46ac1dd..cb2ed92 100644
@@ -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>

@@ -741,6 +743,7 @@ err0:
 
 int __init mpc83xx_add_bridge(struct device_node *dev)
 {
+	struct clk *clk;
 	int ret;
 	int len;
 	struct pci_controller *hose;

@@ -758,6 +761,18 @@ int __init mpc83xx_add_bridge(struct device_node *dev)
 	}
 	pr_debug("Adding PCI host bridge %s\n", dev->full_name);
 
+	/* non-fatal OF clock lookup, but fatal when a clock
+	 * was specified yet could not get enabled */
+	clk = of_clk_get_by_name(dev, "per");
+	if (!IS_ERR(clk)) {
+		ret = clk_prepare_enable(clk);
+		clk_put(clk);
+		if (ret) {
+			pr_err("Could not enable peripheral clock\n");
+			return ret;
+		}
+	}
+
 	/* Fetch host bridge registers address */
 	if (of_address_to_resource(dev, 0, &rsrc_reg)) {
 		printk(KERN_WARNING "Can't get pci register base!\n");

1.7.10.4

completely switch to, i.e. unconditionally use COMMON_CLK for the MPC512x platform, and retire the PPC_CLOCK implementation for that platform after the transition has completed

---
 arch/powerpc/platforms/512x/Kconfig  |   14 +-
 arch/powerpc/platforms/512x/Makefile |    3 +-
 arch/powerpc/platforms/512x/clock.c  |  753 ----------------------------------
 3 files changed, 2 insertions(+), 768 deletions(-)
 delete mode 100644 arch/powerpc/platforms/512x/clock.c

index c5fcdd0..5aa3f4b 100644
@@ -1,21 +1,9 @@
-config MPC512x_COMMON_CLK
-	bool "MPC512x platform uses COMMON_CLK"
-	default y
-	depends on PPC_MPC512x
-	help
-	  This option is only here to support tests and comparison
-	  during development and migration.  This option will get
-	  removed after the COMMON_CLK support for MPC512x has become
-	  fully operational and all drivers were adjusted to explicitly
-	  acquire their required clocks.
-
 config PPC_MPC512x
 	bool "512x-based boards"
 	depends on 6xx
+	select COMMON_CLK
 	select FSL_SOC
 	select IPIC
-	select PPC_CLOCK if !MPC512x_COMMON_CLK
-	select COMMON_CLK if MPC512x_COMMON_CLK
 	select PPC_PCI_CHOICE
 	select FSL_PCI if PCI
 	select ARCH_WANT_OPTIONAL_GPIOLIB

index 1e05f9d..bb20116 100644
@@ -1,8 +1,7 @@
 #
 # Makefile for the Freescale PowerPC 512x linux kernel.
 #
-obj-$(CONFIG_PPC_CLOCK)		+= clock.o
-obj-$(CONFIG_COMMON_CLK)	+= clock-commonclk.o
+obj-y				+= clock-commonclk.o
 obj-y				+= mpc512x_shared.o
 obj-$(CONFIG_MPC5121_ADS)	+= mpc5121_ads.o mpc5121_ads_cpld.o
 obj-$(CONFIG_MPC512x_GENERIC)	+= mpc512x_generic.o

1.7.10.4

transition to the COMMON_CLK framework has completed for the MPC512x platform, remove the now obsolete code path of the mpc5xxx mscan driver which accessed clock control module registers directly

---
 drivers/net/can/mscan/mpc5xxx_can.c |  136 -----------------------------------
 1 file changed, 136 deletions(-)

index dd26ab6..cf5e4cfc 100644
@@ -108,9 +108,6 @@ static u32 mpc52xx_can_get_clock(struct platform_device *ofdev,
 #endif /* CONFIG_PPC_MPC52xx */
 
 #ifdef CONFIG_PPC_MPC512x
-
-#if IS_ENABLED(CONFIG_COMMON_CLK)
-
 static u32 mpc512x_can_get_clock(struct platform_device *ofdev,
 				 const char *clock_source, int *mscan_clksrc)
 {

@@ -244,139 +241,6 @@ err_notavail:
 	dev_err(&ofdev->dev, "cannot acquire or setup clock source\n");
 	return 0;
 }
-#else	/* COMMON_CLK */
-struct mpc512x_clockctl {
-	u32 spmr;		/* System PLL Mode Reg */
-	u32 sccr[2];		/* System Clk Ctrl Reg 1 & 2 */
-	u32 scfr1;		/* System Clk Freq Reg 1 */
-	u32 scfr2;		/* System Clk Freq Reg 2 */
-	u32 reserved;
-	u32 bcr;		/* Bread Crumb Reg */
-	u32 pccr[12];		/* PSC Clk Ctrl Reg 0-11 */
-	u32 spccr;		/* SPDIF Clk Ctrl Reg */
-	u32 cccr;		/* CFM Clk Ctrl Reg */
-	u32 dccr;		/* DIU Clk Cnfg Reg */
-	u32 mccr[4];		/* MSCAN Clk Ctrl Reg 1-3 */
-};
-
-static struct of_device_id mpc512x_clock_ids[] = {
-	{ .compatible = "fsl,mpc5121-clock", },
-	{}
-};
-
-static u32 mpc512x_can_get_clock(struct platform_device *ofdev,
-				 const char *clock_name, int *mscan_clksrc)
-{
-	struct mpc512x_clockctl __iomem *clockctl;
-	struct device_node *np_clock;
-	struct clk *sys_clk, *ref_clk;
-	int plen, clockidx, clocksrc = -1;
-	u32 sys_freq, val, clockdiv = 1, freq = 0;
-	const u32 *pval;
-
-	np_clock = of_find_matching_node(NULL, mpc512x_clock_ids);
-	if (!np_clock) {
-		dev_err(&ofdev->dev, "couldn't find clock node\n");
-		return 0;
-	}
-	clockctl = of_iomap(np_clock, 0);
-	if (!clockctl) {
-		dev_err(&ofdev->dev, "couldn't map clock registers\n");
-		goto exit_put;
-	}
-
-	/* Determine the MSCAN device index from the peripheral's
-	 * physical address. Register address offsets against the
-	 * IMMR base are:  0x1300, 0x1380, 0x2300, 0x2380
-	 */
-	pval = of_get_property(ofdev->dev.of_node, "reg", &plen);
-	BUG_ON(!pval || plen < sizeof(*pval));
-	clockidx = (*pval & 0x80) ? 1 : 0;
-	if (*pval & 0x2000)
-		clockidx += 2;
-
-	/*
-	 * Clock source and divider selection: 3 different clock sources
-	 * can be selected: "ip", "ref" or "sys". For the latter two, a
-	 * clock divider can be defined as well. If the clock source is
-	 * not specified by the device tree, we first try to find an
-	 * optimal CAN source clock based on the system clock. If that
-	 * is not posslible, the reference clock will be used.
-	 */
-	if (clock_name && !strcmp(clock_name, "ip")) {
-		*mscan_clksrc = MSCAN_CLKSRC_IPS;
-		freq = mpc5xxx_get_bus_frequency(ofdev->dev.of_node);
-	} else {
-		*mscan_clksrc = MSCAN_CLKSRC_BUS;
-
-		pval = of_get_property(ofdev->dev.of_node,
-				       "fsl,mscan-clock-divider", &plen);
-		if (pval && plen == sizeof(*pval))
-			clockdiv = *pval;
-		if (!clockdiv)
-			clockdiv = 1;
-
-		if (!clock_name || !strcmp(clock_name, "sys")) {
-			sys_clk = clk_get(&ofdev->dev, "sys_clk");
-			if (IS_ERR(sys_clk)) {
-				dev_err(&ofdev->dev, "couldn't get sys_clk\n");
-				goto exit_unmap;
-			}
-			/* Get and round up/down sys clock rate */
-			sys_freq = 1000000 *
-				((clk_get_rate(sys_clk) + 499999) / 1000000);
-
-			if (!clock_name) {
-				/* A multiple of 16 MHz would be optimal */
-				if ((sys_freq % 16000000) == 0) {
-					clocksrc = 0;
-					clockdiv = sys_freq / 16000000;
-					freq = sys_freq / clockdiv;
-				}
-			} else {
-				clocksrc = 0;
-				freq = sys_freq / clockdiv;
-			}
-		}
-
-		if (clocksrc < 0) {
-			ref_clk = clk_get(&ofdev->dev, "ref_clk");
-			if (IS_ERR(ref_clk)) {
-				dev_err(&ofdev->dev, "couldn't get ref_clk\n");
-				goto exit_unmap;
-			}
-			clocksrc = 1;
-			freq = clk_get_rate(ref_clk) / clockdiv;
-		}
-	}
-
-	/* Disable clock */
-	out_be32(&clockctl->mccr[clockidx], 0x0);
-	if (clocksrc >= 0) {
-		/* Set source and divider */
-		val = (clocksrc << 14) | ((clockdiv - 1) << 17);
-		out_be32(&clockctl->mccr[clockidx], val);
-		/* Enable clock */
-		out_be32(&clockctl->mccr[clockidx], val | 0x10000);
-	}
-
-	/* Enable MSCAN clock domain */
-	val = in_be32(&clockctl->sccr[1]);
-	if (!(val & (1 << 25)))
-		out_be32(&clockctl->sccr[1], val | (1 << 25));
-
-	dev_dbg(&ofdev->dev, "using '%s' with frequency divider %d\n",
-		*mscan_clksrc == MSCAN_CLKSRC_IPS ? "ips_clk" :
-		clocksrc == 1 ? "ref_clk" : "sys_clk", clockdiv);
-
-exit_unmap:
-	iounmap(clockctl);
-exit_put:
-	of_node_put(np_clock);
-	return freq;
-}
-#endif	/* COMMON_CLK */
-
 #else /* !CONFIG_PPC_MPC512x */
 static u32 mpc512x_can_get_clock(struct platform_device *ofdev,
 				 const char *clock_name, int *mscan_clksrc)

1.7.10.4

this series introduces support for the common clock framework (CCF, COMMON_CLK Kconfig option) in the PowerPC based MPC512x platform

although the series does touch several subsystems -- serial, spi, net (can, fs_enet), mtd (nfc), usb, i2c, media (viu), and dts -- all of the patches are strictly clock related

it appears most appropriate to take this series through either the clk or the powerpc trees after it has passed review and other subsystem maintainers ACKed the clock setup related driver modifications

the series passes 'checkpatch.pl --strict' except for one warning which cannot get resolved, since that either breaks compilation (the data type is preset by the clk-provider.h API) or requires a cast which shadows real mismatches:

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

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

each step in the series was build and run tested (with a display that is attached to the DIU as well as SPI, with an SPI attached NOR flash, with multiple UART ports such that one is not the boot console, with EEPROMs attached to I2C, with an SD card, booting from network)

changes in v2:
- cleanup of the UART and SPI 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)
- 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
- keep a reference to the PSC FIFO clock during use instead of looking up the clock again in the uninit() routine
- remove the clkdev.h header file inclusion directive with the removal of the clkdev registration call

Gerhard Sittig (24):
spi: mpc512x: cleanup clock API use
serial: mpc512x: cleanup clock API use
mtd: mpc5121_nfc: prepare clocks before enabling them
powerpc: mpc512x: array decl for MCLK registers in CCM
clk: wrap I/O access for improved portability
dts: mpc512x: prepare for preprocessor support
dts: mpc512x: introduce dt-bindings/clock/ header
dts: mpc512x: add clock related device tree specs
clk: mpc512x: introduce COMMON_CLK for MPC512x
dts: mpc512x: add clock specs for client lookups
spi: mpc512x: remove now obsolete clock lookup name
serial: mpc512x: remove now obsolete clock lookup name
clk: mpc512x: remove now obsolete clkdev registration
serial: mpc512x: setup the PSC FIFO clock as well
net: can: mscan: add a comment on reg to idx mapping
net: can: mscan: make mpc512x code use common clock powerpc/mpc512x: improve DIU related clock setup
i2c: mpc: OF clock lookup for MPC512x
USB: fsl-mph-dr-of: OF clock lookup, prepare and enable
fs_enet: OF clock lookup (non-fatal), prepare and enable [media] fsl-viu: OF clock lookup, prepare before enable powerpc/fsl-pci: OF clock lookup, prepare before enable
clk: mpc512x: switch to COMMON_CLK, remove PPC_CLOCK
net: can: mscan: remove MPC512x non-COMMON_CLK code path

 arch/powerpc/boot/dts/ac14xx.dts                   |    9 +-
 arch/powerpc/boot/dts/include/dt-bindings          |    1 +
 arch/powerpc/boot/dts/mpc5121.dtsi                 |   94 ++-
 arch/powerpc/boot/dts/mpc5121ads.dts               |    2 +-
 arch/powerpc/boot/dts/pdm360ng.dts                 |    2 +-
 arch/powerpc/include/asm/mpc5121.h                 |   18 +-
 arch/powerpc/platforms/512x/Kconfig                |    2 +-
 arch/powerpc/platforms/512x/Makefile               |    3 +-
 arch/powerpc/platforms/512x/clock-commonclk.c      |  761 ++++++++++++++++++++
 arch/powerpc/platforms/512x/clock.c                |  753 -------------------
 arch/powerpc/platforms/512x/mpc512x_shared.c       |  165 +++--
 arch/powerpc/sysdev/fsl_pci.c                      |   15 +
 drivers/clk/clk-divider.c                          |    6 +-
 drivers/clk/clk-gate.c                             |    6 +-
 drivers/clk/clk-mux.c                              |    6 +-
 drivers/i2c/busses/i2c-mpc.c                       |    9 +
 drivers/media/platform/fsl-viu.c                   |   12 +-
 drivers/mtd/nand/mpc5121_nfc.c                     |    4 +-
 drivers/net/can/mscan/mpc5xxx_can.c                |  234 +++---
 .../net/ethernet/freescale/fs_enet/fs_enet-main.c  |   27 +-
 drivers/spi/spi-mpc512x-psc.c                      |   45 +-
 drivers/tty/serial/mpc52xx_uart.c                  |  137 +++-
 drivers/usb/host/fsl-mph-dr-of.c                   |   24 +-
 include/dt-bindings/clock/mpc512x-clock.h          |   59 ++
 include/linux/clk-provider.h                       |   33 +
 include/linux/fs_enet_pd.h                         |    3 +
 26 files changed, 1392 insertions(+), 1038 deletions(-)
 create mode 120000 arch/powerpc/boot/dts/include/dt-bindings
 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

# uname -srm
Linux 3.11.0-rc1-00024-g96ff9fe ppc
# cat /sys/kernel/debug/clk/clk_summary
   clock                        enable_cnt  prepare_cnt  rate
---------------------------------------------------------------------
 ac97                           0           0            24567000
 spdif_rx_in                    0           0            0
 spdif_tx_in                    0           0            0
 psc_mclk_in                    0           0            25000000
 dummy                          1           1            0
 osc                            1           1            25000000
    ref                         1           1            25000000
       sys                      10          10           320000000
          spdif-mux0            0           0            320000000
             spdif-en0          0           0            320000000
                spdif_mclk_div  0           0            80000000
                   spdif_mclk_out 0           0            80000000
                      spdif_mclk 0           0            80000000
          mscan3-mux0           1           1            320000000
             mscan3-en0         1           1            320000000
                mscan3_mclk_div 1           1            80000000
                   mscan3_mclk_out 1           1            80000000
                      mscan3_mclk 1           1            80000000
          mscan2-mux0           1           1            320000000
             mscan2-en0         1           1            320000000
                mscan2_mclk_div 1           1            80000000
                   mscan2_mclk_out 1           1            80000000
                      mscan2_mclk 1           1            80000000
          mscan1-mux0           1           1            320000000
             mscan1-en0         1           1            320000000
                mscan1_mclk_div 1           1            16000000
                   mscan1_mclk_out 1           1            16000000
                      mscan1_mclk 2           2            16000000
          mscan0-mux0           1           1            320000000
             mscan0-en0         1           1            320000000
                mscan0_mclk_div 1           1            16000000
                   mscan0_mclk_out 1           1            16000000
                      mscan0_mclk 2           2            16000000
          psc11-mux0            0           0            320000000
             psc11-en0          0           0            320000000
                psc11_mclk_div  0           0            80000000
                   psc11_mclk_out 0           0            80000000
                      psc11_mclk 0           0            80000000
          psc10-mux0            0           0            320000000
             psc10-en0          0           0            320000000
                psc10_mclk_div  0           0            80000000
                   psc10_mclk_out 0           0            80000000
                      psc10_mclk 0           0            80000000
          psc9-mux0             0           0            320000000
             psc9-en0           0           0            320000000
                psc9_mclk_div   0           0            80000000
                   psc9_mclk_out 0           0            80000000
                      psc9_mclk 0           0            80000000
          psc8-mux0             0           0            320000000
             psc8-en0           0           0            320000000
                psc8_mclk_div   0           0            80000000
                   psc8_mclk_out 0           0            80000000
                      psc8_mclk 0           0            80000000
          psc7-mux0             1           1            320000000
             psc7-en0           1           1            320000000
                psc7_mclk_div   1           1            80000000
                   psc7_mclk_out 1           1            80000000
                      psc7_mclk 1           1            80000000
          psc6-mux0             0           0            320000000
             psc6-en0           0           0            320000000
                psc6_mclk_div   0           0            80000000
                   psc6_mclk_out 0           0            80000000
                      psc6_mclk 0           0            80000000
          psc5-mux0             1           1            320000000
             psc5-en0           1           1            320000000
                psc5_mclk_div   1           1            80000000
                   psc5_mclk_out 1           1            80000000
                      psc5_mclk 1           1            80000000
          psc4-mux0             1           1            320000000
             psc4-en0           1           1            320000000
                psc4_mclk_div   1           1            80000000
                   psc4_mclk_out 1           1            80000000
                      psc4_mclk 1           1            80000000
          psc3-mux0             1           1            320000000
             psc3-en0           1           1            320000000
                psc3_mclk_div   1           1            80000000
                   psc3_mclk_out 1           1            80000000
                      psc3_mclk 2           2            80000000
          psc2-mux0             0           0            320000000
             psc2-en0           0           0            320000000
                psc2_mclk_div   0           0            80000000
                   psc2_mclk_out 0           0            80000000
                      psc2_mclk 0           0            80000000
          psc1-mux0             0           0            320000000
             psc1-en0           0           0            320000000
                psc1_mclk_div   0           0            80000000
                   psc1_mclk_out 0           0            80000000
                      psc1_mclk 0           0            80000000
          psc0-mux0             0           0            320000000
             psc0-en0           0           0            320000000
                psc0_mclk_div   0           0            80000000
                   psc0_mclk_out 0           0            80000000
                      psc0_mclk 0           0            80000000
          ddr-ug                1           1            160000000
             ddr                1           1            160000000
          csb                   4           4            160000000
             viu                0           0            160000000
             iim                0           0            160000000
             usb2               0           0            160000000
             usb1               0           0            160000000
             axe                0           0            160000000
             pci-ug             0           0            26666666
                pci             0           0            26666666
             mbx-bus-ug         0           0            80000000
                mbx-bus         0           0            80000000
                mbx-ug          0           0            80000000
                   mbx          0           0            80000000
                   mbx-3d-ug    0           0            80000000
                      mbx-3d    0           0            80000000
             e300               1           1            400000000
             diu-x4             1           1            640000000
                diu-ug          1           1            2509803
                   diu          2           2            2509803
             sdhc-x4            1           1            640000000
                sdhc-ug         1           1            53333333
                   sdhc-2       0           0            53333333
                   sdhc         1           1            53333333
             ips                7           7            80000000
                i2c             3           3            80000000
                mem             1           1            80000000
                fec             1           1            80000000
                sata            0           0            80000000
                psc-fifo        1           1            80000000
                pata            0           0            80000000
                lpc-ug          1           1            40000000
                   lpc          1           1            40000000
                nfc-ug          0           0            40000000
                   nfc          0           0            40000000

1.7.10.4

cleanup the MPC512x SoC's SPI master's use of the clock API
- get, prepare, and enable the MCLK during probe; disable, unprepare and put the MCLK upon remove; hold a reference to the clock over the period of use
- fetch MCLK rate (reference) once during probe and slightly reword BCLK (bitrate) determination to reduce redundancy as well as to not exceed the maximum text line length
- stick with the PPC_CLOCK 'psc%d_mclk' name for clock lookup, only switch to a fixed string later after device tree based clock lookup will have become available

the omission in the clock handling previously went unnoticed, but will become fatal in the common clock scenario

---
 drivers/spi/spi-mpc512x-psc.c |   49 ++++++++++++++++++++++++++---------------
 1 file changed, 31 insertions(+), 18 deletions(-)

index 29fce6a..7a32373 100644
@@ -38,7 +38,8 @@ struct mpc512x_psc_spi {
 	struct mpc512x_psc_fifo __iomem *fifo;
 	unsigned int irq;
 	u8 bits_per_word;
-	u32 mclk;
+	struct clk *clk_mclk;
+	u32 mclk_rate;
 
 	struct completion txisrdone;
 };

@@ -72,6 +73,7 @@ static void mpc512x_psc_spi_activate_cs(struct spi_device *spi)
 	struct mpc52xx_psc __iomem *psc = mps->psc;
 	u32 sicr;
 	u32 ccr;
+	int speed;
 	u16 bclkdiv;
 
 	sicr = in_be32(&psc->sicr);

@@ -95,10 +97,10 @@ static void mpc512x_psc_spi_activate_cs(struct spi_device *spi)
 
 	ccr = in_be32(&psc->ccr);
 	ccr &= 0xFF000000;
-	if (cs->speed_hz)
-		bclkdiv = (mps->mclk / cs->speed_hz) - 1;
-	else
-		bclkdiv = (mps->mclk / 1000000) - 1;	/* default 1MHz */
+	speed = cs->speed_hz;
+	if (!speed)
+		speed = 1000000;	/* default 1MHz */
+	bclkdiv = (mps->mclk_rate / speed) - 1;
 
 	ccr |= (((bclkdiv & 0xff) << 16) | (((bclkdiv >> 8) & 0xff) << 8));
 	out_be32(&psc->ccr, ccr);

@@ -386,19 +388,11 @@ static int mpc512x_psc_spi_port_config(struct spi_master *master,
 {
 	struct mpc52xx_psc __iomem *psc = mps->psc;
 	struct mpc512x_psc_fifo __iomem *fifo = mps->fifo;
-	struct clk *spiclk;
-	int ret = 0;
-	char name[32];
 	u32 sicr;
 	u32 ccr;
+	int speed;
 	u16 bclkdiv;
 
-	sprintf(name, "psc%d_mclk", master->bus_num);
-	spiclk = clk_get(&master->dev, name);
-	clk_enable(spiclk);
-	mps->mclk = clk_get_rate(spiclk);
-	clk_put(spiclk);
-
 	/* Reset the PSC into a known state */
 	out_8(&psc->command, MPC52xx_PSC_RST_RX);
 	out_8(&psc->command, MPC52xx_PSC_RST_TX);

@@ -425,7 +419,8 @@ static int mpc512x_psc_spi_port_config(struct spi_master *master,
 
 	ccr = in_be32(&psc->ccr);
 	ccr &= 0xFF000000;
-	bclkdiv = (mps->mclk / 1000000) - 1;	/* default 1MHz */
+	speed = 1000000;	/* default 1MHz */
+	bclkdiv = (mps->mclk_rate / speed) - 1;
 	ccr |= (((bclkdiv & 0xff) << 16) | (((bclkdiv >> 8) & 0xff) << 8));
 	out_be32(&psc->ccr, ccr);
 

@@ -445,7 +440,7 @@ static int mpc512x_psc_spi_port_config(struct spi_master *master,
 
 	mps->bits_per_word = 8;
 
-	return ret;
+	return 0;
 }
 
 static irqreturn_t mpc512x_psc_spi_isr(int irq, void *dev_id)

@@ -479,6 +474,8 @@ static int mpc512x_psc_spi_do_probe(struct device *dev, u32 regaddr,
 	struct spi_master *master;
 	int ret;
 	void *tempp;
+	int psc_num;
+	char clk_name[16];
 
 	master = spi_alloc_master(dev, sizeof *mps);
 	if (master == NULL)

@@ -521,16 +518,30 @@ static int mpc512x_psc_spi_do_probe(struct device *dev, u32 regaddr,
 		goto free_master;
 	init_completion(&mps->txisrdone);
 
+	psc_num = master->bus_num;
+	snprintf(clk_name, sizeof(clk_name), "psc%d_mclk", psc_num);
+	mps->clk_mclk = clk_get(dev, clk_name);
+	if (IS_ERR(mps->clk_mclk))
+		goto free_irq;
+	ret = clk_prepare_enable(mps->clk_mclk);
+	if (ret)
+		goto free_putclk;
+	mps->mclk_rate = clk_get_rate(mps->clk_mclk);
+
 	ret = mpc512x_psc_spi_port_config(master, mps);
 	if (ret < 0)
-		goto free_irq;
+		goto free_disclk;
 
 	ret = spi_register_master(master);
 	if (ret < 0)
-		goto free_irq;
+		goto free_disclk;
 
 	return ret;
 
+free_disclk:
+	clk_disable_unprepare(mps->clk_mclk);
+free_putclk:
+	clk_put(mps->clk_mclk);
 free_irq:
 	free_irq(mps->irq, mps);
 free_master:

@@ -547,6 +558,8 @@ static int mpc512x_psc_spi_do_remove(struct device *dev)
 	struct mpc512x_psc_spi *mps = spi_master_get_devdata(master);
 
 	spi_unregister_master(master);
+	clk_disable_unprepare(mps->clk_mclk);
+	clk_put(mps->clk_mclk);
 	free_irq(mps->irq, mps);
 	if (mps->psc)
 		iounmap(mps->psc);

1.7.10.4

cleanup the clock API use of the UART driver which is shared among the MPC512x and the MPC5200 platforms
- get, prepare, and enable the MCLK during port allocation; disable, unprepare and put the MCLK upon port release; hold a reference to the clock over the period of use; check for and propagate enable errors
- fix a buffer overflow for clock names with two digit PSC index numbers
- stick with the PPC_CLOCK 'psc%d_mclk' name for clock lookup, only switch to a fixed string later after device tree based clock lookup will have become available

the omission in the clock handling previously went unnoticed, but will become fatal in the common clock scenario

to achieve support for MPC512x which is neutral to MPC5200, the modification was done as follows
- introduce "clock alloc" and "clock release" routines in addition to the previous "clock enable/disable" routine in the psc_ops struct
- make the clock allocation a part of the port request (resource allocation), and make clock release a part of the port release, such that essential resources get allocated early
- just enable/disable the clock from within the .clock() callback without any allocation or preparation as the former implementation did, since this routine is called from within the startup and shutdown callbacks
- all of the above remains a NOP for the MPC5200 platform (no callbacks are provided on that platform)
- implementation note: the clock gets enabled upon allocation already just in case the clock is not only required for bitrate generation but for register access as well

---
 drivers/tty/serial/mpc52xx_uart.c |  100 ++++++++++++++++++++++++++++++-------
 1 file changed, 83 insertions(+), 17 deletions(-)

index e1280a2..98a1f43 100644
@@ -107,6 +107,8 @@ struct psc_ops {
 	unsigned int	(*set_baudrate)(struct uart_port *port,
 					struct ktermios *new,
 					struct ktermios *old);
+	int		(*clock_alloc)(struct uart_port *port);
+	void		(*clock_relse)(struct uart_port *port);
 	int		(*clock)(struct uart_port *port, int enable);
 	int		(*fifoc_init)(void);
 	void		(*fifoc_uninit)(void);

@@ -616,31 +618,75 @@ static irqreturn_t mpc512x_psc_handle_irq(struct uart_port *port)
 	return IRQ_NONE;
 }
 
-static int mpc512x_psc_clock(struct uart_port *port, int enable)
+static struct clk *psc_mclk_clk[MPC52xx_PSC_MAXNUM];
+
+/* called from within the .request_port() callback (allocation) */
+static int mpc512x_psc_alloc_clock(struct uart_port *port)
 {
-	struct clk *psc_clk;
 	int psc_num;
-	char clk_name[10];
+	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 = clk_get(port->dev, clk_name);
+	if (IS_ERR(clk)) {
+		dev_err(port->dev, "Failed to get MCLK!\n");
+		return PTR_ERR(clk);
+	}
+	err = clk_prepare_enable(clk);
+	if (err) {
+		dev_err(port->dev, "Failed to enable MCLK!\n");
+		clk_put(clk);
+		return err;
+	}
+	psc_mclk_clk[psc_num] = clk;
+	return 0;
+}
+
+/* called from within the .release_port() callback (release) */
+static void mpc512x_psc_relse_clock(struct uart_port *port)
+{
+	int psc_num;
+	struct clk *clk;
+
+	psc_num = (port->mapbase & 0xf00) >> 8;
+	clk = psc_mclk_clk[psc_num];
+	if (clk) {
+		clk_disable_unprepare(clk);
+		clk_put(clk);
+		psc_mclk_clk[psc_num] = NULL;
+	}
+}
+
+/* implementation of the .clock() callback (enable/disable) */
+static int mpc512x_psc_endis_clock(struct uart_port *port, int enable)
+{
+	int psc_num;
+	struct clk *psc_clk;
+	int ret;
 
 	if (uart_console(port))
 		return 0;
 
 	psc_num = (port->mapbase & 0xf00) >> 8;
-	snprintf(clk_name, sizeof(clk_name), "psc%d_mclk", psc_num);
-	psc_clk = clk_get(port->dev, clk_name);
-	if (IS_ERR(psc_clk)) {
+	psc_clk = psc_mclk_clk[psc_num];
+	if (!psc_clk) {
 		dev_err(port->dev, "Failed to get PSC clock entry!\n");
 		return -ENODEV;
 	}
 
-	dev_dbg(port->dev, "%s %sable\n", clk_name, enable ? "en" : "dis");
-
-	if (enable)
-		clk_enable(psc_clk);
-	else
+	dev_dbg(port->dev, "mclk %sable\n", enable ? "en" : "dis");
+	if (enable) {
+		ret = clk_enable(psc_clk);
+		if (ret)
+			dev_err(port->dev, "Failed to enable MCLK!\n");
+		return ret;
+	} else {
 		clk_disable(psc_clk);
-
-	return 0;
+		return 0;
+	}
 }
 
 static void mpc512x_psc_get_irq(struct uart_port *port, struct device_node *np)

@@ -873,7 +919,9 @@ static struct psc_ops mpc5125_psc_ops = {
 	.cw_disable_ints = mpc5125_psc_cw_disable_ints,
 	.cw_restore_ints = mpc5125_psc_cw_restore_ints,
 	.set_baudrate = mpc5125_psc_set_baudrate,
-	.clock = mpc512x_psc_clock,
+	.clock_alloc = mpc512x_psc_alloc_clock,
+	.clock_relse = mpc512x_psc_relse_clock,
+	.clock = mpc512x_psc_endis_clock,
 	.fifoc_init = mpc512x_psc_fifoc_init,
 	.fifoc_uninit = mpc512x_psc_fifoc_uninit,
 	.get_irq = mpc512x_psc_get_irq,

@@ -906,7 +954,9 @@ static struct psc_ops mpc512x_psc_ops = {
 	.cw_disable_ints = mpc512x_psc_cw_disable_ints,
 	.cw_restore_ints = mpc512x_psc_cw_restore_ints,
 	.set_baudrate = mpc512x_psc_set_baudrate,
-	.clock = mpc512x_psc_clock,
+	.clock_alloc = mpc512x_psc_alloc_clock,
+	.clock_relse = mpc512x_psc_relse_clock,
+	.clock = mpc512x_psc_endis_clock,
 	.fifoc_init = mpc512x_psc_fifoc_init,
 	.fifoc_uninit = mpc512x_psc_fifoc_uninit,
 	.get_irq = mpc512x_psc_get_irq,

@@ -1166,6 +1216,9 @@ mpc52xx_uart_type(struct uart_port *port)
 static void
 mpc52xx_uart_release_port(struct uart_port *port)
 {
+	if (psc_ops->clock_relse)
+		psc_ops->clock_relse(port);
+
 	/* remapped by us ? */
 	if (port->flags & UPF_IOREMAP) {
 		iounmap(port->membase);

@@ -1190,11 +1243,24 @@ mpc52xx_uart_request_port(struct uart_port *port)
 	err = request_mem_region(port->mapbase, sizeof(struct mpc52xx_psc),
 			"mpc52xx_psc_uart") != NULL ? 0 : -EBUSY;
 
-	if (err && (port->flags & UPF_IOREMAP)) {
+	if (err)
+		goto out_membase;
+
+	if (psc_ops->clock_alloc) {
+		err = psc_ops->clock_alloc(port);
+		if (err)
+			goto out_mapregion;
+	}
+
+	return 0;
+
+out_mapregion:
+	release_mem_region(port->mapbase, sizeof(struct mpc52xx_psc));
+out_membase:
+	if (port->flags & UPF_IOREMAP) {
 		iounmap(port->membase);
 		port->membase = NULL;
 	}
-
 	return err;
 }
 

1.7.10.4

must prepare clocks before enabling them, unprepare after disable

---
 drivers/mtd/nand/mpc5121_nfc.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

index 3c9cdcb..eb7771d 100644
@@ -618,7 +618,7 @@ static void mpc5121_nfc_free(struct device *dev, struct mtd_info *mtd)
 	struct mpc5121_nfc_prv *prv = chip->priv;
 
 	if (prv->clk) {
-		clk_disable(prv->clk);
+		clk_disable_unprepare(prv->clk);
 		clk_put(prv->clk);
 	}
 

@@ -737,7 +737,7 @@ static int mpc5121_nfc_probe(struct platform_device *op)
 		goto error;
 	}
 
-	clk_enable(prv->clk);
+	clk_prepare_enable(prv->clk);
 
 	/* Reset NAND Flash controller */
 	nfc_set(mtd, NFC_CONFIG1, NFC_RESET);

1.7.10.4

reword the clock control module's registers declaration such that the MCLK related registers form an array and get indexed by PSC number

this change is in preparation to COMMON_CLK support for the MPC512x platform, the changed declaration remains neutral to existing code since the PSC and MSCAN CCR fields declared here aren't referenced anywhere

---
 arch/powerpc/include/asm/mpc5121.h |   18 ++----------------
 1 file changed, 2 insertions(+), 16 deletions(-)

index 8ae133e..887d3d6 100644
@@ -32,25 +32,11 @@ struct mpc512x_ccm {
 	u32	scfr2;	/* System Clock Frequency Register 2 */
 	u32	scfr2s;	/* System Clock Frequency Shadow Register 2 */
 	u32	bcr;	/* Bread Crumb Register */
-	u32	p0ccr;	/* PSC0 Clock Control Register */
-	u32	p1ccr;	/* PSC1 CCR */
-	u32	p2ccr;	/* PSC2 CCR */
-	u32	p3ccr;	/* PSC3 CCR */
-	u32	p4ccr;	/* PSC4 CCR */
-	u32	p5ccr;	/* PSC5 CCR */
-	u32	p6ccr;	/* PSC6 CCR */
-	u32	p7ccr;	/* PSC7 CCR */
-	u32	p8ccr;	/* PSC8 CCR */
-	u32	p9ccr;	/* PSC9 CCR */
-	u32	p10ccr;	/* PSC10 CCR */
-	u32	p11ccr;	/* PSC11 CCR */
+	u32	psc_ccr[12];	/* PSC Clock Control Registers */
 	u32	spccr;	/* SPDIF Clock Control Register */
 	u32	cccr;	/* CFM Clock Control Register */
 	u32	dccr;	/* DIU Clock Control Register */
-	u32	m1ccr;	/* MSCAN1 CCR */
-	u32	m2ccr;	/* MSCAN2 CCR */
-	u32	m3ccr;	/* MSCAN3 CCR */
-	u32	m4ccr;	/* MSCAN4 CCR */
+	u32	mscan_ccr[4];	/* MSCAN Clock Control Registers */
 	u8	res[0x98]; /* Reserved */
 };
 

1.7.10.4

the common clock drivers were motivated/initiated by ARM development and apparently assume little endian peripherals

wrap register/peripherals access in the common code (div, gate, mux) in preparation of adding COMMON_CLK support for other platforms

---
 drivers/clk/clk-divider.c    |    6 +++---
 drivers/clk/clk-gate.c       |    6 +++---
 drivers/clk/clk-mux.c        |    6 +++---
 include/linux/clk-provider.h |   17 +++++++++++++++++
 4 files changed, 26 insertions(+), 9 deletions(-)

index 6d55eb2..2c07061 100644
@@ -104,7 +104,7 @@ static unsigned long clk_divider_recalc_rate(struct clk_hw *hw,
 	struct clk_divider *divider = to_clk_divider(hw);
 	unsigned int div, val;
 
-	val = readl(divider->reg) >> divider->shift;
+	val = clk_readl(divider->reg) >> divider->shift;
 	val &= div_mask(divider);
 
 	div = _get_div(divider, val);

@@ -230,11 +230,11 @@ static int clk_divider_set_rate(struct clk_hw *hw, unsigned long rate,
 	if (divider->flags & CLK_DIVIDER_HIWORD_MASK) {
 		val = div_mask(divider) << (divider->shift + 16);
 	} else {
-		val = readl(divider->reg);
+		val = clk_readl(divider->reg);
 		val &= ~(div_mask(divider) << divider->shift);
 	}
 	val |= value << divider->shift;
-	writel(val, divider->reg);
+	clk_writel(val, divider->reg);
 
 	if (divider->lock)
 		spin_unlock_irqrestore(divider->lock, flags);

index 790306e..b7fbd96 100644
@@ -58,7 +58,7 @@ static void clk_gate_endisable(struct clk_hw *hw, int enable)
 		if (set)
 			reg |= BIT(gate->bit_idx);
 	} else {
-		reg = readl(gate->reg);
+		reg = clk_readl(gate->reg);
 
 		if (set)
 			reg |= BIT(gate->bit_idx);

@@ -66,7 +66,7 @@ static void clk_gate_endisable(struct clk_hw *hw, int enable)
 			reg &= ~BIT(gate->bit_idx);
 	}
 
-	writel(reg, gate->reg);
+	clk_writel(reg, gate->reg);
 
 	if (gate->lock)
 		spin_unlock_irqrestore(gate->lock, flags);

@@ -89,7 +89,7 @@ static int clk_gate_is_enabled(struct clk_hw *hw)
 	u32 reg;
 	struct clk_gate *gate = to_clk_gate(hw);
 
-	reg = readl(gate->reg);
+	reg = clk_readl(gate->reg);
 
 	/* if a set bit disables this clk, flip it before masking */
 	if (gate->flags & CLK_GATE_SET_TO_DISABLE)

index 614444c..02ef506 100644
@@ -42,7 +42,7 @@ static u8 clk_mux_get_parent(struct clk_hw *hw)
 	 * OTOH, pmd_trace_clk_mux_ck uses a separate bit for each clock, so
 	 * val = 0x4 really means "bit 2, index starts at bit 0"
 	 */
-	val = readl(mux->reg) >> mux->shift;
+	val = clk_readl(mux->reg) >> mux->shift;
 	val &= mux->mask;
 
 	if (mux->table) {

@@ -89,11 +89,11 @@ static int clk_mux_set_parent(struct clk_hw *hw, u8 index)
 	if (mux->flags & CLK_MUX_HIWORD_MASK) {
 		val = mux->mask << (mux->shift + 16);
 	} else {
-		val = readl(mux->reg);
+		val = clk_readl(mux->reg);
 		val &= ~(mux->mask << mux->shift);
 	}
 	val |= index << mux->shift;
-	writel(val, mux->reg);
+	clk_writel(val, mux->reg);
 
 	if (mux->lock)
 		spin_unlock_irqrestore(mux->lock, flags);

index 1ec14a7..c4f7799 100644
@@ -12,6 +12,7 @@
 #define __LINUX_CLK_PROVIDER_H
 
 #include <linux/clk.h>
+#include <linux/io.h>
 
 #ifdef CONFIG_COMMON_CLK
 

@@ -490,5 +491,21 @@ static inline const char *of_clk_get_parent_name(struct device_node *np,
 #define of_clk_init(matches) \
 	{ while (0); }
 #endif /* CONFIG_OF */
+
+/*
+ * wrap access to peripherals in accessor routines
+ * for improved portability across platforms
+ */
+
+static inline u32 clk_readl(u32 __iomem *reg)
+{
+	return readl(reg);
+}
+
+static inline void clk_writel(u32 val, u32 __iomem *reg)
+{
+	writel(val, reg);
+}
+
 #endif /* CONFIG_COMMON_CLK */
 #endif /* CLK_PROVIDER_H */

1.7.10.4

prepare C preprocessor support when processing MPC512x DTS files
- switch from DTS syntax to CPP syntax for include specs
- create a symlink such that DTS processing can reference includes

---
 arch/powerpc/boot/dts/ac14xx.dts          |    2 +-
 arch/powerpc/boot/dts/include/dt-bindings |    1 +
 arch/powerpc/boot/dts/mpc5121ads.dts      |    2 +-
 arch/powerpc/boot/dts/pdm360ng.dts        |    2 +-
 4 files changed, 4 insertions(+), 3 deletions(-)
 create mode 120000 arch/powerpc/boot/dts/include/dt-bindings

index a27a460..a543c40 100644
@@ -10,7 +10,7 @@
  */
 
 
-/include/ "mpc5121.dtsi"
+#include <mpc5121.dtsi>
 
 / {
 	model = "ac14xx";

new file mode 120000
index 0000000..08c00e4
@@ -0,0 +1 @@
+../../../../../include/dt-bindings
\ No newline at end of file

index 7d3cb79..c228a0a 100644
@@ -9,7 +9,7 @@
  * option) any later version.
  */
 
-/include/ "mpc5121.dtsi"
+#include <mpc5121.dtsi>
 
 / {
 	model = "mpc5121ads";

index 7433740..871c16d 100644
@@ -13,7 +13,7 @@
  * option) any later version.
  */
 
-/include/ "mpc5121.dtsi"
+#include <mpc5121.dtsi>
 
 / {
 	model = "pdm360ng";

1.7.10.4

introduce a dt-bindings/ header file for MPC512x clocks, providing symbolic identifiers for those SoC clocks which clients will reference from their device tree nodes

---
 include/dt-bindings/clock/mpc512x-clock.h |   59 +++++++++++++++++++++++++++++
 1 file changed, 59 insertions(+)
 create mode 100644 include/dt-bindings/clock/mpc512x-clock.h

new file mode 100644
index 0000000..46c560e
@@ -0,0 +1,59 @@
+/*
+ * This header provides constants for MPC512x clock specs in DT bindings.
+ *
+ * Unfortunately the clock number declaration cannot be an enum but
+ * needs to be a list of #define directives since when referenced from
+ * within DTS files they need to get resolved "at compile time".
+ */
+
+#ifndef _DT_BINDINGS_CLOCK_MPC512x_CLOCK_H
+#define _DT_BINDINGS_CLOCK_MPC512x_CLOCK_H
+
+#define MPC512x_CLK_DUMMY		0
+#define MPC512x_CLK_REF			1
+#define MPC512x_CLK_SYS			2
+#define MPC512x_CLK_DIU			3
+#define MPC512x_CLK_VIU			4
+#define MPC512x_CLK_CSB			5
+#define MPC512x_CLK_E300		6
+#define MPC512x_CLK_IPS			7
+#define MPC512x_CLK_FEC			8
+#define MPC512x_CLK_SATA		9
+#define MPC512x_CLK_PATA		10
+#define MPC512x_CLK_NFC			11
+#define MPC512x_CLK_LPC			12
+#define MPC512x_CLK_MBX_BUS		13
+#define MPC512x_CLK_MBX			14
+#define MPC512x_CLK_MBX_3D		15
+#define MPC512x_CLK_AXE			16
+#define MPC512x_CLK_USB1		17
+#define MPC512x_CLK_USB2		18
+#define MPC512x_CLK_I2C			19
+#define MPC512x_CLK_MSCAN0_MCLK		20
+#define MPC512x_CLK_MSCAN1_MCLK		21
+#define MPC512x_CLK_MSCAN2_MCLK		22
+#define MPC512x_CLK_MSCAN3_MCLK		23
+#define MPC512x_CLK_SDHC		24
+#define MPC512x_CLK_PCI			25
+#define MPC512x_CLK_PSC_MCLK_IN		26
+#define MPC512x_CLK_SPDIF_TX		27
+#define MPC512x_CLK_SPDIF_RX		28
+#define MPC512x_CLK_SPDIF_MCLK		29
+#define MPC512x_CLK_AC97		30
+#define MPC512x_CLK_PSC0_MCLK		31
+#define MPC512x_CLK_PSC1_MCLK		32
+#define MPC512x_CLK_PSC2_MCLK		33
+#define MPC512x_CLK_PSC3_MCLK		34
+#define MPC512x_CLK_PSC4_MCLK		35
+#define MPC512x_CLK_PSC5_MCLK		36
+#define MPC512x_CLK_PSC6_MCLK		37
+#define MPC512x_CLK_PSC7_MCLK		38
+#define MPC512x_CLK_PSC8_MCLK		39
+#define MPC512x_CLK_PSC9_MCLK		40
+#define MPC512x_CLK_PSC10_MCLK		41
+#define MPC512x_CLK_PSC11_MCLK		42
+#define MPC512x_CLK_PSC_FIFO		43
+
+#define MPC512x_CLK_LAST_PUBLIC		43
+
+#endif

1.7.10.4

this addresses the clock driver aka provider's side of clocks
- prepare for future '<&clks ID>' phandle references for device tree based clock lookup in client drivers
- introduce a 'clocks' subtree with an 'osc' node for the crystal or oscillator SoC input (fixed frequency)
- provide default values with 33MHz oscillator frequency in the common include (the 66MHz IPS bus already was there), add override values for the ifm AC14xx board which deviates from the reference design (25MHz xtal, 80MHz IPS bus)

---
 arch/powerpc/boot/dts/ac14xx.dts   |    7 +++++++
 arch/powerpc/boot/dts/mpc5121.dtsi |   15 ++++++++++++++-
 2 files changed, 21 insertions(+), 1 deletion(-)

index a543c40..a1b8837 100644
@@ -139,7 +139,14 @@
 		};
 	};
 
+	clocks {
+		osc {
+			clock-frequency = <25000000>;
+		};
+	};
+
 	soc at 80000000 {
+		bus-frequency = <80000000>;	/* 80 MHz ips bus */
 
 		clock at f00 {
 			compatible = "fsl,mpc5121rev2-clock", "fsl,mpc5121-clock";

index bd14c00..8f4cba0 100644
@@ -9,6 +9,8 @@
  * option) any later version.
  */
 
+#include <dt-bindings/clock/mpc512x-clock.h>
+
 /dts-v1/;
 
 / {

@@ -73,6 +75,16 @@
 		ranges = <0x0 0x0 0xfc000000 0x04000000>;
 	};
 
+	clocks {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		osc {
+			compatible = "fsl,mpc512x-osc", "fixed-clock";
+			clock-frequency = <33000000>;
+		};
+	};
+
 	soc at 80000000 {
 		compatible = "fsl,mpc5121-immr";
 		#address-cells = <1>;

@@ -118,9 +130,10 @@
 		};
 
 		/* Clock control */
-		clock at f00 {
+		clks: clock at f00 {
 			compatible = "fsl,mpc5121-clock";
 			reg = <0xf00 0x100>;
+			#clock-cells = <1>;
 		};
 
 		/* Power Management Controller */

1.7.10.4

this change introduces a clock infrastructure implementation for the MPC512x PowerPC platform which follows the COMMON_CLK approach and uses common clock drivers shared with other platforms

this driver implements the publicly announced set of clocks (which can get referenced by means of symbolic identifiers from the dt-bindings header file), as well as generates additional 'struct clk' items where the SoC hardware cannot easily get mapped to the common primitives of the clock API, or requires "intermediate" clock nodes to represent clocks that have both gates and dividers

the previous PPC_CLOCK implementation is kept in place and resides in parallel to the common clock implementation for test and comparison during migration, a compile time option picks one of the two alternatives (Kconfig switch, common clock used by default)

since not all drivers for peripherals were adjusted yet to properly allocate and release their clock items, this platform clock driver implementation pre-enables some of the clock items to not break peripheral drivers during migration -- these clock pre-enable workarounds will get removed as peripheral drivers get adjusted

to provide clock names which the serial communication drivers are using and where the PSC index number is encoded into, clkdev registration is done to not break these peripheral drivers -- this workaround will get removed as these drivers get adjusted after device tree based clock lookup has become available

---
 arch/powerpc/platforms/512x/Kconfig           |   14 +-
 arch/powerpc/platforms/512x/Makefile          |    4 +-
 arch/powerpc/platforms/512x/clock-commonclk.c |  778 +++++++++++++++++++++++++
 include/linux/clk-provider.h                  |   16 +
 4 files changed, 810 insertions(+), 2 deletions(-)
 create mode 100644 arch/powerpc/platforms/512x/clock-commonclk.c

index fc9c1cb..c5fcdd0 100644
@@ -1,9 +1,21 @@
+config MPC512x_COMMON_CLK
+	bool "MPC512x platform uses COMMON_CLK"
+	default y
+	depends on PPC_MPC512x
+	help
+	  This option is only here to support tests and comparison
+	  during development and migration.  This option will get
+	  removed after the COMMON_CLK support for MPC512x has become
+	  fully operational and all drivers were adjusted to explicitly
+	  acquire their required clocks.
+
 config PPC_MPC512x
 	bool "512x-based boards"
 	depends on 6xx
 	select FSL_SOC
 	select IPIC
-	select PPC_CLOCK
+	select PPC_CLOCK if !MPC512x_COMMON_CLK
+	select COMMON_CLK if MPC512x_COMMON_CLK
 	select PPC_PCI_CHOICE
 	select FSL_PCI if PCI
 	select ARCH_WANT_OPTIONAL_GPIOLIB

index 72fb934..1e05f9d 100644
@@ -1,7 +1,9 @@
 #
 # Makefile for the Freescale PowerPC 512x linux kernel.
 #
-obj-y				+= clock.o mpc512x_shared.o
+obj-$(CONFIG_PPC_CLOCK)		+= clock.o
+obj-$(CONFIG_COMMON_CLK)	+= clock-commonclk.o
+obj-y				+= mpc512x_shared.o
 obj-$(CONFIG_MPC5121_ADS)	+= mpc5121_ads.o mpc5121_ads_cpld.o
 obj-$(CONFIG_MPC512x_GENERIC)	+= mpc512x_generic.o
 obj-$(CONFIG_PDM360NG)		+= pdm360ng.o

index c4f7799..7f8fc64 100644
@@ -497,6 +497,20 @@ static inline const char *of_clk_get_parent_name(struct device_node *np,
  * for improved portability across platforms
  */
 
+#if IS_ENABLED(CONFIG_PPC)
+
+static inline u32 clk_readl(u32 __iomem *reg)
+{
+	return ioread32be(reg);
+}
+
+static inline void clk_writel(u32 val, u32 __iomem *reg)
+{
+	iowrite32be(val, reg);
+}
+
+#else	/* platform dependent I/O accessors */
+
 static inline u32 clk_readl(u32 __iomem *reg)
 {
 	return readl(reg);

@@ -507,5 +521,7 @@ static inline void clk_writel(u32 val, u32 __iomem *reg)
 	writel(val, reg);
 }
 
+#endif	/* platform dependent I/O accessors */
+
 #endif /* CONFIG_COMMON_CLK */
 #endif /* CLK_PROVIDER_H */

1.7.10.4

this addresses the client side of device tree based clock lookups:
add clock specifiers to the mbx, nfc, mscan, sdhc, i2c, axe, diu, viu, mdio, fec, usb, pata, psc, psc fifo, and pci nodes in the shared mpc5121.dtsi include

these specs map 'clock-names' encoded in drivers to their respective 'struct clk' items in the platform's clock driver; some drivers still need to learn how to lookup OF clocks, other drivers will transparently probe successfully on MPC512x as well (sdhc)

few 'clock-names' (nfc, viu) use strings that are encoded in their respective drivers to keep up compatibility, an alternative approach would be to register clkdev items in the platform's clock driver for the migration period

---
 arch/powerpc/boot/dts/mpc5121.dtsi |   79 ++++++++++++++++++++++++++++++++++++
 1 file changed, 79 insertions(+)

index 8f4cba0..983da9c 100644
@@ -51,6 +51,10 @@
 		compatible = "fsl,mpc5121-mbx";
 		reg = <0x20000000 0x4000>;
 		interrupts = <66 0x8>;
+		clocks = <&clks MPC512x_CLK_MBX_BUS>,
+			 <&clks MPC512x_CLK_MBX_3D>,
+			 <&clks MPC512x_CLK_MBX>;
+		clock-names = "mbx-bus", "mbx-3d", "mbx";
 	};
 
 	sram at 30000000 {

@@ -64,6 +68,8 @@
 		interrupts = <6 8>;
 		#address-cells = <1>;
 		#size-cells = <1>;
+		clocks = <&clks MPC512x_CLK_NFC>;
+		clock-names = "nfc_clk";	/* backwards compat */
 	};
 
 	localbus at 80000020 {

@@ -153,12 +159,22 @@
 			compatible = "fsl,mpc5121-mscan";
 			reg = <0x1300 0x80>;
 			interrupts = <12 0x8>;
+			clocks = <&clks MPC512x_CLK_IPS>,
+				 <&clks MPC512x_CLK_SYS>,
+				 <&clks MPC512x_CLK_REF>,
+				 <&clks MPC512x_CLK_MSCAN0_MCLK>;
+			clock-names = "ips", "sys", "ref", "mclk";
 		};
 
 		can at 1380 {
 			compatible = "fsl,mpc5121-mscan";
 			reg = <0x1380 0x80>;
 			interrupts = <13 0x8>;
+			clocks = <&clks MPC512x_CLK_IPS>,
+				 <&clks MPC512x_CLK_SYS>,
+				 <&clks MPC512x_CLK_REF>,
+				 <&clks MPC512x_CLK_MSCAN1_MCLK>;
+			clock-names = "ips", "sys", "ref", "mclk";
 		};
 
 		sdhc at 1500 {

@@ -167,6 +183,9 @@
 			interrupts = <8 0x8>;
 			dmas = <&dma0 30>;
 			dma-names = "rx-tx";
+			clocks = <&clks MPC512x_CLK_IPS>,
+				 <&clks MPC512x_CLK_SDHC>;
+			clock-names = "ipg", "per";
 		};
 
 		i2c at 1700 {

@@ -175,6 +194,8 @@
 			compatible = "fsl,mpc5121-i2c", "fsl-i2c";
 			reg = <0x1700 0x20>;
 			interrupts = <9 0x8>;
+			clocks = <&clks MPC512x_CLK_I2C>;
+			clock-names = "per";
 		};
 
 		i2c at 1720 {

@@ -183,6 +204,8 @@
 			compatible = "fsl,mpc5121-i2c", "fsl-i2c";
 			reg = <0x1720 0x20>;
 			interrupts = <10 0x8>;
+			clocks = <&clks MPC512x_CLK_I2C>;
+			clock-names = "per";
 		};
 
 		i2c at 1740 {

@@ -191,6 +214,8 @@
 			compatible = "fsl,mpc5121-i2c", "fsl-i2c";
 			reg = <0x1740 0x20>;
 			interrupts = <11 0x8>;
+			clocks = <&clks MPC512x_CLK_I2C>;
+			clock-names = "per";
 		};
 
 		i2ccontrol at 1760 {

@@ -202,30 +227,46 @@
 			compatible = "fsl,mpc5121-axe";
 			reg = <0x2000 0x100>;
 			interrupts = <42 0x8>;
+			clocks = <&clks MPC512x_CLK_AXE>;
+			clock-names = "per";
 		};
 
 		display at 2100 {
 			compatible = "fsl,mpc5121-diu";
 			reg = <0x2100 0x100>;
 			interrupts = <64 0x8>;
+			clocks = <&clks MPC512x_CLK_DIU>;
+			clock-names = "per";
 		};
 
 		can at 2300 {
 			compatible = "fsl,mpc5121-mscan";
 			reg = <0x2300 0x80>;
 			interrupts = <90 0x8>;
+			clocks = <&clks MPC512x_CLK_IPS>,
+				 <&clks MPC512x_CLK_SYS>,
+				 <&clks MPC512x_CLK_REF>,
+				 <&clks MPC512x_CLK_MSCAN2_MCLK>;
+			clock-names = "ips", "sys", "ref", "mclk";
 		};
 
 		can at 2380 {
 			compatible = "fsl,mpc5121-mscan";
 			reg = <0x2380 0x80>;
 			interrupts = <91 0x8>;
+			clocks = <&clks MPC512x_CLK_IPS>,
+				 <&clks MPC512x_CLK_SYS>,
+				 <&clks MPC512x_CLK_REF>,
+				 <&clks MPC512x_CLK_MSCAN3_MCLK>;
+			clock-names = "ips", "sys", "ref", "mclk";
 		};
 
 		viu at 2400 {
 			compatible = "fsl,mpc5121-viu";
 			reg = <0x2400 0x400>;
 			interrupts = <67 0x8>;
+			clocks = <&clks MPC512x_CLK_VIU>;
+			clock-names = "viu_clk";	/* backwards compat */
 		};
 
 		mdio at 2800 {

@@ -233,6 +274,8 @@
 			reg = <0x2800 0x800>;
 			#address-cells = <1>;
 			#size-cells = <0>;
+			clocks = <&clks MPC512x_CLK_FEC>;
+			clock-names = "per";
 		};
 
 		eth0: ethernet at 2800 {

@@ -241,6 +284,8 @@
 			reg = <0x2800 0x800>;
 			local-mac-address = [ 00 00 00 00 00 00 ];
 			interrupts = <4 0x8>;
+			clocks = <&clks MPC512x_CLK_FEC>;
+			clock-names = "per";
 		};
 
 		/* USB1 using external ULPI PHY */

@@ -252,6 +297,8 @@
 			interrupts = <43 0x8>;
 			dr_mode = "otg";
 			phy_type = "ulpi";
+			clocks = <&clks MPC512x_CLK_USB1>;
+			clock-names = "per";
 		};
 
 		/* USB0 using internal UTMI PHY */

@@ -263,6 +310,8 @@
 			interrupts = <44 0x8>;
 			dr_mode = "otg";
 			phy_type = "utmi_wide";
+			clocks = <&clks MPC512x_CLK_USB2>;
+			clock-names = "per";
 		};
 
 		/* IO control */

@@ -281,6 +330,8 @@
 			compatible = "fsl,mpc5121-pata";
 			reg = <0x10200 0x100>;
 			interrupts = <5 0x8>;
+			clocks = <&clks MPC512x_CLK_PATA>;
+			clock-names = "per";
 		};
 
 		/* 512x PSCs are not 52xx PSC compatible */

@@ -292,6 +343,8 @@
 			interrupts = <40 0x8>;
 			fsl,rx-fifo-size = <16>;
 			fsl,tx-fifo-size = <16>;
+			clocks = <&clks MPC512x_CLK_PSC0_MCLK>;
+			clock-names = "mclk";
 		};
 
 		/* PSC1 */

@@ -301,6 +354,8 @@
 			interrupts = <40 0x8>;
 			fsl,rx-fifo-size = <16>;
 			fsl,tx-fifo-size = <16>;
+			clocks = <&clks MPC512x_CLK_PSC1_MCLK>;
+			clock-names = "mclk";
 		};
 
 		/* PSC2 */

@@ -310,6 +365,8 @@
 			interrupts = <40 0x8>;
 			fsl,rx-fifo-size = <16>;
 			fsl,tx-fifo-size = <16>;
+			clocks = <&clks MPC512x_CLK_PSC2_MCLK>;
+			clock-names = "mclk";
 		};
 
 		/* PSC3 */

@@ -319,6 +376,8 @@
 			interrupts = <40 0x8>;
 			fsl,rx-fifo-size = <16>;
 			fsl,tx-fifo-size = <16>;
+			clocks = <&clks MPC512x_CLK_PSC3_MCLK>;
+			clock-names = "mclk";
 		};
 
 		/* PSC4 */

@@ -328,6 +387,8 @@
 			interrupts = <40 0x8>;
 			fsl,rx-fifo-size = <16>;
 			fsl,tx-fifo-size = <16>;
+			clocks = <&clks MPC512x_CLK_PSC4_MCLK>;
+			clock-names = "mclk";
 		};
 
 		/* PSC5 */

@@ -337,6 +398,8 @@
 			interrupts = <40 0x8>;
 			fsl,rx-fifo-size = <16>;
 			fsl,tx-fifo-size = <16>;
+			clocks = <&clks MPC512x_CLK_PSC5_MCLK>;
+			clock-names = "mclk";
 		};
 
 		/* PSC6 */

@@ -346,6 +409,8 @@
 			interrupts = <40 0x8>;
 			fsl,rx-fifo-size = <16>;
 			fsl,tx-fifo-size = <16>;
+			clocks = <&clks MPC512x_CLK_PSC6_MCLK>;
+			clock-names = "mclk";
 		};
 
 		/* PSC7 */

@@ -355,6 +420,8 @@
 			interrupts = <40 0x8>;
 			fsl,rx-fifo-size = <16>;
 			fsl,tx-fifo-size = <16>;
+			clocks = <&clks MPC512x_CLK_PSC7_MCLK>;
+			clock-names = "mclk";
 		};
 
 		/* PSC8 */

@@ -364,6 +431,8 @@
 			interrupts = <40 0x8>;
 			fsl,rx-fifo-size = <16>;
 			fsl,tx-fifo-size = <16>;
+			clocks = <&clks MPC512x_CLK_PSC8_MCLK>;
+			clock-names = "mclk";
 		};
 
 		/* PSC9 */

@@ -373,6 +442,8 @@
 			interrupts = <40 0x8>;
 			fsl,rx-fifo-size = <16>;
 			fsl,tx-fifo-size = <16>;
+			clocks = <&clks MPC512x_CLK_PSC9_MCLK>;
+			clock-names = "mclk";
 		};
 
 		/* PSC10 */

@@ -382,6 +453,8 @@
 			interrupts = <40 0x8>;
 			fsl,rx-fifo-size = <16>;
 			fsl,tx-fifo-size = <16>;
+			clocks = <&clks MPC512x_CLK_PSC10_MCLK>;
+			clock-names = "mclk";
 		};
 
 		/* PSC11 */

@@ -391,12 +464,16 @@
 			interrupts = <40 0x8>;
 			fsl,rx-fifo-size = <16>;
 			fsl,tx-fifo-size = <16>;
+			clocks = <&clks MPC512x_CLK_PSC11_MCLK>;
+			clock-names = "mclk";
 		};
 
 		pscfifo at 11f00 {
 			compatible = "fsl,mpc5121-psc-fifo";
 			reg = <0x11f00 0x100>;
 			interrupts = <40 0x8>;
+			clocks = <&clks MPC512x_CLK_PSC_FIFO>;
+			clock-names = "per";
 		};
 
 		dma0: dma at 14000 {

@@ -414,6 +491,8 @@
 		#address-cells = <3>;
 		#size-cells = <2>;
 		#interrupt-cells = <1>;
+		clocks = <&clks MPC512x_CLK_PCI>;
+		clock-names = "per";
 
 		reg = <0x80008500 0x100	/* internal registers */
 		       0x80008300 0x8>;	/* config space access registers */

1.7.10.4

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"

---
 drivers/spi/spi-mpc512x-psc.c |    6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

index 7a32373..b04f094 100644
@@ -474,8 +474,6 @@ static int mpc512x_psc_spi_do_probe(struct device *dev, u32 regaddr,
 	struct spi_master *master;
 	int ret;
 	void *tempp;
-	int psc_num;
-	char clk_name[16];
 
 	master = spi_alloc_master(dev, sizeof *mps);
 	if (master == NULL)

@@ -518,9 +516,7 @@ static int mpc512x_psc_spi_do_probe(struct device *dev, u32 regaddr,
 		goto free_master;
 	init_completion(&mps->txisrdone);
 
-	psc_num = master->bus_num;
-	snprintf(clk_name, sizeof(clk_name), "psc%d_mclk", psc_num);
-	mps->clk_mclk = clk_get(dev, clk_name);
+	mps->clk_mclk = clk_get(dev, "mclk");
 	if (IS_ERR(mps->clk_mclk))
 		goto free_irq;
 	ret = clk_prepare_enable(mps->clk_mclk);

1.7.10.4

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"

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

index 98a1f43..8643dcf 100644
@@ -623,14 +623,11 @@ static struct clk *psc_mclk_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;
+	int psc_num;
 
-	psc_num = (port->mapbase & 0xf00) >> 8;
-	snprintf(clk_name, sizeof(clk_name), "psc%d_mclk", psc_num);
-	clk = clk_get(port->dev, clk_name);
+	clk = clk_get(port->dev, "mclk");
 	if (IS_ERR(clk)) {
 		dev_err(port->dev, "Failed to get MCLK!\n");
 		return PTR_ERR(clk);

@@ -641,6 +638,7 @@ static int mpc512x_psc_alloc_clock(struct uart_port *port)
 		clk_put(clk);
 		return err;
 	}
+	psc_num = (port->mapbase & 0xf00) >> 8;
 	psc_mclk_clk[psc_num] = clk;
 	return 0;
 }

1.7.10.4

after the peripheral drivers for UART and SPI mode (both using the PSC controller) got converted to device tree based clock lookups, the platform clock driver need no longer provide the "psc%d_mclk" name which depends on the PSC index number -- remove the clk_register_clkdev() call

---
 arch/powerpc/platforms/512x/clock-commonclk.c |   12 ------------
 1 file changed, 12 deletions(-)

index 6627957..b822469 100644
@@ -12,7 +12,6 @@
  */
 
 #include <linux/clk-provider.h>
-#include <linux/clkdev.h>
 #include <linux/device.h>
 #include <linux/errno.h>
 #include <linux/io.h>

@@ -529,17 +528,6 @@ static void mpc512x_clk_setup_mclk(struct mclk_setup_data *entry)
 				entry->name_mclk,
 				entry->name_mux1, 1, 1);
 	}
-
-	/*
-	 * without this "clock device" registration, "simple" lookups in
-	 * the SPI master initialization and serial port setup will fail
-	 *
-	 * those drivers need to get adjusted to lookup their required
-	 * clocks from device tree specs, and device tree nodes need to
-	 * provide the clock specs, before this clkdev registration
-	 * becomes obsolete
-	 */
-	clk_register_clkdev(clks[clks_idx_pub], entry->name_mclk, NULL);
 }
 
 static void mpc512x_clk_setup_mclks(struct mclk_setup_data *table, size_t count)

1.7.10.4

prepare and enable the FIFO clock upon PSC FIFO initialization, disable and unprepare the FIFO clock upon PSC FIFO uninitialization, remove the pre-enable workaround from the platform's clock driver

---
 arch/powerpc/platforms/512x/clock-commonclk.c |    2 --
 drivers/tty/serial/mpc52xx_uart.c             |   39 +++++++++++++++++++++----
 2 files changed, 33 insertions(+), 8 deletions(-)

index b822469..99d197d 100644
@@ -689,8 +689,6 @@ static void mpc512x_clk_setup_clock_tree(int busfreq)
 	clk_prepare_enable(clks[MPC512x_CLK_MEM]);	/* SRAM */
 	clk_prepare_enable(clks[MPC512x_CLK_IPS]);	/* SoC periph */
 	clk_prepare_enable(clks[MPC512x_CLK_LPC]);	/* boot media */
-	/* some are required yet no dependencies were declared */
-	clk_prepare_enable(clks[MPC512x_CLK_PSC_FIFO]);
 	/* some are not yet acquired by their respective drivers */
 	clk_prepare_enable(clks[MPC512x_CLK_PSC3_MCLK]);/* serial console */
 	clk_prepare_enable(clks[MPC512x_CLK_FEC]);	/* network, NFS */

index 8643dcf..3fdadf9 100644
@@ -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)
 {

@@ -574,30 +575,56 @@ static int __init mpc512x_psc_fifoc_init(void)
 				     "fsl,mpc5121-psc-fifo");
 	if (!np) {
 		pr_err("%s: Can't find FIFOC node\n", __func__);
-		return -ENODEV;
+		goto out_err;
+	}
+
+	psc_fifoc_clk = of_clk_get_by_name(np, "per");
+	if (IS_ERR(psc_fifoc_clk)) {
+		pr_err("%s: Can't lookup FIFO clock\n", __func__);
+		goto out_ofnode_put;
+	}
+	if (clk_prepare_enable(psc_fifoc_clk)) {
+		pr_err("%s: Can't enable FIFO clock\n", __func__);
+		goto out_clk_put;
 	}
 
 	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);
+out_clk_put:
+	clk_put(psc_fifoc_clk);
+out_ofnode_put:
+	of_node_put(np);
+out_err:
+	return -ENODEV;
 }
 
 static void __exit mpc512x_psc_fifoc_uninit(void)
 {
 	iounmap(psc_fifoc);
+
+	/* disable the clock, errors are not fatal */
+	if (psc_fifoc_clk && !IS_ERR(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

add a comment about the magic of deriving an MSCAN component index from the peripheral's physical address / register offset

---
 drivers/net/can/mscan/mpc5xxx_can.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

index 5b0ee8e..bc422ba 100644
@@ -148,7 +148,10 @@ static u32 mpc512x_can_get_clock(struct platform_device *ofdev,
 		goto exit_put;
 	}
 
-	/* Determine the MSCAN device index from the physical address */
+	/* Determine the MSCAN device index from the peripheral's
+	 * physical address. Register address offsets against the
+	 * IMMR base are:  0x1300, 0x1380, 0x2300, 0x2380
+	 */
 	pval = of_get_property(ofdev->dev.of_node, "reg", &plen);
 	BUG_ON(!pval || plen < sizeof(*pval));
 	clockidx = (*pval & 0x80) ? 1 : 0;

1.7.10.4

extend the mscan(4) driver with alternative support for the COMMON_CLK approach which is an option in the MPC512x platform, keep the existing clock support implementation in place since the driver is shared with other MPC5xxx SoCs which don't have common clock support

one byproduct of this change is that the CAN driver no longer needs to access the SoC's clock control registers, which shall be the domain of the platform's clock driver

---
 drivers/net/can/mscan/mpc5xxx_can.c |  139 +++++++++++++++++++++++++++++++++++
 1 file changed, 139 insertions(+)

index bc422ba..dd26ab6 100644
@@ -108,6 +108,143 @@ static u32 mpc52xx_can_get_clock(struct platform_device *ofdev,
 #endif /* CONFIG_PPC_MPC52xx */
 
 #ifdef CONFIG_PPC_MPC512x
+
+#if IS_ENABLED(CONFIG_COMMON_CLK)
+
+static u32 mpc512x_can_get_clock(struct platform_device *ofdev,
+				 const char *clock_source, int *mscan_clksrc)
+{
+	struct device_node *np;
+	u32 clockdiv;
+	enum {
+		CLK_FROM_AUTO,
+		CLK_FROM_IPS,
+		CLK_FROM_SYS,
+		CLK_FROM_REF,
+	} clk_from;
+	struct clk *clk_in, *clk_can;
+	unsigned long freq_calc;
+
+	/* the caller passed in the clock source spec that was read from
+	 * the device tree, get the optional clock divider as well
+	 */
+	np = ofdev->dev.of_node;
+	clockdiv = 1;
+	of_property_read_u32(np, "fsl,mscan-clock-divider", &clockdiv);
+	dev_dbg(&ofdev->dev, "device tree specs: clk src[%s] div[%d]\n",
+		clock_source ? clock_source : "<NULL>", clockdiv);
+
+	/* when clock-source is 'ip', the CANCTL1[CLKSRC] bit needs to
+	 * get set, and the 'ips' clock is the input to the MSCAN
+	 * component
+	 *
+	 * for clock-source values of 'ref' or 'sys' the CANCTL1[CLKSRC]
+	 * bit needs to get cleared, an optional clock-divider may have
+	 * been specified (the default value is 1), the appropriate
+	 * MSCAN related MCLK is the input to the MSCAN component
+	 *
+	 * in the absence of a clock-source spec, first an optimal clock
+	 * gets determined based on the 'sys' clock, if that fails the
+	 * 'ref' clock is used
+	 */
+	clk_from = CLK_FROM_AUTO;
+	if (clock_source) {
+		/* interpret the device tree's spec for the clock source */
+		if (!strcmp(clock_source, "ip"))
+			clk_from = CLK_FROM_IPS;
+		else if (!strcmp(clock_source, "sys"))
+			clk_from = CLK_FROM_SYS;
+		else if (!strcmp(clock_source, "ref"))
+			clk_from = CLK_FROM_REF;
+		else
+			goto err_invalid;
+		dev_dbg(&ofdev->dev, "got a clk source spec[%d]\n", clk_from);
+	}
+	if (clk_from == CLK_FROM_AUTO) {
+		/* no spec so far, try the 'sys' clock; round to the
+		 * next MHz and see if we can get a multiple of 16MHz
+		 */
+		dev_dbg(&ofdev->dev, "no clk source spec, trying SYS\n");
+		clk_in = clk_get(&ofdev->dev, "sys");
+		if (IS_ERR(clk_in))
+			goto err_notavail;
+		freq_calc = clk_get_rate(clk_in);
+		freq_calc +=  499999;
+		freq_calc /= 1000000;
+		freq_calc *= 1000000;
+		if ((freq_calc % 16000000) == 0) {
+			clk_from = CLK_FROM_SYS;
+			clockdiv = freq_calc / 16000000;
+			dev_dbg(&ofdev->dev,
+				"clk fit, sys[%lu] div[%d] freq[%lu]\n",
+				freq_calc, clockdiv, freq_calc / clockdiv);
+		}
+	}
+	if (clk_from == CLK_FROM_AUTO) {
+		/* no spec so far, use the 'ref' clock */
+		dev_dbg(&ofdev->dev, "no clk source spec, trying REF\n");
+		clk_in = clk_get(&ofdev->dev, "ref");
+		if (IS_ERR(clk_in))
+			goto err_notavail;
+		clk_from = CLK_FROM_REF;
+		freq_calc = clk_get_rate(clk_in);
+		dev_dbg(&ofdev->dev,
+			"clk fit, ref[%lu] (no div) freq[%lu]\n",
+			freq_calc, freq_calc);
+	}
+
+	/* select IPS or MCLK as the MSCAN input (returned to the caller),
+	 * setup the MCLK mux source and rate if applicable, apply the
+	 * optionally specified or derived above divider, and determine
+	 * the actual resulting clock rate to return to the caller
+	 */
+	switch (clk_from) {
+	case CLK_FROM_IPS:
+		clk_can = clk_get(&ofdev->dev, "ips");
+		if (IS_ERR(clk_can))
+			goto err_notavail;
+		clk_prepare_enable(clk_can);
+		freq_calc = clk_get_rate(clk_can);
+		*mscan_clksrc = MSCAN_CLKSRC_IPS;
+		dev_dbg(&ofdev->dev, "clk from IPS, clksrc[%d] freq[%lu]\n",
+			*mscan_clksrc, freq_calc);
+		break;
+	case CLK_FROM_SYS:
+	case CLK_FROM_REF:
+		clk_can = clk_get(&ofdev->dev, "mclk");
+		if (IS_ERR(clk_can))
+			goto err_notavail;
+		clk_prepare_enable(clk_can);
+		if (clk_from == CLK_FROM_SYS)
+			clk_in = clk_get(&ofdev->dev, "sys");
+		if (clk_from == CLK_FROM_REF)
+			clk_in = clk_get(&ofdev->dev, "ref");
+		if (IS_ERR(clk_in))
+			goto err_notavail;
+		clk_set_parent(clk_can, clk_in);
+		freq_calc = clk_get_rate(clk_in);
+		freq_calc /= clockdiv;
+		clk_set_rate(clk_can, freq_calc);
+		freq_calc = clk_get_rate(clk_can);
+		*mscan_clksrc = MSCAN_CLKSRC_BUS;
+		dev_dbg(&ofdev->dev, "clk from MCLK, clksrc[%d] freq[%lu]\n",
+			*mscan_clksrc, freq_calc);
+		break;
+	default:
+		goto err_invalid;
+	}
+
+	return freq_calc;
+
+err_invalid:
+	dev_err(&ofdev->dev, "invalid clock source specification\n");
+	return 0;
+
+err_notavail:
+	dev_err(&ofdev->dev, "cannot acquire or setup clock source\n");
+	return 0;
+}
+#else	/* COMMON_CLK */
 struct mpc512x_clockctl {
 	u32 spmr;		/* System PLL Mode Reg */
 	u32 sccr[2];		/* System Clk Ctrl Reg 1 & 2 */

@@ -238,6 +375,8 @@ exit_put:
 	of_node_put(np_clock);
 	return freq;
 }
+#endif	/* COMMON_CLK */
+
 #else /* !CONFIG_PPC_MPC512x */
 static u32 mpc512x_can_get_clock(struct platform_device *ofdev,
 				 const char *clock_name, int *mscan_clksrc)

1.7.10.4

It's missing, as I could not find the counterpart of the .get_clock() callback where the get and prepare/enable was added to. And it appears that neither are enable errors checked for. This patch clearly needs an update as well, but already should be the last part of the series in a currently inacceptable state.

This .get_clock() routine is supposed to determine the clock source, setup the desired rate and return the actual rate. The callback's API is rather limited in that it does communicate clock rate values, but cannot access a location to store handles to.

Are the introduction of a .put_clock() callback and passing the 'priv' handle to both get and put routines acceptable? As this would hook up clock handling to probe() and remove() just as memory mapping gets approached.

Below is a patch on top of the CAN related patch in v2 of the series. If this approach is acceptable (maybe modulo how the "unused" warnings get silenced), I'll fold it into the introduction of common clock support in the mscan(4) driver.

index cf5e4cfc..16ca712 100644
@@ -39,7 +39,9 @@
 struct mpc5xxx_can_data {
 	unsigned int type;
 	u32 (*get_clock)(struct platform_device *ofdev, const char *clock_name,
-			 int *mscan_clksrc);
+			 int *mscan_clksrc, struct mscan_priv *priv);
+	void (*put_clock)(struct platform_device *ofdev,
+			  struct mscan_priv *priv);
 };
 
 #ifdef CONFIG_PPC_MPC52xx

@@ -49,7 +51,8 @@ static struct of_device_id mpc52xx_cdm_ids[] = {
 };
 
 static u32 mpc52xx_can_get_clock(struct platform_device *ofdev,
-				 const char *clock_name, int *mscan_clksrc)
+				 const char *clock_name, int *mscan_clksrc,
+				 struct mscan_priv *priv)
 {
 	unsigned int pvr;
 	struct mpc52xx_cdm  __iomem *cdm;

@@ -101,15 +104,24 @@ static u32 mpc52xx_can_get_clock(struct platform_device *ofdev,
 }
 #else /* !CONFIG_PPC_MPC52xx */
 static u32 mpc52xx_can_get_clock(struct platform_device *ofdev,
-				 const char *clock_name, int *mscan_clksrc)
+				 const char *clock_name, int *mscan_clksrc,
+				 struct mscan_priv *priv)
 {
 	return 0;
 }
 #endif /* CONFIG_PPC_MPC52xx */
+static void mpc52xx_can_put_clock(struct platform_device *ofdev,
+				  struct mscan_priv *priv)
+{
+	/* EMPTY */
+	(void)ofdev;
+	(void)priv;
+}
 
 #ifdef CONFIG_PPC_MPC512x
 static u32 mpc512x_can_get_clock(struct platform_device *ofdev,
-				 const char *clock_source, int *mscan_clksrc)
+				 const char *clock_source, int *mscan_clksrc,
+				 struct mscan_priv *priv)
 {
 	struct device_node *np;
 	u32 clockdiv;

@@ -200,7 +212,9 @@ static u32 mpc512x_can_get_clock(struct platform_device *ofdev,
 		clk_can = clk_get(&ofdev->dev, "ips");
 		if (IS_ERR(clk_can))
 			goto err_notavail;
-		clk_prepare_enable(clk_can);
+		if (clk_prepare_enable(clk_can))
+			goto err_notavail;
+		priv->clk_can = clk_can;
 		freq_calc = clk_get_rate(clk_can);
 		*mscan_clksrc = MSCAN_CLKSRC_IPS;
 		dev_dbg(&ofdev->dev, "clk from IPS, clksrc[%d] freq[%lu]\n",

@@ -211,7 +225,9 @@ static u32 mpc512x_can_get_clock(struct platform_device *ofdev,
 		clk_can = clk_get(&ofdev->dev, "mclk");
 		if (IS_ERR(clk_can))
 			goto err_notavail;
-		clk_prepare_enable(clk_can);
+		if (clk_prepare_enable(clk_can))
+			goto err_notavail;
+		priv->clk_can = clk_can;
 		if (clk_from == CLK_FROM_SYS)
 			clk_in = clk_get(&ofdev->dev, "sys");
 		if (clk_from == CLK_FROM_REF)

@@ -241,12 +257,31 @@ err_notavail:
 	dev_err(&ofdev->dev, "cannot acquire or setup clock source\n");
 	return 0;
 }
+
+static void mpc512x_can_put_clock(struct platform_device *ofdev,
+				  struct mscan_priv *priv)
+{
+	(void)ofdev;
+	if (priv->clk_can) {
+		clk_disable_unprepare(priv->clk_can);
+		clk_put(priv->clk_can);
+	}
+}
 #else /* !CONFIG_PPC_MPC512x */
 static u32 mpc512x_can_get_clock(struct platform_device *ofdev,
-				 const char *clock_name, int *mscan_clksrc)
+				 const char *clock_name, int *mscan_clksrc,
+				 struct mscan_priv *priv)
 {
 	return 0;
 }
+
+static void mpc512x_can_put_clock(struct platform_device *ofdev,
+				  struct mscan_priv *priv)
+{
+	(void)ofdev;
+	(void)priv;
+	/* EMPTY */
+}
 #endif /* CONFIG_PPC_MPC512x */
 
 static const struct of_device_id mpc5xxx_can_table[];

@@ -293,7 +328,7 @@ static int mpc5xxx_can_probe(struct platform_device *ofdev)
 	BUG_ON(!data);
 	priv->type = data->type;
 	priv->can.clock.freq = data->get_clock(ofdev, clock_name,
-					       &mscan_clksrc);
+					       &mscan_clksrc, priv);
 	if (!priv->can.clock.freq) {
 		dev_err(&ofdev->dev, "couldn't get MSCAN clock properties\n");
 		goto exit_free_mscan;