Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934199AbbKSQ1H (ORCPT ); Thu, 19 Nov 2015 11:27:07 -0500 Received: from mail-yk0-f176.google.com ([209.85.160.176]:35066 "EHLO mail-yk0-f176.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933201AbbKSQ1E (ORCPT ); Thu, 19 Nov 2015 11:27:04 -0500 MIME-Version: 1.0 In-Reply-To: <87lh9uf1fw.fsf@saruman.tx.rr.com> References: <1447721484-22548-1-git-send-email-dianders@chromium.org> <1447721484-22548-7-git-send-email-dianders@chromium.org> <87lh9uf1fw.fsf@saruman.tx.rr.com> Date: Thu, 19 Nov 2015 08:27:03 -0800 X-Google-Sender-Auth: dUmNj82dgIduzBxXFVW89ZCmdJI Message-ID: Subject: Re: [PATCH v3 6/8] usb: dwc2: host: Assume all devices are on one single_tt hub From: Doug Anderson To: Felipe Balbi Cc: John Youn , Yunzhi Li , =?UTF-8?Q?Heiko_St=C3=BCbner?= , "open list:ARM/Rockchip SoC..." , Julius Werner , "Herrero, Gregory" , "Kaukab, Yousaf" , Dinh Nguyen , Alan Stern , Ming Lei , John Youn , Greg Kroah-Hartman , "linux-usb@vger.kernel.org" , "linux-kernel@vger.kernel.org" Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3876 Lines: 99 Felipe, On Thu, Nov 19, 2015 at 7:34 AM, Felipe Balbi wrote: > > Hi, > > Douglas Anderson writes: >> Until we have logic to determine which devices share the same TT let's >> add logic to assume that all devices on a given dwc2 controller are on >> one single_tt hub. This is better than the previous code that assumed >> that all devices were on one multi_tt hub, since single_tt hubs >> appear (in my experience) to be much more common and any schedule that >> would work on a single_tt hub will also work on a multi_tt hub. This >> will prevent more than 8 total low/full speed devices to be on the bus >> at one time, but that's a reasonable restriction until we've made things >> smarter. >> >> Signed-off-by: Douglas Anderson >> --- >> Changes in v3: >> - Assuming single_tt is new for v3; not terribly well tested (yet). >> >> Changes in v2: None >> >> drivers/usb/dwc2/core.h | 1 + >> drivers/usb/dwc2/hcd_queue.c | 40 +++++++++++++++++++++++++++++++++++++++- >> 2 files changed, 40 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h >> index 567ee2c9e69f..09aa2b5ae29e 100644 >> --- a/drivers/usb/dwc2/core.h >> +++ b/drivers/usb/dwc2/core.h >> @@ -782,6 +782,7 @@ struct dwc2_hsotg { >> u16 periodic_usecs; >> unsigned long periodic_bitmap[DIV_ROUND_UP(TOTAL_PERIODIC_USEC, >> BITS_PER_LONG)]; >> + bool has_split[8]; > > why don't you use a u8 instead then just set each bit for each > "has_split" you need to take care of. This array is kinda ugly. Let's actually drop this patch completely. Julius and I sat down and he talked me through things, and with my current understanding the current microframe scheduler in dwc2 is broken enough that small little band-aids like this will do little more than just push the problems around. I'm a good portion of the way through a better microframe scheduler. I have no doubt that it won't be perfect, but hopefully it will at least be based in reality... My latest thinking on the patches in this series: 1. usb: dwc2: rockchip: Make the max_transfer_size automatic I'll probably separate this out into its own patch so I can stop sending it as part of this series. ...or if someone wanted to land it then I won't bother. 2. usb: dwc2: host: Get aligned DMA in a more supported way Still can land any time and has good benefits. I believe that I can't separate this because it will cause conflicts with scheduler patches. 3. usb: dwc2: host: Add scheduler tracing Would be nice to land. 4. usb: dwc2: host: Rewrite the microframe scheduler 5. usb: dwc2: host: Keep track of and use our scheduled microframe 6. usb: dwc2: host: Assume all devices are on one single_tt hub Please drop patches 4-6 right now. 7. usb: dwc2: host: Add a delay before releasing periodic bandwidth 8. usb: dwc2: host: Giveback URB in tasklet context I'll probably move these back up in the series (like in v2) and put microframe rewrite atop them. With my current understanding the scheduling is so broken today that the concerns Alan brought up can wait until we have a proper scheduler to be addressed. In the meantime getting the huge boost in interrupt speed will help with dwc2's correctness (and performance) because it means we're much less likely to miss SOF interrupts. If anyone has any review time, giving a review to v2 of these patches would be helpful. Otherwise I'll double check that v2 still looks good with my current understanding and eventually post them again. -Doug -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/