All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ALSA: Fix capturing for envy1724ht based "audiophile 192"
@ 2008-04-19 22:52 Karsten Wiese
  2008-04-21 11:11 ` Takashi Iwai
  0 siblings, 1 reply; 7+ messages in thread
From: Karsten Wiese @ 2008-04-19 22:52 UTC (permalink / raw)
  To: alsa-devel


The 1st hunk changes snd_ice1712_gpio_write_bits() to not affect the gpio's mask
register. Lets gpio writers that are unaware of a changed mask register
successfully write the gpio.
The 2nd and 3rd hunks toggle "chip reset" and "eeprom based setup" sequence so
"chip reset" becomes first. Increasement of the "chip reset" related timeout
leads to correctly read eeprom's contents here.
Also corrects some arguments in calls to snd_ice1712_gpio_write_bits() from
ap192_set_rate_val().

Signed-off-by: Karsten Wiese <fzu@wemgehoertderstaat.de>
---

Hi,

patch might hamper cards that actually relied on snd_ice1712_gpio_write_bits()
setting the gpio's mask register. Cscope tells me terratec ews and m-audio
rev51 are might be candidates. I guess they are ok ;-)

regards,
      Karsten

diff -pur alsa.hg/alsa-kernel/pci/ice1712/ice1712.h fixed/alsa-kernel/pci/ice1712/ice1712.h
--- alsa.hg/alsa-kernel/pci/ice1712/ice1712.h	2008-03-20 12:25:01.000000000 +0100
+++ fixed/alsa-kernel/pci/ice1712/ice1712.h	2008-04-19 23:21:31.000000000 +0200
@@ -438,10 +438,14 @@ int snd_ice1712_gpio_put(struct snd_kcon
 static inline void snd_ice1712_gpio_write_bits(struct snd_ice1712 *ice,
 					       unsigned int mask, unsigned int bits)
 {
+	unsigned val;
+
 	ice->gpio.direction |= mask;
 	snd_ice1712_gpio_set_dir(ice, ice->gpio.direction);
-	snd_ice1712_gpio_set_mask(ice, ~mask);
-	snd_ice1712_gpio_write(ice, mask & bits);
+	val = snd_ice1712_gpio_read(ice);
+	val &= ~mask;
+	val |= mask & bits;
+	snd_ice1712_gpio_write(ice, val);
 }
 
 static inline int snd_ice1712_gpio_read_bits(struct snd_ice1712 *ice,
diff -pur alsa.hg/alsa-kernel/pci/ice1712/ice1724.c fixed/alsa-kernel/pci/ice1712/ice1724.c
--- alsa.hg/alsa-kernel/pci/ice1712/ice1724.c	2008-03-20 12:25:01.000000000 +0100
+++ fixed/alsa-kernel/pci/ice1712/ice1724.c	2008-04-19 23:07:29.000000000 +0200
@@ -2045,12 +2045,16 @@ static int __devinit snd_vt1724_read_eep
 
 
 
-static int __devinit snd_vt1724_chip_init(struct snd_ice1712 *ice)
+static void __devinit snd_vt1724_chip_reset(struct snd_ice1712 *ice)
 {
 	outb(VT1724_RESET , ICEREG1724(ice, CONTROL));
-	udelay(200);
+	msleep(10);
 	outb(0, ICEREG1724(ice, CONTROL));
-	udelay(200);
+	msleep(10);
+}
+
+static int __devinit snd_vt1724_chip_init(struct snd_ice1712 *ice)
+{
 	outb(ice->eeprom.data[ICE_EEP2_SYSCONF], ICEREG1724(ice, SYS_CFG));
 	outb(ice->eeprom.data[ICE_EEP2_ACLINK], ICEREG1724(ice, AC97_CFG));
 	outb(ice->eeprom.data[ICE_EEP2_I2S], ICEREG1724(ice, I2S_FEATURES));
@@ -2223,6 +2227,7 @@ static int __devinit snd_vt1724_create(s
 
 	ice->irq = pci->irq;
 
+	snd_vt1724_chip_reset(ice);
 	if (snd_vt1724_read_eeprom(ice, modelname) < 0) {
 		snd_vt1724_free(ice);
 		return -EIO;
diff -pur alsa.hg/alsa-kernel/pci/ice1712/revo.c fixed/alsa-kernel/pci/ice1712/revo.c
--- alsa.hg/alsa-kernel/pci/ice1712/revo.c	2008-03-17 09:25:02.000000000 +0100
+++ fixed/alsa-kernel/pci/ice1712/revo.c	2008-04-19 23:12:33.000000000 +0200
@@ -327,7 +327,7 @@ static void ap192_set_rate_val(struct sn
 	revo_set_rate_val(ak, rate);
 
 	/* reset CKS */
-	snd_ice1712_gpio_write_bits(ice, 1 << 8, rate > 96000 ? 1 : 0);
+	snd_ice1712_gpio_write_bits(ice, 1 << 8, rate > 96000 ? 1 << 8 : 0);
 	/* reset DFS pins of AK5385A for ADC, too */
 	if (rate > 96000)
 		dfs = 2;
@@ -338,7 +338,7 @@ static void ap192_set_rate_val(struct sn
 	snd_ice1712_gpio_write_bits(ice, 3 << 9, dfs << 9);
 	/* reset ADC */
 	snd_ice1712_gpio_write_bits(ice, 1 << 11, 0);
-	snd_ice1712_gpio_write_bits(ice, 1 << 11, 1);
+	snd_ice1712_gpio_write_bits(ice, 1 << 11, 1 << 11);
 }
 
 static const struct snd_akm4xxx_dac_channel ap192_dac[] = {

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] ALSA: Fix capturing for envy1724ht based "audiophile 192"
  2008-04-19 22:52 [PATCH] ALSA: Fix capturing for envy1724ht based "audiophile 192" Karsten Wiese
@ 2008-04-21 11:11 ` Takashi Iwai
  2008-04-21 20:39   ` Karsten Wiese
                     ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Takashi Iwai @ 2008-04-21 11:11 UTC (permalink / raw)
  To: Karsten Wiese; +Cc: alsa-devel

At Sun, 20 Apr 2008 00:52:51 +0200,
Karsten Wiese wrote:
> 
> The 1st hunk changes snd_ice1712_gpio_write_bits() to not affect the gpio's mask
> register. Lets gpio writers that are unaware of a changed mask register
> successfully write the gpio.
> The 2nd and 3rd hunks toggle "chip reset" and "eeprom based setup" sequence so
> "chip reset" becomes first. Increasement of the "chip reset" related timeout
> leads to correctly read eeprom's contents here.
> Also corrects some arguments in calls to snd_ice1712_gpio_write_bits() from
> ap192_set_rate_val().
> 
> Signed-off-by: Karsten Wiese <fzu@wemgehoertderstaat.de>

Thanks.  All changes look good to me, but I'd prefer separated
patches.  Could you split it to three patches and give a changelog for
each?


Takashi

> ---
> 
> Hi,
> 
> patch might hamper cards that actually relied on snd_ice1712_gpio_write_bits()
> setting the gpio's mask register. Cscope tells me terratec ews and m-audio
> rev51 are might be candidates. I guess they are ok ;-)
> 
> regards,
>       Karsten
> 
> diff -pur alsa.hg/alsa-kernel/pci/ice1712/ice1712.h fixed/alsa-kernel/pci/ice1712/ice1712.h
> --- alsa.hg/alsa-kernel/pci/ice1712/ice1712.h	2008-03-20 12:25:01.000000000 +0100
> +++ fixed/alsa-kernel/pci/ice1712/ice1712.h	2008-04-19 23:21:31.000000000 +0200
> @@ -438,10 +438,14 @@ int snd_ice1712_gpio_put(struct snd_kcon
>  static inline void snd_ice1712_gpio_write_bits(struct snd_ice1712 *ice,
>  					       unsigned int mask, unsigned int bits)
>  {
> +	unsigned val;
> +
>  	ice->gpio.direction |= mask;
>  	snd_ice1712_gpio_set_dir(ice, ice->gpio.direction);
> -	snd_ice1712_gpio_set_mask(ice, ~mask);
> -	snd_ice1712_gpio_write(ice, mask & bits);
> +	val = snd_ice1712_gpio_read(ice);
> +	val &= ~mask;
> +	val |= mask & bits;
> +	snd_ice1712_gpio_write(ice, val);
>  }
>  
>  static inline int snd_ice1712_gpio_read_bits(struct snd_ice1712 *ice,
> diff -pur alsa.hg/alsa-kernel/pci/ice1712/ice1724.c fixed/alsa-kernel/pci/ice1712/ice1724.c
> --- alsa.hg/alsa-kernel/pci/ice1712/ice1724.c	2008-03-20 12:25:01.000000000 +0100
> +++ fixed/alsa-kernel/pci/ice1712/ice1724.c	2008-04-19 23:07:29.000000000 +0200
> @@ -2045,12 +2045,16 @@ static int __devinit snd_vt1724_read_eep
>  
>  
>  
> -static int __devinit snd_vt1724_chip_init(struct snd_ice1712 *ice)
> +static void __devinit snd_vt1724_chip_reset(struct snd_ice1712 *ice)
>  {
>  	outb(VT1724_RESET , ICEREG1724(ice, CONTROL));
> -	udelay(200);
> +	msleep(10);
>  	outb(0, ICEREG1724(ice, CONTROL));
> -	udelay(200);
> +	msleep(10);
> +}
> +
> +static int __devinit snd_vt1724_chip_init(struct snd_ice1712 *ice)
> +{
>  	outb(ice->eeprom.data[ICE_EEP2_SYSCONF], ICEREG1724(ice, SYS_CFG));
>  	outb(ice->eeprom.data[ICE_EEP2_ACLINK], ICEREG1724(ice, AC97_CFG));
>  	outb(ice->eeprom.data[ICE_EEP2_I2S], ICEREG1724(ice, I2S_FEATURES));
> @@ -2223,6 +2227,7 @@ static int __devinit snd_vt1724_create(s
>  
>  	ice->irq = pci->irq;
>  
> +	snd_vt1724_chip_reset(ice);
>  	if (snd_vt1724_read_eeprom(ice, modelname) < 0) {
>  		snd_vt1724_free(ice);
>  		return -EIO;
> diff -pur alsa.hg/alsa-kernel/pci/ice1712/revo.c fixed/alsa-kernel/pci/ice1712/revo.c
> --- alsa.hg/alsa-kernel/pci/ice1712/revo.c	2008-03-17 09:25:02.000000000 +0100
> +++ fixed/alsa-kernel/pci/ice1712/revo.c	2008-04-19 23:12:33.000000000 +0200
> @@ -327,7 +327,7 @@ static void ap192_set_rate_val(struct sn
>  	revo_set_rate_val(ak, rate);
>  
>  	/* reset CKS */
> -	snd_ice1712_gpio_write_bits(ice, 1 << 8, rate > 96000 ? 1 : 0);
> +	snd_ice1712_gpio_write_bits(ice, 1 << 8, rate > 96000 ? 1 << 8 : 0);
>  	/* reset DFS pins of AK5385A for ADC, too */
>  	if (rate > 96000)
>  		dfs = 2;
> @@ -338,7 +338,7 @@ static void ap192_set_rate_val(struct sn
>  	snd_ice1712_gpio_write_bits(ice, 3 << 9, dfs << 9);
>  	/* reset ADC */
>  	snd_ice1712_gpio_write_bits(ice, 1 << 11, 0);
> -	snd_ice1712_gpio_write_bits(ice, 1 << 11, 1);
> +	snd_ice1712_gpio_write_bits(ice, 1 << 11, 1 << 11);
>  }
>  
>  static const struct snd_akm4xxx_dac_channel ap192_dac[] = {
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] ALSA: Fix capturing for envy1724ht based "audiophile 192"
  2008-04-21 11:11 ` Takashi Iwai
