Date: Mon, 18 Aug 2008 22:58:21 -0700 From: Alfred Perlstein <alfred@freebsd.org> To: freebsd-usb@freebsd.org Subject: Re: usb4bsd patch review [was Re: ...] Message-ID: <20080819055821.GO16977@elvis.mu.org> In-Reply-To: <20080818214711.J88849@maildrop.int.zabbadoz.net> References: <20080818205914.GJ16977@elvis.mu.org> <20080818214711.J88849@maildrop.int.zabbadoz.net>
next in thread | previous in thread | raw e-mail | index | archive | help
Hans, here's some final review questions, I've added responses where I can recall off the top of my head answers, but I need you to fill in the rest. Hans, can you please resend the answers to these that you've given me? Please include short examples why the changes are needed. I've noted the questions I've forgotten with "??Hans??", please fill those out. I'm quite certain we've already covered all of these points but I would like to be sure. They're not in the front of my brain and you can answer this much easier than I can. FYI, I plan on committing the code tomorrow night based on the following two issues: need to wait for smp tty code. need your reply to this email. -Alfred * Bjoern A. Zeeb <bz@FreeBSD.org> [080818 15:24] wrote: > On Mon, 18 Aug 2008, Alfred Perlstein wrote: > > Hi Alfred, > > CC:ing freebsd-usb for additional review. > > A few initial comments on the patch > > >The delta is here: > > http://mu.org/~bright/usb4bsd.diff.gz > > The patch only includes kernel parts, is there any impact on userland? I think the userland tools need to be switched to the new headers, however what's more likely to happen is that after a soak period of a few weeks, we will move the new to replace the old and userland should be just fine. > There are various style bugs introduced. That could be fixed easily I > guess? Yes, this is a LOT of new code, let's leave a few whitespace nits for others to play with. :) > In kern/subr_bus.c you are taking steps to avoid zero size > allocations, which haven't been there up to now. Why do we need that? ??Hans?? > > There is a question about whether to avoid the conditional locking in > dev/sound/pcm/channel.c in favour of recursive locking? ??Hans?? > The locking in dev/sound/pcm/mixer.c appears to be strange; the > locking appears to have substantially changed without corresponding > changes to the non-USB drivers. ??Hans?? > Some of the new sound drivers are Giant-locked? I am not sure we > should be adding new drivers that are not mpsafe. I think this will be addressed shortly... but.. ??Hans?? (please comment) > > There seems to be a README.TXT but no manpages for the new API or > drivers. Are these pending? Hans has documented a LOT of headers and functions far past what is in the old code from what I can tell. If there are specific changes you have questions about I think we can attend to those relatively easily, Hans has been very forthcoming with information. > Some of the files have $FreeBSD$ strings already expanded, probably > because they were copied from existing files. Yes, committing them should fix that. If the tags aren't obliterated they'll serve as a guide as to what files they were based on, if they are obliterated it's not that important anyhow. I consider this a wash. > > Why is the diff to compat/ndis/ntoskrnl_var.h necessary? It's needed for compiling usb2_ndis, but... ??Hans?? -- - Alfred Perlstein
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20080819055821.GO16977>