Skip to contentSkip to conversation

2.6.9-rc3 fix warnings in sound/drivers/opl3/opl3_lib.c

PATCH In review Borislav Petkov 3 participants 7 messages Sep 30, 2004 Sound

lore
Share & tools
lore

Borislav Petkov

author
#0 lore raw

PATCH 1/1 2.6.9-rc3 fix warnings in sound/drivers/opl3/opl3_lib.c

Hi there,
I get these warnings while compiling 2.6.9-rc3:
sound/drivers/opl3/opl3_lib.c: In function `snd_opl3_cs4281_command':
sound/drivers/opl3/opl3_lib.c:101: warning: passing arg 2 of writel' makes pointer from integer without a cast sound/drivers/opl3/opl3_lib.c:104: warning: passing arg 2 of writel' makes pointer from integer without a cast

Hope this fix is correct.

Regards,
Boris.

drivers/opl3/opl3_lib.c renamed from drivers/opl3/opl3_lib.c.orig +2 -2
@@ -98,10 +98,10 @@ void snd_opl3_cs4281_command(opl3_t * op
 
 	spin_lock_irqsave(&opl3->reg_lock, flags);
 
-	writel((unsigned int)cmd, port << 2);
+	writel((unsigned int)cmd, (void __iomem *)(port << 2));
 	udelay(10);
 
-	writel((unsigned int)val, (port + 1) << 2);
+	writel((unsigned int)val, (void __iomem *)((port + 1) << 2));
 	udelay(30);
 
 	spin_unlock_irqrestore(&opl3->reg_lock, flags);
#1 lore raw
Quoted 7 lines · Borislav Petkov

Hi there,
I get these warnings while compiling 2.6.9-rc3:
sound/drivers/opl3/opl3_lib.c: In function `snd_opl3_cs4281_command':
sound/drivers/opl3/opl3_lib.c:101: warning: passing arg 2 of writel' makes pointer from integer without a cast sound/drivers/opl3/opl3_lib.c:104: warning: passing arg 2 of writel' makes pointer from integer without a cast

Hope this fix is correct.

It looks very odd. At the very least we don't want to overload the fields in question (->r_port and ->l_port) that way.

#2 lore raw
Quoted 11 lines · On Thu, Sep 30, 2004 at 04:25:44PM +0100…
Quoted 7 lines · Borislav Petkov

Hi there,
I get these warnings while compiling 2.6.9-rc3:
sound/drivers/opl3/opl3_lib.c: In function `snd_opl3_cs4281_command':
sound/drivers/opl3/opl3_lib.c:101: warning: passing arg 2 of writel' makes pointer from integer without a cast sound/drivers/opl3/opl3_lib.c:104: warning: passing arg 2 of writel' makes pointer from integer without a cast

Hope this fix is correct.

It looks very odd. At the very least we don't want to overload the fields in question (->r_port and ->l_port) that way.

*Yuck*

ALSA code, as pretty as ever. No, that's not a fix; it's only shutting the rightfully complaining compiler up.

What happens there is a dirty kludge created for the benefit of a single driver (sound/pci/cs4281.c). Said driver has a bunch of registers memory-mapped, while its relatives use port IO instead. Driver does (correctly) ioremap(); then it overloads the arguments of snd_opl3_create() normally used for port numbers and shoves *address obtained from ioremap and divided by 4* in them.

Sigh... At the very least that kind of abuse should stop. FWIW, I would suggest having cs4281.c set the ->command() directly and killing that crap with ->l_port/->r_port overloading.

#3 lore raw

At Thu, 30 Sep 2004 16:52:28 +0100,

Quoted 19 lines · [email protected] wrote:
Quoted 11 lines · On Thu, Sep 30, 2004 at 04:25:44PM +0100…
Quoted 7 lines · Borislav Petkov

Hi there,
I get these warnings while compiling 2.6.9-rc3:
sound/drivers/opl3/opl3_lib.c: In function `snd_opl3_cs4281_command':
sound/drivers/opl3/opl3_lib.c:101: warning: passing arg 2 of writel' makes pointer from integer without a cast sound/drivers/opl3/opl3_lib.c:104: warning: passing arg 2 of writel' makes pointer from integer without a cast

Hope this fix is correct.

