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

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

[AnilKumar Ch]

This patch series adds the device tree support to C_CAN/D_CAN
controller with pinmux configuration and device tree data
addition to corresponding dts and dtsi files.

Also adds Runtime PM support to C_CAN/D_CAN controller.

These patches have been tested on AM335x EVM using some additional
patch 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 to mainline.

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.

First and last patches applies on top of linux-next tree, 2nd and
3rd patches applies on linux-omap tree.

AnilKumar Ch (4):
  can: c_can: Add device tree support to Bosch C_CAN/D_CAN controller
  arm/dts: AM33XX: Add D_CAN device tree data
  arm/dts: AM33XX: Configure pinmuxs for D_CAN1 on AM335x-EVM
  can: c_can: Add runtime PM support to Bosch C_CAN/D_CAN controller

 .../devicetree/bindings/net/can/c_can.txt          |   38 +++++++++++++
 arch/arm/boot/dts/am335x-evm.dts                   |   13 +++++
 arch/arm/boot/dts/am33xx.dtsi                      |    5 ++
 drivers/net/can/c_can/c_can_platform.c             |   60 +++++++++++++++-----
 4 files changed, 101 insertions(+), 15 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/can/c_can.txt

-- 
1.7.9.5

[PATCH 1/4] 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          |   38 ++++++++++++++
 drivers/net/can/c_can/c_can_platform.c             |   52 ++++++++++++++------
 2 files changed, 75 insertions(+), 15 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..78b8ae8
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/can/c_can.txt
@@ -0,0 +1,38 @@
+Bosch C_CAN/D_CAN controller Device Tree Bindings
+-------------------------------------------------
+
+Required properties:
+- compatible		: Should be "bosch,c_can_platform" or "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
+- interrupt-parent	: The parent interrupt controller
+
+Optional properties:
+- 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 6ff7ad0..2f5b153 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,51 @@ 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[] = {
+	{
+		.name = KBUILD_MODNAME,
+		.driver_data = C_CAN_DEVTYPE,
+	}, {
+		.name = "c_can",
+		.driver_data = C_CAN_DEVTYPE,
+	}, {
+		.name = "d_can",
+		.driver_data = D_CAN_DEVTYPE,
+	}, {
+	}
+};
+
+static const struct of_device_id c_can_of_table[] = {
+	{ .compatible = "bosch,c_can_platform", .data = &c_can_id_table[0] },
+	{ .compatible = "bosch,c_can", .data = &c_can_id_table[1] },
+	{ .compatible = "bosch,d_can", .data = &c_can_id_table[2] },
+	{ /* 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 +150,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 C_CAN_DEVTYPE:
 		priv->regs = reg_map_c_can;
@@ -195,24 +230,11 @@ static int __devexit c_can_plat_remove(struct platform_device *pdev)
 	return 0;
 }
 
-static const struct platform_device_id c_can_id_table[] = {
-	{
-		.name = KBUILD_MODNAME,
-		.driver_data = C_CAN_DEVTYPE,
-	}, {
-		.name = "c_can",
-		.driver_data = C_CAN_DEVTYPE,
-	}, {
-		.name = "d_can",
-		.driver_data = D_CAN_DEVTYPE,
-	}, {
-	}
-};
-
 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 2/4] arm/dts: AM33XX: Add D_CAN device tree data

[AnilKumar Ch]

Add Bosch D_CAN controller device tree data to AM33XX dtsi file
by adding d_can device node with all the necessary parameters.

Signed-off-by: AnilKumar Ch <anilkumar@ti.com>
---
 arch/arm/boot/dts/am33xx.dtsi |    5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
index 9b974dc..2db2ffb 100644
--- a/arch/arm/boot/dts/am33xx.dtsi
+++ b/arch/arm/boot/dts/am33xx.dtsi
@@ -163,5 +163,10 @@
 			#size-cells = <0>;
 			ti,hwmods = "i2c3";
 		};
+
+		dcan1: d_can@481D0000 {
+			compatible = "bosch,d_can";
+			ti,hwmods = "d_can1";
+		};
 	};
 };
-- 
1.7.9.5

[PATCH 3/4] arm/dts: AM33XX: Configure pinmuxs for D_CAN1 on AM335x-EVM

[AnilKumar Ch]

Add D_CAN1 pinctrl node to am3358_pinmux master node to export
D_CAN functionality on AM335x EVM according to pinctrl-single
driver.

Signed-off-by: AnilKumar Ch <anilkumar@ti.com>
---
 arch/arm/boot/dts/am335x-evm.dts |   13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/arch/arm/boot/dts/am335x-evm.dts b/arch/arm/boot/dts/am335x-evm.dts
index e4e1ccb..7f0fd6c 100644
--- a/arch/arm/boot/dts/am335x-evm.dts
+++ b/arch/arm/boot/dts/am335x-evm.dts
@@ -46,3 +46,16 @@
 	regulator-boot-on;
 	regulator-always-on;
 };
+
+&am3358_pinmux {
+
+	pinctrl-names = "default";
+	pinctrl-0 = <&d_can1_pins>;
+
+	d_can1_pins: pinmux_d_can_pins {
+		pinctrl-single,pins = <
+			0x168 0x2	/* uart0_ctsn.d_can1_tx, OUTPUT | MODE2 */
+			0x16C 0x32	/* uart0_rtsn.d_can1_rx, INPUT_PULLUP | MODE2 */
+		>;
+	};
+};
-- 
1.7.9.5

