* Re: [PATCH v10] leds: USB: HID: Add support for MSI GT683R led panels
From: Johan Hovold @ 2014-06-23 17:27 UTC (permalink / raw)
To: Janne Kanniainen
Cc: johan, greg, jkosina, cooloney, linux-kernel, linux-leds,
linux-usb, linux-input
In-Reply-To: <1403543808-8228-1-git-send-email-janne.kanniainen@gmail.com>
On Mon, Jun 23, 2014 at 08:16:48PM +0300, Janne Kanniainen wrote:
> This driver adds support for USB controlled led panels that exists in
> MSI GT683R laptop
>
> Signed-off-by: Janne Kanniainen <janne.kanniainen@gmail.com>
> ---
> Changes in v2:
> - sorted headers to alphabetic order
> - using devm_kzalloc
> - using BIT(n)
> - using usb_control_msg instead of usb_submit_urb
> - removing unneeded code
> Changes in v3:
> - implemented as HID device
> - some cleanups and bug fixes
> Changes in v4:
> - more cleanups
> - support for selecting leds
> - suppport for selecting status
>
> Changes in v5:
> - mode attribute documented under Documentation/ABI
> - made array for led_classdev
> - led devices uses now recommended naming scheme
>
> Changes in v6:
> - flush_work added
> - using hid device name instead of hard coded gt683r
> - allocating name buffers with devm_kzalloc
>
> Changes in v7:
> - buf with for fixed
>
> Changes in v8:
> - some cleanups and bugs fixed
>
> Changes in v9:
> - few style issues fixed
>
> Changes in v10:
> - race condition fixed
> - using proper attribute group
You need to send a separate patch on top of v9, which Jiri has already
applied to his tree.
<snip>
> +static DEVICE_ATTR_RW(leds_mode);
> +
> +static struct attribute *gt683r_attributes[] = {
> + &dev_attr_leds_mode.attr,
> + NULL,
> +};
> +
> +static struct attribute_group gt683r_attribute_group = {
> + .attrs = gt683r_attributes
> +};
> +
> +static int gt683r_led_probe(struct hid_device *hdev,
> + const struct hid_device_id *id)
> +{
> + int i;
> + int ret;
> + int name_sz;
> + char *name;
> + struct gt683r_led *led;
> +
> + led = devm_kzalloc(&hdev->dev, sizeof(*led), GFP_KERNEL);
> + if (!led)
> + return -ENOMEM;
> +
> + mutex_init(&led->lock);
> + INIT_WORK(&led->work, gt683r_led_work);
> +
> + led->mode = GT683R_LED_NORMAL;
> + led->hdev = hdev;
> + hid_set_drvdata(hdev, led);
> +
> + ret = hid_parse(hdev);
> + if (ret) {
> + hid_err(hdev, "hid parsing failed\n");
> + return ret;
> + }
> +
> + ret = hid_hw_start(hdev, HID_CONNECT_HIDRAW);
> + if (ret) {
> + hid_err(hdev, "hw start failed\n");
> + return ret;
> + }
> +
> + for (i = 0; i < GT683R_LED_COUNT; i++) {
> + name_sz = strlen(dev_name(&hdev->dev)) +
> + strlen(gt683r_panel_names[i]) + 3;
> +
> + name = devm_kzalloc(&hdev->dev, name_sz, GFP_KERNEL);
> + if (!name) {
> + ret = -ENOMEM;
> + goto fail;
> + }
> +
> + snprintf(name, name_sz, "%s::%s",
> + dev_name(&hdev->dev), gt683r_panel_names[i]);
> + led->led_devs[i].name = name;
> + led->led_devs[i].max_brightness = 1;
> + led->led_devs[i].brightness_set = gt683r_brightness_set;
> + ret = led_classdev_register(&hdev->dev, &led->led_devs[i]);
> + if (ret) {
> + hid_err(hdev, "could not register led device\n");
> + goto fail;
> + }
> + }
> +
> + ret = sysfs_create_group(&led->hdev->dev.kobj, >683r_attribute_group);
> + if (ret) {
> + hid_err(hdev, "failed to create sysfs attributes\n");
> + goto fail;
> + }
This does not solve the race Greg is referring to. There's some more
info here:
http://kroah.com/log/blog/2013/06/26/how-to-create-a-sysfs-file-correctly/
but I'm not sure exactly how you'd apply that to an HID driver. Perhaps
you could use the struct device_driver in struct hid_driver (although it
is currently marked as "private").
Johan
> +
> + return 0;
> +
> +fail:
> + for (i = i - 1; i >= 0; i--)
> + led_classdev_unregister(&led->led_devs[i]);
> + hid_hw_stop(hdev);
> + return ret;
> +}
> +
> +static void gt683r_led_remove(struct hid_device *hdev)
> +{
> + int i;
> + struct gt683r_led *led = hid_get_drvdata(hdev);
> +
> + sysfs_remove_group(&led->hdev->dev.kobj, >683r_attribute_group);
> + for (i = 0; i < GT683R_LED_COUNT; i++)
> + led_classdev_unregister(&led->led_devs[i]);
> + flush_work(&led->work);
> + hid_hw_stop(hdev);
> +}
> +
> +static struct hid_driver gt683r_led_driver = {
> + .probe = gt683r_led_probe,
> + .remove = gt683r_led_remove,
> + .name = "gt683r_led",
> + .id_table = gt683r_led_id,
> +};
> +
> +module_hid_driver(gt683r_led_driver);
> +
> +MODULE_AUTHOR("Janne Kanniainen");
> +MODULE_DESCRIPTION("MSI GT683R led driver");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index 34bb220..3692d37 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -641,7 +641,7 @@
> #define USB_DEVICE_ID_GENIUS_KB29E 0x3004
>
> #define USB_VENDOR_ID_MSI 0x1770
> -#define USB_DEVICE_ID_MSI_GX680R_LED_PANEL 0xff00
> +#define USB_DEVICE_ID_MSI_GT683R_LED_PANEL 0xff00
>
> #define USB_VENDOR_ID_NATIONAL_SEMICONDUCTOR 0x0400
> #define USB_DEVICE_ID_N_S_HARMONY 0xc359
> diff --git a/drivers/hid/usbhid/hid-quirks.c b/drivers/hid/usbhid/hid-quirks.c
> index 8e4ddb3..c640e1d 100644
> --- a/drivers/hid/usbhid/hid-quirks.c
> +++ b/drivers/hid/usbhid/hid-quirks.c
> @@ -73,7 +73,7 @@ static const struct hid_blacklist {
> { USB_VENDOR_ID_FORMOSA, USB_DEVICE_ID_FORMOSA_IR_RECEIVER, HID_QUIRK_NO_INIT_REPORTS },
> { USB_VENDOR_ID_FREESCALE, USB_DEVICE_ID_FREESCALE_MX28, HID_QUIRK_NOGET },
> { USB_VENDOR_ID_MGE, USB_DEVICE_ID_MGE_UPS, HID_QUIRK_NOGET },
> - { USB_VENDOR_ID_MSI, USB_DEVICE_ID_MSI_GX680R_LED_PANEL, HID_QUIRK_NO_INIT_REPORTS },
> + { USB_VENDOR_ID_MSI, USB_DEVICE_ID_MSI_GT683R_LED_PANEL, HID_QUIRK_NO_INIT_REPORTS },
> { USB_VENDOR_ID_NEXIO, USB_DEVICE_ID_NEXIO_MULTITOUCH_PTI0750, HID_QUIRK_NO_INIT_REPORTS },
> { USB_VENDOR_ID_NOVATEK, USB_DEVICE_ID_NOVATEK_MOUSE, HID_QUIRK_NO_INIT_REPORTS },
> { USB_VENDOR_ID_PIXART, USB_DEVICE_ID_PIXART_OPTICAL_TOUCH_SCREEN, HID_QUIRK_NO_INIT_REPORTS },
^ permalink raw reply
* Re: [PATCH v3 3/6] iio: adc: fsl,imx25-gcq driver
From: Jonathan Cameron @ 2014-06-23 18:13 UTC (permalink / raw)
To: Denis Carikli, Shawn Guo, Samuel Ortiz, Dmitry Torokhov,
Fabio Estevam, Peter Meerwald, Hartmut Knaack
Cc: Eric Bénard, Sascha Hauer, linux-arm-kernel, Lee Jones,
linux-input, linux-iio, Lars-Peter Clausen, Markus Pargmann
In-Reply-To: <53A85922.1000209@eukrea.com>
On 23/06/14 17:43, Denis Carikli wrote:
> On 06/21/2014 12:51 PM, Jonathan Cameron wrote:
>>> +static irqreturn_t mx25_gcq_irq(int irq, void *data)
>>> +{
>>> + struct mx25_gcq_priv *priv = data;
>>> + u32 stats;
>> Not that much in here, but perhaps still makes sense for this to be a
>> threaded
>> irq?
> What would be the advantage of converting it to be a threaded IRQ?
>
> I guess that there will be no use of IRQF_SHARED and that only this driver will have to handle the GCQ IRQ in the future.
>
Fair enough. Was more of an idle thought that anything else!
^ permalink raw reply
* Re: [PATCH v10] leds: USB: HID: Add support for MSI GT683R led panels
From: Greg KH @ 2014-06-23 18:23 UTC (permalink / raw)
To: Janne Kanniainen
Cc: johan-DgEjT+Ai2ygdnm+yROfE0A, jkosina-AlSwsSmVLrQ,
cooloney-Re5JQEeQqe8AvxtiuMwx3w,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-leds-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA,
linux-input-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1403543808-8228-1-git-send-email-janne.kanniainen-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On Mon, Jun 23, 2014 at 08:16:48PM +0300, Janne Kanniainen wrote:
> + ret = sysfs_create_group(&led->hdev->dev.kobj, >683r_attribute_group);
> + if (ret) {
> + hid_err(hdev, "failed to create sysfs attributes\n");
> + goto fail;
> + }
No, you need to set the attribute group _before_ you call
led_classdev_register, as that is where the device will be created in
sysfs. Surely the other led drivers already do this? I'm almost afraid
to go look...
You also have to document your sysfs file in Documentation/ABI/
thanks,
greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v10] leds: USB: HID: Add support for MSI GT683R led panels
From: Greg KH @ 2014-06-23 18:24 UTC (permalink / raw)
To: Janne Kanniainen
Cc: johan, jkosina, cooloney, linux-kernel, linux-leds, linux-usb,
linux-input
In-Reply-To: <20140623182324.GA30831@kroah.com>
On Mon, Jun 23, 2014 at 02:23:24PM -0400, Greg KH wrote:
> On Mon, Jun 23, 2014 at 08:16:48PM +0300, Janne Kanniainen wrote:
> > + ret = sysfs_create_group(&led->hdev->dev.kobj, >683r_attribute_group);
> > + if (ret) {
> > + hid_err(hdev, "failed to create sysfs attributes\n");
> > + goto fail;
> > + }
>
> No, you need to set the attribute group _before_ you call
> led_classdev_register, as that is where the device will be created in
> sysfs. Surely the other led drivers already do this? I'm almost afraid
> to go look...
Yes, they do it already, set .dev_attr_group and you should be fine.
thanks,
greg k-h
^ permalink raw reply
* Re: PATCH change for HID_BATTERY_STRENGTH kconfig
From: Daniel Nicoletti @ 2014-06-23 19:08 UTC (permalink / raw)
To: Jiri Kosina; +Cc: linux-input
In-Reply-To: <alpine.LNX.2.00.1305240016180.13213@pobox.suse.cz>
2013-05-23 19:16 GMT-03:00 Jiri Kosina <jkosina@suse.cz>:
> On Mon, 13 May 2013, Daniel Nicoletti wrote:
>
>> Hi,
>> I'd like to propose this patch while removes the need for
>> hid to be compiled build-in, as this is the same behavior
>> that WiiMote and Wacom have.
>> Is this ok?
>
> Makes sense to me. Could you please resend with proper changelog and
> Signed-off-by: line?
>
> Thanks.
Sorry it took more than a year for me to do this,
so I called git send-email, and so far the CC and
linux-input didn't get it, tho it said result OK.
git send-email --to "Jiri Kosina <jkosina@suse.cz>" --cc
"linux-input@vger.kernel.org"
0001-HID-Allows-for-HID_BATTERY_STRENGTH-config-be-enable.patch
I'm pasting it here hoping gmail doesn't screw
it, if you prefer that I send it using the above
command mind you telling what I did wrong?
Thanks.
>From df740fd81dffbb74e13c622392fe9a945006fd3c Mon Sep 17 00:00:00 2001
From: Daniel Nicoletti <dantti12@gmail.com>
Date: Mon, 23 Jun 2014 10:31:47 -0300
Subject: [PATCH] HID: Allows for HID_BATTERY_STRENGTH config be enabled
without the need for HID module to be built-in.
Signed-off-by: Daniel Nicoletti <dantti12@gmail.com>
---
drivers/hid/Kconfig | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index fb52f3f..d6d85ad 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -27,7 +27,8 @@ if HID
config HID_BATTERY_STRENGTH
bool "Battery level reporting for HID devices"
- depends on HID && POWER_SUPPLY && HID = POWER_SUPPLY
+ depends on HID
+ select POWER_SUPPLY
default n
---help---
This option adds support of reporting battery strength (for HID devices
--
2.0.0
^ permalink raw reply related
* Re: [PATCH v10] leds: USB: HID: Add support for MSI GT683R led panels
From: Johan Hovold @ 2014-06-23 19:31 UTC (permalink / raw)
To: Greg KH
Cc: Janne Kanniainen, johan, jkosina, cooloney, linux-kernel,
linux-leds, linux-usb, linux-input
In-Reply-To: <20140623182432.GB30831@kroah.com>
On Mon, Jun 23, 2014 at 02:24:32PM -0400, Greg KH wrote:
> On Mon, Jun 23, 2014 at 02:23:24PM -0400, Greg KH wrote:
> > On Mon, Jun 23, 2014 at 08:16:48PM +0300, Janne Kanniainen wrote:
> > > + ret = sysfs_create_group(&led->hdev->dev.kobj, >683r_attribute_group);
> > > + if (ret) {
> > > + hid_err(hdev, "failed to create sysfs attributes\n");
> > > + goto fail;
> > > + }
> >
> > No, you need to set the attribute group _before_ you call
> > led_classdev_register, as that is where the device will be created in
> > sysfs. Surely the other led drivers already do this? I'm almost afraid
> > to go look...
>
> Yes, they do it already, set .dev_attr_group and you should be fine.
But this isn't an attribute of the LEDs but rather of the parent HID
device that is being probed (the led_mode is common to all three LEDs
and thus belongs in the parent device, right?).
Johan
^ permalink raw reply
* Re: [PATCH v10] leds: USB: HID: Add support for MSI GT683R led panels
From: Greg KH @ 2014-06-23 19:40 UTC (permalink / raw)
To: Johan Hovold
Cc: Janne Kanniainen, jkosina, cooloney, linux-kernel, linux-leds,
linux-usb, linux-input
In-Reply-To: <20140623193134.GA15945@localhost>
On Mon, Jun 23, 2014 at 09:31:34PM +0200, Johan Hovold wrote:
> On Mon, Jun 23, 2014 at 02:24:32PM -0400, Greg KH wrote:
> > On Mon, Jun 23, 2014 at 02:23:24PM -0400, Greg KH wrote:
> > > On Mon, Jun 23, 2014 at 08:16:48PM +0300, Janne Kanniainen wrote:
> > > > + ret = sysfs_create_group(&led->hdev->dev.kobj, >683r_attribute_group);
> > > > + if (ret) {
> > > > + hid_err(hdev, "failed to create sysfs attributes\n");
> > > > + goto fail;
> > > > + }
> > >
> > > No, you need to set the attribute group _before_ you call
> > > led_classdev_register, as that is where the device will be created in
> > > sysfs. Surely the other led drivers already do this? I'm almost afraid
> > > to go look...
> >
> > Yes, they do it already, set .dev_attr_group and you should be fine.
>
> But this isn't an attribute of the LEDs but rather of the parent HID
> device that is being probed (the led_mode is common to all three LEDs
> and thus belongs in the parent device, right?).
Then that's even worse :(
The sysfs attribute should be on the class device here for the LED, you
should not put an attribute on a device you are not the driver for.
greg k-h
^ permalink raw reply
* Re: [PATCH v10] leds: USB: HID: Add support for MSI GT683R led panels
From: Johan Hovold @ 2014-06-23 19:52 UTC (permalink / raw)
To: Greg KH
Cc: Johan Hovold, Janne Kanniainen, jkosina, cooloney, linux-kernel,
linux-leds, linux-usb, linux-input
In-Reply-To: <20140623194059.GA19615@kroah.com>
On Mon, Jun 23, 2014 at 03:40:59PM -0400, Greg KH wrote:
> On Mon, Jun 23, 2014 at 09:31:34PM +0200, Johan Hovold wrote:
> > On Mon, Jun 23, 2014 at 02:24:32PM -0400, Greg KH wrote:
> > > On Mon, Jun 23, 2014 at 02:23:24PM -0400, Greg KH wrote:
> > > > On Mon, Jun 23, 2014 at 08:16:48PM +0300, Janne Kanniainen wrote:
> > > > > + ret = sysfs_create_group(&led->hdev->dev.kobj, >683r_attribute_group);
> > > > > + if (ret) {
> > > > > + hid_err(hdev, "failed to create sysfs attributes\n");
> > > > > + goto fail;
> > > > > + }
> > > >
> > > > No, you need to set the attribute group _before_ you call
> > > > led_classdev_register, as that is where the device will be created in
> > > > sysfs. Surely the other led drivers already do this? I'm almost afraid
> > > > to go look...
> > >
> > > Yes, they do it already, set .dev_attr_group and you should be fine.
> >
> > But this isn't an attribute of the LEDs but rather of the parent HID
> > device that is being probed (the led_mode is common to all three LEDs
> > and thus belongs in the parent device, right?).
>
> Then that's even worse :(
>
> The sysfs attribute should be on the class device here for the LED, you
> should not put an attribute on a device you are not the driver for.
But this is the driver for the HID device, which then in turn has three
individual LEDs. This particular device isn't really an input device
(the actual keyboard in this case appears to be connected over PS2), but
there are several HID drivers which are primarily input devices with
LEDs as sub-devices (e.g. drivers/hid/hid-lg4ff.c).
Johan
^ permalink raw reply
* Re: [PATCH v10] leds: USB: HID: Add support for MSI GT683R led panels
From: Greg KH @ 2014-06-23 20:24 UTC (permalink / raw)
To: Johan Hovold
Cc: Janne Kanniainen, jkosina, cooloney, linux-kernel, linux-leds,
linux-usb, linux-input
In-Reply-To: <20140623195212.GB15945@localhost>
On Mon, Jun 23, 2014 at 09:52:12PM +0200, Johan Hovold wrote:
> On Mon, Jun 23, 2014 at 03:40:59PM -0400, Greg KH wrote:
> > On Mon, Jun 23, 2014 at 09:31:34PM +0200, Johan Hovold wrote:
> > > On Mon, Jun 23, 2014 at 02:24:32PM -0400, Greg KH wrote:
> > > > On Mon, Jun 23, 2014 at 02:23:24PM -0400, Greg KH wrote:
> > > > > On Mon, Jun 23, 2014 at 08:16:48PM +0300, Janne Kanniainen wrote:
> > > > > > + ret = sysfs_create_group(&led->hdev->dev.kobj, >683r_attribute_group);
> > > > > > + if (ret) {
> > > > > > + hid_err(hdev, "failed to create sysfs attributes\n");
> > > > > > + goto fail;
> > > > > > + }
> > > > >
> > > > > No, you need to set the attribute group _before_ you call
> > > > > led_classdev_register, as that is where the device will be created in
> > > > > sysfs. Surely the other led drivers already do this? I'm almost afraid
> > > > > to go look...
> > > >
> > > > Yes, they do it already, set .dev_attr_group and you should be fine.
> > >
> > > But this isn't an attribute of the LEDs but rather of the parent HID
> > > device that is being probed (the led_mode is common to all three LEDs
> > > and thus belongs in the parent device, right?).
> >
> > Then that's even worse :(
> >
> > The sysfs attribute should be on the class device here for the LED, you
> > should not put an attribute on a device you are not the driver for.
>
> But this is the driver for the HID device, which then in turn has three
> individual LEDs. This particular device isn't really an input device
> (the actual keyboard in this case appears to be connected over PS2), but
> there are several HID drivers which are primarily input devices with
> LEDs as sub-devices (e.g. drivers/hid/hid-lg4ff.c).
I don't know the specifics here, but as you just created a class device,
why aren't the attributes on that class device? Shouldn't that be the
logical place for it, instead of having them on some "random" other type
of device? Userspace will never be notified that the attribute is on
that device due to the file being created "later", and tools using
libudev and the like will be looking at the LED class device, not the
"parent" device for any specific LED stuff.
but if this really is the way that all LED devices work, and userspace
programs are expecting this, that's seems odd.
greg k-h
^ permalink raw reply
* [PATCH v2] Input - wacom: assign phys field from struct wacom into input_dev
From: Benjamin Tissoires @ 2014-06-23 20:30 UTC (permalink / raw)
To: Dmitry Torokhov, Ping Cheng, Jason Gerecke; +Cc: linux-input, linux-kernel
This field was not used for 9 years, it is time to assign it.
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
Hi Dmitry,
this is the v2 (more a re-spin) of the patch entitled:
"Input - wacom: remove phys field in struct wacom".
Cheers,
Benjamin
drivers/input/tablet/wacom_sys.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/input/tablet/wacom_sys.c b/drivers/input/tablet/wacom_sys.c
index afa4e8d..f03f25b 100644
--- a/drivers/input/tablet/wacom_sys.c
+++ b/drivers/input/tablet/wacom_sys.c
@@ -1133,6 +1133,7 @@ static int wacom_register_input(struct wacom *wacom)
}
input_dev->name = wacom_wac->name;
+ input_dev->phys = wacom->phys;
input_dev->dev.parent = &intf->dev;
input_dev->open = wacom_open;
input_dev->close = wacom_close;
--
1.9.3
^ permalink raw reply related
* Re: [PATCH v10] leds: USB: HID: Add support for MSI GT683R led panels
From: Johan Hovold @ 2014-06-23 20:44 UTC (permalink / raw)
To: Greg KH
Cc: Johan Hovold, Janne Kanniainen, jkosina, cooloney, linux-kernel,
linux-leds, linux-usb, linux-input
In-Reply-To: <20140623202448.GB29035@kroah.com>
On Mon, Jun 23, 2014 at 04:24:48PM -0400, Greg KH wrote:
> On Mon, Jun 23, 2014 at 09:52:12PM +0200, Johan Hovold wrote:
> > On Mon, Jun 23, 2014 at 03:40:59PM -0400, Greg KH wrote:
> > > On Mon, Jun 23, 2014 at 09:31:34PM +0200, Johan Hovold wrote:
> > > > On Mon, Jun 23, 2014 at 02:24:32PM -0400, Greg KH wrote:
> > > > > On Mon, Jun 23, 2014 at 02:23:24PM -0400, Greg KH wrote:
> > > > > > On Mon, Jun 23, 2014 at 08:16:48PM +0300, Janne Kanniainen wrote:
> > > > > > > + ret = sysfs_create_group(&led->hdev->dev.kobj, >683r_attribute_group);
> > > > > > > + if (ret) {
> > > > > > > + hid_err(hdev, "failed to create sysfs attributes\n");
> > > > > > > + goto fail;
> > > > > > > + }
> > > > > >
> > > > > > No, you need to set the attribute group _before_ you call
> > > > > > led_classdev_register, as that is where the device will be created in
> > > > > > sysfs. Surely the other led drivers already do this? I'm almost afraid
> > > > > > to go look...
> > > > >
> > > > > Yes, they do it already, set .dev_attr_group and you should be fine.
> > > >
> > > > But this isn't an attribute of the LEDs but rather of the parent HID
> > > > device that is being probed (the led_mode is common to all three LEDs
> > > > and thus belongs in the parent device, right?).
> > >
> > > Then that's even worse :(
> > >
> > > The sysfs attribute should be on the class device here for the LED, you
> > > should not put an attribute on a device you are not the driver for.
> >
> > But this is the driver for the HID device, which then in turn has three
> > individual LEDs. This particular device isn't really an input device
> > (the actual keyboard in this case appears to be connected over PS2), but
> > there are several HID drivers which are primarily input devices with
> > LEDs as sub-devices (e.g. drivers/hid/hid-lg4ff.c).
>
> I don't know the specifics here, but as you just created a class device,
> why aren't the attributes on that class device? Shouldn't that be the
> logical place for it, instead of having them on some "random" other type
> of device?
This is a non-standard attribute of this particular laptop. It has three
individual LEDs that can be enabled separately (using standard LED class
attributes), but they will all three be in the same "mode" (which here
apparently means that they can be fully on, vary with the volume(?!), or
pulse synchronously when enabled).
If we were to implement this mode attribute as a class attribute,
changing the mode of of one LED would also change the mode of the other
two devices (LEDs). Therefore I think it has to be an attribute of the
parent HID device (that the driver is for).
> Userspace will never be notified that the attribute is on
> that device due to the file being created "later", and tools using
> libudev and the like will be looking at the LED class device, not the
> "parent" device for any specific LED stuff.
Yeah, that seems to be the case. I doubt anyone will care much about
this particular custom attribute, but sure, setting the led_mode of the
HID device from an udev rule could be problematic.
> but if this really is the way that all LED devices work, and userspace
> programs are expecting this, that's seems odd.
Johan
^ permalink raw reply
* [PATCH 1/5] Input - wacom: create a separate input device for pads
From: Benjamin Tissoires @ 2014-06-23 20:57 UTC (permalink / raw)
To: Dmitry Torokhov, Ping Cheng, Jason Gerecke
Cc: linux-input, linux-kernel, linuxwacom-devel
In-Reply-To: <1403557039-1487-1-git-send-email-benjamin.tissoires@redhat.com>
Currently, the pad events are sent through the stylus input device
for the Intuos/Cintiqs, and through the touch input device for the
Bamboos.
To differentiate the buttons pressed on the pad from the ones pressed
on the stylus, the Intuos/Cintiq uses MISC_SERIAL and ABS_MISC. This
lead to a multiplexing of the events into one device, which are then
splitted out in xf86-input-wacom. Bamboos are not using MISC events
because the pad is attached to the touch interface, and only BTN_TOUCH
is used for the finger (and DOUBLE_TAP, etc...). However, the user space
driver still splits out the pad from the touch interface in the same
way it does for the pro line devices.
The other problem we can see with this fact is that some of the Intuos
and Cintiq have a wheel, and the effective range of the reported values
is [0..71]. Unfortunately, the airbrush stylus also sends wheel events
(there is a small wheel on it), but in the range [0..1023]. From the user
space point of view it is kind of difficult to understand that because
the wheel on the pad are quite common, while the airbrush tool is not.
A solution to fix all of these problems is to split out the pad device
from the stylus/touch. This decision makes more sense because the pad is
not linked to the absolute position of the finger or pen, and usually, the
events from the pad are filtered out by the compositor, which then convert
them into actions or keyboard shortcuts.
For backward compatibility with current xf86-input-wacom, the pad devices
still present the ABS_X, ABS_Y and ABS_MISC events, but they can be
completely ignored in the new implementation.
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
drivers/input/tablet/wacom.h | 2 ++
drivers/input/tablet/wacom_sys.c | 63 +++++++++++++++++++++++++++++++++++-----
drivers/input/tablet/wacom_wac.c | 27 ++++++++++++++++-
drivers/input/tablet/wacom_wac.h | 2 ++
4 files changed, 85 insertions(+), 9 deletions(-)
diff --git a/drivers/input/tablet/wacom.h b/drivers/input/tablet/wacom.h
index 9ebf0ed..caa59ca 100644
--- a/drivers/input/tablet/wacom.h
+++ b/drivers/input/tablet/wacom.h
@@ -136,4 +136,6 @@ void wacom_wac_irq(struct wacom_wac *wacom_wac, size_t len);
void wacom_setup_device_quirks(struct wacom_features *features);
int wacom_setup_input_capabilities(struct input_dev *input_dev,
struct wacom_wac *wacom_wac);
+int wacom_setup_pad_input_capabilities(struct input_dev *input_dev,
+ struct wacom_wac *wacom_wac);
#endif
diff --git a/drivers/input/tablet/wacom_sys.c b/drivers/input/tablet/wacom_sys.c
index c993eee..b9bf37e 100644
--- a/drivers/input/tablet/wacom_sys.c
+++ b/drivers/input/tablet/wacom_sys.c
@@ -135,6 +135,9 @@ static int wacom_open(struct input_dev *dev)
mutex_lock(&wacom->lock);
+ if (wacom->open)
+ goto out;
+
if (usb_submit_urb(wacom->irq, GFP_KERNEL)) {
retval = -EIO;
goto out;
@@ -157,9 +160,14 @@ static void wacom_close(struct input_dev *dev)
autopm_error = usb_autopm_get_interface(wacom->intf);
mutex_lock(&wacom->lock);
+ if (!wacom->open)
+ goto out;
+
usb_kill_urb(wacom->irq);
wacom->open = false;
wacom->intf->needs_remote_wakeup = 0;
+
+out:
mutex_unlock(&wacom->lock);
if (!autopm_error)
@@ -1109,19 +1117,16 @@ static void wacom_destroy_battery(struct wacom *wacom)
}
}
-static int wacom_register_input(struct wacom *wacom)
+static struct input_dev *wacom_allocate_input(struct wacom *wacom)
{
struct input_dev *input_dev;
struct usb_interface *intf = wacom->intf;
struct usb_device *dev = interface_to_usbdev(intf);
struct wacom_wac *wacom_wac = &(wacom->wacom_wac);
- int error;
input_dev = input_allocate_device();
- if (!input_dev) {
- error = -ENOMEM;
- goto fail1;
- }
+ if (!input_dev)
+ return NULL;
input_dev->name = wacom_wac->name;
input_dev->phys = wacom->phys;
@@ -1131,21 +1136,59 @@ static int wacom_register_input(struct wacom *wacom)
usb_to_input_id(dev, &input_dev->id);
input_set_drvdata(input_dev, wacom);
+ return input_dev;
+}
+
+static int wacom_register_input(struct wacom *wacom)
+{
+ struct input_dev *input_dev, *pad_input_dev;
+ struct wacom_wac *wacom_wac = &(wacom->wacom_wac);
+ int error;
+
+ input_dev = wacom_allocate_input(wacom);
+ pad_input_dev = wacom_allocate_input(wacom);
+ if (!input_dev || !pad_input_dev) {
+ error = -ENOMEM;
+ goto fail1;
+ }
+
wacom_wac->input = input_dev;
+ wacom_wac->pad_input = pad_input_dev;
+ wacom_wac->pad_input->name = wacom_wac->pad_name;
+
error = wacom_setup_input_capabilities(input_dev, wacom_wac);
if (error)
- goto fail1;
+ goto fail2;
error = input_register_device(input_dev);
if (error)
goto fail2;
+ error = wacom_setup_pad_input_capabilities(pad_input_dev, wacom_wac);
+ if (error) {
+ /* no pad in use on this interface */
+ input_free_device(pad_input_dev);
+ wacom_wac->pad_input = NULL;
+ pad_input_dev = NULL;
+ } else {
+ error = input_register_device(pad_input_dev);
+ if (error)
+ goto fail3;
+ }
+
return 0;
+fail3:
+ input_unregister_device(input_dev);
+ input_dev = NULL;
fail2:
- input_free_device(input_dev);
wacom_wac->input = NULL;
+ wacom_wac->pad_input = NULL;
fail1:
+ if (input_dev)
+ input_free_device(input_dev);
+ if (pad_input_dev)
+ input_free_device(pad_input_dev);
return error;
}
@@ -1364,6 +1407,8 @@ static int wacom_probe(struct usb_interface *intf, const struct usb_device_id *i
wacom_calculate_res(features);
strlcpy(wacom_wac->name, features->name, sizeof(wacom_wac->name));
+ snprintf(wacom_wac->pad_name, sizeof(wacom_wac->pad_name),
+ "%s Pad", features->name);
if (features->quirks & WACOM_QUIRK_MULTI_INPUT) {
struct usb_device *other_dev;
@@ -1438,6 +1483,8 @@ static void wacom_disconnect(struct usb_interface *intf)
cancel_work_sync(&wacom->work);
if (wacom->wacom_wac.input)
input_unregister_device(wacom->wacom_wac.input);
+ if (wacom->wacom_wac.pad_input)
+ input_unregister_device(wacom->wacom_wac.pad_input);
wacom_destroy_battery(wacom);
wacom_destroy_leds(wacom);
usb_free_urb(wacom->irq);
diff --git a/drivers/input/tablet/wacom_wac.c b/drivers/input/tablet/wacom_wac.c
index 977d05c..4b16a34 100644
--- a/drivers/input/tablet/wacom_wac.c
+++ b/drivers/input/tablet/wacom_wac.c
@@ -1489,8 +1489,11 @@ void wacom_wac_irq(struct wacom_wac *wacom_wac, size_t len)
break;
}
- if (sync)
+ if (sync) {
input_sync(wacom_wac->input);
+ if (wacom_wac->pad_input)
+ input_sync(wacom_wac->pad_input);
+ }
}
static void wacom_setup_cintiq(struct wacom_wac *wacom_wac)
@@ -1939,6 +1942,28 @@ int wacom_setup_input_capabilities(struct input_dev *input_dev,
return 0;
}
+int wacom_setup_pad_input_capabilities(struct input_dev *input_dev,
+ struct wacom_wac *wacom_wac)
+{
+ struct wacom_features *features = &wacom_wac->features;
+
+ input_dev->evbit[0] |= BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
+
+ /* kept for making legacy xf86-input-wacom working with the wheels */
+ __set_bit(ABS_MISC, input_dev->absbit);
+
+ /* kept for making legacy xf86-input-wacom accepting the pad */
+ input_set_abs_params(input_dev, ABS_X, 0, 1, 0, 0);
+ input_set_abs_params(input_dev, ABS_Y, 0, 1, 0, 0);
+
+ switch (features->type) {
+ default:
+ /* no pad supported */
+ return 1;
+ }
+ return 0;
+}
+
static const struct wacom_features wacom_features_0x00 =
{ "Wacom Penpartner", WACOM_PKGLEN_PENPRTN, 5040, 3780, 255,
0, PENPARTNER, WACOM_PENPRTN_RES, WACOM_PENPRTN_RES };
diff --git a/drivers/input/tablet/wacom_wac.h b/drivers/input/tablet/wacom_wac.h
index b2c9a9c..f48164c 100644
--- a/drivers/input/tablet/wacom_wac.h
+++ b/drivers/input/tablet/wacom_wac.h
@@ -150,6 +150,7 @@ struct wacom_shared {
struct wacom_wac {
char name[WACOM_NAME_MAX];
+ char pad_name[WACOM_NAME_MAX];
unsigned char *data;
int tool[2];
int id[2];
@@ -157,6 +158,7 @@ struct wacom_wac {
struct wacom_features features;
struct wacom_shared *shared;
struct input_dev *input;
+ struct input_dev *pad_input;
int pid;
int battery_capacity;
int num_contacts_left;
--
1.9.0
^ permalink raw reply related
* [PATCH 2/5] Input - wacom: split out the pad device for Intuos/Cintiq
From: Benjamin Tissoires @ 2014-06-23 20:57 UTC (permalink / raw)
To: Dmitry Torokhov, Ping Cheng, Jason Gerecke
Cc: linux-input, linux-kernel, linuxwacom-devel
In-Reply-To: <1403557039-1487-1-git-send-email-benjamin.tissoires@redhat.com>
MSC_SERIAL can be safely dropped for pad input devices.
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
drivers/input/tablet/wacom_wac.c | 208 +++++++++++++++++++++++----------------
1 file changed, 122 insertions(+), 86 deletions(-)
diff --git a/drivers/input/tablet/wacom_wac.c b/drivers/input/tablet/wacom_wac.c
index 4b16a34..8807ab5 100644
--- a/drivers/input/tablet/wacom_wac.c
+++ b/drivers/input/tablet/wacom_wac.c
@@ -584,6 +584,7 @@ static int wacom_intuos_irq(struct wacom_wac *wacom)
/* pad packets. Works as a second tool and is always in prox */
if (data[0] == WACOM_REPORT_INTUOSPAD || data[0] == WACOM_REPORT_INTUOS5PAD) {
+ input = wacom->pad_input;
if (features->type >= INTUOS4S && features->type <= INTUOS4L) {
input_report_key(input, BTN_0, (data[2] & 0x01));
input_report_key(input, BTN_1, (data[3] & 0x01));
@@ -773,7 +774,6 @@ static int wacom_intuos_irq(struct wacom_wac *wacom)
input_report_abs(input, ABS_MISC, 0);
}
}
- input_event(input, EV_MSC, MSC_SERIAL, 0xffffffff);
return 1;
}
@@ -1656,61 +1656,20 @@ int wacom_setup_input_capabilities(struct input_dev *input_dev,
break;
case WACOM_24HD:
- __set_bit(BTN_A, input_dev->keybit);
- __set_bit(BTN_B, input_dev->keybit);
- __set_bit(BTN_C, input_dev->keybit);
- __set_bit(BTN_X, input_dev->keybit);
- __set_bit(BTN_Y, input_dev->keybit);
- __set_bit(BTN_Z, input_dev->keybit);
-
- for (i = 6; i < 10; i++)
- __set_bit(BTN_0 + i, input_dev->keybit);
-
- __set_bit(KEY_PROG1, input_dev->keybit);
- __set_bit(KEY_PROG2, input_dev->keybit);
- __set_bit(KEY_PROG3, input_dev->keybit);
-
input_set_abs_params(input_dev, ABS_Z, -900, 899, 0, 0);
input_set_abs_params(input_dev, ABS_THROTTLE, 0, 71, 0, 0);
/* fall through */
case DTK:
- for (i = 0; i < 6; i++)
- __set_bit(BTN_0 + i, input_dev->keybit);
-
__set_bit(INPUT_PROP_DIRECT, input_dev->propbit);
wacom_setup_cintiq(wacom_wac);
break;
case WACOM_22HD:
- __set_bit(KEY_PROG1, input_dev->keybit);
- __set_bit(KEY_PROG2, input_dev->keybit);
- __set_bit(KEY_PROG3, input_dev->keybit);
- /* fall through */
-
case WACOM_21UX2:
- __set_bit(BTN_A, input_dev->keybit);
- __set_bit(BTN_B, input_dev->keybit);
- __set_bit(BTN_C, input_dev->keybit);
- __set_bit(BTN_X, input_dev->keybit);
- __set_bit(BTN_Y, input_dev->keybit);
- __set_bit(BTN_Z, input_dev->keybit);
- __set_bit(BTN_BASE, input_dev->keybit);
- __set_bit(BTN_BASE2, input_dev->keybit);
- /* fall through */
-
case WACOM_BEE:
- __set_bit(BTN_8, input_dev->keybit);
- __set_bit(BTN_9, input_dev->keybit);
- /* fall through */
-
case CINTIQ:
- for (i = 0; i < 8; i++)
- __set_bit(BTN_0 + i, input_dev->keybit);
-
- input_set_abs_params(input_dev, ABS_RX, 0, 4096, 0, 0);
- input_set_abs_params(input_dev, ABS_RY, 0, 4096, 0, 0);
input_set_abs_params(input_dev, ABS_Z, -900, 899, 0, 0);
__set_bit(INPUT_PROP_DIRECT, input_dev->propbit);
@@ -1719,9 +1678,6 @@ int wacom_setup_input_capabilities(struct input_dev *input_dev,
break;
case WACOM_13HD:
- for (i = 0; i < 9; i++)
- __set_bit(BTN_0 + i, input_dev->keybit);
-
input_set_abs_params(input_dev, ABS_Z, -900, 899, 0, 0);
__set_bit(INPUT_PROP_DIRECT, input_dev->propbit);
wacom_setup_cintiq(wacom_wac);
@@ -1729,21 +1685,7 @@ int wacom_setup_input_capabilities(struct input_dev *input_dev,
case INTUOS3:
case INTUOS3L:
- __set_bit(BTN_4, input_dev->keybit);
- __set_bit(BTN_5, input_dev->keybit);
- __set_bit(BTN_6, input_dev->keybit);
- __set_bit(BTN_7, input_dev->keybit);
-
- input_set_abs_params(input_dev, ABS_RY, 0, 4096, 0, 0);
- /* fall through */
-
case INTUOS3S:
- __set_bit(BTN_0, input_dev->keybit);
- __set_bit(BTN_1, input_dev->keybit);
- __set_bit(BTN_2, input_dev->keybit);
- __set_bit(BTN_3, input_dev->keybit);
-
- input_set_abs_params(input_dev, ABS_RX, 0, 4096, 0, 0);
input_set_abs_params(input_dev, ABS_Z, -900, 899, 0, 0);
/* fall through */
@@ -1757,20 +1699,11 @@ int wacom_setup_input_capabilities(struct input_dev *input_dev,
case INTUOS5L:
case INTUOSPM:
case INTUOSPL:
- if (features->device_type == BTN_TOOL_PEN) {
- __set_bit(BTN_7, input_dev->keybit);
- __set_bit(BTN_8, input_dev->keybit);
- }
- /* fall through */
-
case INTUOS5S:
case INTUOSPS:
__set_bit(INPUT_PROP_POINTER, input_dev->propbit);
if (features->device_type == BTN_TOOL_PEN) {
- for (i = 0; i < 7; i++)
- __set_bit(BTN_0 + i, input_dev->keybit);
-
input_set_abs_params(input_dev, ABS_DISTANCE, 0,
features->distance_max,
0, 0);
@@ -1791,14 +1724,7 @@ int wacom_setup_input_capabilities(struct input_dev *input_dev,
case INTUOS4:
case INTUOS4L:
- __set_bit(BTN_7, input_dev->keybit);
- __set_bit(BTN_8, input_dev->keybit);
- /* fall through */
-
case INTUOS4S:
- for (i = 0; i < 7; i++)
- __set_bit(BTN_0 + i, input_dev->keybit);
-
input_set_abs_params(input_dev, ABS_Z, -900, 899, 0, 0);
wacom_setup_intuos(wacom_wac);
@@ -1922,17 +1848,6 @@ int wacom_setup_input_capabilities(struct input_dev *input_dev,
break;
case CINTIQ_HYBRID:
- __set_bit(BTN_1, input_dev->keybit);
- __set_bit(BTN_2, input_dev->keybit);
- __set_bit(BTN_3, input_dev->keybit);
- __set_bit(BTN_4, input_dev->keybit);
-
- __set_bit(BTN_5, input_dev->keybit);
- __set_bit(BTN_6, input_dev->keybit);
- __set_bit(BTN_7, input_dev->keybit);
- __set_bit(BTN_8, input_dev->keybit);
- __set_bit(BTN_0, input_dev->keybit);
-
input_set_abs_params(input_dev, ABS_Z, -900, 899, 0, 0);
__set_bit(INPUT_PROP_DIRECT, input_dev->propbit);
@@ -1946,6 +1861,7 @@ int wacom_setup_pad_input_capabilities(struct input_dev *input_dev,
struct wacom_wac *wacom_wac)
{
struct wacom_features *features = &wacom_wac->features;
+ int i;
input_dev->evbit[0] |= BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
@@ -1957,6 +1873,126 @@ int wacom_setup_pad_input_capabilities(struct input_dev *input_dev,
input_set_abs_params(input_dev, ABS_Y, 0, 1, 0, 0);
switch (features->type) {
+ case WACOM_24HD:
+ __set_bit(BTN_A, input_dev->keybit);
+ __set_bit(BTN_B, input_dev->keybit);
+ __set_bit(BTN_C, input_dev->keybit);
+ __set_bit(BTN_X, input_dev->keybit);
+ __set_bit(BTN_Y, input_dev->keybit);
+ __set_bit(BTN_Z, input_dev->keybit);
+
+ for (i = 0; i < 10; i++)
+ __set_bit(BTN_0 + i, input_dev->keybit);
+
+ __set_bit(KEY_PROG1, input_dev->keybit);
+ __set_bit(KEY_PROG2, input_dev->keybit);
+ __set_bit(KEY_PROG3, input_dev->keybit);
+
+ input_set_abs_params(input_dev, ABS_WHEEL, 0, 71, 0, 0);
+ input_set_abs_params(input_dev, ABS_THROTTLE, 0, 71, 0, 0);
+ break;
+
+ case DTK:
+ for (i = 0; i < 6; i++)
+ __set_bit(BTN_0 + i, input_dev->keybit);
+
+ break;
+
+ case WACOM_22HD:
+ __set_bit(KEY_PROG1, input_dev->keybit);
+ __set_bit(KEY_PROG2, input_dev->keybit);
+ __set_bit(KEY_PROG3, input_dev->keybit);
+ /* fall through */
+
+ case WACOM_21UX2:
+ __set_bit(BTN_A, input_dev->keybit);
+ __set_bit(BTN_B, input_dev->keybit);
+ __set_bit(BTN_C, input_dev->keybit);
+ __set_bit(BTN_X, input_dev->keybit);
+ __set_bit(BTN_Y, input_dev->keybit);
+ __set_bit(BTN_Z, input_dev->keybit);
+ __set_bit(BTN_BASE, input_dev->keybit);
+ __set_bit(BTN_BASE2, input_dev->keybit);
+ /* fall through */
+
+ case WACOM_BEE:
+ __set_bit(BTN_8, input_dev->keybit);
+ __set_bit(BTN_9, input_dev->keybit);
+ /* fall through */
+
+ case CINTIQ:
+ for (i = 0; i < 8; i++)
+ __set_bit(BTN_0 + i, input_dev->keybit);
+
+ input_set_abs_params(input_dev, ABS_RX, 0, 4096, 0, 0);
+ input_set_abs_params(input_dev, ABS_RY, 0, 4096, 0, 0);
+ break;
+
+ case WACOM_13HD:
+ for (i = 0; i < 9; i++)
+ __set_bit(BTN_0 + i, input_dev->keybit);
+
+ input_set_abs_params(input_dev, ABS_WHEEL, 0, 71, 0, 0);
+ break;
+
+ case INTUOS3:
+ case INTUOS3L:
+ __set_bit(BTN_4, input_dev->keybit);
+ __set_bit(BTN_5, input_dev->keybit);
+ __set_bit(BTN_6, input_dev->keybit);
+ __set_bit(BTN_7, input_dev->keybit);
+
+ input_set_abs_params(input_dev, ABS_RY, 0, 4096, 0, 0);
+ /* fall through */
+
+ case INTUOS3S:
+ __set_bit(BTN_0, input_dev->keybit);
+ __set_bit(BTN_1, input_dev->keybit);
+ __set_bit(BTN_2, input_dev->keybit);
+ __set_bit(BTN_3, input_dev->keybit);
+
+ input_set_abs_params(input_dev, ABS_RX, 0, 4096, 0, 0);
+ break;
+
+ case INTUOS5:
+ case INTUOS5L:
+ case INTUOSPM:
+ case INTUOSPL:
+ __set_bit(BTN_7, input_dev->keybit);
+ __set_bit(BTN_8, input_dev->keybit);
+ /* fall through */
+
+ case INTUOS5S:
+ case INTUOSPS:
+ /* touch interface does not have the pad device */
+ if (features->device_type != BTN_TOOL_PEN)
+ return 1;
+
+ for (i = 0; i < 7; i++)
+ __set_bit(BTN_0 + i, input_dev->keybit);
+
+ input_set_abs_params(input_dev, ABS_WHEEL, 0, 71, 0, 0);
+ break;
+
+ case INTUOS4:
+ case INTUOS4L:
+ __set_bit(BTN_7, input_dev->keybit);
+ __set_bit(BTN_8, input_dev->keybit);
+ /* fall through */
+
+ case INTUOS4S:
+ for (i = 0; i < 7; i++)
+ __set_bit(BTN_0 + i, input_dev->keybit);
+
+ input_set_abs_params(input_dev, ABS_WHEEL, 0, 71, 0, 0);
+ break;
+
+ case CINTIQ_HYBRID:
+ for (i = 0; i < 9; i++)
+ __set_bit(BTN_0 + i, input_dev->keybit);
+
+ break;
+
default:
/* no pad supported */
return 1;
--
1.9.0
^ permalink raw reply related
* [PATCH 5/5] Input - wacom: split out the pad device for Graphire G4 and MO
From: Benjamin Tissoires @ 2014-06-23 20:57 UTC (permalink / raw)
To: Dmitry Torokhov, Ping Cheng, Jason Gerecke
Cc: linux-input, linux-kernel, linuxwacom-devel
In-Reply-To: <1403557039-1487-1-git-send-email-benjamin.tissoires@redhat.com>
MSC_SERIAL can be safely removed from pad devices. If it is not
here, xf86-input-wacom correctly generates ones for its internal
use.
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
drivers/input/tablet/wacom_wac.c | 44 ++++++++++++++++++++++++----------------
1 file changed, 27 insertions(+), 17 deletions(-)
diff --git a/drivers/input/tablet/wacom_wac.c b/drivers/input/tablet/wacom_wac.c
index d3aef3a..f170277 100644
--- a/drivers/input/tablet/wacom_wac.c
+++ b/drivers/input/tablet/wacom_wac.c
@@ -262,6 +262,7 @@ static int wacom_graphire_irq(struct wacom_wac *wacom)
struct wacom_features *features = &wacom->features;
unsigned char *data = wacom->data;
struct input_dev *input = wacom->input;
+ struct input_dev *pad_input = wacom->pad_input;
int prox;
int rw = 0;
int retval = 0;
@@ -322,7 +323,6 @@ static int wacom_graphire_irq(struct wacom_wac *wacom)
wacom->id[0] = 0;
input_report_abs(input, ABS_MISC, wacom->id[0]); /* report tool id */
input_report_key(input, wacom->tool[0], prox);
- input_event(input, EV_MSC, MSC_SERIAL, 1);
input_sync(input); /* sync last event */
}
@@ -332,14 +332,13 @@ static int wacom_graphire_irq(struct wacom_wac *wacom)
prox = data[7] & 0xf8;
if (prox || wacom->id[1]) {
wacom->id[1] = PAD_DEVICE_ID;
- input_report_key(input, BTN_BACK, (data[7] & 0x40));
- input_report_key(input, BTN_FORWARD, (data[7] & 0x80));
+ input_report_key(pad_input, BTN_BACK, (data[7] & 0x40));
+ input_report_key(pad_input, BTN_FORWARD, (data[7] & 0x80));
rw = ((data[7] & 0x18) >> 3) - ((data[7] & 0x20) >> 3);
- input_report_rel(input, REL_WHEEL, rw);
+ input_report_rel(pad_input, REL_WHEEL, rw);
if (!prox)
wacom->id[1] = 0;
- input_report_abs(input, ABS_MISC, wacom->id[1]);
- input_event(input, EV_MSC, MSC_SERIAL, 0xf0);
+ input_report_abs(pad_input, ABS_MISC, wacom->id[1]);
retval = 1;
}
break;
@@ -348,15 +347,14 @@ static int wacom_graphire_irq(struct wacom_wac *wacom)
prox = (data[7] & 0xf8) || data[8];
if (prox || wacom->id[1]) {
wacom->id[1] = PAD_DEVICE_ID;
- input_report_key(input, BTN_BACK, (data[7] & 0x08));
- input_report_key(input, BTN_LEFT, (data[7] & 0x20));
- input_report_key(input, BTN_FORWARD, (data[7] & 0x10));
- input_report_key(input, BTN_RIGHT, (data[7] & 0x40));
- input_report_abs(input, ABS_WHEEL, (data[8] & 0x7f));
+ input_report_key(pad_input, BTN_BACK, (data[7] & 0x08));
+ input_report_key(pad_input, BTN_LEFT, (data[7] & 0x20));
+ input_report_key(pad_input, BTN_FORWARD, (data[7] & 0x10));
+ input_report_key(pad_input, BTN_RIGHT, (data[7] & 0x40));
+ input_report_abs(pad_input, ABS_WHEEL, (data[8] & 0x7f));
if (!prox)
wacom->id[1] = 0;
- input_report_abs(input, ABS_MISC, wacom->id[1]);
- input_event(input, EV_MSC, MSC_SERIAL, 0xf0);
+ input_report_abs(pad_input, ABS_MISC, wacom->id[1]);
retval = 1;
}
break;
@@ -1624,10 +1622,6 @@ int wacom_setup_input_capabilities(struct input_dev *input_dev,
/* fall through */
case WACOM_G4:
- input_set_capability(input_dev, EV_MSC, MSC_SERIAL);
-
- __set_bit(BTN_BACK, input_dev->keybit);
- __set_bit(BTN_FORWARD, input_dev->keybit);
/* fall through */
case GRAPHIRE:
@@ -1854,6 +1848,22 @@ int wacom_setup_pad_input_capabilities(struct input_dev *input_dev,
input_set_abs_params(input_dev, ABS_Y, 0, 1, 0, 0);
switch (features->type) {
+ case WACOM_MO:
+ __set_bit(BTN_BACK, input_dev->keybit);
+ __set_bit(BTN_LEFT, input_dev->keybit);
+ __set_bit(BTN_FORWARD, input_dev->keybit);
+ __set_bit(BTN_RIGHT, input_dev->keybit);
+ input_set_abs_params(input_dev, ABS_WHEEL, 0, 71, 0, 0);
+ break;
+
+ case WACOM_G4:
+ __set_bit(BTN_BACK, input_dev->keybit);
+ __set_bit(BTN_LEFT, input_dev->keybit);
+ __set_bit(BTN_FORWARD, input_dev->keybit);
+ __set_bit(BTN_RIGHT, input_dev->keybit);
+ input_set_capability(input_dev, EV_REL, REL_WHEEL);
+ break;
+
case WACOM_24HD:
__set_bit(BTN_A, input_dev->keybit);
__set_bit(BTN_B, input_dev->keybit);
--
1.9.0
^ permalink raw reply related
* [PATCH 0/5] Input - wacom: split out pad data from the stylus input device
From: Benjamin Tissoires @ 2014-06-23 20:57 UTC (permalink / raw)
To: Dmitry Torokhov, Ping Cheng, Jason Gerecke
Cc: linux-input, linux-kernel, linuxwacom-devel
Hi,
This patch series aims at simplifying the handling of Wacom devices from the user
space point of view. As mentioned in the commit message from 1/5, the pad data
are interleaved into the stylus input for some devices, and into the touch input
for others.
It doesn't makes real sense to interleave pad with stylus because pad data are
not linked to the absolute position of a stylus (or its presence). It is a
device by itself which should have its own input node.
It will also help the handling of the old Wacom Graphire 4 which shares its
BTN_LEFT and BTN_RIGHT in its pad and in the mouse.
Again, it's not a problem right now because the user space is currently dealing
with it, but for the Wayland libinput implementation, having something clear
will really help.
This patch series does not break current X11 behavior, the only change from the
user will be that an input device will appear as "Wacom ... Pad pad".
I tried to test/debug it as extensively as possible, but I do not guarantee any
mistake :(
Tested on:
- Bamboo 16FG 4x5
- Bamboo 2FG
- BambooFun 4x5
- Bamboo
- Cintiq 12WX
- Cintiq 21UX2
- Cintiq 21UX
- Cintiq 22HD
- Cintiq 22HDT
- Cintiq 24HD
- DTU2231
- Graphire3
- Intuos3 9x12
- Intuos4 WL
- Intuos5 touch M
- Intuos Pro S
Note: this patch series might requesting to apply the patch for input->phys
I just sent:
https://patchwork.kernel.org/patch/4404391/
Cheers,
Benjamin
Benjamin Tissoires (5):
Input - wacom: create a separate input device for pads
Input - wacom: split out the pad device for Intuos/Cintiq
Input - wacom: split out the pad device for Bamboos
Input - wacom: split out the pad device for DTUS
Input - wacom: split out the pad device for Graphire G4 and MO
drivers/input/tablet/wacom.h | 2 +
drivers/input/tablet/wacom_sys.c | 63 +++++++-
drivers/input/tablet/wacom_wac.c | 328 ++++++++++++++++++++++++---------------
drivers/input/tablet/wacom_wac.h | 2 +
4 files changed, 259 insertions(+), 136 deletions(-)
--
1.9.0
^ permalink raw reply
* [PATCH 3/5] Input - wacom: split out the pad device for Bamboos
From: Benjamin Tissoires @ 2014-06-23 20:57 UTC (permalink / raw)
To: Dmitry Torokhov, Ping Cheng, Jason Gerecke
Cc: linux-input, linux-kernel, linuxwacom-devel
In-Reply-To: <1403557039-1487-1-git-send-email-benjamin.tissoires@redhat.com>
We rely on the return code of wacom_bpt*() to do the input_sync().
wacom_wac_irq() then properly sync the input devices.
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
drivers/input/tablet/wacom_wac.c | 39 +++++++++++++++++++++++----------------
1 file changed, 23 insertions(+), 16 deletions(-)
diff --git a/drivers/input/tablet/wacom_wac.c b/drivers/input/tablet/wacom_wac.c
index 8807ab5..5aaa3d2 100644
--- a/drivers/input/tablet/wacom_wac.c
+++ b/drivers/input/tablet/wacom_wac.c
@@ -1143,6 +1143,7 @@ static int wacom_bpt_touch(struct wacom_wac *wacom)
{
struct wacom_features *features = &wacom->features;
struct input_dev *input = wacom->input;
+ struct input_dev *pad_input = wacom->pad_input;
unsigned char *data = wacom->data;
int i;
@@ -1177,14 +1178,12 @@ static int wacom_bpt_touch(struct wacom_wac *wacom)
input_mt_report_pointer_emulation(input, true);
- input_report_key(input, BTN_LEFT, (data[1] & 0x08) != 0);
- input_report_key(input, BTN_FORWARD, (data[1] & 0x04) != 0);
- input_report_key(input, BTN_BACK, (data[1] & 0x02) != 0);
- input_report_key(input, BTN_RIGHT, (data[1] & 0x01) != 0);
-
- input_sync(input);
+ input_report_key(pad_input, BTN_LEFT, (data[1] & 0x08) != 0);
+ input_report_key(pad_input, BTN_FORWARD, (data[1] & 0x04) != 0);
+ input_report_key(pad_input, BTN_BACK, (data[1] & 0x02) != 0);
+ input_report_key(pad_input, BTN_RIGHT, (data[1] & 0x01) != 0);
- return 0;
+ return 1;
}
static void wacom_bpt3_touch_msg(struct wacom_wac *wacom, unsigned char *data)
@@ -1232,7 +1231,7 @@ static void wacom_bpt3_touch_msg(struct wacom_wac *wacom, unsigned char *data)
static void wacom_bpt3_button_msg(struct wacom_wac *wacom, unsigned char *data)
{
- struct input_dev *input = wacom->input;
+ struct input_dev *input = wacom->pad_input;
struct wacom_features *features = &wacom->features;
if (features->type == INTUOSHT) {
@@ -1269,9 +1268,7 @@ static int wacom_bpt3_touch(struct wacom_wac *wacom)
}
input_mt_report_pointer_emulation(input, true);
- input_sync(input);
-
- return 0;
+ return 1;
}
static int wacom_bpt_pen(struct wacom_wac *wacom)
@@ -1806,11 +1803,6 @@ int wacom_setup_input_capabilities(struct input_dev *input_dev,
if (features->device_type == BTN_TOOL_FINGER) {
- __set_bit(BTN_LEFT, input_dev->keybit);
- __set_bit(BTN_FORWARD, input_dev->keybit);
- __set_bit(BTN_BACK, input_dev->keybit);
- __set_bit(BTN_RIGHT, input_dev->keybit);
-
if (features->touch_max) {
/* touch interface */
unsigned int flags = INPUT_MT_POINTER;
@@ -1993,6 +1985,21 @@ int wacom_setup_pad_input_capabilities(struct input_dev *input_dev,
break;
+ case INTUOSHT:
+ case BAMBOO_PT:
+ /* pad device is on the touch interface */
+ if (features->device_type != BTN_TOOL_FINGER)
+ return 1;
+
+ __clear_bit(ABS_MISC, input_dev->absbit);
+
+ __set_bit(BTN_LEFT, input_dev->keybit);
+ __set_bit(BTN_FORWARD, input_dev->keybit);
+ __set_bit(BTN_BACK, input_dev->keybit);
+ __set_bit(BTN_RIGHT, input_dev->keybit);
+
+ break;
+
default:
/* no pad supported */
return 1;
--
1.9.0
^ permalink raw reply related
* [PATCH 4/5] Input - wacom: split out the pad device for DTUS
From: Benjamin Tissoires @ 2014-06-23 20:57 UTC (permalink / raw)
To: Dmitry Torokhov, Ping Cheng, Jason Gerecke
Cc: linux-input, linux-kernel, linuxwacom-devel
In-Reply-To: <1403557039-1487-1-git-send-email-benjamin.tissoires@redhat.com>
MSC_SERIAL can be safely removed from the pad device.
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
drivers/input/tablet/wacom_wac.c | 18 ++++++------------
1 file changed, 6 insertions(+), 12 deletions(-)
diff --git a/drivers/input/tablet/wacom_wac.c b/drivers/input/tablet/wacom_wac.c
index 5aaa3d2..d3aef3a 100644
--- a/drivers/input/tablet/wacom_wac.c
+++ b/drivers/input/tablet/wacom_wac.c
@@ -217,17 +217,13 @@ static int wacom_dtus_irq(struct wacom_wac *wacom)
"%s: received unknown report #%d", __func__, data[0]);
return 0;
} else if (data[0] == WACOM_REPORT_DTUSPAD) {
+ input = wacom->pad_input;
input_report_key(input, BTN_0, (data[1] & 0x01));
input_report_key(input, BTN_1, (data[1] & 0x02));
input_report_key(input, BTN_2, (data[1] & 0x04));
input_report_key(input, BTN_3, (data[1] & 0x08));
input_report_abs(input, ABS_MISC,
data[1] & 0x0f ? PAD_DEVICE_ID : 0);
- /*
- * Serial number is required when expresskeys are
- * reported through pen interface.
- */
- input_event(input, EV_MSC, MSC_SERIAL, 0xf0);
return 1;
} else {
prox = data[1] & 0x80;
@@ -257,7 +253,6 @@ static int wacom_dtus_irq(struct wacom_wac *wacom)
wacom->id[0] = 0;
input_report_key(input, wacom->tool[0], prox);
input_report_abs(input, ABS_MISC, wacom->id[0]);
- input_event(input, EV_MSC, MSC_SERIAL, 1);
return 1;
}
}
@@ -1615,7 +1610,6 @@ int wacom_setup_input_capabilities(struct input_dev *input_dev,
struct wacom_wac *wacom_wac)
{
struct wacom_features *features = &wacom_wac->features;
- int i;
input_dev->evbit[0] |= BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
@@ -1765,11 +1759,6 @@ int wacom_setup_input_capabilities(struct input_dev *input_dev,
case DTUS:
case PL:
case DTU:
- if (features->type == DTUS) {
- input_set_capability(input_dev, EV_MSC, MSC_SERIAL);
- for (i = 0; i < 4; i++)
- __set_bit(BTN_0 + i, input_dev->keybit);
- }
__set_bit(BTN_TOOL_PEN, input_dev->keybit);
__set_bit(BTN_TOOL_RUBBER, input_dev->keybit);
__set_bit(BTN_STYLUS, input_dev->keybit);
@@ -1985,6 +1974,11 @@ int wacom_setup_pad_input_capabilities(struct input_dev *input_dev,
break;
+ case DTUS:
+ for (i = 0; i < 4; i++)
+ __set_bit(BTN_0 + i, input_dev->keybit);
+ break;
+
case INTUOSHT:
case BAMBOO_PT:
/* pad device is on the touch interface */
--
1.9.0
^ permalink raw reply related
* Re: [PATCH] GPIO button wth wakeup attribute is supposed to wake the system up
From: Li, Aubrey @ 2014-06-23 23:35 UTC (permalink / raw)
To: One Thousand Gnomes
Cc: dmitry.torokhov@gmail.com, Rafael J. Wysocki,
linux-input@vger.kernel.org, LKML
In-Reply-To: <20140619114006.72ac8e06@alan.etchedpixels.co.uk>
ping...
On 2014/6/19 18:40, One Thousand Gnomes wrote:
> On Thu, 19 Jun 2014 08:51:25 +0800
> "Li, Aubrey" <aubrey.li@linux.intel.com> wrote:
>
>> When the wakeup attribute is set, the GPIO button is capable of
>> waking up the system from sleep states, including the "freeze"
>> sleep state. For that to work, its driver needs to pass the
>> IRQF_NO_SUSPEND flag to devm_request_any_context_irq(), or the
>> interrupt will be disabled by suspend_device_irqs() and the
>> system won't be woken up by it from the "freeze" sleep state.
>>
>> The suspend_device_irqs() routine is a workaround for drivers
>> that mishandle interrupts triggered when the devices handled
>> by them are suspended, so it is safe to use IRQF_NO_SUSPEND in
>> all drivers that don't have that problem.
>>
>> The affected/tested machines include Dell Venue 11 Pro and Asus T100TA.
>>
>> Signed-off-by: Aubrey Li <aubrey.li@linux.intel.com>
>> Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> Reviewed-by: Alan Cox <alan@linux.intel.com>
>
>
>
^ permalink raw reply
* Re: [PATCH 1/5] Input - wacom: create a separate input device for pads
From: Ping Cheng @ 2014-06-24 0:18 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Dmitry Torokhov, Jason Gerecke, linux-input,
linux-kernel@vger.kernel.org, linuxwacom-devel
In-Reply-To: <1403557039-1487-2-git-send-email-benjamin.tissoires@redhat.com>
Hi Benjamin,
On Mon, Jun 23, 2014 at 1:57 PM, Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
> Currently, the pad events are sent through the stylus input device
> for the Intuos/Cintiqs, and through the touch input device for the
> Bamboos.
>
> To differentiate the buttons pressed on the pad from the ones pressed
> on the stylus, the Intuos/Cintiq uses MISC_SERIAL and ABS_MISC. This
> lead to a multiplexing of the events into one device, which are then
> splitted out in xf86-input-wacom. Bamboos are not using MISC events
> because the pad is attached to the touch interface, and only BTN_TOUCH
> is used for the finger (and DOUBLE_TAP, etc...). However, the user space
> driver still splits out the pad from the touch interface in the same
> way it does for the pro line devices.
>
> The other problem we can see with this fact is that some of the Intuos
> and Cintiq have a wheel, and the effective range of the reported values
> is [0..71]. Unfortunately, the airbrush stylus also sends wheel events
> (there is a small wheel on it), but in the range [0..1023]. From the user
> space point of view it is kind of difficult to understand that because
> the wheel on the pad are quite common, while the airbrush tool is not.
>
> A solution to fix all of these problems is to split out the pad device
> from the stylus/touch. This decision makes more sense because the pad is
> not linked to the absolute position of the finger or pen, and usually, the
> events from the pad are filtered out by the compositor, which then convert
> them into actions or keyboard shortcuts.
This is a very good solution. I like it.
> For backward compatibility with current xf86-input-wacom, the pad devices
> still present the ABS_X, ABS_Y and ABS_MISC events, but they can be
> completely ignored in the new implementation.
I do not think we need to keep ABS_X and ABS_Y for pad. We've already
supported a tablet (Intuos pen small, a pen only tablet with buttons
reported on touch interface) that does not set ABS_X and ABS_Y for
pad.
Unless you plan to use other means to tell userland those events are
from PAD tool, ABS_MISC is necessary and is a reasonable way to group
PAD events. So, I do not think it is for backward compatibility. It is
there to stay. With that said, the whole patchset is
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Reviewed-by: Ping Cheng <pingc@wacom.com>
Thank you, Benjamin, for your support.
Ping
> ---
> drivers/input/tablet/wacom.h | 2 ++
> drivers/input/tablet/wacom_sys.c | 63 +++++++++++++++++++++++++++++++++++-----
> drivers/input/tablet/wacom_wac.c | 27 ++++++++++++++++-
> drivers/input/tablet/wacom_wac.h | 2 ++
> 4 files changed, 85 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/input/tablet/wacom.h b/drivers/input/tablet/wacom.h
> index 9ebf0ed..caa59ca 100644
> --- a/drivers/input/tablet/wacom.h
> +++ b/drivers/input/tablet/wacom.h
> @@ -136,4 +136,6 @@ void wacom_wac_irq(struct wacom_wac *wacom_wac, size_t len);
> void wacom_setup_device_quirks(struct wacom_features *features);
> int wacom_setup_input_capabilities(struct input_dev *input_dev,
> struct wacom_wac *wacom_wac);
> +int wacom_setup_pad_input_capabilities(struct input_dev *input_dev,
> + struct wacom_wac *wacom_wac);
> #endif
> diff --git a/drivers/input/tablet/wacom_sys.c b/drivers/input/tablet/wacom_sys.c
> index c993eee..b9bf37e 100644
> --- a/drivers/input/tablet/wacom_sys.c
> +++ b/drivers/input/tablet/wacom_sys.c
> @@ -135,6 +135,9 @@ static int wacom_open(struct input_dev *dev)
>
> mutex_lock(&wacom->lock);
>
> + if (wacom->open)
> + goto out;
> +
> if (usb_submit_urb(wacom->irq, GFP_KERNEL)) {
> retval = -EIO;
> goto out;
> @@ -157,9 +160,14 @@ static void wacom_close(struct input_dev *dev)
> autopm_error = usb_autopm_get_interface(wacom->intf);
>
> mutex_lock(&wacom->lock);
> + if (!wacom->open)
> + goto out;
> +
> usb_kill_urb(wacom->irq);
> wacom->open = false;
> wacom->intf->needs_remote_wakeup = 0;
> +
> +out:
> mutex_unlock(&wacom->lock);
>
> if (!autopm_error)
> @@ -1109,19 +1117,16 @@ static void wacom_destroy_battery(struct wacom *wacom)
> }
> }
>
> -static int wacom_register_input(struct wacom *wacom)
> +static struct input_dev *wacom_allocate_input(struct wacom *wacom)
> {
> struct input_dev *input_dev;
> struct usb_interface *intf = wacom->intf;
> struct usb_device *dev = interface_to_usbdev(intf);
> struct wacom_wac *wacom_wac = &(wacom->wacom_wac);
> - int error;
>
> input_dev = input_allocate_device();
> - if (!input_dev) {
> - error = -ENOMEM;
> - goto fail1;
> - }
> + if (!input_dev)
> + return NULL;
>
> input_dev->name = wacom_wac->name;
> input_dev->phys = wacom->phys;
> @@ -1131,21 +1136,59 @@ static int wacom_register_input(struct wacom *wacom)
> usb_to_input_id(dev, &input_dev->id);
> input_set_drvdata(input_dev, wacom);
>
> + return input_dev;
> +}
> +
> +static int wacom_register_input(struct wacom *wacom)
> +{
> + struct input_dev *input_dev, *pad_input_dev;
> + struct wacom_wac *wacom_wac = &(wacom->wacom_wac);
> + int error;
> +
> + input_dev = wacom_allocate_input(wacom);
> + pad_input_dev = wacom_allocate_input(wacom);
> + if (!input_dev || !pad_input_dev) {
> + error = -ENOMEM;
> + goto fail1;
> + }
> +
> wacom_wac->input = input_dev;
> + wacom_wac->pad_input = pad_input_dev;
> + wacom_wac->pad_input->name = wacom_wac->pad_name;
> +
> error = wacom_setup_input_capabilities(input_dev, wacom_wac);
> if (error)
> - goto fail1;
> + goto fail2;
>
> error = input_register_device(input_dev);
> if (error)
> goto fail2;
>
> + error = wacom_setup_pad_input_capabilities(pad_input_dev, wacom_wac);
> + if (error) {
> + /* no pad in use on this interface */
> + input_free_device(pad_input_dev);
> + wacom_wac->pad_input = NULL;
> + pad_input_dev = NULL;
> + } else {
> + error = input_register_device(pad_input_dev);
> + if (error)
> + goto fail3;
> + }
> +
> return 0;
>
> +fail3:
> + input_unregister_device(input_dev);
> + input_dev = NULL;
> fail2:
> - input_free_device(input_dev);
> wacom_wac->input = NULL;
> + wacom_wac->pad_input = NULL;
> fail1:
> + if (input_dev)
> + input_free_device(input_dev);
> + if (pad_input_dev)
> + input_free_device(pad_input_dev);
> return error;
> }
>
> @@ -1364,6 +1407,8 @@ static int wacom_probe(struct usb_interface *intf, const struct usb_device_id *i
> wacom_calculate_res(features);
>
> strlcpy(wacom_wac->name, features->name, sizeof(wacom_wac->name));
> + snprintf(wacom_wac->pad_name, sizeof(wacom_wac->pad_name),
> + "%s Pad", features->name);
>
> if (features->quirks & WACOM_QUIRK_MULTI_INPUT) {
> struct usb_device *other_dev;
> @@ -1438,6 +1483,8 @@ static void wacom_disconnect(struct usb_interface *intf)
> cancel_work_sync(&wacom->work);
> if (wacom->wacom_wac.input)
> input_unregister_device(wacom->wacom_wac.input);
> + if (wacom->wacom_wac.pad_input)
> + input_unregister_device(wacom->wacom_wac.pad_input);
> wacom_destroy_battery(wacom);
> wacom_destroy_leds(wacom);
> usb_free_urb(wacom->irq);
> diff --git a/drivers/input/tablet/wacom_wac.c b/drivers/input/tablet/wacom_wac.c
> index 977d05c..4b16a34 100644
> --- a/drivers/input/tablet/wacom_wac.c
> +++ b/drivers/input/tablet/wacom_wac.c
> @@ -1489,8 +1489,11 @@ void wacom_wac_irq(struct wacom_wac *wacom_wac, size_t len)
> break;
> }
>
> - if (sync)
> + if (sync) {
> input_sync(wacom_wac->input);
> + if (wacom_wac->pad_input)
> + input_sync(wacom_wac->pad_input);
> + }
> }
>
> static void wacom_setup_cintiq(struct wacom_wac *wacom_wac)
> @@ -1939,6 +1942,28 @@ int wacom_setup_input_capabilities(struct input_dev *input_dev,
> return 0;
> }
>
> +int wacom_setup_pad_input_capabilities(struct input_dev *input_dev,
> + struct wacom_wac *wacom_wac)
> +{
> + struct wacom_features *features = &wacom_wac->features;
> +
> + input_dev->evbit[0] |= BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
> +
> + /* kept for making legacy xf86-input-wacom working with the wheels */
> + __set_bit(ABS_MISC, input_dev->absbit);
> +
> + /* kept for making legacy xf86-input-wacom accepting the pad */
> + input_set_abs_params(input_dev, ABS_X, 0, 1, 0, 0);
> + input_set_abs_params(input_dev, ABS_Y, 0, 1, 0, 0);
> +
> + switch (features->type) {
> + default:
> + /* no pad supported */
> + return 1;
> + }
> + return 0;
> +}
> +
> static const struct wacom_features wacom_features_0x00 =
> { "Wacom Penpartner", WACOM_PKGLEN_PENPRTN, 5040, 3780, 255,
> 0, PENPARTNER, WACOM_PENPRTN_RES, WACOM_PENPRTN_RES };
> diff --git a/drivers/input/tablet/wacom_wac.h b/drivers/input/tablet/wacom_wac.h
> index b2c9a9c..f48164c 100644
> --- a/drivers/input/tablet/wacom_wac.h
> +++ b/drivers/input/tablet/wacom_wac.h
> @@ -150,6 +150,7 @@ struct wacom_shared {
>
> struct wacom_wac {
> char name[WACOM_NAME_MAX];
> + char pad_name[WACOM_NAME_MAX];
> unsigned char *data;
> int tool[2];
> int id[2];
> @@ -157,6 +158,7 @@ struct wacom_wac {
> struct wacom_features features;
> struct wacom_shared *shared;
> struct input_dev *input;
> + struct input_dev *pad_input;
> int pid;
> int battery_capacity;
> int num_contacts_left;
> --
> 1.9.0
>
^ permalink raw reply
* RE: [PATCH 1/2] lib: int_sqrt: add int64_sqrt routine
From: Yibo Cai @ 2014-06-24 4:51 UTC (permalink / raw)
To: Peter Meerwald, Barry Song
Cc: dmitry.torokhov@gmail.com, dtor@mail.ru,
akpm@linux-foundation.org, linux-input@vger.kernel.org,
DL-SHA-WorkGroupLinux, Barry Song
In-Reply-To: <alpine.DEB.2.01.1406231150000.22151@pmeerw.net>
Hello Peter,
>> Get square root of a 64-bit digit on 32bit platforms. CSR SiRFSoC
>> touchscreen driver needs it.
> maybe the code could be #MACRO()'d to avoid duplication
Do you mean to define int64_sqrt() directly as int_sqrt() on a 64 bit platform where sizeof(long) == sizeof(long long)?
Yibo
________________________________________
From: Peter Meerwald [pmeerw@pmeerw.net]
Sent: Monday, June 23, 2014 5:52 PM
To: Barry Song
Cc: dmitry.torokhov@gmail.com; dtor@mail.ru; akpm@linux-foundation.org; linux-input@vger.kernel.org; DL-SHA-WorkGroupLinux; Yibo Cai; Barry Song
Subject: Re: [PATCH 1/2] lib: int_sqrt: add int64_sqrt routine
> Get square root of a 64-bit digit on 32bit platforms. CSR SiRFSoC
> touchscreen driver needs it.
maybe the code could be #MACRO()'d to avoid duplication
see comment below
> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> index 4c52907..6e63081 100644
> --- a/include/linux/kernel.h
> +++ b/include/linux/kernel.h
> @@ -418,6 +418,7 @@ struct pid;
> extern struct pid *session_of_pgrp(struct pid *pgrp);
>
> unsigned long int_sqrt(unsigned long);
> +unsigned long long int64_sqrt(unsigned long long);
>
> extern void bust_spinlocks(int yes);
> extern int oops_in_progress; /* If set, an oops, panic(), BUG() or die() is in progress */
> diff --git a/lib/int_sqrt.c b/lib/int_sqrt.c
> index 1ef4cc3..2aa9fcc 100644
> --- a/lib/int_sqrt.c
> +++ b/lib/int_sqrt.c
> @@ -36,3 +36,30 @@ unsigned long int_sqrt(unsigned long x)
> return y;
> }
> EXPORT_SYMBOL(int_sqrt);
> +
> +/*
> + * Square root of a 64-bit digit.
> + * Same as int_sqrt on 64-bit platforms where "long" equals "long long"
> + */
> +unsigned long long int64_sqrt(unsigned long long x)
> +{
> + unsigned long long m = 0, y = 0, b = 0;
initialization for b, m not needed
the original int_sqrt() code correctly skips initialization
> +
> + if (x <= 1)
> + return x;
> +
> + m = 1ULL << (BITS_PER_LONG_LONG - 2);
> + while (m != 0) {
> + b = y + m;
> + y >>= 1;
> +
> + if (x >= b) {
> + x -= b;
> + y += m;
> + }
> + m >>= 2;
> + }
> +
> + return y;
> +}
> +EXPORT_SYMBOL(int64_sqrt);
>
--
Peter Meerwald
+43-664-2444418 (mobile)
To report this email as spam click https://www.mailcontrol.com/sr/MZbqvYs5QwJvpeaetUwhCQ== .
Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
More information can be found at www.csr.com. Keep up to date with CSR on our technical blog, www.csr.com/blog, CSR people blog, www.csr.com/people, YouTube, www.youtube.com/user/CSRplc, Facebook, www.facebook.com/pages/CSR/191038434253534, or follow us on Twitter at www.twitter.com/CSR_plc.
New for 2014, you can now access the wide range of products powered by aptX at www.aptx.com.
^ permalink raw reply
* RE: [PATCH 1/2] lib: int_sqrt: add int64_sqrt routine
From: Peter Meerwald @ 2014-06-24 6:15 UTC (permalink / raw)
To: Yibo Cai
Cc: Barry Song, dmitry.torokhov@gmail.com, dtor@mail.ru,
akpm@linux-foundation.org, linux-input@vger.kernel.org,
DL-SHA-WorkGroupLinux, Barry Song
In-Reply-To: <A94419613C6B3B46A0D9E05B960561730BF59488@SHAASIEXM01.ASIA.ROOT.PRI>
Hello,
> > maybe the code could be #MACRO()'d to avoid duplication
>
> Do you mean to define int64_sqrt() directly as int_sqrt() on a 64 bit
> platform where sizeof(long) == sizeof(long long)?
something along the lines of
#define INT_SQRT(name, type) \
type name(type x) \
type m, y = 0, b; \
...
INT_SQRT(int_sqrt, unsigned long);
INT_SQRT(int64_sqrt, unsigned long long);
not sure if it is worth it
regards, p.
> ________________________________________
> From: Peter Meerwald [pmeerw@pmeerw.net]
> Sent: Monday, June 23, 2014 5:52 PM
> To: Barry Song
> Cc: dmitry.torokhov@gmail.com; dtor@mail.ru; akpm@linux-foundation.org; linux-input@vger.kernel.org; DL-SHA-WorkGroupLinux; Yibo Cai; Barry Song
> Subject: Re: [PATCH 1/2] lib: int_sqrt: add int64_sqrt routine
>
> > Get square root of a 64-bit digit on 32bit platforms. CSR SiRFSoC
> > touchscreen driver needs it.
>
> maybe the code could be #MACRO()'d to avoid duplication
>
> see comment below
>
> > diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> > index 4c52907..6e63081 100644
> > --- a/include/linux/kernel.h
> > +++ b/include/linux/kernel.h
> > @@ -418,6 +418,7 @@ struct pid;
> > extern struct pid *session_of_pgrp(struct pid *pgrp);
> >
> > unsigned long int_sqrt(unsigned long);
> > +unsigned long long int64_sqrt(unsigned long long);
> >
> > extern void bust_spinlocks(int yes);
> > extern int oops_in_progress; /* If set, an oops, panic(), BUG() or die() is in progress */
> > diff --git a/lib/int_sqrt.c b/lib/int_sqrt.c
> > index 1ef4cc3..2aa9fcc 100644
> > --- a/lib/int_sqrt.c
> > +++ b/lib/int_sqrt.c
> > @@ -36,3 +36,30 @@ unsigned long int_sqrt(unsigned long x)
> > return y;
> > }
> > EXPORT_SYMBOL(int_sqrt);
> > +
> > +/*
> > + * Square root of a 64-bit digit.
> > + * Same as int_sqrt on 64-bit platforms where "long" equals "long long"
> > + */
> > +unsigned long long int64_sqrt(unsigned long long x)
> > +{
> > + unsigned long long m = 0, y = 0, b = 0;
>
> initialization for b, m not needed
>
> the original int_sqrt() code correctly skips initialization
>
> > +
> > + if (x <= 1)
> > + return x;
> > +
> > + m = 1ULL << (BITS_PER_LONG_LONG - 2);
> > + while (m != 0) {
> > + b = y + m;
> > + y >>= 1;
> > +
> > + if (x >= b) {
> > + x -= b;
> > + y += m;
> > + }
> > + m >>= 2;
> > + }
> > +
> > + return y;
> > +}
> > +EXPORT_SYMBOL(int64_sqrt);
> >
>
> --
>
> Peter Meerwald
> +43-664-2444418 (mobile)
>
>
> To report this email as spam click https://www.mailcontrol.com/sr/MZbqvYs5QwJvpeaetUwhCQ== .
>
>
> Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
> More information can be found at www.csr.com. Keep up to date with CSR on our technical blog, www.csr.com/blog, CSR people blog, www.csr.com/people, YouTube, www.youtube.com/user/CSRplc, Facebook, www.facebook.com/pages/CSR/191038434253534, or follow us on Twitter at www.twitter.com/CSR_plc.
> New for 2014, you can now access the wide range of products powered by aptX at www.aptx.com.
>
--
Peter Meerwald
+43-664-2444418 (mobile)
^ permalink raw reply
* Re: [PATCH v2 10/10] mfd: cros_ec: move EC interrupt to cros_ec_keyb
From: Lee Jones @ 2014-06-24 10:25 UTC (permalink / raw)
To: Doug Anderson
Cc: Andrew Bresticker, swarren, olof, Sonny Rao, linux-samsung-soc,
Javier Martinez Canillas, Bill Richardson, sjg, Wolfram Sang,
broonie, dmitry.torokhov, sameo, geert, linux-input, linux-kernel
In-Reply-To: <1403115247-8853-11-git-send-email-dianders@chromium.org>
On Wed, 18 Jun 2014, Doug Anderson wrote:
> From: Andrew Bresticker <abrestic@chromium.org>
>
> If we receive EC interrupts after the cros_ec driver has probed, but
> before the cros_ec_keyb driver has probed, the cros_ec IRQ handler
> will not run the cros_ec_keyb notifier and the EC will leave the IRQ
> line asserted. The cros_ec IRQ handler then returns IRQ_HANDLED and
> the resulting flood of interrupts causes the machine to hang.
>
> Since the EC interrupt is currently only used for the keyboard, move
> the setup and handling of the EC interrupt to the cros_ec_keyb driver.
>
> Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> ---
> Changes in v2:
> - IRQs should be optional => move EC interrupt to keyboard.
>
> drivers/input/keyboard/cros_ec_keyb.c | 58 ++++++++++++++++++++---------------
> drivers/mfd/cros_ec.c | 35 +--------------------
> include/linux/mfd/cros_ec.h | 2 --
> 3 files changed, 34 insertions(+), 61 deletions(-)
For the MFD changes:
Acked-by: Lee Jones <lee.jones@linaro.org>
> diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
> index b8341ab..791781a 100644
> --- a/drivers/input/keyboard/cros_ec_keyb.c
> +++ b/drivers/input/keyboard/cros_ec_keyb.c
> @@ -24,8 +24,8 @@
> #include <linux/module.h>
> #include <linux/i2c.h>
> #include <linux/input.h>
> +#include <linux/interrupt.h>
> #include <linux/kernel.h>
> -#include <linux/notifier.h>
> #include <linux/platform_device.h>
> #include <linux/slab.h>
> #include <linux/input/matrix_keypad.h>
> @@ -42,7 +42,6 @@
> * @dev: Device pointer
> * @idev: Input device
> * @ec: Top level ChromeOS device to use to talk to EC
> - * @event_notifier: interrupt event notifier for transport devices
> */
> struct cros_ec_keyb {
> unsigned int rows;
> @@ -55,7 +54,6 @@ struct cros_ec_keyb {
> struct device *dev;
> struct input_dev *idev;
> struct cros_ec_device *ec;
> - struct notifier_block notifier;
> };
>
>
> @@ -173,22 +171,6 @@ static void cros_ec_keyb_process(struct cros_ec_keyb *ckdev,
> input_sync(ckdev->idev);
> }
>
> -static int cros_ec_keyb_open(struct input_dev *dev)
> -{
> - struct cros_ec_keyb *ckdev = input_get_drvdata(dev);
> -
> - return blocking_notifier_chain_register(&ckdev->ec->event_notifier,
> - &ckdev->notifier);
> -}
> -
> -static void cros_ec_keyb_close(struct input_dev *dev)
> -{
> - struct cros_ec_keyb *ckdev = input_get_drvdata(dev);
> -
> - blocking_notifier_chain_unregister(&ckdev->ec->event_notifier,
> - &ckdev->notifier);
> -}
> -
> static int cros_ec_keyb_get_state(struct cros_ec_keyb *ckdev, uint8_t *kb_state)
> {
> struct cros_ec_command msg = {
> @@ -203,19 +185,41 @@ static int cros_ec_keyb_get_state(struct cros_ec_keyb *ckdev, uint8_t *kb_state)
> return ckdev->ec->cmd_xfer(ckdev->ec, &msg);
> }
>
> -static int cros_ec_keyb_work(struct notifier_block *nb,
> - unsigned long state, void *_notify)
> +static irqreturn_t cros_ec_keyb_irq(int irq, void *data)
> {
> + struct cros_ec_keyb *ckdev = data;
> + struct cros_ec_device *ec = ckdev->ec;
> int ret;
> - struct cros_ec_keyb *ckdev = container_of(nb, struct cros_ec_keyb,
> - notifier);
> uint8_t kb_state[ckdev->cols];
>
> + if (device_may_wakeup(ec->dev))
> + pm_wakeup_event(ec->dev, 0);
> +
> ret = cros_ec_keyb_get_state(ckdev, kb_state);
> if (ret >= 0)
> cros_ec_keyb_process(ckdev, kb_state, ret);
> + else
> + dev_err(ec->dev, "failed to get keyboard state: %d\n", ret);
>
> - return NOTIFY_DONE;
> + return IRQ_HANDLED;
> +}
> +
> +static int cros_ec_keyb_open(struct input_dev *dev)
> +{
> + struct cros_ec_keyb *ckdev = input_get_drvdata(dev);
> + struct cros_ec_device *ec = ckdev->ec;
> +
> + return request_threaded_irq(ec->irq, NULL, cros_ec_keyb_irq,
> + IRQF_TRIGGER_LOW | IRQF_ONESHOT,
> + "cros_ec_keyb", ckdev);
> +}
> +
> +static void cros_ec_keyb_close(struct input_dev *dev)
> +{
> + struct cros_ec_keyb *ckdev = input_get_drvdata(dev);
> + struct cros_ec_device *ec = ckdev->ec;
> +
> + free_irq(ec->irq, ckdev);
> }
>
> static int cros_ec_keyb_probe(struct platform_device *pdev)
> @@ -246,8 +250,12 @@ static int cros_ec_keyb_probe(struct platform_device *pdev)
> if (!idev)
> return -ENOMEM;
>
> + if (!ec->irq) {
> + dev_err(dev, "no EC IRQ specified\n");
> + return -EINVAL;
> + }
> +
> ckdev->ec = ec;
> - ckdev->notifier.notifier_call = cros_ec_keyb_work;
> ckdev->dev = dev;
> dev_set_drvdata(&pdev->dev, ckdev);
>
> diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
> index 83e30c6..4873f9c 100644
> --- a/drivers/mfd/cros_ec.c
> +++ b/drivers/mfd/cros_ec.c
> @@ -62,18 +62,6 @@ int cros_ec_check_result(struct cros_ec_device *ec_dev,
> }
> EXPORT_SYMBOL(cros_ec_check_result);
>
> -static irqreturn_t ec_irq_thread(int irq, void *data)
> -{
> - struct cros_ec_device *ec_dev = data;
> -
> - if (device_may_wakeup(ec_dev->dev))
> - pm_wakeup_event(ec_dev->dev, 0);
> -
> - blocking_notifier_call_chain(&ec_dev->event_notifier, 1, ec_dev);
> -
> - return IRQ_HANDLED;
> -}
> -
> static const struct mfd_cell cros_devs[] = {
> {
> .name = "cros-ec-keyb",
> @@ -92,8 +80,6 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
> struct device *dev = ec_dev->dev;
> int err = 0;
>
> - BLOCKING_INIT_NOTIFIER_HEAD(&ec_dev->event_notifier);
> -
> if (ec_dev->din_size) {
> ec_dev->din = devm_kzalloc(dev, ec_dev->din_size, GFP_KERNEL);
> if (!ec_dev->din)
> @@ -105,42 +91,23 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
> return -ENOMEM;
> }
>
> - if (!ec_dev->irq) {
> - dev_dbg(dev, "no valid IRQ: %d\n", ec_dev->irq);
> - return err;
> - }
> -
> - err = request_threaded_irq(ec_dev->irq, NULL, ec_irq_thread,
> - IRQF_TRIGGER_LOW | IRQF_ONESHOT,
> - "chromeos-ec", ec_dev);
> - if (err) {
> - dev_err(dev, "request irq %d: error %d\n", ec_dev->irq, err);
> - return err;
> - }
> -
> err = mfd_add_devices(dev, 0, cros_devs,
> ARRAY_SIZE(cros_devs),
> NULL, ec_dev->irq, NULL);
> if (err) {
> dev_err(dev, "failed to add mfd devices\n");
> - goto fail_mfd;
> + return err;
> }
>
> dev_info(dev, "Chrome EC device registered\n");
>
> return 0;
> -
> -fail_mfd:
> - free_irq(ec_dev->irq, ec_dev);
> -
> - return err;
> }
> EXPORT_SYMBOL(cros_ec_register);
>
> int cros_ec_remove(struct cros_ec_device *ec_dev)
> {
> mfd_remove_devices(ec_dev->dev);
> - free_irq(ec_dev->irq, ec_dev);
>
> return 0;
> }
> diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
> index 0ebf26f..fcbe9d1 100644
> --- a/include/linux/mfd/cros_ec.h
> +++ b/include/linux/mfd/cros_ec.h
> @@ -62,7 +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
> - * @event_notifier: interrupt event notifier for transport devices
> * @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
> @@ -93,7 +92,6 @@ struct cros_ec_device {
> struct device *dev;
> bool was_wake_device;
> struct class *cros_class;
> - struct blocking_notifier_head event_notifier;
> int (*cmd_xfer)(struct cros_ec_device *ec,
> struct cros_ec_command *msg);
>
--
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 v2 3/6] iio: adc: fsl,imx25-gcq driver
From: Denis Carikli @ 2014-06-24 10:38 UTC (permalink / raw)
To: Jonathan Cameron, Shawn Guo, Samuel Ortiz, Dmitry Torokhov
Cc: Eric Bénard, Sascha Hauer,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Lee Jones,
linux-input-u79uwXL29TY76Z2rM5mHXA,
linux-iio-u79uwXL29TY76Z2rM5mHXA, Fabio Estevam,
Lars-Peter Clausen, Markus Pargmann, Devicetree List
In-Reply-To: <53A55EDF.4010104-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
On 06/21/2014 12:30 PM, Jonathan Cameron wrote:
>> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> No known info on converting this to real voltages? I'd normally expect
> a scale parameter at least with any raw channel output, but fair enough
> if it's not obvous what this should be.
We know the internal reference voltage.
We can also know the external reference voltage by representing it as a
regulator.
Since there are many combinations should a regulator for each
reference(external reference, xp, yp, xn and yn) be used?
Denis.
^ permalink raw reply
* [PATCH 1/4] HID: sony: Use the SIXAXIS_CONTROLLER constant when possible
From: Antonio Ospite @ 2014-06-24 11:28 UTC (permalink / raw)
To: linux-input; +Cc: Antonio Ospite, Jiri Kosina, simon, Frank Praznik
In-Reply-To: <1403609323-27283-1-git-send-email-ao2@ao2.it>
Instead of checking for SIXAXIS_CONTROLLER_USB and SIXAXIS_CONTROLLER_BT
separately, a check on SIXAXIS_CONTROLLER can be used when setting
connect_mask.
Signed-off-by: Antonio Ospite <ao2@ao2.it>
---
drivers/hid/hid-sony.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index 2259eaa..a77269b 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -1830,9 +1830,7 @@ static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
if (sc->quirks & VAIO_RDESC_CONSTANT)
connect_mask |= HID_CONNECT_HIDDEV_FORCE;
- else if (sc->quirks & SIXAXIS_CONTROLLER_USB)
- connect_mask |= HID_CONNECT_HIDDEV_FORCE;
- else if (sc->quirks & SIXAXIS_CONTROLLER_BT)
+ else if (sc->quirks & SIXAXIS_CONTROLLER)
connect_mask |= HID_CONNECT_HIDDEV_FORCE;
ret = hid_hw_start(hdev, connect_mask);
--
2.0.0
^ permalink raw reply related
* [PATCH 2/4] HID: sony: Always override the Sixaxis descriptor
From: Antonio Ospite @ 2014-06-24 11:28 UTC (permalink / raw)
To: linux-input; +Cc: Antonio Ospite, Jiri Kosina, simon, Frank Praznik
In-Reply-To: <1403609323-27283-1-git-send-email-ao2@ao2.it>
Simplify the logic of overriding the Sixaxis HID descriptor, this will
make it easier to amend the descriptor in future commits.
The current code does this:
if (original sixaxis via USB or BT)
fixup only a part of the descriptor
else if (sixaxis compatible controller)
override the _whole_ descriptor
but the end result is exactly the same, except for the trailing zero in
the case of original BT controllers.
So let's just regularize the process, and always override the HID
descriptor.
Always overriding the descriptor changes the current semantic a little
bit, before this change the BT descriptor still had the trailing zero
byte, while now it is exactly the same as the descriptor of the
controller via USB, but that does not affect proper operation of the
device.
Note that overriding the whole descriptor for original devices is not
strictly necessary for now, but it simplifies the code and in the future
the report descriptor will be patched further and keys will be remapped,
so it's handy to have only one place to patch.
Signed-off-by: Antonio Ospite <ao2@ao2.it>
---
drivers/hid/hid-sony.c | 31 ++++++++++---------------------
1 file changed, 10 insertions(+), 21 deletions(-)
diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index a77269b..11026b5 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -56,13 +56,7 @@
#define MAX_LEDS 4
-static const u8 sixaxis_rdesc_fixup[] = {
- 0x95, 0x13, 0x09, 0x01, 0x81, 0x02, 0x95, 0x0C,
- 0x81, 0x01, 0x75, 0x10, 0x95, 0x04, 0x26, 0xFF,
- 0x03, 0x46, 0xFF, 0x03, 0x09, 0x01, 0x81, 0x02
-};
-
-static const u8 sixaxis_rdesc_fixup2[] = {
+static __u8 sixaxis_rdesc[] = {
0x05, 0x01, 0x09, 0x04, 0xa1, 0x01, 0xa1, 0x02,
0x85, 0x01, 0x75, 0x08, 0x95, 0x01, 0x15, 0x00,
0x26, 0xff, 0x00, 0x81, 0x03, 0x75, 0x01, 0x95,
@@ -778,6 +772,13 @@ struct sony_sc {
__u8 led_count;
};
+static __u8 *sixaxis_fixup(struct hid_device *hdev, __u8 *rdesc,
+ unsigned int *rsize)
+{
+ *rsize = sizeof(sixaxis_rdesc);
+ return sixaxis_rdesc;
+}
+
static __u8 *ps3remote_fixup(struct hid_device *hdev, __u8 *rdesc,
unsigned int *rsize)
{
@@ -857,20 +858,8 @@ static __u8 *sony_report_fixup(struct hid_device *hdev, __u8 *rdesc,
*rsize = sizeof(dualshock4_bt_rdesc);
}
- /* The HID descriptor exposed over BT has a trailing zero byte */
- if ((((sc->quirks & SIXAXIS_CONTROLLER_USB) && *rsize == 148) ||
- ((sc->quirks & SIXAXIS_CONTROLLER_BT) && *rsize == 149)) &&
- rdesc[83] == 0x75) {
- hid_info(hdev, "Fixing up Sony Sixaxis report descriptor\n");
- memcpy((void *)&rdesc[83], (void *)&sixaxis_rdesc_fixup,
- sizeof(sixaxis_rdesc_fixup));
- } else if (sc->quirks & SIXAXIS_CONTROLLER_USB &&
- *rsize > sizeof(sixaxis_rdesc_fixup2)) {
- hid_info(hdev, "Sony Sixaxis clone detected. Using original report descriptor (size: %d clone; %d new)\n",
- *rsize, (int)sizeof(sixaxis_rdesc_fixup2));
- *rsize = sizeof(sixaxis_rdesc_fixup2);
- memcpy(rdesc, &sixaxis_rdesc_fixup2, *rsize);
- }
+ if (sc->quirks & SIXAXIS_CONTROLLER)
+ return sixaxis_fixup(hdev, rdesc, rsize);
if (sc->quirks & PS3REMOTE)
return ps3remote_fixup(hdev, rdesc, rsize);
--
2.0.0
^ 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