Skip to contentSkip to conversation

add COMMON_CLK support for PowerPC MPC512x

PATCH Superseded Gerhard Sittig 12 participants 190 messages Jul 15, 2013 – Aug 28, 2013 Page 2 of 2

Subsystems (15)

v1 of 6 Superseded Latest is v6 →

lore
Share & tools
lore
#113 lore raw

PATCH v3 18/31 dts: mpc512x: add clock specs for client lookups

In reply to Mike Turquette

[ trimming the CC: list to device tree and ARM(clock) and PPC ]

Quoted 8 lines · Mike Turquette

Quoting Gerhard Sittig (2013-07-22 05:14:45)

Quoted 6 lines

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

Quoted 6 lines
Quoted 4 lines

these specs map 'clock-names' encoded in drivers to their respective 'struct clk' items in the platform's clock driver

Thank you for the review, Mike!

Please note that there will be another update for the series. These clock specs for client side lookups will have additional 'ipg' clock items next to 'mclk' for the PSC (UART, SPI) and MSCAN (CAN) nodes. But this change is straight forward.

Quoted 303 lines
Quoted 303 lines
---
 arch/powerpc/boot/dts/mpc5121.dtsi |   79 ++++++++++++++++++++++++++++++++++++
 1 file changed, 79 insertions(+)
arch/powerpc/boot/dts/mpc5121.dtsi +79 -0
index 8f4cba0..3657ae6 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 = "per";
        };
 
        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 = "per";
                };
 
                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";
 
Quoted 4 lines
Quoted 4 lines
arch/powerpc/boot/dts/mpc5121.dtsi +0 -0
                reg = <0x80008500 0x100 /* internal registers */
                       0x80008300 0x8>; /* config space access registers */
Signature · 1 line
1.7.10.4

virtually yours
Gerhard Sittig

Signature · 3 lines
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
#114 lore raw

PATCH v3 19/31 clk: mpc512x: don't pre-enable FEC and I2C clocks

In reply to Gerhard Sittig

after the device tree nodes provide clock specs for client side lookups, peripheral drivers can attach to their clocks and the clock driver need no longer pre-enable those clock items

---
 arch/powerpc/platforms/512x/clock-commonclk.c |    2 --
 1 file changed, 2 deletions(-)
arch/powerpc/platforms/512x/clock-commonclk.c +0 -2
index 762ee85..8b71ceb 100644
@@ -713,9 +713,7 @@ static void mpc512x_clk_setup_clock_tree(int busfreq)
 	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 */
 	clk_prepare_enable(clks[MPC512x_CLK_DIU]);	/* display */
-	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
Signature · 1 line
1.7.10.4
#115 lore raw

PATCH v3 20/31 spi: mpc512x: remove now obsolete clock lookup name

In reply to Gerhard Sittig

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

---
 drivers/spi/spi-mpc512x-psc.c |    6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)
drivers/spi/spi-mpc512x-psc.c +1 -5
index 823e5e0..1a4a983 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];
 	struct clk *clk;
 
 	master = spi_alloc_master(dev, sizeof *mps);
@@ -519,9 +517,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);
-	clk = devm_clk_get(dev, clk_name);
+	clk = devm_clk_get(dev, "mclk");
 	if (IS_ERR(clk))
 		goto free_irq;
 	ret = clk_prepare_enable(clk);
Signature · 1 line
1.7.10.4
#116 lore raw

PATCH v3 21/31 serial: mpc512x: remove now obsolete clock lookup name

In reply to Gerhard Sittig

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

---
 drivers/tty/serial/mpc52xx_uart.c |    8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)
drivers/tty/serial/mpc52xx_uart.c +3 -5
index 8f16ed9..04dc564 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 = devm_clk_get(port->dev, clk_name);
+	clk = devm_clk_get(port->dev, "mclk");
 	if (IS_ERR(clk)) {
 		dev_err(port->dev, "Failed to get MCLK!\n");
 		return PTR_ERR(clk);
@@ -641,6 +638,7 @@ static int mpc512x_psc_alloc_clock(struct uart_port *port)
 		devm_clk_put(port->dev, clk);
 		return err;
 	}
+	psc_num = (port->mapbase & 0xf00) >> 8;
 	psc_mclk_clk[psc_num] = clk;
 	return 0;
 }
Signature · 1 line
1.7.10.4
#117 lore raw

PATCH v3 22/31 clk: mpc512x: remove clkdev registration (uart, spi)

In reply to Gerhard Sittig

after the UART and SPI peripheral drivers have switched to device tree based clock lookup and no longer construct clock names from their PSC component index, the "psc%d_mclk" alias names have become obsolete -- remove the corresponding clk_register_clkdev() calls

---
 arch/powerpc/platforms/512x/clock-commonclk.c |   11 -----------
 1 file changed, 11 deletions(-)
arch/powerpc/platforms/512x/clock-commonclk.c +0 -11
index 8b71ceb..a860ded 100644
@@ -529,17 +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 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)
Signature · 1 line
1.7.10.4
#118 lore raw

PATCH v3 23/31 serial: mpc512x: setup the PSC FIFO clock as well

In reply to Gerhard Sittig

prepare and enable the FIFO clock upon PSC FIFO initialization, check for and propagage errors when enabling the PSC FIFO clock, disable and unprepare the FIFO clock upon PSC FIFO uninitialization, remove the pre-enable workaround from the platform's clock driver

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

---
 arch/powerpc/platforms/512x/clock-commonclk.c |    2 --
 drivers/tty/serial/mpc52xx_uart.c             |   46 +++++++++++++++++++++----
 2 files changed, 40 insertions(+), 8 deletions(-)
arch/powerpc/platforms/512x/clock-commonclk.c +0 -2
index a860ded..53f772b 100644
@@ -698,8 +698,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_DIU]);	/* display */
drivers/tty/serial/mpc52xx_uart.c +40 -6

82 lines not on this page · raw message

Signature · 1 line
1.7.10.4
#119 lore raw

PATCH v3 24/31 USB: fsl-mph-dr-of: remove now obsolete clock lookup name

In reply to Gerhard Sittig

after device tree based clock lookup became available, the peripheral driver need no longer construct clock names which include the component index -- remove the "usb%d_clk" template and unconditionally use "per", remove the clock driver's clkdev registration

---
 arch/powerpc/platforms/512x/clock-commonclk.c |    2 --
 drivers/usb/host/fsl-mph-dr-of.c              |   13 +------------
 2 files changed, 1 insertion(+), 14 deletions(-)
arch/powerpc/platforms/512x/clock-commonclk.c +0 -2
index 53f772b..d0286a5 100644
@@ -678,8 +678,6 @@ static void mpc512x_clk_setup_clock_tree(int busfreq)
 	clk_register_clkdev(clks[MPC512x_CLK_SYS], "sys_clk", NULL);
 	clk_register_clkdev(clks[MPC512x_CLK_VIU], "viu_clk", NULL);
 	clk_register_clkdev(clks[MPC512x_CLK_NFC], "nfc_clk", NULL);
-	clk_register_clkdev(clks[MPC512x_CLK_USB1], "usb1_clk", NULL);
-	clk_register_clkdev(clks[MPC512x_CLK_USB2], "usb2_clk", NULL);
 
 	pr_debug("clock tree setup complete\n");
 	freq = clk_get_rate(clks[MPC512x_CLK_E300]);
drivers/usb/host/fsl-mph-dr-of.c +1 -12
index 5e51384..49c1da8 100644
@@ -261,19 +261,8 @@ int fsl_usb2_mpc5121_init(struct platform_device *pdev)
 	struct fsl_usb2_platform_data *pdata = pdev->dev.platform_data;
 	struct clk *clk;
 	int err;
-	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;
 
-	snprintf(clk_name, sizeof(clk_name), "usb%d_clk", clk_num);
-	clk = devm_clk_get(&pdev->dev, clk_name);
+	clk = devm_clk_get(&pdev->dev, "per");
 	if (IS_ERR(clk)) {
 		dev_err(&pdev->dev, "failed to get clk\n");
 		return PTR_ERR(clk);
Signature · 1 line
1.7.10.4
#120 lore raw

PATCH v3 25/31 mtd: mpc5121_nfc: remove now obsolete clock lookup name

In reply to Gerhard Sittig

after device tree based clock lookup became available, the NAND flash driver need no longer use the previous "nfc_clk" name but can switch to the fixed "per" clock name -- adjust the peripheral driver and remove the clock driver's clkdev registration

---
 arch/powerpc/platforms/512x/clock-commonclk.c |    1 -
 drivers/mtd/nand/mpc5121_nfc.c                |    2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)
arch/powerpc/platforms/512x/clock-commonclk.c +0 -1
index d0286a5..24e1706 100644
@@ -677,7 +677,6 @@ static void mpc512x_clk_setup_clock_tree(int busfreq)
 	clk_register_clkdev(clks[MPC512x_CLK_REF], "ref_clk", NULL);
 	clk_register_clkdev(clks[MPC512x_CLK_SYS], "sys_clk", NULL);
 	clk_register_clkdev(clks[MPC512x_CLK_VIU], "viu_clk", NULL);
-	clk_register_clkdev(clks[MPC512x_CLK_NFC], "nfc_clk", NULL);
 
 	pr_debug("clock tree setup complete\n");
 	freq = clk_get_rate(clks[MPC512x_CLK_E300]);
drivers/mtd/nand/mpc5121_nfc.c +1 -1
index 9c0b8fe..6a5f851 100644
@@ -731,7 +731,7 @@ static int mpc5121_nfc_probe(struct platform_device *op)
 	of_node_put(rootnode);
 
 	/* Enable NFC clock */
-	clk = devm_clk_get(dev, "nfc_clk");
+	clk = devm_clk_get(dev, "per");
 	if (IS_ERR(clk)) {
 		dev_err(dev, "Unable to acquire NFC clock!\n");
 		retval = PTR_ERR(clk);
Signature · 1 line
1.7.10.4
#121 lore raw

PATCH v3 26/31 fsl-viu: remove now obsolete clock lookup name

In reply to Gerhard Sittig

after device tree based clock lookup became available, the VIU driver need no longer use the previous "viu_clk" name but can switch to the fixed "per" clock name -- adjust the peripheral driver and remove the clock driver's clkdev registration

---
 arch/powerpc/platforms/512x/clock-commonclk.c |    1 -
 drivers/media/platform/fsl-viu.c              |    2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)
arch/powerpc/platforms/512x/clock-commonclk.c +0 -1
index 24e1706..f047d4c 100644
@@ -676,7 +676,6 @@ static void mpc512x_clk_setup_clock_tree(int busfreq)
 	/* clkdev registration for compatibility reasons */
 	clk_register_clkdev(clks[MPC512x_CLK_REF], "ref_clk", NULL);
 	clk_register_clkdev(clks[MPC512x_CLK_SYS], "sys_clk", NULL);
-	clk_register_clkdev(clks[MPC512x_CLK_VIU], "viu_clk", NULL);
 
 	pr_debug("clock tree setup complete\n");
 	freq = clk_get_rate(clks[MPC512x_CLK_E300]);
drivers/media/platform/fsl-viu.c +1 -1
index 48fced4..8a17433 100644
@@ -1578,7 +1578,7 @@ static int viu_of_probe(struct platform_device *op)
 	}
 
 	/* enable VIU clock */
