Re: Problem with appletouch driver in Linux version 2.6.23-rc7
Hi Soeren, On 10/13/07, Soeren Sonnenburg [EMAIL PROTECTED] wrote: I am not sure whether it is worth fighting for. For the moment I think we should just use the attached patch (where I as you suggest set idlecount to 2). So if there are no objections, Dmity please apply. I already applied the previous version of the patch. I think we are getting to best is the enemy of good stage at this point so I'll drop this one, at leat fro now. Thanks. -- Dmitry
Re: [patch 1/8] m68k: Atari input drivers cleanup
Hi Geert,
On 10/13/07, Geert Uytterhoeven [EMAIL PROTECTED] wrote:
m68k: Atari input drivers cleanup:
- memleak on failed init/register of input devices fixed
- correct keycodes table (Atari keycodes are almost, but not entirely, equal
to Linux keycodes).
Signed-off-by: Michael Schmitz [EMAIL PROTECTED]
Signed-off-by: Geert Uytterhoeven [EMAIL PROTECTED]
Looks much better, thank you.
- input_register_device(atakbd_dev);
+ /* error check */
+ if (input_register_device(atakbd_dev)) {
+ input_free_device(atakbd_dev);
+ return -ENOMEM;
+ }
I'd be more happy if we returned real error reported by
input_register_device() instead of substituting it with -ENOMEM.
--
Dmitry
Re: [PATCH try #3] Input/Joystick Driver: add support AD7142 joystick driver
Hi Bryan,
On 10/15/07, Bryan Wu [EMAIL PROTECTED] wrote:
+
+static int ad7142_thread(void *nothing)
+{
+ do {
+ wait_for_completion(ad7142_completion);
+ ad7142_decode();
+ enable_irq(CONFIG_BFIN_JOYSTICK_IRQ_PFX);
+ } while (!kthread_should_stop());
+
No, this is not going to work well:
- you at least need to reinitialize the completion before enabling
IRQ, otherwise you will spin in a very tight loop
- if noone would touch the joystick ad7142_clsoe would() block
infinitely because noone would signal the completion and
ad7142_thread() would never stop.
Completion is just not a good abstraction here... Please use work
abstraction and possibly a separate workqueue.
+
+ ad7142_task = kthread_run(ad7142_thread, NULL, ad7142_task);
+ if (IS_ERR(ad7142_task)) {
+ printk(KERN_ERR serio: Failed to start kseriod\n);
kseriod?
+ return PTR_ERR(ad7142_task);
+ }
+ return 0;
+}
+
+static void ad7142_close(struct input_dev *dev)
+{
Don't you need to write something over i2c to tell the device to shut
down? As it is now I expect the device to continue raising its IRQ
until kernel decides that it is unhandled and should be ignored.
+ free_irq(CONFIG_BFIN_JOYSTICK_IRQ_PFX, ad7142_interrupt);
Ok, so you freeing IRQ here, but it is allocated in ad7142_probe().
What happen if you try to open device after it was closed?
+ kthread_stop(ad7142_task);
+}
+
+static int __init ad7142_init(void)
+{
+ return i2c_add_driver(ad7142_driver);
+}
+
+static void __exit ad7142_exit(void)
+{
+ i2c_del_driver(ad7142_driver);
+ input_unregister_device(ad7142_dev);
input_unregister_device() should be in ad7142_detach_client? I am not
sure i2c - there seems to be 2 interface styles and you probably need
to use the new one. I am CC-inj Jean on this.
+}
+
+module_init(ad7142_init);
+module_exit(ad7142_exit);
--
1.5.3.4
--
Dmitry
RE: [PATCH 2/3] Input/Touchscreen Driver: add support AD7877 touchscreen driver
Hi Dmitry,
From: Dmitry Torokhov [mailto:[EMAIL PROTECTED]
Sent: Donnerstag, 11. Oktober 2007 15:28
To: Bryan Wu; Michael Hennerich
Cc: linux-input@atrey.karlin.mff.cuni.cz; linux-
[EMAIL PROTECTED]; [EMAIL PROTECTED]; linux-
[EMAIL PROTECTED]; [EMAIL PROTECTED]
Subject: Re: [PATCH 2/3] Input/Touchscreen Driver: add support AD7877
touchscreen driver
Hi Bryan, Michael,
On 10/11/07, Bryan Wu [EMAIL PROTECTED] wrote:
+
+static int gpio3 = 0;
No need to initialize.
Sure - It's ZERO by default.
+
+static int ad7877_read(struct device *dev, u16 reg)
+{
+ struct spi_device *spi = to_spi_device(dev);
+ struct ser_req *req = kzalloc(sizeof *req,
GFP_KERNEL);
How many reads can happen at once? Maybe allocate 1 ser_req per
touchcsreen when creating it?
ad7877_read_adc, ad7877_read and ad7877_write are just used by the sysfs
hooks. Touchscreen samples are read by the kthread using a different
message struct. So far each sysfs invocation got its own storage for the
spi message, which then is handed over to the SPI bus driver.
The SPI bus driver serializes transfers in a kthread.
Two different processes could access the drivers sysfs hooks.
Using one ser_req per touch screen could require additional locking?
Things at is, looks pretty safe to me.
+
+ if (likely(x z1 !device_suspended(ts-spi-dev))) {
+ /* compute touch pressure resistance using equation
#2 */
+ Rt = z2;
+ Rt -= z1;
+ Rt *= x;
+ Rt *= ts-x_plate_ohms;
+ Rt /= z1;
+ Rt = (Rt + 2047) 12;
+ } else
+ Rt = 0;
+
+ if (Rt) {
+ input_report_abs(input_dev, ABS_X, x);
+ input_report_abs(input_dev, ABS_Y, y);
+ sync = 1;
+ }
+
+ if (sync) {
+ input_report_abs(input_dev, ABS_PRESSURE, Rt);
+ input_sync(input_dev);
+ }
Confused about the logic - you set sync only if Rt != 0 so why don't
fold second if into the first one?
Sure - I guess this was just a leftover form the original driver, this
driver was derived from.
+/* Must be called with ts-lock held */
+static void ad7877_disable(struct ad7877 *ts)
+{
+ if (ts-disabled)
+ return;
+
+ ts-disabled = 1;
+
+ if (!ts-pending) {
+ ts-irq_disabled = 1;
+ disable_irq(ts-spi-irq);
+ } else {
+ /* the kthread will run at least once more, and
+* leave everything in a clean state, IRQ disabled
+*/
+ while (ts-pending) {
+ spin_unlock_irq(ts-lock);
+ msleep(1);
+ spin_lock_irq(ts-lock);
+ }
+ }
+
+ /* we know the chip's in lowpower mode since we always
+* leave it that way after every request
+*/
+
+}
This looks scary. Can you try moving locking inside ad7877_enable and
ad7877_disable?
This is also something imported from the ads7846.c driver.
Since it worked pretty well I never touched this function.
However I think the spin_locks here are not necessary.
+
+static int __devinit ad7877_probe(struct spi_device *spi)
+{
+ struct ad7877 *ts;
+ struct input_dev*input_dev;
+ struct ad7877_platform_data *pdata =
spi-dev.platform_data;
+ struct spi_message *m;
+ int err;
+ u16 verify;
+
+
+ if (!spi-irq) {
+ dev_dbg(spi-dev, no IRQ?\n);
+ return -ENODEV;
+ }
+
+
+ if (!pdata) {
+ dev_dbg(spi-dev, no platform data?\n);
+ return -ENODEV;
+ }
+
+
+ /* don't exceed max specified SPI CLK frequency */
+ if (spi-max_speed_hz MAX_SPI_FREQ_HZ) {
+ dev_dbg(spi-dev, SPI CLK %d
Hz?\n,spi-max_speed_hz);
+ return -EINVAL;
+ }
+
+ ts = kzalloc(sizeof(struct ad7877), GFP_KERNEL);
+ input_dev = input_allocate_device();
+ if (!ts || !input_dev) {
+ err = -ENOMEM;
+ goto err_free_mem;
+ }
+
+
+ dev_set_drvdata(spi-dev, ts);
+ spi-dev.power.power_state = PMSG_ON;
+
+ ts-spi = spi;
+ ts-input = input_dev;
+ ts-intr_flag = 0;
+ init_timer(ts-timer);
+ ts-timer.data = (unsigned long) ts;
+ ts-timer.function = ad7877_timer;
+
+ spin_lock_init(ts-lock);
+
+ ts-model = pdata-model ? : 7877;
+ ts-vref_delay_usecs = pdata-vref_delay_usecs ? : 100;
+ ts-x_plate_ohms = pdata-x_plate_ohms ? : 400;
+ ts-pressure_max = pdata-pressure_max ? : ~0;
+
+
+ ts-stopacq_polarity = pdata-stopacq_polarity;
+ ts-first_conversion_delay =
Re: [PATCH try #3] Input/Joystick Driver: add support AD7142 joystick driver
On 10/15/07, Dmitry Torokhov [EMAIL PROTECTED] wrote:
Hi Bryan,
On 10/15/07, Bryan Wu [EMAIL PROTECTED] wrote:
+
+static int ad7142_thread(void *nothing)
+{
+ do {
+ wait_for_completion(ad7142_completion);
+ ad7142_decode();
+ enable_irq(CONFIG_BFIN_JOYSTICK_IRQ_PFX);
+ } while (!kthread_should_stop());
+
No, this is not going to work well:
- you at least need to reinitialize the completion before enabling
IRQ, otherwise you will spin in a very tight loop
- if noone would touch the joystick ad7142_clsoe would() block
infinitely because noone would signal the completion and
ad7142_thread() would never stop.
Completion is just not a good abstraction here... Please use work
abstraction and possibly a separate workqueue.
Yes, I agree with you now, although I have a little concern about the
possibility of big delay introduced by workqueue.
+
+ ad7142_task = kthread_run(ad7142_thread, NULL, ad7142_task);
+ if (IS_ERR(ad7142_task)) {
+ printk(KERN_ERR serio: Failed to start kseriod\n);
kseriod?
My fault, I did't notice this copy words from other driver.
+ return PTR_ERR(ad7142_task);
+ }
+ return 0;
+}
+
+static void ad7142_close(struct input_dev *dev)
+{
Don't you need to write something over i2c to tell the device to shut
down? As it is now I expect the device to continue raising its IRQ
until kernel decides that it is unhandled and should be ignored.
I am not sure here, should do some investigate of the hardware datasheet.
+ free_irq(CONFIG_BFIN_JOYSTICK_IRQ_PFX, ad7142_interrupt);
Ok, so you freeing IRQ here, but it is allocated in ad7142_probe().
What happen if you try to open device after it was closed?
Yes, it should be move to ad7142_detach_client()
+ kthread_stop(ad7142_task);
+}
+
+static int __init ad7142_init(void)
+{
+ return i2c_add_driver(ad7142_driver);
+}
+
+static void __exit ad7142_exit(void)
+{
+ i2c_del_driver(ad7142_driver);
+ input_unregister_device(ad7142_dev);
input_unregister_device() should be in ad7142_detach_client? I am not
sure i2c - there seems to be 2 interface styles and you probably need
to use the new one. I am CC-inj Jean on this.
Yes, no need input_unregister_device() here.
Thanks a lot for you kindly review.
I will resend update patch later.
Best Regards,
-Bryan Wu
Re: [PATCH 2/3] Input/Touchscreen Driver: add support AD7877 touchscreen driver
Hi Michael,
On 10/15/07, Hennerich, Michael [EMAIL PROTECTED] wrote:
+
+static int ad7877_read(struct device *dev, u16 reg)
+{
+ struct spi_device *spi = to_spi_device(dev);
+ struct ser_req *req = kzalloc(sizeof *req,
GFP_KERNEL);
How many reads can happen at once? Maybe allocate 1 ser_req per
touchcsreen when creating it?
ad7877_read_adc, ad7877_read and ad7877_write are just used by the sysfs
hooks. Touchscreen samples are read by the kthread using a different
message struct. So far each sysfs invocation got its own storage for the
spi message, which then is handed over to the SPI bus driver.
The SPI bus driver serializes transfers in a kthread.
Two different processes could access the drivers sysfs hooks.
Using one ser_req per touch screen could require additional locking?
Things at is, looks pretty safe to me.
OK, fair enough.
--
Dmitry
Re: [PATCH try #3] Input/Joystick Driver: add support AD7142 joystick driver
On 10/15/07, Bryan Wu [EMAIL PROTECTED] wrote: On 10/15/07, Dmitry Torokhov [EMAIL PROTECTED] wrote: Completion is just not a good abstraction here... Please use work abstraction and possibly a separate workqueue. Yes, I agree with you now, although I have a little concern about the possibility of big delay introduced by workqueue. Having a separate workqueue should isolate the driver from users hogging keventd. Otherwise the speed should be pretty much the same as with a kthread. Thanks a lot for you kindly review. I will resend update patch later. Thank you for not getting frustrated with all my change requests. Btw, blackfin keypad driver is in my tree and should be in mainline once Linus does the pull I requested. -- Dmitry
Re: [PATCH try #3] Input/Joystick Driver: add support AD7142 joystick driver
On Mon, Oct 15, 2007 at 11:48:17AM -0400, Dmitry Torokhov wrote:
Hi Bryan,
On 10/15/07, Bryan Wu [EMAIL PROTECTED] wrote:
+
+static int ad7142_thread(void *nothing)
+{
+ do {
+ wait_for_completion(ad7142_completion);
+ ad7142_decode();
+ enable_irq(CONFIG_BFIN_JOYSTICK_IRQ_PFX);
+ } while (!kthread_should_stop());
+
No, this is not going to work well:
- you at least need to reinitialize the completion before enabling
IRQ, otherwise you will spin in a very tight loop
- if noone would touch the joystick ad7142_clsoe would() block
infinitely because noone would signal the completion and
ad7142_thread() would never stop.
Completion is just not a good abstraction here... Please use work
abstraction and possibly a separate workqueue.
Bryan, I'm very interested in the technical advantage of using a completion
here.
In my _not-experienced_ opinion, I remember completions was created mainly for
create_task, wait till task got finished, go on case. Why using it in a
different context while workqueues was created for a similar situation to
ad7142 one (non-irq context bottom-half) ?
Regards,
--
Ahmed S. Darwish
HomePage: http://darwish.07.googlepages.com
Blog: http://darwish-07.blogspot.com
Re: [PATCH 3/3] Blackfin serial driver: this driver enable SPORTs on Blackfin emulate UART
Subject: [PATCH 3/3] Blackfin serial driver: this driver enable SPORTs on
Blackfin emulate UART
That's a bit hard to parse.
On Thu, 11 Oct 2007 18:23:40 +0800
Bryan Wu [EMAIL PROTECTED] wrote:
Signed-off-by: Bryan Wu [EMAIL PROTECTED]
---
drivers/serial/Kconfig | 43 +++
drivers/serial/Makefile |1 +
drivers/serial/bfin_sport_uart.c | 614
++
drivers/serial/bfin_sport_uart.h | 63
include/linux/serial_core.h |2 +
5 files changed, 723 insertions(+), 0 deletions(-)
create mode 100644 drivers/serial/bfin_sport_uart.c
create mode 100644 drivers/serial/bfin_sport_uart.h
diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
index 81b52b7..f3bfbe1 100644
--- a/drivers/serial/Kconfig
+++ b/drivers/serial/Kconfig
@@ -1259,4 +1259,47 @@ config SERIAL_OF_PLATFORM
Currently, only 8250 compatible ports are supported, but
others can easily be added.
+config SERIAL_BFIN_SPORT
+ tristate Blackfin SPORT emulate UART (EXPERIMENTAL)
+ depends on BFIN EXPERIMENTAL
+ select SERIAL_CORE
+ help
+ Enble support SPORT emulate UART on Blackfin series.
There's a typo there. Also the text is pretty meaningless - I'd fix it up
but I'm not sure what it's trying to tell us!
+//#define DEBUG
Probably this shouldn't be here at all - DEBUG is passed in from the gcc
command line.
+#include linux/module.h
+#include linux/ioport.h
+#include linux/init.h
+#include linux/console.h
+#include linux/sysrq.h
+#include linux/platform_device.h
+#include linux/tty.h
+#include linux/tty_flip.h
+#include linux/serial_core.h
+
+#include asm/delay.h
+#include asm/portmux.h
+
+#include bfin_sport_uart.h
+
+unsigned short bfin_uart_pin_req_sport0[] =
+ {P_SPORT0_TFS, P_SPORT0_DTPRI, P_SPORT0_TSCLK, P_SPORT0_RFS, \
+ P_SPORT0_DRPRI, P_SPORT0_RSCLK, P_SPORT0_DRSEC, P_SPORT0_DTSEC, 0};
+
+unsigned short bfin_uart_pin_req_sport1[] =
+ {P_SPORT1_TFS, P_SPORT1_DTPRI, P_SPORT1_TSCLK, P_SPORT1_RFS, \
+ P_SPORT1_DRPRI, P_SPORT1_RSCLK, P_SPORT1_DRSEC, P_SPORT1_DTSEC, 0};
I don't think these need global scope?
...
+/* Reqeust IRQ, Setup clock */
+static int sport_startup(struct uart_port *port)
+{
+ struct sport_uart_port *up = (struct sport_uart_port *)port;
+ char buffer[20];
+ int retval;
+
+ pr_debug(%s enter\n, __FUNCTION__);
+ memset(buffer, 20, '\0');
I don't think this memset is needed.
+ snprintf(buffer, 20, %s rx, up-name);
+ retval = request_irq(up-rx_irq, sport_uart_rx_irq, IRQF_SAMPLE_RANDOM,
buffer, up);
+ if (retval) {
+ printk(KERN_ERR Unable to request interrupt %s\n, buffer);
+ return retval;
+ }
+
+ snprintf(buffer, 20, %s tx, up-name);
+ retval = request_irq(up-tx_irq, sport_uart_tx_irq, IRQF_SAMPLE_RANDOM,
buffer, up);
+ if (retval) {
+ printk(KERN_ERR Unable to request interrupt %s\n, buffer);
+ goto fail1;
+ }
+
+ snprintf(buffer, 20, %s err, up-name);
+ retval = request_irq(up-err_irq, sport_uart_err_irq,
IRQF_SAMPLE_RANDOM, buffer, up);
+ if (retval) {
+ printk(KERN_ERR Unable to request interrupt %s\n, buffer);
+ goto fail2;
+ }
It is not a good idea to create files in /proc which have spaces in their
names. Yes, userspace _should_ be able to cope with that in all cases, but
all software sucks, even including userspace ;)
I'd suggest that we be defensive here, and avoid using spaces in filenames.
+ if (port-line) {
+ if (peripheral_request_list(bfin_uart_pin_req_sport1, DRV_NAME))
+ goto fail3;
+ } else {
+ if (peripheral_request_list(bfin_uart_pin_req_sport0, DRV_NAME))
+ goto fail3;
+ }
+
+ sport_uart_setup(up, get_sclk(), port-uartclk);
+
+ /* Enable receive interrupt */
+ SPORT_PUT_RCR1(up, (SPORT_GET_RCR1(up) | RSPEN));
+ SSYNC();
+
+ return 0;
+
+
+fail3:
+ printk(KERN_ERR DRV_NAME
+ : Requesting Peripherals failed\n);
s/P/p/
+ free_irq(up-err_irq, up);
+fail2:
+ free_irq(up-tx_irq, up);
+fail1:
+ free_irq(up-rx_irq, up);
+
+ return retval;
+
+}
...
+static void sport_shutdown(struct uart_port *port)
+{
+ struct sport_uart_port *up = (struct sport_uart_port *)port;
That's a bit ugly. Perhaps using container_of() would be clearer. it
would also remove the requirement that uart_port be the first member of
sport_uart_port.
+ pr_debug(%s enter\n, __FUNCTION__);
+
+ /* Disable sport */
+ SPORT_PUT_TCR1(up, (SPORT_GET_TCR1(up) ~TSPEN));
+ SPORT_PUT_RCR1(up, (SPORT_GET_RCR1(up) ~RSPEN));
+ SSYNC();
+
+ if (port-line) {
+ peripheral_free_list(bfin_uart_pin_req_sport1);
+ } else {
+
Re: [PATCH 3/3] Blackfin serial driver: this driver enable SPORTs on Blackfin emulate UART
On 10/15/07, Andrew Morton [EMAIL PROTECTED] wrote:
+config SERIAL_BFIN_SPORT
+ tristate Blackfin SPORT emulate UART (EXPERIMENTAL)
+ depends on BFIN EXPERIMENTAL
+ select SERIAL_CORE
+ help
+ Enble support SPORT emulate UART on Blackfin series.
There's a typo there. Also the text is pretty meaningless - I'd fix it up
but I'm not sure what it's trying to tell us!
here it is in english ;)
This driver emulates a standard UART using the SPORT peripherals on a
Blackfin processor.
+ snprintf(buffer, 20, %s rx, up-name);
+ retval = request_irq(up-rx_irq, sport_uart_rx_irq,
IRQF_SAMPLE_RANDOM, buffer, up);
+ if (retval) {
+ printk(KERN_ERR Unable to request interrupt %s\n, buffer);
+ return retval;
+ }
+
+ snprintf(buffer, 20, %s tx, up-name);
+ retval = request_irq(up-tx_irq, sport_uart_tx_irq,
IRQF_SAMPLE_RANDOM, buffer, up);
+ if (retval) {
+ printk(KERN_ERR Unable to request interrupt %s\n, buffer);
+ goto fail1;
+ }
+
+ snprintf(buffer, 20, %s err, up-name);
+ retval = request_irq(up-err_irq, sport_uart_err_irq,
IRQF_SAMPLE_RANDOM, buffer, up);
+ if (retval) {
+ printk(KERN_ERR Unable to request interrupt %s\n, buffer);
+ goto fail2;
+ }
It is not a good idea to create files in /proc which have spaces in their
names. Yes, userspace _should_ be able to cope with that in all cases, but
all software sucks, even including userspace ;)
I'd suggest that we be defensive here, and avoid using spaces in filenames.
i'm not sure i follow ... these are the names given to request_irq()
which means this is what shows up in /proc/interrupts ... does this
function also create an actual file somewhere in /proc that i am not
aware of ?
the rest of the comments look good to me, thanks !
-mike
Re: [PATCH 3/3] Blackfin serial driver: this driver enable SPORTs on Blackfin emulate UART
On 10/11/07, Bryan Wu [EMAIL PROTECTED] wrote: --- a/drivers/serial/Kconfig +++ b/drivers/serial/Kconfig +config SERIAL_BFIN_SPORT + tristate Blackfin SPORT emulate UART (EXPERIMENTAL) + depends on BFIN EXPERIMENTAL + select SERIAL_CORE + help + Enble support SPORT emulate UART on Blackfin series. + + To compile this driver as a module, choose M here: the + module will be called bfin_sport_uart. + +choice + prompt Baud rate for Blackfin SPORT UART + depends on SERIAL_BFIN_SPORT + default SERIAL_SPORT_BAUD_RATE_57600 + help + Choose a baud rate for the SPORT UART, other uart settings are + 8 bit, 1 stop bit, no parity, no flow control. this looks wrong to me ... why cant the standard infrastructure for managing baud rate be used ? -mike
Re: [PATCH 3/3] Blackfin serial driver: this driver enable SPORTs on Blackfin emulate UART
On Mon 15 Oct 2007 16:33, Andrew Morton pondered: Subject: [PATCH 3/3] Blackfin serial driver: this driver enable SPORTs on Blackfin emulate UART That's a bit hard to parse. Blackfin's have a synchronous Serial Peripheral pORT (SPORT). Unlike SPI, UART, I2C, or CAN interfaces which are designed for specific industry standard compatible communication only, the SPORT support a variety (software programmable) serial data communication protocols: - A-law or ยต-law companding according to G.711 specification - Multichannel or Time-Division-Multiplexed (TDM) modes - Stereo Audio I2S Mode - TDM Modes for Multi-Channel audio codecs - H.100 Telephony standard support - others, but if anyone really cares, they need to read the chip specs... Bryan's patch takes the SPORT, and makes a standard UART out of it (exposing /dev/ttySS0) for those people who don't have enough hardware UARTs in their system. Is it a SPORT driver that emulates a UART, or a UART driver on the SPORT? I think it is the latter... Maybe: [PATCH 3/3] Blackfin serial driver: enables a UART interface for the SPORT Which still doesn't make any sense, until you know what a SPORT is :) -Robin
Re: [PATCH 3/3] Blackfin serial driver: this driver enable SPORTs on Blackfin emulate UART
On Mon, 15 Oct 2007 18:03:35 -0400 Mike Frysinger [EMAIL PROTECTED] wrote: It is not a good idea to create files in /proc which have spaces in their names. Yes, userspace _should_ be able to cope with that in all cases, but all software sucks, even including userspace ;) I'd suggest that we be defensive here, and avoid using spaces in filenames. i'm not sure i follow ... these are the names given to request_irq() which means this is what shows up in /proc/interrupts ... does this function also create an actual file somewhere in /proc that i am not aware of ? err, umm, yeah. But the same argument applies: it is imprudent to have space-containing records in /proc/interrupts. However it seems that we've already done that in several places so I guess any /proc/interrupts-parsing programs are already coping with it OK.
Re: [PATCH try #3] Input/Joystick Driver: add support AD7142 joystick driver
On 10/16/07, Dmitry Torokhov [EMAIL PROTECTED] wrote: On 10/15/07, Bryan Wu [EMAIL PROTECTED] wrote: On 10/15/07, Dmitry Torokhov [EMAIL PROTECTED] wrote: Completion is just not a good abstraction here... Please use work abstraction and possibly a separate workqueue. Yes, I agree with you now, although I have a little concern about the possibility of big delay introduced by workqueue. Having a separate workqueue should isolate the driver from users hogging keventd. Otherwise the speed should be pretty much the same as with a kthread. Does this driver need the create a new kthread instead of keventd? I think keventd might be sufficient for this driver. Thanks a lot for you kindly review. I will resend update patch later. Thank you for not getting frustrated with all my change requests. Oh, your help is very useful. It encourages us to send out our drivers to LKML. Btw, blackfin keypad driver is in my tree and should be in mainline once Linus does the pull I requested. Thanks again, I noticed it was merged already. -Bryan Wu
