diff mbox

[1/2] ucm: Automatically load the best config file based on the card long name

Message ID 1482325768-20084-1-git-send-email-mengdong.lin@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

mengdong.lin@linux.intel.com Dec. 21, 2016, 1:09 p.m. UTC
From: Mengdong Lin <mengdong.lin@linux.intel.com>

Intel DSP platform drivers are used by many different devices. For user
space to differentiate them, ASoC machine driver may include the DMI info
(vendor, product and board) in card long name. Possible card long names
are:
broadwell-rt286-Dell Inc.-XPS 13 9343-0310JH
broadwell-rt286-Intel Corp.-Broadwell Client platform-Wilson Beach SDS
bytcr-rt5640-ASUSTeK COMPUTER INC.-T100TA-T100TA
bytcr-rt5651-Circuitco-Minnowboard Max D0 PLATFORM-MinnowBoard MAX
...

And user space can define configuration files including fields separated
by '.' as below:
broadwell-rt286
broadwell-rt286.Dell.XPS
bytcr-rt5640
bytcr-rt5640.ASUS.T100
bytcr-rt5651.MinnowboardMax
...

When being asked to load configuration file of a card, UCM will try to
find the card long name from the local machine, and then scan all
available configuration file names, search every field of config file
name in the card long name. The more characters match, the higher score
the file has. Finally, the file with the highest score will be loaded to
configure the sound card.

Signed-off-by: Mengdong Lin <mengdong.lin@linux.intel.com>

Comments

Liam Girdwood Dec. 21, 2016, 2:38 p.m. UTC | #1
On Wed, 2016-12-21 at 21:09 +0800, mengdong.lin@linux.intel.com wrote:
> From: Mengdong Lin <mengdong.lin@linux.intel.com>
> 
> Intel DSP platform drivers are used by many different devices. For user
> space to differentiate them, ASoC machine driver may include the DMI info
> (vendor, product and board) in card long name. Possible card long names
> are:
> broadwell-rt286-Dell Inc.-XPS 13 9343-0310JH

See my previous comments on the kernel patch, but the longname should be
like :-

"Dell Inc.-XPS 13"


> broadwell-rt286-Intel Corp.-Broadwell Client platform-Wilson Beach SDS
> bytcr-rt5640-ASUSTeK COMPUTER INC.-T100TA-T100TA
> bytcr-rt5651-Circuitco-Minnowboard Max D0 PLATFORM-MinnowBoard MAX
> ...
> 
> And user space can define configuration files including fields separated
> by '.' as below:
> broadwell-rt286
> broadwell-rt286.Dell.XPS

Dont need to use a . as the names should be unique.

> bytcr-rt5640
> bytcr-rt5640.ASUS.T100
> bytcr-rt5651.MinnowboardMax
> ...
> 
> When being asked to load configuration file of a card, UCM will try to
> find the card long name from the local machine, and then scan all
> available configuration file names, search every field of config file
> name in the card long name. The more characters match, the higher score
> the file has. Finally, the file with the highest score will be loaded to
> configure the sound card.
> 
> Signed-off-by: Mengdong Lin <mengdong.lin@linux.intel.com>
> 
> diff --git a/src/ucm/parser.c b/src/ucm/parser.c
> index c98373a..ff75da7 100644
> --- a/src/ucm/parser.c
> +++ b/src/ucm/parser.c
> @@ -55,6 +55,9 @@ static const char * const component_dir[] = {
>  	NULL,		/* terminator */
>  };
>  
> +static int filename_filter(const struct dirent *dirent);
> +static int is_component_directory(const char *dir);
> +
>  static int parse_sequence(snd_use_case_mgr_t *uc_mgr,
>  			  struct list_head *base,
>  			  snd_config_t *cfg);
> @@ -1328,6 +1331,210 @@ static int parse_master_file(snd_use_case_mgr_t *uc_mgr, snd_config_t *cfg)
>  	return 0;
>  }
>  
> +/* trim SPACE (0x20) from the string */
> +static void trim_space(char *str)
> +{
> +	int i, j = 0;
> +
> +	for (i = 0; str[i] && i < MAX_FILE; i++) {
> +		if (str[i] != 0x20)
> +			str[j++] = str[i];
> +	}
> +
> +	str[j] = '\0';
> +}
> +
> +/* find and store the card long name if the card is in this machine */
> +static int get_card_long_name(snd_use_case_mgr_t *mgr)
> +{
> +	const char *card_name = mgr->card_name;
> +	snd_ctl_t *handle;
> +	int card, err, dev, idx;
> +	snd_ctl_card_info_t *info;
> +	const char *_name, *_long_name;
> +
> +	snd_ctl_card_info_alloca(&info);
> +
> +	card = -1;
> +	if (snd_card_next(&card) < 0 || card < 0) {
> +		uc_error("no soundcards found...");
> +		return -1;
> +	}
> +
> +	while (card >= 0) {
> +		char name[32];
> +
> +		sprintf(name, "hw:%d", card);
> +		err = snd_ctl_open(&handle, name, 0);
> +		if (err < 0) {
> +			uc_error("control open (%i): %s", card,
> +				 snd_strerror(err));
> +			goto next_card;
> +		}
> +
> +		err = snd_ctl_card_info(handle, info);
> +		if (err < 0) {
> +			uc_error("control hardware info (%i): %s", card,
> +				 snd_strerror(err));
> +			snd_ctl_close(handle);
> +			goto next_card;
> +		}
> +
> +		_name = snd_ctl_card_info_get_name(info);
> +		if (!strncmp(card_name, _name, 32)) {
> +			_long_name = snd_ctl_card_info_get_longname(info);
> +			strncpy(mgr->card_long_name, _long_name,
> +				MAX_CARD_LONG_NAME);
> +			snd_ctl_close(handle);
> +			return 0;
> +		}
> +
> +		snd_ctl_close(handle);
> +next_card:
> +		if (snd_card_next(&card) < 0) {
> +			uc_error("snd_card_next");
> +			break;
> +		}
> +	}
> +
> +	return -1;
> +}
> +
> +/* This function will find the best device-specific configuration file based
> + * on the sound card long name.
> + * Different devices may share the same sound driver and thus the same sound
> + * card name (short name), but they may still need different device-specific
> + * configurations. For user space to differentiate them, kernel drivers may
> + * include the DMI info (vendor, product and board) in the card long name.
> + * And user space can define configuration file names appending DMI keywords
> + * to the card name, like:
> + * bytcr-rt5640.ASUS.T100
> + * bytcr-rt5651.MinnowBoard
> + *
> + * When being asked to load the configuration file for a card, this function
> + * will try to find the card long name from the local machine, and then scan
> + * all available configuration file names, search every field of the config
> + * file name in the card long name. The more characters match, the higher
> + * score the file has. Finally, the file with the highest score will be loaded.
> + */

Id' expect this function to try and open card longname and then card
name if longname is not found. So providing we use the exact same names
as the DMI name we wont need any string formatting or scoring (i.e. file
open("longname/longname.conf") will either succeed or fail)

Liam 