-	clk = devm_clk_get(&op->dev, "viu_clk");
+	clk = devm_clk_get(&op->dev, "per");
 	if (IS_ERR(clk)) {
 		dev_err(&op->dev, "failed to lookup the clock!\n");
 		ret = PTR_ERR(clk);
Signature · 1 line
1.7.10.4
#122 lore raw

PATCH v3 27/31 net: can: mscan: add common clock support for mpc512x

In reply to Gerhard Sittig

implement a .get_clock() callback for the MPC512x platform which uses the common clock infrastructure (eliminating direct access to the clock control registers from within the CAN network driver), and provide the corresponding .put_clock() callback to release resources after use

keep the previous implementation of MPC512x support in place during migration, since common clock support is optional

this change is neutral to the MPC5200 platform

---
 drivers/net/can/mscan/mpc5xxx_can.c |  169 +++++++++++++++++++++++++++++++++++
 1 file changed, 169 insertions(+)
drivers/net/can/mscan/mpc5xxx_can.c +169 -0
index e59b3a3..4897929 100644
@@ -109,6 +109,167 @@ 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;
+	struct mscan_priv *priv;
+
+	/* 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 = devm_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 = devm_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 = devm_clk_get(&ofdev->dev, "ips");
+		if (IS_ERR(clk_can))
+			goto err_notavail;
+		if (clk_prepare(clk_can)) {
+			devm_clk_put(&ofdev->dev, clk_can);
+			goto err_notavail;
+		}
+		priv = netdev_priv(dev_get_drvdata(&ofdev->dev));
+		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",
+			*mscan_clksrc, freq_calc);
+		break;
+	case CLK_FROM_SYS:
+	case CLK_FROM_REF:
+		clk_can = devm_clk_get(&ofdev->dev, "mclk");
+		if (IS_ERR(clk_can))
+			goto err_notavail;
+		if (clk_prepare(clk_can)) {
+			devm_clk_put(&ofdev->dev, clk_can);
+			goto err_notavail;
+		}
+		priv = netdev_priv(dev_get_drvdata(&ofdev->dev));
+		priv->clk_can = clk_can;
+		if (clk_from == CLK_FROM_SYS)
+			clk_in = devm_clk_get(&ofdev->dev, "sys");
+		if (clk_from == CLK_FROM_REF)
+			clk_in = devm_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;
+}
+
+static void mpc512x_can_put_clock(struct platform_device *ofdev)
+{
+	struct mscan_priv *priv;
+
+	priv = netdev_priv(dev_get_drvdata(&ofdev->dev));
+	if (priv->clk_can) {
+		clk_unprepare(priv->clk_can);
+		devm_clk_put(&ofdev->dev, priv->clk_can);
+	}
+}
+
+#else	/* COMMON_CLK */
+
 struct mpc512x_clockctl {
 	u32 spmr;		/* System PLL Mode Reg */
 	u32 sccr[2];		/* System Clk Ctrl Reg 1 & 2 */
@@ -239,12 +400,18 @@ exit_put:
 	of_node_put(np_clock);
 	return freq;
 }
+
+#define mpc512x_can_put_clock NULL
+
+#endif	/* COMMON_CLK */
+
 #else /* !CONFIG_PPC_MPC512x */
 static u32 mpc512x_can_get_clock(struct platform_device *ofdev,
 				 const char *clock_name, int *mscan_clksrc)
 {
 	return 0;
 }
+#define mpc512x_can_put_clock NULL
 #endif /* CONFIG_PPC_MPC512x */
 
 static const struct of_device_id mpc5xxx_can_table[];
@@ -386,11 +553,13 @@ static int mpc5xxx_can_resume(struct platform_device *ofdev)
 static const struct mpc5xxx_can_data mpc5200_can_data = {
 	.type = MSCAN_TYPE_MPC5200,
 	.get_clock = mpc52xx_can_get_clock,
+	/* .put_clock not applicable */
 };
 
 static const struct mpc5xxx_can_data mpc5121_can_data = {
 	.type = MSCAN_TYPE_MPC5121,
 	.get_clock = mpc512x_can_get_clock,
+	.put_clock = mpc512x_can_put_clock,
 };
 
 static const struct of_device_id mpc5xxx_can_table[] = {
Signature · 1 line
1.7.10.4
#123 lore raw

PATCH v3 27/31 net: can: mscan: add common clock support for mpc512x

Quoted 110 lines · Gerhard Sittig

implement a .get_clock() callback for the MPC512x platform which uses the common clock infrastructure (eliminating direct access to the clock control registers from within the CAN network driver), and provide the corresponding .put_clock() callback to release resources after use

keep the previous implementation of MPC512x support in place during migration, since common clock support is optional

this change is neutral to the MPC5200 platform

---
 drivers/net/can/mscan/mpc5xxx_can.c |  169 +++++++++++++++++++++++++++++++++++
 1 file changed, 169 insertions(+)
drivers/net/can/mscan/mpc5xxx_can.c +87 -0
index e59b3a3..4897929 100644
@@ -109,6 +109,167 @@ 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;
+	struct mscan_priv *priv;
+
+	/* 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 = devm_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 = devm_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),

drivers/net/can/mscan/mpc5xxx_can.c static u32 mpc52xx_can_get_clock(struct platform_device *ofdev, · line 208

Quoted 10 lines
drivers/net/can/mscan/mpc5xxx_can.c +10 -0
+	 * 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 = devm_clk_get(&ofdev->dev, "ips");
+		if (IS_ERR(clk_can))
+			goto err_notavail;
+		if (clk_prepare(clk_can)) {

I would just call prepare_enable in the main mscan driver, then we don't need a special "clock is prepared but not enabled" contract.

drivers/net/can/mscan/mpc5xxx_can.c static u32 mpc52xx_can_get_clock(struct platform_device *ofdev, · line 209

Quoted 1 line
drivers/net/can/mscan/mpc5xxx_can.c +1 -0
+			devm_clk_put(&ofdev->dev, clk_can);

not needed, as this driver instance will fail, doesn't it?

Quoted 6 lines
drivers/net/can/mscan/mpc5xxx_can.c +6 -0
+			goto err_notavail;
+		}
+		priv = netdev_priv(dev_get_drvdata(&ofdev->dev));
+		priv->clk_can = clk_can;
+		freq_calc = clk_get_rate(clk_can);
+		*mscan_clksrc = MSCAN_CLKSRC_IPS;

drivers/net/can/mscan/mpc5xxx_can.c static u32 mpc52xx_can_get_clock(struct platform_device *ofdev, · line 225

Quoted 10 lines
drivers/net/can/mscan/mpc5xxx_can.c +10 -0
+		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 = devm_clk_get(&ofdev->dev, "mclk");
+		if (IS_ERR(clk_can))
+			goto err_notavail;
+		if (clk_prepare(clk_can)) {
+			devm_clk_put(&ofdev->dev, clk_can);

same here

Quoted 18 lines
drivers/net/can/mscan/mpc5xxx_can.c +18 -0
+			goto err_notavail;
+		}
+		priv = netdev_priv(dev_get_drvdata(&ofdev->dev));
+		priv->clk_can = clk_can;
+		if (clk_from == CLK_FROM_SYS)
+			clk_in = devm_clk_get(&ofdev->dev, "sys");
+		if (clk_from == CLK_FROM_REF)
+			clk_in = devm_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);

drivers/net/can/mscan/mpc5xxx_can.c static u32 mpc52xx_can_get_clock(struct platform_device *ofdev, · line 253

Quoted 10 lines
drivers/net/can/mscan/mpc5xxx_can.c +10 -0
+		break;
+	default:
+		goto err_invalid;
+	}
+
+	return freq_calc;
+
+err_invalid:
+	dev_err(&ofdev->dev, "invalid clock source specification\n");
+	return 0;

return 0 in case of error? Please add a comment what this 0 means here.

Quoted 4 lines
drivers/net/can/mscan/mpc5xxx_can.c +4 -0
+
+err_notavail:
+	dev_err(&ofdev->dev, "cannot acquire or setup clock source\n");
+	return 0;

drivers/net/can/mscan/mpc5xxx_can.c static u32 mpc52xx_can_get_clock(struct platform_device *ofdev, · line 267

Quoted 10 lines
drivers/net/can/mscan/mpc5xxx_can.c +10 -0
+}
+
+static void mpc512x_can_put_clock(struct platform_device *ofdev)
+{
+	struct mscan_priv *priv;
+
+	priv = netdev_priv(dev_get_drvdata(&ofdev->dev));
+	if (priv->clk_can) {
+		clk_unprepare(priv->clk_can);
+		devm_clk_put(&ofdev->dev, priv->clk_can);

devm_clk_put can be removed, it's called automatically.

Quoted 32 lines
drivers/net/can/mscan/mpc5xxx_can.c +11 -0
+	}
+}
+
+#else	/* COMMON_CLK */
+
 struct mpc512x_clockctl {
 	u32 spmr;		/* System PLL Mode Reg */
 	u32 sccr[2];		/* System Clk Ctrl Reg 1 & 2 */
@@ -239,12 +400,18 @@ exit_put:
 	of_node_put(np_clock);
 	return freq;
 }
+
+#define mpc512x_can_put_clock NULL
+
+#endif	/* COMMON_CLK */
+
 #else /* !CONFIG_PPC_MPC512x */
 static u32 mpc512x_can_get_clock(struct platform_device *ofdev,
 				 const char *clock_name, int *mscan_clksrc)
 {
 	return 0;
 }
+#define mpc512x_can_put_clock NULL
 #endif /* CONFIG_PPC_MPC512x */
 
 static const struct of_device_id mpc5xxx_can_table[];
@@ -386,11 +553,13 @@ static int mpc5xxx_can_resume(struct platform_device *ofdev)
 static const struct mpc5xxx_can_data mpc5200_can_data = {
 	.type = MSCAN_TYPE_MPC5200,
 	.get_clock = mpc52xx_can_get_clock,

drivers/net/can/mscan/mpc5xxx_can.c static int mpc5xxx_can_resume(struct platform_device *ofdev) · line 565

Quoted 10 lines
drivers/net/can/mscan/mpc5xxx_can.c +2 -0
+	/* .put_clock not applicable */
 };
 
 static const struct mpc5xxx_can_data mpc5121_can_data = {
 	.type = MSCAN_TYPE_MPC5121,
 	.get_clock = mpc512x_can_get_clock,
+	.put_clock = mpc512x_can_put_clock,
 };
 
 static const struct of_device_id mpc5xxx_can_table[] = {

Marc

Signature · 12 lines
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 259 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130722/62e0a5f8/attachment-0001.sig>
#124 lore raw

PATCH v3 27/31 net: can: mscan: add common clock support for mpc512x

[ adjusted devtree to use the vger address ]

Quoted 117 lines · Marc Kleine-Budde
Quoted 115 lines · Gerhard Sittig

implement a .get_clock() callback for the MPC512x platform which uses the common clock infrastructure (eliminating direct access to the clock control registers from within the CAN network driver), and provide the corresponding .put_clock() callback to release resources after use

keep the previous implementation of MPC512x support in place during migration, since common clock support is optional

this change is neutral to the MPC5200 platform

---
 drivers/net/can/mscan/mpc5xxx_can.c |  169 +++++++++++++++++++++++++++++++++++
 1 file changed, 169 insertions(+)
drivers/net/can/mscan/mpc5xxx_can.c +92 -0
index e59b3a3..4897929 100644
@@ -109,6 +109,167 @@ 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;
+	struct mscan_priv *priv;
+
+	/* 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 = devm_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 = devm_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) {
Quoted 8 lines

drivers/net/can/mscan/mpc5xxx_can.c static u32 mpc52xx_can_get_clock(struct platform_device *ofdev, · line 208

Quoted 5 lines
drivers/net/can/mscan/mpc5xxx_can.c +5 -0
+	case CLK_FROM_IPS:
+		clk_can = devm_clk_get(&ofdev->dev, "ips");
+		if (IS_ERR(clk_can))
+			goto err_notavail;
+		if (clk_prepare(clk_can)) {

I would just call prepare_enable in the main mscan driver, then we don't need a special "clock is prepared but not enabled" contract.

Yes, I addressed this concern (although differently after learning more from a local test setup), see the other reply for the CAN driver adjustment for clock API use.

Quoted 4 lines

drivers/net/can/mscan/mpc5xxx_can.c static u32 mpc52xx_can_get_clock(struct platform_device *ofdev, · line 209

Quoted 2 lines
drivers/net/can/mscan/mpc5xxx_can.c +1 -0
+			devm_clk_put(&ofdev->dev, clk_can);

not needed, as this driver instance will fail, doesn't it?

This is discussed in the other subthread as well. The only remaining question is how many of them to remove, before I'll update the series.

Quoted 41 lines
Quoted 6 lines
drivers/net/can/mscan/mpc5xxx_can.c +6 -0
+			goto err_notavail;
+		}
+		priv = netdev_priv(dev_get_drvdata(&ofdev->dev));
+		priv->clk_can = clk_can;
+		freq_calc = clk_get_rate(clk_can);
+		*mscan_clksrc = MSCAN_CLKSRC_IPS;

drivers/net/can/mscan/mpc5xxx_can.c static u32 mpc52xx_can_get_clock(struct platform_device *ofdev, · line 225

Quoted 10 lines
drivers/net/can/mscan/mpc5xxx_can.c +10 -0
+		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 = devm_clk_get(&ofdev->dev, "mclk");
+		if (IS_ERR(clk_can))
+			goto err_notavail;
+		if (clk_prepare(clk_can)) {
+			devm_clk_put(&ofdev->dev, clk_can);

same here

Quoted 22 lines
drivers/net/can/mscan/mpc5xxx_can.c +22 -0
+			goto err_notavail;
+		}
+		priv = netdev_priv(dev_get_drvdata(&ofdev->dev));
+		priv->clk_can = clk_can;
+		if (clk_from == CLK_FROM_SYS)
+			clk_in = devm_clk_get(&ofdev->dev, "sys");
+		if (clk_from == CLK_FROM_REF)
+			clk_in = devm_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;
+	}
Quoted 8 lines

drivers/net/can/mscan/mpc5xxx_can.c static u32 mpc52xx_can_get_clock(struct platform_device *ofdev, · line 253

Quoted 6 lines
drivers/net/can/mscan/mpc5xxx_can.c +6 -0
+
+	return freq_calc;
+
+err_invalid:
+	dev_err(&ofdev->dev, "invalid clock source specification\n");
+	return 0;

return 0 in case of error? Please add a comment what this 0 means here.

The .get_clock() callback is supposed to return the resulting rate after the clock source was determined and the clock subtree was setup. It wasn't (explicitly) documented before (in the non-common-clock case), so I did not bother to comment it in the parallel common-clock case.

But it's true that returning zero in the error case may be unexpected, and I will add a comment in v4 (in all the .get_clock() implementations).

Quoted 57 lines
Quoted 4 lines
drivers/net/can/mscan/mpc5xxx_can.c +4 -0
+
+err_notavail:
+	dev_err(&ofdev->dev, "cannot acquire or setup clock source\n");
+	return 0;

drivers/net/can/mscan/mpc5xxx_can.c static u32 mpc52xx_can_get_clock(struct platform_device *ofdev, · line 267

Quoted 10 lines
drivers/net/can/mscan/mpc5xxx_can.c +10 -0
+}
+
+static void mpc512x_can_put_clock(struct platform_device *ofdev)
+{
+	struct mscan_priv *priv;
+
+	priv = netdev_priv(dev_get_drvdata(&ofdev->dev));
+	if (priv->clk_can) {
+		clk_unprepare(priv->clk_can);
+		devm_clk_put(&ofdev->dev, priv->clk_can);

devm_clk_put can be removed, it's called automatically.

Quoted 40 lines
drivers/net/can/mscan/mpc5xxx_can.c +13 -0
+	}
+}
+
+#else	/* COMMON_CLK */
+
 struct mpc512x_clockctl {
 	u32 spmr;		/* System PLL Mode Reg */
 	u32 sccr[2];		/* System Clk Ctrl Reg 1 & 2 */
@@ -239,12 +400,18 @@ exit_put:
 	of_node_put(np_clock);
 	return freq;
 }
+
+#define mpc512x_can_put_clock NULL
+
+#endif	/* COMMON_CLK */
+
 #else /* !CONFIG_PPC_MPC512x */
 static u32 mpc512x_can_get_clock(struct platform_device *ofdev,
 				 const char *clock_name, int *mscan_clksrc)
 {
 	return 0;
 }
+#define mpc512x_can_put_clock NULL
 #endif /* CONFIG_PPC_MPC512x */
 
 static const struct of_device_id mpc5xxx_can_table[];
@@ -386,11 +553,13 @@ static int mpc5xxx_can_resume(struct platform_device *ofdev)
 static const struct mpc5xxx_can_data mpc5200_can_data = {
 	.type = MSCAN_TYPE_MPC5200,
 	.get_clock = mpc52xx_can_get_clock,
+	/* .put_clock not applicable */
 };
 
 static const struct mpc5xxx_can_data mpc5121_can_data = {
 	.type = MSCAN_TYPE_MPC5121,
 	.get_clock = mpc512x_can_get_clock,
+	.put_clock = mpc512x_can_put_clock,
 };
 
Quoted 4 lines

drivers/net/can/mscan/mpc5xxx_can.c static int mpc5xxx_can_resume(struct platform_device *ofdev) · line 565

Quoted 2 lines
drivers/net/can/mscan/mpc5xxx_can.c +0 -0
 static const struct of_device_id mpc5xxx_can_table[] = {

Marc

Thank you for the fast and detailled review. It's very much appreciated.

virtually yours
Gerhard Sittig

Signature · 3 lines
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
#125 lore raw

PATCH v3 27/31 net: can: mscan: add common clock support for mpc512x

On 07/23/2013 02:07 PM, Gerhard Sittig wrote:
[...]

Quoted 8 lines
Quoted 7 lines

drivers/net/can/mscan/mpc5xxx_can.c static u32 mpc52xx_can_get_clock(struct platform_device *ofdev, · line 253

Quoted 5 lines
drivers/net/can/mscan/mpc5xxx_can.c +5 -0
+	return freq_calc;
+
+err_invalid:
+	dev_err(&ofdev->dev, "invalid clock source specification\n");
+	return 0;

return 0 in case of error? Please add a comment what this 0 means here.

Quoted 5 lines

The .get_clock() callback is supposed to return the resulting rate after the clock source was determined and the clock subtree was setup. It wasn't (explicitly) documented before (in the non-common-clock case), so I did not bother to comment it in the parallel common-clock case.

Yes, but returning 0 in case of error should be documented, as it's very uncommon.

Quoted 3 lines

But it's true that returning zero in the error case may be unexpected, and I will add a comment in v4 (in all the .get_clock() implementations).

I think a one liner stating that the return value is the clock rate and "0" indicates is an invalid clock rate, this meaning an error here, is enough.

Marc

Signature · 12 lines
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 259 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130723/e4b5a260/attachment.sig>
#126 lore raw

PATCH v3 28/31 powerpc/mpc512x: improve DIU related clock setup

In reply to Gerhard Sittig

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(-)
arch/powerpc/platforms/512x/clock-commonclk.c +0 -1
index f047d4c..893fbe5 100644
@@ -696,7 +696,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_DIU]);	/* display */
 	/*
 	 * some have their individual clock subtree with separate clock
 	 * items and their individual enable counters, yet share a
arch/powerpc/platforms/512x/mpc512x_shared.c +88 -77

192 lines not on this page · raw message

Signature · 1 line
1.7.10.4
#127 lore raw

PATCH v3 29/31 clk: mpc512x: switch to COMMON_CLK, remove PPC_CLOCK

In reply to Gerhard Sittig

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
arch/powerpc/platforms/512x/Kconfig +1 -13
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
arch/powerpc/platforms/512x/Makefile +1 -2
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
arch/powerpc/platforms/512x/clock.c deleted +0 -753

753 lines not on this page · raw message

Signature · 1 line
1.7.10.4
#128 lore raw

PATCH v3 30/31 net: can: mscan: remove non-common_clock code for MPC512x

In reply to Gerhard Sittig

transition to the common clock framework has completed and the PPC_CLOCK is no longer available 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 |  141 -----------------------------------
 1 file changed, 141 deletions(-)
drivers/net/can/mscan/mpc5xxx_can.c +0 -141
index 4897929..dd6bdaa 100644
@@ -109,9 +109,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)
 {
@@ -267,144 +264,6 @@ static void mpc512x_can_put_clock(struct platform_device *ofdev)
 		devm_clk_put(&ofdev->dev, priv->clk_can);
 	}
 }
-
-#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 = devm_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 = devm_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;
-}
-
-#define mpc512x_can_put_clock NULL
-
-#endif	/* COMMON_CLK */
-
 #else /* !CONFIG_PPC_MPC512x */
 static u32 mpc512x_can_get_clock(struct platform_device *ofdev,
 				 const char *clock_name, int *mscan_clksrc)
Signature · 1 line
1.7.10.4
#129 lore raw

PATCH v3 31/31 clk: mpc512x: remove clkdev registration (sys/ref, header)

In reply to Gerhard Sittig

remove the last clkdev registration call ("sys_clk" and "ref_clk" for mscan), as well as the clkdev header inclusion and the "compat registration" comment

all client lookups for clock items are device tree based now, no compatibility alias names need to get provided any longer

---
 arch/powerpc/platforms/512x/clock-commonclk.c |    5 -----
 1 file changed, 5 deletions(-)
arch/powerpc/platforms/512x/clock-commonclk.c +0 -5
index 893fbe5..e9451a7 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>
@@ -673,10 +672,6 @@ static void mpc512x_clk_setup_clock_tree(int busfreq)
 	/* fixed frequency for AC97, always 24.567MHz */
 	clks[MPC512x_CLK_AC97] = mpc512x_clk_fixed("ac97", 24567000);
 
-	/* clkdev registration for compatibility reasons */
-	clk_register_clkdev(clks[MPC512x_CLK_REF], "ref_clk", NULL);
-	clk_register_clkdev(clks[MPC512x_CLK_SYS], "sys_clk", NULL);
-
 	pr_debug("clock tree setup complete\n");
 	freq = clk_get_rate(clks[MPC512x_CLK_E300]);
 	pr_debug("derived PPC freq [%d]\n", freq);
Signature · 1 line
1.7.10.4
#130 lore raw

v3 cover add COMMON_CLK support for PowerPC MPC512x

In reply to Gerhard Sittig

[ manually added devicetree at vger now ]

Quoted 9 lines · Gerhard Sittig

this series
- fixes several drivers that are used in the MPC512x platform (UART, SPI, ethernet, PCI, USB, CAN, NAND flash, video capture) in how they handle clocks (appropriately acquire and setup them, hold references during use, release clocks after use)
- introduces support for the common clock framework (CCF, COMMON_CLK Kconfig option) in the PowerPC based MPC512x platform, which brings device tree based clock lookup as well

Haven't noticed before that the ozlabs device tree list started actively rejecting reception. And the scope of the series has widened to include clock related fixes in many more drivers while the initial submission only introduced CCF for MPC512x.

Shall I resend (with vger devicetree included and more subsystems lists added)? Which other lists to include without spamming too many channels? Poor moderators have to ACK messages for lists that I'm not subscribed to.

virtually yours
Gerhard Sittig

Signature · 3 lines
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
#131 lore raw

v4 cover add COMMON_CLK support for PowerPC MPC512x

In reply to Gerhard Sittig

this series
- fixes several drivers that are used in the MPC512x platform (UART, SPI, ethernet, PCI, USB, CAN, NAND flash, video capture) in how they handle clocks (appropriately acquire and setup them, hold references during use, release clocks after use)
- introduces support for the common clock framework (CCF, COMMON_CLK Kconfig option) in the PowerPC based MPC512x platform, which brings device tree based clock lookup as well

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

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 #431: FILE: arch/powerpc/platforms/512x/clock-commonclk.c:334:
+static const char *parent_names_mux0[] = {

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

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

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

Gerhard Sittig (31):
spi: mpc512x: cleanup clock API use
serial: mpc512x: cleanup clock API use
USB: fsl-mph-dr-of: cleanup clock API use
mtd: mpc5121_nfc: cleanup clock API use
[media] fsl-viu: cleanup clock API use
i2c: mpc: cleanup clock API use
fs_enet: silence a build warning (unused variable)
fs_enet: cleanup clock API use
powerpc/fsl-pci: improve clock API use
net: can: mscan: add a comment on reg to idx mapping
net: can: mscan: improve clock API use
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
clk: mpc512x: don't pre-enable FEC and I2C clocks
spi: mpc512x: switch to CCF names in clock lookup
serial: mpc512x: switch to CCF names in clock lookup
clk: mpc512x: remove PSC (UART, SPI) compat workarounds
serial: mpc512x: setup the PSC FIFO clock as well
USB: fsl-mph-dr-of: remove now obsolete clock lookup name
mtd: mpc5121_nfc: remove now obsolete clock lookup name [media] fsl-viu: remove now obsolete clock lookup name
net: can: mscan: add common clock support for mpc512x powerpc/mpc512x: improve DIU related clock setup
clk: mpc512x: switch to COMMON_CLK, remove PPC_CLOCK
net: can: mscan: remove non-CCF code for MPC512x
clk: mpc512x: remove remaining migration workarounds

 arch/powerpc/boot/dts/ac14xx.dts                   |    9 +-
 arch/powerpc/boot/dts/include/dt-bindings          |    1 +
 arch/powerpc/boot/dts/mpc5121.dtsi                 |  110 ++-
 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      |  716 +++++++++++++++++++
 arch/powerpc/platforms/512x/clock.c                |  753 --------------------
 arch/powerpc/platforms/512x/mpc512x_shared.c       |  165 +++--
 arch/powerpc/sysdev/fsl_pci.c                      |   22 +
 drivers/clk/clk-divider.c                          |    6 +-
 drivers/clk/clk-gate.c                             |    6 +-
 drivers/clk/clk-mux.c                              |    6 +-
 drivers/i2c/busses/i2c-mpc.c                       |   24 +
 drivers/media/platform/fsl-viu.c                   |   23 +-
 drivers/mtd/nand/mpc5121_nfc.c                     |   21 +-
 drivers/net/can/mscan/mpc5xxx_can.c                |  281 +++++---
 drivers/net/can/mscan/mscan.c                      |   27 +-
 drivers/net/can/mscan/mscan.h                      |    3 +
 .../net/ethernet/freescale/fs_enet/fs_enet-main.c  |   21 +-
 drivers/spi/spi-mpc512x-psc.c                      |   60 +-
 drivers/tty/serial/mpc52xx_uart.c                  |  179 ++++-
 drivers/usb/host/fsl-mph-dr-of.c                   |   27 +-
 include/dt-bindings/clock/mpc512x-clock.h          |   69 ++
 include/linux/clk-provider.h                       |   33 +
 include/linux/fs_enet_pd.h                         |    3 +
 28 files changed, 1534 insertions(+), 1058 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-rc4-00031-gb38a1de 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                      6           6            320000000
          spdif-mux0            0           0            320000000
             spdif-en0          0           0            320000000
                spdif_mclk_div  0           0            80000000
                   spdif_mclk   0           0            80000000
          mscan3-mux0           0           0            320000000
             mscan3-en0         0           0            320000000
                mscan3_mclk_div 0           0            80000000
                   mscan3_mclk  0           0            80000000
          mscan2-mux0           0           0            320000000
             mscan2-en0         0           0            320000000
                mscan2_mclk_div 0           0            80000000
                   mscan2_mclk  0           0            80000000
          mscan1-mux0           0           0            320000000
             mscan1-en0         0           0            320000000
                mscan1_mclk_div 0           0            16000000
                   mscan1_mclk  0           0            16000000
          mscan0-mux0           0           0            320000000
             mscan0-en0         0           0            320000000
                mscan0_mclk_div 0           0            16000000
                   mscan0_mclk  0           0            16000000
          psc11-mux0            0           0            320000000
             psc11-en0          0           0            320000000
                psc11_mclk_div  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   0           0            80000000
          psc9-mux0             0           0            320000000
             psc9-en0           0           0            320000000
                psc9_mclk_div   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    0           0            80000000
          psc7-mux0             1           1            320000000
             psc7-en0           1           1            320000000
                psc7_mclk_div   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    0           0            80000000
          psc5-mux0             1           1            320000000
             psc5-en0           1           1            320000000
                psc5_mclk_div   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    1           1            80000000
          psc3-mux0             1           1            320000000
             psc3-en0           1           1            320000000
                psc3_mclk_div   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    0           0            80000000
          psc1-mux0             0           0            320000000
             psc1-en0           0           0            320000000
                psc1_mclk_div   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    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                12          12           80000000
                spdif           0           0            80000000
                bdlc            2           2            80000000
                i2c             3           3            80000000
                mem             1           1            80000000
                fec             1           1            80000000
                sata            0           0            80000000
                psc-fifo        1           1            80000000
                psc11           0           0            80000000
                psc10           0           0            80000000
                psc9            0           0            80000000
                psc8            0           0            80000000
                psc7            1           1            80000000
                psc6            0           0            80000000
                psc5            1           1            80000000
                psc4            1           1            80000000
                psc3            1           1            80000000
                psc2            0           0            80000000
                psc1            0           0            80000000
                psc0            0           0            80000000
                pata            0           0            80000000
                lpc-ug          1           1            40000000
                   lpc          1           1            40000000
                nfc-ug          0           0            40000000
                   nfc          0           0            40000000
Signature · 1 line
1.7.10.4
#132 lore raw

PATCH v4 1/31 spi: mpc512x: cleanup clock API use

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

---
 drivers/spi/spi-mpc512x-psc.c |   48 +++++++++++++++++++++++++----------------
 1 file changed, 30 insertions(+), 18 deletions(-)
drivers/spi/spi-mpc512x-psc.c +30 -18
index 29fce6a..85581f3 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,9 @@ 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];
+	struct clk *clk;
 
 	master = spi_alloc_master(dev, sizeof *mps);
 	if (master == NULL)
@@ -521,16 +519,29 @@ 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);
+	clk = devm_clk_get(dev, clk_name);
+	if (IS_ERR(clk))
+		goto free_irq;
+	ret = clk_prepare_enable(clk);
+	if (ret)
+		goto free_irq;
+	mps->clk_mclk = clk;
+	mps->mclk_rate = clk_get_rate(clk);
+
 	ret = mpc512x_psc_spi_port_config(master, mps);
 	if (ret < 0)
-		goto free_irq;
+		goto free_clock;
 
 	ret = spi_register_master(master);
 	if (ret < 0)
-		goto free_irq;
+		goto free_clock;
 
 	return ret;
 
+free_clock:
+	clk_disable_unprepare(mps->clk_mclk);
 free_irq:
 	free_irq(mps->irq, mps);
 free_master:
@@ -547,6 +558,7 @@ 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);
 	free_irq(mps->irq, mps);
 	if (mps->psc)
 		iounmap(mps->psc);
Signature · 1 line
1.7.10.4
#133 lore raw

Re: [PATCH v4 01/31] spi: mpc512x: cleanup clock API use

Quoted 11 lines · On Tue, 6 Aug 2013 22:43:41 +0200 Gerhard Sittig <[email protected]> wrote:

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

Quoted 4 lines
---
 drivers/spi/spi-mpc512x-psc.c |   48 +++++++++++++++++++++++++----------------
 1 file changed, 30 insertions(+), 18 deletions(-)

Mark, are you going to apply this patch? Or should I queue it in my mpc5xxx tree (I'd like to get your Acked-by then)?

Thanks,

Anatolij

maintainer
#134 lore raw

Re: [PATCH v4 01/31] spi: mpc512x: cleanup clock API use

Quoted 2 lines · Anatolij Gustschin

Mark, are you going to apply this patch? Or should I queue it in my mpc5xxx tree (I'd like to get your Acked-by then)?

Has this series settled down? I'd been ignoring it since it was getting so many and so frequent revisions.

#135 lore raw

Re: [PATCH v4 01/31] spi: mpc512x: cleanup clock API use

Quoted 7 lines · On Wed, 21 Aug 2013 20:48:17 +0100 Mark Brown <[email protected]> wrote:
Quoted 2 lines · Anatolij Gustschin

Mark, are you going to apply this patch? Or should I queue it in my mpc5xxx tree (I'd like to get your Acked-by then)?

Has this series settled down? I'd been ignoring it since it was getting so many and so frequent revisions.

Patches 01 - 14 (except 09 and 11) won't change I think. I'd prefer to queue them for v3.12, so there will be no need to resubmit again and again. Other patches are not ready yet.

Anatolij

maintainer
#136 lore raw

PATCH v4 1/31 spi: mpc512x: cleanup clock API use

In reply to Gerhard Sittig
Quoted 4 lines · Gerhard Sittig

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

Applied, thanks.
-------------- 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/20130822/02ff2956/attachment.sig>

#137 lore raw

PATCH v4 2/31 serial: mpc512x: cleanup clock API use

In reply to Gerhard Sittig

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

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 |   98 ++++++++++++++++++++++++++++++-------
 1 file changed, 81 insertions(+), 17 deletions(-)
drivers/tty/serial/mpc52xx_uart.c +81 -17
index e1280a2..5be1df3 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,73 @@ 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 = devm_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");
+		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);
+		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 +917,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 +952,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 +1214,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 +1241,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;
 }
 
Signature · 1 line
1.7.10.4
#138 lore raw

PATCH v4 2/31 serial: mpc512x: cleanup clock API use

Quoted 27 lines · Gerhard Sittig

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

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

Quoted 1 line
maintainer Applied
#139 lore raw

Re: [PATCH v4 02/31] serial: mpc512x: cleanup clock API use

In reply to Gerhard Sittig
Quoted 27 lines · On Tue, 6 Aug 2013 22:43:42 +0200 Gerhard Sittig <[email protected]> wrote:

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

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

Quoted 4 lines
---
 drivers/tty/serial/mpc52xx_uart.c |   98 ++++++++++++++++++++++++++++++-------
 1 file changed, 81 insertions(+), 17 deletions(-)

applied, thanks!

Anatolij

#140 lore raw

PATCH v4 3/31 USB: fsl-mph-dr-of: cleanup clock API use

In reply to Gerhard Sittig

use devm_get_clk() for automatic put upon device close, check for and propagate errors when enabling clocks, must prepare clocks before they can get enabled, unprepare after disable

---
 drivers/usb/host/fsl-mph-dr-of.c |   16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)
drivers/usb/host/fsl-mph-dr-of.c +9 -7
index 11e0b79..b8a1866 100644
@@ -260,6 +260,7 @@ int fsl_usb2_mpc5121_init(struct platform_device *pdev)
 {
 	struct fsl_usb2_platform_data *pdata = pdev->dev.platform_data;
 	struct clk *clk;
+	int err;
 	char clk_name[10];
 	int base, clk_num;
 
@@ -272,13 +273,16 @@ int fsl_usb2_mpc5121_init(struct platform_device *pdev)
 		return -ENODEV;
 
 	snprintf(clk_name, sizeof(clk_name), "usb%d_clk", clk_num);
-	clk = clk_get(&pdev->dev, clk_name);
+	clk = devm_clk_get(&pdev->dev, clk_name);
 	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");
+		return err;
+	}
 	pdata->clk = clk;
 
 	if (pdata->phy_mode == FSL_USB2_PHY_UTMI_WIDE) {
@@ -302,10 +306,8 @@ static void fsl_usb2_mpc5121_exit(struct platform_device *pdev)
 
 	pdata->regs = NULL;
 
-	if (pdata->clk) {
-		clk_disable(pdata->clk);
-		clk_put(pdata->clk);
-	}
+	if (pdata->clk)
+		clk_disable_unprepare(pdata->clk);
 }
 
 static struct fsl_usb2_platform_data fsl_usb2_mpc5121_pd = {
Signature · 1 line
1.7.10.4
maintainer Applied
#141 lore raw

Re: [PATCH v4 03/31] USB: fsl-mph-dr-of: cleanup clock API use

Quoted 8 lines · On Tue, 6 Aug 2013 22:43:43 +0200 Gerhard Sittig <[email protected]> wrote:

use devm_get_clk() for automatic put upon device close, check for and propagate errors when enabling clocks, must prepare clocks before they can get enabled, unprepare after disable

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

applied, thanks!

Anatolij

#142 lore raw

PATCH v4 4/31 mtd: mpc5121_nfc: cleanup clock API use

In reply to Gerhard Sittig

use devm_clk_get() for automatic put after device close, check for and propagate errors when enabling clocks, need to prepare clocks before they can get enabled, adjust error code paths to correctly balance get/put and prepare/unprepare and enable/disable calls

---
 drivers/mtd/nand/mpc5121_nfc.c |   21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)
drivers/mtd/nand/mpc5121_nfc.c +12 -9
index 3c9cdcb..3c60a00 100644
@@ -617,10 +617,8 @@ static void mpc5121_nfc_free(struct device *dev, struct mtd_info *mtd)
 	struct nand_chip *chip = mtd->priv;
 	struct mpc5121_nfc_prv *prv = chip->priv;
 
-	if (prv->clk) {
-		clk_disable(prv->clk);
-		clk_put(prv->clk);
-	}
+	if (prv->clk)
+		clk_disable_unprepare(prv->clk);
 
 	if (prv->csreg)
 		iounmap(prv->csreg);
@@ -629,6 +627,7 @@ static void mpc5121_nfc_free(struct device *dev, struct mtd_info *mtd)
 static int mpc5121_nfc_probe(struct platform_device *op)
 {
 	struct device_node *rootnode, *dn = op->dev.of_node;
+	struct clk *clk;
 	struct device *dev = &op->dev;
 	struct mpc5121_nfc_prv *prv;
 	struct resource res;
@@ -730,14 +729,18 @@ static int mpc5121_nfc_probe(struct platform_device *op)
 	of_node_put(rootnode);
 
 	/* Enable NFC clock */
-	prv->clk = clk_get(dev, "nfc_clk");
-	if (IS_ERR(prv->clk)) {
+	clk = devm_clk_get(dev, "nfc_clk");
+	if (IS_ERR(clk)) {
 		dev_err(dev, "Unable to acquire NFC clock!\n");
-		retval = PTR_ERR(prv->clk);
+		retval = PTR_ERR(clk);
 		goto error;
 	}
-
-	clk_enable(prv->clk);
+	retval = clk_prepare_enable(clk);
+	if (retval) {
+		dev_err(dev, "Unable to enable NFC clock!\n");
+		goto error;
+	}
+	prv->clk = clk;
 
 	/* Reset NAND Flash controller */
 	nfc_set(mtd, NFC_CONFIG1, NFC_RESET);
Signature · 1 line
1.7.10.4
maintainer Applied
#143 lore raw

Re: [PATCH v4 04/31] mtd: mpc5121_nfc: cleanup clock API use

Quoted 9 lines · On Tue, 6 Aug 2013 22:43:44 +0200 Gerhard Sittig <[email protected]> wrote:

use devm_clk_get() for automatic put after device close, check for and propagate errors when enabling clocks, need to prepare clocks before they can get enabled, adjust error code paths to correctly balance get/put and prepare/unprepare and enable/disable calls

---
 drivers/mtd/nand/mpc5121_nfc.c |   21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

Applied to mpc5xxx tree, thanks!

Anatolij

#144 lore raw

PATCH v4 5/31 fsl-viu: cleanup clock API use

In reply to Gerhard Sittig

use devm_clk_get() for automatic put after device close, check for and propagate errors when enabling clocks, need to prepare clocks before they can get enabled, adjust code paths to correctly balance get/put and prepare/unprepare and enable/disable calls

---
 drivers/media/platform/fsl-viu.c |   23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)
drivers/media/platform/fsl-viu.c +13 -10
index 221ec42..fe9898c 100644
@@ -1485,6 +1485,7 @@ static int viu_of_probe(struct platform_device *op)
 	struct viu_reg __iomem *viu_regs;
 	struct i2c_adapter *ad;
 	int ret, viu_irq;
+	struct clk *clk;
 
 	ret = of_address_to_resource(op->dev.of_node, 0, &r);
 	if (ret) {
@@ -1577,14 +1578,18 @@ static int viu_of_probe(struct platform_device *op)
 	}
 
 	/* enable VIU clock */
-	viu_dev->clk = clk_get(&op->dev, "viu_clk");
-	if (IS_ERR(viu_dev->clk)) {
-		dev_err(&op->dev, "failed to find the clock module!\n");
-		ret = -ENODEV;
+	clk = devm_clk_get(&op->dev, "viu_clk");
+	if (IS_ERR(clk)) {
+		dev_err(&op->dev, "failed to lookup the clock!\n");
+		ret = PTR_ERR(clk);
+		goto err_clk;
+	}
+	ret = clk_prepare_enable(clk);
+	if (ret) {
+		dev_err(&op->dev, "failed to enable the clock!\n");
 		goto err_clk;
-	} else {
-		clk_enable(viu_dev->clk);
 	}
+	viu_dev->clk = clk;
 
 	/* reset VIU module */
 	viu_reset(viu_dev->vr);
@@ -1602,8 +1607,7 @@ static int viu_of_probe(struct platform_device *op)
 	return ret;
 
 err_irq:
-	clk_disable(viu_dev->clk);
-	clk_put(viu_dev->clk);
+	clk_disable_unprepare(viu_dev->clk);
 err_clk:
 	video_unregister_device(viu_dev->vdev);
 err_vdev:
@@ -1626,8 +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_put(dev->clk);
+	clk_disable_unprepare(dev->clk);
 
 	video_unregister_device(dev->vdev);
 	i2c_put_adapter(client->adapter);
Signature · 1 line
1.7.10.4
maintainer Applied
#145 lore raw

Re: [PATCH v4 05/31] [media] fsl-viu: cleanup clock API use

Quoted 9 lines · On Tue, 6 Aug 2013 22:43:45 +0200 Gerhard Sittig <[email protected]> wrote:

use devm_clk_get() for automatic put after device close, check for and propagate errors when enabling clocks, need to prepare clocks before they can get enabled, adjust code paths to correctly balance get/put and prepare/unprepare and enable/disable calls

---
 drivers/media/platform/fsl-viu.c |   23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

Applied to mpc5xxx tree, thanks!

Anatolij

#146 lore raw

PATCH v4 6/31 i2c: mpc: cleanup clock API use

In reply to Gerhard Sittig

make the MPC I2C driver get, prepare and enable the peripheral clock during probe ('per' for access to the peripheral's registers); disable and unprepare the clock upon remove(), put is done by the devm approach; hold a reference to the clock over the period of use

clock lookup is non-fatal in this implementation as not all platforms may provide clock specs in their device tree, but enable errors for specified clocks are considered fatal

---
 drivers/i2c/busses/i2c-mpc.c |   24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)
drivers/i2c/busses/i2c-mpc.c +24 -0
index 7607dc0..4b00dd0 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>
@@ -67,6 +68,7 @@ struct mpc_i2c {
 #ifdef CONFIG_PM
 	u8 fdr, dfsrr;
 #endif
+	struct clk *clk_per;
 };
 
 struct mpc_i2c_divider {
@@ -623,6 +625,8 @@ static int fsl_i2c_probe(struct platform_device *op)
 	u32 clock = MPC_I2C_CLOCK_LEGACY;
 	int result = 0;
 	int plen;
+	struct clk *clk;
+	int err;
 
 	match = of_match_device(mpc_i2c_of_match, &op->dev);
 	if (!match)
@@ -653,6 +657,21 @@ static int fsl_i2c_probe(struct platform_device *op)
 		}
 	}
 
+	/*
+	 * enable clock for the I2C peripheral (non fatal),
+	 * keep a reference upon successful allocation
+	 */
+	clk = devm_clk_get(&op->dev, "per");
+	if (!IS_ERR(clk)) {
+		err = clk_prepare_enable(clk);
+		if (err) {
+			dev_err(&op->dev, "failed to enable clock\n");
+			goto fail_request;
+		} else {
+			i2c->clk_per = clk;
+		}
+	}
+
 	if (of_get_property(op->dev.of_node, "fsl,preserve-clocking", NULL)) {
 		clock = MPC_I2C_CLOCK_PRESERVE;
 	} else {
@@ -696,6 +715,8 @@ static int fsl_i2c_probe(struct platform_device *op)
 	return result;
 
  fail_add:
+	if (i2c->clk_per)
+		clk_disable_unprepare(i2c->clk_per);
 	free_irq(i2c->irq, i2c);
  fail_request:
 	irq_dispose_mapping(i2c->irq);
@@ -711,6 +732,9 @@ static int fsl_i2c_remove(struct platform_device *op)
 
 	i2c_del_adapter(&i2c->adap);
 
+	if (i2c->clk_per)
+		clk_disable_unprepare(i2c->clk_per);
+
 	if (i2c->irq)
 		free_irq(i2c->irq, i2c);
 
Signature · 1 line
1.7.10.4
#147 lore raw

PATCH v4 7/31 fs_enet: silence a build warning (unused variable)

In reply to Gerhard Sittig
---
 drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c |    1 -
 1 file changed, 1 deletion(-)
drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c +0 -1
index 8de53a1..c04eb3a 100644
@@ -583,7 +583,6 @@ static struct sk_buff *tx_skb_align_workaround(struct net_device *dev,
 					       struct sk_buff *skb)
 {
 	struct sk_buff *new_skb;
-	struct fs_enet_private *fep = netdev_priv(dev);
 
 	/* Alloc new skb */
 	new_skb = netdev_alloc_skb(dev, skb->len + 4);
Signature · 1 line
1.7.10.4
#148 lore raw

PATCH v4 8/31 fs_enet: cleanup clock API use

In reply to Gerhard Sittig

make the Freescale ethernet driver get, prepare and enable the FEC clock during probe(); disable and unprepare the clock upon remove(), put is done by the devm approach; hold a reference to the clock over the period of use

clock lookup is non-fatal as not all platforms provide clock specs in their device tree; failure to enable specified clocks is fatal

---
 .../net/ethernet/freescale/fs_enet/fs_enet-main.c  |   20 ++++++++++++++++++++
 include/linux/fs_enet_pd.h                         |    3 +++
 2 files changed, 23 insertions(+)
drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c +20 -0
index c04eb3a..6b60582 100644
@@ -999,6 +999,8 @@ static int fs_enet_probe(struct platform_device *ofdev)
 	struct fs_enet_private *fep;
 	struct fs_platform_info *fpi;
 	const u32 *data;
+	struct clk *clk;
+	int err;
 	const u8 *mac_addr;
 	const char *phy_connection_type;
 	int privsize, len, ret = -ENODEV;
@@ -1036,6 +1038,20 @@ static int fs_enet_probe(struct platform_device *ofdev)
 			fpi->use_rmii = 1;
 	}
 
+	/* make clock lookup non-fatal (the driver is shared among platforms),
+	 * but require enable to succeed when a clock was specified/found,
+	 * keep a reference to the clock upon successful acquisition
+	 */
+	clk = devm_clk_get(&ofdev->dev, "per");
+	if (!IS_ERR(clk)) {
+		err = clk_prepare_enable(clk);
+		if (err) {
+			ret = err;
+			goto out_free_fpi;
+		}
+		fpi->clk_per = clk;
+	}
+
 	privsize = sizeof(*fep) +
 	           sizeof(struct sk_buff **) *
 	           (fpi->rx_ring + fpi->tx_ring);
@@ -1107,6 +1123,8 @@ out_free_dev:
 	free_netdev(ndev);
 out_put:
 	of_node_put(fpi->phy_node);
+	if (fpi->clk_per)
+		clk_disable_unprepare(fpi->clk_per);
 out_free_fpi:
 	kfree(fpi);
 	return ret;
@@ -1123,6 +1141,8 @@ 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);
 	free_netdev(ndev);
 	return 0;
 }
include/linux/fs_enet_pd.h +3 -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];
Signature · 1 line
1.7.10.4
#149 lore raw

PATCH v4 9/31 powerpc/fsl-pci: improve clock API use

In reply to Gerhard Sittig

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

clock lookup is non-fatal as not all platforms may provide clock specs in their device tree, but failure to enable specified clocks are fatal

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

---
 arch/powerpc/sysdev/fsl_pci.c |   22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)
arch/powerpc/sysdev/fsl_pci.c +22 -0
index 46ac1dd..549ff08 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>
@@ -926,12 +928,32 @@ void fsl_pci_assign_primary(void)
 
 static int fsl_pci_probe(struct platform_device *pdev)
 {
+	struct clk *clk;
 	int ret;
 	struct device_node *node;
 #ifdef CONFIG_SWIOTLB
 	struct pci_controller *hose;
 #endif
 
+	/*
+	 * clock lookup is non-fatal since the driver is shared among
+	 * platforms and not all of them provide clocks specs in their
+	 * device tree, but failure to enable a specified clock is
+	 * considered fatal
+	 */
+	clk = devm_clk_get(&pdev->dev, "per");
+	if (!IS_ERR(clk)) {
+		ret = clk_prepare_enable(clk);
+		if (ret) {
+			dev_err(dev, "Could not enable peripheral clock\n");
+			return ret;
+		}
+		/*
+		 * TODO where to store the 'clk' reference?  there appears
+		 * to be no remove() routine which undoes what probe() does
+		 */
+	}
+
 	node = pdev->dev.of_node;
 	ret = fsl_add_bridge(pdev, fsl_pci_primary == node);
 
Signature · 1 line
1.7.10.4
#150 lore raw

Re: [PATCH v4 09/31] powerpc/fsl-pci: improve clock API use

On Tue, 6 Aug 2013 22:43:49 +0200
Gerhard Sittig <[email protected]> wrote:
...

Quoted 4 lines
arch/powerpc/sysdev/fsl_pci.c +0 -0
index 46ac1dd..549ff08 100644

...

arch/powerpc/sysdev/fsl_pci.c void fsl_pci_assign_primary(void) · line 948

Quoted 5 lines
arch/powerpc/sysdev/fsl_pci.c +5 -0
+	clk = devm_clk_get(&pdev->dev, "per");
+	if (!IS_ERR(clk)) {
+		ret = clk_prepare_enable(clk);
+		if (ret) {
+			dev_err(dev, "Could not enable peripheral clock\n");

above line will break building. s/dev,/&pdev->dev,/

Thanks,
Anatolij

#151 lore raw

Re: [PATCH v4 09/31] powerpc/fsl-pci: improve clock API use

Quoted 9 lines · Anatolij Gustschin

On Tue, 6 Aug 2013 22:43:49 +0200
Gerhard Sittig <[email protected]> wrote:
...

Quoted 4 lines
arch/powerpc/sysdev/fsl_pci.c +0 -0
index 46ac1dd..549ff08 100644

...

Quoted 7 lines

arch/powerpc/sysdev/fsl_pci.c void fsl_pci_assign_primary(void) · line 948

Quoted 5 lines
arch/powerpc/sysdev/fsl_pci.c +5 -0
+	clk = devm_clk_get(&pdev->dev, "per");
+	if (!IS_ERR(clk)) {
+		ret = clk_prepare_enable(clk);
+		if (ret) {
+			dev_err(dev, "Could not enable peripheral clock\n");

above line will break building. s/dev,/&pdev->dev,/

Thank you for testing and for the feedback.

I've queued this fix for v5. The bug could hide because the file gets compiled on MPC512x, but this specific routine sits behind even more compile time switches (85xx and 86xx related).

virtually yours
Gerhard Sittig

Signature · 3 lines
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: [email protected]
#152 lore raw

PATCH v4 9/31 powerpc/fsl-pci: improve clock API use

In reply to Gerhard Sittig

[ re-created the Cc: list, this is about the PCI clock exclusively ]

Of all the "preparation" patches in the series (parts 01-14/31, forming the "peripheral driver cleanup" phase before the introduction of CCF support), this patch remains the last to get picked up.

But I'd suggest to leave this patch for now (for v3.12, it's rather late). Either ignore this message and the patch, or see below for why application isn't required now, and an update of this patch is needed and will be appropriate for v3.13.

I'm sorry for the confusion, the potentially perceived instability is a result of both widening the series' scope after initial submission as well as a recent extension of test coverage after the scope has been widened. Thank you for your patience!

Quoted 21 lines · Gerhard Sittig

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

clock lookup is non-fatal as not all platforms may provide clock specs in their device tree, but failure to enable specified clocks are fatal

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

---
 arch/powerpc/sysdev/fsl_pci.c |   22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)
arch/powerpc/sysdev/fsl_pci.c +0 -0
index 46ac1dd..549ff08 100644

arch/powerpc/sysdev/fsl_pci.c

Quoted 1 line
arch/powerpc/sysdev/fsl_pci.c +0 -0
@@ -17,6 +17,8 @@
...

What this patch 09/31 does is add a non-fatal device tree based clock lookup in the fsl_pci_probe() routine, to acquire the PCI clock item appropriately if there is a provider and a DT spec.

The patch in v4 has a bug, which has an obvious fix while an update wasn't sent yet, for neither the patch nor the series. There is one more known issue in the series (not with functionality but with policy, specifically in a multi platform configuration), while I don't want to resend the series while known issues are pending. But this is not the problem here.

First of all the patch is a NOP in the forseeable future. It won't harm yet its content isn't urgently needed either, to unbreak stuff or to support upcoming features that were communicated before.

Further analysis has shown that the patch is incomplete.

The 85xx and 86xx platforms will pass through the fsl_pci_probe() routine. That these platforms don't have OF clock providers is not a problem, the patch will remain a NOP then. Its function will kick in when these platforms may grow clock providers (things will transparently keep working, this was the actual intent of the patch). Since the series is about 512x CCF support, the patch will remain a NOP throughout the whole series, but won't harm either.

The 83xx and 512x platforms in contrast _don't_ pass through the fsl_pci_probe() routine, instead they call mpc83xx_add_bridge() from within the .setup_arch() callback in platform initialization code, which iterates over the compatible OF nodes, and runs at a point in time where the platform's clock provider has not yet been setup and thus is not available. In this situation any clock lookup will fail, which is not fatal during PCI setup yet won't acquire the clock item and thus will have the common infrastructure disable the "unused" clock much later.

There is a workaround for this lack of proper clock acquisition in the peripheral driver. The clock provider needs to pre-enable the PCI clock item upon its initialization, because the peripheral driver can't when it initializes. Checking the same condition in the provider's pre-enable workaround which the .setup_arch() routine is checking before the add_bridge() calls (the presence of compatible nodes) results in correct operation as well as most appropriate resource use (clock enabled when PCI hardware was attached to, and clock disabled in the absence of PCI hardware or driver attachment).

So the update of this patch 09/31 will contain
- the fix for the copy'n'paste bug in the probe() routine
- an appropriate comment in the add_bridge() routine
- no change in its nature, the idea remains unaffected

The backend (clock provider) will contain the pre-enable workaround for the PCI clock item.

As a result, the 83xx, 85xx, and 86xx platforms won't see any change (there is a NOP in probe() and a comment in add_bridge(), neither of which break any operation). The 512x platform will have proper PCI operation in the presence of common clock support. Should 8xxx platforms grow CCF support later, they will transparently keep working (85xx, 86xx), or may add the same simple yet appropriate workaround (83xx).

So the outline is there, the approach is straight forward and easily can get implemented, and the resulting code will work for all platforms while there is no potential for breakage. The PCI driver will improve, and all is well. :) There is no need for action for v3.12, and v3.13 can include the improvement.

virtually yours
Gerhard Sittig

Signature · 3 lines
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
#153 lore raw

PATCH v4 9/31 powerpc/fsl-pci: improve clock API use

Quoted 2 lines · Gerhard Sittig

[ re-created the Cc: list, this is about the PCI clock exclusively ]

I just noticed by coincidence that the message which I received back from the linuxppc-dev ML appeared to have dropped Benjamin Herrenschmidt and Kumar Gala from the Cc: list -- while they do appear in the header of the message that I have sent and I can't see what might have caused the loss of information. :-O

Do you want me to re-send the message for the benefit of potential followups, or is it OK that you receive the message via the list but potentially without the Cc: attribute?

The message was mostly "for your information" and contained a status update, while no action is required or problems need to get resolved.

virtually yours
Gerhard Sittig

Signature · 3 lines
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
#154 lore raw

PATCH v4 9/31 powerpc/fsl-pci: improve clock API use

Quoted 9 lines · Gerhard Sittig
Quoted 2 lines · Gerhard Sittig

[ re-created the Cc: list, this is about the PCI clock exclusively ]

I just noticed by coincidence that the message which I received back from the linuxppc-dev ML appeared to have dropped Benjamin Herrenschmidt and Kumar Gala from the Cc: list -- while they do appear in the header of the message that I have sent and I can't see what might have caused the loss of information. :-O

Don't bother with me. I haven't had the bandwidth to look at that at all. I'll leave Anatolij the responsibility here :-)

Cheers,
Ben.

Quoted 11 lines

Do you want me to re-send the message for the benefit of potential followups, or is it OK that you receive the message via the list but potentially without the Cc: attribute?

The message was mostly "for your information" and contained a status update, while no action is required or problems need to get resolved.

virtually yours
Gerhard Sittig

#155 lore raw

PATCH v4 10/31 net: can: mscan: add a comment on reg to idx mapping

In reply to Gerhard Sittig

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(-)
drivers/net/can/mscan/mpc5xxx_can.c +4 -1
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;
Signature · 1 line
1.7.10.4
#156 lore raw

PATCH v4 11/31 net: can: mscan: improve clock API use

In reply to Gerhard Sittig

the .get_clock() callback is run from probe() and might allocate resources, introduce a .put_clock() callback that is run from remove() to undo any allocation activities

prepare and enable the clocks in open(), disable and unprepare the clocks in close() if clocks were acquired during probe(), to not assume knowledge about which activities are done in probe() and remove()

use devm_get_clk() to lookup the SYS and REF clocks, to have the clocks put upon device shutdown

store pointers to data structures upon successful allocation already instead of deferral until complete setup, such that subroutines in the setup sequence may access those data structures as well to track their resource acquisition

since clock allocation remains optional, the release callback as well as the enable/disable calls in open/close are optional as well

---
 drivers/net/can/mscan/mpc5xxx_can.c |   18 ++++++++++++------
 drivers/net/can/mscan/mscan.c       |   27 ++++++++++++++++++++++++++-
 drivers/net/can/mscan/mscan.h       |    3 +++
 3 files changed, 41 insertions(+), 7 deletions(-)
drivers/net/can/mscan/mpc5xxx_can.c +12 -6
index bc422ba..e59b3a3 100644
@@ -40,6 +40,7 @@ struct mpc5xxx_can_data {
 	unsigned int type;
 	u32 (*get_clock)(struct platform_device *ofdev, const char *clock_name,
 			 int *mscan_clksrc);
+	void (*put_clock)(struct platform_device *ofdev);
 };
 
 #ifdef CONFIG_PPC_MPC52xx
@@ -180,7 +181,7 @@ static u32 mpc512x_can_get_clock(struct platform_device *ofdev,
 			clockdiv = 1;
 
 		if (!clock_name || !strcmp(clock_name, "sys")) {
-			sys_clk = clk_get(&ofdev->dev, "sys_clk");
+			sys_clk = devm_clk_get(&ofdev->dev, "sys_clk");
 			if (IS_ERR(sys_clk)) {
 				dev_err(&ofdev->dev, "couldn't get sys_clk\n");
 				goto exit_unmap;
@@ -203,7 +204,7 @@ static u32 mpc512x_can_get_clock(struct platform_device *ofdev,
 		}
 
 		if (clocksrc < 0) {
-			ref_clk = clk_get(&ofdev->dev, "ref_clk");
+			ref_clk = devm_clk_get(&ofdev->dev, "ref_clk");
 			if (IS_ERR(ref_clk)) {
 				dev_err(&ofdev->dev, "couldn't get ref_clk\n");
 				goto exit_unmap;
@@ -280,6 +281,8 @@ static int mpc5xxx_can_probe(struct platform_device *ofdev)
 	dev = alloc_mscandev();
 	if (!dev)
 		goto exit_dispose_irq;
+	platform_set_drvdata(ofdev, dev);
+	SET_NETDEV_DEV(dev, &ofdev->dev);
 
 	priv = netdev_priv(dev);
 	priv->reg_base = base;
@@ -296,8 +299,6 @@ static int mpc5xxx_can_probe(struct platform_device *ofdev)
 		goto exit_free_mscan;
 	}
 
-	SET_NETDEV_DEV(dev, &ofdev->dev);
-
 	err = register_mscandev(dev, mscan_clksrc);
 	if (err) {
 		dev_err(&ofdev->dev, "registering %s failed (err=%d)\n",
@@ -305,8 +306,6 @@ static int mpc5xxx_can_probe(struct platform_device *ofdev)
 		goto exit_free_mscan;
 	}
 
-	platform_set_drvdata(ofdev, dev);
-
 	dev_info(&ofdev->dev, "MSCAN at 0x%p, irq %d, clock %d Hz\n",
 		 priv->reg_base, dev->irq, priv->can.clock.freq);
 
@@ -324,10 +323,17 @@ exit_unmap_mem:
 
 static int mpc5xxx_can_remove(struct platform_device *ofdev)
 {
+	const struct of_device_id *match;
+	const struct mpc5xxx_can_data *data;
 	struct net_device *dev = platform_get_drvdata(ofdev);
 	struct mscan_priv *priv = netdev_priv(dev);
 
+	match = of_match_device(mpc5xxx_can_table, &ofdev->dev);
+	data = match ? match->data : NULL;
+
 	unregister_mscandev(dev);
+	if (data && data->put_clock)
+		data->put_clock(ofdev);
 	iounmap(priv->reg_base);
 	irq_dispose_mapping(dev->irq);
 	free_candev(dev);
drivers/net/can/mscan/mscan.c +26 -1
index e6b4095..4f998f5 100644
@@ -573,10 +573,24 @@ static int mscan_open(struct net_device *dev)
 	struct mscan_priv *priv = netdev_priv(dev);
 	struct mscan_regs __iomem *regs = priv->reg_base;
 
+	if (priv->clk_ipg) {
+		ret = clk_prepare_enable(priv->clk_ipg);
+		if (ret)
+			goto exit_retcode;
+	}
+	if (priv->clk_can) {
+		ret = clk_prepare_enable(priv->clk_can);
+		if (ret) {
+			if (priv->clk_ipg)
+				clk_disable_unprepare(priv->clk_ipg);
+			goto exit_retcode;
+		}
+	}
+
 	/* common open */
 	ret = open_candev(dev);
 	if (ret)
-		return ret;
+		goto exit_dis_clock;
 
 	napi_enable(&priv->napi);
 
@@ -604,6 +618,12 @@ exit_free_irq:
 exit_napi_disable:
 	napi_disable(&priv->napi);
 	close_candev(dev);
+exit_dis_clock:
+	if (priv->clk_can)
+		clk_disable_unprepare(priv->clk_can);
+	if (priv->clk_ipg)
+		clk_disable_unprepare(priv->clk_ipg);
+exit_retcode:
 	return ret;
 }
 
@@ -621,6 +641,11 @@ static int mscan_close(struct net_device *dev)
 	close_candev(dev);
 	free_irq(dev->irq, dev);
 
+	if (priv->clk_can)
+		clk_disable_unprepare(priv->clk_can);
+	if (priv->clk_ipg)
+		clk_disable_unprepare(priv->clk_ipg);
+
 	return 0;
 }
 
drivers/net/can/mscan/mscan.h +3 -0
index af2ed8b..9c24d60 100644
@@ -21,6 +21,7 @@
 #ifndef __MSCAN_H__
 #define __MSCAN_H__
 
+#include <linux/clk.h>
 #include <linux/types.h>
 
 /* MSCAN control register 0 (CANCTL0) bits */
@@ -283,6 +284,8 @@ struct mscan_priv {
 	unsigned int type; 	/* MSCAN type variants */
 	unsigned long flags;
 	void __iomem *reg_base;	/* ioremap'ed address to registers */
+	struct clk *clk_ipg;	/* clock for registers */
+	struct clk *clk_can;	/* clock for bitrates */
 	u8 shadow_statflg;
 	u8 shadow_canrier;
 	u8 cur_pri;
Signature · 1 line
1.7.10.4
#157 lore raw

PATCH v4 11/31 net: can: mscan: improve clock API use

Quoted 110 lines · Gerhard Sittig

the .get_clock() callback is run from probe() and might allocate resources, introduce a .put_clock() callback that is run from remove() to undo any allocation activities

prepare and enable the clocks in open(), disable and unprepare the clocks in close() if clocks were acquired during probe(), to not assume knowledge about which activities are done in probe() and remove()

use devm_get_clk() to lookup the SYS and REF clocks, to have the clocks put upon device shutdown

store pointers to data structures upon successful allocation already instead of deferral until complete setup, such that subroutines in the setup sequence may access those data structures as well to track their resource acquisition

since clock allocation remains optional, the release callback as well as the enable/disable calls in open/close are optional as well

---
 drivers/net/can/mscan/mpc5xxx_can.c |   18 ++++++++++++------
 drivers/net/can/mscan/mscan.c       |   27 ++++++++++++++++++++++++++-
 drivers/net/can/mscan/mscan.h       |    3 +++
 3 files changed, 41 insertions(+), 7 deletions(-)
drivers/net/can/mscan/mpc5xxx_can.c +12 -6
index bc422ba..e59b3a3 100644
@@ -40,6 +40,7 @@ struct mpc5xxx_can_data {
 	unsigned int type;
 	u32 (*get_clock)(struct platform_device *ofdev, const char *clock_name,
 			 int *mscan_clksrc);
+	void (*put_clock)(struct platform_device *ofdev);
 };
 
 #ifdef CONFIG_PPC_MPC52xx
@@ -180,7 +181,7 @@ static u32 mpc512x_can_get_clock(struct platform_device *ofdev,
 			clockdiv = 1;
 
 		if (!clock_name || !strcmp(clock_name, "sys")) {
-			sys_clk = clk_get(&ofdev->dev, "sys_clk");
+			sys_clk = devm_clk_get(&ofdev->dev, "sys_clk");
 			if (IS_ERR(sys_clk)) {
 				dev_err(&ofdev->dev, "couldn't get sys_clk\n");
 				goto exit_unmap;
@@ -203,7 +204,7 @@ static u32 mpc512x_can_get_clock(struct platform_device *ofdev,
 		}
 
 		if (clocksrc < 0) {
-			ref_clk = clk_get(&ofdev->dev, "ref_clk");
+			ref_clk = devm_clk_get(&ofdev->dev, "ref_clk");
 			if (IS_ERR(ref_clk)) {
 				dev_err(&ofdev->dev, "couldn't get ref_clk\n");
 				goto exit_unmap;
@@ -280,6 +281,8 @@ static int mpc5xxx_can_probe(struct platform_device *ofdev)
 	dev = alloc_mscandev();
 	if (!dev)
 		goto exit_dispose_irq;
+	platform_set_drvdata(ofdev, dev);
+	SET_NETDEV_DEV(dev, &ofdev->dev);
 
 	priv = netdev_priv(dev);
 	priv->reg_base = base;
@@ -296,8 +299,6 @@ static int mpc5xxx_can_probe(struct platform_device *ofdev)
 		goto exit_free_mscan;
 	}
 
-	SET_NETDEV_DEV(dev, &ofdev->dev);
-
 	err = register_mscandev(dev, mscan_clksrc);
 	if (err) {
 		dev_err(&ofdev->dev, "registering %s failed (err=%d)\n",
@@ -305,8 +306,6 @@ static int mpc5xxx_can_probe(struct platform_device *ofdev)
 		goto exit_free_mscan;
 	}
 
-	platform_set_drvdata(ofdev, dev);
-
 	dev_info(&ofdev->dev, "MSCAN at 0x%p, irq %d, clock %d Hz\n",
 		 priv->reg_base, dev->irq, priv->can.clock.freq);
 
@@ -324,10 +323,17 @@ exit_unmap_mem:
 
 static int mpc5xxx_can_remove(struct platform_device *ofdev)
 {
+	const struct of_device_id *match;
+	const struct mpc5xxx_can_data *data;
 	struct net_device *dev = platform_get_drvdata(ofdev);
 	struct mscan_priv *priv = netdev_priv(dev);
 
+	match = of_match_device(mpc5xxx_can_table, &ofdev->dev);
+	data = match ? match->data : NULL;
+
 	unregister_mscandev(dev);
+	if (data && data->put_clock)
+		data->put_clock(ofdev);
 	iounmap(priv->reg_base);
 	irq_dispose_mapping(dev->irq);
 	free_candev(dev);
drivers/net/can/mscan/mscan.c +1 -0
index e6b4095..4f998f5 100644
@@ -573,10 +573,24 @@ static int mscan_open(struct net_device *dev)
 	struct mscan_priv *priv = netdev_priv(dev);
 	struct mscan_regs __iomem *regs = priv->reg_base;
 
+	if (priv->clk_ipg) {

drivers/net/can/mscan/mscan.c static int mscan_open(struct net_device *dev) · line 586

Quoted 10 lines
drivers/net/can/mscan/mscan.c +10 -0
+		ret = clk_prepare_enable(priv->clk_ipg);
+		if (ret)
+			goto exit_retcode;
+	}
+	if (priv->clk_can) {
+		ret = clk_prepare_enable(priv->clk_can);
+		if (ret) {
+			if (priv->clk_ipg)
+				clk_disable_unprepare(priv->clk_ipg);
+			goto exit_retcode;

Why don't you add another jump label and jump to that to disable the ipkg clock?

Quoted 13 lines
drivers/net/can/mscan/mscan.c +4 -1
+		}
+	}
+
 	/* common open */
 	ret = open_candev(dev);
 	if (ret)
-		return ret;
+		goto exit_dis_clock;
 
 	napi_enable(&priv->napi);
 
@@ -604,6 +618,12 @@ exit_free_irq:
 exit_napi_disable:

drivers/net/can/mscan/mscan.c exit_free_irq: · line 628

Quoted 10 lines
drivers/net/can/mscan/mscan.c +6 -0
 	napi_disable(&priv->napi);
 	close_candev(dev);
+exit_dis_clock:
+	if (priv->clk_can)
+		clk_disable_unprepare(priv->clk_can);
+	if (priv->clk_ipg)
+		clk_disable_unprepare(priv->clk_ipg);
+exit_retcode:
 	return ret;
 }

Marc

Signature · 12 lines
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 259 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130807/ff55f5e9/attachment.sig>
#158 lore raw

PATCH v4 11/31 net: can: mscan: improve clock API use

Quoted 11 lines · Marc Kleine-Budde
Quoted 9 lines · Gerhard Sittig

[ ... ]

drivers/net/can/mscan/mscan.c +0 -0
index e6b4095..4f998f5 100644
@@ -573,10 +573,24 @@ static int mscan_open(struct net_device *dev)
 	struct mscan_priv *priv = netdev_priv(dev);
 	struct mscan_regs __iomem *regs = priv->reg_base;
 
Quoted 14 lines

drivers/net/can/mscan/mscan.c static int mscan_open(struct net_device *dev) · line 586

Quoted 11 lines
drivers/net/can/mscan/mscan.c +11 -0
+	if (priv->clk_ipg) {
+		ret = clk_prepare_enable(priv->clk_ipg);
+		if (ret)
+			goto exit_retcode;
+	}
+	if (priv->clk_can) {
+		ret = clk_prepare_enable(priv->clk_can);
+		if (ret) {
+			if (priv->clk_ipg)
+				clk_disable_unprepare(priv->clk_ipg);
+			goto exit_retcode;

Why don't you add another jump label and jump to that to disable the ipkg clock?

You are right. I've queued this change for v5 (adding a label in the existing error path, jumping to it instead of explicitly disabling the clock).

Quoted 12 lines
Quoted 12 lines
drivers/net/can/mscan/mscan.c +4 -1
+		}
+	}
+
 	/* common open */
 	ret = open_candev(dev);
 	if (ret)
-		return ret;
+		goto exit_dis_clock;
 
 	napi_enable(&priv->napi);
 
Quoted 14 lines

drivers/net/can/mscan/mscan.c exit_free_irq: · line 628

Quoted 12 lines
drivers/net/can/mscan/mscan.c +6 -0
@@ -604,6 +618,12 @@ exit_free_irq:
 exit_napi_disable:
 	napi_disable(&priv->napi);
 	close_candev(dev);
+exit_dis_clock:
+	if (priv->clk_can)
+		clk_disable_unprepare(priv->clk_can);
+	if (priv->clk_ipg)
+		clk_disable_unprepare(priv->clk_ipg);
+exit_retcode:
 	return ret;
 }

Marc

Thank you for reviewing several versions of the patch!

virtually yours
Gerhard Sittig

Signature · 3 lines
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
#159 lore raw

PATCH v4 11/31 net: can: mscan: improve clock API use

In reply to Gerhard Sittig
Quoted 3 lines · Gerhard Sittig

the .get_clock() callback is run from probe() and might allocate resources, introduce a .put_clock() callback that is run from remove() to undo any allocation activities

AFAICS With this patch put_clock() is still a no-op, is there a patch which adds some code there? If not, please remove.

Marc

Signature · 12 lines
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 259 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130807/aad307fe/attachment.sig>
#160 lore raw

PATCH v4 11/31 net: can: mscan: improve clock API use

Quoted 7 lines · Marc Kleine-Budde
Quoted 3 lines · Gerhard Sittig

the .get_clock() callback is run from probe() and might allocate resources, introduce a .put_clock() callback that is run from remove() to undo any allocation activities

AFAICS With this patch put_clock() is still a no-op, is there a patch which adds some code there? If not, please remove.

I missed patch 27.

sorry for the noise.
Marc

Signature · 12 lines
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 259 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130807/1214eb32/attachment-0001.sig>
#161 lore raw

PATCH v4 12/31 powerpc: mpc512x: array decl for MCLK registers in CCM

In reply to Gerhard Sittig

reword the clock control module's registers declaration such that the MCLK related registers form an array and get indexed by PSC controller or CAN controller component 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(-)
arch/powerpc/include/asm/mpc5121.h +2 -16
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 */
 };
 
Signature · 1 line
1.7.10.4
maintainer Applied
#162 lore raw

Re: [PATCH v4 12/31] powerpc: mpc512x: array decl for MCLK registers in CCM

Quoted 4 lines · On Tue, 6 Aug 2013 22:43:52 +0200 Gerhard Sittig <[email protected]> wrote:

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

Quoted 8 lines

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(-)

Applied, thanks!

Anatolij

#163 lore raw

PATCH v4 13/31 clk: wrap I/O access for improved portability

In reply to Gerhard Sittig

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(-)
drivers/clk/clk-divider.c +3 -3
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);
drivers/clk/clk-gate.c +3 -3
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)
drivers/clk/clk-mux.c +3 -3
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);
include/linux/clk-provider.h +17 -0
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 */
Signature · 1 line
1.7.10.4
#164 lore raw

PATCH v4 14/31 dts: mpc512x: prepare for preprocessor support

In reply to Gerhard Sittig

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
arch/powerpc/boot/dts/ac14xx.dts +1 -1
index a27a460..a543c40 100644
@@ -10,7 +10,7 @@
  */
 
 
-/include/ "mpc5121.dtsi"
+#include <mpc5121.dtsi>
 
 / {
 	model = "ac14xx";
arch/powerpc/boot/dts/include/dt-bindings new file +1 -0
new file mode 120000
index 0000000..08c00e4
@@ -0,0 +1 @@
+../../../../../include/dt-bindings
\ No newline at end of file
arch/powerpc/boot/dts/mpc5121ads.dts +1 -1
index 7d3cb79..c228a0a 100644
@@ -9,7 +9,7 @@
  * option) any later version.
  */
 
-/include/ "mpc5121.dtsi"
+#include <mpc5121.dtsi>
 
 / {
 	model = "mpc5121ads";
arch/powerpc/boot/dts/pdm360ng.dts +1 -1
index 7433740..871c16d 100644
@@ -13,7 +13,7 @@
  * option) any later version.
  */
 
-/include/ "mpc5121.dtsi"
+#include <mpc5121.dtsi>
 
 / {
 	model = "pdm360ng";
Signature · 1 line
1.7.10.4
maintainer Applied
#165 lore raw

Re: [PATCH v4 14/31] dts: mpc512x: prepare for preprocessor support

Quoted 4 lines · On Tue, 6 Aug 2013 22:43:54 +0200 Gerhard Sittig <[email protected]> wrote:

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

Quoted 8 lines
---
 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

Applied, thanks!

Anatolij

#166 lore raw

PATCH v4 15/31 dts: mpc512x: introduce dt-bindings/clock/ header

In reply to Gerhard Sittig

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 |   69 +++++++++++++++++++++++++++++
 1 file changed, 69 insertions(+)
 create mode 100644 include/dt-bindings/clock/mpc512x-clock.h
include/dt-bindings/clock/mpc512x-clock.h new file +69 -0
new file mode 100644
index 0000000..9e81b3b
@@ -0,0 +1,69 @@
+/*
+ * This header provides constants for MPC512x clock specs in DT bindings.
+ */
+
+#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_BDLC		24
+#define MPC512x_CLK_SDHC		25
+#define MPC512x_CLK_PCI			26
+#define MPC512x_CLK_PSC_MCLK_IN		27
+#define MPC512x_CLK_SPDIF_TX		28
+#define MPC512x_CLK_SPDIF_RX		29
+#define MPC512x_CLK_SPDIF_MCLK		30
+#define MPC512x_CLK_SPDIF		31
+#define MPC512x_CLK_AC97		32
+#define MPC512x_CLK_PSC0_MCLK		33
+#define MPC512x_CLK_PSC1_MCLK		34
+#define MPC512x_CLK_PSC2_MCLK		35
+#define MPC512x_CLK_PSC3_MCLK		36
+#define MPC512x_CLK_PSC4_MCLK		37
+#define MPC512x_CLK_PSC5_MCLK		38
+#define MPC512x_CLK_PSC6_MCLK		39
+#define MPC512x_CLK_PSC7_MCLK		40
+#define MPC512x_CLK_PSC8_MCLK		41
+#define MPC512x_CLK_PSC9_MCLK		42
+#define MPC512x_CLK_PSC10_MCLK		43
+#define MPC512x_CLK_PSC11_MCLK		44
+#define MPC512x_CLK_PSC_FIFO		45
+#define MPC512x_CLK_PSC0		46
+#define MPC512x_CLK_PSC1		47
+#define MPC512x_CLK_PSC2		48
+#define MPC512x_CLK_PSC3		49
+#define MPC512x_CLK_PSC4		50
+#define MPC512x_CLK_PSC5		51
+#define MPC512x_CLK_PSC6		52
+#define MPC512x_CLK_PSC7		53
+#define MPC512x_CLK_PSC8		54
+#define MPC512x_CLK_PSC9		55
+#define MPC512x_CLK_PSC10		56
+#define MPC512x_CLK_PSC11		57
+
+#define MPC512x_CLK_LAST_PUBLIC		57
+
+#endif
Signature · 1 line
1.7.10.4
#167 lore raw

PATCH v4 16/31 dts: mpc512x: add clock related device tree specs

In reply to Gerhard Sittig

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(-)
arch/powerpc/boot/dts/ac14xx.dts +7 -0
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";
arch/powerpc/boot/dts/mpc5121.dtsi +14 -1
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 */
Signature · 1 line
1.7.10.4
#168 lore raw

PATCH v4 17/31 clk: mpc512x: introduce COMMON_CLK for MPC512x

In reply to Gerhard Sittig

this change implements a clock driver 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 remains 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)

some of the clock items get pre-enabled in the clock driver to not have them automatically disabled by the underlying clock subsystem because of their being unused -- this approach is desirable because
- some of the clocks are useful to have for diagnostics and information despite their not getting claimed by any drivers (CPU, internal and external RAM, internal busses, boot media)
- some of the clocks aren't claimed by their peripheral drivers yet, either because of missing driver support or because device tree specs aren't available yet (but the workarounds will get removed as the drivers get adjusted and the device tree provides the clock specs)

clkdev registration provides "alias names" for few clock items
- to not break those peripheral drivers which encode their component index into the name that is used for clock lookup (UART, SPI, USB)
- to not break those drivers which use names for the clock lookup which were encoded in the previous PPC_CLOCK implementation (NFC, VIU, CAN)
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 |  748 +++++++++++++++++++++++++
 include/linux/clk-provider.h                  |   16 +
 4 files changed, 780 insertions(+), 2 deletions(-)
 create mode 100644 arch/powerpc/platforms/512x/clock-commonclk.c
arch/powerpc/platforms/512x/Kconfig +13 -1
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
arch/powerpc/platforms/512x/Makefile +3 -1
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
arch/powerpc/platforms/512x/clock-commonclk.c new file +748 -0

748 lines not on this page · raw message

include/linux/clk-provider.h +16 -0
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 */
Signature · 1 line
1.7.10.4
#169 lore raw

PATCH v4 18/31 dts: mpc512x: add clock specs for client lookups

In reply to Gerhard Sittig

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

---
 arch/powerpc/boot/dts/mpc5121.dtsi |   95 ++++++++++++++++++++++++++++++++++++
 1 file changed, 95 insertions(+)
arch/powerpc/boot/dts/mpc5121.dtsi +95 -0
index 8f4cba0..4cef164 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 = "per";
 	};
 
 	localbus at 80000020 {
@@ -153,12 +159,24 @@
 			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>,
+				 <&clks MPC512x_CLK_BDLC>;
+			clock-names = "ips", "sys", "ref", "mclk", "ipg";
 		};
 
 		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>,
+				 <&clks MPC512x_CLK_BDLC>;
+			clock-names = "ips", "sys", "ref", "mclk", "ipg";
 		};
 
 		sdhc at 1500 {
@@ -167,6 +185,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 +196,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 +206,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 +216,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 +229,48 @@
 			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>,
+				 <&clks MPC512x_CLK_BDLC>;
+			clock-names = "ips", "sys", "ref", "mclk", "ipg";
 		};
 
 		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>,
+				 <&clks MPC512x_CLK_BDLC>;
+			clock-names = "ips", "sys", "ref", "mclk", "ipg";
 		};
 
 		viu at 2400 {
 			compatible = "fsl,mpc5121-viu";
 			reg = <0x2400 0x400>;
 			interrupts = <67 0x8>;
+			clocks = <&clks MPC512x_CLK_VIU>;
+			clock-names = "per";
 		};
 
 		mdio at 2800 {
@@ -233,6 +278,8 @@
 			reg = <0x2800 0x800>;
 			#address-cells = <1>;
 			#size-cells = <0>;
+			clocks = <&clks MPC512x_CLK_FEC>;
+			clock-names = "per";
 		};
 
 		eth0: ethernet at 2800 {
@@ -241,6 +288,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 +301,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 +314,8 @@
 			interrupts = <44 0x8>;
 			dr_mode = "otg";
 			phy_type = "utmi_wide";
+			clocks = <&clks MPC512x_CLK_USB2>;
+			clock-names = "per";
 		};
 
 		/* IO control */
@@ -281,6 +334,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 +347,9 @@
 			interrupts = <40 0x8>;
 			fsl,rx-fifo-size = <16>;
 			fsl,tx-fifo-size = <16>;
+			clocks = <&clks MPC512x_CLK_PSC0_MCLK>,
+				 <&clks MPC512x_CLK_PSC0>;
+			clock-names = "mclk", "ipg";
 		};
 
 		/* PSC1 */
@@ -301,6 +359,9 @@
 			interrupts = <40 0x8>;
 			fsl,rx-fifo-size = <16>;
 			fsl,tx-fifo-size = <16>;
+			clocks = <&clks MPC512x_CLK_PSC1_MCLK>,
+				 <&clks MPC512x_CLK_PSC1>;
+			clock-names = "mclk", "ipg";
 		};
 
 		/* PSC2 */
@@ -310,6 +371,9 @@
 			interrupts = <40 0x8>;
 			fsl,rx-fifo-size = <16>;
 			fsl,tx-fifo-size = <16>;
+			clocks = <&clks MPC512x_CLK_PSC2_MCLK>,
+				 <&clks MPC512x_CLK_PSC2>;
+			clock-names = "mclk", "ipg";
 		};
 
 		/* PSC3 */
@@ -319,6 +383,9 @@
 			interrupts = <40 0x8>;
 			fsl,rx-fifo-size = <16>;
 			fsl,tx-fifo-size = <16>;
+			clocks = <&clks MPC512x_CLK_PSC3_MCLK>,
+				 <&clks MPC512x_CLK_PSC3>;
+			clock-names = "mclk", "ipg";
 		};
 
 		/* PSC4 */
