Linux Input/HID development
 help / color / mirror / Atom feed
* Re: [PATCH v9] leds: USB: HID: Add support for MSI GT683R led panels
From: Johan Hovold @ 2014-06-18 18:46 UTC (permalink / raw)
  To: Janne Kanniainen
  Cc: Johan Hovold, Jiri Kosina, Bryan Wu, linux-kernel, linux-leds,
	linux-usb, linux-input
In-Reply-To: <CAEEjW05Hv_9ABMG+KnGmT9tUmM4McTMyFQDFkq6ad5iLmF2LaQ@mail.gmail.com>

On Wed, Jun 18, 2014 at 09:41:35PM +0300, Janne Kanniainen wrote:
> >> This driver adds support for USB controlled led panels that exists in
> >> MSI GT683R laptop
> >>
> >> Signed-off-by: Janne Kanniainen <janne.kanniainen@gmail.com>
> >
> > Reviewed-by: Johan Hovold <johan@kernel.org>
> >
> > Thanks, Janne!
> >
> > Johan
> 
> Thank you for reviewing my patch :) I sure learnt a
> lot from you and if I ever send patch in future, it is for sure much
> easier (Maybe I get it trough in 8th time ;))

My pleasure. It's indeed a very good first patch you have created.

Good luck with the next one! ;)

Johan

^ permalink raw reply

* Re: [PATCH v9] leds: USB: HID: Add support for MSI GT683R led panels
From: Jiri Kosina @ 2014-06-18 22:10 UTC (permalink / raw)
  To: Johan Hovold
  Cc: Janne Kanniainen, cooloney, linux-kernel, linux-leds, linux-usb,
	linux-input
In-Reply-To: <20140618161150.GH31163@localhost>

On Wed, 18 Jun 2014, Johan Hovold wrote:

> > This driver adds support for USB controlled led panels that exists in
> > MSI GT683R laptop
> > 
> > Signed-off-by: Janne Kanniainen <janne.kanniainen@gmail.com>
> 
> Reviewed-by: Johan Hovold <johan@kernel.org>
> 
> Thanks, Janne!

Now applied. Thanks Janne for the driver, and thanks a lot Johan for a 
very careful review, I really appreciate it.

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply

* [PATCH] GPIO button wth wakeup attribute is supposed to wake the system up
From: Li, Aubrey @ 2014-06-19  0:51 UTC (permalink / raw)
  To: dmitry.torokhov@gmail.com, Rafael J. Wysocki,
	linux-input@vger.kernel.org, LKML

When the wakeup attribute is set, the GPIO button is capable of
waking up the system from sleep states, including the "freeze"
sleep state.  For that to work, its driver needs to pass the
IRQF_NO_SUSPEND flag to devm_request_any_context_irq(), or the
interrupt will be disabled by suspend_device_irqs() and the
system won't be woken up by it from the "freeze" sleep state.

The suspend_device_irqs() routine is a workaround for drivers
that mishandle interrupts triggered when the devices handled
by them are suspended, so it is safe to use IRQF_NO_SUSPEND in
all drivers that don't have that problem.

The affected/tested machines include Dell Venue 11 Pro and Asus T100TA.

