[PATCH v12 0/6] flexcan/powerpc: Add support for powerpc flexcan (freescale p1010)

[PATCH v12 0/6] flexcan/powerpc: Add support for powerpc flexcan (freescale p1010)

[Robin Holt]

With all the patches applied, my p1010rdb works for communicating between
its two can ports and also can communicate with an external PSOC.
I have done no testing beyond compile testing for an arm system as I
have no access to an arm based system.

With the latest version, changes to the arch tree really only reflect
changes in the drivers/net/can tree.  I, therefore, believe it is
probably best to route them through David S. Miller's netdev tree.
Wolfgang and Kumar, does that seem correct to you?

Thanks,
Robin Holt

[PATCH v11 1/6] flexcan: Remove #include <mach/clock.h>

[Robin Holt]

powerpc does not have a mach-####/clock.h.  When testing, I found neither
arm nor powerpc needed the mach/clock.h at all so I removed it.

Signed-off-by: Robin Holt <holt@sgi.com>
Acked-by: Marc Kleine-Budde <mkl@pengutronix.de>
Acked-by: Wolfgang Grandegger <wg@grandegger.com>
Cc: U Bhaskar-B22300 <B22300@freescale.com>
Cc: socketcan-core@lists.berlios.de
Cc: netdev@vger.kernel.org
Cc: PPC list <linuxppc-dev@lists.ozlabs.org>
---
 drivers/net/can/flexcan.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 1767811..586b2cd 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -35,8 +35,6 @@
 #include <linux/module.h>
 #include <linux/platform_device.h>
 
-#include <mach/clock.h>
-
 #define DRV_NAME			"flexcan"
 
 /* 8 for RX fifo and 2 error handling */
-- 
1.7.2.1

[PATCH v11 2/6] flexcan: Abstract off read/write for big/little endian.

[Robin Holt]

Make flexcan driver handle register reads in the appropriate endianess.
This was a basic search and replace and then define some inlines.

Signed-off-by: Robin Holt <holt@sgi.com>
Acked-by: Marc Kleine-Budde <mkl@pengutronix.de>
Acked-by: Wolfgang Grandegger <wg@grandegger.com>
Cc: U Bhaskar-B22300 <B22300@freescale.com>
Cc: socketcan-core@lists.berlios.de
Cc: netdev@vger.kernel.org
Cc: PPC list <linuxppc-dev@lists.ozlabs.org>
---
 drivers/net/can/flexcan.c |  140 ++++++++++++++++++++++++++------------------
 1 files changed, 83 insertions(+), 57 deletions(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 586b2cd..68cbe52 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -190,6 +190,31 @@ static struct can_bittiming_const flexcan_bittiming_const = {
 };
 
 /*
+ * Abstract off the read/write for arm versus ppc.
+ */
+#if defined(__BIG_ENDIAN)
+static inline u32 flexcan_read(void __iomem *addr)
+{
+	return in_be32(addr);
+}
+
+static inline void flexcan_write(u32 val, void __iomem *addr)
+{
+	out_be32(addr, val);
+}
+#else
+static inline u32 flexcan_read(void __iomem *addr)
+{
+	return readl(addr);
+}
+
+static inline void flexcan_write(u32 val, void __iomem *addr)
+{
+	writel(val, addr);
+}
+#endif
+
+/*
  * Swtich transceiver on or off
  */
 static void flexcan_transceiver_switch(const struct flexcan_priv *priv, int on)
@@ -210,9 +235,9 @@ static inline void flexcan_chip_enable(struct flexcan_priv *priv)
 	struct flexcan_regs __iomem *regs = priv->base;
 	u32 reg;
 
-	reg = readl(&regs->mcr);
+	reg = flexcan_read(&regs->mcr);
 	reg &= ~FLEXCAN_MCR_MDIS;
-	writel(reg, &regs->mcr);
+	flexcan_write(reg, &regs->mcr);
 
 	udelay(10);
 }
@@ -222,9 +247,9 @@ static inline void flexcan_chip_disable(struct flexcan_priv *priv)
 	struct flexcan_regs __iomem *regs = priv->base;
 	u32 reg;
 
-	reg = readl(&regs->mcr);
+	reg = flexcan_read(&regs->mcr);
 	reg |= FLEXCAN_MCR_MDIS;
