* Re: [PATCH V3 1/2] tps6507x-ts: Add DT support
From: Manish Badarkhe @ 2013-07-29 6:17 UTC (permalink / raw)
To: Vishwanathrao Badarkhe, Manish
Cc: Dmitry Torokhov, davinci-linux-open-source@linux.davincidsp.com,
linux-doc@vger.kernel.org, devicetree-discuss@lists.ozlabs.org,
linux-kernel@vger.kernel.org, rob.herring@calxeda.com,
grant.likely@secretlab.ca, rob@landley.net,
linux-input@vger.kernel.org
In-Reply-To: <CAKDJKT6UETVpPT9B6LE59_EMrxNGpTmUiu9iddHoWOYhGpHP3Q@mail.gmail.com>
Hi Dmitry
On Mon, Jul 1, 2013 at 4:40 PM, Manish Badarkhe
<badarkhe.manish@gmail.com> wrote:
>
> Hi Dmitry
>
>
> On Wed, Jun 12, 2013 at 12:23 PM, Vishwanathrao Badarkhe, Manish
> <manishv.b@ti.com> wrote:
> > Hi Dmitry,
> >
> > On Mon, Jun 10, 2013 at 23:33:11, Dmitry Torokhov wrote:
> >> Manish,
> >>
> >> On Mon, Jun 10, 2013 at 10:23:16AM +0000, Vishwanathrao Badarkhe,
> >> Manish wrote:
> >> > Hi Dmitry,
> >> >
> >> > On Mon, Jun 10, 2013 at 11:34:42, Dmitry Torokhov wrote:
> >> > > Hi Manish,
> >> > >
> >> > > On Tue, May 21, 2013 at 02:24:17PM +0530, Vishwanathrao Badarkhe,
> >> > > Manish wrote:
> >> > >
> >> > > > + struct touchscreen_init_data *init_data = NULL;
> >> > > > + int err;
> >> > > > +
> >> > > > + if (node)
> >> > > > + node = of_find_node_by_name(node, "tsc");
> >> > >
> >> > > Why do you have to locate OF node manually instead of already
> >> > > having it attached to the device stucture?
> >> >
> >> > As TPS6507x is mfd device containing two nodes, regulator and
> >> > touchscreen.
> >> > It is necessary to use "of_find_node_by_name" to find child "tsc"
> >> > node
> >> > of TPS6507x MFD device.
> >>
> >> I understand that TPS6507x is a MFD device, However, I still do not
> >> understand why you do not attach OF data to the child platform device
> >> representing touch screen when you create it.
> >
> > I gone through most of MFD devices in mainline kernel where
> > "of_find_node_by_name"
> > function is used in order to populate child node properties.
> > See below example of max8925 MFD device.
> >
> > Max8025 MFD device has following modules
> > 1. Touch screen
> > 2. Charger
> > 3. Backlight
> > 4. regulator
> >
> > In DT case, device node for max8925 MFD device is in
> > "arch/arm/boot/dts/mmp2-brownstone.dts"
> > file having child nodes as regulator, backlight and charger.
> > Respective drivers (regulator, backlight and charger) of max8925 MFD
> > device used
> > "of_find_node_by_name" function in order to populate child node
> > properties.
> >
> > Here, also same case for TPS6507x MFD device
> >
> > TPS6507x MFD device has two childs:
> > 1. regulator
> > 2. touch screen.
> >
> > Regulator driver for TPS6507x is already in mainline and using function
> > "of_find_node_by_name" to populate device tree properties for
> > regulators.
> > On similar lines I used "of_find_node_by_name" to populate device
> > tree properties for touch screen.
> >
> > Still, I agreed that it is possible to attach OF data to child platform
> > device
> > but it requires changes in MFD driver of TPS6507x.
> >
> > Please let me know your opinion about this.
>
> Please let me know so accordingly I will make changes in code.
>
Please provide your comments so accordingly will make changes in code.
Regards
Manish Badarkhe
^ permalink raw reply
* Re: [PATCH 3/5] input.h: add keycodes for Fn Lock
From: Dmitry Torokhov @ 2013-07-29 4:53 UTC (permalink / raw)
To: Mauro Carvalho Chehab; +Cc: linux-input
In-Reply-To: <1375070379-329-3-git-send-email-m.chehab@samsung.com>
Hi Mauro,
On Mon, Jul 29, 2013 at 12:59:37AM -0300, Mauro Carvalho Chehab wrote:
> Samsung notebooks have a FN LOCK key. It works like CAPS LOCK or NUM
> LOCK keys.
>
> When FN LOCK key is pressed, any further press to a key with a blue label
> on it (Fn keys) will produce the alternate code.
>
> Another press makes the keyboard to return to its normal state.
>
> On the notebooks where such feature were found, a FN LOCK on event
> produces scancode 0xa8, and a FN LOCK off event produces scancode 0xa9.
>
> Yet, it is better to reserve some space at the keymap to allow some
> different implementation of this feature where the same keycode might
> be used.
>
> Also, as this is actually a switch, add a switch indicator to report
> when this switch is set/reset.
>
> Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
> ---
> include/uapi/linux/input.h | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h
> index d584047..4622c34 100644
> --- a/include/uapi/linux/input.h
> +++ b/include/uapi/linux/input.h
> @@ -716,6 +716,10 @@ struct input_keymap_entry {
> #define BTN_DPAD_LEFT 0x222
> #define BTN_DPAD_RIGHT 0x223
>
> +#define KEY_FNLOCK_TOGGLE 0x224 /* Request switch Fn on or off */
> +#define KEY_FNLOCK_ON 0x225
> +#define KEY_FNLOCK_OFF 0x226
> +
> #define BTN_TRIGGER_HAPPY 0x2c0
> #define BTN_TRIGGER_HAPPY1 0x2c0
> #define BTN_TRIGGER_HAPPY2 0x2c1
> @@ -853,6 +857,7 @@ struct input_keymap_entry {
> #define SW_FRONT_PROXIMITY 0x0b /* set = front proximity sensor active */
> #define SW_ROTATE_LOCK 0x0c /* set = rotate locked/disabled */
> #define SW_LINEIN_INSERT 0x0d /* set = inserted */
> +#define SW_FNLOCK 0x0e /* set = Fn locked */
I am not sure if we need both the keys and the switch, so I would
probably simply go with the keys, and not bother with switch. Then we do
not need to touch the atkbd driver at all and rely on udev to set up the
keymap and force release keys.
--
Dmitry
^ permalink raw reply
* Re: [PATCH 2/5] atkbd: Fix key release for Fn keys on Samsung series 5 550P5C
From: Dmitry Torokhov @ 2013-07-29 4:49 UTC (permalink / raw)
To: Mauro Carvalho Chehab; +Cc: linux-input
In-Reply-To: <1375070379-329-2-git-send-email-m.chehab@samsung.com>
Hi Mauro,
On Mon, Jul 29, 2013 at 12:59:36AM -0300, Mauro Carvalho Chehab wrote:
> Fix key release for Fn keys with Samsung series 5
>
> Samsung series 5 notebooks don't produce release events for certain
> keys (Fn+F1, Fn+F11, Fn+F12 and Fn Lock). Add those keys at the release
> fixup table.
As I mentioned, the force release should be handled by udev instead of
adding kernel driver quirk.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH 1/5] atkbd: Fix key release for Fn keys on Samsung series 5 ultra 540UC
From: Dmitry Torokhov @ 2013-07-29 4:49 UTC (permalink / raw)
To: Mauro Carvalho Chehab; +Cc: linux-input
In-Reply-To: <1375070379-329-1-git-send-email-m.chehab@samsung.com>
Hi Mauro,
On Mon, Jul 29, 2013 at 12:59:35AM -0300, Mauro Carvalho Chehab wrote:
> Samsung series 5 ultra notebooks don't produce release events for certain
> keys (Fn+F1, Fn+F11, Fn+F12 and Fn Lock). Add those keys at the release
> fixup table.
>
As I mentioned, the force release should be handled by udev instead of
adding kernel driver quirk.
Thanks.
--
Dmitry
^ permalink raw reply
* [PATCH 5/5] atkbd: only enable SW_FNLOCK on keyboards that have FN LOCK key
From: Mauro Carvalho Chehab @ 2013-07-29 3:59 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, Mauro Carvalho Chehab
In-Reply-To: <1375070379-329-1-git-send-email-m.chehab@samsung.com>
Instead of unconditionally enabling the switch, only make it available
where the key actually exists.
Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
---
drivers/input/keyboard/atkbd.c | 58 ++++++++++++++++++++++++------------------
1 file changed, 33 insertions(+), 25 deletions(-)
diff --git a/drivers/input/keyboard/atkbd.c b/drivers/input/keyboard/atkbd.c
index 7f88132..3ec7d67 100644
--- a/drivers/input/keyboard/atkbd.c
+++ b/drivers/input/keyboard/atkbd.c
@@ -240,6 +240,7 @@ struct atkbd {
/*
* System-specific keymap fixup routine
*/
+static bool atkbd_platform_has_fnlock;
static void (*atkbd_platform_fixup)(struct atkbd *, const void *data);
static void *atkbd_platform_fixup_data;
static unsigned int (*atkbd_platform_scancode_fixup)(struct atkbd *, unsigned int);
@@ -454,26 +455,22 @@ static irqreturn_t atkbd_interrupt(struct serio *serio, unsigned char data,
if (keycode != ATKBD_KEY_NULL)
input_event(dev, EV_MSC, MSC_SCAN, code);
- /*
- * Handles special Fn Lock switch.
- *
- * This switch can't be merged with the next one, as we also want
- * to report those keycodes to userspace, via input_event, and
- * call input_sync().
- */
- switch (keycode) {
- case KEY_FNLOCK_OFF:
- atkbd->fnlock_set = false;
- input_report_switch(dev, SW_FNLOCK, atkbd->fnlock_set);
- break;
- case KEY_FNLOCK_ON:
- atkbd->fnlock_set = true;
- input_report_switch(dev, SW_FNLOCK, atkbd->fnlock_set);
- break;
- case KEY_FNLOCK_TOGGLE:
- atkbd->fnlock_set = !atkbd->fnlock_set;
- input_report_switch(dev, SW_FNLOCK, atkbd->fnlock_set);
- break;
+ /* Handles special Fn Lock switch found on some notebooks */
+ if (test_bit(SW_FNLOCK, dev->swbit)) {
+ switch (keycode) {
+ case KEY_FNLOCK_OFF:
+ atkbd->fnlock_set = false;
+ input_report_switch(dev, SW_FNLOCK, atkbd->fnlock_set);
+ break;
+ case KEY_FNLOCK_ON:
+ atkbd->fnlock_set = true;
+ input_report_switch(dev, SW_FNLOCK, atkbd->fnlock_set);
+ break;
+ case KEY_FNLOCK_TOGGLE:
+ atkbd->fnlock_set = !atkbd->fnlock_set;
+ input_report_switch(dev, SW_FNLOCK, atkbd->fnlock_set);
+ break;
+ }
}
switch (keycode) {
@@ -1150,9 +1147,10 @@ static void atkbd_set_device_attrs(struct atkbd *atkbd)
}
}
- /* FIXME: Unconditionqally sets FN LOCK switch */
- __set_bit(SW_FNLOCK, atkbd->dev->swbit);
- input_dev->evbit[0] |= BIT_MASK(EV_SW);
+ if (atkbd_platform_has_fnlock) {
+ __set_bit(SW_FNLOCK, atkbd->dev->swbit);
+ input_dev->evbit[0] |= BIT_MASK(EV_SW);
+ }
}
/*
@@ -1672,6 +1670,16 @@ static int __init atkbd_setup_forced_release(const struct dmi_system_id *id)
return 1;
}
+static int __init
+atkbd_setup_forced_release_and_fnlock(const struct dmi_system_id *id)
+{
+ atkbd_platform_fixup = atkbd_apply_forced_release_keylist;
+ atkbd_platform_fixup_data = id->driver_data;
+ atkbd_platform_has_fnlock = true;
+
+ return 1;
+}
+
static int __init atkbd_setup_scancode_fixup(const struct dmi_system_id *id)
{
atkbd_platform_scancode_fixup = id->driver_data;
@@ -1778,7 +1786,7 @@ static const struct dmi_system_id atkbd_dmi_quirk_table[] __initconst = {
DMI_MATCH(DMI_SYS_VENDOR, "SAMSUNG ELECTRONICS CO., LTD."),
DMI_MATCH(DMI_PRODUCT_NAME, "530U3C"),
},
- .callback = atkbd_setup_forced_release,
+ .callback = atkbd_setup_forced_release_and_fnlock,
.driver_data = atkbd_samsung_530u3c_forced_release_keys,
},
{
@@ -1787,7 +1795,7 @@ static const struct dmi_system_id atkbd_dmi_quirk_table[] __initconst = {
DMI_MATCH(DMI_SYS_VENDOR, "SAMSUNG ELECTRONICS CO., LTD."),
DMI_MATCH(DMI_PRODUCT_NAME, "550P5C"),
},
- .callback = atkbd_setup_forced_release,
+ .callback = atkbd_setup_forced_release_and_fnlock,
.driver_data = atkbd_samsung_550p5c_forced_release_keys,
},
{
--
1.8.3.1
^ permalink raw reply related
* [PATCH 3/5] input.h: add keycodes for Fn Lock
From: Mauro Carvalho Chehab @ 2013-07-29 3:59 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, Mauro Carvalho Chehab
In-Reply-To: <1375070379-329-1-git-send-email-m.chehab@samsung.com>
Samsung notebooks have a FN LOCK key. It works like CAPS LOCK or NUM
LOCK keys.
When FN LOCK key is pressed, any further press to a key with a blue label
on it (Fn keys) will produce the alternate code.
Another press makes the keyboard to return to its normal state.
On the notebooks where such feature were found, a FN LOCK on event
produces scancode 0xa8, and a FN LOCK off event produces scancode 0xa9.
Yet, it is better to reserve some space at the keymap to allow some
different implementation of this feature where the same keycode might
be used.
Also, as this is actually a switch, add a switch indicator to report
when this switch is set/reset.
Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
---
include/uapi/linux/input.h | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h
index d584047..4622c34 100644
--- a/include/uapi/linux/input.h
+++ b/include/uapi/linux/input.h
@@ -716,6 +716,10 @@ struct input_keymap_entry {
#define BTN_DPAD_LEFT 0x222
#define BTN_DPAD_RIGHT 0x223
+#define KEY_FNLOCK_TOGGLE 0x224 /* Request switch Fn on or off */
+#define KEY_FNLOCK_ON 0x225
+#define KEY_FNLOCK_OFF 0x226
+
#define BTN_TRIGGER_HAPPY 0x2c0
#define BTN_TRIGGER_HAPPY1 0x2c0
#define BTN_TRIGGER_HAPPY2 0x2c1
@@ -853,6 +857,7 @@ struct input_keymap_entry {
#define SW_FRONT_PROXIMITY 0x0b /* set = front proximity sensor active */
#define SW_ROTATE_LOCK 0x0c /* set = rotate locked/disabled */
#define SW_LINEIN_INSERT 0x0d /* set = inserted */
+#define SW_FNLOCK 0x0e /* set = Fn locked */
#define SW_MAX 0x0f
#define SW_CNT (SW_MAX+1)
--
1.8.3.1
^ permalink raw reply related
* [PATCH 4/5] atkbd: add support for handling KEY_FNLOCK
From: Mauro Carvalho Chehab @ 2013-07-29 3:59 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, Mauro Carvalho Chehab
In-Reply-To: <1375070379-329-1-git-send-email-m.chehab@samsung.com>
Samsung keyboards have a key called FN LOCK. This key acts like
CAPS LOCK and NUM LOCK. When pressed, the keyboard switches to a state
where the produced scancode are the blue ones (the Fn keycodes).
Add support for it at atkbd.
Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
---
drivers/input/keyboard/atkbd.c | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/drivers/input/keyboard/atkbd.c b/drivers/input/keyboard/atkbd.c
index 298ac1d..7f88132 100644
--- a/drivers/input/keyboard/atkbd.c
+++ b/drivers/input/keyboard/atkbd.c
@@ -218,6 +218,7 @@ struct atkbd {
bool softraw;
bool scroll;
bool enabled;
+ bool fnlock_set;
/* Accessed only from interrupt */
unsigned char emul;
@@ -453,6 +454,28 @@ static irqreturn_t atkbd_interrupt(struct serio *serio, unsigned char data,
if (keycode != ATKBD_KEY_NULL)
input_event(dev, EV_MSC, MSC_SCAN, code);
+ /*
+ * Handles special Fn Lock switch.
+ *
+ * This switch can't be merged with the next one, as we also want
+ * to report those keycodes to userspace, via input_event, and
+ * call input_sync().
+ */
+ switch (keycode) {
+ case KEY_FNLOCK_OFF:
+ atkbd->fnlock_set = false;
+ input_report_switch(dev, SW_FNLOCK, atkbd->fnlock_set);
+ break;
+ case KEY_FNLOCK_ON:
+ atkbd->fnlock_set = true;
+ input_report_switch(dev, SW_FNLOCK, atkbd->fnlock_set);
+ break;
+ case KEY_FNLOCK_TOGGLE:
+ atkbd->fnlock_set = !atkbd->fnlock_set;
+ input_report_switch(dev, SW_FNLOCK, atkbd->fnlock_set);
+ break;
+ }
+
switch (keycode) {
case ATKBD_KEY_NULL:
break;
@@ -1126,6 +1149,10 @@ static void atkbd_set_device_attrs(struct atkbd *atkbd)
__set_bit(atkbd->keycode[i], input_dev->keybit);
}
}
+
+ /* FIXME: Unconditionqally sets FN LOCK switch */
+ __set_bit(SW_FNLOCK, atkbd->dev->swbit);
+ input_dev->evbit[0] |= BIT_MASK(EV_SW);
}
/*
--
1.8.3.1
^ permalink raw reply related
* [PATCH 1/5] atkbd: Fix key release for Fn keys on Samsung series 5 ultra 540UC
From: Mauro Carvalho Chehab @ 2013-07-29 3:59 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, Mauro Carvalho Chehab
Samsung series 5 ultra notebooks don't produce release events for certain
keys (Fn+F1, Fn+F11, Fn+F12 and Fn Lock). Add those keys at the release
fixup table.
Note: Fn Lock actually produces two scancodes: 0xa8 and 0xa9.
Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
---
drivers/input/keyboard/atkbd.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/drivers/input/keyboard/atkbd.c b/drivers/input/keyboard/atkbd.c
index 2626773..a18bf03 100644
--- a/drivers/input/keyboard/atkbd.c
+++ b/drivers/input/keyboard/atkbd.c
@@ -943,6 +943,13 @@ static unsigned int atkbd_samsung_forced_release_keys[] = {
};
/*
+ * Samsung 530U3C/530U4C/532U3C/540U3C Fn release keys not working
+ */
+static unsigned int atkbd_samsung_530u3c_forced_release_keys[] = {
+ 0xa8, 0xa9, 0xb3, 0xce, 0xd5, -1U
+};
+
+/*
* Amilo Pi 3525 key release for Fn+Volume keys not working
*/
static unsigned int atkbd_amilo_pi3525_forced_release_keys[] = {
@@ -1732,6 +1739,15 @@ static const struct dmi_system_id atkbd_dmi_quirk_table[] __initconst = {
.driver_data = atkbd_samsung_forced_release_keys,
},
{
+ /* Samsung series 5 Ultra (530U3C/530U4C/532U3C/540U3C) */
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "SAMSUNG ELECTRONICS CO., LTD."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "530U3C"),
+ },
+ .callback = atkbd_setup_forced_release,
+ .driver_data = atkbd_samsung_530u3c_forced_release_keys,
+ },
+ {
/* Fujitsu Amilo PA 1510 */
.matches = {
DMI_MATCH(DMI_SYS_VENDOR, "FUJITSU SIEMENS"),
--
1.8.3.1
^ permalink raw reply related
* [PATCH 2/5] atkbd: Fix key release for Fn keys on Samsung series 5 550P5C
From: Mauro Carvalho Chehab @ 2013-07-29 3:59 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, Mauro Carvalho Chehab
In-Reply-To: <1375070379-329-1-git-send-email-m.chehab@samsung.com>
Fix key release for Fn keys with Samsung series 5
Samsung series 5 notebooks don't produce release events for certain
keys (Fn+F1, Fn+F11, Fn+F12 and Fn Lock). Add those keys at the release
fixup table.
Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
---
drivers/input/keyboard/atkbd.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/drivers/input/keyboard/atkbd.c b/drivers/input/keyboard/atkbd.c
index a18bf03..298ac1d 100644
--- a/drivers/input/keyboard/atkbd.c
+++ b/drivers/input/keyboard/atkbd.c
@@ -950,6 +950,13 @@ static unsigned int atkbd_samsung_530u3c_forced_release_keys[] = {
};
/*
+ * Samsung 550P5C/550P7C Fn release keys not working
+ */
+static unsigned int atkbd_samsung_550p5c_forced_release_keys[] = {
+ 0xa8, 0xa9, 0xc5, 0xce, 0xd5, -1U
+};
+
+/*
* Amilo Pi 3525 key release for Fn+Volume keys not working
*/
static unsigned int atkbd_amilo_pi3525_forced_release_keys[] = {
@@ -1748,6 +1755,15 @@ static const struct dmi_system_id atkbd_dmi_quirk_table[] __initconst = {
.driver_data = atkbd_samsung_530u3c_forced_release_keys,
},
{
+ /* Samsung series 5 (550P5C/550P7C) */
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "SAMSUNG ELECTRONICS CO., LTD."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "550P5C"),
+ },
+ .callback = atkbd_setup_forced_release,
+ .driver_data = atkbd_samsung_550p5c_forced_release_keys,
+ },
+ {
/* Fujitsu Amilo PA 1510 */
.matches = {
DMI_MATCH(DMI_SYS_VENDOR, "FUJITSU SIEMENS"),
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH] Fix key release for Fn keys with Samsung series 5 ultra
From: Mauro Carvalho Chehab @ 2013-07-29 2:26 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input
In-Reply-To: <12081355.tI48ExGXTL@dtor-d630.eng.vmware.com>
Em Sun, 28 Jul 2013 19:14:56 -0700
Dmitry Torokhov <dmitry.torokhov@gmail.com> escreveu:
> Hi Mauro,
>
> On Sunday, July 28, 2013 07:08:27 AM Mauro Carvalho Chehab wrote:
> > Samsung series 5 ultra notebooks don't produce release events for certain
> > keys (Fn+F1, Fn+F11, Fn+F12 and Fn Lock). Add those keys at the release
> > fixup table.
> >
> > Note: Fn Lock actually produces two scancodes: 0xa8 and 0xa9.
>
> Please add a quirk to udev to handle this please.
Ok. I actually opened already a BZ# adding it to udev at Fedora:
https://bugzilla.redhat.com/show_bug.cgi?id=989103
Btw, please don't apply the patch I sent you yet. I'm finishing to test a
new patchset that includes also another Samsung series 5 notebook, and adds
the switch for FN LOCK.
Thanks!
Mauro
^ permalink raw reply
* Re: [PATCH] Fix key release for Fn keys with Samsung series 5 ultra
From: Dmitry Torokhov @ 2013-07-29 2:14 UTC (permalink / raw)
To: Mauro Carvalho Chehab; +Cc: linux-input
In-Reply-To: <20130728070827.32e7cd31@infradead.org>
Hi Mauro,
On Sunday, July 28, 2013 07:08:27 AM Mauro Carvalho Chehab wrote:
> Samsung series 5 ultra notebooks don't produce release events for certain
> keys (Fn+F1, Fn+F11, Fn+F12 and Fn Lock). Add those keys at the release
> fixup table.
>
> Note: Fn Lock actually produces two scancodes: 0xa8 and 0xa9.
Please add a quirk to udev to handle this please.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [BUG?] EV_LED events not seperated from other events by SYN_REPORT
From: Rolf Morel @ 2013-07-28 12:35 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input
In-Reply-To: <20130728052608.GB23567@core.coreip.homeip.net>
Hi Dmitry,
2013/7/28 Dmitry Torokhov <dmitry.torokhov@gmail.com>:
> - require application writing events into /dev/eventX write EV_SYN as
> well.
>
> At the moment I am inclined to say that applications shoudl do this.
Yes, this is what I was missing/doing incorrectly.
Clients who write to the stream must add there own EV_SYN (SYN_REPORT).
And this would be most correct indeed, as when the writing client, if
there ever where the need, sends events happening at the same moment
in time.
Thanks for replying, it has helped a lot.
^ permalink raw reply
* Re: Samsung series 5 ultra keyboard
From: Mauro Carvalho Chehab @ 2013-07-28 10:45 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input
In-Reply-To: <20130728052243.GA23567@core.coreip.homeip.net>
Em Sat, 27 Jul 2013 22:22:43 -0700
Dmitry Torokhov <dmitry.torokhov@gmail.com> escreveu:
> Hi Mauro,
>
> On Sat, Jul 27, 2013 at 03:06:42PM -0300, Mauro Carvalho Chehab wrote:
> > Em Sat, 27 Jul 2013 10:11:53 -0300
> > Mauro Carvalho Chehab <m.chehab@samsung.com> escreveu:
> >
> > > Hi Dmitry,
> > >
> > > I received a new notebook those days that has an extra feature of a "Fn Lock"
> > > key.
> > >
> > > When this key is pressed, it produces a scan code 0xa8, and the direction
> > > keys (and other keys) start to produce different codes. When pressed again,
> > > it produces a scan code 0xa9, and the keys return to their normal behavior.
> > >
> > > The input layer doesn't currently produce any EV_KEY event for those two
> > > scancodes. It produces just EV_MSC events (and, of course, EV_SYN).
> > >
> > > I was wandering that the better would be to have a LED indicator to
> > > track this, and maybe have two new keycodes, like KEY_FN_LOCK_ON and
> > > KEY_FN_LOCK_OFF.
> > >
> > > This way, some userspace program, like mate-lockkeys-applet could be
> > > presenting not only the CAPS LOCK indicator, but also the FN LOCK
> > > indicator.
> > >
> > > Do you think it would be doable?
>
> Hmm, the behavior better matches a switch than a key, but we do have
> quite a few key-like ON/OFF keys/switches so yes, we could add a couple
> more.
We could map it as a switch. Yet, there are just a couple switches
left on kernel 3.10. So, if I understood well, the patch for input.h
would be something like the one below, right?
diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h
index d584047..13c3c79 100644
--- a/include/uapi/linux/input.h
+++ b/include/uapi/linux/input.h
@@ -716,6 +716,10 @@ struct input_keymap_entry {
#define BTN_DPAD_LEFT 0x222
#define BTN_DPAD_RIGHT 0x223
+#define KEY_FNNLOCK_TOGGLE 0x224 /* Request switch Fn on or off */
+#define KEY_FNLOCK_ON 0x225
+#define KEY_FNLOCK_OFF 0x226
+
#define BTN_TRIGGER_HAPPY 0x2c0
#define BTN_TRIGGER_HAPPY1 0x2c0
#define BTN_TRIGGER_HAPPY2 0x2c1
@@ -853,6 +857,7 @@ struct input_keymap_entry {
#define SW_FRONT_PROXIMITY 0x0b /* set = front proximity sensor active */
#define SW_ROTATE_LOCK 0x0c /* set = rotate locked/disabled */
#define SW_LINEIN_INSERT 0x0d /* set = inserted */
+#define SW_FNLOCK 0x0e /* set = Fn locked */
#define SW_MAX 0x0f
#define SW_CNT (SW_MAX+1)
The changes at atkbd could be a little more complex, as it will
require to support more than one fixups at the same time:
- the release Fn fixup;
- the table fixup for the two addicional keycodes;
- the switch (or led) fixup.
I'll work on it after we decide the better approach (either as a
switch or as a LED).
> > >
> > > FYI, this is how this keyboard identifies itself:
> > >
> > > $ cat /sys/class/input/event3/device/uevent
> > > PRODUCT=11/1/1/ab41
> > > NAME="AT Translated Set 2 keyboard"
> > > PHYS="isa0060/serio0/input0"
> > > PROP=0
> > > EV=120013
> > > KEY=500f02000403 3803078f870d001 feffffdffbefffff fffffffffffffffe
> > > MSC=10
> > > LED=7
> > > MODALIAS=input:b0011v0001p0001eAB41-e0,1,4,11,14,k71,72,73,74,75,76,77,79,7A,7B,7C,7D,7E,7F,80,8C,8E,8F,94,95,96,9B,9C,9D,9E,9F,A3,A4,A5,A6,AC,AD,B7,B8,B9,C0,C1,CA,D9,E0,E1,E2,E3,EC,EE,ram4,l0,1,2,sfw
> > >
> > > I'm not sure if the PRODUCT there is unique, and if it could be used
> > > to add this kind of extra feature to the input subsystem (or if it would
> > > be just fine to add those extra features at the standard AT keyboard
> > > driver, although I personally don't like this idea, as other keyboards
> > > might be using scancodes 0xa8/0xa9 for other meanings).
> > >
> > > What do you think?
> >
> > Just discovered something weird while trying to write some code for
> > systemd/udev: there are a few keys that only produce key down events:
> > Fn + F1 (KEY_SETUP)
> > Fn + F12 (KEY_WLAN)
> > Fn Lock (both scan codes 0xa8 and 0xa9)
>
> That is not new, quite a few laptops do not bother with producing
> release events. There is force_release attribute on serio ports bound to
> atkbd and udev has code to manipulate it to ensure all keys are
> released properly. You just need to add rule for your model.
Ok, that's a trivial patch.
Patch for that sent (and of course tested on the notebook).
Thanks!
Mauro
^ permalink raw reply related
* [PATCH] Fix key release for Fn keys with Samsung series 5 ultra
From: Mauro Carvalho Chehab @ 2013-07-28 10:08 UTC (permalink / raw)
To: linux-input
Samsung series 5 ultra notebooks don't produce release events for certain
keys (Fn+F1, Fn+F11, Fn+F12 and Fn Lock). Add those keys at the release
fixup table.
Note: Fn Lock actually produces two scancodes: 0xa8 and 0xa9.
Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>
diff --git a/drivers/input/keyboard/atkbd.c b/drivers/input/keyboard/atkbd.c
index 2626773..90c3113 100644
--- a/drivers/input/keyboard/atkbd.c
+++ b/drivers/input/keyboard/atkbd.c
@@ -943,6 +943,13 @@ static unsigned int atkbd_samsung_forced_release_keys[] = {
};
/*
+ * Samsung 530U3C/530U4C/532U3C/540U3C Fn release keys not working
+ */
+static unsigned int atkbd_samsung_ultra_forced_release_keys[] = {
+ 0xa8, 0xa9, 0xb3, 0xce, 0xd5, -1U
+};
+
+/*
* Amilo Pi 3525 key release for Fn+Volume keys not working
*/
static unsigned int atkbd_amilo_pi3525_forced_release_keys[] = {
@@ -1732,6 +1739,15 @@ static const struct dmi_system_id atkbd_dmi_quirk_table[] __initconst = {
.driver_data = atkbd_samsung_forced_release_keys,
},
{
+ /* Samsung series 5 Ultra (530U3C/530U4C/532U3C/540U3C) */
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "SAMSUNG ELECTRONICS CO., LTD."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "530U3C"),
+ },
+ .callback = atkbd_setup_forced_release,
+ .driver_data = atkbd_samsung_ultra_forced_release_keys,
+ },
+ {
/* Fujitsu Amilo PA 1510 */
.matches = {
DMI_MATCH(DMI_SYS_VENDOR, "FUJITSU SIEMENS"),
Cheers,
Mauro
^ permalink raw reply related
* Re: [BUG?] EV_LED events not seperated from other events by SYN_REPORT
From: Dmitry Torokhov @ 2013-07-28 5:26 UTC (permalink / raw)
To: Rolf Morel; +Cc: linux-input
In-Reply-To: <CAFRyfCzE2_SE2YJuApUF35+s_Kt-P_tH1dwDwucJL3J_73w8=Q@mail.gmail.com>
Hi Rolf,
On Fri, Jul 26, 2013 at 04:41:11PM +0200, Rolf Morel wrote:
> Hello,
>
> I am having some trouble with how device LED switches are being
> presented in the, userspace API, event stream.
>
> If a program sets the LED using the evdev interface, write EV_LED
> input_event to device, other programs connected to the device will
> have the event in their stream, as expected. The problem is the
> received EV_LED events are not separated in time, as in moment in
> time. It does not generate a separate SYN_REPORT to make the EV_LED
> event a separate event in time.
>
> I have only two devices which have this functionality, set LED state
> through evdev, to test and they both are 3Dconnexion devices. They
> both exhibit the same behavior.
>
> So my questions are:
> Is this considered a bug (/should it be filed as a bug)?
> Does the fault lie in the input-subsystem or a more specific driver?
We have 2 options:
- emit EV_SYN from evedev_write() for every even written or
- require application writing events into /dev/eventX write EV_SYN as
well.
At the moment I am inclined to say that applications shoudl do this.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: Samsung series 5 ultra keyboard
From: Dmitry Torokhov @ 2013-07-28 5:22 UTC (permalink / raw)
To: Mauro Carvalho Chehab; +Cc: linux-input
In-Reply-To: <20130727150642.5a1f26cd@infradead.org>
Hi Mauro,
On Sat, Jul 27, 2013 at 03:06:42PM -0300, Mauro Carvalho Chehab wrote:
> Em Sat, 27 Jul 2013 10:11:53 -0300
> Mauro Carvalho Chehab <m.chehab@samsung.com> escreveu:
>
> > Hi Dmitry,
> >
> > I received a new notebook those days that has an extra feature of a "Fn Lock"
> > key.
> >
> > When this key is pressed, it produces a scan code 0xa8, and the direction
> > keys (and other keys) start to produce different codes. When pressed again,
> > it produces a scan code 0xa9, and the keys return to their normal behavior.
> >
> > The input layer doesn't currently produce any EV_KEY event for those two
> > scancodes. It produces just EV_MSC events (and, of course, EV_SYN).
> >
> > I was wandering that the better would be to have a LED indicator to
> > track this, and maybe have two new keycodes, like KEY_FN_LOCK_ON and
> > KEY_FN_LOCK_OFF.
> >
> > This way, some userspace program, like mate-lockkeys-applet could be
> > presenting not only the CAPS LOCK indicator, but also the FN LOCK
> > indicator.
> >
> > Do you think it would be doable?
Hmm, the behavior better matches a switch than a key, but we do have
quite a few key-like ON/OFF keys/switches so yes, we could add a couple
more.
> >
> > FYI, this is how this keyboard identifies itself:
> >
> > $ cat /sys/class/input/event3/device/uevent
> > PRODUCT=11/1/1/ab41
> > NAME="AT Translated Set 2 keyboard"
> > PHYS="isa0060/serio0/input0"
> > PROP=0
> > EV=120013
> > KEY=500f02000403 3803078f870d001 feffffdffbefffff fffffffffffffffe
> > MSC=10
> > LED=7
> > MODALIAS=input:b0011v0001p0001eAB41-e0,1,4,11,14,k71,72,73,74,75,76,77,79,7A,7B,7C,7D,7E,7F,80,8C,8E,8F,94,95,96,9B,9C,9D,9E,9F,A3,A4,A5,A6,AC,AD,B7,B8,B9,C0,C1,CA,D9,E0,E1,E2,E3,EC,EE,ram4,l0,1,2,sfw
> >
> > I'm not sure if the PRODUCT there is unique, and if it could be used
> > to add this kind of extra feature to the input subsystem (or if it would
> > be just fine to add those extra features at the standard AT keyboard
> > driver, although I personally don't like this idea, as other keyboards
> > might be using scancodes 0xa8/0xa9 for other meanings).
> >
> > What do you think?
>
> Just discovered something weird while trying to write some code for
> systemd/udev: there are a few keys that only produce key down events:
> Fn + F1 (KEY_SETUP)
> Fn + F12 (KEY_WLAN)
> Fn Lock (both scan codes 0xa8 and 0xa9)
That is not new, quite a few laptops do not bother with producing
release events. There is force_release attribute on serio ports bound to
atkbd and udev has code to manipulate it to ensure all keys are
released properly. You just need to add rule for your model.
Thanks,
Dmitry
--
Dmitry
^ permalink raw reply
* Re: [RESEND PATCH v5] Input: sysrq - DT binding for key sequence
From: Grant Likely @ 2013-07-28 4:52 UTC (permalink / raw)
To: robherring2, dmitry.torokhov
Cc: devicetree, john.stultz, linux-input, kernel-team,
mathieu.poirier
In-Reply-To: <1374705316-8520-1-git-send-email-mathieu.poirier@linaro.org>
On Wed, 24 Jul 2013 16:35:16 -0600, mathieu.poirier@linaro.org wrote:
> Sorry about the noise - device tree mailing has changed.
>
> I haven't heard back from anyone after making the last modifications
> a week ago. If it's all good with you Rob, please ack it so that
> Dmitry can pick it up on his side.
>
> Thanks,
> Mathieu.
>
> From 37318cb3c6339747f4e96e09f07f732a395a5ae1 Mon Sep 17 00:00:00 2001
> From: "Mathieu J. Poirier" <mathieu.poirier@linaro.org>
> Date: Wed, 10 Jul 2013 09:53:35 -0600
> Subject: [RESEND PATCH v5] Input: sysrq - DT binding for key sequence
>
> Adding a simple device tree binding for the specification of key sequences.
> Definition of the keys found in the sequence are located in
> 'include/uapi/linux/input.h'.
>
> For the sysrq driver, holding the sequence of keys down for a specific amount of time
> will reset the system.
>
> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> ---
> changes for v5:
> - Corrected error in binding definition.
> - Using helper macro to fetch the key sequence.
> - Removing white space.
> ---
> .../devicetree/bindings/input/input-reset.txt | 34 ++++++++++++++++++++
> drivers/tty/sysrq.c | 37 ++++++++++++++++++++++
> 2 files changed, 71 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/input/input-reset.txt
>
> diff --git a/Documentation/devicetree/bindings/input/input-reset.txt b/Documentation/devicetree/bindings/input/input-reset.txt
> new file mode 100644
> index 0000000..c69390c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/input-reset.txt
> @@ -0,0 +1,34 @@
> +Input: sysrq reset sequence
> +
> +A simple binding to represent a set of keys as described in
> +include/uapi/linux/input.h. This is to communicate a
> +sequence of keys to the sysrq driver. Upon holding the keys
> +for a specified amount of time (if specified) the system is
> +sync'ed and reset.
> +
> +Key sequences are global to the system but all the keys in a
> +set must be coming from the same input device.
> +
> +The /chosen node should contain a 'linux,sysrq-reset-seq' child
> +node to define a set of keys.
> +
> +Required property:
> +sysrq-reset-seq: array of keycodes
Please be more specific: "array of Linux keycodes, one keycode per cell"
> +
> +Optional property:
> +timeout-ms: duration keys must be pressed together in milliseconds
> +before generating a sysrq
What is the default behaviour if timeout-ms isn't present?
Otherwise, Acked-by: Grant Likely <grant.likely@linaro.org>
> +
> +Example:
> +
> + chosen {
> + linux,sysrq-reset-seq {
> + keyset = <0x03
> + 0x04
> + 0x0a>;
> + timeout-ms = <3000>;
> + };
> + };
> +
> +Would represent KEY_2, KEY_3 and KEY_9.
> +
> diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
> index b51c154..ed8f00f 100644
> --- a/drivers/tty/sysrq.c
> +++ b/drivers/tty/sysrq.c
> @@ -44,6 +44,7 @@
> #include <linux/uaccess.h>
> #include <linux/moduleparam.h>
> #include <linux/jiffies.h>
> +#include <linux/of.h>
>
> #include <asm/ptrace.h>
> #include <asm/irq_regs.h>
> @@ -671,6 +672,34 @@ static void sysrq_detect_reset_sequence(struct sysrq_state *state,
> }
> }
>
> +static void sysrq_of_get_keyreset_config(void)
> +{
> + u32 key;
> + struct device_node *np;
> + struct property *prop;
> + const __be32 *p;
> +
> + np = of_find_node_by_path("/chosen/linux,sysrq-reset-seq");
> + if (!np) {
> + pr_debug("No sysrq node found");
> + return;
> + }
> +
> + /* reset in case a __weak definition was present */
> + sysrq_reset_seq_len = 0;
> +
> + of_property_for_each_u32(np, "keyset", prop, p, key) {
> + if ((key == KEY_RESERVED || key > KEY_MAX) ||
> + (sysrq_reset_seq_len == SYSRQ_KEY_RESET_MAX))
> + break;
> +
> + sysrq_reset_seq[sysrq_reset_seq_len++] = (unsigned short)key;
> + }
> +
> + /* get reset timeout if any */
> + of_property_read_u32(np, "timeout-ms", &sysrq_reset_downtime_ms);
> +}
> +
> static void sysrq_reinject_alt_sysrq(struct work_struct *work)
> {
> struct sysrq_state *sysrq =
> @@ -903,6 +932,7 @@ static inline void sysrq_register_handler(void)
> int error;
> int i;
>
> + /* first check if a __weak interface was instantiated */
> for (i = 0; i < ARRAY_SIZE(sysrq_reset_seq); i++) {
> key = platform_sysrq_reset_seq[i];
> if (key == KEY_RESERVED || key > KEY_MAX)
> @@ -911,6 +941,13 @@ static inline void sysrq_register_handler(void)
> sysrq_reset_seq[sysrq_reset_seq_len++] = key;
> }
>
> + /*
> + * DT configuration takes precedence over anything
> + * that would have been defined via the __weak
> + * interface
> + */
> + sysrq_of_get_keyreset_config();
> +
> error = input_register_handler(&sysrq_handler);
> if (error)
> pr_err("Failed to register input handler, error %d", error);
> --
> 1.8.1.2
>
^ permalink raw reply
* Re: Samsung series 5 ultra keyboard
From: Mauro Carvalho Chehab @ 2013-07-27 18:06 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input
In-Reply-To: <20130727101153.6a3ece65@samsung.com>
Em Sat, 27 Jul 2013 10:11:53 -0300
Mauro Carvalho Chehab <m.chehab@samsung.com> escreveu:
> Hi Dmitry,
>
> I received a new notebook those days that has an extra feature of a "Fn Lock"
> key.
>
> When this key is pressed, it produces a scan code 0xa8, and the direction
> keys (and other keys) start to produce different codes. When pressed again,
> it produces a scan code 0xa9, and the keys return to their normal behavior.
>
> The input layer doesn't currently produce any EV_KEY event for those two
> scancodes. It produces just EV_MSC events (and, of course, EV_SYN).
>
> I was wandering that the better would be to have a LED indicator to
> track this, and maybe have two new keycodes, like KEY_FN_LOCK_ON and
> KEY_FN_LOCK_OFF.
>
> This way, some userspace program, like mate-lockkeys-applet could be
> presenting not only the CAPS LOCK indicator, but also the FN LOCK
> indicator.
>
> Do you think it would be doable?
>
> FYI, this is how this keyboard identifies itself:
>
> $ cat /sys/class/input/event3/device/uevent
> PRODUCT=11/1/1/ab41
> NAME="AT Translated Set 2 keyboard"
> PHYS="isa0060/serio0/input0"
> PROP=0
> EV=120013
> KEY=500f02000403 3803078f870d001 feffffdffbefffff fffffffffffffffe
> MSC=10
> LED=7
> MODALIAS=input:b0011v0001p0001eAB41-e0,1,4,11,14,k71,72,73,74,75,76,77,79,7A,7B,7C,7D,7E,7F,80,8C,8E,8F,94,95,96,9B,9C,9D,9E,9F,A3,A4,A5,A6,AC,AD,B7,B8,B9,C0,C1,CA,D9,E0,E1,E2,E3,EC,EE,ram4,l0,1,2,sfw
>
> I'm not sure if the PRODUCT there is unique, and if it could be used
> to add this kind of extra feature to the input subsystem (or if it would
> be just fine to add those extra features at the standard AT keyboard
> driver, although I personally don't like this idea, as other keyboards
> might be using scancodes 0xa8/0xa9 for other meanings).
>
> What do you think?
Just discovered something weird while trying to write some code for
systemd/udev: there are a few keys that only produce key down events:
Fn + F1 (KEY_SETUP)
Fn + F12 (KEY_WLAN)
Fn Lock (both scan codes 0xa8 and 0xa9)
See:
# ir-keytable -d /dev/input/event3 -t
Testing events. Please, press CTRL-C to abort.
1374948216.879719: event type EV_MSC(0x04): scancode = 0xce
1374948216.879719: event type EV_KEY(0x01) key_down: KEY_SETUP(0x0001)
1374948216.879719: event type EV_SYN(0x00).
1374948218.420177: event type EV_MSC(0x04): scancode = 0xce
1374948218.420177: event type EV_KEY(0x01) key_down: KEY_SETUP(0x0001)
1374948218.420177: event type EV_SYN(0x00).
1374948219.362185: event type EV_MSC(0x04): scancode = 0x89
1374948219.362185: event type EV_KEY(0x01) key_down: KEY_BRIGHTNESSDOWN(0x0001)
1374948219.362185: event type EV_SYN(0x00).
1374948219.362218: event type EV_KEY(0x01) key_up: KEY_BRIGHTNESSDOWN(0x0001)
1374948219.362218: event type EV_SYN(0x00).
1374948222.746160: event type EV_MSC(0x04): scancode = 0xce
1374948222.746160: event type EV_KEY(0x01) key_down: KEY_SETUP(0x0001)
1374948222.746160: event type EV_SYN(0x00).
1374948223.348321: event type EV_MSC(0x04): scancode = 0x89
1374948223.348321: event type EV_KEY(0x01) key_down: KEY_BRIGHTNESSDOWN(0x0001)
1374948223.348321: event type EV_SYN(0x00).
1374948223.348356: event type EV_KEY(0x01) key_up: KEY_BRIGHTNESSDOWN(0x0001)
1374948223.348356: event type EV_SYN(0x00).
1374948224.864993: event type EV_MSC(0x04): scancode = 0xce
1374948224.864993: event type EV_KEY(0x01) key_down: KEY_SETUP(0x0001)
1374948224.864993: event type EV_SYN(0x00).
1374948227.762895: event type EV_MSC(0x04): scancode = 0x82
1374948227.762895: event type EV_KEY(0x01) key_down: KEY_SWITCHVIDEOMODE(0x0001)
1374948227.762895: event type EV_SYN(0x00).
1374948227.762930: event type EV_KEY(0x01) key_up: KEY_SWITCHVIDEOMODE(0x0001)
1374948227.762930: event type EV_SYN(0x00).
1374948229.212478: event type EV_MSC(0x04): scancode = 0x82
1374948229.212478: event type EV_KEY(0x01) key_down: KEY_SWITCHVIDEOMODE(0x0001)
1374948229.212478: event type EV_SYN(0x00).
1374948229.212513: event type EV_KEY(0x01) key_up: KEY_SWITCHVIDEOMODE(0x0001)
1374948229.212513: event type EV_SYN(0x00).
1374948232.961500: event type EV_MSC(0x04): scancode = 0xd5
1374948232.961500: event type EV_KEY(0x01) key_down: KEY_WLAN(0x0001)
1374948232.961500: event type EV_SYN(0x00).
1374948234.377876: event type EV_MSC(0x04): scancode = 0xd5
1374948234.377876: event type EV_KEY(0x01) key_down: KEY_WLAN(0x0001)
1374948234.377876: event type EV_SYN(0x00).
1374948235.090077: event type EV_MSC(0x04): scancode = 0xa8
1374948235.090077: event type EV_KEY(0x01) key_down: KEY_PROG1(0x0001)
1374948235.090077: event type EV_SYN(0x00).
1374948236.650001: event type EV_MSC(0x04): scancode = 0xa9
1374948236.650001: event type EV_KEY(0x01) key_down: KEY_PROG2(0x0001)
1374948236.650001: event type EV_SYN(0x00).
1374948237.094992: event type EV_MSC(0x04): scancode = 0xb7
1374948237.094992: event type EV_KEY(0x01) key_down: KEY_SYSRQ(0x0001)
1374948237.094992: event type EV_SYN(0x00).
1374948237.213233: event type EV_MSC(0x04): scancode = 0xb7
1374948237.213233: event type EV_KEY(0x01) key_up: KEY_SYSRQ(0x0001)
1374948237.213233: event type EV_SYN(0x00).
1374948247.126649: event type EV_MSC(0x04): scancode = 0xb7
1374948247.126649: event type EV_KEY(0x01) key_down: KEY_SYSRQ(0x0001)
1374948247.126649: event type EV_SYN(0x00).
1374948247.299130: event type EV_MSC(0x04): scancode = 0xb7
1374948247.299130: event type EV_KEY(0x01) key_up: KEY_SYSRQ(0x0001)
1374948247.299130: event type EV_SYN(0x00).
1374948249.158131: event type EV_MSC(0x04): scancode = 0xb7
1374948249.158131: event type EV_KEY(0x01) key_down: KEY_SYSRQ(0x0001)
1374948249.158131: event type EV_SYN(0x00).
1374948249.294389: event type EV_MSC(0x04): scancode = 0xb7
1374948249.294389: event type EV_KEY(0x01) key_up: KEY_SYSRQ(0x0001)
1374948249.294389: event type EV_SYN(0x00).
1374948250.904246: event type EV_MSC(0x04): scancode = 0xa8
1374948250.904246: event type EV_KEY(0x01) key_down: KEY_PROG1(0x0001)
1374948250.904246: event type EV_SYN(0x00).
1374948252.938739: event type EV_MSC(0x04): scancode = 0xa9
1374948252.938739: event type EV_KEY(0x01) key_down: KEY_PROG2(0x0001)
1374948252.938739: event type EV_SYN(0x00).
So, perhaps some quirk will be needed at the Kernel driver to artificially
produce KEYUP events for those scancodes.
Cheers,
Mauro
^ permalink raw reply
* [PATCH 0/1] hid: Add new driver for non-compliant Xin-Mo devices.
From: oscherler @ 2013-07-27 17:20 UTC (permalink / raw)
To: jkosina, linux-kernel, linux-input; +Cc: Olivier Scherler
From: Olivier Scherler <oscherler@ithink.ch>
The Xin-Mo Dual Arcade Controller sends axis values that are out of
range with respect to the HID report descriptor. This patch adds a
driver to correct the input values so they are not thrown out by the
range check in hid-input.c.
The issue is discussed here: <https://bugs.launchpad.net/bugs/1077359>
checkpatch.pl gives warnings for two lines that are over 80 characters,
but I decided not to change them since similar lines in other HID
drivers. The third warning asks for a paragraph that describes the new
config symbol in Kconfig fully. I will gladly accept advice on how to
improve it.
This has been tested on Ubuntu Desktop 13.04 in a VMware VM and on a
Raspberry Pi running Raspbian.
Olivier Scherler (1):
hid: Add new driver for non-compliant Xin-Mo devices.
drivers/hid/Kconfig | 8 ++++++
drivers/hid/Makefile | 1 +
drivers/hid/hid-core.c | 1 +
drivers/hid/hid-ids.h | 3 +++
drivers/hid/hid-xinmo.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++++
5 files changed, 85 insertions(+)
create mode 100644 drivers/hid/hid-xinmo.c
--
1.8.1.2
^ permalink raw reply
* [PATCH] hid: Add new driver for non-compliant Xin-Mo devices.
From: oscherler @ 2013-07-27 17:20 UTC (permalink / raw)
To: jkosina, linux-kernel, linux-input; +Cc: Olivier Scherler
In-Reply-To: <1374945602-11894-1-git-send-email-oscherler@ithink.ch>
From: Olivier Scherler <oscherler@ithink.ch>
The driver currently only supports the Dual Arcade controller.
It fixes the negative axis event values (the devices sends -2) to match the
logical axis minimum of the HID report descriptor (the report announces -1).
It is needed because hid-input discards out of bounds values.
Signed-off-by: Olivier Scherler <oscherler@ithink.ch>
---
drivers/hid/Kconfig | 8 ++++++
drivers/hid/Makefile | 1 +
drivers/hid/hid-core.c | 1 +
drivers/hid/hid-ids.h | 3 +++
drivers/hid/hid-xinmo.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++++
5 files changed, 85 insertions(+)
create mode 100644 drivers/hid/hid-xinmo.c
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 14ef6ab..3d7c9f6 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -743,6 +743,14 @@ config HID_WIIMOTE
To compile this driver as a module, choose M here: the
module will be called hid-wiimote.
+config HID_XINMO
+ tristate "Xin-Mo non-fully compliant devices"
+ depends on HID
+ ---help---
+ Support for Xin-Mo devices that are not fully compliant with the HID
+ standard. Currently only supports the Xin-Mo Dual Arcade. Say Y here
+ if you have a Xin-Mo Dual Arcade controller.
+
config HID_ZEROPLUS
tristate "Zeroplus based game controller support"
depends on HID
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index 6f68728..a959f4a 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -110,6 +110,7 @@ obj-$(CONFIG_HID_TIVO) += hid-tivo.o
obj-$(CONFIG_HID_TOPSEED) += hid-topseed.o
obj-$(CONFIG_HID_TWINHAN) += hid-twinhan.o
obj-$(CONFIG_HID_UCLOGIC) += hid-uclogic.o
+obj-$(CONFIG_HID_XINMO) += hid-xinmo.o
obj-$(CONFIG_HID_ZEROPLUS) += hid-zpff.o
obj-$(CONFIG_HID_ZYDACRON) += hid-zydacron.o
obj-$(CONFIG_HID_WACOM) += hid-wacom.o
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 36668d1..283c70f 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1736,6 +1736,7 @@ static const struct hid_device_id hid_have_special_driver[] = {
{ HID_USB_DEVICE(USB_VENDOR_ID_WALTOP, USB_DEVICE_ID_WALTOP_MEDIA_TABLET_14_1_INCH) },
{ HID_USB_DEVICE(USB_VENDOR_ID_WALTOP, USB_DEVICE_ID_WALTOP_SIRIUS_BATTERY_FREE_TABLET) },
{ HID_USB_DEVICE(USB_VENDOR_ID_X_TENSIONS, USB_DEVICE_ID_SPEEDLINK_VAD_CEZANNE) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_XIN_MO, USB_DEVICE_ID_XIN_MO_DUAL_ARCADE) },
{ HID_USB_DEVICE(USB_VENDOR_ID_ZEROPLUS, 0x0005) },
{ HID_USB_DEVICE(USB_VENDOR_ID_ZEROPLUS, 0x0030) },
{ HID_USB_DEVICE(USB_VENDOR_ID_ZYDACRON, USB_DEVICE_ID_ZYDACRON_REMOTE_CONTROL) },
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index ffe4c7a..6051be8 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -887,6 +887,9 @@
#define USB_VENDOR_ID_XAT 0x2505
#define USB_DEVICE_ID_XAT_CSR 0x0220
+#define USB_VENDOR_ID_XIN_MO 0x16c0
+#define USB_DEVICE_ID_XIN_MO_DUAL_ARCADE 0x05e1
+
#define USB_VENDOR_ID_XIROKU 0x1477
#define USB_DEVICE_ID_XIROKU_SPX 0x1006
#define USB_DEVICE_ID_XIROKU_MPX 0x1007
diff --git a/drivers/hid/hid-xinmo.c b/drivers/hid/hid-xinmo.c
new file mode 100644
index 0000000..6153e50
--- /dev/null
+++ b/drivers/hid/hid-xinmo.c
@@ -0,0 +1,72 @@
+/*
+ * HID driver for Xin-Mo devices, currently only the Dual Arcade controller.
+ * Fixes the negative axis event values (the devices sends -2) to match the
+ * logical axis minimum of the HID report descriptor (the report announces
+ * -1). It is needed because hid-input discards out of bounds values.
+ * (This module is based on "hid-saitek" and "hid-lg".)
+ *
+ * Copyright (c) 2013 Olivier Scherler
+ */
+
+/*
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the Free
+ * Software Foundation; either version 2 of the License, or (at your option)
+ * any later version.
+ */
+
+#include <linux/device.h>
+#include <linux/hid.h>
+#include <linux/module.h>
+#include <linux/kernel.h>
+
+#include "hid-ids.h"
+
+/*
+ * Fix negative events that are out of bounds.
+ */
+static int xinmo_event(struct hid_device *hdev, struct hid_field *field,
+ struct hid_usage *usage, __s32 value)
+{
+ switch (usage->code) {
+ case ABS_X:
+ case ABS_Y:
+ case ABS_Z:
+ case ABS_RX:
+ if (value < -1) {
+ input_event(field->hidinput->input, usage->type,
+ usage->code, -1);
+ return 1;
+ }
+ break;
+ }
+
+ return 0;
+}
+
+static const struct hid_device_id xinmo_devices[] = {
+ { HID_USB_DEVICE(USB_VENDOR_ID_XIN_MO, USB_DEVICE_ID_XIN_MO_DUAL_ARCADE) },
+ { }
+};
+
+MODULE_DEVICE_TABLE(hid, xinmo_devices);
+
+static struct hid_driver xinmo_driver = {
+ .name = "xinmo",
+ .id_table = xinmo_devices,
+ .event = xinmo_event
+};
+
+static int __init xinmo_init(void)
+{
+ return hid_register_driver(&xinmo_driver);
+}
+
+static void __exit xinmo_exit(void)
+{
+ hid_unregister_driver(&xinmo_driver);
+}
+
+module_init(xinmo_init);
+module_exit(xinmo_exit);
+MODULE_LICENSE("GPL");
--
1.8.1.2
^ permalink raw reply related
* Re: [PATCH v2 3/3] Input: omap-keypad: Set irq to level instead of edge
From: Felipe Balbi @ 2013-07-27 17:07 UTC (permalink / raw)
To: Illia Smyrnov; +Cc: Dmitry Torokhov, linux-input, linux-kernel, linux-omap
In-Reply-To: <1374858546-25034-4-git-send-email-illia.smyrnov@ti.com>
[-- Attachment #1: Type: text/plain, Size: 1424 bytes --]
Hi,
On Fri, Jul 26, 2013 at 08:09:06PM +0300, Illia Smyrnov wrote:
> From: Axel Haslam <axelhaslam@ti.com>
>
> Set mpu irq to level instead of edge, since if mpu is in low power
> an edge detection may be lost if the event is a wkup event.
>
> Signed-off-by: Axel Haslam <axelhaslam@ti.com>
> Signed-off-by: Illia Smyrnov <illia.smyrnov@ti.com>
> ---
> Based on top of v3.11-rc2
>
> Depends on:
> Input: omap-keypad: Convert to threaded IRQ
> https://patchwork.kernel.org/patch/2832920/
>
> drivers/input/keyboard/omap4-keypad.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/input/keyboard/omap4-keypad.c b/drivers/input/keyboard/omap4-keypad.c
> index e8bdc76..7adb53e 100644
> --- a/drivers/input/keyboard/omap4-keypad.c
> +++ b/drivers/input/keyboard/omap4-keypad.c
> @@ -374,9 +374,13 @@ static int omap4_keypad_probe(struct platform_device *pdev)
> goto err_free_keymap;
> }
>
> + /*
> + * Set irq level detection for mpu. Edge event are missed in gic
> + * if the mpu is in low power and keypad event is a wakeup
> + */
> error = request_threaded_irq(keypad_data->irq, omap4_keypad_irq_handler,
> omap4_keypad_irq_thread_fn,
> - IRQF_TRIGGER_RISING,
> + IRQF_TRIGGER_HIGH,
omap4 is DT-only, why don't you pass zero here and expect DT to setup
IRQ type correctly for you ?
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Samsung series 5 ultra keyboard
From: Mauro Carvalho Chehab @ 2013-07-27 13:11 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, Mauro Carvalho Chehab
Hi Dmitry,
I received a new notebook those days that has an extra feature of a "Fn Lock"
key.
When this key is pressed, it produces a scan code 0xa8, and the direction
keys (and other keys) start to produce different codes. When pressed again,
it produces a scan code 0xa9, and the keys return to their normal behavior.
The input layer doesn't currently produce any EV_KEY event for those two
scancodes. It produces just EV_MSC events (and, of course, EV_SYN).
I was wandering that the better would be to have a LED indicator to
track this, and maybe have two new keycodes, like KEY_FN_LOCK_ON and
KEY_FN_LOCK_OFF.
This way, some userspace program, like mate-lockkeys-applet could be
presenting not only the CAPS LOCK indicator, but also the FN LOCK
indicator.
Do you think it would be doable?
FYI, this is how this keyboard identifies itself:
$ cat /sys/class/input/event3/device/uevent
PRODUCT=11/1/1/ab41
NAME="AT Translated Set 2 keyboard"
PHYS="isa0060/serio0/input0"
PROP=0
EV=120013
KEY=500f02000403 3803078f870d001 feffffdffbefffff fffffffffffffffe
MSC=10
LED=7
MODALIAS=input:b0011v0001p0001eAB41-e0,1,4,11,14,k71,72,73,74,75,76,77,79,7A,7B,7C,7D,7E,7F,80,8C,8E,8F,94,95,96,9B,9C,9D,9E,9F,A3,A4,A5,A6,AC,AD,B7,B8,B9,C0,C1,CA,D9,E0,E1,E2,E3,EC,EE,ram4,l0,1,2,sfw
I'm not sure if the PRODUCT there is unique, and if it could be used
to add this kind of extra feature to the input subsystem (or if it would
be just fine to add those extra features at the standard AT keyboard
driver, although I personally don't like this idea, as other keyboards
might be using scancodes 0xa8/0xa9 for other meanings).
What do you think?
Thanks!
Mauro
^ permalink raw reply
* Re: [PATCH v2 2/2] input/uinput: add UI_GET_SYSPATH ioctl to retrieve the sysfs path
From: David Herrmann @ 2013-07-27 13:07 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Dmitry Torokhov, Benjamin Tissoires, Peter Hutterer,
open list:HID CORE LAYER, linux-kernel
In-Reply-To: <1373895429-8846-2-git-send-email-benjamin.tissoires@redhat.com>
Hi
On Mon, Jul 15, 2013 at 3:37 PM, Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
> Evemu [1] uses uinput to replay devices traces it has recorded. However,
> the way evemu uses uinput is slightly different from how uinput is
> supposed to be used.
> Evemu creates the device node through uinput, bu inject events through
> the input device node directly (and skipping the uinput node).
>
> Currently, evemu relies on an heuristic to guess which input node was
> created. The problem is that is heuristic is subjected to races between
> different uinput devices or even with physical devices. Having a way
> to retrieve the sysfs path allows us to find the event node without
> having to rely on this heuristic.
It would actually be enough to return the "input_no" from
input_register_device() (which is currently local but we could save it
in "dev"). Or only the device-name. I don't know why you want the full
syspath. It's just overhead in the kernel that we could easily let
user-space do. And the path /sys/class/input/<devname> can be put
together by user-space.
Anyway, Dmitry has to decide on that. Apart from some style-issues I
mentioned below:
Reviewed-by: David Herrmann <dh.herrmann@gmail.com>
Cheers
David
> [1] http://www.freedesktop.org/wiki/Evemu/
>
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> ---
> drivers/input/misc/uinput.c | 37 ++++++++++++++++++++++++++++++++++++-
> include/linux/uinput.h | 1 +
> include/uapi/linux/uinput.h | 3 +++
> 3 files changed, 40 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c
> index 7d518b4..49a9f7d 100644
> --- a/drivers/input/misc/uinput.c
> +++ b/drivers/input/misc/uinput.c
> @@ -22,6 +22,7 @@
> * Changes/Revisions:
> * 0.4 12/07/2013 (Peter Hutterer <peter.hutterer@redhat.com>)
> * - update uinput_user_dev struct to allow abs resolution
> + * - add UI_GET_SYSPATH ioctl
What tree is that patch against? I cannot see an "0.4" entry in:
http://git.kernel.org/cgit/linux/kernel/git/dtor/input.git/tree/drivers/input/misc/uinput.c?h=next
> * 0.3 09/04/2006 (Anssi Hannula <anssi.hannula@gmail.com>)
> * - updated ff support for the changes in kernel interface
> * - added MODULE_VERSION
> @@ -667,6 +668,21 @@ static int uinput_ff_upload_from_user(const char __user *buffer,
> __ret; \
> })
>
> +static int uinput_str_to_user(const char *str, unsigned int maxlen,
> + void __user *p)
As Peter mentioned, I'd move "maxlen" to the end.
> +{
> + int len;
> +
> + if (!str)
> + return -ENOENT;
> +
> + len = strlen(str) + 1;
> + if (len > maxlen)
> + len = maxlen;
> +
> + return copy_to_user(p, str, len) ? -EFAULT : len;
I'd prefer a "strlcpy()" so we guarantee a terminating 0 for
user-space, but I guess that'd be rather complex to do here. I
couldn't find any strlcpy_to_user()...
> +}
> +
> static long uinput_ioctl_handler(struct file *file, unsigned int cmd,
> unsigned long arg, void __user *p)
> {
> @@ -676,6 +692,8 @@ static long uinput_ioctl_handler(struct file *file, unsigned int cmd,
> struct uinput_ff_erase ff_erase;
> struct uinput_request *req;
> char *phys;
> + const char *path;
> + unsigned int size;
>
> retval = mutex_lock_interruptible(&udev->mutex);
> if (retval)
> @@ -828,7 +846,24 @@ static long uinput_ioctl_handler(struct file *file, unsigned int cmd,
> break;
>
> default:
> - retval = -EINVAL;
> + retval = -EAGAIN;
> + }
> +
> + if (retval == -EAGAIN) {
> + size = _IOC_SIZE(cmd);
> +
> + /* Now check variable-length commands */
> + switch (cmd & ~IOCSIZE_MASK) {
> + case UI_GET_SYSPATH(0):
> + path = kobject_get_path(&udev->dev->dev.kobj,
> + GFP_KERNEL);
I know, device registration is protected by udev->mutex but I'd still prefer:
if (udev->state != UIST_CREATED)
return -ENOENT;
But that's probably a matter of taste.
> + retval = uinput_str_to_user(path, size, p);
> + kfree(path);
> + break;
> +
> + default:
> + retval = -EINVAL;
> + }
> }
>
> out:
> diff --git a/include/linux/uinput.h b/include/linux/uinput.h
> index 6291a22..64fab81 100644
> --- a/include/linux/uinput.h
> +++ b/include/linux/uinput.h
> @@ -22,6 +22,7 @@
> * Changes/Revisions:
> * 0.4 12/07/2013 (Peter Hutterer <peter.hutterer@redhat.com>)
> * - update uinput_user_dev struct to allow abs resolution
> + * - add UI_GET_SYSPATH ioctl
> * 0.3 24/05/2006 (Anssi Hannula <anssi.hannulagmail.com>)
> * - update ff support for the changes in kernel interface
> * - add UINPUT_VERSION
> diff --git a/include/uapi/linux/uinput.h b/include/uapi/linux/uinput.h
> index f6a393b..d826409 100644
> --- a/include/uapi/linux/uinput.h
> +++ b/include/uapi/linux/uinput.h
> @@ -22,6 +22,7 @@
> * Changes/Revisions:
> * 0.4 12/07/2013 (Peter Hutterer <peter.hutterer@redhat.com>)
> * - update uinput_user_dev struct to allow abs resolution
> + * - add UI_GET_SYSPATH ioctl
> * 0.3 24/05/2006 (Anssi Hannula <anssi.hannulagmail.com>)
> * - update ff support for the changes in kernel interface
> * - add UINPUT_VERSION
> @@ -75,6 +76,8 @@ struct uinput_ff_erase {
> #define UI_BEGIN_FF_ERASE _IOWR(UINPUT_IOCTL_BASE, 202, struct uinput_ff_erase)
> #define UI_END_FF_ERASE _IOW(UINPUT_IOCTL_BASE, 203, struct uinput_ff_erase)
>
> +#define UI_GET_SYSPATH(len) _IOC(_IOC_READ, UINPUT_IOCTL_BASE, 300, len)
> +
> /*
> * To write a force-feedback-capable driver, the upload_effect
> * and erase_effect callbacks in input_dev must be implemented.
> --
> 1.8.3.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply
* Re: [PATCH 35/50] media: usb: cx231xx: spin_lock in complete() cleanup
From: Ming Lei @ 2013-07-27 9:34 UTC (permalink / raw)
To: Hans Verkuil
Cc: Greg Kroah-Hartman, linux-usb, Oliver Neukum, Alan Stern,
linux-input, linux-bluetooth, netdev, linux-wireless, linux-media,
alsa-devel, Mauro Carvalho Chehab, Hans Verkuil
In-Reply-To: <51F2878E.90705@xs4all.nl>
On Fri, Jul 26, 2013 at 10:28 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
>
>
> On 07/11/2013 11:05 AM, Ming Lei wrote:
>> Complete() will be run with interrupt enabled, so change to
>> spin_lock_irqsave().
>>
>> Cc: Mauro Carvalho Chehab <mchehab@redhat.com>
>> Cc: Hans Verkuil <hans.verkuil@cisco.com>
>> Cc: linux-media@vger.kernel.org
>> Signed-off-by: Ming Lei <ming.lei@canonical.com>
>> ---
>> drivers/media/usb/cx231xx/cx231xx-audio.c | 6 ++++++
>> drivers/media/usb/cx231xx/cx231xx-core.c | 10 ++++++----
>> drivers/media/usb/cx231xx/cx231xx-vbi.c | 5 +++--
>> 3 files changed, 15 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/media/usb/cx231xx/cx231xx-audio.c b/drivers/media/usb/cx231xx/cx231xx-audio.c
>> index 81a1d97..58c1b5c 100644
>> --- a/drivers/media/usb/cx231xx/cx231xx-audio.c
>> +++ b/drivers/media/usb/cx231xx/cx231xx-audio.c
>> @@ -136,6 +136,7 @@ static void cx231xx_audio_isocirq(struct urb *urb)
>> stride = runtime->frame_bits >> 3;
>>
>> for (i = 0; i < urb->number_of_packets; i++) {
>> + unsigned long flags;
>> int length = urb->iso_frame_desc[i].actual_length /
>> stride;
>> cp = (unsigned char *)urb->transfer_buffer +
>> @@ -158,6 +159,7 @@ static void cx231xx_audio_isocirq(struct urb *urb)
>> length * stride);
>> }
>>
>> + local_irq_save(flags);
>> snd_pcm_stream_lock(substream);
>
> Can't you use snd_pcm_stream_lock_irqsave here?
Sure, that is already in my mind, :-)
> Ditto for the other media drivers where this happens: em28xx and tlg2300.
Yes.
>
> I've reviewed the media driver changes and they look OK to me, so if
> my comment above is fixed, then I can merge them for 3.12. Or are these
> changes required for 3.11?
These are for 3.12.
I will send out v2 next week, and thanks for your review.
Thanks,
--
Ming Lei
^ permalink raw reply
* [PATCH 2/2] iio: ti_am335x_adc: Add continuous sampling and trigger support
From: Zubair Lutfullah @ 2013-07-26 23:51 UTC (permalink / raw)
To: jic23; +Cc: linux-iio, linux-kernel, linux-input, gregkh, Russ.Dill
In-Reply-To: <1374882674-18403-1-git-send-email-zubair.lutfullah@gmail.com>
Previously the driver had only one-shot reading functionality.
This patch adds triggered buffer support to the driver.
A buffer of samples can now be read via /dev/iio.
Patil Rachna (TI) laid the ground work for ADC HW register access.
Russ Dill (TI) fixed bugs in the driver relevant to FIFOs and IRQs.
I fixed channel scanning so multiple ADC channels can be read
simultaneously and pushed to userspace.
Restructured the driver to fit IIO ABI.
And added trigger support.
Signed-off-by: Zubair Lutfullah <zubair.lutfullah@gmail.com>
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Russ Dill <Russ.Dill@ti.com>
---
drivers/iio/adc/ti_am335x_adc.c | 334 +++++++++++++++++++++++++++-------
include/linux/mfd/ti_am335x_tscadc.h | 13 +-
2 files changed, 285 insertions(+), 62 deletions(-)
diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c
index 3ceac3e..630ce85 100644
--- a/drivers/iio/adc/ti_am335x_adc.c
+++ b/drivers/iio/adc/ti_am335x_adc.c
@@ -26,14 +26,25 @@
#include <linux/of_device.h>
#include <linux/iio/machine.h>
#include <linux/iio/driver.h>
-
#include <linux/mfd/ti_am335x_tscadc.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/trigger.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/iio/triggered_buffer.h>
+#include <linux/wait.h>
+#include <linux/sched.h>
struct tiadc_device {
struct ti_tscadc_dev *mfd_tscadc;
int channels;
u8 channel_line[8];
u8 channel_step[8];
+ struct work_struct poll_work;
+ wait_queue_head_t wq_data_avail;
+ bool data_avail;
+ u32 *inputbuffer;
+ int sample_count;
+ int irq;
};
static unsigned int tiadc_readl(struct tiadc_device *adc, unsigned int reg)
@@ -56,27 +67,28 @@ static u32 get_adc_step_mask(struct tiadc_device *adc_dev)
return step_en;
}
-static void tiadc_step_config(struct tiadc_device *adc_dev)
+static void tiadc_step_config(struct iio_dev *indio_dev)
{
+ struct tiadc_device *adc_dev = iio_priv(indio_dev);
unsigned int stepconfig;
- int i, steps;
+ int i, steps, chan;
/*
* There are 16 configurable steps and 8 analog input
* lines available which are shared between Touchscreen and ADC.
- *
* Steps backwards i.e. from 16 towards 0 are used by ADC
* depending on number of input lines needed.
* Channel would represent which analog input
* needs to be given to ADC to digitalize data.
*/
-
steps = TOTAL_STEPS - adc_dev->channels;
- stepconfig = STEPCONFIG_AVG_16 | STEPCONFIG_FIFO1;
+ if (iio_buffer_enabled(indio_dev))
+ stepconfig = STEPCONFIG_AVG_16 | STEPCONFIG_FIFO1
+ | STEPCONFIG_MODE_SWCNT;
+ else
+ stepconfig = STEPCONFIG_AVG_16 | STEPCONFIG_FIFO1;
for (i = 0; i < adc_dev->channels; i++) {
- int chan;
-
chan = adc_dev->channel_line[i];
tiadc_writel(adc_dev, REG_STEPCONFIG(steps),
stepconfig | STEPCONFIG_INP(chan));
@@ -85,7 +97,190 @@ static void tiadc_step_config(struct tiadc_device *adc_dev)
adc_dev->channel_step[i] = steps;
steps++;
}
+}
+
+static irqreturn_t tiadc_irq(int irq, void *private)
+{
+ struct iio_dev *idev = private;
+ struct tiadc_device *adc_dev = iio_priv(idev);
+ unsigned int status, config;
+ status = tiadc_readl(adc_dev, REG_IRQSTATUS);
+
+ /* FIFO Overrun. Clear flag. Disable/Enable ADC to recover */
+ if (status & IRQENB_FIFO1OVRRUN) {
+ config = tiadc_readl(adc_dev, REG_CTRL);
+ config &= ~(CNTRLREG_TSCSSENB);
+ tiadc_writel(adc_dev, REG_CTRL, config);
+ tiadc_writel(adc_dev, REG_IRQSTATUS, IRQENB_FIFO1OVRRUN |
+ IRQENB_FIFO1UNDRFLW | IRQENB_FIFO1THRES);
+ tiadc_writel(adc_dev, REG_CTRL, (config | CNTRLREG_TSCSSENB));
+ return IRQ_HANDLED;
+ } else if (status & IRQENB_FIFO1THRES) {
+ /* Wake adc_work that pushes FIFO data to iio buffer */
+ tiadc_writel(adc_dev, REG_IRQCLR, IRQENB_FIFO1THRES);
+ adc_dev->data_avail = 1;
+ wake_up_interruptible(&adc_dev->wq_data_avail);
+ return IRQ_HANDLED;
+ } else
+ return IRQ_NONE;
+}
+
+static irqreturn_t tiadc_trigger_h(int irq, void *p)
+{
+ struct iio_poll_func *pf = p;
+ struct iio_dev *indio_dev = pf->indio_dev;
+ struct tiadc_device *adc_dev = iio_priv(indio_dev);
+ unsigned int config;
+
+ schedule_work(&adc_dev->poll_work);
+ config = tiadc_readl(adc_dev, REG_CTRL);
+ tiadc_writel(adc_dev, REG_CTRL, config & ~CNTRLREG_TSCSSENB);
+ tiadc_writel(adc_dev, REG_CTRL, config | CNTRLREG_TSCSSENB);
+
+ tiadc_writel(adc_dev, REG_IRQSTATUS, IRQENB_FIFO1THRES |
+ IRQENB_FIFO1OVRRUN | IRQENB_FIFO1UNDRFLW);
+ tiadc_writel(adc_dev, REG_IRQENABLE, IRQENB_FIFO1THRES
+ | IRQENB_FIFO1OVRRUN);
+
+ iio_trigger_notify_done(indio_dev->trig);
+ return IRQ_HANDLED;
+}
+
+static int tiadc_buffer_preenable(struct iio_dev *indio_dev)
+{
+ return iio_sw_buffer_preenable(indio_dev);
+}
+
+static int tiadc_buffer_postenable(struct iio_dev *indio_dev)
+{
+ struct tiadc_device *adc_dev = iio_priv(indio_dev);
+ struct iio_buffer *buffer = indio_dev->buffer;
+ unsigned int enb, stepnum;
+ u8 bit;
+
+ tiadc_step_config(indio_dev);
+ tiadc_writel(adc_dev, REG_SE, 0x00);
+ for_each_set_bit(bit, buffer->scan_mask,
+ adc_dev->channels) {
+ struct iio_chan_spec const *chan = indio_dev->channels + bit;
+ /*
+ * There are a total of 16 steps available
+ * that are shared between ADC and touchscreen.
+ * We start configuring from step 16 to 0 incase of
+ * ADC. Hence the relation between input channel
+ * and step for ADC would be as below.
+ */
+ stepnum = chan->channel + 9;
+ enb = tiadc_readl(adc_dev, REG_SE);
+ enb |= (1 << stepnum);
+ tiadc_writel(adc_dev, REG_SE, enb);
+ }
+
+ return iio_triggered_buffer_postenable(indio_dev);
+}
+
+static int tiadc_buffer_predisable(struct iio_dev *indio_dev)
+{
+ struct tiadc_device *adc_dev = iio_priv(indio_dev);
+ int fifo1count, i, read, config;
+
+ config = tiadc_readl(adc_dev, REG_CTRL);
+ config &= ~(CNTRLREG_TSCSSENB);
+ tiadc_writel(adc_dev, REG_CTRL, config);
+ tiadc_writel(adc_dev, REG_IRQCLR, (IRQENB_FIFO1THRES |
+ IRQENB_FIFO1OVRRUN | IRQENB_FIFO1UNDRFLW));
+ tiadc_writel(adc_dev, REG_SE, STPENB_STEPENB_TC);
+
+ /* Flush FIFO of any leftover data */
+ fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
+ for (i = 0; i < fifo1count; i++)
+ read = tiadc_readl(adc_dev, REG_FIFO1);
+
+ return iio_triggered_buffer_predisable(indio_dev);
+}
+
+static int tiadc_buffer_postdisable(struct iio_dev *indio_dev)
+{
+ struct tiadc_device *adc_dev = iio_priv(indio_dev);
+ int config;
+
+ tiadc_step_config(indio_dev);
+ config = tiadc_readl(adc_dev, REG_CTRL);
+ tiadc_writel(adc_dev, REG_CTRL, (config | CNTRLREG_TSCSSENB));
+
+ return 0;
+}
+
+static const struct iio_buffer_setup_ops tiadc_buffer_setup_ops = {
+ .preenable = &tiadc_buffer_preenable,
+ .postenable = &tiadc_buffer_postenable,
+ .predisable = &tiadc_buffer_predisable,
+ .postdisable = &tiadc_buffer_postdisable,
+};
+
+static void tiadc_adc_work(struct work_struct *work_s)
+{
+ struct tiadc_device *adc_dev =
+ container_of(work_s, struct tiadc_device, poll_work);
+ struct iio_dev *indio_dev = iio_priv_to_dev(adc_dev);
+ struct iio_buffer *buffer = indio_dev->buffer;
+ int i, j, k, fifo1count, read;
+ unsigned int config;
+ int size_to_acquire = buffer->access->get_length(buffer);
+ int sample_count = 0;
+ u32 *data;
+
+ adc_dev->data_avail = 0;
+ data = kmalloc(indio_dev->scan_bytes, GFP_KERNEL);
+ if (data == NULL)
+ goto out;
+
+ while (sample_count < size_to_acquire) {
+ tiadc_writel(adc_dev, REG_IRQSTATUS, IRQENB_FIFO1THRES);
+ tiadc_writel(adc_dev, REG_IRQENABLE, IRQENB_FIFO1THRES);
+
+ wait_event_interruptible(adc_dev->wq_data_avail,
+ (adc_dev->data_avail == 1));
+ adc_dev->data_avail = 0;
+
+ fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
+ if (fifo1count * sizeof(u32) <
+ buffer->access->get_bytes_per_datum(buffer))
+ continue;
+
+ sample_count = sample_count + fifo1count;
+ for (k = 0; k < fifo1count; k = k + i) {
+ for (i = 0, j = 0; i < (indio_dev->scan_bytes)/4; i++) {
+ read = tiadc_readl(adc_dev, REG_FIFO1);
+ data[i] = read & FIFOREAD_DATA_MASK;
+ }
+ iio_push_to_buffers(indio_dev, (u8 *) data);
+ }
+ }
+out:
+ tiadc_writel(adc_dev, REG_IRQCLR, (IRQENB_FIFO1THRES |
+ IRQENB_FIFO1OVRRUN | IRQENB_FIFO1UNDRFLW));
+ config = tiadc_readl(adc_dev, REG_CTRL);
+ tiadc_writel(adc_dev, REG_CTRL, config & ~CNTRLREG_TSCSSENB);
+}
+irqreturn_t tiadc_iio_pollfunc(int irq, void *p)
+{
+ struct iio_poll_func *pf = p;
+ struct iio_dev *indio_dev = pf->indio_dev;
+ struct tiadc_device *adc_dev = iio_priv(indio_dev);
+ int i, fifo1count, read;
+
+ tiadc_writel(adc_dev, REG_IRQCLR, (IRQENB_FIFO1THRES |
+ IRQENB_FIFO1OVRRUN |
+ IRQENB_FIFO1UNDRFLW));
+
+ /* Flush FIFO before trigger */
+ fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
+ for (i = 0; i < fifo1count; i++)
+ read = tiadc_readl(adc_dev, REG_FIFO1);
+
+ return IRQ_WAKE_THREAD;
}
static const char * const chan_name_ain[] = {
@@ -120,13 +315,13 @@ static int tiadc_channel_init(struct iio_dev *indio_dev, int channels)
chan->channel = adc_dev->channel_line[i];
chan->info_mask_separate = BIT(IIO_CHAN_INFO_RAW);
chan->datasheet_name = chan_name_ain[chan->channel];
+ chan->scan_index = i;
chan->scan_type.sign = 'u';
chan->scan_type.realbits = 12;
chan->scan_type.storagebits = 32;
}
indio_dev->channels = chan_array;
-
return 0;
}
@@ -141,58 +336,48 @@ static int tiadc_read_raw(struct iio_dev *indio_dev,
{
struct tiadc_device *adc_dev = iio_priv(indio_dev);
int i, map_val;
- unsigned int fifo1count, read, stepid;
- u32 step = UINT_MAX;
- bool found = false;
- u32 step_en;
- unsigned long timeout = jiffies + usecs_to_jiffies
- (IDLE_TIMEOUT * adc_dev->channels);
- step_en = get_adc_step_mask(adc_dev);
- am335x_tsc_se_set(adc_dev->mfd_tscadc, step_en);
-
- /* Wait for ADC sequencer to complete sampling */
- while (tiadc_readl(adc_dev, REG_ADCFSM) & SEQ_STATUS) {
- if (time_after(jiffies, timeout))
- return -EAGAIN;
- }
- map_val = chan->channel + TOTAL_CHANNELS;
-
- /*
- * When the sub-system is first enabled,
- * the sequencer will always start with the
- * lowest step (1) and continue until step (16).
- * For ex: If we have enabled 4 ADC channels and
- * currently use only 1 out of them, the
- * sequencer still configures all the 4 steps,
- * leading to 3 unwanted data.
- * Hence we need to flush out this data.
- */
+ unsigned int fifo1count, read, stepid, step_en;
- for (i = 0; i < ARRAY_SIZE(adc_dev->channel_step); i++) {
- if (chan->channel == adc_dev->channel_line[i]) {
- step = adc_dev->channel_step[i];
- break;
- }
- }
- if (WARN_ON_ONCE(step == UINT_MAX))
- return -EINVAL;
-
- fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
- for (i = 0; i < fifo1count; i++) {
- read = tiadc_readl(adc_dev, REG_FIFO1);
- stepid = read & FIFOREAD_CHNLID_MASK;
- stepid = stepid >> 0x10;
-
- if (stepid == map_val) {
- read = read & FIFOREAD_DATA_MASK;
- found = true;
- *val = read;
+ if (iio_buffer_enabled(indio_dev))
+ return -EBUSY;
+ else {
+ unsigned long timeout = jiffies + usecs_to_jiffies
+ (IDLE_TIMEOUT * adc_dev->channels);
+ step_en = get_adc_step_mask(adc_dev);
+ am335x_tsc_se_set(adc_dev->mfd_tscadc, step_en);
+
+ /* Wait for ADC sequencer to complete sampling */
+ while (tiadc_readl(adc_dev, REG_ADCFSM) & SEQ_STATUS) {
+ if (time_after(jiffies, timeout))
+ return -EAGAIN;
+ }
+ map_val = chan->channel + TOTAL_CHANNELS;
+
+ /*
+ * When the sub-system is first enabled,
+ * the sequencer will always start with the
+ * lowest step (1) and continue until step (16).
+ * For ex: If we have enabled 4 ADC channels and
+ * currently use only 1 out of them, the
+ * sequencer still configures all the 4 steps,
+ * leading to 3 unwanted data.
+ * Hence we need to flush out this data.
+ */
+
+ fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
+ for (i = 0; i < fifo1count; i++) {
+ read = tiadc_readl(adc_dev, REG_FIFO1);
+ stepid = read & FIFOREAD_CHNLID_MASK;
+ stepid = stepid >> 0x10;
+
+ if (stepid == map_val) {
+ read = read & FIFOREAD_DATA_MASK;
+ *val = read;
+ return IIO_VAL_INT;
+ }
}
+ return -EAGAIN;
}
-
- if (found == false)
- return -EBUSY;
- return IIO_VAL_INT;
}
static const struct iio_info tiadc_info = {
@@ -231,18 +416,33 @@ static int tiadc_probe(struct platform_device *pdev)
channels++;
}
adc_dev->channels = channels;
+ adc_dev->irq = adc_dev->mfd_tscadc->irq;
indio_dev->dev.parent = &pdev->dev;
indio_dev->name = dev_name(&pdev->dev);
indio_dev->modes = INDIO_DIRECT_MODE;
indio_dev->info = &tiadc_info;
- tiadc_step_config(adc_dev);
+ tiadc_step_config(indio_dev);
+ tiadc_writel(adc_dev, REG_FIFO1THR, FIFO1_THRESHOLD);
err = tiadc_channel_init(indio_dev, adc_dev->channels);
if (err < 0)
goto err_free_device;
+ INIT_WORK(&adc_dev->poll_work, &tiadc_adc_work);
+ init_waitqueue_head(&adc_dev->wq_data_avail);
+
+ err = request_irq(adc_dev->irq, tiadc_irq, IRQF_SHARED,
+ indio_dev->name, indio_dev);
+ if (err)
+ goto err_free_irq;
+
+ err = iio_triggered_buffer_setup(indio_dev, &tiadc_iio_pollfunc,
+ &tiadc_trigger_h, &tiadc_buffer_setup_ops);
+ if (err)
+ goto err_unregister;
+
err = iio_device_register(indio_dev);
if (err)
goto err_free_channels;
@@ -251,6 +451,10 @@ static int tiadc_probe(struct platform_device *pdev)
return 0;
+err_unregister:
+ iio_buffer_unregister(indio_dev);
+err_free_irq:
+ free_irq(adc_dev->irq, indio_dev);
err_free_channels:
tiadc_channels_remove(indio_dev);
err_free_device:
@@ -265,7 +469,9 @@ static int tiadc_remove(struct platform_device *pdev)
struct tiadc_device *adc_dev = iio_priv(indio_dev);
u32 step_en;
+ free_irq(adc_dev->irq, indio_dev);
iio_device_unregister(indio_dev);
+ iio_buffer_unregister(indio_dev);
tiadc_channels_remove(indio_dev);
step_en = get_adc_step_mask(adc_dev);
@@ -303,10 +509,16 @@ static int tiadc_resume(struct device *dev)
/* Make sure ADC is powered up */
restore = tiadc_readl(adc_dev, REG_CTRL);
- restore &= ~(CNTRLREG_POWERDOWN);
+ restore &= ~(CNTRLREG_TSCSSENB);
tiadc_writel(adc_dev, REG_CTRL, restore);
- tiadc_step_config(adc_dev);
+ tiadc_writel(adc_dev, REG_FIFO1THR, FIFO1_THRESHOLD);
+ tiadc_step_config(indio_dev);
+
+ /* Make sure ADC is powered up */
+ restore &= ~(CNTRLREG_POWERDOWN);
+ restore |= CNTRLREG_TSCSSENB;
+ tiadc_writel(adc_dev, REG_CTRL, restore);
return 0;
}
diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
index db1791b..cb61027 100644
--- a/include/linux/mfd/ti_am335x_tscadc.h
+++ b/include/linux/mfd/ti_am335x_tscadc.h
@@ -46,17 +46,23 @@
/* Step Enable */
#define STEPENB_MASK (0x1FFFF << 0)
#define STEPENB(val) ((val) << 0)
+#define ENB(val) (1 << (val))
+#define STPENB_STEPENB STEPENB(0x1FFFF)
+#define STPENB_STEPENB_TC STEPENB(0x1FFF)
/* IRQ enable */
#define IRQENB_HW_PEN BIT(0)
#define IRQENB_FIFO0THRES BIT(2)
#define IRQENB_FIFO1THRES BIT(5)
#define IRQENB_PENUP BIT(9)
+#define IRQENB_FIFO1OVRRUN BIT(6)
+#define IRQENB_FIFO1UNDRFLW BIT(7)
/* Step Configuration */
#define STEPCONFIG_MODE_MASK (3 << 0)
#define STEPCONFIG_MODE(val) ((val) << 0)
#define STEPCONFIG_MODE_HWSYNC STEPCONFIG_MODE(2)
+#define STEPCONFIG_MODE_SWCNT STEPCONFIG_MODE(1)
#define STEPCONFIG_AVG_MASK (7 << 2)
#define STEPCONFIG_AVG(val) ((val) << 2)
#define STEPCONFIG_AVG_16 STEPCONFIG_AVG(4)
@@ -124,7 +130,8 @@
#define MAX_CLK_DIV 7
#define TOTAL_STEPS 16
#define TOTAL_CHANNELS 8
-
+#define FIFO1_THRESHOLD 19
+#define FIFO_SIZE 64
/*
* ADC runs at 3MHz, and it takes
* 15 cycles to latch one data output.
@@ -153,6 +160,10 @@ struct ti_tscadc_dev {
/* adc device */
struct adc_device *adc;
+
+ /* Context save */
+ unsigned int irqstat;
+ unsigned int ctrl;
};
static inline struct ti_tscadc_dev *ti_tscadc_dev_get(struct platform_device *p)
--
1.7.9.5
^ permalink raw reply related
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