Patchwork [5/6] ARM: mxc: add dt support for audmux-v2

login
register
mail settings
Submitter Richard Zhao
Date Jan. 6, 2012, 3:25 a.m.
Message ID <1325820343-11875-6-git-send-email-richard.zhao@linaro.org>
Download mbox | patch
Permalink /patch/134589/
State New
Headers show

Comments

Richard Zhao - Jan. 6, 2012, 3:25 a.m.
If CONFIG_OF, we create a audmux device driver.

For other platforms that don't set CONFIG_OF, it still initialize
everything in postcore_initcall. I didn't create device driver for
them, because it would be big change and one day the platforms will
convert to device tree too.

Signed-off-by: Richard Zhao <richard.zhao@linaro.org>
---
 arch/arm/plat-mxc/audmux-v2.c |   59 +++++++++++++++++++++++++++++++++++++++++
 1 files changed, 59 insertions(+), 0 deletions(-)
Russell King - ARM Linux - Jan. 6, 2012, 8:56 a.m.
On Fri, Jan 06, 2012 at 11:25:42AM +0800, Richard Zhao wrote:
> +#ifdef CONFIG_OF
> +
> +static int audmux_v2_probe(struct platform_device *pdev)
> +{
> +	struct resource *res;
> +	int ret = 0;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!res)
> +		return -ENODEV;
> +	if (!request_mem_region(res->start, resource_size(res), "audmux_v2")) {
> +		dev_err(&pdev->dev, "request_mem_region failed\n");
> +		return -EBUSY;
> +	}
> +
> +	audmux_base = ioremap(res->start, resource_size(res));
> +	if (!audmux_base) {
> +		dev_err(&pdev->dev, "ioremap failed\n");
> +		ret = -ENODEV;
> +		goto failed_ioremap;
> +	}
> +
> +	audmux_clk = clk_get(NULL, "audmux");

