[PATCH v3 0/3] Add DT support to C_CAN/D_CAN controller

[PATCH v3 0/3] Add DT support to C_CAN/D_CAN controller

[AnilKumar Ch]

This patch series adds the device tree support and Runtime PM support
to C_CAN/D_CAN controller. First patch cleans up the device names used
in c_can driver.

These patches have been tested on AM335x EVM using some additional
patches to add device tree data to EVM dts files and to initialize
D_CAN RAM. D_CAN raminit is controlled from control module register.
This patch will be submitted once control module MFD driver support
is added. And these patches are based on linx-can-next tree.

Due to lack of hardware I am not able to test c_can functionality.
I appreciate if anyone can test c_can functionality with this patch
series.

Changes from v3:
	- Changed the patch sequence

Changes from v2:
	- Incorporated Marcs on v2
	  * Fix compilation errors in pci due to device name changes
	  in v2 by adding new patch.

Changes from v1:
	- Separated 4 patches into CAN driver specific and device
	  tree data addition specific.
	- Incorporated Marc's comments on v1
	  * Modified c_can_dev_id enum to handle both devtype and
	    platform device id index.
	  * Removed "legacy bosch,c_can_platform" from DT bindings

AnilKumar Ch (3):
  can: c_can: Modify c_can device names
  can: c_can: Add device tree support to Bosch C_CAN/D_CAN controller
  can: c_can: Add runtime PM support to Bosch C_CAN/D_CAN controller

 .../devicetree/bindings/net/can/c_can.txt          |   37 +++++++++++
 drivers/net/can/c_can/c_can.h                      |    5 +-
 drivers/net/can/c_can/c_can_pci.c                  |    6 +-
 drivers/net/can/c_can/c_can_platform.c             |   65 +++++++++++++++-----
 4 files changed, 91 insertions(+), 22 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/can/c_can.txt

-- 
1.7.9.5

[PATCH v4 1/3] can: c_can: Modify c_can device names

[AnilKumar Ch]

Modify c_can device names from *_CAN_DEVTYPE to BOSCH_*_CAN to make
use of same names for array indexes in c_can_id_table[] as well as
device names.

This patch also add indexes to c_can_id_table array.

Signed-off-by: AnilKumar Ch <anilkumar@ti.com>
---
 drivers/net/can/c_can/c_can.h          |    5 +++--
 drivers/net/can/c_can/c_can_pci.c      |    6 +++---
 drivers/net/can/c_can/c_can_platform.c |   18 ++++++++++--------
 3 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/drivers/net/can/c_can/c_can.h b/drivers/net/can/c_can/c_can.h
index 01a7049..4e56baa 100644
--- a/drivers/net/can/c_can/c_can.h
+++ b/drivers/net/can/c_can/c_can.h
@@ -143,8 +143,9 @@ static const u16 reg_map_d_can[] = {
 };
 
 enum c_can_dev_id {
-	C_CAN_DEVTYPE,
-	D_CAN_DEVTYPE,
+	BOSCH_C_CAN_PLATFORM,
+	BOSCH_C_CAN,
+	BOSCH_D_CAN,
 };
 
 /* c_can private data structure */