-	writel(reg, &regs->mcr);
+	flexcan_write(reg, &regs->mcr);
 }
 
 static int flexcan_get_berr_counter(const struct net_device *dev,
@@ -232,7 +257,7 @@ static int flexcan_get_berr_counter(const struct net_device *dev,
 {
 	const struct flexcan_priv *priv = netdev_priv(dev);
 	struct flexcan_regs __iomem *regs = priv->base;
-	u32 reg = readl(&regs->ecr);
+	u32 reg = flexcan_read(&regs->ecr);
 
 	bec->txerr = (reg >> 0) & 0xff;
 	bec->rxerr = (reg >> 8) & 0xff;
@@ -266,15 +291,15 @@ static int flexcan_start_xmit(struct sk_buff *skb, struct net_device *dev)
 
 	if (cf->can_dlc > 0) {
 		u32 data = be32_to_cpup((__be32 *)&cf->data[0]);
-		writel(data, &regs->cantxfg[FLEXCAN_TX_BUF_ID].data[0]);
+		flexcan_write(data, &regs->cantxfg[FLEXCAN_TX_BUF_ID].data[0]);
 	}
 	if (cf->can_dlc > 3) {
 		u32 data = be32_to_cpup((__be32 *)&cf->data[4]);
-		writel(data, &regs->cantxfg[FLEXCAN_TX_BUF_ID].data[1]);
+		flexcan_write(data, &regs->cantxfg[FLEXCAN_TX_BUF_ID].data[1]);
 	}
 
-	writel(can_id, &regs->cantxfg[FLEXCAN_TX_BUF_ID].can_id);
-	writel(ctrl, &regs->cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl);
+	flexcan_write(can_id, &regs->cantxfg[FLEXCAN_TX_BUF_ID].can_id);
+	flexcan_write(ctrl, &regs->cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl);
 
 	kfree_skb(skb);
 
@@ -462,8 +487,8 @@ static void flexcan_read_fifo(const struct net_device *dev,
 	struct flexcan_mb __iomem *mb = &regs->cantxfg[0];
 	u32 reg_ctrl, reg_id;
 
-	reg_ctrl = readl(&mb->can_ctrl);
-	reg_id = readl(&mb->can_id);
+	reg_ctrl = flexcan_read(&mb->can_ctrl);
+	reg_id = flexcan_read(&mb->can_id);
 	if (reg_ctrl & FLEXCAN_MB_CNT_IDE)
 		cf->can_id = ((reg_id >> 0) & CAN_EFF_MASK) | CAN_EFF_FLAG;
 	else
@@ -473,12 +498,12 @@ static void flexcan_read_fifo(const struct net_device *dev,
 		cf->can_id |= CAN_RTR_FLAG;
 	cf->can_dlc = get_can_dlc((reg_ctrl >> 16) & 0xf);
 
-	*(__be32 *)(cf->data + 0) = cpu_to_be32(readl(&mb->data[0]));
-	*(__be32 *)(cf->data + 4) = cpu_to_be32(readl(&mb->data[1]));
+	*(__be32 *)(cf->data + 0) = cpu_to_be32(flexcan_read(&mb->data[0]));
+	*(__be32 *)(cf->data + 4) = cpu_to_be32(flexcan_read(&mb->data[1]));
 
 	/* mark as read */
-	writel(FLEXCAN_IFLAG_RX_FIFO_AVAILABLE, &regs->iflag1);
-	readl(&regs->timer);
+	flexcan_write(FLEXCAN_IFLAG_RX_FIFO_AVAILABLE, &regs->iflag1);
+	flexcan_read(&regs->timer);
 }
 
 static int flexcan_read_frame(struct net_device *dev)
@@ -514,17 +539,17 @@ static int flexcan_poll(struct napi_struct *napi, int quota)
 	 * The error bits are cleared on read,
 	 * use saved value from irq handler.
 	 */
-	reg_esr = readl(&regs->esr) | priv->reg_esr;
+	reg_esr = flexcan_read(&regs->esr) | priv->reg_esr;
 
 	/* handle state changes */
 	work_done += flexcan_poll_state(dev, reg_esr);
 
 	/* handle RX-FIFO */
-	reg_iflag1 = readl(&regs->iflag1);
+	reg_iflag1 = flexcan_read(&regs->iflag1);
 	while (reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_AVAILABLE &&
 	       work_done < quota) {
 		work_done += flexcan_read_frame(dev);
-		reg_iflag1 = readl(&regs->iflag1);
+		reg_iflag1 = flexcan_read(&regs->iflag1);
 	}
 
 	/* report bus errors */
@@ -534,8 +559,8 @@ static int flexcan_poll(struct napi_struct *napi, int quota)
 	if (work_done < quota) {
 		napi_complete(napi);
 		/* enable IRQs */
-		writel(FLEXCAN_IFLAG_DEFAULT, &regs->imask1);
-		writel(priv->reg_ctrl_default, &regs->ctrl);
+		flexcan_write(FLEXCAN_IFLAG_DEFAULT, &regs->imask1);
+		flexcan_write(priv->reg_ctrl_default, &regs->ctrl);
 	}
 
 	return work_done;
@@ -549,9 +574,9 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
 	struct flexcan_regs __iomem *regs = priv->base;
 	u32 reg_iflag1, reg_esr;
 
-	reg_iflag1 = readl(&regs->iflag1);
-	reg_esr = readl(&regs->esr);
-	writel(FLEXCAN_ESR_ERR_INT, &regs->esr);	/* ACK err IRQ */
+	reg_iflag1 = flexcan_read(&regs->iflag1);
+	reg_esr = flexcan_read(&regs->esr);
+	flexcan_write(FLEXCAN_ESR_ERR_INT, &regs->esr);	/* ACK err IRQ */
 
 	/*
 	 * schedule NAPI in case of:
@@ -567,16 +592,16 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
 		 * save them for later use.
 		 */
 		priv->reg_esr = reg_esr & FLEXCAN_ESR_ERR_BUS;
-		writel(FLEXCAN_IFLAG_DEFAULT & ~FLEXCAN_IFLAG_RX_FIFO_AVAILABLE,
-		       &regs->imask1);
-		writel(priv->reg_ctrl_default & ~FLEXCAN_CTRL_ERR_ALL,
+		flexcan_write(FLEXCAN_IFLAG_DEFAULT &
+			~FLEXCAN_IFLAG_RX_FIFO_AVAILABLE, &regs->imask1);
+		flexcan_write(priv->reg_ctrl_default & ~FLEXCAN_CTRL_ERR_ALL,
 		       &regs->ctrl);
 		napi_schedule(&priv->napi);
 	}
 
 	/* FIFO overflow */
 	if (reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_OVERFLOW) {
-		writel(FLEXCAN_IFLAG_RX_FIFO_OVERFLOW, &regs->iflag1);
+		flexcan_write(FLEXCAN_IFLAG_RX_FIFO_OVERFLOW, &regs->iflag1);
 		dev->stats.rx_over_errors++;
 		dev->stats.rx_errors++;
 	}
@@ -585,7 +610,7 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
 	if (reg_iflag1 & (1 << FLEXCAN_TX_BUF_ID)) {
 		/* tx_bytes is incremented in flexcan_start_xmit */
 		stats->tx_packets++;
-		writel((1 << FLEXCAN_TX_BUF_ID), &regs->iflag1);
+		flexcan_write((1 << FLEXCAN_TX_BUF_ID), &regs->iflag1);
 		netif_wake_queue(dev);
 	}
 
@@ -599,7 +624,7 @@ static void flexcan_set_bittiming(struct net_device *dev)
 	struct flexcan_regs __iomem *regs = priv->base;
 	u32 reg;
 
-	reg = readl(&regs->ctrl);
+	reg = flexcan_read(&regs->ctrl);
 	reg &= ~(FLEXCAN_CTRL_PRESDIV(0xff) |
 		 FLEXCAN_CTRL_RJW(0x3) |
 		 FLEXCAN_CTRL_PSEG1(0x7) |
@@ -623,11 +648,11 @@ static void flexcan_set_bittiming(struct net_device *dev)
 		reg |= FLEXCAN_CTRL_SMP;
 
 	dev_info(dev->dev.parent, "writing ctrl=0x%08x\n", reg);
-	writel(reg, &regs->ctrl);
+	flexcan_write(reg, &regs->ctrl);
 
 	/* print chip status */
 	dev_dbg(dev->dev.parent, "%s: mcr=0x%08x ctrl=0x%08x\n", __func__,
-		readl(&regs->mcr), readl(&regs->ctrl));
+		flexcan_read(&regs->mcr), flexcan_read(&regs->ctrl));
 }
 
 /*
@@ -648,10 +673,10 @@ static int flexcan_chip_start(struct net_device *dev)
 	flexcan_chip_enable(priv);
 
 	/* soft reset */
-	writel(FLEXCAN_MCR_SOFTRST, &regs->mcr);
+	flexcan_write(FLEXCAN_MCR_SOFTRST, &regs->mcr);
 	udelay(10);
 
-	reg_mcr = readl(&regs->mcr);
+	reg_mcr = flexcan_read(&regs->mcr);
 	if (reg_mcr & FLEXCAN_MCR_SOFTRST) {
 		dev_err(dev->dev.parent,
 			"Failed to softreset can module (mcr=0x%08x)\n",
@@ -673,12 +698,12 @@ static int flexcan_chip_start(struct net_device *dev)
 	 * choose format C
 	 *
 	 */
-	reg_mcr = readl(&regs->mcr);
+	reg_mcr = flexcan_read(&regs->mcr);
 	reg_mcr |= FLEXCAN_MCR_FRZ | FLEXCAN_MCR_FEN | FLEXCAN_MCR_HALT |
 		FLEXCAN_MCR_SUPV | FLEXCAN_MCR_WRN_EN |
 		FLEXCAN_MCR_IDAM_C;
 	dev_dbg(dev->dev.parent, "%s: writing mcr=0x%08x", __func__, reg_mcr);
-	writel(reg_mcr, &regs->mcr);
+	flexcan_write(reg_mcr, &regs->mcr);
 
 	/*
 	 * CTRL
@@ -696,7 +721,7 @@ static int flexcan_chip_start(struct net_device *dev)
 	 * (FLEXCAN_CTRL_ERR_MSK), too. Otherwise we don't get any
 	 * warning or bus passive interrupts.
 	 */
-	reg_ctrl = readl(&regs->ctrl);
+	reg_ctrl = flexcan_read(&regs->ctrl);
 	reg_ctrl &= ~FLEXCAN_CTRL_TSYN;
 	reg_ctrl |= FLEXCAN_CTRL_BOFF_REC | FLEXCAN_CTRL_LBUF |
 		FLEXCAN_CTRL_ERR_STATE | FLEXCAN_CTRL_ERR_MSK;
@@ -704,38 +729,39 @@ static int flexcan_chip_start(struct net_device *dev)
 	/* save for later use */
 	priv->reg_ctrl_default = reg_ctrl;
 	dev_dbg(dev->dev.parent, "%s: writing ctrl=0x%08x", __func__, reg_ctrl);
-	writel(reg_ctrl, &regs->ctrl);
+	flexcan_write(reg_ctrl, &regs->ctrl);
 
 	for (i = 0; i < ARRAY_SIZE(regs->cantxfg); i++) {
-		writel(0, &regs->cantxfg[i].can_ctrl);
-		writel(0, &regs->cantxfg[i].can_id);
-		writel(0, &regs->cantxfg[i].data[0]);
-		writel(0, &regs->cantxfg[i].data[1]);
+		flexcan_write(0, &regs->cantxfg[i].can_ctrl);
+		flexcan_write(0, &regs->cantxfg[i].can_id);
+		flexcan_write(0, &regs->cantxfg[i].data[0]);
+		flexcan_write(0, &regs->cantxfg[i].data[1]);
 
 		/* put MB into rx queue */
-		writel(FLEXCAN_MB_CNT_CODE(0x4), &regs->cantxfg[i].can_ctrl);
+		flexcan_write(FLEXCAN_MB_CNT_CODE(0x4),
+			&regs->cantxfg[i].can_ctrl);
 	}
 
 	/* acceptance mask/acceptance code (accept everything) */
-	writel(0x0, &regs->rxgmask);
-	writel(0x0, &regs->rx14mask);
-	writel(0x0, &regs->rx15mask);
+	flexcan_write(0x0, &regs->rxgmask);
+	flexcan_write(0x0, &regs->rx14mask);
+	flexcan_write(0x0, &regs->rx15mask);
 
 	flexcan_transceiver_switch(priv, 1);
 
 	/* synchronize with the can bus */
-	reg_mcr = readl(&regs->mcr);
+	reg_mcr = flexcan_read(&regs->mcr);
 	reg_mcr &= ~FLEXCAN_MCR_HALT;
-	writel(reg_mcr, &regs->mcr);
+	flexcan_write(reg_mcr, &regs->mcr);
 
 	priv->can.state = CAN_STATE_ERROR_ACTIVE;
 
 	/* enable FIFO interrupts */
-	writel(FLEXCAN_IFLAG_DEFAULT, &regs->imask1);
+	flexcan_write(FLEXCAN_IFLAG_DEFAULT, &regs->imask1);
 
 	/* print chip status */
 	dev_dbg(dev->dev.parent, "%s: reading mcr=0x%08x ctrl=0x%08x\n",
-		__func__, readl(&regs->mcr), readl(&regs->ctrl));
+		__func__, flexcan_read(&regs->mcr), flexcan_read(&regs->ctrl));
 
 	return 0;
 
@@ -757,12 +783,12 @@ static void flexcan_chip_stop(struct net_device *dev)
 	u32 reg;
 
 	/* Disable all interrupts */
-	writel(0, &regs->imask1);
+	flexcan_write(0, &regs->imask1);
 
 	/* Disable + halt module */
-	reg = readl(&regs->mcr);
+	reg = flexcan_read(&regs->mcr);
 	reg |= FLEXCAN_MCR_MDIS | FLEXCAN_MCR_HALT;
-	writel(reg, &regs->mcr);
+	flexcan_write(reg, &regs->mcr);
 
 	flexcan_transceiver_switch(priv, 0);
 	priv->can.state = CAN_STATE_STOPPED;
@@ -854,24 +880,24 @@ static int __devinit register_flexcandev(struct net_device *dev)
 
 	/* select "bus clock", chip must be disabled */
 	flexcan_chip_disable(priv);
-	reg = readl(&regs->ctrl);
+	reg = flexcan_read(&regs->ctrl);
 	reg |= FLEXCAN_CTRL_CLK_SRC;
-	writel(reg, &regs->ctrl);
+	flexcan_write(reg, &regs->ctrl);
 
 	flexcan_chip_enable(priv);
 
 	/* set freeze, halt and activate FIFO, restrict register access */
-	reg = readl(&regs->mcr);
+	reg = flexcan_read(&regs->mcr);
 	reg |= FLEXCAN_MCR_FRZ | FLEXCAN_MCR_HALT |
 		FLEXCAN_MCR_FEN | FLEXCAN_MCR_SUPV;
-	writel(reg, &regs->mcr);
+	flexcan_write(reg, &regs->mcr);
 
 	/*
 	 * Currently we only support newer versions of this core
 	 * featuring a RX FIFO. Older cores found on some Coldfire
 	 * derivates are not yet supported.
 	 */
-	reg = readl(&regs->mcr);
+	reg = flexcan_read(&regs->mcr);
 	if (!(reg & FLEXCAN_MCR_FEN)) {
 		dev_err(dev->dev.parent,
 			"Could not enable RX FIFO, unsupported core\n");
-- 
1.7.2.1

[PATCH v11 3/6] flexcan: Fix up fsl-flexcan device tree binding.

[Robin Holt]

This patch cleans up the documentation of the device-tree binding for
the Flexcan devices on Freescale's PowerPC and ARM cores. Extra
properties are not used by the driver so we are removing them.

Signed-off-by: Robin Holt <holt@sgi.com>
To: Marc Kleine-Budde <mkl@pengutronix.de>,
To: Wolfgang Grandegger <wg@grandegger.com>,
To: U Bhaskar-B22300 <B22300@freescale.com>
To: Scott Wood <scottwood@freescale.com>
To: Grant Likely <grant.likely@secretlab.ca>
To: Kumar Gala <galak@kernel.crashing.org>
Cc: socketcan-core@lists.berlios.de,
Cc: netdev@vger.kernel.org,
Cc: PPC list <linuxppc-dev@lists.ozlabs.org>
Cc: devicetree-discuss@lists.ozlabs.org
---
 .../devicetree/bindings/net/can/fsl-flexcan.txt    |   69 ++++---------------
 arch/powerpc/boot/dts/p1010rdb.dts                 |   10 +--
 arch/powerpc/boot/dts/p1010si.dtsi                 |   10 +--
 3 files changed, 21 insertions(+), 68 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
index 1a729f0..c78dcbb 100644
--- a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
+++ b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
@@ -1,61 +1,22 @@
-CAN Device Tree Bindings
-------------------------
-2011 Freescale Semiconductor, Inc.
+Flexcan CAN contoller on Freescale's ARM and PowerPC processors
 
-fsl,flexcan-v1.0 nodes
------------------------
-In addition to the required compatible-, reg- and interrupt-properties, you can
-also specify which clock source shall be used for the controller.
+Required properties:
 
-CPI Clock- Can Protocol Interface Clock
-	This CLK_SRC bit of CTRL(control register) selects the clock source to
-	the CAN Protocol Interface(CPI) to be either the peripheral clock
-	(driven by the PLL) or the crystal oscillator clock. The selected clock
-	is the one fed to the prescaler to generate the Serial Clock (Sclock).
-	The PRESDIV field of CTRL(control register) controls a prescaler that
-	generates the Serial Clock (Sclock), whose period defines the
-	time quantum used to compose the CAN waveform.
+- compatible : Should be "fsl,<processor>-flexcan" and "fsl,flexcan"
 
-Can Engine Clock Source
-	There are two sources for CAN clock
-	- Platform Clock  It represents the bus clock
-	- Oscillator Clock
+  An implementation should also claim any of the following compatibles
+  that it is fully backwards compatible with:
 
-	Peripheral Clock (PLL)
-	--------------
-		     |
-		    ---------		      -------------
-		    |       |CPI Clock	      | Prescaler |       Sclock
-		    |       |---------------->| (1.. 256) |------------>
-		    ---------		      -------------
-                     |  |
-	--------------  ---------------------CLK_SRC
-	Oscillator Clock
+  - fsl,p1010-flexcan
 
-- fsl,flexcan-clock-source : CAN Engine Clock Source.This property selects
-			     the peripheral clock. PLL clock is fed to the
-			     prescaler to generate the Serial Clock (Sclock).
-			     Valid values are "oscillator" and "platform"
-			     "oscillator": CAN engine clock source is oscillator clock.
-			     "platform" The CAN engine clock source is the bus clock
-		             (platform clock).
+- reg : Offset and length of the register set for this device
+- interrupts : Interrupt tuple for this device
 
-- fsl,flexcan-clock-divider : for the reference and system clock, an additional
-			      clock divider can be specified.
-- clock-frequency: frequency required to calculate the bitrate for FlexCAN.
+Example:
 
-Note:
-	- v1.0 of flexcan-v1.0 represent the IP block version for P1010 SOC.
-	- P1010 does not have oscillator as the Clock Source.So the default
-	  Clock Source is platform clock.
-Examples:
-
-	can0@1c000 {
-		compatible = "fsl,flexcan-v1.0";
-		reg = <0x1c000 0x1000>;
-		interrupts = <48 0x2>;
-		interrupt-parent = <&mpic>;
-		fsl,flexcan-clock-source = "platform";
-		fsl,flexcan-clock-divider = <2>;
-		clock-frequency = <fixed by u-boot>;
-	};
+  can@1c000 {
+          compatible = "fsl,p1010-flexcan", "fsl,flexcan";
+          reg = <0x1c000 0x1000>;
+          interrupts = <48 0x2>;
+          interrupt-parent = <&mpic>;
+  };
diff --git a/arch/powerpc/boot/dts/p1010rdb.dts b/arch/powerpc/boot/dts/p1010rdb.dts
index 6b33b73..d6c669c 100644
--- a/arch/powerpc/boot/dts/p1010rdb.dts
+++ b/arch/powerpc/boot/dts/p1010rdb.dts
@@ -23,6 +23,8 @@
 		ethernet2 = &enet2;
 		pci0 = &pci0;
 		pci1 = &pci1;
+		can0 = &can0;
+		can1 = &can1;
 	};
 
 	memory {
@@ -169,14 +171,6 @@
 			};
 		};
 
-		can0@1c000 {
-			fsl,flexcan-clock-source = "platform";
-		};
-
-		can1@1d000 {
-			fsl,flexcan-clock-source = "platform";
-		};
-
 		usb@22000 {
 			phy_type = "utmi";
 		};
diff --git a/arch/powerpc/boot/dts/p1010si.dtsi b/arch/powerpc/boot/dts/p1010si.dtsi
index 7f51104..f00076b 100644
--- a/arch/powerpc/boot/dts/p1010si.dtsi
+++ b/arch/powerpc/boot/dts/p1010si.dtsi
@@ -140,20 +140,18 @@
 			interrupt-parent = <&mpic>;
 		};
 
-		can0@1c000 {
-			compatible = "fsl,flexcan-v1.0";
+		can0: can@1c000 {
+			compatible = "fsl,p1010-flexcan", "fsl,flexcan";
 			reg = <0x1c000 0x1000>;
 			interrupts = <48 0x2>;
 			interrupt-parent = <&mpic>;
-			fsl,flexcan-clock-divider = <2>;
 		};
 
-		can1@1d000 {
-			compatible = "fsl,flexcan-v1.0";
+		can1: can@1d000 {
+			compatible = "fsl,p1010-flexcan", "fsl,flexcan";
 			reg = <0x1d000 0x1000>;
 			interrupts = <61 0x2>;
 			interrupt-parent = <&mpic>;
-			fsl,flexcan-clock-divider = <2>;
 		};
 
 		L2: l2-cache-controller@20000 {
-- 
1.7.2.1

Re: [PATCH v11 3/6] flexcan: Fix up fsl-flexcan device tree binding.

[Grant Likely]

On Thu, Aug 11, 2011 at 10:07 AM, Robin Holt <holt@sgi.com> wrote:
> This patch cleans up the documentation of the device-tree binding for
> the Flexcan devices on Freescale's PowerPC and ARM cores. Extra
> properties are not used by the driver so we are removing them.
>
> Signed-off-by: Robin Holt <holt@sgi.com>
> To: Marc Kleine-Budde <mkl@pengutronix.de>,
> To: Wolfgang Grandegger <wg@grandegger.com>,
> To: U Bhaskar-B22300 <B22300@freescale.com>
> To: Scott Wood <scottwood@freescale.com>
> To: Grant Likely <grant.likely@secretlab.ca>
> To: Kumar Gala <galak@kernel.crashing.org>
> Cc: socketcan-core@lists.berlios.de,
> Cc: netdev@vger.kernel.org,
> Cc: PPC list <linuxppc-dev@lists.ozlabs.org>
> Cc: devicetree-discuss@lists.ozlabs.org
> ---
> =A0.../devicetree/bindings/net/can/fsl-flexcan.txt =A0 =A0| =A0 69 ++++--=
-------------
> =A0arch/powerpc/boot/dts/p1010rdb.dts =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 | =
=A0 10 +--
> =A0arch/powerpc/boot/dts/p1010si.dtsi =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 | =
=A0 10 +--
> =A03 files changed, 21 insertions(+), 68 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt b/=
Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> index 1a729f0..c78dcbb 100644
> --- a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> +++ b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> @@ -1,61 +1,22 @@
> -CAN Device Tree Bindings
> -------------------------
> -2011 Freescale Semiconductor, Inc.
> +Flexcan CAN contoller on Freescale's ARM and PowerPC processors
>
> -fsl,flexcan-v1.0 nodes
> ------------------------
> -In addition to the required compatible-, reg- and interrupt-properties, =
you can
> -also specify which clock source shall be used for the controller.
> +Required properties:
>
> -CPI Clock- Can Protocol Interface Clock
> - =A0 =A0 =A0 This CLK_SRC bit of CTRL(control register) selects the cloc=
k source to
> - =A0 =A0 =A0 the CAN Protocol Interface(CPI) to be either the peripheral=
 clock
> - =A0 =A0 =A0 (driven by the PLL) or the crystal oscillator clock. The se=
lected clock
> - =A0 =A0 =A0 is the one fed to the prescaler to generate the Serial Cloc=
k (Sclock).
> - =A0 =A0 =A0 The PRESDIV field of CTRL(control register) controls a pres=
caler that
> - =A0 =A0 =A0 generates the Serial Clock (Sclock), whose period defines t=
he
> - =A0 =A0 =A0 time quantum used to compose the CAN waveform.
> +- compatible : Should be "fsl,<processor>-flexcan" and "fsl,flexcan"

Don't do this.  "fsl,flexcan" is far too generic.  Be specific to the
soc part number or the ip core implementation version.

>
> -Can Engine Clock Source
> - =A0 =A0 =A0 There are two sources for CAN clock
> - =A0 =A0 =A0 - Platform Clock =A0It represents the bus clock
> - =A0 =A0 =A0 - Oscillator Clock
> + =A0An implementation should also claim any of the following compatibles
> + =A0that it is fully backwards compatible with:
>
> - =A0 =A0 =A0 Peripheral Clock (PLL)
> - =A0 =A0 =A0 --------------
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0|
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 --------- =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 -------------
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 | =A0 =A0 =A0 |CPI Clock =A0 =A0 =
=A0 =A0| Prescaler | =A0 =A0 =A0 Sclock
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 | =A0 =A0 =A0 |---------------->| (=
1.. 256) |------------>
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 --------- =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 -------------
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 | =A0|
> - =A0 =A0 =A0 -------------- =A0---------------------CLK_SRC
> - =A0 =A0 =A0 Oscillator Clock
> + =A0- fsl,p1010-flexcan
>
> -- fsl,flexcan-clock-source : CAN Engine Clock Source.This property selec=
ts
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0the peripheral c=
lock. PLL clock is fed to the
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0prescaler to gen=
erate the Serial Clock (Sclock).
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0Valid values are=
 "oscillator" and "platform"
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0"oscillator": CA=
N engine clock source is oscillator clock.
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0"platform" The C=
AN engine clock source is the bus clock
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0(platform clock)=
.
> +- reg : Offset and length of the register set for this device
> +- interrupts : Interrupt tuple for this device
>
> -- fsl,flexcan-clock-divider : for the reference and system clock, an add=
itional
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 clock divider c=
an be specified.
> -- clock-frequency: frequency required to calculate the bitrate for FlexC=
AN.
> +Example:
>
> -Note:
> - =A0 =A0 =A0 - v1.0 of flexcan-v1.0 represent the IP block version for P=
1010 SOC.
> - =A0 =A0 =A0 - P1010 does not have oscillator as the Clock Source.So the=
 default
> - =A0 =A0 =A0 =A0 Clock Source is platform clock.
> -Examples:
> -
> - =A0 =A0 =A0 can0@1c000 {
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 compatible =3D "fsl,flexcan-v1.0";
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 reg =3D <0x1c000 0x1000>;
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 interrupts =3D <48 0x2>;
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 interrupt-parent =3D <&mpic>;
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 fsl,flexcan-clock-source =3D "platform";
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 fsl,flexcan-clock-divider =3D <2>;
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 clock-frequency =3D <fixed by u-boot>;
> - =A0 =A0 =A0 };
> + =A0can@1c000 {
> + =A0 =A0 =A0 =A0 =A0compatible =3D "fsl,p1010-flexcan", "fsl,flexcan";
> + =A0 =A0 =A0 =A0 =A0reg =3D <0x1c000 0x1000>;
> + =A0 =A0 =A0 =A0 =A0interrupts =3D <48 0x2>;
> + =A0 =A0 =A0 =A0 =A0interrupt-parent =3D <&mpic>;
> + =A0};

The diffstat for this patch looks too big because the whitespace has
changed.  Try to restrict whitespace changes so that the patch is
friendly to reviewers.

g.

Re: [PATCH v11 3/6] flexcan: Fix up fsl-flexcan device tree binding.

[Robin Holt]

On Thu, Aug 11, 2011 at 10:53:43AM -0600, Grant Likely wrote:
> On Thu, Aug 11, 2011 at 10:07 AM, Robin Holt <holt@sgi.com> wrote:
> > +- compatible : Should be "fsl,<processor>-flexcan" and "fsl,flexcan"
> 
> Don't do this.  "fsl,flexcan" is far too generic.  Be specific to the
> soc part number or the ip core implementation version.

I don't have any crumbs to go with here.  There is nothing in the
documentation I have found to indicate what this is or should be.
I looked at the documentation for the P1010 processor and there is nothing
in there which I noticed that indicates what I could possibly use other
than flexcan.  They don't even indicate the registers are equivalent or
identical to their i.MX implementations for i.MX25 and i.MX35.  The only
thing they call it is flexcan.  I have asked our local freescale rep and
he said "There is no 'chip', it is just flexcan.  flexcan is flexcan."
His tone was such that I got the feeling he thought the question was
crazy as flexcan is flexcan.

> > -Can Engine Clock Source
> > -       There are two sources for CAN clock
> > -       - Platform Clock  It represents the bus clock
> > -       - Oscillator Clock
> > +  An implementation should also claim any of the following compatibles
> > +  that it is fully backwards compatible with:
> >
> > -       Peripheral Clock (PLL)
> > -       --------------
> > -                    |
> > -                   ---------                 -------------
> > -                   |       |CPI Clock        | Prescaler |       Sclock
> > -                   |       |---------------->| (1.. 256) |------------>
> > -                   ---------                 -------------
> > -                     |  |
> > -       --------------  ---------------------CLK_SRC
> > -       Oscillator Clock
> > +  - fsl,p1010-flexcan
> >
> > -- fsl,flexcan-clock-source : CAN Engine Clock Source.This property selects
> > -                            the peripheral clock. PLL clock is fed to the
> > -                            prescaler to generate the Serial Clock (Sclock).
> > -                            Valid values are "oscillator" and "platform"
> > -                            "oscillator": CAN engine clock source is oscillator clock.
> > -                            "platform" The CAN engine clock source is the bus clock
> > -                            (platform clock).
> > +- reg : Offset and length of the register set for this device
> > +- interrupts : Interrupt tuple for this device
> >
> > -- fsl,flexcan-clock-divider : for the reference and system clock, an additional
> > -                             clock divider can be specified.
> > -- clock-frequency: frequency required to calculate the bitrate for FlexCAN.
> > +Example:
> >
> > -Note:
> > -       - v1.0 of flexcan-v1.0 represent the IP block version for P1010 SOC.
> > -       - P1010 does not have oscillator as the Clock Source.So the default
> > -         Clock Source is platform clock.
> > -Examples:
> > -
> > -       can0@1c000 {
> > -               compatible = "fsl,flexcan-v1.0";
> > -               reg = <0x1c000 0x1000>;
> > -               interrupts = <48 0x2>;
> > -               interrupt-parent = <&mpic>;
> > -               fsl,flexcan-clock-source = "platform";
> > -               fsl,flexcan-clock-divider = <2>;
> > -               clock-frequency = <fixed by u-boot>;
> > -       };
> > +  can@1c000 {
> > +          compatible = "fsl,p1010-flexcan", "fsl,flexcan";
> > +          reg = <0x1c000 0x1000>;
> > +          interrupts = <48 0x2>;
> > +          interrupt-parent = <&mpic>;
> > +  };
> 
> The diffstat for this patch looks too big because the whitespace has
> changed.  Try to restrict whitespace changes so that the patch is
> friendly to reviewers.

Reworked the best I can.  That reduced the diffstat by 3 lines.

Robin

[PATCH v11 4/6] flexcan: Add of_match to platform_device definition.

[Robin Holt]

On powerpc, the OpenFirmware devices are not matched without specifying
an of_match array.  Introduce that array as that is used for matching
on the Freescale P1010 processor.

Signed-off-by: Robin Holt <holt@sgi.com>
Acked-by: Marc Kleine-Budde <mkl@pengutronix.de>
Acked-by: Wolfgang Grandegger <wg@grandegger.com>
Cc: U Bhaskar-B22300 <B22300@freescale.com>
Cc: Grant Likely <grant.likely@secretlab.ca>
Cc: socketcan-core@lists.berlios.de
Cc: netdev@vger.kernel.org
Cc: PPC list <linuxppc-dev@lists.ozlabs.org>
Cc: devicetree-discuss@lists.ozlabs.org
---
 drivers/net/can/flexcan.c |   13 ++++++++++++-
 1 files changed, 12 insertions(+), 1 deletions(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 68cbe52..662f832 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -1027,8 +1027,19 @@ static int __devexit flexcan_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static struct of_device_id flexcan_of_match[] = {
+	{
+		.compatible = "fsl,flexcan",
+	},
+	{},
+};
+
 static struct platform_driver flexcan_driver = {
-	.driver.name = DRV_NAME,
+	.driver = {
+		.name = DRV_NAME,
+		.owner = THIS_MODULE,
+		.of_match_table = flexcan_of_match,
+	},
 	.probe = flexcan_probe,
 	.remove = __devexit_p(flexcan_remove),
 };
-- 
1.7.2.1

[PATCH v11 5/6] flexcan: Prefer device tree clock frequency if available.

[Robin Holt]

If our CAN device's device tree node has a clock-frequency property,
then use that value for the can devices clock frequency.  If not, fall
back to asking the platform/mach code for the clock frequency associated
with the flexcan device.

Signed-off-by: Robin Holt <holt@sgi.com>
To: Kumar Gala <galak@kernel.crashing.org>
To: Wolfgang Grandegger <wg@grandegger.com>,
To: Marc Kleine-Budde <mkl@pengutronix.de>,
To: U Bhaskar-B22300 <B22300@freescale.com>
To: Scott Wood <scottwood@freescale.com>
To: Grant Likely <grant.likely@secretlab.ca>
Cc: socketcan-core@lists.berlios.de,
Cc: netdev@vger.kernel.org,
Cc: PPC list <linuxppc-dev@lists.ozlabs.org>
Cc: devicetree-discuss@lists.ozlabs.org
---
 .../devicetree/bindings/net/can/fsl-flexcan.txt    |    2 +
 drivers/net/can/flexcan.c                          |   33 +++++++++++++++-----
 2 files changed, 27 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
index c78dcbb..a4382c7 100644
--- a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
+++ b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
@@ -11,6 +11,7 @@ Required properties:
 
 - reg : Offset and length of the register set for this device
 - interrupts : Interrupt tuple for this device
+- clock-frequency : The oscillator frequency driving the flexcan device
 
 Example:
 
@@ -19,4 +20,5 @@ Example:
           reg = <0x1c000 0x1000>;
           interrupts = <48 0x2>;
           interrupt-parent = <&mpic>;
+          clock-frequency = <0x0bebc1fc>;
   };
diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 662f832..d40c38e 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -33,6 +33,7 @@
 #include <linux/kernel.h>
 #include <linux/list.h>
 #include <linux/module.h>
+#include <linux/of.h>
 #include <linux/platform_device.h>
 
 #define DRV_NAME			"flexcan"
@@ -929,12 +930,26 @@ static int __devinit flexcan_probe(struct platform_device *pdev)
 	void __iomem *base;
 	resource_size_t mem_size;
 	int err, irq;
+	u32 clock_freq = 0;
 
-	clk = clk_get(&pdev->dev, NULL);
-	if (IS_ERR(clk)) {
-		dev_err(&pdev->dev, "no clock defined\n");
-		err = PTR_ERR(clk);
-		goto failed_clock;
+	if (pdev->dev.of_node) {
+		const u32 *clock_freq_p;
+
+		clk = NULL;
+		clock_freq_p = of_get_property(pdev->dev.of_node,
+						"clock-frequency", NULL);
+		if (clock_freq_p)
+			clock_freq = *clock_freq_p;
+	}
+
+	if (!clock_freq) {
+		clk = clk_get(&pdev->dev, NULL);
+		if (IS_ERR(clk)) {
+			dev_err(&pdev->dev, "no clock defined\n");
+			err = PTR_ERR(clk);
+			goto failed_clock;
+		}
+		clock_freq = clk_get_rate(clk);
 	}
 
 	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -967,7 +982,7 @@ static int __devinit flexcan_probe(struct platform_device *pdev)
 	dev->flags |= IFF_ECHO; /* we support local echo in hardware */
 
 	priv = netdev_priv(dev);
-	priv->can.clock.freq = clk_get_rate(clk);
+	priv->can.clock.freq = clock_freq;
 	priv->can.bittiming_const = &flexcan_bittiming_const;
 	priv->can.do_set_mode = flexcan_set_mode;
 	priv->can.do_get_berr_counter = flexcan_get_berr_counter;
@@ -1002,7 +1017,8 @@ static int __devinit flexcan_probe(struct platform_device *pdev)
  failed_map:
 	release_mem_region(mem->start, mem_size);
  failed_get:
-	clk_put(clk);
+	if (clk)
+		clk_put(clk);
  failed_clock:
 	return err;
 }
@@ -1020,7 +1036,8 @@ static int __devexit flexcan_remove(struct platform_device *pdev)
 	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	release_mem_region(mem->start, resource_size(mem));
 
-	clk_put(priv->clk);
+	if (priv->clk)
+		clk_put(priv->clk);
 
 	free_candev(dev);
 
-- 
1.7.2.1

Re: [PATCH v11 5/6] flexcan: Prefer device tree clock frequency if available.

[Marc Kleine-Budde]

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

On 08/11/2011 06:07 PM, Robin Holt wrote:
> If our CAN device's device tree node has a clock-frequency property,
> then use that value for the can devices clock frequency.  If not, fall
> back to asking the platform/mach code for the clock frequency associated
> with the flexcan device.

nitpicking follows inline:

> Signed-off-by: Robin Holt <holt@sgi.com>
> To: Kumar Gala <galak@kernel.crashing.org>
> To: Wolfgang Grandegger <wg@grandegger.com>,
> To: Marc Kleine-Budde <mkl@pengutronix.de>,
> To: U Bhaskar-B22300 <B22300@freescale.com>
> To: Scott Wood <scottwood@freescale.com>
> To: Grant Likely <grant.likely@secretlab.ca>
> Cc: socketcan-core@lists.berlios.de,
> Cc: netdev@vger.kernel.org,
> Cc: PPC list <linuxppc-dev@lists.ozlabs.org>
> Cc: devicetree-discuss@lists.ozlabs.org
> ---
>  .../devicetree/bindings/net/can/fsl-flexcan.txt    |    2 +
>  drivers/net/can/flexcan.c                          |   33 +++++++++++++++-----
>  2 files changed, 27 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> index c78dcbb..a4382c7 100644
> --- a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> +++ b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> @@ -11,6 +11,7 @@ Required properties:
>  
>  - reg : Offset and length of the register set for this device
>  - interrupts : Interrupt tuple for this device
> +- clock-frequency : The oscillator frequency driving the flexcan device
>  
>  Example:
>  
> @@ -19,4 +20,5 @@ Example:
>            reg = <0x1c000 0x1000>;
>            interrupts = <48 0x2>;
>            interrupt-parent = <&mpic>;
> +          clock-frequency = <0x0bebc1fc>;

Does the device tree support dec coded integers? IMHO a frequency is
best expressed in decimal.


>    };
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> index 662f832..d40c38e 100644
> --- a/drivers/net/can/flexcan.c
> +++ b/drivers/net/can/flexcan.c
> @@ -33,6 +33,7 @@
>  #include <linux/kernel.h>
>  #include <linux/list.h>
>  #include <linux/module.h>
> +#include <linux/of.h>
>  #include <linux/platform_device.h>
>  
>  #define DRV_NAME			"flexcan"
> @@ -929,12 +930,26 @@ static int __devinit flexcan_probe(struct platform_device *pdev)
>  	void __iomem *base;
>  	resource_size_t mem_size;
>  	int err, irq;
> +	u32 clock_freq = 0;
>  
> -	clk = clk_get(&pdev->dev, NULL);
> -	if (IS_ERR(clk)) {
> -		dev_err(&pdev->dev, "no clock defined\n");
> -		err = PTR_ERR(clk);
> -		goto failed_clock;
> +	if (pdev->dev.of_node) {
> +		const u32 *clock_freq_p;
> +
> +		clk = NULL;

Hmmm - what about moving the clk = NULL into the definition of clk?

> +		clock_freq_p = of_get_property(pdev->dev.of_node,
> +						"clock-frequency", NULL);
> +		if (clock_freq_p)
> +			clock_freq = *clock_freq_p;
> +	}
> +
> +	if (!clock_freq) {
> +		clk = clk_get(&pdev->dev, NULL);
> +		if (IS_ERR(clk)) {
> +			dev_err(&pdev->dev, "no clock defined\n");
> +			err = PTR_ERR(clk);
> +			goto failed_clock;
> +		}
> +		clock_freq = clk_get_rate(clk);
>  	}
>  
>  	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> @@ -967,7 +982,7 @@ static int __devinit flexcan_probe(struct platform_device *pdev)
>  	dev->flags |= IFF_ECHO; /* we support local echo in hardware */
>  
>  	priv = netdev_priv(dev);
> -	priv->can.clock.freq = clk_get_rate(clk);
> +	priv->can.clock.freq = clock_freq;
>  	priv->can.bittiming_const = &flexcan_bittiming_const;
>  	priv->can.do_set_mode = flexcan_set_mode;
>  	priv->can.do_get_berr_counter = flexcan_get_berr_counter;
> @@ -1002,7 +1017,8 @@ static int __devinit flexcan_probe(struct platform_device *pdev)
>   failed_map:
>  	release_mem_region(mem->start, mem_size);
>   failed_get:
> -	clk_put(clk);
> +	if (clk)
> +		clk_put(clk);
>   failed_clock:
>  	return err;
>  }
> @@ -1020,7 +1036,8 @@ static int __devexit flexcan_remove(struct platform_device *pdev)
>  	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	release_mem_region(mem->start, resource_size(mem));
>  
> -	clk_put(priv->clk);
> +	if (priv->clk)
> +		clk_put(priv->clk);
>  
>  	free_candev(dev);
>  

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 v11 5/6] flexcan: Prefer device tree clock frequency if available.

[Kumar Gala]

On Aug 11, 2011, at 11:18 AM, Marc Kleine-Budde wrote:

> On 08/11/2011 06:07 PM, Robin Holt wrote:
>> If our CAN device's device tree node has a clock-frequency property,
>> then use that value for the can devices clock frequency.  If not, =
fall
>> back to asking the platform/mach code for the clock frequency =
associated
>> with the flexcan device.
>=20
> nitpicking follows inline:
>=20
>> Signed-off-by: Robin Holt <holt@sgi.com>
>> To: Kumar Gala <galak@kernel.crashing.org>
>> To: Wolfgang Grandegger <wg@grandegger.com>,
>> To: Marc Kleine-Budde <mkl@pengutronix.de>,
>> To: U Bhaskar-B22300 <B22300@freescale.com>
>> To: Scott Wood <scottwood@freescale.com>
>> To: Grant Likely <grant.likely@secretlab.ca>
>> Cc: socketcan-core@lists.berlios.de,
>> Cc: netdev@vger.kernel.org,
>> Cc: PPC list <linuxppc-dev@lists.ozlabs.org>
>> Cc: devicetree-discuss@lists.ozlabs.org
>> ---
>> .../devicetree/bindings/net/can/fsl-flexcan.txt    |    2 +
>> drivers/net/can/flexcan.c                          |   33 =
+++++++++++++++-----
>> 2 files changed, 27 insertions(+), 8 deletions(-)
>>=20
>> diff --git =
a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt =
b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
>> index c78dcbb..a4382c7 100644
>> --- a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
>> +++ b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
>> @@ -11,6 +11,7 @@ Required properties:
>>=20
>> - reg : Offset and length of the register set for this device
>> - interrupts : Interrupt tuple for this device
>> +- clock-frequency : The oscillator frequency driving the flexcan =
device
>>=20
>> Example:
>>=20
>> @@ -19,4 +20,5 @@ Example:
>>           reg =3D <0x1c000 0x1000>;
>>           interrupts =3D <48 0x2>;
>>           interrupt-parent =3D <&mpic>;
>> +          clock-frequency =3D <0x0bebc1fc>;
>=20
> Does the device tree support dec coded integers? IMHO a frequency is
> best expressed in decimal.

Yes it does, and agree that in the example a dec # might be better

- k=

Re: [PATCH v11 5/6] flexcan: Prefer device tree clock frequency if available.

[Wolfgang Grandegger]

On 08/11/2011 06:07 PM, Robin Holt wrote:
> If our CAN device's device tree node has a clock-frequency property,
> then use that value for the can devices clock frequency.  If not, fall
> back to asking the platform/mach code for the clock frequency associated
> with the flexcan device.
> 
> Signed-off-by: Robin Holt <holt@sgi.com>
> To: Kumar Gala <galak@kernel.crashing.org>
> To: Wolfgang Grandegger <wg@grandegger.com>,
> To: Marc Kleine-Budde <mkl@pengutronix.de>,
> To: U Bhaskar-B22300 <B22300@freescale.com>
> To: Scott Wood <scottwood@freescale.com>
> To: Grant Likely <grant.likely@secretlab.ca>
> Cc: socketcan-core@lists.berlios.de,
> Cc: netdev@vger.kernel.org,
> Cc: PPC list <linuxppc-dev@lists.ozlabs.org>
> Cc: devicetree-discuss@lists.ozlabs.org
> ---
>  .../devicetree/bindings/net/can/fsl-flexcan.txt    |    2 +
>  drivers/net/can/flexcan.c                          |   33 +++++++++++++++-----
>  2 files changed, 27 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> index c78dcbb..a4382c7 100644
> --- a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> +++ b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> @@ -11,6 +11,7 @@ Required properties:
>  
>  - reg : Offset and length of the register set for this device
>  - interrupts : Interrupt tuple for this device
> +- clock-frequency : The oscillator frequency driving the flexcan device
>  
>  Example:
>  
> @@ -19,4 +20,5 @@ Example:
>            reg = <0x1c000 0x1000>;
>            interrupts = <48 0x2>;
>            interrupt-parent = <&mpic>;
> +          clock-frequency = <0x0bebc1fc>;

             clock-frequency = <200000000>; // filled in by bootloader

Is better readable. You should also add the comment.
	
Wolfgang.

[PATCH v11 6/6] powerpc: Add flexcan device support for p1010rdb.

[Robin Holt]

Allow the p1010 processor to select the flexcan network driver.

Signed-off-by: Robin Holt <holt@sgi.com>
Acked-by: Marc Kleine-Budde <mkl@pengutronix.de>,
Acked-by: Wolfgang Grandegger <wg@grandegger.com>,
Cc: U Bhaskar-B22300 <B22300@freescale.com>
Cc: socketcan-core@lists.berlios.de,
Cc: netdev@vger.kernel.org,
Cc: PPC list <linuxppc-dev@lists.ozlabs.org>
Cc: Kumar Gala <galak@kernel.crashing.org>
---
 arch/powerpc/boot/dts/p1010rdb.dts  |    8 ++++++++
 arch/powerpc/platforms/85xx/Kconfig |    2 ++
 2 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/boot/dts/p1010rdb.dts b/arch/powerpc/boot/dts/p1010rdb.dts
index d6c669c..df89b60 100644
--- a/arch/powerpc/boot/dts/p1010rdb.dts
+++ b/arch/powerpc/boot/dts/p1010rdb.dts
@@ -171,6 +171,14 @@
 			};
 		};
 
+		can@1c000 {
+			clock-frequency = <0x0bebc1fc>;
+		};
+
+		can1: can@1d000 {
+			clock-frequency = <0x0bebc1fc>;
+		};
+
 		usb@22000 {
 			phy_type = "utmi";
 		};
diff --git a/arch/powerpc/platforms/85xx/Kconfig b/arch/powerpc/platforms/85xx/Kconfig
index 498534c..c4304ae 100644
--- a/arch/powerpc/platforms/85xx/Kconfig
+++ b/arch/powerpc/platforms/85xx/Kconfig
@@ -70,6 +70,8 @@ config MPC85xx_RDB
 config P1010_RDB
 	bool "Freescale P1010RDB"
 	select DEFAULT_UIMAGE
+	select HAVE_CAN_FLEXCAN if NET && CAN
+	select PPC_CLOCK if CAN_FLEXCAN
 	help
 	  This option enables support for the MPC85xx RDB (P1010 RDB) board
 
-- 
1.7.2.1

Re: [PATCH v11 6/6] powerpc: Add flexcan device support for p1010rdb.

[Wolfgang Grandegger]

On 08/11/2011 06:07 PM, Robin Holt wrote:
> Allow the p1010 processor to select the flexcan network driver.
> 
> Signed-off-by: Robin Holt <holt@sgi.com>
> Acked-by: Marc Kleine-Budde <mkl@pengutronix.de>,
> Acked-by: Wolfgang Grandegger <wg@grandegger.com>,
> Cc: U Bhaskar-B22300 <B22300@freescale.com>
> Cc: socketcan-core@lists.berlios.de,
> Cc: netdev@vger.kernel.org,
> Cc: PPC list <linuxppc-dev@lists.ozlabs.org>
> Cc: Kumar Gala <galak@kernel.crashing.org>
> ---
>  arch/powerpc/boot/dts/p1010rdb.dts  |    8 ++++++++
>  arch/powerpc/platforms/85xx/Kconfig |    2 ++
>  2 files changed, 10 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/powerpc/boot/dts/p1010rdb.dts b/arch/powerpc/boot/dts/p1010rdb.dts
> index d6c669c..df89b60 100644
> --- a/arch/powerpc/boot/dts/p1010rdb.dts
> +++ b/arch/powerpc/boot/dts/p1010rdb.dts
> @@ -171,6 +171,14 @@
>  			};
>  		};
>  
> +		can@1c000 {
> +			clock-frequency = <0x0bebc1fc>;
> +		};


	                clock-frequency = <200000000>; // filled in by boot-loader

