Linux Input/HID development
 help / color / mirror / Atom feed
* Re: Backport request to fix a WARNING in input_mt_init_slots
From: Greg KH @ 2024-08-27 14:19 UTC (permalink / raw)
  To: George Kennedy
  Cc: stable, rydberg, dmitry.torokhov, penguin-kernel, linux-input,
	linux-kernel, harshit.m.mogalapalli
In-Reply-To: <1724766547-24435-1-git-send-email-george.kennedy@oracle.com>

On Tue, Aug 27, 2024 at 08:49:07AM -0500, George Kennedy wrote:
> Hello,
> 
> We have seen a WARNING message while fuzzing with syzkaller.

Then don't run syzkaller :)

Now queued up, thanks.

greg k-h

^ permalink raw reply

* Backport request to fix a WARNING in input_mt_init_slots
From: George Kennedy @ 2024-08-27 13:49 UTC (permalink / raw)
  To: stable, rydberg, dmitry.torokhov, penguin-kernel
  Cc: george.kennedy, linux-input, linux-kernel, harshit.m.mogalapalli

Hello,

We have seen a WARNING message while fuzzing with syzkaller.


Kernel 5.15.165 on an x86_64


------------[ cut here ]------------
WARNING: CPU: 1 PID: 1592 at mm/page_alloc.c:5398 __alloc_pages+0x4aa/0x5b0 mm/page_alloc.c:5398
Modules linked in:
CPU: 1 PID: 1592 Comm: syz-executor777 Not tainted 5.15.165-rc1-305-ge122be7431ef1-syzk #1
Hardware name: Red Hat KVM, BIOS 1.16.0-4.module+el8.9.0+90052+d3bf71d8 04/01/2014
RIP: 0010:__alloc_pages+0x4aa/0x5b0 mm/page_alloc.c:5398
Code: 00 48 89 44 24 58 e9 fa fc ff ff 48 89 f2 48 89 c7 44 89 c6 e8 77 32 f2 ff 49 89 c7 e9 72 fd ff ff 80 e7 20 0f 85 d8 fe ff ff <0f> 0b e9 d1 fe ff ff a9 00 00 08 00 75 48 89 da 80 e2 7f a9 00 00
RSP: 0018:ffff88801c18fb58 EFLAGS: 00010246
RAX: 0000000000000000 RBX: 00000000000400c0 RCX: 0000000000000000
RDX: dffffc0000000000 RSI: 0000000000000017 RDI: 0000000000040dc0
RBP: 1ffff11003831f6f R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000001
R13: 0000000000000017 R14: 0000000000000000 R15: 0000000000000000
FS:  00007ff8aaa97740(0000) GS:ffff888107080000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007ff8aa38a520 CR3: 0000000022534000 CR4: 00000000000006e0
Call Trace:
 <TASK>
 alloc_pages+0x21e/0x3d0 mm/mempolicy.c:2185
 kmalloc_order+0x31/0xb0 mm/slab_common.c:966
 kmalloc_order_trace+0x19/0xa0 mm/slab_common.c:982
 kmalloc include/linux/slab.h:596 [inline]
 kzalloc include/linux/slab.h:721 [inline]
 input_mt_init_slots+0xf6/0x620 drivers/input/input-mt.c:49
 uinput_create_device+0x1e6/0x6e0 drivers/input/misc/uinput.c:327
 uinput_ioctl_handler.isra.0+0x46f/0x15e0 drivers/input/misc/uinput.c:870
 vfs_ioctl fs/ioctl.c:51 [inline]
 __do_sys_ioctl fs/ioctl.c:874 [inline]
 __se_sys_ioctl fs/ioctl.c:860 [inline]
 __x64_sys_ioctl+0x199/0x220 fs/ioctl.c:860
 do_syscall_x64 arch/x86/entry/common.c:50 [inline]
 do_syscall_64+0x35/0x90 arch/x86/entry/common.c:80
 entry_SYSCALL_64_after_hwframe+0x6c/0xd6
RIP: 0033:0x7ff8aa38a53d
Code: 00 c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 1b 79 2c 00 f7 d8 64 89 01 48
RSP: 002b:00007ffc7eebe838 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007ff8aa38a53d
RDX: 0000000000000000 RSI: 0000000000005501 RDI: 0000000000000003
RBP: 00000000004017a0 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 431bde82d7b634db
R13: 00007ffc7eebe960 R14: 0000000000000000 R15: 0000000000000000
 </TASK>
---[ end trace ced5c0b641032976 ]---

Fix commit: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=99d3bf5f7377d42f8be60a6b9cb60fb0be34dceb

Can you please backport this commit to stable kernels on 5.15.y (and 
other stable kernels 6.1.y, 6.6.y)

commit: 99d3bf5f7377 ("Input: MT - limit max slots")

author	Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>	2024-07-29 21:51:30 +0900
committer	Linus Torvalds <torvalds@linux-foundation.org>	2024-07-29 10:44:48 -0700
commit	99d3bf5f7377d42f8be60a6b9cb60fb0be34dceb (patch)
tree	78dd9dff2065f2eaf5a9e981f84d56eed2346d10
parent	3894840a7a11aa06cc3b0d5a2d1b5f6878127903 (diff)
download	linux-99d3bf5f7377d42f8be60a6b9cb60fb0be34dceb.tar.gz
Input: MT - limit max slots
syzbot is reporting too large allocation at input_mt_init_slots(), for
num_slots is supplied from userspace using ioctl(UI_DEV_CREATE).

Since nobody knows possible max slots, this patch chose 1024.

Reported-by: syzbot <syzbot+0122fa359a69694395d5@syzkaller.appspotmail.com>
Closes: https://syzkaller.appspot.com/bug?extid=0122fa359a69694395d5
Suggested-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Diffstat
-rw-r--r--	drivers/input/input-mt.c	3	
		
