| Message ID | 20211228190931.df5d518220080a734532ebfd@uvos.xyz |
|---|---|
| State | New |
| Headers | show |
| Series | [RFC,1/3] ASoC: simple-card-utils: add support for componants provideing jack events via set_jack | expand |
Hi Carl > Yes, so asoc-audio-graph-card currently only supports headphone jack plug > detection on devices that provide a simple gpio to sense plug state. The > intent of this patch is to allow the componant driver to implement jack > detection in cases where something else has to be done to sense state, > sutch as comunicating with device firmware or using a shared irq. See > the other patches in this series for an example. This is performed by > sharing the jack with the component via set_jack(). OK, I see. It seems other drivers are using dai_link->init for detecting and set_jack(). I guess we can call set_jack() at asoc_simple_init_jack() and call asoc_simple_init_hp() at dai_link->init ? It is not tested, but I added sample code below. I hope my understanding was correct and it solves your issue. > sutch as comunicating with device firmware or using a shared irq. See > the other patches in this series for an example. This is performed by I now noticed that I had mailing list issue... Thank you for your help !! ----------- From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> Subject: [PATCH] ASoC: simple_card_utils: call snd_soc_component_set_jack() at asoc_simple_init_jack() Current simple-card / audio-graph-card are detecting HP/MIC at card->probe timing (= A), and not calling snd_soc_component_set_jack() for it. Other sound card drivers are using dai_link->init timing (= B) for both detecting and set_jack(). static int snd_soc_bind_card(...) { .... (A) ret = snd_soc_card_probe(card); ... for_each_card_rtds(card, rtd) { (B) ret = soc_init_pcm_runtime(card, rtd); ... } ... } This patch do (a) call set_jack() (= Y) at asoc_simple_init_jack() (= X) which is used to detect HP/MIC. (b) call it from dai_link->init timing (= B) instead of card->probe timing (= A). (c) remove card->init (= A) timing function from simple-card / audio-graph-card. (X) int asoc_simple_init_jack(...) { ... if (gpio_is_valid(det)) { ... snd_soc_card_jack_new(...); snd_soc_jack_add_gpios(...); for_each_card_components(card, component) (Y) snd_soc_component_set_jack(component, ...); } ... } One note here is that simple-card needs PREFIX to detecting HP/MIC, but it is not needed on audio-graph-card. Thus simple-card uses local function for it, and audio-graph-card is using global function and sharing the code with audio-graph-card / audio-graph-card2 / audio-graph-card2-custom-sample / tegra_audio_graph_card. Reported-by: Carl Philipp Klemm <philipp@uvos.xyz> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> --- include/sound/simple_card_utils.h | 2 +- sound/soc/generic/audio-graph-card.c | 3 +- .../generic/audio-graph-card2-custom-sample.c | 3 +- sound/soc/generic/audio-graph-card2.c | 3 +- sound/soc/generic/simple-card-utils.c | 14 ++++++- sound/soc/generic/simple-card.c | 40 ++++++++++--------- sound/soc/tegra/tegra_audio_graph_card.c | 2 +- 7 files changed, 39 insertions(+), 28 deletions(-) diff --git a/include/sound/simple_card_utils.h b/include/sound/simple_card_utils.h index df430f1c2a10..34891da5a0fa 100644 --- a/include/sound/simple_card_utils.h +++ b/include/sound/simple_card_utils.h @@ -182,7 +182,7 @@ int asoc_simple_init_priv(struct asoc_simple_priv *priv, struct link_info *li); int asoc_simple_remove(struct platform_device *pdev); -int asoc_graph_card_probe(struct snd_soc_card *card); +int asoc_graph_dai_init(struct snd_soc_pcm_runtime *rtd); int asoc_graph_is_ports0(struct device_node *port); #ifdef DEBUG diff --git a/sound/soc/generic/audio-graph-card.c b/sound/soc/generic/audio-graph-card.c index 2b598af8feef..456db1f894af 100644 --- a/sound/soc/generic/audio-graph-card.c +++ b/sound/soc/generic/audio-graph-card.c @@ -259,7 +259,7 @@ static int graph_link_init(struct asoc_simple_priv *priv, if (ret < 0) return ret; - dai_link->init = asoc_simple_dai_init; + dai_link->init = asoc_graph_dai_init; dai_link->ops = &graph_ops; if (priv->ops) dai_link->ops = priv->ops; @@ -716,7 +716,6 @@ static int graph_probe(struct platform_device *pdev) card = simple_priv_to_card(priv); card->dapm_widgets = graph_dapm_widgets; card->num_dapm_widgets = ARRAY_SIZE(graph_dapm_widgets); - card->probe = asoc_graph_card_probe; if (of_device_get_match_data(dev)) priv->dpcm_selectable = 1; diff --git a/sound/soc/generic/audio-graph-card2-custom-sample.c b/sound/soc/generic/audio-graph-card2-custom-sample.c index 4a2c743e286c..da6cb69faa8d 100644 --- a/sound/soc/generic/audio-graph-card2-custom-sample.c +++ b/sound/soc/generic/audio-graph-card2-custom-sample.c @@ -34,8 +34,7 @@ static int custom_card_probe(struct snd_soc_card *card) custom_priv->custom_params = 1; - /* you can use generic probe function */ - return asoc_graph_card_probe(card); + return 0; } static int custom_hook_pre(struct asoc_simple_priv *priv) diff --git a/sound/soc/generic/audio-graph-card2.c b/sound/soc/generic/audio-graph-card2.c index c3947347dda3..75a1aeb75d2c 100644 --- a/sound/soc/generic/audio-graph-card2.c +++ b/sound/soc/generic/audio-graph-card2.c @@ -710,7 +710,7 @@ static void graph_link_init(struct asoc_simple_priv *priv, daiclk = snd_soc_daifmt_clock_provider_fliped(daiclk); dai_link->dai_fmt = daifmt | daiclk; - dai_link->init = asoc_simple_dai_init; + dai_link->init = asoc_graph_dai_init; dai_link->ops = &graph_ops; if (priv->ops) dai_link->ops = priv->ops; @@ -1180,7 +1180,6 @@ int audio_graph2_parse_of(struct asoc_simple_priv *priv, struct device *dev, if (!li) return -ENOMEM; - card->probe = asoc_graph_card_probe; card->owner = THIS_MODULE; card->dev = dev; diff --git a/sound/soc/generic/simple-card-utils.c b/sound/soc/generic/simple-card-utils.c index 850e968677f1..398fc4cb1d07 100644 --- a/sound/soc/generic/simple-card-utils.c +++ b/sound/soc/generic/simple-card-utils.c @@ -588,6 +588,8 @@ int asoc_simple_init_jack(struct snd_soc_card *card, return -EPROBE_DEFER; if (gpio_is_valid(det)) { + struct snd_soc_component *component; + sjack->pin.pin = pin_name; sjack->pin.mask = mask; @@ -603,6 +605,9 @@ int asoc_simple_init_jack(struct snd_soc_card *card, snd_soc_jack_add_gpios(&sjack->jack, 1, &sjack->gpio); + + for_each_card_components(card, component) + snd_soc_component_set_jack(component, &sjack->jack, NULL); } return 0; @@ -758,11 +763,16 @@ int asoc_simple_remove(struct platform_device *pdev) } EXPORT_SYMBOL_GPL(asoc_simple_remove); -int asoc_graph_card_probe(struct snd_soc_card *card) +int asoc_graph_dai_init(struct snd_soc_pcm_runtime *rtd) { + struct snd_soc_card *card = rtd->card; struct asoc_simple_priv *priv = snd_soc_card_get_drvdata(card); int ret; + ret = asoc_simple_dai_init(rtd); + if (ret < 0) + return ret; + ret = asoc_simple_init_hp(card, &priv->hp_jack, NULL); if (ret < 0) return ret; @@ -773,7 +783,7 @@ int asoc_graph_card_probe(struct snd_soc_card *card) return 0; } -EXPORT_SYMBOL_GPL(asoc_graph_card_probe); +EXPORT_SYMBOL_GPL(asoc_graph_dai_init); int asoc_graph_is_ports0(struct device_node *np) { diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c index a89d1cfdda32..e76bfae6ced4 100644 --- a/sound/soc/generic/simple-card.c +++ b/sound/soc/generic/simple-card.c @@ -149,6 +149,27 @@ static int simple_parse_node(struct asoc_simple_priv *priv, return 0; } +static int simple_dai_init(struct snd_soc_pcm_runtime *rtd) +{ + struct snd_soc_card *card = rtd->card; + struct asoc_simple_priv *priv = snd_soc_card_get_drvdata(card); + int ret; + + ret = asoc_simple_dai_init(rtd); + if (ret < 0) + return ret; + + ret = asoc_simple_init_hp(card, &priv->hp_jack, PREFIX); + if (ret < 0) + return ret; + + ret = asoc_simple_init_mic(card, &priv->mic_jack, PREFIX); + if (ret < 0) + return ret; + + return 0; +} + static int simple_link_init(struct asoc_simple_priv *priv, struct device_node *node, struct device_node *codec, @@ -164,7 +185,7 @@ static int simple_link_init(struct asoc_simple_priv *priv, if (ret < 0) return 0; - dai_link->init = asoc_simple_dai_init; + dai_link->init = simple_dai_init; dai_link->ops = &simple_ops; return asoc_simple_set_dailink_name(dev, dai_link, name); @@ -587,22 +608,6 @@ static int simple_get_dais_count(struct asoc_simple_priv *priv, simple_count_dpcm); } -static int simple_soc_probe(struct snd_soc_card *card) -{ - struct asoc_simple_priv *priv = snd_soc_card_get_drvdata(card); - int ret; - - ret = asoc_simple_init_hp(card, &priv->hp_jack, PREFIX); - if (ret < 0) - return ret; - - ret = asoc_simple_init_mic(card, &priv->mic_jack, PREFIX); - if (ret < 0) - return ret; - - return 0; -} - static int asoc_simple_probe(struct platform_device *pdev) { struct asoc_simple_priv *priv; @@ -620,7 +625,6 @@ static int asoc_simple_probe(struct platform_device *pdev) card = simple_priv_to_card(priv); card->owner = THIS_MODULE; card->dev = dev; - card->probe = simple_soc_probe; card->driver_name = "simple-card"; li = devm_kzalloc(dev, sizeof(*li), GFP_KERNEL); diff --git a/sound/soc/tegra/tegra_audio_graph_card.c b/sound/soc/tegra/tegra_audio_graph_card.c index 1f2c5018bf5a..404762d40389 100644 --- a/sound/soc/tegra/tegra_audio_graph_card.c +++ b/sound/soc/tegra/tegra_audio_graph_card.c @@ -184,7 +184,7 @@ static int tegra_audio_graph_card_probe(struct snd_soc_card *card) return PTR_ERR(priv->clk_plla_out0); } - return asoc_graph_card_probe(card); + return 0; } static int tegra_audio_graph_probe(struct platform_device *pdev)
Hi, > (X) int asoc_simple_init_jack(...) > { > ... > if (gpio_is_valid(det)) { > ... > > snd_soc_card_jack_new(...); > snd_soc_jack_add_gpios(...); > for_each_card_components(card, component) > (Y) snd_soc_component_set_jack(component, ...); > } > ... > } So for the case of cpcap codec on motorola mapphones this dosent help, because we dont have a gpio to sense plug state, thus no gpio in dts and thus gpio_is_valid will return false, therefore, no jack. Moving sjack->pin.pin = pin_name; sjack->pin.mask = mask; snd_soc_card_jack_new(card, pin_name, mask, &sjack->jack, &sjack->pin, 1); and for_each_card_components(card, component) snd_soc_component_set_jack(component, &sjack->jack, NULL); outside of the if block should make this work, at least cpcap gets the jack then.
Hi Carl >> (X) int asoc_simple_init_jack(...) >> { >> ... >> if (gpio_is_valid(det)) { >> ... >> >> snd_soc_card_jack_new(...); >> snd_soc_jack_add_gpios(...); >> for_each_card_components(card, component) >> (Y) snd_soc_component_set_jack(component, ...); >> } >> ... >> } > > So for the case of cpcap codec on motorola mapphones this dosent help, > because we dont have a gpio to sense plug state, thus no gpio in dts > and thus gpio_is_valid will return false, therefore, no jack. > > Moving > > sjack->pin.pin = pin_name; > sjack->pin.mask = mask; > > snd_soc_card_jack_new(card, pin_name, mask, > &sjack->jack, > &sjack->pin, 1); > > and > > for_each_card_components(card, component) > snd_soc_component_set_jack(component, &sjack->jack, NULL); > > outside of the if block should make this work, at least cpcap gets the jack then. I see. simple-card is checking hp-det-gpio on DT now. I guess it can help you if simple-card also check "hp-det" (no gpio) (and customize previous patch) ? Is "enable-hp" better naming... ? Best regards --- Kuninori Morimoto
diff --git a/include/sound/simple_card_utils.h b/include/sound/simple_card_utils.h index 51b3b485a92e..547ad537613d 100644 --- a/include/sound/simple_card_utils.h +++ b/include/sound/simple_card_utils.h @@ -57,8 +57,8 @@ struct asoc_simple_priv { struct prop_nums num; unsigned int mclk_fs; } *dai_props; - struct asoc_simple_jack hp_jack; - struct asoc_simple_jack mic_jack; + struct asoc_simple_jack *hp_jack; + struct asoc_simple_jack *mic_jack; struct snd_soc_dai_link *dai_link; struct asoc_simple_dai *dais; struct snd_soc_dai_link_component *dlcs; @@ -173,7 +173,7 @@ int asoc_simple_parse_pin_switches(struct snd_soc_card *card, char *prefix); int asoc_simple_init_jack(struct snd_soc_card *card, - struct asoc_simple_jack *sjack, + struct asoc_simple_jack **sjack, int is_hp, char *prefix, char *pin); int asoc_simple_init_priv(struct asoc_simple_priv *priv, struct link_info *li); diff --git a/sound/soc/generic/simple-card-utils.c b/sound/soc/generic/simple-card-utils.c index 10c63b73900c..1899feba16cc 100644 --- a/sound/soc/generic/simple-card-utils.c +++ b/sound/soc/generic/simple-card-utils.c @@ -395,6 +395,7 @@ int asoc_simple_dai_init(struct snd_soc_pcm_runtime *rtd) struct asoc_simple_priv *priv = snd_soc_card_get_drvdata(rtd->card); struct simple_dai_props *props = simple_priv_to_props(priv, rtd->num); struct asoc_simple_dai *dai; + struct snd_soc_component *component; int i, ret; for_each_prop_dai_codec(props, i, dai) { @@ -412,6 +413,21 @@ int asoc_simple_dai_init(struct snd_soc_pcm_runtime *rtd) if (ret < 0) return ret; + for_each_rtd_components(rtd, i, component) { + if (component->driver->set_jack) { + if (!priv->hp_jack) { + priv->hp_jack = devm_kzalloc(priv->snd_card.dev, + sizeof(*priv->hp_jack), GFP_KERNEL); + snd_soc_card_jack_new(&priv->snd_card, + "Headphones", + SND_JACK_HEADPHONE, + &priv->hp_jack->jack, + NULL, 0); + } + snd_soc_component_set_jack(component, &priv->hp_jack->jack, NULL); + } + } + return 0; } EXPORT_SYMBOL_GPL(asoc_simple_dai_init); @@ -554,7 +570,7 @@ int asoc_simple_parse_pin_switches(struct snd_soc_card *card, EXPORT_SYMBOL_GPL(asoc_simple_parse_pin_switches); int asoc_simple_init_jack(struct snd_soc_card *card, - struct asoc_simple_jack *sjack, + struct asoc_simple_jack **sjack, int is_hp, char *prefix, char *pin) { @@ -569,8 +585,6 @@ int asoc_simple_init_jack(struct snd_soc_card *card, if (!prefix) prefix = ""; - sjack->gpio.gpio = -ENOENT; - if (is_hp) { snprintf(prop, sizeof(prop), "%shp-det-gpio", prefix); pin_name = pin ? pin : "Headphones"; @@ -588,21 +602,26 @@ int asoc_simple_init_jack(struct snd_soc_card *card, return -EPROBE_DEFER; if (gpio_is_valid(det)) { - sjack->pin.pin = pin_name; - sjack->pin.mask = mask; + struct asoc_simple_jack *sjack_d; + + sjack = devm_kzalloc(dev, sizeof(*(*sjack)), GFP_KERNEL); + sjack_d = *sjack; + + sjack_d->pin.pin = pin_name; + sjack_d->pin.mask = mask; - sjack->gpio.name = gpio_name; - sjack->gpio.report = mask; - sjack->gpio.gpio = det; - sjack->gpio.invert = !!(flags & OF_GPIO_ACTIVE_LOW); - sjack->gpio.debounce_time = 150; + sjack_d->gpio.name = gpio_name; + sjack_d->gpio.report = mask; + sjack_d->gpio.gpio = det; + sjack_d->gpio.invert = !!(flags & OF_GPIO_ACTIVE_LOW); + sjack_d->gpio.debounce_time = 150; snd_soc_card_jack_new(card, pin_name, mask, - &sjack->jack, - &sjack->pin, 1); + &sjack_d->jack, + &sjack_d->pin, 1); - snd_soc_jack_add_gpios(&sjack->jack, 1, - &sjack->gpio); + snd_soc_jack_add_gpios(&sjack_d->jack, 1, + &sjack_d->gpio); } return 0;
This allows componants that want a jack to report state on to do so by calling set_jack on components implementing this function. Im not entirely sure this is the right way to do this so RFC Signed-off-by: Carl Philipp Klemm <philipp@uvos.xyz> --- include/sound/simple_card_utils.h | 6 ++-- sound/soc/generic/simple-card-utils.c | 47 +++++++++++++++++++-------- 2 files changed, 36 insertions(+), 17 deletions(-)