Is better readable and makes clear that the value is filled in by the
boot loader. Usually we specify "<0>" in that case but it will *not*
work with your board because U-Boot fills into the property "clock_freq"
the CCB-frequency, which is twice as much. This needs to be fixed.

> +		can1: can@1d000 {

What is the "can1:" good for?

> +			clock-frequency = <0x0bebc1fc>;

See above.

Wolfgang.




>  		usb@22000 {
>  			phy_type = "utmi";
>  		};
> diff --git a/arch/powerpc/platforms/85xx/Kconfig b/arch/powerpc/platforms/85xx/Kconfig
> index 498534c..c4304ae 100644
> --- a/arch/powerpc/platforms/85xx/Kconfig
> +++ b/arch/powerpc/platforms/85xx/Kconfig
> @@ -70,6 +70,8 @@ config MPC85xx_RDB
>  config P1010_RDB
>  	bool "Freescale P1010RDB"
>  	select DEFAULT_UIMAGE
> +	select HAVE_CAN_FLEXCAN if NET && CAN
> +	select PPC_CLOCK if CAN_FLEXCAN
>  	help
>  	  This option enables support for the MPC85xx RDB (P1010 RDB) board
>  

Re: [PATCH v11 6/6] powerpc: Add flexcan device support for p1010rdb.

[Robin Holt]

On Thu, Aug 11, 2011 at 06:35:08PM +0200, Wolfgang Grandegger wrote:
> On 08/11/2011 06:07 PM, Robin Holt wrote:
> > Allow the p1010 processor to select the flexcan network driver.
> > 
> > Signed-off-by: Robin Holt <holt@sgi.com>
> > Acked-by: Marc Kleine-Budde <mkl@pengutronix.de>,
> > Acked-by: Wolfgang Grandegger <wg@grandegger.com>,
> > Cc: U Bhaskar-B22300 <B22300@freescale.com>
> > Cc: socketcan-core@lists.berlios.de,
> > Cc: netdev@vger.kernel.org,
> > Cc: PPC list <linuxppc-dev@lists.ozlabs.org>
> > Cc: Kumar Gala <galak@kernel.crashing.org>
> > ---
> >  arch/powerpc/boot/dts/p1010rdb.dts  |    8 ++++++++
> >  arch/powerpc/platforms/85xx/Kconfig |    2 ++
> >  2 files changed, 10 insertions(+), 0 deletions(-)
> > 
> > diff --git a/arch/powerpc/boot/dts/p1010rdb.dts b/arch/powerpc/boot/dts/p1010rdb.dts
> > index d6c669c..df89b60 100644
> > --- a/arch/powerpc/boot/dts/p1010rdb.dts
> > +++ b/arch/powerpc/boot/dts/p1010rdb.dts
> > @@ -171,6 +171,14 @@
> >  			};
> >  		};
> >  
> > +		can@1c000 {
> > +			clock-frequency = <0x0bebc1fc>;
> > +		};
> 
> 
> 	                clock-frequency = <200000000>; // filled in by boot-loader
> 
> Is better readable and makes clear that the value is filled in by the
> boot loader. Usually we specify "<0>" in that case but it will *not*
> work with your board because U-Boot fills into the property "clock_freq"
> the CCB-frequency, which is twice as much. This needs to be fixed.

