* [PATCH] ASOC: imx: audmux: add a integrated configuration function
@ 2013-01-31 6:42 Gary Zhang
2013-01-31 14:22 ` Shawn Guo
0 siblings, 1 reply; 6+ messages in thread
From: Gary Zhang @ 2013-01-31 6:42 UTC (permalink / raw)
To: broonie, lgirdwood, shawn.guo; +Cc: alsa-devel
based on imx_audmux_v2_configure_port(), add a integrated
interface to configure audmux port conveniently
Signed-off-by: Gary Zhang <b13634@freescale.com>
---
sound/soc/fsl/imx-audmux.c | 43 ++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 42 insertions(+), 1 deletions(-)
diff --git a/sound/soc/fsl/imx-audmux.c b/sound/soc/fsl/imx-audmux.c
index 251f4d9..ebd38a0 100644
--- a/sound/soc/fsl/imx-audmux.c
+++ b/sound/soc/fsl/imx-audmux.c
@@ -1,5 +1,5 @@
/*
- * Copyright 2012 Freescale Semiconductor, Inc.
+ * Copyright 2012-2013 Freescale Semiconductor, Inc.
* Copyright 2012 Linaro Ltd.
* Copyright 2009 Pengutronix, Sascha Hauer <s.hauer@pengutronix.de>
*
@@ -244,6 +244,47 @@ int imx_audmux_v2_configure_port(unsigned int port, unsigned int ptcr,
}
EXPORT_SYMBOL_GPL(imx_audmux_v2_configure_port);
+int imx_audmux_configure_port(unsigned int int_port, unsigned int ext_port)
+{
+ int ret;
+
+ if ((int_port > 7) || (ext_port > 7))
+ return -EINVAL;
+
+ if (audmux_type != IMX31_AUDMUX)
+ return -EINVAL;
+
+ if (!audmux_base)
+ return -ENOSYS;
+
+ if (audmux_clk)
+ clk_prepare_enable(audmux_clk);
+
+ int_port--;
+ ext_port--;
+ ret = imx_audmux_v2_configure_port(int_port,
+ IMX_AUDMUX_V2_PTCR_SYN |
+ IMX_AUDMUX_V2_PTCR_TFSEL(ext_port) |
+ IMX_AUDMUX_V2_PTCR_TCSEL(ext_port) |
+ IMX_AUDMUX_V2_PTCR_TFSDIR |
+ IMX_AUDMUX_V2_PTCR_TCLKDIR,
+ IMX_AUDMUX_V2_PDCR_RXDSEL(ext_port));
+ if (ret) {
+ pr_err("audmux internal port setup failed\n");
+ return ret;
+ }
+ imx_audmux_v2_configure_port(ext_port,
+ IMX_AUDMUX_V2_PTCR_SYN,
+ IMX_AUDMUX_V2_PDCR_RXDSEL(int_port));
+ if (ret) {
+ pr_err("audmux external port setup failed\n");
+ return ret;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(imx_audmux_configure_port);
+
static int imx_audmux_probe(struct platform_device *pdev)
{
struct resource *res;
--
1.7.0.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] ASOC: imx: audmux: add a integrated configuration function
2013-01-31 6:42 [PATCH] ASOC: imx: audmux: add a integrated configuration function Gary Zhang
@ 2013-01-31 14:22 ` Shawn Guo
2013-01-31 16:35 ` Mark Brown
0 siblings, 1 reply; 6+ messages in thread
From: Shawn Guo @ 2013-01-31 14:22 UTC (permalink / raw)
To: Gary Zhang; +Cc: alsa-devel, broonie, lgirdwood
On Thu, Jan 31, 2013 at 02:42:46PM +0800, Gary Zhang wrote:
> based on imx_audmux_v2_configure_port(), add a integrated
> interface to configure audmux port conveniently
>
> Signed-off-by: Gary Zhang <b13634@freescale.com>
> ---
> sound/soc/fsl/imx-audmux.c | 43 ++++++++++++++++++++++++++++++++++++++++++-
> 1 files changed, 42 insertions(+), 1 deletions(-)
>
> diff --git a/sound/soc/fsl/imx-audmux.c b/sound/soc/fsl/imx-audmux.c
> index 251f4d9..ebd38a0 100644
> --- a/sound/soc/fsl/imx-audmux.c
> +++ b/sound/soc/fsl/imx-audmux.c
> @@ -1,5 +1,5 @@
> /*
> - * Copyright 2012 Freescale Semiconductor, Inc.
> + * Copyright 2012-2013 Freescale Semiconductor, Inc.
> * Copyright 2012 Linaro Ltd.
> * Copyright 2009 Pengutronix, Sascha Hauer <s.hauer@pengutronix.de>
> *
> @@ -244,6 +244,47 @@ int imx_audmux_v2_configure_port(unsigned int port, unsigned int ptcr,
> }
> EXPORT_SYMBOL_GPL(imx_audmux_v2_configure_port);
>
> +int imx_audmux_configure_port(unsigned int int_port, unsigned int ext_port)
We have imx_audmux_v1_configure_port and imx_audmux_v2_configure_port,
but you name a imx_audmux_v2_configure_port only helper as
imx_audmux_configure_port which is not good.
> +{
> + int ret;
> +
> + if ((int_port > 7) || (ext_port > 7))
> + return -EINVAL;
This check is not really scalable, as we may have different ports
on different SoCs.
> +
> + if (audmux_type != IMX31_AUDMUX)
> + return -EINVAL;
> +
> + if (!audmux_base)
> + return -ENOSYS;
These checks will be done imx_audmux_v2_configure_port() again then.
So all in all, this consolidation does not really fit well.
Mark,
I just found there are much more code between imx_sgtl5000_probe
and imx_wm8962_probe than just above audmux setup. I'm thinking
about consolidate them at a higher level. But can you give it a
go on Gary's patch, and then I can start the consolidation from
there? I promise I will send a follow-up patch to do it.
Shawn
> +
> + if (audmux_clk)
> + clk_prepare_enable(audmux_clk);
> +
> + int_port--;
> + ext_port--;
> + ret = imx_audmux_v2_configure_port(int_port,
> + IMX_AUDMUX_V2_PTCR_SYN |
> + IMX_AUDMUX_V2_PTCR_TFSEL(ext_port) |
> + IMX_AUDMUX_V2_PTCR_TCSEL(ext_port) |
> + IMX_AUDMUX_V2_PTCR_TFSDIR |
> + IMX_AUDMUX_V2_PTCR_TCLKDIR,
> + IMX_AUDMUX_V2_PDCR_RXDSEL(ext_port));
> + if (ret) {
> + pr_err("audmux internal port setup failed\n");
> + return ret;
> + }
> + imx_audmux_v2_configure_port(ext_port,
> + IMX_AUDMUX_V2_PTCR_SYN,
> + IMX_AUDMUX_V2_PDCR_RXDSEL(int_port));
> + if (ret) {
> + pr_err("audmux external port setup failed\n");
> + return ret;
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(imx_audmux_configure_port);
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] ASOC: imx: audmux: add a integrated configuration function
2013-01-31 14:22 ` Shawn Guo
@ 2013-01-31 16:35 ` Mark Brown
2013-02-01 13:10 ` Shawn Guo
0 siblings, 1 reply; 6+ messages in thread
From: Mark Brown @ 2013-01-31 16:35 UTC (permalink / raw)
To: Shawn Guo; +Cc: alsa-devel, Gary Zhang, lgirdwood
[-- Attachment #1.1: Type: text/plain, Size: 481 bytes --]
On Thu, Jan 31, 2013 at 10:22:45PM +0800, Shawn Guo wrote:
> Mark,
> I just found there are much more code between imx_sgtl5000_probe
> and imx_wm8962_probe than just above audmux setup. I'm thinking
> about consolidate them at a higher level. But can you give it a
> go on Gary's patch, and then I can start the consolidation from
> there? I promise I will send a follow-up patch to do it.
I'm not sure what you mean here? I don't have any intention to work on
this stuff.
[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ASOC: imx: audmux: add a integrated configuration function
2013-01-31 16:35 ` Mark Brown
@ 2013-02-01 13:10 ` Shawn Guo
2013-02-04 10:01 ` Mark Brown
0 siblings, 1 reply; 6+ messages in thread
From: Shawn Guo @ 2013-02-01 13:10 UTC (permalink / raw)
To: Mark Brown; +Cc: alsa-devel, Gary Zhang, lgirdwood
On Fri, Feb 01, 2013 at 12:35:19AM +0800, Mark Brown wrote:
> On Thu, Jan 31, 2013 at 10:22:45PM +0800, Shawn Guo wrote:
>
> > Mark,
>
> > I just found there are much more code between imx_sgtl5000_probe
> > and imx_wm8962_probe than just above audmux setup. I'm thinking
> > about consolidate them at a higher level. But can you give it a
> > go on Gary's patch, and then I can start the consolidation from
> > there? I promise I will send a follow-up patch to do it.
>
> I'm not sure what you mean here? I don't have any intention to work on
> this stuff.
You had a comment on Gary's imx-wm8962 patch, saying that the audmux
setup should become a helper. And that's why Gary came up this patch.
But the patch does not look good. While reviewing the patch, I found
there are much more than just audmux setup code could be shared between
imx-sgtl5000 and imx-wm8960 driver. I'm asking if you can merge Gary's
the patch with leaving audmux setup as it is, and let me consolidate
the common part between imx-sgtl5000 and imx-wm8960 with a follow-up
patch.
In any case, I'm not expecting you work on this stuff :)
Shawn
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ASOC: imx: audmux: add a integrated configuration function
2013-02-01 13:10 ` Shawn Guo
@ 2013-02-04 10:01 ` Mark Brown
2013-02-04 13:39 ` Shawn Guo
0 siblings, 1 reply; 6+ messages in thread
From: Mark Brown @ 2013-02-04 10:01 UTC (permalink / raw)
To: Shawn Guo; +Cc: alsa-devel, Gary Zhang, lgirdwood
[-- Attachment #1.1: Type: text/plain, Size: 380 bytes --]
On Fri, Feb 01, 2013 at 09:10:09PM +0800, Shawn Guo wrote:
> imx-sgtl5000 and imx-wm8960 driver. I'm asking if you can merge Gary's
> the patch with leaving audmux setup as it is, and let me consolidate
> the common part between imx-sgtl5000 and imx-wm8960 with a follow-up
> patch.
Well, there were a bunch of other issues with the driver that I was
looking to see addressed.
[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ASOC: imx: audmux: add a integrated configuration function
2013-02-04 10:01 ` Mark Brown
@ 2013-02-04 13:39 ` Shawn Guo
0 siblings, 0 replies; 6+ messages in thread
From: Shawn Guo @ 2013-02-04 13:39 UTC (permalink / raw)
To: Mark Brown; +Cc: alsa-devel, Gary Zhang, lgirdwood
On Mon, Feb 04, 2013 at 10:01:49AM +0000, Mark Brown wrote:
> On Fri, Feb 01, 2013 at 09:10:09PM +0800, Shawn Guo wrote:
>
> > imx-sgtl5000 and imx-wm8960 driver. I'm asking if you can merge Gary's
> > the patch with leaving audmux setup as it is, and let me consolidate
> > the common part between imx-sgtl5000 and imx-wm8960 with a follow-up
> > patch.
>
> Well, there were a bunch of other issues with the driver that I was
> looking to see addressed.
Sure. I did not mean for any other issues, and they should be
addressed properly before getting merged.
Shawn
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-02-04 13:39 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-01-31 6:42 [PATCH] ASOC: imx: audmux: add a integrated configuration function Gary Zhang
2013-01-31 14:22 ` Shawn Guo
2013-01-31 16:35 ` Mark Brown
2013-02-01 13:10 ` Shawn Guo
2013-02-04 10:01 ` Mark Brown
2013-02-04 13:39 ` Shawn Guo
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.