[PATCH v10 0/5] [flexcan/powerpc] Add support for powerpc flexcan (freescale p1010)

[PATCH v10 0/5] [flexcan/powerpc] Add support for powerpc flexcan (freescale p1010)

[Robin Holt]

With all the patches applied, my p1010rdb works for communicating between
its two can ports and also can communicate with an external PSOC.  I have
done no testing beyond compile testing 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 forwarding to David S. Miller for the netdev tree.  I think patch
4 is ready for submission to the PPC85xx maintainer.  Patch 5 changed
from the previous post by adding a second compatible string for the
fsl,p1010_flexcan.

Thanks,
Robin Holt

[PATCH v10 1/5] [flexcan] Remove #include <mach/clock.h>

[Robin Holt]

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

Signed-off-by: Robin Holt <holt@sgi.com>
To: Marc Kleine-Budde <mkl@pengutronix.de>
Acked-by: 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 |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

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

[PATCH v10 2/5] [flexcan] Abstract off read/write for big/little endian.

[Robin Holt]

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

Signed-off-by: Robin Holt <holt@sgi.com>
Acked-by: Marc Kleine-Budde <mkl@pengutronix.de>
Acked-by: Wolfgang Grandegger <wg@grandegger.com>
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 |  140 ++++++++++++++++++++++++++------------------
 1 files changed, 83 insertions(+), 57 deletions(-)

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

[PATCH v10 3/5] [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>
Acked-by: 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

Re: [PATCH v10 3/5] [flexcan] Add of_match to platform_device definition.

[Robin Holt]

On Tue, Aug 09, 2011 at 10:06:02PM -0500, Robin Holt wrote:
> 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>
> Acked-by: 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",

Let me make sure I have this correct.  At this point, we would want it
to be fsl,flexcan here.  If, at some point, we find the i.MX-wonderful
has diverged from the -p1010, we would, at that point in the code, use
of_device_is_compatible to differentiate the two, correct?  That would
mean we should make no change to this patch for the fsl,flexcan-p1010,
right?

Robin

[PATCH v10 4/5] [powerpc] Add flexcan device support for p1010rdb.

[Robin Holt]

I added a simple clock source for the p1010rdb so the flexcan driver
could determine a clock frequency.  The p1010 can device only has an
oscillator of system bus frequency divided by 2.

Signed-off-by: Robin Holt <holt@sgi.com>
Acked-by: Marc Kleine-Budde <mkl@pengutronix.de>,
Acked-by: 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>
Cc: Kumar Gala <galak@kernel.crashing.org>
---
 arch/powerpc/platforms/85xx/Kconfig    |    2 +
 arch/powerpc/platforms/85xx/Makefile   |    2 +
 arch/powerpc/platforms/85xx/clock.c    |   53 ++++++++++++++++++++++++++++++++
 arch/powerpc/platforms/85xx/p1010rdb.c |    8 +++++
 4 files changed, 65 insertions(+), 0 deletions(-)
 create mode 100644 arch/powerpc/platforms/85xx/clock.c

diff --git a/arch/powerpc/platforms/85xx/Kconfig b/arch/powerpc/platforms/85xx/Kconfig
index 498534c..c4304ae 100644
--- a/arch/powerpc/platforms/85xx/Kconfig
+++ b/arch/powerpc/platforms/85xx/Kconfig
@@ -70,6 +70,8 @@ config MPC85xx_RDB
 config P1010_RDB
 	bool "Freescale P1010RDB"
 	select DEFAULT_UIMAGE
+	select HAVE_CAN_FLEXCAN if NET && CAN
+	select PPC_CLOCK if CAN_FLEXCAN
 	help
 	  This option enables support for the MPC85xx RDB (P1010 RDB) board
 
diff --git a/arch/powerpc/platforms/85xx/Makefile b/arch/powerpc/platforms/85xx/Makefile
index a971b32..cc7f381 100644
--- a/arch/powerpc/platforms/85xx/Makefile
+++ b/arch/powerpc/platforms/85xx/Makefile
@@ -3,6 +3,8 @@
 #
 obj-$(CONFIG_SMP) += smp.o
 
+obj-$(CONFIG_PPC_CLOCK)   += clock.o
+
 obj-$(CONFIG_MPC8540_ADS) += mpc85xx_ads.o
 obj-$(CONFIG_MPC8560_ADS) += mpc85xx_ads.o
 obj-$(CONFIG_MPC85xx_CDS) += mpc85xx_cds.o
diff --git a/arch/powerpc/platforms/85xx/clock.c b/arch/powerpc/platforms/85xx/clock.c
new file mode 100644
index 0000000..16fae04
--- /dev/null
+++ b/arch/powerpc/platforms/85xx/clock.c
@@ -0,0 +1,53 @@
+/*
+ * Copyright 2011 SGI, inc.
+ *
+ * This code is licensed for use under the GPL V2 as published by
+ * the Free Software Foundation.
+ */
+
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/of.h>
+
+#include <asm/clk_interface.h>
+
+#include <sysdev/fsl_soc.h>
+
+/*
+ * p1010 needs to provide a clock source for the flexcan driver. The
+ * oscillator for the p1010 processor is only ever the system clock / 2.
+ */
+
+static struct clk *mpc85xx_clk_get(struct device *dev, const char *id)
+{
+	if (!dev)
+		return ERR_PTR(-ENOENT);
+
+        if (!dev->of_node ||
+            !of_device_is_compatible(dev->of_node, "fsl,flexcan"))
+                return ERR_PTR(-ENOENT);
+
+	return NULL;
+}
+
+static void mpc85xx_clk_put(struct clk *clk)
+{
+	return;
+}
+
+static unsigned long mpc85xx_clk_get_rate(struct clk *clk)
+{
+	return fsl_get_sys_freq() / 2;
+}
+
+static struct clk_interface mpc85xx_clk_functions = {
+	.clk_get = mpc85xx_clk_get,
+	.clk_get_rate = mpc85xx_clk_get_rate,
+	.clk_put = mpc85xx_clk_put,
+};
+
+void __init mpc85xx_clk_init(void)
+{
+	clk_functions = mpc85xx_clk_functions;
+}
+
diff --git a/arch/powerpc/platforms/85xx/p1010rdb.c b/arch/powerpc/platforms/85xx/p1010rdb.c
index d7387fa..5e52122 100644
--- a/arch/powerpc/platforms/85xx/p1010rdb.c
+++ b/arch/powerpc/platforms/85xx/p1010rdb.c
@@ -81,6 +81,13 @@ static void __init p1010_rdb_setup_arch(void)
 	printk(KERN_INFO "P1010 RDB board from Freescale Semiconductor\n");
 }
 
+extern void mpc85xx_clk_init(void);
+
+static void __init p1010_rdb_init(void)
+{
+	mpc85xx_clk_init();
+}
+
 static struct of_device_id __initdata p1010rdb_ids[] = {
 	{ .type = "soc", },
 	{ .compatible = "soc", },
@@ -111,6 +118,7 @@ define_machine(p1010_rdb) {
 	.name			= "P1010 RDB",
 	.probe			= p1010_rdb_probe,
 	.setup_arch		= p1010_rdb_setup_arch,
+	.init			= p1010_rdb_init,
 	.init_IRQ		= p1010_rdb_pic_init,
 #ifdef CONFIG_PCI
 	.pcibios_fixup_bus	= fsl_pcibios_fixup_bus,
-- 
1.7.2.1

Re: [PATCH v10 4/5] [powerpc] Add flexcan device support for p1010rdb.

[Marc Kleine-Budde]

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

On 08/10/2011 05:06 AM, Robin Holt wrote:
> I added a simple clock source for the p1010rdb so the flexcan driver
> could determine a clock frequency.  The p1010 can device only has an
> oscillator of system bus frequency divided by 2.
> 
> Signed-off-by: Robin Holt <holt@sgi.com>
> Acked-by: Marc Kleine-Budde <mkl@pengutronix.de>,
> Acked-by: 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>
> Cc: Kumar Gala <galak@kernel.crashing.org>
> ---
>  arch/powerpc/platforms/85xx/Kconfig    |    2 +
>  arch/powerpc/platforms/85xx/Makefile   |    2 +
>  arch/powerpc/platforms/85xx/clock.c    |   53 ++++++++++++++++++++++++++++++++
>  arch/powerpc/platforms/85xx/p1010rdb.c |    8 +++++
>  4 files changed, 65 insertions(+), 0 deletions(-)
>  create mode 100644 arch/powerpc/platforms/85xx/clock.c
> 
> diff --git a/arch/powerpc/platforms/85xx/Kconfig b/arch/powerpc/platforms/85xx/Kconfig
> index 498534c..c4304ae 100644
> --- a/arch/powerpc/platforms/85xx/Kconfig
> +++ b/arch/powerpc/platforms/85xx/Kconfig
> @@ -70,6 +70,8 @@ config MPC85xx_RDB
>  config P1010_RDB
>  	bool "Freescale P1010RDB"
>  	select DEFAULT_UIMAGE
> +	select HAVE_CAN_FLEXCAN if NET && CAN
> +	select PPC_CLOCK if CAN_FLEXCAN
>  	help
>  	  This option enables support for the MPC85xx RDB (P1010 RDB) board
>  
> diff --git a/arch/powerpc/platforms/85xx/Makefile b/arch/powerpc/platforms/85xx/Makefile
> index a971b32..cc7f381 100644
> --- a/arch/powerpc/platforms/85xx/Makefile
> +++ b/arch/powerpc/platforms/85xx/Makefile
> @@ -3,6 +3,8 @@
>  #
>  obj-$(CONFIG_SMP) += smp.o
>  
> +obj-$(CONFIG_PPC_CLOCK)   += clock.o
> +
>  obj-$(CONFIG_MPC8540_ADS) += mpc85xx_ads.o
>  obj-$(CONFIG_MPC8560_ADS) += mpc85xx_ads.o
>  obj-$(CONFIG_MPC85xx_CDS) += mpc85xx_cds.o
> diff --git a/arch/powerpc/platforms/85xx/clock.c b/arch/powerpc/platforms/85xx/clock.c
> new file mode 100644
> index 0000000..16fae04
> --- /dev/null
> +++ b/arch/powerpc/platforms/85xx/clock.c
> @@ -0,0 +1,53 @@
> +/*
> + * Copyright 2011 SGI, inc.
> + *
> + * This code is licensed for use under the GPL V2 as published by
> + * the Free Software Foundation.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/of.h>
> +
> +#include <asm/clk_interface.h>
> +
> +#include <sysdev/fsl_soc.h>
> +
> +/*
> + * p1010 needs to provide a clock source for the flexcan driver. The
> + * oscillator for the p1010 processor is only ever the system clock / 2.
> + */
> +
> +static struct clk *mpc85xx_clk_get(struct device *dev, const char *id)
> +{
> +	if (!dev)
> +		return ERR_PTR(-ENOENT);
> +
> +        if (!dev->of_node ||
> +            !of_device_is_compatible(dev->of_node, "fsl,flexcan"))
> +                return ERR_PTR(-ENOENT);
> +
> +	return NULL;
> +}
> +
> +static void mpc85xx_clk_put(struct clk *clk)
> +{
> +	return;
> +}
> +
> +static unsigned long mpc85xx_clk_get_rate(struct clk *clk)
> +{
> +	return fsl_get_sys_freq() / 2;
> +}
> +
> +static struct clk_interface mpc85xx_clk_functions = {
> +	.clk_get = mpc85xx_clk_get,
> +	.clk_get_rate = mpc85xx_clk_get_rate,
> +	.clk_put = mpc85xx_clk_put,
> +};
> +
> +void __init mpc85xx_clk_init(void)
> +{
> +	clk_functions = mpc85xx_clk_functions;
> +}
> +

git is even picker then me: "new blank line at EOF."
please fix

> diff --git a/arch/powerpc/platforms/85xx/p1010rdb.c b/arch/powerpc/platforms/85xx/p1010rdb.c
> index d7387fa..5e52122 100644
> --- a/arch/powerpc/platforms/85xx/p1010rdb.c
> +++ b/arch/powerpc/platforms/85xx/p1010rdb.c
> @@ -81,6 +81,13 @@ static void __init p1010_rdb_setup_arch(void)
>  	printk(KERN_INFO "P1010 RDB board from Freescale Semiconductor\n");
>  }
>  
> +extern void mpc85xx_clk_init(void);
> +
> +static void __init p1010_rdb_init(void)
> +{
> +	mpc85xx_clk_init();
> +}
> +
>  static struct of_device_id __initdata p1010rdb_ids[] = {
>  	{ .type = "soc", },
>  	{ .compatible = "soc", },
> @@ -111,6 +118,7 @@ define_machine(p1010_rdb) {
>  	.name			= "P1010 RDB",
>  	.probe			= p1010_rdb_probe,
>  	.setup_arch		= p1010_rdb_setup_arch,
> +	.init			= p1010_rdb_init,
>  	.init_IRQ		= p1010_rdb_pic_init,
>  #ifdef CONFIG_PCI
>  	.pcibios_fixup_bus	= fsl_pcibios_fixup_bus,

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


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

Re: [PATCH v10 4/5] [powerpc] Add flexcan device support for p1010rdb.

[Robin Holt]

On Wed, Aug 10, 2011 at 11:56:01AM +0200, Marc Kleine-Budde wrote:
> On 08/10/2011 05:06 AM, Robin Holt wrote:
> > diff --git a/arch/powerpc/platforms/85xx/clock.c b/arch/powerpc/platforms/85xx/clock.c
> > new file mode 100644
> > index 0000000..16fae04
> > --- /dev/null
> > +++ b/arch/powerpc/platforms/85xx/clock.c
> > @@ -0,0 +1,53 @@
...
> > +void __init mpc85xx_clk_init(void)
> > +{
> > +	clk_functions = mpc85xx_clk_functions;
> > +}
> > +
> 
> git is even picker then me: "new blank line at EOF."
> please fix

That reminded me that I have slacked on using checkpatch.pl in a few
resends and it really does not like this patch at all so I will rework
the patch with its concerns.  I am going to hold off on resending until
there are more substantial changes.

Thanks,
Robin

[PATCH v10 5/5] [powerpc] Fix up fsl-flexcan device tree binding.

[Robin Holt]

In working with the socketcan developers, we have come to the conclusion
the Documentation...fsl-flexcan.txt device tree documentation needs to
be cleaned up.  The driver does not depend upon any properties other
than the required properties so we are removing the file.  Additionally,
the p1010*dts* files are not following the standard for node naming in
that they have a trailing -v1.0.

Signed-off-by: Robin Holt <holt@sgi.com>
To: Marc Kleine-Budde <mkl@pengutronix.de>,
To: Wolfgang Grandegger <wg@grandegger.com>,
To: U Bhaskar-B22300 <B22300@freescale.com>
To: Scott Wood <scottwood@freescale.com>
Cc: socketcan-core@lists.berlios.de,
Cc: netdev@vger.kernel.org,
Cc: PPC list <linuxppc-dev@lists.ozlabs.org>
Cc: Kumar Gala <galak@kernel.crashing.org>
---
 .../devicetree/bindings/net/can/fsl-flexcan.txt    |   61 --------------------
 arch/powerpc/boot/dts/p1010rdb.dts                 |    8 ---
 arch/powerpc/boot/dts/p1010si.dtsi                 |    8 +-
 3 files changed, 4 insertions(+), 73 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/net/can/fsl-flexcan.txt

diff --git a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
deleted file mode 100644
index 1a729f0..0000000
--- a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
+++ /dev/null
@@ -1,61 +0,0 @@
-CAN Device Tree Bindings
-------------------------
-2011 Freescale Semiconductor, Inc.
-
-fsl,flexcan-v1.0 nodes
------------------------
-In addition to the required compatible-, reg- and interrupt-properties, you can
-also specify which clock source shall be used for the controller.
-
-CPI Clock- Can Protocol Interface Clock
-	This CLK_SRC bit of CTRL(control register) selects the clock source to
-	the CAN Protocol Interface(CPI) to be either the peripheral clock
-	(driven by the PLL) or the crystal oscillator clock. The selected clock
-	is the one fed to the prescaler to generate the Serial Clock (Sclock).
-	The PRESDIV field of CTRL(control register) controls a prescaler that
-	generates the Serial Clock (Sclock), whose period defines the
-	time quantum used to compose the CAN waveform.
-
-Can Engine Clock Source
-	There are two sources for CAN clock
-	- Platform Clock  It represents the bus clock
-	- Oscillator Clock
-
-	Peripheral Clock (PLL)
-	--------------
-		     |
-		    ---------		      -------------
-		    |       |CPI Clock	      | Prescaler |       Sclock
-		    |       |---------------->| (1.. 256) |------------>
-		    ---------		      -------------
-                     |  |
-	--------------  ---------------------CLK_SRC
-	Oscillator Clock
-
-- fsl,flexcan-clock-source : CAN Engine Clock Source.This property selects
-			     the peripheral clock. PLL clock is fed to the
-			     prescaler to generate the Serial Clock (Sclock).
-			     Valid values are "oscillator" and "platform"
-			     "oscillator": CAN engine clock source is oscillator clock.
-			     "platform" The CAN engine clock source is the bus clock
-		             (platform clock).
-
-- fsl,flexcan-clock-divider : for the reference and system clock, an additional
-			      clock divider can be specified.
-- clock-frequency: frequency required to calculate the bitrate for FlexCAN.
-
-Note:
-	- v1.0 of flexcan-v1.0 represent the IP block version for P1010 SOC.
-	- P1010 does not have oscillator as the Clock Source.So the default
-	  Clock Source is platform clock.
-Examples:
-
-	can0@1c000 {
-		compatible = "fsl,flexcan-v1.0";
-		reg = <0x1c000 0x1000>;
-		interrupts = <48 0x2>;
-		interrupt-parent = <&mpic>;
-		fsl,flexcan-clock-source = "platform";
-		fsl,flexcan-clock-divider = <2>;
-		clock-frequency = <fixed by u-boot>;
-	};
diff --git a/arch/powerpc/boot/dts/p1010rdb.dts b/arch/powerpc/boot/dts/p1010rdb.dts
index 6b33b73..d6a0bb2 100644
--- a/arch/powerpc/boot/dts/p1010rdb.dts
+++ b/arch/powerpc/boot/dts/p1010rdb.dts
@@ -169,14 +169,6 @@
 			};
 		};
 
-		can0@1c000 {
-			fsl,flexcan-clock-source = "platform";
-		};
-
-		can1@1d000 {
-			fsl,flexcan-clock-source = "platform";
-		};
-
 		usb@22000 {
 			phy_type = "utmi";
 		};
diff --git a/arch/powerpc/boot/dts/p1010si.dtsi b/arch/powerpc/boot/dts/p1010si.dtsi
index 7f51104..20c396d 100644
--- a/arch/powerpc/boot/dts/p1010si.dtsi
+++ b/arch/powerpc/boot/dts/p1010si.dtsi
@@ -141,19 +141,19 @@
 		};
 
 		can0@1c000 {
-			compatible = "fsl,flexcan-v1.0";
+			compatible = "fsl,p1010-flexcan",
+					"fsl,flexcan";
 			reg = <0x1c000 0x1000>;
 			interrupts = <48 0x2>;
 			interrupt-parent = <&mpic>;
-			fsl,flexcan-clock-divider = <2>;
 		};
 
 		can1@1d000 {
-			compatible = "fsl,flexcan-v1.0";
+			compatible = "fsl,p1010-flexcan",
+					"fsl,flexcan";
 			reg = <0x1d000 0x1000>;
 			interrupts = <61 0x2>;
 			interrupt-parent = <&mpic>;
-			fsl,flexcan-clock-divider = <2>;
 		};
 
 		L2: l2-cache-controller@20000 {
-- 
1.7.2.1

Re: [PATCH v10 5/5] [powerpc] Fix up fsl-flexcan device tree binding.

[Robert Schwebel]

Robin,

On Tue, Aug 09, 2011 at 10:06:04PM -0500, Robin Holt wrote:
> In working with the socketcan developers, we have come to the conclusion
> the Documentation...fsl-flexcan.txt device tree documentation needs to
> be cleaned up.  The driver does not depend upon any properties other
> than the required properties so we are removing the file.  Additionally,
> the p1010*dts* files are not following the standard for node naming in
> that they have a trailing -v1.0.
>
> Signed-off-by: Robin Holt <holt@sgi.com>
> To: Marc Kleine-Budde <mkl@pengutronix.de>,
> To: Wolfgang Grandegger <wg@grandegger.com>,
> To: U Bhaskar-B22300 <B22300@freescale.com>
> To: Scott Wood <scottwood@freescale.com>
> Cc: socketcan-core@lists.berlios.de,
> Cc: netdev@vger.kernel.org,
> Cc: PPC list <linuxppc-dev@lists.ozlabs.org>
> Cc: Kumar Gala <galak@kernel.crashing.org>
> ---
>  .../devicetree/bindings/net/can/fsl-flexcan.txt    |   61 --------------------
>  arch/powerpc/boot/dts/p1010rdb.dts                 |    8 ---
>  arch/powerpc/boot/dts/p1010si.dtsi                 |    8 +-
>  3 files changed, 4 insertions(+), 73 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/net/can/fsl-flexcan.txt

I suggest that you set devicetree-discuss@lists.ozlabs.org and Grant
Likely on Cc: for this patch.

rsc
-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH v10 5/5] [powerpc] Fix up fsl-flexcan device tree binding.