I guess the question then is, should I not add the clock-frequency to
this file and let this fall back upon the boot-loader?  Until the boot
loader is up-to-speed, my test .dts file will need to define it.

Thanks,
Robin

> 
> > +		can1: can@1d000 {
> 
> What is the "can1:" good for?
> 
> > +			clock-frequency = <0x0bebc1fc>;
> 
> See above.
> 
> Wolfgang.
> 
> 
> 
> 
> >  		usb@22000 {
> >  			phy_type = "utmi";
> >  		};
> > diff --git a/arch/powerpc/platforms/85xx/Kconfig b/arch/powerpc/platforms/85xx/Kconfig
> > index 498534c..c4304ae 100644
> > --- a/arch/powerpc/platforms/85xx/Kconfig
> > +++ b/arch/powerpc/platforms/85xx/Kconfig
> > @@ -70,6 +70,8 @@ config MPC85xx_RDB
> >  config P1010_RDB
> >  	bool "Freescale P1010RDB"
> >  	select DEFAULT_UIMAGE
> > +	select HAVE_CAN_FLEXCAN if NET && CAN
> > +	select PPC_CLOCK if CAN_FLEXCAN
> >  	help
> >  	  This option enables support for the MPC85xx RDB (P1010 RDB) board
> >  

Re: [PATCH v11 6/6] powerpc: Add flexcan device support for p1010rdb.

[Kumar Gala]

On Aug 11, 2011, at 11:07 AM, Robin Holt wrote:

> Allow the p1010 processor to select the flexcan network driver.
>=20
> Signed-off-by: Robin Holt <holt@sgi.com>
> Acked-by: Marc Kleine-Budde <mkl@pengutronix.de>,
> Acked-by: Wolfgang Grandegger <wg@grandegger.com>,
> Cc: U Bhaskar-B22300 <B22300@freescale.com>
> Cc: socketcan-core@lists.berlios.de,
> Cc: netdev@vger.kernel.org,
> Cc: PPC list <linuxppc-dev@lists.ozlabs.org>
> Cc: Kumar Gala <galak@kernel.crashing.org>
> ---
> arch/powerpc/boot/dts/p1010rdb.dts  |    8 ++++++++
> arch/powerpc/platforms/85xx/Kconfig |    2 ++
> 2 files changed, 10 insertions(+), 0 deletions(-)
>=20
> diff --git a/arch/powerpc/boot/dts/p1010rdb.dts =
b/arch/powerpc/boot/dts/p1010rdb.dts
> index d6c669c..df89b60 100644
> --- a/arch/powerpc/boot/dts/p1010rdb.dts
> +++ b/arch/powerpc/boot/dts/p1010rdb.dts
> @@ -171,6 +171,14 @@
> 			};
> 		};
>=20
> +		can@1c000 {
> +			clock-frequency =3D <0x0bebc1fc>;
> +		};
> +
> +		can1: can@1d000 {
> +			clock-frequency =3D <0x0bebc1fc>;
> +		};
> +

set them to 0, as we expect u-boot to fill them in.

> 		usb@22000 {
> 			phy_type =3D "utmi";
> 		};
> diff --git a/arch/powerpc/platforms/85xx/Kconfig =
b/arch/powerpc/platforms/85xx/Kconfig
> index 498534c..c4304ae 100644
> --- a/arch/powerpc/platforms/85xx/Kconfig
> +++ b/arch/powerpc/platforms/85xx/Kconfig
> @@ -70,6 +70,8 @@ config MPC85xx_RDB
> config P1010_RDB
> 	bool "Freescale P1010RDB"
> 	select DEFAULT_UIMAGE
> +	select HAVE_CAN_FLEXCAN if NET && CAN
> +	select PPC_CLOCK if CAN_FLEXCAN

Can you move this to arch/powerpc/Kconfig & FSL_SOC instead.

> 	help
> 	  This option enables support for the MPC85xx RDB (P1010 RDB) =
board
>=20
> --=20
> 1.7.2.1

Re: [PATCH v11 6/6] powerpc: Add flexcan device support for p1010rdb.

[Robin Holt]

On Thu, Aug 11, 2011 at 12:41:34PM -0500, Kumar Gala wrote:
> 
> On Aug 11, 2011, at 11:07 AM, Robin Holt wrote:
> 
> > Allow the p1010 processor to select the flexcan network driver.
> > 
> > Signed-off-by: Robin Holt <holt@sgi.com>
> > Acked-by: Marc Kleine-Budde <mkl@pengutronix.de>,
> > Acked-by: Wolfgang Grandegger <wg@grandegger.com>,
> > Cc: U Bhaskar-B22300 <B22300@freescale.com>
> > Cc: socketcan-core@lists.berlios.de,
> > Cc: netdev@vger.kernel.org,
> > Cc: PPC list <linuxppc-dev@lists.ozlabs.org>
> > Cc: Kumar Gala <galak@kernel.crashing.org>
> > ---
> > arch/powerpc/boot/dts/p1010rdb.dts  |    8 ++++++++
> > arch/powerpc/platforms/85xx/Kconfig |    2 ++
> > 2 files changed, 10 insertions(+), 0 deletions(-)
> > 
> > diff --git a/arch/powerpc/boot/dts/p1010rdb.dts b/arch/powerpc/boot/dts/p1010rdb.dts
> > index d6c669c..df89b60 100644
> > --- a/arch/powerpc/boot/dts/p1010rdb.dts
> > +++ b/arch/powerpc/boot/dts/p1010rdb.dts
> > @@ -171,6 +171,14 @@
> > 			};
> > 		};
> > 
> > +		can@1c000 {
> > +			clock-frequency = <0x0bebc1fc>;
> > +		};
> > +
> > +		can1: can@1d000 {
> > +			clock-frequency = <0x0bebc1fc>;
> > +		};
> > +
> 
> set them to 0, as we expect u-boot to fill them in.

