* Re: Linux 3.16-rc6
From: John Stoffel @ 2014-07-24 22:06 UTC (permalink / raw)
To: Waiman Long
Cc: Peter Zijlstra, Linus Torvalds, Borislav Petkov, Ingo Molnar,
Linux Kernel Mailing List, USB list, linux-input@vger.kernel.org
In-Reply-To: <53D16EC4.1000801@hp.com>
>>>>> "Waiman" == Waiman Long <waiman.long@hp.com> writes:
Waiman> On 07/24/2014 02:36 PM, Peter Zijlstra wrote:
>> On Thu, Jul 24, 2014 at 11:18:16AM -0700, Linus Torvalds wrote:
>>> On Thu, Jul 24, 2014 at 5:58 AM, Peter Zijlstra<peterz@infradead.org> wrote:
>>>> So going by the nifty picture rostedt made:
>>>>
>>>> [ 61.454336] CPU0 CPU1
>>>> [ 61.454336] ---- ----
>>>> [ 61.454336] lock(&(&p->alloc_lock)->rlock);
>>>> [ 61.454336] local_irq_disable();
>>>> [ 61.454336] lock(tasklist_lock);
>>>> [ 61.454336] lock(&(&p->alloc_lock)->rlock);
>>>> [ 61.454336]<Interrupt>
>>>> [ 61.454336] lock(tasklist_lock);
>>> So this *should* be fine. It always has been in the past, and it was
>>> certainly the *intention* that it should continue to work with
>>> qrwlock, even in the presense of pending writers on other cpu's.
>>>
>>> The qrwlock rules are that a read-lock in an interrupt is still going
>>> to be unfair and succeed if there are other readers.
>> Ah, indeed. Should have checked :/
>>
>>> So it sounds to me like the new lockdep rules in tip/master are too
>>> strict and are throwing a false positive.
>> Right. Waiman can you have a look?
Waiman> Yes, I think I may have a solution for that.
Waiman> Borislav, can you apply the following patch on top of the lockdep patch
Waiman> to see if it can fix the problem?
Waiman> diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
Waiman> index d24e433..507a8ce 100644
Waiman> --- a/kernel/locking/lockdep.c
Waiman> +++ b/kernel/locking/lockdep.c
Waiman> @@ -3595,6 +3595,12 @@ void lock_acquire(struct lockdep_map *lock,
Waiman> unsigned int
Waiman> raw_local_irq_save(flags);
Waiman> check_flags(flags);
Waiman> + /*
Waiman> + * An interrupt recursive read in interrupt context can be
Waiman> considered
Waiman> + * to be the same as a recursive read from checking perspective.
Waiman> + */
Waiman> + if ((read == 3) && in_interrupt())
Waiman> + read = 2;
current-> lockdep_recursion = 1;
Waiman> trace_lock_acquire(lock, subclass, trylock, read, check,
Waiman> nest_lock, ip);
Waiman> __lock_acquire(lock, subclass, trylock, read, check,
Instead of the magic numbers 1,2,3, could you use some nicely named
constants here instead?
John
^ permalink raw reply
* [PATCH v4] Input: synaptics-rmi4: Add F30 support
From: Andrew Duggan @ 2014-07-24 23:47 UTC (permalink / raw)
To: Dmitry Torokhov, linux-input
Cc: Andrew Duggan, Christopher Heiny, Vincent Huang, Vivian Ly,
Allie Xiong, Linus Walleij, Benjamin Tissoires, David Herrmann,
Jiri Kosina
RMI4 Function 0x30 provides support for GPIOs, LEDs and mechanical
buttons. In particular, the mechanical button support is used in
an increasing number of touchpads.
Signed-off-by: Andrew Duggan <aduggan@synaptics.com>
Signed-off-by: Allie Xiong <axiong@synaptics.com>
Acked-by: Christopher Heiny <cheiny@synaptics.com>
---
I discovered that the v3 version of this patch from March wasn't compatible with the version
of the driver currently in the synaptics-rmi4 branch. This patch fixes that. The only difference
from v3 is adding a #define and fixing a spacing issue.
drivers/input/rmi4/Kconfig | 12 ++
drivers/input/rmi4/Makefile | 1 +
drivers/input/rmi4/rmi_f30.c | 400 +++++++++++++++++++++++++++++++++++++++++++
include/linux/rmi.h | 7 +-
4 files changed, 419 insertions(+), 1 deletion(-)
create mode 100644 drivers/input/rmi4/rmi_f30.c
diff --git a/drivers/input/rmi4/Kconfig b/drivers/input/rmi4/Kconfig
index d0c7b6e..14d5f49 100644
--- a/drivers/input/rmi4/Kconfig
+++ b/drivers/input/rmi4/Kconfig
@@ -63,3 +63,15 @@ config RMI4_F11_PEN
If your system is not recognizing pen touches and you know your
sensor supports pen input, you probably want to turn this feature
off.
+
+config RMI4_F30
+ tristate "RMI4 Function 30 (GPIO LED)"
+ depends on RMI4_CORE
+ help
+ Say Y here if you want to add support for RMI4 function 30.
+
+ Function 30 provides GPIO and LED support for RMI4 devices. This
+ includes support for buttons on TouchPads and ClickPads.
+
+ To compile this driver as a module, choose M here: the
+ module will be called rmi-f30.
diff --git a/drivers/input/rmi4/Makefile b/drivers/input/rmi4/Makefile
index 5c6bad5..ecffd72 100644
--- a/drivers/input/rmi4/Makefile
+++ b/drivers/input/rmi4/Makefile
@@ -3,6 +3,7 @@ rmi_core-y := rmi_bus.o rmi_driver.o rmi_f01.o
# Function drivers
obj-$(CONFIG_RMI4_F11) += rmi_f11.o
+obj-$(CONFIG_RMI4_F30) += rmi_f30.o
# Transports
obj-$(CONFIG_RMI4_I2C) += rmi_i2c.o
diff --git a/drivers/input/rmi4/rmi_f30.c b/drivers/input/rmi4/rmi_f30.c
new file mode 100644
index 0000000..05cc8e8
--- /dev/null
+++ b/drivers/input/rmi4/rmi_f30.c
@@ -0,0 +1,400 @@
+/*
+ * Copyright (c) 2012 - 2014 Synaptics Incorporated
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ */
+
+#include <linux/kernel.h>
+#include <linux/rmi.h>
+#include <linux/input.h>
+#include <linux/slab.h>
+#include <linux/gpio.h>
+#include <linux/leds.h>
+#include "rmi_driver.h"
+
+#define NAME_BUFFER_SIZE 256
+#define RMI_F30_QUERY_SIZE 2
+
+/* Defs for Query 0 */
+#define RMI_F30_EXTENDED_PATTERNS 0x01
+#define RMI_F30_HAS_MAPPABLE_BUTTONS (1 << 1)
+#define RMI_F30_HAS_LED (1 << 2)
+#define RMI_F30_HAS_GPIO (1 << 3)
+#define RMI_F30_HAS_HAPTIC (1 << 4)
+#define RMI_F30_HAS_GPIO_DRV_CTL (1 << 5)
+#define RMI_F30_HAS_MECH_MOUSE_BTNS (1 << 6)
+
+/* Defs for Query 1 */
+#define RMI_F30_GPIO_LED_COUNT 0x1F
+
+/* Defs for Control Registers */
+#define RMI_F30_CTRL_1_GPIO_DEBOUNCE 0x01
+#define RMI_F30_CTRL_1_HALT (1 << 4)
+#define RMI_F30_CTRL_1_HALTED (1 << 5)
+#define RMI_F30_CTRL_10_NUM_MECH_MOUSE_BTNS 0x03
+
+struct rmi_f30_ctrl_data {
+ int address;
+ int length;
+ u8 *regs;
+};
+
+#define RMI_F30_CTRL_MAX_REGS 32
+#define RMI_F30_CTRL_MAX_BYTES ((RMI_F30_CTRL_MAX_REGS + 7) >> 3)
+#define RMI_F30_CTRL_MAX_REG_BLOCKS 11
+
+#define RMI_F30_CTRL_REGS_MAX_SIZE (RMI_F30_CTRL_MAX_BYTES \
+ + 1 \
+ + RMI_F30_CTRL_MAX_BYTES \
+ + RMI_F30_CTRL_MAX_BYTES \
+ + RMI_F30_CTRL_MAX_BYTES \
+ + 6 \
+ + RMI_F30_CTRL_MAX_REGS \
+ + RMI_F30_CTRL_MAX_REGS \
+ + RMI_F30_CTRL_MAX_BYTES \
+ + 1 \
+ + 1)
+
+struct f30_data {
+ /* Query Data */
+ bool has_extended_pattern;
+ bool has_mappable_buttons;
+ bool has_led;
+ bool has_gpio;
+ bool has_haptic;
+ bool has_gpio_driver_control;
+ bool has_mech_mouse_btns;
+ u8 gpioled_count;
+
+ u8 register_count;
+
+ /* Control Register Data */
+ struct rmi_f30_ctrl_data ctrl[RMI_F30_CTRL_MAX_REG_BLOCKS];
+ u8 ctrl_regs[RMI_F30_CTRL_REGS_MAX_SIZE];
+ u32 ctrl_regs_size;
+
+ u8 data_regs[RMI_F30_CTRL_MAX_BYTES];
+ u16 *gpioled_key_map;
+ u8 *gpioled_sense_map;
+
+ char input_phys[NAME_BUFFER_SIZE];
+ struct input_dev *input;
+};
+
+static int rmi_f30_read_control_parameters(struct rmi_function *fn,
+ struct f30_data *f30)
+{
+ struct rmi_device *rmi_dev = fn->rmi_dev;
+ int error = 0;
+
+ error = rmi_read_block(rmi_dev, fn->fd.control_base_addr,
+ f30->ctrl_regs, f30->ctrl_regs_size);
+ if (error) {
+ dev_err(&rmi_dev->dev, "%s : Could not read control registers at 0x%x error (%d)\n",
+ __func__, fn->fd.control_base_addr, error);
+ return error;
+ }
+
+ return 0;
+}
+
+static int rmi_f30_attention(struct rmi_function *fn, unsigned long *irq_bits)
+{
+ struct f30_data *f30 = dev_get_drvdata(&fn->dev);
+ int retval;
+ int gpiled = 0;
+ int value = 0;
+ int i;
+ int reg_num;
+
+ /* Read the gpi led data. */
+ retval = rmi_read_block(fn->rmi_dev, fn->fd.data_base_addr,
+ f30->data_regs, f30->register_count);
+
+ if (retval) {
+ dev_err(&fn->dev, "%s: Failed to read F30 data registers.\n",
+ __func__);
+ return retval;
+ }
+
+ for (reg_num = 0; reg_num < f30->register_count; ++reg_num) {
+ for (i = 0; gpiled < f30->gpioled_count && i < 8; ++i,
+ ++gpiled) {
+ if (f30->gpioled_key_map[gpiled] != 0) {
+ value = (((f30->data_regs[reg_num] >> i) & 0x01)
+ == f30->gpioled_sense_map[gpiled]);
+
+ dev_dbg(&fn->dev,
+ "%s: call input report key (0x%04x) value (0x%02x)",
+ __func__,
+ f30->gpioled_key_map[gpiled], value);
+ input_report_key(f30->input,
+ f30->gpioled_key_map[gpiled],
+ value);
+ }
+
+ }
+ }
+
+ input_sync(f30->input); /* sync after groups of events */
+ return 0;
+}
+
+static int rmi_f30_register_device(struct rmi_function *fn)
+{
+ int i;
+ int rc;
+ struct rmi_device *rmi_dev = fn->rmi_dev;
+ struct f30_data *f30 = dev_get_drvdata(&fn->dev);
+ struct rmi_driver *driver = fn->rmi_dev->driver;
+ struct input_dev *input_dev;
+
+ input_dev = devm_input_allocate_device(&fn->dev);
+ if (!input_dev) {
+ dev_err(&fn->dev, "Failed to allocate input device.\n");
+ return -ENOMEM;
+ }
+
+ f30->input = input_dev;
+
+ if (driver->set_input_params) {
+ rc = driver->set_input_params(rmi_dev, input_dev);
+ if (rc < 0) {
+ dev_err(&fn->dev, "%s: Error in setting input device.\n",
+ __func__);
+ return rc;
+ }
+ }
+ sprintf(f30->input_phys, "%s/input0", dev_name(&fn->dev));
+ input_dev->phys = f30->input_phys;
+ input_dev->dev.parent = &rmi_dev->dev;
+ input_set_drvdata(input_dev, f30);
+
+ set_bit(EV_SYN, input_dev->evbit);
+ set_bit(EV_KEY, input_dev->evbit);
+
+ input_dev->keycode = f30->gpioled_key_map;
+ input_dev->keycodesize = sizeof(u16);
+ input_dev->keycodemax = f30->gpioled_count;
+
+ for (i = 0; i < f30->gpioled_count; i++) {
+ if (f30->gpioled_key_map[i] != 0)
+ input_set_capability(input_dev, EV_KEY,
+ f30->gpioled_key_map[i]);
+ }
+
+ rc = input_register_device(input_dev);
+ if (rc < 0) {
+ dev_err(&fn->dev, "Failed to register input device.\n");
+ return rc;
+ }
+ return 0;
+}
+
+static int rmi_f30_config(struct rmi_function *fn)
+{
+ struct f30_data *f30 = dev_get_drvdata(&fn->dev);
+ int error;
+
+ /* Write Control Register values back to device */
+ error = rmi_write_block(fn->rmi_dev, fn->fd.control_base_addr,
+ f30->ctrl_regs, f30->ctrl_regs_size);
+ if (error) {
+ dev_err(&fn->rmi_dev->dev,
+ "%s : Could not write control registers at 0x%x error (%d)\n",
+ __func__, fn->fd.control_base_addr, error);
+ return error;
+ }
+
+ return 0;
+}
+
+static inline void rmi_f30_set_ctrl_data(struct rmi_f30_ctrl_data *ctrl,
+ int *ctrl_addr, int len, u8 **reg)
+{
+ ctrl->address = *ctrl_addr;
+ ctrl->length = len;
+ ctrl->regs = *reg;
+ *ctrl_addr += len;
+ *reg += len;
+}
+
+static inline int rmi_f30_initialize(struct rmi_function *fn)
+{
+ struct f30_data *f30;
+ struct rmi_device *rmi_dev = fn->rmi_dev;
+ const struct rmi_device_platform_data *pdata;
+ int retval = 0;
+ int control_address;
+ u8 buf[RMI_F30_QUERY_SIZE];
+ u8 *ctrl_reg;
+ u8 *map_memory;
+
+ f30 = devm_kzalloc(&fn->dev, sizeof(struct f30_data),
+ GFP_KERNEL);
+ if (!f30) {
+ dev_err(&fn->dev, "Failed to allocate f30_data.\n");
+ return -ENOMEM;
+ }
+ dev_set_drvdata(&fn->dev, f30);
+
+ retval = rmi_read_block(fn->rmi_dev, fn->fd.query_base_addr, buf,
+ RMI_F30_QUERY_SIZE);
+
+ if (retval) {
+ dev_err(&fn->dev, "Failed to read query register.\n");
+ return retval;
+ }
+
+ f30->has_extended_pattern = buf[0] & RMI_F30_EXTENDED_PATTERNS;
+ f30->has_mappable_buttons = buf[0] & RMI_F30_HAS_MAPPABLE_BUTTONS;
+ f30->has_led = buf[0] & RMI_F30_HAS_LED;
+ f30->has_gpio = buf[0] & RMI_F30_HAS_GPIO;
+ f30->has_haptic = buf[0] & RMI_F30_HAS_HAPTIC;
+ f30->has_gpio_driver_control = buf[0] & RMI_F30_HAS_GPIO_DRV_CTL;
+ f30->has_mech_mouse_btns = buf[0] & RMI_F30_HAS_MECH_MOUSE_BTNS;
+ f30->gpioled_count = buf[1] & RMI_F30_GPIO_LED_COUNT;
+
+ f30->register_count = (f30->gpioled_count + 7) >> 3;
+
+ control_address = fn->fd.control_base_addr;
+ ctrl_reg = f30->ctrl_regs;
+
+ /* Allocate buffers for the control registers */
+ if (f30->has_led && f30->has_led)
+ rmi_f30_set_ctrl_data(&f30->ctrl[0], &control_address,
+ f30->register_count, &ctrl_reg);
+
+ rmi_f30_set_ctrl_data(&f30->ctrl[1], &control_address, sizeof(u8),
+ &ctrl_reg);
+
+ if (f30->has_gpio) {
+ rmi_f30_set_ctrl_data(&f30->ctrl[2], &control_address,
+ f30->register_count, &ctrl_reg);
+
+ rmi_f30_set_ctrl_data(&f30->ctrl[3], &control_address,
+ f30->register_count, &ctrl_reg);
+ }
+
+ if (f30->has_led) {
+ int ctrl5_len;
+
+ rmi_f30_set_ctrl_data(&f30->ctrl[4], &control_address,
+ f30->register_count, &ctrl_reg);
+
+ if (f30->has_extended_pattern)
+ ctrl5_len = 6;
+ else
+ ctrl5_len = 2;
+
+ rmi_f30_set_ctrl_data(&f30->ctrl[5], &control_address,
+ ctrl5_len, &ctrl_reg);
+ }
+
+ if (f30->has_led || f30->has_gpio_driver_control) {
+ /* control 6 uses a byte per gpio/led */
+ rmi_f30_set_ctrl_data(&f30->ctrl[6], &control_address,
+ f30->gpioled_count, &ctrl_reg);
+ }
+
+ if (f30->has_mappable_buttons) {
+ /* control 7 uses a byte per gpio/led */
+ rmi_f30_set_ctrl_data(&f30->ctrl[7], &control_address,
+ f30->gpioled_count, &ctrl_reg);
+ }
+
+ if (f30->has_haptic) {
+ rmi_f30_set_ctrl_data(&f30->ctrl[8], &control_address,
+ f30->register_count, &ctrl_reg);
+
+ rmi_f30_set_ctrl_data(&f30->ctrl[9], &control_address,
+ sizeof(u8), &ctrl_reg);
+ }
+
+ if (f30->has_mech_mouse_btns)
+ rmi_f30_set_ctrl_data(&f30->ctrl[10], &control_address,
+ sizeof(u8), &ctrl_reg);
+
+ f30->ctrl_regs_size = ctrl_reg - f30->ctrl_regs
+ ?: RMI_F30_CTRL_REGS_MAX_SIZE;
+
+ map_memory = devm_kzalloc(&fn->dev,
+ (f30->gpioled_count
+ * (sizeof(u16) + sizeof(u8))),
+ GFP_KERNEL);
+ if (!map_memory) {
+ dev_err(&fn->dev, "Failed to allocate gpioled map memory.\n");
+ return -ENOMEM;
+ }
+
+ f30->gpioled_key_map = (u16 *)map_memory;
+ f30->gpioled_sense_map = map_memory + sizeof(u16)
+ * f30->gpioled_count;
+
+ pdata = rmi_get_platform_data(rmi_dev);
+ if (pdata) {
+ if (!pdata->gpioled_map) {
+ dev_warn(&fn->dev,
+ "%s - gpioled_map is NULL", __func__);
+ } else if (!pdata->gpioled_map->map) {
+ dev_warn(&fn->dev,
+ "Platform Data button map is missing!\n");
+ } else {
+ int i;
+ for (i = 0; i < f30->gpioled_count
+ || i < pdata->gpioled_map->ngpioleds; i++) {
+ f30->gpioled_key_map[i] =
+ pdata->gpioled_map->map[i].button;
+ f30->gpioled_sense_map[i] =
+ pdata->gpioled_map->map[i].sense;
+ }
+ }
+ }
+
+ retval = rmi_f30_read_control_parameters(fn, f30);
+ if (retval < 0) {
+ dev_err(&fn->dev,
+ "Failed to initialize F19 control params.\n");
+ return retval;
+ }
+
+ return 0;
+}
+
+static int rmi_f30_probe(struct rmi_function *fn)
+{
+ int rc;
+
+ rc = rmi_f30_initialize(fn);
+ if (rc < 0)
+ goto error_exit;
+
+ rc = rmi_f30_register_device(fn);
+ if (rc < 0)
+ goto error_exit;
+
+ return 0;
+
+error_exit:
+ return rc;
+
+}
+
+static struct rmi_function_handler rmi_f30_handler = {
+ .driver = {
+ .name = "rmi_f30",
+ },
+ .func = 0x30,
+ .probe = rmi_f30_probe,
+ .config = rmi_f30_config,
+ .attention = rmi_f30_attention,
+};
+
+module_rmi_driver(rmi_f30_handler);
+
+MODULE_AUTHOR("Allie Xiong <axiong@synaptics.com>");
+MODULE_AUTHOR("Andrew Duggan <aduggan@synaptics.com>");
+MODULE_DESCRIPTION("RMI F30 module");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/rmi.h b/include/linux/rmi.h
index ca35b2f..5150eb9 100644
--- a/include/linux/rmi.h
+++ b/include/linux/rmi.h
@@ -146,9 +146,14 @@ struct rmi_button_map {
u8 *map;
};
+struct rmi_f30_button {
+ u16 button;
+ int sense;
+};
+
struct rmi_f30_gpioled_map {
u8 ngpioleds;
- u8 *map;
+ struct rmi_f30_button *map;
};
/**
--
1.9.1
^ permalink raw reply related
* Re: [PATCH] gpio: extend gpiod_get*() with flags parameter
From: Alexandre Courbot @ 2014-07-25 1:32 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linux-wireless, Ulf Hansson, Wolfram Sang, linux-mmc, Chris Ball,
dri-devel@lists.freedesktop.org, Liam Girdwood, Peter Ujfalusi,
Laurent Pinchart, Pavel Machek, Jiri Slaby, Florian Fainelli,
Samuel Ortiz, Alexander Shiyan, Dmitry Eremin-Solenikov,
Lee Jones, Paul Handrigan, linux-iio, linux-omap, Tomi Valkeinen,
Hans Verkuil, linux-serial, linux-input
In-Reply-To: <6059032.T3tqvLkWTo@wuerfel>
On Fri, Jul 25, 2014 at 1:10 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Friday 25 July 2014 00:04:58 Alexandre Courbot wrote:
>> I'm not sure how this could be applied harmlessly though - maybe through
>> a dedicated branch for -next? Problem is that a lot of new code is not
>> yet merged into mainline, and conflicts are very likely to occur. Linus,
>> do you have any suggestion as to how this can be done without blood being
>> spilled?
>
> There is a trick that we sometime use in this situation,
> though it has to be done carefully:
>
>> diff --git a/Documentation/gpio/consumer.txt b/Documentation/gpio/consumer.txt
>> index 7ff30d2..a3fb1d7 100644
>> --- a/Documentation/gpio/consumer.txt
>> +++ b/Documentation/gpio/consumer.txt
>> @@ -29,13 +29,24 @@ gpiod_get() functions. Like many other kernel subsystems, gpiod_get() takes the
>> device that will use the GPIO and the function the requested GPIO is supposed to
>> fulfill:
>>
>> - struct gpio_desc *gpiod_get(struct device *dev, const char *con_id)
>> + struct gpio_desc *gpiod_get(struct device *dev, const char *con_id,
>> + enum gpio_flags flags)
>>
>>
>
>
> - struct gpio_desc *gpiod_get(struct device *dev, const char *con_id)
> + struct gpio_desc *__gpiod_get(struct device *dev, const char *con_id,
> + enum gpio_flags flags);
> +
> +#define __gpiod_get(dev, con_id, flags, ...) __gpiod_get(dev, con_id, flags)
> +#define gpiod_get(varargs ...) __gpiod_get(varargs, 0)
>
> This will allow both variants to be called, and any users of the three-argument
> version will pass zero as the fourth argument (or whatever you choose there).
>
> Once the conversion is complete, the macros can be removed.
Wow, that works great, thanks! Indeed, this will allow me to send more
localized patches and to not spam half of the kernel developers. Sorry
about the noise.
Let's close this thread, I will submit the gpiolib update with this
trick first, then update each user before eventually removing the
macros.
Thanks,
Alex.
^ permalink raw reply
* Re: [PATCH] gpio: extend gpiod_get*() with flags parameter
From: Alexandre Courbot @ 2014-07-25 1:36 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-wireless, Ulf Hansson, Wolfram Sang, linux-mmc, Chris Ball,
dri-devel@lists.freedesktop.org, Liam Girdwood, Peter Ujfalusi,
linux-i2c@vger.kernel.org, Pavel Machek, Jiri Slaby,
Florian Fainelli, Samuel Ortiz, Alexander Shiyan,
Dmitry Eremin-Solenikov, Lee Jones, Paul Handrigan, linux-iio,
linux-omap, Tomi Valkeinen, Hans Verkuil, linux-serial,
linux-input
In-Reply-To: <1496956.3bpx902hFg@avalon>
On Fri, Jul 25, 2014 at 1:23 AM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>> diff --git a/Documentation/gpio/consumer.txt
>> b/Documentation/gpio/consumer.txt index 7ff30d2..a3fb1d7 100644
>> --- a/Documentation/gpio/consumer.txt
>> +++ b/Documentation/gpio/consumer.txt
>> @@ -29,13 +29,24 @@ gpiod_get() functions. Like many other kernel
>> subsystems, gpiod_get() takes the device that will use the GPIO and the
>> function the requested GPIO is supposed to fulfill:
>>
>> - struct gpio_desc *gpiod_get(struct device *dev, const char *con_id)
>> + struct gpio_desc *gpiod_get(struct device *dev, const char *con_id,
>> + enum gpio_flags flags)
>
> I assume you mean enum gpiod_flags here and in the rest of the documentation.
Indeed, thanks for pointing it out.
>
>> If a function is implemented by using several GPIOs together (e.g. a simple
>> LED device that displays digits), an additional index argument can be
>> specified:
>>
>> struct gpio_desc *gpiod_get_index(struct device *dev,
>> - const char *con_id, unsigned int idx)
>> + const char *con_id, unsigned int idx,
>> + enum gpio_flags flags)
>> +
>> +The flags parameter is used to optionally specify a direction and initial
>> value +for the GPIO. Values can be:
>> +
>> +* AS_IS or 0 to not initialize the GPIO at all. The direction must be set
>> later
>> + with one of the dedicated functions.
>> +* INPUT to initialize the GPIO as input.
>> +* OUTPUT_LOW to initialize the GPIO as output with a value of 0.
>> +* OUTPUT_HIGH to initialize the GPIO as output with a value of 1.
>
> Pretty please, could you at least prefix that with GPIOD_ ? Those names are
> too generic.
>
> How about renaming them to GPIOD_AS_IS, GPIOD_IN, GPIOD_OUT_INIT_LOW and
> GPIOD_OUT_INIT_HIGH in order to match the GPIOF_ flags ?
Yeah, you're right, the possibility of name collision is quite high here.
Thanks,
Alex.
^ permalink raw reply
* [PATCH] hid: usbhid: Use flag HID_DISCONNECTED when a usb device is removed
From: Reyad Attiyat @ 2014-07-25 5:13 UTC (permalink / raw)
To: linux-input, jkosina, srinivas.pandruvada, linux-kernel; +Cc: Reyad Attiyat
Set disconnected flag in struct usbhid when a usb device
is removed. Check for disconnected flag before sending urb
requests. This prevents a kernel panic when a hid driver calls
hid_hw_request() after removing a usb device.
Signed-off-by: Reyad Attiyat <reyad.attiyat@gmail.com>
---
drivers/hid/usbhid/hid-core.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index 7b88f4c..c8fa957 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -536,7 +536,8 @@ static void __usbhid_submit_report(struct hid_device *hid, struct hid_report *re
int head;
struct usbhid_device *usbhid = hid->driver_data;
- if ((hid->quirks & HID_QUIRK_NOGET) && dir == USB_DIR_IN)
+ if (((hid->quirks & HID_QUIRK_NOGET) && dir == USB_DIR_IN) ||
+ test_bit(HID_DISCONNECTED, &usbhid->iofl))
return;
if (usbhid->urbout && dir == USB_DIR_OUT && report->type == HID_OUTPUT_REPORT) {
@@ -1366,6 +1367,9 @@ static void usbhid_disconnect(struct usb_interface *intf)
return;
usbhid = hid->driver_data;
+ spin_lock_irq(&usbhid->lock); /* Sync with error and led handlers */
+ set_bit(HID_DISCONNECTED, &usbhid->iofl);
+ spin_unlock_irq(&usbhid->lock);
hid_destroy_device(hid);
kfree(usbhid);
}
--
1.9.3
^ permalink raw reply related
* Re: [PATCH] gpio: extend gpiod_get*() with flags parameter
From: Lee Jones @ 2014-07-25 6:52 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: linux-wireless, Ulf Hansson, Wolfram Sang, Takashi Iwai,
linux-mmc, Linus Walleij, Chris Ball, dri-devel, Liam Girdwood,
Peter Ujfalusi, Thierry Reding, Laurent Pinchart, Pavel Machek,
Jiri Slaby, gnurou, Florian Fainelli, Samuel Ortiz,
Alexander Shiyan, Dmitry Eremin-Solenikov, Jingoo Han,
Paul Handrigan, linux-iio, linux-omap, Tomi Valkeinen,
Hans Verkuil
In-Reply-To: <20140724161052.GA15615@kroah.com>
On Thu, 24 Jul 2014, Greg Kroah-Hartman wrote:
> On Fri, Jul 25, 2014 at 12:04:58AM +0900, Alexandre Courbot wrote:
> > The huge majority of GPIOs have their direction and initial value set
> > right after being obtained by one of the gpiod_get() functions. The
> > integer GPIO API had gpio_request_one() that took a convenience flags
> > parameter allowing to specify an direction and value applied to the
> > returned GPIO. This feature greatly simplifies client code and ensures
> > errors are always handled properly.
> >
> > A similar feature has been requested for the gpiod API. Since GPIOs need
> > a direction to be used anyway, we prefer to extend the existing
> > functions instead of introducing new functions that would raise the
> > number of gpiod getters to 16 (!).
> >
> > The drawback of this approach is that all gpiod clients need to be
> > updated, but there aren't that many and the moment and this results in
> > smaller (and hopefully safer) code.
> >
> > Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> > ---
> > This change will be difficult to apply without breaking things, but
> > let's try to do it right. Hopefully the benefit will outweight the
> > disturbance.
> >
> > This is a patch against -next to list and update all current gpiod
> > consumers. Updates are trivial at first sight, but it would be nice to
> > get as many acks as possible from the respective subsystem maintainers.
> >
> > I'm not sure how this could be applied harmlessly though - maybe through
> > a dedicated branch for -next? Problem is that a lot of new code is not
> > yet merged into mainline, and conflicts are very likely to occur. Linus,
> > do you have any suggestion as to how this can be done without blood being
> > spilled?
>
> Do this in 3 steps, not all at once.
>
> Make a new function that takes the new argument, get that merged
>
> Submit patches that convert drivers over to use the new function.
>
> Once all of those are merged, remove the old function.
>
> That way there are no "flag days" needed, and everyone is happy you
> don't send out emails with 40+ people in the To: and Cc: lines :)
+99999
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply
* [PATCH] Input: soc_button_array: Remove kfree on data allocated with devm_zalloc
From: pramod.gurav.etc @ 2014-07-25 7:09 UTC (permalink / raw)
To: linux-input, linux-kernel
Cc: Pramod Gurav, Dmitry Torokhov, Lejun Zhu, Sachin Kamat
From: Pramod Gurav <pramod.gurav@smartplayin.com>
This patch does below:
- Removes kfree done on data allocated with devm_zalloc in probe
path of the driver.
- Adds a check on return value from devm_kzalloc which was missing
and renames a lable from err_free_mem to err_mem.
- Adds couple of dev_err on failure to allocate memory
CC: Dmitry Torokhov <dmitry.torokhov@gmail.com>
CC: Lejun Zhu <lejun.zhu@linux.intel.com>
CC: Sachin Kamat <sachin.kamat@linaro.org>
Signed-off-by: Pramod Gurav <pramod.gurav@smartplayin.com>
---
drivers/input/misc/soc_button_array.c | 19 ++++++++++++++-----
1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/drivers/input/misc/soc_button_array.c b/drivers/input/misc/soc_button_array.c
index 5a6334b..ac82971 100644
--- a/drivers/input/misc/soc_button_array.c
+++ b/drivers/input/misc/soc_button_array.c
@@ -83,6 +83,13 @@ soc_button_device_create(struct pnp_dev *pdev,
sizeof(*gpio_keys_pdata) +
sizeof(*gpio_keys) * MAX_NBUTTONS,
GFP_KERNEL);
+ if (!gpio_keys_pdata) {
+ dev_err(&pdev->dev,
+ "Can't allocate memory for gpio_keys_platform_data\n");
+ error = -ENOMEM;
+ goto err_mem;
+ }
+
gpio_keys = (void *)(gpio_keys_pdata + 1);
for (info = button_info; info->name; info++) {
@@ -104,7 +111,7 @@ soc_button_device_create(struct pnp_dev *pdev,
if (n_buttons == 0) {
error = -ENODEV;
- goto err_free_mem;
+ goto err_mem;
}
gpio_keys_pdata->buttons = gpio_keys;
@@ -114,7 +121,7 @@ soc_button_device_create(struct pnp_dev *pdev,
pd = platform_device_alloc("gpio-keys", PLATFORM_DEVID_AUTO);
if (!pd) {
error = -ENOMEM;
- goto err_free_mem;
+ goto err_mem;
}
error = platform_device_add_data(pd, gpio_keys_pdata,
@@ -130,8 +137,7 @@ soc_button_device_create(struct pnp_dev *pdev,
err_free_pdev:
platform_device_put(pd);
-err_free_mem:
- devm_kfree(&pdev->dev, gpio_keys_pdata);
+err_mem:
return ERR_PTR(error);
}
@@ -155,8 +161,11 @@ static int soc_button_pnp_probe(struct pnp_dev *pdev,
int error;
priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
- if (!priv)
+ if (!priv) {
+ dev_err(&pdev->dev,
+ "Can't allocate memory for soc_button_data\n");
return -ENOMEM;
+ }
pnp_set_drvdata(pdev, priv);
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH] Input: soc_button_array: Remove kfree on data allocated with devm_zalloc
From: Sachin Kamat @ 2014-07-25 8:29 UTC (permalink / raw)
To: pramod.gurav.etc
Cc: linux-input, open list, Pramod Gurav, Dmitry Torokhov, Lejun Zhu,
Sachin Kamat
In-Reply-To: <1406272180-25101-1-git-send-email-pramod.gurav@smartplayin.com>
On Fri, Jul 25, 2014 at 12:39 PM, <pramod.gurav.etc@gmail.com> wrote:
> From: Pramod Gurav <pramod.gurav@smartplayin.com>
>
> This patch does below:
> - Removes kfree done on data allocated with devm_zalloc in probe
> path of the driver.
> - Adds a check on return value from devm_kzalloc which was missing
> and renames a lable from err_free_mem to err_mem.
> - Adds couple of dev_err on failure to allocate memory
>
> CC: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> CC: Lejun Zhu <lejun.zhu@linux.intel.com>
> CC: Sachin Kamat <sachin.kamat@linaro.org>
>
> Signed-off-by: Pramod Gurav <pramod.gurav@smartplayin.com>
> ---
> drivers/input/misc/soc_button_array.c | 19 ++++++++++++++-----
> 1 file changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/input/misc/soc_button_array.c b/drivers/input/misc/soc_button_array.c
> index 5a6334b..ac82971 100644
> --- a/drivers/input/misc/soc_button_array.c
> +++ b/drivers/input/misc/soc_button_array.c
> @@ -83,6 +83,13 @@ soc_button_device_create(struct pnp_dev *pdev,
> sizeof(*gpio_keys_pdata) +
> sizeof(*gpio_keys) * MAX_NBUTTONS,
> GFP_KERNEL);
> + if (!gpio_keys_pdata) {
> + dev_err(&pdev->dev,
> + "Can't allocate memory for gpio_keys_platform_data\n");
OOM message is not needed as memory subsystem will take care of it.
> + error = -ENOMEM;
> + goto err_mem;
Return directly from here.
> + }
> +
> gpio_keys = (void *)(gpio_keys_pdata + 1);
>
> for (info = button_info; info->name; info++) {
> @@ -104,7 +111,7 @@ soc_button_device_create(struct pnp_dev *pdev,
>
> if (n_buttons == 0) {
> error = -ENODEV;
> - goto err_free_mem;
> + goto err_mem;
ditto
> }
>
> gpio_keys_pdata->buttons = gpio_keys;
> @@ -114,7 +121,7 @@ soc_button_device_create(struct pnp_dev *pdev,
> pd = platform_device_alloc("gpio-keys", PLATFORM_DEVID_AUTO);
> if (!pd) {
> error = -ENOMEM;
> - goto err_free_mem;
> + goto err_mem;
ditto
> }
>
> error = platform_device_add_data(pd, gpio_keys_pdata,
> @@ -130,8 +137,7 @@ soc_button_device_create(struct pnp_dev *pdev,
>
> err_free_pdev:
> platform_device_put(pd);
> -err_free_mem:
> - devm_kfree(&pdev->dev, gpio_keys_pdata);
> +err_mem:
This label would not be required once returned directly from above.
> return ERR_PTR(error);
> }
>
> @@ -155,8 +161,11 @@ static int soc_button_pnp_probe(struct pnp_dev *pdev,
> int error;
>
> priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> - if (!priv)
> + if (!priv) {
> + dev_err(&pdev->dev,
> + "Can't allocate memory for soc_button_data\n");
OOM message is not needed as memory subsystem will take care of it.
--
Regards,
Sachin.
^ permalink raw reply
* Re: [PATCH] Input: soc_button_array: Remove kfree on data allocated with devm_zalloc
From: pramod gurav @ 2014-07-25 8:48 UTC (permalink / raw)
To: Sachin Kamat
Cc: linux-input, open list, Pramod Gurav, Dmitry Torokhov, Lejun Zhu,
Sachin Kamat
In-Reply-To: <CAK5sBcFMxKzB8KgSDTtfsGV173TV6Ji3VFNETVqgpGeHPS7LEg@mail.gmail.com>
Thanks Sachin for the review. Will resend the patch addressing your
review comments.
On Fri, Jul 25, 2014 at 1:59 PM, Sachin Kamat <spk.linux@gmail.com> wrote:
> On Fri, Jul 25, 2014 at 12:39 PM, <pramod.gurav.etc@gmail.com> wrote:
>> From: Pramod Gurav <pramod.gurav@smartplayin.com>
>>
>> This patch does below:
>> - Removes kfree done on data allocated with devm_zalloc in probe
>> path of the driver.
>> - Adds a check on return value from devm_kzalloc which was missing
>> and renames a lable from err_free_mem to err_mem.
>> - Adds couple of dev_err on failure to allocate memory
>>
>> CC: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>> CC: Lejun Zhu <lejun.zhu@linux.intel.com>
>> CC: Sachin Kamat <sachin.kamat@linaro.org>
>>
>> Signed-off-by: Pramod Gurav <pramod.gurav@smartplayin.com>
>> ---
>> drivers/input/misc/soc_button_array.c | 19 ++++++++++++++-----
>> 1 file changed, 14 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/input/misc/soc_button_array.c b/drivers/input/misc/soc_button_array.c
>> index 5a6334b..ac82971 100644
>> --- a/drivers/input/misc/soc_button_array.c
>> +++ b/drivers/input/misc/soc_button_array.c
>> @@ -83,6 +83,13 @@ soc_button_device_create(struct pnp_dev *pdev,
>> sizeof(*gpio_keys_pdata) +
>> sizeof(*gpio_keys) * MAX_NBUTTONS,
>> GFP_KERNEL);
>> + if (!gpio_keys_pdata) {
>> + dev_err(&pdev->dev,
>> + "Can't allocate memory for gpio_keys_platform_data\n");
>
> OOM message is not needed as memory subsystem will take care of it.
>
>> + error = -ENOMEM;
>> + goto err_mem;
>
> Return directly from here.
>
>> + }
>> +
>> gpio_keys = (void *)(gpio_keys_pdata + 1);
>>
>> for (info = button_info; info->name; info++) {
>> @@ -104,7 +111,7 @@ soc_button_device_create(struct pnp_dev *pdev,
>>
>> if (n_buttons == 0) {
>> error = -ENODEV;
>> - goto err_free_mem;
>> + goto err_mem;
>
> ditto
>
>> }
>>
>> gpio_keys_pdata->buttons = gpio_keys;
>> @@ -114,7 +121,7 @@ soc_button_device_create(struct pnp_dev *pdev,
>> pd = platform_device_alloc("gpio-keys", PLATFORM_DEVID_AUTO);
>> if (!pd) {
>> error = -ENOMEM;
>> - goto err_free_mem;
>> + goto err_mem;
>
> ditto
>
>> }
>>
>> error = platform_device_add_data(pd, gpio_keys_pdata,
>> @@ -130,8 +137,7 @@ soc_button_device_create(struct pnp_dev *pdev,
>>
>> err_free_pdev:
>> platform_device_put(pd);
>> -err_free_mem:
>> - devm_kfree(&pdev->dev, gpio_keys_pdata);
>> +err_mem:
>
> This label would not be required once returned directly from above.
>
>> return ERR_PTR(error);
>> }
>>
>> @@ -155,8 +161,11 @@ static int soc_button_pnp_probe(struct pnp_dev *pdev,
>> int error;
>>
>> priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
>> - if (!priv)
>> + if (!priv) {
>> + dev_err(&pdev->dev,
>> + "Can't allocate memory for soc_button_data\n");
>
> OOM message is not needed as memory subsystem will take care of it.
>
>
> --
> Regards,
> Sachin.
--
Thanks and Regards
Pramod
^ permalink raw reply
* Re: [PATCH] Input: soc_button_array: Remove kfree on data allocated with devm_zalloc
From: David Rientjes @ 2014-07-25 9:02 UTC (permalink / raw)
To: Sachin Kamat
Cc: pramod.gurav.etc, linux-input, open list, Pramod Gurav,
Dmitry Torokhov, Lejun Zhu, Sachin Kamat
In-Reply-To: <CAK5sBcFMxKzB8KgSDTtfsGV173TV6Ji3VFNETVqgpGeHPS7LEg@mail.gmail.com>
On Fri, 25 Jul 2014, Sachin Kamat wrote:
> On Fri, Jul 25, 2014 at 12:39 PM, <pramod.gurav.etc@gmail.com> wrote:
> > From: Pramod Gurav <pramod.gurav@smartplayin.com>
> >
> > This patch does below:
> > - Removes kfree done on data allocated with devm_zalloc in probe
> > path of the driver.
> > - Adds a check on return value from devm_kzalloc which was missing
> > and renames a lable from err_free_mem to err_mem.
> > - Adds couple of dev_err on failure to allocate memory
> >
> > CC: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > CC: Lejun Zhu <lejun.zhu@linux.intel.com>
> > CC: Sachin Kamat <sachin.kamat@linaro.org>
> >
> > Signed-off-by: Pramod Gurav <pramod.gurav@smartplayin.com>
> > ---
> > drivers/input/misc/soc_button_array.c | 19 ++++++++++++++-----
> > 1 file changed, 14 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/input/misc/soc_button_array.c b/drivers/input/misc/soc_button_array.c
> > index 5a6334b..ac82971 100644
> > --- a/drivers/input/misc/soc_button_array.c
> > +++ b/drivers/input/misc/soc_button_array.c
> > @@ -83,6 +83,13 @@ soc_button_device_create(struct pnp_dev *pdev,
> > sizeof(*gpio_keys_pdata) +
> > sizeof(*gpio_keys) * MAX_NBUTTONS,
> > GFP_KERNEL);
> > + if (!gpio_keys_pdata) {
> > + dev_err(&pdev->dev,
> > + "Can't allocate memory for gpio_keys_platform_data\n");
>
> OOM message is not needed as memory subsystem will take care of it.
>
In the worst case, the memory subsystem will print that a process was
killed for this allocation to succeed so it's never actually going to
fail. However, it is indeed frowed upon to print your own oom message so
you're right that it should be removed.
Same comment about soc_button_pnp_probe().
^ permalink raw reply
* [PATCH v2] Input: soc_button_array: Remove kfree on data allocated with devm_zalloc
From: pramod.gurav.etc @ 2014-07-25 9:12 UTC (permalink / raw)
To: linux-input, linux-kernel
Cc: Pramod Gurav, Dmitry Torokhov, Lejun Zhu, Sachin Kamat
From: Pramod Gurav <pramod.gurav@smartplayin.com>
This patch does below:
- Removes kfree done on data allocated with devm_zalloc in probe
path of the driver.
- Adds a check on return value from devm_kzalloc which was missing
- Removes the error lables and instead releases resources and returns
after failures
CC: Dmitry Torokhov <dmitry.torokhov@gmail.com>
CC: Lejun Zhu <lejun.zhu@linux.intel.com>
CC: Sachin Kamat <sachin.kamat@linaro.org>
Signed-off-by: Pramod Gurav <pramod.gurav@smartplayin.com>
---
Changes since v1:
- Removes de_err logs on OOM cases which is not needed.
drivers/input/misc/soc_button_array.c | 27 +++++++++++++++------------
1 file changed, 15 insertions(+), 12 deletions(-)
diff --git a/drivers/input/misc/soc_button_array.c b/drivers/input/misc/soc_button_array.c
index 5a6334b..f421cf4 100644
--- a/drivers/input/misc/soc_button_array.c
+++ b/drivers/input/misc/soc_button_array.c
@@ -83,6 +83,11 @@ soc_button_device_create(struct pnp_dev *pdev,
sizeof(*gpio_keys_pdata) +
sizeof(*gpio_keys) * MAX_NBUTTONS,
GFP_KERNEL);
+ if (!gpio_keys_pdata) {
+ error = -ENOMEM;
+ return ERR_PTR(error);
+ }
+
gpio_keys = (void *)(gpio_keys_pdata + 1);
for (info = button_info; info->name; info++) {
@@ -104,7 +109,7 @@ soc_button_device_create(struct pnp_dev *pdev,
if (n_buttons == 0) {
error = -ENODEV;
- goto err_free_mem;
+ return ERR_PTR(error);
}
gpio_keys_pdata->buttons = gpio_keys;
@@ -114,25 +119,23 @@ soc_button_device_create(struct pnp_dev *pdev,
pd = platform_device_alloc("gpio-keys", PLATFORM_DEVID_AUTO);
if (!pd) {
error = -ENOMEM;
- goto err_free_mem;
+ return ERR_PTR(error);
}
error = platform_device_add_data(pd, gpio_keys_pdata,
sizeof(*gpio_keys_pdata));
- if (error)
- goto err_free_pdev;
+ if (error) {
+ platform_device_put(pd);
+ return ERR_PTR(error);
+ }
error = platform_device_add(pd);
- if (error)
- goto err_free_pdev;
+ if (error) {
+ platform_device_put(pd);
+ return ERR_PTR(error);
+ }
return pd;
-
-err_free_pdev:
- platform_device_put(pd);
-err_free_mem:
- devm_kfree(&pdev->dev, gpio_keys_pdata);
- return ERR_PTR(error);
}
static void soc_button_remove(struct pnp_dev *pdev)
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH v2] Input: soc_button_array: Remove kfree on data allocated with devm_zalloc
From: Sachin Kamat @ 2014-07-25 10:29 UTC (permalink / raw)
To: pramod gurav
Cc: linux-input, open list, Pramod Gurav, Dmitry Torokhov, Lejun Zhu,
Sachin Kamat
In-Reply-To: <1406279557-26754-1-git-send-email-pramod.gurav@smartplayin.com>
Hi Pramod,
On Fri, Jul 25, 2014 at 2:42 PM, <pramod.gurav.etc@gmail.com> wrote:
> From: Pramod Gurav <pramod.gurav@smartplayin.com>
>
> This patch does below:
> - Removes kfree done on data allocated with devm_zalloc in probe
> path of the driver.
> - Adds a check on return value from devm_kzalloc which was missing
> - Removes the error lables and instead releases resources and returns
> after failures
>
> CC: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> CC: Lejun Zhu <lejun.zhu@linux.intel.com>
> CC: Sachin Kamat <sachin.kamat@linaro.org>
>
> Signed-off-by: Pramod Gurav <pramod.gurav@smartplayin.com>
> ---
>
> Changes since v1:
> - Removes de_err logs on OOM cases which is not needed.
>
> drivers/input/misc/soc_button_array.c | 27 +++++++++++++++------------
> 1 file changed, 15 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/input/misc/soc_button_array.c b/drivers/input/misc/soc_button_array.c
> index 5a6334b..f421cf4 100644
> --- a/drivers/input/misc/soc_button_array.c
> +++ b/drivers/input/misc/soc_button_array.c
> @@ -83,6 +83,11 @@ soc_button_device_create(struct pnp_dev *pdev,
> sizeof(*gpio_keys_pdata) +
> sizeof(*gpio_keys) * MAX_NBUTTONS,
> GFP_KERNEL);
> + if (!gpio_keys_pdata) {
> + error = -ENOMEM;
> + return ERR_PTR(error);
You don't need a variable here. You could directly do:
return ERR_PTR(-ENOMEM);
> + }
> +
> gpio_keys = (void *)(gpio_keys_pdata + 1);
>
> for (info = button_info; info->name; info++) {
> @@ -104,7 +109,7 @@ soc_button_device_create(struct pnp_dev *pdev,
>
> if (n_buttons == 0) {
> error = -ENODEV;
> - goto err_free_mem;
> + return ERR_PTR(error);
ditto
> }
>
> gpio_keys_pdata->buttons = gpio_keys;
> @@ -114,25 +119,23 @@ soc_button_device_create(struct pnp_dev *pdev,
> pd = platform_device_alloc("gpio-keys", PLATFORM_DEVID_AUTO);
> if (!pd) {
> error = -ENOMEM;
> - goto err_free_mem;
> + return ERR_PTR(error);
ditto
> }
>
> error = platform_device_add_data(pd, gpio_keys_pdata,
> sizeof(*gpio_keys_pdata));
> - if (error)
> - goto err_free_pdev;
This goto could be retained to avoid code duplication.
> + if (error) {
> + platform_device_put(pd);
> + return ERR_PTR(error);
> + }
>
> error = platform_device_add(pd);
> - if (error)
> - goto err_free_pdev;
ditto
> + if (error) {
> + platform_device_put(pd);
> + return ERR_PTR(error);
> + }
>
> return pd;
> -
> -err_free_pdev:
> - platform_device_put(pd);
> -err_free_mem:
> - devm_kfree(&pdev->dev, gpio_keys_pdata);
> - return ERR_PTR(error);
> }
>
> static void soc_button_remove(struct pnp_dev *pdev)
> --
> 1.7.9.5
>
--
Regards,
Sachin.
^ permalink raw reply
* Re: [PATCH v2] Input: soc_button_array: Remove kfree on data allocated with devm_zalloc
From: pramod gurav @ 2014-07-25 10:44 UTC (permalink / raw)
To: Sachin Kamat
Cc: linux-input, open list, Pramod Gurav, Dmitry Torokhov, Lejun Zhu,
Sachin Kamat
In-Reply-To: <CAK5sBcH6hunJZxnhLGdbAzbRCdgWr9+L=JcrhXc2zSLFAbRRdQ@mail.gmail.com>
On Fri, Jul 25, 2014 at 3:59 PM, Sachin Kamat <spk.linux@gmail.com> wrote:
> Hi Pramod,
>
>
> On Fri, Jul 25, 2014 at 2:42 PM, <pramod.gurav.etc@gmail.com> wrote:
>> From: Pramod Gurav <pramod.gurav@smartplayin.com>
>>
>> This patch does below:
>> - Removes kfree done on data allocated with devm_zalloc in probe
>> path of the driver.
>> - Adds a check on return value from devm_kzalloc which was missing
>> - Removes the error lables and instead releases resources and returns
>> after failures
>>
>> CC: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>> CC: Lejun Zhu <lejun.zhu@linux.intel.com>
>> CC: Sachin Kamat <sachin.kamat@linaro.org>
>>
>> Signed-off-by: Pramod Gurav <pramod.gurav@smartplayin.com>
>> ---
>>
>> Changes since v1:
>> - Removes de_err logs on OOM cases which is not needed.
>>
>> drivers/input/misc/soc_button_array.c | 27 +++++++++++++++------------
>> 1 file changed, 15 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/input/misc/soc_button_array.c b/drivers/input/misc/soc_button_array.c
>> index 5a6334b..f421cf4 100644
>> --- a/drivers/input/misc/soc_button_array.c
>> +++ b/drivers/input/misc/soc_button_array.c
>> @@ -83,6 +83,11 @@ soc_button_device_create(struct pnp_dev *pdev,
>> sizeof(*gpio_keys_pdata) +
>> sizeof(*gpio_keys) * MAX_NBUTTONS,
>> GFP_KERNEL);
>> + if (!gpio_keys_pdata) {
>> + error = -ENOMEM;
>> + return ERR_PTR(error);
>
> You don't need a variable here. You could directly do:
I tried to retain the original code as it is. I was not sure if am
supposed to do that. Wil do it now.
> return ERR_PTR(-ENOMEM);
>
>> + }
>> +
>> gpio_keys = (void *)(gpio_keys_pdata + 1);
>>
>> for (info = button_info; info->name; info++) {
>> @@ -104,7 +109,7 @@ soc_button_device_create(struct pnp_dev *pdev,
>>
>> if (n_buttons == 0) {
>> error = -ENODEV;
>> - goto err_free_mem;
>> + return ERR_PTR(error);
>
> ditto
>
>> }
>>
>> gpio_keys_pdata->buttons = gpio_keys;
>> @@ -114,25 +119,23 @@ soc_button_device_create(struct pnp_dev *pdev,
>> pd = platform_device_alloc("gpio-keys", PLATFORM_DEVID_AUTO);
>> if (!pd) {
>> error = -ENOMEM;
>> - goto err_free_mem;
>> + return ERR_PTR(error);
>
> ditto
>
>> }
>>
>> error = platform_device_add_data(pd, gpio_keys_pdata,
>> sizeof(*gpio_keys_pdata));
>> - if (error)
>> - goto err_free_pdev;
>
> This goto could be retained to avoid code duplication.
If I keep this here, I have to handle the following returns in this
lable, which is what my previous patch was doing. So lable would look
something like this:
+err_free_pdev:
+ platform_device_put(pd);
+ return ERR_PTR(error);
>
>> + if (error) {
>> + platform_device_put(pd);
>> + return ERR_PTR(error);
>> + }
>>
>> error = platform_device_add(pd);
>> - if (error)
>> - goto err_free_pdev;
>
> ditto
>
>> + if (error) {
>> + platform_device_put(pd);
>> + return ERR_PTR(error);
>> + }
>>
>> return pd;
>> -
>> -err_free_pdev:
>> - platform_device_put(pd);
>> -err_free_mem:
>> - devm_kfree(&pdev->dev, gpio_keys_pdata);
>> - return ERR_PTR(error);
>> }
>>
>> static void soc_button_remove(struct pnp_dev *pdev)
>> --
>> 1.7.9.5
>>
>
>
>
> --
> Regards,
> Sachin.
--
Thanks and Regards
Pramod
^ permalink raw reply
* [PATCH v2] Input: soc_button_array: Remove kfree on data allocated with devm_zalloc
From: pramod.gurav.etc @ 2014-07-25 11:34 UTC (permalink / raw)
To: linux-input, linux-kernel
Cc: Pramod Gurav, Dmitry Torokhov, Lejun Zhu, Sachin Kamat
From: Pramod Gurav <pramod.gurav@smartplayin.com>
This patch does below:
- Removes kfree done on data allocated with devm_zalloc in probe
path of the driver.
- Adds a check on return value from devm_kzalloc which was missing
CC: Dmitry Torokhov <dmitry.torokhov@gmail.com>
CC: Lejun Zhu <lejun.zhu@linux.intel.com>
CC: Sachin Kamat <sachin.kamat@linaro.org>
Signed-off-by: Pramod Gurav <pramod.gurav@smartplayin.com>
---
drivers/input/misc/soc_button_array.c | 17 +++++++----------
1 file changed, 7 insertions(+), 10 deletions(-)
diff --git a/drivers/input/misc/soc_button_array.c b/drivers/input/misc/soc_button_array.c
index 5a6334b..fc64ec6 100644
--- a/drivers/input/misc/soc_button_array.c
+++ b/drivers/input/misc/soc_button_array.c
@@ -83,6 +83,9 @@ soc_button_device_create(struct pnp_dev *pdev,
sizeof(*gpio_keys_pdata) +
sizeof(*gpio_keys) * MAX_NBUTTONS,
GFP_KERNEL);
+ if (!gpio_keys_pdata)
+ return ERR_PTR(-ENOMEM);
+
gpio_keys = (void *)(gpio_keys_pdata + 1);
for (info = button_info; info->name; info++) {
@@ -102,20 +105,16 @@ soc_button_device_create(struct pnp_dev *pdev,
n_buttons++;
}
- if (n_buttons == 0) {
- error = -ENODEV;
- goto err_free_mem;
- }
+ if (n_buttons == 0)
+ return ERR_PTR(-ENODEV);
gpio_keys_pdata->buttons = gpio_keys;
gpio_keys_pdata->nbuttons = n_buttons;
gpio_keys_pdata->rep = autorepeat;
pd = platform_device_alloc("gpio-keys", PLATFORM_DEVID_AUTO);
- if (!pd) {
- error = -ENOMEM;
- goto err_free_mem;
- }
+ if (!pd)
+ return ERR_PTR(-ENOMEM);
error = platform_device_add_data(pd, gpio_keys_pdata,
sizeof(*gpio_keys_pdata));
@@ -130,8 +129,6 @@ soc_button_device_create(struct pnp_dev *pdev,
err_free_pdev:
platform_device_put(pd);
-err_free_mem:
- devm_kfree(&pdev->dev, gpio_keys_pdata);
return ERR_PTR(error);
}
--
1.7.9.5
^ permalink raw reply related
* re: Input: atmel_mxt_ts - initialise IRQ before probing
From: Dan Carpenter @ 2014-07-25 12:20 UTC (permalink / raw)
Cc: Nick Dyer, linux-input
Hello Nick Dyer,
The patch dd24dcf566d0: "Input: atmel_mxt_ts - initialise IRQ before
probing" from Jul 23, 2014, leads to the following static checker
warning:
drivers/input/touchscreen/atmel_mxt_ts.c:1701 mxt_initialize()
warn: we tested 'error' before and it was 'false'
drivers/input/touchscreen/atmel_mxt_ts.c
1693 /* Get object table information */
1694 error = mxt_get_object_table(data);
1695 if (error) {
1696 dev_err(&client->dev, "Error %d reading object table\n", error);
1697 return error;
1698 }
1699
1700 mxt_acquire_irq(data);
Probably error = mxt_acquire_irq(data) was intended here?
1701 if (error)
1702 goto err_free_object_table;
1703
1704 request_firmware_nowait(THIS_MODULE, true, MXT_CFG_NAME,
1705 &data->client->dev, GFP_KERNEL, data,
1706 mxt_config_cb);
1707
1708 return 0;
1709
1710 err_free_object_table:
1711 mxt_free_object_table(data);
1712 return error;
1713 }
regards,
dan carpenter
^ permalink raw reply
* Re: [PATCH v2 00/10] Input - wacom: conversion to HID driver, series 2
From: Przemo Firszt @ 2014-07-25 12:42 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Dmitry Torokhov, Jiri Kosina, Ping Cheng, Jason Gerecke,
linux-kernel, linux-input
In-Reply-To: <1406225645-32141-1-git-send-email-benjamin.tissoires@redhat.com>
Dnia 2014-07-24, czw o godzinie 14:13 -0400, Benjamin Tissoires pisze:
[..]
> Hi Przemo,
Hi Benjamin,
> Normally, this series contains all the bits of hid-wacom (except the custom
> LED/OLED API). Please tell me if I am missing anything and if you like the
> change.
I can't cleanly apply your set yet to test it, so:
Acked-by: Przemo Firszt <przemo@firszt.eu>
What's your plan about LED/OLED API?
> Benjamin Tissoires (9):
> Input - wacom: put a flag when the led are initialized
> Input - wacom: enhance Wireless Receiver battery reporting
> Input - wacom: use a uniq name for the battery device
> Input - wacom: register an ac power supply for wireless devices
> Input - wacom: prepare the driver to include BT devices
> Input - wacom: handle Graphire BT tablets in wacom.ko
> Input - wacom: handle Intuos 4 BT in wacom.ko
> Input - wacom: add copyright note and bump version to 2.0
> HID: remove hid-wacom Bluetooth driver
>
> Jason Gerecke (1):
> Input - wacom: Support up to 2048 pressure levels with ISDv4
>
> drivers/hid/Kconfig | 10 +-
> drivers/hid/Makefile | 3 +-
> drivers/hid/hid-core.c | 2 -
> drivers/hid/hid-wacom.c | 973 ------------------------------------------------
> drivers/hid/wacom.h | 11 +
> drivers/hid/wacom_sys.c | 217 ++++++++++-
> drivers/hid/wacom_wac.c | 195 +++++++++-
> drivers/hid/wacom_wac.h | 10 +
> 8 files changed, 415 insertions(+), 1006 deletions(-)
> delete mode 100644 drivers/hid/hid-wacom.c
>
--
Kind regards,
Przemo Firszt
^ permalink raw reply
* Re: [PATCH v5 1/4] HID: lenovo: Rename hid-lenovo-tpkbd to hid-lenovo
From: Antonio Ospite @ 2014-07-25 12:56 UTC (permalink / raw)
To: Jamie Lentin; +Cc: Jiri Kosina, Hans de Goede, linux-input, linux-kernel
In-Reply-To: <1406154648-14000-2-git-send-email-jm@lentin.co.uk>
On Wed, 23 Jul 2014 23:30:45 +0100
Jamie Lentin <jm@lentin.co.uk> wrote:
> Rename module and all functions within so we can add support for other
> keyboards in the same file. Rename the _tp postfix to _tpkbd, to
> signify functions relevant to the TP USB keyboard.
>
> Signed-off-by: Jamie Lentin <jm@lentin.co.uk>
Reviewed-by: Antonio Ospite <ao2@ao2.it>
> ---
> ...er-hid-lenovo-tpkbd => sysfs-driver-hid-lenovo} | 0
> drivers/hid/Kconfig | 14 +-
> drivers/hid/Makefile | 2 +-
> drivers/hid/hid-core.c | 2 +-
> drivers/hid/{hid-lenovo-tpkbd.c => hid-lenovo.c} | 185 +++++++++++----------
> 5 files changed, 102 insertions(+), 101 deletions(-)
> rename Documentation/ABI/testing/{sysfs-driver-hid-lenovo-tpkbd => sysfs-driver-hid-lenovo} (100%)
> rename drivers/hid/{hid-lenovo-tpkbd.c => hid-lenovo.c} (64%)
>
> diff --git a/Documentation/ABI/testing/sysfs-driver-hid-lenovo-tpkbd b/Documentation/ABI/testing/sysfs-driver-hid-lenovo
> similarity index 100%
> rename from Documentation/ABI/testing/sysfs-driver-hid-lenovo-tpkbd
> rename to Documentation/ABI/testing/sysfs-driver-hid-lenovo
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index 800c8b6..9b7acfc 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -331,18 +331,18 @@ config HID_LCPOWER
> ---help---
> Support for LC-Power RC1000MCE RF remote control.
>
> -config HID_LENOVO_TPKBD
> - tristate "Lenovo ThinkPad USB Keyboard with TrackPoint"
> +config HID_LENOVO
> + tristate "Lenovo / Thinkpad devices"
> depends on HID
> select NEW_LEDS
> select LEDS_CLASS
> ---help---
> - Support for the Lenovo ThinkPad USB Keyboard with TrackPoint.
> + Support for Lenovo devices that are not fully compliant with HID standard.
>
> - Say Y here if you have a Lenovo ThinkPad USB Keyboard with TrackPoint
> - and would like to use device-specific features like changing the
> - sensitivity of the trackpoint, using the microphone mute button or
> - controlling the mute and microphone mute LEDs.
> + Say Y if you want support for the non-compliant features of the Lenovo
> + Thinkpad standalone keyboards, e.g:
> + - ThinkPad USB Keyboard with TrackPoint (supports extra LEDs and trackpoint
> + configuration)
>
> config HID_LOGITECH
> tristate "Logitech devices" if EXPERT
> diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
> index a6fa6ba..5e96be3 100644
> --- a/drivers/hid/Makefile
> +++ b/drivers/hid/Makefile
> @@ -59,7 +59,7 @@ obj-$(CONFIG_HID_KENSINGTON) += hid-kensington.o
> obj-$(CONFIG_HID_KEYTOUCH) += hid-keytouch.o
> obj-$(CONFIG_HID_KYE) += hid-kye.o
> obj-$(CONFIG_HID_LCPOWER) += hid-lcpower.o
> -obj-$(CONFIG_HID_LENOVO_TPKBD) += hid-lenovo-tpkbd.o
> +obj-$(CONFIG_HID_LENOVO) += hid-lenovo.o
> obj-$(CONFIG_HID_LOGITECH) += hid-logitech.o
> obj-$(CONFIG_HID_LOGITECH_DJ) += hid-logitech-dj.o
> obj-$(CONFIG_HID_MAGICMOUSE) += hid-magicmouse.o
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index 8ed66fd..55841bd 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -1796,7 +1796,7 @@ static const struct hid_device_id hid_have_special_driver[] = {
> { HID_USB_DEVICE(USB_VENDOR_ID_KYE, USB_DEVICE_ID_KYE_EASYPEN_M610X) },
> { HID_USB_DEVICE(USB_VENDOR_ID_LABTEC, USB_DEVICE_ID_LABTEC_WIRELESS_KEYBOARD) },
> { HID_USB_DEVICE(USB_VENDOR_ID_LCPOWER, USB_DEVICE_ID_LCPOWER_LC1000 ) },
> -#if IS_ENABLED(CONFIG_HID_LENOVO_TPKBD)
> +#if IS_ENABLED(CONFIG_HID_LENOVO)
> { HID_USB_DEVICE(USB_VENDOR_ID_LENOVO, USB_DEVICE_ID_LENOVO_TPKBD) },
> #endif
> { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_MX3000_RECEIVER) },
> diff --git a/drivers/hid/hid-lenovo-tpkbd.c b/drivers/hid/hid-lenovo.c
> similarity index 64%
> rename from drivers/hid/hid-lenovo-tpkbd.c
> rename to drivers/hid/hid-lenovo.c
> index 2d25b6c..0320b96 100644
> --- a/drivers/hid/hid-lenovo-tpkbd.c
> +++ b/drivers/hid/hid-lenovo.c
> @@ -20,8 +20,7 @@
>
> #include "hid-ids.h"
>
> -/* This is only used for the trackpoint part of the driver, hence _tp */
> -struct tpkbd_data_pointer {
> +struct lenovo_drvdata_tpkbd {
> int led_state;
> struct led_classdev led_mute;
> struct led_classdev led_micmute;
> @@ -35,7 +34,7 @@ struct tpkbd_data_pointer {
>
> #define map_key_clear(c) hid_map_usage_clear(hi, usage, bit, max, EV_KEY, (c))
>
> -static int tpkbd_input_mapping(struct hid_device *hdev,
> +static int lenovo_input_mapping_tpkbd(struct hid_device *hdev,
> struct hid_input *hi, struct hid_field *field,
> struct hid_usage *usage, unsigned long **bit, int *max)
> {
> @@ -50,10 +49,10 @@ static int tpkbd_input_mapping(struct hid_device *hdev,
>
> #undef map_key_clear
>
> -static int tpkbd_features_set(struct hid_device *hdev)
> +static int lenovo_features_set_tpkbd(struct hid_device *hdev)
> {
> struct hid_report *report;
> - struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev);
> + struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev);
>
> report = hdev->report_enum[HID_FEATURE_REPORT].report_id_hash[4];
>
> @@ -69,23 +68,23 @@ static int tpkbd_features_set(struct hid_device *hdev)
> return 0;
> }
>
> -static ssize_t pointer_press_to_select_show(struct device *dev,
> +static ssize_t attr_press_to_select_show_tpkbd(struct device *dev,
> struct device_attribute *attr,
> char *buf)
> {
> struct hid_device *hdev = container_of(dev, struct hid_device, dev);
> - struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev);
> + struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev);
>
> return snprintf(buf, PAGE_SIZE, "%u\n", data_pointer->press_to_select);
> }
>
> -static ssize_t pointer_press_to_select_store(struct device *dev,
> +static ssize_t attr_press_to_select_store_tpkbd(struct device *dev,
> struct device_attribute *attr,
> const char *buf,
> size_t count)
> {
> struct hid_device *hdev = container_of(dev, struct hid_device, dev);
> - struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev);
> + struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev);
> int value;
>
> if (kstrtoint(buf, 10, &value))
> @@ -94,28 +93,28 @@ static ssize_t pointer_press_to_select_store(struct device *dev,
> return -EINVAL;
>
> data_pointer->press_to_select = value;
> - tpkbd_features_set(hdev);
> + lenovo_features_set_tpkbd(hdev);
>
> return count;
> }
>
> -static ssize_t pointer_dragging_show(struct device *dev,
> +static ssize_t attr_dragging_show_tpkbd(struct device *dev,
> struct device_attribute *attr,
> char *buf)
> {
> struct hid_device *hdev = container_of(dev, struct hid_device, dev);
> - struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev);
> + struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev);
>
> return snprintf(buf, PAGE_SIZE, "%u\n", data_pointer->dragging);
> }
>
> -static ssize_t pointer_dragging_store(struct device *dev,
> +static ssize_t attr_dragging_store_tpkbd(struct device *dev,
> struct device_attribute *attr,
> const char *buf,
> size_t count)
> {
> struct hid_device *hdev = container_of(dev, struct hid_device, dev);
> - struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev);
> + struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev);
> int value;
>
> if (kstrtoint(buf, 10, &value))
> @@ -124,28 +123,28 @@ static ssize_t pointer_dragging_store(struct device *dev,
> return -EINVAL;
>
> data_pointer->dragging = value;
> - tpkbd_features_set(hdev);
> + lenovo_features_set_tpkbd(hdev);
>
> return count;
> }
>
> -static ssize_t pointer_release_to_select_show(struct device *dev,
> +static ssize_t attr_release_to_select_show_tpkbd(struct device *dev,
> struct device_attribute *attr,
> char *buf)
> {
> struct hid_device *hdev = container_of(dev, struct hid_device, dev);
> - struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev);
> + struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev);
>
> return snprintf(buf, PAGE_SIZE, "%u\n", data_pointer->release_to_select);
> }
>
> -static ssize_t pointer_release_to_select_store(struct device *dev,
> +static ssize_t attr_release_to_select_store_tpkbd(struct device *dev,
> struct device_attribute *attr,
> const char *buf,
> size_t count)
> {
> struct hid_device *hdev = container_of(dev, struct hid_device, dev);
> - struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev);
> + struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev);
> int value;
>
> if (kstrtoint(buf, 10, &value))
> @@ -154,28 +153,28 @@ static ssize_t pointer_release_to_select_store(struct device *dev,
> return -EINVAL;
>
> data_pointer->release_to_select = value;
> - tpkbd_features_set(hdev);
> + lenovo_features_set_tpkbd(hdev);
>
> return count;
> }
>
> -static ssize_t pointer_select_right_show(struct device *dev,
> +static ssize_t attr_select_right_show_tpkbd(struct device *dev,
> struct device_attribute *attr,
> char *buf)
> {
> struct hid_device *hdev = container_of(dev, struct hid_device, dev);
> - struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev);
> + struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev);
>
> return snprintf(buf, PAGE_SIZE, "%u\n", data_pointer->select_right);
> }
>
> -static ssize_t pointer_select_right_store(struct device *dev,
> +static ssize_t attr_select_right_store_tpkbd(struct device *dev,
> struct device_attribute *attr,
> const char *buf,
> size_t count)
> {
> struct hid_device *hdev = container_of(dev, struct hid_device, dev);
> - struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev);
> + struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev);
> int value;
>
> if (kstrtoint(buf, 10, &value))
> @@ -184,119 +183,119 @@ static ssize_t pointer_select_right_store(struct device *dev,
> return -EINVAL;
>
> data_pointer->select_right = value;
> - tpkbd_features_set(hdev);
> + lenovo_features_set_tpkbd(hdev);
>
> return count;
> }
>
> -static ssize_t pointer_sensitivity_show(struct device *dev,
> +static ssize_t attr_sensitivity_show_tpkbd(struct device *dev,
> struct device_attribute *attr,
> char *buf)
> {
> struct hid_device *hdev = container_of(dev, struct hid_device, dev);
> - struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev);
> + struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev);
>
> return snprintf(buf, PAGE_SIZE, "%u\n",
> data_pointer->sensitivity);
> }
>
> -static ssize_t pointer_sensitivity_store(struct device *dev,
> +static ssize_t attr_sensitivity_store_tpkbd(struct device *dev,
> struct device_attribute *attr,
> const char *buf,
> size_t count)
> {
> struct hid_device *hdev = container_of(dev, struct hid_device, dev);
> - struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev);
> + struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev);
> int value;
>
> if (kstrtoint(buf, 10, &value) || value < 1 || value > 255)
> return -EINVAL;
>
> data_pointer->sensitivity = value;
> - tpkbd_features_set(hdev);
> + lenovo_features_set_tpkbd(hdev);
>
> return count;
> }
>
> -static ssize_t pointer_press_speed_show(struct device *dev,
> +static ssize_t attr_press_speed_show_tpkbd(struct device *dev,
> struct device_attribute *attr,
> char *buf)
> {
> struct hid_device *hdev = container_of(dev, struct hid_device, dev);
> - struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev);
> + struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev);
>
> return snprintf(buf, PAGE_SIZE, "%u\n",
> data_pointer->press_speed);
> }
>
> -static ssize_t pointer_press_speed_store(struct device *dev,
> +static ssize_t attr_press_speed_store_tpkbd(struct device *dev,
> struct device_attribute *attr,
> const char *buf,
> size_t count)
> {
> struct hid_device *hdev = container_of(dev, struct hid_device, dev);
> - struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev);
> + struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev);
> int value;
>
> if (kstrtoint(buf, 10, &value) || value < 1 || value > 255)
> return -EINVAL;
>
> data_pointer->press_speed = value;
> - tpkbd_features_set(hdev);
> + lenovo_features_set_tpkbd(hdev);
>
> return count;
> }
>
> -static struct device_attribute dev_attr_pointer_press_to_select =
> +static struct device_attribute dev_attr_press_to_select_tpkbd =
> __ATTR(press_to_select, S_IWUSR | S_IRUGO,
> - pointer_press_to_select_show,
> - pointer_press_to_select_store);
> + attr_press_to_select_show_tpkbd,
> + attr_press_to_select_store_tpkbd);
>
> -static struct device_attribute dev_attr_pointer_dragging =
> +static struct device_attribute dev_attr_dragging_tpkbd =
> __ATTR(dragging, S_IWUSR | S_IRUGO,
> - pointer_dragging_show,
> - pointer_dragging_store);
> + attr_dragging_show_tpkbd,
> + attr_dragging_store_tpkbd);
>
> -static struct device_attribute dev_attr_pointer_release_to_select =
> +static struct device_attribute dev_attr_release_to_select_tpkbd =
> __ATTR(release_to_select, S_IWUSR | S_IRUGO,
> - pointer_release_to_select_show,
> - pointer_release_to_select_store);
> + attr_release_to_select_show_tpkbd,
> + attr_release_to_select_store_tpkbd);
>
> -static struct device_attribute dev_attr_pointer_select_right =
> +static struct device_attribute dev_attr_select_right_tpkbd =
> __ATTR(select_right, S_IWUSR | S_IRUGO,
> - pointer_select_right_show,
> - pointer_select_right_store);
> + attr_select_right_show_tpkbd,
> + attr_select_right_store_tpkbd);
>
> -static struct device_attribute dev_attr_pointer_sensitivity =
> +static struct device_attribute dev_attr_sensitivity_tpkbd =
> __ATTR(sensitivity, S_IWUSR | S_IRUGO,
> - pointer_sensitivity_show,
> - pointer_sensitivity_store);
> + attr_sensitivity_show_tpkbd,
> + attr_sensitivity_store_tpkbd);
>
> -static struct device_attribute dev_attr_pointer_press_speed =
> +static struct device_attribute dev_attr_press_speed_tpkbd =
> __ATTR(press_speed, S_IWUSR | S_IRUGO,
> - pointer_press_speed_show,
> - pointer_press_speed_store);
> -
> -static struct attribute *tpkbd_attributes_pointer[] = {
> - &dev_attr_pointer_press_to_select.attr,
> - &dev_attr_pointer_dragging.attr,
> - &dev_attr_pointer_release_to_select.attr,
> - &dev_attr_pointer_select_right.attr,
> - &dev_attr_pointer_sensitivity.attr,
> - &dev_attr_pointer_press_speed.attr,
> + attr_press_speed_show_tpkbd,
> + attr_press_speed_store_tpkbd);
> +
> +static struct attribute *lenovo_attributes_tpkbd[] = {
> + &dev_attr_press_to_select_tpkbd.attr,
> + &dev_attr_dragging_tpkbd.attr,
> + &dev_attr_release_to_select_tpkbd.attr,
> + &dev_attr_select_right_tpkbd.attr,
> + &dev_attr_sensitivity_tpkbd.attr,
> + &dev_attr_press_speed_tpkbd.attr,
> NULL
> };
>
> -static const struct attribute_group tpkbd_attr_group_pointer = {
> - .attrs = tpkbd_attributes_pointer,
> +static const struct attribute_group lenovo_attr_group_tpkbd = {
> + .attrs = lenovo_attributes_tpkbd,
> };
>
> -static enum led_brightness tpkbd_led_brightness_get(
> +static enum led_brightness lenovo_led_brightness_get_tpkbd(
> struct led_classdev *led_cdev)
> {
> struct device *dev = led_cdev->dev->parent;
> struct hid_device *hdev = container_of(dev, struct hid_device, dev);
> - struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev);
> + struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev);
> int led_nr = 0;
>
> if (led_cdev == &data_pointer->led_micmute)
> @@ -307,12 +306,12 @@ static enum led_brightness tpkbd_led_brightness_get(
> : LED_OFF;
> }
>
> -static void tpkbd_led_brightness_set(struct led_classdev *led_cdev,
> +static void lenovo_led_brightness_set_tpkbd(struct led_classdev *led_cdev,
> enum led_brightness value)
> {
> struct device *dev = led_cdev->dev->parent;
> struct hid_device *hdev = container_of(dev, struct hid_device, dev);
> - struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev);
> + struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev);
> struct hid_report *report;
> int led_nr = 0;
>
> @@ -330,10 +329,10 @@ static void tpkbd_led_brightness_set(struct led_classdev *led_cdev,
> hid_hw_request(hdev, report, HID_REQ_SET_REPORT);
> }
>
> -static int tpkbd_probe_tp(struct hid_device *hdev)
> +static int lenovo_probe_tpkbd(struct hid_device *hdev)
> {
> struct device *dev = &hdev->dev;
> - struct tpkbd_data_pointer *data_pointer;
> + struct lenovo_drvdata_tpkbd *data_pointer;
> size_t name_sz = strlen(dev_name(dev)) + 16;
> char *name_mute, *name_micmute;
> int i;
> @@ -347,12 +346,12 @@ static int tpkbd_probe_tp(struct hid_device *hdev)
> return -ENODEV;
>
> if (sysfs_create_group(&hdev->dev.kobj,
> - &tpkbd_attr_group_pointer)) {
> + &lenovo_attr_group_tpkbd)) {
> hid_warn(hdev, "Could not create sysfs group\n");
> }
>
> data_pointer = devm_kzalloc(&hdev->dev,
> - sizeof(struct tpkbd_data_pointer),
> + sizeof(struct lenovo_drvdata_tpkbd),
> GFP_KERNEL);
> if (data_pointer == NULL) {
> hid_err(hdev, "Could not allocate memory for driver data\n");
> @@ -375,23 +374,25 @@ static int tpkbd_probe_tp(struct hid_device *hdev)
> hid_set_drvdata(hdev, data_pointer);
>
> data_pointer->led_mute.name = name_mute;
> - data_pointer->led_mute.brightness_get = tpkbd_led_brightness_get;
> - data_pointer->led_mute.brightness_set = tpkbd_led_brightness_set;
> + data_pointer->led_mute.brightness_get = lenovo_led_brightness_get_tpkbd;
> + data_pointer->led_mute.brightness_set = lenovo_led_brightness_set_tpkbd;
> data_pointer->led_mute.dev = dev;
> led_classdev_register(dev, &data_pointer->led_mute);
>
> data_pointer->led_micmute.name = name_micmute;
> - data_pointer->led_micmute.brightness_get = tpkbd_led_brightness_get;
> - data_pointer->led_micmute.brightness_set = tpkbd_led_brightness_set;
> + data_pointer->led_micmute.brightness_get =
> + lenovo_led_brightness_get_tpkbd;
> + data_pointer->led_micmute.brightness_set =
> + lenovo_led_brightness_set_tpkbd;
> data_pointer->led_micmute.dev = dev;
> led_classdev_register(dev, &data_pointer->led_micmute);
>
> - tpkbd_features_set(hdev);
> + lenovo_features_set_tpkbd(hdev);
>
> return 0;
> }
>
> -static int tpkbd_probe(struct hid_device *hdev,
> +static int lenovo_probe(struct hid_device *hdev,
> const struct hid_device_id *id)
> {
> int ret;
> @@ -410,7 +411,7 @@ static int tpkbd_probe(struct hid_device *hdev,
>
> if (hid_get_drvdata(hdev)) {
> hid_set_drvdata(hdev, NULL);
> - ret = tpkbd_probe_tp(hdev);
> + ret = lenovo_probe_tpkbd(hdev);
> if (ret)
> goto err_hid;
> }
> @@ -422,12 +423,12 @@ err:
> return ret;
> }
>
> -static void tpkbd_remove_tp(struct hid_device *hdev)
> +static void lenovo_remove_tpkbd(struct hid_device *hdev)
> {
> - struct tpkbd_data_pointer *data_pointer = hid_get_drvdata(hdev);
> + struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev);
>
> sysfs_remove_group(&hdev->dev.kobj,
> - &tpkbd_attr_group_pointer);
> + &lenovo_attr_group_tpkbd);
>
> led_classdev_unregister(&data_pointer->led_micmute);
> led_classdev_unregister(&data_pointer->led_mute);
> @@ -435,28 +436,28 @@ static void tpkbd_remove_tp(struct hid_device *hdev)
> hid_set_drvdata(hdev, NULL);
> }
>
> -static void tpkbd_remove(struct hid_device *hdev)
> +static void lenovo_remove(struct hid_device *hdev)
> {
> if (hid_get_drvdata(hdev))
> - tpkbd_remove_tp(hdev);
> + lenovo_remove_tpkbd(hdev);
>
> hid_hw_stop(hdev);
> }
>
> -static const struct hid_device_id tpkbd_devices[] = {
> +static const struct hid_device_id lenovo_devices[] = {
> { HID_USB_DEVICE(USB_VENDOR_ID_LENOVO, USB_DEVICE_ID_LENOVO_TPKBD) },
> { }
> };
>
> -MODULE_DEVICE_TABLE(hid, tpkbd_devices);
> +MODULE_DEVICE_TABLE(hid, lenovo_devices);
>
> -static struct hid_driver tpkbd_driver = {
> - .name = "lenovo_tpkbd",
> - .id_table = tpkbd_devices,
> - .input_mapping = tpkbd_input_mapping,
> - .probe = tpkbd_probe,
> - .remove = tpkbd_remove,
> +static struct hid_driver lenovo_driver = {
> + .name = "lenovo",
> + .id_table = lenovo_devices,
> + .input_mapping = lenovo_input_mapping_tpkbd,
> + .probe = lenovo_probe,
> + .remove = lenovo_remove,
> };
> -module_hid_driver(tpkbd_driver);
> +module_hid_driver(lenovo_driver);
>
> MODULE_LICENSE("GPL");
> --
> 2.0.0
>
--
Antonio Ospite
http://ao2.it
A: Because it messes up the order in which people normally read text.
See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?
^ permalink raw reply
* Re: [PATCH v5 2/4] HID: lenovo: Prepare support for adding other devices
From: Antonio Ospite @ 2014-07-25 12:58 UTC (permalink / raw)
To: Jamie Lentin; +Cc: Jiri Kosina, Hans de Goede, linux-input, linux-kernel
In-Reply-To: <1406154648-14000-3-git-send-email-jm@lentin.co.uk>
On Wed, 23 Jul 2014 23:30:46 +0100
Jamie Lentin <jm@lentin.co.uk> wrote:
> Ensure all tpkbd specifics are within a postfixed function, the
> main functions for the driver should just switch to the appropriate
> function depending on product ID. Given this, we can add extra devices
> by including extra postfixed functions.
>
> Signed-off-by: Jamie Lentin <jm@lentin.co.uk>
>
Reviewed-by: Antonio Ospite <ao2@ao2.it>
> ---
> drivers/hid/hid-lenovo.c | 53 ++++++++++++++++++++++++++++++++++++++++--------
> 1 file changed, 45 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/hid/hid-lenovo.c b/drivers/hid/hid-lenovo.c
> index 0320b96..a56b9e7 100644
> --- a/drivers/hid/hid-lenovo.c
> +++ b/drivers/hid/hid-lenovo.c
> @@ -1,5 +1,6 @@
> /*
> - * HID driver for Lenovo ThinkPad USB Keyboard with TrackPoint
> + * HID driver for Lenovo:
> + * - ThinkPad USB Keyboard with TrackPoint (tpkbd)
> *
> * Copyright (c) 2012 Bernhard Seibold
> */
> @@ -39,7 +40,7 @@ static int lenovo_input_mapping_tpkbd(struct hid_device *hdev,
> struct hid_usage *usage, unsigned long **bit, int *max)
> {
> if (usage->hid == (HID_UP_BUTTON | 0x0010)) {
> - /* mark the device as pointer */
> + /* This sub-device contains trackpoint, mark it */
> hid_set_drvdata(hdev, (void *)1);
> map_key_clear(KEY_MICMUTE);
> return 1;
> @@ -47,6 +48,19 @@ static int lenovo_input_mapping_tpkbd(struct hid_device *hdev,
> return 0;
> }
>
> +static int lenovo_input_mapping(struct hid_device *hdev,
> + struct hid_input *hi, struct hid_field *field,
> + struct hid_usage *usage, unsigned long **bit, int *max)
> +{
> + switch (hdev->product) {
> + case USB_DEVICE_ID_LENOVO_TPKBD:
> + return lenovo_input_mapping_tpkbd(hdev, hi, field,
> + usage, bit, max);
> + default:
> + return 0;
> + }
> +}
> +
> #undef map_key_clear
>
> static int lenovo_features_set_tpkbd(struct hid_device *hdev)
> @@ -337,6 +351,15 @@ static int lenovo_probe_tpkbd(struct hid_device *hdev)
> char *name_mute, *name_micmute;
> int i;
>
> + /*
> + * Only register extra settings against subdevice where input_mapping
> + * set drvdata to 1, i.e. the trackpoint.
> + */
> + if (!hid_get_drvdata(hdev))
> + return 0;
> +
> + hid_set_drvdata(hdev, NULL);
> +
> /* Validate required reports. */
> for (i = 0; i < 4; i++) {
> if (!hid_validate_values(hdev, HID_FEATURE_REPORT, 4, i, 1))
> @@ -409,12 +432,16 @@ static int lenovo_probe(struct hid_device *hdev,
> goto err;
> }
>
> - if (hid_get_drvdata(hdev)) {
> - hid_set_drvdata(hdev, NULL);
> + switch (hdev->product) {
> + case USB_DEVICE_ID_LENOVO_TPKBD:
> ret = lenovo_probe_tpkbd(hdev);
> - if (ret)
> - goto err_hid;
> + break;
> + default:
> + ret = 0;
> + break;
> }
> + if (ret)
> + goto err_hid;
>
> return 0;
> err_hid:
> @@ -427,6 +454,13 @@ static void lenovo_remove_tpkbd(struct hid_device *hdev)
> {
> struct lenovo_drvdata_tpkbd *data_pointer = hid_get_drvdata(hdev);
>
> + /*
> + * Only the trackpoint half of the keyboard has drvdata and stuff that
> + * needs unregistering.
> + */
> + if (data_pointer == NULL)
> + return;
> +
> sysfs_remove_group(&hdev->dev.kobj,
> &lenovo_attr_group_tpkbd);
>
> @@ -438,8 +472,11 @@ static void lenovo_remove_tpkbd(struct hid_device *hdev)
>
> static void lenovo_remove(struct hid_device *hdev)
> {
> - if (hid_get_drvdata(hdev))
> + switch (hdev->product) {
> + case USB_DEVICE_ID_LENOVO_TPKBD:
> lenovo_remove_tpkbd(hdev);
> + break;
> + }
>
> hid_hw_stop(hdev);
> }
> @@ -454,7 +491,7 @@ MODULE_DEVICE_TABLE(hid, lenovo_devices);
> static struct hid_driver lenovo_driver = {
> .name = "lenovo",
> .id_table = lenovo_devices,
> - .input_mapping = lenovo_input_mapping_tpkbd,
> + .input_mapping = lenovo_input_mapping,
> .probe = lenovo_probe,
> .remove = lenovo_remove,
> };
> --
> 2.0.0
>
>
--
Antonio Ospite
http://ao2.it
A: Because it messes up the order in which people normally read text.
See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?
^ permalink raw reply
* Re: [PATCH v2 00/10] Input - wacom: conversion to HID driver, series 2
From: Benjamin Tissoires @ 2014-07-25 12:58 UTC (permalink / raw)
To: Przemo Firszt
Cc: Dmitry Torokhov, Jiri Kosina, Ping Cheng, Jason Gerecke,
linux-kernel, linux-input
In-Reply-To: <1406292140.2664.17.camel@fedora-lan>
Hi Przemo,
On Jul 25 2014 or thereabouts, Przemo Firszt wrote:
> Dnia 2014-07-24, czw o godzinie 14:13 -0400, Benjamin Tissoires pisze:
> [..]
> > Hi Przemo,
> Hi Benjamin,
> > Normally, this series contains all the bits of hid-wacom (except the custom
> > LED/OLED API). Please tell me if I am missing anything and if you like the
> > change.
>
> I can't cleanly apply your set yet to test it, so:
Hmm, you need to apply the first series I sent on July 15th, on top of
Dmitry's next branch.
http://www.spinics.net/lists/linux-input/msg32385.html
>
> Acked-by: Przemo Firszt <przemo@firszt.eu>
Thanks!
>
> What's your plan about LED/OLED API?
I thought I would only preserve the current, wider used, LED/OLED API and
just drop the one in hid-wacom. This way, g-s-d will access both
bluetooth and USB the same way.
My decision was mostly guided because the support of BT in g-s-d was
only added recently (3.12 and backported to gnome 3.10 IIRC). And as
soon as these patches hit Dmitry's tree, I'll send the g-s-d patches to
fix all that.
Cheers,
Benjamin
>
> > Benjamin Tissoires (9):
> > Input - wacom: put a flag when the led are initialized
> > Input - wacom: enhance Wireless Receiver battery reporting
> > Input - wacom: use a uniq name for the battery device
> > Input - wacom: register an ac power supply for wireless devices
> > Input - wacom: prepare the driver to include BT devices
> > Input - wacom: handle Graphire BT tablets in wacom.ko
> > Input - wacom: handle Intuos 4 BT in wacom.ko
> > Input - wacom: add copyright note and bump version to 2.0
> > HID: remove hid-wacom Bluetooth driver
> >
> > Jason Gerecke (1):
> > Input - wacom: Support up to 2048 pressure levels with ISDv4
> >
> > drivers/hid/Kconfig | 10 +-
> > drivers/hid/Makefile | 3 +-
> > drivers/hid/hid-core.c | 2 -
> > drivers/hid/hid-wacom.c | 973 ------------------------------------------------
> > drivers/hid/wacom.h | 11 +
> > drivers/hid/wacom_sys.c | 217 ++++++++++-
> > drivers/hid/wacom_wac.c | 195 +++++++++-
> > drivers/hid/wacom_wac.h | 10 +
> > 8 files changed, 415 insertions(+), 1006 deletions(-)
> > delete mode 100644 drivers/hid/hid-wacom.c
> >
>
> --
> Kind regards,
> Przemo Firszt
>
>
^ permalink raw reply
* Re: [PATCH v2 00/10] Input - wacom: conversion to HID driver, series 2
From: Przemo Firszt @ 2014-07-25 13:30 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Dmitry Torokhov, Jiri Kosina, Ping Cheng, Jason Gerecke,
linux-kernel, linux-input
In-Reply-To: <20140725125820.GA12268@mail.corp.redhat.com>
Dnia 2014-07-25, pią o godzinie 08:58 -0400, Benjamin Tissoires pisze:
> Hi Przemo,
Hi Benjamin,
> On Jul 25 2014 or thereabouts, Przemo Firszt wrote:
> > Dnia 2014-07-24, czw o godzinie 14:13 -0400, Benjamin Tissoires pisze:
> > [..]
> > > Hi Przemo,
> > Hi Benjamin,
> > > Normally, this series contains all the bits of hid-wacom (except the custom
> > > LED/OLED API). Please tell me if I am missing anything and if you like the
> > > change.
> >
> > I can't cleanly apply your set yet to test it, so:
>
> Hmm, you need to apply the first series I sent on July 15th, on top of
> Dmitry's next branch.
>
> http://www.spinics.net/lists/linux-input/msg32385.html
>
OK, thnaks!
> > Acked-by: Przemo Firszt <przemo@firszt.eu>
>
> Thanks!
>
> >
> > What's your plan about LED/OLED API?
>
> I thought I would only preserve the current, wider used, LED/OLED API and
> just drop the one in hid-wacom. This way, g-s-d will access both
> bluetooth and USB the same way.
>
> My decision was mostly guided because the support of BT in g-s-d was
> only added recently (3.12 and backported to gnome 3.10 IIRC). And as
> soon as these patches hit Dmitry's tree, I'll send the g-s-d patches to
> fix all that.
If I understand you correctly we cannot have the same entry point in
sysfs for OLEDs unless we can tell userspace somehow if the tablet is
conected over USB or over bluetooth. The hardware of Intuos4 Wireless
over bluetooth allows only 1-bit images. The same hardware over USB
allows 4-bit images. Formatting of the images is also completely
different and it's not "plain". Check [1] for usb and exisitng
hid-wacom.c/wacom_scramble function for bluetooth.
If we want to make it consistent single entry on kernel level we
probably have to implement image conversion in the kernel. The user land
would always use 4-bit, plain formatted images and kernel driver would
convert them to 4-bit, wacom usb format or 1-bit wacom bluetooth format
depending on how the tablet is connected.
The downside of this approach is that the user land wouldn't have 100%
control over 1-bit images for bluetooth as kernel would have to create
them from 4-bit images.
[1] https://lkml.org/lkml/2012/9/9/80
--
Kind regards,
Przemo Firszt
--
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 v5 3/4] HID: lenovo: Style fixes
From: Antonio Ospite @ 2014-07-25 13:33 UTC (permalink / raw)
To: Jamie Lentin; +Cc: Jiri Kosina, Hans de Goede, linux-input, linux-kernel
In-Reply-To: <1406154648-14000-4-git-send-email-jm@lentin.co.uk>
On Wed, 23 Jul 2014 23:30:47 +0100
Jamie Lentin <jm@lentin.co.uk> wrote:
> Signed-off-by: Jamie Lentin <jm@lentin.co.uk>
Jamie, please send a v6 of this patch alone[*], where you print the
return value as Dmitry suggested . You do print it in a similar
statement of the subsequent patch, so this makes even more sense.
FWIW I'd already take the patch as it is and consider it a readability
improvement, but I now realize that the suggestion of not calling
functions in conditions is not mentioned anywhere in
Documentation/CodingStyle, so maybe I would just be over-zealous.
Ciao,
Antonio
[*] You can use the --in-relpy-to option of git-send-email to make it a
reply to your v5 3/4
> ---
> drivers/hid/hid-lenovo.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/hid/hid-lenovo.c b/drivers/hid/hid-lenovo.c
> index a56b9e7..6d55939 100644
> --- a/drivers/hid/hid-lenovo.c
> +++ b/drivers/hid/hid-lenovo.c
> @@ -350,6 +350,7 @@ static int lenovo_probe_tpkbd(struct hid_device *hdev)
> size_t name_sz = strlen(dev_name(dev)) + 16;
> char *name_mute, *name_micmute;
> int i;
> + int ret;
>
> /*
> * Only register extra settings against subdevice where input_mapping
> @@ -368,10 +369,9 @@ static int lenovo_probe_tpkbd(struct hid_device *hdev)
> if (!hid_validate_values(hdev, HID_OUTPUT_REPORT, 3, 0, 2))
> return -ENODEV;
>
> - if (sysfs_create_group(&hdev->dev.kobj,
> - &lenovo_attr_group_tpkbd)) {
> + ret = sysfs_create_group(&hdev->dev.kobj, &lenovo_attr_group_tpkbd);
> + if (ret)
> hid_warn(hdev, "Could not create sysfs group\n");
> - }
>
> data_pointer = devm_kzalloc(&hdev->dev,
> sizeof(struct lenovo_drvdata_tpkbd),
> --
> 2.0.0
>
--
Antonio Ospite
http://ao2.it
A: Because it messes up the order in which people normally read text.
See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?
^ permalink raw reply
* Re: [PATCH v5 4/4] HID: lenovo: Add support for Compact (BT|USB) keyboard
From: Antonio Ospite @ 2014-07-25 13:39 UTC (permalink / raw)
To: Jamie Lentin; +Cc: Jiri Kosina, Hans de Goede, linux-input, linux-kernel
In-Reply-To: <1406154648-14000-5-git-send-email-jm@lentin.co.uk>
On Wed, 23 Jul 2014 23:30:48 +0100
Jamie Lentin <jm@lentin.co.uk> wrote:
> Add support for both ThinkPad Compact Bluetooth Keyboard with
> TrackPoint and ThinkPad Compact USB Keyboard with TrackPoint.
>
> Signed-off-by: Jamie Lentin <jm@lentin.co.uk>
I think this version is OK, thanks.
Reviewed-by: Antonio Ospite <ao2@ao2.it>
> ---
> Documentation/ABI/testing/sysfs-driver-hid-lenovo | 12 ++
> drivers/hid/Kconfig | 2 +
> drivers/hid/hid-core.c | 2 +
> drivers/hid/hid-ids.h | 2 +
> drivers/hid/hid-lenovo.c | 208 ++++++++++++++++++++++
> include/linux/hid.h | 1 +
> 6 files changed, 227 insertions(+)
>
> diff --git a/Documentation/ABI/testing/sysfs-driver-hid-lenovo b/Documentation/ABI/testing/sysfs-driver-hid-lenovo
> index 57b92cb..53a0725 100644
> --- a/Documentation/ABI/testing/sysfs-driver-hid-lenovo
> +++ b/Documentation/ABI/testing/sysfs-driver-hid-lenovo
> @@ -4,18 +4,21 @@ Contact: linux-input@vger.kernel.org
> Description: This controls if mouse clicks should be generated if the trackpoint is quickly pressed. How fast this press has to be
> is being controlled by press_speed.
> Values are 0 or 1.
> + Applies to Thinkpad USB Keyboard with TrackPoint.
>
> What: /sys/bus/usb/devices/<busnum>-<devnum>:<config num>.<interface num>/<hid-bus>:<vendor-id>:<product-id>.<num>/dragging
> Date: July 2011
> Contact: linux-input@vger.kernel.org
> Description: If this setting is enabled, it is possible to do dragging by pressing the trackpoint. This requires press_to_select to be enabled.
> Values are 0 or 1.
> + Applies to Thinkpad USB Keyboard with TrackPoint.
>
> What: /sys/bus/usb/devices/<busnum>-<devnum>:<config num>.<interface num>/<hid-bus>:<vendor-id>:<product-id>.<num>/release_to_select
> Date: July 2011
> Contact: linux-input@vger.kernel.org
> Description: For details regarding this setting please refer to http://www.pc.ibm.com/ww/healthycomputing/trkpntb.html
> Values are 0 or 1.
> + Applies to Thinkpad USB Keyboard with TrackPoint.
>
> What: /sys/bus/usb/devices/<busnum>-<devnum>:<config num>.<interface num>/<hid-bus>:<vendor-id>:<product-id>.<num>/select_right
> Date: July 2011
> @@ -23,16 +26,25 @@ Contact: linux-input@vger.kernel.org
> Description: This setting controls if the mouse click events generated by pressing the trackpoint (if press_to_select is enabled) generate
> a left or right mouse button click.
> Values are 0 or 1.
> + Applies to Thinkpad USB Keyboard with TrackPoint.
>
> What: /sys/bus/usb/devices/<busnum>-<devnum>:<config num>.<interface num>/<hid-bus>:<vendor-id>:<product-id>.<num>/sensitivity
> Date: July 2011
> Contact: linux-input@vger.kernel.org
> Description: This file contains the trackpoint sensitivity.
> Values are decimal integers from 1 (lowest sensitivity) to 255 (highest sensitivity).
> + Applies to Thinkpad USB Keyboard with TrackPoint.
>
> What: /sys/bus/usb/devices/<busnum>-<devnum>:<config num>.<interface num>/<hid-bus>:<vendor-id>:<product-id>.<num>/press_speed
> Date: July 2011
> Contact: linux-input@vger.kernel.org
> Description: This setting controls how fast the trackpoint needs to be pressed to generate a mouse click if press_to_select is enabled.
> Values are decimal integers from 1 (slowest) to 255 (fastest).
> + Applies to Thinkpad USB Keyboard with TrackPoint.
>
> +What: /sys/bus/usb/devices/<busnum>-<devnum>:<config num>.<interface num>/<hid-bus>:<vendor-id>:<product-id>.<num>/fn_lock
> +Date: July 2014
> +Contact: linux-input@vger.kernel.org
> +Description: This setting controls whether Fn Lock is enabled on the keyboard (i.e. if F1 is Mute or F1)
> + Values are 0 or 1
> + Applies to ThinkPad Compact (USB|Bluetooth) Keyboard with TrackPoint.
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index 9b7acfc..1e19292 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -343,6 +343,8 @@ config HID_LENOVO
> Thinkpad standalone keyboards, e.g:
> - ThinkPad USB Keyboard with TrackPoint (supports extra LEDs and trackpoint
> configuration)
> + - ThinkPad Compact Bluetooth Keyboard with TrackPoint (supports Fn keys)
> + - ThinkPad Compact USB Keyboard with TrackPoint (supports Fn keys)
>
> config HID_LOGITECH
> tristate "Logitech devices" if EXPERT
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index 55841bd..81b3bb6 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -1798,6 +1798,8 @@ static const struct hid_device_id hid_have_special_driver[] = {
> { HID_USB_DEVICE(USB_VENDOR_ID_LCPOWER, USB_DEVICE_ID_LCPOWER_LC1000 ) },
> #if IS_ENABLED(CONFIG_HID_LENOVO)
> { HID_USB_DEVICE(USB_VENDOR_ID_LENOVO, USB_DEVICE_ID_LENOVO_TPKBD) },
> + { HID_USB_DEVICE(USB_VENDOR_ID_LENOVO, USB_DEVICE_ID_LENOVO_CUSBKBD) },
> + { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LENOVO, USB_DEVICE_ID_LENOVO_CBTKBD) },
> #endif
> { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_MX3000_RECEIVER) },
> { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_S510_RECEIVER) },
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index 6d00bb9..d2e2a96 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -560,6 +560,8 @@
>
> #define USB_VENDOR_ID_LENOVO 0x17ef
> #define USB_DEVICE_ID_LENOVO_TPKBD 0x6009
> +#define USB_DEVICE_ID_LENOVO_CUSBKBD 0x6047
> +#define USB_DEVICE_ID_LENOVO_CBTKBD 0x6048
>
> #define USB_VENDOR_ID_LG 0x1fd2
> #define USB_DEVICE_ID_LG_MULTITOUCH 0x0064
> diff --git a/drivers/hid/hid-lenovo.c b/drivers/hid/hid-lenovo.c
> index 6d55939..eb5493e 100644
> --- a/drivers/hid/hid-lenovo.c
> +++ b/drivers/hid/hid-lenovo.c
> @@ -1,8 +1,11 @@
> /*
> * HID driver for Lenovo:
> * - ThinkPad USB Keyboard with TrackPoint (tpkbd)
> + * - ThinkPad Compact Bluetooth Keyboard with TrackPoint (cptkbd)
> + * - ThinkPad Compact USB Keyboard with TrackPoint (cptkbd)
> *
> * Copyright (c) 2012 Bernhard Seibold
> + * Copyright (c) 2014 Jamie Lentin <jm@lentin.co.uk>
> */
>
> /*
> @@ -33,6 +36,10 @@ struct lenovo_drvdata_tpkbd {
> int press_speed;
> };
>
> +struct lenovo_drvdata_cptkbd {
> + bool fn_lock;
> +};
> +
> #define map_key_clear(c) hid_map_usage_clear(hi, usage, bit, max, EV_KEY, (c))
>
> static int lenovo_input_mapping_tpkbd(struct hid_device *hdev,
> @@ -48,6 +55,49 @@ static int lenovo_input_mapping_tpkbd(struct hid_device *hdev,
> return 0;
> }
>
> +static int lenovo_input_mapping_cptkbd(struct hid_device *hdev,
> + struct hid_input *hi, struct hid_field *field,
> + struct hid_usage *usage, unsigned long **bit, int *max)
> +{
> + /* HID_UP_LNVENDOR = USB, HID_UP_MSVENDOR = BT */
> + if ((usage->hid & HID_USAGE_PAGE) == HID_UP_MSVENDOR ||
> + (usage->hid & HID_USAGE_PAGE) == HID_UP_LNVENDOR) {
> + set_bit(EV_REP, hi->input->evbit);
> + switch (usage->hid & HID_USAGE) {
> + case 0x00f1: /* Fn-F4: Mic mute */
> + map_key_clear(KEY_MICMUTE);
> + return 1;
> + case 0x00f2: /* Fn-F5: Brightness down */
> + map_key_clear(KEY_BRIGHTNESSDOWN);
> + return 1;
> + case 0x00f3: /* Fn-F6: Brightness up */
> + map_key_clear(KEY_BRIGHTNESSUP);
> + return 1;
> + case 0x00f4: /* Fn-F7: External display (projector) */
> + map_key_clear(KEY_SWITCHVIDEOMODE);
> + return 1;
> + case 0x00f5: /* Fn-F8: Wireless */
> + map_key_clear(KEY_WLAN);
> + return 1;
> + case 0x00f6: /* Fn-F9: Control panel */
> + map_key_clear(KEY_CONFIG);
> + return 1;
> + case 0x00f8: /* Fn-F11: View open applications (3 boxes) */
> + map_key_clear(KEY_SCALE);
> + return 1;
> + case 0x00fa: /* Fn-Esc: Fn-lock toggle */
> + map_key_clear(KEY_FN_ESC);
> + return 1;
> + case 0x00fb: /* Fn-F12: Open My computer (6 boxes) USB-only */
> + /* NB: This mapping is invented in raw_event below */
> + map_key_clear(KEY_FILE);
> + return 1;
> + }
> + }
> +
> + return 0;
> +}
> +
> static int lenovo_input_mapping(struct hid_device *hdev,
> struct hid_input *hi, struct hid_field *field,
> struct hid_usage *usage, unsigned long **bit, int *max)
> @@ -56,6 +106,10 @@ static int lenovo_input_mapping(struct hid_device *hdev,
> case USB_DEVICE_ID_LENOVO_TPKBD:
> return lenovo_input_mapping_tpkbd(hdev, hi, field,
> usage, bit, max);
> + case USB_DEVICE_ID_LENOVO_CUSBKBD:
> + case USB_DEVICE_ID_LENOVO_CBTKBD:
> + return lenovo_input_mapping_cptkbd(hdev, hi, field,
> + usage, bit, max);
> default:
> return 0;
> }
> @@ -63,6 +117,103 @@ static int lenovo_input_mapping(struct hid_device *hdev,
>
> #undef map_key_clear
>
> +/* Send a config command to the keyboard */
> +static int lenovo_send_cmd_cptkbd(struct hid_device *hdev,
> + unsigned char byte2, unsigned char byte3)
> +{
> + int ret;
> + unsigned char buf[] = {0x18, byte2, byte3};
> +
> + switch (hdev->product) {
> + case USB_DEVICE_ID_LENOVO_CUSBKBD:
> + ret = hid_hw_raw_request(hdev, 0x13, buf, sizeof(buf),
> + HID_FEATURE_REPORT, HID_REQ_SET_REPORT);
> + break;
> + case USB_DEVICE_ID_LENOVO_CBTKBD:
> + ret = hid_hw_output_report(hdev, buf, sizeof(buf));
> + break;
> + default:
> + ret = -EINVAL;
> + break;
> + }
> +
> + return ret < 0 ? ret : 0; /* BT returns 0, USB returns sizeof(buf) */
> +}
> +
> +static void lenovo_features_set_cptkbd(struct hid_device *hdev)
> +{
> + int ret;
> + struct lenovo_drvdata_cptkbd *cptkbd_data = hid_get_drvdata(hdev);
> +
> + ret = lenovo_send_cmd_cptkbd(hdev, 0x05, cptkbd_data->fn_lock);
> + if (ret)
> + hid_err(hdev, "Fn-lock setting failed: %d\n", ret);
> +}
> +
> +static ssize_t attr_fn_lock_show_cptkbd(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct hid_device *hdev = container_of(dev, struct hid_device, dev);
> + struct lenovo_drvdata_cptkbd *cptkbd_data = hid_get_drvdata(hdev);
> +
> + return snprintf(buf, PAGE_SIZE, "%u\n", cptkbd_data->fn_lock);
> +}
> +
> +static ssize_t attr_fn_lock_store_cptkbd(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf,
> + size_t count)
> +{
> + struct hid_device *hdev = container_of(dev, struct hid_device, dev);
> + struct lenovo_drvdata_cptkbd *cptkbd_data = hid_get_drvdata(hdev);
> + int value;
> +
> + if (kstrtoint(buf, 10, &value))
> + return -EINVAL;
> + if (value < 0 || value > 1)
> + return -EINVAL;
> +
> + cptkbd_data->fn_lock = !!value;
> + lenovo_features_set_cptkbd(hdev);
> +
> + return count;
> +}
> +
> +static struct device_attribute dev_attr_fn_lock_cptkbd =
> + __ATTR(fn_lock, S_IWUSR | S_IRUGO,
> + attr_fn_lock_show_cptkbd,
> + attr_fn_lock_store_cptkbd);
> +
> +static struct attribute *lenovo_attributes_cptkbd[] = {
> + &dev_attr_fn_lock_cptkbd.attr,
> + NULL
> +};
> +
> +static const struct attribute_group lenovo_attr_group_cptkbd = {
> + .attrs = lenovo_attributes_cptkbd,
> +};
> +
> +static int lenovo_raw_event(struct hid_device *hdev,
> + struct hid_report *report, u8 *data, int size)
> +{
> + /*
> + * Compact USB keyboard's Fn-F12 report holds down many other keys, and
> + * its own key is outside the usage page range. Remove extra
> + * keypresses and remap to inside usage page.
> + */
> + if (unlikely(hdev->product == USB_DEVICE_ID_LENOVO_CUSBKBD
> + && size == 3
> + && data[0] == 0x15
> + && data[1] == 0x94
> + && data[2] == 0x01)) {
> + data[1] = 0x0;
> + data[2] = 0x4;
> + }
> +
> + return 0;
> +}
> +
> static int lenovo_features_set_tpkbd(struct hid_device *hdev)
> {
> struct hid_report *report;
> @@ -415,6 +566,46 @@ static int lenovo_probe_tpkbd(struct hid_device *hdev)
> return 0;
> }
>
> +static int lenovo_probe_cptkbd(struct hid_device *hdev)
> +{
> + int ret;
> + struct lenovo_drvdata_cptkbd *cptkbd_data;
> +
> + /* All the custom action happens on the USBMOUSE device for USB */
> + if (hdev->product == USB_DEVICE_ID_LENOVO_CUSBKBD
> + && hdev->type != HID_TYPE_USBMOUSE) {
> + hid_dbg(hdev, "Ignoring keyboard half of device\n");
> + return 0;
> + }
> +
> + cptkbd_data = devm_kzalloc(&hdev->dev,
> + sizeof(*cptkbd_data),
> + GFP_KERNEL);
> + if (cptkbd_data == NULL) {
> + hid_err(hdev, "can't alloc keyboard descriptor\n");
> + return -ENOMEM;
> + }
> + hid_set_drvdata(hdev, cptkbd_data);
> +
> + /*
> + * Tell the keyboard a driver understands it, and turn F7, F9, F11 into
> + * regular keys
> + */
> + ret = lenovo_send_cmd_cptkbd(hdev, 0x01, 0x03);
> + if (ret)
> + hid_warn(hdev, "Failed to switch F7/9/11 mode: %d\n", ret);
> +
> + /* Turn Fn-Lock on by default */
> + cptkbd_data->fn_lock = true;
> + lenovo_features_set_cptkbd(hdev);
> +
> + ret = sysfs_create_group(&hdev->dev.kobj, &lenovo_attr_group_cptkbd);
> + if (ret)
> + hid_warn(hdev, "Could not create sysfs group: %d\n", ret);
> +
> + return 0;
> +}
> +
> static int lenovo_probe(struct hid_device *hdev,
> const struct hid_device_id *id)
> {
> @@ -436,6 +627,10 @@ static int lenovo_probe(struct hid_device *hdev,
> case USB_DEVICE_ID_LENOVO_TPKBD:
> ret = lenovo_probe_tpkbd(hdev);
> break;
> + case USB_DEVICE_ID_LENOVO_CUSBKBD:
> + case USB_DEVICE_ID_LENOVO_CBTKBD:
> + ret = lenovo_probe_cptkbd(hdev);
> + break;
> default:
> ret = 0;
> break;
> @@ -470,12 +665,22 @@ static void lenovo_remove_tpkbd(struct hid_device *hdev)
> hid_set_drvdata(hdev, NULL);
> }
>
> +static void lenovo_remove_cptkbd(struct hid_device *hdev)
> +{
> + sysfs_remove_group(&hdev->dev.kobj,
> + &lenovo_attr_group_cptkbd);
> +}
> +
> static void lenovo_remove(struct hid_device *hdev)
> {
> switch (hdev->product) {
> case USB_DEVICE_ID_LENOVO_TPKBD:
> lenovo_remove_tpkbd(hdev);
> break;
> + case USB_DEVICE_ID_LENOVO_CUSBKBD:
> + case USB_DEVICE_ID_LENOVO_CBTKBD:
> + lenovo_remove_cptkbd(hdev);
> + break;
> }
>
> hid_hw_stop(hdev);
> @@ -483,6 +688,8 @@ static void lenovo_remove(struct hid_device *hdev)
>
> static const struct hid_device_id lenovo_devices[] = {
> { HID_USB_DEVICE(USB_VENDOR_ID_LENOVO, USB_DEVICE_ID_LENOVO_TPKBD) },
> + { HID_USB_DEVICE(USB_VENDOR_ID_LENOVO, USB_DEVICE_ID_LENOVO_CUSBKBD) },
> + { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LENOVO, USB_DEVICE_ID_LENOVO_CBTKBD) },
> { }
> };
>
> @@ -494,6 +701,7 @@ static struct hid_driver lenovo_driver = {
> .input_mapping = lenovo_input_mapping,
> .probe = lenovo_probe,
> .remove = lenovo_remove,
> + .raw_event = lenovo_raw_event,
> };
> module_hid_driver(lenovo_driver);
>
> diff --git a/include/linux/hid.h b/include/linux/hid.h
> index 77632cf..fca74f1 100644
> --- a/include/linux/hid.h
> +++ b/include/linux/hid.h
> @@ -167,6 +167,7 @@ struct hid_item {
> #define HID_UP_MSVENDOR 0xff000000
> #define HID_UP_CUSTOM 0x00ff0000
> #define HID_UP_LOGIVENDOR 0xffbc0000
> +#define HID_UP_LNVENDOR 0xffa00000
> #define HID_UP_SENSOR 0x00200000
>
> #define HID_USAGE 0x0000ffff
> --
> 2.0.0
>
--
Antonio Ospite
http://ao2.it
A: Because it messes up the order in which people normally read text.
See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?
^ permalink raw reply
* Re: [PATCH v5 0/4] HID: lenovo: Add support for Lenovo Compact Keyboard
From: Antonio Ospite @ 2014-07-25 14:05 UTC (permalink / raw)
To: Jamie Lentin; +Cc: Jiri Kosina, Hans de Goede, linux-input, linux-kernel
In-Reply-To: <1406154648-14000-1-git-send-email-jm@lentin.co.uk>
On Wed, 23 Jul 2014 23:30:44 +0100
Jamie Lentin <jm@lentin.co.uk> wrote:
> This patchset follows on from my previous attempts to add support for
> these keyboards from Lenovo.
>
> Antonio, I've used cptkbd_data and left data_pointer for now. It'd be
> an easy patch to change data_pointer to tpkbd_data, but is drifting
> away from what this patchset is supposed to do. Could send that
> afterwards though.
>
Sure, no problem. I think we are good to go.
BTW I was considering picking up one of such compact keyboards myself,
but I read that the BT one uses the USB cable only for charging
and it does not show as a USB HID device when plugged in, can you
confirm that?
Thanks,
Antonio
--
Antonio Ospite
http://ao2.it
A: Because it messes up the order in which people normally read text.
See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?
^ permalink raw reply
* Re: [PATCH 00/15] atmel_mxt_ts - device tree, bootloader, etc
From: Nick Dyer @ 2014-07-25 14:10 UTC (permalink / raw)
To: Stephen Warren
Cc: Dmitry Torokhov, Yufeng Shen, Daniel Kurtz, Henrik Rydberg,
Joonyoung Shim, Alan Bowens, linux-input, linux-kernel,
Peter Meerwald, Benson Leung, Olof Johansson, Sekhar Nori
In-Reply-To: <53D17845.4020507@wwwdotorg.org>
On 24/07/14 22:19, Stephen Warren wrote:
>> mxt-app [device] --save fail.xcfg
>
> That command always experiences a timeout error, but still seems to dump
> something out. Is this timeout error an issue? I'm a little hesitant to
> modify the COMMSCONFIG settings until I know load/save are really working.
>
> root@localhost:~/obp-utils# ./mxt-app -d i2c-dev:1-004b --save
> ~/mxt-save-no-movement.xml
> Version:1.16-65-g0a4c
> Registered i2c-dev adapter:1 address:0x4b
> Opening config file /root/mxt-save-no-movement.xml...
> REPORTALL command issued
> Timeout
The timeout is benign. In the mode that we're using it here, it is unable
to query the chip to query the configuration checksum, hence in the file it
says "CHECKSUM=0x000000". This won't affect --load/--save.
> I've uploaded 2 logs to:
>
> http://avon.wwwdotorg.org/downloads/mxt-logs/
> (note there's no directory indexing, so manually add the filenames below to
> the URL)
>
> mxt-save-no-movement.xml
>
> This is with the whole series applied. Neither mouse movement nor clicks
> works. I tried mxt-app --reset and it made no difference to the dump results.
>
> mxt-save-move-ok-no-clicking.xml
>
> This is with "Input: atmel_mxt_ts - use deep sleep mode when stopped"
> reverted; mouse movement works, but clicking doesn't.
Great, this has identified the issue with mouse movement (touch).
The config programmed into the NVRAM on your touch controller has the first
byte of the T9 touchscreen object set to zero. This is the CTRL byte which
enables/disables the touch object and what it reports. It is relying on
this to enable the touchscreen on resume:
https://github.com/dtor/input/blob/9d8dc3e529/drivers/input/touchscreen/atmel_mxt_ts.c#L2005-L2006
My "use deep sleep mode when stopped" patch stops the driver writing to the
T9.CTRL byte, so whatever config you have in NVRAM for that byte will be
used (ie zero, disabled). Going forward, deep sleep is more generic.
Indeed, newer chips do not have T9 at all, or they might be using other
touch objects. The deep sleep mode is a lower power state to be in, and is
what Atmel recommends.
However, it does mean changing the maxtouch cfg - you can write the 0x83 to
the first byte of T9 and save it to NVRAM, by doing:
mxt-app [device] -W -T9 83
mxt-app [device] --backup
It should still work fine with the older driver, it will just be enabled
for an additional short time during bootup before the first call to
mxt_input_open().
About the clicking - what does getevent -lp show? It should show the
BTN_LEFT key. If that is working correctly, then the driver isn't parsing
the messages correctly, it would be useful if you could add a
mxt_dump_message() call to mxt_input_button() and capture some dmesg output
of pressing the button.
Thanks for your patience in debugging this.
^ permalink raw reply
* Re: [PATCH v2 00/10] Input - wacom: conversion to HID driver, series 2
From: Benjamin Tissoires @ 2014-07-25 14:54 UTC (permalink / raw)
To: Przemo Firszt
Cc: Dmitry Torokhov, Jiri Kosina, Ping Cheng, Jason Gerecke,
linux-kernel, linux-input
In-Reply-To: <1406295035.2664.43.camel@fedora-lan>
On Jul 25 2014 or thereabouts, Przemo Firszt wrote:
> Dnia 2014-07-25, pią o godzinie 08:58 -0400, Benjamin Tissoires pisze:
> > Hi Przemo,
> Hi Benjamin,
> > On Jul 25 2014 or thereabouts, Przemo Firszt wrote:
> > > Dnia 2014-07-24, czw o godzinie 14:13 -0400, Benjamin Tissoires pisze:
> > > [..]
> > > > Hi Przemo,
> > > Hi Benjamin,
> > > > Normally, this series contains all the bits of hid-wacom (except the custom
> > > > LED/OLED API). Please tell me if I am missing anything and if you like the
> > > > change.
> > >
> > > I can't cleanly apply your set yet to test it, so:
> >
> > Hmm, you need to apply the first series I sent on July 15th, on top of
> > Dmitry's next branch.
> >
> > http://www.spinics.net/lists/linux-input/msg32385.html
> >
> OK, thnaks!
> > > Acked-by: Przemo Firszt <przemo@firszt.eu>
> >
> > Thanks!
> >
> > >
> > > What's your plan about LED/OLED API?
> >
> > I thought I would only preserve the current, wider used, LED/OLED API and
> > just drop the one in hid-wacom. This way, g-s-d will access both
> > bluetooth and USB the same way.
> >
> > My decision was mostly guided because the support of BT in g-s-d was
> > only added recently (3.12 and backported to gnome 3.10 IIRC). And as
> > soon as these patches hit Dmitry's tree, I'll send the g-s-d patches to
> > fix all that.
>
> If I understand you correctly we cannot have the same entry point in
> sysfs for OLEDs unless we can tell userspace somehow if the tablet is
> conected over USB or over bluetooth. The hardware of Intuos4 Wireless
> over bluetooth allows only 1-bit images. The same hardware over USB
> allows 4-bit images. Formatting of the images is also completely
Holy crap! I missed that. I did not noticed the 1-bit vs 4-bits
difference :(
> different and it's not "plain". Check [1] for usb and exisitng
> hid-wacom.c/wacom_scramble function for bluetooth.
Maybe I overlooked it, but I thought that in case of USB, the scrambling
is done in user space, and in case of BT, the same scrambling made in the
kernel. They looks very similar so I thought the user-space scramble for
USB would have fit. However, the 4-bits/1-bits kills that assumption.
>
> If we want to make it consistent single entry on kernel level we
> probably have to implement image conversion in the kernel. The user land
> would always use 4-bit, plain formatted images and kernel driver would
> convert them to 4-bit, wacom usb format or 1-bit wacom bluetooth format
> depending on how the tablet is connected.
>
> The downside of this approach is that the user land wouldn't have 100%
> control over 1-bit images for bluetooth as kernel would have to create
> them from 4-bit images.
The USB interface is *very* simple:
- if incoming data != 1024 -> reject
- forward everything to the device without in kernel treatment*
How about just changing the expected size to 256 in case of a BT tablet,
and let the user-space scramble in both cases and forward proper raw
data?
This way, user space will still have control over 1-bit vs 4-bits, and
the changes required in the kernel will be small enough.
My concern is that I'd like to have this series in 3.17 and I will not
have access to the hardware until next week :/
Having all of the user-spaces breakages in the same kernel release will
I think simplify the user space work.
Cheers,
Benjamin
* speaking about that: I just noticed that hid-wacom sent the
WAC_CMD_ICON_START_STOP message twice with 0 as the second parameter.
However, the USB spec tells that you have to use 1 for 'start' and 0
for 'stop'. Rather weird that this is working with both 0 in BT mode.
>
> [1] https://lkml.org/lkml/2012/9/9/80
> --
> Kind regards,
> Przemo Firszt
>
>
--
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
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