Re: Input: add MAX7359 key switch controller driver
From: Trilok Soni <hidden>
Date: 2009-05-06 13:44:59
Also in:
linux-i2c, lkml
Hi Kim, On Wed, May 6, 2009 at 11:51 AM, Kim Kyuwon [off-list ref] wrote:
The Maxim MAX7359 is a I2C interfaced key switch controller which provides microprocessors with management of up to 64 key switches. This patch adds support for the MAX7359 key switch controller.
Could you please restrict the commit text line to 80/72 columns? It will look good.
+static irqreturn_t max7359_interrupt(int irq, void *dev_id)
+{
+ struct max7359_keypad *keypad = (struct max7359_keypad *) dev_id;No need of casting. Please remove.
+
+ if (!work_pending(&keypad->work)) {
+ disable_irq_nosync(keypad->irq);
+ schedule_work(&keypad->work);
+ } else {
+ dev_err(&keypad->client->dev,
+ "Another job is currently pending \n");
+ }
+
+ return IRQ_HANDLED;
+}
++ +static int __devinit max7359_probe(struct i2c_client *client, + const struct i2c_device_id *id)
You may want to remove __devinit as it is i2c client driver . I have added linux-i2c for i2c specific review.
+static int max7359_suspend(struct i2c_client *client, pm_message_t mesg)
+{
+ /* If no keys are pressed, enter sleep mode for 8192 ms */
+ max7359_write_reg(client, MAX7359_REG_SLEEP, 0x01);
+
+ return 0;
+}
+
+static int max7359_resume(struct i2c_client *client)
+{
+ /* Restore the default setting (autosleep for 256 ms) */
+ max7359_write_reg(client, MAX7359_REG_SLEEP, 0x07);
+
+ return 0;
+}Protect these two function with #ifdef CONFIG_PM please.
+
+
+static struct i2c_driver max7359_i2c_driver = {
+ .driver = {
+ .name = "max7359",
+ },
+ .probe = max7359_probe,
+ .remove = __exit_p(max7359_remove),+};
+
+
+MODULE_AUTHOR("Kim Kyuwon [off-list ref]");
+MODULE_DESCRIPTION("MAX7359 Key Switch Controller Driver");
+MODULE_LICENSE("GPL v2");MODULE_ALIAS ??
quoted hunk ↗ jump to hunk
diff --git a/include/linux/max7359_keypad.h b/include/linux/max7359_keypad.h
+ +#define KEY(row, col, val) (((row) << 28) | ((col) << 24) | (val))
Looks like this macro is highly used by many input drivers, anyone looking at it to place it at common location? -- ---Trilok Soni http://triloksoni.wordpress.com http://www.linkedin.com/in/triloksoni -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html