diff --git a/drivers/net/can/c_can/c_can_pci.c b/drivers/net/can/c_can/c_can_pci.c
index 1011146..3d7830b 100644
--- a/drivers/net/can/c_can/c_can_pci.c
+++ b/drivers/net/can/c_can/c_can_pci.c
@@ -120,10 +120,10 @@ static int __devinit c_can_pci_probe(struct pci_dev *pdev,
 
 	/* Configure CAN type */
 	switch (c_can_pci_data->type) {
-	case C_CAN_DEVTYPE:
+	case BOSCH_C_CAN:
 		priv->regs = reg_map_c_can;
 		break;
-	case D_CAN_DEVTYPE:
+	case BOSCH_D_CAN:
 		priv->regs = reg_map_d_can;
 		priv->can.ctrlmode_supported |= CAN_CTRLMODE_3_SAMPLES;
 		break;
@@ -192,7 +192,7 @@ static void __devexit c_can_pci_remove(struct pci_dev *pdev)
 }
 
 static struct c_can_pci_data c_can_sta2x11= {
-	.type = C_CAN_DEVTYPE,
+	.type = BOSCH_C_CAN,
 	.reg_align = C_CAN_REG_ALIGN_32,
 	.freq = 52000000, /* 52 Mhz */
 };
diff --git a/drivers/net/can/c_can/c_can_platform.c b/drivers/net/can/c_can/c_can_platform.c
index 6ff7ad0..5c46d1c 100644
--- a/drivers/net/can/c_can/c_can_platform.c
+++ b/drivers/net/can/c_can/c_can_platform.c
@@ -116,7 +116,7 @@ static int __devinit c_can_plat_probe(struct platform_device *pdev)
 	priv = netdev_priv(dev);
 	id = platform_get_device_id(pdev);
 	switch (id->driver_data) {
-	case C_CAN_DEVTYPE:
+	case BOSCH_C_CAN:
 		priv->regs = reg_map_c_can;
 		switch (mem->flags & IORESOURCE_MEM_TYPE_MASK) {
 		case IORESOURCE_MEM_32BIT:
@@ -130,7 +130,7 @@ static int __devinit c_can_plat_probe(struct platform_device *pdev)
 			break;
 		}
 		break;
-	case D_CAN_DEVTYPE:
+	case BOSCH_D_CAN:
 		priv->regs = reg_map_d_can;
 		priv->can.ctrlmode_supported |= CAN_CTRLMODE_3_SAMPLES;
 		priv->read_reg = c_can_plat_read_reg_aligned_to_16bit;
@@ -196,15 +196,17 @@ static int __devexit c_can_plat_remove(struct platform_device *pdev)
 }
 
 static const struct platform_device_id c_can_id_table[] = {
-	{
+	[BOSCH_C_CAN_PLATFORM] = {
 		.name = KBUILD_MODNAME,
-		.driver_data = C_CAN_DEVTYPE,
-	}, {
+		.driver_data = BOSCH_C_CAN,
+	},
+	[BOSCH_C_CAN] = {
 		.name = "c_can",
-		.driver_data = C_CAN_DEVTYPE,
-	}, {
+		.driver_data = BOSCH_C_CAN,
+	},
+	[BOSCH_D_CAN] = {
 		.name = "d_can",
-		.driver_data = D_CAN_DEVTYPE,
+		.driver_data = BOSCH_D_CAN,
 	}, {
 	}
 };
-- 
1.7.9.5

[PATCH v4 2/3] can: c_can: Add device tree support to Bosch C_CAN/D_CAN controller

[AnilKumar Ch]

Add device tree support to C_CAN/D_CAN controller and usage details
are added to device tree documentation. Driver was tested on AM335x
EVM.

Signed-off-by: AnilKumar Ch <anilkumar@ti.com>
---
 .../devicetree/bindings/net/can/c_can.txt          |   37 +++++++++++++
 drivers/net/can/c_can/c_can_platform.c             |   55 ++++++++++++++------
 2 files changed, 75 insertions(+), 17 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/can/c_can.txt

diff --git a/Documentation/devicetree/bindings/net/can/c_can.txt b/Documentation/devicetree/bindings/net/can/c_can.txt
new file mode 100644
index 0000000..a43f083
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/can/c_can.txt
@@ -0,0 +1,37 @@
+Bosch C_CAN/D_CAN controller Device Tree Bindings
+-------------------------------------------------
+
+Required properties:
+- compatible		: Should be "bosch,c_can" for C_CAN controllers and
+			  "bosch,d_can" for D_CAN controllers.
+- reg			: physical base address and size of the C_CAN/D_CAN
+			  registers map
+- interrupts		: property with a value describing the interrupt
+			  number
+
+Optional properties:
+- interrupt-parent	: The parent interrupt controller
+- ti,hwmods		: Must be "d_can<n>" or "c_can<n>", n being the
+			  instance number
+
+Note: "ti,hwmods" field is used to fetch the base address and irq
+resources from TI, omap hwmod data base during device registration.
+Future plan is to migrate hwmod data base contents into device tree
+blob so that, all the required data will be used from device tree dts
+file.
+
+Examples:
+
+	d_can@481D0000 {
+		compatible = "bosch,d_can";
+		reg = <0x481D0000 0x1000>;
+		interrupts = <55 0x4>;
+		interrupt-parent = <&intc>;
+	};
+
+(or)
+
+	d_can@481D0000 {
+		compatible = "bosch,d_can";
+		ti,hwmods = "d_can1";
+	};
diff --git a/drivers/net/can/c_can/c_can_platform.c b/drivers/net/can/c_can/c_can_platform.c
index 5c46d1c..d0a66cf 100644
--- a/drivers/net/can/c_can/c_can_platform.c
+++ b/drivers/net/can/c_can/c_can_platform.c
@@ -30,6 +30,8 @@
 #include <linux/io.h>
 #include <linux/platform_device.h>
 #include <linux/clk.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
 
 #include <linux/can/dev.h>
 
@@ -65,17 +67,52 @@ static void c_can_plat_write_reg_aligned_to_32bit(struct c_can_priv *priv,
 	writew(val, priv->base + 2 * priv->regs[index]);
 }
 
+static struct platform_device_id c_can_id_table[] = {
+	[BOSCH_C_CAN_PLATFORM] = {
+		.name = KBUILD_MODNAME,
+		.driver_data = BOSCH_C_CAN,
+	},
+	[BOSCH_C_CAN] = {
+		.name = "c_can",
+		.driver_data = BOSCH_C_CAN,
+	},
+	[BOSCH_D_CAN] = {
+		.name = "d_can",
+		.driver_data = BOSCH_D_CAN,
+	}, {
+	}
+};
+
+static const struct of_device_id c_can_of_table[] = {
+	{ .compatible = "bosch,c_can", .data = &c_can_id_table[BOSCH_C_CAN] },
+	{ .compatible = "bosch,d_can", .data = &c_can_id_table[BOSCH_D_CAN] },
+	{ /* sentinel */ },
+};
+
 static int __devinit c_can_plat_probe(struct platform_device *pdev)
 {
 	int ret;
 	void __iomem *addr;
 	struct net_device *dev;
 	struct c_can_priv *priv;
+	const struct of_device_id *match;
 	const struct platform_device_id *id;
 	struct resource *mem;
 	int irq;
 	struct clk *clk;
 
+	if (pdev->dev.of_node) {
+		match = of_match_device(c_can_of_table, &pdev->dev);
+		if (!match) {
+			dev_err(&pdev->dev, "Failed to find matching dt id\n");
+			ret = -EINVAL;
+			goto exit;
+		}
+		id = match->data;
+	} else {
+		id = platform_get_device_id(pdev);
+	}
+
 	/* get the appropriate clk */
 	clk = clk_get(&pdev->dev, NULL);
 	if (IS_ERR(clk)) {
@@ -114,7 +151,6 @@ static int __devinit c_can_plat_probe(struct platform_device *pdev)
 	}
 
 	priv = netdev_priv(dev);
-	id = platform_get_device_id(pdev);
 	switch (id->driver_data) {
 	case BOSCH_C_CAN:
 		priv->regs = reg_map_c_can;
@@ -195,26 +231,11 @@ static int __devexit c_can_plat_remove(struct platform_device *pdev)
 	return 0;
 }
 
-static const struct platform_device_id c_can_id_table[] = {
-	[BOSCH_C_CAN_PLATFORM] = {
-		.name = KBUILD_MODNAME,
-		.driver_data = BOSCH_C_CAN,
-	},
-	[BOSCH_C_CAN] = {
-		.name = "c_can",
-		.driver_data = BOSCH_C_CAN,
-	},
-	[BOSCH_D_CAN] = {
-		.name = "d_can",
-		.driver_data = BOSCH_D_CAN,
-	}, {
-	}
-};
-
 static struct platform_driver c_can_plat_driver = {
 	.driver = {
 		.name = KBUILD_MODNAME,
 		.owner = THIS_MODULE,
+		.of_match_table = of_match_ptr(c_can_of_table),
 	},
 	.probe = c_can_plat_probe,
 	.remove = __devexit_p(c_can_plat_remove),
-- 
1.7.9.5

[PATCH v4 3/3] can: c_can: Add runtime PM support to Bosch C_CAN/D_CAN controller

[AnilKumar Ch]

Add Runtime PM support to C_CAN/D_CAN controller. The runtime PM
APIs control clocks for C_CAN/D_CAN IP and prevent access to the
register of C_CAN/D_CAN IP when clock is turned off.

Signed-off-by: AnilKumar Ch <anilkumar@ti.com>
---
 drivers/net/can/c_can/c_can_platform.c |    8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/net/can/c_can/c_can_platform.c b/drivers/net/can/c_can/c_can_platform.c
index d0a66cf..83a1e17 100644
--- a/drivers/net/can/c_can/c_can_platform.c
+++ b/drivers/net/can/c_can/c_can_platform.c
@@ -32,6 +32,7 @@
 #include <linux/clk.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
+#include <linux/pm_runtime.h>
 
 #include <linux/can/dev.h>
 
@@ -177,6 +178,9 @@ static int __devinit c_can_plat_probe(struct platform_device *pdev)
 		goto exit_free_device;
 	}
 
+	pm_runtime_enable(&pdev->dev);
+	pm_runtime_get_sync(&pdev->dev);
+
 	dev->irq = irq;
 	priv->base = addr;
 	priv->can.clock.freq = clk_get_rate(clk);
@@ -198,6 +202,8 @@ static int __devinit c_can_plat_probe(struct platform_device *pdev)
 
 exit_free_device:
 	platform_set_drvdata(pdev, NULL);
+	pm_runtime_put_sync(&pdev->dev);
+	pm_runtime_disable(&pdev->dev);
 	free_c_can_dev(dev);
 exit_iounmap:
 	iounmap(addr);
@@ -226,6 +232,8 @@ static int __devexit c_can_plat_remove(struct platform_device *pdev)
 	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	release_mem_region(mem->start, resource_size(mem));
 
+	pm_runtime_put_sync(&pdev->dev);
+	pm_runtime_disable(&pdev->dev);
 	clk_put(priv->priv);
 
 	return 0;
-- 
1.7.9.5

RE: [PATCH v4 3/3] can: c_can: Add runtime PM support to Bosch C_CAN/D_CAN controller

[Hiremath, Vaibhav]

On Thu, Aug 02, 2012 at 18:43:11, AnilKumar, Chimata wrote:
> Add Runtime PM support to C_CAN/D_CAN controller. The runtime PM
> APIs control clocks for C_CAN/D_CAN IP and prevent access to the
> register of C_CAN/D_CAN IP when clock is turned off.
> 
> Signed-off-by: AnilKumar Ch <anilkumar-l0cyMroinI0@public.gmane.org>
> ---
>  drivers/net/can/c_can/c_can_platform.c |    8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/net/can/c_can/c_can_platform.c b/drivers/net/can/c_can/c_can_platform.c
> index d0a66cf..83a1e17 100644
> --- a/drivers/net/can/c_can/c_can_platform.c
> +++ b/drivers/net/can/c_can/c_can_platform.c
> @@ -32,6 +32,7 @@
>  #include <linux/clk.h>
>  #include <linux/of.h>
>  #include <linux/of_device.h>
> +#include <linux/pm_runtime.h>
>  
>  #include <linux/can/dev.h>
>  
> @@ -177,6 +178,9 @@ static int __devinit c_can_plat_probe(struct platform_device *pdev)
>  		goto exit_free_device;
>  	}
>  
> +	pm_runtime_enable(&pdev->dev);
> +	pm_runtime_get_sync(&pdev->dev);
> +

If module is inserted or built into the kernel, module stays in enabled 
state always, isn't that wrong?
Ideally, you should enable the module when it is required or being used.

Thanks,
Vaibhav

Re: [PATCH v4 3/3] can: c_can: Add runtime PM support to Bosch C_CAN/D_CAN controller

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 1830 bytes --]

On 08/03/2012 08:32 AM, Hiremath, Vaibhav wrote:
> On Thu, Aug 02, 2012 at 18:43:11, AnilKumar, Chimata wrote:
>> Add Runtime PM support to C_CAN/D_CAN controller. The runtime PM
>> APIs control clocks for C_CAN/D_CAN IP and prevent access to the
>> register of C_CAN/D_CAN IP when clock is turned off.
>>
>> Signed-off-by: AnilKumar Ch <anilkumar@ti.com>
>> ---
>>  drivers/net/can/c_can/c_can_platform.c |    8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/drivers/net/can/c_can/c_can_platform.c b/drivers/net/can/c_can/c_can_platform.c
>> index d0a66cf..83a1e17 100644
>> --- a/drivers/net/can/c_can/c_can_platform.c
>> +++ b/drivers/net/can/c_can/c_can_platform.c
>> @@ -32,6 +32,7 @@
>>  #include <linux/clk.h>
>>  #include <linux/of.h>
>>  #include <linux/of_device.h>
>> +#include <linux/pm_runtime.h>
>>  
>>  #include <linux/can/dev.h>
>>  
>> @@ -177,6 +178,9 @@ static int __devinit c_can_plat_probe(struct platform_device *pdev)
>>  		goto exit_free_device;
>>  	}
>>  
>> +	pm_runtime_enable(&pdev->dev);
>> +	pm_runtime_get_sync(&pdev->dev);
>> +
> 
> If module is inserted or built into the kernel, module stays in enabled 
> state always, isn't that wrong?
> Ideally, you should enable the module when it is required or being used.

