diff mbox

[07/39,v2] ASoC: simple-card-utils: add asoc_simple_card_parse_card_name()

Message ID 8760tusj2q.wl%kuninori.morimoto.gx@renesas.com (mailing list archive)
State Changes Requested
Delegated to: Geert Uytterhoeven
Headers show

Commit Message

Kuninori Morimoto May 31, 2016, 9:01 a.m. UTC
From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

simple-card needs to get its card name.
This patch makes this method simple style standard.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 include/sound/simple_card_utils.h     |  2 ++
 sound/soc/generic/simple-card-utils.c | 20 ++++++++++++++++++++
 2 files changed, 22 insertions(+)

Comments

Mark Brown June 29, 2016, 6:15 p.m. UTC | #1
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.
Kuninori Morimoto June 30, 2016, 2:55 a.m. UTC | #2
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>
--------------------
Mark Brown July 1, 2016, 9:52 a.m. UTC | #3
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.
Kuninori Morimoto July 3, 2016, 11:54 p.m. UTC | #4
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.
Kuninori Morimoto July 4, 2016, 12:20 a.m. UTC | #5
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.
Mark Brown July 4, 2016, 8:50 a.m. UTC | #6
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.
Kuninori Morimoto July 4, 2016, 8:59 a.m. UTC | #7
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;
Mark Brown July 5, 2016, 12:10 p.m. UTC | #8
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.
Kuninori Morimoto July 5, 2016, 11:12 p.m. UTC | #9
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 mbox

Patch

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);