[alsa-devel] [PATCH 0/4] Minor cleanups to hda code
Hi,
this is a series of trivial cleanup patches for HDA driver, mostly for procfs code.
Takashi Iwai (4): ALSA: hda - Make some helper functions local ALSA: hda/proc - Add const to possible places ALSA: hda/proc - Fix racy string access for power states ALSA: hda/eld - Add const to possible places
sound/pci/hda/hda_codec.c | 70 ------------------------------ sound/pci/hda/hda_codec.h | 7 --- sound/pci/hda/hda_eld.c | 10 ++--- sound/pci/hda/hda_proc.c | 107 +++++++++++++++++++++++++++++++++++----------- 4 files changed, 86 insertions(+), 108 deletions(-)
A few helper functions to convert the pin information to strings have been exported with assumption that they were used by other drivers. But they are referred only in the proc interface in the end.
Let's make them local so that we can get rid of a few exports.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/pci/hda/hda_codec.c | 70 ----------------------------------------------- sound/pci/hda/hda_codec.h | 7 ----- sound/pci/hda/hda_proc.c | 65 +++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 62 insertions(+), 80 deletions(-)
diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c index d78fa713c103..00c9a42af656 100644 --- a/sound/pci/hda/hda_codec.c +++ b/sound/pci/hda/hda_codec.c @@ -53,76 +53,6 @@ #define codec_has_clkstop(codec) \ ((codec)->core.power_caps & AC_PWRST_CLKSTOP)
-/** - * snd_hda_get_jack_location - Give a location string of the jack - * @cfg: pin default config value - * - * Parse the pin default config value and returns the string of the - * jack location, e.g. "Rear", "Front", etc. - */ -const char *snd_hda_get_jack_location(u32 cfg) -{ - static char *bases[7] = { - "N/A", "Rear", "Front", "Left", "Right", "Top", "Bottom", - }; - static unsigned char specials_idx[] = { - 0x07, 0x08, - 0x17, 0x18, 0x19, - 0x37, 0x38 - }; - static char *specials[] = { - "Rear Panel", "Drive Bar", - "Riser", "HDMI", "ATAPI", - "Mobile-In", "Mobile-Out" - }; - int i; - cfg = (cfg & AC_DEFCFG_LOCATION) >> AC_DEFCFG_LOCATION_SHIFT; - if ((cfg & 0x0f) < 7) - return bases[cfg & 0x0f]; - for (i = 0; i < ARRAY_SIZE(specials_idx); i++) { - if (cfg == specials_idx[i]) - return specials[i]; - } - return "UNKNOWN"; -} -EXPORT_SYMBOL_GPL(snd_hda_get_jack_location); - -/** - * snd_hda_get_jack_connectivity - Give a connectivity string of the jack - * @cfg: pin default config value - * - * Parse the pin default config value and returns the string of the - * jack connectivity, i.e. external or internal connection. - */ -const char *snd_hda_get_jack_connectivity(u32 cfg) -{ - static char *jack_locations[4] = { "Ext", "Int", "Sep", "Oth" }; - - return jack_locations[(cfg >> (AC_DEFCFG_LOCATION_SHIFT + 4)) & 3]; -} -EXPORT_SYMBOL_GPL(snd_hda_get_jack_connectivity); - -/** - * snd_hda_get_jack_type - Give a type string of the jack - * @cfg: pin default config value - * - * Parse the pin default config value and returns the string of the - * jack type, i.e. the purpose of the jack, such as Line-Out or CD. - */ -const char *snd_hda_get_jack_type(u32 cfg) -{ - static char *jack_types[16] = { - "Line Out", "Speaker", "HP Out", "CD", - "SPDIF Out", "Digital Out", "Modem Line", "Modem Hand", - "Line In", "Aux", "Mic", "Telephony", - "SPDIF In", "Digital In", "Reserved", "Other" - }; - - return jack_types[(cfg & AC_DEFCFG_DEVICE) - >> AC_DEFCFG_DEVICE_SHIFT]; -} -EXPORT_SYMBOL_GPL(snd_hda_get_jack_type); - /* * Send and receive a verb - passed to exec_verb override for hdac_device */ diff --git a/sound/pci/hda/hda_codec.h b/sound/pci/hda/hda_codec.h index 12837abbbbe5..2970413f18a0 100644 --- a/sound/pci/hda/hda_codec.h +++ b/sound/pci/hda/hda_codec.h @@ -469,13 +469,6 @@ int hda_call_check_power_status(struct hda_codec *codec, hda_nid_t nid) }
/* - * get widget information - */ -const char *snd_hda_get_jack_connectivity(u32 cfg); -const char *snd_hda_get_jack_type(u32 cfg); -const char *snd_hda_get_jack_location(u32 cfg); - -/* * power saving */ #define snd_hda_power_up(codec) snd_hdac_power_up(&(codec)->core) diff --git a/sound/pci/hda/hda_proc.c b/sound/pci/hda/hda_proc.c index baaf7ed06875..3a470f7b9f41 100644 --- a/sound/pci/hda/hda_proc.c +++ b/sound/pci/hda/hda_proc.c @@ -269,6 +269,65 @@ static const char *get_jack_color(u32 cfg) return "UNKNOWN"; }
+/* + * Parse the pin default config value and returns the string of the + * jack location, e.g. "Rear", "Front", etc. + */ +static const char *get_jack_location(u32 cfg) +{ + static char *bases[7] = { + "N/A", "Rear", "Front", "Left", "Right", "Top", "Bottom", + }; + static unsigned char specials_idx[] = { + 0x07, 0x08, + 0x17, 0x18, 0x19, + 0x37, 0x38 + }; + static char *specials[] = { + "Rear Panel", "Drive Bar", + "Riser", "HDMI", "ATAPI", + "Mobile-In", "Mobile-Out" + }; + int i; + + cfg = (cfg & AC_DEFCFG_LOCATION) >> AC_DEFCFG_LOCATION_SHIFT; + if ((cfg & 0x0f) < 7) + return bases[cfg & 0x0f]; + for (i = 0; i < ARRAY_SIZE(specials_idx); i++) { + if (cfg == specials_idx[i]) + return specials[i]; + } + return "UNKNOWN"; +} + +/* + * Parse the pin default config value and returns the string of the + * jack connectivity, i.e. external or internal connection. + */ +static const char *get_jack_connectivity(u32 cfg) +{ + static char *jack_locations[4] = { "Ext", "Int", "Sep", "Oth" }; + + return jack_locations[(cfg >> (AC_DEFCFG_LOCATION_SHIFT + 4)) & 3]; +} + +/* + * Parse the pin default config value and returns the string of the + * jack type, i.e. the purpose of the jack, such as Line-Out or CD. + */ +static const char *get_jack_type(u32 cfg) +{ + static char *jack_types[16] = { + "Line Out", "Speaker", "HP Out", "CD", + "SPDIF Out", "Digital Out", "Modem Line", "Modem Hand", + "Line In", "Aux", "Mic", "Telephony", + "SPDIF In", "Digital In", "Reserved", "Other" + }; + + return jack_types[(cfg & AC_DEFCFG_DEVICE) + >> AC_DEFCFG_DEVICE_SHIFT]; +} + static void print_pin_caps(struct snd_info_buffer *buffer, struct hda_codec *codec, hda_nid_t nid, int *supports_vref) @@ -340,9 +399,9 @@ static void print_pin_caps(struct snd_info_buffer *buffer, caps = snd_hda_codec_read(codec, nid, 0, AC_VERB_GET_CONFIG_DEFAULT, 0); snd_iprintf(buffer, " Pin Default 0x%08x: [%s] %s at %s %s\n", caps, jack_conns[(caps & AC_DEFCFG_PORT_CONN) >> AC_DEFCFG_PORT_CONN_SHIFT], - snd_hda_get_jack_type(caps), - snd_hda_get_jack_connectivity(caps), - snd_hda_get_jack_location(caps)); + get_jack_type(caps), + get_jack_connectivity(caps), + get_jack_location(caps)); snd_iprintf(buffer, " Conn = %s, Color = %s\n", get_jack_connection(caps), get_jack_color(caps));
Many arrays in hda_proc.c are string arrays that should be covered by const prefix for increasing the safety and reducing the size.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/pci/hda/hda_proc.c | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-)
diff --git a/sound/pci/hda/hda_proc.c b/sound/pci/hda/hda_proc.c index 3a470f7b9f41..5efcffc32c50 100644 --- a/sound/pci/hda/hda_proc.c +++ b/sound/pci/hda/hda_proc.c @@ -36,7 +36,7 @@ MODULE_PARM_DESC(dump_coef, "Dump processing coefficients in codec proc file (-1 #define param_read(codec, nid, parm) \ snd_hdac_read_parm_uncached(&(codec)->core, nid, parm)
-static char *bits_names(unsigned int bits, char *names[], int size) +static char *bits_names(unsigned int bits, const char * const names[], int size) { int i, n; static char buf[128]; @@ -53,7 +53,7 @@ static char *bits_names(unsigned int bits, char *names[], int size)
static const char *get_wid_type_name(unsigned int wid_value) { - static char *names[16] = { + static const char * const names[16] = { [AC_WID_AUD_OUT] = "Audio Output", [AC_WID_AUD_IN] = "Audio Input", [AC_WID_AUD_MIX] = "Audio Mixer", @@ -241,7 +241,7 @@ static void print_pcm_caps(struct snd_info_buffer *buffer,
static const char *get_jack_connection(u32 cfg) { - static char *names[16] = { + static const char * const names[16] = { "Unknown", "1/8", "1/4", "ATAPI", "RCA", "Optical","Digital", "Analog", "DIN", "XLR", "RJ11", "Comb", @@ -256,7 +256,7 @@ static const char *get_jack_connection(u32 cfg)
static const char *get_jack_color(u32 cfg) { - static char *names[16] = { + static const char * const names[16] = { "Unknown", "Black", "Grey", "Blue", "Green", "Red", "Orange", "Yellow", "Purple", "Pink", NULL, NULL, @@ -275,15 +275,15 @@ static const char *get_jack_color(u32 cfg) */ static const char *get_jack_location(u32 cfg) { - static char *bases[7] = { + static const char * const bases[7] = { "N/A", "Rear", "Front", "Left", "Right", "Top", "Bottom", }; - static unsigned char specials_idx[] = { + static const unsigned char specials_idx[] = { 0x07, 0x08, 0x17, 0x18, 0x19, 0x37, 0x38 }; - static char *specials[] = { + static const char * const specials[] = { "Rear Panel", "Drive Bar", "Riser", "HDMI", "ATAPI", "Mobile-In", "Mobile-Out" @@ -306,7 +306,9 @@ static const char *get_jack_location(u32 cfg) */ static const char *get_jack_connectivity(u32 cfg) { - static char *jack_locations[4] = { "Ext", "Int", "Sep", "Oth" }; + static const char * const jack_locations[4] = { + "Ext", "Int", "Sep", "Oth" + };
return jack_locations[(cfg >> (AC_DEFCFG_LOCATION_SHIFT + 4)) & 3]; } @@ -317,7 +319,7 @@ static const char *get_jack_connectivity(u32 cfg) */ static const char *get_jack_type(u32 cfg) { - static char *jack_types[16] = { + static const char * const jack_types[16] = { "Line Out", "Speaker", "HP Out", "CD", "SPDIF Out", "Digital Out", "Modem Line", "Modem Hand", "Line In", "Aux", "Mic", "Telephony", @@ -332,7 +334,9 @@ static void print_pin_caps(struct snd_info_buffer *buffer, struct hda_codec *codec, hda_nid_t nid, int *supports_vref) { - static char *jack_conns[4] = { "Jack", "N/A", "Fixed", "Both" }; + static const char * const jack_conns[4] = { + "Jack", "N/A", "Fixed", "Both" + }; unsigned int caps, val;
caps = param_read(codec, nid, AC_PAR_PIN_CAP); @@ -537,7 +541,7 @@ static const char *get_pwr_state(u32 state) static void print_power_state(struct snd_info_buffer *buffer, struct hda_codec *codec, hda_nid_t nid) { - static char *names[] = { + static const char * const names[] = { [ilog2(AC_PWRST_D0SUP)] = "D0", [ilog2(AC_PWRST_D1SUP)] = "D1", [ilog2(AC_PWRST_D2SUP)] = "D2",
The power states in a proc file are printed in a racy manner on a single static string buffer. Fix it by calling snd_iprintf() directly for each state instead of processing on a temporary buffer.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/pci/hda/hda_proc.c | 28 ++++++++++------------------ 1 file changed, 10 insertions(+), 18 deletions(-)
diff --git a/sound/pci/hda/hda_proc.c b/sound/pci/hda/hda_proc.c index 5efcffc32c50..033aa84365b9 100644 --- a/sound/pci/hda/hda_proc.c +++ b/sound/pci/hda/hda_proc.c @@ -36,21 +36,6 @@ MODULE_PARM_DESC(dump_coef, "Dump processing coefficients in codec proc file (-1 #define param_read(codec, nid, parm) \ snd_hdac_read_parm_uncached(&(codec)->core, nid, parm)
-static char *bits_names(unsigned int bits, const char * const names[], int size) -{ - int i, n; - static char buf[128]; - - for (i = 0, n = 0; i < size; i++) { - if (bits & (1U<<i) && names[i]) - n += snprintf(buf + n, sizeof(buf) - n, " %s", - names[i]); - } - buf[n] = '\0'; - - return buf; -} - static const char *get_wid_type_name(unsigned int wid_value) { static const char * const names[16] = { @@ -555,9 +540,16 @@ static void print_power_state(struct snd_info_buffer *buffer, int sup = param_read(codec, nid, AC_PAR_POWER_STATE); int pwr = snd_hda_codec_read(codec, nid, 0, AC_VERB_GET_POWER_STATE, 0); - if (sup != -1) - snd_iprintf(buffer, " Power states: %s\n", - bits_names(sup, names, ARRAY_SIZE(names))); + if (sup != -1) { + int i; + + snd_iprintf(buffer, " Power states: "); + for (i = 0; i < ARRAY_SIZE(names); i++) { + if (sup & (1U << i)) + snd_iprintf(buffer, " %s", names[i]); + } + snd_iprintf(buffer, "\n"); + }
snd_iprintf(buffer, " Power: setting=%s, actual=%s", get_pwr_state(pwr & AC_PWRST_SETTING),
Similar like the previous fix to hda_proc.c, adding const prefix will save our world (a little bit).
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/pci/hda/hda_eld.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/sound/pci/hda/hda_eld.c b/sound/pci/hda/hda_eld.c index c746cd9a4450..563984dd2562 100644 --- a/sound/pci/hda/hda_eld.c +++ b/sound/pci/hda/hda_eld.c @@ -42,7 +42,7 @@ enum cea_edid_versions { CEA_EDID_VER_RESERVED = 4, };
-static char *cea_speaker_allocation_names[] = { +static const char * const cea_speaker_allocation_names[] = { /* 0 */ "FL/FR", /* 1 */ "LFE", /* 2 */ "FC", @@ -56,7 +56,7 @@ static char *cea_speaker_allocation_names[] = { /* 10 */ "FCH", };
-static char *eld_connection_type_names[4] = { +static const char * const eld_connection_type_names[4] = { "HDMI", "DisplayPort", "2-reserved", @@ -94,7 +94,7 @@ enum cea_audio_coding_xtypes { AUDIO_CODING_XTYPE_FIRST_RESERVED = 4, };
-static char *cea_audio_coding_type_names[] = { +static const char * const cea_audio_coding_type_names[] = { /* 0 */ "undefined", /* 1 */ "LPCM", /* 2 */ "AC-3", @@ -482,14 +482,14 @@ void snd_hdmi_print_eld_info(struct hdmi_eld *eld, struct parsed_hdmi_eld *e = &eld->info; char buf[SND_PRINT_CHANNEL_ALLOCATION_ADVISED_BUFSIZE]; int i; - static char *eld_version_names[32] = { + static const char * const eld_version_names[32] = { "reserved", "reserved", "CEA-861D or below", [3 ... 30] = "reserved", [31] = "partial" }; - static char *cea_edid_version_names[8] = { + static const char * const cea_edid_version_names[8] = { "no CEA EDID Timing Extension block present", "CEA-861", "CEA-861-A",
participants (1)
-
Takashi Iwai