[RFC 0/4] [flexcan] Add support for powerpc (freescale p1010) -V4

[RFC 0/4] [flexcan] Add support for powerpc (freescale p1010) -V4

[Robin Holt]

Marc or Wolfgang,

This patch set should have all your comments included.

Could you please apply these four patches to a test branch, compile
and test them on an arm based system?  I would like to at least feel
comfortable that I have not broken anything there so far.

Before the integration of the comments, I had verified the flexcan
driver was able to communicate.  I still need to do more testing, but
it certainly looks very promising at this point.

Thanks,
Robin Holt

[RFC 1/4] [flexcan] Abstract off read/write for big/little endian.

[Robin Holt]

First step in converting the flexcan driver from supporting just arm to
supporting both arm and powerpc architectures.

Signed-off-by: Robin Holt <holt@sgi.com>
To: Marc Kleine-Budde <mkl@pengutronix.de>
To: Wolfgang Grandegger <wg@grandegger.com>
Cc: socketcan-core@lists.berlios.de
Cc: netdev@vger.kernel.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 67d9fc0..74b1706 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -196,6 +196,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)
@@ -216,9 +241,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);
 }
@@ -228,9 +253,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,
@@ -238,7 +263,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;
@@ -272,15 +297,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);
 
@@ -468,8 +493,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
@@ -479,12 +504,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)
@@ -520,17 +545,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 */
@@ -540,8 +565,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;
@@ -555,9 +580,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:
@@ -573,16 +598,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++;
 	}
@@ -591,7 +616,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);
 	}
 
@@ -605,7 +630,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) |
@@ -629,11 +654,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));
 }
 
 /*
@@ -654,10 +679,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",
@@ -679,12 +704,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
@@ -702,7 +727,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;
@@ -710,38 +735,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;
 
@@ -763,12 +789,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;
@@ -860,24 +886,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

Re: [RFC 1/4] [flexcan] Abstract off read/write for big/little endian.

[Marc Kleine-Budde]

[-- Attachment #1.1: Type: text/plain, Size: 858 bytes --]

On 08/05/2011 04:06 AM, Robin Holt wrote:
> First step in converting the flexcan driver from supporting just arm to
> supporting both arm and powerpc architectures.
> 
> Signed-off-by: Robin Holt <holt-sJ/iWh9BUns@public.gmane.org>
> To: Marc Kleine-Budde <mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> To: Wolfgang Grandegger <wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>
> Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org
> Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org

Acked-by: Marc Kleine-Budde <mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

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 #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

[-- Attachment #2: Type: text/plain, Size: 188 bytes --]

_______________________________________________
Socketcan-core mailing list
Socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org
https://lists.berlios.de/mailman/listinfo/socketcan-core

[RFC 2/4] [flexcan] Introduce a flexcan_clk set of functions.

[Robin Holt]

The freescale P1010RDB board does not have a
clk_{get,put,get_rate,enable,disable} set of functions.  Wrap these with a
flexcan_ #define for arm, and implement a more complete function for ppc.

Signed-off-by: Robin Holt <holt@sgi.com>
To: Marc Kleine-Budde <mkl@pengutronix.de>
To: Wolfgang Grandegger <wg@grandegger.com>
Cc: socketcan-core@lists.berlios.de
Cc: netdev@vger.kernel.org
---
 drivers/net/can/flexcan.c |  114 +++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 105 insertions(+), 9 deletions(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 74b1706..3417d0b 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -220,6 +220,102 @@ static inline void flexcan_write(u32 val, void __iomem *addr)
 }
 #endif
 
+#if defined(__powerpc__)
+struct flexcan_clk {
+	unsigned long rate;
+	void *data;
+};
+
+static struct clk *flexcan_clk_get(struct device *dev, const char *id)
+{
+	struct flexcan_clk *clock;
+	u32 *clock_freq;
+	u32 *clock_divider;
+	int err;
+
+	clock = kmalloc(sizeof(struct flexcan_clk), GFP_KERNEL);
+	if (!clock) {
+		dev_err(dev, "Cannot allocate memory\n");
+		err = -ENOMEM;
+		goto failed_clock;
+	}
+	clock_freq = (u32 *)of_get_property(dev->of_node, "clock_freq", NULL);
+	if (!clock_freq) {
+		dev_err(dev, "Cannot find clock_freq property\n");
+		err = -EINVAL;
+		goto failed_clock;
+	}
+
+	clock_divider = (u32 *) of_get_property(dev->of_node,
+					"fsl,flexcan-clock-divider", NULL);
+	if (clock_divider == NULL) {
+		dev_err(dev, "Cannot find fsl,flexcan-clock-divider property\n");
+		err = -EINVAL;
+		goto failed_clock;
+	}
+
+	clock->rate = DIV_ROUND_CLOSEST(*clock_freq / *clock_divider, 1000);
+	clock->rate *= 1000;
+
+	return (struct clk *)clock;
+
+ failed_clock:
+	kfree(clock);
+	return ERR_PTR(err);
+}
+
+static inline void flexcan_clk_put(struct clk *_clk)
+{
+	struct flexcan_clk *clk = (struct flexcan_clk *)_clk;
+
+	kfree(clk);
+}
+
+static inline int flexcan_clk_enable(struct clk *clk)
+{
+	return 0;
+}
+
+static inline void flexcan_clk_disable(struct clk *clk)
+{
+	return;
+}
+
+static inline unsigned long flexcan_clk_get_rate(struct clk *_clk)
+{
+	struct flexcan_clk *clk = (struct flexcan_clk *)_clk;
+
+	return clk->rate;
+}
+
+#else
+static inline struct clk *flexcan_clk_get(struct device *dev, const char *id)
+{
+	return clk_get(dev, id);
+}
+
+static inline void flexcan_clk_put(struct clk *clk)
+{
+	clk_put(clk);
+}
+
+static inline int flexcan_clk_enable(struct clk *clk)
+{
+	return clk_enable(clk);
+}
+
+static inline void flexcan_clk_disable(struct clk *clk)
+{
+	clk_disable(clk);
+}
+
+static inline unsigned long flexcan_clk_get_rate(struct clk *clk)
+{
+	return clk_get_rate(clk);
+}
+
+#endif
+
 /*
  * Swtich transceiver on or off
  */