[PATCH 4/4] 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 2f5b153..f0dc912 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>
 
@@ -176,6 +177,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);
@@ -197,6 +201,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);
@@ -225,6 +231,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 2/4] arm/dts: AM33XX: Add D_CAN device tree data

[AnilKumar, Chimata]

+Tony

On Wed, Jul 25, 2012 at 11:53:35, AnilKumar, Chimata wrote:
> Add Bosch D_CAN controller device tree data to AM33XX dtsi file
> by adding d_can device node with all the necessary parameters.
> 
> Signed-off-by: AnilKumar Ch <anilkumar@ti.com>
> ---
>  arch/arm/boot/dts/am33xx.dtsi |    5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
> index 9b974dc..2db2ffb 100644
> --- a/arch/arm/boot/dts/am33xx.dtsi
> +++ b/arch/arm/boot/dts/am33xx.dtsi
> @@ -163,5 +163,10 @@
>  			#size-cells = <0>;
>  			ti,hwmods = "i2c3";
>  		};
> +
> +		dcan1: d_can@481D0000 {
> +			compatible = "bosch,d_can";
> +			ti,hwmods = "d_can1";
> +		};
>  	};
>  };
> -- 
> 1.7.9.5
> 
> 

RE: [PATCH 3/4] arm/dts: AM33XX: Configure pinmuxs for D_CAN1 on AM335x-EVM

[AnilKumar, Chimata]

+Tony

On Wed, Jul 25, 2012 at 11:53:36, AnilKumar, Chimata wrote:
> Add D_CAN1 pinctrl node to am3358_pinmux master node to export
> D_CAN functionality on AM335x EVM according to pinctrl-single
> driver.
> 
> Signed-off-by: AnilKumar Ch <anilkumar@ti.com>
> ---
>  arch/arm/boot/dts/am335x-evm.dts |   13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/am335x-evm.dts b/arch/arm/boot/dts/am335x-evm.dts
> index e4e1ccb..7f0fd6c 100644
> --- a/arch/arm/boot/dts/am335x-evm.dts
> +++ b/arch/arm/boot/dts/am335x-evm.dts
> @@ -46,3 +46,16 @@
>  	regulator-boot-on;
>  	regulator-always-on;
>  };
> +
> +&am3358_pinmux {
> +
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&d_can1_pins>;
> +
> +	d_can1_pins: pinmux_d_can_pins {
> +		pinctrl-single,pins = <
> +			0x168 0x2	/* uart0_ctsn.d_can1_tx, OUTPUT | MODE2 */
> +			0x16C 0x32	/* uart0_rtsn.d_can1_rx, INPUT_PULLUP | MODE2 */
> +		>;
> +	};
> +};
> -- 
> 1.7.9.5
> 
> 