@@ -328,6 +395,9 @@
 			interrupts = <40 0x8>;
 			fsl,rx-fifo-size = <16>;
 			fsl,tx-fifo-size = <16>;
+			clocks = <&clks MPC512x_CLK_PSC4_MCLK>,
+				 <&clks MPC512x_CLK_PSC4>;
+			clock-names = "mclk", "ipg";
 		};
 
 		/* PSC5 */
@@ -337,6 +407,9 @@
 			interrupts = <40 0x8>;
 			fsl,rx-fifo-size = <16>;
 			fsl,tx-fifo-size = <16>;
+			clocks = <&clks MPC512x_CLK_PSC5_MCLK>,
+				 <&clks MPC512x_CLK_PSC5>;
+			clock-names = "mclk", "ipg";
 		};
 
 		/* PSC6 */
@@ -346,6 +419,9 @@
 			interrupts = <40 0x8>;
 			fsl,rx-fifo-size = <16>;
 			fsl,tx-fifo-size = <16>;
+			clocks = <&clks MPC512x_CLK_PSC6_MCLK>,
+				 <&clks MPC512x_CLK_PSC6>;
+			clock-names = "mclk", "ipg";
 		};
 
 		/* PSC7 */
@@ -355,6 +431,9 @@
 			interrupts = <40 0x8>;
 			fsl,rx-fifo-size = <16>;
 			fsl,tx-fifo-size = <16>;
