| Message ID | 8760tusj2q.wl%kuninori.morimoto.gx@renesas.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, May 31, 2016 at 09:01:34AM +0000, Kuninori Morimoto wrote: > + if (!card->name) > + card->name = card->dai_link->name; This will unconditionally defererence dai_link but it's optional - we can have analogue only cards.
Hi Mark > > + if (!card->name) > > + card->name = card->dai_link->name; > > This will unconditionally defererence dai_link but it's optional - we > can have analogue only cards. This is not new feature. Current simple-card already has it. -------------------- commit 2772555b6c5ba79783c04ea6c60549530d737e2e Author: Xiubo Li <Li.Xiubo@freescale.com> Date: Fri Jan 24 15:43:02 2014 +0800 ASoC: simple-card: Add snd_card's name parsing from DT node support If the DT is used and the CPU DAI device has only one DAI, the card name will be like : ALSA device list: 0: 40031000.sai-sgtl5000 And this name maybe a little ugly to some customers, so here the card name parsing from DT node is supported. Signed-off-by: Xiubo Li <Li.Xiubo@freescale.com> Signed-off-by: Mark Brown <broonie@linaro.org> --------------------
On Thu, Jun 30, 2016 at 02:55:06AM +0000, Kuninori Morimoto wrote: > > > + if (!card->name) > > > + card->name = card->dai_link->name; > > This will unconditionally defererence dai_link but it's optional - we > > can have analogue only cards. > This is not new feature. Current simple-card already has it. Right, but simple-card does need DAIs IIRC while this is intended to be more general. All it needs is a check before the dereference to be safe so it's trivial to handle.
Hi Mark > > > > + if (!card->name) > > > > + card->name = card->dai_link->name; > > > > This will unconditionally defererence dai_link but it's optional - we > > > can have analogue only cards. > > > This is not new feature. Current simple-card already has it. > > Right, but simple-card does need DAIs IIRC while this is intended to be > more general. All it needs is a check before the dereference to be safe > so it's trivial to handle. Sorry, I'm not 100% understand. Do you mean we don't need handle card->name ? (= we should remove above ?) If so, we can't register card on simple-card. Because snd_soc_register_card() requests it.
Hi Mark, again sorry previous was not good question > > > > > + if (!card->name) > > > > > + card->name = card->dai_link->name; > > > > > > This will unconditionally defererence dai_link but it's optional - we > > > > can have analogue only cards. > > > > > This is not new feature. Current simple-card already has it. > > > > Right, but simple-card does need DAIs IIRC while this is intended to be > > more general. All it needs is a check before the dereference to be safe > > so it's trivial to handle. > > Sorry, I'm not 100% understand. > Do you mean we don't need handle card->name ? (= we should remove above ?) > If so, we can't register card on simple-card. Because snd_soc_register_card() > requests it. This function tries to get card name from snd_soc_of_parse_card_name(). and it tries to set card->name from dai_link if card still doesn't have name. So, above is optional 2nd try. Or, do you mean this if (!card->name) can goes to simple-card, instead of utils ? I have no objection about it , but it can be double handling ? Because other simple family have same situation.
On Mon, Jul 04, 2016 at 12:20:29AM +0000, Kuninori Morimoto wrote: > > > > > > + if (!card->name) > > > > > > + card->name = card->dai_link->name; > > > > > This will unconditionally defererence dai_link but it's optional - we > > > > > can have analogue only cards. > > > > This is not new feature. Current simple-card already has it. > > > Right, but simple-card does need DAIs IIRC while this is intended to be > > > more general. All it needs is a check before the dereference to be safe > > > so it's trivial to handle. > > Sorry, I'm not 100% understand. > > Do you mean we don't need handle card->name ? (= we should remove above ?) > > If so, we can't register card on simple-card. Because snd_soc_register_card() > > requests it. > This function tries to get card name from snd_soc_of_parse_card_name(). > and it tries to set card->name from dai_link if card still doesn't have name. > So, above is optional 2nd try. > Or, do you mean this if (!card->name) can goes to simple-card, instead of utils ? > I have no objection about it , but it can be double handling ? > Because other simple family have same situation. If we try to dereference card->dai_link without checking to see if it's set then we'll crash.
Hi Mark Thank you for your feedback > > This function tries to get card name from snd_soc_of_parse_card_name(). > > and it tries to set card->name from dai_link if card still doesn't have name. > > So, above is optional 2nd try. > > Or, do you mean this if (!card->name) can goes to simple-card, instead of utils ? > > I have no objection about it , but it can be double handling ? > > Because other simple family have same situation. > > If we try to dereference card->dai_link without checking to see if it's > set then we'll crash. Ahh, do you mean we need like this ? if (!card->name && card->dai_link) card->name = card->dai_link->name;
On Mon, Jul 04, 2016 at 08:59:53AM +0000, Kuninori Morimoto wrote: > > If we try to dereference card->dai_link without checking to see if it's > > set then we'll crash. > Ahh, do you mean we need like this ? > if (!card->name && card->dai_link) > card->name = card->dai_link->name; Yes, exactly.
Hi Mark > > Ahh, do you mean we need like this ? > > > if (!card->name && card->dai_link) > > card->name = card->dai_link->name; > > Yes, exactly. OK, I understand. next version will do it.
diff --git a/include/sound/simple_card_utils.h b/include/sound/simple_card_utils.h index 41e567b..2f991da 100644 --- a/include/sound/simple_card_utils.h +++ b/include/sound/simple_card_utils.h @@ -31,5 +31,7 @@ int asoc_simple_card_parse_tdm(struct device_node *port_np, struct asoc_simple_dai *simple_dai); int asoc_simple_card_parse_dailink_name(struct device *dev, struct snd_soc_dai_link *dai_link); +int asoc_simple_card_parse_card_name(struct snd_soc_card *card, + char *prefix); #endif /* __SIMPLE_CARD_CORE_H */ diff --git a/sound/soc/generic/simple-card-utils.c b/sound/soc/generic/simple-card-utils.c index 9b49b5a..c782b3a 100644 --- a/sound/soc/generic/simple-card-utils.c +++ b/sound/soc/generic/simple-card-utils.c @@ -105,3 +105,23 @@ int asoc_simple_card_parse_dailink_name(struct device *dev, return ret; } EXPORT_SYMBOL_GPL(asoc_simple_card_parse_dailink_name); + +int asoc_simple_card_parse_card_name(struct snd_soc_card *card, + char *prefix) +{ + char prop[128]; + int ret; + + snprintf(prop, sizeof(prop), "%sname", prefix); + + /* Parse the card name from DT */ + ret = snd_soc_of_parse_card_name(card, prop); + if (ret < 0) + return ret; + + if (!card->name) + card->name = card->dai_link->name; + + return 0; +} +EXPORT_SYMBOL_GPL(asoc_simple_card_parse_card_name);