Re: A FIX FOR alsa-lib emu10k1.h (IT FIXES ld10k1)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



At Wed, 17 Jan 2007 13:35:10 +0100,
私 wrote:
> 
> At Wed, 17 Jan 2007 00:12:07 -0500,
> terminator356@xxxxxxxxxxxxxxxxxxxxx wrote:
> > 
> > Hi, ld10k1 doesn't work, crashes. Lots of people saying it.
> > In the alsa-kernel/include emu10k1.h, 
> > 	union {
> > 		snd_kcontrol_tlv_rw_t *c;
> > 		unsigned int *p;
> > 	} tlv;
> >  was added to struct 'snd_emu10k1_fx8010_control_gpr'.
> > But alsa-lib/include/sound/emu10k1.h was not changed.
> > So I did this:
> >         ...
> > 	unsigned int min;		/* minimum range */
> > 	unsigned int max;		/* maximum range */
> >         unsigned int *p;    // <----- ADDED THIS  <----------------
> > 	unsigned int translation;	/* translation type (EMU10K1_GPR_TRANSLATION*) */
> > } emu10k1_fx8010_control_gpr_t;
> >  
> >  which is in alsa-lib/include/sound/emu10k1.h
> > 
> > For me, it fixes ld10k1. *p acts as a placeholder because it is useless 
> >  (at this point) to ld10k1. 
> > 
> > ----Can you tell me what those changes were for? What are *c and *p ? ---
> 
> Well, this is an ABI breakage by introducing the TLV support.
> The tlv keeps the additional information such as dB range.
> 
> James, do you have already a fix?
> I'm afraid that fixing this isn't so obvious and relatively
> complicated.

The patch below changes the driver to accept the old ABI as default,
then switch to the new ABI if EMU10K1_PVERSION ioctl is called.
Could you give it a try?  If it works, I'll post a patch to ld10k1 to
switch to the new ABI.


Takashi

