On Sun, May 10, 2009 at 5:01 AM, Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx> wrote:
> P.S. I made a bunch of random changes, mostly to streamline the probe
> code (below), hopfully I did not break anything.
>
> diff --git a/drivers/input/keyboard/max7359_keypad.c b/drivers/input/keyboard/max7359_keypad.c
> index 918386c..280edc3 100644
> --- a/drivers/input/keyboard/max7359_keypad.c
> +++ b/drivers/input/keyboard/max7359_keypad.c
> @@ -204,71 +198,59 @@ static int __devinit max7359_probe(struct i2c_client *client,
> const struct i2c_device_id *id)
> {
> struct max7359_keypad *keypad;
> - struct max7359_keypad_platform_data *pdata;
> + struct max7359_keypad_platform_data *pdata = client->dev.platform_data;
> struct input_dev *input_dev;
> int ret;
> + int error;
>
> - keypad = kzalloc(sizeof(struct max7359_keypad), GFP_KERNEL);
> - if (keypad == NULL) {
> - dev_err(&client->dev, "failed to allocate driver data\n");
> - return -ENOMEM;
> + if (!client->irq) {
> + dev_err(&client->dev, "The irq number should not be zero\n");
> + return -EINVAL;
> }
>
> - keypad->client = client;
> - pdata = keypad->pdata = client->dev.platform_data;
> - i2c_set_clientdata(client, keypad);
> -
> /* Detect MAX7359: The initial Keys FIFO value is '0x3F' */
> ret = max7359_read_reg(client, MAX7359_REG_KEYFIFO);
> if (ret < 0) {
> dev_err(&client->dev, "failed to detect device\n");
> - goto failed_free;
> - } else {
> - dev_info(&client->dev, "keys FIFO is 0x%02x"
> - ", succeeded in detecting device\n", ret);
> + return -ENODEV;
> }
>
> - /* Initialize MAX7359 */
> - max7359_initialize(client);
> + dev_dbg(&client->dev, "keys FIFO is 0x%02x\n", ret);
>
> - /* Create and register the input driver. */
> + keypad = kzalloc(sizeof(struct max7359_keypad), GFP_KERNEL);
> input_dev = input_allocate_device();
> - if (!input_dev) {
> - dev_err(&client->dev, "failed to allocate input device\n");
> - ret = -ENOMEM;
> - goto failed_free;
> + if (!keypad || !input_dev) {
> + dev_err(&client->dev, "failed to allocate memory\n");
> + error = -ENOMEM;
> + goto failed_free_mem;
> }
Hi Dmitry,
Thank you for streamlining this driver. I confirmed this streamlined driver
is nicely working on my S3C6410 board and of course I mostly like your
changes. But I'm afraid this 'error' variable is not used at the final
return statement in the probe function.
Thanks & Regards,
Kyuwon
P.S. You may need this kind of patch.
--
diff --git a/drivers/input/keyboard/max7359_keypad.c b/drivers/input/keyboard/max7359_keypad.c
index 280edc3..6e09e0d 100644
--- a/drivers/input/keyboard/max7359_keypad.c
+++ b/drivers/input/keyboard/max7359_keypad.c
@@ -200,7 +200,6 @@ static int __devinit max7359_probe(struct i2c_client *client,
struct max7359_keypad *keypad;
struct max7359_keypad_platform_data *pdata = client->dev.platform_data;
struct input_dev *input_dev;
- int ret;
int error;
if (!client->irq) {
@@ -209,13 +208,13 @@ static int __devinit max7359_probe(struct i2c_client *client,
}
/* Detect MAX7359: The initial Keys FIFO value is '0x3F' */
- ret = max7359_read_reg(client, MAX7359_REG_KEYFIFO);
- if (ret < 0) {
+ error = max7359_read_reg(client, MAX7359_REG_KEYFIFO);
+ if (error < 0) {
dev_err(&client->dev, "failed to detect device\n");
return -ENODEV;
}
- dev_dbg(&client->dev, "keys FIFO is 0x%02x\n", ret);
+ dev_dbg(&client->dev, "keys FIFO is 0x%02x\n", error);
keypad = kzalloc(sizeof(struct max7359_keypad), GFP_KERNEL);
input_dev = input_allocate_device();
@@ -254,8 +253,8 @@ static int __devinit max7359_probe(struct i2c_client *client,
}
/* Register the input device */
- ret = input_register_device(input_dev);
- if (ret) {
+ error = input_register_device(input_dev);
+ if (error) {
dev_err(&client->dev, "failed to register input device\n");
goto failed_free_irq;
}
@@ -273,7 +272,7 @@ failed_free_irq:
failed_free_mem:
input_free_device(input_dev);
kfree(keypad);
- return ret;
+ return error;
}
static int __devexit max7359_remove(struct i2c_client *client)
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
