[PATCH] alsa-lib: if card driver name is empty string, use card name instead

[PATCH] alsa-lib: if card driver name is empty string, use card name instead

[Scott Jiang]

Some drivers don't have a card driver name, but they have a nice card name containing enough info.
So we can use card name to search card config if we found driver name is not valid.

Signed-off-by: Scott Jiang <scott.jiang.linux@gmail.com>
---
 src/confmisc.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/src/confmisc.c b/src/confmisc.c
index 80b0027..3c4b5aa 100644
--- a/src/confmisc.c
+++ b/src/confmisc.c
@@ -666,6 +666,7 @@ int snd_determine_driver(int card, char **driver)
 	snd_ctl_t *ctl = NULL;
 	snd_ctl_card_info_t *info;
 	char *res = NULL;
+	const char *name = NULL;
 	int err;
 
 	assert(card >= 0 && card <= 32);
@@ -680,7 +681,12 @@ int snd_determine_driver(int card, char **driver)
 		SNDERR("snd_ctl_card_info error: %s", snd_strerror(err));
 		goto __error;
 	}
-	res = strdup(snd_ctl_card_info_get_driver(info));
+	name = snd_ctl_card_info_get_driver(info);
+	/* if card driver name is empty string, try to use card name */
+	if (name[0] == '\0')
+		name = snd_ctl_card_info_get_name(info);
+
+	res = strdup(name);
 	if (res == NULL)
 		err = -ENOMEM;
 	else {
-- 
1.7.0.4

Re: [PATCH] alsa-lib: if card driver name is empty string, use card name instead

[Takashi Iwai]

At Tue, 20 Sep 2011 13:15:44 -0400,
Scott Jiang wrote:
> 
> Some drivers don't have a card driver name, but they have a nice card name containing enough info.
> So we can use card name to search card config if we found driver name is not valid.
> 
> Signed-off-by: Scott Jiang <scott.jiang.linux@gmail.com>

It's rather a bug of driver, no?
I don't think we need to work around it.


thanks,

Takashi

> ---
>  src/confmisc.c |    8 +++++++-
>  1 files changed, 7 insertions(+), 1 deletions(-)
> 
> diff --git a/src/confmisc.c b/src/confmisc.c
> index 80b0027..3c4b5aa 100644
> --- a/src/confmisc.c
> +++ b/src/confmisc.c
> @@ -666,6 +666,7 @@ int snd_determine_driver(int card, char **driver)
>  	snd_ctl_t *ctl = NULL;
>  	snd_ctl_card_info_t *info;
>  	char *res = NULL;
> +	const char *name = NULL;
>  	int err;
>  
>  	assert(card >= 0 && card <= 32);
> @@ -680,7 +681,12 @@ int snd_determine_driver(int card, char **driver)
>  		SNDERR("snd_ctl_card_info error: %s", snd_strerror(err));
>  		goto __error;
>  	}
> -	res = strdup(snd_ctl_card_info_get_driver(info));
> +	name = snd_ctl_card_info_get_driver(info);
> +	/* if card driver name is empty string, try to use card name */
> +	if (name[0] == '\0')
> +		name = snd_ctl_card_info_get_name(info);
> +
> +	res = strdup(name);
>  	if (res == NULL)
>  		err = -ENOMEM;
>  	else {
> -- 
> 1.7.0.4
> 
> 

Re: [PATCH] alsa-lib: if card driver name is empty string, use card name instead

[Scott Jiang]

2011/9/20 Takashi Iwai <tiwai@suse.de>:
> At Tue, 20 Sep 2011 13:15:44 -0400,
> Scott Jiang wrote:
>>
>> Some drivers don't have a card driver name, but they have a nice card name containing enough info.
>> So we can use card name to search card config if we found driver name is not valid.
>>
>> Signed-off-by: Scott Jiang <scott.jiang.linux@gmail.com>
>
> It's rather a bug of driver, no?
> I don't think we need to work around it.
>
Mark said it's a bug of app..., he believes driver has provided enough
info in card name.

Re: [PATCH] alsa-lib: if card driver name is empty string, use card name instead

[Takashi Iwai]

At Tue, 20 Sep 2011 14:37:04 +0800,
Scott Jiang wrote:
> 
> 2011/9/20 Takashi Iwai <tiwai@suse.de>:
> > At Tue, 20 Sep 2011 13:15:44 -0400,
> > Scott Jiang wrote:
> >>
> >> Some drivers don't have a card driver name, but they have a nice card name containing enough info.
> >> So we can use card name to search card config if we found driver name is not valid.
> >>
> >> Signed-off-by: Scott Jiang <scott.jiang.linux@gmail.com>
> >
> > It's rather a bug of driver, no?
> > I don't think we need to work around it.
> >
> Mark said it's a bug of app..., he believes driver has provided enough
> info in card name.

No, the driver name should be set, too.


Takashi

Re: [PATCH] alsa-lib: if card driver name is empty string, use card name instead

[Scott Jiang]

2011/9/20 Takashi Iwai <tiwai@suse.de>:
> At Tue, 20 Sep 2011 14:37:04 +0800,
> Scott Jiang wrote:
>>
>> 2011/9/20 Takashi Iwai <tiwai@suse.de>:
>> > At Tue, 20 Sep 2011 13:15:44 -0400,
>> > Scott Jiang wrote:
>> >>
>> >> Some drivers don't have a card driver name, but they have a nice card name containing enough info.
>> >> So we can use card name to search card config if we found driver name is not valid.
>> >>
>> >> Signed-off-by: Scott Jiang <scott.jiang.linux@gmail.com>
>> >
>> > It's rather a bug of driver, no?
>> > I don't think we need to work around it.
>> >
>> Mark said it's a bug of app..., he believes driver has provided enough
>> info in card name.
>
> No, the driver name should be set, too.
>
I agree with you, but my driver patch has been rejected by Mark.

On Fri, Sep 16, 2011 at 11:53:16AM +0800, Scott Jiang wrote:
> 2011/9/15 Mark Brown <broonie@opensource.wolfsonmicro.com>:
> > On Wed, Sep 14, 2011 at 11:27:28AM +0800, Scott Jiang wrote:


> >> in alsa lib snd_config_hook_load_for_all_cards(), use
> >> sndrv_ctl_card_info->driver to determine card config,
> >> not sndrv_ctl_card_info->name, though card name contains enough info.

> > So the issue isn't that the driver doesn't have a name, it's that you
> > don't like the name it was given.

> Do you mean alsa lib needs to modify its way to determine which card
> config to load?
> I don't think alsa lib guys will agree.


Well, that's one option.


> >The changelog needs to explain this,
> > and also explain why this is an issue in this one driver.

> Not only for this one driver, it's a problem for all drivers under
> asoc. Because sndrv_ctl_card_info->driver is an empty string now.


So if that's the case then it seems very clear that a driver specific
change isn't a good way of addressing the issue.  If the driver name is
null it actually seems like modifying userspace should be very easy, we
simply need to fall back to reading the card name if the driver name is
empty.

Re: [PATCH] alsa-lib: if card driver name is empty string, use card name instead

[Takashi Iwai]

At Tue, 20 Sep 2011 14:55:14 +0800,
Scott Jiang wrote:
> 
> 2011/9/20 Takashi Iwai <tiwai@suse.de>:
> > At Tue, 20 Sep 2011 14:37:04 +0800,
> > Scott Jiang wrote:
> >>
> >> 2011/9/20 Takashi Iwai <tiwai@suse.de>:
> >> > At Tue, 20 Sep 2011 13:15:44 -0400,
> >> > Scott Jiang wrote:
> >> >>
> >> >> Some drivers don't have a card driver name, but they have a nice card name containing enough info.
> >> >> So we can use card name to search card config if we found driver name is not valid.
> >> >>
> >> >> Signed-off-by: Scott Jiang <scott.jiang.linux@gmail.com>
> >> >
> >> > It's rather a bug of driver, no?
> >> > I don't think we need to work around it.
> >> >
> >> Mark said it's a bug of app..., he believes driver has provided enough
> >> info in card name.
> >
> > No, the driver name should be set, too.
> >
> I agree with you, but my driver patch has been rejected by Mark.

That's bad.  But it's a rule since over 10 years ago: the driver
should set driver field.


Takashi

Re: [PATCH] alsa-lib: if card driver name is empty string, use card name instead

[Mark Brown]

On Tue, Sep 20, 2011 at 09:12:19AM +0200, Takashi Iwai wrote:
> Scott Jiang wrote:

> > I agree with you, but my driver patch has been rejected by Mark.

> That's bad.  But it's a rule since over 10 years ago: the driver
> should set driver field.

Scott, as I've said every time you've posted your patch there's several
problems with your patch:

 - The name you've chosen is really not at all suitable, it doesn't
   identify the system at all.
 - This clearly isn't something that affects only one driver - no ASoC
   driver sets a driver name - so a change to a single driver is clearly
   not a good solution even for just Blackfin.

You need to address these issues, they're both blocking.

Re: [PATCH] alsa-lib: if card driver name is empty string, use card name instead

[Scott Jiang]

2011/9/20 Mark Brown <broonie@opensource.wolfsonmicro.com>:
> On Tue, Sep 20, 2011 at 09:12:19AM +0200, Takashi Iwai wrote:
>> Scott Jiang wrote:
>
>> > I agree with you, but my driver patch has been rejected by Mark.
>
>> That's bad.  But it's a rule since over 10 years ago: the driver
>> should set driver field.
>
> Scott, as I've said every time you've posted your patch there's several
> problems with your patch:
>
>  - The name you've chosen is really not at all suitable, it doesn't
>   identify the system at all.
I said I wound modify the name. But I want to know how to fix this
problem before I send a new patch.

>  - This clearly isn't something that affects only one driver - no ASoC
>   driver sets a driver name - so a change to a single driver is clearly
>   not a good solution even for just Blackfin.
>
I thought about this. I wanted to copy codec name to driver name as
before in asoc but I don't think it's a good idea.
Now we can have multicodecs in one card, they may not be the same, right?
If yes, we should add this name in each snd_soc_card.

Re: [PATCH] alsa-lib: if card driver name is empty string, use card name instead

[Mark Brown]

On Tue, Sep 20, 2011 at 06:36:15PM +0800, Scott Jiang wrote:
> 2011/9/20 Mark Brown <broonie@opensource.wolfsonmicro.com>:

> >  - This clearly isn't something that affects only one driver - no ASoC
> >   driver sets a driver name - so a change to a single driver is clearly
> >   not a good solution even for just Blackfin.

> I thought about this. I wanted to copy codec name to driver name as
> before in asoc but I don't think it's a good idea.
> Now we can have multicodecs in one card, they may not be the same, right?
> If yes, we should add this name in each snd_soc_card.

If we want to do that then we need to go round every single machine
driver and add a driver name, plus add code in the core which flags
cards that don't set one.

Re: [PATCH] alsa-lib: if card driver name is empty string, use card name instead

[Mark Brown]

On Tue, Sep 20, 2011 at 08:24:52AM +0200, Takashi Iwai wrote:
> Scott Jiang wrote:

> > Some drivers don't have a card driver name, but they have a nice card name containing enough info.
> > So we can use card name to search card config if we found driver name is not valid.

> It's rather a bug of driver, no?
> I don't think we need to work around it.

This was introduced by your commit 873bd4 (ASoC: don't set invalid name
string to snd_card->driver field) which means we no longer provide a
driver name for any ASoC cards as they're all relying on using the card
name as the driver name.  We should be doing something like replacing '
' by '_' in the driver name we generate rather than just not generating
a name.

I have to confess I didn't review the patch at the time as it was
applied too quickly after it was mailed out for me to have a chance to
see it.  Sorry about that.

Re: [PATCH] alsa-lib: if card driver name is empty string, use card name instead

[Takashi Iwai]

At Tue, 20 Sep 2011 11:14:34 +0100,
Mark Brown wrote:
> 
> On Tue, Sep 20, 2011 at 08:24:52AM +0200, Takashi Iwai wrote:
> > Scott Jiang wrote:
> 
> > > Some drivers don't have a card driver name, but they have a nice card name containing enough info.
> > > So we can use card name to search card config if we found driver name is not valid.
> 
> > It's rather a bug of driver, no?
> > I don't think we need to work around it.
> 
> This was introduced by your commit 873bd4 (ASoC: don't set invalid name
> string to snd_card->driver field) which means we no longer provide a
> driver name for any ASoC cards as they're all relying on using the card
> name as the driver name.  We should be doing something like replacing '
> ' by '_' in the driver name we generate rather than just not generating
> a name.
> 
> I have to confess I didn't review the patch at the time as it was
> applied too quickly after it was mailed out for me to have a chance to
> see it.  Sorry about that.

Well, you can follow a short history from the commit: the commit above
was really a fix to get back to the old good behavior.  Until 3.0,
ASoC had no way to set card->driver field.  The method to set
card->driver was firstly introduced by Liam's patch in 3.0-rc2,
22de71ba03311cdc1063757c50a1488cb90a1fca
	ASoC: core - allow ASoC more flexible machine name

Then it turned out this gives a string "(null)", so some workaround
was added, 2b39535b9e54888649923beaab443af212b6c0fd
    ASoC: core: Don't set "(null)" as a driver name

But, this was no good move, too.  The card->driver field is to be a
concise string without special letters while card->name contains more
flexible string.  So, I changed the way back to the state before 3.0
there, the commit 873bd4.

All this happened in 3.0 development cycle, so it's no longer history
than NULL driver name :)

Of course, it'd be nice to implement a logic in ASoC core to
automatically generate some valid driver-name string.  But, the driver
name string is at most 15 letters, and card->name is an arbitrary
string, so you'd need to do it a bit carefully.


thanks,

Takashi

Re: [PATCH] alsa-lib: if card driver name is empty string, use card name instead

[Mark Brown]

On Tue, Sep 20, 2011 at 12:33:41PM +0200, Takashi Iwai wrote:

> Well, you can follow a short history from the commit: the commit above
> was really a fix to get back to the old good behavior.  Until 3.0,
> ASoC had no way to set card->driver field.  The method to set

No - older versions of ASoC always automatically generated a driver
name (badly but that's a separate story).  I guess that got broken with
multi-component but didn't bother checking.

> But, this was no good move, too.  The card->driver field is to be a
> concise string without special letters while card->name contains more
> flexible string.  So, I changed the way back to the state before 3.0
> there, the commit 873bd4.

Which unfortunately restored the original problem which was being fixed
by Jarkko.

> Of course, it'd be nice to implement a logic in ASoC core to
> automatically generate some valid driver-name string.  But, the driver
> name string is at most 15 letters, and card->name is an arbitrary
> string, so you'd need to do it a bit carefully.

I think the card name is fine, people don't tend to write anything
terribly long there and very little actually cares about the driver name
- alsa-lib's config loading thing is the only thing I'm aware of (and
that's not being terribly useful really, one driver can easily support
multiple cards).  If it's a problem people can always explicitly set
something.

Re: [PATCH] alsa-lib: if card driver name is empty string, use card name instead

[Takashi Iwai]

At Tue, 20 Sep 2011 11:51:57 +0100,
Mark Brown wrote:
> 
> On Tue, Sep 20, 2011 at 12:33:41PM +0200, Takashi Iwai wrote:
> 
> > Well, you can follow a short history from the commit: the commit above
> > was really a fix to get back to the old good behavior.  Until 3.0,
> > ASoC had no way to set card->driver field.  The method to set
> 
> No - older versions of ASoC always automatically generated a driver
> name (badly but that's a separate story).  I guess that got broken with
> multi-component but didn't bother checking.

Ah, OK, so the problem existed since long time ago...

> > But, this was no good move, too.  The card->driver field is to be a
> > concise string without special letters while card->name contains more
> > flexible string.  So, I changed the way back to the state before 3.0
> > there, the commit 873bd4.
> 
> Which unfortunately restored the original problem which was being fixed
> by Jarkko.
> 
> > Of course, it'd be nice to implement a logic in ASoC core to
> > automatically generate some valid driver-name string.  But, the driver
> > name string is at most 15 letters, and card->name is an arbitrary
> > string, so you'd need to do it a bit carefully.
> 
> I think the card name is fine, people don't tend to write anything
> terribly long there and very little actually cares about the driver name
> - alsa-lib's config loading thing is the only thing I'm aware of (and
> that's not being terribly useful really, one driver can easily support
> multiple cards).  If it's a problem people can always explicitly set
> something.

Sounds reasonable.


thanks,

Takashi