Good point.

If you don't access the module's registers in the probe- (or its
subroutines) it should be enough to enable the module in the open()
function. Have a look at clk_prepare_enable / clk_disable_unprepare in
the flexcan driver.

Marc

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

RE: [PATCH v4 3/3] can: c_can: Add runtime PM support to Bosch C_CAN/D_CAN controller

[Hiremath, Vaibhav]

On Sat, Aug 04, 2012 at 00:39:25, Marc Kleine-Budde wrote:
> On 08/03/2012 08:32 AM, Hiremath, Vaibhav wrote:
> > On Thu, Aug 02, 2012 at 18:43:11, AnilKumar, Chimata wrote:
> >> Add Runtime PM support to C_CAN/D_CAN controller. The runtime PM
> >> APIs control clocks for C_CAN/D_CAN IP and prevent access to the
> >> register of C_CAN/D_CAN IP when clock is turned off.
> >>
> >> Signed-off-by: AnilKumar Ch <anilkumar@ti.com>
> >> ---
> >>  drivers/net/can/c_can/c_can_platform.c |    8 ++++++++
> >>  1 file changed, 8 insertions(+)
> >>
> >> diff --git a/drivers/net/can/c_can/c_can_platform.c b/drivers/net/can/c_can/c_can_platform.c
> >> index d0a66cf..83a1e17 100644
> >> --- a/drivers/net/can/c_can/c_can_platform.c
> >> +++ b/drivers/net/can/c_can/c_can_platform.c
> >> @@ -32,6 +32,7 @@
> >>  #include <linux/clk.h>
> >>  #include <linux/of.h>
> >>  #include <linux/of_device.h>
> >> +#include <linux/pm_runtime.h>
> >>  
> >>  #include <linux/can/dev.h>
> >>  
> >> @@ -177,6 +178,9 @@ static int __devinit c_can_plat_probe(struct platform_device *pdev)
> >>  		goto exit_free_device;
> >>  	}
> >>  
> >> +	pm_runtime_enable(&pdev->dev);
> >> +	pm_runtime_get_sync(&pdev->dev);
> >> +
> > 
> > If module is inserted or built into the kernel, module stays in enabled 
> > state always, isn't that wrong?
> > Ideally, you should enable the module when it is required or being used.
> 
> Good point.
> 
> If you don't access the module's registers in the probe- (or its
> subroutines) it should be enough to enable the module in the open()
> function. Have a look at clk_prepare_enable / clk_disable_unprepare in
> the flexcan driver.
> 

