* [Patch 0/5] [flexcan/powerpc] Add support for powerpc flexcan (freescale p1010) -V9
@ 2011-08-09 14:43 Robin Holt
2011-08-09 14:43 ` [PATCH 1/5] [flexcan] Remove #include <mach/clock.h> Robin Holt
` (4 more replies)
0 siblings, 5 replies; 16+ messages in thread
From: Robin Holt @ 2011-08-09 14:43 UTC (permalink / raw)
To: Robin Holt, Marc Kleine-Budde, Wolfgang Grandegger,
U Bhaskar-B22300
Cc: socketcan-core, netdev, PPC list, Robin Holt
Marc, Wolfgang or U Bhaskar,
This patch set should have all your comments included. It is based on
the David S. Miller net-next-2.6 tree commit 19fd617.
This series adds a fifth patch which cleans up and corrects the device
bindigs for the fsl-flexcan nodes.
I have compiled each patch in the series individually for both arm and
powerpc (cheated on ppc and reordered them with the last patch first so
I could select CAN_FLEXCAN.
With all the patches applied, my p1010rdb works for communicating between
its two can ports and also can communicate with an external PSOC. I have
done no testing beyond compile testing on an arm system as I have no
access to an arm based system.
For the first three patches in the series, I believe they are all ready
for forwarding to David S. Miller for the netdev tree. I think patch
4 is ready for submission to the PPC85xx maintainer. This is the
first submission of patch 5.
Thanks,
Robin Holt
^ permalink raw reply [flat|nested] 16+ messages in thread* [PATCH 1/5] [flexcan] Remove #include <mach/clock.h> 2011-08-09 14:43 [Patch 0/5] [flexcan/powerpc] Add support for powerpc flexcan (freescale p1010) -V9 Robin Holt @ 2011-08-09 14:43 ` Robin Holt 2011-08-09 14:43 ` [PATCH 2/5] [flexcan] Abstract off read/write for big/little endian Robin Holt ` (3 subsequent siblings) 4 siblings, 0 replies; 16+ messages in thread From: Robin Holt @ 2011-08-09 14:43 UTC (permalink / raw) To: Robin Holt, Marc Kleine-Budde, Wolfgang Grandegger, U Bhaskar-B22300 Cc: socketcan-core, netdev, PPC list, 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 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 2/5] [flexcan] Abstract off read/write for big/little endian. 2011-08-09 14:43 [Patch 0/5] [flexcan/powerpc] Add support for powerpc flexcan (freescale p1010) -V9 Robin Holt 2011-08-09 14:43 ` [PATCH 1/5] [flexcan] Remove #include <mach/clock.h> Robin Holt @ 2011-08-09 14:43 ` Robin Holt 2011-08-09 14:43 ` [PATCH 3/5] [flexcan] Add of_match to platform_device definition Robin Holt ` (2 subsequent siblings) 4 siblings, 0 replies; 16+ messages in thread From: Robin Holt @ 2011-08-09 14:43 UTC (permalink / raw) To: Robin Holt, Marc Kleine-Budde, Wolfgang Grandegger, U Bhaskar-B22300 Cc: netdev, socketcan-core, Robin Holt, PPC list 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(®s->mcr); + reg = flexcan_read(®s->mcr); reg &= ~FLEXCAN_MCR_MDIS; - writel(reg, ®s->mcr); + flexcan_write(reg, ®s->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(®s->mcr); + reg = flexcan_read(®s->mcr); reg |= FLEXCAN_MCR_MDIS; - writel(reg, ®s->mcr); + flexcan_write(reg, ®s->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(®s->ecr); + u32 reg = flexcan_read(®s->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, ®s->cantxfg[FLEXCAN_TX_BUF_ID].data[0]); + flexcan_write(data, ®s->cantxfg[FLEXCAN_TX_BUF_ID].data[0]); } if (cf->can_dlc > 3) { u32 data = be32_to_cpup((__be32 *)&cf->data[4]); - writel(data, ®s->cantxfg[FLEXCAN_TX_BUF_ID].data[1]); + flexcan_write(data, ®s->cantxfg[FLEXCAN_TX_BUF_ID].data[1]); } - writel(can_id, ®s->cantxfg[FLEXCAN_TX_BUF_ID].can_id); - writel(ctrl, ®s->cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl); + flexcan_write(can_id, ®s->cantxfg[FLEXCAN_TX_BUF_ID].can_id); + flexcan_write(ctrl, ®s->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 = ®s->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, ®s->iflag1); - readl(®s->timer); + flexcan_write(FLEXCAN_IFLAG_RX_FIFO_AVAILABLE, ®s->iflag1); + flexcan_read(®s->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(®s->esr) | priv->reg_esr; + reg_esr = flexcan_read(®s->esr) | priv->reg_esr; /* handle state changes */ work_done += flexcan_poll_state(dev, reg_esr); /* handle RX-FIFO */ - reg_iflag1 = readl(®s->iflag1); + reg_iflag1 = flexcan_read(®s->iflag1); while (reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_AVAILABLE && work_done < quota) { work_done += flexcan_read_frame(dev); - reg_iflag1 = readl(®s->iflag1); + reg_iflag1 = flexcan_read(®s->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, ®s->imask1); - writel(priv->reg_ctrl_default, ®s->ctrl); + flexcan_write(FLEXCAN_IFLAG_DEFAULT, ®s->imask1); + flexcan_write(priv->reg_ctrl_default, ®s->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(®s->iflag1); - reg_esr = readl(®s->esr); - writel(FLEXCAN_ESR_ERR_INT, ®s->esr); /* ACK err IRQ */ + reg_iflag1 = flexcan_read(®s->iflag1); + reg_esr = flexcan_read(®s->esr); + flexcan_write(FLEXCAN_ESR_ERR_INT, ®s->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, - ®s->imask1); - writel(priv->reg_ctrl_default & ~FLEXCAN_CTRL_ERR_ALL, + flexcan_write(FLEXCAN_IFLAG_DEFAULT & + ~FLEXCAN_IFLAG_RX_FIFO_AVAILABLE, ®s->imask1); + flexcan_write(priv->reg_ctrl_default & ~FLEXCAN_CTRL_ERR_ALL, ®s->ctrl); napi_schedule(&priv->napi); } /* FIFO overflow */ if (reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_OVERFLOW) { - writel(FLEXCAN_IFLAG_RX_FIFO_OVERFLOW, ®s->iflag1); + flexcan_write(FLEXCAN_IFLAG_RX_FIFO_OVERFLOW, ®s->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), ®s->iflag1); + flexcan_write((1 << FLEXCAN_TX_BUF_ID), ®s->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(®s->ctrl); + reg = flexcan_read(®s->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, ®s->ctrl); + flexcan_write(reg, ®s->ctrl); /* print chip status */ dev_dbg(dev->dev.parent, "%s: mcr=0x%08x ctrl=0x%08x\n", __func__, - readl(®s->mcr), readl(®s->ctrl)); + flexcan_read(®s->mcr), flexcan_read(®s->ctrl)); } /* @@ -648,10 +673,10 @@ static int flexcan_chip_start(struct net_device *dev) flexcan_chip_enable(priv); /* soft reset */ - writel(FLEXCAN_MCR_SOFTRST, ®s->mcr); + flexcan_write(FLEXCAN_MCR_SOFTRST, ®s->mcr); udelay(10); - reg_mcr = readl(®s->mcr); + reg_mcr = flexcan_read(®s->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(®s->mcr); + reg_mcr = flexcan_read(®s->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, ®s->mcr); + flexcan_write(reg_mcr, ®s->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(®s->ctrl); + reg_ctrl = flexcan_read(®s->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, ®s->ctrl); + flexcan_write(reg_ctrl, ®s->ctrl); for (i = 0; i < ARRAY_SIZE(regs->cantxfg); i++) { - writel(0, ®s->cantxfg[i].can_ctrl); - writel(0, ®s->cantxfg[i].can_id); - writel(0, ®s->cantxfg[i].data[0]); - writel(0, ®s->cantxfg[i].data[1]); + flexcan_write(0, ®s->cantxfg[i].can_ctrl); + flexcan_write(0, ®s->cantxfg[i].can_id); + flexcan_write(0, ®s->cantxfg[i].data[0]); + flexcan_write(0, ®s->cantxfg[i].data[1]); /* put MB into rx queue */ - writel(FLEXCAN_MB_CNT_CODE(0x4), ®s->cantxfg[i].can_ctrl); + flexcan_write(FLEXCAN_MB_CNT_CODE(0x4), + ®s->cantxfg[i].can_ctrl); } /* acceptance mask/acceptance code (accept everything) */ - writel(0x0, ®s->rxgmask); - writel(0x0, ®s->rx14mask); - writel(0x0, ®s->rx15mask); + flexcan_write(0x0, ®s->rxgmask); + flexcan_write(0x0, ®s->rx14mask); + flexcan_write(0x0, ®s->rx15mask); flexcan_transceiver_switch(priv, 1); /* synchronize with the can bus */ - reg_mcr = readl(®s->mcr); + reg_mcr = flexcan_read(®s->mcr); reg_mcr &= ~FLEXCAN_MCR_HALT; - writel(reg_mcr, ®s->mcr); + flexcan_write(reg_mcr, ®s->mcr); priv->can.state = CAN_STATE_ERROR_ACTIVE; /* enable FIFO interrupts */ - writel(FLEXCAN_IFLAG_DEFAULT, ®s->imask1); + flexcan_write(FLEXCAN_IFLAG_DEFAULT, ®s->imask1); /* print chip status */ dev_dbg(dev->dev.parent, "%s: reading mcr=0x%08x ctrl=0x%08x\n", - __func__, readl(®s->mcr), readl(®s->ctrl)); + __func__, flexcan_read(®s->mcr), flexcan_read(®s->ctrl)); return 0; @@ -757,12 +783,12 @@ static void flexcan_chip_stop(struct net_device *dev) u32 reg; /* Disable all interrupts */ - writel(0, ®s->imask1); + flexcan_write(0, ®s->imask1); /* Disable + halt module */ - reg = readl(®s->mcr); + reg = flexcan_read(®s->mcr); reg |= FLEXCAN_MCR_MDIS | FLEXCAN_MCR_HALT; - writel(reg, ®s->mcr); + flexcan_write(reg, ®s->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(®s->ctrl); + reg = flexcan_read(®s->ctrl); reg |= FLEXCAN_CTRL_CLK_SRC; - writel(reg, ®s->ctrl); + flexcan_write(reg, ®s->ctrl); flexcan_chip_enable(priv); /* set freeze, halt and activate FIFO, restrict register access */ - reg = readl(®s->mcr); + reg = flexcan_read(®s->mcr); reg |= FLEXCAN_MCR_FRZ | FLEXCAN_MCR_HALT | FLEXCAN_MCR_FEN | FLEXCAN_MCR_SUPV; - writel(reg, ®s->mcr); + flexcan_write(reg, ®s->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(®s->mcr); + reg = flexcan_read(®s->mcr); if (!(reg & FLEXCAN_MCR_FEN)) { dev_err(dev->dev.parent, "Could not enable RX FIFO, unsupported core\n"); -- 1.7.2.1 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 3/5] [flexcan] Add of_match to platform_device definition. 2011-08-09 14:43 [Patch 0/5] [flexcan/powerpc] Add support for powerpc flexcan (freescale p1010) -V9 Robin Holt 2011-08-09 14:43 ` [PATCH 1/5] [flexcan] Remove #include <mach/clock.h> Robin Holt 2011-08-09 14:43 ` [PATCH 2/5] [flexcan] Abstract off read/write for big/little endian Robin Holt @ 2011-08-09 14:43 ` Robin Holt 2011-08-09 14:43 ` [PATCH 4/5] [powerpc] Add flexcan device support for p1010rdb Robin Holt 2011-08-09 14:43 ` [PATCH 5/5] [powerpc] Fix up fsl-flexcan device tree binding Robin Holt 4 siblings, 0 replies; 16+ messages in thread From: Robin Holt @ 2011-08-09 14:43 UTC (permalink / raw) To: Robin Holt, Marc Kleine-Budde, Wolfgang Grandegger, U Bhaskar-B22300 Cc: socketcan-core, netdev, PPC list, 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 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 4/5] [powerpc] Add flexcan device support for p1010rdb. 2011-08-09 14:43 [Patch 0/5] [flexcan/powerpc] Add support for powerpc flexcan (freescale p1010) -V9 Robin Holt ` (2 preceding siblings ...) 2011-08-09 14:43 ` [PATCH 3/5] [flexcan] Add of_match to platform_device definition Robin Holt @ 2011-08-09 14:43 ` Robin Holt 2011-08-09 14:43 ` [PATCH 5/5] [powerpc] Fix up fsl-flexcan device tree binding Robin Holt 4 siblings, 0 replies; 16+ messages in thread From: Robin Holt @ 2011-08-09 14:43 UTC (permalink / raw) To: Robin Holt, Marc Kleine-Budde, Wolfgang Grandegger, U Bhaskar-B22300 Cc: netdev, socketcan-core, Robin Holt, PPC list 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 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 5/5] [powerpc] Fix up fsl-flexcan device tree binding. 2011-08-09 14:43 [Patch 0/5] [flexcan/powerpc] Add support for powerpc flexcan (freescale p1010) -V9 Robin Holt ` (3 preceding siblings ...) 2011-08-09 14:43 ` [PATCH 4/5] [powerpc] Add flexcan device support for p1010rdb Robin Holt @ 2011-08-09 14:43 ` Robin Holt 2011-08-09 18:17 ` Scott Wood 4 siblings, 1 reply; 16+ messages in thread From: Robin Holt @ 2011-08-09 14:43 UTC (permalink / raw) To: Robin Holt, Marc Kleine-Budde, Wolfgang Grandegger, U Bhaskar-B22300 Cc: netdev, socketcan-core, Robin Holt, PPC list In working with the socketcan developers, we have come to the conclusion the fsl-flexcan device tree bindings need 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> 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 | 6 +- 3 files changed, 2 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..37e47cd 100644 --- a/arch/powerpc/boot/dts/p1010si.dtsi +++ b/arch/powerpc/boot/dts/p1010si.dtsi @@ -141,19 +141,17 @@ }; can0@1c000 { - compatible = "fsl,flexcan-v1.0"; + compatible = "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,flexcan"; reg = <0x1d000 0x1000>; interrupts = <61 0x2>; interrupt-parent = <&mpic>; - fsl,flexcan-clock-divider = <2>; }; L2: l2-cache-controller@20000 { -- 1.7.2.1 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 5/5] [powerpc] Fix up fsl-flexcan device tree binding. 2011-08-09 14:43 ` [PATCH 5/5] [powerpc] Fix up fsl-flexcan device tree binding Robin Holt @ 2011-08-09 18:17 ` Scott Wood 2011-08-09 18:45 ` Robin Holt 2011-08-09 19:32 ` Wolfgang Grandegger 0 siblings, 2 replies; 16+ messages in thread From: Scott Wood @ 2011-08-09 18:17 UTC (permalink / raw) To: Robin Holt; +Cc: netdev, U Bhaskar-B22300, socketcan-core, PPC list On 08/09/2011 09:43 AM, Robin Holt wrote: > In working with the socketcan developers, we have come to the conclusion > the fsl-flexcan device tree bindings need to be cleaned up. > The driver does not depend upon any properties other than the required properties > so we are removing the file. That is not the criterion for whether something should be expresed in the device tree. It's a description of the hardware, not a Linux driver configuration file. If there are integration parameters that can not be inferred from "this is FSL flexcan v1.0", they should be expressed in the node. Removing the binding altogether seems extreme as well -- we should have bindings for all devices, even if there are no special properties. > Additionally, the p1010*dts files are not > following the standard for node naming in that they have a trailing -v1.0. What "standard for node naming"? There's nothing wrong with putting a block version number in the compatible string, and it looks like the p1010 dts files were following the binding document in this regard. It is common practice when the block version is publicly documented but there's no register it can be read from at runtime. -Scott ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 5/5] [powerpc] Fix up fsl-flexcan device tree binding. 2011-08-09 18:17 ` Scott Wood @ 2011-08-09 18:45 ` Robin Holt 2011-08-09 19:13 ` Scott Wood 2011-08-09 19:32 ` Wolfgang Grandegger 1 sibling, 1 reply; 16+ messages in thread From: Robin Holt @ 2011-08-09 18:45 UTC (permalink / raw) To: Scott Wood; +Cc: netdev, U Bhaskar-B22300, socketcan-core, Robin Holt, PPC list On Tue, Aug 09, 2011 at 01:17:47PM -0500, Scott Wood wrote: > On 08/09/2011 09:43 AM, Robin Holt wrote: > > In working with the socketcan developers, we have come to the conclusion > > the fsl-flexcan device tree bindings need to be cleaned up. > > The driver does not depend upon any properties other than the required properties > > so we are removing the file. > > That is not the criterion for whether something should be expresed in > the device tree. It's a description of the hardware, not a Linux driver > configuration file. If there are integration parameters that can not be > inferred from "this is FSL flexcan v1.0", they should be expressed in > the node. There are no properties other than the required properties. The others were wrongly introduced and are not needed by the driver. When we removed the other properties and the wrong documentation of the mscan oscillator source in the fsl-flexcan.txt file, we were left with an Example: section and a one-line statement "The only properties supported are the required properties." That seemed like the fsl-flexcan.txt file was then pointless. > Removing the binding altogether seems extreme as well -- we should have > bindings for all devices, even if there are no special properties. Ok. I can do that too. Who is the definitive source for that answer? I assume we are talking about the fsl-flexcan.txt file when we say binding. Is that correct? > > Additionally, the p1010*dts files are not > > following the standard for node naming in that they have a trailing -v1.0. > > What "standard for node naming"? There's nothing wrong with putting a For the answer to that, you will need to ask Wolfgang Grandegger. I was working from his feedback. Looking at the plethora of other node names, the vast majority do not have any -v#.#, and the ones that do also tend to have multiple versions. Based upon that, I suspect he is correct, but I do not know where the documentation is or if it even exists. > block version number in the compatible string, and it looks like the > p1010 dts files were following the binding document in this regard. It > is common practice when the block version is publicly documented but > there's no register it can be read from at runtime. Thanks, Robin ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 5/5] [powerpc] Fix up fsl-flexcan device tree binding. 2011-08-09 18:45 ` Robin Holt @ 2011-08-09 19:13 ` Scott Wood 2011-08-09 19:49 ` Wolfgang Grandegger 0 siblings, 1 reply; 16+ messages in thread From: Scott Wood @ 2011-08-09 19:13 UTC (permalink / raw) To: Robin Holt Cc: netdev, devicetree-discuss@lists.ozlabs.org, U Bhaskar-B22300, socketcan-core, PPC list On 08/09/2011 01:45 PM, Robin Holt wrote: > On Tue, Aug 09, 2011 at 01:17:47PM -0500, Scott Wood wrote: >> On 08/09/2011 09:43 AM, Robin Holt wrote: >>> In working with the socketcan developers, we have come to the conclusion >>> the fsl-flexcan device tree bindings need to be cleaned up. >>> The driver does not depend upon any properties other than the required properties >>> so we are removing the file. >> >> That is not the criterion for whether something should be expresed in >> the device tree. It's a description of the hardware, not a Linux driver >> configuration file. If there are integration parameters that can not be >> inferred from "this is FSL flexcan v1.0", they should be expressed in >> the node. > > There are no properties other than the required properties. The others > were wrongly introduced and are not needed by the driver. Not needed by this driver, or will never be needed by any reasonable driver (or is not a good description of the hardware)? The device tree is not an internal Linux implementation detail. It is shared by other OSes, firmwares, hypervisors, etc. Bindings should be created with care, and kept stable unless there's a good reason to break compatibility. devicetree-discuss@lists.ozlabs.org should be CCed on device tree discussions. > When we > removed the other properties and the wrong documentation of the mscan > oscillator source in the fsl-flexcan.txt file, we were left with an > Example: section and a one-line statement "The only properties supported > are the required properties." That seemed like the fsl-flexcan.txt > file was then pointless. There is the compatible string, and you could mention that there is a single reg resource and a single interrupt. >> Removing the binding altogether seems extreme as well -- we should have >> bindings for all devices, even if there are no special properties. > > Ok. I can do that too. Who is the definitive source for that answer? For policy questions on device tree bindings? Grant Likely is the maintainer for device tree stuff. A lot of the simpler bindings have been left undocumented so far, IMHO it should be a goal to document them all. There are some existing ones that are documented despite not having special properties, e.g. Documentation/devicetree/bindings/serio/altera_ps2.txt, Documentation/devicetree/bindings/arm/sirf.txt, Documentation/devicetree/bindings/powerpc/nintendo/wii.txt, etc. > I assume we are talking about the fsl-flexcan.txt file when we say > binding. Is that correct? Yes, although devicetree.org is another possibility. >>> Additionally, the p1010*dts files are not >>> following the standard for node naming in that they have a trailing -v1.0. >> >> What "standard for node naming"? There's nothing wrong with putting a > > For the answer to that, you will need to ask Wolfgang Grandegger. I was > working from his feedback. Looking at the plethora of other node names, > the vast majority do not have any -v#.#, and the ones that do also tend > to have multiple versions. Based upon that, I suspect he is correct, > but I do not know where the documentation is or if it even exists. There's a lot of crap in old bindings, plus it's not appropriate for all circumstances (specifying bindings should be done a little more carefully than "what do most other bindings do?"). It's something we've been doing lately for blocks that have a version number, but it's not dynamically readable. Looking in the FlexCAN chapter of the p1010 manual, I don't see any reference to a block version, and I do see references to "previous FlexCAN versions". So I suggest "fsl,p1010-flexcan". -Scott ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 5/5] [powerpc] Fix up fsl-flexcan device tree binding. 2011-08-09 19:13 ` Scott Wood @ 2011-08-09 19:49 ` Wolfgang Grandegger 2011-08-09 19:58 ` Scott Wood 0 siblings, 1 reply; 16+ messages in thread From: Wolfgang Grandegger @ 2011-08-09 19:49 UTC (permalink / raw) To: Scott Wood Cc: netdev, devicetree-discuss@lists.ozlabs.org, U Bhaskar-B22300, socketcan-core, Robin Holt, PPC list On 08/09/2011 09:13 PM, Scott Wood wrote: > On 08/09/2011 01:45 PM, Robin Holt wrote: >> On Tue, Aug 09, 2011 at 01:17:47PM -0500, Scott Wood wrote: >>> On 08/09/2011 09:43 AM, Robin Holt wrote: >>>> In working with the socketcan developers, we have come to the conclusion >>>> the fsl-flexcan device tree bindings need to be cleaned up. >>>> The driver does not depend upon any properties other than the required properties >>>> so we are removing the file. >>> >>> That is not the criterion for whether something should be expresed in >>> the device tree. It's a description of the hardware, not a Linux driver >>> configuration file. If there are integration parameters that can not be >>> inferred from "this is FSL flexcan v1.0", they should be expressed in >>> the node. >> >> There are no properties other than the required properties. The others >> were wrongly introduced and are not needed by the driver. > > Not needed by this driver, or will never be needed by any reasonable > driver (or is not a good description of the hardware)? > > The device tree is not an internal Linux implementation detail. It is > shared by other OSes, firmwares, hypervisors, etc. Bindings should be > created with care, and kept stable unless there's a good reason to break > compatibility. > > devicetree-discuss@lists.ozlabs.org should be CCed on device tree > discussions. Yes. The doc for the bindings we speak about http://lxr.linux.no/#linux+v3.0.1/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt sneaked into the kernel without been presented on any mailing list and without the corresponding driver patch. >> When we >> removed the other properties and the wrong documentation of the mscan >> oscillator source in the fsl-flexcan.txt file, we were left with an >> Example: section and a one-line statement "The only properties supported >> are the required properties." That seemed like the fsl-flexcan.txt >> file was then pointless. > > There is the compatible string, and you could mention that there is a > single reg resource and a single interrupt. > >>> Removing the binding altogether seems extreme as well -- we should have >>> bindings for all devices, even if there are no special properties. >> >> Ok. I can do that too. Who is the definitive source for that answer? > > For policy questions on device tree bindings? Grant Likely is the > maintainer for device tree stuff. > > A lot of the simpler bindings have been left undocumented so far, IMHO > it should be a goal to document them all. There are some existing ones > that are documented despite not having special properties, e.g. > Documentation/devicetree/bindings/serio/altera_ps2.txt, > Documentation/devicetree/bindings/arm/sirf.txt, > Documentation/devicetree/bindings/powerpc/nintendo/wii.txt, etc. > >> I assume we are talking about the fsl-flexcan.txt file when we say >> binding. Is that correct? > > Yes, although devicetree.org is another possibility. > >>>> Additionally, the p1010*dts files are not >>>> following the standard for node naming in that they have a trailing -v1.0. >>> >>> What "standard for node naming"? There's nothing wrong with putting a >> >> For the answer to that, you will need to ask Wolfgang Grandegger. I was >> working from his feedback. Looking at the plethora of other node names, >> the vast majority do not have any -v#.#, and the ones that do also tend >> to have multiple versions. Based upon that, I suspect he is correct, >> but I do not know where the documentation is or if it even exists. > > There's a lot of crap in old bindings, plus it's not appropriate for all > circumstances (specifying bindings should be done a little more > carefully than "what do most other bindings do?"). It's something we've > been doing lately for blocks that have a version number, but it's not > dynamically readable. > > Looking in the FlexCAN chapter of the p1010 manual, I don't see any > reference to a block version, and I do see references to "previous > FlexCAN versions". So I suggest "fsl,p1010-flexcan". OK, just "fsl,p1010-flexcan" or "fsl,p1010-flexcan", "fsl,flexcan" Note that the Flexcan is used on Freescale ARM cores as well (and device tree for ARM will show up soon). Wolfgang. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 5/5] [powerpc] Fix up fsl-flexcan device tree binding. 2011-08-09 19:49 ` Wolfgang Grandegger @ 2011-08-09 19:58 ` Scott Wood 2011-08-09 20:59 ` Robin Holt 0 siblings, 1 reply; 16+ messages in thread From: Scott Wood @ 2011-08-09 19:58 UTC (permalink / raw) To: Wolfgang Grandegger Cc: netdev, devicetree-discuss@lists.ozlabs.org, U Bhaskar-B22300, socketcan-core, Robin Holt, PPC list On 08/09/2011 02:49 PM, Wolfgang Grandegger wrote: > Yes. The doc for the bindings we speak about > > http://lxr.linux.no/#linux+v3.0.1/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt > > sneaked into the kernel without been presented on any mailing list and > without the corresponding driver patch. It was posted on linuxppc-dev: http://patchwork.ozlabs.org/patch/91980/ Though I agree it should have been posted more widely. > OK, just > > "fsl,p1010-flexcan" > > or > > "fsl,p1010-flexcan", "fsl,flexcan" I'm ok with the latter, if there's enough in common that it's conceivable that a driver wouldn't care. The more specific compatible will be there if the driver wants to make use of it later. -Scot ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 5/5] [powerpc] Fix up fsl-flexcan device tree binding. 2011-08-09 19:58 ` Scott Wood @ 2011-08-09 20:59 ` Robin Holt 2011-08-10 14:52 ` Kumar Gala 0 siblings, 1 reply; 16+ messages in thread From: Robin Holt @ 2011-08-09 20:59 UTC (permalink / raw) To: Scott Wood Cc: netdev, devicetree-discuss@lists.ozlabs.org, U Bhaskar-B22300, socketcan-core, Robin Holt, PPC list I guess my poor wording may have gotten me in trouble. I am getting ready to repost this patch, but I want to ensure I am getting it as right as possible. I think I should reword the commit message to indicate we are removing the Documentation/.../fsl-flexcan.txt file which has essentially become empty and change the p1010si.dtsi file's can nodes to "fsl,p1010-flexcan", "fsl,flexcan". Is that correct? Thanks, Robin On Tue, Aug 09, 2011 at 02:58:56PM -0500, Scott Wood wrote: > On 08/09/2011 02:49 PM, Wolfgang Grandegger wrote: > > Yes. The doc for the bindings we speak about > > > > http://lxr.linux.no/#linux+v3.0.1/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt > > > > sneaked into the kernel without been presented on any mailing list and > > without the corresponding driver patch. > > It was posted on linuxppc-dev: > http://patchwork.ozlabs.org/patch/91980/ > > Though I agree it should have been posted more widely. > > > OK, just > > > > "fsl,p1010-flexcan" > > > > or > > > > "fsl,p1010-flexcan", "fsl,flexcan" > > I'm ok with the latter, if there's enough in common that it's > conceivable that a driver wouldn't care. The more specific compatible > will be there if the driver wants to make use of it later. > > -Scot ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 5/5] [powerpc] Fix up fsl-flexcan device tree binding. 2011-08-09 20:59 ` Robin Holt @ 2011-08-10 14:52 ` Kumar Gala 2011-08-10 16:16 ` Robin Holt 0 siblings, 1 reply; 16+ messages in thread From: Kumar Gala @ 2011-08-10 14:52 UTC (permalink / raw) To: Robin Holt Cc: netdev, devicetree-discuss@lists.ozlabs.org, U Bhaskar-B22300, socketcan-core, Scott Wood, PPC list On Aug 9, 2011, at 3:59 PM, Robin Holt wrote: > I guess my poor wording may have gotten me in trouble. I am getting > ready to repost this patch, but I want to ensure I am getting it as > right as possible. >=20 > I think I should reword the commit message to indicate we are removing > the Documentation/.../fsl-flexcan.txt file which has essentially = become > empty and change the p1010si.dtsi file's can nodes to = "fsl,p1010-flexcan", > "fsl,flexcan". Is that correct? >=20 > Thanks, > Robin This is wrong. Again, what binding covers "fsl,flexcan" if you remove = fsl-flexcan.txt: [galak@right powerpc]$ git grep flexcan = Documentation/devicetree/bindings = Documentation/devicetree/bindings/net/can/fsl-flexcan.txt:fsl,flexcan-v1.0= nodes Documentation/devicetree/bindings/net/can/fsl-flexcan.txt:- = fsl,flexcan-clock-source : CAN Engine Clock Source.This property selects Documentation/devicetree/bindings/net/can/fsl-flexcan.txt:- = fsl,flexcan-clock-divider : for the reference and system clock, an = additional Documentation/devicetree/bindings/net/can/fsl-flexcan.txt: - v1.0 = of flexcan-v1.0 represent the IP block version for P1010 SOC. Documentation/devicetree/bindings/net/can/fsl-flexcan.txt: = compatible =3D "fsl,flexcan-v1.0"; Documentation/devicetree/bindings/net/can/fsl-flexcan.txt: = fsl,flexcan-clock-source =3D "platform"; Documentation/devicetree/bindings/net/can/fsl-flexcan.txt: = fsl,flexcan-clock-divider =3D <2>; Not seeing anything that covers it. I think the issue should be resolved by patching fsl-flexcan.txt to = remove wording or update it. - k= ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 5/5] [powerpc] Fix up fsl-flexcan device tree binding. 2011-08-10 14:52 ` Kumar Gala @ 2011-08-10 16:16 ` Robin Holt 0 siblings, 0 replies; 16+ messages in thread From: Robin Holt @ 2011-08-10 16:16 UTC (permalink / raw) To: Kumar Gala Cc: netdev, devicetree-discuss@lists.ozlabs.org, U Bhaskar-B22300, socketcan-core, Robin Holt, Scott Wood, PPC list On Wed, Aug 10, 2011 at 09:52:07AM -0500, Kumar Gala wrote: > > On Aug 9, 2011, at 3:59 PM, Robin Holt wrote: > > > I guess my poor wording may have gotten me in trouble. I am getting > > ready to repost this patch, but I want to ensure I am getting it as > > right as possible. > > > > I think I should reword the commit message to indicate we are removing > > the Documentation/.../fsl-flexcan.txt file which has essentially become > > empty and change the p1010si.dtsi file's can nodes to "fsl,p1010-flexcan", > > "fsl,flexcan". Is that correct? > > > > Thanks, > > Robin > > This is wrong. Again, what binding covers "fsl,flexcan" if you remove fsl-flexcan.txt: > > [galak@right powerpc]$ git grep flexcan Documentation/devicetree/bindings > Documentation/devicetree/bindings/net/can/fsl-flexcan.txt:fsl,flexcan-v1.0 nodes > Documentation/devicetree/bindings/net/can/fsl-flexcan.txt:- fsl,flexcan-clock-source : CAN Engine Clock Source.This property selects > Documentation/devicetree/bindings/net/can/fsl-flexcan.txt:- fsl,flexcan-clock-divider : for the reference and system clock, an additional > Documentation/devicetree/bindings/net/can/fsl-flexcan.txt: - v1.0 of flexcan-v1.0 represent the IP block version for P1010 SOC. > Documentation/devicetree/bindings/net/can/fsl-flexcan.txt: compatible = "fsl,flexcan-v1.0"; > Documentation/devicetree/bindings/net/can/fsl-flexcan.txt: fsl,flexcan-clock-source = "platform"; > Documentation/devicetree/bindings/net/can/fsl-flexcan.txt: fsl,flexcan-clock-divider = <2>; > > Not seeing anything that covers it. > > I think the issue should be resolved by patching fsl-flexcan.txt to remove wording or update it. Done. Robin ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 5/5] [powerpc] Fix up fsl-flexcan device tree binding. 2011-08-09 18:17 ` Scott Wood 2011-08-09 18:45 ` Robin Holt @ 2011-08-09 19:32 ` Wolfgang Grandegger 2011-08-09 20:11 ` Scott Wood 1 sibling, 1 reply; 16+ messages in thread From: Wolfgang Grandegger @ 2011-08-09 19:32 UTC (permalink / raw) To: Scott Wood Cc: netdev, Devicetree-discuss@lists.ozlabs.org, U Bhaskar-B22300, socketcan-core, Robin Holt, PPC list On 08/09/2011 08:17 PM, Scott Wood wrote: > On 08/09/2011 09:43 AM, Robin Holt wrote: >> In working with the socketcan developers, we have come to the conclusion >> the fsl-flexcan device tree bindings need to be cleaned up. >> The driver does not depend upon any properties other than the required properties >> so we are removing the file. > > That is not the criterion for whether something should be expresed in > the device tree. It's a description of the hardware, not a Linux driver > configuration file. If there are integration parameters that can not be > inferred from "this is FSL flexcan v1.0", they should be expressed in > the node. > > Removing the binding altogether seems extreme as well -- we should have > bindings for all devices, even if there are no special properties. Yes, of course. The commit message misleading. We do not intend to remove the binding but just a few unused and confusing properties. Concerning the compatible string, Freescale introduced for the Flexcan on the P1010 "fsl,flexcan-v1.0". That's not the usual convention also because the v1.0 if for the PowerPC cores only, I assume, but we have ARM cores as well. If we need to distinguish I think we should use: "fsl,p1010-flexcan", "fsl,flexcan" Do you agree? >> Additionally, the p1010*dts files are not >> following the standard for node naming in that they have a trailing -v1.0. > > What "standard for node naming"? There's nothing wrong with putting a > block version number in the compatible string, and it looks like the > p1010 dts files were following the binding document in this regard. It > is common practice when the block version is publicly documented but > there's no register it can be read from at runtime. See above. Furthermore I must admit, that the bindings shown up mainline Linux have never been presented on any mailing list. Wolfgang. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 5/5] [powerpc] Fix up fsl-flexcan device tree binding. 2011-08-09 19:32 ` Wolfgang Grandegger @ 2011-08-09 20:11 ` Scott Wood 0 siblings, 0 replies; 16+ messages in thread From: Scott Wood @ 2011-08-09 20:11 UTC (permalink / raw) To: Wolfgang Grandegger Cc: netdev, Devicetree-discuss@lists.ozlabs.org, U Bhaskar-B22300, socketcan-core, Robin Holt, PPC list On 08/09/2011 02:32 PM, Wolfgang Grandegger wrote: > On 08/09/2011 08:17 PM, Scott Wood wrote: >> On 08/09/2011 09:43 AM, Robin Holt wrote: >>> In working with the socketcan developers, we have come to the conclusion >>> the fsl-flexcan device tree bindings need to be cleaned up. >>> The driver does not depend upon any properties other than the required properties >>> so we are removing the file. >> >> That is not the criterion for whether something should be expresed in >> the device tree. It's a description of the hardware, not a Linux driver >> configuration file. If there are integration parameters that can not be >> inferred from "this is FSL flexcan v1.0", they should be expressed in >> the node. >> >> Removing the binding altogether seems extreme as well -- we should have >> bindings for all devices, even if there are no special properties. > > Yes, of course. The commit message misleading. We do not intend to > remove the binding but just a few unused and confusing properties. Is it a matter of the current driver not caring, or the properties just not making sense for any reasonable driver (ambiguous, inferrable from the flexcan version, software configuration, etc)? -Scott ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2011-08-10 16:16 UTC | newest] Thread overview: 16+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-08-09 14:43 [Patch 0/5] [flexcan/powerpc] Add support for powerpc flexcan (freescale p1010) -V9 Robin Holt 2011-08-09 14:43 ` [PATCH 1/5] [flexcan] Remove #include <mach/clock.h> Robin Holt 2011-08-09 14:43 ` [PATCH 2/5] [flexcan] Abstract off read/write for big/little endian Robin Holt 2011-08-09 14:43 ` [PATCH 3/5] [flexcan] Add of_match to platform_device definition Robin Holt 2011-08-09 14:43 ` [PATCH 4/5] [powerpc] Add flexcan device support for p1010rdb Robin Holt 2011-08-09 14:43 ` [PATCH 5/5] [powerpc] Fix up fsl-flexcan device tree binding Robin Holt 2011-08-09 18:17 ` Scott Wood 2011-08-09 18:45 ` Robin Holt 2011-08-09 19:13 ` Scott Wood 2011-08-09 19:49 ` Wolfgang Grandegger 2011-08-09 19:58 ` Scott Wood 2011-08-09 20:59 ` Robin Holt 2011-08-10 14:52 ` Kumar Gala 2011-08-10 16:16 ` Robin Holt 2011-08-09 19:32 ` Wolfgang Grandegger 2011-08-09 20:11 ` Scott Wood
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).