Signed-off-by: Aubrey Li <aubrey.li@linux.intel.com>
Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/input/keyboard/gpio_keys.c |    8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
index 8c98e97..b046dc6 100644
--- a/drivers/input/keyboard/gpio_keys.c
+++ b/drivers/input/keyboard/gpio_keys.c
@@ -527,6 +527,14 @@ static int gpio_keys_setup_key(struct platform_device *pdev,
 	if (!button->can_disable)
 		irqflags |= IRQF_SHARED;
 
+	/*
+	 * If platform has specified the wakeup attribute of the button,
+	 * we add IRQF_NO_SUSPEND to desc->action->flag. So the wake up
+	 * interrupt will not be disabled in suspend_device_irqs
+	 */
+	if (button->wakeup)
+		irqflags |= IRQF_NO_SUSPEND;
+
 	error = devm_request_any_context_irq(&pdev->dev, bdata->irq,
 					     isr, irqflags, desc, bdata);
 	if (error < 0) {
-- 
1.7.10.4

^ permalink raw reply related

* [Asus X550LD] Touchpad not working/properly detected
From: Jessica Groben @ 2014-06-19  8:10 UTC (permalink / raw)
  To: linux-input
In-Reply-To: <53A19B5F.3020604@gmx.de>



[1.] Summary:

[Asus X550LD] Touchpad not working/detected

[2.] Description:

Touchpad is detected as "PS/2 Logitech Wheel Mouse". The pointer is
moving itself every three seconds a cm to the left side of the screen.
Can't make anything with it. No moving of cursor, no clicking.

The newest kernel (v3.16-rc1-utopic) isn't even able to start. Getting a
neverending error:
"psmouse serio4 : Failed to enable mouse on isa0060/serio4"


[3.] Keywords:


[4.] Kernel version:

Linux version 3.15.1-031501-generic (apw@gomeisa) (gcc version 4.6.3
(Ubuntu/Linaro 4.6.3-1ubuntu5) ) #201406161841 SMP Mon Jun 16 22:42:50
UTC 2014


[5.] Output of Oops..

none


[6.] A small shell script...

none


[7.] Environment

Description: Ubuntu 14.04 LTS

Release: 14.04


[7.1.] Software (add the output of the ver_linux script here)


Linux jibba-X550LD 3.15.1-031501-generic #201406161841 SMP Mon Jun 16
22:42:50 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux

Gnu C 4.8

Gnu make 3.81

binutils 2.24

util-linux 2.20.1

mount support

module-init-tools 15

e2fsprogs 1.42.9

pcmciautils 018

PPP 2.4.5

Linux C Library 2.19

Dynamic linker (ldd) 2.19

Procps 3.3.9

Net-tools 1.60

Kbd 1.15.5

Sh-utils 8.21

wireless-tools 30

Modules Loaded ctr ccm nvram rfcomm bnep bluetooth 6lowpan_iphc
binfmt_misc nls_iso8859_1 snd_hda_codec_realtek snd_hda_codec_hdmi
snd_hda_codec_generic uvcvideo intel_rapl x86_pkg_temp_thermal
intel_powerclamp coretemp kvm_intel videobuf2_vmalloc videobuf2_memops
arc4 videobuf2_core kvm ath9k videodev snd_hda_intel snd_soc_rt5640
crct10dif_pclmul crc32_pclmul ghash_clmulni_intel snd_hda_controller
aesni_intel snd_soc_core snd_hda_codec aes_x86_64 lrw gf128mul
ath9k_common snd_compress snd_hwdep snd_pcm ath9k_hw glue_helper
ablk_helper snd_seq_midi cryptd asus_nb_wmi ath snd_seq_midi_event
snd_rawmidi mac80211 nouveau snd_seq i915 asus_wmi sparse_keymap
parport_pc snd_seq_device ppdev snd_timer serio_raw lp mei_me mxm_wmi
cfg80211 ttm drm_kms_helper mei snd drm parport snd_soc_sst_acpi
soundcore video wmi dw_dmac dw_dmac_core i2c_hid 8250_dw i2c_algo_bit
i2c_designware_platform rtsx_pci_ms i2c_designware_core memstick lpc_ich
spi_pxa2xx_platform mac_hid hid_generic usbhid hid rtsx_pci_sdmmc ahci
libahci psmouse r8169 rtsx_pci mii sdhci_acpi sdhci


[7.2.] Processor information (from /proc/cpuinfo):


processor : 0

vendor_id : GenuineIntel

cpu family : 6

model : 69

model name : Intel(R) Core(TM) i3-4010U CPU @ 1.70GHz

stepping : 1

microcode : 0x17

cpu MHz : 1700.000

cache size : 3072 KB

physical id : 0

siblings : 4

core id : 0

cpu cores : 2

apicid : 0

initial apicid : 0

fpu : yes

fpu_exception : yes

cpuid level : 13

wp : yes

flags : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov
pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe syscall nx
pdpe1gb rdtscp lm constant_tsc arch_perfmon pebs bts rep_good nopl
xtopology nonstop_tsc aperfmperf eagerfpu pni pclmulqdq dtes64 monitor
ds_cpl vmx est tm2 ssse3 fma cx16 xtpr pdcm pcid sse4_1 sse4_2 x2apic
movbe popcnt tsc_deadline_timer aes xsave avx f16c rdrand lahf_lm abm
arat epb xsaveopt pln pts dtherm tpr_shadow vnmi flexpriority ept vpid
fsgsbase tsc_adjust bmi1 avx2 smep bmi2 erms invpcid

bogomips : 3392.28

clflush size : 64

cache_alignment : 64

address sizes : 39 bits physical, 48 bits virtual

power management:

processor : 1

vendor_id : GenuineIntel

cpu family : 6

model : 69

model name : Intel(R) Core(TM) i3-4010U CPU @ 1.70GHz

stepping : 1

microcode : 0x17

cpu MHz : 1700.132

cache size : 3072 KB

physical id : 0

siblings : 4

core id : 1

cpu cores : 2

apicid : 2

initial apicid : 2

fpu : yes

fpu_exception : yes

cpuid level : 13

wp : yes

flags : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov
pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe syscall nx
pdpe1gb rdtscp lm constant_tsc arch_perfmon pebs bts rep_good nopl
xtopology nonstop_tsc aperfmperf eagerfpu pni pclmulqdq dtes64 monitor
ds_cpl vmx est tm2 ssse3 fma cx16 xtpr pdcm pcid sse4_1 sse4_2 x2apic
movbe popcnt tsc_deadline_timer aes xsave avx f16c rdrand lahf_lm abm
arat epb xsaveopt pln pts dtherm tpr_shadow vnmi flexpriority ept vpid
fsgsbase tsc_adjust bmi1 avx2 smep bmi2 erms invpcid

bogomips : 3392.28

clflush size : 64

cache_alignment : 64

address sizes : 39 bits physical, 48 bits virtual

power management:

processor : 2

vendor_id : GenuineIntel

cpu family : 6

model : 69

model name : Intel(R) Core(TM) i3-4010U CPU @ 1.70GHz

stepping : 1

microcode : 0x17

cpu MHz : 1700.000

cache size : 3072 KB

physical id : 0

siblings : 4

core id : 0

cpu cores : 2

apicid : 1

initial apicid : 1

fpu : yes

fpu_exception : yes

cpuid level : 13

wp : yes

flags : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov
pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe syscall nx
pdpe1gb rdtscp lm constant_tsc arch_perfmon pebs bts rep_good nopl
xtopology nonstop_tsc aperfmperf eagerfpu pni pclmulqdq dtes64 monitor
ds_cpl vmx est tm2 ssse3 fma cx16 xtpr pdcm pcid sse4_1 sse4_2 x2apic
movbe popcnt tsc_deadline_timer aes xsave avx f16c rdrand lahf_lm abm
arat epb xsaveopt pln pts dtherm tpr_shadow vnmi flexpriority ept vpid
fsgsbase tsc_adjust bmi1 avx2 smep bmi2 erms invpcid

bogomips : 3392.28

clflush size : 64

cache_alignment : 64

address sizes : 39 bits physical, 48 bits virtual

power management:

processor : 3

vendor_id : GenuineIntel

cpu family : 6

model : 69

model name : Intel(R) Core(TM) i3-4010U CPU @ 1.70GHz

stepping : 1

microcode : 0x17

cpu MHz : 1700.132

cache size : 3072 KB

physical id : 0

siblings : 4

core id : 1

cpu cores : 2

apicid : 3

initial apicid : 3

fpu : yes

fpu_exception : yes

cpuid level : 13

wp : yes

flags : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov
pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe syscall nx
pdpe1gb rdtscp lm constant_tsc arch_perfmon pebs bts rep_good nopl
xtopology nonstop_tsc aperfmperf eagerfpu pni pclmulqdq dtes64 monitor
ds_cpl vmx est tm2 ssse3 fma cx16 xtpr pdcm pcid sse4_1 sse4_2 x2apic
movbe popcnt tsc_deadline_timer aes xsave avx f16c rdrand lahf_lm abm
arat epb xsaveopt pln pts dtherm tpr_shadow vnmi flexpriority ept vpid
fsgsbase tsc_adjust bmi1 avx2 smep bmi2 erms invpcid

bogomips : 3392.28

clflush size : 64

cache_alignment : 64

address sizes : 39 bits physical, 48 bits virtual

power management:

[7.3.] Module information (from /proc/modules):


ctr 13193 1 - Live 0x0000000000000000

ccm 17856 1 - Live 0x0000000000000000

nvram 14462 0 - Live 0x0000000000000000

rfcomm 75078 0 - Live 0x0000000000000000

bnep 19884 2 - Live 0x0000000000000000

bluetooth 461775 10 rfcomm,bnep, Live 0x0000000000000000

6lowpan_iphc 18968 1 bluetooth, Live 0x0000000000000000

binfmt_misc 17508 1 - Live 0x0000000000000000

nls_iso8859_1 12713 1 - Live 0x0000000000000000

snd_hda_codec_realtek 72529 1 - Live 0x0000000000000000

snd_hda_codec_hdmi 48229 1 - Live 0x0000000000000000

snd_hda_codec_generic 70087 1 snd_hda_codec_realtek, Live
0x0000000000000000

uvcvideo 82299 0 - Live 0x0000000000000000

intel_rapl 19238 0 - Live 0x0000000000000000

x86_pkg_temp_thermal 14312 0 - Live 0x0000000000000000

intel_powerclamp 19099 0 - Live 0x0000000000000000

coretemp 13638 0 - Live 0x0000000000000000

kvm_intel 148919 0 - Live 0x0000000000000000

videobuf2_vmalloc 13216 1 uvcvideo, Live 0x0000000000000000

videobuf2_memops 13362 1 videobuf2_vmalloc, Live 0x0000000000000000

arc4 12573 2 - Live 0x0000000000000000

videobuf2_core 45435 1 uvcvideo, Live 0x0000000000000000

kvm 463855 1 kvm_intel, Live 0x0000000000000000

ath9k 147545 0 - Live 0x0000000000000000

videodev 149504 2 uvcvideo,videobuf2_core, Live 0x0000000000000000

snd_hda_intel 30608 5 - Live 0x0000000000000000

snd_soc_rt5640 88063 0 - Live 0x0000000000000000

crct10dif_pclmul 14268 0 - Live 0x0000000000000000

crc32_pclmul 13180 0 - Live 0x0000000000000000

ghash_clmulni_intel 13230 0 - Live 0x0000000000000000

snd_hda_controller 35518 1 snd_hda_intel, Live 0x0000000000000000

aesni_intel 152648 2 - Live 0x0000000000000000

snd_soc_core 201495 1 snd_soc_rt5640, Live 0x0000000000000000

snd_hda_codec 144671 5
snd_hda_codec_realtek,snd_hda_codec_hdmi,snd_hda_codec_generic,snd_hda_intel,snd_hda_controller,
Live 0x0000000000000000

aes_x86_64 17131 1 aesni_intel, Live 0x0000000000000000

lrw 13323 1 aesni_intel, Live 0x0000000000000000

gf128mul 14951 1 lrw, Live 0x0000000000000000

ath9k_common 15984 1 ath9k, Live 0x0000000000000000

snd_compress 19435 1 snd_soc_core, Live 0x0000000000000000

snd_hwdep 13613 1 snd_hda_codec, Live 0x0000000000000000

snd_pcm 113863 5
snd_hda_codec_hdmi,snd_hda_intel,snd_hda_controller,snd_soc_core,snd_hda_codec,
Live 0x0000000000000000

ath9k_hw 459576 2 ath9k,ath9k_common, Live 0x0000000000000000

glue_helper 14095 1 aesni_intel, Live 0x0000000000000000

ablk_helper 13597 1 aesni_intel, Live 0x0000000000000000

snd_seq_midi 13564 0 - Live 0x0000000000000000

cryptd 20531 3 ghash_clmulni_intel,aesni_intel,ablk_helper, Live
0x0000000000000000

asus_nb_wmi 16990 0 - Live 0x0000000000000000

ath 29397 3 ath9k,ath9k_common,ath9k_hw, Live 0x0000000000000000

snd_seq_midi_event 14899 1 snd_seq_midi, Live 0x0000000000000000

snd_rawmidi 30865 1 snd_seq_midi, Live 0x0000000000000000

mac80211 663788 1 ath9k, Live 0x0000000000000000

nouveau 1212985 0 - Live 0x0000000000000000

snd_seq 67636 2 snd_seq_midi,snd_seq_midi_event, Live 0x0000000000000000

i915 873414 5 - Live 0x0000000000000000

asus_wmi 24697 1 asus_nb_wmi, Live 0x0000000000000000

sparse_keymap 13890 1 asus_wmi, Live 0x0000000000000000

parport_pc 32906 0 - Live 0x0000000000000000

snd_seq_device 14497 3 snd_seq_midi,snd_rawmidi,snd_seq, Live
0x0000000000000000

ppdev 17711 0 - Live 0x0000000000000000

snd_timer 30118 2 snd_pcm,snd_seq, Live 0x0000000000000000

serio_raw 13483 0 - Live 0x0000000000000000

lp 17799 0 - Live 0x0000000000000000

mei_me 18562 0 - Live 0x0000000000000000

mxm_wmi 13021 1 nouveau, Live 0x0000000000000000

cfg80211 514187 4 ath9k,ath9k_common,ath,mac80211, Live 0x0000000000000000

ttm 90341 1 nouveau, Live 0x0000000000000000

drm_kms_helper 59729 2 nouveau,i915, Live 0x0000000000000000

mei 87522 1 mei_me, Live 0x0000000000000000

snd 74195 23
snd_hda_codec_realtek,snd_hda_codec_hdmi,snd_hda_codec_generic,snd_hda_intel,snd_soc_core,snd_hda_codec,snd_compress,snd_hwdep,snd_pcm,snd_rawmidi,snd_seq,snd_seq_device,snd_timer,
Live 0x0000000000000000

drm 310655 7 nouveau,i915,ttm,drm_kms_helper, Live 0x0000000000000000

parport 42481 3 parport_pc,ppdev,lp, Live 0x0000000000000000

snd_soc_sst_acpi 13007 0 - Live 0x0000000000000000

soundcore 12680 2 snd_hda_codec,snd, Live 0x0000000000000000

video 19932 3 nouveau,i915,asus_wmi, Live 0x0000000000000000

wmi 19379 3 nouveau,asus_wmi,mxm_wmi, Live 0x0000000000000000

dw_dmac 12836 0 - Live 0x0000000000000000

dw_dmac_core 28517 1 dw_dmac, Live 0x0000000000000000

i2c_hid 19233 0 - Live 0x0000000000000000

8250_dw 13431 0 - Live 0x0000000000000000

i2c_algo_bit 13564 2 nouveau,i915, Live 0x0000000000000000

i2c_designware_platform 13025 0 - Live 0x0000000000000000

rtsx_pci_ms 18337 0 - Live 0x0000000000000000

i2c_designware_core 14990 1 i2c_designware_platform, Live
0x0000000000000000

memstick 16968 1 rtsx_pci_ms, Live 0x0000000000000000

lpc_ich 21176 0 - Live 0x0000000000000000

spi_pxa2xx_platform 23302 0 - Live 0x0000000000000000

mac_hid 13275 0 - Live 0x0000000000000000

hid_generic 12559 0 - Live 0x0000000000000000

usbhid 53121 0 - Live 0x0000000000000000

hid 106436 3 i2c_hid,hid_generic,usbhid, Live 0x0000000000000000

rtsx_pci_sdmmc 23482 0 - Live 0x0000000000000000

ahci 30167 4 - Live 0x0000000000000000

libahci 32191 1 ahci, Live 0x0000000000000000

psmouse 97592 0 - Live 0x0000000000000000 (OE)

r8169 73316 0 - Live 0x0000000000000000

rtsx_pci 46393 2 rtsx_pci_ms,rtsx_pci_sdmmc, Live 0x0000000000000000

mii 13981 1 r8169, Live 0x0000000000000000

sdhci_acpi 13395 0 - Live 0x0000000000000000

sdhci 43409 1 sdhci_acpi, Live 0x0000000000000000


[7.4.] Loaded driver and hardware information (/proc/ioports, /proc/iomem)

While booted into the newest upstream mainline kernel only, execute the
following in a terminal, and paste the results:

cat /proc/ioports


0000-0cf7 : PCI Bus 0000:00

0000-001f : dma1

0020-0021 : pic1

0040-0043 : timer0

0050-0053 : timer1

0060-0060 : keyboard

0062-0062 : EC data

0064-0064 : keyboard

0066-0066 : EC cmd

0070-0077 : rtc0

0080-008f : dma page reg

00a0-00a1 : pic2

00c0-00df : dma2

00f0-00ff : fpu

0240-0259 : pnp 00:08

04d0-04d1 : pnp 00:07

0680-069f : pnp 00:04

0cf8-0cff : PCI conf1

0d00-ffff : PCI Bus 0000:00

164e-164f : pnp 00:04

1800-1803 : ACPI PM1a_EVT_BLK

1804-1805 : ACPI PM1a_CNT_BLK

1808-180b : ACPI PM_TMR

1810-1815 : ACPI CPU throttle

1830-1833 : iTCO_wdt

1850-1850 : ACPI PM2_CNT_BLK

1854-1857 : pnp 00:06

1860-187f : iTCO_wdt

1880-189f : ACPI GPE0_BLK

1c00-1cfe : pnp 00:04

1d00-1dfe : pnp 00:04

1e00-1efe : pnp 00:04

1f00-1ffe : pnp 00:04

d000-dfff : PCI Bus 0000:04

d000-d07f : 0000:04:00.0

e000-efff : PCI Bus 0000:02

e000-e0ff : 0000:02:00.1

e000-e0ff : r8169

f000-f03f : 0000:00:02.0

f040-f05f : 0000:00:1f.3

f060-f07f : 0000:00:1f.2

f060-f07f : ahci

f080-f083 : 0000:00:1f.2

f080-f083 : ahci

f090-f097 : 0000:00:1f.2

f090-f097 : ahci

f0a0-f0a3 : 0000:00:1f.2

f0a0-f0a3 : ahci

f0b0-f0b7 : 0000:00:1f.2

f0b0-f0b7 : ahci

ffff-ffff : pnp 00:04

ffff-ffff : pnp 00:04

ffff-ffff : pnp 00:04


cat /proc/iomem


00000000-00000fff : reserved

00001000-00057fff : System RAM

00058000-00058fff : reserved

00059000-0009efff : System RAM

0009f000-0009ffff : reserved

000a0000-000bffff : PCI Bus 0000:00

000c0000-000cebff : Video ROM

000d0000-000d3fff : PCI Bus 0000:00

000d4000-000d7fff : PCI Bus 0000:00

000d8000-000dbfff : PCI Bus 0000:00

000dc000-000dffff : PCI Bus 0000:00

000f0000-000fffff : System ROM

00100000-b9a63fff : System RAM

02000000-02788e58 : Kernel code

02788e59-02d265bf : Kernel data

02e84000-02febfff : Kernel bss

b9a64000-b9a6afff : ACPI Non-volatile Storage

b9a6b000-ba2a1fff : System RAM

ba2a2000-ba507fff : reserved

ba508000-c9910fff : System RAM

c9911000-c9b18fff : reserved

c9b19000-c9e40fff : System RAM

c9e41000-cab4cfff : ACPI Non-volatile Storage

cab4d000-caf57fff : reserved

caf58000-caffefff : reserved

cafff000-caffffff : System RAM

cb000000-cbbfffff : RAM buffer

cbc00000-cfdfffff : reserved

cbe00000-cfdfffff : Graphics Stolen Memory

cfe00000-feafffff : PCI Bus 0000:00

d0000000-dfffffff : 0000:00:02.0

e0000000-f1ffffff : PCI Bus 0000:04

e0000000-efffffff : 0000:04:00.0

f0000000-f1ffffff : 0000:04:00.0

f6000000-f70fffff : PCI Bus 0000:04

f6000000-f6ffffff : 0000:04:00.0

f7000000-f707ffff : 0000:04:00.0

f7400000-f77fffff : 0000:00:02.0

f7800000-f78fffff : PCI Bus 0000:03

f7800000-f787ffff : 0000:03:00.0

f7800000-f787ffff : ath9k

f7880000-f788ffff : 0000:03:00.0

f7900000-f79fffff : PCI Bus 0000:02

f7900000-f790ffff : 0000:02:00.0

f7910000-f7913fff : 0000:02:00.1

f7910000-f7913fff : r8169

f7914000-f7914fff : 0000:02:00.1

f7914000-f7914fff : r8169

f7915000-f7915fff : 0000:02:00.0

f7915000-f7915fff : rtsx_pci

f7a00000-f7a0ffff : 0000:00:14.0

f7a00000-f7a0ffff : xhci_hcd

f7a10000-f7a17fff : 0000:00:04.0

f7a18000-f7a1bfff : 0000:00:1b.0

f7a18000-f7a1bfff : ICH HD audio

f7a1c000-f7a1ffff : 0000:00:03.0

f7a1c000-f7a1ffff : ICH HD audio

f7a20000-f7a20fff : 0000:00:1f.6

f7a21000-f7a210ff : 0000:00:1f.3

f7a22000-f7a227ff : 0000:00:1f.2

f7a22000-f7a227ff : ahci

f7a23000-f7a233ff : 0000:00:1d.0

f7a23000-f7a233ff : ehci_hcd

f7a25000-f7a2501f : 0000:00:16.0

f7a25000-f7a2501f : mei_me

f7fdf000-f7fdffff : pnp 00:0c

f7fe0000-f7feffff : pnp 00:0c

f8000000-fbffffff : PCI MMCONFIG 0000 [bus 00-3f]

f8000000-fbffffff : reserved

f8000000-fbffffff : pnp 00:0c

fec00000-fec00fff : reserved

fec00000-fec003ff : IOAPIC 0

fed00000-fed03fff : reserved

fed00000-fed003ff : HPET 0

fed10000-fed17fff : pnp 00:0c

fed18000-fed18fff : pnp 00:0c

fed19000-fed19fff : pnp 00:0c

fed1c000-fed1ffff : reserved

fed1c000-fed1ffff : pnp 00:0c

fed1f410-fed1f414 : iTCO_wdt

fed20000-fed3ffff : pnp 00:0c

fed40000-fed44fff : pnp 00:00

fed45000-fed8ffff : pnp 00:0c

fed90000-fed90fff : dmar0

fed91000-fed91fff : dmar1

fee00000-fee00fff : Local APIC

fee00000-fee00fff : reserved

ff000000-ffffffff : reserved

ff000000-ffffffff : pnp 00:0c

100000000-12f1fffff : System RAM

12f200000-12fffffff : RAM buffer


[7.5.] PCI information ('lspci -vvv' as root)


L1SubCap: PCI-PM_L1.2+ PCI-PM_L1.1+ ASPM_L1.2+ ASPM_L1.1+ L1_PM_Substates+

PortCommonModeRestoreTime=40us PortTPowerOnTime=10us

Kernel driver in use: pcieport

00:1c.3 PCI bridge: Intel Corporation Lynx Point-LP PCI Express Root
Port 4 (rev e4) (prog-if 00 [Normal decode])

Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr-
Stepping- SERR- FastB2B- DisINTx+

Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort-
<MAbort- >SERR- <PERR- INTx-

Latency: 0, Cache Line Size: 64 bytes

Bus: primary=00, secondary=03, subordinate=03, sec-latency=0

I/O behind bridge: 0000f000-00000fff

Memory behind bridge: f7800000-f78fffff

Prefetchable memory behind bridge: 00000000fff00000-00000000000fffff

Secondary status: 66MHz- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort-
<MAbort+ <SERR- <PERR-

BridgeCtl: Parity- SERR- NoISA- VGA- MAbort- >Reset- FastB2B-

PriDiscTmr- SecDiscTmr- DiscTmrStat- DiscTmrSERREn-

Capabilities: [40] Express (v2) Root Port (Slot+), MSI 00

DevCap: MaxPayload 128 bytes, PhantFunc 0

ExtTag- RBE+

DevCtl: Report errors: Correctable- Non-Fatal- Fatal- Unsupported-

RlxdOrd- ExtTag- PhantFunc- AuxPwr- NoSnoop-

MaxPayload 128 bytes, MaxReadReq 128 bytes

DevSta: CorrErr- UncorrErr- FatalErr- UnsuppReq- AuxPwr+ TransPend-

LnkCap: Port #4, Speed 5GT/s, Width x1, ASPM L0s L1, Exit Latency L0s
<512ns, L1 <16us

ClockPM- Surprise- LLActRep+ BwNot+

LnkCtl: ASPM Disabled; RCB 64 bytes Disabled- CommClk+

ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt-

LnkSta: Speed 2.5GT/s, Width x1, TrErr- Train- SlotClk+ DLActive+
BWMgmt+ ABWMgmt-

SltCap: AttnBtn- PwrCtrl- MRL- AttnInd- PwrInd- HotPlug- Surprise-

Slot #3, PowerLimit 10.000W; Interlock- NoCompl+

SltCtl: Enable: AttnBtn- PwrFlt- MRL- PresDet- CmdCplt- HPIrq- LinkChg-

Control: AttnInd Unknown, PwrInd Unknown, Power- Interlock-

SltSta: Status: AttnBtn- PowerFlt- MRL- CmdCplt- PresDet+ Interlock-

Changed: MRL- PresDet- LinkState-

RootCtl: ErrCorrectable- ErrNon-Fatal- ErrFatal- PMEIntEna+ CRSVisible-

RootCap: CRSVisible-

RootSta: PME ReqID 0000, PMEStatus- PMEPending-

DevCap2: Completion Timeout: Range ABC, TimeoutDis+, LTR+, OBFF Not
Supported ARIFwd-

DevCtl2: Completion Timeout: 50us to 50ms, TimeoutDis-, LTR+, OBFF
Disabled ARIFwd-

LnkCtl2: Target Link Speed: 5GT/s, EnterCompliance- SpeedDis-

Transmit Margin: Normal Operating Range, EnterModifiedCompliance-
ComplianceSOS-

Compliance De-emphasis: -6dB

LnkSta2: Current De-emphasis Level: -3.5dB, EqualizationComplete-,
EqualizationPhase1-

EqualizationPhase2-, EqualizationPhase3-, LinkEqualizationRequest-

Capabilities: [80] MSI: Enable+ Count=1/1 Maskable- 64bit-

Address: fee00318 Data: 0000

Capabilities: [90] Subsystem: ASUSTeK Computer Inc. Device 131d

Capabilities: [a0] Power Management version 3

Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA PME(D0+,D1-,D2-,D3hot+,D3cold+)

Status: D0 NoSoftRst- PME-Enable- DSel=0 DScale=0 PME-

Capabilities: [100 v0] #00

Capabilities: [200 v1] L1 PM Substates

L1SubCap: PCI-PM_L1.2+ PCI-PM_L1.1+ ASPM_L1.2+ ASPM_L1.1+ L1_PM_Substates+

PortCommonModeRestoreTime=40us PortTPowerOnTime=10us

Kernel driver in use: pcieport

00:1c.4 PCI bridge: Intel Corporation Lynx Point-LP PCI Express Root
Port 5 (rev e4) (prog-if 00 [Normal decode])

Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr-
Stepping- SERR- FastB2B- DisINTx+

Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort-
<MAbort- >SERR- <PERR- INTx-

Latency: 0, Cache Line Size: 64 bytes

Bus: primary=00, secondary=04, subordinate=04, sec-latency=0

I/O behind bridge: 0000d000-0000dfff

Memory behind bridge: f6000000-f70fffff

Prefetchable memory behind bridge: 00000000e0000000-00000000f1ffffff

Secondary status: 66MHz- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort-
<MAbort+ <SERR- <PERR-

BridgeCtl: Parity- SERR- NoISA- VGA- MAbort- >Reset- FastB2B-

PriDiscTmr- SecDiscTmr- DiscTmrStat- DiscTmrSERREn-

Capabilities: [40] Express (v2) Root Port (Slot+), MSI 00

DevCap: MaxPayload 128 bytes, PhantFunc 0

ExtTag- RBE+

DevCtl: Report errors: Correctable- Non-Fatal- Fatal- Unsupported-

RlxdOrd- ExtTag- PhantFunc- AuxPwr- NoSnoop-

MaxPayload 128 bytes, MaxReadReq 128 bytes

DevSta: CorrErr- UncorrErr- FatalErr- UnsuppReq- AuxPwr+ TransPend-

LnkCap: Port #5, Speed 5GT/s, Width x4, ASPM L0s L1, Exit Latency L0s
<512ns, L1 <16us

ClockPM- Surprise- LLActRep+ BwNot+

LnkCtl: ASPM Disabled; RCB 64 bytes Disabled- CommClk+

ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt-

LnkSta: Speed 2.5GT/s, Width x4, TrErr- Train- SlotClk+ DLActive+
BWMgmt- ABWMgmt-

SltCap: AttnBtn- PwrCtrl- MRL- AttnInd- PwrInd- HotPlug- Surprise-

Slot #4, PowerLimit 25.000W; Interlock- NoCompl+

SltCtl: Enable: AttnBtn- PwrFlt- MRL- PresDet- CmdCplt- HPIrq- LinkChg-

Control: AttnInd Unknown, PwrInd Unknown, Power- Interlock-

SltSta: Status: AttnBtn- PowerFlt- MRL- CmdCplt- PresDet+ Interlock-

Changed: MRL- PresDet+ LinkState+

RootCtl: ErrCorrectable- ErrNon-Fatal- ErrFatal- PMEIntEna+ CRSVisible-

RootCap: CRSVisible-

RootSta: PME ReqID 0000, PMEStatus- PMEPending-

DevCap2: Completion Timeout: Range ABC, TimeoutDis+, LTR+, OBFF Not
Supported ARIFwd-

DevCtl2: Completion Timeout: 50us to 50ms, TimeoutDis-, LTR-, OBFF
Disabled ARIFwd-

LnkCtl2: Target Link Speed: 5GT/s, EnterCompliance- SpeedDis-

Transmit Margin: Normal Operating Range, EnterModifiedCompliance-
ComplianceSOS-

Compliance De-emphasis: -6dB

LnkSta2: Current De-emphasis Level: -3.5dB, EqualizationComplete-,
EqualizationPhase1-

EqualizationPhase2-, EqualizationPhase3-, LinkEqualizationRequest-

Capabilities: [80] MSI: Enable+ Count=1/1 Maskable- 64bit-

Address: fee00338 Data: 0000

Capabilities: [90] Subsystem: ASUSTeK Computer Inc. Device 131d

Capabilities: [a0] Power Management version 3

Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA PME(D0+,D1-,D2-,D3hot+,D3cold+)

Status: D0 NoSoftRst- PME-Enable- DSel=0 DScale=0 PME-

Capabilities: [100 v0] #00

Capabilities: [200 v1] L1 PM Substates

L1SubCap: PCI-PM_L1.2+ PCI-PM_L1.1+ ASPM_L1.2+ ASPM_L1.1+ L1_PM_Substates+

PortCommonModeRestoreTime=40us PortTPowerOnTime=10us

Kernel driver in use: pcieport

00:1d.0 USB controller: Intel Corporation Lynx Point-LP USB EHCI #1 (rev
04) (prog-if 20 [EHCI])

Subsystem: ASUSTeK Computer Inc. Device 201f

Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr-
Stepping- SERR- FastB2B- DisINTx-

Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort-
<TAbort- <MAbort- >SERR- <PERR- INTx-

Latency: 0

Interrupt: pin A routed to IRQ 23

Region 0: Memory at f7a23000 (32-bit, non-prefetchable) [size=1K]

Capabilities: [50] Power Management version 3

Flags: PMEClk- DSI- D1- D2- AuxCurrent=375mA
PME(D0+,D1-,D2-,D3hot+,D3cold+)

Status: D0 NoSoftRst+ PME-Enable- DSel=0 DScale=0 PME-

Capabilities: [58] Debug port: BAR=1 offset=00a0

Capabilities: [98] PCI Advanced Features

AFCap: TP+ FLR+

AFCtrl: FLR-

AFStatus: TP-

Kernel driver in use: ehci-pci

00:1f.0 ISA bridge: Intel Corporation Lynx Point-LP LPC Controller (rev 04)

Subsystem: ASUSTeK Computer Inc. Device 131d

Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr-
Stepping- SERR- FastB2B- DisINTx-

Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=medium >TAbort-
<TAbort- <MAbort- >SERR- <PERR- INTx-

Latency: 0

Capabilities: [e0] Vendor Specific Information: Len=0c <?>

Kernel driver in use: lpc_ich

00:1f.2 SATA controller: Intel Corporation Lynx Point-LP SATA Controller
1 [AHCI mode] (rev 04) (prog-if 01 [AHCI 1.0])

Subsystem: ASUSTeK Computer Inc. Device 131d

Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr-
Stepping- SERR- FastB2B- DisINTx+

Status: Cap+ 66MHz+ UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort-
<TAbort- <MAbort- >SERR- <PERR- INTx-

Latency: 0

Interrupt: pin B routed to IRQ 64

Region 0: I/O ports at f0b0 [size=8]

Region 1: I/O ports at f0a0 [size=4]

Region 2: I/O ports at f090 [size=8]

Region 3: I/O ports at f080 [size=4]

Region 4: I/O ports at f060 [size=32]

Region 5: Memory at f7a22000 (32-bit, non-prefetchable) [size=2K]

Capabilities: [80] MSI: Enable+ Count=1/1 Maskable- 64bit-

Address: fee00398 Data: 0000

Capabilities: [70] Power Management version 3

Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA PME(D0-,D1-,D2-,D3hot+,D3cold-)

Status: D0 NoSoftRst+ PME-Enable- DSel=0 DScale=0 PME-

Capabilities: [a8] SATA HBA v1.0 BAR4 Offset=00000004

Kernel driver in use: ahci

00:1f.3 SMBus: Intel Corporation Lynx Point-LP SMBus Controller (rev 04)

Subsystem: ASUSTeK Computer Inc. Device 131d

Control: I/O+ Mem+ BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr-
Stepping- SERR- FastB2B- DisINTx-

Status: Cap- 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort-
<TAbort- <MAbort- >SERR- <PERR- INTx-

Interrupt: pin C routed to IRQ 3

Region 0: Memory at f7a21000 (64-bit, non-prefetchable) [size=256]

Region 4: I/O ports at f040 [size=32]

00:1f.6 Signal processing controller: Intel Corporation Lynx Point-LP
Thermal (rev 04)

Subsystem: ASUSTeK Computer Inc. Device 131d

Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr-
Stepping- SERR- FastB2B- DisINTx-

Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort-
<MAbort- >SERR- <PERR- INTx-

Latency: 0

Interrupt: pin C routed to IRQ 3

Region 0: Memory at f7a20000 (64-bit, non-prefetchable) [size=4K]

Capabilities: [50] Power Management version 3

Flags: PMEClk- DSI+ D1- D2- AuxCurrent=0mA PME(D0-,D1-,D2-,D3hot-,D3cold-)

Status: D0 NoSoftRst+ PME-Enable- DSel=0 DScale=0 PME-

Capabilities: [80] MSI: Enable- Count=1/1 Maskable- 64bit-

Address: 00000000 Data: 0000

02:00.0 Unassigned class [ff00]: Realtek Semiconductor Co., Ltd. Device
5287 (rev 01)

Subsystem: ASUSTeK Computer Inc. Device 202f

Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr-
Stepping- SERR- FastB2B- DisINTx+

Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort-
<MAbort- >SERR- <PERR- INTx-

Latency: 0, Cache Line Size: 64 bytes

Interrupt: pin B routed to IRQ 63

Region 0: Memory at f7915000 (32-bit, non-prefetchable) [size=4K]

Expansion ROM at f7900000 [disabled] [size=64K]

Capabilities: [40] Power Management version 3

Flags: PMEClk- DSI- D1+ D2+ AuxCurrent=375mA
PME(D0-,D1+,D2+,D3hot+,D3cold+)

Status: D0 NoSoftRst- PME-Enable- DSel=0 DScale=0 PME-

Capabilities: [50] MSI: Enable+ Count=1/1 Maskable- 64bit+

Address: 00000000fee00378 Data: 0000

Capabilities: [70] Express (v2) Endpoint, MSI 00

DevCap: MaxPayload 128 bytes, PhantFunc 0, Latency L0s <512ns, L1 <64us

ExtTag- AttnBtn- AttnInd- PwrInd- RBE+ FLReset-

DevCtl: Report errors: Correctable- Non-Fatal- Fatal- Unsupported-

RlxdOrd- ExtTag- PhantFunc- AuxPwr- NoSnoop-

MaxPayload 128 bytes, MaxReadReq 512 bytes

DevSta: CorrErr+ UncorrErr- FatalErr- UnsuppReq+ AuxPwr+ TransPend-

LnkCap: Port #0, Speed 2.5GT/s, Width x1, ASPM L0s L1, Exit Latency L0s
unlimited, L1 <64us

ClockPM+ Surprise- LLActRep- BwNot-

LnkCtl: ASPM L1 Enabled; RCB 64 bytes Disabled- CommClk-

ExtSynch- ClockPM+ AutWidDis- BWInt- AutBWInt-

LnkSta: Speed 2.5GT/s, Width x1, TrErr- Train- SlotClk+ DLActive-
BWMgmt- ABWMgmt-

DevCap2: Completion Timeout: Range ABCD, TimeoutDis+, LTR+, OBFF Via
message/WAKE#

DevCtl2: Completion Timeout: 50us to 50ms, TimeoutDis-, LTR+, OBFF Disabled

LnkCtl2: Target Link Speed: 2.5GT/s, EnterCompliance- SpeedDis-

Transmit Margin: Normal Operating Range, EnterModifiedCompliance-
ComplianceSOS-

Compliance De-emphasis: -6dB

LnkSta2: Current De-emphasis Level: -6dB, EqualizationComplete-,
EqualizationPhase1-

EqualizationPhase2-, EqualizationPhase3-, LinkEqualizationRequest-

Capabilities: [b0] MSI-X: Enable- Count=1 Masked-

Vector table: BAR=0 offset=00000000

PBA: BAR=0 offset=00000000

Capabilities: [d0] Vital Product Data

pcilib: sysfs_read_vpd: read failed: Connection timed out

Not readable

Capabilities: [100 v2] Advanced Error Reporting

UESta: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP-
ECRC- UnsupReq- ACSViol-

UEMsk: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP-
ECRC- UnsupReq- ACSViol-

UESvrt: DLP+ SDES+ TLP- FCP+ CmpltTO- CmpltAbrt- UnxCmplt- RxOF+
MalfTLP+ ECRC- UnsupReq- ACSViol-

CESta: RxErr- BadTLP- BadDLLP- Rollover- Timeout- NonFatalErr+

CEMsk: RxErr- BadTLP- BadDLLP- Rollover- Timeout- NonFatalErr+

AERCap: First Error Pointer: 00, GenCap+ CGenEn- ChkCap+ ChkEn-

Capabilities: [140 v1] Virtual Channel

Caps: LPEVC=0 RefClk=100ns PATEntryBits=1

Arb: Fixed- WRR32- WRR64- WRR128-

Ctrl: ArbSelect=Fixed

Status: InProgress-

VC0: Caps: PATOffset=00 MaxTimeSlots=1 RejSnoopTrans-

Arb: Fixed- WRR32- WRR64- WRR128- TWRR128- WRR256-

Ctrl: Enable+ ID=0 ArbSelect=Fixed TC/VC=01

Status: NegoPending- InProgress-

Capabilities: [160 v1] Device Serial Number 00-00-00-00-00-00-00-00

Capabilities: [170 v1] Latency Tolerance Reporting

Max snoop latency: 3145728ns

Max no snoop latency: 3145728ns

Capabilities: [178 v1] L1 PM Substates

L1SubCap: PCI-PM_L1.2+ PCI-PM_L1.1+ ASPM_L1.2+ ASPM_L1.1+ L1_PM_Substates+

PortCommonModeRestoreTime=150us PortTPowerOnTime=150us

Kernel driver in use: rtsx_pci

02:00.1 Ethernet controller: Realtek Semiconductor Co., Ltd.
RTL8111/8168/8411 PCI Express Gigabit Ethernet Controller (rev 12)

Subsystem: ASUSTeK Computer Inc. Device 200f

Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr-
Stepping- SERR- FastB2B- DisINTx+

Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort-
<MAbort- >SERR- <PERR- INTx-

Latency: 0, Cache Line Size: 64 bytes

Interrupt: pin A routed to IRQ 65

Region 0: I/O ports at e000 [size=256]

Region 2: Memory at f7914000 (64-bit, non-prefetchable) [size=4K]

Region 4: Memory at f7910000 (64-bit, non-prefetchable) [size=16K]

Capabilities: [40] Power Management version 3

Flags: PMEClk- DSI- D1+ D2+ AuxCurrent=375mA
PME(D0+,D1+,D2+,D3hot+,D3cold+)

Status: D0 NoSoftRst+ PME-Enable- DSel=0 DScale=0 PME-

Capabilities: [50] MSI: Enable+ Count=1/1 Maskable- 64bit+

Address: 00000000fee003b8 Data: 0000

Capabilities: [70] Express (v2) Endpoint, MSI 01

DevCap: MaxPayload 128 bytes, PhantFunc 0, Latency L0s <512ns, L1 <64us

ExtTag- AttnBtn- AttnInd- PwrInd- RBE+ FLReset-

DevCtl: Report errors: Correctable- Non-Fatal- Fatal- Unsupported-

RlxdOrd- ExtTag- PhantFunc- AuxPwr- NoSnoop-

MaxPayload 128 bytes, MaxReadReq 4096 bytes

DevSta: CorrErr+ UncorrErr- FatalErr- UnsuppReq+ AuxPwr+ TransPend-

LnkCap: Port #0, Speed 2.5GT/s, Width x1, ASPM L0s L1, Exit Latency L0s
unlimited, L1 <64us

ClockPM+ Surprise- LLActRep- BwNot-

LnkCtl: ASPM Disabled; RCB 64 bytes Disabled- CommClk+

ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt-

LnkSta: Speed 2.5GT/s, Width x1, TrErr- Train- SlotClk+ DLActive-
BWMgmt- ABWMgmt-

DevCap2: Completion Timeout: Range ABCD, TimeoutDis+, LTR+, OBFF Via
message/WAKE#

DevCtl2: Completion Timeout: 50us to 50ms, TimeoutDis-, LTR-, OBFF Disabled

LnkSta2: Current De-emphasis Level: -6dB, EqualizationComplete-,
EqualizationPhase1-

EqualizationPhase2-, EqualizationPhase3-, LinkEqualizationRequest-

Capabilities: [b0] MSI-X: Enable- Count=4 Masked-

Vector table: BAR=4 offset=00000000

PBA: BAR=4 offset=00000800

Capabilities: [d0] Vital Product Data

Unknown small resource type 00, will not decode more.

Capabilities: [100 v2] Advanced Error Reporting

UESta: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP-
ECRC- UnsupReq- ACSViol-

UEMsk: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP-
ECRC- UnsupReq- ACSViol-

UESvrt: DLP+ SDES+ TLP- FCP+ CmpltTO- CmpltAbrt- UnxCmplt- RxOF+
MalfTLP+ ECRC- UnsupReq- ACSViol-

CESta: RxErr- BadTLP- BadDLLP- Rollover- Timeout- NonFatalErr+

CEMsk: RxErr- BadTLP- BadDLLP- Rollover- Timeout- NonFatalErr+

AERCap: First Error Pointer: 00, GenCap+ CGenEn- ChkCap+ ChkEn-

Capabilities: [160 v1] Device Serial Number 17-31-71-11-68-4c-e0-00

Capabilities: [170 v1] Latency Tolerance Reporting

Max snoop latency: 3145728ns

Max no snoop latency: 3145728ns

Capabilities: [178 v1] L1 PM Substates

L1SubCap: PCI-PM_L1.2+ PCI-PM_L1.1+ ASPM_L1.2+ ASPM_L1.1+ L1_PM_Substates+

PortCommonModeRestoreTime=150us PortTPowerOnTime=150us

Kernel driver in use: r8169

03:00.0 Network controller: Qualcomm Atheros AR9485 Wireless Network
Adapter (rev 01)

Subsystem: AzureWave Device 1186

Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr-
Stepping- SERR- FastB2B- DisINTx-

Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort-
<MAbort- >SERR- <PERR- INTx-

Latency: 0, Cache Line Size: 64 bytes

Interrupt: pin A routed to IRQ 19

Region 0: Memory at f7800000 (64-bit, non-prefetchable) [size=512K]

Expansion ROM at f7880000 [disabled] [size=64K]

Capabilities: [40] Power Management version 2

Flags: PMEClk- DSI- D1+ D2+ AuxCurrent=375mA
PME(D0+,D1+,D2+,D3hot+,D3cold+)

Status: D0 NoSoftRst- PME-Enable- DSel=0 DScale=0 PME-

Capabilities: [50] MSI: Enable- Count=1/4 Maskable+ 64bit+

Address: 0000000000000000 Data: 0000

Masking: 00000000 Pending: 00000000

Capabilities: [70] Express (v2) Endpoint, MSI 00

DevCap: MaxPayload 128 bytes, PhantFunc 0, Latency L0s unlimited, L1 <64us

ExtTag- AttnBtn- AttnInd- PwrInd- RBE+ FLReset-

DevCtl: Report errors: Correctable- Non-Fatal- Fatal- Unsupported-

RlxdOrd- ExtTag- PhantFunc- AuxPwr- NoSnoop-

MaxPayload 128 bytes, MaxReadReq 512 bytes

DevSta: CorrErr+ UncorrErr- FatalErr- UnsuppReq+ AuxPwr+ TransPend-

LnkCap: Port #0, Speed 2.5GT/s, Width x1, ASPM L0s L1, Exit Latency L0s
<4us, L1 <64us

ClockPM- Surprise- LLActRep- BwNot-

LnkCtl: ASPM Disabled; RCB 64 bytes Disabled- CommClk+

ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt-

LnkSta: Speed 2.5GT/s, Width x1, TrErr- Train- SlotClk+ DLActive-
BWMgmt- ABWMgmt-

DevCap2: Completion Timeout: Not Supported, TimeoutDis+, LTR-, OBFF Not
Supported

DevCtl2: Completion Timeout: 50us to 50ms, TimeoutDis-, LTR-, OBFF Disabled

LnkCtl2: Target Link Speed: 2.5GT/s, EnterCompliance- SpeedDis-

Transmit Margin: Normal Operating Range, EnterModifiedCompliance-
ComplianceSOS-

Compliance De-emphasis: -6dB

LnkSta2: Current De-emphasis Level: -6dB, EqualizationComplete-,
EqualizationPhase1-

EqualizationPhase2-, EqualizationPhase3-, LinkEqualizationRequest-

Capabilities: [100 v1] Advanced Error Reporting

UESta: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP-
ECRC- UnsupReq- ACSViol-

UEMsk: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP-
ECRC- UnsupReq- ACSViol-

UESvrt: DLP+ SDES+ TLP- FCP+ CmpltTO- CmpltAbrt- UnxCmplt- RxOF+
MalfTLP+ ECRC- UnsupReq- ACSViol-

CESta: RxErr- BadTLP- BadDLLP- Rollover- Timeout- NonFatalErr+

CEMsk: RxErr- BadTLP- BadDLLP- Rollover- Timeout- NonFatalErr+

AERCap: First Error Pointer: 00, GenCap- CGenEn- ChkCap- ChkEn-

Capabilities: [140 v1] Virtual Channel

Caps: LPEVC=0 RefClk=100ns PATEntryBits=1

Arb: Fixed- WRR32- WRR64- WRR128-

Ctrl: ArbSelect=Fixed

Status: InProgress-

VC0: Caps: PATOffset=00 MaxTimeSlots=1 RejSnoopTrans-

Arb: Fixed- WRR32- WRR64- WRR128- TWRR128- WRR256-

Ctrl: Enable+ ID=0 ArbSelect=Fixed TC/VC=01

Status: NegoPending- InProgress-

Capabilities: [160 v1] Device Serial Number 00-00-00-00-00-00-00-00

Kernel driver in use: ath9k

04:00.0 3D controller: NVIDIA Corporation GF117M [GeForce 610M/710M/820M
/ GT 620M/625M/630M/720M] (rev a1)

Subsystem: ASUSTeK Computer Inc. GeForce 820M

Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr-
Stepping- SERR- FastB2B- DisINTx+

Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort-
<MAbort- >SERR- <PERR- INTx-

Latency: 0, Cache Line Size: 64 bytes

Interrupt: pin A routed to IRQ 70

Region 0: Memory at f6000000 (32-bit, non-prefetchable) [size=16M]

Region 1: Memory at e0000000 (64-bit, prefetchable) [size=256M]

Region 3: Memory at f0000000 (64-bit, prefetchable) [size=32M]

Region 5: I/O ports at d000 [size=128]

Expansion ROM at f7000000 [disabled] [size=512K]

Capabilities: [60] Power Management version 3

Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA PME(D0-,D1-,D2-,D3hot-,D3cold-)

Status: D0 NoSoftRst+ PME-Enable- DSel=0 DScale=0 PME-

Capabilities: [68] MSI: Enable+ Count=1/1 Maskable- 64bit+

Address: 00000000fee00458 Data: 0000

Capabilities: [78] Express (v2) Endpoint, MSI 00

DevCap: MaxPayload 256 bytes, PhantFunc 0, Latency L0s unlimited, L1 <64us

ExtTag+ AttnBtn- AttnInd- PwrInd- RBE+ FLReset-

DevCtl: Report errors: Correctable- Non-Fatal- Fatal- Unsupported-

RlxdOrd- ExtTag- PhantFunc- AuxPwr- NoSnoop+

MaxPayload 128 bytes, MaxReadReq 512 bytes

DevSta: CorrErr- UncorrErr- FatalErr- UnsuppReq- AuxPwr- TransPend-

LnkCap: Port #0, Speed 2.5GT/s, Width x8, ASPM L0s L1, Exit Latency L0s
<512ns, L1 <4us

ClockPM+ Surprise- LLActRep- BwNot-

LnkCtl: ASPM Disabled; RCB 64 bytes Disabled- CommClk+

ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt-

LnkSta: Speed 2.5GT/s, Width x4, TrErr- Train- SlotClk+ DLActive-
BWMgmt- ABWMgmt-

DevCap2: Completion Timeout: Range AB, TimeoutDis+, LTR-, OBFF Not
Supported

DevCtl2: Completion Timeout: 50us to 50ms, TimeoutDis-, LTR-, OBFF Disabled

LnkCtl2: Target Link Speed: 2.5GT/s, EnterCompliance- SpeedDis-

Transmit Margin: Normal Operating Range, EnterModifiedCompliance-
ComplianceSOS-

Compliance De-emphasis: -6dB

LnkSta2: Current De-emphasis Level: -6dB, EqualizationComplete-,
EqualizationPhase1-

EqualizationPhase2-, EqualizationPhase3-, LinkEqualizationRequest-

Capabilities: [b4] Vendor Specific Information: Len=14 <?>

Capabilities: [100 v1] Virtual Channel

Caps: LPEVC=0 RefClk=100ns PATEntryBits=1

Arb: Fixed- WRR32- WRR64- WRR128-

Ctrl: ArbSelect=Fixed

Status: InProgress-

VC0: Caps: PATOffset=00 MaxTimeSlots=1 RejSnoopTrans-

Arb: Fixed- WRR32- WRR64- WRR128- TWRR128- WRR256-

Ctrl: Enable+ ID=0 ArbSelect=Fixed TC/VC=01

Status: NegoPending- InProgress-

Capabilities: [128 v1] Power Budgeting <?>

Capabilities: [600 v1] Vendor Specific Information: ID=0001 Rev=1
Len=024 <?>

Kernel driver in use: nouveau


[7.6.] SCSI information (from /proc/scsi/scsi)


Attached devices:

Host: scsi0 Channel: 00 Id: 00 Lun: 00

Vendor: ATA Model: TOSHIBA MQ01ABF0 Rev: AM0B

Type: Direct-Access ANSI SCSI revision: 05

Host: scsi1 Channel: 00 Id: 00 Lun: 00

Vendor: MATSHITA Model: DVD-RAM UJ8E2 S Rev: 1.00

Type: CD-ROM ANSI SCSI revision: 05


[7.7.] Other information that might be relevant to the problem (please
look in /proc and include all information that you think to be relevant):

1 15 1646 1936 2573 4 750 crypto net

10 150 1648 1946 2575 40 764 devices pagetypeinfo

1035 1501 1675 1967 26 400 765 diskstats partitions

1037 1507 168 1975 2631 401 785 dma sched_debug

1058 151 1683 2 2645 41 788 driver schedstat

1060 1513 1685 20 2794 42 789 execdomains scsi

1065 1515 1686 2018 28 43 794 fb self

11 1519 1688 2070 2840 44 8 filesystems slabinfo

1141 152 1689 2074 2884 45 803 fs softirqs

1143 1521 169 2085 2885 46 808 interrupts stat

1146 153 1691 21 294 47 871 iomem swaps

12 1535 1694 2176 2940 48 877 ioports sys

1234 1539 1695 2179 2943 49 884 ipmi sysrq-trigger

1245 1542 1696 2184 2945 5 9 irq sysvipc

1249 1543 17 2187 2948 51 91 kallsyms timer_list

1252 1545 1736 2188 2951 519 92 kcore timer_stats

13 1549 1743 2193 3 53 963 key-users tty

1397 1553 1821 2214 30 54 967 kmsg uptime

14 156 1840 2299 31 542 973 kpagecount version

1402 1563 1850 23 313 55 974 kpageflags vmallocinfo

1419 1566 1857 2329 33 601 977 latency_stats vmstat

143 1567 1859 2340 34 618 acpi loadavg zoneinfo

145 1569 1861 2391 35 623 asound locks

146 1586 1864 2407 355 67 buddyinfo mdstat

147 16 1890 2426 36 68 bus meminfo

148 160 1907 25 366 7 cgroups misc

1487 1604 1911 2544 37 70 cmdline modules

149 1623 1927 2555 38 71 consoles mounts

1493 1632 1932 2560 39 722 cpuinfo mtrr


[X.] Other notes, patches, fixes, workarounds:


link to Launchpad bug report:
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1325881





^ permalink raw reply

* Mapping of F11 and F12 on new lenovo laptops and Lenovo Compact Keyboard
From: Hans de Goede @ 2014-06-19  8:41 UTC (permalink / raw)
  To: Jamie Lentin, Jiri Kosina, Antonio Ospite
  Cc: linux-usb, platform-driver-x86@vger.kernel.org

Hi Jamie,

I saw your patch-set for the Lenovo Compact Keyboard on the lwn.net
kernel page.

This spiked my interest as I'm the author of this patch:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/platform/x86/thinkpad_acpi.c?id=8b9dd4fab26a0f328420cbda0845a325f45bcd92

Which adds mapping for the F9 - F12 on the Lenovo *40 series
laptops, which have the same weird symbols on F11 and F12 as the
Lenovo Compact Keyboard, the ones which you describe as:

/* Fn-F11: View open applications (3 boxes) */
/* Fn-F12: Open My computer (6 boxes) USB-only */

You map these to:
KEY_FN_F11
KEY_FILE

Where as my (already merged into Linus tree) patch maps these to:

KEY_SCALE
KEY_COMPUTER

Which are defined in linux/uapi/input.h as:

#define KEY_SCALE               120     /* AL Compiz Scale (Expose) */
#define KEY_COMPUTER            157

Which I believe maps closes to View open applications (which to me
sounds like expose mode) and Open My computer.

Note that on the laptops the keys have their special meaning by
default and using Fn turns them back into normal F11 keys, so
KEY_FN_F11 seems like a particular bad match as that suggests
a key combo which it is not on the laptops.

Anyways lets discuss and coordinate, so that we end up with the
same mappings for the weird symbols on F11 and F12 for the laptops
and for the Lenovo Compact Keyboard.

Regards,

Hans


^ permalink raw reply

* Re: [PATCH] GPIO button wth wakeup attribute is supposed to wake the system up
From: One Thousand Gnomes @ 2014-06-19 10:40 UTC (permalink / raw)
  To: Li, Aubrey
  Cc: dmitry.torokhov@gmail.com, Rafael J. Wysocki,
	linux-input@vger.kernel.org, LKML
In-Reply-To: <53A2340D.9030503@linux.intel.com>

On Thu, 19 Jun 2014 08:51:25 +0800
"Li, Aubrey" <aubrey.li@linux.intel.com> wrote:

> When the wakeup attribute is set, the GPIO button is capable of
> waking up the system from sleep states, including the "freeze"
> sleep state.  For that to work, its driver needs to pass the
> IRQF_NO_SUSPEND flag to devm_request_any_context_irq(), or the
> interrupt will be disabled by suspend_device_irqs() and the
> system won't be woken up by it from the "freeze" sleep state.
> 
> The suspend_device_irqs() routine is a workaround for drivers
> that mishandle interrupts triggered when the devices handled
> by them are suspended, so it is safe to use IRQF_NO_SUSPEND in
> all drivers that don't have that problem.
> 
> The affected/tested machines include Dell Venue 11 Pro and Asus T100TA.
> 
> Signed-off-by: Aubrey Li <aubrey.li@linux.intel.com>
> Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Reviewed-by: Alan Cox <alan@linux.intel.com>


^ permalink raw reply

* Re: [PATCH 13/24] Drivers:input: Use is_compat_task for ARM64 also.
From: Catalin Marinas @ 2014-06-19 12:50 UTC (permalink / raw)
  To: Andrew Pinski; +Cc: linux-arm-kernel, linux-kernel, linux-input
In-Reply-To: <1400914939-9708-14-git-send-email-apinski@cavium.com>

On Sat, May 24, 2014 at 12:02:08AM -0700, Andrew Pinski wrote:
> diff --git a/drivers/input/input-compat.h b/drivers/input/input-compat.h
> index 148f66f..6656957 100644
> --- a/drivers/input/input-compat.h
> +++ b/drivers/input/input-compat.h
> @@ -19,7 +19,7 @@
>  
>  /* Note to the author of this code: did it ever occur to
>     you why the ifdefs are needed? Think about it again. -AK */
> -#if defined(CONFIG_X86_64) || defined(CONFIG_TILE)
> +#if defined(CONFIG_X86_64) || defined(CONFIG_TILE) || defined(CONFIG_ARM64)
>  #  define INPUT_COMPAT_TEST is_compat_task()
>  #elif defined(CONFIG_S390)
>  #  define INPUT_COMPAT_TEST test_thread_flag(TIF_31BIT)

And this patch could be removed if we keep TIF_32BIT common.

-- 
Catalin

^ permalink raw reply

* [RFC] Input: uinput - add new UINPUT_DEV_SETUP ioctl
From: David Herrmann @ 2014-06-19 13:11 UTC (permalink / raw)
  To: linux-input
  Cc: Dmitry Torokhov, Peter Hutterer, Benjamin Tissoires,
	David Herrmann

This adds a new ioctl UINPUT_DEV_SETUP that replaces the old device setup
method (by write()'ing "struct uinput_user_dev" to the node). The old
method is not easily extendable and requires huge payloads. Furthermore,
overloading write() without properly versioned objects is error-prone.

Therefore, we introduce a new ioctl to replace the old method. The ioctl
supports all features of the old method, plus a "resolution" field for
absinfo. Furthermore, it's properly forward-compatible to new ABS codes
and a growing "struct input_absinfo" structure.

The ioctl also allows user-space to skip unknown axes if not set. The
payload-size can now be specified by the caller. There is no need to copy
the whole array temporarily into the kernel, but instead we can iterate
over it and copy each value manually.

Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
Hi

I replaced my old approach with a new ioctl(). In my opinion the write() method
for setup is error-prone and hard to replace correctly with a dynamic structure
like I did for this ioctl. The ioctl-way is also much more flexible and allows
read _and_ write in the same call.

This is untested so far, just wanted to get it out as Peter asked for it.

Btw., this should even perform better than the old method as it avoids copying
the whole object from the user into a temporary buffer.

Comments?
Thanks
David

 drivers/input/misc/uinput.c | 83 +++++++++++++++++++++++++++++++++++++++++++--
 include/uapi/linux/uinput.h | 79 ++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 156 insertions(+), 6 deletions(-)

diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c
index 615324c..e3952f4 100644
--- a/drivers/input/misc/uinput.c
+++ b/drivers/input/misc/uinput.c
@@ -371,8 +371,81 @@ static int uinput_allocate_device(struct uinput_device *udev)
 	return 0;
 }
 
-static int uinput_setup_device(struct uinput_device *udev,
-			       const char __user *buffer, size_t count)
+static int uinput_dev_setup(struct uinput_device *udev,
+			    struct uinput_setup __user *arg)
+{
+	struct uinput_setup setup;
+	struct input_dev *dev;
+	int i, retval;
+	u16 ver;
+
+	/*
+	 * Currently, we require:
+	 *   setup.size >= sizeof(struct uinput_setup)
+	 *   setup.absinfo_size >= sizeof(struct input_absinfo)
+	 * because we only support the initial version of uinput_setup. In case
+	 * we extend this type, we need to adjust those checks and fall-back to
+	 * older types if we run old user-space.
+	 */
+
+	if (get_user(setup.size, (__u64 __user*)arg))
+		return -EFAULT;
+	if (setup.size < sizeof(struct uinput_setup))
+		return -EINVAL;
+	if (copy_from_user(&setup, arg, sizeof(setup)))
+		return -EFAULT;
+
+	ver = setup.version;
+	if (put_user((__u16)UINPUT_VERSION, &arg->version))
+		return -EFAULT;
+	if (ver > UINPUT_VERSION)
+		return -EOPNOTSUPP;
+
+	if (!setup.name[0])
+		return -EINVAL;
+	if (setup.absinfo_size < sizeof(struct input_absinfo))
+		return -EINVAL;
+
+	dev = udev->dev;
+	dev->id = setup.id;
+	udev->ff_effects_max = setup.ff_effects_max;
+
+	kfree(dev->name);
+	dev->name = kstrndup(setup.name, UINPUT_MAX_NAME_SIZE, GFP_KERNEL);
+	if (!dev->name)
+		return -ENOMEM;
+
+	if (setup.abs_cnt > ABS_CNT)
+		setup.abs_cnt = ABS_CNT;
+
+	if (setup.abs_cnt > 0) {
+		input_alloc_absinfo(dev);
+		if (!dev->absinfo)
+			return -ENOMEM;
+
+		for (i = 0; i < setup.abs_cnt; ++i) {
+			struct input_absinfo absinfo;
+			u8 __user *p = (u8 __user*)arg->abs;
+
+			if (copy_from_user(&absinfo, p, sizeof(absinfo)))
+				return -EFAULT;
+
+			dev->absinfo[i] = absinfo;
+			p += setup.absinfo_size;
+		}
+	}
+
+	retval = uinput_validate_absbits(dev);
+	if (retval < 0)
+		return retval;
+
+	udev->state = UIST_SETUP_COMPLETE;
+	return 0;
+}
+
+/* legacy setup via write() */
+static int __uinput_setup_device(struct uinput_device *udev,
+				 const char __user *buffer, size_t count)
 {
 	struct uinput_user_dev	*user_dev;
 	struct input_dev	*dev;
@@ -475,7 +548,7 @@ static ssize_t uinput_write(struct file *file, const char __user *buffer,
 
 	retval = udev->state == UIST_CREATED ?
 			uinput_inject_events(udev, buffer, count) :
-			uinput_setup_device(udev, buffer, count);
+			__uinput_setup_device(udev, buffer, count);
 
 	mutex_unlock(&udev->mutex);
 
@@ -730,6 +803,10 @@ static long uinput_ioctl_handler(struct file *file, unsigned int cmd,
 			uinput_destroy_device(udev);
 			goto out;
 
+		case UI_DEV_SETUP:
+			retval = uinput_dev_setup(udev, p);
+			goto out;
+
 		case UI_SET_EVBIT:
 			retval = uinput_set_bit(arg, evbit, EV_MAX);
 			goto out;
diff --git a/include/uapi/linux/uinput.h b/include/uapi/linux/uinput.h
index 0389b48..bafc0dd 100644
--- a/include/uapi/linux/uinput.h
+++ b/include/uapi/linux/uinput.h
@@ -20,6 +20,8 @@
  * Author: Aristeu Sergio Rozanski Filho <aris@cathedrallabs.org>
  *
  * Changes/Revisions:
+ *	0.5	06/19/2014 (David Herrmann <dh.herrmann@gmail.com>
+ *		- add UI_DEV_SETUP ioctl
  *	0.4	01/09/2014 (Benjamin Tissoires <benjamin.tissoires@redhat.com>)
  *		- add UI_GET_SYSNAME ioctl
  *	0.3	24/05/2006 (Anssi Hannula <anssi.hannulagmail.com>)
@@ -37,8 +39,8 @@
 #include <linux/types.h>
 #include <linux/input.h>
 
-#define UINPUT_VERSION		4
-
+#define UINPUT_VERSION		5
+#define UINPUT_MAX_NAME_SIZE	80
 
 struct uinput_ff_upload {
 	__u32			request_id;
@@ -84,6 +86,78 @@ struct uinput_ff_erase {
  */
 #define UI_GET_SYSNAME(len)	_IOC(_IOC_READ, UINPUT_IOCTL_BASE, 300, len)
 
+struct uinput_setup {
+	__u32 size;
+	__u32 absinfo_size;
+	struct input_id id;
+	char name[UINPUT_MAX_NAME_SIZE];
+	__u32 ff_effects_max;
+	__u16 version;
+	__u16 abs_cnt;
+	struct input_absinfo abs[];
+};
+
+/**
+ * UI_DEV_SETUP - Set device parameters for setup
+ *
+ * This ioctl sets parameters for the input-device to be created. It must be
+ * issued _before_ calling UI_DEV_CREATE or it will fail. This ioctl supersedes
+ * the old "struct uinput_user_dev" method, which wrote this data via write().
+ *
+ * This ioctl takes a "struct uinput_setup" object as argument. The fields of
+ * this object are as follows:
+ *            size: This field _must_ be initialized to
+ *                  "sizeof(struct uinput_setup)". It *may not* include the
+ *                  size of the "abs" array at its end. The kernel uses this
+ *                  field to detect the revision of this ioctl. "uinput_setup"
+ *                  might be increased in the future, but will always be
+ *                  backwards compatible. If you pass a new object to an old
+ *                  kernel, the kernel will ignore unknown new fields.
+ *    absinfo_size: This field _must_ be initialized to
+ *                  "sizeof(struct input_absinfo)". It allows to extend the API
+ *                  to support more absinfo fields. Older kernels ignore unknown
+ *                  extensions to "struct input_absinfo".
+ *              id: See the description of "struct input_id". This field is
+ *                  copied unchanged into the new device.
+ *            name: This is used unchanged as name for the new device.
+ *  ff_effects_max: This limits the maximum numbers of force-feedback effects.
+ *                  See below for a description of FF with uinput.
+ *         version: This field must be set to the least required UINPUT version.
+ *                  If you do not require a specific version, set this to 0.
+ *                  Note that this ioctl was introduced with UINPUT_VERSION==5,
+ *                  so any version below will be treated as 0.
+ *                  Upon ioctl return, the kernel will overwrite this field
+ *                  with the actual UINPUT version of the kernel. You can
+ *                  obviously only rely on this, if the ioctl was successful.
+ *                  Otherwise, writing "version" might have caused the failure.
+ *                  However, the kernel tries to write this field as early as
+ *                  possible. So if it changed, you know that it was written
+ *                  properly.
+ *                  The kernel always writes a version greater than, or equal
+ *                  to, UINPUT_VERSION==5.
+ *         abs_cnt: This field defines the amount of elements in the following
+ *                  "abs" array. If this field is bigger than the kernel's view
+ *                  of ABS_CNT, the kernel will ignore any values beyond
+ *                  ABS_CNT.
+ *             abs: This is an appended array that contains parameters for ABS
+ *                  axes. See "struct input_absinfo" for a description of these
+ *                  fields. This array is copied unchanged into the kernel for
+ *                  all specified axes. However, to have effect, you must also
+ *                  enable the wanted axes via UI_SET_ABSBIT.
+ *
+ * This ioctl can be called multiple times and will overwrite previous values.
+ * If this ioctl fails, you're recommended to use the old "uinput_user_dev"
+ * method via write() as fallback, in case you run on an old kernel that does
+ * not support this ioctl.
+ *
+ * This ioctl may fail with -EINVAL if it is not supported or if you passed
+ * incorrect values, -ENOMEM if the kernel runs out of memory or -EFAULT if the
+ * passed uinput_setup object cannot be read/written.
+ * If this call fails, partial data may have already been applied to the
+ * internal device.
+ */
+#define UI_DEV_SETUP _IOWR(UINPUT_IOCTL_BASE, 301, struct uinput_setup)
+
 /*
  * To write a force-feedback-capable driver, the upload_effect
  * and erase_effect callbacks in input_dev must be implemented.
@@ -135,7 +209,6 @@ struct uinput_ff_erase {
 #define UI_FF_UPLOAD		1
 #define UI_FF_ERASE		2
 
-#define UINPUT_MAX_NAME_SIZE	80
 struct uinput_user_dev {
 	char name[UINPUT_MAX_NAME_SIZE];
 	struct input_id id;
-- 
2.0.0


^ permalink raw reply related

* [PATCH] Input: uinput - uinput_validate_absbits() cleanup
From: David Herrmann @ 2014-06-19 13:12 UTC (permalink / raw)
  To: linux-input
  Cc: Dmitry Torokhov, Peter Hutterer, Benjamin Tissoires,
	David Herrmann

This moves basic checks and setup from uinput_setup_device() into
uinput_validate_absbits() to make it easier to use. This way, we can call
it from other places without copying the boilerplate code.

Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
 drivers/input/misc/uinput.c | 40 +++++++++++++++++++++-------------------
 1 file changed, 21 insertions(+), 19 deletions(-)

diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c
index 8569362..615324c 100644
--- a/drivers/input/misc/uinput.c
+++ b/drivers/input/misc/uinput.c
@@ -311,7 +311,13 @@ static int uinput_open(struct inode *inode, struct file *file)
 static int uinput_validate_absbits(struct input_dev *dev)
 {
 	unsigned int cnt;
-	int retval = 0;
+	int nslot;
+
+	if (!test_bit(EV_ABS, dev->evbit))
+		return 0;
+
+	/* check if absmin/absmax/absfuzz/absflat are filled as
+	 * told in Documentation/input/input-programming.txt */
 
 	for (cnt = 0; cnt < ABS_CNT; cnt++) {
 		int min, max;
@@ -327,8 +333,7 @@ static int uinput_validate_absbits(struct input_dev *dev)
 				UINPUT_NAME, cnt,
 				input_abs_get_min(dev, cnt),
 				input_abs_get_max(dev, cnt));
-			retval = -EINVAL;
-			break;
+			return -EINVAL;
 		}
 
 		if (input_abs_get_flat(dev, cnt) >
@@ -340,11 +345,18 @@ static int uinput_validate_absbits(struct input_dev *dev)
 				input_abs_get_flat(dev, cnt),
 				input_abs_get_min(dev, cnt),
 				input_abs_get_max(dev, cnt));
-			retval = -EINVAL;
-			break;
+			return -EINVAL;
 		}
 	}
-	return retval;
+
+	if (test_bit(ABS_MT_SLOT, dev->absbit)) {
+		nslot = input_abs_get_max(dev, ABS_MT_SLOT) + 1;
+		input_mt_init_slots(dev, nslot, 0);
+	} else if (test_bit(ABS_MT_POSITION_X, dev->absbit)) {
+		input_set_events_per_packet(dev, 60);
+	}
+
+	return 0;
 }
 
 static int uinput_allocate_device(struct uinput_device *udev)
@@ -410,19 +422,9 @@ static int uinput_setup_device(struct uinput_device *udev,
 		input_abs_set_flat(dev, i, user_dev->absflat[i]);
 	}
 
-	/* check if absmin/absmax/absfuzz/absflat are filled as
-	 * told in Documentation/input/input-programming.txt */
-	if (test_bit(EV_ABS, dev->evbit)) {
-		retval = uinput_validate_absbits(dev);
-		if (retval < 0)
-			goto exit;
-		if (test_bit(ABS_MT_SLOT, dev->absbit)) {
-			int nslot = input_abs_get_max(dev, ABS_MT_SLOT) + 1;
-			input_mt_init_slots(dev, nslot, 0);
-		} else if (test_bit(ABS_MT_POSITION_X, dev->absbit)) {
-			input_set_events_per_packet(dev, 60);
-		}
-	}
+	retval = uinput_validate_absbits(dev);
+	if (retval < 0)
+		goto exit;
 
 	udev->state = UIST_SETUP_COMPLETE;
 	retval = count;
-- 
2.0.0


^ permalink raw reply related

* Re: [RFC] Input: uinput - add new UINPUT_DEV_SETUP ioctl
From: David Herrmann @ 2014-06-19 13:14 UTC (permalink / raw)
  To: open list:HID CORE LAYER
  Cc: Dmitry Torokhov, Peter Hutterer, Benjamin Tissoires,
	David Herrmann
In-Reply-To: <1403183515-15621-1-git-send-email-dh.herrmann@gmail.com>

Hi

On Thu, Jun 19, 2014 at 3:11 PM, David Herrmann <dh.herrmann@gmail.com> wrote:
> This adds a new ioctl UINPUT_DEV_SETUP that replaces the old device setup
> method (by write()'ing "struct uinput_user_dev" to the node). The old
> method is not easily extendable and requires huge payloads. Furthermore,
> overloading write() without properly versioned objects is error-prone.
>
> Therefore, we introduce a new ioctl to replace the old method. The ioctl
> supports all features of the old method, plus a "resolution" field for
> absinfo. Furthermore, it's properly forward-compatible to new ABS codes
> and a growing "struct input_absinfo" structure.
>
> The ioctl also allows user-space to skip unknown axes if not set. The
> payload-size can now be specified by the caller. There is no need to copy
> the whole array temporarily into the kernel, but instead we can iterate
> over it and copy each value manually.
>
> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
> ---
> Hi
>
> I replaced my old approach with a new ioctl(). In my opinion the write() method
> for setup is error-prone and hard to replace correctly with a dynamic structure
> like I did for this ioctl. The ioctl-way is also much more flexible and allows
> read _and_ write in the same call.
>
> This is untested so far, just wanted to get it out as Peter asked for it.
>
> Btw., this should even perform better than the old method as it avoids copying
> the whole object from the user into a temporary buffer.

I forgot to mention, this obviously relies on my other patch:
    [PATCH] Input: uinput - uinput_validate_absbits() cleanup

Thanks
David

^ permalink raw reply

* Re: [RFC] Input: uinput - add new UINPUT_DEV_SETUP ioctl
From: Peter Hutterer @ 2014-06-20  0:55 UTC (permalink / raw)
  To: David Herrmann; +Cc: linux-input, Dmitry Torokhov, Benjamin Tissoires
In-Reply-To: <1403183515-15621-1-git-send-email-dh.herrmann@gmail.com>

On Thu, Jun 19, 2014 at 03:11:55PM +0200, David Herrmann wrote:
> This adds a new ioctl UINPUT_DEV_SETUP that replaces the old device setup
> method (by write()'ing "struct uinput_user_dev" to the node). The old
> method is not easily extendable and requires huge payloads. Furthermore,
> overloading write() without properly versioned objects is error-prone.
> 
> Therefore, we introduce a new ioctl to replace the old method. The ioctl
> supports all features of the old method, plus a "resolution" field for
> absinfo. Furthermore, it's properly forward-compatible to new ABS codes
> and a growing "struct input_absinfo" structure.
> 
> The ioctl also allows user-space to skip unknown axes if not set. The
> payload-size can now be specified by the caller. There is no need to copy
> the whole array temporarily into the kernel, but instead we can iterate
> over it and copy each value manually.
> 
> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
> ---
> Hi
> 
> I replaced my old approach with a new ioctl(). In my opinion the write() method
> for setup is error-prone and hard to replace correctly with a dynamic structure
> like I did for this ioctl. The ioctl-way is also much more flexible and allows
> read _and_ write in the same call.
> 
> This is untested so far, just wanted to get it out as Peter asked for it.
> 
> Btw., this should even perform better than the old method as it avoids copying
> the whole object from the user into a temporary buffer.
> 
> Comments?

so on the whole, works for me. the API is a bit clunky with the various
sizeofs() but I'm not sure how to make this any better. One thought would be
a UI_SET_ABSINFO(abs_cnt, sizeof(input_absinfo), *data) that can set the
absinfo for a set of axes. You'd still need sizeof(input_absinfo) but the
rest of userspace would largely stay the same.

The main problem here is that you'd need a kernel behaviour change:
either ignore the abs axes in the write() once the new ioctl has been
called, or allow for absinfo of min/max 0/0 for axes already set.
I'm not a big fan of either.

>  drivers/input/misc/uinput.c | 83 +++++++++++++++++++++++++++++++++++++++++++--
>  include/uapi/linux/uinput.h | 79 ++++++++++++++++++++++++++++++++++++++++--
>  2 files changed, 156 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c
> index 615324c..e3952f4 100644
> --- a/drivers/input/misc/uinput.c
> +++ b/drivers/input/misc/uinput.c
> @@ -371,8 +371,81 @@ static int uinput_allocate_device(struct uinput_device *udev)
>  	return 0;
>  }
>  
> -static int uinput_setup_device(struct uinput_device *udev,
> -			       const char __user *buffer, size_t count)
> +static int uinput_dev_setup(struct uinput_device *udev,
> +			    struct uinput_setup __user *arg)
> +{
> +	struct uinput_setup setup;
> +	struct input_dev *dev;
> +	int i, retval;
> +	u16 ver;
> +
> +	/*
> +	 * Currently, we require:
> +	 *   setup.size >= sizeof(struct uinput_setup)
> +	 *   setup.absinfo_size >= sizeof(struct input_absinfo)
> +	 * because we only support the initial version of uinput_setup. In case
> +	 * we extend this type, we need to adjust those checks and fall-back to
> +	 * older types if we run old user-space.
> +	 */
> +
> +	if (get_user(setup.size, (__u64 __user*)arg))
> +		return -EFAULT;
> +	if (setup.size < sizeof(struct uinput_setup))
> +		return -EINVAL;
> +	if (copy_from_user(&setup, arg, sizeof(setup)))
> +		return -EFAULT;
> +
> +	ver = setup.version;
> +	if (put_user((__u16)UINPUT_VERSION, &arg->version))
> +		return -EFAULT;
> +	if (ver > UINPUT_VERSION)
> +		return -EOPNOTSUPP;

see my comments at the end of this email, other than that the code looks
good.

> +
> +	if (!setup.name[0])
> +		return -EINVAL;
> +	if (setup.absinfo_size < sizeof(struct input_absinfo))
> +		return -EINVAL;
> +
> +	dev = udev->dev;
> +	dev->id = setup.id;
> +	udev->ff_effects_max = setup.ff_effects_max;
> +
> +	kfree(dev->name);
> +	dev->name = kstrndup(setup.name, UINPUT_MAX_NAME_SIZE, GFP_KERNEL);
> +	if (!dev->name)
> +		return -ENOMEM;
> +
> +	if (setup.abs_cnt > ABS_CNT)
> +		setup.abs_cnt = ABS_CNT;
> +
> +	if (setup.abs_cnt > 0) {
> +		input_alloc_absinfo(dev);
> +		if (!dev->absinfo)
> +			return -ENOMEM;
> +
> +		for (i = 0; i < setup.abs_cnt; ++i) {
> +			struct input_absinfo absinfo;
> +			u8 __user *p = (u8 __user*)arg->abs;
> +
> +			if (copy_from_user(&absinfo, p, sizeof(absinfo)))
> +				return -EFAULT;
> +
> +			dev->absinfo[i] = absinfo;
> +			p += setup.absinfo_size;
> +		}
> +	}
> +
> +	retval = uinput_validate_absbits(dev);
> +	if (retval < 0)
> +		return retval;
> +
> +	udev->state = UIST_SETUP_COMPLETE;
> +	return 0;
> +}
> +
> +/* legacy setup via write() */
> +static int __uinput_setup_device(struct uinput_device *udev,
> +				 const char __user *buffer, size_t count)

wouldn't it be more obvious to rename this to "....legacy" or so?

>  {
>  	struct uinput_user_dev	*user_dev;
>  	struct input_dev	*dev;
> @@ -475,7 +548,7 @@ static ssize_t uinput_write(struct file *file, const char __user *buffer,
>  
>  	retval = udev->state == UIST_CREATED ?
>  			uinput_inject_events(udev, buffer, count) :
> -			uinput_setup_device(udev, buffer, count);
> +			__uinput_setup_device(udev, buffer, count);
>  
>  	mutex_unlock(&udev->mutex);
>  
> @@ -730,6 +803,10 @@ static long uinput_ioctl_handler(struct file *file, unsigned int cmd,
>  			uinput_destroy_device(udev);
>  			goto out;
>  
> +		case UI_DEV_SETUP:
> +			retval = uinput_dev_setup(udev, p);
> +			goto out;
> +
>  		case UI_SET_EVBIT:
>  			retval = uinput_set_bit(arg, evbit, EV_MAX);
>  			goto out;
> diff --git a/include/uapi/linux/uinput.h b/include/uapi/linux/uinput.h
> index 0389b48..bafc0dd 100644
> --- a/include/uapi/linux/uinput.h
> +++ b/include/uapi/linux/uinput.h
> @@ -20,6 +20,8 @@
>   * Author: Aristeu Sergio Rozanski Filho <aris@cathedrallabs.org>
>   *
>   * Changes/Revisions:
> + *	0.5	06/19/2014 (David Herrmann <dh.herrmann@gmail.com>
> + *		- add UI_DEV_SETUP ioctl

how comes the version here is 0.5 but the UINPUT_VERSION is 5? where does
the 0 come from?


>   *	0.4	01/09/2014 (Benjamin Tissoires <benjamin.tissoires@redhat.com>)
>   *		- add UI_GET_SYSNAME ioctl
>   *	0.3	24/05/2006 (Anssi Hannula <anssi.hannulagmail.com>)
> @@ -37,8 +39,8 @@
>  #include <linux/types.h>
>  #include <linux/input.h>
>  
> -#define UINPUT_VERSION		4
> -
> +#define UINPUT_VERSION		5
> +#define UINPUT_MAX_NAME_SIZE	80
>  
>  struct uinput_ff_upload {
>  	__u32			request_id;
> @@ -84,6 +86,78 @@ struct uinput_ff_erase {
>   */
>  #define UI_GET_SYSNAME(len)	_IOC(_IOC_READ, UINPUT_IOCTL_BASE, 300, len)
>  
> +struct uinput_setup {
> +	__u32 size;
> +	__u32 absinfo_size;
> +	struct input_id id;
> +	char name[UINPUT_MAX_NAME_SIZE];
> +	__u32 ff_effects_max;
> +	__u16 version;
> +	__u16 abs_cnt;
> +	struct input_absinfo abs[];
> +};
> +
> +/**
> + * UI_DEV_SETUP - Set device parameters for setup
> + *
> + * This ioctl sets parameters for the input-device to be created. It must be
> + * issued _before_ calling UI_DEV_CREATE or it will fail. This ioctl supersedes
> + * the old "struct uinput_user_dev" method, which wrote this data via write().
> + *
> + * This ioctl takes a "struct uinput_setup" object as argument. The fields of
> + * this object are as follows:
> + *            size: This field _must_ be initialized to
> + *                  "sizeof(struct uinput_setup)". It *may not* include the

s/may/must/

also, I notice different markup: "_must_" but "*may not*", is this intended?

> + *                  size of the "abs" array at its end. The kernel uses this
> + *                  field to detect the revision of this ioctl. "uinput_setup"
> + *                  might be increased in the future, but will always be

s/increased/enlarged/

> + *                  backwards compatible. If you pass a new object to an old
> + *                  kernel, the kernel will ignore unknown new fields.
> + *    absinfo_size: This field _must_ be initialized to
> + *                  "sizeof(struct input_absinfo)". It allows to extend the API
> + *                  to support more absinfo fields. Older kernels ignore unknown
> + *                  extensions to "struct input_absinfo".
> + *              id: See the description of "struct input_id". This field is
> + *                  copied unchanged into the new device.
> + *            name: This is used unchanged as name for the new device.
> + *  ff_effects_max: This limits the maximum numbers of force-feedback effects.
> + *                  See below for a description of FF with uinput.
> + *         version: This field must be set to the least required UINPUT version.

s/least/minimum/

> + *                  If you do not require a specific version, set this to 0.
> + *                  Note that this ioctl was introduced with UINPUT_VERSION==5,
> + *                  so any version below will be treated as 0.

IMO a client that wants a uinput version below the first that was introduced
should be greeted with an error, patted on the head and shown the door.
I can't think of a way to legitimately pass in 1-4 for the version field
that's not a result of a programming error or memory corruption.

> + *                  Upon ioctl return, the kernel will overwrite this field
> + *                  with the actual UINPUT version of the kernel. You can
> + *                  obviously only rely on this, if the ioctl was successful.

drop the comma

> + *                  Otherwise, writing "version" might have caused the failure.
> + *                  However, the kernel tries to write this field as early as
> + *                  possible. So if it changed, you know that it was written
> + *                  properly.

This is confusing. First you say you can't rely on it, but then I'm
supposed to know it was written properly anyway. And if I request a fixed
version (e.g. 5) I can't know if the -EINVAL overwrote the field or not.
How about just limiting it to success and EOPNOTSUPP:
anything else is off-limits:

On success, the kernel overwrites this field with the UINPUT version
supported by the kernel. If the kernel version is less than the requested
version, the kernel overwrites this field with the UINPUT version supported
by the kernel and the ioctl fails with -EOPNOTSUPP. For any other error,
this field is unmodified."

And then in the actual code you do:

  ver = setup.version;
  if (ver > UINPUT_VERSION) {
        if (put_user())
            -EFAULT
        return -EOPNOTSUPP;
  }

  ... everything else

  if (put_user(version))
       -EFAULT
  udev->state = UIST_SETUP_COMPLETE;
  return 0



> + *                  The kernel always writes a version greater than, or equal
> + *                  to, UINPUT_VERSION==5.

drop the commas

> + *         abs_cnt: This field defines the amount of elements in the following
> + *                  "abs" array. If this field is bigger than the kernel's view
> + *                  of ABS_CNT, the kernel will ignore any values beyond
> + *                  ABS_CNT.

A simple "axes beyond the kernel's ABS_CNT are ignored" is enough, imo.


> + *             abs: This is an appended array that contains parameters for ABS
> + *                  axes. See "struct input_absinfo" for a description of these
> + *                  fields. This array is copied unchanged into the kernel for
> + *                  all specified axes. However, to have effect, you must also
> + *                  enable the wanted axes via UI_SET_ABSBIT.

last sentence: "Only axes previously enabled with UI_SET_ABSBIT are copied."

> + *
> + * This ioctl can be called multiple times and will overwrite previous values.
> + * If this ioctl fails, you're recommended to use the old "uinput_user_dev"

"fails with -EINVAL" ...

> + * method via write() as fallback, in case you run on an old kernel that does
> + * not support this ioctl.
> + *
> + * This ioctl may fail with -EINVAL if it is not supported or if you passed
> + * incorrect values, -ENOMEM if the kernel runs out of memory or -EFAULT if the
> + * passed uinput_setup object cannot be read/written.

-EOPNOTSUPP for version > kernel version

and: doesn't it look like it's time to add a UI_GET_VERSION ioctl? because
with this ioctl you're pushing backwards-compat to the client, so knowing
what the kernel expects would be nice before assembling the data in a format
it may not like. That would also make it a lot easier to figure out if the
new ioctl is supported in the first place.

> + * If this call fails, partial data may have already been applied to the
> + * internal device.

maybe "The state of the internal device is undefined"

Cheers,
   Peter

> + */
> +#define UI_DEV_SETUP _IOWR(UINPUT_IOCTL_BASE, 301, struct uinput_setup)
> +
>  /*
>   * To write a force-feedback-capable driver, the upload_effect
>   * and erase_effect callbacks in input_dev must be implemented.
> @@ -135,7 +209,6 @@ struct uinput_ff_erase {
>  #define UI_FF_UPLOAD		1
>  #define UI_FF_ERASE		2
>  
> -#define UINPUT_MAX_NAME_SIZE	80
>  struct uinput_user_dev {
>  	char name[UINPUT_MAX_NAME_SIZE];
>  	struct input_id id;
> -- 
> 2.0.0
> 

^ permalink raw reply

* Re: [PATCH] Input: uinput - uinput_validate_absbits() cleanup
From: Peter Hutterer @ 2014-06-20  0:56 UTC (permalink / raw)
  To: David Herrmann; +Cc: linux-input, Dmitry Torokhov, Benjamin Tissoires
In-Reply-To: <1403183579-15680-1-git-send-email-dh.herrmann@gmail.com>

On Thu, Jun 19, 2014 at 03:12:59PM +0200, David Herrmann wrote:
> This moves basic checks and setup from uinput_setup_device() into
> uinput_validate_absbits() to make it easier to use. This way, we can call
> it from other places without copying the boilerplate code.
> 
> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>

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

Cheers,
   Peter

> ---
>  drivers/input/misc/uinput.c | 40 +++++++++++++++++++++-------------------
>  1 file changed, 21 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c
> index 8569362..615324c 100644
> --- a/drivers/input/misc/uinput.c
> +++ b/drivers/input/misc/uinput.c
> @@ -311,7 +311,13 @@ static int uinput_open(struct inode *inode, struct file *file)
>  static int uinput_validate_absbits(struct input_dev *dev)
>  {
>  	unsigned int cnt;
> -	int retval = 0;
> +	int nslot;
> +
> +	if (!test_bit(EV_ABS, dev->evbit))
> +		return 0;
> +
> +	/* check if absmin/absmax/absfuzz/absflat are filled as
> +	 * told in Documentation/input/input-programming.txt */
>  
>  	for (cnt = 0; cnt < ABS_CNT; cnt++) {
>  		int min, max;
> @@ -327,8 +333,7 @@ static int uinput_validate_absbits(struct input_dev *dev)
>  				UINPUT_NAME, cnt,
>  				input_abs_get_min(dev, cnt),
>  				input_abs_get_max(dev, cnt));
> -			retval = -EINVAL;
> -			break;
> +			return -EINVAL;
>  		}
>  
>  		if (input_abs_get_flat(dev, cnt) >
> @@ -340,11 +345,18 @@ static int uinput_validate_absbits(struct input_dev *dev)
>  				input_abs_get_flat(dev, cnt),
>  				input_abs_get_min(dev, cnt),
>  				input_abs_get_max(dev, cnt));
> -			retval = -EINVAL;
> -			break;
> +			return -EINVAL;
>  		}
>  	}
> -	return retval;
> +
> +	if (test_bit(ABS_MT_SLOT, dev->absbit)) {
> +		nslot = input_abs_get_max(dev, ABS_MT_SLOT) + 1;
> +		input_mt_init_slots(dev, nslot, 0);
> +	} else if (test_bit(ABS_MT_POSITION_X, dev->absbit)) {
> +		input_set_events_per_packet(dev, 60);
> +	}
> +
> +	return 0;
>  }
>  
>  static int uinput_allocate_device(struct uinput_device *udev)
> @@ -410,19 +422,9 @@ static int uinput_setup_device(struct uinput_device *udev,
>  		input_abs_set_flat(dev, i, user_dev->absflat[i]);
>  	}
>  
> -	/* check if absmin/absmax/absfuzz/absflat are filled as
> -	 * told in Documentation/input/input-programming.txt */
> -	if (test_bit(EV_ABS, dev->evbit)) {
> -		retval = uinput_validate_absbits(dev);
> -		if (retval < 0)
> -			goto exit;
> -		if (test_bit(ABS_MT_SLOT, dev->absbit)) {
> -			int nslot = input_abs_get_max(dev, ABS_MT_SLOT) + 1;
> -			input_mt_init_slots(dev, nslot, 0);
> -		} else if (test_bit(ABS_MT_POSITION_X, dev->absbit)) {
> -			input_set_events_per_packet(dev, 60);
> -		}
> -	}
> +	retval = uinput_validate_absbits(dev);
> +	if (retval < 0)
> +		goto exit;
>  
>  	udev->state = UIST_SETUP_COMPLETE;
>  	retval = count;
> -- 
> 2.0.0
> 

^ permalink raw reply

* Re: [PATCH v2 10/10] mfd: cros_ec: move EC interrupt to cros_ec_keyb
From: Simon Glass @ 2014-06-20  3:45 UTC (permalink / raw)
  To: Doug Anderson
  Cc: Lee Jones, Andrew Bresticker, Stephen Warren, Olof Johansson,
	Sonny Rao, linux-samsung-soc, Javier Martinez Canillas,
	Bill Richardson, Wolfram Sang, Mark Brown, Dmitry Torokhov,
	Samuel Ortiz, Geert Uytterhoeven, linux-input@vger.kernel.org, lk
In-Reply-To: <1403115247-8853-11-git-send-email-dianders@chromium.org>

On 18 June 2014 12:14, Doug Anderson <dianders@chromium.org> wrote:
> From: Andrew Bresticker <abrestic@chromium.org>
>
> If we receive EC interrupts after the cros_ec driver has probed, but
> before the cros_ec_keyb driver has probed, the cros_ec IRQ handler
> will not run the cros_ec_keyb notifier and the EC will leave the IRQ
> line asserted.  The cros_ec IRQ handler then returns IRQ_HANDLED and
> the resulting flood of interrupts causes the machine to hang.
>
> Since the EC interrupt is currently only used for the keyboard, move
> the setup and handling of the EC interrupt to the cros_ec_keyb driver.
>
> Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
> Signed-off-by: Doug Anderson <dianders@chromium.org>

Reviewed-by: Simon Glass <sjg@chromium.org>

We never needed an EC-level interrupt, and have shipped at least three
products now that use this code, so I think it is safe enough to
declare that we won't need it.

Regards,
Simon

^ permalink raw reply

* Re: [RFC] Input: uinput - add new UINPUT_DEV_SETUP ioctl
From: David Herrmann @ 2014-06-20 11:10 UTC (permalink / raw)
  To: Peter Hutterer
  Cc: open list:HID CORE LAYER, Dmitry Torokhov, Benjamin Tissoires
In-Reply-To: <20140620005557.GA22050@jelly.redhat.com>

Hi

On Fri, Jun 20, 2014 at 2:55 AM, Peter Hutterer
<peter.hutterer@who-t.net> wrote:
> On Thu, Jun 19, 2014 at 03:11:55PM +0200, David Herrmann wrote:
>> This adds a new ioctl UINPUT_DEV_SETUP that replaces the old device setup
>> method (by write()'ing "struct uinput_user_dev" to the node). The old
>> method is not easily extendable and requires huge payloads. Furthermore,
>> overloading write() without properly versioned objects is error-prone.
>>
>> Therefore, we introduce a new ioctl to replace the old method. The ioctl
>> supports all features of the old method, plus a "resolution" field for
>> absinfo. Furthermore, it's properly forward-compatible to new ABS codes
>> and a growing "struct input_absinfo" structure.
>>
>> The ioctl also allows user-space to skip unknown axes if not set. The
>> payload-size can now be specified by the caller. There is no need to copy
>> the whole array temporarily into the kernel, but instead we can iterate
>> over it and copy each value manually.
>>
>> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
>> ---
>> Hi
>>
>> I replaced my old approach with a new ioctl(). In my opinion the write() method
>> for setup is error-prone and hard to replace correctly with a dynamic structure
>> like I did for this ioctl. The ioctl-way is also much more flexible and allows
>> read _and_ write in the same call.
>>
>> This is untested so far, just wanted to get it out as Peter asked for it.
>>
>> Btw., this should even perform better than the old method as it avoids copying
>> the whole object from the user into a temporary buffer.
>>
>> Comments?
>
> so on the whole, works for me. the API is a bit clunky with the various
> sizeofs() but I'm not sure how to make this any better. One thought would be
> a UI_SET_ABSINFO(abs_cnt, sizeof(input_absinfo), *data) that can set the
> absinfo for a set of axes. You'd still need sizeof(input_absinfo) but the
> rest of userspace would largely stay the same.
>
> The main problem here is that you'd need a kernel behaviour change:
> either ignore the abs axes in the write() once the new ioctl has been
> called, or allow for absinfo of min/max 0/0 for axes already set.
> I'm not a big fan of either.

Thanks a lot for your comments. I played a little bit more with it and
came up with two changes:
 * Remove the "version" field in favor of a UI_GET_VERSION (like
   EVIOCGVERSION). This allows user-space to do whatever they
   want with the version-field and put their own restrictions on it.
 * Drop the "size" field. This was a leftover from when I tried to
   keep the write() method for SETUP. Now that I switched to ioctls,
   we can easily add UI_DEV_SETUP2 in case we every extend the
   structure. I mean, the code we end up with is the same as if
   we added the "size" field. We still have to check in the kernel
   what ioctl-version user-space used. So if we do this explicitly
   via a new ioctl-number, it's much easier and reliable to detect.
   Furthermore, we can always redefine the existing UI_DEV_SETUP
   to a new ioctl in case we're backwards-compatible. This way,
   we can extend the ioctl without requiring user-space to adopt
   it (besides recompiling if they want to use the new features).

That makes most of your comments obsolete. Sorry for not coming up
with this earlier, but it was mostly your feedback that caused me to
rethink the interface. So thanks a lot! But see below.

>>  drivers/input/misc/uinput.c | 83 +++++++++++++++++++++++++++++++++++++++++++--
>>  include/uapi/linux/uinput.h | 79 ++++++++++++++++++++++++++++++++++++++++--
>>  2 files changed, 156 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c
>> index 615324c..e3952f4 100644
>> --- a/drivers/input/misc/uinput.c
>> +++ b/drivers/input/misc/uinput.c
>> @@ -371,8 +371,81 @@ static int uinput_allocate_device(struct uinput_device *udev)
>>       return 0;
>>  }
>>
>> -static int uinput_setup_device(struct uinput_device *udev,
>> -                            const char __user *buffer, size_t count)
>> +static int uinput_dev_setup(struct uinput_device *udev,
>> +                         struct uinput_setup __user *arg)
>> +{
>> +     struct uinput_setup setup;
>> +     struct input_dev *dev;
>> +     int i, retval;
>> +     u16 ver;
>> +
>> +     /*
>> +      * Currently, we require:
>> +      *   setup.size >= sizeof(struct uinput_setup)
>> +      *   setup.absinfo_size >= sizeof(struct input_absinfo)
>> +      * because we only support the initial version of uinput_setup. In case
>> +      * we extend this type, we need to adjust those checks and fall-back to
>> +      * older types if we run old user-space.
>> +      */
>> +
>> +     if (get_user(setup.size, (__u64 __user*)arg))
>> +             return -EFAULT;
>> +     if (setup.size < sizeof(struct uinput_setup))
>> +             return -EINVAL;
>> +     if (copy_from_user(&setup, arg, sizeof(setup)))
>> +             return -EFAULT;
>> +
>> +     ver = setup.version;
>> +     if (put_user((__u16)UINPUT_VERSION, &arg->version))
>> +             return -EFAULT;
>> +     if (ver > UINPUT_VERSION)
>> +             return -EOPNOTSUPP;
>
> see my comments at the end of this email, other than that the code looks
> good.
>
>> +
>> +     if (!setup.name[0])
>> +             return -EINVAL;
>> +     if (setup.absinfo_size < sizeof(struct input_absinfo))
>> +             return -EINVAL;
>> +
>> +     dev = udev->dev;
>> +     dev->id = setup.id;
>> +     udev->ff_effects_max = setup.ff_effects_max;
>> +
>> +     kfree(dev->name);
>> +     dev->name = kstrndup(setup.name, UINPUT_MAX_NAME_SIZE, GFP_KERNEL);
>> +     if (!dev->name)
>> +             return -ENOMEM;
>> +
>> +     if (setup.abs_cnt > ABS_CNT)
>> +             setup.abs_cnt = ABS_CNT;
>> +
>> +     if (setup.abs_cnt > 0) {
>> +             input_alloc_absinfo(dev);
>> +             if (!dev->absinfo)
>> +                     return -ENOMEM;
>> +
>> +             for (i = 0; i < setup.abs_cnt; ++i) {
>> +                     struct input_absinfo absinfo;
>> +                     u8 __user *p = (u8 __user*)arg->abs;

For completeness, this lines obviously needs to be outside of the
for() loop. I fixed that up.

>> +
>> +                     if (copy_from_user(&absinfo, p, sizeof(absinfo)))
>> +                             return -EFAULT;
>> +
>> +                     dev->absinfo[i] = absinfo;
>> +                     p += setup.absinfo_size;
>> +             }
>> +     }
>> +
>> +     retval = uinput_validate_absbits(dev);
>> +     if (retval < 0)
>> +             return retval;
>> +
>> +     udev->state = UIST_SETUP_COMPLETE;
>> +     return 0;
>> +}
>> +
>> +/* legacy setup via write() */
>> +static int __uinput_setup_device(struct uinput_device *udev,
>> +                              const char __user *buffer, size_t count)
>
> wouldn't it be more obvious to rename this to "....legacy" or so?

Done.

>>  {
>>       struct uinput_user_dev  *user_dev;
>>       struct input_dev        *dev;
>> @@ -475,7 +548,7 @@ static ssize_t uinput_write(struct file *file, const char __user *buffer,
>>
>>       retval = udev->state == UIST_CREATED ?
>>                       uinput_inject_events(udev, buffer, count) :
>> -                     uinput_setup_device(udev, buffer, count);
>> +                     __uinput_setup_device(udev, buffer, count);
>>
>>       mutex_unlock(&udev->mutex);
>>
>> @@ -730,6 +803,10 @@ static long uinput_ioctl_handler(struct file *file, unsigned int cmd,
>>                       uinput_destroy_device(udev);
>>                       goto out;
>>
>> +             case UI_DEV_SETUP:
>> +                     retval = uinput_dev_setup(udev, p);
>> +                     goto out;
>> +
>>               case UI_SET_EVBIT:
>>                       retval = uinput_set_bit(arg, evbit, EV_MAX);
>>                       goto out;
>> diff --git a/include/uapi/linux/uinput.h b/include/uapi/linux/uinput.h
>> index 0389b48..bafc0dd 100644
>> --- a/include/uapi/linux/uinput.h
>> +++ b/include/uapi/linux/uinput.h
>> @@ -20,6 +20,8 @@
>>   * Author: Aristeu Sergio Rozanski Filho <aris@cathedrallabs.org>
>>   *
>>   * Changes/Revisions:
>> + *   0.5     06/19/2014 (David Herrmann <dh.herrmann@gmail.com>
>> + *           - add UI_DEV_SETUP ioctl
>
> how comes the version here is 0.5 but the UINPUT_VERSION is 5? where does
> the 0 come from?

Errr, I just followed suit.. Imo, we can change this to version "5"
and avoid any "0.x" prefixes.

Fixed.

>
>>   *   0.4     01/09/2014 (Benjamin Tissoires <benjamin.tissoires@redhat.com>)
>>   *           - add UI_GET_SYSNAME ioctl
>>   *   0.3     24/05/2006 (Anssi Hannula <anssi.hannulagmail.com>)
>> @@ -37,8 +39,8 @@
>>  #include <linux/types.h>
>>  #include <linux/input.h>
>>
>> -#define UINPUT_VERSION               4
>> -
>> +#define UINPUT_VERSION               5
>> +#define UINPUT_MAX_NAME_SIZE 80
>>
>>  struct uinput_ff_upload {
>>       __u32                   request_id;
>> @@ -84,6 +86,78 @@ struct uinput_ff_erase {
>>   */
>>  #define UI_GET_SYSNAME(len)  _IOC(_IOC_READ, UINPUT_IOCTL_BASE, 300, len)
>>
>> +struct uinput_setup {
>> +     __u32 size;
>> +     __u32 absinfo_size;
>> +     struct input_id id;
>> +     char name[UINPUT_MAX_NAME_SIZE];
>> +     __u32 ff_effects_max;
>> +     __u16 version;
>> +     __u16 abs_cnt;
>> +     struct input_absinfo abs[];
>> +};
>> +
>> +/**
>> + * UI_DEV_SETUP - Set device parameters for setup
>> + *
>> + * This ioctl sets parameters for the input-device to be created. It must be
>> + * issued _before_ calling UI_DEV_CREATE or it will fail. This ioctl supersedes
>> + * the old "struct uinput_user_dev" method, which wrote this data via write().
>> + *
>> + * This ioctl takes a "struct uinput_setup" object as argument. The fields of
>> + * this object are as follows:
>> + *            size: This field _must_ be initialized to
>> + *                  "sizeof(struct uinput_setup)". It *may not* include the
>
> s/may/must/
>
> also, I notice different markup: "_must_" but "*may not*", is this intended?

Both fixed.

>> + *                  size of the "abs" array at its end. The kernel uses this
>> + *                  field to detect the revision of this ioctl. "uinput_setup"
>> + *                  might be increased in the future, but will always be
>
> s/increased/enlarged/

Not needed anymore.

>> + *                  backwards compatible. If you pass a new object to an old
>> + *                  kernel, the kernel will ignore unknown new fields.
>> + *    absinfo_size: This field _must_ be initialized to
>> + *                  "sizeof(struct input_absinfo)". It allows to extend the API
>> + *                  to support more absinfo fields. Older kernels ignore unknown
>> + *                  extensions to "struct input_absinfo".
>> + *              id: See the description of "struct input_id". This field is
>> + *                  copied unchanged into the new device.
>> + *            name: This is used unchanged as name for the new device.
>> + *  ff_effects_max: This limits the maximum numbers of force-feedback effects.
>> + *                  See below for a description of FF with uinput.
>> + *         version: This field must be set to the least required UINPUT version.
>
> s/least/minimum/

likewise

>> + *                  If you do not require a specific version, set this to 0.
>> + *                  Note that this ioctl was introduced with UINPUT_VERSION==5,
>> + *                  so any version below will be treated as 0.
>
> IMO a client that wants a uinput version below the first that was introduced
> should be greeted with an error, patted on the head and shown the door.
> I can't think of a way to legitimately pass in 1-4 for the version field
> that's not a result of a programming error or memory corruption.

likewise

>> + *                  Upon ioctl return, the kernel will overwrite this field
>> + *                  with the actual UINPUT version of the kernel. You can
>> + *                  obviously only rely on this, if the ioctl was successful.
>
> drop the comma

likewise

>> + *                  Otherwise, writing "version" might have caused the failure.
>> + *                  However, the kernel tries to write this field as early as
>> + *                  possible. So if it changed, you know that it was written
>> + *                  properly.
>
> This is confusing. First you say you can't rely on it, but then I'm
> supposed to know it was written properly anyway. And if I request a fixed
> version (e.g. 5) I can't know if the -EINVAL overwrote the field or not.
> How about just limiting it to success and EOPNOTSUPP:
> anything else is off-limits:
>
> On success, the kernel overwrites this field with the UINPUT version
> supported by the kernel. If the kernel version is less than the requested
> version, the kernel overwrites this field with the UINPUT version supported
> by the kernel and the ioctl fails with -EOPNOTSUPP. For any other error,
> this field is unmodified."
>
> And then in the actual code you do:
>
>   ver = setup.version;
>   if (ver > UINPUT_VERSION) {
>         if (put_user())
>             -EFAULT
>         return -EOPNOTSUPP;
>   }
>
>   ... everything else
>
>   if (put_user(version))
>        -EFAULT
>   udev->state = UIST_SETUP_COMPLETE;
>   return 0

likewise

>> + *                  The kernel always writes a version greater than, or equal
>> + *                  to, UINPUT_VERSION==5.
>
> drop the commas

likewise

>> + *         abs_cnt: This field defines the amount of elements in the following
>> + *                  "abs" array. If this field is bigger than the kernel's view
>> + *                  of ABS_CNT, the kernel will ignore any values beyond
>> + *                  ABS_CNT.
>
> A simple "axes beyond the kernel's ABS_CNT are ignored" is enough, imo.

Fixed.

>> + *             abs: This is an appended array that contains parameters for ABS
>> + *                  axes. See "struct input_absinfo" for a description of these
>> + *                  fields. This array is copied unchanged into the kernel for
>> + *                  all specified axes. However, to have effect, you must also
>> + *                  enable the wanted axes via UI_SET_ABSBIT.
>
> last sentence: "Only axes previously enabled with UI_SET_ABSBIT are copied."

This is not true. All axes are copied. That's how uinput always
worked. You can set UI_SET_ABSBIT after SETUP. However, given that we
do the "uinput_validate_absbits" only on SETUP, you would get weird
errors during UI_DEV_CREATE if the axis data was invalid (afaik
uinput_register_device() validates those values. Not sure, though).

>> + *
>> + * This ioctl can be called multiple times and will overwrite previous values.
>> + * If this ioctl fails, you're recommended to use the old "uinput_user_dev"
>
> "fails with -EINVAL" ...

Fixed.

>> + * method via write() as fallback, in case you run on an old kernel that does
>> + * not support this ioctl.
>> + *
>> + * This ioctl may fail with -EINVAL if it is not supported or if you passed
>> + * incorrect values, -ENOMEM if the kernel runs out of memory or -EFAULT if the
>> + * passed uinput_setup object cannot be read/written.
>
> -EOPNOTSUPP for version > kernel version

No longer needed.

> and: doesn't it look like it's time to add a UI_GET_VERSION ioctl? because
> with this ioctl you're pushing backwards-compat to the client, so knowing
> what the kernel expects would be nice before assembling the data in a format
> it may not like. That would also make it a lot easier to figure out if the
> new ioctl is supported in the first place.
>
>> + * If this call fails, partial data may have already been applied to the
>> + * internal device.
>
> maybe "The state of the internal device is undefined"

"undefined" is so.. undefined. I prefer explicitly allowing calling
this ioctl multiple times. I think my comment implies that you need to
provide at least the same set of information on the second invokation
to overwrite all previous data. Does that make sense?

Thanks a lot for the review! I will send v2 shortly.

Thanks
David

^ permalink raw reply

* Re: [RFC] Input: uinput - add new UINPUT_DEV_SETUP ioctl
From: David Herrmann @ 2014-06-20 11:14 UTC (permalink / raw)
  To: Peter Hutterer
  Cc: open list:HID CORE LAYER, Dmitry Torokhov, Benjamin Tissoires
In-Reply-To: <CANq1E4Sx4=ko=sbbOR-S=gOsnHU7m6asiz3XGzU_BD81F9mqeA@mail.gmail.com>

Hi

On Fri, Jun 20, 2014 at 1:10 PM, David Herrmann <dh.herrmann@gmail.com> wrote:
>>> + *             abs: This is an appended array that contains parameters for ABS
>>> + *                  axes. See "struct input_absinfo" for a description of these
>>> + *                  fields. This array is copied unchanged into the kernel for
>>> + *                  all specified axes. However, to have effect, you must also
>>> + *                  enable the wanted axes via UI_SET_ABSBIT.
>>
>> last sentence: "Only axes previously enabled with UI_SET_ABSBIT are copied."
>
> This is not true. All axes are copied. That's how uinput always
> worked. You can set UI_SET_ABSBIT after SETUP. However, given that we
> do the "uinput_validate_absbits" only on SETUP, you would get weird
> errors during UI_DEV_CREATE if the axis data was invalid (afaik
> uinput_register_device() validates those values. Not sure, though).

Ok, I changed my mind again. I now explicitly ignore any axes not
enabled via UI_SET_ABSBIT. I think you're right and that's how users
would expect this to work.

Thanks
David

^ permalink raw reply

* [PATCH v2 1/2] Input: uinput - add UI_GET_VERSION ioctl
From: David Herrmann @ 2014-06-20 11:26 UTC (permalink / raw)
  To: linux-input
  Cc: Dmitry Torokhov, Peter Hutterer, Benjamin Tissoires,
	David Herrmann

This ioctl is the counterpart to EVIOCGVERSION and returns the
uinput-version the kernel was compiled with.

Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
 drivers/input/misc/uinput.c | 6 ++++++
 include/uapi/linux/uinput.h | 9 +++++++++
 2 files changed, 15 insertions(+)

diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c
index 615324c..a2a3895 100644
--- a/drivers/input/misc/uinput.c
+++ b/drivers/input/misc/uinput.c
@@ -722,6 +722,12 @@ static long uinput_ioctl_handler(struct file *file, unsigned int cmd,
 	}
 
 	switch (cmd) {
+		case UI_GET_VERSION:
+			if (put_user(UINPUT_VERSION,
+				     (unsigned int __user*)p))
+				retval = -EFAULT;
+			goto out;
+
 		case UI_DEV_CREATE:
 			retval = uinput_create_device(udev);
 			goto out;
diff --git a/include/uapi/linux/uinput.h b/include/uapi/linux/uinput.h
index 0389b48..19339cf 100644
--- a/include/uapi/linux/uinput.h
+++ b/include/uapi/linux/uinput.h
@@ -84,6 +84,15 @@ struct uinput_ff_erase {
  */
 #define UI_GET_SYSNAME(len)	_IOC(_IOC_READ, UINPUT_IOCTL_BASE, 300, len)
 
+/**
+ * UI_GET_VERSION - Return uinput version of the kernel
+ *
+ * This writes the kernel's uinput version into the integer pointed to by the
+ * ioctl argument. The uinput-version is hard-coded in the kernel and
+ * independent of the uinput device.
+ */
+#define UI_GET_VERSION _IOR(UINPUT_IOCTL_BASE, 301, unsigned int)
+
 /*
  * To write a force-feedback-capable driver, the upload_effect
  * and erase_effect callbacks in input_dev must be implemented.
-- 
2.0.0


^ permalink raw reply related

* [PATCH v2 2/2] Input: uinput - add new UINPUT_DEV_SETUP ioctl
From: David Herrmann @ 2014-06-20 11:26 UTC (permalink / raw)
  To: linux-input
  Cc: Dmitry Torokhov, Peter Hutterer, Benjamin Tissoires,
	David Herrmann
In-Reply-To: <1403263600-24736-1-git-send-email-dh.herrmann@gmail.com>

This adds a new ioctl UINPUT_DEV_SETUP that replaces the old device setup
method (by write()'ing "struct uinput_user_dev" to the node). The old
method is not easily extendable and requires huge payloads. Furthermore,
overloading write() without properly versioned objects is error-prone.

Therefore, we introduce a new ioctl to replace the old method. The ioctl
supports all features of the old method, plus a "resolution" field for
absinfo. Furthermore, it's properly forward-compatible to new ABS codes
and a growing "struct input_absinfo" structure.

The ioctl also allows user-space to skip unknown axes if not set. The
payload-size can now be specified by the caller. There is no need to copy
the whole array temporarily into the kernel, but instead we can iterate
over it and copy each value manually.

Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
v2:
 - drop "version" field
 - drop "size" field
 - only copy absinfo if UI_SET_ABSBIT was called for the axis
 - minor linguistic changes

 drivers/input/misc/uinput.c | 67 +++++++++++++++++++++++++++++++++++++++++++--
 include/uapi/linux/uinput.h | 55 +++++++++++++++++++++++++++++++++++--
 2 files changed, 116 insertions(+), 6 deletions(-)

diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c
index a2a3895..5c125e8 100644
--- a/drivers/input/misc/uinput.c
+++ b/drivers/input/misc/uinput.c
@@ -371,8 +371,65 @@ static int uinput_allocate_device(struct uinput_device *udev)
 	return 0;
 }
 
-static int uinput_setup_device(struct uinput_device *udev,
-			       const char __user *buffer, size_t count)
+static int uinput_dev_setup(struct uinput_device *udev,
+			    struct uinput_setup __user *arg)
+{
+	struct uinput_setup setup;
+	struct input_dev *dev;
+	int i, retval;
+
+	if (copy_from_user(&setup, arg, sizeof(setup)))
+		return -EFAULT;
+	if (!setup.name[0])
+		return -EINVAL;
+	/* So far we only support the original "struct input_absinfo", but be
+	 * forward compatible and allow larger payloads. */
+	if (setup.absinfo_size < sizeof(struct input_absinfo))
+		return -EINVAL;
+
+	dev = udev->dev;
+	dev->id = setup.id;
+	udev->ff_effects_max = setup.ff_effects_max;
+
+	kfree(dev->name);
+	dev->name = kstrndup(setup.name, UINPUT_MAX_NAME_SIZE, GFP_KERNEL);
+	if (!dev->name)
+		return -ENOMEM;
+
+	if (setup.abs_cnt > ABS_CNT)
+		setup.abs_cnt = ABS_CNT;
+
+	if (setup.abs_cnt > 0) {
+		u8 __user *p = (u8 __user*)arg->abs;
+
+		input_alloc_absinfo(dev);
+		if (!dev->absinfo)
+			return -ENOMEM;
+
+		for (i = 0; i < setup.abs_cnt; ++i, p += setup.absinfo_size) {
+			struct input_absinfo absinfo;
+
+			if (!test_bit(i, dev->absbit))
+				continue;
+
+			if (copy_from_user(&absinfo, p, sizeof(absinfo)))
+				return -EFAULT;
+
+			dev->absinfo[i] = absinfo;
+		}
+	}
+
+	retval = uinput_validate_absbits(dev);
+	if (retval < 0)
+		return retval;
+
+	udev->state = UIST_SETUP_COMPLETE;
+	return 0;
+}
+
+/* legacy setup via write() */
+static int uinput_setup_device_legacy(struct uinput_device *udev,
+				      const char __user *buffer, size_t count)
 {
 	struct uinput_user_dev	*user_dev;
 	struct input_dev	*dev;
@@ -475,7 +532,7 @@ static ssize_t uinput_write(struct file *file, const char __user *buffer,
 
 	retval = udev->state == UIST_CREATED ?
 			uinput_inject_events(udev, buffer, count) :
-			uinput_setup_device(udev, buffer, count);
+			uinput_setup_device_legacy(udev, buffer, count);
 
 	mutex_unlock(&udev->mutex);
 
@@ -736,6 +793,10 @@ static long uinput_ioctl_handler(struct file *file, unsigned int cmd,
 			uinput_destroy_device(udev);
 			goto out;
 
+		case UI_DEV_SETUP:
+			retval = uinput_dev_setup(udev, p);
+			goto out;
+
 		case UI_SET_EVBIT:
 			retval = uinput_set_bit(arg, evbit, EV_MAX);
 			goto out;
diff --git a/include/uapi/linux/uinput.h b/include/uapi/linux/uinput.h
index 19339cf..982144d 100644
--- a/include/uapi/linux/uinput.h
+++ b/include/uapi/linux/uinput.h
@@ -20,6 +20,8 @@
  * Author: Aristeu Sergio Rozanski Filho <aris@cathedrallabs.org>
  *
  * Changes/Revisions:
+ *	5	06/20/2014 (David Herrmann <dh.herrmann@gmail.com>
+ *		- add UI_DEV_SETUP ioctl
  *	0.4	01/09/2014 (Benjamin Tissoires <benjamin.tissoires@redhat.com>)
  *		- add UI_GET_SYSNAME ioctl
  *	0.3	24/05/2006 (Anssi Hannula <anssi.hannulagmail.com>)
@@ -37,8 +39,8 @@
 #include <linux/types.h>
 #include <linux/input.h>
 
-#define UINPUT_VERSION		4
-
+#define UINPUT_VERSION		5
+#define UINPUT_MAX_NAME_SIZE	80
 
 struct uinput_ff_upload {
 	__u32			request_id;
@@ -93,6 +95,54 @@ struct uinput_ff_erase {
  */
 #define UI_GET_VERSION _IOR(UINPUT_IOCTL_BASE, 301, unsigned int)
 
+struct uinput_setup {
+	__u64 absinfo_size;
+	struct input_id id;
+	char name[UINPUT_MAX_NAME_SIZE];
+	__u32 ff_effects_max;
+	__u32 abs_cnt;
+	struct input_absinfo abs[];
+};
+
+/**
+ * UI_DEV_SETUP - Set device parameters for setup
+ *
+ * This ioctl sets parameters for the input-device to be created. It must be
+ * issued *before* calling UI_DEV_CREATE or it will fail. This ioctl supersedes
+ * the old "struct uinput_user_dev" method, which wrote this data via write().
+ *
+ * This ioctl takes a "struct uinput_setup" object as argument. The fields of
+ * this object are as follows:
+ *    absinfo_size: This field *must* be initialized to
+ *                  "sizeof(struct input_absinfo)". It allows to extend the API
+ *                  to support more absinfo fields. Older kernels ignore unknown
+ *                  extensions to "struct input_absinfo".
+ *              id: See the description of "struct input_id". This field is
+ *                  copied unchanged into the new device.
+ *            name: This is used unchanged as name for the new device.
+ *  ff_effects_max: This limits the maximum numbers of force-feedback effects.
+ *                  See below for a description of FF with uinput.
+ *         abs_cnt: This field defines the amount of elements in the following
+ *                  "abs" array. Axes beyond the kernel's ABS_CNT are ignored.
+ *             abs: This is an appended array that contains parameters for ABS
+ *                  axes. See "struct input_absinfo" for a description of these
+ *                  fields. This array is copied unchanged into the kernel for
+ *                  all specified axes. Axes not enabled via UI_SET_ABSBIT are
+ *                  ignored.
+ *
+ * This ioctl can be called multiple times and will overwrite previous values.
+ * If this ioctl fails with -EINVAL, you're recommended to use the old
+ * "uinput_user_dev" method via write() as fallback, in case you run on an old
+ * kernel that does not support this ioctl.
+ *
+ * This ioctl may fail with -EINVAL if it is not supported or if you passed
+ * incorrect values, -ENOMEM if the kernel runs out of memory or -EFAULT if the
+ * passed uinput_setup object cannot be read/written.
+ * If this call fails, partial data may have already been applied to the
+ * internal device.
+ */
+#define UI_DEV_SETUP _IOWR(UINPUT_IOCTL_BASE, 302, struct uinput_setup)
+
 /*
  * To write a force-feedback-capable driver, the upload_effect
  * and erase_effect callbacks in input_dev must be implemented.
@@ -144,7 +194,6 @@ struct uinput_ff_erase {
 #define UI_FF_UPLOAD		1
 #define UI_FF_ERASE		2
 
-#define UINPUT_MAX_NAME_SIZE	80
 struct uinput_user_dev {
 	char name[UINPUT_MAX_NAME_SIZE];
 	struct input_id id;
-- 
2.0.0


^ permalink raw reply related

* Re: Mapping of F11 and F12 on new lenovo laptops and Lenovo Compact Keyboard
From: Jamie Lentin @ 2014-06-20 12:00 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Jiri Kosina, Antonio Ospite, linux-usb,
	platform-driver-x86@vger.kernel.org
In-Reply-To: <53A2A231.4030700@redhat.com>

On Thu, 19 Jun 2014, Hans de Goede wrote:

> Hi Jamie,
>
> I saw your patch-set for the Lenovo Compact Keyboard on the lwn.net
> kernel page.
>
> This spiked my interest as I'm the author of this patch:
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/platform/x86/thinkpad_acpi.c?id=8b9dd4fab26a0f328420cbda0845a325f45bcd92
>
> Which adds mapping for the F9 - F12 on the Lenovo *40 series
> laptops, which have the same weird symbols on F11 and F12 as the
> Lenovo Compact Keyboard, the ones which you describe as:
>
> /* Fn-F11: View open applications (3 boxes) */
> /* Fn-F12: Open My computer (6 boxes) USB-only */
>
> You map these to:
> KEY_FN_F11
> KEY_FILE
>
> Where as my (already merged into Linus tree) patch maps these to:
>
> KEY_SCALE
> KEY_COMPUTER
>
> Which are defined in linux/uapi/input.h as:
>
> #define KEY_SCALE               120     /* AL Compiz Scale (Expose) */
> #define KEY_COMPUTER            157
>
> Which I believe maps closes to View open applications (which to me
> sounds like expose mode) and Open My computer.

Yes, the Function keys look the same:-
http://www.lenovo.com/images/gallery/1060x596/lenovo-laptop-thinkpad-t440-overhead-keyboard-2.jpg
https://c2.staticflickr.com/6/5467/9092693533_440cfcf311_z.jpg

I went with KEY_FILE on the USB keyboard, since this is what it's mapped 
to on the Bluetooth keyboard, as part of the CONSUMER usage page:-
http://lxr.free-electrons.com/source/drivers/hid/hid-input.c#L694

Personally I've no fondness to what the keys map to, since I remap them to 
how they are labelled on an Thinkpad X230, which is pause/nexttrack. 
However, it seems somewhat silly customising something that appears to be 
a standard mapping.

> Note that on the laptops the keys have their special meaning by
> default and using Fn turns them back into normal F11 keys, so
> KEY_FN_F11 seems like a particular bad match as that suggests
> a key combo which it is not on the laptops.

That's true. It works fine with Fn-ESC but yes, on these keyboards they 
are labelled such that F11 is the Fn-modified version.

> Anyways lets discuss and coordinate, so that we end up with the
> same mappings for the weird symbols on F11 and F12 for the laptops
> and for the Lenovo Compact Keyboard.

I think there are also some Apple keyboards using the same symbols, it 
might be worth seeing if these are supported and what they produce.

>
> Regards,
>
> Hans
>
>

-- 
Jamie Lentin

^ permalink raw reply

* Re: Mapping of F11 and F12 on new lenovo laptops and Lenovo Compact Keyboard
From: Hans de Goede @ 2014-06-20 12:44 UTC (permalink / raw)
  To: Jamie Lentin
  Cc: Jiri Kosina, Antonio Ospite, linux-usb,
	platform-driver-x86@vger.kernel.org
In-Reply-To: <alpine.DEB.2.02.1406201238550.4059@marmot.wormnet.eu>

Hi,

On 06/20/2014 02:00 PM, Jamie Lentin wrote:
> On Thu, 19 Jun 2014, Hans de Goede wrote:
> 
>> Hi Jamie,
>>
>> I saw your patch-set for the Lenovo Compact Keyboard on the lwn.net
>> kernel page.
>>
>> This spiked my interest as I'm the author of this patch:
>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/platform/x86/thinkpad_acpi.c?id=8b9dd4fab26a0f328420cbda0845a325f45bcd92
>>
>> Which adds mapping for the F9 - F12 on the Lenovo *40 series
>> laptops, which have the same weird symbols on F11 and F12 as the
>> Lenovo Compact Keyboard, the ones which you describe as:
>>
>> /* Fn-F11: View open applications (3 boxes) */
>> /* Fn-F12: Open My computer (6 boxes) USB-only */
>>
>> You map these to:
>> KEY_FN_F11
>> KEY_FILE
>>
>> Where as my (already merged into Linus tree) patch maps these to:
>>
>> KEY_SCALE
>> KEY_COMPUTER
>>
>> Which are defined in linux/uapi/input.h as:
>>
>> #define KEY_SCALE               120     /* AL Compiz Scale (Expose) */
>> #define KEY_COMPUTER            157
>>
>> Which I believe maps closes to View open applications (which to me
>> sounds like expose mode) and Open My computer.
> 
> Yes, the Function keys look the same:-
> http://www.lenovo.com/images/gallery/1060x596/lenovo-laptop-thinkpad-t440-overhead-keyboard-2.jpg
> https://c2.staticflickr.com/6/5467/9092693533_440cfcf311_z.jpg
> 
> I went with KEY_FILE on the USB keyboard, since this is what it's mapped to on the Bluetooth keyboard, as part of the CONSUMER usage page:-
> http://lxr.free-electrons.com/source/drivers/hid/hid-input.c#L694

Ah, well if the bluetooth version of the keyboard actually uses a standard
HUT code for that and that maps to KEY_FILE, then I agree that it would
be best to map the "F-12" key to KEY_FILE.

> Personally I've no fondness to what the keys map to, since I remap them to how they are labelled on an Thinkpad X230, which is pause/nexttrack. However, it seems somewhat silly customising something that appears to be a standard mapping.
> 
>> Note that on the laptops the keys have their special meaning by
>> default and using Fn turns them back into normal F11 keys, so
>> KEY_FN_F11 seems like a particular bad match as that suggests
>> a key combo which it is not on the laptops.
> 
> That's true. It works fine with Fn-ESC but yes, on these keyboards they are labelled such that F11 is the Fn-modified version.

Right so lets just go with KEY_SCALE then? If you switch F-11 to
KEY_SCALE in the next version of your patch set, then I'll send
a followup patch for thinkpad-acpi to change F-12 to KEY_FILE, and
then the 2 mappings will be in sync.

Agreed ?

Regards,

Hans

^ permalink raw reply

* [RFC] Input: evdev - add new EVIOCGABSRANGE ioctl
From: David Herrmann @ 2014-06-20 14:05 UTC (permalink / raw)
  To: linux-input
  Cc: Dmitry Torokhov, Peter Hutterer, Benjamin Tissoires,
	David Herrmann

When we introduced the slotted MT ABS extensions, we didn't take care to
make SYN_DROPPED recoverable. Imagine a client recevies a SYN_DROPPED and
syncs its current state via EVIOCGABS. It has to call this ioctl for each
and every ABS code separately. Besides being horribly slow, this series
of ioctl-calls is not atomic. The kernel might queue new ABS events while
the client fetches the data.

Now for normal ABS codes this is negligible as ABS values provide absolute
data. That is, there is usually no need to resync ABS codes as we don't
need previous values to interpret the next ABS code. Furthermore, most ABS
codes are also sent pretty frequently so a refresh is usually useless.

However, with the introduction of slotted ABS axes we added a relative
component: ABS_MT_SLOT. If a client syncs all ABS codes via EVIOCGABS
while the kernel has ABS-events and an ABS_MT_SLOT event queued, the
client will think any read ABS-event is for the retrieved SLOT, however,
this is not true as all events until the next ABS_MT_SLOT event are for
the previously active slot:

    Kernel queue is: { ABS_DROPPED,
                       ABS_MT_POSITION_X(slot: 0),
                       ABS_MT_SLOT(slot: 1),
                       ABS_MT_POSITION_X(slot: 1) }
    Client reads ABS_DROPPED from queue.
    Client syncs all ABS values:
      As part of that, client syncs ABS_MT_SLOT, which is 1 in the current
      view of the kernel.
    Client reads ABS_MT_POSITION_X and attributes it to slot 1 instead of
    slot 0, as the slot-value is not explicit.

This is just a simple example how the relative information provided by the
ABS_MT_SLOT axis can be problematic to clients.

Now there are many ways to fix this:
 * Make ABS_MT_SLOT a per-evdev-client attribute. On each
   EVIOCGABS(ABS_MT_SLOT) we can add fake ABS_MT_SLOT events to the queue.
   => Ugly and overkill
 * Flush all ABS events when clients read ABS_MT_SLOT.
   => Ugly hack and client might loose important ABS_MT_* events
 * Provide atomic EVIOCGABS API.
   => Sounds good!

This patch introduces EVIOCGABSRANGE. Unlike EVIOCGABS, this ioctl only
fetches ABS values, rather than the whole "struct input_absinfo" set.
However, the new ioctl can fetch a range of ABS axes atomically and will
flush matching events from the client's receive queue. Moreover, you can
fetch all axes for *all* slots with a single call.

This way, a client can simply run EVIOCGABSRANGE(0, ABS_CNT) and it will
receive a consistent view of the whole ABS state, while the kernel flushes
the receive-buffer for a consistent view.
While most clients probably only need
EVIOCGABSRANGE(ABS_MT_SLOT, ABS_MT_TOOL_y - ABS_MT_SLOT + 1), the ioctl
allows to receive an arbitrary range of axes.

Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
 drivers/input/evdev.c      | 180 ++++++++++++++++++++++++++++++++++++++++++++-
 include/uapi/linux/input.h |  42 +++++++++++
 2 files changed, 218 insertions(+), 4 deletions(-)

diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
index 6386882..7a25a7a 100644
--- a/drivers/input/evdev.c
+++ b/drivers/input/evdev.c
@@ -175,8 +175,9 @@ static bool __evdev_is_filtered(struct evdev_client *client,
 	return mask && !test_bit(code, mask);
 }
 
-/* flush queued events of type @type, caller must hold client->buffer_lock */
-static void __evdev_flush_queue(struct evdev_client *client, unsigned int type)
+/* flush queued, matching events, caller must hold client->buffer_lock */
+static void __evdev_flush_queue(struct evdev_client *client, unsigned int type,
+				unsigned int code_first, unsigned int code_last)
 {
 	unsigned int i, head, num;
 	unsigned int mask = client->bufsize - 1;
@@ -195,7 +196,9 @@ static void __evdev_flush_queue(struct evdev_client *client, unsigned int type)
 		ev = &client->buffer[i];
 		is_report = ev->type == EV_SYN && ev->code == SYN_REPORT;
 
-		if (ev->type == type) {
+		if (ev->type == type &&
+		    ev->code >= code_first &&
+		    ev->code <= code_last) {
 			/* drop matched entry */
 			continue;
 		} else if (is_report && !num) {
@@ -786,6 +789,172 @@ static int handle_eviocgbit(struct input_dev *dev,
 	return bits_to_user(bits, len, size, p, compat_mode);
 }
 
+static inline void free_absrange(s32 **pages, size_t page_cnt)
+{
+	if (page_cnt > 1) {
+		while (page_cnt > 0) {
+			if (!pages[--page_cnt])
+				break;
+			__free_page(virt_to_page(pages[page_cnt]));
+		}
+		kfree(pages);
+	} else if (page_cnt == 1) {
+		kfree(pages);
+	}
+}
+
+static inline s32 *absrange_ptr(s32 **pages, size_t page_cnt, size_t slots,
+				size_t i_code, size_t j_slot)
+{
+	size_t idx, off;
+
+	idx = (i_code * slots + j_slot) / (PAGE_SIZE / sizeof(s32));
+	off = (i_code * slots + j_slot) % (PAGE_SIZE / sizeof(s32));
+
+	if (page_cnt == 1)
+		return &((s32*)pages)[off];
+	else
+		return &pages[idx][off];
+}
+
+static inline ssize_t fetch_absrange(struct evdev_client *client,
+				     struct input_dev *dev, size_t start,
+				     size_t count, size_t slots, s32 ***out)
+{
+	size_t size, page_cnt, i, j;
+	unsigned long flags;
+	s32 **pages;
+
+	/*
+	 * Fetch data atomically from the device and flush buffers. We need to
+	 * allocate a temporary buffer as copy_to_user() is not allowed while
+	 * holding spinlocks. However, to-be-copied data might be huge and
+	 * high-order allocations should be avoided. Therefore, do the
+	 * page-allocation manually.
+	 */
+
+	BUILD_BUG_ON(PAGE_SIZE % sizeof(s32) != 0);
+
+	size = sizeof(s32) * count * slots;
+	page_cnt = DIV_ROUND_UP(size, PAGE_SIZE);
+	if (page_cnt < 1) {
+		return 0;
+	} else if (page_cnt == 1) {
+		pages = kzalloc(size, GFP_TEMPORARY);
+		if (!pages)
+			return -ENOMEM;
+	} else {
+		pages = kzalloc(sizeof(*pages) * page_cnt, GFP_TEMPORARY);
+		if (!pages)
+			return -ENOMEM;
+
+		for (i = 0; i < page_cnt; ++i) {
+			pages[i] = (void*)get_zeroed_page(GFP_TEMPORARY);
+			if (!pages[i]) {
+				free_absrange(pages, page_cnt);
+				return -ENOMEM;
+			}
+		}
+	}
+
+	spin_lock_irqsave(&dev->event_lock, flags);
+	spin_lock(&client->buffer_lock);
+
+	for (i = 0; i < count; ++i) {
+		__u16 code;
+		bool is_mt;
+
+		code = start + i;
+		is_mt = input_is_mt_value(code);
+		if (is_mt && !dev->mt)
+			continue;
+
+		for (j = 0; j < slots; ++j) {
+			__s32 v;
+
+			if (is_mt)
+				v = input_mt_get_value(&dev->mt->slots[j],
+						       code);
+			else
+				v = dev->absinfo[code].value;
+
+			*absrange_ptr(pages, page_cnt, slots, i, j) = v;
+
+			if (!is_mt)
+				break;
+		}
+	}
+
+	spin_unlock(&client->buffer_lock);
+	__evdev_flush_queue(client, EV_ABS, start, start + count - 1);
+	spin_unlock_irqrestore(&dev->event_lock, flags);
+
+	*out = pages;
+	return page_cnt;
+}
+
+static int evdev_handle_get_absrange(struct evdev_client *client,
+				     struct input_dev *dev,
+				     struct input_absrange __user *p)
+{
+	size_t slots, code, count, i, j;
+	struct input_absrange absbuf;
+	s32 **vals = NULL;
+	ssize_t val_cnt;
+	s32 __user *b;
+	int retval;
+
+	if (!dev->absinfo)
+		return -EINVAL;
+	if (copy_from_user(&absbuf, p, sizeof(absbuf)))
+		return -EFAULT;
+
+	slots = min_t(size_t, dev->mt ? dev->mt->num_slots : 1, absbuf.slots);
+	code = min_t(size_t, absbuf.code, ABS_CNT);
+	count = min_t(size_t, absbuf.count, ABS_CNT);
+
+	/* first fetch data atomically from device */
+
+	if (code + count > ABS_CNT)
+		count = ABS_CNT - code;
+
+	if (!slots || !count) {
+		val_cnt = 0;
+	} else {
+		val_cnt = fetch_absrange(client, dev, code, count,
+					 slots, &vals);
+		if (val_cnt < 0)
+			return val_cnt;
+	}
+
+	/* now copy data to user-space */
+
+	b = (void __user*)(unsigned long)absbuf.buffer;
+	for (i = 0; i < absbuf.count; ++i) {
+		for (j = 0; j < absbuf.slots; ++j, ++b) {
+			s32 v;
+
+			if (i >= count || j >= slots)
+				v = 0;
+			else
+				v = *absrange_ptr(vals, val_cnt, slots, i, j);
+
+			if (put_user(v, b)) {
+				retval = -EFAULT;
+				goto out;
+			}
+		}
+	}
+
+	retval = 0;
+
+out:
+	free_absrange(vals, val_cnt);
+	if (retval < 0)
+		evdev_queue_syn_dropped(client);
+	return retval;
+}
+
 static int evdev_handle_get_keycode(struct input_dev *dev, void __user *p)
 {
 	struct input_keymap_entry ke = {
@@ -889,7 +1058,7 @@ static int evdev_handle_get_val(struct evdev_client *client,
 
 	spin_unlock(&dev->event_lock);
 
-	__evdev_flush_queue(client, type);
+	__evdev_flush_queue(client, type, 0, UINT_MAX);
 
 	spin_unlock_irq(&client->buffer_lock);
 
@@ -1006,6 +1175,9 @@ static long evdev_do_ioctl(struct file *file, unsigned int cmd,
 		else
 			return evdev_revoke(evdev, client, file);
 
+	case EVIOCGABSRANGE:
+		return evdev_handle_get_absrange(client, dev, p);
+
 	case EVIOCGMASK:
 		if (copy_from_user(&mask, p, sizeof(mask)))
 			return -EFAULT;
diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h
index f6ace0e..32a6443 100644
--- a/include/uapi/linux/input.h
+++ b/include/uapi/linux/input.h
@@ -210,6 +210,48 @@ struct input_mask {
  */
 #define EVIOCSMASK		_IOW('E', 0x93, struct input_mask)	/* Set event-masks */
 
+struct input_absrange {
+	__u16 slots;
+	__u16 code;
+	__u32 count;
+	__u64 buffer;
+};
+
+/**
+ * EVIOCGABSRANGE - Fetch range of ABS values
+ *
+ * This fetches the current values of a range of ABS codes atomically. The range
+ * of codes to fetch and the buffer-types are passed as "struct input_absrange",
+ * which has the following fields:
+ *      slots: Number of MT slots to fetch data for.
+ *       code: First ABS axis to query.
+ *      count: Number of ABS axes to query starting at @code.
+ *     buffer: Pointer to a receive buffer where to store the fetched ABS
+ *             values. This buffer must be an array of __s32 with at least
+ *             (@slots * @code) elements. The buffer is interpreted as two
+ *             dimensional __s32 array, declared as: __s32[slots][codes]
+ *
+ * Compared to EVIOCGABS this ioctl allows to retrieve a range of ABS codes
+ * atomically regarding any concurrent buffer modifications. Furthermore, any
+ * pending events for codes that were retrived via this call are flushed from
+ * the client's receive buffer. But unlike EVIOCGABS, this ioctl only returns
+ * the current value of an axis, rather than the whole "struct input_absinfo"
+ * set. All fields of "struct input_absinfo" except for the value are constant,
+ * though.
+ *
+ * The kernel's current view of the ABS axes is copied into the provided buffer.
+ * If an ABS axis is not enabled on the device, its value will be zero. Also, if
+ * an axis is not a slotted MT-axis, values for all but the first slot will be
+ * 0. If @slots is greater than the actual number of slots provided by the
+ * device, values for all slots higher than that will be 0.
+ *
+ * This call may fail with -EINVAL if the kernel doesn't support this call or
+ * the arguments are invalid, with -ENODEV if access was revoked, -ENOMEM if the
+ * kernel couldn't allocate temporary buffers for data-copy or -EFAULT if the
+ * passed pointer was invalid.
+ */
+#define EVIOCGABSRANGE		_IOR('E', 0x94, struct input_absrange)
+
 #define EVIOCSCLOCKID		_IOW('E', 0xa0, int)			/* Set clockid to be used for timestamps */
 
 /*
-- 
2.0.0


^ permalink raw reply related

* Re: [PATCH] Input: evdev - Fix incorrect kfree of err_free_client after vzalloc
From: David Herrmann @ 2014-06-20 14:14 UTC (permalink / raw)
  To: Yongtaek Lee
  Cc: Henrik Rydberg, Dmitry Torokhov, daniels,
	open list:HID CORE LAYER, linux-kernel
In-Reply-To: <1402537537-17945-1-git-send-email-ytk.lee@samsung.com>

Hi

On Thu, Jun 12, 2014 at 3:45 AM, Yongtaek Lee <ytk.lee@samsung.com> wrote:
> This bug was introduced by commit 92eb77d ("Input: evdev - fall back
> to vmalloc for client event buffer").
>
> vzalloc is used to alloc memory as fallback in case of failure
> of kzalloc. But err_free_client was not considered on below case.
> 1. kzalloc fail
> 2. vzalloc success
> 3. evdev_open_device fail
> 4. kfree
>
> So that address checking is needed to call correct free function.
>
> Signed-off-by: Yongtaek Lee <ytk.lee@samsung.com>
> Reviewed-by: Daniel Stone <daniels@collabora.com>

Reviewed-by: David Herrmann <dh.herrmann@gmail.com>

Thanks
David

> ---
>  drivers/input/evdev.c |    5 ++++-
>  1 files changed, 4 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
> index ce953d8..f60daa0 100644
> --- a/drivers/input/evdev.c
> +++ b/drivers/input/evdev.c
> @@ -422,7 +422,10 @@ static int evdev_open(struct inode *inode, struct file *file)
>
>   err_free_client:
>         evdev_detach_client(evdev, client);
> -       kfree(client);
> +       if (is_vmalloc_addr(client))
> +               vfree(client);
> +       else
> +               kfree(client);
>         return error;
>  }
>
> --
> 1.7.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [RFC] Input: evdev - add new EVIOCGABSRANGE ioctl
From: Benjamin Tissoires @ 2014-06-20 14:49 UTC (permalink / raw)
  To: David Herrmann; +Cc: linux-input, Dmitry Torokhov, Peter Hutterer
In-Reply-To: <1403273149-29224-1-git-send-email-dh.herrmann@gmail.com>

Hi David,

On Fri, Jun 20, 2014 at 10:05 AM, David Herrmann <dh.herrmann@gmail.com> wrote:
> When we introduced the slotted MT ABS extensions, we didn't take care to
> make SYN_DROPPED recoverable. Imagine a client recevies a SYN_DROPPED and
> syncs its current state via EVIOCGABS. It has to call this ioctl for each
> and every ABS code separately. Besides being horribly slow, this series
> of ioctl-calls is not atomic. The kernel might queue new ABS events while
> the client fetches the data.
>
> Now for normal ABS codes this is negligible as ABS values provide absolute
> data. That is, there is usually no need to resync ABS codes as we don't
> need previous values to interpret the next ABS code. Furthermore, most ABS
> codes are also sent pretty frequently so a refresh is usually useless.
>
> However, with the introduction of slotted ABS axes we added a relative
> component: ABS_MT_SLOT. If a client syncs all ABS codes via EVIOCGABS
> while the kernel has ABS-events and an ABS_MT_SLOT event queued, the
> client will think any read ABS-event is for the retrieved SLOT, however,
> this is not true as all events until the next ABS_MT_SLOT event are for
> the previously active slot:
>
>     Kernel queue is: { ABS_DROPPED,
>                        ABS_MT_POSITION_X(slot: 0),
>                        ABS_MT_SLOT(slot: 1),
>                        ABS_MT_POSITION_X(slot: 1) }
>     Client reads ABS_DROPPED from queue.
>     Client syncs all ABS values:
>       As part of that, client syncs ABS_MT_SLOT, which is 1 in the current
>       view of the kernel.
>     Client reads ABS_MT_POSITION_X and attributes it to slot 1 instead of
>     slot 0, as the slot-value is not explicit.
>
> This is just a simple example how the relative information provided by the
> ABS_MT_SLOT axis can be problematic to clients.
>
> Now there are many ways to fix this:
>  * Make ABS_MT_SLOT a per-evdev-client attribute. On each
>    EVIOCGABS(ABS_MT_SLOT) we can add fake ABS_MT_SLOT events to the queue.
>    => Ugly and overkill
>  * Flush all ABS events when clients read ABS_MT_SLOT.
>    => Ugly hack and client might loose important ABS_MT_* events
>  * Provide atomic EVIOCGABS API.
>    => Sounds good!
>
> This patch introduces EVIOCGABSRANGE. Unlike EVIOCGABS, this ioctl only
> fetches ABS values, rather than the whole "struct input_absinfo" set.
> However, the new ioctl can fetch a range of ABS axes atomically and will
> flush matching events from the client's receive queue. Moreover, you can
> fetch all axes for *all* slots with a single call.
>
> This way, a client can simply run EVIOCGABSRANGE(0, ABS_CNT) and it will
> receive a consistent view of the whole ABS state, while the kernel flushes
> the receive-buffer for a consistent view.
> While most clients probably only need
> EVIOCGABSRANGE(ABS_MT_SLOT, ABS_MT_TOOL_y - ABS_MT_SLOT + 1), the ioctl
> allows to receive an arbitrary range of axes.
>
> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
> ---

Thanks for this. This sounds very promising. I did not went into the
tiny details of the code, but I already have some comments. See below.

>  drivers/input/evdev.c      | 180 ++++++++++++++++++++++++++++++++++++++++++++-
>  include/uapi/linux/input.h |  42 +++++++++++
>  2 files changed, 218 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
> index 6386882..7a25a7a 100644
> --- a/drivers/input/evdev.c
> +++ b/drivers/input/evdev.c
> @@ -175,8 +175,9 @@ static bool __evdev_is_filtered(struct evdev_client *client,
>         return mask && !test_bit(code, mask);
>  }
>
> -/* flush queued events of type @type, caller must hold client->buffer_lock */
> -static void __evdev_flush_queue(struct evdev_client *client, unsigned int type)
> +/* flush queued, matching events, caller must hold client->buffer_lock */
> +static void __evdev_flush_queue(struct evdev_client *client, unsigned int type,
> +                               unsigned int code_first, unsigned int code_last)
>  {
>         unsigned int i, head, num;
>         unsigned int mask = client->bufsize - 1;
> @@ -195,7 +196,9 @@ static void __evdev_flush_queue(struct evdev_client *client, unsigned int type)
>                 ev = &client->buffer[i];
>                 is_report = ev->type == EV_SYN && ev->code == SYN_REPORT;
>
> -               if (ev->type == type) {
> +               if (ev->type == type &&
> +                   ev->code >= code_first &&
> +                   ev->code <= code_last) {
>                         /* drop matched entry */
>                         continue;
>                 } else if (is_report && !num) {
> @@ -786,6 +789,172 @@ static int handle_eviocgbit(struct input_dev *dev,
>         return bits_to_user(bits, len, size, p, compat_mode);
>  }
>
> +static inline void free_absrange(s32 **pages, size_t page_cnt)
> +{
> +       if (page_cnt > 1) {
> +               while (page_cnt > 0) {
> +                       if (!pages[--page_cnt])
> +                               break;
> +                       __free_page(virt_to_page(pages[page_cnt]));
> +               }
> +               kfree(pages);
> +       } else if (page_cnt == 1) {
> +               kfree(pages);
> +       }
> +}
> +
> +static inline s32 *absrange_ptr(s32 **pages, size_t page_cnt, size_t slots,
> +                               size_t i_code, size_t j_slot)
> +{
> +       size_t idx, off;
> +
> +       idx = (i_code * slots + j_slot) / (PAGE_SIZE / sizeof(s32));
> +       off = (i_code * slots + j_slot) % (PAGE_SIZE / sizeof(s32));
> +
> +       if (page_cnt == 1)
> +               return &((s32*)pages)[off];
> +       else
> +               return &pages[idx][off];
> +}
> +
> +static inline ssize_t fetch_absrange(struct evdev_client *client,
> +                                    struct input_dev *dev, size_t start,
> +                                    size_t count, size_t slots, s32 ***out)
> +{
> +       size_t size, page_cnt, i, j;
> +       unsigned long flags;
> +       s32 **pages;
> +
> +       /*
> +        * Fetch data atomically from the device and flush buffers. We need to
> +        * allocate a temporary buffer as copy_to_user() is not allowed while
> +        * holding spinlocks. However, to-be-copied data might be huge and
> +        * high-order allocations should be avoided. Therefore, do the
> +        * page-allocation manually.
> +        */
> +
> +       BUILD_BUG_ON(PAGE_SIZE % sizeof(s32) != 0);
> +
> +       size = sizeof(s32) * count * slots;
> +       page_cnt = DIV_ROUND_UP(size, PAGE_SIZE);
> +       if (page_cnt < 1) {
> +               return 0;
> +       } else if (page_cnt == 1) {
> +               pages = kzalloc(size, GFP_TEMPORARY);
> +               if (!pages)
> +                       return -ENOMEM;
> +       } else {
> +               pages = kzalloc(sizeof(*pages) * page_cnt, GFP_TEMPORARY);
> +               if (!pages)
> +                       return -ENOMEM;
> +
> +               for (i = 0; i < page_cnt; ++i) {
> +                       pages[i] = (void*)get_zeroed_page(GFP_TEMPORARY);
> +                       if (!pages[i]) {
> +                               free_absrange(pages, page_cnt);
> +                               return -ENOMEM;
> +                       }
> +               }
> +       }
> +
> +       spin_lock_irqsave(&dev->event_lock, flags);
> +       spin_lock(&client->buffer_lock);
> +
> +       for (i = 0; i < count; ++i) {
> +               __u16 code;
> +               bool is_mt;
> +
> +               code = start + i;
> +               is_mt = input_is_mt_value(code);
> +               if (is_mt && !dev->mt)
> +                       continue;
> +
> +               for (j = 0; j < slots; ++j) {
> +                       __s32 v;
> +
> +                       if (is_mt)
> +                               v = input_mt_get_value(&dev->mt->slots[j],
> +                                                      code);
> +                       else
> +                               v = dev->absinfo[code].value;
> +
> +                       *absrange_ptr(pages, page_cnt, slots, i, j) = v;
> +
> +                       if (!is_mt)
> +                               break;
> +               }
> +       }
> +
> +       spin_unlock(&client->buffer_lock);
> +       __evdev_flush_queue(client, EV_ABS, start, start + count - 1);
> +       spin_unlock_irqrestore(&dev->event_lock, flags);
> +
> +       *out = pages;
> +       return page_cnt;
> +}
> +
> +static int evdev_handle_get_absrange(struct evdev_client *client,
> +                                    struct input_dev *dev,
> +                                    struct input_absrange __user *p)
> +{
> +       size_t slots, code, count, i, j;
> +       struct input_absrange absbuf;
> +       s32 **vals = NULL;
> +       ssize_t val_cnt;
> +       s32 __user *b;
> +       int retval;
> +
> +       if (!dev->absinfo)
> +               return -EINVAL;
> +       if (copy_from_user(&absbuf, p, sizeof(absbuf)))
> +               return -EFAULT;
> +
> +       slots = min_t(size_t, dev->mt ? dev->mt->num_slots : 1, absbuf.slots);
> +       code = min_t(size_t, absbuf.code, ABS_CNT);
> +       count = min_t(size_t, absbuf.count, ABS_CNT);
> +
> +       /* first fetch data atomically from device */
> +
> +       if (code + count > ABS_CNT)
> +               count = ABS_CNT - code;
> +
> +       if (!slots || !count) {
> +               val_cnt = 0;
> +       } else {
> +               val_cnt = fetch_absrange(client, dev, code, count,
> +                                        slots, &vals);
> +               if (val_cnt < 0)
> +                       return val_cnt;
> +       }
> +
> +       /* now copy data to user-space */
> +
> +       b = (void __user*)(unsigned long)absbuf.buffer;

What if the user space buffer is not long enough, or if the pointer is
invalid? Is there any means from the kernel to guarantee that we are
not writing in a restricted memory area or in a buffer not owned by
the process?

> +       for (i = 0; i < absbuf.count; ++i) {
> +               for (j = 0; j < absbuf.slots; ++j, ++b) {
> +                       s32 v;
> +
> +                       if (i >= count || j >= slots)
> +                               v = 0;
> +                       else
> +                               v = *absrange_ptr(vals, val_cnt, slots, i, j);
> +
> +                       if (put_user(v, b)) {
> +                               retval = -EFAULT;
> +                               goto out;
> +                       }
> +               }
> +       }
> +

Shouldn't we also call free_absrange(vals, val_cnt); before returning?

> +       retval = 0;

Not sure it matters a lot, but returning the size of what has been
written is more common. This would make sense if the buffer is not
long enough or if it is too big.

Cheers,
Benjamin

> +
> +out:
> +       free_absrange(vals, val_cnt);
> +       if (retval < 0)
> +               evdev_queue_syn_dropped(client);
> +       return retval;
> +}
> +
>  static int evdev_handle_get_keycode(struct input_dev *dev, void __user *p)
>  {
>         struct input_keymap_entry ke = {
> @@ -889,7 +1058,7 @@ static int evdev_handle_get_val(struct evdev_client *client,
>
>         spin_unlock(&dev->event_lock);
>
> -       __evdev_flush_queue(client, type);
> +       __evdev_flush_queue(client, type, 0, UINT_MAX);
>
>         spin_unlock_irq(&client->buffer_lock);
>
> @@ -1006,6 +1175,9 @@ static long evdev_do_ioctl(struct file *file, unsigned int cmd,
>                 else
>                         return evdev_revoke(evdev, client, file);
>
> +       case EVIOCGABSRANGE:
> +               return evdev_handle_get_absrange(client, dev, p);
> +
>         case EVIOCGMASK:
>                 if (copy_from_user(&mask, p, sizeof(mask)))
>                         return -EFAULT;
> diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h
> index f6ace0e..32a6443 100644
> --- a/include/uapi/linux/input.h
> +++ b/include/uapi/linux/input.h
> @@ -210,6 +210,48 @@ struct input_mask {
>   */
>  #define EVIOCSMASK             _IOW('E', 0x93, struct input_mask)      /* Set event-masks */
>
> +struct input_absrange {
> +       __u16 slots;
> +       __u16 code;
> +       __u32 count;
> +       __u64 buffer;
> +};
> +
> +/**
> + * EVIOCGABSRANGE - Fetch range of ABS values
> + *
> + * This fetches the current values of a range of ABS codes atomically. The range
> + * of codes to fetch and the buffer-types are passed as "struct input_absrange",
> + * which has the following fields:
> + *      slots: Number of MT slots to fetch data for.
> + *       code: First ABS axis to query.
> + *      count: Number of ABS axes to query starting at @code.
> + *     buffer: Pointer to a receive buffer where to store the fetched ABS
> + *             values. This buffer must be an array of __s32 with at least
> + *             (@slots * @code) elements. The buffer is interpreted as two
> + *             dimensional __s32 array, declared as: __s32[slots][codes]
> + *
> + * Compared to EVIOCGABS this ioctl allows to retrieve a range of ABS codes
> + * atomically regarding any concurrent buffer modifications. Furthermore, any
> + * pending events for codes that were retrived via this call are flushed from
> + * the client's receive buffer. But unlike EVIOCGABS, this ioctl only returns
> + * the current value of an axis, rather than the whole "struct input_absinfo"
> + * set. All fields of "struct input_absinfo" except for the value are constant,
> + * though.
> + *
> + * The kernel's current view of the ABS axes is copied into the provided buffer.
> + * If an ABS axis is not enabled on the device, its value will be zero. Also, if
> + * an axis is not a slotted MT-axis, values for all but the first slot will be
> + * 0. If @slots is greater than the actual number of slots provided by the
> + * device, values for all slots higher than that will be 0.
> + *
> + * This call may fail with -EINVAL if the kernel doesn't support this call or
> + * the arguments are invalid, with -ENODEV if access was revoked, -ENOMEM if the
> + * kernel couldn't allocate temporary buffers for data-copy or -EFAULT if the
> + * passed pointer was invalid.
> + */
> +#define EVIOCGABSRANGE         _IOR('E', 0x94, struct input_absrange)
> +
>  #define EVIOCSCLOCKID          _IOW('E', 0xa0, int)                    /* Set clockid to be used for timestamps */
>
>  /*
> --
> 2.0.0
>

^ permalink raw reply

* Re: [RFC] Input: evdev - add new EVIOCGABSRANGE ioctl
From: David Herrmann @ 2014-06-20 14:59 UTC (permalink / raw)
  To: Benjamin Tissoires; +Cc: linux-input, Dmitry Torokhov, Peter Hutterer
In-Reply-To: <CAN+gG=H9n5Sun2nrVATu79MxS4p4xqdGncWEB-6a+7HCWx_bsA@mail.gmail.com>

Hi

On Fri, Jun 20, 2014 at 4:49 PM, Benjamin Tissoires
<benjamin.tissoires@gmail.com> wrote:
> Hi David,
>
> On Fri, Jun 20, 2014 at 10:05 AM, David Herrmann <dh.herrmann@gmail.com> wrote:
>> When we introduced the slotted MT ABS extensions, we didn't take care to
>> make SYN_DROPPED recoverable. Imagine a client recevies a SYN_DROPPED and
>> syncs its current state via EVIOCGABS. It has to call this ioctl for each
>> and every ABS code separately. Besides being horribly slow, this series
>> of ioctl-calls is not atomic. The kernel might queue new ABS events while
>> the client fetches the data.
>>
>> Now for normal ABS codes this is negligible as ABS values provide absolute
>> data. That is, there is usually no need to resync ABS codes as we don't
>> need previous values to interpret the next ABS code. Furthermore, most ABS
>> codes are also sent pretty frequently so a refresh is usually useless.
>>
>> However, with the introduction of slotted ABS axes we added a relative
>> component: ABS_MT_SLOT. If a client syncs all ABS codes via EVIOCGABS
>> while the kernel has ABS-events and an ABS_MT_SLOT event queued, the
>> client will think any read ABS-event is for the retrieved SLOT, however,
>> this is not true as all events until the next ABS_MT_SLOT event are for
>> the previously active slot:
>>
>>     Kernel queue is: { ABS_DROPPED,
>>                        ABS_MT_POSITION_X(slot: 0),
>>                        ABS_MT_SLOT(slot: 1),
>>                        ABS_MT_POSITION_X(slot: 1) }
>>     Client reads ABS_DROPPED from queue.
>>     Client syncs all ABS values:
>>       As part of that, client syncs ABS_MT_SLOT, which is 1 in the current
>>       view of the kernel.
>>     Client reads ABS_MT_POSITION_X and attributes it to slot 1 instead of
>>     slot 0, as the slot-value is not explicit.
>>
>> This is just a simple example how the relative information provided by the
>> ABS_MT_SLOT axis can be problematic to clients.
>>
>> Now there are many ways to fix this:
>>  * Make ABS_MT_SLOT a per-evdev-client attribute. On each
>>    EVIOCGABS(ABS_MT_SLOT) we can add fake ABS_MT_SLOT events to the queue.
>>    => Ugly and overkill
>>  * Flush all ABS events when clients read ABS_MT_SLOT.
>>    => Ugly hack and client might loose important ABS_MT_* events
>>  * Provide atomic EVIOCGABS API.
>>    => Sounds good!
>>
>> This patch introduces EVIOCGABSRANGE. Unlike EVIOCGABS, this ioctl only
>> fetches ABS values, rather than the whole "struct input_absinfo" set.
>> However, the new ioctl can fetch a range of ABS axes atomically and will
>> flush matching events from the client's receive queue. Moreover, you can
>> fetch all axes for *all* slots with a single call.
>>
>> This way, a client can simply run EVIOCGABSRANGE(0, ABS_CNT) and it will
>> receive a consistent view of the whole ABS state, while the kernel flushes
>> the receive-buffer for a consistent view.
>> While most clients probably only need
>> EVIOCGABSRANGE(ABS_MT_SLOT, ABS_MT_TOOL_y - ABS_MT_SLOT + 1), the ioctl
>> allows to receive an arbitrary range of axes.
>>
>> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
>> ---
>
> Thanks for this. This sounds very promising. I did not went into the
> tiny details of the code, but I already have some comments. See below.

Thanks for the comments!

>>  drivers/input/evdev.c      | 180 ++++++++++++++++++++++++++++++++++++++++++++-
>>  include/uapi/linux/input.h |  42 +++++++++++
>>  2 files changed, 218 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
>> index 6386882..7a25a7a 100644
>> --- a/drivers/input/evdev.c
>> +++ b/drivers/input/evdev.c
>> @@ -175,8 +175,9 @@ static bool __evdev_is_filtered(struct evdev_client *client,
>>         return mask && !test_bit(code, mask);
>>  }
>>
>> -/* flush queued events of type @type, caller must hold client->buffer_lock */
>> -static void __evdev_flush_queue(struct evdev_client *client, unsigned int type)
>> +/* flush queued, matching events, caller must hold client->buffer_lock */
>> +static void __evdev_flush_queue(struct evdev_client *client, unsigned int type,
>> +                               unsigned int code_first, unsigned int code_last)
>>  {
>>         unsigned int i, head, num;
>>         unsigned int mask = client->bufsize - 1;
>> @@ -195,7 +196,9 @@ static void __evdev_flush_queue(struct evdev_client *client, unsigned int type)
>>                 ev = &client->buffer[i];
>>                 is_report = ev->type == EV_SYN && ev->code == SYN_REPORT;
>>
>> -               if (ev->type == type) {
>> +               if (ev->type == type &&
>> +                   ev->code >= code_first &&
>> +                   ev->code <= code_last) {
>>                         /* drop matched entry */
>>                         continue;
>>                 } else if (is_report && !num) {
>> @@ -786,6 +789,172 @@ static int handle_eviocgbit(struct input_dev *dev,
>>         return bits_to_user(bits, len, size, p, compat_mode);
>>  }
>>
>> +static inline void free_absrange(s32 **pages, size_t page_cnt)
>> +{
>> +       if (page_cnt > 1) {
>> +               while (page_cnt > 0) {
>> +                       if (!pages[--page_cnt])
>> +                               break;
>> +                       __free_page(virt_to_page(pages[page_cnt]));
>> +               }
>> +               kfree(pages);
>> +       } else if (page_cnt == 1) {
>> +               kfree(pages);
>> +       }
>> +}
>> +
>> +static inline s32 *absrange_ptr(s32 **pages, size_t page_cnt, size_t slots,
>> +                               size_t i_code, size_t j_slot)
>> +{
>> +       size_t idx, off;
>> +
>> +       idx = (i_code * slots + j_slot) / (PAGE_SIZE / sizeof(s32));
>> +       off = (i_code * slots + j_slot) % (PAGE_SIZE / sizeof(s32));
>> +
>> +       if (page_cnt == 1)
>> +               return &((s32*)pages)[off];
>> +       else
>> +               return &pages[idx][off];
>> +}
>> +
>> +static inline ssize_t fetch_absrange(struct evdev_client *client,
>> +                                    struct input_dev *dev, size_t start,
>> +                                    size_t count, size_t slots, s32 ***out)
>> +{
>> +       size_t size, page_cnt, i, j;
>> +       unsigned long flags;
>> +       s32 **pages;
>> +
>> +       /*
>> +        * Fetch data atomically from the device and flush buffers. We need to
>> +        * allocate a temporary buffer as copy_to_user() is not allowed while
>> +        * holding spinlocks. However, to-be-copied data might be huge and
>> +        * high-order allocations should be avoided. Therefore, do the
>> +        * page-allocation manually.
>> +        */
>> +
>> +       BUILD_BUG_ON(PAGE_SIZE % sizeof(s32) != 0);
>> +
>> +       size = sizeof(s32) * count * slots;
>> +       page_cnt = DIV_ROUND_UP(size, PAGE_SIZE);
>> +       if (page_cnt < 1) {
>> +               return 0;
>> +       } else if (page_cnt == 1) {
>> +               pages = kzalloc(size, GFP_TEMPORARY);
>> +               if (!pages)
>> +                       return -ENOMEM;
>> +       } else {
>> +               pages = kzalloc(sizeof(*pages) * page_cnt, GFP_TEMPORARY);
>> +               if (!pages)
>> +                       return -ENOMEM;
>> +
>> +               for (i = 0; i < page_cnt; ++i) {
>> +                       pages[i] = (void*)get_zeroed_page(GFP_TEMPORARY);
>> +                       if (!pages[i]) {
>> +                               free_absrange(pages, page_cnt);
>> +                               return -ENOMEM;
>> +                       }
>> +               }
>> +       }
>> +
>> +       spin_lock_irqsave(&dev->event_lock, flags);
>> +       spin_lock(&client->buffer_lock);
>> +
>> +       for (i = 0; i < count; ++i) {
>> +               __u16 code;
>> +               bool is_mt;
>> +
>> +               code = start + i;
>> +               is_mt = input_is_mt_value(code);
>> +               if (is_mt && !dev->mt)
>> +                       continue;
>> +
>> +               for (j = 0; j < slots; ++j) {
>> +                       __s32 v;
>> +
>> +                       if (is_mt)
>> +                               v = input_mt_get_value(&dev->mt->slots[j],
>> +                                                      code);
>> +                       else
>> +                               v = dev->absinfo[code].value;
>> +
>> +                       *absrange_ptr(pages, page_cnt, slots, i, j) = v;
>> +
>> +                       if (!is_mt)
>> +                               break;
>> +               }
>> +       }
>> +
>> +       spin_unlock(&client->buffer_lock);
>> +       __evdev_flush_queue(client, EV_ABS, start, start + count - 1);
>> +       spin_unlock_irqrestore(&dev->event_lock, flags);
>> +
>> +       *out = pages;
>> +       return page_cnt;
>> +}
>> +
>> +static int evdev_handle_get_absrange(struct evdev_client *client,
>> +                                    struct input_dev *dev,
>> +                                    struct input_absrange __user *p)
>> +{
>> +       size_t slots, code, count, i, j;
>> +       struct input_absrange absbuf;
>> +       s32 **vals = NULL;
>> +       ssize_t val_cnt;
>> +       s32 __user *b;
>> +       int retval;
>> +
>> +       if (!dev->absinfo)
>> +               return -EINVAL;
>> +       if (copy_from_user(&absbuf, p, sizeof(absbuf)))
>> +               return -EFAULT;
>> +
>> +       slots = min_t(size_t, dev->mt ? dev->mt->num_slots : 1, absbuf.slots);
>> +       code = min_t(size_t, absbuf.code, ABS_CNT);
>> +       count = min_t(size_t, absbuf.count, ABS_CNT);
>> +
>> +       /* first fetch data atomically from device */
>> +
>> +       if (code + count > ABS_CNT)
>> +               count = ABS_CNT - code;
>> +
>> +       if (!slots || !count) {
>> +               val_cnt = 0;
>> +       } else {
>> +               val_cnt = fetch_absrange(client, dev, code, count,
>> +                                        slots, &vals);
>> +               if (val_cnt < 0)
>> +                       return val_cnt;
>> +       }
>> +
>> +       /* now copy data to user-space */
>> +
>> +       b = (void __user*)(unsigned long)absbuf.buffer;
>
> What if the user space buffer is not long enough, or if the pointer is
> invalid? Is there any means from the kernel to guarantee that we are
> not writing in a restricted memory area or in a buffer not owned by
> the process?

The buffer is given as pointer in absbuf.buffer, the size of the
buffer is given as absbuf.slots * absbuf.count. This is the same as
for the read() syscall, where we get a pointer plus the size.
I use put_user() to write data into that buffer, so in case it's not
valid user-memory, this will fail with -EFAULT.

This is no different from other calls that return data, apart from
splitting the size into two ints "slots" and "count". So I cannot
follow what you mean? Note that "put_user()" is equivalent to
"copy_to_user()", maybe you missed that part?

>> +       for (i = 0; i < absbuf.count; ++i) {
>> +               for (j = 0; j < absbuf.slots; ++j, ++b) {
>> +                       s32 v;
>> +
>> +                       if (i >= count || j >= slots)
>> +                               v = 0;
>> +                       else
>> +                               v = *absrange_ptr(vals, val_cnt, slots, i, j);
>> +
>> +                       if (put_user(v, b)) {
>> +                               retval = -EFAULT;
>> +                               goto out;
>> +                       }
>> +               }
>> +       }
>> +
>
> Shouldn't we also call free_absrange(vals, val_cnt); before returning?

I do. There's a fall-through to "out:", so we always free the buffer.
Otherwise, I would have called it "error:" :)

>> +       retval = 0;
>
> Not sure it matters a lot, but returning the size of what has been
> written is more common. This would make sense if the buffer is not
> long enough or if it is too big.

This would always be "absbuf.slots * absbuf.count". I don't think
there's much gain in returning a constant size, is there?

Thanks!
David

>
>> +
>> +out:
>> +       free_absrange(vals, val_cnt);
>> +       if (retval < 0)
>> +               evdev_queue_syn_dropped(client);
>> +       return retval;
>> +}
>> +
>>  static int evdev_handle_get_keycode(struct input_dev *dev, void __user *p)
>>  {
>>         struct input_keymap_entry ke = {
>> @@ -889,7 +1058,7 @@ static int evdev_handle_get_val(struct evdev_client *client,
>>
>>         spin_unlock(&dev->event_lock);
>>
>> -       __evdev_flush_queue(client, type);
>> +       __evdev_flush_queue(client, type, 0, UINT_MAX);
>>
>>         spin_unlock_irq(&client->buffer_lock);
>>
>> @@ -1006,6 +1175,9 @@ static long evdev_do_ioctl(struct file *file, unsigned int cmd,
>>                 else
>>                         return evdev_revoke(evdev, client, file);
>>
>> +       case EVIOCGABSRANGE:
>> +               return evdev_handle_get_absrange(client, dev, p);
>> +
>>         case EVIOCGMASK:
>>                 if (copy_from_user(&mask, p, sizeof(mask)))
>>                         return -EFAULT;
>> diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h
>> index f6ace0e..32a6443 100644
>> --- a/include/uapi/linux/input.h
>> +++ b/include/uapi/linux/input.h
>> @@ -210,6 +210,48 @@ struct input_mask {
>>   */
>>  #define EVIOCSMASK             _IOW('E', 0x93, struct input_mask)      /* Set event-masks */
>>
>> +struct input_absrange {
>> +       __u16 slots;
>> +       __u16 code;
>> +       __u32 count;
>> +       __u64 buffer;
>> +};
>> +
>> +/**
>> + * EVIOCGABSRANGE - Fetch range of ABS values
>> + *
>> + * This fetches the current values of a range of ABS codes atomically. The range
>> + * of codes to fetch and the buffer-types are passed as "struct input_absrange",
>> + * which has the following fields:
>> + *      slots: Number of MT slots to fetch data for.
>> + *       code: First ABS axis to query.
>> + *      count: Number of ABS axes to query starting at @code.
>> + *     buffer: Pointer to a receive buffer where to store the fetched ABS
>> + *             values. This buffer must be an array of __s32 with at least
>> + *             (@slots * @code) elements. The buffer is interpreted as two
>> + *             dimensional __s32 array, declared as: __s32[slots][codes]
>> + *
>> + * Compared to EVIOCGABS this ioctl allows to retrieve a range of ABS codes
>> + * atomically regarding any concurrent buffer modifications. Furthermore, any
>> + * pending events for codes that were retrived via this call are flushed from
>> + * the client's receive buffer. But unlike EVIOCGABS, this ioctl only returns
>> + * the current value of an axis, rather than the whole "struct input_absinfo"
>> + * set. All fields of "struct input_absinfo" except for the value are constant,
>> + * though.
>> + *
>> + * The kernel's current view of the ABS axes is copied into the provided buffer.
>> + * If an ABS axis is not enabled on the device, its value will be zero. Also, if
>> + * an axis is not a slotted MT-axis, values for all but the first slot will be
>> + * 0. If @slots is greater than the actual number of slots provided by the
>> + * device, values for all slots higher than that will be 0.
>> + *
>> + * This call may fail with -EINVAL if the kernel doesn't support this call or
>> + * the arguments are invalid, with -ENODEV if access was revoked, -ENOMEM if the
>> + * kernel couldn't allocate temporary buffers for data-copy or -EFAULT if the
>> + * passed pointer was invalid.
>> + */
>> +#define EVIOCGABSRANGE         _IOR('E', 0x94, struct input_absrange)
>> +
>>  #define EVIOCSCLOCKID          _IOW('E', 0xa0, int)                    /* Set clockid to be used for timestamps */
>>
>>  /*
>> --
>> 2.0.0
>>

^ permalink raw reply

* Re: [RFC] Input: evdev - add new EVIOCGABSRANGE ioctl
From: Benjamin Tissoires @ 2014-06-20 15:53 UTC (permalink / raw)
  To: David Herrmann; +Cc: linux-input, Dmitry Torokhov, Peter Hutterer
In-Reply-To: <CANq1E4RPZTj8Y+PrNNzYfEvn-K6fer_ZCxutCeyFVi8cgCr=pw@mail.gmail.com>

On Fri, Jun 20, 2014 at 10:59 AM, David Herrmann <dh.herrmann@gmail.com> wrote:
> Hi
>
> On Fri, Jun 20, 2014 at 4:49 PM, Benjamin Tissoires
> <benjamin.tissoires@gmail.com> wrote:
>> Hi David,
>>
[stripped]
>>> +
>>> +static int evdev_handle_get_absrange(struct evdev_client *client,
>>> +                                    struct input_dev *dev,
>>> +                                    struct input_absrange __user *p)
>>> +{
>>> +       size_t slots, code, count, i, j;
>>> +       struct input_absrange absbuf;
>>> +       s32 **vals = NULL;
>>> +       ssize_t val_cnt;
>>> +       s32 __user *b;
>>> +       int retval;
>>> +
>>> +       if (!dev->absinfo)
>>> +               return -EINVAL;
>>> +       if (copy_from_user(&absbuf, p, sizeof(absbuf)))
>>> +               return -EFAULT;
>>> +
>>> +       slots = min_t(size_t, dev->mt ? dev->mt->num_slots : 1, absbuf.slots);
>>> +       code = min_t(size_t, absbuf.code, ABS_CNT);
>>> +       count = min_t(size_t, absbuf.count, ABS_CNT);
>>> +
>>> +       /* first fetch data atomically from device */
>>> +
>>> +       if (code + count > ABS_CNT)
>>> +               count = ABS_CNT - code;
>>> +
>>> +       if (!slots || !count) {
>>> +               val_cnt = 0;
>>> +       } else {
>>> +               val_cnt = fetch_absrange(client, dev, code, count,
>>> +                                        slots, &vals);
>>> +               if (val_cnt < 0)
>>> +                       return val_cnt;
>>> +       }
>>> +
>>> +       /* now copy data to user-space */
>>> +
>>> +       b = (void __user*)(unsigned long)absbuf.buffer;
>>
>> What if the user space buffer is not long enough, or if the pointer is
>> invalid? Is there any means from the kernel to guarantee that we are
>> not writing in a restricted memory area or in a buffer not owned by
>> the process?
>
> The buffer is given as pointer in absbuf.buffer, the size of the
> buffer is given as absbuf.slots * absbuf.count. This is the same as
> for the read() syscall, where we get a pointer plus the size.
> I use put_user() to write data into that buffer, so in case it's not
> valid user-memory, this will fail with -EFAULT.
>
> This is no different from other calls that return data, apart from
> splitting the size into two ints "slots" and "count". So I cannot
> follow what you mean? Note that "put_user()" is equivalent to
> "copy_to_user()", maybe you missed that part?

Well, my concern here is not to have a CVE in 2 months time, when the
kernel is out. So I prefer being cautious when dealing with user /
kernel exchanges.

And yes, you are right, put_user will check for the validity of the
pointer for each data you send in it.

>
>>> +       for (i = 0; i < absbuf.count; ++i) {
>>> +               for (j = 0; j < absbuf.slots; ++j, ++b) {
>>> +                       s32 v;
>>> +
>>> +                       if (i >= count || j >= slots)
>>> +                               v = 0;
>>> +                       else
>>> +                               v = *absrange_ptr(vals, val_cnt, slots, i, j);
>>> +
>>> +                       if (put_user(v, b)) {

>From what I read in include/asm-generic/uaccess.h put_user does not change "b".
Aren't you missing a b++ ? (but maybe I should just stop reviewing
this today because I may not have the eyes completely opened today...)

>>> +                               retval = -EFAULT;
>>> +                               goto out;
>>> +                       }
>>> +               }
>>> +       }
>>> +
>>
>> Shouldn't we also call free_absrange(vals, val_cnt); before returning?
>
> I do. There's a fall-through to "out:", so we always free the buffer.
> Otherwise, I would have called it "error:" :)

Oops, my bad, I read the next statement as "return 0" :o)

>
>>> +       retval = 0;
>>
>> Not sure it matters a lot, but returning the size of what has been
>> written is more common. This would make sense if the buffer is not
>> long enough or if it is too big.
>
> This would always be "absbuf.slots * absbuf.count". I don't think
> there's much gain in returning a constant size, is there?
>

With the current code, if the buffer is not long enough, you return
-EFAULT. However, one can argue that we can simply return the current
count of valid written data (especially because the data have been
dropped from the event queue).

Also, returning the actual written data may help in two corner cases
(when the programmer made a mistake, but programmers make mistakes):
- if count or slots is null -> the return value will be 0 (success),
whereas nothing happened
- if the allocated buffer is bigger than what is required -> a lazy
programmer will consider the whole buffer being valid, whereas only
the first bytes have been written (it happened to me, not in this
case, but still).

It's not a matter of returning a constant, it's a matter of notifying
how many data have been forwarded to the user space. But yes, for the
general case, the user space will now which value will be returned.
(this is just my personal taste, and maybe others will prefer the 0)

Cheers,
Benjamin

>
>>
>>> +
>>> +out:
>>> +       free_absrange(vals, val_cnt);
>>> +       if (retval < 0)
>>> +               evdev_queue_syn_dropped(client);
>>> +       return retval;
>>> +}
>>> +
>>>  static int evdev_handle_get_keycode(struct input_dev *dev, void __user *p)
>>>  {
>>>         struct input_keymap_entry ke = {
>>> @@ -889,7 +1058,7 @@ static int evdev_handle_get_val(struct evdev_client *client,
>>>
>>>         spin_unlock(&dev->event_lock);
>>>
>>> -       __evdev_flush_queue(client, type);
>>> +       __evdev_flush_queue(client, type, 0, UINT_MAX);
>>>
>>>         spin_unlock_irq(&client->buffer_lock);
>>>
>>> @@ -1006,6 +1175,9 @@ static long evdev_do_ioctl(struct file *file, unsigned int cmd,
>>>                 else
>>>                         return evdev_revoke(evdev, client, file);
>>>
>>> +       case EVIOCGABSRANGE:
>>> +               return evdev_handle_get_absrange(client, dev, p);
>>> +
>>>         case EVIOCGMASK:
>>>                 if (copy_from_user(&mask, p, sizeof(mask)))
>>>                         return -EFAULT;
>>> diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h
>>> index f6ace0e..32a6443 100644
>>> --- a/include/uapi/linux/input.h
>>> +++ b/include/uapi/linux/input.h
>>> @@ -210,6 +210,48 @@ struct input_mask {
>>>   */
>>>  #define EVIOCSMASK             _IOW('E', 0x93, struct input_mask)      /* Set event-masks */
>>>
>>> +struct input_absrange {
>>> +       __u16 slots;
>>> +       __u16 code;
>>> +       __u32 count;
>>> +       __u64 buffer;
>>> +};
>>> +
>>> +/**
>>> + * EVIOCGABSRANGE - Fetch range of ABS values
>>> + *
>>> + * This fetches the current values of a range of ABS codes atomically. The range
>>> + * of codes to fetch and the buffer-types are passed as "struct input_absrange",
>>> + * which has the following fields:
>>> + *      slots: Number of MT slots to fetch data for.
>>> + *       code: First ABS axis to query.
>>> + *      count: Number of ABS axes to query starting at @code.
>>> + *     buffer: Pointer to a receive buffer where to store the fetched ABS
>>> + *             values. This buffer must be an array of __s32 with at least
>>> + *             (@slots * @code) elements. The buffer is interpreted as two
>>> + *             dimensional __s32 array, declared as: __s32[slots][codes]
>>> + *
>>> + * Compared to EVIOCGABS this ioctl allows to retrieve a range of ABS codes
>>> + * atomically regarding any concurrent buffer modifications. Furthermore, any
>>> + * pending events for codes that were retrived via this call are flushed from
>>> + * the client's receive buffer. But unlike EVIOCGABS, this ioctl only returns
>>> + * the current value of an axis, rather than the whole "struct input_absinfo"
>>> + * set. All fields of "struct input_absinfo" except for the value are constant,
>>> + * though.
>>> + *
>>> + * The kernel's current view of the ABS axes is copied into the provided buffer.
>>> + * If an ABS axis is not enabled on the device, its value will be zero. Also, if
>>> + * an axis is not a slotted MT-axis, values for all but the first slot will be
>>> + * 0. If @slots is greater than the actual number of slots provided by the
>>> + * device, values for all slots higher than that will be 0.
>>> + *
>>> + * This call may fail with -EINVAL if the kernel doesn't support this call or
>>> + * the arguments are invalid, with -ENODEV if access was revoked, -ENOMEM if the
>>> + * kernel couldn't allocate temporary buffers for data-copy or -EFAULT if the
>>> + * passed pointer was invalid.
>>> + */
>>> +#define EVIOCGABSRANGE         _IOR('E', 0x94, struct input_absrange)
>>> +
>>>  #define EVIOCSCLOCKID          _IOW('E', 0xa0, int)                    /* Set clockid to be used for timestamps */
>>>
>>>  /*
>>> --
>>> 2.0.0
>>>

^ permalink raw reply

* [PATCH] Fixes kernel panic with Null pointer in hid-appleir.c
From: Nicholas Krause @ 2014-06-20 16:25 UTC (permalink / raw)
  To: jkosina; +Cc: linux-input, linux-kernel

In for loop of function appleir_input_configured we hit
a Null pointer after the for loop due to array_size not
being correct needs to be changed to input_dev->keycodemax.

Signed-off-by: Nicholas Krause <xerofoify@gmail.com>
---
 drivers/hid/hid-appleir.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hid/hid-appleir.c b/drivers/hid/hid-appleir.c
index 0e6a42d..ab0a702 100644
--- a/drivers/hid/hid-appleir.c
+++ b/drivers/hid/hid-appleir.c
@@ -272,7 +272,7 @@ static void appleir_input_configured(struct hid_device *hid,
 	input_dev->evbit[0] = BIT(EV_KEY) | BIT(EV_REP);
 
 	memcpy(appleir->keymap, appleir_key_table, sizeof(appleir->keymap));
-	for (i = 0; i < ARRAY_SIZE(appleir_key_table); i++)
+	for (i = 0; i < ARRAY_SIZE(input_dev->keycodemax); i++)
 		set_bit(appleir->keymap[i], input_dev->keybit);
 	clear_bit(KEY_RESERVED, input_dev->keybit);
 }
-- 
1.9.1


^ 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