From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754636AbcA0KYJ (ORCPT ); Wed, 27 Jan 2016 05:24:09 -0500 Received: from regular1.263xmail.com ([211.150.99.138]:50447 "EHLO regular1.263xmail.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751030AbcA0KYE (ORCPT ); Wed, 27 Jan 2016 05:24:04 -0500 X-Greylist: delayed 641 seconds by postgrey-1.27 at vger.kernel.org; Wed, 27 Jan 2016 05:24:02 EST X-263anti-spam: KSV:0; X-MAIL-GRAY: 0 X-MAIL-DELIVERY: 1 X-KSVirus-check: 0 X-ABS-CHECKED: 4 X-ADDR-CHECKED: 0 X-RL-SENDER: kever.yang@rock-chips.com X-FST-TO: linux-kernel@vger.kernel.org X-SENDER-IP: 58.22.7.114 X-LOGIN-NAME: kever.yang@rock-chips.com X-UNIQUE-TAG: <567c1596b9fb4d95be23367fcb4b335d> X-ATTACHMENT-NUM: 0 X-DNS-TYPE: 0 Message-ID: <56A89ABA.7000809@rock-chips.com> Date: Wed, 27 Jan 2016 18:23:54 +0800 From: Kever Yang User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0 MIME-Version: 1.0 To: Douglas Anderson , John Youn , balbi@ti.com CC: william.wu@rock-chips.com, huangtao@rock-chips.com, heiko@sntech.de, linux-rockchip@lists.infradead.org, Julius Werner , gregory.herrero@intel.com, yousaf.kaukab@intel.com, dinguyen@opensource.altera.com, stern@rowland.harvard.edu, ming.lei@canonical.com, johnyoun@synopsys.com, gregkh@linuxfoundation.org, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v5 06/21] usb: dwc2: host: Always add to the tail of queues References: <1453486736-15358-1-git-send-email-dianders@chromium.org> <1453486736-15358-7-git-send-email-dianders@chromium.org> In-Reply-To: <1453486736-15358-7-git-send-email-dianders@chromium.org> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Doug, This is obviously a bug in dwc2 driver which not meet the usb 2.0 spec, and this patch can fix it. Reviewed-by: Kever Yang Thanks, - Kever On 01/23/2016 02:18 AM, Douglas Anderson wrote: > The queues the the dwc2 host controller used are truly queues. That > means FIFO or first in first out. > > Unfortunately though the code was iterating through these queues > starting from the head, some places in the code was adding things to the > queue by adding at the head instead of the tail. That means last in > first out. Doh. > > Go through and just always add to the tail. > > Doing this makes things much happier when I've got: > * 7-port USB 2.0 Single-TT hub > * - Microsoft 2.4 GHz Transceiver v7.0 dongle > * - Jabra speakerphone playing music > > Signed-off-by: Douglas Anderson > --- > Changes in v5: None > Changes in v4: > - Always add to the tail of queues new for v4. > > Changes in v3: None > Changes in v2: None > > drivers/usb/dwc2/hcd.c | 11 ++++++----- > drivers/usb/dwc2/hcd_ddma.c | 4 ++-- > drivers/usb/dwc2/hcd_intr.c | 4 ++-- > drivers/usb/dwc2/hcd_queue.c | 6 ++++-- > 4 files changed, 14 insertions(+), 11 deletions(-) > > diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c > index 7783c8ba0173..d2daaea88d91 100644 > --- a/drivers/usb/dwc2/hcd.c > +++ b/drivers/usb/dwc2/hcd.c > @@ -969,7 +969,8 @@ enum dwc2_transaction_type dwc2_hcd_select_transactions( > * periodic assigned schedule > */ > qh_ptr = qh_ptr->next; > - list_move(&qh->qh_list_entry, &hsotg->periodic_sched_assigned); > + list_move_tail(&qh->qh_list_entry, > + &hsotg->periodic_sched_assigned); > ret_val = DWC2_TRANSACTION_PERIODIC; > } > > @@ -1002,8 +1003,8 @@ enum dwc2_transaction_type dwc2_hcd_select_transactions( > * non-periodic active schedule > */ > qh_ptr = qh_ptr->next; > - list_move(&qh->qh_list_entry, > - &hsotg->non_periodic_sched_active); > + list_move_tail(&qh->qh_list_entry, > + &hsotg->non_periodic_sched_active); > > if (ret_val == DWC2_TRANSACTION_NONE) > ret_val = DWC2_TRANSACTION_NON_PERIODIC; > @@ -1176,8 +1177,8 @@ static void dwc2_process_periodic_channels(struct dwc2_hsotg *hsotg) > * Move the QH from the periodic assigned schedule to > * the periodic queued schedule > */ > - list_move(&qh->qh_list_entry, > - &hsotg->periodic_sched_queued); > + list_move_tail(&qh->qh_list_entry, > + &hsotg->periodic_sched_queued); > > /* done queuing high bandwidth */ > hsotg->queuing_high_bandwidth = 0; > diff --git a/drivers/usb/dwc2/hcd_ddma.c b/drivers/usb/dwc2/hcd_ddma.c > index 36606fc33c0d..16b261cfa92d 100644 > --- a/drivers/usb/dwc2/hcd_ddma.c > +++ b/drivers/usb/dwc2/hcd_ddma.c > @@ -1327,8 +1327,8 @@ void dwc2_hcd_complete_xfer_ddma(struct dwc2_hsotg *hsotg, > dwc2_hcd_qh_unlink(hsotg, qh); > } else { > /* Keep in assigned schedule to continue transfer */ > - list_move(&qh->qh_list_entry, > - &hsotg->periodic_sched_assigned); > + list_move_tail(&qh->qh_list_entry, > + &hsotg->periodic_sched_assigned); > /* > * If channel has been halted during giveback of urb > * then prevent any new scheduling. > diff --git a/drivers/usb/dwc2/hcd_intr.c b/drivers/usb/dwc2/hcd_intr.c > index 99efc2bd1617..2c521c00e5e0 100644 > --- a/drivers/usb/dwc2/hcd_intr.c > +++ b/drivers/usb/dwc2/hcd_intr.c > @@ -143,7 +143,7 @@ static void dwc2_sof_intr(struct dwc2_hsotg *hsotg) > * Move QH to the ready list to be executed next > * (micro)frame > */ > - list_move(&qh->qh_list_entry, > + list_move_tail(&qh->qh_list_entry, > &hsotg->periodic_sched_ready); > } > tr_type = dwc2_hcd_select_transactions(hsotg); > @@ -794,7 +794,7 @@ static void dwc2_halt_channel(struct dwc2_hsotg *hsotg, > * halt to be queued when the periodic schedule is > * processed. > */ > - list_move(&chan->qh->qh_list_entry, > + list_move_tail(&chan->qh->qh_list_entry, > &hsotg->periodic_sched_assigned); > > /* > diff --git a/drivers/usb/dwc2/hcd_queue.c b/drivers/usb/dwc2/hcd_queue.c > index e0933a9dfad7..bc632a72f611 100644 > --- a/drivers/usb/dwc2/hcd_queue.c > +++ b/drivers/usb/dwc2/hcd_queue.c > @@ -732,9 +732,11 @@ void dwc2_hcd_qh_deactivate(struct dwc2_hsotg *hsotg, struct dwc2_qh *qh, > dwc2_frame_num_le(qh->sched_frame, frame_number)) || > (hsotg->core_params->uframe_sched <= 0 && > qh->sched_frame == frame_number)) > - list_move(&qh->qh_list_entry, &hsotg->periodic_sched_ready); > + list_move_tail(&qh->qh_list_entry, > + &hsotg->periodic_sched_ready); > else > - list_move(&qh->qh_list_entry, &hsotg->periodic_sched_inactive); > + list_move_tail(&qh->qh_list_entry, > + &hsotg->periodic_sched_inactive); > } > > /**