On Thu, Jul 4, 2013 at 10:02 AM, Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote: > 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, Looks the change isn't found in linux-next-20130701. > 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. Yes, it might work, but many details need to be payed attention to, such as, changing altsetting may not be done during suspend/resume. > >> > 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. According to the discussion, URB_ISO_ASAP should be set on the first URB of one audio stream, and keep unset on all other URBs, which looks a new usage, IMO. Thanks, -- Ming Lei -- 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
