Re: [PATCHv1 2/6] dmaengine: dw_dmac: add driver for Atmel AT32
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
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
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 = &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
> > +#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(&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
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
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 = &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
> +#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(&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 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-
[PATCHv1 2/6] dmaengine: dw_dmac: add driver for Atmel AT32
From: Heikki Krogerus
This driver should be usable on all platforms that depend on clk API.
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
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
+ 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/Makefile b/drivers/dma/Makefile
index 7428fea..d76020b 100644
--- a/drivers/dma/Makefile
+++ b/drivers/dma/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_FSL_DMA) += fsldma.o
obj-$(CONFIG_MPC512X_DMA) += mpc512x_dma.o
obj-$(CONFIG_MV_XOR) += mv_xor.o
obj-$(CONFIG_DW_DMAC) += dw_dmac.o
+obj-$(CONFIG_DW_DMAC_AT32) += dw_dmac_at32.o
obj-$(CONFIG_AT_HDMAC) += at_hdmac.o
obj-$(CONFIG_MX3_IPU) += ipu/
obj-$(CONFIG_TXX9_DMAC) += txx9dmac.o
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 = &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);
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
+ *
+ * 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(&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;
+ }
+
+ platform_set_drvdata(pdev, at32);
+
+ pd->dev.parent = &pdev->dev;
+ at32->pdev = pd;
+
+ at32->clk = devm_clk_get(&pdev->dev, "hclk");
+ if (IS_ERR(at32->clk)) {
+ dev_err(&pdev->dev, "failed to get clock\n");
+ ret = -EINVAL;
+ goto err0;
+ }
+ clk_prepare_enable(at32->clk);
+
+ ret = platform_device_add_resources(pd, pdev->resource,
+ pdev->num_res