It looks very odd. At the very least we don't want to overload the fields in question (->r_port and ->l_port) that way.

*Yuck*

ALSA code, as pretty as ever. No, that's not a fix; it's only shutting the rightfully complaining compiler up.

Quoted 6 lines

What happens there is a dirty kludge created for the benefit of a single driver (sound/pci/cs4281.c). Said driver has a bunch of registers memory-mapped, while its relatives use port IO instead. Driver does (correctly) ioremap(); then it overloads the arguments of snd_opl3_create() normally used for port numbers and shoves *address obtained from ioremap and divided by 4* in them.

This ugly shift was already removed in the current version in linux-sound bk. The l_port and r_port point the iomem pointers now on cs4281 like others. I don't know why the author of cs4281 implemented in such a way.

BTW, all __iomem fixes are already there, too.

Quoted 3 lines

Sigh... At the very least that kind of abuse should stop. FWIW, I would suggest having cs4281.c set the ->command() directly and killing that crap with ->l_port/->r_port overloading.

Yes, it'd be definitely better.
Will work on it.

Takashi

#4 lore raw

At Thu, 30 Sep 2004 18:07:10 +0200,

Quoted 7 lines · I wrote:
Quoted 4 lines

Sigh... At the very least that kind of abuse should stop. FWIW, I would suggest having cs4281.c set the ->command() directly and killing that crap with ->l_port/->r_port overloading.

Yes, it'd be definitely better.
Will work on it.

The patch below.

--
Takashi Iwai <[email protected]> ALSA Developer - www.alsa-project.org

==

Summary: Fix / clean up OPL3 for CS4281

Moved cs4281-specific code into cs4281 driver from opl3. The ugly type-casting is removed now.

The opl3 instance can be created via snd_opl3_new() (followed by snd_opl3_init()) to allow the driver to set its own command and private_data/private_free.

snd_opl3_create() is kept for compatibility as it was.

Signed-off-by Takashi Iwai <[email protected]>
include/sound/opl3.h +5 -0
@@ -274,6 +274,9 @@
 	snd_timer_t *timer2;
 	spinlock_t timer_lock;
 
+	void *private_data;
+	void (*private_free)(opl3_t *);
+
 	spinlock_t reg_lock;
 	snd_card_t *card;		/* The card that this belongs to */
 	int used;			/* usage flag - exclusive */

@@ -314,6 +317,8 @@
 
 /* opl3.c */
 void snd_opl3_interrupt(snd_hwdep_t * hw);
+int snd_opl3_new(snd_card_t *card, unsigned short hardware, opl3_t **ropl3);
+int snd_opl3_init(opl3_t *opl3);
 int snd_opl3_create(snd_card_t * card,
 		    unsigned long l_port, unsigned long r_port,
 		    unsigned short hardware,
sound/drivers/opl3/opl3_lib.c +70 -68

193 lines not on this page · raw message

sound/pci/cs4281.c +29 -4
@@ -1914,6 +1914,31 @@
 }
 
 
+/*
+ * OPL3 command
+ */
+static void snd_cs4281_opl3_command(opl3_t * opl3, unsigned short cmd, unsigned char val)
+{
+	unsigned long flags;
+	cs4281_t *chip = opl3->private_data;
+	void __iomem *port;
+
+	if (cmd & OPL3_RIGHT)
+		port = chip->ba0 + BA0_B1AP; /* right port */
+	else
+		port = chip->ba0 + BA0_B0AP; /* left port */
+
+	spin_lock_irqsave(&opl3->reg_lock, flags);
+
+	writel((unsigned int)cmd, port);
+	udelay(10);
+
+	writel((unsigned int)val, port + 4);
+	udelay(30);
+
+	spin_unlock_irqrestore(&opl3->reg_lock, flags);
+}
+
 static int __devinit snd_cs4281_probe(struct pci_dev *pci,
 				      const struct pci_device_id *pci_id)
 {

@@ -1951,13 +1976,13 @@
 		snd_card_free(card);
 		return err;
 	}
