Re: [PATCH] net/can: add driver for mscan family & mpc52xx_mscan

2009-11-17 Thread Wolfgang Grandegger
Wolfram Sang wrote:
> Hi Grant,
> 
> Wolfgang commented on some points already, I will pick up the other remarks,
> just one question:
> 
>>> +   clk_src = of_get_property(np, "fsl,mscan-clk-src", NULL);
>>> +   if (clk_src && strcmp(clk_src, "ip") == 0)
>> Should protect against non-null.  strncmp() maybe?
> 
> "ip" is null-terminated, or what do you mean?

Imagine somebody defines:

  fsl,mscan-clk-src = <0xbaeee>;

Wolfgang.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH] net/can: add driver for mscan family & mpc52xx_mscan

2009-11-17 Thread Wolfgang Grandegger
Wolfgang Grandegger wrote:
> Wolfram Sang wrote:
>> Hi Grant,
>>
>> Wolfgang commented on some points already, I will pick up the other remarks,
>> just one question:
>>
 +   clk_src = of_get_property(np, "fsl,mscan-clk-src", NULL);
 +   if (clk_src && strcmp(clk_src, "ip") == 0)
>>> Should protect against non-null.  strncmp() maybe?
>> "ip" is null-terminated, or what do you mean?
> 
> Imagine somebody defines:
> 
>   fsl,mscan-clk-src = <0xbaeee>;

Forget my comment. I will not harm in the above case. I was just worried
about non-null-teminated strings.

Wolfgang.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH] net/can: add driver for mscan family & mpc52xx_mscan

2009-11-16 Thread Wolfram Sang
Hi Grant,

Wolfgang commented on some points already, I will pick up the other remarks,
just one question:

> > +       clk_src = of_get_property(np, "fsl,mscan-clk-src", NULL);
> > +       if (clk_src && strcmp(clk_src, "ip") == 0)
> 
> Should protect against non-null.  strncmp() maybe?

"ip" is null-terminated, or what do you mean?

-- 
Pengutronix e.K.   | Wolfram Sang|
Industrial Linux Solutions | http://www.pengutronix.de/  |


signature.asc
Description: Digital signature
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH] net/can: add driver for mscan family & mpc52xx_mscan

2009-11-16 Thread Wolfgang Grandegger
Hi Wolfram,

Wolfram Sang wrote:
> Hi Wolfgang,
> 
> On Mon, Nov 16, 2009 at 09:44:05AM +0100, Wolfgang Grandegger wrote:
>> Hi Wolfram,
>>
>> thanks for pushing this driver to mainline. I think you should also add
>> a CC to the Devicetree-discuss ML.
> 
> thank you very much for your review! I agree with nearly all of your points 
> and will
> send an update today. The only thing I have doubts about is removing those 
> lines:

Thanks, quite a bit of work.

>> +MODULE_AUTHOR("Andrey Volkov ");
>> +MODULE_LICENSE("GPL v2");
>> +MODULE_DESCRIPTION("CAN port driver for a MSCAN based chips");
> 
> I looked around in the kernel sources and found that they are often present in
> generic modules, even if they can't be used without a wrapper (examples are 
> all
> files in drivers/i2c/algos or drivers/net/wireless/iwlwifi/iwl-core.c). As I'm
> also a bit anxious to fiddle with other people's authorship, I'd prefer to 
> keep
> them.

They do not harm, fine for me.

> Finally, I'll also try to test suspend/resume, but I have to find out if it is
> supported on that board in general.

Maybe somebody else already uses suspend/resume on a MPC5200 board with
Socket-CAN and could provide some feedback.

Wolfgang.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH] net/can: add driver for mscan family & mpc52xx_mscan

2009-11-16 Thread Wolfram Sang
Hi Wolfgang,

On Mon, Nov 16, 2009 at 09:44:05AM +0100, Wolfgang Grandegger wrote:
> Hi Wolfram,
> 
> thanks for pushing this driver to mainline. I think you should also add
> a CC to the Devicetree-discuss ML.

thank you very much for your review! I agree with nearly all of your points and 
will
send an update today. The only thing I have doubts about is removing those 
lines:

