[PATCH v2 0/4] can: c_can: Add suspend/resume and pinctrl support

[PATCH v2 0/4] can: c_can: Add suspend/resume and pinctrl support

[AnilKumar Ch]

Adds suspend/resume functionality of d_can driver along with d_can
raminit support which is required to initialize RAM while data
transmission.

This patch series also adds pinctrl support to c_can driver. These
patches were tested on AM335x-EVM.

This patch-series is based ontop of runtimePM patch submitted
earlier - https://patchwork.kernel.org/patch/1348081/

Changes from v1:
	- Incorporated Marc's comments on v1
	  * Changed marco name with units in it
	  * Used ndev->flags for link status instead of new member
	  * Added BUG_ON inside *_power_down and *_power_up APIs
	  * Added new member "type" in c_can_priv struct for keeping
	    dev id details/name.

AnilKumar Ch (4):
  can: c_can: Adopt pinctrl support
  can: c_can: Add d_can raminit support
  ARM: AM33XX: board-generic: Add of_dev_auxdata to pass d_can raminit
  can: c_can: Add d_can suspend resume support

 arch/arm/mach-omap2/board-generic.c    |   40 +++++++++++++-
 arch/arm/mach-omap2/control.h          |    4 ++
 drivers/net/can/c_can/c_can.c          |   95 ++++++++++++++++++++++++++++++++
 drivers/net/can/c_can/c_can.h          |   10 ++++
 drivers/net/can/c_can/c_can_platform.c |   79 ++++++++++++++++++++++++++
 include/linux/can/platform/c_can.h     |   36 ++++++++++++
 6 files changed, 263 insertions(+), 1 deletion(-)
 create mode 100644 include/linux/can/platform/c_can.h

-- 
1.7.9.5

[PATCH RESEND 1/4] can: c_can: Adopt pinctrl support

[AnilKumar Ch]

Adopt pinctrl support to c_can driver based on c_can device
pointer, pinctrl driver configure SoC pins to d_can mode
according to definitions provided in .dts file.

In device specific device tree file 'pinctrl-names = "default";'
and 'pinctrl-0 = <&d_can1_pins>;' needs to add to configure pins
from c_can driver. d_can1_pins node contains the pinmux/config
details of d_can L/H pins.

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

diff --git a/drivers/net/can/c_can/c_can_platform.c b/drivers/net/can/c_can/c_can_platform.c
index 90801c4..c351975 100644
--- a/drivers/net/can/c_can/c_can_platform.c
+++ b/drivers/net/can/c_can/c_can_platform.c
@@ -33,6 +33,7 @@
 #include <linux/of.h>
 #include <linux/of_device.h>
 #include <linux/pm_runtime.h>
+#include <linux/pinctrl/consumer.h>
 
 #include <linux/can/dev.h>
 
@@ -98,6 +99,7 @@ static int __devinit c_can_plat_probe(struct platform_device *pdev)
 	struct c_can_priv *priv;
 	const struct of_device_id *match;
 	const struct platform_device_id *id;
+	struct pinctrl *pinctrl;
 	struct resource *mem;
 	int irq;
 	struct clk *clk;
@@ -114,6 +116,11 @@ static int __devinit c_can_plat_probe(struct platform_device *pdev)
 		id = platform_get_device_id(pdev);
 	}
 
+	pinctrl = devm_pinctrl_get_select_default(&pdev->dev);
+	if (IS_ERR(pinctrl))
+		dev_warn(&pdev->dev,
+			"failed to configure pins from driver\n");
+
 	/* get the appropriate clk */
 	clk = clk_get(&pdev->dev, NULL);
 	if (IS_ERR(clk)) {
-- 
1.7.9.5

[PATCH RESEND 2/4] can: c_can: Add d_can raminit support

[AnilKumar Ch]

Add D_CAN raminit support to C_CAN driver to enable D_CAN RAM,
which holds all the message objects during transmission or
receiving of data. This initialization/de-initialization should
be done in synchronous with D_CAN clock.

Signed-off-by: AnilKumar Ch <anilkumar@ti.com>
---
 drivers/net/can/c_can/c_can.c          |   13 ++++++++++++
 drivers/net/can/c_can/c_can.h          |    2 ++
 drivers/net/can/c_can/c_can_platform.c |   10 +++++++++
 include/linux/can/platform/c_can.h     |   36 ++++++++++++++++++++++++++++++++
 4 files changed, 61 insertions(+)
 create mode 100644 include/linux/can/platform/c_can.h

diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
index aa6c5eb..c175410 100644
--- a/drivers/net/can/c_can/c_can.c
+++ b/drivers/net/can/c_can/c_can.c
@@ -214,6 +214,12 @@ static inline void c_can_pm_runtime_put_sync(const struct c_can_priv *priv)
 		pm_runtime_put_sync(priv->device);
 }
 
+static inline void c_can_reset_ram(const struct c_can_priv *priv, bool enable)
+{
+	if (priv->ram_init)
+		priv->ram_init(priv->instance, enable);
+}
+
 static inline int get_tx_next_msg_obj(const struct c_can_priv *priv)
 {
 	return (priv->tx_next & C_CAN_NEXT_MSG_OBJ_MASK) +
@@ -1071,6 +1077,8 @@ static int c_can_open(struct net_device *dev)
 	struct c_can_priv *priv = netdev_priv(dev);
 
 	c_can_pm_runtime_get_sync(priv);
+	/* Initialize DCAN RAM */
+	c_can_reset_ram(priv, true);
 
 	/* open the can device */
 	err = open_candev(dev);
@@ -1099,6 +1107,8 @@ static int c_can_open(struct net_device *dev)
 exit_irq_fail:
 	close_candev(dev);
 exit_open_fail:
+	/* De-Initialize DCAN RAM */
+	c_can_reset_ram(priv, false);
 	c_can_pm_runtime_put_sync(priv);
 	return err;
 }
@@ -1112,6 +1122,9 @@ static int c_can_close(struct net_device *dev)
 	c_can_stop(dev);
 	free_irq(dev->irq, dev);
 	close_candev(dev);
+
+	/* De-Initialize DCAN RAM */
+	c_can_reset_ram(priv, false);
 	c_can_pm_runtime_put_sync(priv);
 
 	return 0;
diff --git a/drivers/net/can/c_can/c_can.h b/drivers/net/can/c_can/c_can.h
index 1437a6d..5f6339c 100644
--- a/drivers/net/can/c_can/c_can.h
+++ b/drivers/net/can/c_can/c_can.h
@@ -166,6 +166,8 @@ struct c_can_priv {
 	unsigned int tx_echo;
 	void *priv;		/* for board-specific data */
 	u16 irqstatus;
+	unsigned int instance;
+	void (*ram_init) (unsigned int instance, bool enable);
 };
 
 struct net_device *alloc_c_can_dev(void);
diff --git a/drivers/net/can/c_can/c_can_platform.c b/drivers/net/can/c_can/c_can_platform.c
index c351975..c6963b2 100644
--- a/drivers/net/can/c_can/c_can_platform.c
+++ b/drivers/net/can/c_can/c_can_platform.c
@@ -34,6 +34,7 @@
 #include <linux/of_device.h>
 #include <linux/pm_runtime.h>
 #include <linux/pinctrl/consumer.h>
+#include <linux/can/platform/c_can.h>
 
 #include <linux/can/dev.h>
 
@@ -98,6 +99,7 @@ static int __devinit c_can_plat_probe(struct platform_device *pdev)
 	struct net_device *dev;
 	struct c_can_priv *priv;
 	const struct of_device_id *match;
