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

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

[terminator356]

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

      Thanks. Tim. 
      terminator356  -> users -> sourceforge -> net
      (Developer on ld10k1 project).

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

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

[Takashi Iwai]

At Wed, 17 Jan 2007 00:12:07 -0500,
terminator356@users.sourceforge.net 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.


Takashi

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

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

[Takashi Iwai]

At Wed, 17 Jan 2007 13:35:10 +0100,
私 wrote:
> 
> At Wed, 17 Jan 2007 00:12:07 -0500,
> terminator356@users.sourceforge.net 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

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

[James Courtier-Dutton]

Takashi Iwai wrote:
> At Wed, 17 Jan 2007 00:12:07 -0500,
> terminator356@users.sourceforge.net 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.
>
>
> Takashi
>   
I am busy at the moment, but I will look into this in a week or so.

James


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

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

[Tim]

On Thursday 18 January 2007 6:08 am, you wrote:
> At Wed, 17 Jan 2007 13:35:10 +0100,
> > terminator356 -> users.sourceforge.net wrote:
> > > Hi, ld10k1 doesn't work, crashes. Lots of people saying it.
> > > In the alsa-kernel/include emu10k1.h,
> > > 	union {
Sorry that should be alsa-kernel/include/sound/emu10k1.h

> > Well, this is an ABI breakage by introducing the TLV support.
What is ABI? "Always Broken Interface"? He he...

> 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
(Should be include/sound/emu10k1.h)

>
> @@ -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;

This is what I get, what am I doing wrong?:
[me@localhost sound]# patch -p1 emu10k1.h alsa-driver-tiwai-emu10k1-h-patch
patching file emu10k1.h
patch: **** malformed patch at line 5: unsigned int tos_link: 1,/* tos link 
           detected */

(That line is present in the emu10k1.h file).
Thanks.



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

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

[Takashi Iwai]

[-- Attachment #1: Type: text/plain, Size: 1662 bytes --]

At Fri, 19 Jan 2007 01:17:38 +0000 (UTC),
Tim wrote:
> 
> On Thursday 18 January 2007 6:08 am, you wrote:
> > At Wed, 17 Jan 2007 13:35:10 +0100,
> > > terminator356 -> users.sourceforge.net wrote:
> > > > Hi, ld10k1 doesn't work, crashes. Lots of people saying it.
> > > > In the alsa-kernel/include emu10k1.h,
> > > > 	union {
> Sorry that should be alsa-kernel/include/sound/emu10k1.h

No, it's really alsa-kernel/include/emu10k1.h.  It's a fix for the
driver, not the library.

> > > Well, this is an ABI breakage by introducing the TLV support.
> What is ABI? "Always Broken Interface"? He he...

Ask google :)

> > 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
> (Should be include/sound/emu10k1.h)
> 
> > @@ -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;
> 
> This is what I get, what am I doing wrong?:
> [me@localhost sound]# patch -p1 emu10k1.h alsa-driver-tiwai-emu10k1-h-patch
> patching file emu10k1.h
> patch: **** malformed patch at line 5: unsigned int tos_link: 1,/* tos link 
>            detected */

You mailer seems not able to handle the embedded patch...
I put the patch below as an attachment.


Takashi

[-- Attachment #2: emu10k1-ld10k1-compat-fix.diff --]
[-- Type: application/octet-stream, Size: 9697 bytes --]

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)

[-- Attachment #3: Type: text/plain, Size: 347 bytes --]

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

[-- Attachment #4: Type: text/plain, Size: 161 bytes --]

_______________________________________________
Alsa-devel mailing list
Alsa-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/alsa-devel

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

[Takashi Iwai]

At Fri, 19 Jan 2007 09:58:17 +1100,
James Courtier-Dutton wrote:
> 
> Takashi Iwai wrote:
> > At Wed, 17 Jan 2007 00:12:07 -0500,
> > terminator356@users.sourceforge.net 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.
> >
> >
> > Takashi
> >   
> I am busy at the moment, but I will look into this in a week or so.

I built up a test machine with sb live now.
Fixed and committed to HG tree.

The driver behaves backward-compatible as default.  Once after it gets
the new SNDRV_EMU10K1_IOCTL_PVERSION ioctl, it changes to the new
ABI.


Takashi

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

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

[Tim]

Takashi Iwai <tiwai <at> suse.de> writes:
> No, it's really alsa-kernel/include/emu10k1.h.  It's a fix for the
> driver, not the library.
Yes, sorry, sometimes I look in my kernel tree, where it's
 include/sound/emu10k1.h
and other times I look in the seperate alsa-driver where it's
 alsa-kernel/include/emu10k1.h

> I put the patch below as an attachment.

OK. I applied the patch, built and installed alsa-driver, rebuilt
 and installed alsa-lib (without MY emu10k1.h fix), rebuilt and
 installed ld10k1. Test OK. 
Thanks a bunch!
So this means apps (like ld10k1) could take advantage of the tlv.




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

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

[Takashi Iwai]

At Fri, 19 Jan 2007 23:14:59 +0000 (UTC),
Tim wrote:
> 
> > I put the patch below as an attachment.
> 
> OK. I applied the patch, built and installed alsa-driver, rebuilt
>  and installed alsa-lib (without MY emu10k1.h fix), rebuilt and
>  installed ld10k1. Test OK. 
> Thanks a bunch!
> So this means apps (like ld10k1) could take advantage of the tlv.

Yes, but currently ld10k1 doesn't support it.
Anyway, the codes in HG tree were already fixed (driver, lib and
tools).

Thanks for report and testing.

Takashi

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