[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <8760mjq03b.fsf@linux.intel.com>
Date: Fri, 16 Dec 2016 19:02:00 +0200
From: Felipe Balbi <balbi@...nel.org>
To: Jerry Huang <jerry.huang@....com>,
"gregkh\@linuxfoundation.org" <gregkh@...uxfoundation.org>
Cc: "linux-usb\@vger.kernel.org" <linux-usb@...r.kernel.org>,
"linux-kernel\@vger.kernel.org" <linux-kernel@...r.kernel.org>,
Rajesh Bhagat <rajesh.bhagat@....com>
Subject: RE: [PATCH] USB3/DWC3: Enable undefined length INCR burst type
Hi,
Jerry Huang <jerry.huang@....com> writes:
> Hi, Balbi,
>> -----Original Message-----
>> From: Felipe Balbi [mailto:balbi@...nel.org]
>> Sent: Friday, December 16, 2016 7:44 PM
>> To: Jerry Huang <jerry.huang@....com>; gregkh@...uxfoundation.org
>> Cc: linux-usb@...r.kernel.org; linux-kernel@...r.kernel.org; Rajesh Bhagat
>> <rajesh.bhagat@....com>
>> Subject: RE: [PATCH] USB3/DWC3: Enable undefined length INCR burst type
>>
>>
>> Hi,
>>
>> Jerry Huang <jerry.huang@....com> writes:
>> >> there's no need for that. This patch is in good format. I do have a
>> >> question,
>> >> however: how do you know this will work for all users? Burst size is
>> >> a function of how wide the interconnect where dwc3 is attached to, is.
>> > So I need to generate one new property in usb node to identify my
>> platform?
>>
>> Well, we probably need a property to be passed, yes. But let's go through it
>> all first :-)
>
> I think "snps,quirk-frame-length-adjustment" is one good reference,
> which can pass the required value to driver from DTS file.
that's not for burst increment, though.
>> >> You could very well be degrading performance for some users here. Can
>> >> you send me the result of the following commands *without* this patch
>> applied?
>> >>
>> >> # mkdir -p /d
>> >> # mount -t debugfs none /d
>> >> # cat /d/*dwc3*/regdump
>> >>
>> > Below is the regdump:
>> > root@...043ardb:/d/3000000.usb3# cat regdump
>> > GSBUSCFG0 = 0x00100080
>>
>> so you already have INCR256 here. There's one note in the databook which
>> just caught my attention. It states the following:
>>
>> "Undefined burst length has priority over all other burst lenghts."
>>
>> This means that setting both INCR16 and undefined INCR is unnecessary.
> When bit0 = 1 (Undefined Length INCR Burst Type Enable), which means:
> 1: INCR (undefined length) burst mode
> - AHB configurations: HBURST uses SINGLE or INCR
> of any length less than or equal to the largest-enabled
> burst length of INCR4/8/16/32/64/128/256.
> - AXI configurations: ARLEN/AWLEN uses any length
> less than or equal to the largest-enabled burst length
> of INCR4/8/16/32/64/128/256.
interesting, it doesn't describe what happens to OCP or PCI.
> So, after enable undefined length INCR burst and enable INCR16,
> controller will use less than or equal to 16byte.
>
>> Only Undefined INCR will be taken into consideration. Can you check with
>> your HW engineers what's the largest burst the interconnect is supposed to
>> support?
> I will check it with IP designer.
cool, thanks :-)
>> > GSBUSCFG1 = 0x00000700
>>
>> 8 AXI pipelined requests
>>
>> > GSNPSID = 0x5533280a
>>
>> 2.80a cool :-)
>>
>> I'll check these settings on my platform as well and see if there's any setting
>> which would improve transfer speed. This is a very good idea, btw, but we
>> need to be careful about how to play with it.
>>
>> --
>> balbi
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
balbi
Powered by blists - more mailing lists