* Re: [PATCH V11 0/3] iio: input: ti_am335x_adc: Add continuous sampling support
From: Jonathan Cameron @ 2013-09-21 10:52 UTC (permalink / raw)
To: Zubair Lutfullah
Cc: jic23, dmitry.torokhov, linux-iio, linux-input, linux-kernel,
bigeasy, gregkh
In-Reply-To: <1379571876-12420-1-git-send-email-zubair.lutfullah@gmail.com>
Hi Zubair,
Thanks for persevering with this patch set. Now this
gets to be the example for dealing with hardware buffer
equipped devices ;)
Applied to the togreg branch of
git://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git
On 09/19/13 07:24, Zubair Lutfullah wrote:
> Hi,
>
> These apply to the togreg branch in iio.
>
> Round 11
> - Optimized memory usage based on list feedback
> - Changed devm_*_irq to normal *_irq functions
>
> Round 10 updates
> - Removed devm_free as not needed
>
> Round 9 updates
>
> - Removed Trigger.
> - Restructured everything for INDIO_BUFFERED_HARDWARE mode.
> - Threaded IRQ with top/bottom halfs. Top half HW IRQ wakes
> bottom half threaded worker that pushes samples to iio/userspace.
> - Removed some of the patchwork to the driver that was irrelevant to the
> continuous sampling patch.
>
> Cleanup will be done separately once this goes through.
> Hope these patches meet the mighty kernel standards :)
>
> Zubair
>
> Zubair Lutfullah (3):
> input: ti_am335x_tsc: Enable shared IRQ for TSC
> iio: ti_am335x_adc: optimize memory usage.
> iio: ti_am335x_adc: Add continuous sampling support
>
> drivers/iio/adc/ti_am335x_adc.c | 217 ++++++++++++++++++++++++++++-
> drivers/input/touchscreen/ti_am335x_tsc.c | 12 +-
> include/linux/mfd/ti_am335x_tscadc.h | 9 ++
> 3 files changed, 228 insertions(+), 10 deletions(-)
>
^ permalink raw reply
* Input: 8042 - only one i8042_filter
From: Andrey Moiseev @ 2013-09-21 6:48 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-kernel, linux-input
Is it ok that in /drivers/input/serio/i8042.c it's allowed to install
only one i8042_filter? Theoretically, there may be several different
devices that need their drivers to install i8042_filter, to eat out
some device-specific subset of scancodes. Shouldn't that single
pointer to the i8042_filter be replaced by a list, for example?
^ permalink raw reply
* Re: [PATCH] Input: atmel_tscadcc - update to devm_* API
From: Dmitry Torokhov @ 2013-09-20 17:01 UTC (permalink / raw)
To: Manish Badarkhe; +Cc: linux-input, linux-kernel, josh.wu
In-Reply-To: <1379343178-6468-1-git-send-email-badarkhe.manish@gmail.com>
Hi Manish,
On Mon, Sep 16, 2013 at 08:22:58PM +0530, Manish Badarkhe wrote:
> static int atmel_tsadcc_remove(struct platform_device *pdev)
> {
> struct atmel_tsadcc *ts_dev = platform_get_drvdata(pdev);
> - struct resource *res;
> -
> - free_irq(ts_dev->irq, ts_dev);
>
> input_unregister_device(ts_dev->input);
>
> - res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - iounmap(tsc_base);
> - release_mem_region(res->start, resource_size(res));
> -
> clk_disable(ts_dev->clk);
> - clk_put(ts_dev->clk);
> -
> - kfree(ts_dev);
>
> return 0;
> }
By doing this conversion we disable the clock too early. I need to
resurrect my patches to add devm_clk_enable and then we can revisit this
patch.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH 1/4] Input: ad714x-spi - Remove redundant spi_set_drvdata
From: Dmitry Torokhov @ 2013-09-20 16:39 UTC (permalink / raw)
To: Sachin Kamat; +Cc: linux-input, michael.hennerich, Barry Song
In-Reply-To: <1379652173-32477-1-git-send-email-sachin.kamat@linaro.org>
On Fri, Sep 20, 2013 at 10:12:50AM +0530, Sachin Kamat wrote:
> Driver core sets driver data to NULL upon failure or remove.
>
> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
> Cc: Barry Song <21cnbao@gmail.com>
> ---
> Series compile tested.
Applied all 4, thank you.
> ---
> drivers/input/misc/ad714x-spi.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/input/misc/ad714x-spi.c b/drivers/input/misc/ad714x-spi.c
> index 6189148..3a90b71 100644
> --- a/drivers/input/misc/ad714x-spi.c
> +++ b/drivers/input/misc/ad714x-spi.c
> @@ -108,7 +108,6 @@ static int ad714x_spi_remove(struct spi_device *spi)
> struct ad714x_chip *chip = spi_get_drvdata(spi);
>
> ad714x_remove(chip);
> - spi_set_drvdata(spi, NULL);
>
> return 0;
> }
> --
> 1.7.9.5
>
--
Dmitry
^ permalink raw reply
* [PATCH 4/4] Input: tsc2005 - Remove redundant spi_set_drvdata
From: Sachin Kamat @ 2013-09-20 4:44 UTC (permalink / raw)
To: linux-input
Cc: dmitry.torokhov, michael.hennerich, sachin.kamat, Lauri Leukkunen
In-Reply-To: <1379652289-300-1-git-send-email-sachin.kamat@linaro.org>
Driver core sets driver data to NULL upon failure or remove.
Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
Cc: Lauri Leukkunen <lauri.leukkunen@nokia.com>
---
drivers/input/touchscreen/tsc2005.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/input/touchscreen/tsc2005.c b/drivers/input/touchscreen/tsc2005.c
index 7213e8b..8113533 100644
--- a/drivers/input/touchscreen/tsc2005.c
+++ b/drivers/input/touchscreen/tsc2005.c
@@ -678,7 +678,6 @@ static int tsc2005_probe(struct spi_device *spi)
err_remove_sysfs:
sysfs_remove_group(&spi->dev.kobj, &tsc2005_attr_group);
err_clear_drvdata:
- spi_set_drvdata(spi, NULL);
free_irq(spi->irq, ts);
err_free_mem:
input_free_device(input_dev);
@@ -696,7 +695,6 @@ static int tsc2005_remove(struct spi_device *spi)
input_unregister_device(ts->idev);
kfree(ts);
- spi_set_drvdata(spi, NULL);
return 0;
}
--
1.7.9.5
^ permalink raw reply related
* [PATCH 3/4] Input: ad7879-spi - Remove redundant spi_set_drvdata
From: Sachin Kamat @ 2013-09-20 4:44 UTC (permalink / raw)
To: linux-input; +Cc: dmitry.torokhov, michael.hennerich, sachin.kamat
Driver core sets driver data to NULL upon failure or remove.
Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
---
drivers/input/touchscreen/ad7879-spi.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/input/touchscreen/ad7879-spi.c b/drivers/input/touchscreen/ad7879-spi.c
index 606da5b..1a7b114 100644
--- a/drivers/input/touchscreen/ad7879-spi.c
+++ b/drivers/input/touchscreen/ad7879-spi.c
@@ -142,7 +142,6 @@ static int ad7879_spi_remove(struct spi_device *spi)
struct ad7879 *ts = spi_get_drvdata(spi);
ad7879_remove(ts);
- spi_set_drvdata(spi, NULL);
return 0;
}
--
1.7.9.5
^ permalink raw reply related
* [PATCH 2/4] Input: ad7877 - Remove redundant spi_set_drvdata
From: Sachin Kamat @ 2013-09-20 4:42 UTC (permalink / raw)
To: linux-input; +Cc: dmitry.torokhov, michael.hennerich, sachin.kamat
In-Reply-To: <1379652173-32477-1-git-send-email-sachin.kamat@linaro.org>
Driver core sets driver data to NULL upon failure or remove.
Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
---
drivers/input/touchscreen/ad7877.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/input/touchscreen/ad7877.c b/drivers/input/touchscreen/ad7877.c
index f3a174a..69834dd 100644
--- a/drivers/input/touchscreen/ad7877.c
+++ b/drivers/input/touchscreen/ad7877.c
@@ -806,7 +806,6 @@ err_free_irq:
err_free_mem:
input_free_device(input_dev);
kfree(ts);
- spi_set_drvdata(spi, NULL);
return err;
}
@@ -823,7 +822,6 @@ static int ad7877_remove(struct spi_device *spi)
kfree(ts);
dev_dbg(&spi->dev, "unregistered touchscreen\n");
- spi_set_drvdata(spi, NULL);
return 0;
}
--
1.7.9.5
^ permalink raw reply related
* [PATCH 1/4] Input: ad714x-spi - Remove redundant spi_set_drvdata
From: Sachin Kamat @ 2013-09-20 4:42 UTC (permalink / raw)
To: linux-input; +Cc: dmitry.torokhov, michael.hennerich, sachin.kamat, Barry Song
Driver core sets driver data to NULL upon failure or remove.
Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
Cc: Barry Song <21cnbao@gmail.com>
---
Series compile tested.
---
drivers/input/misc/ad714x-spi.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/input/misc/ad714x-spi.c b/drivers/input/misc/ad714x-spi.c
index 6189148..3a90b71 100644
--- a/drivers/input/misc/ad714x-spi.c
+++ b/drivers/input/misc/ad714x-spi.c
@@ -108,7 +108,6 @@ static int ad714x_spi_remove(struct spi_device *spi)
struct ad714x_chip *chip = spi_get_drvdata(spi);
ad714x_remove(chip);
- spi_set_drvdata(spi, NULL);
return 0;
}
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH] input: touchscreen: add driver for Pixcir Tango C series capacitive touchscreen
From: Dmitry Torokhov @ 2013-09-20 0:09 UTC (permalink / raw)
To: Barry Song
Cc: linux-input@vger.kernel.org, DL-SHA-WorkGroupLinux, Lisai Wang,
Qipan Li, Barry Song
In-Reply-To: <CAGsJ_4zd0YDDHibBHuARrW7WkzojTpY_rzgjSAO1Y4cGnaeGHA@mail.gmail.com>
Barry Song <21cnbao@gmail.com> wrote:
>>>> +
>>>> + fingers = ts->touch_data.touch_fingers & 0x07;
>>>> + if (fingers > 0) {
>>>> + for (i = 0; i < fingers; i++) {
>>>> + /*Get the touch position*/
>>>> + input_report_abs(ts->input_dev,
>ABS_MT_POSITION_X,
>>>> +
>ts->touch_data.point[i].posx);
>>>> + input_report_abs(ts->input_dev,
>ABS_MT_POSITION_Y,
>>>> +
>ts->touch_data.point[i].posy);
>>>> +
>>>> + input_report_key(ts->input_dev,
>ABS_MT_TRACKING_ID,
>>>> + ts->touch_data.point[i].id);
>>>> + input_report_abs(ts->input_dev,
>>>> + ABS_MT_TOUCH_MAJOR,
>TOUCH_MAJOR_MAX);
>>>> + input_report_abs(ts->input_dev,
>>>> + ABS_MT_WIDTH_MAJOR,
>WIDTH_MAJOR_MAX);
>>>> + input_mt_sync(ts->input_dev);
>>>
>>> Any chance we could use the B flavor of MT protocol?
>>
>> i am sorry i missed some, what do you mean for B flavor ?
>
>do you mean API wrappers like input_mt_init_slots(),
>input_mt_report_slot_state() ...?
>
Yes, exactly.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] input: touchscreen: add driver for Pixcir Tango C series capacitive touchscreen
From: Barry Song @ 2013-09-19 23:59 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: linux-input@vger.kernel.org, DL-SHA-WorkGroupLinux, Lisai Wang,
Qipan Li, Barry Song
In-Reply-To: <CAGsJ_4wLc3aaSZyoEKNiyWhCQHKnv3MLHq6riS_=YZeqNGSeJg@mail.gmail.com>
>>> +
>>> + fingers = ts->touch_data.touch_fingers & 0x07;
>>> + if (fingers > 0) {
>>> + for (i = 0; i < fingers; i++) {
>>> + /*Get the touch position*/
>>> + input_report_abs(ts->input_dev, ABS_MT_POSITION_X,
>>> + ts->touch_data.point[i].posx);
>>> + input_report_abs(ts->input_dev, ABS_MT_POSITION_Y,
>>> + ts->touch_data.point[i].posy);
>>> +
>>> + input_report_key(ts->input_dev, ABS_MT_TRACKING_ID,
>>> + ts->touch_data.point[i].id);
>>> + input_report_abs(ts->input_dev,
>>> + ABS_MT_TOUCH_MAJOR, TOUCH_MAJOR_MAX);
>>> + input_report_abs(ts->input_dev,
>>> + ABS_MT_WIDTH_MAJOR, WIDTH_MAJOR_MAX);
>>> + input_mt_sync(ts->input_dev);
>>
>> Any chance we could use the B flavor of MT protocol?
>
> i am sorry i missed some, what do you mean for B flavor ?
do you mean API wrappers like input_mt_init_slots(),
input_mt_report_slot_state() ...?
-barry
^ permalink raw reply
* Re: [PATCH] input: touchscreen: add driver for Pixcir Tango C series capacitive touchscreen
From: Barry Song @ 2013-09-19 23:52 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: linux-input@vger.kernel.org, DL-SHA-WorkGroupLinux, Lisai Wang,
Qipan Li, Barry Song
In-Reply-To: <20130919161239.GB5731@core.coreip.homeip.net>
Hi Dmitry,
thanks for quick reviewing.
2013/9/20 Dmitry Torokhov <dmitry.torokhov@gmail.com>:
> Hi Barry,
>
> On Thu, Sep 19, 2013 at 04:38:31PM +0800, Barry Song wrote:
>> From: Lisai Wang <Lisai.Wang@csr.com>
>>
>> Pixcir Tango C series touchscreen support true multi-touch for 5 fingers. this
>> patch adds a driver for it.
>>
>> Signed-off-by: Lisai Wang <Lisai.Wang@csr.com>
>> Signed-off-by: Qipan Li <Qipan.Li@csr.com>
i made a mistake here, this should be Guoying.Zhang <Guoying.Zhang@csr.com>
>> Signed-off-by: Barry Song <Baohua.Song@csr.com>
>> ---
>> drivers/input/touchscreen/Kconfig | 10 +
>> drivers/input/touchscreen/Makefile | 1 +
>> drivers/input/touchscreen/pixcir_tangoc_ts.c | 242 ++++++++++++++++++++++++++
>> 3 files changed, 253 insertions(+), 0 deletions(-)
>> create mode 100644 drivers/input/touchscreen/pixcir_tangoc_ts.c
>>
>> diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
>> index e09ec67..2b80df6 100644
>> --- a/drivers/input/touchscreen/Kconfig
>> +++ b/drivers/input/touchscreen/Kconfig
>> @@ -919,4 +919,14 @@ config TOUCHSCREEN_TPS6507X
>> To compile this driver as a module, choose M here: the
>> module will be called tps6507x_ts.
>>
>> +config TOUCHSCREEN_TANGOC
>> + tristate "PIXCIR TANGOC touchscreen"
>> + depends on I2C
>> + help
>> + Say Y here if you support PIXCIR TangoC touchscreen
>> + controller
>> +
>> + If unsure, Say N.
>> + To compile this driver as a module, choose M here.
>> +
>> endif
>> diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
>> index f5216c1..ef73c06 100644
>> --- a/drivers/input/touchscreen/Makefile
>> +++ b/drivers/input/touchscreen/Makefile
>> @@ -75,3 +75,4 @@ obj-$(CONFIG_TOUCHSCREEN_WM97XX_MAINSTONE) += mainstone-wm97xx.o
>> obj-$(CONFIG_TOUCHSCREEN_WM97XX_ZYLONITE) += zylonite-wm97xx.o
>> obj-$(CONFIG_TOUCHSCREEN_W90X900) += w90p910_ts.o
>> obj-$(CONFIG_TOUCHSCREEN_TPS6507X) += tps6507x-ts.o
>> +obj-$(CONFIG_TOUCHSCREEN_TANGOC) += pixcir_tangoc_ts.o
>> diff --git a/drivers/input/touchscreen/pixcir_tangoc_ts.c b/drivers/input/touchscreen/pixcir_tangoc_ts.c
>> new file mode 100644
>> index 0000000..8274917
>> --- /dev/null
>> +++ b/drivers/input/touchscreen/pixcir_tangoc_ts.c
>> @@ -0,0 +1,242 @@
>> +/*
>> +* Pixcir Tango C series 5 points touch controller Driver
>> +*
>> +* Copyright (c) 2013 Cambridge Silicon Radio Limited, a CSR plc group company.
>> +*
>> +* Licensed under GPLv2 or later.
>> +*/
>> +
>> +#include <linux/module.h>
>> +#include <linux/kernel.h>
>> +#include <linux/i2c.h>
>> +#include <linux/gpio.h>
>> +#include <linux/of_gpio.h>
>> +#include <linux/input.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/slab.h>
>> +
>> +#define TOUCHSCREEN_MINX 0
>> +#define TOUCHSCREEN_MAXX 1024
>> +#define TOUCHSCREEN_MINY 0
>> +#define TOUCHSCREEN_MAXY 600
>> +#define TOUCH_MAJOR_MAX 50
>> +#define WIDTH_MAJOR_MAX 15
>> +#define TRACKING_ID_MAX 5
>> +
>> +struct pixcir_ts_point_data {
>> + u16 posx; /* x coordinate */
>> + u16 posy; /* y coordinate */
>
> Since this represents on-wire data you need to specify endianess (__le16?) and
> then explicitly convert it to CPU endianess when you use the data.
agree, this was tested on le ARM. so the original codes ignored be ARCH.
>
>> + u8 id; /* finger ID */
>> +} __packed;
>> +
>> +struct pixcir_ts_touch_data {
>> + u16 touch_fingers;
>
> This should be __le16 or __be16 as well.
>
>> + struct pixcir_ts_point_data point[5];
>> +} __packed;
>> +
>> +struct pixcir_ts_data {
>> + struct i2c_client *client;
>> + struct input_dev *input_dev;
>> + unsigned int touch_pin;
>> + struct pixcir_ts_touch_data touch_data;
>> +};
>> +
>> +/*Report the touch position*/
>> +static void pixcir_ts_report_event(struct pixcir_ts_data *ts)
>> +{
>> + int ret, i;
>> + int fingers;
>> + u8 addr;
>> +
>> + addr = 0;
>> + ret = i2c_master_send(ts->client, &addr, sizeof(addr));
>> + if (ret != sizeof(addr)) {
>> + dev_err(&ts->client->dev,
>> + "pixcir_ts:Unable to reset pixcir_ts!\n");
>
> Reset?
typo.
>
>> + return;
>> + }
>> +
>> + ret = i2c_master_recv(ts->client, (char *)&ts->touch_data,
>> + sizeof(struct pixcir_ts_touch_data));
>> + if (ret != sizeof(struct pixcir_ts_touch_data)) {
>> + dev_err(&ts->client->dev,
>> + "pixcir_ts:Unable get the touch info\n");
>> + return;
>> + }
>
> Do you need 2 transactions? Can you use i2c_transfer() here?
ok.
>
>> +
>> + fingers = ts->touch_data.touch_fingers & 0x07;
>> + if (fingers > 0) {
>> + for (i = 0; i < fingers; i++) {
>> + /*Get the touch position*/
>> + input_report_abs(ts->input_dev, ABS_MT_POSITION_X,
>> + ts->touch_data.point[i].posx);
>> + input_report_abs(ts->input_dev, ABS_MT_POSITION_Y,
>> + ts->touch_data.point[i].posy);
>> +
>> + input_report_key(ts->input_dev, ABS_MT_TRACKING_ID,
>> + ts->touch_data.point[i].id);
>> + input_report_abs(ts->input_dev,
>> + ABS_MT_TOUCH_MAJOR, TOUCH_MAJOR_MAX);
>> + input_report_abs(ts->input_dev,
>> + ABS_MT_WIDTH_MAJOR, WIDTH_MAJOR_MAX);
>> + input_mt_sync(ts->input_dev);
>
> Any chance we could use the B flavor of MT protocol?
i am sorry i missed some, what do you mean for B flavor ?
>
>> + }
>> + } else
>> + input_mt_sync(ts->input_dev);
>> +
>> + input_sync(ts->input_dev);
>> +}
>> +
>> +static irqreturn_t pixcir_ts_irq_handler(int irq, void *dev_id)
>> +{
>> + struct pixcir_ts_data *ts = dev_id;
>> +
>> + pixcir_ts_report_event(ts);
>> +
>> + return IRQ_HANDLED;
>> +}
>> +
>> +#ifdef CONFIG_PM_SLEEP
>> +static int pixcir_ts_suspend(struct device *dev)
>> +{
>> + struct i2c_client *client = to_i2c_client(dev);
>> +
>> + if (device_may_wakeup(&client->dev))
>> + enable_irq_wake(client->irq);
>> +
>> + return 0;
>> +}
>> +
>> +static int pixcir_ts_resume(struct device *dev)
>> +{
>> + struct i2c_client *client = to_i2c_client(dev);
>> +
>> + if (device_may_wakeup(&client->dev))
>> + disable_irq_wake(client->irq);
>> +
>> + return 0;
>> +}
>> +
>> +static SIMPLE_DEV_PM_OPS(pixcir_dev_pm_ops,
>> + pixcir_ts_suspend, pixcir_ts_resume);
>> +#endif
>> +
>> +static int pixcir_ts_probe(struct i2c_client *client,
>> + const struct i2c_device_id *id)
>> +{
>> + struct pixcir_ts_data *ts;
>> + struct input_dev *input_dev;
>> + struct device_node *np = client->dev.of_node;
>> + u8 tmp = 0;
>> + int ret;
>> +
>> + if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
>> + return -ENODEV;
>> +
>> + ts = devm_kzalloc(&client->dev, sizeof(*ts), GFP_KERNEL);
>> + if (!ts)
>> + return -ENOMEM;
>> + ts->client = client;
>> + i2c_set_clientdata(client, ts);
>> +
>> + ts->touch_pin = of_get_named_gpio(np, "touch-gpio", 0);
>> + if (!gpio_is_valid(ts->touch_pin)) {
>> + dev_err(&client->dev, "invalid touch_pin supplied\n");
>> + return -EINVAL;
>> + }
>> + client->irq = gpio_to_irq(ts->touch_pin);
>
> This should be done by the code that sets up i2c client, this driver
> should simply use client->irq, not set it up.
the driver is based on DT. so here i guess the node should put right
interrupt_parent to a gpio controller and fill right interrupts prop
to fix it.
non-DT users can still fill i2c board_info in mach.
>
>> +
>> + input_dev = input_allocate_device();
>> + if (!input_dev) {
>> + ret = -ENOMEM;
>> + goto err_free_mem;
>> + }
>> +
>> + /* if the client exists, this i2c transfer should be ok */
>> + ret = i2c_master_send(ts->client, &tmp, 1);
>> + if (ret != 1)
>> + goto err_free_mem;
>> +
>> + i2c_set_clientdata(client, ts);
>> + input_set_drvdata(input_dev, ts);
>> + input_dev->name = "tangoc-touchscreen";
>> + input_dev->id.bustype = BUS_I2C;
>> + input_dev->dev.parent = &client->dev;
>> + set_bit(EV_SYN, input_dev->evbit);
>> + set_bit(EV_ABS, input_dev->evbit);
>> + set_bit(ABS_MT_TOUCH_MAJOR, input_dev->absbit);
>> + set_bit(ABS_MT_WIDTH_MAJOR, input_dev->absbit);
>> + set_bit(ABS_MT_POSITION_X, input_dev->absbit);
>> + set_bit(ABS_MT_POSITION_Y, input_dev->absbit);
>> + set_bit(ABS_MT_TRACKING_ID, input_dev->absbit);
>> + set_bit(INPUT_PROP_DIRECT, input_dev->propbit);
>> + input_set_abs_params(input_dev, ABS_MT_POSITION_X,
>> + TOUCHSCREEN_MINX, TOUCHSCREEN_MAXX, 0, 0);
>> + input_set_abs_params(input_dev, ABS_MT_POSITION_Y,
>> + TOUCHSCREEN_MINY, TOUCHSCREEN_MAXY, 0, 0);
>> + input_set_abs_params(input_dev, ABS_MT_TOUCH_MAJOR,
>> + 0, TOUCH_MAJOR_MAX, 0, 0);
>> + input_set_abs_params(input_dev, ABS_MT_WIDTH_MAJOR,
>> + 0, WIDTH_MAJOR_MAX, 0, 0);
>> + input_set_abs_params(input_dev, ABS_MT_TRACKING_ID,
>> + 0, TRACKING_ID_MAX, 0, 0);
>> +
>> + ret = devm_request_threaded_irq(&client->dev,
>> + client->irq,
>> + NULL, pixcir_ts_irq_handler,
>> + IRQF_ONESHOT | IRQF_TRIGGER_FALLING,
>> + client->name, ts);
>> + if (ret) {
>> + dev_err(&client->dev, "\nFailed to register interrupt\n");
>> + goto err_free_mem;
>> + }
>> +
>> + ts->input_dev = input_dev;
>
> You need to make this assignment earlier, otherwise, if somebody is
> touching the screen you'll get a nice oops here.
ok.
>
>> + ret = input_register_device(ts->input_dev);
>> + if (ret) {
>> + dev_err(&client->dev, "Unable to register %s input device\n",
>> + input_dev->name);
>> + goto err_free_mem;
>> + }
>> + device_init_wakeup(&client->dev, 1);
>> +
>> + return 0;
>> +
>> +err_free_mem:
>> + input_free_device(input_dev);
>> + return ret;
>> +}
>> +
>> +static int pixcir_ts_remove(struct i2c_client *client)
>> +{
>> + struct pixcir_ts_data *ts = i2c_get_clientdata(client);
>> + device_init_wakeup(&client->dev, 0);
>> + input_unregister_device(ts->input_dev);
>
> This is so going to bomb because input device disappears but IRQ is not
> going to be unregistered till later. The easiest way would be to use
> devm_input_allocate_device() and get rid of input_unregister_device()
> altogether.
ok.
>
>> + return 0;
>> +}
>> +
>> +static const struct i2c_device_id pixcir_ts_id[] = {
>> + { "tangoc-ts", 0 },
>> + { }
>> +};
>> +
>> +MODULE_DEVICE_TABLE(i2c, pixcir_ts_id);
>> +
>> +static struct i2c_driver pixcir_ts_driver = {
>> + .driver = {
>> + .name = "tangoc-ts",
>> + .owner = THIS_MODULE,
>> +#ifdef CONFIG_PM_SLEEP
>> + .pm = &pixcir_dev_pm_ops,
>> +#endif
>> + },
>> + .id_table = pixcir_ts_id,
>> + .probe = pixcir_ts_probe,
>> + .remove = pixcir_ts_remove,
>> +};
>> +
>> +module_i2c_driver(pixcir_ts_driver);
>> +
>> +MODULE_AUTHOR("Lisai Wang <Lisai.Wang@csr.com>, Guoying Zhang <Guoying.Zhang@csr.com>");
>> +MODULE_DESCRIPTION("PIXCIR-TangoC 5 points touch controller Driver");
>> +MODULE_LICENSE("GPL v2");
>> --
>> 1.7.5.4
>>
>
> Thanks.
>
> --
> Dmitry
-barry
^ permalink raw reply
* Re: [PATCH 3.12-rc1] USB: input: cm109.c: Convert high volume dev_err() to dev_err_ratelimited()
From: Dmitry Torokhov @ 2013-09-19 21:52 UTC (permalink / raw)
To: Tim Gardner; +Cc: linux-input, linux-kernel
In-Reply-To: <1378830193-48384-1-git-send-email-tim.gardner@canonical.com>
Hi Tim,
On Tue, Sep 10, 2013 at 10:23:13AM -0600, Tim Gardner wrote:
> BugLink: http://bugs.launchpad.net/bugs/1222850
>
> This input device can get into a state that produces a high
> volume of device status errors. Attempt to throttle these
> error messages such that the kernel log is not flooded.
>
Only 2 of these printks need to be rate-limited, as other failures are
fatal to the driver since it will not resubmit the IO.
Also I think we need to try and resubmit control URB to try and execute
buzzer command if previous one failed.
BTW, EPROTO/EILSEQ errors mentioned in the launchpad bug seem to relate
to timeout/CRC errors reported by the host controller, so it must indeed
be the extender that is misbehaving.
Thanks.
--
Dmitry
Input: cm109 - convert high volume dev_err() to dev_err_ratelimited()
From: Tim Gardner <tim.gardner@canonical.com>
BugLink: http://bugs.launchpad.net/bugs/1222850
This input device can get into a state that produces a high
volume of device status errors. Attempt to throttle these
error messages such that the kernel log is not flooded.
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/misc/cm109.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/input/misc/cm109.c b/drivers/input/misc/cm109.c
index 082684e..9365535 100644
--- a/drivers/input/misc/cm109.c
+++ b/drivers/input/misc/cm109.c
@@ -351,7 +351,9 @@ static void cm109_urb_irq_callback(struct urb *urb)
if (status) {
if (status == -ESHUTDOWN)
return;
- dev_err(&dev->intf->dev, "%s: urb status %d\n", __func__, status);
+ dev_err_ratelimited(&dev->intf->dev, "%s: urb status %d\n",
+ __func__, status);
+ goto out;
}
/* Special keys */
@@ -418,8 +420,12 @@ static void cm109_urb_ctl_callback(struct urb *urb)
dev->ctl_data->byte[2],
dev->ctl_data->byte[3]);
- if (status)
- dev_err(&dev->intf->dev, "%s: urb status %d\n", __func__, status);
+ if (status) {
+ if (status == -ESHUTDOWN)
+ return;
+ dev_err_ratelimited(&dev->intf->dev, "%s: urb status %d\n",
+ __func__, status);
+ }
spin_lock(&dev->ctl_submit_lock);
@@ -427,7 +433,7 @@ static void cm109_urb_ctl_callback(struct urb *urb)
if (likely(!dev->shutdown)) {
- if (dev->buzzer_pending) {
+ if (dev->buzzer_pending || status) {
dev->buzzer_pending = 0;
dev->ctl_urb_pending = 1;
cm109_submit_buzz_toggle(dev);
^ permalink raw reply related
* Re: [PATCH] Input: gpio_keys - wakeup_trigger
From: Benson Leung @ 2013-09-19 20:43 UTC (permalink / raw)
To: Tomasz Figa
Cc: Dmitry Torokhov, linux-kernel@vger.kernel.org, linux-input,
linux-gpio, devicetree, david, Douglas Anderson
In-Reply-To: <106090496.3hTZTLSVzK@flatron>
Hi Tomasz,
On Sat, Sep 14, 2013 at 5:16 AM, Tomasz Figa <tomasz.figa@gmail.com> wrote:
> I don't like two things in this patch.
>
> First is that this looks completely like a configuration option, not
> hardware description, so it's not something that should be put into DT.
> Especially that users might want to use another wake-up trigger depending
> on their use cases. I'd rather see this as a sysfs entry.
>
> Another thing is that this driver assumes that key events are indicated by
> edges on the GPIO line, so I don't think level trigger setting make any
> sense here. I'd rather allow three settings here (through a sysfs knob) -
> key down, key up, key down or up.
Thanks for your feedback. I agree with your suggestions. I will move
this from platform data and DT to a sysfs property with three settings
: rising, falling, both.
--
Benson Leung
Software Engineer, Chrom* OS
bleung@chromium.org
^ permalink raw reply
* RE: [PATCH V2 1/1] Drivers: input: serio: New driver to support Hyper-V synthetic keyboard
From: KY Srinivasan @ 2013-09-19 16:31 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org,
devel@linuxdriverproject.org, olaf@aepfle.de, apw@canonical.com,
jasowang@redhat.com, dan.carpenter@oracle.com,
linux-input@vger.kernel.org, vojtech@suse.cz
In-Reply-To: <20130919155259.GA5731@core.coreip.homeip.net>
> -----Original Message-----
> From: Dmitry Torokhov [mailto:dmitry.torokhov@gmail.com]
> Sent: Thursday, September 19, 2013 8:53 AM
> To: KY Srinivasan
> Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com;
> jasowang@redhat.com; dan.carpenter@oracle.com; linux-
> input@vger.kernel.org; vojtech@suse.cz
> Subject: Re: [PATCH V2 1/1] Drivers: input: serio: New driver to support Hyper-V
> synthetic keyboard
>
> On Wed, Sep 18, 2013 at 11:27:51PM +0000, KY Srinivasan wrote:
> >
> >
> > > -----Original Message-----
> > > From: Dmitry Torokhov [mailto:dmitry.torokhov@gmail.com]
> > > Sent: Wednesday, September 18, 2013 2:01 PM
> > > To: KY Srinivasan
> > > Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;
> > > devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com;
> > > jasowang@redhat.com; dan.carpenter@oracle.com; linux-
> > > input@vger.kernel.org; vojtech@suse.cz
> > > Subject: Re: [PATCH V2 1/1] Drivers: input: serio: New driver to support
> Hyper-V
> > > synthetic keyboard
> > >
> > > Hi K.Y.,
> > >
> > > On Tue, Sep 17, 2013 at 04:26:58PM -0700, K. Y. Srinivasan wrote:
> > > > Add a new driver to support synthetic keyboard. On the next generation
> > > > Hyper-V guest firmware, many legacy devices will not be emulated and this
> > > > driver will be required.
> > > >
> > > > I would like to thank Vojtech Pavlik <vojtech@suse.cz> for helping me with
> the
> > > > details of the AT keyboard driver. I would also like to thank
> > > > Dan Carpenter <dan.carpenter@oracle.com> and
> > > > Dmitry Torokhov <dmitry.torokhov@gmail.com> for their detailed review of
> > > this
> > > > driver.
> > > >
> > > > I have addressed all the comments of Dan and Dmitry in this version of
> > > > the patch
> > >
> > > This looks much better. Could you tell me if the patch below (on top of
> > > yours) still works?
> > >
> > > Thanks.
> >
> > Thank you. The code looks much better now. You forgot to initialize the
> port_data and
> > after I fixed that everything seems to work as it did before: Here is the patch I
> used:
>
> Excellent, thank you for trying it out. I folded it all together and
> queued for 3.13.
Thank you.
Regards,
K. Y
^ permalink raw reply
* Re: [PATCH] input: touchscreen: add driver for Pixcir Tango C series capacitive touchscreen
From: Dmitry Torokhov @ 2013-09-19 16:12 UTC (permalink / raw)
To: Barry Song; +Cc: linux-input, workgroup.linux, Lisai Wang, Qipan Li, Barry Song
In-Reply-To: <1379579911-9763-1-git-send-email-Baohua.Song@csr.com>
Hi Barry,
On Thu, Sep 19, 2013 at 04:38:31PM +0800, Barry Song wrote:
> From: Lisai Wang <Lisai.Wang@csr.com>
>
> Pixcir Tango C series touchscreen support true multi-touch for 5 fingers. this
> patch adds a driver for it.
>
> Signed-off-by: Lisai Wang <Lisai.Wang@csr.com>
> Signed-off-by: Qipan Li <Qipan.Li@csr.com>
> Signed-off-by: Barry Song <Baohua.Song@csr.com>
> ---
> drivers/input/touchscreen/Kconfig | 10 +
> drivers/input/touchscreen/Makefile | 1 +
> drivers/input/touchscreen/pixcir_tangoc_ts.c | 242 ++++++++++++++++++++++++++
> 3 files changed, 253 insertions(+), 0 deletions(-)
> create mode 100644 drivers/input/touchscreen/pixcir_tangoc_ts.c
>
> diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
> index e09ec67..2b80df6 100644
> --- a/drivers/input/touchscreen/Kconfig
> +++ b/drivers/input/touchscreen/Kconfig
> @@ -919,4 +919,14 @@ config TOUCHSCREEN_TPS6507X
> To compile this driver as a module, choose M here: the
> module will be called tps6507x_ts.
>
> +config TOUCHSCREEN_TANGOC
> + tristate "PIXCIR TANGOC touchscreen"
> + depends on I2C
> + help
> + Say Y here if you support PIXCIR TangoC touchscreen
> + controller
> +
> + If unsure, Say N.
> + To compile this driver as a module, choose M here.
> +
> endif
> diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
> index f5216c1..ef73c06 100644
> --- a/drivers/input/touchscreen/Makefile
> +++ b/drivers/input/touchscreen/Makefile
> @@ -75,3 +75,4 @@ obj-$(CONFIG_TOUCHSCREEN_WM97XX_MAINSTONE) += mainstone-wm97xx.o
> obj-$(CONFIG_TOUCHSCREEN_WM97XX_ZYLONITE) += zylonite-wm97xx.o
> obj-$(CONFIG_TOUCHSCREEN_W90X900) += w90p910_ts.o
> obj-$(CONFIG_TOUCHSCREEN_TPS6507X) += tps6507x-ts.o
> +obj-$(CONFIG_TOUCHSCREEN_TANGOC) += pixcir_tangoc_ts.o
> diff --git a/drivers/input/touchscreen/pixcir_tangoc_ts.c b/drivers/input/touchscreen/pixcir_tangoc_ts.c
> new file mode 100644
> index 0000000..8274917
> --- /dev/null
> +++ b/drivers/input/touchscreen/pixcir_tangoc_ts.c
> @@ -0,0 +1,242 @@
> +/*
> +* Pixcir Tango C series 5 points touch controller Driver
> +*
> +* Copyright (c) 2013 Cambridge Silicon Radio Limited, a CSR plc group company.
> +*
> +* Licensed under GPLv2 or later.
> +*/
> +
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/i2c.h>
> +#include <linux/gpio.h>
> +#include <linux/of_gpio.h>
> +#include <linux/input.h>
> +#include <linux/interrupt.h>
> +#include <linux/slab.h>
> +
> +#define TOUCHSCREEN_MINX 0
> +#define TOUCHSCREEN_MAXX 1024
> +#define TOUCHSCREEN_MINY 0
> +#define TOUCHSCREEN_MAXY 600
> +#define TOUCH_MAJOR_MAX 50
> +#define WIDTH_MAJOR_MAX 15
> +#define TRACKING_ID_MAX 5
> +
> +struct pixcir_ts_point_data {
> + u16 posx; /* x coordinate */
> + u16 posy; /* y coordinate */
Since this represents on-wire data you need to specify endianess (__le16?) and
then explicitly convert it to CPU endianess when you use the data.
> + u8 id; /* finger ID */
> +} __packed;
> +
> +struct pixcir_ts_touch_data {
> + u16 touch_fingers;
This should be __le16 or __be16 as well.
> + struct pixcir_ts_point_data point[5];
> +} __packed;
> +
> +struct pixcir_ts_data {
> + struct i2c_client *client;
> + struct input_dev *input_dev;
> + unsigned int touch_pin;
> + struct pixcir_ts_touch_data touch_data;
> +};
> +
> +/*Report the touch position*/
> +static void pixcir_ts_report_event(struct pixcir_ts_data *ts)
> +{
> + int ret, i;
> + int fingers;
> + u8 addr;
> +
> + addr = 0;
> + ret = i2c_master_send(ts->client, &addr, sizeof(addr));
> + if (ret != sizeof(addr)) {
> + dev_err(&ts->client->dev,
> + "pixcir_ts:Unable to reset pixcir_ts!\n");
Reset?
> + return;
> + }
> +
> + ret = i2c_master_recv(ts->client, (char *)&ts->touch_data,
> + sizeof(struct pixcir_ts_touch_data));
> + if (ret != sizeof(struct pixcir_ts_touch_data)) {
> + dev_err(&ts->client->dev,
> + "pixcir_ts:Unable get the touch info\n");
> + return;
> + }
Do you need 2 transactions? Can you use i2c_transfer() here?
> +
> + fingers = ts->touch_data.touch_fingers & 0x07;
> + if (fingers > 0) {
> + for (i = 0; i < fingers; i++) {
> + /*Get the touch position*/
> + input_report_abs(ts->input_dev, ABS_MT_POSITION_X,
> + ts->touch_data.point[i].posx);
> + input_report_abs(ts->input_dev, ABS_MT_POSITION_Y,
> + ts->touch_data.point[i].posy);
> +
> + input_report_key(ts->input_dev, ABS_MT_TRACKING_ID,
> + ts->touch_data.point[i].id);
> + input_report_abs(ts->input_dev,
> + ABS_MT_TOUCH_MAJOR, TOUCH_MAJOR_MAX);
> + input_report_abs(ts->input_dev,
> + ABS_MT_WIDTH_MAJOR, WIDTH_MAJOR_MAX);
> + input_mt_sync(ts->input_dev);
Any chance we could use the B flavor of MT protocol?
> + }
> + } else
> + input_mt_sync(ts->input_dev);
> +
> + input_sync(ts->input_dev);
> +}
> +
> +static irqreturn_t pixcir_ts_irq_handler(int irq, void *dev_id)
> +{
> + struct pixcir_ts_data *ts = dev_id;
> +
> + pixcir_ts_report_event(ts);
> +
> + return IRQ_HANDLED;
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int pixcir_ts_suspend(struct device *dev)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> +
> + if (device_may_wakeup(&client->dev))
> + enable_irq_wake(client->irq);
> +
> + return 0;
> +}
> +
> +static int pixcir_ts_resume(struct device *dev)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> +
> + if (device_may_wakeup(&client->dev))
> + disable_irq_wake(client->irq);
> +
> + return 0;
> +}
> +
> +static SIMPLE_DEV_PM_OPS(pixcir_dev_pm_ops,
> + pixcir_ts_suspend, pixcir_ts_resume);
> +#endif
> +
> +static int pixcir_ts_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + struct pixcir_ts_data *ts;
> + struct input_dev *input_dev;
> + struct device_node *np = client->dev.of_node;
> + u8 tmp = 0;
> + int ret;
> +
> + if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
> + return -ENODEV;
> +
> + ts = devm_kzalloc(&client->dev, sizeof(*ts), GFP_KERNEL);
> + if (!ts)
> + return -ENOMEM;
> + ts->client = client;
> + i2c_set_clientdata(client, ts);
> +
> + ts->touch_pin = of_get_named_gpio(np, "touch-gpio", 0);
> + if (!gpio_is_valid(ts->touch_pin)) {
> + dev_err(&client->dev, "invalid touch_pin supplied\n");
> + return -EINVAL;
> + }
> + client->irq = gpio_to_irq(ts->touch_pin);
This should be done by the code that sets up i2c client, this driver
should simply use client->irq, not set it up.
> +
> + input_dev = input_allocate_device();
> + if (!input_dev) {
> + ret = -ENOMEM;
> + goto err_free_mem;
> + }
> +
> + /* if the client exists, this i2c transfer should be ok */
> + ret = i2c_master_send(ts->client, &tmp, 1);
> + if (ret != 1)
> + goto err_free_mem;
> +
> + i2c_set_clientdata(client, ts);
> + input_set_drvdata(input_dev, ts);
> + input_dev->name = "tangoc-touchscreen";
> + input_dev->id.bustype = BUS_I2C;
> + input_dev->dev.parent = &client->dev;
> + set_bit(EV_SYN, input_dev->evbit);
> + set_bit(EV_ABS, input_dev->evbit);
> + set_bit(ABS_MT_TOUCH_MAJOR, input_dev->absbit);
> + set_bit(ABS_MT_WIDTH_MAJOR, input_dev->absbit);
> + set_bit(ABS_MT_POSITION_X, input_dev->absbit);
> + set_bit(ABS_MT_POSITION_Y, input_dev->absbit);
> + set_bit(ABS_MT_TRACKING_ID, input_dev->absbit);
> + set_bit(INPUT_PROP_DIRECT, input_dev->propbit);
> + input_set_abs_params(input_dev, ABS_MT_POSITION_X,
> + TOUCHSCREEN_MINX, TOUCHSCREEN_MAXX, 0, 0);
> + input_set_abs_params(input_dev, ABS_MT_POSITION_Y,
> + TOUCHSCREEN_MINY, TOUCHSCREEN_MAXY, 0, 0);
> + input_set_abs_params(input_dev, ABS_MT_TOUCH_MAJOR,
> + 0, TOUCH_MAJOR_MAX, 0, 0);
> + input_set_abs_params(input_dev, ABS_MT_WIDTH_MAJOR,
> + 0, WIDTH_MAJOR_MAX, 0, 0);
> + input_set_abs_params(input_dev, ABS_MT_TRACKING_ID,
> + 0, TRACKING_ID_MAX, 0, 0);
> +
> + ret = devm_request_threaded_irq(&client->dev,
> + client->irq,
> + NULL, pixcir_ts_irq_handler,
> + IRQF_ONESHOT | IRQF_TRIGGER_FALLING,
> + client->name, ts);
> + if (ret) {
> + dev_err(&client->dev, "\nFailed to register interrupt\n");
> + goto err_free_mem;
> + }
> +
> + ts->input_dev = input_dev;
You need to make this assignment earlier, otherwise, if somebody is
touching the screen you'll get a nice oops here.
> + ret = input_register_device(ts->input_dev);
> + if (ret) {
> + dev_err(&client->dev, "Unable to register %s input device\n",
> + input_dev->name);
> + goto err_free_mem;
> + }
> + device_init_wakeup(&client->dev, 1);
> +
> + return 0;
> +
> +err_free_mem:
> + input_free_device(input_dev);
> + return ret;
> +}
> +
> +static int pixcir_ts_remove(struct i2c_client *client)
> +{
> + struct pixcir_ts_data *ts = i2c_get_clientdata(client);
> + device_init_wakeup(&client->dev, 0);
> + input_unregister_device(ts->input_dev);
This is so going to bomb because input device disappears but IRQ is not
going to be unregistered till later. The easiest way would be to use
devm_input_allocate_device() and get rid of input_unregister_device()
altogether.
> + return 0;
> +}
> +
> +static const struct i2c_device_id pixcir_ts_id[] = {
> + { "tangoc-ts", 0 },
> + { }
> +};
> +
> +MODULE_DEVICE_TABLE(i2c, pixcir_ts_id);
> +
> +static struct i2c_driver pixcir_ts_driver = {
> + .driver = {
> + .name = "tangoc-ts",
> + .owner = THIS_MODULE,
> +#ifdef CONFIG_PM_SLEEP
> + .pm = &pixcir_dev_pm_ops,
> +#endif
> + },
> + .id_table = pixcir_ts_id,
> + .probe = pixcir_ts_probe,
> + .remove = pixcir_ts_remove,
> +};
> +
> +module_i2c_driver(pixcir_ts_driver);
> +
> +MODULE_AUTHOR("Lisai Wang <Lisai.Wang@csr.com>, Guoying Zhang <Guoying.Zhang@csr.com>");
> +MODULE_DESCRIPTION("PIXCIR-TangoC 5 points touch controller Driver");
> +MODULE_LICENSE("GPL v2");
> --
> 1.7.5.4
>
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH 5/6] Staging/iio/adc/touchscreen/MXS: add interrupt driven touch detection
From: Jonathan Cameron @ 2013-09-19 16:12 UTC (permalink / raw)
To: Jürgen Beisert,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA,
devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b, marex-ynQEQJNshbs,
fabio.estevam-KZfg59tc24xl57MIdRCFDg,
jic23-KWPb1pKIrIJaa/9Udqfwiw, Torokhov,
linux-input-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <201309191449.37274.jbe-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
.
"Jürgen Beisert" <jbe-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> wrote:
>Hi Jonathan,
>
>On Monday 16 September 2013 17:30:32 Jonathan Cameron wrote:
>> >On Sunday 15 September 2013 12:56:25 Jonathan ,,, wrote:
>> >> On 09/11/13 09:18, Juergen Beisert wrote:
>> >> > For battery driven systems it is a very bad idea to collect the
>> >> > touchscreen data within a kernel busy loop.
>> >> >
>> >> > This change uses the features of the hardware to delay and
>> >> > accumulate samples in hardware to avoid a high interrupt and CPU
>load.
>> >> >
>> >> > Note: this is only tested on an i.MX23 SoC yet.
>> >> >
>> >> > Signed-off-by: Juergen Beisert <jbe-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
>> >> > CC: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
>> >> > CC: devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b@public.gmane.org
>> >> > CC: Marek Vasut <marex-ynQEQJNshbs@public.gmane.org>
>> >> > CC: Fabio Estevam <fabio.estevam-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
>> >> > CC: Jonathan Cameron <jic23-KWPb1pKIrIJaa/9Udqfwiw@public.gmane.org>
>> >>
>> >> While this driver is placed in IIO within staging at the moment,
>> >> these changes are definitely input related. Hence I have cc'd
>Dmitry and
>> >> the input list.
>> >>
>> >> I am personaly a little uncomfortable that we have such a complex
>bit
>> >> of input code sat within an IIO driver but such is life.
>> >
>> > Maybe an MFD for this ADC unit would be a better way to go?
>>
>> That would be great and is definitely the preferred method.
>
>Cannot continue to convert the driver into an MFD device. The project
>does not
>give me the time to do so.
>
>Regards,
>Juergen
Fair enough. Perhaps someone else will pick up the gauntlet :)
The old call of why there isn't enough time in the day!
--
Sent from my Android phone with K-9 Mail. Please excuse my brevity.
^ permalink raw reply
* Re: [PATCH V2 1/1] Drivers: input: serio: New driver to support Hyper-V synthetic keyboard
From: Dmitry Torokhov @ 2013-09-19 15:52 UTC (permalink / raw)
To: KY Srinivasan
Cc: gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org,
devel@linuxdriverproject.org, olaf@aepfle.de, apw@canonical.com,
jasowang@redhat.com, dan.carpenter@oracle.com,
linux-input@vger.kernel.org, vojtech@suse.cz
In-Reply-To: <304cbdeb0aa34f9a9ba6b44828fb5647@SN2PR03MB061.namprd03.prod.outlook.com>
On Wed, Sep 18, 2013 at 11:27:51PM +0000, KY Srinivasan wrote:
>
>
> > -----Original Message-----
> > From: Dmitry Torokhov [mailto:dmitry.torokhov@gmail.com]
> > Sent: Wednesday, September 18, 2013 2:01 PM
> > To: KY Srinivasan
> > Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;
> > devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com;
> > jasowang@redhat.com; dan.carpenter@oracle.com; linux-
> > input@vger.kernel.org; vojtech@suse.cz
> > Subject: Re: [PATCH V2 1/1] Drivers: input: serio: New driver to support Hyper-V
> > synthetic keyboard
> >
> > Hi K.Y.,
> >
> > On Tue, Sep 17, 2013 at 04:26:58PM -0700, K. Y. Srinivasan wrote:
> > > Add a new driver to support synthetic keyboard. On the next generation
> > > Hyper-V guest firmware, many legacy devices will not be emulated and this
> > > driver will be required.
> > >
> > > I would like to thank Vojtech Pavlik <vojtech@suse.cz> for helping me with the
> > > details of the AT keyboard driver. I would also like to thank
> > > Dan Carpenter <dan.carpenter@oracle.com> and
> > > Dmitry Torokhov <dmitry.torokhov@gmail.com> for their detailed review of
> > this
> > > driver.
> > >
> > > I have addressed all the comments of Dan and Dmitry in this version of
> > > the patch
> >
> > This looks much better. Could you tell me if the patch below (on top of
> > yours) still works?
> >
> > Thanks.
>
> Thank you. The code looks much better now. You forgot to initialize the port_data and
> after I fixed that everything seems to work as it did before: Here is the patch I used:
Excellent, thank you for trying it out. I folded it all together and
queued for 3.13.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH 5/6] Staging/iio/adc/touchscreen/MXS: add interrupt driven touch detection
From: Jürgen Beisert @ 2013-09-19 12:49 UTC (permalink / raw)
To: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Cc: Jonathan Cameron, linux-iio-u79uwXL29TY76Z2rM5mHXA,
devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b, marex-ynQEQJNshbs,
fabio.estevam-KZfg59tc24xl57MIdRCFDg,
jic23-KWPb1pKIrIJaa/9Udqfwiw, Torokhov,
linux-input-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <0fb59809-c835-49b0-bd82-e791db1a9db7-2ueSQiBKiTY7tOexoI0I+QC/G2K4zDHf@public.gmane.org>
Hi Jonathan,
On Monday 16 September 2013 17:30:32 Jonathan Cameron wrote:
> >On Sunday 15 September 2013 12:56:25 Jonathan Cameron wrote:
> >> On 09/11/13 09:18, Juergen Beisert wrote:
> >> > For battery driven systems it is a very bad idea to collect the
> >> > touchscreen data within a kernel busy loop.
> >> >
> >> > This change uses the features of the hardware to delay and
> >> > accumulate samples in hardware to avoid a high interrupt and CPU load.
> >> >
> >> > Note: this is only tested on an i.MX23 SoC yet.
> >> >
> >> > Signed-off-by: Juergen Beisert <jbe-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> >> > CC: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> >> > CC: devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b@public.gmane.org
> >> > CC: Marek Vasut <marex-ynQEQJNshbs@public.gmane.org>
> >> > CC: Fabio Estevam <fabio.estevam-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> >> > CC: Jonathan Cameron <jic23-KWPb1pKIrIJaa/9Udqfwiw@public.gmane.org>
> >>
> >> While this driver is placed in IIO within staging at the moment,
> >> these changes are definitely input related. Hence I have cc'd Dmitry and
> >> the input list.
> >>
> >> I am personaly a little uncomfortable that we have such a complex bit
> >> of input code sat within an IIO driver but such is life.
> >
> > Maybe an MFD for this ADC unit would be a better way to go?
>
> That would be great and is definitely the preferred method.
Cannot continue to convert the driver into an MFD device. The project does not
give me the time to do so.
Regards,
Juergen
--
Pengutronix e.K. | Juergen Beisert |
Linux Solutions for Science and Industry | http://www.pengutronix.de/ |
^ permalink raw reply
* [PATCH] input: touchscreen: add driver for Pixcir Tango C series capacitive touchscreen
From: Barry Song @ 2013-09-19 8:38 UTC (permalink / raw)
To: dmitry.torokhov, dtor
Cc: linux-input, workgroup.linux, Lisai Wang, Qipan Li, Barry Song
From: Lisai Wang <Lisai.Wang@csr.com>
Pixcir Tango C series touchscreen support true multi-touch for 5 fingers. this
patch adds a driver for it.
Signed-off-by: Lisai Wang <Lisai.Wang@csr.com>
Signed-off-by: Qipan Li <Qipan.Li@csr.com>
Signed-off-by: Barry Song <Baohua.Song@csr.com>
---
drivers/input/touchscreen/Kconfig | 10 +
drivers/input/touchscreen/Makefile | 1 +
drivers/input/touchscreen/pixcir_tangoc_ts.c | 242 ++++++++++++++++++++++++++
3 files changed, 253 insertions(+), 0 deletions(-)
create mode 100644 drivers/input/touchscreen/pixcir_tangoc_ts.c
diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
index e09ec67..2b80df6 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -919,4 +919,14 @@ config TOUCHSCREEN_TPS6507X
To compile this driver as a module, choose M here: the
module will be called tps6507x_ts.
+config TOUCHSCREEN_TANGOC
+ tristate "PIXCIR TANGOC touchscreen"
+ depends on I2C
+ help
+ Say Y here if you support PIXCIR TangoC touchscreen
+ controller
+
+ If unsure, Say N.
+ To compile this driver as a module, choose M here.
+
endif
diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
index f5216c1..ef73c06 100644
--- a/drivers/input/touchscreen/Makefile
+++ b/drivers/input/touchscreen/Makefile
@@ -75,3 +75,4 @@ obj-$(CONFIG_TOUCHSCREEN_WM97XX_MAINSTONE) += mainstone-wm97xx.o
obj-$(CONFIG_TOUCHSCREEN_WM97XX_ZYLONITE) += zylonite-wm97xx.o
obj-$(CONFIG_TOUCHSCREEN_W90X900) += w90p910_ts.o
obj-$(CONFIG_TOUCHSCREEN_TPS6507X) += tps6507x-ts.o
+obj-$(CONFIG_TOUCHSCREEN_TANGOC) += pixcir_tangoc_ts.o
diff --git a/drivers/input/touchscreen/pixcir_tangoc_ts.c b/drivers/input/touchscreen/pixcir_tangoc_ts.c
new file mode 100644
index 0000000..8274917
--- /dev/null
+++ b/drivers/input/touchscreen/pixcir_tangoc_ts.c
@@ -0,0 +1,242 @@
+/*
+* Pixcir Tango C series 5 points touch controller Driver
+*
+* Copyright (c) 2013 Cambridge Silicon Radio Limited, a CSR plc group company.
+*
+* Licensed under GPLv2 or later.
+*/
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/i2c.h>
+#include <linux/gpio.h>
+#include <linux/of_gpio.h>
+#include <linux/input.h>
+#include <linux/interrupt.h>
+#include <linux/slab.h>
+
+#define TOUCHSCREEN_MINX 0
+#define TOUCHSCREEN_MAXX 1024
+#define TOUCHSCREEN_MINY 0
+#define TOUCHSCREEN_MAXY 600
+#define TOUCH_MAJOR_MAX 50
+#define WIDTH_MAJOR_MAX 15
+#define TRACKING_ID_MAX 5
+
+struct pixcir_ts_point_data {
+ u16 posx; /* x coordinate */
+ u16 posy; /* y coordinate */
+ u8 id; /* finger ID */
+} __packed;
+
+struct pixcir_ts_touch_data {
+ u16 touch_fingers;
+ struct pixcir_ts_point_data point[5];
+} __packed;
+
+struct pixcir_ts_data {
+ struct i2c_client *client;
+ struct input_dev *input_dev;
+ unsigned int touch_pin;
+ struct pixcir_ts_touch_data touch_data;
+};
+
+/*Report the touch position*/
+static void pixcir_ts_report_event(struct pixcir_ts_data *ts)
+{
+ int ret, i;
+ int fingers;
+ u8 addr;
+
+ addr = 0;
+ ret = i2c_master_send(ts->client, &addr, sizeof(addr));
+ if (ret != sizeof(addr)) {
+ dev_err(&ts->client->dev,
+ "pixcir_ts:Unable to reset pixcir_ts!\n");
+ return;
+ }
+
+ ret = i2c_master_recv(ts->client, (char *)&ts->touch_data,
+ sizeof(struct pixcir_ts_touch_data));
+ if (ret != sizeof(struct pixcir_ts_touch_data)) {
+ dev_err(&ts->client->dev,
+ "pixcir_ts:Unable get the touch info\n");
+ return;
+ }
+
+ fingers = ts->touch_data.touch_fingers & 0x07;
+ if (fingers > 0) {
+ for (i = 0; i < fingers; i++) {
+ /*Get the touch position*/
+ input_report_abs(ts->input_dev, ABS_MT_POSITION_X,
+ ts->touch_data.point[i].posx);
+ input_report_abs(ts->input_dev, ABS_MT_POSITION_Y,
+ ts->touch_data.point[i].posy);
+
+ input_report_key(ts->input_dev, ABS_MT_TRACKING_ID,
+ ts->touch_data.point[i].id);
+ input_report_abs(ts->input_dev,
+ ABS_MT_TOUCH_MAJOR, TOUCH_MAJOR_MAX);
+ input_report_abs(ts->input_dev,
+ ABS_MT_WIDTH_MAJOR, WIDTH_MAJOR_MAX);
+ input_mt_sync(ts->input_dev);
+ }
+ } else
+ input_mt_sync(ts->input_dev);
+
+ input_sync(ts->input_dev);
+}
+
+static irqreturn_t pixcir_ts_irq_handler(int irq, void *dev_id)
+{
+ struct pixcir_ts_data *ts = dev_id;
+
+ pixcir_ts_report_event(ts);
+
+ return IRQ_HANDLED;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int pixcir_ts_suspend(struct device *dev)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+
+ if (device_may_wakeup(&client->dev))
+ enable_irq_wake(client->irq);
+
+ return 0;
+}
+
+static int pixcir_ts_resume(struct device *dev)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+
+ if (device_may_wakeup(&client->dev))
+ disable_irq_wake(client->irq);
+
+ return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(pixcir_dev_pm_ops,
+ pixcir_ts_suspend, pixcir_ts_resume);
+#endif
+
+static int pixcir_ts_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ struct pixcir_ts_data *ts;
+ struct input_dev *input_dev;
+ struct device_node *np = client->dev.of_node;
+ u8 tmp = 0;
+ int ret;
+
+ if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
+ return -ENODEV;
+
+ ts = devm_kzalloc(&client->dev, sizeof(*ts), GFP_KERNEL);
+ if (!ts)
+ return -ENOMEM;
+ ts->client = client;
+ i2c_set_clientdata(client, ts);
+
+ ts->touch_pin = of_get_named_gpio(np, "touch-gpio", 0);
+ if (!gpio_is_valid(ts->touch_pin)) {
+ dev_err(&client->dev, "invalid touch_pin supplied\n");
+ return -EINVAL;
+ }
+ client->irq = gpio_to_irq(ts->touch_pin);
+
+ input_dev = input_allocate_device();
+ if (!input_dev) {
+ ret = -ENOMEM;
+ goto err_free_mem;
+ }
+
+ /* if the client exists, this i2c transfer should be ok */
+ ret = i2c_master_send(ts->client, &tmp, 1);
+ if (ret != 1)
+ goto err_free_mem;
+
+ i2c_set_clientdata(client, ts);
+ input_set_drvdata(input_dev, ts);
+ input_dev->name = "tangoc-touchscreen";
+ input_dev->id.bustype = BUS_I2C;
+ input_dev->dev.parent = &client->dev;
+ set_bit(EV_SYN, input_dev->evbit);
+ set_bit(EV_ABS, input_dev->evbit);
+ set_bit(ABS_MT_TOUCH_MAJOR, input_dev->absbit);
+ set_bit(ABS_MT_WIDTH_MAJOR, input_dev->absbit);
+ set_bit(ABS_MT_POSITION_X, input_dev->absbit);
+ set_bit(ABS_MT_POSITION_Y, input_dev->absbit);
+ set_bit(ABS_MT_TRACKING_ID, input_dev->absbit);
+ set_bit(INPUT_PROP_DIRECT, input_dev->propbit);
+ input_set_abs_params(input_dev, ABS_MT_POSITION_X,
+ TOUCHSCREEN_MINX, TOUCHSCREEN_MAXX, 0, 0);
+ input_set_abs_params(input_dev, ABS_MT_POSITION_Y,
+ TOUCHSCREEN_MINY, TOUCHSCREEN_MAXY, 0, 0);
+ input_set_abs_params(input_dev, ABS_MT_TOUCH_MAJOR,
+ 0, TOUCH_MAJOR_MAX, 0, 0);
+ input_set_abs_params(input_dev, ABS_MT_WIDTH_MAJOR,
+ 0, WIDTH_MAJOR_MAX, 0, 0);
+ input_set_abs_params(input_dev, ABS_MT_TRACKING_ID,
+ 0, TRACKING_ID_MAX, 0, 0);
+
+ ret = devm_request_threaded_irq(&client->dev,
+ client->irq,
+ NULL, pixcir_ts_irq_handler,
+ IRQF_ONESHOT | IRQF_TRIGGER_FALLING,
+ client->name, ts);
+ if (ret) {
+ dev_err(&client->dev, "\nFailed to register interrupt\n");
+ goto err_free_mem;
+ }
+
+ ts->input_dev = input_dev;
+ ret = input_register_device(ts->input_dev);
+ if (ret) {
+ dev_err(&client->dev, "Unable to register %s input device\n",
+ input_dev->name);
+ goto err_free_mem;
+ }
+ device_init_wakeup(&client->dev, 1);
+
+ return 0;
+
+err_free_mem:
+ input_free_device(input_dev);
+ return ret;
+}
+
+static int pixcir_ts_remove(struct i2c_client *client)
+{
+ struct pixcir_ts_data *ts = i2c_get_clientdata(client);
+ device_init_wakeup(&client->dev, 0);
+ input_unregister_device(ts->input_dev);
+ return 0;
+}
+
+static const struct i2c_device_id pixcir_ts_id[] = {
+ { "tangoc-ts", 0 },
+ { }
+};
+
+MODULE_DEVICE_TABLE(i2c, pixcir_ts_id);
+
+static struct i2c_driver pixcir_ts_driver = {
+ .driver = {
+ .name = "tangoc-ts",
+ .owner = THIS_MODULE,
+#ifdef CONFIG_PM_SLEEP
+ .pm = &pixcir_dev_pm_ops,
+#endif
+ },
+ .id_table = pixcir_ts_id,
+ .probe = pixcir_ts_probe,
+ .remove = pixcir_ts_remove,
+};
+
+module_i2c_driver(pixcir_ts_driver);
+
+MODULE_AUTHOR("Lisai Wang <Lisai.Wang@csr.com>, Guoying Zhang <Guoying.Zhang@csr.com>");
+MODULE_DESCRIPTION("PIXCIR-TangoC 5 points touch controller Driver");
+MODULE_LICENSE("GPL v2");
--
1.7.5.4
^ permalink raw reply related
* [PATCH V11 0/3] iio: input: ti_am335x_adc: Add continuous sampling support
From: Zubair Lutfullah @ 2013-09-19 6:24 UTC (permalink / raw)
To: jic23, dmitry.torokhov
Cc: linux-iio, linux-input, linux-kernel, bigeasy, gregkh,
zubair.lutfullah
Hi,
These apply to the togreg branch in iio.
Round 11
- Optimized memory usage based on list feedback
- Changed devm_*_irq to normal *_irq functions
Round 10 updates
- Removed devm_free as not needed
Round 9 updates
- Removed Trigger.
- Restructured everything for INDIO_BUFFERED_HARDWARE mode.
- Threaded IRQ with top/bottom halfs. Top half HW IRQ wakes
bottom half threaded worker that pushes samples to iio/userspace.
- Removed some of the patchwork to the driver that was irrelevant to the
continuous sampling patch.
Cleanup will be done separately once this goes through.
Hope these patches meet the mighty kernel standards :)
Zubair
Zubair Lutfullah (3):
input: ti_am335x_tsc: Enable shared IRQ for TSC
iio: ti_am335x_adc: optimize memory usage.
iio: ti_am335x_adc: Add continuous sampling support
drivers/iio/adc/ti_am335x_adc.c | 217 ++++++++++++++++++++++++++++-
drivers/input/touchscreen/ti_am335x_tsc.c | 12 +-
include/linux/mfd/ti_am335x_tscadc.h | 9 ++
3 files changed, 228 insertions(+), 10 deletions(-)
--
1.7.9.5
^ permalink raw reply
* [PATCH 3/3] iio: ti_am335x_adc: Add continuous sampling support
From: Zubair Lutfullah @ 2013-09-19 6:24 UTC (permalink / raw)
To: jic23, dmitry.torokhov
Cc: linux-iio, linux-input, linux-kernel, bigeasy, gregkh,
zubair.lutfullah
In-Reply-To: <1379571876-12420-1-git-send-email-zubair.lutfullah@gmail.com>
Previously the driver had only one-shot reading functionality.
This patch adds continuous sampling support to the driver.
Continuous sampling starts when buffer is enabled.
HW IRQ wakes worker thread that pushes samples to userspace.
Sampling stops when buffer is disabled by userspace.
Patil Rachna (TI) laid the ground work for ADC HW register access.
Russ Dill (TI) fixed bugs in the driver relevant to FIFOs and IRQs.
I fixed channel scanning so multiple ADC channels can be read
simultaneously and pushed to userspace.
Restructured the driver to fit IIO ABI.
And added INDIO_BUFFER_HARDWARE mode.
Signed-off-by: Zubair Lutfullah <zubair.lutfullah@gmail.com>
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Russ Dill <Russ.Dill@ti.com>
Acked-by: Lee Jones <lee.jones@linaro.org>
Acked-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
drivers/iio/adc/ti_am335x_adc.c | 213 +++++++++++++++++++++++++++++++++-
include/linux/mfd/ti_am335x_tscadc.h | 9 ++
2 files changed, 217 insertions(+), 5 deletions(-)
diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c
index ebe93eb..5287bff 100644
--- a/drivers/iio/adc/ti_am335x_adc.c
+++ b/drivers/iio/adc/ti_am335x_adc.c
@@ -28,12 +28,20 @@
#include <linux/iio/driver.h>
#include <linux/mfd/ti_am335x_tscadc.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/kfifo_buf.h>
+#include <linux/iio/trigger.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/iio/triggered_buffer.h>
struct tiadc_device {
struct ti_tscadc_dev *mfd_tscadc;
int channels;
u8 channel_line[8];
u8 channel_step[8];
+ int buffer_en_ch_steps;
+ struct iio_trigger *trig;
+ u16 data[8];
};
static unsigned int tiadc_readl(struct tiadc_device *adc, unsigned int reg)
@@ -56,8 +64,14 @@ static u32 get_adc_step_mask(struct tiadc_device *adc_dev)
return step_en;
}
-static void tiadc_step_config(struct tiadc_device *adc_dev)
+static u32 get_adc_step_bit(struct tiadc_device *adc_dev, int chan)
{
+ return 1 << adc_dev->channel_step[chan];
+}
+
+static void tiadc_step_config(struct iio_dev *indio_dev)
+{
+ struct tiadc_device *adc_dev = iio_priv(indio_dev);
unsigned int stepconfig;
int i, steps;
@@ -72,7 +86,11 @@ static void tiadc_step_config(struct tiadc_device *adc_dev)
*/
steps = TOTAL_STEPS - adc_dev->channels;
- stepconfig = STEPCONFIG_AVG_16 | STEPCONFIG_FIFO1;
+ if (iio_buffer_enabled(indio_dev))
+ stepconfig = STEPCONFIG_AVG_16 | STEPCONFIG_FIFO1
+ | STEPCONFIG_MODE_SWCNT;
+ else
+ stepconfig = STEPCONFIG_AVG_16 | STEPCONFIG_FIFO1;
for (i = 0; i < adc_dev->channels; i++) {
int chan;
@@ -85,9 +103,175 @@ static void tiadc_step_config(struct tiadc_device *adc_dev)
adc_dev->channel_step[i] = steps;
steps++;
}
+}
+
+static irqreturn_t tiadc_irq_h(int irq, void *private)
+{
+ struct iio_dev *indio_dev = private;
+ struct tiadc_device *adc_dev = iio_priv(indio_dev);
+ unsigned int status, config;
+ status = tiadc_readl(adc_dev, REG_IRQSTATUS);
+
+ /*
+ * ADC and touchscreen share the IRQ line.
+ * FIFO0 interrupts are used by TSC. Handle FIFO1 IRQs here only
+ */
+ if (status & IRQENB_FIFO1OVRRUN) {
+ /* FIFO Overrun. Clear flag. Disable/Enable ADC to recover */
+ config = tiadc_readl(adc_dev, REG_CTRL);
+ config &= ~(CNTRLREG_TSCSSENB);
+ tiadc_writel(adc_dev, REG_CTRL, config);
+ tiadc_writel(adc_dev, REG_IRQSTATUS, IRQENB_FIFO1OVRRUN
+ | IRQENB_FIFO1UNDRFLW | IRQENB_FIFO1THRES);
+ tiadc_writel(adc_dev, REG_CTRL, (config | CNTRLREG_TSCSSENB));
+ return IRQ_HANDLED;
+ } else if (status & IRQENB_FIFO1THRES) {
+ /* Disable irq and wake worker thread */
+ tiadc_writel(adc_dev, REG_IRQCLR, IRQENB_FIFO1THRES);
+ return IRQ_WAKE_THREAD;
+ }
+
+ return IRQ_NONE;
+}
+
+static irqreturn_t tiadc_worker_h(int irq, void *private)
+{
+ struct iio_dev *indio_dev = private;
+ struct tiadc_device *adc_dev = iio_priv(indio_dev);
+ int i, k, fifo1count, read;
+ u16 *data = adc_dev->data;
+
+ fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
+ for (k = 0; k < fifo1count; k = k + i) {
+ for (i = 0; i < (indio_dev->scan_bytes)/2; i++) {
+ read = tiadc_readl(adc_dev, REG_FIFO1);
+ data[i] = read & FIFOREAD_DATA_MASK;
+ }
+ iio_push_to_buffers(indio_dev, (u8 *) data);
+ }
+
+ tiadc_writel(adc_dev, REG_IRQSTATUS, IRQENB_FIFO1THRES);
+ tiadc_writel(adc_dev, REG_IRQENABLE, IRQENB_FIFO1THRES);
+
+ return IRQ_HANDLED;
+}
+
+static int tiadc_buffer_preenable(struct iio_dev *indio_dev)
+{
+ struct tiadc_device *adc_dev = iio_priv(indio_dev);
+ int i, fifo1count, read;
+
+ tiadc_writel(adc_dev, REG_IRQCLR, (IRQENB_FIFO1THRES |
+ IRQENB_FIFO1OVRRUN |
+ IRQENB_FIFO1UNDRFLW));
+
+ /* Flush FIFO. Needed in corner cases in simultaneous tsc/adc use */
+ fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
+ for (i = 0; i < fifo1count; i++)
+ read = tiadc_readl(adc_dev, REG_FIFO1);
+
+ return iio_sw_buffer_preenable(indio_dev);
+}
+
+static int tiadc_buffer_postenable(struct iio_dev *indio_dev)
+{
+ struct tiadc_device *adc_dev = iio_priv(indio_dev);
+ struct iio_buffer *buffer = indio_dev->buffer;
+ unsigned int enb = 0;
+ u8 bit;
+
+ tiadc_step_config(indio_dev);
+ for_each_set_bit(bit, buffer->scan_mask, adc_dev->channels)
+ enb |= (get_adc_step_bit(adc_dev, bit) << 1);
+ adc_dev->buffer_en_ch_steps = enb;
+
+ am335x_tsc_se_set(adc_dev->mfd_tscadc, enb);
+
+ tiadc_writel(adc_dev, REG_IRQSTATUS, IRQENB_FIFO1THRES
+ | IRQENB_FIFO1OVRRUN | IRQENB_FIFO1UNDRFLW);
+ tiadc_writel(adc_dev, REG_IRQENABLE, IRQENB_FIFO1THRES
+ | IRQENB_FIFO1OVRRUN);
+
+ return 0;
+}
+
+static int tiadc_buffer_predisable(struct iio_dev *indio_dev)
+{
+ struct tiadc_device *adc_dev = iio_priv(indio_dev);
+ int fifo1count, i, read;
+
+ tiadc_writel(adc_dev, REG_IRQCLR, (IRQENB_FIFO1THRES |
+ IRQENB_FIFO1OVRRUN | IRQENB_FIFO1UNDRFLW));
+ am335x_tsc_se_clr(adc_dev->mfd_tscadc, adc_dev->buffer_en_ch_steps);
+ /* Flush FIFO of leftover data in the time it takes to disable adc */
+ fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
+ for (i = 0; i < fifo1count; i++)
+ read = tiadc_readl(adc_dev, REG_FIFO1);
+
+ return 0;
}
+static int tiadc_buffer_postdisable(struct iio_dev *indio_dev)
+{
+ tiadc_step_config(indio_dev);
+
+ return 0;
+}
+
+static const struct iio_buffer_setup_ops tiadc_buffer_setup_ops = {
+ .preenable = &tiadc_buffer_preenable,
+ .postenable = &tiadc_buffer_postenable,
+ .predisable = &tiadc_buffer_predisable,
+ .postdisable = &tiadc_buffer_postdisable,
+};
+
+int tiadc_iio_buffered_hardware_setup(struct iio_dev *indio_dev,
+ irqreturn_t (*pollfunc_bh)(int irq, void *p),
+ irqreturn_t (*pollfunc_th)(int irq, void *p),
+ int irq,
+ unsigned long flags,
+ const struct iio_buffer_setup_ops *setup_ops)
+{
+ int ret;
+
+ indio_dev->buffer = iio_kfifo_allocate(indio_dev);
+ if (!indio_dev->buffer)
+ return -ENOMEM;
+
+ ret = request_threaded_irq(irq, pollfunc_th, pollfunc_bh,
+ flags, indio_dev->name, indio_dev);
+ if (ret)
+ goto error_kfifo_free;
+
+ indio_dev->setup_ops = setup_ops;
+ indio_dev->modes |= INDIO_BUFFER_HARDWARE;
+
+ ret = iio_buffer_register(indio_dev,
+ indio_dev->channels,
+ indio_dev->num_channels);
+ if (ret)
+ goto error_free_irq;
+
+ return 0;
+
+error_free_irq:
+ free_irq(irq, indio_dev);
+error_kfifo_free:
+ iio_kfifo_free(indio_dev->buffer);
+ return ret;
+}
+
+static void tiadc_iio_buffered_hardware_remove(struct iio_dev *indio_dev)
+{
+ struct tiadc_device *adc_dev = iio_priv(indio_dev);
+
+ free_irq(adc_dev->mfd_tscadc->irq, indio_dev);
+ iio_kfifo_free(indio_dev->buffer);
+ iio_buffer_unregister(indio_dev);
+}
+
+
static const char * const chan_name_ain[] = {
"AIN0",
"AIN1",
@@ -120,6 +304,7 @@ static int tiadc_channel_init(struct iio_dev *indio_dev, int channels)
chan->channel = adc_dev->channel_line[i];
chan->info_mask_separate = BIT(IIO_CHAN_INFO_RAW);
chan->datasheet_name = chan_name_ain[chan->channel];
+ chan->scan_index = i;
chan->scan_type.sign = 'u';
chan->scan_type.realbits = 12;
chan->scan_type.storagebits = 16;
@@ -147,6 +332,10 @@ static int tiadc_read_raw(struct iio_dev *indio_dev,
u32 step_en;
unsigned long timeout = jiffies + usecs_to_jiffies
(IDLE_TIMEOUT * adc_dev->channels);
+
+ if (iio_buffer_enabled(indio_dev))
+ return -EBUSY;
+
step_en = get_adc_step_mask(adc_dev);
am335x_tsc_se_set(adc_dev->mfd_tscadc, step_en);
@@ -237,20 +426,33 @@ static int tiadc_probe(struct platform_device *pdev)
indio_dev->modes = INDIO_DIRECT_MODE;
indio_dev->info = &tiadc_info;
- tiadc_step_config(adc_dev);
+ tiadc_step_config(indio_dev);
+ tiadc_writel(adc_dev, REG_FIFO1THR, FIFO1_THRESHOLD);
err = tiadc_channel_init(indio_dev, adc_dev->channels);
if (err < 0)
return err;
- err = iio_device_register(indio_dev);
+ err = tiadc_iio_buffered_hardware_setup(indio_dev,
+ &tiadc_worker_h,
+ &tiadc_irq_h,
+ adc_dev->mfd_tscadc->irq,
+ IRQF_SHARED,
+ &tiadc_buffer_setup_ops);
+
if (err)
goto err_free_channels;
+ err = iio_device_register(indio_dev);
+ if (err)
+ goto err_buffer_unregister;
+
platform_set_drvdata(pdev, indio_dev);
return 0;
+err_buffer_unregister:
+ tiadc_iio_buffered_hardware_remove(indio_dev);
err_free_channels:
tiadc_channels_remove(indio_dev);
return err;
@@ -263,6 +465,7 @@ static int tiadc_remove(struct platform_device *pdev)
u32 step_en;
iio_device_unregister(indio_dev);
+ tiadc_iio_buffered_hardware_remove(indio_dev);
tiadc_channels_remove(indio_dev);
step_en = get_adc_step_mask(adc_dev);
@@ -301,7 +504,7 @@ static int tiadc_resume(struct device *dev)
restore &= ~(CNTRLREG_POWERDOWN);
tiadc_writel(adc_dev, REG_CTRL, restore);
- tiadc_step_config(adc_dev);
+ tiadc_step_config(indio_dev);
return 0;
}
diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
index db1791b..7d98562 100644
--- a/include/linux/mfd/ti_am335x_tscadc.h
+++ b/include/linux/mfd/ti_am335x_tscadc.h
@@ -46,16 +46,24 @@
/* Step Enable */
#define STEPENB_MASK (0x1FFFF << 0)
#define STEPENB(val) ((val) << 0)
+#define ENB(val) (1 << (val))
+#define STPENB_STEPENB STEPENB(0x1FFFF)
+#define STPENB_STEPENB_TC STEPENB(0x1FFF)
/* IRQ enable */
#define IRQENB_HW_PEN BIT(0)
#define IRQENB_FIFO0THRES BIT(2)
+#define IRQENB_FIFO0OVRRUN BIT(3)
+#define IRQENB_FIFO0UNDRFLW BIT(4)
#define IRQENB_FIFO1THRES BIT(5)
+#define IRQENB_FIFO1OVRRUN BIT(6)
+#define IRQENB_FIFO1UNDRFLW BIT(7)
#define IRQENB_PENUP BIT(9)
/* Step Configuration */
#define STEPCONFIG_MODE_MASK (3 << 0)
#define STEPCONFIG_MODE(val) ((val) << 0)
+#define STEPCONFIG_MODE_SWCNT STEPCONFIG_MODE(1)
#define STEPCONFIG_MODE_HWSYNC STEPCONFIG_MODE(2)
#define STEPCONFIG_AVG_MASK (7 << 2)
#define STEPCONFIG_AVG(val) ((val) << 2)
@@ -124,6 +132,7 @@
#define MAX_CLK_DIV 7
#define TOTAL_STEPS 16
#define TOTAL_CHANNELS 8
+#define FIFO1_THRESHOLD 19
/*
* ADC runs at 3MHz, and it takes
--
1.7.9.5
^ permalink raw reply related
* [PATCH 2/3] iio: ti_am335x_adc: optimize memory usage
From: Zubair Lutfullah @ 2013-09-19 6:24 UTC (permalink / raw)
To: jic23, dmitry.torokhov
Cc: linux-iio, linux-input, linux-kernel, bigeasy, gregkh,
zubair.lutfullah
In-Reply-To: <1379571876-12420-1-git-send-email-zubair.lutfullah@gmail.com>
12 bit ADC data is stored in 32 bits of storage.
Change from u32 to u16 to reduce wasted memory.
Signed-off-by: Zubair Lutfullah <zubair.lutfullah@gmail.com>
---
drivers/iio/adc/ti_am335x_adc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c
index a952538..ebe93eb 100644
--- a/drivers/iio/adc/ti_am335x_adc.c
+++ b/drivers/iio/adc/ti_am335x_adc.c
@@ -122,7 +122,7 @@ static int tiadc_channel_init(struct iio_dev *indio_dev, int channels)
chan->datasheet_name = chan_name_ain[chan->channel];
chan->scan_type.sign = 'u';
chan->scan_type.realbits = 12;
- chan->scan_type.storagebits = 32;
+ chan->scan_type.storagebits = 16;
}
indio_dev->channels = chan_array;
@@ -186,7 +186,7 @@ static int tiadc_read_raw(struct iio_dev *indio_dev,
if (stepid == map_val) {
read = read & FIFOREAD_DATA_MASK;
found = true;
- *val = read;
+ *val = (u16) read;
}
}
--
1.7.9.5
^ permalink raw reply related
* [PATCH 1/3] input: ti_am335x_tsc: Enable shared IRQ for TSC
From: Zubair Lutfullah @ 2013-09-19 6:24 UTC (permalink / raw)
To: jic23, dmitry.torokhov
Cc: linux-iio, linux-input, linux-kernel, bigeasy, gregkh,
zubair.lutfullah
In-Reply-To: <1379571876-12420-1-git-send-email-zubair.lutfullah@gmail.com>
Enable shared IRQ to allow ADC to share IRQ line from
parent MFD core. Only FIFO0 IRQs are for TSC and handled
on the TSC side.
Step mask would be updated from cached variable only previously.
In rare cases when both TSC and ADC are used, the cached
variable gets mixed up.
The step mask is written with the required mask every time.
Rachna Patil (TI) laid ground work for shared IRQ.
Signed-off-by: Zubair Lutfullah <zubair.lutfullah@gmail.com>
Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/touchscreen/ti_am335x_tsc.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/input/touchscreen/ti_am335x_tsc.c b/drivers/input/touchscreen/ti_am335x_tsc.c
index e1c5300..24e625c 100644
--- a/drivers/input/touchscreen/ti_am335x_tsc.c
+++ b/drivers/input/touchscreen/ti_am335x_tsc.c
@@ -52,6 +52,7 @@ struct titsc {
u32 config_inp[4];
u32 bit_xp, bit_xn, bit_yp, bit_yn;
u32 inp_xp, inp_xn, inp_yp, inp_yn;
+ u32 step_mask;
};
static unsigned int titsc_readl(struct titsc *ts, unsigned int reg)
@@ -196,7 +197,8 @@ static void titsc_step_config(struct titsc *ts_dev)
/* The steps1 … end and bit 0 for TS_Charge */
stepenable = (1 << (end_step + 2)) - 1;
- am335x_tsc_se_set(ts_dev->mfd_tscadc, stepenable);
+ ts_dev->step_mask = stepenable;
+ am335x_tsc_se_set(ts_dev->mfd_tscadc, ts_dev->step_mask);
}
static void titsc_read_coordinates(struct titsc *ts_dev,
@@ -260,6 +262,10 @@ static irqreturn_t titsc_irq(int irq, void *dev)
unsigned int fsm;
status = titsc_readl(ts_dev, REG_IRQSTATUS);
+ /*
+ * ADC and touchscreen share the IRQ line.
+ * FIFO1 interrupts are used by ADC. Handle FIFO0 IRQs here only
+ */
if (status & IRQENB_FIFO0THRES) {
titsc_read_coordinates(ts_dev, &x, &y, &z1, &z2);
@@ -316,7 +322,7 @@ static irqreturn_t titsc_irq(int irq, void *dev)
if (irqclr) {
titsc_writel(ts_dev, REG_IRQSTATUS, irqclr);
- am335x_tsc_se_update(ts_dev->mfd_tscadc);
+ am335x_tsc_se_set(ts_dev->mfd_tscadc, ts_dev->step_mask);
return IRQ_HANDLED;
}
return IRQ_NONE;
@@ -389,7 +395,7 @@ static int titsc_probe(struct platform_device *pdev)
}
err = request_irq(ts_dev->irq, titsc_irq,
- 0, pdev->dev.driver->name, ts_dev);
+ IRQF_SHARED, pdev->dev.driver->name, ts_dev);
if (err) {
dev_err(&pdev->dev, "failed to allocate irq.\n");
goto err_free_mem;
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH 2/2] iio: ti_am335x_adc: Add continuous sampling support
From: Zubair Lutfullah : @ 2013-09-19 5:55 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Zubair Lutfullah :, jic23-KWPb1pKIrIJaa/9Udqfwiw,
dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w,
linux-iio-u79uwXL29TY76Z2rM5mHXA,
linux-input-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
bigeasy-hfZtesqFncYOwBW4kG4KsQ,
gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r
In-Reply-To: <085d9527-c80e-4307-b8b9-c008976b4be1-2ueSQiBKiTY7tOexoI0I+QC/G2K4zDHf@public.gmane.org>
On Thu, Sep 19, 2013 at 06:41:22AM +0100, Jonathan Cameron wrote:
>
>
> >...
> >> >
> >> > struct tiadc_device {
> >> > struct ti_tscadc_dev *mfd_tscadc;
> >> > int channels;
> >> > u8 channel_line[8];
> >> > u8 channel_step[8];
> >> >+ int buffer_en_ch_steps;
> >> >+ u32 *data;
> >> u16 *data;
> >>
> >> Also it might actually save memory to simply have a fixed size array
> >> of the maximum size used here and avoid the extra allocations for
> >> the cost
> >> of 16 bytes in here.
> >>
> >> Hence,
> >>
> >> u16 data[8];
> >> > };
> >
> >Why data[8]? The length is dynamic.
> >This would be valid for the usual one sample per trigger case.
> >But here its continuous sampling and the hardware pushes samples
> >*quickly*
> >Dynamic allocation is needed.
>
> As far as I can see you pull one set of channels into data[] then push that out to the kfifo.
>
> That is repeated lots of times but only up to 8 entries are ever used in this array. If not what is the maximum scanbytes can be?
>
You have a good eye :).
I understand and will update.
Thanks
Zubair
> >
> >
> >...
> >
> >> >+static irqreturn_t tiadc_worker_h(int irq, void *private)
> >> >+{
> >> >+ struct iio_dev *indio_dev = private;
> >> >+ struct tiadc_device *adc_dev = iio_priv(indio_dev);
> >> >+ int i, k, fifo1count, read;
> >> >+ u32 *data = adc_dev->data;
> >> u16* data = adc_dev->data;
> >> >+
> >> >+ fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
> >> >+ for (k = 0; k < fifo1count; k = k + i) {
> >> >+ for (i = 0; i < (indio_dev->scan_bytes)/4; i++) {
> >> >+ read = tiadc_readl(adc_dev, REG_FIFO1);
> >> >+ data[i] = read & FIFOREAD_DATA_MASK;
>
> i is only ever up to scanbytes / 4.
> Hence max of 8 I think?
>
> Now there is a good argument for adding some bulk filling code for the buffers but that is not happening here.
> >> //This is a 12 bit number after the mask so will fit just fine into
> >16 bits.
> >
^ permalink raw reply
* Re: [PATCH 2/2] iio: ti_am335x_adc: Add continuous sampling support
From: Jonathan Cameron @ 2013-09-19 5:41 UTC (permalink / raw)
To: Zubair Lutfullah :
Cc: jic23-KWPb1pKIrIJaa/9Udqfwiw,
dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w,
linux-iio-u79uwXL29TY76Z2rM5mHXA,
linux-input-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
bigeasy-hfZtesqFncYOwBW4kG4KsQ,
gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r
In-Reply-To: <20130919052419.GB4363-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
"Zubair Lutfullah :" <zubair.lutfullah-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>On Wed, Sep 18, 2013 at 02:58:47PM +0100, Jonathan Cameron wrote:
>> On 18/09/13 12:23, Zubair Lutfullah wrote:
>> >Previously the driver had only one-shot reading functionality.
>> >This patch adds continuous sampling support to the driver.
>> >
>...
>>
>> Very very nearly there. Couple of suggestions in-line.
>> (about 30 seconds work + testing ;)
>
>Thank-you so much. Makes me want to put in more work and finish it :)
>
>>
>> I'm still unsure of why we need 32bit storage in the fifo
>> for the data. I've proposed the changes I'd make to put it in 16 bit
>> storage inline. The fact that the device is working in 32bits
>> is irrelevant given we only have a 12 bit number coming out and
>> it is in 12 least significant bits. I guess there might be a tiny
>> cost in doing the conversion, but you kfifo will be half the size
>> as a result and that seems more likely to a worthwhile gain!
>>
>
>I understand and will make the changes.
>
>> Out of interest, are you testing this with generic_buffer.c?
>> If so, what changes were necessary? Given this driver will not
>> have a trigger it would be nice to update that example code to handle
>> that case if it doesn't already.
>
>I simply remove the lines like goto err_trigger etc. :p
>Sneaky but works. I wasn't sure how to make the code understand its a
>INDIO_BUFFER_HARDWARE..
>But this is a separate discussion..
>
>>
>>
>> >---
>> > drivers/iio/adc/ti_am335x_adc.c | 216
>+++++++++++++++++++++++++++++++++-
>> > include/linux/mfd/ti_am335x_tscadc.h | 9 ++
>> > 2 files changed, 220 insertions(+), 5 deletions(-)
>> >
>> >diff --git a/drivers/iio/adc/ti_am335x_adc.c
>b/drivers/iio/adc/ti_am335x_adc.c
>...
>> >
>> > struct tiadc_device {
>> > struct ti_tscadc_dev *mfd_tscadc;
>> > int channels;
>> > u8 channel_line[8];
>> > u8 channel_step[8];
>> >+ int buffer_en_ch_steps;
>> >+ u32 *data;
>> u16 *data;
>>
>> Also it might actually save memory to simply have a fixed size array
>> of the maximum size used here and avoid the extra allocations for
>> the cost
>> of 16 bytes in here.
>>
>> Hence,
>>
>> u16 data[8];
>> > };
>
>Why data[8]? The length is dynamic.
>This would be valid for the usual one sample per trigger case.
>But here its continuous sampling and the hardware pushes samples
>*quickly*
>Dynamic allocation is needed.
As far as I can see you pull one set of channels into data[] then push that out to the kfifo.
That is repeated lots of times but only up to 8 entries are ever used in this array. If not what is the maximum scanbytes can be?
>
>
>...
>
>> >+static irqreturn_t tiadc_worker_h(int irq, void *private)
>> >+{
>> >+ struct iio_dev *indio_dev = private;
>> >+ struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> >+ int i, k, fifo1count, read;
>> >+ u32 *data = adc_dev->data;
>> u16* data = adc_dev->data;
>> >+
>> >+ fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
>> >+ for (k = 0; k < fifo1count; k = k + i) {
>> >+ for (i = 0; i < (indio_dev->scan_bytes)/4; i++) {
>> >+ read = tiadc_readl(adc_dev, REG_FIFO1);
>> >+ data[i] = read & FIFOREAD_DATA_MASK;
i is only ever up to scanbytes / 4.
Hence max of 8 I think?
Now there is a good argument for adding some bulk filling code for the buffers but that is not happening here.
>> //This is a 12 bit number after the mask so will fit just fine into
>16 bits.
>
>Indeed
>...
>> >+
>> >+static int tiadc_buffer_postenable(struct iio_dev *indio_dev)
>> >+{
>> >+ struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> >+ struct iio_buffer *buffer = indio_dev->buffer;
>> >+ unsigned int enb = 0;
>> >+ u8 bit;
>> >+
>> (can drop this if doing the array with adc_dev as suggested above)
>> >+ adc_dev->data = kmalloc(indio_dev->scan_bytes, GFP_KERNEL);
>> >+ if (adc_dev->data == NULL)
>> >+ return -ENOMEM;
>
>As explained previously. Large array.. This is needed..
>
>...
>> >+{
>> >+ struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> >+
>> >+ tiadc_step_config(indio_dev);
>> (can drop this if doing the array withing adc_dev as suggested above)
>> >+ kfree(adc_dev->data);
>> This is also missbalanced with the preenable (which is true of quite
>> a few drivers - one day I'll clean those up!)
>
>Misbalanced? :s
>
>> >+
>> >+ return 0;
>> >+}
>>
>
>Thanks for the review and feedback.
>I'll resend the patches with 16 bit everything and dynamic allocation.
>
>Zubair
>>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox