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?
Thanks,