Re: [PATCH 0/7] HID: picoLCD updates
Hello Tejun,
[Tejun: sorry for duplicate, did hit reply instead of reply to all]
On Mon, 13 Aug 2012 16:27:08 Tejun Heo t...@kernel.org wrote:
On Thu, Aug 09, 2012 at 08:09:47PM +0200, Bruno Prémont wrote:
As you are working on workqueues and related code, could you have a look
at my usage of them in combination with db_defio?
The delayed memory corruptions or system reboots after unbinding/unplugging
the PicoLCD seem very much related to workqueue used to handle the deferred
IO to framebuffer.
I think things don't get cleaned-up as they should though I'm not sure
where the trouble lies.
For ease of reading, I'm inlineing below the framebuffer related code of
PicoLCD (for complete driver after this patch series apply the whole series
on top of 3.5 https://lkml.org/lkml/2012/7/30/375 )
...
void picolcd_exit_framebuffer(struct picolcd_data *data)
{
struct fb_info *info = data-fb_info;
u8 *fb_vbitmap = data-fb_vbitmap;
if (!info)
return;
device_remove_file(data-hdev-dev, dev_attr_fb_update_rate);
info-par = NULL;
unregister_framebuffer(info);
data-fb_vbitmap = NULL;
data-fb_bitmap = NULL;
data-fb_bpp = 0;
data-fb_info= NULL;
kfree(fb_vbitmap);
}
I'm kinda shooting in the dark but who flushes / cancels
fb_info-deferred_work?
fb_deferred_io_cleanup() does so and is called by destroy fbops
(when last reference to fb_info is returned):
static void picolcd_fb_destroy(struct fb_info *info)
{
/* make sure no work is deferred */
fb_deferred_io_cleanup(info);
vfree((u8 *)info-fix.smem_start);
framebuffer_release(info);
printk(KERN_DEBUG picolcd_fb_destroy(%p)\n, info);
}
= drivers/video/fb_defio.c =
void fb_deferred_io_cleanup(struct fb_info *info)
{
struct fb_deferred_io *fbdefio = info-fbdefio;
struct page *page;
int i;
BUG_ON(!fbdefio);
cancel_delayed_work_sync(info-deferred_work);
/* clear out the mapping that we setup */
for (i = 0 ; i info-fix.smem_len; i += PAGE_SIZE) {
page = fb_deferred_io_page(info, i);
page-mapping = NULL;
}
info-fbops-fb_mmap = NULL;
mutex_destroy(fbdefio-lock);
}
Thanks,
Bruno
--
To unsubscribe from this list: send the line unsubscribe linux-input in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 8/8] input: ab8500-ponkey: Rely on MFD core to convert IRQs to virtual
On Thu, Aug 9, 2012 at 5:53 PM, Lee Jones lee.jo...@linaro.org wrote: There was a plan to place ab8500_irq_get_virq() calls in each AB8500 child device prior to requesting an IRQ, but as we're no longer using Device Tree to collect our IRQ numbers, it's actually better to allow the core to do this during device registration time. So the IRQ number we pull from its resource has already been converted to a virtual IRQ. CC: Dmitry Torokhov dmitry.torok...@gmail.com CC: linux-input@vger.kernel.org Signed-off-by: Lee Jones lee.jo...@linaro.org This is looking good, I guess you need all patches to go in at the same time so Dmitry's ACK is required. FWIW: Acked-by: Linus Walleij linus.wall...@linaro.org BTW: this makes me suspect that the public ab8500_irq_get_virq() interface can be *deleted* and the function made static in the AB8500 driver, right? Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: New Alps protocol in the wild?
I just got a Dell E6230 with the same E7 report: 73 03 0a
As noted above using the qemu serio logging doesn't work.
The driver only enables multitouch if it sees the right
name for the device in the DSDT.
The following patch (against the seabios in qemu-0.14.1)
enables the windows 7 driver (7.1211.101.114 from
http://www.dell.com/support/drivers/us/en/04/DriverDetails/DriverFileFormats/Product/latitude-e6230?DriverId=CKNFNFileId=2988050063urlProductCode=False
) to bind successfully and alows one to get data from the serio
logging patch
Are there any standard tools for reverse engineering the
protocol - I saw mention of an alps-reg-dump but was
unable to find it?
thanks,
James.
--- ./qemu-0.14.1/roms/seabios/src/acpi-dsdt.dsl2011-05-06
20:02:01.0 +0100
+++ ./qemu-0.14.1.jmm/roms/seabios/src/acpi-dsdt.dsl2012-08-14
11:09:22.0 +0100
@@ -253,7 +253,7 @@
}
/* Keyboard seems to be important for WinXP install */
-Device (KBD)
+Device (PS2K)
{
Name (_HID, EisaId (PNP0303))
Method (_STA, 0, NotSerialized)
@@ -261,10 +261,8 @@
Return (0x0f)
}
-Method (_CRS, 0, NotSerialized)
+ Name (_CRS, ResourceTemplate ()
{
- Name (TMP, ResourceTemplate ()
- {
IO (Decode16,
0x0060, // Address Range Minimum
0x0060, // Address Range Maximum
@@ -277,30 +275,52 @@
0x01, // Address Alignment
0x01, // Address Length
)
+IRQ (Edge, ActiveHigh, Exclusive, )
+{1}
+
+})
+
+
+
+
+Name (_PRS, ResourceTemplate ()
+{
+StartDependentFn (0x00, 0x00)
+{
+FixedIO (
+0x0060, // Address
+0x01, // Length
+)
+FixedIO (
+0x0064, // Address
+0x01, // Length
+)
IRQNoFlags ()
{1}
-})
-Return (TMP)
}
-}
+EndDependentFn ()
+})
+ }
/* PS/2 mouse */
-Device (MOU)
+Device (PS2M)
{
-Name (_HID, EisaId (PNP0F13))
-Method (_STA, 0, NotSerialized)
-{
-Return (0x0f)
-}
-
-Method (_CRS, 0, NotSerialized)
+Name (_CID, EisaId (PNP0F13))
+ Name (_HID, EisaId (DLL0532))
+Name (_CRS, ResourceTemplate ()
+{
+IRQ (Edge, ActiveHigh, Exclusive, )
+{12}
+})
+Name (_PRS, ResourceTemplate ()
+{
+StartDependentFn (0x00, 0x00)
{
-Name (TMP, ResourceTemplate ()
-{
- IRQNoFlags () {12}
-})
-Return (TMP)
+IRQNoFlags ()
+{12}
}
+EndDependentFn ()
+})
}
/* PS/2 floppy controller */
Re: New Alps protocol in the wild?
James linux-in...@madingley.org writes: I just got a Dell E6230 with the same E7 report: 73 03 0a As noted above using the qemu serio logging doesn't work. The driver only enables multitouch if it sees the right name for the device in the DSDT. The following patch (against the seabios in qemu-0.14.1) enables the windows 7 driver (7.1211.101.114 from http://www.dell.com/support/drivers/us/en/04/DriverDetails/DriverFileFormats/Product/latitude-e6230?DriverId=CKNFNFileId=2988050063urlProductCode=False ) to bind successfully and alows one to get data from the serio logging patch Are there any standard tools for reverse engineering the protocol - I saw mention of an alps-reg-dump but was unable to find it? alps-reg-dump can be found here[1]. Depending upon how different the protocol is it may or may not be useful, however. Cheers, - Ben [1] http://kernel.ubuntu.com/git?p=sforshee/alps-reg-dump.git;a=summary -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: New Alps protocol in the wild?
On Tue, Aug 14, 2012 at 12:01:29PM -0400, Ben Gamari wrote: James linux-in...@madingley.org writes: I just got a Dell E6230 with the same E7 report: 73 03 0a As noted above using the qemu serio logging doesn't work. The driver only enables multitouch if it sees the right name for the device in the DSDT. The following patch (against the seabios in qemu-0.14.1) enables the windows 7 driver (7.1211.101.114 from http://www.dell.com/support/drivers/us/en/04/DriverDetails/DriverFileFormats/Product/latitude-e6230?DriverId=CKNFNFileId=2988050063urlProductCode=False ) to bind successfully and alows one to get data from the serio logging patch Are there any standard tools for reverse engineering the protocol - I saw mention of an alps-reg-dump but was unable to find it? alps-reg-dump can be found here[1]. Depending upon how different the protocol is it may or may not be useful, however. Just a warning that I haven't updated that in quite some time, and I'm not even sure exactly how far I was in reverse engineering the protocol when I stopped updating it. So it may or may not be useful to you. Seth -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/7] HID: picoLCD updates
Hello, On Tue, Aug 14, 2012 at 08:30:44AM +0200, Bruno Prémont wrote: I'm kinda shooting in the dark but who flushes / cancels fb_info-deferred_work? fb_deferred_io_cleanup() does so and is called by destroy fbops (when last reference to fb_info is returned): I see. Sorry but just from glancing it I can't really tell what's going wrong. Thanks. -- tejun -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: linux-next: Tree for Aug 14 (drivers/input/touchscreen/edt-ft5x06.c)
On 08/13/2012 08:55 PM, Stephen Rothwell wrote: Hi all, Changes since 20120813: on x86_64, when CONFIG_DEBUG_FS is not enabled: drivers/input/touchscreen/edt-ft5x06.c: In function 'edt_ft5x06_ts_remove': drivers/input/touchscreen/edt-ft5x06.c:846:14: error: 'struct edt_ft5x06_ts_data' has no member named 'raw_buffer' -- ~Randy -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: USB interrupt times
Hi Russell, On Tue, Aug 14, 2012 at 12:54:00PM +0200, Jiri Kosina wrote: Actually, Henrik (added to CC) has been doing some latency improvements both for input core in general, and for HID devices as well lately. I still have his patchset in my to-review queue, as I have just came back from offline vacation, but the patch below definitely can't hurt and should significantly lower the time spent in handling the irq for hid device in common situation (i.e. noone listening for debugfs events). Could you please measure how much it helps on your system? Ok, it looks like it's changed the maximum USB interrupt execution time from around 364us to 255us. With the on-review input patches on top of that, the latency should drop further. I have measured a total of 2.5 times lower latency in other areas, so I would expect something like 150 us in 3.7 for your case. If I also do a similar trick with the debug code in hid_input_report() then I get down to 212us - iow, something like the patch below. Linus' master already has a patch for that code path, actually. Given that debugfs is fairly ubiquitous in the kernel, and that this is fairly invasive in terms of interrupt execution impact, wouldn't having this debug code enabled by a separate Kconfig symbol be reasonable too? I would like that, too. Thanks, Henrik -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 02/19] Input: Improve the events-per-packet estimate
On Sun, Aug 12, 2012 at 2:42 PM, Henrik Rydberg rydb...@euromail.se wrote:
Many MT devices send a number of keys along with the mt information.
This patch makes sure that there is room for them in the packet
buffer.
Signed-off-by: Henrik Rydberg rydb...@euromail.se
---
drivers/input/input.c | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/input/input.c b/drivers/input/input.c
index 6e90705..8ebf116 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -1777,6 +1777,9 @@ static unsigned int
input_estimate_events_per_packet(struct input_dev *dev)
if (test_bit(i, dev-relbit))
events++;
+ /* Make room for KEY and MSC events */
+ events += 7;
Hi Henrik,
It is nice to get rid of the redundant pieces and to incorporate
common functions. Thank you.
I have a question about the code above though. Why do we use 7
instead of going through the keys like:
for (i = 0; i KEY_MAX; i++)
if (test_bit(i, dev-keybit))
events++;
Ping
+
return events;
}
@@ -1815,6 +1818,7 @@ int input_register_device(struct input_dev *dev)
{
static atomic_t input_no = ATOMIC_INIT(0);
struct input_handler *handler;
+ unsigned int packet_size;
const char *path;
int error;
@@ -1827,9 +1831,9 @@ int input_register_device(struct input_dev *dev)
/* Make sure that bitmasks not mentioned in dev-evbit are clean. */
input_cleanse_bitmasks(dev);
- if (!dev-hint_events_per_packet)
- dev-hint_events_per_packet =
- input_estimate_events_per_packet(dev);
+ packet_size = input_estimate_events_per_packet(dev);
+ if (dev-hint_events_per_packet packet_size)
+ dev-hint_events_per_packet = packet_size;
/*
* If delay and period are pre-set by the driver, then autorepeating
--
1.7.11.4
--
To unsubscribe from this list: send the line unsubscribe linux-input in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe linux-input in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 02/19] Input: Improve the events-per-packet estimate
On Tuesday, August 14, 2012 12:32:21 PM Ping Cheng wrote: On Sun, Aug 12, 2012 at 2:42 PM, Henrik Rydberg rydb...@euromail.se wrote: Many MT devices send a number of keys along with the mt information. This patch makes sure that there is room for them in the packet buffer. Signed-off-by: Henrik Rydberg rydb...@euromail.se --- drivers/input/input.c | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/drivers/input/input.c b/drivers/input/input.c index 6e90705..8ebf116 100644 --- a/drivers/input/input.c +++ b/drivers/input/input.c @@ -1777,6 +1777,9 @@ static unsigned int input_estimate_events_per_packet(struct input_dev *dev) if (test_bit(i, dev-relbit)) events++; + /* Make room for KEY and MSC events */ + events += 7; Hi Henrik, It is nice to get rid of the redundant pieces and to incorporate common functions. Thank you. I have a question about the code above though. Why do we use 7 instead of going through the keys like: for (i = 0; i KEY_MAX; i++) if (test_bit(i, dev-keybit)) events++; Because that would result in gross over-estimation for many devices - my keyboard has 100+ keys but it never sends all of them in one event frame, not even if I can get a cat to lay on it ;) -- Dmitry -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 02/19] Input: Improve the events-per-packet estimate
Hi Ping, Long time no see. :-) + /* Make room for KEY and MSC events */ + events += 7; It is nice to get rid of the redundant pieces and to incorporate common functions. Thank you. I have a question about the code above though. Why do we use 7 instead of going through the keys like: for (i = 0; i KEY_MAX; i++) if (test_bit(i, dev-keybit)) events++; Keyboards register a large amount of different keys, but seldom send more than one or two at a time. The value 7 is ad hoc, admittedly, but it makes the default buffer 8 bytes, which happens to precisely match the default buffer in evdev. Thanks, Henrik -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 02/19] Input: Improve the events-per-packet estimate
On Tue, Aug 14, 2012 at 12:53 PM, Dmitry Torokhov dmitry.torok...@gmail.com wrote: On Tuesday, August 14, 2012 12:32:21 PM Ping Cheng wrote: On Sun, Aug 12, 2012 at 2:42 PM, Henrik Rydberg rydb...@euromail.se wrote: Many MT devices send a number of keys along with the mt information. This patch makes sure that there is room for them in the packet buffer. Signed-off-by: Henrik Rydberg rydb...@euromail.se --- drivers/input/input.c | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/drivers/input/input.c b/drivers/input/input.c index 6e90705..8ebf116 100644 --- a/drivers/input/input.c +++ b/drivers/input/input.c @@ -1777,6 +1777,9 @@ static unsigned int input_estimate_events_per_packet(struct input_dev *dev) if (test_bit(i, dev-relbit)) events++; + /* Make room for KEY and MSC events */ + events += 7; Hi Henrik, It is nice to get rid of the redundant pieces and to incorporate common functions. Thank you. I have a question about the code above though. Why do we use 7 instead of going through the keys like: for (i = 0; i KEY_MAX; i++) if (test_bit(i, dev-keybit)) events++; Because that would result in gross over-estimation for many devices - my keyboard has 100+ keys but it never sends all of them in one event frame, not even if I can get a cat to lay on it ;) Thanks for the prompt reply. I thought you were on vacation ;-). So, what device are we talking about here? I thought it is a touch device with a few extra buttons, which are reported as key events. Am I missing something? If it is a touch device, we won't have too many buttons. So, test_bit(i, dev-keybit) won't be true for more than the number of buttons that declared by __set_bit(). I would think we could play a keyboard (this keyboard does not have letters on it ;-) with ten fingers. Ping -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: linux-next: Tree for Aug 14 (drivers/input/touchscreen/edt-ft5x06.c)
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 08/14/2012 07:59 PM, Randy Dunlap wrote: on x86_64, when CONFIG_DEBUG_FS is not enabled: drivers/input/touchscreen/edt-ft5x06.c: In function 'edt_ft5x06_ts_remove': drivers/input/touchscreen/edt-ft5x06.c:846:14: error: 'struct edt_ft5x06_ts_data' has no member named 'raw_buffer' Yeah, unfortunate oversight on my part. Guenter Roeck provided a patch for this issue which looks good to me. (Aug 7th, linux-input mailinglist) Dmitry: Can you upstream this? Is there any action required on my part? Thanks, Simon - -- Simon Budigkernel concepts GmbH simon.bu...@kernelconcepts.de Sieghuetter Hauptweg 48 +49-271-771091-17 D-57072 Siegen -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAlAq3C4ACgkQO2O/RXesiHAofACgsPG5EcF3fn0me1STCIK5nYwm 4QkAoL+Q30zA0NLNbUSPJIRt6CmR4o91 =y99v -END PGP SIGNATURE- -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: New Alps protocol in the wild?
After doing some fiddling around myself, I've put together a few tools
and I think I now have the beginnings of an understanding of the report
frame structure. I've attached some notes below. The data packets
appear to be 6 bytes long, consistent with earlier versions of the
protocol. The first and fifth bytes of the touchpad packet are still
quite mysterious. While they likely have something to do with
multitouch, they both fluctuate even with single touch events. Touchpad
packets can be distinguished from stick packets by examining the byte 5,
which is 0x3f (out of the range of the pressure field) in touchstick
packets.
I've also attached two tools I've developed. ps2-parse.py annotates PS/2
traces produced by the VM with common command names (simply pipe a trace
in to stdin, out comes the annotate trace on stdout). Alps.py is a first
attempt at communicating with the hardware. It currently has the ability
to put playback a trace (say, the attached serio-init.log) and start
dumping frames to stdout. It also has an incomplete version of the
initialization sequence (enter_absolute_mode).
Hopefully I'll find some more time in the next few days to figure out
the last few bits (primarily how multitouch events work). I wouldn't be
sad if someone finished the task for me, however.
Cheers,
- Ben
Touchpad Packet format:
byte 0: ??? starts with 0x9f, 0x8f, changes
0x10: ??
0x20: ??
0x8f: Always set
Only 0x10 and 0x20 are set with single-touch events. 0x40 seems to be
set with multitouch events
byte 1: X position? (- is left, + is right)
byte 2: Y position? (- is up, + is down)
byte 3: button state:
0x1: left touchpad
0x2: right touchpad
0x4: middle touchpad?
0x8: Always set?
0x10: left touchstick
0x20: right touchstick
0x40: middle touchstick
0x80: ???
byte 4: ???
byte 5: Pressure (0x00 - 0x3e)
0x3f: Reporting stick?
Touchstick Packet format:
byte 0: 0x10: Y Sign
0x20: X Sign
0x4f: Always set
byte 1: X pressure (0 - 0x7f)
byte 2: Y pressure (0 - 0x7f)
byte 3: always 0x48
byte 4: Z pressure (0 - 0x7c)
byte 5: always 0x3f
#!/usr/bin/python3.2
import sys
from glob import glob
from time import sleep
import logging
logging.basicConfig(level=logging.DEBUG)
def find_serio_device():
for f in glob('/sys/bus/serio/devices/serio*'):
a = open('%s/description'%f).read()
if 'i8042 AUX' in a:
return f
#dev = find_serio_device()
#logging.info(Found device %s % dev)
#open('%s/drvctl'%dev, 'w').write('serio_raw')
#sleep(1)
serio_dev = sys.argv[1]
#serio_dev = glob('/dev/serio*')[0]
f = open(serio_dev, 'wb+')
def playback(cmds):
for dir,data in cmds:
if dir == 'S':
logging.debug('Sent %02x' % data)
f.write(bytes([data]))
elif dir == 'R':
a = f.read(1)
logging.debug('Recieved %02x' % a[0])
if data != a[0]:
logging.warn('reply mismatch: expected %02x, saw %02x' % (data, a[0]))
else:
raise RuntimeError(uh oh)
def recv_ack():
a = b''
logging.debug('Waiting for ACK')
while len(a) == 0:
a = f.read(1)
print(len(a))
if a != b'\xfa':
raise RuntimeError(oops: %s % str(a))
def reset():
f.write(b'\xff')
recv_ack()
a = f.read(2)
return a
def get_device_id():
f.write(b'\xf2')
recv_ack()
a = f.read(1)
return a
def set_resolution(arg):
f.write(b'\xe8')
recv_ack()
f.write(arg)
recv_ack()
def set_sample_rate(arg):
f.write(b'\xf3')
recv_ack()
f.write(arg)
recv_ack()
def set_1_1_scaling():
f.write(b'\xe6')
recv_ack()
def status_rq():
f.write(b'\xe9')
recv_ack()
a = f.read(3)
return a
def enable_data_reporting():
f.write(b'\xf4')
recv_ack()
def disable_data_reporting():
f.write(b'\xf5')
recv_ack()
def enter_absolute_mode():
reset()
reset()
get_device_id()
set_resolution(b'\x00')
set_1_1_scaling()
set_1_1_scaling()
set_1_1_scaling()
status_rq()
set_resolution(b'\x03')
set_sample_rate(b'\xc8')
set_sample_rate(b'\x64')
set_sample_rate(b'\x50')
get_device_id()
set_sample_rate(b'\xc8')
set_sample_rate(b'\xc8')
set_sample_rate(b'\x50')
set_resolution(b'\x03')
enable_data_reporting()
def format_bytes(bytes):
return map(lambda b: '%02x' % b, bytes)
cmds = []
for l in