> +MODULE_AUTHOR("Andrey Volkov ");
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("CAN port driver for a MSCAN based chips");

I looked around in the kernel sources and found that they are often present in
generic modules, even if they can't be used without a wrapper (examples are all
files in drivers/i2c/algos or drivers/net/wireless/iwlwifi/iwl-core.c). As I'm
also a bit anxious to fiddle with other people's authorship, I'd prefer to keep
them.

Finally, I'll also try to test suspend/resume, but I have to find out if it is
supported on that board in general.

Regards,

   Wolfram

-- 
Pengutronix e.K.   | Wolfram Sang|
Industrial Linux Solutions | http://www.pengutronix.de/  |


signature.asc
Description: Digital signature
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH] net/can: add driver for mscan family & mpc52xx_mscan

2009-11-16 Thread David Miller
From: Wolfram Sang 
Date: Mon, 16 Nov 2009 11:12:05 +0100

>> >> Taken from socketcan-svn, fixed remaining todos, cleaned up, tested with a
>> >> phyCORE-MPC5200B-IO and a custom board.
>> >>
>> >> Signed-off-by: Wolfram Sang 
>> > 
>> > Applied.
>> 
>> Unfortunately too early. I will send my review tomorrow. Sorry for the
>> delay.
> 
> So, I'd assume that I now have to send an incremental patch instead of a V2?

Yes.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH] net/can: add driver for mscan family & mpc52xx_mscan

2009-11-16 Thread Wolfram Sang
David,

> >> Taken from socketcan-svn, fixed remaining todos, cleaned up, tested with a
> >> phyCORE-MPC5200B-IO and a custom board.
> >>
> >> Signed-off-by: Wolfram Sang 
> > 
> > Applied.
> 
> Unfortunately too early. I will send my review tomorrow. Sorry for the
> delay.

So, I'd assume that I now have to send an incremental patch instead of a V2?

Regards,

   Wolfram

-- 
Pengutronix e.K.   | Wolfram Sang|
Industrial Linux Solutions | http://www.pengutronix.de/  |


signature.asc
Description: Digital signature
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH] net/can: add driver for mscan family & mpc52xx_mscan

2009-11-16 Thread Wolfgang Grandegger
Hi Wolfram,

thanks for pushing this driver to mainline. I think you should also add
a CC to the Devicetree-discuss ML.

Wolfram Sang wrote:
> Taken from socketcan-svn, fixed remaining todos, cleaned up, tested with a
> phyCORE-MPC5200B-IO and a custom board.
> 
> Signed-off-by: Wolfram Sang 
> Cc: Wolfgang Grandegger 
> Cc: Grant Likely 
> Cc: David Miller 
> ---
> 
> This patch is based on net-next as of yesterday.
> 
> To make the review easier for those who are already familiar with earlier
> versions of this driver (especially for Wolfgang), I put my development branch
> online, so you can check my changes incrementally:
> 
> http://git.pengutronix.de/?p=wsa/linux-2.6.git;a=summary
> 
>  Documentation/powerpc/dts-bindings/fsl/mpc5200.txt |9 +
>  drivers/net/can/Kconfig|   19 +
>  drivers/net/can/Makefile   |1 +
>  drivers/net/can/mscan/Makefile |5 +
>  drivers/net/can/mscan/mpc52xx_can.c|  279 

With the mpc521x in mind, please change the name to mpc5xxx_can.c. In
this file, you already use mpc5xxx_* functions.