+			clocks = <&clks MPC512x_CLK_PSC7_MCLK>,
+				 <&clks MPC512x_CLK_PSC7>;
+			clock-names = "mclk", "ipg";
 		};
 
 		/* PSC8 */
@@ -364,6 +443,9 @@
 			interrupts = <40 0x8>;
 			fsl,rx-fifo-size = <16>;
 			fsl,tx-fifo-size = <16>;
+			clocks = <&clks MPC512x_CLK_PSC8_MCLK>,
+				 <&clks MPC512x_CLK_PSC8>;
+			clock-names = "mclk", "ipg";
 		};
 
 		/* PSC9 */
@@ -373,6 +455,9 @@
 			interrupts = <40 0x8>;
 			fsl,rx-fifo-size = <16>;
 			fsl,tx-fifo-size = <16>;
+			clocks = <&clks MPC512x_CLK_PSC9_MCLK>,
+				 <&clks MPC512x_CLK_PSC9>;
+			clock-names = "mclk", "ipg";
 		};
 
 		/* PSC10 */
@@ -382,6 +467,9 @@
 			interrupts = <40 0x8>;
 			fsl,rx-fifo-size = <16>;
 			fsl,tx-fifo-size = <16>;
+			clocks = <&clks MPC512x_CLK_PSC10_MCLK>,
+				 <&clks MPC512x_CLK_PSC10>;
+			clock-names = "mclk", "ipg";
 		};
 
 		/* PSC11 */