diff -r beb52b12c43f include/emu10k1.h
--- a/include/emu10k1.h	Tue Jan 16 17:49:21 2007 +0100
+++ b/include/emu10k1.h	Thu Jan 18 12:04:51 2007 +0100
@@ -1449,6 +1449,7 @@ struct snd_emu10k1 {
 	unsigned int tos_link: 1,		/* tos link detected */
 		rear_ac97: 1,			/* rear channels are on AC'97 */
 		enable_ir: 1;
+	unsigned int support_tlv :1;
 	/* Contains profile of card capabilities */
 	const struct snd_emu_chip_details *card_capabilities;
 	unsigned int audigy;			/* is Audigy? */
@@ -1901,11 +1902,20 @@ struct snd_emu10k1_fx8010_control_gpr {
 	unsigned int value[32];		/* initial values */
 	unsigned int min;		/* minimum range */
 	unsigned int max;		/* maximum range */
-	union {
-		snd_kcontrol_tlv_rw_t *c;
-		unsigned int *p;
-	} tlv;
 	unsigned int translation;	/* translation type (EMU10K1_GPR_TRANSLATION*) */
+	unsigned int *tlv;
+};
+
+/* old ABI without TLV support */
+struct snd_emu10k1_fx8010_control_old_gpr {
+	struct snd_ctl_elem_id id;
+	unsigned int vcount;
+	unsigned int count;
+	unsigned short gpr[32];
+	unsigned int value[32];
+	unsigned int min;
+	unsigned int max;
+	unsigned int translation;
 };
 
 struct snd_emu10k1_fx8010_code {
@@ -1956,6 +1966,8 @@ struct snd_emu10k1_fx8010_pcm_rec {
 	unsigned int res2;		/* reserved */
 };
 
+#define SNDRV_EMU10K1_VERSION		SNDRV_PROTOCOL_VERSION(1, 0, 1)
+
 #define SNDRV_EMU10K1_IOCTL_INFO	_IOR ('H', 0x10, struct snd_emu10k1_fx8010_info)
 #define SNDRV_EMU10K1_IOCTL_CODE_POKE	_IOW ('H', 0x11, struct snd_emu10k1_fx8010_code)
 #define SNDRV_EMU10K1_IOCTL_CODE_PEEK	_IOWR('H', 0x12, struct snd_emu10k1_fx8010_code)
@@ -1964,6 +1976,7 @@ struct snd_emu10k1_fx8010_pcm_rec {
 #define SNDRV_EMU10K1_IOCTL_TRAM_PEEK	_IOWR('H', 0x22, struct snd_emu10k1_fx8010_tram)
 #define SNDRV_EMU10K1_IOCTL_PCM_POKE	_IOW ('H', 0x30, struct snd_emu10k1_fx8010_pcm_rec)
 #define SNDRV_EMU10K1_IOCTL_PCM_PEEK	_IOWR('H', 0x31, struct snd_emu10k1_fx8010_pcm_rec)
+#define SNDRV_EMU10K1_IOCTL_PVERSION	_IOR ('H', 0x40, int)
 #define SNDRV_EMU10K1_IOCTL_STOP	_IO  ('H', 0x80)
 #define SNDRV_EMU10K1_IOCTL_CONTINUE	_IO  ('H', 0x81)
 #define SNDRV_EMU10K1_IOCTL_ZERO_TRAM_COUNTER _IO ('H', 0x82)
diff -r beb52b12c43f pci/emu10k1/emufx.c
--- a/pci/emu10k1/emufx.c	Tue Jan 16 17:49:21 2007 +0100
+++ b/pci/emu10k1/emufx.c	Thu Jan 18 12:06:54 2007 +0100
@@ -655,13 +655,66 @@ snd_emu10k1_look_for_ctl(struct snd_emu1
 	return NULL;
 }
 
+#define MAX_TLV_SIZE	256
+
+static unsigned int *copy_tlv(unsigned int __user *_tlv)
+{
+	unsigned int data[2];
+	unsigned int *tlv;
+
+	if (!_tlv)
+		return NULL;
+	if (copy_from_user(data, _tlv, sizeof(data)))
+		return NULL;
+	if (data[1] >= MAX_TLV_SIZE)
+		return NULL;
+	tlv = kmalloc(data[1] * 4 + sizeof(data), GFP_KERNEL);
+	if (!tlv)
+		return NULL;
+	memcpy(tlv, data, sizeof(data));
+	if (copy_from_user(tlv + 2, _tlv + 2, data[1])) {
+		kfree(tlv);
+		return NULL;
+	}
+	return tlv;
+}
+
+static int copy_gctl(struct snd_emu10k1 *emu,
+		     struct snd_emu10k1_fx8010_control_gpr *gctl,
+		     struct snd_emu10k1_fx8010_control_gpr __user *_gctl,
+		     int idx)
+{
+	struct snd_emu10k1_fx8010_control_old_gpr __user *octl;
+
+	if (emu->support_tlv)
+		return copy_from_user(gctl, &_gctl[idx], sizeof(*gctl));
+	octl = (struct snd_emu10k1_fx8010_control_old_gpr __user *)_gctl;
+	if (copy_from_user(gctl, &octl[idx], sizeof(*octl)))
+		return -EFAULT;
+	gctl->tlv = NULL;
+	return 0;
+}
+
+static int copy_gctl_to_user(struct snd_emu10k1 *emu,
+		     struct snd_emu10k1_fx8010_control_gpr __user *_gctl,
+		     struct snd_emu10k1_fx8010_control_gpr *gctl,
+		     int idx)
+{
+	struct snd_emu10k1_fx8010_control_old_gpr __user *octl;
+
+	if (emu->support_tlv)
+		return copy_to_user(&_gctl[idx], gctl, sizeof(*gctl));
+	
+	octl = (struct snd_emu10k1_fx8010_control_old_gpr __user *)_gctl;
+	return copy_to_user(&octl[idx], gctl, sizeof(*octl));
+}
+
 static int snd_emu10k1_verify_controls(struct snd_emu10k1 *emu,
 				       struct snd_emu10k1_fx8010_code *icode)
 {
 	unsigned int i;
 	struct snd_ctl_elem_id __user *_id;
 	struct snd_ctl_elem_id id;
-	struct snd_emu10k1_fx8010_control_gpr __user *_gctl;
 	struct snd_emu10k1_fx8010_control_gpr *gctl;
 	int err;
 	
@@ -676,9 +729,8 @@ static int snd_emu10k1_verify_controls(s
 	if (! gctl)
 		return -ENOMEM;
 	err = 0;
-	for (i = 0, _gctl = icode->gpr_add_controls;
-	     i < icode->gpr_add_control_count; i++, _gctl++) {
-		if (copy_from_user(gctl, _gctl, sizeof(*gctl))) {
+	for (i = 0; i < icode->gpr_add_control_count; i++) {
+		if (copy_gctl(emu, gctl, icode->gpr_add_controls, i)) {
 			err = -EFAULT;
 			goto __error;
 		}
@@ -697,10 +749,9 @@ static int snd_emu10k1_verify_controls(s
 			goto __error;
 		}
 	}
-	for (i = 0, _gctl = icode->gpr_list_controls;
-	     i < icode->gpr_list_control_count; i++, _gctl++) {
+	for (i = 0; i < icode->gpr_list_control_count; i++) {
 	     	/* FIXME: we need to check the WRITE access */
-		if (copy_from_user(gctl, _gctl, sizeof(*gctl))) {
+		if (copy_gctl(emu, gctl, icode->gpr_list_controls, i)) {
 			err = -EFAULT;
 			goto __error;
 		}
@@ -718,13 +769,14 @@ static void snd_emu10k1_ctl_private_free
 	kctl->private_value = 0;
 	list_del(&ctl->list);
 	kfree(ctl);
+	if (kctl->tlv.p)
+		kfree(kctl->tlv.p);
 }
 
 static int snd_emu10k1_add_controls(struct snd_emu10k1 *emu,
 				    struct snd_emu10k1_fx8010_code *icode)
 {
 	unsigned int i, j;
-	struct snd_emu10k1_fx8010_control_gpr __user *_gctl;
 	struct snd_emu10k1_fx8010_control_gpr *gctl;
 	struct snd_emu10k1_fx8010_ctl *ctl, *nctl;
 	struct snd_kcontrol_new knew;
@@ -740,9 +792,8 @@ static int snd_emu10k1_add_controls(stru
 		goto __error;
 	}
 
-	for (i = 0, _gctl = icode->gpr_add_controls;
-	     i < icode->gpr_add_control_count; i++, _gctl++) {
-		if (copy_from_user(gctl, _gctl, sizeof(*gctl))) {
+	for (i = 0; i < icode->gpr_add_control_count; i++) {
+		if (copy_gctl(emu, gctl, icode->gpr_add_controls, i)) {
 			err = -EFAULT;
 			goto __error;
 		}
@@ -763,11 +814,10 @@ static int snd_emu10k1_add_controls(stru
 		knew.device = gctl->id.device;
 		knew.subdevice = gctl->id.subdevice;
 		knew.info = snd_emu10k1_gpr_ctl_info;
-		if (gctl->tlv.p) {
-			knew.tlv.p = gctl->tlv.p;
+		knew.tlv.p = copy_tlv(gctl->tlv);
+		if (knew.tlv.p)
 			knew.access = SNDRV_CTL_ELEM_ACCESS_READWRITE |
 				SNDRV_CTL_ELEM_ACCESS_TLV_READ;
-		} 
 		knew.get = snd_emu10k1_gpr_ctl_get;
 		knew.put = snd_emu10k1_gpr_ctl_put;
 		memset(nctl, 0, sizeof(*nctl));
@@ -785,12 +835,14 @@ static int snd_emu10k1_add_controls(stru
 			ctl = kmalloc(sizeof(*ctl), GFP_KERNEL);
 			if (ctl == NULL) {
 				err = -ENOMEM;
+				kfree(knew.tlv.p);
 				goto __error;
 			}
 			knew.private_value = (unsigned long)ctl;
 			*ctl = *nctl;
 			if ((err = snd_ctl_add(emu->card, kctl = snd_ctl_new1(&knew, emu))) < 0) {
 				kfree(ctl);
+				kfree(knew.tlv.p);
 				goto __error;
 			}
 			kctl->private_free = snd_emu10k1_ctl_private_free;
@@ -841,7 +893,6 @@ static int snd_emu10k1_list_controls(str
 	unsigned int i = 0, j;
 	unsigned int total = 0;
 	struct snd_emu10k1_fx8010_control_gpr *gctl;
-	struct snd_emu10k1_fx8010_control_gpr __user *_gctl;
 	struct snd_emu10k1_fx8010_ctl *ctl;
 	struct snd_ctl_elem_id *id;
 	struct list_head *list;
@@ -850,11 +901,11 @@ static int snd_emu10k1_list_controls(str
 	if (! gctl)
 		return -ENOMEM;
 
-	_gctl = icode->gpr_list_controls;	
 	list_for_each(list, &emu->fx8010.gpr_ctl) {
 		ctl = emu10k1_gpr_ctl(list);
 		total++;
-		if (_gctl && i < icode->gpr_list_control_count) {
+		if (icode->gpr_list_controls &&
+		    i < icode->gpr_list_control_count) {
 			memset(gctl, 0, sizeof(*gctl));
 			id = &ctl->kcontrol->id;
 			gctl->id.iface = id->iface;
@@ -871,11 +922,11 @@ static int snd_emu10k1_list_controls(str
 			gctl->min = ctl->min;
 			gctl->max = ctl->max;
 			gctl->translation = ctl->translation;
-			if (copy_to_user(_gctl, gctl, sizeof(*gctl))) {
+			if (copy_gctl_to_user(emu, icode->gpr_list_controls,
+					      gctl, i)) {
 				kfree(gctl);
 				return -EFAULT;
 			}
-			_gctl++;
 			i++;
 		}
 	}
@@ -1026,7 +1077,7 @@ snd_emu10k1_init_mono_control(struct snd
 	ctl->gpr[0] = gpr + 0; ctl->value[0] = defval;
 	ctl->min = 0;
 	ctl->max = 100;
-	ctl->tlv.p = snd_emu10k1_db_scale1;
+	ctl->tlv = snd_emu10k1_db_scale1;
 	ctl->translation = EMU10K1_GPR_TRANSLATION_TABLE100;	
 }
 
@@ -1041,7 +1092,7 @@ snd_emu10k1_init_stereo_control(struct s
 	ctl->gpr[1] = gpr + 1; ctl->value[1] = defval;
 	ctl->min = 0;
 	ctl->max = 100;
-	ctl->tlv.p = snd_emu10k1_db_scale1;
+	ctl->tlv = snd_emu10k1_db_scale1;
 	ctl->translation = EMU10K1_GPR_TRANSLATION_TABLE100;
 }
 
@@ -1566,7 +1617,9 @@ A_OP(icode, &ptr, iMAC0, A_GPR(var), A_G
 	seg = snd_enter_user();
 	icode->gpr_add_control_count = nctl;
 	icode->gpr_add_controls = (struct snd_emu10k1_fx8010_control_gpr __user *)controls;
+	emu->support_tlv = 1; /* support TLV */
 	err = snd_emu10k1_icode_poke(emu, icode);
+	emu->support_tlv = 0; /* clear again */
 	snd_leave_user(seg);
 
  __err:
@@ -2183,7 +2236,9 @@ static int __devinit _snd_emu10k1_init_e
 	seg = snd_enter_user();
 	icode->gpr_add_control_count = i;
 	icode->gpr_add_controls = (struct snd_emu10k1_fx8010_control_gpr __user *)controls;
+	emu->support_tlv = 1; /* support TLV */
 	err = snd_emu10k1_icode_poke(emu, icode);
+	emu->support_tlv = 0; /* clear again */
 	snd_leave_user(seg);
 	if (err >= 0)
 		err = snd_emu10k1_ipcm_poke(emu, ipcm);
@@ -2327,6 +2382,9 @@ static int snd_emu10k1_fx8010_ioctl(stru
 	int res;
 	
 	switch (cmd) {
+	case SNDRV_EMU10K1_IOCTL_PVERSION:
+		emu->support_tlv = 1;
+		return put_user(SNDRV_EMU10K1_VERSION, (int __user *)argp);
 	case SNDRV_EMU10K1_IOCTL_INFO:
 		info = kmalloc(sizeof(*info), GFP_KERNEL);
 		if (!info)

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.sourceforge.net/lists/listinfo/alsa-devel

[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux