[PATCH v8 0/8] can: c_can: Use syscon regmap for TI specific RAMINIT register

[PATCH v8 0/8] can: c_can: Use syscon regmap for TI specific RAMINIT register

[Marc Kleine-Budde]

Picking up Roger's work:

Changes since v7:
- combined raminit start and done bit into a struct (3/8, 7/8, 8/8)
- constified struct c_can_driver_data (2/8, 7/8, 8/8)
- silenced compiler warning in c_can_hw_raminit_{wait,}_syscon (4/8)
- use platform_get_device_id in probe() instead of open coding it (2/8)

Marc

[PATCH v8 1/8] can: c_can: Add timeout to c_can_hw_raminit_ti()

[Marc Kleine-Budde]

From: Roger Quadros <rogerq@ti.com>

TI's RAMINIT DONE mechanism is buggy on AM43xx SoC and may not always
be set after the START bit is set. Although it seems to work fine even
in that case. So add a timeout mechanism to c_can_hw_raminit_wait_ti().
Don't bail out in that failure case but just print an error message.

Signed-off-by: Roger Quadros <rogerq@ti.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/c_can/c_can_platform.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/net/can/c_can/c_can_platform.c b/drivers/net/can/c_can/c_can_platform.c
index fb279d6ae484..106c203fc5eb 100644
--- a/drivers/net/can/c_can/c_can_platform.c
+++ b/drivers/net/can/c_can/c_can_platform.c
@@ -75,10 +75,19 @@ static void c_can_plat_write_reg_aligned_to_32bit(const struct c_can_priv *priv,
 static void c_can_hw_raminit_wait_ti(const struct c_can_priv *priv, u32 mask,
 				  u32 val)
 {
+	int timeout = 0;
+
 	/* We look only at the bits of our instance. */
 	val &= mask;
-	while ((readl(priv->raminit_ctrlreg) & mask) != val)
+	while ((readl(priv->raminit_ctrlreg) & mask) != val) {
 		udelay(1);
+		timeout++;
+
+		if (timeout == 1000) {
+			dev_err(&priv->dev->dev, "%s: time out\n", __func__);
+			break;
+		}
+	}
 }
 
 static void c_can_hw_raminit_ti(const struct c_can_priv *priv, bool enable)
-- 
2.1.1

[PATCH v8 2/8] can: c_can: Introduce c_can_driver_data structure

[Marc Kleine-Budde]

From: Roger Quadros <rogerq@ti.com>

We want to have more data than just can_dev_id to be present
in the driver data e.g. TI platforms need RAMINIT register
description. Introduce the c_can_driver_data structure and move
the can_dev_id into it.

Tidy up the way it is used on probe().

Signed-off-by: Roger Quadros <rogerq@ti.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/c_can/c_can.h          |  4 +++
 drivers/net/can/c_can/c_can_platform.c | 52 +++++++++++++++++++---------------
 2 files changed, 33 insertions(+), 23 deletions(-)

diff --git a/drivers/net/can/c_can/c_can.h b/drivers/net/can/c_can/c_can.h
index 99ad1aa576b0..26c975d914e3 100644
--- a/drivers/net/can/c_can/c_can.h
+++ b/drivers/net/can/c_can/c_can.h
@@ -169,6 +169,10 @@ enum c_can_dev_id {
 	BOSCH_D_CAN,
 };
 
+struct c_can_driver_data {
+	enum c_can_dev_id id;
+};
+
 /* c_can private data structure */
 struct c_can_priv {
 	struct can_priv can;	/* must be the first member */
diff --git a/drivers/net/can/c_can/c_can_platform.c b/drivers/net/can/c_can/c_can_platform.c
index 106c203fc5eb..44c293926f78 100644
--- a/drivers/net/can/c_can/c_can_platform.c
+++ b/drivers/net/can/c_can/c_can_platform.c
@@ -168,26 +168,34 @@ static void c_can_hw_raminit(const struct c_can_priv *priv, bool enable)
 	}
 }
 
+static const struct c_can_driver_data c_can_drvdata = {
+	.id = BOSCH_C_CAN,
+};
+
+static const struct c_can_driver_data d_can_drvdata = {
+	.id = BOSCH_D_CAN,
+};
+
 static struct platform_device_id c_can_id_table[] = {
-	[BOSCH_C_CAN_PLATFORM] = {
+	{
 		.name = KBUILD_MODNAME,
-		.driver_data = BOSCH_C_CAN,
+		.driver_data = (kernel_ulong_t)&c_can_drvdata,
 	},
-	[BOSCH_C_CAN] = {
+	{
 		.name = "c_can",
-		.driver_data = BOSCH_C_CAN,
+		.driver_data = (kernel_ulong_t)&c_can_drvdata,
 	},
-	[BOSCH_D_CAN] = {
+	{
 		.name = "d_can",
-		.driver_data = BOSCH_D_CAN,
-	}, {
-	}
+		.driver_data = (kernel_ulong_t)&d_can_drvdata,
+	},
+	{ /* sentinel */ },
 };
 MODULE_DEVICE_TABLE(platform, c_can_id_table);
 
 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] },
+	{ .compatible = "bosch,c_can", .data = &c_can_drvdata },
+	{ .compatible = "bosch,d_can", .data = &d_can_drvdata },
 	{ /* sentinel */ },
 };
 MODULE_DEVICE_TABLE(of, c_can_of_table);
@@ -199,21 +207,19 @@ static int c_can_plat_probe(struct platform_device *pdev)
 	struct net_device *dev;
 	struct c_can_priv *priv;
 	const struct of_device_id *match;
-	const struct platform_device_id *id;
 	struct resource *mem, *res;
 	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;
+	const struct c_can_driver_data *drvdata;
+
+	match = of_match_device(c_can_of_table, &pdev->dev);
+	if (match) {
+		drvdata = match->data;
+	} else if (pdev->id_entry->driver_data) {
+		drvdata = (struct c_can_driver_data *)
+			platform_get_device_id(pdev)->driver_data;
 	} else {
-		id = platform_get_device_id(pdev);
+		return -ENODEV;
 	}
 
 	/* get the appropriate clk */
@@ -245,7 +251,7 @@ static int c_can_plat_probe(struct platform_device *pdev)
 	}
 
 	priv = netdev_priv(dev);
-	switch (id->driver_data) {
+	switch (drvdata->id) {
 	case BOSCH_C_CAN:
 		priv->regs = reg_map_c_can;
 		switch (mem->flags & IORESOURCE_MEM_TYPE_MASK) {
@@ -304,7 +310,7 @@ static int 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;
+	priv->type = drvdata->id;
 
 	platform_set_drvdata(pdev, dev);
 	SET_NETDEV_DEV(dev, &pdev->dev);
-- 
2.1.1

[PATCH v8 3/8] can: c_can: Add RAMINIT register information to driver data

[Marc Kleine-Budde]

From: Roger Quadros <rogerq@ti.com>

Some platforms (e.g. TI) need special RAMINIT register handling.
Provide a way to store RAMINIT register description in driver data.

Signed-off-by: Roger Quadros <rogerq@ti.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/c_can/c_can.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/net/can/c_can/c_can.h b/drivers/net/can/c_can/c_can.h
index 26c975d914e3..3f111f4f0f6e 100644
--- a/drivers/net/can/c_can/c_can.h
+++ b/drivers/net/can/c_can/c_can.h
@@ -169,8 +169,18 @@ enum c_can_dev_id {
 	BOSCH_D_CAN,
 };
 
+struct raminit_bits {
+	u8 start;
+	u8 done;
+};
+
 struct c_can_driver_data {
 	enum c_can_dev_id id;
+
+	/* RAMINIT register description. Optional. */
+	const struct raminit_bits *raminit_bits; /* Array of START/DONE bit positions */
+	u8 raminit_num;		/* Number of CAN instances on the SoC */
+	bool raminit_pulse;	/* If set, sets and clears START bit (pulse) */
 };
 
 /* c_can private data structure */
-- 
2.1.1

[PATCH v8 4/8] can: c_can: Add syscon/regmap RAMINIT mechanism

[Marc Kleine-Budde]

From: Roger Quadros <rogerq@ti.com>

Some TI SoCs like DRA7 have a RAMINIT register specification
different from the other AMxx SoCs and as expected by the
existing driver.

To add more insanity, this register is shared with other
IPs like DSS, PCIe and PWM.

Provides a more generic mechanism to specify the RAMINIT
register location and START/DONE bit position and use the
syscon/regmap framework to access the register.

Signed-off-by: Roger Quadros <rogerq@ti.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 .../devicetree/bindings/net/can/c_can.txt          |   3 +
 drivers/net/can/c_can/c_can.h                      |  10 +-
 drivers/net/can/c_can/c_can_platform.c             | 109 +++++++++++++--------
 3 files changed, 81 insertions(+), 41 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/can/c_can.txt b/Documentation/devicetree/bindings/net/can/c_can.txt
index 8f1ae81228e3..a3ca3ee53546 100644
--- a/Documentation/devicetree/bindings/net/can/c_can.txt
+++ b/Documentation/devicetree/bindings/net/can/c_can.txt
@@ -12,6 +12,9 @@ Required properties:
 Optional properties:
 - ti,hwmods		: Must be "d_can<n>" or "c_can<n>", n being the
 			  instance number
+- syscon-raminit	: Handle to system control region that contains the
+			  RAMINIT register, register offset to the RAMINIT
+			  register and the CAN instance number (0 offset).
 
 Note: "ti,hwmods" field is used to fetch the base address and irq
 resources from TI, omap hwmod data base during device registration.
diff --git a/drivers/net/can/c_can/c_can.h b/drivers/net/can/c_can/c_can.h
index 3f111f4f0f6e..28a73d14ea8d 100644
--- a/drivers/net/can/c_can/c_can.h
+++ b/drivers/net/can/c_can/c_can.h
@@ -183,6 +183,13 @@ struct c_can_driver_data {
 	bool raminit_pulse;	/* If set, sets and clears START bit (pulse) */
 };
 
+/* Out of band RAMINIT register access via syscon regmap */
+struct c_can_raminit {
+	struct regmap *syscon;	/* for raminit ctrl. reg. access */
+	unsigned int reg;	/* register index within syscon */
+	struct raminit_bits bits;
+};
+
 /* c_can private data structure */
 struct c_can_priv {
 	struct can_priv can;	/* must be the first member */
@@ -200,8 +207,7 @@ struct c_can_priv {
 	const u16 *regs;
 	void *priv;		/* for board-specific data */
 	enum c_can_dev_id type;
-	u32 __iomem *raminit_ctrlreg;
-	int instance;
+	struct c_can_raminit raminit_sys;	/* RAMINIT via syscon regmap */
 	void (*raminit) (const struct c_can_priv *priv, bool enable);
 	u32 comm_rcv_high;
 	u32 rxmasked;
diff --git a/drivers/net/can/c_can/c_can_platform.c b/drivers/net/can/c_can/c_can_platform.c
index 44c293926f78..1fbfa1d59c29 100644
--- a/drivers/net/can/c_can/c_can_platform.c
+++ b/drivers/net/can/c_can/c_can_platform.c
@@ -32,14 +32,13 @@
 #include <linux/clk.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
+#include <linux/mfd/syscon.h>
+#include <linux/regmap.h>
 
 #include <linux/can/dev.h>
 
 #include "c_can.h"
 
-#define CAN_RAMINIT_START_MASK(i)	(0x001 << (i))
-#define CAN_RAMINIT_DONE_MASK(i)	(0x100 << (i))
-#define CAN_RAMINIT_ALL_MASK(i)		(0x101 << (i))
 #define DCAN_RAM_INIT_BIT		(1 << 3)
 static DEFINE_SPINLOCK(raminit_lock);
 /*
@@ -72,48 +71,57 @@ static void c_can_plat_write_reg_aligned_to_32bit(const struct c_can_priv *priv,
 	writew(val, priv->base + 2 * priv->regs[index]);
 }
 
-static void c_can_hw_raminit_wait_ti(const struct c_can_priv *priv, u32 mask,
-				  u32 val)
+static void c_can_hw_raminit_wait_syscon(const struct c_can_priv *priv,
+					 u32 mask, u32 val)
 {
+	const struct c_can_raminit *raminit = &priv->raminit_sys;
 	int timeout = 0;
+	u32 ctrl = 0;
 
 	/* We look only at the bits of our instance. */
 	val &= mask;
-	while ((readl(priv->raminit_ctrlreg) & mask) != val) {
+	do {
 		udelay(1);
 		timeout++;
 
+		regmap_read(raminit->syscon, raminit->reg, &ctrl);
 		if (timeout == 1000) {
 			dev_err(&priv->dev->dev, "%s: time out\n", __func__);
 			break;
 		}
-	}
+	} while ((ctrl & mask) != val);
 }
 
-static void c_can_hw_raminit_ti(const struct c_can_priv *priv, bool enable)
+static void c_can_hw_raminit_syscon(const struct c_can_priv *priv, bool enable)
 {
-	u32 mask = CAN_RAMINIT_ALL_MASK(priv->instance);
-	u32 ctrl;
+	const struct c_can_raminit *raminit = &priv->raminit_sys;
+	u32 ctrl = 0;
+	u32 mask;
 
 	spin_lock(&raminit_lock);
 
-	ctrl = readl(priv->raminit_ctrlreg);
+	mask = 1 << raminit->bits.start | 1 << raminit->bits.done;
+	regmap_read(raminit->syscon, raminit->reg, &ctrl);
+
 	/* We clear the done and start bit first. The start bit is
 	 * looking at the 0 -> transition, but is not self clearing;
 	 * And we clear the init done bit as well.
+	 * NOTE: DONE must be written with 1 to clear it.
 	 */
-	ctrl &= ~CAN_RAMINIT_START_MASK(priv->instance);
-	ctrl |= CAN_RAMINIT_DONE_MASK(priv->instance);
-	writel(ctrl, priv->raminit_ctrlreg);
-	ctrl &= ~CAN_RAMINIT_DONE_MASK(priv->instance);
-	c_can_hw_raminit_wait_ti(priv, mask, ctrl);
+	ctrl &= ~(1 << raminit->bits.start);
+	ctrl |= 1 << raminit->bits.done;
+	regmap_write(raminit->syscon, raminit->reg, ctrl);
+
+	ctrl &= ~(1 << raminit->bits.done);
+	c_can_hw_raminit_wait_syscon(priv, mask, ctrl);
 
 	if (enable) {
 		/* Set start bit and wait for the done bit. */
-		ctrl |= CAN_RAMINIT_START_MASK(priv->instance);
-		writel(ctrl, priv->raminit_ctrlreg);
-		ctrl |= CAN_RAMINIT_DONE_MASK(priv->instance);
-		c_can_hw_raminit_wait_ti(priv, mask, ctrl);
+		ctrl |= 1 << raminit->bits.start;
+		regmap_write(raminit->syscon, raminit->reg, ctrl);
+
+		ctrl |= 1 << raminit->bits.done;
+		c_can_hw_raminit_wait_syscon(priv, mask, ctrl);
 	}
 	spin_unlock(&raminit_lock);
 }
@@ -207,10 +215,11 @@ static int c_can_plat_probe(struct platform_device *pdev)
 	struct net_device *dev;
 	struct c_can_priv *priv;
 	const struct of_device_id *match;
-	struct resource *mem, *res;
+	struct resource *mem;
 	int irq;
 	struct clk *clk;
 	const struct c_can_driver_data *drvdata;
+	struct device_node *np = pdev->dev.of_node;
 
 	match = of_match_device(c_can_of_table, &pdev->dev);
 	if (match) {
@@ -278,27 +287,49 @@ static int c_can_plat_probe(struct platform_device *pdev)
 		priv->read_reg32 = d_can_plat_read_reg32;
 		priv->write_reg32 = d_can_plat_write_reg32;
 
-		if (pdev->dev.of_node)
-			priv->instance = of_alias_get_id(pdev->dev.of_node, "d_can");
-		else
-			priv->instance = pdev->id;
-
-		res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
-		/* Not all D_CAN modules have a separate register for the D_CAN
-		 * RAM initialization. Use default RAM init bit in D_CAN module
-		 * if not specified in DT.
+		/* Check if we need custom RAMINIT via syscon. Mostly for TI
+		 * platforms. Only supported with DT boot.
 		 */
-		if (!res) {
+		if (np && of_property_read_bool(np, "syscon-raminit")) {
+			u32 id;
+			struct c_can_raminit *raminit = &priv->raminit_sys;
+
+			ret = -EINVAL;
+			raminit->syscon = syscon_regmap_lookup_by_phandle(np,
+									  "syscon-raminit");
+			if (IS_ERR(raminit->syscon)) {
+				/* can fail with -EPROBE_DEFER */
+				ret = PTR_ERR(raminit->syscon);
+				free_c_can_dev(dev);
+				return ret;
+			}
+
+			if (of_property_read_u32_index(np, "syscon-raminit", 1,
+						       &raminit->reg)) {
+				dev_err(&pdev->dev,
+					"couldn't get the RAMINIT reg. offset!\n");
+				goto exit_free_device;
+			}
+
+			if (of_property_read_u32_index(np, "syscon-raminit", 2,
+						       &id)) {
+				dev_err(&pdev->dev,
+					"couldn't get the CAN instance ID\n");
+				goto exit_free_device;
+			}
+
+			if (id >= drvdata->raminit_num) {
+				dev_err(&pdev->dev,
+					"Invalid CAN instance ID\n");
+				goto exit_free_device;
+			}
+
+			raminit->bits = drvdata->raminit_bits[id];
+
+			priv->raminit = c_can_hw_raminit_syscon;
+		} else {
 			priv->raminit = c_can_hw_raminit;
-			break;
 		}
-
-		priv->raminit_ctrlreg = devm_ioremap(&pdev->dev, res->start,
-						     resource_size(res));
-		if (!priv->raminit_ctrlreg || priv->instance < 0)
-			dev_info(&pdev->dev, "control memory is not used for raminit\n");
-		else
-			priv->raminit = c_can_hw_raminit_ti;
 		break;
 	default:
 		ret = -EINVAL;
-- 
2.1.1

[PATCH v8 5/8] can: c_can: Add support for START pulse in RAMINIT sequence

[Marc Kleine-Budde]

From: Roger Quadros <rogerq@ti.com>

Some SoCs e.g. (TI DRA7xx) need a START pulse to start the
RAMINIT sequence i.e. START bit must be set and cleared before
checking for the DONE bit status.

Signed-off-by: Roger Quadros <rogerq@ti.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/c_can/c_can.h          | 1 +
 drivers/net/can/c_can/c_can_platform.c | 7 +++++++
 2 files changed, 8 insertions(+)

diff --git a/drivers/net/can/c_can/c_can.h b/drivers/net/can/c_can/c_can.h
index 28a73d14ea8d..8acdc7fa4792 100644
--- a/drivers/net/can/c_can/c_can.h
+++ b/drivers/net/can/c_can/c_can.h
@@ -188,6 +188,7 @@ struct c_can_raminit {
 	struct regmap *syscon;	/* for raminit ctrl. reg. access */
 	unsigned int reg;	/* register index within syscon */
 	struct raminit_bits bits;
+	bool needs_pulse;
 };
 
 /* c_can private data structure */
diff --git a/drivers/net/can/c_can/c_can_platform.c b/drivers/net/can/c_can/c_can_platform.c
index 1fbfa1d59c29..41fa460c3592 100644
--- a/drivers/net/can/c_can/c_can_platform.c
+++ b/drivers/net/can/c_can/c_can_platform.c
@@ -120,6 +120,12 @@ static void c_can_hw_raminit_syscon(const struct c_can_priv *priv, bool enable)
 		ctrl |= 1 << raminit->bits.start;
 		regmap_write(raminit->syscon, raminit->reg, ctrl);
 
+		/* clear START bit if start pulse is needed */
+		if (raminit->needs_pulse) {
+			ctrl &= ~(1 << raminit->bits.start);
+			regmap_write(raminit->syscon, raminit->reg, ctrl);
+		}
+
 		ctrl |= 1 << raminit->bits.done;
 		c_can_hw_raminit_wait_syscon(priv, mask, ctrl);
 	}
@@ -325,6 +331,7 @@ static int c_can_plat_probe(struct platform_device *pdev)
 			}
 
 			raminit->bits = drvdata->raminit_bits[id];
+			raminit->needs_pulse = drvdata->raminit_pulse;
 
 			priv->raminit = c_can_hw_raminit_syscon;
 		} else {
-- 
2.1.1

[PATCH v8 6/8] can: c_can: Disable pins when CAN interface is down

[Marc Kleine-Budde]

From: Roger Quadros <rogerq@ti.com>

DRA7 CAN IP suffers from a problem which causes it to be prevented
from fully turning OFF (i.e. stuck in transition) if the module was
disabled while there was traffic on the CAN_RX line.

To work around this issue we select the SLEEP pin state by default
on probe and use the DEFAULT pin state on CAN up and back to the
SLEEP pin state on CAN down.

Signed-off-by: Roger Quadros <rogerq@ti.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/c_can/c_can.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
index 8e78bb48f5a4..f94a9fa60488 100644
--- a/drivers/net/can/c_can/c_can.c
+++ b/drivers/net/can/c_can/c_can.c
@@ -35,6 +35,7 @@
 #include <linux/list.h>
 #include <linux/io.h>
 #include <linux/pm_runtime.h>
+#include <linux/pinctrl/consumer.h>
 
 #include <linux/can.h>
 #include <linux/can/dev.h>
@@ -603,6 +604,8 @@ static int c_can_start(struct net_device *dev)
 
 	priv->can.state = CAN_STATE_ERROR_ACTIVE;
 
+	/* activate pins */
+	pinctrl_pm_select_default_state(dev->dev.parent);
 	return 0;
 }
 
@@ -611,6 +614,9 @@ static void c_can_stop(struct net_device *dev)
 	struct c_can_priv *priv = netdev_priv(dev);
 
 	c_can_irq_control(priv, false);
+
+	/* deactivate pins */
+	pinctrl_pm_select_sleep_state(dev->dev.parent);
 	priv->can.state = CAN_STATE_STOPPED;
 }
 
@@ -1244,6 +1250,13 @@ int register_c_can_dev(struct net_device *dev)
 	struct c_can_priv *priv = netdev_priv(dev);
 	int err;
 
+	/* Deactivate pins to prevent DRA7 DCAN IP from being
+	 * stuck in transition when module is disabled.
+	 * Pins are activated in c_can_start() and deactivated
+	 * in c_can_stop()
+	 */
+	pinctrl_pm_select_sleep_state(dev->dev.parent);
+
 	c_can_pm_runtime_enable(priv);
 
 	dev->flags |= IFF_ECHO;	/* we support local echo */
-- 
2.1.1

[PATCH v8 7/8] can: c_can: Add support for TI DRA7 DCAN

[Marc Kleine-Budde]

From: Roger Quadros <rogerq@ti.com>

DRA7 SoC has 2 CAN IPs. Provide compatible IDs and RAMINIT
register data for both.

Signed-off-by: Roger Quadros <rogerq@ti.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 Documentation/devicetree/bindings/net/can/c_can.txt |  1 +
 drivers/net/can/c_can/c_can_platform.c              | 13 +++++++++++++
 2 files changed, 14 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/can/c_can.txt b/Documentation/devicetree/bindings/net/can/c_can.txt
index a3ca3ee53546..f682fdb6d921 100644
--- a/Documentation/devicetree/bindings/net/can/c_can.txt
+++ b/Documentation/devicetree/bindings/net/can/c_can.txt
@@ -4,6 +4,7 @@ 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.
+			  Can be "ti,dra7-d_can".
 - reg			: physical base address and size of the C_CAN/D_CAN
 			  registers map
 - interrupts		: property with a value describing the interrupt
diff --git a/drivers/net/can/c_can/c_can_platform.c b/drivers/net/can/c_can/c_can_platform.c
index 41fa460c3592..570da5f56aae 100644
--- a/drivers/net/can/c_can/c_can_platform.c
+++ b/drivers/net/can/c_can/c_can_platform.c
@@ -190,6 +190,18 @@ static const struct c_can_driver_data d_can_drvdata = {
 	.id = BOSCH_D_CAN,
 };
 
+static const struct raminit_bits dra7_raminit_bits[] = {
+	[0] = { .start = 3, .done = 1, },
+	[1] = { .start = 5, .done = 2, },
+};
+
+static const struct c_can_driver_data dra7_dcan_drvdata = {
+	.id = BOSCH_D_CAN,
+	.raminit_num = ARRAY_SIZE(dra7_raminit_bits),
+	.raminit_bits = dra7_raminit_bits,
+	.raminit_pulse = true,
+};
+
 static struct platform_device_id c_can_id_table[] = {
 	{
 		.name = KBUILD_MODNAME,
@@ -210,6 +222,7 @@ MODULE_DEVICE_TABLE(platform, c_can_id_table);
 static const struct of_device_id c_can_of_table[] = {
 	{ .compatible = "bosch,c_can", .data = &c_can_drvdata },
 	{ .compatible = "bosch,d_can", .data = &d_can_drvdata },
+	{ .compatible = "ti,dra7-d_can", .data = &dra7_dcan_drvdata },
 	{ /* sentinel */ },
 };
 MODULE_DEVICE_TABLE(of, c_can_of_table);
-- 
2.1.1

[PATCH v8 8/8] can: c_can: Add support for TI am3352 DCAN

[Marc Kleine-Budde]

From: Roger Quadros <rogerq@ti.com>

AM3352 SoC has 2 DCAN modules. Add compatible id and
raminit driver data for am3352 DCAN.

Signed-off-by: Roger Quadros <rogerq@ti.com>
Acked-by: Wolfram Sang <wsa@the-dreams.de>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 Documentation/devicetree/bindings/net/can/c_can.txt |  2 +-
 drivers/net/can/c_can/c_can_platform.c              | 12 ++++++++++++
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/can/c_can.txt b/Documentation/devicetree/bindings/net/can/c_can.txt
index f682fdb6d921..6731730eec0d 100644
--- a/Documentation/devicetree/bindings/net/can/c_can.txt
+++ b/Documentation/devicetree/bindings/net/can/c_can.txt
@@ -4,7 +4,7 @@ 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.
-			  Can be "ti,dra7-d_can".
+			  Can be "ti,dra7-d_can" or "ti,am3352-d_can".
 - reg			: physical base address and size of the C_CAN/D_CAN
 			  registers map
 - interrupts		: property with a value describing the interrupt
diff --git a/drivers/net/can/c_can/c_can_platform.c b/drivers/net/can/c_can/c_can_platform.c
index 570da5f56aae..f4488e5d5d68 100644
--- a/drivers/net/can/c_can/c_can_platform.c
+++ b/drivers/net/can/c_can/c_can_platform.c
@@ -202,6 +202,17 @@ static const struct c_can_driver_data dra7_dcan_drvdata = {
 	.raminit_pulse = true,
 };
 
+static const struct raminit_bits am3352_raminit_bits[] = {
+	[0] = { .start = 0, .done = 8, },
+	[1] = { .start = 1, .done = 9, },
+};
+
+static const struct c_can_driver_data am3352_dcan_drvdata = {
+	.id = BOSCH_D_CAN,
+	.raminit_num = ARRAY_SIZE(am3352_raminit_bits),
+	.raminit_bits = am3352_raminit_bits,
+};
+
 static struct platform_device_id c_can_id_table[] = {
 	{
 		.name = KBUILD_MODNAME,
@@ -223,6 +234,7 @@ static const struct of_device_id c_can_of_table[] = {
 	{ .compatible = "bosch,c_can", .data = &c_can_drvdata },
 	{ .compatible = "bosch,d_can", .data = &d_can_drvdata },
 	{ .compatible = "ti,dra7-d_can", .data = &dra7_dcan_drvdata },
+	{ .compatible = "ti,am3352-d_can", .data = &am3352_dcan_drvdata },
 	{ /* sentinel */ },
 };
 MODULE_DEVICE_TABLE(of, c_can_of_table);
-- 
2.1.1

Re: [PATCH v8 0/8] can: c_can: Use syscon regmap for TI specific RAMINIT register

[Roger Quadros]

On 11/14/2014 08:09 PM, Marc Kleine-Budde wrote:
> Picking up Roger's work:
> 
> Changes since v7:
> - combined raminit start and done bit into a struct (3/8, 7/8, 8/8)
> - constified struct c_can_driver_data (2/8, 7/8, 8/8)
> - silenced compiler warning in c_can_hw_raminit_{wait,}_syscon (4/8)
> - use platform_get_device_id in probe() instead of open coding it (2/8)
> 

Thanks for saving me one more iteration :).
I've tested this to work on AM437x-evm and DRA7-evm.

Please pick patch 9 as well which I will send in response to this thread.
That one adds compatible id for am4372-d_can.

cheers,
-roger

[PATCH v8 9/9] net: can: c_can: Add support for TI am4372 DCAN

[Roger Quadros]

AM4372 SoC has 2 DCAN modules. Add compatible id and
raminit driver data for it. The driver data is same as AM3352
but this gives us flexibility to add AM4372 specific quirks
if required later.

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 Documentation/devicetree/bindings/net/can/c_can.txt | 3 ++-
 drivers/net/can/c_can/c_can_platform.c              | 1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/can/c_can.txt b/Documentation/devicetree/bindings/net/can/c_can.txt
index 6731730..5a1d8b0 100644
--- a/Documentation/devicetree/bindings/net/can/c_can.txt
+++ b/Documentation/devicetree/bindings/net/can/c_can.txt
@@ -4,7 +4,8 @@ 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.
-			  Can be "ti,dra7-d_can" or "ti,am3352-d_can".
+			  Can be "ti,dra7-d_can", "ti,am3352-d_can" or
+			  "ti,am4372-d_can".
 - reg			: physical base address and size of the C_CAN/D_CAN
 			  registers map
 - interrupts		: property with a value describing the interrupt
diff --git a/drivers/net/can/c_can/c_can_platform.c b/drivers/net/can/c_can/c_can_platform.c
index f4488e5..a4535d2 100644
--- a/drivers/net/can/c_can/c_can_platform.c
+++ b/drivers/net/can/c_can/c_can_platform.c
@@ -235,6 +235,7 @@ static const struct of_device_id c_can_of_table[] = {
 	{ .compatible = "bosch,d_can", .data = &d_can_drvdata },
 	{ .compatible = "ti,dra7-d_can", .data = &dra7_dcan_drvdata },
 	{ .compatible = "ti,am3352-d_can", .data = &am3352_dcan_drvdata },
+	{ .compatible = "ti,am4372-d_can", .data = &am3352_dcan_drvdata },
 	{ /* sentinel */ },
 };
 MODULE_DEVICE_TABLE(of, c_can_of_table);
-- 
1.8.3.2

Re: [PATCH v8 9/9] net: can: c_can: Add support for TI am4372 DCAN

[Marc Kleine-Budde]

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

On 11/17/2014 01:09 PM, Roger Quadros wrote:
> AM4372 SoC has 2 DCAN modules. Add compatible id and
> raminit driver data for it. The driver data is same as AM3352
> but this gives us flexibility to add AM4372 specific quirks
> if required later.

Strictly speaking we don't need the hunk in
"drivers/net/can/c_can/c_can_platform.c" yet, iff the AM4372 dts(i) has
this compatible:

"ti,am4372-d_can", "ti,am3352-d_can"

> 
> Signed-off-by: Roger Quadros <rogerq@ti.com>
> ---
>  Documentation/devicetree/bindings/net/can/c_can.txt | 3 ++-
>  drivers/net/can/c_can/c_can_platform.c              | 1 +
>  2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/can/c_can.txt b/Documentation/devicetree/bindings/net/can/c_can.txt
> index 6731730..5a1d8b0 100644
> --- a/Documentation/devicetree/bindings/net/can/c_can.txt
> +++ b/Documentation/devicetree/bindings/net/can/c_can.txt
> @@ -4,7 +4,8 @@ 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.
> -			  Can be "ti,dra7-d_can" or "ti,am3352-d_can".
> +			  Can be "ti,dra7-d_can", "ti,am3352-d_can" or
> +			  "ti,am4372-d_can".
>  - reg			: physical base address and size of the C_CAN/D_CAN
>  			  registers map
>  - interrupts		: property with a value describing the interrupt
> diff --git a/drivers/net/can/c_can/c_can_platform.c b/drivers/net/can/c_can/c_can_platform.c
> index f4488e5..a4535d2 100644
> --- a/drivers/net/can/c_can/c_can_platform.c
> +++ b/drivers/net/can/c_can/c_can_platform.c
> @@ -235,6 +235,7 @@ static const struct of_device_id c_can_of_table[] = {
>  	{ .compatible = "bosch,d_can", .data = &d_can_drvdata },
>  	{ .compatible = "ti,dra7-d_can", .data = &dra7_dcan_drvdata },
>  	{ .compatible = "ti,am3352-d_can", .data = &am3352_dcan_drvdata },
> +	{ .compatible = "ti,am4372-d_can", .data = &am3352_dcan_drvdata },
>  	{ /* sentinel */ },
>  };
>  MODULE_DEVICE_TABLE(of, c_can_of_table);
> 

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: 819 bytes --]

Re: [PATCH v8 9/9] net: can: c_can: Add support for TI am4372 DCAN

[Roger Quadros]

On 11/17/2014 02:12 PM, Marc Kleine-Budde wrote:
> On 11/17/2014 01:09 PM, Roger Quadros wrote:
>> AM4372 SoC has 2 DCAN modules. Add compatible id and
>> raminit driver data for it. The driver data is same as AM3352
>> but this gives us flexibility to add AM4372 specific quirks
>> if required later.
> 
> Strictly speaking we don't need the hunk in
> "drivers/net/can/c_can/c_can_platform.c" yet, iff the AM4372 dts(i) has
> this compatible:
> 
> "ti,am4372-d_can", "ti,am3352-d_can"

I tried with that, but checkpatch still gives this warning
WARNING: DT compatible string "ti,am4372-d_can" appears un-documented -- check ./Documentation/devicetree/bindings/

I guess we can just live with it. Please ignore this patch in that case.

cheers,
-roger

> 
>>
>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>> ---
>>  Documentation/devicetree/bindings/net/can/c_can.txt | 3 ++-
>>  drivers/net/can/c_can/c_can_platform.c              | 1 +
>>  2 files changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/net/can/c_can.txt b/Documentation/devicetree/bindings/net/can/c_can.txt
>> index 6731730..5a1d8b0 100644
>> --- a/Documentation/devicetree/bindings/net/can/c_can.txt
>> +++ b/Documentation/devicetree/bindings/net/can/c_can.txt
>> @@ -4,7 +4,8 @@ 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.
>> -			  Can be "ti,dra7-d_can" or "ti,am3352-d_can".
>> +			  Can be "ti,dra7-d_can", "ti,am3352-d_can" or
>> +			  "ti,am4372-d_can".
>>  - reg			: physical base address and size of the C_CAN/D_CAN
>>  			  registers map
>>  - interrupts		: property with a value describing the interrupt
>> diff --git a/drivers/net/can/c_can/c_can_platform.c b/drivers/net/can/c_can/c_can_platform.c
>> index f4488e5..a4535d2 100644
>> --- a/drivers/net/can/c_can/c_can_platform.c
>> +++ b/drivers/net/can/c_can/c_can_platform.c
>> @@ -235,6 +235,7 @@ static const struct of_device_id c_can_of_table[] = {
>>  	{ .compatible = "bosch,d_can", .data = &d_can_drvdata },
>>  	{ .compatible = "ti,dra7-d_can", .data = &dra7_dcan_drvdata },
>>  	{ .compatible = "ti,am3352-d_can", .data = &am3352_dcan_drvdata },
>> +	{ .compatible = "ti,am4372-d_can", .data = &am3352_dcan_drvdata },
>>  	{ /* sentinel */ },
>>  };
>>  MODULE_DEVICE_TABLE(of, c_can_of_table);
>>
> 
> Marc
> 

Re: [PATCH v8 9/9] net: can: c_can: Add support for TI am4372 DCAN

[Roger Quadros]

On 11/17/2014 02:31 PM, Roger Quadros wrote:
> On 11/17/2014 02:12 PM, Marc Kleine-Budde wrote:
>> On 11/17/2014 01:09 PM, Roger Quadros wrote:
>>> AM4372 SoC has 2 DCAN modules. Add compatible id and
>>> raminit driver data for it. The driver data is same as AM3352
>>> but this gives us flexibility to add AM4372 specific quirks
>>> if required later.
>>
>> Strictly speaking we don't need the hunk in
>> "drivers/net/can/c_can/c_can_platform.c" yet, iff the AM4372 dts(i) has
>> this compatible:
>>
>> "ti,am4372-d_can", "ti,am3352-d_can"
> 
> I tried with that, but checkpatch still gives this warning
> WARNING: DT compatible string "ti,am4372-d_can" appears un-documented -- check ./Documentation/devicetree/bindings/
> 
> I guess we can just live with it. Please ignore this patch in that case.

Did you meant that we still need to add it in the binding documentation?
It is not usable by itself so won't it be a problem to exist in documentation?

cheers,
-roger

> 
>>
>>>
>>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>>> ---
>>>  Documentation/devicetree/bindings/net/can/c_can.txt | 3 ++-
>>>  drivers/net/can/c_can/c_can_platform.c              | 1 +
>>>  2 files changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/net/can/c_can.txt b/Documentation/devicetree/bindings/net/can/c_can.txt
>>> index 6731730..5a1d8b0 100644
>>> --- a/Documentation/devicetree/bindings/net/can/c_can.txt
>>> +++ b/Documentation/devicetree/bindings/net/can/c_can.txt
>>> @@ -4,7 +4,8 @@ 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.
>>> -			  Can be "ti,dra7-d_can" or "ti,am3352-d_can".
>>> +			  Can be "ti,dra7-d_can", "ti,am3352-d_can" or
>>> +			  "ti,am4372-d_can".
>>>  - reg			: physical base address and size of the C_CAN/D_CAN
>>>  			  registers map
>>>  - interrupts		: property with a value describing the interrupt
>>> diff --git a/drivers/net/can/c_can/c_can_platform.c b/drivers/net/can/c_can/c_can_platform.c
>>> index f4488e5..a4535d2 100644
>>> --- a/drivers/net/can/c_can/c_can_platform.c
>>> +++ b/drivers/net/can/c_can/c_can_platform.c
>>> @@ -235,6 +235,7 @@ static const struct of_device_id c_can_of_table[] = {
>>>  	{ .compatible = "bosch,d_can", .data = &d_can_drvdata },
>>>  	{ .compatible = "ti,dra7-d_can", .data = &dra7_dcan_drvdata },
>>>  	{ .compatible = "ti,am3352-d_can", .data = &am3352_dcan_drvdata },
>>> +	{ .compatible = "ti,am4372-d_can", .data = &am3352_dcan_drvdata },
>>>  	{ /* sentinel */ },
>>>  };
>>>  MODULE_DEVICE_TABLE(of, c_can_of_table);
>>>
>>
>> Marc
>>
> 

Re: [PATCH v8 9/9] net: can: c_can: Add support for TI am4372 DCAN

[Marc Kleine-Budde]

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

On 11/17/2014 03:19 PM, Roger Quadros wrote:
> On 11/17/2014 02:31 PM, Roger Quadros wrote:
>> On 11/17/2014 02:12 PM, Marc Kleine-Budde wrote:
>>> On 11/17/2014 01:09 PM, Roger Quadros wrote:
>>>> AM4372 SoC has 2 DCAN modules. Add compatible id and
>>>> raminit driver data for it. The driver data is same as AM3352
>>>> but this gives us flexibility to add AM4372 specific quirks
>>>> if required later.
>>>
>>> Strictly speaking we don't need the hunk in
>>> "drivers/net/can/c_can/c_can_platform.c" yet, iff the AM4372 dts(i) has
>>> this compatible:
>>>
>>> "ti,am4372-d_can", "ti,am3352-d_can"
>>
>> I tried with that, but checkpatch still gives this warning
>> WARNING: DT compatible string "ti,am4372-d_can" appears un-documented -- check ./Documentation/devicetree/bindings/
>>
>> I guess we can just live with it. Please ignore this patch in that case.
> 
> Did you meant that we still need to add it in the binding documentation?
> It is not usable by itself so won't it be a problem to exist in documentation?

I'll take patch 9 as is, since checkpatch is a bit more picky now :)

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: 819 bytes --]

Re: [PATCH v8 4/8] can: c_can: Add syscon/regmap RAMINIT mechanism

[Tomi Valkeinen]

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

Hi Roger,

On 14/11/14 20:09, Marc Kleine-Budde wrote:
> From: Roger Quadros <rogerq@ti.com>
> 
> Some TI SoCs like DRA7 have a RAMINIT register specification
> different from the other AMxx SoCs and as expected by the
> existing driver.
> 
> To add more insanity, this register is shared with other
> IPs like DSS, PCIe and PWM.
> 
> Provides a more generic mechanism to specify the RAMINIT
> register location and START/DONE bit position and use the
> syscon/regmap framework to access the register.

This patch updates the syscon regmap using regmap_read + regmap_write.
That's not a safe way to update the bits, as some other driver may touch
the register between the read and write. The change has to be made using
regmap_update_bits.

We don't have other drivers using the register at the moment, but I
presume we will sooner or later.

 Tomi



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

Re: [PATCH v8 4/8] can: c_can: Add syscon/regmap RAMINIT mechanism

[Roger Quadros]

Tomi,

On 05/01/15 11:18, Tomi Valkeinen wrote:
> Hi Roger,
> 
> On 14/11/14 20:09, Marc Kleine-Budde wrote:
>> From: Roger Quadros <rogerq@ti.com>
>>
>> Some TI SoCs like DRA7 have a RAMINIT register specification
>> different from the other AMxx SoCs and as expected by the
>> existing driver.
>>
>> To add more insanity, this register is shared with other
>> IPs like DSS, PCIe and PWM.
>>
>> Provides a more generic mechanism to specify the RAMINIT
>> register location and START/DONE bit position and use the
>> syscon/regmap framework to access the register.
> 
> This patch updates the syscon regmap using regmap_read + regmap_write.
> That's not a safe way to update the bits, as some other driver may touch
> the register between the read and write. The change has to be made using
> regmap_update_bits.
> 
> We don't have other drivers using the register at the moment, but I
> presume we will sooner or later.

I remember updating this after you pointed it out to me earlier, but it seems I picked up the older version while sending. :(.
Thanks for pointing it again. I'll prepare a fix on top.

cheers,
-roger