Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S269353AbUI3Qvx (ORCPT ); Thu, 30 Sep 2004 12:51:53 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S269356AbUI3Qvx (ORCPT ); Thu, 30 Sep 2004 12:51:53 -0400 Received: from cantor.suse.de ([195.135.220.2]:45952 "EHLO Cantor.suse.de") by vger.kernel.org with ESMTP id S269353AbUI3Qvf (ORCPT ); Thu, 30 Sep 2004 12:51:35 -0400 Date: Thu, 30 Sep 2004 18:50:25 +0200 Message-ID: From: Takashi Iwai To: viro@parcelfarce.linux.theplanet.co.uk Cc: Borislav Petkov , linux-kernel@vger.kernel.org, bbpetkov@yahoo.de Subject: Re: [PATCH] 2.6.9-rc3 fix warnings in sound/drivers/opl3/opl3_lib.c In-Reply-To: References: <20040930122853.GA28332@none> <20040930152544.GD23987@parcelfarce.linux.theplanet.co.uk> <20040930155228.GE23987@parcelfarce.linux.theplanet.co.uk> User-Agent: Wanderlust/2.10.1 (Watching The Wheels) SEMI/1.14.5 (Awara-Onsen) FLIM/1.14.5 (Demachiyanagi) APEL/10.6 MULE XEmacs/21.4 (patch 15) (Security Through Obscurity) (i386-suse-linux) MIME-Version: 1.0 (generated by SEMI 1.14.5 - "Awara-Onsen") Content-Type: text/plain; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 8360 Lines: 309 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 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 --- 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; - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/