Linux Input/HID development
 help / color / mirror / Atom feed
* Re: [PATCH] Input - wacom: update the ABI doc according to latest changes
From: Benjamin Tissoires @ 2014-08-06 21:29 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Ping Cheng, Jiri Kosina, Jason Gerecke, Przemo Firszt,
	linux-kernel@vger.kernel.org, linux-input
In-Reply-To: <20140806210401.GB22889@core.coreip.homeip.net>

On Aug 06 2014 or thereabouts, Dmitry Torokhov wrote:
> On Wed, Aug 06, 2014 at 04:44:08PM -0400, Benjamin Tissoires wrote:
> > On Aug 05 2014 or thereabouts, Ping Cheng wrote:
> > > On Tue, Aug 5, 2014 at 2:34 PM, Benjamin Tissoires
> > > <benjamin.tissoires@redhat.com> wrote:
> > > > Hi Ping,
> > > >
> > > > On Aug 05 2014 or thereabouts, Ping Cheng wrote:
> > > >> Thank you Benjamin for updating the ABI docs. My comments are inline.
> > > >>
> > > >> With those lines updated, this patch is:
> > > >>
> > > >> Reviewed-by: Ping Cheng <pingc@wacom.com>
> > > >
> > > > Thanks!
> > > >
> > > >>
> > > >> Cheers,
> > > >>
> > > >> Ping
> > > >>
> > > >> On Tue, Aug 5, 2014 at 8:34 AM, Benjamin Tissoires
> > > >> <benjamin.tissoires@redhat.com> wrote:
> > > >> > Now the devices show up under hid no matter the connection. So update the
> > > >> > sysfs path to the shortest and common one.
> > > >> > The ABI has also been unified regarding the OLEDs, but Bluetooth handle
> > > >> > only 1-bit images instead of 4.
> > > >> >
> > > >> > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> > > >> > ---
> > > >> >
> > > >> > Guys,
> > > >> >
> > > >> > by updating this and working on the g-s-d client side for OLEDs, I think now
> > > >> > that we should also keep the oled*_img for Bluetooth and implement this file
> > > >> > for USB too. The user space code gets messy quite fast due to the need to
> > > >> > support old and new kernels, whereas if we handle the scrambling in the kernel,
> > > >> > it should be quite straightforward.
> > > >> > Of course, we have to keep the current button<n>_rawimg for backward
> > > >> > compatibility.
> > > >> >
> > > >> > Any thoughts?
> > > >> >
> > > >> > Cheers,
> > > >> > Benjamin
> > > >> >
> > > >> >  Documentation/ABI/testing/sysfs-driver-wacom | 56 ++++++++++------------------
> > > >> >  1 file changed, 20 insertions(+), 36 deletions(-)
> > > >> >
> > > >> > diff --git a/Documentation/ABI/testing/sysfs-driver-wacom b/Documentation/ABI/testing/sysfs-driver-wacom
> > > >> > index 7fc7810..c75ed4c 100644
> > > >> > --- a/Documentation/ABI/testing/sysfs-driver-wacom
> > > >> > +++ b/Documentation/ABI/testing/sysfs-driver-wacom
> > > >> > @@ -1,47 +1,26 @@
> > > >> > -WWhat:         /sys/class/hidraw/hidraw*/device/oled*_img
> > > >> > -Date:          June 2012
> > > >> > -Contact:       linux-bluetooth@vger.kernel.org
> > > >> > -Description:
> > > >> > -               The /sys/class/hidraw/hidraw*/device/oled*_img files control
> > > >> > -               OLED mocro displays on Intuos4 Wireless tablet. Accepted image
> > > >> > -               has to contain 256 bytes (64x32 px 1 bit colour). The format
> > > >> > -               is the same as PBM image 62x32px without header (64 bits per
> > > >> > -               horizontal line, 32 lines). An example of setting OLED No. 0:
> > > >> > -               dd bs=256 count=1 if=img_file of=[path to oled0_img]/oled0_img
> > > >> > -               The attribute is read only and no local copy of the image is
> > > >> > -               stored.
> > > >> > -
> > > >> > -What:          /sys/class/hidraw/hidraw*/device/speed
> > > >> > +What:          /sys/bus/hid/devices/<bus>:<vid>:<pid>.<n>/speed
> > > >> >  Date:          April 2010
> > > >>
> > > >> Should the Date be updated?
> > > >
> > > > I don't think that this one should. The ABI is the same since April
> > > > 2010, and the "new" path works also for older kernels. So basically,
> > > > here, for Bluetooth, there has been no changes.
> > > 
> > > Oh, I see your point. But, don't we consider path part of the interface?
> > > 
> > 
> > Well, in this case, both the path /sys/class/hidraw/hidraw*/device/speed
> > and /sys/bus/hid/devices/<bus>:<vid>:<pid>.<n>/speed refers to the same
> > device, even in April 2010.
> > 
> > /sys/class/hidraw/hidraw*/device is IMO not accurate because you have to
> > check all the hidraw devices to find the right one.
> > /sys/bus/hid/devices/<bus>:<vid>:<pid>.<n>/speed gives you the correct
> > at first sight. This is just a fix to have a more reliable path in the
> > documentation, not a change.
> 
> Can we document this in changelog?
> 

Hmm, yes. I guess I'll have to send a new version with this
documentation in the commit message and the update of the other dates.

I should be offline for the next 3 to 4 hours, so it will have to wait
tonight or tomorrow morning.

BTW, thanks for merging the rest of the series (and the various
patches). Greatly appreciated.

Cheers,
Benjamin


^ permalink raw reply

* Re: [PATCH] Input - wacom: update the ABI doc according to latest changes
From: Dmitry Torokhov @ 2014-08-06 21:36 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: Ping Cheng, Jiri Kosina, Jason Gerecke, Przemo Firszt,
	linux-kernel@vger.kernel.org, linux-input
In-Reply-To: <20140806212902.GD15018@mail.corp.redhat.com>

On Wed, Aug 06, 2014 at 05:29:02PM -0400, Benjamin Tissoires wrote:
> On Aug 06 2014 or thereabouts, Dmitry Torokhov wrote:
> > On Wed, Aug 06, 2014 at 04:44:08PM -0400, Benjamin Tissoires wrote:
> > > On Aug 05 2014 or thereabouts, Ping Cheng wrote:
> > > > On Tue, Aug 5, 2014 at 2:34 PM, Benjamin Tissoires
> > > > <benjamin.tissoires@redhat.com> wrote:
> > > > > Hi Ping,
> > > > >
> > > > > On Aug 05 2014 or thereabouts, Ping Cheng wrote:
> > > > >> Thank you Benjamin for updating the ABI docs. My comments are inline.
> > > > >>
> > > > >> With those lines updated, this patch is:
> > > > >>
> > > > >> Reviewed-by: Ping Cheng <pingc@wacom.com>
> > > > >
> > > > > Thanks!
> > > > >
> > > > >>
> > > > >> Cheers,
> > > > >>
> > > > >> Ping
> > > > >>
> > > > >> On Tue, Aug 5, 2014 at 8:34 AM, Benjamin Tissoires
> > > > >> <benjamin.tissoires@redhat.com> wrote:
> > > > >> > Now the devices show up under hid no matter the connection. So update the
> > > > >> > sysfs path to the shortest and common one.
> > > > >> > The ABI has also been unified regarding the OLEDs, but Bluetooth handle
> > > > >> > only 1-bit images instead of 4.
> > > > >> >
> > > > >> > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> > > > >> > ---
> > > > >> >
> > > > >> > Guys,
> > > > >> >
> > > > >> > by updating this and working on the g-s-d client side for OLEDs, I think now
> > > > >> > that we should also keep the oled*_img for Bluetooth and implement this file
> > > > >> > for USB too. The user space code gets messy quite fast due to the need to
> > > > >> > support old and new kernels, whereas if we handle the scrambling in the kernel,
> > > > >> > it should be quite straightforward.
> > > > >> > Of course, we have to keep the current button<n>_rawimg for backward
> > > > >> > compatibility.
> > > > >> >
> > > > >> > Any thoughts?
> > > > >> >
> > > > >> > Cheers,
> > > > >> > Benjamin
> > > > >> >
> > > > >> >  Documentation/ABI/testing/sysfs-driver-wacom | 56 ++++++++++------------------
> > > > >> >  1 file changed, 20 insertions(+), 36 deletions(-)
> > > > >> >
> > > > >> > diff --git a/Documentation/ABI/testing/sysfs-driver-wacom b/Documentation/ABI/testing/sysfs-driver-wacom
> > > > >> > index 7fc7810..c75ed4c 100644
> > > > >> > --- a/Documentation/ABI/testing/sysfs-driver-wacom
> > > > >> > +++ b/Documentation/ABI/testing/sysfs-driver-wacom
> > > > >> > @@ -1,47 +1,26 @@
> > > > >> > -WWhat:         /sys/class/hidraw/hidraw*/device/oled*_img
> > > > >> > -Date:          June 2012
> > > > >> > -Contact:       linux-bluetooth@vger.kernel.org
> > > > >> > -Description:
> > > > >> > -               The /sys/class/hidraw/hidraw*/device/oled*_img files control
> > > > >> > -               OLED mocro displays on Intuos4 Wireless tablet. Accepted image
> > > > >> > -               has to contain 256 bytes (64x32 px 1 bit colour). The format
> > > > >> > -               is the same as PBM image 62x32px without header (64 bits per
> > > > >> > -               horizontal line, 32 lines). An example of setting OLED No. 0:
> > > > >> > -               dd bs=256 count=1 if=img_file of=[path to oled0_img]/oled0_img
> > > > >> > -               The attribute is read only and no local copy of the image is
> > > > >> > -               stored.
> > > > >> > -
> > > > >> > -What:          /sys/class/hidraw/hidraw*/device/speed
> > > > >> > +What:          /sys/bus/hid/devices/<bus>:<vid>:<pid>.<n>/speed
> > > > >> >  Date:          April 2010
> > > > >>
> > > > >> Should the Date be updated?
> > > > >
> > > > > I don't think that this one should. The ABI is the same since April
> > > > > 2010, and the "new" path works also for older kernels. So basically,
> > > > > here, for Bluetooth, there has been no changes.
> > > > 
> > > > Oh, I see your point. But, don't we consider path part of the interface?
> > > > 
> > > 
> > > Well, in this case, both the path /sys/class/hidraw/hidraw*/device/speed
> > > and /sys/bus/hid/devices/<bus>:<vid>:<pid>.<n>/speed refers to the same
> > > device, even in April 2010.
> > > 
> > > /sys/class/hidraw/hidraw*/device is IMO not accurate because you have to
> > > check all the hidraw devices to find the right one.
> > > /sys/bus/hid/devices/<bus>:<vid>:<pid>.<n>/speed gives you the correct
> > > at first sight. This is just a fix to have a more reliable path in the
> > > documentation, not a change.
> > 
> > Can we document this in changelog?
> > 
> 
> Hmm, yes. I guess I'll have to send a new version with this
> documentation in the commit message and the update of the other dates.
> 
> I should be offline for the next 3 to 4 hours, so it will have to wait
> tonight or tomorrow morning.

Well, it is a doc patch, it can go in at pretty much any time.

> 
> BTW, thanks for merging the rest of the series (and the various
> patches). Greatly appreciated.

No, thank you for making them ;)

Thanks.

-- 
Dmitry

^ permalink raw reply

* Re: TSC2004 driver
From: Michael Welling @ 2014-08-06 22:13 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Joachim Eastwood, linux-omap, linux-input, Hiremath, Vaibhav
In-Reply-To: <20140806211626.GC22889@core.coreip.homeip.net>

On Wed, Aug 6, 2014 at 4:16 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Wed, Aug 06, 2014 at 10:33:18PM +0200, Joachim Eastwood wrote:
>> On 6 August 2014 21:37, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
>> > Hi Michael,
>> >
>> > On Tue, Aug 05, 2014 at 12:06:40PM -0500, Michael Welling wrote:
>> >> The TSC2004 driver has yet to appear in the mainline kernel. We have
>> >> been using the driver referenced here as provided by TI:
>> >>
>> >> https://www.mail-archive.com/linux-omap@vger.kernel.org/msg22018.html
>> >>
>> >> Are there any plans of supporting this device in the mainline kernel?
>> >
>> > I still believe that support for TSC2004 should be added 5to tsc2007, they are
>> > too much alike to be separate drivers.
>>
>> I tried to add tsc2004 support to the tsc2007 driver but I didn't
>> really get it to work properly because the tsc2007 interrupt handling
>> differs from what tsc2004 needs.
>
> OK.
>
>>
>> I think it would be better to add tsc2004 support to the tsc2005
>> driver. They only difference between those two chips is the interface
>> (i2c vs spi). If regmap was added to tsc2005 I think it should be easy
>> to support tsc2004.
>
> That would also be acceptable.

