| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <56A89ABA.7000809@rock-chips.com>
Date: Wed, 27 Jan 2016 18:23:54 +0800
From: Kever Yang <kever.yang@...k-chips.com>
To: Douglas Anderson <dianders@...omium.org>,
John Youn <John.Youn@...opsys.com>, balbi@...com
CC: william.wu@...k-chips.com, huangtao@...k-chips.com,
heiko@...ech.de, linux-rockchip@...ts.infradead.org,
Julius Werner <jwerner@...omium.org>,
gregory.herrero@...el.com, yousaf.kaukab@...el.com,
dinguyen@...nsource.altera.com, stern@...land.harvard.edu,
ming.lei@...onical.com, johnyoun@...opsys.com,
gregkh@...uxfoundation.org, linux-usb@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5 06/21] usb: dwc2: host: Always add to the tail of queues
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 <kever.yang@...k-chips.com>
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 <dianders@...omium.org>
> ---
> 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);
> }
>
> /**
Powered by blists - more mailing lists