Yeah Marc, something similar, above runtime pm api's should be moved to 
open-n-close.

Thanks,
Vaibhav

> Marc
> 
> -- 
> 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   |
> 
> 

Re: [PATCH v4 1/3] can: c_can: Modify c_can device names

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 710 bytes --]

On 08/02/2012 03:13 PM, AnilKumar Ch wrote:
> Modify c_can device names from *_CAN_DEVTYPE to BOSCH_*_CAN to make
> use of same names for array indexes in c_can_id_table[] as well as
> device names.
> 
> This patch also add indexes to c_can_id_table array.
> 
> Signed-off-by: AnilKumar Ch <anilkumar@ti.com>

I'm taking patch 1 and 2. Runtime pm support seems to need discussion.

Applied to can-next/master

Tnx,
Marc
-- 
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   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

Re: [PATCH v4 2/3] can: c_can: Add device tree support to Bosch C_CAN/D_CAN controller

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 570 bytes --]

On 08/02/2012 03:13 PM, AnilKumar Ch wrote:
> Add device tree support to C_CAN/D_CAN controller and usage details
> are added to device tree documentation. Driver was tested on AM335x
> EVM.
> 
> Signed-off-by: AnilKumar Ch <anilkumar@ti.com>

Applied to can-next/master

tnx,
Marc

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

RE: [PATCH v4 1/3] can: c_can: Modify c_can device names

[AnilKumar, Chimata]

On Mon, Aug 06, 2012 at 15:06:49, Marc Kleine-Budde wrote:
> On 08/02/2012 03:13 PM, AnilKumar Ch wrote:
> > Modify c_can device names from *_CAN_DEVTYPE to BOSCH_*_CAN to make
> > use of same names for array indexes in c_can_id_table[] as well as
> > device names.
> > 
> > This patch also add indexes to c_can_id_table array.
> > 
> > Signed-off-by: AnilKumar Ch <anilkumar-l0cyMroinI0@public.gmane.org>
> 
> I'm taking patch 1 and 2. Runtime pm support seems to need discussion.
> 
> Applied to can-next/master
> 

Thanks Marc,

I will update on runtime PM functionality.

Regards
AnilKumar

RE: [PATCH v4 3/3] can: c_can: Add runtime PM support to Bosch C_CAN/D_CAN controller

[AnilKumar, Chimata]

Hi Vaibhav,

Thanks for the review.

On Mon, Aug 06, 2012 at 12:18:32, Hiremath, Vaibhav wrote:
> On Sat, Aug 04, 2012 at 00:39:25, Marc Kleine-Budde wrote:
> > On 08/03/2012 08:32 AM, Hiremath, Vaibhav wrote:
> > > On Thu, Aug 02, 2012 at 18:43:11, AnilKumar, Chimata wrote:
> > >> Add Runtime PM support to C_CAN/D_CAN controller. The runtime PM
> > >> APIs control clocks for C_CAN/D_CAN IP and prevent access to the
> > >> register of C_CAN/D_CAN IP when clock is turned off.
> > >>
> > >> Signed-off-by: AnilKumar Ch <anilkumar-l0cyMroinI0@public.gmane.org>
> > >> ---
> > >>  drivers/net/can/c_can/c_can_platform.c |    8 ++++++++
> > >>  1 file changed, 8 insertions(+)
> > >>
> > >> diff --git a/drivers/net/can/c_can/c_can_platform.c b/drivers/net/can/c_can/c_can_platform.c
> > >> index d0a66cf..83a1e17 100644
> > >> --- a/drivers/net/can/c_can/c_can_platform.c
> > >> +++ b/drivers/net/can/c_can/c_can_platform.c
> > >> @@ -32,6 +32,7 @@
> > >>  #include <linux/clk.h>
> > >>  #include <linux/of.h>
> > >>  #include <linux/of_device.h>
> > >> +#include <linux/pm_runtime.h>
> > >>  
> > >>  #include <linux/can/dev.h>
> > >>  
> > >> @@ -177,6 +178,9 @@ static int __devinit c_can_plat_probe(struct platform_device *pdev)
> > >>  		goto exit_free_device;
> > >>  	}
> > >>  
> > >> +	pm_runtime_enable(&pdev->dev);
> > >> +	pm_runtime_get_sync(&pdev->dev);
> > >> +
> > > 
> > > If module is inserted or built into the kernel, module stays in enabled 
> > > state always, isn't that wrong?
> > > Ideally, you should enable the module when it is required or being used.
> > 
> > Good point.
> > 
> > If you don't access the module's registers in the probe- (or its
> > subroutines) it should be enough to enable the module in the open()
> > function. Have a look at clk_prepare_enable / clk_disable_unprepare in
> > the flexcan driver.
> > 
> 
> Yeah Marc, something similar, above runtime pm api's should be moved to 
> open-n-close.
> 

Not all APIs, only *_get_sync and *_put_sync to appropriate positions.
I will send next version after fixing this.

Regards
AnilKumar

Re: [PATCH v4 2/3] can: c_can: Add device tree support to Bosch C_CAN/D_CAN controller

[Amit]

Dear Anil,

AnilKumar Ch <anilkumar <at> ti.com> writes:

> 
> Add device tree support to C_CAN/D_CAN controller and usage details
> are added to device tree documentation. Driver was tested on AM335x
> EVM.
> 
> Signed-off-by: AnilKumar Ch <anilkumar <at> ti.com>

Did you test this on any system where CAN registers are 32 bit aligned?
I couldn't find a way to pass the "flags" option thro' DT and the first look at 
the OF DT code suggests me that there's no way to pass it.

What do you think?

Regards
Amit Virdi