@@ -391,12 +479,17 @@
 			interrupts = <40 0x8>;
 			fsl,rx-fifo-size = <16>;
 			fsl,tx-fifo-size = <16>;
+			clocks = <&clks MPC512x_CLK_PSC11_MCLK>,
+				 <&clks MPC512x_CLK_PSC11>;
+			clock-names = "mclk", "ipg";
 		};
 
 		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 +507,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 */
Signature · 1 line
1.7.10.4
#170 lore raw

PATCH v4 19/31 clk: mpc512x: don't pre-enable FEC and I2C clocks

In reply to Gerhard Sittig

after the device tree nodes provide clock specs for client side lookups, peripheral drivers can attach to their clocks and the clock driver need no longer pre-enable those clock items

---
 arch/powerpc/platforms/512x/clock-commonclk.c |    2 --
 1 file changed, 2 deletions(-)
arch/powerpc/platforms/512x/clock-commonclk.c +0 -2
index 09721d5..3f559fe 100644
@@ -685,9 +685,7 @@ static void mpc512x_clk_setup_clock_tree(int busfreq)
 	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 */
 	clk_prepare_enable(clks[MPC512x_CLK_DIU]);	/* display */
-	clk_prepare_enable(clks[MPC512x_CLK_I2C]);
 	for (mclk_idx = 0; mclk_idx < ARRAY_SIZE(mclk_psc_data); mclk_idx++)
 		clk_prepare_enable(clks[MPC512x_CLK_PSC0 + mclk_idx]);
 	clk_prepare_enable(clks[MPC512x_CLK_BDLC]);
Signature · 1 line
1.7.10.4
#171 lore raw

PATCH v4 20/31 spi: mpc512x: switch to CCF names in clock lookup

In reply to Gerhard Sittig

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

acquire and release the 'ipg' clock for register access as well

---
 drivers/spi/spi-mpc512x-psc.c |   30 +++++++++++++++++++++---------
 1 file changed, 21 insertions(+), 9 deletions(-)
drivers/spi/spi-mpc512x-psc.c +21 -9
index 85581f3..daedee7 100644
@@ -38,6 +38,7 @@ struct mpc512x_psc_spi {
 	struct mpc512x_psc_fifo __iomem *fifo;
 	unsigned int irq;
 	u8 bits_per_word;
+	struct clk *clk_ipg;
 	struct clk *clk_mclk;
 	u32 mclk_rate;
 
@@ -474,8 +475,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];
 	struct clk *clk;
 
 	master = spi_alloc_master(dev, sizeof *mps);
@@ -519,29 +518,41 @@ 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);
-	clk = devm_clk_get(dev, clk_name);
-	if (IS_ERR(clk))
+	clk = devm_clk_get(dev, "ipg");
+	if (IS_ERR(clk)) {
+		ret = PTR_ERR(clk);
 		goto free_irq;
+	}
 	ret = clk_prepare_enable(clk);
 	if (ret)
 		goto free_irq;
+	mps->clk_ipg = clk;
+
+	clk = devm_clk_get(dev, "mclk");
+	if (IS_ERR(clk)) {
+		ret = PTR_ERR(clk);
+		goto free_ipg_clock;
+	}
+	ret = clk_prepare_enable(clk);
+	if (ret)
+		goto free_ipg_clock;
 	mps->clk_mclk = clk;
 	mps->mclk_rate = clk_get_rate(clk);
 
 	ret = mpc512x_psc_spi_port_config(master, mps);
 	if (ret < 0)
-		goto free_clock;
+		goto free_mclk_clock;
 
 	ret = spi_register_master(master);
 	if (ret < 0)
-		goto free_clock;
+		goto free_mclk_clock;
 
 	return ret;
 
-free_clock:
+free_mclk_clock:
 	clk_disable_unprepare(mps->clk_mclk);
+free_ipg_clock:
+	clk_disable_unprepare(mps->clk_ipg);
 free_irq:
 	free_irq(mps->irq, mps);
 free_master:
@@ -559,6 +570,7 @@ static int mpc512x_psc_spi_do_remove(struct device *dev)
 
 	spi_unregister_master(master);
 	clk_disable_unprepare(mps->clk_mclk);
+	clk_disable_unprepare(mps->clk_ipg);
 	free_irq(mps->irq, mps);
 	if (mps->psc)
 		iounmap(mps->psc);
Signature · 1 line
1.7.10.4
#172 lore raw

PATCH v4 21/31 serial: mpc512x: switch to CCF names in clock lookup

In reply to Gerhard Sittig

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

acquire and release the 'ipg' clock for register access as well

---
 drivers/tty/serial/mpc52xx_uart.c |   45 ++++++++++++++++++++++++++++++++-----
 1 file changed, 40 insertions(+), 5 deletions(-)
drivers/tty/serial/mpc52xx_uart.c +40 -5
index 5be1df3..f4d2b70 100644
@@ -618,30 +618,58 @@ static irqreturn_t mpc512x_psc_handle_irq(struct uart_port *port)
 	return IRQ_NONE;
 }
 
+static struct clk *psc_ipg_clk[MPC52xx_PSC_MAXNUM];
 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;
 
 	psc_num = (port->mapbase & 0xf00) >> 8;
-	snprintf(clk_name, sizeof(clk_name), "psc%d_mclk", psc_num);
-	clk = devm_clk_get(port->dev, clk_name);
+
+	clk = devm_clk_get(port->dev, "ipg");
+	if (IS_ERR(clk)) {
+		dev_err(port->dev, "Failed to get IPG clock!\n");
+		err = PTR_ERR(clk);
+		goto out_err;
+	}
+	err = clk_prepare_enable(clk);
+	if (err) {
+		dev_err(port->dev, "Failed to enable IPG clock!\n");
+		goto out_err;
+	}
+	psc_ipg_clk[psc_num] = clk;
+
+	clk = devm_clk_get(port->dev, "mclk");
 	if (IS_ERR(clk)) {
 		dev_err(port->dev, "Failed to get MCLK!\n");
-		return PTR_ERR(clk);
+		err = PTR_ERR(clk);
+		goto out_err;
 	}
 	err = clk_prepare_enable(clk);
 	if (err) {
 		dev_err(port->dev, "Failed to enable MCLK!\n");
-		return err;
+		goto out_err;
 	}
 	psc_mclk_clk[psc_num] = clk;
+
 	return 0;
+
+out_err:
+	clk = psc_mclk_clk[psc_num];
+	if (clk) {
+		clk_disable_unprepare(clk);
+		psc_mclk_clk[psc_num] = NULL;
+	}
+	clk = psc_ipg_clk[psc_num];
+	if (clk) {
+		clk_disable_unprepare(clk);
+		psc_ipg_clk[psc_num] = NULL;
+	}
+	return err;
 }
 
 /* called from within the .release_port() callback (release) */
@@ -651,11 +679,18 @@ static void mpc512x_psc_relse_clock(struct uart_port *port)
 	struct clk *clk;
 
 	psc_num = (port->mapbase & 0xf00) >> 8;
+
 	clk = psc_mclk_clk[psc_num];
 	if (clk) {
 		clk_disable_unprepare(clk);
 		psc_mclk_clk[psc_num] = NULL;
 	}
+
+	clk = psc_ipg_clk[psc_num];
+	if (clk) {
+		clk_disable_unprepare(clk);
+		psc_ipg_clk[psc_num] = NULL;
+	}
 }
 
 /* implementation of the .clock() callback (enable/disable) */
Signature · 1 line
1.7.10.4
#173 lore raw

PATCH v4 21/31 serial: mpc512x: switch to CCF names in clock lookup

Quoted 7 lines · Gerhard Sittig

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

acquire and release the 'ipg' clock for register access as well

#174 lore raw

PATCH v4 22/31 clk: mpc512x: remove PSC (UART, SPI) compat workarounds

In reply to Gerhard Sittig

after the UART and SPI peripheral drivers have switched to device tree based clock lookup and no longer construct clock names from their PSC component index, the "psc%d_mclk" alias names have become obsolete -- remove the corresponding clk_register_clkdev() calls

after the UART and SPI peripheral drivers acquire the 'ipg' clock item, the platform's clock driver need no longer pre-enable it

---
 arch/powerpc/platforms/512x/clock-commonclk.c |   13 -------------
 1 file changed, 13 deletions(-)
arch/powerpc/platforms/512x/clock-commonclk.c +0 -13
index 3f559fe..bd704b3 100644
@@ -500,17 +500,6 @@ static void mpc512x_clk_setup_mclk(struct mclk_setup_data *entry, size_t idx)
 				entry->parent_names_mux1[0],
 				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_clock_tree(int busfreq)
@@ -686,8 +675,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_DIU]);	/* display */
-	for (mclk_idx = 0; mclk_idx < ARRAY_SIZE(mclk_psc_data); mclk_idx++)
-		clk_prepare_enable(clks[MPC512x_CLK_PSC0 + mclk_idx]);
 	clk_prepare_enable(clks[MPC512x_CLK_BDLC]);
 	for (mclk_idx = 0; mclk_idx < ARRAY_SIZE(mclk_mscan_data); mclk_idx++)
 		clk_prepare_enable(clks[MPC512x_CLK_MSCAN0_MCLK + mclk_idx]);
Signature · 1 line
1.7.10.4
#175 lore raw

PATCH v4 23/31 serial: mpc512x: setup the PSC FIFO clock as well

In reply to Gerhard Sittig

prepare and enable the FIFO clock upon PSC FIFO initialization, check for and propagage errors when enabling the PSC FIFO clock, disable and unprepare the FIFO clock upon PSC FIFO uninitialization, remove the pre-enable workaround from the platform's clock driver

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

---
 arch/powerpc/platforms/512x/clock-commonclk.c |    2 --
 drivers/tty/serial/mpc52xx_uart.c             |   46 +++++++++++++++++++++----
 2 files changed, 40 insertions(+), 8 deletions(-)
arch/powerpc/platforms/512x/clock-commonclk.c +0 -2
index bd704b3..803f3be 100644
@@ -670,8 +670,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_DIU]);	/* display */
drivers/tty/serial/mpc52xx_uart.c +40 -6

82 lines not on this page · raw message

Signature · 1 line
1.7.10.4
#176 lore raw

PATCH v4 23/31 serial: mpc512x: setup the PSC FIFO clock as well

Quoted 10 lines · Gerhard Sittig

prepare and enable the FIFO clock upon PSC FIFO initialization, check for and propagage errors when enabling the PSC FIFO clock, disable and unprepare the FIFO clock upon PSC FIFO uninitialization, remove the pre-enable workaround from the platform's clock driver

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

#177 lore raw

PATCH v4 24/31 USB: fsl-mph-dr-of: remove now obsolete clock lookup name

In reply to Gerhard Sittig

after device tree based clock lookup became available, the peripheral driver need no longer construct clock names which include the component index -- remove the "usb%d_clk" template and unconditionally use "per", remove the clock driver's clkdev registration

---
 arch/powerpc/platforms/512x/clock-commonclk.c |    2 --
 drivers/usb/host/fsl-mph-dr-of.c              |   13 +------------
 2 files changed, 1 insertion(+), 14 deletions(-)
arch/powerpc/platforms/512x/clock-commonclk.c +0 -2
index 803f3be..4615fad 100644
@@ -658,8 +658,6 @@ static void mpc512x_clk_setup_clock_tree(int busfreq)
 	clk_register_clkdev(clks[MPC512x_CLK_SYS], "sys_clk", NULL);
 	clk_register_clkdev(clks[MPC512x_CLK_VIU], "viu_clk", NULL);
 	clk_register_clkdev(clks[MPC512x_CLK_NFC], "nfc_clk", NULL);
-	clk_register_clkdev(clks[MPC512x_CLK_USB1], "usb1_clk", NULL);
-	clk_register_clkdev(clks[MPC512x_CLK_USB2], "usb2_clk", NULL);
 
 	/* enable some of the clocks here unconditionally because ... */
 	pr_debug("automatically enabling some clocks\n");
drivers/usb/host/fsl-mph-dr-of.c +1 -12
index b8a1866..2b323fb 100644
@@ -261,19 +261,8 @@ int fsl_usb2_mpc5121_init(struct platform_device *pdev)
 	struct fsl_usb2_platform_data *pdata = pdev->dev.platform_data;
 	struct clk *clk;
 	int err;
-	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;
 
-	snprintf(clk_name, sizeof(clk_name), "usb%d_clk", clk_num);
-	clk = devm_clk_get(&pdev->dev, clk_name);
+	clk = devm_clk_get(&pdev->dev, "per");
 	if (IS_ERR(clk)) {
 		dev_err(&pdev->dev, "failed to get clk\n");
 		return PTR_ERR(clk);
Signature · 1 line
1.7.10.4
#178 lore raw

PATCH v4 25/31 mtd: mpc5121_nfc: remove now obsolete clock lookup name

In reply to Gerhard Sittig

after device tree based clock lookup became available, the NAND flash driver need no longer use the previous "nfc_clk" name but can switch to the fixed "per" clock name -- adjust the peripheral driver and remove the clock driver's clkdev registration

---
 arch/powerpc/platforms/512x/clock-commonclk.c |    1 -
 drivers/mtd/nand/mpc5121_nfc.c                |    2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)
arch/powerpc/platforms/512x/clock-commonclk.c +0 -1
index 4615fad..0e23b45 100644
@@ -657,7 +657,6 @@ static void mpc512x_clk_setup_clock_tree(int busfreq)
 	clk_register_clkdev(clks[MPC512x_CLK_REF], "ref_clk", NULL);
 	clk_register_clkdev(clks[MPC512x_CLK_SYS], "sys_clk", NULL);
 	clk_register_clkdev(clks[MPC512x_CLK_VIU], "viu_clk", NULL);
-	clk_register_clkdev(clks[MPC512x_CLK_NFC], "nfc_clk", NULL);
 
 	/* enable some of the clocks here unconditionally because ... */
 	pr_debug("automatically enabling some clocks\n");
drivers/mtd/nand/mpc5121_nfc.c +1 -1
index 3c60a00..e541ec0 100644
@@ -729,7 +729,7 @@ static int mpc5121_nfc_probe(struct platform_device *op)
 	of_node_put(rootnode);
 
 	/* Enable NFC clock */
-	clk = devm_clk_get(dev, "nfc_clk");
+	clk = devm_clk_get(dev, "per");
 	if (IS_ERR(clk)) {
 		dev_err(dev, "Unable to acquire NFC clock!\n");
 		retval = PTR_ERR(clk);
Signature · 1 line
1.7.10.4
#179 lore raw

PATCH v4 26/31 fsl-viu: remove now obsolete clock lookup name

In reply to Gerhard Sittig

after device tree based clock lookup became available, the VIU driver need no longer use the previous "viu_clk" name but can switch to the fixed "per" clock name -- adjust the peripheral driver and remove the clock driver's clkdev registration

---
 arch/powerpc/platforms/512x/clock-commonclk.c |    1 -
 drivers/media/platform/fsl-viu.c              |    2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)
arch/powerpc/platforms/512x/clock-commonclk.c +0 -1
index 0e23b45..e5e9cbe 100644
@@ -656,7 +656,6 @@ static void mpc512x_clk_setup_clock_tree(int busfreq)
 	/* clkdev registration for compatibility reasons */
 	clk_register_clkdev(clks[MPC512x_CLK_REF], "ref_clk", NULL);
 	clk_register_clkdev(clks[MPC512x_CLK_SYS], "sys_clk", NULL);
-	clk_register_clkdev(clks[MPC512x_CLK_VIU], "viu_clk", NULL);
 
 	/* enable some of the clocks here unconditionally because ... */
 	pr_debug("automatically enabling some clocks\n");
drivers/media/platform/fsl-viu.c +1 -1
index fe9898c..1b1a070 100644
@@ -1578,7 +1578,7 @@ static int viu_of_probe(struct platform_device *op)
 	}
 
 	/* enable VIU clock */
-	clk = devm_clk_get(&op->dev, "viu_clk");
+	clk = devm_clk_get(&op->dev, "per");
 	if (IS_ERR(clk)) {
 		dev_err(&op->dev, "failed to lookup the clock!\n");
 		ret = PTR_ERR(clk);
Signature · 1 line
1.7.10.4
#180 lore raw

PATCH v4 27/31 net: can: mscan: add common clock support for mpc512x

In reply to Gerhard Sittig

implement a .get_clock() callback for the MPC512x platform which uses the common clock infrastructure (eliminating direct access to the clock control registers from within the CAN network driver), and provide the corresponding .put_clock() callback to release resources after use

keep the previous implementation of MPC512x support in place during migration, since common clock support is optional

acquire both the 'ipg' as well as "can" clock items in the mscan driver, remove pre-enable workarounds for the BDLC (CAN related 'ipg') as well as the CAN related MCLK (bitrate) items in the platform's clock driver

this change is neutral to the MPC5200 platform

---
 arch/powerpc/platforms/512x/clock-commonclk.c |    3 -
 drivers/net/can/mscan/mpc5xxx_can.c           |  179 +++++++++++++++++++++++++
 2 files changed, 179 insertions(+), 3 deletions(-)
arch/powerpc/platforms/512x/clock-commonclk.c +0 -3
index e5e9cbe..afbb01d 100644
@@ -669,9 +669,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_DIU]);	/* display */
-	clk_prepare_enable(clks[MPC512x_CLK_BDLC]);
-	for (mclk_idx = 0; mclk_idx < ARRAY_SIZE(mclk_mscan_data); mclk_idx++)
-		clk_prepare_enable(clks[MPC512x_CLK_MSCAN0_MCLK + mclk_idx]);
 }
 
 /*
drivers/net/can/mscan/mpc5xxx_can.c +179 -0

208 lines not on this page · raw message

Signature · 1 line
1.7.10.4
#181 lore raw

PATCH v4 28/31 powerpc/mpc512x: improve DIU related clock setup

In reply to Gerhard Sittig

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(-)
arch/powerpc/platforms/512x/clock-commonclk.c +0 -1
index afbb01d..fa95814 100644
@@ -668,7 +668,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_DIU]);	/* display */
 }
 
 /*
arch/powerpc/platforms/512x/mpc512x_shared.c +88 -77

192 lines not on this page · raw message

Signature · 1 line
1.7.10.4
#182 lore raw

PATCH v4 29/31 clk: mpc512x: switch to COMMON_CLK, remove PPC_CLOCK

In reply to Gerhard Sittig

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
arch/powerpc/platforms/512x/Kconfig +1 -13
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
arch/powerpc/platforms/512x/Makefile +1 -2
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
arch/powerpc/platforms/512x/clock.c deleted +0 -753

753 lines not on this page · raw message

Signature · 1 line
1.7.10.4
#183 lore raw

PATCH v4 30/31 net: can: mscan: remove non-CCF code for MPC512x

In reply to Gerhard Sittig

transition to the common clock framework has completed and the PPC_CLOCK is no longer available 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 |  141 -----------------------------------
 1 file changed, 141 deletions(-)
drivers/net/can/mscan/mpc5xxx_can.c +0 -141
index f48f129..6b0c995 100644
@@ -109,9 +109,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)
 {
@@ -277,144 +274,6 @@ static void mpc512x_can_put_clock(struct platform_device *ofdev)
 	if (priv->clk_ipg)
 		clk_disable_unprepare(priv->clk_ipg);
 }
-
-#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 = devm_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 = devm_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;
-}
-
-#define mpc512x_can_put_clock NULL
-
-#endif	/* COMMON_CLK */
-
 #else /* !CONFIG_PPC_MPC512x */
 static u32 mpc512x_can_get_clock(struct platform_device *ofdev,
 				 const char *clock_name, int *mscan_clksrc)
Signature · 1 line
1.7.10.4
#184 lore raw

PATCH v4 31/31 clk: mpc512x: remove remaining migration workarounds

In reply to Gerhard Sittig

remove the last clkdev registration call ("sys_clk" and "ref_clk" for CAN), as well as the clkdev header inclusion and the "compat registration" comment

all client lookups for clock items are device tree based now, no compatibility alias names need to get provided any longer

remove the now obsolete TODO comment on trimming the list of pre-enabled clock items, after all the respective peripheral drivers were adjusted

---
 arch/powerpc/platforms/512x/clock-commonclk.c |    7 -------
 1 file changed, 7 deletions(-)
arch/powerpc/platforms/512x/clock-commonclk.c +0 -7
index fa95814..621a8d9 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>
@@ -522,8 +521,6 @@ static void mpc512x_clk_setup_clock_tree(int busfreq)
 	 *   specific and cannot get mapped to componsites (at least not
 	 *   a single one, maybe two of them, but see the comment about
 	 *   "intermediates are referenced from elsewhere, too")
-	 * - trim the list of auto-enabled clocks after drivers acquire
-	 *   them correctly as needed
 	 */
 
 	/* regardless of whether XTAL/OSC exists, have REF created */
@@ -653,10 +650,6 @@ static void mpc512x_clk_setup_clock_tree(int busfreq)
 	/* fixed frequency for AC97, always 24.567MHz */
 	clks[MPC512x_CLK_AC97] = mpc512x_clk_fixed("ac97", 24567000);
 
-	/* clkdev registration for compatibility reasons */
-	clk_register_clkdev(clks[MPC512x_CLK_REF], "ref_clk", NULL);
-	clk_register_clkdev(clks[MPC512x_CLK_SYS], "sys_clk", NULL);
-
 	/* enable some of the clocks here unconditionally because ... */
 	pr_debug("automatically enabling some clocks\n");
 	/* some are essential yet never get claimed by any driver */
Signature · 1 line
1.7.10.4
#185 lore raw

v4 cover add COMMON_CLK support for PowerPC MPC512x

In reply to Gerhard Sittig

[ this is an overview on how to split the series if necessary ]

Quoted 10 lines · Gerhard Sittig

this series
- fixes several drivers that are used in the MPC512x platform (UART, SPI, ethernet, PCI, USB, CAN, NAND flash, video capture) in how they handle clocks (appropriately acquire and setup them, hold references during use, release clocks after use)
- introduces support for the common clock framework (CCF, COMMON_CLK Kconfig option) in the PowerPC based MPC512x platform, which brings device tree based clock lookup as well

Quoted 7 lines

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

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

It was suggested that the series may not be kept together when applying, i.e. that individual patches may need to take separate routes through individual subsystem trees.

Here is how the parts of the series interact:
- parts 01-14 are apparent fixes and improvements to existing drivers, they may immediately get applied via their respective subsystems when they are considered acceptable in review (number 13 already was taken into clk-next)
- parts 15-16 are device tree improvements (providing previously lacking information on the hardware, plus introducing support for symbolic references in DT nodes), and can get applied immediately via their respective subsystem
- part 17 introduces the platform clock driver which provides common clock support (uses shared drivers and common infrastructure, is an OF clock provider) -- this patch introduces _and_ enables the driver which strictly depends on the device tree update from part 16, and when common clock support gets enabled all peripheral drivers shall have received their clock API use cleanup
- part 18 introduces clock specs in the device tree which is used for client side lookups (from within peripheral drivers), this part must not get applied before the OF clock provider from part 17
- parts 19-21 and 23-28 further adjust peripheral drivers _after_ the common clock support and OF clock lookup have become available, they strictly depend on parts 17 and 18, finally bring peripheral drivers into the common clock world and remove workarounds in the backend which support migration
- parts 22 and 29-31 finally remove remaining compat and migration stuff that has become obsolete after everything else was adjusted (I'd happily re-spin the series to move 22 to the end if eliminating this seemingly out-of-order item helps, currently it's at that position since it finishes what patch 20 and 21 have prepared, while I did not dare to squash SPI and UART related patches since they reside in different subsystems)

