* Re: [PATCH] Input: twl4030 - convert to using managed resources
From: 'Sebastian Reichel' @ 2014-01-06 2:33 UTC (permalink / raw)
To: Jingoo Han
Cc: 'Dmitry Torokhov', linux-input, 'Fugang Duan',
'Peter Ujfalusi', ".linux-kernel"
In-Reply-To: <007901cf0a86$a0639550$e12abff0$%han@samsung.com>
[-- Attachment #1: Type: text/plain, Size: 476 bytes --]
On Mon, Jan 06, 2014 at 11:25:54AM +0900, Jingoo Han wrote:
> IMHO, input_unregister_device() seems to be necessary,
> because input_register_device() is still used.
> If I am wrong, please let me know kindly.
>
> Sebastian Reichel,
> Would you repeat insmod & rmmod 'twl4030_keypad.ko'?
mh. I only tested with twl4030-keypad compiled into the kernel.
I will try building the driver as module and reloading it on
the device when I find some time.
-- Sebastian
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH] Input: twl4030 - convert to using managed resources
From: Jingoo Han @ 2014-01-06 2:25 UTC (permalink / raw)
To: 'Dmitry Torokhov', 'Sebastian Reichel'
Cc: linux-input, 'Fugang Duan', 'Peter Ujfalusi',
".linux-kernel", 'Jingoo Han'
In-Reply-To: <20140104090147.GA16680@core.coreip.homeip.net>
On Saturday, January 04, 2014 6:02 PM, Dmitry Torokhov wrote:
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
>
> Compile-tested only.
>
>
> drivers/input/keyboard/twl4030_keypad.c | 70 +++++++++++----------------------
> 1 file changed, 22 insertions(+), 48 deletions(-)
>
> diff --git a/drivers/input/keyboard/twl4030_keypad.c b/drivers/input/keyboard/twl4030_keypad.c
> index f663c19..71eb041 100644
> --- a/drivers/input/keyboard/twl4030_keypad.c
> +++ b/drivers/input/keyboard/twl4030_keypad.c
[.....]
> -static int twl4030_kp_remove(struct platform_device *pdev)
> -{
> - struct twl4030_keypad *kp = platform_get_drvdata(pdev);
> -
> - free_irq(kp->irq, kp);
> - input_unregister_device(kp->input);
Hi Dmitry Torokhov,
IMHO, input_unregister_device() seems to be necessary,
because input_register_device() is still used.
If I am wrong, please let me know kindly.
Sebastian Reichel,
Would you repeat insmod & rmmod 'twl4030_keypad.ko'?
Happy New Year! :-)
Best regards,
Jingoo Han
^ permalink raw reply
* Re: Another ALPS touchpad
From: Dmitry Torokhov @ 2014-01-05 23:47 UTC (permalink / raw)
To: Tommy Will; +Cc: Dylan Paul Thurston, Yunkang Tang, linux-input
In-Reply-To: <CA+F6ckMzRnP2E--nimZA+-cfR_wmbsN4hPvr4AONt4J_c_X4kw@mail.gmail.com>
On Sun, Jan 05, 2014 at 10:53:52AM +0800, Tommy Will wrote:
> 2014/1/4 Dylan Paul Thurston <dpthurst@indiana.edu>:
> >
> > I didn't quite follow this or the following discussion. One thing I
> > wanted to look into was whether it was possible to have a middle mouse
> > click in addition to the left and right clicks.
>
> Normally, clickpad only supports Left/Right click. I'm afraid middle
> click would not be supported as standard.
On devices that lack middle button middle click is usually emulated by
pressing both left and right buttons. Also, since this is multi-touch
device quite often middle click is mapped on 2- or 3-finger tap. All of
that is done in X driver and not in kernel.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCHv4 0/2] twl4030-keypad DT binding
From: Sebastian Reichel @ 2014-01-05 3:50 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: linux-input, Rob Herring, Pawel Moll, Mark Rutland,
Stephen Warren, Ian Campbell, Rob Landley, Grant Likely,
devicetree, linux-doc, linux-kernel
In-Reply-To: <20140104084901.GA1991@core.coreip.homeip.net>
[-- Attachment #1: Type: text/plain, Size: 255 bytes --]
On Sat, Jan 04, 2014 at 12:49:01AM -0800, Dmitry Torokhov wrote:
> On Sat, Jan 04, 2014 at 06:40:06AM +0100, Sebastian Reichel wrote:
> > What's the status of this patch? Can you queue this for 3.14?
>
> I just queued it.
Thanks.
-- Sebastian
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH] Input: twl4030 - convert to using managed resources
From: Sebastian Reichel @ 2014-01-05 3:47 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: linux-input, Jingoo Han, Fugang Duan, Peter Ujfalusi,
".linux-kernel"
In-Reply-To: <20140104090147.GA16680@core.coreip.homeip.net>
[-- Attachment #1: Type: text/plain, Size: 520 bytes --]
On Sat, Jan 04, 2014 at 01:01:48AM -0800, Dmitry Torokhov wrote:
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
>
> Compile-tested only.
>
> drivers/input/keyboard/twl4030_keypad.c | 70 +++++++++++----------------------
> 1 file changed, 22 insertions(+), 48 deletions(-)
I have successfully booted dtor/input.git:master with this patch on
top and the addition of twl4030-keypad DT nodes in omap3-n900.dts on
my N900.
Tested-by: Sebastian Reichel <sre@debian.org>
-- Sebastian
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: Another ALPS touchpad
From: Tommy Will @ 2014-01-05 2:53 UTC (permalink / raw)
To: Dylan Paul Thurston; +Cc: Dmitry Torokhov, Yunkang Tang, linux-input
In-Reply-To: <20140104133108.GA7296@whitehail.bostoncoop.net>
2014/1/4 Dylan Paul Thurston <dpthurst@indiana.edu>:
>
> I didn't quite follow this or the following discussion. One thing I
> wanted to look into was whether it was possible to have a middle mouse
> click in addition to the left and right clicks.
Normally, clickpad only supports Left/Right click. I'm afraid middle
click would not be supported as standard.
However, if you have interest in middle click, based on our patch, you
can support that by yourself.
# Since it's a clickpad with only one button, you can report any
button event as you like by detecting the finger area when clicking.
> I had reported spurious clicks with the generic driver; I'm quite sure I'm not
> actually touching the pad when this happens, in case that's relevant.
I'm not sure where is wrong with generic driver. We would check the
behavior with latest kernel later.
--
Best Regards,
Tommy
^ permalink raw reply
* Re: [PATCH 1/2] HID: sony: Add force feedback for the Dualshock 4
From: simon @ 2014-01-05 1:10 UTC (permalink / raw)
To: Frank Praznik; +Cc: linux-input
In-Reply-To: <alpine.DEB.2.10.1401031518010.29517@franz-virtual-machine>
> This patch adds force feedback support for Sony's Dualshock 4 controller.
> It does this by adding a device ID for the new controller and creating a
> new dualshock4_worker function to send data commands formatted for the
> controller. Unlike the Sixaxis, the Dualshock 4 requires a magnitude
> value for the small motor so the actual value is now sent to the worker
> function and the sixaxis worker was modified to clamp it to 1 or 0 as
> required.
I was able to build this and the LED patch against the 3.13rc6 kernel
having first applied this sequence of patches:
https://patchwork.kernel.org/patch/3203761/
However I am seeing some weird behaviour in whether FF/LED actually
functions. It seems that in a 'complete' kernel installation the driver
does not present FF or LED.
But if I use a stock 3.13rc6 kernel, load the 'hid-sony' module from that
and then remove and install the patched one it does work.
--
$ sudo modprobe hid-sony (stock)
$ sudo rmmod hid-sony
$ sudo insmod hid-sony.ko (patched)
--
Not sure what this means, lsmod looks to be the same in both. Perhaps
there is something which is only initialised in the stock version.
Willing to test if anyone has suggestions/comments. I am testing with the
DS4 connected with USB (not BT).
--
Jan 4 17:55:14 simon-virtual-machine kernel: [ 138.964626] usb 1-1: new
full-speed USB device number 3 using ohci-pci
Jan 4 17:55:14 simon-virtual-machine mtp-probe: checking bus 1, device 3:
"/sys/devices/pci0000:00/0000:00:06.0/usb1/1-1"
Jan 4 17:55:14 simon-virtual-machine mtp-probe: bus: 1, device: 3 was not
an MTP device
Jan 4 17:55:14 simon-virtual-machine kernel: [ 139.416656] usb 1-1: New
USB device found, idVendor=054c, idProduct=05c4
Jan 4 17:55:14 simon-virtual-machine kernel: [ 139.416661] usb 1-1: New
USB device strings: Mfr=1, Product=2, SerialNumber=0
Jan 4 17:55:14 simon-virtual-machine kernel: [ 139.416663] usb 1-1:
Product: Wireless Controller
Jan 4 17:55:14 simon-virtual-machine kernel: [ 139.416665] usb 1-1:
Manufacturer: Sony Computer Entertainment
Jan 4 17:55:14 simon-virtual-machine kernel: [ 139.517356] input: Sony
Computer Entertainment Wireless Controller as
/devices/pci0000:00/0000:00:06.0/usb1/1-1/1-1:1.0/input/input6
Jan 4 17:55:14 simon-virtual-machine kernel: [ 139.518359] hid-generic
0003:054C:05C4.0002: input,hidraw0: USB HID v1.11 Gamepad [Sony Computer
Entertainment Wireless Controller] on usb-0000:00:06.0-1/input0
--
simon@simon-virtual-machine:~$ ls /sys/class/leds/
simon@simon-virtual-machine:~$
--
Cheers,
Simon
^ permalink raw reply
* Re: [PATCH] Input: ims-pcu - remove unneeded get_unaligned_xxx
From: Dmitry Torokhov @ 2014-01-04 22:32 UTC (permalink / raw)
To: Andrey Smirnov; +Cc: linux-input, linux-kernel
In-Reply-To: <CAHQ1cqEmpejiMPij+jQM0YEceShyHPRJ2dPkuaLkuBLYLXQyJg@mail.gmail.com>
On Sat, Jan 04, 2014 at 09:41:43AM -0800, Andrey Smirnov wrote:
>
> Anyway, TL;DR of the whole thing is:
> - Your arguments do not convince me in the slightest
> - I don't think that mine do you
> - Let's stop wasting out time
> - Do as you see fit, since you are the maintainer
>
Nah, I think I'll drop the patch for now, that macro is a bit ugly and
as you said the code is not in a hot path. I'll reserve the right to
revisit it based on how accelerometer code looks.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] Input: ims-pcu - remove unneeded get_unaligned_xxx
From: Andrey Smirnov @ 2014-01-04 17:41 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, linux-kernel
In-Reply-To: <20140104074621.GD20272@core.coreip.homeip.net>
On Fri, Jan 3, 2014 at 11:46 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Fri, Jan 03, 2014 at 10:49:07PM -0800, Andrey Smirnov wrote:
>> On Fri, Jan 3, 2014 at 10:16 PM, Dmitry Torokhov
>> <dmitry.torokhov@gmail.com> wrote:
>> > On Fri, Jan 03, 2014 at 09:52:25PM -0800, Andrey Smirnov wrote:
>> >> On Fri, Jan 3, 2014 at 9:28 PM, Dmitry Torokhov
>> >> <dmitry.torokhov@gmail.com> wrote:
>> >> > pcu->cmd_buf[IMS_PCU_DATA_OFFSET] is word aligned so we do not need to use
>> >> > get_unaligned_le16 to access it.
>> >> >
>> >> > Also let's add build time check to make sure it stays aligned.
>> >>
>> >> - AFAIK, there's no guarantee the "pcu" itself is aligned
>> >
>> > Yes. kmalloc returns aligned pointer, otherwise every access to members
>> > other than u8 would risk unaligned exception.
>>
>> OK, fair point.
>>
>> >
>> >> - This change assumes that aligning data on the 2-byte boundary is
>> >> sufficient for all architectures that do not allow unaligned data
>> >> access, which I don't think is a good assumption to make
>> >
>> > What arches require word access be double-word aligned?
>>
>> I don't know and neither do I care. Even if there aren't any in Linux
>> today do you expect it to never add a support to one that would impose
>> such a restriction?
>
> There is a lot of assumptions in kernel that might get broken by a
> random arch.
I fact that there are such assumptions in kernel doesn't mean we
should add more. IMHO, especially input drivers should be as CPU
architecture agnostic as possible.
> For example, the assumption that pointer and long require
> the same storage.
That is an assumption about compiler implementation, since the size of
the basic types is implementation-specific.
>
> Anyway, it looks like gcc has __alignof__(type) construct that will make
> sure we ensure the right alignment for type.
>
>>
>> The whole point of get_unaligned_* "framework" is to provide drivers
>> with unified interface that would allow you not to care about
>> alignment.
>
> No, it is not that you do not care about alignment, it is so that you
> can safely access data that you know to be unaligned.
Or the data the alignment of which you cannot guarantee, which was
exactly my point.
> Otherwise code would be littered with get_uanligned_*.
>
>> "Framework" in which architectures that support unaligned
>> access can fallback to good old functions like *_to_cpup and
>> architectures that do would provide the code to handle access in
>> whatever manner is best suited for them.
>>
>> >
>> >> - On x86 or any other architecture that allows unaligned access
>> >> get_unaligned_le16() is actually results to call to le16_to_cpup(), so
>> >> this change doesn't really save anything while imposing restrictions
>> >> on the arrangement of the fields in struct ims_pcu and causing
>> >> unnecessary build errors.
>> >
>> > Unless somebody changes the layout there won't be any new build errors,
>> > will there?
>>
>> So do you expect that code to never change from now on? I most likely
>> will be working on changes to support accelerometer data aggregation
>> and implementing associated with it input devices and this may or may
>> not require me to add fields to that structure, so what am I supposed
>> to do in that case? Juggle fields around until I find the right
>> combination that does not trigger build error?.
>>
>
> Umm, yes? And your juggling will be reduced to nothing if you add new
> fields at the end of the structure.
Oh, OK. I did not not expect you to be OK with such pointless
arbitrary limitations placed in order to save a handful of cycles in a
function that is not very often called, but if you are I don't believe
we would arrive anywhere with this argument. Let's at least add an
appropriate comment to the structure warning the potential user that
the way it is organized is important, since it may affect layout and
break the build.
>
>> I honestly don't understand the purpose of this change, it doesn't
>> really save any performance,
>
> Technically it does as it does not require going through unaligned
> access on arches that can't do it.
OK, if we are going to stick to technicalities, then technically until
we actually compiled both versions of the code on the platforms in
question and did the actual performance measurements nothing can be
said about performance improvement and we can only speculate about
potential performance improvements.
>
>> IMHO decreases potability of the driver,
>
> I think that your concern can be solved with alignof.
OK, as I said before I don't think you and I will come to an agreement
on this one, since I see this change as a premature micro-optimization
that makes the code less abstract since now it has knowledge of CPU
alignment(which IMHO should be contained, as much as possible, in
architecture specific code) and you don't think that those are
actually problems of the patch. But since you the maintainer of the
subsystem and the final call about the inclusion of the patch is
yours, then you should do whatever you see fit(Or as another option
you can as ask other subsystems maintainers to weigh in on the
matter).
Anyway, TL;DR of the whole thing is:
- Your arguments do not convince me in the slightest
- I don't think that mine do you
- Let's stop wasting out time
- Do as you see fit, since you are the maintainer
Cheers,
Andrey
>
> Thanks.
>
> --
> Dmitry
^ permalink raw reply
* Re: [lm-sensors] [PATCH v3 2/5] input: sun4i-ts: Add support for temperature sensor
From: Guenter Roeck @ 2014-01-04 17:35 UTC (permalink / raw)
To: Hans de Goede, Dmitry Torokhov
Cc: LM Sensors, linux-sunxi, Corentin LABBE, linux-input,
Maxime Ripard, linux-arm-kernel
In-Reply-To: <1388506852-3548-3-git-send-email-hdegoede@redhat.com>
On 12/31/2013 08:20 AM, Hans de Goede wrote:
> The sun4i resisitive touchscreen controller also comes with a built-in
> temperature sensor. This commit adds support for it.
>
> This commit also introduces a new "ts-attached" device-tree property,
> when this is not set, the input part of the driver won't register. This way
> the internal temperature sensor can be used to measure the SoC temperature
> independent of there actually being a touchscreen attached to the controller.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Couple of minor comments below, though no need to resubmit unless someone else has comments.
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> ---
> .../bindings/input/touchscreen/sun4i.txt | 5 +
> drivers/input/touchscreen/sun4i-ts.c | 140 ++++++++++++++++-----
> 2 files changed, 114 insertions(+), 31 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/sun4i.txt b/Documentation/devicetree/bindings/input/touchscreen/sun4i.txt
> index e45927e..6bac67b 100644
> --- a/Documentation/devicetree/bindings/input/touchscreen/sun4i.txt
> +++ b/Documentation/devicetree/bindings/input/touchscreen/sun4i.txt
> @@ -6,10 +6,15 @@ Required properties:
> - reg: mmio address range of the chip
> - interrupts: interrupt to which the chip is connected
>
> +Optional properties:
> + - allwinner,ts-attached: boolean indicating that an actual touchscreen is
> + attached to the controller
> +
Brr. While I understand that you were asked to do this, I don't really see the benefit
of another "allwinner" here. As if this wasn't implied by the "compatible" property.
> Example:
>
> rtp: rtp@01c25000 {
> compatible = "allwinner,sun4i-ts";
> reg = <0x01c25000 0x100>;
> interrupts = <29>;
> + allwinner,ts-attached;
> };
> diff --git a/drivers/input/touchscreen/sun4i-ts.c b/drivers/input/touchscreen/sun4i-ts.c
> index 5945219..16cbb01 100644
> --- a/drivers/input/touchscreen/sun4i-ts.c
> +++ b/drivers/input/touchscreen/sun4i-ts.c
> @@ -3,6 +3,9 @@
> *
> * Copyright (C) 2013 - 2014 Hans de Goede <hdegoede@redhat.com>
> *
> + * The hwmon parts are based on work by Corentin LABBE which is:
> + * Copyright (C) 2013 Corentin LABBE <clabbe.montjoie@gmail.com>
> + *
> * This program is free software; you can redistribute it and/or modify
> * it under the terms of the GNU General Public License as published by
> * the Free Software Foundation; either version 2 of the License, or
> @@ -30,6 +33,7 @@
> */
>
> #include <linux/err.h>
> +#include <linux/hwmon.h>
> #include <linux/init.h>
> #include <linux/input.h>
> #include <linux/interrupt.h>
> @@ -106,14 +110,12 @@ struct sun4i_ts_data {
> void __iomem *base;
> unsigned int irq;
> bool ignore_fifo_data;
> + int temp_data;
> };
>
> -static irqreturn_t sun4i_ts_irq(int irq, void *dev_id)
> +static void sun4i_ts_irq_handle_input(struct sun4i_ts_data *ts, u32 reg_val)
> {
> - struct sun4i_ts_data *ts = dev_id;
> - u32 reg_val, x, y;
> -
> - reg_val = readl(ts->base + TP_INT_FIFOS);
> + u32 x, y;
>
> if (reg_val & FIFO_DATA_PENDING) {
> x = readl(ts->base + TP_DATA);
> @@ -139,6 +141,20 @@ static irqreturn_t sun4i_ts_irq(int irq, void *dev_id)
> input_report_key(ts->input, BTN_TOUCH, 0);
> input_sync(ts->input);
> }
> +}
> +
> +static irqreturn_t sun4i_ts_irq(int irq, void *dev_id)
> +{
> + struct sun4i_ts_data *ts = dev_id;
> + u32 reg_val;
> +
> + reg_val = readl(ts->base + TP_INT_FIFOS);
> +
> + if (reg_val & TEMP_DATA_PENDING)
> + ts->temp_data = readl(ts->base + TEMP_DATA);
> +
> + if (ts->input)
> + sun4i_ts_irq_handle_input(ts, reg_val);
>
> writel(reg_val, ts->base + TP_INT_FIFOS);
>
> @@ -149,9 +165,9 @@ static int sun4i_ts_open(struct input_dev *dev)
> {
> struct sun4i_ts_data *ts = input_get_drvdata(dev);
>
> - /* Flush, set trig level to 1, enable data and up irqs */
> - writel(DATA_IRQ_EN(1) | FIFO_TRIG(1) | FIFO_FLUSH(1) | TP_UP_IRQ_EN(1),
> - ts->base + TP_INT_FIFOC);
> + /* Flush, set trig level to 1, enable temp, data and up irqs */
> + writel(TEMP_IRQ_EN(1) | DATA_IRQ_EN(1) | FIFO_TRIG(1) | FIFO_FLUSH(1) |
> + TP_UP_IRQ_EN(1), ts->base + TP_INT_FIFOC);
>
> return 0;
> }
> @@ -160,15 +176,48 @@ static void sun4i_ts_close(struct input_dev *dev)
> {
> struct sun4i_ts_data *ts = input_get_drvdata(dev);
>
> - /* Deactivate all IRQs */
> - writel(0, ts->base + TP_INT_FIFOC);
> + /* Deactivate all input IRQs */
> + writel(TEMP_IRQ_EN(1), ts->base + TP_INT_FIFOC);
> +}
> +
> +static ssize_t show_temp(struct device *dev, struct device_attribute *devattr,
> + char *buf)
> +{
> + struct sun4i_ts_data *ts = dev_get_drvdata(dev);
> +
> + /* No temp_data until the first irq */
> + if (ts->temp_data == -1)
> + return -EAGAIN;
> +
> + return sprintf(buf, "%d\n", (ts->temp_data - 1447) * 100);
> +}
> +
> +static ssize_t show_temp_label(struct device *dev,
> + struct device_attribute *devattr, char *buf)
> +{
> + return sprintf(buf, "SoC temperature\n");
> }
>
> +static DEVICE_ATTR(temp1_input, S_IRUGO, show_temp, NULL);
> +static DEVICE_ATTR(temp1_label, S_IRUGO, show_temp_label, NULL);
> +
Just remembered ... there is now also a DEVICE_ATTR_RO() macro.
> +static struct attribute *sun4i_ts_attrs[] = {
> + &dev_attr_temp1_input.attr,
> + &dev_attr_temp1_label.attr,
> + NULL
> +};
> +ATTRIBUTE_GROUPS(sun4i_ts);
> +
> static int sun4i_ts_probe(struct platform_device *pdev)
> {
> struct sun4i_ts_data *ts;
> struct device *dev = &pdev->dev;
> + struct device_node *np = dev->of_node;
> + struct device *hwmon;
> int ret;
> + bool ts_attached;
> +
> + ts_attached = of_property_read_bool(np, "allwinner,ts-attached");
>
> ts = devm_kzalloc(dev, sizeof(struct sun4i_ts_data), GFP_KERNEL);
> if (!ts)
> @@ -176,24 +225,27 @@ static int sun4i_ts_probe(struct platform_device *pdev)
>
> ts->dev = dev;
> ts->ignore_fifo_data = true;
> -
> - ts->input = devm_input_allocate_device(dev);
> - if (!ts->input)
> - return -ENOMEM;
> -
> - ts->input->name = pdev->name;
> - ts->input->phys = "sun4i_ts/input0";
> - ts->input->open = sun4i_ts_open;
> - ts->input->close = sun4i_ts_close;
> - ts->input->id.bustype = BUS_HOST;
> - ts->input->id.vendor = 0x0001;
> - ts->input->id.product = 0x0001;
> - ts->input->id.version = 0x0100;
> - ts->input->evbit[0] = BIT(EV_SYN) | BIT(EV_KEY) | BIT(EV_ABS);
> - set_bit(BTN_TOUCH, ts->input->keybit);
> - input_set_abs_params(ts->input, ABS_X, 0, 4095, 0, 0);
> - input_set_abs_params(ts->input, ABS_Y, 0, 4095, 0, 0);
> - input_set_drvdata(ts->input, ts);
> + ts->temp_data = -1;
> +
> + if (ts_attached) {
> + ts->input = devm_input_allocate_device(dev);
> + if (!ts->input)
> + return -ENOMEM;
> +
> + ts->input->name = pdev->name;
> + ts->input->phys = "sun4i_ts/input0";
> + ts->input->open = sun4i_ts_open;
> + ts->input->close = sun4i_ts_close;
> + ts->input->id.bustype = BUS_HOST;
> + ts->input->id.vendor = 0x0001;
> + ts->input->id.product = 0x0001;
> + ts->input->id.version = 0x0100;
> + ts->input->evbit[0] = BIT(EV_SYN) | BIT(EV_KEY) | BIT(EV_ABS);
> + set_bit(BTN_TOUCH, ts->input->keybit);
> + input_set_abs_params(ts->input, ABS_X, 0, 4095, 0, 0);
> + input_set_abs_params(ts->input, ABS_Y, 0, 4095, 0, 0);
> + input_set_drvdata(ts->input, ts);
> + }
>
> ts->base = devm_ioremap_resource(dev,
> platform_get_resource(pdev, IORESOURCE_MEM, 0));
> @@ -232,14 +284,39 @@ static int sun4i_ts_probe(struct platform_device *pdev)
> writel(STYLUS_UP_DEBOUN(5) | STYLUS_UP_DEBOUN_EN(1) | TP_MODE_EN(1),
> ts->base + TP_CTRL1);
>
> - ret = input_register_device(ts->input);
> - if (ret)
> - return ret;
> + hwmon = devm_hwmon_device_register_with_groups(ts->dev, "sun4i_ts",
> + ts, sun4i_ts_groups);
> + if (IS_ERR(hwmon))
> + return PTR_ERR(hwmon);
> +
> + writel(TEMP_IRQ_EN(1), ts->base + TP_INT_FIFOC);
> +
> + if (ts_attached) {
> + ret = input_register_device(ts->input);
> + if (ret) {
> + writel(0, ts->base + TP_INT_FIFOC);
> + return ret;
> + }
> + }
>
> platform_set_drvdata(pdev, ts);
> return 0;
> }
>
> +static int sun4i_ts_remove(struct platform_device *pdev)
> +{
> + struct sun4i_ts_data *ts = platform_get_drvdata(pdev);
> +
> + /* Explicit unregister to avoid open/close changing the imask later */
> + if (ts->input)
> + input_unregister_device(ts->input);
> +
> + /* Deactivate all IRQs */
> + writel(0, ts->base + TP_INT_FIFOC);
> +
> + return 0;
> +}
> +
> static const struct of_device_id sun4i_ts_of_match[] = {
> { .compatible = "allwinner,sun4i-ts", },
> { /* sentinel */ }
> @@ -253,6 +330,7 @@ static struct platform_driver sun4i_ts_driver = {
> .of_match_table = of_match_ptr(sun4i_ts_of_match),
> },
> .probe = sun4i_ts_probe,
> + .remove = sun4i_ts_remove,
> };
>
> module_platform_driver(sun4i_ts_driver);
>
^ permalink raw reply
* Re: Another ALPS touchpad
From: Dylan Paul Thurston @ 2014-01-04 13:31 UTC (permalink / raw)
To: Tommy Will; +Cc: Dmitry Torokhov, Yunkang Tang, linux-input
In-Reply-To: <CA+F6ckPRj0NrerBC7j41QOE6ZsV7tPQ35gZEfk8cFZws7sT1vg@mail.gmail.com>
On Sat, Jan 04, 2014 at 01:27:24PM +0800, Tommy Will wrote:
> Hi Dmitry,
>
> > Do you have anything in works to support this new model? Or it is
> > not an ALPS device at all?
>
> Yes, it's an ALPS device and we're now preparing the patch for this new one.
Glad to hear it's being worked on! I'm eager to test out anything.
> BTW, since it's a ClickPad without seperate buttons, simple resting
> finger logic would be added.
I didn't quite follow this or the following discussion. One thing I
wanted to look into was whether it was possible to have a middle mouse
click in addition to the left and right clicks. I had reported
spurious clicks with the generic driver; I'm quite sure I'm not
actually touching the pad when this happens, in case that's relevant.
--Dylan Thurston
^ permalink raw reply
* Re: [PATCH] Input: twl6040_vibra - remove unneeded check for CONFIG_OF
From: David Herrmann @ 2014-01-04 10:30 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: open list:HID CORE LAYER, Peter Ujfalusi, linux-kernel
In-Reply-To: <20140104081142.GA3987@core.coreip.homeip.net>
Hi
On Sat, Jan 4, 2014 at 9:11 AM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> Since the driver requires DT now we do not need to check if CONFIG_OF is
> defined.
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
> drivers/input/misc/twl6040-vibra.c | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/drivers/input/misc/twl6040-vibra.c b/drivers/input/misc/twl6040-vibra.c
> index 89bca76..77dc23b 100644
> --- a/drivers/input/misc/twl6040-vibra.c
> +++ b/drivers/input/misc/twl6040-vibra.c
> @@ -258,17 +258,14 @@ static SIMPLE_DEV_PM_OPS(twl6040_vibra_pm_ops, twl6040_vibra_suspend, NULL);
> static int twl6040_vibra_probe(struct platform_device *pdev)
> {
> struct device *twl6040_core_dev = pdev->dev.parent;
> - struct device_node *twl6040_core_node = NULL;
> + struct device_node *twl6040_core_node;
> struct vibra_info *info;
> int vddvibl_uV = 0;
> int vddvibr_uV = 0;
> int ret;
>
> -#ifdef CONFIG_OF
> twl6040_core_node = of_find_node_by_name(twl6040_core_dev->of_node,
> "vibra");
If CONFIG_OF=n, of_find_node_by_name() always returns NULL. See
include/linux/of.h. So no need to depend on DT.
Reviewed-by: David Herrmann <dh.herrmann@gmail.com>
Thanks
David
> -#endif
> -
> if (!twl6040_core_node) {
> dev_err(&pdev->dev, "parent of node is missing?\n");
> return -EINVAL;
> --
> 1.8.4.2
>
>
> --
> Dmitry
> --
> 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: [PATCHv4 0/2] twl4030-keypad DT binding
From: Dmitry Torokhov @ 2014-01-04 8:49 UTC (permalink / raw)
To: linux-input, Rob Herring, Pawel Moll, Mark Rutland,
Stephen Warren, Ian Campbell, Rob Landley, Grant Likely,
devicetree, linux-doc, linux-kernel
In-Reply-To: <20140104054006.GA23424@earth.universe>
Hi Sebastian,
On Sat, Jan 04, 2014 at 06:40:06AM +0100, Sebastian Reichel wrote:
> Dmitry,
>
> What's the status of this patch? Can you queue this for 3.14?
I just queued it.
Thanks.
--
Dmitry
^ permalink raw reply
* [PATCH] Input: twl6040_vibra - remove unneeded check for CONFIG_OF
From: Dmitry Torokhov @ 2014-01-04 8:11 UTC (permalink / raw)
To: linux-input; +Cc: Peter Ujfalusi, linux-kernel
Since the driver requires DT now we do not need to check if CONFIG_OF is
defined.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/misc/twl6040-vibra.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/input/misc/twl6040-vibra.c b/drivers/input/misc/twl6040-vibra.c
index 89bca76..77dc23b 100644
--- a/drivers/input/misc/twl6040-vibra.c
+++ b/drivers/input/misc/twl6040-vibra.c
@@ -258,17 +258,14 @@ static SIMPLE_DEV_PM_OPS(twl6040_vibra_pm_ops, twl6040_vibra_suspend, NULL);
static int twl6040_vibra_probe(struct platform_device *pdev)
{
struct device *twl6040_core_dev = pdev->dev.parent;
- struct device_node *twl6040_core_node = NULL;
+ struct device_node *twl6040_core_node;
struct vibra_info *info;
int vddvibl_uV = 0;
int vddvibr_uV = 0;
int ret;
-#ifdef CONFIG_OF
twl6040_core_node = of_find_node_by_name(twl6040_core_dev->of_node,
"vibra");
-#endif
-
if (!twl6040_core_node) {
dev_err(&pdev->dev, "parent of node is missing?\n");
return -EINVAL;
--
1.8.4.2
--
Dmitry
^ permalink raw reply related
* Re: [PATCH] Input: ims-pcu - remove unneeded get_unaligned_xxx
From: Dmitry Torokhov @ 2014-01-04 7:46 UTC (permalink / raw)
To: Andrey Smirnov; +Cc: linux-input, linux-kernel
In-Reply-To: <CAHQ1cqEQtXRaPWo6wYgpDrjwGhnAySvb7Jvs8KFRTTRb3g++GQ@mail.gmail.com>
On Fri, Jan 03, 2014 at 10:49:07PM -0800, Andrey Smirnov wrote:
> On Fri, Jan 3, 2014 at 10:16 PM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> > On Fri, Jan 03, 2014 at 09:52:25PM -0800, Andrey Smirnov wrote:
> >> On Fri, Jan 3, 2014 at 9:28 PM, Dmitry Torokhov
> >> <dmitry.torokhov@gmail.com> wrote:
> >> > pcu->cmd_buf[IMS_PCU_DATA_OFFSET] is word aligned so we do not need to use
> >> > get_unaligned_le16 to access it.
> >> >
> >> > Also let's add build time check to make sure it stays aligned.
> >>
> >> - AFAIK, there's no guarantee the "pcu" itself is aligned
> >
> > Yes. kmalloc returns aligned pointer, otherwise every access to members
> > other than u8 would risk unaligned exception.
>
> OK, fair point.
>
> >
> >> - This change assumes that aligning data on the 2-byte boundary is
> >> sufficient for all architectures that do not allow unaligned data
> >> access, which I don't think is a good assumption to make
> >
> > What arches require word access be double-word aligned?
>
> I don't know and neither do I care. Even if there aren't any in Linux
> today do you expect it to never add a support to one that would impose
> such a restriction?
There is a lot of assumptions in kernel that might get broken by a
random arch. For example, the assumption that pointer and long require
the same storage.
Anyway, it looks like gcc has __alignof__(type) construct that will make
sure we ensure the right alignment for type.
>
> The whole point of get_unaligned_* "framework" is to provide drivers
> with unified interface that would allow you not to care about
> alignment.
No, it is not that you do not care about alignment, it is so that you
can safely access data that you know to be unaligned. Otherwise code
would be littered with get_uanligned_*.
> "Framework" in which architectures that support unaligned
> access can fallback to good old functions like *_to_cpup and
> architectures that do would provide the code to handle access in
> whatever manner is best suited for them.
>
> >
> >> - On x86 or any other architecture that allows unaligned access
> >> get_unaligned_le16() is actually results to call to le16_to_cpup(), so
> >> this change doesn't really save anything while imposing restrictions
> >> on the arrangement of the fields in struct ims_pcu and causing
> >> unnecessary build errors.
> >
> > Unless somebody changes the layout there won't be any new build errors,
> > will there?
>
> So do you expect that code to never change from now on? I most likely
> will be working on changes to support accelerometer data aggregation
> and implementing associated with it input devices and this may or may
> not require me to add fields to that structure, so what am I supposed
> to do in that case? Juggle fields around until I find the right
> combination that does not trigger build error?.
>
Umm, yes? And your juggling will be reduced to nothing if you add new
fields at the end of the structure.
> I honestly don't understand the purpose of this change, it doesn't
> really save any performance,
Technically it does as it does not require going through unaligned
access on arches that can't do it.
> IMHO decreases potability of the driver,
I think that your concern can be solved with alignof.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] Input: ims-pcu - remove unneeded get_unaligned_xxx
From: Andrey Smirnov @ 2014-01-04 6:49 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, linux-kernel
In-Reply-To: <20140104061650.GB20272@core.coreip.homeip.net>
On Fri, Jan 3, 2014 at 10:16 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Fri, Jan 03, 2014 at 09:52:25PM -0800, Andrey Smirnov wrote:
>> On Fri, Jan 3, 2014 at 9:28 PM, Dmitry Torokhov
>> <dmitry.torokhov@gmail.com> wrote:
>> > pcu->cmd_buf[IMS_PCU_DATA_OFFSET] is word aligned so we do not need to use
>> > get_unaligned_le16 to access it.
>> >
>> > Also let's add build time check to make sure it stays aligned.
>>
>> - AFAIK, there's no guarantee the "pcu" itself is aligned
>
> Yes. kmalloc returns aligned pointer, otherwise every access to members
> other than u8 would risk unaligned exception.
OK, fair point.
>
>> - This change assumes that aligning data on the 2-byte boundary is
>> sufficient for all architectures that do not allow unaligned data
>> access, which I don't think is a good assumption to make
>
> What arches require word access be double-word aligned?
I don't know and neither do I care. Even if there aren't any in Linux
today do you expect it to never add a support to one that would impose
such a restriction?
The whole point of get_unaligned_* "framework" is to provide drivers
with unified interface that would allow you not to care about
alignment. "Framework" in which architectures that support unaligned
access can fallback to good old functions like *_to_cpup and
architectures that do would provide the code to handle access in
whatever manner is best suited for them.
>
>> - On x86 or any other architecture that allows unaligned access
>> get_unaligned_le16() is actually results to call to le16_to_cpup(), so
>> this change doesn't really save anything while imposing restrictions
>> on the arrangement of the fields in struct ims_pcu and causing
>> unnecessary build errors.
>
> Unless somebody changes the layout there won't be any new build errors,
> will there?
So do you expect that code to never change from now on? I most likely
will be working on changes to support accelerometer data aggregation
and implementing associated with it input devices and this may or may
not require me to add fields to that structure, so what am I supposed
to do in that case? Juggle fields around until I find the right
combination that does not trigger build error?.
I honestly don't understand the purpose of this change, it doesn't
really save any performance, IMHO decreases potability of the driver,
so what is the gain. What does adding all the restrictions that this
change imposes buy us?
>
> Thanks.
>
> --
> Dmitry
^ permalink raw reply
* Re: Another ALPS touchpad
From: Tommy Will @ 2014-01-04 6:32 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Yunkang Tang, Dylan Paul Thurston, linux-input
In-Reply-To: <20140104061815.GC20272@core.coreip.homeip.net>
2014/1/4 Dmitry Torokhov <dmitry.torokhov@gmail.com>:
> On Sat, Jan 04, 2014 at 02:08:26PM +0800, Tommy Will wrote:
>> Does X-server have already supported this feature? (I'm sorry I did
>> not try the latest X-Server)
>
> It should as far as I know (I do not have any clickpad devices).
>
Got that, thanks! We would check that with latest kernel.
--
Best Regards,
Tommy
^ permalink raw reply
* Re: Another ALPS touchpad
From: Dmitry Torokhov @ 2014-01-04 6:18 UTC (permalink / raw)
To: Tommy Will; +Cc: Yunkang Tang, Dylan Paul Thurston, linux-input
In-Reply-To: <CA+F6ckNPxxmGHqHuPoRNMzgyS=kNYc5NZhqOYuMsaReqaimffQ@mail.gmail.com>
On Sat, Jan 04, 2014 at 02:08:26PM +0800, Tommy Will wrote:
> 2014/1/4 Dmitry Torokhov <dmitry.torokhov@gmail.com>:
> > On Thursday, January 02, 2014 11:59:15 PM Dylan Paul Thurston wrote:
>
> >> Yes, it's an ALPS device and we're now preparing the patch for this new one.
> >> BTW, since it's a ClickPad without seperate buttons, simple resting
> >> finger logic would be added.
> >
> > Hmm, what finger resting logic? We already properly support clickpads in
> > X driver (as long as the kernel tells userspace that the device is a
> > clickpad) so what is missing?
> >
>
> I remember in previous kernel version, I tried to register the device
> as clickpad but only left/right button click can be recognized by
> X-Server. When putting 1finger still in button area and moving another
> finger in normal area, X-Server would not consider it as 1finger
> cursoring.
>
> Looks like this:
>
> http://ubuntuforums.org/showthread.php?t=1991841
>
> Does X-server have already supported this feature? (I'm sorry I did
> not try the latest X-Server)
It should as far as I know (I do not have any clickpad devices).
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] Input: ims-pcu - remove unneeded get_unaligned_xxx
From: Dmitry Torokhov @ 2014-01-04 6:16 UTC (permalink / raw)
To: Andrey Smirnov; +Cc: linux-input, linux-kernel
In-Reply-To: <CAHQ1cqG2=CX6ypONUgVgzjG_3nVJOBLq5LPYwN0BrzwOLCTxzQ@mail.gmail.com>
On Fri, Jan 03, 2014 at 09:52:25PM -0800, Andrey Smirnov wrote:
> On Fri, Jan 3, 2014 at 9:28 PM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> > pcu->cmd_buf[IMS_PCU_DATA_OFFSET] is word aligned so we do not need to use
> > get_unaligned_le16 to access it.
> >
> > Also let's add build time check to make sure it stays aligned.
>
> - AFAIK, there's no guarantee the "pcu" itself is aligned
Yes. kmalloc returns aligned pointer, otherwise every access to members
other than u8 would risk unaligned exception.
> - This change assumes that aligning data on the 2-byte boundary is
> sufficient for all architectures that do not allow unaligned data
> access, which I don't think is a good assumption to make
What arches require word access be double-word aligned?
> - On x86 or any other architecture that allows unaligned access
> get_unaligned_le16() is actually results to call to le16_to_cpup(), so
> this change doesn't really save anything while imposing restrictions
> on the arrangement of the fields in struct ims_pcu and causing
> unnecessary build errors.
Unless somebody changes the layout there won't be any new build errors,
will there?
Thanks.
--
Dmitry
^ permalink raw reply
* Re: Another ALPS touchpad
From: Tommy Will @ 2014-01-04 6:08 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Yunkang Tang, Dylan Paul Thurston, linux-input
In-Reply-To: <20140104054021.GB32576@core.coreip.homeip.net>
2014/1/4 Dmitry Torokhov <dmitry.torokhov@gmail.com>:
> On Thursday, January 02, 2014 11:59:15 PM Dylan Paul Thurston wrote:
>> Yes, it's an ALPS device and we're now preparing the patch for this new one.
>> BTW, since it's a ClickPad without seperate buttons, simple resting
>> finger logic would be added.
>
> Hmm, what finger resting logic? We already properly support clickpads in
> X driver (as long as the kernel tells userspace that the device is a
> clickpad) so what is missing?
>
I remember in previous kernel version, I tried to register the device
as clickpad but only left/right button click can be recognized by
X-Server. When putting 1finger still in button area and moving another
finger in normal area, X-Server would not consider it as 1finger
cursoring.
Looks like this:
http://ubuntuforums.org/showthread.php?t=1991841
Does X-server have already supported this feature? (I'm sorry I did
not try the latest X-Server)
--
Best Regards,
Tommy
^ permalink raw reply
* Re: [PATCH] Input: ims-pcu - remove unneeded get_unaligned_xxx
From: Andrey Smirnov @ 2014-01-04 5:52 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, linux-kernel
In-Reply-To: <20140104052756.GA32561@core.coreip.homeip.net>
On Fri, Jan 3, 2014 at 9:28 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> pcu->cmd_buf[IMS_PCU_DATA_OFFSET] is word aligned so we do not need to use
> get_unaligned_le16 to access it.
>
> Also let's add build time check to make sure it stays aligned.
- AFAIK, there's no guarantee the "pcu" itself is aligned
- This change assumes that aligning data on the 2-byte boundary is
sufficient for all architectures that do not allow unaligned data
access, which I don't think is a good assumption to make
- On x86 or any other architecture that allows unaligned access
get_unaligned_le16() is actually results to call to le16_to_cpup(), so
this change doesn't really save anything while imposing restrictions
on the arrangement of the fields in struct ims_pcu and causing
unnecessary build errors.
So, for what its worth, NACK for this patch from me.
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
> drivers/input/misc/ims-pcu.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/input/misc/ims-pcu.c b/drivers/input/misc/ims-pcu.c
> index 513ecdf..b8f1029 100644
> --- a/drivers/input/misc/ims-pcu.c
> +++ b/drivers/input/misc/ims-pcu.c
> @@ -116,6 +116,8 @@ struct ims_pcu {
> bool setup_complete; /* Input and LED devices have been created */
> };
>
> +#define IMS_PCU_CHECK_CMD_BUF_ALIGNMENT(offset, alignment) \
> + IS_ALIGNED(offsetof(struct ims_pcu, cmd_buf[offset]), alignment)
>
> /*********************************************************************
> * Buttons Input device support *
> @@ -999,8 +1001,10 @@ ims_pcu_backlight_get_brightness(struct led_classdev *cdev)
> /* Assume the LED is OFF */
> brightness = LED_OFF;
> } else {
> - brightness =
> - get_unaligned_le16(&pcu->cmd_buf[IMS_PCU_DATA_OFFSET]);
> + BUILD_BUG_ON(!IMS_PCU_CHECK_CMD_BUF_ALIGNMENT(
> + IMS_PCU_DATA_OFFSET, 2));
> + brightness = le16_to_cpup(
> + (__le16 *)&pcu->cmd_buf[IMS_PCU_DATA_OFFSET]);
> }
>
> mutex_unlock(&pcu->cmd_mutex);
> --
> 1.8.4.2
>
>
> --
> Dmitry
^ permalink raw reply
* Re: [PATCH v2] ims-pcu: Add commands supported by the new version of the FW
From: Dmitry Torokhov @ 2014-01-04 5:41 UTC (permalink / raw)
To: Andrey Smirnov; +Cc: linux-input, linux-kernel
In-Reply-To: <CAHQ1cqHJOi5mLptHSTw3W8dpdyJrUp8QnmCTtYdNX4YCTTNoaA@mail.gmail.com>
On Fri, Jan 03, 2014 at 09:03:11PM -0800, Andrey Smirnov wrote:
> On Fri, Jan 3, 2014 at 8:44 PM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> > On Fri, Jan 03, 2014 at 08:24:17PM -0800, Andrey Smirnov wrote:
> >> On Fri, Jan 3, 2014 at 5:39 PM, Dmitry Torokhov
> >> <dmitry.torokhov@gmail.com> wrote:
> >> > Hi Andrey,
> >> >
> >> > On Wed, Jan 01, 2014 at 04:47:01PM -0800, Andrey Smirnov wrote:
> >> >> New version of the PCU firmware supports two new commands:
> >> >> - IMS_PCU_CMD_OFN_SET_CONFIG which allows to write data to the
> >> >> registers of one finger navigation(OFN) chip present on the device
> >> >> - IMS_PCU_CMD_OFN_GET_CONFIG which allows to read data form the
> >> >> registers of said chip.
> >> >>
> >> >> This commit adds two helper functions to use those commands and sysfs
> >> >> attributes to use them. It also exposes some OFN configuration
> >> >> parameters via sysfs.
> >> >
> >> > Thank you for making the changes. I do not still quite like the games we
> >> > play with the OFN attributes, how about the patch below (on top of
> >> > yours)?
> >> >
> >>
> >> Yeah, I agree I like it the "two separate sysfs groups" group approach
> >> better. The only small nitpick about your patch is that I think we
> >> should use "get_unaligned_le16" instead of "le16_to_cpup"(In case
> >> anyone decides to run the driver on SuperH or C6x DSPs from TI :-)).
> >> Let me test it and if everything works as expected I'll apply you
> >> patch, convert it to "get_unaligned_le16", squash and send v3 of the
> >> patch.
> >
> > Why do we need get_unaligned_le16()? As far as I can see pcu->cmd_buf is
> > aligned and therefore pcu->cmd_buf[2] is also aligned on word boundary.
>
> * The "pcu" structure is allocated with kmalloc which doesn't give any
> guarantees about address alignment.
> * I am not sure if the cmd_buf field in that structure is aligned, and
> even if it is, any future changes to that structure may shift its
> offset.
> * Also even if the data we are interested in is aligned on 2-byte
> border, I think all those architectures require 4-byte border
> alignment.
As far as I know word access only requires word alignment. Please see
the other patch I just posted that adds alignment check in balcklight
handling code.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: Another ALPS touchpad
From: Dmitry Torokhov @ 2014-01-04 5:40 UTC (permalink / raw)
To: Tommy Will; +Cc: Yunkang Tang, Dylan Paul Thurston, linux-input
In-Reply-To: <CA+F6ckPRj0NrerBC7j41QOE6ZsV7tPQ35gZEfk8cFZws7sT1vg@mail.gmail.com>
Hi Tommy,
On Sat, Jan 04, 2014 at 01:27:24PM +0800, Tommy Will wrote:
> Hi Dmitry,
>
> > Do you have anything in works to support this new model? Or it is
> > not an ALPS device at all?
>
> Yes, it's an ALPS device and we're now preparing the patch for this new one.
> BTW, since it's a ClickPad without seperate buttons, simple resting
> finger logic would be added.
Hmm, what finger resting logic? We already properly support clickpads in
X driver (as long as the kernel tells userspace that the device is a
clickpad) so what is missing?
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCHv4 0/2] twl4030-keypad DT binding
From: Sebastian Reichel @ 2014-01-04 5:40 UTC (permalink / raw)
To: Dmitry Torokhov, Dmitry Torokhov, linux-input
Cc: Rob Herring, Pawel Moll, Mark Rutland, Stephen Warren,
Ian Campbell, Rob Landley, Grant Likely, devicetree, linux-doc,
linux-kernel
In-Reply-To: <1385830416-12900-1-git-send-email-sre@debian.org>
[-- Attachment #1: Type: text/plain, Size: 85 bytes --]
Dmitry,
What's the status of this patch? Can you queue this for 3.14?
-- Sebastian
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH] Input: ims-pcu - fix error unwinding path in application mode
From: Dmitry Torokhov @ 2014-01-04 5:29 UTC (permalink / raw)
To: Andrey Smirnov; +Cc: linux-input, Chris Healy, linux-kernel
In-Reply-To: <CAHQ1cqGZT7TOCX+5yPhWt1MpsSbAfbr07SProgSxrn7RO=7aig@mail.gmail.com>
On Fri, Jan 03, 2014 at 09:13:10PM -0800, Andrey Smirnov wrote:
> Dmitry,
>
> Do you want this patch to go separatly or do you want me to bundle it
> with my other changes(probably the first one)?
I'll apply it separately.
> Also, offtopic, do you want me to backport this changes to 2.6 kernel
> that IMS uses or would you do it?
It should apply as is, no backport needed as far as I can tell.
--
Dmitry
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox