[PATCH] sgalaxy: checkpatch fixes

[PATCH] sgalaxy: checkpatch fixes

[Krzysztof Helt]

From: Krzysztof Helt <krzysztof.h1@wp.pl>

This patch fixes white spaces and errors pointed by
the checkpatch.pl script.

Signed-off-by: Krzysztof Helt <krzysztof.h1@wp.pl>
---
A nice side effect: sgalaxy.c gets shorter.

diff -urp linux-2.6.23.orig/sound/isa/sgalaxy.c linux-2.6.23/sound/isa/sgalaxy.c
--- linux-2.6.23.orig/sound/isa/sgalaxy.c	2007-07-09 01:32:17.000000000 +0200
+++ linux-2.6.23/sound/isa/sgalaxy.c	2007-09-04 23:41:11.860030310 +0200
@@ -47,7 +47,8 @@ static int index[SNDRV_CARDS] = SNDRV_DE
 static char *id[SNDRV_CARDS] = SNDRV_DEFAULT_STR;	/* ID for this card */
 static int enable[SNDRV_CARDS] = SNDRV_DEFAULT_ENABLE;	/* Enable this card */
 static long sbport[SNDRV_CARDS] = SNDRV_DEFAULT_PORT;	/* 0x220,0x240 */
-static long wssport[SNDRV_CARDS] = SNDRV_DEFAULT_PORT;	/* 0x530,0xe80,0xf40,0x604 */
+static long wssport[SNDRV_CARDS] = SNDRV_DEFAULT_PORT;
+						/* 0x530,0xe80,0xf40,0x604 */
 static int irq[SNDRV_CARDS] = SNDRV_DEFAULT_IRQ;	/* 7,9,10,11 */
 static int dma1[SNDRV_CARDS] = SNDRV_DEFAULT_DMA;	/* 0,1,3 */
 
@@ -69,14 +70,10 @@ MODULE_PARM_DESC(dma1, "DMA1 # for Sound
 
 #define PFX	"sgalaxy: "
 
-/*
+#define AD1848P1(port, x) (port + c_d_c_AD1848##x)
 
- */
-
-#define AD1848P1( port, x ) ( port + c_d_c_AD1848##x )
-
-/* from lowlevel/sb/sb.c - to avoid having to allocate a struct snd_sb for the */
-/* short time we actually need it.. */
+/* from lowlevel/sb/sb.c - to avoid having to allocate a struct snd_sb */
+/* for the short time we actually need it.. */
 
 static int snd_sgalaxy_sbdsp_reset(unsigned long port)
 {
@@ -98,7 +95,7 @@ static int __devinit snd_sgalaxy_sbdsp_c
 					       unsigned char val)
 {
 	int i;
-       	
+
 	for (i = 10000; i; i--)
 		if ((inb(SBP1(port, STATUS)) & 0x80) == 0) {
 			outb(val, SBP1(port, COMMAND));
@@ -115,45 +112,39 @@ static irqreturn_t snd_sgalaxy_dummy_int
 
 static int __devinit snd_sgalaxy_setup_wss(unsigned long port, int irq, int dma)
 {
-	static int interrupt_bits[] = {-1, -1, -1, -1, -1, -1, -1, 0x08, -1, 
+	static int interrupt_bits[] = {-1, -1, -1, -1, -1, -1, -1, 0x08, -1,
 				       0x10, 0x18, 0x20, -1, -1, -1, -1};
 	static int dma_bits[] = {1, 2, 0, 3};
 	int tmp, tmp1;
 
-	if ((tmp = inb(port + 3)) == 0xff)
-	{
+	tmp = inb(port + 3);
+	if (tmp == 0xff) {
 		snd_printdd("I/O address dead (0x%lx)\n", port);
 		return 0;
 	}
-#if 0
-	snd_printdd("WSS signature = 0x%x\n", tmp);
-#endif
 
-        if ((tmp & 0x3f) != 0x04 &&
-            (tmp & 0x3f) != 0x0f &&
-            (tmp & 0x3f) != 0x00) {
+	tmp &= 0x3f;
+	if (tmp != 0x04 && tmp != 0x0f && tmp != 0x00) {
 		snd_printdd("No WSS signature detected on port 0x%lx\n",
 			    port + 3);
 		return 0;
 	}
 
-#if 0
-	snd_printdd(PFX "setting up IRQ/DMA for WSS\n");
-#endif
-
-        /* initialize IRQ for WSS codec */
-        tmp = interrupt_bits[irq % 16];
-        if (tmp < 0)
-                return -EINVAL;
-
-	if (request_irq(irq, snd_sgalaxy_dummy_interrupt, IRQF_DISABLED, "sgalaxy", NULL)) {
+	/* initialize IRQ for WSS codec */
+	tmp = interrupt_bits[irq % 16];
+	if (tmp < 0)
+		return -EINVAL;
+
+	tmp = request_irq(irq, snd_sgalaxy_dummy_interrupt, IRQF_DISABLED,
+			  "sgalaxy", NULL);
+	if (tmp) {
 		snd_printk(KERN_ERR "sgalaxy: can't grab irq %d\n", irq);
 		return -EIO;
 	}
 
-        outb(tmp | 0x40, port);
-        tmp1 = dma_bits[dma % 4];
-        outb(tmp | tmp1, port);
+	outb(tmp | 0x40, port);
+	tmp1 = dma_bits[dma % 4];
+	outb(tmp | tmp1, port);
 
 	free_irq(irq, NULL);
 
@@ -162,10 +153,6 @@ static int __devinit snd_sgalaxy_setup_w
 
 static int __devinit snd_sgalaxy_detect(int dev, int irq, int dma)
 {
-#if 0
-	snd_printdd(PFX "switching to WSS mode\n");
-#endif
-
 	/* switch to WSS mode */
 	snd_sgalaxy_sbdsp_reset(sbport[dev]);
 
@@ -177,8 +164,10 @@ static int __devinit snd_sgalaxy_detect(
 }
 
 static struct ad1848_mix_elem snd_sgalaxy_controls[] = {
-AD1848_DOUBLE("Aux Playback Switch", 0, SGALAXY_AUXC_LEFT, SGALAXY_AUXC_RIGHT, 7, 7, 1, 1),
-AD1848_DOUBLE("Aux Playback Volume", 0, SGALAXY_AUXC_LEFT, SGALAXY_AUXC_RIGHT, 0, 0, 31, 0)
+AD1848_DOUBLE("Aux Playback Switch", 0, SGALAXY_AUXC_LEFT, SGALAXY_AUXC_RIGHT,
+		7, 7, 1, 1),
+AD1848_DOUBLE("Aux Playback Volume", 0, SGALAXY_AUXC_LEFT, SGALAXY_AUXC_RIGHT,
+		0, 0, 31, 0)
 };
 
 static int __devinit snd_sgalaxy_mixer(struct snd_ad1848 *chip)
@@ -190,28 +179,34 @@ static int __devinit snd_sgalaxy_mixer(s
 
 	memset(&id1, 0, sizeof(id1));
 	memset(&id2, 0, sizeof(id2));
-	id1.iface = id2.iface = SNDRV_CTL_ELEM_IFACE_MIXER;
+	id1.iface = SNDRV_CTL_ELEM_IFACE_MIXER;
+	id2.iface = SNDRV_CTL_ELEM_IFACE_MIXER;
 	/* reassign AUX0 to LINE */
 	strcpy(id1.name, "Aux Playback Switch");
 	strcpy(id2.name, "Line Playback Switch");
-	if ((err = snd_ctl_rename_id(card, &id1, &id2)) < 0)
+	err = snd_ctl_rename_id(card, &id1, &id2);
+	if (err < 0)
 		return err;
 	strcpy(id1.name, "Aux Playback Volume");
 	strcpy(id2.name, "Line Playback Volume");
-	if ((err = snd_ctl_rename_id(card, &id1, &id2)) < 0)
+	err = snd_ctl_rename_id(card, &id1, &id2);
+	if (err < 0)
 		return err;
 	/* reassign AUX1 to FM */
 	strcpy(id1.name, "Aux Playback Switch"); id1.index = 1;
 	strcpy(id2.name, "FM Playback Switch");
-	if ((err = snd_ctl_rename_id(card, &id1, &id2)) < 0)
+	err = snd_ctl_rename_id(card, &id1, &id2);
+	if (err < 0)
 		return err;
 	strcpy(id1.name, "Aux Playback Volume");
 	strcpy(id2.name, "FM Playback Volume");
-	if ((err = snd_ctl_rename_id(card, &id1, &id2)) < 0)
+	err = snd_ctl_rename_id(card, &id1, &id2);
+	if (err < 0)
 		return err;
 	/* build AUX2 input */
 	for (idx = 0; idx < ARRAY_SIZE(snd_sgalaxy_controls); idx++) {
-		if ((err = snd_ad1848_add_ctl_elem(chip, &snd_sgalaxy_controls[idx])) < 0)
+		err = snd_ad1848_add_ctl_elem(chip, &snd_sgalaxy_controls[idx]);
+		if (err < 0)
 			return err;
 	}
 	return 0;
@@ -241,44 +236,50 @@ static int __devinit snd_sgalaxy_probe(s
 	struct snd_ad1848 *chip;
 
 	card = snd_card_new(index[dev], id[dev], THIS_MODULE, 0);
-	if (card == NULL)
+	if (!card)
 		return -ENOMEM;
 
 	xirq = irq[dev];
 	if (xirq == SNDRV_AUTO_IRQ) {
-		if ((xirq = snd_legacy_find_free_irq(possible_irqs)) < 0) {
+		xirq = snd_legacy_find_free_irq(possible_irqs);
+		if (xirq < 0) {
 			snd_printk(KERN_ERR PFX "unable to find a free IRQ\n");
 			err = -EBUSY;
 			goto _err;
 		}
 	}
 	xdma1 = dma1[dev];
-        if (xdma1 == SNDRV_AUTO_DMA) {
-		if ((xdma1 = snd_legacy_find_free_dma(possible_dmas)) < 0) {
+	if (xdma1 == SNDRV_AUTO_DMA) {
+		xdma1 = snd_legacy_find_free_dma(possible_dmas);
+		if (xdma1 < 0) {
 			snd_printk(KERN_ERR PFX "unable to find a free DMA\n");
 			err = -EBUSY;
 			goto _err;
 		}
 	}
 
-	if ((err = snd_sgalaxy_detect(dev, xirq, xdma1)) < 0)
+	err = snd_sgalaxy_detect(dev, xirq, xdma1);
+	if (err < 0)
 		goto _err;
 
-	if ((err = snd_ad1848_create(card, wssport[dev] + 4,
-				     xirq, xdma1,
-				     AD1848_HW_DETECT, &chip)) < 0)
+	err = snd_ad1848_create(card, wssport[dev] + 4, xirq, xdma1,
+				AD1848_HW_DETECT, &chip);
+	if (err < 0)
 		goto _err;
 	card->private_data = chip;
 
-	if ((err = snd_ad1848_pcm(chip, 0, NULL)) < 0) {
+	err = snd_ad1848_pcm(chip, 0, NULL);
+	if (err < 0) {
 		snd_printdd(PFX "error creating new ad1848 PCM device\n");
 		goto _err;
 	}
-	if ((err = snd_ad1848_mixer(chip)) < 0) {
+	err = snd_ad1848_mixer(chip);
+	if (err < 0) {
 		snd_printdd(PFX "error creating new ad1848 mixer\n");
 		goto _err;
 	}
-	if ((err = snd_sgalaxy_mixer(chip)) < 0) {
+	err = snd_sgalaxy_mixer(chip);
+	if (err < 0) {
 		snd_printdd(PFX "the mixer rewrite failed\n");
 		goto _err;
 	}
@@ -290,7 +291,8 @@ static int __devinit snd_sgalaxy_probe(s
 
 	snd_card_set_dev(card, devptr);
 
-	if ((err = snd_card_register(card)) < 0)
+	err = snd_card_register(card);
+	if (err < 0)
 		goto _err;
 
 	dev_set_drvdata(devptr, card);
@@ -327,7 +329,8 @@ static int snd_sgalaxy_resume(struct dev
 
 	chip->resume(chip);
 	snd_ad1848_out(chip, SGALAXY_AUXC_LEFT, chip->image[SGALAXY_AUXC_LEFT]);
-	snd_ad1848_out(chip, SGALAXY_AUXC_RIGHT, chip->image[SGALAXY_AUXC_RIGHT]);
+	snd_ad1848_out(chip, SGALAXY_AUXC_RIGHT,
+			chip->image[SGALAXY_AUXC_RIGHT]);
 
 	snd_power_change_state(card, SNDRV_CTL_POWER_D0);
 	return 0;

Re: [PATCH] sgalaxy: checkpatch fixes

[Rene Herman]

On 09/04/2007 11:49 PM, Krzysztof Helt wrote:

A few comments, but also note that while I've been "dragging my feet" (grin) 
on getting the thing tested on more cards, I've a new driver for these cards 
in the works. Was already posted a while ago -- just need to clean it up, 
integrate some more and then test and submit it for real...

> From: Krzysztof Helt <krzysztof.h1@wp.pl>
> 
> This patch fixes white spaces and errors pointed by
> the checkpatch.pl script.

That thing is not actually intended for existing code...

> Signed-off-by: Krzysztof Helt <krzysztof.h1@wp.pl>
> ---
> A nice side effect: sgalaxy.c gets shorter.
> 
> diff -urp linux-2.6.23.orig/sound/isa/sgalaxy.c linux-2.6.23/sound/isa/sgalaxy.c
> --- linux-2.6.23.orig/sound/isa/sgalaxy.c	2007-07-09 01:32:17.000000000 +0200
> +++ linux-2.6.23/sound/isa/sgalaxy.c	2007-09-04 23:41:11.860030310 +0200
> @@ -47,7 +47,8 @@ static int index[SNDRV_CARDS] = SNDRV_DE
>  static char *id[SNDRV_CARDS] = SNDRV_DEFAULT_STR;	/* ID for this card */
>  static int enable[SNDRV_CARDS] = SNDRV_DEFAULT_ENABLE;	/* Enable this card */
>  static long sbport[SNDRV_CARDS] = SNDRV_DEFAULT_PORT;	/* 0x220,0x240 */
> -static long wssport[SNDRV_CARDS] = SNDRV_DEFAULT_PORT;	/* 0x530,0xe80,0xf40,0x604 */
> +static long wssport[SNDRV_CARDS] = SNDRV_DEFAULT_PORT;
> +						/* 0x530,0xe80,0xf40,0x604 */

Please don't. The idea of coding style fixes is to make things _easier_ to read.

>  static int irq[SNDRV_CARDS] = SNDRV_DEFAULT_IRQ;	/* 7,9,10,11 */
>  static int dma1[SNDRV_CARDS] = SNDRV_DEFAULT_DMA;	/* 0,1,3 */
>  
> @@ -69,14 +70,10 @@ MODULE_PARM_DESC(dma1, "DMA1 # for Sound
>  
>  #define PFX	"sgalaxy: "
>  
> -/*
> +#define AD1848P1(port, x) (port + c_d_c_AD1848##x)
>  
> - */
> -
> -#define AD1848P1( port, x ) ( port + c_d_c_AD1848##x )
> -
> -/* from lowlevel/sb/sb.c - to avoid having to allocate a struct snd_sb for the */
> -/* short time we actually need it.. */
> +/* from lowlevel/sb/sb.c - to avoid having to allocate a struct snd_sb */
> +/* for the short time we actually need it.. */
>  
>  static int snd_sgalaxy_sbdsp_reset(unsigned long port)
>  {
> @@ -98,7 +95,7 @@ static int __devinit snd_sgalaxy_sbdsp_c
>  					       unsigned char val)
>  {
>  	int i;
> -       	
> +
>  	for (i = 10000; i; i--)
>  		if ((inb(SBP1(port, STATUS)) & 0x80) == 0) {
>  			outb(val, SBP1(port, COMMAND));
> @@ -115,45 +112,39 @@ static irqreturn_t snd_sgalaxy_dummy_int
>  
>  static int __devinit snd_sgalaxy_setup_wss(unsigned long port, int irq, int dma)
>  {
> -	static int interrupt_bits[] = {-1, -1, -1, -1, -1, -1, -1, 0x08, -1, 
> +	static int interrupt_bits[] = {-1, -1, -1, -1, -1, -1, -1, 0x08, -1,
>  				       0x10, 0x18, 0x20, -1, -1, -1, -1};
>  	static int dma_bits[] = {1, 2, 0, 3};
>  	int tmp, tmp1;
>  
> -	if ((tmp = inb(port + 3)) == 0xff)
> -	{
> +	tmp = inb(port + 3);
> +	if (tmp == 0xff) {
>  		snd_printdd("I/O address dead (0x%lx)\n", port);
>  		return 0;
>  	}
> -#if 0
> -	snd_printdd("WSS signature = 0x%x\n", tmp);
> -#endif

Please don't just kill debug code. It's very useful for the next person 
stepping in -- it's "functional commentary".

> -        if ((tmp & 0x3f) != 0x04 &&
> -            (tmp & 0x3f) != 0x0f &&
> -            (tmp & 0x3f) != 0x00) {
> +	tmp &= 0x3f;
> +	if (tmp != 0x04 && tmp != 0x0f && tmp != 0x00) {
>  		snd_printdd("No WSS signature detected on port 0x%lx\n",
>  			    port + 3);
>  		return 0;
>  	}
>  
> -#if 0
> -	snd_printdd(PFX "setting up IRQ/DMA for WSS\n");
> -#endif

Same.

> -
> -        /* initialize IRQ for WSS codec */
> -        tmp = interrupt_bits[irq % 16];
> -        if (tmp < 0)
> -                return -EINVAL;
> -
> -	if (request_irq(irq, snd_sgalaxy_dummy_interrupt, IRQF_DISABLED, "sgalaxy", NULL)) {
> +	/* initialize IRQ for WSS codec */
> +	tmp = interrupt_bits[irq % 16];
> +	if (tmp < 0)
> +		return -EINVAL;
> +
> +	tmp = request_irq(irq, snd_sgalaxy_dummy_interrupt, IRQF_DISABLED,
> +			  "sgalaxy", NULL);
> +	if (tmp) {

NAK -- you just overwrote tmp with the return value from request_irq() 
meaning we're now poking garbage:

>  		snd_printk(KERN_ERR "sgalaxy: can't grab irq %d\n", irq);
>  		return -EIO;
>  	}
>  
> -        outb(tmp | 0x40, port);
> -        tmp1 = dma_bits[dma % 4];
> -        outb(tmp | tmp1, port);
> +	outb(tmp | 0x40, port);
> +	tmp1 = dma_bits[dma % 4];
> +	outb(tmp | tmp1, port);
>  
>  	free_irq(irq, NULL);
>  
> @@ -162,10 +153,6 @@ static int __devinit snd_sgalaxy_setup_w
>  
>  static int __devinit snd_sgalaxy_detect(int dev, int irq, int dma)
>  {
> -#if 0
> -	snd_printdd(PFX "switching to WSS mode\n");
> -#endif

Same.

> -
>  	/* switch to WSS mode */
>  	snd_sgalaxy_sbdsp_reset(sbport[dev]);
>  
> @@ -177,8 +164,10 @@ static int __devinit snd_sgalaxy_detect(
>  }
>  
>  static struct ad1848_mix_elem snd_sgalaxy_controls[] = {
> -AD1848_DOUBLE("Aux Playback Switch", 0, SGALAXY_AUXC_LEFT, SGALAXY_AUXC_RIGHT, 7, 7, 1, 1),
> -AD1848_DOUBLE("Aux Playback Volume", 0, SGALAXY_AUXC_LEFT, SGALAXY_AUXC_RIGHT, 0, 0, 31, 0)
> +AD1848_DOUBLE("Aux Playback Switch", 0, SGALAXY_AUXC_LEFT, SGALAXY_AUXC_RIGHT,
> +		7, 7, 1, 1),
> +AD1848_DOUBLE("Aux Playback Volume", 0, SGALAXY_AUXC_LEFT, SGALAXY_AUXC_RIGHT,
> +		0, 0, 31, 0)

I'd leave these on single lines again.

>  };
>  
>  static int __devinit snd_sgalaxy_mixer(struct snd_ad1848 *chip)
> @@ -190,28 +179,34 @@ static int __devinit snd_sgalaxy_mixer(s
>  
>  	memset(&id1, 0, sizeof(id1));
>  	memset(&id2, 0, sizeof(id2));
> -	id1.iface = id2.iface = SNDRV_CTL_ELEM_IFACE_MIXER;
> +	id1.iface = SNDRV_CTL_ELEM_IFACE_MIXER;
> +	id2.iface = SNDRV_CTL_ELEM_IFACE_MIXER;
>  	/* reassign AUX0 to LINE */
>  	strcpy(id1.name, "Aux Playback Switch");
>  	strcpy(id2.name, "Line Playback Switch");
> -	if ((err = snd_ctl_rename_id(card, &id1, &id2)) < 0)
> +	err = snd_ctl_rename_id(card, &id1, &id2);
> +	if (err < 0)
>  		return err;
>  	strcpy(id1.name, "Aux Playback Volume");
>  	strcpy(id2.name, "Line Playback Volume");
> -	if ((err = snd_ctl_rename_id(card, &id1, &id2)) < 0)
> +	err = snd_ctl_rename_id(card, &id1, &id2);
> +	if (err < 0)
>  		return err;
>  	/* reassign AUX1 to FM */
>  	strcpy(id1.name, "Aux Playback Switch"); id1.index = 1;
>  	strcpy(id2.name, "FM Playback Switch");
> -	if ((err = snd_ctl_rename_id(card, &id1, &id2)) < 0)
> +	err = snd_ctl_rename_id(card, &id1, &id2);
> +	if (err < 0)
>  		return err;
>  	strcpy(id1.name, "Aux Playback Volume");
>  	strcpy(id2.name, "FM Playback Volume");
> -	if ((err = snd_ctl_rename_id(card, &id1, &id2)) < 0)
> +	err = snd_ctl_rename_id(card, &id1, &id2);
> +	if (err < 0)
>  		return err;
>  	/* build AUX2 input */
>  	for (idx = 0; idx < ARRAY_SIZE(snd_sgalaxy_controls); idx++) {
> -		if ((err = snd_ad1848_add_ctl_elem(chip, &snd_sgalaxy_controls[idx])) < 0)
> +		err = snd_ad1848_add_ctl_elem(chip, &snd_sgalaxy_controls[idx]);
> +		if (err < 0)
>  			return err;
>  	}
>  	return 0;
> @@ -241,44 +236,50 @@ static int __devinit snd_sgalaxy_probe(s
>  	struct snd_ad1848 *chip;
>  
>  	card = snd_card_new(index[dev], id[dev], THIS_MODULE, 0);
> -	if (card == NULL)
> +	if (!card)
>  		return -ENOMEM;
>  
>  	xirq = irq[dev];
>  	if (xirq == SNDRV_AUTO_IRQ) {
> -		if ((xirq = snd_legacy_find_free_irq(possible_irqs)) < 0) {
> +		xirq = snd_legacy_find_free_irq(possible_irqs);
> +		if (xirq < 0) {
>  			snd_printk(KERN_ERR PFX "unable to find a free IRQ\n");
>  			err = -EBUSY;
>  			goto _err;
>  		}
>  	}
>  	xdma1 = dma1[dev];
> -        if (xdma1 == SNDRV_AUTO_DMA) {
> -		if ((xdma1 = snd_legacy_find_free_dma(possible_dmas)) < 0) {
> +	if (xdma1 == SNDRV_AUTO_DMA) {
> +		xdma1 = snd_legacy_find_free_dma(possible_dmas);
> +		if (xdma1 < 0) {
>  			snd_printk(KERN_ERR PFX "unable to find a free DMA\n");
>  			err = -EBUSY;
>  			goto _err;
>  		}
>  	}
>  
> -	if ((err = snd_sgalaxy_detect(dev, xirq, xdma1)) < 0)
> +	err = snd_sgalaxy_detect(dev, xirq, xdma1);
> +	if (err < 0)
>  		goto _err;
>  
> -	if ((err = snd_ad1848_create(card, wssport[dev] + 4,
> -				     xirq, xdma1,
> -				     AD1848_HW_DETECT, &chip)) < 0)
> +	err = snd_ad1848_create(card, wssport[dev] + 4, xirq, xdma1,
> +				AD1848_HW_DETECT, &chip);
> +	if (err < 0)
>  		goto _err;
>  	card->private_data = chip;
>  
> -	if ((err = snd_ad1848_pcm(chip, 0, NULL)) < 0) {
> +	err = snd_ad1848_pcm(chip, 0, NULL);
> +	if (err < 0) {
>  		snd_printdd(PFX "error creating new ad1848 PCM device\n");
>  		goto _err;
>  	}
> -	if ((err = snd_ad1848_mixer(chip)) < 0) {
> +	err = snd_ad1848_mixer(chip);
> +	if (err < 0) {
>  		snd_printdd(PFX "error creating new ad1848 mixer\n");
>  		goto _err;
>  	}
> -	if ((err = snd_sgalaxy_mixer(chip)) < 0) {
> +	err = snd_sgalaxy_mixer(chip);
> +	if (err < 0) {
>  		snd_printdd(PFX "the mixer rewrite failed\n");
>  		goto _err;
>  	}
> @@ -290,7 +291,8 @@ static int __devinit snd_sgalaxy_probe(s
>  
>  	snd_card_set_dev(card, devptr);
>  
> -	if ((err = snd_card_register(card)) < 0)
> +	err = snd_card_register(card);
> +	if (err < 0)
>  		goto _err;
>  
>  	dev_set_drvdata(devptr, card);
> @@ -327,7 +329,8 @@ static int snd_sgalaxy_resume(struct dev
>  
>  	chip->resume(chip);
>  	snd_ad1848_out(chip, SGALAXY_AUXC_LEFT, chip->image[SGALAXY_AUXC_LEFT]);
> -	snd_ad1848_out(chip, SGALAXY_AUXC_RIGHT, chip->image[SGALAXY_AUXC_RIGHT]);
> +	snd_ad1848_out(chip, SGALAXY_AUXC_RIGHT,
> +			chip->image[SGALAXY_AUXC_RIGHT]);

Definitely would keep this on one line.

>  
>  	snd_power_change_state(card, SNDRV_CTL_POWER_D0);
>  	return 0;

Rene.

Re: [PATCH] sgalaxy: checkpatch fixes

[Krzysztof Helt]

On Wed, 05 Sep 2007 00:24:33 +0200
Rene Herman <rene.herman@gmail.com> wrote:

> On 09/04/2007 11:49 PM, Krzysztof Helt wrote:
> 
> A few comments, but also note that while I've been "dragging my feet" (grin) 
> on getting the thing tested on more cards, I've a new driver for these cards 
> in the works. Was already posted a while ago -- just need to clean it up, 
> integrate some more and then test and submit it for real...
> 

I about acquring the SG Waverider card. I saw your driver and thought
about asking you to cooperate to merge you driver into the sgalaxy or polish
the driver more. You are just one step before me.

> > From: Krzysztof Helt <krzysztof.h1@wp.pl>
> > 
> > This patch fixes white spaces and errors pointed by
> > the checkpatch.pl script.
> 
> That thing is not actually intended for existing code...
> 

Yes, but as I am about merging your patch into the sgalaxy
(my first idea).
I want to avoid situation with patches being rejected due
 to coding style problem in context lines.

Regards,
Krzysztof

Re: [PATCH] sgalaxy: checkpatch fixes

[Rene Herman]

On 09/05/2007 06:47 AM, Krzysztof Helt wrote:

> I about acquring the SG Waverider card. I saw your driver and thought 
> about asking you to cooperate to merge you driver into the sgalaxy or
> polish the driver more. You are just one step before me.

Ah, recently grabbed a Waverider as well. I now have some 15 Sound Galaxy 
models I believe which is partly why testing them all is taking too 
much/long ;-)

Will make time now though.

> Yes, but as I am about merging your patch into the sgalaxy
> (my first idea).
> I want to avoid situation with patches being rejected due
>  to coding style problem in context lines.

Nah, really the only "important" thing is (while you're there) to do the "if 
((err = ))" breaking -- noone here is religious about the rest.

But, well, forgetting all about sgalaxy.c makes as much sense. The new 
driver already support more models anyway (the ones without EEPROMs).

Rene.

Re: [PATCH] sgalaxy: checkpatch fixes

[Takashi Iwai]

At Wed, 05 Sep 2007 00:24:33 +0200,
Rene Herman wrote:
> > -#if 0
> > -	snd_printdd("WSS signature = 0x%x\n", tmp);
> > -#endif
> 
> Please don't just kill debug code. It's very useful for the next person 
> stepping in -- it's "functional commentary".

Then let's get rid of "#if 0".  snd_printdd() won't get compiled
unless CONFIG_SND_DEBUG_DETECT is set, so there is no reason to kill
twice.

> > @@ -327,7 +329,8 @@ static int snd_sgalaxy_resume(struct dev
> >  
> >  	chip->resume(chip);
> >  	snd_ad1848_out(chip, SGALAXY_AUXC_LEFT, chip->image[SGALAXY_AUXC_LEFT]);
> > -	snd_ad1848_out(chip, SGALAXY_AUXC_RIGHT, chip->image[SGALAXY_AUXC_RIGHT]);
> > +	snd_ad1848_out(chip, SGALAXY_AUXC_RIGHT,
> > +			chip->image[SGALAXY_AUXC_RIGHT]);
> 
> Definitely would keep this on one line.

Well, I don't think that's too bad to fold.  But, it's not forcible
enough, too.

After all, the 80-chars rule is for readability, and the readability
is more or less a matter of taste.


Takashi

Re: [PATCH] sgalaxy: checkpatch fixes

[Rene Herman]

On 09/05/2007 03:34 PM, Krzysztof Helt wrote:

> Ok. Now I am confused. Should I fix the patch and post it or
> should I do nothing with the sgalaxy.c as it will be (hopefully)
> soon replaced by a new driver (Rene's)?

Working on it as wel speak, so as far as I'm concerned, you can leave 
sgalaxy be.

Rene.

Re: [PATCH] sgalaxy: checkpatch fixes

[Krzysztof Helt]

Dnia 5-09-2007 o godz. 15:20 Takashi Iwai napisał(a):
> At Wed, 05 Sep 2007 00:24:33 +0200,
> Rene Herman wrote:
> > > -#if 0
> > > -	snd_printdd("WSS signature = 0x%x\n", tmp);
> > > -#endif
> > 
> > Please don't just kill debug code. It's very useful for the
next person 
> > stepping in -- it's "functional commentary".
> 
> Then let's get rid of "#if 0".  snd_printdd() won't get compiled
> unless CONFIG_SND_DEBUG_DETECT is set, so there is no reason to
kill
> twice.
> 

I have thought the same this morning.

Ok. Now I am confused. Should I fix the patch and post it or
should I do nothing with the sgalaxy.c as it will be (hopefully)
soon replaced by a new driver (Rene's)?

Regards,
Krzysztof

----------------------------------------------------
Walka jakiej nie widziałeś! KSW eliminacje, 
nie wszyscy przejdą cało! 
Zobacz na żywo we Wrocławiu już 15 września. 
Więcej -> http://klik.wp.pl/?adr=http%3A%2F%2Fadv.reklama.wp.pl%2Fas%2Fksweliminacje.html&sid=12