Re: [PATCH 0/4] Add DT support to C_CAN/D_CAN controller

[Marc Kleine-Budde]

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

On 07/25/2012 08:23 AM, AnilKumar Ch wrote:
> This patch series adds the device tree support to C_CAN/D_CAN
> controller with pinmux configuration and device tree data
> addition to corresponding dts and dtsi files.
> 
> Also adds Runtime PM support to C_CAN/D_CAN controller.
> 
> These patches have been tested on AM335x EVM using some additional
> patch 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 to mainline.
> 
> 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.
> 
> First and last patches applies on top of linux-next tree, 2nd and
> 3rd patches applies on linux-omap tree.

I think 1st and 4th patch (if not all) should go though the
linux-can-next tree. Please rebase against linux-can-next master. If you
need features from the v3.6 release cycle we have to wait until David
merges then back.

regards, Marc

> 
> AnilKumar Ch (4):
>   can: c_can: Add device tree support to Bosch C_CAN/D_CAN controller
>   arm/dts: AM33XX: Add D_CAN device tree data
>   arm/dts: AM33XX: Configure pinmuxs for D_CAN1 on AM335x-EVM
>   can: c_can: Add runtime PM support to Bosch C_CAN/D_CAN controller
> 
>  .../devicetree/bindings/net/can/c_can.txt          |   38 +++++++++++++
>  arch/arm/boot/dts/am335x-evm.dts                   |   13 +++++
>  arch/arm/boot/dts/am33xx.dtsi                      |    5 ++
>  drivers/net/can/c_can/c_can_platform.c             |   60 +++++++++++++++-----
>  4 files changed, 101 insertions(+), 15 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/net/can/c_can.txt
> 


-- 
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 0/4] Add DT support to C_CAN/D_CAN controller

[AnilKumar, Chimata]

Marc,

Thanks for the review.

On Wed, Jul 25, 2012 at 14:34:21, Marc Kleine-Budde wrote:
> On 07/25/2012 08:23 AM, AnilKumar Ch wrote:
> > This patch series adds the device tree support to C_CAN/D_CAN
> > controller with pinmux configuration and device tree data
> > addition to corresponding dts and dtsi files.
> > 
> > Also adds Runtime PM support to C_CAN/D_CAN controller.
> > 
> > These patches have been tested on AM335x EVM using some additional
> > patch 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 to mainline.
> > 
> > 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.
> > 
> > First and last patches applies on top of linux-next tree, 2nd and
> > 3rd patches applies on linux-omap tree.
> 
> I think 1st and 4th patch (if not all) should go though the
> linux-can-next tree. Please rebase against linux-can-next master. If you
> need features from the v3.6 release cycle we have to wait until David
> merges then back.
> 

I will separate out into two sereis one with 1st and 4th patches and other
with 2nd and 3rd patches. I will submit v2 based on linux-can-nest master.

Thanks
AnilKumar

Re: [PATCH 1/4] can: c_can: Add device tree support to Bosch C_CAN/D_CAN controller

[Marc Kleine-Budde]

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

On 07/25/2012 08:23 AM, 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.

AFAIK you should add devicetree-discuss@lists.ozlabs.org on Cc for DT
related patches. More comments inline.

