Re: [PATCHv1 2/6] dmaengine: dw_dmac: add driver for Atmel AT32

2012-09-26 Thread Andy Shevchenko
On Wed, Sep 26, 2012 at 9:51 AM, viresh kumar  wrote:
> On Wed, Sep 26, 2012 at 12:17 PM, Andy Shevchenko
>  wrote:
>
>> This separate driver makes no sense in case it is built properly without
>> CLK framework. Let me check this and leave comments at patch 1/6.
>
> Following is the commit that introduced this change :)
Thanks for pointing to it

>
> commit 93abe8e4b13ae9a0428ce940a8a03ac72a7626f1
> Author: Viresh Kumar 
> Date:   Mon Jul 30 14:39:27 2012 -0700
A-ha, this explains: Heikki's patches are stamped as follows

commit 7c33a7ec5f1f68c1ab06eee7ff7118a7b62db5da
Author: Heikki Krogerus 
Date:   Mon May 7 12:31:29 2012 +0300

>
> clk: add non CONFIG_HAVE_CLK routines
>
> Many drivers are shared between architectures that may or may not have
> HAVE_CLK selected for them.  To remove compilation errors for them we
> enclose clk_*() calls in these drivers within #ifdef CONFIG_HAVE_CLK,
> #endif.
>
> This patch removes the need of these CONFIG_HAVE_CLK statements, by
> introducing dummy routines when HAVE_CLK is not selected by platforms.
> So, definition of these routines will always be available.  These calls
> will return error for platforms that don't select HAVE_CLK.
>
> Signed-off-by: Viresh Kumar 
> Cc: Wolfram Sang 
> Cc: Greg Kroah-Hartman 
> Cc: Jeff Garzik 
> Cc: Andrew Lunn 
> Cc: Bhupesh Sharma 
> Cc: Giuseppe Cavallaro 
> Cc: Russell King 
> Cc: Mike Turquette 
> Cc: Sergei Shtylyov 
> Cc: viresh kumar 
> Signed-off-by: Andrew Morton 
> Signed-off-by: Linus Torvalds 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/



-- 
With Best Regards,
Andy Shevchenko
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCHv1 2/6] dmaengine: dw_dmac: add driver for Atmel AT32

2012-09-26 Thread viresh kumar
On Wed, Sep 26, 2012 at 12:17 PM, Andy Shevchenko
 wrote:

> This separate driver makes no sense in case it is built properly without
> CLK framework. Let me check this and leave comments at patch 1/6.

Following is the commit that introduced this change :)

commit 93abe8e4b13ae9a0428ce940a8a03ac72a7626f1
Author: Viresh Kumar 
Date:   Mon Jul 30 14:39:27 2012 -0700

clk: add non CONFIG_HAVE_CLK routines

Many drivers are shared between architectures that may or may not have
HAVE_CLK selected for them.  To remove compilation errors for them we
enclose clk_*() calls in these drivers within #ifdef CONFIG_HAVE_CLK,
#endif.

This patch removes the need of these CONFIG_HAVE_CLK statements, by
introducing dummy routines when HAVE_CLK is not selected by platforms.
So, definition of these routines will always be available.  These calls
will return error for platforms that don't select HAVE_CLK.

Signed-off-by: Viresh Kumar 
Cc: Wolfram Sang 
Cc: Greg Kroah-Hartman 
Cc: Jeff Garzik 
Cc: Andrew Lunn 
Cc: Bhupesh Sharma 
Cc: Giuseppe Cavallaro 
Cc: Russell King 
Cc: Mike Turquette 
Cc: Sergei Shtylyov 
Cc: viresh kumar 
Signed-off-by: Andrew Morton 
Signed-off-by: Linus Torvalds 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCHv1 2/6] dmaengine: dw_dmac: add driver for Atmel AT32

