Hi felipe,
On Thu, Feb 19, 2009 at 02:31:17PM +0200, Felipe Balbi wrote:
> From: Felipe Balbi <[email protected]>
>
> lm8323 is the keypad driver used in n810 device. This
> driver has been sitting in linux-omap for quite a long
> time, it's about time to get comments from upstream and
> get it merged.
>
Thank you ofr the patch. I did not quite like the hard-coding of 3 PWMs,
I think an array works better and also you enable IRQ before allocating
input device, which is not safe. Also probe() leaks input device
structure in certain scenarios. I have a fixup patch and would
appreciate if you coould try it.
Thanks!
--
Dmitry
Input: lm8323 fixups
Signed-off-by: Dmitry Torokhov <[email protected]>
---
drivers/input/keyboard/lm8323.c | 236 ++++++++++++++++-----------------------
include/linux/i2c/lm8323.h | 10 +-
2 files changed, 100 insertions(+), 146 deletions(-)
diff --git a/drivers/input/keyboard/lm8323.c b/drivers/input/keyboard/lm8323.c
index a796680..9f4ef51 100644
--- a/drivers/input/keyboard/lm8323.c
+++ b/drivers/input/keyboard/lm8323.c
@@ -137,6 +137,7 @@ struct lm8323_pwm {
struct mutex lock;
struct work_struct work;
struct led_classdev cdev;
+ struct lm8323_chip *chip;
};
struct lm8323_chip {
@@ -154,9 +155,7 @@ struct lm8323_chip {
int size_y;
int debounce_time;
int active_time;
- struct lm8323_pwm pwm1;
- struct lm8323_pwm pwm2;
- struct lm8323_pwm pwm3;
+ struct lm8323_pwm pwm[LM8323_NUM_PWMS];
};
#define client_to_lm8323(c) container_of(c, struct lm8323_chip, client)
@@ -165,20 +164,6 @@ struct lm8323_chip {
#define cdev_to_pwm(c) container_of(c, struct lm8323_pwm, cdev)
#define work_to_pwm(w) container_of(w, struct lm8323_pwm, work)
-static struct lm8323_chip *pwm_to_lm8323(struct lm8323_pwm *pwm)
-{
- switch (pwm->id) {
- case 1:
- return container_of(pwm, struct lm8323_chip, pwm1);
- case 2:
- return container_of(pwm, struct lm8323_chip, pwm2);
- case 3:
- return container_of(pwm, struct lm8323_chip, pwm3);
- default:
- return NULL;
- }
-}
-
#define LM8323_MAX_DATA 8
/*
@@ -395,6 +380,7 @@ static void lm8323_work(struct work_struct *work)
{
struct lm8323_chip *lm = work_to_lm8323(work);
u8 ints;
+ int i;
mutex_lock(&lm->lock);
@@ -414,17 +400,12 @@ static void lm8323_work(struct work_struct *work)
"reinitialising\n");
lm8323_configure(lm);
}
- if (ints & INT_PWM1) {
- dev_vdbg(&lm->client->dev, "pwm1 engine completed\n");
- pwm_done(&lm->pwm1);
- }
- if (ints & INT_PWM2) {
- dev_vdbg(&lm->client->dev, "pwm2 engine completed\n");
- pwm_done(&lm->pwm2);
- }
- if (ints & INT_PWM3) {
- dev_vdbg(&lm->client->dev, "pwm3 engine completed\n");
- pwm_done(&lm->pwm3);
+ for (i = 0; i < LM8323_NUM_PWMS; i++) {
+ if (ints & (1 << (INT_PWM1 + i))) {
+ dev_vdbg(&lm->client->dev,
+ "pwm%d engine completed\n", i);
+ pwm_done(&lm->pwm[i]);
+ }
}
}
@@ -459,9 +440,7 @@ static int lm8323_read_id(struct lm8323_chip *lm, u8 *buf)
static void lm8323_write_pwm_one(struct lm8323_pwm *pwm, int pos, u16 cmd)
{
- struct lm8323_chip *lm = pwm_to_lm8323(pwm);
-
- lm8323_write(lm, 4, LM8323_CMD_PWM_WRITE, (pos << 2) | pwm->id,
+ lm8323_write(pwm->chip, 4, LM8323_CMD_PWM_WRITE, (pos << 2) | pwm->id,
(cmd & 0xff00) >> 8, cmd & 0x00ff);
}
@@ -474,14 +453,13 @@ static void lm8323_write_pwm_one(struct lm8323_pwm *pwm, int pos, u16 cmd)
static void lm8323_write_pwm(struct lm8323_pwm *pwm, int kill,
int len, const u16 *cmds)
{
- struct lm8323_chip *lm = pwm_to_lm8323(pwm);
int i;
for (i = 0; i < len; i++)
lm8323_write_pwm_one(pwm, i, cmds[i]);
lm8323_write_pwm_one(pwm, i++, PWM_END(kill));
- lm8323_write(lm, 2, LM8323_CMD_START_PWM, pwm->id);
+ lm8323_write(pwm->chip, 2, LM8323_CMD_START_PWM, pwm->id);
pwm->running = 1;
}
@@ -546,7 +524,7 @@ static void lm8323_pwm_set_brightness(struct led_classdev *led_cdev,
enum led_brightness brightness)
{
struct lm8323_pwm *pwm = cdev_to_pwm(led_cdev);
- struct lm8323_chip *lm = pwm_to_lm8323(pwm);
+ struct lm8323_chip *lm = pwm->chip;
mutex_lock(&pwm->lock);
pwm->desired_brightness = brightness;
@@ -582,7 +560,7 @@ static ssize_t lm8323_pwm_store_time(struct device *dev,
struct led_classdev *led_cdev = dev_get_drvdata(dev);
struct lm8323_pwm *pwm = cdev_to_pwm(led_cdev);
int ret;
- int time;
+ unsigned long time;
ret = strict_strtoul(buf, 10, &time);
/* Numbers only, please. */
@@ -598,28 +576,22 @@ static DEVICE_ATTR(time, 0644, lm8323_pwm_show_time, lm8323_pwm_store_time);
static int init_pwm(struct lm8323_chip *lm, int id, struct device *dev,
const char *name)
{
- struct lm8323_pwm *pwm = NULL;
+ struct lm8323_pwm *pwm;
BUG_ON(id > 3);
- switch (id) {
- case 1:
- pwm = &lm->pwm1;
- break;
- case 2:
- pwm = &lm->pwm2;
- break;
- case 3:
- pwm = &lm->pwm3;
- break;
- }
+ pwm = &lm->pwm[id - 1];
pwm->id = id;
pwm->fade_time = 0;
pwm->brightness = 0;
pwm->desired_brightness = 0;
pwm->running = 0;
+ pwm->enabled = 0;
+ INIT_WORK(&pwm->work, lm8323_pwm_work);
mutex_init(&pwm->lock);
+ pwm->chip = lm;
+
if (name) {
pwm->cdev.name = name;
pwm->cdev.brightness_set = lm8323_pwm_set_brightness;
@@ -628,15 +600,12 @@ static int init_pwm(struct lm8323_chip *lm, int id, struct device *dev,
return -1;
}
if (device_create_file(pwm->cdev.dev,
- &dev_attr_time) < 0) {
+ &dev_attr_time) < 0) {
dev_err(dev, "couldn't register time attribute\n");
led_classdev_unregister(&pwm->cdev);
return -1;
}
- INIT_WORK(&pwm->work, lm8323_pwm_work);
pwm->enabled = 1;
- } else {
- pwm->enabled = 0;
}
return 0;
@@ -658,7 +627,7 @@ static ssize_t lm8323_set_disable(struct device *dev,
{
struct lm8323_chip *lm = dev_get_drvdata(dev);
int ret;
- int i;
+ unsigned long i;
ret = strict_strtoul(buf, 10, &i);
@@ -671,46 +640,50 @@ static ssize_t lm8323_set_disable(struct device *dev,
static DEVICE_ATTR(disable_kp, 0644, lm8323_show_disable, lm8323_set_disable);
static int lm8323_probe(struct i2c_client *client,
- const struct i2c_device_id *id)
+ const struct i2c_device_id *id)
{
- struct lm8323_platform_data *pdata;
+ struct lm8323_platform_data *pdata = client->dev.platform_data;
struct input_dev *idev;
struct lm8323_chip *lm;
- int i, err = 0;
+ int i, err;
unsigned long tmo;
u8 data[2];
- lm = kzalloc(sizeof *lm, GFP_KERNEL);
- if (!lm)
- return -ENOMEM;
-
- i2c_set_clientdata(client, lm);
- lm->client = client;
- pdata = client->dev.platform_data;
if (!pdata || !pdata->size_x || !pdata->size_y) {
dev_err(&client->dev, "missing platform_data\n");
- err = -EINVAL;
- goto fail2;
+ return -EINVAL;
}
- lm->size_x = pdata->size_x;
- if (lm->size_x > 8) {
+ if (pdata->size_x > 8) {
dev_err(&client->dev, "invalid x size %d specified\n",
- lm->size_x);
- err = -EINVAL;
- goto fail2;
+ pdata->size_x);
+ return -EINVAL;
}
- lm->size_y = pdata->size_y;
- if (lm->size_y > 12) {
+ if (pdata->size_y > 12) {
dev_err(&client->dev, "invalid y size %d specified\n",
- lm->size_y);
- err = -EINVAL;
- goto fail2;
+ pdata->size_y);
+ return -EINVAL;
}
+ lm = kzalloc(sizeof *lm, GFP_KERNEL);
+ idev = input_allocate_device();
+ if (!lm || !idev) {
+ err = -ENOMEM;
+ goto fail1;
+ }
+
+ i2c_set_clientdata(client, lm);
+
+ lm->client = client;
+ lm->idev = idev;
+ mutex_init(&lm->lock);
+ INIT_WORK(&lm->work, lm8323_work);
+
+ lm->size_x = pdata->size_x;
+ lm->size_y = pdata->size_y;
dev_vdbg(&client->dev, "Keypad size: %d x %d\n",
- lm->size_x, lm->size_y);
+ lm->size_x, lm->size_y);
lm->debounce_time = pdata->debounce_time;
lm->active_time = pdata->active_time;
@@ -726,61 +699,38 @@ static int lm8323_probe(struct i2c_client *client,
if (time_after(jiffies, tmo)) {
dev_err(&client->dev,
- "timeout waiting for initialisation\n");
+ "timeout waiting for initialisation\n");
break;
}
msleep(1);
}
+
lm8323_configure(lm);
/* If a true probe check the device */
if (lm8323_read_id(lm, data) != 0) {
dev_err(&client->dev, "device not found\n");
err = -ENODEV;
- goto fail2;
+ goto fail1;
}
- if (init_pwm(lm, 1, &client->dev, pdata->pwm1_name) < 0)
- goto fail3;
- if (init_pwm(lm, 2, &client->dev, pdata->pwm2_name) < 0)
- goto fail4;
- if (init_pwm(lm, 3, &client->dev, pdata->pwm3_name) < 0)
- goto fail5;
-
- mutex_init(&lm->lock);
- INIT_WORK(&lm->work, lm8323_work);
-
- err = request_irq(client->irq, lm8323_irq,
- IRQF_TRIGGER_FALLING | IRQF_DISABLED,
- "lm8323", lm);
- if (err) {
- dev_err(&client->dev, "could not get IRQ %d\n", client->irq);
- goto fail6;
+ for (i = 0; i < LM8323_NUM_PWMS; i++) {
+ err = init_pwm(lm, i + 1, &client->dev, pdata->pwm_names[i]);
+ if (err < 0)
+ goto fail2;
}
- device_init_wakeup(&client->dev, 1);
- enable_irq_wake(client->irq);
-
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;
- }
+ goto fail2;
- 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->name = pdata->name ? : "LM8323 keypad";
+ snprintf(lm->phys, sizeof(lm->phys),
+ "%s/input-kp", dev_name(&client->dev));
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)
@@ -792,30 +742,36 @@ static int lm8323_probe(struct i2c_client *client,
if (pdata->repeat)
__set_bit(EV_REP, idev->evbit);
- lm->idev = idev;
err = input_register_device(idev);
if (err) {
dev_dbg(&client->dev, "error registering input device\n");
- goto fail8;
+ goto fail3;
}
+ err = request_irq(client->irq, lm8323_irq,
+ IRQF_TRIGGER_FALLING | IRQF_DISABLED,
+ "lm8323", lm);
+ if (err) {
+ dev_err(&client->dev, "could not get IRQ %d\n", client->irq);
+ goto fail4;
+ }
+
+ device_init_wakeup(&client->dev, 1);
+ enable_irq_wake(client->irq);
+
return 0;
-fail8:
- device_remove_file(&client->dev, &dev_attr_disable_kp);
-fail7:
- free_irq(client->irq, lm);
-fail6:
- if (lm->pwm3.enabled)
- led_classdev_unregister(&lm->pwm3.cdev);
-fail5:
- if (lm->pwm2.enabled)
- led_classdev_unregister(&lm->pwm2.cdev);
fail4:
- if (lm->pwm1.enabled)
- led_classdev_unregister(&lm->pwm1.cdev);
+ input_unregister_device(idev);
+ idev = NULL;
fail3:
+ device_remove_file(&client->dev, &dev_attr_disable_kp);
fail2:
+ while (--i >= 0)
+ if (lm->pwm[i].enabled)
+ led_classdev_unregister(&lm->pwm[i].cdev);
+fail1:
+ input_free_device(idev);
kfree(lm);
return err;
}
@@ -823,18 +779,20 @@ fail2:
static int lm8323_remove(struct i2c_client *client)
{
struct lm8323_chip *lm = i2c_get_clientdata(client);
+ int i;
disable_irq_wake(client->irq);
free_irq(client->irq, lm);
cancel_work_sync(&lm->work);
+
input_unregister_device(lm->idev);
+
device_remove_file(&lm->client->dev, &dev_attr_disable_kp);
- if (lm->pwm3.enabled)
- led_classdev_unregister(&lm->pwm3.cdev);
- if (lm->pwm2.enabled)
- led_classdev_unregister(&lm->pwm2.cdev);
- if (lm->pwm1.enabled)
- led_classdev_unregister(&lm->pwm1.cdev);
+
+ for (i = 0; i < 3; i++)
+ if (lm->pwm[i].enabled)
+ led_classdev_unregister(&lm->pwm[i].cdev);
+
kfree(lm);
return 0;
@@ -848,6 +806,7 @@ static int lm8323_remove(struct i2c_client *client)
static int lm8323_suspend(struct i2c_client *client, pm_message_t mesg)
{
struct lm8323_chip *lm = i2c_get_clientdata(client);
+ int i;
set_irq_wake(client->irq, 0);
disable_irq(client->irq);
@@ -856,12 +815,9 @@ static int lm8323_suspend(struct i2c_client *client, pm_message_t mesg)
lm->pm_suspend = 1;
mutex_unlock(&lm->lock);
- if (lm->pwm1.enabled)
- led_classdev_suspend(&lm->pwm1.cdev);
- if (lm->pwm2.enabled)
- led_classdev_suspend(&lm->pwm2.cdev);
- if (lm->pwm3.enabled)
- led_classdev_suspend(&lm->pwm3.cdev);
+ for (i = 0; i < 3; i++)
+ if (lm->pwm[i].enabled)
+ led_classdev_suspend(&lm->pwm[i].cdev);
return 0;
}
@@ -869,17 +825,15 @@ static int lm8323_suspend(struct i2c_client *client, pm_message_t mesg)
static int lm8323_resume(struct i2c_client *client)
{
struct lm8323_chip *lm = i2c_get_clientdata(client);
+ int i;
mutex_lock(&lm->lock);
lm->pm_suspend = 0;
mutex_unlock(&lm->lock);
- if (lm->pwm1.enabled)
- led_classdev_resume(&lm->pwm1.cdev);
- if (lm->pwm2.enabled)
- led_classdev_resume(&lm->pwm2.cdev);
- if (lm->pwm3.enabled)
- led_classdev_resume(&lm->pwm3.cdev);
+ for (i = 0; i < 3; i++)
+ if (lm->pwm[i].enabled)
+ led_classdev_resume(&lm->pwm[i].cdev);
enable_irq(client->irq);
set_irq_wake(client->irq, 1);
diff --git a/include/linux/i2c/lm8323.h b/include/linux/i2c/lm8323.h
index 67e82bc..50fad47 100644
--- a/include/linux/i2c/lm8323.h
+++ b/include/linux/i2c/lm8323.h
@@ -25,7 +25,9 @@
* so keys can be mapped directly at the index of the
* LM8323 keycode instead of subtracting one.
*/
-#define LM8323_KEYMAP_SIZE (0x7f + 1)
+#define LM8323_KEYMAP_SIZE (0x7f + 1)
+
+#define LM8323_NUM_PWMS 3
struct lm8323_platform_data {
int debounce_time; /* Time to watch for key bouncing, in ms. */
@@ -36,11 +38,9 @@ struct lm8323_platform_data {
int repeat:1;
const s16 *keymap;
- char *pwm1_name; /* Device name for PWM1. */
- char *pwm2_name; /* Device name for PWM2. */
- char *pwm3_name; /* Device name for PWM3. */
+ const char *pwm_names[LM8323_NUM_PWMS];
- char *name; /* Device name. */
+ const char *name; /* Device name. */
};
#endif /* __LINUX_LM8323_H */
Hi,
On Sat, Mar 07, 2009 at 06:38:37PM -0800, Dmitry Torokhov wrote:
> Hi felipe,
>
> On Thu, Feb 19, 2009 at 02:31:17PM +0200, Felipe Balbi wrote:
> > From: Felipe Balbi <[email protected]>
> >
> > lm8323 is the keypad driver used in n810 device. This
> > driver has been sitting in linux-omap for quite a long
> > time, it's about time to get comments from upstream and
> > get it merged.
> >
>
> Thank you ofr the patch. I did not quite like the hard-coding of 3 PWMs,
> I think an array works better and also you enable IRQ before allocating
> input device, which is not safe. Also probe() leaks input device
> structure in certain scenarios. I have a fixup patch and would
> appreciate if you coould try it.
I'll try to test it on monday. Keeping the patch below and adding
linux-omap to Cc as there might be other people interested in this
patch.
> Input: lm8323 fixups
>
> Signed-off-by: Dmitry Torokhov <[email protected]>
> ---
>
> drivers/input/keyboard/lm8323.c | 236 ++++++++++++++++-----------------------
> include/linux/i2c/lm8323.h | 10 +-
> 2 files changed, 100 insertions(+), 146 deletions(-)
>
>
> diff --git a/drivers/input/keyboard/lm8323.c b/drivers/input/keyboard/lm8323.c
> index a796680..9f4ef51 100644
> --- a/drivers/input/keyboard/lm8323.c
> +++ b/drivers/input/keyboard/lm8323.c
> @@ -137,6 +137,7 @@ struct lm8323_pwm {
> struct mutex lock;
> struct work_struct work;
> struct led_classdev cdev;
> + struct lm8323_chip *chip;
> };
>
> struct lm8323_chip {
> @@ -154,9 +155,7 @@ struct lm8323_chip {
> int size_y;
> int debounce_time;
> int active_time;
> - struct lm8323_pwm pwm1;
> - struct lm8323_pwm pwm2;
> - struct lm8323_pwm pwm3;
> + struct lm8323_pwm pwm[LM8323_NUM_PWMS];
> };
>
> #define client_to_lm8323(c) container_of(c, struct lm8323_chip, client)
> @@ -165,20 +164,6 @@ struct lm8323_chip {
> #define cdev_to_pwm(c) container_of(c, struct lm8323_pwm, cdev)
> #define work_to_pwm(w) container_of(w, struct lm8323_pwm, work)
>
> -static struct lm8323_chip *pwm_to_lm8323(struct lm8323_pwm *pwm)
> -{
> - switch (pwm->id) {
> - case 1:
> - return container_of(pwm, struct lm8323_chip, pwm1);
> - case 2:
> - return container_of(pwm, struct lm8323_chip, pwm2);
> - case 3:
> - return container_of(pwm, struct lm8323_chip, pwm3);
> - default:
> - return NULL;
> - }
> -}
> -
> #define LM8323_MAX_DATA 8
>
> /*
> @@ -395,6 +380,7 @@ static void lm8323_work(struct work_struct *work)
> {
> struct lm8323_chip *lm = work_to_lm8323(work);
> u8 ints;
> + int i;
>
> mutex_lock(&lm->lock);
>
> @@ -414,17 +400,12 @@ static void lm8323_work(struct work_struct *work)
> "reinitialising\n");
> lm8323_configure(lm);
> }
> - if (ints & INT_PWM1) {
> - dev_vdbg(&lm->client->dev, "pwm1 engine completed\n");
> - pwm_done(&lm->pwm1);
> - }
> - if (ints & INT_PWM2) {
> - dev_vdbg(&lm->client->dev, "pwm2 engine completed\n");
> - pwm_done(&lm->pwm2);
> - }
> - if (ints & INT_PWM3) {
> - dev_vdbg(&lm->client->dev, "pwm3 engine completed\n");
> - pwm_done(&lm->pwm3);
> + for (i = 0; i < LM8323_NUM_PWMS; i++) {
> + if (ints & (1 << (INT_PWM1 + i))) {
> + dev_vdbg(&lm->client->dev,
> + "pwm%d engine completed\n", i);
> + pwm_done(&lm->pwm[i]);
> + }
> }
> }
>
> @@ -459,9 +440,7 @@ static int lm8323_read_id(struct lm8323_chip *lm, u8 *buf)
>
> static void lm8323_write_pwm_one(struct lm8323_pwm *pwm, int pos, u16 cmd)
> {
> - struct lm8323_chip *lm = pwm_to_lm8323(pwm);
> -
> - lm8323_write(lm, 4, LM8323_CMD_PWM_WRITE, (pos << 2) | pwm->id,
> + lm8323_write(pwm->chip, 4, LM8323_CMD_PWM_WRITE, (pos << 2) | pwm->id,
> (cmd & 0xff00) >> 8, cmd & 0x00ff);
> }
>
> @@ -474,14 +453,13 @@ static void lm8323_write_pwm_one(struct lm8323_pwm *pwm, int pos, u16 cmd)
> static void lm8323_write_pwm(struct lm8323_pwm *pwm, int kill,
> int len, const u16 *cmds)
> {
> - struct lm8323_chip *lm = pwm_to_lm8323(pwm);
> int i;
>
> for (i = 0; i < len; i++)
> lm8323_write_pwm_one(pwm, i, cmds[i]);
>
> lm8323_write_pwm_one(pwm, i++, PWM_END(kill));
> - lm8323_write(lm, 2, LM8323_CMD_START_PWM, pwm->id);
> + lm8323_write(pwm->chip, 2, LM8323_CMD_START_PWM, pwm->id);
> pwm->running = 1;
> }
>
> @@ -546,7 +524,7 @@ static void lm8323_pwm_set_brightness(struct led_classdev *led_cdev,
> enum led_brightness brightness)
> {
> struct lm8323_pwm *pwm = cdev_to_pwm(led_cdev);
> - struct lm8323_chip *lm = pwm_to_lm8323(pwm);
> + struct lm8323_chip *lm = pwm->chip;
>
> mutex_lock(&pwm->lock);
> pwm->desired_brightness = brightness;
> @@ -582,7 +560,7 @@ static ssize_t lm8323_pwm_store_time(struct device *dev,
> struct led_classdev *led_cdev = dev_get_drvdata(dev);
> struct lm8323_pwm *pwm = cdev_to_pwm(led_cdev);
> int ret;
> - int time;
> + unsigned long time;
>
> ret = strict_strtoul(buf, 10, &time);
> /* Numbers only, please. */
> @@ -598,28 +576,22 @@ static DEVICE_ATTR(time, 0644, lm8323_pwm_show_time, lm8323_pwm_store_time);
> static int init_pwm(struct lm8323_chip *lm, int id, struct device *dev,
> const char *name)
> {
> - struct lm8323_pwm *pwm = NULL;
> + struct lm8323_pwm *pwm;
>
> BUG_ON(id > 3);
>
> - switch (id) {
> - case 1:
> - pwm = &lm->pwm1;
> - break;
> - case 2:
> - pwm = &lm->pwm2;
> - break;
> - case 3:
> - pwm = &lm->pwm3;
> - break;
> - }
> + pwm = &lm->pwm[id - 1];
>
> pwm->id = id;
> pwm->fade_time = 0;
> pwm->brightness = 0;
> pwm->desired_brightness = 0;
> pwm->running = 0;
> + pwm->enabled = 0;
> + INIT_WORK(&pwm->work, lm8323_pwm_work);
> mutex_init(&pwm->lock);
> + pwm->chip = lm;
> +
> if (name) {
> pwm->cdev.name = name;
> pwm->cdev.brightness_set = lm8323_pwm_set_brightness;
> @@ -628,15 +600,12 @@ static int init_pwm(struct lm8323_chip *lm, int id, struct device *dev,
> return -1;
> }
> if (device_create_file(pwm->cdev.dev,
> - &dev_attr_time) < 0) {
> + &dev_attr_time) < 0) {
> dev_err(dev, "couldn't register time attribute\n");
> led_classdev_unregister(&pwm->cdev);
> return -1;
> }
> - INIT_WORK(&pwm->work, lm8323_pwm_work);
> pwm->enabled = 1;
> - } else {
> - pwm->enabled = 0;
> }
>
> return 0;
> @@ -658,7 +627,7 @@ static ssize_t lm8323_set_disable(struct device *dev,
> {
> struct lm8323_chip *lm = dev_get_drvdata(dev);
> int ret;
> - int i;
> + unsigned long i;
>
> ret = strict_strtoul(buf, 10, &i);
>
> @@ -671,46 +640,50 @@ static ssize_t lm8323_set_disable(struct device *dev,
> static DEVICE_ATTR(disable_kp, 0644, lm8323_show_disable, lm8323_set_disable);
>
> static int lm8323_probe(struct i2c_client *client,
> - const struct i2c_device_id *id)
> + const struct i2c_device_id *id)
> {
> - struct lm8323_platform_data *pdata;
> + struct lm8323_platform_data *pdata = client->dev.platform_data;
> struct input_dev *idev;
> struct lm8323_chip *lm;
> - int i, err = 0;
> + int i, err;
> unsigned long tmo;
> u8 data[2];
>
> - lm = kzalloc(sizeof *lm, GFP_KERNEL);
> - if (!lm)
> - return -ENOMEM;
> -
> - i2c_set_clientdata(client, lm);
> - lm->client = client;
> - pdata = client->dev.platform_data;
> if (!pdata || !pdata->size_x || !pdata->size_y) {
> dev_err(&client->dev, "missing platform_data\n");
> - err = -EINVAL;
> - goto fail2;
> + return -EINVAL;
> }
>
> - lm->size_x = pdata->size_x;
> - if (lm->size_x > 8) {
> + if (pdata->size_x > 8) {
> dev_err(&client->dev, "invalid x size %d specified\n",
> - lm->size_x);
> - err = -EINVAL;
> - goto fail2;
> + pdata->size_x);
> + return -EINVAL;
> }
>
> - lm->size_y = pdata->size_y;
> - if (lm->size_y > 12) {
> + if (pdata->size_y > 12) {
> dev_err(&client->dev, "invalid y size %d specified\n",
> - lm->size_y);
> - err = -EINVAL;
> - goto fail2;
> + pdata->size_y);
> + return -EINVAL;
> }
>
> + lm = kzalloc(sizeof *lm, GFP_KERNEL);
> + idev = input_allocate_device();
> + if (!lm || !idev) {
> + err = -ENOMEM;
> + goto fail1;
> + }
> +
> + i2c_set_clientdata(client, lm);
> +
> + lm->client = client;
> + lm->idev = idev;
> + mutex_init(&lm->lock);
> + INIT_WORK(&lm->work, lm8323_work);
> +
> + lm->size_x = pdata->size_x;
> + lm->size_y = pdata->size_y;
> dev_vdbg(&client->dev, "Keypad size: %d x %d\n",
> - lm->size_x, lm->size_y);
> + lm->size_x, lm->size_y);
>
> lm->debounce_time = pdata->debounce_time;
> lm->active_time = pdata->active_time;
> @@ -726,61 +699,38 @@ static int lm8323_probe(struct i2c_client *client,
>
> if (time_after(jiffies, tmo)) {
> dev_err(&client->dev,
> - "timeout waiting for initialisation\n");
> + "timeout waiting for initialisation\n");
> break;
> }
>
> msleep(1);
> }
> +
> lm8323_configure(lm);
>
> /* If a true probe check the device */
> if (lm8323_read_id(lm, data) != 0) {
> dev_err(&client->dev, "device not found\n");
> err = -ENODEV;
> - goto fail2;
> + goto fail1;
> }
>
> - if (init_pwm(lm, 1, &client->dev, pdata->pwm1_name) < 0)
> - goto fail3;
> - if (init_pwm(lm, 2, &client->dev, pdata->pwm2_name) < 0)
> - goto fail4;
> - if (init_pwm(lm, 3, &client->dev, pdata->pwm3_name) < 0)
> - goto fail5;
> -
> - mutex_init(&lm->lock);
> - INIT_WORK(&lm->work, lm8323_work);
> -
> - err = request_irq(client->irq, lm8323_irq,
> - IRQF_TRIGGER_FALLING | IRQF_DISABLED,
> - "lm8323", lm);
> - if (err) {
> - dev_err(&client->dev, "could not get IRQ %d\n", client->irq);
> - goto fail6;
> + for (i = 0; i < LM8323_NUM_PWMS; i++) {
> + err = init_pwm(lm, i + 1, &client->dev, pdata->pwm_names[i]);
> + if (err < 0)
> + goto fail2;
> }
>
> - device_init_wakeup(&client->dev, 1);
> - enable_irq_wake(client->irq);
> -
> 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;
> - }
> + goto fail2;
>
> - 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->name = pdata->name ? : "LM8323 keypad";
> + snprintf(lm->phys, sizeof(lm->phys),
> + "%s/input-kp", dev_name(&client->dev));
> 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)
> @@ -792,30 +742,36 @@ static int lm8323_probe(struct i2c_client *client,
> if (pdata->repeat)
> __set_bit(EV_REP, idev->evbit);
>
> - lm->idev = idev;
> err = input_register_device(idev);
> if (err) {
> dev_dbg(&client->dev, "error registering input device\n");
> - goto fail8;
> + goto fail3;
> }
>
> + err = request_irq(client->irq, lm8323_irq,
> + IRQF_TRIGGER_FALLING | IRQF_DISABLED,
> + "lm8323", lm);
> + if (err) {
> + dev_err(&client->dev, "could not get IRQ %d\n", client->irq);
> + goto fail4;
> + }
> +
> + device_init_wakeup(&client->dev, 1);
> + enable_irq_wake(client->irq);
> +
> return 0;
>
> -fail8:
> - device_remove_file(&client->dev, &dev_attr_disable_kp);
> -fail7:
> - free_irq(client->irq, lm);
> -fail6:
> - if (lm->pwm3.enabled)
> - led_classdev_unregister(&lm->pwm3.cdev);
> -fail5:
> - if (lm->pwm2.enabled)
> - led_classdev_unregister(&lm->pwm2.cdev);
> fail4:
> - if (lm->pwm1.enabled)
> - led_classdev_unregister(&lm->pwm1.cdev);
> + input_unregister_device(idev);
> + idev = NULL;
> fail3:
> + device_remove_file(&client->dev, &dev_attr_disable_kp);
> fail2:
> + while (--i >= 0)
> + if (lm->pwm[i].enabled)
> + led_classdev_unregister(&lm->pwm[i].cdev);
> +fail1:
> + input_free_device(idev);
> kfree(lm);
> return err;
> }
> @@ -823,18 +779,20 @@ fail2:
> static int lm8323_remove(struct i2c_client *client)
> {
> struct lm8323_chip *lm = i2c_get_clientdata(client);
> + int i;
>
> disable_irq_wake(client->irq);
> free_irq(client->irq, lm);
> cancel_work_sync(&lm->work);
> +
> input_unregister_device(lm->idev);
> +
> device_remove_file(&lm->client->dev, &dev_attr_disable_kp);
> - if (lm->pwm3.enabled)
> - led_classdev_unregister(&lm->pwm3.cdev);
> - if (lm->pwm2.enabled)
> - led_classdev_unregister(&lm->pwm2.cdev);
> - if (lm->pwm1.enabled)
> - led_classdev_unregister(&lm->pwm1.cdev);
> +
> + for (i = 0; i < 3; i++)
> + if (lm->pwm[i].enabled)
> + led_classdev_unregister(&lm->pwm[i].cdev);
> +
> kfree(lm);
>
> return 0;
> @@ -848,6 +806,7 @@ static int lm8323_remove(struct i2c_client *client)
> static int lm8323_suspend(struct i2c_client *client, pm_message_t mesg)
> {
> struct lm8323_chip *lm = i2c_get_clientdata(client);
> + int i;
>
> set_irq_wake(client->irq, 0);
> disable_irq(client->irq);
> @@ -856,12 +815,9 @@ static int lm8323_suspend(struct i2c_client *client, pm_message_t mesg)
> lm->pm_suspend = 1;
> mutex_unlock(&lm->lock);
>
> - if (lm->pwm1.enabled)
> - led_classdev_suspend(&lm->pwm1.cdev);
> - if (lm->pwm2.enabled)
> - led_classdev_suspend(&lm->pwm2.cdev);
> - if (lm->pwm3.enabled)
> - led_classdev_suspend(&lm->pwm3.cdev);
> + for (i = 0; i < 3; i++)
> + if (lm->pwm[i].enabled)
> + led_classdev_suspend(&lm->pwm[i].cdev);
>
> return 0;
> }
> @@ -869,17 +825,15 @@ static int lm8323_suspend(struct i2c_client *client, pm_message_t mesg)
> static int lm8323_resume(struct i2c_client *client)
> {
> struct lm8323_chip *lm = i2c_get_clientdata(client);
> + int i;
>
> mutex_lock(&lm->lock);
> lm->pm_suspend = 0;
> mutex_unlock(&lm->lock);
>
> - if (lm->pwm1.enabled)
> - led_classdev_resume(&lm->pwm1.cdev);
> - if (lm->pwm2.enabled)
> - led_classdev_resume(&lm->pwm2.cdev);
> - if (lm->pwm3.enabled)
> - led_classdev_resume(&lm->pwm3.cdev);
> + for (i = 0; i < 3; i++)
> + if (lm->pwm[i].enabled)
> + led_classdev_resume(&lm->pwm[i].cdev);
>
> enable_irq(client->irq);
> set_irq_wake(client->irq, 1);
> diff --git a/include/linux/i2c/lm8323.h b/include/linux/i2c/lm8323.h
> index 67e82bc..50fad47 100644
> --- a/include/linux/i2c/lm8323.h
> +++ b/include/linux/i2c/lm8323.h
> @@ -25,7 +25,9 @@
> * so keys can be mapped directly at the index of the
> * LM8323 keycode instead of subtracting one.
> */
> -#define LM8323_KEYMAP_SIZE (0x7f + 1)
> +#define LM8323_KEYMAP_SIZE (0x7f + 1)
> +
> +#define LM8323_NUM_PWMS 3
>
> struct lm8323_platform_data {
> int debounce_time; /* Time to watch for key bouncing, in ms. */
> @@ -36,11 +38,9 @@ struct lm8323_platform_data {
> int repeat:1;
> const s16 *keymap;
>
> - char *pwm1_name; /* Device name for PWM1. */
> - char *pwm2_name; /* Device name for PWM2. */
> - char *pwm3_name; /* Device name for PWM3. */
> + const char *pwm_names[LM8323_NUM_PWMS];
>
> - char *name; /* Device name. */
> + const char *name; /* Device name. */
> };
>
> #endif /* __LINUX_LM8323_H */
--
balbi