[Robin Holt]

Grant,

Do you need this patch resent with you on the Cc: list or can you pick
up the discussion from here?  I am just trying to minimize noise on
the mailing lists if it is not needed.

The essence of the discussion to this point is:

1) Freescale built a board support package for a new processor and eval
   board (p1010) based upon some older boards.

2) As part of that effort, they made changes as they saw fit to get it
   working without submitting those changes to the community.

3) In the case of the flexcan device, that included defining a device
   tree node for the integrated Controller Area Network device and set
   .compatible="fsl,flexcan-v1.0".  Apparently this was copied from
   another device, possibly mscan.

4) In an effort to get my p1010rdb eval board working with a community
   kernel, I started submitting patches for flexcan which have evolved
   to the point of nearly being accepted with the exception of we need
   to settle on the node's .compatible= strings and whether it should
   be documented.

5) To this point, I believe the concensus has settled on
   "fsl,p1010-flexcan" and "fsl,flexcan" as the compatible strings.
   This leaves the most flexibility for segragating future specific
   revisions of the flexcan interface if needed.

6) I failed to Cc: you or the device tree list when I submitted version
   10 of the patches.  I have added both to the patch header so they
   will be included in the next pass.

Thanks,
Robin Holt

On Tue, Aug 09, 2011 at 10:06:04PM -0500, Robin Holt wrote:
> In working with the socketcan developers, we have come to the conclusion
> the Documentation...fsl-flexcan.txt device tree documentation needs to
> be cleaned up.  The driver does not depend upon any properties other
> than the required properties so we are removing the file.  Additionally,
> the p1010*dts* files are not following the standard for node naming in
> that they have a trailing -v1.0.
> 
> Signed-off-by: Robin Holt <holt@sgi.com>
> To: Marc Kleine-Budde <mkl@pengutronix.de>,
> To: Wolfgang Grandegger <wg@grandegger.com>,
> To: U Bhaskar-B22300 <B22300@freescale.com>
> To: Scott Wood <scottwood@freescale.com>
> Cc: socketcan-core@lists.berlios.de,
> Cc: netdev@vger.kernel.org,
> Cc: PPC list <linuxppc-dev@lists.ozlabs.org>
> Cc: Kumar Gala <galak@kernel.crashing.org>
> ---
>  .../devicetree/bindings/net/can/fsl-flexcan.txt    |   61 --------------------
>  arch/powerpc/boot/dts/p1010rdb.dts                 |    8 ---
>  arch/powerpc/boot/dts/p1010si.dtsi                 |    8 +-
>  3 files changed, 4 insertions(+), 73 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> 
> diff --git a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> deleted file mode 100644
> index 1a729f0..0000000
> --- a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> +++ /dev/null
> @@ -1,61 +0,0 @@
> -CAN Device Tree Bindings
> -------------------------
> -2011 Freescale Semiconductor, Inc.
> -
> -fsl,flexcan-v1.0 nodes
> ------------------------
> -In addition to the required compatible-, reg- and interrupt-properties, you can
> -also specify which clock source shall be used for the controller.
> -
> -CPI Clock- Can Protocol Interface Clock
> -	This CLK_SRC bit of CTRL(control register) selects the clock source to
> -	the CAN Protocol Interface(CPI) to be either the peripheral clock
> -	(driven by the PLL) or the crystal oscillator clock. The selected clock
> -	is the one fed to the prescaler to generate the Serial Clock (Sclock).
> -	The PRESDIV field of CTRL(control register) controls a prescaler that
> -	generates the Serial Clock (Sclock), whose period defines the
> -	time quantum used to compose the CAN waveform.
> -
> -Can Engine Clock Source
> -	There are two sources for CAN clock
> -	- Platform Clock  It represents the bus clock
> -	- Oscillator Clock
> -
> -	Peripheral Clock (PLL)
> -	--------------
> -		     |
> -		    ---------		      -------------
> -		    |       |CPI Clock	      | Prescaler |       Sclock
> -		    |       |---------------->| (1.. 256) |------------>
> -		    ---------		      -------------
> -                     |  |
> -	--------------  ---------------------CLK_SRC
> -	Oscillator Clock
> -
> -- fsl,flexcan-clock-source : CAN Engine Clock Source.This property selects
> -			     the peripheral clock. PLL clock is fed to the
> -			     prescaler to generate the Serial Clock (Sclock).
> -			     Valid values are "oscillator" and "platform"
> -			     "oscillator": CAN engine clock source is oscillator clock.
> -			     "platform" The CAN engine clock source is the bus clock
> -		             (platform clock).
> -
> -- fsl,flexcan-clock-divider : for the reference and system clock, an additional
> -			      clock divider can be specified.
> -- clock-frequency: frequency required to calculate the bitrate for FlexCAN.
> -
> -Note:
> -	- v1.0 of flexcan-v1.0 represent the IP block version for P1010 SOC.
> -	- P1010 does not have oscillator as the Clock Source.So the default
> -	  Clock Source is platform clock.
> -Examples:
> -
> -	can0@1c000 {
> -		compatible = "fsl,flexcan-v1.0";
> -		reg = <0x1c000 0x1000>;
> -		interrupts = <48 0x2>;
> -		interrupt-parent = <&mpic>;
> -		fsl,flexcan-clock-source = "platform";
> -		fsl,flexcan-clock-divider = <2>;
> -		clock-frequency = <fixed by u-boot>;
> -	};
> diff --git a/arch/powerpc/boot/dts/p1010rdb.dts b/arch/powerpc/boot/dts/p1010rdb.dts
> index 6b33b73..d6a0bb2 100644
> --- a/arch/powerpc/boot/dts/p1010rdb.dts
> +++ b/arch/powerpc/boot/dts/p1010rdb.dts
> @@ -169,14 +169,6 @@
>  			};
>  		};
>  
> -		can0@1c000 {
> -			fsl,flexcan-clock-source = "platform";
> -		};
> -
> -		can1@1d000 {
> -			fsl,flexcan-clock-source = "platform";
> -		};
> -
>  		usb@22000 {
>  			phy_type = "utmi";
>  		};
> diff --git a/arch/powerpc/boot/dts/p1010si.dtsi b/arch/powerpc/boot/dts/p1010si.dtsi
> index 7f51104..20c396d 100644
> --- a/arch/powerpc/boot/dts/p1010si.dtsi
> +++ b/arch/powerpc/boot/dts/p1010si.dtsi
> @@ -141,19 +141,19 @@
>  		};
>  
>  		can0@1c000 {
> -			compatible = "fsl,flexcan-v1.0";
> +			compatible = "fsl,p1010-flexcan",
> +					"fsl,flexcan";
>  			reg = <0x1c000 0x1000>;
>  			interrupts = <48 0x2>;
>  			interrupt-parent = <&mpic>;
> -			fsl,flexcan-clock-divider = <2>;
>  		};
>  
>  		can1@1d000 {
> -			compatible = "fsl,flexcan-v1.0";
> +			compatible = "fsl,p1010-flexcan",
> +					"fsl,flexcan";
>  			reg = <0x1d000 0x1000>;
>  			interrupts = <61 0x2>;
>  			interrupt-parent = <&mpic>;
> -			fsl,flexcan-clock-divider = <2>;
>  		};
>  
>  		L2: l2-cache-controller@20000 {
> -- 
> 1.7.2.1

Re: [PATCH v10 5/5] [powerpc] Fix up fsl-flexcan device tree binding.

[Wolfgang Grandegger]

Hi Robin,

On 08/10/2011 05:06 AM, Robin Holt wrote:
> In working with the socketcan developers, we have come to the conclusion
> the Documentation...fsl-flexcan.txt device tree documentation needs to
> be cleaned up.  The driver does not depend upon any properties other

Your first sentence could be misleading. Please just describe what the
patch does and why, something like:

"This patch cleans up the documentation of the device-tree binding for
 the Flexcan devices on Freescale's PowerPC and ARM cores. Extra
 properties are not needed as the frequency of the source clock is
 fixed..." and so on.

> than the required properties so we are removing the file.  Additionally,
> the p1010*dts* files are not following the standard for node naming in
> that they have a trailing -v1.0.

> Signed-off-by: Robin Holt <holt@sgi.com>
> To: Marc Kleine-Budde <mkl@pengutronix.de>,
> To: Wolfgang Grandegger <wg@grandegger.com>,
> To: U Bhaskar-B22300 <B22300@freescale.com>
> To: Scott Wood <scottwood@freescale.com>
> Cc: socketcan-core@lists.berlios.de,
> Cc: netdev@vger.kernel.org,
> Cc: PPC list <linuxppc-dev@lists.ozlabs.org>
> Cc: Kumar Gala <galak@kernel.crashing.org>
> ---
>  .../devicetree/bindings/net/can/fsl-flexcan.txt    |   61 --------------------
>  arch/powerpc/boot/dts/p1010rdb.dts                 |    8 ---
>  arch/powerpc/boot/dts/p1010si.dtsi                 |    8 +-
>  3 files changed, 4 insertions(+), 73 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> 
> diff --git a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> deleted file mode 100644
> index 1a729f0..0000000
> --- a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> +++ /dev/null
> @@ -1,61 +0,0 @@
> -CAN Device Tree Bindings
> -------------------------
> -2011 Freescale Semiconductor, Inc.
> -
> -fsl,flexcan-v1.0 nodes
> ------------------------
> -In addition to the required compatible-, reg- and interrupt-properties, you can
> -also specify which clock source shall be used for the controller.
> -
> -CPI Clock- Can Protocol Interface Clock
> -	This CLK_SRC bit of CTRL(control register) selects the clock source to
> -	the CAN Protocol Interface(CPI) to be either the peripheral clock
> -	(driven by the PLL) or the crystal oscillator clock. The selected clock
> -	is the one fed to the prescaler to generate the Serial Clock (Sclock).
> -	The PRESDIV field of CTRL(control register) controls a prescaler that
> -	generates the Serial Clock (Sclock), whose period defines the
> -	time quantum used to compose the CAN waveform.
> -
> -Can Engine Clock Source
> -	There are two sources for CAN clock
> -	- Platform Clock  It represents the bus clock
> -	- Oscillator Clock
> -
> -	Peripheral Clock (PLL)
> -	--------------
> -		     |
> -		    ---------		      -------------
> -		    |       |CPI Clock	      | Prescaler |       Sclock
> -		    |       |---------------->| (1.. 256) |------------>
> -		    ---------		      -------------
> -                     |  |
> -	--------------  ---------------------CLK_SRC
> -	Oscillator Clock
> -
> -- fsl,flexcan-clock-source : CAN Engine Clock Source.This property selects
> -			     the peripheral clock. PLL clock is fed to the
> -			     prescaler to generate the Serial Clock (Sclock).
> -			     Valid values are "oscillator" and "platform"
> -			     "oscillator": CAN engine clock source is oscillator clock.
> -			     "platform" The CAN engine clock source is the bus clock
> -		             (platform clock).
> -
> -- fsl,flexcan-clock-divider : for the reference and system clock, an additional
> -			      clock divider can be specified.
> -- clock-frequency: frequency required to calculate the bitrate for FlexCAN.
> -
> -Note:
> -	- v1.0 of flexcan-v1.0 represent the IP block version for P1010 SOC.
> -	- P1010 does not have oscillator as the Clock Source.So the default
> -	  Clock Source is platform clock.
> -Examples:
> -
> -	can0@1c000 {
> -		compatible = "fsl,flexcan-v1.0";
> -		reg = <0x1c000 0x1000>;
> -		interrupts = <48 0x2>;
> -		interrupt-parent = <&mpic>;
> -		fsl,flexcan-clock-source = "platform";
> -		fsl,flexcan-clock-divqider = <2>;
> -		clock-frequency = <fixed by u-boot>;
> -	};

Do we really want to drop the documentation for that binding. I think
something like the following text would be still useful:

------------------------
Flexcan CAN contoller on Freescale's ARM and PowerPC processors

Required properties:

- compatible : Should be "fsl,flexcan" and optionally
               "fsl,flexcan-<processor>"
- reg : Offset and length of the register set for this device
- interrupts : Interrupt tuple for this device

Example:

  can@1c000 {
          compatible = "fsl,p1010-flexcan", "fsl,flexcan";
          reg = <0x1c000 0x1000>;
          interrupts = <48 0x2>;
          interrupt-parent = <&mpic>;
  };
-------------------------

What do you think?

> diff --git a/arch/powerpc/boot/dts/p1010rdb.dts b/arch/powerpc/boot/dts/p1010rdb.dts
> index 6b33b73..d6a0bb2 100644
> --- a/arch/powerpc/boot/dts/p1010rdb.dts
> +++ b/arch/powerpc/boot/dts/p1010rdb.dts
> @@ -169,14 +169,6 @@
>  			};
>  		};
>  
> -		can0@1c000 {
> -			fsl,flexcan-clock-source = "platform";
> -		};
> -
> -		can1@1d000 {
> -			fsl,flexcan-clock-source = "platform";
> -		};
> -
>  		usb@22000 {
>  			phy_type = "utmi";
>  		};
> diff --git a/arch/powerpc/boot/dts/p1010si.dtsi b/arch/powerpc/boot/dts/p1010si.dtsi
> index 7f51104..20c396d 100644
> --- a/arch/powerpc/boot/dts/p1010si.dtsi
> +++ b/arch/powerpc/boot/dts/p1010si.dtsi
> @@ -141,19 +141,19 @@
>  		};
>  
>  		can0@1c000 {
> -			compatible = "fsl,flexcan-v1.0";
> +			compatible = "fsl,p1010-flexcan",
> +					"fsl,flexcan";

Does fit on one line.

>  			reg = <0x1c000 0x1000>;
>  			interrupts = <48 0x2>;
>  			interrupt-parent = <&mpic>;
> -			fsl,flexcan-clock-divider = <2>;
>  		};
>  
>  		can1@1d000 {
> -			compatible = "fsl,flexcan-v1.0";
> +			compatible = "fsl,p1010-flexcan",
> +					"fsl,flexcan";

Ditto

>  			reg = <0x1d000 0x1000>;
>  			interrupts = <61 0x2>;
>  			interrupt-parent = <&mpic>;
> -			fsl,flexcan-clock-divider = <2>;
>  		};
>  
>  		L2: l2-cache-controller@20000 {

Please also correct the node names (not using the number suffix).

Wolfgang.

Re: [PATCH v10 5/5] [powerpc] Fix up fsl-flexcan device tree binding.

[Robin Holt]

On Wed, Aug 10, 2011 at 03:47:43PM +0200, Wolfgang Grandegger wrote:
> Hi Robin,
> 
> On 08/10/2011 05:06 AM, Robin Holt wrote:
> > In working with the socketcan developers, we have come to the conclusion
> > the Documentation...fsl-flexcan.txt device tree documentation needs to
> > be cleaned up.  The driver does not depend upon any properties other
> 
> Your first sentence could be misleading. Please just describe what the
> patch does and why, something like:
> 
> "This patch cleans up the documentation of the device-tree binding for
>  the Flexcan devices on Freescale's PowerPC and ARM cores. Extra
>  properties are not needed as the frequency of the source clock is
>  fixed..." and so on.

I borrowed heavily from your message. ;)

> > than the required properties so we are removing the file.  Additionally,
> > the p1010*dts* files are not following the standard for node naming in
> > that they have a trailing -v1.0.
> 
> > Signed-off-by: Robin Holt <holt@sgi.com>
> > To: Marc Kleine-Budde <mkl@pengutronix.de>,
> > To: Wolfgang Grandegger <wg@grandegger.com>,
> > To: U Bhaskar-B22300 <B22300@freescale.com>
> > To: Scott Wood <scottwood@freescale.com>
> > Cc: socketcan-core@lists.berlios.de,
> > Cc: netdev@vger.kernel.org,
> > Cc: PPC list <linuxppc-dev@lists.ozlabs.org>
> > Cc: Kumar Gala <galak@kernel.crashing.org>
> > ---
> >  .../devicetree/bindings/net/can/fsl-flexcan.txt    |   61 --------------------
> >  arch/powerpc/boot/dts/p1010rdb.dts                 |    8 ---
> >  arch/powerpc/boot/dts/p1010si.dtsi                 |    8 +-
> >  3 files changed, 4 insertions(+), 73 deletions(-)
> >  delete mode 100644 Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> > deleted file mode 100644
> > index 1a729f0..0000000
> > --- a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> > +++ /dev/null
> > @@ -1,61 +0,0 @@
> > -CAN Device Tree Bindings
> > -------------------------
> > -2011 Freescale Semiconductor, Inc.
> > -
> > -fsl,flexcan-v1.0 nodes
> > ------------------------
> > -In addition to the required compatible-, reg- and interrupt-properties, you can
> > -also specify which clock source shall be used for the controller.
> > -
> > -CPI Clock- Can Protocol Interface Clock
> > -	This CLK_SRC bit of CTRL(control register) selects the clock source to
> > -	the CAN Protocol Interface(CPI) to be either the peripheral clock
> > -	(driven by the PLL) or the crystal oscillator clock. The selected clock
> > -	is the one fed to the prescaler to generate the Serial Clock (Sclock).
> > -	The PRESDIV field of CTRL(control register) controls a prescaler that
> > -	generates the Serial Clock (Sclock), whose period defines the
> > -	time quantum used to compose the CAN waveform.
> > -
> > -Can Engine Clock Source
> > -	There are two sources for CAN clock
> > -	- Platform Clock  It represents the bus clock
> > -	- Oscillator Clock
> > -
> > -	Peripheral Clock (PLL)
> > -	--------------
> > -		     |
> > -		    ---------		      -------------
> > -		    |       |CPI Clock	      | Prescaler |       Sclock
> > -		    |       |---------------->| (1.. 256) |------------>
> > -		    ---------		      -------------
> > -                     |  |
> > -	--------------  ---------------------CLK_SRC
> > -	Oscillator Clock
> > -
> > -- fsl,flexcan-clock-source : CAN Engine Clock Source.This property selects
> > -			     the peripheral clock. PLL clock is fed to the
> > -			     prescaler to generate the Serial Clock (Sclock).
> > -			     Valid values are "oscillator" and "platform"
> > -			     "oscillator": CAN engine clock source is oscillator clock.
> > -			     "platform" The CAN engine clock source is the bus clock
> > -		             (platform clock).
> > -
> > -- fsl,flexcan-clock-divider : for the reference and system clock, an additional
> > -			      clock divider can be specified.
> > -- clock-frequency: frequency required to calculate the bitrate for FlexCAN.
> > -
> > -Note:
> > -	- v1.0 of flexcan-v1.0 represent the IP block version for P1010 SOC.
> > -	- P1010 does not have oscillator as the Clock Source.So the default
> > -	  Clock Source is platform clock.
> > -Examples:
> > -
> > -	can0@1c000 {
> > -		compatible = "fsl,flexcan-v1.0";
> > -		reg = <0x1c000 0x1000>;
> > -		interrupts = <48 0x2>;
> > -		interrupt-parent = <&mpic>;
> > -		fsl,flexcan-clock-source = "platform";
> > -		fsl,flexcan-clock-divqider = <2>;
> > -		clock-frequency = <fixed by u-boot>;
> > -	};
> 
> Do we really want to drop the documentation for that binding. I think
> something like the following text would be still useful:
> 
> ------------------------
> Flexcan CAN contoller on Freescale's ARM and PowerPC processors
> 
> Required properties:
> 
> - compatible : Should be "fsl,flexcan" and optionally
>                "fsl,flexcan-<processor>"
> - reg : Offset and length of the register set for this device
> - interrupts : Interrupt tuple for this device
> 
> Example:
> 
>   can@1c000 {
>           compatible = "fsl,p1010-flexcan", "fsl,flexcan";
>           reg = <0x1c000 0x1000>;
>           interrupts = <48 0x2>;
>           interrupt-parent = <&mpic>;
>   };
> -------------------------

Done, except the
>           compatible = "fsl,p1010-flexcan", "fsl,flexcan";

line is
	compatible = "fsl,flexcan", "fsl,flexcan-p1010";

> 
> What do you think?
> 
> > diff --git a/arch/powerpc/boot/dts/p1010rdb.dts b/arch/powerpc/boot/dts/p1010rdb.dts
> > index 6b33b73..d6a0bb2 100644
> > --- a/arch/powerpc/boot/dts/p1010rdb.dts
> > +++ b/arch/powerpc/boot/dts/p1010rdb.dts
> > @@ -169,14 +169,6 @@
> >  			};
> >  		};
> >  
> > -		can0@1c000 {
> > -			fsl,flexcan-clock-source = "platform";
> > -		};
> > -
> > -		can1@1d000 {
> > -			fsl,flexcan-clock-source = "platform";
> > -		};
> > -
> >  		usb@22000 {
> >  			phy_type = "utmi";
> >  		};
> > diff --git a/arch/powerpc/boot/dts/p1010si.dtsi b/arch/powerpc/boot/dts/p1010si.dtsi
> > index 7f51104..20c396d 100644
> > --- a/arch/powerpc/boot/dts/p1010si.dtsi
> > +++ b/arch/powerpc/boot/dts/p1010si.dtsi
> > @@ -141,19 +141,19 @@
> >  		};
> >  
> >  		can0@1c000 {
> > -			compatible = "fsl,flexcan-v1.0";
> > +			compatible = "fsl,p1010-flexcan",
> > +					"fsl,flexcan";
> 
> Does fit on one line.
> 
> >  			reg = <0x1c000 0x1000>;
> >  			interrupts = <48 0x2>;
> >  			interrupt-parent = <&mpic>;
> > -			fsl,flexcan-clock-divider = <2>;
> >  		};
> >  
> >  		can1@1d000 {
> > -			compatible = "fsl,flexcan-v1.0";
> > +			compatible = "fsl,p1010-flexcan",
> > +					"fsl,flexcan";
> 
> Ditto
> 
> >  			reg = <0x1d000 0x1000>;
> >  			interrupts = <61 0x2>;
> >  			interrupt-parent = <&mpic>;
> > -			fsl,flexcan-clock-divider = <2>;
> >  		};
> >  
> >  		L2: l2-cache-controller@20000 {
> 
> Please also correct the node names (not using the number suffix).

So the node names should be
		can@1c000 {
		can@1d000 {
correct?

Robin

RE: [PATCH v10 5/5] [powerpc] Fix up fsl-flexcan device tree binding.

[U Bhaskar-B22300]

> -----Original Message-----
> From: Robin Holt [mailto:holt@sgi.com]
> Sent: Wednesday, August 10, 2011 7:46 PM
> To: Wolfgang Grandegger
> Cc: Robin Holt; Marc Kleine-Budde; U Bhaskar-B22300; Wood Scott-B07421;
> netdev@vger.kernel.org; Kumar Gala; socketcan-core@lists.berlios.de; PPC
> list
> Subject: Re: [PATCH v10 5/5] [powerpc] Fix up fsl-flexcan device tree
> binding.
>=20
> On Wed, Aug 10, 2011 at 03:47:43PM +0200, Wolfgang Grandegger wrote:
> > Hi Robin,
> >
> > On 08/10/2011 05:06 AM, Robin Holt wrote:
> > > In working with the socketcan developers, we have come to the
> > > conclusion the Documentation...fsl-flexcan.txt device tree
> > > documentation needs to be cleaned up.  The driver does not depend
> > > upon any properties other
> >
> > Your first sentence could be misleading. Please just describe what the
> > patch does and why, something like:
> >
> > "This patch cleans up the documentation of the device-tree binding for
> > the Flexcan devices on Freescale's PowerPC and ARM cores. Extra
> > properties are not needed as the frequency of the source clock is
> > fixed..." and so on.
>=20
> I borrowed heavily from your message. ;)
>=20
> > > than the required properties so we are removing the file.
> > > Additionally, the p1010*dts* files are not following the standard
> > > for node naming in that they have a trailing -v1.0.
> >
> > > Signed-off-by: Robin Holt <holt@sgi.com>
> > > To: Marc Kleine-Budde <mkl@pengutronix.de>,
> > > To: Wolfgang Grandegger <wg@grandegger.com>,
> > > To: U Bhaskar-B22300 <B22300@freescale.com>
> > > To: Scott Wood <scottwood@freescale.com>
> > > Cc: socketcan-core@lists.berlios.de,
> > > Cc: netdev@vger.kernel.org,
> > > Cc: PPC list <linuxppc-dev@lists.ozlabs.org>
> > > Cc: Kumar Gala <galak@kernel.crashing.org>
> > > ---
> > >  .../devicetree/bindings/net/can/fsl-flexcan.txt    |   61 ----------
> ----------
> > >  arch/powerpc/boot/dts/p1010rdb.dts                 |    8 ---
> > >  arch/powerpc/boot/dts/p1010si.dtsi                 |    8 +-
> > >  3 files changed, 4 insertions(+), 73 deletions(-)  delete mode
> > > 100644 Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> > >
> > > diff --git
> > > a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> > > b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> > > deleted file mode 100644
> > > index 1a729f0..0000000
> > > --- a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> > > +++ /dev/null
> > > @@ -1,61 +0,0 @@
> > > -CAN Device Tree Bindings
> > > -------------------------
> > > -2011 Freescale Semiconductor, Inc.
> > > -
> > > -fsl,flexcan-v1.0 nodes
> > > ------------------------
> > > -In addition to the required compatible-, reg- and
> > > interrupt-properties, you can -also specify which clock source shall
> be used for the controller.
> > > -
> > > -CPI Clock- Can Protocol Interface Clock
> > > -	This CLK_SRC bit of CTRL(control register) selects the clock source
> to
> > > -	the CAN Protocol Interface(CPI) to be either the peripheral clock
> > > -	(driven by the PLL) or the crystal oscillator clock. The selected
> clock
> > > -	is the one fed to the prescaler to generate the Serial Clock
> (Sclock).
> > > -	The PRESDIV field of CTRL(control register) controls a prescaler
> that
> > > -	generates the Serial Clock (Sclock), whose period defines the
> > > -	time quantum used to compose the CAN waveform.
> > > -
> > > -Can Engine Clock Source
> > > -	There are two sources for CAN clock
> > > -	- Platform Clock  It represents the bus clock
> > > -	- Oscillator Clock
> > > -
> > > -	Peripheral Clock (PLL)
> > > -	--------------
> > > -		     |
> > > -		    ---------		      -------------
> > > -		    |       |CPI Clock	      | Prescaler |       Sclock
> > > -		    |       |---------------->| (1.. 256) |------------>
> > > -		    ---------		      -------------
> > > -                     |  |
> > > -	--------------  ---------------------CLK_SRC
> > > -	Oscillator Clock
> > > -
> > > -- fsl,flexcan-clock-source : CAN Engine Clock Source.This property
> selects
> > > -			     the peripheral clock. PLL clock is fed to the
> > > -			     prescaler to generate the Serial Clock (Sclock).
> > > -			     Valid values are "oscillator" and "platform"
> > > -			     "oscillator": CAN engine clock source is
> oscillator clock.
> > > -			     "platform" The CAN engine clock source is the bus
> clock
> > > -		             (platform clock).
> > > -
> > > -- fsl,flexcan-clock-divider : for the reference and system clock, an
> additional
> > > -			      clock divider can be specified.
> > > -- clock-frequency: frequency required to calculate the bitrate for
> FlexCAN.
> > > -
> > > -Note:
> > > -	- v1.0 of flexcan-v1.0 represent the IP block version for P1010
> SOC.
> > > -	- P1010 does not have oscillator as the Clock Source.So the default
> > > -	  Clock Source is platform clock.
> > > -Examples:
> > > -
> > > -	can0@1c000 {
> > > -		compatible =3D "fsl,flexcan-v1.0";
> > > -		reg =3D <0x1c000 0x1000>;
> > > -		interrupts =3D <48 0x2>;
> > > -		interrupt-parent =3D <&mpic>;
> > > -		fsl,flexcan-clock-source =3D "platform";
> > > -		fsl,flexcan-clock-divqider =3D <2>;
> > > -		clock-frequency =3D <fixed by u-boot>;
> > > -	};
> >
> > Do we really want to drop the documentation for that binding. I think
> > something like the following text would be still useful:
> >
> > ------------------------
> > Flexcan CAN contoller on Freescale's ARM and PowerPC processors
> >
> > Required properties:
> >
> > - compatible : Should be "fsl,flexcan" and optionally
> >                "fsl,flexcan-<processor>"
> > - reg : Offset and length of the register set for this device
> > - interrupts : Interrupt tuple for this device
> >
> > Example:
> >
> >   can@1c000 {
> >           compatible =3D "fsl,p1010-flexcan", "fsl,flexcan";
> >           reg =3D <0x1c000 0x1000>;
> >           interrupts =3D <48 0x2>;
> >           interrupt-parent =3D <&mpic>;
> >   };
> > -------------------------
>=20
> Done, except the
> >           compatible =3D "fsl,p1010-flexcan", "fsl,flexcan";
>=20
> line is
> 	compatible =3D "fsl,flexcan", "fsl,flexcan-p1010";
>=20
> >
> > What do you think?
> >
> > > diff --git a/arch/powerpc/boot/dts/p1010rdb.dts
> > > b/arch/powerpc/boot/dts/p1010rdb.dts
> > > index 6b33b73..d6a0bb2 100644
> > > --- a/arch/powerpc/boot/dts/p1010rdb.dts
> > > +++ b/arch/powerpc/boot/dts/p1010rdb.dts
> > > @@ -169,14 +169,6 @@
> > >  			};
> > >  		};
> > >
> > > -		can0@1c000 {
> > > -			fsl,flexcan-clock-source =3D "platform";
> > > -		};
> > > -
> > > -		can1@1d000 {
> > > -			fsl,flexcan-clock-source =3D "platform";
> > > -		};
> > > -
> > >  		usb@22000 {
> > >  			phy_type =3D "utmi";
> > >  		};
> > > diff --git a/arch/powerpc/boot/dts/p1010si.dtsi
> > > b/arch/powerpc/boot/dts/p1010si.dtsi
> > > index 7f51104..20c396d 100644
> > > --- a/arch/powerpc/boot/dts/p1010si.dtsi
> > > +++ b/arch/powerpc/boot/dts/p1010si.dtsi
> > > @@ -141,19 +141,19 @@
> > >  		};
> > >
> > >  		can0@1c000 {
> > > -			compatible =3D "fsl,flexcan-v1.0";
> > > +			compatible =3D "fsl,p1010-flexcan",
> > > +					"fsl,flexcan";
> >
> > Does fit on one line.
> >
> > >  			reg =3D <0x1c000 0x1000>;
> > >  			interrupts =3D <48 0x2>;
> > >  			interrupt-parent =3D <&mpic>;
> > > -			fsl,flexcan-clock-divider =3D <2>;
> > >  		};
> > >
> > >  		can1@1d000 {
> > > -			compatible =3D "fsl,flexcan-v1.0";
> > > +			compatible =3D "fsl,p1010-flexcan",
> > > +					"fsl,flexcan";
> >
> > Ditto
> >
> > >  			reg =3D <0x1d000 0x1000>;
> > >  			interrupts =3D <61 0x2>;
> > >  			interrupt-parent =3D <&mpic>;
> > > -			fsl,flexcan-clock-divider =3D <2>;
> > >  		};
> > >
> > >  		L2: l2-cache-controller@20000 {
> >
> > Please also correct the node names (not using the number suffix).
>=20
> So the node names should be
> 		can@1c000 {
> 		can@1d000 {
> correct?
>=20
[Bhaskar] As there are two CAN controllers on P1010,So won't it be better
	    to distinguish it by can0 and can1 instead by simple "can" ?
> Robin

Re: [PATCH v10 5/5] [powerpc] Fix up fsl-flexcan device tree binding.

[Robin Holt]

On Wed, Aug 10, 2011 at 02:36:20PM +0000, U Bhaskar-B22300 wrote:
> 
> 
> > -----Original Message-----
> > From: Robin Holt [mailto:holt@sgi.com]
> > Sent: Wednesday, August 10, 2011 7:46 PM
> > To: Wolfgang Grandegger
> > Cc: Robin Holt; Marc Kleine-Budde; U Bhaskar-B22300; Wood Scott-B07421;
> > netdev@vger.kernel.org; Kumar Gala; socketcan-core@lists.berlios.de; PPC
> > list
> > Subject: Re: [PATCH v10 5/5] [powerpc] Fix up fsl-flexcan device tree
> > binding.
> > 
> > On Wed, Aug 10, 2011 at 03:47:43PM +0200, Wolfgang Grandegger wrote:
> > > Hi Robin,
> > >
> > > On 08/10/2011 05:06 AM, Robin Holt wrote:
> > > > In working with the socketcan developers, we have come to the
> > > > conclusion the Documentation...fsl-flexcan.txt device tree
> > > > documentation needs to be cleaned up.  The driver does not depend
> > > > upon any properties other
> > >
> > > Your first sentence could be misleading. Please just describe what the
> > > patch does and why, something like:
> > >
> > > "This patch cleans up the documentation of the device-tree binding for
> > > the Flexcan devices on Freescale's PowerPC and ARM cores. Extra
> > > properties are not needed as the frequency of the source clock is
> > > fixed..." and so on.
> > 
> > I borrowed heavily from your message. ;)
> > 
> > > > than the required properties so we are removing the file.
> > > > Additionally, the p1010*dts* files are not following the standard
> > > > for node naming in that they have a trailing -v1.0.
> > >
> > > > Signed-off-by: Robin Holt <holt@sgi.com>
> > > > To: Marc Kleine-Budde <mkl@pengutronix.de>,
> > > > To: Wolfgang Grandegger <wg@grandegger.com>,
> > > > To: U Bhaskar-B22300 <B22300@freescale.com>
> > > > To: Scott Wood <scottwood@freescale.com>
> > > > Cc: socketcan-core@lists.berlios.de,
> > > > Cc: netdev@vger.kernel.org,
> > > > Cc: PPC list <linuxppc-dev@lists.ozlabs.org>
> > > > Cc: Kumar Gala <galak@kernel.crashing.org>
> > > > ---
> > > >  .../devicetree/bindings/net/can/fsl-flexcan.txt    |   61 ----------
> > ----------
> > > >  arch/powerpc/boot/dts/p1010rdb.dts                 |    8 ---
> > > >  arch/powerpc/boot/dts/p1010si.dtsi                 |    8 +-
> > > >  3 files changed, 4 insertions(+), 73 deletions(-)  delete mode
> > > > 100644 Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> > > >
> > > > diff --git
> > > > a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> > > > b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> > > > deleted file mode 100644
> > > > index 1a729f0..0000000
> > > > --- a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> > > > +++ /dev/null
> > > > @@ -1,61 +0,0 @@
> > > > -CAN Device Tree Bindings
> > > > -------------------------
> > > > -2011 Freescale Semiconductor, Inc.
> > > > -
> > > > -fsl,flexcan-v1.0 nodes
> > > > ------------------------
> > > > -In addition to the required compatible-, reg- and
> > > > interrupt-properties, you can -also specify which clock source shall
> > be used for the controller.
> > > > -
> > > > -CPI Clock- Can Protocol Interface Clock
> > > > -	This CLK_SRC bit of CTRL(control register) selects the clock source
> > to
> > > > -	the CAN Protocol Interface(CPI) to be either the peripheral clock
> > > > -	(driven by the PLL) or the crystal oscillator clock. The selected
> > clock
> > > > -	is the one fed to the prescaler to generate the Serial Clock
> > (Sclock).
> > > > -	The PRESDIV field of CTRL(control register) controls a prescaler
> > that
> > > > -	generates the Serial Clock (Sclock), whose period defines the
> > > > -	time quantum used to compose the CAN waveform.
> > > > -
> > > > -Can Engine Clock Source
> > > > -	There are two sources for CAN clock
> > > > -	- Platform Clock  It represents the bus clock
> > > > -	- Oscillator Clock
> > > > -
> > > > -	Peripheral Clock (PLL)
> > > > -	--------------
> > > > -		     |
> > > > -		    ---------		      -------------
> > > > -		    |       |CPI Clock	      | Prescaler |       Sclock
> > > > -		    |       |---------------->| (1.. 256) |------------>
> > > > -		    ---------		      -------------
> > > > -                     |  |
> > > > -	--------------  ---------------------CLK_SRC
> > > > -	Oscillator Clock
> > > > -
> > > > -- fsl,flexcan-clock-source : CAN Engine Clock Source.This property
> > selects
> > > > -			     the peripheral clock. PLL clock is fed to the
> > > > -			     prescaler to generate the Serial Clock (Sclock).
> > > > -			     Valid values are "oscillator" and "platform"
> > > > -			     "oscillator": CAN engine clock source is
> > oscillator clock.
> > > > -			     "platform" The CAN engine clock source is the bus
> > clock
> > > > -		             (platform clock).
> > > > -
> > > > -- fsl,flexcan-clock-divider : for the reference and system clock, an
> > additional
> > > > -			      clock divider can be specified.
> > > > -- clock-frequency: frequency required to calculate the bitrate for
> > FlexCAN.
> > > > -
> > > > -Note:
> > > > -	- v1.0 of flexcan-v1.0 represent the IP block version for P1010
> > SOC.
> > > > -	- P1010 does not have oscillator as the Clock Source.So the default
> > > > -	  Clock Source is platform clock.
> > > > -Examples:
> > > > -
> > > > -	can0@1c000 {
> > > > -		compatible = "fsl,flexcan-v1.0";
> > > > -		reg = <0x1c000 0x1000>;
> > > > -		interrupts = <48 0x2>;
> > > > -		interrupt-parent = <&mpic>;
> > > > -		fsl,flexcan-clock-source = "platform";
> > > > -		fsl,flexcan-clock-divqider = <2>;
> > > > -		clock-frequency = <fixed by u-boot>;
> > > > -	};
> > >
> > > Do we really want to drop the documentation for that binding. I think
> > > something like the following text would be still useful:
> > >
> > > ------------------------
> > > Flexcan CAN contoller on Freescale's ARM and PowerPC processors
> > >
> > > Required properties:
> > >
> > > - compatible : Should be "fsl,flexcan" and optionally
> > >                "fsl,flexcan-<processor>"
> > > - reg : Offset and length of the register set for this device
> > > - interrupts : Interrupt tuple for this device
> > >
> > > Example:
> > >
> > >   can@1c000 {
> > >           compatible = "fsl,p1010-flexcan", "fsl,flexcan";
> > >           reg = <0x1c000 0x1000>;
> > >           interrupts = <48 0x2>;
> > >           interrupt-parent = <&mpic>;
> > >   };
> > > -------------------------
> > 
> > Done, except the
> > >           compatible = "fsl,p1010-flexcan", "fsl,flexcan";
> > 
> > line is
> > 	compatible = "fsl,flexcan", "fsl,flexcan-p1010";
> > 
> > >
> > > What do you think?
> > >
> > > > diff --git a/arch/powerpc/boot/dts/p1010rdb.dts
> > > > b/arch/powerpc/boot/dts/p1010rdb.dts
> > > > index 6b33b73..d6a0bb2 100644
> > > > --- a/arch/powerpc/boot/dts/p1010rdb.dts
> > > > +++ b/arch/powerpc/boot/dts/p1010rdb.dts
> > > > @@ -169,14 +169,6 @@
> > > >  			};
> > > >  		};
> > > >
> > > > -		can0@1c000 {
> > > > -			fsl,flexcan-clock-source = "platform";
> > > > -		};
> > > > -
> > > > -		can1@1d000 {
> > > > -			fsl,flexcan-clock-source = "platform";
> > > > -		};
> > > > -
> > > >  		usb@22000 {
> > > >  			phy_type = "utmi";
> > > >  		};
> > > > diff --git a/arch/powerpc/boot/dts/p1010si.dtsi
> > > > b/arch/powerpc/boot/dts/p1010si.dtsi
> > > > index 7f51104..20c396d 100644
> > > > --- a/arch/powerpc/boot/dts/p1010si.dtsi
> > > > +++ b/arch/powerpc/boot/dts/p1010si.dtsi
> > > > @@ -141,19 +141,19 @@
> > > >  		};
> > > >
> > > >  		can0@1c000 {
> > > > -			compatible = "fsl,flexcan-v1.0";
> > > > +			compatible = "fsl,p1010-flexcan",
> > > > +					"fsl,flexcan";
> > >
> > > Does fit on one line.
> > >
> > > >  			reg = <0x1c000 0x1000>;
> > > >  			interrupts = <48 0x2>;
> > > >  			interrupt-parent = <&mpic>;
> > > > -			fsl,flexcan-clock-divider = <2>;
> > > >  		};
> > > >
> > > >  		can1@1d000 {
> > > > -			compatible = "fsl,flexcan-v1.0";
> > > > +			compatible = "fsl,p1010-flexcan",
> > > > +					"fsl,flexcan";
> > >
> > > Ditto
> > >
> > > >  			reg = <0x1d000 0x1000>;
> > > >  			interrupts = <61 0x2>;
> > > >  			interrupt-parent = <&mpic>;
> > > > -			fsl,flexcan-clock-divider = <2>;
> > > >  		};
> > > >
> > > >  		L2: l2-cache-controller@20000 {
> > >
> > > Please also correct the node names (not using the number suffix).
> > 
> > So the node names should be
> > 		can@1c000 {
> > 		can@1d000 {
> > correct?
> > 
> [Bhaskar] As there are two CAN controllers on P1010,So won't it be better
> 	    to distinguish it by can0 and can1 instead by simple "can" ?

It looks like the way to do that is to assign a label to those devices
and then associate the label with an alias.  I have no idea how that
works under the hood, but it is the way other files are set up.  Take a
look at arch/powerpc/boot/dts/bamboo.dts for how they define the serial
interfaces.

Grant or Wolfgang, is that the right way to handle the concern about
names or does it have no practical effect with the Linux kernel?

Thanks,
Robin

Re: [PATCH v10 5/5] [powerpc] Fix up fsl-flexcan device tree binding.

[Kumar Gala]

On Aug 10, 2011, at 11:00 AM, Robin Holt wrote:

> On Wed, Aug 10, 2011 at 02:36:20PM +0000, U Bhaskar-B22300 wrote:
>>=20
>>=20
>>> -----Original Message-----
>>> From: Robin Holt [mailto:holt@sgi.com]
>>> Sent: Wednesday, August 10, 2011 7:46 PM
>>> To: Wolfgang Grandegger
>>> Cc: Robin Holt; Marc Kleine-Budde; U Bhaskar-B22300; Wood =
Scott-B07421;
>>> netdev@vger.kernel.org; Kumar Gala; socketcan-core@lists.berlios.de; =
PPC
>>> list
>>> Subject: Re: [PATCH v10 5/5] [powerpc] Fix up fsl-flexcan device =
tree
>>> binding.
>>>=20
>>> On Wed, Aug 10, 2011 at 03:47:43PM +0200, Wolfgang Grandegger wrote:
>>>> Hi Robin,
>>>>=20
>>>> On 08/10/2011 05:06 AM, Robin Holt wrote:
>>>>> In working with the socketcan developers, we have come to the
>>>>> conclusion the Documentation...fsl-flexcan.txt device tree
>>>>> documentation needs to be cleaned up.  The driver does not depend
>>>>> upon any properties other
>>>>=20
>>>> Your first sentence could be misleading. Please just describe what =
the
>>>> patch does and why, something like:
>>>>=20
>>>> "This patch cleans up the documentation of the device-tree binding =
for
>>>> the Flexcan devices on Freescale's PowerPC and ARM cores. Extra
>>>> properties are not needed as the frequency of the source clock is
>>>> fixed..." and so on.
>>>=20
>>> I borrowed heavily from your message. ;)
>>>=20
>>>>> than the required properties so we are removing the file.
>>>>> Additionally, the p1010*dts* files are not following the standard
>>>>> for node naming in that they have a trailing -v1.0.
>>>>=20
>>>>> Signed-off-by: Robin Holt <holt@sgi.com>
>>>>> To: Marc Kleine-Budde <mkl@pengutronix.de>,
>>>>> To: Wolfgang Grandegger <wg@grandegger.com>,
>>>>> To: U Bhaskar-B22300 <B22300@freescale.com>
>>>>> To: Scott Wood <scottwood@freescale.com>
>>>>> Cc: socketcan-core@lists.berlios.de,
>>>>> Cc: netdev@vger.kernel.org,
>>>>> Cc: PPC list <linuxppc-dev@lists.ozlabs.org>
>>>>> Cc: Kumar Gala <galak@kernel.crashing.org>
>>>>> ---
>>>>> .../devicetree/bindings/net/can/fsl-flexcan.txt    |   61 =
----------
>>> ----------
>>>>> arch/powerpc/boot/dts/p1010rdb.dts                 |    8 ---
>>>>> arch/powerpc/boot/dts/p1010si.dtsi                 |    8 +-
>>>>> 3 files changed, 4 insertions(+), 73 deletions(-)  delete mode
>>>>> 100644 Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
>>>>>=20
>>>>> diff --git
>>>>> a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
>>>>> b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
>>>>> deleted file mode 100644
>>>>> index 1a729f0..0000000
>>>>> --- a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
>>>>> +++ /dev/null
>>>>> @@ -1,61 +0,0 @@
>>>>> -CAN Device Tree Bindings
>>>>> -------------------------
>>>>> -2011 Freescale Semiconductor, Inc.
>>>>> -
>>>>> -fsl,flexcan-v1.0 nodes
>>>>> ------------------------
>>>>> -In addition to the required compatible-, reg- and
>>>>> interrupt-properties, you can -also specify which clock source =
shall
>>> be used for the controller.
>>>>> -
>>>>> -CPI Clock- Can Protocol Interface Clock
>>>>> -	This CLK_SRC bit of CTRL(control register) selects the clock =
source
>>> to
>>>>> -	the CAN Protocol Interface(CPI) to be either the peripheral =
clock
>>>>> -	(driven by the PLL) or the crystal oscillator clock. The =
selected
>>> clock
>>>>> -	is the one fed to the prescaler to generate the Serial Clock
>>> (Sclock).
>>>>> -	The PRESDIV field of CTRL(control register) controls a prescaler
>>> that
>>>>> -	generates the Serial Clock (Sclock), whose period defines the
>>>>> -	time quantum used to compose the CAN waveform.
>>>>> -
>>>>> -Can Engine Clock Source
>>>>> -	There are two sources for CAN clock
>>>>> -	- Platform Clock  It represents the bus clock
>>>>> -	- Oscillator Clock
>>>>> -
>>>>> -	Peripheral Clock (PLL)
>>>>> -	--------------
>>>>> -		     |
>>>>> -		    ---------		      -------------
>>>>> -		    |       |CPI Clock	      | Prescaler |       Sclock
>>>>> -		    |       |---------------->| (1.. 256) |------------>
>>>>> -		    ---------		      -------------
>>>>> -                     |  |
>>>>> -	--------------  ---------------------CLK_SRC
>>>>> -	Oscillator Clock
>>>>> -
>>>>> -- fsl,flexcan-clock-source : CAN Engine Clock Source.This =
property
>>> selects
>>>>> -			     the peripheral clock. PLL clock is fed to =
the
>>>>> -			     prescaler to generate the Serial Clock =
(Sclock).
>>>>> -			     Valid values are "oscillator" and =
"platform"
>>>>> -			     "oscillator": CAN engine clock source is
>>> oscillator clock.
>>>>> -			     "platform" The CAN engine clock source is =
the bus
>>> clock
>>>>> -		             (platform clock).
>>>>> -
>>>>> -- fsl,flexcan-clock-divider : for the reference and system clock, =
an
>>> additional
>>>>> -			      clock divider can be specified.
>>>>> -- clock-frequency: frequency required to calculate the bitrate =
for
>>> FlexCAN.
>>>>> -
>>>>> -Note:
>>>>> -	- v1.0 of flexcan-v1.0 represent the IP block version for P1010
>>> SOC.
>>>>> -	- P1010 does not have oscillator as the Clock Source.So the =
default
>>>>> -	  Clock Source is platform clock.
>>>>> -Examples:
>>>>> -
>>>>> -	can0@1c000 {
>>>>> -		compatible =3D "fsl,flexcan-v1.0";
>>>>> -		reg =3D <0x1c000 0x1000>;
>>>>> -		interrupts =3D <48 0x2>;
>>>>> -		interrupt-parent =3D <&mpic>;
>>>>> -		fsl,flexcan-clock-source =3D "platform";
>>>>> -		fsl,flexcan-clock-divqider =3D <2>;
>>>>> -		clock-frequency =3D <fixed by u-boot>;
>>>>> -	};
>>>>=20
>>>> Do we really want to drop the documentation for that binding. I =
think
>>>> something like the following text would be still useful:
>>>>=20
>>>> ------------------------
>>>> Flexcan CAN contoller on Freescale's ARM and PowerPC processors
>>>>=20
>>>> Required properties:
>>>>=20
>>>> - compatible : Should be "fsl,flexcan" and optionally
>>>>               "fsl,flexcan-<processor>"
>>>> - reg : Offset and length of the register set for this device
>>>> - interrupts : Interrupt tuple for this device
>>>>=20
>>>> Example:
>>>>=20
>>>>  can@1c000 {
>>>>          compatible =3D "fsl,p1010-flexcan", "fsl,flexcan";
>>>>          reg =3D <0x1c000 0x1000>;
>>>>          interrupts =3D <48 0x2>;
>>>>          interrupt-parent =3D <&mpic>;
>>>>  };
>>>> -------------------------
>>>=20
>>> Done, except the
>>>>          compatible =3D "fsl,p1010-flexcan", "fsl,flexcan";
>>>=20
>>> line is
>>> 	compatible =3D "fsl,flexcan", "fsl,flexcan-p1010";
>>>=20
>>>>=20
>>>> What do you think?
>>>>=20
>>>>> diff --git a/arch/powerpc/boot/dts/p1010rdb.dts
>>>>> b/arch/powerpc/boot/dts/p1010rdb.dts
>>>>> index 6b33b73..d6a0bb2 100644
>>>>> --- a/arch/powerpc/boot/dts/p1010rdb.dts
>>>>> +++ b/arch/powerpc/boot/dts/p1010rdb.dts
>>>>> @@ -169,14 +169,6 @@
>>>>> 			};
>>>>> 		};
>>>>>=20
>>>>> -		can0@1c000 {
>>>>> -			fsl,flexcan-clock-source =3D "platform";
>>>>> -		};
>>>>> -
>>>>> -		can1@1d000 {
>>>>> -			fsl,flexcan-clock-source =3D "platform";
>>>>> -		};
>>>>> -
>>>>> 		usb@22000 {
>>>>> 			phy_type =3D "utmi";
>>>>> 		};
>>>>> diff --git a/arch/powerpc/boot/dts/p1010si.dtsi
>>>>> b/arch/powerpc/boot/dts/p1010si.dtsi
>>>>> index 7f51104..20c396d 100644
>>>>> --- a/arch/powerpc/boot/dts/p1010si.dtsi
>>>>> +++ b/arch/powerpc/boot/dts/p1010si.dtsi
>>>>> @@ -141,19 +141,19 @@
>>>>> 		};
>>>>>=20
>>>>> 		can0@1c000 {
>>>>> -			compatible =3D "fsl,flexcan-v1.0";
>>>>> +			compatible =3D "fsl,p1010-flexcan",
>>>>> +					"fsl,flexcan";
>>>>=20
>>>> Does fit on one line.
>>>>=20
>>>>> 			reg =3D <0x1c000 0x1000>;
>>>>> 			interrupts =3D <48 0x2>;
>>>>> 			interrupt-parent =3D <&mpic>;
>>>>> -			fsl,flexcan-clock-divider =3D <2>;
>>>>> 		};
>>>>>=20
>>>>> 		can1@1d000 {
>>>>> -			compatible =3D "fsl,flexcan-v1.0";
>>>>> +			compatible =3D "fsl,p1010-flexcan",
>>>>> +					"fsl,flexcan";
>>>>=20
>>>> Ditto
>>>>=20
>>>>> 			reg =3D <0x1d000 0x1000>;
>>>>> 			interrupts =3D <61 0x2>;
>>>>> 			interrupt-parent =3D <&mpic>;
>>>>> -			fsl,flexcan-clock-divider =3D <2>;
>>>>> 		};
>>>>>=20
>>>>> 		L2: l2-cache-controller@20000 {
>>>>=20
>>>> Please also correct the node names (not using the number suffix).
>>>=20
>>> So the node names should be
>>> 		can@1c000 {
>>> 		can@1d000 {
>>> correct?
>>>=20
>> [Bhaskar] As there are two CAN controllers on P1010,So won't it be =
better
>> 	    to distinguish it by can0 and can1 instead by simple "can" ?
>=20
> It looks like the way to do that is to assign a label to those devices
> and then associate the label with an alias.  I have no idea how that
> works under the hood, but it is the way other files are set up.  Take =
a
> look at arch/powerpc/boot/dts/bamboo.dts for how they define the =
serial
> interfaces.
>=20
> Grant or Wolfgang, is that the right way to handle the concern about
> names or does it have no practical effect with the Linux kernel?

It has not effect.  The label is just if you need to reference it via =
some other means.

- k=

Re: [PATCH v10 5/5] [powerpc] Fix up fsl-flexcan device tree binding.

[Robin Holt]

On Wed, Aug 10, 2011 at 11:53:15AM -0500, Kumar Gala wrote:
> 
> On Aug 10, 2011, at 11:00 AM, Robin Holt wrote:
> 
> > On Wed, Aug 10, 2011 at 02:36:20PM +0000, U Bhaskar-B22300 wrote:
> >> 
> >> 
> >>> -----Original Message-----
> >>> From: Robin Holt [mailto:holt@sgi.com]
> >>> Sent: Wednesday, August 10, 2011 7:46 PM
> >>> To: Wolfgang Grandegger
> >>> Cc: Robin Holt; Marc Kleine-Budde; U Bhaskar-B22300; Wood Scott-B07421;
> >>> netdev@vger.kernel.org; Kumar Gala; socketcan-core@lists.berlios.de; PPC
> >>> list
> >>> Subject: Re: [PATCH v10 5/5] [powerpc] Fix up fsl-flexcan device tree
> >>> binding.
> >>> 
> >>> On Wed, Aug 10, 2011 at 03:47:43PM +0200, Wolfgang Grandegger wrote:
> >>>> Hi Robin,
> >>>> 
> >>>> On 08/10/2011 05:06 AM, Robin Holt wrote:
> >>>>> In working with the socketcan developers, we have come to the
> >>>>> conclusion the Documentation...fsl-flexcan.txt device tree
> >>>>> documentation needs to be cleaned up.  The driver does not depend
> >>>>> upon any properties other
> >>>> 
> >>>> Your first sentence could be misleading. Please just describe what the
> >>>> patch does and why, something like:
> >>>> 
> >>>> "This patch cleans up the documentation of the device-tree binding for
> >>>> the Flexcan devices on Freescale's PowerPC and ARM cores. Extra
> >>>> properties are not needed as the frequency of the source clock is
> >>>> fixed..." and so on.
> >>> 
> >>> I borrowed heavily from your message. ;)
> >>> 
> >>>>> than the required properties so we are removing the file.
> >>>>> Additionally, the p1010*dts* files are not following the standard
> >>>>> for node naming in that they have a trailing -v1.0.
> >>>> 
> >>>>> Signed-off-by: Robin Holt <holt@sgi.com>
> >>>>> To: Marc Kleine-Budde <mkl@pengutronix.de>,
> >>>>> To: Wolfgang Grandegger <wg@grandegger.com>,
> >>>>> To: U Bhaskar-B22300 <B22300@freescale.com>
> >>>>> To: Scott Wood <scottwood@freescale.com>
> >>>>> Cc: socketcan-core@lists.berlios.de,
> >>>>> Cc: netdev@vger.kernel.org,
> >>>>> Cc: PPC list <linuxppc-dev@lists.ozlabs.org>
> >>>>> Cc: Kumar Gala <galak@kernel.crashing.org>
> >>>>> ---
> >>>>> .../devicetree/bindings/net/can/fsl-flexcan.txt    |   61 ----------
> >>> ----------
> >>>>> arch/powerpc/boot/dts/p1010rdb.dts                 |    8 ---
> >>>>> arch/powerpc/boot/dts/p1010si.dtsi                 |    8 +-
> >>>>> 3 files changed, 4 insertions(+), 73 deletions(-)  delete mode
> >>>>> 100644 Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> >>>>> 
> >>>>> diff --git
> >>>>> a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> >>>>> b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> >>>>> deleted file mode 100644
> >>>>> index 1a729f0..0000000
> >>>>> --- a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> >>>>> +++ /dev/null
> >>>>> @@ -1,61 +0,0 @@
> >>>>> -CAN Device Tree Bindings
> >>>>> -------------------------
> >>>>> -2011 Freescale Semiconductor, Inc.
> >>>>> -
> >>>>> -fsl,flexcan-v1.0 nodes
> >>>>> ------------------------
> >>>>> -In addition to the required compatible-, reg- and
> >>>>> interrupt-properties, you can -also specify which clock source shall
> >>> be used for the controller.
> >>>>> -
> >>>>> -CPI Clock- Can Protocol Interface Clock
> >>>>> -	This CLK_SRC bit of CTRL(control register) selects the clock source
> >>> to
> >>>>> -	the CAN Protocol Interface(CPI) to be either the peripheral clock
> >>>>> -	(driven by the PLL) or the crystal oscillator clock. The selected
> >>> clock
> >>>>> -	is the one fed to the prescaler to generate the Serial Clock
> >>> (Sclock).
> >>>>> -	The PRESDIV field of CTRL(control register) controls a prescaler
> >>> that
> >>>>> -	generates the Serial Clock (Sclock), whose period defines the
> >>>>> -	time quantum used to compose the CAN waveform.
> >>>>> -
> >>>>> -Can Engine Clock Source
> >>>>> -	There are two sources for CAN clock
> >>>>> -	- Platform Clock  It represents the bus clock
> >>>>> -	- Oscillator Clock
> >>>>> -
> >>>>> -	Peripheral Clock (PLL)
> >>>>> -	--------------
> >>>>> -		     |
> >>>>> -		    ---------		      -------------
> >>>>> -		    |       |CPI Clock	      | Prescaler |       Sclock
> >>>>> -		    |       |---------------->| (1.. 256) |------------>
> >>>>> -		    ---------		      -------------
> >>>>> -                     |  |
> >>>>> -	--------------  ---------------------CLK_SRC
> >>>>> -	Oscillator Clock
> >>>>> -
> >>>>> -- fsl,flexcan-clock-source : CAN Engine Clock Source.This property
> >>> selects
> >>>>> -			     the peripheral clock. PLL clock is fed to the
> >>>>> -			     prescaler to generate the Serial Clock (Sclock).
> >>>>> -			     Valid values are "oscillator" and "platform"
> >>>>> -			     "oscillator": CAN engine clock source is
> >>> oscillator clock.
> >>>>> -			     "platform" The CAN engine clock source is the bus
> >>> clock
> >>>>> -		             (platform clock).
> >>>>> -
> >>>>> -- fsl,flexcan-clock-divider : for the reference and system clock, an
> >>> additional
> >>>>> -			      clock divider can be specified.
> >>>>> -- clock-frequency: frequency required to calculate the bitrate for
> >>> FlexCAN.
> >>>>> -
> >>>>> -Note:
> >>>>> -	- v1.0 of flexcan-v1.0 represent the IP block version for P1010
> >>> SOC.
> >>>>> -	- P1010 does not have oscillator as the Clock Source.So the default
> >>>>> -	  Clock Source is platform clock.
> >>>>> -Examples:
> >>>>> -
> >>>>> -	can0@1c000 {
> >>>>> -		compatible = "fsl,flexcan-v1.0";
> >>>>> -		reg = <0x1c000 0x1000>;
> >>>>> -		interrupts = <48 0x2>;
> >>>>> -		interrupt-parent = <&mpic>;
> >>>>> -		fsl,flexcan-clock-source = "platform";
> >>>>> -		fsl,flexcan-clock-divqider = <2>;
> >>>>> -		clock-frequency = <fixed by u-boot>;
> >>>>> -	};
> >>>> 
> >>>> Do we really want to drop the documentation for that binding. I think
> >>>> something like the following text would be still useful:
> >>>> 
> >>>> ------------------------
> >>>> Flexcan CAN contoller on Freescale's ARM and PowerPC processors
> >>>> 
> >>>> Required properties:
> >>>> 
> >>>> - compatible : Should be "fsl,flexcan" and optionally
> >>>>               "fsl,flexcan-<processor>"
> >>>> - reg : Offset and length of the register set for this device
> >>>> - interrupts : Interrupt tuple for this device
> >>>> 
> >>>> Example:
> >>>> 
> >>>>  can@1c000 {
> >>>>          compatible = "fsl,p1010-flexcan", "fsl,flexcan";
> >>>>          reg = <0x1c000 0x1000>;
> >>>>          interrupts = <48 0x2>;
> >>>>          interrupt-parent = <&mpic>;
> >>>>  };
> >>>> -------------------------
> >>> 
> >>> Done, except the
> >>>>          compatible = "fsl,p1010-flexcan", "fsl,flexcan";
> >>> 
> >>> line is
> >>> 	compatible = "fsl,flexcan", "fsl,flexcan-p1010";
> >>> 
> >>>> 
> >>>> What do you think?
> >>>> 
> >>>>> diff --git a/arch/powerpc/boot/dts/p1010rdb.dts
> >>>>> b/arch/powerpc/boot/dts/p1010rdb.dts
> >>>>> index 6b33b73..d6a0bb2 100644
> >>>>> --- a/arch/powerpc/boot/dts/p1010rdb.dts
> >>>>> +++ b/arch/powerpc/boot/dts/p1010rdb.dts
> >>>>> @@ -169,14 +169,6 @@
> >>>>> 			};
> >>>>> 		};
> >>>>> 
> >>>>> -		can0@1c000 {
> >>>>> -			fsl,flexcan-clock-source = "platform";
> >>>>> -		};
> >>>>> -
> >>>>> -		can1@1d000 {
> >>>>> -			fsl,flexcan-clock-source = "platform";
> >>>>> -		};
> >>>>> -
> >>>>> 		usb@22000 {
> >>>>> 			phy_type = "utmi";
> >>>>> 		};
> >>>>> diff --git a/arch/powerpc/boot/dts/p1010si.dtsi
> >>>>> b/arch/powerpc/boot/dts/p1010si.dtsi
> >>>>> index 7f51104..20c396d 100644
> >>>>> --- a/arch/powerpc/boot/dts/p1010si.dtsi
> >>>>> +++ b/arch/powerpc/boot/dts/p1010si.dtsi
> >>>>> @@ -141,19 +141,19 @@
> >>>>> 		};
> >>>>> 
> >>>>> 		can0@1c000 {
> >>>>> -			compatible = "fsl,flexcan-v1.0";
> >>>>> +			compatible = "fsl,p1010-flexcan",
> >>>>> +					"fsl,flexcan";
> >>>> 
> >>>> Does fit on one line.
> >>>> 
> >>>>> 			reg = <0x1c000 0x1000>;
> >>>>> 			interrupts = <48 0x2>;
> >>>>> 			interrupt-parent = <&mpic>;
> >>>>> -			fsl,flexcan-clock-divider = <2>;
> >>>>> 		};
> >>>>> 
> >>>>> 		can1@1d000 {
> >>>>> -			compatible = "fsl,flexcan-v1.0";
> >>>>> +			compatible = "fsl,p1010-flexcan",
> >>>>> +					"fsl,flexcan";
> >>>> 
> >>>> Ditto
> >>>> 
> >>>>> 			reg = <0x1d000 0x1000>;
> >>>>> 			interrupts = <61 0x2>;
> >>>>> 			interrupt-parent = <&mpic>;
> >>>>> -			fsl,flexcan-clock-divider = <2>;
> >>>>> 		};
> >>>>> 
> >>>>> 		L2: l2-cache-controller@20000 {
> >>>> 
> >>>> Please also correct the node names (not using the number suffix).
> >>> 
> >>> So the node names should be
> >>> 		can@1c000 {
> >>> 		can@1d000 {
> >>> correct?
> >>> 
> >> [Bhaskar] As there are two CAN controllers on P1010,So won't it be better
> >> 	    to distinguish it by can0 and can1 instead by simple "can" ?
> > 
> > It looks like the way to do that is to assign a label to those devices
> > and then associate the label with an alias.  I have no idea how that
> > works under the hood, but it is the way other files are set up.  Take a
> > look at arch/powerpc/boot/dts/bamboo.dts for how they define the serial
> > interfaces.
> > 
> > Grant or Wolfgang, is that the right way to handle the concern about
> > names or does it have no practical effect with the Linux kernel?
> 
> It has not effect.  The label is just if you need to reference it via some other means.

Does the alias have an effect?

Robin

Re: [PATCH v10 5/5] [powerpc] Fix up fsl-flexcan device tree binding.

[Kumar Gala]

On Aug 10, 2011, at 12:16 PM, Robin Holt wrote:

> On Wed, Aug 10, 2011 at 11:53:15AM -0500, Kumar Gala wrote:
>>=20
>> On Aug 10, 2011, at 11:00 AM, Robin Holt wrote:
>>=20
>>> On Wed, Aug 10, 2011 at 02:36:20PM +0000, U Bhaskar-B22300 wrote:
>>>>=20
>>>>=20
>>>>> -----Original Message-----
>>>>> From: Robin Holt [mailto:holt@sgi.com]
>>>>> Sent: Wednesday, August 10, 2011 7:46 PM
>>>>> To: Wolfgang Grandegger
>>>>> Cc: Robin Holt; Marc Kleine-Budde; U Bhaskar-B22300; Wood =
Scott-B07421;
>>>>> netdev@vger.kernel.org; Kumar Gala; =
socketcan-core@lists.berlios.de; PPC
>>>>> list
>>>>> Subject: Re: [PATCH v10 5/5] [powerpc] Fix up fsl-flexcan device =
tree
>>>>> binding.
>>>>>=20
>>>>> On Wed, Aug 10, 2011 at 03:47:43PM +0200, Wolfgang Grandegger =
wrote:
>>>>>> Hi Robin,
>>>>>>=20
>>>>>> On 08/10/2011 05:06 AM, Robin Holt wrote:
>>>>>>> In working with the socketcan developers, we have come to the
>>>>>>> conclusion the Documentation...fsl-flexcan.txt device tree
>>>>>>> documentation needs to be cleaned up.  The driver does not =
depend
>>>>>>> upon any properties other
>>>>>>=20
>>>>>> Your first sentence could be misleading. Please just describe =
what the
>>>>>> patch does and why, something like:
>>>>>>=20
>>>>>> "This patch cleans up the documentation of the device-tree =
binding for
>>>>>> the Flexcan devices on Freescale's PowerPC and ARM cores. Extra
>>>>>> properties are not needed as the frequency of the source clock is
>>>>>> fixed..." and so on.
>>>>>=20
>>>>> I borrowed heavily from your message. ;)
>>>>>=20
>>>>>>> than the required properties so we are removing the file.
>>>>>>> Additionally, the p1010*dts* files are not following the =
standard
>>>>>>> for node naming in that they have a trailing -v1.0.
>>>>>>=20
>>>>>>> Signed-off-by: Robin Holt <holt@sgi.com>
>>>>>>> To: Marc Kleine-Budde <mkl@pengutronix.de>,
>>>>>>> To: Wolfgang Grandegger <wg@grandegger.com>,
>>>>>>> To: U Bhaskar-B22300 <B22300@freescale.com>
>>>>>>> To: Scott Wood <scottwood@freescale.com>
>>>>>>> Cc: socketcan-core@lists.berlios.de,
>>>>>>> Cc: netdev@vger.kernel.org,
>>>>>>> Cc: PPC list <linuxppc-dev@lists.ozlabs.org>
>>>>>>> Cc: Kumar Gala <galak@kernel.crashing.org>
>>>>>>> ---
>>>>>>> .../devicetree/bindings/net/can/fsl-flexcan.txt    |   61 =
----------
>>>>> ----------
>>>>>>> arch/powerpc/boot/dts/p1010rdb.dts                 |    8 ---
>>>>>>> arch/powerpc/boot/dts/p1010si.dtsi                 |    8 +-
>>>>>>> 3 files changed, 4 insertions(+), 73 deletions(-)  delete mode
>>>>>>> 100644 Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
>>>>>>>=20
>>>>>>> diff --git
>>>>>>> a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
>>>>>>> b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
>>>>>>> deleted file mode 100644
>>>>>>> index 1a729f0..0000000
>>>>>>> --- a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
>>>>>>> +++ /dev/null
>>>>>>> @@ -1,61 +0,0 @@
>>>>>>> -CAN Device Tree Bindings
>>>>>>> -------------------------
>>>>>>> -2011 Freescale Semiconductor, Inc.
>>>>>>> -
>>>>>>> -fsl,flexcan-v1.0 nodes
>>>>>>> ------------------------
>>>>>>> -In addition to the required compatible-, reg- and
>>>>>>> interrupt-properties, you can -also specify which clock source =
shall
>>>>> be used for the controller.
>>>>>>> -
>>>>>>> -CPI Clock- Can Protocol Interface Clock
>>>>>>> -	This CLK_SRC bit of CTRL(control register) selects the =
clock source
>>>>> to
>>>>>>> -	the CAN Protocol Interface(CPI) to be either the =
peripheral clock
>>>>>>> -	(driven by the PLL) or the crystal oscillator clock. The =
selected
>>>>> clock
>>>>>>> -	is the one fed to the prescaler to generate the Serial =
Clock
>>>>> (Sclock).
>>>>>>> -	The PRESDIV field of CTRL(control register) controls a =
prescaler
>>>>> that
>>>>>>> -	generates the Serial Clock (Sclock), whose period =
defines the
>>>>>>> -	time quantum used to compose the CAN waveform.
>>>>>>> -
>>>>>>> -Can Engine Clock Source
>>>>>>> -	There are two sources for CAN clock
>>>>>>> -	- Platform Clock  It represents the bus clock
>>>>>>> -	- Oscillator Clock
>>>>>>> -
>>>>>>> -	Peripheral Clock (PLL)
>>>>>>> -	--------------
>>>>>>> -		     |
>>>>>>> -		    ---------		      -------------
>>>>>>> -		    |       |CPI Clock	      | Prescaler |      =
 Sclock
>>>>>>> -		    |       |---------------->| (1.. 256) =
|------------>
>>>>>>> -		    ---------		      -------------
>>>>>>> -                     |  |
>>>>>>> -	--------------  ---------------------CLK_SRC
>>>>>>> -	Oscillator Clock
>>>>>>> -
>>>>>>> -- fsl,flexcan-clock-source : CAN Engine Clock Source.This =
property
>>>>> selects
>>>>>>> -			     the peripheral clock. PLL clock is =
fed to the
>>>>>>> -			     prescaler to generate the Serial =
Clock (Sclock).
>>>>>>> -			     Valid values are "oscillator" and =
"platform"
>>>>>>> -			     "oscillator": CAN engine clock =
source is
>>>>> oscillator clock.
>>>>>>> -			     "platform" The CAN engine clock =
source is the bus
>>>>> clock
>>>>>>> -		             (platform clock).
>>>>>>> -
>>>>>>> -- fsl,flexcan-clock-divider : for the reference and system =
clock, an
>>>>> additional
>>>>>>> -			      clock divider can be specified.
>>>>>>> -- clock-frequency: frequency required to calculate the bitrate =
for
>>>>> FlexCAN.
>>>>>>> -
>>>>>>> -Note:
>>>>>>> -	- v1.0 of flexcan-v1.0 represent the IP block version =
for P1010
>>>>> SOC.
>>>>>>> -	- P1010 does not have oscillator as the Clock Source.So =
the default
>>>>>>> -	  Clock Source is platform clock.
>>>>>>> -Examples:
>>>>>>> -
>>>>>>> -	can0@1c000 {
>>>>>>> -		compatible =3D "fsl,flexcan-v1.0";
>>>>>>> -		reg =3D <0x1c000 0x1000>;
>>>>>>> -		interrupts =3D <48 0x2>;
>>>>>>> -		interrupt-parent =3D <&mpic>;
>>>>>>> -		fsl,flexcan-clock-source =3D "platform";
>>>>>>> -		fsl,flexcan-clock-divqider =3D <2>;
>>>>>>> -		clock-frequency =3D <fixed by u-boot>;
>>>>>>> -	};
>>>>>>=20
>>>>>> Do we really want to drop the documentation for that binding. I =
think
>>>>>> something like the following text would be still useful:
>>>>>>=20
>>>>>> ------------------------
>>>>>> Flexcan CAN contoller on Freescale's ARM and PowerPC processors
>>>>>>=20
>>>>>> Required properties:
>>>>>>=20
>>>>>> - compatible : Should be "fsl,flexcan" and optionally
>>>>>>              "fsl,flexcan-<processor>"
>>>>>> - reg : Offset and length of the register set for this device
>>>>>> - interrupts : Interrupt tuple for this device
>>>>>>=20
>>>>>> Example:
>>>>>>=20
>>>>>> can@1c000 {
>>>>>>         compatible =3D "fsl,p1010-flexcan", "fsl,flexcan";
>>>>>>         reg =3D <0x1c000 0x1000>;
>>>>>>         interrupts =3D <48 0x2>;
>>>>>>         interrupt-parent =3D <&mpic>;
>>>>>> };
>>>>>> -------------------------
>>>>>=20
>>>>> Done, except the
>>>>>>         compatible =3D "fsl,p1010-flexcan", "fsl,flexcan";
>>>>>=20
>>>>> line is
>>>>> 	compatible =3D "fsl,flexcan", "fsl,flexcan-p1010";
>>>>>=20
>>>>>>=20
>>>>>> What do you think?
>>>>>>=20
>>>>>>> diff --git a/arch/powerpc/boot/dts/p1010rdb.dts
>>>>>>> b/arch/powerpc/boot/dts/p1010rdb.dts
>>>>>>> index 6b33b73..d6a0bb2 100644
>>>>>>> --- a/arch/powerpc/boot/dts/p1010rdb.dts
>>>>>>> +++ b/arch/powerpc/boot/dts/p1010rdb.dts
>>>>>>> @@ -169,14 +169,6 @@
>>>>>>> 			};
>>>>>>> 		};
>>>>>>>=20
>>>>>>> -		can0@1c000 {
>>>>>>> -			fsl,flexcan-clock-source =3D "platform";
>>>>>>> -		};
>>>>>>> -
>>>>>>> -		can1@1d000 {
>>>>>>> -			fsl,flexcan-clock-source =3D "platform";
>>>>>>> -		};
>>>>>>> -
>>>>>>> 		usb@22000 {
>>>>>>> 			phy_type =3D "utmi";
>>>>>>> 		};
>>>>>>> diff --git a/arch/powerpc/boot/dts/p1010si.dtsi
>>>>>>> b/arch/powerpc/boot/dts/p1010si.dtsi
>>>>>>> index 7f51104..20c396d 100644
>>>>>>> --- a/arch/powerpc/boot/dts/p1010si.dtsi
>>>>>>> +++ b/arch/powerpc/boot/dts/p1010si.dtsi
>>>>>>> @@ -141,19 +141,19 @@
>>>>>>> 		};
>>>>>>>=20
>>>>>>> 		can0@1c000 {
>>>>>>> -			compatible =3D "fsl,flexcan-v1.0";
>>>>>>> +			compatible =3D "fsl,p1010-flexcan",
>>>>>>> +					"fsl,flexcan";
>>>>>>=20
>>>>>> Does fit on one line.
>>>>>>=20
>>>>>>> 			reg =3D <0x1c000 0x1000>;
>>>>>>> 			interrupts =3D <48 0x2>;
>>>>>>> 			interrupt-parent =3D <&mpic>;
>>>>>>> -			fsl,flexcan-clock-divider =3D <2>;
>>>>>>> 		};
>>>>>>>=20
>>>>>>> 		can1@1d000 {
>>>>>>> -			compatible =3D "fsl,flexcan-v1.0";
>>>>>>> +			compatible =3D "fsl,p1010-flexcan",
>>>>>>> +					"fsl,flexcan";
>>>>>>=20
>>>>>> Ditto
>>>>>>=20
>>>>>>> 			reg =3D <0x1d000 0x1000>;
>>>>>>> 			interrupts =3D <61 0x2>;
>>>>>>> 			interrupt-parent =3D <&mpic>;
>>>>>>> -			fsl,flexcan-clock-divider =3D <2>;
>>>>>>> 		};
>>>>>>>=20
>>>>>>> 		L2: l2-cache-controller@20000 {
>>>>>>=20
>>>>>> Please also correct the node names (not using the number suffix).
>>>>>=20
>>>>> So the node names should be
>>>>> 		can@1c000 {
>>>>> 		can@1d000 {
>>>>> correct?
>>>>>=20
>>>> [Bhaskar] As there are two CAN controllers on P1010,So won't it be =
better
>>>> 	    to distinguish it by can0 and can1 instead by simple "can" ?
>>>=20
>>> It looks like the way to do that is to assign a label to those =
devices
>>> and then associate the label with an alias.  I have no idea how that
>>> works under the hood, but it is the way other files are set up.  =
Take a
>>> look at arch/powerpc/boot/dts/bamboo.dts for how they define the =
serial
>>> interfaces.
>>>=20
>>> Grant or Wolfgang, is that the right way to handle the concern about
>>> names or does it have no practical effect with the Linux kernel?
>>=20
>> It has not effect.  The label is just if you need to reference it via =
some other means.
>=20
> Does the alias have an effect?

nope

- k

Re: [PATCH v10 5/5] [powerpc] Fix up fsl-flexcan device tree binding.

[Robin Holt]

On Wed, Aug 10, 2011 at 12:17:45PM -0500, Kumar Gala wrote:
> 
> On Aug 10, 2011, at 12:16 PM, Robin Holt wrote:
> 
> > On Wed, Aug 10, 2011 at 11:53:15AM -0500, Kumar Gala wrote:
> >> 
> >> On Aug 10, 2011, at 11:00 AM, Robin Holt wrote:
> >> 
> >>> On Wed, Aug 10, 2011 at 02:36:20PM +0000, U Bhaskar-B22300 wrote:
> >>>> 
> >>>> 
> >>>>> -----Original Message-----
> >>>>> From: Robin Holt [mailto:holt@sgi.com]
> >>>>> Sent: Wednesday, August 10, 2011 7:46 PM
> >>>>> To: Wolfgang Grandegger
> >>>>> Cc: Robin Holt; Marc Kleine-Budde; U Bhaskar-B22300; Wood Scott-B07421;
> >>>>> netdev@vger.kernel.org; Kumar Gala; socketcan-core@lists.berlios.de; PPC
> >>>>> list
> >>>>> Subject: Re: [PATCH v10 5/5] [powerpc] Fix up fsl-flexcan device tree
> >>>>> binding.
> >>>>> 
> >>>>> On Wed, Aug 10, 2011 at 03:47:43PM +0200, Wolfgang Grandegger wrote:
> >>>>>> Hi Robin,
> >>>>>> 
> >>>>>> On 08/10/2011 05:06 AM, Robin Holt wrote:
> >>>>>>> In working with the socketcan developers, we have come to the
> >>>>>>> conclusion the Documentation...fsl-flexcan.txt device tree
> >>>>>>> documentation needs to be cleaned up.  The driver does not depend
> >>>>>>> upon any properties other
> >>>>>> 
> >>>>>> Your first sentence could be misleading. Please just describe what the
> >>>>>> patch does and why, something like:
> >>>>>> 
> >>>>>> "This patch cleans up the documentation of the device-tree binding for
> >>>>>> the Flexcan devices on Freescale's PowerPC and ARM cores. Extra
> >>>>>> properties are not needed as the frequency of the source clock is
> >>>>>> fixed..." and so on.
> >>>>> 
> >>>>> I borrowed heavily from your message. ;)
> >>>>> 
> >>>>>>> than the required properties so we are removing the file.
> >>>>>>> Additionally, the p1010*dts* files are not following the standard
> >>>>>>> for node naming in that they have a trailing -v1.0.
> >>>>>> 
> >>>>>>> Signed-off-by: Robin Holt <holt@sgi.com>
> >>>>>>> To: Marc Kleine-Budde <mkl@pengutronix.de>,
> >>>>>>> To: Wolfgang Grandegger <wg@grandegger.com>,
> >>>>>>> To: U Bhaskar-B22300 <B22300@freescale.com>
> >>>>>>> To: Scott Wood <scottwood@freescale.com>
> >>>>>>> Cc: socketcan-core@lists.berlios.de,
> >>>>>>> Cc: netdev@vger.kernel.org,
> >>>>>>> Cc: PPC list <linuxppc-dev@lists.ozlabs.org>
> >>>>>>> Cc: Kumar Gala <galak@kernel.crashing.org>
> >>>>>>> ---
> >>>>>>> .../devicetree/bindings/net/can/fsl-flexcan.txt    |   61 ----------
> >>>>> ----------
> >>>>>>> arch/powerpc/boot/dts/p1010rdb.dts                 |    8 ---
> >>>>>>> arch/powerpc/boot/dts/p1010si.dtsi                 |    8 +-
> >>>>>>> 3 files changed, 4 insertions(+), 73 deletions(-)  delete mode
> >>>>>>> 100644 Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> >>>>>>> 
> >>>>>>> diff --git
> >>>>>>> a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> >>>>>>> b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> >>>>>>> deleted file mode 100644
> >>>>>>> index 1a729f0..0000000
> >>>>>>> --- a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> >>>>>>> +++ /dev/null
> >>>>>>> @@ -1,61 +0,0 @@
> >>>>>>> -CAN Device Tree Bindings
> >>>>>>> -------------------------
> >>>>>>> -2011 Freescale Semiconductor, Inc.
> >>>>>>> -
> >>>>>>> -fsl,flexcan-v1.0 nodes
> >>>>>>> ------------------------
> >>>>>>> -In addition to the required compatible-, reg- and
> >>>>>>> interrupt-properties, you can -also specify which clock source shall
> >>>>> be used for the controller.
> >>>>>>> -
> >>>>>>> -CPI Clock- Can Protocol Interface Clock
> >>>>>>> -	This CLK_SRC bit of CTRL(control register) selects the clock source
> >>>>> to
> >>>>>>> -	the CAN Protocol Interface(CPI) to be either the peripheral clock
> >>>>>>> -	(driven by the PLL) or the crystal oscillator clock. The selected
> >>>>> clock
> >>>>>>> -	is the one fed to the prescaler to generate the Serial Clock
> >>>>> (Sclock).
> >>>>>>> -	The PRESDIV field of CTRL(control register) controls a prescaler
> >>>>> that
> >>>>>>> -	generates the Serial Clock (Sclock), whose period defines the
> >>>>>>> -	time quantum used to compose the CAN waveform.
> >>>>>>> -
> >>>>>>> -Can Engine Clock Source
> >>>>>>> -	There are two sources for CAN clock
> >>>>>>> -	- Platform Clock  It represents the bus clock
> >>>>>>> -	- Oscillator Clock
> >>>>>>> -
> >>>>>>> -	Peripheral Clock (PLL)
> >>>>>>> -	--------------
> >>>>>>> -		     |
> >>>>>>> -		    ---------		      -------------
> >>>>>>> -		    |       |CPI Clock	      | Prescaler |       Sclock
> >>>>>>> -		    |       |---------------->| (1.. 256) |------------>
> >>>>>>> -		    ---------		      -------------
> >>>>>>> -                     |  |
> >>>>>>> -	--------------  ---------------------CLK_SRC
> >>>>>>> -	Oscillator Clock
> >>>>>>> -
> >>>>>>> -- fsl,flexcan-clock-source : CAN Engine Clock Source.This property
> >>>>> selects
> >>>>>>> -			     the peripheral clock. PLL clock is fed to the
> >>>>>>> -			     prescaler to generate the Serial Clock (Sclock).
> >>>>>>> -			     Valid values are "oscillator" and "platform"
> >>>>>>> -			     "oscillator": CAN engine clock source is
> >>>>> oscillator clock.
> >>>>>>> -			     "platform" The CAN engine clock source is the bus
> >>>>> clock
> >>>>>>> -		             (platform clock).
> >>>>>>> -
> >>>>>>> -- fsl,flexcan-clock-divider : for the reference and system clock, an
> >>>>> additional
> >>>>>>> -			      clock divider can be specified.
> >>>>>>> -- clock-frequency: frequency required to calculate the bitrate for
> >>>>> FlexCAN.
> >>>>>>> -
> >>>>>>> -Note:
> >>>>>>> -	- v1.0 of flexcan-v1.0 represent the IP block version for P1010
> >>>>> SOC.
> >>>>>>> -	- P1010 does not have oscillator as the Clock Source.So the default
> >>>>>>> -	  Clock Source is platform clock.
> >>>>>>> -Examples:
> >>>>>>> -
> >>>>>>> -	can0@1c000 {
> >>>>>>> -		compatible = "fsl,flexcan-v1.0";
> >>>>>>> -		reg = <0x1c000 0x1000>;
> >>>>>>> -		interrupts = <48 0x2>;
> >>>>>>> -		interrupt-parent = <&mpic>;
> >>>>>>> -		fsl,flexcan-clock-source = "platform";
> >>>>>>> -		fsl,flexcan-clock-divqider = <2>;
> >>>>>>> -		clock-frequency = <fixed by u-boot>;
> >>>>>>> -	};
> >>>>>> 
> >>>>>> Do we really want to drop the documentation for that binding. I think
> >>>>>> something like the following text would be still useful:
> >>>>>> 
> >>>>>> ------------------------
> >>>>>> Flexcan CAN contoller on Freescale's ARM and PowerPC processors
> >>>>>> 
> >>>>>> Required properties:
> >>>>>> 
> >>>>>> - compatible : Should be "fsl,flexcan" and optionally
> >>>>>>              "fsl,flexcan-<processor>"
> >>>>>> - reg : Offset and length of the register set for this device
> >>>>>> - interrupts : Interrupt tuple for this device
> >>>>>> 
> >>>>>> Example:
> >>>>>> 
> >>>>>> can@1c000 {
> >>>>>>         compatible = "fsl,p1010-flexcan", "fsl,flexcan";
> >>>>>>         reg = <0x1c000 0x1000>;
> >>>>>>         interrupts = <48 0x2>;
> >>>>>>         interrupt-parent = <&mpic>;
> >>>>>> };
> >>>>>> -------------------------
> >>>>> 
> >>>>> Done, except the
> >>>>>>         compatible = "fsl,p1010-flexcan", "fsl,flexcan";
> >>>>> 
> >>>>> line is
> >>>>> 	compatible = "fsl,flexcan", "fsl,flexcan-p1010";
> >>>>> 
> >>>>>> 
> >>>>>> What do you think?
> >>>>>> 
> >>>>>>> diff --git a/arch/powerpc/boot/dts/p1010rdb.dts
> >>>>>>> b/arch/powerpc/boot/dts/p1010rdb.dts
> >>>>>>> index 6b33b73..d6a0bb2 100644
> >>>>>>> --- a/arch/powerpc/boot/dts/p1010rdb.dts
> >>>>>>> +++ b/arch/powerpc/boot/dts/p1010rdb.dts
> >>>>>>> @@ -169,14 +169,6 @@
> >>>>>>> 			};
> >>>>>>> 		};
> >>>>>>> 
> >>>>>>> -		can0@1c000 {
> >>>>>>> -			fsl,flexcan-clock-source = "platform";
> >>>>>>> -		};
> >>>>>>> -
> >>>>>>> -		can1@1d000 {
> >>>>>>> -			fsl,flexcan-clock-source = "platform";
> >>>>>>> -		};
> >>>>>>> -
> >>>>>>> 		usb@22000 {
> >>>>>>> 			phy_type = "utmi";
> >>>>>>> 		};
> >>>>>>> diff --git a/arch/powerpc/boot/dts/p1010si.dtsi
> >>>>>>> b/arch/powerpc/boot/dts/p1010si.dtsi
> >>>>>>> index 7f51104..20c396d 100644
> >>>>>>> --- a/arch/powerpc/boot/dts/p1010si.dtsi
> >>>>>>> +++ b/arch/powerpc/boot/dts/p1010si.dtsi
> >>>>>>> @@ -141,19 +141,19 @@
> >>>>>>> 		};
> >>>>>>> 
> >>>>>>> 		can0@1c000 {
> >>>>>>> -			compatible = "fsl,flexcan-v1.0";
> >>>>>>> +			compatible = "fsl,p1010-flexcan",
> >>>>>>> +					"fsl,flexcan";
> >>>>>> 
> >>>>>> Does fit on one line.
> >>>>>> 
> >>>>>>> 			reg = <0x1c000 0x1000>;
> >>>>>>> 			interrupts = <48 0x2>;
> >>>>>>> 			interrupt-parent = <&mpic>;
> >>>>>>> -			fsl,flexcan-clock-divider = <2>;
> >>>>>>> 		};
> >>>>>>> 
> >>>>>>> 		can1@1d000 {
> >>>>>>> -			compatible = "fsl,flexcan-v1.0";
> >>>>>>> +			compatible = "fsl,p1010-flexcan",
> >>>>>>> +					"fsl,flexcan";
> >>>>>> 
> >>>>>> Ditto
> >>>>>> 
> >>>>>>> 			reg = <0x1d000 0x1000>;
> >>>>>>> 			interrupts = <61 0x2>;
> >>>>>>> 			interrupt-parent = <&mpic>;
> >>>>>>> -			fsl,flexcan-clock-divider = <2>;
> >>>>>>> 		};
> >>>>>>> 
> >>>>>>> 		L2: l2-cache-controller@20000 {
> >>>>>> 
> >>>>>> Please also correct the node names (not using the number suffix).
> >>>>> 
> >>>>> So the node names should be
> >>>>> 		can@1c000 {
> >>>>> 		can@1d000 {
> >>>>> correct?
> >>>>> 
> >>>> [Bhaskar] As there are two CAN controllers on P1010,So won't it be better
> >>>> 	    to distinguish it by can0 and can1 instead by simple "can" ?
> >>> 
> >>> It looks like the way to do that is to assign a label to those devices
> >>> and then associate the label with an alias.  I have no idea how that
> >>> works under the hood, but it is the way other files are set up.  Take a
> >>> look at arch/powerpc/boot/dts/bamboo.dts for how they define the serial
> >>> interfaces.
> >>> 
> >>> Grant or Wolfgang, is that the right way to handle the concern about
> >>> names or does it have no practical effect with the Linux kernel?
> >> 
> >> It has not effect.  The label is just if you need to reference it via some other means.
> > 
> > Does the alias have an effect?
> 
> nope

Then how does the device number get associated with a particular device
and how is user-space ensured a consistent namespace?

Robin

Re: [PATCH v10 5/5] [powerpc] Fix up fsl-flexcan device tree binding.

[Kumar Gala]

>>>>>>> So the node names should be
>>>>>>> 		can@1c000 {
>>>>>>> 		can@1d000 {
>>>>>>> correct?
>>>>>>>=20
>>>>>> [Bhaskar] As there are two CAN controllers on P1010,So won't it =
be better
>>>>>> 	    to distinguish it by can0 and can1 instead by simple "can" ?
>>>>>=20
>>>>> It looks like the way to do that is to assign a label to those =
devices
>>>>> and then associate the label with an alias.  I have no idea how =
that
>>>>> works under the hood, but it is the way other files are set up.  =
Take a
>>>>> look at arch/powerpc/boot/dts/bamboo.dts for how they define the =
serial
>>>>> interfaces.
>>>>>=20
>>>>> Grant or Wolfgang, is that the right way to handle the concern =
about
>>>>> names or does it have no practical effect with the Linux kernel?
>>>>=20
>>>> It has not effect.  The label is just if you need to reference it =
via some other means.
>>>=20
>>> Does the alias have an effect?
>>=20
>> nope
>=20
> Then how does the device number get associated with a particular =
device

What do you mean by device number?

> and how is user-space ensured a consistent namespace?

that is left to udev rules.

- k=

Re: [PATCH v10 5/5] [powerpc] Fix up fsl-flexcan device tree binding.

[Wolfgang Grandegger]

On 08/10/2011 06:00 PM, Robin Holt wrote:
> On Wed, Aug 10, 2011 at 02:36:20PM +0000, U Bhaskar-B22300 wrote:
...
> It looks like the way to do that is to assign a label to those devices
> and then associate the label with an alias.  I have no idea how that
> works under the hood, but it is the way other files are set up.  Take a
> look at arch/powerpc/boot/dts/bamboo.dts for how they define the serial
> interfaces.

With a label you mean "label:" at the beginning of a node. Such labels
are translated by the device tree compiler in node handles, which can be
referenced within nodes by using <&label>, e.g.:

UIC0: interrupt-controller0 {
	...
};
UIC1: interrupt-controller1 {
	...
	interrupt-parent = <&UIC0>;
	...
};

It has nothing to do with the name of the node.

Wolfgang.

Re: [PATCH v10 5/5] [powerpc] Fix up fsl-flexcan device tree binding.

[Scott Wood]

On 08/10/2011 01:23 PM, Wolfgang Grandegger wrote:
> On 08/10/2011 06:00 PM, Robin Holt wrote:
>> On Wed, Aug 10, 2011 at 02:36:20PM +0000, U Bhaskar-B22300 wrote:
> ...
>> It looks like the way to do that is to assign a label to those devices
>> and then associate the label with an alias.  I have no idea how that
>> works under the hood, but it is the way other files are set up.  Take a
>> look at arch/powerpc/boot/dts/bamboo.dts for how they define the serial
>> interfaces.
> 
> With a label you mean "label:" at the beginning of a node. Such labels
> are translated by the device tree compiler in node handles, which can be
> referenced within nodes by using <&label>, e.g.:
> 
> UIC0: interrupt-controller0 {
> 	...
> };
> UIC1: interrupt-controller1 {
> 	...
> 	interrupt-parent = <&UIC0>;
> 	...
> };
> 
> It has nothing to do with the name of the node.

"...and then associate the label with an alias."

The alias can then be used if you want "can0" versus "can1".

Appending numbers to the node name is typically only done when there's
no unit address, and a need to disambiguate.

-Scott

Re: [PATCH v10 5/5] [powerpc] Fix up fsl-flexcan device tree binding.

[Robin Holt]

On Wed, Aug 10, 2011 at 01:27:52PM -0500, Scott Wood wrote:
> On 08/10/2011 01:23 PM, Wolfgang Grandegger wrote:
> > On 08/10/2011 06:00 PM, Robin Holt wrote:
> >> On Wed, Aug 10, 2011 at 02:36:20PM +0000, U Bhaskar-B22300 wrote:
> > ...
> >> It looks like the way to do that is to assign a label to those devices
> >> and then associate the label with an alias.  I have no idea how that
> >> works under the hood, but it is the way other files are set up.  Take a
> >> look at arch/powerpc/boot/dts/bamboo.dts for how they define the serial
> >> interfaces.
> > 
> > With a label you mean "label:" at the beginning of a node. Such labels
> > are translated by the device tree compiler in node handles, which can be
> > referenced within nodes by using <&label>, e.g.:
> > 
> > UIC0: interrupt-controller0 {
> > 	...
> > };
> > UIC1: interrupt-controller1 {
> > 	...
> > 	interrupt-parent = <&UIC0>;
> > 	...
> > };
> > 
> > It has nothing to do with the name of the node.
> 
> "...and then associate the label with an alias."
> 
> The alias can then be used if you want "can0" versus "can1".

Does the alias get used by either the kernel or something else or is it
just extra detail with no purpose?

Robin

Re: [PATCH v10 5/5] [powerpc] Fix up fsl-flexcan device tree binding.

[Scott Wood]

On 08/10/2011 01:35 PM, Robin Holt wrote:
> On Wed, Aug 10, 2011 at 01:27:52PM -0500, Scott Wood wrote:
>> "...and then associate the label with an alias."
>>
>> The alias can then be used if you want "can0" versus "can1".
> 
> Does the alias get used by either the kernel or something else or is it
> just extra detail with no purpose?

It could be used by udev to produce a symlink.  Currently, the major
non-human consumer of the aliases is U-Boot.

-Scott

Re: [PATCH v10 5/5] [powerpc] Fix up fsl-flexcan device tree binding.

[Wolfgang Grandegger]

On 08/10/2011 04:15 PM, Robin Holt wrote:
> On Wed, Aug 10, 2011 at 03:47:43PM +0200, Wolfgang Grandegger wrote:
...
> Done, except the
>>           compatible = "fsl,p1010-flexcan", "fsl,flexcan";
> 
> line is
> 	compatible = "fsl,flexcan", "fsl,flexcan-p1010";

IIRC, there order is more to less specific, e.g. for I2C:

  compatible = "fsl,mpc5200-i2c", "fsl-i2c"

...

>> Please also correct the node names (not using the number suffix).
> 
> So the node names should be
> 		can@1c000 {
> 		can@1d000 {
> correct?

Yes, just have a look how other node names are constructed, e.g. for
sata, serial, rtc, etc.:

  $ grep serial@ *.dts
  ...
  pdm360ng.dts:		serial@11000 {
  pdm360ng.dts:		serial@11100 {
  pdm360ng.dts:		serial@11200 {
  pdm360ng.dts:		serial@11300 {
  pdm360ng.dts:		serial@11400 {
  pdm360ng.dts:		serial@11600 {
  pdm360ng.dts:		serial@11800 {
  pdm360ng.dts:		serial@11B00 {
  ...

Wolfgang.

Re: [PATCH v10 5/5] [powerpc] Fix up fsl-flexcan device tree binding.

[Kumar Gala]

On Aug 10, 2011, at 9:41 AM, Wolfgang Grandegger wrote:

> On 08/10/2011 04:15 PM, Robin Holt wrote:
>> On Wed, Aug 10, 2011 at 03:47:43PM +0200, Wolfgang Grandegger wrote:
> ...
>> Done, except the
>>>          compatible = "fsl,p1010-flexcan", "fsl,flexcan";
>> 
>> line is
>> 	compatible = "fsl,flexcan", "fsl,flexcan-p1010";
> 
> IIRC, there order is more to less specific, e.g. for I2C:
> 
>  compatible = "fsl,mpc5200-i2c", "fsl-i2c"
> 
> ...
> 
>>> Please also correct the node names (not using the number suffix).
>> 
>> So the node names should be
>> 		can@1c000 {
>> 		can@1d000 {
>> correct?
> 
> Yes, just have a look how other node names are constructed, e.g. for
> sata, serial, rtc, etc.:
> 
>  $ grep serial@ *.dts
>  ...
>  pdm360ng.dts:		serial@11000 {
>  pdm360ng.dts:		serial@11100 {
>  pdm360ng.dts:		serial@11200 {
>  pdm360ng.dts:		serial@11300 {
>  pdm360ng.dts:		serial@11400 {
>  pdm360ng.dts:		serial@11600 {
>  pdm360ng.dts:		serial@11800 {
>  pdm360ng.dts:		serial@11B00 {
>  ...
> 
> Wolfgang.

Agree w/Wolfgang here, can@1c000, can@1d000 is what we should use.

- k

Re: [PATCH v10 5/5] [powerpc] Fix up fsl-flexcan device tree binding.

[Kumar Gala]

On Aug 10, 2011, at 9:15 AM, Robin Holt wrote:

> On Wed, Aug 10, 2011 at 03:47:43PM +0200, Wolfgang Grandegger wrote:
>> Hi Robin,
>>=20
>> On 08/10/2011 05:06 AM, Robin Holt wrote:
>>> In working with the socketcan developers, we have come to the =
conclusion
>>> the Documentation...fsl-flexcan.txt device tree documentation needs =
to
>>> be cleaned up.  The driver does not depend upon any properties other
>>=20
>> Your first sentence could be misleading. Please just describe what =
the
>> patch does and why, something like:
>>=20
>> "This patch cleans up the documentation of the device-tree binding =
for
>> the Flexcan devices on Freescale's PowerPC and ARM cores. Extra
>> properties are not needed as the frequency of the source clock is
>> fixed..." and so on.
>=20
> I borrowed heavily from your message. ;)
>=20
>>> than the required properties so we are removing the file.  =
Additionally,
>>> the p1010*dts* files are not following the standard for node naming =
in
>>> that they have a trailing -v1.0.
>>=20
>>> Signed-off-by: Robin Holt <holt@sgi.com>
>>> To: Marc Kleine-Budde <mkl@pengutronix.de>,
>>> To: Wolfgang Grandegger <wg@grandegger.com>,
>>> To: U Bhaskar-B22300 <B22300@freescale.com>
>>> To: Scott Wood <scottwood@freescale.com>
>>> Cc: socketcan-core@lists.berlios.de,
>>> Cc: netdev@vger.kernel.org,
>>> Cc: PPC list <linuxppc-dev@lists.ozlabs.org>
>>> Cc: Kumar Gala <galak@kernel.crashing.org>
>>> ---
>>> .../devicetree/bindings/net/can/fsl-flexcan.txt    |   61 =
--------------------
>>> arch/powerpc/boot/dts/p1010rdb.dts                 |    8 ---
>>> arch/powerpc/boot/dts/p1010si.dtsi                 |    8 +-
>>> 3 files changed, 4 insertions(+), 73 deletions(-)
>>> delete mode 100644 =
Documentation/devicetree/bindings/net/can/fsl-flexcan.txt

I don't understand how we can do this?  What binding spec covers the =
P1010 CAN support if you remove this?

- k

Re: [PATCH v10 5/5] [powerpc] Fix up fsl-flexcan device tree binding.

[Robin Holt]

On Wed, Aug 10, 2011 at 09:45:17AM -0500, Kumar Gala wrote:
> 
> On Aug 10, 2011, at 9:15 AM, Robin Holt wrote:
> 
> > On Wed, Aug 10, 2011 at 03:47:43PM +0200, Wolfgang Grandegger wrote:
> >> Hi Robin,
> >> 
> >> On 08/10/2011 05:06 AM, Robin Holt wrote:
> >>> In working with the socketcan developers, we have come to the conclusion
> >>> the Documentation...fsl-flexcan.txt device tree documentation needs to
> >>> be cleaned up.  The driver does not depend upon any properties other
> >> 
> >> Your first sentence could be misleading. Please just describe what the
> >> patch does and why, something like:
> >> 
> >> "This patch cleans up the documentation of the device-tree binding for
> >> the Flexcan devices on Freescale's PowerPC and ARM cores. Extra
> >> properties are not needed as the frequency of the source clock is
> >> fixed..." and so on.
> > 
> > I borrowed heavily from your message. ;)
> > 
> >>> than the required properties so we are removing the file.  Additionally,
> >>> the p1010*dts* files are not following the standard for node naming in
> >>> that they have a trailing -v1.0.
> >> 
> >>> Signed-off-by: Robin Holt <holt@sgi.com>
> >>> To: Marc Kleine-Budde <mkl@pengutronix.de>,
> >>> To: Wolfgang Grandegger <wg@grandegger.com>,
> >>> To: U Bhaskar-B22300 <B22300@freescale.com>
> >>> To: Scott Wood <scottwood@freescale.com>
> >>> Cc: socketcan-core@lists.berlios.de,
> >>> Cc: netdev@vger.kernel.org,
> >>> Cc: PPC list <linuxppc-dev@lists.ozlabs.org>
> >>> Cc: Kumar Gala <galak@kernel.crashing.org>
> >>> ---
> >>> .../devicetree/bindings/net/can/fsl-flexcan.txt    |   61 --------------------
> >>> arch/powerpc/boot/dts/p1010rdb.dts                 |    8 ---
> >>> arch/powerpc/boot/dts/p1010si.dtsi                 |    8 +-
> >>> 3 files changed, 4 insertions(+), 73 deletions(-)
> >>> delete mode 100644 Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> 
> I don't understand how we can do this?  What binding spec covers the P1010 CAN support if you remove this?

We have added it back in based upon an earlier comment from Wolfgang.
I will post a new version shortly.

Robin

Re: [PATCH v10 0/5] [flexcan/powerpc] Add support for powerpc flexcan (freescale p1010)

[Marc Kleine-Budde]

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

On 08/10/2011 05:05 AM, Robin Holt wrote:
> 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 forwarding to David S. Miller for the netdev tree.  I think patch
> 4 is ready for submission to the PPC85xx maintainer.  Patch 5 changed
> from the previous post by adding a second compatible string for the
> fsl,p1010_flexcan.

One remark for the subjects. It seems no one is using "[$subsystem]"
anymore, but rather "$subsystem:".

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

Re: [PATCH v10 0/5] [flexcan/powerpc] Add support for powerpc flexcan (freescale p1010)

[Robin Holt]

On Wed, Aug 10, 2011 at 12:01:58PM +0200, Marc Kleine-Budde wrote:
> On 08/10/2011 05:05 AM, Robin Holt wrote:
> > 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 forwarding to David S. Miller for the netdev tree.  I think patch
> > 4 is ready for submission to the PPC85xx maintainer.  Patch 5 changed
> > from the previous post by adding a second compatible string for the
> > fsl,p1010_flexcan.
> 
> One remark for the subjects. It seems no one is using "[$subsystem]"
> anymore, but rather "$subsystem:".

I will, one day, get these patches subject lines correct ;)

Did you happen to get a chance to test the patches on arm?

Thanks,
Robin

Re: [PATCH v10 0/5] [flexcan/powerpc] Add support for powerpc flexcan (freescale p1010)

[Marc Kleine-Budde]

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

On 08/10/2011 12:11 PM, Robin Holt wrote:
> On Wed, Aug 10, 2011 at 12:01:58PM +0200, Marc Kleine-Budde wrote:
>> On 08/10/2011 05:05 AM, Robin Holt wrote:
>>> 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 forwarding to David S. Miller for the netdev tree.  I think patch
>>> 4 is ready for submission to the PPC85xx maintainer.  Patch 5 changed
>>> from the previous post by adding a second compatible string for the
>>> fsl,p1010_flexcan.
>>
>> One remark for the subjects. It seems no one is using "[$subsystem]"
>> anymore, but rather "$subsystem:".
> 
> I will, one day, get these patches subject lines correct ;)
> 
> Did you happen to get a chance to test the patches on arm?

It's still compiling...

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


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

Re: [PATCH v10 0/5] [flexcan/powerpc] Add support for powerpc flexcan (freescale p1010)

[Marc Kleine-Budde]

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

On 08/10/2011 05:05 AM, Robin Holt wrote:
> 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 forwarding to David S. Miller for the netdev tree.  I think patch
> 4 is ready for submission to the PPC85xx maintainer.  Patch 5 changed
> from the previous post by adding a second compatible string for the
> fsl,p1010_flexcan.

This patch series is working on phytec's pcm043 (mx35) based on current
net-next. Feel free to add my Acked-by to all patches.

good work,
Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


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