+	struct c_can_platform_data *pdata = NULL;
 	const struct platform_device_id *id;
 	struct pinctrl *pinctrl;
 	struct resource *mem;
@@ -179,6 +181,14 @@ static int __devinit c_can_plat_probe(struct platform_device *pdev)
 		priv->can.ctrlmode_supported |= CAN_CTRLMODE_3_SAMPLES;
 		priv->read_reg = c_can_plat_read_reg_aligned_to_16bit;
 		priv->write_reg = c_can_plat_write_reg_aligned_to_16bit;
+		pdata = pdev->dev.platform_data;
+		if (!pdata) {
+			dev_err(&pdev->dev, "d_can platform data missing\n");
+			ret = -EINVAL;
+			goto exit_free_device;
+		}
+		priv->ram_init = pdata->ram_init;
+		priv->instance = pdata->instance;
 		break;
 	default:
 		ret = -EINVAL;
diff --git a/include/linux/can/platform/c_can.h b/include/linux/can/platform/c_can.h
new file mode 100644
index 0000000..84b27d2
--- /dev/null
+++ b/include/linux/can/platform/c_can.h
@@ -0,0 +1,36 @@
+/*
+ * C_CAN controller driver platform header
+ *
+ * Copyright (C) 2012 Texas Instruments Incorporated - http://www.ti.com/
+ *
+ * Bosch C_CAN/D_CAN controller is compliant to CAN protocol version 2.0
+ * part A and B.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation version 2.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef __CAN_PLATFORM_C_CAN_H__
+#define __CAN_PLATFORM_C_CAN_H__
+
+/**
+ * struct c_can_platform_data - C_CAN/D_CAN Platform Data
+ *
+ * @instance:		CAN instance, required for d_can raminit
+ * @ram_init:		CAN RAM initialization
+ *
+ * Platform data structure to get all platform specific settings.
+ * this structure also accounts the fact that the IP may have different
+ * RAM for different SOC's
+ */
+struct c_can_platform_data {
+	unsigned int instance;
+	void (*ram_init) (unsigned int instance, bool enable);
+};
+#endif
-- 
1.7.9.5

[PATCH RESEND 3/4] ARM: AM33XX: board-generic: Add of_dev_auxdata to pass d_can raminit

[AnilKumar Ch]

Add of_dev_auxdata to pass d_can raminit callback APIs to initialize
d_can RAM. D_CAN RAM initialization bits are present in CONTROL module
address space, which can be accessed by platform specific code. So
callback functions are added to serve this purpose, this can done by
using of_dev_auxdata.

Two callback APIs are added to of_dev_auxdata used by two instances of
D_CAN IP. These callback functions are used to enable/disable D_CAN RAM
from CAN driver.

Signed-off-by: AnilKumar Ch <anilkumar@ti.com>
---
 arch/arm/mach-omap2/board-generic.c |   40 ++++++++++++++++++++++++++++++++++-
 arch/arm/mach-omap2/control.h       |    4 ++++
 2 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-omap2/board-generic.c b/arch/arm/mach-omap2/board-generic.c
index 6f93a20..b68e642 100644
--- a/arch/arm/mach-omap2/board-generic.c
+++ b/arch/arm/mach-omap2/board-generic.c
@@ -15,6 +15,7 @@
 #include <linux/of_irq.h>
 #include <linux/of_platform.h>
 #include <linux/irqdomain.h>
+#include <linux/can/platform/c_can.h>
 
 #include <mach/hardware.h>
 #include <asm/hardware/gic.h>
@@ -22,6 +23,8 @@
 
 #include <plat/board.h>
 #include "common.h"
+#include "control.h"
+#include "iomap.h"
 #include "common-board-devices.h"
 
 #if !(defined(CONFIG_ARCH_OMAP2) || defined(CONFIG_ARCH_OMAP3))
@@ -37,11 +40,46 @@ static struct of_device_id omap_dt_match_table[] __initdata = {
 	{ }
 };
 
+void d_can_hw_raminit(unsigned int instance, bool enable)
+{
+	u32 val;
+
+	val = readl(AM33XX_CTRL_REGADDR(AM33XX_CONTROL_DCAN_RAMINIT));
+	if (enable) {
+		val &= ~AM33XX_DCAN_RAMINIT_START_MASK(instance);
+		val |= AM33XX_DCAN_RAMINIT_START_MASK(instance);
+		writel(val, AM33XX_CTRL_REGADDR(AM33XX_CONTROL_DCAN_RAMINIT));
+	} else {
+		val &= ~AM33XX_DCAN_RAMINIT_START_MASK(instance);
+		writel(val, AM33XX_CTRL_REGADDR(AM33XX_CONTROL_DCAN_RAMINIT));
+	}
+}
+
+static struct c_can_platform_data d_can0_pdata = {
+	.ram_init	= d_can_hw_raminit,
+	.instance	= 0,
+};
+
+static struct c_can_platform_data d_can1_pdata = {
+	.ram_init	= d_can_hw_raminit,
+	.instance	= 1,
+};
+
+static const struct of_dev_auxdata am33xx_auxdata_lookup[] __initconst = {
+	OF_DEV_AUXDATA("bosch,d_can", 0x481cc000, NULL, &d_can0_pdata),
+	OF_DEV_AUXDATA("bosch,d_can", 0x481d0000, NULL, &d_can1_pdata),
+	{ },
+};
+
 static void __init omap_generic_init(void)
 {
 	omap_sdrc_init(NULL, NULL);
 
-	of_platform_populate(NULL, omap_dt_match_table, NULL, NULL);
+	if (of_machine_is_compatible("ti,am33xx"))
+		of_platform_populate(NULL, omap_dt_match_table,
+				am33xx_auxdata_lookup, NULL);
+	else
+		of_platform_populate(NULL, omap_dt_match_table, NULL, NULL);
 }
 
 #ifdef CONFIG_SOC_OMAP2420
diff --git a/arch/arm/mach-omap2/control.h b/arch/arm/mach-omap2/control.h
index b8cdc85..afd189b 100644
--- a/arch/arm/mach-omap2/control.h
+++ b/arch/arm/mach-omap2/control.h
@@ -356,6 +356,10 @@
 #define AM33XX_CONTROL_STATUS_SYSBOOT1_SHIFT		22
 #define AM33XX_CONTROL_STATUS_SYSBOOT1_MASK		(0x3 << 22)
 
+/* AM33XX DCAN bitfields */
+#define AM33XX_CONTROL_DCAN_RAMINIT		0x644
+#define AM33XX_DCAN_RAMINIT_START_MASK(i)	(1 << (i))
+
 /* CONTROL OMAP STATUS register to identify OMAP3 features */
 #define OMAP3_CONTROL_OMAP_STATUS	0x044c
 
-- 
1.7.9.5

[PATCH v2 4/4] can: c_can: Add d_can suspend resume support

[AnilKumar Ch]

Adds suspend resume support to DCAN driver which enables
DCAN power down mode bit (PDR). Then DCAN will ack the local
power-down mode by setting PDA bit in STATUS register.

Signed-off-by: AnilKumar Ch <anilkumar@ti.com>
---
 drivers/net/can/c_can/c_can.c          |   82 ++++++++++++++++++++++++++++++++
 drivers/net/can/c_can/c_can.h          |    8 ++++
 drivers/net/can/c_can/c_can_platform.c |   62 ++++++++++++++++++++++++
 3 files changed, 152 insertions(+)

diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
index c175410..c601136 100644
--- a/drivers/net/can/c_can/c_can.c
+++ b/drivers/net/can/c_can/c_can.c
@@ -46,6 +46,9 @@
 #define IF_ENUM_REG_LEN		11
 #define C_CAN_IFACE(reg, iface)	(C_CAN_IF1_##reg + (iface) * IF_ENUM_REG_LEN)
 
+/* control extension register D_CAN specific */
+#define CONTROL_EX_PDR		BIT(8)
+
 /* control register */
 #define CONTROL_TEST		BIT(7)
 #define CONTROL_CCE		BIT(6)
@@ -65,6 +68,7 @@
 #define TEST_BASIC		BIT(2)
 
 /* status register */
+#define STATUS_PDA		BIT(10)
 #define STATUS_BOFF		BIT(7)
 #define STATUS_EWARN		BIT(6)
 #define STATUS_EPASS		BIT(5)
@@ -164,6 +168,9 @@
 /* minimum timeout for checking BUSY status */
 #define MIN_TIMEOUT_VALUE	6
 
+/* Wait for ~1 sec for INIT bit */
+#define INIT_WAIT_MS		1000
+
 /* napi related */
 #define C_CAN_NAPI_WEIGHT	C_CAN_MSG_OBJ_RX_NUM
 
@@ -1154,6 +1161,81 @@ struct net_device *alloc_c_can_dev(void)
 }
 EXPORT_SYMBOL_GPL(alloc_c_can_dev);
 