@ 2008-04-21 20:39   ` Karsten Wiese
  2008-04-21 20:45   ` [PATCH] ice1724.c: toggle "chip reset" and "eeprom based setup" sequence Karsten Wiese
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: Karsten Wiese @ 2008-04-21 20:39 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: alsa-devel

Am Montag, 21. April 2008 schrieb Takashi Iwai:
> Could you split it to three patches and give a changelog for
> each?

Yes.

regards,
      Karsten 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH] ice1724.c: toggle "chip reset" and "eeprom based setup" sequence
  2008-04-21 11:11 ` Takashi Iwai
  2008-04-21 20:39   ` Karsten Wiese
@ 2008-04-21 20:45   ` Karsten Wiese
  2008-04-22 10:14     ` Takashi Iwai
  2008-04-21 20:45   ` [PATCH] Don't set gpio mask register in snd_ice1712_gpio_write_bits() Karsten Wiese
  2008-04-21 20:45   ` [PATCH] Audiophile 192: Fix ad converter initialization Karsten Wiese
  3 siblings, 1 reply; 7+ messages in thread
From: Karsten Wiese @ 2008-04-21 20:45 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: alsa-devel


Let "chip reset" become first. Increasement of the "chip reset" related timeout
leads to correctly read eeprom's contents here.

