lkml.org 
[lkml]   [2009]   [Feb]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] input: keyboard: introduce lm8323 driver
On Thu, 19 Feb 2009 14:14:14 +0200
Felipe Balbi <me@felipebalbi.com> wrote:

> From: Felipe Balbi <felipe.balbi@nokia.com>
>
> 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.
>

Please feed it through scripts/checkpatch.pl and review the resulting
report?


> +static ssize_t lm8323_pwm_store_time(struct device *dev,
> + struct device_attribute *attr, const char *buf, size_t len)
> +{
> + struct led_classdev *led_cdev = dev_get_drvdata(dev);
> + struct lm8323_pwm *pwm = cdev_to_pwm(led_cdev);
> + int ret;
> + int time;
> +
> + ret = sscanf(buf, "%d", &time);
> + /* Numbers only, please. */
> + if (ret)
> + return -EINVAL;

strict_strtoul() does a better job of this.

> + pwm->fade_time = time;
> +
> + return strlen(buf);
> +}
> +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;
> +
> + 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->id = id;
> + pwm->fade_time = 0;
> + pwm->brightness = 0;
> + pwm->desired_brightness = 0;
> + pwm->running = 0;
> + mutex_init(&pwm->lock);
> + if (name) {
> + pwm->cdev.name = name;
> + pwm->cdev.brightness_set = lm8323_pwm_set_brightness;
> + if (led_classdev_register(dev, &pwm->cdev) < 0) {

I did't see a dependency on LEDS in the Kconfig entry. Will it compile
with LEDS=n?


> + dev_err(dev, "couldn't register PWM %d\n", id);
> + return -1;
> + }
> + if (device_create_file(pwm->cdev.dev,
> + &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;
> +}
> +
> +static struct i2c_driver lm8323_i2c_driver;
> +
> +static ssize_t lm8323_show_disable(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct lm8323_chip *lm = dev_get_drvdata(dev);
> +
> + return sprintf(buf, "%u\n", !lm->kp_enabled);
> +}
> +
> +static ssize_t lm8323_set_disable(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct lm8323_chip *lm = dev_get_drvdata(dev);
> + int ret;
> + int i;
> +
> + i = sscanf(buf, "%d", &ret);

Again, strict_strto*() would be better. It will disallow input of the
form "42foo".

> + mutex_lock(&lm->lock);
> + lm->kp_enabled = !i;
> + mutex_unlock(&lm->lock);
> +
> + return count;
> +}
>
> ...
>
> +static struct i2c_driver lm8323_i2c_driver = {
> + .driver = {
> + .name = "lm8323",
> + },
> + .probe = lm8323_probe,
> + .remove = lm8323_remove,
> + .suspend = lm8323_suspend,
> + .resume = lm8323_resume,
> + .id_table = lm8323_id,
> +};

Does this compile and run OK with CONFIG_PM=n?

>
> ...
>



\
 
 \ /
  Last update: 2009-02-20 22:47    [W:0.061 / U:0.308 seconds]
©2003-2014 Jasper Spaans. Advertise on this site