can: flexcan: add 2nd clock to support imx53 and newer

PATCH New Marc Kleine-Budde 4 participants 7 messages Jul 17, 2012 – Jul 18, 2012 CAN network drivers

lore

Share & tools

lore

Marc Kleine-Budde

author maintainer

2012-07-17 22:05 UTC #0 lore raw

PATCH 1/1 can: flexcan: add 2nd clock to support imx53 and newer

From: Steffen Trumtrar <[email protected]>

This patch add support for a second clock to the flexcan driver. On modern freescale ARM cores like the imx53 and imx6q two clocks ("ipg" and "per") must be enabled in order to access the CAN core.

In the original driver, the clock was requested without specifying the connection id, further all mainline ARM archs with flexcan support (imx28, imx25, imx35) register their flexcan clock without a connection id, too.

This patch first renames the existing clk variable to clk_ipg and adds the connection id "ipg" to the clk_get() call. Then a second clock "per" is requested. As all archs don't specify a connection id, both clk_get return the same clock. This ensures compatibility to existing flexcan support and adds support for imx53 at the same time.

After this patch hits mainline, the archs may give their existing flexcan clock the "ipg" connection id and implement a dummy "per" clock.

This patch has been tested on imx28 (unmodified clk tree) and on imx53 with a seperate "ipg" and "per" clock.

Cc Sascha Hauer <[email protected]>

Cc Shawn Guo <[email protected]>

Cc Hui Wang <[email protected]>

Signed-off-by Steffen Trumtrar <[email protected]>

Signed-off-by Marc Kleine-Budde <[email protected]>

---
 drivers/net/can/flexcan.c |   52 ++++++++++++++++++++++++++++++---------------
 1 file changed, 35 insertions(+), 17 deletions(-)

drivers/net/can/flexcan.c +35 ~~-17~~

index c8a6fc7..281d51f 100644
@@ -190,7 +190,8 @@ struct flexcan_priv {
 	u32 reg_esr;
 	u32 reg_ctrl_default;
 
-	struct clk *clk;
+	struct clk *clk_ipg;
+	struct clk *clk_per;
 	struct flexcan_platform_data *pdata;
 	const struct flexcan_devtype_data *devtype_data;
 };

@@ -828,7 +829,8 @@ static int flexcan_open(struct net_device *dev)
 	struct flexcan_priv *priv = netdev_priv(dev);
 	int err;
 
-	clk_prepare_enable(priv->clk);
+	clk_prepare_enable(priv->clk_ipg);
+	clk_prepare_enable(priv->clk_per);
 
 	err = open_candev(dev);
 	if (err)

@@ -850,7 +852,8 @@ static int flexcan_open(struct net_device *dev)
  out_close:
 	close_candev(dev);
  out:
-	clk_disable_unprepare(priv->clk);
+	clk_disable_unprepare(priv->clk_per);
+	clk_disable_unprepare(priv->clk_ipg);
 
 	return err;
 }

@@ -864,7 +867,8 @@ static int flexcan_close(struct net_device *dev)
 	flexcan_chip_stop(dev);
 
 	free_irq(dev->irq, dev);
-	clk_disable_unprepare(priv->clk);
+	clk_disable_unprepare(priv->clk_per);
+	clk_disable_unprepare(priv->clk_ipg);
 
 	close_candev(dev);

@@ -903,7 +907,8 @@ static int __devinit register_flexcandev(struct net_device *dev)
 	struct flexcan_regs __iomem *regs = priv->base;
 	u32 reg, err;
 
-	clk_prepare_enable(priv->clk);
+	clk_prepare_enable(priv->clk_ipg);
+	clk_prepare_enable(priv->clk_per);
 
 	/* select "bus clock", chip must be disabled */
 	flexcan_chip_disable(priv);

@@ -936,7 +941,8 @@ static int __devinit register_flexcandev(struct net_device *dev)
  out:
 	/* disable core and turn off clocks */
 	flexcan_chip_disable(priv);
-	clk_disable_unprepare(priv->clk);
+	clk_disable_unprepare(priv->clk_per);
+	clk_disable_unprepare(priv->clk_ipg);
 
 	return err;
 }

@@ -964,7 +970,7 @@ static int __devinit flexcan_probe(struct platform_device *pdev)
 	struct net_device *dev;
 	struct flexcan_priv *priv;
 	struct resource *mem;
-	struct clk *clk = NULL;
+	struct clk *clk_ipg = NULL, *clk_per = NULL;
 	struct pinctrl *pinctrl;
 	void __iomem *base;
 	resource_size_t mem_size;

@@ -980,13 +986,20 @@ static int __devinit flexcan_probe(struct platform_device *pdev)
 						"clock-frequency", &clock_freq);
 
 	if (!clock_freq) {
-		clk = clk_get(&pdev->dev, NULL);
-		if (IS_ERR(clk)) {
-			dev_err(&pdev->dev, "no clock defined\n");
-			err = PTR_ERR(clk);
+		clk_ipg = clk_get(&pdev->dev, "ipg");
+		if (IS_ERR(clk_ipg)) {
+			dev_err(&pdev->dev, "no ipg clock defined\n");
+			err = PTR_ERR(clk_ipg);
+			goto failed_clock;
+		}
+		clock_freq = clk_get_rate(clk_ipg);
+
+		clk_per = clk_get(&pdev->dev, "per");
+		if (IS_ERR(clk_per)) {
+			dev_err(&pdev->dev, "no per clock defined\n");
+			err = PTR_ERR(clk_per);
 			goto failed_clock;
 		}
-		clock_freq = clk_get_rate(clk);
 	}
 
 	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);

@@ -1039,7 +1052,8 @@ static int __devinit flexcan_probe(struct platform_device *pdev)
 		CAN_CTRLMODE_BERR_REPORTING;
 	priv->base = base;
 	priv->dev = dev;
-	priv->clk = clk;
+	priv->clk_ipg = clk_ipg;
+	priv->clk_per = clk_per;
 	priv->pdata = pdev->dev.platform_data;
 	priv->devtype_data = devtype_data;

@@ -1067,9 +1081,11 @@ static int __devinit flexcan_probe(struct platform_device *pdev)
  failed_map:
 	release_mem_region(mem->start, mem_size);
  failed_get:
-	if (clk)
-		clk_put(clk);
  failed_clock:
+	if (clk_per)
+		clk_put(clk_per);
+	if (clk_ipg)
+		clk_put(clk_ipg);
 	return err;
 }

@@ -1086,8 +1102,10 @@ static int __devexit flexcan_remove(struct platform_device *pdev)
 	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	release_mem_region(mem->start, resource_size(mem));
 
-	if (priv->clk)
-		clk_put(priv->clk);
+	if (priv->clk_per)
+		clk_put(priv->clk_per);
+	if (priv->clk_ipg)
+		clk_put(priv->clk_ipg);
 
 	free_candev(dev);

Signature · 1 line

1.7.10

Hui Wang

2012-07-18 02:12 UTC #1 lore raw

PATCH 1/1 can: flexcan: add 2nd clock to support imx53 and newer

Quoted 39 lines · Marc Kleine-Budde

From: Steffen Trumtrar <[email protected]>
This patch add support for a second clock to the flexcan driver. On modern freescale ARM cores like the imx53 and imx6q two clocks ("ipg" and "per") must be enabled in order to access the CAN core.
In the original driver, the clock was requested without specifying the connection id, further all mainline ARM archs with flexcan support (imx28, imx25, imx35) register their flexcan clock without a connection id, too.
This patch first renames the existing clk variable to clk_ipg and adds the connection id "ipg" to the clk_get() call. Then a second clock "per" is requested. As all archs don't specify a connection id, both clk_get return the same clock. This ensures compatibility to existing flexcan support and adds support for imx53 at the same time.
After this patch hits mainline, the archs may give their existing flexcan clock the "ipg" connection id and implement a dummy "per" clock.
This patch has been tested on imx28 (unmodified clk tree) and on imx53 with a seperate "ipg" and "per" clock.
Cc Sascha Hauer <[email protected]>
Cc Shawn Guo <[email protected]>
Cc Hui Wang <[email protected]>
Signed-off-by Steffen Trumtrar <[email protected]>
Signed-off-by Marc Kleine-Budde <[email protected]>
---
 drivers/net/can/flexcan.c |   52 ++++++++++++++++++++++++++++++---------------
 1 file changed, 35 insertions(+), 17 deletions(-)
drivers/net/can/flexcan.c +0 -0
index c8a6fc7..281d51f 100644
@@ -190,7 +190,8 @@ struct flexcan_priv {
 
 

[...]

Quoted 10 lines

drivers/net/can/flexcan.c +4 -4

 	if (!clock_freq) {
-		clk = clk_get(&pdev->dev, NULL);
-		if (IS_ERR(clk)) {
-			dev_err(&pdev->dev, "no clock defined\n");
-			err = PTR_ERR(clk);
+		clk_ipg = clk_get(&pdev->dev, "ipg");
+		if (IS_ERR(clk_ipg)) {
+			dev_err(&pdev->dev, "no ipg clock defined\n");
+			err = PTR_ERR(clk_ipg);

drivers/net/can/flexcan.c static int __devinit flexcan_probe(struct platform_device *pdev) · line 1002

Quoted 10 lines

drivers/net/can/flexcan.c +8 -0

+			goto failed_clock;
+		}
+		clock_freq = clk_get_rate(clk_ipg);
+
+		clk_per = clk_get(&pdev->dev, "per");
+		if (IS_ERR(clk_per)) {
+			dev_err(&pdev->dev, "no per clock defined\n");
+			err = PTR_ERR(clk_per);
 			goto failed_clock;
 		}

For those only register one clk and without con_id (mx35), clk_per will equal to clk_ipg, how to handle this situation, modify mx35 clk tree?

Quoted 15 lines

drivers/net/can/flexcan.c +2 -2

-		clock_freq = clk_get_rate(clk);
 	}
 
 	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);

@@ -1039,7 +1052,8 @@ static int __devinit flexcan_probe(struct platform_device *pdev)
 		CAN_CTRLMODE_BERR_REPORTING;
 	priv->base = base;
 	priv->dev = dev;
-	priv->clk = clk;
+	priv->clk_ipg = clk_ipg;
+	priv->clk_per = clk_per;
 	priv->pdata = pdev->dev.platform_data;
 	priv->devtype_data = devtype_data;

drivers/net/can/flexcan.c static int __devinit flexcan_probe(struct platform_device *pdev) · line 1086

Quoted 8 lines

drivers/net/can/flexcan.c +1 -2

@@ -1067,9 +1081,11 @@ static int __devinit flexcan_probe(struct platform_device *pdev)
  failed_map:
 	release_mem_region(mem->start, mem_size);
  failed_get:
-	if (clk)
-		clk_put(clk);
  failed_clock:
+	if (clk_per)

Use if (!IS_ERR(clk_per))

drivers/net/can/flexcan.c static int __devinit flexcan_probe(struct platform_device *pdev) · line 1091

Quoted 3 lines

drivers/net/can/flexcan.c +2 -0
+		clk_put(clk_per);
+	if (clk_ipg)
 

Ditto.

If we use devm_clk_get(), we can save to call clk_put().

Quoted 18 lines

drivers/net/can/flexcan.c +1 -0

+		clk_put(clk_ipg);
 	return err;

}

drivers/net/can/flexcan.c +4 -2

@@ -1086,8 +1102,10 @@ static int __devexit flexcan_remove(struct platform_device *pdev)
 	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	release_mem_region(mem->start, resource_size(mem));
 
-	if (priv->clk)
-		clk_put(priv->clk);
+	if (priv->clk_per)
+		clk_put(priv->clk_per);
+	if (priv->clk_ipg)
+		clk_put(priv->clk_ipg);
 
 	free_candev(dev);

Marc Kleine-Budde

author maintainer

2012-07-18 08:37 UTC #2 lore raw

PATCH 1/1 can: flexcan: add 2nd clock to support imx53 and newer

Quoted 66 lines · Hui Wang

Quoted 44 lines · Marc Kleine-Budde
From: Steffen Trumtrar <[email protected]>
This patch add support for a second clock to the flexcan driver. On modern
freescale ARM cores like the imx53 and imx6q two clocks ("ipg" and "per") must be enabled in order to access the CAN core.
In the original driver, the clock was requested without specifying the connection id, further all mainline ARM archs with flexcan support (imx28, imx25, imx35) register their flexcan clock without a connection id,
too.
This patch first renames the existing clk variable to clk_ipg and adds the
connection id "ipg" to the clk_get() call. Then a second clock "per" is requested. As all archs don't specify a connection id, both clk_get return
the same clock. This ensures compatibility to existing flexcan support and adds support for imx53 at the same time.
After this patch hits mainline, the archs may give their existing flexcan clock the "ipg" connection id and implement a dummy "per" clock.
This patch has been tested on imx28 (unmodified clk tree) and on imx53 with a seperate "ipg" and "per" clock.
Cc Sascha Hauer <[email protected]>
Cc Shawn Guo <[email protected]>
Cc Hui Wang <[email protected]>
Signed-off-by Steffen Trumtrar <[email protected]>
Signed-off-by Marc Kleine-Budde <[email protected]>
---
 drivers/net/can/flexcan.c |   52
++++++++++++++++++++++++++++++---------------
 1 file changed, 35 insertions(+), 17 deletions(-)
drivers/net/can/flexcan.c +0 -0
index c8a6fc7..281d51f 100644
@@ -190,7 +190,8 @@ struct flexcan_priv {
 
 
[...]
Quoted 10 lines
     if (!clock_freq) {
drivers/net/can/flexcan.c +4 -4
-        clk = clk_get(&pdev->dev, NULL);
-        if (IS_ERR(clk)) {
-            dev_err(&pdev->dev, "no clock defined\n");
-            err = PTR_ERR(clk);
+        clk_ipg = clk_get(&pdev->dev, "ipg");
+        if (IS_ERR(clk_ipg)) {
+            dev_err(&pdev->dev, "no ipg clock defined\n");
+            err = PTR_ERR(clk_ipg);
drivers/net/can/flexcan.c static int __devinit flexcan_probe(struct platform_device *pdev) · line 1002
Quoted 10 lines
drivers/net/can/flexcan.c +8 -0
+            goto failed_clock;
+        }
+        clock_freq = clk_get_rate(clk_ipg);
+
+        clk_per = clk_get(&pdev->dev, "per");
+        if (IS_ERR(clk_per)) {
+            dev_err(&pdev->dev, "no per clock defined\n");
+            err = PTR_ERR(clk_per);
             goto failed_clock;
         }

Quoted 2 lines

For those only register one clk and without con_id (mx35), clk_per will equal to clk_ipg, how to handle this situation, modify mx35 clk tree?

This isn't a problem, the clock is just enabled twice. As soon as this patch is mainline the clock tree on the one-clock-for-flexcan archs can modify their clock tree. I've commented on this in the commit message, have you read it?

Quoted 15 lines

drivers/net/can/flexcan.c +2 -2

-        clock_freq = clk_get_rate(clk);
     }
 
     mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);

@@ -1039,7 +1052,8 @@ static int __devinit flexcan_probe(struct
 platform_device *pdev)
         CAN_CTRLMODE_BERR_REPORTING;
     priv->base = base;
     priv->dev = dev;
-    priv->clk = clk;
+    priv->clk_ipg = clk_ipg;
+    priv->clk_per = clk_per;
     priv->pdata = pdev->dev.platform_data;
     priv->devtype_data = devtype_data;

Quoted 11 lines

drivers/net/can/flexcan.c static int __devinit flexcan_probe(struct · line 1087
Quoted 10 lines
drivers/net/can/flexcan.c +1 -2
@@ -1067,9 +1081,11 @@ static int __devinit flexcan_probe(struct
 platform_device *pdev)
  failed_map:
     release_mem_region(mem->start, mem_size);
  failed_get:
-    if (clk)
-        clk_put(clk);
  failed_clock:
+    if (clk_per)
 
Use if (!IS_ERR(clk_per))

Yes, good catch.

Is it allowed to call clk_put with a NULL pointer? Both clocks can be NULL, if the frequency is defined via the device tree, this is case for powerpc.

Quoted 6 lines

drivers/net/can/flexcan.c static int __devinit flexcan_probe(struct platform_device *pdev) · line 1091
Quoted 3 lines
drivers/net/can/flexcan.c +2 -0
+        clk_put(clk_per);
+    if (clk_ipg)
 
Ditto.
If we use devm_clk_get(), we can save to call clk_put().

Even better. I think I'll send a separate patch to convert the whole driver to devm.

Quoted 9 lines

drivers/net/can/flexcan.c +1 -0

+        clk_put(clk_ipg);

     return err;

}

drivers/net/can/flexcan.c +0 -0

@@ -1086,8 +1102,10 @@ static int __devexit flexcan_remove(struct
 platform_device *pdev)
     mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
     release_mem_region(mem->start, resource_size(mem));

Quoted 6 lines

drivers/net/can/flexcan.c static int __devexit flexcan_remove(struct platform_device *pdev) · line 1108
Quoted 6 lines
drivers/net/can/flexcan.c +4 -2
-    if (priv->clk)
-        clk_put(priv->clk);
+    if (priv->clk_per)
+        clk_put(priv->clk_per);
+    if (priv->clk_ipg)
+        clk_put(priv->clk_ipg);

This will go away with devm then :)