The tsc2005 still needs devicetree conversion, so it is at least 2 patches away
from where I can use it.

I have missed the window for my companies OE release so I doubt I will
have time to code
in all or any of this.

I am in the process of routing a AM335x SoM which obsoletes this AM3517 one
so my resources will be elsewhere.

>
> Thanks.
>
> --
> Dmitry

^ permalink raw reply

* Re: TSC2004 driver
From: Sebastian Reichel @ 2014-08-06 22:28 UTC (permalink / raw)
  To: Michael Welling
  Cc: Dmitry Torokhov, Joachim Eastwood, linux-omap, linux-input,
	Hiremath, Vaibhav
In-Reply-To: <CAO5Z=i1YMBP8mbyh=ESbsJLkHZqZik+tw-aF1FWR1etaBrBN1g@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 353 bytes --]

Hi,

On Wed, Aug 06, 2014 at 05:13:19PM -0500, Michael Welling wrote:
> The tsc2005 still needs devicetree conversion, so it is at least 2
> patches away from where I can use it.

TSC2005 DT support has been added in 3.16:

http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=a38cfebb56898633687ab337fd53710e63a0aedd

-- Sebastian

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: TSC2004 driver
From: Michael Welling @ 2014-08-06 22:42 UTC (permalink / raw)
  To: Sebastian Reichel
  Cc: Dmitry Torokhov, Joachim Eastwood, linux-omap, linux-input,
	Hiremath, Vaibhav
In-Reply-To: <20140806222817.GB9686@earth.universe>

On Wed, Aug 6, 2014 at 5:28 PM, Sebastian Reichel <sre@kernel.org> wrote:
> Hi,
>
> On Wed, Aug 06, 2014 at 05:13:19PM -0500, Michael Welling wrote:
>> The tsc2005 still needs devicetree conversion, so it is at least 2
>> patches away from where I can use it.
>
> TSC2005 DT support has been added in 3.16:
>
> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=a38cfebb56898633687ab337fd53710e63a0aedd

Sorry for the misinformation.

If someone else is volunteering to do the regmap support let me know.

>
> -- Sebastian

^ permalink raw reply

* Re: [PATCH v2 05/06] input synaptics-rmi4: Add firmware update support
From: Christopher Heiny @ 2014-08-06 23:28 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Linux Input, Andrew Duggan, Vincent Huang, Vivian Ly,
	Daniel Rosenberg, Linus Walleij, Benjamin Tissoires,
	David Herrmann, Jiri Kosina
In-Reply-To: <20140731215852.GB36491@core.coreip.homeip.net>

[I sent this last Thursday, but it never showed up on the input list. 
I'm assuming nobody else saw it.]


On 07/31/2014 02:58 PM, Dmitry Torokhov wrote:
> On Thu, Jul 31, 2014 at 02:43:47PM -0700, Christopher Heiny wrote:
>> On 07/31/2014 02:19 PM, Dmitry Torokhov wrote:
>>> On Thu, Jul 31, 2014 at 02:00:14PM -0700, Christopher Heiny wrote:
>>>>> On 07/31/2014 10:53 AM, Dmitry Torokhov wrote:
>>>>>>> Hi Christopher,
>>>>>>>
>>>>>>> On Wed, Mar 12, 2014 at 06:53:56PM -0700, Christopher Heiny wrote:
>>>>>>>>> Add support for updating firmware on RMI4 devices with V5 bootloader.
>>>>>>>
>>>>>>> I am wondering why F34 is not following the staindard RMI function
>>>>>>> implementation. By that I mean that it does not declare itself as struct
>>>>>>> rmi_function_handler and does not rely on RMI core to bind itself to the device
>>>>>>> if device supports it.
>>>>>
>>>>> Hi Dmitry,
>>>>>
>>>>> We originally had an F34 implementation that followed the RMI4
>>>>> function standard and exposed most of the basic F34 operations via
>>>>> sysfs. However, we got feedback (both on LKML and offline) (a)
>>>>> recommending to use request_firmware, and (b) improve reflash times
>>>>> while (c) reducing impact on boot time, and (d) "get rid of all that
>>>>> sysfs crap" (paraphrased, but close to it).
>>>>>
>>>>> So after looking at how some other drivers use request_firmware, we
>>>>> came up with the current approach.  Switching to request_firmware
>>>>> definitely sped up the reflash times!  Including a check to see if
>>>>> firmware update is required before setting up the RMI4
>>>>> sensor/function structures also significantly reduced boot times.
>>>
>>> I am not suggesting you stop using request-firmware or introduce
>>> bazillion of new sysfs attributes. I just wondered why you have manual
>>> "binding" of F34 functionality instead of standrad RMI4 function
>>> binding, liek you do for F01, F11 and so forth.
>>
>> Sorry!  My answer wasn't very clear on that part, was it?
>>
>> The manual binding gets the reflash (if required) done very early in
>> the boot/probe process.  This eliminates the need to set up the
>> whole sensor + functions structure, tear it down in order to
>> reflash, and then build it all back up again.  It is felt that the
>> time savings is significant, especially on highly featured products.
>
> I am sorry but I have hard time accepting this argument. How often do
> you reflash devices during normal operation and how long does it take to
> initialize the device compared to getting entire userspace up and
> running to be able to actually supply or serve flash data (even without
> using usermode helper to flash you need filesystem with the firmware to
> be mounted)?

That was my argument exactly, but that was the direction we were pushed. 
  I'd much rather implement it as we discussed offline earlier this 
week.  If you were to say: "I'm sorry, but this simply can't be merged 
as it stands." you wouldn't get any argument from me on technical 
grounds. There might be people who will argue about the additional 
calendar time it would take to restructure it, though.

^ permalink raw reply

* RE: [PATCH] Input: hyperv-keyboard: register as a wakeup source
From: Dexuan Cui @ 2014-08-07  0:19 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: gregkh@linuxfoundation.org, linux-input@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	driverdev-devel@linuxdriverproject.org, olaf@aepfle.de,
	apw@canonical.com, jasowang@redhat.com, KY Srinivasan,
	Haiyang Zhang
In-Reply-To: <20140806203339.GA22889@core.coreip.homeip.net>

> -----Original Message-----
> From: Dmitry Torokhov [mailto:dmitry.torokhov@gmail.com]
> Sent: Thursday, August 7, 2014 4:34 AM
> To: Dexuan Cui
> On Fri, Aug 01, 2014 at 07:28:24AM -0700, Dexuan Cui wrote:
> > With this patch, we can press a key to wake up the VM after the VM
> executes
> > "echo freeze > /sys/power/state".
> >
> > This addresses part of
> https://bugzilla.redhat.com/show_bug.cgi?id=1086100
> 
> I am sorry but this bug is not open to the public. Please avoid
> mentioning such bugs in changelogs. Otherwise I'll apply it.
Hi Dmitry,
Sorry, I didn't realize it's not public... :-)

I'll remove the line and re-send the patch.

Thanks,
-- Dexuan

^ permalink raw reply

* Re: [PATCH] Input: hyperv-keyboard: register as a wakeup source
From: Dmitry Torokhov @ 2014-08-07  0:23 UTC (permalink / raw)
  To: Dexuan Cui
  Cc: olaf@aepfle.de, gregkh@linuxfoundation.org, jasowang@redhat.com,
	driverdev-devel@linuxdriverproject.org,
	linux-kernel@vger.kernel.org, linux-input@vger.kernel.org,
	apw@canonical.com, Haiyang Zhang
In-Reply-To: <EE124450C0AAF944A40DD71E61F878C9910E57@SINEX14MBXC417.southpacific.corp.microsoft.com>

On Wed, Aug 6, 2014 at 5:19 PM, Dexuan Cui <decui@microsoft.com> wrote:
>
> > -----Original Message-----
> > From: Dmitry Torokhov [mailto:dmitry.torokhov@gmail.com]
> > Sent: Thursday, August 7, 2014 4:34 AM
> > To: Dexuan Cui
> > On Fri, Aug 01, 2014 at 07:28:24AM -0700, Dexuan Cui wrote:
> > > With this patch, we can press a key to wake up the VM after the VM
> > executes
> > > "echo freeze > /sys/power/state".
> > >
> > > This addresses part of
> > https://bugzilla.redhat.com/show_bug.cgi?id=1086100
> >
> > I am sorry but this bug is not open to the public. Please avoid
> > mentioning such bugs in changelogs. Otherwise I'll apply it.
> Hi Dmitry,
> Sorry, I didn't realize it's not public... :-)
>
> I'll remove the line and re-send the patch.
>

No need to resend, I already applied it.

Thanks.

-- 
Dmitry

^ permalink raw reply

* RE: [PATCH] Input: hyperv-keyboard: register as a wakeup source
From: Dexuan Cui @ 2014-08-07  0:32 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: olaf@aepfle.de, gregkh@linuxfoundation.org, jasowang@redhat.com,
	driverdev-devel@linuxdriverproject.org,
	linux-kernel@vger.kernel.org, linux-input@vger.kernel.org,
	apw@canonical.com, Haiyang Zhang
In-Reply-To: <CAKdAkRRjPC3zx+Xud=RuXQuQLkUya89VOb+JBxhwsP9Vt=_cjA@mail.gmail.com>

> -----Original Message-----
> From: linux-kernel-owner@vger.kernel.org [mailto:linux-kernel-
> owner@vger.kernel.org] On Behalf Of Dmitry Torokhov
> > Hi Dmitry,
> > Sorry, I didn't realize it's not public... :-)
> >
> > I'll remove the line and re-send the patch.
> >
> 
> No need to resend, I already applied it.
> 
Dmitry,
That's great!  :-)

Thanks a lot!

-- Dexuan

^ permalink raw reply

* [PATCH 1/2] Input: atmel_mxt_ts - Get IRQ edge/level flags on DT booting
From: Javier Martinez Canillas @ 2014-08-07  0:48 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Nick Dyer, Stephen Warren, Yufeng Shen, Benson Leung,
	Doug Anderson, Olof Johansson, linux-input, devicetree,
	linux-samsung-soc, linux-kernel, Javier Martinez Canillas

The Atmel maXTouch driver assumed that the IRQ type flags will
always be passed using platform data but this is not true when
booting using Device Trees. In these setups the interrupt type
was ignored by the driver when requesting an IRQ.

This means that it will fail if a machine specified other type
than IRQ_TYPE_NONE. The right approach is to get the IRQ flags
that was parsed by OF from the "interrupt" Device Tree propery.

Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---
 drivers/input/touchscreen/atmel_mxt_ts.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 03b8571..0fb56c9 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -22,6 +22,7 @@
 #include <linux/i2c.h>
 #include <linux/i2c/atmel_mxt_ts.h>
 #include <linux/input/mt.h>
+#include <linux/irq.h>
 #include <linux/interrupt.h>
 #include <linux/of.h>
 #include <linux/slab.h>
@@ -2130,6 +2131,7 @@ static int mxt_probe(struct i2c_client *client, const struct i2c_device_id *id)
 	struct mxt_data *data;
 	const struct mxt_platform_data *pdata;
 	int error;
+	unsigned long irqflags;
 
 	pdata = dev_get_platdata(&client->dev);
 	if (!pdata) {
@@ -2156,8 +2158,13 @@ static int mxt_probe(struct i2c_client *client, const struct i2c_device_id *id)
 	init_completion(&data->reset_completion);
 	init_completion(&data->crc_completion);
 
+	if (client->dev.of_node)
+		irqflags = irq_get_trigger_type(client->irq);
+	else
+		irqflags = pdata->irqflags;
+
 	error = request_threaded_irq(client->irq, NULL, mxt_interrupt,
-				     pdata->irqflags | IRQF_ONESHOT,
+				     irqflags | IRQF_ONESHOT,
 				     client->name, data);
 	if (error) {
 		dev_err(&client->dev, "Failed to register interrupt\n");
-- 
2.0.0.rc2

^ permalink raw reply related

* [PATCH 2/2] Input: atmel_mxt_ts - Add keycodes array example
From: Javier Martinez Canillas @ 2014-08-07  0:48 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Nick Dyer, Stephen Warren, Yufeng Shen, Benson Leung,
	Doug Anderson, Olof Johansson, linux-input, devicetree,
	linux-samsung-soc, linux-kernel, Javier Martinez Canillas
In-Reply-To: <1407372486-25881-1-git-send-email-javier.martinez@collabora.co.uk>

The Atmel maXTouch driver allows to dynamically define the
event keycodes set that the device is capable of. This may
not be evident when reading the DT binding document so add
an example to make it easier to understand how this works.

Also, the current documentation says that the array limit
is four entries but the driver dynamically allocates the
keymap array and does not limit the array size.

Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---
 Documentation/devicetree/bindings/input/atmel,maxtouch.txt | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/input/atmel,maxtouch.txt b/Documentation/devicetree/bindings/input/atmel,maxtouch.txt
index baef432..be50476 100644
--- a/Documentation/devicetree/bindings/input/atmel,maxtouch.txt
+++ b/Documentation/devicetree/bindings/input/atmel,maxtouch.txt
@@ -11,7 +11,7 @@ Required properties:
 
 Optional properties for main touchpad device:
 
-- linux,gpio-keymap: An array of up to 4 entries indicating the Linux
+- linux,gpio-keymap: An array of entries indicating the Linux
     keycode generated by each GPIO. Linux keycodes are defined in
     <dt-bindings/input/input.h>.
 
@@ -22,4 +22,10 @@ Example:
 		reg = <0x4b>;
 		interrupt-parent = <&gpio>;
 		interrupts = <TEGRA_GPIO(W, 3) IRQ_TYPE_LEVEL_LOW>;
+		linux,gpio-keymap = < BTN_LEFT
+				      BTN_TOOL_FINGER
+				      BTN_TOOL_DOUBLETAP
+				      BTN_TOOL_TRIPLETAP
+				      BTN_TOOL_QUADTAP
+				      BTN_TOOL_QUINTTAP >;
 	};
-- 
2.0.0.rc2


^ permalink raw reply related

* [PATCH 1/1] ARM: dts: Add Peach Pit and Pi dts entry for atmel touchpad
From: Javier Martinez Canillas @ 2014-08-07  1:08 UTC (permalink / raw)
  To: Kukjin Kim
  Cc: Sjoerd Simons, Dmitry Torokhov, Nick Dyer, Stephen Warren,
	Yufeng Shen, Benson Leung, Doug Anderson, Olof Johansson,
	linux-input, devicetree, linux-samsung-soc, linux-kernel,
	Javier Martinez Canillas

From: Sjoerd Simons <sjoerd.simons@collabora.co.uk>

The Peach Pit and Pi boards have an Atmel maXTouch device.
Add the needed Device Tree nodes to support it.

Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
[javier.martinez: added linux,gpio-keymap property and changed IRQ type]
Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---

With only this patch the touchpad is probed but interrupts are not being
generated. The following is needed to have a fully functional touchpad:

[PATCH 1/2] "Input: atmel_mxt_ts - Get IRQ edge/level flags on DT booting"
https://lkml.org/lkml/2014/8/6/585

 arch/arm/boot/dts/exynos5420-peach-pit.dts | 29 +++++++++++++++++++++++++++++
 arch/arm/boot/dts/exynos5800-peach-pi.dts  | 29 +++++++++++++++++++++++++++++
 2 files changed, 58 insertions(+)

diff --git a/arch/arm/boot/dts/exynos5420-peach-pit.dts b/arch/arm/boot/dts/exynos5420-peach-pit.dts
index 228a6b1..7dce444 100644
--- a/arch/arm/boot/dts/exynos5420-peach-pit.dts
+++ b/arch/arm/boot/dts/exynos5420-peach-pit.dts
@@ -11,6 +11,7 @@
 /dts-v1/;
 #include <dt-bindings/input/input.h>
 #include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/interrupt-controller/irq.h>
 #include "exynos5420.dtsi"
 
 / {
@@ -157,6 +158,27 @@
 	};
 };
 
+&hsi2c_8 {
+	status = "okay";
+	clock-frequency = <333000>;
+
+	trackpad@4b {
+		compatible="atmel,maxtouch";
+		reg=<0x4b>;
+		interrupt-parent=<&gpx1>;
+		interrupts=<1 IRQ_TYPE_EDGE_FALLING>;
+		wakeup-source;
+		pinctrl-names = "default";
+		pinctrl-0 = <&trackpad_irq>;
+		linux,gpio-keymap = < BTN_LEFT
+				      BTN_TOOL_FINGER
+				      BTN_TOOL_DOUBLETAP
+				      BTN_TOOL_TRIPLETAP
+				      BTN_TOOL_QUADTAP
+				      BTN_TOOL_QUINTTAP >;
+	};
+};
+
 &hsi2c_9 {
 	status = "okay";
 	clock-frequency = <400000>;
@@ -249,6 +271,13 @@
 		samsung,pin-drv = <0>;
 	};
 
+	trackpad_irq: trackpad-irq {
+		samsung,pins = "gpx1-1";
+		samsung,pin-function = <0>;
+		samsung,pin-pud = <0>;
+		samsung,pin-drv = <0>;
+	};
+
 	power_key_irq: power-key-irq {
 		samsung,pins = "gpx1-2";
 		samsung,pin-function = <0>;
diff --git a/arch/arm/boot/dts/exynos5800-peach-pi.dts b/arch/arm/boot/dts/exynos5800-peach-pi.dts
index f3ee48b..de946b3 100644
--- a/arch/arm/boot/dts/exynos5800-peach-pi.dts
+++ b/arch/arm/boot/dts/exynos5800-peach-pi.dts
@@ -11,6 +11,7 @@
 /dts-v1/;
 #include <dt-bindings/input/input.h>
 #include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/interrupt-controller/irq.h>
 #include "exynos5800.dtsi"
 
 / {
@@ -155,6 +156,27 @@
 	};
 };
 
+&hsi2c_8 {
+	status = "okay";
+	clock-frequency = <333000>;
+
+	trackpad@4b {
+		compatible="atmel,maxtouch";
+		reg=<0x4b>;
+		interrupt-parent=<&gpx1>;
+		interrupts=<1 IRQ_TYPE_EDGE_FALLING>;
+		wakeup-source;
+		pinctrl-names = "default";
+		pinctrl-0 = <&trackpad_irq>;
+		linux,gpio-keymap = < BTN_LEFT
+				      BTN_TOOL_FINGER
+				      BTN_TOOL_DOUBLETAP
+				      BTN_TOOL_TRIPLETAP
+				      BTN_TOOL_QUADTAP
+				      BTN_TOOL_QUINTTAP >;
+	};
+};
+
 &hsi2c_9 {
 	status = "okay";
 	clock-frequency = <400000>;
@@ -247,6 +269,13 @@
 		samsung,pin-drv = <0>;
 	};
 
+	trackpad_irq: trackpad-irq {
+		samsung,pins = "gpx1-1";
+		samsung,pin-function = <0>;
+		samsung,pin-pud = <0>;
+		samsung,pin-drv = <0>;
+	};
+
 	power_key_irq: power-key-irq {
 		samsung,pins = "gpx1-2";
 		samsung,pin-function = <0>;
-- 
2.0.0.rc2


^ permalink raw reply related

* Re: [PATCH 1/2] Input: atmel_mxt_ts - Get IRQ edge/level flags on DT booting
From: Tomasz Figa @ 2014-08-07  1:14 UTC (permalink / raw)
  To: Javier Martinez Canillas, Dmitry Torokhov
  Cc: Nick Dyer, Stephen Warren, Yufeng Shen, Benson Leung,
	Doug Anderson, Olof Johansson, linux-input, devicetree,
	linux-samsung-soc, linux-kernel
In-Reply-To: <1407372486-25881-1-git-send-email-javier.martinez@collabora.co.uk>

Hi Javier,

On 07.08.2014 02:48, Javier Martinez Canillas wrote:
> The Atmel maXTouch driver assumed that the IRQ type flags will
> always be passed using platform data but this is not true when
> booting using Device Trees. In these setups the interrupt type
> was ignored by the driver when requesting an IRQ.
> 
> This means that it will fail if a machine specified other type
> than IRQ_TYPE_NONE. The right approach is to get the IRQ flags
> that was parsed by OF from the "interrupt" Device Tree propery.

Have you observed an actual failure due to this? I believe that
irq_of_parse_and_map() already sets up IRQ trigger type based on DT
data, by calling irq_create_of_mapping() which in turn calls
irq_set_irq_type().

> 
> Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
> ---
>  drivers/input/touchscreen/atmel_mxt_ts.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
> index 03b8571..0fb56c9 100644
> --- a/drivers/input/touchscreen/atmel_mxt_ts.c
> +++ b/drivers/input/touchscreen/atmel_mxt_ts.c
> @@ -22,6 +22,7 @@
>  #include <linux/i2c.h>
>  #include <linux/i2c/atmel_mxt_ts.h>
>  #include <linux/input/mt.h>
> +#include <linux/irq.h>
>  #include <linux/interrupt.h>
>  #include <linux/of.h>
>  #include <linux/slab.h>
> @@ -2130,6 +2131,7 @@ static int mxt_probe(struct i2c_client *client, const struct i2c_device_id *id)
>  	struct mxt_data *data;
>  	const struct mxt_platform_data *pdata;
>  	int error;
> +	unsigned long irqflags;
>  
>  	pdata = dev_get_platdata(&client->dev);
>  	if (!pdata) {
> @@ -2156,8 +2158,13 @@ static int mxt_probe(struct i2c_client *client, const struct i2c_device_id *id)
>  	init_completion(&data->reset_completion);
>  	init_completion(&data->crc_completion);
>  
> +	if (client->dev.of_node)
> +		irqflags = irq_get_trigger_type(client->irq);

It might be a bit cleaner to just assign the flags to pdata->irqflags in
mxt_parse_dt() instead. That would also account for the fact that pdata,
if provided, should have priority over DT.

Best regards,
Tomasz

^ permalink raw reply

* Re: [PATCH 1/1] ARM: dts: Add Peach Pit and Pi dts entry for atmel touchpad
From: Fabio Estevam @ 2014-08-07  1:35 UTC (permalink / raw)
  To: Javier Martinez Canillas
  Cc: Kukjin Kim, Sjoerd Simons, Dmitry Torokhov, Nick Dyer,
	Stephen Warren, Yufeng Shen, Benson Leung, Doug Anderson,
	Olof Johansson, linux-input, devicetree@vger.kernel.org,
	linux-samsung-soc, linux-kernel
In-Reply-To: <1407373713-26281-1-git-send-email-javier.martinez@collabora.co.uk>

On Wed, Aug 6, 2014 at 10:08 PM, Javier Martinez Canillas
<javier.martinez@collabora.co.uk> wrote:

> +&hsi2c_8 {
> +       status = "okay";
> +       clock-frequency = <333000>;

Doesn't it work at the more standard 400kHz i2c frequency?

^ permalink raw reply

* Re: [PATCH 1/2] Input: atmel_mxt_ts - Get IRQ edge/level flags on DT booting
From: Javier Martinez Canillas @ 2014-08-07  1:47 UTC (permalink / raw)
  To: Tomasz Figa, Dmitry Torokhov
  Cc: Nick Dyer, Stephen Warren, Yufeng Shen, Benson Leung,
	Doug Anderson, Olof Johansson, linux-input, devicetree,
	linux-samsung-soc, linux-kernel
In-Reply-To: <53E2D2F8.7010606@gmail.com>

Hello Tomasz,

Thanks a lot for your feedback.

On 08/07/2014 03:14 AM, Tomasz Figa wrote:
> Hi Javier,
> 
> 
> Have you observed an actual failure due to this? I believe that

Yes, I found this issue since the driver was not taking into account the value
defined in the edge/level type cells from the "interrupts" DT property.

Only doing the change in the following patch was not enough:

[PATCH 1/1] ARM: dts: Add Peach Pit and Pi dts entry for atmel touchpad [0].

> irq_of_parse_and_map() already sets up IRQ trigger type based on DT
> data, by calling irq_create_of_mapping() which in turn calls
> irq_set_irq_type().
>

Right but somehow when the IRQ is actually requested the type is overwritten by
the value passed to request_threaded_irq() and interrupts are not being
generated by the device without this patch.

Do you think that this is a bug in the "interrupt-parent" irqchip driver or the
IRQ core? I'm not that familiar with the IRQ subsystem.

>>  
>> +	if (client->dev.of_node)
>> +		irqflags = irq_get_trigger_type(client->irq);
> 
> It might be a bit cleaner to just assign the flags to pdata->irqflags in
> mxt_parse_dt() instead. That would also account for the fact that pdata,
> if provided, should have priority over DT.
> 

You are totally right, also this will break if CONFIG_OF is not enabled since
dev.of_node will not be defined. While this already is taken into account for
mxt_parse_dt() by defining an empty function.

I'll change it in v2 if getting the flags from the driver is the right approach
instead of fixing the irqchip driver or the IRQ core.

> Best regards,
> Tomasz
> 

Best regards,
Javier

[0]: http://www.spinics.net/lists/kernel/msg1802099.html

^ permalink raw reply

* Re: [PATCH 1/1] ARM: dts: Add Peach Pit and Pi dts entry for atmel touchpad
From: Javier Martinez Canillas @ 2014-08-07  1:49 UTC (permalink / raw)
  To: Fabio Estevam
  Cc: Kukjin Kim, Sjoerd Simons, Dmitry Torokhov, Nick Dyer,
	Stephen Warren, Yufeng Shen, Benson Leung, Doug Anderson,
	Olof Johansson, linux-input, devicetree@vger.kernel.org,
	linux-samsung-soc, linux-kernel
In-Reply-To: <CAOMZO5CsUKuAcDMzHhy32457ihGqEBTdUijDbwA7QwUqq-ukbQ@mail.gmail.com>

Hello Fabio,

On 08/07/2014 03:35 AM, Fabio Estevam wrote:
> On Wed, Aug 6, 2014 at 10:08 PM, Javier Martinez Canillas
> <javier.martinez@collabora.co.uk> wrote:
> 
>> +&hsi2c_8 {
>> +       status = "okay";
>> +       clock-frequency = <333000>;
> 
> Doesn't it work at the more standard 400kHz i2c frequency?
> 

Most bits of this DTS snippet have been taken from the downstream Chrome OS 3.8
kernel so I'll let one of the Chromium folks to answer this question.

Best regards,
Javier

^ permalink raw reply

* Re: [PATCH 1/1] ARM: dts: Add Peach Pit and Pi dts entry for atmel touchpad
From: Doug Anderson @ 2014-08-07  2:18 UTC (permalink / raw)
  To: Fabio Estevam
  Cc: Javier Martinez Canillas, Kukjin Kim, Sjoerd Simons,
	Dmitry Torokhov, Nick Dyer, Stephen Warren, Yufeng Shen,
	Benson Leung, Olof Johansson, linux-input@vger.kernel.org,
	devicetree@vger.kernel.org, linux-samsung-soc, linux-kernel
In-Reply-To: <CAOMZO5CsUKuAcDMzHhy32457ihGqEBTdUijDbwA7QwUqq-ukbQ@mail.gmail.com>

Fabio,

On Wed, Aug 6, 2014 at 6:35 PM, Fabio Estevam <festevam@gmail.com> wrote:
> On Wed, Aug 6, 2014 at 10:08 PM, Javier Martinez Canillas
> <javier.martinez@collabora.co.uk> wrote:
>
>> +&hsi2c_8 {
>> +       status = "okay";
>> +       clock-frequency = <333000>;
>
> Doesn't it work at the more standard 400kHz i2c frequency?

I'm pretty sure that they had signaling problems at 400kHz, but
perhaps Benson will rememer.

^ permalink raw reply

* [PATCH] HID:hid-logitech: Prevent possibility of infinite loop when using /sys interface
From: Simon Wood @ 2014-08-07  2:52 UTC (permalink / raw)
  To: linux-input; +Cc: linux-kernel, Jiri Kosina, Simon Wood

If the device data is not accessible for some reason, returning 0 will cause the call to be
continuously called again as none of the string has been 'consumed'.
---
 drivers/hid/hid-lg4ff.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/hid-lg4ff.c b/drivers/hid/hid-lg4ff.c
index cc2bd20..7835717 100644
--- a/drivers/hid/hid-lg4ff.c
+++ b/drivers/hid/hid-lg4ff.c
@@ -451,13 +451,13 @@ static ssize_t lg4ff_range_store(struct device *dev, struct device_attribute *at
 	drv_data = hid_get_drvdata(hid);
 	if (!drv_data) {
 		hid_err(hid, "Private driver data not found!\n");
-		return 0;
+		return -EINVAL;
 	}
 
 	entry = drv_data->device_props;
 	if (!entry) {
 		hid_err(hid, "Device properties not found!\n");
-		return 0;
+		return -EINVAL;
 	}
 
 	if (range == 0)
-- 
1.9.1


^ permalink raw reply related

* [PATCH] Input: imx_keypad - use dev_get_drvdata()
From: Fabio Estevam @ 2014-08-07  4:40 UTC (permalink / raw)
  To: dmitry.torokhov; +Cc: linux-input, Fabio Estevam

From: Fabio Estevam <fabio.estevam@freescale.com>

There is no need to first get a platform_device structure in the suspend/resume
functions. 

Make the code simpler by calling dev_get_drvdata() directly.

Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
 drivers/input/keyboard/imx_keypad.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/input/keyboard/imx_keypad.c b/drivers/input/keyboard/imx_keypad.c
index 20a99c3..56f6e99 100644
--- a/drivers/input/keyboard/imx_keypad.c
+++ b/drivers/input/keyboard/imx_keypad.c
@@ -533,8 +533,7 @@ static int imx_keypad_probe(struct platform_device *pdev)
 
 static int __maybe_unused imx_kbd_suspend(struct device *dev)
 {
-	struct platform_device *pdev = to_platform_device(dev);
-	struct imx_keypad *kbd = platform_get_drvdata(pdev);
+	struct imx_keypad *kbd = dev_get_drvdata(dev);
 	struct input_dev *input_dev = kbd->input_dev;
 
 	/* imx kbd can wake up system even clock is disabled */
@@ -545,7 +544,7 @@ static int __maybe_unused imx_kbd_suspend(struct device *dev)
 
 	mutex_unlock(&input_dev->mutex);
 
-	if (device_may_wakeup(&pdev->dev))
+	if (device_may_wakeup(dev))
 		enable_irq_wake(kbd->irq);
 
 	return 0;
@@ -553,12 +552,11 @@ static int __maybe_unused imx_kbd_suspend(struct device *dev)
 
 static int __maybe_unused imx_kbd_resume(struct device *dev)
 {
-	struct platform_device *pdev = to_platform_device(dev);
-	struct imx_keypad *kbd = platform_get_drvdata(pdev);
+	struct imx_keypad *kbd = dev_get_drvdata(dev);
 	struct input_dev *input_dev = kbd->input_dev;
 	int ret = 0;
 
-	if (device_may_wakeup(&pdev->dev))
+	if (device_may_wakeup(dev))
 		disable_irq_wake(kbd->irq);
 
 	mutex_lock(&input_dev->mutex);
-- 
1.9.1


^ permalink raw reply related

* Re: [PATCH 1/2] Input: atmel_mxt_ts - Get IRQ edge/level flags on DT booting
From: Dmitry Torokhov @ 2014-08-07  6:09 UTC (permalink / raw)
  To: Javier Martinez Canillas
  Cc: Tomasz Figa, Nick Dyer, Stephen Warren, Yufeng Shen, Benson Leung,
	Doug Anderson, Olof Johansson, linux-input, devicetree,
	linux-samsung-soc, linux-kernel
In-Reply-To: <53E2DAAC.1060602@collabora.co.uk>

On Thu, Aug 07, 2014 at 03:47:24AM +0200, Javier Martinez Canillas wrote:
> Hello Tomasz,
> 
> Thanks a lot for your feedback.
> 
> On 08/07/2014 03:14 AM, Tomasz Figa wrote:
> > Hi Javier,
> > 
> > 
> > Have you observed an actual failure due to this? I believe that
> 
> Yes, I found this issue since the driver was not taking into account the value
> defined in the edge/level type cells from the "interrupts" DT property.
> 
> Only doing the change in the following patch was not enough:
> 
> [PATCH 1/1] ARM: dts: Add Peach Pit and Pi dts entry for atmel touchpad [0].
> 
> > irq_of_parse_and_map() already sets up IRQ trigger type based on DT
> > data, by calling irq_create_of_mapping() which in turn calls
> > irq_set_irq_type().
> >
> 
> Right but somehow when the IRQ is actually requested the type is overwritten by
> the value passed to request_threaded_irq() and interrupts are not being
> generated by the device without this patch.
> 
> Do you think that this is a bug in the "interrupt-parent" irqchip driver or the
> IRQ core? I'm not that familiar with the IRQ subsystem.

No, this is clearly driver fault - it smashed previously done setup with new
flags.

> 
> >>  
> >> +	if (client->dev.of_node)
> >> +		irqflags = irq_get_trigger_type(client->irq);
> > 
> > It might be a bit cleaner to just assign the flags to pdata->irqflags in
> > mxt_parse_dt() instead. That would also account for the fact that pdata,
> > if provided, should have priority over DT.
> > 
> 
> You are totally right, also this will break if CONFIG_OF is not enabled since
> dev.of_node will not be defined. While this already is taken into account for
> mxt_parse_dt() by defining an empty function.
> 
> I'll change it in v2 if getting the flags from the driver is the right approach

Yes, please.

> instead of fixing the irqchip driver or the IRQ core.

Thanks.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH] Input: imx_keypad - use dev_get_drvdata()
From: Dmitry Torokhov @ 2014-08-07  6:12 UTC (permalink / raw)
  To: Fabio Estevam; +Cc: linux-input, Fabio Estevam
In-Reply-To: <1407386457-32587-1-git-send-email-festevam@gmail.com>

Hi Fabio,

On Thu, Aug 07, 2014 at 01:40:57AM -0300, Fabio Estevam wrote:
> From: Fabio Estevam <fabio.estevam@freescale.com>
> 
> There is no need to first get a platform_device structure in the suspend/resume
> functions. 
> 
> Make the code simpler by calling dev_get_drvdata() directly.

I actually believe that we should be using  the proper accessors fro the type.
The fact that they resolve to the same data object is implementation detail, as
far as I am concerned (unless driver core folks want to deprecate or get rid of
everything but dev_{get|set}_drvdata()).

Thanks.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH v5] input: drv260x: Add TI drv260x haptics driver
From: Dmitry Torokhov @ 2014-08-07  6:16 UTC (permalink / raw)
  Cc: linux-input@vger.kernel.org, linux-kernel@vger.kernel.org,
	devicetree@vger.kernel.org, mark.rutland@arm.com,
	madcatxster@devoid-pointer.net, simon@mungewell.org,
	elias.vds@gmail.com, Dan Murphy
In-Reply-To: <00FC9A978A94B7418C33AFAE8A35ED49DCAF45@DFLE09.ent.ti.com>

On Wed, Aug 06, 2014 at 07:47:25PM +0000, Murphy, Dan wrote:
> Bump
> 
> On 07/31/2014 02:14 PM, Murphy, Dan wrote:
> > Add the TI drv260x haptics/vibrator driver.
> > This device uses the input force feedback
> > to produce a wave form to driver an
> > ERM or LRA actuator device.
> > 
> > The initial driver supports the devices
> > real time playback mode.  But the device
> > has additional wave patterns in ROM.
> > 
> > This functionality will be added in
> > future patchsets.
> > 
> > Product data sheet is located here:
> > http://www.ti.com/product/drv2605
> > 
> > Signed-off-by: Dan Murphy <dmurphy@ti.com>
> > ---
> > 
> > v5 - Move register defines to c file and rm header file, error check
> > init in probe, fixed identation, remove empty labels in probe
> > and just return on fail and removed the remove callback and function.
> > Did not factor out the suspend into a stop function. - https://patchwork.kernel.org/patch/4649631/
> > v4 - Convert regulator to devm, added error checking where required,
> > updated bindings doc, moved dt parsing to separate call and made platform

Mark, are you OK with this new updated binding?

> > data the first data point, moved vibrator enable and mode programming from
> > play entry to worker thread, added user check and input mutex in suspend/resume
> > calls, moved platform data to individual file and into include platform-data directory,
> > removed file names from file headers - https://patchwork.kernel.org/patch/4642221/
> > v3 - Updated binding doc, changed to memless device, updated input alloc to
> > devm, removed mutex locking, add sanity checks for mode and library - https://patchwork.kernel.org/patch/4635421/
> > v2 - Fixed binding doc and patch headline - https://patchwork.kernel.org/patch/4619641/
> > 
> > 
> >  .../devicetree/bindings/input/ti,drv260x.txt       |   50 ++
> >  drivers/input/misc/Kconfig                         |    9 +
> >  drivers/input/misc/Makefile                        |    1 +
> >  drivers/input/misc/drv260x.c                       |  697 ++++++++++++++++++++
> >  include/dt-bindings/input/ti-drv260x.h             |   35 +
> >  include/linux/platform_data/drv260x-pdata.h        |   29 +
> >  6 files changed, 821 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/input/ti,drv260x.txt
> >  create mode 100644 drivers/input/misc/drv260x.c
> >  create mode 100644 include/dt-bindings/input/ti-drv260x.h
> >  create mode 100644 include/linux/platform_data/drv260x-pdata.h
> > 
> > diff --git a/Documentation/devicetree/bindings/input/ti,drv260x.txt b/Documentation/devicetree/bindings/input/ti,drv260x.txt
> > new file mode 100644
> > index 0000000..8e6970d
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/input/ti,drv260x.txt
> > @@ -0,0 +1,50 @@
> > +Texas Instruments - drv260x Haptics driver family
> > +
> > +The drv260x family serial control bus communicates through I2C protocols
> > +
> > +Required properties:
> > +	- compatible - One of:
> > +		"ti,drv2604" - DRV2604
> > +		"ti,drv2605" - DRV2605
> > +		"ti,drv2605l" - DRV2605L
> > +	- reg -  I2C slave address
> > +	- supply- Required supply regulators are:
> > +		"vbat" - battery voltage
> > +	- mode - Power up mode of the chip (defined in include/dt-bindings/input/ti-drv260x.h)
> > +		DRV260X_LRA_MODE - Linear Resonance Actuator mode (Piezoelectric)
> > +		DRV260X_LRA_NO_CAL_MODE - This is a LRA Mode but there is no calibration
> > +				sequence during init.  And the device is configured for real
> > +				time playback mode (RTP mode).
> > +		DRV260X_ERM_MODE - Eccentric Rotating Mass mode (Rotary vibrator)
> > +	- library-sel - These are ROM based waveforms pre-programmed into the IC.
> > +				This should be set to set the library to use at power up.
> > +				(defined in include/dt-bindings/input/ti-drv260x.h)
> > +		DRV260X_LIB_A - Pre-programmed Library
> > +		DRV260X_LIB_B - Pre-programmed Library
> > +		DRV260X_LIB_C - Pre-programmed Library
> > +		DRV260X_LIB_D - Pre-programmed Library
> > +		DRV260X_LIB_E - Pre-programmed Library
> > +		DRV260X_LIB_F - Pre-programmed Library
> > +
> > +Optional properties:
> > +	- enable-gpio - gpio pin to enable/disable the device.
> > +	- vib_rated_voltage - The rated voltage of the actuator in millivolts.
> > +			  If this is not set then the value will be defaulted to
> > +			  3.2 v.
> > +	- vib_overdrive_voltage - The overdrive voltage of the actuator in millivolts.
> > +			  If this is not set then the value will be defaulted to
> > +			  3.2 v.
> > +Example:
> > +
> > +drv2605l: drv2605l@5a {
> > +		compatible = "ti,drv2605l";
> > +		reg = <0x5a>;
> > +		enable-gpio = <&gpio1 28 GPIO_ACTIVE_HIGH>;
> > +		mode = <DRV260X_LRA_MODE>;
> > +		library-sel = <DRV260X_LIB_SEL_DEFAULT>;
> > +		vib-rated-voltage = <3200>;
> > +		vib-overdriver-voltage = <3200>;
> > +};
> > +
> > +For more product information please see the link below:
> > +http://www.ti.com/product/drv2605
> > diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
> > index 2ff4425..99f6762 100644
> > --- a/drivers/input/misc/Kconfig
> > +++ b/drivers/input/misc/Kconfig
> > @@ -676,4 +676,13 @@ config INPUT_SOC_BUTTON_ARRAY
> >  	  To compile this driver as a module, choose M here: the
> >  	  module will be called soc_button_array.
> >  
> > +config INPUT_DRV260X_HAPTICS
> > +	tristate "TI DRV260X haptics support"
> > +	depends on INPUT && I2C
> > +	help
> > +	  Say Y to enable support for the TI DRV260X haptics driver.
> > +
> > +	  To compile this driver as a module, choose M here: the
> > +	  module will be called drv260x-haptics.
> > +
> >  endif
> > diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
> > index 4955ad3..d8ef3c7 100644
> > --- a/drivers/input/misc/Makefile
> > +++ b/drivers/input/misc/Makefile
> > @@ -64,3 +64,4 @@ obj-$(CONFIG_INPUT_WM831X_ON)		+= wm831x-on.o
> >  obj-$(CONFIG_INPUT_XEN_KBDDEV_FRONTEND)	+= xen-kbdfront.o
> >  obj-$(CONFIG_INPUT_YEALINK)		+= yealink.o
> >  obj-$(CONFIG_INPUT_IDEAPAD_SLIDEBAR)	+= ideapad_slidebar.o
> > +obj-$(CONFIG_INPUT_DRV260X_HAPTICS)	+= drv260x.o
> > diff --git a/drivers/input/misc/drv260x.c b/drivers/input/misc/drv260x.c
> > new file mode 100644
> > index 0000000..6ccdeff
> > --- /dev/null
> > +++ b/drivers/input/misc/drv260x.c
> > @@ -0,0 +1,697 @@
> > +/*
> > + * DRV260X haptics driver family
> > + *
> > + * Author: Dan Murphy <dmurphy@ti.com>
> > + *
> > + * Copyright:   (C) 2014 Texas Instruments, Inc.
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope that it will be useful, but
> > + * WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > + * General Public License for more details.
> > + */
> > +
> > +#include <linux/i2c.h>
> > +#include <linux/input.h>
> > +#include <linux/module.h>
> > +#include <linux/of_gpio.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/regmap.h>
> > +#include <linux/slab.h>
> > +#include <linux/delay.h>
> > +#include <linux/regulator/consumer.h>
> > +
> > +#include <dt-bindings/input/ti-drv260x.h>
> > +#include <linux/platform_data/drv260x-pdata.h>
> > +
> > +#define DRV260X_STATUS		0x0
> > +#define DRV260X_MODE		0x1
> > +#define DRV260X_RT_PB_IN	0x2
> > +#define DRV260X_LIB_SEL		0x3
> > +#define DRV260X_WV_SEQ_1	0x4
> > +#define DRV260X_WV_SEQ_2	0x5
> > +#define DRV260X_WV_SEQ_3	0x6
> > +#define DRV260X_WV_SEQ_4	0x7
> > +#define DRV260X_WV_SEQ_5	0x8
> > +#define DRV260X_WV_SEQ_6	0x9
> > +#define DRV260X_WV_SEQ_7	0xa
> > +#define DRV260X_WV_SEQ_8	0xb
> > +#define DRV260X_GO				0xc
> > +#define DRV260X_OVERDRIVE_OFF	0xd
> > +#define DRV260X_SUSTAIN_P_OFF	0xe
> > +#define DRV260X_SUSTAIN_N_OFF	0xf
> > +#define DRV260X_BRAKE_OFF		0x10
> > +#define DRV260X_A_TO_V_CTRL		0x11
> > +#define DRV260X_A_TO_V_MIN_INPUT	0x12
> > +#define DRV260X_A_TO_V_MAX_INPUT	0x13
> > +#define DRV260X_A_TO_V_MIN_OUT	0x14
> > +#define DRV260X_A_TO_V_MAX_OUT	0x15
> > +#define DRV260X_RATED_VOLT		0x16
> > +#define DRV260X_OD_CLAMP_VOLT	0x17
> > +#define DRV260X_CAL_COMP		0x18
> > +#define DRV260X_CAL_BACK_EMF	0x19
> > +#define DRV260X_FEEDBACK_CTRL	0x1a
> > +#define DRV260X_CTRL1			0x1b
> > +#define DRV260X_CTRL2			0x1c
> > +#define DRV260X_CTRL3			0x1d
> > +#define DRV260X_CTRL4			0x1e
> > +#define DRV260X_CTRL5			0x1f
> > +#define DRV260X_LRA_LOOP_PERIOD	0x20
> > +#define DRV260X_VBAT_MON		0x21
> > +#define DRV260X_LRA_RES_PERIOD	0x22
> > +#define DRV260X_MAX_REG			0x23
> > +
> > +#define DRV260X_ALLOWED_R_BYTES	25
> > +#define DRV260X_ALLOWED_W_BYTES	2
> > +#define DRV260X_MAX_RW_RETRIES	5
> > +#define DRV260X_I2C_RETRY_DELAY 10
> > +
> > +#define DRV260X_GO_BIT				0x01
> > +
> > +/* Library Selection */
> > +#define DRV260X_LIB_SEL_MASK		0x07
> > +#define DRV260X_LIB_SEL_RAM			0x0
> > +#define DRV260X_LIB_SEL_OD			0x1
> > +#define DRV260X_LIB_SEL_40_60		0x2
> > +#define DRV260X_LIB_SEL_60_80		0x3
> > +#define DRV260X_LIB_SEL_100_140		0x4
> > +#define DRV260X_LIB_SEL_140_PLUS	0x5
> > +
> > +#define DRV260X_LIB_SEL_HIZ_MASK	0x10
> > +#define DRV260X_LIB_SEL_HIZ_EN		0x01
> > +#define DRV260X_LIB_SEL_HIZ_DIS		0
> > +
> > +/* Mode register */
> > +#define DRV260X_STANDBY				(1 << 6)
> > +#define DRV260X_STANDBY_MASK		0x40
> > +#define DRV260X_INTERNAL_TRIGGER	0x00
> > +#define DRV260X_EXT_TRIGGER_EDGE	0x01
> > +#define DRV260X_EXT_TRIGGER_LEVEL	0x02
> > +#define DRV260X_PWM_ANALOG_IN		0x03
> > +#define DRV260X_AUDIOHAPTIC			0x04
> > +#define DRV260X_RT_PLAYBACK			0x05
> > +#define DRV260X_DIAGNOSTICS			0x06
> > +#define DRV260X_AUTO_CAL			0x07
> > +
> > +/* Audio to Haptics Control */
> > +#define DRV260X_AUDIO_HAPTICS_PEAK_10MS		(0 << 2)
> > +#define DRV260X_AUDIO_HAPTICS_PEAK_20MS		(1 << 2)
> > +#define DRV260X_AUDIO_HAPTICS_PEAK_30MS		(2 << 2)
> > +#define DRV260X_AUDIO_HAPTICS_PEAK_40MS		(3 << 2)
> > +
> > +#define DRV260X_AUDIO_HAPTICS_FILTER_100HZ	0x00
> > +#define DRV260X_AUDIO_HAPTICS_FILTER_125HZ	0x01
> > +#define DRV260X_AUDIO_HAPTICS_FILTER_150HZ	0x02
> > +#define DRV260X_AUDIO_HAPTICS_FILTER_200HZ	0x03
> > +
> > +/* Min/Max Input/Output Voltages */
> > +#define DRV260X_AUDIO_HAPTICS_MIN_IN_VOLT	0x19
> > +#define DRV260X_AUDIO_HAPTICS_MAX_IN_VOLT	0x64
> > +#define DRV260X_AUDIO_HAPTICS_MIN_OUT_VOLT	0x19
> > +#define DRV260X_AUDIO_HAPTICS_MAX_OUT_VOLT	0xFF
> > +
> > +/* Feedback register */
> > +#define DRV260X_FB_REG_ERM_MODE			0x7f
> > +#define DRV260X_FB_REG_LRA_MODE			(1 << 7)
> > +
> > +#define DRV260X_BRAKE_FACTOR_MASK	0x1f
> > +#define DRV260X_BRAKE_FACTOR_2X		(1 << 0)
> > +#define DRV260X_BRAKE_FACTOR_3X		(2 << 4)
> > +#define DRV260X_BRAKE_FACTOR_4X		(3 << 4)
> > +#define DRV260X_BRAKE_FACTOR_6X		(4 << 4)
> > +#define DRV260X_BRAKE_FACTOR_8X		(5 << 4)
> > +#define DRV260X_BRAKE_FACTOR_16		(6 << 4)
> > +#define DRV260X_BRAKE_FACTOR_DIS	(7 << 4)
> > +
> > +#define DRV260X_LOOP_GAIN_LOW		0xf3
> > +#define DRV260X_LOOP_GAIN_MED		(1 << 2)
> > +#define DRV260X_LOOP_GAIN_HIGH		(2 << 2)
> > +#define DRV260X_LOOP_GAIN_VERY_HIGH	(3 << 2)
> > +
> > +#define DRV260X_BEMF_GAIN_0			0xfc
> > +#define DRV260X_BEMF_GAIN_1		(1 << 0)
> > +#define DRV260X_BEMF_GAIN_2		(2 << 0)
> > +#define DRV260X_BEMF_GAIN_3		(3 << 0)
> > +
> > +/* Control 1 register */
> > +#define DRV260X_AC_CPLE_EN			(1 << 5)
> > +#define DRV260X_STARTUP_BOOST		(1 << 7)
> > +
> > +/* Control 2 register */
> > +
> > +#define DRV260X_IDISS_TIME_45		0
> > +#define DRV260X_IDISS_TIME_75		(1 << 0)
> > +#define DRV260X_IDISS_TIME_150		(1 << 1)
> > +#define DRV260X_IDISS_TIME_225		0x03
> > +
> > +#define DRV260X_BLANK_TIME_45	(0 << 2)
> > +#define DRV260X_BLANK_TIME_75	(1 << 2)
> > +#define DRV260X_BLANK_TIME_150	(2 << 2)
> > +#define DRV260X_BLANK_TIME_225	(3 << 2)
> > +
> > +#define DRV260X_SAMP_TIME_150	(0 << 4)
> > +#define DRV260X_SAMP_TIME_200	(1 << 4)
> > +#define DRV260X_SAMP_TIME_250	(2 << 4)
> > +#define DRV260X_SAMP_TIME_300	(3 << 4)
> > +
> > +#define DRV260X_BRAKE_STABILIZER	(1 << 6)
> > +#define DRV260X_UNIDIR_IN			(0 << 7)
> > +#define DRV260X_BIDIR_IN			(1 << 7)
> > +
> > +/* Control 3 Register */
> > +#define DRV260X_LRA_OPEN_LOOP		(1 << 0)
> > +#define DRV260X_ANANLOG_IN			(1 << 1)
> > +#define DRV260X_LRA_DRV_MODE		(1 << 2)
> > +#define DRV260X_RTP_UNSIGNED_DATA	(1 << 3)
> > +#define DRV260X_SUPPLY_COMP_DIS		(1 << 4)
> > +#define DRV260X_ERM_OPEN_LOOP		(1 << 5)
> > +#define DRV260X_NG_THRESH_0			(0 << 6)
> > +#define DRV260X_NG_THRESH_2			(1 << 6)
> > +#define DRV260X_NG_THRESH_4			(2 << 6)
> > +#define DRV260X_NG_THRESH_8			(3 << 6)
> > +
> > +/* Control 4 Register */
> > +#define DRV260X_AUTOCAL_TIME_150MS		(0 << 4)
> > +#define DRV260X_AUTOCAL_TIME_250MS		(1 << 4)
> > +#define DRV260X_AUTOCAL_TIME_500MS		(2 << 4)
> > +#define DRV260X_AUTOCAL_TIME_1000MS		(3 << 4)
> > +
> > +/**
> > + * struct drv260x_data -
> > + * @input_dev - Pointer to the input device
> > + * @client - Pointer to the I2C client
> > + * @regmap - Register map of the device
> > + * @work - Work item used to off load the enable/disable of the vibration
> > + * @enable_gpio - Pointer to the gpio used for enable/disabling
> > + * @regulator - Pointer to the regulator for the IC
> > + * @magnitude - Magnitude of the vibration event
> > + * @mode - The operating mode of the IC (LRA_NO_CAL, ERM or LRA)
> > + * @library - The vibration library to be used
> > + * @rated_voltage - The rated_voltage of the actuator
> > + * @overdriver_voltage - The over drive voltage of the actuator
> > +**/
> > +struct drv260x_data {
> > +	struct input_dev *input_dev;
> > +	struct i2c_client *client;
> > +	struct regmap *regmap;
> > +	struct work_struct work;
> > +	struct gpio_desc *enable_gpio;
> > +	struct regulator *regulator;
> > +	u32 magnitude;
> > +	u32 mode;
> > +	u32 library;
> > +	int rated_voltage;
> > +	int overdrive_voltage;
> > +};
> > +
> > +static struct reg_default drv260x_reg_defs[] = {
> > +	{ DRV260X_STATUS, 0xe0 },
> > +	{ DRV260X_MODE, 0x40 },
> > +	{ DRV260X_RT_PB_IN, 0x00},
> > +	{ DRV260X_LIB_SEL, 0x00},
> > +	{ DRV260X_WV_SEQ_1, 0x01},
> > +	{ DRV260X_WV_SEQ_2, 0x00},
> > +	{ DRV260X_WV_SEQ_3, 0x00},
> > +	{ DRV260X_WV_SEQ_4, 0x00},
> > +	{ DRV260X_WV_SEQ_5, 0x00},
> > +	{ DRV260X_WV_SEQ_6, 0x00},
> > +	{ DRV260X_WV_SEQ_7, 0x00},
> > +	{ DRV260X_WV_SEQ_8, 0x00},
> > +	{ DRV260X_GO, 0x00},
> > +	{ DRV260X_OVERDRIVE_OFF, 0x00},
> > +	{ DRV260X_SUSTAIN_P_OFF, 0x00},
> > +	{ DRV260X_SUSTAIN_N_OFF, 0x00},
> > +	{ DRV260X_BRAKE_OFF	, 0x00},
> > +	{ DRV260X_A_TO_V_CTRL	, 0x05},
> > +	{ DRV260X_A_TO_V_MIN_INPUT, 0x19},
> > +	{ DRV260X_A_TO_V_MAX_INPUT, 0xff},
> > +	{ DRV260X_A_TO_V_MIN_OUT, 0x19},
> > +	{ DRV260X_A_TO_V_MAX_OUT, 0xff},
> > +	{ DRV260X_RATED_VOLT, 0x3e},
> > +	{ DRV260X_OD_CLAMP_VOLT, 0x8c},
> > +	{ DRV260X_CAL_COMP, 0x0c},
> > +	{ DRV260X_CAL_BACK_EMF, 0x6c},
> > +	{ DRV260X_FEEDBACK_CTRL, 0x36},
> > +	{ DRV260X_CTRL1, 0x93},
> > +	{ DRV260X_CTRL2, 0xfa},
> > +	{ DRV260X_CTRL3, 0xa0},
> > +	{ DRV260X_CTRL4, 0x20},
> > +	{ DRV260X_CTRL5, 0x80},
> > +	{ DRV260X_LRA_LOOP_PERIOD, 0x33},
> > +	{ DRV260X_VBAT_MON, 0x00},
> > +	{ DRV260X_LRA_RES_PERIOD, 0x00},
> > +};
> > +
> > +/**
> > + * Rated and Overdriver Voltages:
> > + * Calculated using the formula r = v * 255 / 5.6
> > + * where r is what will be written to the register
> > + * and v is the rated or overdriver voltage of the actuator
> > + **/
> > +#define DRV260X_DEF_RATED_VOLT		0x90
> > +#define DRV260X_DEF_OD_CLAMP_VOLT	0x90
> > +
> > +static int drv260x_calculate_voltage(int voltage)
> > +{
> > +	return (voltage * 255 / 5600);
> > +}
> > +
> > +static void drv260x_worker(struct work_struct *work)
> > +{
> > +	struct drv260x_data *haptics = container_of(work, struct drv260x_data, work);
> > +	int ret;
> > +
> > +	gpiod_set_value(haptics->enable_gpio, 1);
> > +	/* Data sheet says to wait 250us before trying to communicate */
> > +	udelay(250);
> > +
> > +	ret = regmap_write(haptics->regmap, DRV260X_MODE, DRV260X_RT_PLAYBACK);
> > +	if (ret != 0) {
> > +		dev_err(&haptics->client->dev,
> > +			"Failed to write set mode: %d\n", ret);
> > +		return;
> > +	}
> > +
> > +	ret = regmap_write(haptics->regmap, DRV260X_RT_PB_IN, haptics->magnitude);
> > +	if (ret != 0) {
> > +		dev_err(&haptics->client->dev,
> > +			"Failed to set magnitude: %d\n", ret);
> > +	}
> > +}
> > +
> > +static int drv260x_haptics_play(struct input_dev *input, void *data,
> > +				struct ff_effect *effect)
> > +{
> > +	struct drv260x_data *haptics = input_get_drvdata(input);
> > +
> > +	haptics->mode = DRV260X_LRA_NO_CAL_MODE;
> > +	haptics->magnitude = 0;
> > +
> > +	if (effect->u.rumble.strong_magnitude ||
> > +		effect->u.rumble.weak_magnitude) {
> > +		if (effect->u.rumble.strong_magnitude > 0)
> > +			haptics->magnitude = effect->u.rumble.strong_magnitude;
> > +		else if	(effect->u.rumble.weak_magnitude > 0)
> > +			haptics->magnitude = effect->u.rumble.weak_magnitude;
> > +	}
> > +
> > +	schedule_work(&haptics->work);
> > +
> > +	return 0;
> > +}
> > +
> > +static void drv260x_close(struct input_dev *input)
> > +{
> > +	struct drv260x_data *haptics = input_get_drvdata(input);
> > +	int ret;
> > +
> > +	cancel_work_sync(&haptics->work);
> > +
> > +	ret = regmap_write(haptics->regmap, DRV260X_MODE, DRV260X_STANDBY);
> > +	if (ret != 0) {
> > +		dev_err(&haptics->client->dev,
> > +			"Failed to write standby mode: %d\n", ret);
> > +	}
> > +
> > +	gpiod_set_value(haptics->enable_gpio, 0);
> > +}
> > +
> > +static const struct reg_default drv260x_lra_cal_regs[] = {
> > +	{ DRV260X_MODE, DRV260X_AUTO_CAL },
> > +	{ DRV260X_CTRL3, DRV260X_NG_THRESH_2},
> > +	{ DRV260X_FEEDBACK_CTRL, DRV260X_FB_REG_LRA_MODE | DRV260X_BRAKE_FACTOR_4X | DRV260X_LOOP_GAIN_HIGH },
> > +};
> > +
> > +static const struct reg_default drv260x_lra_init_regs[] = {
> > +	{ DRV260X_MODE, DRV260X_RT_PLAYBACK},
> > +	{ DRV260X_A_TO_V_CTRL, DRV260X_AUDIO_HAPTICS_PEAK_20MS | DRV260X_AUDIO_HAPTICS_FILTER_125HZ},
> > +	{ DRV260X_A_TO_V_MIN_INPUT, DRV260X_AUDIO_HAPTICS_MIN_IN_VOLT },
> > +	{ DRV260X_A_TO_V_MAX_INPUT, DRV260X_AUDIO_HAPTICS_MAX_IN_VOLT },
> > +	{ DRV260X_A_TO_V_MIN_OUT, DRV260X_AUDIO_HAPTICS_MIN_OUT_VOLT },
> > +	{ DRV260X_A_TO_V_MAX_OUT, DRV260X_AUDIO_HAPTICS_MAX_OUT_VOLT },
> > +	{ DRV260X_FEEDBACK_CTRL, DRV260X_FB_REG_LRA_MODE | DRV260X_BRAKE_FACTOR_2X | DRV260X_LOOP_GAIN_MED | DRV260X_BEMF_GAIN_3 },
> > +	{ DRV260X_CTRL1, DRV260X_STARTUP_BOOST },
> > +	{ DRV260X_CTRL2, DRV260X_SAMP_TIME_250 },
> > +	{ DRV260X_CTRL3, DRV260X_NG_THRESH_2 | DRV260X_ANANLOG_IN },
> > +	{ DRV260X_CTRL4, DRV260X_AUTOCAL_TIME_500MS },
> > +};
> > +
> > +static const struct reg_default drv260x_erm_cal_regs[] = {
> > +	{ DRV260X_MODE, DRV260X_AUTO_CAL },
> > +	{ DRV260X_A_TO_V_MIN_INPUT, DRV260X_AUDIO_HAPTICS_MIN_IN_VOLT },
> > +	{ DRV260X_A_TO_V_MAX_INPUT, DRV260X_AUDIO_HAPTICS_MAX_IN_VOLT },
> > +	{ DRV260X_A_TO_V_MIN_OUT, DRV260X_AUDIO_HAPTICS_MIN_OUT_VOLT },
> > +	{ DRV260X_A_TO_V_MAX_OUT, DRV260X_AUDIO_HAPTICS_MAX_OUT_VOLT },
> > +	{ DRV260X_FEEDBACK_CTRL, DRV260X_BRAKE_FACTOR_3X | DRV260X_LOOP_GAIN_MED | DRV260X_BEMF_GAIN_2 },
> > +	{ DRV260X_CTRL1, DRV260X_STARTUP_BOOST },
> > +	{ DRV260X_CTRL2, DRV260X_SAMP_TIME_250 | DRV260X_BLANK_TIME_75 | DRV260X_SAMP_TIME_250 | DRV260X_IDISS_TIME_75 },
> > +	{ DRV260X_CTRL3, DRV260X_NG_THRESH_2 | DRV260X_ERM_OPEN_LOOP },
> > +	{ DRV260X_CTRL4, DRV260X_AUTOCAL_TIME_500MS },
> > +};
> > +
> > +static int drv260x_init(struct drv260x_data *haptics)
> > +{
> > +	int ret;
> > +	unsigned int cal_buf;
> > +
> > +	ret = regmap_write(haptics->regmap,
> > +			   DRV260X_RATED_VOLT, haptics->rated_voltage);
> > +	if (ret != 0)
> > +		goto write_failure;
> > +
> > +	ret = regmap_write(haptics->regmap,
> > +			   DRV260X_OD_CLAMP_VOLT, haptics->overdrive_voltage);
> > +	if (ret != 0)
> > +		goto write_failure;
> > +
> > +	switch(haptics->mode) {
> > +	case DRV260X_LRA_MODE:
> > +		ret = regmap_register_patch(haptics->regmap,
> > +					drv260x_lra_cal_regs,
> > +					ARRAY_SIZE(drv260x_lra_cal_regs));
> > +		if (ret != 0)
> > +			goto write_failure;
> > +		break;
> > +	case DRV260X_ERM_MODE:
> > +		ret = regmap_register_patch(haptics->regmap,
> > +					drv260x_erm_cal_regs,
> > +					ARRAY_SIZE(drv260x_erm_cal_regs));
> > +		if (ret != 0)
> > +			goto write_failure;
> > +
> > +		ret = regmap_update_bits(haptics->regmap, DRV260X_LIB_SEL,
> > +					DRV260X_LIB_SEL_MASK,
> > +					haptics->library);
> > +		if (ret != 0)
> > +			goto write_failure;
> > +		break;
> > +	default:
> > +		ret = regmap_register_patch(haptics->regmap,
> > +					drv260x_lra_init_regs,
> > +					ARRAY_SIZE(drv260x_lra_init_regs));
> > +		if (ret != 0)
> > +			goto write_failure;
> > +
> > +		ret = regmap_update_bits(haptics->regmap, DRV260X_LIB_SEL,
> > +					DRV260X_LIB_SEL_MASK,
> > +					haptics->library);
> > +		if (ret != 0)
> > +			goto write_failure;
> > +
> > +		goto skip_go_bit;
> > +		break;
> > +	}
> > +
> > +	if (ret != 0) {
> > +		dev_err(&haptics->client->dev,
> > +			"Failed to write init registers: %d\n",
> > +			ret);
> > +		goto write_failure;
> > +	}
> > +
> > +	ret = regmap_write(haptics->regmap, DRV260X_GO, DRV260X_GO_BIT);
> > +	if (ret != 0)
> > +		goto write_failure;
> > +
> > +	do {
> > +		ret = regmap_read(haptics->regmap, DRV260X_GO, &cal_buf);
> > +		if (ret != 0)
> > +			goto write_failure;
> > +	} while (cal_buf == DRV260X_GO_BIT || ret != 0);
> > +
> > +	return ret;
> > +
> > +write_failure:
> > +	dev_err(&haptics->client->dev,
> > +		"Failed to write init registers: %d\n",
> > +		ret);
> > +skip_go_bit:
> > +	return ret;
> > +}
> > +
> > +static const struct regmap_config drv260x_regmap_config = {
> > +	.reg_bits = 8,
> > +	.val_bits = 8,
> > +
> > +	.max_register = DRV260X_MAX_REG,
> > +	.reg_defaults = drv260x_reg_defs,
> > +	.num_reg_defaults = ARRAY_SIZE(drv260x_reg_defs),
> > +	.cache_type = REGCACHE_NONE,
> > +};
> > +
> > +#ifdef CONFIG_OF
> > +static int drv260x_parse_dt(struct device_node *dev_node,
> > +				struct drv260x_data *haptics,
> > +				struct device *dev)
> > +{
> > +	int ret;
> > +	int voltage;
> > +
> > +	ret = of_property_read_u32(dev_node, "mode", &haptics->mode);
> > +	if (ret < 0) {
> > +		dev_err(dev, "%s: No entry for mode\n", __func__);
> > +
> > +		return ret;
> > +	}
> > +	ret = of_property_read_u32(dev_node, "library-sel",
> > +				&haptics->library);
> > +	if (ret < 0) {
> > +		dev_err(dev, "%s: No entry for library selection\n",
> > +			__func__);
> > +
> > +		return ret;
> > +	}
> > +	ret = of_property_read_u32(dev_node, "vib-rated-voltage",
> > +				&voltage);
> > +	if (!ret)
> > +		haptics->rated_voltage = drv260x_calculate_voltage(voltage);
> > +
> > +
> > +	ret = of_property_read_u32(dev_node, "vib-overdrive-voltage",
> > +				&voltage);
> > +	if (!ret)
> > +		haptics->overdrive_voltage = drv260x_calculate_voltage(voltage);
> > +
> > +	return ret;
> > +
> > +}
> > +#else
> > +static inline int drv260x_parse_dt(struct device *dev)
> > +{
> > +	dev_err(dev, "no platform data defined\n");
> > +
> > +	return -EINVAL;
> > +}
> > +#endif
> > +static int drv260x_probe(struct i2c_client *client,
> > +			   const struct i2c_device_id *id)
> > +{
> > +	struct drv260x_data *haptics;
> > +	struct device_node *np = client->dev.of_node;
> > +	struct drv260x_platform_data *pdata = dev_get_platdata(&client->dev);
> > +	int ret;
> > +
> > +	haptics = devm_kzalloc(&client->dev, sizeof(*haptics), GFP_KERNEL);
> > +	if (!haptics)
> > +		return -ENOMEM;
> > +
> > +	haptics->rated_voltage = DRV260X_DEF_OD_CLAMP_VOLT;
> > +	haptics->rated_voltage = DRV260X_DEF_RATED_VOLT;
> > +
> > +	 if (pdata) {
> > +		haptics->mode = pdata->mode;
> > +		haptics->library = pdata->library_selection;
> > +		if (pdata->vib_overdrive_voltage)
> > +			haptics->overdrive_voltage = drv260x_calculate_voltage(pdata->vib_overdrive_voltage);
> > +		if (pdata->vib_rated_voltage)
> > +			haptics->rated_voltage = drv260x_calculate_voltage(pdata->vib_rated_voltage);
> > +	} else if (np) {
> > +		ret = drv260x_parse_dt(np, haptics, &client->dev);
> > +		if (ret)
> > +			return ret;
> > +	} else {
> > +		dev_err(&client->dev, "Platform data not set\n");
> > +		return -ENODEV;
> > +	}
> > +
> > +
> > +	if (haptics->mode < DRV260X_LRA_MODE ||
> > +	    haptics->mode > DRV260X_ERM_MODE) {
> > +		dev_err(&client->dev,
> > +			"Vibrator mode is invalid: %i\n",
> > +			haptics->mode);
> > +		return -EINVAL;
> > +	}
> > +	
> > +	if (haptics->library < DRV260X_LIB_SEL_DEFAULT ||
> > +	    haptics->library > DRV260X_LIB_F) {
> > +		dev_err(&client->dev,
> > +			"Library value is invalid: %i\n", haptics->library);
> > +		return -EINVAL;
> > +	}	
> > +
> > +	haptics->regulator = devm_regulator_get(&client->dev, "vbat");
> > +	if (IS_ERR(haptics->regulator)) {
> > +		ret = PTR_ERR(haptics->regulator);
> > +		dev_err(&client->dev,
> > +			"unable to get regulator, error: %d\n", ret);
> > +		return ret;
> > +	}
> > +
> > +	haptics->enable_gpio = devm_gpiod_get(&client->dev, "enable");
> > +	if (IS_ERR(haptics->enable_gpio)) {
> > +		ret = PTR_ERR(haptics->enable_gpio);
> > +		if (ret != -ENOENT && ret != -ENOSYS)
> > +			return ret;
> > +		haptics->enable_gpio = NULL;
> > +	} else {
> > +		gpiod_direction_output(haptics->enable_gpio, 1);
> > +	}
> > +
> > +	haptics->input_dev = devm_input_allocate_device(&client->dev);
> > +	if (haptics->input_dev == NULL) {
> > +		dev_err(&client->dev, "Failed to allocate input device\n");
> > +		ret = -ENOMEM;
> > +		return ret;
> > +	}
> > +
> > +	haptics->input_dev->name = "drv260x:haptics";
> > +	haptics->input_dev->dev.parent = client->dev.parent;
> > +	haptics->input_dev->close = drv260x_close;
> > +	input_set_drvdata(haptics->input_dev, haptics);
> > +	input_set_capability(haptics->input_dev, EV_FF, FF_RUMBLE);
> > +
> > +	ret = input_ff_create_memless(haptics->input_dev, NULL,
> > +				      drv260x_haptics_play);
> > +	if (ret < 0) {
> > +		dev_err(&client->dev, "input_ff_create() failed: %d\n",
> > +			ret);
> > +		return ret;
> > +	}
> > +
> > +	INIT_WORK(&haptics->work, drv260x_worker);
> > +
> > +	haptics->client = client;
> > +	i2c_set_clientdata(client, haptics);
> > +
> > +	haptics->regmap = devm_regmap_init_i2c(client, &drv260x_regmap_config);
> > +	if (IS_ERR(haptics->regmap)) {
> > +		ret = PTR_ERR(haptics->regmap);
> > +		dev_err(&client->dev, "Failed to allocate register map: %d\n",
> > +			ret);
> > +		return ret;
> > +	}
> > +
> > +	ret = drv260x_init(haptics);
> > +	if (ret < 0) {
> > +		dev_err(&client->dev, "Device init failed: %d\n", ret);
> > +		return ret;
> > +	}
> > +
> > +	ret = input_register_device(haptics->input_dev);
> > +	if (ret < 0) {
> > +		dev_err(&client->dev, "couldn't register input device: %d\n",
> > +			ret);
> > +		return ret;
> > +	}
> > +	return 0;
> > +}
> > +
> > +#ifdef CONFIG_PM_SLEEP
> > +static int drv260x_suspend(struct device *dev)
> > +{
> > +	struct drv260x_data *haptics = dev_get_drvdata(dev);
> > +	int ret = 0;
> > +
> > +	mutex_lock(&haptics->input_dev->mutex);
> > +
> > +	if (haptics->input_dev->users) {
> > +		ret = regmap_update_bits(haptics->regmap,
> > +				   DRV260X_MODE,
> > +				   DRV260X_STANDBY_MASK,
> > +				   DRV260X_STANDBY);
> > +		if (ret) {
> > +			dev_err(dev, "Failed to set standby mode\n");
> > +			goto out;
> > +		}
> > +
> > +		gpiod_set_value(haptics->enable_gpio, 0);
> > +
> > +		ret = regulator_disable(haptics->regulator);
> > +		if (ret)
> > +			dev_err(dev, "Failed to disable regulator\n");
> > +	}
> > +out:
> > +	mutex_unlock(&haptics->input_dev->mutex);
> > +	return ret;
> > +}
> > +
> > +static int drv260x_resume(struct device *dev)
> > +{
> > +	struct drv260x_data *haptics = dev_get_drvdata(dev);
> > +	int ret = 0;
> > +
> > +	mutex_lock(&haptics->input_dev->mutex);
> > +
> > +	if (haptics->input_dev->users) {
> > +		ret = regulator_enable(haptics->regulator);
> > +		if (ret) {
> > +			dev_err(dev, "Failed to enable regulator\n");
> > +			goto out;
> > +		}
> > +		ret = regmap_update_bits(haptics->regmap,
> > +				   DRV260X_MODE,
> > +				   DRV260X_STANDBY_MASK, 0);
> > +		if (ret) {
> > +			dev_err(dev, "Failed to unset standby mode\n");
> > +			goto out;
> > +		}
> > +
> > +		gpiod_set_value(haptics->enable_gpio, 1);
> > +	}
> > +
> > +out:
> > +	mutex_unlock(&haptics->input_dev->mutex);
> > +	return ret;
> > +}
> > +#endif
> > +
> > +static SIMPLE_DEV_PM_OPS(drv260x_pm_ops, drv260x_suspend, drv260x_resume);
> > +
> > +static const struct i2c_device_id drv260x_id[] = {
> > +	{ "drv2605l", 0 },
> > +	{ }
> > +};
> > +MODULE_DEVICE_TABLE(i2c, drv260x_id);
> > +
> > +#if IS_ENABLED(CONFIG_OF)
> > +static const struct of_device_id drv260x_of_match[] = {
> > +	{ .compatible = "ti,drv2604", },
> > +	{ .compatible = "ti,drv2604l", },
> > +	{ .compatible = "ti,drv2605", },
> > +	{ .compatible = "ti,drv2605l", },
> > +	{}
> > +};
> > +MODULE_DEVICE_TABLE(of, drv260x_of_match);
> > +#endif
> > +
> > +static struct i2c_driver drv260x_driver = {
> > +	.probe		= drv260x_probe,
> > +	.driver		= {
> > +		.name	= "drv260x-haptics",
> > +		.owner	= THIS_MODULE,
> > +		.of_match_table = of_match_ptr(drv260x_of_match),
> > +		.pm	= &drv260x_pm_ops,
> > +	},
> > +	.id_table = drv260x_id,
> > +};
> > +module_i2c_driver(drv260x_driver);
> > +
> > +MODULE_ALIAS("platform:drv260x-haptics");
> > +MODULE_DESCRIPTION("TI DRV260x haptics driver");
> > +MODULE_LICENSE("GPL");
> > +MODULE_AUTHOR("Dan Murphy <dmurphy@ti.com>");
> > diff --git a/include/dt-bindings/input/ti-drv260x.h b/include/dt-bindings/input/ti-drv260x.h
> > new file mode 100644
> > index 0000000..be7f245
> > --- /dev/null
> > +++ b/include/dt-bindings/input/ti-drv260x.h
> > @@ -0,0 +1,35 @@
> > +/*
> > + * DRV260X haptics driver family
> > + *
> > + * Author: Dan Murphy <dmurphy@ti.com>
> > + *
> > + * Copyright:   (C) 2014 Texas Instruments, Inc.
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope that it will be useful, but
> > + * WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > + * General Public License for more details.
> > + */
> > +
> > +#ifndef _DT_BINDINGS_TI_DRV260X_H
> > +#define _DT_BINDINGS_TI_DRV260X_H
> > +
> > +/* Calibration Types */
> > +#define DRV260X_LRA_MODE		0x00
> > +#define DRV260X_LRA_NO_CAL_MODE	0x01
> > +#define DRV260X_ERM_MODE		0x02
> > +
> > +/* Library Selection */
> > +#define DRV260X_LIB_SEL_DEFAULT		0x00
> > +#define DRV260X_LIB_A				0x01
> > +#define DRV260X_LIB_B				0x02
> > +#define DRV260X_LIB_C				0x03
> > +#define DRV260X_LIB_D				0x04
> > +#define DRV260X_LIB_E				0x05
> > +#define DRV260X_LIB_F				0x06
> > +
> > +#endif
> > diff --git a/include/linux/platform_data/drv260x-pdata.h b/include/linux/platform_data/drv260x-pdata.h
> > new file mode 100644
> > index 0000000..4091a9b
> > --- /dev/null
> > +++ b/include/linux/platform_data/drv260x-pdata.h
> > @@ -0,0 +1,29 @@
> > +/*
> > + * Platform data for DRV260X haptics driver family
> > + *
> > + * Author: Dan Murphy <dmurphy@ti.com>
> > + *
> > + * Copyright:   (C) 2014 Texas Instruments, Inc.
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope that it will be useful, but
> > + * WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > + * General Public License for more details.
> > + */
> > +
> > +#ifndef _LINUX_DRV260X_PDATA_H
> > +#define _LINUX_DRV260X_PDATA_H
> > +
> > +struct drv260x_platform_data {
> > +	int enable_gpio;
> > +	u32 library_selection;
> > +	u32 mode;
> > +	u32 vib_rated_voltage;
> > +	u32 vib_overdrive_voltage;
> > +};
> > +
> > +#endif
> > 
> 
> 
> -- 
> ------------------
> Dan Murphy

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH] Input: cap1106 - allow changing key mapping from userspace
From: Dmitry Torokhov @ 2014-08-07  6:20 UTC (permalink / raw)
  To: Daniel Mack; +Cc: linux-input, linux-kernel
In-Reply-To: <20140721012045.GA4395@core.coreip.homeip.net>

On Sun, Jul 20, 2014 at 06:20:49PM -0700, Dmitry Torokhov wrote:
> Wire up support for EVIOC{G|S}KEYCODE to allow users change key mappings
> from userspace.
> 
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
> 
> Just compiled, not tested.

*ping* Any chance you could give it a try? Thanks!

> 
>  drivers/input/keyboard/cap1106.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/input/keyboard/cap1106.c b/drivers/input/keyboard/cap1106.c
> index f7d7a0d..180b184 100644
> --- a/drivers/input/keyboard/cap1106.c
> +++ b/drivers/input/keyboard/cap1106.c
> @@ -64,7 +64,7 @@ struct cap1106_priv {
>  	struct input_dev *idev;
>  
>  	/* config */
> -	unsigned int keycodes[CAP1106_NUM_CHN];
> +	unsigned short keycodes[CAP1106_NUM_CHN];
>  };
>  
>  static const struct reg_default cap1106_reg_defaults[] = {
> @@ -272,6 +272,12 @@ static int cap1106_i2c_probe(struct i2c_client *i2c_client,
>  	for (i = 0; i < CAP1106_NUM_CHN; i++)
>  		__set_bit(priv->keycodes[i], priv->idev->keybit);
>  
> +	__clear_bit(KEY_RESERVED, priv->idev->keybit);
> +
> +	priv->idev->keycode = priv->keycodes;
> +	priv->idev->keycodesize = sizeof(priv->keycodes[0]);
> +	priv->idev->keycodemax = ARRAY_SIZE(priv->keycodes);
> +
>  	priv->idev->id.vendor = CAP1106_MANUFACTURER_ID;
>  	priv->idev->id.product = CAP1106_PRODUCT_ID;
>  	priv->idev->id.version = rev;
> -- 
> 2.0.0.526.g5318336
> 
> 
> -- 
> Dmitry

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH v2 05/06] input synaptics-rmi4: Add firmware update support
From: Dmitry Torokhov @ 2014-08-07  6:42 UTC (permalink / raw)
  To: Christopher Heiny
  Cc: Linux Input, Andrew Duggan, Vincent Huang, Vivian Ly,
	Daniel Rosenberg, Linus Walleij, Benjamin Tissoires,
	David Herrmann, Jiri Kosina
In-Reply-To: <53E2BA1A.7000204@synaptics.com>

On Wed, Aug 06, 2014 at 04:28:26PM -0700, Christopher Heiny wrote:
> [I sent this last Thursday, but it never showed up on the input
> list. I'm assuming nobody else saw it.]
> 
> 
> On 07/31/2014 02:58 PM, Dmitry Torokhov wrote:
> >On Thu, Jul 31, 2014 at 02:43:47PM -0700, Christopher Heiny wrote:
> >>On 07/31/2014 02:19 PM, Dmitry Torokhov wrote:
> >>>On Thu, Jul 31, 2014 at 02:00:14PM -0700, Christopher Heiny wrote:
> >>>>>On 07/31/2014 10:53 AM, Dmitry Torokhov wrote:
> >>>>>>>Hi Christopher,
> >>>>>>>
> >>>>>>>On Wed, Mar 12, 2014 at 06:53:56PM -0700, Christopher Heiny wrote:
> >>>>>>>>>Add support for updating firmware on RMI4 devices with V5 bootloader.
> >>>>>>>
> >>>>>>>I am wondering why F34 is not following the staindard RMI function
> >>>>>>>implementation. By that I mean that it does not declare itself as struct
> >>>>>>>rmi_function_handler and does not rely on RMI core to bind itself to the device
> >>>>>>>if device supports it.
> >>>>>
> >>>>>Hi Dmitry,
> >>>>>
> >>>>>We originally had an F34 implementation that followed the RMI4
> >>>>>function standard and exposed most of the basic F34 operations via
> >>>>>sysfs. However, we got feedback (both on LKML and offline) (a)
> >>>>>recommending to use request_firmware, and (b) improve reflash times
> >>>>>while (c) reducing impact on boot time, and (d) "get rid of all that
> >>>>>sysfs crap" (paraphrased, but close to it).
> >>>>>
> >>>>>So after looking at how some other drivers use request_firmware, we
> >>>>>came up with the current approach.  Switching to request_firmware
> >>>>>definitely sped up the reflash times!  Including a check to see if
> >>>>>firmware update is required before setting up the RMI4
> >>>>>sensor/function structures also significantly reduced boot times.
> >>>
> >>>I am not suggesting you stop using request-firmware or introduce
> >>>bazillion of new sysfs attributes. I just wondered why you have manual
> >>>"binding" of F34 functionality instead of standrad RMI4 function
> >>>binding, liek you do for F01, F11 and so forth.
> >>
> >>Sorry!  My answer wasn't very clear on that part, was it?
> >>
> >>The manual binding gets the reflash (if required) done very early in
> >>the boot/probe process.  This eliminates the need to set up the
> >>whole sensor + functions structure, tear it down in order to
> >>reflash, and then build it all back up again.  It is felt that the
> >>time savings is significant, especially on highly featured products.
> >
> >I am sorry but I have hard time accepting this argument. How often do
> >you reflash devices during normal operation and how long does it take to
> >initialize the device compared to getting entire userspace up and
> >running to be able to actually supply or serve flash data (even without
> >using usermode helper to flash you need filesystem with the firmware to
> >be mounted)?
> 
> That was my argument exactly, but that was the direction we were
> pushed.  I'd much rather implement it as we discussed offline
> earlier this week.  If you were to say: "I'm sorry, but this simply
> can't be merged as it stands." you wouldn't get any argument from me
> on technical grounds. There might be people who will argue about the
> additional calendar time it would take to restructure it, though.

OK, then I will just say this: "I'm sorry, but this simply can't be merged as
it stands."

Now, I am talking about mainline here, I am fairly certain we can resolve
scheduling issues between what you currently have and what is needed in the
end.

Thanks.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH] Input: cap1106 - allow changing key mapping from userspace
From: Daniel Mack @ 2014-08-07  6:50 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: linux-input, linux-kernel
In-Reply-To: <20140807062033.GD651@core.coreip.homeip.net>

Hi Dmitry,

On 08/07/2014 08:20 AM, Dmitry Torokhov wrote:
> On Sun, Jul 20, 2014 at 06:20:49PM -0700, Dmitry Torokhov wrote:
>> Wire up support for EVIOC{G|S}KEYCODE to allow users change key mappings
>> from userspace.
>>
>> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>> ---
>>
>> Just compiled, not tested.
> 
> *ping* Any chance you could give it a try? Thanks!

Oh, sorry, I didn't realize you're waiting for me on this.

Unfortunately, I can't test it myself anymore, as I had to return the
board I found this chip on. But as the change looks very straight
forward, I'd suggest you just apply that patch and I ask the vendor to
give it a quick try. In case of any problems, I'll let you know. Does
that sound feasible?


Thanks,
Daniel


> 
>>
>>  drivers/input/keyboard/cap1106.c | 8 +++++++-
>>  1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/input/keyboard/cap1106.c b/drivers/input/keyboard/cap1106.c
>> index f7d7a0d..180b184 100644
>> --- a/drivers/input/keyboard/cap1106.c
>> +++ b/drivers/input/keyboard/cap1106.c
>> @@ -64,7 +64,7 @@ struct cap1106_priv {
>>  	struct input_dev *idev;
>>  
>>  	/* config */
>> -	unsigned int keycodes[CAP1106_NUM_CHN];
>> +	unsigned short keycodes[CAP1106_NUM_CHN];
>>  };
>>  
>>  static const struct reg_default cap1106_reg_defaults[] = {
>> @@ -272,6 +272,12 @@ static int cap1106_i2c_probe(struct i2c_client *i2c_client,
>>  	for (i = 0; i < CAP1106_NUM_CHN; i++)
>>  		__set_bit(priv->keycodes[i], priv->idev->keybit);
>>  
>> +	__clear_bit(KEY_RESERVED, priv->idev->keybit);
>> +
>> +	priv->idev->keycode = priv->keycodes;
>> +	priv->idev->keycodesize = sizeof(priv->keycodes[0]);
>> +	priv->idev->keycodemax = ARRAY_SIZE(priv->keycodes);
>> +
>>  	priv->idev->id.vendor = CAP1106_MANUFACTURER_ID;
>>  	priv->idev->id.product = CAP1106_PRODUCT_ID;
>>  	priv->idev->id.version = rev;
>> -- 
>> 2.0.0.526.g5318336
>>
>>
>> -- 
>> Dmitry
> 


^ permalink raw reply


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