>  drivers/net/can/mscan/mscan.c  |  699 
> 
>  drivers/net/can/mscan/mscan.h  |  262 
>  7 files changed, 1274 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/net/can/mscan/Makefile
>  create mode 100644 drivers/net/can/mscan/mpc52xx_can.c
>  create mode 100644 drivers/net/can/mscan/mscan.c
>  create mode 100644 drivers/net/can/mscan/mscan.h
> 
> diff --git a/Documentation/powerpc/dts-bindings/fsl/mpc5200.txt 
> b/Documentation/powerpc/dts-bindings/fsl/mpc5200.txt
> index 8447fd7..b151fb1 100644
> --- a/Documentation/powerpc/dts-bindings/fsl/mpc5200.txt
> +++ b/Documentation/powerpc/dts-bindings/fsl/mpc5200.txt
> @@ -178,3 +178,12 @@ External interrupts:
>   external irq3:  interrupts = <1 3 n>;
>  'n' is sense (0: level high, 1: edge rising, 2: edge falling 3: level low)
>  
> +fsl,mpc5200-mscan nodes
> +---
> +In addition to the required compatible-, reg- and interrupt-properites, you 
> can
> +also specify which clock shall be used for the bus:

I think "which clock source shall be used for the MSCAN controller" is
more appropriate. It's not the clock for the CAN bus.

> +
> +- fsl,mscan-clk-src  - a string describing the clock source. Valid values
> +   are "ip" for IP_CLK and "sys" for SYS_XTAL.
> +   "sys" is the default in case the property is not
> +   present.

I think it's common to use long names, e.g. fsl,mscan-clock-source. Also
the "system" clock is normally not equal to the XTAL clock. "ref" for
reference clock would be more appropriate. Anyhow, in the code you only
check for "ip". If it's not defined, the other clock will be used.

> diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig
> index b819cc2..c16e6ff 100644
> --- a/drivers/net/can/Kconfig
> +++ b/drivers/net/can/Kconfig

As the mscan driver is in a sub-directory, we should move the Kconfig
file into it. Oliver has sent a patch recently to do so for the sja1000
and usb sub-directory.

> @@ -108,6 +108,25 @@ config CAN_MCP251X
>   ---help---
> Driver for the Microchip MCP251x SPI CAN controllers.
>  
> +config CAN_MSCAN
> + depends on CAN_DEV && (PPC || M68K || M68KNOMMU)
> + tristate "Support for Freescale MSCAN based chips"
> + ---help---
> +   The Motorola Scalable Controller Area Network (MSCAN) definition
> +   is based on the MSCAN12 definition which is the specific
> +   implementation of the Motorola Scalable CAN concept targeted for
> +   the Motorola MC68HC12 Microcontroller Family.
> +
> +config CAN_MPC52XX

s/CAN_MPC52XX/CAN_MPC5XXX/, (or CAN_MPC5xxx) see above.

> + tristate "Freescale MPC5xxx onboard CAN controller"
> + depends on CAN_MSCAN && PPC_MPC52xx
> + ---help---
> +   If you say yes here you get support for Freescale's MPC52xx
> +   onboard dualCAN controller.
> +
> +   This driver can also be built as a module.  If so, the module
> +   will be called mpc5xxx_can.

Also here you already speak about mpc5xxx. And the name of the module is
mscan-mpc52xx.ko, IIRC.

> +
>  config CAN_DEBUG_DEVICES
>   bool "CAN devices debugging messages"
>   depends on CAN
> diff --git a/drivers/net/can/Makefile b/drivers/net/can/Makefile
> index 1489181..56899fe 100644
> --- a/drivers/net/can/Makefile
> +++ b/drivers/net/can/Makefile
> @@ -10,6 +10,7 @@ can-dev-y   := dev.o
>  obj-y+= usb/
>  
>  obj-$(CONFIG_CAN_SJA1000)+= sja1000/
> +obj-$(CONFIG_CAN_MSCAN)  += mscan/
>  obj-$(CONFIG_CAN_AT91)   += at91_can.o
>  obj-$(CONFIG_CAN_TI_HECC)+= ti_hecc.o
>  obj-$(CONFIG_CAN_MCP251X)+= mcp251x.o
> diff --git a/drivers/net/can/mscan/Makefile b/drivers/net/can/mscan/Makefile
> new file mod

Re: [PATCH] net/can: add driver for mscan family & mpc52xx_mscan