2012-09-26 Thread Andy Shevchenko
On Wed, 2012-09-26 at 09:20 +0530, viresh kumar wrote: 
> On Tue, Sep 25, 2012 at 5:43 PM, Andy Shevchenko
>  wrote:
> > From: Heikki Krogerus 
> >
> > This driver should be usable on all platforms that depend on clk API.
> 
> This is not what you mentioned in subject :(

> 
> > Signed-off-by: Heikki Krogerus 
> > Signed-off-by: Andy Shevchenko 
> > ---
> >  drivers/dma/Kconfig|9 +++
> >  drivers/dma/Makefile   |1 +
> >  drivers/dma/dw_dmac.c  |   23 +--
> >  drivers/dma/dw_dmac_at32.c |  150 
> > 
> 
> I don't agree with the naming used here. There is nothing AT32
> specific here. It is actively used
> by SPEAr. It should be named dw_dmac_platform.c or *pltfm.c

This separate driver makes no sense in case it is built properly without
CLK framework. Let me check this and leave comments at patch 1/6.

> 
> >  4 files changed, 162 insertions(+), 21 deletions(-)
> >  create mode 100644 drivers/dma/dw_dmac_at32.c
> >
> > diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
> > index df32537..e47cc90 100644
> > --- a/drivers/dma/Kconfig
> > +++ b/drivers/dma/Kconfig
> > @@ -89,6 +89,15 @@ config DW_DMAC
> >   Support the Synopsys DesignWare AHB DMA controller.  This
> >   can be integrated in chips such as the Atmel AT32ap7000.
> >
> > +config DW_DMAC_AT32
> > +   tristate "Synopsys DesignWare AHB DMA support for Atmel"
> 
> :(
> 
> > +   depends on HAVE_CLK
> 
> Even this is not a must for all users of platform driver. So can leave this.
> 
> > +   select DW_DMAC
> > +   default y if CPU_AT32AP7000
> > +   help
> > + Support the Synopsys DesignWare AHB DMA controller in the chips
> > + such as the Atmel AT32ap7000.
> > +
> >  config AT_HDMAC
> > tristate "Atmel AHB DMA support"
> > depends on ARCH_AT91
> 
> > diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
> > index d9344a7..9c8a500 100644
> > --- a/drivers/dma/dw_dmac.c
> > +++ b/drivers/dma/dw_dmac.c
> > @@ -18,7 +18,6 @@
> >  #include 
> >  #include 
> >  #include 
> > -#include 
> >  #include 
> >  #include 
> >  #include 
> > @@ -1681,35 +1680,17 @@ static const struct dev_pm_ops dw_dev_pm_ops = {
> > .poweroff_noirq = dw_suspend_noirq,
> >  };
> >
> > -#ifdef CONFIG_OF
> > -static const struct of_device_id dw_dma_id_table[] = {
> > -   { .compatible = "snps,dma-spear1340" },
> > -   {}
> > -};
> > -MODULE_DEVICE_TABLE(of, dw_dma_id_table);
> > -#endif
> > -
> >  static struct platform_driver dw_driver = {
> > +   .probe  = dw_probe,
> > .remove = __devexit_p(dw_remove),
> > .shutdown   = dw_shutdown,
> > .driver = {
> > .name   = "dw_dmac",
> > .pm = _dev_pm_ops,
> > -   .of_match_table = of_match_ptr(dw_dma_id_table),
> > },
> >  };
> >
> > -static int __init dw_init(void)
> > -{
> > -   return platform_driver_probe(_driver, dw_probe);
> > -}
> > -subsys_initcall(dw_init);
> > -
> > -static void __exit dw_exit(void)
> > -{
> > -   platform_driver_unregister(_driver);
> > -}
> > -module_exit(dw_exit);
> > +module_platform_driver(dw_driver);
> 
> Shouldn't this be a separate patch?
> 
> >
> >  MODULE_LICENSE("GPL v2");
> >  MODULE_DESCRIPTION("Synopsys DesignWare DMA Controller driver");
> > diff --git a/drivers/dma/dw_dmac_at32.c b/drivers/dma/dw_dmac_at32.c
> > new file mode 100644
> > index 000..7bc7ac4
> > --- /dev/null
> > +++ b/drivers/dma/dw_dmac_at32.c
> > @@ -0,0 +1,150 @@
> > +/*
> > + * Driver for the Synopsys DesignWare DMA Controller
> 
> How is this file different from dw_dmac.c?
> 
> > + *
> > + * The driver is based on the excerpts from the original dw_dmac.c. That's 
> > why
> > + * the same copyright holders are mentioned here as well.
> > + *
> > + * Copyright (C) 2007-2008 Atmel Corporation
> > + * Copyright (C) 2010-2011 ST Microelectronics
> > + * Copyright (C) 2012 Intel Corporation
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + */
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +struct dw_at32 {
> > +   struct platform_device  *pdev;
> > +   struct clk  *clk;
> > +};
> > +
> > +static int __init dw_at32_probe(struct platform_device *pdev)
> > +{
> > +   struct dw_at32  *at32;
> > +   struct platform_device  *pd;
> > +   struct dw_dma_platform_data *pdata = pdev->dev.platform_data;
> > +   static int  instance;
> > +   int ret;
> > +
> > +   at32 = devm_kzalloc(>dev, sizeof(*at32), GFP_KERNEL);
> > +   if (!at32) {
> > +   dev_err(>dev, "can't allocate memory\n");
> > +   return -ENOMEM;
> 

Re: [PATCHv1 2/6] dmaengine: dw_dmac: add driver for Atmel AT32

2012-09-26 Thread Andy Shevchenko
On Wed, 2012-09-26 at 09:20 +0530, viresh kumar wrote: 
 On Tue, Sep 25, 2012 at 5:43 PM, Andy Shevchenko
 andriy.shevche...@linux.intel.com wrote:
  From: Heikki Krogerus heikki.kroge...@linux.intel.com
 
  This driver should be usable on all platforms that depend on clk API.
 
 This is not what you mentioned in subject :(

 
  Signed-off-by: Heikki Krogerus heikki.kroge...@linux.intel.com
  Signed-off-by: Andy Shevchenko andriy.shevche...@linux.intel.com
  ---
   drivers/dma/Kconfig|9 +++
   drivers/dma/Makefile   |1 +
   drivers/dma/dw_dmac.c  |   23 +--
   drivers/dma/dw_dmac_at32.c |  150 
  
 
 I don't agree with the naming used here. There is nothing AT32
 specific here. It is actively used
 by SPEAr. It should be named dw_dmac_platform.c or *pltfm.c

This separate driver makes no sense in case it is built properly without
CLK framework. Let me check this and leave comments at patch 1/6.

 
   4 files changed, 162 insertions(+), 21 deletions(-)
   create mode 100644 drivers/dma/dw_dmac_at32.c
 
  diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
  index df32537..e47cc90 100644
  --- a/drivers/dma/Kconfig
  +++ b/drivers/dma/Kconfig
  @@ -89,6 +89,15 @@ config DW_DMAC
Support the Synopsys DesignWare AHB DMA controller.  This
can be integrated in chips such as the Atmel AT32ap7000.
 
  +config DW_DMAC_AT32
  +   tristate Synopsys DesignWare AHB DMA support for Atmel
 
 :(
 
  +   depends on HAVE_CLK
 
 Even this is not a must for all users of platform driver. So can leave this.
 
  +   select DW_DMAC
  +   default y if CPU_AT32AP7000
  +   help
  + Support the Synopsys DesignWare AHB DMA controller in the chips
  + such as the Atmel AT32ap7000.
  +
   config AT_HDMAC
  tristate Atmel AHB DMA support
  depends on ARCH_AT91
 
  diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
  index d9344a7..9c8a500 100644
  --- a/drivers/dma/dw_dmac.c
  +++ b/drivers/dma/dw_dmac.c
  @@ -18,7 +18,6 @@
   #include linux/init.h
   #include linux/interrupt.h
   #include linux/io.h
  -#include linux/of.h
   #include linux/mm.h
   #include linux/module.h
   #include linux/platform_device.h
  @@ -1681,35 +1680,17 @@ static const struct dev_pm_ops dw_dev_pm_ops = {
  .poweroff_noirq = dw_suspend_noirq,
   };
 
  -#ifdef CONFIG_OF
  -static const struct of_device_id dw_dma_id_table[] = {
  -   { .compatible = snps,dma-spear1340 },
  -   {}
  -};
  -MODULE_DEVICE_TABLE(of, dw_dma_id_table);
  -#endif
  -
   static struct platform_driver dw_driver = {
  +   .probe  = dw_probe,
  .remove = __devexit_p(dw_remove),
  .shutdown   = dw_shutdown,
  .driver = {
  .name   = dw_dmac,
  .pm = dw_dev_pm_ops,
  -   .of_match_table = of_match_ptr(dw_dma_id_table),
  },
   };
 
  -static int __init dw_init(void)
  -{
  -   return platform_driver_probe(dw_driver, dw_probe);
  -}
  -subsys_initcall(dw_init);
  -
  -static void __exit dw_exit(void)
  -{
  -   platform_driver_unregister(dw_driver);
  -}
  -module_exit(dw_exit);
  +module_platform_driver(dw_driver);
 
 Shouldn't this be a separate patch?
 
 
   MODULE_LICENSE(GPL v2);
   MODULE_DESCRIPTION(Synopsys DesignWare DMA Controller driver);
  diff --git a/drivers/dma/dw_dmac_at32.c b/drivers/dma/dw_dmac_at32.c
  new file mode 100644
  index 000..7bc7ac4
  --- /dev/null
  +++ b/drivers/dma/dw_dmac_at32.c
  @@ -0,0 +1,150 @@
  +/*
  + * Driver for the Synopsys DesignWare DMA Controller
 
 How is this file different from dw_dmac.c?
 
  + *
  + * The driver is based on the excerpts from the original dw_dmac.c. That's 
  why
  + * the same copyright holders are mentioned here as well.
  + *
  + * Copyright (C) 2007-2008 Atmel Corporation
  + * Copyright (C) 2010-2011 ST Microelectronics
  + * Copyright (C) 2012 Intel Corporation
  + *
  + * This program is free software; you can redistribute it and/or modify
  + * it under the terms of the GNU General Public License version 2 as
  + * published by the Free Software Foundation.
  + */
  +#include linux/module.h
  +#include linux/platform_device.h
  +#include linux/slab.h
  +#include linux/clk.h
  +#include linux/of.h
  +#include linux/dw_dmac.h
  +
  +struct dw_at32 {
  +   struct platform_device  *pdev;
  +   struct clk  *clk;
  +};
  +
  +static int __init dw_at32_probe(struct platform_device *pdev)
  +{
  +   struct dw_at32  *at32;
  +   struct platform_device  *pd;
  +   struct dw_dma_platform_data *pdata = pdev-dev.platform_data;
  +   static int  instance;
  +   int ret;
  +
  +   at32 = devm_kzalloc(pdev-dev, sizeof(*at32), GFP_KERNEL);
  +   if (!at32) {
  +   dev_err(pdev-dev, can't allocate memory\n);
  +   

Re: [PATCHv1 2/6] dmaengine: dw_dmac: add driver for Atmel AT32

2012-09-26 Thread viresh kumar
On Wed, Sep 26, 2012 at 12:17 PM, Andy Shevchenko
andriy.shevche...@linux.intel.com wrote:

 This separate driver makes no sense in case it is built properly without
 CLK framework. Let me check this and leave comments at patch 1/6.

Following is the commit that introduced this change :)

commit 93abe8e4b13ae9a0428ce940a8a03ac72a7626f1
Author: Viresh Kumar viresh.ku...@st.com
Date:   Mon Jul 30 14:39:27 2012 -0700

clk: add non CONFIG_HAVE_CLK routines

Many drivers are shared between architectures that may or may not have
HAVE_CLK selected for them.  To remove compilation errors for them we
enclose clk_*() calls in these drivers within #ifdef CONFIG_HAVE_CLK,
#endif.

This patch removes the need of these CONFIG_HAVE_CLK statements, by
introducing dummy routines when HAVE_CLK is not selected by platforms.
So, definition of these routines will always be available.  These calls
will return error for platforms that don't select HAVE_CLK.

Signed-off-by: Viresh Kumar viresh.ku...@st.com
Cc: Wolfram Sang w.s...@pengutronix.de
Cc: Greg Kroah-Hartman gre...@linuxfoundation.org
Cc: Jeff Garzik jgar...@redhat.com
Cc: Andrew Lunn and...@lunn.ch
Cc: Bhupesh Sharma bhupesh.sha...@st.com
Cc: Giuseppe Cavallaro peppe.cavall...@st.com
Cc: Russell King r...@arm.linux.org.uk
Cc: Mike Turquette mturque...@linaro.org
Cc: Sergei Shtylyov sshtyl...@ru.mvista.com
Cc: viresh kumar viresh.li...@gmail.com
Signed-off-by: Andrew Morton a...@linux-foundation.org
Signed-off-by: Linus Torvalds torva...@linux-foundation.org
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCHv1 2/6] dmaengine: dw_dmac: add driver for Atmel AT32

2012-09-26 Thread Andy Shevchenko
On Wed, Sep 26, 2012 at 9:51 AM, viresh kumar viresh.ku...@linaro.org wrote:
 On Wed, Sep 26, 2012 at 12:17 PM, Andy Shevchenko
 andriy.shevche...@linux.intel.com wrote:

 This separate driver makes no sense in case it is built properly without
 CLK framework. Let me check this and leave comments at patch 1/6.

 Following is the commit that introduced this change :)
Thanks for pointing to it


 commit 93abe8e4b13ae9a0428ce940a8a03ac72a7626f1
 Author: Viresh Kumar viresh.ku...@st.com
 Date:   Mon Jul 30 14:39:27 2012 -0700
A-ha, this explains: Heikki's patches are stamped as follows

commit 7c33a7ec5f1f68c1ab06eee7ff7118a7b62db5da
Author: Heikki Krogerus heikki.kroge...@linux.intel.com
Date:   Mon May 7 12:31:29 2012 +0300


 clk: add non CONFIG_HAVE_CLK routines

 Many drivers are shared between architectures that may or may not have
 HAVE_CLK selected for them.  To remove compilation errors for them we
 enclose clk_*() calls in these drivers within #ifdef CONFIG_HAVE_CLK,
 #endif.

 This patch removes the need of these CONFIG_HAVE_CLK statements, by
 introducing dummy routines when HAVE_CLK is not selected by platforms.
 So, definition of these routines will always be available.  These calls
 will return error for platforms that don't select HAVE_CLK.

 Signed-off-by: Viresh Kumar viresh.ku...@st.com
 Cc: Wolfram Sang w.s...@pengutronix.de
 Cc: Greg Kroah-Hartman gre...@linuxfoundation.org
 Cc: Jeff Garzik jgar...@redhat.com
 Cc: Andrew Lunn and...@lunn.ch
 Cc: Bhupesh Sharma bhupesh.sha...@st.com
 Cc: Giuseppe Cavallaro peppe.cavall...@st.com
 Cc: Russell King r...@arm.linux.org.uk
 Cc: Mike Turquette mturque...@linaro.org
 Cc: Sergei Shtylyov sshtyl...@ru.mvista.com
 Cc: viresh kumar viresh.li...@gmail.com
 Signed-off-by: Andrew Morton a...@linux-foundation.org
 Signed-off-by: Linus Torvalds torva...@linux-foundation.org
 --
 To unsubscribe from this list: send the line unsubscribe linux-kernel in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 Please read the FAQ at  http://www.tux.org/lkml/



-- 
With Best Regards,
Andy Shevchenko
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCHv1 2/6] dmaengine: dw_dmac: add driver for Atmel AT32

2012-09-25 Thread viresh kumar
On Tue, Sep 25, 2012 at 5:43 PM, Andy Shevchenko
 wrote:
> diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
> index df32537..e47cc90 100644
> --- a/drivers/dma/Kconfig
> +++ b/drivers/dma/Kconfig
> @@ -89,6 +89,15 @@ config DW_DMAC
>   Support the Synopsys DesignWare AHB DMA controller.  This
>   can be integrated in chips such as the Atmel AT32ap7000.
>
> +config DW_DMAC_AT32
> +   tristate "Synopsys DesignWare AHB DMA support for Atmel"
> +   depends on HAVE_CLK
> +   select DW_DMAC
> +   default y if CPU_AT32AP7000

Also, remove
default y if CPU_AT32AP7000 from  config DW_DMAC
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCHv1 2/6] dmaengine: dw_dmac: add driver for Atmel AT32

2012-09-25 Thread viresh kumar
On Tue, Sep 25, 2012 at 5:43 PM, Andy Shevchenko
 wrote:
> From: Heikki Krogerus 
>
> This driver should be usable on all platforms that depend on clk API.

This is not what you mentioned in subject :(

> Signed-off-by: Heikki Krogerus 
> Signed-off-by: Andy Shevchenko 
> ---
>  drivers/dma/Kconfig|9 +++
>  drivers/dma/Makefile   |1 +
>  drivers/dma/dw_dmac.c  |   23 +--
>  drivers/dma/dw_dmac_at32.c |  150 
> 

I don't agree with the naming used here. There is nothing AT32
specific here. It is actively used
by SPEAr. It should be named dw_dmac_platform.c or *pltfm.c

>  4 files changed, 162 insertions(+), 21 deletions(-)
>  create mode 100644 drivers/dma/dw_dmac_at32.c
>
> diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
> index df32537..e47cc90 100644
> --- a/drivers/dma/Kconfig
> +++ b/drivers/dma/Kconfig
> @@ -89,6 +89,15 @@ config DW_DMAC
>   Support the Synopsys DesignWare AHB DMA controller.  This
>   can be integrated in chips such as the Atmel AT32ap7000.
>
> +config DW_DMAC_AT32
> +   tristate "Synopsys DesignWare AHB DMA support for Atmel"

:(

> +   depends on HAVE_CLK

Even this is not a must for all users of platform driver. So can leave this.

> +   select DW_DMAC
> +   default y if CPU_AT32AP7000
> +   help
> + Support the Synopsys DesignWare AHB DMA controller in the chips
> + such as the Atmel AT32ap7000.
> +
>  config AT_HDMAC
> tristate "Atmel AHB DMA support"
> depends on ARCH_AT91

> diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
> index d9344a7..9c8a500 100644
> --- a/drivers/dma/dw_dmac.c
> +++ b/drivers/dma/dw_dmac.c
> @@ -18,7 +18,6 @@
>  #include 
>  #include 
>  #include 
> -#include 
>  #include 
>  #include 
>  #include 
> @@ -1681,35 +1680,17 @@ static const struct dev_pm_ops dw_dev_pm_ops = {
> .poweroff_noirq = dw_suspend_noirq,
>  };
>
> -#ifdef CONFIG_OF
> -static const struct of_device_id dw_dma_id_table[] = {
> -   { .compatible = "snps,dma-spear1340" },
> -   {}
> -};
> -MODULE_DEVICE_TABLE(of, dw_dma_id_table);
> -#endif
> -
>  static struct platform_driver dw_driver = {
> +   .probe  = dw_probe,
> .remove = __devexit_p(dw_remove),
> .shutdown   = dw_shutdown,
> .driver = {
> .name   = "dw_dmac",
> .pm = _dev_pm_ops,
> -   .of_match_table = of_match_ptr(dw_dma_id_table),
> },
>  };
>
> -static int __init dw_init(void)
> -{
> -   return platform_driver_probe(_driver, dw_probe);
> -}
> -subsys_initcall(dw_init);
> -
> -static void __exit dw_exit(void)
> -{
> -   platform_driver_unregister(_driver);
> -}
> -module_exit(dw_exit);
> +module_platform_driver(dw_driver);

Shouldn't this be a separate patch?

>
>  MODULE_LICENSE("GPL v2");
>  MODULE_DESCRIPTION("Synopsys DesignWare DMA Controller driver");
> diff --git a/drivers/dma/dw_dmac_at32.c b/drivers/dma/dw_dmac_at32.c
> new file mode 100644
> index 000..7bc7ac4
> --- /dev/null
> +++ b/drivers/dma/dw_dmac_at32.c
> @@ -0,0 +1,150 @@
> +/*
> + * Driver for the Synopsys DesignWare DMA Controller

How is this file different from dw_dmac.c?

> + *
> + * The driver is based on the excerpts from the original dw_dmac.c. That's 
> why
> + * the same copyright holders are mentioned here as well.
> + *
> + * Copyright (C) 2007-2008 Atmel Corporation
> + * Copyright (C) 2010-2011 ST Microelectronics
> + * Copyright (C) 2012 Intel Corporation
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +struct dw_at32 {
> +   struct platform_device  *pdev;
> +   struct clk  *clk;
> +};
> +
> +static int __init dw_at32_probe(struct platform_device *pdev)
> +{
> +   struct dw_at32  *at32;
> +   struct platform_device  *pd;
> +   struct dw_dma_platform_data *pdata = pdev->dev.platform_data;
> +   static int  instance;
> +   int ret;
> +
> +   at32 = devm_kzalloc(>dev, sizeof(*at32), GFP_KERNEL);
> +   if (!at32) {
> +   dev_err(>dev, "can't allocate memory\n");
> +   return -ENOMEM;
> +   }
> +
> +   pd = platform_device_alloc("dw_dmac", instance);
> +   if (!pd) {
> +   dev_err(>dev, "can't allocate dw_dmac platform 
> device\n");
> +   return -ENOMEM;
> +   }

Why create another device? Why not simply call dw_probe() from here?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the 

Re: [PATCHv1 2/6] dmaengine: dw_dmac: add driver for Atmel AT32

2012-09-25 Thread viresh kumar
On Tue, Sep 25, 2012 at 5:43 PM, Andy Shevchenko
andriy.shevche...@linux.intel.com wrote:
 From: Heikki Krogerus heikki.kroge...@linux.intel.com

 This driver should be usable on all platforms that depend on clk API.

This is not what you mentioned in subject :(

 Signed-off-by: Heikki Krogerus heikki.kroge...@linux.intel.com
 Signed-off-by: Andy Shevchenko andriy.shevche...@linux.intel.com
 ---
  drivers/dma/Kconfig|9 +++
  drivers/dma/Makefile   |1 +
  drivers/dma/dw_dmac.c  |   23 +--
  drivers/dma/dw_dmac_at32.c |  150 
 

I don't agree with the naming used here. There is nothing AT32
specific here. It is actively used
by SPEAr. It should be named dw_dmac_platform.c or *pltfm.c

  4 files changed, 162 insertions(+), 21 deletions(-)
  create mode 100644 drivers/dma/dw_dmac_at32.c

 diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
 index df32537..e47cc90 100644
 --- a/drivers/dma/Kconfig
 +++ b/drivers/dma/Kconfig
 @@ -89,6 +89,15 @@ config DW_DMAC
   Support the Synopsys DesignWare AHB DMA controller.  This
   can be integrated in chips such as the Atmel AT32ap7000.

 +config DW_DMAC_AT32
 +   tristate Synopsys DesignWare AHB DMA support for Atmel

:(

 +   depends on HAVE_CLK

Even this is not a must for all users of platform driver. So can leave this.

 +   select DW_DMAC
 +   default y if CPU_AT32AP7000
 +   help
 + Support the Synopsys DesignWare AHB DMA controller in the chips
 + such as the Atmel AT32ap7000.
 +
  config AT_HDMAC
 tristate Atmel AHB DMA support
 depends on ARCH_AT91

 diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
 index d9344a7..9c8a500 100644
 --- a/drivers/dma/dw_dmac.c
 +++ b/drivers/dma/dw_dmac.c
 @@ -18,7 +18,6 @@
  #include linux/init.h
  #include linux/interrupt.h
  #include linux/io.h
 -#include linux/of.h
  #include linux/mm.h
  #include linux/module.h
  #include linux/platform_device.h
 @@ -1681,35 +1680,17 @@ static const struct dev_pm_ops dw_dev_pm_ops = {
 .poweroff_noirq = dw_suspend_noirq,
  };

 -#ifdef CONFIG_OF
 -static const struct of_device_id dw_dma_id_table[] = {
 -   { .compatible = snps,dma-spear1340 },
 -   {}
 -};
 -MODULE_DEVICE_TABLE(of, dw_dma_id_table);
 -#endif
 -
  static struct platform_driver dw_driver = {
 +   .probe  = dw_probe,
 .remove = __devexit_p(dw_remove),
 .shutdown   = dw_shutdown,
 .driver = {
 .name   = dw_dmac,
 .pm = dw_dev_pm_ops,
 -   .of_match_table = of_match_ptr(dw_dma_id_table),
 },
  };

 -static int __init dw_init(void)
 -{
 -   return platform_driver_probe(dw_driver, dw_probe);
 -}
 -subsys_initcall(dw_init);
 -
 -static void __exit dw_exit(void)
 -{
 -   platform_driver_unregister(dw_driver);
 -}
 -module_exit(dw_exit);
 +module_platform_driver(dw_driver);

Shouldn't this be a separate patch?


  MODULE_LICENSE(GPL v2);
  MODULE_DESCRIPTION(Synopsys DesignWare DMA Controller driver);
 diff --git a/drivers/dma/dw_dmac_at32.c b/drivers/dma/dw_dmac_at32.c
 new file mode 100644
 index 000..7bc7ac4
 --- /dev/null
 +++ b/drivers/dma/dw_dmac_at32.c
 @@ -0,0 +1,150 @@
 +/*
 + * Driver for the Synopsys DesignWare DMA Controller

How is this file different from dw_dmac.c?

 + *
 + * The driver is based on the excerpts from the original dw_dmac.c. That's 
 why
 + * the same copyright holders are mentioned here as well.
 + *
 + * Copyright (C) 2007-2008 Atmel Corporation
 + * Copyright (C) 2010-2011 ST Microelectronics
 + * Copyright (C) 2012 Intel Corporation
 + *
 + * This program is free software; you can redistribute it and/or modify
 + * it under the terms of the GNU General Public License version 2 as
 + * published by the Free Software Foundation.
 + */
 +#include linux/module.h
 +#include linux/platform_device.h
 +#include linux/slab.h
 +#include linux/clk.h
 +#include linux/of.h
 +#include linux/dw_dmac.h
 +
 +struct dw_at32 {
 +   struct platform_device  *pdev;
 +   struct clk  *clk;
 +};
 +
 +static int __init dw_at32_probe(struct platform_device *pdev)
 +{
 +   struct dw_at32  *at32;
 +   struct platform_device  *pd;
 +   struct dw_dma_platform_data *pdata = pdev-dev.platform_data;
 +   static int  instance;
 +   int ret;
 +
 +   at32 = devm_kzalloc(pdev-dev, sizeof(*at32), GFP_KERNEL);
 +   if (!at32) {
 +   dev_err(pdev-dev, can't allocate memory\n);
 +   return -ENOMEM;
 +   }
 +
 +   pd = platform_device_alloc(dw_dmac, instance);
 +   if (!pd) {
 +   dev_err(pdev-dev, can't allocate dw_dmac platform 
 device\n);
 +   return -ENOMEM;
 +   }

Why create another device? Why not simply call dw_probe() from here?
--
To unsubscribe from this list: send 

Re: [PATCHv1 2/6] dmaengine: dw_dmac: add driver for Atmel AT32

2012-09-25 Thread viresh kumar
On Tue, Sep 25, 2012 at 5:43 PM, Andy Shevchenko
andriy.shevche...@linux.intel.com wrote:
 diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
 index df32537..e47cc90 100644
 --- a/drivers/dma/Kconfig
 +++ b/drivers/dma/Kconfig
 @@ -89,6 +89,15 @@ config DW_DMAC
   Support the Synopsys DesignWare AHB DMA controller.  This
   can be integrated in chips such as the Atmel AT32ap7000.

 +config DW_DMAC_AT32
 +   tristate Synopsys DesignWare AHB DMA support for Atmel
 +   depends on HAVE_CLK
 +   select DW_DMAC
 +   default y if CPU_AT32AP7000

Also, remove
default y if CPU_AT32AP7000 from  config DW_DMAC
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/