* [PATCH v2 1/4] input: touchscreen: of: Use input_set_abs_params
From: Maxime Ripard @ 2014-11-13 14:06 UTC (permalink / raw)
To: Lothar Waßmann, Dmitry Torokhov, Henrik Rydberg
Cc: linux-input, linux-kernel, Markus Pargmann, Maxime Ripard
In-Reply-To: <1415887618-29844-1-git-send-email-maxime.ripard@free-electrons.com>
Drivers are still required to call input_set_abs_params for their axes, as if
they only use the touchscreen_parse_of_params function, the axis bit in absbit
won't be set.
Switch to using input_set_abs_params to fully setup each and every available
axis so that drivers will be able to solely use this function.
Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
drivers/input/touchscreen/of_touchscreen.c | 50 ++++++++++++++++++++++++------
1 file changed, 40 insertions(+), 10 deletions(-)
diff --git a/drivers/input/touchscreen/of_touchscreen.c b/drivers/input/touchscreen/of_touchscreen.c
index f8f9b84230b1..fe80d8ba7efa 100644
--- a/drivers/input/touchscreen/of_touchscreen.c
+++ b/drivers/input/touchscreen/of_touchscreen.c
@@ -13,6 +13,33 @@
#include <linux/input.h>
#include <linux/input/touchscreen.h>
+static u32 of_get_optional_u32(struct device_node *np,
+ const char *property)
+{
+ u32 val = 0;
+
+ of_property_read_u32(np, property, &val);
+
+ return val;
+}
+
+static void touchscreen_set_params(struct input_dev *dev,
+ unsigned long axis,
+ int max, int fuzz)
+{
+ struct input_absinfo *absinfo;
+
+ if (!test_bit(axis, dev->absbit)) {
+ dev_warn(&dev->dev,
+ "DT specifies parameters but the axis is not set up\n");
+ return;
+ }
+
+ absinfo = &dev->absinfo[axis];
+ absinfo->maximum = max;
+ absinfo->fuzz = fuzz;
+}
+
/**
* touchscreen_parse_of_params - parse common touchscreen DT properties
* @dev: device that should be parsed
@@ -24,22 +51,25 @@
void touchscreen_parse_of_params(struct input_dev *dev)
{
struct device_node *np = dev->dev.parent->of_node;
- struct input_absinfo *absinfo;
+ u32 maximum, fuzz;
input_alloc_absinfo(dev);
if (!dev->absinfo)
return;
- absinfo = &dev->absinfo[ABS_X];
- of_property_read_u32(np, "touchscreen-size-x", &absinfo->maximum);
- of_property_read_u32(np, "touchscreen-fuzz-x", &absinfo->fuzz);
+ maximum = of_get_optional_u32(np, "touchscreen-size-x");
+ fuzz = of_get_optional_u32(np, "touchscreen-fuzz-x");
+ if (maximum || fuzz)
+ touchscreen_set_params(dev, ABS_X, maximum, fuzz);
- absinfo = &dev->absinfo[ABS_Y];
- of_property_read_u32(np, "touchscreen-size-y", &absinfo->maximum);
- of_property_read_u32(np, "touchscreen-fuzz-y", &absinfo->fuzz);
+ maximum = of_get_optional_u32(np, "touchscreen-size-y");
+ fuzz = of_get_optional_u32(np, "touchscreen-fuzz-y");
+ if (maximum || fuzz)
+ touchscreen_set_params(dev, ABS_Y, maximum, fuzz);
- absinfo = &dev->absinfo[ABS_PRESSURE];
- of_property_read_u32(np, "touchscreen-max-pressure", &absinfo->maximum);
- of_property_read_u32(np, "touchscreen-fuzz-pressure", &absinfo->fuzz);
+ maximum = of_get_optional_u32(np, "touchscreen-max-pressure");
+ fuzz = of_get_optional_u32(np, "touchscreen-fuzz-pressure");
+ if (maximum || fuzz)
+ touchscreen_set_params(dev, ABS_PRESSURE, maximum, fuzz);
}
EXPORT_SYMBOL(touchscreen_parse_of_params);
--
2.1.1
^ permalink raw reply related
* [PATCH v2 0/4] input: ft5x06: Fix userspace reported maximum value
From: Maxime Ripard @ 2014-11-13 14:06 UTC (permalink / raw)
To: Lothar Waßmann, Dmitry Torokhov, Henrik Rydberg
Cc: linux-input, linux-kernel, Markus Pargmann, Maxime Ripard
Hi,
The current ft5x06 reports to the user-space that its maximum
coordinates are, on both X and Y, way higher than what could be
actually usable on the screen (in my case, 5759x1151 instead of
480x800).
This causes trouble on some userspace stacks that then try to re-scale
these coordinates back to the framebuffer resolution, like QT does.
Use the of_touchscreen code to find the real touchscreen limits in the
DT case, and report that to the userspace.
Thanks,
Maxime
Changes from v1:
- Do not use input_set_abs_params anymore to avoid enabling an axis
that was not enabled by the driver itself in of_touchscreen
- Emit a warning when an axis is set in the DT but not enabled in
the driver
- Remove EV_SYN from the edt-ft5x06 driver
Maxime Ripard (4):
input: touchscreen: of: Use input_set_abs_params
input: touchscreen: of: Register multitouch axes
input: ft5x06: Allow to set the maximum axes value through the DT
input: edt-ft5x06: Remove EV_SYN event report
drivers/input/touchscreen/edt-ft5x06.c | 6 ++-
drivers/input/touchscreen/of_touchscreen.c | 62 +++++++++++++++++++++++++-----
2 files changed, 57 insertions(+), 11 deletions(-)
--
2.1.1
^ permalink raw reply
* Re: [PATCH v3] Input: evdev - add event-mask API
From: David Herrmann @ 2014-11-13 12:38 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: open list:HID CORE LAYER, Peter Hutterer
In-Reply-To: <CANq1E4SGnOcPbKaUKzg4zVYSCE+TMJmrvDXPgAm2Gj=4Zu-PTA@mail.gmail.com>
Hi
On Wed, Nov 12, 2014 at 1:41 PM, David Herrmann <dh.herrmann@gmail.com> wrote:
> Hi
>
> On Wed, Nov 12, 2014 at 8:40 AM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
>> Hi David,
>>
>> On Tue, Nov 04, 2014 at 06:12:23PM +0100, David Herrmann wrote:
>>> +static int bits_from_user(unsigned long *bits, unsigned int maxbit,
>>> + unsigned int maxlen, const void __user *p, int compat)
>>> +{
>>> + int len;
>>> +
>>> +#if IS_ENABLED(CONFIG_COMPAT)
>>> + if (compat)
>>
>> I think this can be written as:
>> if (IS_ENABLED(CONFIG_COMPAT) && compat)
>
> BITS_TO_LONGS_COMPAT is only defined if CONFIG_COMPAT is. We'd rely on
> compiler-optimizations (dead code elimination) if we'd do that change.
> I'm not really fond of making -O0 compiles fail, but there're several
> places in the kernel that do. Your call ;)
>
>>> + len = BITS_TO_LONGS_COMPAT(maxbit) * sizeof(compat_long_t);
>>> + else
>>> +#endif
>>> + len = BITS_TO_LONGS(maxbit) * sizeof(long);
>>> +
>>> + if (len > maxlen)
>>> + len = maxlen;
>>> +
>>> +#if IS_ENABLED(CONFIG_COMPAT) && defined(__BIG_ENDIAN)
>>> + if (compat) {
>>> + int i;
>>> +
>>> + for (i = 0; i < len / sizeof(compat_long_t); i++)
>>> + if (copy_from_user((compat_long_t *) bits +
>>> + i + 1 - ((i % 2) << 1),
>>> + (compat_long_t __user *) p + i,
>>> + sizeof(compat_long_t)))
>>
>> Unfortunately this is not quite enough, you need to also zero out the
>> upper bits of the last partial if userspace did not supply multiple of
>> 64 bits.
>>
>> Frankly, bitmask APIs that we have in input are god-awful on big endian:
>> they do not work if usespace uses quantities less than long (or compat
>> long). I think we'll have to enforce it in the code.
>
> Right, I relied on the caller of bits_from_user() to initialize it to
> zero and make it long-aligned. I can move it into that helper.
I now added a memset(bits, 0, len); before "if (len > maxlen)". This
just clears the buffer before copying data. Due to the cross-over in
the for()-loop, I'm not entirely sure how to only clear the unused
parts, as the tail "long"s might be only set partially. Quite
confusing..
I didn't change anything else. Lemme know if that's fine and I will
resend the patch.
Thanks
David
>>> + } else {
>>> + /* fake mask with all bits set */
>>> + out = (u8 __user*)codes;
>>> + for (i = 0; i < min; ++i) {
>>> + if (put_user((u8)0xff, out + i))
>>> + return -EFAULT;
>>
>> I wonder if we should not replace this with access_ok() + straight
>> memset instead of doing byte-by-byte copy.
>
> I'm actually not very familiar with the details of copy_to_user().
> That's why I went with put_user(). I can look into it, but if you know
> it well, I am open for suggestions.
>
> Thanks
> David
^ permalink raw reply
* Re: [PATCH 0/4] Touchscreen performance related fixes
From: Vignesh R @ 2014-11-13 12:23 UTC (permalink / raw)
To: Johannes Pointner
Cc: Richard Cochran, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, Benoit Cousson, Tony Lindgren,
Russell King, Jonathan Cameron, Dmitry Torokhov, devicetree,
Lars-Peter Clausen, Samuel Ortiz, Jan Kardell, linux-iio,
Sebastian Andrzej Siewior, linux-input, linux-kernel,
Felipe Balbi, Paul Gortmaker, Peter Meerwald, Hartmut Knaack,
linux-om
In-Reply-To: <CAJKbBDDyShaJkzzfdFs07EK5gDUfwqWiv39Qi4FVSvRrfc6_fA@mail.gmail.com>
Hi,
On Wednesday 12 November 2014 06:30 PM, Johannes Pointner wrote:
> Hello Vignesh,
>
> I tried your patch version 3 on a customized board and had some
> behavior I couldn't explain.
> If I only use the touchscreen it works fine but if I also read values
> from the ADCs then I get a lot of pen_up events even if I am still
> touching the screen.
> For the test I read via
> # cat /sys/bus/iio/devices/iio\:device0/in_voltage5_raw
> values from the ADC in an busy loop as you explained in an email
> before. Did you also experience such behavior or do you know what
> causes it?
Thanks for testing. I was able to fix this issue. Will post version 4
shortly.
Regards
Vignesh
>
> Without the patches the touchscreen works fine during the iio test.
>
> Thanks,
> Hannes
>
> 2014-11-06 8:42 GMT+01:00 Vignesh R <vigneshr@ti.com>:
>>
>>
>> On Monday 03 November 2014 11:39 PM, Richard Cochran wrote:
>>> On Mon, Oct 27, 2014 at 04:38:27PM +0530, Vignesh R wrote:
>>>> This series of patches fix TSC defects related to lag in touchscreen
>>>> performance and cursor jump at touch release. The lag was result of
>>>> udelay in TSC interrupt handler. Cursor jump due to false pen-up event.
>>>> The patches implement Advisory 1.0.31 in silicon errata of am335x-evm
>>>> to avoid false pen-up events and remove udelay.
>>>
>>> That advisory has two workarounds. You have chosen the second one?
>>
>> Work around one. Hence 5 wire design is not broken.
>>
>>>
>>> The text of the second workaround says it only works on 4 wire setups,
>>> so I wonder how 5 wire designs will be affected.
>>>
>>>> The advisory says to use
>>>> steps 1 to 4 for ADC and 5 to 16 for TSC (assuming 4 wire TSC and 4 channel
>>>> ADC).
>>>
>>> No, it doesn't say that. (sprz360f.pdf)
>>
>> The pen up event detection happens immediately after charge step. Hence,
>> interchanging ADC and TSC steps makes sure that sampling of touch
>> co-ordinates and pen events are done one after the other. This
>> workaround was suggested by internal hardware folks. Earlier ADC steps
>> intervened between sampling of co-ordinates and pen event detection
>> which is not desirable.
>>
>>>
>>>> Further the X co-ordinate must be the last one to be sampled just
>>>> before charge step. The first two patches implement the required changes.
>>>
>>> FWIW, I implemented the first workaround and removed the udelay not
>>> too long ago. Like Sebastian, I saw the TSC unit hang after about
>>> 50000 interrupts when running with the workaround.
>>>
>>> Did you test you patch for very long?
>>
>> Yes, I tested for about 200000 interrupts and I didn't see any hang.
>> This patch series does not just implement workaround but also does some
>> minor changes, such as interchanging ADC and TSC steps etc, which makes
>> TSC driver more robust. Let me know if you encounter any issues with my
>> patch series.
>>
>> Regards
>> Vignesh
>>
>>>
>>> Thanks,
>>> Richard
>>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2 1/1] Input: gpio_keys - add device tree support for interrupt only keys
From: Laxman Dewangan @ 2014-11-13 11:17 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Alexander Stein, Dmitry Torokhov, Rob Herring, Pawel Moll,
Mark Rutland, Ian Campbell, Kumar Gala, linux-input, devicetree,
linux-gpio
In-Reply-To: <1587930.UcHkd3vGmk@wuerfel>
On Thursday 13 November 2014 04:35 PM, Arnd Bergmann wrote:
> On Thursday 13 November 2014 16:29:02 Laxman Dewangan wrote:
>>
>> Yes, I posted patch for interrupt key on context on key connected to
>> PMIC-Onkey.
>> On PMIC, there is onkey pin input which generates interrupt only when it
>> toggles. So if we have key (power key on our platforms) connected to
>> this pin then we will only get interrupt from PMIC.
>> Instead of implementing full interrupt key driver, I added this support
>> on existing gpio keys.
> So is this code still in place, and do you think it's worthwhile to
> have a DT binding for it?
>
> Could the PMIC register a gpio controller instead?
>
>
This pin is not supported as GPIO and so exporting the functionality
through GPIO is not feasible until we treat the onkey as GPIO pin
without actual gpio functionality i..e only interrupt functionality.
^ permalink raw reply
* Re: [PATCH v2 1/1] Input: gpio_keys - add device tree support for interrupt only keys
From: Arnd Bergmann @ 2014-11-13 11:05 UTC (permalink / raw)
To: Laxman Dewangan
Cc: Alexander Stein, Dmitry Torokhov, Rob Herring, Pawel Moll,
Mark Rutland, Ian Campbell, Kumar Gala, linux-input, devicetree,
linux-gpio
In-Reply-To: <54648EF6.10402@nvidia.com>
On Thursday 13 November 2014 16:29:02 Laxman Dewangan wrote:
> On Thursday 13 November 2014 04:17 PM, Arnd Bergmann wrote:
> > I notice that Laxman Dewangan initially added the feature as part of
> > (I assume) work on Tegra, but these days Tegra is DT-only so it can't
> > actually get used for that any more.
> >
> > If we have a real usecase, I think we can still take your patch, but
> > my impression at the moment is that it may be better to instead
> > remove the feature entirely by reverting d8ee4a1c9052 ("Input:
> > gpio_keys - add support for interrupt only keys").
> >
> > Regarding your initial use case of testing interrupt polarity, would
> > you have been able to do the same thing by looking at the interrupt
> > count in /proc/interrupts?
> >
>
> Yes, I posted patch for interrupt key on context on key connected to
> PMIC-Onkey.
> On PMIC, there is onkey pin input which generates interrupt only when it
> toggles. So if we have key (power key on our platforms) connected to
> this pin then we will only get interrupt from PMIC.
> Instead of implementing full interrupt key driver, I added this support
> on existing gpio keys.
So is this code still in place, and do you think it's worthwhile to
have a DT binding for it?
Could the PMIC register a gpio controller instead?
Arnd
^ permalink raw reply
* Re: [PATCH v2 1/1] Input: gpio_keys - add device tree support for interrupt only keys
From: Laxman Dewangan @ 2014-11-13 11:02 UTC (permalink / raw)
To: Arnd Bergmann, Alexander Stein
Cc: Dmitry Torokhov, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, linux-input, devicetree, linux-gpio
In-Reply-To: <1616597.6UJpl1xeSr@wuerfel>
On Thursday 13 November 2014 04:30 PM, Arnd Bergmann wrote:
> On Thursday 13 November 2014 11:52:48 Alexander Stein wrote:
>> On Thursday 13 November 2014 11:47:13, Arnd Bergmann wrote:
>>> Regarding your initial use case of testing interrupt polarity, would
>>> you have been able to do the same thing by looking at the interrupt
>>> count in /proc/interrupts?
>> This is only possible if some driver actually requests this interrupt, no?
> Yes, I think that is correct, at least with sparseirq, which is now the
> default. For interrupts that are mapped by the irqchip, you can look
> up the number of spurious interrupts in /proc/irq/<nr>/spurious.
>
>
yes, I came across multiple PMIC(TPS65910, MAXIM, Ricoh) module which
generates interrupt when their On-key pin get toggle. Key is connected
with this pin of PMIC and for on-key driver, we are using the gpio-keys
driver by passing interrupt number instead of complete new driver.
^ permalink raw reply
* Re: [PATCH v2 1/1] Input: gpio_keys - add device tree support for interrupt only keys
From: Arnd Bergmann @ 2014-11-13 11:00 UTC (permalink / raw)
To: Alexander Stein
Cc: Dmitry Torokhov, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, linux-input, devicetree, linux-gpio,
Laxman Dewangan
In-Reply-To: <4899804.1F6dNN1eJX@ws-stein>
On Thursday 13 November 2014 11:52:48 Alexander Stein wrote:
> On Thursday 13 November 2014 11:47:13, Arnd Bergmann wrote:
> > Regarding your initial use case of testing interrupt polarity, would
> > you have been able to do the same thing by looking at the interrupt
> > count in /proc/interrupts?
>
> This is only possible if some driver actually requests this interrupt, no?
Yes, I think that is correct, at least with sparseirq, which is now the
default. For interrupts that are mapped by the irqchip, you can look
up the number of spurious interrupts in /proc/irq/<nr>/spurious.
Arnd
^ permalink raw reply
* Re: [PATCH v2 1/1] Input: gpio_keys - add device tree support for interrupt only keys
From: Laxman Dewangan @ 2014-11-13 10:59 UTC (permalink / raw)
To: Arnd Bergmann, Alexander Stein
Cc: Dmitry Torokhov, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, linux-input, devicetree, linux-gpio
In-Reply-To: <1763965.yCxTDubTPH@wuerfel>
On Thursday 13 November 2014 04:17 PM, Arnd Bergmann wrote:
> On Thursday 13 November 2014 11:35:42 Alexander Stein wrote:
>> On Wednesday 12 November 2014 20:16:06, Arnd Bergmann wrote:
>>> On Wednesday 12 November 2014 17:38:31 Alexander Stein wrote:
>>>> On Wednesday 12 November 2014 17:04:57, Arnd Bergmann wrote:
>>>>> On Wednesday 12 November 2014 17:02:56 Alexander Stein wrote:
>>>>>> This features already exists for board config setups. Add support for
>>>>>> device tree based systems.
>>>>>>
>>>>>> Signed-off-by: Alexander Stein <alexander.stein@systec-electronic.com>
>>>>>> ---
>>>>>> Please note: Due to current lack of hardware I could not test it yet. V2
>>>>>> includes the changes proposed by Dmitry.
>>>>>>
>>>>>> Changes in v2:
>>>>>> * Added device tree bindings
>>>>>> * IRQ is only parsed and mapped when there is no "gpios" property
>>>>>>
>>>>> Can you list one or more examples in the patch description?
>>>>> Are these systems that don't expose the GPIO controller with a
>>>>> standalone driver, or systems that really actually connect the
>>>>> buttons to an interrupt pin?
>>>> You mean a use case? I came to this situation to test interrupt polarity on a microcontroller, thus a simple /IRQ pin, no GPIO at all. So in the end I have an input just connected to an interrupt line.
>>>> I noticed gpio_keys using platform data only can be used for this setup. So I added this support for device tree.
>>>>
>>> I meant a specific board file that uses this, which can't be converted
>>> to DT without your change.
>> I've searched (hopefully) the complete arch/ tree on v3.18-rc4 for
>> struct gpio_keys_button and checked each occurrence. I didn't found
>> any usage of IRQ based input keys.
> Ok, I see.
>
> I notice that Laxman Dewangan initially added the feature as part of
> (I assume) work on Tegra, but these days Tegra is DT-only so it can't
> actually get used for that any more.
>
> If we have a real usecase, I think we can still take your patch, but
> my impression at the moment is that it may be better to instead
> remove the feature entirely by reverting d8ee4a1c9052 ("Input:
> gpio_keys - add support for interrupt only keys").
>
> Regarding your initial use case of testing interrupt polarity, would
> you have been able to do the same thing by looking at the interrupt
> count in /proc/interrupts?
>
Yes, I posted patch for interrupt key on context on key connected to
PMIC-Onkey.
On PMIC, there is onkey pin input which generates interrupt only when it
toggles. So if we have key (power key on our platforms) connected to
this pin then we will only get interrupt from PMIC.
Instead of implementing full interrupt key driver, I added this support
on existing gpio keys.
^ permalink raw reply
* Re: [PATCH v2 1/1] Input: gpio_keys - add device tree support for interrupt only keys
From: Alexander Stein @ 2014-11-13 10:52 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Dmitry Torokhov, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, linux-input, devicetree, linux-gpio,
Laxman Dewangan
In-Reply-To: <1763965.yCxTDubTPH@wuerfel>
On Thursday 13 November 2014 11:47:13, Arnd Bergmann wrote:
> Regarding your initial use case of testing interrupt polarity, would
> you have been able to do the same thing by looking at the interrupt
> count in /proc/interrupts?
This is only possible if some driver actually requests this interrupt, no?
Best regards,
Alexander
--
Dipl.-Inf. Alexander Stein
SYS TEC electronic GmbH
Am Windrad 2
08468 Heinsdorfergrund
Tel.: 03765 38600-1156
Fax: 03765 38600-4100
Email: alexander.stein@systec-electronic.com
Website: www.systec-electronic.com
Managing Director: Dipl.-Phys. Siegmar Schmidt
Commercial registry: Amtsgericht Chemnitz, HRB 28082
^ permalink raw reply
* Re: [PATCH v2 1/1] Input: gpio_keys - add device tree support for interrupt only keys
From: Arnd Bergmann @ 2014-11-13 10:47 UTC (permalink / raw)
To: Alexander Stein
Cc: Dmitry Torokhov, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, linux-input, devicetree, linux-gpio,
Laxman Dewangan
In-Reply-To: <5340312.bOFF4aoNWW@ws-stein>
On Thursday 13 November 2014 11:35:42 Alexander Stein wrote:
> On Wednesday 12 November 2014 20:16:06, Arnd Bergmann wrote:
> > On Wednesday 12 November 2014 17:38:31 Alexander Stein wrote:
> > > On Wednesday 12 November 2014 17:04:57, Arnd Bergmann wrote:
> > > > On Wednesday 12 November 2014 17:02:56 Alexander Stein wrote:
> > > > > This features already exists for board config setups. Add support for
> > > > > device tree based systems.
> > > > >
> > > > > Signed-off-by: Alexander Stein <alexander.stein@systec-electronic.com>
> > > > > ---
> > > > > Please note: Due to current lack of hardware I could not test it yet. V2
> > > > > includes the changes proposed by Dmitry.
> > > > >
> > > > > Changes in v2:
> > > > > * Added device tree bindings
> > > > > * IRQ is only parsed and mapped when there is no "gpios" property
> > > > >
> > > >
> > > > Can you list one or more examples in the patch description?
> > > > Are these systems that don't expose the GPIO controller with a
> > > > standalone driver, or systems that really actually connect the
> > > > buttons to an interrupt pin?
> > >
> > > You mean a use case? I came to this situation to test interrupt polarity on a microcontroller, thus a simple /IRQ pin, no GPIO at all. So in the end I have an input just connected to an interrupt line.
> > > I noticed gpio_keys using platform data only can be used for this setup. So I added this support for device tree.
> > >
> >
> > I meant a specific board file that uses this, which can't be converted
> > to DT without your change.
>
> I've searched (hopefully) the complete arch/ tree on v3.18-rc4 for
> struct gpio_keys_button and checked each occurrence. I didn't found
> any usage of IRQ based input keys.
Ok, I see.
I notice that Laxman Dewangan initially added the feature as part of
(I assume) work on Tegra, but these days Tegra is DT-only so it can't
actually get used for that any more.
If we have a real usecase, I think we can still take your patch, but
my impression at the moment is that it may be better to instead
remove the feature entirely by reverting d8ee4a1c9052 ("Input:
gpio_keys - add support for interrupt only keys").
Regarding your initial use case of testing interrupt polarity, would
you have been able to do the same thing by looking at the interrupt
count in /proc/interrupts?
Arnd
^ permalink raw reply
* Re: [PATCH v2 1/1] Input: gpio_keys - add device tree support for interrupt only keys
From: Alexander Stein @ 2014-11-13 10:35 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Dmitry Torokhov, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, linux-input, devicetree
In-Reply-To: <2197757.1V0yxO5sLr@wuerfel>
On Wednesday 12 November 2014 20:16:06, Arnd Bergmann wrote:
> On Wednesday 12 November 2014 17:38:31 Alexander Stein wrote:
> > On Wednesday 12 November 2014 17:04:57, Arnd Bergmann wrote:
> > > On Wednesday 12 November 2014 17:02:56 Alexander Stein wrote:
> > > > This features already exists for board config setups. Add support for
> > > > device tree based systems.
> > > >
> > > > Signed-off-by: Alexander Stein <alexander.stein@systec-electronic.com>
> > > > ---
> > > > Please note: Due to current lack of hardware I could not test it yet. V2
> > > > includes the changes proposed by Dmitry.
> > > >
> > > > Changes in v2:
> > > > * Added device tree bindings
> > > > * IRQ is only parsed and mapped when there is no "gpios" property
> > > >
> > >
> > > Can you list one or more examples in the patch description?
> > > Are these systems that don't expose the GPIO controller with a
> > > standalone driver, or systems that really actually connect the
> > > buttons to an interrupt pin?
> >
> > You mean a use case? I came to this situation to test interrupt polarity on a microcontroller, thus a simple /IRQ pin, no GPIO at all. So in the end I have an input just connected to an interrupt line.
> > I noticed gpio_keys using platform data only can be used for this setup. So I added this support for device tree.
> >
>
> I meant a specific board file that uses this, which can't be converted
> to DT without your change.
I've searched (hopefully) the complete arch/ tree on v3.18-rc4 for struct gpio_keys_button and checked each occurrence. I didn't found any usage of IRQ based input keys.
Best regards,
Alexander
^ permalink raw reply
* Re: [PATCH v3 3/6] mfd: ti_am335x_tscadc: Remove unwanted reg_se_cache save
From: Lee Jones @ 2014-11-13 10:25 UTC (permalink / raw)
To: R, Vignesh
Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
Benoit Cousson, Tony Lindgren, Russell King, Jonathan Cameron,
Hartmut Knaack,
richardcochran-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
Dmitry Torokhov, Sebastian Andrzej Siewior, Lars-Peter Clausen,
Peter Meerwald, Samuel Ortiz, Balbi, Felipe, Griffis, Brad,
Sanjeev Sharma, Paul Gortmaker, Jan Kardell,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <71C24A547111EA4E84AE759B748D9EAE4B976C-yXqyApvAXouIQmiDNMet8wC/G2K4zDHf@public.gmane.org>
On Tue, 11 Nov 2014, R, Vignesh wrote:
> On Tue, 11 Nov 2014, Vignesh R wrote:
> > In one shot mode, sequencer automatically disables all enabled steps
> > at the end of each cycle. (both ADC steps and TSC steps) Hence these
> > steps need not be saved in reg_se_cache for clearing these steps at a
> > later stage.
> > Also, when ADC wakes up Sequencer should not be busy executing any of
> > the config steps except for the charge step. Previously charge step
> > was 1 ADC clock cycle and hence it was ignored.
> >
> > Signed-off-by: Vignesh R <vigneshr-l0cyMroinI0@public.gmane.org>
> > ---
[...]
> > - tsadc->reg_se_cache |= val;
>
> You didn't answer my question about this?
> I did reply to the question in the previous thread.
>
> Previously, TSC did not reliably re-enable its steps as the TSC irq handler received
> false pen up events. Hence, in order to use TSC after ADC operation, it was necessary to
> save and re-enable TSC steps (basically, to keep TSC steps enabled always).
> The change was more of a workaround to overcome limitation of TSC irq handler. With
> this series of patches, TSC irq handler is very reliable and the workaround is no longer required.
Okay, thanks for the explanation.
By the way, your mailer doesn't appear to quote previous messages. Is
there any way to fix that?
> > am335x_tscadc_need_adc(tsadc);
> >
> > tscadc_writel(tsadc, REG_SE, val);
> > diff --git a/include/linux/mfd/ti_am335x_tscadc.h
> > b/include/linux/mfd/ti_am335x_tscadc.h
> > index c99be5dc0f5c..fcce182e4a35 100644
> > --- a/include/linux/mfd/ti_am335x_tscadc.h
> > +++ b/include/linux/mfd/ti_am335x_tscadc.h
> > @@ -128,6 +128,7 @@
> >
> > /* Sequencer Status */
> > #define SEQ_STATUS BIT(5)
> > +#define CHARGE_STEP 0x11
> >
> > #define ADC_CLK 3000000
> > #define TOTAL_STEPS 16
>
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* [PATCH v5] psmouse: Add some support for the FocalTech PS/2 protocol extensions.
From: Mathias Gottschlag @ 2014-11-12 23:00 UTC (permalink / raw)
To: Dmitry Torokhov, Hans de Goede; +Cc: linux-input, Mathias Gottschlag
In-Reply-To: <1415832976-1680-1-git-send-email-mgottschlag@gmail.com>
Most of the protocol for these touchpads has been reverse engineered. This
commit adds a basic multitouch-capable driver.
A lot of the protocol is still unknown. Especially, we don't know how to
identify the device yet apart from the PNP ID.
The previous workaround for these devices has been left in place in case
the driver is not compiled into the kernel or in case some other device
with the same PNP ID is not recognized by the driver yet still has the same
problems with the device probing code.
Signed-off-by: Mathias Gottschlag <mgottschlag@gmail.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
---
(Resent with the correct subject to prevent confusion, sorry, this is v5)
Various smaller changes as requested. Also, accidentally pressing the touchpad
disable button can cause really frustrating debugging sessions.
drivers/input/mouse/Kconfig | 10 ++
drivers/input/mouse/focaltech.c | 313 ++++++++++++++++++++++++++++++++++++-
drivers/input/mouse/focaltech.h | 60 +++++++
drivers/input/mouse/psmouse-base.c | 32 ++--
drivers/input/mouse/psmouse.h | 1 +
5 files changed, 399 insertions(+), 17 deletions(-)
diff --git a/drivers/input/mouse/Kconfig b/drivers/input/mouse/Kconfig
index 366fc7a..db973e5 100644
--- a/drivers/input/mouse/Kconfig
+++ b/drivers/input/mouse/Kconfig
@@ -146,6 +146,16 @@ config MOUSE_PS2_OLPC
If unsure, say N.
+config MOUSE_PS2_FOCALTECH
+ bool "FocalTech PS/2 mouse protocol extension" if EXPERT
+ default y
+ depends on MOUSE_PS2
+ help
+ Say Y here if you have a FocalTech PS/2 TouchPad connected to
+ your system.
+
+ If unsure, say Y.
+
config MOUSE_SERIAL
tristate "Serial mouse"
select SERIO
diff --git a/drivers/input/mouse/focaltech.c b/drivers/input/mouse/focaltech.c
index f4d657e..f838cab4 100644
--- a/drivers/input/mouse/focaltech.c
+++ b/drivers/input/mouse/focaltech.c
@@ -2,6 +2,7 @@
* Focaltech TouchPad PS/2 mouse driver
*
* Copyright (c) 2014 Red Hat Inc.
+ * Copyright (c) 2014 Mathias Gottschlag <mgottschlag@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
@@ -13,15 +14,14 @@
* Hans de Goede <hdegoede@redhat.com>
*/
-/*
- * The Focaltech PS/2 touchpad protocol is unknown. This drivers deals with
- * detection only, to avoid further detection attempts confusing the touchpad
- * this way it at least works in PS/2 mouse compatibility mode.
- */
#include <linux/device.h>
#include <linux/libps2.h>
+#include <linux/input/mt.h>
+#include <linux/serio.h>
+#include <linux/slab.h>
#include "psmouse.h"
+#include "focaltech.h"
static const char * const focaltech_pnp_ids[] = {
"FLT0101",
@@ -30,6 +30,12 @@ static const char * const focaltech_pnp_ids[] = {
NULL
};
+/*
+ * Even if the kernel is built without support for Focaltech PS/2 touchpads (or
+ * when the real driver fails to recognize the device), we still have to detect
+ * them in order to avoid further detection attempts confusing the touchpad.
+ * This way it at least works in PS/2 mouse compatibility mode.
+ */
int focaltech_detect(struct psmouse *psmouse, bool set_properties)
{
if (!psmouse_matches_pnp_id(psmouse, focaltech_pnp_ids))
@@ -37,16 +43,309 @@ int focaltech_detect(struct psmouse *psmouse, bool set_properties)
if (set_properties) {
psmouse->vendor = "FocalTech";
- psmouse->name = "FocalTech Touchpad in mouse emulation mode";
+ psmouse->name = "FocalTech Touchpad";
}
return 0;
}
-int focaltech_init(struct psmouse *psmouse)
+static void focaltech_reset(struct psmouse *psmouse)
{
ps2_command(&psmouse->ps2dev, NULL, PSMOUSE_CMD_RESET_DIS);
psmouse_reset(psmouse);
+}
+
+#ifdef CONFIG_MOUSE_PS2_FOCALTECH
+
+static void focaltech_report_state(struct psmouse *psmouse)
+{
+ int i;
+ struct focaltech_data *priv = psmouse->private;
+ struct focaltech_hw_state *state = &priv->state;
+ struct input_dev *dev = psmouse->dev;
+
+ for (i = 0; i < FOC_MAX_FINGERS; i++) {
+ struct focaltech_finger_state *finger = &state->fingers[i];
+ bool active = finger->active && finger->valid;
+ input_mt_slot(dev, i);
+ input_mt_report_slot_state(dev, MT_TOOL_FINGER, active);
+ if (active) {
+ input_report_abs(dev, ABS_MT_POSITION_X, finger->x);
+ input_report_abs(dev, ABS_MT_POSITION_Y,
+ focaltech_invert_y(finger->y));
+ }
+ }
+ input_mt_report_pointer_emulation(dev, true);
+
+ input_report_key(psmouse->dev, BTN_LEFT, state->pressed);
+ input_sync(psmouse->dev);
+}
+
+static void process_touch_packet(struct focaltech_hw_state *state,
+ unsigned char *packet)
+{
+ int i;
+ unsigned char fingers = packet[1];
+
+ state->pressed = (packet[0] >> 4) & 1;
+ /* the second byte contains a bitmap of all fingers touching the pad */
+ for (i = 0; i < FOC_MAX_FINGERS; i++) {
+ state->fingers[i].active = fingers & 0x1;
+ if (!state->fingers[i].active) {
+ /* even when the finger becomes active again, we still
+ * will have to wait for the first valid position */
+ state->fingers[i].valid = false;
+ }
+ fingers >>= 1;
+ }
+}
+
+static void process_abs_packet(struct psmouse *psmouse,
+ unsigned char *packet)
+{
+ struct focaltech_data *priv = psmouse->private;
+ struct focaltech_hw_state *state = &priv->state;
+ unsigned int finger;
+
+ finger = (packet[1] >> 4) - 1;
+ if (finger >= FOC_MAX_FINGERS) {
+ psmouse_err(psmouse, "Invalid finger in abs packet: %d",
+ finger);
+ return;
+ }
+
+ state->pressed = (packet[0] >> 4) & 1;
+ /*
+ * packet[5] contains some kind of tool size in the most significant
+ * nibble. 0xff is a special value (latching) that signals a large
+ * contact area.
+ */
+ if (packet[5] == 0xff) {
+ state->fingers[finger].valid = false;
+ return;
+ }
+ state->fingers[finger].x = ((packet[1] & 0xf) << 8) | packet[2];
+ state->fingers[finger].y = (packet[3] << 8) | packet[4];
+ state->fingers[finger].valid = true;
+}
+
+static void process_rel_packet(struct psmouse *psmouse,
+ unsigned char *packet)
+{
+ struct focaltech_data *priv = psmouse->private;
+ struct focaltech_hw_state *state = &priv->state;
+ int finger1, finger2;
+
+ state->pressed = packet[0] >> 7;
+ finger1 = ((packet[0] >> 4) & 0x7) - 1;
+ if (finger1 < FOC_MAX_FINGERS) {
+ state->fingers[finger1].x += (char)packet[1];
+ state->fingers[finger1].y += (char)packet[2];
+ } else {
+ psmouse_err(psmouse, "First finger in rel packet invalid: %d",
+ finger1);
+ }
+ /*
+ * If there is an odd number of fingers, the last relative packet only
+ * contains one finger. In this case, the second finger index in the
+ * packet is 0 (we subtract 1 in the lines above to create array
+ * indices, so the finger will overflow and be above FOC_MAX_FINGERS).
+ */
+ finger2 = ((packet[3] >> 4) & 0x7) - 1;
+ if (finger2 < FOC_MAX_FINGERS) {
+ state->fingers[finger2].x += (char)packet[4];
+ state->fingers[finger2].y += (char)packet[5];
+ }
+}
+
+static void focaltech_process_packet(struct psmouse *psmouse)
+{
+ struct focaltech_data *priv = psmouse->private;
+ unsigned char *packet = psmouse->packet;
+
+ switch (packet[0] & 0xf) {
+ case FOC_TOUCH:
+ process_touch_packet(&priv->state, packet);
+ break;
+ case FOC_ABS:
+ process_abs_packet(psmouse, packet);
+ break;
+ case FOC_REL:
+ process_rel_packet(psmouse, packet);
+ break;
+ default:
+ psmouse_err(psmouse, "Unknown packet type: %02x", packet[0]);
+ break;
+ }
+
+ focaltech_report_state(psmouse);
+}
+
+static psmouse_ret_t focaltech_process_byte(struct psmouse *psmouse)
+{
+ if (psmouse->pktcnt >= 6) { /* Full packet received */
+ focaltech_process_packet(psmouse);
+ return PSMOUSE_FULL_PACKET;
+ }
+ /*
+ * we might want to do some validation of the data here, but we do not
+ * know the protocoll well enough
+ */
+ return PSMOUSE_GOOD_DATA;
+}
+
+static int focaltech_switch_protocol(struct psmouse *psmouse)
+{
+ struct ps2dev *ps2dev = &psmouse->ps2dev;
+ unsigned char param[3];
+
+ param[0] = 0;
+ if (ps2_command(ps2dev, param, 0x10f8))
+ return -EIO;
+ if (ps2_command(ps2dev, param, 0x10f8))
+ return -EIO;
+ if (ps2_command(ps2dev, param, 0x10f8))
+ return -EIO;
+ param[0] = 1;
+ if (ps2_command(ps2dev, param, 0x10f8))
+ return -EIO;
+ if (ps2_command(ps2dev, param, PSMOUSE_CMD_SETSCALE11))
+ return -EIO;
+
+ if (ps2_command(ps2dev, param, PSMOUSE_CMD_ENABLE))
+ return -EIO;
+
+ return 0;
+}
+
+static void focaltech_disconnect(struct psmouse *psmouse)
+{
+ focaltech_reset(psmouse);
+ kfree(psmouse->private);
+ psmouse->private = NULL;
+}
+
+static int focaltech_reconnect(struct psmouse *psmouse)
+{
+ focaltech_reset(psmouse);
+ if (focaltech_switch_protocol(psmouse)) {
+ psmouse_err(psmouse,
+ "Unable to initialize the device.");
+ return -1;
+ }
+ return 0;
+}
+
+static void set_input_params(struct psmouse *psmouse)
+{
+ struct input_dev *dev = psmouse->dev;
+ struct focaltech_data *priv = psmouse->private;
+
+ __set_bit(EV_ABS, dev->evbit);
+ input_set_abs_params(dev, ABS_MT_POSITION_X, 0, priv->x_max, 0, 0);
+ input_set_abs_params(dev, ABS_MT_POSITION_Y, 0, priv->y_max, 0, 0);
+ input_mt_init_slots(dev, 5, INPUT_MT_POINTER);
+ __clear_bit(EV_REL, dev->evbit);
+ __clear_bit(REL_X, dev->relbit);
+ __clear_bit(REL_Y, dev->relbit);
+ __clear_bit(BTN_RIGHT, dev->keybit);
+ __clear_bit(BTN_MIDDLE, dev->keybit);
+ __set_bit(INPUT_PROP_BUTTONPAD, dev->propbit);
+}
+
+static int focaltech_read_register(struct ps2dev *ps2dev, int reg,
+ unsigned char *param)
+{
+ if (ps2_command(ps2dev, param, PSMOUSE_CMD_SETSCALE11))
+ return -EIO;
+ param[0] = 0;
+ if (ps2_command(ps2dev, param, PSMOUSE_CMD_SETRES))
+ return -EIO;
+ if (ps2_command(ps2dev, param, PSMOUSE_CMD_SETRES))
+ return -EIO;
+ if (ps2_command(ps2dev, param, PSMOUSE_CMD_SETRES))
+ return -EIO;
+ param[0] = reg;
+ if (ps2_command(ps2dev, param, PSMOUSE_CMD_SETRES))
+ return -EIO;
+ if (ps2_command(ps2dev, param, PSMOUSE_CMD_GETINFO))
+ return -EIO;
+ return 0;
+}
+
+static int focaltech_read_size(struct psmouse *psmouse)
+{
+ struct ps2dev *ps2dev = &psmouse->ps2dev;
+ struct focaltech_data *priv = psmouse->private;
+ char param[3];
+
+ if (focaltech_read_register(ps2dev, 2, param))
+ return -EIO;
+ /* not sure whether this is 100% correct */
+ priv->x_max = (unsigned char)param[1] * 128;
+ priv->y_max = (unsigned char)param[2] * 128;
+
+ return 0;
+}
+int focaltech_init(struct psmouse *psmouse)
+{
+ struct focaltech_data *priv;
+ int err;
+
+ psmouse->private = priv = kzalloc(sizeof(struct focaltech_data), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ focaltech_reset(psmouse);
+ err = focaltech_read_size(psmouse);
+ if (err) {
+ focaltech_reset(psmouse);
+ psmouse_err(psmouse,
+ "Unable to read the size of the touchpad.");
+ goto fail;
+ }
+ if (focaltech_switch_protocol(psmouse)) {
+ focaltech_reset(psmouse);
+ psmouse_err(psmouse,
+ "Unable to initialize the device.");
+ err = -ENOSYS;
+ goto fail;
+ }
+
+ set_input_params(psmouse);
+
+ psmouse->protocol_handler = focaltech_process_byte;
+ psmouse->pktsize = 6;
+ psmouse->disconnect = focaltech_disconnect;
+ psmouse->reconnect = focaltech_reconnect;
+ psmouse->cleanup = focaltech_reset;
+ /* resync is not supported yet */
+ psmouse->resync_time = 0;
return 0;
+fail:
+ focaltech_reset(psmouse);
+ kfree(priv);
+ return err;
+}
+
+bool focaltech_supported(void)
+{
+ return true;
}
+
+#else /* CONFIG_MOUSE_PS2_FOCALTECH */
+
+int focaltech_init(struct psmouse *psmouse)
+{
+ focaltech_reset(psmouse);
+
+ return 0;
+}
+
+bool focaltech_supported(void)
+{
+ return false;
+}
+
+#endif /* CONFIG_MOUSE_PS2_FOCALTECH */
diff --git a/drivers/input/mouse/focaltech.h b/drivers/input/mouse/focaltech.h
index 498650c..bda3241 100644
--- a/drivers/input/mouse/focaltech.h
+++ b/drivers/input/mouse/focaltech.h
@@ -2,6 +2,7 @@
* Focaltech TouchPad PS/2 mouse driver
*
* Copyright (c) 2014 Red Hat Inc.
+ * Copyright (c) 2014 Mathias Gottschlag <mgottschlag@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
@@ -16,7 +17,66 @@
#ifndef _FOCALTECH_H
#define _FOCALTECH_H
+/*
+ * Packet types - the numbers are not consecutive, so we might be missing
+ * something here.
+ */
+#define FOC_TOUCH 0x3 /* bitmap of active fingers */
+#define FOC_ABS 0x6 /* absolute position of one finger */
+#define FOC_REL 0x9 /* relative position of 1-2 fingers */
+
+#define FOC_MAX_FINGERS 5
+
+#define FOC_MAX_X 2431
+#define FOC_MAX_Y 1663
+
+static inline int focaltech_invert_y(int y)
+{
+ return FOC_MAX_Y - y;
+}
+
+/*
+ * Current state of a single finger on the touchpad.
+ */
+struct focaltech_finger_state {
+ /* the touchpad has generated a touch event for the finger */
+ bool active;
+ /*
+ * The touchpad has sent position data for the finger. The flag is 0
+ * when the finger is not active, and there is a time between the first
+ * touch event for the finger and the following absolute position
+ * packet for the finger where the touchpad has declared the finger to
+ * be valid, but we do not have any valid position yet.
+ */
+ bool valid;
+ /* absolute position (from the bottom left corner) of the finger */
+ unsigned int x;
+ unsigned int y;
+};
+
+/*
+ * Description of the current state of the touchpad hardware.
+ */
+struct focaltech_hw_state {
+ /*
+ * The touchpad tracks the positions of the fingers for us, the array
+ * indices correspond to the finger indices returned in the report
+ * packages.
+ */
+ struct focaltech_finger_state fingers[FOC_MAX_FINGERS];
+ /*
+ * True if the clickpad has been pressed.
+ */
+ bool pressed;
+};
+
+struct focaltech_data {
+ unsigned int x_max, y_max;
+ struct focaltech_hw_state state;
+};
+
int focaltech_detect(struct psmouse *psmouse, bool set_properties);
int focaltech_init(struct psmouse *psmouse);
+bool focaltech_supported(void);
#endif
diff --git a/drivers/input/mouse/psmouse-base.c b/drivers/input/mouse/psmouse-base.c
index 95a3a6e..68469fe 100644
--- a/drivers/input/mouse/psmouse-base.c
+++ b/drivers/input/mouse/psmouse-base.c
@@ -725,16 +725,19 @@ static int psmouse_extensions(struct psmouse *psmouse,
/* Always check for focaltech, this is safe as it uses pnp-id matching */
if (psmouse_do_detect(focaltech_detect, psmouse, set_properties) == 0) {
- if (!set_properties || focaltech_init(psmouse) == 0) {
- /*
- * Not supported yet, use bare protocol.
- * Note that we need to also restrict
- * psmouse_max_proto so that psmouse_initialize()
- * does not try to reset rate and resolution,
- * because even that upsets the device.
- */
- psmouse_max_proto = PSMOUSE_PS2;
- return PSMOUSE_PS2;
+ if (max_proto > PSMOUSE_IMEX) {
+ if (!set_properties || focaltech_init(psmouse) == 0) {
+ if (focaltech_supported())
+ return PSMOUSE_FOCALTECH;
+ /*
+ * Note that we need to also restrict
+ * psmouse_max_proto so that psmouse_initialize()
+ * does not try to reset rate and resolution,
+ * because even that upsets the device.
+ */
+ psmouse_max_proto = PSMOUSE_PS2;
+ return PSMOUSE_PS2;
+ }
}
}
@@ -1063,6 +1066,15 @@ static const struct psmouse_protocol psmouse_protocols[] = {
.alias = "cortps",
.detect = cortron_detect,
},
+#ifdef CONFIG_MOUSE_PS2_FOCALTECH
+ {
+ .type = PSMOUSE_FOCALTECH,
+ .name = "FocalTechPS/2",
+ .alias = "focaltech",
+ .detect = focaltech_detect,
+ .init = focaltech_init,
+ },
+#endif
{
.type = PSMOUSE_AUTO,
.name = "auto",
diff --git a/drivers/input/mouse/psmouse.h b/drivers/input/mouse/psmouse.h
index f4cf664..c2ff137 100644
--- a/drivers/input/mouse/psmouse.h
+++ b/drivers/input/mouse/psmouse.h
@@ -96,6 +96,7 @@ enum psmouse_type {
PSMOUSE_FSP,
PSMOUSE_SYNAPTICS_RELATIVE,
PSMOUSE_CYPRESS,
+ PSMOUSE_FOCALTECH,
PSMOUSE_AUTO /* This one should always be last */
};
--
1.9.1
^ permalink raw reply related
* [PATCH] psmouse: Add some support for the FocalTech PS/2 protocol extensions.
From: Mathias Gottschlag @ 2014-11-12 22:56 UTC (permalink / raw)
To: Dmitry Torokhov, Hans de Goede; +Cc: linux-input, Mathias Gottschlag
Most of the protocol for these touchpads has been reverse engineered. This
commit adds a basic multitouch-capable driver.
A lot of the protocol is still unknown. Especially, we don't know how to
identify the device yet apart from the PNP ID.
The previous workaround for these devices has been left in place in case
the driver is not compiled into the kernel or in case some other device
with the same PNP ID is not recognized by the driver yet still has the same
problems with the device probing code.
Signed-off-by: Mathias Gottschlag <mgottschlag@gmail.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
---
Various smaller changes as requested. Also, accidentally pressing the touchpad
disable button can cause really frustrating debugging sessions.
drivers/input/mouse/Kconfig | 10 ++
drivers/input/mouse/focaltech.c | 313 ++++++++++++++++++++++++++++++++++++-
drivers/input/mouse/focaltech.h | 60 +++++++
drivers/input/mouse/psmouse-base.c | 32 ++--
drivers/input/mouse/psmouse.h | 1 +
5 files changed, 399 insertions(+), 17 deletions(-)
diff --git a/drivers/input/mouse/Kconfig b/drivers/input/mouse/Kconfig
index 366fc7a..db973e5 100644
--- a/drivers/input/mouse/Kconfig
+++ b/drivers/input/mouse/Kconfig
@@ -146,6 +146,16 @@ config MOUSE_PS2_OLPC
If unsure, say N.
+config MOUSE_PS2_FOCALTECH
+ bool "FocalTech PS/2 mouse protocol extension" if EXPERT
+ default y
+ depends on MOUSE_PS2
+ help
+ Say Y here if you have a FocalTech PS/2 TouchPad connected to
+ your system.
+
+ If unsure, say Y.
+
config MOUSE_SERIAL
tristate "Serial mouse"
select SERIO
diff --git a/drivers/input/mouse/focaltech.c b/drivers/input/mouse/focaltech.c
index f4d657e..f838cab4 100644
--- a/drivers/input/mouse/focaltech.c
+++ b/drivers/input/mouse/focaltech.c
@@ -2,6 +2,7 @@
* Focaltech TouchPad PS/2 mouse driver
*
* Copyright (c) 2014 Red Hat Inc.
+ * Copyright (c) 2014 Mathias Gottschlag <mgottschlag@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
@@ -13,15 +14,14 @@
* Hans de Goede <hdegoede@redhat.com>
*/
-/*
- * The Focaltech PS/2 touchpad protocol is unknown. This drivers deals with
- * detection only, to avoid further detection attempts confusing the touchpad
- * this way it at least works in PS/2 mouse compatibility mode.
- */
#include <linux/device.h>
#include <linux/libps2.h>
+#include <linux/input/mt.h>
+#include <linux/serio.h>
+#include <linux/slab.h>
#include "psmouse.h"
+#include "focaltech.h"
static const char * const focaltech_pnp_ids[] = {
"FLT0101",
@@ -30,6 +30,12 @@ static const char * const focaltech_pnp_ids[] = {
NULL
};
+/*
+ * Even if the kernel is built without support for Focaltech PS/2 touchpads (or
+ * when the real driver fails to recognize the device), we still have to detect
+ * them in order to avoid further detection attempts confusing the touchpad.
+ * This way it at least works in PS/2 mouse compatibility mode.
+ */
int focaltech_detect(struct psmouse *psmouse, bool set_properties)
{
if (!psmouse_matches_pnp_id(psmouse, focaltech_pnp_ids))
@@ -37,16 +43,309 @@ int focaltech_detect(struct psmouse *psmouse, bool set_properties)
if (set_properties) {
psmouse->vendor = "FocalTech";
- psmouse->name = "FocalTech Touchpad in mouse emulation mode";
+ psmouse->name = "FocalTech Touchpad";
}
return 0;
}
-int focaltech_init(struct psmouse *psmouse)
+static void focaltech_reset(struct psmouse *psmouse)
{
ps2_command(&psmouse->ps2dev, NULL, PSMOUSE_CMD_RESET_DIS);
psmouse_reset(psmouse);
+}
+
+#ifdef CONFIG_MOUSE_PS2_FOCALTECH
+
+static void focaltech_report_state(struct psmouse *psmouse)
+{
+ int i;
+ struct focaltech_data *priv = psmouse->private;
+ struct focaltech_hw_state *state = &priv->state;
+ struct input_dev *dev = psmouse->dev;
+
+ for (i = 0; i < FOC_MAX_FINGERS; i++) {
+ struct focaltech_finger_state *finger = &state->fingers[i];
+ bool active = finger->active && finger->valid;
+ input_mt_slot(dev, i);
+ input_mt_report_slot_state(dev, MT_TOOL_FINGER, active);
+ if (active) {
+ input_report_abs(dev, ABS_MT_POSITION_X, finger->x);
+ input_report_abs(dev, ABS_MT_POSITION_Y,
+ focaltech_invert_y(finger->y));
+ }
+ }
+ input_mt_report_pointer_emulation(dev, true);
+
+ input_report_key(psmouse->dev, BTN_LEFT, state->pressed);
+ input_sync(psmouse->dev);
+}
+
+static void process_touch_packet(struct focaltech_hw_state *state,
+ unsigned char *packet)
+{
+ int i;
+ unsigned char fingers = packet[1];
+
+ state->pressed = (packet[0] >> 4) & 1;
+ /* the second byte contains a bitmap of all fingers touching the pad */
+ for (i = 0; i < FOC_MAX_FINGERS; i++) {
+ state->fingers[i].active = fingers & 0x1;
+ if (!state->fingers[i].active) {
+ /* even when the finger becomes active again, we still
+ * will have to wait for the first valid position */
+ state->fingers[i].valid = false;
+ }
+ fingers >>= 1;
+ }
+}
+
+static void process_abs_packet(struct psmouse *psmouse,
+ unsigned char *packet)
+{
+ struct focaltech_data *priv = psmouse->private;
+ struct focaltech_hw_state *state = &priv->state;
+ unsigned int finger;
+
+ finger = (packet[1] >> 4) - 1;
+ if (finger >= FOC_MAX_FINGERS) {
+ psmouse_err(psmouse, "Invalid finger in abs packet: %d",
+ finger);
+ return;
+ }
+
+ state->pressed = (packet[0] >> 4) & 1;
+ /*
+ * packet[5] contains some kind of tool size in the most significant
+ * nibble. 0xff is a special value (latching) that signals a large
+ * contact area.
+ */
+ if (packet[5] == 0xff) {
+ state->fingers[finger].valid = false;
+ return;
+ }
+ state->fingers[finger].x = ((packet[1] & 0xf) << 8) | packet[2];
+ state->fingers[finger].y = (packet[3] << 8) | packet[4];
+ state->fingers[finger].valid = true;
+}
+
+static void process_rel_packet(struct psmouse *psmouse,
+ unsigned char *packet)
+{
+ struct focaltech_data *priv = psmouse->private;
+ struct focaltech_hw_state *state = &priv->state;
+ int finger1, finger2;
+
+ state->pressed = packet[0] >> 7;
+ finger1 = ((packet[0] >> 4) & 0x7) - 1;
+ if (finger1 < FOC_MAX_FINGERS) {
+ state->fingers[finger1].x += (char)packet[1];
+ state->fingers[finger1].y += (char)packet[2];
+ } else {
+ psmouse_err(psmouse, "First finger in rel packet invalid: %d",
+ finger1);
+ }
+ /*
+ * If there is an odd number of fingers, the last relative packet only
+ * contains one finger. In this case, the second finger index in the
+ * packet is 0 (we subtract 1 in the lines above to create array
+ * indices, so the finger will overflow and be above FOC_MAX_FINGERS).
+ */
+ finger2 = ((packet[3] >> 4) & 0x7) - 1;
+ if (finger2 < FOC_MAX_FINGERS) {
+ state->fingers[finger2].x += (char)packet[4];
+ state->fingers[finger2].y += (char)packet[5];
+ }
+}
+
+static void focaltech_process_packet(struct psmouse *psmouse)
+{
+ struct focaltech_data *priv = psmouse->private;
+ unsigned char *packet = psmouse->packet;
+
+ switch (packet[0] & 0xf) {
+ case FOC_TOUCH:
+ process_touch_packet(&priv->state, packet);
+ break;
+ case FOC_ABS:
+ process_abs_packet(psmouse, packet);
+ break;
+ case FOC_REL:
+ process_rel_packet(psmouse, packet);
+ break;
+ default:
+ psmouse_err(psmouse, "Unknown packet type: %02x", packet[0]);
+ break;
+ }
+
+ focaltech_report_state(psmouse);
+}
+
+static psmouse_ret_t focaltech_process_byte(struct psmouse *psmouse)
+{
+ if (psmouse->pktcnt >= 6) { /* Full packet received */
+ focaltech_process_packet(psmouse);
+ return PSMOUSE_FULL_PACKET;
+ }
+ /*
+ * we might want to do some validation of the data here, but we do not
+ * know the protocoll well enough
+ */
+ return PSMOUSE_GOOD_DATA;
+}
+
+static int focaltech_switch_protocol(struct psmouse *psmouse)
+{
+ struct ps2dev *ps2dev = &psmouse->ps2dev;
+ unsigned char param[3];
+
+ param[0] = 0;
+ if (ps2_command(ps2dev, param, 0x10f8))
+ return -EIO;
+ if (ps2_command(ps2dev, param, 0x10f8))
+ return -EIO;
+ if (ps2_command(ps2dev, param, 0x10f8))
+ return -EIO;
+ param[0] = 1;
+ if (ps2_command(ps2dev, param, 0x10f8))
+ return -EIO;
+ if (ps2_command(ps2dev, param, PSMOUSE_CMD_SETSCALE11))
+ return -EIO;
+
+ if (ps2_command(ps2dev, param, PSMOUSE_CMD_ENABLE))
+ return -EIO;
+
+ return 0;
+}
+
+static void focaltech_disconnect(struct psmouse *psmouse)
+{
+ focaltech_reset(psmouse);
+ kfree(psmouse->private);
+ psmouse->private = NULL;
+}
+
+static int focaltech_reconnect(struct psmouse *psmouse)
+{
+ focaltech_reset(psmouse);
+ if (focaltech_switch_protocol(psmouse)) {
+ psmouse_err(psmouse,
+ "Unable to initialize the device.");
+ return -1;
+ }
+ return 0;
+}
+
+static void set_input_params(struct psmouse *psmouse)
+{
+ struct input_dev *dev = psmouse->dev;
+ struct focaltech_data *priv = psmouse->private;
+
+ __set_bit(EV_ABS, dev->evbit);
+ input_set_abs_params(dev, ABS_MT_POSITION_X, 0, priv->x_max, 0, 0);
+ input_set_abs_params(dev, ABS_MT_POSITION_Y, 0, priv->y_max, 0, 0);
+ input_mt_init_slots(dev, 5, INPUT_MT_POINTER);
+ __clear_bit(EV_REL, dev->evbit);
+ __clear_bit(REL_X, dev->relbit);
+ __clear_bit(REL_Y, dev->relbit);
+ __clear_bit(BTN_RIGHT, dev->keybit);
+ __clear_bit(BTN_MIDDLE, dev->keybit);
+ __set_bit(INPUT_PROP_BUTTONPAD, dev->propbit);
+}
+
+static int focaltech_read_register(struct ps2dev *ps2dev, int reg,
+ unsigned char *param)
+{
+ if (ps2_command(ps2dev, param, PSMOUSE_CMD_SETSCALE11))
+ return -EIO;
+ param[0] = 0;
+ if (ps2_command(ps2dev, param, PSMOUSE_CMD_SETRES))
+ return -EIO;
+ if (ps2_command(ps2dev, param, PSMOUSE_CMD_SETRES))
+ return -EIO;
+ if (ps2_command(ps2dev, param, PSMOUSE_CMD_SETRES))
+ return -EIO;
+ param[0] = reg;
+ if (ps2_command(ps2dev, param, PSMOUSE_CMD_SETRES))
+ return -EIO;
+ if (ps2_command(ps2dev, param, PSMOUSE_CMD_GETINFO))
+ return -EIO;
+ return 0;
+}
+
+static int focaltech_read_size(struct psmouse *psmouse)
+{
+ struct ps2dev *ps2dev = &psmouse->ps2dev;
+ struct focaltech_data *priv = psmouse->private;
+ char param[3];
+
+ if (focaltech_read_register(ps2dev, 2, param))
+ return -EIO;
+ /* not sure whether this is 100% correct */
+ priv->x_max = (unsigned char)param[1] * 128;
+ priv->y_max = (unsigned char)param[2] * 128;
+
+ return 0;
+}
+int focaltech_init(struct psmouse *psmouse)
+{
+ struct focaltech_data *priv;
+ int err;
+
+ psmouse->private = priv = kzalloc(sizeof(struct focaltech_data), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ focaltech_reset(psmouse);
+ err = focaltech_read_size(psmouse);
+ if (err) {
+ focaltech_reset(psmouse);
+ psmouse_err(psmouse,
+ "Unable to read the size of the touchpad.");
+ goto fail;
+ }
+ if (focaltech_switch_protocol(psmouse)) {
+ focaltech_reset(psmouse);
+ psmouse_err(psmouse,
+ "Unable to initialize the device.");
+ err = -ENOSYS;
+ goto fail;
+ }
+
+ set_input_params(psmouse);
+
+ psmouse->protocol_handler = focaltech_process_byte;
+ psmouse->pktsize = 6;
+ psmouse->disconnect = focaltech_disconnect;
+ psmouse->reconnect = focaltech_reconnect;
+ psmouse->cleanup = focaltech_reset;
+ /* resync is not supported yet */
+ psmouse->resync_time = 0;
return 0;
+fail:
+ focaltech_reset(psmouse);
+ kfree(priv);
+ return err;
+}
+
+bool focaltech_supported(void)
+{
+ return true;
}
+
+#else /* CONFIG_MOUSE_PS2_FOCALTECH */
+
+int focaltech_init(struct psmouse *psmouse)
+{
+ focaltech_reset(psmouse);
+
+ return 0;
+}
+
+bool focaltech_supported(void)
+{
+ return false;
+}
+
+#endif /* CONFIG_MOUSE_PS2_FOCALTECH */
diff --git a/drivers/input/mouse/focaltech.h b/drivers/input/mouse/focaltech.h
index 498650c..bda3241 100644
--- a/drivers/input/mouse/focaltech.h
+++ b/drivers/input/mouse/focaltech.h
@@ -2,6 +2,7 @@
* Focaltech TouchPad PS/2 mouse driver
*
* Copyright (c) 2014 Red Hat Inc.
+ * Copyright (c) 2014 Mathias Gottschlag <mgottschlag@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
@@ -16,7 +17,66 @@
#ifndef _FOCALTECH_H
#define _FOCALTECH_H
+/*
+ * Packet types - the numbers are not consecutive, so we might be missing
+ * something here.
+ */
+#define FOC_TOUCH 0x3 /* bitmap of active fingers */
+#define FOC_ABS 0x6 /* absolute position of one finger */
+#define FOC_REL 0x9 /* relative position of 1-2 fingers */
+
+#define FOC_MAX_FINGERS 5
+
+#define FOC_MAX_X 2431
+#define FOC_MAX_Y 1663
+
+static inline int focaltech_invert_y(int y)
+{
+ return FOC_MAX_Y - y;
+}
+
+/*
+ * Current state of a single finger on the touchpad.
+ */
+struct focaltech_finger_state {
+ /* the touchpad has generated a touch event for the finger */
+ bool active;
+ /*
+ * The touchpad has sent position data for the finger. The flag is 0
+ * when the finger is not active, and there is a time between the first
+ * touch event for the finger and the following absolute position
+ * packet for the finger where the touchpad has declared the finger to
+ * be valid, but we do not have any valid position yet.
+ */
+ bool valid;
+ /* absolute position (from the bottom left corner) of the finger */
+ unsigned int x;
+ unsigned int y;
+};
+
+/*
+ * Description of the current state of the touchpad hardware.
+ */
+struct focaltech_hw_state {
+ /*
+ * The touchpad tracks the positions of the fingers for us, the array
+ * indices correspond to the finger indices returned in the report
+ * packages.
+ */
+ struct focaltech_finger_state fingers[FOC_MAX_FINGERS];
+ /*
+ * True if the clickpad has been pressed.
+ */
+ bool pressed;
+};
+
+struct focaltech_data {
+ unsigned int x_max, y_max;
+ struct focaltech_hw_state state;
+};
+
int focaltech_detect(struct psmouse *psmouse, bool set_properties);
int focaltech_init(struct psmouse *psmouse);
+bool focaltech_supported(void);
#endif
diff --git a/drivers/input/mouse/psmouse-base.c b/drivers/input/mouse/psmouse-base.c
index 95a3a6e..68469fe 100644
--- a/drivers/input/mouse/psmouse-base.c
+++ b/drivers/input/mouse/psmouse-base.c
@@ -725,16 +725,19 @@ static int psmouse_extensions(struct psmouse *psmouse,
/* Always check for focaltech, this is safe as it uses pnp-id matching */
if (psmouse_do_detect(focaltech_detect, psmouse, set_properties) == 0) {
- if (!set_properties || focaltech_init(psmouse) == 0) {
- /*
- * Not supported yet, use bare protocol.
- * Note that we need to also restrict
- * psmouse_max_proto so that psmouse_initialize()
- * does not try to reset rate and resolution,
- * because even that upsets the device.
- */
- psmouse_max_proto = PSMOUSE_PS2;
- return PSMOUSE_PS2;
+ if (max_proto > PSMOUSE_IMEX) {
+ if (!set_properties || focaltech_init(psmouse) == 0) {
+ if (focaltech_supported())
+ return PSMOUSE_FOCALTECH;
+ /*
+ * Note that we need to also restrict
+ * psmouse_max_proto so that psmouse_initialize()
+ * does not try to reset rate and resolution,
+ * because even that upsets the device.
+ */
+ psmouse_max_proto = PSMOUSE_PS2;
+ return PSMOUSE_PS2;
+ }
}
}
@@ -1063,6 +1066,15 @@ static const struct psmouse_protocol psmouse_protocols[] = {
.alias = "cortps",
.detect = cortron_detect,
},
+#ifdef CONFIG_MOUSE_PS2_FOCALTECH
+ {
+ .type = PSMOUSE_FOCALTECH,
+ .name = "FocalTechPS/2",
+ .alias = "focaltech",
+ .detect = focaltech_detect,
+ .init = focaltech_init,
+ },
+#endif
{
.type = PSMOUSE_AUTO,
.name = "auto",
diff --git a/drivers/input/mouse/psmouse.h b/drivers/input/mouse/psmouse.h
index f4cf664..c2ff137 100644
--- a/drivers/input/mouse/psmouse.h
+++ b/drivers/input/mouse/psmouse.h
@@ -96,6 +96,7 @@ enum psmouse_type {
PSMOUSE_FSP,
PSMOUSE_SYNAPTICS_RELATIVE,
PSMOUSE_CYPRESS,
+ PSMOUSE_FOCALTECH,
PSMOUSE_AUTO /* This one should always be last */
};
--
1.9.1
^ permalink raw reply related
* [PATCH] hid: sony: Add support for the third-party SMK PS3 Bluetooth Remote
From: Frank Praznik @ 2014-11-12 19:52 UTC (permalink / raw)
To: linux-input; +Cc: jkosina, Frank Praznik
Add vid/pid for the SMK branded third-party PS3 Bluetooth remote and enable
support in the hid-sony driver.
Signed-off-by: Frank Praznik <frank.praznik@oh.rr.com>
---
drivers/hid/hid-core.c | 1 +
drivers/hid/hid-ids.h | 3 +++
drivers/hid/hid-sony.c | 3 +++
3 files changed, 7 insertions(+)
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 73bd9e2..c8b8691 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1913,6 +1913,7 @@ static const struct hid_device_id hid_have_special_driver[] = {
{ HID_USB_DEVICE(USB_VENDOR_ID_SAMSUNG, USB_DEVICE_ID_SAMSUNG_IR_REMOTE) },
{ HID_USB_DEVICE(USB_VENDOR_ID_SAMSUNG, USB_DEVICE_ID_SAMSUNG_WIRELESS_KBD_MOUSE) },
{ HID_USB_DEVICE(USB_VENDOR_ID_SKYCABLE, USB_DEVICE_ID_SKYCABLE_WIRELESS_PRESENTER) },
+ { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SMK, USB_DEVICE_ID_SMK_PS3_BDREMOTE) },
{ HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_BUZZ_CONTROLLER) },
{ HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_WIRELESS_BUZZ_CONTROLLER) },
{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS3_BDREMOTE) },
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index e23ab8b..b2c8a6a 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -812,6 +812,9 @@
#define USB_VENDOR_ID_SKYCABLE 0x1223
#define USB_DEVICE_ID_SKYCABLE_WIRELESS_PRESENTER 0x3F07
+#define USB_VENDOR_ID_SMK 0x0609
+#define USB_DEVICE_ID_SMK_PS3_BDREMOTE 0x0306
+
#define USB_VENDOR_ID_SONY 0x054c
#define USB_DEVICE_ID_SONY_VAIO_VGX_MOUSE 0x024b
#define USB_DEVICE_ID_SONY_VAIO_VGP_MOUSE 0x0374
diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index b6e6102..31e9d25 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -2113,6 +2113,9 @@ static const struct hid_device_id sony_devices[] = {
/* Logitech Harmony Adapter for PS3 */
{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_HARMONY_PS3),
.driver_data = PS3REMOTE },
+ /* SMK-Link PS3 BD Remote Control */
+ { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SMK, USB_DEVICE_ID_SMK_PS3_BDREMOTE),
+ .driver_data = PS3REMOTE },
/* Sony Dualshock 4 controllers for PS4 */
{ HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS4_CONTROLLER),
.driver_data = DUALSHOCK4_CONTROLLER_USB },
--
2.1.0
^ permalink raw reply related
* Re: [PATCH v2 1/1] Input: gpio_keys - add device tree support for interrupt only keys
From: Arnd Bergmann @ 2014-11-12 19:16 UTC (permalink / raw)
To: Alexander Stein
Cc: Dmitry Torokhov, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, linux-input, devicetree
In-Reply-To: <8280855.M123yTnXfy@ws-stein>
On Wednesday 12 November 2014 17:38:31 Alexander Stein wrote:
> On Wednesday 12 November 2014 17:04:57, Arnd Bergmann wrote:
> > On Wednesday 12 November 2014 17:02:56 Alexander Stein wrote:
> > > This features already exists for board config setups. Add support for
> > > device tree based systems.
> > >
> > > Signed-off-by: Alexander Stein <alexander.stein@systec-electronic.com>
> > > ---
> > > Please note: Due to current lack of hardware I could not test it yet. V2
> > > includes the changes proposed by Dmitry.
> > >
> > > Changes in v2:
> > > * Added device tree bindings
> > > * IRQ is only parsed and mapped when there is no "gpios" property
> > >
> >
> > Can you list one or more examples in the patch description?
> > Are these systems that don't expose the GPIO controller with a
> > standalone driver, or systems that really actually connect the
> > buttons to an interrupt pin?
>
> You mean a use case? I came to this situation to test interrupt polarity on a microcontroller, thus a simple /IRQ pin, no GPIO at all. So in the end I have an input just connected to an interrupt line.
> I noticed gpio_keys using platform data only can be used for this setup. So I added this support for device tree.
>
I meant a specific board file that uses this, which can't be converted
to DT without your change.
Arnd
^ permalink raw reply
* [PATCH v3] hid: sony: Use kernel allocated buffers for HID reports
From: Frank Praznik @ 2014-11-12 19:10 UTC (permalink / raw)
To: linux-input; +Cc: jkosina, dmitry.torokhov, ao2, Frank Praznik
Replace stack buffers with kernel allocated buffers for sending
and receiving HID reports to prevent issues with DMA transfers
on certain hardware.
Output report buffers are allocated at initialization time to avoid
excessive calls to kmalloc and kfree.
Signed-off-by: Frank Praznik <frank.praznik@oh.rr.com>
---
The original reporter confirms that this fixes the bug reported in
https://bugzilla.kernel.org/show_bug.cgi?id=87991
v2 fixes a sizeof(pointer) mistake and corrects some cosmetic issues
(spacing and #defines instead of magic constants).
v3 uses hex notation for the report size definitions instead of decimal for
consistency since hex notation is used for the report numbers in the rest of
the driver.
drivers/hid/hid-sony.c | 147 +++++++++++++++++++++++++++++++++++++------------
1 file changed, 113 insertions(+), 34 deletions(-)
diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index bc4269e..b6e6102 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -798,6 +798,12 @@ union sixaxis_output_report_01 {
__u8 buf[36];
};
+#define DS4_REPORT_0x02_SIZE 37
+#define DS4_REPORT_0x05_SIZE 32
+#define DS4_REPORT_0x11_SIZE 78
+#define DS4_REPORT_0x81_SIZE 7
+#define SIXAXIS_REPORT_0xF2_SIZE 18
+
static spinlock_t sony_dev_list_lock;
static LIST_HEAD(sony_device_list);
static DEFINE_IDA(sony_device_id_allocator);
@@ -811,6 +817,7 @@ struct sony_sc {
struct work_struct state_worker;
struct power_supply battery;
int device_id;
+ __u8 *output_report_dmabuf;
#ifdef CONFIG_SONY_FF
__u8 left;
@@ -1142,9 +1149,20 @@ static int sixaxis_set_operational_usb(struct hid_device *hdev)
static int sixaxis_set_operational_bt(struct hid_device *hdev)
{
- unsigned char buf[] = { 0xf4, 0x42, 0x03, 0x00, 0x00 };
- return hid_hw_raw_request(hdev, buf[0], buf, sizeof(buf),
+ static const __u8 report[] = { 0xf4, 0x42, 0x03, 0x00, 0x00 };
+ __u8 *buf;
+ int ret;
+
+ buf = kmemdup(report, sizeof(report), GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ ret = hid_hw_raw_request(hdev, buf[0], buf, sizeof(report),
HID_FEATURE_REPORT, HID_REQ_SET_REPORT);
+
+ kfree(buf);
+
+ return ret;
}
/*
@@ -1153,10 +1171,19 @@ static int sixaxis_set_operational_bt(struct hid_device *hdev)
*/
static int dualshock4_set_operational_bt(struct hid_device *hdev)
{
- __u8 buf[37] = { 0 };
+ __u8 *buf;
+ int ret;
- return hid_hw_raw_request(hdev, 0x02, buf, sizeof(buf),
+ buf = kmalloc(DS4_REPORT_0x02_SIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ ret = hid_hw_raw_request(hdev, 0x02, buf, DS4_REPORT_0x02_SIZE,
HID_FEATURE_REPORT, HID_REQ_GET_REPORT);
+
+ kfree(buf);
+
+ return ret;
}
static void sixaxis_set_leds_from_id(int id, __u8 values[MAX_LEDS])
@@ -1471,9 +1498,7 @@ error_leds:
static void sixaxis_state_worker(struct work_struct *work)
{
- struct sony_sc *sc = container_of(work, struct sony_sc, state_worker);
- int n;
- union sixaxis_output_report_01 report = {
+ static const union sixaxis_output_report_01 default_report = {
.buf = {
0x01,
0x00, 0xff, 0x00, 0xff, 0x00,
@@ -1485,20 +1510,27 @@ static void sixaxis_state_worker(struct work_struct *work)
0x00, 0x00, 0x00, 0x00, 0x00
}
};
+ struct sony_sc *sc = container_of(work, struct sony_sc, state_worker);
+ struct sixaxis_output_report *report =
+ (struct sixaxis_output_report *)sc->output_report_dmabuf;
+ int n;
+
+ /* Initialize the report with default values */
+ memcpy(report, &default_report, sizeof(struct sixaxis_output_report));
#ifdef CONFIG_SONY_FF
- report.data.rumble.right_motor_on = sc->right ? 1 : 0;
- report.data.rumble.left_motor_force = sc->left;
+ report->rumble.right_motor_on = sc->right ? 1 : 0;
+ report->rumble.left_motor_force = sc->left;
#endif
- report.data.leds_bitmap |= sc->led_state[0] << 1;
- report.data.leds_bitmap |= sc->led_state[1] << 2;
- report.data.leds_bitmap |= sc->led_state[2] << 3;
- report.data.leds_bitmap |= sc->led_state[3] << 4;
+ report->leds_bitmap |= sc->led_state[0] << 1;
+ report->leds_bitmap |= sc->led_state[1] << 2;
+ report->leds_bitmap |= sc->led_state[2] << 3;
+ report->leds_bitmap |= sc->led_state[3] << 4;
/* Set flag for all leds off, required for 3rd party INTEC controller */
- if ((report.data.leds_bitmap & 0x1E) == 0)
- report.data.leds_bitmap |= 0x20;
+ if ((report->leds_bitmap & 0x1E) == 0)
+ report->leds_bitmap |= 0x20;
/*
* The LEDs in the report are indexed in reverse order to their
@@ -1511,28 +1543,30 @@ static void sixaxis_state_worker(struct work_struct *work)
*/
for (n = 0; n < 4; n++) {
if (sc->led_delay_on[n] || sc->led_delay_off[n]) {
- report.data.led[3 - n].duty_off = sc->led_delay_off[n];
- report.data.led[3 - n].duty_on = sc->led_delay_on[n];
+ report->led[3 - n].duty_off = sc->led_delay_off[n];
+ report->led[3 - n].duty_on = sc->led_delay_on[n];
}
}
- hid_hw_raw_request(sc->hdev, report.data.report_id, report.buf,
- sizeof(report), HID_OUTPUT_REPORT, HID_REQ_SET_REPORT);
+ hid_hw_raw_request(sc->hdev, report->report_id, (__u8 *)report,
+ sizeof(struct sixaxis_output_report),
+ HID_OUTPUT_REPORT, HID_REQ_SET_REPORT);
}
static void dualshock4_state_worker(struct work_struct *work)
{
struct sony_sc *sc = container_of(work, struct sony_sc, state_worker);
struct hid_device *hdev = sc->hdev;
+ __u8 *buf = sc->output_report_dmabuf;
int offset;
- __u8 buf[78] = { 0 };
-
if (sc->quirks & DUALSHOCK4_CONTROLLER_USB) {
+ memset(buf, 0, DS4_REPORT_0x05_SIZE);
buf[0] = 0x05;
buf[1] = 0xFF;
offset = 4;
} else {
+ memset(buf, 0, DS4_REPORT_0x11_SIZE);
buf[0] = 0x11;
buf[1] = 0xB0;
buf[3] = 0x0F;
@@ -1560,12 +1594,33 @@ static void dualshock4_state_worker(struct work_struct *work)
buf[offset++] = sc->led_delay_off[3];
if (sc->quirks & DUALSHOCK4_CONTROLLER_USB)
- hid_hw_output_report(hdev, buf, 32);
+ hid_hw_output_report(hdev, buf, DS4_REPORT_0x05_SIZE);
else
- hid_hw_raw_request(hdev, 0x11, buf, 78,
+ hid_hw_raw_request(hdev, 0x11, buf, DS4_REPORT_0x11_SIZE,
HID_OUTPUT_REPORT, HID_REQ_SET_REPORT);
}
+static int sony_allocate_output_report(struct sony_sc *sc)
+{
+ if (sc->quirks & SIXAXIS_CONTROLLER)
+ sc->output_report_dmabuf =
+ kmalloc(sizeof(union sixaxis_output_report_01),
+ GFP_KERNEL);
+ else if (sc->quirks & DUALSHOCK4_CONTROLLER_BT)
+ sc->output_report_dmabuf = kmalloc(DS4_REPORT_0x11_SIZE,
+ GFP_KERNEL);
+ else if (sc->quirks & DUALSHOCK4_CONTROLLER_USB)
+ sc->output_report_dmabuf = kmalloc(DS4_REPORT_0x05_SIZE,
+ GFP_KERNEL);
+ else
+ return 0;
+
+ if (!sc->output_report_dmabuf)
+ return -ENOMEM;
+
+ return 0;
+}
+
#ifdef CONFIG_SONY_FF
static int sony_play_effect(struct input_dev *dev, void *data,
struct ff_effect *effect)
@@ -1754,6 +1809,7 @@ static int sony_get_bt_devaddr(struct sony_sc *sc)
static int sony_check_add(struct sony_sc *sc)
{
+ __u8 *buf = NULL;
int n, ret;
if ((sc->quirks & DUALSHOCK4_CONTROLLER_BT) ||
@@ -1769,36 +1825,44 @@ static int sony_check_add(struct sony_sc *sc)
return 0;
}
} else if (sc->quirks & DUALSHOCK4_CONTROLLER_USB) {
- __u8 buf[7];
+ buf = kmalloc(DS4_REPORT_0x81_SIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
/*
* The MAC address of a DS4 controller connected via USB can be
* retrieved with feature report 0x81. The address begins at
* offset 1.
*/
- ret = hid_hw_raw_request(sc->hdev, 0x81, buf, sizeof(buf),
- HID_FEATURE_REPORT, HID_REQ_GET_REPORT);
+ ret = hid_hw_raw_request(sc->hdev, 0x81, buf,
+ DS4_REPORT_0x81_SIZE, HID_FEATURE_REPORT,
+ HID_REQ_GET_REPORT);
- if (ret != 7) {
+ if (ret != DS4_REPORT_0x81_SIZE) {
hid_err(sc->hdev, "failed to retrieve feature report 0x81 with the DualShock 4 MAC address\n");
- return ret < 0 ? ret : -EINVAL;
+ ret = ret < 0 ? ret : -EINVAL;
+ goto out_free;
}
memcpy(sc->mac_address, &buf[1], sizeof(sc->mac_address));
} else if (sc->quirks & SIXAXIS_CONTROLLER_USB) {
- __u8 buf[18];
+ buf = kmalloc(SIXAXIS_REPORT_0xF2_SIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
/*
* The MAC address of a Sixaxis controller connected via USB can
* be retrieved with feature report 0xf2. The address begins at
* offset 4.
*/
- ret = hid_hw_raw_request(sc->hdev, 0xf2, buf, sizeof(buf),
- HID_FEATURE_REPORT, HID_REQ_GET_REPORT);
+ ret = hid_hw_raw_request(sc->hdev, 0xf2, buf,
+ SIXAXIS_REPORT_0xF2_SIZE, HID_FEATURE_REPORT,
+ HID_REQ_GET_REPORT);
- if (ret != 18) {
+ if (ret != SIXAXIS_REPORT_0xF2_SIZE) {
hid_err(sc->hdev, "failed to retrieve feature report 0xf2 with the Sixaxis MAC address\n");
- return ret < 0 ? ret : -EINVAL;
+ ret = ret < 0 ? ret : -EINVAL;
+ goto out_free;
}
/*
@@ -1811,7 +1875,13 @@ static int sony_check_add(struct sony_sc *sc)
return 0;
}
- return sony_check_add_dev_list(sc);
+ ret = sony_check_add_dev_list(sc);
+
+out_free:
+
+ kfree(buf);
+
+ return ret;
}
static int sony_set_device_id(struct sony_sc *sc)
@@ -1895,6 +1965,12 @@ static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
return ret;
}
+ ret = sony_allocate_output_report(sc);
+ if (ret < 0) {
+ hid_err(hdev, "failed to allocate the output report buffer\n");
+ goto err_stop;
+ }
+
ret = sony_set_device_id(sc);
if (ret < 0) {
hid_err(hdev, "failed to allocate the device id\n");
@@ -1984,6 +2060,7 @@ err_stop:
if (sc->quirks & SONY_BATTERY_SUPPORT)
sony_battery_remove(sc);
sony_cancel_work_sync(sc);
+ kfree(sc->output_report_dmabuf);
sony_remove_dev_list(sc);
sony_release_device_id(sc);
hid_hw_stop(hdev);
@@ -2004,6 +2081,8 @@ static void sony_remove(struct hid_device *hdev)
sony_cancel_work_sync(sc);
+ kfree(sc->output_report_dmabuf);
+
sony_remove_dev_list(sc);
sony_release_device_id(sc);
--
2.1.0
^ permalink raw reply related
* Re: [PATCH v5 1/3] mfd: max8997: use regmap to access registers
From: Sebastian Reichel @ 2014-11-12 17:01 UTC (permalink / raw)
To: Robert Baldyga
Cc: lee.jones, sameo, myungjoo.ham, cw00.choi, dmitry.torokhov,
cooloney, rpurdie, dbaryshkov, dwmw2, lgirdwood, broonie, a.zummo,
paul.gortmaker, sachin.kamat, k.kozlowski, linux-kernel,
linux-input, linux-leds, linux-pm, rtc-linux
In-Reply-To: <1415776996-11569-2-git-send-email-r.baldyga@samsung.com>
[-- Attachment #1: Type: text/plain, Size: 695 bytes --]
Hi,
On Wed, Nov 12, 2014 at 08:23:14AM +0100, Robert Baldyga wrote:
> This patch modifies max8997 driver and each associated function driver,
> to use regmap instead of operating directly on i2c bus. It will allow to
> simplify IRQ handling using regmap-irq.
>
> Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
> Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>
> [For extcon part]
> Acked-by: Chanwoo Choi <cw00.choi@samsung.com>
>
> [For leds part]
> Acked-by: Bryan Wu <cooloney@gmail.com>
>
> [For the mfd part]
> Acked-by: Lee Jones <lee.jones@linaro.org>
[For the power supply part]
Acked-By: Sebastian Reichel <sre@kernel.org>
-- Sebastian
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH v2 1/1] Input: gpio_keys - add device tree support for interrupt only keys
From: Alexander Stein @ 2014-11-12 16:38 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Dmitry Torokhov, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, linux-input, devicetree
In-Reply-To: <3325183.d9PtLzFMKy@wuerfel>
On Wednesday 12 November 2014 17:04:57, Arnd Bergmann wrote:
> On Wednesday 12 November 2014 17:02:56 Alexander Stein wrote:
> > This features already exists for board config setups. Add support for
> > device tree based systems.
> >
> > Signed-off-by: Alexander Stein <alexander.stein@systec-electronic.com>
> > ---
> > Please note: Due to current lack of hardware I could not test it yet. V2
> > includes the changes proposed by Dmitry.
> >
> > Changes in v2:
> > * Added device tree bindings
> > * IRQ is only parsed and mapped when there is no "gpios" property
> >
>
> Can you list one or more examples in the patch description?
> Are these systems that don't expose the GPIO controller with a
> standalone driver, or systems that really actually connect the
> buttons to an interrupt pin?
You mean a use case? I came to this situation to test interrupt polarity on a microcontroller, thus a simple /IRQ pin, no GPIO at all. So in the end I have an input just connected to an interrupt line.
I noticed gpio_keys using platform data only can be used for this setup. So I added this support for device tree.
Best regards,
Alexander
^ permalink raw reply
* Re: [PATCH v2] hid: sony: Use kernel allocated buffers for HID reports
From: Antonio Ospite @ 2014-11-12 16:06 UTC (permalink / raw)
To: Frank Praznik; +Cc: linux-input, jkosina, dmitry.torokhov
In-Reply-To: <1415805534-8401-1-git-send-email-frank.praznik@oh.rr.com>
On Wed, 12 Nov 2014 10:18:54 -0500
Frank Praznik <frank.praznik@oh.rr.com> wrote:
> Replace stack buffers with kernel allocated buffers for sending
> and receiving HID reports to prevent issues with DMA transfers
> on certain hardware.
>
> Output report buffers are allocated at initialization time to avoid
> excessive calls to kmalloc and kfree.
>
> Signed-off-by: Frank Praznik <frank.praznik@oh.rr.com>
> ---
>
> v2 fixes a sizeof(pointer) mistake and corrects some cosmetic issues
> (spacing and #defines instead of magic constants).
>
> The original reporter confirms that this fixes the bug reported in
> https://bugzilla.kernel.org/show_bug.cgi?id=87991
>
> drivers/hid/hid-sony.c | 147 +++++++++++++++++++++++++++++++++++++------------
> 1 file changed, 113 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
> index bc4269e..048aa6b 100644
> --- a/drivers/hid/hid-sony.c
> +++ b/drivers/hid/hid-sony.c
> @@ -798,6 +798,12 @@ union sixaxis_output_report_01 {
> __u8 buf[36];
> };
>
> +#define DS4_REPORT_2_SIZE 37
> +#define DS4_REPORT_5_SIZE 32
> +#define DS4_REPORT_17_SIZE 78
> +#define DS4_REPORT_129_SIZE 7
> +#define SIXAXIS_REPORT_242_SIZE 18
> +
Hi Frank,
just nitpicking, what about using hex numbers in the report names?
i.e.
#define DS4_REPORT_0x02_SIZE 37
#define DS4_REPORT_0x05_SIZE 32
#define DS4_REPORT_0x11_SIZE 78
#define DS4_REPORT_0x81_SIZE 7
#define SIXAXIS_REPORT_0xF2_SIZE 18
either with or without the 0x prefix is fine by me.
That'll be a little easier on the eyes in code like the one below.
[...]
> + ret = hid_hw_raw_request(sc->hdev, 0x81, buf,
> + DS4_REPORT_129_SIZE, HID_FEATURE_REPORT,
> + HID_REQ_GET_REPORT);
[...]
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 v2 1/1] Input: gpio_keys - add device tree support for interrupt only keys
From: Arnd Bergmann @ 2014-11-12 16:04 UTC (permalink / raw)
To: Alexander Stein
Cc: Dmitry Torokhov, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, linux-input, devicetree
In-Reply-To: <1415808176-13282-1-git-send-email-alexander.stein@systec-electronic.com>
On Wednesday 12 November 2014 17:02:56 Alexander Stein wrote:
> This features already exists for board config setups. Add support for
> device tree based systems.
>
> Signed-off-by: Alexander Stein <alexander.stein@systec-electronic.com>
> ---
> Please note: Due to current lack of hardware I could not test it yet. V2
> includes the changes proposed by Dmitry.
>
> Changes in v2:
> * Added device tree bindings
> * IRQ is only parsed and mapped when there is no "gpios" property
>
Can you list one or more examples in the patch description?
Are these systems that don't expose the GPIO controller with a
standalone driver, or systems that really actually connect the
buttons to an interrupt pin?
Arnd
^ permalink raw reply
* [PATCH v2 1/1] Input: gpio_keys - add device tree support for interrupt only keys
From: Alexander Stein @ 2014-11-12 16:02 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala
Cc: Alexander Stein, linux-input, devicetree
This features already exists for board config setups. Add support for
device tree based systems.
Signed-off-by: Alexander Stein <alexander.stein@systec-electronic.com>
---
Please note: Due to current lack of hardware I could not test it yet. V2
includes the changes proposed by Dmitry.
Changes in v2:
* Added device tree bindings
* IRQ is only parsed and mapped when there is no "gpios" property
.../devicetree/bindings/input/gpio-keys.txt | 10 ++++++-
drivers/input/keyboard/gpio_keys.c | 34 +++++++++++++---------
2 files changed, 29 insertions(+), 15 deletions(-)
diff --git a/Documentation/devicetree/bindings/input/gpio-keys.txt b/Documentation/devicetree/bindings/input/gpio-keys.txt
index 5c2c021..a4a38fc 100644
--- a/Documentation/devicetree/bindings/input/gpio-keys.txt
+++ b/Documentation/devicetree/bindings/input/gpio-keys.txt
@@ -10,10 +10,13 @@ Optional properties:
Each button (key) is represented as a sub-node of "gpio-keys":
Subnode properties:
- - gpios: OF device-tree gpio specification.
- label: Descriptive name of the key.
- linux,code: Keycode to emit.
+Required mutual exclusive subnode-properties:
+ - gpios: OF device-tree gpio specification.
+ - interrupts: the interrupt line for that input
+
Optional subnode-properties:
- linux,input-type: Specify event type this button/key generates.
If not specified defaults to <1> == EV_KEY.
@@ -33,4 +36,9 @@ Example nodes:
linux,code = <103>;
gpios = <&gpio1 0 1>;
};
+ button@22 {
+ label = "GPIO Key DOWN";
+ linux,code = <108>;
+ interrupts = <1 IRQ_TYPE_LEVEL_HIGH 7>;
+ };
...
diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
index 8c98e97..ce0d909 100644
--- a/drivers/input/keyboard/gpio_keys.c
+++ b/drivers/input/keyboard/gpio_keys.c
@@ -29,6 +29,7 @@
#include <linux/of.h>
#include <linux/of_platform.h>
#include <linux/of_gpio.h>
+#include <linux/of_irq.h>
#include <linux/spinlock.h>
struct gpio_button_data {
@@ -617,28 +618,33 @@ gpio_keys_get_devtree_pdata(struct device *dev)
i = 0;
for_each_child_of_node(node, pp) {
- int gpio;
+ int gpio = -1;
+ int irq;
enum of_gpio_flags flags;
if (!of_find_property(pp, "gpios", NULL)) {
- pdata->nbuttons--;
- dev_warn(dev, "Found button without gpios\n");
- continue;
- }
-
- gpio = of_get_gpio_flags(pp, 0, &flags);
- if (gpio < 0) {
- error = gpio;
- if (error != -EPROBE_DEFER)
- dev_err(dev,
- "Failed to get gpio flags, error: %d\n",
- error);
- return ERR_PTR(error);
+ irq = irq_of_parse_and_map(pp, 0);
+ if (irq == 0) {
+ pdata->nbuttons--;
+ dev_warn(dev, "Found button without gpios or irqs\n");
+ continue;
+ }
+ } else {
+ gpio = of_get_gpio_flags(pp, 0, &flags);
+ if (gpio < 0) {
+ error = gpio;
+ if (error != -EPROBE_DEFER)
+ dev_err(dev,
+ "Failed to get gpio flags, error: %d\n",
+ error);
+ return ERR_PTR(error);
+ }
}
button = &pdata->buttons[i++];
button->gpio = gpio;
+ button->irq = irq;
button->active_low = flags & OF_GPIO_ACTIVE_LOW;
if (of_property_read_u32(pp, "linux,code", &button->code)) {
--
2.0.4
^ permalink raw reply related
* [PATCH v2] hid: sony: Use kernel allocated buffers for HID reports
From: Frank Praznik @ 2014-11-12 15:18 UTC (permalink / raw)
To: linux-input; +Cc: jkosina, dmitry.torokhov, Frank Praznik
Replace stack buffers with kernel allocated buffers for sending
and receiving HID reports to prevent issues with DMA transfers
on certain hardware.
Output report buffers are allocated at initialization time to avoid
excessive calls to kmalloc and kfree.
Signed-off-by: Frank Praznik <frank.praznik@oh.rr.com>
---
v2 fixes a sizeof(pointer) mistake and corrects some cosmetic issues
(spacing and #defines instead of magic constants).
The original reporter confirms that this fixes the bug reported in
https://bugzilla.kernel.org/show_bug.cgi?id=87991
drivers/hid/hid-sony.c | 147 +++++++++++++++++++++++++++++++++++++------------
1 file changed, 113 insertions(+), 34 deletions(-)
diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index bc4269e..048aa6b 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -798,6 +798,12 @@ union sixaxis_output_report_01 {
__u8 buf[36];
};
+#define DS4_REPORT_2_SIZE 37
+#define DS4_REPORT_5_SIZE 32
+#define DS4_REPORT_17_SIZE 78
+#define DS4_REPORT_129_SIZE 7
+#define SIXAXIS_REPORT_242_SIZE 18
+
static spinlock_t sony_dev_list_lock;
static LIST_HEAD(sony_device_list);
static DEFINE_IDA(sony_device_id_allocator);
@@ -811,6 +817,7 @@ struct sony_sc {
struct work_struct state_worker;
struct power_supply battery;
int device_id;
+ __u8 *output_report_dmabuf;
#ifdef CONFIG_SONY_FF
__u8 left;
@@ -1142,9 +1149,20 @@ static int sixaxis_set_operational_usb(struct hid_device *hdev)
static int sixaxis_set_operational_bt(struct hid_device *hdev)
{
- unsigned char buf[] = { 0xf4, 0x42, 0x03, 0x00, 0x00 };
- return hid_hw_raw_request(hdev, buf[0], buf, sizeof(buf),
+ static const __u8 report[] = { 0xf4, 0x42, 0x03, 0x00, 0x00 };
+ __u8 *buf;
+ int ret;
+
+ buf = kmemdup(report, sizeof(report), GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ ret = hid_hw_raw_request(hdev, buf[0], buf, sizeof(report),
HID_FEATURE_REPORT, HID_REQ_SET_REPORT);
+
+ kfree(buf);
+
+ return ret;
}
/*
@@ -1153,10 +1171,19 @@ static int sixaxis_set_operational_bt(struct hid_device *hdev)
*/
static int dualshock4_set_operational_bt(struct hid_device *hdev)
{
- __u8 buf[37] = { 0 };
+ __u8 *buf;
+ int ret;
- return hid_hw_raw_request(hdev, 0x02, buf, sizeof(buf),
+ buf = kmalloc(DS4_REPORT_2_SIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ ret = hid_hw_raw_request(hdev, 0x02, buf, DS4_REPORT_2_SIZE,
HID_FEATURE_REPORT, HID_REQ_GET_REPORT);
+
+ kfree(buf);
+
+ return ret;
}
static void sixaxis_set_leds_from_id(int id, __u8 values[MAX_LEDS])
@@ -1471,9 +1498,7 @@ error_leds:
static void sixaxis_state_worker(struct work_struct *work)
{
- struct sony_sc *sc = container_of(work, struct sony_sc, state_worker);
- int n;
- union sixaxis_output_report_01 report = {
+ static const union sixaxis_output_report_01 default_report = {
.buf = {
0x01,
0x00, 0xff, 0x00, 0xff, 0x00,
@@ -1485,20 +1510,27 @@ static void sixaxis_state_worker(struct work_struct *work)
0x00, 0x00, 0x00, 0x00, 0x00
}
};
+ struct sony_sc *sc = container_of(work, struct sony_sc, state_worker);
+ struct sixaxis_output_report *report =
+ (struct sixaxis_output_report *)sc->output_report_dmabuf;
+ int n;
+
+ /* Initialize the report with default values */
+ memcpy(report, &default_report, sizeof(struct sixaxis_output_report));
#ifdef CONFIG_SONY_FF
- report.data.rumble.right_motor_on = sc->right ? 1 : 0;
- report.data.rumble.left_motor_force = sc->left;
+ report->rumble.right_motor_on = sc->right ? 1 : 0;
+ report->rumble.left_motor_force = sc->left;
#endif
- report.data.leds_bitmap |= sc->led_state[0] << 1;
- report.data.leds_bitmap |= sc->led_state[1] << 2;
- report.data.leds_bitmap |= sc->led_state[2] << 3;
- report.data.leds_bitmap |= sc->led_state[3] << 4;
+ report->leds_bitmap |= sc->led_state[0] << 1;
+ report->leds_bitmap |= sc->led_state[1] << 2;
+ report->leds_bitmap |= sc->led_state[2] << 3;
+ report->leds_bitmap |= sc->led_state[3] << 4;
/* Set flag for all leds off, required for 3rd party INTEC controller */
- if ((report.data.leds_bitmap & 0x1E) == 0)
- report.data.leds_bitmap |= 0x20;
+ if ((report->leds_bitmap & 0x1E) == 0)
+ report->leds_bitmap |= 0x20;
/*
* The LEDs in the report are indexed in reverse order to their
@@ -1511,28 +1543,30 @@ static void sixaxis_state_worker(struct work_struct *work)
*/
for (n = 0; n < 4; n++) {
if (sc->led_delay_on[n] || sc->led_delay_off[n]) {
- report.data.led[3 - n].duty_off = sc->led_delay_off[n];
- report.data.led[3 - n].duty_on = sc->led_delay_on[n];
+ report->led[3 - n].duty_off = sc->led_delay_off[n];
+ report->led[3 - n].duty_on = sc->led_delay_on[n];
}
}
- hid_hw_raw_request(sc->hdev, report.data.report_id, report.buf,
- sizeof(report), HID_OUTPUT_REPORT, HID_REQ_SET_REPORT);
+ hid_hw_raw_request(sc->hdev, report->report_id, (__u8 *)report,
+ sizeof(struct sixaxis_output_report),
+ HID_OUTPUT_REPORT, HID_REQ_SET_REPORT);
}
static void dualshock4_state_worker(struct work_struct *work)
{
struct sony_sc *sc = container_of(work, struct sony_sc, state_worker);
struct hid_device *hdev = sc->hdev;
+ __u8 *buf = sc->output_report_dmabuf;
int offset;
- __u8 buf[78] = { 0 };
-
if (sc->quirks & DUALSHOCK4_CONTROLLER_USB) {
+ memset(buf, 0, DS4_REPORT_5_SIZE);
buf[0] = 0x05;
buf[1] = 0xFF;
offset = 4;
} else {
+ memset(buf, 0, DS4_REPORT_17_SIZE);
buf[0] = 0x11;
buf[1] = 0xB0;
buf[3] = 0x0F;
@@ -1560,12 +1594,33 @@ static void dualshock4_state_worker(struct work_struct *work)
buf[offset++] = sc->led_delay_off[3];
if (sc->quirks & DUALSHOCK4_CONTROLLER_USB)
- hid_hw_output_report(hdev, buf, 32);
+ hid_hw_output_report(hdev, buf, DS4_REPORT_5_SIZE);
else
- hid_hw_raw_request(hdev, 0x11, buf, 78,
+ hid_hw_raw_request(hdev, 0x11, buf, DS4_REPORT_17_SIZE,
HID_OUTPUT_REPORT, HID_REQ_SET_REPORT);
}
+static int sony_allocate_output_report(struct sony_sc *sc)
+{
+ if (sc->quirks & SIXAXIS_CONTROLLER)
+ sc->output_report_dmabuf =
+ kmalloc(sizeof(union sixaxis_output_report_01),
+ GFP_KERNEL);
+ else if (sc->quirks & DUALSHOCK4_CONTROLLER_BT)
+ sc->output_report_dmabuf = kmalloc(DS4_REPORT_17_SIZE,
+ GFP_KERNEL);
+ else if (sc->quirks & DUALSHOCK4_CONTROLLER_USB)
+ sc->output_report_dmabuf = kmalloc(DS4_REPORT_5_SIZE,
+ GFP_KERNEL);
+ else
+ return 0;
+
+ if (!sc->output_report_dmabuf)
+ return -ENOMEM;
+
+ return 0;
+}
+
#ifdef CONFIG_SONY_FF
static int sony_play_effect(struct input_dev *dev, void *data,
struct ff_effect *effect)
@@ -1754,6 +1809,7 @@ static int sony_get_bt_devaddr(struct sony_sc *sc)
static int sony_check_add(struct sony_sc *sc)
{
+ __u8 *buf = NULL;
int n, ret;
if ((sc->quirks & DUALSHOCK4_CONTROLLER_BT) ||
@@ -1769,36 +1825,44 @@ static int sony_check_add(struct sony_sc *sc)
return 0;
}
} else if (sc->quirks & DUALSHOCK4_CONTROLLER_USB) {
- __u8 buf[7];
+ buf = kmalloc(DS4_REPORT_129_SIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
/*
* The MAC address of a DS4 controller connected via USB can be
* retrieved with feature report 0x81. The address begins at
* offset 1.
*/
- ret = hid_hw_raw_request(sc->hdev, 0x81, buf, sizeof(buf),
- HID_FEATURE_REPORT, HID_REQ_GET_REPORT);
+ ret = hid_hw_raw_request(sc->hdev, 0x81, buf,
+ DS4_REPORT_129_SIZE, HID_FEATURE_REPORT,
+ HID_REQ_GET_REPORT);
- if (ret != 7) {
+ if (ret != DS4_REPORT_129_SIZE) {
hid_err(sc->hdev, "failed to retrieve feature report 0x81 with the DualShock 4 MAC address\n");
- return ret < 0 ? ret : -EINVAL;
+ ret = ret < 0 ? ret : -EINVAL;
+ goto out_free;
}
memcpy(sc->mac_address, &buf[1], sizeof(sc->mac_address));
} else if (sc->quirks & SIXAXIS_CONTROLLER_USB) {
- __u8 buf[18];
+ buf = kmalloc(SIXAXIS_REPORT_242_SIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
/*
* The MAC address of a Sixaxis controller connected via USB can
* be retrieved with feature report 0xf2. The address begins at
* offset 4.
*/
- ret = hid_hw_raw_request(sc->hdev, 0xf2, buf, sizeof(buf),
- HID_FEATURE_REPORT, HID_REQ_GET_REPORT);
+ ret = hid_hw_raw_request(sc->hdev, 0xf2, buf,
+ SIXAXIS_REPORT_242_SIZE, HID_FEATURE_REPORT,
+ HID_REQ_GET_REPORT);
- if (ret != 18) {
+ if (ret != SIXAXIS_REPORT_242_SIZE) {
hid_err(sc->hdev, "failed to retrieve feature report 0xf2 with the Sixaxis MAC address\n");
- return ret < 0 ? ret : -EINVAL;
+ ret = ret < 0 ? ret : -EINVAL;
+ goto out_free;
}
/*
@@ -1811,7 +1875,13 @@ static int sony_check_add(struct sony_sc *sc)
return 0;
}
- return sony_check_add_dev_list(sc);
+ ret = sony_check_add_dev_list(sc);
+
+out_free:
+
+ kfree(buf);
+
+ return ret;
}
static int sony_set_device_id(struct sony_sc *sc)
@@ -1895,6 +1965,12 @@ static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
return ret;
}
+ ret = sony_allocate_output_report(sc);
+ if (ret < 0) {
+ hid_err(hdev, "failed to allocate the output report buffer\n");
+ goto err_stop;
+ }
+
ret = sony_set_device_id(sc);
if (ret < 0) {
hid_err(hdev, "failed to allocate the device id\n");
@@ -1984,6 +2060,7 @@ err_stop:
if (sc->quirks & SONY_BATTERY_SUPPORT)
sony_battery_remove(sc);
sony_cancel_work_sync(sc);
+ kfree(sc->output_report_dmabuf);
sony_remove_dev_list(sc);
sony_release_device_id(sc);
hid_hw_stop(hdev);
@@ -2004,6 +2081,8 @@ static void sony_remove(struct hid_device *hdev)
sony_cancel_work_sync(sc);
+ kfree(sc->output_report_dmabuf);
+
sony_remove_dev_list(sc);
sony_release_device_id(sc);
--
2.1.0
^ permalink raw reply related
* Re: [PATCH] hid: sony: Use kernel allocated buffers for HID reports
From: Frank Praznik @ 2014-11-12 15:18 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: open list:HID CORE LAYER <linux-input@vger.kernel.org>, Jiri Kosina,
open list:HID CORE LAYER <linux-input@vger.kernel.org>, Jiri Kosina
In-Reply-To: <20141111190412.GC27720@dtor-ws>
On Tue, Nov 11, 2014 at 2:04 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> Hi Frank,
>> static spinlock_t sony_dev_list_lock;
>> static LIST_HEAD(sony_device_list);
>> static DEFINE_IDA(sony_device_id_allocator);
>> @@ -811,6 +814,7 @@ struct sony_sc {
>> struct work_struct state_worker;
>> struct power_supply battery;
>> int device_id;
>> + __u8 *output_report_dmabuf;
>
> Just to confirm as I haven't looked at the entire driver: there is no
> possibility of 2 requests being submitted at the same time so that one
> will overwrite other's data?
>
> --
> Dmitry
Output reports are sent using a work request submitted to the kernel
work queue with schedule_work(), so using one buffer is safe since the
work_struct can't be scheduled multiple times or simultaneously
execute across multiple cores.
The other comments you had are addressed in v2 of the patch.
^ 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