@@ -807,7 +903,7 @@ static int flexcan_open(struct net_device *dev)
 	struct flexcan_priv *priv = netdev_priv(dev);
 	int err;
 
-	clk_enable(priv->clk);
+	flexcan_clk_enable(priv->clk);
 
 	err = open_candev(dev);
 	if (err)
@@ -829,7 +925,7 @@ static int flexcan_open(struct net_device *dev)
  out_close:
 	close_candev(dev);
  out:
-	clk_disable(priv->clk);
+	flexcan_clk_disable(priv->clk);
 
 	return err;
 }
@@ -843,7 +939,7 @@ static int flexcan_close(struct net_device *dev)
 	flexcan_chip_stop(dev);
 
 	free_irq(dev->irq, dev);
-	clk_disable(priv->clk);
+	flexcan_clk_disable(priv->clk);
 
 	close_candev(dev);
 
@@ -882,7 +978,7 @@ static int __devinit register_flexcandev(struct net_device *dev)
 	struct flexcan_regs __iomem *regs = priv->base;
 	u32 reg, err;
 
-	clk_enable(priv->clk);
+	flexcan_clk_enable(priv->clk);
 
 	/* select "bus clock", chip must be disabled */
 	flexcan_chip_disable(priv);
@@ -916,7 +1012,7 @@ static int __devinit register_flexcandev(struct net_device *dev)
  out:
 	/* disable core and turn off clocks */
 	flexcan_chip_disable(priv);
-	clk_disable(priv->clk);
+	flexcan_clk_disable(priv->clk);
 
 	return err;
 }
@@ -936,7 +1032,7 @@ static int __devinit flexcan_probe(struct platform_device *pdev)
 	resource_size_t mem_size;
 	int err, irq;
 
-	clk = clk_get(&pdev->dev, NULL);
+	clk = flexcan_clk_get(&pdev->dev, NULL);
 	if (IS_ERR(clk)) {
 		dev_err(&pdev->dev, "no clock defined\n");
 		err = PTR_ERR(clk);
@@ -973,7 +1069,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 = flexcan_clk_get_rate(clk);
 	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;
@@ -1008,7 +1104,7 @@ static int __devinit flexcan_probe(struct platform_device *pdev)
  failed_map:
 	release_mem_region(mem->start, mem_size);
  failed_get:
-	clk_put(clk);
+	flexcan_clk_put(clk);
  failed_clock:
 	return err;
 }
@@ -1026,7 +1122,7 @@ 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);
+	flexcan_clk_put(priv->clk);
 
 	free_candev(dev);
 
-- 
1.7.2.1

Re: [RFC 2/4] [flexcan] Introduce a flexcan_clk set of functions.

[Marc Kleine-Budde]

