* Re: [PATCH] HID: add new gamepad LED constants
From: Dmitry Torokhov @ 2014-09-16 0:54 UTC (permalink / raw)
To: Michael Wright
Cc: David Herrmann, open list:HID CORE LAYER, Michael Wright,
Jiri Kosina
In-Reply-To: <CAON4U=NqrMK=baJBLGWY=-+jwDdAE==LRDHU35Rya6GhdnrsQA@mail.gmail.com>
On Mon, Sep 15, 2014 at 04:41:42PM -0700, Michael Wright wrote:
> > LEDs should be controlled via LED subsystem. The fact that they happen
> > to be located on a given device does not mean they belong to input
> > subsystem. The same as LEds on network card are not part of network stack,
> > etc.
>
> I think the fundamental difference for me is that the primary input
> standard does
> have a specification for how to interact with LEDs,
... That predates LED subsystem. When we saw that it is not flexible enough LED
subsystem was created. Input, for better or worse, has "sound" support as well,
but I do not think anyone would want to to enhance it to support 7.1 surround
output ;)
> and right now the LED
> subsystem has no knowledge of HID whatsoever.
Then that is something that needs to be changed.
>
> Even if I did add the plumbing for HID over into the LED subsystem, is there an
> easy way to correlate input devices and their LEDs from userspace?
What does it mean "their led"? The led that happens to share the same physical
package?
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] HID: add new gamepad LED constants
From: Michael Wright @ 2014-09-16 1:02 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: David Herrmann, open list:HID CORE LAYER, Michael Wright,
Jiri Kosina
In-Reply-To: <20140916005403.GA21751@core.coreip.homeip.net>
On Mon, Sep 15, 2014 at 5:54 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> ... That predates LED subsystem. When we saw that it is not flexible enough LED
> subsystem was created. Input, for better or worse, has "sound" support as well,
> but I do not think anyone would want to to enhance it to support 7.1 surround
> output ;)
When I said standard I meant HID, not the input subsystem. I totally
understand wanting to create a better separation of responsibilities
internally.
> What does it mean "their led"? The led that happens to share the same physical
> package?
Sure, but in this case we really care that they're part of the same
physical package. We're lighting up the LED for the first player's
input device, so we *need* this ability to correlate.
LEDs also don't appear to have any uapi headers. Is the sysfs entry
considered stable API? Are we supposed to classify what they're for
(e.g. PLAYER_1 vs. PLAYER_2) by their name?
--
Michael Wright
Android Frameworks
^ permalink raw reply
* Re: [PATCH] HID: add new gamepad LED constants
From: simon @ 2014-09-16 2:00 UTC (permalink / raw)
To: Michael Wright
Cc: Dmitry Torokhov, David Herrmann, open list:HID CORE LAYER,
Michael Wright, Jiri Kosina
In-Reply-To: <CAON4U=NqrMK=baJBLGWY=-+jwDdAE==LRDHU35Rya6GhdnrsQA@mail.gmail.com>
> Even if I did add the plumbing for HID over into the LED subsystem, is
> there an
> easy way to correlate input devices and their LEDs from userspace?
You can follow the symlinks through the '/sys' filesystem. For example the
following all point to the same LED.
--
simon@womble:~$ ls
/sys/class/input/js0/device/device/leds/0003\:054C\:0268.0001\:\:sony1
brightness device max_brightness power subsystem trigger uevent
simon@womble:~$ ls /sys/class/leds/0003\:054C\:0268.0001\:\:sony1/
brightness device max_brightness power subsystem trigger uevent
simon@womble:~$ ls
/sys/bus/hid/devices/0003\:054C\:0268.0001/leds/0003\:054C\:0268.0001\:\:sony1/
brightness device max_brightness power subsystem trigger uevent
--
One problem that exists is that many HID devices don't have an (embedded)
serial number, so identification can be confused if you have more that one
identical device.
Simon.
^ permalink raw reply
* Re: [PATCH] HID: add new gamepad LED constants
From: Michael Wright @ 2014-09-16 4:06 UTC (permalink / raw)
To: simon
Cc: Dmitry Torokhov, David Herrmann, open list:HID CORE LAYER,
Michael Wright, Jiri Kosina
In-Reply-To: <8227f0fc35950a902241a8e053924ef3.squirrel@mungewell.org>
On Mon, Sep 15, 2014 at 7:00 PM, <simon@mungewell.org> wrote:
> One problem that exists is that many HID devices don't have an (embedded)
> serial number, so identification can be confused if you have more that one
> identical device.
That's a pretty serious problem since this is explicitly for multiple
gamepads connected to the same device, which will most likely be all
the same type.
--
Michael Wright
Android Frameworks
^ permalink raw reply
* Re: [PATCH v4 4/4] mfd: max77693: Update DT binding to support haptic
From: Jaewon Kim @ 2014-09-16 7:59 UTC (permalink / raw)
To: Lee Jones
Cc: Dmitry Torokhov, Samuel Ortiz, Chanwoo Choi, linux-kernel,
linux-input
In-Reply-To: <20140915230043.GI25162@lee--X1>
Dear Lee Jones,
2014년 09월 16일 08:00에 Lee Jones 이(가) 쓴 글:
> On Thu, 11 Sep 2014, Jaewon Kim wrote:
>
>> This patch add haptic DT binding documentation and example
>> to support haptic driver in max77693 Multifunction device.
>>
>> Signed-off-by: Jaewon Kim <jaewon02.kim@samsung.com>
>> Acked-by: Chanwoo Choi <cw00.choi@samsung.com>
>> ---
>> Documentation/devicetree/bindings/mfd/max77693.txt | 18 ++++++++++++++++++
>> 1 file changed, 18 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/mfd/max77693.txt b/Documentation/devicetree/bindings/mfd/max77693.txt
>> index 11921cc..d178b9e 100644
>> --- a/Documentation/devicetree/bindings/mfd/max77693.txt
>> +++ b/Documentation/devicetree/bindings/mfd/max77693.txt
>> @@ -27,6 +27,17 @@ Optional properties:
>>
>> [*] refer Documentation/devicetree/bindings/regulator/regulator.txt
>>
>> +- haptic : The haptic of max77693 have to be instantiated under subnod
> Perhaps a description as to what "The haptic" is might be handy?
Ok, I will add description about haptic.
>
> s/have/has
>
> s/subnod/subnode
>
> What do you mean by "under" a subnode? Do you mean "as a" subnode?
>
>> + named "haptic" using the following haptic format in example.
> What is the "haptic format"? It just looks like a normal node to me.
>
>> + Haptic sensation from motor can be changed by changing a period cycle in pwms.
> Is "Haptic sensation" a technical term?
>
> s/from motor/from a motor/
>
> s/changed by changing/changed by varying/
>
> s/period cycle/duty cycle/
>
> s/in pwms/in the pwms property/
How is it if I change in this way?
haptic : Referring to the term Haptic, It is the use of advanced
vibration patterns
and waveforms to convey information to a user or operator.
MAX77693 uses PWM and MOTOR in order to give Haptic.
The Haptic from a motor can be changed by varying a period/duty
cycle in pwms.
>
>> +
>> + Required properties:
>> + - compatible : Must be "maxim,max77693-hpatic"
>> + - haptic-supply : power supply for haptic motor
> s/for haptic/for the haptic/
>
>> + [*] refer Documentation/devicetree/bindings/regulator/regulator.txt
>> + - pwms : phandle to the physical PWM device,
> It's not just the phandle though is it?
Do you want more description?
Or it mean the wrong?
>
>> + [*] refer Documentation/devicetree/bindings/pwm/pwm.txt
>> +
>> Example:
>> max77693@66 {
>> compatible = "maxim,max77693";
>> @@ -52,4 +63,11 @@ Example:
>> regulator-boot-on;
>> };
>> };
>> +
>> + haptic {
>> + compatible = "maxim,max77693-haptic";
>> + haptic-supply = <&haptic_supply>;
>> + pwms = <&pwm 0 40000 0>;
>> + pwm-names = "haptic";
>> + };
>> };
Thank you for feedback.
And plz review "[PATCH v4 3/4] mfd: max77693: add haptic of_compatible
in mfd_cell".
thanks.
--
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] HID: add new gamepad LED constants
From: David Herrmann @ 2014-09-16 10:00 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Michael Wright, open list:HID CORE LAYER, Michael Wright,
Jiri Kosina
In-Reply-To: <20140915225809.GA31798@core.coreip.homeip.net>
Hi
On Tue, Sep 16, 2014 at 12:58 AM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>> The LED subsystem lacks any unprivileged API to control LEDs.
>
> As far as I can see it is the same as for input devices. You just need
> to make sure the device owner can access needed attributes, such as
> brightness.
>
>> There's no cdev to control write-access to the LED API. /sys has never
>> been intended as unprivileged API
>
> chmod/chown works just fine on sysfs attributes.
Sure it is. You can even use ACLs. But that doesn't mean it provides a
proper API environment. Udev maintainers have always said "sysfs is
only writable by root". Reasons mostly being the following:
* sysfs exists only once. Unlike char-devs, you cannot create multiple
independent entry-points to the device node. Instead, there's only a
single entry point which has to be shared between all users (across
sessions, across containers, across user-namespaces, across
pid-namespaces).
* sysfs does not provide per-file contexts. Unlike char-devs, sysfs
never knows the context of the user that writes into an attribute. It
cannot associate private data to the open file and it cannot track the
time the user releases the device again. It cannot multiplex across
multiple simultaneous users.
* sysfs is not "lightweight". People always say they don't want full
blown char-devs because sysfs is much lighter. Don't know where that
came from.. but looking at the amount of inodes and files needed for
sysfs APIs, it's definitely not faster nor smaller than char-devs.
Just look at IIO to see what happens if you write evdev-like APIs
based on sysfs attributes "because it's lightweight". It's a mess.
Sure, one solution would be to provide char-devs for LEDs. But then
one char-dev per LED sounds like overkill, so my proposal is to
include it in the input API like we always did. You're free to
disagree on that. I'm just saying that sysfs APIs to access LEDs on
GamePads (which this patch is mostly targeted at, I guess) is making
user-space life miserable. It's the same discussion we had with
backlights for years and we're trying eagerly to merge them into the
DRM char-devs.
Thanks
David
^ permalink raw reply
* [PATCH 15/22] Input: edt-ft5x06 - Replace strnicmp with strncasecmp
From: Rasmus Villemoes @ 2014-09-16 20:51 UTC (permalink / raw)
To: Andrew Morton
Cc: linux-kernel, Rasmus Villemoes, Dmitry Torokhov, linux-input
In-Reply-To: <1410900696-6481-1-git-send-email-linux@rasmusvillemoes.dk>
The kernel used to contain two functions for length-delimited,
case-insensitive string comparison, strnicmp with correct semantics
and a slightly buggy strncasecmp. The latter is the POSIX name, so
strnicmp was renamed to strncasecmp, and strnicmp made into a wrapper
for the new strncasecmp to avoid breaking existing users.
To allow the compat wrapper strnicmp to be removed at some point in
the future, and to avoid the extra indirection cost, do
s/strnicmp/strncasecmp/g.
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: linux-input@vger.kernel.org
Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
drivers/input/touchscreen/edt-ft5x06.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
index 8857d5b..ee3434f 100644
--- a/drivers/input/touchscreen/edt-ft5x06.c
+++ b/drivers/input/touchscreen/edt-ft5x06.c
@@ -812,7 +812,7 @@ static int edt_ft5x06_ts_identify(struct i2c_client *client,
/* if we find something consistent, stay with that assumption
* at least M09 won't send 3 bytes here
*/
- if (!(strnicmp(rdbuf + 1, "EP0", 3))) {
+ if (!(strncasecmp(rdbuf + 1, "EP0", 3))) {
tsdata->version = M06;
/* remove last '$' end marker */
--
2.0.4
^ permalink raw reply related
* [PATCH] HID: wacom: fix timeout on probe for some wacoms
From: Benjamin Tissoires @ 2014-09-16 20:56 UTC (permalink / raw)
To: Jiri Kosina, Ping Cheng, killertofu; +Cc: linux-input, linux-kernel
Some Wacom tablets (at least the ISDv4 found in the Lenovo X230) timeout
during probe while retrieving the input reports.
The only time this information is valuable is during the feature_mapping
stage, so we can ask for it there and discard the generic input reports
retrieval.
This gives a code path closer to the wacom.ko driver when it was in the
input subtree (not HID).
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
Hi Jiri,
I run into this today while trying to improve the generic support of the ISDv4
sensors (those found on laptops not sold by Wacom).
This is a regression from 3.16, in a sense that it can add up to 10 seconds to
the boot of the laptop. So IMO, this should go as a fix for 3.17-rc6.
Unfortunately, c64d883476 changed the ABI and wacom_get_report() takes now one
less argument. I based my patch on top of the for-3.18/wacom branch, so in 3.17,
it will not compile.
I would personally be in favor of having c64d883476 and this one in 3.17, but
c64d883476 will also require some manual conflict resolution (cherry-pick is
complaining).
I can also resend it based on the v3.17-rc5 tag, but then we will have to update
the for-3.18/wacom when you will merge with Linus' tree.
Let me know me what you prefer.
Hi Jason,
I managed to make the wacom driver capable of dealing with the sensor on the
Lenovo X230 without having to add any quirks in the kernel (both pen and touch).
:)
Patches will come soon.
Cheers,
Benjamin
drivers/hid/wacom_sys.c | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index a8b7f16..2508628 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -110,12 +110,24 @@ static void wacom_feature_mapping(struct hid_device *hdev,
{
struct wacom *wacom = hid_get_drvdata(hdev);
struct wacom_features *features = &wacom->wacom_wac.features;
+ u8 *data;
+ int ret;
switch (usage->hid) {
case HID_DG_CONTACTMAX:
/* leave touch_max as is if predefined */
- if (!features->touch_max)
- features->touch_max = field->value[0];
+ if (!features->touch_max) {
+ /* read manually */
+ data = kzalloc(2, GFP_KERNEL);
+ if (!data)
+ break;
+ data[0] = field->report->id;
+ ret = wacom_get_report(hdev, HID_FEATURE_REPORT,
+ data, 2, 0);
+ if (ret == 2)
+ features->touch_max = data[1];
+ kfree(data);
+ }
break;
}
}
@@ -1280,6 +1292,8 @@ static int wacom_probe(struct hid_device *hdev,
if (!id->driver_data)
return -EINVAL;
+ hdev->quirks |= HID_QUIRK_NO_INIT_REPORTS;
+
wacom = kzalloc(sizeof(struct wacom), GFP_KERNEL);
if (!wacom)
return -ENOMEM;
--
2.1.0
^ permalink raw reply related
* Re: [PATCH v4 4/4] mfd: max77693: Update DT binding to support haptic
From: Lee Jones @ 2014-09-16 22:40 UTC (permalink / raw)
To: Jaewon Kim
Cc: Dmitry Torokhov, Samuel Ortiz, Chanwoo Choi, linux-kernel,
linux-input
In-Reply-To: <5417EDFE.7030700@samsung.com>
On Tue, 16 Sep 2014, Jaewon Kim wrote:
> 2014년 09월 16일 08:00에 Lee Jones 이(가) 쓴 글:
> >On Thu, 11 Sep 2014, Jaewon Kim wrote:
> >
> >>This patch add haptic DT binding documentation and example
> >>to support haptic driver in max77693 Multifunction device.
> >>
> >>Signed-off-by: Jaewon Kim <jaewon02.kim@samsung.com>
> >>Acked-by: Chanwoo Choi <cw00.choi@samsung.com>
> >>---
> >> Documentation/devicetree/bindings/mfd/max77693.txt | 18 ++++++++++++++++++
> >> 1 file changed, 18 insertions(+)
> >>
> >>diff --git a/Documentation/devicetree/bindings/mfd/max77693.txt b/Documentation/devicetree/bindings/mfd/max77693.txt
> >>index 11921cc..d178b9e 100644
> >>--- a/Documentation/devicetree/bindings/mfd/max77693.txt
> >>+++ b/Documentation/devicetree/bindings/mfd/max77693.txt
> >>@@ -27,6 +27,17 @@ Optional properties:
> >> [*] refer Documentation/devicetree/bindings/regulator/regulator.txt
> >>+- haptic : The haptic of max77693 have to be instantiated under subnod
> >Perhaps a description as to what "The haptic" is might be handy?
> Ok, I will add description about haptic.
>
> >
> >s/have/has
> >
> >s/subnod/subnode
> >
> >What do you mean by "under" a subnode? Do you mean "as a" subnode?
I see what you did here now. You copied this from the discription
above. That will also need to be changed.
> >>+ named "haptic" using the following haptic format in example.
> >What is the "haptic format"? It just looks like a normal node to me.
> >
> >>+ Haptic sensation from motor can be changed by changing a period cycle in pwms.
> >Is "Haptic sensation" a technical term?
> >
> >s/from motor/from a motor/
> >
> >s/changed by changing/changed by varying/
> >
> >s/period cycle/duty cycle/
> >
> >s/in pwms/in the pwms property/
> How is it if I change in this way?
>
> haptic : Referring to the term Haptic, It is the use of advanced
> vibration patterns
> and waveforms to convey information to a user or operator.
>
> MAX77693 uses PWM and MOTOR in order to give Haptic.
> The Haptic from a motor can be changed by varying a period/duty
> cycle in pwms.
Hmm... How about:
The MAX77693 haptic device utilises a PWM controlled motor to provide
users with tactile feedback. PWM period and duty-cycle are varied in
order to provide the approprite level of feedback.
> >>+
> >>+ Required properties:
> >>+ - compatible : Must be "maxim,max77693-hpatic"
> >>+ - haptic-supply : power supply for haptic motor
> >s/for haptic/for the haptic/
> >
> >>+ [*] refer Documentation/devicetree/bindings/regulator/regulator.txt
> >>+ - pwms : phandle to the physical PWM device,
> >It's not just the phandle though is it?
> Do you want more description?
Yes please.
> Or it mean the wrong?
>
> >
> >>+ [*] refer Documentation/devicetree/bindings/pwm/pwm.txt
> >>+
> >> Example:
> >> max77693@66 {
> >> compatible = "maxim,max77693";
> >>@@ -52,4 +63,11 @@ Example:
> >> regulator-boot-on;
> >> };
> >> };
> >>+
> >>+ haptic {
> >>+ compatible = "maxim,max77693-haptic";
> >>+ haptic-supply = <&haptic_supply>;
> >>+ pwms = <&pwm 0 40000 0>;
> >>+ pwm-names = "haptic";
> >>+ };
> >> };
>
> Thank you for feedback.
> And plz review "[PATCH v4 3/4] mfd: max77693: add haptic
> of_compatible in mfd_cell".
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v4 3/4] mfd: max77693: add haptic of_compatible in mfd_cell
From: Lee Jones @ 2014-09-16 22:41 UTC (permalink / raw)
To: Jaewon Kim
Cc: Dmitry Torokhov, Samuel Ortiz, Chanwoo Choi, linux-kernel,
linux-input
In-Reply-To: <1410440062-19963-4-git-send-email-jaewon02.kim@samsung.com>
On Thu, 11 Sep 2014, Jaewon Kim wrote:
> This patch add haptic of_compatible in order to use the haptic
> device driver using Devicetree.
>
> Signed-off-by: Jaewon Kim <jaewon02.kim@samsung.com>
> Acked-by: Chanwoo Choi <cw00.choi@samsung.com>
> ---
> drivers/mfd/max77693.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
Acked-by: Lee Jones <lee.jones@linaro.org>
> diff --git a/drivers/mfd/max77693.c b/drivers/mfd/max77693.c
> index fbfed56..5b516fe 100644
> --- a/drivers/mfd/max77693.c
> +++ b/drivers/mfd/max77693.c
> @@ -46,7 +46,10 @@ static const struct mfd_cell max77693_devs[] = {
> { .name = "max77693-charger", },
> { .name = "max77693-flash", },
> { .name = "max77693-muic", },
> - { .name = "max77693-haptic", },
> + {
> + .name = "max77693-haptic",
> + .of_compatible = "maxim,max77693-haptic",
> + },
> };
>
> static const struct regmap_config max77693_regmap_config = {
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
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] HID: add new gamepad LED constants
From: Michael Wright @ 2014-09-17 1:56 UTC (permalink / raw)
To: Pierre-Loup A. Griffais
Cc: simon, Dmitry Torokhov, David Herrmann, HID CORE LAYER,
Michael Wright, Jiri Kosina
In-Reply-To: <5418E762.5040802@valvesoftware.com>
Yes, pretty much exactly that.
^ permalink raw reply
* Re: [PATCH] HID: add new gamepad LED constants
From: Pierre-Loup A. Griffais @ 2014-09-17 1:44 UTC (permalink / raw)
To: Michael Wright, simon
Cc: Dmitry Torokhov, David Herrmann, HID CORE LAYER, Michael Wright,
Jiri Kosina
In-Reply-To: <CAON4U=PT9k9j15QyuA-qjDtPQcXMz4Y9_Z0wFV4ydMqoOk8CAg@mail.gmail.com>
My issue with the LED subsystem is that to my knowledge, the only thing
that it currently exposes is an opaque brightness value that has plenty
of different meanings depending on what is actually being exposed. It
would need to be overhauled to support the usecase at hand, which would
be a lot more work than just applying this patch, on top of all the
other problems already mentioned with finding and using the sysfs LED
interface.
With this change all the gamepad drivers could easily plumb through
their device-specific way of exposing player IDs, and a userspace
component could easily ensure consistency between logical player IDs and
their LEDs.
If such a thing was attempted currently with gamepads that are wired up
to LED classes, the userspace component would need to have special
knowledge of each driver to be able to know which special brightness
value to set to get player N to light up on the controller. This isn't
solvable without significantly expanding the interface to the LED subsystem.
Michael, I assume such a userspace component is what you're shooting for
with this work?
In SteamOS the individual gamepad drivers are modified to automatically
apply player IDs from the joydev device IDs, which in turn are the
logical game controller IDs that are exposed by libraries such as SDL to
games and as such are reflected through their UI.
This was deemed too ad-hoc to be upstreamable and your patch seems like
the right first step towards a cleaner, albeit slightly more complex
solution. Trying to leverage the LED system seems a lot more complex,
without adding any value to this solution.
Thanks,
- Pierre-Loup
On 09/15/2014 09:06 PM, Michael Wright wrote:
> On Mon, Sep 15, 2014 at 7:00 PM, <simon@mungewell.org> wrote:
>> One problem that exists is that many HID devices don't have an (embedded)
>> serial number, so identification can be confused if you have more that one
>> identical device.
>
>
> That's a pretty serious problem since this is explicitly for multiple
> gamepads connected to the same device, which will most likely be all
> the same type.
>
^ permalink raw reply
* Re: [PATCH 15/22] Input: edt-ft5x06 - Replace strnicmp with strncasecmp
From: Dmitry Torokhov @ 2014-09-17 16:26 UTC (permalink / raw)
To: Rasmus Villemoes; +Cc: Andrew Morton, linux-kernel, linux-input
In-Reply-To: <1410900696-6481-16-git-send-email-linux@rasmusvillemoes.dk>
On Tue, Sep 16, 2014 at 10:51:29PM +0200, Rasmus Villemoes wrote:
> The kernel used to contain two functions for length-delimited,
> case-insensitive string comparison, strnicmp with correct semantics
> and a slightly buggy strncasecmp. The latter is the POSIX name, so
> strnicmp was renamed to strncasecmp, and strnicmp made into a wrapper
> for the new strncasecmp to avoid breaking existing users.
>
> To allow the compat wrapper strnicmp to be removed at some point in
> the future, and to avoid the extra indirection cost, do
> s/strnicmp/strncasecmp/g.
>
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: linux-input@vger.kernel.org
> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Applied, thank you.
--
Dmitry
^ permalink raw reply
* [PATCH] hid: sony: Set the DualShock 4 touchpad bits in the input_configured callback
From: Frank Praznik @ 2014-09-17 17:23 UTC (permalink / raw)
To: linux-input; +Cc: jkosina, Frank Praznik
Set the DualShock4 touchpad bits in the input_configured callback
so that they are registered properly for any input devices created
during hid_hw_start.
Signed-off-by: Frank Praznik <frank.praznik@oh.rr.com>
---
This fixes the issue with touch axes either not being present or being
incorrectly mapped to axis 0 in the joydev device.
drivers/hid/hid-sony.c | 73 ++++++++++++++++++++++++++++----------------------
1 file changed, 41 insertions(+), 32 deletions(-)
diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index 96a8ec5..41beb48 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -1084,6 +1084,38 @@ static int sony_mapping(struct hid_device *hdev, struct hid_input *hi,
return 0;
}
+static int sony_register_touchpad(struct hid_input *hi, int touch_count,
+ int w, int h)
+{
+ struct input_dev *input_dev = hi->input;
+ int ret;
+
+ ret = input_mt_init_slots(input_dev, touch_count, 0);
+ if (ret < 0)
+ return ret;
+
+ input_set_abs_params(input_dev, ABS_MT_POSITION_X, 0, w, 0, 0);
+ input_set_abs_params(input_dev, ABS_MT_POSITION_Y, 0, h, 0, 0);
+
+ return 0;
+}
+
+static void sony_input_configured(struct hid_device *hdev,
+ struct hid_input *hidinput)
+{
+ struct sony_sc *sc = hid_get_drvdata(hdev);
+
+ /*
+ * The Dualshock 4 touchpad supports 2 touches and has a
+ * resolution of 1920x940.
+ */
+ if (sc->quirks & DUALSHOCK4_CONTROLLER) {
+ if (sony_register_touchpad(hidinput, 2, 1920, 940) != 0)
+ hid_err(sc->hdev,
+ "Unable to initialize multi-touch slots\n");
+ }
+}
+
/*
* Sending HID_REQ_GET_REPORT changes the operation mode of the ps3 controller
* to "operational". Without this, the ps3 controller will not report any
@@ -1656,26 +1688,6 @@ static void sony_battery_remove(struct sony_sc *sc)
sc->battery.name = NULL;
}
-static int sony_register_touchpad(struct sony_sc *sc, int touch_count,
- int w, int h)
-{
- struct hid_input *hidinput = list_entry(sc->hdev->inputs.next,
- struct hid_input, list);
- struct input_dev *input_dev = hidinput->input;
- int ret;
-
- ret = input_mt_init_slots(input_dev, touch_count, 0);
- if (ret < 0) {
- hid_err(sc->hdev, "Unable to initialize multi-touch slots\n");
- return ret;
- }
-
- input_set_abs_params(input_dev, ABS_MT_POSITION_X, 0, w, 0, 0);
- input_set_abs_params(input_dev, ABS_MT_POSITION_Y, 0, h, 0, 0);
-
- return 0;
-}
-
/*
* If a controller is plugged in via USB while already connected via Bluetooth
* it will show up as two devices. A global list of connected controllers and
@@ -1925,11 +1937,7 @@ static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
goto err_stop;
}
}
- /*
- * The Dualshock 4 touchpad supports 2 touches and has a
- * resolution of 1920x940.
- */
- ret = sony_register_touchpad(sc, 2, 1920, 940);
+
if (ret < 0)
goto err_stop;
@@ -2039,13 +2047,14 @@ static const struct hid_device_id sony_devices[] = {
MODULE_DEVICE_TABLE(hid, sony_devices);
static struct hid_driver sony_driver = {
- .name = "sony",
- .id_table = sony_devices,
- .input_mapping = sony_mapping,
- .probe = sony_probe,
- .remove = sony_remove,
- .report_fixup = sony_report_fixup,
- .raw_event = sony_raw_event
+ .name = "sony",
+ .id_table = sony_devices,
+ .input_mapping = sony_mapping,
+ .input_configured = sony_input_configured,
+ .probe = sony_probe,
+ .remove = sony_remove,
+ .report_fixup = sony_report_fixup,
+ .raw_event = sony_raw_event
};
static int __init sony_init(void)
--
2.1.0
^ permalink raw reply related
* Re: [PATCH v2] Input: joystick - Use ktime for measuring timing
From: Andreas Mohr @ 2014-09-17 19:23 UTC (permalink / raw)
To: Takashi Iwai
Cc: Dmitry Torokhov, Clemens Ladisch, Andreas Mohr, Vojtech Pavlik,
Jiri Kosina, linux-input, linux-kernel
In-Reply-To: <s5h8ulrzcwy.wl-tiwai@suse.de>
Hi,
On Wed, Sep 10, 2014 at 05:57:17PM +0200, Takashi Iwai wrote:
> The current codes in gameport and analog joystick drivers for the time
> accounting have a long-standing problem when the system is running
> with CPU freq; since the timing is measured via TSC or sample counter,
> the calculation isn't reliable.
Thank you very much for having followed through on this!
(somehow you never seem to disappoint me :)
While working on testing this and doing various gameport/soundcard modifications,
I'm afraid I have seen the following checkpatch.pl (of v3.16) warnings:
WARNING: Missing a blank line after declarations
#164: FILE: drivers/input/joystick/analog.c:192:
+ unsigned int x;
+ GET_TIME(x);
WARNING: line over 80 characters
#224: FILE: drivers/input/joystick/analog.c:301:
+ port->axes[j] = (delta(start, time[i]) << ANALOG_FUZZ_BITS) / port->loop;
BTW the commit as-is will not be compatible with v3.16
since there's no ktime_get_ns() there yet
and (to add insult to injury) even the #include header files have changed, too.
That's a bit of a pity since I just had intended to say
that it's a very good idea
to release a quick(&dirty) initial timing hotfix for gameport handling
*prior* to possibly doing any subsequent less-related gameport cleanup commits,
since the quick initial timing hotfix would be very prominent -stable material,
except it... ain't so (--> life sucks :-).
So, for -stable reasons it might be very worthwhile
to add some compat code to the analog.c patch -
I am currently using the following compat fix (on v3.16):
--- a/drivers/input/joystick/analog.c
+++ b/drivers/input/joystick/analog.c
@@ -36,7 +36,14 @@
#include <linux/gameport.h>
#include <linux/jiffies.h>
#include <linux/timex.h>
-#include <linux/timekeeping.h>
+//#include <linux/timekeeping.h>
+#include <linux/hrtimer.h>
+
+static inline u64 ktime_get_ns(void)
+{
+ return ktime_to_ns(ktime_get());
+}
+
(which obviously isn't fit for purpose yet
since you'd need a versioned #include conditional,
and the name of this function when unconditionally added
is in direct conflict with the same-name > v3.16 one)
So, should that commit be improved
to have a simple versioning check
in order to be fully -stable deployable,
or would -stable be handled differently anyway?
(I'll now have these updates and checkpatch.pl stuff
maintained in a FIXUP commit locally)
So much for a quick status update
(I'll be rebooting into 3.16 now - my last kernel build was as low as 3.11.x even, WOW).
Thanks,
Andreas Mohr
^ permalink raw reply
* Re: [PATCH v2] Input: joystick - Use ktime for measuring timing
From: Takashi Iwai @ 2014-09-18 7:15 UTC (permalink / raw)
To: Andreas Mohr
Cc: Dmitry Torokhov, Clemens Ladisch, Vojtech Pavlik, Jiri Kosina,
linux-input, linux-kernel
In-Reply-To: <20140917192337.GA27319@rhlx01.hs-esslingen.de>
At Wed, 17 Sep 2014 21:23:37 +0200,
Andreas Mohr wrote:
>
> Hi,
>
> On Wed, Sep 10, 2014 at 05:57:17PM +0200, Takashi Iwai wrote:
> > The current codes in gameport and analog joystick drivers for the time
> > accounting have a long-standing problem when the system is running
> > with CPU freq; since the timing is measured via TSC or sample counter,
> > the calculation isn't reliable.
>
> Thank you very much for having followed through on this!
> (somehow you never seem to disappoint me :)
>
> While working on testing this and doing various gameport/soundcard modifications,
> I'm afraid I have seen the following checkpatch.pl (of v3.16) warnings:
>
> WARNING: Missing a blank line after declarations
> #164: FILE: drivers/input/joystick/analog.c:192:
> + unsigned int x;
> + GET_TIME(x);
>
> WARNING: line over 80 characters
> #224: FILE: drivers/input/joystick/analog.c:301:
> + port->axes[j] = (delta(start, time[i]) << ANALOG_FUZZ_BITS) / port->loop;
Both are cosmetic and I won't be bothered if it were sent to my tree,
for example ;) It depends on the maintainer, so if Dmitry wants these
to be fixed, I'm willing to resubmit, of course.
> BTW the commit as-is will not be compatible with v3.16
> since there's no ktime_get_ns() there yet
> and (to add insult to injury) even the #include header files have changed, too.
>
> That's a bit of a pity since I just had intended to say
> that it's a very good idea
> to release a quick(&dirty) initial timing hotfix for gameport handling
> *prior* to possibly doing any subsequent less-related gameport cleanup commits,
> since the quick initial timing hotfix would be very prominent -stable material,
> except it... ain't so (--> life sucks :-).
Hey, why not using 3.17 kernel?
If the fix is *seriously* demanded, one can backport easily with the
equivalent function and the header change, and send to stable kernel,
too. The important point is to track the upstream commit ID (once if
it's really merged).
Takashi
> So, for -stable reasons it might be very worthwhile
> to add some compat code to the analog.c patch -
> I am currently using the following compat fix (on v3.16):
>
> --- a/drivers/input/joystick/analog.c
> +++ b/drivers/input/joystick/analog.c
> @@ -36,7 +36,14 @@
> #include <linux/gameport.h>
> #include <linux/jiffies.h>
> #include <linux/timex.h>
> -#include <linux/timekeeping.h>
> +//#include <linux/timekeeping.h>
> +#include <linux/hrtimer.h>
> +
> +static inline u64 ktime_get_ns(void)
> +{
> + return ktime_to_ns(ktime_get());
> +}
> +
>
>
> (which obviously isn't fit for purpose yet
> since you'd need a versioned #include conditional,
> and the name of this function when unconditionally added
> is in direct conflict with the same-name > v3.16 one)
>
>
> So, should that commit be improved
> to have a simple versioning check
> in order to be fully -stable deployable,
> or would -stable be handled differently anyway?
> (I'll now have these updates and checkpatch.pl stuff
> maintained in a FIXUP commit locally)
>
> So much for a quick status update
> (I'll be rebooting into 3.16 now - my last kernel build was as low as 3.11.x even, WOW).
>
> Thanks,
>
> Andreas Mohr
>
^ permalink raw reply
* Re: [PATCH] mfd: ti_am335x_tscadc: Update logic in CTRL register for 5-wire TS
From: Sebastian Andrzej Siewior @ 2014-09-18 13:38 UTC (permalink / raw)
To: Samuel Ortiz, Lee Jones
Cc: linux-kernel, linux-input, Felipe Balbi, linux-omap, Jeff Lance,
Sebastian Andrzej Siewior
In-Reply-To: <1409850117-5156-1-git-send-email-bigeasy@linutronix.de>
On 2014-09-04 19:01:57 [+0200], Sebastian Andrzej Siewior wrote:
> From: Jeff Lance <j-lance1@ti.com>
>
> The logic in AFE_Pen_Ctrl bitmask in the CTRL register is different for five
> wire versus four or eight wire touschscreens. This patch should fix this for
> five-wire touch screens. There should be no change needed here for four and
> eight wire tousch screens.
>
> Signed-off-by: Jeff Lance <j-lance1@ti.com>
> [bigeasy: keep the change mfd only]
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
ping
> I've been looking at moving this piece and the idle mode setting into the
> TSC driver. Kinda odd that I have to patch within the MFD driver for this
> TSC related change.
> Anyone disagrees?
>
> drivers/mfd/ti_am335x_tscadc.c | 30 +++++++++++++++++-------------
> include/linux/mfd/ti_am335x_tscadc.h | 1 +
> 2 files changed, 18 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c
> index dd4bf5816221..6c9cfb8c1d01 100644
> --- a/drivers/mfd/ti_am335x_tscadc.c
> +++ b/drivers/mfd/ti_am335x_tscadc.c
> @@ -241,18 +241,20 @@ static int ti_tscadc_probe(struct platform_device *pdev)
> tscadc_writel(tscadc, REG_CLKDIV, tscadc->clk_div);
>
> /* Set the control register bits */
> - ctrl = CNTRLREG_STEPCONFIGWRT |
> - CNTRLREG_STEPID;
> - if (tsc_wires > 0)
> - ctrl |= CNTRLREG_4WIRE | CNTRLREG_TSCENB;
> + ctrl = CNTRLREG_STEPCONFIGWRT | CNTRLREG_STEPID;
> tscadc_writel(tscadc, REG_CTRL, ctrl);
>
> /* Set register bits for Idle Config Mode */
> - if (tsc_wires > 0)
> + if (tsc_wires > 0) {
> + tscadc->tsc_wires = tsc_wires;
> + if (tsc_wires == 5)
> + ctrl |= CNTRLREG_5WIRE | CNTRLREG_TSCENB;
> + else
> + ctrl |= CNTRLREG_4WIRE | CNTRLREG_TSCENB;
> tscadc_idle_config(tscadc);
> + }
>
> /* Enable the TSC module enable bit */
> - ctrl = tscadc_readl(tscadc, REG_CTRL);
> ctrl |= CNTRLREG_TSCSSENB;
> tscadc_writel(tscadc, REG_CTRL, ctrl);
>
> @@ -324,21 +326,23 @@ static int tscadc_suspend(struct device *dev)
> static int tscadc_resume(struct device *dev)
> {
> struct ti_tscadc_dev *tscadc_dev = dev_get_drvdata(dev);
> - unsigned int restore, ctrl;
> + u32 ctrl;
>
> pm_runtime_get_sync(dev);
>
> /* context restore */
> ctrl = CNTRLREG_STEPCONFIGWRT | CNTRLREG_STEPID;
> - if (tscadc_dev->tsc_cell != -1)
> - ctrl |= CNTRLREG_TSCENB | CNTRLREG_4WIRE;
> tscadc_writel(tscadc_dev, REG_CTRL, ctrl);
>
> - if (tscadc_dev->tsc_cell != -1)
> + if (tscadc_dev->tsc_cell != -1) {
> + if (tscadc_dev->tsc_wires == 5)
> + ctrl |= CNTRLREG_5WIRE | CNTRLREG_TSCENB;
> + else
> + ctrl |= CNTRLREG_4WIRE | CNTRLREG_TSCENB;
> tscadc_idle_config(tscadc_dev);
> - restore = tscadc_readl(tscadc_dev, REG_CTRL);
> - tscadc_writel(tscadc_dev, REG_CTRL,
> - (restore | CNTRLREG_TSCSSENB));
> + }
> + ctrl |= CNTRLREG_TSCSSENB;
> + tscadc_writel(tscadc_dev, REG_CTRL, ctrl);
>
> tscadc_writel(tscadc_dev, REG_CLKDIV, tscadc_dev->clk_div);
>
> diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
> index fb96c84dada5..e2e70053470e 100644
> --- a/include/linux/mfd/ti_am335x_tscadc.h
> +++ b/include/linux/mfd/ti_am335x_tscadc.h
> @@ -155,6 +155,7 @@ struct ti_tscadc_dev {
> void __iomem *tscadc_base;
> int irq;
> int used_cells; /* 1-2 */
> + int tsc_wires;
> int tsc_cell; /* -1 if not used */
> int adc_cell; /* -1 if not used */
> struct mfd_cell cells[TSCADC_CELLS];
> --
> 2.1.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply
* (unknown),
From: Maria Caballero @ 2014-09-18 14:15 UTC (permalink / raw)
Loan Offer contact us for more details (gibonline11@gmail.com<mailto:gibonline11@gmail.com>)
All Details should be forward to this E-mail address for fast respond: gibonline11@gmail.com<mailto:gibonline11@gmail.com>
^ permalink raw reply
* [PATCH v4 1/5] mfd: cros_ec: Delay for 50ms when we see EC_CMD_REBOOT_EC
From: Javier Martinez Canillas @ 2014-09-18 15:18 UTC (permalink / raw)
To: Lee Jones
Cc: Wolfram Sang, Dmitry Torokhov, Doug Anderson, Simon Glass,
Bill Richardson, Andrew Bresticker, Derek Basehore,
Gwendal Grignou, linux-i2c-u79uwXL29TY76Z2rM5mHXA,
linux-input-u79uwXL29TY76Z2rM5mHXA,
linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA,
Javier Martinez Canillas
In-Reply-To: <1411053538-17237-1-git-send-email-javier.martinez-ZGY8ohtN/8pPYcu2f3hruQ@public.gmane.org>
From: Doug Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
If someone sends a EC_CMD_REBOOT_EC to the EC, the EC will likely be
unresponsive for quite a while. Add a delay to the end of the command
to prevent random failures of future commands.
NOTES:
* This could be optimized a bit by simply delaying the next command
sent, but EC_CMD_REBOOT_EC is such a rare command that the extra
complexity doesn't seem worth it.
* This is a bit of an "ugly hack" since the SPI driver is effectively
snooping on the communication and making a lot of assumptions. It
would be nice to architect in some better solution long term.
* This same logic probably needs to be applied to the i2c driver.
Signed-off-by: Doug Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Reviewed-by: Randall Spangler <rspangler-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Reviewed-by: Vadim Bendebury <vbendeb-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Signed-off-by: Javier Martinez Canillas <javier.martinez-ZGY8ohtN/8pPYcu2f3hruQ@public.gmane.org>
Acked-by: Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
drivers/mfd/cros_ec_spi.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/mfd/cros_ec_spi.c b/drivers/mfd/cros_ec_spi.c
index 588c700..b396705 100644
--- a/drivers/mfd/cros_ec_spi.c
+++ b/drivers/mfd/cros_ec_spi.c
@@ -65,6 +65,12 @@
*/
#define EC_SPI_RECOVERY_TIME_NS (200 * 1000)
+/*
+ * The EC is unresponsive for a time after a reboot command. Add a
+ * simple delay to make sure that the bus stays locked.
+ */
+#define EC_REBOOT_DELAY_MS 50
+
/**
* struct cros_ec_spi - information about a SPI-connected EC
*
@@ -318,6 +324,9 @@ static int cros_ec_cmd_xfer_spi(struct cros_ec_device *ec_dev,
ret = len;
exit:
+ if (ec_msg->command == EC_CMD_REBOOT_EC)
+ msleep(EC_REBOOT_DELAY_MS);
+
mutex_unlock(&ec_spi->lock);
return ret;
}
--
2.1.0
^ permalink raw reply related
* [PATCH v4 2/5] i2c: i2c-cros-ec-tunnel: Set retries to 3
From: Javier Martinez Canillas @ 2014-09-18 15:18 UTC (permalink / raw)
To: Lee Jones
Cc: Wolfram Sang, Dmitry Torokhov, Doug Anderson, Simon Glass,
Bill Richardson, Andrew Bresticker, Derek Basehore,
Gwendal Grignou, linux-i2c, linux-input, linux-samsung-soc,
Javier Martinez Canillas
In-Reply-To: <1411053538-17237-1-git-send-email-javier.martinez@collabora.co.uk>
From: Derek Basehore <dbasehore@chromium.org>
Since the i2c bus can get wedged on the EC sometimes, set the number of retries
to 3. Since we un-wedge the bus immediately after the wedge happens, this is the
correct fix since only one transfer will fail.
Signed-off-by: Derek Basehore <dbasehore@chromium.org>
Reviewed-by: Doug Anderson <dianders@chromium.org>
Acked-by: Wolfram Sang <wsa@the-dreams.de>
Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---
drivers/i2c/busses/i2c-cros-ec-tunnel.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/i2c/busses/i2c-cros-ec-tunnel.c b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
index 3c15dcc..97e6369 100644
--- a/drivers/i2c/busses/i2c-cros-ec-tunnel.c
+++ b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
@@ -16,6 +16,8 @@
#include <linux/platform_device.h>
#include <linux/slab.h>
+#define I2C_MAX_RETRIES 3
+
/**
* struct ec_i2c_device - Driver data for I2C tunnel
*
@@ -290,6 +292,7 @@ static int ec_i2c_probe(struct platform_device *pdev)
bus->adap.algo_data = bus;
bus->adap.dev.parent = &pdev->dev;
bus->adap.dev.of_node = np;
+ bus->adap.retries = I2C_MAX_RETRIES;
err = i2c_add_adapter(&bus->adap);
if (err) {
--
2.1.0
^ permalink raw reply related
* [PATCH v4 3/5] mfd: cros_ec: stop calling ->cmd_xfer() directly
From: Javier Martinez Canillas @ 2014-09-18 15:18 UTC (permalink / raw)
To: Lee Jones
Cc: Wolfram Sang, Dmitry Torokhov, Doug Anderson, Simon Glass,
Bill Richardson, Andrew Bresticker, Derek Basehore,
Gwendal Grignou, linux-i2c, linux-input, linux-samsung-soc,
Javier Martinez Canillas
In-Reply-To: <1411053538-17237-1-git-send-email-javier.martinez@collabora.co.uk>
From: Andrew Bresticker <abrestic@chromium.org>
Instead of having users of the ChromeOS EC call the interface-specific
cmd_xfer() callback directly, introduce a central cros_ec_cmd_xfer()
to use instead. This will allow us to put all the locking and retry
logic in one place instead of duplicating it across the different
drivers.
Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
Reviewed-by: Simon Glass <sjg@chromium.org>
Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
Reviewed-by: Doug Anderson <dianders@chromium.org>
Acked-by: Lee Jones <lee.jones@linaro.org>
---
drivers/i2c/busses/i2c-cros-ec-tunnel.c | 2 +-
drivers/input/keyboard/cros_ec_keyb.c | 2 +-
drivers/mfd/cros_ec.c | 7 +++++++
include/linux/mfd/cros_ec.h | 24 ++++++++++++++++++------
4 files changed, 27 insertions(+), 8 deletions(-)
diff --git a/drivers/i2c/busses/i2c-cros-ec-tunnel.c b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
index 97e6369..ec5c38d 100644
--- a/drivers/i2c/busses/i2c-cros-ec-tunnel.c
+++ b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
@@ -229,7 +229,7 @@ static int ec_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg i2c_msgs[],
msg.indata = response;
msg.insize = response_len;
- result = bus->ec->cmd_xfer(bus->ec, &msg);
+ result = cros_ec_cmd_xfer(bus->ec, &msg);
if (result < 0)
goto exit;
diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
index 8c09b3e..462bfcb 100644
--- a/drivers/input/keyboard/cros_ec_keyb.c
+++ b/drivers/input/keyboard/cros_ec_keyb.c
@@ -157,7 +157,7 @@ static int cros_ec_keyb_get_state(struct cros_ec_keyb *ckdev, uint8_t *kb_state)
.insize = ckdev->cols,
};
- return ckdev->ec->cmd_xfer(ckdev->ec, &msg);
+ return cros_ec_cmd_xfer(ckdev->ec, &msg);
}
static irqreturn_t cros_ec_keyb_irq(int irq, void *data)
diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
index 4873f9c..a9faebd 100644
--- a/drivers/mfd/cros_ec.c
+++ b/drivers/mfd/cros_ec.c
@@ -62,6 +62,13 @@ int cros_ec_check_result(struct cros_ec_device *ec_dev,
}
EXPORT_SYMBOL(cros_ec_check_result);
+int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev,
+ struct cros_ec_command *msg)
+{
+ return ec_dev->cmd_xfer(ec_dev, msg);
+}
+EXPORT_SYMBOL(cros_ec_cmd_xfer);
+
static const struct mfd_cell cros_devs[] = {
{
.name = "cros-ec-keyb",
diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
index fcbe9d1..0e166b9 100644
--- a/include/linux/mfd/cros_ec.h
+++ b/include/linux/mfd/cros_ec.h
@@ -62,10 +62,6 @@ struct cros_ec_command {
* @dev: Device pointer
* @was_wake_device: true if this device was set to wake the system from
* sleep at the last suspend
- * @cmd_xfer: send command to EC and get response
- * Returns the number of bytes received if the communication succeeded, but
- * that doesn't mean the EC was happy with the command. The caller
- * should check msg.result for the EC's result code.
*
* @priv: Private data
* @irq: Interrupt to use
@@ -82,6 +78,10 @@ struct cros_ec_command {
* @dout_size: size of dout buffer to allocate (zero to use static dout)
* @parent: pointer to parent device (e.g. i2c or spi device)
* @wake_enabled: true if this device can wake the system from sleep
+ * @cmd_xfer: send command to EC and get response
+ * Returns the number of bytes received if the communication succeeded, but
+ * that doesn't mean the EC was happy with the command. The caller
+ * should check msg.result for the EC's result code.
* @lock: one transaction at a time
*/
struct cros_ec_device {
@@ -92,8 +92,6 @@ struct cros_ec_device {
struct device *dev;
bool was_wake_device;
struct class *cros_class;
- int (*cmd_xfer)(struct cros_ec_device *ec,
- struct cros_ec_command *msg);
/* These are used to implement the platform-specific interface */
void *priv;
@@ -104,6 +102,8 @@ struct cros_ec_device {
int dout_size;
struct device *parent;
bool wake_enabled;
+ int (*cmd_xfer)(struct cros_ec_device *ec,
+ struct cros_ec_command *msg);
struct mutex lock;
};
@@ -153,6 +153,18 @@ int cros_ec_check_result(struct cros_ec_device *ec_dev,
struct cros_ec_command *msg);
/**
+ * cros_ec_cmd_xfer - Send a command to the ChromeOS EC
+ *
+ * Call this to send a command to the ChromeOS EC. This should be used
+ * instead of calling the EC's cmd_xfer() callback directly.
+ *
+ * @ec_dev: EC device
+ * @msg: Message to write
+ */
+int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev,
+ struct cros_ec_command *msg);
+
+/**
* cros_ec_remove - Remove a ChromeOS EC
*
* Call this to deregister a ChromeOS EC, then clean up any private data.
--
2.1.0
^ permalink raw reply related
* [PATCH v4 4/5] mfd: cros_ec: move locking into cros_ec_cmd_xfer
From: Javier Martinez Canillas @ 2014-09-18 15:18 UTC (permalink / raw)
To: Lee Jones
Cc: Wolfram Sang, Dmitry Torokhov, Doug Anderson, Simon Glass,
Bill Richardson, Andrew Bresticker, Derek Basehore,
Gwendal Grignou, linux-i2c, linux-input, linux-samsung-soc,
Javier Martinez Canillas
In-Reply-To: <1411053538-17237-1-git-send-email-javier.martinez@collabora.co.uk>
From: Andrew Bresticker <abrestic@chromium.org>
Now that there's a central cros_ec_cmd_xfer(), move the locking
out of the SPI driver.
Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
Reviewed-by: Simon Glass <sjg@chromium.org>
Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
Reviewed-by: Doug Anderson <dianders@chromium.org>
Acked-by: Lee Jones <lee.jones@linaro.org>
---
Changes since v1:
- Remove mention of LPC driver in the commit message since it does not
exist in mainline yet. Suggested by Doug Anderson.
drivers/mfd/cros_ec.c | 10 +++++++++-
drivers/mfd/cros_ec_spi.c | 11 -----------
2 files changed, 9 insertions(+), 12 deletions(-)
diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
index a9faebd..c53804a 100644
--- a/drivers/mfd/cros_ec.c
+++ b/drivers/mfd/cros_ec.c
@@ -65,7 +65,13 @@ EXPORT_SYMBOL(cros_ec_check_result);
int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev,
struct cros_ec_command *msg)
{
- return ec_dev->cmd_xfer(ec_dev, msg);
+ int ret;
+
+ mutex_lock(&ec_dev->lock);
+ ret = ec_dev->cmd_xfer(ec_dev, msg);
+ mutex_unlock(&ec_dev->lock);
+
+ return ret;
}
EXPORT_SYMBOL(cros_ec_cmd_xfer);
@@ -98,6 +104,8 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
return -ENOMEM;
}
+ mutex_init(&ec_dev->lock);
+
err = mfd_add_devices(dev, 0, cros_devs,
ARRAY_SIZE(cros_devs),
NULL, ec_dev->irq, NULL);
diff --git a/drivers/mfd/cros_ec_spi.c b/drivers/mfd/cros_ec_spi.c
index b396705..bf6e08e 100644
--- a/drivers/mfd/cros_ec_spi.c
+++ b/drivers/mfd/cros_ec_spi.c
@@ -79,13 +79,11 @@
* if no record
* @end_of_msg_delay: used to set the delay_usecs on the spi_transfer that
* is sent when we want to turn off CS at the end of a transaction.
- * @lock: mutex to ensure only one user of cros_ec_cmd_xfer_spi at a time
*/
struct cros_ec_spi {
struct spi_device *spi;
s64 last_transfer_ns;
unsigned int end_of_msg_delay;
- struct mutex lock;
};
static void debug_packet(struct device *dev, const char *name, u8 *ptr,
@@ -232,13 +230,6 @@ static int cros_ec_cmd_xfer_spi(struct cros_ec_device *ec_dev,
int sum;
int ret = 0, final_ret;
- /*
- * We have the shared ec_dev buffer plus we do lots of separate spi_sync
- * calls, so we need to make sure only one person is using this at a
- * time.
- */
- mutex_lock(&ec_spi->lock);
-
len = cros_ec_prepare_tx(ec_dev, ec_msg);
dev_dbg(ec_dev->dev, "prepared, len=%d\n", len);
@@ -327,7 +318,6 @@ exit:
if (ec_msg->command == EC_CMD_REBOOT_EC)
msleep(EC_REBOOT_DELAY_MS);
- mutex_unlock(&ec_spi->lock);
return ret;
}
@@ -359,7 +349,6 @@ static int cros_ec_spi_probe(struct spi_device *spi)
if (ec_spi == NULL)
return -ENOMEM;
ec_spi->spi = spi;
- mutex_init(&ec_spi->lock);
ec_dev = devm_kzalloc(dev, sizeof(*ec_dev), GFP_KERNEL);
if (!ec_dev)
return -ENOMEM;
--
2.1.0
^ permalink raw reply related
* [PATCH v4 5/5] mfd: cros_ec: wait for completion of commands that return IN_PROGRESS
From: Javier Martinez Canillas @ 2014-09-18 15:18 UTC (permalink / raw)
To: Lee Jones
Cc: Wolfram Sang, Dmitry Torokhov, Doug Anderson, Simon Glass,
Bill Richardson, Andrew Bresticker, Derek Basehore,
Gwendal Grignou, linux-i2c-u79uwXL29TY76Z2rM5mHXA,
linux-input-u79uwXL29TY76Z2rM5mHXA,
linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA,
Javier Martinez Canillas
In-Reply-To: <1411053538-17237-1-git-send-email-javier.martinez-ZGY8ohtN/8pPYcu2f3hruQ@public.gmane.org>
From: Andrew Bresticker <abrestic-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
When an EC command returns EC_RES_IN_PROGRESS, we need to query
the state of the EC until it indicates that it is no longer busy.
Do this in cros_ec_cmd_xfer() under the EC's mutex so that other
commands (e.g. keyboard, I2C passtru) aren't issued to the EC while
it is working on the in-progress command.
The 10 milliseconds delay and the number of retries are the values
that were used by the flashrom tool when retrying commands.
Signed-off-by: Andrew Bresticker <abrestic-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Reviewed-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Signed-off-by: Javier Martinez Canillas <javier.martinez-ZGY8ohtN/8pPYcu2f3hruQ@public.gmane.org>
---
Changes since v3:
- Fix us time and use raw values instead of a define. Suggested by Lee Jones.
Changes since v2:
- Explain in the commit message from where the delay and retry values come from.
Commented by Andrew Bresticker.
- Move the needed definitions inside the if block. Suggested by Lee Jones.
- Only check if result is EC_RES_IN_PROGRESS instead of checking also if ret is
-EAGAIN since the former implies the later. Suggested by Lee Jones.
- Use usleep_range() instead of msleep() since doesn't handle values between 1~20.
Suggested by Lee Jones.
Changes since v1:
- The *xfer() calls don't modify the passed cros_ec_command so there is
no need to populate it inside the for loop. Suggested by Lee Jones.
drivers/mfd/cros_ec.c | 33 +++++++++++++++++++++++++++++++++
1 file changed, 33 insertions(+)
diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
index c53804a..fc0c81e 100644
--- a/drivers/mfd/cros_ec.c
+++ b/drivers/mfd/cros_ec.c
@@ -23,6 +23,9 @@
#include <linux/mfd/core.h>
#include <linux/mfd/cros_ec.h>
#include <linux/mfd/cros_ec_commands.h>
+#include <linux/delay.h>
+
+#define EC_COMMAND_RETRIES 50
int cros_ec_prepare_tx(struct cros_ec_device *ec_dev,
struct cros_ec_command *msg)
@@ -69,6 +72,36 @@ int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev,
mutex_lock(&ec_dev->lock);
ret = ec_dev->cmd_xfer(ec_dev, msg);
+ if (msg->result == EC_RES_IN_PROGRESS) {
+ int i;
+ struct cros_ec_command status_msg;
+ struct ec_response_get_comms_status status;
+
+ status_msg.version = 0;
+ status_msg.command = EC_CMD_GET_COMMS_STATUS;
+ status_msg.outdata = NULL;
+ status_msg.outsize = 0;
+ status_msg.indata = (uint8_t *)&status;
+ status_msg.insize = sizeof(status);
+
+ /*
+ * Query the EC's status until it's no longer busy or
+ * we encounter an error.
+ */
+ for (i = 0; i < EC_COMMAND_RETRIES; i++) {
+ usleep_range(10000, 11000);
+
+ ret = ec_dev->cmd_xfer(ec_dev, &status_msg);
+ if (ret < 0)
+ break;
+
+ msg->result = status_msg.result;
+ if (status_msg.result != EC_RES_SUCCESS)
+ break;
+ if (!(status.flags & EC_COMMS_STATUS_PROCESSING))
+ break;
+ }
+ }
mutex_unlock(&ec_dev->lock);
return ret;
--
2.1.0
^ permalink raw reply related
* [PATCH v4 0/5] Second batch of cleanups for cros_ec
From: Javier Martinez Canillas @ 2014-09-18 15:18 UTC (permalink / raw)
To: Lee Jones
Cc: Wolfram Sang, Dmitry Torokhov, Doug Anderson, Simon Glass,
Bill Richardson, Andrew Bresticker, Derek Basehore,
Gwendal Grignou, linux-i2c, linux-input, linux-samsung-soc,
Javier Martinez Canillas
Hello,
This is a second batch of cleanups patches for the mfd cros_ec
driver and its subdevices drivers. The first batch of cleanups
was posted by Doug Anderson [0] and have already been merged.
The patches were picked from the ChromeOS 3.8 kernel and after
these no cleanups patches for cros_ec are left, only commits
that add cros ec support not yet available in mainline.
This is a fourth version of the patch series that addresses
issues pointed out by Lee Jones on v3 [1]. I dropped patch #6
from this version since that change will make more sense once
the cros_ec_dev driver is added and the cells array isn't empty.
There is almost no functionality added on this series but the
idea is to reduce the delta between the mainline drivers and
the ones in the downstream Chrome OS 3.8 kernel so the missing
functionality can be added on top once these cleanups patches
are merged. The missing functionality currently in mainline is:
- Chrome OS Embedded Controller userspace device interface
- Chrome OS Embedded Controller Low Pin Count (LPC) inteface
- Access to vboot context stored on a block device
- Access to vboot context stored on EC's nvram
The patches in this series are authored by different people
(all on cc) and consist of the following:
Andrew Bresticker (3):
mfd: cros_ec: stop calling ->cmd_xfer() directly
mfd: cros_ec: move locking into cros_ec_cmd_xfer
mfd: cros_ec: wait for completion of commands that return IN_PROGRESS
Derek Basehore (1):
i2c: i2c-cros-ec-tunnel: Set retries to 3
Doug Anderson (1):
mfd: cros_ec: Delay for 50ms when we see EC_CMD_REBOOT_EC
drivers/i2c/busses/i2c-cros-ec-tunnel.c | 5 +++-
drivers/input/keyboard/cros_ec_keyb.c | 2 +-
drivers/mfd/cros_ec.c | 48 +++++++++++++++++++++++++++++++++
drivers/mfd/cros_ec_spi.c | 20 +++++++-------
include/linux/mfd/cros_ec.h | 24 ++++++++++++-----
5 files changed, 80 insertions(+), 19 deletions(-)
Patches #1 and #2 can be applied independently but patches #3, #4 and #5
rely on the previous one so they should be applied together and in that order.
Best regards,
Javier
[0]: https://www.mail-archive.com/linux-input@vger.kernel.org/msg11385.html
[1]: https://www.mail-archive.com/linux-samsung-soc@vger.kernel.org/msg36505.html
^ permalink raw reply
* Re: [PATCH v2] Input: joystick - Use ktime for measuring timing
From: Dmitry Torokhov @ 2014-09-18 16:56 UTC (permalink / raw)
To: Takashi Iwai
Cc: Andreas Mohr, Clemens Ladisch, Vojtech Pavlik, Jiri Kosina,
linux-input, linux-kernel
In-Reply-To: <s5hmw9x8km7.wl-tiwai@suse.de>
On Thu, Sep 18, 2014 at 09:15:44AM +0200, Takashi Iwai wrote:
> At Wed, 17 Sep 2014 21:23:37 +0200,
> Andreas Mohr wrote:
> >
> > Hi,
> >
> > On Wed, Sep 10, 2014 at 05:57:17PM +0200, Takashi Iwai wrote:
> > > The current codes in gameport and analog joystick drivers for the time
> > > accounting have a long-standing problem when the system is running
> > > with CPU freq; since the timing is measured via TSC or sample counter,
> > > the calculation isn't reliable.
> >
> > Thank you very much for having followed through on this!
> > (somehow you never seem to disappoint me :)
> >
> > While working on testing this and doing various gameport/soundcard modifications,
> > I'm afraid I have seen the following checkpatch.pl (of v3.16) warnings:
> >
> > WARNING: Missing a blank line after declarations
> > #164: FILE: drivers/input/joystick/analog.c:192:
> > + unsigned int x;
> > + GET_TIME(x);
> >
> > WARNING: line over 80 characters
> > #224: FILE: drivers/input/joystick/analog.c:301:
> > + port->axes[j] = (delta(start, time[i]) << ANALOG_FUZZ_BITS) / port->loop;
>
> Both are cosmetic and I won't be bothered if it were sent to my tree,
> for example ;) It depends on the maintainer, so if Dmitry wants these
> to be fixed, I'm willing to resubmit, of course.
No, I do not. If there is a large rework we might want to reformat it,
otherwise let's leave it as is.
Thanks.
--
Dmitry
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox