Hi Oliver an updated version of the CM109 driver is available: (this time as a complete patch)
http://aeh.db.org/patch/cm109-20070227.patch see comments inline.. Oliver Neukum wrote: > Am Montag, 26. Februar 2007 23:58 schrieb Alfred E. Heggestad: > >>> static DECLARE_RWSEM(sysfs_rwsema); >>> >>> A mutex is enough. You never do anything but down_write(). >>> >> ok that can be fixed. but I am not sure yet how to use the sysfs >> interface. at the moment the external buzzer is activated via >> doing "echo 1 > /sys/.../buzzer" so that is write only as you >> say. But in the future we might want to read/write the EEPROM >> via the sysfs interface. > > Nevertheless a simple mutex will do. There's no need to worry > about contention here. > OK, code is now using mutex. >> ideally I would like to use a more standard user interface >> for the buzzer, like SND_BELL/SND_TONE. I added some code >> in input_ev() for triggering the buzzer, but unfortunately >> it crashes the whole box .. also I am not sure how to > > How does it crash? Any oops report? > turning buzzer on/off from sysfs interface works fine. But when the same function is called from the input_ev() event handler, it will start the buzzer, and hang the whole machine with two LEDs flashing, nothing in the logs.. mind you, the function input_ev() is actually called 3 times, so there might be some restrictions on what can be done in an input event handler. I tried various locks but nothing seem to help.. >> actually turn *off* the buzzer if using the input event API. >> any ideas? > > You might ask on the input list. > ok, we can probably disable buzzer when the keypad is pressed. >>> static void input_close(struct input_dev *dev) >>> { >>> struct cm109_dev *cmd = dev->private; >>> >>> usb_kill_urb(cmd->urb_ctl); >>> usb_kill_urb(cmd->urb_irq); >>> } >>> >>> This is a race condition. Due to your special requirement of submitting >>> the URBs from each other's completion handler the guarantees of >>> usb_kill_urb() >>> are not strong enough for you. You need to use a flag under a spin_lock. >>> >> I added "struct mutex io_mutex" to my device object. Should this be >> locked in the USB irq/ctl handlers ? > > No. You cannot use a mutex in interrupt. In fact that mutex currently has no > function in your code. > > The problem is as follows: > > CPU A CPU B > urb_irq_callback() called > usb_kill_urb(urb_irq) > //will wait for the callback to finish > usb_submit_urb(urb_ctl) //still allowed > urb_ctl_callback() > usb_submit_urb(urb_irq) > usb_kill_urb(urb_ctl) > > Your URBs are still running. > You need to make conditional submitting the URBs on a flag and > the whole thing protected by a spinlock, eg in urb_ctl_callback: > > spin_lock(&dev->submit_lock); > /* ask for a response */ > if (!dev->disconnecting) > ret = usb_submit_urb(dev->urb_irq, GFP_KERNEL); > spin_unlock(&dev->submit_lock); > > And in usb_disconnect: > > spin_lock_irq(&dev->submit_lock); > dev->disconnecting = 1; > spin_unlock_irq(&dev->submit_lock); > and now cleanup ok that should be fixed now. thanks for the help. should the same be done in urb_irq_callback() ? /alfred ------------------------------------------------------------------------- Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys-and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
