| Message ID | 1328148728-32258-7-git-send-email-richard.zhao@linaro.org |
|---|---|
| State | New |
| Headers | show |
On Thu, Feb 02, 2012 at 10:12:05AM +0800, Richard Zhao wrote: > Signed-off-by: Richard Zhao <richard.zhao@linaro.org> > --- > arch/arm/mach-imx/mach-mx31ads.c | 10 ++++++++++ > arch/arm/plat-mxc/include/mach/mx31.h | 1 + > 2 files changed, 11 insertions(+), 0 deletions(-) > Hmm, let's see who are actually using mxc_audmux_v2_configure_port(). $ git grep -n mxc_audmux_v2_configure_port arch/arm/ arch/arm/mach-imx/eukrea_mbimxsd25-baseboard.c:246: mxc_audmux_v2_configure_port(0, arch/arm/mach-imx/eukrea_mbimxsd25-baseboard.c:254: mxc_audmux_v2_configure_port(4, arch/arm/mach-imx/eukrea_mbimxsd35-baseboard.c:257: mxc_audmux_v2_configure_port(0, arch/arm/mach-imx/eukrea_mbimxsd35-baseboard.c:265: mxc_audmux_v2_configure_port(3, arch/arm/mach-imx/mach-pcm043.c:365: mxc_audmux_v2_configure_port(3, arch/arm/mach-imx/mach-pcm043.c:371: mxc_audmux_v2_configure_port(0, $ git grep -n mxc_audmux_v2_configure_port sound/soc/imx/ sound/soc/imx/wm1133-ev1.c:277: mxc_audmux_v2_configure_port(MX31_AUDMUX_PORT1_SSI0, ptcr, pdcr); sound/soc/imx/wm1133-ev1.c:281: mxc_audmux_v2_configure_port(MX31_AUDMUX_PORT5_SSI_PINS_5, ptcr, pdcr); I guess audmux device needs to be added for all these users. And for sake of bisect, it should be added as part of patch #5.
On Thu, Feb 02, 2012 at 04:55:23PM +0800, Shawn Guo wrote: > On Thu, Feb 02, 2012 at 10:12:05AM +0800, Richard Zhao wrote: > > Signed-off-by: Richard Zhao <richard.zhao@linaro.org> > > --- > > arch/arm/mach-imx/mach-mx31ads.c | 10 ++++++++++ > > arch/arm/plat-mxc/include/mach/mx31.h | 1 + > > 2 files changed, 11 insertions(+), 0 deletions(-) > > > Hmm, let's see who are actually using mxc_audmux_v2_configure_port(). > > $ git grep -n mxc_audmux_v2_configure_port arch/arm/ > arch/arm/mach-imx/eukrea_mbimxsd25-baseboard.c:246: mxc_audmux_v2_configure_port(0, > arch/arm/mach-imx/eukrea_mbimxsd25-baseboard.c:254: mxc_audmux_v2_configure_port(4, > arch/arm/mach-imx/eukrea_mbimxsd35-baseboard.c:257: mxc_audmux_v2_configure_port(0, > arch/arm/mach-imx/eukrea_mbimxsd35-baseboard.c:265: mxc_audmux_v2_configure_port(3, > arch/arm/mach-imx/mach-pcm043.c:365: mxc_audmux_v2_configure_port(3, > arch/arm/mach-imx/mach-pcm043.c:371: mxc_audmux_v2_configure_port(0, > As we are moving audmux into sound/soc/imx, it makes less sense to still keep these calls in board files. Instead, I prefer to call it from machine driver like what wm1133-ev1 does below. Or we can simply make the it a audmux-self call with 3 parameters it needs retrieved from platform_data or device tree, so that machine driver does not even bother with the call. Makes sense? Regards, Shawn > $ git grep -n mxc_audmux_v2_configure_port sound/soc/imx/ > sound/soc/imx/wm1133-ev1.c:277: mxc_audmux_v2_configure_port(MX31_AUDMUX_PORT1_SSI0, ptcr, pdcr); > sound/soc/imx/wm1133-ev1.c:281: mxc_audmux_v2_configure_port(MX31_AUDMUX_PORT5_SSI_PINS_5, ptcr, pdcr); > > I guess audmux device needs to be added for all these users. And for > sake of bisect, it should be added as part of patch #5. >
On Thu, Feb 02, 2012 at 05:11:34PM +0800, Shawn Guo wrote: > On Thu, Feb 02, 2012 at 04:55:23PM +0800, Shawn Guo wrote: > > On Thu, Feb 02, 2012 at 10:12:05AM +0800, Richard Zhao wrote: > > > Signed-off-by: Richard Zhao <richard.zhao@linaro.org> > > > --- > > > arch/arm/mach-imx/mach-mx31ads.c | 10 ++++++++++ > > > arch/arm/plat-mxc/include/mach/mx31.h | 1 + > > > 2 files changed, 11 insertions(+), 0 deletions(-) > > > > > Hmm, let's see who are actually using mxc_audmux_v2_configure_port(). > > > > $ git grep -n mxc_audmux_v2_configure_port arch/arm/ > > arch/arm/mach-imx/eukrea_mbimxsd25-baseboard.c:246: mxc_audmux_v2_configure_port(0, > > arch/arm/mach-imx/eukrea_mbimxsd25-baseboard.c:254: mxc_audmux_v2_configure_port(4, > > arch/arm/mach-imx/eukrea_mbimxsd35-baseboard.c:257: mxc_audmux_v2_configure_port(0, > > arch/arm/mach-imx/eukrea_mbimxsd35-baseboard.c:265: mxc_audmux_v2_configure_port(3, > > arch/arm/mach-imx/mach-pcm043.c:365: mxc_audmux_v2_configure_port(3, > > arch/arm/mach-imx/mach-pcm043.c:371: mxc_audmux_v2_configure_port(0, I didn't grep arch/arm. where is pc043 asoc machine file? > > > As we are moving audmux into sound/soc/imx, it makes less sense to > still keep these calls in board files. It don't corrupt git bisect. We don't have to include it in this patch. > Instead, I prefer to call it > from machine driver like what wm1133-ev1 does below. Maybe pass the info as asoc machine driver pdata. > Or we can simply > make the it a audmux-self call with 3 parameters it needs retrieved > from platform_data or device tree, so that machine driver does not > even bother with the call. Makes sense? audux configuration may change after initial set. For example, it may use one configuration for audio playback, but use another when you connect a BT audio device. > > Regards, > Shawn > > > $ git grep -n mxc_audmux_v2_configure_port sound/soc/imx/ > > sound/soc/imx/wm1133-ev1.c:277: mxc_audmux_v2_configure_port(MX31_AUDMUX_PORT1_SSI0, ptcr, pdcr); > > sound/soc/imx/wm1133-ev1.c:281: mxc_audmux_v2_configure_port(MX31_AUDMUX_PORT5_SSI_PINS_5, ptcr, pdcr); It's imx31ads board. > > > > I guess audmux device needs to be added for all these users. And for > > sake of bisect, it should be added as part of patch #5. Yes. Thanks Richard > > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >
On Thu, Feb 02, 2012 at 10:12:05AM +0800, Richard Zhao wrote: > static void __init mxc_init_audio(void) > { > imx31_add_imx_ssi(0, NULL); > mxc_iomux_setup_multiple_pins(ssi_pins, ARRAY_SIZE(ssi_pins), "ssi"); > + imx_add_platform_device("audmux-v2", 0, > + audmux_res, ARRAY_SIZE(audmux_res), NULL, 0); Since the audmux is a part of the SoC silicon shouldn't the SoC just register the device without individual boards having to do anything (possibly conditional on ASoC being selected in Kconfig or something)? It's going to be connected in exactly the same fashion on any system using the SoC.
On Thu, Feb 02, 2012 at 05:24:28PM +0800, Richard Zhao wrote: > On Thu, Feb 02, 2012 at 05:11:34PM +0800, Shawn Guo wrote: > > On Thu, Feb 02, 2012 at 04:55:23PM +0800, Shawn Guo wrote: > > > On Thu, Feb 02, 2012 at 10:12:05AM +0800, Richard Zhao wrote: > > > > Signed-off-by: Richard Zhao <richard.zhao@linaro.org> > > > > --- > > > > arch/arm/mach-imx/mach-mx31ads.c | 10 ++++++++++ > > > > arch/arm/plat-mxc/include/mach/mx31.h | 1 + > > > > 2 files changed, 11 insertions(+), 0 deletions(-) > > > > > > > Hmm, let's see who are actually using mxc_audmux_v2_configure_port(). > > > > > > $ git grep -n mxc_audmux_v2_configure_port arch/arm/ > > > arch/arm/mach-imx/eukrea_mbimxsd25-baseboard.c:246: mxc_audmux_v2_configure_port(0, > > > arch/arm/mach-imx/eukrea_mbimxsd25-baseboard.c:254: mxc_audmux_v2_configure_port(4, > > > arch/arm/mach-imx/eukrea_mbimxsd35-baseboard.c:257: mxc_audmux_v2_configure_port(0, > > > arch/arm/mach-imx/eukrea_mbimxsd35-baseboard.c:265: mxc_audmux_v2_configure_port(3, > > > arch/arm/mach-imx/mach-pcm043.c:365: mxc_audmux_v2_configure_port(3, > > > arch/arm/mach-imx/mach-pcm043.c:371: mxc_audmux_v2_configure_port(0, > I didn't grep arch/arm. where is pc043 asoc machine file? It seems to be sound/soc/imx/phycore-ac97.c. > > > > > As we are moving audmux into sound/soc/imx, it makes less sense to > > still keep these calls in board files. > It don't corrupt git bisect. We don't have to include it in this patch. It's logically part of this series. > > Instead, I prefer to call it > > from machine driver like what wm1133-ev1 does below. > Maybe pass the info as asoc machine driver pdata. Sounds good. > > Or we can simply > > make the it a audmux-self call with 3 parameters it needs retrieved > > from platform_data or device tree, so that machine driver does not > > even bother with the call. Makes sense? > audux configuration may change after initial set. For example, it may use > one configuration for audio playback, but use another when you connect a > BT audio device. Right. I forgot this point.
On Thu, Feb 02, 2012 at 12:09:01PM +0000, Mark Brown wrote: > On Thu, Feb 02, 2012 at 10:12:05AM +0800, Richard Zhao wrote: > > > static void __init mxc_init_audio(void) > > { > > imx31_add_imx_ssi(0, NULL); > > mxc_iomux_setup_multiple_pins(ssi_pins, ARRAY_SIZE(ssi_pins), "ssi"); > > + imx_add_platform_device("audmux-v2", 0, > > + audmux_res, ARRAY_SIZE(audmux_res), NULL, 0); > > Since the audmux is a part of the SoC silicon shouldn't the SoC just > register the device without individual boards having to do anything > (possibly conditional on ASoC being selected in Kconfig or something)? > It's going to be connected in exactly the same fashion on any system > using the SoC. Hmm, we are trying to save adding the device for those boards which do not route any audmux pins out at all.
On Thu, Feb 02, 2012 at 09:17:18PM +0800, Shawn Guo wrote: > On Thu, Feb 02, 2012 at 12:09:01PM +0000, Mark Brown wrote: > > Since the audmux is a part of the SoC silicon shouldn't the SoC just > > register the device without individual boards having to do anything > > (possibly conditional on ASoC being selected in Kconfig or something)? > > It's going to be connected in exactly the same fashion on any system > > using the SoC. > Hmm, we are trying to save adding the device for those boards which do > not route any audmux pins out at all. That's why I'm saying perhaps make it conditional on having ASoC built (or even on having the AUDMUX driver built).
On Thu, Feb 02, 2012 at 09:09:03PM +0800, Shawn Guo wrote: > On Thu, Feb 02, 2012 at 05:24:28PM +0800, Richard Zhao wrote: > > On Thu, Feb 02, 2012 at 05:11:34PM +0800, Shawn Guo wrote: > > > On Thu, Feb 02, 2012 at 04:55:23PM +0800, Shawn Guo wrote: > > > > On Thu, Feb 02, 2012 at 10:12:05AM +0800, Richard Zhao wrote: > > > > > Signed-off-by: Richard Zhao <richard.zhao@linaro.org> > > > > > --- > > > > > arch/arm/mach-imx/mach-mx31ads.c | 10 ++++++++++ > > > > > arch/arm/plat-mxc/include/mach/mx31.h | 1 + > > > > > 2 files changed, 11 insertions(+), 0 deletions(-) > > > > > > > > > Hmm, let's see who are actually using mxc_audmux_v2_configure_port(). > > > > > > > > $ git grep -n mxc_audmux_v2_configure_port arch/arm/ > > > > arch/arm/mach-imx/eukrea_mbimxsd25-baseboard.c:246: mxc_audmux_v2_configure_port(0, > > > > arch/arm/mach-imx/eukrea_mbimxsd25-baseboard.c:254: mxc_audmux_v2_configure_port(4, > > > > arch/arm/mach-imx/eukrea_mbimxsd35-baseboard.c:257: mxc_audmux_v2_configure_port(0, > > > > arch/arm/mach-imx/eukrea_mbimxsd35-baseboard.c:265: mxc_audmux_v2_configure_port(3, > > > > arch/arm/mach-imx/mach-pcm043.c:365: mxc_audmux_v2_configure_port(3, > > > > arch/arm/mach-imx/mach-pcm043.c:371: mxc_audmux_v2_configure_port(0, > > I didn't grep arch/arm. where is pc043 asoc machine file? > > It seems to be sound/soc/imx/phycore-ac97.c. You see, we're not famaliar with the boards. > > > > > > > > As we are moving audmux into sound/soc/imx, it makes less sense to > > > still keep these calls in board files. > > It don't corrupt git bisect. We don't have to include it in this patch. > > It's logically part of this series. I don't know much about the above boards and I can not test either. I think I have to leave it to other volunteers. I mainly focus on audmux itself. > > > > Instead, I prefer to call it > > > from machine driver like what wm1133-ev1 does below. > > Maybe pass the info as asoc machine driver pdata. > > Sounds good. > > > > Or we can simply > > > make the it a audmux-self call with 3 parameters it needs retrieved > > > from platform_data or device tree, so that machine driver does not > > > even bother with the call. Makes sense? > > audux configuration may change after initial set. For example, it may use > > one configuration for audio playback, but use another when you connect a > > BT audio device. > > Right. I forgot this point. > > -- > Regards, > Shawn
On Thu, Feb 02, 2012 at 09:58:07PM +0800, Richard Zhao wrote: > On Thu, Feb 02, 2012 at 09:09:03PM +0800, Shawn Guo wrote: > > It's logically part of this series. > I don't know much about the above boards and I can not test either. I think I > have to leave it to other volunteers. I mainly focus on audmux itself. For stuff like this a build test and CCing the relevant maintainers is fine, you don't need to test everything.
On Thu, Feb 02, 2012 at 01:26:18PM +0000, Mark Brown wrote: > On Thu, Feb 02, 2012 at 09:17:18PM +0800, Shawn Guo wrote: > > On Thu, Feb 02, 2012 at 12:09:01PM +0000, Mark Brown wrote: > > > > Since the audmux is a part of the SoC silicon shouldn't the SoC just > > > register the device without individual boards having to do anything > > > (possibly conditional on ASoC being selected in Kconfig or something)? > > > It's going to be connected in exactly the same fashion on any system > > > using the SoC. > > > Hmm, we are trying to save adding the device for those boards which do > > not route any audmux pins out at all. > > That's why I'm saying perhaps make it conditional on having ASoC built > (or even on having the AUDMUX driver built). Do you mean by having the below in some place like function imx31_soc_init()? #ifdef CONFIG_SND_MXC_SOC_AUDMUXV2 imx_add_platform_device("audmux-v2", 0, audmux_res, ARRAY_SIZE(audmux_res), NULL, 0); #endif I do not think it's nice and consistent to the way that imx sub-architecture adds platform device. Furthermore, when a DT based board boots here, the code is broken. Explicitly adding the device by individual board as needed can easily align with DT based boards. By default, the audmux node in <soc>.dtsi file has status = "disabled", and any board that needs audmux device only need to overwrite status property of audmux node as 'okay' in its <board>.dts. Then DT core will add the audmux device when the board boots.
On Thu, Feb 02, 2012 at 10:11:26PM +0800, Shawn Guo wrote: > On Thu, Feb 02, 2012 at 01:26:18PM +0000, Mark Brown wrote: > > That's why I'm saying perhaps make it conditional on having ASoC built > > (or even on having the AUDMUX driver built). > Do you mean by having the below in some place like function > imx31_soc_init()? > #ifdef CONFIG_SND_MXC_SOC_AUDMUXV2 > imx_add_platform_device("audmux-v2", 0, > audmux_res, ARRAY_SIZE(audmux_res), NULL, 0); > #endif Yes (you need to check for module too, there's a macro for that the name of which escapes me right now). > I do not think it's nice and consistent to the way that imx > sub-architecture adds platform device. Well, the i.MX thus far has had relatively few of these always present type devices - it makes sense to make things conditional for devices with external signals but for things entirely within the SoC the above is less work. > Furthermore, when a DT based board boots here, the code is broken. > Explicitly adding the device by individual board as needed can easily > align with DT based boards. By default, the audmux node in <soc>.dtsi > file has status = "disabled", and any board that needs audmux device > only need to overwrite status property of audmux node as 'okay' in its > <board>.dts. Then DT core will add the audmux device when the board > boots. That seems like more work than is needed for boards, same issue applies.
On Thu, Feb 02, 2012 at 09:58:07PM +0800, Richard Zhao wrote: > On Thu, Feb 02, 2012 at 09:09:03PM +0800, Shawn Guo wrote: > > On Thu, Feb 02, 2012 at 05:24:28PM +0800, Richard Zhao wrote: > > > On Thu, Feb 02, 2012 at 05:11:34PM +0800, Shawn Guo wrote: > > > > On Thu, Feb 02, 2012 at 04:55:23PM +0800, Shawn Guo wrote: > > > > > On Thu, Feb 02, 2012 at 10:12:05AM +0800, Richard Zhao wrote: > > > > > > Signed-off-by: Richard Zhao <richard.zhao@linaro.org> > > > > > > --- > > > > > > arch/arm/mach-imx/mach-mx31ads.c | 10 ++++++++++ > > > > > > arch/arm/plat-mxc/include/mach/mx31.h | 1 + > > > > > > 2 files changed, 11 insertions(+), 0 deletions(-) > > > > > > > > > > > Hmm, let's see who are actually using mxc_audmux_v2_configure_port(). > > > > > > > > > > $ git grep -n mxc_audmux_v2_configure_port arch/arm/ > > > > > arch/arm/mach-imx/eukrea_mbimxsd25-baseboard.c:246: mxc_audmux_v2_configure_port(0, > > > > > arch/arm/mach-imx/eukrea_mbimxsd25-baseboard.c:254: mxc_audmux_v2_configure_port(4, > > > > > arch/arm/mach-imx/eukrea_mbimxsd35-baseboard.c:257: mxc_audmux_v2_configure_port(0, > > > > > arch/arm/mach-imx/eukrea_mbimxsd35-baseboard.c:265: mxc_audmux_v2_configure_port(3, > > > > > arch/arm/mach-imx/mach-pcm043.c:365: mxc_audmux_v2_configure_port(3, > > > > > arch/arm/mach-imx/mach-pcm043.c:371: mxc_audmux_v2_configure_port(0, > > > I didn't grep arch/arm. where is pc043 asoc machine file? > > > > It seems to be sound/soc/imx/phycore-ac97.c. > You see, we're not famaliar with the boards. It does not need to be familiar with the board to find that out. I'm not familiar with the board either, but I gave the answer. > > > > > > > > > > > As we are moving audmux into sound/soc/imx, it makes less sense to > > > > still keep these calls in board files. > > > It don't corrupt git bisect. We don't have to include it in this patch. > > > > It's logically part of this series. > I don't know much about the above boards and I can not test either. I think I > have to leave it to other volunteers. I mainly focus on audmux itself. Since you get there, you should be the one cleaning that up. You will need to touch those board files anyway, since you need to add audmux device for those boards. So no hardware for testing is not an excuse. For those boards, all you need to do are: * Change and compile-test the code * Cc board maintainers when submitting the patch We will wait for board maintainers to respond for a reasonable period of time. If we do not get any response during the time, we will send patch upstream anyway.
adding Eric Bénard. On Thu, Feb 02, 2012 at 09:58:07PM +0800, Richard Zhao wrote: > On Thu, Feb 02, 2012 at 09:09:03PM +0800, Shawn Guo wrote: > > On Thu, Feb 02, 2012 at 05:24:28PM +0800, Richard Zhao wrote: > > > On Thu, Feb 02, 2012 at 05:11:34PM +0800, Shawn Guo wrote: > > > > On Thu, Feb 02, 2012 at 04:55:23PM +0800, Shawn Guo wrote: > > > > > On Thu, Feb 02, 2012 at 10:12:05AM +0800, Richard Zhao wrote: > > > > > > Signed-off-by: Richard Zhao <richard.zhao@linaro.org> > > > > > > --- > > > > > > arch/arm/mach-imx/mach-mx31ads.c | 10 ++++++++++ > > > > > > arch/arm/plat-mxc/include/mach/mx31.h | 1 + > > > > > > 2 files changed, 11 insertions(+), 0 deletions(-) > > > > > > > > > > > Hmm, let's see who are actually using mxc_audmux_v2_configure_port(). > > > > > > > > > > $ git grep -n mxc_audmux_v2_configure_port arch/arm/ > > > > > arch/arm/mach-imx/eukrea_mbimxsd25-baseboard.c:246: mxc_audmux_v2_configure_port(0, > > > > > arch/arm/mach-imx/eukrea_mbimxsd25-baseboard.c:254: mxc_audmux_v2_configure_port(4, > > > > > arch/arm/mach-imx/eukrea_mbimxsd35-baseboard.c:257: mxc_audmux_v2_configure_port(0, > > > > > arch/arm/mach-imx/eukrea_mbimxsd35-baseboard.c:265: mxc_audmux_v2_configure_port(3, > > > > > arch/arm/mach-imx/mach-pcm043.c:365: mxc_audmux_v2_configure_port(3, > > > > > arch/arm/mach-imx/mach-pcm043.c:371: mxc_audmux_v2_configure_port(0, It's machine specific code, though audmux driver is common. Might be ok? The asoc machine file for the above 3 boards is not platform driver, and don't get any plat info. Sascha, Any suggestion? I saw pcm043 board was added by you. > > > I didn't grep arch/arm. where is pc043 asoc machine file? > > > > It seems to be sound/soc/imx/phycore-ac97.c. > You see, we're not famaliar with the boards. > > > > > > > > > > > As we are moving audmux into sound/soc/imx, it makes less sense to > > > > still keep these calls in board files. > > > It don't corrupt git bisect. We don't have to include it in this patch. > > > > It's logically part of this series. > I don't know much about the above boards and I can not test either. I think I > have to leave it to other volunteers. I mainly focus on audmux itself. > > > > > > Instead, I prefer to call it > > > > from machine driver like what wm1133-ev1 does below. > > > Maybe pass the info as asoc machine driver pdata. Sorry, they're not platform drivers. > > > > Sounds good. > > > > > > Or we can simply > > > > make the it a audmux-self call with 3 parameters it needs retrieved > > > > from platform_data or device tree, so that machine driver does not > > > > even bother with the call. Makes sense? > > > audux configuration may change after initial set. For example, it may use > > > one configuration for audio playback, but use another when you connect a > > > BT audio device. > > > > Right. I forgot this point. > > > > -- > > Regards, > > Shawn >
On Thu, Feb 02, 2012 at 09:09:03PM +0800, Shawn Guo wrote: > On Thu, Feb 02, 2012 at 05:24:28PM +0800, Richard Zhao wrote: > > On Thu, Feb 02, 2012 at 05:11:34PM +0800, Shawn Guo wrote: ... > > > Or we can simply > > > make the it a audmux-self call with 3 parameters it needs retrieved > > > from platform_data or device tree, so that machine driver does not > > > even bother with the call. Makes sense? > > audux configuration may change after initial set. For example, it may use > > one configuration for audio playback, but use another when you connect a > > BT audio device. > > Right. I forgot this point. > With a second thought on this, we can still do this as long as we have mxc_audmux_v2_configure_port() exported. For init-time setup, we can just do it in audmux driver probe function.
On Fri, Feb 03, 2012 at 10:15:54AM +0800, Richard Zhao wrote: > adding Eric Bénard. > > On Thu, Feb 02, 2012 at 09:58:07PM +0800, Richard Zhao wrote: > > On Thu, Feb 02, 2012 at 09:09:03PM +0800, Shawn Guo wrote: > > > On Thu, Feb 02, 2012 at 05:24:28PM +0800, Richard Zhao wrote: > > > > On Thu, Feb 02, 2012 at 05:11:34PM +0800, Shawn Guo wrote: > > > > > On Thu, Feb 02, 2012 at 04:55:23PM +0800, Shawn Guo wrote: > > > > > > On Thu, Feb 02, 2012 at 10:12:05AM +0800, Richard Zhao wrote: > > > > > > > Signed-off-by: Richard Zhao <richard.zhao@linaro.org> > > > > > > > --- > > > > > > > arch/arm/mach-imx/mach-mx31ads.c | 10 ++++++++++ > > > > > > > arch/arm/plat-mxc/include/mach/mx31.h | 1 + > > > > > > > 2 files changed, 11 insertions(+), 0 deletions(-) > > > > > > > > > > > > > Hmm, let's see who are actually using mxc_audmux_v2_configure_port(). > > > > > > > > > > > > $ git grep -n mxc_audmux_v2_configure_port arch/arm/ > > > > > > arch/arm/mach-imx/eukrea_mbimxsd25-baseboard.c:246: mxc_audmux_v2_configure_port(0, > > > > > > arch/arm/mach-imx/eukrea_mbimxsd25-baseboard.c:254: mxc_audmux_v2_configure_port(4, > > > > > > arch/arm/mach-imx/eukrea_mbimxsd35-baseboard.c:257: mxc_audmux_v2_configure_port(0, > > > > > > arch/arm/mach-imx/eukrea_mbimxsd35-baseboard.c:265: mxc_audmux_v2_configure_port(3, > > > > > > arch/arm/mach-imx/mach-pcm043.c:365: mxc_audmux_v2_configure_port(3, > > > > > > arch/arm/mach-imx/mach-pcm043.c:371: mxc_audmux_v2_configure_port(0, > It's machine specific code, though audmux driver is common. Might be ok? With audmux driver moved out, it's not ok to me to have board file call this function. > The asoc machine file for the above 3 boards is not platform driver, and > don't get any plat info. It can be nicely solved if we do these initial setup in audmux driver probe function with the configuration data retrieved from audmux platform data or device tree.
Shawn Guo <shawn.guo@linaro.org> wrote: >On Fri, Feb 03, 2012 at 10:15:54AM +0800, Richard Zhao wrote: >> adding Eric Bénard. >> >> On Thu, Feb 02, 2012 at 09:58:07PM +0800, Richard Zhao wrote: >> > On Thu, Feb 02, 2012 at 09:09:03PM +0800, Shawn Guo wrote: >> > > On Thu, Feb 02, 2012 at 05:24:28PM +0800, Richard Zhao wrote: >> > > > On Thu, Feb 02, 2012 at 05:11:34PM +0800, Shawn Guo wrote: >> > > > > On Thu, Feb 02, 2012 at 04:55:23PM +0800, Shawn Guo wrote: >> > > > > > On Thu, Feb 02, 2012 at 10:12:05AM +0800, Richard Zhao >wrote: >> > > > > > > Signed-off-by: Richard Zhao <richard.zhao@linaro.org> >> > > > > > > --- >> > > > > > > arch/arm/mach-imx/mach-mx31ads.c | 10 ++++++++++ >> > > > > > > arch/arm/plat-mxc/include/mach/mx31.h | 1 + >> > > > > > > 2 files changed, 11 insertions(+), 0 deletions(-) >> > > > > > > >> > > > > > Hmm, let's see who are actually using >mxc_audmux_v2_configure_port(). >> > > > > > >> > > > > > $ git grep -n mxc_audmux_v2_configure_port arch/arm/ >> > > > > > arch/arm/mach-imx/eukrea_mbimxsd25-baseboard.c:246: >mxc_audmux_v2_configure_port(0, >> > > > > > arch/arm/mach-imx/eukrea_mbimxsd25-baseboard.c:254: >mxc_audmux_v2_configure_port(4, >> > > > > > arch/arm/mach-imx/eukrea_mbimxsd35-baseboard.c:257: >mxc_audmux_v2_configure_port(0, >> > > > > > arch/arm/mach-imx/eukrea_mbimxsd35-baseboard.c:265: >mxc_audmux_v2_configure_port(3, >> > > > > > arch/arm/mach-imx/mach-pcm043.c:365: >mxc_audmux_v2_configure_port(3, >> > > > > > arch/arm/mach-imx/mach-pcm043.c:371: >mxc_audmux_v2_configure_port(0, >> It's machine specific code, though audmux driver is common. Might be >ok? > >With audmux driver moved out, it's not ok to me to have board file >call this function. > >> The asoc machine file for the above 3 boards is not platform driver, >and >> don't get any plat info. > >It can be nicely solved if we do these initial setup in audmux driver >probe function with the configuration data retrieved from audmux >platform data or device tree. no, it is asoc machine driver to have machine specific code. the machine driver do not correspond to any hw device, which cause hard to bind dt or create platform device. > >-- >Regards, >Shawn > >_______________________________________________ >linux-arm-kernel mailing list >linux-arm-kernel@lists.infradead.org >http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Sun, Feb 05, 2012 at 12:50:15PM +0800, Richard Zhao wrote: > > > Shawn Guo <shawn.guo@linaro.org> wrote: > > >On Fri, Feb 03, 2012 at 10:15:54AM +0800, Richard Zhao wrote: > >> adding Eric Bénard. > >> > >> On Thu, Feb 02, 2012 at 09:58:07PM +0800, Richard Zhao wrote: > >> > On Thu, Feb 02, 2012 at 09:09:03PM +0800, Shawn Guo wrote: > >> > > On Thu, Feb 02, 2012 at 05:24:28PM +0800, Richard Zhao wrote: > >> > > > On Thu, Feb 02, 2012 at 05:11:34PM +0800, Shawn Guo wrote: > >> > > > > On Thu, Feb 02, 2012 at 04:55:23PM +0800, Shawn Guo wrote: > >> > > > > > On Thu, Feb 02, 2012 at 10:12:05AM +0800, Richard Zhao > >wrote: > >> > > > > > > Signed-off-by: Richard Zhao <richard.zhao@linaro.org> > >> > > > > > > --- > >> > > > > > > arch/arm/mach-imx/mach-mx31ads.c | 10 ++++++++++ > >> > > > > > > arch/arm/plat-mxc/include/mach/mx31.h | 1 + > >> > > > > > > 2 files changed, 11 insertions(+), 0 deletions(-) > >> > > > > > > > >> > > > > > Hmm, let's see who are actually using > >mxc_audmux_v2_configure_port(). > >> > > > > > > >> > > > > > $ git grep -n mxc_audmux_v2_configure_port arch/arm/ > >> > > > > > arch/arm/mach-imx/eukrea_mbimxsd25-baseboard.c:246: > >mxc_audmux_v2_configure_port(0, > >> > > > > > arch/arm/mach-imx/eukrea_mbimxsd25-baseboard.c:254: > >mxc_audmux_v2_configure_port(4, > >> > > > > > arch/arm/mach-imx/eukrea_mbimxsd35-baseboard.c:257: > >mxc_audmux_v2_configure_port(0, > >> > > > > > arch/arm/mach-imx/eukrea_mbimxsd35-baseboard.c:265: > >mxc_audmux_v2_configure_port(3, > >> > > > > > arch/arm/mach-imx/mach-pcm043.c:365: > >mxc_audmux_v2_configure_port(3, > >> > > > > > arch/arm/mach-imx/mach-pcm043.c:371: > >mxc_audmux_v2_configure_port(0, > >> It's machine specific code, though audmux driver is common. Might be > >ok? > > > >With audmux driver moved out, it's not ok to me to have board file > >call this function. > > > >> The asoc machine file for the above 3 boards is not platform driver, > >and > >> don't get any plat info. > > > >It can be nicely solved if we do these initial setup in audmux driver > >probe function with the configuration data retrieved from audmux > >platform data or device tree. > no, it is asoc machine driver to have machine specific code. > the machine driver do not correspond to any hw device, which cause hard > to bind dt or create platform device. I'll have to keep audmux driver in arch/, till ASOC machine driver has a way to get platfrom parameters. Thanks Richard > > > >-- > >Regards, > >Shawn > > > >_______________________________________________ > >linux-arm-kernel mailing list > >linux-arm-kernel@lists.infradead.org > >http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > >
On Tue, Feb 14, 2012 at 09:35:07AM +0800, Richard Zhao wrote: > On Sun, Feb 05, 2012 at 12:50:15PM +0800, Richard Zhao wrote: > > no, it is asoc machine driver to have machine specific code. > > the machine driver do not correspond to any hw device, which cause hard > > to bind dt or create platform device. > I'll have to keep audmux driver in arch/, till ASOC machine driver > has a way to get platfrom parameters. Machine drivers can easily get platform data, they're just regular drivers of whatever type so can get platform data in the same way that any other driver for their bus can.
On Mon, Feb 13, 2012 at 10:06:28PM -0800, Mark Brown wrote: > On Tue, Feb 14, 2012 at 09:35:07AM +0800, Richard Zhao wrote: > > On Sun, Feb 05, 2012 at 12:50:15PM +0800, Richard Zhao wrote: > > > > no, it is asoc machine driver to have machine specific code. > > > the machine driver do not correspond to any hw device, which cause hard > > > to bind dt or create platform device. > > > I'll have to keep audmux driver in arch/, till ASOC machine driver > > has a way to get platfrom parameters. > > Machine drivers can easily get platform data, they're just regular > drivers of whatever type so can get platform data in the same way that > any other driver for their bus can. Machine drivers don't correspond to any hw devices. If we create a virtual platform device, it'll be hard for DT binding. Thanks Richard
On Tue, Feb 14, 2012 at 03:34:15PM +0800, Richard Zhao wrote: > On Mon, Feb 13, 2012 at 10:06:28PM -0800, Mark Brown wrote: > > Machine drivers can easily get platform data, they're just regular > > drivers of whatever type so can get platform data in the same way that > > any other driver for their bus can. > Machine drivers don't correspond to any hw devices. If we create a > virtual platform device, it'll be hard for DT binding. As has been discussed repeatedly and at some considerable length the board design for audio is considered sufficiently interesting to be worth representing in the device tree directly, there are plenty of choices made during board design. There is absolutely no technical problem from doing this on the device tree side. People working on device tree really need to talk to each other more...
diff --git a/arch/arm/mach-imx/mach-mx31ads.c b/arch/arm/mach-imx/mach-mx31ads.c index 4917aab..bb69e71 100644 --- a/arch/arm/mach-imx/mach-mx31ads.c +++ b/arch/arm/mach-imx/mach-mx31ads.c @@ -486,10 +486,20 @@ static unsigned int ssi_pins[] = { MX31_PIN_STXD5__STXD5, }; +static const struct resource audmux_res[] __initconst = { + { + .start = MX31_AUDMUX_BASE_ADDR, + .end = MX31_AUDMUX_SIZE, + .flags = IORESOURCE_MEM, + }, +}; + static void __init mxc_init_audio(void) { imx31_add_imx_ssi(0, NULL); mxc_iomux_setup_multiple_pins(ssi_pins, ARRAY_SIZE(ssi_pins), "ssi"); + imx_add_platform_device("audmux-v2", 0, + audmux_res, ARRAY_SIZE(audmux_res), NULL, 0); } /* static mappings */ diff --git a/arch/arm/plat-mxc/include/mach/mx31.h b/arch/arm/plat-mxc/include/mach/mx31.h index e27619e..8a3d5ef 100644 --- a/arch/arm/plat-mxc/include/mach/mx31.h +++ b/arch/arm/plat-mxc/include/mach/mx31.h @@ -66,6 +66,7 @@ #define MX31_RNGA_BASE_ADDR (MX31_AIPS2_BASE_ADDR + 0xb0000) #define MX31_IPU_CTRL_BASE_ADDR (MX31_AIPS2_BASE_ADDR + 0xc0000) #define MX31_AUDMUX_BASE_ADDR (MX31_AIPS2_BASE_ADDR + 0xc4000) +#define MX31_AUDMUX_SIZE (SZ_16K) #define MX31_MPEG4_ENC_BASE_ADDR (MX31_AIPS2_BASE_ADDR + 0xc8000) #define MX31_GPIO1_BASE_ADDR (MX31_AIPS2_BASE_ADDR + 0xcc000) #define MX31_GPIO2_BASE_ADDR (MX31_AIPS2_BASE_ADDR + 0xd0000)
Signed-off-by: Richard Zhao <richard.zhao@linaro.org> --- arch/arm/mach-imx/mach-mx31ads.c | 10 ++++++++++ arch/arm/plat-mxc/include/mach/mx31.h | 1 + 2 files changed, 11 insertions(+), 0 deletions(-)