[-- Attachment #1.1: Type: text/plain, Size: 6442 bytes --]

On 08/05/2011 04:06 AM, Robin Holt wrote:
> The freescale P1010RDB board does not have a
> clk_{get,put,get_rate,enable,disable} set of functions.  Wrap these with a
> flexcan_ #define for arm, and implement a more complete function for ppc.

Some codingstyle nitpicks inline. I hope we'll find a cleaner solution
than this patch.

Marc

> Signed-off-by: Robin Holt <holt-sJ/iWh9BUns@public.gmane.org>
> To: Marc Kleine-Budde <mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> To: Wolfgang Grandegger <wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>
> Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org
> Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> ---
>  drivers/net/can/flexcan.c |  114 +++++++++++++++++++++++++++++++++++++++++----
>  1 files changed, 105 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> index 74b1706..3417d0b 100644
> --- a/drivers/net/can/flexcan.c
> +++ b/drivers/net/can/flexcan.c
> @@ -220,6 +220,102 @@ static inline void flexcan_write(u32 val, void __iomem *addr)
>  }
>  #endif
>  
> +#if defined(__powerpc__)
> +struct flexcan_clk {
> +	unsigned long rate;
> +	void *data;
> +};
> +
> +static struct clk *flexcan_clk_get(struct device *dev, const char *id)
> +{
> +	struct flexcan_clk *clock;
> +	u32 *clock_freq;
> +	u32 *clock_divider;
> +	int err;
> +
> +	clock = kmalloc(sizeof(struct flexcan_clk), GFP_KERNEL);
> +	if (!clock) {
> +		dev_err(dev, "Cannot allocate memory\n");
> +		err = -ENOMEM;
> +		goto failed_clock;
> +	}
> +	clock_freq = (u32 *)of_get_property(dev->of_node, "clock_freq", NULL);
> +	if (!clock_freq) {
> +		dev_err(dev, "Cannot find clock_freq property\n");
> +		err = -EINVAL;
> +		goto failed_clock;
> +	}
> +
> +	clock_divider = (u32 *) of_get_property(dev->of_node,
                               ^

remove this space, please
> +					"fsl,flexcan-clock-divider", NULL);
> +	if (clock_divider == NULL) {

!clock_divider

> +		dev_err(dev, "Cannot find fsl,flexcan-clock-divider property\n");
> +		err = -EINVAL;
> +		goto failed_clock;
> +	}
> +
> +	clock->rate = DIV_ROUND_CLOSEST(*clock_freq / *clock_divider, 1000);
> +	clock->rate *= 1000;
> +
> +	return (struct clk *)clock;
> +
> + failed_clock:
> +	kfree(clock);
> +	return ERR_PTR(err);
> +}
> +
> +static inline void flexcan_clk_put(struct clk *_clk)
> +{
> +	struct flexcan_clk *clk = (struct flexcan_clk *)_clk;

that cast is not needed.

> +
> +	kfree(clk);
> +}
> +
> +static inline int flexcan_clk_enable(struct clk *clk)
> +{
> +	return 0;
> +}
> +
> +static inline void flexcan_clk_disable(struct clk *clk)
> +{
> +	return;
> +}
> +
> +static inline unsigned long flexcan_clk_get_rate(struct clk *_clk)
> +{
> +	struct flexcan_clk *clk = (struct flexcan_clk *)_clk;
> +
> +	return clk->rate;
> +}
> +
> +#else
> +static inline struct clk *flexcan_clk_get(struct device *dev, const char *id)
> +{
> +	return clk_get(dev, id);
> +}
> +
> +static inline void flexcan_clk_put(struct clk *clk)
> +{
> +	clk_put(clk);
> +}
> +
> +static inline int flexcan_clk_enable(struct clk *clk)
> +{
> +	return clk_enable(clk);
> +}
> +
> +static inline void flexcan_clk_disable(struct clk *clk)
> +{
> +	clk_disable(clk);
> +}
> +
> +static inline unsigned long flexcan_clk_get_rate(struct clk *clk)
> +{
> +	return clk_get_rate(clk);
> +}
> +
> +#endif
> +
>  /*
>   * Swtich transceiver on or off
>   */
> @@ -807,7 +903,7 @@ static int flexcan_open(struct net_device *dev)
>  	struct flexcan_priv *priv = netdev_priv(dev);
>  	int err;
>  
> -	clk_enable(priv->clk);
> +	flexcan_clk_enable(priv->clk);
>  
>  	err = open_candev(dev);
>  	if (err)
> @@ -829,7 +925,7 @@ static int flexcan_open(struct net_device *dev)
>   out_close:
>  	close_candev(dev);
>   out:
> -	clk_disable(priv->clk);
> +	flexcan_clk_disable(priv->clk);
>  
>  	return err;
>  }
> @@ -843,7 +939,7 @@ static int flexcan_close(struct net_device *dev)
>  	flexcan_chip_stop(dev);
>  
>  	free_irq(dev->irq, dev);
> -	clk_disable(priv->clk);
> +	flexcan_clk_disable(priv->clk);
>  
>  	close_candev(dev);
>  
> @@ -882,7 +978,7 @@ static int __devinit register_flexcandev(struct net_device *dev)
>  	struct flexcan_regs __iomem *regs = priv->base;
>  	u32 reg, err;
>  
> -	clk_enable(priv->clk);
> +	flexcan_clk_enable(priv->clk);
>  
>  	/* select "bus clock", chip must be disabled */
>  	flexcan_chip_disable(priv);
> @@ -916,7 +1012,7 @@ static int __devinit register_flexcandev(struct net_device *dev)
>   out:
>  	/* disable core and turn off clocks */
>  	flexcan_chip_disable(priv);
> -	clk_disable(priv->clk);
> +	flexcan_clk_disable(priv->clk);
>  
>  	return err;
>  }
> @@ -936,7 +1032,7 @@ static int __devinit flexcan_probe(struct platform_device *pdev)
>  	resource_size_t mem_size;
>  	int err, irq;
>  
> -	clk = clk_get(&pdev->dev, NULL);
> +	clk = flexcan_clk_get(&pdev->dev, NULL);
>  	if (IS_ERR(clk)) {
>  		dev_err(&pdev->dev, "no clock defined\n");
>  		err = PTR_ERR(clk);
> @@ -973,7 +1069,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 = flexcan_clk_get_rate(clk);
>  	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;
> @@ -1008,7 +1104,7 @@ static int __devinit flexcan_probe(struct platform_device *pdev)
>   failed_map:
>  	release_mem_region(mem->start, mem_size);
>   failed_get:
> -	clk_put(clk);
> +	flexcan_clk_put(clk);
>   failed_clock:
>  	return err;
>  }
> @@ -1026,7 +1122,7 @@ 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);
> +	flexcan_clk_put(priv->clk);
>  
>  	free_candev(dev);
>  


