Hi Bob, On Tue, Aug 08, 2006, Bob Copeland wrote: > I have done a bit of editing of the eject patches and the result is below > (applies to 2.6.17.7). Namely my changes were: > > - Use EV's empirically determined 5s timeout instead of 6s
Why? It is only an upper limit and shouldn't matter. I would've thought it would be better to err on the side of a longer timeout to avoid the possibility that it might need changing yet again in the future. If you think that 5s will be enough for all users then I guess that's okay. I'm not so sure though. > - Keep rio_karma prefix... I know this is different from the other .c files, > but I don't think it the maintainer will care and it makes it more obvious > that the initializer just moved Okay. > - Simplified the send_command a bit Haven't had time to read through this yet. Will comment later this evening. > - Got rid of karma_data. I'm not so sure that we care whether or not we are > already in storage mode or not; please advise if you feel differently. It > definitely reduces the complexity of the patch to kill that. As for the recv > buffer, since we call karma_send_command so rarely, keeping the buffer > allocated all that time seems wasteful so now we just allocate it for > send_command. > - Why did we intercept READ_10? I couldn't think of a reason why, since once > we eject the device the dev is dead until we replug it. Let me know if > there's a reason to add it back. As you may have noticed from my other patches, I dislike continual allocation and de-allocations of the same piece of memory. Hence the buffer. If you feel that it makes more sense to do it that way in the kernel then that's fine by me. Both the READ_10 interception and the storage mode flag are there to enable the device to be remounted without having to unplug and replug the device. The READ_10 intercepts any mount request for the device and re-initialises when one is detected. I've got another patch at home which catches the mount command more explicitly (found by experimentation). This is the way I use my player all the time now - I just leave it plugged in via usb and mount/eject as needed. If you think that it will make a big difference to the patches acceptance then remove it but I would rather it stayed if possible. > One other question - > + if (seq > 0xf0) seq = 1; > > Did you find this by experimentation or is it a guess? This is just to prevent the sequence number from overflowing. Since it is an unsigned char, the test is a bit redundant and can be removed. > If you are fine with the changes, please add a Signed-off-by line as in > Documentation/SubmittingPatches, then I'll push a version of this upstream > that applies against my previously submitted timeout patch. I'll read through the changes properly this evening. Keith. ------------------------------------------------------------------------- Using Tomcat but need to do more? Need to support web services, security? Get stuff done quickly with pre-integrated technology to make your job easier Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642 _______________________________________________ linux-karma-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/linux-karma-devel