+#ifdef CONFIG_PM
+int c_can_power_down(struct net_device *dev)
+{
+	u32 val;
+	unsigned long time_out;
+	struct c_can_priv *priv = netdev_priv(dev);
+
+	if (!(dev->flags & IFF_UP))
+		return 0;
+
+	BUG_ON(priv->type != BOSCH_D_CAN);
+
+	/* set PDR value so the device goes to power down mode */
+	val = priv->read_reg(priv, C_CAN_CTRL_EX_REG);
+	val |= CONTROL_EX_PDR;
+	priv->write_reg(priv, C_CAN_CTRL_EX_REG, val);
+
+	/* Wait for the PDA bit to get set */
+	time_out = jiffies + msecs_to_jiffies(INIT_WAIT_MS);
+	while (!(priv->read_reg(priv, C_CAN_STS_REG) & STATUS_PDA) &&
+				time_after(time_out, jiffies))
+		cpu_relax();
+
+	if (time_after(jiffies, time_out))
+		return -ETIMEDOUT;
+
+	c_can_stop(dev);
+
+	/* De-initialize DCAN RAM */
+	c_can_reset_ram(priv, false);
+	c_can_pm_runtime_put_sync(priv);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(c_can_power_down);
+
+int c_can_power_up(struct net_device *dev)
+{
+	u32 val;
+	unsigned long time_out;
+	struct c_can_priv *priv = netdev_priv(dev);
+
+	if (!(dev->flags & IFF_UP))
+		return 0;
+
+	BUG_ON(priv->type != BOSCH_D_CAN);
+
+	c_can_pm_runtime_get_sync(priv);
+	/* Initialize DCAN RAM */
+	c_can_reset_ram(priv, true);
+
+	/* Clear PDR and INIT bits */
+	val = priv->read_reg(priv, C_CAN_CTRL_EX_REG);
+	val &= ~CONTROL_EX_PDR;
+	priv->write_reg(priv, C_CAN_CTRL_EX_REG, val);
+	val = priv->read_reg(priv, C_CAN_CTRL_REG);
+	val &= ~CONTROL_INIT;
+	priv->write_reg(priv, C_CAN_CTRL_REG, val);
+
+	/* Wait for the PDA bit to get clear */
+	time_out = jiffies + msecs_to_jiffies(INIT_WAIT_MS);
+	while ((priv->read_reg(priv, C_CAN_STS_REG) & STATUS_PDA) &&
+				time_after(time_out, jiffies))
+		cpu_relax();
+
+	if (time_after(jiffies, time_out))
+		return -ETIMEDOUT;
+
+	c_can_start(dev);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(c_can_power_up);
+#endif
+
 void free_c_can_dev(struct net_device *dev)
 {
 	free_candev(dev);
diff --git a/drivers/net/can/c_can/c_can.h b/drivers/net/can/c_can/c_can.h
index 5f6339c..ca149eb 100644
--- a/drivers/net/can/c_can/c_can.h
+++ b/drivers/net/can/c_can/c_can.h
@@ -24,6 +24,7 @@
 
 enum reg {
 	C_CAN_CTRL_REG = 0,
+	C_CAN_CTRL_EX_REG,
 	C_CAN_STS_REG,
 	C_CAN_ERR_CNT_REG,
 	C_CAN_BTR_REG,
@@ -104,6 +105,7 @@ static const u16 reg_map_c_can[] = {
 
 static const u16 reg_map_d_can[] = {
 	[C_CAN_CTRL_REG]	= 0x00,
+	[C_CAN_CTRL_EX_REG]	= 0x02,
 	[C_CAN_STS_REG]		= 0x04,
 	[C_CAN_ERR_CNT_REG]	= 0x08,
 	[C_CAN_BTR_REG]		= 0x0C,
@@ -166,6 +168,7 @@ struct c_can_priv {
 	unsigned int tx_echo;
 	void *priv;		/* for board-specific data */
 	u16 irqstatus;
+	enum c_can_dev_id type;
 	unsigned int instance;
 	void (*ram_init) (unsigned int instance, bool enable);
 };
@@ -175,4 +178,9 @@ void free_c_can_dev(struct net_device *dev);
 int register_c_can_dev(struct net_device *dev);
 void unregister_c_can_dev(struct net_device *dev);
 
+#ifdef CONFIG_PM
+int c_can_power_up(struct net_device *dev);
+int c_can_power_down(struct net_device *dev);
+#endif
+
 #endif /* C_CAN_H */
diff --git a/drivers/net/can/c_can/c_can_platform.c b/drivers/net/can/c_can/c_can_platform.c
index c6963b2..68c186b 100644
--- a/drivers/net/can/c_can/c_can_platform.c
+++ b/drivers/net/can/c_can/c_can_platform.c
@@ -202,6 +202,7 @@ static int __devinit c_can_plat_probe(struct platform_device *pdev)
 	priv->device = &pdev->dev;
 	priv->can.clock.freq = clk_get_rate(clk);
 	priv->priv = clk;
+	priv->type = id->driver_data;
 
 	platform_set_drvdata(pdev, dev);
 	SET_NETDEV_DEV(dev, &pdev->dev);
@@ -255,6 +256,65 @@ static int __devexit c_can_plat_remove(struct platform_device *pdev)
 	return 0;
 }
 
+#ifdef CONFIG_PM
+static int c_can_suspend(struct platform_device *pdev, pm_message_t state)
+{
+	int ret;
+	struct net_device *ndev = platform_get_drvdata(pdev);
+	struct c_can_priv *priv = netdev_priv(ndev);
+
+	if (priv->type != BOSCH_D_CAN) {
+		dev_warn(&pdev->dev, "Not supported\n");
+		return 0;
+	}
+
+	if (netif_running(ndev)) {
+		netif_stop_queue(ndev);
+		netif_device_detach(ndev);
+	}
+
+	ret = c_can_power_down(ndev);
+	if (ret) {
+		netdev_err(ndev, "failed to enter power down mode\n");
+		return ret;
+	}
+
+	priv->can.state = CAN_STATE_SLEEPING;
+
+	return 0;
+}
+
+static int c_can_resume(struct platform_device *pdev)
+{
+	int ret;
+	struct net_device *ndev = platform_get_drvdata(pdev);
+	struct c_can_priv *priv = netdev_priv(ndev);
+
+	if (priv->type != BOSCH_D_CAN) {
+		dev_warn(&pdev->dev, "Not supported\n");
+		return 0;
+	}
+
+	ret = c_can_power_up(ndev);
+	if (ret) {
+		netdev_err(ndev, "Still in power down mode\n");
+		return ret;
+	}
+
+	priv->can.state = CAN_STATE_ERROR_ACTIVE;
+
+	if (netif_running(ndev)) {
+		netif_device_attach(ndev);
+		netif_start_queue(ndev);
+	}
+
+	return 0;
+}
+#else
+#define c_can_suspend NULL
+#define c_can_resume NULL
+#endif
+
 static struct platform_driver c_can_plat_driver = {
 	.driver = {
 		.name = KBUILD_MODNAME,
@@ -263,6 +323,8 @@ static struct platform_driver c_can_plat_driver = {
 	},
 	.probe = c_can_plat_probe,
 	.remove = __devexit_p(c_can_plat_remove),
+	.suspend = c_can_suspend,
+	.resume = c_can_resume,
 	.id_table = c_can_id_table,
 };
 
-- 
1.7.9.5

[PATCH v2 0/4] can: c_can: Add suspend/resume and pinctrl support

[Marc Kleine-Budde]

On 09/05/2012 01:12 PM, AnilKumar Ch wrote:
> Adds suspend/resume functionality of d_can driver along with d_can
> raminit support which is required to initialize RAM while data
> transmission.
> 
> This patch series also adds pinctrl support to c_can driver. These
> patches were tested on AM335x-EVM.
> 
> This patch-series is based ontop of runtimePM patch submitted
> earlier - https://patchwork.kernel.org/patch/1348081/

Looks good, I'll apply the series after Tony acks the patch 3
([PATCH RESEND 3/4] ARM: AM33XX: board-generic: Add of_dev_auxdata to
pass d_can raminit).

Marc
> 
> Changes from v1:
> 	- Incorporated Marc's comments on v1
> 	  * Changed marco name with units in it
> 	  * Used ndev->flags for link status instead of new member
> 	  * Added BUG_ON inside *_power_down and *_power_up APIs
> 	  * Added new member "type" in c_can_priv struct for keeping
> 	    dev id details/name.
> 
> AnilKumar Ch (4):
>   can: c_can: Adopt pinctrl support
>   can: c_can: Add d_can raminit support
>   ARM: AM33XX: board-generic: Add of_dev_auxdata to pass d_can raminit
>   can: c_can: Add d_can suspend resume support
> 
>  arch/arm/mach-omap2/board-generic.c    |   40 +++++++++++++-
>  arch/arm/mach-omap2/control.h          |    4 ++
>  drivers/net/can/c_can/c_can.c          |   95 ++++++++++++++++++++++++++++++++
>  drivers/net/can/c_can/c_can.h          |   10 ++++
>  drivers/net/can/c_can/c_can_platform.c |   79 ++++++++++++++++++++++++++
>  include/linux/can/platform/c_can.h     |   36 ++++++++++++
>  6 files changed, 263 insertions(+), 1 deletion(-)
>  create mode 100644 include/linux/can/platform/c_can.h
> 


-- 
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/20120905/17236821/attachment.sig>

[PATCH v2 0/4] can: c_can: Add suspend/resume and pinctrl support

[Marc Kleine-Budde]

On 09/05/2012 01:54 PM, Marc Kleine-Budde wrote:
> On 09/05/2012 01:12 PM, AnilKumar Ch wrote:
>> Adds suspend/resume functionality of d_can driver along with d_can
>> raminit support which is required to initialize RAM while data
>> transmission.
>>
>> This patch series also adds pinctrl support to c_can driver. These
>> patches were tested on AM335x-EVM.
>>
>> This patch-series is based ontop of runtimePM patch submitted
>> earlier - https://patchwork.kernel.org/patch/1348081/
> 
> Looks good, I'll apply the series after Tony acks the patch 3
> ([PATCH RESEND 3/4] ARM: AM33XX: board-generic: Add of_dev_auxdata to
> pass d_can raminit).

I've pushed all patches to can-next/master [1]. When Tony gives his Ack,
I'll send David a pull request (of a tree including that Ack).

Marc

[1] https://gitorious.org/linux-can/linux-can-next

-- 
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/20120905/05315798/attachment-0001.sig>

[PATCH RESEND 3/4] ARM: AM33XX: board-generic: Add of_dev_auxdata to pass d_can raminit

[Tony Lindgren]

Hi,

* AnilKumar Ch <anilkumar@ti.com> [120905 04:14]:
> Add of_dev_auxdata to pass d_can raminit callback APIs to initialize
> d_can RAM. D_CAN RAM initialization bits are present in CONTROL module
> address space, which can be accessed by platform specific code. So
> callback functions are added to serve this purpose, this can done by
> using of_dev_auxdata.
> 
> Two callback APIs are added to of_dev_auxdata used by two instances of
> D_CAN IP. These callback functions are used to enable/disable D_CAN RAM
> from CAN driver.

I'd like to avoid the callbacks to the platform code where possible as
that's the biggest pain we already have moving things to work with device
tree for the existing drivers.

And I'm pretty convinced that whatever is done with callbacks should be
done with some Linux generic framework from the driver that has it's own
binding, such as clock framework, regulator framework, pinctrl framework,
runtime PM etc.

> --- a/arch/arm/mach-omap2/board-generic.c
> +++ b/arch/arm/mach-omap2/board-generic.c
> @@ -37,11 +40,46 @@ static struct of_device_id omap_dt_match_table[] __initdata = {
>  	{ }
>  };
>  
> +void d_can_hw_raminit(unsigned int instance, bool enable)
> +{
> +	u32 val;
> +
> +	val = readl(AM33XX_CTRL_REGADDR(AM33XX_CONTROL_DCAN_RAMINIT));
> +	if (enable) {
> +		val &= ~AM33XX_DCAN_RAMINIT_START_MASK(instance);
> +		val |= AM33XX_DCAN_RAMINIT_START_MASK(instance);
> +		writel(val, AM33XX_CTRL_REGADDR(AM33XX_CONTROL_DCAN_RAMINIT));
> +	} else {
> +		val &= ~AM33XX_DCAN_RAMINIT_START_MASK(instance);
> +		writel(val, AM33XX_CTRL_REGADDR(AM33XX_CONTROL_DCAN_RAMINIT));
> +	}
> +}

This part does not look good to me, this is tweaking the omap control
module register bits directly. To me it seems that the above should
be implemented in the omap/am33xx hwmod code that gets initialized when
the dcan driver calls pm_runtime_enable()? Paul, got any other ideas?

Regards,

Tony

[PATCH RESEND 1/4] can: c_can: Adopt pinctrl support

[Tony Lindgren]

* AnilKumar Ch <anilkumar@ti.com> [120905 04:14]:
> Adopt pinctrl support to c_can driver based on c_can device
> pointer, pinctrl driver configure SoC pins to d_can mode
> according to definitions provided in .dts file.
> 
> In device specific device tree file 'pinctrl-names = "default";'
> and 'pinctrl-0 = <&d_can1_pins>;' needs to add to configure pins
> from c_can driver. d_can1_pins node contains the pinmux/config
> details of d_can L/H pins.
> 
> Signed-off-by: AnilKumar Ch <anilkumar@ti.com>

Acked-by: Tony Lindgren <tony@atomide.com>

> ---
>  drivers/net/can/c_can/c_can_platform.c |    7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/net/can/c_can/c_can_platform.c b/drivers/net/can/c_can/c_can_platform.c
> index 90801c4..c351975 100644
> --- a/drivers/net/can/c_can/c_can_platform.c
> +++ b/drivers/net/can/c_can/c_can_platform.c
> @@ -33,6 +33,7 @@
>  #include <linux/of.h>
>  #include <linux/of_device.h>
>  #include <linux/pm_runtime.h>
> +#include <linux/pinctrl/consumer.h>
>  
>  #include <linux/can/dev.h>
>  
> @@ -98,6 +99,7 @@ static int __devinit c_can_plat_probe(struct platform_device *pdev)
>  	struct c_can_priv *priv;
>  	const struct of_device_id *match;
>  	const struct platform_device_id *id;
> +	struct pinctrl *pinctrl;
>  	struct resource *mem;
>  	int irq;
>  	struct clk *clk;
> @@ -114,6 +116,11 @@ static int __devinit c_can_plat_probe(struct platform_device *pdev)
>  		id = platform_get_device_id(pdev);
>  	}
>  
> +	pinctrl = devm_pinctrl_get_select_default(&pdev->dev);
> +	if (IS_ERR(pinctrl))
> +		dev_warn(&pdev->dev,
> +			"failed to configure pins from driver\n");
> +
>  	/* get the appropriate clk */
>  	clk = clk_get(&pdev->dev, NULL);
>  	if (IS_ERR(clk)) {
> -- 
> 1.7.9.5
> 

[PATCH RESEND 2/4] can: c_can: Add d_can raminit support

[Tony Lindgren]

* AnilKumar Ch <anilkumar@ti.com> [120905 04:14]:
> Add D_CAN raminit support to C_CAN driver to enable D_CAN RAM,
> which holds all the message objects during transmission or
> receiving of data. This initialization/de-initialization should
> be done in synchronous with D_CAN clock.

Sounds like you should just implement runtime PM calls in the driver
and let the lower level code take care of this for you along with the
clocks etc.

> @@ -1071,6 +1077,8 @@ static int c_can_open(struct net_device *dev)
>  	struct c_can_priv *priv = netdev_priv(dev);
>  
>  	c_can_pm_runtime_get_sync(priv);
> +	/* Initialize DCAN RAM */
> +	c_can_reset_ram(priv, true);
>  
>  	/* open the can device */
>  	err = open_candev(dev);
> @@ -1099,6 +1107,8 @@ static int c_can_open(struct net_device *dev)
>  exit_irq_fail:
>  	close_candev(dev);
>  exit_open_fail:
> +	/* De-Initialize DCAN RAM */
> +	c_can_reset_ram(priv, false);
>  	c_can_pm_runtime_put_sync(priv);
>  	return err;
>  }

Oh your already have pm_runtime here, OK so yeah let's let the omap/am33xx
hwmod code do this for you. No changes to this driver needed then?

Tony

[PATCH v2 0/4] can: c_can: Add suspend/resume and pinctrl support

[Tony Lindgren]

* Marc Kleine-Budde <mkl@pengutronix.de> [120905 06:55]:
> On 09/05/2012 01:54 PM, Marc Kleine-Budde wrote:
> > On 09/05/2012 01:12 PM, AnilKumar Ch wrote:
> >> Adds suspend/resume functionality of d_can driver along with d_can
> >> raminit support which is required to initialize RAM while data
> >> transmission.
> >>
> >> This patch series also adds pinctrl support to c_can driver. These
> >> patches were tested on AM335x-EVM.
> >>
> >> This patch-series is based ontop of runtimePM patch submitted
> >> earlier - https://patchwork.kernel.org/patch/1348081/
> > 
> > Looks good, I'll apply the series after Tony acks the patch 3
> > ([PATCH RESEND 3/4] ARM: AM33XX: board-generic: Add of_dev_auxdata to
> > pass d_can raminit).
> 
> I've pushed all patches to can-next/master [1]. When Tony gives his Ack,
> I'll send David a pull request (of a tree including that Ack).

To me it looks like we can avoid the platform callbacks and the ram
init changes to the driver by implementing right runtime PM calls
for omap/am33xx. So NAK on few patches, and ACK on one :)

Regards,

Tony

> [1] https://gitorious.org/linux-can/linux-can-next

[PATCH RESEND 3/4] ARM: AM33XX: board-generic: Add of_dev_auxdata to pass d_can raminit

[Vaibhav Hiremath]

On 9/6/2012 4:48 AM, Tony Lindgren wrote:
> Hi,
> 
> * AnilKumar Ch <anilkumar@ti.com> [120905 04:14]:
>> Add of_dev_auxdata to pass d_can raminit callback APIs to initialize
>> d_can RAM. D_CAN RAM initialization bits are present in CONTROL module
>> address space, which can be accessed by platform specific code. So
>> callback functions are added to serve this purpose, this can done by
>> using of_dev_auxdata.
>>
>> Two callback APIs are added to of_dev_auxdata used by two instances of
>> D_CAN IP. These callback functions are used to enable/disable D_CAN RAM
>> from CAN driver.
> 
> I'd like to avoid the callbacks to the platform code where possible as
> that's the biggest pain we already have moving things to work with device
> tree for the existing drivers.
> 
> And I'm pretty convinced that whatever is done with callbacks should be
> done with some Linux generic framework from the driver that has it's own
> binding, such as clock framework, regulator framework, pinctrl framework,
> runtime PM etc.
> 
>> --- a/arch/arm/mach-omap2/board-generic.c
>> +++ b/arch/arm/mach-omap2/board-generic.c
>> @@ -37,11 +40,46 @@ static struct of_device_id omap_dt_match_table[] __initdata = {
>>  	{ }
>>  };
>>  
>> +void d_can_hw_raminit(unsigned int instance, bool enable)
>> +{
>> +	u32 val;
>> +
>> +	val = readl(AM33XX_CTRL_REGADDR(AM33XX_CONTROL_DCAN_RAMINIT));
>> +	if (enable) {
>> +		val &= ~AM33XX_DCAN_RAMINIT_START_MASK(instance);
>> +		val |= AM33XX_DCAN_RAMINIT_START_MASK(instance);
>> +		writel(val, AM33XX_CTRL_REGADDR(AM33XX_CONTROL_DCAN_RAMINIT));
>> +	} else {
>> +		val &= ~AM33XX_DCAN_RAMINIT_START_MASK(instance);
>> +		writel(val, AM33XX_CTRL_REGADDR(AM33XX_CONTROL_DCAN_RAMINIT));
>> +	}
>> +}
> 
> This part does not look good to me, this is tweaking the omap control
> module register bits directly. To me it seems that the above should
> be implemented in the omap/am33xx hwmod code that gets initialized when
> the dcan driver calls pm_runtime_enable()? Paul, got any other ideas?
> 

Technically yes, this is required during module enable/disable sequence.
But there is no way currently supported in hwmod layer. Also I am not
quite sure how many other modules/devices may use this.

Couple of more examples I have here,

In case of AM3517 we have similar SoC integration, where VPFE, MAC and
USB required clock control (handled by clock-tree) and interrupt status
(handled by callbacks) from control module.
So not sure whether we can get rid of callbacks until we have control
module MFD driver (on which Konstantin is working on)

Thanks,
Vaibhav

> Regards,
> 
> Tony
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

[PATCH RESEND 3/4] ARM: AM33XX: board-generic: Add of_dev_auxdata to pass d_can raminit

[Bedia, Vaibhav]

On Thu, Sep 06, 2012 at 09:26:07, Hiremath, Vaibhav wrote:
> 
> 
> On 9/6/2012 4:48 AM, Tony Lindgren wrote:
> > Hi,
> > 
> > * AnilKumar Ch <anilkumar@ti.com> [120905 04:14]:
> >> Add of_dev_auxdata to pass d_can raminit callback APIs to initialize
> >> d_can RAM. D_CAN RAM initialization bits are present in CONTROL module
> >> address space, which can be accessed by platform specific code. So
> >> callback functions are added to serve this purpose, this can done by
> >> using of_dev_auxdata.
> >>
> >> Two callback APIs are added to of_dev_auxdata used by two instances of
> >> D_CAN IP. These callback functions are used to enable/disable D_CAN RAM
> >> from CAN driver.
> > 
> > I'd like to avoid the callbacks to the platform code where possible as
> > that's the biggest pain we already have moving things to work with device
> > tree for the existing drivers.
> > 
> > And I'm pretty convinced that whatever is done with callbacks should be
> > done with some Linux generic framework from the driver that has it's own
> > binding, such as clock framework, regulator framework, pinctrl framework,
> > runtime PM etc.
> > 
> >> --- a/arch/arm/mach-omap2/board-generic.c
> >> +++ b/arch/arm/mach-omap2/board-generic.c
> >> @@ -37,11 +40,46 @@ static struct of_device_id omap_dt_match_table[] __initdata = {
> >>  	{ }
> >>  };
> >>  
> >> +void d_can_hw_raminit(unsigned int instance, bool enable)
> >> +{
> >> +	u32 val;
> >> +
> >> +	val = readl(AM33XX_CTRL_REGADDR(AM33XX_CONTROL_DCAN_RAMINIT));
> >> +	if (enable) {
> >> +		val &= ~AM33XX_DCAN_RAMINIT_START_MASK(instance);
> >> +		val |= AM33XX_DCAN_RAMINIT_START_MASK(instance);
> >> +		writel(val, AM33XX_CTRL_REGADDR(AM33XX_CONTROL_DCAN_RAMINIT));
> >> +	} else {
> >> +		val &= ~AM33XX_DCAN_RAMINIT_START_MASK(instance);
> >> +		writel(val, AM33XX_CTRL_REGADDR(AM33XX_CONTROL_DCAN_RAMINIT));
> >> +	}
> >> +}
> > 
> > This part does not look good to me, this is tweaking the omap control
> > module register bits directly. To me it seems that the above should
> > be implemented in the omap/am33xx hwmod code that gets initialized when
> > the dcan driver calls pm_runtime_enable()? Paul, got any other ideas?
> > 
> 
> Technically yes, this is required during module enable/disable sequence.
> But there is no way currently supported in hwmod layer. Also I am not
> quite sure how many other modules/devices may use this.
> 

It should be possible to do this by providing custom activate/deactivate
functions similar to what was done for EMAC on AM35x [1].

However, right now on DT based systems omap_device_alloc() is called without
any pm_lats

	od = omap_device_alloc(pdev, hwmods, oh_cnt, NULL, 0);

The function pointers from the of_dev_auxdata somehow needs to be passed to
omap_device_alloc().

Regards,
Vaibhav B.

> Couple of more examples I have here,
> 
> In case of AM3517 we have similar SoC integration, where VPFE, MAC and
> USB required clock control (handled by clock-tree) and interrupt status
> (handled by callbacks) from control module.
> So not sure whether we can get rid of callbacks until we have control
> module MFD driver (on which Konstantin is working on)
> 
> Thanks,
> Vaibhav
> 
> > Regards,
> > 
> > Tony
> > 
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel at lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> > 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

[PATCH RESEND 3/4] ARM: AM33XX: board-generic: Add of_dev_auxdata to pass d_can raminit

[Hiremath, Vaibhav]

On Thu, Sep 06, 2012 at 10:33:32, Bedia, Vaibhav wrote:
> On Thu, Sep 06, 2012 at 09:26:07, Hiremath, Vaibhav wrote:
> > 
> > 
> > On 9/6/2012 4:48 AM, Tony Lindgren wrote:
> > > Hi,
> > > 
> > > * AnilKumar Ch <anilkumar@ti.com> [120905 04:14]:
> > >> Add of_dev_auxdata to pass d_can raminit callback APIs to initialize
> > >> d_can RAM. D_CAN RAM initialization bits are present in CONTROL module
> > >> address space, which can be accessed by platform specific code. So
> > >> callback functions are added to serve this purpose, this can done by
> > >> using of_dev_auxdata.
> > >>
> > >> Two callback APIs are added to of_dev_auxdata used by two instances of
> > >> D_CAN IP. These callback functions are used to enable/disable D_CAN RAM
> > >> from CAN driver.
> > > 
> > > I'd like to avoid the callbacks to the platform code where possible as
> > > that's the biggest pain we already have moving things to work with device
> > > tree for the existing drivers.
> > > 
> > > And I'm pretty convinced that whatever is done with callbacks should be
> > > done with some Linux generic framework from the driver that has it's own
> > > binding, such as clock framework, regulator framework, pinctrl framework,
> > > runtime PM etc.
> > > 
> > >> --- a/arch/arm/mach-omap2/board-generic.c
> > >> +++ b/arch/arm/mach-omap2/board-generic.c
> > >> @@ -37,11 +40,46 @@ static struct of_device_id omap_dt_match_table[] __initdata = {
> > >>  	{ }
> > >>  };
> > >>  
> > >> +void d_can_hw_raminit(unsigned int instance, bool enable)
> > >> +{
> > >> +	u32 val;
> > >> +
> > >> +	val = readl(AM33XX_CTRL_REGADDR(AM33XX_CONTROL_DCAN_RAMINIT));
> > >> +	if (enable) {
> > >> +		val &= ~AM33XX_DCAN_RAMINIT_START_MASK(instance);
> > >> +		val |= AM33XX_DCAN_RAMINIT_START_MASK(instance);
> > >> +		writel(val, AM33XX_CTRL_REGADDR(AM33XX_CONTROL_DCAN_RAMINIT));
> > >> +	} else {
> > >> +		val &= ~AM33XX_DCAN_RAMINIT_START_MASK(instance);
> > >> +		writel(val, AM33XX_CTRL_REGADDR(AM33XX_CONTROL_DCAN_RAMINIT));
> > >> +	}
> > >> +}
> > > 
> > > This part does not look good to me, this is tweaking the omap control
> > > module register bits directly. To me it seems that the above should
> > > be implemented in the omap/am33xx hwmod code that gets initialized when
> > > the dcan driver calls pm_runtime_enable()? Paul, got any other ideas?
> > > 
> > 
> > Technically yes, this is required during module enable/disable sequence.
> > But there is no way currently supported in hwmod layer. Also I am not
> > quite sure how many other modules/devices may use this.
> > 
> 
> It should be possible to do this by providing custom activate/deactivate
> functions similar to what was done for EMAC on AM35x [1].
> 

Still didn't make it to mainline :)

> However, right now on DT based systems omap_device_alloc() is called without
> any pm_lats
> 
> 	od = omap_device_alloc(pdev, hwmods, oh_cnt, NULL, 0);
> 
> The function pointers from the of_dev_auxdata somehow needs to be passed to
> omap_device_alloc().
> 

It is doable, and I believe it is the only option we have currently unless 
we add something newly.


Thanks,
Vaibhav
> Regards,
> Vaibhav B.
> 
> > Couple of more examples I have here,
> > 
> > In case of AM3517 we have similar SoC integration, where VPFE, MAC and
> > USB required clock control (handled by clock-tree) and interrupt status
> > (handled by callbacks) from control module.
> > So not sure whether we can get rid of callbacks until we have control
> > module MFD driver (on which Konstantin is working on)
> > 
> > Thanks,
> > Vaibhav
> > 
> > > Regards,
> > > 
> > > Tony
> > > 
> > > _______________________________________________
> > > linux-arm-kernel mailing list
> > > linux-arm-kernel at lists.infradead.org
> > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> > > 
> > 
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel at lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> > 
> 
> 

[PATCH v2 0/4] can: c_can: Add suspend/resume and pinctrl support

[AnilKumar, Chimata]

On Thu, Sep 06, 2012 at 04:56:10, Tony Lindgren wrote:
> * Marc Kleine-Budde <mkl@pengutronix.de> [120905 06:55]:
> > On 09/05/2012 01:54 PM, Marc Kleine-Budde wrote:
> > > On 09/05/2012 01:12 PM, AnilKumar Ch wrote:
> > >> Adds suspend/resume functionality of d_can driver along with d_can
> > >> raminit support which is required to initialize RAM while data
> > >> transmission.
> > >>
> > >> This patch series also adds pinctrl support to c_can driver. These
> > >> patches were tested on AM335x-EVM.
> > >>
> > >> This patch-series is based ontop of runtimePM patch submitted
> > >> earlier - https://patchwork.kernel.org/patch/1348081/
> > > 
> > > Looks good, I'll apply the series after Tony acks the patch 3
> > > ([PATCH RESEND 3/4] ARM: AM33XX: board-generic: Add of_dev_auxdata to
> > > pass d_can raminit).
> > 
> > I've pushed all patches to can-next/master [1]. When Tony gives his Ack,
> > I'll send David a pull request (of a tree including that Ack).
> 
> To me it looks like we can avoid the platform callbacks and the ram
> init changes to the driver by implementing right runtime PM calls
> for omap/am33xx. So NAK on few patches, and ACK on one :)
> 

Marc,

Can you please queue ACK patch for 3.7, I will submit v3 for rest of
the patches, once we found the clear way of doing RAMINIT.

Thanks
AnilKumar

[PATCH v2 0/4] can: c_can: Add suspend/resume and pinctrl support

[Marc Kleine-Budde]

On 09/06/2012 08:09 AM, AnilKumar, Chimata wrote:
> On Thu, Sep 06, 2012 at 04:56:10, Tony Lindgren wrote:
>> * Marc Kleine-Budde <mkl@pengutronix.de> [120905 06:55]:
>>> On 09/05/2012 01:54 PM, Marc Kleine-Budde wrote:
>>>> On 09/05/2012 01:12 PM, AnilKumar Ch wrote:
>>>>> Adds suspend/resume functionality of d_can driver along with d_can
>>>>> raminit support which is required to initialize RAM while data
>>>>> transmission.
>>>>>
>>>>> This patch series also adds pinctrl support to c_can driver. These
>>>>> patches were tested on AM335x-EVM.
>>>>>
>>>>> This patch-series is based ontop of runtimePM patch submitted
>>>>> earlier - https://patchwork.kernel.org/patch/1348081/
>>>>
>>>> Looks good, I'll apply the series after Tony acks the patch 3
>>>> ([PATCH RESEND 3/4] ARM: AM33XX: board-generic: Add of_dev_auxdata to
>>>> pass d_can raminit).
>>>
>>> I've pushed all patches to can-next/master [1]. When Tony gives his Ack,
>>> I'll send David a pull request (of a tree including that Ack).
>>
>> To me it looks like we can avoid the platform callbacks and the ram
>> init changes to the driver by implementing right runtime PM calls
>> for omap/am33xx. So NAK on few patches, and ACK on one :)
>>
> 
> Marc,
> 
> Can you please queue ACK patch for 3.7, I will submit v3 for rest of
> the patches, once we found the clear way of doing RAMINIT.

Done. I've updated the master branch on my repo[1]. I'll send a pull
request to David soon.

Marc

[1] https://gitorious.org/linux-can/linux-can-next
-- 
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/20120906/54e1b78d/attachment.sig>

[PATCH v2 4/4] can: c_can: Add d_can suspend resume support

[AnilKumar, Chimata]

On Wed, Sep 05, 2012 at 16:42:44, AnilKumar, Chimata wrote:
> Adds suspend resume support to DCAN driver which enables
> DCAN power down mode bit (PDR). Then DCAN will ack the local
> power-down mode by setting PDA bit in STATUS register.
> 

Marc,

I missed out this patch, I will remove raminit calls from this
patch and submit new version. But I will test using RAMINIT.

Thanks
AnilKumar

> Signed-off-by: AnilKumar Ch <anilkumar@ti.com>
> ---
>  drivers/net/can/c_can/c_can.c          |   82 ++++++++++++++++++++++++++++++++
>  drivers/net/can/c_can/c_can.h          |    8 ++++
>  drivers/net/can/c_can/c_can_platform.c |   62 ++++++++++++++++++++++++
>  3 files changed, 152 insertions(+)
> 
> diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
> index c175410..c601136 100644
> --- a/drivers/net/can/c_can/c_can.c
> +++ b/drivers/net/can/c_can/c_can.c
> @@ -46,6 +46,9 @@
>  #define IF_ENUM_REG_LEN		11
>  #define C_CAN_IFACE(reg, iface)	(C_CAN_IF1_##reg + (iface) * IF_ENUM_REG_LEN)
>  
> +/* control extension register D_CAN specific */
> +#define CONTROL_EX_PDR		BIT(8)
> +
>  /* control register */
>  #define CONTROL_TEST		BIT(7)
>  #define CONTROL_CCE		BIT(6)
> @@ -65,6 +68,7 @@
>  #define TEST_BASIC		BIT(2)
>  
>  /* status register */
> +#define STATUS_PDA		BIT(10)
>  #define STATUS_BOFF		BIT(7)
>  #define STATUS_EWARN		BIT(6)
>  #define STATUS_EPASS		BIT(5)
> @@ -164,6 +168,9 @@
>  /* minimum timeout for checking BUSY status */
>  #define MIN_TIMEOUT_VALUE	6
>  
> +/* Wait for ~1 sec for INIT bit */
> +#define INIT_WAIT_MS		1000
> +
>  /* napi related */
>  #define C_CAN_NAPI_WEIGHT	C_CAN_MSG_OBJ_RX_NUM
>  
> @@ -1154,6 +1161,81 @@ struct net_device *alloc_c_can_dev(void)
>  }
>  EXPORT_SYMBOL_GPL(alloc_c_can_dev);
>  
> +#ifdef CONFIG_PM
> +int c_can_power_down(struct net_device *dev)
> +{
> +	u32 val;
> +	unsigned long time_out;
> +	struct c_can_priv *priv = netdev_priv(dev);
> +
> +	if (!(dev->flags & IFF_UP))
> +		return 0;
> +
> +	BUG_ON(priv->type != BOSCH_D_CAN);
> +
> +	/* set PDR value so the device goes to power down mode */
> +	val = priv->read_reg(priv, C_CAN_CTRL_EX_REG);
> +	val |= CONTROL_EX_PDR;
> +	priv->write_reg(priv, C_CAN_CTRL_EX_REG, val);
> +
> +	/* Wait for the PDA bit to get set */
> +	time_out = jiffies + msecs_to_jiffies(INIT_WAIT_MS);
> +	while (!(priv->read_reg(priv, C_CAN_STS_REG) & STATUS_PDA) &&
> +				time_after(time_out, jiffies))
> +		cpu_relax();
> +
> +	if (time_after(jiffies, time_out))
> +		return -ETIMEDOUT;
> +
> +	c_can_stop(dev);
> +
> +	/* De-initialize DCAN RAM */
> +	c_can_reset_ram(priv, false);
> +	c_can_pm_runtime_put_sync(priv);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(c_can_power_down);
> +
> +int c_can_power_up(struct net_device *dev)
> +{
> +	u32 val;
> +	unsigned long time_out;
> +	struct c_can_priv *priv = netdev_priv(dev);
> +
> +	if (!(dev->flags & IFF_UP))
> +		return 0;
> +
> +	BUG_ON(priv->type != BOSCH_D_CAN);
> +
> +	c_can_pm_runtime_get_sync(priv);
> +	/* Initialize DCAN RAM */
> +	c_can_reset_ram(priv, true);
> +
> +	/* Clear PDR and INIT bits */
> +	val = priv->read_reg(priv, C_CAN_CTRL_EX_REG);
> +	val &= ~CONTROL_EX_PDR;
> +	priv->write_reg(priv, C_CAN_CTRL_EX_REG, val);
> +	val = priv->read_reg(priv, C_CAN_CTRL_REG);
> +	val &= ~CONTROL_INIT;
> +	priv->write_reg(priv, C_CAN_CTRL_REG, val);
> +
> +	/* Wait for the PDA bit to get clear */
> +	time_out = jiffies + msecs_to_jiffies(INIT_WAIT_MS);
> +	while ((priv->read_reg(priv, C_CAN_STS_REG) & STATUS_PDA) &&
> +				time_after(time_out, jiffies))
> +		cpu_relax();
> +
> +	if (time_after(jiffies, time_out))
> +		return -ETIMEDOUT;
> +
> +	c_can_start(dev);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(c_can_power_up);
> +#endif
> +
>  void free_c_can_dev(struct net_device *dev)
>  {
>  	free_candev(dev);
> diff --git a/drivers/net/can/c_can/c_can.h b/drivers/net/can/c_can/c_can.h
> index 5f6339c..ca149eb 100644
> --- a/drivers/net/can/c_can/c_can.h
> +++ b/drivers/net/can/c_can/c_can.h
> @@ -24,6 +24,7 @@
>  
>  enum reg {
>  	C_CAN_CTRL_REG = 0,
> +	C_CAN_CTRL_EX_REG,
>  	C_CAN_STS_REG,
>  	C_CAN_ERR_CNT_REG,
>  	C_CAN_BTR_REG,
> @@ -104,6 +105,7 @@ static const u16 reg_map_c_can[] = {
>  
>  static const u16 reg_map_d_can[] = {
>  	[C_CAN_CTRL_REG]	= 0x00,
> +	[C_CAN_CTRL_EX_REG]	= 0x02,
>  	[C_CAN_STS_REG]		= 0x04,
>  	[C_CAN_ERR_CNT_REG]	= 0x08,
>  	[C_CAN_BTR_REG]		= 0x0C,
> @@ -166,6 +168,7 @@ struct c_can_priv {
>  	unsigned int tx_echo;
>  	void *priv;		/* for board-specific data */
>  	u16 irqstatus;
> +	enum c_can_dev_id type;
>  	unsigned int instance;
>  	void (*ram_init) (unsigned int instance, bool enable);
>  };
> @@ -175,4 +178,9 @@ void free_c_can_dev(struct net_device *dev);
>  int register_c_can_dev(struct net_device *dev);
>  void unregister_c_can_dev(struct net_device *dev);
>  
> +#ifdef CONFIG_PM
> +int c_can_power_up(struct net_device *dev);
> +int c_can_power_down(struct net_device *dev);
> +#endif
> +
>  #endif /* C_CAN_H */
> diff --git a/drivers/net/can/c_can/c_can_platform.c b/drivers/net/can/c_can/c_can_platform.c
> index c6963b2..68c186b 100644
> --- a/drivers/net/can/c_can/c_can_platform.c
> +++ b/drivers/net/can/c_can/c_can_platform.c
> @@ -202,6 +202,7 @@ static int __devinit c_can_plat_probe(struct platform_device *pdev)
>  	priv->device = &pdev->dev;
>  	priv->can.clock.freq = clk_get_rate(clk);
>  	priv->priv = clk;
> +	priv->type = id->driver_data;
>  
>  	platform_set_drvdata(pdev, dev);
>  	SET_NETDEV_DEV(dev, &pdev->dev);
> @@ -255,6 +256,65 @@ static int __devexit c_can_plat_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> +#ifdef CONFIG_PM
> +static int c_can_suspend(struct platform_device *pdev, pm_message_t state)
> +{
> +	int ret;
> +	struct net_device *ndev = platform_get_drvdata(pdev);
> +	struct c_can_priv *priv = netdev_priv(ndev);
> +
> +	if (priv->type != BOSCH_D_CAN) {
> +		dev_warn(&pdev->dev, "Not supported\n");
> +		return 0;
> +	}
> +
> +	if (netif_running(ndev)) {
> +		netif_stop_queue(ndev);
> +		netif_device_detach(ndev);
> +	}
> +
> +	ret = c_can_power_down(ndev);
> +	if (ret) {
> +		netdev_err(ndev, "failed to enter power down mode\n");
> +		return ret;
> +	}
> +
> +	priv->can.state = CAN_STATE_SLEEPING;
> +
> +	return 0;
> +}
> +
> +static int c_can_resume(struct platform_device *pdev)
> +{
> +	int ret;
> +	struct net_device *ndev = platform_get_drvdata(pdev);
> +	struct c_can_priv *priv = netdev_priv(ndev);
> +
> +	if (priv->type != BOSCH_D_CAN) {
> +		dev_warn(&pdev->dev, "Not supported\n");
> +		return 0;
> +	}
> +
> +	ret = c_can_power_up(ndev);
> +	if (ret) {
> +		netdev_err(ndev, "Still in power down mode\n");
> +		return ret;
> +	}
> +
> +	priv->can.state = CAN_STATE_ERROR_ACTIVE;
> +
> +	if (netif_running(ndev)) {
> +		netif_device_attach(ndev);
> +		netif_start_queue(ndev);
> +	}
> +
> +	return 0;
> +}
> +#else
> +#define c_can_suspend NULL
> +#define c_can_resume NULL
> +#endif
> +
>  static struct platform_driver c_can_plat_driver = {
>  	.driver = {
>  		.name = KBUILD_MODNAME,
> @@ -263,6 +323,8 @@ static struct platform_driver c_can_plat_driver = {
>  	},
>  	.probe = c_can_plat_probe,
>  	.remove = __devexit_p(c_can_plat_remove),
> +	.suspend = c_can_suspend,
> +	.resume = c_can_resume,
>  	.id_table = c_can_id_table,
>  };
>  
> -- 
> 1.7.9.5
> 
>