* Re: [PATCH v5 0/4] new driver for Valve Steam Controller
From: Clément VUCHENER @ 2018-03-12 14:30 UTC (permalink / raw)
To: Rodrigo Rivas Costa
Cc: Jiri Kosina, Benjamin Tissoires, Pierre-Loup A. Griffais,
Cameron Gutman, lkml, linux-input
In-Reply-To: <20180311195842.5551-1-rodrigorivascosta@gmail.com>
2018-03-11 20:58 GMT+01:00 Rodrigo Rivas Costa <rodrigorivascosta@gmail.com>:
> This patchset implements a driver for Valve Steam Controller, based on a
> reverse analysis by myself.
>
> Sorry, I've been out of town for a few weeks and couldn't keep up with this...
>
> @Pierre-Loup and @Clément, could you please have another look at this and
> check if it is worthy? Benjamin will not commit it without an express ACK from
> Valve. Of course he is right to be cautious, but I checked this driver with
> the Steam Client and all seems to go just fine. I think that there is a lot of
> Linux out of the desktop that could use this driver and cannot use the Steam
> Client. Worst case scenario, this driver can now be blacklisted, but I hope
> that will not be needed.
I tested the driver with my 4.15 fedora kernel (I only built the
module not the whole kernel) and I got double inputs (your driver
input device + steam uinput device) when testing Shovel Knight with
Steam Big Picture. It seems to work fine when the inputs are the same,
but after changing the controller configuration in Steam, the issue
became apparent.
And without Steam and your external tool, you get double inputs too. I
tried RetroArch and it was unusable because of the keyboard inputs
from the lizard mode (e.g. pressing B also presses Esc and quits
RetroArch). Having to download and compile an external tool to make
the driver work properly may be too difficult for the user. Your goal
was to provide an alternative to user space drivers but now you
actually depend on (a very simple) one.
Also the button and axis codes do not match the gamepad API doc
(https://www.kernel.org/doc/Documentation/input/gamepad.txt).
>
> For full reference, I'm adding a full changelog of this patchset.
>
> Changes in v5:
> * Fix license SPDX to GPL-2.0+.
> * Minor stylistic changes (BIT(3) instead 0x08 and so on).
>
> Changes in v4:
> * Add command to check the wireless connection status on probe, without
> waiting for a message (thanks to Clément Vuchener for the tip).
> * Removed the error code on redundant connection/disconnection messages. That
> was harmless but polluted dmesg.
> * Added buttons for touching the left-pad and right-pad.
> * Fixed a misplaced #include from 2/4 to 1/4.
>
> Changes in v3:
> * Use RCU to do the dynamic connec/disconnect of wireless devices.
> * Remove entries in hid-quirks.c as they are no longer needed. This allows
> this module to be blacklisted without side effects.
> * Do not bypass the virtual keyboard/mouse HID devices to avoid breaking
> existing use cases (lizard mode). A user-space tool to do that is
> linked.
> * Fully separated axes for joystick and left-pad. As it happens.
> * Add fuzz values for left/right pad axes, they are a little wiggly.
>
> Changes in v2:
> * Remove references to USB. Now the interesting interfaces are selected by
> looking for the ones with feature reports.
> * Feature reports buffers are allocated with hid_alloc_report_buf().
> * Feature report length is checked, to avoid overflows in case of
> corrupt/malicius USB devices.
> * Resolution added to the ABS axes.
> * A lot of minor cleanups.
>
> Rodrigo Rivas Costa (4):
> HID: add driver for Valve Steam Controller
> HID: steam: add serial number information.
> HID: steam: command to check wireless connection
> HID: steam: add battery device.
>
> drivers/hid/Kconfig | 8 +
> drivers/hid/Makefile | 1 +
> drivers/hid/hid-ids.h | 4 +
> drivers/hid/hid-steam.c | 794 ++++++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 807 insertions(+)
> create mode 100644 drivers/hid/hid-steam.c
>
> --
> 2.16.2
>
^ permalink raw reply
* Re: [PATCH v5 1/4] mfd: syscon: Set name of regmap_config
From: Lee Jones @ 2018-03-12 14:06 UTC (permalink / raw)
To: Jeffy Chen
Cc: linux-kernel, broonie, alexandre.belloni, Guenter Roeck,
Arnd Bergmann, Joseph Lo, Rob Herring, Catalin Marinas,
Emil Renner Berthing, Heiko Stuebner, Brian Norris,
Thomas Gleixner, Philippe Ombredanne, linux-rockchip,
Kate Stewart, linux-input, Will Deacon, Matthias Kaehlcke,
devicetree, stephen lu, Greg Kroah-Hartman,
Arvind Yadav <arvind.>
In-Reply-To: <20180312095142.30854-2-jeffy.chen@rock-chips.com>
On Mon, 12 Mar 2018, Jeffy Chen wrote:
> We are now allowing to register debugfs without a valid device, and not
> having a valid name will end up using "dummy*" to create debugfs dir.
>
> Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
> ---
>
> Changes in v5:
> CC lee.jones@linaro.org
>
> Changes in v4: None
> Changes in v3:
> Modify commit message.
>
> drivers/mfd/syscon.c | 1 +
> 1 file changed, 1 insertion(+)
Looks okay. Applied, thanks.
> diff --git a/drivers/mfd/syscon.c b/drivers/mfd/syscon.c
> index fc9ba0ea4e44..b6d05cd934e6 100644
> --- a/drivers/mfd/syscon.c
> +++ b/drivers/mfd/syscon.c
> @@ -110,6 +110,7 @@ static struct syscon *of_syscon_register(struct device_node *np)
> syscon_config.reg_stride = reg_io_width;
> syscon_config.val_bits = reg_io_width * 8;
> syscon_config.max_register = resource_size(&res) - reg_io_width;
> + syscon_config.name = of_node_full_name(np);
>
> regmap = regmap_init_mmio(NULL, base, &syscon_config);
> if (IS_ERR(regmap)) {
--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* Re: [PATCH v5 1/3] Input: gpio-keys - add support for wakeup event action
From: JeffyChen @ 2018-03-12 10:40 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: linux-kernel, briannorris, heiko, dtor, dianders, devicetree,
Guenter Roeck, Thomas Gleixner, Joseph Lo, stephen lu,
Rob Herring, Kate Stewart, linux-input, Greg Kroah-Hartman,
Mark Rutland, Philippe Ombredanne, Arvind Yadav
In-Reply-To: <20180310181509.GB260013@dtor-ws>
Hi Dmitry,
Thanks for your reply.
On 03/11/2018 02:15 AM, Dmitry Torokhov wrote:>> +static int
gpio_keys_enable_wakeup(struct gpio_button_data *bdata)
> These new helpers need to be __maybe_unused in case we compile on system
> without suspend support.
>
> I also wanted a bit more of error handling, so I ended up with the
> version of the patch below. Can you please try it and let me know if I
> broke it.
sure, the patch you sent works :)
Tested-by: Jeffy Chen <jeffy.chen@rock-chips.com>
>
> Thanks.
>
> -- Dmitry
>
> Input: gpio-keys - add support for wakeup event action
>
> From: Jeffy Chen <jeffy.chen@rock-chips.com>
>
> Add support for specifying event actions to trigger wakeup when using
> the gpio-keys input device as a wakeup source.
>
> This would allow the device to configure when to wakeup the system. For
> example a gpio-keys input device for pen insert, may only want to wakeup
> the system when ejecting the pen.
>
> Suggested-by: Brian Norris <briannorris@chromium.org>
> Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
> Reviewed-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
^ permalink raw reply
* Re: [PATCH v5 4/4] ARM: rockchip: Set name of regmap_config
From: Heiko Stuebner @ 2018-03-12 10:11 UTC (permalink / raw)
To: Jeffy Chen
Cc: linux-kernel, broonie, alexandre.belloni, Guenter Roeck,
Arnd Bergmann, Joseph Lo, Rob Herring, Catalin Marinas, lee.jones,
Emil Renner Berthing, Brian Norris, Thomas Gleixner,
Philippe Ombredanne, linux-rockchip, Kate Stewart, linux-input,
Will Deacon, Matthias Kaehlcke, devicetree, stephen lu,
Greg Kroah-Hartman, Arvind Yadav
In-Reply-To: <20180312095142.30854-5-jeffy.chen@rock-chips.com>
Am Montag, 12. März 2018, 10:51:42 CET schrieb Jeffy Chen:
> We are now allowing to register debugfs without a valid device, and not
> having a valid name will end up using "dummy*" to create debugfs dir.
>
> Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
applied for 4.17
Thanks
Heiko
^ permalink raw reply
* [PATCH v5 4/4] ARM: rockchip: Set name of regmap_config
From: Jeffy Chen @ 2018-03-12 9:51 UTC (permalink / raw)
To: linux-kernel, broonie, alexandre.belloni
Cc: Guenter Roeck, Arnd Bergmann, Joseph Lo, Rob Herring,
Catalin Marinas, lee.jones, Emil Renner Berthing, Heiko Stuebner,
Brian Norris, Thomas Gleixner, Philippe Ombredanne,
linux-rockchip, Kate Stewart, linux-input, Will Deacon,
Jeffy Chen, Matthias Kaehlcke, devicetree, stephen lu,
Greg Kroah-Hartman, Arvind Yadav
In-Reply-To: <20180312095142.30854-1-jeffy.chen@rock-chips.com>
We are now allowing to register debugfs without a valid device, and not
having a valid name will end up using "dummy*" to create debugfs dir.
Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
---
Changes in v5: None
Changes in v4: None
Changes in v3: None
arch/arm/mach-rockchip/platsmp.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/mach-rockchip/platsmp.c b/arch/arm/mach-rockchip/platsmp.c
index ecec340ca345..51984a40b097 100644
--- a/arch/arm/mach-rockchip/platsmp.c
+++ b/arch/arm/mach-rockchip/platsmp.c
@@ -208,6 +208,7 @@ static int __init rockchip_smp_prepare_sram(struct device_node *node)
}
static const struct regmap_config rockchip_pmu_regmap_config = {
+ .name = "rockchip-pmu",
.reg_bits = 32,
.val_bits = 32,
.reg_stride = 4,
--
2.11.0
^ permalink raw reply related
* [PATCH v5 3/4] clk: lpc32xx: Set name of regmap_config
From: Jeffy Chen @ 2018-03-12 9:51 UTC (permalink / raw)
To: linux-kernel, broonie, alexandre.belloni
Cc: Guenter Roeck, Arnd Bergmann, Joseph Lo, Rob Herring,
Catalin Marinas, lee.jones, Emil Renner Berthing, Heiko Stuebner,
Brian Norris, Thomas Gleixner, Philippe Ombredanne,
linux-rockchip, Kate Stewart, linux-input, Will Deacon,
Jeffy Chen, Matthias Kaehlcke, devicetree, stephen lu,
Greg Kroah-Hartman, Arvind Yadav
In-Reply-To: <20180312095142.30854-1-jeffy.chen@rock-chips.com>
We are now allowing to register debugfs without a valid device, and not
having a valid name will end up using "dummy*" to create debugfs dir.
Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
---
Changes in v5: None
Changes in v4: None
Changes in v3: None
drivers/clk/nxp/clk-lpc32xx.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/clk/nxp/clk-lpc32xx.c b/drivers/clk/nxp/clk-lpc32xx.c
index f5d815f577e0..a2a0a7f3bc57 100644
--- a/drivers/clk/nxp/clk-lpc32xx.c
+++ b/drivers/clk/nxp/clk-lpc32xx.c
@@ -67,6 +67,7 @@
#define LPC32XX_USB_CLK_STS 0xF8
static struct regmap_config lpc32xx_scb_regmap_config = {
+ .name = "lpc32xx-scb",
.reg_bits = 32,
.val_bits = 32,
.reg_stride = 4,
--
2.11.0
^ permalink raw reply related
* [PATCH v5 2/4] rtc: at91sam9: Set name of regmap_config
From: Jeffy Chen @ 2018-03-12 9:51 UTC (permalink / raw)
To: linux-kernel, broonie, alexandre.belloni
Cc: Guenter Roeck, Arnd Bergmann, Joseph Lo, Rob Herring,
Catalin Marinas, lee.jones, Emil Renner Berthing, Heiko Stuebner,
Brian Norris, Thomas Gleixner, Philippe Ombredanne,
linux-rockchip, Kate Stewart, linux-input, Will Deacon,
Jeffy Chen, Matthias Kaehlcke, devicetree, stephen lu,
Greg Kroah-Hartman, Arvind Yadav
In-Reply-To: <20180312095142.30854-1-jeffy.chen@rock-chips.com>
We are now allowing to register debugfs without a valid device, and not
having a valid name will end up using "dummy*" to create debugfs dir.
Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
---
Changes in v5: None
Changes in v4:
Not to use rtc dev to init regmap, the gpbr is not directly related to
the rtc.
Changes in v3: None
drivers/rtc/rtc-at91sam9.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/rtc/rtc-at91sam9.c b/drivers/rtc/rtc-at91sam9.c
index 7418a763ce52..ee71e647fd43 100644
--- a/drivers/rtc/rtc-at91sam9.c
+++ b/drivers/rtc/rtc-at91sam9.c
@@ -349,6 +349,7 @@ static const struct rtc_class_ops at91_rtc_ops = {
};
static const struct regmap_config gpbr_regmap_config = {
+ .name = "gpbr",
.reg_bits = 32,
.val_bits = 32,
.reg_stride = 4,
--
2.11.0
^ permalink raw reply related
* [PATCH v5 1/4] mfd: syscon: Set name of regmap_config
From: Jeffy Chen @ 2018-03-12 9:51 UTC (permalink / raw)
To: linux-kernel, broonie, alexandre.belloni
Cc: Guenter Roeck, Arnd Bergmann, Joseph Lo, Rob Herring,
Catalin Marinas, lee.jones, Emil Renner Berthing, Heiko Stuebner,
Brian Norris, Thomas Gleixner, Philippe Ombredanne,
linux-rockchip, Kate Stewart, linux-input, Will Deacon,
Jeffy Chen, Matthias Kaehlcke, devicetree, stephen lu,
Greg Kroah-Hartman, Arvind Yadav
In-Reply-To: <20180312095142.30854-1-jeffy.chen@rock-chips.com>
We are now allowing to register debugfs without a valid device, and not
having a valid name will end up using "dummy*" to create debugfs dir.
Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
---
Changes in v5:
CC lee.jones@linaro.org
Changes in v4: None
Changes in v3:
Modify commit message.
drivers/mfd/syscon.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/mfd/syscon.c b/drivers/mfd/syscon.c
index fc9ba0ea4e44..b6d05cd934e6 100644
--- a/drivers/mfd/syscon.c
+++ b/drivers/mfd/syscon.c
@@ -110,6 +110,7 @@ static struct syscon *of_syscon_register(struct device_node *np)
syscon_config.reg_stride = reg_io_width;
syscon_config.val_bits = reg_io_width * 8;
syscon_config.max_register = resource_size(&res) - reg_io_width;
+ syscon_config.name = of_node_full_name(np);
regmap = regmap_init_mmio(NULL, base, &syscon_config);
if (IS_ERR(regmap)) {
--
2.11.0
^ permalink raw reply related
* [PATCH v5 0/4] Set name of regmap_config
From: Jeffy Chen @ 2018-03-12 9:51 UTC (permalink / raw)
To: linux-kernel, broonie, alexandre.belloni
Cc: Guenter Roeck, Arnd Bergmann, Joseph Lo, Rob Herring,
Catalin Marinas, lee.jones, Emil Renner Berthing, Heiko Stuebner,
Brian Norris, Thomas Gleixner, Philippe Ombredanne,
linux-rockchip, Kate Stewart, linux-input, Will Deacon,
Jeffy Chen, Matthias Kaehlcke, devicetree, stephen lu,
Greg Kroah-Hartman, Arvind Yadav
After 9b947a13e7f6 ("regmap: use debugfs even when no device"), we are
allowing regmap to create debugfs without a valid device. Some of them
would end up using "dummy*" as debugfs name.
This series try to set names in some regmap_config to avoid that.
Changes in v5:
CC lee.jones@linaro.org
Changes in v4:
Not to use rtc dev to init regmap, the gpbr is not directly related to
the rtc.
Changes in v3:
Modify commit message.
Jeffy Chen (4):
mfd: syscon: Set name of regmap_config
rtc: at91sam9: Set name of regmap_config
clk: lpc32xx: Set name of regmap_config
ARM: rockchip: Set name of regmap_config
arch/arm/mach-rockchip/platsmp.c | 1 +
drivers/clk/nxp/clk-lpc32xx.c | 1 +
drivers/mfd/syscon.c | 1 +
drivers/rtc/rtc-at91sam9.c | 1 +
4 files changed, 4 insertions(+)
--
2.11.0
^ permalink raw reply
* Re: [RESEND PATCH v4 1/4] mfd: syscon: Set name of regmap_config
From: JeffyChen @ 2018-03-12 9:47 UTC (permalink / raw)
To: Lee Jones
Cc: linux-kernel, broonie, alexandre.belloni, Guenter Roeck,
Arnd Bergmann, Joseph Lo, Rob Herring, Catalin Marinas,
Emil Renner Berthing, Heiko Stuebner, Brian Norris,
Thomas Gleixner, Philippe Ombredanne, linux-rockchip,
Kate Stewart, linux-input, Will Deacon, Matthias Kaehlcke,
devicetree, stephen lu, Greg Kroah-Hartman,
Arvind Yadav <arvind.>
In-Reply-To: <20180312091214.f6dwq6q3eqqblfnf@dell>
Hi Lee,
On 03/12/2018 05:12 PM, Lee Jones wrote:
> On Fri, 09 Mar 2018, Jeffy Chen wrote:
>
>> >We are now allowing to register debugfs without a valid device, and not
>> >having a valid name will end up using "dummy*" to create debugfs dir.
>> >
>> >Signed-off-by: Jeffy Chen<jeffy.chen@rock-chips.com>
>> >---
>> >
>> >Changes in v4: None
>> >Changes in v3:
>> >Modify commit message.
>> >
>> > drivers/mfd/syscon.c | 1 +
>> > 1 file changed, 1 insertion(+)
> Is this the RESEND I requested? I can still only see this patch.
>
oops, sorry, i was using patman, and i force it to CC everyone in the
cover letter's CC list, but that doesn't contain your mail address..i'll
try another way :)
^ permalink raw reply
* Re: [RESEND PATCH v4 1/4] mfd: syscon: Set name of regmap_config
From: Lee Jones @ 2018-03-12 9:12 UTC (permalink / raw)
To: Jeffy Chen
Cc: linux-kernel, broonie, alexandre.belloni, Guenter Roeck,
Arnd Bergmann, Joseph Lo, Rob Herring, Catalin Marinas,
Emil Renner Berthing, Heiko Stuebner, Brian Norris,
Thomas Gleixner, Philippe Ombredanne, linux-rockchip,
Kate Stewart, linux-input, Will Deacon, Matthias Kaehlcke,
devicetree, stephen lu, Greg Kroah-Hartman,
Arvind Yadav <arvind.>
In-Reply-To: <20180309084351.32009-2-jeffy.chen@rock-chips.com>
On Fri, 09 Mar 2018, Jeffy Chen wrote:
> We are now allowing to register debugfs without a valid device, and not
> having a valid name will end up using "dummy*" to create debugfs dir.
>
> Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
> ---
>
> Changes in v4: None
> Changes in v3:
> Modify commit message.
>
> drivers/mfd/syscon.c | 1 +
> 1 file changed, 1 insertion(+)
Is this the RESEND I requested? I can still only see this patch.
> diff --git a/drivers/mfd/syscon.c b/drivers/mfd/syscon.c
> index fc9ba0ea4e44..b6d05cd934e6 100644
> --- a/drivers/mfd/syscon.c
> +++ b/drivers/mfd/syscon.c
> @@ -110,6 +110,7 @@ static struct syscon *of_syscon_register(struct device_node *np)
> syscon_config.reg_stride = reg_io_width;
> syscon_config.val_bits = reg_io_width * 8;
> syscon_config.max_register = resource_size(&res) - reg_io_width;
> + syscon_config.name = of_node_full_name(np);
>
> regmap = regmap_init_mmio(NULL, base, &syscon_config);
> if (IS_ERR(regmap)) {
--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* Re: [PATCH v5 0/4] new driver for Valve Steam Controller
From: Rodrigo Rivas Costa @ 2018-03-12 7:35 UTC (permalink / raw)
To: Pierre-Loup A. Griffais
Cc: Jiri Kosina, Benjamin Tissoires, Cameron Gutman,
Clément VUCHENER, linux-kernel, linux-input
In-Reply-To: <99cb195e-589c-b3fc-1237-e1c8f557c51e@valvesoftware.com>
On Sun, Mar 11, 2018 at 04:12:41PM -0700, Pierre-Loup A. Griffais wrote:
>
>
> On 03/11/2018 12:58 PM, Rodrigo Rivas Costa wrote:
> > This patchset implements a driver for Valve Steam Controller, based on a
> > reverse analysis by myself.
> >
> > Sorry, I've been out of town for a few weeks and couldn't keep up with this...
> >
> > @Pierre-Loup and @Clément, could you please have another look at this and
> > check if it is worthy? Benjamin will not commit it without an express ACK from
> > Valve. Of course he is right to be cautious, but I checked this driver with
> > the Steam Client and all seems to go just fine. I think that there is a lot of
> > Linux out of the desktop that could use this driver and cannot use the Steam
> > Client. Worst case scenario, this driver can now be blacklisted, but I hope
> > that will not be needed.
>
> Hi Rodrigo,
>
> I think the approach you outlined earlier of only sending configuration
> commands to the device when something is actively using the driver is sane.
> I won't have the cycles to thoroughly check all the possible interactions
> with the client in the near future, so in the interest of not blocking
> development I'd say go for it. I'll try to get someone to take the patchset
> for a spin soon.
First of all, thank you very much for your attention.
Note that this driver does not send any configuration command at all. It
only sends commands get_serial (ae 15 01) and get_comm_status (b4), and
those just on probe time. That should be quite safe IMO.
Sending a configuration command, such as enable/disable_gyro on
input->open() is probably not so safe, so I'm leaving that for an
eventual future patch. Many games (and even X) enumerate the input
devices by opening and closing them. That could cause trouble...
Best regards.
Rodrigo
>
> Thanks,
> - Pierre-Loup
>
> >
> > For full reference, I'm adding a full changelog of this patchset.
> >
> > Changes in v5:
> > * Fix license SPDX to GPL-2.0+.
> > * Minor stylistic changes (BIT(3) instead 0x08 and so on).
> >
> > Changes in v4:
> > * Add command to check the wireless connection status on probe, without
> > waiting for a message (thanks to Clément Vuchener for the tip).
> > * Removed the error code on redundant connection/disconnection messages. That
> > was harmless but polluted dmesg.
> > * Added buttons for touching the left-pad and right-pad.
> > * Fixed a misplaced #include from 2/4 to 1/4.
> >
> > Changes in v3:
> > * Use RCU to do the dynamic connec/disconnect of wireless devices.
> > * Remove entries in hid-quirks.c as they are no longer needed. This allows
> > this module to be blacklisted without side effects.
> > * Do not bypass the virtual keyboard/mouse HID devices to avoid breaking
> > existing use cases (lizard mode). A user-space tool to do that is
> > linked.
> > * Fully separated axes for joystick and left-pad. As it happens.
> > * Add fuzz values for left/right pad axes, they are a little wiggly.
> >
> > Changes in v2:
> > * Remove references to USB. Now the interesting interfaces are selected by
> > looking for the ones with feature reports.
> > * Feature reports buffers are allocated with hid_alloc_report_buf().
> > * Feature report length is checked, to avoid overflows in case of
> > corrupt/malicius USB devices.
> > * Resolution added to the ABS axes.
> > * A lot of minor cleanups.
> >
> > Rodrigo Rivas Costa (4):
> > HID: add driver for Valve Steam Controller
> > HID: steam: add serial number information.
> > HID: steam: command to check wireless connection
> > HID: steam: add battery device.
> >
> > drivers/hid/Kconfig | 8 +
> > drivers/hid/Makefile | 1 +
> > drivers/hid/hid-ids.h | 4 +
> > drivers/hid/hid-steam.c | 794 ++++++++++++++++++++++++++++++++++++++++++++++++
> > 4 files changed, 807 insertions(+)
> > create mode 100644 drivers/hid/hid-steam.c
> >
>
^ permalink raw reply
* Re: [PATCH v5 0/4] new driver for Valve Steam Controller
From: Pierre-Loup A. Griffais @ 2018-03-11 23:12 UTC (permalink / raw)
To: Rodrigo Rivas Costa, Jiri Kosina, Benjamin Tissoires,
Cameron Gutman, Clément VUCHENER, linux-kernel, linux-input
In-Reply-To: <20180311195842.5551-1-rodrigorivascosta@gmail.com>
On 03/11/2018 12:58 PM, Rodrigo Rivas Costa wrote:
> This patchset implements a driver for Valve Steam Controller, based on a
> reverse analysis by myself.
>
> Sorry, I've been out of town for a few weeks and couldn't keep up with this...
>
> @Pierre-Loup and @Clément, could you please have another look at this and
> check if it is worthy? Benjamin will not commit it without an express ACK from
> Valve. Of course he is right to be cautious, but I checked this driver with
> the Steam Client and all seems to go just fine. I think that there is a lot of
> Linux out of the desktop that could use this driver and cannot use the Steam
> Client. Worst case scenario, this driver can now be blacklisted, but I hope
> that will not be needed.
Hi Rodrigo,
I think the approach you outlined earlier of only sending configuration
commands to the device when something is actively using the driver is
sane. I won't have the cycles to thoroughly check all the possible
interactions with the client in the near future, so in the interest of
not blocking development I'd say go for it. I'll try to get someone to
take the patchset for a spin soon.
Thanks,
- Pierre-Loup
>
> For full reference, I'm adding a full changelog of this patchset.
>
> Changes in v5:
> * Fix license SPDX to GPL-2.0+.
> * Minor stylistic changes (BIT(3) instead 0x08 and so on).
>
> Changes in v4:
> * Add command to check the wireless connection status on probe, without
> waiting for a message (thanks to Clément Vuchener for the tip).
> * Removed the error code on redundant connection/disconnection messages. That
> was harmless but polluted dmesg.
> * Added buttons for touching the left-pad and right-pad.
> * Fixed a misplaced #include from 2/4 to 1/4.
>
> Changes in v3:
> * Use RCU to do the dynamic connec/disconnect of wireless devices.
> * Remove entries in hid-quirks.c as they are no longer needed. This allows
> this module to be blacklisted without side effects.
> * Do not bypass the virtual keyboard/mouse HID devices to avoid breaking
> existing use cases (lizard mode). A user-space tool to do that is
> linked.
> * Fully separated axes for joystick and left-pad. As it happens.
> * Add fuzz values for left/right pad axes, they are a little wiggly.
>
> Changes in v2:
> * Remove references to USB. Now the interesting interfaces are selected by
> looking for the ones with feature reports.
> * Feature reports buffers are allocated with hid_alloc_report_buf().
> * Feature report length is checked, to avoid overflows in case of
> corrupt/malicius USB devices.
> * Resolution added to the ABS axes.
> * A lot of minor cleanups.
>
> Rodrigo Rivas Costa (4):
> HID: add driver for Valve Steam Controller
> HID: steam: add serial number information.
> HID: steam: command to check wireless connection
> HID: steam: add battery device.
>
> drivers/hid/Kconfig | 8 +
> drivers/hid/Makefile | 1 +
> drivers/hid/hid-ids.h | 4 +
> drivers/hid/hid-steam.c | 794 ++++++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 807 insertions(+)
> create mode 100644 drivers/hid/hid-steam.c
>
^ permalink raw reply
* [PATCH v5 4/4] HID: steam: add battery device.
From: Rodrigo Rivas Costa @ 2018-03-11 19:58 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Pierre-Loup A. Griffais,
Cameron Gutman, Clément VUCHENER, linux-kernel, linux-input
Cc: Rodrigo Rivas Costa
In-Reply-To: <20180311195842.5551-1-rodrigorivascosta@gmail.com>
The wireless Steam Controller is battery operated, so add the battery
device and power information.
Signed-off-by: Rodrigo Rivas Costa <rodrigorivascosta@gmail.com>
---
drivers/hid/hid-steam.c | 141 +++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 140 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
index 3d34716ae55f..188d8c1a69d7 100644
--- a/drivers/hid/hid-steam.c
+++ b/drivers/hid/hid-steam.c
@@ -19,6 +19,7 @@
#include <linux/workqueue.h>
#include <linux/rcupdate.h>
#include <linux/delay.h>
+#include <linux/power_supply.h>
#include "hid-ids.h"
MODULE_LICENSE("GPL");
@@ -44,6 +45,10 @@ struct steam_device {
struct work_struct work_connect;
bool connected;
char serial_no[STEAM_SERIAL_LEN + 1];
+ struct power_supply_desc battery_desc;
+ struct power_supply __rcu *battery;
+ u8 battery_charge;
+ u16 voltage;
};
static int steam_recv_report(struct steam_device *steam,
@@ -168,6 +173,85 @@ static void steam_input_close(struct input_dev *dev)
hid_hw_close(steam->hdev);
}
+static enum power_supply_property steam_battery_props[] = {
+ POWER_SUPPLY_PROP_PRESENT,
+ POWER_SUPPLY_PROP_SCOPE,
+ POWER_SUPPLY_PROP_VOLTAGE_NOW,
+ POWER_SUPPLY_PROP_CAPACITY,
+};
+
+static int steam_battery_get_property(struct power_supply *psy,
+ enum power_supply_property psp,
+ union power_supply_propval *val)
+{
+ struct steam_device *steam = power_supply_get_drvdata(psy);
+ unsigned long flags;
+ s16 volts;
+ u8 batt;
+ int ret = 0;
+
+ spin_lock_irqsave(&steam->lock, flags);
+ volts = steam->voltage;
+ batt = steam->battery_charge;
+ spin_unlock_irqrestore(&steam->lock, flags);
+
+ switch (psp) {
+ case POWER_SUPPLY_PROP_PRESENT:
+ val->intval = 1;
+ break;
+ case POWER_SUPPLY_PROP_SCOPE:
+ val->intval = POWER_SUPPLY_SCOPE_DEVICE;
+ break;
+ case POWER_SUPPLY_PROP_VOLTAGE_NOW:
+ val->intval = volts * 1000; /* mV -> uV */
+ break;
+ case POWER_SUPPLY_PROP_CAPACITY:
+ val->intval = batt;
+ break;
+ default:
+ ret = -EINVAL;
+ break;
+ }
+ return ret;
+}
+
+static int steam_battery_register(struct steam_device *steam)
+{
+ struct power_supply *battery;
+ struct power_supply_config battery_cfg = { .drv_data = steam, };
+ unsigned long flags;
+ int ret;
+
+ steam->battery_desc.type = POWER_SUPPLY_TYPE_BATTERY;
+ steam->battery_desc.properties = steam_battery_props;
+ steam->battery_desc.num_properties = ARRAY_SIZE(steam_battery_props);
+ steam->battery_desc.get_property = steam_battery_get_property;
+ steam->battery_desc.name = devm_kasprintf(&steam->hdev->dev,
+ GFP_KERNEL, "steam-controller-%s-battery",
+ steam->serial_no);
+ if (!steam->battery_desc.name)
+ return -ENOMEM;
+
+ /* avoid the warning of 0% battery while waiting for the first info */
+ spin_lock_irqsave(&steam->lock, flags);
+ steam->voltage = 3000;
+ steam->battery_charge = 100;
+ spin_unlock_irqrestore(&steam->lock, flags);
+
+ battery = power_supply_register(&steam->hdev->dev,
+ &steam->battery_desc, &battery_cfg);
+ if (IS_ERR(battery)) {
+ ret = PTR_ERR(battery);
+ hid_err(steam->hdev,
+ "%s:power_supply_register failed with error %d\n",
+ __func__, ret);
+ return ret;
+ }
+ rcu_assign_pointer(steam->battery, battery);
+ power_supply_powers(battery, &steam->hdev->dev);
+ return 0;
+}
+
static int steam_register(struct steam_device *steam)
{
struct hid_device *hdev = steam->hdev;
@@ -255,6 +339,10 @@ static int steam_register(struct steam_device *steam)
rcu_assign_pointer(steam->input, input);
+ /* ignore battery errors, we can live without it */
+ if (steam->quirks & STEAM_QUIRK_WIRELESS)
+ steam_battery_register(steam);
+
return 0;
input_register_fail:
@@ -265,11 +353,18 @@ static int steam_register(struct steam_device *steam)
static void steam_unregister(struct steam_device *steam)
{
struct input_dev *input;
+ struct power_supply *battery;
rcu_read_lock();
input = rcu_dereference(steam->input);
+ battery = rcu_dereference(steam->battery);
rcu_read_unlock();
+ if (battery) {
+ RCU_INIT_POINTER(steam->battery, NULL);
+ synchronize_rcu();
+ power_supply_unregister(battery);
+ }
if (input) {
RCU_INIT_POINTER(steam->input, NULL);
synchronize_rcu();
@@ -565,12 +660,44 @@ static void steam_do_input_event(struct steam_device *steam,
input_sync(input);
}
+/*
+ * The size for this message payload is 11.
+ * The known values are:
+ * Offset| Type | Meaning
+ * -------+-------+---------------------------
+ * 4-7 | u32 | sequence number
+ * 8-11 | -- | always 0
+ * 12-13 | u16 | voltage (mV)
+ * 14 | u8 | battery percent
+ */
+static void steam_do_battery_event(struct steam_device *steam,
+ struct power_supply *battery, u8 *data)
+{
+ unsigned long flags;
+
+ s16 volts = le16_to_cpup((__le16 *)(data + 12));
+ u8 batt = data[14];
+
+ /* Creating the battery may have failed */
+ rcu_read_lock();
+ battery = rcu_dereference(steam->battery);
+ if (likely(battery)) {
+ spin_lock_irqsave(&steam->lock, flags);
+ steam->voltage = volts;
+ steam->battery_charge = batt;
+ spin_unlock_irqrestore(&steam->lock, flags);
+ power_supply_changed(battery);
+ }
+ rcu_read_unlock();
+}
+
static int steam_raw_event(struct hid_device *hdev,
struct hid_report *report, u8 *data,
int size)
{
struct steam_device *steam = hid_get_drvdata(hdev);
struct input_dev *input;
+ struct power_supply *battery;
if (!steam)
return 0;
@@ -623,7 +750,19 @@ static int steam_raw_event(struct hid_device *hdev,
}
break;
case 0x04:
- /* TODO battery status */
+ if (steam->quirks & STEAM_QUIRK_WIRELESS) {
+ rcu_read_lock();
+ battery = rcu_dereference(steam->battery);
+ if (likely(battery)) {
+ steam_do_battery_event(steam, battery, data);
+ } else {
+ dbg_hid(
+ "%s: battery data without connect event\n",
+ __func__);
+ steam_do_connect_event(steam, true);
+ }
+ rcu_read_unlock();
+ }
break;
}
return 0;
--
2.16.2
^ permalink raw reply related
* [PATCH v5 3/4] HID: steam: command to check wireless connection
From: Rodrigo Rivas Costa @ 2018-03-11 19:58 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Pierre-Loup A. Griffais,
Cameron Gutman, Clément VUCHENER, linux-kernel, linux-input
Cc: Rodrigo Rivas Costa
In-Reply-To: <20180311195842.5551-1-rodrigorivascosta@gmail.com>
The wireless adaptor does not tell if a device is already connected when
steam_probe() is run.
Use a command to request the connection status.
Signed-off-by: Rodrigo Rivas Costa <rodrigorivascosta@gmail.com>
---
drivers/hid/hid-steam.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
index 03eebc4c28ba..3d34716ae55f 100644
--- a/drivers/hid/hid-steam.c
+++ b/drivers/hid/hid-steam.c
@@ -118,6 +118,11 @@ static int steam_send_report(struct steam_device *steam,
return ret;
}
+static inline int steam_send_report_byte(struct steam_device *steam, u8 cmd)
+{
+ return steam_send_report(steam, &cmd, 1);
+}
+
static int steam_get_serial(struct steam_device *steam)
{
/*
@@ -139,6 +144,16 @@ static int steam_get_serial(struct steam_device *steam)
return 0;
}
+/*
+ * This command requests the wireless adaptor to post an event
+ * with the connection status. Useful if this driver is loaded when
+ * the controller is already connected.
+ */
+static inline int steam_request_conn_status(struct steam_device *steam)
+{
+ return steam_send_report_byte(steam, 0xb4);
+}
+
static int steam_input_open(struct input_dev *dev)
{
struct steam_device *steam = input_get_drvdata(dev);
@@ -360,6 +375,7 @@ static int steam_probe(struct hid_device *hdev,
goto hid_hw_open_fail;
}
hid_info(hdev, "Steam wireless receiver connected");
+ steam_request_conn_status(steam);
} else {
ret = steam_register(steam);
if (ret) {
--
2.16.2
^ permalink raw reply related
* [PATCH v5 2/4] HID: steam: add serial number information.
From: Rodrigo Rivas Costa @ 2018-03-11 19:58 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Pierre-Loup A. Griffais,
Cameron Gutman, Clément VUCHENER, linux-kernel, linux-input
Cc: Rodrigo Rivas Costa
In-Reply-To: <20180311195842.5551-1-rodrigorivascosta@gmail.com>
This device has a feature report to send and receive commands.
Use it to get the serial number and set the device's uniq value.
Signed-off-by: Rodrigo Rivas Costa <rodrigorivascosta@gmail.com>
---
drivers/hid/hid-steam.c | 108 ++++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 105 insertions(+), 3 deletions(-)
diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
index bc8b706311a9..03eebc4c28ba 100644
--- a/drivers/hid/hid-steam.c
+++ b/drivers/hid/hid-steam.c
@@ -18,6 +18,7 @@
#include <linux/module.h>
#include <linux/workqueue.h>
#include <linux/rcupdate.h>
+#include <linux/delay.h>
#include "hid-ids.h"
MODULE_LICENSE("GPL");
@@ -25,6 +26,7 @@ MODULE_AUTHOR("Rodrigo Rivas Costa <rodrigorivascosta@gmail.com>");
#define STEAM_QUIRK_WIRELESS BIT(0)
+#define STEAM_SERIAL_LEN 10
/* Touch pads are 40 mm in diameter and 65535 units */
#define STEAM_PAD_RESOLUTION 1638
/* Trigger runs are about 5 mm and 256 units */
@@ -41,8 +43,102 @@ struct steam_device {
unsigned long quirks;
struct work_struct work_connect;
bool connected;
+ char serial_no[STEAM_SERIAL_LEN + 1];
};
+static int steam_recv_report(struct steam_device *steam,
+ u8 *data, int size)
+{
+ struct hid_report *r;
+ u8 *buf;
+ int ret;
+
+ r = steam->hdev->report_enum[HID_FEATURE_REPORT].report_id_hash[0];
+ if (hid_report_len(r) < 64)
+ return -EINVAL;
+
+ buf = hid_alloc_report_buf(r, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ /*
+ * The report ID is always 0, so strip the first byte from the output.
+ * hid_report_len() is not counting the report ID, so +1 to the length
+ * or else we get a EOVERFLOW. We are safe from a buffer overflow
+ * because hid_alloc_report_buf() allocates +7 bytes.
+ */
+ ret = hid_hw_raw_request(steam->hdev, 0x00,
+ buf, hid_report_len(r) + 1,
+ HID_FEATURE_REPORT, HID_REQ_GET_REPORT);
+ if (ret > 0)
+ memcpy(data, buf + 1, min(size, ret - 1));
+ kfree(buf);
+ return ret;
+}
+
+static int steam_send_report(struct steam_device *steam,
+ u8 *cmd, int size)
+{
+ struct hid_report *r;
+ u8 *buf;
+ unsigned int retries = 10;
+ int ret;
+
+ r = steam->hdev->report_enum[HID_FEATURE_REPORT].report_id_hash[0];
+ if (hid_report_len(r) < 64)
+ return -EINVAL;
+
+ buf = hid_alloc_report_buf(r, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ /* The report ID is always 0 */
+ memcpy(buf + 1, cmd, size);
+
+ /*
+ * Sometimes the wireless controller fails with EPIPE
+ * when sending a feature report.
+ * Doing a HID_REQ_GET_REPORT and waiting for a while
+ * seems to fix that.
+ */
+ do {
+ ret = hid_hw_raw_request(steam->hdev, 0,
+ buf, size + 1,
+ HID_FEATURE_REPORT, HID_REQ_SET_REPORT);
+ if (ret != -EPIPE)
+ break;
+ steam_recv_report(steam, NULL, 0);
+ msleep(50);
+ } while (--retries);
+
+ kfree(buf);
+ if (ret < 0)
+ hid_err(steam->hdev, "%s: error %d (%*ph)\n", __func__,
+ ret, size, cmd);
+ return ret;
+}
+
+static int steam_get_serial(struct steam_device *steam)
+{
+ /*
+ * Send: 0xae 0x15 0x01
+ * Recv: 0xae 0x15 0x01 serialnumber (10 chars)
+ */
+ int ret;
+ u8 cmd[] = {0xae, 0x15, 0x01};
+ u8 reply[3 + STEAM_SERIAL_LEN + 1];
+
+ ret = steam_send_report(steam, cmd, sizeof(cmd));
+ if (ret < 0)
+ return ret;
+ ret = steam_recv_report(steam, reply, sizeof(reply));
+ if (ret < 0)
+ return ret;
+ reply[3 + STEAM_SERIAL_LEN] = 0;
+ strlcpy(steam->serial_no, reply + 3, sizeof(steam->serial_no));
+ return 0;
+}
+
static int steam_input_open(struct input_dev *dev)
{
struct steam_device *steam = input_get_drvdata(dev);
@@ -71,7 +167,12 @@ static int steam_register(struct steam_device *steam)
return 0;
}
- hid_info(hdev, "Steam Controller connected");
+ ret = steam_get_serial(steam);
+ if (ret)
+ return ret;
+
+ hid_info(hdev, "Steam Controller '%s' connected",
+ steam->serial_no);
input = input_allocate_device();
if (!input)
@@ -86,7 +187,7 @@ static int steam_register(struct steam_device *steam)
"Wireless Steam Controller" :
"Steam Controller";
input->phys = hdev->phys;
- input->uniq = hdev->uniq;
+ input->uniq = steam->serial_no;
input->id.bustype = hdev->bus;
input->id.vendor = hdev->vendor;
input->id.product = hdev->product;
@@ -157,7 +258,8 @@ static void steam_unregister(struct steam_device *steam)
if (input) {
RCU_INIT_POINTER(steam->input, NULL);
synchronize_rcu();
- hid_info(steam->hdev, "Steam Controller disconnected");
+ hid_info(steam->hdev, "Steam Controller '%s' disconnected",
+ steam->serial_no);
input_unregister_device(input);
}
}
--
2.16.2
^ permalink raw reply related
* [PATCH v5 1/4] HID: add driver for Valve Steam Controller
From: Rodrigo Rivas Costa @ 2018-03-11 19:58 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Pierre-Loup A. Griffais,
Cameron Gutman, Clément VUCHENER, linux-kernel, linux-input
Cc: Rodrigo Rivas Costa
In-Reply-To: <20180311195842.5551-1-rodrigorivascosta@gmail.com>
There are two ways to connect the Steam Controller: directly to the USB
or with the USB wireless adapter. Both methods are similar, but the
wireless adapter can connect up to 4 devices at the same time.
The wired device will appear as 3 interfaces: a virtual mouse, a virtual
keyboard and a custom HID device.
The wireless device will appear as 5 interfaces: a virtual keyboard and
4 custom HID devices, that will remain silent until a device is actually
connected.
The custom HID device has a report descriptor with all vendor specific
usages, so the hid-generic is not very useful. In a PC/SteamBox Valve
Steam Client provices a software translation by using direct USB access
and a creates a uinput virtual gamepad.
This driver was reverse engineered to provide direct kernel support in
case you cannot, or do not want to, use Valve Steam Client. It disables
the virtual keyboard and mouse, as they are not so useful when you have
a working gamepad.
Working: buttons, axes, pads, wireless connect/disconnect.
TO-DO: Battery, force-feedback, accelerometer/gyro, led, beeper...
Signed-off-by: Rodrigo Rivas Costa <rodrigorivascosta@gmail.com>
---
drivers/hid/Kconfig | 8 +
drivers/hid/Makefile | 1 +
drivers/hid/hid-ids.h | 4 +
drivers/hid/hid-steam.c | 537 ++++++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 550 insertions(+)
create mode 100644 drivers/hid/hid-steam.c
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 19c499f5623d..6e80fbf04e03 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -823,6 +823,14 @@ config HID_SPEEDLINK
---help---
Support for Speedlink Vicious and Divine Cezanne mouse.
+config HID_STEAM
+ tristate "Steam Controller support"
+ depends on HID
+ ---help---
+ Say Y here if you have a Steam Controller if you want to use it
+ without running the Steam Client. It supports both the wired and
+ the wireless adaptor.
+
config HID_STEELSERIES
tristate "Steelseries SRW-S1 steering wheel support"
depends on HID
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index eb13b9e92d85..60a8abf84682 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -95,6 +95,7 @@ obj-$(CONFIG_HID_SAMSUNG) += hid-samsung.o
obj-$(CONFIG_HID_SMARTJOYPLUS) += hid-sjoy.o
obj-$(CONFIG_HID_SONY) += hid-sony.o
obj-$(CONFIG_HID_SPEEDLINK) += hid-speedlink.o
+obj-$(CONFIG_HID_STEAM) += hid-steam.o
obj-$(CONFIG_HID_STEELSERIES) += hid-steelseries.o
obj-$(CONFIG_HID_SUNPLUS) += hid-sunplus.o
obj-$(CONFIG_HID_GREENASIA) += hid-gaff.o
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 43ddcdfbd0da..be31a3c20818 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -988,6 +988,10 @@
#define USB_VENDOR_ID_STANTUM_SITRONIX 0x1403
#define USB_DEVICE_ID_MTP_SITRONIX 0x5001
+#define USB_VENDOR_ID_VALVE 0x28de
+#define USB_DEVICE_ID_STEAM_CONTROLLER 0x1102
+#define USB_DEVICE_ID_STEAM_CONTROLLER_WIRELESS 0x1142
+
#define USB_VENDOR_ID_STEELSERIES 0x1038
#define USB_DEVICE_ID_STEELSERIES_SRWS1 0x1410
diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
new file mode 100644
index 000000000000..bc8b706311a9
--- /dev/null
+++ b/drivers/hid/hid-steam.c
@@ -0,0 +1,537 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * HID driver for Valve Steam Controller
+ *
+ * Copyright (c) 2018 Rodrigo Rivas Costa <rodrigorivascosta@gmail.com>
+ *
+ * Supports both the wired and wireless interfaces.
+ *
+ * For additional functions, such as disabling the virtual mouse/keyboard or
+ * changing the right-pad margin you can use the user-space tool at:
+ *
+ * https://github.com/rodrigorc/steamctrl
+ */
+
+#include <linux/device.h>
+#include <linux/input.h>
+#include <linux/hid.h>
+#include <linux/module.h>
+#include <linux/workqueue.h>
+#include <linux/rcupdate.h>
+#include "hid-ids.h"
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Rodrigo Rivas Costa <rodrigorivascosta@gmail.com>");
+
+#define STEAM_QUIRK_WIRELESS BIT(0)
+
+/* Touch pads are 40 mm in diameter and 65535 units */
+#define STEAM_PAD_RESOLUTION 1638
+/* Trigger runs are about 5 mm and 256 units */
+#define STEAM_TRIGGER_RESOLUTION 51
+/* Joystick runs are about 5 mm and 256 units */
+#define STEAM_JOYSTICK_RESOLUTION 51
+
+#define STEAM_PAD_FUZZ 256
+
+struct steam_device {
+ spinlock_t lock;
+ struct hid_device *hdev;
+ struct input_dev __rcu *input;
+ unsigned long quirks;
+ struct work_struct work_connect;
+ bool connected;
+};
+
+static int steam_input_open(struct input_dev *dev)
+{
+ struct steam_device *steam = input_get_drvdata(dev);
+
+ return hid_hw_open(steam->hdev);
+}
+
+static void steam_input_close(struct input_dev *dev)
+{
+ struct steam_device *steam = input_get_drvdata(dev);
+
+ hid_hw_close(steam->hdev);
+}
+
+static int steam_register(struct steam_device *steam)
+{
+ struct hid_device *hdev = steam->hdev;
+ struct input_dev *input;
+ int ret;
+
+ rcu_read_lock();
+ input = rcu_dereference(steam->input);
+ rcu_read_unlock();
+ if (input) {
+ dbg_hid("%s: already connected\n", __func__);
+ return 0;
+ }
+
+ hid_info(hdev, "Steam Controller connected");
+
+ input = input_allocate_device();
+ if (!input)
+ return -ENOMEM;
+
+ input_set_drvdata(input, steam);
+ input->dev.parent = &hdev->dev;
+ input->open = steam_input_open;
+ input->close = steam_input_close;
+
+ input->name = (steam->quirks & STEAM_QUIRK_WIRELESS) ?
+ "Wireless Steam Controller" :
+ "Steam Controller";
+ input->phys = hdev->phys;
+ input->uniq = hdev->uniq;
+ input->id.bustype = hdev->bus;
+ input->id.vendor = hdev->vendor;
+ input->id.product = hdev->product;
+ input->id.version = hdev->version;
+
+ input_set_capability(input, EV_KEY, BTN_TR2);
+ input_set_capability(input, EV_KEY, BTN_TL2);
+ input_set_capability(input, EV_KEY, BTN_TR);
+ input_set_capability(input, EV_KEY, BTN_TL);
+ input_set_capability(input, EV_KEY, BTN_Y);
+ input_set_capability(input, EV_KEY, BTN_B);
+ input_set_capability(input, EV_KEY, BTN_X);
+ input_set_capability(input, EV_KEY, BTN_A);
+ input_set_capability(input, EV_KEY, BTN_SELECT);
+ input_set_capability(input, EV_KEY, BTN_MODE);
+ input_set_capability(input, EV_KEY, BTN_START);
+ input_set_capability(input, EV_KEY, BTN_GEAR_DOWN);
+ input_set_capability(input, EV_KEY, BTN_GEAR_UP);
+ input_set_capability(input, EV_KEY, BTN_THUMBR);
+ input_set_capability(input, EV_KEY, BTN_THUMBL);
+ input_set_capability(input, EV_KEY, BTN_THUMB);
+ input_set_capability(input, EV_KEY, BTN_THUMB2);
+
+ input_set_abs_params(input, ABS_Z, 0, 255, 0, 0);
+ input_set_abs_params(input, ABS_RZ, 0, 255, 0, 0);
+ input_set_abs_params(input, ABS_X, -32767, 32767, 0, 0);
+ input_set_abs_params(input, ABS_Y, -32767, 32767, 0, 0);
+ input_set_abs_params(input, ABS_RX, -32767, 32767,
+ STEAM_PAD_FUZZ, 0);
+ input_set_abs_params(input, ABS_RY, -32767, 32767,
+ STEAM_PAD_FUZZ, 0);
+ input_set_abs_params(input, ABS_HAT1X, -32767, 32767,
+ STEAM_PAD_FUZZ, 0);
+ input_set_abs_params(input, ABS_HAT1Y, -32767, 32767,
+ STEAM_PAD_FUZZ, 0);
+ input_set_abs_params(input, ABS_HAT0X, -1, 1, 0, 0);
+ input_set_abs_params(input, ABS_HAT0Y, -1, 1, 0, 0);
+ input_abs_set_res(input, ABS_X, STEAM_JOYSTICK_RESOLUTION);
+ input_abs_set_res(input, ABS_Y, STEAM_JOYSTICK_RESOLUTION);
+ input_abs_set_res(input, ABS_RX, STEAM_PAD_RESOLUTION);
+ input_abs_set_res(input, ABS_RY, STEAM_PAD_RESOLUTION);
+ input_abs_set_res(input, ABS_HAT1X, STEAM_PAD_RESOLUTION);
+ input_abs_set_res(input, ABS_HAT1Y, STEAM_PAD_RESOLUTION);
+ input_abs_set_res(input, ABS_Z, STEAM_TRIGGER_RESOLUTION);
+ input_abs_set_res(input, ABS_RZ, STEAM_TRIGGER_RESOLUTION);
+
+ ret = input_register_device(input);
+ if (ret)
+ goto input_register_fail;
+
+ rcu_assign_pointer(steam->input, input);
+
+ return 0;
+
+input_register_fail:
+ input_free_device(input);
+ return ret;
+}
+
+static void steam_unregister(struct steam_device *steam)
+{
+ struct input_dev *input;
+
+ rcu_read_lock();
+ input = rcu_dereference(steam->input);
+ rcu_read_unlock();
+
+ if (input) {
+ RCU_INIT_POINTER(steam->input, NULL);
+ synchronize_rcu();
+ hid_info(steam->hdev, "Steam Controller disconnected");
+ input_unregister_device(input);
+ }
+}
+
+static void steam_work_connect_cb(struct work_struct *work)
+{
+ struct steam_device *steam = container_of(work, struct steam_device,
+ work_connect);
+ unsigned long flags;
+ bool connected;
+ int ret;
+
+ spin_lock_irqsave(&steam->lock, flags);
+ connected = steam->connected;
+ spin_unlock_irqrestore(&steam->lock, flags);
+
+ if (connected) {
+ ret = steam_register(steam);
+ if (ret) {
+ hid_err(steam->hdev,
+ "%s:steam_register failed with error %d\n",
+ __func__, ret);
+ }
+ } else {
+ steam_unregister(steam);
+ }
+}
+
+static bool steam_is_valve_interface(struct hid_device *hdev)
+{
+ struct hid_report_enum *rep_enum;
+
+ /*
+ * The wired device creates 3 interfaces:
+ * 0: emulated mouse.
+ * 1: emulated keyboard.
+ * 2: the real game pad.
+ * The wireless device creates 5 interfaces:
+ * 0: emulated keyboard.
+ * 1-4: slots where up to 4 real game pads will be connected to.
+ * We know which one is the real gamepad interface because they are the
+ * only ones with a feature report.
+ */
+ rep_enum = &hdev->report_enum[HID_FEATURE_REPORT];
+ return !list_empty(&rep_enum->report_list);
+}
+
+static int steam_probe(struct hid_device *hdev,
+ const struct hid_device_id *id)
+{
+ struct steam_device *steam;
+ int ret;
+
+ ret = hid_parse(hdev);
+ if (ret) {
+ hid_err(hdev,
+ "%s:parse of hid interface failed\n", __func__);
+ return ret;
+ }
+
+ /*
+ * Connect the HID device so that Steam Controller keeps on working
+ * without changes.
+ */
+ ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
+ if (ret) {
+ hid_err(hdev,
+ "%s:hid_hw_start failed with error %d\n",
+ __func__, ret);
+ return ret;
+ }
+
+ if (!steam_is_valve_interface(hdev))
+ return 0;
+
+ /*
+ * From this point on, if anything fails return 0 and ignores
+ * the error, so that the default HID devices are still bound.
+ */
+ steam = devm_kzalloc(&hdev->dev, sizeof(*steam), GFP_KERNEL);
+ if (!steam) {
+ ret = -ENOMEM;
+ goto mem_fail;
+ }
+
+ spin_lock_init(&steam->lock);
+ steam->hdev = hdev;
+ hid_set_drvdata(hdev, steam);
+ steam->quirks = id->driver_data;
+ INIT_WORK(&steam->work_connect, steam_work_connect_cb);
+
+ if (steam->quirks & STEAM_QUIRK_WIRELESS) {
+ ret = hid_hw_open(hdev);
+ if (ret) {
+ hid_err(hdev,
+ "%s:hid_hw_open for wireless\n",
+ __func__);
+ goto hid_hw_open_fail;
+ }
+ hid_info(hdev, "Steam wireless receiver connected");
+ } else {
+ ret = steam_register(steam);
+ if (ret) {
+ hid_err(hdev,
+ "%s:steam_register failed with error %d\n",
+ __func__, ret);
+ goto input_register_fail;
+ }
+ }
+
+ return 0;
+
+input_register_fail:
+hid_hw_open_fail:
+ cancel_work_sync(&steam->work_connect);
+ hid_set_drvdata(hdev, NULL);
+mem_fail:
+ hid_err(hdev, "%s: failed with error %d\n",
+ __func__, ret);
+ return 0;
+}
+
+static void steam_remove(struct hid_device *hdev)
+{
+ struct steam_device *steam = hid_get_drvdata(hdev);
+
+ if (!steam) {
+ hid_hw_stop(hdev);
+ return;
+ }
+
+ if (steam->quirks & STEAM_QUIRK_WIRELESS) {
+ hid_info(hdev, "Steam wireless receiver disconnected");
+ hid_hw_close(hdev);
+ }
+ hid_hw_stop(hdev);
+ cancel_work_sync(&steam->work_connect);
+ steam_unregister(steam);
+}
+
+static void steam_do_connect_event(struct steam_device *steam, bool connected)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&steam->lock, flags);
+ steam->connected = connected;
+ spin_unlock_irqrestore(&steam->lock, flags);
+
+ if (schedule_work(&steam->work_connect) == 0)
+ dbg_hid("%s: connected=%d event already queued\n",
+ __func__, connected);
+}
+
+/*
+ * The size for this message payload is 60.
+ * The known values are:
+ * (* values are not sent through wireless)
+ * (* accelerator/gyro is disabled by default)
+ * Offset| Type | Mapped to |Meaning
+ * -------+-------+-----------+--------------------------
+ * 4-7 | u32 | -- | sequence number
+ * 8-10 | 24bit | see below | buttons
+ * 11 | u8 | ABS_Z | left trigger
+ * 12 | u8 | ABS_RZ | right trigger
+ * 13-15 | -- | -- | always 0
+ * 16-17 | s16 | ABS_X | X value
+ * 18-19 | s16 | ABS_Y | Y value
+ * 20-21 | s16 | ABS_RX | right-pad X value
+ * 22-23 | s16 | ABS_RY | right-pad Y value
+ * 24-25 | s16 | -- | * left trigger
+ * 26-27 | s16 | -- | * right trigger
+ * 28-29 | s16 | -- | * accelerometer X value
+ * 30-31 | s16 | -- | * accelerometer Y value
+ * 32-33 | s16 | -- | * accelerometer Z value
+ * 34-35 | s16 | -- | gyro X value
+ * 36-36 | s16 | -- | gyro Y value
+ * 38-39 | s16 | -- | gyro Z value
+ * 40-41 | s16 | -- | quaternion W value
+ * 42-43 | s16 | -- | quaternion X value
+ * 44-45 | s16 | -- | quaternion Y value
+ * 46-47 | s16 | -- | quaternion Z value
+ * 48-49 | -- | -- | always 0
+ * 50-51 | s16 | -- | * left trigger (uncalibrated)
+ * 52-53 | s16 | -- | * right trigger (uncalibrated)
+ * 54-55 | s16 | -- | * joystick X value (uncalibrated)
+ * 56-57 | s16 | -- | * joystick Y value (uncalibrated)
+ * 58-59 | s16 | -- | * left-pad X value
+ * 60-61 | s16 | -- | * left-pad Y value
+ * 62-63 | u16 | -- | * battery voltage
+ *
+ * The buttons are:
+ * Bit | Mapped to | Description
+ * ------+------------+--------------------------------
+ * 8.0 | BTN_TR2 | right trigger fully pressed
+ * 8.1 | BTN_TL2 | left trigger fully pressed
+ * 8.2 | BTN_TR | right shoulder
+ * 8.3 | BTN_TL | left shoulder
+ * 8.4 | BTN_Y | button Y
+ * 8.5 | BTN_B | button B
+ * 8.6 | BTN_X | button X
+ * 8.7 | BTN_A | button A
+ * 9.0 | -ABS_HAT0Y | lef-pad up
+ * 9.1 | +ABS_HAT0X | lef-pad right
+ * 9.2 | -ABS_HAT0X | lef-pad left
+ * 9.3 | +ABS_HAT0Y | lef-pad down
+ * 9.4 | BTN_SELECT | menu left
+ * 9.5 | BTN_MODE | steam logo
+ * 9.6 | BTN_START | menu right
+ * 9.7 | BTN_GEAR_DOWN | left back lever
+ * 10.0 | BTN_GEAR_UP | right back lever
+ * 10.1 | -- | left-pad clicked
+ * 10.2 | BTN_THUMBR | right-pad clicked
+ * 10.3 | BTN_THUMB | left-pad touched (but see explanation below)
+ * 10.4 | BTN_THUMB2 | right-pad touched
+ * 10.5 | -- | unknown
+ * 10.6 | BTN_THUMBL | joystick clicked
+ * 10.7 | -- | lpad_and_joy
+ */
+
+static void steam_do_input_event(struct steam_device *steam,
+ struct input_dev *input, u8 *data)
+{
+ /* 24 bits of buttons */
+ u8 b8, b9, b10;
+ bool lpad_touched, lpad_and_joy;
+
+ b8 = data[8];
+ b9 = data[9];
+ b10 = data[10];
+
+ input_report_abs(input, ABS_Z, data[11]);
+ input_report_abs(input, ABS_RZ, data[12]);
+
+ /*
+ * These two bits tells how to interpret the values X and Y.
+ * lpad_and_joy tells that the joystick and the lpad are used at the
+ * same time.
+ * lpad_touched tells whether X/Y are to be read as lpad coord or
+ * joystick values.
+ * (lpad_touched || lpad_and_joy) tells if the lpad is really touched.
+ */
+ lpad_touched = b10 & BIT(3);
+ lpad_and_joy = b10 & BIT(7);
+ input_report_abs(input, lpad_touched ? ABS_HAT1X : ABS_X,
+ (s16) le16_to_cpup((__le16 *)(data + 16)));
+ input_report_abs(input, lpad_touched ? ABS_HAT1Y : ABS_Y,
+ -(s16) le16_to_cpup((__le16 *)(data + 18)));
+ /* Check if joystick is centered */
+ if (lpad_touched && !lpad_and_joy) {
+ input_report_abs(input, ABS_X, 0);
+ input_report_abs(input, ABS_Y, 0);
+ }
+ /* Check if lpad is untouched */
+ if (!(lpad_touched || lpad_and_joy)) {
+ input_report_abs(input, ABS_HAT1X, 0);
+ input_report_abs(input, ABS_HAT1Y, 0);
+ }
+
+ input_report_abs(input, ABS_RX,
+ (s16) le16_to_cpup((__le16 *)(data + 20)));
+ input_report_abs(input, ABS_RY,
+ -(s16) le16_to_cpup((__le16 *)(data + 22)));
+
+ input_event(input, EV_KEY, BTN_TR2, !!(b8 & BIT(0)));
+ input_event(input, EV_KEY, BTN_TL2, !!(b8 & BIT(1)));
+ input_event(input, EV_KEY, BTN_TR, !!(b8 & BIT(2)));
+ input_event(input, EV_KEY, BTN_TL, !!(b8 & BIT(3)));
+ input_event(input, EV_KEY, BTN_Y, !!(b8 & BIT(4)));
+ input_event(input, EV_KEY, BTN_B, !!(b8 & BIT(5)));
+ input_event(input, EV_KEY, BTN_X, !!(b8 & BIT(6)));
+ input_event(input, EV_KEY, BTN_A, !!(b8 & BIT(7)));
+ input_event(input, EV_KEY, BTN_SELECT, !!(b9 & BIT(4)));
+ input_event(input, EV_KEY, BTN_MODE, !!(b9 & BIT(5)));
+ input_event(input, EV_KEY, BTN_START, !!(b9 & BIT(6)));
+ input_event(input, EV_KEY, BTN_GEAR_DOWN, !!(b9 & BIT(7)));
+ input_event(input, EV_KEY, BTN_GEAR_UP, !!(b10 & BIT(0)));
+ input_event(input, EV_KEY, BTN_THUMBR, !!(b10 & BIT(2)));
+ input_event(input, EV_KEY, BTN_THUMBL, !!(b10 & BIT(6)));
+ input_event(input, EV_KEY, BTN_THUMB, lpad_touched || lpad_and_joy);
+ input_event(input, EV_KEY, BTN_THUMB2, !!(b10 & BIT(4)));
+
+ input_report_abs(input, ABS_HAT0X,
+ !!(b9 & BIT(1)) - !!(b9 & BIT(2)));
+ input_report_abs(input, ABS_HAT0Y,
+ !!(b9 & BIT(3)) - !!(b9 & BIT(0)));
+
+ input_sync(input);
+}
+
+static int steam_raw_event(struct hid_device *hdev,
+ struct hid_report *report, u8 *data,
+ int size)
+{
+ struct steam_device *steam = hid_get_drvdata(hdev);
+ struct input_dev *input;
+
+ if (!steam)
+ return 0;
+
+ /*
+ * All messages are size=64, all values little-endian.
+ * The format is:
+ * Offset| Meaning
+ * -------+--------------------------------------------
+ * 0-1 | always 0x01, 0x00, maybe protocol version?
+ * 2 | type of message
+ * 3 | length of the real payload (not checked)
+ * 4-n | payload data, depends on the type
+ *
+ * There are these known types of message:
+ * 0x01: input data (60 bytes)
+ * 0x03: wireless connect/disconnect (1 byte)
+ * 0x04: battery status (11 bytes)
+ */
+
+ if (size != 64 || data[0] != 1 || data[1] != 0)
+ return 0;
+
+ switch (data[2]) {
+ case 0x01:
+ rcu_read_lock();
+ input = rcu_dereference(steam->input);
+ if (likely(input)) {
+ steam_do_input_event(steam, input, data);
+ } else {
+ dbg_hid("%s: input data without connect event\n",
+ __func__);
+ steam_do_connect_event(steam, true);
+ }
+ rcu_read_unlock();
+ break;
+ case 0x03:
+ /*
+ * The payload of this event is a single byte:
+ * 0x01: disconnected.
+ * 0x02: connected.
+ */
+ switch (data[4]) {
+ case 0x01:
+ steam_do_connect_event(steam, false);
+ break;
+ case 0x02:
+ steam_do_connect_event(steam, true);
+ break;
+ }
+ break;
+ case 0x04:
+ /* TODO battery status */
+ break;
+ }
+ return 0;
+}
+
+static const struct hid_device_id steam_controllers[] = {
+ { /* Wired Steam Controller */
+ HID_USB_DEVICE(USB_VENDOR_ID_VALVE,
+ USB_DEVICE_ID_STEAM_CONTROLLER)
+ },
+ { /* Wireless Steam Controller */
+ HID_USB_DEVICE(USB_VENDOR_ID_VALVE,
+ USB_DEVICE_ID_STEAM_CONTROLLER_WIRELESS),
+ .driver_data = STEAM_QUIRK_WIRELESS
+ },
+ {}
+};
+
+MODULE_DEVICE_TABLE(hid, steam_controllers);
+
+static struct hid_driver steam_controller_driver = {
+ .name = "hid-steam",
+ .id_table = steam_controllers,
+ .probe = steam_probe,
+ .remove = steam_remove,
+ .raw_event = steam_raw_event,
+};
+
+module_hid_driver(steam_controller_driver);
--
2.16.2
^ permalink raw reply related
* [PATCH v5 0/4] new driver for Valve Steam Controller
From: Rodrigo Rivas Costa @ 2018-03-11 19:58 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Pierre-Loup A. Griffais,
Cameron Gutman, Clément VUCHENER, linux-kernel, linux-input
Cc: Rodrigo Rivas Costa
This patchset implements a driver for Valve Steam Controller, based on a
reverse analysis by myself.
Sorry, I've been out of town for a few weeks and couldn't keep up with this...
@Pierre-Loup and @Clément, could you please have another look at this and
check if it is worthy? Benjamin will not commit it without an express ACK from
Valve. Of course he is right to be cautious, but I checked this driver with
the Steam Client and all seems to go just fine. I think that there is a lot of
Linux out of the desktop that could use this driver and cannot use the Steam
Client. Worst case scenario, this driver can now be blacklisted, but I hope
that will not be needed.
For full reference, I'm adding a full changelog of this patchset.
Changes in v5:
* Fix license SPDX to GPL-2.0+.
* Minor stylistic changes (BIT(3) instead 0x08 and so on).
Changes in v4:
* Add command to check the wireless connection status on probe, without
waiting for a message (thanks to Clément Vuchener for the tip).
* Removed the error code on redundant connection/disconnection messages. That
was harmless but polluted dmesg.
* Added buttons for touching the left-pad and right-pad.
* Fixed a misplaced #include from 2/4 to 1/4.
Changes in v3:
* Use RCU to do the dynamic connec/disconnect of wireless devices.
* Remove entries in hid-quirks.c as they are no longer needed. This allows
this module to be blacklisted without side effects.
* Do not bypass the virtual keyboard/mouse HID devices to avoid breaking
existing use cases (lizard mode). A user-space tool to do that is
linked.
* Fully separated axes for joystick and left-pad. As it happens.
* Add fuzz values for left/right pad axes, they are a little wiggly.
Changes in v2:
* Remove references to USB. Now the interesting interfaces are selected by
looking for the ones with feature reports.
* Feature reports buffers are allocated with hid_alloc_report_buf().
* Feature report length is checked, to avoid overflows in case of
corrupt/malicius USB devices.
* Resolution added to the ABS axes.
* A lot of minor cleanups.
Rodrigo Rivas Costa (4):
HID: add driver for Valve Steam Controller
HID: steam: add serial number information.
HID: steam: command to check wireless connection
HID: steam: add battery device.
drivers/hid/Kconfig | 8 +
drivers/hid/Makefile | 1 +
drivers/hid/hid-ids.h | 4 +
drivers/hid/hid-steam.c | 794 ++++++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 807 insertions(+)
create mode 100644 drivers/hid/hid-steam.c
--
2.16.2
^ permalink raw reply
* Re: [PATCH v2 4/4] input: misc: Add Gateworks System Controller support
From: Dmitry Torokhov @ 2018-03-10 18:45 UTC (permalink / raw)
To: Tim Harvey
Cc: Lee Jones, Rob Herring, Mark Rutland, Mark Brown, linux-kernel,
devicetree, linux-arm-kernel, linux-hwmon, linux-input
In-Reply-To: <1520287361-12569-5-git-send-email-tharvey@gateworks.com>
On Mon, Mar 05, 2018 at 02:02:41PM -0800, Tim Harvey wrote:
> Add support for dispatching Linux Input events for the various interrupts
> the Gateworks System Controller provides.
>
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Signed-off-by: Tim Harvey <tharvey@gateworks.com>
> ---
> v2:
> - reword Kconfig
> - revise license comment block
> - remove unnecessary read of status register
> - remove unnecessary setting of input->dev.parent
> - use dt bindings for irq to keycode mapping
> - add support for platform-data
> - remove work-queue
>
> drivers/input/misc/Kconfig | 9 ++
> drivers/input/misc/Makefile | 1 +
> drivers/input/misc/gsc-input.c | 180 ++++++++++++++++++++++++++++++++
> include/linux/platform_data/gsc_input.h | 30 ++++++
> 4 files changed, 220 insertions(+)
> create mode 100644 drivers/input/misc/gsc-input.c
> create mode 100644 include/linux/platform_data/gsc_input.h
>
> diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
> index 9f082a3..e05f4fe 100644
> --- a/drivers/input/misc/Kconfig
> +++ b/drivers/input/misc/Kconfig
> @@ -117,6 +117,15 @@ config INPUT_E3X0_BUTTON
> To compile this driver as a module, choose M here: the
> module will be called e3x0_button.
>
> +config INPUT_GSC
> + tristate "Gateworks System Controller input support"
> + depends on MFD_GSC
> + help
> + Support GSC events as Linux input events.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called gsc_input.
> +
> config INPUT_PCSPKR
> tristate "PC Speaker support"
> depends on PCSPKR_PLATFORM
> diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
> index 4b6118d..969debe 100644
> --- a/drivers/input/misc/Makefile
> +++ b/drivers/input/misc/Makefile
> @@ -38,6 +38,7 @@ obj-$(CONFIG_INPUT_GP2A) += gp2ap002a00f.o
> obj-$(CONFIG_INPUT_GPIO_BEEPER) += gpio-beeper.o
> obj-$(CONFIG_INPUT_GPIO_TILT_POLLED) += gpio_tilt_polled.o
> obj-$(CONFIG_INPUT_GPIO_DECODER) += gpio_decoder.o
> +obj-$(CONFIG_INPUT_GSC) += gsc-input.o
> obj-$(CONFIG_INPUT_HISI_POWERKEY) += hisi_powerkey.o
> obj-$(CONFIG_HP_SDC_RTC) += hp_sdc_rtc.o
> obj-$(CONFIG_INPUT_IMS_PCU) += ims-pcu.o
> diff --git a/drivers/input/misc/gsc-input.c b/drivers/input/misc/gsc-input.c
> new file mode 100644
> index 0000000..27b5e93
> --- /dev/null
> +++ b/drivers/input/misc/gsc-input.c
> @@ -0,0 +1,180 @@
> +/* SPDX-License-Identifier: GPL-2.0
> + *
> + * Copyright (C) 2018 Gateworks Corporation
> + *
> + * This driver dispatches Linux input events for GSC interrupt events
> + */
> +#include <linux/input.h>
> +#include <linux/interrupt.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_irq.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +
> +#include <linux/mfd/gsc.h>
> +#include <linux/platform_data/gsc_input.h>
> +
> +struct gsc_input_button_priv {
> + struct input_dev *input;
> + const struct gsc_input_button *button;
> +};
> +
> +struct gsc_input_data {
> + struct gsc_dev *gsc;
> + struct gsc_input_platform_data *pdata;
> + struct input_dev *input;
> + struct gsc_input_button_priv *buttons;
> + int nbuttons;
> + unsigned int irqs[];
> +#if 0
> + int irq;
> + struct work_struct irq_work;
> + struct mutex mutex;
> +#endif
> +};
> +
> +static irqreturn_t gsc_input_irq(int irq, void *data)
> +{
> + const struct gsc_input_button_priv *button_priv = data;
> + struct input_dev *input = button_priv->input;
> +
> + dev_dbg(&input->dev, "irq%d code=%d\n", irq, button_priv->button->code);
> + input_report_key(input, button_priv->button->code, 1);
> + input_sync(input);
> + input_report_key(input, button_priv->button->code, 0);
> + input_sync(input);
> +
> + return IRQ_HANDLED;
Hmm, so in the end this is just a bunch of buttons connected to
interrupt lines. We already have a driver for that, called gpio-keys. It
can work in pure interrupt mode (i.e. do not need real GPIO pin, just
interrupt, and it will generate key down and up events when interrupt
arrives, with possible delay for the up event).
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v5 1/3] Input: gpio-keys - add support for wakeup event action
From: Andy Shevchenko @ 2018-03-10 18:32 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Jeffy Chen, Linux Kernel Mailing List, Brian Norris,
Heiko Stübner, Dmitry Torokhov, Douglas Anderson, devicetree,
Guenter Roeck, Thomas Gleixner, Joseph Lo, stephen lu,
Rob Herring, Kate Stewart, linux-input, Greg Kroah-Hartman,
Mark Rutland, Philippe Ombredanne, Arvind Yadav
In-Reply-To: <20180310181509.GB260013@dtor-ws>
On Sat, Mar 10, 2018 at 8:15 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> + while (--i >= 0) {
Just in case
while (i--) {
is slightly better to read.
> + bdata = &ddata->data[i];
> + if (bdata->button->wakeup)
> + gpio_keys_button_disable_wakeup(bdata);
> + bdata->suspended = false;
> + }
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH V1] Input: pm8941-pwrkey: add resin key capabilities
From: Dmitry Torokhov @ 2018-03-10 18:27 UTC (permalink / raw)
To: Tirupathi Reddy
Cc: robh+dt, mark.rutland, linux-input, linux-arm-msm, devicetree,
linux-kernel
In-Reply-To: <20180307202147.GB195580@dtor-ws>
On Wed, Mar 07, 2018 at 12:21:47PM -0800, Dmitry Torokhov wrote:
> Hi Tirupathi,
>
> On Wed, Mar 07, 2018 at 01:39:33PM +0530, Tirupathi Reddy wrote:
> > Add resin key support to handle different types of key events
> > defined in different platforms.
Also, when you say "resin" key, I do not suppose it refers to teh
material of the key, but rather have something to so with "reset in"
signal or similar when is not actually used for resetting the device?
> >
> > Signed-off-by: Tirupathi Reddy <tirupath@codeaurora.org>
> > ---
> > .../bindings/input/qcom,pm8941-pwrkey.txt | 20 ++++++-
> > drivers/input/misc/pm8941-pwrkey.c | 63 +++++++++++++++++++++-
> > 2 files changed, 81 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/input/qcom,pm8941-pwrkey.txt b/Documentation/devicetree/bindings/input/qcom,pm8941-pwrkey.txt
> > index 07bf55f..1c437e9 100644
> > --- a/Documentation/devicetree/bindings/input/qcom,pm8941-pwrkey.txt
> > +++ b/Documentation/devicetree/bindings/input/qcom,pm8941-pwrkey.txt
> > @@ -20,6 +20,14 @@ PROPERTIES
> > defined by the binding document describing the node's
> > interrupt parent.
> >
> > +- interrupt-names:
> > + Usage: required
> > + Value type: <stringlist>
> > + Definition: Interrupt names. This list must match up 1-to-1 with the
> > + interrupts specified in the 'interrupts' property. "kpdpwr"
> > + must be specified. "resin" may be specified for some
> > + platforms.
> > +
> > - debounce:
> > Usage: optional
> > Value type: <u32>
> > @@ -32,12 +40,22 @@ PROPERTIES
> > Definition: presence of this property indicates that the KPDPWR_N pin
> > should be configured for pull up.
> >
> > +- linux,code:
> > + Usage: required for "resin" key
> > + Value type: <u32>
> > + Definition: The input key-code associated with the resin key.
> > + Use the linux event codes defined in
> > + include/dt-bindings/input/linux-event-codes.h
> > +
> > EXAMPLE
> >
> > pwrkey@800 {
> > compatible = "qcom,pm8941-pwrkey";
> > reg = <0x800>;
> > - interrupts = <0x0 0x8 0 IRQ_TYPE_EDGE_BOTH>;
> > + interrupts = <0x0 0x8 0 IRQ_TYPE_EDGE_BOTH>,
> > + <0x0 0x8 1 IRQ_TYPE_EDGE_BOTH>;
> > + interrupt-names = "kpdpwr", "resin";
> > debounce = <15625>;
> > bias-pull-up;
> > + linux,code = <KEY_VOLUMEDOWN>;
> > };
> > diff --git a/drivers/input/misc/pm8941-pwrkey.c b/drivers/input/misc/pm8941-pwrkey.c
> > index 18ad956..3f801cf 100644
> > --- a/drivers/input/misc/pm8941-pwrkey.c
> > +++ b/drivers/input/misc/pm8941-pwrkey.c
> > @@ -28,6 +28,7 @@
> >
> > #define PON_RT_STS 0x10
> > #define PON_KPDPWR_N_SET BIT(0)
> > +#define PON_RESIN_N_SET BIT(1)
> >
> > #define PON_PS_HOLD_RST_CTL 0x5a
> > #define PON_PS_HOLD_RST_CTL2 0x5b
> > @@ -46,6 +47,8 @@
> > struct pm8941_pwrkey {
> > struct device *dev;
> > int irq;
> > + int resin_irq;
> > + u32 resin_key_code;
> > u32 baseaddr;
> > struct regmap *regmap;
> > struct input_dev *input;
> > @@ -130,6 +133,24 @@ static irqreturn_t pm8941_pwrkey_irq(int irq, void *_data)
> > return IRQ_HANDLED;
> > }
> >
> > +static irqreturn_t pm8941_resinkey_irq(int irq, void *_data)
> > +{
> > + struct pm8941_pwrkey *pwrkey = _data;
> > + unsigned int sts;
> > + int error;
> > +
> > + error = regmap_read(pwrkey->regmap,
> > + pwrkey->baseaddr + PON_RT_STS, &sts);
> > + if (error)
> > + return IRQ_HANDLED;
>
> It looks like you are reading the same register as the power on key.
> Won't you lose events if both are pressed at the same time?
>
> I think you need a unified interrupt handler...
>
> > +
> > + input_report_key(pwrkey->input, pwrkey->resin_key_code,
> > + !!(sts & PON_RESIN_N_SET));
> > + input_sync(pwrkey->input);
> > +
> > + return IRQ_HANDLED;
> > +}
> > +
> > static int __maybe_unused pm8941_pwrkey_suspend(struct device *dev)
> > {
> > struct pm8941_pwrkey *pwrkey = dev_get_drvdata(dev);
> > @@ -153,6 +174,35 @@ static int __maybe_unused pm8941_pwrkey_resume(struct device *dev)
> > static SIMPLE_DEV_PM_OPS(pm8941_pwr_key_pm_ops,
> > pm8941_pwrkey_suspend, pm8941_pwrkey_resume);
> >
> > +static int pm8941_resin_key_init(struct pm8941_pwrkey *pwrkey)
> > +{
> > + int error;
> > +
> > + /*
> > + * Get the standard-key parameters. This might not be
> > + * specified if there is no key mapping on the reset line.
> > + */
> > + error = of_property_read_u32(pwrkey->dev->of_node, "linux,code",
> > + &pwrkey->resin_key_code);
> > + if (error) {
> > + dev_err(pwrkey->dev, "failed to read key-code for resin key\n");
> > + return error;
> > + }
> > +
> > + /* Register key configuration */
> > + input_set_capability(pwrkey->input, EV_KEY, pwrkey->resin_key_code);
> > +
> > + error = devm_request_threaded_irq(pwrkey->dev, pwrkey->resin_irq,
> > + NULL, pm8941_resinkey_irq,
> > + IRQF_ONESHOT,
> > + "pm8941_resinkey", pwrkey);
> > + if (error)
> > + dev_err(pwrkey->dev, "failed requesting resin key IRQ: %d\n",
> > + error);
> > +
> > + return error;
> > +}
> > +
> > static int pm8941_pwrkey_probe(struct platform_device *pdev)
> > {
> > struct pm8941_pwrkey *pwrkey;
> > @@ -182,7 +232,7 @@ static int pm8941_pwrkey_probe(struct platform_device *pdev)
> > return -ENODEV;
> > }
> >
> > - pwrkey->irq = platform_get_irq(pdev, 0);
> > + pwrkey->irq = platform_get_irq_byname(pdev, "kpdpwr");
>
> You are breaking support for old DTS here I'm afraid.
>
> > if (pwrkey->irq < 0) {
> > dev_err(&pdev->dev, "failed to get irq\n");
> > return pwrkey->irq;
> > @@ -241,6 +291,17 @@ static int pm8941_pwrkey_probe(struct platform_device *pdev)
> > return error;
> > }
> >
> > + pwrkey->resin_irq = platform_get_irq_byname(pdev, "resin");
> > + if (pwrkey->resin_irq >= 0) {
>
> This is not entirely correct. I'd say if you have -ENXIO you can
> conclude that the support for "resin" key is not enabled, but the rest
> of the errors you should report up the stack. Deferrals will be retried
> and other errors are fatal.
>
> > + /* resin key capabilities are defined in device node */
> > + error = pm8941_resin_key_init(pwrkey);
> > + if (error) {
> > + dev_err(&pdev->dev, "failed resin key initialization: %d\n",
> > + error);
> > + return error;
> > + }
> > + }
> > +
> > error = input_register_device(pwrkey->input);
> > if (error) {
> > dev_err(&pdev->dev, "failed to register input device: %d\n",
> > --
> > QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
> >
>
> Thanks.
>
> --
> Dmitry
--
Dmitry
^ permalink raw reply
* Re: [PATCH v2] Input: stmpe-keypad - remove VLA usage
From: Dmitry Torokhov @ 2018-03-10 18:18 UTC (permalink / raw)
To: Gustavo A. R. Silva; +Cc: linux-input, linux-kernel, Gustavo A. R. Silva
In-Reply-To: <20180310033612.GA8010@embeddedgus>
On Fri, Mar 09, 2018 at 09:36:12PM -0600, Gustavo A. R. Silva wrote:
> In preparation to enabling -Wvla, remove VLA and replace it
> with a fixed-length array instead.
>
> Fixed as part of the directive to remove all VLAs from
> the kernel: https://lkml.org/lkml/2018/3/7/621
>
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
Applied, thank you.
> ---
> Changes in v2:
> - Update the code based on Dmitry Torokhov comments. Thanks Dmitry.
>
> drivers/input/keyboard/stmpe-keypad.c | 16 ++++++++++++----
> 1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/input/keyboard/stmpe-keypad.c b/drivers/input/keyboard/stmpe-keypad.c
> index 8c6c0b9..d69e631 100644
> --- a/drivers/input/keyboard/stmpe-keypad.c
> +++ b/drivers/input/keyboard/stmpe-keypad.c
> @@ -48,6 +48,14 @@
> #define STMPE_KEYPAD_KEYMAP_MAX_SIZE \
> (STMPE_KEYPAD_MAX_ROWS * STMPE_KEYPAD_MAX_COLS)
>
> +
> +#define STMPE1601_NUM_DATA 5
> +#define STMPE2401_NUM_DATA 3
> +#define STMPE2403_NUM_DATA 5
> +
> +/* Make sure it covers all cases above */
> +#define MAX_NUM_DATA 5
> +
> /**
> * struct stmpe_keypad_variant - model-specific attributes
> * @auto_increment: whether the KPC_DATA_BYTE register address
> @@ -74,7 +82,7 @@ struct stmpe_keypad_variant {
> static const struct stmpe_keypad_variant stmpe_keypad_variants[] = {
> [STMPE1601] = {
> .auto_increment = true,
> - .num_data = 5,
> + .num_data = STMPE1601_NUM_DATA,
> .num_normal_data = 3,
> .max_cols = 8,
> .max_rows = 8,
> @@ -84,7 +92,7 @@ static const struct stmpe_keypad_variant stmpe_keypad_variants[] = {
> [STMPE2401] = {
> .auto_increment = false,
> .set_pullup = true,
> - .num_data = 3,
> + .num_data = STMPE2401_NUM_DATA,
> .num_normal_data = 2,
> .max_cols = 8,
> .max_rows = 12,
> @@ -94,7 +102,7 @@ static const struct stmpe_keypad_variant stmpe_keypad_variants[] = {
> [STMPE2403] = {
> .auto_increment = true,
> .set_pullup = true,
> - .num_data = 5,
> + .num_data = STMPE2403_NUM_DATA,
> .num_normal_data = 3,
> .max_cols = 8,
> .max_rows = 12,
> @@ -156,7 +164,7 @@ static irqreturn_t stmpe_keypad_irq(int irq, void *dev)
> struct stmpe_keypad *keypad = dev;
> struct input_dev *input = keypad->input;
> const struct stmpe_keypad_variant *variant = keypad->variant;
> - u8 fifo[variant->num_data];
> + u8 fifo[MAX_NUM_DATA];
> int ret;
> int i;
>
> --
> 2.7.4
>
--
Dmitry
^ permalink raw reply
* Re: [PATCH v5 1/3] Input: gpio-keys - add support for wakeup event action
From: Dmitry Torokhov @ 2018-03-10 18:15 UTC (permalink / raw)
To: Jeffy Chen
Cc: linux-kernel, briannorris, heiko, dtor, dianders, devicetree,
Guenter Roeck, Thomas Gleixner, Joseph Lo, stephen lu,
Rob Herring, Kate Stewart, linux-input, Greg Kroah-Hartman,
Mark Rutland, Philippe Ombredanne, Arvind Yadav
In-Reply-To: <20180308073213.8419-2-jeffy.chen@rock-chips.com>
Hi Jeffy,
On Thu, Mar 08, 2018 at 03:32:11PM +0800, Jeffy Chen wrote:
> Add support for specifying event actions to trigger wakeup when using
> the gpio-keys input device as a wakeup source.
>
> This would allow the device to configure when to wakeup the system. For
> example a gpio-keys input device for pen insert, may only want to wakeup
> the system when ejecting the pen.
>
> Suggested-by: Brian Norris <briannorris@chromium.org>
> Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
> ---
>
> Changes in v5:
> Remove unneeded irq_wake flag as Andy suggested.
>
> Changes in v4:
> Add dt-binding gpio-keys.h, stop saving irq trigger type, add enable/disable wakeup helpers as Dmitry suggested.
>
> Changes in v3:
> Adding more comments as Brian suggested.
>
> Changes in v2:
> Specify wakeup event action instead of irq trigger type as Brian
> suggested.
>
> drivers/input/keyboard/gpio_keys.c | 63 +++++++++++++++++++++++++++++++++--
> include/dt-bindings/input/gpio-keys.h | 13 ++++++++
> include/linux/gpio_keys.h | 2 ++
> 3 files changed, 75 insertions(+), 3 deletions(-)
> create mode 100644 include/dt-bindings/input/gpio-keys.h
>
> diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
> index 87e613dc33b8..f6d5cfd44833 100644
> --- a/drivers/input/keyboard/gpio_keys.c
> +++ b/drivers/input/keyboard/gpio_keys.c
> @@ -30,6 +30,7 @@
> #include <linux/of.h>
> #include <linux/of_irq.h>
> #include <linux/spinlock.h>
> +#include <dt-bindings/input/gpio-keys.h>
>
> struct gpio_button_data {
> const struct gpio_keys_button *button;
> @@ -45,6 +46,7 @@ struct gpio_button_data {
> unsigned int software_debounce; /* in msecs, for GPIO-driven buttons */
>
> unsigned int irq;
> + unsigned int wakeup_trigger_type;
> spinlock_t lock;
> bool disabled;
> bool key_pressed;
> @@ -540,6 +542,8 @@ static int gpio_keys_setup_key(struct platform_device *pdev,
> }
>
> if (bdata->gpiod) {
> + int active_low = gpiod_is_active_low(bdata->gpiod);
> +
> if (button->debounce_interval) {
> error = gpiod_set_debounce(bdata->gpiod,
> button->debounce_interval * 1000);
> @@ -568,6 +572,24 @@ static int gpio_keys_setup_key(struct platform_device *pdev,
> isr = gpio_keys_gpio_isr;
> irqflags = IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING;
>
> + switch (button->wakeup_event_action) {
> + case EV_ACT_ASSERTED:
> + bdata->wakeup_trigger_type = active_low ?
> + IRQ_TYPE_EDGE_FALLING : IRQ_TYPE_EDGE_RISING;
> + break;
> + case EV_ACT_DEASSERTED:
> + bdata->wakeup_trigger_type = active_low ?
> + IRQ_TYPE_EDGE_RISING : IRQ_TYPE_EDGE_FALLING;
> + break;
> + case EV_ACT_ANY:
> + /* fall through */
> + default:
> + /*
> + * For other cases, we are OK letting suspend/resume
> + * not reconfigure the trigger type.
> + */
> + break;
> + }
> } else {
> if (!button->irq) {
> dev_err(dev, "Found button without gpio or irq\n");
> @@ -586,6 +608,11 @@ static int gpio_keys_setup_key(struct platform_device *pdev,
>
> isr = gpio_keys_irq_isr;
> irqflags = 0;
> +
> + /*
> + * For IRQ buttons, there is no interrupt for release.
> + * So we don't need to reconfigure the trigger type for wakeup.
> + */
> }
>
> bdata->code = &ddata->keymap[idx];
> @@ -718,6 +745,9 @@ gpio_keys_get_devtree_pdata(struct device *dev)
> /* legacy name */
> fwnode_property_read_bool(child, "gpio-key,wakeup");
>
> + fwnode_property_read_u32(child, "wakeup-event-action",
> + &button->wakeup_event_action);
> +
> button->can_disable =
> fwnode_property_read_bool(child, "linux,can-disable");
>
> @@ -845,6 +875,31 @@ static int gpio_keys_probe(struct platform_device *pdev)
> return 0;
> }
>
> +static int gpio_keys_enable_wakeup(struct gpio_button_data *bdata)
These new helpers need to be __maybe_unused in case we compile on system
without suspend support.
I also wanted a bit more of error handling, so I ended up with the
version of the patch below. Can you please try it and let me know if I
broke it.
Thanks.
--
Dmitry
Input: gpio-keys - add support for wakeup event action
From: Jeffy Chen <jeffy.chen@rock-chips.com>
Add support for specifying event actions to trigger wakeup when using
the gpio-keys input device as a wakeup source.
This would allow the device to configure when to wakeup the system. For
example a gpio-keys input device for pen insert, may only want to wakeup
the system when ejecting the pen.
Suggested-by: Brian Norris <briannorris@chromium.org>
Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
.../devicetree/bindings/input/gpio-keys.txt | 8 +
drivers/input/keyboard/gpio_keys.c | 145 ++++++++++++++++++--
include/dt-bindings/input/gpio-keys.h | 13 ++
include/linux/gpio_keys.h | 2
4 files changed, 154 insertions(+), 14 deletions(-)
create mode 100644 include/dt-bindings/input/gpio-keys.h
diff --git a/Documentation/devicetree/bindings/input/gpio-keys.txt b/Documentation/devicetree/bindings/input/gpio-keys.txt
index a94940481e55a..996ce84352cbf 100644
--- a/Documentation/devicetree/bindings/input/gpio-keys.txt
+++ b/Documentation/devicetree/bindings/input/gpio-keys.txt
@@ -26,6 +26,14 @@ Optional subnode-properties:
If not specified defaults to 5.
- wakeup-source: Boolean, button can wake-up the system.
(Legacy property supported: "gpio-key,wakeup")
+ - wakeup-event-action: Specifies whether the key should wake the
+ system when asserted, when deasserted, or both. This property is
+ only valid for keys that wake up the system (e.g., when the
+ "wakeup-source" property is also provided).
+ Supported values are defined in linux-event-codes.h:
+ EV_ACT_ASSERTED - asserted
+ EV_ACT_DEASSERTED - deasserted
+ EV_ACT_ANY - both asserted and deasserted
- linux,can-disable: Boolean, indicates that button is connected
to dedicated (not shared) interrupt which can be disabled to
suppress events from the button.
diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
index 87e613dc33b80..966428da6bc71 100644
--- a/drivers/input/keyboard/gpio_keys.c
+++ b/drivers/input/keyboard/gpio_keys.c
@@ -30,6 +30,7 @@
#include <linux/of.h>
#include <linux/of_irq.h>
#include <linux/spinlock.h>
+#include <dt-bindings/input/gpio-keys.h>
struct gpio_button_data {
const struct gpio_keys_button *button;
@@ -45,6 +46,7 @@ struct gpio_button_data {
unsigned int software_debounce; /* in msecs, for GPIO-driven buttons */
unsigned int irq;
+ unsigned int wakeup_trigger_type;
spinlock_t lock;
bool disabled;
bool key_pressed;
@@ -540,6 +542,8 @@ static int gpio_keys_setup_key(struct platform_device *pdev,
}
if (bdata->gpiod) {
+ bool active_low = gpiod_is_active_low(bdata->gpiod);
+
if (button->debounce_interval) {
error = gpiod_set_debounce(bdata->gpiod,
button->debounce_interval * 1000);
@@ -568,6 +572,24 @@ static int gpio_keys_setup_key(struct platform_device *pdev,
isr = gpio_keys_gpio_isr;
irqflags = IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING;
+ switch (button->wakeup_event_action) {
+ case EV_ACT_ASSERTED:
+ bdata->wakeup_trigger_type = active_low ?
+ IRQ_TYPE_EDGE_FALLING : IRQ_TYPE_EDGE_RISING;
+ break;
+ case EV_ACT_DEASSERTED:
+ bdata->wakeup_trigger_type = active_low ?
+ IRQ_TYPE_EDGE_RISING : IRQ_TYPE_EDGE_FALLING;
+ break;
+ case EV_ACT_ANY:
+ /* fall through */
+ default:
+ /*
+ * For other cases, we are OK letting suspend/resume
+ * not reconfigure the trigger type.
+ */
+ break;
+ }
} else {
if (!button->irq) {
dev_err(dev, "Found button without gpio or irq\n");
@@ -586,6 +608,11 @@ static int gpio_keys_setup_key(struct platform_device *pdev,
isr = gpio_keys_irq_isr;
irqflags = 0;
+
+ /*
+ * For IRQ buttons, there is no interrupt for release.
+ * So we don't need to reconfigure the trigger type for wakeup.
+ */
}
bdata->code = &ddata->keymap[idx];
@@ -718,6 +745,9 @@ gpio_keys_get_devtree_pdata(struct device *dev)
/* legacy name */
fwnode_property_read_bool(child, "gpio-key,wakeup");
+ fwnode_property_read_u32(child, "wakeup-event-action",
+ &button->wakeup_event_action);
+
button->can_disable =
fwnode_property_read_bool(child, "linux,can-disable");
@@ -845,19 +875,112 @@ static int gpio_keys_probe(struct platform_device *pdev)
return 0;
}
+static int __maybe_unused
+gpio_keys_button_enable_wakeup(struct gpio_button_data *bdata)
+{
+ int error;
+
+ error = enable_irq_wake(bdata->irq);
+ if (error) {
+ dev_err(bdata->input->dev.parent,
+ "failed to configure IRQ %d as wakeup source: %d\n",
+ bdata->irq, error);
+ return error;
+ }
+
+ if (bdata->wakeup_trigger_type) {
+ error = irq_set_irq_type(bdata->irq,
+ bdata->wakeup_trigger_type);
+ if (error) {
+ dev_err(bdata->input->dev.parent,
+ "failed to set wakeup trigger %08x for IRQ %d: %d\n",
+ bdata->wakeup_trigger_type, bdata->irq, error);
+ disable_irq_wake(bdata->irq);
+ return error;
+ }
+ }
+
+ return 0;
+}
+
+static void __maybe_unused
+gpio_keys_button_disable_wakeup(struct gpio_button_data *bdata)
+{
+ int error;
+
+ /*
+ * The trigger type is always both edges for gpio-based keys and we do
+ * not support changing wakeup trigger for interrupt-based keys.
+ */
+ if (bdata->wakeup_trigger_type) {
+ error = irq_set_irq_type(bdata->irq, IRQ_TYPE_EDGE_BOTH);
+ if (error)
+ dev_warn(bdata->input->dev.parent,
+ "failed to restore interrupt trigger for IRQ %d: %d\n",
+ bdata->irq, error);
+ }
+
+ error = disable_irq_wake(bdata->irq);
+ if (error)
+ dev_warn(bdata->input->dev.parent,
+ "failed to disable IRQ %d as wake source: %d\n",
+ bdata->irq, error);
+}
+
+static int __maybe_unused
+gpio_keys_enable_wakeup(struct gpio_keys_drvdata *ddata)
+{
+ struct gpio_button_data *bdata;
+ int error;
+ int i;
+
+ for (i = 0; i < ddata->pdata->nbuttons; i++) {
+ bdata = &ddata->data[i];
+ if (bdata->button->wakeup) {
+ error = gpio_keys_button_enable_wakeup(bdata);
+ if (error)
+ goto err_out;
+ }
+ bdata->suspended = true;
+ }
+
+ return 0;
+
+err_out:
+ while (--i >= 0) {
+ bdata = &ddata->data[i];
+ if (bdata->button->wakeup)
+ gpio_keys_button_disable_wakeup(bdata);
+ bdata->suspended = false;
+ }
+
+ return error;
+}
+
+static void __maybe_unused
+gpio_keys_disable_wakeup(struct gpio_keys_drvdata *ddata)
+{
+ struct gpio_button_data *bdata;
+ int i;
+
+ for (i = 0; i < ddata->pdata->nbuttons; i++) {
+ bdata = &ddata->data[i];
+ bdata->suspended = false;
+ if (irqd_is_wakeup_set(irq_get_irq_data(bdata->irq)))
+ gpio_keys_button_disable_wakeup(bdata);
+ }
+}
+
static int __maybe_unused gpio_keys_suspend(struct device *dev)
{
struct gpio_keys_drvdata *ddata = dev_get_drvdata(dev);
struct input_dev *input = ddata->input;
- int i;
+ int error;
if (device_may_wakeup(dev)) {
- for (i = 0; i < ddata->pdata->nbuttons; i++) {
- struct gpio_button_data *bdata = &ddata->data[i];
- if (bdata->button->wakeup)
- enable_irq_wake(bdata->irq);
- bdata->suspended = true;
- }
+ error = gpio_keys_enable_wakeup(ddata);
+ if (error)
+ return error;
} else {
mutex_lock(&input->mutex);
if (input->users)
@@ -873,15 +996,9 @@ static int __maybe_unused gpio_keys_resume(struct device *dev)
struct gpio_keys_drvdata *ddata = dev_get_drvdata(dev);
struct input_dev *input = ddata->input;
int error = 0;
- int i;
if (device_may_wakeup(dev)) {
- for (i = 0; i < ddata->pdata->nbuttons; i++) {
- struct gpio_button_data *bdata = &ddata->data[i];
- if (bdata->button->wakeup)
- disable_irq_wake(bdata->irq);
- bdata->suspended = false;
- }
+ gpio_keys_disable_wakeup(ddata);
} else {
mutex_lock(&input->mutex);
if (input->users)
diff --git a/include/dt-bindings/input/gpio-keys.h b/include/dt-bindings/input/gpio-keys.h
new file mode 100644
index 0000000000000..8962df79e753b
--- /dev/null
+++ b/include/dt-bindings/input/gpio-keys.h
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * This header provides constants for gpio keys bindings.
+ */
+
+#ifndef _DT_BINDINGS_GPIO_KEYS_H
+#define _DT_BINDINGS_GPIO_KEYS_H
+
+#define EV_ACT_ANY 0x00 /* asserted or deasserted */
+#define EV_ACT_ASSERTED 0x01 /* asserted */
+#define EV_ACT_DEASSERTED 0x02 /* deasserted */
+
+#endif /* _DT_BINDINGS_GPIO_KEYS_H */
diff --git a/include/linux/gpio_keys.h b/include/linux/gpio_keys.h
index d06bf77400f16..7160df54a6fe3 100644
--- a/include/linux/gpio_keys.h
+++ b/include/linux/gpio_keys.h
@@ -13,6 +13,7 @@ struct device;
* @desc: label that will be attached to button's gpio
* @type: input event type (%EV_KEY, %EV_SW, %EV_ABS)
* @wakeup: configure the button as a wake-up source
+ * @wakeup_event_action: event action to trigger wakeup
* @debounce_interval: debounce ticks interval in msecs
* @can_disable: %true indicates that userspace is allowed to
* disable button via sysfs
@@ -26,6 +27,7 @@ struct gpio_keys_button {
const char *desc;
unsigned int type;
int wakeup;
+ int wakeup_event_action;
int debounce_interval;
bool can_disable;
int value;
^ permalink raw reply related
* Re: [PATCH v5 1/3] Input: gpio-keys - add support for wakeup event action
From: Dmitry Torokhov @ 2018-03-10 18:11 UTC (permalink / raw)
To: Rob Herring
Cc: Jeffy Chen, linux-kernel, briannorris, heiko, dtor, dianders,
devicetree, Guenter Roeck, Thomas Gleixner, Joseph Lo, stephen lu,
Kate Stewart, linux-input, Greg Kroah-Hartman, Mark Rutland,
Philippe Ombredanne, Arvind Yadav
In-Reply-To: <20180309232720.pmqaulifiqa5czul@rob-hp-laptop>
On Fri, Mar 09, 2018 at 05:27:21PM -0600, Rob Herring wrote:
> On Thu, Mar 08, 2018 at 03:32:11PM +0800, Jeffy Chen wrote:
> > Add support for specifying event actions to trigger wakeup when using
> > the gpio-keys input device as a wakeup source.
> >
> > This would allow the device to configure when to wakeup the system. For
> > example a gpio-keys input device for pen insert, may only want to wakeup
> > the system when ejecting the pen.
> >
> > Suggested-by: Brian Norris <briannorris@chromium.org>
> > Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
> > ---
> >
> > Changes in v5:
> > Remove unneeded irq_wake flag as Andy suggested.
> >
> > Changes in v4:
> > Add dt-binding gpio-keys.h, stop saving irq trigger type, add enable/disable wakeup helpers as Dmitry suggested.
> >
> > Changes in v3:
> > Adding more comments as Brian suggested.
> >
> > Changes in v2:
> > Specify wakeup event action instead of irq trigger type as Brian
> > suggested.
> >
> > drivers/input/keyboard/gpio_keys.c | 63 +++++++++++++++++++++++++++++++++--
> > include/dt-bindings/input/gpio-keys.h | 13 ++++++++
>
> This file should be in the binding patch.
No, this is a folly. Both the header, along with the documentation (i.e.
binding doc) should be together with the patch that introduces the
changes. The patches are supposed to be self contained and sufficient to
stand on their own. Neither this header, not the binding doc make
sense without the change to the driver itself.
Next we'll start adding #defines in a separate patch before using
them...
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v2 1/4] dt-bindings: mfd: Add Gateworks System Controller bindings
From: Fabio Estevam @ 2018-03-10 13:57 UTC (permalink / raw)
To: Tim Harvey
Cc: Lee Jones, Rob Herring, Mark Rutland, Mark Brown, Dmitry Torokhov,
linux-kernel,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
linux-hwmon, linux-input
In-Reply-To: <1520287361-12569-2-git-send-email-tharvey@gateworks.com>
Hi Tim,
On Mon, Mar 5, 2018 at 7:02 PM, Tim Harvey <tharvey@gateworks.com> wrote:
> +
> + hwmon@1 { /* A1: Input Voltage */
> + type = <1>;
> + reg = <0x02>;
Unit address (@1) does not match the 'reg' value.
If someone build this example with W=1 a warning will be shown.
Same problem happens in other places of this example.
^ 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