Signed-off-by: Karsten Wiese <fzu@wemgehoertderstaat.de>



diff -pur alsa.hg/alsa-kernel/pci/ice1712/ice1724.c fixed/alsa-kernel/pci/ice1712/ice1724.c
--- alsa.hg/alsa-kernel/pci/ice1712/ice1724.c	2008-03-20 12:25:01.000000000 +0100
+++ fixed/alsa-kernel/pci/ice1712/ice1724.c	2008-04-19 23:07:29.000000000 +0200
@@ -2045,12 +2045,16 @@ static int __devinit snd_vt1724_read_eep
 
 
 
-static int __devinit snd_vt1724_chip_init(struct snd_ice1712 *ice)
+static void __devinit snd_vt1724_chip_reset(struct snd_ice1712 *ice)
 {
 	outb(VT1724_RESET , ICEREG1724(ice, CONTROL));
-	udelay(200);
+	msleep(10);
 	outb(0, ICEREG1724(ice, CONTROL));
-	udelay(200);
+	msleep(10);
+}
+
+static int __devinit snd_vt1724_chip_init(struct snd_ice1712 *ice)
+{
 	outb(ice->eeprom.data[ICE_EEP2_SYSCONF], ICEREG1724(ice, SYS_CFG));
 	outb(ice->eeprom.data[ICE_EEP2_ACLINK], ICEREG1724(ice, AC97_CFG));
 	outb(ice->eeprom.data[ICE_EEP2_I2S], ICEREG1724(ice, I2S_FEATURES));
@@ -2223,6 +2227,7 @@ static int __devinit snd_vt1724_create(s
 
 	ice->irq = pci->irq;
 
+	snd_vt1724_chip_reset(ice);
 	if (snd_vt1724_read_eeprom(ice, modelname) < 0) {
 		snd_vt1724_free(ice);
 		return -EIO;

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH] Don't set gpio mask register in snd_ice1712_gpio_write_bits()
  2008-04-21 11:11 ` Takashi Iwai
  2008-04-21 20:39   ` Karsten Wiese
  2008-04-21 20:45   ` [PATCH] ice1724.c: toggle "chip reset" and "eeprom based setup" sequence Karsten Wiese
@ 2008-04-21 20:45   ` Karsten Wiese
  2008-04-21 20:45   ` [PATCH] Audiophile 192: Fix ad converter initialization Karsten Wiese
  3 siblings, 0 replies; 7+ messages in thread
From: Karsten Wiese @ 2008-04-21 20:45 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: alsa-devel


Some calls to snd_ice1712_gpio_write() go wrong, if
snd_ice1712_gpio_write_bits() ran before and changed the gpio mask register.
Read the actual gpio value and combine it with the to be set bits in the cpu
instead.

Signed-off-by: Karsten Wiese <fzu@wemgehoertderstaat.de>


diff -pur alsa.hg/alsa-kernel/pci/ice1712/ice1712.h fixed/alsa-kernel/pci/ice1712/ice1712.h
--- alsa.hg/alsa-kernel/pci/ice1712/ice1712.h	2008-03-20 12:25:01.000000000 +0100
+++ fixed/alsa-kernel/pci/ice1712/ice1712.h	2008-04-19 23:21:31.000000000 +0200
@@ -438,10 +438,14 @@ int snd_ice1712_gpio_put(struct snd_kcon
 static inline void snd_ice1712_gpio_write_bits(struct snd_ice1712 *ice,
 					       unsigned int mask, unsigned int bits)
 {
+	unsigned val;
+
 	ice->gpio.direction |= mask;
 	snd_ice1712_gpio_set_dir(ice, ice->gpio.direction);
-	snd_ice1712_gpio_set_mask(ice, ~mask);
-	snd_ice1712_gpio_write(ice, mask & bits);
+	val = snd_ice1712_gpio_read(ice);
+	val &= ~mask;
+	val |= mask & bits;
+	snd_ice1712_gpio_write(ice, val);
 }
 
 static inline int snd_ice1712_gpio_read_bits(struct snd_ice1712 *ice,

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH] Audiophile 192: Fix ad converter initialization
  2008-04-21 11:11 ` Takashi Iwai
                     ` (2 preceding siblings ...)
  2008-04-21 20:45   ` [PATCH] Don't set gpio mask register in snd_ice1712_gpio_write_bits() Karsten Wiese
@ 2008-04-21 20:45   ` Karsten Wiese
  3 siblings, 0 replies; 7+ messages in thread
From: Karsten Wiese @ 2008-04-21 20:45 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: alsa-devel


Correct some arguments in calls to snd_ice1712_gpio_write_bits() from
ap192_set_rate_val().

Signed-off-by: Karsten Wiese <fzu@wemgehoertderstaat.de>



diff -pur alsa.hg/alsa-kernel/pci/ice1712/revo.c fixed/alsa-kernel/pci/ice1712/revo.c
--- alsa.hg/alsa-kernel/pci/ice1712/revo.c	2008-03-17 09:25:02.000000000 +0100
+++ fixed/alsa-kernel/pci/ice1712/revo.c	2008-04-19 23:12:33.000000000 +0200
@@ -327,7 +327,7 @@ static void ap192_set_rate_val(struct sn
 	revo_set_rate_val(ak, rate);
 
 	/* reset CKS */
-	snd_ice1712_gpio_write_bits(ice, 1 << 8, rate > 96000 ? 1 : 0);
+	snd_ice1712_gpio_write_bits(ice, 1 << 8, rate > 96000 ? 1 << 8 : 0);
 	/* reset DFS pins of AK5385A for ADC, too */
 	if (rate > 96000)
 		dfs = 2;
@@ -338,7 +338,7 @@ static void ap192_set_rate_val(struct sn
 	snd_ice1712_gpio_write_bits(ice, 3 << 9, dfs << 9);
 	/* reset ADC */
 	snd_ice1712_gpio_write_bits(ice, 1 << 11, 0);
-	snd_ice1712_gpio_write_bits(ice, 1 << 11, 1);
+	snd_ice1712_gpio_write_bits(ice, 1 << 11, 1 << 11);
 }
 
 static const struct snd_akm4xxx_dac_channel ap192_dac[] = {

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] ice1724.c: toggle "chip reset" and "eeprom based setup" sequence
  2008-04-21 20:45   ` [PATCH] ice1724.c: toggle "chip reset" and "eeprom based setup" sequence Karsten Wiese
@ 2008-04-22 10:14     ` Takashi Iwai
  0 siblings, 0 replies; 7+ messages in thread
From: Takashi Iwai @ 2008-04-22 10:14 UTC (permalink / raw)
  To: Karsten Wiese; +Cc: alsa-devel

At Mon, 21 Apr 2008 22:45:03 +0200,
Karsten Wiese wrote:
> 
> 
> Let "chip reset" become first. Increasement of the "chip reset" related timeout
> leads to correctly read eeprom's contents here.
> 
> Signed-off-by: Karsten Wiese <fzu@wemgehoertderstaat.de>

Thanks, applied all three patches now.


Takashi

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2008-04-22 10:14 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-04-19 22:52 [PATCH] ALSA: Fix capturing for envy1724ht based "audiophile 192" Karsten Wiese
2008-04-21 11:11 ` Takashi Iwai
2008-04-21 20:39   ` Karsten Wiese
2008-04-21 20:45   ` [PATCH] ice1724.c: toggle "chip reset" and "eeprom based setup" sequence Karsten Wiese
2008-04-22 10:14     ` Takashi Iwai
2008-04-21 20:45   ` [PATCH] Don't set gpio mask register in snd_ice1712_gpio_write_bits() Karsten Wiese
2008-04-21 20:45   ` [PATCH] Audiophile 192: Fix ad converter initialization Karsten Wiese

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.