linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Wesley Cheng <wcheng@codeaurora.org>
To: Greg KH <gregkh@linuxfoundation.org>
Cc: agross@kernel.org, bjorn.andersson@linaro.org, balbi@kernel.org,
	robh+dt@kernel.org, frowand.list@gmail.com,
	linux-arm-msm@vger.kernel.org, linux-usb@vger.kernel.org,
	linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
	jackp@codeaurora.org, fntoth@gmail.com
Subject: Re: [PATCH v12 3/6] usb: dwc3: Resize TX FIFOs to meet EP bursting requirements
Date: Tue, 6 Jul 2021 13:19:38 -0700	[thread overview]
Message-ID: <2ee71465-d921-8904-2009-1e46ad1a988b@codeaurora.org> (raw)
In-Reply-To: <YOSdRKTy3+CdV/UF@kroah.com>



On 7/6/2021 11:13 AM, Greg KH wrote:
> On Fri, Jul 02, 2021 at 02:37:32AM -0700, Wesley Cheng wrote:
>> Some devices have USB compositions which may require multiple endpoints
>> that support EP bursting.  HW defined TX FIFO sizes may not always be
>> sufficient for these compositions.  By utilizing flexible TX FIFO
>> allocation, this allows for endpoints to request the required FIFO depth to
>> achieve higher bandwidth.  With some higher bMaxBurst configurations, using
>> a larger TX FIFO size results in better TX throughput.
>>
>> By introducing the check_config() callback, the resizing logic can fetch
>> the maximum number of endpoints used in the USB composition (can contain
>> multiple configurations), which helps ensure that the resizing logic can
>> fulfill the configuration(s), or return an error to the gadget layer
>> otherwise during bind time.
>>
>> Signed-off-by: Wesley Cheng <wcheng@codeaurora.org>
>> ---
>>  drivers/usb/dwc3/core.c   |   9 ++
>>  drivers/usb/dwc3/core.h   |  15 ++++
>>  drivers/usb/dwc3/ep0.c    |   2 +
>>  drivers/usb/dwc3/gadget.c | 221 ++++++++++++++++++++++++++++++++++++++++++++++
>>  4 files changed, 247 insertions(+)
>>
>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>> index e0a8e79..a7bcdb9d 100644
>> --- a/drivers/usb/dwc3/core.c
>> +++ b/drivers/usb/dwc3/core.c
>> @@ -1267,6 +1267,7 @@ static void dwc3_get_properties(struct dwc3 *dwc)
>>  	u8			rx_max_burst_prd;
>>  	u8			tx_thr_num_pkt_prd;
>>  	u8			tx_max_burst_prd;
>> +	u8			tx_fifo_resize_max_num;
>>  	const char		*usb_psy_name;
>>  	int			ret;
>>  
>> @@ -1282,6 +1283,8 @@ static void dwc3_get_properties(struct dwc3 *dwc)
>>  	 */
>>  	hird_threshold = 12;
>>  
>> +	tx_fifo_resize_max_num = 6;
>> +

Hi Greg,
> 
> No comment as to why 6 was picked, like the other defaults in this
> function?
> 
> Why was 6 picked?
> 
> 
Talked with Thinh about this sometime back about why 6 was picked.  It
was just an arbitrary setting we decided on throughout our testing, as
that was what provided the best tput numbers for our system.  Hence why
it was suggested to have a separate property, so other vendors can set
this to accommodate their difference in HW latencies.

>>  	dwc->maximum_speed = usb_get_maximum_speed(dev);
>>  	dwc->max_ssp_rate = usb_get_maximum_ssp_rate(dev);
>>  	dwc->dr_mode = usb_get_dr_mode(dev);
>> @@ -1325,6 +1328,10 @@ static void dwc3_get_properties(struct dwc3 *dwc)
>>  				&tx_thr_num_pkt_prd);
>>  	device_property_read_u8(dev, "snps,tx-max-burst-prd",
>>  				&tx_max_burst_prd);
>> +	dwc->do_fifo_resize = device_property_read_bool(dev,
>> +							"tx-fifo-resize");
>> +	device_property_read_u8(dev, "tx-fifo-max-num",
>> +				&tx_fifo_resize_max_num);
> 
> So you overwrite the "max" with whatever is given to you?  What if
> tx-fifo-resize is not enabled?
>
If tx-fifo-resize is not enabled, then there shouldn't be anything that
will reference this property.  As mentioned in the previous comment, HW
vendors may not need a FIFO size of 6 max packets for their particular
system, so they should be able to program this to their needs.

If someone programs to this a large number, the logic works where it
will allocate based off the space left after ensuring enough space for 1
FIFO per ep.

Thanks
Wesley Cheng

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

  reply	other threads:[~2021-07-06 20:19 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-02  9:37 [PATCH v12 0/6] Re-introduce TX FIFO resize for larger EP bursting Wesley Cheng
2021-07-02  9:37 ` [PATCH v12 1/6] usb: gadget: udc: core: Introduce check_config to verify USB configuration Wesley Cheng
2021-07-02  9:37 ` [PATCH v12 2/6] usb: gadget: configfs: Check USB configuration before adding Wesley Cheng
2021-07-02  9:37 ` [PATCH v12 3/6] usb: dwc3: Resize TX FIFOs to meet EP bursting requirements Wesley Cheng
2021-07-06 18:13   ` Greg KH
2021-07-06 20:19     ` Wesley Cheng [this message]
2021-07-07  6:36       ` Greg KH
2021-07-02  9:37 ` [PATCH v12 4/6] of: Add stub for of_add_property() Wesley Cheng
2021-07-02  9:37 ` [PATCH v12 5/6] usb: dwc3: dwc3-qcom: Enable tx-fifo-resize property by default Wesley Cheng
2021-07-02  9:37 ` [PATCH v12 6/6] dt-bindings: usb: dwc3: Update dwc3 TX fifo properties Wesley Cheng
2021-07-06 18:14   ` Greg KH

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2ee71465-d921-8904-2009-1e46ad1a988b@codeaurora.org \
    --to=wcheng@codeaurora.org \
    --cc=agross@kernel.org \
    --cc=balbi@kernel.org \
    --cc=bjorn.andersson@linaro.org \
    --cc=devicetree@vger.kernel.org \
    --cc=fntoth@gmail.com \
    --cc=frowand.list@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jackp@codeaurora.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=robh+dt@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).