On Thu, 4 Jul 2013, Ming Lei wrote: > On Wed, Jul 3, 2013 at 10:15 PM, Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote: > > On Wed, 3 Jul 2013, Ming Lei wrote: > > > >> >> Yes, that is the change tasklet is bringing, so looks we need to find a way > >> >> to distinguish streaming-on from underrun when stream->td_list becomes > >> >> empty in iso_stream_schedule(). > >> > > >> > The difference will be the URB_ISO_ASAP flag. The flag should be set > >> > in the first URB of a new stream. It should not be set in any other > >> > URBs, unless the client driver doesn't care about losing > >> > synchronization when an underrun occurs. > >> > >> So sounds a better name for the flag should be URB_ISO_STREAM_ON, :-) > > > > I agree, but the existing name is already exposed to userspace in > > include/uapi/linux/usbdevice_fs.h. We could change the internal name > > If so, your coming change may break ABI because as you described > that "the flag should be set in the first URB of a new stream", but > some user-space drivers may not set it before. Even USB audio driver > doesn't set it in current -next tree. Really? I thought Clemens's changes were already merged. In any case, the audio driver should be easy enough to change. Setting the flag on the first URB isn't a big deal. User programs probably don't matter much. In general, the practice is to leave the interface on altsetting 0 when nothing is running and then change to a different altsetting when starting an isochronous stream. Changing the altsetting is another way to tell the HCD that a stream is starting fresh. > > while leaving the external name the same, but that would be confusing. > > > > (URB_ISO_ASAP used to mean that the HCD was responsible for scheduling > > the URB, and if the flag wasn't set then the value of urb->start_frame > > would be used directly. This doesn't make sense, because drivers > > should not be in charge of URB scheduling -- HCDs should take care of > > it. So I have felt free to alter the flag's meaning.) > > Anyway the name will become a bit confusing, so if we don't plan to > change it, we should add detailed comment about its usage. Yes. There already are detailed comments; they will need to be updated. But I never did add anything to Documentation/usb/URB.txt; obviously that file needs attention. > > them simply set URB_ISO_ASAP on all their URBs. Clemens has already > > added a fix for the snd-usb-audio driver. > > I am wondering the fix has been added because it may depend on the > coming change of URB_ISO_ASAP, :-) The new meaning of URB_ISO_ASAP is very close to the current meaning. Whatever changes Clemens made ought to work for both the current code and the upcoming code. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