1 files changed, 3 insertions, 0 deletions
diff --git a/drivers/input/input-mt.c b/drivers/input/input-mt.c
index 14b53dac1253bf..6b04a674f832a0 100644
--- a/drivers/input/input-mt.c
+++ b/drivers/input/input-mt.c
@@ -46,6 +46,9 @@ int input_mt_init_slots(struct input_dev *dev, unsigned int num_slots,
 		return 0;
 	if (mt)
 		return mt->num_slots != num_slots ? -EINVAL : 0;
+	/* Arbitrary limit for avoiding too large memory allocation. */
+	if (num_slots > 1024)
+		return -EINVAL;
 
 	mt = kzalloc(struct_size(mt, slots, num_slots), GFP_KERNEL);
 	if (!mt)

^ permalink raw reply related

* [PATCH -next] Input: mt6779-keypad - fix module autoloading
From: Liao Chen @ 2024-08-27 12:34 UTC (permalink / raw)
  To: linux-input, linux-kernel, linux-arm-kernel, linux-mediatek
  Cc: mkorpershoek, dmitry.torokhov, matthias.bgg,
	angelogioacchino.delregno

Add MODULE_DEVICE_TABLE(), so modules could be properly autoloaded
based on the alias from of_device_id table.

Signed-off-by: Liao Chen <liaochen4@huawei.com>
---
 drivers/input/keyboard/mt6779-keypad.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/input/keyboard/mt6779-keypad.c b/drivers/input/keyboard/mt6779-keypad.c
index 19f69d167fbd..6d2557a9b9f5 100644
--- a/drivers/input/keyboard/mt6779-keypad.c
+++ b/drivers/input/keyboard/mt6779-keypad.c
@@ -260,6 +260,7 @@ static const struct of_device_id mt6779_keypad_of_match[] = {
 	{ .compatible = "mediatek,mt6873-keypad" },
 	{ /* sentinel */ }
 };
+MODULE_DEVICE_TABLE(of, mt6779_keypad_of_match);
 
 static struct platform_driver mt6779_keypad_pdrv = {
 	.probe = mt6779_keypad_pdrv_probe,
-- 
2.34.1


^ permalink raw reply related

* RE: [PATCH] Input: keypad-nomadik-ske - remove the driver
From: Hennerich, Michael @ 2024-08-27  9:52 UTC (permalink / raw)
  To: Dmitry Torokhov, Arnd Bergmann, Nuno Sá
  Cc: Linus Walleij, linux-input@vger.kernel.org,
	linux-kernel@vger.kernel.org, Lee Jones,
	linux-arm-kernel@lists.infradead.org, Agarwal, Utsav
In-Reply-To: <ZsNcpom_Fm5uCyEj@google.com>



> -----Original Message-----
> From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Sent: Monday, August 19, 2024 4:55 PM
> To: Arnd Bergmann <arnd@arndb.de>; Nuno Sá <noname.nuno@gmail.com>;
> Hennerich, Michael <Michael.Hennerich@analog.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>; linux-input@vger.kernel.org; linux-
> kernel@vger.kernel.org; Lee Jones <lee@kernel.org>; linux-arm-
> kernel@lists.infradead.org; Agarwal, Utsav <Utsav.Agarwal@analog.com>
> Subject: Re: [PATCH] Input: keypad-nomadik-ske - remove the driver
> 
> On Mon, Aug 19, 2024 at 11:29:32AM +0200, Arnd Bergmann wrote:
> > On Fri, Aug 16, 2024, at 20:54, Dmitry Torokhov wrote:
> > > The users of this driver were removed in 2013 in commit 28633c54bda6
> > > ("ARM: ux500: Rip out keypad initialisation which is no longer used").
> > >
> > > Remove the driver as well.
> > >
> > > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > > ---
> > >  drivers/input/keyboard/Kconfig                |  11 -
> > >  drivers/input/keyboard/Makefile               |   1 -
> > >  drivers/input/keyboard/nomadik-ske-keypad.c   | 378 ------------------
> > >  .../linux/platform_data/keypad-nomadik-ske.h  |  50 ---
> > >  4 files changed, 440 deletions(-)
> > >
> >
> > Acked-by: Arnd Bergmann <arnd@arndb.de>
> >
> > I have a list of drivers that I determined to be likely unused as well
> > and found a few more input drivers that were unused in 2022:
> >
> > CONFIG_KEYBOARD_ADP5520/CONFIG_PMIC_ADP5520
> > CONFIG_KEYBOARD_ADP5589
> > CONFIG_INPUT_AD714X
> > CONFIG_TOUCHSCREEN_AD7877
> >
> > As far as I can tell, these all lost their last device definition, or
> > they never had one and are impossible to be used with device tree
> > data.
> 
> I asked Analog Devices folks (CCed) about 5589 and Nuno said that it is still
> relevant and promised to do conversion to DT similar to adp5588.
> 
> Nuno, Michale, what about the other drivers that Arnd listed?

We'll provide dt conversion patches for AD7877 and AD714x as well.

Regards,
Michael

> 
> Thanks.
> 
> --
> Dmitry

^ permalink raw reply

* Re: [PATCH v8 6/7] rtc: support i.MX95 BBM RTC
From: Sudeep Holla @ 2024-08-27 10:34 UTC (permalink / raw)
  To: Peng Fan (OSS), Alexandre Belloni
  Cc: Cristian Marussi, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	Peng Fan, Dmitry Torokhov, arm-scmi, linux-arm-kernel, devicetree,
	linux-kernel, imx, linux-rtc, linux-input
In-Reply-To: <20240823-imx95-bbm-misc-v2-v8-6-e600ed9e9271@nxp.com>

Hi Alexandre,

On Fri, Aug 23, 2024 at 05:05:22PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> The BBM module provides RTC feature. To i.MX95, this module is managed by
> System Manager and exported System Control Management Interface(SCMI).
> Linux could use i.MX SCMI BBM Extension protocol to use RTC feature.
> 
> This driver is to use SCMI interface to get/set RTC.
>

Are you fine if I take this along with dependent SCMI changes via SoC tree ?

-- 
Regards,
Sudeep

^ permalink raw reply

* RE: [PATCH RESEND v11 2/3] Input: adp5588-keys - add support for pure gpio
From: Agarwal, Utsav @ 2024-08-27 10:03 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Hennerich, Michael, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Sa, Nuno, linux-input@vger.kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	Artamonovs, Arturs, Bimpikas, Vasileios, Gaskell, Oliver
In-Reply-To: <ZszU5xzd6S8JKd5E@google.com>

Hi Dmitry,

> -----Original Message-----
> From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Sent: Monday, August 26, 2024 8:18 PM
> To: Agarwal, Utsav <Utsav.Agarwal@analog.com>
> Cc: Hennerich, Michael <Michael.Hennerich@analog.com>; Rob Herring
> <robh@kernel.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor
> Dooley <conor+dt@kernel.org>; Sa, Nuno <Nuno.Sa@analog.com>; linux-
> input@vger.kernel.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; Artamonovs, Arturs
> <Arturs.Artamonovs@analog.com>; Bimpikas, Vasileios
> <Vasileios.Bimpikas@analog.com>; Gaskell, Oliver
> <Oliver.Gaskell@analog.com>
> Subject: Re: [PATCH RESEND v11 2/3] Input: adp5588-keys - add support for
> pure gpio
> 
> [External]
> 
> Hi Utsav,
> 
> On Mon, Aug 26, 2024 at 06:22:02PM +0100, Utsav Agarwal via B4 Relay
> wrote:
> > From: Utsav Agarwal <utsav.agarwal@analog.com>
> >
> > Keypad specific setup is relaxed if no keypad rows/columns are specified,
> > enabling a purely gpio operation.
> >
> > Reviewed-by: Nuno Sa <nuno.sa@analog.com>
> > Signed-off-by: Utsav Agarwal <utsav.agarwal@analog.com>
> > ---
> >  drivers/input/keyboard/adp5588-keys.c | 37
> +++++++++++++++++++++++++++++++----
> >  1 file changed, 33 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/input/keyboard/adp5588-keys.c
> b/drivers/input/keyboard/adp5588-keys.c
> > index 09bcfc6b9408..7c32f8b69a3e 100644
> > --- a/drivers/input/keyboard/adp5588-keys.c
> > +++ b/drivers/input/keyboard/adp5588-keys.c
> > @@ -188,6 +188,7 @@ struct adp5588_kpad {
> >  	u32 cols;
> >  	u32 unlock_keys[2];
> >  	int nkeys_unlock;
> > +	bool gpio_only;
> >  	unsigned short keycode[ADP5588_KEYMAPSIZE];
> >  	unsigned char gpiomap[ADP5588_MAXGPIO];
> >  	struct gpio_chip gc;
> > @@ -431,10 +432,12 @@ static int adp5588_gpio_add(struct adp5588_kpad
> *kpad)
> >  	kpad->gc.label = kpad->client->name;
> >  	kpad->gc.owner = THIS_MODULE;
> >
> > -	girq = &kpad->gc.irq;
> > -	gpio_irq_chip_set_chip(girq, &adp5588_irq_chip);
> > -	girq->handler = handle_bad_irq;
> > -	girq->threaded = true;
> > +	if (kpad->client->irq) {
> > +		girq = &kpad->gc.irq;
> > +		gpio_irq_chip_set_chip(girq, &adp5588_irq_chip);
> > +		girq->handler = handle_bad_irq;
> > +		girq->threaded = true;
> > +	}
> 
> I think we should only set up irqchip if we have "interrupt-controller"
> property in the device tree.
> 
> >
> >  	mutex_init(&kpad->gpio_lock);
> >
> > @@ -632,6 +635,21 @@ static int adp5588_fw_parse(struct adp5588_kpad
> *kpad)
> >  	struct i2c_client *client = kpad->client;
> >  	int ret, i;
> >
> > +	/*
> > +	 * Check if the device is to be operated purely in GPIO mode. To do
> > +	 * so, check that no keypad rows or columns have been specified,
> > +	 * since all GPINS should be configured as GPIO.
> > +	 */
> > +	ret = device_property_present(&client->dev,
> > +			"keypad,num-rows");
> > +	ret |= device_property_present(&client->dev,
> > +			"keypad,num-columns");
> > +	/* If purely GPIO, skip keypad setup */
> > +	if (!ret) {
> > +		kpad->gpio_only = true;
> > +		return 0;
> > +	}
> > +
> >  	ret = matrix_keypad_parse_properties(&client->dev, &kpad->rows,
> >  					     &kpad->cols);
> >  	if (ret)
> > @@ -775,6 +793,11 @@ static int adp5588_probe(struct i2c_client *client)
> >  	if (error)
> >  		return error;
> >
> > +	if (kpad->gpio_only && !client->irq) {
> > +		dev_info(&client->dev, "Rev.%d, started as GPIO only\n",
> revid);
> > +		return 0;
> > +	}
> 
> This is way too noisy. I think one message logging the revision ID
> should be enough. The rest of the data can be found from elsewhere if
> needed.
> 
> Can you try the below on top of yours? If this works I'' squash it
> together with your change.

I have applied and tested the same on relevant hardware, it works as
intended.

> Thanks.
>
> --
> Dmitry
> 
- Utsav
> 
> diff --git a/drivers/input/keyboard/adp5588-keys.c
> b/drivers/input/keyboard/adp5588-keys.c
> index 7c32f8b69a3e..b5f4becf5cb6 100644
> --- a/drivers/input/keyboard/adp5588-keys.c
> +++ b/drivers/input/keyboard/adp5588-keys.c
> @@ -432,7 +432,12 @@ static int adp5588_gpio_add(struct adp5588_kpad
> *kpad)
>  	kpad->gc.label = kpad->client->name;
>  	kpad->gc.owner = THIS_MODULE;
> 
> -	if (kpad->client->irq) {
> +	if (device_property_present(dev, "interrupt-controller")) {
> +		if (!kpad->client->irq) {
> +			dev_err(dev, "Unable to serve as interrupt controller
> without interrupt");
> +			return -EINVAL;
> +		}
> +
>  		girq = &kpad->gc.irq;
>  		gpio_irq_chip_set_chip(girq, &adp5588_irq_chip);
>  		girq->handler = handle_bad_irq;
> @@ -640,12 +645,9 @@ static int adp5588_fw_parse(struct adp5588_kpad
> *kpad)
>  	 * so, check that no keypad rows or columns have been specified,
>  	 * since all GPINS should be configured as GPIO.
>  	 */
> -	ret = device_property_present(&client->dev,
> -			"keypad,num-rows");
> -	ret |= device_property_present(&client->dev,
> -			"keypad,num-columns");
> -	/* If purely GPIO, skip keypad setup */
> -	if (!ret) {
> +	if (!device_property_present(&client->dev, "keypad,num-rows") &&
> +	    !device_property_present(&client->dev, "keypad,num-columns"))
> {
> +		/* If purely GPIO, skip keypad setup */
>  		kpad->gpio_only = true;
>  		return 0;
>  	}
> @@ -793,28 +795,19 @@ static int adp5588_probe(struct i2c_client *client)
>  	if (error)
>  		return error;
> 
> -	if (kpad->gpio_only && !client->irq) {
> -		dev_info(&client->dev, "Rev.%d, started as GPIO only\n",
> revid);
> -		return 0;
> -	}
> -
> -	error = devm_request_threaded_irq(&client->dev, client->irq,
> -					  adp5588_hard_irq,
> adp5588_thread_irq,
> -					  IRQF_TRIGGER_FALLING |
> IRQF_ONESHOT,
> -					  client->dev.driver->name, kpad);
> -	if (error) {
> -		dev_err(&client->dev, "failed to request irq %d: %d\n",
> -			client->irq, error);
> -		return error;
> -	}
> -
> -	if (kpad->gpio_only) {
> -		dev_info(&client->dev, "Rev.%d GPIO only, irq %d\n",
> -				revid, client->irq);
> -		return 0;
> +	if (client->irq) {
> +		error = devm_request_threaded_irq(&client->dev, client-
> >irq,
> +						  adp5588_hard_irq,
> adp5588_thread_irq,
> +						  IRQF_TRIGGER_FALLING |
> IRQF_ONESHOT,
> +						  client->dev.driver->name,
> kpad);
> +		if (error) {
> +			dev_err(&client->dev, "failed to request irq %d:
> %d\n",
> +				client->irq, error);
> +			return error;
> +		}
>  	}
> 
> -	dev_info(&client->dev, "Rev.%d keypad, irq %d\n", revid, client-
> >irq);
> +	dev_info(&client->dev, "Rev.%d controller\n", revid);
>  	return 0;
>  }
> 

^ permalink raw reply

* Re: [PATCH 17/17] Input: tegra-kbc - use guard notation when acquiring mutex and spinlock
From: Thierry Reding @ 2024-08-27  9:08 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: linux-input, Michael Hennerich, Shawn Guo, Sascha Hauer,
	Fabio Estevam, Laxman Dewangan, Hans de Goede, Tony Lindgren,
	Jeff LaBundy, linux-kernel, imx, linux-arm-kernel, linux-tegra
In-Reply-To: <20240825051627.2848495-18-dmitry.torokhov@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 480 bytes --]

On Sat, Aug 24, 2024 at 10:16:21PM GMT, Dmitry Torokhov wrote:
> This makes the code more compact and error handling more robust
> by ensuring that locks are released in all code paths when control
> leaves critical section.
> 
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
>  drivers/input/keyboard/tegra-kbc.c | 45 +++++++++++++-----------------
>  1 file changed, 19 insertions(+), 26 deletions(-)

Acked-by: Thierry Reding <treding@nvidia.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* [PATCH v5 4/4] selftests/hid: Add HIDIOCREVOKE tests
From: Benjamin Tissoires @ 2024-08-27  8:19 UTC (permalink / raw)
  To: Jiri Kosina, Shuah Khan, Peter Hutterer
  Cc: linux-input, linux-kernel, linux-kselftest, Benjamin Tissoires
In-Reply-To: <20240827-hidraw-revoke-v5-0-d004a7451aea@kernel.org>

Add 4 tests for the new revoke ioctl, for read/write/ioctl and poll.

Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>

---

new in v4

Changes to v4:
- fix a few error messages
- also check for ENODEV errno when required
- use ENODEV instead of its plain value
---
 tools/testing/selftests/hid/hidraw.c | 147 +++++++++++++++++++++++++++++++++++
 1 file changed, 147 insertions(+)

diff --git a/tools/testing/selftests/hid/hidraw.c b/tools/testing/selftests/hid/hidraw.c
index 37372709130c..f8b4f7ff292c 100644
--- a/tools/testing/selftests/hid/hidraw.c
+++ b/tools/testing/selftests/hid/hidraw.c
@@ -3,6 +3,11 @@
 
 #include "hid_common.h"
 
+/* for older kernels */
+#ifndef HIDIOCREVOKE
+#define HIDIOCREVOKE	      _IOW('H', 0x0D, int) /* Revoke device access */
+#endif /* HIDIOCREVOKE */
+
 FIXTURE(hidraw) {
 	int dev_id;
 	int uhid_fd;
@@ -84,6 +89,148 @@ TEST_F(hidraw, raw_event)
 	ASSERT_EQ(buf[1], 42);
 }
 
+/*
+ * After initial opening/checks of hidraw, revoke the hidraw
+ * node and check that we can not read any more data.
+ */
+TEST_F(hidraw, raw_event_revoked)
+{
+	__u8 buf[10] = {0};
+	int err;
+
+	/* inject one event */
+	buf[0] = 1;
+	buf[1] = 42;
+	uhid_send_event(_metadata, self->uhid_fd, buf, 6);
+
+	/* read the data from hidraw */
+	memset(buf, 0, sizeof(buf));
+	err = read(self->hidraw_fd, buf, sizeof(buf));
+	ASSERT_EQ(err, 6) TH_LOG("read_hidraw");
+	ASSERT_EQ(buf[0], 1);
+	ASSERT_EQ(buf[1], 42);
+
+	/* call the revoke ioctl */
+	err = ioctl(self->hidraw_fd, HIDIOCREVOKE, NULL);
+	ASSERT_OK(err) TH_LOG("couldn't revoke the hidraw fd");
+
+	/* inject one other event */
+	buf[0] = 1;
+	buf[1] = 43;
+	uhid_send_event(_metadata, self->uhid_fd, buf, 6);
+
+	/* read the data from hidraw */
+	memset(buf, 0, sizeof(buf));
+	err = read(self->hidraw_fd, buf, sizeof(buf));
+	ASSERT_EQ(err, -1) TH_LOG("read_hidraw");
+	ASSERT_EQ(errno, ENODEV) TH_LOG("unexpected error code while reading the hidraw node: %d",
+					errno);
+}
+
+/*
+ * Revoke the hidraw node and check that we can not do any ioctl.
+ */
+TEST_F(hidraw, ioctl_revoked)
+{
+	int err, desc_size = 0;
+
+	/* call the revoke ioctl */
+	err = ioctl(self->hidraw_fd, HIDIOCREVOKE, NULL);
+	ASSERT_OK(err) TH_LOG("couldn't revoke the hidraw fd");
+
+	/* do an ioctl */
+	err = ioctl(self->hidraw_fd, HIDIOCGRDESCSIZE, &desc_size);
+	ASSERT_EQ(err, -1) TH_LOG("ioctl_hidraw");
+	ASSERT_EQ(errno, ENODEV) TH_LOG("unexpected error code while doing an ioctl: %d",
+					errno);
+}
+
+/*
+ * Setup polling of the fd, and check that revoke works properly.
+ */
+TEST_F(hidraw, poll_revoked)
+{
+	struct pollfd pfds[1];
+	__u8 buf[10] = {0};
+	int err, ready;
+
+	/* setup polling */
+	pfds[0].fd = self->hidraw_fd;
+	pfds[0].events = POLLIN;
+
+	/* inject one event */
+	buf[0] = 1;
+	buf[1] = 42;
+	uhid_send_event(_metadata, self->uhid_fd, buf, 6);
+
+	while (true) {
+		ready = poll(pfds, 1, 5000);
+		ASSERT_EQ(ready, 1) TH_LOG("poll return value");
+
+		if (pfds[0].revents & POLLIN) {
+			memset(buf, 0, sizeof(buf));
+			err = read(self->hidraw_fd, buf, sizeof(buf));
+			ASSERT_EQ(err, 6) TH_LOG("read_hidraw");
+			ASSERT_EQ(buf[0], 1);
+			ASSERT_EQ(buf[1], 42);
+
+			/* call the revoke ioctl */
+			err = ioctl(self->hidraw_fd, HIDIOCREVOKE, NULL);
+			ASSERT_OK(err) TH_LOG("couldn't revoke the hidraw fd");
+		} else {
+			break;
+		}
+	}
+
+	ASSERT_TRUE(pfds[0].revents & POLLHUP);
+}
+
+/*
+ * After initial opening/checks of hidraw, revoke the hidraw
+ * node and check that we can not read any more data.
+ */
+TEST_F(hidraw, write_event_revoked)
+{
+	struct timespec time_to_wait;
+	__u8 buf[10] = {0};
+	int err;
+
+	/* inject one event from hidraw */
+	buf[0] = 1; /* report ID */
+	buf[1] = 2;
+	buf[2] = 42;
+
+	pthread_mutex_lock(&uhid_output_mtx);
+
+	memset(output_report, 0, sizeof(output_report));
+	clock_gettime(CLOCK_REALTIME, &time_to_wait);
+	time_to_wait.tv_sec += 2;
+
+	err = write(self->hidraw_fd, buf, 3);
+	ASSERT_EQ(err, 3) TH_LOG("unexpected error while writing to hidraw node: %d", err);
+
+	err = pthread_cond_timedwait(&uhid_output_cond, &uhid_output_mtx, &time_to_wait);
+	ASSERT_OK(err) TH_LOG("error while calling waiting for the condition");
+
+	ASSERT_EQ(output_report[0], 1);
+	ASSERT_EQ(output_report[1], 2);
+	ASSERT_EQ(output_report[2], 42);
+
+	/* call the revoke ioctl */
+	err = ioctl(self->hidraw_fd, HIDIOCREVOKE, NULL);
+	ASSERT_OK(err) TH_LOG("couldn't revoke the hidraw fd");
+
+	/* inject one other event */
+	buf[0] = 1;
+	buf[1] = 43;
+	err = write(self->hidraw_fd, buf, 3);
+	ASSERT_LT(err, 0) TH_LOG("unexpected success while writing to hidraw node: %d", err);
+	ASSERT_EQ(errno, ENODEV) TH_LOG("unexpected error code while writing to hidraw node: %d",
+					errno);
+
+	pthread_mutex_unlock(&uhid_output_mtx);
+}
+
 int main(int argc, char **argv)
 {
 	return test_harness_run(argc, argv);

-- 
2.46.0


^ permalink raw reply related

* [PATCH v5 3/4] selftests/hid: Add initial hidraw tests skeleton
From: Benjamin Tissoires @ 2024-08-27  8:19 UTC (permalink / raw)
  To: Jiri Kosina, Shuah Khan, Peter Hutterer
  Cc: linux-input, linux-kernel, linux-kselftest, Benjamin Tissoires
In-Reply-To: <20240827-hidraw-revoke-v5-0-d004a7451aea@kernel.org>

Largely inspired from hid_bpf.c for the fixture setup.

Create a couple of tests for hidraw:
- create a uhid device and check if the fixture is working properly
- inject one uhid event and read it through hidraw

These tests are not that useful for now, but will be once we start adding
the ioctl and BPFs to revoke the hidraw node.

Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>

---

new in v4

Changes to v4:
- dropped the common code in favor of hid_common.h
---
 tools/testing/selftests/hid/.gitignore |  1 +
 tools/testing/selftests/hid/Makefile   |  2 +-
 tools/testing/selftests/hid/hidraw.c   | 90 ++++++++++++++++++++++++++++++++++
 3 files changed, 92 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/hid/.gitignore b/tools/testing/selftests/hid/.gitignore
index 995af0670f69..746c62361f77 100644
--- a/tools/testing/selftests/hid/.gitignore
+++ b/tools/testing/selftests/hid/.gitignore
@@ -2,4 +2,5 @@ bpftool
 *.skel.h
 /tools
 hid_bpf
+hidraw
 results
diff --git a/tools/testing/selftests/hid/Makefile b/tools/testing/selftests/hid/Makefile
index 2b5ea18bde38..72be55ac4bdf 100644
--- a/tools/testing/selftests/hid/Makefile
+++ b/tools/testing/selftests/hid/Makefile
@@ -32,7 +32,7 @@ CFLAGS += -Wno-unused-command-line-argument
 endif
 
 # Order correspond to 'make run_tests' order
-TEST_GEN_PROGS = hid_bpf
+TEST_GEN_PROGS = hid_bpf hidraw
 
 # Emit succinct information message describing current building step
 # $1 - generic step name (e.g., CC, LINK, etc);
diff --git a/tools/testing/selftests/hid/hidraw.c b/tools/testing/selftests/hid/hidraw.c
new file mode 100644
index 000000000000..37372709130c
--- /dev/null
+++ b/tools/testing/selftests/hid/hidraw.c
@@ -0,0 +1,90 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2022-2024 Red Hat */
+
+#include "hid_common.h"
+
+FIXTURE(hidraw) {
+	int dev_id;
+	int uhid_fd;
+	int hidraw_fd;
+	int hid_id;
+	pthread_t tid;
+};
+static void close_hidraw(FIXTURE_DATA(hidraw) * self)
+{
+	if (self->hidraw_fd)
+		close(self->hidraw_fd);
+	self->hidraw_fd = 0;
+}
+
+FIXTURE_TEARDOWN(hidraw) {
+	void *uhid_err;
+
+	uhid_destroy(_metadata, self->uhid_fd);
+
+	close_hidraw(self);
+	pthread_join(self->tid, &uhid_err);
+}
+#define TEARDOWN_LOG(fmt, ...) do { \
+	TH_LOG(fmt, ##__VA_ARGS__); \
+	hidraw_teardown(_metadata, self, variant); \
+} while (0)
+
+FIXTURE_SETUP(hidraw)
+{
+	time_t t;
+	int err;
+
+	/* initialize random number generator */
+	srand((unsigned int)time(&t));
+
+	self->dev_id = rand() % 1024;
+
+	self->uhid_fd = setup_uhid(_metadata, self->dev_id);
+
+	/* locate the uev, self, variant);ent file of the created device */
+	self->hid_id = get_hid_id(self->dev_id);
+	ASSERT_GT(self->hid_id, 0)
+		TEARDOWN_LOG("Could not locate uhid device id: %d", self->hid_id);
+
+	err = uhid_start_listener(_metadata, &self->tid, self->uhid_fd);
+	ASSERT_EQ(0, err) TEARDOWN_LOG("could not start udev listener: %d", err);
+
+	self->hidraw_fd = open_hidraw(self->dev_id);
+	ASSERT_GE(self->hidraw_fd, 0) TH_LOG("open_hidraw");
+}
+
+/*
+ * A simple test to see if the fixture is working fine.
+ * If this fails, none of the other tests will pass.
+ */
+TEST_F(hidraw, test_create_uhid)
+{
+}
+
+/*
+ * Inject one event in the uhid device,
+ * check that we get the same data through hidraw
+ */
+TEST_F(hidraw, raw_event)
+{
+	__u8 buf[10] = {0};
+	int err;
+
+	/* inject one event */
+	buf[0] = 1;
+	buf[1] = 42;
+	uhid_send_event(_metadata, self->uhid_fd, buf, 6);
+
+	/* read the data from hidraw */
+	memset(buf, 0, sizeof(buf));
+	err = read(self->hidraw_fd, buf, sizeof(buf));
+	ASSERT_EQ(err, 6) TH_LOG("read_hidraw");
+	ASSERT_EQ(buf[0], 1);
+	ASSERT_EQ(buf[1], 42);
+}
+
+int main(int argc, char **argv)
+{
+	return test_harness_run(argc, argv);
+}

-- 
2.46.0


^ permalink raw reply related

* [PATCH v5 2/4] selftests/hid: extract the utility part of hid_bpf.c into its own header
From: Benjamin Tissoires @ 2024-08-27  8:19 UTC (permalink / raw)
  To: Jiri Kosina, Shuah Khan, Peter Hutterer
  Cc: linux-input, linux-kernel, linux-kselftest, Benjamin Tissoires
In-Reply-To: <20240827-hidraw-revoke-v5-0-d004a7451aea@kernel.org>

When adding new tests programs, we need the same mechanics to create
new virtual devices, and read from their matching hidraw node.

Extract the common part into its own header so we can easily add new
tests C-files.

Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>

---

new in v5
---
 tools/testing/selftests/hid/hid_bpf.c    | 437 +------------------------------
 tools/testing/selftests/hid/hid_common.h | 436 ++++++++++++++++++++++++++++++
 2 files changed, 438 insertions(+), 435 deletions(-)

diff --git a/tools/testing/selftests/hid/hid_bpf.c b/tools/testing/selftests/hid/hid_bpf.c
index 75b7b4ef6cfa..d10cf6883683 100644
--- a/tools/testing/selftests/hid/hid_bpf.c
+++ b/tools/testing/selftests/hid/hid_bpf.c
@@ -1,93 +1,8 @@
 // SPDX-License-Identifier: GPL-2.0
-/* Copyright (c) 2022 Red Hat */
+/* Copyright (c) 2022-2024 Red Hat */
 #include "hid.skel.h"
-
-#include "../kselftest_harness.h"
-
+#include "hid_common.h"
 #include <bpf/bpf.h>
-#include <fcntl.h>
-#include <fnmatch.h>
-#include <dirent.h>
-#include <poll.h>
-#include <pthread.h>
-#include <stdbool.h>
-#include <linux/hidraw.h>
-#include <linux/uhid.h>
-
-#define SHOW_UHID_DEBUG 0
-
-#define min(a, b) \
-	({ __typeof__(a) _a = (a); \
-	__typeof__(b) _b = (b); \
-	_a < _b ? _a : _b; })
-
-static unsigned char rdesc[] = {
-	0x06, 0x00, 0xff,	/* Usage Page (Vendor Defined Page 1) */
-	0x09, 0x21,		/* Usage (Vendor Usage 0x21) */
-	0xa1, 0x01,		/* COLLECTION (Application) */
-	0x09, 0x01,			/* Usage (Vendor Usage 0x01) */
-	0xa1, 0x00,			/* COLLECTION (Physical) */
-	0x85, 0x02,				/* REPORT_ID (2) */
-	0x19, 0x01,				/* USAGE_MINIMUM (1) */
-	0x29, 0x08,				/* USAGE_MAXIMUM (3) */
-	0x15, 0x00,				/* LOGICAL_MINIMUM (0) */
-	0x25, 0xff,				/* LOGICAL_MAXIMUM (255) */
-	0x95, 0x08,				/* REPORT_COUNT (8) */
-	0x75, 0x08,				/* REPORT_SIZE (8) */
-	0x81, 0x02,				/* INPUT (Data,Var,Abs) */
-	0xc0,				/* END_COLLECTION */
-	0x09, 0x01,			/* Usage (Vendor Usage 0x01) */
-	0xa1, 0x00,			/* COLLECTION (Physical) */
-	0x85, 0x01,				/* REPORT_ID (1) */
-	0x06, 0x00, 0xff,			/* Usage Page (Vendor Defined Page 1) */
-	0x19, 0x01,				/* USAGE_MINIMUM (1) */
-	0x29, 0x03,				/* USAGE_MAXIMUM (3) */
-	0x15, 0x00,				/* LOGICAL_MINIMUM (0) */
-	0x25, 0x01,				/* LOGICAL_MAXIMUM (1) */
-	0x95, 0x03,				/* REPORT_COUNT (3) */
-	0x75, 0x01,				/* REPORT_SIZE (1) */
-	0x81, 0x02,				/* INPUT (Data,Var,Abs) */
-	0x95, 0x01,				/* REPORT_COUNT (1) */
-	0x75, 0x05,				/* REPORT_SIZE (5) */
-	0x81, 0x01,				/* INPUT (Cnst,Var,Abs) */
-	0x05, 0x01,				/* USAGE_PAGE (Generic Desktop) */
-	0x09, 0x30,				/* USAGE (X) */
-	0x09, 0x31,				/* USAGE (Y) */
-	0x15, 0x81,				/* LOGICAL_MINIMUM (-127) */
-	0x25, 0x7f,				/* LOGICAL_MAXIMUM (127) */
-	0x75, 0x10,				/* REPORT_SIZE (16) */
-	0x95, 0x02,				/* REPORT_COUNT (2) */
-	0x81, 0x06,				/* INPUT (Data,Var,Rel) */
-
-	0x06, 0x00, 0xff,			/* Usage Page (Vendor Defined Page 1) */
-	0x19, 0x01,				/* USAGE_MINIMUM (1) */
-	0x29, 0x03,				/* USAGE_MAXIMUM (3) */
-	0x15, 0x00,				/* LOGICAL_MINIMUM (0) */
-	0x25, 0x01,				/* LOGICAL_MAXIMUM (1) */
-	0x95, 0x03,				/* REPORT_COUNT (3) */
-	0x75, 0x01,				/* REPORT_SIZE (1) */
-	0x91, 0x02,				/* Output (Data,Var,Abs) */
-	0x95, 0x01,				/* REPORT_COUNT (1) */
-	0x75, 0x05,				/* REPORT_SIZE (5) */
-	0x91, 0x01,				/* Output (Cnst,Var,Abs) */
-
-	0x06, 0x00, 0xff,			/* Usage Page (Vendor Defined Page 1) */
-	0x19, 0x06,				/* USAGE_MINIMUM (6) */
-	0x29, 0x08,				/* USAGE_MAXIMUM (8) */
-	0x15, 0x00,				/* LOGICAL_MINIMUM (0) */
-	0x25, 0x01,				/* LOGICAL_MAXIMUM (1) */
-	0x95, 0x03,				/* REPORT_COUNT (3) */
-	0x75, 0x01,				/* REPORT_SIZE (1) */
-	0xb1, 0x02,				/* Feature (Data,Var,Abs) */
-	0x95, 0x01,				/* REPORT_COUNT (1) */
-	0x75, 0x05,				/* REPORT_SIZE (5) */
-	0x91, 0x01,				/* Output (Cnst,Var,Abs) */
-
-	0xc0,				/* END_COLLECTION */
-	0xc0,			/* END_COLLECTION */
-};
-
-static __u8 feature_data[] = { 1, 2 };
 
 struct attach_prog_args {
 	int prog_fd;
@@ -105,354 +20,6 @@ struct hid_hw_request_syscall_args {
 	__u8 request_type;
 };
 
-#define ASSERT_OK(data) ASSERT_FALSE(data)
-#define ASSERT_OK_PTR(ptr) ASSERT_NE(NULL, ptr)
-
-#define UHID_LOG(fmt, ...) do { \
-	if (SHOW_UHID_DEBUG) \
-		TH_LOG(fmt, ##__VA_ARGS__); \
-} while (0)
-
-static pthread_mutex_t uhid_started_mtx = PTHREAD_MUTEX_INITIALIZER;
-static pthread_cond_t uhid_started = PTHREAD_COND_INITIALIZER;
-
-static pthread_mutex_t uhid_output_mtx = PTHREAD_MUTEX_INITIALIZER;
-static pthread_cond_t uhid_output_cond = PTHREAD_COND_INITIALIZER;
-static unsigned char output_report[10];
-
-/* no need to protect uhid_stopped, only one thread accesses it */
-static bool uhid_stopped;
-
-static int uhid_write(struct __test_metadata *_metadata, int fd, const struct uhid_event *ev)
-{
-	ssize_t ret;
-
-	ret = write(fd, ev, sizeof(*ev));
-	if (ret < 0) {
-		TH_LOG("Cannot write to uhid: %m");
-		return -errno;
-	} else if (ret != sizeof(*ev)) {
-		TH_LOG("Wrong size written to uhid: %zd != %zu",
-			ret, sizeof(ev));
-		return -EFAULT;
-	} else {
-		return 0;
-	}
-}
-
-static int uhid_create(struct __test_metadata *_metadata, int fd, int rand_nb)
-{
-	struct uhid_event ev;
-	char buf[25];
-
-	sprintf(buf, "test-uhid-device-%d", rand_nb);
-
-	memset(&ev, 0, sizeof(ev));
-	ev.type = UHID_CREATE;
-	strcpy((char *)ev.u.create.name, buf);
-	ev.u.create.rd_data = rdesc;
-	ev.u.create.rd_size = sizeof(rdesc);
-	ev.u.create.bus = BUS_USB;
-	ev.u.create.vendor = 0x0001;
-	ev.u.create.product = 0x0a37;
-	ev.u.create.version = 0;
-	ev.u.create.country = 0;
-
-	sprintf(buf, "%d", rand_nb);
-	strcpy((char *)ev.u.create.phys, buf);
-
-	return uhid_write(_metadata, fd, &ev);
-}
-
-static void uhid_destroy(struct __test_metadata *_metadata, int fd)
-{
-	struct uhid_event ev;
-
-	memset(&ev, 0, sizeof(ev));
-	ev.type = UHID_DESTROY;
-
-	uhid_write(_metadata, fd, &ev);
-}
-
-static int uhid_event(struct __test_metadata *_metadata, int fd)
-{
-	struct uhid_event ev, answer;
-	ssize_t ret;
-
-	memset(&ev, 0, sizeof(ev));
-	ret = read(fd, &ev, sizeof(ev));
-	if (ret == 0) {
-		UHID_LOG("Read HUP on uhid-cdev");
-		return -EFAULT;
-	} else if (ret < 0) {
-		UHID_LOG("Cannot read uhid-cdev: %m");
-		return -errno;
-	} else if (ret != sizeof(ev)) {
-		UHID_LOG("Invalid size read from uhid-dev: %zd != %zu",
-			ret, sizeof(ev));
-		return -EFAULT;
-	}
-
-	switch (ev.type) {
-	case UHID_START:
-		pthread_mutex_lock(&uhid_started_mtx);
-		pthread_cond_signal(&uhid_started);
-		pthread_mutex_unlock(&uhid_started_mtx);
-
-		UHID_LOG("UHID_START from uhid-dev");
-		break;
-	case UHID_STOP:
-		uhid_stopped = true;
-
-		UHID_LOG("UHID_STOP from uhid-dev");
-		break;
-	case UHID_OPEN:
-		UHID_LOG("UHID_OPEN from uhid-dev");
-		break;
-	case UHID_CLOSE:
-		UHID_LOG("UHID_CLOSE from uhid-dev");
-		break;
-	case UHID_OUTPUT:
-		UHID_LOG("UHID_OUTPUT from uhid-dev");
-
-		pthread_mutex_lock(&uhid_output_mtx);
-		memcpy(output_report,
-		       ev.u.output.data,
-		       min(ev.u.output.size, sizeof(output_report)));
-		pthread_cond_signal(&uhid_output_cond);
-		pthread_mutex_unlock(&uhid_output_mtx);
-		break;
-	case UHID_GET_REPORT:
-		UHID_LOG("UHID_GET_REPORT from uhid-dev");
-
-		answer.type = UHID_GET_REPORT_REPLY;
-		answer.u.get_report_reply.id = ev.u.get_report.id;
-		answer.u.get_report_reply.err = ev.u.get_report.rnum == 1 ? 0 : -EIO;
-		answer.u.get_report_reply.size = sizeof(feature_data);
-		memcpy(answer.u.get_report_reply.data, feature_data, sizeof(feature_data));
-
-		uhid_write(_metadata, fd, &answer);
-
-		break;
-	case UHID_SET_REPORT:
-		UHID_LOG("UHID_SET_REPORT from uhid-dev");
-		break;
-	default:
-		TH_LOG("Invalid event from uhid-dev: %u", ev.type);
-	}
-
-	return 0;
-}
-
-struct uhid_thread_args {
-	int fd;
-	struct __test_metadata *_metadata;
-};
-static void *uhid_read_events_thread(void *arg)
-{
-	struct uhid_thread_args *args = (struct uhid_thread_args *)arg;
-	struct __test_metadata *_metadata = args->_metadata;
-	struct pollfd pfds[1];
-	int fd = args->fd;
-	int ret = 0;
-
-	pfds[0].fd = fd;
-	pfds[0].events = POLLIN;
-
-	uhid_stopped = false;
-
-	while (!uhid_stopped) {
-		ret = poll(pfds, 1, 100);
-		if (ret < 0) {
-			TH_LOG("Cannot poll for fds: %m");
-			break;
-		}
-		if (pfds[0].revents & POLLIN) {
-			ret = uhid_event(_metadata, fd);
-			if (ret)
-				break;
-		}
-	}
-
-	return (void *)(long)ret;
-}
-
-static int uhid_start_listener(struct __test_metadata *_metadata, pthread_t *tid, int uhid_fd)
-{
-	struct uhid_thread_args args = {
-		.fd = uhid_fd,
-		._metadata = _metadata,
-	};
-	int err;
-
-	pthread_mutex_lock(&uhid_started_mtx);
-	err = pthread_create(tid, NULL, uhid_read_events_thread, (void *)&args);
-	ASSERT_EQ(0, err) {
-		TH_LOG("Could not start the uhid thread: %d", err);
-		pthread_mutex_unlock(&uhid_started_mtx);
-		close(uhid_fd);
-		return -EIO;
-	}
-	pthread_cond_wait(&uhid_started, &uhid_started_mtx);
-	pthread_mutex_unlock(&uhid_started_mtx);
-
-	return 0;
-}
-
-static int uhid_send_event(struct __test_metadata *_metadata, int fd, __u8 *buf, size_t size)
-{
-	struct uhid_event ev;
-
-	if (size > sizeof(ev.u.input.data))
-		return -E2BIG;
-
-	memset(&ev, 0, sizeof(ev));
-	ev.type = UHID_INPUT2;
-	ev.u.input2.size = size;
-
-	memcpy(ev.u.input2.data, buf, size);
-
-	return uhid_write(_metadata, fd, &ev);
-}
-
-static int setup_uhid(struct __test_metadata *_metadata, int rand_nb)
-{
-	int fd;
-	const char *path = "/dev/uhid";
-	int ret;
-
-	fd = open(path, O_RDWR | O_CLOEXEC);
-	ASSERT_GE(fd, 0) TH_LOG("open uhid-cdev failed; %d", fd);
-
-	ret = uhid_create(_metadata, fd, rand_nb);
-	ASSERT_EQ(0, ret) {
-		TH_LOG("create uhid device failed: %d", ret);
-		close(fd);
-	}
-
-	return fd;
-}
-
-static bool match_sysfs_device(int dev_id, const char *workdir, struct dirent *dir)
-{
-	const char *target = "0003:0001:0A37.*";
-	char phys[512];
-	char uevent[1024];
-	char temp[512];
-	int fd, nread;
-	bool found = false;
-
-	if (fnmatch(target, dir->d_name, 0))
-		return false;
-
-	/* we found the correct VID/PID, now check for phys */
-	sprintf(uevent, "%s/%s/uevent", workdir, dir->d_name);
-
-	fd = open(uevent, O_RDONLY | O_NONBLOCK);
-	if (fd < 0)
-		return false;
-
-	sprintf(phys, "PHYS=%d", dev_id);
-
-	nread = read(fd, temp, ARRAY_SIZE(temp));
-	if (nread > 0 && (strstr(temp, phys)) != NULL)
-		found = true;
-
-	close(fd);
-
-	return found;
-}
-
-static int get_hid_id(int dev_id)
-{
-	const char *workdir = "/sys/devices/virtual/misc/uhid";
-	const char *str_id;
-	DIR *d;
-	struct dirent *dir;
-	int found = -1, attempts = 3;
-
-	/* it would be nice to be able to use nftw, but the no_alu32 target doesn't support it */
-
-	while (found < 0 && attempts > 0) {
-		attempts--;
-		d = opendir(workdir);
-		if (d) {
-			while ((dir = readdir(d)) != NULL) {
-				if (!match_sysfs_device(dev_id, workdir, dir))
-					continue;
-
-				str_id = dir->d_name + sizeof("0003:0001:0A37.");
-				found = (int)strtol(str_id, NULL, 16);
-
-				break;
-			}
-			closedir(d);
-		}
-		if (found < 0)
-			usleep(100000);
-	}
-
-	return found;
-}
-
-static int get_hidraw(int dev_id)
-{
-	const char *workdir = "/sys/devices/virtual/misc/uhid";
-	char sysfs[1024];
-	DIR *d, *subd;
-	struct dirent *dir, *subdir;
-	int i, found = -1;
-
-	/* retry 5 times in case the system is loaded */
-	for (i = 5; i > 0; i--) {
-		usleep(10);
-		d = opendir(workdir);
-
-		if (!d)
-			continue;
-
-		while ((dir = readdir(d)) != NULL) {
-			if (!match_sysfs_device(dev_id, workdir, dir))
-				continue;
-
-			sprintf(sysfs, "%s/%s/hidraw", workdir, dir->d_name);
-
-			subd = opendir(sysfs);
-			if (!subd)
-				continue;
-
-			while ((subdir = readdir(subd)) != NULL) {
-				if (fnmatch("hidraw*", subdir->d_name, 0))
-					continue;
-
-				found = atoi(subdir->d_name + strlen("hidraw"));
-			}
-
-			closedir(subd);
-
-			if (found > 0)
-				break;
-		}
-		closedir(d);
-	}
-
-	return found;
-}
-
-static int open_hidraw(int dev_id)
-{
-	int hidraw_number;
-	char hidraw_path[64] = { 0 };
-
-	hidraw_number = get_hidraw(dev_id);
-	if (hidraw_number < 0)
-		return hidraw_number;
-
-	/* open hidraw node to check the other side of the pipe */
-	sprintf(hidraw_path, "/dev/hidraw%d", hidraw_number);
-	return open(hidraw_path, O_RDWR | O_NONBLOCK);
-}
-
 FIXTURE(hid_bpf) {
 	int dev_id;
 	int uhid_fd;
diff --git a/tools/testing/selftests/hid/hid_common.h b/tools/testing/selftests/hid/hid_common.h
new file mode 100644
index 000000000000..f151f151a1ed
--- /dev/null
+++ b/tools/testing/selftests/hid/hid_common.h
@@ -0,0 +1,436 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (c) 2022-2024 Red Hat */
+
+#include "../kselftest_harness.h"
+
+#include <fcntl.h>
+#include <fnmatch.h>
+#include <dirent.h>
+#include <poll.h>
+#include <pthread.h>
+#include <stdbool.h>
+#include <linux/hidraw.h>
+#include <linux/uhid.h>
+
+#define SHOW_UHID_DEBUG 0
+
+#define min(a, b) \
+	({ __typeof__(a) _a = (a); \
+	__typeof__(b) _b = (b); \
+	_a < _b ? _a : _b; })
+
+static unsigned char rdesc[] = {
+	0x06, 0x00, 0xff,	/* Usage Page (Vendor Defined Page 1) */
+	0x09, 0x21,		/* Usage (Vendor Usage 0x21) */
+	0xa1, 0x01,		/* COLLECTION (Application) */
+	0x09, 0x01,			/* Usage (Vendor Usage 0x01) */
+	0xa1, 0x00,			/* COLLECTION (Physical) */
+	0x85, 0x02,				/* REPORT_ID (2) */
+	0x19, 0x01,				/* USAGE_MINIMUM (1) */
+	0x29, 0x08,				/* USAGE_MAXIMUM (3) */
+	0x15, 0x00,				/* LOGICAL_MINIMUM (0) */
+	0x25, 0xff,				/* LOGICAL_MAXIMUM (255) */
+	0x95, 0x08,				/* REPORT_COUNT (8) */
+	0x75, 0x08,				/* REPORT_SIZE (8) */
+	0x81, 0x02,				/* INPUT (Data,Var,Abs) */
+	0xc0,				/* END_COLLECTION */
+	0x09, 0x01,			/* Usage (Vendor Usage 0x01) */
+	0xa1, 0x00,			/* COLLECTION (Physical) */
+	0x85, 0x01,				/* REPORT_ID (1) */
+	0x06, 0x00, 0xff,			/* Usage Page (Vendor Defined Page 1) */
+	0x19, 0x01,				/* USAGE_MINIMUM (1) */
+	0x29, 0x03,				/* USAGE_MAXIMUM (3) */
+	0x15, 0x00,				/* LOGICAL_MINIMUM (0) */
+	0x25, 0x01,				/* LOGICAL_MAXIMUM (1) */
+	0x95, 0x03,				/* REPORT_COUNT (3) */
+	0x75, 0x01,				/* REPORT_SIZE (1) */
+	0x81, 0x02,				/* INPUT (Data,Var,Abs) */
+	0x95, 0x01,				/* REPORT_COUNT (1) */
+	0x75, 0x05,				/* REPORT_SIZE (5) */
+	0x81, 0x01,				/* INPUT (Cnst,Var,Abs) */
+	0x05, 0x01,				/* USAGE_PAGE (Generic Desktop) */
+	0x09, 0x30,				/* USAGE (X) */
+	0x09, 0x31,				/* USAGE (Y) */
+	0x15, 0x81,				/* LOGICAL_MINIMUM (-127) */
+	0x25, 0x7f,				/* LOGICAL_MAXIMUM (127) */
+	0x75, 0x10,				/* REPORT_SIZE (16) */
+	0x95, 0x02,				/* REPORT_COUNT (2) */
+	0x81, 0x06,				/* INPUT (Data,Var,Rel) */
+
+	0x06, 0x00, 0xff,			/* Usage Page (Vendor Defined Page 1) */
+	0x19, 0x01,				/* USAGE_MINIMUM (1) */
+	0x29, 0x03,				/* USAGE_MAXIMUM (3) */
+	0x15, 0x00,				/* LOGICAL_MINIMUM (0) */
+	0x25, 0x01,				/* LOGICAL_MAXIMUM (1) */
+	0x95, 0x03,				/* REPORT_COUNT (3) */
+	0x75, 0x01,				/* REPORT_SIZE (1) */
+	0x91, 0x02,				/* Output (Data,Var,Abs) */
+	0x95, 0x01,				/* REPORT_COUNT (1) */
+	0x75, 0x05,				/* REPORT_SIZE (5) */
+	0x91, 0x01,				/* Output (Cnst,Var,Abs) */
+
+	0x06, 0x00, 0xff,			/* Usage Page (Vendor Defined Page 1) */
+	0x19, 0x06,				/* USAGE_MINIMUM (6) */
+	0x29, 0x08,				/* USAGE_MAXIMUM (8) */
+	0x15, 0x00,				/* LOGICAL_MINIMUM (0) */
+	0x25, 0x01,				/* LOGICAL_MAXIMUM (1) */
+	0x95, 0x03,				/* REPORT_COUNT (3) */
+	0x75, 0x01,				/* REPORT_SIZE (1) */
+	0xb1, 0x02,				/* Feature (Data,Var,Abs) */
+	0x95, 0x01,				/* REPORT_COUNT (1) */
+	0x75, 0x05,				/* REPORT_SIZE (5) */
+	0x91, 0x01,				/* Output (Cnst,Var,Abs) */
+
+	0xc0,				/* END_COLLECTION */
+	0xc0,			/* END_COLLECTION */
+};
+
+static __u8 feature_data[] = { 1, 2 };
+
+#define ASSERT_OK(data) ASSERT_FALSE(data)
+#define ASSERT_OK_PTR(ptr) ASSERT_NE(NULL, ptr)
+
+#define UHID_LOG(fmt, ...) do { \
+	if (SHOW_UHID_DEBUG) \
+		TH_LOG(fmt, ##__VA_ARGS__); \
+} while (0)
+
+static pthread_mutex_t uhid_started_mtx = PTHREAD_MUTEX_INITIALIZER;
+static pthread_cond_t uhid_started = PTHREAD_COND_INITIALIZER;
+
+static pthread_mutex_t uhid_output_mtx = PTHREAD_MUTEX_INITIALIZER;
+static pthread_cond_t uhid_output_cond = PTHREAD_COND_INITIALIZER;
+static unsigned char output_report[10];
+
+/* no need to protect uhid_stopped, only one thread accesses it */
+static bool uhid_stopped;
+
+static int uhid_write(struct __test_metadata *_metadata, int fd, const struct uhid_event *ev)
+{
+	ssize_t ret;
+
+	ret = write(fd, ev, sizeof(*ev));
+	if (ret < 0) {
+		TH_LOG("Cannot write to uhid: %m");
+		return -errno;
+	} else if (ret != sizeof(*ev)) {
+		TH_LOG("Wrong size written to uhid: %zd != %zu",
+			ret, sizeof(ev));
+		return -EFAULT;
+	} else {
+		return 0;
+	}
+}
+
+static int uhid_create(struct __test_metadata *_metadata, int fd, int rand_nb)
+{
+	struct uhid_event ev;
+	char buf[25];
+
+	sprintf(buf, "test-uhid-device-%d", rand_nb);
+
+	memset(&ev, 0, sizeof(ev));
+	ev.type = UHID_CREATE;
+	strcpy((char *)ev.u.create.name, buf);
+	ev.u.create.rd_data = rdesc;
+	ev.u.create.rd_size = sizeof(rdesc);
+	ev.u.create.bus = BUS_USB;
+	ev.u.create.vendor = 0x0001;
+	ev.u.create.product = 0x0a37;
+	ev.u.create.version = 0;
+	ev.u.create.country = 0;
+
+	sprintf(buf, "%d", rand_nb);
+	strcpy((char *)ev.u.create.phys, buf);
+
+	return uhid_write(_metadata, fd, &ev);
+}
+
+static void uhid_destroy(struct __test_metadata *_metadata, int fd)
+{
+	struct uhid_event ev;
+
+	memset(&ev, 0, sizeof(ev));
+	ev.type = UHID_DESTROY;
+
+	uhid_write(_metadata, fd, &ev);
+}
+
+static int uhid_event(struct __test_metadata *_metadata, int fd)
+{
+	struct uhid_event ev, answer;
+	ssize_t ret;
+
+	memset(&ev, 0, sizeof(ev));
+	ret = read(fd, &ev, sizeof(ev));
+	if (ret == 0) {
+		UHID_LOG("Read HUP on uhid-cdev");
+		return -EFAULT;
+	} else if (ret < 0) {
+		UHID_LOG("Cannot read uhid-cdev: %m");
+		return -errno;
+	} else if (ret != sizeof(ev)) {
+		UHID_LOG("Invalid size read from uhid-dev: %zd != %zu",
+			ret, sizeof(ev));
+		return -EFAULT;
+	}
+
+	switch (ev.type) {
+	case UHID_START:
+		pthread_mutex_lock(&uhid_started_mtx);
+		pthread_cond_signal(&uhid_started);
+		pthread_mutex_unlock(&uhid_started_mtx);
+
+		UHID_LOG("UHID_START from uhid-dev");
+		break;
+	case UHID_STOP:
+		uhid_stopped = true;
+
+		UHID_LOG("UHID_STOP from uhid-dev");
+		break;
+	case UHID_OPEN:
+		UHID_LOG("UHID_OPEN from uhid-dev");
+		break;
+	case UHID_CLOSE:
+		UHID_LOG("UHID_CLOSE from uhid-dev");
+		break;
+	case UHID_OUTPUT:
+		UHID_LOG("UHID_OUTPUT from uhid-dev");
+
+		pthread_mutex_lock(&uhid_output_mtx);
+		memcpy(output_report,
+		       ev.u.output.data,
+		       min(ev.u.output.size, sizeof(output_report)));
+		pthread_cond_signal(&uhid_output_cond);
+		pthread_mutex_unlock(&uhid_output_mtx);
+		break;
+	case UHID_GET_REPORT:
+		UHID_LOG("UHID_GET_REPORT from uhid-dev");
+
+		answer.type = UHID_GET_REPORT_REPLY;
+		answer.u.get_report_reply.id = ev.u.get_report.id;
+		answer.u.get_report_reply.err = ev.u.get_report.rnum == 1 ? 0 : -EIO;
+		answer.u.get_report_reply.size = sizeof(feature_data);
+		memcpy(answer.u.get_report_reply.data, feature_data, sizeof(feature_data));
+
+		uhid_write(_metadata, fd, &answer);
+
+		break;
+	case UHID_SET_REPORT:
+		UHID_LOG("UHID_SET_REPORT from uhid-dev");
+		break;
+	default:
+		TH_LOG("Invalid event from uhid-dev: %u", ev.type);
+	}
+
+	return 0;
+}
+
+struct uhid_thread_args {
+	int fd;
+	struct __test_metadata *_metadata;
+};
+static void *uhid_read_events_thread(void *arg)
+{
+	struct uhid_thread_args *args = (struct uhid_thread_args *)arg;
+	struct __test_metadata *_metadata = args->_metadata;
+	struct pollfd pfds[1];
+	int fd = args->fd;
+	int ret = 0;
+
+	pfds[0].fd = fd;
+	pfds[0].events = POLLIN;
+
+	uhid_stopped = false;
+
+	while (!uhid_stopped) {
+		ret = poll(pfds, 1, 100);
+		if (ret < 0) {
+			TH_LOG("Cannot poll for fds: %m");
+			break;
+		}
+		if (pfds[0].revents & POLLIN) {
+			ret = uhid_event(_metadata, fd);
+			if (ret)
+				break;
+		}
+	}
+
+	return (void *)(long)ret;
+}
+
+static int uhid_start_listener(struct __test_metadata *_metadata, pthread_t *tid, int uhid_fd)
+{
+	struct uhid_thread_args args = {
+		.fd = uhid_fd,
+		._metadata = _metadata,
+	};
+	int err;
+
+	pthread_mutex_lock(&uhid_started_mtx);
+	err = pthread_create(tid, NULL, uhid_read_events_thread, (void *)&args);
+	ASSERT_EQ(0, err) {
+		TH_LOG("Could not start the uhid thread: %d", err);
+		pthread_mutex_unlock(&uhid_started_mtx);
+		close(uhid_fd);
+		return -EIO;
+	}
+	pthread_cond_wait(&uhid_started, &uhid_started_mtx);
+	pthread_mutex_unlock(&uhid_started_mtx);
+
+	return 0;
+}
+
+static int uhid_send_event(struct __test_metadata *_metadata, int fd, __u8 *buf, size_t size)
+{
+	struct uhid_event ev;
+
+	if (size > sizeof(ev.u.input.data))
+		return -E2BIG;
+
+	memset(&ev, 0, sizeof(ev));
+	ev.type = UHID_INPUT2;
+	ev.u.input2.size = size;
+
+	memcpy(ev.u.input2.data, buf, size);
+
+	return uhid_write(_metadata, fd, &ev);
+}
+
+static int setup_uhid(struct __test_metadata *_metadata, int rand_nb)
+{
+	int fd;
+	const char *path = "/dev/uhid";
+	int ret;
+
+	fd = open(path, O_RDWR | O_CLOEXEC);
+	ASSERT_GE(fd, 0) TH_LOG("open uhid-cdev failed; %d", fd);
+
+	ret = uhid_create(_metadata, fd, rand_nb);
+	ASSERT_EQ(0, ret) {
+		TH_LOG("create uhid device failed: %d", ret);
+		close(fd);
+	}
+
+	return fd;
+}
+
+static bool match_sysfs_device(int dev_id, const char *workdir, struct dirent *dir)
+{
+	const char *target = "0003:0001:0A37.*";
+	char phys[512];
+	char uevent[1024];
+	char temp[512];
+	int fd, nread;
+	bool found = false;
+
+	if (fnmatch(target, dir->d_name, 0))
+		return false;
+
+	/* we found the correct VID/PID, now check for phys */
+	sprintf(uevent, "%s/%s/uevent", workdir, dir->d_name);
+
+	fd = open(uevent, O_RDONLY | O_NONBLOCK);
+	if (fd < 0)
+		return false;
+
+	sprintf(phys, "PHYS=%d", dev_id);
+
+	nread = read(fd, temp, ARRAY_SIZE(temp));
+	if (nread > 0 && (strstr(temp, phys)) != NULL)
+		found = true;
+
+	close(fd);
+
+	return found;
+}
+
+static int get_hid_id(int dev_id)
+{
+	const char *workdir = "/sys/devices/virtual/misc/uhid";
+	const char *str_id;
+	DIR *d;
+	struct dirent *dir;
+	int found = -1, attempts = 3;
+
+	/* it would be nice to be able to use nftw, but the no_alu32 target doesn't support it */
+
+	while (found < 0 && attempts > 0) {
+		attempts--;
+		d = opendir(workdir);
+		if (d) {
+			while ((dir = readdir(d)) != NULL) {
+				if (!match_sysfs_device(dev_id, workdir, dir))
+					continue;
+
+				str_id = dir->d_name + sizeof("0003:0001:0A37.");
+				found = (int)strtol(str_id, NULL, 16);
+
+				break;
+			}
+			closedir(d);
+		}
+		if (found < 0)
+			usleep(100000);
+	}
+
+	return found;
+}
+
+static int get_hidraw(int dev_id)
+{
+	const char *workdir = "/sys/devices/virtual/misc/uhid";
+	char sysfs[1024];
+	DIR *d, *subd;
+	struct dirent *dir, *subdir;
+	int i, found = -1;
+
+	/* retry 5 times in case the system is loaded */
+	for (i = 5; i > 0; i--) {
+		usleep(10);
+		d = opendir(workdir);
+
+		if (!d)
+			continue;
+
+		while ((dir = readdir(d)) != NULL) {
+			if (!match_sysfs_device(dev_id, workdir, dir))
+				continue;
+
+			sprintf(sysfs, "%s/%s/hidraw", workdir, dir->d_name);
+
+			subd = opendir(sysfs);
+			if (!subd)
+				continue;
+
+			while ((subdir = readdir(subd)) != NULL) {
+				if (fnmatch("hidraw*", subdir->d_name, 0))
+					continue;
+
+				found = atoi(subdir->d_name + strlen("hidraw"));
+			}
+
+			closedir(subd);
+
+			if (found > 0)
+				break;
+		}
+		closedir(d);
+	}
+
+	return found;
+}
+
+static int open_hidraw(int dev_id)
+{
+	int hidraw_number;
+	char hidraw_path[64] = { 0 };
+
+	hidraw_number = get_hidraw(dev_id);
+	if (hidraw_number < 0)
+		return hidraw_number;
+
+	/* open hidraw node to check the other side of the pipe */
+	sprintf(hidraw_path, "/dev/hidraw%d", hidraw_number);
+	return open(hidraw_path, O_RDWR | O_NONBLOCK);
+}

-- 
2.46.0


^ permalink raw reply related

* [PATCH v5 1/4] HID: hidraw: add HIDIOCREVOKE ioctl
From: bentiss @ 2024-08-27  8:19 UTC (permalink / raw)
  To: Jiri Kosina, Shuah Khan, Peter Hutterer
  Cc: linux-input, linux-kernel, linux-kselftest, Benjamin Tissoires
In-Reply-To: <20240827-hidraw-revoke-v5-0-d004a7451aea@kernel.org>

From: Peter Hutterer <peter.hutterer@who-t.net>

There is a need for userspace applications to open HID devices directly.
Use-cases include configuration of gaming mice or direct access to
joystick devices. The latter is currently handled by the uaccess tag in
systemd, other devices include more custom/local configurations or just
sudo.

A better approach is what we already have for evdev devices: give the
application a file descriptor and revoke it when it may no longer access
that device.

This patch is the hidraw equivalent to the EVIOCREVOKE ioctl, see
commit c7dc65737c9a ("Input: evdev - add EVIOCREVOKE ioctl") for full
details.

An MR for systemd-logind has been filed here:
https://github.com/systemd/systemd/pull/33970

Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>

---

First version of the patch:
https://patchwork.kernel.org/project/linux-input/patch/YmEAPZKDisM2HAsG@quokka/

Changes to v1:
- add the hidraw_is_revoked and hidraw_open_errno weak functions as
  suggested by Benjamin

Changes to v2:
- use __bpf_hook_start/end to silence compiler warnings  (see kernel
  test bot)

Changes to v3 (bentiss):
- drop the BPF related changes, the API is not clear ATM, and it's wrong
  in v2, as ERROR_INJECTION should not be used for normal fmodret.
- drop hidraw_open_errno() new function. The API seems interesting, but
  nothing is really ready, so let's not add a useless API for nothing.

Changes to v4:
none
---
 drivers/hid/hidraw.c        | 39 +++++++++++++++++++++++++++++++++++----
 include/linux/hidraw.h      |  1 +
 include/uapi/linux/hidraw.h |  1 +
 3 files changed, 37 insertions(+), 4 deletions(-)

diff --git a/drivers/hid/hidraw.c b/drivers/hid/hidraw.c
index 716294e40e8a..c887f48756f4 100644
--- a/drivers/hid/hidraw.c
+++ b/drivers/hid/hidraw.c
@@ -38,12 +38,20 @@ static const struct class hidraw_class = {
 static struct hidraw *hidraw_table[HIDRAW_MAX_DEVICES];
 static DECLARE_RWSEM(minors_rwsem);
 
+static inline bool hidraw_is_revoked(struct hidraw_list *list)
+{
+	return list->revoked;
+}
+
 static ssize_t hidraw_read(struct file *file, char __user *buffer, size_t count, loff_t *ppos)
 {
 	struct hidraw_list *list = file->private_data;
 	int ret = 0, len;
 	DECLARE_WAITQUEUE(wait, current);
 
+	if (hidraw_is_revoked(list))
+		return -ENODEV;
+
 	mutex_lock(&list->read_mutex);
 
 	while (ret == 0) {
@@ -161,9 +169,13 @@ static ssize_t hidraw_send_report(struct file *file, const char __user *buffer,
 
 static ssize_t hidraw_write(struct file *file, const char __user *buffer, size_t count, loff_t *ppos)
 {
+	struct hidraw_list *list = file->private_data;
 	ssize_t ret;
 	down_read(&minors_rwsem);
-	ret = hidraw_send_report(file, buffer, count, HID_OUTPUT_REPORT);
+	if (hidraw_is_revoked(list))
+		ret = -ENODEV;
+	else
+		ret = hidraw_send_report(file, buffer, count, HID_OUTPUT_REPORT);
 	up_read(&minors_rwsem);
 	return ret;
 }
@@ -256,7 +268,7 @@ static __poll_t hidraw_poll(struct file *file, poll_table *wait)
 	poll_wait(file, &list->hidraw->wait, wait);
 	if (list->head != list->tail)
 		mask |= EPOLLIN | EPOLLRDNORM;
-	if (!list->hidraw->exist)
+	if (!list->hidraw->exist || hidraw_is_revoked(list))
 		mask |= EPOLLERR | EPOLLHUP;
 	return mask;
 }
@@ -320,6 +332,9 @@ static int hidraw_fasync(int fd, struct file *file, int on)
 {
 	struct hidraw_list *list = file->private_data;
 
+	if (hidraw_is_revoked(list))
+		return -ENODEV;
+
 	return fasync_helper(fd, file, on, &list->fasync);
 }
 
@@ -372,6 +387,13 @@ static int hidraw_release(struct inode * inode, struct file * file)
 	return 0;
 }
 
+static int hidraw_revoke(struct hidraw_list *list)
+{
+	list->revoked = true;
+
+	return 0;
+}
+
 static long hidraw_ioctl(struct file *file, unsigned int cmd,
 							unsigned long arg)
 {
@@ -379,11 +401,12 @@ static long hidraw_ioctl(struct file *file, unsigned int cmd,
 	unsigned int minor = iminor(inode);
 	long ret = 0;
 	struct hidraw *dev;
+	struct hidraw_list *list = file->private_data;
 	void __user *user_arg = (void __user*) arg;
 
 	down_read(&minors_rwsem);
 	dev = hidraw_table[minor];
-	if (!dev || !dev->exist) {
+	if (!dev || !dev->exist || hidraw_is_revoked(list)) {
 		ret = -ENODEV;
 		goto out;
 	}
@@ -421,6 +444,14 @@ static long hidraw_ioctl(struct file *file, unsigned int cmd,
 					ret = -EFAULT;
 				break;
 			}
+		case HIDIOCREVOKE:
+			{
+				if (user_arg)
+					ret = -EINVAL;
+				else
+					ret = hidraw_revoke(list);
+				break;
+			}
 		default:
 			{
 				struct hid_device *hid = dev->hid;
@@ -527,7 +558,7 @@ int hidraw_report_event(struct hid_device *hid, u8 *data, int len)
 	list_for_each_entry(list, &dev->list, node) {
 		int new_head = (list->head + 1) & (HIDRAW_BUFFER_SIZE - 1);
 
-		if (new_head == list->tail)
+		if (hidraw_is_revoked(list) || new_head == list->tail)
 			continue;
 
 		if (!(list->buffer[list->head].value = kmemdup(data, len, GFP_ATOMIC))) {
diff --git a/include/linux/hidraw.h b/include/linux/hidraw.h
index cd67f4ca5599..18fd30a288de 100644
--- a/include/linux/hidraw.h
+++ b/include/linux/hidraw.h
@@ -32,6 +32,7 @@ struct hidraw_list {
 	struct hidraw *hidraw;
 	struct list_head node;
 	struct mutex read_mutex;
+	bool revoked;
 };
 
 #ifdef CONFIG_HIDRAW
diff --git a/include/uapi/linux/hidraw.h b/include/uapi/linux/hidraw.h
index 33ebad81720a..d5ee269864e0 100644
--- a/include/uapi/linux/hidraw.h
+++ b/include/uapi/linux/hidraw.h
@@ -46,6 +46,7 @@ struct hidraw_devinfo {
 /* The first byte of SOUTPUT and GOUTPUT is the report number */
 #define HIDIOCSOUTPUT(len)    _IOC(_IOC_WRITE|_IOC_READ, 'H', 0x0B, len)
 #define HIDIOCGOUTPUT(len)    _IOC(_IOC_WRITE|_IOC_READ, 'H', 0x0C, len)
+#define HIDIOCREVOKE	      _IOW('H', 0x0D, int) /* Revoke device access */
 
 #define HIDRAW_FIRST_MINOR 0
 #define HIDRAW_MAX_DEVICES 64

-- 
2.46.0


^ permalink raw reply related

* [PATCH v5 0/4] HID: hidraw: HIDIOCREVOKE introduction
From: bentiss @ 2024-08-27  8:19 UTC (permalink / raw)
  To: Jiri Kosina, Shuah Khan, Peter Hutterer
  Cc: linux-input, linux-kernel, linux-kselftest, Benjamin Tissoires

The is the v5 of the HIDIOCREVOKE patches.

After a small discussion with Peter, we decided to:
- drop the BPF hooks that are problematic (Linus doesn't want
  "ALLOW_ERROR_INJECTION" to be used as "normal" fmodret bpf hooks)
- punt those BPF hooks later once we get the API right
- I'll be the one sending that new version, given that it's easier for
  me ATM

For testing the patch, and for convenience, I added a new selftest
program that can test this new ioctl. This will also allow us to
integrate the (future) BPF hooks and show how this should be used.

Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
---
Changes in v5:
- check for ENODEV when required in selftests
- create new common header for the HID tests that can be reused in other
  HID selftests
- Link to v4: https://lore.kernel.org/r/20240827-hidraw-revoke-v4-0-88c6795bf867@kernel.org

Link to v3: https://lore.kernel.org/all/20240812052753.GA478917@quokka/

---
Benjamin Tissoires (3):
      selftests/hid: extract the utility part of hid_bpf.c into its own header
      selftests/hid: Add initial hidraw tests skeleton
      selftests/hid: Add HIDIOCREVOKE tests

Peter Hutterer (1):
      HID: hidraw: add HIDIOCREVOKE ioctl

 drivers/hid/hidraw.c                     |  39 ++-
 include/linux/hidraw.h                   |   1 +
 include/uapi/linux/hidraw.h              |   1 +
 tools/testing/selftests/hid/.gitignore   |   1 +
 tools/testing/selftests/hid/Makefile     |   2 +-
 tools/testing/selftests/hid/hid_bpf.c    | 437 +------------------------------
 tools/testing/selftests/hid/hid_common.h | 436 ++++++++++++++++++++++++++++++
 tools/testing/selftests/hid/hidraw.c     | 237 +++++++++++++++++
 8 files changed, 714 insertions(+), 440 deletions(-)
---
base-commit: 6e4436539ae182dc86d57d13849862bcafaa4709
change-id: 20240826-hidraw-revoke-0a02ebb21743

Best regards,
-- 
Benjamin Tissoires <bentiss@kernel.org>


^ permalink raw reply

* Re: [PATCH v4 3/3] selftests/hid: Add HIDIOCREVOKE tests
From: Peter Hutterer @ 2024-08-27  5:16 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: Jiri Kosina, Shuah Khan, linux-input, linux-kernel,
	linux-kselftest
In-Reply-To: <20240827-hidraw-revoke-v4-3-88c6795bf867@kernel.org>

Thanks for picking this up and adding the tests, much appreciated

On Tue, Aug 27, 2024 at 12:47:53AM +0900, Benjamin Tissoires wrote:
> Add 4 tests for the new revoke ioctl, for read/write/ioctl and poll.
> 
> Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
> 
> ---
> 
> new in v4
> ---
>  tools/testing/selftests/hid/hidraw.c | 143 +++++++++++++++++++++++++++++++++++
>  1 file changed, 143 insertions(+)
> 
> diff --git a/tools/testing/selftests/hid/hidraw.c b/tools/testing/selftests/hid/hidraw.c
> index f8c933476dcd..669eada8886b 100644
> --- a/tools/testing/selftests/hid/hidraw.c
> +++ b/tools/testing/selftests/hid/hidraw.c
> @@ -19,6 +19,11 @@
>  	__typeof__(b) _b = (b); \
>  	_a < _b ? _a : _b; })
>  
> +/* for older kernels */
> +#ifndef HIDIOCREVOKE
> +#define HIDIOCREVOKE	      _IOW('H', 0x0D, int) /* Revoke device access */
> +#endif /* HIDIOCREVOKE */
> +
>  static unsigned char rdesc[] = {
>  	0x06, 0x00, 0xff,	/* Usage Page (Vendor Defined Page 1) */
>  	0x09, 0x21,		/* Usage (Vendor Usage 0x21) */
> @@ -516,6 +521,144 @@ TEST_F(hidraw, raw_event)
>  	ASSERT_EQ(buf[1], 42);
>  }
>  
> +/*
> + * After initial opening/checks of hidraw, revoke the hidraw
> + * node and check that we can not read any more data.
> + */
> +TEST_F(hidraw, raw_event_revoked)
> +{
> +	__u8 buf[10] = {0};
> +	int err;
> +
> +	/* inject one event */
> +	buf[0] = 1;
> +	buf[1] = 42;
> +	uhid_send_event(_metadata, self->uhid_fd, buf, 6);
> +
> +	/* read the data from hidraw */
> +	memset(buf, 0, sizeof(buf));
> +	err = read(self->hidraw_fd, buf, sizeof(buf));
> +	ASSERT_EQ(err, 6) TH_LOG("read_hidraw");
> +	ASSERT_EQ(buf[0], 1);
> +	ASSERT_EQ(buf[1], 42);
> +
> +	/* call the revoke ioctl */
> +	err = ioctl(self->hidraw_fd, HIDIOCREVOKE, NULL);
> +	ASSERT_OK(err) TH_LOG("couldn't revoke the hidraw fd");
> +
> +	/* inject one other event */
> +	buf[0] = 1;
> +	buf[1] = 43;
> +	uhid_send_event(_metadata, self->uhid_fd, buf, 6);
> +
> +	/* read the data from hidraw */
> +	memset(buf, 0, sizeof(buf));
> +	err = read(self->hidraw_fd, buf, sizeof(buf));
> +	ASSERT_EQ(err, -1) TH_LOG("read_hidraw");

do you want to check for errno == ENODEV here to avoid false positives?
Shouldn't really happen in this test suite but it costs very little.

Same for the various cases below.

With or without - series:
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>

Cheers,
  Peter

^ permalink raw reply

* [PATCH] HID: multitouch: Add support for lenovo Y9000P Touchpad
From: He Lugang @ 2024-08-27  2:56 UTC (permalink / raw)
  To: jikos, bentiss; +Cc: linux-input, He Lugang

The 2024 Lenovo Y9000P which use GT7868Q chip also needs a fixup.
The information of the chip is as follows:
I2C HID v1.00 Mouse [GXTP5100:00 27C6:01E0]

Signed-off-by: He Lugang <helugang@uniontech.com>
---
 drivers/hid/hid-ids.h        | 1 +
 drivers/hid/hid-multitouch.c | 8 ++++++--
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 53655f81d995..83a03edb37e9 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -505,6 +505,7 @@
 #define USB_DEVICE_ID_GENERAL_TOUCH_WIN8_PIT_E100 0xe100
 
 #define I2C_VENDOR_ID_GOODIX		0x27c6
+#define I2C_DEVICE_ID_GOODIX_01E0	0x01e0
 #define I2C_DEVICE_ID_GOODIX_01E8	0x01e8
 #define I2C_DEVICE_ID_GOODIX_01E9	0x01e9
 #define I2C_DEVICE_ID_GOODIX_01F0	0x01f0
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 99812c0f830b..0faaa2c23d2e 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -1446,7 +1446,8 @@ static __u8 *mt_report_fixup(struct hid_device *hdev, __u8 *rdesc,
 {
 	if (hdev->vendor == I2C_VENDOR_ID_GOODIX &&
 	    (hdev->product == I2C_DEVICE_ID_GOODIX_01E8 ||
-	     hdev->product == I2C_DEVICE_ID_GOODIX_01E9)) {
+	     hdev->product == I2C_DEVICE_ID_GOODIX_01E9 ||
+		 hdev->product == I2C_DEVICE_ID_GOODIX_01E0)) {
 		if (rdesc[607] == 0x15) {
 			rdesc[607] = 0x25;
 			dev_info(
@@ -2065,7 +2066,10 @@ static const struct hid_device_id mt_devices[] = {
 		     I2C_DEVICE_ID_GOODIX_01E8) },
 	{ .driver_data = MT_CLS_WIN_8_FORCE_MULTI_INPUT_NSMU,
 	  HID_DEVICE(BUS_I2C, HID_GROUP_ANY, I2C_VENDOR_ID_GOODIX,
-		     I2C_DEVICE_ID_GOODIX_01E8) },
+		     I2C_DEVICE_ID_GOODIX_01E9) },
+	{ .driver_data = MT_CLS_WIN_8_FORCE_MULTI_INPUT_NSMU,
+	  HID_DEVICE(BUS_I2C, HID_GROUP_ANY, I2C_VENDOR_ID_GOODIX,
+		     I2C_DEVICE_ID_GOODIX_01E0) },
 
 	/* GoodTouch panels */
 	{ .driver_data = MT_CLS_NSMU,
-- 
2.45.2


^ permalink raw reply related

* Re: [PATCH 08/17] Input: iqs62x-keys - use cleanup facility for fwnodes
From: Jeff LaBundy @ 2024-08-26 22:43 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: linux-input, Michael Hennerich, Shawn Guo, Sascha Hauer,
	Fabio Estevam, Laxman Dewangan, Thierry Reding, Hans de Goede,
	Tony Lindgren, linux-kernel, imx, linux-arm-kernel, linux-tegra
In-Reply-To: <20240825051627.2848495-9-dmitry.torokhov@gmail.com>

Hi Dmitry,

On Sat, Aug 24, 2024 at 10:16:12PM -0700, Dmitry Torokhov wrote:
> Use __free(fwnode_handle) cleanup facility to ensure that references
> to acquired fwnodes are dropped at appropriate times automatically.
> 
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Reviewed-by: Jeff LaBundy <jeff@labundy.com>

> ---
>  drivers/input/keyboard/iqs62x-keys.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/input/keyboard/iqs62x-keys.c b/drivers/input/keyboard/iqs62x-keys.c
> index 688d61244b5f..1315b0f0862f 100644
> --- a/drivers/input/keyboard/iqs62x-keys.c
> +++ b/drivers/input/keyboard/iqs62x-keys.c
> @@ -45,7 +45,6 @@ struct iqs62x_keys_private {
>  static int iqs62x_keys_parse_prop(struct platform_device *pdev,
>  				  struct iqs62x_keys_private *iqs62x_keys)
>  {
> -	struct fwnode_handle *child;
>  	unsigned int val;
>  	int ret, i;
>  
> @@ -68,7 +67,8 @@ static int iqs62x_keys_parse_prop(struct platform_device *pdev,
>  	}
>  
>  	for (i = 0; i < ARRAY_SIZE(iqs62x_keys->switches); i++) {
> -		child = device_get_named_child_node(&pdev->dev,
> +		struct fwnode_handle *child __free(fwnode_handle) =
> +			device_get_named_child_node(&pdev->dev,
>  						    iqs62x_switch_names[i]);
>  		if (!child)
>  			continue;
> @@ -77,7 +77,6 @@ static int iqs62x_keys_parse_prop(struct platform_device *pdev,
>  		if (ret) {
>  			dev_err(&pdev->dev, "Failed to read switch code: %d\n",
>  				ret);
> -			fwnode_handle_put(child);
>  			return ret;
>  		}
>  		iqs62x_keys->switches[i].code = val;
> @@ -91,8 +90,6 @@ static int iqs62x_keys_parse_prop(struct platform_device *pdev,
>  			iqs62x_keys->switches[i].flag = (i == IQS62X_SW_HALL_N ?
>  							 IQS62X_EVENT_HALL_N_T :
>  							 IQS62X_EVENT_HALL_S_T);
> -
> -		fwnode_handle_put(child);
>  	}
>  
>  	return 0;
> -- 
> 2.46.0.295.g3b9ea8a38a-goog
> 

Kind regards,
Jeff LaBundy

^ permalink raw reply

* Re: [PATCH RESEND v11 2/3] Input: adp5588-keys - add support for pure gpio
From: Dmitry Torokhov @ 2024-08-26 19:17 UTC (permalink / raw)
  To: utsav.agarwal
  Cc: Michael Hennerich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Nuno Sá, linux-input, devicetree, linux-kernel,
	Arturs Artamonovs, Vasileios Bimpikas, Oliver Gaskell
In-Reply-To: <20240826-adp5588_gpio_support-v11-2-3e5ac2bd31b7@analog.com>

Hi Utsav,

On Mon, Aug 26, 2024 at 06:22:02PM +0100, Utsav Agarwal via B4 Relay wrote:
> From: Utsav Agarwal <utsav.agarwal@analog.com>
> 
> Keypad specific setup is relaxed if no keypad rows/columns are specified,
> enabling a purely gpio operation.
> 
> Reviewed-by: Nuno Sa <nuno.sa@analog.com>
> Signed-off-by: Utsav Agarwal <utsav.agarwal@analog.com>
> ---
>  drivers/input/keyboard/adp5588-keys.c | 37 +++++++++++++++++++++++++++++++----
>  1 file changed, 33 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/input/keyboard/adp5588-keys.c b/drivers/input/keyboard/adp5588-keys.c
> index 09bcfc6b9408..7c32f8b69a3e 100644
> --- a/drivers/input/keyboard/adp5588-keys.c
> +++ b/drivers/input/keyboard/adp5588-keys.c
> @@ -188,6 +188,7 @@ struct adp5588_kpad {
>  	u32 cols;
>  	u32 unlock_keys[2];
>  	int nkeys_unlock;
> +	bool gpio_only;
>  	unsigned short keycode[ADP5588_KEYMAPSIZE];
>  	unsigned char gpiomap[ADP5588_MAXGPIO];
>  	struct gpio_chip gc;
> @@ -431,10 +432,12 @@ static int adp5588_gpio_add(struct adp5588_kpad *kpad)
>  	kpad->gc.label = kpad->client->name;
>  	kpad->gc.owner = THIS_MODULE;
>  
> -	girq = &kpad->gc.irq;
> -	gpio_irq_chip_set_chip(girq, &adp5588_irq_chip);
> -	girq->handler = handle_bad_irq;
> -	girq->threaded = true;
> +	if (kpad->client->irq) {
> +		girq = &kpad->gc.irq;
> +		gpio_irq_chip_set_chip(girq, &adp5588_irq_chip);
> +		girq->handler = handle_bad_irq;
> +		girq->threaded = true;
> +	}

I think we should only set up irqchip if we have "interrupt-controller"
property in the device tree.

>  
>  	mutex_init(&kpad->gpio_lock);
>  
> @@ -632,6 +635,21 @@ static int adp5588_fw_parse(struct adp5588_kpad *kpad)
>  	struct i2c_client *client = kpad->client;
>  	int ret, i;
>  
> +	/*
> +	 * Check if the device is to be operated purely in GPIO mode. To do
> +	 * so, check that no keypad rows or columns have been specified,
> +	 * since all GPINS should be configured as GPIO.
> +	 */
> +	ret = device_property_present(&client->dev,
> +			"keypad,num-rows");
> +	ret |= device_property_present(&client->dev,
> +			"keypad,num-columns");
> +	/* If purely GPIO, skip keypad setup */
> +	if (!ret) {
> +		kpad->gpio_only = true;
> +		return 0;
> +	}
> +
>  	ret = matrix_keypad_parse_properties(&client->dev, &kpad->rows,
>  					     &kpad->cols);
>  	if (ret)
> @@ -775,6 +793,11 @@ static int adp5588_probe(struct i2c_client *client)
>  	if (error)
>  		return error;
>  
> +	if (kpad->gpio_only && !client->irq) {
> +		dev_info(&client->dev, "Rev.%d, started as GPIO only\n", revid);
> +		return 0;
> +	}

This is way too noisy. I think one message logging the revision ID
should be enough. The rest of the data can be found from elsewhere if
needed.

Can you try the below on top of yours? If this works I'' squash it
together with your change.

Thanks.

-- 
Dmitry


diff --git a/drivers/input/keyboard/adp5588-keys.c b/drivers/input/keyboard/adp5588-keys.c
index 7c32f8b69a3e..b5f4becf5cb6 100644
--- a/drivers/input/keyboard/adp5588-keys.c
+++ b/drivers/input/keyboard/adp5588-keys.c
@@ -432,7 +432,12 @@ static int adp5588_gpio_add(struct adp5588_kpad *kpad)
 	kpad->gc.label = kpad->client->name;
 	kpad->gc.owner = THIS_MODULE;
 
-	if (kpad->client->irq) {
+	if (device_property_present(dev, "interrupt-controller")) {
+		if (!kpad->client->irq) {
+			dev_err(dev, "Unable to serve as interrupt controller without interrupt");
+			return -EINVAL;
+		}
+
 		girq = &kpad->gc.irq;
 		gpio_irq_chip_set_chip(girq, &adp5588_irq_chip);
 		girq->handler = handle_bad_irq;
@@ -640,12 +645,9 @@ static int adp5588_fw_parse(struct adp5588_kpad *kpad)
 	 * so, check that no keypad rows or columns have been specified,
 	 * since all GPINS should be configured as GPIO.
 	 */
-	ret = device_property_present(&client->dev,
-			"keypad,num-rows");
-	ret |= device_property_present(&client->dev,
-			"keypad,num-columns");
-	/* If purely GPIO, skip keypad setup */
-	if (!ret) {
+	if (!device_property_present(&client->dev, "keypad,num-rows") &&
+	    !device_property_present(&client->dev, "keypad,num-columns")) {
+		/* If purely GPIO, skip keypad setup */
 		kpad->gpio_only = true;
 		return 0;
 	}
@@ -793,28 +795,19 @@ static int adp5588_probe(struct i2c_client *client)
 	if (error)
 		return error;
 
-	if (kpad->gpio_only && !client->irq) {
-		dev_info(&client->dev, "Rev.%d, started as GPIO only\n", revid);
-		return 0;
-	}
-
-	error = devm_request_threaded_irq(&client->dev, client->irq,
-					  adp5588_hard_irq, adp5588_thread_irq,
-					  IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
-					  client->dev.driver->name, kpad);
-	if (error) {
-		dev_err(&client->dev, "failed to request irq %d: %d\n",
-			client->irq, error);
-		return error;
-	}
-
-	if (kpad->gpio_only) {
-		dev_info(&client->dev, "Rev.%d GPIO only, irq %d\n",
-				revid, client->irq);
-		return 0;
+	if (client->irq) {
+		error = devm_request_threaded_irq(&client->dev, client->irq,
+						  adp5588_hard_irq, adp5588_thread_irq,
+						  IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
+						  client->dev.driver->name, kpad);
+		if (error) {
+			dev_err(&client->dev, "failed to request irq %d: %d\n",
+				client->irq, error);
+			return error;
+		}
 	}
 
-	dev_info(&client->dev, "Rev.%d keypad, irq %d\n", revid, client->irq);
+	dev_info(&client->dev, "Rev.%d controller\n", revid);
 	return 0;
 }
 

^ permalink raw reply related

* Re: [PATCH v6 1/1] dt-bindings: input: touchscreen: convert ads7846.txt to yaml
From: Dmitry Torokhov @ 2024-08-26 18:59 UTC (permalink / raw)
  To: Frank Li
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Marek Vasut,
	Linus Walleij, Mark Hasemeyer, Alexander Stein,
	open list:INPUT (KEYBOARD, MOUSE, JOYSTICK, TOUCHSCREEN)...,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list, imx
In-Reply-To: <20240826162302.960732-1-Frank.Li@nxp.com>

On Mon, Aug 26, 2024 at 12:22:56PM -0400, Frank Li wrote:
> Convert binding doc ads7846.txt to yaml format.
> Additional change:
> - add ref to touchscreen.yaml and spi-peripheral-props.yaml.
> - use common node name touchscreen.
> - sort ti properties alphabetically.
> - sort common properties alphabetically.
> - sort compatible string alphabetically.
> - remove vcc-supply from required list.
> - deprecated ti,x-min, ti,y-min
> 
> Fix below warning: arch/arm64/boot/dts/freescale/imx8mm-var-som-symphony.dtb: touchscreen@0:
> 	ti,x-min: b'\x00}' is not of type 'object', 'array', 'boolean', 'null'
> 
> Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
> Reviewed-by: Marek Vasut <marex@denx.de>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>

Applied, thank you.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH 1/2] dt-bindings: input: qcom,pm8xxx-vib: Document PM6150 compatible
From: Rob Herring (Arm) @ 2024-08-26 17:57 UTC (permalink / raw)
  To: Jens Reidel
  Cc: linux-input, krzk+dt, linux-kernel, andersson, linux-arm-msm,
	konrad.dybcio, devicetree, conor+dt, dmitry.torokhov
In-Reply-To: <20240606181027.98537-2-adrian@travitia.xyz>


On Thu, 06 Jun 2024 20:10:26 +0200, Jens Reidel wrote:
> The PM6150 vibrator module is compatible with the PMI632 vibrator
> module, document the PM6150 vibrator compatible as fallback for the
> PMI632 vibrator.
> 
> Signed-off-by: Jens Reidel <adrian@travitia.xyz>
> ---
>  Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 

Seems this one was missed. Applied, thanks!


^ permalink raw reply

* Re: [PATCH v1] Input: wistron_btns - use kmemdup_array instead of kmemdup for multiple allocation
From: Dmitry Torokhov @ 2024-08-26 17:25 UTC (permalink / raw)
  To: Shen Lichuan; +Cc: mitr, linux-input, linux-kernel
In-Reply-To: <20240826045253.3503-1-shenlichuan@vivo.com>

On Mon, Aug 26, 2024 at 12:52:53PM +0800, Shen Lichuan wrote:
> Let the kmemdup_array() take care about multiplication
> and possible overflows.
> 
> Using kmemdup_array() is more appropriate and makes the code
> easier to audit.
> 
> Signed-off-by: Shen Lichuan <shenlichuan@vivo.com>

Applied, thank you.

-- 
Dmitry

^ permalink raw reply

* [PATCH RESEND v11 3/3] dt-bindings: input: pure gpio support for adp5588
From: Utsav Agarwal via B4 Relay @ 2024-08-26 17:22 UTC (permalink / raw)
  To: Utsav Agarwal, Michael Hennerich, Dmitry Torokhov, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Nuno Sá
  Cc: linux-input, devicetree, linux-kernel, Arturs Artamonovs,
	Vasileios Bimpikas, Oliver Gaskell, Conor Dooley
In-Reply-To: <20240826-adp5588_gpio_support-v11-0-3e5ac2bd31b7@analog.com>

From: Utsav Agarwal <utsav.agarwal@analog.com>

Adding software support for enabling the pure gpio capability of the
device - which allows all I/O to be used as GPIO. Previously, I/O
configuration was limited by software to partial GPIO support only.

When working in a pure gpio mode, the device does not require the
certain properties and hence, the following are now made optional:
	- interrupts
	- keypad,num-rows
	- keypad,num-columns
	- linux,keymap

However, note that the above are required to be specified when
configuring the device as a keypad, for which dependencies have been added
such that specifying either one requires the remaining as well.

Also, note that interrupts are made optional, but required when the device
has either been configured in keypad mode or as an interrupt controller.
This has been done since they may not necessarily be used when leveraging
the device purely for GPIO.

Acked-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: Utsav Agarwal <utsav.agarwal@analog.com>
---
 .../devicetree/bindings/input/adi,adp5588.yaml     | 38 +++++++++++++++++++---
 1 file changed, 33 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/input/adi,adp5588.yaml b/Documentation/devicetree/bindings/input/adi,adp5588.yaml
index 26ea66834ae2..336bc352579a 100644
--- a/Documentation/devicetree/bindings/input/adi,adp5588.yaml
+++ b/Documentation/devicetree/bindings/input/adi,adp5588.yaml
@@ -49,7 +49,10 @@ properties:
   interrupt-controller:
     description:
       This property applies if either keypad,num-rows lower than 8 or
-      keypad,num-columns lower than 10.
+      keypad,num-columns lower than 10. This property is optional if
+      keypad,num-rows or keypad,num-columns are not specified as the
+      device is then configured to be used purely for gpio during which
+      interrupts may or may not be utilized.
 
   '#interrupt-cells':
     const: 2
@@ -65,13 +68,23 @@ properties:
     minItems: 1
     maxItems: 2
 
+dependencies:
+  keypad,num-rows:
+    - linux,keymap
+    - keypad,num-columns
+  keypad,num-columns:
+    - linux,keymap
+    - keypad,num-rows
+  linux,keymap:
+    - keypad,num-rows
+    - keypad,num-columns
+    - interrupts
+  interrupt-controller:
+    - interrupts
+
 required:
   - compatible
   - reg
-  - interrupts
-  - keypad,num-rows
-  - keypad,num-columns
-  - linux,keymap
 
 unevaluatedProperties: false
 
@@ -108,4 +121,19 @@ examples:
             >;
         };
     };
+
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        gpio@34 {
+            compatible = "adi,adp5588";
+            reg = <0x34>;
+
+            #gpio-cells = <2>;
+            gpio-controller;
+        };
+    };
+
 ...

-- 
2.34.1



^ permalink raw reply related

* [PATCH RESEND v11 2/3] Input: adp5588-keys - add support for pure gpio
From: Utsav Agarwal via B4 Relay @ 2024-08-26 17:22 UTC (permalink / raw)
  To: Utsav Agarwal, Michael Hennerich, Dmitry Torokhov, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Nuno Sá
  Cc: linux-input, devicetree, linux-kernel, Arturs Artamonovs,
	Vasileios Bimpikas, Oliver Gaskell
In-Reply-To: <20240826-adp5588_gpio_support-v11-0-3e5ac2bd31b7@analog.com>

From: Utsav Agarwal <utsav.agarwal@analog.com>

Keypad specific setup is relaxed if no keypad rows/columns are specified,
enabling a purely gpio operation.

Reviewed-by: Nuno Sa <nuno.sa@analog.com>
Signed-off-by: Utsav Agarwal <utsav.agarwal@analog.com>
---
 drivers/input/keyboard/adp5588-keys.c | 37 +++++++++++++++++++++++++++++++----
 1 file changed, 33 insertions(+), 4 deletions(-)

diff --git a/drivers/input/keyboard/adp5588-keys.c b/drivers/input/keyboard/adp5588-keys.c
index 09bcfc6b9408..7c32f8b69a3e 100644
--- a/drivers/input/keyboard/adp5588-keys.c
+++ b/drivers/input/keyboard/adp5588-keys.c
@@ -188,6 +188,7 @@ struct adp5588_kpad {
 	u32 cols;
 	u32 unlock_keys[2];
 	int nkeys_unlock;
+	bool gpio_only;
 	unsigned short keycode[ADP5588_KEYMAPSIZE];
 	unsigned char gpiomap[ADP5588_MAXGPIO];
 	struct gpio_chip gc;
@@ -431,10 +432,12 @@ static int adp5588_gpio_add(struct adp5588_kpad *kpad)
 	kpad->gc.label = kpad->client->name;
 	kpad->gc.owner = THIS_MODULE;
 
-	girq = &kpad->gc.irq;
-	gpio_irq_chip_set_chip(girq, &adp5588_irq_chip);
-	girq->handler = handle_bad_irq;
-	girq->threaded = true;
+	if (kpad->client->irq) {
+		girq = &kpad->gc.irq;
+		gpio_irq_chip_set_chip(girq, &adp5588_irq_chip);
+		girq->handler = handle_bad_irq;
+		girq->threaded = true;
+	}
 
 	mutex_init(&kpad->gpio_lock);
 
@@ -632,6 +635,21 @@ static int adp5588_fw_parse(struct adp5588_kpad *kpad)
 	struct i2c_client *client = kpad->client;
 	int ret, i;
 
+	/*
+	 * Check if the device is to be operated purely in GPIO mode. To do
+	 * so, check that no keypad rows or columns have been specified,
+	 * since all GPINS should be configured as GPIO.
+	 */
+	ret = device_property_present(&client->dev,
+			"keypad,num-rows");
+	ret |= device_property_present(&client->dev,
+			"keypad,num-columns");
+	/* If purely GPIO, skip keypad setup */
+	if (!ret) {
+		kpad->gpio_only = true;
+		return 0;
+	}
+
 	ret = matrix_keypad_parse_properties(&client->dev, &kpad->rows,
 					     &kpad->cols);
 	if (ret)
@@ -775,6 +793,11 @@ static int adp5588_probe(struct i2c_client *client)
 	if (error)
 		return error;
 
+	if (kpad->gpio_only && !client->irq) {
+		dev_info(&client->dev, "Rev.%d, started as GPIO only\n", revid);
+		return 0;
+	}
+
 	error = devm_request_threaded_irq(&client->dev, client->irq,
 					  adp5588_hard_irq, adp5588_thread_irq,
 					  IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
@@ -785,6 +808,12 @@ static int adp5588_probe(struct i2c_client *client)
 		return error;
 	}
 
+	if (kpad->gpio_only) {
+		dev_info(&client->dev, "Rev.%d GPIO only, irq %d\n",
+				revid, client->irq);
+		return 0;
+	}
+
 	dev_info(&client->dev, "Rev.%d keypad, irq %d\n", revid, client->irq);
 	return 0;
 }

-- 
2.34.1



^ permalink raw reply related

* [PATCH RESEND v11 0/3] adp5588-keys: Support for dedicated gpio operation
From: Utsav Agarwal via B4 Relay @ 2024-08-26 17:22 UTC (permalink / raw)
  To: Utsav Agarwal, Michael Hennerich, Dmitry Torokhov, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Nuno Sá
  Cc: linux-input, devicetree, linux-kernel, Arturs Artamonovs,
	Vasileios Bimpikas, Oliver Gaskell, Conor Dooley

Current state of the driver for the ADP5588/87 only allows partial
I/O to be used as GPIO. This support was previously present as a
separate gpio driver, which was dropped with the commit
5ddc896088b0 ("gpio: gpio-adp5588: drop the driver") since the
functionality was deemed to have been merged with adp5588-keys.

This series of patches re-enables this support by allowing the driver to
relax the requirement for registering a keymap and enable pure GPIO
operation.

Signed-off-by: Utsav Agarwal <utsav.agarwal@analog.com>
---
Changelog
==========

Changes in v11:
	- dtbinding: Restored accidently deleted characters in previous
	  revision
- Link to v10: https://lore.kernel.org/r/20240813-adp5588_gpio_support-v10-0-aab3c52cc8bf@analog.com

Changes in v10:
	- Corrected changelog ordering
	- Changed dtbinding commit to clarify all changes are made in
	  software. The commit message now also expands on what
	  the desired pure gpio mode is
	- Added acquired tags to commits
	- dt-binding:
		Removed multiple blank lines before the dependecies block
	  	Removed excess headers included in dtbinding example
	  	Removed constraint being repeated in free form text
	  	Merged outlying dependency into a single block
- Link to v9: https://lore.kernel.org/r/20240806-adp5588_gpio_support-v9-0-4d6118b6d653@analog.com

Changes in v9:
	- Added dt-binding dependency for interrupt-controller. Now if
	  interrupt-controller is specified, interrupts must be
	  provided.
- Link to v8: https://lore.kernel.org/r/20240704-adp5588_gpio_support-v8-0-208cf5d4c2d6@analog.com

Changes in v8:
	- Fixed indentation in document example (removed extra spaces)
- Link to v7: https://lore.kernel.org/r/20240704-adp5588_gpio_support-v7-0-e34eb7eba5ab@analog.com

Changes in v7:
	- Fixed commit subject for transported patch
	- Driver now does not setup gpio_irq_chip if
	  interrupt has not been provided
	- Fixed indentation for dtbinding example
- Link to v6: https://lore.kernel.org/r/20240704-adp5588_gpio_support-v6-0-cb65514d714b@analog.com

Changes in v6:
	- Restored functionality to register interrupts in GPIO
	  mode(i.e, these are optional but not exclusive to keypad mode
	  since even in pure gpio mode, they can be used as inputs via
	  gpio-keys)
	- Updated dt-bindings such that each keypad property depends on
	  the others. Interrupts, although optional are now required by
	  keypad mode but are not limited to it.
- Link to v5: https://lore.kernel.org/r/20240703-adp5588_gpio_support-v5-0-49fcead0d390@analog.com

V5:
	- Removed extra property "gpio-only", now pure gpio mode is
	  detected via the adbsence of keypad specific properties.
	- Added dependencies for keypad properties to preserve
	  the original requirements in case a pure gpio mode is not
	  being used.
	- Added additional description for why the "interrupts" property
	  was made optional
	- Rebased current work based on https://lore.kernel.org/linux-input/ZoLt_qBCQS-tG8Ar@google.com/
- Link to v4: https://lore.kernel.org/r/20240701-adp5588_gpio_support-v4-0-44bba0445e90@analog.com

V4:
	- Added dt-bindings patch

V3:
	-  Moved device_property_present() for reading "gpio-only" into
	adp558_fw_parse()
	-  Added print statements in case of error

V2:
	-  Changed gpio_only from a local variable to a member of struct
	adp5588_kpad
	-  Removed condition from adp5588_probe() to skip adp5588_fw_parse() if
	gpio-only specified. adp558_fw_parse() now handles and returns
	0 if gpio-only has been specified.
	-  Added a check in adp5588_fw_parse() to make sure keypad
	properties(keypad,num-columns and keypad,num-rows) were not defined when
	gpio-only specified

---

---
Dmitry Torokhov (1):
      Input: adp5588-keys - use guard notation when acquiring mutexes

Utsav Agarwal (2):
      Input: adp5588-keys - add support for pure gpio
      dt-bindings: input: pure gpio support for adp5588

 .../devicetree/bindings/input/adi,adp5588.yaml     | 38 ++++++++--
 drivers/input/keyboard/adp5588-keys.c              | 86 +++++++++++++---------
 2 files changed, 83 insertions(+), 41 deletions(-)
---
base-commit: 1c52cf5e79d30ac996f34b64284f2c317004d641
change-id: 20240701-adp5588_gpio_support-65db2bd21a9f

Best regards,
-- 
Utsav Agarwal <utsav.agarwal@analog.com>



^ permalink raw reply

* [PATCH RESEND v11 1/3] Input: adp5588-keys - use guard notation when acquiring mutexes
From: Utsav Agarwal via B4 Relay @ 2024-08-26 17:22 UTC (permalink / raw)
  To: Utsav Agarwal, Michael Hennerich, Dmitry Torokhov, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Nuno Sá
  Cc: linux-input, devicetree, linux-kernel, Arturs Artamonovs,
	Vasileios Bimpikas, Oliver Gaskell
In-Reply-To: <20240826-adp5588_gpio_support-v11-0-3e5ac2bd31b7@analog.com>

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>

This makes the code more compact and error handling more robust.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Acked-by: Michael Hennerich <michael.hennerich@analog.com>
---
 drivers/input/keyboard/adp5588-keys.c | 49 ++++++++++++-----------------------
 1 file changed, 17 insertions(+), 32 deletions(-)

diff --git a/drivers/input/keyboard/adp5588-keys.c b/drivers/input/keyboard/adp5588-keys.c
index 1b0279393df4..09bcfc6b9408 100644
--- a/drivers/input/keyboard/adp5588-keys.c
+++ b/drivers/input/keyboard/adp5588-keys.c
@@ -221,15 +221,13 @@ static int adp5588_gpio_get_value(struct gpio_chip *chip, unsigned int off)
 	unsigned int bit = ADP5588_BIT(kpad->gpiomap[off]);
 	int val;
 
-	mutex_lock(&kpad->gpio_lock);
+	guard(mutex)(&kpad->gpio_lock);
 
 	if (kpad->dir[bank] & bit)
 		val = kpad->dat_out[bank];
 	else
 		val = adp5588_read(kpad->client, GPIO_DAT_STAT1 + bank);
 
-	mutex_unlock(&kpad->gpio_lock);
-
 	return !!(val & bit);
 }
 
@@ -240,7 +238,7 @@ static void adp5588_gpio_set_value(struct gpio_chip *chip,
 	unsigned int bank = ADP5588_BANK(kpad->gpiomap[off]);
 	unsigned int bit = ADP5588_BIT(kpad->gpiomap[off]);
 
-	mutex_lock(&kpad->gpio_lock);
+	guard(mutex)(&kpad->gpio_lock);
 
 	if (val)
 		kpad->dat_out[bank] |= bit;
@@ -248,8 +246,6 @@ static void adp5588_gpio_set_value(struct gpio_chip *chip,
 		kpad->dat_out[bank] &= ~bit;
 
 	adp5588_write(kpad->client, GPIO_DAT_OUT1 + bank, kpad->dat_out[bank]);
-
-	mutex_unlock(&kpad->gpio_lock);
 }
 
 static int adp5588_gpio_set_config(struct gpio_chip *chip,  unsigned int off,
@@ -259,7 +255,6 @@ static int adp5588_gpio_set_config(struct gpio_chip *chip,  unsigned int off,
 	unsigned int bank = ADP5588_BANK(kpad->gpiomap[off]);
 	unsigned int bit = ADP5588_BIT(kpad->gpiomap[off]);
 	bool pull_disable;
-	int ret;
 
 	switch (pinconf_to_config_param(config)) {
 	case PIN_CONFIG_BIAS_PULL_UP:
@@ -272,19 +267,15 @@ static int adp5588_gpio_set_config(struct gpio_chip *chip,  unsigned int off,
 		return -ENOTSUPP;
 	}
 
-	mutex_lock(&kpad->gpio_lock);
+	guard(mutex)(&kpad->gpio_lock);
 
 	if (pull_disable)
 		kpad->pull_dis[bank] |= bit;
 	else
 		kpad->pull_dis[bank] &= bit;
 
-	ret = adp5588_write(kpad->client, GPIO_PULL1 + bank,
-			    kpad->pull_dis[bank]);
-
-	mutex_unlock(&kpad->gpio_lock);
-
-	return ret;
+	return adp5588_write(kpad->client, GPIO_PULL1 + bank,
+			     kpad->pull_dis[bank]);
 }
 
 static int adp5588_gpio_direction_input(struct gpio_chip *chip, unsigned int off)
@@ -292,16 +283,11 @@ static int adp5588_gpio_direction_input(struct gpio_chip *chip, unsigned int off
 	struct adp5588_kpad *kpad = gpiochip_get_data(chip);
 	unsigned int bank = ADP5588_BANK(kpad->gpiomap[off]);
 	unsigned int bit = ADP5588_BIT(kpad->gpiomap[off]);
-	int ret;
 
-	mutex_lock(&kpad->gpio_lock);
+	guard(mutex)(&kpad->gpio_lock);
 
 	kpad->dir[bank] &= ~bit;
-	ret = adp5588_write(kpad->client, GPIO_DIR1 + bank, kpad->dir[bank]);
-
-	mutex_unlock(&kpad->gpio_lock);
-
-	return ret;
+	return adp5588_write(kpad->client, GPIO_DIR1 + bank, kpad->dir[bank]);
 }
 
 static int adp5588_gpio_direction_output(struct gpio_chip *chip,
@@ -310,9 +296,9 @@ static int adp5588_gpio_direction_output(struct gpio_chip *chip,
 	struct adp5588_kpad *kpad = gpiochip_get_data(chip);
 	unsigned int bank = ADP5588_BANK(kpad->gpiomap[off]);
 	unsigned int bit = ADP5588_BIT(kpad->gpiomap[off]);
-	int ret;
+	int error;
 
-	mutex_lock(&kpad->gpio_lock);
+	guard(mutex)(&kpad->gpio_lock);
 
 	kpad->dir[bank] |= bit;
 
@@ -321,17 +307,16 @@ static int adp5588_gpio_direction_output(struct gpio_chip *chip,
 	else
 		kpad->dat_out[bank] &= ~bit;
 
-	ret = adp5588_write(kpad->client, GPIO_DAT_OUT1 + bank,
-			    kpad->dat_out[bank]);
-	if (ret)
-		goto out_unlock;
-
-	ret = adp5588_write(kpad->client, GPIO_DIR1 + bank, kpad->dir[bank]);
+	error = adp5588_write(kpad->client, GPIO_DAT_OUT1 + bank,
+			      kpad->dat_out[bank]);
+	if (error)
+		return error;
 
-out_unlock:
-	mutex_unlock(&kpad->gpio_lock);
+	error = adp5588_write(kpad->client, GPIO_DIR1 + bank, kpad->dir[bank]);
+	if (error)
+		return error;
 
-	return ret;
+	return 0;
 }
 
 static int adp5588_build_gpiomap(struct adp5588_kpad *kpad)

-- 
2.34.1



^ permalink raw reply related

* [PATCH v6 1/1] dt-bindings: input: touchscreen: convert ads7846.txt to yaml
From: Frank Li @ 2024-08-26 16:22 UTC (permalink / raw)
  To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Marek Vasut, Linus Walleij, Mark Hasemeyer, Alexander Stein,
	open list:INPUT (KEYBOARD, MOUSE, JOYSTICK, TOUCHSCREEN)...,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list
  Cc: imx

Convert binding doc ads7846.txt to yaml format.
Additional change:
- add ref to touchscreen.yaml and spi-peripheral-props.yaml.
- use common node name touchscreen.
- sort ti properties alphabetically.
- sort common properties alphabetically.
- sort compatible string alphabetically.
- remove vcc-supply from required list.
- deprecated ti,x-min, ti,y-min

Fix below warning: arch/arm64/boot/dts/freescale/imx8mm-var-som-symphony.dtb: touchscreen@0:
	ti,x-min: b'\x00}' is not of type 'object', 'array', 'boolean', 'null'

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
Reviewed-by: Marek Vasut <marex@denx.de>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
Change from v5 to v6:
- Fix ti,y,min at commit message
- Add maxItems for gpios
- Add rob's review tag

Change from v4 to v5
- Add Reviewed-by: Marek Vasut <marex@denx.de>
- Start sentence with uppercase letter

Change from v3 to v4
- new line for all descrptions
- add . after sentense.

Change from v2 to v3
- Remove u16(u32) in descriptions
- deprecated ti,x-min and ti, y-min

Change from v1 to v2
- sort properties, by 3 group:
  1. General (compatible, reg, interrupt)
  2. Common properties
  3. ti properties
- sort maintainers name alphabetically.
- uint16 have to be kept because default is uint32
- remove vcc-supply from required list
- remove unfinished sentence "all mandatory properties described in"
because it refer to /schemas/spi/spi-peripheral-props.yaml#
- fix make refcheckdoc error
---
 .../bindings/input/touchscreen/ads7846.txt    | 107 ----------
 .../input/touchscreen/ti,ads7843.yaml         | 184 ++++++++++++++++++
 .../bindings/power/wakeup-source.txt          |   2 +-
 3 files changed, 185 insertions(+), 108 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/input/touchscreen/ads7846.txt
 create mode 100644 Documentation/devicetree/bindings/input/touchscreen/ti,ads7843.yaml

diff --git a/Documentation/devicetree/bindings/input/touchscreen/ads7846.txt b/Documentation/devicetree/bindings/input/touchscreen/ads7846.txt
deleted file mode 100644
index 399c87782935c..0000000000000
--- a/Documentation/devicetree/bindings/input/touchscreen/ads7846.txt
+++ /dev/null
@@ -1,107 +0,0 @@
-Device tree bindings for TI's ADS7843, ADS7845, ADS7846, ADS7873, TSC2046
-SPI driven touch screen controllers.
-
-The node for this driver must be a child node of a SPI controller, hence
-all mandatory properties described in
-
-	Documentation/devicetree/bindings/spi/spi-bus.txt
-
-must be specified.
-
-Additional required properties:
-
-	compatible		Must be one of the following, depending on the
-				model:
-					"ti,tsc2046"
-					"ti,ads7843"
-					"ti,ads7845"
-					"ti,ads7846"
-					"ti,ads7873"
-
-	interrupts		An interrupt node describing the IRQ line the chip's
-				!PENIRQ pin is connected to.
-	vcc-supply		A regulator node for the supply voltage.
-
-
-Optional properties:
-
-	ti,vref-delay-usecs		vref supply delay in usecs, 0 for
-					external vref (u16).
-	ti,vref-mv			The VREF voltage, in millivolts (u16).
-					Set to 0 to use internal references
-					(ADS7846).
-	ti,keep-vref-on			set to keep vref on for differential
-					measurements as well
-	ti,settle-delay-usec		Settling time of the analog signals;
-					a function of Vcc and the capacitance
-					on the X/Y drivers.  If set to non-zero,
-					two samples are taken with settle_delay
-					us apart, and the second one is used.
-					~150 uSec with 0.01uF caps (u16).
-	ti,penirq-recheck-delay-usecs	If set to non-zero, after samples are
-					taken this delay is applied and penirq
-					is rechecked, to help avoid false
-					events.  This value is affected by the
-					material used to build the touch layer
-					(u16).
-	ti,x-plate-ohms			Resistance of the X-plate,
-					in Ohms (u16).
-	ti,y-plate-ohms			Resistance of the Y-plate,
-					in Ohms (u16).
-	ti,x-min			Minimum value on the X axis (u16).
-	ti,y-min			Minimum value on the Y axis (u16).
-	ti,debounce-tol			Tolerance used for filtering (u16).
-	ti,debounce-rep			Additional consecutive good readings
-					required after the first two (u16).
-	ti,pendown-gpio-debounce	Platform specific debounce time for the
-					pendown-gpio (u32).
-	pendown-gpio			GPIO handle describing the pin the !PENIRQ
-					line is connected to.
-	ti,hsync-gpios			GPIO line to poll for hsync
-	wakeup-source			use any event on touchscreen as wakeup event.
-					(Legacy property support: "linux,wakeup")
-	touchscreen-size-x		General touchscreen binding, see [1].
-	touchscreen-size-y		General touchscreen binding, see [1].
-	touchscreen-max-pressure	General touchscreen binding, see [1].
-	touchscreen-min-pressure	General touchscreen binding, see [1].
-	touchscreen-average-samples	General touchscreen binding, see [1].
-	touchscreen-inverted-x		General touchscreen binding, see [1].
-	touchscreen-inverted-y		General touchscreen binding, see [1].
-	touchscreen-swapped-x-y		General touchscreen binding, see [1].
-
-[1] All general touchscreen properties are described in
-    Documentation/devicetree/bindings/input/touchscreen/touchscreen.txt.
-
-Deprecated properties:
-
-	ti,swap-xy			swap x and y axis
-	ti,x-max			Maximum value on the X axis (u16).
-	ti,y-max			Maximum value on the Y axis (u16).
-	ti,pressure-min			Minimum reported pressure value
-					(threshold) - u16.
-	ti,pressure-max			Maximum reported pressure value (u16).
-	ti,debounce-max			Max number of additional readings per
-					sample (u16).
-
-Example for a TSC2046 chip connected to an McSPI controller of an OMAP SoC::
-
-	spi_controller {
-		tsc2046@0 {
-			reg = <0>;	/* CS0 */
-			compatible = "ti,tsc2046";
-			interrupt-parent = <&gpio1>;
-			interrupts = <8 0>;	/* BOOT6 / GPIO 8 */
-			spi-max-frequency = <1000000>;
-			pendown-gpio = <&gpio1 8 0>;
-			vcc-supply = <&reg_vcc3>;
-
-			ti,x-min = /bits/ 16 <0>;
-			ti,x-max = /bits/ 16 <8000>;
-			ti,y-min = /bits/ 16 <0>;
-			ti,y-max = /bits/ 16 <4800>;
-			ti,x-plate-ohms = /bits/ 16 <40>;
-			ti,pressure-max = /bits/ 16 <255>;
-
-			wakeup-source;
-		};
-	};
diff --git a/Documentation/devicetree/bindings/input/touchscreen/ti,ads7843.yaml b/Documentation/devicetree/bindings/input/touchscreen/ti,ads7843.yaml
new file mode 100644
index 0000000000000..34c95ec9da025
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/ti,ads7843.yaml
@@ -0,0 +1,184 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/touchscreen/ti,ads7843.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: TI's SPI driven touch screen controllers
+
+maintainers:
+  - Alexander Stein <alexander.stein@ew.tq-group.com>
+  - Dmitry Torokhov <dmitry.torokhov@gmail.com>
+  - Marek Vasut <marex@denx.de>
+
+description:
+  TI's ADS7843, ADS7845, ADS7846, ADS7873, TSC2046 SPI driven touch screen
+  controllers.
+
+properties:
+  compatible:
+    enum:
+      - ti,ads7843
+      - ti,ads7845
+      - ti,ads7846
+      - ti,ads7873
+      - ti,tsc2046
+
+  interrupts:
+    maxItems: 1
+
+  pendown-gpio:
+    maxItems: 1
+    description:
+      GPIO handle describing the pin the !PENIRQ line is connected to.
+
+  vcc-supply:
+    description:
+      A regulator node for the supply voltage.
+
+  wakeup-source: true
+
+  ti,debounce-max:
+    deprecated: true
+    $ref: /schemas/types.yaml#/definitions/uint16
+    description:
+      Max number of additional readings per sample.
+
+  ti,debounce-rep:
+    $ref: /schemas/types.yaml#/definitions/uint16
+    description:
+      Additional consecutive good readings required after the first two.
+
+  ti,debounce-tol:
+    $ref: /schemas/types.yaml#/definitions/uint16
+    description:
+      Tolerance used for filtering.
+
+  ti,hsync-gpios:
+    maxItems: 1
+    description:
+      GPIO line to poll for hsync.
+
+  ti,keep-vref-on:
+    $ref: /schemas/types.yaml#/definitions/flag
+    description:
+      Set to keep Vref on for differential measurements as well.
+
+  ti,pendown-gpio-debounce:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      Platform specific debounce time for the pendown-gpio.
+
+  ti,penirq-recheck-delay-usecs:
+    $ref: /schemas/types.yaml#/definitions/uint16
+    description:
+      If set to non-zero, after samples are taken this delay is applied and
+      penirq is rechecked, to help avoid false events.  This value is
+      affected by the material used to build the touch layer.
+
+  ti,pressure-max:
+    deprecated: true
+    $ref: /schemas/types.yaml#/definitions/uint16
+    description:
+      Maximum reported pressure value.
+
+  ti,pressure-min:
+    deprecated: true
+    $ref: /schemas/types.yaml#/definitions/uint16
+    description:
+      Minimum reported pressure value (threshold).
+
+  ti,settle-delay-usec:
+    $ref: /schemas/types.yaml#/definitions/uint16
+    description:
+      Settling time of the analog signals; a function of Vcc and the
+      capacitance on the X/Y drivers.  If set to non-zero, two samples are
+      taken with settle_delay us apart, and the second one is used. ~150
+      uSec with 0.01uF caps.
+
+  ti,swap-xy:
+    deprecated: true
+    $ref: /schemas/types.yaml#/definitions/flag
+    description:
+      Swap x and y axis.
+
+  ti,vref-delay-usecs:
+    $ref: /schemas/types.yaml#/definitions/uint16
+    description:
+      Vref supply delay in usecs, 0 for external Vref.
+
+  ti,vref-mv:
+    $ref: /schemas/types.yaml#/definitions/uint16
+    description:
+      The VREF voltage, in millivolts.
+      Set to 0 to use internal references (ADS7846).
+
+  ti,x-plate-ohms:
+    $ref: /schemas/types.yaml#/definitions/uint16
+    description:
+      Resistance of the X-plate, in Ohms.
+
+  ti,x-max:
+    deprecated: true
+    $ref: /schemas/types.yaml#/definitions/uint16
+    description:
+      Maximum value on the X axis.
+
+  ti,x-min:
+    deprecated: true
+    $ref: /schemas/types.yaml#/definitions/uint16
+    description:
+      Minimum value on the X axis.
+
+  ti,y-plate-ohms:
+    $ref: /schemas/types.yaml#/definitions/uint16
+    description:
+      Resistance of the Y-plate, in Ohms.
+
+  ti,y-max:
+    deprecated: true
+    $ref: /schemas/types.yaml#/definitions/uint16
+    description:
+      Maximum value on the Y axis.
+
+  ti,y-min:
+    deprecated: true
+    $ref: /schemas/types.yaml#/definitions/uint16
+    description:
+      Minimum value on the Y axis.
+
+required:
+  - compatible
+  - reg
+
+allOf:
+  - $ref: touchscreen.yaml#
+  - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    spi{
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        touchscreen@0 {
+           compatible = "ti,tsc2046";
+           reg = <0>;	/* CS0 */
+           interrupt-parent = <&gpio1>;
+           interrupts = <8 0>;	/* BOOT6 / GPIO 8 */
+           pendown-gpio = <&gpio1 8 0>;
+           spi-max-frequency = <1000000>;
+           vcc-supply = <&reg_vcc3>;
+           wakeup-source;
+
+           ti,pressure-max = /bits/ 16 <255>;
+           ti,x-max = /bits/ 16 <8000>;
+           ti,x-min = /bits/ 16 <0>;
+           ti,x-plate-ohms = /bits/ 16 <40>;
+           ti,y-max = /bits/ 16 <4800>;
+           ti,y-min = /bits/ 16 <0>;
+       };
+    };
+
diff --git a/Documentation/devicetree/bindings/power/wakeup-source.txt b/Documentation/devicetree/bindings/power/wakeup-source.txt
index a6c8978964aa1..9a4f8310eb67d 100644
--- a/Documentation/devicetree/bindings/power/wakeup-source.txt
+++ b/Documentation/devicetree/bindings/power/wakeup-source.txt
@@ -25,7 +25,7 @@ List of legacy properties and respective binding document
 2. "has-tpo"			Documentation/devicetree/bindings/rtc/rtc-opal.txt
 3. "linux,wakeup"		Documentation/devicetree/bindings/input/gpio-matrix-keypad.txt
 				Documentation/devicetree/bindings/mfd/tc3589x.txt
-				Documentation/devicetree/bindings/input/touchscreen/ads7846.txt
+				Documentation/devicetree/bindings/input/touchscreen/ti,ads7843.yaml
 4. "linux,keypad-wakeup"	Documentation/devicetree/bindings/input/qcom,pm8xxx-keypad.txt
 5. "linux,input-wakeup"		Documentation/devicetree/bindings/input/samsung,s3c6410-keypad.yaml
 6. "nvidia,wakeup-source"	Documentation/devicetree/bindings/input/nvidia,tegra20-kbc.txt
-- 
2.34.1


^ permalink raw reply related

* [PATCH v4 3/3] selftests/hid: Add HIDIOCREVOKE tests
From: Benjamin Tissoires @ 2024-08-26 15:47 UTC (permalink / raw)
  To: Jiri Kosina, Shuah Khan, Peter Hutterer
  Cc: linux-input, linux-kernel, linux-kselftest, Benjamin Tissoires
In-Reply-To: <20240827-hidraw-revoke-v4-0-88c6795bf867@kernel.org>

Add 4 tests for the new revoke ioctl, for read/write/ioctl and poll.

Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>

---

new in v4
---
 tools/testing/selftests/hid/hidraw.c | 143 +++++++++++++++++++++++++++++++++++
 1 file changed, 143 insertions(+)

diff --git a/tools/testing/selftests/hid/hidraw.c b/tools/testing/selftests/hid/hidraw.c
index f8c933476dcd..669eada8886b 100644
--- a/tools/testing/selftests/hid/hidraw.c
+++ b/tools/testing/selftests/hid/hidraw.c
@@ -19,6 +19,11 @@
 	__typeof__(b) _b = (b); \
 	_a < _b ? _a : _b; })
 
+/* for older kernels */
+#ifndef HIDIOCREVOKE
+#define HIDIOCREVOKE	      _IOW('H', 0x0D, int) /* Revoke device access */
+#endif /* HIDIOCREVOKE */
+
 static unsigned char rdesc[] = {
 	0x06, 0x00, 0xff,	/* Usage Page (Vendor Defined Page 1) */
 	0x09, 0x21,		/* Usage (Vendor Usage 0x21) */
@@ -516,6 +521,144 @@ TEST_F(hidraw, raw_event)
 	ASSERT_EQ(buf[1], 42);
 }
 
+/*
+ * After initial opening/checks of hidraw, revoke the hidraw
+ * node and check that we can not read any more data.
+ */
+TEST_F(hidraw, raw_event_revoked)
+{
+	__u8 buf[10] = {0};
+	int err;
+
+	/* inject one event */
+	buf[0] = 1;
+	buf[1] = 42;
+	uhid_send_event(_metadata, self->uhid_fd, buf, 6);
+
+	/* read the data from hidraw */
+	memset(buf, 0, sizeof(buf));
+	err = read(self->hidraw_fd, buf, sizeof(buf));
+	ASSERT_EQ(err, 6) TH_LOG("read_hidraw");
+	ASSERT_EQ(buf[0], 1);
+	ASSERT_EQ(buf[1], 42);
+
+	/* call the revoke ioctl */
+	err = ioctl(self->hidraw_fd, HIDIOCREVOKE, NULL);
+	ASSERT_OK(err) TH_LOG("couldn't revoke the hidraw fd");
+
+	/* inject one other event */
+	buf[0] = 1;
+	buf[1] = 43;
+	uhid_send_event(_metadata, self->uhid_fd, buf, 6);
+
+	/* read the data from hidraw */
+	memset(buf, 0, sizeof(buf));
+	err = read(self->hidraw_fd, buf, sizeof(buf));
+	ASSERT_EQ(err, -1) TH_LOG("read_hidraw");
+}
+
+/*
+ * Revoke the hidraw node and check that we can not do any ioctl.
+ */
+TEST_F(hidraw, ioctl_revoked)
+{
+	int err, desc_size = 0;
+
+	/* call the revoke ioctl */
+	err = ioctl(self->hidraw_fd, HIDIOCREVOKE, NULL);
+	ASSERT_OK(err) TH_LOG("couldn't revoke the hidraw fd");
+
+	/* do an ioctl */
+	err = ioctl(self->hidraw_fd, HIDIOCGRDESCSIZE, &desc_size);
+	ASSERT_EQ(err, -1) TH_LOG("read_hidraw");
+}
+
+/*
+ * Setup polling of the fd, and check that revoke works properly.
+ */
+TEST_F(hidraw, poll_revoked)
+{
+	struct pollfd pfds[1];
+	__u8 buf[10] = {0};
+	int err, ready;
+
+	/* setup polling */
+	pfds[0].fd = self->hidraw_fd;
+	pfds[0].events = POLLIN;
+
+	/* inject one event */
+	buf[0] = 1;
+	buf[1] = 42;
+	uhid_send_event(_metadata, self->uhid_fd, buf, 6);
+
+	while (true) {
+		ready = poll(pfds, 1, 5000);
+		ASSERT_EQ(ready, 1) TH_LOG("poll return value");
+
+		if (pfds[0].revents & POLLIN) {
+			memset(buf, 0, sizeof(buf));
+			err = read(self->hidraw_fd, buf, sizeof(buf));
+			ASSERT_EQ(err, 6) TH_LOG("read_hidraw");
+			ASSERT_EQ(buf[0], 1);
+			ASSERT_EQ(buf[1], 42);
+
+			/* call the revoke ioctl */
+			err = ioctl(self->hidraw_fd, HIDIOCREVOKE, NULL);
+			ASSERT_OK(err) TH_LOG("couldn't revoke the hidraw fd");
+		} else {
+			break;
+		}
+	}
+
+	ASSERT_TRUE(pfds[0].revents & POLLHUP);
+}
+
+/*
+ * After initial opening/checks of hidraw, revoke the hidraw
+ * node and check that we can not read any more data.
+ */
+TEST_F(hidraw, write_event_revoked)
+{
+	struct timespec time_to_wait;
+	__u8 buf[10] = {0};
+	int err;
+
+	/* inject one event from hidraw */
+	buf[0] = 1; /* report ID */
+	buf[1] = 2;
+	buf[2] = 42;
+
+	pthread_mutex_lock(&uhid_output_mtx);
+
+	memset(output_report, 0, sizeof(output_report));
+	clock_gettime(CLOCK_REALTIME, &time_to_wait);
+	time_to_wait.tv_sec += 2;
+
+	err = write(self->hidraw_fd, buf, 3);
+	ASSERT_EQ(err, 3) TH_LOG("unexpected error while writing to hidraw node: %d", err);
+
+	err = pthread_cond_timedwait(&uhid_output_cond, &uhid_output_mtx, &time_to_wait);
+	ASSERT_OK(err) TH_LOG("error while calling waiting for the condition");
+
+	ASSERT_EQ(output_report[0], 1);
+	ASSERT_EQ(output_report[1], 2);
+	ASSERT_EQ(output_report[2], 42);
+
+	/* call the revoke ioctl */
+	err = ioctl(self->hidraw_fd, HIDIOCREVOKE, NULL);
+	ASSERT_OK(err) TH_LOG("couldn't revoke the hidraw fd");
+
+	/* inject one other event */
+	buf[0] = 1;
+	buf[1] = 43;
+	err = write(self->hidraw_fd, buf, 3);
+	ASSERT_LT(err, 0) TH_LOG("unexpected success while writing to hidraw node: %d", err);
+	ASSERT_EQ(errno, 19) TH_LOG("unexpected error code while writing to hidraw node: %d",
+				    errno);
+
+	pthread_mutex_unlock(&uhid_output_mtx);
+}
+
 int main(int argc, char **argv)
 {
 	return test_harness_run(argc, argv);

-- 
2.46.0


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox