Linux Input/HID development
 help / color / mirror / Atom feed
* Re: [PATCH V1] input: use device managed memory in da9052 touchscreen driver
From: Dmitry Torokhov @ 2014-01-10 16:36 UTC (permalink / raw)
  To: Opensource [Anthony Olech]
  Cc: linux-input@vger.kernel.org, linux-kernel@vger.kernel.org,
	Huqiu Liu, David Dajun Chen
In-Reply-To: <24DF37198A1E704D9811D8F72B87EB51AC050B52@NB-EX-MBX02.diasemi.com>

On Fri, Jan 10, 2014 at 09:02:10AM +0000, Opensource [Anthony Olech] wrote:
> > -----Original Message-----
> > From: Dmitry Torokhov [mailto:dmitry.torokhov@gmail.com]
> > 
> > I am uncomfortable with the touchscreen portion of this driver ignoring the
> > framework of it's MFD core and mixing native IRQ management with the
> > ones done through the core.
> > 
> > What would happen if somebody changes da9052_request_irq() to do some
> > thing more than it is doing now so that your open-coded duplicate of the
> > same in da9052_ts_probe() is no longer equivalent? Or
> > da9052_disable_irq() no longer works correctly with IRQs allocated by this
> > sub-module?
> > Thanks.
> > --
> > Dmitry

> Hi Dmitry,
>
> unfortunately the PMIC is a multifunction device and the component
> drivers come under different subsystem maintainers. Thus it is not
> possible to do one patch in one go to change them all.

This could be arranged if you really want to do that. You could post a
patch series and have various maintainers ack it and then Samuel could
take it all through MFD tree.

This is up to you however. The change does not fix any bugs and error
unwinding paths in the input portion of the driver are simple enough, so
I do not see a very strong reason for moving to managed devices. I said
that I would not be opposed to this (given that conversion is solid),
but that was never a request form me.

Thanks.

-- 
Dmitry

^ permalink raw reply

* Re: Re: [PATCH] Introduce Naming Convention in Input Subsystem
From: Dmitry Torokhov @ 2014-01-10 16:31 UTC (permalink / raw)
  To: Aniroop Mathur
  Cc: linux-input@vger.kernel.org, cpgs ., Anurag Aggarwal,
	Naveen Kumar, aniroop.mathur@gmail.com, VIKAS KALA,
	Poorva Srivastava
In-Reply-To: <FB.45.09100.85A10D25@epcpsbgx2.samsung.com>

Hi Aniroop,

On Fri, Jan 10, 2014 at 04:05:44PM +0000, Aniroop Mathur wrote:
> <HTML><HEAD><TITLE>Samsung Enterprise Portal mySingle</TITLE>
> <META content="text/html; charset=windows-1252" http-equiv=Content-Type>
> <STYLE id=mysingle_style type=text/css>P {

Please no HTML mail as mainling lists drop it.

Thanks.

> 	MARGIN-TOP: 5px; FONT-FAMILY: Arial, arial; MARGIN-BOTTOM: 5px; FONT-SIZE: 9pt
> }
> TD {
> 	MARGIN-TOP: 5px; FONT-FAMILY: Arial, arial; MARGIN-BOTTOM: 5px; FONT-SIZE: 9pt
> }
> LI {
> 	MARGIN-TOP: 5px; FONT-FAMILY: Arial, arial; MARGIN-BOTTOM: 5px; FONT-SIZE: 9pt
> }
> BODY {
> 	LINE-HEIGHT: 1.4; MARGIN: 10px; FONT-FAMILY: Arial, arial; FONT-SIZE: 9pt
> }
> </STYLE>
> 
> <META name=GENERATOR content=ActiveSquare></HEAD>
> <BODY>
> <P>Hello Mr. Torokhov,</P>
> <P>Greetings!</P>
> <P><BR>On Thu, Jan 09, 2014 at 10:27:56AM +0530, Aniroop Mathur wrote:<BR>&gt; This patch allows user(driver) to set sysfs node name of input<BR>&gt; devices.&nbsp;&nbsp;To set sysfs node name, user(driver) just needs to set<BR>&gt; node_name_unique variable.&nbsp;&nbsp;If node_name_unique is not set, default<BR>&gt; name is given(as before).&nbsp;&nbsp;So, this patch is completely<BR>&gt; backward-compatible.<BR>&gt; <BR>&gt; Sysfs Input node name format is: input_<NAME><BR>&gt; Sysfs Event node name format is: event_<NAME><BR>&gt;<BR>&gt; This "name" is given by user and automatically, prefix(input and<BR>&gt; event) is added by input core.<BR>&gt; <BR>&gt; This name must be unique among all input devices and driver(user) has<BR>&gt; the responsibility to ensure it.&nbsp;&nbsp;If same 
 name is used again for other<BR>&gt; input device, registration of that input device will fail because two<BR>&gt; input devices cannot have same name.<BR>&gt; <BR>&gt; Advantages of this patch are:<BR>&gt; <BR>&gt; 1. Reduces Booting Time of HAL/Upper-Layer because now HAL or<BR>&gt; Upper-Layer do not need to search input/event number corresponding to<BR>&gt; each input device in /dev/input/...&nbsp;&nbsp;This searching in /dev/input/ was<BR>&gt; taking too much time.&nbsp;&nbsp;(Especially in mobile devices, where there are<BR>&gt; many input devices (many sensors, touchscreen, etc), it reduces a lot<BR>&gt; of booting time)<BR><BR>I am sorry, how much time does it take to scan a directory of what, 20<BR>devices? If it such a factor have udev create nodes that are easier for<BR>you to
  locate, similarly how we already create nodes by-id and by-path.<BR>For example you can encode major:minor in device name.</P>