NAK.  You have a struct device.  Use it.
Sascha Hauer - Jan. 6, 2012, 9:13 a.m.
On Fri, Jan 06, 2012 at 11:25:42AM +0800, Richard Zhao wrote:
> If CONFIG_OF, we create a audmux device driver.
> 
> For other platforms that don't set CONFIG_OF, it still initialize
> everything in postcore_initcall. I didn't create device driver for
> them, because it would be big change and one day the platforms will
> convert to device tree too.
> 
> Signed-off-by: Richard Zhao <richard.zhao@linaro.org>
> ---
>  arch/arm/plat-mxc/audmux-v2.c |   59 +++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 59 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/plat-mxc/audmux-v2.c b/arch/arm/plat-mxc/audmux-v2.c
> index 8cced35..87e05e2 100644
> --- a/arch/arm/plat-mxc/audmux-v2.c
> +++ b/arch/arm/plat-mxc/audmux-v2.c
> @@ -20,7 +20,9 @@
>  #include <linux/io.h>
>  #include <linux/clk.h>
>  #include <linux/debugfs.h>
> +#include <linux/platform_device.h>
>  #include <linux/slab.h>
> +#include <linux/of.h>
>  #include <mach/audmux.h>
>  #include <mach/hardware.h>
>  
> @@ -184,8 +186,64 @@ int mxc_audmux_v2_configure_port(unsigned int port, unsigned int ptcr,
>  }
>  EXPORT_SYMBOL_GPL(mxc_audmux_v2_configure_port);
>  
> +#ifdef CONFIG_OF
> +
> +static int audmux_v2_probe(struct platform_device *pdev)
> +{
> +	struct resource *res;
> +	int ret = 0;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!res)
> +		return -ENODEV;
> +	if (!request_mem_region(res->start, resource_size(res), "audmux_v2")) {
> +		dev_err(&pdev->dev, "request_mem_region failed\n");
> +		return -EBUSY;
> +	}
> +
> +	audmux_base = ioremap(res->start, resource_size(res));
> +	if (!audmux_base) {
> +		dev_err(&pdev->dev, "ioremap failed\n");
> +		ret = -ENODEV;
> +		goto failed_ioremap;
> +	}
> +
> +	audmux_clk = clk_get(NULL, "audmux");
> +	if (IS_ERR(audmux_clk)) {
> +			dev_warn(&pdev->dev, "%s: cannot get clock: %d\n",
> +				__func__, ret);
> +			audmux_clk = NULL;
> +	}
> +
> +	audmux_debugfs_init();
> +	return 0;
> +
> +failed_ioremap:
> +	release_mem_region(res->start, resource_size(res));
> +
> +	return ret;
> +}
> +
> +static const struct of_device_id audmux_v2_dt_ids[] = {
> +	{ .compatible = "fsl,audmux-v2", },
> +	{ /* sentinel */ }
> +};
> +
> +static struct platform_driver audmux_v2_driver = {
> +	.probe = audmux_v2_probe,
> +	.driver = {
> +		.name = "audmux_v2",
> +		.of_match_table = audmux_v2_dt_ids,
> +	},
> +};
> +
> +#endif
> +
>  static int mxc_audmux_v2_init(void)
>  {
> +#ifdef CONFIG_OF
> +	return platform_driver_register(&audmux_v2_driver);
> +#else

Don't do this. Just because CONFIG_OF is enabled does not mean that this
kernel is only used for OF. This #else breaks starting the kernel the
traditional machid way.

Sascha
Richard Zhao - Jan. 6, 2012, 9:14 a.m.
On Fri, Jan 06, 2012 at 08:56:17AM +0000, Russell King - ARM Linux wrote:
> On Fri, Jan 06, 2012 at 11:25:42AM +0800, Richard Zhao wrote:
> > +#ifdef CONFIG_OF
> > +
> > +static int audmux_v2_probe(struct platform_device *pdev)
> > +{
> > +	struct resource *res;
> > +	int ret = 0;
> > +
> > +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +	if (!res)
> > +		return -ENODEV;
> > +	if (!request_mem_region(res->start, resource_size(res), "audmux_v2")) {
> > +		dev_err(&pdev->dev, "request_mem_region failed\n");
> > +		return -EBUSY;
> > +	}
> > +
> > +	audmux_base = ioremap(res->start, resource_size(res));
> > +	if (!audmux_base) {
> > +		dev_err(&pdev->dev, "ioremap failed\n");
> > +		ret = -ENODEV;
> > +		goto failed_ioremap;
> > +	}
> > +
> > +	audmux_clk = clk_get(NULL, "audmux");
> 
> NAK.  You have a struct device.  Use it.
Right, Thanks.

Richard
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
Richard Zhao - Jan. 6, 2012, 9:21 a.m.
On Fri, Jan 06, 2012 at 10:13:15AM +0100, Sascha Hauer wrote:
> On Fri, Jan 06, 2012 at 11:25:42AM +0800, Richard Zhao wrote:
> > If CONFIG_OF, we create a audmux device driver.
> > 
> > For other platforms that don't set CONFIG_OF, it still initialize
> > everything in postcore_initcall. I didn't create device driver for
> > them, because it would be big change and one day the platforms will
> > convert to device tree too.
> > 
> > Signed-off-by: Richard Zhao <richard.zhao@linaro.org>
> > ---
> >  arch/arm/plat-mxc/audmux-v2.c |   59 +++++++++++++++++++++++++++++++++++++++++
> >  1 files changed, 59 insertions(+), 0 deletions(-)
> > 
> > diff --git a/arch/arm/plat-mxc/audmux-v2.c b/arch/arm/plat-mxc/audmux-v2.c
> > index 8cced35..87e05e2 100644
> > --- a/arch/arm/plat-mxc/audmux-v2.c
> > +++ b/arch/arm/plat-mxc/audmux-v2.c
> > @@ -20,7 +20,9 @@
> >  #include <linux/io.h>
> >  #include <linux/clk.h>
> >  #include <linux/debugfs.h>
> > +#include <linux/platform_device.h>
> >  #include <linux/slab.h>
> > +#include <linux/of.h>
> >  #include <mach/audmux.h>
> >  #include <mach/hardware.h>
> >  
> > @@ -184,8 +186,64 @@ int mxc_audmux_v2_configure_port(unsigned int port, unsigned int ptcr,
> >  }
> >  EXPORT_SYMBOL_GPL(mxc_audmux_v2_configure_port);
> >  
> > +#ifdef CONFIG_OF
> > +
> > +static int audmux_v2_probe(struct platform_device *pdev)
> > +{
> > +	struct resource *res;
> > +	int ret = 0;
> > +
> > +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +	if (!res)
> > +		return -ENODEV;
> > +	if (!request_mem_region(res->start, resource_size(res), "audmux_v2")) {
> > +		dev_err(&pdev->dev, "request_mem_region failed\n");
> > +		return -EBUSY;
> > +	}
> > +
> > +	audmux_base = ioremap(res->start, resource_size(res));
> > +	if (!audmux_base) {
> > +		dev_err(&pdev->dev, "ioremap failed\n");
> > +		ret = -ENODEV;
> > +		goto failed_ioremap;
> > +	}
> > +
> > +	audmux_clk = clk_get(NULL, "audmux");
> > +	if (IS_ERR(audmux_clk)) {
> > +			dev_warn(&pdev->dev, "%s: cannot get clock: %d\n",
> > +				__func__, ret);
> > +			audmux_clk = NULL;
> > +	}
> > +
> > +	audmux_debugfs_init();
> > +	return 0;
> > +
> > +failed_ioremap:
> > +	release_mem_region(res->start, resource_size(res));
> > +
> > +	return ret;
> > +}
> > +
> > +static const struct of_device_id audmux_v2_dt_ids[] = {
> > +	{ .compatible = "fsl,audmux-v2", },
> > +	{ /* sentinel */ }
> > +};
> > +
> > +static struct platform_driver audmux_v2_driver = {
> > +	.probe = audmux_v2_probe,
> > +	.driver = {
> > +		.name = "audmux_v2",
> > +		.of_match_table = audmux_v2_dt_ids,
> > +	},
> > +};
> > +
> > +#endif
> > +
> >  static int mxc_audmux_v2_init(void)
> >  {
> > +#ifdef CONFIG_OF
> > +	return platform_driver_register(&audmux_v2_driver);
> > +#else
> 
> Don't do this. Just because CONFIG_OF is enabled does not mean that this
> kernel is only used for OF. This #else breaks starting the kernel the
> traditional machid way.
You are right. Look like I have to convert audmux to platform devices
for all platforms.

Thanks
Richard
> 
> Sascha
> 
> 
> -- 
> Pengutronix e.K.                           |                             |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
Sascha Hauer - Jan. 6, 2012, 9:38 a.m.
On Fri, Jan 06, 2012 at 05:21:24PM +0800, Richard Zhao wrote:
> On Fri, Jan 06, 2012 at 10:13:15AM +0100, Sascha Hauer wrote:
> > On Fri, Jan 06, 2012 at 11:25:42AM +0800, Richard Zhao wrote:
> > > If CONFIG_OF, we create a audmux device driver.
> > > 
> > > For other platforms that don't set CONFIG_OF, it still initialize
> > > everything in postcore_initcall. I didn't create device driver for
> > > them, because it would be big change and one day the platforms will
> > > convert to device tree too.
> > > 
> > > Signed-off-by: Richard Zhao <richard.zhao@linaro.org>
> > > ---
> > >  arch/arm/plat-mxc/audmux-v2.c |   59 +++++++++++++++++++++++++++++++++++++++++
> > >  1 files changed, 59 insertions(+), 0 deletions(-)
> > > 
> > > diff --git a/arch/arm/plat-mxc/audmux-v2.c b/arch/arm/plat-mxc/audmux-v2.c
> > > index 8cced35..87e05e2 100644
> > > --- a/arch/arm/plat-mxc/audmux-v2.c
> > > +++ b/arch/arm/plat-mxc/audmux-v2.c
> > > @@ -20,7 +20,9 @@
> > >  #include <linux/io.h>
> > >  #include <linux/clk.h>
> > >  #include <linux/debugfs.h>
> > > +#include <linux/platform_device.h>
> > >  #include <linux/slab.h>
> > > +#include <linux/of.h>
> > >  #include <mach/audmux.h>
> > >  #include <mach/hardware.h>
> > >  
> > > @@ -184,8 +186,64 @@ int mxc_audmux_v2_configure_port(unsigned int port, unsigned int ptcr,
> > >  }
> > >  EXPORT_SYMBOL_GPL(mxc_audmux_v2_configure_port);
> > >  
> > > +#ifdef CONFIG_OF
> > > +
> > > +static int audmux_v2_probe(struct platform_device *pdev)
> > > +{
> > > +	struct resource *res;
> > > +	int ret = 0;
> > > +
> > > +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > > +	if (!res)
> > > +		return -ENODEV;
> > > +	if (!request_mem_region(res->start, resource_size(res), "audmux_v2")) {
> > > +		dev_err(&pdev->dev, "request_mem_region failed\n");
> > > +		return -EBUSY;
> > > +	}
> > > +
> > > +	audmux_base = ioremap(res->start, resource_size(res));
> > > +	if (!audmux_base) {
> > > +		dev_err(&pdev->dev, "ioremap failed\n");
> > > +		ret = -ENODEV;
> > > +		goto failed_ioremap;
> > > +	}
> > > +
> > > +	audmux_clk = clk_get(NULL, "audmux");
> > > +	if (IS_ERR(audmux_clk)) {
> > > +			dev_warn(&pdev->dev, "%s: cannot get clock: %d\n",
> > > +				__func__, ret);
> > > +			audmux_clk = NULL;
> > > +	}
> > > +
> > > +	audmux_debugfs_init();
> > > +	return 0;
> > > +
> > > +failed_ioremap:
> > > +	release_mem_region(res->start, resource_size(res));
> > > +
> > > +	return ret;
> > > +}
> > > +
> > > +static const struct of_device_id audmux_v2_dt_ids[] = {
> > > +	{ .compatible = "fsl,audmux-v2", },
> > > +	{ /* sentinel */ }
> > > +};
> > > +
> > > +static struct platform_driver audmux_v2_driver = {
> > > +	.probe = audmux_v2_probe,
> > > +	.driver = {
> > > +		.name = "audmux_v2",
> > > +		.of_match_table = audmux_v2_dt_ids,
> > > +	},
> > > +};
> > > +
> > > +#endif
> > > +
> > >  static int mxc_audmux_v2_init(void)
> > >  {
> > > +#ifdef CONFIG_OF
> > > +	return platform_driver_register(&audmux_v2_driver);
> > > +#else
> > 
> > Don't do this. Just because CONFIG_OF is enabled does not mean that this
> > kernel is only used for OF. This #else breaks starting the kernel the
> > traditional machid way.
> You are right. Look like I have to convert audmux to platform devices
> for all platforms.

I hoped you would say this :)

Sascha
Shawn Guo - Jan. 11, 2012, 5:26 a.m.
On Fri, Jan 06, 2012 at 05:21:24PM +0800, Richard Zhao wrote:
> You are right. Look like I have to convert audmux to platform devices
> for all platforms.
> 
With doing so, it may be also good for us to move this driver into
sound/soc/imx/.  Any particular reason to keep it in platform?
Richard Zhao - Jan. 11, 2012, 1:02 p.m.
On Wed, Jan 11, 2012 at 01:26:06PM +0800, Shawn Guo wrote:
> On Fri, Jan 06, 2012 at 05:21:24PM +0800, Richard Zhao wrote:
> > You are right. Look like I have to convert audmux to platform devices
> > for all platforms.
> > 
> With doing so, it may be also good for us to move this driver into
> sound/soc/imx/.  Any particular reason to keep it in platform?
Yes, we can move it out. But I'm not sure we can put it in asoc,
because it's not a standard part of ASoC. Mark?

Thanks
Richard
> 
> -- 
> Regards,
> Shawn
>
Mark Brown - Jan. 11, 2012, 5:38 p.m.
On Wed, Jan 11, 2012 at 09:02:44PM +0800, Richard Zhao wrote:
> On Wed, Jan 11, 2012 at 01:26:06PM +0800, Shawn Guo wrote:

> > With doing so, it may be also good for us to move this driver into
> > sound/soc/imx/.  Any particular reason to keep it in platform?

> Yes, we can move it out. But I'm not sure we can put it in asoc,
> because it's not a standard part of ASoC. Mark?

It should be supported using standard ASoC mechanisms.
Richard Zhao - Jan. 12, 2012, 8:54 a.m.
On Wed, Jan 11, 2012 at 05:38:27PM +0000, Mark Brown wrote:
> On Wed, Jan 11, 2012 at 09:02:44PM +0800, Richard Zhao wrote:
> > On Wed, Jan 11, 2012 at 01:26:06PM +0800, Shawn Guo wrote:
> 
> > > With doing so, it may be also good for us to move this driver into
> > > sound/soc/imx/.  Any particular reason to keep it in platform?
> 
> > Yes, we can move it out. But I'm not sure we can put it in asoc,
> > because it's not a standard part of ASoC. Mark?
> 
> It should be supported using standard ASoC mechanisms.
Anyway, it needs another patch. I'll not do it in this patch series.

Thanks
Richard

Patch

diff --git a/arch/arm/plat-mxc/audmux-v2.c b/arch/arm/plat-mxc/audmux-v2.c
index 8cced35..87e05e2 100644
--- a/arch/arm/plat-mxc/audmux-v2.c
+++ b/arch/arm/plat-mxc/audmux-v2.c
@@ -20,7 +20,9 @@ 
 #include <linux/io.h>
 #include <linux/clk.h>
 #include <linux/debugfs.h>
+#include <linux/platform_device.h>
 #include <linux/slab.h>
+#include <linux/of.h>
 #include <mach/audmux.h>
 #include <mach/hardware.h>
 
@@ -184,8 +186,64 @@  int mxc_audmux_v2_configure_port(unsigned int port, unsigned int ptcr,
 }
 EXPORT_SYMBOL_GPL(mxc_audmux_v2_configure_port);
 
+#ifdef CONFIG_OF
+
+static int audmux_v2_probe(struct platform_device *pdev)
+{
+	struct resource *res;
+	int ret = 0;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res)
+		return -ENODEV;
+	if (!request_mem_region(res->start, resource_size(res), "audmux_v2")) {
+		dev_err(&pdev->dev, "request_mem_region failed\n");
+		return -EBUSY;
+	}
+
+	audmux_base = ioremap(res->start, resource_size(res));
+	if (!audmux_base) {
+		dev_err(&pdev->dev, "ioremap failed\n");
+		ret = -ENODEV;
+		goto failed_ioremap;
+	}
+
+	audmux_clk = clk_get(NULL, "audmux");
+	if (IS_ERR(audmux_clk)) {
+			dev_warn(&pdev->dev, "%s: cannot get clock: %d\n",
+				__func__, ret);
+			audmux_clk = NULL;
+	}
+
+	audmux_debugfs_init();
+	return 0;
+
+failed_ioremap:
+	release_mem_region(res->start, resource_size(res));
+
+	return ret;
+}
+
+static const struct of_device_id audmux_v2_dt_ids[] = {
+	{ .compatible = "fsl,audmux-v2", },
+	{ /* sentinel */ }
+};
+
+static struct platform_driver audmux_v2_driver = {
+	.probe = audmux_v2_probe,
+	.driver = {
+		.name = "audmux_v2",
+		.of_match_table = audmux_v2_dt_ids,
+	},
+};
+
+#endif
+
 static int mxc_audmux_v2_init(void)
 {
+#ifdef CONFIG_OF
+	return platform_driver_register(&audmux_v2_driver);
+#else
 	int ret;
 	if (cpu_is_mx51()) {
 		audmux_base = MX51_IO_ADDRESS(MX51_AUDMUX_BASE_ADDR);
@@ -214,6 +272,7 @@  static int mxc_audmux_v2_init(void)
 	audmux_debugfs_init();
 
 	return 0;
+#endif
 }
 
 postcore_initcall(mxc_audmux_v2_init);