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

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

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

   Regards,
   Boris.


--- sound/drivers/opl3/opl3_lib.c.orig	2004-09-30 14:22:08.000000000 +0200
+++ sound/drivers/opl3/opl3_lib.c	2004-09-30 14:23:50.000000000 +0200
@@ -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);

Re: [PATCH] 2.6.9-rc3 fix warnings in sound/drivers/opl3/opl3_lib.c

[viro]

On Thu, Sep 30, 2004 at 02:28:53PM +0200, Borislav Petkov wrote:
> 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.

Re: [PATCH] 2.6.9-rc3 fix warnings in sound/drivers/opl3/opl3_lib.c

[viro]

On Thu, Sep 30, 2004 at 04:25:44PM +0100, viro@parcelfarce.linux.theplanet.co.uk wrote:
> On Thu, Sep 30, 2004 at 02:28:53PM +0200, Borislav Petkov wrote:
> > 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.

Re: [PATCH] 2.6.9-rc3 fix warnings in sound/drivers/opl3/opl3_lib.c

[Takashi Iwai]

At Thu, 30 Sep 2004 16:52:28 +0100,
viro@parcelfarce.linux.theplanet.co.uk wrote:
> 
> On Thu, Sep 30, 2004 at 04:25:44PM +0100, viro@parcelfarce.linux.theplanet.co.uk wrote:
> > On Thu, Sep 30, 2004 at 02:28:53PM +0200, Borislav Petkov wrote:
> > > 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.

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.

> 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

Re: [PATCH] 2.6.9-rc3 fix warnings in sound/drivers/opl3/opl3_lib.c

[Takashi Iwai]

At Thu, 30 Sep 2004 18:07:10 +0200,
I wrote:
> 
> > 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 <tiwai@suse.de>		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 <tiwai@suse.de>

--- linux/include/sound/opl3.h	2003-03-25 16:13:46.000000000 +0100
+++ linux/include/sound/opl3.h	2004-09-30 18:25:00.000000000 +0200
@@ -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,
--- linux/sound/drivers/opl3/opl3_lib.c	2004-09-16 19:34:18.000000000 +0200
+++ linux/sound/drivers/opl3/opl3_lib.c	2004-09-30 18:23:28.000000000 +0200
@@ -85,28 +85,6 @@
 	spin_unlock_irqrestore(&opl3->reg_lock, flags);
 }
 
-void snd_opl3_cs4281_command(opl3_t * opl3, unsigned short cmd, unsigned char val)
-{
-	unsigned long flags;
-	void __iomem *port;
-
-	/*
-	 * CS4281 requires a special access to I/O registers
-	 */
-
-	port = (void __iomem *)((cmd & OPL3_RIGHT) ? opl3->r_port : opl3->l_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 snd_opl3_detect(opl3_t * opl3)
 {
 	/*
@@ -344,6 +322,9 @@
 
 static int snd_opl3_free(opl3_t *opl3)
 {
+	snd_assert(opl3 != NULL, return -ENXIO);
+	if (opl3->private_free)
+		opl3->private_free(opl3);
 	if (opl3->res_l_port) {
 		release_resource(opl3->res_l_port);
 		kfree_nocheck(opl3->res_l_port);
@@ -362,51 +343,89 @@
 	return snd_opl3_free(opl3);
 }
 
-int snd_opl3_create(snd_card_t * card,
-		    unsigned long l_port,
-		    unsigned long r_port,
-		    unsigned short hardware,
-		    int integrated,
-		    opl3_t ** ropl3)
+int snd_opl3_new(snd_card_t *card,
+		 unsigned short hardware,
+		 opl3_t **ropl3)
 {
-	opl3_t *opl3;
-	int err;
 	static snd_device_ops_t ops = {
 		.dev_free = snd_opl3_dev_free,
 	};
+	opl3_t *opl3;
+	int err;
 
 	*ropl3 = NULL;
-
 	opl3 = kcalloc(1, sizeof(*opl3), GFP_KERNEL);
 	if (opl3 == NULL)
 		return -ENOMEM;
 
-	if (integrated)
-		goto __step1; /* ports are already reserved */
+	opl3->card = card;
+	opl3->hardware = hardware;
+	spin_lock_init(&opl3->reg_lock);
+	spin_lock_init(&opl3->timer_lock);
+	init_MUTEX(&opl3->access_mutex);
 
-	if ((opl3->res_l_port = request_region(l_port, 2, "OPL2/3 (left)")) == NULL) {
-		snd_printk(KERN_ERR "opl3: can't grab left port 0x%lx\n", l_port);
+	if ((err = snd_device_new(card, SNDRV_DEV_LOWLEVEL, opl3, &ops)) < 0) {
 		snd_opl3_free(opl3);
-		return -EBUSY;
+		return err;
 	}
-	if (r_port != 0 &&
-	    (opl3->res_r_port = request_region(r_port, 2, "OPL2/3 (right)")) == NULL) {
-		snd_printk(KERN_ERR "opl3: can't grab right port 0x%lx\n", r_port);
-		snd_opl3_free(opl3);
-		return -EBUSY;
+
+	*ropl3 = opl3;
+	return 0;
+}
+
+int snd_opl3_init(opl3_t *opl3)
+{
+	if (! opl3->command) {
+		printk(KERN_ERR "snd_opl3_init: command not defined!\n");
+		return -EINVAL;
 	}
 
-      __step1:
+	opl3->command(opl3, OPL3_LEFT | OPL3_REG_TEST, OPL3_ENABLE_WAVE_SELECT);
+	/* Melodic mode */
+	opl3->command(opl3, OPL3_LEFT | OPL3_REG_PERCUSSION, 0x00);
 
-	opl3->card = card;
-	opl3->hardware = hardware;
+	switch (opl3->hardware & OPL3_HW_MASK) {
+	case OPL3_HW_OPL2:
+		opl3->max_voices = MAX_OPL2_VOICES;
+		break;
+	case OPL3_HW_OPL3:
+	case OPL3_HW_OPL4:
+		opl3->max_voices = MAX_OPL3_VOICES;
+		/* Enter OPL3 mode */
+		opl3->command(opl3, OPL3_RIGHT | OPL3_REG_MODE, OPL3_OPL3_ENABLE);
+	}
+	return 0;
+}
+
+int snd_opl3_create(snd_card_t * card,
+		    unsigned long l_port,
+		    unsigned long r_port,
+		    unsigned short hardware,
+		    int integrated,
+		    opl3_t ** ropl3)
+{
+	opl3_t *opl3;
+	int err;
+
+	*ropl3 = NULL;
+	if ((err = snd_opl3_new(card, hardware, &opl3)) < 0)
+		return err;
+	if (! integrated) {
+		if ((opl3->res_l_port = request_region(l_port, 2, "OPL2/3 (left)")) == NULL) {
+			snd_printk(KERN_ERR "opl3: can't grab left port 0x%lx\n", l_port);
+			snd_opl3_free(opl3);
+			return -EBUSY;
+		}
+		if (r_port != 0 &&
+		    (opl3->res_r_port = request_region(r_port, 2, "OPL2/3 (right)")) == NULL) {
+			snd_printk(KERN_ERR "opl3: can't grab right port 0x%lx\n", r_port);
+			snd_opl3_free(opl3);
+			return -EBUSY;
+		}
+	}
 	opl3->l_port = l_port;
 	opl3->r_port = r_port;
 
-	spin_lock_init(&opl3->reg_lock);
-	spin_lock_init(&opl3->timer_lock);
-	init_MUTEX(&opl3->access_mutex);
-
 	switch (opl3->hardware) {
 	/* some hardware doesn't support timers */
 	case OPL3_HW_OPL3_SV:
@@ -414,9 +433,6 @@
 	case OPL3_HW_OPL3_FM801:
 		opl3->command = &snd_opl3_command;
 		break;
-	case OPL3_HW_OPL3_CS4281:
-		opl3->command = &snd_opl3_cs4281_command;
-		break;
 	default:
 		opl3->command = &snd_opl2_command;
 		if ((err = snd_opl3_detect(opl3)) < 0) {
@@ -433,23 +449,7 @@
 		}
 	}
 
-	opl3->command(opl3, OPL3_LEFT | OPL3_REG_TEST, OPL3_ENABLE_WAVE_SELECT);
-	opl3->command(opl3, OPL3_LEFT | OPL3_REG_PERCUSSION, 0x00);	/* Melodic mode */
-
-	switch (opl3->hardware & OPL3_HW_MASK) {
-	case OPL3_HW_OPL2:
-		opl3->max_voices = MAX_OPL2_VOICES;
-		break;
-	case OPL3_HW_OPL3:
-	case OPL3_HW_OPL4:
-		opl3->max_voices = MAX_OPL3_VOICES;
-		snd_assert(opl3->r_port != 0, snd_opl3_free(opl3); return -ENODEV);
-		opl3->command(opl3, OPL3_RIGHT | OPL3_REG_MODE, OPL3_OPL3_ENABLE);	/* Enter OPL3 mode */
-	}
-	if ((err = snd_device_new(card, SNDRV_DEV_LOWLEVEL, opl3, &ops)) < 0) {
-		snd_opl3_free(opl3);
-		return err;
-	}
+	snd_opl3_init(opl3);
 
 	*ropl3 = opl3;
 	return 0;
@@ -531,6 +531,8 @@
 }
 
 EXPORT_SYMBOL(snd_opl3_interrupt);
+EXPORT_SYMBOL(snd_opl3_new);
+EXPORT_SYMBOL(snd_opl3_init);
 EXPORT_SYMBOL(snd_opl3_create);
 EXPORT_SYMBOL(snd_opl3_timer_new);
 EXPORT_SYMBOL(snd_opl3_hwdep_new);
--- linux/sound/pci/cs4281.c	2004-09-16 19:35:30.000000000 +0200
+++ linux/sound/pci/cs4281.c	2004-09-30 18:30:29.000000000 +0200
@@ -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;

Re: [PATCH] 2.6.9-rc3 fix warnings in sound/drivers/opl3/opl3_lib.c

[Takashi Iwai]

At Thu, 30 Sep 2004 18:50:25 +0200,
I wrote:
> 
> At Thu, 30 Sep 2004 18:07:10 +0200,
> I wrote:
> > 
> > > 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


--- linux-2.6.9-rc3/include/sound/opl3.h	2004-08-14 12:54:50.000000000 +0200
+++ linux/include/sound/opl3.h	2004-09-30 18:25:00.000000000 +0200
@@ -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,
--- linux-2.6.9-rc3/sound/drivers/opl3/opl3_lib.c	2004-09-30 19:04:07.496702568 +0200
+++ linux/sound/drivers/opl3/opl3_lib.c	2004-09-30 18:23:28.000000000 +0200
@@ -85,28 +85,6 @@
 	spin_unlock_irqrestore(&opl3->reg_lock, flags);
 }
 
-void snd_opl3_cs4281_command(opl3_t * opl3, unsigned short cmd, unsigned char val)
-{
-	unsigned long flags;
-	unsigned long port;
-
-	/*
-	 * CS4281 requires a special access to I/O registers
-	 */
-
-	port = (cmd & OPL3_RIGHT) ? opl3->r_port : opl3->l_port;
-
-	spin_lock_irqsave(&opl3->reg_lock, flags);
-
-	writel((unsigned int)cmd, port << 2);
-	udelay(10);
-
-	writel((unsigned int)val, (port + 1) << 2);
-	udelay(30);
-
-	spin_unlock_irqrestore(&opl3->reg_lock, flags);
-}
-
 static int snd_opl3_detect(opl3_t * opl3)
 {
 	/*
@@ -344,6 +322,9 @@
 
 static int snd_opl3_free(opl3_t *opl3)
 {
+	snd_assert(opl3 != NULL, return -ENXIO);
+	if (opl3->private_free)
+		opl3->private_free(opl3);
 	if (opl3->res_l_port) {
 		release_resource(opl3->res_l_port);
 		kfree_nocheck(opl3->res_l_port);
@@ -362,51 +343,89 @@
 	return snd_opl3_free(opl3);
 }
 
-int snd_opl3_create(snd_card_t * card,
-		    unsigned long l_port,
-		    unsigned long r_port,
-		    unsigned short hardware,
-		    int integrated,
-		    opl3_t ** ropl3)
+int snd_opl3_new(snd_card_t *card,
+		 unsigned short hardware,
+		 opl3_t **ropl3)
 {
-	opl3_t *opl3;
-	int err;
 	static snd_device_ops_t ops = {
 		.dev_free = snd_opl3_dev_free,
 	};
+	opl3_t *opl3;
+	int err;
 
 	*ropl3 = NULL;
-
 	opl3 = kcalloc(1, sizeof(*opl3), GFP_KERNEL);
 	if (opl3 == NULL)
 		return -ENOMEM;
 
-	if (integrated)
-		goto __step1; /* ports are already reserved */
+	opl3->card = card;
+	opl3->hardware = hardware;
+	spin_lock_init(&opl3->reg_lock);
+	spin_lock_init(&opl3->timer_lock);
+	init_MUTEX(&opl3->access_mutex);
 
-	if ((opl3->res_l_port = request_region(l_port, 2, "OPL2/3 (left)")) == NULL) {
-		snd_printk(KERN_ERR "opl3: can't grab left port 0x%lx\n", l_port);
+	if ((err = snd_device_new(card, SNDRV_DEV_LOWLEVEL, opl3, &ops)) < 0) {
 		snd_opl3_free(opl3);
-		return -EBUSY;
+		return err;
 	}
-	if (r_port != 0 &&
-	    (opl3->res_r_port = request_region(r_port, 2, "OPL2/3 (right)")) == NULL) {
-		snd_printk(KERN_ERR "opl3: can't grab right port 0x%lx\n", r_port);
-		snd_opl3_free(opl3);
-		return -EBUSY;
+
+	*ropl3 = opl3;
+	return 0;
+}
+
+int snd_opl3_init(opl3_t *opl3)
+{
+	if (! opl3->command) {
+		printk(KERN_ERR "snd_opl3_init: command not defined!\n");
+		return -EINVAL;
 	}
 
-      __step1:
+	opl3->command(opl3, OPL3_LEFT | OPL3_REG_TEST, OPL3_ENABLE_WAVE_SELECT);
+	/* Melodic mode */
+	opl3->command(opl3, OPL3_LEFT | OPL3_REG_PERCUSSION, 0x00);
 
-	opl3->card = card;
-	opl3->hardware = hardware;
+	switch (opl3->hardware & OPL3_HW_MASK) {
+	case OPL3_HW_OPL2:
+		opl3->max_voices = MAX_OPL2_VOICES;
+		break;
+	case OPL3_HW_OPL3:
+	case OPL3_HW_OPL4:
+		opl3->max_voices = MAX_OPL3_VOICES;
+		/* Enter OPL3 mode */
+		opl3->command(opl3, OPL3_RIGHT | OPL3_REG_MODE, OPL3_OPL3_ENABLE);
+	}
+	return 0;
+}
+
+int snd_opl3_create(snd_card_t * card,
+		    unsigned long l_port,
+		    unsigned long r_port,
+		    unsigned short hardware,
+		    int integrated,
+		    opl3_t ** ropl3)
+{
+	opl3_t *opl3;
+	int err;
+
+	*ropl3 = NULL;
+	if ((err = snd_opl3_new(card, hardware, &opl3)) < 0)
+		return err;
+	if (! integrated) {
+		if ((opl3->res_l_port = request_region(l_port, 2, "OPL2/3 (left)")) == NULL) {
+			snd_printk(KERN_ERR "opl3: can't grab left port 0x%lx\n", l_port);
+			snd_opl3_free(opl3);
+			return -EBUSY;
+		}
+		if (r_port != 0 &&
+		    (opl3->res_r_port = request_region(r_port, 2, "OPL2/3 (right)")) == NULL) {
+			snd_printk(KERN_ERR "opl3: can't grab right port 0x%lx\n", r_port);
+			snd_opl3_free(opl3);
+			return -EBUSY;
+		}
+	}
 	opl3->l_port = l_port;
 	opl3->r_port = r_port;
 
-	spin_lock_init(&opl3->reg_lock);
-	spin_lock_init(&opl3->timer_lock);
-	init_MUTEX(&opl3->access_mutex);
-
 	switch (opl3->hardware) {
 	/* some hardware doesn't support timers */
 	case OPL3_HW_OPL3_SV:
@@ -414,9 +433,6 @@
 	case OPL3_HW_OPL3_FM801:
 		opl3->command = &snd_opl3_command;
 		break;
-	case OPL3_HW_OPL3_CS4281:
-		opl3->command = &snd_opl3_cs4281_command;
-		break;
 	default:
 		opl3->command = &snd_opl2_command;
 		if ((err = snd_opl3_detect(opl3)) < 0) {
@@ -433,23 +449,7 @@
 		}
 	}
 
-	opl3->command(opl3, OPL3_LEFT | OPL3_REG_TEST, OPL3_ENABLE_WAVE_SELECT);
-	opl3->command(opl3, OPL3_LEFT | OPL3_REG_PERCUSSION, 0x00);	/* Melodic mode */
-
-	switch (opl3->hardware & OPL3_HW_MASK) {
-	case OPL3_HW_OPL2:
-		opl3->max_voices = MAX_OPL2_VOICES;
-		break;
-	case OPL3_HW_OPL3:
-	case OPL3_HW_OPL4:
-		opl3->max_voices = MAX_OPL3_VOICES;
-		snd_assert(opl3->r_port != 0, snd_opl3_free(opl3); return -ENODEV);
-		opl3->command(opl3, OPL3_RIGHT | OPL3_REG_MODE, OPL3_OPL3_ENABLE);	/* Enter OPL3 mode */
-	}
-	if ((err = snd_device_new(card, SNDRV_DEV_LOWLEVEL, opl3, &ops)) < 0) {
-		snd_opl3_free(opl3);
-		return err;
-	}
+	snd_opl3_init(opl3);
 
 	*ropl3 = opl3;
 	return 0;
@@ -531,6 +531,8 @@
 }
 
 EXPORT_SYMBOL(snd_opl3_interrupt);
+EXPORT_SYMBOL(snd_opl3_new);
+EXPORT_SYMBOL(snd_opl3_init);
 EXPORT_SYMBOL(snd_opl3_create);
 EXPORT_SYMBOL(snd_opl3_timer_new);
 EXPORT_SYMBOL(snd_opl3_hwdep_new);
--- linux-2.6.9-rc3/sound/pci/cs4281.c	2004-09-30 19:04:07.778638858 +0200
+++ linux/sound/pci/cs4281.c	2004-09-30 18:30:29.000000000 +0200
@@ -464,8 +464,8 @@
 struct snd_cs4281 {
 	int irq;
 
-	unsigned long ba0;		/* virtual (accessible) address */
-	unsigned long ba1;		/* virtual (accessible) address */
+	void __iomem *ba0;		/* virtual (accessible) address */
+	void __iomem *ba1;		/* virtual (accessible) address */
 	unsigned long ba0_addr;
 	unsigned long ba1_addr;
 
@@ -1359,9 +1359,9 @@
 	if (chip->irq >= 0)
 		free_irq(chip->irq, (void *)chip);
 	if (chip->ba0)
-		iounmap((void *) chip->ba0);
+		iounmap(chip->ba0);
 	if (chip->ba1)
-		iounmap((void *) chip->ba1);
+		iounmap(chip->ba1);
 	pci_release_regions(chip->pci);
 
 	kfree(chip);
@@ -1423,8 +1423,8 @@
 	}
 	chip->irq = pci->irq;
 
-	chip->ba0 = (unsigned long) ioremap_nocache(chip->ba0_addr, pci_resource_len(pci, 0));
-	chip->ba1 = (unsigned long) ioremap_nocache(chip->ba1_addr, pci_resource_len(pci, 1));
+	chip->ba0 = ioremap_nocache(chip->ba0_addr, pci_resource_len(pci, 0));
+	chip->ba1 = ioremap_nocache(chip->ba1_addr, pci_resource_len(pci, 1));
 	if (!chip->ba0 || !chip->ba1) {
 		snd_cs4281_free(chip);
 		return -ENOMEM;
@@ -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,
-				   (chip->ba0 + BA0_B0AP) >> 2,
-				   (chip->ba0 + BA0_B1AP) >> 2,
-				   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;

Re: [PATCH] 2.6.9-rc3 fix warnings in sound/drivers/opl3/opl3_lib.c

[Borislav Petkov]

On Thursday 30 September 2004 18:07, Takashi Iwai wrote:
> At Thu, 30 Sep 2004 16:52:28 +0100,
>
> viro@parcelfarce.linux.theplanet.co.uk wrote:
> > On Thu, Sep 30, 2004 at 04:25:44PM +0100, 
viro@parcelfarce.linux.theplanet.co.uk wrote:
> > > On Thu, Sep 30, 2004 at 02:28:53PM +0200, Borislav Petkov wrote:
> > > > 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.
>
> 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.
>
> > 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

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

Regards,
Boris.