-- 
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 #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

[-- Attachment #2: Type: text/plain, Size: 188 bytes --]

_______________________________________________
Socketcan-core mailing list
Socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org
https://lists.berlios.de/mailman/listinfo/socketcan-core

Re: [RFC 2/4] [flexcan] Introduce a flexcan_clk set of functions.

[Robin Holt]

I implemented the coding style changes below (Sorry I missed the two
the first time).

As for a better implementation, I guess I would need to understand what
is being attempted here.  I only marginally understand the flexcan
hardware on the Freescale P1010 and certainly am clueless about arm
implementations of flexcan.  I just skimmed over freescale's site
and it looks like I would be looking at their i.MX25, i.MX28, i.MX35,
and i.MX53 family of processors.  I will attempt to find some useful
documentation of those and look at the kernel sources for what the clk_*
functions are trying to accomplish.

I _THINK_ I understand.  It looks like I could implement this as a powerpc
p1010 specific thing and get the same effect without impacting flexcan.c.
I also think I understand that the i.MX25 family of processors do
essentially the same thing the p1010 is doing for determining the
clock rate.

Thanks,
Robin

On Fri, Aug 05, 2011 at 10:34:11AM +0200, Marc Kleine-Budde wrote:
> On 08/05/2011 04:06 AM, Robin Holt wrote:
> > The freescale P1010RDB board does not have a
> > clk_{get,put,get_rate,enable,disable} set of functions.  Wrap these with a
> > flexcan_ #define for arm, and implement a more complete function for ppc.
> 
> Some codingstyle nitpicks inline. I hope we'll find a cleaner solution
> than this patch.
> 
> Marc
> 
> > Signed-off-by: Robin Holt <holt@sgi.com>
> > To: Marc Kleine-Budde <mkl@pengutronix.de>
> > To: Wolfgang Grandegger <wg@grandegger.com>
> > Cc: socketcan-core@lists.berlios.de
> > Cc: netdev@vger.kernel.org
> > ---
> >  drivers/net/can/flexcan.c |  114 +++++++++++++++++++++++++++++++++++++++++----
> >  1 files changed, 105 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> > index 74b1706..3417d0b 100644
> > --- a/drivers/net/can/flexcan.c
> > +++ b/drivers/net/can/flexcan.c
> > @@ -220,6 +220,102 @@ static inline void flexcan_write(u32 val, void __iomem *addr)
> >  }
> >  #endif
> >  
> > +#if defined(__powerpc__)
> > +struct flexcan_clk {
> > +	unsigned long rate;
> > +	void *data;
> > +};
> > +
> > +static struct clk *flexcan_clk_get(struct device *dev, const char *id)
> > +{
> > +	struct flexcan_clk *clock;
> > +	u32 *clock_freq;
> > +	u32 *clock_divider;
> > +	int err;
> > +
> > +	clock = kmalloc(sizeof(struct flexcan_clk), GFP_KERNEL);
> > +	if (!clock) {
> > +		dev_err(dev, "Cannot allocate memory\n");
> > +		err = -ENOMEM;
> > +		goto failed_clock;
> > +	}
> > +	clock_freq = (u32 *)of_get_property(dev->of_node, "clock_freq", NULL);
> > +	if (!clock_freq) {
> > +		dev_err(dev, "Cannot find clock_freq property\n");
> > +		err = -EINVAL;
> > +		goto failed_clock;
> > +	}
> > +
> > +	clock_divider = (u32 *) of_get_property(dev->of_node,
>                                ^
> 
> remove this space, please
> > +					"fsl,flexcan-clock-divider", NULL);
> > +	if (clock_divider == NULL) {
> 
> !clock_divider
> 
> > +		dev_err(dev, "Cannot find fsl,flexcan-clock-divider property\n");
> > +		err = -EINVAL;
> > +		goto failed_clock;
> > +	}
> > +
> > +	clock->rate = DIV_ROUND_CLOSEST(*clock_freq / *clock_divider, 1000);
> > +	clock->rate *= 1000;
> > +
> > +	return (struct clk *)clock;
> > +
> > + failed_clock:
> > +	kfree(clock);
> > +	return ERR_PTR(err);
> > +}
> > +
> > +static inline void flexcan_clk_put(struct clk *_clk)
> > +{
> > +	struct flexcan_clk *clk = (struct flexcan_clk *)_clk;
> 
> that cast is not needed.
> 
> > +
> > +	kfree(clk);
> > +}
> > +
> > +static inline int flexcan_clk_enable(struct clk *clk)
> > +{
> > +	return 0;
> > +}
> > +
> > +static inline void flexcan_clk_disable(struct clk *clk)
> > +{
> > +	return;
> > +}
> > +
> > +static inline unsigned long flexcan_clk_get_rate(struct clk *_clk)
> > +{
> > +	struct flexcan_clk *clk = (struct flexcan_clk *)_clk;
> > +
> > +	return clk->rate;
> > +}
> > +
> > +#else
> > +static inline struct clk *flexcan_clk_get(struct device *dev, const char *id)
> > +{
> > +	return clk_get(dev, id);
> > +}
> > +
> > +static inline void flexcan_clk_put(struct clk *clk)
> > +{
> > +	clk_put(clk);
> > +}
> > +
> > +static inline int flexcan_clk_enable(struct clk *clk)
> > +{
> > +	return clk_enable(clk);
> > +}
> > +
> > +static inline void flexcan_clk_disable(struct clk *clk)
> > +{
> > +	clk_disable(clk);
> > +}
> > +
> > +static inline unsigned long flexcan_clk_get_rate(struct clk *clk)
> > +{
> > +	return clk_get_rate(clk);
> > +}
> > +
> > +#endif
> > +
> >  /*
> >   * Swtich transceiver on or off
> >   */
> > @@ -807,7 +903,7 @@ static int flexcan_open(struct net_device *dev)
> >  	struct flexcan_priv *priv = netdev_priv(dev);
> >  	int err;
> >  
> > -	clk_enable(priv->clk);
> > +	flexcan_clk_enable(priv->clk);
> >  
> >  	err = open_candev(dev);
> >  	if (err)
> > @@ -829,7 +925,7 @@ static int flexcan_open(struct net_device *dev)
> >   out_close:
> >  	close_candev(dev);
> >   out:
> > -	clk_disable(priv->clk);
> > +	flexcan_clk_disable(priv->clk);
> >  
> >  	return err;
> >  }
> > @@ -843,7 +939,7 @@ static int flexcan_close(struct net_device *dev)
> >  	flexcan_chip_stop(dev);
> >  
> >  	free_irq(dev->irq, dev);
> > -	clk_disable(priv->clk);
> > +	flexcan_clk_disable(priv->clk);
> >  
> >  	close_candev(dev);
> >  
> > @@ -882,7 +978,7 @@ static int __devinit register_flexcandev(struct net_device *dev)
> >  	struct flexcan_regs __iomem *regs = priv->base;
> >  	u32 reg, err;
> >  
> > -	clk_enable(priv->clk);
> > +	flexcan_clk_enable(priv->clk);
> >  
> >  	/* select "bus clock", chip must be disabled */
> >  	flexcan_chip_disable(priv);
> > @@ -916,7 +1012,7 @@ static int __devinit register_flexcandev(struct net_device *dev)
> >   out:
> >  	/* disable core and turn off clocks */
> >  	flexcan_chip_disable(priv);
> > -	clk_disable(priv->clk);
> > +	flexcan_clk_disable(priv->clk);
> >  
> >  	return err;
> >  }
> > @@ -936,7 +1032,7 @@ static int __devinit flexcan_probe(struct platform_device *pdev)
> >  	resource_size_t mem_size;
> >  	int err, irq;
> >  
> > -	clk = clk_get(&pdev->dev, NULL);
> > +	clk = flexcan_clk_get(&pdev->dev, NULL);
> >  	if (IS_ERR(clk)) {
> >  		dev_err(&pdev->dev, "no clock defined\n");
> >  		err = PTR_ERR(clk);
> > @@ -973,7 +1069,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 = flexcan_clk_get_rate(clk);
> >  	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;
> > @@ -1008,7 +1104,7 @@ static int __devinit flexcan_probe(struct platform_device *pdev)
> >   failed_map:
> >  	release_mem_region(mem->start, mem_size);
> >   failed_get:
> > -	clk_put(clk);
> > +	flexcan_clk_put(clk);
> >   failed_clock:
> >  	return err;
> >  }
> > @@ -1026,7 +1122,7 @@ 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);
> > +	flexcan_clk_put(priv->clk);
> >  
> >  	free_candev(dev);
> >  
> 
> 
> -- 
> Pengutronix e.K.                  | Marc Kleine-Budde           |
> Industrial Linux Solutions        | Phone: +49-231-2826-924     |
> Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
> Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |
> 

