lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20110611231854.GA3979@core.coreip.homeip.net>
Date:	Sat, 11 Jun 2011 16:18:54 -0700
From:	Dmitry Torokhov <dmitry.torokhov@...il.com>
To:	Peter Ujfalusi <peter.ujfalusi@...com>
Cc:	Liam Girdwood <lrg@...com>, Tony Lindgren <tony@...mide.com>,
	Mark Brown <broonie@...nsource.wolfsonmicro.com>,
	Samuel Ortiz <sameo@...ux.intel.com>,
	linux-input@...r.kernel.org, linux-omap@...r.kernel.org,
	linux-kernel@...r.kernel.org, alsa-devel@...a-project.org,
	Misael Lopez Cruz <misael.lopez@...com>,
	Jorge Eduardo Candelaria <jorge.candelaria@...com>
Subject: Re: [PATCH v4 11/18] input: Add initial support for TWL6040 vibrator

Hi Peter,

On Fri, Jun 10, 2011 at 02:54:29PM +0300, Peter Ujfalusi wrote:
> From: Misael Lopez Cruz <misael.lopez@...com>
> 
> Add twl6040_vibra as a child of MFD device twl6040_codec. This
> implementation covers the PCM-to-PWM mode of TWL6040 vibrator
> module.
> 
> Signed-off-by: Jorge Eduardo Candelaria <jorge.candelaria@...com>
> Signed-off-by: Misael Lopez Cruz <misael.lopez@...com>
> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@...com>
> ---
>  drivers/input/misc/Kconfig         |   11 +
>  drivers/input/misc/Makefile        |    1 +
>  drivers/input/misc/twl6040-vibra.c |  428 ++++++++++++++++++++++++++++++++++++
>  include/linux/i2c/twl.h            |    8 +
>  4 files changed, 448 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/input/misc/twl6040-vibra.c
> 
> diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
> index 077309a..d1bf872 100644
> --- a/drivers/input/misc/Kconfig
> +++ b/drivers/input/misc/Kconfig
> @@ -275,6 +275,17 @@ config INPUT_TWL4030_VIBRA
>  	  To compile this driver as a module, choose M here. The module will
>  	  be called twl4030_vibra.
>  
> +config INPUT_TWL6040_VIBRA
> +	tristate "Support for TWL6040 Vibrator"
> +	depends on TWL4030_CORE
> +	select TWL6040_CORE
> +	select INPUT_FF_MEMLESS
> +	help
> +	  This option enables support for TWL6040 Vibrator Driver.
> +
> +	  To compile this driver as a module, choose M here. The module will
> +	  be called twl6040_vibra.
> +
>  config INPUT_UINPUT
>  	tristate "User level driver support"
>  	help
> diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
> index 38efb2c..4da7c3a 100644
> --- a/drivers/input/misc/Makefile
> +++ b/drivers/input/misc/Makefile
> @@ -40,6 +40,7 @@ obj-$(CONFIG_INPUT_SGI_BTNS)		+= sgi_btns.o
>  obj-$(CONFIG_INPUT_SPARCSPKR)		+= sparcspkr.o
>  obj-$(CONFIG_INPUT_TWL4030_PWRBUTTON)	+= twl4030-pwrbutton.o
>  obj-$(CONFIG_INPUT_TWL4030_VIBRA)	+= twl4030-vibra.o
> +obj-$(CONFIG_INPUT_TWL6040_VIBRA)	+= twl6040-vibra.o
>  obj-$(CONFIG_INPUT_UINPUT)		+= uinput.o
>  obj-$(CONFIG_INPUT_WISTRON_BTNS)	+= wistron_btns.o
>  obj-$(CONFIG_INPUT_WM831X_ON)		+= wm831x-on.o
> diff --git a/drivers/input/misc/twl6040-vibra.c b/drivers/input/misc/twl6040-vibra.c
> new file mode 100644
> index 0000000..5a54515
> --- /dev/null
> +++ b/drivers/input/misc/twl6040-vibra.c
> @@ -0,0 +1,428 @@
> +/*
> + * twl6040-vibra.c - TWL6040 Vibrator driver
> + *
> + * Author:      Jorge Eduardo Candelaria <jorge.candelaria@...com>
> + * Author:      Misael Lopez Cruz <misael.lopez@...com>
> + *
> + * Copyright:   (C) 2011 Texas Instruments, Inc.
> + *
> + * Based on twl4030-vibra.c by Henrik Saari <henrik.saari@...ia.com>
> + *				Felipe Balbi <felipe.balbi@...ia.com>
> + *				Jari Vanhala <ext-javi.vanhala@...ia.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
> + * 02110-1301 USA
> + *
> + */
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/workqueue.h>
> +#include <linux/i2c/twl.h>
> +#include <linux/mfd/twl6040.h>
> +#include <linux/slab.h>
> +#include <linux/delay.h>
> +#include <linux/regulator/consumer.h>
> +
> +#define EFFECT_DIR_180_DEG	0x8000
> +
> +/* Recommended modulation index 85% */
> +#define TWL6040_VIBRA_MOD	85
> +
> +#define TWL6040_NUM_SUPPLIES 2
> +
> +struct vibra_info {
> +	struct device *dev;
> +	struct input_dev *input_dev;
> +	struct workqueue_struct *workqueue;
> +	struct work_struct play_work;
> +	struct mutex mutex;
> +
> +	bool enabled;
> +	int weak_speed;
> +	int strong_speed;
> +	int direction;
> +
> +	unsigned int vibldrv_res;
> +	unsigned int vibrdrv_res;
> +	unsigned int viblmotor_res;
> +	unsigned int vibrmotor_res;
> +
> +	struct regulator_bulk_data supplies[TWL6040_NUM_SUPPLIES];
> +
> +	struct twl6040 *twl6040;
> +};
> +
> +static irqreturn_t twl6040_vib_irq_handler(int irq, void *data)
> +{
> +	struct vibra_info *info = data;
> +	struct twl6040 *twl6040 = info->twl6040;
> +	u8 status;
> +
> +	status = twl6040_reg_read(twl6040, TWL6040_REG_STATUS);
> +	if (status & TWL6040_VIBLOCDET) {
> +		dev_warn(info->dev, "Left Vibrator overcurrent detected\n");
> +		twl6040_clear_bits(twl6040, TWL6040_REG_VIBCTLL,
> +				   TWL6040_VIBENAL);
> +	}
> +	if (status & TWL6040_VIBROCDET) {
> +		dev_warn(info->dev, "Right Vibrator overcurrent detected\n");
> +		twl6040_clear_bits(twl6040, TWL6040_REG_VIBCTLR,
> +				   TWL6040_VIBENAR);
> +	}
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static void twl6040_vibra_enable(struct vibra_info *info)
> +{
> +	struct twl6040 *twl6040 = info->twl6040;
> +	int ret = 0;

Initialization is not needed.

> +
> +	ret = regulator_bulk_enable(ARRAY_SIZE(info->supplies), info->supplies);
> +	if (ret) {
> +		dev_err(info->dev, "failed to enable regulators %d\n", ret);
> +		return;
> +	}
> +
> +	twl6040_power(info->twl6040, 1);
> +	if (twl6040_get_rev(twl6040) <= TWL6040_REV_ES1_1) {
> +		/*
> +		 * ERRATA: Disable overcurrent protection for at least
> +		 * 3ms when enabling vibrator drivers to avoid false
> +		 * overcurrent detection
> +		 */
> +		twl6040_reg_write(twl6040, TWL6040_REG_VIBCTLL,
> +				  TWL6040_VIBENAL | TWL6040_VIBCTRLL);
> +		twl6040_reg_write(twl6040, TWL6040_REG_VIBCTLR,
> +				  TWL6040_VIBENAR | TWL6040_VIBCTRLR);
> +		usleep_range(3000, 3500);
> +	}
> +
> +	twl6040_reg_write(twl6040, TWL6040_REG_VIBCTLL,
> +			  TWL6040_VIBENAL);
> +	twl6040_reg_write(twl6040, TWL6040_REG_VIBCTLR,
> +			  TWL6040_VIBENAR);
> +
> +	info->enabled = true;
> +}
> +
> +static void twl6040_vibra_disable(struct vibra_info *info)
> +{
> +	struct twl6040 *twl6040 = info->twl6040;
> +
> +	twl6040_reg_write(twl6040, TWL6040_REG_VIBCTLL, 0x00);
> +	twl6040_reg_write(twl6040, TWL6040_REG_VIBCTLR, 0x00);
> +	twl6040_power(info->twl6040, 0);
> +
> +	regulator_bulk_disable(ARRAY_SIZE(info->supplies), info->supplies);
> +
> +	info->enabled = false;
> +}
> +
> +static u8 twl6040_vibra_code(int vddvib, int vibdrv_res, int motor_res,
> +			     int speed, int direction)
> +{
> +	int vpk, max_code;
> +	u8 vibdat;
> +
> +	/* output swing */
> +	vpk = (vddvib * motor_res * TWL6040_VIBRA_MOD) /
> +		(100 * (vibdrv_res + motor_res));
> +
> +	/* 50mV per VIBDAT code step */
> +	max_code = vpk / 50;
> +	if (max_code > TWL6040_VIBDAT_MAX)
> +		max_code = TWL6040_VIBDAT_MAX;
> +
> +	/* scale speed to max allowed code */
> +	vibdat = (u8)((speed * max_code) / USHRT_MAX);
> +
> +	/* 2's complement for direction > 180 degrees */
> +	vibdat *= direction;
> +
> +	return vibdat;
> +}
> +
> +static void twl6040_vibra_set_effect(struct vibra_info *info)
> +{
> +	struct twl6040 *twl6040 = info->twl6040;
> +	u8 vibdatl, vibdatr;
> +	int volt;
> +
> +	/* weak motor */
> +	volt = regulator_get_voltage(info->supplies[0].consumer) / 1000;
> +	vibdatl = twl6040_vibra_code(volt, info->vibldrv_res,
> +				     info->viblmotor_res,
> +				     info->weak_speed, info->direction);
> +
> +	/* strong motor */
> +	volt = regulator_get_voltage(info->supplies[1].consumer) / 1000;
> +	vibdatr = twl6040_vibra_code(volt, info->vibrdrv_res,
> +				     info->vibrmotor_res,
> +				     info->strong_speed, info->direction);
> +
> +	twl6040_reg_write(twl6040, TWL6040_REG_VIBDATL, vibdatl);
> +	twl6040_reg_write(twl6040, TWL6040_REG_VIBDATR, vibdatr);
> +}
> +
> +static void vibra_play_work(struct work_struct *work)
> +{
> +	struct vibra_info *info = container_of(work,
> +				struct vibra_info, play_work);
> +
> +	mutex_lock(&info->mutex);
> +
> +	if (info->weak_speed || info->strong_speed) {
> +		if (!info->enabled)
> +			twl6040_vibra_enable(info);
> +
> +		twl6040_vibra_set_effect(info);
> +	} else if (info->enabled)
> +		twl6040_vibra_disable(info);

Why do we play with enabling/disabling the device here? Nobody can
request playing of an effect unless the device has been opened so if we
manage power state in open/close methods we should not be doing it here
I think.

> +
> +	mutex_unlock(&info->mutex);
> +}
> +
> +static int vibra_play(struct input_dev *input, void *data,
> +		      struct ff_effect *effect)
> +{
> +	struct vibra_info *info = input_get_drvdata(input);
> +	int ret;
> +
> +	info->weak_speed = effect->u.rumble.weak_magnitude;
> +	info->strong_speed = effect->u.rumble.strong_magnitude;
> +	info->direction = effect->direction < EFFECT_DIR_180_DEG ? 1 : -1;
> +
> +	ret = queue_work(info->workqueue, &info->play_work);
> +	if (!ret) {
> +		dev_info(&input->dev, "work is already on queue\n");
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static int twl6040_vibra_open(struct input_dev *input)
> +{
> +	struct vibra_info *info = input_get_drvdata(input);
> +
> +	info->workqueue = create_singlethread_workqueue("vibra");
> +	if (info->workqueue == NULL) {
> +		dev_err(&input->dev, "couldn't create workqueue\n");
> +		return -ENOMEM;
> +	}

Why do we need to create a separate workqueue? With arrival of CWQ
it looks like we should be able to use one of the system-wide
workqueues for this.

> +
> +	return 0;
> +}
> +
> +static void twl6040_vibra_close(struct input_dev *input)
> +{
> +	struct vibra_info *info = input_get_drvdata(input);
> +
> +	cancel_work_sync(&info->play_work);
> +	INIT_WORK(&info->play_work, vibra_play_work);
> +	destroy_workqueue(info->workqueue);
> +	info->workqueue = NULL;
> +
> +	mutex_lock(&info->mutex);
> +
> +	if (info->enabled)
> +		twl6040_vibra_disable(info);
> +
> +	mutex_unlock(&info->mutex);
> +}
> +
> +#if CONFIG_PM

CONFIG_PM_SLEEP.

> +static int twl6040_vibra_suspend(struct device *dev)
> +{
> +	struct platform_device *pdev = to_platform_device(dev);
> +	struct vibra_info *info = platform_get_drvdata(pdev);
> +
> +	mutex_lock(&info->mutex);
> +
> +	if (info->enabled)
> +		twl6040_vibra_disable(info);
> +
> +	mutex_unlock(&info->mutex);
> +
> +	return 0;
> +}
> +
> +static SIMPLE_DEV_PM_OPS(twl6040_vibra_pm_ops,
> +			 twl6040_vibra_suspend, NULL);

Move twl6040_vibra_pm_ops definition out of the #ifdef block so you
won't need to protect it use with ifdefs later.

> +#endif
> +
> +static int __devinit twl6040_vibra_probe(struct platform_device *pdev)
> +{
> +	struct twl4030_vibra_data *pdata = pdev->dev.platform_data;
> +	struct vibra_info *info;
> +	int ret;
> +
> +	if (!pdata) {
> +		dev_err(&pdev->dev, "platform_data not available\n");
> +		return -EINVAL;
> +	}
> +
> +	info = kzalloc(sizeof(*info), GFP_KERNEL);
> +	if (!info) {
> +		dev_err(&pdev->dev, "couldn't allocate memory\n");
> +		return -ENOMEM;
> +	}
> +
> +	info->dev = &pdev->dev;
> +	info->twl6040 = dev_get_drvdata(pdev->dev.parent);
> +	info->vibldrv_res = pdata->vibldrv_res;
> +	info->vibrdrv_res = pdata->vibrdrv_res;
> +	info->viblmotor_res = pdata->viblmotor_res;
> +	info->vibrmotor_res = pdata->vibrmotor_res;
> +	if ((!info->vibldrv_res && !info->viblmotor_res) ||
> +	    (!info->vibrdrv_res && !info->vibrmotor_res)) {
> +		dev_err(info->dev, "invalid vibra driver/motor resistance\n");
> +		ret = -EINVAL;
> +		goto err_kzalloc;
> +	}
> +
> +	mutex_init(&info->mutex);
> +	INIT_WORK(&info->play_work, vibra_play_work);
> +
> +	info->input_dev = input_allocate_device();
> +	if (info->input_dev == NULL) {
> +		dev_err(info->dev, "couldn't allocate input device\n");
> +		ret = -ENOMEM;
> +		goto err_kzalloc;
> +	}
> +
> +	input_set_drvdata(info->input_dev, info);
> +
> +	info->input_dev->name = "twl6040:vibrator";
> +	info->input_dev->id.version = 1;
> +	info->input_dev->dev.parent = pdev->dev.parent;
> +	info->input_dev->open = twl6040_vibra_open;
> +	info->input_dev->close = twl6040_vibra_close;
> +	__set_bit(FF_RUMBLE, info->input_dev->ffbit);
> +
> +	ret = input_ff_create_memless(info->input_dev, NULL, vibra_play);
> +	if (ret < 0) {
> +		dev_err(info->dev, "couldn't register vibrator to FF\n");
> +		goto err_ialloc;
> +	}
> +
> +	ret = input_register_device(info->input_dev);
> +	if (ret < 0) {
> +		dev_err(info->dev, "couldn't register input device\n");
> +		goto err_iff;
> +	}
> +
> +	platform_set_drvdata(pdev, info);
> +
> +	ret = twl6040_request_irq(info->twl6040, TWL6040_IRQ_VIB,
> +				  twl6040_vib_irq_handler, 0,
> +				  "twl6040_irq_vib", info);
> +	if (ret) {
> +		dev_err(info->dev, "VIB IRQ request failed: %d\n", ret);
> +		goto err_irq;
> +	}
> +
> +	info->supplies[0].supply = "vddvibl";
> +	info->supplies[1].supply = "vddvibr";
> +	ret = regulator_bulk_get(info->dev, ARRAY_SIZE(info->supplies),
> +				 info->supplies);
> +	if (ret) {
> +		dev_err(info->dev, "couldn't get regulators %d\n", ret);
> +		goto err_regulator;
> +	}
> +
> +	if (pdata->vddvibl_uV) {
> +		ret = regulator_set_voltage(info->supplies[0].consumer,
> +					    pdata->vddvibl_uV,
> +					    pdata->vddvibl_uV);
> +		if (ret) {
> +			dev_err(info->dev, "failed to set VDDVIBL volt %d\n",
> +				ret);
> +			goto err_voltage;
> +		}
> +	}
> +
> +	if (pdata->vddvibr_uV) {
> +		ret = regulator_set_voltage(info->supplies[1].consumer,
> +					    pdata->vddvibr_uV,
> +					    pdata->vddvibr_uV);
> +		if (ret) {
> +			dev_err(info->dev, "failed to set VDDVIBR volt %d\n",
> +				ret);
> +			goto err_voltage;
> +		}
> +	}
> +
> +	return 0;
> +
> +err_voltage:
> +	regulator_bulk_free(ARRAY_SIZE(info->supplies), info->supplies);
> +err_regulator:
> +	twl6040_free_irq(info->twl6040, TWL6040_IRQ_VIB, info);
> +err_irq:
> +	input_unregister_device(info->input_dev);
> +	info->input_dev = NULL;
> +err_iff:
> +	if (info->input_dev)
> +		input_ff_destroy(info->input_dev);
> +err_ialloc:
> +	input_free_device(info->input_dev);
> +err_kzalloc:
> +	kfree(info);
> +	return ret;
> +}
> +
> +static int __devexit twl6040_vibra_remove(struct platform_device *pdev)
> +{
> +	struct vibra_info *info = platform_get_drvdata(pdev);
> +
> +	twl6040_power(info->twl6040, 0);

If we ensure that device is powered off until open() is called, and
also powered off when close() is called, then we do not need to switch
off power here.

> +	regulator_bulk_free(ARRAY_SIZE(info->supplies), info->supplies);
> +	twl6040_free_irq(info->twl6040, TWL6040_IRQ_VIB, info);
> +	input_unregister_device(info->input_dev);
> +	kfree(info);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver twl6040_vibra_driver = {
> +	.probe		= twl6040_vibra_probe,
> +	.remove		= __devexit_p(twl6040_vibra_remove),
> +	.driver		= {
> +		.name	= "twl6040-vibra",
> +		.owner	= THIS_MODULE,
> +#if CONFIG_PM
> +		.pm	= &twl6040_vibra_pm_ops,
> +#endif
> +	},
> +};
> +
> +static int __init twl6040_vibra_init(void)
> +{
> +	return platform_driver_register(&twl6040_vibra_driver);
> +}
> +module_init(twl6040_vibra_init);
> +
> +static void __exit twl6040_vibra_exit(void)
> +{
> +	platform_driver_unregister(&twl6040_vibra_driver);
> +}
> +module_exit(twl6040_vibra_exit);
> +
> +MODULE_ALIAS("platform:twl6040-vibra");
> +MODULE_DESCRIPTION("TWL6040 Vibra driver");
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Jorge Eduardo Candelaria <jorge.candelaria@...com>");
> +MODULE_AUTHOR("Misael Lopez Cruz <misael.lopez@...com>");
> diff --git a/include/linux/i2c/twl.h b/include/linux/i2c/twl.h
> index ea5baa2..685fd76 100644
> --- a/include/linux/i2c/twl.h
> +++ b/include/linux/i2c/twl.h
> @@ -669,6 +669,14 @@ struct twl4030_codec_data {
>  
>  struct twl4030_vibra_data {
>  	unsigned int	coexist;
> +
> +	/* twl6040 */
> +	unsigned int vibldrv_res;	/* left driver resistance */
> +	unsigned int vibrdrv_res;	/* right driver resistance */
> +	unsigned int viblmotor_res;	/* left motor resistance */
> +	unsigned int vibrmotor_res;	/* right motor resistance */
> +	int vddvibl_uV;			/* VDDVIBL volt, set 0 for fixed reg */
> +	int vddvibr_uV;			/* VDDVIBR volt, set 0 for fixed reg */
>  };
>  
>  struct twl4030_audio_data {
> -- 
> 1.7.5.3
> 

Thanks.

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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