On 04/06/2016 04:53 PM, Takashi Iwai wrote:
On Wed, 06 Apr 2016 09:29:17 +0200, han.lu@intel.com wrote:
From: "Lu, Han" han.lu@intel.com
Share more product information, for user space utils such as PA and UCM to distinguish different products.
- Add core APIs to register and cleanup DMI names for card.
- Apply the APIs to bytcr-rt5640 driver.
Han, one good advice is: be patient. This is no urgent fix, so there is no many reason to rush too much with a pile of newer patchset. Give some more time for other people to review.
sorry, I was too hurry. I'll update patch after more review.
About the patch: I still have a few concerns, and some are in the fundamental level:
- Is calling dmi_*() function in ASoC core is appropriate and preferred?
I was intended to add a common function for each machine driver to call, and to add firmware name if necessary. I can move all dmi_*() functions to machine drivers too, only pass the name strings as arguments.
When is this function supposed to be called? Since you're accessing card->snd_card, it must be after instantiation, that is, snd_soc_register_card(). If so, it should be documented. And, in that case, there is no need to allocate a buffer; you can set the strings directly in card->snd_card. (For snd_component_add(), you may pass the card->snd_card->longname[]).
OTOH, if the function may be called before instantiation, the code needs more rethink, including the string allocation and release.
the intention was to set the card->long_name, before the accessing to it in snd_soc_instantiate_card(): ... snprintf(card->snd_card->longname, sizeof(card->snd_card->longname), "%s", card->long_name ? card->long_name : card->name); ... or it may be better to overwrite the card->snd_card->longname[] after instantiation, so no need to allocate and release at all?
- A semicolon is no taboo character, either. A firmware or vendor string may contain such a letter, too. You need to either escape or replace such a letter instead of praying the well-mannered firmware.
so any printable ASCII character may have the same risk, may I use a control code such as '\t', or a non ASCII value like 0x80?
BR, Han
thanks,
Takashi
changes on V7:
- Remove inconsistent API description
changes on V6:
- Use dynamic allocate and cleanup for card long name
- Remove unneccessary arguments to simplify the API
changes on V5:
- 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:
- Replace kmalloc() and snprintf() with ksaprintf() to simplify code
changes on V3:
- Split the core API and the API call to two patches
- Replace misused strcat() with snprintf()
- 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 | 85 +++++++++++++++++++++++++++++++++++ 3 files changed, 106 insertions(+)
-- 2.5.0
Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel