| Message ID | 20200226210414.28133-3-linux@roeck-us.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | usb: dwc2: Fixes and improvements | expand |
Hi, On Wed, Feb 26, 2020 at 1:04 PM Guenter Roeck <linux@roeck-us.net> wrote: > > The DWC2 documentation states that transfers with zero data length should > set the number of packets to 1 and the transfer length to 0. This is not > currently the case for inbound transfers: the transfer length is set to > the maximum packet length. This can have adverse effects if the chip > actually does transfer data as it is programmed to do. Follow chip > documentation and keep the transfer length set to 0 in that situation. > > Signed-off-by: Guenter Roeck <linux@roeck-us.net> > --- > drivers/usb/dwc2/hcd.c | 15 ++++++++------- > 1 file changed, 8 insertions(+), 7 deletions(-) I don't have any other test setup that you don't have, so just giving my review tag and not tested tag. I will note that it feels like this should have a "Fixes" tag or a direct Cc to stable to make it obvious that it should make its way back to stable trees. Reviewed-by: Douglas Anderson <dianders@chromium.org>
On Thu, Feb 27, 2020 at 02:06:44PM -0800, Doug Anderson wrote: > Hi, > > On Wed, Feb 26, 2020 at 1:04 PM Guenter Roeck <linux@roeck-us.net> wrote: > > > > The DWC2 documentation states that transfers with zero data length should > > set the number of packets to 1 and the transfer length to 0. This is not > > currently the case for inbound transfers: the transfer length is set to > > the maximum packet length. This can have adverse effects if the chip > > actually does transfer data as it is programmed to do. Follow chip > > documentation and keep the transfer length set to 0 in that situation. > > > > Signed-off-by: Guenter Roeck <linux@roeck-us.net> > > --- > > drivers/usb/dwc2/hcd.c | 15 ++++++++------- > > 1 file changed, 8 insertions(+), 7 deletions(-) > > I don't have any other test setup that you don't have, so just giving > my review tag and not tested tag. > > I will note that it feels like this should have a "Fixes" tag or a > direct Cc to stable to make it obvious that it should make its way > back to stable trees. > I added Fixes: 56f5b1cff22a1 ("staging: Core files for the DWC2 driver") > Reviewed-by: Douglas Anderson <dianders@chromium.org> Thanks! Guenter
diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c index f6d8cc9cee34..506fdffd82ab 100644 --- a/drivers/usb/dwc2/hcd.c +++ b/drivers/usb/dwc2/hcd.c @@ -1313,19 +1313,20 @@ static void dwc2_hc_start_transfer(struct dwc2_hsotg *hsotg, if (num_packets > max_hc_pkt_count) { num_packets = max_hc_pkt_count; chan->xfer_len = num_packets * chan->max_packet; + } else if (chan->ep_is_in) { + /* + * Always program an integral # of max packets + * for IN transfers. + * Note: This assumes that the input buffer is + * aligned and sized accordingly. + */ + chan->xfer_len = num_packets * chan->max_packet; } } else { /* Need 1 packet for transfer length of 0 */ num_packets = 1; } - if (chan->ep_is_in) - /* - * Always program an integral # of max packets for IN - * transfers - */ - chan->xfer_len = num_packets * chan->max_packet; - if (chan->ep_type == USB_ENDPOINT_XFER_INT || chan->ep_type == USB_ENDPOINT_XFER_ISOC) /*
The DWC2 documentation states that transfers with zero data length should set the number of packets to 1 and the transfer length to 0. This is not currently the case for inbound transfers: the transfer length is set to the maximum packet length. This can have adverse effects if the chip actually does transfer data as it is programmed to do. Follow chip documentation and keep the transfer length set to 0 in that situation. Signed-off-by: Guenter Roeck <linux@roeck-us.net> --- drivers/usb/dwc2/hcd.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-)