Hi Anirudh, On Tue, Feb 01, 2011 at 07:17:43PM +0530, Anirudh Ghayal wrote: > + > +#include <linux/mfd/pmic8058.h> > +#include <linux/input/pmic8058-vibrator.h>
Where is this header file? Shouldn't it be part of this patch? > + > +#define VIB_DRV 0x4A > + > +#define VIB_DRV_SEL_MASK 0xf8 > +#define VIB_DRV_SEL_SHIFT 0x03 > +#define VIB_DRV_EN_MANUAL_MASK 0xfc > + > +#define VIB_MAX_LEVEL_mV (3100) > +#define VIB_MIN_LEVEL_mV (1200) > +#define VIB_MAX_LEVELS (VIB_MAX_LEVEL_mV - VIB_MIN_LEVEL_mV) > + > +#define MAX_FF_SPEED 0xff > + > +/* > + * struct pmic8058_vib - structure to hold vibrator data > + * vib_input_dev: input device supporting force feedback > + * work: work structure to set the vibration parameters > + * dev: device supporting force feedback > + * pdata: platform data > + * pm_chip: reference to pmic chip to carry out read/write operations > + * speed: speed of vibration set from userland > + * state: state of vibrator > + * level: level of vibration to set in the chip > + * reg_vib_drv: VIB_DRV register value Hmm, this is kind of kerneldoc but not quite (kerneldoc's arguments start with '@'). > + * > + */ > +struct pmic8058_vib { > + struct input_dev *vib_input_dev; > + struct work_struct work; > + struct device *dev; > + const struct pmic8058_vibrator_pdata *pdata; > + struct pm8058_chip *pm_chip; > + int speed; > + int state; > + int level; > + u8 reg_vib_drv; > +}; > + > +/* > + * pmic8058_vib_read_u8 - helper to read a byte from pmic chip > + * vib: pointer to vibrator structure > + * data: placeholder for data to be read > + * reg: register address > + * > + */ > +static int pmic8058_vib_read_u8(struct pmic8058_vib *vib, > + u8 *data, u16 reg) > +{ > + int rc; > + > + rc = pm8058_read(vib->pm_chip, reg, data, 1); > + if (rc < 0) > + dev_warn(vib->dev, "Error reading pmic8058 reg 0x%x(0x%x)\n", > + reg, rc); > + return rc; > +} > + > +/* > + * pmic8058_vib_write_u8 - helper to write a byte to pmic chip > + * vib: pointer to vibrator structure > + * data: data to write > + * reg: register address > + * > + */ > +static int pmic8058_vib_write_u8(struct pmic8058_vib *vib, > + u8 data, u16 reg) > +{ > + int rc; > + > + rc = pm8058_write(vib->pm_chip, reg, &data, 1); > + if (rc < 0) > + dev_warn(vib->dev, "Error writing pmic8058 reg 0x%x(0x%x)\n", > + reg, rc); > + return rc; > +} > + > +/* > + * pmic8058_vib_set - handler to start/stop vibration > + * vib: pointer to vibrator structure > + * on: state to set > + * > + */ > +static int pmic8058_vib_set(struct pmic8058_vib *vib, int on) > +{ > + int rc; > + u8 val; > + > + val = vib->reg_vib_drv; > + > + if (on) > + val |= ((vib->level << VIB_DRV_SEL_SHIFT) & VIB_DRV_SEL_MASK); > + else > + val &= ~VIB_DRV_SEL_MASK; > + > + rc = pmic8058_vib_write_u8(vib, val, VIB_DRV); > + if (rc < 0) > + return rc; > + > + vib->reg_vib_drv = val; > + return rc; > +} > + > +/* > + * pmic8058_work_handler - worker to set vibration level > + * work: pointer to work_struct > + * > + */ > +static void pmic8058_work_handler(struct work_struct *work) > +{ > + struct pmic8058_vib *vib; > + int rc; > + u8 val; > + > + vib = container_of(work, struct pmic8058_vib, work); > + > + rc = pmic8058_vib_read_u8(vib, &val, VIB_DRV); > + if (rc < 0) > + return; > + > + /* > + * pmic vibrator supports voltage ranges from 1.2 to 3.1V, so > + * scale the level to fit into these ranges. > + */ > + if (vib->speed) { > + vib->state = 1; > + vib->level = ((VIB_MAX_LEVELS * vib->speed) / MAX_FF_SPEED) + > + VIB_MIN_LEVEL_mV; > + vib->level /= 100; > + } else { > + vib->state = 0; > + vib->level = VIB_MIN_LEVEL_mV / 100; > + } > + pmic8058_vib_set(vib, vib->state); > +} > + > +/* > + * pmic8058_vib_play_effect - function to handle vib effects. > + * dev: input device pointer > + * data: data of effect > + * effect: effect to play > + * > + * Currently this driver supports only rumble effects. > + * > + */ > +static int pmic8058_vib_play_effect(struct input_dev *dev, void *data, > + struct ff_effect *effect) > +{ > + struct pmic8058_vib *vib = input_get_drvdata(dev); > + > + vib->speed = effect->u.rumble.strong_magnitude >> 8; > + if (!vib->speed) > + vib->speed = effect->u.rumble.weak_magnitude >> 9; > + schedule_work(&vib->work); > + return 0; > +} > + > +static int __devinit pmic8058_vib_probe(struct platform_device *pdev) > + > +{ > + struct pm8058_chip *pm_chip; > + struct pmic8058_vib *vib; > + const struct pmic8058_vibrator_pdata *pdata = pdev->dev.platform_data; > + int rc; > + u8 val; > + > + pm_chip = platform_get_drvdata(pdev); The parent should not abuse platform data of _this_ device, it belongs to this driver. In fact you overwrite it with pmic8058_vib below, which means that you can't unbind the driver and bind it again. Please find other way to pass pm_chip. > + if (pm_chip == NULL) { > + dev_err(&pdev->dev, "no parent data passed in\n"); > + return -EINVAL; > + } > + > + if (!pdata) > + return -EINVAL; > + > + if (pdata->level_mV < VIB_MIN_LEVEL_mV || > + pdata->level_mV > VIB_MAX_LEVEL_mV) > + return -EINVAL; > + > + vib = kzalloc(sizeof(*vib), GFP_KERNEL); > + if (!vib) > + return -ENOMEM; > + > + vib->pm_chip = pm_chip; > + vib->pdata = pdata; > + vib->dev = &pdev->dev; > + > + INIT_WORK(&vib->work, pmic8058_work_handler); > + > + vib->vib_input_dev = input_allocate_device(); > + > + if (vib->vib_input_dev == NULL) { > + dev_err(&pdev->dev, "couldn't allocate input device\n"); > + rc = -ENOMEM; > + goto err_alloc_dev; > + } > + > + input_set_drvdata(vib->vib_input_dev, vib); > + > + vib->vib_input_dev->name = "pmic8058_vibrator"; > + vib->vib_input_dev->id.version = 1; > + vib->vib_input_dev->dev.parent = &pdev->dev; > + > + /* operate in manual mode */ > + rc = pmic8058_vib_read_u8(vib, &val, VIB_DRV); > + if (rc < 0) > + goto err_read_vib; > + val &= ~VIB_DRV_EN_MANUAL_MASK; > + rc = pmic8058_vib_write_u8(vib, val, VIB_DRV); > + if (rc < 0) > + goto err_read_vib; > + > + vib->reg_vib_drv = val; > + > + input_set_capability(vib->vib_input_dev, EV_FF, FF_RUMBLE); > + > + rc = input_ff_create_memless(vib->vib_input_dev, NULL, > + pmic8058_vib_play_effect); > + if (rc < 0) { > + dev_dbg(&pdev->dev, "couldn't register vibrator to FF\n"); > + goto err_create_memless; > + } > + > + platform_set_drvdata(pdev, vib); > + > + rc = input_register_device(vib->vib_input_dev); > + if (rc < 0) { > + dev_dbg(&pdev->dev, "couldn't register input device\n"); > + goto err_reg_input_dev; > + } platform_set_drvdata(pdev, vib) should go here so you do not need to clean it if input_register_device() fails. > + return 0; > + > +err_reg_input_dev: > + input_ff_destroy(vib->vib_input_dev); > +err_create_memless: > +err_read_vib: > + input_free_device(vib->vib_input_dev); > +err_alloc_dev: > + kfree(vib); > + return rc; > +} > + > +static int __devexit pmic8058_vib_remove(struct platform_device *pdev) > +{ > + struct pmic8058_vib *vib = platform_get_drvdata(pdev); > + > + cancel_work_sync(&vib->work); > + if (vib->state) > + pmic8058_vib_set(vib, 0); This should probably be part of pmic8058_vib_close() method. > + > + input_unregister_device(vib->vib_input_dev); > + kfree(vib); Need to call platform_set_drvdata(pdev, NULL); > + return 0; > +} > + > +static struct platform_driver pmic8058_vib_driver = { > + .probe = pmic8058_vib_probe, > + .remove = __devexit_p(pmic8058_vib_remove), > + .driver = { > + .name = "pm8058-vib", > + .owner = THIS_MODULE, > + }, > +}; What about PM? Do you need to shut off vibration if device goes to sleep? Thanks. -- Dmitry -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html