On 11/19/2015 8:27 AM, Doug Anderson wrote:
> Felipe,
>
> On Thu, Nov 19, 2015 at 7:34 AM, Felipe Balbi <balbi@xxxxxx> wrote:
>>
>> Hi,
>>
>> Douglas Anderson <dianders@xxxxxxxxxxxx> 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 <dianders@xxxxxxxxxxxx>
>>> ---
>>> 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
>
Hi Doug,
Patches 1-3:
Acked-by: John Youn <johnyoun@xxxxxxxxxxxx>
Patch 2:
Tested-by: John Youn <johnyoun@xxxxxxxxxxxx>
Tested on core version 3.20 using internal TE for un-aligned
buffers.
I haven't had time to look into the scheduling patches yet. But I
agree with you that there are fundamental problems. I'll await
your rewrite.
Regards,
John
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
