| Message ID | 58c23ee62fabb5753ce9ea7fc1e93743a5b7781c.1541643786.git.thinhn@synopsys.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] usb: gadget: Add start_frame to usb_request | expand |
Hi, Thinh Nguyen <thinh.nguyen@synopsys.com> writes: > Similar to URB's start_frame, add a field start_frame to the usb_request > to report the scheduled (micro)frame number of an isochronous transfer. > This option is useful for debugging purposes. > > Signed-off-by: Thinh Nguyen <thinhn@synopsys.com> > --- > include/linux/usb/gadget.h | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h > index e5cd84a0f84a..ed9dbbce55ee 100644 > --- a/include/linux/usb/gadget.h > +++ b/include/linux/usb/gadget.h > @@ -50,6 +50,7 @@ struct usb_ep; > * @short_not_ok: When reading data, makes short packets be > * treated as errors (queue stops advancing till cleanup). > * @dma_mapped: Indicates if request has been mapped to DMA (internal) > + * @start_frame: the reported (micro)frame of the scheduled isoc transfer > * @complete: Function called when request completes, so this request and > * its buffer may be re-used. The function will always be called with > * interrupts disabled, and it must not sleep. > @@ -107,6 +108,8 @@ struct usb_request { > unsigned short_not_ok:1; > unsigned dma_mapped:1; > > + int start_frame; /* ISO ONLY */ this name is a bit misleading. I can see functions trying to use it to request that a particular request start on a specific frame. Would "started_frame" be a better name, perhaps?
Hi, On 11/7/2018 11:03 PM, Felipe Balbi wrote: > Hi, > > Thinh Nguyen <thinh.nguyen@synopsys.com> writes: >> Similar to URB's start_frame, add a field start_frame to the usb_request >> to report the scheduled (micro)frame number of an isochronous transfer. >> This option is useful for debugging purposes. >> >> Signed-off-by: Thinh Nguyen <thinhn@synopsys.com> >> --- >> include/linux/usb/gadget.h | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h >> index e5cd84a0f84a..ed9dbbce55ee 100644 >> --- a/include/linux/usb/gadget.h >> +++ b/include/linux/usb/gadget.h >> @@ -50,6 +50,7 @@ struct usb_ep; >> * @short_not_ok: When reading data, makes short packets be >> * treated as errors (queue stops advancing till cleanup). >> * @dma_mapped: Indicates if request has been mapped to DMA (internal) >> + * @start_frame: the reported (micro)frame of the scheduled isoc transfer >> * @complete: Function called when request completes, so this request and >> * its buffer may be re-used. The function will always be called with >> * interrupts disabled, and it must not sleep. >> @@ -107,6 +108,8 @@ struct usb_request { >> unsigned short_not_ok:1; >> unsigned dma_mapped:1; >> >> + int start_frame; /* ISO ONLY */ > this name is a bit misleading. I can see functions trying to use it to > request that a particular request start on a specific frame. Would > "started_frame" be a better name, perhaps? > Sure. That's more accurate. Thinh
Hi Felipe, On 11/7/2018 11:03 PM, Felipe Balbi wrote: > Hi, > > Thinh Nguyen <thinh.nguyen@synopsys.com> writes: >> Similar to URB's start_frame, add a field start_frame to the usb_request >> to report the scheduled (micro)frame number of an isochronous transfer. >> This option is useful for debugging purposes. >> >> Signed-off-by: Thinh Nguyen <thinhn@synopsys.com> >> --- >> include/linux/usb/gadget.h | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h >> index e5cd84a0f84a..ed9dbbce55ee 100644 >> --- a/include/linux/usb/gadget.h >> +++ b/include/linux/usb/gadget.h >> @@ -50,6 +50,7 @@ struct usb_ep; >> * @short_not_ok: When reading data, makes short packets be >> * treated as errors (queue stops advancing till cleanup). >> * @dma_mapped: Indicates if request has been mapped to DMA (internal) >> + * @start_frame: the reported (micro)frame of the scheduled isoc transfer >> * @complete: Function called when request completes, so this request and >> * its buffer may be re-used. The function will always be called with >> * interrupts disabled, and it must not sleep. >> @@ -107,6 +108,8 @@ struct usb_request { >> unsigned short_not_ok:1; >> unsigned dma_mapped:1; >> >> + int start_frame; /* ISO ONLY */ > this name is a bit misleading. I can see functions trying to use it to > request that a particular request start on a specific frame. Would > "started_frame" be a better name, perhaps? What do you think of changing this field name to "frame_number"? It's generic enough so that this field can potentially be updated in the future to be used as a starting frame in the future, for synchronization. Thinh
Hi, Thinh Nguyen <thinh.nguyen@synopsys.com> writes: > Hi Felipe, > > On 11/7/2018 11:03 PM, Felipe Balbi wrote: >> Hi, >> >> Thinh Nguyen <thinh.nguyen@synopsys.com> writes: >>> Similar to URB's start_frame, add a field start_frame to the usb_request >>> to report the scheduled (micro)frame number of an isochronous transfer. >>> This option is useful for debugging purposes. >>> >>> Signed-off-by: Thinh Nguyen <thinhn@synopsys.com> >>> --- >>> include/linux/usb/gadget.h | 3 +++ >>> 1 file changed, 3 insertions(+) >>> >>> diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h >>> index e5cd84a0f84a..ed9dbbce55ee 100644 >>> --- a/include/linux/usb/gadget.h >>> +++ b/include/linux/usb/gadget.h >>> @@ -50,6 +50,7 @@ struct usb_ep; >>> * @short_not_ok: When reading data, makes short packets be >>> * treated as errors (queue stops advancing till cleanup). >>> * @dma_mapped: Indicates if request has been mapped to DMA (internal) >>> + * @start_frame: the reported (micro)frame of the scheduled isoc transfer >>> * @complete: Function called when request completes, so this request and >>> * its buffer may be re-used. The function will always be called with >>> * interrupts disabled, and it must not sleep. >>> @@ -107,6 +108,8 @@ struct usb_request { >>> unsigned short_not_ok:1; >>> unsigned dma_mapped:1; >>> >>> + int start_frame; /* ISO ONLY */ >> this name is a bit misleading. I can see functions trying to use it to >> request that a particular request start on a specific frame. Would >> "started_frame" be a better name, perhaps? > > What do you think of changing this field name to "frame_number"? It's > generic enough so that this field can potentially be updated in the > future to be used as a starting frame in the future, for synchronization. I don't mind, but remember changing semantics will mean auditing every user of this :-)
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h index e5cd84a0f84a..ed9dbbce55ee 100644 --- a/include/linux/usb/gadget.h +++ b/include/linux/usb/gadget.h @@ -50,6 +50,7 @@ struct usb_ep; * @short_not_ok: When reading data, makes short packets be * treated as errors (queue stops advancing till cleanup). * @dma_mapped: Indicates if request has been mapped to DMA (internal) + * @start_frame: the reported (micro)frame of the scheduled isoc transfer * @complete: Function called when request completes, so this request and * its buffer may be re-used. The function will always be called with * interrupts disabled, and it must not sleep. @@ -107,6 +108,8 @@ struct usb_request { unsigned short_not_ok:1; unsigned dma_mapped:1; + int start_frame; /* ISO ONLY */ + void (*complete)(struct usb_ep *ep, struct usb_request *req); void *context;
Similar to URB's start_frame, add a field start_frame to the usb_request to report the scheduled (micro)frame number of an isochronous transfer. This option is useful for debugging purposes. Signed-off-by: Thinh Nguyen <thinhn@synopsys.com> --- include/linux/usb/gadget.h | 3 +++ 1 file changed, 3 insertions(+)