Re: linux driver for CM109 voip chipset

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.

> 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?

> actually turn *off* the buzzer if using the input event API.
> any ideas?

You might ask on the input list.
 
> > 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

-------------------------------------------------------------------------
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
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel