Re: linux driver for CM109 voip chipset

Hi Oliver

many thanks for your feedback, see comments inline..


Oliver Neukum wrote:
> Am Sonntag, 25. Februar 2007 21:45 schrieb Alfred E. Heggestad:
>> hi
>>
>> I have written a small driver for the C-Media CM109 chipset
>> used in USB VoIP phones like the KIP-1000. The source can
>> be found here:
>>
>>   http://aeh.db.org/pub/cm109.c
> 
> 	/* allocate usb buffers */
> 	cm109->irq_data = usb_buffer_alloc(udev, USB_PKT_LEN,
> 					GFP_ATOMIC, &cm109->irq_dma);
> 	if (cm109->irq_data == NULL)
> 		goto err;
> 
> 	cm109->ctl_data = usb_buffer_alloc(udev, USB_PKT_LEN,
> 					GFP_ATOMIC, &cm109->ctl_dma);
> 	if (!cm109->ctl_data)
> 		goto err;
> 
> 	cm109->ctl_req = usb_buffer_alloc(udev, sizeof(*(cm109->ctl_req)),
> 					GFP_ATOMIC, &cm109->ctl_req_dma);
> 	if (cm109->ctl_req == NULL)
> 		goto err;
> 
> No need for atomic allocations.
> 

ok fixed. I uploaded a new version here:

   http://aeh.db.org/pub/cm109-v0.2.c


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

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
actually turn *off* the buzzer if using the input event API.
any ideas?


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


/alfred

> 	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