2009-11-16 Thread Wolfgang Grandegger
Grant Likely wrote:
> On Fri, Nov 13, 2009 at 9:14 AM, Wolfram Sang  wrote:
>> Taken from socketcan-svn, fixed remaining todos, cleaned up, tested with a
>> phyCORE-MPC5200B-IO and a custom board.
>>
>> Signed-off-by: Wolfram Sang 
>> Cc: Wolfgang Grandegger 
>> Cc: Grant Likely 
>> Cc: David Miller 
> 
> I don't see any locking in this driver.  What keeps the mscan_isr or
> other routines from conflicting with each other?  What is the
> concurrency model for CAN devices?

There is concurrency between the mscan_start_xmit() and mscan_irq()
routine, which is handled by disabling/enabling the TX interrupt source.
CAN configuration (bit-timing) can only be changed when the device is
stopped (down) and bus-off recovery requires that interrupts are
disabled or the hadrware does not send/receive messages after the
bus-off occurred.

> More comments below.  I don't have the background to delve into the
> CAN details, but I can make some comments on the general structure of
> the driver.

[snip]
>> +static unsigned int  __devinit mpc52xx_can_clock_freq(struct of_device *of,
>> + int clock_src)
>> +{
>> +   unsigned int pvr;
>> +
>> +   pvr = mfspr(SPRN_PVR);
>> +
>> +   if (clock_src == MSCAN_CLKSRC_BUS || pvr == 0x80822011)
>> +   return mpc5xxx_get_bus_frequency(of->node);
>> +
>> +   return mpc52xx_can_xtal_freq(of);
>> +}
> 
> mpc52xx_can_xtal_freq() is only used by this function.  Do they need
> to be separate?

Not really, and it would save some lines of code.

[snip]
>> +static int mscan_set_mode(struct net_device *dev, u8 mode)
>> +{
>> +   struct mscan_priv *priv = netdev_priv(dev);
>> +   struct mscan_regs *regs = (struct mscan_regs *)priv->reg_base;
>> +   int ret = 0;
>> +   int i;
>> +   u8 canctl1;
>> +
>> +   if (mode != MSCAN_NORMAL_MODE) {
>> +
>> +   if (priv->tx_active) {
>> +   /* Abort transfers before going to sleep */#
>> +   out_8(®s->cantarq, priv->tx_active);
>> +   /* Suppress TX done interrupts */
>> +   out_8(®s->cantier, 0);
>> +   }
>> +
>> +   canctl1 = in_8(®s->canctl1);
>> +   if ((mode & MSCAN_SLPRQ) && (canctl1 & MSCAN_SLPAK) == 0) {
>> +   out_8(®s->canctl0,
>> + in_8(®s->canctl0) | MSCAN_SLPRQ);
>> +   for (i = 0; i < MSCAN_SET_MODE_RETRIES; i++) {
>> +   if (in_8(®s->canctl1) & MSCAN_SLPAK)
>> +   break;
>> +   udelay(100);
> 
> Ugh.  Can you sleep instead?  This burns a lot of CPU cycles to no purpose.

A real sleep for 100us? The usual jiffy based sleep would take 1..10 ms,
at least. I think we should check how much time/cycles it usually takes.

[snip]
>> +static int mscan_do_set_mode(struct net_device *dev, enum can_mode mode)
>> +{
>> +
>> +   struct mscan_priv *priv = netdev_priv(dev);
>> +   int ret = 0;
>> +
>> +   if (!priv->open_time)
>> +   return -EINVAL;
>> +
>> +   switch (mode) {
>> +   case CAN_MODE_SLEEP:
>> +   case CAN_MODE_STOP:
>> +   netif_stop_queue(dev);
>> +   mscan_set_mode(dev,
>> +  (mode ==
>> +   CAN_MODE_STOP) ? MSCAN_INIT_MODE :
>> +  MSCAN_SLEEP_MODE);
> 
> A little hard on the eyes.  Can you rework to not spill over 4 lines?
> (ie. calc mode flag on the line above?)

These cases can safely be removed as currently only CAN_MODE_START is
supported by the upper layer.

Wolfgang.

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH] net/can: add driver for mscan family & mpc52xx_mscan

2009-11-15 Thread Wolfgang Grandegger
David Miller wrote:
> From: Wolfram Sang 
> Date: Fri, 13 Nov 2009 17:14:52 +0100
> 
>> Taken from socketcan-svn, fixed remaining todos, cleaned up, tested with a
>> phyCORE-MPC5200B-IO and a custom board.
>>
>> Signed-off-by: Wolfram Sang 
> 
> Applied.

Unfortunately too early. I will send my review tomorrow. Sorry for the
delay.

Wolfgang.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH] net/can: add driver for mscan family & mpc52xx_mscan

2009-11-13 Thread David Miller
From: Wolfram Sang 
Date: Fri, 13 Nov 2009 17:14:52 +0100

> Taken from socketcan-svn, fixed remaining todos, cleaned up, tested with a
> phyCORE-MPC5200B-IO and a custom board.
> 
> Signed-off-by: Wolfram Sang 

Applied.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH] net/can: add driver for mscan family & mpc52xx_mscan

2009-11-13 Thread Grant Likely
On Fri, Nov 13, 2009 at 9:14 AM, Wolfram Sang  wrote:
> Taken from socketcan-svn, fixed remaining todos, cleaned up, tested with a
> phyCORE-MPC5200B-IO and a custom board.
>
> Signed-off-by: Wolfram Sang 
> Cc: Wolfgang Grandegger 
> Cc: Grant Likely 
> Cc: David Miller 

I don't see any locking in this driver.  What keeps the mscan_isr or
other routines from conflicting with each other?  What is the
concurrency model for CAN devices?

More comments below.  I don't have the background to delve into the
CAN details, but I can make some comments on the general structure of
the driver.

g.

> ---
>
> +static struct of_device_id mpc52xx_cdm_ids[] __devinitdata = {
> +       { .compatible = "fsl,mpc5200-cdm", },
> +       { .compatible = "fsl,mpc5200b-cdm", },
> +       {}
> +};

You can drop the 'b' line. The 'b' version is compatible with the
original, and all in-tree 5200b files claim compatibility with the
non-'b' version.

> +
> +/*
> + * Get the frequency of the external oscillator clock connected
> + * to the SYS_XTAL_IN pin, or return 0 if it cannot be determined.
> + */
> +static unsigned int  __devinit mpc52xx_can_xtal_freq(struct of_device *of)
> +{
> +       struct mpc52xx_cdm  __iomem *cdm;
> +       struct device_node *np_cdm;
> +       unsigned int freq;
> +       u32 val;
> +
> +       freq = mpc5xxx_get_bus_frequency(of->node);
> +       if (!freq)
> +               return 0;
> +
> +       /*
> +        * Determine SYS_XTAL_IN frequency from the clock domain settings
> +        */
> +       np_cdm = of_find_matching_node(NULL, mpc52xx_cdm_ids);
> +       if (!np_cdm) {
> +               dev_err(&of->dev, "can't get clock node!\n");
> +               return 0;
> +       }
> +       cdm = of_iomap(np_cdm, 0);
> +       of_node_put(np_cdm);
> +
> +       if (in_8(&cdm->ipb_clk_sel) & 0x1)
> +               freq *= 2;
> +       val  = in_be32(&cdm->rstcfg);
> +       if (val & (1 << 5))
> +               freq *= 8;
> +       else
> +               freq *= 4;

freq *= (val & (1 << 5)) ? 8 : 4;

> +       if (val & (1 << 6))
> +               freq /= 12;
> +       else
> +               freq /= 16;

Ditto.

> +
> +       iounmap(cdm);
> +
> +       return freq;
> +}
> +
> +/*
> + * Get frequency of the MSCAN clock source
> + *
> + * Either the oscillator clock (SYS_XTAL_IN) or the IP bus clock (IP_CLK)
> + * can be selected. According to the MPC5200 user's manual, the oscillator
> + * clock is the better choice as it has less jitter but due to a hardware
> + * bug, it can not be selected for the old MPC5200 Rev. A chips.
> + */
> +
> +static unsigned int  __devinit mpc52xx_can_clock_freq(struct of_device *of,
> +                                                     int clock_src)
> +{
> +       unsigned int pvr;
> +
> +       pvr = mfspr(SPRN_PVR);
> +
> +       if (clock_src == MSCAN_CLKSRC_BUS || pvr == 0x80822011)
> +               return mpc5xxx_get_bus_frequency(of->node);
> +
> +       return mpc52xx_can_xtal_freq(of);
> +}

mpc52xx_can_xtal_freq() is only used by this function.  Do they need
to be separate?

> +static int __devinit mpc5xxx_can_probe(struct of_device *ofdev,
> +                                      const struct of_device_id *id)
> +{
> +       struct device_node *np = ofdev->node;
> +       struct net_device *dev;
> +       struct mscan_priv *priv;
> +       void __iomem *base;
> +       const char *clk_src;
> +       int err, irq, clock_src;
> +
> +       base = of_iomap(ofdev->node, 0);
> +       if (!base) {
> +               dev_err(&ofdev->dev, "couldn't ioremap\n");
> +               err = -ENOMEM;
> +               goto exit_release_mem;
> +       }
> +
> +       irq = irq_of_parse_and_map(np, 0);
> +       if (!irq) {
> +               dev_err(&ofdev->dev, "no irq found\n");
> +               err = -ENODEV;
> +               goto exit_unmap_mem;
> +       }
> +
> +       dev = alloc_mscandev();
> +       if (!dev) {
> +               err = -ENOMEM;
> +               goto exit_dispose_irq;
> +       }
> +
> +       priv = netdev_priv(dev);
> +       priv->reg_base = base;
> +       dev->irq = irq;
> +
> +       /*
> +        * Either the oscillator clock (SYS_XTAL_IN) or the IP bus clock
> +        * (IP_CLK) can be selected as MSCAN clock source. According to
> +        * the MPC5200 user's manual, the oscillator clock is the better
> +        * choice as it has less jitter. For this reason, it is selected
> +        * by default.
> +        */
> +       clk_src = of_get_property(np, "fsl,mscan-clk-src", NULL);
> +       if (clk_src && strcmp(clk_src, "ip") == 0)

Should protect against non-null.  strncmp() maybe?


> +static struct of_device_id __devinitdata mpc5xxx_can_table[] = {
> +       {.compatible = "fsl,mpc5200-mscan"},
> +       {.compatible = "fsl,mpc5200b-mscan"},
> +       {},
> +};

Ditto here; the 'b' version can be dropped.

> +static int mscan_set_mode(struct net_device *dev, u8 mode)
> +{
> +       struct mscan_priv *priv = netdev_priv(dev);
> +       

[PATCH] net/can: add driver for mscan family & mpc52xx_mscan

2009-11-13 Thread Wolfram Sang
Taken from socketcan-svn, fixed remaining todos, cleaned up, tested with a
phyCORE-MPC5200B-IO and a custom board.

Signed-off-by: Wolfram Sang 
Cc: Wolfgang Grandegger 
Cc: Grant Likely 
Cc: David Miller 
---

This patch is based on net-next as of yesterday.

To make the review easier for those who are already familiar with earlier
versions of this driver (especially for Wolfgang), I put my development branch
online, so you can check my changes incrementally:

http://git.pengutronix.de/?p=wsa/linux-2.6.git;a=summary

 Documentation/powerpc/dts-bindings/fsl/mpc5200.txt |9 +
 drivers/net/can/Kconfig|   19 +
 drivers/net/can/Makefile   |1 +
 drivers/net/can/mscan/Makefile |5 +
 drivers/net/can/mscan/mpc52xx_can.c|  279 
 drivers/net/can/mscan/mscan.c  |  699 
 drivers/net/can/mscan/mscan.h  |  262 
 7 files changed, 1274 insertions(+), 0 deletions(-)
 create mode 100644 drivers/net/can/mscan/Makefile
 create mode 100644 drivers/net/can/mscan/mpc52xx_can.c
 create mode 100644 drivers/net/can/mscan/mscan.c
 create mode 100644 drivers/net/can/mscan/mscan.h

diff --git a/Documentation/powerpc/dts-bindings/fsl/mpc5200.txt 
b/Documentation/powerpc/dts-bindings/fsl/mpc5200.txt
index 8447fd7..b151fb1 100644
--- a/Documentation/powerpc/dts-bindings/fsl/mpc5200.txt
+++ b/Documentation/powerpc/dts-bindings/fsl/mpc5200.txt
@@ -178,3 +178,12 @@ External interrupts:
external irq3:  interrupts = <1 3 n>;
 'n' is sense (0: level high, 1: edge rising, 2: edge falling 3: level low)
 
+fsl,mpc5200-mscan nodes
+---
+In addition to the required compatible-, reg- and interrupt-properites, you can
+also specify which clock shall be used for the bus:
+
+- fsl,mscan-clk-src- a string describing the clock source. Valid values
+ are "ip" for IP_CLK and "sys" for SYS_XTAL.
+ "sys" is the default in case the property is not
+ present.
diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig
index b819cc2..c16e6ff 100644
--- a/drivers/net/can/Kconfig
+++ b/drivers/net/can/Kconfig
@@ -108,6 +108,25 @@ config CAN_MCP251X
---help---
  Driver for the Microchip MCP251x SPI CAN controllers.
 
+config CAN_MSCAN
+   depends on CAN_DEV && (PPC || M68K || M68KNOMMU)
+   tristate "Support for Freescale MSCAN based chips"
+   ---help---
+ The Motorola Scalable Controller Area Network (MSCAN) definition
+ is based on the MSCAN12 definition which is the specific
+ implementation of the Motorola Scalable CAN concept targeted for
+ the Motorola MC68HC12 Microcontroller Family.
+
+config CAN_MPC52XX
+   tristate "Freescale MPC5xxx onboard CAN controller"
+   depends on CAN_MSCAN && PPC_MPC52xx
+   ---help---
+ If you say yes here you get support for Freescale's MPC52xx
+ onboard dualCAN controller.
+
+ This driver can also be built as a module.  If so, the module
+ will be called mpc5xxx_can.
+
 config CAN_DEBUG_DEVICES
bool "CAN devices debugging messages"
depends on CAN
diff --git a/drivers/net/can/Makefile b/drivers/net/can/Makefile
index 1489181..56899fe 100644
--- a/drivers/net/can/Makefile
+++ b/drivers/net/can/Makefile
@@ -10,6 +10,7 @@ can-dev-y := dev.o
 obj-y  += usb/
 
 obj-$(CONFIG_CAN_SJA1000)  += sja1000/
+obj-$(CONFIG_CAN_MSCAN)+= mscan/
 obj-$(CONFIG_CAN_AT91) += at91_can.o
 obj-$(CONFIG_CAN_TI_HECC)  += ti_hecc.o
 obj-$(CONFIG_CAN_MCP251X)  += mcp251x.o
diff --git a/drivers/net/can/mscan/Makefile b/drivers/net/can/mscan/Makefile
new file mode 100644
index 000..2bd9f04
--- /dev/null
+++ b/drivers/net/can/mscan/Makefile
@@ -0,0 +1,5 @@
+
+obj-$(CONFIG_CAN_MPC52XX)  += mscan-mpc52xx.o
+mscan-mpc52xx-objs := mscan.o mpc52xx_can.o
+
+ccflags-$(CONFIG_CAN_DEBUG_DEVICES) := -DDEBUG
diff --git a/drivers/net/can/mscan/mpc52xx_can.c 
b/drivers/net/can/mscan/mpc52xx_can.c
new file mode 100644
index 000..4707a82
--- /dev/null
+++ b/drivers/net/can/mscan/mpc52xx_can.c
@@ -0,0 +1,279 @@
+/*
+ * CAN bus driver for the Freescale MPC5xxx embedded CPU.
+ *
+ * Copyright (C) 2004-2005 Andrey Volkov ,
+ * Varma Electronics Oy
+ * Copyright (C) 2008-2009 Wolfgang Grandegger 
+ * Copyright (C) 2009 Wolfram Sang, Pengutronix 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the version 2 of the GNU General Public License
+ * as published by the Free Software Foundation
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Genera