All of lore.kernel.org
 help / color / mirror / Atom feed
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
Date: Wed, 03 Dec 2003 12:22:39 +0100	[thread overview]
Message-ID: <s5hekvmcfi8.wl@alsa2.suse.de> (raw)
In-Reply-To: <20031202234432.GA14730@neo.rr.com>

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

  reply	other threads:[~2003-12-03 11:26 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2003-12-02 17:06 vanilla 2.6.0-test11 and CS4236 card Joseph Pingenot
2003-12-02 17:31 ` Takashi Iwai
2003-12-02 17:52   ` Joseph Pingenot
2003-12-02 21:04   ` Joseph Pingenot
2003-12-02 23:44   ` Adam Belay
2003-12-03 11:22     ` Takashi Iwai [this message]
2003-12-03 15:00       ` wes schreiner
2003-12-03 15:16         ` Joseph Pingenot
2003-12-03  3:17   ` Joseph Pingenot
2003-12-03 11:29     ` Takashi Iwai
2003-12-03 14:09       ` Joseph Pingenot
2003-12-04 20:31   ` Joseph Pingenot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=s5hekvmcfi8.wl@alsa2.suse.de \
    --to=tiwai@suse.de \
    --cc=ambx1@neo.rr.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=trelane@digitasaru.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.