Re: [RFC 2/4] [flexcan] Introduce a flexcan_clk set of functions.

[Marc Kleine-Budde]

[-- Attachment #1.1: Type: text/plain, Size: 1710 bytes --]

On 08/05/2011 01:36 PM, Robin Holt wrote:
> I implemented the coding style changes below (Sorry I missed the two
> the first time).

np :)

> As for a better implementation, I guess I would need to understand what
> is being attempted here.  I only marginally understand the flexcan
> hardware on the Freescale P1010 and certainly am clueless about arm
> implementations of flexcan.  I just skimmed over freescale's site

The arm side is working already :)
However we just support the busclock on ARM [1]. For the first shot
stick to that clock, too.

[1] (http://lxr.linux.no/linux+v3.0/drivers/net/can/flexcan.c#L857)

> and it looks like I would be looking at their i.MX25, i.MX28, i.MX35,
> and i.MX53 family of processors.  I will attempt to find some useful
> documentation of those and look at the kernel sources for what the clk_*
> functions are trying to accomplish.
> 
> I _THINK_ I understand.  It looks like I could implement this as a powerpc
> p1010 specific thing and get the same effect without impacting flexcan.c.
> I also think I understand that the i.MX25 family of processors do
> essentially the same thing the p1010 is doing for determining the
> clock rate.

It seems that arch/powerpc/platforms/512x/clock.c implements a clock
interface. I think the p1010 should implement something similar. (Note:
I'm not a ppc guy :)

I'm looking forward for new patches.

cheers, 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 #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

[-- Attachment #2: Type: text/plain, Size: 188 bytes --]

_______________________________________________
Socketcan-core mailing list
Socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org
https://lists.berlios.de/mailman/listinfo/socketcan-core

Re: [RFC 2/4] [flexcan] Introduce a flexcan_clk set of functions.

[Robin Holt]

On Fri, Aug 05, 2011 at 02:29:46PM +0200, Marc Kleine-Budde wrote:
> On 08/05/2011 01:36 PM, Robin Holt wrote:
> > I implemented the coding style changes below (Sorry I missed the two
> > the first time).
> 
> np :)
> 
> > As for a better implementation, I guess I would need to understand what
> > is being attempted here.  I only marginally understand the flexcan
> > hardware on the Freescale P1010 and certainly am clueless about arm
> > implementations of flexcan.  I just skimmed over freescale's site
> 
> The arm side is working already :)
> However we just support the busclock on ARM [1]. For the first shot
> stick to that clock, too.
> 
> [1] (http://lxr.linux.no/linux+v3.0/drivers/net/can/flexcan.c#L857)
> 
> > and it looks like I would be looking at their i.MX25, i.MX28, i.MX35,
> > and i.MX53 family of processors.  I will attempt to find some useful
> > documentation of those and look at the kernel sources for what the clk_*
> > functions are trying to accomplish.
> > 
> > I _THINK_ I understand.  It looks like I could implement this as a powerpc
> > p1010 specific thing and get the same effect without impacting flexcan.c.
> > I also think I understand that the i.MX25 family of processors do
> > essentially the same thing the p1010 is doing for determining the
> > clock rate.
> 
> It seems that arch/powerpc/platforms/512x/clock.c implements a clock
> interface. I think the p1010 should implement something similar. (Note:
> I'm not a ppc guy :)
> 
> I'm looking forward for new patches.

I agree.  I think I will have that implemented shortly after I get to the
office (about an hour for drive, etc).  I think I am going to implement
a simple match on the dev_id of flexcan.0 or flexcan.1 and otherwise
return failure.

I also looked at implementing the CLOCKDEV_LOOKUP but think that is
going to drive me to drink.

Thanks,
Robin

[RFC 3/4] [flexcan] Add of_match to platform_device definition.

[Robin Holt]

It looks like the of_device stuff got moved under the
platform_device->driver and all we should need to do is define an of_match
to get a flexcan_probe call out.

Signed-off-by: Robin Holt <holt@sgi.com>
To: Marc Kleine-Budde <mkl@pengutronix.de>
To: Wolfgang Grandegger <wg@grandegger.com>
Cc: socketcan-core@lists.berlios.de
Cc: netdev@vger.kernel.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 3417d0b..fbb61c6 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -1129,8 +1129,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

Re: [RFC 3/4] [flexcan] Add of_match to platform_device definition.

[Marc Kleine-Budde]

[-- Attachment #1.1: Type: text/plain, Size: 1747 bytes --]

On 08/05/2011 04:06 AM, Robin Holt wrote:
> It looks like the of_device stuff got moved under the
> platform_device->driver and all we should need to do is define an of_match
> to get a flexcan_probe call out.

Please rephrase this commit message, at it goes into the git tree.
Otherwise the patche looks good.

Marc
> 
> Signed-off-by: Robin Holt <holt-sJ/iWh9BUns@public.gmane.org>
> To: Marc Kleine-Budde <mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> To: Wolfgang Grandegger <wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>
> Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org
> Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.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 3417d0b..fbb61c6 100644
> --- a/drivers/net/can/flexcan.c
> +++ b/drivers/net/can/flexcan.c
> @@ -1129,8 +1129,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),
>  };


-- 
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 #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

[-- Attachment #2: Type: text/plain, Size: 188 bytes --]

_______________________________________________
Socketcan-core mailing list
Socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org
https://lists.berlios.de/mailman/listinfo/socketcan-core

[RFC 4/4] [flexcan] Add support for FLEXCAN_DEBUG

[Robin Holt]

Add a wrapper function for a register dump when a developer defines
FLEXCAN_DEBUG.

Signed-off-by: Robin Holt <holt@sgi.com>
To: Marc Kleine-Budde <mkl@pengutronix.de>
To: Wolfgang Grandegger <wg@grandegger.com>
Cc: socketcan-core@lists.berlios.de
Cc: netdev@vger.kernel.org
---
 drivers/net/can/flexcan.c |   40 ++++++++++++++++++++++++++++++++++++++++
 1 files changed, 40 insertions(+), 0 deletions(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index fbb61c6..96684ca 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -316,6 +316,38 @@ static inline unsigned long flexcan_clk_get_rate(struct clk *clk)
 
 #endif
 
+#if defined(FLEXCAN_DEBUG)
+void _flexcan_reg_dump(struct net_device *dev, const char *file, int line,
+		       const char *func)
+{
+	const struct flexcan_priv *priv = netdev_priv(dev);
+	struct flexcan_regs __iomem *regs = priv->base;
+
+	printk(KERN_INFO "flexcan_reg_dump:%s:%d:%s()\n", file, line, func);
+	printk(KERN_INFO
+		"\t  mcr 0x%08x  ctrl 0x%08x timer 0x%08x   rxg 0x%08x",
+		flexcan_read(&regs->mcr),
+		flexcan_read(&regs->ctrl),
+		flexcan_read(&regs->timer),
+		flexcan_read(&regs->rxgmask));
+	printk(KERN_INFO
+		"\t rx14 0x%08x  rx15 0x%08x   ecr 0x%08x   esr 0x%08x",
+		flexcan_read(&regs->rx14mask),
+		flexcan_read(&regs->rx15mask),
+		flexcan_read(&regs->ecr),
+		flexcan_read(&regs->esr));
+	printk(KERN_INFO
+		"\timsk2 0x%08x imsk1 0x%08x iflg2 0x%08x iflg1 0x%08x",
+		flexcan_read(&regs->imask2),
+		flexcan_read(&regs->imask1),
+		flexcan_read(&regs->iflag2),
+		flexcan_read(&regs->iflag1));
+}
+#define flexcan_reg_dump(_d) _flexcan_reg_dump(_d, __FILE__, __LINE__, __func__)
+#else
+#define flexcan_reg_dump(_d)
+#endif
+
 /*
  * Swtich transceiver on or off
  */
@@ -376,6 +408,8 @@ static int flexcan_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	u32 can_id;
 	u32 ctrl = FLEXCAN_MB_CNT_CODE(0xc) | (cf->can_dlc << 16);
 
+	flexcan_reg_dump(dev);
+
 	if (can_dropped_invalid_skb(dev, skb))
 		return NETDEV_TX_OK;
 
@@ -408,6 +442,8 @@ static int flexcan_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	/* tx_packets is incremented in flexcan_irq */
 	stats->tx_bytes += cf->can_dlc;
 
+	flexcan_reg_dump(dev);
+
 	return NETDEV_TX_OK;
 }
 
@@ -676,6 +712,8 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
 	struct flexcan_regs __iomem *regs = priv->base;
 	u32 reg_iflag1, reg_esr;
 
+	flexcan_reg_dump(dev);
+
 	reg_iflag1 = flexcan_read(&regs->iflag1);
 	reg_esr = flexcan_read(&regs->esr);
 	flexcan_write(FLEXCAN_ESR_ERR_INT, &regs->esr);	/* ACK err IRQ */
@@ -716,6 +754,8 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
 		netif_wake_queue(dev);
 	}
 
+	flexcan_reg_dump(dev);
+
 	return IRQ_HANDLED;
 }
 
-- 
1.7.2.1

Re: [RFC 4/4] [flexcan] Add support for FLEXCAN_DEBUG

[Marc Kleine-Budde]

[-- Attachment #1.1: Type: text/plain, Size: 3634 bytes --]

On 08/05/2011 04:06 AM, Robin Holt wrote:
> Add a wrapper function for a register dump when a developer defines
> FLEXCAN_DEBUG

Comments inline..however I'm not sure if we need this patch.

> Signed-off-by: Robin Holt <holt-sJ/iWh9BUns@public.gmane.org>
> To: Marc Kleine-Budde <mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> To: Wolfgang Grandegger <wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>
> Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org
> Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> ---
>  drivers/net/can/flexcan.c |   40 ++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 40 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> index fbb61c6..96684ca 100644
> --- a/drivers/net/can/flexcan.c
> +++ b/drivers/net/can/flexcan.c
> @@ -316,6 +316,38 @@ static inline unsigned long flexcan_clk_get_rate(struct clk *clk)
>  
>  #endif
>  
> +#if defined(FLEXCAN_DEBUG)
> +void _flexcan_reg_dump(struct net_device *dev, const char *file, int line,
> +		       const char *func)
> +{
> +	const struct flexcan_priv *priv = netdev_priv(dev);
> +	struct flexcan_regs __iomem *regs = priv->base;
> +
> +	printk(KERN_INFO "flexcan_reg_dump:%s:%d:%s()\n", file, line, func);

Use netdev_$LEVEL, please.
If you use dbg, you can remove the ifdef altogether.

> +	printk(KERN_INFO
> +		"\t  mcr 0x%08x  ctrl 0x%08x timer 0x%08x   rxg 0x%08x",
> +		flexcan_read(&regs->mcr),
> +		flexcan_read(&regs->ctrl),
> +		flexcan_read(&regs->timer),
> +		flexcan_read(&regs->rxgmask));
> +	printk(KERN_INFO
> +		"\t rx14 0x%08x  rx15 0x%08x   ecr 0x%08x   esr 0x%08x",
> +		flexcan_read(&regs->rx14mask),
> +		flexcan_read(&regs->rx15mask),
> +		flexcan_read(&regs->ecr),
> +		flexcan_read(&regs->esr));
> +	printk(KERN_INFO
> +		"\timsk2 0x%08x imsk1 0x%08x iflg2 0x%08x iflg1 0x%08x",
> +		flexcan_read(&regs->imask2),
> +		flexcan_read(&regs->imask1),
> +		flexcan_read(&regs->iflag2),
> +		flexcan_read(&regs->iflag1));
> +}
> +#define flexcan_reg_dump(_d) _flexcan_reg_dump(_d, __FILE__, __LINE__, __func__)
> +#else
> +#define flexcan_reg_dump(_d)
> +#endif
> +
>  /*
>   * Swtich transceiver on or off
>   */
> @@ -376,6 +408,8 @@ static int flexcan_start_xmit(struct sk_buff *skb, struct net_device *dev)
>  	u32 can_id;
>  	u32 ctrl = FLEXCAN_MB_CNT_CODE(0xc) | (cf->can_dlc << 16);
>  
> +	flexcan_reg_dump(dev);
> +
>  	if (can_dropped_invalid_skb(dev, skb))
>  		return NETDEV_TX_OK;
>  
> @@ -408,6 +442,8 @@ static int flexcan_start_xmit(struct sk_buff *skb, struct net_device *dev)
>  	/* tx_packets is incremented in flexcan_irq */
>  	stats->tx_bytes += cf->can_dlc;
>  
> +	flexcan_reg_dump(dev);
> +
>  	return NETDEV_TX_OK;
>  }
>  
> @@ -676,6 +712,8 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
>  	struct flexcan_regs __iomem *regs = priv->base;
>  	u32 reg_iflag1, reg_esr;
>  
> +	flexcan_reg_dump(dev);
> +
>  	reg_iflag1 = flexcan_read(&regs->iflag1);
>  	reg_esr = flexcan_read(&regs->esr);
>  	flexcan_write(FLEXCAN_ESR_ERR_INT, &regs->esr);	/* ACK err IRQ */
> @@ -716,6 +754,8 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
>  		netif_wake_queue(dev);
>  	}
>  
> +	flexcan_reg_dump(dev);
> +
>  	return IRQ_HANDLED;
>  }
>  

