At Sun, 4 Oct 2009 11:35:21 +0200, Jean Delvare wrote:
Hi Takashi,
On Thu, 01 Oct 2009 08:52:59 +0200, Takashi Iwai wrote:
At Wed, 30 Sep 2009 18:55:05 +0200, Jean Delvare wrote:
On Wed, 30 Sep 2009 17:15:49 +0200, Takashi Iwai wrote:
Yes, indeed I prefer NULL check because the user can know the error at the right place. I share your concern about the code addition, though :)
I already made a patch below, but it's totally untested. It'd be helpful if someone can do review and build-test it.
thanks,
Takashi
diff --git a/sound/aoa/codecs/tas.c b/sound/aoa/codecs/tas.c index f0ebc97..0f810c8 100644 --- a/sound/aoa/codecs/tas.c +++ b/sound/aoa/codecs/tas.c @@ -897,6 +897,10 @@ static int tas_create(struct i2c_adapter *adapter, client = i2c_new_device(adapter, &info); if (!client) return -ENODEV;
if (!client->driver) {
i2c_unregister_device(client);return -ENODEV;}
/*
- Let i2c-core delete that device on driver removal.
diff --git a/sound/ppc/keywest.c b/sound/ppc/keywest.c index 835fa19..473c5a6 100644 --- a/sound/ppc/keywest.c +++ b/sound/ppc/keywest.c @@ -59,6 +59,13 @@ static int keywest_attach_adapter(struct i2c_adapter *adapter) strlcpy(info.type, "keywest", I2C_NAME_SIZE); info.addr = keywest_ctx->addr; keywest_ctx->client = i2c_new_device(adapter, &info);
if (!keywest_ctx->client)
return -ENODEV;if (!keywest_ctx->client->driver) {
i2c_unregister_device(keywest_ctx->client);keywest_ctx->client = NULL;return -ENODEV;}
/*
- Let i2c-core delete that device on driver removal.
This looks good to me. Please add the following comment before the client->driver check in both drivers:
/* * We know the driver is already loaded, so the device should be * already bound. If not it means binding failed, and then there * is no point in keeping the device instantiated. */
Otherwise it's a little difficult to understand why the check is there.
Fair enough. I applied the patch with the comment now. Thanks!
I see this is upstream now. While the keywest fix was essentially theoretical, the tas one addresses a crash which really could happen, so I think it would be worth sending to stable for 2.6.31. What do you think? Will you take care, or do you want me to?
Agreed, it's safer to send the patch to stable tree. I'm going to submit it.
thanks,
Takashi