Done.

> 
> > 		usb@22000 {
> > 			phy_type = "utmi";
> > 		};
> > diff --git a/arch/powerpc/platforms/85xx/Kconfig b/arch/powerpc/platforms/85xx/Kconfig
> > index 498534c..c4304ae 100644
> > --- a/arch/powerpc/platforms/85xx/Kconfig
> > +++ b/arch/powerpc/platforms/85xx/Kconfig
> > @@ -70,6 +70,8 @@ config MPC85xx_RDB
> > config P1010_RDB
> > 	bool "Freescale P1010RDB"
> > 	select DEFAULT_UIMAGE
> > +	select HAVE_CAN_FLEXCAN if NET && CAN
> > +	select PPC_CLOCK if CAN_FLEXCAN
> 
> Can you move this to arch/powerpc/Kconfig & FSL_SOC instead.

I am not sure.  FSL_SOC seems to come with any of the freescale system
on a chip.  I would not be that worried, about the flexcan build as
I think that is sufficiently agostic where we will not see problems,
but now we could end up with build failures on any of the other configs
which select CAN_FLEXCAN.  I would normally want to do all those builds,
but there is no way I would know how to do that with my limited knowledge
of powerpc and freescale.

If you are comfortable with that, I will happily make the change.

Thanks,
Robin