> +static void find_best_config_file(snd_use_case_mgr_t *mgr)
> +{
> +	const char *card_name = mgr->card_name;
> +	char name[MAX_FILE];
> +	char *env = getenv(ALSA_CONFIG_UCM_VAR);
> +	struct dirent **namelist;
> +	char card_long_name[MAX_CARD_LONG_NAME];
> +	char *lpos, *lpos_next, *lname_end;
> +	char *d_name, *dpos, *dpos_next, *d_name_end;
> +	int d_name_len, field_len, long_name_len;
> +	int i, cnt, err;
> +	int score, score_max = 0, d_index = -1;
> +	int  is_first_field;
> +
> +	if (get_card_long_name(mgr) < 0) {
> +		/* cannot get long name, use card name as the conf file name */
> +		strcpy(mgr->conf_file_name, mgr->card_name);
> +		return;
> +	}
> +
> +	strncpy(card_long_name, mgr->card_long_name, MAX_CARD_LONG_NAME);
> +	trim_space(card_long_name); /* trim SPACE for matching */
> +	lname_end = card_long_name + strlen(card_long_name);
> +
> +	/* get list of card directories under /usr/share/alsa/ucm */
> +	snprintf(name, sizeof(name)-1,
> +		"%s", env ? env : ALSA_USE_CASE_DIR);
> +	name[MAX_FILE-1] = '\0';
> +
> +	err = scandir(name, &namelist, filename_filter, alphasort);
> +	if (err < 0) {
> +		err = -errno;
> +		uc_error("error: could not scan directory %s: %s",
> +				name, strerror(-err));
> +		return;
> +	}
> +	cnt = err;
> +
> +	/* scan the card directory names */
> +	for (i = 0; i < cnt; i++) {
> +
> +		/* Skip the directories for component devices */
> +		if (is_component_directory(namelist[i]->d_name))
> +			continue;
> +
> +		score = 0;
> +		lpos = card_long_name;
> +
> +		d_name = namelist[i]->d_name;
> +		d_name_len = strlen(d_name);
> +		d_name_end = d_name + d_name_len;
> +		is_first_field = 1;
> +		dpos = d_name;
> +
> +		/* scan fields in the card directory name, separated by '.' */
> +		while (1) {
> +			dpos_next = strchr(dpos, '.');
> +			if (dpos_next == dpos)	/* start with '.' */
> +				goto next_field;
> +
> +			if (!dpos_next) /* last field */
> +				field_len = d_name_end - dpos;
> +			else
> +				field_len = dpos_next - dpos;
> +
> +			memcpy(name, dpos, field_len);
> +			name[field_len] = '\0';
> +
> +			/* search the field in the given card long name */
> +			lpos_next = strstr(lpos, name);
> +			if (!lpos_next) {
> +				/* first field is the card name, must match */
> +				if (is_first_field)
> +					break;
> +				goto next_field;
> +			} else {
> +				/* The more characters match,
> +				 * the higher the score is.
> +				 */
> +				score += field_len;
> +				if (lpos_next + field_len >= lname_end)
> +					break; /* reach end of long name */
> +
> +				/* Skip the matched field,
> +				 * for searching next field.
> +				 */
> +				lpos = lpos_next + field_len;
> +			}
> +
> +next_field:
> +			if (!dpos_next)
> +				break; /* no more fields */
> +
> +			dpos = dpos_next + 1; /* skip the separator '.' */
> +			is_first_field = 0;
> +			if (dpos >= d_name_end)
> +				break;
> +		}
> +
> +		if (score > score_max) {
> +			score_max = score;
> +			d_index = i;
> +		} else if (score == 0 && score_max > 0) {
> +			/* passed the card directories that can match,
> +			 * since directories are in alphabetical order.
> +			 */
> +			break;
> +		}
> +	}
> +
> +	if (d_index >= 0)
> +		strncpy(mgr->conf_file_name, namelist[d_index]->d_name,
> +			MAX_CARD_LONG_NAME);
> +	else
> +		strncpy(mgr->conf_file_name, mgr->card_name,
> +			MAX_CARD_LONG_NAME);
> +}
> +
>  static int load_master_config(const char *card_name, snd_config_t **cfg)
>  {
>  	char filename[MAX_FILE];
> @@ -1355,7 +1562,9 @@ int uc_mgr_import_master_config(snd_use_case_mgr_t *uc_mgr)
>  	snd_config_t *cfg;
>  	int err;
>  
> -	err = load_master_config(uc_mgr->card_name, &cfg);
> +	find_best_config_file(uc_mgr);
> +
> +	err = load_master_config(uc_mgr->conf_file_name, &cfg);
>  	if (err < 0)
>  		return err;
>  	err = parse_master_file(uc_mgr, cfg);
> diff --git a/src/ucm/ucm_local.h b/src/ucm/ucm_local.h
> index 6d3302f..299a5b9 100644
> --- a/src/ucm/ucm_local.h
> +++ b/src/ucm/ucm_local.h
> @@ -41,6 +41,7 @@
>  #include "use-case.h"
>  
>  #define MAX_FILE		256
> +#define MAX_CARD_LONG_NAME	80
>  #define ALSA_USE_CASE_DIR	ALSA_CONFIG_DIR "/ucm"
>  
>  #define SEQUENCE_ELEMENT_TYPE_CDEV	1
> @@ -190,6 +191,8 @@ struct use_case_verb {
>   */
>  struct snd_use_case_mgr {
>  	char *card_name;
> +	char card_long_name[MAX_CARD_LONG_NAME];
> +	char conf_file_name[MAX_CARD_LONG_NAME];
>  	char *comment;
>  
>  	/* use case verb, devices and modifier configs parsed from files */
Pierre-Louis Bossart Dec. 21, 2016, 2:46 p.m. UTC | #2
On 12/21/16 7:09 AM, mengdong.lin@linux.intel.com wrote:
> From: Mengdong Lin <mengdong.lin@linux.intel.com>
>
> Intel DSP platform drivers are used by many different devices. For user
> space to differentiate them, ASoC machine driver may include the DMI info
> (vendor, product and board) in card long name. Possible card long names
> are:
> broadwell-rt286-Dell Inc.-XPS 13 9343-0310JH
> broadwell-rt286-Intel Corp.-Broadwell Client platform-Wilson Beach SDS
> bytcr-rt5640-ASUSTeK COMPUTER INC.-T100TA-T100TA
> bytcr-rt5651-Circuitco-Minnowboard Max D0 PLATFORM-MinnowBoard MAX
> ...
>
> And user space can define configuration files including fields separated
> by '.' as below:
> broadwell-rt286
> broadwell-rt286.Dell.XPS
> bytcr-rt5640
> bytcr-rt5640.ASUS.T100
> bytcr-rt5651.MinnowboardMax
> ...

It's weird to use a different naming convention in driver and userspace. 
Now the '.' is a separator?

> When being asked to load configuration file of a card, UCM will try to
> find the card long name from the local machine, and then scan all
> available configuration file names, search every field of config file
> name in the card long name. The more characters match, the higher score
> the file has. Finally, the file with the highest score will be loaded to
> configure the sound card.

There are cases where we absolutely don't want to let users use a 
shorter name. T100 is terrible for example, it would work for T100TA, 
T100TAF, T00HA which are completely different platforms.

While the partial matching is elegant, I would err on the side of 
simplicity and keep the filename as reported by the driver. that way 
people just copy/paste the longname name and there are fewer 
opportunities for screwups. Yes the file names would be a tad long but 
we are not using DOS, are we?


>
> Signed-off-by: Mengdong Lin <mengdong.lin@linux.intel.com>
>
> diff --git a/src/ucm/parser.c b/src/ucm/parser.c
> index c98373a..ff75da7 100644
> --- a/src/ucm/parser.c
> +++ b/src/ucm/parser.c
> @@ -55,6 +55,9 @@ static const char * const component_dir[] = {
>  	NULL,		/* terminator */
>  };
>
> +static int filename_filter(const struct dirent *dirent);
> +static int is_component_directory(const char *dir);
> +
>  static int parse_sequence(snd_use_case_mgr_t *uc_mgr,
>  			  struct list_head *base,
>  			  snd_config_t *cfg);
> @@ -1328,6 +1331,210 @@ static int parse_master_file(snd_use_case_mgr_t *uc_mgr, snd_config_t *cfg)
>  	return 0;
>  }
>
> +/* trim SPACE (0x20) from the string */
> +static void trim_space(char *str)
> +{
> +	int i, j = 0;
> +
> +	for (i = 0; str[i] && i < MAX_FILE; i++) {
> +		if (str[i] != 0x20)
> +			str[j++] = str[i];
> +	}
> +
> +	str[j] = '\0';
> +}
> +
> +/* find and store the card long name if the card is in this machine */
> +static int get_card_long_name(snd_use_case_mgr_t *mgr)
> +{
> +	const char *card_name = mgr->card_name;
> +	snd_ctl_t *handle;
> +	int card, err, dev, idx;
> +	snd_ctl_card_info_t *info;
> +	const char *_name, *_long_name;
> +
> +	snd_ctl_card_info_alloca(&info);
> +
> +	card = -1;
> +	if (snd_card_next(&card) < 0 || card < 0) {
> +		uc_error("no soundcards found...");
> +		return -1;
> +	}
> +
> +	while (card >= 0) {
> +		char name[32];
> +
> +		sprintf(name, "hw:%d", card);
> +		err = snd_ctl_open(&handle, name, 0);
> +		if (err < 0) {
> +			uc_error("control open (%i): %s", card,
> +				 snd_strerror(err));
> +			goto next_card;
> +		}
> +
> +		err = snd_ctl_card_info(handle, info);
> +		if (err < 0) {
> +			uc_error("control hardware info (%i): %s", card,
> +				 snd_strerror(err));
> +			snd_ctl_close(handle);
> +			goto next_card;
> +		}
> +
> +		_name = snd_ctl_card_info_get_name(info);
> +		if (!strncmp(card_name, _name, 32)) {
> +			_long_name = snd_ctl_card_info_get_longname(info);
> +			strncpy(mgr->card_long_name, _long_name,
> +				MAX_CARD_LONG_NAME);
> +			snd_ctl_close(handle);
> +			return 0;
> +		}
> +
> +		snd_ctl_close(handle);
> +next_card:
> +		if (snd_card_next(&card) < 0) {
> +			uc_error("snd_card_next");
> +			break;
> +		}
> +	}
> +
> +	return -1;
> +}
> +
> +/* This function will find the best device-specific configuration file based
> + * on the sound card long name.
> + * Different devices may share the same sound driver and thus the same sound
> + * card name (short name), but they may still need different device-specific
> + * configurations. For user space to differentiate them, kernel drivers may
> + * include the DMI info (vendor, product and board) in the card long name.
> + * And user space can define configuration file names appending DMI keywords
> + * to the card name, like:
> + * bytcr-rt5640.ASUS.T100
> + * bytcr-rt5651.MinnowBoard
> + *
> + * When being asked to load the configuration file for a card, this function
> + * will try to find the card long name from the local machine, and then scan
> + * all available configuration file names, search every field of the config
> + * file name in the card long name. The more characters match, the higher
> + * score the file has. Finally, the file with the highest score will be loaded.
> + */
> +static void find_best_config_file(snd_use_case_mgr_t *mgr)
> +{
> +	const char *card_name = mgr->card_name;
> +	char name[MAX_FILE];
> +	char *env = getenv(ALSA_CONFIG_UCM_VAR);
> +	struct dirent **namelist;
> +	char card_long_name[MAX_CARD_LONG_NAME];
> +	char *lpos, *lpos_next, *lname_end;
> +	char *d_name, *dpos, *dpos_next, *d_name_end;
> +	int d_name_len, field_len, long_name_len;
> +	int i, cnt, err;
> +	int score, score_max = 0, d_index = -1;
> +	int  is_first_field;
> +
> +	if (get_card_long_name(mgr) < 0) {
> +		/* cannot get long name, use card name as the conf file name */
> +		strcpy(mgr->conf_file_name, mgr->card_name);
> +		return;
> +	}
> +
> +	strncpy(card_long_name, mgr->card_long_name, MAX_CARD_LONG_NAME);
> +	trim_space(card_long_name); /* trim SPACE for matching */
> +	lname_end = card_long_name + strlen(card_long_name);
> +
> +	/* get list of card directories under /usr/share/alsa/ucm */
> +	snprintf(name, sizeof(name)-1,
> +		"%s", env ? env : ALSA_USE_CASE_DIR);
> +	name[MAX_FILE-1] = '\0';
> +
> +	err = scandir(name, &namelist, filename_filter, alphasort);
> +	if (err < 0) {
> +		err = -errno;
> +		uc_error("error: could not scan directory %s: %s",
> +				name, strerror(-err));
> +		return;
> +	}
> +	cnt = err;
> +
> +	/* scan the card directory names */
> +	for (i = 0; i < cnt; i++) {
> +
> +		/* Skip the directories for component devices */
> +		if (is_component_directory(namelist[i]->d_name))
> +			continue;
> +
> +		score = 0;
> +		lpos = card_long_name;
> +
> +		d_name = namelist[i]->d_name;
> +		d_name_len = strlen(d_name);
> +		d_name_end = d_name + d_name_len;
> +		is_first_field = 1;
> +		dpos = d_name;
> +
> +		/* scan fields in the card directory name, separated by '.' */
> +		while (1) {
> +			dpos_next = strchr(dpos, '.');
> +			if (dpos_next == dpos)	/* start with '.' */
> +				goto next_field;
> +
> +			if (!dpos_next) /* last field */
> +				field_len = d_name_end - dpos;
> +			else
> +				field_len = dpos_next - dpos;
> +
> +			memcpy(name, dpos, field_len);
> +			name[field_len] = '\0';
> +
> +			/* search the field in the given card long name */
> +			lpos_next = strstr(lpos, name);
> +			if (!lpos_next) {
> +				/* first field is the card name, must match */
> +				if (is_first_field)
> +					break;
> +				goto next_field;
> +			} else {
> +				/* The more characters match,
> +				 * the higher the score is.
> +				 */
> +				score += field_len;
> +				if (lpos_next + field_len >= lname_end)
> +					break; /* reach end of long name */
> +
> +				/* Skip the matched field,
> +				 * for searching next field.
> +				 */
> +				lpos = lpos_next + field_len;
> +			}
> +
> +next_field:
> +			if (!dpos_next)
> +				break; /* no more fields */
> +
> +			dpos = dpos_next + 1; /* skip the separator '.' */
> +			is_first_field = 0;
> +			if (dpos >= d_name_end)
> +				break;
> +		}
> +
> +		if (score > score_max) {
> +			score_max = score;
> +			d_index = i;
> +		} else if (score == 0 && score_max > 0) {
> +			/* passed the card directories that can match,
> +			 * since directories are in alphabetical order.
> +			 */
> +			break;
> +		}
> +	}
> +
> +	if (d_index >= 0)
> +		strncpy(mgr->conf_file_name, namelist[d_index]->d_name,
> +			MAX_CARD_LONG_NAME);
> +	else
> +		strncpy(mgr->conf_file_name, mgr->card_name,
> +			MAX_CARD_LONG_NAME);
> +}
> +
>  static int load_master_config(const char *card_name, snd_config_t **cfg)
>  {
>  	char filename[MAX_FILE];
> @@ -1355,7 +1562,9 @@ int uc_mgr_import_master_config(snd_use_case_mgr_t *uc_mgr)
>  	snd_config_t *cfg;
>  	int err;
>
> -	err = load_master_config(uc_mgr->card_name, &cfg);
> +	find_best_config_file(uc_mgr);
> +
> +	err = load_master_config(uc_mgr->conf_file_name, &cfg);
>  	if (err < 0)
>  		return err;
>  	err = parse_master_file(uc_mgr, cfg);
> diff --git a/src/ucm/ucm_local.h b/src/ucm/ucm_local.h
> index 6d3302f..299a5b9 100644
> --- a/src/ucm/ucm_local.h
> +++ b/src/ucm/ucm_local.h
> @@ -41,6 +41,7 @@
>  #include "use-case.h"
>
>  #define MAX_FILE		256
> +#define MAX_CARD_LONG_NAME	80
>  #define ALSA_USE_CASE_DIR	ALSA_CONFIG_DIR "/ucm"
>
>  #define SEQUENCE_ELEMENT_TYPE_CDEV	1
> @@ -190,6 +191,8 @@ struct use_case_verb {
>   */
>  struct snd_use_case_mgr {
>  	char *card_name;
> +	char card_long_name[MAX_CARD_LONG_NAME];
> +	char conf_file_name[MAX_CARD_LONG_NAME];
>  	char *comment;
>
>  	/* use case verb, devices and modifier configs parsed from files */
>
Lin, Mengdong Dec. 21, 2016, 4:12 p.m. UTC | #3
> -----Original Message-----

> From: Liam Girdwood [mailto:liam.r.girdwood@linux.intel.com]

> Sent: Wednesday, December 21, 2016 10:39 PM

> 

> On Wed, 2016-12-21 at 21:09 +0800, mengdong.lin@linux.intel.com wrote:

> > From: Mengdong Lin <mengdong.lin@linux.intel.com>

> >

> > Intel DSP platform drivers are used by many different devices. For

> > user space to differentiate them, ASoC machine driver may include the

> > DMI info (vendor, product and board) in card long name. Possible card

> > long names

> > are:

> > broadwell-rt286-Dell Inc.-XPS 13 9343-0310JH

> 

> See my previous comments on the kernel patch, but the longname should be

> like :-

> 

> "Dell Inc.-XPS 13"


May we keep it? It can make the matching code simpler.
> 

> > broadwell-rt286-Intel Corp.-Broadwell Client platform-Wilson Beach SDS

> > bytcr-rt5640-ASUSTeK COMPUTER INC.-T100TA-T100TA

> > bytcr-rt5651-Circuitco-Minnowboard Max D0 PLATFORM-MinnowBoard

> MAX ...

> >

> > And user space can define configuration files including fields

> > separated by '.' as below:

> > broadwell-rt286

> > broadwell-rt286.Dell.XPS

> 

> Dont need to use a . as the names should be unique.

> 

> > bytcr-rt5640

> > bytcr-rt5640.ASUS.T100

> > bytcr-rt5651.MinnowboardMax

> > ...

> >

> > When being asked to load configuration file of a card, UCM will try to

> > find the card long name from the local machine, and then scan all

> > available configuration file names, search every field of config file

> > name in the card long name. The more characters match, the higher

> > score the file has. Finally, the file with the highest score will be

> > loaded to configure the sound card.

> >

> > Signed-off-by: Mengdong Lin <mengdong.lin@linux.intel.com>

> >

> > diff --git a/src/ucm/parser.c b/src/ucm/parser.c index

> > c98373a..ff75da7 100644

> > --- a/src/ucm/parser.c

> > +++ b/src/ucm/parser.c

> > @@ -55,6 +55,9 @@ static const char * const component_dir[] = {

> >  	NULL,		/* terminator */

> >  };

> >

> > +static int filename_filter(const struct dirent *dirent); static int

> > +is_component_directory(const char *dir);

> > +

> >  static int parse_sequence(snd_use_case_mgr_t *uc_mgr,

> >  			  struct list_head *base,

> >  			  snd_config_t *cfg);

> > @@ -1328,6 +1331,210 @@ static int

> parse_master_file(snd_use_case_mgr_t *uc_mgr, snd_config_t *cfg)

> >  	return 0;

> >  }

> >

> > +/* trim SPACE (0x20) from the string */ static void trim_space(char

> > +*str) {

> > +	int i, j = 0;

> > +

> > +	for (i = 0; str[i] && i < MAX_FILE; i++) {

> > +		if (str[i] != 0x20)

> > +			str[j++] = str[i];

> > +	}

> > +

> > +	str[j] = '\0';

> > +}

> > +

> > +/* find and store the card long name if the card is in this machine

> > +*/ static int get_card_long_name(snd_use_case_mgr_t *mgr) {

> > +	const char *card_name = mgr->card_name;

> > +	snd_ctl_t *handle;

> > +	int card, err, dev, idx;

> > +	snd_ctl_card_info_t *info;

> > +	const char *_name, *_long_name;

> > +

> > +	snd_ctl_card_info_alloca(&info);

> > +

> > +	card = -1;

> > +	if (snd_card_next(&card) < 0 || card < 0) {

> > +		uc_error("no soundcards found...");

> > +		return -1;

> > +	}

> > +

> > +	while (card >= 0) {

> > +		char name[32];

> > +

> > +		sprintf(name, "hw:%d", card);

> > +		err = snd_ctl_open(&handle, name, 0);

> > +		if (err < 0) {

> > +			uc_error("control open (%i): %s", card,

> > +				 snd_strerror(err));

> > +			goto next_card;

> > +		}

> > +

> > +		err = snd_ctl_card_info(handle, info);

> > +		if (err < 0) {

> > +			uc_error("control hardware info (%i): %s", card,

> > +				 snd_strerror(err));

> > +			snd_ctl_close(handle);

> > +			goto next_card;

> > +		}

> > +

> > +		_name = snd_ctl_card_info_get_name(info);

> > +		if (!strncmp(card_name, _name, 32)) {

> > +			_long_name = snd_ctl_card_info_get_longname(info);

> > +			strncpy(mgr->card_long_name, _long_name,

> > +				MAX_CARD_LONG_NAME);

> > +			snd_ctl_close(handle);

> > +			return 0;

> > +		}

> > +

> > +		snd_ctl_close(handle);

> > +next_card:

> > +		if (snd_card_next(&card) < 0) {

> > +			uc_error("snd_card_next");

> > +			break;

> > +		}

> > +	}

> > +

> > +	return -1;

> > +}

> > +

> > +/* This function will find the best device-specific configuration

> > +file based

> > + * on the sound card long name.

> > + * Different devices may share the same sound driver and thus the

> > +same sound

> > + * card name (short name), but they may still need different

> > +device-specific

> > + * configurations. For user space to differentiate them, kernel

> > +drivers may

> > + * include the DMI info (vendor, product and board) in the card long

> name.

> > + * And user space can define configuration file names appending DMI

> > +keywords

> > + * to the card name, like:

> > + * bytcr-rt5640.ASUS.T100

> > + * bytcr-rt5651.MinnowBoard

> > + *

> > + * When being asked to load the configuration file for a card, this

> > +function

> > + * will try to find the card long name from the local machine, and

> > +then scan

> > + * all available configuration file names, search every field of the

> > +config

> > + * file name in the card long name. The more characters match, the

> > +higher

> > + * score the file has. Finally, the file with the highest score will be loaded.

> > + */

> 

> Id' expect this function to try and open card longname and then card name if

> longname is not found. So providing we use the exact same names as the

> DMI name we wont need any string formatting or scoring (i.e. file

> open("longname/longname.conf") will either succeed or fail)

> 

> Liam


I feel we cannot use the exact same names for the configuration file and card long name. The DMI info in the card long name can be lengthy, including '.' , SPACE, meaningless info like "Corp." and sometime duplicated info for some OEMs.I hope developers can just extract key words from the messy DMI info to make a simple configuration file name, and UCM automatically do the matching and give each configuration file a score. Then the file with highest score will be used for the card.

Thanks
Mengdong
Lin, Mengdong Dec. 21, 2016, 4:24 p.m. UTC | #4
> -----Original Message-----
> From: Pierre-Louis Bossart [mailto:pierre-louis.bossart@linux.intel.com]
> Sent: Wednesday, December 21, 2016 10:47 PM
> 
> On 12/21/16 7:09 AM, mengdong.lin@linux.intel.com wrote:
> > From: Mengdong Lin <mengdong.lin@linux.intel.com>
> >
> > Intel DSP platform drivers are used by many different devices. For
> > user space to differentiate them, ASoC machine driver may include the
> > DMI info (vendor, product and board) in card long name. Possible card
> > long names
> > are:
> > broadwell-rt286-Dell Inc.-XPS 13 9343-0310JH broadwell-rt286-Intel
> > Corp.-Broadwell Client platform-Wilson Beach SDS bytcr-rt5640-ASUSTeK
> > COMPUTER INC.-T100TA-T100TA bytcr-rt5651-Circuitco-Minnowboard Max
> D0
> > PLATFORM-MinnowBoard MAX ...
> >
> > And user space can define configuration files including fields
> > separated by '.' as below:
> > broadwell-rt286
> > broadwell-rt286.Dell.XPS
> > bytcr-rt5640
> > bytcr-rt5640.ASUS.T100
> > bytcr-rt5651.MinnowboardMax
> > ...
> 
> It's weird to use a different naming convention in driver and userspace.
> Now the '.' is a separator?

Yes. Now '.' is a separator in user space. The code use it to separate each keyword for matching in the card long name. We can also use '.' as separators in kernel, it will not affect the matching. 

> > When being asked to load configuration file of a card, UCM will try to
> > find the card long name from the local machine, and then scan all
> > available configuration file names, search every field of config file
> > name in the card long name. The more characters match, the higher
> > score the file has. Finally, the file with the highest score will be
> > loaded to configure the sound card.
> 
> There are cases where we absolutely don't want to let users use a shorter
> name. T100 is terrible for example, it would work for T100TA, T100TAF,
> T00HA which are completely different platforms.
> 
> While the partial matching is elegant, I would err on the side of simplicity
> and keep the filename as reported by the driver. that way people just
> copy/paste the longname name and there are fewer opportunities for
> screwups. Yes the file names would be a tad long but we are not using DOS,
> are we?

The file names would be really long and machines have different product name can potentially use the same configuration file for DELL XPS 13 or 15(2014) and DELL XPS 13 or 15 (2015) although they have a slight different number in DMI product and board names. For such products, a simple configuration name as "broadwell-rt286.Dell.XPS" can serve them all.

For the AUS T100 family, we could define different configuration files as below:
bytcr-rt5640.ASUS.T100TA
bytcr-rt5640.ASUS.T100TAF
bytcr-rt5640.ASUS.T100HA

They will have different scores, so UCM can choose the best one.

Thanks
Mengdong
mengdong.lin@linux.intel.com Dec. 22, 2016, 3 a.m. UTC | #5
On 12/22/2016 12:12 AM, Lin, Mengdong wrote:
>> -----Original Message-----
>> From: Liam Girdwood [mailto:liam.r.girdwood@linux.intel.com]
>> Sent: Wednesday, December 21, 2016 10:39 PM
>>
>> On Wed, 2016-12-21 at 21:09 +0800, mengdong.lin@linux.intel.com wrote:
>>> From: Mengdong Lin <mengdong.lin@linux.intel.com>
>>>
>>> Intel DSP platform drivers are used by many different devices. For
>>> user space to differentiate them, ASoC machine driver may include the
>>> DMI info (vendor, product and board) in card long name. Possible card
>>> long names
>>> are:
>>> broadwell-rt286-Dell Inc.-XPS 13 9343-0310JH
>>
>> See my previous comments on the kernel patch, but the longname should be
>> like :-
>>
>> "Dell Inc.-XPS 13"
>
> May we keep it? It can make the matching code simpler.

I'll remove the card name field from the card long name, to remove the 
duplicated info and so save the size of long name. Removing the card 
name  field could save more room for the possible "flavor" field.

If we include the card name in the long name, the maximum length of long 
name I observed is 70 characters, that means we don't have much room 
left to add the "flavor" field if need.

In user space, the UCM configure file names will still have the "card 
name" as the first field, and UCM will check this field with card name 
and other fields with the card long name (DMI-flavor).

>>
>>> broadwell-rt286-Intel Corp.-Broadwell Client platform-Wilson Beach SDS
>>> bytcr-rt5640-ASUSTeK COMPUTER INC.-T100TA-T100TA
>>> bytcr-rt5651-Circuitco-Minnowboard Max D0 PLATFORM-MinnowBoard
>> MAX ...
>>>
>>> And user space can define configuration files including fields
>>> separated by '.' as below:
>>> broadwell-rt286
>>> broadwell-rt286.Dell.XPS
>>
>> Dont need to use a . as the names should be unique.

Users can define configuration file name like 
"cardname.longname/cardname.longname", and the longname can be verbatim 
copy of the card long name. It will have the highest match score.

To avoid unnecessary scan and string matching, I'll revise the function: 
it will try to directly open the card file "cardname.longname". And only 
if this file does not exist, fall back to do the scan and score the 
available card files to find the best match.

Take ASUS T100TA as example: its card name is 'bytcr-rt5640', long name 
is 'ASUSTeK COMPUTER INC.-T100TA-T100TA'. UCM will try to open card file 
"bytcr-rt5640.ASUSTeK COMPUTER INC.-T100TA-T100TA". If the file is not 
found, "bytcr-rt5640.ASUS.T100TA" may match, ... and the last fallback 
is "bytcr-rt5640".

For Dell Broadwell-based XPS family: its card name is "broadwell-rt286", 
long name is "Dell Inc.-XPS xx xxxx-xxxxxx". UCM will try file 
"broadwell-rt286.Dell Inc.-XPS xx xxxx-xxxxxx" at first, and may fall 
back to "broadwell-rt286.Dell.XPS" or "broadwell-rt286" at last.

Thanks
Mengdong


>>
>>> bytcr-rt5640
>>> bytcr-rt5640.ASUS.T100
>>> bytcr-rt5651.MinnowboardMax
>>> ...
>>>
>>> When being asked to load configuration file of a card, UCM will try to
>>> find the card long name from the local machine, and then scan all
>>> available configuration file names, search every field of config file
>>> name in the card long name. The more characters match, the higher
>>> score the file has. Finally, the file with the highest score will be
>>> loaded to configure the sound card.
>>>
>>> Signed-off-by: Mengdong Lin <mengdong.lin@linux.intel.com>
>>>
>>> diff --git a/src/ucm/parser.c b/src/ucm/parser.c index
>>> c98373a..ff75da7 100644
>>> --- a/src/ucm/parser.c
>>> +++ b/src/ucm/parser.c
>>> @@ -55,6 +55,9 @@ static const char * const component_dir[] = {
>>>   	NULL,		/* terminator */
>>>   };
>>>
>>> +static int filename_filter(const struct dirent *dirent); static int
>>> +is_component_directory(const char *dir);
>>> +
>>>   static int parse_sequence(snd_use_case_mgr_t *uc_mgr,
>>>   			  struct list_head *base,
>>>   			  snd_config_t *cfg);
>>> @@ -1328,6 +1331,210 @@ static int
>> parse_master_file(snd_use_case_mgr_t *uc_mgr, snd_config_t *cfg)
>>>   	return 0;
>>>   }
>>>
>>> +/* trim SPACE (0x20) from the string */ static void trim_space(char
>>> +*str) {
>>> +	int i, j = 0;
>>> +
>>> +	for (i = 0; str[i] && i < MAX_FILE; i++) {
>>> +		if (str[i] != 0x20)
>>> +			str[j++] = str[i];
>>> +	}
>>> +
>>> +	str[j] = '\0';
>>> +}
>>> +
>>> +/* find and store the card long name if the card is in this machine
>>> +*/ static int get_card_long_name(snd_use_case_mgr_t *mgr) {
>>> +	const char *card_name = mgr->card_name;
>>> +	snd_ctl_t *handle;
>>> +	int card, err, dev, idx;
>>> +	snd_ctl_card_info_t *info;
>>> +	const char *_name, *_long_name;
>>> +
>>> +	snd_ctl_card_info_alloca(&info);
>>> +
>>> +	card = -1;
>>> +	if (snd_card_next(&card) < 0 || card < 0) {
>>> +		uc_error("no soundcards found...");
>>> +		return -1;
>>> +	}
>>> +
>>> +	while (card >= 0) {
>>> +		char name[32];
>>> +
>>> +		sprintf(name, "hw:%d", card);
>>> +		err = snd_ctl_open(&handle, name, 0);
>>> +		if (err < 0) {
>>> +			uc_error("control open (%i): %s", card,
>>> +				 snd_strerror(err));
>>> +			goto next_card;
>>> +		}
>>> +
>>> +		err = snd_ctl_card_info(handle, info);
>>> +		if (err < 0) {
>>> +			uc_error("control hardware info (%i): %s", card,
>>> +				 snd_strerror(err));
>>> +			snd_ctl_close(handle);
>>> +			goto next_card;
>>> +		}
>>> +
>>> +		_name = snd_ctl_card_info_get_name(info);
>>> +		if (!strncmp(card_name, _name, 32)) {
>>> +			_long_name = snd_ctl_card_info_get_longname(info);
>>> +			strncpy(mgr->card_long_name, _long_name,
>>> +				MAX_CARD_LONG_NAME);
>>> +			snd_ctl_close(handle);
>>> +			return 0;
>>> +		}
>>> +
>>> +		snd_ctl_close(handle);
>>> +next_card:
>>> +		if (snd_card_next(&card) < 0) {
>>> +			uc_error("snd_card_next");
>>> +			break;
>>> +		}
>>> +	}
>>> +
>>> +	return -1;
>>> +}
>>> +
>>> +/* This function will find the best device-specific configuration
>>> +file based
>>> + * on the sound card long name.
>>> + * Different devices may share the same sound driver and thus the
>>> +same sound
>>> + * card name (short name), but they may still need different
>>> +device-specific
>>> + * configurations. For user space to differentiate them, kernel
>>> +drivers may
>>> + * include the DMI info (vendor, product and board) in the card long
>> name.
>>> + * And user space can define configuration file names appending DMI
>>> +keywords
>>> + * to the card name, like:
>>> + * bytcr-rt5640.ASUS.T100
>>> + * bytcr-rt5651.MinnowBoard
>>> + *
>>> + * When being asked to load the configuration file for a card, this
>>> +function
>>> + * will try to find the card long name from the local machine, and
>>> +then scan
>>> + * all available configuration file names, search every field of the
>>> +config
>>> + * file name in the card long name. The more characters match, the
>>> +higher
>>> + * score the file has. Finally, the file with the highest score will be loaded.
>>> + */
>>
>> Id' expect this function to try and open card longname and then card name if
>> longname is not found. So providing we use the exact same names as the
>> DMI name we wont need any string formatting or scoring (i.e. file
>> open("longname/longname.conf") will either succeed or fail)
>>
>> Liam
>
> I feel we cannot use the exact same names for the configuration file and card long name. The DMI info in the card long name can be lengthy, including '.' , SPACE, meaningless info like "Corp." and sometime duplicated info for some OEMs.I hope developers can just extract key words from the messy DMI info to make a simple configuration file name, and UCM automatically do the matching and give each configuration file a score. Then the file with highest score will be used for the card.
>
> Thanks
> Mengdong
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>
Liam Girdwood Dec. 22, 2016, 12:14 p.m. UTC | #6
On Wed, 2016-12-21 at 16:12 +0000, Lin, Mengdong wrote:
> > -----Original Message-----
> > From: Liam Girdwood [mailto:liam.r.girdwood@linux.intel.com]
> > Sent: Wednesday, December 21, 2016 10:39 PM
> > 
> > On Wed, 2016-12-21 at 21:09 +0800, mengdong.lin@linux.intel.com wrote:
> > > From: Mengdong Lin <mengdong.lin@linux.intel.com>
> > >
> > > Intel DSP platform drivers are used by many different devices. For
> > > user space to differentiate them, ASoC machine driver may include the
> > > DMI info (vendor, product and board) in card long name. Possible card
> > > long names
> > > are:
> > > broadwell-rt286-Dell Inc.-XPS 13 9343-0310JH
> > 
> > See my previous comments on the kernel patch, but the longname should be
> > like :-
> > 
> > "Dell Inc.-XPS 13"
> 
> May we keep it? It can make the matching code simpler.

Sorry, dont follow. 

> > 
> > > broadwell-rt286-Intel Corp.-Broadwell Client platform-Wilson Beach SDS
> > > bytcr-rt5640-ASUSTeK COMPUTER INC.-T100TA-T100TA
> > > bytcr-rt5651-Circuitco-Minnowboard Max D0 PLATFORM-MinnowBoard
> > MAX ...
> > >
> > > And user space can define configuration files including fields
> > > separated by '.' as below:
> > > broadwell-rt286
> > > broadwell-rt286.Dell.XPS
> > 
> > Dont need to use a . as the names should be unique.
> > 
> > > bytcr-rt5640
> > > bytcr-rt5640.ASUS.T100
> > > bytcr-rt5651.MinnowboardMax
> > > ...
> > >
> > > When being asked to load configuration file of a card, UCM will try to
> > > find the card long name from the local machine, and then scan all
> > > available configuration file names, search every field of config file
> > > name in the card long name. The more characters match, the higher
> > > score the file has. Finally, the file with the highest score will be
> > > loaded to configure the sound card.
> > >
> > > Signed-off-by: Mengdong Lin <mengdong.lin@linux.intel.com>
> > >
> > > diff --git a/src/ucm/parser.c b/src/ucm/parser.c index
> > > c98373a..ff75da7 100644
> > > --- a/src/ucm/parser.c
> > > +++ b/src/ucm/parser.c
> > > @@ -55,6 +55,9 @@ static const char * const component_dir[] = {
> > >  	NULL,		/* terminator */
> > >  };
> > >
> > > +static int filename_filter(const struct dirent *dirent); static int
> > > +is_component_directory(const char *dir);
> > > +
> > >  static int parse_sequence(snd_use_case_mgr_t *uc_mgr,
> > >  			  struct list_head *base,
> > >  			  snd_config_t *cfg);
> > > @@ -1328,6 +1331,210 @@ static int
> > parse_master_file(snd_use_case_mgr_t *uc_mgr, snd_config_t *cfg)
> > >  	return 0;
> > >  }
> > >
> > > +/* trim SPACE (0x20) from the string */ static void trim_space(char
> > > +*str) {
> > > +	int i, j = 0;
> > > +
> > > +	for (i = 0; str[i] && i < MAX_FILE; i++) {
> > > +		if (str[i] != 0x20)
> > > +			str[j++] = str[i];
> > > +	}
> > > +
> > > +	str[j] = '\0';
> > > +}
> > > +
> > > +/* find and store the card long name if the card is in this machine
> > > +*/ static int get_card_long_name(snd_use_case_mgr_t *mgr) {
> > > +	const char *card_name = mgr->card_name;
> > > +	snd_ctl_t *handle;
> > > +	int card, err, dev, idx;
> > > +	snd_ctl_card_info_t *info;
> > > +	const char *_name, *_long_name;
> > > +
> > > +	snd_ctl_card_info_alloca(&info);
> > > +
> > > +	card = -1;
> > > +	if (snd_card_next(&card) < 0 || card < 0) {
> > > +		uc_error("no soundcards found...");
> > > +		return -1;
> > > +	}
> > > +
> > > +	while (card >= 0) {
> > > +		char name[32];
> > > +
> > > +		sprintf(name, "hw:%d", card);
> > > +		err = snd_ctl_open(&handle, name, 0);
> > > +		if (err < 0) {
> > > +			uc_error("control open (%i): %s", card,
> > > +				 snd_strerror(err));
> > > +			goto next_card;
> > > +		}
> > > +
> > > +		err = snd_ctl_card_info(handle, info);
> > > +		if (err < 0) {
> > > +			uc_error("control hardware info (%i): %s", card,
> > > +				 snd_strerror(err));
> > > +			snd_ctl_close(handle);
> > > +			goto next_card;
> > > +		}
> > > +
> > > +		_name = snd_ctl_card_info_get_name(info);
> > > +		if (!strncmp(card_name, _name, 32)) {
> > > +			_long_name = snd_ctl_card_info_get_longname(info);
> > > +			strncpy(mgr->card_long_name, _long_name,
> > > +				MAX_CARD_LONG_NAME);
> > > +			snd_ctl_close(handle);
> > > +			return 0;
> > > +		}
> > > +
> > > +		snd_ctl_close(handle);
> > > +next_card:
> > > +		if (snd_card_next(&card) < 0) {
> > > +			uc_error("snd_card_next");
> > > +			break;
> > > +		}
> > > +	}
> > > +
> > > +	return -1;
> > > +}
> > > +
> > > +/* This function will find the best device-specific configuration
> > > +file based
> > > + * on the sound card long name.
> > > + * Different devices may share the same sound driver and thus the
> > > +same sound
> > > + * card name (short name), but they may still need different
> > > +device-specific
> > > + * configurations. For user space to differentiate them, kernel
> > > +drivers may
> > > + * include the DMI info (vendor, product and board) in the card long
> > name.
> > > + * And user space can define configuration file names appending DMI
> > > +keywords
> > > + * to the card name, like:
> > > + * bytcr-rt5640.ASUS.T100
> > > + * bytcr-rt5651.MinnowBoard
> > > + *
> > > + * When being asked to load the configuration file for a card, this
> > > +function
> > > + * will try to find the card long name from the local machine, and
> > > +then scan
> > > + * all available configuration file names, search every field of the
> > > +config
> > > + * file name in the card long name. The more characters match, the
> > > +higher
> > > + * score the file has. Finally, the file with the highest score will be loaded.
> > > + */
> > 
> > Id' expect this function to try and open card longname and then card name if
> > longname is not found. So providing we use the exact same names as the
> > DMI name we wont need any string formatting or scoring (i.e. file
> > open("longname/longname.conf") will either succeed or fail)
> > 
> > Liam
> 
> I feel we cannot use the exact same names for the configuration file
> and card long name. The DMI info in the card long name can be lengthy,
> including '.' , SPACE, meaningless info like "Corp." and sometime
> duplicated info for some OEMs.I hope developers can just extract key
> words from the messy DMI info to make a simple configuration file
> name, and UCM automatically do the matching and give each
> configuration file a score. Then the file with highest score will be
> used for the card.

Providing the DMI name does not use invalid filename characters we are
ok. Extracting key words and matching is prone to many problems and
painful to debug.... 

The alternative is to include the exact DMI name in a top level config
file that points to the UCM config.

> 
> Thanks
> Mengdong
Lin, Mengdong Dec. 22, 2016, 2:35 p.m. UTC | #7
> -----Original Message-----

> From: Liam Girdwood [mailto:liam.r.girdwood@linux.intel.com]

> Sent: Thursday, December 22, 2016 8:15 PM

> To: Lin, Mengdong <mengdong.lin@intel.com>

> Cc: mengdong.lin@linux.intel.com; alsa-devel@alsa-project.org;

> broonie@kernel.org; tiwai@suse.de; Koul, Vinod <vinod.koul@intel.com>;

> Bossart, Pierre-louis <pierre-louis.bossart@intel.com>

> Subject: Re: [alsa-devel] [PATCH 1/2] ucm: Automatically load the best config

> file based on the card long name

> 

> On Wed, 2016-12-21 at 16:12 +0000, Lin, Mengdong wrote:

> > > -----Original Message-----

> > > From: Liam Girdwood [mailto:liam.r.girdwood@linux.intel.com]

> > > Sent: Wednesday, December 21, 2016 10:39 PM

> > >

> > > On Wed, 2016-12-21 at 21:09 +0800, mengdong.lin@linux.intel.com

> wrote:

> > > > From: Mengdong Lin <mengdong.lin@linux.intel.com>

> > > >

> > > > Intel DSP platform drivers are used by many different devices. For

> > > > user space to differentiate them, ASoC machine driver may include

> > > > the DMI info (vendor, product and board) in card long name.

> > > > Possible card long names

> > > > are:

> > > > broadwell-rt286-Dell Inc.-XPS 13 9343-0310JH

> > >

> > > See my previous comments on the kernel patch, but the longname

> > > should be like :-

> > >

> > > "Dell Inc.-XPS 13"

> >

> > May we keep it? It can make the matching code simpler.

> 

> Sorry, dont follow.


I'll remove the card name (driver name) from the long name as you suggested, please see my previous mail:
http://mailman.alsa-project.org/pipermail/alsa-devel/2016-December/115989.html

> 

> > >

> > > > broadwell-rt286-Intel Corp.-Broadwell Client platform-Wilson Beach

> > > > SDS bytcr-rt5640-ASUSTeK COMPUTER INC.-T100TA-T100TA

> > > > bytcr-rt5651-Circuitco-Minnowboard Max D0 PLATFORM-MinnowBoard

> > > MAX ...

> > > >

> > > > And user space can define configuration files including fields

> > > > separated by '.' as below:

> > > > broadwell-rt286

> > > > broadwell-rt286.Dell.XPS

> > >

> > > Dont need to use a . as the names should be unique.

> > >

> > > > bytcr-rt5640

> > > > bytcr-rt5640.ASUS.T100

> > > > bytcr-rt5651.MinnowboardMax

> > > > ...

> > > >

> > > > When being asked to load configuration file of a card, UCM will

> > > > try to find the card long name from the local machine, and then

> > > > scan all available configuration file names, search every field of

> > > > config file name in the card long name. The more characters match,

> > > > the higher score the file has. Finally, the file with the highest

> > > > score will be loaded to configure the sound card.

> > > >

> > > > Signed-off-by: Mengdong Lin <mengdong.lin@linux.intel.com>

> > > >

> > > > diff --git a/src/ucm/parser.c b/src/ucm/parser.c index

> > > > c98373a..ff75da7 100644

> > > > --- a/src/ucm/parser.c

> > > > +++ b/src/ucm/parser.c

> > > > @@ -55,6 +55,9 @@ static const char * const component_dir[] = {

> > > >  	NULL,		/* terminator */

> > > >  };

> > > >

> > > > +static int filename_filter(const struct dirent *dirent); static

> > > > +int is_component_directory(const char *dir);

> > > > +

> > > >  static int parse_sequence(snd_use_case_mgr_t *uc_mgr,

> > > >  			  struct list_head *base,

> > > >  			  snd_config_t *cfg);

> > > > @@ -1328,6 +1331,210 @@ static int

> > > parse_master_file(snd_use_case_mgr_t *uc_mgr, snd_config_t *cfg)

> > > >  	return 0;

> > > >  }

> > > >

> > > > +/* trim SPACE (0x20) from the string */ static void

> > > > +trim_space(char

> > > > +*str) {

> > > > +	int i, j = 0;

> > > > +

> > > > +	for (i = 0; str[i] && i < MAX_FILE; i++) {

> > > > +		if (str[i] != 0x20)

> > > > +			str[j++] = str[i];

> > > > +	}

> > > > +

> > > > +	str[j] = '\0';

> > > > +}

> > > > +

> > > > +/* find and store the card long name if the card is in this

> > > > +machine */ static int get_card_long_name(snd_use_case_mgr_t *mgr)

> {

> > > > +	const char *card_name = mgr->card_name;

> > > > +	snd_ctl_t *handle;

> > > > +	int card, err, dev, idx;

> > > > +	snd_ctl_card_info_t *info;

> > > > +	const char *_name, *_long_name;

> > > > +

> > > > +	snd_ctl_card_info_alloca(&info);

> > > > +

> > > > +	card = -1;

> > > > +	if (snd_card_next(&card) < 0 || card < 0) {

> > > > +		uc_error("no soundcards found...");

> > > > +		return -1;

> > > > +	}

> > > > +

> > > > +	while (card >= 0) {

> > > > +		char name[32];

> > > > +

> > > > +		sprintf(name, "hw:%d", card);

> > > > +		err = snd_ctl_open(&handle, name, 0);

> > > > +		if (err < 0) {

> > > > +			uc_error("control open (%i): %s", card,

> > > > +				 snd_strerror(err));

> > > > +			goto next_card;

> > > > +		}

> > > > +

> > > > +		err = snd_ctl_card_info(handle, info);

> > > > +		if (err < 0) {

> > > > +			uc_error("control hardware info (%i): %s", card,

> > > > +				 snd_strerror(err));

> > > > +			snd_ctl_close(handle);

> > > > +			goto next_card;

> > > > +		}

> > > > +

> > > > +		_name = snd_ctl_card_info_get_name(info);

> > > > +		if (!strncmp(card_name, _name, 32)) {

> > > > +			_long_name = snd_ctl_card_info_get_longname(info);

> > > > +			strncpy(mgr->card_long_name, _long_name,

> > > > +				MAX_CARD_LONG_NAME);

> > > > +			snd_ctl_close(handle);

> > > > +			return 0;

> > > > +		}

> > > > +

> > > > +		snd_ctl_close(handle);

> > > > +next_card:

> > > > +		if (snd_card_next(&card) < 0) {

> > > > +			uc_error("snd_card_next");

> > > > +			break;

> > > > +		}

> > > > +	}

> > > > +

> > > > +	return -1;

> > > > +}

> > > > +

> > > > +/* This function will find the best device-specific configuration

> > > > +file based

> > > > + * on the sound card long name.

> > > > + * Different devices may share the same sound driver and thus the

> > > > +same sound

> > > > + * card name (short name), but they may still need different

> > > > +device-specific

> > > > + * configurations. For user space to differentiate them, kernel

> > > > +drivers may

> > > > + * include the DMI info (vendor, product and board) in the card

> > > > +long

> > > name.

> > > > + * And user space can define configuration file names appending

> > > > +DMI keywords

> > > > + * to the card name, like:

> > > > + * bytcr-rt5640.ASUS.T100

> > > > + * bytcr-rt5651.MinnowBoard

> > > > + *

> > > > + * When being asked to load the configuration file for a card,

> > > > +this function

> > > > + * will try to find the card long name from the local machine,

> > > > +and then scan

> > > > + * all available configuration file names, search every field of

> > > > +the config

> > > > + * file name in the card long name. The more characters match,

> > > > +the higher

> > > > + * score the file has. Finally, the file with the highest score will be

> loaded.

> > > > + */

> > >

> > > Id' expect this function to try and open card longname and then card

> > > name if longname is not found. So providing we use the exact same

> > > names as the DMI name we wont need any string formatting or scoring

> > > (i.e. file

> > > open("longname/longname.conf") will either succeed or fail)

> > >

> > > Liam

> >

> > I feel we cannot use the exact same names for the configuration file

> > and card long name. The DMI info in the card long name can be lengthy,

> > including '.' , SPACE, meaningless info like "Corp." and sometime

> > duplicated info for some OEMs.I hope developers can just extract key

> > words from the messy DMI info to make a simple configuration file

> > name, and UCM automatically do the matching and give each

> > configuration file a score. Then the file with highest score will be

> > used for the card.

> 

> Providing the DMI name does not use invalid filename characters we are ok.

> Extracting key words and matching is prone to many problems and painful to

> debug....


May I revise the UCM code like this? It will try to open the file with DMI name at first. And only if that file is not available, fall back to key words matching. 

I hope to keep the possibility that user can use a simple configuration file for a product family, without generating too many card directories and files.

Thanks
Mengdong

> 

> The alternative is to include the exact DMI name in a top level config file

> that points to the UCM config.




Thanks
Mengdong
Liam Girdwood Dec. 22, 2016, 3:35 p.m. UTC | #8
On Thu, 2016-12-22 at 14:35 +0000, Lin, Mengdong wrote:
> > -----Original Message-----
> > From: Liam Girdwood [mailto:liam.r.girdwood@linux.intel.com]
> > Sent: Thursday, December 22, 2016 8:15 PM
> > To: Lin, Mengdong <mengdong.lin@intel.com>
> > Cc: mengdong.lin@linux.intel.com; alsa-devel@alsa-project.org;
> > broonie@kernel.org; tiwai@suse.de; Koul, Vinod <vinod.koul@intel.com>;
> > Bossart, Pierre-louis <pierre-louis.bossart@intel.com>
> > Subject: Re: [alsa-devel] [PATCH 1/2] ucm: Automatically load the best config
> > file based on the card long name
> > 
> > On Wed, 2016-12-21 at 16:12 +0000, Lin, Mengdong wrote:
> > > > -----Original Message-----
> > > > From: Liam Girdwood [mailto:liam.r.girdwood@linux.intel.com]
> > > > Sent: Wednesday, December 21, 2016 10:39 PM
> > > >
> > > > On Wed, 2016-12-21 at 21:09 +0800, mengdong.lin@linux.intel.com
> > wrote:
> > > > > From: Mengdong Lin <mengdong.lin@linux.intel.com>
> > > > >
> > > > > Intel DSP platform drivers are used by many different devices. For
> > > > > user space to differentiate them, ASoC machine driver may include
> > > > > the DMI info (vendor, product and board) in card long name.
> > > > > Possible card long names
> > > > > are:
> > > > > broadwell-rt286-Dell Inc.-XPS 13 9343-0310JH
> > > >
> > > > See my previous comments on the kernel patch, but the longname
> > > > should be like :-
> > > >
> > > > "Dell Inc.-XPS 13"
> > >
> > > May we keep it? It can make the matching code simpler.
> > 
> > Sorry, dont follow.
> 
> I'll remove the card name (driver name) from the long name as you suggested, please see my previous mail:
> http://mailman.alsa-project.org/pipermail/alsa-devel/2016-December/115989.html
> 
> > 
> > > >
> > > > > broadwell-rt286-Intel Corp.-Broadwell Client platform-Wilson Beach
> > > > > SDS bytcr-rt5640-ASUSTeK COMPUTER INC.-T100TA-T100TA
> > > > > bytcr-rt5651-Circuitco-Minnowboard Max D0 PLATFORM-MinnowBoard
> > > > MAX ...
> > > > >
> > > > > And user space can define configuration files including fields
> > > > > separated by '.' as below:
> > > > > broadwell-rt286
> > > > > broadwell-rt286.Dell.XPS
> > > >
> > > > Dont need to use a . as the names should be unique.
> > > >
> > > > > bytcr-rt5640
> > > > > bytcr-rt5640.ASUS.T100
> > > > > bytcr-rt5651.MinnowboardMax
> > > > > ...
> > > > >
> > > > > When being asked to load configuration file of a card, UCM will
> > > > > try to find the card long name from the local machine, and then
> > > > > scan all available configuration file names, search every field of
> > > > > config file name in the card long name. The more characters match,
> > > > > the higher score the file has. Finally, the file with the highest
> > > > > score will be loaded to configure the sound card.
> > > > >
> > > > > Signed-off-by: Mengdong Lin <mengdong.lin@linux.intel.com>
> > > > >
> > > > > diff --git a/src/ucm/parser.c b/src/ucm/parser.c index
> > > > > c98373a..ff75da7 100644
> > > > > --- a/src/ucm/parser.c
> > > > > +++ b/src/ucm/parser.c
> > > > > @@ -55,6 +55,9 @@ static const char * const component_dir[] = {
> > > > >  	NULL,		/* terminator */
> > > > >  };
> > > > >
> > > > > +static int filename_filter(const struct dirent *dirent); static
> > > > > +int is_component_directory(const char *dir);
> > > > > +
> > > > >  static int parse_sequence(snd_use_case_mgr_t *uc_mgr,
> > > > >  			  struct list_head *base,
> > > > >  			  snd_config_t *cfg);
> > > > > @@ -1328,6 +1331,210 @@ static int
> > > > parse_master_file(snd_use_case_mgr_t *uc_mgr, snd_config_t *cfg)
> > > > >  	return 0;
> > > > >  }
> > > > >
> > > > > +/* trim SPACE (0x20) from the string */ static void
> > > > > +trim_space(char
> > > > > +*str) {
> > > > > +	int i, j = 0;
> > > > > +
> > > > > +	for (i = 0; str[i] && i < MAX_FILE; i++) {
> > > > > +		if (str[i] != 0x20)
> > > > > +			str[j++] = str[i];
> > > > > +	}
> > > > > +
> > > > > +	str[j] = '\0';
> > > > > +}
> > > > > +
> > > > > +/* find and store the card long name if the card is in this
> > > > > +machine */ static int get_card_long_name(snd_use_case_mgr_t *mgr)
> > {
> > > > > +	const char *card_name = mgr->card_name;
> > > > > +	snd_ctl_t *handle;
> > > > > +	int card, err, dev, idx;
> > > > > +	snd_ctl_card_info_t *info;
> > > > > +	const char *_name, *_long_name;
> > > > > +
> > > > > +	snd_ctl_card_info_alloca(&info);
> > > > > +
> > > > > +	card = -1;
> > > > > +	if (snd_card_next(&card) < 0 || card < 0) {
> > > > > +		uc_error("no soundcards found...");
> > > > > +		return -1;
> > > > > +	}
> > > > > +
> > > > > +	while (card >= 0) {
> > > > > +		char name[32];
> > > > > +
> > > > > +		sprintf(name, "hw:%d", card);
> > > > > +		err = snd_ctl_open(&handle, name, 0);
> > > > > +		if (err < 0) {
> > > > > +			uc_error("control open (%i): %s", card,
> > > > > +				 snd_strerror(err));
> > > > > +			goto next_card;
> > > > > +		}
> > > > > +
> > > > > +		err = snd_ctl_card_info(handle, info);
> > > > > +		if (err < 0) {
> > > > > +			uc_error("control hardware info (%i): %s", card,
> > > > > +				 snd_strerror(err));
> > > > > +			snd_ctl_close(handle);
> > > > > +			goto next_card;
> > > > > +		}
> > > > > +
> > > > > +		_name = snd_ctl_card_info_get_name(info);
> > > > > +		if (!strncmp(card_name, _name, 32)) {
> > > > > +			_long_name = snd_ctl_card_info_get_longname(info);
> > > > > +			strncpy(mgr->card_long_name, _long_name,
> > > > > +				MAX_CARD_LONG_NAME);
> > > > > +			snd_ctl_close(handle);
> > > > > +			return 0;
> > > > > +		}
> > > > > +
> > > > > +		snd_ctl_close(handle);
> > > > > +next_card:
> > > > > +		if (snd_card_next(&card) < 0) {
> > > > > +			uc_error("snd_card_next");
> > > > > +			break;
> > > > > +		}
> > > > > +	}
> > > > > +
> > > > > +	return -1;
> > > > > +}
> > > > > +
> > > > > +/* This function will find the best device-specific configuration
> > > > > +file based
> > > > > + * on the sound card long name.
> > > > > + * Different devices may share the same sound driver and thus the
> > > > > +same sound
> > > > > + * card name (short name), but they may still need different
> > > > > +device-specific
> > > > > + * configurations. For user space to differentiate them, kernel
> > > > > +drivers may
> > > > > + * include the DMI info (vendor, product and board) in the card
> > > > > +long
> > > > name.
> > > > > + * And user space can define configuration file names appending
> > > > > +DMI keywords
> > > > > + * to the card name, like:
> > > > > + * bytcr-rt5640.ASUS.T100
> > > > > + * bytcr-rt5651.MinnowBoard
> > > > > + *
> > > > > + * When being asked to load the configuration file for a card,
> > > > > +this function
> > > > > + * will try to find the card long name from the local machine,
> > > > > +and then scan
> > > > > + * all available configuration file names, search every field of
> > > > > +the config
> > > > > + * file name in the card long name. The more characters match,
> > > > > +the higher
> > > > > + * score the file has. Finally, the file with the highest score will be
> > loaded.
> > > > > + */
> > > >
> > > > Id' expect this function to try and open card longname and then card
> > > > name if longname is not found. So providing we use the exact same
> > > > names as the DMI name we wont need any string formatting or scoring
> > > > (i.e. file
> > > > open("longname/longname.conf") will either succeed or fail)
> > > >
> > > > Liam
> > >
> > > I feel we cannot use the exact same names for the configuration file
> > > and card long name. The DMI info in the card long name can be lengthy,
> > > including '.' , SPACE, meaningless info like "Corp." and sometime
> > > duplicated info for some OEMs.I hope developers can just extract key
> > > words from the messy DMI info to make a simple configuration file
> > > name, and UCM automatically do the matching and give each
> > > configuration file a score. Then the file with highest score will be
> > > used for the card.
> > 
> > Providing the DMI name does not use invalid filename characters we are ok.
> > Extracting key words and matching is prone to many problems and painful to
> > debug....
> 
> May I revise the UCM code like this? It will try to open the file with
> DMI name at first. And only if that file is not available, fall back
> to key words matching. 
> 
> I hope to keep the possibility that user can use a simple
> configuration file for a product family, without generating too many
> card directories and files.
> 

If the DMI names are too complex for directory names, then you could add
an alias file at the top level that would list the DMI name and the
shortened name i.e.

# DMI name                     # UCM directory name
"Dell blah Corp. XPS, 13 blah" "Dell XPS13 BDW"

So the alias file could be searched for the DMI name, before the legacy
name was tried.

The alias method above also allows for reuse of configs on machines that
are similar, but with different DMI names. So this would also keep the
number of configs low.

Liam


> Thanks
> Mengdong
> 
> > 
> > The alternative is to include the exact DMI name in a top level config file
> > that points to the UCM config.
> 
> 
> 
> Thanks
> Mengdong
mengdong.lin@linux.intel.com Dec. 23, 2016, 8:08 a.m. UTC | #9
On 12/22/2016 11:35 PM, Liam Girdwood wrote:

>>>>>> +/* This function will find the best device-specific configuration
>>>>>> +file based
>>>>>> + * on the sound card long name.
>>>>>> + * Different devices may share the same sound driver and thus the
>>>>>> +same sound
>>>>>> + * card name (short name), but they may still need different
>>>>>> +device-specific
>>>>>> + * configurations. For user space to differentiate them, kernel
>>>>>> +drivers may
>>>>>> + * include the DMI info (vendor, product and board) in the card
>>>>>> +long
>>>>> name.
>>>>>> + * And user space can define configuration file names appending
>>>>>> +DMI keywords
>>>>>> + * to the card name, like:
>>>>>> + * bytcr-rt5640.ASUS.T100
>>>>>> + * bytcr-rt5651.MinnowBoard
>>>>>> + *
>>>>>> + * When being asked to load the configuration file for a card,
>>>>>> +this function
>>>>>> + * will try to find the card long name from the local machine,
>>>>>> +and then scan
>>>>>> + * all available configuration file names, search every field of
>>>>>> +the config
>>>>>> + * file name in the card long name. The more characters match,
>>>>>> +the higher
>>>>>> + * score the file has. Finally, the file with the highest score will be
>>> loaded.
>>>>>> + */
>>>>>
>>>>> Id' expect this function to try and open card longname and then card
>>>>> name if longname is not found. So providing we use the exact same
>>>>> names as the DMI name we wont need any string formatting or scoring
>>>>> (i.e. file
>>>>> open("longname/longname.conf") will either succeed or fail)
>>>>>
>>>>> Liam
>>>>
>>>> I feel we cannot use the exact same names for the configuration file
>>>> and card long name. The DMI info in the card long name can be lengthy,
>>>> including '.' , SPACE, meaningless info like "Corp." and sometime
>>>> duplicated info for some OEMs.I hope developers can just extract key
>>>> words from the messy DMI info to make a simple configuration file
>>>> name, and UCM automatically do the matching and give each
>>>> configuration file a score. Then the file with highest score will be
>>>> used for the card.
>>>
>>> Providing the DMI name does not use invalid filename characters we are ok.
>>> Extracting key words and matching is prone to many problems and painful to
>>> debug....
>>
>> May I revise the UCM code like this? It will try to open the file with
>> DMI name at first. And only if that file is not available, fall back
>> to key words matching.
>>
>> I hope to keep the possibility that user can use a simple
>> configuration file for a product family, without generating too many
>> card directories and files.
>>
>
> If the DMI names are too complex for directory names, then you could add
> an alias file at the top level that would list the DMI name and the
> shortened name i.e.
>
> # DMI name                     # UCM directory name
> "Dell blah Corp. XPS, 13 blah" "Dell XPS13 BDW"
>
> So the alias file could be searched for the DMI name, before the legacy
> name was tried.
>
> The alias method above also allows for reuse of configs on machines that
> are similar, but with different DMI names. So this would also keep the
> number of configs low.
>
> Liam

