[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