> 
> Signed-off-by: AnilKumar Ch <anilkumar@ti.com>
> ---
>  .../devicetree/bindings/net/can/c_can.txt          |   38 ++++++++++++++
>  drivers/net/can/c_can/c_can_platform.c             |   52 ++++++++++++++------
>  2 files changed, 75 insertions(+), 15 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..78b8ae8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/can/c_can.txt
> @@ -0,0 +1,38 @@
> +Bosch C_CAN/D_CAN controller Device Tree Bindings
> +-------------------------------------------------
> +
> +Required properties:
> +- compatible		: Should be "bosch,c_can_platform" or "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
> +- interrupt-parent	: The parent interrupt controller
> +
> +Optional properties:
> +- 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 6ff7ad0..2f5b153 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,51 @@ 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[] = {
> +	{
> +		.name = KBUILD_MODNAME,
> +		.driver_data = C_CAN_DEVTYPE,
> +	}, {
> +		.name = "c_can",
> +		.driver_data = C_CAN_DEVTYPE,
> +	}, {
> +		.name = "d_can",
> +		.driver_data = D_CAN_DEVTYPE,
> +	}, {
> +	}
> +};
> +
> +static const struct of_device_id c_can_of_table[] = {
> +	{ .compatible = "bosch,c_can_platform", .data = &c_can_id_table[0] },

Please don't add the "legacy bosch,c_can_platform" to the device tree
bindings. I personally would appreciate if you introduce an enum as
array index (BOSCH_C_CAN_PLATFORM, BOSCH_C_CAN, BOSCH_D_CAN) and
initialize the c_can_id_table above using the array indexes, then you
can use these indexes here too.

> +	{ .compatible = "bosch,c_can", .data = &c_can_id_table[1] },
> +	{ .compatible = "bosch,d_can", .data = &c_can_id_table[2] },
> +	{ /* 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 +150,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 C_CAN_DEVTYPE:
>  		priv->regs = reg_map_c_can;
> @@ -195,24 +230,11 @@ static int __devexit c_can_plat_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> -static const struct platform_device_id c_can_id_table[] = {
> -	{
> -		.name = KBUILD_MODNAME,
> -		.driver_data = C_CAN_DEVTYPE,
> -	}, {
> -		.name = "c_can",
> -		.driver_data = C_CAN_DEVTYPE,
> -	}, {
> -		.name = "d_can",
> -		.driver_data = D_CAN_DEVTYPE,
> -	}, {
> -	}
> -};
> -
>  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),
> 

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 0/4] Add DT support to C_CAN/D_CAN controller

[Marc Kleine-Budde]

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

On 07/25/2012 11:19 AM, AnilKumar, Chimata wrote:
[...]

>>> First and last patches applies on top of linux-next tree, 2nd and
>>> 3rd patches applies on linux-omap tree.
>>
>> I think 1st and 4th patch (if not all) should go though the
>> linux-can-next tree. Please rebase against linux-can-next master. If you
>> need features from the v3.6 release cycle we have to wait until David
>> merges then back.
>>
> 
> I will separate out into two sereis one with 1st and 4th patches and other
> with 2nd and 3rd patches. I will submit v2 based on linux-can-nest master.

I think the #ifdef CONFIG_HAVE_CLK will be removed in -rc1, maybe it's
better to wait until this is done.

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 0/4] Add DT support to C_CAN/D_CAN controller

[AnilKumar, Chimata]

Hi Marc,

On Wed, Jul 25, 2012 at 14:56:35, Marc Kleine-Budde wrote:
> On 07/25/2012 11:19 AM, AnilKumar, Chimata wrote:
> [...]
> 
> >>> First and last patches applies on top of linux-next tree, 2nd and
> >>> 3rd patches applies on linux-omap tree.
> >>
> >> I think 1st and 4th patch (if not all) should go though the
> >> linux-can-next tree. Please rebase against linux-can-next master. If you
> >> need features from the v3.6 release cycle we have to wait until David
> >> merges then back.
> >>
> > 
> > I will separate out into two sereis one with 1st and 4th patches and other
> > with 2nd and 3rd patches. I will submit v2 based on linux-can-nest master.
> 
> I think the #ifdef CONFIG_HAVE_CLK will be removed in -rc1, maybe it's
> better to wait until this is done.

My patches are based on linux-next tree have this patch, which removes
CONFIG_HAVE_CLK dependency.

In that case we can use this time for reviewing the patches and once the
-rc1 is ready we can push these patches.

I will submit v2 based on your review comments.

Thanks
AnilKumar

RE: [PATCH 1/4] can: c_can: Add device tree support to Bosch C_CAN/D_CAN controller

[AnilKumar, Chimata]

On Wed, Jul 25, 2012 at 14:54:24, Marc Kleine-Budde wrote:
> On 07/25/2012 08:23 AM, 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.
> 
> AFAIK you should add devicetree-discuss@lists.ozlabs.org on Cc for DT
> related patches. More comments inline.

I will take care in v2

> 
> > 
> > Signed-off-by: AnilKumar Ch <anilkumar@ti.com>
> > ---
> >  .../devicetree/bindings/net/can/c_can.txt          |   38 ++++++++++++++
> >  drivers/net/can/c_can/c_can_platform.c             |   52 ++++++++++++++------
> >  2 files changed, 75 insertions(+), 15 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..78b8ae8
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/can/c_can.txt
> > @@ -0,0 +1,38 @@
> > +Bosch C_CAN/D_CAN controller Device Tree Bindings
> > +-------------------------------------------------
> > +
> > +Required properties:
> > +- compatible		: Should be "bosch,c_can_platform" or "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
> > +- interrupt-parent	: The parent interrupt controller
> > +
> > +Optional properties:
> > +- 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 6ff7ad0..2f5b153 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,51 @@ 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[] = {
> > +	{
> > +		.name = KBUILD_MODNAME,
> > +		.driver_data = C_CAN_DEVTYPE,
> > +	}, {
> > +		.name = "c_can",
> > +		.driver_data = C_CAN_DEVTYPE,
> > +	}, {
> > +		.name = "d_can",
> > +		.driver_data = D_CAN_DEVTYPE,
> > +	}, {
> > +	}
> > +};
> > +
> > +static const struct of_device_id c_can_of_table[] = {
> > +	{ .compatible = "bosch,c_can_platform", .data = &c_can_id_table[0] },
> 
> Please don't add the "legacy bosch,c_can_platform" to the device tree

I will remove this from DT bindings.

> bindings. I personally would appreciate if you introduce an enum as
> array index (BOSCH_C_CAN_PLATFORM, BOSCH_C_CAN, BOSCH_D_CAN) and
> initialize the c_can_id_table above using the array indexes, then you
> can use these indexes here too.

What about having the same enum for devtype and index like

enum c_can_dev_id {
	BOSCH_C_CAN_PLATFORM,
	BOSCH_C_CAN,
	BOSCH_D_CAN,
};

static struct platform_device_id c_can_id_table[] = {
	[BOSCH_C_CAN_PLATFORM] = {
		.name = KBUILD_MODNAME,
		.driver_data = BOSCH_C_CAN,
	},
};

Regards
AnilKumar

Re: [PATCH 1/4] can: c_can: Add device tree support to Bosch C_CAN/D_CAN controller

[Marc Kleine-Budde]

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

On 07/25/2012 12:06 PM, AnilKumar, Chimata wrote:
[...]
>>> --- 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,51 @@ 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[] = {
>>> +	{
>>> +		.name = KBUILD_MODNAME,
>>> +		.driver_data = C_CAN_DEVTYPE,
>>> +	}, {
>>> +		.name = "c_can",
>>> +		.driver_data = C_CAN_DEVTYPE,
>>> +	}, {
>>> +		.name = "d_can",
>>> +		.driver_data = D_CAN_DEVTYPE,
>>> +	}, {
>>> +	}
>>> +};
>>> +
>>> +static const struct of_device_id c_can_of_table[] = {
>>> +	{ .compatible = "bosch,c_can_platform", .data = &c_can_id_table[0] },
>>
>> Please don't add the "legacy bosch,c_can_platform" to the device tree
> 
> I will remove this from DT bindings.
> 
>> bindings. I personally would appreciate if you introduce an enum as
>> array index (BOSCH_C_CAN_PLATFORM, BOSCH_C_CAN, BOSCH_D_CAN) and
>> initialize the c_can_id_table above using the array indexes, then you
>> can use these indexes here too.
> 
> What about having the same enum for devtype and index like

That was 100% exactly what I was thinking of :)

> enum c_can_dev_id {
> 	BOSCH_C_CAN_PLATFORM,
> 	BOSCH_C_CAN,
> 	BOSCH_D_CAN,
> };
> 
> static struct platform_device_id c_can_id_table[] = {
> 	[BOSCH_C_CAN_PLATFORM] = {
> 		.name = KBUILD_MODNAME,
> 		.driver_data = BOSCH_C_CAN,
> 	},
> };

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