On Wed, Jul 22, 2009 at 05:22:59AM +0200, Janusz Krzysztofik wrote:
> Signed-off-by: Janusz Krzysztofik <jkrzyszt@xxxxxxxxxxxx>
> ---
> CPU DAI parameters best matching the codec DAI has been selected out
> empirically for best user experience.
Again, all the documentation you've got here could quite happily go in
the commit message and there's a bunch of checkpatch issues.
> +#include <sound/soc-dapm.h>
> +#include <sound/jack.h>
ASoC will pull that one in for you, not that it really matters.
> + /* Setup pins after corresponding bits if changed */
> + if ((bool)snd_soc_dapm_get_pin_status(codec, "Speaker") !=
> + (bool)(function & (1 << AMS_DELTA_SPEAKER))) {
Don't like these casts... why are they needed?
> +static const struct snd_kcontrol_new ams_delta_audio_controls[] = {
> + SOC_ENUM_EXT("Audio Function", ams_delta_audio_enum[0],
> + ams_delta_get_audio_mode, ams_delta_set_audio_mode),
> +};
Is it possible to control all the functions of the audio mode
independantly or are only certain combinations possible?
> +static struct snd_soc_jack_gpio ams_delta_hook_switch_gpios[];
> +static struct snd_soc_jack_pin ams_delta_hook_switch_pins[] = {
> + {
> + .pin = "Mouthpiece",
> + .mask = SND_JACK_MICROPHONE,
> + },
> + {
> + .pin = "Earphone",
> + .mask = SND_JACK_HEADPHONE,
> + },
> + {
> + .pin = "Speaker",
> + .mask = SND_JACK_HEADPHONE,
> + .invert = 1,
> + },
> + {
> + .pin = "Microphone",
> + .mask = SND_JACK_MICROPHONE,
> + .invert = 1,
> + },
> +};
I guess microphone and speaker are for speakerphone mode while
mouthpiece and earpiece are a headset? Might be nice to come up with
names that make the paring a bit clearer, or possibly just put a comment
in there.
> +/* To actually apply any modem controlled configuration changes to the codec,
> + * we must connect codec DAI pins to the modem for a moment. Be carefull
> + * not to interfere with digital mute function that shares the same hardware. */
> +static struct timer_list cx81801_timer;
> +static bool cx81801_cmd_pending = 0;
> +static bool ams_delta_muted;
> +
> +static void cx81801_timeout(unsigned long data)
> +{
> + /* REVISIT - locking? */
Yeah, locking is probably a good idea :)
> + /* Set DAPM pins after hook switch present state */
> +#if 0
> + /* Fails for switch state matching initial gpio->state = 0 */
> + snd_soc_jack_report(&ams_delta_ams_delta_hook_switch,
> + gpio_get_value(ams_delta_hook_switch_gpios[0].gpio) ?
> + 0 : SND_JACK_HEADSET, SND_JACK_HEADSET);
Hrm. We should just fix that so that adding a pin forces a sync rather
than just doing a report. The GPIO jack wrapper ought to just do
everything you need without any code in the machine driver.
> +
> + /* REVISIT - Don't know how to do that */
> + /*
> + * Remove controls that we expose when over the modem control available:
> + * - virtual audio mode switch,
> + * - hook switch to DAPM pins links
> + */
You can't do that. Rather than adding and removing the controls
dynamically I'd suggest hiding the controls from applications while
they're inactive - this will also avoid any renumbering which might
confuse them. However, that API isn't in mainline yet.
Takashi, there is a patch adding a snd_ctl_activate_id() call in your
unstable tree which hasn't made it into mainline and should help here -
would there be any problem merging it? I've got another use case for it
in ASoC that I'm hoping to find time to look at soon.
> + /* Add board specific DAPM controls */
> + if (!snd_soc_dapm_new_controls(codec, ams_delta_dapm_widgets,
> + ARRAY_SIZE(ams_delta_dapm_widgets))) {
> + if (!snd_soc_dapm_add_routes(codec, ams_delta_audio_map,
> + ARRAY_SIZE(ams_delta_audio_map))) {
The error handling here is a bit odd...
> + /* Add hook switch */
> + if (!snd_soc_jack_new(&ams_delta_audio_card, "hook_switch",
> + SND_JACK_HEADSET, &ams_delta_hook_switch)) {
> + if (!snd_soc_jack_add_gpios(&ams_delta_hook_switch,
> + ARRAY_SIZE(ams_delta_hook_switch_gpios),
> + ams_delta_hook_switch_gpios)) {
> +#ifdef CONFIG_GPIO_SYSFS
> + /* Expose hook switch over sysfs if configured */
> + gpio_export(ams_delta_hook_switch_gpios[0].gpio, false);
> +#endif
The gpio_export() should be in the ASoC GPIO code rather than here, I'd
expect - care to cook up a patch?
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