-	if ((err = snd_opl3_create(card,
-				   (unsigned long)(chip->ba0 + BA0_B0AP),
-				   (unsigned long)(chip->ba0 + BA0_B1AP),
-				   OPL3_HW_OPL3_CS4281, 1, &opl3)) < 0) {
+	if ((err = snd_opl3_new(card, OPL3_HW_OPL3_CS4281, &opl3)) < 0) {
 		snd_card_free(card);
 		return err;
 	}
+	opl3->private_data = chip;
+	opl3->command = snd_cs4281_opl3_command;
+	snd_opl3_init(opl3);
 	if ((err = snd_opl3_hwdep_new(opl3, 0, 1, NULL)) < 0) {
 		snd_card_free(card);
 		return err;
#5 lore raw

At Thu, 30 Sep 2004 18:50:25 +0200,

Quoted 4 lines · I wrote:

At Thu, 30 Sep 2004 18:07:10 +0200,

Quoted 1 line · I wrote:
Quoted 8 lines
Quoted 6 lines
Quoted 3 lines

Sigh... At the very least that kind of abuse should stop. FWIW, I would suggest having cs4281.c set the ->command() directly and killing that crap with ->l_port/->r_port overloading.

Yes, it'd be definitely better.
Will work on it.

The patch below.

Oh, the last one was for linux-sound bk.

The patch to 2.6.9-rc3 is below.

Takashi

include/sound/opl3.h +5 -0
@@ -274,6 +274,9 @@
 	snd_timer_t *timer2;
 	spinlock_t timer_lock;
 
+	void *private_data;
+	void (*private_free)(opl3_t *);
+
 	spinlock_t reg_lock;
 	snd_card_t *card;		/* The card that this belongs to */
 	int used;			/* usage flag - exclusive */

@@ -314,6 +317,8 @@
 
 /* opl3.c */
 void snd_opl3_interrupt(snd_hwdep_t * hw);
+int snd_opl3_new(snd_card_t *card, unsigned short hardware, opl3_t **ropl3);
+int snd_opl3_init(opl3_t *opl3);
 int snd_opl3_create(snd_card_t * card,
 		    unsigned long l_port, unsigned long r_port,
 		    unsigned short hardware,
sound/drivers/opl3/opl3_lib.c +70 -68

193 lines not on this page · raw message

sound/pci/cs4281.c +35 -10

79 lines not on this page · raw message

author
#6 lore raw
In reply to Takashi Iwai
Quoted 4 lines · Takashi Iwai

At Thu, 30 Sep 2004 16:52:28 +0100,

Quoted 1 line · [email protected] wrote:

On Thu, Sep 30, 2004 at 04:25:44PM +0100,

Quoted 37 lines · [email protected] wrote:
Quoted 19 lines
Quoted 9 lines · Borislav Petkov
Quoted 8 lines

Hi there,
I get these warnings while compiling 2.6.9-rc3:
sound/drivers/opl3/opl3_lib.c: In function
`snd_opl3_cs4281_command': sound/drivers/opl3/opl3_lib.c:101:
warning: passing arg 2 of writel' makes pointer from integer without a cast sound/drivers/opl3/opl3_lib.c:104: warning: passing arg 2 of writel' makes pointer from integer without a cast

Quoted 4 lines
Quoted 1 line

Hope this fix is correct.

It looks very odd. At the very least we don't want to overload the fields in question (->r_port and ->l_port) that way.

*Yuck*

ALSA code, as pretty as ever. No, that's not a fix; it's only shutting the rightfully complaining compiler up.

Quoted 6 lines

What happens there is a dirty kludge created for the benefit of a single driver (sound/pci/cs4281.c). Said driver has a bunch of registers memory-mapped, while its relatives use port IO instead. Driver does (correctly) ioremap(); then it overloads the arguments of snd_opl3_create() normally used for port numbers and shoves *address obtained from ioremap and divided by 4* in them.

This ugly shift was already removed in the current version in linux-sound bk. The l_port and r_port point the iomem pointers now on cs4281 like others. I don't know why the author of cs4281 implemented in such a way.

BTW, all __iomem fixes are already there, too.

Quoted 3 lines

Sigh... At the very least that kind of abuse should stop. FWIW, I would suggest having cs4281.c set the ->command() directly and killing that crap with ->l_port/->r_port overloading.

Quoted 5 lines

Yes, it'd be definitely better.
Will work on it.

Takashi

Ups,
tip of the iceberg, :) I actually thought that this would be too easy to be true.

Regards,
Boris.