[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <858c2dc7-e439-b3b7-f768-8974c3e1b15b@axis.com>
Date: Wed, 18 Jan 2023 15:04:47 +0100
From: Astrid Rost <astridr@...s.com>
To: Amadeusz Sławiński
<amadeuszx.slawinski@...ux.intel.com>,
Astrid Rost <astrid.rost@...s.com>,
Mark Brown <broonie@...nel.org>,
Liam Girdwood <lgirdwood@...il.com>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Jaroslav Kysela <perex@...ex.cz>, Takashi Iwai <tiwai@...e.com>
CC: <alsa-devel@...a-project.org>, <kernel@...s.com>,
<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v1 2/4] ASoC: simple-card-utils: create jack inputs for
aux_devs
On 1/18/23 14:39, Amadeusz Sławiński wrote:
> On 1/18/2023 1:52 PM, Astrid Rost wrote:
>> Add a generic way to create jack inputs for auxiliary jack detection
>> drivers (e.g. via i2c, spi), which are not part of any real codec.
>> The simple-card can be used as combining card driver to add the jacks,
>> no new one is required.
>>
>> Create a jack (for input-events) for jack devices in the auxiliary
>> device list (aux_devs). A device which has the functions set_jack and
>> get_jack_supported_type counts as jack device.
>>
>> Add a devicetree entry, in case not all input types are required.
>> simple-card,aux-jack-types:
>> Array of snd_jack_type to create jack-input-event for jack devices in
>> aux-devs. If the setting is 0, the supported type of the device is used.
>>
>> Signed-off-by: Astrid Rost <astrid.rost@...s.com>
>> ---
>> include/sound/simple_card_utils.h | 3 ++
>> sound/soc/generic/simple-card-utils.c | 63 +++++++++++++++++++++++++++
>> sound/soc/generic/simple-card.c | 4 ++
>> 3 files changed, 70 insertions(+)
>>
>> diff --git a/include/sound/simple_card_utils.h
>> b/include/sound/simple_card_utils.h
>> index 38590f1ae9ee..a3f3f3aa9e6e 100644
>> --- a/include/sound/simple_card_utils.h
>> +++ b/include/sound/simple_card_utils.h
>> @@ -69,6 +69,7 @@ struct asoc_simple_priv {
>> } *dai_props;
>> struct asoc_simple_jack hp_jack;
>> struct asoc_simple_jack mic_jack;
>> + struct snd_soc_jack *aux_jacks;
>> struct snd_soc_dai_link *dai_link;
>> struct asoc_simple_dai *dais;
>> struct snd_soc_dai_link_component *dlcs;
>> @@ -187,6 +188,8 @@ int asoc_simple_parse_pin_switches(struct
>> snd_soc_card *card,
>> int asoc_simple_init_jack(struct snd_soc_card *card,
>> struct asoc_simple_jack *sjack,
>> int is_hp, char *prefix, char *pin);
>> +int asoc_simple_init_aux_jacks(struct asoc_simple_priv *priv,
>> + char *prefix);
>> int asoc_simple_init_priv(struct asoc_simple_priv *priv,
>> struct link_info *li);
>> int asoc_simple_remove(struct platform_device *pdev);
>> diff --git a/sound/soc/generic/simple-card-utils.c
>> b/sound/soc/generic/simple-card-utils.c
>> index e35becce9635..668123549fbf 100644
>> --- a/sound/soc/generic/simple-card-utils.c
>> +++ b/sound/soc/generic/simple-card-utils.c
>> @@ -786,6 +786,69 @@ int asoc_simple_init_jack(struct snd_soc_card *card,
>> }
>> EXPORT_SYMBOL_GPL(asoc_simple_init_jack);
>> +int asoc_simple_init_aux_jacks(struct asoc_simple_priv *priv, char
>> *prefix)
>> +{
>> + struct snd_soc_card *card = simple_priv_to_card(priv);
>> + struct device *dev = card->dev;
>> + struct snd_soc_component *component;
>> + char prop[128];
>> + int found_jack_index = 0;
>> + int num = 0;
>> + int ret;
>> +
>> + if (priv->aux_jacks)
>> + return 0;
>> +
>> + snprintf(prop, sizeof(prop), "%saux-jack-types", prefix);
>> + num = of_property_count_u32_elems(dev->of_node, prop);
>> + if (num < 1)
>> + return 0;
>> +
>> + priv->aux_jacks = devm_kcalloc(card->dev, num,
>> + sizeof(struct snd_soc_jack), GFP_KERNEL);
>> + if (!priv->aux_jacks)
>> + return -ENOMEM;
>> +
>> + for_each_card_auxs(card, component) {
>> + if (found_jack_index >= num)
>> + break;
>> +
>> + if (component->driver->set_jack &&
>> + component->driver->get_jack_supported_type) {
>
> This check is really weird, you are checking if callbacks exist at all,
> which should be unnecessary as it duplicates the work oh
> snd_soc_component_ functions. I would just try to call
> snd_soc_component_get_jack_supported_type() directly and if it returns
> -ENOTSUPP, just go to next iteration.
> I guess your main problem is snd_soc_component_set_jack(), but you can
> just call it with NULL jack to check if it returns -ENOTSUPP, but I
> guess the overall asumption would be that if someone implements
> .get_jack_supported_type, they also implemented .set_jack, so maybe it
> is just unnecessary?
>
Thank you!
Yes, it works fine without it. I will remove it.
>> + char id[128];
>> + int type = 0;
>> + struct snd_soc_jack *jack =
>> + &(priv->aux_jacks[found_jack_index]);
>> + int type_supp_mask =
>> + snd_soc_component_get_jack_supported_type(
>> + component);
>> +
>> + ret = of_property_read_u32_index(
>> + dev->of_node, prop, found_jack_index++, &type);
>> + if (ret)
>> + continue;
>> +
>> + if (type)
>> + type &= type_supp_mask; /* use only supported types */
>> + else
>> + type = type_supp_mask; /* use all supported types */
>> +
>> + if (!type)
>> + continue;
>> +
>> + /* create jack */
>> + snprintf(id, sizeof(id), "%s-jack", component->name);
>> + ret = snd_soc_card_jack_new(card, id, type, jack);
>> + if (ret)
>> + continue;
>> +
>> + (void)snd_soc_component_set_jack(component, jack, NULL);
>> + }
>> + }
>> + return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(asoc_simple_init_aux_jacks);
>> +
>> int asoc_simple_init_priv(struct asoc_simple_priv *priv,
>> struct link_info *li)
>> {
>> diff --git a/sound/soc/generic/simple-card.c
>> b/sound/soc/generic/simple-card.c
>> index feb55b66239b..e98932c16754 100644
>> --- a/sound/soc/generic/simple-card.c
>> +++ b/sound/soc/generic/simple-card.c
>> @@ -623,6 +623,10 @@ static int simple_soc_probe(struct snd_soc_card
>> *card)
>> if (ret < 0)
>> return ret;
>> + ret = asoc_simple_init_aux_jacks(priv, PREFIX);
>> + if (ret < 0)
>> + return ret;
>> +
>> return 0;
>> }
>
Powered by blists - more mailing lists