* Re: [PATCH v2] Add rumble support to latest xbox controllers
From: Siarhei Vishniakou @ 2023-04-25 1:07 UTC (permalink / raw)
To: Benjamin Tissoires; +Cc: Bastien Nocera, Jiri Kosina, linux-input, linux-kernel
In-Reply-To: <CAO-hwJKOjfThF_CdF8kj=SiXKKLe5gr-Zh4iqgDpLmLNyjmbnw@mail.gmail.com>
Thanks folks,
I uploaded a v3 patch with the following changes:
1. Added link to wikipedia
2. Removed Change-Id from commit message
3. Added Bastien's 'Reviewed-by'.
On Thu, Mar 9, 2023 at 6:56 AM Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
>
> On Wed, Mar 8, 2023 at 7:54 PM Bastien Nocera <hadess@hadess.net> wrote:
> >
> > On Wed, 2023-03-08 at 09:55 -0800, Siarhei Vishniakou wrote:
> > > Thanks Bastien!
> > >
> > > I can definitely add a link to the wikipedia page.
> > >
> > > Are you talking about adding the link to the commit message, or to
> > > hid-ids.h ?
> >
> > I think the commit message would be enough so we know which device
> > we're talking about, but now that you mention it, maybe the source code
> > would be a good idea too.
>
> Agree, adding the link in the source would be fine.
>
> >
> > As far as I've understood, and Benjamin can correct me, we don't need
> > to have IDs be in hid-ids.h because we don't need to declare them both
> > as a blocklist in the hid core, and then again in the driver itself.
>
> We don't need to have the blocklist anymore, but using hid-ids.h is
> still used, and sometimes has the benefit of raising eyebrows when you
> add support for a new device and realize that it was already defined.
> So keeping the list around is not so much of a bad thing.
>
> >
> > My take is that the patches could move the IDs from hid-ids.h to hid-
> > microsoft.c as they're changed. You then wouldn't need the macros, just
> > add a comment for each of variants, and that Wikipedia article can be
> > linked above the whole XBox controller section.
>
> We are definitely in the bikeshedding phase, but I would leave the
> code as it is in this patch :)
>
> One more comment below:
>
> >
> > What do you think?
> >
> > >
> > >
> > > On Wed, Mar 8, 2023 at 2:23 AM Bastien Nocera <hadess@hadess.net>
> > > wrote:
> > > >
> > > > On Tue, 2023-03-07 at 13:35 -0800, Siarhei Vishniakou wrote:
> > > > > Currently, rumble is only supported via bluetooth on a single
> > > > > xbox
> > > > > controller, called 'model 1708'. On the back of the device, it's
> > > > > named
> > > > > 'wireless controller for xbox one'. However, in 2021, Microsoft
> > > > > released
> > > > > a firmware update for this controller. As part of this update,
> > > > > the
> > > > > HID
> > > > > descriptor of the device changed. The product ID was also changed
> > > > > from
> > > > > 0x02fd to 0x0b20. On this controller, rumble was supported via
> > > > > hid-microsoft, which matched against the old product id (0x02fd).
> > > > > As
> > > > > a
> > > > > result, the firmware update broke rumble support on this
> > > > > controller.
> > > > >
> > > > > The hid-microsoft driver actually supports rumble on the new
> > > > > firmware,
> > > > > as well. So simply adding new product id is sufficient to bring
> > > > > back
> > > > > this support.
> > > > >
> > > > > After discussing further with the xbox team, it was pointed out
> > > > > that
> > > > > another xbox controller, xbox elite series 2, can be supported in
> > > > > a
> > > > > similar way.
> > > > >
> > > > > Add rumble support for all of these devices in this patch. Two of
> > > > > the
> > > > > devices have received firmware updates that caused their product
> > > > > id's
> > > > > to
> > > > > change. Both old and new firmware versions of these devices were
> > > > > tested.
> > > > >
> > > > > The tested controllers are:
> > > > >
> > > > > 1. 'wireless controller for xbox one', model 1708
> > > > > 2. 'xbox wireless controller', model 1914. This is also sometimes
> > > > > referred to as 'xbox series S|X'.
> > > > > 3. 'elite series 2', model 1797.
> > > > >
> > > > > The tested configurations are:
> > > > > 1. model 1708, pid 0x02fd (old firmware)
> > > > > 2. model 1708, pid 0x0b20 (new firmware)
> > > > > 3. model 1914, pid 0x0b13
> > > > > 4. model 1797, pid 0x0b05 (old firmware)
> > > > > 5. model 1797, pid 0x0b22 (new firmware)
> > > > >
> > > > > I verified rumble support on both bluetooth and usb.
> > > >
> > > > Looks good although I would personally have preferred separate
> > > > patches
> > > > for each controller.
> > > >
> > > > Reviewed-by: Bastien Nocera <hadess@hadess.net>
> > > >
> > > > Would a link to:
> > > > https://en.wikipedia.org/wiki/Xbox_Wireless_Controller#Summary
> > > > also be useful to make which model is which clearer in the minds of
> > > > future readers?
> > > >
> > > > Cheers
> > > >
> > > > >
> > > > > Signed-off-by: Siarhei Vishniakou <svv@google.com>
> > > > > Change-Id: I3337a7ab5f40759c85bf67bf0dbe5d4de31ce1ff
>
> That change-id should be dropped, it has no meaning to us.
>
> Cheers,
> Benjamin
>
> > > > > ---
> > > > > drivers/hid/hid-ids.h | 6 +++++-
> > > > > drivers/hid/hid-microsoft.c | 11 ++++++++++-
> > > > > 2 files changed, 15 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> > > > > index 053853a891c5..c9b75f8ba49a 100644
> > > > > --- a/drivers/hid/hid-ids.h
> > > > > +++ b/drivers/hid/hid-ids.h
> > > > > @@ -903,7 +903,11 @@
> > > > > #define USB_DEVICE_ID_MS_TYPE_COVER_2 0x07a9
> > > > > #define USB_DEVICE_ID_MS_POWER_COVER 0x07da
> > > > > #define USB_DEVICE_ID_MS_SURFACE3_COVER 0x07de
> > > > > -#define USB_DEVICE_ID_MS_XBOX_ONE_S_CONTROLLER 0x02fd
> > > > > +#define USB_DEVICE_ID_MS_XBOX_CONTROLLER_MODEL_1708 0x02fd
> > > > > +#define
> > > > > USB_DEVICE_ID_MS_XBOX_CONTROLLER_MODEL_1708_BLE 0x0b20
> > > > > +#define USB_DEVICE_ID_MS_XBOX_CONTROLLER_MODEL_1914 0x0b13
> > > > > +#define USB_DEVICE_ID_MS_XBOX_CONTROLLER_MODEL_1797 0x0b05
> > > > > +#define
> > > > > USB_DEVICE_ID_MS_XBOX_CONTROLLER_MODEL_1797_BLE 0x0b22
> > > > > #define USB_DEVICE_ID_MS_PIXART_MOUSE 0x00cb
> > > > > #define USB_DEVICE_ID_8BITDO_SN30_PRO_PLUS 0x02e0
> > > > >
> > > > > diff --git a/drivers/hid/hid-microsoft.c b/drivers/hid/hid-
> > > > > microsoft.c
> > > > > index 071fd093a5f4..9345e2bfd56e 100644
> > > > > --- a/drivers/hid/hid-microsoft.c
> > > > > +++ b/drivers/hid/hid-microsoft.c
> > > > > @@ -446,7 +446,16 @@ static const struct hid_device_id
> > > > > ms_devices[] =
> > > > > {
> > > > > .driver_data = MS_PRESENTER },
> > > > > { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_MICROSOFT, 0x091B),
> > > > > .driver_data = MS_SURFACE_DIAL },
> > > > > - { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_MICROSOFT,
> > > > > USB_DEVICE_ID_MS_XBOX_ONE_S_CONTROLLER),
> > > > > +
> > > > > + { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_MICROSOFT,
> > > > > USB_DEVICE_ID_MS_XBOX_CONTROLLER_MODEL_1708),
> > > > > + .driver_data = MS_QUIRK_FF },
> > > > > + { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_MICROSOFT,
> > > > > USB_DEVICE_ID_MS_XBOX_CONTROLLER_MODEL_1708_BLE),
> > > > > + .driver_data = MS_QUIRK_FF },
> > > > > + { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_MICROSOFT,
> > > > > USB_DEVICE_ID_MS_XBOX_CONTROLLER_MODEL_1914),
> > > > > + .driver_data = MS_QUIRK_FF },
> > > > > + { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_MICROSOFT,
> > > > > USB_DEVICE_ID_MS_XBOX_CONTROLLER_MODEL_1797),
> > > > > + .driver_data = MS_QUIRK_FF },
> > > > > + { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_MICROSOFT,
> > > > > USB_DEVICE_ID_MS_XBOX_CONTROLLER_MODEL_1797_BLE),
> > > > > .driver_data = MS_QUIRK_FF },
> > > > > { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_MICROSOFT,
> > > > > USB_DEVICE_ID_8BITDO_SN30_PRO_PLUS),
> > > > > .driver_data = MS_QUIRK_FF },
> > > >
> >
>
^ permalink raw reply
* [PATCH] drivers: input: input: Fix possible sleep-in-atomic bug in input_alloc_absinfo()
From: starmiku1207184332 @ 2023-04-25 7:49 UTC (permalink / raw)
To: dmitry.torokhov, rydberg; +Cc: linux-input, linux-kernel, Teng Qi
From: Teng Qi <starmiku1207184332@gmail.com>
input_alloc_absinfo() may cause the kernel to sleep by calling kcalloc() with
the GFP_KERNEL flag. It may be called by input_event() holding spinlock through
the following call path:
input_event(), 433
input_handle_event(), 399
input_get_disposition(), 294
input_handle_abs_event(), 232
input_abs_set_val(), Clang suggests it calls input_alloc_absinfo()
unconditionally
input_alloc_absinfo(), 483
kcalloc(..., GFP_KERNEL)
This call path can potentially trigger a sleep-in-atomic bug. To prevent this
potential bug, we modify input_alloc_absinfo() to use the GFP_ATOMIC flag
instead of the GFP_KERNEL flag.
The possible bug is detected by a static code analysis tool.
Signed-off-by: Teng Qi <starmiku1207184332@gmail.com>
---
drivers/input/input.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/input/input.c b/drivers/input/input.c
index 37e876d45eb9..eefc4c85a693 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -480,7 +480,7 @@ void input_alloc_absinfo(struct input_dev *dev)
if (dev->absinfo)
return;
- dev->absinfo = kcalloc(ABS_CNT, sizeof(*dev->absinfo), GFP_KERNEL);
+ dev->absinfo = kcalloc(ABS_CNT, sizeof(*dev->absinfo), GFP_ATOMIC);
if (!dev->absinfo) {
dev_err(dev->dev.parent ?: &dev->dev,
"%s: unable to allocate memory\n", __func__);
--
2.25.1
^ permalink raw reply related
* Re: [PATCH v3] Add rumble support to latest xbox controllers
From: Bastien Nocera @ 2023-04-25 8:26 UTC (permalink / raw)
To: Siarhei Vishniakou, Jiri Kosina, Benjamin Tissoires, linux-input,
linux-kernel
In-Reply-To: <20230425010618.2303982-1-svv@google.com>
On Mon, 2023-04-24 at 18:06 -0700, Siarhei Vishniakou wrote:
> Currently, rumble is only supported via bluetooth on a single xbox
> controller, called 'model 1708'. On the back of the device, it's named
> 'wireless controller for xbox one'. However, in 2021, Microsoft released
> a firmware update for this controller. As part of this update, the HID
> descriptor of the device changed. The product ID was also changed from
> 0x02fd to 0x0b20. On this controller, rumble was supported via
> hid-microsoft, which matched against the old product id (0x02fd). As a
> result, the firmware update broke rumble support on this controller.
>
> The hid-microsoft driver actually supports rumble on the new firmware,
> as well. So simply adding new product id is sufficient to bring back
> this support.
>
> After discussing further with the xbox team, it was pointed out that
> another xbox controller, xbox elite series 2, can be supported in a
> similar way.
>
> Add rumble support for all of these devices in this patch. Two of the
> devices have received firmware updates that caused their product id's to
> change. Both old and new firmware versions of these devices were tested.
>
> The tested controllers are:
>
> 1. 'wireless controller for xbox one', model 1708
> 2. 'xbox wireless controller', model 1914. This is also sometimes
> referred to as 'xbox series S|X'.
> 3. 'elite series 2', model 1797.
>
> The tested configurations are:
> 1. model 1708, pid 0x02fd (old firmware)
> 2. model 1708, pid 0x0b20 (new firmware)
> 3. model 1914, pid 0x0b13
> 4. model 1797, pid 0x0b05 (old firmware)
> 5. model 1797, pid 0x0b22 (new firmware)
>
> I verified rumble support on both bluetooth and usb.
>
> Reviewed-by: Bastien Nocera <hadess@hadess.net>
> Signed-off-by: Siarhei Vishniakou <svv@google.com>
> ---
> drivers/hid/hid-ids.h | 8 +++++++-
> drivers/hid/hid-microsoft.c | 11 ++++++++++-
> 2 files changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index 053853a891c5..5d98b2a3a5e4 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -903,7 +903,13 @@
> #define USB_DEVICE_ID_MS_TYPE_COVER_2 0x07a9
> #define USB_DEVICE_ID_MS_POWER_COVER 0x07da
> #define USB_DEVICE_ID_MS_SURFACE3_COVER 0x07de
> -#define USB_DEVICE_ID_MS_XBOX_ONE_S_CONTROLLER 0x02fd
> +// For a description of the Xbox controller models, refer to:
> +// https://en.wikipedia.org/wiki/Xbox_Wireless_Controller#Summary
The kernel doesn't use C++ style comments, but C-style comments.
Checking your patch with checkpatch.pl should warn about it:
https://docs.kernel.org/process/submitting-patches.html#style-check-your-changes
> +#define USB_DEVICE_ID_MS_XBOX_CONTROLLER_MODEL_1708 0x02fd
> +#define USB_DEVICE_ID_MS_XBOX_CONTROLLER_MODEL_1708_BLE 0x0b20
The 1708 model uses Bluetooth Classic, not Bluetooth LE.
> +#define USB_DEVICE_ID_MS_XBOX_CONTROLLER_MODEL_1914 0x0b13
> +#define USB_DEVICE_ID_MS_XBOX_CONTROLLER_MODEL_1797 0x0b05
> +#define USB_DEVICE_ID_MS_XBOX_CONTROLLER_MODEL_1797_BLE 0x0b22
> #define USB_DEVICE_ID_MS_PIXART_MOUSE 0x00cb
> #define USB_DEVICE_ID_8BITDO_SN30_PRO_PLUS 0x02e0
>
> diff --git a/drivers/hid/hid-microsoft.c b/drivers/hid/hid-microsoft.c
> index 071fd093a5f4..9345e2bfd56e 100644
> --- a/drivers/hid/hid-microsoft.c
> +++ b/drivers/hid/hid-microsoft.c
> @@ -446,7 +446,16 @@ static const struct hid_device_id ms_devices[] = {
> .driver_data = MS_PRESENTER },
> { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_MICROSOFT, 0x091B),
> .driver_data = MS_SURFACE_DIAL },
> - { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_XBOX_ONE_S_CONTROLLER),
> +
> + { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_XBOX_CONTROLLER_MODEL_1708),
> + .driver_data = MS_QUIRK_FF },
> + { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_XBOX_CONTROLLER_MODEL_1708_BLE),
> + .driver_data = MS_QUIRK_FF },
> + { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_XBOX_CONTROLLER_MODEL_1914),
> + .driver_data = MS_QUIRK_FF },
> + { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_XBOX_CONTROLLER_MODEL_1797),
> + .driver_data = MS_QUIRK_FF },
> + { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_XBOX_CONTROLLER_MODEL_1797_BLE),
> .driver_data = MS_QUIRK_FF },
> { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_8BITDO_SN30_PRO_PLUS),
> .driver_data = MS_QUIRK_FF },
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: input: goodix: Add powered-in-suspend property
From: Fei Shao @ 2023-04-25 8:34 UTC (permalink / raw)
To: Doug Anderson
Cc: Jeff LaBundy, Benjamin Tissoires, Rob Herring, linux-mediatek,
Dmitry Torokhov, Krzysztof Kozlowski, devicetree, linux-input,
linux-kernel
In-Reply-To: <CAD=FV=VtrJDVB0gQp-O82V3mgU6MFVMSGrvzh0fRa7sYAt_Pqw@mail.gmail.com>
On Tue, Apr 25, 2023 at 2:15 AM Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Sun, Apr 23, 2023 at 8:31 PM Jeff LaBundy <jeff@labundy.com> wrote:
> >
> > Back-powering can come in two forms:
> >
> > 1. The one you've described, which is by far the most common. As you mention,
> > it is not the case here since the issue happens only when we drive the GPIO
> > low and not high.
> >
> > 2. Through a forbidden power supply sequence, an input pin of an IC with
> > multiple power supplies becomes a weak power supply itself. Grounding the
> > GPIO then sinks current into the SoC.
> >
> > This problem smelled like (2), especially since asserting the GPIO or powering
> > down the supply alleviates the symptom. Most Goodix controllers I've worked
> > with have two or more supplies, and the datasheet requires them to be enabled
> > or disabled in a specific order.
> >
> > Based on Fei's feedback, however, this IC does not seem to be one of those
> > since there is reportedly only a single rail. I guess either vdd or vddio is
> > tied to a dummy regulator? If so, then perhaps we can scratch this theory.
>
> There is one rail provided from the main board, but there can be two
> voltage levels involved depending on stuffings on the touchscreen
> itself. I talked a bit about this in commit 1d18c1f3b7d9
> ("dt-bindings: HID: i2c-hid: goodix: Add mainboard-vddio-supply").
>
>
> > Fair enough, I was simply sketpical that this was the _right_ workaround.
> > Were this an issue of only supply A left on yet the IC datasheet requires
> > supply B to remain on if supply A is on, I would rather see this solved by
> > updating a regulator dts node, trusted FW sequence, etc.
>
> Right. That type of stuff was looked at in detail by two separate
> teams, so I don't think this is the issue.
>
>
> > Thanks for your due diligence; if this really is a silicon issue, then
> > the driver should indeed accommodate it.
> >
> > That being said, the name 'powered-in-suspend' seems a bit conflated. We
> > should not be duplicating regulator policy in this driver; the existing
> > naming also may cause confusion for other HW configurations that do leave
> > vdd and vddio on during suspend, but don't have this problem.
> >
> > Here, we actually mean to control the behavior of the reset GPIO through
> > suspend and therefore something like 'goodix,no-reset-during-suspend' is
> > closer to what I understand us to intend to do. I will add more details
> > in patch [2/2].
>
> I'd be fine with that name. ...ah, and adding the "goodix," prefix is
> a good call.
>
> -Doug
That sounds good to me, too. I'll update the name in the next patch.
Regards,
Fei
^ permalink raw reply
* Re: [PATCH 2/2] HID: i2c-hid: goodix: Add support for powered-in-suspend property
From: Fei Shao @ 2023-04-25 8:36 UTC (permalink / raw)
To: Doug Anderson
Cc: Jeff LaBundy, Benjamin Tissoires, Rob Herring, linux-mediatek,
Dmitry Torokhov, Jiri Kosina, Matthias Kaehlcke, Stephen Kitt,
linux-input, linux-kernel
In-Reply-To: <CAD=FV=XnrSskS=0UKRGGPBxSnPYQtUkJeoBm44bDnwKVBHJLyA@mail.gmail.com>
Hi,
On Tue, Apr 25, 2023 at 2:16 AM Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Sun, Apr 23, 2023 at 8:38 PM Jeff LaBundy <jeff@labundy.com> wrote:
> >
> > > @@ -37,13 +38,34 @@ static int goodix_i2c_hid_power_up(struct i2chid_ops *ops)
> > > container_of(ops, struct i2c_hid_of_goodix, ops);
> > > int ret;
> > >
> > > - ret = regulator_enable(ihid_goodix->vdd);
> > > - if (ret)
> > > - return ret;
> > > -
> > > - ret = regulator_enable(ihid_goodix->vddio);
> > > - if (ret)
> > > - return ret;
> > > + /*
> > > + * This is to ensure that the reset GPIO will be asserted and the
> > > + * regulators will be enabled for all cases.
> > > + */
> > > + if (ihid_goodix->powered_in_suspend) {
> > > + /*
> > > + * This is not mandatory, but we assert reset here (instead of
> > > + * in power-down) to ensure that the device will have a clean
> > > + * state later on just like the normal scenarios would have.
> > > + *
> > > + * Also, since the regulators were not disabled in power-down,
> > > + * we don't need to enable them here.
> > > + */
> > > + gpiod_set_value_cansleep(ihid_goodix->reset_gpio, 1);
> > > + } else {
> > > + /*
> > > + * In this case, the reset is already asserted (either in
> > > + * probe or power-down).
> > > + * All we need is to enable the regulators.
> > > + */
> > > + ret = regulator_enable(ihid_goodix->vdd);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + ret = regulator_enable(ihid_goodix->vddio);
> > > + if (ret)
> > > + return ret;
> > > + }
> >
> > Please let me know in case I have misunderstood, but I don't see a need
> > to change the regulator_enable/disable() logic if this property is set.
> > If the regulators are truly always-on, the regulator core already knows
> > what to do and we should not duplicate that logic here.
Your understanding is totally right, let me restore that in the next
revision. Thanks!
Regards,
Fei
> >
> > Based on the alleged silicon erratum discussed in patch [1/2], it seems
> > we only want to control the behavior of the reset GPIO. Therefore, only
> > the calls to gpiod_set_value_cansleep() should be affected and the name
> > of the property updated to reflect what it's actually doing.
>
> This would be OK w/ me.
^ permalink raw reply
* Re: [PATCH linux-next] HID: amd_sfh: Fix max supported HID devices
From: Benjamin Tissoires @ 2023-04-25 9:00 UTC (permalink / raw)
To: jikos, linux-input, Basavaraj Natikar
In-Reply-To: <20230424160406.2579888-1-Basavaraj.Natikar@amd.com>
On Mon, 24 Apr 2023 21:34:06 +0530, Basavaraj Natikar wrote:
> commit 4bd763568dbd ("HID: amd_sfh: Support for additional light sensor")
> adds additional sensor devices, but forgets to add the number of HID
> devices to match. Thus, the number of HID devices does not match the
> actual number of sensors.
>
> In order to prevent corruption and system hangs when more than the
> allowed number of HID devices are accessed, the number of HID devices is
> increased accordingly.
>
> [...]
Applied to hid/hid.git (for-6.4/amd-sfh), thanks!
[1/1] HID: amd_sfh: Fix max supported HID devices
https://git.kernel.org/hid/hid/c/37386669887d
Cheers,
--
Benjamin Tissoires <benjamin.tissoires@redhat.com>
^ permalink raw reply
* Re: [PATCH v3] staging: HID: Add ShanWan USB WirelessGamepad driver
From: Benjamin Tissoires @ 2023-04-25 9:04 UTC (permalink / raw)
To: Mubashshir; +Cc: Jiri Kosina, Huseyin BIYIK, linux-kernel, linux-input
In-Reply-To: <3ONCTR.Q80RY3TDW2I8@gmail.com>
On Apr 19 2023, Mubashshir wrote:
> On Thu, Apr 13 2023 at 06:13:05 PM +02:00:00, Benjamin Tissoires
> <benjamin.tissoires@redhat.com> wrote:
> > What's the point of keeping it opened for the lifetime of the device? Do
> > you really need this?
> >
> > > + if (error) {
> > > + dev_err(&hdev->dev, "hw open failed\n");
> > > + hid_hw_stop(hdev);
> > > + return error;
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static void shanwan_remove(struct hid_device *hdev)
> > > +{
> > > + hid_hw_close(hdev);
> >
> > If you can drop the last hid_hw_open/close, then you can entirely skip
> > the ->remove().
>
> Would hid_hw_request work without open device?
No, it won't work if the device is not opened. But what's the point of
sending rumble data to the joystick if the device is not opened (so used)
by anybody? open() is called whenever a client opens the device, by
accessing the evdev node, so I think it should be fine.
Cheers,
Benjamin
^ permalink raw reply
* Re: [PATCH v3] staging: HID: Add ShanWan USB WirelessGamepad driver
From: Mubashshir @ 2023-04-25 9:06 UTC (permalink / raw)
To: Benjamin Tissoires; +Cc: Jiri Kosina, Huseyin BIYIK, linux-kernel, linux-input
In-Reply-To: <udogxn4kbihht7isswwwofra373lotxtfvyj7kwxbe3436fjsp@4yxggpana75c>
On Tue, Apr 25 2023 at 11:04:43 AM +02:00:00, Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
> No, it won't work if the device is not opened. But what's the point of
> sending rumble data to the joystick if the device is not opened (so
> used)
> by anybody? open() is called whenever a client opens the device, by
> accessing the evdev node, so I think it should be fine.
I'll test it tonight...
^ permalink raw reply
* Re: [PATCH 00/43] ep93xx device tree conversion
From: Krzysztof Kozlowski @ 2023-04-25 9:20 UTC (permalink / raw)
To: Jakub Kicinski, Arnd Bergmann
Cc: Nikita Shubin, Arnd Bergmann, Linus Walleij, Alexander Sverdlin,
David S . Miller, Jonathan Neuschäfer, Russell King,
Uwe Kleine-König, Alessandro Zummo, Alexander Gordeev,
Alexandre Belloni, Andy Shevchenko, Bartosz Golaszewski,
Brian Norris, Chuanhong Guo, Conor.Dooley, Damien Le Moal,
Daniel Lezcano, Dmitry Torokhov, Emil Renner Berthing,
Eric Dumazet, Florian Fainelli, Guenter Roeck, Hartley Sweeten,
Heiko Stübner, Hitomi Hasegawa, Jaroslav Kysela,
Jean Delvare, Joel Stanley, Jonathan Cameron, Krzysztof Kozlowski,
Damien Le Moal, Liam Girdwood, Liang Yang, Linus Walleij,
Lukasz Majewski, Lv Ruyi, Mark Brown, Masahiro Yamada,
Michael Turquette, Miquel Raynal, Nathan Chancellor,
Nick Desaulniers, Nicolas Saenz Julienne, Olof Johansson,
Paolo Abeni, Qin Jian, Richard Weinberger, Rob Herring,
Robert Jarzmik, Russell King, Sebastian Reichel, Sergey Shtylyov,
Stephen Boyd, Sumanth Korikkar, Sven Peter, Takashi Iwai,
Thierry Reding, Thomas Gleixner, Ulf Hansson, Vasily Gorbik,
Vignesh Raghavendra, Vinod Koul, Walker Chen, Wim Van Sebroeck,
Yinbo Zhu, alsa-devel, devicetree, dmaengine, linux-arm-kernel,
linux-clk, open list:GPIO SUBSYSTEM, linux-ide, linux-input,
linux-kernel, linux-mtd, linux-pm, linux-pwm, linux-rtc,
linux-spi, linux-watchdog, Netdev, soc
In-Reply-To: <20230424152933.48b2ede1@kernel.org>
On 25/04/2023 00:29, Jakub Kicinski wrote:
> On Mon, 24 Apr 2023 13:31:25 +0200 Arnd Bergmann wrote:
>> Thanks a lot for your continued work. I can't merge any of this at
>> the moment since the upstream merge window just opened, but I'm
>> happy to take this all through the soc tree for 6.5, provided we
>> get the sufficient Acks from the subsystem maintainers. Merging
>> it through each individual tree would take a lot longer, so I
>> hope we can avoid that.
>
> Is there a dependency between the patches?
I didn't get entire patchset and cover letter does not mention
dependencies, but usually there shouldn't be such. Maybe for the next
versions this should be split per subsystem?
Best regards,
Krzysztof
^ permalink raw reply
* Słowa kluczowe do wypozycjonowania
From: Adam Charachuta @ 2023-04-25 9:42 UTC (permalink / raw)
To: linux-input
Dzień dobry,
zapoznałem się z Państwa ofertą i z przyjemnością przyznaję, że przyciąga uwagę i zachęca do dalszych rozmów.
Pomyślałem, że może mógłbym mieć swój wkład w Państwa rozwój i pomóc dotrzeć z tą ofertą do większego grona odbiorców. Pozycjonuję strony www, dzięki czemu generują świetny ruch w sieci.
Możemy porozmawiać w najbliższym czasie?
Pozdrawiam
Adam Charachuta
^ permalink raw reply
* [RFC v1 0/4] Input: support virtual objects on touchscreens
From: Javier Carrasco @ 2023-04-25 11:50 UTC (permalink / raw)
To: linux-input, devicetree, linux-kernel
Cc: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Henrik Rydberg,
Javier Carrasco, Ulf Hansson, Hans Verkuil, Stephen Boyd,
Sebastian Reichel, Linus Walleij, Jonathan Cameron, Uwe Kleine-g,
Bastian Hecht, Michael Riesch
Some touchscreens are shipped with a physical layer on top of them where
a number of buttons and a resized touchscreen surface might be available.
In order to generate proper key events by overlay buttons and adjust the
touch events to a clipped surface, these patches offer a documented,
device-tree-based solution by means of helper functions.
An implementation for a specific touchscreen driver is also included.
The functions in ts-virtobj provide a simple workflow to acquire
physical objects from the device tree, map them into the device driver
structures as virtual objects and generate events according to
the object descriptions.
This solution has been tested with a JT240MHQS-E3 display, which uses
the st1624 as a touchscreen and provides two overly buttons and a frame
that clips its effective surface.
Javier Carrasco (4):
Input: ts-virtobj - Add touchsreen virtual object handling to the core
dt-bindings: touchscreen: add virtual-touchscreen and virtual-buttons
properties
Input: st1232 - add virtual touchscreen and buttons handling
dt-bindings: input: touchscreen: st1232: add virtual objects to the
example
.../input/touchscreen/sitronix,st1232.yaml | 28 ++
.../input/touchscreen/touchscreen.yaml | 54 ++++
drivers/input/Makefile | 1 +
drivers/input/touchscreen/st1232.c | 41 ++-
drivers/input/ts-virtobj.c | 305 ++++++++++++++++++
include/linux/input/ts-virtobj.h | 59 ++++
6 files changed, 478 insertions(+), 10 deletions(-)
create mode 100644 drivers/input/ts-virtobj.c
create mode 100644 include/linux/input/ts-virtobj.h
--
2.37.2
^ permalink raw reply
* [RFC v1 1/4] Input: ts-virtobj - Add touchsreen virtual object handling to the core
From: Javier Carrasco @ 2023-04-25 11:50 UTC (permalink / raw)
To: linux-input, devicetree, linux-kernel
Cc: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Henrik Rydberg,
Javier Carrasco, Ulf Hansson, Hans Verkuil, Stephen Boyd,
Sebastian Reichel, Linus Walleij, Jonathan Cameron, Uwe Kleine-g,
Bastian Hecht, Michael Riesch
In-Reply-To: <20230425115049.870003-1-javier.carrasco@wolfvision.net>
Some touchscreens provide mechanical overlays with different objects
like buttons or downsized touchscreen surfaces.
In order to support these objects, add a series of helper functions
to the input core to transform them into virtual objects via device
tree nodes.
These virtual objects consume the raw touch events and report the
expected input events depending on the object properties.
Signed-off-by: Javier Carrasco <javier.carrasco@wolfvision.net>
---
drivers/input/Makefile | 1 +
drivers/input/ts-virtobj.c | 305 +++++++++++++++++++++++++++++++
include/linux/input/ts-virtobj.h | 59 ++++++
3 files changed, 365 insertions(+)
create mode 100644 drivers/input/ts-virtobj.c
create mode 100644 include/linux/input/ts-virtobj.h
diff --git a/drivers/input/Makefile b/drivers/input/Makefile
index 2266c7d010ef..9c717e28719e 100644
--- a/drivers/input/Makefile
+++ b/drivers/input/Makefile
@@ -8,6 +8,7 @@
obj-$(CONFIG_INPUT) += input-core.o
input-core-y := input.o input-compat.o input-mt.o input-poller.o ff-core.o
input-core-y += touchscreen.o
+input-core-y += ts-virtobj.o
obj-$(CONFIG_INPUT_FF_MEMLESS) += ff-memless.o
obj-$(CONFIG_INPUT_SPARSEKMAP) += sparse-keymap.o
diff --git a/drivers/input/ts-virtobj.c b/drivers/input/ts-virtobj.c
new file mode 100644
index 000000000000..4bddd8015af3
--- /dev/null
+++ b/drivers/input/ts-virtobj.c
@@ -0,0 +1,305 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Helper functions for virtual objects on touchscreens
+ *
+ * Copyright (c) 2023 Javier Carrasco <javier.carrasco@wolfvision.net>
+ */
+
+#include <linux/property.h>
+#include <linux/input.h>
+#include <linux/input/mt.h>
+#include <linux/module.h>
+#include <linux/input/ts-virtobj.h>
+
+static int ts_virtobj_count_buttons(struct device *dev)
+{
+ struct fwnode_handle *child_node;
+ struct fwnode_handle *child_button;
+ int count = 0;
+
+ child_node = device_get_named_child_node(dev, ts_virtobj_names[BUTTON]);
+ if (!child_node)
+ return 0;
+
+ fwnode_for_each_child_node(child_node, child_button)
+ count++;
+ fwnode_handle_put(child_node);
+
+ return count;
+}
+
+static int ts_virtobj_get_shape_properties(struct fwnode_handle *child_node,
+ struct ts_virtobj_shape *shape)
+{
+ int rc;
+
+ rc = fwnode_property_read_u32(child_node, "x-origin", &shape->x_origin);
+ if (rc < 0)
+ return rc;
+
+ rc = fwnode_property_read_u32(child_node, "y-origin", &shape->y_origin);
+ if (rc < 0)
+ return rc;
+
+ rc = fwnode_property_read_u32(child_node, "x-size", &shape->x_size);
+ if (rc < 0)
+ return rc;
+
+ rc = fwnode_property_read_u32(child_node, "y-size", &shape->y_size);
+ if (rc < 0)
+ return rc;
+
+ return 0;
+}
+
+static int ts_virtobj_get_button_properties(struct device *dev,
+ struct fwnode_handle *child_node,
+ struct ts_virtobj_button *btn)
+{
+ struct fwnode_handle *child_btn;
+ int rc;
+ int j = 0;
+
+ fwnode_for_each_child_node(child_node, child_btn) {
+ rc = ts_virtobj_get_shape_properties(child_btn, &btn[j].shape);
+ if (rc < 0) {
+ fwnode_handle_put(child_btn);
+ return rc;
+ }
+
+ rc = fwnode_property_read_u32(child_btn, "linux,code",
+ &btn[j].key);
+ if (rc < 0) {
+ fwnode_handle_put(child_btn);
+ return rc;
+ }
+
+ dev_info(dev, "Added button at (%u, %u), size %ux%u, code=%u\n",
+ btn[j].shape.x_origin, btn[j].shape.y_origin,
+ btn[j].shape.x_size, btn[j].shape.y_size, btn[j].key);
+ j++;
+ }
+
+ return 0;
+}
+
+static void ts_virtobj_set_button_caps(struct ts_virtobj_map *map,
+ struct input_dev *dev)
+{
+ int i;
+
+ for (i = 0; i < map->button_count; i++)
+ input_set_capability(dev, EV_KEY,
+ map->buttons[i].key);
+}
+
+static int ts_virtobj_map_touchscreen(struct device *dev,
+ struct ts_virtobj_map *map)
+{
+ struct fwnode_handle *child;
+ int rc = 0;
+
+ child = device_get_named_child_node(dev, ts_virtobj_names[TOUCHSCREEN]);
+ if (!child)
+ return 0;
+
+ map->touchscreen = devm_kzalloc(dev, sizeof(*map->touchscreen),
+ GFP_KERNEL);
+ if (!map->touchscreen) {
+ fwnode_handle_put(child);
+ return -ENOMEM;
+ }
+ rc = ts_virtobj_get_shape_properties(child, map->touchscreen);
+ if (rc < 0) {
+ devm_kfree(dev, map->touchscreen);
+ fwnode_handle_put(child);
+ return rc;
+ }
+ map->virtual_touchscreen = 1;
+ dev_info(dev, "Added virtual touchscreen at (%u, %u), size %u x %u\n",
+ map->touchscreen->x_origin, map->touchscreen->y_origin,
+ map->touchscreen->x_size, map->touchscreen->y_size);
+
+ fwnode_handle_put(child);
+ return 0;
+}
+
+static int ts_virtobj_map_buttons(struct device *dev,
+ struct ts_virtobj_map *map,
+ struct input_dev *input)
+{
+ struct fwnode_handle *child;
+ u32 button_count;
+ int rc;
+
+ button_count = ts_virtobj_count_buttons(dev);
+ if (button_count) {
+ map->buttons = devm_kcalloc(dev, button_count,
+ sizeof(*map->buttons), GFP_KERNEL);
+ if (!map->buttons)
+ return -ENOMEM;
+
+ child = device_get_named_child_node(dev,
+ ts_virtobj_names[BUTTON]);
+ if (unlikely(!child)) {
+ devm_kfree(dev, map->buttons);
+ return 0;
+ }
+ rc = ts_virtobj_get_button_properties(dev, child, map->buttons);
+ if (rc < 0) {
+ devm_kfree(dev, map->buttons);
+ return rc;
+ }
+ map->button_count = button_count;
+ ts_virtobj_set_button_caps(map, input);
+ }
+
+ return 0;
+}
+
+static bool ts_virtobj_defined_objects(struct device *dev)
+{
+ struct fwnode_handle *child;
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(ts_virtobj_names); i++) {
+ child = device_get_named_child_node(dev, ts_virtobj_names[i]);
+ if (child) {
+ fwnode_handle_put(child);
+ return true;
+ }
+ fwnode_handle_put(child);
+ }
+
+ return false;
+}
+
+struct ts_virtobj_map *ts_virtobj_map_objects(struct device *dev,
+ struct input_dev *input)
+{
+ struct ts_virtobj_map *map;
+ int rc;
+
+ if (!ts_virtobj_defined_objects(dev))
+ return NULL;
+
+ map = devm_kzalloc(dev, sizeof(*map), GFP_KERNEL);
+ if (!map)
+ return ERR_PTR(-ENOMEM);
+
+ rc = ts_virtobj_map_touchscreen(dev, map);
+ if (rc < 0) {
+ devm_kfree(dev, map);
+ return ERR_PTR(rc);
+ }
+ rc = ts_virtobj_map_buttons(dev, map, input);
+ if (rc < 0) {
+ devm_kfree(dev, map);
+ return ERR_PTR(rc);
+ }
+
+ return map;
+}
+EXPORT_SYMBOL(ts_virtobj_map_objects);
+
+void ts_virtobj_retrieve_abs(struct ts_virtobj_map *map, u16 *x, u16 *y)
+{
+ *x = map->touchscreen->x_size - 1;
+ *y = map->touchscreen->y_size - 1;
+}
+EXPORT_SYMBOL(ts_virtobj_retrieve_abs);
+
+static bool ts_virtobj_event_in_shape_range(struct ts_virtobj_shape *shape,
+ u32 x, u32 y)
+{
+ if (!shape)
+ return false;
+
+ if (x >= shape->x_origin && x < (shape->x_origin + shape->x_size) &&
+ y >= shape->y_origin && y < (shape->y_origin + shape->y_size))
+ return true;
+
+ return false;
+}
+
+bool ts_virtobj_touchscreen_event(struct ts_virtobj_shape *touchscreen,
+ u32 *x, u32 *y)
+{
+ if (ts_virtobj_event_in_shape_range(touchscreen, *x, *y)) {
+ *x -= touchscreen->x_origin;
+ *y -= touchscreen->y_origin;
+ return true;
+ }
+
+ return false;
+}
+EXPORT_SYMBOL(ts_virtobj_touchscreen_event);
+
+bool ts_virtobj_mapped_touchscreen(struct ts_virtobj_map *map)
+{
+ if (!map || !map->virtual_touchscreen)
+ return false;
+
+ return true;
+}
+EXPORT_SYMBOL(ts_virtobj_mapped_touchscreen);
+
+static bool ts_virtobj_mapped_button(struct ts_virtobj_map *map)
+{
+ if (!map || !map->button_count)
+ return false;
+
+ return true;
+}
+
+bool ts_virtobj_mt_on_touchscreen(struct ts_virtobj_map *map, u32 *x, u32 *y)
+{
+ if (!ts_virtobj_mapped_touchscreen(map))
+ return true;
+
+ if (!ts_virtobj_touchscreen_event(map->touchscreen, x, y))
+ return false;
+
+ return true;
+}
+EXPORT_SYMBOL(ts_virtobj_mt_on_touchscreen);
+
+bool ts_virtobj_button_event(struct ts_virtobj_map *map,
+ struct input_dev *input, u32 x, u32 y)
+{
+ int i;
+
+ if (!ts_virtobj_mapped_button(map))
+ return false;
+
+ for (i = 0; i < map->button_count; i++) {
+ if (ts_virtobj_event_in_shape_range(&map->buttons[i].shape, x, y)) {
+ input_report_key(input, map->buttons[i].key, 1);
+ map->buttons[i].pressed = true;
+ return true;
+ }
+ }
+
+ return false;
+}
+EXPORT_SYMBOL(ts_virtobj_button_event);
+
+void ts_virtobj_button_release_pressed(struct ts_virtobj_map *map,
+ struct input_dev *input)
+{
+ int i;
+
+ if (!map || !map->button_count)
+ return;
+
+ for (i = 0; i < map->button_count; i++) {
+ if (map->buttons[i].pressed) {
+ input_report_key(input, map->buttons[i].key, 0);
+ map->buttons[i].pressed = false;
+ }
+ }
+}
+EXPORT_SYMBOL(ts_virtobj_button_release_pressed);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Helper functions for virtual objects on touchscreens");
diff --git a/include/linux/input/ts-virtobj.h b/include/linux/input/ts-virtobj.h
new file mode 100644
index 000000000000..4b61709a2680
--- /dev/null
+++ b/include/linux/input/ts-virtobj.h
@@ -0,0 +1,59 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) 2023 Javier Carrasco <javier.carrasco@wolfvision.net>
+ */
+
+#ifndef _TS_VIRTOBJ
+#define _TS_VIRTOBJ
+
+struct input_dev;
+struct device;
+
+enum ts_virtobj_valid_objects {
+ TOUCHSCREEN,
+ BUTTON,
+};
+
+static const char * const ts_virtobj_names[] = {
+ [TOUCHSCREEN] = "virtual-touchscreen",
+ [BUTTON] = "virtual-buttons",
+};
+
+struct ts_virtobj_shape {
+ u32 x_origin;
+ u32 y_origin;
+ u32 x_size;
+ u32 y_size;
+};
+
+struct ts_virtobj_button {
+ struct ts_virtobj_shape shape;
+ u32 key;
+ bool pressed;
+};
+
+struct ts_virtobj_map {
+ struct ts_virtobj_shape *touchscreen;
+ bool virtual_touchscreen;
+ struct ts_virtobj_button *buttons;
+ u32 button_count;
+};
+
+struct ts_virtobj_map *ts_virtobj_map_objects(struct device *dev,
+ struct input_dev *input);
+
+void ts_virtobj_retrieve_abs(struct ts_virtobj_map *map, u16 *x, u16 *y);
+
+bool ts_virtobj_touchscreen_event(struct ts_virtobj_shape *touchscreen,
+ u32 *x, u32 *y);
+
+bool ts_virtobj_mapped_touchscreen(struct ts_virtobj_map *map);
+
+bool ts_virtobj_mt_on_touchscreen(struct ts_virtobj_map *map, u32 *x, u32 *y);
+
+bool ts_virtobj_button_event(struct ts_virtobj_map *map,
+ struct input_dev *input, u32 x, u32 y);
+
+void ts_virtobj_button_release_pressed(struct ts_virtobj_map *map,
+ struct input_dev *input);
+#endif
--
2.37.2
^ permalink raw reply related
* [RFC v1 2/4] dt-bindings: touchscreen: add virtual-touchscreen and virtual-buttons properties
From: Javier Carrasco @ 2023-04-25 11:50 UTC (permalink / raw)
To: linux-input, devicetree, linux-kernel
Cc: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Henrik Rydberg,
Javier Carrasco, Ulf Hansson, Hans Verkuil, Stephen Boyd,
Sebastian Reichel, Linus Walleij, Jonathan Cameron, Uwe Kleine-g,
Bastian Hecht, Michael Riesch
In-Reply-To: <20230425115049.870003-1-javier.carrasco@wolfvision.net>
The virtual-touchscreen object defines an area within the touchscreen
where touch events are reported and their coordinates get converted to
the virtual origin. This object avoids getting events from areas that
are physically hidden by overlay frames.
For touchscreens where overlay buttons on the touchscreen surface are
provided, the virtual-buttons object contains a node for every button
and the key event that should be reported when pressed.
Signed-off-by: Javier Carrasco <javier.carrasco@wolfvision.net>
---
.../input/touchscreen/touchscreen.yaml | 54 +++++++++++++++++++
1 file changed, 54 insertions(+)
diff --git a/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml b/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml
index 895592da9626..869be007eb6f 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml
+++ b/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml
@@ -80,6 +80,60 @@ properties:
touchscreen-y-plate-ohms:
description: Resistance of the Y-plate in Ohms
+ virtual-touchscreen:
+ description: Clipped touchscreen area
+ type: object
+
+ properties:
+ x-origin:
+ description: horizontal origin of the clipped area
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ y-origin:
+ description: vertical origin of the clipped area
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ x-size:
+ description: horizontal resolution of the clipped area
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ y-size:
+ description: vertical resolution of the clipped area
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ virtual-buttons:
+ description: list of nodes defining the buttons on the touchscreen
+ type: object
+
+ patternProperties:
+ '^button-':
+ type: object
+ description:
+ Each button (key) is represented as a sub-node.
+
+ properties:
+ label:
+ $ref: /schemas/types.yaml#/definitions/string
+ description: descriptive name of the button
+
+ linux,code: true
+
+ x-origin:
+ description: horizontal origin of the button area
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ y-origin:
+ description: vertical origin of the button area
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ x-size:
+ description: horizontal resolution of the button area
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ y-size:
+ description: vertical resolution of the button area
+ $ref: /schemas/types.yaml#/definitions/uint32
+
dependencies:
touchscreen-size-x: [ touchscreen-size-y ]
touchscreen-size-y: [ touchscreen-size-x ]
--
2.37.2
^ permalink raw reply related
* [RFC v1 3/4] Input: st1232 - add virtual touchscreen and buttons handling
From: Javier Carrasco @ 2023-04-25 11:50 UTC (permalink / raw)
To: linux-input, devicetree, linux-kernel
Cc: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Henrik Rydberg,
Javier Carrasco, Ulf Hansson, Hans Verkuil, Stephen Boyd,
Sebastian Reichel, Linus Walleij, Jonathan Cameron, Uwe Kleine-g,
Bastian Hecht, Michael Riesch
In-Reply-To: <20230425115049.870003-1-javier.carrasco@wolfvision.net>
Use ts-virtobj to support overlay objects such as buttons and resized
frames defined in the device tree.
Signed-off-by: Javier Carrasco <javier.carrasco@wolfvision.net>
---
drivers/input/touchscreen/st1232.c | 41 ++++++++++++++++++++++--------
1 file changed, 31 insertions(+), 10 deletions(-)
diff --git a/drivers/input/touchscreen/st1232.c b/drivers/input/touchscreen/st1232.c
index f49566dc96f8..025f43c532d8 100644
--- a/drivers/input/touchscreen/st1232.c
+++ b/drivers/input/touchscreen/st1232.c
@@ -22,6 +22,7 @@
#include <linux/pm_qos.h>
#include <linux/slab.h>
#include <linux/types.h>
+#include <linux/input/ts-virtobj.h>
#define ST1232_TS_NAME "st1232-ts"
#define ST1633_TS_NAME "st1633-ts"
@@ -56,6 +57,7 @@ struct st1232_ts_data {
struct touchscreen_properties prop;
struct dev_pm_qos_request low_latency_req;
struct gpio_desc *reset_gpio;
+ struct ts_virtobj_map *map;
const struct st_chip_info *chip_info;
int read_buf_len;
u8 *read_buf;
@@ -133,6 +135,7 @@ static int st1232_ts_parse_and_report(struct st1232_ts_data *ts)
struct input_mt_pos pos[ST_TS_MAX_FINGERS];
u8 z[ST_TS_MAX_FINGERS];
int slots[ST_TS_MAX_FINGERS];
+ bool button_slot[ST_TS_MAX_FINGERS] = {false};
int n_contacts = 0;
int i;
@@ -143,6 +146,11 @@ static int st1232_ts_parse_and_report(struct st1232_ts_data *ts)
unsigned int x = ((buf[0] & 0x70) << 4) | buf[1];
unsigned int y = ((buf[0] & 0x07) << 8) | buf[2];
+ if (ts_virtobj_button_event(ts->map, ts->input_dev, x, y))
+ button_slot[n_contacts] = true;
+ else if (!ts_virtobj_mt_on_touchscreen(ts->map, &x, &y))
+ continue;
+
touchscreen_set_mt_pos(&pos[n_contacts],
&ts->prop, x, y);
@@ -158,12 +166,16 @@ static int st1232_ts_parse_and_report(struct st1232_ts_data *ts)
for (i = 0; i < n_contacts; i++) {
input_mt_slot(input, slots[i]);
input_mt_report_slot_state(input, MT_TOOL_FINGER, true);
- input_report_abs(input, ABS_MT_POSITION_X, pos[i].x);
- input_report_abs(input, ABS_MT_POSITION_Y, pos[i].y);
+ if (!button_slot[i]) {
+ input_report_abs(input, ABS_MT_POSITION_X, pos[i].x);
+ input_report_abs(input, ABS_MT_POSITION_Y, pos[i].y);
+ }
if (ts->chip_info->have_z)
input_report_abs(input, ABS_MT_TOUCH_MAJOR, z[i]);
}
+ if (!n_contacts)
+ ts_virtobj_button_release_pressed(ts->map, ts->input_dev);
input_mt_sync_frame(input);
input_sync(input);
@@ -266,6 +278,11 @@ static int st1232_ts_probe(struct i2c_client *client)
ts->client = client;
ts->input_dev = input_dev;
+ /* map virtual objects if defined in the device tree */
+ ts->map = ts_virtobj_map_objects(&ts->client->dev, ts->input_dev);
+ if (IS_ERR(ts->map))
+ return PTR_ERR(ts->map);
+
ts->reset_gpio = devm_gpiod_get_optional(&client->dev, NULL,
GPIOD_OUT_HIGH);
if (IS_ERR(ts->reset_gpio)) {
@@ -292,18 +309,22 @@ static int st1232_ts_probe(struct i2c_client *client)
if (error)
return error;
- /* Read resolution from the chip */
- error = st1232_ts_read_resolution(ts, &max_x, &max_y);
- if (error) {
- dev_err(&client->dev,
- "Failed to read resolution: %d\n", error);
- return error;
- }
-
if (ts->chip_info->have_z)
input_set_abs_params(input_dev, ABS_MT_TOUCH_MAJOR, 0,
ts->chip_info->max_area, 0, 0);
+ if (ts_virtobj_mapped_touchscreen(ts->map)) {
+ ts_virtobj_retrieve_abs(ts->map, &max_x, &max_y);
+ } else {
+ /* Read resolution from the chip */
+ error = st1232_ts_read_resolution(ts, &max_x, &max_y);
+ if (error) {
+ dev_err(&client->dev,
+ "Failed to read resolution: %d\n", error);
+ return error;
+ }
+ }
+
input_set_abs_params(input_dev, ABS_MT_POSITION_X,
0, max_x, 0, 0);
input_set_abs_params(input_dev, ABS_MT_POSITION_Y,
--
2.37.2
^ permalink raw reply related
* [RFC v1 4/4] dt-bindings: input: touchscreen: st1232: add virtual objects to the example
From: Javier Carrasco @ 2023-04-25 11:50 UTC (permalink / raw)
To: linux-input, devicetree, linux-kernel
Cc: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Henrik Rydberg,
Javier Carrasco, Ulf Hansson, Hans Verkuil, Stephen Boyd,
Sebastian Reichel, Linus Walleij, Jonathan Cameron, Uwe Kleine-g,
Bastian Hecht, Michael Riesch
In-Reply-To: <20230425115049.870003-1-javier.carrasco@wolfvision.net>
The st1232 driver supports the virtual-touchscreen and virtual-buttons
objects defined in the generic touchscreen bindings. In order to support
the key codes properties within the virtual buttons, add the required
linux-event-codes include as well.
Signed-off-by: Javier Carrasco <javier.carrasco@wolfvision.net>
---
.../input/touchscreen/sitronix,st1232.yaml | 28 +++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/Documentation/devicetree/bindings/input/touchscreen/sitronix,st1232.yaml b/Documentation/devicetree/bindings/input/touchscreen/sitronix,st1232.yaml
index 1d8ca19fd37a..66b8c85135b1 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/sitronix,st1232.yaml
+++ b/Documentation/devicetree/bindings/input/touchscreen/sitronix,st1232.yaml
@@ -37,6 +37,7 @@ unevaluatedProperties: false
examples:
- |
+ #include <dt-bindings/input/linux-event-codes.h>
i2c {
#address-cells = <1>;
#size-cells = <0>;
@@ -46,5 +47,32 @@ examples:
reg = <0x55>;
interrupts = <2 0>;
gpios = <&gpio1 166 0>;
+
+ virtual-touchscreen {
+ x-origin = <0>;
+ x-size = <240>;
+ y-origin = <40>;
+ y-size = <280>;
+ };
+
+ virtual-buttons {
+ button-light {
+ label = "Camera light";
+ linux,code = <KEY_LIGHTS_TOGGLE>;
+ x-origin = <40>;
+ x-size = <40>;
+ y-origin = <0>;
+ y-size = <40>;
+ };
+
+ button-suspend {
+ label = "Suspend";
+ linux,code = <KEY_SUSPEND>;
+ x-origin = <160>;
+ x-size = <40>;
+ y-origin = <0>;
+ y-size = <40>;
+ };
+ };
};
};
--
2.37.2
^ permalink raw reply related
* [PATCH] Fix freeze in lm8333 i2c keyboard driver
From: Tomas Mudrunka @ 2023-04-25 13:00 UTC (permalink / raw)
Cc: Tomas Mudrunka, Dmitry Torokhov, linux-input, linux-kernel
LM8333 uses gpio interrupt line which is active-low.
When interrupt is set to FALLING edge and button is pressed
before driver loads, driver will miss the edge and never respond.
To fix this we handle ONESHOT LOW interrupt rather than edge.
Signed-off-by: Tomas Mudrunka <tomas.mudrunka@gmail.com>
---
drivers/input/keyboard/lm8333.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/input/keyboard/lm8333.c b/drivers/input/keyboard/lm8333.c
index 7457c3220..c5770ebb2 100644
--- a/drivers/input/keyboard/lm8333.c
+++ b/drivers/input/keyboard/lm8333.c
@@ -179,7 +179,7 @@ static int lm8333_probe(struct i2c_client *client)
}
err = request_threaded_irq(client->irq, NULL, lm8333_irq_thread,
- IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
+ IRQF_TRIGGER_LOW | IRQF_ONESHOT,
"lm8333", lm8333);
if (err)
goto free_mem;
--
2.40.0
^ permalink raw reply related
* Re: [PATCH 00/43] ep93xx device tree conversion
From: Arnd Bergmann @ 2023-04-25 13:27 UTC (permalink / raw)
To: Krzysztof Kozlowski, Jakub Kicinski
Cc: Nikita Shubin, Arnd Bergmann, Linus Walleij, Alexander Sverdlin,
David S . Miller, Jonathan Neuschäfer, Russell King,
Uwe Kleine-König, Alessandro Zummo, Alexander Gordeev,
Alexandre Belloni, Andy Shevchenko, Bartosz Golaszewski,
Brian Norris, Chuanhong Guo, Conor.Dooley, Damien Le Moal,
Daniel Lezcano, Dmitry Torokhov, Emil Renner Berthing,
Eric Dumazet, Florian Fainelli, Guenter Roeck, Hartley Sweeten,
Heiko Stübner, Hitomi Hasegawa, Jaroslav Kysela,
Jean Delvare, Joel Stanley, Jonathan Cameron, Krzysztof Kozlowski,
Damien Le Moal, Liam Girdwood, Liang Yang, Linus Walleij,
Lukasz Majewski, Lv Ruyi, Mark Brown, Masahiro Yamada,
Michael Turquette, Miquel Raynal, Nathan Chancellor,
Nick Desaulniers, Nicolas Saenz Julienne, Olof Johansson,
Paolo Abeni, Qin Jian, Richard Weinberger, Rob Herring,
Robert Jarzmik, Russell King, Sebastian Reichel, Sergey Shtylyov,
Stephen Boyd, Sumanth Korikkar, Sven Peter, Takashi Iwai,
Thierry Reding, Thomas Gleixner, Ulf Hansson, Vasily Gorbik,
Vignesh Raghavendra, Vinod Koul, Walker Chen, Wim Van Sebroeck,
Yinbo Zhu, alsa-devel, devicetree, dmaengine, linux-arm-kernel,
linux-clk, open list:GPIO SUBSYSTEM, linux-ide, linux-input,
linux-kernel, linux-mtd, linux-pm, linux-pwm, linux-rtc,
linux-spi, linux-watchdog, Netdev, soc
In-Reply-To: <0210316b-9e21-347c-ed15-ce8200aeeb94@linaro.org>
On Tue, Apr 25, 2023, at 10:20, Krzysztof Kozlowski wrote:
> On 25/04/2023 00:29, Jakub Kicinski wrote:
>> On Mon, 24 Apr 2023 13:31:25 +0200 Arnd Bergmann wrote:
>>> Thanks a lot for your continued work. I can't merge any of this at
>>> the moment since the upstream merge window just opened, but I'm
>>> happy to take this all through the soc tree for 6.5, provided we
>>> get the sufficient Acks from the subsystem maintainers. Merging
>>> it through each individual tree would take a lot longer, so I
>>> hope we can avoid that.
>>
>> Is there a dependency between the patches?
>
> I didn't get entire patchset and cover letter does not mention
> dependencies, but usually there shouldn't be such. Maybe for the next
> versions this should be split per subsystem?
Clearly the last patch that removes the board files depends on
all the previous patches, but I assume that the other ones
are all independent.
We don't do complete conversions from boardfiles to DT that often
any more, but in the past we tended to do this through a cross-
subsystem branch in the soc tree, which helps do it more quickly
and is less work for Nikita. In this case, I would make it a
separate top-level branch in the soc tree.
If anyone strongly feels that the patches should go through
the subsystem trees here, we'll take the longer path and
do the changes separately, with the boardfile removal
coming a release later.
Arnd
^ permalink raw reply
* Re: Proposal: Add a means to disable kernel driver logic when associated hidraw is opened
From: Benjamin Tissoires @ 2023-04-25 14:52 UTC (permalink / raw)
To: Vicki Pfau; +Cc: open list:HID CORE LAYER, Pierre-Loup Griffais
In-Reply-To: <CAO-hwJLcWMujHXaCKdW9n2Y21rtg=7gMkzzd0K95DyC9y3W_Mw@mail.gmail.com>
On Mon, Apr 24, 2023 at 3:07 PM Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
>
> Hi Vicki,
>
> On Fri, Apr 21, 2023 at 4:36 AM Vicki Pfau <vi@endrift.com> wrote:
> >
> > Hello,
> >
> > Following up on a conversation from last year (cf. https://lore.kernel.org/linux-input/CAO-hwJLfY+D0NyCUCncrjcXETKwOBqj1CuHtB_mtGhYfKV0Bww@mail.gmail.com/) about how to approach differing opinions about how drivers should work between userspace and kernel, we're looking for a way to effectively stop the kernel from doing anything "smart" with a a HID device, e.g. a hid-sony device, when the associated hidraw is opened. At the moment, we have a specialized daemon that will find the mouse evdev associated with the controller when it's created and inhibit it, but this is anything but ideal and only handles the mouse itself, not the kernel logic in general. You can also see the hackish way this is implemented in the hid-steam driver, which uses an intermediary hid ll_driver to intercept the open and close commands and stop passing data if the hidraw gets opened. I consider this implementation to be very much a hack and an anti-pattern, and I think other HID implementers would agree, but I'm not sure there's a cleaner way to detect this in the kernel at the moment.
> >
> > I see a bit of a difference of opinion as to what should be happening here between developers on our end and the kernel end. Our position is "we have a userspace driver that does everything we want, we'd like the kernel to stop trying to be smart when our driver is active", and I expect the kernel developer opinion is "why should you have a userspace driver at all when our kernel driver is fine, and if it's not you can upstream patches?", so there's probably some need to find a middle ground if we want things to work well for users in the end.
>
> The main problem we have here, is that if userspace is actively using
> hidraw to configure the device, the kernel is not aware, and thus its
> state will be wrong. So my initial reaction is "we need to have ad-hoc
> and opt-in functionality for these, because the states the kernel
> needs to monitor will differ from one device to one other". But this
> opt-in could (should) be controlled from userspace through eBPF.
>
> OTOH, maybe the simplest solution would be to entirely unbind the
> device from the kernel driver when steam opens and rebinds it when it
> closes. This way the kernel will reinitialize the device itself
> properly and keep its state coherent. We can think of multiple
> solutions for disabling the driver and still keeping hidraw open, and
> guess what, eBPF is one of them :)
>
> Note though that disabling the kernel module might require some
> changes in the kernel driver itself, so maybe that's not the best path
> too. (eBPF can not change any quirks on a given device, but maybe that
> is something we can think of).
>
> >
> > We'd previously discussed an ioctl for the hidraw, or perhaps an eBPF approach, though I think at the time of that discussion, eBPF wasn't mature for the HID subsystem. I don't know the current state of that, or if it's possible to do this with just write access to the hidraw device (the ideal case for how we want to handle this--root is pretty much out of the question for an "ideal case"). Since we still don't have a good answer for this, as far as I'm aware, I'd like to try to reach an approach that's amicable for both sides.
>
> Good timing. Kernel 6.3 was out just yesterday, and HID-BPF is in :)
> (well, not all use cases I envisioned are implemented, but at least
> the bulk is there).
>
> >
> > I know that the ioctl approach meant having to introduce logic for programs in userspace, but in this specific case, and possibly others, that's actually what we're looking for. Further, the reason root is out is because Steam isn't the OS (in most cases), and we want this to only happen when Steam is running. Having to run a daemon as root underneath Steam would require something like a setuid binary or custom sudo/polkit rules, which of course need root to set up anyway. So while eBPF may fit some use cases, e.g. full control over the system via DE or systemd, etc, it's not what we're looking for here.
>
> I think you are dismissing eBPF too quickly :)
>
> Basically, eBPF doesn't require you to have a daemon loaded: you can
> load the BPF program, attach it to the device, pin it in the kernel
> and close your eBPF loader. So all you need is a new udev rule that
> calls for a program to load the eBPF program just at plugging time.
> Eventually, for such "no-daemon" programs, the kernel itself will
> load those programs, and userspace can just override them if they are
> not up to date.
>
> Which means that when you load the eBPF program, you just "patched"
> the kernel with your current API.
>
> [Side note: my push for eBPF is because this is exactly the kind of
> use case I envisioned for it: an app (or set of apps) wants to tweak
> the kernel behavior only when it is started, and wants to be in
> control. If this app wants to change some behaviour, fixing the eBPF
> program is way simpler and can be done in sync with the app update
> rather than updating the API introduced in the kernel]
>
> I think the following (in pseudo-ish-code) would work for
[...]
I got a working WIP at
https://gitlab.freedesktop.org/bentiss/udev-hid-bpf/-/tree/hidraw-close
I need a couple of kernel changes to allow for the introspection of
the hidraw device from the struct hid_device and to let
hid-playstation go with the events.
We probably still want the opt-in capability, so we'd also need the
hid_hw_request() BPF hook set up, but this should give you an idea
that BPF could be a simple and elegant solution.
Cheers,
Benjamin
^ permalink raw reply
* Re: [RFC v1 0/4] Input: support virtual objects on touchscreens
From: Thomas Weißschuh @ 2023-04-25 15:29 UTC (permalink / raw)
To: Javier Carrasco
Cc: linux-input, devicetree, linux-kernel, Dmitry Torokhov,
Rob Herring, Krzysztof Kozlowski, Henrik Rydberg, Ulf Hansson,
Hans Verkuil, Stephen Boyd, Sebastian Reichel, Linus Walleij,
Jonathan Cameron, Uwe Kleine-g, Bastian Hecht, Michael Riesch
In-Reply-To: <20230425115049.870003-1-javier.carrasco@wolfvision.net>
Hi Javier,
On 2023-04-25 13:50:45+0200, Javier Carrasco wrote:
> Some touchscreens are shipped with a physical layer on top of them where
> a number of buttons and a resized touchscreen surface might be available.
>
> In order to generate proper key events by overlay buttons and adjust the
> touch events to a clipped surface, these patches offer a documented,
> device-tree-based solution by means of helper functions.
> An implementation for a specific touchscreen driver is also included.
>
> The functions in ts-virtobj provide a simple workflow to acquire
> physical objects from the device tree, map them into the device driver
> structures as virtual objects and generate events according to
> the object descriptions.
>
> This solution has been tested with a JT240MHQS-E3 display, which uses
> the st1624 as a touchscreen and provides two overly buttons and a frame
> that clips its effective surface.
There are quite a few of notebooks from Asus that feature a printed
numpad on their touchpad [0]. The mapping from the touch events to the
numpad events needs to happen in software.
Do you think your solution is general enough to also support this
usecase?
The differences I see are
* not device-tree based
* touchpads instead of touchscreens
> [..]
[0] https://unix.stackexchange.com/q/494400
^ permalink raw reply
* Re: [PATCH] Fix freeze in lm8333 i2c keyboard driver
From: Jeff LaBundy @ 2023-04-25 15:39 UTC (permalink / raw)
To: Tomas Mudrunka; +Cc: dmitry.torokhov, linux-input, linux-kernel
In-Reply-To: <20230425130054.591007-1-tomas.mudrunka@gmail.com>
Hi Tomas,
On Tue, Apr 25, 2023 at 03:00:53PM +0200, Tomas Mudrunka wrote:
> LM8333 uses gpio interrupt line which is active-low.
> When interrupt is set to FALLING edge and button is pressed
> before driver loads, driver will miss the edge and never respond.
> To fix this we handle ONESHOT LOW interrupt rather than edge.
>
> Signed-off-by: Tomas Mudrunka <tomas.mudrunka@gmail.com>
> ---
> drivers/input/keyboard/lm8333.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/input/keyboard/lm8333.c b/drivers/input/keyboard/lm8333.c
> index 7457c3220..c5770ebb2 100644
> --- a/drivers/input/keyboard/lm8333.c
> +++ b/drivers/input/keyboard/lm8333.c
> @@ -179,7 +179,7 @@ static int lm8333_probe(struct i2c_client *client)
> }
>
> err = request_threaded_irq(client->irq, NULL, lm8333_irq_thread,
> - IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> + IRQF_TRIGGER_LOW | IRQF_ONESHOT,
> "lm8333", lm8333);
> if (err)
> goto free_mem;
Thanks for the patch, but this is a NAK in my opinion.
First of all, we should not be hard-coding interrupt polarity in the
first place; that is an existing piece of technical debt in this driver.
Second, changing from edge-triggered to level-triggered interrupts runs
the risk of creating an interrupt storm depending on the time it takes
the device to deassert the irq following the I2C read and the point at
which the threaded handler returns. Have you measured this?
Can we not simply read the interrupt status registers once at start-up
to clear any pending status? This is essentially what your change does
anyway, albeit indirectly.
> --
> 2.40.0
>
Kind regards,
Jeff LaBundy
^ permalink raw reply
* Re: [PATCH] HID: intel-ish-hid: pci-ish: Fix use after free bug in ish_remove due to race condition
From: srinivas pandruvada @ 2023-04-25 15:55 UTC (permalink / raw)
To: Zheng Wang
Cc: jikos, benjamin.tissoires, rafael, hdegoede, gregkh, linux-input,
linux-kernel, hackerzheng666, 1395428693sheep, alex000young
In-Reply-To: <20230412171441.18958-1-zyytlz.wz@163.com>
On Thu, 2023-04-13 at 01:14 +0800, Zheng Wang wrote:
> In ish_probe, it calls ish_dev_init to init the device. In this
> function,
> ishtp_device_init is called and &dev->bh_hbm_work is bound with
> bh_hbm_work_fn. recv_hbm may be called to start the timer work.
>
> If we remove the module which will call ish_remove to make cleanup,
> there may be an unfinished work. The possible sequence is as follows:
>
> Fix it by canceling the work before cleanup in
> ishtp_bus_remove_all_clients
>
> CPU0 CPUc1
>
> |bh_hbm_work_fn
> ish_remove |
> ishtp_bus_remove_all_clients |
> kfree(ishtp_dev->fw_clients); |
> |
> |&dev->fw_clients[...]
> | //use
>
> Fixes: 3703f53b99e4 ("HID: intel_ish-hid: ISH Transport layer")
> Signed-off-by: Zheng Wang <zyytlz.wz@163.com>
Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
It is a extremely rare case, so it is OK to not mark for stable.
> ---
> drivers/hid/intel-ish-hid/ishtp/bus.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/hid/intel-ish-hid/ishtp/bus.c
> b/drivers/hid/intel-ish-hid/ishtp/bus.c
> index 81385ab37fa9..ada7cd08dbeb 100644
> --- a/drivers/hid/intel-ish-hid/ishtp/bus.c
> +++ b/drivers/hid/intel-ish-hid/ishtp/bus.c
> @@ -744,6 +744,7 @@ void ishtp_bus_remove_all_clients(struct
> ishtp_device *ishtp_dev,
> */
> }
> spin_unlock_irqrestore(&ishtp_dev->cl_list_lock, flags);
> + cancel_work_sync(&ishtp_dev->bh_hbm_work);
>
> /* Release DMA buffers for client messages */
> ishtp_cl_free_dma_buf(ishtp_dev);
^ permalink raw reply
* Re: [RFC v1 0/4] Input: support virtual objects on touchscreens
From: Jeff LaBundy @ 2023-04-25 16:02 UTC (permalink / raw)
To: Thomas Weißschuh
Cc: Javier Carrasco, linux-input, devicetree, linux-kernel,
Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Henrik Rydberg,
Ulf Hansson, Hans Verkuil, Stephen Boyd, Sebastian Reichel,
Linus Walleij, Jonathan Cameron, Uwe Kleine-g, Bastian Hecht,
Michael Riesch
In-Reply-To: <419c9d72-9791-46ff-8317-b4dfe2e2d0a3@t-8ch.de>
Hi Thomas,
On Tue, Apr 25, 2023 at 05:29:39PM +0200, Thomas Weißschuh wrote:
> Hi Javier,
>
> On 2023-04-25 13:50:45+0200, Javier Carrasco wrote:
> > Some touchscreens are shipped with a physical layer on top of them where
> > a number of buttons and a resized touchscreen surface might be available.
> >
> > In order to generate proper key events by overlay buttons and adjust the
> > touch events to a clipped surface, these patches offer a documented,
> > device-tree-based solution by means of helper functions.
> > An implementation for a specific touchscreen driver is also included.
> >
> > The functions in ts-virtobj provide a simple workflow to acquire
> > physical objects from the device tree, map them into the device driver
> > structures as virtual objects and generate events according to
> > the object descriptions.
> >
> > This solution has been tested with a JT240MHQS-E3 display, which uses
> > the st1624 as a touchscreen and provides two overly buttons and a frame
> > that clips its effective surface.
>
> There are quite a few of notebooks from Asus that feature a printed
> numpad on their touchpad [0]. The mapping from the touch events to the
> numpad events needs to happen in software.
That example seems a kind of fringe use-case in my opinion; I think the
gap filled by this RFC is the case where a touchscreen has a printed
overlay with a key that represents a fixed function.
One problem I do see here is something like libinput or multitouch taking
hold of the input device, and swallowing the key presses because it sees
the device as a touchscreen and is not interested in these keys.
Therefore, my first impression is that the virtual keypad may be better
served by registering its own input device.
Great work by the way, Javier!
>
> Do you think your solution is general enough to also support this
> usecase?
>
> The differences I see are
> * not device-tree based
> * touchpads instead of touchscreens
>
> > [..]
>
> [0] https://unix.stackexchange.com/q/494400
Kind regards,
Jeff LaBundy
^ permalink raw reply
* Re: [PATCH v3] Add rumble support to latest xbox controllers
From: Bastien Nocera @ 2023-04-25 16:07 UTC (permalink / raw)
To: Edward Matijevic
Cc: benjamin.tissoires, jikos, linux-input, linux-kernel, svv
In-Reply-To: <254bb806-c5ac-371e-4e25-1cfa5c8ce388@gmail.com>
On Tue, 2023-04-25 at 10:46 -0500, Edward Matijevic wrote:
> > > +#define USB_DEVICE_ID_MS_XBOX_CONTROLLER_MODEL_1708 0x02fd
> > > +#define
> > > USB_DEVICE_ID_MS_XBOX_CONTROLLER_MODEL_1708_BLE 0x0b20
> > The 1708 model uses Bluetooth Classic, not Bluetooth LE.
>
> The new firmware adds Bluetooth LE support to the 1708
> and prioritizes BLE over Classic which necessitates the change
> The controllers are broken without the "new firmware" IDs which are
> for BLE
Oh! I completely missed that. So both the 1708 and 1797 used Bluetooth
Classic with the old firmware, and support Bluetooth LE with the new
one. Am I understanding this correctly?
If that's right, looks like I might need to update Wikipedia ;)
Seeing as you will be updating the patch for that comment style
problem, you could probably add a reference to this article in the
commit message, it seems authoritative enough:
https://news.xbox.com/en-us/2021/09/08/xbox-controller-firmware-update-rolling-out-to-insiders-starting-today/
^ permalink raw reply
* [PATCH v4] Add rumble support to latest xbox controllers
From: Siarhei Vishniakou @ 2023-04-25 16:38 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, linux-input, linux-kernel
Cc: Siarhei Vishniakou, Bastien Nocera
Currently, rumble is only supported via bluetooth on a single xbox
controller, called 'model 1708'. On the back of the device, it's named
'wireless controller for xbox one'. However, in 2021, Microsoft released
a firmware update for this controller. As part of this update, the HID
descriptor of the device changed. The product ID was also changed from
0x02fd to 0x0b20. On this controller, rumble was supported via
hid-microsoft, which matched against the old product id (0x02fd). As a
result, the firmware update broke rumble support on this controller.
See:
https://news.xbox.com/en-us/2021/09/08/xbox-controller-firmware-update-rolling-out-to-insiders-starting-today/
The hid-microsoft driver actually supports rumble on the new firmware,
as well. So simply adding new product id is sufficient to bring back
this support.
After discussing further with the xbox team, it was pointed out that
another xbox controller, xbox elite series 2, can be supported in a
similar way.
Add rumble support for all of these devices in this patch. Two of the
devices have received firmware updates that caused their product id's to
change. Both old and new firmware versions of these devices were tested.
The tested controllers are:
1. 'wireless controller for xbox one', model 1708
2. 'xbox wireless controller', model 1914. This is also sometimes
referred to as 'xbox series S|X'.
3. 'elite series 2', model 1797.
The tested configurations are:
1. model 1708, pid 0x02fd (old firmware)
2. model 1708, pid 0x0b20 (new firmware)
3. model 1914, pid 0x0b13
4. model 1797, pid 0x0b05 (old firmware)
5. model 1797, pid 0x0b22 (new firmware)
I verified rumble support on both bluetooth and usb.
Reviewed-by: Bastien Nocera <hadess@hadess.net>
Signed-off-by: Siarhei Vishniakou <svv@google.com>
---
drivers/hid/hid-ids.h | 10 +++++++++-
drivers/hid/hid-microsoft.c | 11 ++++++++++-
2 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 053853a891c5..e252ea48bb74 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -903,7 +903,15 @@
#define USB_DEVICE_ID_MS_TYPE_COVER_2 0x07a9
#define USB_DEVICE_ID_MS_POWER_COVER 0x07da
#define USB_DEVICE_ID_MS_SURFACE3_COVER 0x07de
-#define USB_DEVICE_ID_MS_XBOX_ONE_S_CONTROLLER 0x02fd
+/*
+ * For a description of the Xbox controller models, refer to:
+ * https://en.wikipedia.org/wiki/Xbox_Wireless_Controller#Summary
+ */
+#define USB_DEVICE_ID_MS_XBOX_CONTROLLER_MODEL_1708 0x02fd
+#define USB_DEVICE_ID_MS_XBOX_CONTROLLER_MODEL_1708_BLE 0x0b20
+#define USB_DEVICE_ID_MS_XBOX_CONTROLLER_MODEL_1914 0x0b13
+#define USB_DEVICE_ID_MS_XBOX_CONTROLLER_MODEL_1797 0x0b05
+#define USB_DEVICE_ID_MS_XBOX_CONTROLLER_MODEL_1797_BLE 0x0b22
#define USB_DEVICE_ID_MS_PIXART_MOUSE 0x00cb
#define USB_DEVICE_ID_8BITDO_SN30_PRO_PLUS 0x02e0
diff --git a/drivers/hid/hid-microsoft.c b/drivers/hid/hid-microsoft.c
index 071fd093a5f4..9345e2bfd56e 100644
--- a/drivers/hid/hid-microsoft.c
+++ b/drivers/hid/hid-microsoft.c
@@ -446,7 +446,16 @@ static const struct hid_device_id ms_devices[] = {
.driver_data = MS_PRESENTER },
{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_MICROSOFT, 0x091B),
.driver_data = MS_SURFACE_DIAL },
- { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_XBOX_ONE_S_CONTROLLER),
+
+ { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_XBOX_CONTROLLER_MODEL_1708),
+ .driver_data = MS_QUIRK_FF },
+ { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_XBOX_CONTROLLER_MODEL_1708_BLE),
+ .driver_data = MS_QUIRK_FF },
+ { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_XBOX_CONTROLLER_MODEL_1914),
+ .driver_data = MS_QUIRK_FF },
+ { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_XBOX_CONTROLLER_MODEL_1797),
+ .driver_data = MS_QUIRK_FF },
+ { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_XBOX_CONTROLLER_MODEL_1797_BLE),
.driver_data = MS_QUIRK_FF },
{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_8BITDO_SN30_PRO_PLUS),
.driver_data = MS_QUIRK_FF },
--
2.40.0.634.g4ca3ef3211-goog
^ permalink raw reply related
* Re: [PATCH v3] Add rumble support to latest xbox controllers
From: Siarhei Vishniakou @ 2023-04-25 16:40 UTC (permalink / raw)
To: Bastien Nocera
Cc: Edward Matijevic, benjamin.tissoires, jikos, linux-input,
linux-kernel
In-Reply-To: <f82bdf01743b11e92873c5c9937ae253bd1c8bb3.camel@hadess.net>
Right, confirmed on the BLE support.
I uploaded a new patch with the C-style comments and added the
xbox.com link to the commit message.
On Tue, Apr 25, 2023 at 9:07 AM Bastien Nocera <hadess@hadess.net> wrote:
>
> On Tue, 2023-04-25 at 10:46 -0500, Edward Matijevic wrote:
> > > > +#define USB_DEVICE_ID_MS_XBOX_CONTROLLER_MODEL_1708 0x02fd
> > > > +#define
> > > > USB_DEVICE_ID_MS_XBOX_CONTROLLER_MODEL_1708_BLE 0x0b20
> > > The 1708 model uses Bluetooth Classic, not Bluetooth LE.
> >
> > The new firmware adds Bluetooth LE support to the 1708
> > and prioritizes BLE over Classic which necessitates the change
> > The controllers are broken without the "new firmware" IDs which are
> > for BLE
>
> Oh! I completely missed that. So both the 1708 and 1797 used Bluetooth
> Classic with the old firmware, and support Bluetooth LE with the new
> one. Am I understanding this correctly?
>
> If that's right, looks like I might need to update Wikipedia ;)
>
> Seeing as you will be updating the patch for that comment style
> problem, you could probably add a reference to this article in the
> commit message, it seems authoritative enough:
> https://news.xbox.com/en-us/2021/09/08/xbox-controller-firmware-update-rolling-out-to-insiders-starting-today/
^ 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