I'll try to use the alias file. But it seems we need to maintain a lot 
of entries in the file :-(

In addition, I found we cannot use paths with SPACE as the UCM directory 
name. Autoconf cannot support this. It complains although I tried \ or " 
to escape. We need something like "Dell.XPS13.BDW" as the UCM directory 
names.

Thanks
mengdong


>
>> Thanks
>> Mengdong
>>
>>>
>>> The alternative is to include the exact DMI name in a top level config file
>>> that points to the UCM config.
>>
>>
>>
>> Thanks
>> Mengdong
>
>
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>
Liam Girdwood Dec. 23, 2016, 9:07 a.m. UTC | #10
On Fri, 2016-12-23 at 16:08 +0800, Mengdong Lin wrote:
> On 12/22/2016 11:35 PM, Liam Girdwood wrote:
> 
> >>>>>> +/* This function will find the best device-specific configuration
> >>>>>> +file based
> >>>>>> + * on the sound card long name.
> >>>>>> + * Different devices may share the same sound driver and thus the
> >>>>>> +same sound
> >>>>>> + * card name (short name), but they may still need different
> >>>>>> +device-specific
> >>>>>> + * configurations. For user space to differentiate them, kernel
> >>>>>> +drivers may
> >>>>>> + * include the DMI info (vendor, product and board) in the card
> >>>>>> +long
> >>>>> name.
> >>>>>> + * And user space can define configuration file names appending
> >>>>>> +DMI keywords
> >>>>>> + * to the card name, like:
> >>>>>> + * bytcr-rt5640.ASUS.T100
> >>>>>> + * bytcr-rt5651.MinnowBoard
> >>>>>> + *
> >>>>>> + * When being asked to load the configuration file for a card,
> >>>>>> +this function
> >>>>>> + * will try to find the card long name from the local machine,
> >>>>>> +and then scan
> >>>>>> + * all available configuration file names, search every field of
> >>>>>> +the config
> >>>>>> + * file name in the card long name. The more characters match,
> >>>>>> +the higher
> >>>>>> + * score the file has. Finally, the file with the highest score will be
> >>> loaded.
> >>>>>> + */
> >>>>>
> >>>>> Id' expect this function to try and open card longname and then card
> >>>>> name if longname is not found. So providing we use the exact same
> >>>>> names as the DMI name we wont need any string formatting or scoring
> >>>>> (i.e. file
> >>>>> open("longname/longname.conf") will either succeed or fail)
> >>>>>
> >>>>> Liam
> >>>>
> >>>> I feel we cannot use the exact same names for the configuration file
> >>>> and card long name. The DMI info in the card long name can be lengthy,
> >>>> including '.' , SPACE, meaningless info like "Corp." and sometime
> >>>> duplicated info for some OEMs.I hope developers can just extract key
> >>>> words from the messy DMI info to make a simple configuration file
> >>>> name, and UCM automatically do the matching and give each
> >>>> configuration file a score. Then the file with highest score will be
> >>>> used for the card.
> >>>
> >>> Providing the DMI name does not use invalid filename characters we are ok.
> >>> Extracting key words and matching is prone to many problems and painful to
> >>> debug....
> >>
> >> May I revise the UCM code like this? It will try to open the file with
> >> DMI name at first. And only if that file is not available, fall back
> >> to key words matching.
> >>
> >> I hope to keep the possibility that user can use a simple
> >> configuration file for a product family, without generating too many
> >> card directories and files.
> >>
> >
> > If the DMI names are too complex for directory names, then you could add
> > an alias file at the top level that would list the DMI name and the
> > shortened name i.e.
> >
> > # DMI name                     # UCM directory name
> > "Dell blah Corp. XPS, 13 blah" "Dell XPS13 BDW"
> >
> > So the alias file could be searched for the DMI name, before the legacy
> > name was tried.
> >
> > The alias method above also allows for reuse of configs on machines that
> > are similar, but with different DMI names. So this would also keep the
> > number of configs low.
> >
> > Liam
> 
> I'll try to use the alias file. But it seems we need to maintain a lot 
> of entries in the file :-(
> 
> In addition, I found we cannot use paths with SPACE as the UCM directory 
> name. Autoconf cannot support this. It complains although I tried \ or " 
> to escape. We need something like "Dell.XPS13.BDW" as the UCM directory 
> names.
> 

Gah, it looks like autoconf may have defeated this ! Autoconf has
special uses for some chars in names.

So you may want to "fix" the DMI name to exclude any special chars when
creating your longname (maybe also set a shorter length too). I still
dont want any scoring, we just want to open the "longname.conf" then
"drivername.conf"

Liam
Lin, Mengdong Dec. 23, 2016, 12:56 p.m. UTC | #11
> -----Original Message-----

> From: Liam Girdwood [mailto:liam.r.girdwood@linux.intel.com]

> Sent: Friday, December 23, 2016 5:08 PM

> To: Mengdong Lin <mengdong.lin@linux.intel.com>

> Cc: Lin, Mengdong <mengdong.lin@intel.com>; alsa-devel@alsa-project.org;

> tiwai@suse.de; Koul, Vinod <vinod.koul@intel.com>; broonie@kernel.org;

> Bossart, Pierre-louis <pierre-louis.bossart@intel.com>

> Subject: Re: [alsa-devel] [PATCH 1/2] ucm: Automatically load the best config

> file based on the card long name

> 

> On Fri, 2016-12-23 at 16:08 +0800, Mengdong Lin wrote:

> > On 12/22/2016 11:35 PM, Liam Girdwood wrote:

> >

> > >>>>>> +/* This function will find the best device-specific

> > >>>>>> +configuration file based

> > >>>>>> + * on the sound card long name.

> > >>>>>> + * Different devices may share the same sound driver and thus

> > >>>>>> +the same sound

> > >>>>>> + * card name (short name), but they may still need different

> > >>>>>> +device-specific

> > >>>>>> + * configurations. For user space to differentiate them,

> > >>>>>> +kernel drivers may

> > >>>>>> + * include the DMI info (vendor, product and board) in the

> > >>>>>> +card long

> > >>>>> name.

> > >>>>>> + * And user space can define configuration file names

> > >>>>>> +appending DMI keywords

> > >>>>>> + * to the card name, like:

> > >>>>>> + * bytcr-rt5640.ASUS.T100

> > >>>>>> + * bytcr-rt5651.MinnowBoard

> > >>>>>> + *

> > >>>>>> + * When being asked to load the configuration file for a card,

> > >>>>>> +this function

> > >>>>>> + * will try to find the card long name from the local machine,

> > >>>>>> +and then scan

> > >>>>>> + * all available configuration file names, search every field

> > >>>>>> +of the config

> > >>>>>> + * file name in the card long name. The more characters match,

> > >>>>>> +the higher

> > >>>>>> + * score the file has. Finally, the file with the highest

> > >>>>>> +score will be

> > >>> loaded.

> > >>>>>> + */

> > >>>>>

> > >>>>> Id' expect this function to try and open card longname and then

> > >>>>> card name if longname is not found. So providing we use the

> > >>>>> exact same names as the DMI name we wont need any string

> > >>>>> formatting or scoring (i.e. file

> > >>>>> open("longname/longname.conf") will either succeed or fail)

> > >>>>>

> > >>>>> Liam

> > >>>>

> > >>>> I feel we cannot use the exact same names for the configuration

> > >>>> file and card long name. The DMI info in the card long name can

> > >>>> be lengthy, including '.' , SPACE, meaningless info like "Corp."

> > >>>> and sometime duplicated info for some OEMs.I hope developers

> can

> > >>>> just extract key words from the messy DMI info to make a simple

> > >>>> configuration file name, and UCM automatically do the matching

> > >>>> and give each configuration file a score. Then the file with

> > >>>> highest score will be used for the card.

> > >>>

> > >>> Providing the DMI name does not use invalid filename characters we

> are ok.

> > >>> Extracting key words and matching is prone to many problems and

> > >>> painful to debug....

> > >>

> > >> May I revise the UCM code like this? It will try to open the file

> > >> with DMI name at first. And only if that file is not available,

> > >> fall back to key words matching.

> > >>

> > >> I hope to keep the possibility that user can use a simple

> > >> configuration file for a product family, without generating too

> > >> many card directories and files.

> > >>

> > >

> > > If the DMI names are too complex for directory names, then you could

> > > add an alias file at the top level that would list the DMI name and

> > > the shortened name i.e.

> > >

> > > # DMI name                     # UCM directory name

> > > "Dell blah Corp. XPS, 13 blah" "Dell XPS13 BDW"

> > >

> > > So the alias file could be searched for the DMI name, before the

> > > legacy name was tried.

> > >

> > > The alias method above also allows for reuse of configs on machines

> > > that are similar, but with different DMI names. So this would also

> > > keep the number of configs low.

> > >

> > > Liam

> >

> > I'll try to use the alias file. But it seems we need to maintain a lot

> > of entries in the file :-(

> >

> > In addition, I found we cannot use paths with SPACE as the UCM

> > directory name. Autoconf cannot support this. It complains although I tried

> \ or "

> > to escape. We need something like "Dell.XPS13.BDW" as the UCM

> > directory names.

> >

> 

> Gah, it looks like autoconf may have defeated this ! Autoconf has

> special uses for some chars in names.

> 

> So you may want to "fix" the DMI name to exclude any special chars when

> creating your longname (maybe also set a shorter length too). I still

> dont want any scoring, we just want to open the "longname.conf" then

> "drivername.conf"

> 

> Liam

> 


Okay. I'll remove the special chars from the card long name and make UCM code simpler as you suggested :-)

Thanks
Mengdong
diff mbox

Patch

diff --git a/src/ucm/parser.c b/src/ucm/parser.c
index c98373a..ff75da7 100644
--- a/src/ucm/parser.c
+++ b/src/ucm/parser.c
@@ -55,6 +55,9 @@  static const char * const component_dir[] = {
 	NULL,		/* terminator */
 };
 
+static int filename_filter(const struct dirent *dirent);
+static int is_component_directory(const char *dir);
+
 static int parse_sequence(snd_use_case_mgr_t *uc_mgr,
 			  struct list_head *base,
 			  snd_config_t *cfg);
@@ -1328,6 +1331,210 @@  static int parse_master_file(snd_use_case_mgr_t *uc_mgr, snd_config_t *cfg)
 	return 0;
 }
 
+/* trim SPACE (0x20) from the string */
+static void trim_space(char *str)
+{
+	int i, j = 0;
+
+	for (i = 0; str[i] && i < MAX_FILE; i++) {
+		if (str[i] != 0x20)
+			str[j++] = str[i];
+	}
+
+	str[j] = '\0';
+}
+
+/* find and store the card long name if the card is in this machine */
+static int get_card_long_name(snd_use_case_mgr_t *mgr)
+{
+	const char *card_name = mgr->card_name;
+	snd_ctl_t *handle;
+	int card, err, dev, idx;
+	snd_ctl_card_info_t *info;
+	const char *_name, *_long_name;
+
+	snd_ctl_card_info_alloca(&info);
+
+	card = -1;
+	if (snd_card_next(&card) < 0 || card < 0) {
+		uc_error("no soundcards found...");
+		return -1;
+	}
+
+	while (card >= 0) {
+		char name[32];
+
+		sprintf(name, "hw:%d", card);
+		err = snd_ctl_open(&handle, name, 0);
+		if (err < 0) {
+			uc_error("control open (%i): %s", card,
+				 snd_strerror(err));
+			goto next_card;
+		}
+
+		err = snd_ctl_card_info(handle, info);
+		if (err < 0) {
+			uc_error("control hardware info (%i): %s", card,
+				 snd_strerror(err));
+			snd_ctl_close(handle);
+			goto next_card;
+		}
+
+		_name = snd_ctl_card_info_get_name(info);
+		if (!strncmp(card_name, _name, 32)) {
+			_long_name = snd_ctl_card_info_get_longname(info);
+			strncpy(mgr->card_long_name, _long_name,
+				MAX_CARD_LONG_NAME);
+			snd_ctl_close(handle);
+			return 0;
+		}
+
+		snd_ctl_close(handle);
+next_card:
+		if (snd_card_next(&card) < 0) {
+			uc_error("snd_card_next");
+			break;
+		}
+	}
+
+	return -1;
+}
+
+/* This function will find the best device-specific configuration file based
+ * on the sound card long name.
+ * Different devices may share the same sound driver and thus the same sound
+ * card name (short name), but they may still need different device-specific
+ * configurations. For user space to differentiate them, kernel drivers may
+ * include the DMI info (vendor, product and board) in the card long name.
+ * And user space can define configuration file names appending DMI keywords
+ * to the card name, like:
+ * bytcr-rt5640.ASUS.T100
+ * bytcr-rt5651.MinnowBoard
+ *
+ * When being asked to load the configuration file for a card, this function
+ * will try to find the card long name from the local machine, and then scan
+ * all available configuration file names, search every field of the config
+ * file name in the card long name. The more characters match, the higher
+ * score the file has. Finally, the file with the highest score will be loaded.
+ */
+static void find_best_config_file(snd_use_case_mgr_t *mgr)
+{
+	const char *card_name = mgr->card_name;
+	char name[MAX_FILE];
+	char *env = getenv(ALSA_CONFIG_UCM_VAR);
+	struct dirent **namelist;
+	char card_long_name[MAX_CARD_LONG_NAME];
+	char *lpos, *lpos_next, *lname_end;
+	char *d_name, *dpos, *dpos_next, *d_name_end;
+	int d_name_len, field_len, long_name_len;
+	int i, cnt, err;
+	int score, score_max = 0, d_index = -1;
+	int  is_first_field;
+
+	if (get_card_long_name(mgr) < 0) {
+		/* cannot get long name, use card name as the conf file name */
+		strcpy(mgr->conf_file_name, mgr->card_name);
+		return;
+	}
+
+	strncpy(card_long_name, mgr->card_long_name, MAX_CARD_LONG_NAME);
+	trim_space(card_long_name); /* trim SPACE for matching */
+	lname_end = card_long_name + strlen(card_long_name);
+
+	/* get list of card directories under /usr/share/alsa/ucm */
+	snprintf(name, sizeof(name)-1,
+		"%s", env ? env : ALSA_USE_CASE_DIR);
+	name[MAX_FILE-1] = '\0';
+
+	err = scandir(name, &namelist, filename_filter, alphasort);
+	if (err < 0) {
+		err = -errno;
+		uc_error("error: could not scan directory %s: %s",
+				name, strerror(-err));
+		return;
+	}
+	cnt = err;
+
+	/* scan the card directory names */
+	for (i = 0; i < cnt; i++) {
+
+		/* Skip the directories for component devices */
+		if (is_component_directory(namelist[i]->d_name))
+			continue;
+
+		score = 0;
+		lpos = card_long_name;
+
+		d_name = namelist[i]->d_name;
+		d_name_len = strlen(d_name);
+		d_name_end = d_name + d_name_len;
+		is_first_field = 1;
+		dpos = d_name;
+
+		/* scan fields in the card directory name, separated by '.' */
+		while (1) {
+			dpos_next = strchr(dpos, '.');
+			if (dpos_next == dpos)	/* start with '.' */
+				goto next_field;
+
+			if (!dpos_next) /* last field */
+				field_len = d_name_end - dpos;
+			else
+				field_len = dpos_next - dpos;
+
+			memcpy(name, dpos, field_len);
+			name[field_len] = '\0';
+
+			/* search the field in the given card long name */
+			lpos_next = strstr(lpos, name);
+			if (!lpos_next) {
+				/* first field is the card name, must match */
+				if (is_first_field)
+					break;
+				goto next_field;
+			} else {
+				/* The more characters match,
+				 * the higher the score is.
+				 */
+				score += field_len;
+				if (lpos_next + field_len >= lname_end)
+					break; /* reach end of long name */
+
+				/* Skip the matched field,
+				 * for searching next field.
+				 */
+				lpos = lpos_next + field_len;
+			}
+
+next_field:
+			if (!dpos_next)
+				break; /* no more fields */
+
+			dpos = dpos_next + 1; /* skip the separator '.' */
+			is_first_field = 0;
+			if (dpos >= d_name_end)
+				break;
+		}
+
+		if (score > score_max) {
+			score_max = score;
+			d_index = i;
+		} else if (score == 0 && score_max > 0) {
+			/* passed the card directories that can match,
+			 * since directories are in alphabetical order.
+			 */
+			break;
+		}
+	}
+
+	if (d_index >= 0)
+		strncpy(mgr->conf_file_name, namelist[d_index]->d_name,
+			MAX_CARD_LONG_NAME);
+	else
+		strncpy(mgr->conf_file_name, mgr->card_name,
+			MAX_CARD_LONG_NAME);
+}
+
 static int load_master_config(const char *card_name, snd_config_t **cfg)
 {
 	char filename[MAX_FILE];
@@ -1355,7 +1562,9 @@  int uc_mgr_import_master_config(snd_use_case_mgr_t *uc_mgr)
 	snd_config_t *cfg;
 	int err;
 
-	err = load_master_config(uc_mgr->card_name, &cfg);
+	find_best_config_file(uc_mgr);
+
+	err = load_master_config(uc_mgr->conf_file_name, &cfg);
 	if (err < 0)
 		return err;
 	err = parse_master_file(uc_mgr, cfg);
diff --git a/src/ucm/ucm_local.h b/src/ucm/ucm_local.h
index 6d3302f..299a5b9 100644
--- a/src/ucm/ucm_local.h
+++ b/src/ucm/ucm_local.h
@@ -41,6 +41,7 @@ 
 #include "use-case.h"
 
 #define MAX_FILE		256
+#define MAX_CARD_LONG_NAME	80
 #define ALSA_USE_CASE_DIR	ALSA_CONFIG_DIR "/ucm"
 
 #define SEQUENCE_ELEMENT_TYPE_CDEV	1
@@ -190,6 +191,8 @@  struct use_case_verb {
  */
 struct snd_use_case_mgr {
 	char *card_name;
+	char card_long_name[MAX_CARD_LONG_NAME];
+	char conf_file_name[MAX_CARD_LONG_NAME];
 	char *comment;
 
 	/* use case verb, devices and modifier configs parsed from files */