Adding CC Sorry, I still have one more quesiton embedded. > From: oneu...@suse.de > To: unicorn_w...@outlook.com > Subject: Re: question on skel_read func of usb_skeleton.c > Date: Tue, 9 Jul 2013 08:42:19 +0200 > > On Tuesday 09 July 2013 01:25:00 汪辰 wrote: >> Thanks. >> Regarding my first question. I think the original sample is used to demo >> both sync call in skel_read and async call in skel_write. > > Yes. However, a sync call is just incompatible with asynchronous IO. > And drivers should support non-blocking IO. > >> From practice perspective, I agree your improvements looks better. But only >> one quick quesiton is, current skel_read and skel_write both use async >> method but they are a bit different. For skel_write, I see there is just >> small improvement to limit the # of concurrent out bulks and for the case >> urb callback returns status error, we just record it and leave the next >> write failed on this, is that your design intent? > > Writing is different because you can just free the buffer after IO. > If you do async IO you have no choice but to report the error at a later > time. > >> Regarding the "retry" question, waht I'm talking is about the code segment >> from line 289 to line 315 (use the sample file with 3.10 kernel). >> line 289, size_t available = dev->bulk_in_filled - dev->bulk_in_copied; when >> and in what case, available equals to zero? What I can think of is for the >> case urb returns with 0 but the bulk_in_filled equals to zero too, bcos >> buil_in_copied will be always init as zero for every urb request, right? > > Yes. You also need to consider the reset case. Sorry, I still have not understood why we need to get the var - available by "dev->bulk_in_filled - dev->bulk_in_copied". dev->bulk_in_copied is always reset as zero in skel_do_read_io, right? So when we go to line 289, I think it happens after read callback returns, and at that time, bulk_in_copied should have been set as zero. Though I see at line 315, bulk_in_copied is given new data, but it will be reset to zero in next retry - skel_do_read_io, right? Maybe I have not understood the reset case you mentioned makes me can not reach your idea. For the case there may be data left from the last read, I have the same quesiton. I don't know what the bulk_in_copied is used for. Maybe I have to do some more practice test to see how these codes work.
> >> You mentioned urb may finish at once with an error, but in the code line >> 273, I see for all error, read will exit, right? > > Yes, of course. But that is good. Errors should be reported soon. > >> Anway, upon is what I can not unerstand plus the comments line 294 to 295. >> Also, I'm not sure if we need the bulk_in_copied, I see it is always filled >> as zero in skel_do_read_io. Is it a bug or intend to this? > > There may be data left from the last read. > >> Regarding my thrid question, as to line 321 to 322, I would suggest just >> remove them and leave to userspace app to call more read() if they found the >> last return of read() doesn't reach the total length they wanted. This code >> in driver looks a bit tricky, what do you think? > > That would be bad for performance. The skeleton driver does what a driver > ought to do, just as plain as possible. > > Regards > Oliver > > PS: Please don't cut CC. Other people want to learn, too. ;-) >