cheers, 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 #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

[-- Attachment #2: Type: text/plain, Size: 188 bytes --]

_______________________________________________
Socketcan-core mailing list
Socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org
https://lists.berlios.de/mailman/listinfo/socketcan-core

Re: [RFC 4/4] [flexcan] Add support for FLEXCAN_DEBUG

[Robin Holt]

On Fri, Aug 05, 2011 at 09:16:06AM +0200, Marc Kleine-Budde wrote:
> On 08/05/2011 04:06 AM, Robin Holt wrote:
> > Add a wrapper function for a register dump when a developer defines
> > FLEXCAN_DEBUG
> 
> Comments inline..however I'm not sure if we need this patch.

I really do like the ability to dump the registers.  It has come in handy
a couple of times while bringing up the board.  I do not know how hard
I would push for them, but I do not see them as having much down side
and I do know they have proven useful for me.

At this point, I have interpretted both your's and Wolfgang's comment as
a suggestion.  I do not know how this group of developers works and if
I should be taking that suggestion as a dope-slap indicating I should
drop this right now because I am an idiot or if the patch just leaves
a bad taste in your mouth.  Would either or both of you please clarify.

> > +#if defined(FLEXCAN_DEBUG)
> > +void _flexcan_reg_dump(struct net_device *dev, const char *file, int line,
> > +		       const char *func)
> > +{
> > +	const struct flexcan_priv *priv = netdev_priv(dev);
> > +	struct flexcan_regs __iomem *regs = priv->base;
> > +
> > +	printk(KERN_INFO "flexcan_reg_dump:%s:%d:%s()\n", file, line, func);
> 
> Use netdev_$LEVEL, please.
> If you use dbg, you can remove the ifdef altogether.

I assume you mean netdev_dbg.  If that were the case, then setting
CONFIG_CAN_DEBUG_DEVICES would bring these printks back and that was
explicitly stated as undesirable.  Am I understanding this wrong?

If not, I am going to fall back to netdev_info instead and leave the
#ifdef FLEXCAN_DEBUG.  Will that work?

Thanks,
Robin

[RFC 0/4] [flexcan/powerpc] Add support for powerpc flexcan (freescale p1010) -V7

[Robin Holt]

Marc, Wolfgang or U Bhaskar,

This patch set should have all your comments included.  It is based on
the David S. Miller net-next-2.6 tree commit 19fd617.

I have compiled each patch in the series individually for both arm and
powerpc (cheated on ppc and reordered them with the last patch first so
I could select CAN_FLEXCAN.

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 on an arm system as I have no
access to an arm based system.

For the first three patches in the series, I believe they are all ready
for submission to David S. Miller for the netdev tree.  This is the
first submission of patch 4 which, I believe, will end up going through
the linuxppc-dev list and can go at any time after the flexcan patches
are submitted.  As long the first and last patches are both applied,
the tree will compile for either ppc or arm, but will not work without
all patches.  If the ppc patch (4/4) gets applied before 1/4, we could
have a random config tester detect a compile failure on ppc if they
select NET, CAN, and CAN_FLEXCAN.

Thanks,
Robin Holt

[RFC 3/4] [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-sJ/iWh9BUns@public.gmane.org>
To: Marc Kleine-Budde <mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
To: Wolfgang Grandegger <wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>
To: U Bhaskar-B22300 <B22300-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org
Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: PPC list <linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.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

[RFC 3/4] [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>
To: Marc Kleine-Budde <mkl@pengutronix.de>
To: Wolfgang Grandegger <wg@grandegger.com>
To: 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 |   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