Re: [PATCH] input: keyboard: introduce lm8323 driver

[Trilok Soni]

Hi Flipe,

> diff --git a/drivers/input/keyboard/lm8323.c b/drivers/input/keyboard/lm8323.c
> new file mode 100644
> index 0000000..27da93c
> --- /dev/null
> +++ b/drivers/input/keyboard/lm8323.c
> @@ -0,0 +1,913 @@
> +/*
> + * drivers/i2c/chips/lm8323.c
> + *
> + * Copyright (C) 2007 Nokia Corporation

You might want to update this if you have updated some code before
submitting to kernel list.

> +
> +       err = request_irq(client->irq, lm8323_irq,
> +                         IRQF_TRIGGER_FALLING | IRQF_DISABLED |
> +                         IRQF_SAMPLE_RANDOM, "lm8323", lm);

No need to add IRQF_SAMPLE_RANDOM, it is done input subsystem. Please
check input_report_event.

> +
> +       set_irq_wake(client->irq, 1);

Shall we move to device_init_wakup + enable_irq_wake friends ?

> +
> +       lm->kp_enabled = 1;
> +       err = device_create_file(&client->dev, &dev_attr_disable_kp);
> +       if (err < 0)
> +               goto fail7;
> +
> +       idev = input_allocate_device();
> +       if (!idev) {
> +               err = -ENOMEM;
> +               goto fail8;
> +       }
> +
> +       if (pdata->name)
> +               idev->name = pdata->name;
> +       else
> +               idev->name = "LM8323 keypad";
> +       snprintf(lm->phys, sizeof(lm->phys), "%s/input-kp", client->dev.bus_id);
> +       idev->phys = lm->phys;
> +
> +       lm->keys_down = 0;
> +       idev->evbit[0] = BIT(EV_KEY);
> +       for (i = 0; i < LM8323_KEYMAP_SIZE; i++) {
> +               if (pdata->keymap[i] > 0)
> +                       set_bit(pdata->keymap[i], idev->keybit);

No difference but better to use __set_bit, as atomicity is not required here.

> +
> +               lm->keymap[i] = pdata->keymap[i];
> +       }
> +
> +       if (pdata->repeat)
> +               set_bit(EV_REP, idev->evbit);

Ditto.

Also in lm8323_remove we need to set_irq_wake(..., 0) right?

> +
> diff --git a/include/linux/i2c/lm8323.h b/include/linux/i2c/lm8323.h
> new file mode 100644
> index 0000000..5eae5cc
> --- /dev/null
> +++ b/include/linux/i2c/lm8323.h
> @@ -0,0 +1,35 @@
> +/*
> + * include/lm8323.h

Path problem...better to remove it.




---Trilok Soni
http://triloksoni.wordpress.com
http://www.linkedin.com/in/triloksoni
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/