[alsa-devel] [PATCH] Revert "ALSA: hda - Remove limit of widget connections"

Takashi Iwai tiwai at suse.de
Tue Mar 12 15:10:39 CET 2013


At Tue, 12 Mar 2013 15:05:02 +0100,
David Henningsson wrote:
> 
> This reverts commit 4eea30914facd2c99061cd70e5b05d3c76c743a2.
> 
> The commit causes an error with node ranges: In my case, connection
> list length is 3, with a connection list read of 0x1e9915: a total of
> 6 nids (0x15, 0x16, 0x17, 0x18, 0x19, 0x1e). Since the new code makes
> us allocate only 3 nids to the array sent to get_raw_connections,
> the function fails.

Please give more information.  Need to know which codec exactly you
hit, and which values are returned for which verbs.


thanks,

Takashi

> ---
>  sound/pci/hda/hda_codec.c  |   62 +++++++++++++++-----------------------------
>  sound/pci/hda/hda_codec.h  |    4 ++-
>  sound/pci/hda/hda_proc.c   |   20 +++-----------
>  sound/pci/hda/patch_hdmi.c |    3 ---
>  4 files changed, 28 insertions(+), 61 deletions(-)
> 
> diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
> index 97c68dd..e5e8549 100644
> --- a/sound/pci/hda/hda_codec.c
> +++ b/sound/pci/hda/hda_codec.c
> @@ -382,23 +382,13 @@ static void remove_conn_list(struct hda_codec *codec)
>  /* read the connection and add to the cache */
>  static int read_and_add_raw_conns(struct hda_codec *codec, hda_nid_t nid)
>  {
> -	hda_nid_t list[32];
> -	hda_nid_t *result = list;
> +	hda_nid_t list[HDA_MAX_CONNECTIONS];
>  	int len;
>  
>  	len = snd_hda_get_raw_connections(codec, nid, list, ARRAY_SIZE(list));
> -	if (len == -ENOSPC) {
> -		len = snd_hda_get_num_raw_conns(codec, nid);
> -		result = kmalloc(sizeof(hda_nid_t) * len, GFP_KERNEL);
> -		if (!result)
> -			return -ENOMEM;
> -		len = snd_hda_get_raw_connections(codec, nid, result, len);
> -	}
> -	if (len >= 0)
> -		len = snd_hda_override_conn_list(codec, nid, len, result);
> -	if (result != list)
> -		kfree(result);
> -	return len;
> +	if (len < 0)
> +		return len;
> +	return snd_hda_override_conn_list(codec, nid, len, list);
>  }
>  
>  /**
> @@ -476,27 +466,6 @@ int snd_hda_get_connections(struct hda_codec *codec, hda_nid_t nid,
>  }
>  EXPORT_SYMBOL_HDA(snd_hda_get_connections);
>  
> -/* return CONNLIST_LEN parameter of the given widget */
> -static unsigned int get_num_conns(struct hda_codec *codec, hda_nid_t nid)
> -{
> -	unsigned int wcaps = get_wcaps(codec, nid);
> -	unsigned int parm;
> -
> -	if (!(wcaps & AC_WCAP_CONN_LIST) &&
> -	    get_wcaps_type(wcaps) != AC_WID_VOL_KNB)
> -		return 0;
> -
> -	parm = snd_hda_param_read(codec, nid, AC_PAR_CONNLIST_LEN);
> -	if (parm == -1)
> -		parm = 0;
> -	return parm;
> -}
> -
> -int snd_hda_get_num_raw_conns(struct hda_codec *codec, hda_nid_t nid)
> -{
> -	return get_num_conns(codec, nid) & AC_CLIST_LENGTH;
> -}
> -
>  /**
>   * snd_hda_get_raw_connections - copy connection list without cache
>   * @codec: the HDA codec
> @@ -514,16 +483,19 @@ int snd_hda_get_raw_connections(struct hda_codec *codec, hda_nid_t nid,
>  	unsigned int parm;
>  	int i, conn_len, conns;
>  	unsigned int shift, num_elems, mask;
> +	unsigned int wcaps;
>  	hda_nid_t prev_nid;
>  	int null_count = 0;
>  
>  	if (snd_BUG_ON(!conn_list || max_conns <= 0))
>  		return -EINVAL;
>  
> -	parm = get_num_conns(codec, nid);
> -	if (!parm)
> +	wcaps = get_wcaps(codec, nid);
> +	if (!(wcaps & AC_WCAP_CONN_LIST) &&
> +	    get_wcaps_type(wcaps) != AC_WID_VOL_KNB)
>  		return 0;
>  
> +	parm = snd_hda_param_read(codec, nid, AC_PAR_CONNLIST_LEN);
>  	if (parm & AC_CLIST_LONG) {
>  		/* long form */
>  		shift = 16;
> @@ -580,13 +552,21 @@ int snd_hda_get_raw_connections(struct hda_codec *codec, hda_nid_t nid,
>  				continue;
>  			}
>  			for (n = prev_nid + 1; n <= val; n++) {
> -				if (conns >= max_conns)
> -					return -ENOSPC;
> +				if (conns >= max_conns) {
> +					snd_printk(KERN_ERR "hda_codec: "
> +						   "Too many connections %d for NID 0x%x\n",
> +						   conns, nid);
> +					return -EINVAL;
> +				}
>  				conn_list[conns++] = n;
>  			}
>  		} else {
> -			if (conns >= max_conns)
> -				return -ENOSPC;
> +			if (conns >= max_conns) {
> +				snd_printk(KERN_ERR "hda_codec: "
> +					   "Too many connections %d for NID 0x%x\n",
> +					   conns, nid);
> +				return -EINVAL;
> +			}
>  			conn_list[conns++] = val;
>  		}
>  		prev_nid = val;
> diff --git a/sound/pci/hda/hda_codec.h b/sound/pci/hda/hda_codec.h
> index 23ca172..246ed53 100644
> --- a/sound/pci/hda/hda_codec.h
> +++ b/sound/pci/hda/hda_codec.h
> @@ -551,6 +551,9 @@ enum {
>  	AC_JACK_PORT_BOTH,
>  };
>  
> +/* max. connections to a widget */
> +#define HDA_MAX_CONNECTIONS	32
> +
>  /* max. codec address */
>  #define HDA_MAX_CODEC_ADDRESS	0x0f
>  
> @@ -957,7 +960,6 @@ snd_hda_get_num_conns(struct hda_codec *codec, hda_nid_t nid)
>  {
>  	return snd_hda_get_connections(codec, nid, NULL, 0);
>  }
> -int snd_hda_get_num_raw_conns(struct hda_codec *codec, hda_nid_t nid);
>  int snd_hda_get_raw_connections(struct hda_codec *codec, hda_nid_t nid,
>  			    hda_nid_t *conn_list, int max_conns);
>  int snd_hda_get_conn_list(struct hda_codec *codec, hda_nid_t nid,
> diff --git a/sound/pci/hda/hda_proc.c b/sound/pci/hda/hda_proc.c
> index 0fee8fa..5e02f26 100644
> --- a/sound/pci/hda/hda_proc.c
> +++ b/sound/pci/hda/hda_proc.c
> @@ -22,7 +22,6 @@
>   */
>  
>  #include <linux/init.h>
> -#include <linux/slab.h>
>  #include <sound/core.h>
>  #include "hda_codec.h"
>  #include "hda_local.h"
> @@ -624,7 +623,7 @@ static void print_codec_info(struct snd_info_entry *entry,
>  			snd_hda_param_read(codec, nid,
>  					   AC_PAR_AUDIO_WIDGET_CAP);
>  		unsigned int wid_type = get_wcaps_type(wid_caps);
> -		hda_nid_t *conn = NULL;
> +		hda_nid_t conn[HDA_MAX_CONNECTIONS];
>  		int conn_len = 0;
>  
>  		snd_iprintf(buffer, "Node 0x%02x [%s] wcaps 0x%x:", nid,
> @@ -661,18 +660,9 @@ static void print_codec_info(struct snd_info_entry *entry,
>  		if (wid_type == AC_WID_VOL_KNB)
>  			wid_caps |= AC_WCAP_CONN_LIST;
>  
> -		if (wid_caps & AC_WCAP_CONN_LIST) {
> -			conn_len = snd_hda_get_num_raw_conns(codec, nid);
> -			if (conn_len > 0) {
> -				conn = kmalloc(sizeof(hda_nid_t) * conn_len,
> -					       GFP_KERNEL);
> -				if (!conn)
> -					return;
> -				if (snd_hda_get_raw_connections(codec, nid, conn,
> -								conn_len) < 0)
> -					conn_len = 0;
> -			}
> -		}
> +		if (wid_caps & AC_WCAP_CONN_LIST)
> +			conn_len = snd_hda_get_raw_connections(codec, nid, conn,
> +							   HDA_MAX_CONNECTIONS);
>  
>  		if (wid_caps & AC_WCAP_IN_AMP) {
>  			snd_iprintf(buffer, "  Amp-In caps: ");
> @@ -745,8 +735,6 @@ static void print_codec_info(struct snd_info_entry *entry,
>  
>  		if (codec->proc_widget_hook)
>  			codec->proc_widget_hook(buffer, codec, nid);
> -
> -		kfree(conn);
>  	}
>  	snd_hda_power_down(codec);
>  }
> diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
> index 78e1827..d24af4a 100644
> --- a/sound/pci/hda/patch_hdmi.c
> +++ b/sound/pci/hda/patch_hdmi.c
> @@ -64,9 +64,6 @@ struct hdmi_spec_per_cvt {
>  	unsigned int maxbps;
>  };
>  
> -/* max. connections to a widget */
> -#define HDA_MAX_CONNECTIONS	32
> -
>  struct hdmi_spec_per_pin {
>  	hda_nid_t pin_nid;
>  	int num_mux_nids;
> -- 
> 1.7.9.5
> 


More information about the Alsa-devel mailing list