* Re: [PATCH v2] input: touchscreen: edt-ft5x06: fix driver autoprobing
From: Wolfram Sang @ 2014-11-22 9:06 UTC (permalink / raw)
To: Felipe Balbi
Cc: Dmitry Torokhov, Linux OMAP Mailing List, linux-input, stable
In-Reply-To: <20141122010524.GA26751@saruman>
[-- Attachment #1: Type: text/plain, Size: 325 bytes --]
> Can you comment on this ? As of today i2c-based drivers will probe with
> DT-based boot if DT's compatible is set to the same id as in struct
> i2c_device_id. No i2c driver with of_device_id can autoprobe using OF
> ids.
Long standing problem. Last one to tackle this was Lee Jones:
https://lkml.org/lkml/2014/8/28/283
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH v2] input: touchscreen: edt-ft5x06: fix driver autoprobing
From: Felipe Balbi @ 2014-11-22 1:05 UTC (permalink / raw)
To: Felipe Balbi
Cc: Dmitry Torokhov, Linux OMAP Mailing List, linux-input, stable,
wsa
In-Reply-To: <20141118201907.GQ6179@saruman>
[-- Attachment #1: Type: text/plain, Size: 3903 bytes --]
Hi Wolfram,
On Tue, Nov 18, 2014 at 02:19:07PM -0600, Felipe Balbi wrote:
> On Tue, Nov 18, 2014 at 12:14:21PM -0800, Dmitry Torokhov wrote:
> > On Tue, Nov 18, 2014 at 02:03:40PM -0600, Felipe Balbi wrote:
> > > On Tue, Nov 18, 2014 at 11:59:02AM -0800, Dmitry Torokhov wrote:
> > > > On Tue, Nov 18, 2014 at 01:27:42PM -0600, Felipe Balbi wrote:
> > > > > Because with OF we can pass more specific
> > > > > compatible flags (such as edt-ft5306) instead
> > > > > of generic edt-ft5x06, when i2c-core's
> > > > > of_i2c_register_devices() tries to request_module(),
> > > > > it'll request it with a non-existent specific module
> > > > > alias.
> > > > >
> > > > > In order to have this driver autoprobing again, we
> > > > > just need to add missing MODULE_ALIAS() entries to
> > > > > edt-ft5x06 driver.
> > > > >
> > > > > Thanks to Dmitry for noticing that it actually should
> > > > > autoprobe even with of_device_id.
> > > > >
> > > > > Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > > > > Cc: <linux-input@vger.kernel.org>
> > > > > Cc: <stable@vger.kernel.org> # v3.15+
> > > > > Fixes: dac90dc2 (Input: edt-ft5x06 - add DT support)
> > > > > Signed-off-by: Felipe Balbi <balbi@ti.com>
> > > > > ---
> > > > > drivers/input/touchscreen/edt-ft5x06.c | 3 +++
> > > > > 1 file changed, 3 insertions(+)
> > > > >
> > > > > diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
> > > > > index ee3434f..bcbf688 100644
> > > > > --- a/drivers/input/touchscreen/edt-ft5x06.c
> > > > > +++ b/drivers/input/touchscreen/edt-ft5x06.c
> > > > > @@ -1147,6 +1147,9 @@ static struct i2c_driver edt_ft5x06_ts_driver = {
> > > > >
> > > > > module_i2c_driver(edt_ft5x06_ts_driver);
> > > > >
> > > > > +MODULE_ALIAS("i2c:edt-ft5206");
> > > > > +MODULE_ALIAS("i2c:edt-ft5306");
> > > > > +MODULE_ALIAS("i2c:edt-ft5406");
> > > > > MODULE_AUTHOR("Simon Budig <simon.budig@kernelconcepts.de>");
> > > > > MODULE_DESCRIPTION("EDT FT5x06 I2C Touchscreen Driver");
> > > > > MODULE_LICENSE("GPL");
> > > > > --
> > > > > 2.1.0.GIT
> > > > >
> > > >
> > > > Here is what I see if I run modinfo on the module:
> > > >
> > > > dtor@dtor-ws:~/kernel/work$ modinfo
> > > > drivers/input/touchscreen/edt-ft5x06.ko
> > > > filename:
> > > > /home/dtor/kernel/work/drivers/input/touchscreen/edt-ft5x06.ko
> > > > license: GPL
> > > > description: EDT FT5x06 I2C Touchscreen Driver
> > > > author: Simon Budig <simon.budig@kernelconcepts.de>
> > > > alias: i2c:edt-ft5x06
> > > > alias: of:N*T*Cedt,edt-ft5406*
> > > > alias: of:N*T*Cedt,edt-ft5306*
> > > > alias: of:N*T*Cedt,edt-ft5206*
> > > > depends: i2c-core
> > > > intree: Y
> > > > vermagic: 3.17.0+ SMP preempt mod_unload
> > > >
> > > > As you can see we already have what I consider proper modaliases for the
> > > > driver. Why don't they work? Is it because modprobe doe snot know how to
> > > > handle OF-style modaliases or device's uevents are missing OF data in
> > > > them?
> > >
> > > no, it's because i2c call request_module for i2c:edt-ft5306. i2c core
> > > never uses any of the of aliases.
> >
> > 1. I think both i2c and spi are cheating in this regard: they should
> > rely on normal driver request paths through udev.
> >
> > 2. I2C should emit proper modalias for devices coming by the way of
> > devicetree. It already does special handling for ACPI, it needs to do
> > the same for OF. I think SPI does the right thing there.
>
> I'll let Wolfram comment here for I2C.
Can you comment on this ? As of today i2c-based drivers will probe with
DT-based boot if DT's compatible is set to the same id as in struct
i2c_device_id. No i2c driver with of_device_id can autoprobe using OF
ids.
cheers
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] HID: usbhid: get/put around clearing needs_remote_wakeup
From: Benson Leung @ 2014-11-22 1:00 UTC (permalink / raw)
To: Oliver Neukum
Cc: johan, Jiri Kosina, linux-usb, linux-input@vger.kernel.org,
linux-kernel@vger.kernel.org, Sameer Nanda
In-Reply-To: <1415956110.2640.5.camel@linux-0dmf.site>
On Fri, Nov 14, 2014 at 1:08 AM, Oliver Neukum <oneukum@suse.de> wrote:
> On Thu, 2014-11-13 at 12:16 -0800, Benson Leung wrote:
>
>> In usbhid_open, usb_autopm_get_interface is called
>> before setting the needs_remote_wakeup flag, and
>> usb_autopm_put_interface is called after hid_start_in.
>>
>> However, when the device is closed in usbhid_close, the same
>> protection isn't there when clearing needs_remote_wakeup. This will
>> add that to usbhid_close as well as usbhid_stop.
>
> Interesting, but this has the side effect of waking devices
> that are asleep just to remove the flag.
>
> Regards
If devices are already asleep with this flag enabled, that means that
they are presently configured for remote wake.
Waking the device in the case of a close() is appropriate because it
also has the effect of re-suspending the device with the capability
disabled, as it is no longer necessary.
--
Benson Leung
Software Engineer, Chrom* OS
bleung@chromium.org
^ permalink raw reply
* Re: [PATCH] HID: usbhid: get/put around clearing needs_remote_wakeup
From: Benson Leung @ 2014-11-22 0:44 UTC (permalink / raw)
To: Alan Stern
Cc: johan, Jiri Kosina, linux-usb, linux-input@vger.kernel.org,
linux-kernel@vger.kernel.org, Sameer Nanda
In-Reply-To: <Pine.LNX.4.44L0.1411141008020.1043-100000@iolanthe.rowland.org>
Hi Alan,
On Fri, Nov 14, 2014 at 7:17 AM, Alan Stern <stern@rowland.harvard.edu> wrote:
>
> The reason for the get/put is to force a call to autosuspend_check().
> But in this case, if killing the interrupt URB causes
> autosuspend_check() to run then the get/put isn't needed.
>
> On the other hand, I don't see why killing the interrupt URB would
> cause autosuspend_check() to run. Can you explain that?
Sorry for the delay in my response. I did some more checking of my
particular failure, and my commit message is incorrect. The
usb_kill_urb is actually not the cause of this problem. It does not
result in autosuspend_check() itself, and is only serving to add some
delay.
hidraw_release() in hidraw.c calls drop_ref(), which calls the
following in sequence upon clearing the last reader :
/* close device for last reader */
hid_hw_power(hidraw->hid, PM_HINT_NORMAL);
hid_hw_close(hidraw->hid);
hid_hw_power results in a usb_autopm_put_interface. In this case, the
reference count is decremented to 0, and a delayed autosuspend request
is attempted.
hid_hw_close leads to usbhid_close, which clears needs_remote_wakeup.
However, there's no guarantee that the clear of needs_remote_wakeup
will occur before the delayed work ( runtime_idle() ->
autosuspend_check() ) runs. Moving usbhid->intf->needs_remote_wakeup
= 0 to before the usb_kill_urb(usbhid->urbin) only serves to reduce
the amount of time between these events and makes this particular
failure less likely.
The correct solution is to put get/put around each change of
needs_remote_wakeup, as that will correctly trigger another delayed
autosuspend_check(), whose result is affected by the state of
needs_remote_wakeup.
Since autosuspend_check() occurs as delayed work, I think it is
appropriate to add get/put around the clear in usbhid_stop as well.
--
Benson Leung
Software Engineer, Chrom* OS
bleung@chromium.org
On Fri, Nov 14, 2014 at 7:17 AM, Alan Stern <stern@rowland.harvard.edu> wrote:
> On Thu, 13 Nov 2014, Benson Leung wrote:
>
>> Hi Alan,
>>
>> On Thu, Nov 13, 2014 at 2:11 PM, Alan Stern <stern@rowland.harvard.edu> wrote:
>> > Wait a minute -- in your previous email you said this approach didn't
>> > work. So does it work or doesn't it?
>>
>> Sorry for the confusion. The approach *does* work.
>>
>> That was actually my original idea to fix the problem, but I saw other
>> places in the kernel where it was done with a get/put.
>
> The reason for the get/put is to force a call to autosuspend_check().
> But in this case, if killing the interrupt URB causes
> autosuspend_check() to run then the get/put isn't needed.
>
> On the other hand, I don't see why killing the interrupt URB would
> cause autosuspend_check() to run. Can you explain that?
>
> Alan Stern
>
>
>
--
Benson Leung
Software Engineer, Chrom* OS
bleung@chromium.org
^ permalink raw reply
* RE: [PATCH v4 3/6] mfd: ti_am335x_tscadc: Remove unwanted reg_se_cache save
From: Griffis, Brad @ 2014-11-21 22:25 UTC (permalink / raw)
To: Richard Cochran, Johannes Pointner
Cc: Nori, Sekhar, R, Vignesh, Dmitry Torokhov, Lee Jones, Rob Herring,
Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
Benoit Cousson, Tony Lindgren, Russell King, Jonathan Cameron,
Hartmut Knaack, Sebastian Andrzej Siewior, Lars-Peter Clausen,
Peter Meerwald, Samuel Ortiz, Balbi, Felipe, Sanjeev Sharma,
Paul Gortmaker, Jan Kardell, devicetree
In-Reply-To: <20141121214005.GA4020@localhost.localdomain>
> -----Original Message-----
> From: Richard Cochran [mailto:richardcochran@gmail.com]
>
> On Fri, Nov 21, 2014 at 07:17:18PM +0100, Johannes Pointner wrote:
> > Before the patches were also jumps but I thought it is something
> > Vignesh should know. Maybe there is some fix for that too?
I believe I've seen the "jumping" behavior pop up at a couple random times in my testing. I think it relates to the hardware not properly registering a pen-up interrupt. When that's happening can you try running ts_calibrate? If you're not registering a pen-up event then you won't be able to advance through those touch points. Is this behavior related to capturing ADC samples, or do you see this behavior even without capturing ADC samples?
> > As Richard also noted, it would be nice if ti could let us know how to
> > get the delay values right. By trial and error is IMHO not the best
> > way.
>
> That is not only an opinion, it is a matter of fact. TI really dropped the ball on
> this one. I thought the patch series was a sign they finally were going to
> properly address this issue. Wrong again.
These patches originate from work I did on TI's SDK 7.00 (3.12 kernel). The touchscreen is working beautifully in that environment and we are putting forth significant effort to bring those improvements to the community.
Things do not seem to be working as well with the current mainline kernel. I've been reviewing various patches between 3.12 and the mainline to understand why it's different. I believe I have figured this out, and I have discussions going separately with individuals that contributed those patches in order to come up with the best possible solution, i.e. we likely need a few additional changes to this patch set.
The CHARGECONFIG delay serves precisely the same purpose as the udelay in the original code base. How did you determine the udelay value in the first place? I went through the effort of figuring out a way to make the delay into a HARDWARE delay instead of a software delay so that it could be removed from the ISR. That delay time relates to "settling time" and so it's going to be dependent on your PCB. You could hook up an oscilloscope and attempt to capture this parameter from the signals themselves. Personally I think that's a lot more difficult than just doing a few iterations of testing.
> The data sheet for the TI part used to have a reference to an app-note for
> the touch controller.
>
> spruh73f page 1154
>
> The Pen IRQ can only occur if the correct Pen Ctrl bits are high
> (AFE Pen Ctrl bits 6:5 in the TSC_ADC control register) and if
> the correct ground transistor biasing is set in the StepConfig
> [N] Register. Refer to the application notes for the correct
> settings.
>
> I searched high and low for this application note. Then, the data sheet got
> revised.
Such an application note does not exist, which explains why you didn't find it and why we have removed that reference from the TRM. Sorry for your trouble. We always strive for accurate documentation.
^ permalink raw reply
* Re: [PATCH 1/2] HID: wacom: Report ABS_TILT_{X,Y} as signed values
From: Jiri Kosina @ 2014-11-21 21:41 UTC (permalink / raw)
To: Ping Cheng; +Cc: Jason Gerecke, linux-input, benjamin.tissores
In-Reply-To: <CAF8JNhJMcD2QcC-Gp9+avxdQUpEjyTJ=R3-XKv+1Sm-uYhxNXg@mail.gmail.com>
On Fri, 21 Nov 2014, Ping Cheng wrote:
> On Tue, Nov 18, 2014 at 4:50 PM, Jason Gerecke <killertofu@gmail.com> wrote:
> > Centers the ABS_TILT_{X,Y} axes so that a value of zero is reported when
> > the pen is vertical. Combined with resolution information in the next
> > patch, this makes it possible for userspace to calculate the pen angle
> > without needing hardware-specific knowledge. The xf86-input-wacom driver
> > was updated to support signed tilt values in late-2012 (2f2acec).
> >
> > Signed-off-by: Jason Gerecke <killertofu@gmail.com>
>
> Reviewed-by: Ping Cheng <pingc@wacom.com> for the series.
Series now queued in for-3.19/wacom. Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH v4 3/6] mfd: ti_am335x_tscadc: Remove unwanted reg_se_cache save
From: Richard Cochran @ 2014-11-21 21:40 UTC (permalink / raw)
To: Johannes Pointner
Cc: Sekhar Nori, Vignesh R, Dmitry Torokhov, Lee Jones, Rob Herring,
Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
Benoit Cousson, Tony Lindgren, Russell King, Jonathan Cameron,
Hartmut Knaack, Sebastian Andrzej Siewior, Lars-Peter Clausen,
Peter Meerwald, Samuel Ortiz, Felipe Balbi, Brad Griffis,
Sanjeev Sharma, Paul Gortmaker, Jan Kardell, devicetree
In-Reply-To: <CAJKbBDD=eYmU=LuViwLPNV=kYCziDQgKjL07bT7og4vsj0Ee7A@mail.gmail.com>
On Fri, Nov 21, 2014 at 07:17:18PM +0100, Johannes Pointner wrote:
> Before the patches were also jumps but I thought it is something
> Vignesh should know. Maybe there is some fix for that too?
> As Richard also noted, it would be nice if ti could let us know how to
> get the delay values right. By trial and error is IMHO not the best
> way.
That is not only an opinion, it is a matter of fact. TI really dropped
the ball on this one. I thought the patch series was a sign they
finally were going to properly address this issue. Wrong again.
The data sheet for the TI part used to have a reference to an app-note
for the touch controller.
spruh73f page 1154
The Pen IRQ can only occur if the correct Pen Ctrl bits are high
(AFE Pen Ctrl bits 6:5 in the TSC_ADC control register) and if
the correct ground transistor biasing is set in the StepConfig
[N] Register. Refer to the application notes for the correct
settings.
I searched high and low for this application note. Then, the data
sheet got revised.
Thanks,
Richard
^ permalink raw reply
* Re: [PATCH 1/2] HID: wacom: Report ABS_TILT_{X,Y} as signed values
From: Ping Cheng @ 2014-11-21 19:55 UTC (permalink / raw)
To: Jason Gerecke; +Cc: Jiri Kosina, linux-input, benjamin.tissores
In-Reply-To: <1416358209-3937-1-git-send-email-killertofu@gmail.com>
On Tue, Nov 18, 2014 at 4:50 PM, Jason Gerecke <killertofu@gmail.com> wrote:
> Centers the ABS_TILT_{X,Y} axes so that a value of zero is reported when
> the pen is vertical. Combined with resolution information in the next
> patch, this makes it possible for userspace to calculate the pen angle
> without needing hardware-specific knowledge. The xf86-input-wacom driver
> was updated to support signed tilt values in late-2012 (2f2acec).
>
> Signed-off-by: Jason Gerecke <killertofu@gmail.com>
Reviewed-by: Ping Cheng <pingc@wacom.com> for the series.
Thanks Jason for the patches,
Ping
> ---
> drivers/hid/wacom_wac.c | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
> index 8a83da9..525b648 100644
> --- a/drivers/hid/wacom_wac.c
> +++ b/drivers/hid/wacom_wac.c
> @@ -600,8 +600,8 @@ static void wacom_intuos_general(struct wacom_wac *wacom)
> }
> input_report_abs(input, ABS_PRESSURE, t);
> input_report_abs(input, ABS_TILT_X,
> - ((data[7] << 1) & 0x7e) | (data[8] >> 7));
> - input_report_abs(input, ABS_TILT_Y, data[8] & 0x7f);
> + (((data[7] << 1) & 0x7e) | (data[8] >> 7)) - 64);
> + input_report_abs(input, ABS_TILT_Y, (data[8] & 0x7f) - 64);
> input_report_key(input, BTN_STYLUS, data[1] & 2);
> input_report_key(input, BTN_STYLUS2, data[1] & 4);
> input_report_key(input, BTN_TOUCH, t > 10);
> @@ -612,8 +612,8 @@ static void wacom_intuos_general(struct wacom_wac *wacom)
> input_report_abs(input, ABS_WHEEL,
> (data[6] << 2) | ((data[7] >> 6) & 3));
> input_report_abs(input, ABS_TILT_X,
> - ((data[7] << 1) & 0x7e) | (data[8] >> 7));
> - input_report_abs(input, ABS_TILT_Y, data[8] & 0x7f);
> + (((data[7] << 1) & 0x7e) | (data[8] >> 7)) - 64);
> + input_report_abs(input, ABS_TILT_Y, (data[8] & 0x7f) - 64);
> }
> }
>
> @@ -915,8 +915,8 @@ static int wacom_intuos_irq(struct wacom_wac *wacom)
> input_report_key(input, BTN_EXTRA, data[6] & 0x10);
>
> input_report_abs(input, ABS_TILT_X,
> - ((data[7] << 1) & 0x7e) | (data[8] >> 7));
> - input_report_abs(input, ABS_TILT_Y, data[8] & 0x7f);
> + (((data[7] << 1) & 0x7e) | (data[8] >> 7)) - 64);
> + input_report_abs(input, ABS_TILT_Y, (data[8] & 0x7f) - 64);
> } else {
> /* 2D mouse packet */
> input_report_key(input, BTN_LEFT, data[8] & 0x04);
> @@ -1926,8 +1926,8 @@ static void wacom_setup_cintiq(struct wacom_wac *wacom_wac)
> input_set_abs_params(input_dev, ABS_DISTANCE,
> 0, wacom_wac->features.distance_max, 0, 0);
> input_set_abs_params(input_dev, ABS_WHEEL, 0, 1023, 0, 0);
> - input_set_abs_params(input_dev, ABS_TILT_X, 0, 127, 0, 0);
> - input_set_abs_params(input_dev, ABS_TILT_Y, 0, 127, 0, 0);
> + input_set_abs_params(input_dev, ABS_TILT_X, -64, 63, 0, 0);
> + input_set_abs_params(input_dev, ABS_TILT_Y, -64, 63, 0, 0);
> }
>
> static void wacom_setup_intuos(struct wacom_wac *wacom_wac)
> --
> 2.1.3
>
^ permalink raw reply
* [PATCH] Input: Initialize input_no by -1
From: Aniroop Mathur @ 2014-11-21 18:48 UTC (permalink / raw)
To: Dmitry Torokhov, linux-input@vger.kernel.org
This patch initializes input_no by -1 in order to avoid extra subtraction
operation performed everytime for allocation of an input device.
Signed-off-by: Aniroop Mathur <aniroop.mathur@gmail.com>
---
drivers/input/input.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/input/input.c b/drivers/input/input.c
index 29ca0bb..01fe49e 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -1774,7 +1774,7 @@ EXPORT_SYMBOL_GPL(input_class);
*/
struct input_dev *input_allocate_device(void)
{
- static atomic_t input_no = ATOMIC_INIT(0);
+ static atomic_t input_no = ATOMIC_INIT(-1);
struct input_dev *dev;
dev = kzalloc(sizeof(struct input_dev), GFP_KERNEL);
@@ -1789,7 +1789,7 @@ struct input_dev *input_allocate_device(void)
INIT_LIST_HEAD(&dev->node);
dev_set_name(&dev->dev, "input%ld",
- (unsigned long) atomic_inc_return(&input_no) - 1);
+ (unsigned long) atomic_inc_return(&input_no));
__module_get(THIS_MODULE);
}
--
1.9.1
^ permalink raw reply related
* Re: [PATCH v4 3/6] mfd: ti_am335x_tscadc: Remove unwanted reg_se_cache save
From: Johannes Pointner @ 2014-11-21 18:17 UTC (permalink / raw)
To: Sekhar Nori
Cc: Vignesh R, Dmitry Torokhov, Lee Jones, Rob Herring, Pawel Moll,
Mark Rutland, Ian Campbell, Kumar Gala, Benoit Cousson,
Tony Lindgren, Russell King, Jonathan Cameron, Hartmut Knaack,
Sebastian Andrzej Siewior, Lars-Peter Clausen, Peter Meerwald,
Samuel Ortiz, Felipe Balbi, Brad Griffis, Sanjeev Sharma,
Paul Gortmaker, Jan Kardell, devicetree, linux-kernel
In-Reply-To: <546F5C27.7040004@ti.com>
Before the patches were also jumps but I thought it is something
Vignesh should know. Maybe there is some fix for that too?
As Richard also noted, it would be nice if ti could let us know how to
get the delay values right. By trial and error is IMHO not the best
way.
For the testing I used 3.16.7.
2014-11-21 16:37 GMT+01:00 Sekhar Nori <nsekhar@ti.com>:
> On Friday 21 November 2014 08:41 PM, Johannes Pointner wrote:
>> I tested version 4 of the patch series on a custom design and I saw
>> also the jumps as described by Richard. I played a little with the
>> sample delay and it got better but I couldn't completely remove the
>> jumps.
>
> And before the patches were applied, there were no jumps at all? Which
> kernel are you using for testing?
>
> Thanks,
> Sekhar
^ permalink raw reply
* Re: [PATCH v4 3/6] mfd: ti_am335x_tscadc: Remove unwanted reg_se_cache save
From: Sekhar Nori @ 2014-11-21 15:37 UTC (permalink / raw)
To: Johannes Pointner, Vignesh R
Cc: Dmitry Torokhov, Lee Jones, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, Benoit Cousson, Tony Lindgren,
Russell King, Jonathan Cameron, Hartmut Knaack,
Sebastian Andrzej Siewior, Lars-Peter Clausen, Peter Meerwald,
Samuel Ortiz, Felipe Balbi, Brad Griffis, Sanjeev Sharma,
Paul Gortmaker, Jan Kardell, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-omap@
In-Reply-To: <CAJKbBDDVTA+=priRu7sCD=vHrC8AUg4DM3kycuGrqbpF-5FGAA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Friday 21 November 2014 08:41 PM, Johannes Pointner wrote:
> I tested version 4 of the patch series on a custom design and I saw
> also the jumps as described by Richard. I played a little with the
> sample delay and it got better but I couldn't completely remove the
> jumps.
And before the patches were applied, there were no jumps at all? Which
kernel are you using for testing?
Thanks,
Sekhar
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] hid-multitouch: Add quirk for VTL touch panels
From: Benjamin Tissoires @ 2014-11-21 15:37 UTC (permalink / raw)
To: Mathieu Magnaudet
Cc: Jiri Kosina, Benjamin Tissoires, linux-input,
linux-kernel@vger.kernel.org
In-Reply-To: <1416422480-3241-1-git-send-email-mathieu.magnaudet@enac.fr>
Hi Mathieu,
On Wed, Nov 19, 2014 at 1:41 PM, Mathieu Magnaudet
<mathieu.magnaudet@gmail.com> wrote:
> VTL panels do not switch to the multitouch mode until the input mode
> feature is read by the host. This should normally be done by
> usbhid, but it looks like an other bug prevents usbhid to properly
> retrieve the feature state. As a workaround, we force the reading of
> the feature in mt_set_input_mode for such devices.
>
> Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> Signed-off-by: Mathieu Magnaudet <mathieu.magnaudet@enac.fr>
> ---
> drivers/hid/hid-ids.h | 3 +++
> drivers/hid/hid-multitouch.c | 27 +++++++++++++++++++++++++++
> 2 files changed, 30 insertions(+)
>
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index 7c86373..d6cc6a9 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -931,6 +931,9 @@
> #define USB_DEVICE_ID_VERNIER_CYCLOPS 0x0004
> #define USB_DEVICE_ID_VERNIER_LCSPEC 0x0006
>
> +#define USB_VENDOR_ID_VTL 0x0306
> +#define USB_DEVICE_ID_VTL_MULTITOUCH_FF3F 0xff3f
> +
> #define USB_VENDOR_ID_WACOM 0x056a
> #define USB_DEVICE_ID_WACOM_GRAPHIRE_BLUETOOTH 0x81
> #define USB_DEVICE_ID_WACOM_INTUOS4_BLUETOOTH 0x00BD
> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
> index 51e25b9..a4bc111 100644
> --- a/drivers/hid/hid-multitouch.c
> +++ b/drivers/hid/hid-multitouch.c
> @@ -67,6 +67,7 @@ MODULE_LICENSE("GPL");
> #define MT_QUIRK_IGNORE_DUPLICATES (1 << 10)
> #define MT_QUIRK_HOVERING (1 << 11)
> #define MT_QUIRK_CONTACT_CNT_ACCURATE (1 << 12)
> +#define MT_QUIRK_FORCE_GET_FEATURE (1 << 13)
>
> #define MT_INPUTMODE_TOUCHSCREEN 0x02
> #define MT_INPUTMODE_TOUCHPAD 0x03
> @@ -150,6 +151,7 @@ static void mt_post_parse(struct mt_device *td);
> #define MT_CLS_FLATFROG 0x0107
> #define MT_CLS_GENERALTOUCH_TWOFINGERS 0x0108
> #define MT_CLS_GENERALTOUCH_PWT_TENFINGERS 0x0109
> +#define MT_CLS_VTL 0x0110
>
> #define MT_DEFAULT_MAXCONTACT 10
> #define MT_MAX_MAXCONTACT 250
> @@ -255,6 +257,11 @@ static struct mt_class mt_classes[] = {
> .sn_move = 2048,
> .maxcontacts = 40,
> },
> + { .name = MT_CLS_VTL,
> + .quirks = MT_QUIRK_ALWAYS_VALID |
> + MT_QUIRK_CONTACT_CNT_ACCURATE |
> + MT_QUIRK_FORCE_GET_FEATURE,
> + },
> { }
> };
>
> @@ -809,6 +816,9 @@ static void mt_set_input_mode(struct hid_device *hdev)
> struct mt_device *td = hid_get_drvdata(hdev);
> struct hid_report *r;
> struct hid_report_enum *re;
> + struct mt_class *cls = &td->mtclass;
> + char *buf;
> + int report_len;
>
> if (td->inputmode < 0)
> return;
> @@ -816,6 +826,18 @@ static void mt_set_input_mode(struct hid_device *hdev)
> re = &(hdev->report_enum[HID_FEATURE_REPORT]);
> r = re->report_id_hash[td->inputmode];
> if (r) {
> + if (cls->quirks & MT_QUIRK_FORCE_GET_FEATURE) {
> + report_len = ((r->size - 1) >> 3) + 1 + (r->id > 0);
> + buf = kzalloc(report_len, GFP_KERNEL);
Sorry for not spotting this earlier.
These 2 lines can be replaced by a call to hid_alloc_report_buf(r, GFP_KERNEL);
Cheers,
Benjamin
> + if (!buf) {
> + hid_err(hdev, "failed to allocate buffer for report\n");
> + return;
> + }
> + hid_hw_raw_request(hdev, r->id, buf, report_len,
> + HID_FEATURE_REPORT,
> + HID_REQ_GET_REPORT);
> + kfree(buf);
> + }
> r->field[0]->value[td->inputmode_index] = td->inputmode_value;
> hid_hw_request(hdev, r, HID_REQ_SET_REPORT);
> }
> @@ -1281,6 +1303,11 @@ static const struct hid_device_id mt_devices[] = {
> MT_USB_DEVICE(USB_VENDOR_ID_UNITEC,
> USB_DEVICE_ID_UNITEC_USB_TOUCH_0A19) },
>
> + /* VTL panels */
> + { .driver_data = MT_CLS_VTL,
> + MT_USB_DEVICE(USB_VENDOR_ID_VTL,
> + USB_DEVICE_ID_VTL_MULTITOUCH_FF3F) },
> +
> /* Wistron panels */
> { .driver_data = MT_CLS_NSMU,
> MT_USB_DEVICE(USB_VENDOR_ID_WISTRON,
> --
> 1.9.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply
* Re: [PATCH] hid-multitouch: Add quirk for VTL touch panels
From: Benjamin Tissoires @ 2014-11-21 15:35 UTC (permalink / raw)
To: Jiri Kosina
Cc: Mathieu Magnaudet, Benjamin Tissoires, linux-input,
linux-kernel@vger.kernel.org
In-Reply-To: <CAN+gG=F5AENtv0a-sK3WmvYLdVUWEyteXce1avX7+eRSeOAswQ@mail.gmail.com>
On Thu, Nov 20, 2014 at 10:27 AM, Benjamin Tissoires
<benjamin.tissoires@gmail.com> wrote:
> On Thu, Nov 20, 2014 at 8:21 AM, Jiri Kosina <jkosina@suse.cz> wrote:
>> On Wed, 19 Nov 2014, Benjamin Tissoires wrote:
>>
>>> though this patch works for this particular device, I just thought at
>>> something which may solve the problem in a different way. I asked
>>> Mathieu to test the different solution, so I'd rather you to wait for
>>> final confirmation before merging this patch.
>>
>> Thanks for the heads up, I am putting this patch on hold until further
>> notice from you or Mathieu.
>>
>
> Mathieu confirmed this morning that using hid_hw_raw_request instead
> of hid_hw_request while _setting_ the input_mode works for this
> particular touchscreen. So I think we will simply switch to use this
> sync operation instead of relying on the async one. We will need to
> test this change a little bit, but it should be safe for BT and I2C at
> least.
>
> So, yes, we will drop this patch and have a better one in the next few
> days. Thanks for holding this one :)
>
Actually, later tests showed that this was still required. I have a
comment on the patch (see the other mail), so, please, Jiri, wait for
the v2 :)
Cheers,
Benjamin
^ permalink raw reply
* Re: [PATCH v4 3/6] mfd: ti_am335x_tscadc: Remove unwanted reg_se_cache save
From: Johannes Pointner @ 2014-11-21 15:11 UTC (permalink / raw)
To: Vignesh R
Cc: Sekhar Nori, Dmitry Torokhov, Lee Jones, Rob Herring, Pawel Moll,
Mark Rutland, Ian Campbell, Kumar Gala, Benoit Cousson,
Tony Lindgren, Russell King, Jonathan Cameron, Hartmut Knaack,
Sebastian Andrzej Siewior, Lars-Peter Clausen, Peter Meerwald,
Samuel Ortiz, Felipe Balbi, Brad Griffis, Sanjeev Sharma,
Paul Gortmaker, Jan Kardell, devicetree, linux-kernel
In-Reply-To: <20141121131036.GA18931@localhost.localdomain>
I tested version 4 of the patch series on a custom design and I saw
also the jumps as described by Richard. I played a little with the
sample delay and it got better but I couldn't completely remove the
jumps.
The other issue I had with version 3, the pen_ups during the busy
loop, is solved. I experienced some pen_up misses during the busy loop
if I used a too high sample delay, without the sample delay it was ok.
Thanks,
Hannes
2014-11-21 14:10 GMT+01:00 Richard Cochran <richardcochran@gmail.com>:
> On Fri, Nov 21, 2014 at 05:40:12PM +0530, Sekhar Nori wrote:
>> Not sure how to reproduce the jumping on pen-up.
>
> Does the cursor stay in exactly the same spot when you lift up the
> stylus? Then you don't have the issue.
>
> On the BB white using the LCD4 cape and the shipped debian kernel, the
> cursor *does* jump away, but not as often or as far as on the custom
> design I was working with.
>
> Thanks,
> Richard
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v4 3/6] mfd: ti_am335x_tscadc: Remove unwanted reg_se_cache save
From: Richard Cochran @ 2014-11-21 13:10 UTC (permalink / raw)
To: Sekhar Nori
Cc: Dmitry Torokhov, Lee Jones, Vignesh R, Rob Herring, Pawel Moll,
Mark Rutland, Ian Campbell, Kumar Gala, Benoit Cousson,
Tony Lindgren, Russell King, Jonathan Cameron, Hartmut Knaack,
Sebastian Andrzej Siewior, Lars-Peter Clausen, Peter Meerwald,
Samuel Ortiz, Felipe Balbi, Brad Griffis, Sanjeev Sharma,
Paul Gortmaker, Jan Kardell, devicetree, linux-kernel
In-Reply-To: <546F2BA4.4000808@ti.com>
On Fri, Nov 21, 2014 at 05:40:12PM +0530, Sekhar Nori wrote:
> Not sure how to reproduce the jumping on pen-up.
Does the cursor stay in exactly the same spot when you lift up the
stylus? Then you don't have the issue.
On the BB white using the LCD4 cape and the shipped debian kernel, the
cursor *does* jump away, but not as often or as far as on the custom
design I was working with.
Thanks,
Richard
^ permalink raw reply
* Re: [PATCH v4 3/6] mfd: ti_am335x_tscadc: Remove unwanted reg_se_cache save
From: Sekhar Nori @ 2014-11-21 12:10 UTC (permalink / raw)
To: Richard Cochran
Cc: Dmitry Torokhov, Lee Jones, Vignesh R, Rob Herring, Pawel Moll,
Mark Rutland, Ian Campbell, Kumar Gala, Benoit Cousson,
Tony Lindgren, Russell King, Jonathan Cameron, Hartmut Knaack,
Sebastian Andrzej Siewior, Lars-Peter Clausen, Peter Meerwald,
Samuel Ortiz, Felipe Balbi, Brad Griffis, Sanjeev Sharma,
Paul Gortmaker, Jan Kardell, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel
In-Reply-To: <20141120144028.GB5809-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
On Thursday 20 November 2014 08:10 PM, Richard Cochran wrote:
> On Thu, Nov 20, 2014 at 07:26:00PM +0530, Sekhar Nori wrote:
>> I tested this using lcd7 cape connected to beaglebone black. The latest
>> kernel I could find on this board was a TI BSP based v3.14 kernel. So I
>> had to port these patches to that kernel. Cc Robert Nelson to see if he
>> knows about a more recent kernel supporting that cape.
>
> What is missing from mainline for the black?
>
> (I thought it was fully supported by now.)
Black is supported, but LCD7 cape is not.
>
>> I did not see any breakage with these patches applied although I did not
>> see any noticeable performance improvement as well.
>
> So, the jumping on pen-up persists, right? That means the patch
> series does not provide a general fix for that issue.
Not sure how to reproduce the jumping on pen-up. I was able to draw
circles, sign my name and actually today I wrote up the whole English
alphabet on the screen. Picture here.
https://drive.google.com/file/d/0ByWLwvGx8MeyVF9CTlhHU0FHMFk/view?usp=sharing
Is there any particular pattern you want me to test with?
I would still like to test with a more recent kernel on LCD7 cape. So,
if anyone has ideas (other than I porting the LCD7 support) please let
me know.
Thanks,
Sekhar
^ permalink raw reply
* Re: [PATCH 1/2] Input: add regulator haptic driver
From: Jaewon Kim @ 2014-11-21 1:49 UTC (permalink / raw)
To: Dan Murphy, Pankaj Dubey
Cc: Kukjin Kim, Dmitry Torokhov, Chanwoo Choi, linux-kernel,
linux-input, linux-samsung-soc, Hyunhee Kim
In-Reply-To: <546E0438.90808@ti.com>
Hi Dan,
2014년 11월 21일 00:09에 Dan Murphy 이(가) 쓴 글:
> Hi
>
> On 11/20/2014 08:33 AM, Pankaj Dubey wrote:
>> Hi Jaewon,
>>
>> On 20 November 2014 19:01, Jaewon Kim <jaewon02.kim@samsung.com> wrote:
>>> This patch adds support for haptic driver controlled by
>>> voltage of regulator. And this driver support for
>>> Force Feedback interface from input framework
>>>
>>> Signed-off-by: Jaewon Kim <jaewon02.kim@samsung.com>
>>> Signed-off-by: Hyunhee Kim <hyunhee.kim@samsung.com>
>>> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
>>> ---
>>> .../devicetree/bindings/input/regulator-haptic.txt | 24 ++
>>> drivers/input/misc/Kconfig | 11 +
>>> drivers/input/misc/Makefile | 1 +
>>> drivers/input/misc/regulator-haptic.c | 241 ++++++++++++++++++++
>>> 4 files changed, 277 insertions(+)
>>> create mode 100644 Documentation/devicetree/bindings/input/regulator-haptic.txt
>>> create mode 100644 drivers/input/misc/regulator-haptic.c
>>>
>>> diff --git a/Documentation/devicetree/bindings/input/regulator-haptic.txt b/Documentation/devicetree/bindings/input/regulator-haptic.txt
>>> new file mode 100644
>>> index 0000000..9f60e17
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/input/regulator-haptic.txt
>>> @@ -0,0 +1,24 @@
>>> +* Requlator Haptic Device Tree Bindings
>>> +
>>> +The regulator haptic driver controlled by voltage of regulator.
>>> +This driver implemented via Force Feedback interface.
>>> +
>>> +Required Properties:
>>> + - compatible : Should be "regulator-haptic"
>>> + - haptic-supply : Power supply for the haptic motor.
>>> + [*] refer Documentation/devicetree/bindings/regulator/regulator.txt
>>> +
>>> + - max-microvolt : The maximum voltage value supplied to haptic motor.
>>> + [The unit of the voltage is a micro]
>>> +
>>> + - min-microvolt : The minimum voltage value in which haptic motor reacts.
>>> + [The unit of the voltage is a micro]
>>> +
>>> +Example:
>>> +
>>> + regulator-haptic {
> Should this be more about the function and not the driver name?
> Somehting like
>
> haptics {
Yes, haptics is better than driver name.
>
>>> + compatible = "regulator-haptic";
>>> + haptic-supply = <&motor_regulator>;
>>> + max-microvolt = <2700000>;
>>> + min-microvolt = <1100000>;
>>> + };
>>> diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
>>> index 23297ab..e5e556d 100644
>>> --- a/drivers/input/misc/Kconfig
>>> +++ b/drivers/input/misc/Kconfig
>>> @@ -394,6 +394,17 @@ config INPUT_CM109
>>> To compile this driver as a module, choose M here: the module will be
>>> called cm109.
>>>
>>> +config INPUT_REGULATOR_HAPTIC
>>> + tristate "regulator haptics support"
>>> + select INPUT_FF_MEMLESS
>>> + help
>>> + This option enables device driver support for the haptic controlled
>>> + by regulator. This driver supports ff-memless interface
>>> + from input framework.
>>> +
>>> + To compile this driver as a module, choose M here: the
>>> + module will be called regulator-haptic.
>>> +
>>> config INPUT_RETU_PWRBUTTON
>>> tristate "Retu Power button Driver"
>>> depends on MFD_RETU
>>> diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
>>> index 19c7603..1f135af 100644
>>> --- a/drivers/input/misc/Makefile
>>> +++ b/drivers/input/misc/Makefile
>>> @@ -53,6 +53,7 @@ obj-$(CONFIG_INPUT_PMIC8XXX_PWRKEY) += pmic8xxx-pwrkey.o
>>> obj-$(CONFIG_INPUT_POWERMATE) += powermate.o
>>> obj-$(CONFIG_INPUT_PWM_BEEPER) += pwm-beeper.o
>>> obj-$(CONFIG_INPUT_RB532_BUTTON) += rb532_button.o
>>> +obj-$(CONFIG_INPUT_REGULATOR_HAPTIC) += regulator-haptic.o
>>> obj-$(CONFIG_INPUT_RETU_PWRBUTTON) += retu-pwrbutton.o
>>> obj-$(CONFIG_INPUT_GPIO_ROTARY_ENCODER) += rotary_encoder.o
>>> obj-$(CONFIG_INPUT_SGI_BTNS) += sgi_btns.o
>>> diff --git a/drivers/input/misc/regulator-haptic.c b/drivers/input/misc/regulator-haptic.c
>>> new file mode 100644
>>> index 0000000..1a83ecb
>>> --- /dev/null
>>> +++ b/drivers/input/misc/regulator-haptic.c
>>> @@ -0,0 +1,241 @@
>>> +/*
>>> + * Regulator haptic driver
>>> + *
>>> + * Copyright (c) 2014 Samsung Electronics Co., Ltd.
>>> + * Author: Jaewon Kim <jaewon02.kim@samsung.com>
>>> + * Author: Hyunhee Kim <hyunhee.kim@samsung.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.
>>> + */
>>> +
>>> +#include <linux/module.h>
>>> +#include <linux/platform_device.h>
>>> +#include <linux/input.h>
>>> +#include <linux/slab.h>
>>> +#include <linux/regulator/consumer.h>
>>> +#include <linux/of.h>
>>> +
>>> +#define MAX_MAGNITUDE_SHIFT 16
>>> +
>>> +struct regulator_haptic {
>>> + struct device *dev;
>>> + struct input_dev *input_dev;
>>> + struct regulator *regulator;
>>> + struct work_struct work;
>>> +
>>> + bool enabled;
>>> + bool suspend_state;
> I don't understand what you are tracking here. You only set it and unset
> this value but never do any checking
>
>>> + unsigned int max_volt;
>>> + unsigned int min_volt;
>>> + unsigned int intensity;
>>> + unsigned int magnitude;
>>> +};
>>> +
>>> +static void regulator_haptic_enable(struct regulator_haptic *haptic)
>>> +{
>>> + int error;
>>> +
>>> + if (haptic->enabled)
>>> + return;
>>> +
>>> + error = regulator_enable(haptic->regulator);
>>> + if (error) {
>>> + dev_err(haptic->dev, "cannot enable regulator\n");
>>> + return;
>>> + }
>>> +
>>> + haptic->enabled = true;
>>> +}
>>> +
>>> +static void regulator_haptic_disable(struct regulator_haptic *haptic)
>>> +{
>>> + int error;
>>> +
>>> + if (!haptic->enabled)
>>> + return;
>>> +
>>> + error = regulator_disable(haptic->regulator);
>>> + if (error) {
>>> + dev_err(haptic->dev, "cannot disable regulator\n");
>>> + return;
>>> + }
>>> +
>>> + haptic->enabled = false;
>>> +}
> Can the enable/disable just be one function with an argument?
> Then you can centralize the check and reduce the code footprint.
Ok. I will merge two function with a boolean parameter.
>
>>> +
>>> +static void regulator_haptic_work(struct work_struct *work)
>>> +{
>>> + struct regulator_haptic *haptic = container_of(work,
>>> + struct regulator_haptic, work);
>>> + int error;
>>> +
>>> + error = regulator_set_voltage(haptic->regulator,
>>> + haptic->intensity + haptic->min_volt, haptic->max_volt);
>>> + if (error) {
>>> + dev_err(haptic->dev, "cannot set regulator voltage\n");
>>> + return;
>>> + }
>>> +
>>> + if (haptic->magnitude)
>>> + regulator_haptic_enable(haptic);
>>> + else
>>> + regulator_haptic_disable(haptic);
>>> +}
>>> +
>>> +static int regulator_haptic_play_effect(struct input_dev *input, void *data,
>>> + struct ff_effect *effect)
>>> +{
>>> + struct regulator_haptic *haptic = input_get_drvdata(input);
>>> + u64 volt_mag_multi;
>>> +
>>> + haptic->magnitude = effect->u.rumble.strong_magnitude;
>>> + if (!haptic->magnitude)
>>> + haptic->magnitude = effect->u.rumble.weak_magnitude;
>>> +
>>> +
>>> + volt_mag_multi = (u64)(haptic->max_volt - haptic->min_volt) *
>>> + haptic->magnitude;
>>> + haptic->intensity = (unsigned int)(volt_mag_multi >>
>>> + MAX_MAGNITUDE_SHIFT);
>>> +
>>> + schedule_work(&haptic->work);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static void regulator_haptic_close(struct input_dev *input)
>>> +{
>>> + struct regulator_haptic *haptic = input_get_drvdata(input);
>>> +
>>> + cancel_work_sync(&haptic->work);
>>> + regulator_haptic_disable(haptic);
>>> +}
>>> +
>>> +static int regulator_haptic_parse_dt(struct regulator_haptic *haptic)
>>> +{
>>> + struct device_node *node = haptic->dev->of_node;
>>> + int error;
>>> +
>>> + error = of_property_read_u32(node, "max-microvolt", &haptic->max_volt);
>>> + if (error) {
>>> + dev_err(haptic->dev, "cannot parse max-microvolt\n");
>>> + return error;
>>> + }
>>> +
>>> + error = of_property_read_u32(node, "min-microvolt", &haptic->min_volt);
>>> + if (error) {
>>> + dev_err(haptic->dev, "cannot parse min-microvolt\n");
>>> + return error;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int regulator_haptic_probe(struct platform_device *pdev)
>>> +{
>>> + struct regulator_haptic *haptic;
>>> + struct input_dev *input_dev;
>>> + int error;
>>> +
>>> + haptic = devm_kzalloc(&pdev->dev, sizeof(*haptic), GFP_KERNEL);
>>> + if (!haptic)
>>> + return -ENOMEM;
>>> +
>>> + haptic->dev = &pdev->dev;
>>> + haptic->enabled = false;
>>> + haptic->suspend_state = false;
>>> + INIT_WORK(&haptic->work, regulator_haptic_work);
>>> +
>>> + error = regulator_haptic_parse_dt(haptic);
> Do you need platform data support for platforms that do not currently support
> dt?
Ok I will support platform data in next version.
>
>>> + if (error) {
>>> + dev_err(&pdev->dev, "failed to parse device tree\n");
>>> + return error;
>>> + }
>>> +
>>> + haptic->regulator = devm_regulator_get(&pdev->dev, "haptic");
>>> + if (IS_ERR(haptic->regulator)) {
>>> + dev_err(&pdev->dev, "failed to get regulator\n");
>>> + return PTR_ERR(haptic->regulator);
>>> + }
>>> +
>>> + input_dev = devm_input_allocate_device(&pdev->dev);
>>> + if (!input_dev)
>>> + return -ENOMEM;
>>> +
>>> + haptic->input_dev = input_dev;
>>> + haptic->input_dev->name = "regulator-haptic";
>>> + haptic->input_dev->dev.parent = &pdev->dev;
>>> + haptic->input_dev->close = regulator_haptic_close;
>>> + input_set_drvdata(haptic->input_dev, haptic);
>>> + input_set_capability(haptic->input_dev, EV_FF, FF_RUMBLE);
>>> +
>>> + error = input_ff_create_memless(input_dev, NULL,
>>> + regulator_haptic_play_effect);
>>> + if (error) {
>>> + dev_err(&pdev->dev, "failed to create force-feedback\n");
>>> + return error;
>>> + }
>>> +
>>> + error = input_register_device(haptic->input_dev);
>>> + if (error) {
>>> + dev_err(&pdev->dev, "failed to register input device\n");
>>> + return error;
>>> + }
>>> +
>>> + platform_set_drvdata(pdev, haptic);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int __maybe_unused regulator_haptic_suspend(struct device *dev)
>>> +{
>>> + struct platform_device *pdev = to_platform_device(dev);
>>> + struct regulator_haptic *haptic = platform_get_drvdata(pdev);
>>> +
>>> + if (haptic->enabled) {
>>> + regulator_haptic_disable(haptic);
>>> + haptic->suspend_state = true;
>>>
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int __maybe_unused regulator_haptic_resume(struct device *dev)
>>> +{
>>> + struct platform_device *pdev = to_platform_device(dev);
>>> + struct regulator_haptic *haptic = platform_get_drvdata(pdev);
>>> +
>>> + if (haptic->enabled) {
>>> + regulator_haptic_enable(haptic);
> We enable the regulator when we do the haptics work if magnitude is > 0.
> And during suspend haptic->enabled may get set to false if it is true
> so wouldn't haptic->enabled always be false here?
You are right, It was my typo. It is "if (haptic->suspend_state)".
My intention is restore past state.
thanks to point error.
>
> Plus do we want to continue the haptic response when coming out of suspend state?
>
>>> + haptic->suspend_state = false;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static SIMPLE_DEV_PM_OPS(regulator_haptic_pm_ops,
>>> + regulator_haptic_suspend, regulator_haptic_resume);
>>> +
>>> +static struct of_device_id regulator_haptic_dt_match[] = {
>>> + { .compatible = "regulator-haptic" },
>>> + {},
>>> +};
>>> +
>>> +static struct platform_driver regulator_haptic_driver = {
>>> + .probe = regulator_haptic_probe,
>>> + .driver = {
>>> + .name = "regulator-haptic",
>>> + .owner = THIS_MODULE,
>> .owner is no more required for drivers using module_platform_driver()
>>
>> Thanks,
>> Pankaj Dubey
>>
>>> + .of_match_table = regulator_haptic_dt_match,
>>> + .pm = ®ulator_haptic_pm_ops,
>>> + },
>>> +};
>>> +module_platform_driver(regulator_haptic_driver);
>>> +
>>> +MODULE_AUTHOR("Jaewon Kim <jaewon02.kim@samsung.com>");
>>> +MODULE_AUTHOR("Hyunhee Kim <hyunhee.kim@samsung.com>");
>>> +MODULE_ALIAS("platform:regulator-haptic");
>>> +MODULE_DESCRIPTION("Regulator haptic driver");
>>> +MODULE_LICENSE("GPL");
>>> --
>>> 1.7.9.5
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-input" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 1/2] Input: add regulator haptic driver
From: Jaewon Kim @ 2014-11-21 1:22 UTC (permalink / raw)
To: Pankaj Dubey
Cc: Kukjin Kim, Dmitry Torokhov, Chanwoo Choi, linux-kernel,
linux-input, linux-samsung-soc, Hyunhee Kim
In-Reply-To: <CAGcde9HNGDbm8sNHjyJ-PW75o0eaH1w6KtsOsQx6kDjVqxch5Q@mail.gmail.com>
Hi Pankaj,
2014년 11월 20일 23:33에 Pankaj Dubey 이(가) 쓴 글:
> Hi Jaewon,
>
> On 20 November 2014 19:01, Jaewon Kim <jaewon02.kim@samsung.com> wrote:
>> This patch adds support for haptic driver controlled by
>> voltage of regulator. And this driver support for
>> Force Feedback interface from input framework
>>
>> Signed-off-by: Jaewon Kim <jaewon02.kim@samsung.com>
>> Signed-off-by: Hyunhee Kim <hyunhee.kim@samsung.com>
>> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
>> ---
>> .../devicetree/bindings/input/regulator-haptic.txt | 24 ++
>> drivers/input/misc/Kconfig | 11 +
>> drivers/input/misc/Makefile | 1 +
>> drivers/input/misc/regulator-haptic.c | 241 ++++++++++++++++++++
>> 4 files changed, 277 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/input/regulator-haptic.txt
>> create mode 100644 drivers/input/misc/regulator-haptic.c
>>
>> diff --git a/Documentation/devicetree/bindings/input/regulator-haptic.txt b/Documentation/devicetree/bindings/input/regulator-haptic.txt
>> new file mode 100644
>> index 0000000..9f60e17
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/input/regulator-haptic.txt
>> @@ -0,0 +1,24 @@
>> +* Requlator Haptic Device Tree Bindings
>> +
>> +The regulator haptic driver controlled by voltage of regulator.
>> +This driver implemented via Force Feedback interface.
>> +
>> +Required Properties:
>> + - compatible : Should be "regulator-haptic"
>> + - haptic-supply : Power supply for the haptic motor.
>> + [*] refer Documentation/devicetree/bindings/regulator/regulator.txt
>> +
>> + - max-microvolt : The maximum voltage value supplied to haptic motor.
>> + [The unit of the voltage is a micro]
>> +
>> + - min-microvolt : The minimum voltage value in which haptic motor reacts.
>> + [The unit of the voltage is a micro]
>> +
>> +Example:
>> +
>> + regulator-haptic {
>> + compatible = "regulator-haptic";
>> + haptic-supply = <&motor_regulator>;
>> + max-microvolt = <2700000>;
>> + min-microvolt = <1100000>;
>> + };
>> diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
>> index 23297ab..e5e556d 100644
>> --- a/drivers/input/misc/Kconfig
>> +++ b/drivers/input/misc/Kconfig
>> @@ -394,6 +394,17 @@ config INPUT_CM109
>> To compile this driver as a module, choose M here: the module will be
>> called cm109.
>>
>> +config INPUT_REGULATOR_HAPTIC
>> + tristate "regulator haptics support"
>> + select INPUT_FF_MEMLESS
>> + help
>> + This option enables device driver support for the haptic controlled
>> + by regulator. This driver supports ff-memless interface
>> + from input framework.
>> +
>> + To compile this driver as a module, choose M here: the
>> + module will be called regulator-haptic.
>> +
>> config INPUT_RETU_PWRBUTTON
>> tristate "Retu Power button Driver"
>> depends on MFD_RETU
>> diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
>> index 19c7603..1f135af 100644
>> --- a/drivers/input/misc/Makefile
>> +++ b/drivers/input/misc/Makefile
>> @@ -53,6 +53,7 @@ obj-$(CONFIG_INPUT_PMIC8XXX_PWRKEY) += pmic8xxx-pwrkey.o
>> obj-$(CONFIG_INPUT_POWERMATE) += powermate.o
>> obj-$(CONFIG_INPUT_PWM_BEEPER) += pwm-beeper.o
>> obj-$(CONFIG_INPUT_RB532_BUTTON) += rb532_button.o
>> +obj-$(CONFIG_INPUT_REGULATOR_HAPTIC) += regulator-haptic.o
>> obj-$(CONFIG_INPUT_RETU_PWRBUTTON) += retu-pwrbutton.o
>> obj-$(CONFIG_INPUT_GPIO_ROTARY_ENCODER) += rotary_encoder.o
>> obj-$(CONFIG_INPUT_SGI_BTNS) += sgi_btns.o
>> diff --git a/drivers/input/misc/regulator-haptic.c b/drivers/input/misc/regulator-haptic.c
>> new file mode 100644
>> index 0000000..1a83ecb
>> --- /dev/null
>> +++ b/drivers/input/misc/regulator-haptic.c
>> @@ -0,0 +1,241 @@
>> +/*
>> + * Regulator haptic driver
>> + *
>> + * Copyright (c) 2014 Samsung Electronics Co., Ltd.
>> + * Author: Jaewon Kim <jaewon02.kim@samsung.com>
>> + * Author: Hyunhee Kim <hyunhee.kim@samsung.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.
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/input.h>
>> +#include <linux/slab.h>
>> +#include <linux/regulator/consumer.h>
>> +#include <linux/of.h>
>> +
>> +#define MAX_MAGNITUDE_SHIFT 16
>> +
>> +struct regulator_haptic {
>> + struct device *dev;
>> + struct input_dev *input_dev;
>> + struct regulator *regulator;
>> + struct work_struct work;
>> +
>> + bool enabled;
>> + bool suspend_state;
>> + unsigned int max_volt;
>> + unsigned int min_volt;
>> + unsigned int intensity;
>> + unsigned int magnitude;
>> +};
>> +
>> +static void regulator_haptic_enable(struct regulator_haptic *haptic)
>> +{
>> + int error;
>> +
>> + if (haptic->enabled)
>> + return;
>> +
>> + error = regulator_enable(haptic->regulator);
>> + if (error) {
>> + dev_err(haptic->dev, "cannot enable regulator\n");
>> + return;
>> + }
>> +
>> + haptic->enabled = true;
>> +}
>> +
>> +static void regulator_haptic_disable(struct regulator_haptic *haptic)
>> +{
>> + int error;
>> +
>> + if (!haptic->enabled)
>> + return;
>> +
>> + error = regulator_disable(haptic->regulator);
>> + if (error) {
>> + dev_err(haptic->dev, "cannot disable regulator\n");
>> + return;
>> + }
>> +
>> + haptic->enabled = false;
>> +}
>> +
>> +static void regulator_haptic_work(struct work_struct *work)
>> +{
>> + struct regulator_haptic *haptic = container_of(work,
>> + struct regulator_haptic, work);
>> + int error;
>> +
>> + error = regulator_set_voltage(haptic->regulator,
>> + haptic->intensity + haptic->min_volt, haptic->max_volt);
>> + if (error) {
>> + dev_err(haptic->dev, "cannot set regulator voltage\n");
>> + return;
>> + }
>> +
>> + if (haptic->magnitude)
>> + regulator_haptic_enable(haptic);
>> + else
>> + regulator_haptic_disable(haptic);
>> +}
>> +
>> +static int regulator_haptic_play_effect(struct input_dev *input, void *data,
>> + struct ff_effect *effect)
>> +{
>> + struct regulator_haptic *haptic = input_get_drvdata(input);
>> + u64 volt_mag_multi;
>> +
>> + haptic->magnitude = effect->u.rumble.strong_magnitude;
>> + if (!haptic->magnitude)
>> + haptic->magnitude = effect->u.rumble.weak_magnitude;
>> +
>> +
>> + volt_mag_multi = (u64)(haptic->max_volt - haptic->min_volt) *
>> + haptic->magnitude;
>> + haptic->intensity = (unsigned int)(volt_mag_multi >>
>> + MAX_MAGNITUDE_SHIFT);
>> +
>> + schedule_work(&haptic->work);
>> +
>> + return 0;
>> +}
>> +
>> +static void regulator_haptic_close(struct input_dev *input)
>> +{
>> + struct regulator_haptic *haptic = input_get_drvdata(input);
>> +
>> + cancel_work_sync(&haptic->work);
>> + regulator_haptic_disable(haptic);
>> +}
>> +
>> +static int regulator_haptic_parse_dt(struct regulator_haptic *haptic)
>> +{
>> + struct device_node *node = haptic->dev->of_node;
>> + int error;
>> +
>> + error = of_property_read_u32(node, "max-microvolt", &haptic->max_volt);
>> + if (error) {
>> + dev_err(haptic->dev, "cannot parse max-microvolt\n");
>> + return error;
>> + }
>> +
>> + error = of_property_read_u32(node, "min-microvolt", &haptic->min_volt);
>> + if (error) {
>> + dev_err(haptic->dev, "cannot parse min-microvolt\n");
>> + return error;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int regulator_haptic_probe(struct platform_device *pdev)
>> +{
>> + struct regulator_haptic *haptic;
>> + struct input_dev *input_dev;
>> + int error;
>> +
>> + haptic = devm_kzalloc(&pdev->dev, sizeof(*haptic), GFP_KERNEL);
>> + if (!haptic)
>> + return -ENOMEM;
>> +
>> + haptic->dev = &pdev->dev;
>> + haptic->enabled = false;
>> + haptic->suspend_state = false;
>> + INIT_WORK(&haptic->work, regulator_haptic_work);
>> +
>> + error = regulator_haptic_parse_dt(haptic);
>> + if (error) {
>> + dev_err(&pdev->dev, "failed to parse device tree\n");
>> + return error;
>> + }
>> +
>> + haptic->regulator = devm_regulator_get(&pdev->dev, "haptic");
>> + if (IS_ERR(haptic->regulator)) {
>> + dev_err(&pdev->dev, "failed to get regulator\n");
>> + return PTR_ERR(haptic->regulator);
>> + }
>> +
>> + input_dev = devm_input_allocate_device(&pdev->dev);
>> + if (!input_dev)
>> + return -ENOMEM;
>> +
>> + haptic->input_dev = input_dev;
>> + haptic->input_dev->name = "regulator-haptic";
>> + haptic->input_dev->dev.parent = &pdev->dev;
>> + haptic->input_dev->close = regulator_haptic_close;
>> + input_set_drvdata(haptic->input_dev, haptic);
>> + input_set_capability(haptic->input_dev, EV_FF, FF_RUMBLE);
>> +
>> + error = input_ff_create_memless(input_dev, NULL,
>> + regulator_haptic_play_effect);
>> + if (error) {
>> + dev_err(&pdev->dev, "failed to create force-feedback\n");
>> + return error;
>> + }
>> +
>> + error = input_register_device(haptic->input_dev);
>> + if (error) {
>> + dev_err(&pdev->dev, "failed to register input device\n");
>> + return error;
>> + }
>> +
>> + platform_set_drvdata(pdev, haptic);
>> +
>> + return 0;
>> +}
>> +
>> +static int __maybe_unused regulator_haptic_suspend(struct device *dev)
>> +{
>> + struct platform_device *pdev = to_platform_device(dev);
>> + struct regulator_haptic *haptic = platform_get_drvdata(pdev);
>> +
>> + if (haptic->enabled) {
>> + regulator_haptic_disable(haptic);
>> + haptic->suspend_state = true;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int __maybe_unused regulator_haptic_resume(struct device *dev)
>> +{
>> + struct platform_device *pdev = to_platform_device(dev);
>> + struct regulator_haptic *haptic = platform_get_drvdata(pdev);
>> +
>> + if (haptic->enabled) {
>> + regulator_haptic_enable(haptic);
>> + haptic->suspend_state = false;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static SIMPLE_DEV_PM_OPS(regulator_haptic_pm_ops,
>> + regulator_haptic_suspend, regulator_haptic_resume);
>> +
>> +static struct of_device_id regulator_haptic_dt_match[] = {
>> + { .compatible = "regulator-haptic" },
>> + {},
>> +};
>> +
>> +static struct platform_driver regulator_haptic_driver = {
>> + .probe = regulator_haptic_probe,
>> + .driver = {
>> + .name = "regulator-haptic",
>> + .owner = THIS_MODULE,
> .owner is no more required for drivers using module_platform_driver()
>
> Thanks,
> Pankaj Dubey
Thanks to review my patches.
I will remove next version.
>
>> + .of_match_table = regulator_haptic_dt_match,
>> + .pm = ®ulator_haptic_pm_ops,
>> + },
>> +};
>> +module_platform_driver(regulator_haptic_driver);
>> +
>> +MODULE_AUTHOR("Jaewon Kim <jaewon02.kim@samsung.com>");
>> +MODULE_AUTHOR("Hyunhee Kim <hyunhee.kim@samsung.com>");
>> +MODULE_ALIAS("platform:regulator-haptic");
>> +MODULE_DESCRIPTION("Regulator haptic driver");
>> +MODULE_LICENSE("GPL");
>> --
>> 1.7.9.5
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
Regards,
Jaewon Kim
^ permalink raw reply
* RE: [PATCH v1] Input: elants_i2c: Add Elan touchscreen support
From: ELAN 劉嘉駿 @ 2014-11-21 1:16 UTC (permalink / raw)
To: 'Dmitry Torokhov'
Cc: 'Oliver Neukum', linux-kernel, linux-input,
'Vincent Wang', 'Jeff Chuang',
'Benson Leung', 'Benjamin Tissoires',
'Henrik Rydberg', 'David Herrmann', james.chen,
'蔡仁豪'
In-Reply-To: <20141120175158.GB14798@dtor-glaptop>
Hi Dmitry:
> -----Original Message-----
> From: 'Dmitry Torokhov' [mailto:dmitry.torokhov@gmail.com]
> Sent: Friday, November 21, 2014 1:52 AM
> To: ELAN 劉嘉駿
> Cc: 'Oliver Neukum'; linux-kernel@vger.kernel.org; linux-input@vger.kernel.org;
> 'Vincent Wang'; 'Jeff Chuang'; 'Benson Leung'; 'Benjamin Tissoires'; 'Henrik
> Rydberg'; 'David Herrmann'
> Subject: Re: [PATCH v1] Input: elants_i2c: Add Elan touchscreen support
>
> On Thu, Nov 20, 2014 at 08:25:15PM +0800, ELAN 劉嘉駿 wrote:
> >
> > Hi Oliver:
> >
> > -----Original Message-----
> > From: Oliver Neukum [mailto:oneukum@suse.de]
> > Sent: Thursday, November 20, 2014 6:32 PM
> > To: Dmitry Torokhov
> > Cc: scott; linux-kernel@vger.kernel.org; linux-input@vger.kernel.org;
> > Vincent Wang; Jeff Chuang; Benson Leung; Benjamin Tissoires; Henrik
> > Rydberg; David Herrmann
> > Subject: Re: [PATCH v1] Input: elants_i2c: Add Elan touchscreen
> > support
> >
> >
> >
> > > +static int elants_i2c_sw_reset(struct i2c_client *client) {
> > > + const u8 soft_rst_cmd[] = { 0x77, 0x77, 0x77, 0x77 };
> > > + int error;
> > > +
> > > + error = elants_i2c_send(client, soft_rst_cmd,
> > > + sizeof(soft_rst_cmd));
> > > + if (error) {
> > > + dev_err(&client->dev, "software reset failed: %d\n",
> > > error);
> > > + return error;
> > > + }
> > > +
> > > + /*
> > > + * We should wait at least 10 msec (but no more than 40)
> > > before
> > > + * sending fastboot or IAP command to the device.
> > > + */
> > > + msleep(30);
> >
> > If timing is critical in both ways, you should take our guarantee if sufficient
> sleep. I suggest you sleep 11msecs.
> >
> > Sorry, this is our firmware tricky but it should be sleep longer than 30ms.
> Actually 70ms.
> > (Dmitry, I need to modify this delay time to 70ms. Sorry for the changed over
> and over again;
> > because we consider big sensor solution which need to use more ICs
> > and need more longer delay)
> >
>
> Wait, before you said that upper bound is critical, otherwise the device will
> ignore the next command and will not enter IAP mode. So what will happen if
> we increase the wait to 70 msecs and use the driver with smaller sensors?
>
I think the timeout(40ms) was wrong information. (We do have that, but we will no longer use it on Chrome projects)
As we encountered some problem on Chrome project of big sensor solution, and we already have resolved by modify bootcode timeout value.
Precisely speaking, the 40ms bootcode is special solution for specific project, and general solution is the 100ms bootcode.
So we have replaced 40ms bootcode with 100ms bootcode. That’s what our firmware engineer told me.
Thanks.
--
Scott
> Thanks.
>
> --
> Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH] HID: wacom - PAD is independent with pen/touch
From: Ping Cheng @ 2014-11-21 0:31 UTC (permalink / raw)
To: jkosina; +Cc: linux-input, benjamin.tissoires, Ping Cheng
PAD can be on pen interface (Intuos Pro and Cintiq series) or touch
interface (Bamboo PT and Intuos PT series) or its own interface
(Bamboo pen-only and Intuos Pen M/S). We need to mark it independently.
Signed-off-by: Ping Cheng <pingc@wacom.com>
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
drivers/hid/wacom_sys.c | 8 +++++---
drivers/hid/wacom_wac.h | 1 +
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 68b6cd6..eb55316 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -1135,7 +1135,7 @@ static void wacom_clean_inputs(struct wacom *wacom)
input_free_device(wacom->wacom_wac.input);
}
if (wacom->wacom_wac.pad_input) {
- if (wacom->wacom_wac.input_registered)
+ if (wacom->wacom_wac.pad_registered)
input_unregister_device(wacom->wacom_wac.pad_input);
else
input_free_device(wacom->wacom_wac.pad_input);
@@ -1162,6 +1162,7 @@ static int wacom_register_inputs(struct wacom *wacom)
error = input_register_device(input_dev);
if (error)
return error;
+ wacom_wac->input_registered = true;
}
error = wacom_setup_pad_input_capabilities(pad_input_dev, wacom_wac);
@@ -1174,22 +1175,23 @@ static int wacom_register_inputs(struct wacom *wacom)
error = input_register_device(pad_input_dev);
if (error)
goto fail_register_pad_input;
+ wacom_wac->pad_registered = true;
error = wacom_initialize_leds(wacom);
if (error)
goto fail_leds;
}
- wacom_wac->input_registered = true;
-
return 0;
fail_leds:
input_unregister_device(pad_input_dev);
pad_input_dev = NULL;
+ wacom_wac->pad_registered = false;
fail_register_pad_input:
input_unregister_device(input_dev);
wacom_wac->input = NULL;
+ wacom_wac->input_registered = false;
return error;
}
diff --git a/drivers/hid/wacom_wac.h b/drivers/hid/wacom_wac.h
index 0f0b85e..128cbb3 100644
--- a/drivers/hid/wacom_wac.h
+++ b/drivers/hid/wacom_wac.h
@@ -183,6 +183,7 @@ struct wacom_wac {
struct input_dev *input;
struct input_dev *pad_input;
bool input_registered;
+ bool pad_registered;
int pid;
int battery_capacity;
int num_contacts_left;
--
1.9.1
^ permalink raw reply related
* Fix Penguin Penalty 17th October2014 ( mail-archive.com )
From: cessations00624 @ 2014-11-20 20:52 UTC (permalink / raw)
To: linux-input
Dear Sir
Did your website get hit by Google Penguin update on October 17th 2014? What basically is Google Penguin Update? It is actually a code name for Google algorithm which aims at decreasing your websites search engine rankings that violate Google’s guidelines by using black hat SEO techniques to rank your webpage by giving number of spammy links to the page.
We are one of those few SEO companies that can help you avoid penalties from Google Updates like Penguin and Panda. Our clients have survived all the previous and present updates with ease. They have never been hit because we use 100% white hat SEO techniques to rank Webpages. Simple thing that we do to keep websites away from any Penguin or Panda penalties is follow Google guidelines and we give Google users the best answers to their queries.
If you are looking to increase the quality of your websites and to get more targeted traffic or save your websites from these Google penalties email us back with your interest.
We will be glad to serve you and help you grow your business.
Regards
Julia kites
SEO Manager ( TOB )
B7 Green Avenue, Amritsar 143001 Punjab
____________________________
NO CLICK in the subject to STOP EMAILS
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v1] Input: elants_i2c: Add Elan touchscreen support
From: Dmitry Torokhov @ 2014-11-20 20:35 UTC (permalink / raw)
To: Oliver Neukum
Cc: scott, linux-kernel, linux-input, Vincent Wang, Jeff Chuang,
Benson Leung, Benjamin Tissoires, Henrik Rydberg, David Herrmann
In-Reply-To: <1416515255.25223.1.camel@linux-0dmf.site>
On Thu, Nov 20, 2014 at 09:27:35PM +0100, Oliver Neukum wrote:
> On Thu, 2014-11-20 at 09:47 -0800, Dmitry Torokhov wrote:
> > Hi Oliver,
> >
> > On Thu, Nov 20, 2014 at 11:31:30AM +0100, Oliver Neukum wrote:
> > > > +static int elants_i2c_initialize(struct elants_data *ts)
> > > > +{
> > > > + struct i2c_client *client = ts->client;
> > > > + int error, retry_cnt;
> > > > + const u8 hello_packet[] = { 0x55, 0x55, 0x55, 0x55 };
> > > > + const u8 recov_packet[] = { 0x55, 0x55, 0x80, 0x80 };
> > > > + u8 buf[HEADER_SIZE];
> > > > +
> > >
> > > Strictly speaking you should disable preemption here.
> >
> > Umm, why?
>
> You said the upper bound matters. So you need to protect yourself
> against losing too much time by preemption.
We are using i2c so we can't really disable preemption here... I think
we have enough slack, although I would indeed lower the sleep time.
I'll let Scott comment on that.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v1] Input: elants_i2c: Add Elan touchscreen support
From: Oliver Neukum @ 2014-11-20 20:27 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: scott, linux-kernel, linux-input, Vincent Wang, Jeff Chuang,
Benson Leung, Benjamin Tissoires, Henrik Rydberg, David Herrmann
In-Reply-To: <20141120174709.GA14798@dtor-glaptop>
On Thu, 2014-11-20 at 09:47 -0800, Dmitry Torokhov wrote:
> Hi Oliver,
>
> On Thu, Nov 20, 2014 at 11:31:30AM +0100, Oliver Neukum wrote:
> > > +static int elants_i2c_initialize(struct elants_data *ts)
> > > +{
> > > + struct i2c_client *client = ts->client;
> > > + int error, retry_cnt;
> > > + const u8 hello_packet[] = { 0x55, 0x55, 0x55, 0x55 };
> > > + const u8 recov_packet[] = { 0x55, 0x55, 0x80, 0x80 };
> > > + u8 buf[HEADER_SIZE];
> > > +
> >
> > Strictly speaking you should disable preemption here.
>
> Umm, why?
You said the upper bound matters. So you need to protect yourself
against losing too much time by preemption.
Regards
Oliver
^ permalink raw reply
* Re: [PATCH 1/2] Input: synaptics: Use in-kernel tracking for reporting mt data
From: Benjamin Tissoires @ 2014-11-20 19:42 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Benjamin Tissoires, Henrik Rydberg, Hans de Goede, Peter Hutterer,
linux-input, linux-kernel@vger.kernel.org, Daniel Kurtz,
Chung-yih Wang
In-Reply-To: <20141031165136.GB32331@dtor-ws>
On Fri, Oct 31, 2014 at 12:51 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Thu, Oct 30, 2014 at 02:33:06PM -0400, Benjamin Tissoires wrote:
>> The current code tries to consider all states and transitions to properly
>> detect which finger is attached to which slot. The code is quite huge
>> and difficult to read.
>> If the sensor manages to group the touch points but is not reliable in
>> giving tracking ids, we can simply use the kernel tracking method. Note
>> that it is already used by Cr-48 Chromebooks.
>>
>> Incidentaly, this fixes a bug reported by Peter Hutterer:
>> """
>> on the Lenovo T440, run:
>> evemu-record /dev/input/event4 | grep BTN_
>>
>> then put one, two, three, two fingers down
>> when you go from 3 to 2 fingers the driver sends a spurious BTN_TOUCH 0
>> event:
>>
>> E: 0.000000 0001 014a 0001 # EV_KEY / BTN_TOUCH 1
>> E: 0.000000 0001 0145 0001 # EV_KEY / BTN_TOOL_FINGER 1
>> E: 0.770008 0001 0145 0000 # EV_KEY / BTN_TOOL_FINGER 0
>> E: 0.770008 0001 014d 0001 # EV_KEY / BTN_TOOL_DOUBLETAP 1
>> E: 1.924716 0001 014d 0000 # EV_KEY / BTN_TOOL_DOUBLETAP 0
>> E: 1.924716 0001 014e 0001 # EV_KEY / BTN_TOOL_TRIPLETAP 1
>>
>> .. changing from 3 to 2 fingers now
>>
>> E: 3.152641 0001 014a 0000 # EV_KEY / BTN_TOUCH 0
>> E: 3.152641 0001 014d 0001 # EV_KEY / BTN_TOOL_DOUBLETAP 1
>> E: 3.152641 0001 014e 0000 # EV_KEY / BTN_TOOL_TRIPLETAP 0
>> E: 3.176948 0001 014a 0001 # EV_KEY / BTN_TOUCH 1
>>
>> quick look in the kernel shows it's caused by hw.z going to 0 for a packet,
>> so probably a firmware bug. either way, it makes it hard to track BTN_TOUCH
>> as signal that at least one finger is down.
>> """
>>
>> The in-kernel tracking is enough to remove this spurious BTN_TOUCH 0.
>>
>> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
>> ---
>>
>> Hi Dmitry,
>>
>> I started working on that for 2 other bug reports
>> https://bugs.freedesktop.org/show_bug.cgi?id=81278
>> and
>> https://bugs.freedesktop.org/show_bug.cgi?id=76722
>>
>> I thought the cursor jumps could be fixed by the in-kernel tracking, but the
>> tracking needs a little bit more work to filter them out (patches to follow soon).
>>
>> From a user perspective, this patch does not change anything to what the user
>> previously had. It also fixes Peter's bug that's why I decide to send this out
>> by itself (removing ~350 lines of code and fixing bugs is always nice).
>>
>> I think the cursor jump fixes will need more bikeshedding in input-mt.c (I am
>> *really* bad at designing APIs), so I'll send it later as an RFC.
>
> Daniel and Chung-yih were working on the driver so let's see if they
> have a moment...
>
Any news from the chrome team? This is a requirement for fixing the
cursor jumps, and I'd rather have this series in shape before
introducing the changes in input-mt.c.
Cheers,
Benjamin
>>
>> Cheers,
>> Benjamin
>>
>> drivers/input/mouse/synaptics.c | 397 ++++------------------------------------
>> drivers/input/mouse/synaptics.h | 18 +-
>> 2 files changed, 34 insertions(+), 381 deletions(-)
>>
>> diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
>> index 9031a0a..fd89249 100644
>> --- a/drivers/input/mouse/synaptics.c
>> +++ b/drivers/input/mouse/synaptics.c
>> @@ -569,14 +569,6 @@ static void synaptics_pt_create(struct psmouse *psmouse)
>> * Functions to interpret the absolute mode packets
>> ****************************************************************************/
>>
>> -static void synaptics_mt_state_set(struct synaptics_mt_state *state, int count,
>> - int sgm, int agm)
>> -{
>> - state->count = count;
>> - state->sgm = sgm;
>> - state->agm = agm;
>> -}
>> -
>> static void synaptics_parse_agm(const unsigned char buf[],
>> struct synaptics_data *priv,
>> struct synaptics_hw_state *hw)
>> @@ -595,16 +587,13 @@ static void synaptics_parse_agm(const unsigned char buf[],
>> break;
>>
>> case 2:
>> - /* AGM-CONTACT packet: (count, sgm, agm) */
>> - synaptics_mt_state_set(&agm->mt_state, buf[1], buf[2], buf[4]);
>> + /* AGM-CONTACT packet: we are only interested in the count */
>> + priv->agm_count = buf[1];
>> break;
>>
>> default:
>> break;
>> }
>> -
>> - /* Record that at least one AGM has been received since last SGM */
>> - priv->agm_pending = true;
>> }
>>
>> static bool is_forcepad;
>> @@ -798,388 +787,68 @@ static void synaptics_report_buttons(struct psmouse *psmouse,
>> input_report_key(dev, BTN_0 + i, hw->ext_buttons & (1 << i));
>> }
>>
>> -static void synaptics_report_slot(struct input_dev *dev, int slot,
>> - const struct synaptics_hw_state *hw)
>> -{
>> - input_mt_slot(dev, slot);
>> - input_mt_report_slot_state(dev, MT_TOOL_FINGER, (hw != NULL));
>> - if (!hw)
>> - return;
>> -
>> - input_report_abs(dev, ABS_MT_POSITION_X, hw->x);
>> - input_report_abs(dev, ABS_MT_POSITION_Y, synaptics_invert_y(hw->y));
>> - input_report_abs(dev, ABS_MT_PRESSURE, hw->z);
>> -}
>> -
>> static void synaptics_report_mt_data(struct psmouse *psmouse,
>> - struct synaptics_mt_state *mt_state,
>> - const struct synaptics_hw_state *sgm)
>> + const struct synaptics_hw_state *sgm,
>> + int num_fingers)
>> {
>> struct input_dev *dev = psmouse->dev;
>> struct synaptics_data *priv = psmouse->private;
>> - struct synaptics_hw_state *agm = &priv->agm;
>> - struct synaptics_mt_state *old = &priv->mt_state;
>> + const struct synaptics_hw_state *hw[2] = { sgm, &priv->agm };
>> + struct input_mt_pos pos[2];
>> + int slot[2], nsemi, i;
>>
>> - switch (mt_state->count) {
>> - case 0:
>> - synaptics_report_slot(dev, 0, NULL);
>> - synaptics_report_slot(dev, 1, NULL);
>> - break;
>> - case 1:
>> - if (mt_state->sgm == -1) {
>> - synaptics_report_slot(dev, 0, NULL);
>> - synaptics_report_slot(dev, 1, NULL);
>> - } else if (mt_state->sgm == 0) {
>> - synaptics_report_slot(dev, 0, sgm);
>> - synaptics_report_slot(dev, 1, NULL);
>> - } else {
>> - synaptics_report_slot(dev, 0, NULL);
>> - synaptics_report_slot(dev, 1, sgm);
>> - }
>> - break;
>> - default:
>> - /*
>> - * If the finger slot contained in SGM is valid, and either
>> - * hasn't changed, or is new, or the old SGM has now moved to
>> - * AGM, then report SGM in MTB slot 0.
>> - * Otherwise, empty MTB slot 0.
>> - */
>> - if (mt_state->sgm != -1 &&
>> - (mt_state->sgm == old->sgm ||
>> - old->sgm == -1 || mt_state->agm == old->sgm))
>> - synaptics_report_slot(dev, 0, sgm);
>> - else
>> - synaptics_report_slot(dev, 0, NULL);
>> + nsemi = clamp_val(num_fingers, 0, 2);
>>
>> - /*
>> - * If the finger slot contained in AGM is valid, and either
>> - * hasn't changed, or is new, then report AGM in MTB slot 1.
>> - * Otherwise, empty MTB slot 1.
>> - *
>> - * However, in the case where the AGM is new, make sure that
>> - * that it is either the same as the old SGM, or there was no
>> - * SGM.
>> - *
>> - * Otherwise, if the SGM was just 1, and the new AGM is 2, then
>> - * the new AGM will keep the old SGM's tracking ID, which can
>> - * cause apparent drumroll. This happens if in the following
>> - * valid finger sequence:
>> - *
>> - * Action SGM AGM (MTB slot:Contact)
>> - * 1. Touch contact 0 (0:0)
>> - * 2. Touch contact 1 (0:0, 1:1)
>> - * 3. Lift contact 0 (1:1)
>> - * 4. Touch contacts 2,3 (0:2, 1:3)
>> - *
>> - * In step 4, contact 3, in AGM must not be given the same
>> - * tracking ID as contact 1 had in step 3. To avoid this,
>> - * the first agm with contact 3 is dropped and slot 1 is
>> - * invalidated (tracking ID = -1).
>> - */
>> - if (mt_state->agm != -1 &&
>> - (mt_state->agm == old->agm ||
>> - (old->agm == -1 &&
>> - (old->sgm == -1 || mt_state->agm == old->sgm))))
>> - synaptics_report_slot(dev, 1, agm);
>> - else
>> - synaptics_report_slot(dev, 1, NULL);
>> - break;
>> + for (i = 0; i < nsemi; i++) {
>> + pos[i].x = hw[i]->x;
>> + pos[i].y = synaptics_invert_y(hw[i]->y);
>> }
>>
>> + input_mt_assign_slots(dev, slot, pos, nsemi);
>> +
>> + for (i = 0; i < nsemi; i++) {
>> + input_mt_slot(dev, slot[i]);
>> + input_mt_report_slot_state(dev, MT_TOOL_FINGER, true);
>> + input_report_abs(dev, ABS_MT_POSITION_X, pos[i].x);
>> + input_report_abs(dev, ABS_MT_POSITION_Y, pos[i].y);
>> + input_report_abs(dev, ABS_MT_PRESSURE, hw[i]->z);
>> + }
>> +
>> + input_mt_drop_unused(dev);
>> +
>> /* Don't use active slot count to generate BTN_TOOL events. */
>> input_mt_report_pointer_emulation(dev, false);
>>
>> /* Send the number of fingers reported by touchpad itself. */
>> - input_mt_report_finger_count(dev, mt_state->count);
>> + input_mt_report_finger_count(dev, num_fingers);
>>
>> synaptics_report_buttons(psmouse, sgm);
>>
>> input_sync(dev);
>> }
>>
>> -/* Handle case where mt_state->count = 0 */
>> -static void synaptics_image_sensor_0f(struct synaptics_data *priv,
>> - struct synaptics_mt_state *mt_state)
>> -{
>> - synaptics_mt_state_set(mt_state, 0, -1, -1);
>> - priv->mt_state_lost = false;
>> -}
>> -
>> -/* Handle case where mt_state->count = 1 */
>> -static void synaptics_image_sensor_1f(struct synaptics_data *priv,
>> - struct synaptics_mt_state *mt_state)
>> -{
>> - struct synaptics_hw_state *agm = &priv->agm;
>> - struct synaptics_mt_state *old = &priv->mt_state;
>> -
>> - /*
>> - * If the last AGM was (0,0,0), and there is only one finger left,
>> - * then we absolutely know that SGM contains slot 0, and all other
>> - * fingers have been removed.
>> - */
>> - if (priv->agm_pending && agm->z == 0) {
>> - synaptics_mt_state_set(mt_state, 1, 0, -1);
>> - priv->mt_state_lost = false;
>> - return;
>> - }
>> -
>> - switch (old->count) {
>> - case 0:
>> - synaptics_mt_state_set(mt_state, 1, 0, -1);
>> - break;
>> - case 1:
>> - /*
>> - * If mt_state_lost, then the previous transition was 3->1,
>> - * and SGM now contains either slot 0 or 1, but we don't know
>> - * which. So, we just assume that the SGM now contains slot 1.
>> - *
>> - * If pending AGM and either:
>> - * (a) the previous SGM slot contains slot 0, or
>> - * (b) there was no SGM slot
>> - * then, the SGM now contains slot 1
>> - *
>> - * Case (a) happens with very rapid "drum roll" gestures, where
>> - * slot 0 finger is lifted and a new slot 1 finger touches
>> - * within one reporting interval.
>> - *
>> - * Case (b) happens if initially two or more fingers tap
>> - * briefly, and all but one lift before the end of the first
>> - * reporting interval.
>> - *
>> - * (In both these cases, slot 0 will becomes empty, so SGM
>> - * contains slot 1 with the new finger)
>> - *
>> - * Else, if there was no previous SGM, it now contains slot 0.
>> - *
>> - * Otherwise, SGM still contains the same slot.
>> - */
>> - if (priv->mt_state_lost ||
>> - (priv->agm_pending && old->sgm <= 0))
>> - synaptics_mt_state_set(mt_state, 1, 1, -1);
>> - else if (old->sgm == -1)
>> - synaptics_mt_state_set(mt_state, 1, 0, -1);
>> - break;
>> - case 2:
>> - /*
>> - * If mt_state_lost, we don't know which finger SGM contains.
>> - *
>> - * So, report 1 finger, but with both slots empty.
>> - * We will use slot 1 on subsequent 1->1
>> - */
>> - if (priv->mt_state_lost) {
>> - synaptics_mt_state_set(mt_state, 1, -1, -1);
>> - break;
>> - }
>> - /*
>> - * Since the last AGM was NOT (0,0,0), it was the finger in
>> - * slot 0 that has been removed.
>> - * So, SGM now contains previous AGM's slot, and AGM is now
>> - * empty.
>> - */
>> - synaptics_mt_state_set(mt_state, 1, old->agm, -1);
>> - break;
>> - case 3:
>> - /*
>> - * Since last AGM was not (0,0,0), we don't know which finger
>> - * is left.
>> - *
>> - * So, report 1 finger, but with both slots empty.
>> - * We will use slot 1 on subsequent 1->1
>> - */
>> - synaptics_mt_state_set(mt_state, 1, -1, -1);
>> - priv->mt_state_lost = true;
>> - break;
>> - case 4:
>> - case 5:
>> - /* mt_state was updated by AGM-CONTACT packet */
>> - break;
>> - }
>> -}
>> -
>> -/* Handle case where mt_state->count = 2 */
>> -static void synaptics_image_sensor_2f(struct synaptics_data *priv,
>> - struct synaptics_mt_state *mt_state)
>> -{
>> - struct synaptics_mt_state *old = &priv->mt_state;
>> -
>> - switch (old->count) {
>> - case 0:
>> - synaptics_mt_state_set(mt_state, 2, 0, 1);
>> - break;
>> - case 1:
>> - /*
>> - * If previous SGM contained slot 1 or higher, SGM now contains
>> - * slot 0 (the newly touching finger) and AGM contains SGM's
>> - * previous slot.
>> - *
>> - * Otherwise, SGM still contains slot 0 and AGM now contains
>> - * slot 1.
>> - */
>> - if (old->sgm >= 1)
>> - synaptics_mt_state_set(mt_state, 2, 0, old->sgm);
>> - else
>> - synaptics_mt_state_set(mt_state, 2, 0, 1);
>> - break;
>> - case 2:
>> - /*
>> - * If mt_state_lost, SGM now contains either finger 1 or 2, but
>> - * we don't know which.
>> - * So, we just assume that the SGM contains slot 0 and AGM 1.
>> - */
>> - if (priv->mt_state_lost)
>> - synaptics_mt_state_set(mt_state, 2, 0, 1);
>> - /*
>> - * Otherwise, use the same mt_state, since it either hasn't
>> - * changed, or was updated by a recently received AGM-CONTACT
>> - * packet.
>> - */
>> - break;
>> - case 3:
>> - /*
>> - * 3->2 transitions have two unsolvable problems:
>> - * 1) no indication is given which finger was removed
>> - * 2) no way to tell if agm packet was for finger 3
>> - * before 3->2, or finger 2 after 3->2.
>> - *
>> - * So, report 2 fingers, but empty all slots.
>> - * We will guess slots [0,1] on subsequent 2->2.
>> - */
>> - synaptics_mt_state_set(mt_state, 2, -1, -1);
>> - priv->mt_state_lost = true;
>> - break;
>> - case 4:
>> - case 5:
>> - /* mt_state was updated by AGM-CONTACT packet */
>> - break;
>> - }
>> -}
>> -
>> -/* Handle case where mt_state->count = 3 */
>> -static void synaptics_image_sensor_3f(struct synaptics_data *priv,
>> - struct synaptics_mt_state *mt_state)
>> -{
>> - struct synaptics_mt_state *old = &priv->mt_state;
>> -
>> - switch (old->count) {
>> - case 0:
>> - synaptics_mt_state_set(mt_state, 3, 0, 2);
>> - break;
>> - case 1:
>> - /*
>> - * If previous SGM contained slot 2 or higher, SGM now contains
>> - * slot 0 (one of the newly touching fingers) and AGM contains
>> - * SGM's previous slot.
>> - *
>> - * Otherwise, SGM now contains slot 0 and AGM contains slot 2.
>> - */
>> - if (old->sgm >= 2)
>> - synaptics_mt_state_set(mt_state, 3, 0, old->sgm);
>> - else
>> - synaptics_mt_state_set(mt_state, 3, 0, 2);
>> - break;
>> - case 2:
>> - /*
>> - * If the AGM previously contained slot 3 or higher, then the
>> - * newly touching finger is in the lowest available slot.
>> - *
>> - * If SGM was previously 1 or higher, then the new SGM is
>> - * now slot 0 (with a new finger), otherwise, the new finger
>> - * is now in a hidden slot between 0 and AGM's slot.
>> - *
>> - * In all such cases, the SGM now contains slot 0, and the AGM
>> - * continues to contain the same slot as before.
>> - */
>> - if (old->agm >= 3) {
>> - synaptics_mt_state_set(mt_state, 3, 0, old->agm);
>> - break;
>> - }
>> -
>> - /*
>> - * After some 3->1 and all 3->2 transitions, we lose track
>> - * of which slot is reported by SGM and AGM.
>> - *
>> - * For 2->3 in this state, report 3 fingers, but empty all
>> - * slots, and we will guess (0,2) on a subsequent 0->3.
>> - *
>> - * To userspace, the resulting transition will look like:
>> - * 2:[0,1] -> 3:[-1,-1] -> 3:[0,2]
>> - */
>> - if (priv->mt_state_lost) {
>> - synaptics_mt_state_set(mt_state, 3, -1, -1);
>> - break;
>> - }
>> -
>> - /*
>> - * If the (SGM,AGM) really previously contained slots (0, 1),
>> - * then we cannot know what slot was just reported by the AGM,
>> - * because the 2->3 transition can occur either before or after
>> - * the AGM packet. Thus, this most recent AGM could contain
>> - * either the same old slot 1 or the new slot 2.
>> - * Subsequent AGMs will be reporting slot 2.
>> - *
>> - * To userspace, the resulting transition will look like:
>> - * 2:[0,1] -> 3:[0,-1] -> 3:[0,2]
>> - */
>> - synaptics_mt_state_set(mt_state, 3, 0, -1);
>> - break;
>> - case 3:
>> - /*
>> - * If, for whatever reason, the previous agm was invalid,
>> - * Assume SGM now contains slot 0, AGM now contains slot 2.
>> - */
>> - if (old->agm <= 2)
>> - synaptics_mt_state_set(mt_state, 3, 0, 2);
>> - /*
>> - * mt_state either hasn't changed, or was updated by a recently
>> - * received AGM-CONTACT packet.
>> - */
>> - break;
>> -
>> - case 4:
>> - case 5:
>> - /* mt_state was updated by AGM-CONTACT packet */
>> - break;
>> - }
>> -}
>> -
>> -/* Handle case where mt_state->count = 4, or = 5 */
>> -static void synaptics_image_sensor_45f(struct synaptics_data *priv,
>> - struct synaptics_mt_state *mt_state)
>> -{
>> - /* mt_state was updated correctly by AGM-CONTACT packet */
>> - priv->mt_state_lost = false;
>> -}
>> -
>> static void synaptics_image_sensor_process(struct psmouse *psmouse,
>> struct synaptics_hw_state *sgm)
>> {
>> struct synaptics_data *priv = psmouse->private;
>> - struct synaptics_hw_state *agm = &priv->agm;
>> - struct synaptics_mt_state mt_state;
>> -
>> - /* Initialize using current mt_state (as updated by last agm) */
>> - mt_state = agm->mt_state;
>> + int num_fingers;
>>
>> /*
>> * Update mt_state using the new finger count and current mt_state.
>> */
>> if (sgm->z == 0)
>> - synaptics_image_sensor_0f(priv, &mt_state);
>> + num_fingers = 0;
>> else if (sgm->w >= 4)
>> - synaptics_image_sensor_1f(priv, &mt_state);
>> + num_fingers = 1;
>> else if (sgm->w == 0)
>> - synaptics_image_sensor_2f(priv, &mt_state);
>> - else if (sgm->w == 1 && mt_state.count <= 3)
>> - synaptics_image_sensor_3f(priv, &mt_state);
>> + num_fingers = 2;
>> + else if (sgm->w == 1)
>> + num_fingers = priv->agm_count ? priv->agm_count : 3;
>> else
>> - synaptics_image_sensor_45f(priv, &mt_state);
>> + num_fingers = 4;
>>
>> /* Send resulting input events to user space */
>> - synaptics_report_mt_data(psmouse, &mt_state, sgm);
>> -
>> - /* Store updated mt_state */
>> - priv->mt_state = agm->mt_state = mt_state;
>> - priv->agm_pending = false;
>> + synaptics_report_mt_data(psmouse, sgm, num_fingers);
>> }
>>
>> static void synaptics_profile_sensor_process(struct psmouse *psmouse,
>> @@ -1439,7 +1108,7 @@ static void set_input_params(struct psmouse *psmouse,
>> ABS_MT_POSITION_Y);
>> /* Image sensors can report per-contact pressure */
>> input_set_abs_params(dev, ABS_MT_PRESSURE, 0, 255, 0, 0);
>> - input_mt_init_slots(dev, 2, INPUT_MT_POINTER);
>> + input_mt_init_slots(dev, 2, INPUT_MT_POINTER | INPUT_MT_TRACK);
>>
>> /* Image sensors can signal 4 and 5 finger clicks */
>> __set_bit(BTN_TOOL_QUADTAP, dev->keybit);
>> diff --git a/drivers/input/mouse/synaptics.h b/drivers/input/mouse/synaptics.h
>> index 1bd01f2..6faf9bb 100644
>> --- a/drivers/input/mouse/synaptics.h
>> +++ b/drivers/input/mouse/synaptics.h
>> @@ -119,16 +119,6 @@
>> #define SYN_REDUCED_FILTER_FUZZ 8
>>
>> /*
>> - * A structure to describe which internal touchpad finger slots are being
>> - * reported in raw packets.
>> - */
>> -struct synaptics_mt_state {
>> - int count; /* num fingers being tracked */
>> - int sgm; /* which slot is reported by sgm pkt */
>> - int agm; /* which slot is reported by agm pkt*/
>> -};
>> -
>> -/*
>> * A structure to describe the state of the touchpad hardware (buttons and pad)
>> */
>> struct synaptics_hw_state {
>> @@ -143,9 +133,6 @@ struct synaptics_hw_state {
>> unsigned int down:1;
>> unsigned char ext_buttons;
>> signed char scroll;
>> -
>> - /* As reported in last AGM-CONTACT packets */
>> - struct synaptics_mt_state mt_state;
>> };
>>
>> struct synaptics_data {
>> @@ -170,15 +157,12 @@ struct synaptics_data {
>>
>> struct serio *pt_port; /* Pass-through serio port */
>>
>> - struct synaptics_mt_state mt_state; /* Current mt finger state */
>> - bool mt_state_lost; /* mt_state may be incorrect */
>> -
>> /*
>> * Last received Advanced Gesture Mode (AGM) packet. An AGM packet
>> * contains position data for a second contact, at half resolution.
>> */
>> struct synaptics_hw_state agm;
>> - bool agm_pending; /* new AGM packet received */
>> + unsigned int agm_count; /* finger count reported by agm */
>>
>> /* ForcePad handling */
>> unsigned long press_start;
>> --
>> 2.1.0
>>
>
> --
> Dmitry
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply
* Re: [Question: drivers/input/input.c] Why input_no is initialized to 0 ?
From: Dmitry Torokhov @ 2014-11-20 19:29 UTC (permalink / raw)
To: Aniroop Mathur; +Cc: linux-input@vger.kernel.org
In-Reply-To: <CADYu30_O1CPKyAkQb+dQfQViDurej5Y37AbR-bbWd3TpBg=HKw@mail.gmail.com>
On Thursday, November 20, 2014 02:49:24 AM Aniroop Mathur wrote:
> Hello Mr. Torokhov,
>
> Thank you for your answer.
>
> As it is not a problem now, How about changing it to -1 ?
> Although extra subtraction is not expensive but still we could optimize it
> as we want kernel to optimized as much as possible.
Send patches. I think we have the same pattern in several subsystems.
Thanks.
--
Dmitry
^ 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