Re: linux driver for CM109 voip chipset

Am Mittwoch, 28. Februar 2007 01:29 schrieb Alfred E. Heggestad:
> 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

+	spin_lock(&dev->submit_lock);
+	/* ask for a response */
+	if (!dev->disconnecting)
+		ret = usb_submit_urb(dev->urb_irq, GFP_KERNEL);

must be GFP_ATOMIC

+	if (urb->status) {
+		if (-ESHUTDOWN == urb->status)
+			return;

Test for being killed by usb_kill_urb() or you might report
bogus key presses

+      out:
+	ret = usb_submit_urb(dev->urb_ctl, GFP_KERNEL);
+	if (ret)

GFP_ATOMIC

+	case '1':
+		on = 1;
+		break;
+
+	default:
+		goto out;
+	}

Shouldn't default report an error?

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

+#if 0
+	/* TODO crashes the kernel - use better locking ? */
+	buzz(dev, 1);
+#else

This is the code for buzzing? I should have noticed earlier.
You cannot use usb_control_msg() in interrupt. Use an URB.


> should the same be done in urb_irq_callback() ?

Yes.

	Regards
		Oliver

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