[alsa-devel] [PATCH V7 0/2] ASoC: Add core API to register and cleanup DMI names for card

[Takashi Iwai]

On Wed, 06 Apr 2016 09:29:17 +0200,
han.lu at intel.com wrote:
> 
> From: "Lu, Han" <han.lu at 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.

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.

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?

- 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.

- 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.


thanks,

Takashi

> 
>  changes on V7:
>   1. Remove inconsistent API description
>   
>  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                  | 85 +++++++++++++++++++++++++++++++++++
>  3 files changed, 106 insertions(+)
> 
> -- 
> 2.5.0
> 
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel at alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>