> <P>&nbsp;</P>
> <P><STRONG><SPAN style="FONT-SIZE: 11pt">Re: (Aniroop Mathur)</SPAN></STRONG></P>
> <P>Its correct that we can set name of a device node using udev. <BR>Yes, this will change the name of device node(/dev/...) but not sysfs node.(/sys/class/input/...)<BR><STRONG>So now, the problem area will shift from dev path to sysfs path, <BR></STRONG>because now we dont know which sysfs node to refer for a particular input device<BR>and hence HAL/Upper-Layer will need to search in /sys/class/input/... instead of /dev/... directory.</P>
> <P>&nbsp;</P>
> <P>Moreover, as i know, udev is mainly for hot-pluggable devices, but my problem is for platform devices,<BR>which are already present on the board during boot up. (Like in Embedded devices)</P>
> <P>&nbsp;</P>
> <P>To avoid confusion and make the problem more clear, <BR>I would like to explain the problem and my suggestion by taking an example:</P>
> <P>&nbsp;</P>
> <P>Suppose in a mobile device, there are 10 embedded input devices as below:<BR><STRONG>Proximity&nbsp;---&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;/dev/input0&nbsp; --- /sys/class/input/input0&nbsp;--- /sys/class/input/event0<BR>Magnetometer&nbsp;---&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;/dev/input1&nbsp;&nbsp; --- /sys/class/input/input1&nbsp;--- /sys/class/input/event1<BR>Accelerometer&nbsp;---&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;/dev/input2&nbsp; --- /sys/class/input/input2&nbsp;--- /sys/class/input/event2<BR>Touchscreen&nbsp;---&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;/dev/input3&nbsp; --- /sys/class/input/input3&nbsp;--- /sys/class/input/event3 </STRONG></P>
> <P><STRONG>... 6 more like this<BR></STRONG>(All these are created during boot up time)</P>
> <P>&nbsp;</P>
> <P>Kernel has created all these nodes, so that HAL/UpperLayer can read or write values from it.<BR>HAL/Upper-Layer needs to do main tasks like:<BR>1. Read raw data - does through /dev/input&lt;num&gt;<BR>2. Enable device - does through sys/class/input&lt;num&gt;/enable<BR>3. Set delay - does through sys/class/input&lt;num&gt;/delay<BR>and many more...</P>
> <P>&nbsp;</P>
> <P>Now, Lets suppose we need to do these tasks for Accelerometer.</P>
> <P>If dev node name is set, HAL can directly read value from it (no search required)<BR><STRONG>But for enabling the accelerometer device or set the delay of a hardware chip, <BR>there is no direct way, HAL can know which input node to refer for accelerometer<BR>because the input number is created dynamically as per device probe order,<BR>so this input number can be anything (0,1,2,3...)<BR></STRONG>So HAL will need to search every input node and read its name attribute<BR>and keep on searching until a match is found between the "attribute name" and "name passed as parameter". <BR>Like for accelerometer, this searching needs to be done for all other input devices.<BR>All of this part is done during booting and this takes a lot for time from booting perspective.</P>
> <P>&nbsp;</P>
> <P><STRONG>As I measured, if there are ten devices, it is taking 1 second to do all this searching. (for all devices)<BR>So for 20 devices, i guess, it could take upto 2 seconds.</STRONG></P>
> <P><STRONG></STRONG>&nbsp;</P>
> <P><STRONG>With naming convention, there is no need of search neither for dev path nor for sysfs path<BR>because HAL directly know which node to refer for which input device<BR>and hence this 1 second is reduced to 10ms or even less, therefore saving 990ms.<BR>I believe, this is a very good time saving. (from device booting perspective)</STRONG></P>
> <P>&nbsp;</P>
> <P>(Is there any direct way, without scanning all nodes for every input device ?)</P>
> <P><BR>&gt; <BR>&gt; 2. Improves Readabilty of input and event sysfs node paths because<BR>&gt; names are used instead of numbers.<BR><BR>I do not see why it is that important. If one wants overview<BR>/proc/bus/input/devices gives nice picture.</P>
> <P>&nbsp;</P>
> <P><STRONG><SPAN style="FONT-SIZE: 11pt">Re: (Aniroop Mathur)</SPAN></STRONG></P>
> <P>Its correct, we can get an overview from /proc/bus/input/devices.<BR>And therefore using this, we can know input node number for every input device.<BR>But there are many input devices and input numbers are not fixed, <BR>so its quite difficult to memorize input number for all input devices.<BR><STRONG>Therefore, if a user needs to open some input node from sysfs path,<BR>he needs to check /proc/bus/input/devices before opening because <BR>he does not know the input number. Moreover, this applies for all other<BR>input devices and hence a user need to check this every time.</STRONG><STRONG><BR></P></STRONG>
> <P>It improves readabilty&nbsp;as below</P>
> <P>&nbsp;</P>
> <TABLE style="BACKGROUND-COLOR: #ffffff; WIDTH: 375px" border=1 cellSpacing=0 width=375>
> <TBODY>
> <TR>
> <TD height=26 width=151>
> <P>&nbsp;Before</P></TD>
> <TD height=26 width=214>
> <P>&nbsp;After Patch</P></TD></TR>
> <TR>
> <TD height=26 width=151>
> <P><STRONG>dev node paths:</STRONG></P>
> <P><STRONG></STRONG>&nbsp;</P>
> <P>/dev/input0</P>
> <P>/dev/input1</P>
> <P>... many more&nbsp;</P></TD>
> <TD height=26 width=214>
> <P><STRONG>dev node paths:</STRONG></P>
> <P><STRONG></STRONG>&nbsp;</P>
> <P>/dev/input_proximity</P>
> <P>/dev/input_accelerometer</P>
> <P>... many more&nbsp;</P></TD></TR>
> <TR>
> <TD height=26 width=151>
> <P><STRONG>sysfs node paths:</STRONG></P>
> <P><STRONG></STRONG>&nbsp;</P>
> <P>/sys/class/input/input0</P>
> <P>/sys/class/input/input1&nbsp;</P>
> <P>... many more</P>
> <P>&nbsp;</P>
> <P>/sys/class/input/event0</P>
> <P>/sys/class/input/event1</P>
> <P>...many more</P></TD>
> <TD height=26 width=214>
> <P><STRONG>sysfs node paths:</STRONG></P>
> <P><STRONG></STRONG>&nbsp;</P>
> <P>/sys/class/input/input_proximity</P>
> <P>/sys/class/input/input_accelerometer</P>
> <P>... many more</P>
> <P>&nbsp;</P>
> <P>/sys/class/input/event_proximity</P>
> <P>/sys/class/input/event_accelerometer</P>
> <P>... many more</P></TD></TR></TBODY></TABLE>
> <P>&nbsp;</P>
> <P><STRONG>So, just by looking, user can directly open or refer any input node.<BR>(no need to refer any other path)</STRONG><STRONG></P>
> <P><BR></STRONG>&gt; <BR>&gt; 3. Removes Input Devices Dependency. If one input device probe fails,<BR>&gt; other input devices still work.&nbsp;&nbsp;Before this patch, if one input<BR>&gt; device probe fails before input_register_device, then input number of<BR>&gt; other input devices changes and due to this permission settings are<BR>&gt; disturbed and hence HAL or upper layer cannot open the required sysfs<BR>&gt; node because permission denied error comes.<BR><BR>I have only one suggestion here: fix your userspace so that does not<BR>depend on device initialization ordering.<BR></P>
> <P><STRONG><SPAN style="FONT-SIZE: 11pt">Re: (Aniroop Mathur)</SPAN></STRONG></P>
> <P><STRONG>We cannot fix userspace because these input/event/dev number are decided/allocated in kernel<BR>as per device initialization ordering during boot up. (userspace has no role in it)<BR></STRONG>So, userspace is not aware, which exact input number corresponds to which input device <BR>so it ends up searching/scanning every input node untill a match is found.</P>
> <P>So, there is input device dependency which needs to be removed.</P>
> <P>&nbsp;</P>
> <P><BR>IOW I am totally unconvinced that this facility is needed.</P>
> <P><STRONG><SPAN style="FONT-SIZE: 11pt">Re: (Aniroop Mathur)</SPAN></STRONG></P>
> <P>I hope my problem and suggestion is more clear and convincing now.</P>
> <P>&nbsp;</P>
> <P>&nbsp;</P>
> <P><STRONG>For reference, copying Patch again:</STRONG></P>
> <P>---<BR>drivers/input/evdev.c |&nbsp;&nbsp; 11 ++++++++++-<BR>drivers/input/input.c |&nbsp;&nbsp; 12 +++++++++++-<BR>include/linux/input.h |&nbsp;&nbsp;&nbsp; 4 ++++<BR>3 files changed, 25 insertions(+), 2 deletions(-)<BR><BR>diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c<BR>index b6ded17..b6a5848 100644<BR>--- a/drivers/input/evdev.c<BR>+++ b/drivers/input/evdev.c<BR>@@ -1131,7 +1131,16 @@ static int evdev_connect(struct input_handler *handler, struct input_dev *dev,<BR>/* Normalize device number if it falls into legacy range */<BR>if (dev_no &lt; EVDEV_MINOR_BASE + EVDEV_MINORS)<BR>dev_no -= EVDEV_MINOR_BASE;<BR>- dev_set_name(&amp;evdev-&gt;dev, "event%d", dev_no);<BR>+<BR>+ /*<BR>+ * As per user choice (driver),<BR>+ * name of sysfs node is set, as mentioned in node_nam
 e_unique variable.<BR>+ * If node_name_unique is not set, default name is given.<BR>+ */<BR>+ if (dev-&gt;node_name_unique)<BR>+ dev_set_name(&amp;evdev-&gt;dev, "event_%s", dev-&gt;node_name_unique);<BR>+ else<BR>+ dev_set_name(&amp;evdev-&gt;dev, "event%d", dev_no);<BR><BR>evdev-&gt;handle.dev = input_get_device(dev);<BR>evdev-&gt;handle.name = dev_name(&amp;evdev-&gt;dev);<BR>diff --git a/drivers/input/input.c b/drivers/input/input.c<BR>index c044699..c8126b3 100644<BR>--- a/drivers/input/input.c<BR>+++ b/drivers/input/input.c<BR>@@ -2073,7 +2073,17 @@ int input_register_device(struct input_dev *dev)<BR>if (!dev-&gt;setkeycode)<BR>dev-&gt;setkeycode = input_default_setkeycode;<BR><BR>- dev_set_name(&amp;dev-&gt;dev, "input%ld",<BR>+ /*<BR>+ * As per user choice (driver),<BR>+ * name o
 f sysfs node is set, as mentioned in node_name_unique variable.<BR>+ * If node_name_unique is not set, default name is given.<BR>+ */<BR>+ if (dev-&gt;node_name_unique) {<BR>+ atomic_inc_return(&amp;input_no);<BR>+ dev_set_name(&amp;dev-&gt;dev, "input_%s",<BR>+ &nbsp;&nbsp;&nbsp;&nbsp; dev-&gt;node_name_unique);<BR>+ } else<BR>+ dev_set_name(&amp;dev-&gt;dev, "input%ld",<BR>&nbsp;&nbsp;&nbsp;&nbsp; (unsigned long) atomic_inc_return(&amp;input_no) - 1);<BR><BR>error = device_add(&amp;dev-&gt;dev);<BR>diff --git a/include/linux/input.h b/include/linux/input.h<BR>index 82ce323..fe44643 100644<BR>--- a/include/linux/input.h<BR>+++ b/include/linux/input.h<BR>@@ -43,6 +43,9 @@ struct input_value {<BR>&nbsp; * @uniq: unique identification code for the device (if device has it)<BR>&nbsp; * @id:
  id of the device (struct input_id)<BR>&nbsp; * @propbit: bitmap of device properties and quirks<BR>+ * @node_name_unique: name of input and event sysfs device node (char *).<BR>+ * This name must be unique among all input devices and driver(user)<BR>+ * has the responsibility to ensure it (if using).<BR>&nbsp; * @evbit: bitmap of types of events supported by the device (EV_KEY,<BR>&nbsp; * EV_REL, etc.)<BR>&nbsp; * @keybit: bitmap of keys/buttons this device has<BR>@@ -123,6 +126,7 @@ struct input_dev {<BR>const char *phys;<BR>const char *uniq;<BR>struct input_id id;<BR>+ char *node_name_unique;<BR><BR>unsigned long propbit[BITS_TO_LONGS(INPUT_PROP_CNT)];<BR><BR>-- <BR>1.7.0.4<BR><BR></P>
> <P>Thanks,</P>
> <P>Aniroop Mathur</P>
> <P>&nbsp;</P>
> <TABLE id=confidentialsignimg>
> <TBODY>
> <TR>
> <TD NAMO_LOCK>
> <P><IMG border=0 src="cid:QKNMBDIFBEI0@namo.co.kr" width=520></P></TD></TR></TBODY></TABLE></BODY></HTML><img src='http://ext.samsung.net/mailcheck/SeenTimeChecker?do=166e1c9f408a9352202b65e5c362a372ac21ff698029c65acf20909b63ed3e8c4e627947a285d7d76b91358bc02b4ef9c7b41e955949e5c8a728c55b39cc59eacf878f9a26ce15a0' border=0 width=0 height=0 style='display:none'>


-- 
Dmitry

^ permalink raw reply

* DiePresse.com Mein Freund, der Fahrradhändler
From: franklinbuthelezi1 @ 2014-01-10 12:33 UTC (permalink / raw)
  To: linux-input

Folgender Artikel auf http://diepresse.com wird Ihnen von Franklin Buthelezi empfohlen:

Mein Freund, der Fahrradhändler
Manche Dinge verändern sich über die langen Wintermonate hinweg. 

http://diepresse.com/home/leben/mode/kolumnezumtag/548717/index.do

Notiz:
Dear friend,
 
I am the international operations manager of ABSA bank South Africa. My name is Franklin Buthelezi. I have a sensitive and private offer from the top executive to seek your partnership in re-profiling some offshore investment funds worth 11.5M U.S.D (Eleven Million five hundred thousand United States Dollars) I am constrained however to withhold most of the details for now. This is a legitimate transaction. 
 
If we agree on the terms you will be given 15% of the total funds, if you are interested please write back via email providing me your personal details and phone number for further directives.
 
If you are interested and willing to render your assistance please respond via my private email address stated below.
 
I look forward to your response.
 
Best regards
Franklin Buthelezi
Email: franklinbuthelezi1@yahoo.com


------------------------------------
DiePresse.com übernimmt keine Haftung für den Inhalt der persönlichen Nachricht.

© DiePresse.com - http://diepresse.com

--
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: use device managed memory in da9052 touchscreen driver
From: Opensource [Anthony Olech] @ 2014-01-10  9:02 UTC (permalink / raw)
  To: Dmitry Torokhov, Opensource [Anthony Olech]
  Cc: linux-input@vger.kernel.org, linux-kernel@vger.kernel.org,
	Huqiu Liu, David Dajun Chen
In-Reply-To: <20140109191236.GA28020@core.coreip.homeip.net>

> -----Original Message-----
> From: Dmitry Torokhov [mailto:dmitry.torokhov@gmail.com]
> Sent: 09 January 2014 19:13
> To: Opensource [Anthony Olech]
> Cc: linux-input@vger.kernel.org; linux-kernel@vger.kernel.org; Huqiu Liu;
> David Dajun Chen
> Subject: Re: [PATCH V1] input: use device managed memory in da9052
> touchscreen driver
> 
> Hi Anthony,
> 
> On Thu, Jan 09, 2014 at 12:51:37PM +0000, Anthony Olech wrote:
> > The touchscreen component driver for the da9052/3 Dialog PMICs is
> > changed to use device managed memory allocation.
> >
> > This results in simpler error paths as failures in the probe()
> > function do not require explicit calls to free the devm_...
> > allocated memory.
> > The allocation functions used in this driver are:
> >     devm_kzalloc()
> >     devm_input_allocate_device()
> >     devm_request_threaded_irq()
> >
> > Suggested-by: Huqiu Liu <liuhq11@mails.tsinghua.edu.cn>
> > Signed-off-by: Anthony Olech <anthony.olech.opensource@diasemi.com>
> > ---
> > This patch is relative to linux-next repository tag next-20140109
> >
> > Many thanks to Huqiu Liu who instigated this patch.
> >
> >  drivers/input/touchscreen/da9052_tsi.c |   62 ++++++++++++++++-----------
> -----
> >  1 file changed, 30 insertions(+), 32 deletions(-)
> >
> > diff --git a/drivers/input/touchscreen/da9052_tsi.c
> > b/drivers/input/touchscreen/da9052_tsi.c
> > index ab64d58..dcc4cf1 100644
> > --- a/drivers/input/touchscreen/da9052_tsi.c
> > +++ b/drivers/input/touchscreen/da9052_tsi.c
> > @@ -233,18 +233,30 @@ static int da9052_ts_probe(struct
> platform_device *pdev)
> >  	struct da9052_tsi *tsi;
> >  	struct input_dev *input_dev;
> >  	int error;
> > +	int pdown_irq;
> > +	int ready_irq;
> >
> >  	da9052 = dev_get_drvdata(pdev->dev.parent);
> >  	if (!da9052)
> >  		return -EINVAL;
> >
> > -	tsi = kzalloc(sizeof(struct da9052_tsi), GFP_KERNEL);
> > -	input_dev = input_allocate_device();
> > -	if (!tsi || !input_dev) {
> > -		error = -ENOMEM;
> > -		goto err_free_mem;
> > -	}
> > +	pdown_irq = regmap_irq_get_virq(da9052->irq_data,
> DA9052_IRQ_PENDOWN);
> > +	if (pdown_irq < 0)
> > +		return pdown_irq;
> ...
> >
> > -	error = da9052_request_irq(tsi->da9052, DA9052_IRQ_PENDOWN,
> > -				"pendown-irq", da9052_ts_pendwn_irq, tsi);
> > +	error = devm_request_threaded_irq(&pdev->dev, pdown_irq,
> > +				NULL, da9052_ts_pendwn_irq,
> > +				IRQF_TRIGGER_LOW | IRQF_ONESHOT,
> > +				"pendown-irq", tsi);
> 
> I am uncomfortable with the touchscreen portion of this driver ignoring the
> framework of it's MFD core and mixing native IRQ management with the
> ones done through the core.
> 
> What would happen if somebody changes da9052_request_irq() to do some
> thing more than it is doing now so that your open-coded duplicate of the
> same in da9052_ts_probe() is no longer equivalent? Or
> da9052_disable_irq() no longer works correctly with IRQs allocated by this
> sub-module?
> Thanks.
> --
> Dmitry
Hi Dmitry,
unfortunately the PMIC is a multifunction device and the component drivers come
under different subsystem maintainers. Thus it is not possible to do one patch in one
go to change them all.
Tony Olech



^ permalink raw reply

* Re: [PATCHv6] staging/iio/adc: change the MXS touchscreen driver implementation
From: Jürgen Beisert @ 2014-01-10  8:55 UTC (permalink / raw)
  To: Alexandre Belloni
  Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b, marex-ynQEQJNshbs,
	fabio.estevam-KZfg59tc24xl57MIdRCFDg,
	jic23-KWPb1pKIrIJaa/9Udqfwiw, linux-input-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <52CEA4AA.8050503-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>

Hi Alexandre,

On Thursday 09 January 2014 14:31:22 Alexandre Belloni wrote:
> Sorry to chime in only now but it seems that this series is breaking the
> touchscreen calibration on 3.13 (and -rc7 is out so it might be too
> late).
>
> At first, I though I became a terrible clicker ;) but I found some
> evidences:
>
> xinput_calibrator is complaining about misclicks, debug output:
> DEBUG: Adding click 0 (X=105, Y=59)
> DEBUG: Not adding click 1 (X=100, Y=59): within 7 pixels of previous click
> DEBUG: Adding click 1 (X=696, Y=58)
> DEBUG: Not adding click 2 (X=700, Y=55): within 7 pixels of previous click
> DEBUG: Adding click 2 (X=103, Y=422)
> DEBUG: Mis-click detected, click 3 (X=438, Y=415) not aligned with click 1 (X=696, Y=58) or click 2 (X=103, Y=422) (threshold=15)
> DEBUG: Adding click 0 (X=424, Y=411)
>
> Do you see the confusion ? At some point one of the coordinates is not
> updated. Successful calibration with 3.12.6 gives:
> DEBUG: Adding click 0 (X=126, Y=405)
> DEBUG: Adding click 1 (X=684, Y=399)
> DEBUG: Adding click 2 (X=128, Y=77)
> DEBUG: Adding click 3 (X=683, Y=77)
>
>
> With ts_calibrate:
> xres = 800, yres = 480
> Took 1 samples...
> Top left : X =  435 Y = 3572
> Took 2 samples...
> Top right : X = 2094 Y = 3553
> Took 2 samples...
> Bot right : X = 2939 Y = 2077
> Took 2 samples...
> Bot left : X = 2060 Y =  644
> Took 2 samples...
> Center : X = 1279 Y = 1346
>
> We experience the same thing, when changing position on an axis, we get
> an average between the previous and the new position (probably because
> we got 2 samples):
>  - Top Left is ok
>  - Top right is almost ok: X should be around 3500/3700
>  - Bot right is starting to get really wrong: X is getting better (it
>    should still be around 3500/3700) but Y should be quite lower
>  - Bot left: Y is ok but X should be lower
>  - Center is completely wrong, both values should be around 2000
>
> Last test with evtest which is always difficult because it outputs a lot
> of debug. Two separate touchs, bottom left and then top right (note that
> this is with fsl,ave-ctrl = <8>;):
>
> Event: time 1389210862.451000, type 3 (Absolute), code 0 (X), value 2183
> Event: time 1389210862.451000, type 3 (Absolute), code 1 (Y), value 720
> Event: time 1389210862.451000, type 3 (Absolute), code 24 (Pressure), value 6
> Event: time 1389210862.451000, type 1 (Key), code 330 (Touch), value 1 
> Event: time 1389210862.451000, -------------- Report Sync ------------
> Event: time 1389210862.477721, type 3 (Absolute), code 0 (X), value 448
> Event: time 1389210862.477721, type 3 (Absolute), code 1 (Y), value 667
> Event: time 1389210862.477721, type 3 (Absolute), code 24 (Pressure), value 595
> Event: time 1389210862.477721, -------------- Report Sync ------------ 
> Event: time 1389210862.504718, type 3 (Absolute), code 0 (X), value 434
> Event: time 1389210862.504718, type 3 (Absolute), code 1 (Y), value 704
> Event: time 1389210862.504718, type 3 (Absolute), code 24 (Pressure), value 580
> Event: time 1389210862.504718, -------------- Report Sync ------------ 
> Event: time 1389210862.536688, type 1 (Key), code 330 (Touch), value 0
> Event: time 1389210862.536688, -------------- Report Sync ------------
> Event: time 1389210863.359697, type 3 (Absolute), code 0 (X), value 432
> Event: time 1389210863.359697, type 3 (Absolute), code 1 (Y), value 726
> Event: time 1389210863.359697, type 3 (Absolute), code 24 (Pressure), value 210
> Event: time 1389210863.359697, type 1 (Key), code 330 (Touch), value 1 
> Event: time 1389210863.359697, -------------- Report Sync ------------
> Event: time 1389210863.391687, type 1 (Key), code 330 (Touch), value 0
> Event: time 1389210863.391687, -------------- Report Sync ------------
>
> We get 3 reports for bottom left, then pen up, then one report for top
> right that is almost exactly the same a bottom left !
>
> Output from 3.12:
> Event: time 1389210022.146809, type 3 (Absolute), code 0 (X), value 286
> Event: time 1389210022.146809, type 3 (Absolute), code 1 (Y), value 504
> Event: time 1389210022.146809, type 3 (Absolute), code 24 (Pressure), value 3045
> Event: time 1389210022.146809, type 1 (Key), code 330 (Touch), value 1 
> Event: time 1389210022.146809, -------------- Report Sync ------------
> Event: time 1389210022.166830, type 3 (Absolute), code 0 (X), value 256
> Event: time 1389210022.166830, type 3 (Absolute), code 1 (Y), value 489
> Event: time 1389210022.166830, type 3 (Absolute), code 24 (Pressure), value 3033
> Event: time 1389210022.166830, -------------- Report Sync ------------ 
> Event: time 1389210022.186812, type 3 (Absolute), code 24 (Pressure), value 0
> Event: time 1389210022.186812, type 1 (Key), code 330 (Touch), value 0 
> Event: time 1389210022.186812, -------------- Report Sync ------------
> Event: time 1389210025.196808, type 3 (Absolute), code 0 (X), value 3812
> Event: time 1389210025.196808, type 3 (Absolute), code 1 (Y), value 3637
> Event: time 1389210025.196808, type 3 (Absolute), code 24 (Pressure), value 4032
> Event: time 1389210025.196808, type 1 (Key), code 330 (Touch), value 1 
> Event: time 1389210025.196808, -------------- Report Sync ------------
> Event: time 1389210025.216819, type 3 (Absolute), code 24 (Pressure), value 0
> Event: time 1389210025.216819, type 1 (Key), code 330 (Touch), value 0 
> Event: time 1389210025.216819, -------------- Report Sync ------------
>
>
> While this is not necessarily an issue with ts_calibrate (it is possible
> to click longer so it collects more samples), I don't see that working
> with xinput_calibrator.
>
> While I don't have much experience with the TS part of the code but I
> can investigate if you don't have any idea.

You are right. I have seen the same behaviour here a few times. Currently I'm
short in time to dig deeper into this issue, but I will try to do so.

Juergen

-- 
Pengutronix e.K.                              | Juergen Beisert             |
Linux Solutions for Science and Industry      | http://www.pengutronix.de/  |

^ permalink raw reply

* [PATCH 1/4] Input: synaptics-rmi4 - split of transport ops into a separate structure
From: Dmitry Torokhov @ 2014-01-10  7:44 UTC (permalink / raw)
  To: Christopher Heiny
  Cc: Andrew Duggan, Vincent Huang, Vivian Ly, Daniel Rosenberg,
	Linus Walleij, Benjamin Tissoires, Linux Input, Linux Kernel

Split off transport operations from rmi_transport_dev into a separate
structure that will be shared between all devices using the same transport
and use const pointer to access it.

Change signature on transport methods so that length is using the proper
tyep - size_t.

Also rename rmi_transport_info to rmi_transport_stats and move protocol
name (which is the only immutable piece of data there) into the transport
device itself.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/rmi4/rmi_bus.h    | 64 ++++++++++++++++++++++++-----------------
 drivers/input/rmi4/rmi_driver.c |  8 +++---
 drivers/input/rmi4/rmi_i2c.c    | 49 ++++++++++++++++---------------
 3 files changed, 67 insertions(+), 54 deletions(-)

diff --git a/drivers/input/rmi4/rmi_bus.h b/drivers/input/rmi4/rmi_bus.h
index 3e8b57a..ccf26dc 100644
--- a/drivers/input/rmi4/rmi_bus.h
+++ b/drivers/input/rmi4/rmi_bus.h
@@ -135,26 +135,25 @@ struct rmi_driver {
 #define to_rmi_driver(d) \
 	container_of(d, struct rmi_driver, driver);
 
-/** struct rmi_transport_info - diagnostic information about the RMI transport
+/**
+ * struct rmi_transport_stats - diagnostic information about the RMI transport
  * device, used in the xport_info debugfs file.
  *
  * @proto String indicating the protocol being used.
  * @tx_count Number of transmit operations.
- * @tx_bytes Number of bytes transmitted.
  * @tx_errs  Number of errors encountered during transmit operations.
+ * @tx_bytes Number of bytes transmitted.
  * @rx_count Number of receive operations.
- * @rx_bytes Number of bytes received.
  * @rx_errs  Number of errors encountered during receive operations.
- * @att_count Number of times ATTN assertions have been handled.
+ * @rx_bytes Number of bytes received.
  */
-struct rmi_transport_info {
-	const char *proto;
-	long tx_count;
-	long tx_bytes;
-	long tx_errs;
-	long rx_count;
-	long rx_bytes;
-	long rx_errs;
+struct rmi_transport_stats {
+	unsigned long tx_count;
+	unsigned long tx_errs;
+	size_t tx_bytes;
+	unsigned long rx_count;
+	unsigned long rx_errs;
+	size_t rx_bytes;
 };
 
 /**
@@ -162,13 +161,14 @@ struct rmi_transport_info {
  *
  * @dev: Pointer to the communication device, e.g. i2c or spi
  * @rmi_dev: Pointer to the RMI device
- * @write_block: Writing a block of data to the specified address
- * @read_block: Read a block of data from the specified address.
  * @irq_thread: if not NULL, the sensor driver will use this instead of the
  * default irq_thread implementation.
  * @hard_irq: if not NULL, the sensor driver will use this for the hard IRQ
  * handling
  * @data: Private data pointer
+ * @proto_name: name of the transport protocol (SPI, i2c, etc)
+ * @ops: pointer to transport operations implementation
+ * @stats: transport statistics
  *
  * The RMI transport device implements the glue between different communication
  * buses such as I2C and SPI.
@@ -178,20 +178,30 @@ struct rmi_transport_dev {
 	struct device *dev;
 	struct rmi_device *rmi_dev;
 
-	int (*write_block)(struct rmi_transport_dev *xport, u16 addr,
-			   const void *buf, const int len);
-	int (*read_block)(struct rmi_transport_dev *xport, u16 addr,
-			  void *buf, const int len);
-
-	int (*enable_device) (struct rmi_transport_dev *xport);
-	void (*disable_device) (struct rmi_transport_dev *xport);
-
 	irqreturn_t (*irq_thread)(int irq, void *p);
 	irqreturn_t (*hard_irq)(int irq, void *p);
 
 	void *data;
 
-	struct rmi_transport_info info;
+	const char *proto_name;
+	const struct rmi_transport_ops *ops;
+	struct rmi_transport_stats stats;
+};
+
+/**
+ * struct rmi_transport_ops - defines transport protocol operations.
+ *
+ * @write_block: Writing a block of data to the specified address
+ * @read_block: Read a block of data from the specified address.
+ */
+struct rmi_transport_ops {
+	int (*write_block)(struct rmi_transport_dev *xport, u16 addr,
+			   const void *buf, size_t len);
+	int (*read_block)(struct rmi_transport_dev *xport, u16 addr,
+			  void *buf, size_t len);
+
+	int (*enable_device) (struct rmi_transport_dev *xport);
+	void (*disable_device) (struct rmi_transport_dev *xport);
 };
 
 /**
@@ -232,7 +242,7 @@ bool rmi_is_physical_device(struct device *dev);
  */
 static inline int rmi_read(struct rmi_device *d, u16 addr, void *buf)
 {
-	return d->xport->read_block(d->xport, addr, buf, 1);
+	return d->xport->ops->read_block(d->xport, addr, buf, 1);
 }
 
 /**
@@ -248,7 +258,7 @@ static inline int rmi_read(struct rmi_device *d, u16 addr, void *buf)
 static inline int rmi_read_block(struct rmi_device *d, u16 addr, void *buf,
 				 const int len)
 {
-	return d->xport->read_block(d->xport, addr, buf, len);
+	return d->xport->ops->read_block(d->xport, addr, buf, len);
 }
 
 /**
@@ -262,7 +272,7 @@ static inline int rmi_read_block(struct rmi_device *d, u16 addr, void *buf,
  */
 static inline int rmi_write(struct rmi_device *d, u16 addr, const u8 data)
 {
-	return d->xport->write_block(d->xport, addr, &data, 1);
+	return d->xport->ops->write_block(d->xport, addr, &data, 1);
 }
 
 /**
@@ -278,7 +288,7 @@ static inline int rmi_write(struct rmi_device *d, u16 addr, const u8 data)
 static inline int rmi_write_block(struct rmi_device *d, u16 addr,
 				  const void *buf, const int len)
 {
-	return d->xport->write_block(d->xport, addr, buf, len);
+	return d->xport->ops->write_block(d->xport, addr, buf, len);
 }
 
 int rmi_register_transport_device(struct rmi_transport_dev *xport);
diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
index eb790ff..3483e5b 100644
--- a/drivers/input/rmi4/rmi_driver.c
+++ b/drivers/input/rmi4/rmi_driver.c
@@ -126,8 +126,8 @@ static void disable_sensor(struct rmi_device *rmi_dev)
 	if (!data->irq)
 		disable_polling(rmi_dev);
 
-	if (rmi_dev->xport->disable_device)
-		rmi_dev->xport->disable_device(rmi_dev->xport);
+	if (rmi_dev->xport->ops->disable_device)
+		rmi_dev->xport->ops->disable_device(rmi_dev->xport);
 
 	if (data->irq) {
 		disable_irq(data->irq);
@@ -146,8 +146,8 @@ static int enable_sensor(struct rmi_device *rmi_dev)
 	if (data->enabled)
 		return 0;
 
-	if (rmi_dev->xport->enable_device) {
-		retval = rmi_dev->xport->enable_device(rmi_dev->xport);
+	if (rmi_dev->xport->ops->enable_device) {
+		retval = rmi_dev->xport->ops->enable_device(rmi_dev->xport);
 		if (retval)
 			return retval;
 	}
diff --git a/drivers/input/rmi4/rmi_i2c.c b/drivers/input/rmi4/rmi_i2c.c
index 12aea8c..40badf3 100644
--- a/drivers/input/rmi4/rmi_i2c.c
+++ b/drivers/input/rmi4/rmi_i2c.c
@@ -61,11 +61,11 @@ static int rmi_set_page(struct rmi_transport_dev *xport, u8 page)
 
 	dev_dbg(&client->dev, "writes 3 bytes: %02x %02x\n",
 			txbuf[0], txbuf[1]);
-	xport->info.tx_count++;
-	xport->info.tx_bytes += sizeof(txbuf);
+	xport->stats.tx_count++;
+	xport->stats.tx_bytes += sizeof(txbuf);
 	retval = i2c_master_send(client, txbuf, sizeof(txbuf));
 	if (retval != sizeof(txbuf)) {
-		xport->info.tx_errs++;
+		xport->stats.tx_errs++;
 		dev_err(&client->dev,
 			"%s: set page failed: %d.", __func__, retval);
 		return (retval < 0) ? retval : -EIO;
@@ -75,12 +75,12 @@ static int rmi_set_page(struct rmi_transport_dev *xport, u8 page)
 }
 
 static int rmi_i2c_write_block(struct rmi_transport_dev *xport, u16 addr,
-			       const void *buf, const int len)
+			       const void *buf, size_t len)
 {
 	struct i2c_client *client = to_i2c_client(xport->dev);
 	struct rmi_i2c_data *data = xport->data;
+	size_t tx_size = len + 1;
 	int retval;
-	int tx_size = len + 1;
 
 	mutex_lock(&data->page_mutex);
 
@@ -106,13 +106,13 @@ static int rmi_i2c_write_block(struct rmi_transport_dev *xport, u16 addr,
 	}
 
 	dev_dbg(&client->dev,
-		"writes %d bytes at %#06x: %*ph\n", len, addr, len, buf);
+		"writes %zd bytes at %#06x: %*ph\n", len, addr, (int)len, buf);
 
-	xport->info.tx_count++;
-	xport->info.tx_bytes += tx_size;
+	xport->stats.tx_count++;
+	xport->stats.tx_bytes += tx_size;
 	retval = i2c_master_send(client, data->tx_buf, tx_size);
 	if (retval < 0)
-		xport->info.tx_errs++;
+		xport->stats.tx_errs++;
 	else
 		retval--; /* don't count the address byte */
 
@@ -121,9 +121,8 @@ exit:
 	return retval;
 }
 
-
 static int rmi_i2c_read_block(struct rmi_transport_dev *xport, u16 addr,
-			      void *buf, const int len)
+			      void *buf, size_t len)
 {
 	struct i2c_client *client = to_i2c_client(xport->dev);
 	struct rmi_i2c_data *data = xport->data;
@@ -140,31 +139,36 @@ static int rmi_i2c_read_block(struct rmi_transport_dev *xport, u16 addr,
 
 	dev_dbg(&client->dev, "writes 1 bytes: %02x\n", txbuf[0]);
 
-	xport->info.tx_count++;
-	xport->info.tx_bytes += sizeof(txbuf);
+	xport->stats.tx_count++;
+	xport->stats.tx_bytes += sizeof(txbuf);
 	retval = i2c_master_send(client, txbuf, sizeof(txbuf));
 	if (retval != sizeof(txbuf)) {
-		xport->info.tx_errs++;
+		xport->stats.tx_errs++;
 		retval = (retval < 0) ? retval : -EIO;
 		goto exit;
 	}
 
-	retval = i2c_master_recv(client, buf, len);
+	xport->stats.rx_count++;
+	xport->stats.rx_bytes += len;
 
-	xport->info.rx_count++;
-	xport->info.rx_bytes += len;
+	retval = i2c_master_recv(client, buf, len);
 	if (retval < 0)
-		xport->info.rx_errs++;
+		xport->stats.rx_errs++;
 	else
 		dev_dbg(&client->dev,
-			"read %d bytes at %#06x: %*ph\n",
-			len, addr, len, buf);
+			"read %zd bytes at %#06x: %*ph\n",
+			len, addr, (int)len, buf);
 
 exit:
 	mutex_unlock(&data->page_mutex);
 	return retval;
 }
 
+static const struct rmi_transport_ops rmi_i2c_ops = {
+	.write_block	= rmi_i2c_write_block,
+	.read_block	= rmi_i2c_read_block,
+};
+
 static int rmi_i2c_probe(struct i2c_client *client,
 			 const struct i2c_device_id *id)
 {
@@ -214,9 +218,8 @@ static int rmi_i2c_probe(struct i2c_client *client,
 	xport->data = data;
 	xport->dev = &client->dev;
 
-	xport->write_block = rmi_i2c_write_block;
-	xport->read_block = rmi_i2c_read_block;
-	xport->info.proto = "i2c";
+	xport->proto_name = "i2c";
+	xport->ops = &rmi_i2c_ops;
 
 	mutex_init(&data->page_mutex);
 
-- 
1.8.4.2


^ permalink raw reply related

* [PATCH 4/4] Input: synaptics-rmi4 - switch to using i2c_transfer()
From: Dmitry Torokhov @ 2014-01-10  7:44 UTC (permalink / raw)
  To: Christopher Heiny
  Cc: Andrew Duggan, Vincent Huang, Vivian Ly, Daniel Rosenberg,
	Linus Walleij, Benjamin Tissoires, Linux Input, Linux Kernel
In-Reply-To: <1389339867-8399-1-git-send-email-dmitry.torokhov@gmail.com>

Instead of using 2 separate transactions when reading from the device let's
use i2c_transfer. Because we now have single point of failure I had to
change how we collect statistics. I elected to drop control data from the
stats and only track number of bytes read/written for the device data.

Also, since we are not prepared to deal with short reads and writes change
read_block_data and write_block_data to indicate error if we detect short
transfers.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/rmi4/rmi_i2c.c | 71 ++++++++++++++++++++++++--------------------
 1 file changed, 39 insertions(+), 32 deletions(-)

diff --git a/drivers/input/rmi4/rmi_i2c.c b/drivers/input/rmi4/rmi_i2c.c
index c176218..51f5bc8 100644
--- a/drivers/input/rmi4/rmi_i2c.c
+++ b/drivers/input/rmi4/rmi_i2c.c
@@ -57,22 +57,17 @@ struct rmi_i2c_xport {
  */
 static int rmi_set_page(struct rmi_i2c_xport *rmi_i2c, u8 page)
 {
-	struct rmi_transport_dev *xport = &rmi_i2c->xport;
 	struct i2c_client *client = rmi_i2c->client;
 	u8 txbuf[2] = {RMI_PAGE_SELECT_REGISTER, page};
 	int retval;
 
-	dev_dbg(&client->dev, "writes 3 bytes: %02x %02x\n",
-			txbuf[0], txbuf[1]);
-	xport->stats.tx_count++;
-	xport->stats.tx_bytes += sizeof(txbuf);
 	retval = i2c_master_send(client, txbuf, sizeof(txbuf));
 	if (retval != sizeof(txbuf)) {
-		xport->stats.tx_errs++;
 		dev_err(&client->dev,
 			"%s: set page failed: %d.", __func__, retval);
 		return (retval < 0) ? retval : -EIO;
 	}
+
 	rmi_i2c->page = page;
 	return 0;
 }
@@ -107,22 +102,27 @@ static int rmi_i2c_write_block(struct rmi_transport_dev *xport, u16 addr,
 
 	if (RMI_I2C_PAGE(addr) != rmi_i2c->page) {
 		retval = rmi_set_page(rmi_i2c, RMI_I2C_PAGE(addr));
-		if (retval < 0)
+		if (retval)
 			goto exit;
 	}
 
+	retval = i2c_master_send(client, rmi_i2c->tx_buf, tx_size);
+	if (retval == tx_size)
+		retval = 0;
+	else if (retval >= 0)
+		retval = -EIO;
+
+exit:
 	dev_dbg(&client->dev,
-		"writes %zd bytes at %#06x: %*ph\n", len, addr, (int)len, buf);
+		"write %zd bytes at %#06x: %d (%*ph)\n",
+		len, addr, retval, (int)len, buf);
 
 	xport->stats.tx_count++;
-	xport->stats.tx_bytes += tx_size;
-	retval = i2c_master_send(client, rmi_i2c->tx_buf, tx_size);
-	if (retval < 0)
+	if (retval)
 		xport->stats.tx_errs++;
 	else
-		retval--; /* don't count the address byte */
+		xport->stats.tx_bytes += len;
 
-exit:
 	mutex_unlock(&rmi_i2c->page_mutex);
 	return retval;
 }
@@ -133,40 +133,47 @@ static int rmi_i2c_read_block(struct rmi_transport_dev *xport, u16 addr,
 	struct rmi_i2c_xport *rmi_i2c =
 		container_of(xport, struct rmi_i2c_xport, xport);
 	struct i2c_client *client = rmi_i2c->client;
-	u8 txbuf[1] = {addr & 0xff};
+	u8 addr_offset = addr & 0xff;
 	int retval;
+	struct i2c_msg msgs[] = {
+		{
+			.addr	= client->addr,
+			.len	= sizeof(addr_offset),
+			.buf	= &addr_offset,
+		},
+		{
+			.addr	= client->addr,
+			.flags	= I2C_M_RD,
+			.len	= len,
+			.buf	= buf,
+		},
+	};
 
 	mutex_lock(&rmi_i2c->page_mutex);
 
 	if (RMI_I2C_PAGE(addr) != rmi_i2c->page) {
 		retval = rmi_set_page(rmi_i2c, RMI_I2C_PAGE(addr));
-		if (retval < 0)
+		if (retval)
 			goto exit;
 	}
 
-	dev_dbg(&client->dev, "writes 1 bytes: %02x\n", txbuf[0]);
+	retval = i2c_transfer(client->adapter, msgs, sizeof(msgs));
+	if (retval == sizeof(msgs))
+		retval = 0; /* success */
+	else if (retval >= 0)
+		retval = -EIO;
 
-	xport->stats.tx_count++;
-	xport->stats.tx_bytes += sizeof(txbuf);
-	retval = i2c_master_send(client, txbuf, sizeof(txbuf));
-	if (retval != sizeof(txbuf)) {
-		xport->stats.tx_errs++;
-		retval = (retval < 0) ? retval : -EIO;
-		goto exit;
-	}
+exit:
+	dev_dbg(&client->dev,
+		"read %zd bytes at %#06x: %d (%*ph)\n",
+		len, addr, retval, (int)len, buf);
 
 	xport->stats.rx_count++;
-	xport->stats.rx_bytes += len;
-
-	retval = i2c_master_recv(client, buf, len);
-	if (retval < 0)
+	if (retval)
 		xport->stats.rx_errs++;
 	else
-		dev_dbg(&client->dev,
-			"read %zd bytes at %#06x: %*ph\n",
-			len, addr, (int)len, buf);
+		xport->stats.rx_bytes += len;
 
-exit:
 	mutex_unlock(&rmi_i2c->page_mutex);
 	return retval;
 }
-- 
1.8.4.2

^ permalink raw reply related

* [PATCH 3/4] Input: synaptics-rmi4 - fix I2C functionality check
From: Dmitry Torokhov @ 2014-01-10  7:44 UTC (permalink / raw)
  To: Christopher Heiny
  Cc: Andrew Duggan, Vincent Huang, Vivian Ly, Daniel Rosenberg,
	Linus Walleij, Benjamin Tissoires, Linux Input, Linux Kernel
In-Reply-To: <1389339867-8399-1-git-send-email-dmitry.torokhov@gmail.com>

When adapter does not support required functionality (I2C_FUNC_I2C) we were
returning 0 to the upper layers, making them believe that device bound
successfully.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/rmi4/rmi_i2c.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/input/rmi4/rmi_i2c.c b/drivers/input/rmi4/rmi_i2c.c
index cdc8527..c176218 100644
--- a/drivers/input/rmi4/rmi_i2c.c
+++ b/drivers/input/rmi4/rmi_i2c.c
@@ -193,11 +193,10 @@ static int rmi_i2c_probe(struct i2c_client *client,
 		pdata->sensor_name ? pdata->sensor_name : "-no name-",
 		client->addr, pdata->attn_gpio);
 
-	retval = i2c_check_functionality(client->adapter, I2C_FUNC_I2C);
-	if (!retval) {
-		dev_err(&client->dev, "i2c_check_functionality error %d.\n",
-			retval);
-		return retval;
+	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
+		dev_err(&client->dev,
+			"adapter does not support required functionality.\n");
+		return -ENODEV;
 	}
 
 	if (pdata->gpio_config) {
-- 
1.8.4.2

^ permalink raw reply related

* [PATCH 2/4] Input: synaptics-rmi4 - rework transport device allocation
From: Dmitry Torokhov @ 2014-01-10  7:44 UTC (permalink / raw)
  To: Christopher Heiny
  Cc: Andrew Duggan, Vincent Huang, Vivian Ly, Daniel Rosenberg,
	Linus Walleij, Benjamin Tissoires, Linux Input, Linux Kernel
In-Reply-To: <1389339867-8399-1-git-send-email-dmitry.torokhov@gmail.com>

Instead of allocating common and private part of transport device
separately make private wrap common part and get rid of private data
pointer in the transport device.

Also rename rmi_i2c_data -> rmi_i2c_xport and data -> rmi_i2c.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/rmi4/rmi_bus.h |   3 --
 drivers/input/rmi4/rmi_i2c.c | 112 +++++++++++++++++++++----------------------
 2 files changed, 56 insertions(+), 59 deletions(-)

diff --git a/drivers/input/rmi4/rmi_bus.h b/drivers/input/rmi4/rmi_bus.h
index ccf26dc..decb479 100644
--- a/drivers/input/rmi4/rmi_bus.h
+++ b/drivers/input/rmi4/rmi_bus.h
@@ -165,7 +165,6 @@ struct rmi_transport_stats {
  * default irq_thread implementation.
  * @hard_irq: if not NULL, the sensor driver will use this for the hard IRQ
  * handling
- * @data: Private data pointer
  * @proto_name: name of the transport protocol (SPI, i2c, etc)
  * @ops: pointer to transport operations implementation
  * @stats: transport statistics
@@ -181,8 +180,6 @@ struct rmi_transport_dev {
 	irqreturn_t (*irq_thread)(int irq, void *p);
 	irqreturn_t (*hard_irq)(int irq, void *p);
 
-	void *data;
-
 	const char *proto_name;
 	const struct rmi_transport_ops *ops;
 	struct rmi_transport_stats stats;
diff --git a/drivers/input/rmi4/rmi_i2c.c b/drivers/input/rmi4/rmi_i2c.c
index 40badf3..cdc8527 100644
--- a/drivers/input/rmi4/rmi_i2c.c
+++ b/drivers/input/rmi4/rmi_i2c.c
@@ -17,22 +17,25 @@
 #define BUFFER_SIZE_INCREMENT 32
 
 /**
- * struct rmi_i2c_data - stores information for i2c communication
+ * struct rmi_i2c_xport - stores information for i2c communication
+ *
+ * @xport: The transport interface structure
  *
  * @page_mutex: Locks current page to avoid changing pages in unexpected ways.
  * @page: Keeps track of the current virtual page
- * @xport: Pointer to the transport interface
  *
  * @tx_buf: Buffer used for transmitting data to the sensor over i2c.
  * @tx_buf_size: Size of the buffer
  */
-struct rmi_i2c_data {
+struct rmi_i2c_xport {
+	struct rmi_transport_dev xport;
+	struct i2c_client *client;
+
 	struct mutex page_mutex;
 	int page;
-	struct rmi_transport_dev *xport;
 
 	u8 *tx_buf;
-	int tx_buf_size;
+	size_t tx_buf_size;
 };
 
 #define RMI_PAGE_SELECT_REGISTER 0xff
@@ -52,10 +55,10 @@ struct rmi_i2c_data {
  *
  * Returns zero on success, non-zero on failure.
  */
-static int rmi_set_page(struct rmi_transport_dev *xport, u8 page)
+static int rmi_set_page(struct rmi_i2c_xport *rmi_i2c, u8 page)
 {
-	struct i2c_client *client = to_i2c_client(xport->dev);
-	struct rmi_i2c_data *data = xport->data;
+	struct rmi_transport_dev *xport = &rmi_i2c->xport;
+	struct i2c_client *client = rmi_i2c->client;
 	u8 txbuf[2] = {RMI_PAGE_SELECT_REGISTER, page};
 	int retval;
 
@@ -70,37 +73,40 @@ static int rmi_set_page(struct rmi_transport_dev *xport, u8 page)
 			"%s: set page failed: %d.", __func__, retval);
 		return (retval < 0) ? retval : -EIO;
 	}
-	data->page = page;
+	rmi_i2c->page = page;
 	return 0;
 }
 
 static int rmi_i2c_write_block(struct rmi_transport_dev *xport, u16 addr,
 			       const void *buf, size_t len)
 {
-	struct i2c_client *client = to_i2c_client(xport->dev);
-	struct rmi_i2c_data *data = xport->data;
+	struct rmi_i2c_xport *rmi_i2c =
+		container_of(xport, struct rmi_i2c_xport, xport);
+	struct i2c_client *client = rmi_i2c->client;
 	size_t tx_size = len + 1;
 	int retval;
 
-	mutex_lock(&data->page_mutex);
-
-	if (!data->tx_buf || data->tx_buf_size < tx_size) {
-		if (data->tx_buf)
-			devm_kfree(&client->dev, data->tx_buf);
-		data->tx_buf_size = tx_size + BUFFER_SIZE_INCREMENT;
-		data->tx_buf = devm_kzalloc(&client->dev, data->tx_buf_size,
-					    GFP_KERNEL);
-		if (!data->tx_buf) {
-			data->tx_buf_size = 0;
+	mutex_lock(&rmi_i2c->page_mutex);
+
+	if (!rmi_i2c->tx_buf || rmi_i2c->tx_buf_size < tx_size) {
+		if (rmi_i2c->tx_buf)
+			devm_kfree(&client->dev, rmi_i2c->tx_buf);
+		rmi_i2c->tx_buf_size = tx_size + BUFFER_SIZE_INCREMENT;
+		rmi_i2c->tx_buf = devm_kzalloc(&client->dev,
+					       rmi_i2c->tx_buf_size,
+					       GFP_KERNEL);
+		if (!rmi_i2c->tx_buf) {
+			rmi_i2c->tx_buf_size = 0;
 			retval = -ENOMEM;
 			goto exit;
 		}
 	}
-	data->tx_buf[0] = addr & 0xff;
-	memcpy(data->tx_buf + 1, buf, len);
 
-	if (RMI_I2C_PAGE(addr) != data->page) {
-		retval = rmi_set_page(xport, RMI_I2C_PAGE(addr));
+	rmi_i2c->tx_buf[0] = addr & 0xff;
+	memcpy(rmi_i2c->tx_buf + 1, buf, len);
+
+	if (RMI_I2C_PAGE(addr) != rmi_i2c->page) {
+		retval = rmi_set_page(rmi_i2c, RMI_I2C_PAGE(addr));
 		if (retval < 0)
 			goto exit;
 	}
@@ -110,29 +116,30 @@ static int rmi_i2c_write_block(struct rmi_transport_dev *xport, u16 addr,
 
 	xport->stats.tx_count++;
 	xport->stats.tx_bytes += tx_size;
-	retval = i2c_master_send(client, data->tx_buf, tx_size);
+	retval = i2c_master_send(client, rmi_i2c->tx_buf, tx_size);
 	if (retval < 0)
 		xport->stats.tx_errs++;
 	else
 		retval--; /* don't count the address byte */
 
 exit:
-	mutex_unlock(&data->page_mutex);
+	mutex_unlock(&rmi_i2c->page_mutex);
 	return retval;
 }
 
 static int rmi_i2c_read_block(struct rmi_transport_dev *xport, u16 addr,
 			      void *buf, size_t len)
 {
-	struct i2c_client *client = to_i2c_client(xport->dev);
-	struct rmi_i2c_data *data = xport->data;
+	struct rmi_i2c_xport *rmi_i2c =
+		container_of(xport, struct rmi_i2c_xport, xport);
+	struct i2c_client *client = rmi_i2c->client;
 	u8 txbuf[1] = {addr & 0xff};
 	int retval;
 
-	mutex_lock(&data->page_mutex);
+	mutex_lock(&rmi_i2c->page_mutex);
 
-	if (RMI_I2C_PAGE(addr) != data->page) {
-		retval = rmi_set_page(xport, RMI_I2C_PAGE(addr));
+	if (RMI_I2C_PAGE(addr) != rmi_i2c->page) {
+		retval = rmi_set_page(rmi_i2c, RMI_I2C_PAGE(addr));
 		if (retval < 0)
 			goto exit;
 	}
@@ -160,7 +167,7 @@ static int rmi_i2c_read_block(struct rmi_transport_dev *xport, u16 addr,
 			len, addr, (int)len, buf);
 
 exit:
-	mutex_unlock(&data->page_mutex);
+	mutex_unlock(&rmi_i2c->page_mutex);
 	return retval;
 }
 
@@ -174,14 +181,14 @@ static int rmi_i2c_probe(struct i2c_client *client,
 {
 	const struct rmi_device_platform_data *pdata =
 				dev_get_platdata(&client->dev);
-	struct rmi_transport_dev *xport;
-	struct rmi_i2c_data *data;
+	struct rmi_i2c_xport *rmi_i2c;
 	int retval;
 
 	if (!pdata) {
 		dev_err(&client->dev, "no platform data\n");
 		return -EINVAL;
 	}
+
 	dev_dbg(&client->dev, "Probing %s at %#02x (GPIO %d).\n",
 		pdata->sensor_name ? pdata->sensor_name : "-no name-",
 		client->addr, pdata->attn_gpio);
@@ -202,44 +209,36 @@ static int rmi_i2c_probe(struct i2c_client *client,
 		}
 	}
 
-	xport = devm_kzalloc(&client->dev, sizeof(struct rmi_transport_dev),
+	rmi_i2c = devm_kzalloc(&client->dev, sizeof(struct rmi_i2c_xport),
 				GFP_KERNEL);
-
-	if (!xport)
+	if (!rmi_i2c)
 		return -ENOMEM;
 
-	data = devm_kzalloc(&client->dev, sizeof(struct rmi_i2c_data),
-				GFP_KERNEL);
-	if (!data)
-		return -ENOMEM;
-
-	data->xport = xport;
-
-	xport->data = data;
-	xport->dev = &client->dev;
+	rmi_i2c->client = client;
+	mutex_init(&rmi_i2c->page_mutex);
 
-	xport->proto_name = "i2c";
-	xport->ops = &rmi_i2c_ops;
-
-	mutex_init(&data->page_mutex);
+	rmi_i2c->xport.dev = &client->dev;
+	rmi_i2c->xport.proto_name = "i2c";
+	rmi_i2c->xport.ops = &rmi_i2c_ops;
 
 	/*
 	 * Setting the page to zero will (a) make sure the PSR is in a
 	 * known state, and (b) make sure we can talk to the device.
 	 */
-	retval = rmi_set_page(xport, 0);
+	retval = rmi_set_page(rmi_i2c, 0);
 	if (retval) {
 		dev_err(&client->dev, "Failed to set page select to 0.\n");
 		return retval;
 	}
 
-	retval = rmi_register_transport_device(xport);
+	retval = rmi_register_transport_device(&rmi_i2c->xport);
 	if (retval) {
 		dev_err(&client->dev, "Failed to register transport driver at 0x%.2X.\n",
 			client->addr);
 		goto err_gpio;
 	}
-	i2c_set_clientdata(client, xport);
+
+	i2c_set_clientdata(client, rmi_i2c);
 
 	dev_info(&client->dev, "registered rmi i2c driver at %#04x.\n",
 			client->addr);
@@ -248,16 +247,17 @@ static int rmi_i2c_probe(struct i2c_client *client,
 err_gpio:
 	if (pdata->gpio_config)
 		pdata->gpio_config(pdata->gpio_data, false);
+
 	return retval;
 }
 
 static int rmi_i2c_remove(struct i2c_client *client)
 {
-	struct rmi_transport_dev *xport = i2c_get_clientdata(client);
 	const struct rmi_device_platform_data *pdata =
 				dev_get_platdata(&client->dev);
+	struct rmi_i2c_xport *rmi_i2c = i2c_get_clientdata(client);
 
-	rmi_unregister_transport_device(xport);
+	rmi_unregister_transport_device(&rmi_i2c->xport);
 
 	if (pdata->gpio_config)
 		pdata->gpio_config(pdata->gpio_data, false);
-- 
1.8.4.2

^ permalink raw reply related

* Re: [PATCH] input: synaptics-rmi4 - rmi_driver.h tidy up
From: Dmitry Torokhov @ 2014-01-10  6:28 UTC (permalink / raw)
  To: Christopher Heiny
  Cc: Linux Input, Andrew Duggan, Vincent Huang, Vivian Ly,
	Daniel Rosenberg, Jean Delvare, Joerie de Gram, Linus Walleij,
	Benjamin Tissoires
In-Reply-To: <1389319178-27751-1-git-send-email-cheiny@synaptics.com>

On Thu, Jan 09, 2014 at 05:59:38PM -0800, Christopher Heiny wrote:
> This is a trivial change to remove an unused macro, and change CONFIG_PM to
> CONFIG_PM_SLEEP.
> 
> Signed-off-by: Christopher Heiny <cheiny@synaptics.com>
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>

Applied, thank you.

> 
> ---
> 
>  drivers/input/rmi4/rmi_driver.h | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/input/rmi4/rmi_driver.h b/drivers/input/rmi4/rmi_driver.h
> index 4d82b90..ad3ee33 100644
> --- a/drivers/input/rmi4/rmi_driver.h
> +++ b/drivers/input/rmi4/rmi_driver.h
> @@ -24,8 +24,6 @@
>  	.attrs = _attrs,  \
>  }
>  
> -#define attrify(nm) (&dev_attr_##nm.attr)
> -
>  #define PDT_PROPERTIES_LOCATION 0x00EF
>  #define BSR_LOCATION 0x00FE
>  
> @@ -62,7 +60,7 @@ struct rmi_driver_data {
>  	u8 bsr;
>  
>  	bool enabled;
> -#ifdef CONFIG_PM
> +#ifdef CONFIG_PM_SLEEP
>  	bool suspended;
>  	struct mutex suspend_mutex;
>  

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH] input: synaptics-rmi4 - fix F01 DOM formatting
From: Dmitry Torokhov @ 2014-01-10  6:27 UTC (permalink / raw)
  To: Christopher Heiny
  Cc: Linux Input, Andrew Duggan, Vincent Huang, Vivian Ly,
	Daniel Rosenberg, Jean Delvare, Joerie de Gram, Linus Walleij,
	Benjamin Tissoires
In-Reply-To: <1389298893-4043-1-git-send-email-cheiny@synaptics.com>

On Thu, Jan 09, 2014 at 12:21:33PM -0800, Christopher Heiny wrote:
> Use a sensible format string for the date of manufacture formatting.
> 
> Signed-off-by: Christopher Heiny <cheiny@synaptics.com>
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>

Applied, thank you.

> 
> ---
> 
>  drivers/input/rmi4/rmi_f01.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/input/rmi4/rmi_f01.c b/drivers/input/rmi4/rmi_f01.c
> index 1cb11ea..9e6a578 100644
> --- a/drivers/input/rmi4/rmi_f01.c
> +++ b/drivers/input/rmi4/rmi_f01.c
> @@ -50,7 +50,7 @@ struct f01_basic_properties {
>  	bool has_lts;
>  	bool has_adjustable_doze;
>  	bool has_adjustable_doze_holdoff;
> -	char dom[9]; /* YYYYMMDD + '\0' */
> +	char dom[11]; /* YYYY/MM/DD + '\0' */
>  	u8 product_id[RMI_PRODUCT_ID_LENGTH + 1];
>  	u16 productinfo;
>  };
> @@ -190,8 +190,7 @@ static int rmi_f01_read_properties(struct rmi_device *rmi_dev,
>  	props->has_adjustable_doze_holdoff =
>  			basic_query[1] & RMI_F01_QRY1_HAS_ADJ_DOZE_HOFF;
>  
> -	snprintf(props->dom, sizeof(props->dom),
> -		 "20%02x%02x%02x",
> +	snprintf(props->dom, sizeof(props->dom), "20%02d/%02d/%02d",
>  		 basic_query[5] & RMI_F01_QRY5_YEAR_MASK,
>  		 basic_query[6] & RMI_F01_QRY6_MONTH_MASK,
>  		 basic_query[7] & RMI_F01_QRY7_DAY_MASK);

-- 
Dmitry

^ permalink raw reply

* [PATCH] add support for ALPS v7 protocol device
From: Qiting Chen @ 2014-01-10  5:53 UTC (permalink / raw)
  To: dmitry.torokhov, cernekee, dturvene
  Cc: linux-input, ndevos, jclift, Qiting Chen

Here is the patch of supporting ALPS v7 protocol device.

v7 device is a clickpad device.
Device info:
	Device ID = 0x73, 0x03, 0x0a
	Firmware ID = 0x88, 0xb*, 0x**

Support function of v7 device:
- Cursor
- Tap, double tap, tap drag, 2finger tap
- Pan, pinch
- Button click: v7 device has one physical button under the touchpad. A Button Area covers the bottom of touchpad. Clicking on Button Area produces button acitivities.
  Click touchpad with all fingers outside right Button Area --> left click
  Click touchpad with at lease 1 finger inside right Button Area --> right click
- Resting finger function: Cursor and gestures won't be influenced with one finger placed still on Button Area.

The resting finger process is in alps.c as it seems the latest xserver-xorg-input-synaptics(1.7.1) doesn't support that part.
We tried registering our device as a clickpad by:
	set_bit(INPUT_PROP_BUTTONPAD, dev1->propbit)
But only button click can be correctly recognized. Yet a finger at Button Area won't be ignored while doing cursroing.


Signed-off-by: Qiting Chen <qiting.chen@cn.alps.com>
---
 drivers/input/mouse/alps.c | 478 ++++++++++++++++++++++++++++++++++++++++++---
 drivers/input/mouse/alps.h | 127 ++++++++++--
 2 files changed, 554 insertions(+), 51 deletions(-)

diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
index fb15c64..3e8e8f7 100644
--- a/drivers/input/mouse/alps.c
+++ b/drivers/input/mouse/alps.c
@@ -32,6 +32,11 @@
 #define ALPS_REG_BASE_RUSHMORE	0xc2c0
 #define ALPS_REG_BASE_PINNACLE	0x0000
 
+#define LEFT_BUTTON_BIT			0x01
+#define RIGHT_BUTTON_BIT		0x02
+
+#define V7_LARGE_MOVEMENT		130
+
 static const struct alps_nibble_commands alps_v3_nibble_commands[] = {
 	{ PSMOUSE_CMD_SETPOLL,		0x00 }, /* 0 */
 	{ PSMOUSE_CMD_RESET_DIS,	0x00 }, /* 1 */
@@ -99,6 +104,7 @@ static const struct alps_nibble_commands alps_v6_nibble_commands[] = {
 #define ALPS_FOUR_BUTTONS	0x40	/* 4 direction button present */
 #define ALPS_PS2_INTERLEAVED	0x80	/* 3-byte PS/2 packet interleaved with
 					   6-byte ALPS packet */
+#define ALPS_BTNLESS			0x100	/* ALPS ClickPad flag */
 
 static const struct alps_model_info alps_model_data[] = {
 	{ { 0x32, 0x02, 0x14 },	0x00, ALPS_PROTO_V2, 0xf8, 0xf8, ALPS_PASS | ALPS_DUALPOINT },	/* Toshiba Salellite Pro M10 */
@@ -140,6 +146,20 @@ static void alps_set_abs_params_mt(struct alps_data *priv,
  * isn't valid per PS/2 spec.
  */
 
+static unsigned int alps_pt_distance(struct alps_abs_data *pt0,
+				    struct alps_abs_data *pt1)
+{
+	int vect_x, vect_y;
+
+	if (!pt0 || !pt1)
+		return 0;
+
+	vect_x = pt0->x - pt1->x;
+	vect_y = pt0->y - pt1->y;
+
+	return int_sqrt(vect_x * vect_x + vect_y * vect_y);
+}
+
 /* Packet formats are described in Documentation/input/alps.txt */
 
 static bool alps_is_valid_first_byte(struct alps_data *priv,
@@ -320,8 +340,8 @@ static void alps_process_bitmap_dolphin(struct alps_data *priv,
 		end_bit = y_msb - 1;
 		box_middle_y = (priv->y_max * (start_bit + end_bit)) /
 				(2 * (priv->y_bits - 1));
-		*x1 = fields->x;
-		*y1 = fields->y;
+		*x1 = fields->pt.x;
+		*y1 = fields->pt.y;
 		*x2 = 2 * box_middle_x - *x1;
 		*y2 = 2 * box_middle_y - *y1;
 	}
@@ -461,6 +481,38 @@ static void alps_report_semi_mt_data(struct input_dev *dev, int num_fingers,
 	alps_set_slot(dev, 1, num_fingers == 2, x2, y2);
 }
 
+static void alps_report_coord_and_btn(struct psmouse *psmouse,
+				      struct alps_fields *f)
+{
+	struct input_dev *dev;
+
+	if (!psmouse || !f)
+		return;
+
+	dev = psmouse->dev;
+
+	if (f->fingers) {
+		input_report_key(dev, BTN_TOUCH, 1);
+		alps_report_semi_mt_data(dev, f->fingers,
+			f->pt_img[0].x, f->pt_img[0].y,
+			f->pt_img[1].x, f->pt_img[1].y);
+		input_mt_report_finger_count(dev, f->fingers);
+
+		input_report_abs(dev, ABS_X, f->pt_img[0].x);
+		input_report_abs(dev, ABS_Y, f->pt_img[0].y);
+		input_report_abs(dev, ABS_PRESSURE, f->pt_img[0].z);
+	} else {
+		input_report_key(dev, BTN_TOUCH, 0);
+		input_mt_report_finger_count(dev, 0);
+		input_report_abs(dev, ABS_PRESSURE, 0);
+	}
+
+	input_report_key(dev, BTN_LEFT, f->btn.left);
+	input_report_key(dev, BTN_RIGHT, f->btn.right);
+
+	input_sync(dev);
+}
+
 static void alps_process_trackstick_packet_v3(struct psmouse *psmouse)
 {
 	struct alps_data *priv = psmouse->private;
@@ -523,13 +575,13 @@ static void alps_process_trackstick_packet_v3(struct psmouse *psmouse)
 
 static void alps_decode_buttons_v3(struct alps_fields *f, unsigned char *p)
 {
-	f->left = !!(p[3] & 0x01);
-	f->right = !!(p[3] & 0x02);
-	f->middle = !!(p[3] & 0x04);
+	f->btn.left = !!(p[3] & 0x01);
+	f->btn.right = !!(p[3] & 0x02);
+	f->btn.middle = !!(p[3] & 0x04);
 
-	f->ts_left = !!(p[3] & 0x10);
-	f->ts_right = !!(p[3] & 0x20);
-	f->ts_middle = !!(p[3] & 0x40);
+	f->btn.ts_left = !!(p[3] & 0x10);
+	f->btn.ts_right = !!(p[3] & 0x20);
+	f->btn.ts_middle = !!(p[3] & 0x40);
 }
 
 static void alps_decode_pinnacle(struct alps_fields *f, unsigned char *p,
@@ -546,10 +598,10 @@ static void alps_decode_pinnacle(struct alps_fields *f, unsigned char *p,
 		   ((p[2] & 0x7f) << 1) |
 		   (p[4] & 0x01);
 
-	f->x = ((p[1] & 0x7f) << 4) | ((p[4] & 0x30) >> 2) |
+	f->pt.x = ((p[1] & 0x7f) << 4) | ((p[4] & 0x30) >> 2) |
 	       ((p[0] & 0x30) >> 4);
-	f->y = ((p[2] & 0x7f) << 4) | (p[4] & 0x0f);
-	f->z = p[5] & 0x7f;
+	f->pt.y = ((p[2] & 0x7f) << 4) | (p[4] & 0x0f);
+	f->pt.z = p[5] & 0x7f;
 
 	alps_decode_buttons_v3(f, p);
 }
@@ -573,9 +625,9 @@ static void alps_decode_dolphin(struct alps_fields *f, unsigned char *p,
 	f->is_mp = !!(p[0] & 0x20);
 
 	if (!f->is_mp) {
-		f->x = ((p[1] & 0x7f) | ((p[4] & 0x0f) << 7));
-		f->y = ((p[2] & 0x7f) | ((p[4] & 0xf0) << 3));
-		f->z = (p[0] & 4) ? 0 : p[5] & 0x7f;
+		f->pt.x = ((p[1] & 0x7f) | ((p[4] & 0x0f) << 7));
+		f->pt.y = ((p[2] & 0x7f) | ((p[4] & 0xf0) << 3));
+		f->pt.z = (p[0] & 4) ? 0 : p[5] & 0x7f;
 		alps_decode_buttons_v3(f, p);
 	} else {
 		f->fingers = ((p[0] & 0x6) >> 1 |
@@ -687,7 +739,7 @@ static void alps_process_touchpad_packet_v3_v5(struct psmouse *psmouse)
 	 * with x, y, and z all zero, so these seem to be flukes.
 	 * Ignore them.
 	 */
-	if (f.x && f.y && !f.z)
+	if (f.pt.x && f.pt.y && !f.pt.z)
 		return;
 
 	/*
@@ -695,12 +747,12 @@ static void alps_process_touchpad_packet_v3_v5(struct psmouse *psmouse)
 	 * to rely on ST data.
 	 */
 	if (!fingers) {
-		x1 = f.x;
-		y1 = f.y;
-		fingers = f.z > 0 ? 1 : 0;
+		x1 = f.pt.x;
+		y1 = f.pt.y;
+		fingers = f.pt.z > 0 ? 1 : 0;
 	}
 
-	if (f.z >= 64)
+	if (f.pt.z >= 64)
 		input_report_key(dev, BTN_TOUCH, 1);
 	else
 		input_report_key(dev, BTN_TOUCH, 0);
@@ -709,22 +761,22 @@ static void alps_process_touchpad_packet_v3_v5(struct psmouse *psmouse)
 
 	input_mt_report_finger_count(dev, fingers);
 
-	input_report_key(dev, BTN_LEFT, f.left);
-	input_report_key(dev, BTN_RIGHT, f.right);
-	input_report_key(dev, BTN_MIDDLE, f.middle);
+	input_report_key(dev, BTN_LEFT, f.btn.left);
+	input_report_key(dev, BTN_RIGHT, f.btn.right);
+	input_report_key(dev, BTN_MIDDLE, f.btn.middle);
 
-	if (f.z > 0) {
-		input_report_abs(dev, ABS_X, f.x);
-		input_report_abs(dev, ABS_Y, f.y);
+	if (f.pt.z > 0) {
+		input_report_abs(dev, ABS_X, f.pt.x);
+		input_report_abs(dev, ABS_Y, f.pt.y);
 	}
-	input_report_abs(dev, ABS_PRESSURE, f.z);
+	input_report_abs(dev, ABS_PRESSURE, f.pt.z);
 
 	input_sync(dev);
 
 	if (!(priv->quirks & ALPS_QUIRK_TRACKSTICK_BUTTONS)) {
-		input_report_key(dev2, BTN_LEFT, f.ts_left);
-		input_report_key(dev2, BTN_RIGHT, f.ts_right);
-		input_report_key(dev2, BTN_MIDDLE, f.ts_middle);
+		input_report_key(dev2, BTN_LEFT, f.btn.ts_left);
+		input_report_key(dev2, BTN_RIGHT, f.btn.ts_right);
+		input_report_key(dev2, BTN_MIDDLE, f.btn.ts_middle);
 		input_sync(dev2);
 	}
 }
@@ -916,6 +968,294 @@ static void alps_process_packet_v4(struct psmouse *psmouse)
 	input_sync(dev);
 }
 
+static bool alps_is_valid_package_v7(struct psmouse *psmouse)
+{
+	if ((psmouse->pktcnt == 3) && ((psmouse->packet[2] & 0x40) != 0x40))
+		return false;
+	if ((psmouse->pktcnt == 4) && ((psmouse->packet[3] & 0x48) != 0x48))
+		return false;
+	if ((psmouse->pktcnt == 6) && ((psmouse->packet[5] & 0x40) != 0x0))
+		return false;
+	return true;
+}
+
+static int alps_drop_unsupported_packet_v7(struct psmouse *psmouse)
+{
+	struct alps_data *priv = psmouse->private;
+	int drop = 1;
+
+	if (priv->r.v7.pkt_id == V7_PACKET_ID_NEW ||
+	    priv->r.v7.pkt_id == V7_PACKET_ID_TWO ||
+	    priv->r.v7.pkt_id == V7_PACKET_ID_MULTI ||
+	    priv->r.v7.pkt_id == V7_PACKET_ID_IDLE)
+		drop = 0;
+
+	return drop;
+}
+
+static unsigned char alps_get_packet_id_v7(char *byte)
+{
+	unsigned char packet_id;
+
+	if (byte[4] & 0x40)
+		packet_id = V7_PACKET_ID_TWO;
+	else if (byte[4] & 0x01)
+		packet_id = V7_PACKET_ID_MULTI;
+	else if ((byte[0] & 0x10) && !(byte[4] & 0x43))
+		packet_id = V7_PACKET_ID_NEW;
+	else
+		packet_id = V7_PACKET_ID_IDLE;
+
+	return packet_id;
+}
+
+static void alps_get_finger_coordinate_v7(struct alps_abs_data *pt,
+					  unsigned char *pkt,
+					  unsigned char pkt_id)
+{
+	if ((pkt_id == V7_PACKET_ID_TWO) ||
+	   (pkt_id == V7_PACKET_ID_MULTI) ||
+	   (pkt_id == V7_PACKET_ID_NEW)) {
+		pt[0].x = ((pkt[2] & 0x80) << 4);
+		pt[0].x |= ((pkt[2] & 0x3F) << 5);
+		pt[0].x |= ((pkt[3] & 0x30) >> 1);
+		pt[0].x |= (pkt[3] & 0x07);
+		pt[0].y = (pkt[1] << 3) | (pkt[0] & 0x07);
+
+		pt[1].x = ((pkt[3] & 0x80) << 4);
+		pt[1].x |= ((pkt[4] & 0x80) << 3);
+		pt[1].x |= ((pkt[4] & 0x3F) << 4);
+		pt[1].y = ((pkt[5] & 0x80) << 3);
+		pt[1].y |= ((pkt[5] & 0x3F) << 4);
+
+		if (pkt_id == V7_PACKET_ID_TWO) {
+			pt[1].x &= ~0x000F;
+			pt[1].y |= 0x000F;
+		} else if (pkt_id == V7_PACKET_ID_MULTI) {
+			pt[1].x &= ~0x003F;
+			pt[1].y &= ~0x0020;
+			pt[1].y |= ((pkt[4] & 0x02) << 4);
+			pt[1].y |= 0x001F;
+		} else if (pkt_id == V7_PACKET_ID_NEW) {
+			pt[1].x &= ~0x003F;
+			pt[1].x |= (pkt[0] & 0x20);
+			pt[1].y |= 0x000F;
+		}
+
+		pt[0].y = 0x7FF - pt[0].y;
+		pt[1].y = 0x7FF - pt[1].y;
+
+		pt[0].z = (pt[0].x && pt[0].y) ? 62 : 0;
+		pt[1].z = (pt[1].x && pt[1].y) ? 62 : 0;
+	}
+}
+
+static void alps_decode_packet_v7(struct alps_fields *f,
+				  unsigned char *p,
+				  struct psmouse *psmouse)
+{
+	struct alps_data *priv = psmouse->private;
+
+	priv->r.v7.pkt_id = alps_get_packet_id_v7(p);
+
+	alps_get_finger_coordinate_v7(f->pt_img, p, priv->r.v7.pkt_id);
+
+	priv->r.v7.rest_left = 0;
+	priv->r.v7.rest_right = 0;
+	priv->r.v7.additional_fingers = 0;
+	priv->phy_btn = 0;
+
+	if (priv->r.v7.pkt_id == V7_PACKET_ID_TWO ||
+	    priv->r.v7.pkt_id == V7_PACKET_ID_MULTI) {
+		priv->r.v7.rest_left = (p[0] & 0x10) >> 4;
+		priv->r.v7.rest_right = (p[0] & 0x20) >> 5;
+	}
+
+	if (priv->r.v7.pkt_id == V7_PACKET_ID_MULTI)
+		priv->r.v7.additional_fingers = p[5] & 0x03;
+
+	priv->phy_btn = (p[0] & 0x80) >> 7;
+}
+
+static void alps_set_each_pt_attr_v7(struct psmouse *psmouse,
+				     struct alps_abs_data *pt,
+				     struct alps_bl_pt_attr *pt_attr)
+{
+	struct alps_data *priv = psmouse->private;
+	unsigned int dist;
+
+	if (!pt_attr->is_init_pt_got && pt->z != 0) {
+		pt_attr->is_init_pt_got = 1;
+		pt_attr->is_counted = 0;
+		memcpy(&pt_attr->init_pt, pt, sizeof(pt_attr->init_pt));
+	}
+
+	if (pt->z != 0) {
+		if (pt->y < priv->resting_zone_y_min) {
+			/* A finger is recognized as a non-resting finger
+			if it's position is outside the resting finger zone.*/
+			pt_attr->zone = ZONE_NORMAL;
+			pt_attr->is_counted = 1;
+		} else {
+			/* A finger is recognized as a resting finger if it's
+			position is inside the resting finger zone and there's
+			no large movement from it's touch down position.*/
+			pt_attr->zone = ZONE_RESTING;
+
+			if (pt->x > priv->x_max / 2)
+				pt_attr->zone |= ZONE_RIGHT_BTN;
+			else
+				pt_attr->zone |= ZONE_LEFT_BTN;
+
+			/* A resting finger will turn to be a non-resting
+			finger if it has made large movement from it's touch
+			down position. A non-resting finger will never turn
+			to a resting finger before it leaves the touchpad
+			surface.*/
+			if (pt_attr->is_init_pt_got) {
+				dist = alps_pt_distance(pt, &pt_attr->init_pt);
+
+				if (dist > V7_LARGE_MOVEMENT)
+					pt_attr->is_counted = 1;
+			}
+		}
+	}
+}
+
+static void alps_set_pt_attr_v7(struct psmouse *psmouse,
+				       struct alps_fields *f)
+{
+	struct alps_data *priv = psmouse->private;
+	int i;
+
+	switch (priv->r.v7.pkt_id) {
+	case  V7_PACKET_ID_TWO:
+	case  V7_PACKET_ID_MULTI:
+		for (i = 0; i < V7_IMG_PT_NUM; i++)
+			alps_set_each_pt_attr_v7(psmouse,
+						 &f->pt_img[i],
+						 &priv->pt_attr[i]);
+		break;
+	default:
+		/*All finger attributes are cleared when packet ID is
+		'IDLE', 'New'or other unknown IDs. An 'IDLE' packet
+		indicates that there's no finger and no button activity.
+		A 'NEW' packet indicates the finger position in packet
+		is not continues from previous packet. Such as the
+		condition there's finger placed or lifted. In these cases,
+		finger attributes will be reset.*/
+		memset(priv->pt_attr, 0, sizeof(priv->pt_attr[0]) * 2);
+		break;
+	}
+}
+
+static void alps_cal_output_finger_num_v7(struct psmouse *psmouse,
+					struct alps_fields *f)
+{
+	struct alps_data *priv = psmouse->private;
+	unsigned int fn = 0;
+	int i;
+
+	switch (priv->r.v7.pkt_id) {
+	case V7_PACKET_ID_IDLE:
+	case V7_PACKET_ID_NEW:
+		/*No finger is reported when packet ID is 'IDLE' or 'New'.
+		An 'IDLE' packet indicates that there's no finger on touchpad.
+		A 'NEW' packet indicates there's finger placed or lifted.
+		Finger position of 'New' packet is not continues from the
+		previous packet.*/
+		fn = 0;
+		break;
+	case V7_PACKET_ID_TWO:
+		if (f->pt_img[0].z == 0) {
+			/*The first finger slot is zero when a non-resting
+			finger lifted and remaining only one resting finger
+			on touchpad. Hardware report the remaining resting
+			finger in second slot. This resting is ignored*/
+			fn = 0;
+		} else if (f->pt_img[1].z == 0) {
+			/* The second finger slot is zero if there's
+			only one finger*/
+			fn = 1;
+		} else {
+			/*All non-resting fingers will be counted to report*/
+			fn = 0;
+			for (i = 0; i < V7_IMG_PT_NUM; i++) {
+				if (priv->pt_attr[i].is_counted)
+					fn++;
+			}
+
+			/*In the case that both fingers are
+			resting fingers, report the first one*/
+			if (!priv->pt_attr[0].is_counted &&
+			    !priv->pt_attr[1].is_counted) {
+				fn = 1;
+			}
+		}
+		break;
+	case V7_PACKET_ID_MULTI:
+		/*A packet ID 'MULTI' indicats that at least 3 non-resting
+		finger exist.*/
+		fn = 3 + priv->r.v7.additional_fingers;
+		break;
+	}
+
+	f->fingers = fn;
+}
+
+static void alps_assign_buttons_v7(struct psmouse *psmouse,
+				   struct alps_fields *f)
+{
+	struct alps_data *priv = psmouse->private;
+
+	if (priv->phy_btn) {
+		if (!priv->prev_phy_btn) {
+			/* Report a right click as long as there's finger on
+			right button zone. Othrewise, report a left click.*/
+			if (priv->r.v7.rest_right ||
+			    priv->pt_attr[0].zone & ZONE_RIGHT_BTN ||
+			    priv->pt_attr[1].zone & ZONE_RIGHT_BTN) {
+				f->btn.right = 1;
+				priv->pressed_btn_bits |= RIGHT_BUTTON_BIT;
+			} else {
+				f->btn.left = 1;
+				priv->pressed_btn_bits |= LEFT_BUTTON_BIT;
+			}
+		} else {
+			if (priv->pressed_btn_bits & RIGHT_BUTTON_BIT)
+				f->btn.right = 1;
+			if (priv->pressed_btn_bits & LEFT_BUTTON_BIT)
+				f->btn.left = 1;
+		}
+	} else {
+		priv->pressed_btn_bits = 0;
+		f->btn.right = 0;
+		f->btn.left = 0;
+	}
+
+	priv->prev_phy_btn = priv->phy_btn;
+}
+
+static void alps_process_packet_v7(struct psmouse *psmouse)
+{
+	struct alps_data *priv = psmouse->private;
+	struct alps_fields f = {0};
+	unsigned char *packet = psmouse->packet;
+
+	priv->decode_fields(&f, packet, psmouse);
+
+	if (alps_drop_unsupported_packet_v7(psmouse))
+		return;
+
+	alps_set_pt_attr_v7(psmouse, &f);
+
+	alps_cal_output_finger_num_v7(psmouse, &f);
+
+	alps_assign_buttons_v7(psmouse, &f);
+
+	alps_report_coord_and_btn(psmouse, &f);
+}
+
 static void alps_report_bare_ps2_packet(struct psmouse *psmouse,
 					unsigned char packet[],
 					bool report_buttons)
@@ -1080,6 +1420,14 @@ static psmouse_ret_t alps_process_byte(struct psmouse *psmouse)
 		return PSMOUSE_BAD_DATA;
 	}
 
+	if ((priv->proto_version == ALPS_PROTO_V7 &&
+	    !alps_is_valid_package_v7(psmouse))) {
+		psmouse_dbg(psmouse, "refusing packet[%i] = %x\n",
+			    psmouse->pktcnt - 1,
+			    psmouse->packet[psmouse->pktcnt - 1]);
+		return PSMOUSE_BAD_DATA;
+	}
+
 	if (psmouse->pktcnt == psmouse->pktsize) {
 		priv->process_packet(psmouse);
 		return PSMOUSE_FULL_PACKET;
@@ -1192,6 +1540,22 @@ static int alps_rpt_cmd(struct psmouse *psmouse, int init_command,
 	return 0;
 }
 
+static int alps_check_valid_firmware_id(unsigned char id[])
+{
+	int valid = 1;
+
+	if (id[0] == 0x73)
+		valid = 1;
+	else if (id[0] == 0x88) {
+		if ((id[1] == 0x07) ||
+		    (id[1] == 0x08) ||
+		    ((id[1] & 0xf0) == 0xB0))
+			valid = 1;
+	}
+
+	return valid;
+}
+
 static int alps_enter_command_mode(struct psmouse *psmouse)
 {
 	unsigned char param[4];
@@ -1201,8 +1565,7 @@ static int alps_enter_command_mode(struct psmouse *psmouse)
 		return -1;
 	}
 
-	if ((param[0] != 0x88 || (param[1] != 0x07 && param[1] != 0x08)) &&
-	    param[0] != 0x73) {
+	if (!alps_check_valid_firmware_id(param)) {
 		psmouse_dbg(psmouse,
 			    "unknown response while entering command mode\n");
 		return -1;
@@ -1704,6 +2067,32 @@ error:
 	return ret;
 }
 
+static int alps_hw_init_v7(struct psmouse *psmouse)
+{
+	struct ps2dev *ps2dev = &psmouse->ps2dev;
+	int reg_val, ret = -1;
+
+	if (alps_enter_command_mode(psmouse) ||
+	    alps_command_mode_read_reg(psmouse, 0xc2d9) == -1)
+		goto error;
+
+	if (alps_command_mode_write_reg(psmouse, 0xc2c9, 0x64))
+		goto error;
+
+	reg_val = alps_command_mode_read_reg(psmouse, 0xc2c4);
+	if (reg_val == -1)
+		goto error;
+	if (__alps_command_mode_write_reg(psmouse, reg_val | 0x02))
+		goto error;
+
+	alps_exit_command_mode(psmouse);
+	return ps2_command(ps2dev, NULL, PSMOUSE_CMD_ENABLE);
+
+error:
+	alps_exit_command_mode(psmouse);
+	return ret;
+}
+
 /* Must be in command mode when calling this function */
 static int alps_absolute_mode_v4(struct psmouse *psmouse)
 {
@@ -1875,6 +2264,7 @@ static void alps_set_defaults(struct alps_data *priv)
 		priv->set_abs_params = alps_set_abs_params_st;
 		priv->x_max = 1023;
 		priv->y_max = 767;
+		priv->slot_number = 1;
 		break;
 	case ALPS_PROTO_V3:
 		priv->hw_init = alps_hw_init_v3;
@@ -1883,6 +2273,7 @@ static void alps_set_defaults(struct alps_data *priv)
 		priv->decode_fields = alps_decode_pinnacle;
 		priv->nibble_commands = alps_v3_nibble_commands;
 		priv->addr_command = PSMOUSE_CMD_RESET_WRAP;
+		priv->slot_number = 2;
 		break;
 	case ALPS_PROTO_V4:
 		priv->hw_init = alps_hw_init_v4;
@@ -1890,6 +2281,7 @@ static void alps_set_defaults(struct alps_data *priv)
 		priv->set_abs_params = alps_set_abs_params_mt;
 		priv->nibble_commands = alps_v4_nibble_commands;
 		priv->addr_command = PSMOUSE_CMD_DISABLE;
+		priv->slot_number = 2;
 		break;
 	case ALPS_PROTO_V5:
 		priv->hw_init = alps_hw_init_dolphin_v1;
@@ -1905,6 +2297,7 @@ static void alps_set_defaults(struct alps_data *priv)
 		priv->y_max = 660;
 		priv->x_bits = 23;
 		priv->y_bits = 12;
+		priv->slot_number = 2;
 		break;
 	case ALPS_PROTO_V6:
 		priv->hw_init = alps_hw_init_v6;
@@ -1913,6 +2306,22 @@ static void alps_set_defaults(struct alps_data *priv)
 		priv->nibble_commands = alps_v6_nibble_commands;
 		priv->x_max = 2047;
 		priv->y_max = 1535;
+		priv->slot_number = 2;
+		break;
+	case ALPS_PROTO_V7:
+		priv->hw_init = alps_hw_init_v7;
+		priv->process_packet = alps_process_packet_v7;
+		priv->decode_fields = alps_decode_packet_v7;
+		priv->set_abs_params = alps_set_abs_params_mt;
+		priv->nibble_commands = alps_v3_nibble_commands;
+		priv->addr_command = PSMOUSE_CMD_RESET_WRAP;
+		priv->x_max = 0xfff;
+		priv->y_max = 0x7ff;
+		priv->resting_zone_y_min = 0x654;
+		priv->byte0 = 0x48;
+		priv->mask0 = 0x48;
+		priv->flags = 0;
+		priv->slot_number = 2;
 		break;
 	}
 }
@@ -1982,6 +2391,11 @@ static int alps_identify(struct psmouse *psmouse, struct alps_data *priv)
 			return -EIO;
 		else
 			return 0;
+	} else if (ec[0] == 0x88 && (ec[1] & 0xf0) == 0xB0) {
+		priv->proto_version = ALPS_PROTO_V7;
+		alps_set_defaults(priv);
+
+		return 0;
 	} else if (ec[0] == 0x88 && ec[1] == 0x08) {
 		priv->proto_version = ALPS_PROTO_V3;
 		alps_set_defaults(priv);
@@ -2045,7 +2459,7 @@ static void alps_set_abs_params_mt(struct alps_data *priv,
 				   struct input_dev *dev1)
 {
 	set_bit(INPUT_PROP_SEMI_MT, dev1->propbit);
-	input_mt_init_slots(dev1, 2, 0);
+	input_mt_init_slots(dev1, priv->slot_number, 0);
 	input_set_abs_params(dev1, ABS_MT_POSITION_X, 0, priv->x_max, 0, 0);
 	input_set_abs_params(dev1, ABS_MT_POSITION_Y, 0, priv->y_max, 0, 0);
 
diff --git a/drivers/input/mouse/alps.h b/drivers/input/mouse/alps.h
index 03f88b6..5d2f9ea 100644
--- a/drivers/input/mouse/alps.h
+++ b/drivers/input/mouse/alps.h
@@ -18,11 +18,36 @@
 #define ALPS_PROTO_V4	4
 #define ALPS_PROTO_V5	5
 #define ALPS_PROTO_V6	6
+#define ALPS_PROTO_V7	7
+
+#define MAX_IMG_PT_NUM		5
+#define V7_IMG_PT_NUM		2
+
+#define ZONE_NORMAL				0x01
+#define ZONE_RESTING			0x02
+#define ZONE_LEFT_BTN			0x04
+#define ZONE_RIGHT_BTN			0x08
 
 #define DOLPHIN_COUNT_PER_ELECTRODE	64
 #define DOLPHIN_PROFILE_XOFFSET		8	/* x-electrode offset */
 #define DOLPHIN_PROFILE_YOFFSET		1	/* y-electrode offset */
 
+/*
+ * enum V7_PACKET_ID - defines the packet type for V7
+ * V7_PACKET_ID_IDLE: There's no finger and no button activity.
+ * V7_PACKET_ID_TWO: There's one or two non-resting fingers on touchpad
+ *  or there's button activities.
+ * V7_PACKET_ID_MULTI: There are at least three non-resting fingers.
+ * V7_PACKET_ID_NEW: The finger position in slot is not continues from
+ *  previous packet.
+*/
+enum V7_PACKET_ID {
+	 V7_PACKET_ID_IDLE,
+	 V7_PACKET_ID_TWO,
+	 V7_PACKET_ID_MULTI,
+	 V7_PACKET_ID_NEW,
+};
+
 /**
  * struct alps_model_info - touchpad ID table
  * @signature: E7 response string to match.
@@ -66,15 +91,7 @@ struct alps_nibble_commands {
 };
 
 /**
- * struct alps_fields - decoded version of the report packet
- * @x_map: Bitmap of active X positions for MT.
- * @y_map: Bitmap of active Y positions for MT.
- * @fingers: Number of fingers for MT.
- * @x: X position for ST.
- * @y: Y position for ST.
- * @z: Z position for ST.
- * @first_mp: Packet is the first of a multi-packet report.
- * @is_mp: Packet is part of a multi-packet report.
+ * struct alps_btn - decoded version of the button status
  * @left: Left touchpad button is active.
  * @right: Right touchpad button is active.
  * @middle: Middle touchpad button is active.
@@ -82,16 +99,7 @@ struct alps_nibble_commands {
  * @ts_right: Right trackstick button is active.
  * @ts_middle: Middle trackstick button is active.
  */
-struct alps_fields {
-	unsigned int x_map;
-	unsigned int y_map;
-	unsigned int fingers;
-	unsigned int x;
-	unsigned int y;
-	unsigned int z;
-	unsigned int first_mp:1;
-	unsigned int is_mp:1;
-
+struct alps_btn {
 	unsigned int left:1;
 	unsigned int right:1;
 	unsigned int middle:1;
@@ -102,6 +110,69 @@ struct alps_fields {
 };
 
 /**
+ * struct alps_btn - decoded version of the X Y Z postion for ST.
+ * @x: X position for ST.
+ * @y: Y position for ST.
+ * @z: Z position for ST.
+ */
+struct alps_abs_data {
+	unsigned int x;
+	unsigned int y;
+	unsigned int z;
+};
+
+/**
+ * struct alps_fields - decoded version of the report packet
+ * @fingers: Number of fingers for MT.
+ * @pt: X Y Z postion for ST.
+ * @pt: X Y Z postion for image MT.
+ * @x_map: Bitmap of active X positions for MT.
+ * @y_map: Bitmap of active Y positions for MT.
+ * @first_mp: Packet is the first of a multi-packet report.
+ * @is_mp: Packet is part of a multi-packet report.
+ * @btn: Button activity status
+ */
+struct alps_fields {
+	unsigned int fingers;
+	struct alps_abs_data pt;
+	struct alps_abs_data pt_img[MAX_IMG_PT_NUM];
+	unsigned int x_map;
+	unsigned int y_map;
+	unsigned int first_mp:1;
+	unsigned int is_mp:1;
+	struct alps_btn btn;
+};
+
+/**
+ * struct v7_raw - data decoded from raw packet for V7.
+ * @pkt_id: An id that specifies the type of packet.
+ * @additional_fingers: Number of additional finger that is neighter included
+ *  in pt slot nor reflected in rest_left and rest_right flag of data packet.
+ * @rest_left: There are fingers on left resting zone.
+ * @rest_right: There are fingers on right resting zone.
+ */
+struct v7_raw {
+	unsigned char pkt_id;
+	unsigned int additional_fingers;
+	unsigned char rest_left;
+	unsigned char rest_right;
+};
+
+/**
+ * struct alps_bl_pt_attr - generic attributes of touch points for buttonless device
+ * @zone: The part of touchpad that the touch point locates
+ * @is_counted: The touch point is not a resting finger.
+ * @is_init_pt_got: The touch down point is got.
+ * @init_pt: The X Y Z position of the touch down point.
+ */
+struct alps_bl_pt_attr {
+	unsigned char zone;
+	unsigned char is_counted;
+	unsigned char is_init_pt_got;
+	struct alps_abs_data init_pt;
+};
+
+/**
  * struct alps_data - private data structure for the ALPS driver
  * @dev2: "Relative" device used to report trackstick or mouse activity.
  * @phys: Physical path for the relative device.
@@ -116,8 +187,10 @@ struct alps_fields {
  * @flags: Additional device capabilities (passthrough port, trackstick, etc.).
  * @x_max: Largest possible X position value.
  * @y_max: Largest possible Y position value.
+ * @resting_zone_y_min: Smallest Y postion value of the bottom resting zone.
  * @x_bits: Number of X bits in the MT bitmap.
  * @y_bits: Number of Y bits in the MT bitmap.
+ * @img_fingers: Number of image fingers.
  * @hw_init: Protocol-specific hardware init function.
  * @process_packet: Protocol-specific function to process a report packet.
  * @decode_fields: Protocol-specific function to read packet bitfields.
@@ -132,6 +205,11 @@ struct alps_fields {
  * @fingers: Number of fingers from last MT report.
  * @quirks: Bitmap of ALPS_QUIRK_*.
  * @timer: Timer for flushing out the final report packet in the stream.
+ * @v7: Data decoded from raw packet for V7
+ * @phy_btn: Physical button is active.
+ * @prev_phy_btn: Physical button of previous packet is active.
+ * @pressed_btn_bits: Pressed positon of button zone
+ * @pt_attr: Generic attributes of touch points for buttonless device.
  */
 struct alps_data {
 	struct input_dev *dev2;
@@ -145,8 +223,10 @@ struct alps_data {
 	unsigned char flags;
 	int x_max;
 	int y_max;
+	int resting_zone_y_min;
 	int x_bits;
 	int y_bits;
+	unsigned char slot_number;
 
 	int (*hw_init)(struct psmouse *psmouse);
 	void (*process_packet)(struct psmouse *psmouse);
@@ -161,6 +241,15 @@ struct alps_data {
 	int fingers;
 	u8 quirks;
 	struct timer_list timer;
+
+	/* these are used for buttonless touchpad*/
+	union {
+		struct v7_raw v7;
+	} r;
+	unsigned char phy_btn;
+	unsigned char prev_phy_btn;
+	unsigned char pressed_btn_bits;
+	struct alps_bl_pt_attr pt_attr[MAX_IMG_PT_NUM];
 };
 
 #define ALPS_QUIRK_TRACKSTICK_BUTTONS	1 /* trakcstick buttons in trackstick packet */
-- 
1.8.3.2


^ permalink raw reply related

* [PATCH] input: synaptics-rmi4 - rmi_driver.h tidy up
From: Christopher Heiny @ 2014-01-10  1:59 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Linux Input, Christopher Heiny, Andrew Duggan, Vincent Huang,
	Vivian Ly, Daniel Rosenberg, Jean Delvare, Joerie de Gram,
	Linus Walleij, Benjamin Tissoires

This is a trivial change to remove an unused macro, and change CONFIG_PM to
CONFIG_PM_SLEEP.

Signed-off-by: Christopher Heiny <cheiny@synaptics.com>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>

---

 drivers/input/rmi4/rmi_driver.h | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/input/rmi4/rmi_driver.h b/drivers/input/rmi4/rmi_driver.h
index 4d82b90..ad3ee33 100644
--- a/drivers/input/rmi4/rmi_driver.h
+++ b/drivers/input/rmi4/rmi_driver.h
@@ -24,8 +24,6 @@
 	.attrs = _attrs,  \
 }
 
-#define attrify(nm) (&dev_attr_##nm.attr)
-
 #define PDT_PROPERTIES_LOCATION 0x00EF
 #define BSR_LOCATION 0x00FE
 
@@ -62,7 +60,7 @@ struct rmi_driver_data {
 	u8 bsr;
 
 	bool enabled;
-#ifdef CONFIG_PM
+#ifdef CONFIG_PM_SLEEP
 	bool suspended;
 	struct mutex suspend_mutex;
 

^ permalink raw reply related

* [PATCH 1/2] input: appletouch: parametrize and set saner defaults for fuzz and threshold
From: Clinton Sprain @ 2014-01-10  0:01 UTC (permalink / raw)
  To: linux-input; +Cc: Dmitry Torokhov, Henrik Rydberg
In-Reply-To: <52CF37E1.3030809@gmail.com>

From: Clinton Sprain <clintonsprain@gmail.com>

Dials back the default fuzz and threshold settings for most devices using
this driver, based on values from user feedback from forums and bug reports.
This increases smoothness and movement granularity. No changes were made for
the older devices that use the driver, as I did not find any user feedback
for them regarding these settings; however, the two settings can also now
both be specified as module parameters in case there is a desire to change
the values for devices that do not have new defaults.

There are also a couple of style cleanups per checkpatch.pl.

Signed-off-by: Clinton Sprain <clintonsprain@gmail.com>
---
 drivers/input/mouse/appletouch.c |   66 ++++++++++++++++++++++++--------------
 1 file changed, 42 insertions(+), 24 deletions(-)

diff --git a/drivers/input/mouse/appletouch.c b/drivers/input/mouse/appletouch.c
index e42f1fa..ba6bbd7 100644
--- a/drivers/input/mouse/appletouch.c
+++ b/drivers/input/mouse/appletouch.c
@@ -43,12 +43,14 @@
  */
 struct atp_info {
 	int xsensors;				/* number of X sensors */
-	int xsensors_17;			/* 17" models have more sensors */
+	int xsensors_17;			/* 17" model has more sensors */
 	int ysensors;				/* number of Y sensors */
 	int xfact;				/* X multiplication factor */
 	int yfact;				/* Y multiplication factor */
 	int datalen;				/* size of USB transfers */
 	void (*callback)(struct urb *);		/* callback function */
+	int fuzz;				/* fuzz touchpad generates */
+	int threshold;				/* sensitivity threshold */
 };

 static void atp_complete_geyser_1_2(struct urb *urb);
@@ -62,6 +64,8 @@ static const struct atp_info fountain_info = {
 	.yfact		= 43,
 	.datalen	= 81,
 	.callback	= atp_complete_geyser_1_2,
+	.fuzz		= 16,
+	.threshold	= 5,
 };

 static const struct atp_info geyser1_info = {
@@ -72,6 +76,8 @@ static const struct atp_info geyser1_info = {
 	.yfact		= 43,
 	.datalen	= 81,
 	.callback	= atp_complete_geyser_1_2,
+	.fuzz		= 16,
+	.threshold	= 5,
 };

 static const struct atp_info geyser2_info = {
@@ -82,6 +88,8 @@ static const struct atp_info geyser2_info = {
 	.yfact		= 43,
 	.datalen	= 64,
 	.callback	= atp_complete_geyser_1_2,
+	.fuzz		= 0,
+	.threshold	= 3,
 };

 static const struct atp_info geyser3_info = {
@@ -91,6 +99,8 @@ static const struct atp_info geyser3_info = {
 	.yfact		= 64,
 	.datalen	= 64,
 	.callback	= atp_complete_geyser_3_4,
+	.fuzz		= 0,
+	.threshold	= 3,
 };

 static const struct atp_info geyser4_info = {
@@ -100,6 +110,8 @@ static const struct atp_info geyser4_info = {
 	.yfact		= 64,
 	.datalen	= 64,
 	.callback	= atp_complete_geyser_3_4,
+	.fuzz		= 0,
+	.threshold	= 2,
 };

 #define ATP_DEVICE(prod, info)					\
@@ -156,18 +168,9 @@ MODULE_DEVICE_TABLE(usb, atp_table);
 #define ATP_XSENSORS	26
 #define ATP_YSENSORS	16

-/* amount of fuzz this touchpad generates */
-#define ATP_FUZZ	16
-
 /* maximum pressure this driver will report */
 #define ATP_PRESSURE	300

-/*
- * Threshold for the touchpad sensors. Any change less than ATP_THRESHOLD is
- * ignored.
- */
-#define ATP_THRESHOLD	 5
-
 /* Geyser initialization constants */
 #define ATP_GEYSER_MODE_READ_REQUEST_ID		1
 #define ATP_GEYSER_MODE_WRITE_REQUEST_ID	9
@@ -213,14 +216,16 @@ struct atp {
 	struct work_struct	work;
 };

-#define dbg_dump(msg, tab) \
+#define dbg_dump(msg, tab)						\
+{									\
 	if (debug > 1) {						\
 		int __i;						\
 		printk(KERN_DEBUG "appletouch: %s", msg);		\
 		for (__i = 0; __i < ATP_XSENSORS + ATP_YSENSORS; __i++)	\
 			printk(" %02x", tab[__i]);			\
 		printk("\n");						\
-	}
+	}								\
+}

 #define dprintk(format, a...)						\
 	do {								\
@@ -236,14 +241,20 @@ MODULE_AUTHOR("Sven Anders");
 MODULE_DESCRIPTION("Apple PowerBook and MacBook USB touchpad driver");
 MODULE_LICENSE("GPL");

-/*
- * Make the threshold a module parameter
- */
-static int threshold = ATP_THRESHOLD;
+static int threshold = -1;
 module_param(threshold, int, 0644);
 MODULE_PARM_DESC(threshold, "Discard any change in data from a sensor"
 			    " (the trackpad has many of these sensors)"
-			    " less than this value.");
+			    " less than this value. Devices have defaults;"
+			    " this parameter overrides them.");
+static int fuzz = -1;
+
+module_param(fuzz, int, 0644);
+MODULE_PARM_DESC(fuzz, "Fuzz the trackpad generates. The higher"
+		       " this is, the less sensitive the trackpad"
+		       " will feel, but values too low may generate"
+		       " random movement. Devices have defaults;"
+		       " this parameter overrides them.");

 static int debug;
 module_param(debug, int, 0644);
@@ -363,7 +374,8 @@ static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
 		    (!is_increasing && xy_sensors[i - 1] < xy_sensors[i])) {
 			(*fingers)++;
 			is_increasing = 1;
-		} else if (i > 0 && (xy_sensors[i - 1] - xy_sensors[i] > threshold)) {
+		} else if (i > 0 && (xy_sensors[i - 1] -
+			xy_sensors[i] > threshold)) {
 			is_increasing = 0;
 		}

@@ -456,7 +468,7 @@ static void atp_detect_size(struct atp *dev)
 			input_set_abs_params(dev->input, ABS_X, 0,
 					     (dev->info->xsensors_17 - 1) *
 							dev->info->xfact - 1,
-					     ATP_FUZZ, 0);
+					     fuzz, 0);
 			break;
 		}
 	}
@@ -513,7 +525,8 @@ static void atp_complete_geyser_1_2(struct urb *urb)

 			/* Y values */
 			dev->xy_cur[ATP_XSENSORS + i] = dev->data[5 * i +  1];
-			dev->xy_cur[ATP_XSENSORS + i + 8] = dev->data[5 * i + 3];
+			dev->xy_cur[ATP_XSENSORS + i + 8] =
+				dev->data[5 * i + 3];
 		}
 	}

@@ -809,12 +822,17 @@ static int atp_probe(struct usb_interface *iface,
 	dev->info = info;
 	dev->overflow_warned = false;

+	if (fuzz < 0)
+		fuzz = dev->info->fuzz;
+	if (threshold < 0)
+		threshold = dev->info->threshold;
+
 	dev->urb = usb_alloc_urb(0, GFP_KERNEL);
 	if (!dev->urb)
 		goto err_free_devs;

-	dev->data = usb_alloc_coherent(dev->udev, dev->info->datalen, GFP_KERNEL,
-				       &dev->urb->transfer_dma);
+	dev->data = usb_alloc_coherent(dev->udev, dev->info->datalen,
+					GFP_KERNEL, &dev->urb->transfer_dma);
 	if (!dev->data)
 		goto err_free_urb;

@@ -844,10 +862,10 @@ static int atp_probe(struct usb_interface *iface,

 	input_set_abs_params(input_dev, ABS_X, 0,
 			     (dev->info->xsensors - 1) * dev->info->xfact - 1,
-			     ATP_FUZZ, 0);
+			     fuzz, 0);
 	input_set_abs_params(input_dev, ABS_Y, 0,
 			     (dev->info->ysensors - 1) * dev->info->yfact - 1,
-			     ATP_FUZZ, 0);
+			     fuzz, 0);
 	input_set_abs_params(input_dev, ABS_PRESSURE, 0, ATP_PRESSURE, 0, 0);

 	set_bit(EV_KEY, input_dev->evbit);
--
1.7.9.5


^ permalink raw reply related

* [PATCH 2/2] input: appletouch: use better cursor movement smoothing algorithm
From: Clinton Sprain @ 2014-01-10  0:02 UTC (permalink / raw)
  To: linux-input; +Cc: Dmitry Torokhov, Henrik Rydberg
In-Reply-To: <52CF37E1.3030809@gmail.com>

From: Clinton Sprain <clintonsprain@gmail.com>

Implements a new algorithm used to smooth cursor movement, using the inertia
of the cursor's prior movements to modulate the next cursor movement. This
further mitigates the 'stepping' users see when moving the cursor diagonally.
A 'legacy' module parameter has been added in case the older devices do not
like the new algorithm.

Signed-Off-By: Clinton Sprain <clintonsprain@gmail.com>
---
 drivers/input/mouse/appletouch.c |  118 ++++++++++++++++++++++++++++++++------
 1 file changed, 100 insertions(+), 18 deletions(-)

diff --git a/drivers/input/mouse/appletouch.c b/drivers/input/mouse/appletouch.c
index ba6bbd7..7a33188 100644
--- a/drivers/input/mouse/appletouch.c
+++ b/drivers/input/mouse/appletouch.c
@@ -209,13 +209,16 @@ struct atp {
 	bool			overflow_warned;
 	int			x_old;		/* last reported x/y, */
 	int			y_old;		/* used for smoothing */
+	int			x_older;	/* 2nd to last reported x/y,*/
+	int			y_older;	/* used for smoothing */
+	int			x_oldest;       /* 3rd to last reported x/y,*/
+	int			y_oldest;       /* used for smoothing */
 	signed char		xy_cur[ATP_XSENSORS + ATP_YSENSORS];
 	signed char		xy_old[ATP_XSENSORS + ATP_YSENSORS];
 	int			xy_acc[ATP_XSENSORS + ATP_YSENSORS];
 	int			idlecount;	/* number of empty packets */
 	struct work_struct	work;
 };
-
 #define dbg_dump(msg, tab)						\
 {									\
 	if (debug > 1) {						\
@@ -260,6 +263,13 @@ static int debug;
 module_param(debug, int, 0644);
 MODULE_PARM_DESC(debug, "Activate debugging output");

+static int legacy;
+module_param(legacy, int, 0644);
+MODULE_PARM_DESC(legacy, "1 = Use old cursor movement smoothing."
+			 " Older behavior averages current with"
+			 " last cursor position. New behavior"
+			 " uses cursor movement inertia.");
+
 /*
  * By default newer Geyser devices send standard USB HID mouse
  * packets (Report ID 2). This code changes device mode, so it
@@ -338,6 +348,59 @@ static void atp_reinit(struct work_struct *work)
 			retval);
 }

+static int atp_smooth_legacy(int curr, int old)
+{
+	return ((old * 3) + curr) >> 2;
+}
+
+static int atp_smooth(int curr, int old, int older, int oldest)
+{
+	int diff, projected, olderprojected, smooth;
+
+	diff = old - older;
+	projected = old + diff;
+
+	diff = older - oldest;
+	olderprojected = older + diff;
+
+	smooth = (projected * 7 + olderprojected * 8 + curr) >> 4;
+
+	return smooth;
+}
+
+static void atp_refresh_old_xy(int x, int y, struct atp *dev)
+{
+	if (legacy != true) {
+		dev->x_oldest = dev->x_older;
+		dev->y_oldest = dev->y_older;
+		dev->x_older = dev->x_old;
+		dev->y_older = dev->y_old;
+	}
+	dev->x_old = x;
+	dev->y_old = y;
+}
+
+static void atp_reset_old_xy(struct atp *dev)
+{
+	if (legacy != true) {
+		dev->x_oldest = dev->y_oldest = -1;
+		dev->x_older = dev->y_older = -1;
+	}
+	dev->x_old = dev->y_old = -1;
+}
+
+static void atp_default_old_xy(struct atp *dev)
+{
+	if (dev->x_older == -1)
+		dev->x_older = dev->x_old;
+	if (dev->y_older == -1)
+		dev->y_older = dev->y_old;
+	if (dev->x_oldest == -1)
+		dev->x_oldest = dev->x_older;
+	if (dev->y_oldest == -1)
+		dev->y_oldest = dev->y_older;
+}
+
 static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
 			     int *z, int *fingers)
 {
@@ -386,8 +449,13 @@ static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
 		 * occasionally jump a number of pixels (slowly moving the
 		 * finger makes this issue most apparent.)
 		 */
-		pcum += (xy_sensors[i] - threshold) * i;
-		psum += (xy_sensors[i] - threshold);
+		if (legacy == true) {
+			pcum += (xy_sensors[i] - threshold) * i;
+			psum += (xy_sensors[i] - threshold);
+		} else {
+			pcum += (xy_sensors[i]) * i;
+			psum += (xy_sensors[i]);
+		}
 	}

 	if (psum > 0) {
@@ -570,10 +638,18 @@ static void atp_complete_geyser_1_2(struct urb *urb)

 	if (x && y) {
 		if (dev->x_old != -1) {
-			x = (dev->x_old * 3 + x) >> 2;
-			y = (dev->y_old * 3 + y) >> 2;
-			dev->x_old = x;
-			dev->y_old = y;
+			if (legacy == true) {
+				x = atp_smooth_legacy(x, dev->x_old);
+				y = atp_smooth_legacy(y, dev->y_old);
+			} else {
+
+				atp_default_old_xy(dev);
+
+				x = atp_smooth(x, dev->x_old,
+					dev->x_older, dev->x_oldest);
+				y = atp_smooth(y, dev->y_old,
+					dev->y_older, dev->y_oldest);
+			}

 			if (debug > 1)
 				printk(KERN_DEBUG "appletouch: "
@@ -587,12 +663,11 @@ static void atp_complete_geyser_1_2(struct urb *urb)
 					 min(ATP_PRESSURE, x_z + y_z));
 			atp_report_fingers(dev->input, max(x_f, y_f));
 		}
-		dev->x_old = x;
-		dev->y_old = y;
+		atp_refresh_old_xy(x, y, dev);

 	} else if (!x && !y) {

-		dev->x_old = dev->y_old = -1;
+		atp_reset_old_xy(dev);
 		input_report_key(dev->input, BTN_TOUCH, 0);
 		input_report_abs(dev->input, ABS_PRESSURE, 0);
 		atp_report_fingers(dev->input, 0);
@@ -682,10 +757,18 @@ static void atp_complete_geyser_3_4(struct urb *urb)

 	if (x && y) {
 		if (dev->x_old != -1) {
-			x = (dev->x_old * 3 + x) >> 2;
-			y = (dev->y_old * 3 + y) >> 2;
-			dev->x_old = x;
-			dev->y_old = y;
+			if (legacy == true) {
+				x = atp_smooth_legacy(x, dev->x_old);
+				y = atp_smooth_legacy(y, dev->y_old);
+			} else {
+
+				atp_default_old_xy(dev);
+
+				x = atp_smooth(x, dev->x_old,
+					dev->x_older, dev->x_oldest);
+				y = atp_smooth(y, dev->y_old,
+					dev->y_older, dev->y_oldest);
+			}

 			if (debug > 1)
 				printk(KERN_DEBUG "appletouch: X: %3d Y: %3d "
@@ -699,12 +782,11 @@ static void atp_complete_geyser_3_4(struct urb *urb)
 					 min(ATP_PRESSURE, x_z + y_z));
 			atp_report_fingers(dev->input, max(x_f, y_f));
 		}
-		dev->x_old = x;
-		dev->y_old = y;
+		atp_refresh_old_xy(x, y, dev);

 	} else if (!x && !y) {

-		dev->x_old = dev->y_old = -1;
+		atp_reset_old_xy(dev);
 		input_report_key(dev->input, BTN_TOUCH, 0);
 		input_report_abs(dev->input, ABS_PRESSURE, 0);
 		atp_report_fingers(dev->input, 0);
@@ -730,7 +812,7 @@ static void atp_complete_geyser_3_4(struct urb *urb)
 	if (!x && !y && !key) {
 		dev->idlecount++;
 		if (dev->idlecount == 10) {
-			dev->x_old = dev->y_old = -1;
+			atp_reset_old_xy(dev);
 			dev->idlecount = 0;
 			schedule_work(&dev->work);
 			/* Don't resubmit urb here, wait for reinit */
--
1.7.9.5


^ permalink raw reply related

* [PATCH 0/2] input: appletouch: fixes for jagged/uneven cursor movement
From: Clinton Sprain @ 2014-01-09 23:59 UTC (permalink / raw)
  To: linux-input; +Cc: Dmitry Torokhov, Henrik Rydberg

From: Clinton Sprain <clintonsprain@gmail.com>

For many devices, the appletouch driver can make the trackpad feel
insensitive and cursor movement will tend to jerk in steps. This is
particularly noticeable when moving the cursor diagonally. This can
greatly impact the trackpad's usability.

The proposed changes address this by dialing back the fuzz and threshold
parameters and by implementing a new cursor movement smoothing algorithm.

User feedback couldn't be found for some older devices that use this driver,
so module parameters have been added to allow users to manually set the
fuzz and threshold settings and/or opt out of the new smoothing algorithm.

Signed-off-by: Clinton Sprain <clintonsprain@gmail.com>
Tested-by: Curtis Rueden <ctrueden@gmail.com>

Clinton Sprain (2):
  input: appletouch: parametrize and set saner defaults for fuzz and
    threshold
  input: appletouch: use better cursor movement smoothing algorithm

 drivers/input/mouse/appletouch.c |  184 +++++++++++++++++++++++++++++---------
 1 file changed, 142 insertions(+), 42 deletions(-)

--
1.7.9.5

^ permalink raw reply

* Re: [PATCH v2] input: synaptics-rmi4 - use snprintf instead of sprintf in rmi_i2c.c
From: Christopher Heiny @ 2014-01-09 22:47 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Linux Input, Andrew Duggan, Vincent Huang, Vivian Ly,
	Daniel Rosenberg, Jean Delvare, Joerie de Gram, Linus Walleij,
	Benjamin Tissoires
In-Reply-To: <20140109222546.GA20994@core.coreip.homeip.net>

On 01/09/2014 02:25 PM, Dmitry Torokhov wrote:
> Input: synaptics-rmi4 - fix disabling gpio config in i2c transport
>
> From: Dmitry Torokhov<dmitry.torokhov@gmail.com>
>
> We need to pass into pdata->gpio_config() gpio_data which is already a
> pointer, and not its address.
>
> Signed-off-by: Dmitry Torokhov<dmitry.torokhov@gmail.com>
> ---
>   drivers/input/rmi4/rmi_i2c.c |    8 +++++---
>   1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/input/rmi4/rmi_i2c.c b/drivers/input/rmi4/rmi_i2c.c
> index ebe74ec..12aea8c 100644
> --- a/drivers/input/rmi4/rmi_i2c.c
> +++ b/drivers/input/rmi4/rmi_i2c.c
> @@ -168,7 +168,8 @@ exit:
>   static int rmi_i2c_probe(struct i2c_client *client,
>   			 const struct i2c_device_id *id)
>   {
> -	const struct rmi_device_platform_data *pdata = dev_get_platdata(&client->dev);
> +	const struct rmi_device_platform_data *pdata =
> +				dev_get_platdata(&client->dev);
>   	struct rmi_transport_dev *xport;
>   	struct rmi_i2c_data *data;
>   	int retval;
> @@ -250,12 +251,13 @@ err_gpio:
>   static int rmi_i2c_remove(struct i2c_client *client)
>   {
>   	struct rmi_transport_dev *xport = i2c_get_clientdata(client);
> -	struct rmi_device_platform_data *pdata = dev_get_platdata(&client->dev);
> +	const struct rmi_device_platform_data *pdata =
> +				dev_get_platdata(&client->dev);
>
>   	rmi_unregister_transport_device(xport);
>
>   	if (pdata->gpio_config)
> -		pdata->gpio_config(&pdata->gpio_data, false);
> +		pdata->gpio_config(pdata->gpio_data, false);
>
>   	return 0;
>   }

It's just plain freaky that no problem ever arose from that, but 
definitely it's a bug.

Acked-by: Christopher Heiny <cheiny@synaptics.com>

^ permalink raw reply

* Re: [PATCH v2] input: synaptics-rmi4 - use snprintf instead of sprintf in rmi_i2c.c
From: Dmitry Torokhov @ 2014-01-09 22:25 UTC (permalink / raw)
  To: Christopher Heiny
  Cc: Linux Input, Andrew Duggan, Vincent Huang, Vivian Ly,
	Daniel Rosenberg, Jean Delvare, Joerie de Gram, Linus Walleij,
	Benjamin Tissoires
In-Reply-To: <52CF1E74.6060202@synaptics.com>

On Thu, Jan 09, 2014 at 02:11:00PM -0800, Christopher Heiny wrote:
> On 01/09/2014 01:38 PM, Christopher Heiny wrote:
> >On 01/09/2014 01:29 PM, Dmitry Torokhov wrote:
> >>On Thu, Jan 09, 2014 at 01:23:37PM -0800, Christopher Heiny wrote:
> >>>>On 01/09/2014 12:04 AM, Dmitry Torokhov wrote:
> >>>>> >On Wed, Jan 08, 2014 at 05:18:39PM -0800, Christopher Heiny wrote:
> >>>>>> >>This is a trivial change to replace the sprintf loop with
> >>>>>snprintf using
> >>>>>> >>up-to-date format capability.
> >>>>> >
> >>>>> >Hmm, how about we do this instead:
> >>>>> >
> >>>>> >Input: synaptics-rmi4 - clean up debug handling in rmi_i2c
> >>>>> >
> >>>>> >From: Dmitry Torokhov<dmitry.torokhov@gmail.com>
> >>>>> >
> >>>>> >Kernel now has standard facility to format and print binary
> >>>>buffers, let's
> >>>>> >use it. By doing so we no longer need to allocate memory for
> >>>>debug buffers
> >>>>> >and we can let debugfs code go as well.
> >>>>
> >>>>Not sure where to put this comment, so I'll drop it here.
> >>>>
> >>>>I agree the buffers can go.  I realized that on the drive home last
> >>>>night, but was too tired to follow up.
> >>>>
> >>>>Talking with some of the folks who use this feature, there's a
> >>>>desire to keep some sort of finer control on whether the comms
> >>>>buffers are printed or not - either the existing debugfs setup
> >>>>(preferred, since it lets them turn on the dmesg clutter only when
> >>>>needed), or by converting to a config option such as
> >>>>CONFIG_RMI4_COMMS_DEBUG.  It's very useful in new platform
> >>>>development, since there's a surprising number of ways in which the
> >>>>reads and writes can go wonky on new hardware.
> > >
> >>That is why you have CONFIG_DYNAMIC_DEBUG: you can activate these debug
> >>statements at will using the common kernel mechanisms.
> >
> >I'll check this out and get back.
> 
> Looks CONFIG_DYNAMIC_DEBUG will work fine!
> 
> Acked-by: Christopher Heiny <cheiny@synaptics.com>

Cool, thanks! And another one:

Input: synaptics-rmi4 - fix disabling gpio config in i2c transport

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>

We need to pass into pdata->gpio_config() gpio_data which is already a
pointer, and not its address.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/rmi4/rmi_i2c.c |    8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/input/rmi4/rmi_i2c.c b/drivers/input/rmi4/rmi_i2c.c
index ebe74ec..12aea8c 100644
--- a/drivers/input/rmi4/rmi_i2c.c
+++ b/drivers/input/rmi4/rmi_i2c.c
@@ -168,7 +168,8 @@ exit:
 static int rmi_i2c_probe(struct i2c_client *client,
 			 const struct i2c_device_id *id)
 {
-	const struct rmi_device_platform_data *pdata = dev_get_platdata(&client->dev);
+	const struct rmi_device_platform_data *pdata =
+				dev_get_platdata(&client->dev);
 	struct rmi_transport_dev *xport;
 	struct rmi_i2c_data *data;
 	int retval;
@@ -250,12 +251,13 @@ err_gpio:
 static int rmi_i2c_remove(struct i2c_client *client)
 {
 	struct rmi_transport_dev *xport = i2c_get_clientdata(client);
-	struct rmi_device_platform_data *pdata = dev_get_platdata(&client->dev);
+	const struct rmi_device_platform_data *pdata =
+				dev_get_platdata(&client->dev);
 
 	rmi_unregister_transport_device(xport);
 
 	if (pdata->gpio_config)
-		pdata->gpio_config(&pdata->gpio_data, false);
+		pdata->gpio_config(pdata->gpio_data, false);
 
 	return 0;
 }

-- 
Dmitry

^ permalink raw reply related

* Re: [PATCH v2] input: synaptics-rmi4 - use snprintf instead of sprintf in rmi_i2c.c
From: Christopher Heiny @ 2014-01-09 22:11 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Linux Input, Andrew Duggan, Vincent Huang, Vivian Ly,
	Daniel Rosenberg, Jean Delvare, Joerie de Gram, Linus Walleij,
	Benjamin Tissoires
In-Reply-To: <52CF16B8.7000409@synaptics.com>

On 01/09/2014 01:38 PM, Christopher Heiny wrote:
> On 01/09/2014 01:29 PM, Dmitry Torokhov wrote:
>> On Thu, Jan 09, 2014 at 01:23:37PM -0800, Christopher Heiny wrote:
>>> >On 01/09/2014 12:04 AM, Dmitry Torokhov wrote:
>>>> > >On Wed, Jan 08, 2014 at 05:18:39PM -0800, Christopher Heiny wrote:
>>>>> > >>This is a trivial change to replace the sprintf loop with
>>>>> snprintf using
>>>>> > >>up-to-date format capability.
>>>> > >
>>>> > >Hmm, how about we do this instead:
>>>> > >
>>>> > >Input: synaptics-rmi4 - clean up debug handling in rmi_i2c
>>>> > >
>>>> > >From: Dmitry Torokhov<dmitry.torokhov@gmail.com>
>>>> > >
>>>> > >Kernel now has standard facility to format and print binary
>>>> buffers, let's
>>>> > >use it. By doing so we no longer need to allocate memory for
>>>> debug buffers
>>>> > >and we can let debugfs code go as well.
>>> >
>>> >Not sure where to put this comment, so I'll drop it here.
>>> >
>>> >I agree the buffers can go.  I realized that on the drive home last
>>> >night, but was too tired to follow up.
>>> >
>>> >Talking with some of the folks who use this feature, there's a
>>> >desire to keep some sort of finer control on whether the comms
>>> >buffers are printed or not - either the existing debugfs setup
>>> >(preferred, since it lets them turn on the dmesg clutter only when
>>> >needed), or by converting to a config option such as
>>> >CONFIG_RMI4_COMMS_DEBUG.  It's very useful in new platform
>>> >development, since there's a surprising number of ways in which the
>>> >reads and writes can go wonky on new hardware.
>  >
>> That is why you have CONFIG_DYNAMIC_DEBUG: you can activate these debug
>> statements at will using the common kernel mechanisms.
>
> I'll check this out and get back.

Looks CONFIG_DYNAMIC_DEBUG will work fine!

Acked-by: Christopher Heiny <cheiny@synaptics.com>

^ permalink raw reply

* Re: [PATCH v2] input: synaptics-rmi4 - use snprintf instead of sprintf in rmi_i2c.c
From: Christopher Heiny @ 2014-01-09 21:38 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Linux Input, Andrew Duggan, Vincent Huang, Vivian Ly,
	Daniel Rosenberg, Jean Delvare, Joerie de Gram, Linus Walleij,
	Benjamin Tissoires
In-Reply-To: <20140109212949.GA31257@core.coreip.homeip.net>

On 01/09/2014 01:29 PM, Dmitry Torokhov wrote:
> On Thu, Jan 09, 2014 at 01:23:37PM -0800, Christopher Heiny wrote:
>> >On 01/09/2014 12:04 AM, Dmitry Torokhov wrote:
>>> > >On Wed, Jan 08, 2014 at 05:18:39PM -0800, Christopher Heiny wrote:
>>>> > >>This is a trivial change to replace the sprintf loop with snprintf using
>>>> > >>up-to-date format capability.
>>> > >
>>> > >Hmm, how about we do this instead:
>>> > >
>>> > >Input: synaptics-rmi4 - clean up debug handling in rmi_i2c
>>> > >
>>> > >From: Dmitry Torokhov<dmitry.torokhov@gmail.com>
>>> > >
>>> > >Kernel now has standard facility to format and print binary buffers, let's
>>> > >use it. By doing so we no longer need to allocate memory for debug buffers
>>> > >and we can let debugfs code go as well.
>> >
>> >Not sure where to put this comment, so I'll drop it here.
>> >
>> >I agree the buffers can go.  I realized that on the drive home last
>> >night, but was too tired to follow up.
>> >
>> >Talking with some of the folks who use this feature, there's a
>> >desire to keep some sort of finer control on whether the comms
>> >buffers are printed or not - either the existing debugfs setup
>> >(preferred, since it lets them turn on the dmesg clutter only when
>> >needed), or by converting to a config option such as
>> >CONFIG_RMI4_COMMS_DEBUG.  It's very useful in new platform
>> >development, since there's a surprising number of ways in which the
>> >reads and writes can go wonky on new hardware.
 >
> That is why you have CONFIG_DYNAMIC_DEBUG: you can activate these debug
> statements at will using the common kernel mechanisms.

I'll check this out and get back.

>
> Or we could convert them to dev_vdbg() and then it will be just a tiny
> transport module recompile with DEBUG defined.


-- 

Christopher Heiny
Senior Staff Firmware Engineer
Synaptics Incorporated

^ permalink raw reply

* Re: [PATCH v2] input: synaptics-rmi4 - use snprintf instead of sprintf in rmi_i2c.c
From: Dmitry Torokhov @ 2014-01-09 21:29 UTC (permalink / raw)
  To: Christopher Heiny
  Cc: Linux Input, Andrew Duggan, Vincent Huang, Vivian Ly,
	Daniel Rosenberg, Jean Delvare, Joerie de Gram, Linus Walleij,
	Benjamin Tissoires
In-Reply-To: <52CF1359.6080207@synaptics.com>

On Thu, Jan 09, 2014 at 01:23:37PM -0800, Christopher Heiny wrote:
> On 01/09/2014 12:04 AM, Dmitry Torokhov wrote:
> >On Wed, Jan 08, 2014 at 05:18:39PM -0800, Christopher Heiny wrote:
> >>This is a trivial change to replace the sprintf loop with snprintf using
> >>up-to-date format capability.
> >
> >Hmm, how about we do this instead:
> >
> >Input: synaptics-rmi4 - clean up debug handling in rmi_i2c
> >
> >From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> >
> >Kernel now has standard facility to format and print binary buffers, let's
> >use it. By doing so we no longer need to allocate memory for debug buffers
> >and we can let debugfs code go as well.
> 
> Not sure where to put this comment, so I'll drop it here.
> 
> I agree the buffers can go.  I realized that on the drive home last
> night, but was too tired to follow up.
> 
> Talking with some of the folks who use this feature, there's a
> desire to keep some sort of finer control on whether the comms
> buffers are printed or not - either the existing debugfs setup
> (preferred, since it lets them turn on the dmesg clutter only when
> needed), or by converting to a config option such as
> CONFIG_RMI4_COMMS_DEBUG.  It's very useful in new platform
> development, since there's a surprising number of ways in which the
> reads and writes can go wonky on new hardware.

That is why you have CONFIG_DYNAMIC_DEBUG: you can activate these debug
statements at will using the common kernel mechanisms.

Or we could convert them to dev_vdbg() and then it will be just a tiny
transport module recompile with DEBUG defined.

Thanks,

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH v2] input: synaptics-rmi4 - use snprintf instead of sprintf in rmi_i2c.c
From: Christopher Heiny @ 2014-01-09 21:23 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Linux Input, Andrew Duggan, Vincent Huang, Vivian Ly,
	Daniel Rosenberg, Jean Delvare, Joerie de Gram, Linus Walleij,
	Benjamin Tissoires
In-Reply-To: <20140109080454.GA27160@core.coreip.homeip.net>

On 01/09/2014 12:04 AM, Dmitry Torokhov wrote:
> On Wed, Jan 08, 2014 at 05:18:39PM -0800, Christopher Heiny wrote:
>> This is a trivial change to replace the sprintf loop with snprintf using
>> up-to-date format capability.
>
> Hmm, how about we do this instead:
>
> Input: synaptics-rmi4 - clean up debug handling in rmi_i2c
>
> From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>
> Kernel now has standard facility to format and print binary buffers, let's
> use it. By doing so we no longer need to allocate memory for debug buffers
> and we can let debugfs code go as well.

Not sure where to put this comment, so I'll drop it here.

I agree the buffers can go.  I realized that on the drive home last 
night, but was too tired to follow up.

Talking with some of the folks who use this feature, there's a desire to 
keep some sort of finer control on whether the comms buffers are printed 
or not - either the existing debugfs setup (preferred, since it lets 
them turn on the dmesg clutter only when needed), or by converting to a 
config option such as CONFIG_RMI4_COMMS_DEBUG.  It's very useful in new 
platform development, since there's a surprising number of ways in which 
the reads and writes can go wonky on new hardware.

> We have a new limitation however - output is limited to the first 64 bytes
> of the buffer. However if we really need to capture larger messages dumping
> them in dmesg is not right interface anyway.
>
> Also clean up extra includes, while we are at it.

That's good, too.


> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
>   drivers/input/rmi4/rmi_i2c.c |  131 +++++-------------------------------------
>   1 file changed, 15 insertions(+), 116 deletions(-)
>
> diff --git a/drivers/input/rmi4/rmi_i2c.c b/drivers/input/rmi4/rmi_i2c.c
> index b33074c..c2ad1aa 100644
> --- a/drivers/input/rmi4/rmi_i2c.c
> +++ b/drivers/input/rmi4/rmi_i2c.c
> @@ -8,16 +8,10 @@
>    */
>
>   #include <linux/kernel.h>
> -#include <linux/delay.h>
>   #include <linux/i2c.h>
> -#include <linux/interrupt.h>
> -#include <linux/kconfig.h>
> -#include <linux/lockdep.h>
>   #include <linux/module.h>
> -#include <linux/pm.h>
>   #include <linux/rmi.h>
>   #include <linux/slab.h>
> -#include <linux/uaccess.h>
>   #include "rmi_driver.h"
>
>   #define BUFFER_SIZE_INCREMENT 32
> @@ -31,11 +25,6 @@
>    *
>    * @tx_buf: Buffer used for transmitting data to the sensor over i2c.
>    * @tx_buf_size: Size of the buffer
> - * @debug_buf: Buffer used for exposing buffer contents using dev_dbg
> - * @debug_buf_size: Size of the debug buffer.
> - *
> - * @comms_debug: Latest data read/written for debugging I2C communications
> - * @debugfs_comms: Debugfs file for debugging I2C communications
>    */
>   struct rmi_i2c_data {
>   	struct mutex page_mutex;
> @@ -44,56 +33,8 @@ struct rmi_i2c_data {
>
>   	u8 *tx_buf;
>   	int tx_buf_size;
> -	u8 *debug_buf;
> -	int debug_buf_size;
> -
> -	u32 comms_debug;
> -#ifdef CONFIG_RMI4_DEBUG
> -	struct dentry *debugfs_comms;
> -#endif
>   };
>
> -#ifdef CONFIG_RMI4_DEBUG
> -
> -static int setup_debugfs(struct rmi_device *rmi_dev, struct rmi_i2c_data *data)
> -{
> -	if (!rmi_dev->debugfs_root)
> -		return -ENODEV;
> -
> -	data->debugfs_comms = debugfs_create_bool("comms_debug",
> -			(S_IRUGO | S_IWUGO), rmi_dev->debugfs_root,
> -			&data->comms_debug);
> -	if (!data->debugfs_comms || IS_ERR(data->debugfs_comms)) {
> -		dev_warn(&rmi_dev->dev,
> -			 "Failed to create debugfs comms_debug.\n");
> -		data->debugfs_comms = NULL;
> -	}
> -
> -	return 0;
> -}
> -
> -static void teardown_debugfs(struct rmi_i2c_data *data)
> -{
> -	if (data->debugfs_comms)
> -		debugfs_remove(data->debugfs_comms);
> -}
> -
> -#else
> -
> -static inline int setup_debugfs(struct rmi_device *rmi_dev,
> -				struct rmi_i2c_data *data)
> -{
> -	return 0;
> -}
> -
> -static inline void teardown_debugfs(struct rmi_i2c_data *data)
> -{
> -}
> -
> -#endif
> -
> -#define COMMS_DEBUG(data) (IS_ENABLED(CONFIG_RMI4_DEBUG) && data->comms_debug)
> -
>   #define RMI_PAGE_SELECT_REGISTER 0xff
>   #define RMI_I2C_PAGE(addr) (((addr) >> 8) & 0xff)
>
> @@ -120,8 +61,7 @@ static int rmi_set_page(struct rmi_transport_dev *xport, u8 page)
>   	u8 txbuf[2] = {RMI_PAGE_SELECT_REGISTER, page};
>   	int retval;
>
> -	if (COMMS_DEBUG(data))
> -		dev_dbg(&client->dev, "writes 3 bytes: %02x %02x\n",
> +	dev_dbg(&client->dev, "writes 3 bytes: %02x %02x\n",
>   			txbuf[0], txbuf[1]);
>   	xport->info.tx_count++;
>   	xport->info.tx_bytes += sizeof(txbuf);
> @@ -136,34 +76,6 @@ static int rmi_set_page(struct rmi_transport_dev *xport, u8 page)
>   	return 0;
>   }
>
> -static int copy_to_debug_buf(struct device *dev, struct rmi_i2c_data *data,
> -			     const u8 *buf, const int len) {
> -	int i;
> -	int n = 0;
> -	char *temp;
> -	int dbg_size = 3 * len + 1;
> -
> -	if (!data->debug_buf || data->debug_buf_size < dbg_size) {
> -		if (data->debug_buf)
> -			devm_kfree(dev, data->debug_buf);
> -		data->debug_buf_size = dbg_size + BUFFER_SIZE_INCREMENT;
> -		data->debug_buf = devm_kzalloc(dev, data->debug_buf_size,
> -					       GFP_KERNEL);
> -		if (!data->debug_buf) {
> -			data->debug_buf_size = 0;
> -			return -ENOMEM;
> -		}
> -	}
> -	temp = data->debug_buf;
> -
> -	for (i = 0; i < len; i++) {
> -		n = sprintf(temp, " %02x", buf[i]);
> -		temp += n;
> -	}
> -
> -	return 0;
> -}
> -
>   static int rmi_i2c_write_block(struct rmi_transport_dev *xport, u16 addr,
>   			       const void *buf, const int len)
>   {
> @@ -195,12 +107,8 @@ static int rmi_i2c_write_block(struct rmi_transport_dev *xport, u16 addr,
>   			goto exit;
>   	}
>
> -	if (COMMS_DEBUG(data)) {
> -		int rc = copy_to_debug_buf(&client->dev, data, (u8 *) buf, len);
> -		if (!rc)
> -			dev_dbg(&client->dev, "writes %d bytes at %#06x:%s\n",
> -				len, addr, data->debug_buf);
> -	}
> +	dev_dbg(&client->dev,
> +		"writes %d bytes at %#06x: %*ph\n", len, addr, len, buf);
>
>   	xport->info.tx_count++;
>   	xport->info.tx_bytes += tx_size;
> @@ -232,8 +140,7 @@ static int rmi_i2c_read_block(struct rmi_transport_dev *xport, u16 addr,
>   			goto exit;
>   	}
>
> -	if (COMMS_DEBUG(data))
> -		dev_dbg(&client->dev, "writes 1 bytes: %02x\n", txbuf[0]);
> +	dev_dbg(&client->dev, "writes 1 bytes: %02x\n", txbuf[0]);
>
>   	xport->info.tx_count++;
>   	xport->info.tx_bytes += sizeof(txbuf);
> @@ -244,18 +151,16 @@ static int rmi_i2c_read_block(struct rmi_transport_dev *xport, u16 addr,
>   		goto exit;
>   	}
>
> -	retval = i2c_master_recv(client, (u8 *) buf, len);
> +	retval = i2c_master_recv(client, buf, len);
>
>   	xport->info.rx_count++;
>   	xport->info.rx_bytes += len;
>   	if (retval < 0)
>   		xport->info.rx_errs++;
> -	else if (COMMS_DEBUG(data)) {
> -		int rc = copy_to_debug_buf(&client->dev, data, (u8 *) buf, len);
> -		if (!rc)
> -			dev_dbg(&client->dev, "read %d bytes at %#06x:%s\n",
> -				len, addr, data->debug_buf);
> -	}
> +	else
> +		dev_dbg(&client->dev,
> +			"read %d bytes at %#06x: %*ph\n",
> +			len, addr, len, buf);
>
>   exit:
>   	mutex_unlock(&data->page_mutex);
> @@ -265,9 +170,9 @@ exit:
>   static int rmi_i2c_probe(struct i2c_client *client,
>   			 const struct i2c_device_id *id)
>   {
> +	const struct rmi_device_platform_data *pdata = dev_get_platdata(&client->dev);
>   	struct rmi_transport_dev *xport;
>   	struct rmi_i2c_data *data;
> -	struct rmi_device_platform_data *pdata = client->dev.platform_data;
>   	int retval;
>
>   	if (!pdata) {
> @@ -318,7 +223,8 @@ static int rmi_i2c_probe(struct i2c_client *client,
>
>   	mutex_init(&data->page_mutex);
>
> -	/* Setting the page to zero will (a) make sure the PSR is in a
> +	/*
> +	 * Setting the page to zero will (a) make sure the PSR is in a
>   	 * known state, and (b) make sure we can talk to the device.
>   	 */
>   	retval = rmi_set_page(xport, 0);
> @@ -335,11 +241,6 @@ static int rmi_i2c_probe(struct i2c_client *client,
>   	}
>   	i2c_set_clientdata(client, xport);
>
> -	retval = setup_debugfs(xport->rmi_dev, data);
> -	if (retval < 0)
> -		dev_warn(&client->dev, "Failed to setup debugfs. Code: %d.\n",
> -			 retval);
> -
>   	dev_info(&client->dev, "registered rmi i2c driver at %#04x.\n",
>   			client->addr);
>   	return 0;
> @@ -353,14 +254,12 @@ err_gpio:
>   static int rmi_i2c_remove(struct i2c_client *client)
>   {
>   	struct rmi_transport_dev *xport = i2c_get_clientdata(client);
> -	struct rmi_device_platform_data *pd = client->dev.platform_data;
> -
> -	teardown_debugfs(xport->data);
> +	struct rmi_device_platform_data *pdata = dev_get_platdata(&client->dev);
>
>   	rmi_unregister_transport_device(xport);
>
> -	if (pd->gpio_config)
> -		pd->gpio_config(&pd->gpio_data, false);
> +	if (pdata->gpio_config)
> +		pdata->gpio_config(&pdata->gpio_data, false);
>
>   	return 0;
>   }
>


-- 

Christopher Heiny
Senior Staff Firmware Engineer
Synaptics Incorporated

^ permalink raw reply

* Re: [PATCH 0/5] HID logitech DJ fixes and preps for enabling extended features
From: Benjamin Tissoires @ 2014-01-09 21:22 UTC (permalink / raw)
  To: Andrew de los Reyes
  Cc: Benjamin Tissoires, Jiri Kosina, Nestor Lopez Casado,
	Andrew de los Reyes, Linux Input, linux-kernel
In-Reply-To: <CAG_cf+eYoK--k=z+f5xQBx0FP3_z5sbszrSZ1qYq7jR_Zi9Vaw@mail.gmail.com>

On 09/01/14 16:08, Andrew de los Reyes wrote:
> In general, I'm positive on the change to fix the USB3 issue (yay!),
> and for the others I'm happy it's going upstream. It seem to open up
> the possibility of user-space drivers, which is great, even though we
> don't need this on our team.
> 
> One thing I want to double-check: on some devices (T651, at least),
> the raw data comes in not via HID++, but tacked onto the end of the
> normal mouse reports. Will a driver for this device be able to get all
> packets, not just HID++ ones? Sorry if this was clear and I missed it.

Yeah, that is partly why I can not send the rest of the series right
now. Nestor already warned me about those funny devices, so I need to
double check how to implement HID++.

On a technical point of view, a driver connected through the unifying
receiver currently only get the regular input reports, and this series
adds the HID++ reports to these ones. So yes, a device will receive all
reports dedicated to it.

Cheers,
Benjamin

> 
> -andrew
> 
> On Wed, Jan 8, 2014 at 2:18 PM, Benjamin Tissoires
> <benjamin.tissoires@redhat.com> wrote:
>> Hi Jiri,
>>
>> Well, this work is _not_ for 3.14 (except maybe patch 1), especially since it is
>> missing the biggest part where we enable the capabilities of Logitech devices.
>>
>> Long story short:
>> This work is based on the work I did back in Summer 2011. I worked at Logitech
>> for a few weeks to show up a demo of a driver for the Logitech Wireless Touchpad.
>> At that time, a first draft has been done, but due to a lack of resources, noone
>> upstreamed it.
>> Since then, the code marinated a little at Logitech and in the ChromeOS kernel
>> tree, but nobody tried to push it upstream. So here I am, trying to push this stuff
>> upstream.
>>
>> I can not send the full series right now because I am lacking most of the
>> testing hardware (I mean I only have the oldest Wireless Touchpad).
>> Hopefully, I should receive some other soon, and I'll be able to send the second
>> part then.
>>
>> Now, let me roughly explain the patches:
>> - patch 1 can be applied right now I think, but it's entirely up to you Jiri.
>>   This patch should fix the missing notifications with some USB 3.0 boards.
>> - patches 2 to 5 allows to forward in both direction the proprietary protocol
>>   used by Logitech (HID++ [1]) between the driver and the hardware.
>> - later patches will introduce a transport layer for HID++ and also a driver
>>   to support full multitouch on various Logitech touchpads.
>>
>> Nestor, Andrew, feel free to add your "Signed-off-by" whereever you want, I lost
>> a little bit the track of who added what.
>>
>> Cheers,
>> Benjamin
>>
>> [1] HID++: Documentation is provided at
>> https://drive.google.com/a/logitech.com/?tab=mo#folders/0BxbRzx7vEV7eWmgwazJ3NUFfQ28
>>
>> Benjamin Tisssoires (5):
>>   HID: logitech-dj: Fix USB 3.0 issue
>>   HID: core: do not scan reports if the group is already set
>>   HID: logitech-dj: rely on hid groups to separate receivers from dj
>>     devices
>>   HID: logitech-dj: forward incoming HID++ reports to the correct dj
>>     device
>>   HID: logitech-dj: add .request callback
>>
>>  drivers/hid/hid-core.c        |   3 +-
>>  drivers/hid/hid-logitech-dj.c | 161 +++++++++++++++++++++++++++++++++---------
>>  drivers/hid/hid-logitech-dj.h |  16 ++---
>>  include/linux/hid.h           |   1 +
>>  4 files changed, 136 insertions(+), 45 deletions(-)
>>
>> --
>> 1.8.4.2
>>
>> --
>> 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 3/3] Input: zforce: add devicetree support
From: Heiko Stübner @ 2014-01-09 21:22 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: Henrik Rydberg, linux-input
In-Reply-To: <2587208.XYbbI8xJiH@phil>

This makes the zforce driver usable on devicetree-based platforms too.

Signed-off-by: Heiko Stuebner <heiko.stuebner@bqreaders.com>
---
 drivers/input/touchscreen/zforce_ts.c |   57 +++++++++++++++++++++++++++++++--
 1 file changed, 55 insertions(+), 2 deletions(-)

diff --git a/drivers/input/touchscreen/zforce_ts.c b/drivers/input/touchscreen/zforce_ts.c
index 5a1a93b..a33e173 100644
--- a/drivers/input/touchscreen/zforce_ts.c
+++ b/drivers/input/touchscreen/zforce_ts.c
@@ -29,6 +29,8 @@
 #include <linux/sysfs.h>
 #include <linux/input/mt.h>
 #include <linux/platform_data/zforce_ts.h>
+#include <linux/of.h>
+#include <linux/of_gpio.h>
 
 #define WAIT_TIMEOUT		msecs_to_jiffies(1000)
 
@@ -684,6 +686,45 @@ static void zforce_reset(void *data)
 	gpio_set_value(ts->pdata->gpio_rst, 0);
 }
 
+static struct zforce_ts_platdata *zforce_parse_dt(struct device *dev)
+{
+	struct zforce_ts_platdata *pdata;
+	struct device_node *np = dev->of_node;
+
+	if (!np)
+		return ERR_PTR(-ENOENT);
+
+	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata) {
+		dev_err(dev, "failed to allocate platform data\n");
+		return ERR_PTR(-ENOMEM);
+	}
+
+	pdata->gpio_int = of_get_gpio(np, 0);
+	if (!gpio_is_valid(pdata->gpio_int)) {
+		dev_err(dev, "failed to get interrupt gpio\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	pdata->gpio_rst = of_get_gpio(np, 1);
+	if (!gpio_is_valid(pdata->gpio_rst)) {
+		dev_err(dev, "failed to get reset gpio\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	if (of_property_read_u32(np, "x-size", &pdata->x_max)) {
+		dev_err(dev, "failed to get x-size property\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	if (of_property_read_u32(np, "y-size", &pdata->y_max)) {
+		dev_err(dev, "failed to get y-size property\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	return pdata;
+}
+
 static int zforce_probe(struct i2c_client *client,
 			const struct i2c_device_id *id)
 {
@@ -692,8 +733,11 @@ static int zforce_probe(struct i2c_client *client,
 	struct input_dev *input_dev;
 	int ret;
 
-	if (!pdata)
-		return -EINVAL;
+	if (!pdata) {
+		pdata = zforce_parse_dt(&client->dev);
+		if (IS_ERR(pdata))
+			return PTR_ERR(pdata);
+	}
 
 	ts = devm_kzalloc(&client->dev, sizeof(struct zforce_ts), GFP_KERNEL);
 	if (!ts)
@@ -829,11 +873,20 @@ static struct i2c_device_id zforce_idtable[] = {
 };
 MODULE_DEVICE_TABLE(i2c, zforce_idtable);
 
+#ifdef CONFIG_OF
+static struct of_device_id zforce_dt_idtable[] = {
+	{ .compatible = "neonode,zforce" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, zforce_dt_idtable);
+#endif
+
 static struct i2c_driver zforce_driver = {
 	.driver = {
 		.owner	= THIS_MODULE,
 		.name	= "zforce-ts",
 		.pm	= &zforce_pm_ops,
+		.of_match_table	= of_match_ptr(zforce_dt_idtable),
 	},
 	.probe		= zforce_probe,
 	.id_table	= zforce_idtable,
-- 
1.7.10.4



^ permalink raw reply related

* [PATCH 2/3] Input: zforce: Use internal pdata pointer instead of dev_get_platdata
From: Heiko Stübner @ 2014-01-09 21:21 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: Henrik Rydberg, linux-input
In-Reply-To: <2587208.XYbbI8xJiH@phil>

When the device is initialized from devicetree the platformdata is created
locally making dev_get_platdata return NULL.

Therefore directly use the internal pointer to the pdata instead.

Signed-off-by: Heiko Stuebner <heiko.stuebner@bqreaders.com>
---
 drivers/input/touchscreen/zforce_ts.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/input/touchscreen/zforce_ts.c b/drivers/input/touchscreen/zforce_ts.c
index 2175f34..5a1a93b 100644
--- a/drivers/input/touchscreen/zforce_ts.c
+++ b/drivers/input/touchscreen/zforce_ts.c
@@ -255,7 +255,7 @@ static int zforce_setconfig(struct zforce_ts *ts, char b1)
 static int zforce_start(struct zforce_ts *ts)
 {
 	struct i2c_client *client = ts->client;
-	const struct zforce_ts_platdata *pdata = dev_get_platdata(&client->dev);
+	const struct zforce_ts_platdata *pdata = ts->pdata;
 	int ret;
 
 	dev_dbg(&client->dev, "starting device\n");
@@ -326,7 +326,7 @@ static int zforce_stop(struct zforce_ts *ts)
 static int zforce_touch_event(struct zforce_ts *ts, u8 *payload)
 {
 	struct i2c_client *client = ts->client;
-	const struct zforce_ts_platdata *pdata = dev_get_platdata(&client->dev);
+	const struct zforce_ts_platdata *pdata = ts->pdata;
 	struct zforce_point point;
 	int count, i, num = 0;
 
@@ -471,7 +471,7 @@ static irqreturn_t zforce_irq_thread(int irq, void *dev_id)
 {
 	struct zforce_ts *ts = dev_id;
 	struct i2c_client *client = ts->client;
-	const struct zforce_ts_platdata *pdata = dev_get_platdata(&client->dev);
+	const struct zforce_ts_platdata *pdata = ts->pdata;
 	int ret;
 	u8 payload_buffer[512];
 	u8 *payload;
-- 
1.7.10.4



^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox