From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner+willy=40w.ods.org@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S264544AbTLCL0N (ORCPT <rfc822;willy@w.ods.org>);
	Wed, 3 Dec 2003 06:26:13 -0500
Received: (majordomo@vger.kernel.org) by vger.kernel.org id S264550AbTLCL0N
	(ORCPT <rfc822;linux-kernel-outgoing>);
	Wed, 3 Dec 2003 06:26:13 -0500
Received: from ns.suse.de ([195.135.220.2]:25765 "EHLO Cantor.suse.de")
	by vger.kernel.org with ESMTP id S264544AbTLCL0I (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Wed, 3 Dec 2003 06:26:08 -0500
Date: Wed, 03 Dec 2003 12:22:39 +0100
Message-ID: <s5hekvmcfi8.wl@alsa2.suse.de>
From: Takashi Iwai <tiwai@suse.de>
To: Adam Belay <ambx1@neo.rr.com>
Cc: linux-kernel@vger.kernel.org, trelane@digitasaru.net
Subject: Re: vanilla 2.6.0-test11 and CS4236 card
In-Reply-To: <20031202234432.GA14730@neo.rr.com>
References: <20031202170637.GD5475@digitasaru.net>
	<s5hsmk3ceia.wl@alsa2.suse.de>
	<20031202234432.GA14730@neo.rr.com>
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 13) (Rational FORTRAN) (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

Hi Adam,

At Tue, 2 Dec 2003 23:44:32 +0000,
Adam Belay wrote:
> 
> Here's a slight cleanup of the above patch -- tested with the cs4232 driver.
> Any additional testing would be appreciated.
> 
> --- a/drivers/pnp/card.c	2003-11-26 20:45:38.000000000 +0000
> +++ b/drivers/pnp/card.c	2003-12-02 23:06:55.000000000 +0000
> @@ -26,8 +26,18 @@
>  {
>  	const struct pnp_card_device_id * drv_id = drv->id_table;
>  	while (*drv_id->id){
> -		if (compare_pnp_id(card->id,drv_id->id))
> -			return drv_id;
> +		if (compare_pnp_id(card->id,drv_id->id)) {
> +			int i = 0;
> +			for (;;i++) {
> +				struct pnp_dev *dev;
> +				if (i == PNP_MAX_DEVICES || ! *drv_id->devs[i].id)
> +					return drv_id;
> +				card_for_each_dev(card, dev) {
> +					if (!compare_pnp_id(dev->id, drv_id->devs[i].id))
> +						break;
> +				}
> +			}
> +		}
>  		drv_id++;
>  	}
>  	return NULL;
> 

i think the patch above doesn't work properly since it will continue
even if no devices match.  so, the loop after comparison of card id
makes no sense.


> In this case we could also just continue if the MPU device isn't present.  It
> would probably be a good convention to do so because if, for whatever reason
> (3rd party driver, resource conflicts, etc), the MPU device is busy in any
> matched card id set, the entire probe would fail.
> 
> How about something like this? (untested)
> 
> --- a/sound/isa/cs423x/cs4236.c	2003-11-26 20:43:05.000000000 +0000
> +++ b/sound/isa/cs423x/cs4236.c	2003-12-02 23:36:59.000000000 +0000
> @@ -294,13 +294,8 @@
>  		kfree(cfg);
>  		return -EBUSY;
>  	}
> -	if (id->devs[2].id[0]) {
> +	if (id->devs[2].id[0])
>  		acard->mpu = pnp_request_card_device(card, id->devs[2].id, NULL);
> -		if (acard->mpu == NULL) {
> -			kfree(cfg);
> -			return -EBUSY;
> -		}
> -	}
> 
>  	/* WSS initialization */
>  	pdev = acard->wss;

this looks fine to me.


ciao,

Takashi