The above results in the following stages, when the series needs to get split:
- parts 01-16 could be taken immediately and need not be kept in any order, as they don't depend on each other
- parts 17-18 could be taken after 01-16 all are in place (while 18 requires 17)
- parts 19-28 (see the remark on 22) could be taken after 17 and 18 are in place (22 depends on 20 and 21 but may get moved to any later position at will, all other parts don't depend on each other and can get applied in any order)
- parts 29-31 (and 22 if not applied so far) could be taken after all parts up to 28 were applied

Please consider that I'm not trying to be pushy :) but that I want to provide all the possible help and information on how the series might get applied, assuming that it will pass review. So far there has not been objection, all feedback got addressed, while explicit ACKs still are pending.

For your reference here is the stats and the (numbered) list of files again:

0000-cover-letter.patch 0001-spi-mpc512x-cleanup-clock-API-use.patch 0002-serial-mpc512x-cleanup-clock-API-use.patch 0003-USB-fsl-mph-dr-of-cleanup-clock-API-use.patch 0004-mtd-mpc5121_nfc-cleanup-clock-API-use.patch 0005-media-fsl-viu-cleanup-clock-API-use.patch 0006-i2c-mpc-cleanup-clock-API-use.patch 0007-fs_enet-silence-a-build-warning-unused-variable.patch 0008-fs_enet-cleanup-clock-API-use.patch 0009-powerpc-fsl-pci-improve-clock-API-use.patch 0010-net-can-mscan-add-a-comment-on-reg-to-idx-mapping.patch 0011-net-can-mscan-improve-clock-API-use.patch 0012-powerpc-mpc512x-array-decl-for-MCLK-registers-in-CCM.patch 0013-clk-wrap-I-O-access-for-improved-portability.patch 0014-dts-mpc512x-prepare-for-preprocessor-support.patch 0015-dts-mpc512x-introduce-dt-bindings-clock-header.patch 0016-dts-mpc512x-add-clock-related-device-tree-specs.patch 0017-clk-mpc512x-introduce-COMMON_CLK-for-MPC512x.patch 0018-dts-mpc512x-add-clock-specs-for-client-lookups.patch 0019-clk-mpc512x-don-t-pre-enable-FEC-and-I2C-clocks.patch 0020-spi-mpc512x-switch-to-CCF-names-in-clock-lookup.patch 0021-serial-mpc512x-switch-to-CCF-names-in-clock-lookup.patch 0022-clk-mpc512x-remove-PSC-UART-SPI-compat-workarounds.patch 0023-serial-mpc512x-setup-the-PSC-FIFO-clock-as-well.patch 0024-USB-fsl-mph-dr-of-remove-now-obsolete-clock-lookup-n.patch 0025-mtd-mpc5121_nfc-remove-now-obsolete-clock-lookup-nam.patch 0026-media-fsl-viu-remove-now-obsolete-clock-lookup-name.patch 0027-net-can-mscan-add-common-clock-support-for-mpc512x.patch 0028-powerpc-mpc512x-improve-DIU-related-clock-setup.patch 0029-clk-mpc512x-switch-to-COMMON_CLK-remove-PPC_CLOCK.patch 0030-net-can-mscan-remove-non-CCF-code-for-MPC512x.patch 0031-clk-mpc512x-remove-remaining-migration-workarounds.patch

Quoted 58 lines

Gerhard Sittig (31):
spi: mpc512x: cleanup clock API use
serial: mpc512x: cleanup clock API use
USB: fsl-mph-dr-of: cleanup clock API use
mtd: mpc5121_nfc: cleanup clock API use
[media] fsl-viu: cleanup clock API use
i2c: mpc: cleanup clock API use
fs_enet: silence a build warning (unused variable)
fs_enet: cleanup clock API use
powerpc/fsl-pci: improve clock API use
net: can: mscan: add a comment on reg to idx mapping
net: can: mscan: improve clock API use
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
clk: mpc512x: don't pre-enable FEC and I2C clocks
spi: mpc512x: switch to CCF names in clock lookup
serial: mpc512x: switch to CCF names in clock lookup
clk: mpc512x: remove PSC (UART, SPI) compat workarounds
serial: mpc512x: setup the PSC FIFO clock as well
USB: fsl-mph-dr-of: remove now obsolete clock lookup name
mtd: mpc5121_nfc: remove now obsolete clock lookup name [media] fsl-viu: remove now obsolete clock lookup name
net: can: mscan: add common clock support for mpc512x powerpc/mpc512x: improve DIU related clock setup
clk: mpc512x: switch to COMMON_CLK, remove PPC_CLOCK
net: can: mscan: remove non-CCF code for MPC512x
clk: mpc512x: remove remaining migration workarounds

 arch/powerpc/boot/dts/ac14xx.dts                   |    9 +-
 arch/powerpc/boot/dts/include/dt-bindings          |    1 +
 arch/powerpc/boot/dts/mpc5121.dtsi                 |  110 ++-
 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      |  716 +++++++++++++++++++
 arch/powerpc/platforms/512x/clock.c                |  753 --------------------
 arch/powerpc/platforms/512x/mpc512x_shared.c       |  165 +++--
 arch/powerpc/sysdev/fsl_pci.c                      |   22 +
 drivers/clk/clk-divider.c                          |    6 +-
 drivers/clk/clk-gate.c                             |    6 +-
 drivers/clk/clk-mux.c                              |    6 +-
 drivers/i2c/busses/i2c-mpc.c                       |   24 +
 drivers/media/platform/fsl-viu.c                   |   23 +-
 drivers/mtd/nand/mpc5121_nfc.c                     |   21 +-
 drivers/net/can/mscan/mpc5xxx_can.c                |  281 +++++---
 drivers/net/can/mscan/mscan.c                      |   27 +-
 drivers/net/can/mscan/mscan.h                      |    3 +
 .../net/ethernet/freescale/fs_enet/fs_enet-main.c  |   21 +-
 drivers/spi/spi-mpc512x-psc.c                      |   60 +-
 drivers/tty/serial/mpc52xx_uart.c                  |  179 ++++-
 drivers/usb/host/fsl-mph-dr-of.c                   |   27 +-
Quoted 8 lines
 include/dt-bindings/clock/mpc512x-clock.h          |   69 ++
 include/linux/clk-provider.h                       |   33 +
 include/linux/fs_enet_pd.h                         |    3 +
 28 files changed, 1534 insertions(+), 1058 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

virtually yours
Gerhard Sittig

Signature · 3 lines
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
#186 lore raw

v4 cover add COMMON_CLK support for PowerPC MPC512x

[ no change in content, but order shuffling for better application ]

Quoted 59 lines · Gerhard Sittig

[ this is an overview on how to split the series if necessary ]

Quoted 10 lines · Gerhard Sittig

this series
- fixes several drivers that are used in the MPC512x platform (UART, SPI, ethernet, PCI, USB, CAN, NAND flash, video capture) in how they handle clocks (appropriately acquire and setup them, hold references during use, release clocks after use)
- introduces support for the common clock framework (CCF, COMMON_CLK Kconfig option) in the PowerPC based MPC512x platform, which brings device tree based clock lookup as well

Quoted 7 lines

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

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

It was suggested that the series may not be kept together when applying, i.e. that individual patches may need to take separate routes through individual subsystem trees.

Here is how the parts of the series interact:
- parts 01-14 are apparent fixes and improvements to existing drivers, they may immediately get applied via their respective subsystems when they are considered acceptable in review (number 13 already was taken into clk-next)
- parts 15-16 are device tree improvements (providing previously lacking information on the hardware, plus introducing support for symbolic references in DT nodes), and can get applied immediately via their respective subsystem
- part 17 introduces the platform clock driver which provides common clock support (uses shared drivers and common infrastructure, is an OF clock provider) -- this patch introduces _and_ enables the driver which strictly depends on the device tree update from part 16, and when common clock support gets enabled all peripheral drivers shall have received their clock API use cleanup
- part 18 introduces clock specs in the device tree which is used for client side lookups (from within peripheral drivers), this part must not get applied before the OF clock provider from part 17
- parts 19-21 and 23-28 further adjust peripheral drivers _after_ the common clock support and OF clock lookup have become available, they strictly depend on parts 17 and 18, finally bring peripheral drivers into the common clock world and remove workarounds in the backend which support migration
- parts 22 and 29-31 finally remove remaining compat and migration stuff that has become obsolete after everything else was adjusted (I'd happily re-spin the series to move 22 to the end if eliminating this seemingly out-of-order item helps, currently it's at that position since it finishes what patch 20 and 21 have prepared, while I did not dare to squash SPI and UART related patches since they reside in different subsystems)

Quoted 12 lines

The above results in the following stages, when the series needs to get split:
- parts 01-16 could be taken immediately and need not be kept in any order, as they don't depend on each other
- parts 17-18 could be taken after 01-16 all are in place (while 18 requires 17)
- parts 19-28 (see the remark on 22) could be taken after 17 and 18 are in place (22 depends on 20 and 21 but may get moved to any later position at will, all other parts don't depend on each other and can get applied in any order)
- parts 29-31 (and 22 if not applied so far) could be taken after all parts up to 28 were applied

Here I erred. The order of the patches within phase 3 appears to actually matter. I'm sorry for bothering you again.

While the _content_ of the patches (which manipulation gets done to the source) is correct and independent from neighbour patches, yet a patch in that third phase will change the _context_ of subsequent patches. The sum of all the patches applied will be identical again, but changed order of application may cause conflicts (regardless of how simple to resolve they may be).

Build and run tests and the organization of v4 (immediate removal upon driver adjustment) have shown that removing a migration workaround in the very moment where driver adjustment obsoletes it keeps the system operational. I consider this a very positive feature, and would only rearrange the order if the series' content was ACKed in complete.

Moving removal of the workarounds to a later point in time won't affect operation (just leaves the workaround in place a little longer, while it does not harm), and will eliminate potential for conflicts. This I would do when accepting the series was decided, to not lose the precious test feature during development.

A byproduct of this rearrangement is that patches in phase 3 won't any longer touch a peripheral driver _and_ the clock driver within a single patch, for whatever that may be worth.

So I suggest the following:

I can move parts 19 and 22 (completely) and the "workaround removal" aspect of parts 20-21 and 23-28 to a later point in the sequence, maybe squash them with part 31 (cleanup after migration has completed).

This allows for even better (conflict free) application of the series through individual and most appropriate trees. All of this of course assumes that the series passes review, and that you agree on the outlined application in four phases if the series cannot be kept together:
- clock API use cleanup for peripheral drivers, and device tree update to provide previously missing hardware information
- introduction of the platform clock driver and OF clock lookup
- adjustment of peripheral drivers after common clock support has become available
- removal of migration support and compat workarounds after migration has completed

Quoted 33 lines

Please consider that I'm not trying to be pushy :) but that I want to provide all the possible help and information on how the series might get applied, assuming that it will pass review. So far there has not been objection, all feedback got addressed, while explicit ACKs still are pending.

For your reference here is the stats and the (numbered) list of files again:

0000-cover-letter.patch 0001-spi-mpc512x-cleanup-clock-API-use.patch 0002-serial-mpc512x-cleanup-clock-API-use.patch 0003-USB-fsl-mph-dr-of-cleanup-clock-API-use.patch 0004-mtd-mpc5121_nfc-cleanup-clock-API-use.patch 0005-media-fsl-viu-cleanup-clock-API-use.patch 0006-i2c-mpc-cleanup-clock-API-use.patch 0007-fs_enet-silence-a-build-warning-unused-variable.patch 0008-fs_enet-cleanup-clock-API-use.patch 0009-powerpc-fsl-pci-improve-clock-API-use.patch 0010-net-can-mscan-add-a-comment-on-reg-to-idx-mapping.patch 0011-net-can-mscan-improve-clock-API-use.patch 0012-powerpc-mpc512x-array-decl-for-MCLK-registers-in-CCM.patch 0013-clk-wrap-I-O-access-for-improved-portability.patch 0014-dts-mpc512x-prepare-for-preprocessor-support.patch 0015-dts-mpc512x-introduce-dt-bindings-clock-header.patch 0016-dts-mpc512x-add-clock-related-device-tree-specs.patch 0017-clk-mpc512x-introduce-COMMON_CLK-for-MPC512x.patch 0018-dts-mpc512x-add-clock-specs-for-client-lookups.patch 0019-clk-mpc512x-don-t-pre-enable-FEC-and-I2C-clocks.patch 0020-spi-mpc512x-switch-to-CCF-names-in-clock-lookup.patch 0021-serial-mpc512x-switch-to-CCF-names-in-clock-lookup.patch 0022-clk-mpc512x-remove-PSC-UART-SPI-compat-workarounds.patch 0023-serial-mpc512x-setup-the-PSC-FIFO-clock-as-well.patch

Quoted 75 lines

0024-USB-fsl-mph-dr-of-remove-now-obsolete-clock-lookup-n.patch 0025-mtd-mpc5121_nfc-remove-now-obsolete-clock-lookup-nam.patch 0026-media-fsl-viu-remove-now-obsolete-clock-lookup-name.patch 0027-net-can-mscan-add-common-clock-support-for-mpc512x.patch 0028-powerpc-mpc512x-improve-DIU-related-clock-setup.patch 0029-clk-mpc512x-switch-to-COMMON_CLK-remove-PPC_CLOCK.patch 0030-net-can-mscan-remove-non-CCF-code-for-MPC512x.patch 0031-clk-mpc512x-remove-remaining-migration-workarounds.patch

Quoted 66 lines

Gerhard Sittig (31):
spi: mpc512x: cleanup clock API use
serial: mpc512x: cleanup clock API use
USB: fsl-mph-dr-of: cleanup clock API use
mtd: mpc5121_nfc: cleanup clock API use
[media] fsl-viu: cleanup clock API use
i2c: mpc: cleanup clock API use
fs_enet: silence a build warning (unused variable)
fs_enet: cleanup clock API use
powerpc/fsl-pci: improve clock API use
net: can: mscan: add a comment on reg to idx mapping
net: can: mscan: improve clock API use
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
clk: mpc512x: don't pre-enable FEC and I2C clocks
spi: mpc512x: switch to CCF names in clock lookup
serial: mpc512x: switch to CCF names in clock lookup
clk: mpc512x: remove PSC (UART, SPI) compat workarounds
serial: mpc512x: setup the PSC FIFO clock as well
USB: fsl-mph-dr-of: remove now obsolete clock lookup name
mtd: mpc5121_nfc: remove now obsolete clock lookup name [media] fsl-viu: remove now obsolete clock lookup name
net: can: mscan: add common clock support for mpc512x powerpc/mpc512x: improve DIU related clock setup
clk: mpc512x: switch to COMMON_CLK, remove PPC_CLOCK
net: can: mscan: remove non-CCF code for MPC512x
clk: mpc512x: remove remaining migration workarounds

 arch/powerpc/boot/dts/ac14xx.dts                   |    9 +-
 arch/powerpc/boot/dts/include/dt-bindings          |    1 +
 arch/powerpc/boot/dts/mpc5121.dtsi                 |  110 ++-
 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      |  716 +++++++++++++++++++
 arch/powerpc/platforms/512x/clock.c                |  753 --------------------
 arch/powerpc/platforms/512x/mpc512x_shared.c       |  165 +++--
 arch/powerpc/sysdev/fsl_pci.c                      |   22 +
 drivers/clk/clk-divider.c                          |    6 +-
 drivers/clk/clk-gate.c                             |    6 +-
 drivers/clk/clk-mux.c                              |    6 +-
 drivers/i2c/busses/i2c-mpc.c                       |   24 +
 drivers/media/platform/fsl-viu.c                   |   23 +-
 drivers/mtd/nand/mpc5121_nfc.c                     |   21 +-
 drivers/net/can/mscan/mpc5xxx_can.c                |  281 +++++---
 drivers/net/can/mscan/mscan.c                      |   27 +-
 drivers/net/can/mscan/mscan.h                      |    3 +
 .../net/ethernet/freescale/fs_enet/fs_enet-main.c  |   21 +-
 drivers/spi/spi-mpc512x-psc.c                      |   60 +-
 drivers/tty/serial/mpc52xx_uart.c                  |  179 ++++-
 drivers/usb/host/fsl-mph-dr-of.c                   |   27 +-
 include/dt-bindings/clock/mpc512x-clock.h          |   69 ++
 include/linux/clk-provider.h                       |   33 +
 include/linux/fs_enet_pd.h                         |    3 +
 28 files changed, 1534 insertions(+), 1058 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

virtually yours
Gerhard Sittig

Signature · 3 lines
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
#187 lore raw

v4 cover add COMMON_CLK support for PowerPC MPC512x

In reply to Gerhard Sittig
Quoted 26 lines · Gerhard Sittig

this series
- fixes several drivers that are used in the MPC512x platform (UART, SPI, ethernet, PCI, USB, CAN, NAND flash, video capture) in how they handle clocks (appropriately acquire and setup them, hold references during use, release clocks after use)
- introduces support for the common clock framework (CCF, COMMON_CLK Kconfig option) in the PowerPC based MPC512x platform, which brings device tree based clock lookup as well

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

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 #431: FILE: arch/powerpc/platforms/512x/clock-commonclk.c:334:
+static const char *parent_names_mux0[] = {

Quoted 7 lines

total: 0 errors, 1 warnings, 0 checks, 807 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)

How do the driver changes impact other PPC SoCs that use the same drivers (i2c, fs_enet, usb) ?

- k

#188 lore raw

v4 cover add COMMON_CLK support for PowerPC MPC512x

Quoted 31 lines · Kumar Gala
Quoted 28 lines · Gerhard Sittig

this series
- fixes several drivers that are used in the MPC512x platform (UART, SPI, ethernet, PCI, USB, CAN, NAND flash, video capture) in how they handle clocks (appropriately acquire and setup them, hold references during use, release clocks after use)
- introduces support for the common clock framework (CCF, COMMON_CLK Kconfig option) in the PowerPC based MPC512x platform, which brings device tree based clock lookup as well

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

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 #431: FILE: arch/powerpc/platforms/512x/clock-commonclk.c:334:
+static const char *parent_names_mux0[] = {

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

Quoted 8 lines
Quoted 5 lines

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)

How do the driver changes impact other PPC SoCs that use the same drivers (i2c, fs_enet, usb) ?

For SPI and UART (the PSC component), the hardware is shared between MPC512x and MPC5200, but only routines and data specific to MPC512x get changed.

For USB the "fsl.*usb2" hardware appears to be shared among Freescale SoCs. AFAICS i.MX has a separate driver under arm/, MPC83xx has a separate driver under arch/powerpc/platforms/83xx/, and the driver I'm touching is only changed in routines specific to MPC512x.

The NAND and VIU drivers only attach to hardware on the MPC512x platform (checked the compatible string, only referenced from the mpc5121.dtsi).

I2C, ethernet, PCI all are similar: A non-fatal clock lookup is introduced, CCF platforms (512x only ATM) will carry out appropriate clock operations, non-CCF platforms won't see a change in behaviour (lookup fails which isn't fatal, and the drivers assume that somebody else will have taken care of clocks for them).

MSCAN is shared among 512x and 52xx, the common code introduces transparent yet optional support for CCF, the 512x code path makes use of it, 52xx sees no change in behaviour.

The DIU component (display output) is shared among platforms, but only the platform initialization in the MPC512x code path gets changed to make use of the CCF support, while no other platform sees any change.

The MPC512x common clock core driver does use common primitives and redirects the register access primitives. But the series doesn't change register access for ARM (static inline call to the previous hardcoded routine and thus identical object code), and doesn't modify nor extend the shared code for gates, dividers and multiplexers.

The device tree changes only apply to MPC512x, and only provide hardware related information that formerly was missing.

To summarize, I see no impact for other architectures or platforms. Although it would be good to get a second opinion from persons with USB knowledge, to make sure I haven't missed something.

virtually yours
Gerhard Sittig

Signature · 3 lines
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
#189 lore raw

v4 cover add COMMON_CLK support for PowerPC MPC512x

In reply to Gerhard Sittig

[ summary for the busy or the impatient:
this is a status update on the series
- peripheral driver cleanup considered appropriate for v3.12
- common clock support introduction isn't ready yet
- which in turn holds subsequent parts
- while the overall shape of the series is looking good ]

Quoted 10 lines · Gerhard Sittig

this series
- fixes several drivers that are used in the MPC512x platform (UART, SPI, ethernet, PCI, USB, CAN, NAND flash, video capture) in how they handle clocks (appropriately acquire and setup them, hold references during use, release clocks after use)
- introduces support for the common clock framework (CCF, COMMON_CLK Kconfig option) in the PowerPC based MPC512x platform, which brings device tree based clock lookup as well

Quoted 7 lines

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

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

Since the status of this series was questioned recently, I felt that I should officially and publicly provide a status update in the absence of a v5 submission update.

The series has undergone some review and has received changes as concerns were raised and feedback was provided. While I consider the nature and frequency of the changes totally appropriate -- each revision addressed all of the issues raised, and did so in an appropriate manner, but could not forsee what else would be raised upon re-submission. Actually not sending another version before _all_ concerns are addressed appropriately is what held back submission of v5. See the phase overview below for details.

Adding the cleanup of existing code before the introduction of new features did widen the scope of the series, yet has heavily improved the series, and the feedback was gratefully accepted and thoroughly got addressed.

Actually this driver cleanup, which only was introduced after initial submission upon Mark's request, could be considered the most desirable part of the series at this very point in time. And as I write this, the patches of the "peripheral driver cleanup" phase are being picked up for v3.12 after they have become stable in the review iterations.

Further extension of test coverage for the series after submission of v4 has led to minimal fixes in CAN, USB, and PCI, and has revealed one problem in multi platform configurations which currently is the only remaining blocker for phase 2 and subsequent steps. While phase 1 with its obvious cleanup is stable and has become desirable and acceptable and currently is being picked up.

The current status of the v4 series in detail is:

Phase 1, patches 01-14/31, peripheral driver cleanup and DTS
improvement: has addressed all concerns raised, and can be applied via any subtree in any order since the parts are independent from each other, with a few minor additions

- USB 03/31 received another adjustment of the clock lookup 'dev' parameter, the applied version works in all three cases of the PPC_CLOCK implementation where clock names are global, the CCF implementation with clkdev registration (during migration), and the CCF implementation with device tree based clock lookup (the end result of the series); the v4 patch wasn't broken but just in need of an addendum before/within phase 3, which now was folded into phase 1

- PCI 09/31 had a compile error on 85xx/86xx due to a copy'n'paste bug in an error path; since the (fixed) patch still remains a NOP for now and within the whole series, I have suggested to leave this patch for v3.12, and to address the remaining issue of the PCI driver patch being incomplete later, see the followup for 09/31 for details (what gets added in a future version is another comment in the PCI driver and a workaround in the clock provider backend, because in the given implementation the peripheral driver cannot appropriately acquire its clock item on some platforms)

- CAN 11/31 could save one more instruction by adding another jump label in the error path instead of explicit undo of a setup step, Marc's suggestion was implemented and has been applied

So all parts of phase 1 (with the exception of the PCI driver change which is and remains a NOP) were applied, and followup patches for fixup were avoided. Nothing was broken, no breakage was introduced, it's all about improvements.

Phase 2, patches 15-18/31, introduction of CCF support for
MPC512x: works correctly for MPC512x and doesn't break other platforms, but won't work in multi platform configurations with MPC52xx (PPC_CLOCK and COMMON_CLK will collide in the linker), shall not be considered for v3.12, multi platform needs to get sorted out before consideration for v3.13 (and is the only known issue of the series feature- or policy-wise)

Phase 3, patches 20,21,23-28/31, adoption of peripheral drivers to the CCF world: is complete feature-wise and recently has received even more test coverage than before, remaining fixes got folded into phase 1, patches of phase 3 depend on CCF support which gets introduced in phase 2, and the "workaround removal" aspects of phase 3 will explicitly be moved to phase 4 while the content remains unaffected (mere split and re-order)

Phase 4, patches 19,22,29-31/31, removal of migration support after complete adoption: is complete feature-wise, but partial removal of workarounds and compatibility from phase 3 shall move explicitly to phase 4, to more strictly tell those phases apart and for collision free application via individual subtrees if application through a single tree cannot be done, so a mere re-ordering remains to get communicated while nothing changes in the content (re-ordering the sequence as well as verifying that the patches in phase 3 are independent from each other has already been done internally)

To summarize:
- The series is in a good shape, one multi platform issue needs to get addressed, everything else either is already there or just needs to get communicated.
- Phase 1 with the obvious cleanup is being considered for v3.12, and patches have been queued in their respective subtrees.
- Phase 2 will become acceptable when the multi platform configuration has been sorted out. Each platform works in itself, just not the combination of 52xx and 512x, and actually MPC52xx could be considered the out-lier here (is the only remaining user of PPC_CLOCK, and does so with a dummy implementation in the absence of a real provider).
- Phases 3 and 4 are "complete" but depend on phase 2. What remains is a re-sort of the CCF adjustment and the migration support removal aspects.

Thanks to those involved in the feedback and application so far! In my eyes, changes have been few, and necessary, and always an improvement. Regardless of which potential for further improvement remains, which just happens to be way outside of the scope of the series (power consumption aspects that neither have been addressed nor prepared before, or CCF support for other PowerPC based platforms maybe).

virtually yours
Gerhard Sittig

Signature · 3 lines
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