[alsa-devel] [PATCH V6 0/2] ASoC: Add core API to register and cleanup DMI names for card
From: "Lu, Han" han.lu@intel.com
Share more product information, for user space utils such as PA and UCM to distinguish different products. 1. Add core APIs to register and cleanup DMI names for card. 2. Apply the APIs to bytcr-rt5640 driver.
changes on V6: 1. Use dynamic allocate and cleanup for card long name 2. Remove unneccessary arguments to simplify the API
changes on V5: 1. Use independent space to store card long_name, to avoid irrelavant info sharing from card component 2. Use letter ';' instead of ':' to separate strings in long name, in case name strings may also contain ':' and confuse user 3. Fix error that vendor name and firmware name were not optional
changes on V4: 1. Replace kmalloc() and snprintf() with ksaprintf() to simplify code
changes on V3: 1. Split the core API and the API call to two patches 2. Replace misused strcat() with snprintf() 3. Code and comment fix
Lu, Han (2): ASoC: core: add API for registering and cleaning up DMI card names ASoC: bytcr-rt5640: register DMI names for card
include/sound/soc.h | 3 ++ sound/soc/intel/boards/bytcr_rt5640.c | 18 ++++++++ sound/soc/soc-core.c | 87 +++++++++++++++++++++++++++++++++++ 3 files changed, 108 insertions(+)
From: "Lu, Han" han.lu@intel.com
Add core API for registering and cleaning up DMI card names, so user space utils such as PA and UCM can distinguish various products. Previously on ASoC, the card short name, driver name and long name are all the same as the machine driver name. The patch adds more board information: card driver name ---> machine driver name card short name ---> DMI_BOARD_NAME or DMI_PRODUCT_NAME card long name and card component ---> short name;driver name;(DMI_SYS_VENDOR, optional);(the firmware name, optional)
Signed-off-by: Lu, Han han.lu@intel.com
diff --git a/include/sound/soc.h b/include/sound/soc.h index 02b4a21..911d09e 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -486,6 +486,9 @@ void snd_soc_runtime_deactivate(struct snd_soc_pcm_runtime *rtd, int stream); int snd_soc_runtime_set_dai_fmt(struct snd_soc_pcm_runtime *rtd, unsigned int dai_fmt);
+int snd_soc_set_card_names(struct snd_soc_card *card, const char *firmware); +void snd_soc_cleanup_card_names(struct snd_soc_card *card); + /* Utility functions to get clock rates from various things */ int snd_soc_calc_frame_size(int sample_size, int channels, int tdm_slots); int snd_soc_params_to_frame_size(struct snd_pcm_hw_params *params); diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index d2e62b15..0cfffff 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -34,6 +34,7 @@ #include <linux/ctype.h> #include <linux/slab.h> #include <linux/of.h> +#include <linux/dmi.h> #include <sound/core.h> #include <sound/jack.h> #include <sound/pcm.h> @@ -1828,6 +1829,92 @@ int snd_soc_runtime_set_dai_fmt(struct snd_soc_pcm_runtime *rtd, } EXPORT_SYMBOL_GPL(snd_soc_runtime_set_dai_fmt);
+/** + * snd_soc_set_card_names() - Register DMI names to card + * @card: The card to register DMI names + * @board: DMI_BOARD_NAME or DMI_PRODUCT_NAME + * @vendor: DMI_SYS_VENDOR, optional + * @firmware: The firmware name, optional + * + * This function registers DMI names to card for the userspace to distinguish + * different boards/products: + * card driver name ---> machine driver name + * card short name ---> DMI_BOARD_NAME or DMI_PRODUCT_NAME + * card long name and + * card component ---> short name;driver name;(DMI_SYS_VENDOR, optional) + * ;(firmware name, optional) + * + * Returns 0 on success, otherwise a negative error code. + */ +int snd_soc_set_card_names(struct snd_soc_card *card, const char *firmware) +{ + int ret = 0; + size_t buf_size, name_size; + const char *board, *vendor; + char *longname; + + board = dmi_get_system_info(DMI_BOARD_NAME); + if (!board) + board = dmi_get_system_info(DMI_PRODUCT_NAME); + if (!board) { + dev_err(card->dev, "ASoC: the board/product name is empty!\n"); + return -EINVAL; + } + + vendor = dmi_get_system_info(DMI_SYS_VENDOR); + + /* card driver name */ + card->driver_name = card->name; + + /* card short name */ + card->name = board; + + /* card long name */ + buf_size = sizeof(card->snd_card->longname); + name_size = strlen(card->name) + strlen(card->driver_name) + 4; + if (vendor) + name_size += strlen(vendor); + if (firmware) + name_size += strlen(firmware); + if (name_size > buf_size) + return -ENOMEM; + + longname = kmalloc(buf_size, GFP_KERNEL); + if (!longname) + return -ENOMEM; + snprintf(longname, buf_size, "%s;%s;", + card->name, card->driver_name); + if (vendor) + strlcat(longname, vendor, buf_size); + strlcat(longname, ";", buf_size); + if (firmware) + strlcat(longname, firmware, buf_size); + + card->long_name = longname; + + /* card component */ + if (sizeof(card->snd_card->components) < name_size + + strlen(card->snd_card->components)) + return -ENOMEM; + + ret = snd_component_add(card->snd_card, card->long_name); + + return ret; +} +EXPORT_SYMBOL_GPL(snd_soc_set_card_names); + +/** + * snd_soc_cleanup_card_names() - cleanup registered DMI names + * @card: The card to cleanup + * + * This function cleanup the registered DMI names from card + */ +void snd_soc_cleanup_card_names(struct snd_soc_card *card) +{ + kfree(card->long_name); +} +EXPORT_SYMBOL_GPL(snd_soc_cleanup_card_names); + static int snd_soc_instantiate_card(struct snd_soc_card *card) { struct snd_soc_codec *codec;
Hi,
On Apr 6 2016 13:43, han.lu@intel.com wrote:
From: "Lu, Han" han.lu@intel.com
Add core API for registering and cleaning up DMI card names, so user space utils such as PA and UCM can distinguish various products. Previously on ASoC, the card short name, driver name and long name are all the same as the machine driver name. The patch adds more board information: card driver name ---> machine driver name card short name ---> DMI_BOARD_NAME or DMI_PRODUCT_NAME card long name and card component ---> short name;driver name;(DMI_SYS_VENDOR, optional);(the firmware name, optional)
Signed-off-by: Lu, Han han.lu@intel.com
diff --git a/include/sound/soc.h b/include/sound/soc.h index 02b4a21..911d09e 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -486,6 +486,9 @@ void snd_soc_runtime_deactivate(struct snd_soc_pcm_runtime *rtd, int stream); int snd_soc_runtime_set_dai_fmt(struct snd_soc_pcm_runtime *rtd, unsigned int dai_fmt);
+int snd_soc_set_card_names(struct snd_soc_card *card, const char *firmware); +void snd_soc_cleanup_card_names(struct snd_soc_card *card);
- /* Utility functions to get clock rates from various things */ int snd_soc_calc_frame_size(int sample_size, int channels, int tdm_slots); int snd_soc_params_to_frame_size(struct snd_pcm_hw_params *params);
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index d2e62b15..0cfffff 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -34,6 +34,7 @@ #include <linux/ctype.h> #include <linux/slab.h> #include <linux/of.h> +#include <linux/dmi.h> #include <sound/core.h> #include <sound/jack.h> #include <sound/pcm.h> @@ -1828,6 +1829,92 @@ int snd_soc_runtime_set_dai_fmt(struct snd_soc_pcm_runtime *rtd, } EXPORT_SYMBOL_GPL(snd_soc_runtime_set_dai_fmt);
+/**
- snd_soc_set_card_names() - Register DMI names to card
- @card: The card to register DMI names
- @board: DMI_BOARD_NAME or DMI_PRODUCT_NAME
- @vendor: DMI_SYS_VENDOR, optional
- @firmware: The firmware name, optional
- This function registers DMI names to card for the userspace to distinguish
- different boards/products:
- card driver name ---> machine driver name
- card short name ---> DMI_BOARD_NAME or DMI_PRODUCT_NAME
- card long name and
- card component ---> short name;driver name;(DMI_SYS_VENDOR, optional)
;(firmware name, optional)
- Returns 0 on success, otherwise a negative error code.
- */
+int snd_soc_set_card_names(struct snd_soc_card *card, const char *firmware) +{
- int ret = 0;
- size_t buf_size, name_size;
- const char *board, *vendor;
- char *longname;
- board = dmi_get_system_info(DMI_BOARD_NAME);
- if (!board)
board = dmi_get_system_info(DMI_PRODUCT_NAME);- if (!board) {
dev_err(card->dev, "ASoC: the board/product name is empty!\n");return -EINVAL;- }
- vendor = dmi_get_system_info(DMI_SYS_VENDOR);
- /* card driver name */
- card->driver_name = card->name;
- /* card short name */
- card->name = board;
- /* card long name */
- buf_size = sizeof(card->snd_card->longname);
- name_size = strlen(card->name) + strlen(card->driver_name) + 4;
- if (vendor)
name_size += strlen(vendor);- if (firmware)
name_size += strlen(firmware);- if (name_size > buf_size)
return -ENOMEM;- longname = kmalloc(buf_size, GFP_KERNEL);
- if (!longname)
return -ENOMEM;- snprintf(longname, buf_size, "%s;%s;",
card->name, card->driver_name);- if (vendor)
strlcat(longname, vendor, buf_size);- strlcat(longname, ";", buf_size);
- if (firmware)
strlcat(longname, firmware, buf_size);- card->long_name = longname;
- /* card component */
- if (sizeof(card->snd_card->components) < name_size
+ strlen(card->snd_card->components))return -ENOMEM;- ret = snd_component_add(card->snd_card, card->long_name);
- return ret;
+} +EXPORT_SYMBOL_GPL(snd_soc_set_card_names);
+/**
- snd_soc_cleanup_card_names() - cleanup registered DMI names
- @card: The card to cleanup
- This function cleanup the registered DMI names from card
- */
+void snd_soc_cleanup_card_names(struct snd_soc_card *card) +{
- kfree(card->long_name);
+} +EXPORT_SYMBOL_GPL(snd_soc_cleanup_card_names);
Hm. How about do it in a path to release devres? In short,
When registering card instance, devm_card_release is assigned to the resource. devm_snd_soc_register_card() ->devres_alloc(devm_card_release, sizeof(*ptr), GFP_KERNEL);
When releasing the resource: (sound/soc/soc-devres.c) ->devm_card_release() (sound/soc/soc-core.c) ->snd_soc_unregister_card() ->kfree(card->long_name) (yours)
I just have a quick glance. It's better for you to confirm it in your case.
Regards
Takashi Sakamoto
static int snd_soc_instantiate_card(struct snd_soc_card *card) { struct snd_soc_codec *codec;
Hi,
On 04/06/2016 02:10 PM, Takashi Sakamoto wrote:
Hi,
On Apr 6 2016 13:43, han.lu@intel.com wrote:
From: "Lu, Han" han.lu@intel.com
Add core API for registering and cleaning up DMI card names, so user space utils such as PA and UCM can distinguish various products. Previously on ASoC, the card short name, driver name and long name are all the same as the machine driver name. The patch adds more board information: card driver name ---> machine driver name card short name ---> DMI_BOARD_NAME or DMI_PRODUCT_NAME card long name and card component ---> short name;driver name;(DMI_SYS_VENDOR, optional);(the firmware name, optional)
Signed-off-by: Lu, Han han.lu@intel.com
diff --git a/include/sound/soc.h b/include/sound/soc.h index 02b4a21..911d09e 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -486,6 +486,9 @@ void snd_soc_runtime_deactivate(struct snd_soc_pcm_runtime *rtd, int stream); int snd_soc_runtime_set_dai_fmt(struct snd_soc_pcm_runtime *rtd, unsigned int dai_fmt);
+int snd_soc_set_card_names(struct snd_soc_card *card, const char *firmware); +void snd_soc_cleanup_card_names(struct snd_soc_card *card);
- /* Utility functions to get clock rates from various things */ int snd_soc_calc_frame_size(int sample_size, int channels, int
tdm_slots); int snd_soc_params_to_frame_size(struct snd_pcm_hw_params *params); diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index d2e62b15..0cfffff 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -34,6 +34,7 @@ #include <linux/ctype.h> #include <linux/slab.h> #include <linux/of.h> +#include <linux/dmi.h> #include <sound/core.h> #include <sound/jack.h> #include <sound/pcm.h> @@ -1828,6 +1829,92 @@ int snd_soc_runtime_set_dai_fmt(struct snd_soc_pcm_runtime *rtd, } EXPORT_SYMBOL_GPL(snd_soc_runtime_set_dai_fmt);
+/**
- snd_soc_set_card_names() - Register DMI names to card
- @card: The card to register DMI names
- @board: DMI_BOARD_NAME or DMI_PRODUCT_NAME
- @vendor: DMI_SYS_VENDOR, optional
- @firmware: The firmware name, optional
- This function registers DMI names to card for the userspace to
distinguish
- different boards/products:
- card driver name ---> machine driver name
- card short name ---> DMI_BOARD_NAME or DMI_PRODUCT_NAME
- card long name and
- card component ---> short name;driver name;(DMI_SYS_VENDOR,
optional)
;(firmware name, optional)
- Returns 0 on success, otherwise a negative error code.
- */
+int snd_soc_set_card_names(struct snd_soc_card *card, const char *firmware) +{
- int ret = 0;
- size_t buf_size, name_size;
- const char *board, *vendor;
- char *longname;
- board = dmi_get_system_info(DMI_BOARD_NAME);
- if (!board)
board = dmi_get_system_info(DMI_PRODUCT_NAME);- if (!board) {
dev_err(card->dev, "ASoC: the board/product name is empty!\n");return -EINVAL;- }
- vendor = dmi_get_system_info(DMI_SYS_VENDOR);
- /* card driver name */
- card->driver_name = card->name;
- /* card short name */
- card->name = board;
- /* card long name */
- buf_size = sizeof(card->snd_card->longname);
- name_size = strlen(card->name) + strlen(card->driver_name) + 4;
- if (vendor)
name_size += strlen(vendor);- if (firmware)
name_size += strlen(firmware);- if (name_size > buf_size)
return -ENOMEM;- longname = kmalloc(buf_size, GFP_KERNEL);
- if (!longname)
return -ENOMEM;- snprintf(longname, buf_size, "%s;%s;",
card->name, card->driver_name);- if (vendor)
strlcat(longname, vendor, buf_size);- strlcat(longname, ";", buf_size);
- if (firmware)
strlcat(longname, firmware, buf_size);- card->long_name = longname;
- /* card component */
- if (sizeof(card->snd_card->components) < name_size
+ strlen(card->snd_card->components))return -ENOMEM;- ret = snd_component_add(card->snd_card, card->long_name);
- return ret;
+} +EXPORT_SYMBOL_GPL(snd_soc_set_card_names);
+/**
- snd_soc_cleanup_card_names() - cleanup registered DMI names
- @card: The card to cleanup
- This function cleanup the registered DMI names from card
- */
+void snd_soc_cleanup_card_names(struct snd_soc_card *card) +{
- kfree(card->long_name);
+} +EXPORT_SYMBOL_GPL(snd_soc_cleanup_card_names);
Hm. How about do it in a path to release devres? In short,
When registering card instance, devm_card_release is assigned to the resource. devm_snd_soc_register_card() ->devres_alloc(devm_card_release, sizeof(*ptr), GFP_KERNEL);
When releasing the resource: (sound/soc/soc-devres.c) ->devm_card_release() (sound/soc/soc-core.c) ->snd_soc_unregister_card() ->kfree(card->long_name) (yours)
I just have a quick glance. It's better for you to confirm it in your case.
My concern is that driver developers may set card->long_name without calling snd_soc_set_card_names(), for example, in card definition: (not sure if it's rare) static struct snd_soc_card XXX_card = { .long_name = "XXXX", ... } I cannot kfree card->long_name in this case. So I added two APIs and they are supposed to be used in pair.
BR, Han
Regards
Takashi Sakamoto
static int snd_soc_instantiate_card(struct snd_soc_card *card) { struct snd_soc_codec *codec;
Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
From: "Lu, Han" han.lu@intel.com
Register DMI names for products using bytcr-rt5640 driver, such as ASUS T100TA and Dell Venue 8 Pro 5830, for user space utils to distinguish different products.
For example, previously on T100TA: $ cat /proc/asound/cards 0 [bytcrrt5640 ]: bytcr-rt5640 - bytcr-rt5640 bytcr-rt5640 $ amixer -c0 info Card hw:0 'bytcrrt5640'/'bytcr-rt5640' Mixer name : '' Components : '' Controls : 256 Simple ctrls : 228
After apply the patch: $ cat /proc/asound/cards 0 [T100TA ]: bytcr-rt5640 - T100TA T100TA;bytcr-rt5640;ASUSTek COMPUTER INC.;intel/fw_ sst_0f28.bin $ amixer -c0 info Card hw:0 'T100TA'/'T100TA;bytcr-rt5640;ASUSTek COMPUTER INC.;intel/fw_ sst_0f28.bin' Mixer name : '' Components : 'T100TA;bytcr-rt5640;ASUSTek COMPUTER INC.;intel/fw_s st_0f28.bin' Controls : 256 Simple ctrls : 228
Signed-off-by: Lu, Han han.lu@intel.com
diff --git a/sound/soc/intel/boards/bytcr_rt5640.c b/sound/soc/intel/boards/bytcr_rt5640.c index 032a2e7..014b549 100644 --- a/sound/soc/intel/boards/bytcr_rt5640.c +++ b/sound/soc/intel/boards/bytcr_rt5640.c @@ -152,6 +152,8 @@ static const struct dmi_system_id byt_rt5640_quirk_table[] = { {} };
+static struct snd_soc_card byt_rt5640_card; + static int byt_rt5640_init(struct snd_soc_pcm_runtime *runtime) { int ret; @@ -159,6 +161,13 @@ static int byt_rt5640_init(struct snd_soc_pcm_runtime *runtime) struct snd_soc_card *card = runtime->card; const struct snd_soc_dapm_route *custom_map; int num_routes; + struct sst_acpi_mach *mach = byt_rt5640_card.dev->platform_data; + + ret = snd_soc_set_card_names(card, mach->fw_filename); + if (ret < 0) { + dev_err(card->dev, "unable to register card names\n"); + return ret; + }
card->dapm.idle_bias_off = true;
@@ -380,12 +389,21 @@ static int snd_byt_rt5640_mc_probe(struct platform_device *pdev) return ret_val; }
+static int snd_byt_rt5640_mc_remove(struct platform_device *pdev) +{ + struct snd_soc_card *card = &byt_rt5640_card; + + snd_soc_cleanup_card_names(card); + return 0; +} + static struct platform_driver snd_byt_rt5640_mc_driver = { .driver = { .name = "bytcr_rt5640", .pm = &snd_soc_pm_ops, }, .probe = snd_byt_rt5640_mc_probe, + .remove = snd_byt_rt5640_mc_remove, };
module_platform_driver(snd_byt_rt5640_mc_driver);
participants (3)
-
Han Lu -
han.lu＠intel.com -
Takashi Sakamoto