Quoted 9 lines

Quoted 4 lines

drivers/net/can/flexcan.c +0 -0
     free_candev(dev);
 

Signature · 3 lines

To unsubscribe from this list: send the line "unsubscribe linux-can" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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: 262 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120718/413fb0a7/attachment.sig>

Hui Wang

2012-07-18 08:48 UTC #3 lore raw

PATCH 1/1 can: flexcan: add 2nd clock to support imx53 and newer

Quoted 72 lines · Marc Kleine-Budde

Quoted 70 lines · Hui Wang
Quoted 45 lines · Marc Kleine-Budde
From: Steffen Trumtrar <[email protected]>
This patch add support for a second clock to the flexcan driver. On modern
freescale ARM cores like the imx53 and imx6q two clocks ("ipg" and "per") must be enabled in order to access the CAN core.
In the original driver, the clock was requested without specifying the connection id, further all mainline ARM archs with flexcan support (imx28, imx25, imx35) register their flexcan clock without a connection id,
too.
This patch first renames the existing clk variable to clk_ipg and adds the
connection id "ipg" to the clk_get() call. Then a second clock "per" is requested. As all archs don't specify a connection id, both clk_get return
the same clock. This ensures compatibility to existing flexcan support and adds support for imx53 at the same time.
After this patch hits mainline, the archs may give their existing flexcan clock the "ipg" connection id and implement a dummy "per" clock.
This patch has been tested on imx28 (unmodified clk tree) and on imx53 with a seperate "ipg" and "per" clock.
Cc Sascha Hauer <[email protected]>
Cc Shawn Guo <[email protected]>
Cc Hui Wang <[email protected]>
Signed-off-by Steffen Trumtrar <[email protected]>
Signed-off-by Marc Kleine-Budde <[email protected]>
---
 drivers/net/can/flexcan.c |   52
++++++++++++++++++++++++++++++---------------
 1 file changed, 35 insertions(+), 17 deletions(-)
drivers/net/can/flexcan.c +0 -0
index c8a6fc7..281d51f 100644
@@ -190,7 +190,8 @@ struct flexcan_priv {
 
 
 
[...]
Quoted 21 lines
     if (!clock_freq) {
drivers/net/can/flexcan.c +12 -4
-        clk = clk_get(&pdev->dev, NULL);
-        if (IS_ERR(clk)) {
-            dev_err(&pdev->dev, "no clock defined\n");
-            err = PTR_ERR(clk);
+        clk_ipg = clk_get(&pdev->dev, "ipg");
+        if (IS_ERR(clk_ipg)) {
+            dev_err(&pdev->dev, "no ipg clock defined\n");
+            err = PTR_ERR(clk_ipg);
+            goto failed_clock;
+        }
+        clock_freq = clk_get_rate(clk_ipg);
+
+        clk_per = clk_get(&pdev->dev, "per");
+        if (IS_ERR(clk_per)) {
+            dev_err(&pdev->dev, "no per clock defined\n");
+            err = PTR_ERR(clk_per);
             goto failed_clock;
         }

Quoted 9 lines

Quoted 3 lines
For those only register one clk and without con_id (mx35), clk_per will equal to clk_ipg, how to handle this situation, modify mx35 clk tree?
This isn't a problem, the clock is just enabled twice. As soon as this patch is mainline the clock tree on the one-clock-for-flexcan archs can modify their clock tree. I've commented on this in the commit message, have you read it?

Got it now. :-)

Sascha Hauer

2012-07-18 08:49 UTC #4 lore raw

PATCH 1/1 can: flexcan: add 2nd clock to support imx53 and newer

In reply to Marc Kleine-Budde

Quoted 4 lines · Marc Kleine-Budde

Yes, good catch.
Is it allowed to call clk_put with a NULL pointer?

As NULL is a valid clock by definition it is allowed to do this. However, calling clk_put with a NULL pointer when a corresponding call to clk_get never returned NULL is probably not a good idea.

Sascha

Signature · 4 lines

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

Lothar Waßmann

2012-07-18 09:05 UTC #5 lore raw

PATCH 1/1 can: flexcan: add 2nd clock to support imx53 and newer

In reply to Marc Kleine-Budde

Hi,

Quoted 35 lines · Marc Kleine-Budde

Quoted 22 lines · Hui Wang
Quoted 21 lines · Marc Kleine-Budde
From: Steffen Trumtrar <[email protected]>
This patch add support for a second clock to the flexcan driver. On modern
freescale ARM cores like the imx53 and imx6q two clocks ("ipg" and "per") must be enabled in order to access the CAN core.
drivers/net/can/flexcan.c +2 -2
-        clock_freq = clk_get_rate(clk);
     }
 
     mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -1039,7 +1052,8 @@ static int __devinit flexcan_probe(struct
 platform_device *pdev)
         CAN_CTRLMODE_BERR_REPORTING;
     priv->base = base;
     priv->dev = dev;
-    priv->clk = clk;
+    priv->clk_ipg = clk_ipg;
+    priv->clk_per = clk_per;
     priv->pdata = pdev->dev.platform_data;
     priv->devtype_data = devtype_data;
Quoted 11 lines
drivers/net/can/flexcan.c static int __devinit flexcan_probe(struct · line 1087
Quoted 10 lines
drivers/net/can/flexcan.c +1 -2
@@ -1067,9 +1081,11 @@ static int __devinit flexcan_probe(struct
 platform_device *pdev)
  failed_map:
     release_mem_region(mem->start, mem_size);
  failed_get:
-    if (clk)
-        clk_put(clk);
  failed_clock:
+    if (clk_per)
 
Use if (!IS_ERR(clk_per))

Quoted 6 lines

Yes, good catch.
Is it allowed to call clk_put with a NULL pointer? Both clocks can be NULL, if the frequency is defined via the device tree, this is case for powerpc.

This has been discussed here already several times. Everything returned by clk_get() that does not satisfy the IS_ERR() check, is a valid clk cookie and can be used for the clk API calls.

Lothar Wa?mann

Signature · 9 lines

___________________________________________________________

Ka-Ro electronics GmbH | Pascalstra?e 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Gesch?ftsf?hrer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

www.karo-electronics.de | info at karo-electronics.de
___________________________________________________________

Marc Kleine-Budde

author maintainer

2012-07-18 09:21 UTC #6 lore raw

PATCH 1/1 can: flexcan: add 2nd clock to support imx53 and newer

Quoted 40 lines · Lothar Wa?mann

Hi,
Quoted 37 lines · Marc Kleine-Budde
Quoted 22 lines · Hui Wang
Quoted 21 lines · Marc Kleine-Budde
From: Steffen Trumtrar <[email protected]>
This patch add support for a second clock to the flexcan driver. On modern
freescale ARM cores like the imx53 and imx6q two clocks ("ipg" and "per") must be enabled in order to access the CAN core.
drivers/net/can/flexcan.c +2 -2
-        clock_freq = clk_get_rate(clk);
     }
 
     mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -1039,7 +1052,8 @@ static int __devinit flexcan_probe(struct
 platform_device *pdev)
         CAN_CTRLMODE_BERR_REPORTING;
     priv->base = base;
     priv->dev = dev;
-    priv->clk = clk;
+    priv->clk_ipg = clk_ipg;
+    priv->clk_per = clk_per;
     priv->pdata = pdev->dev.platform_data;
     priv->devtype_data = devtype_data;
Quoted 11 lines
drivers/net/can/flexcan.c static int __devinit flexcan_probe(struct · line 1087
Quoted 10 lines
drivers/net/can/flexcan.c +1 -2
@@ -1067,9 +1081,11 @@ static int __devinit flexcan_probe(struct
 platform_device *pdev)
  failed_map:
     release_mem_region(mem->start, mem_size);
  failed_get:
-    if (clk)
-        clk_put(clk);
  failed_clock:
+    if (clk_per)
 
Use if (!IS_ERR(clk_per))
Yes, good catch.

Quoted 7 lines

Quoted 4 lines
Is it allowed to call clk_put with a NULL pointer? Both clocks can be NULL, if the frequency is defined via the device tree, this is case for powerpc.
This has been discussed here already several times. Everything returned by clk_get() that does not satisfy the IS_ERR() check, is a valid clk cookie and can be used for the clk API calls.

I've switched to devm completely (see Patch v2), so the clk_put is obsolete.

In this particular driver the NULL pointer in question is not returned via the clk_get(). On powerpc we don't use clk_get(), because it's not working (AFIAK) on the p1010 (the powerpc arch that has a flexcan).

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: 262 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120718/34408bf6/attachment.sig>