* [PATCH v3] usb: dwc3: gadget: Refine the logic for resizing Tx FIFOs
@ 2024-09-17 10:13 Akash Kumar
2024-09-17 22:40 ` Thinh Nguyen
0 siblings, 1 reply; 5+ messages in thread
From: Akash Kumar @ 2024-09-17 10:13 UTC (permalink / raw)
To: Thinh Nguyen, Greg Kroah-Hartman, Jing Leng, Felipe Balbi,
Jack Pham, kernel, Wesley Cheng, Laurent Pinchart, Daniel Scally
Cc: Vijayavardhan Vennapusa, Krishna Kurapati, linux-usb,
linux-kernel, Akash Kumar
The current logic is rigid, setting num_fifos to fixed values:
3 for any maxburst greater than 1.
tx_fifo_resize_max_num for maxburst greater than 6.
Additionally, it did not differentiate much between bulk and
isochronous transfers, applying similar logic to both.
The new logic is more dynamic and tailored to the specific needs of
bulk and isochronous transfers:
Bulk Transfers: Ensures that num_fifos is optimized by considering
both the maxburst value and the maximum allowed number of FIFOs
based on the DT property tx_fifo_resize_max_num and the maximum
packet multiplier for HS.
Isochronous Transfers: Ensures that num_fifos is sufficient by
considering the maximum packet multiplier for HS and maxburst for SS,
along with a constraint with the DT property tx_fifo_resize_max_num.
This change aims to optimize the allocation of Tx FIFOs for both bulk
and isochronous endpoints, potentially improving data transfer
efficiency and overall performance. It also enhances support for all
use cases, which can be tweaked with DT parameters and the
endpoint’s maxburst and maxpacket
Signed-off-by: Akash Kumar <quic_akakum@quicinc.com>
---
Changes for v3:
Redefine logic for resizing tx fifos,added check based on
operating speed and used maxp for HS and maxburst for SS
and defined max allocation based on dt property.
Changes for v2:
Redefine logic for resizing tx fifos, handled fifo based on
minimum of maxp and maxburts.
Changes for v1:
Added additional condition to allocate tx fifo for hs isoc
eps, keeping the other resize logic same.
---
drivers/usb/dwc3/gadget.c | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 89fc690fdf34..7557bd0053a7 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -778,15 +778,19 @@ static int dwc3_gadget_resize_tx_fifos(struct dwc3_ep *dep)
ram1_depth = DWC3_RAM1_DEPTH(dwc->hwparams.hwparams7);
- if ((dep->endpoint.maxburst > 1 &&
- usb_endpoint_xfer_bulk(dep->endpoint.desc)) ||
- usb_endpoint_xfer_isoc(dep->endpoint.desc))
- num_fifos = 3;
+ if (dwc->gadget->speed <= USB_SPEED_HIGH &&
+ (usb_endpoint_xfer_bulk(dep->endpoint.desc) ||
+ usb_endpoint_xfer_isoc(dep->endpoint.desc)))
+ num_fifos = min_t(unsigned int,
+ usb_endpoint_maxp_mult(dep->endpoint.desc) + 1,
+ dwc->tx_fifo_resize_max_num);
- if (dep->endpoint.maxburst > 6 &&
+ if (dwc->gadget->speed > USB_SPEED_HIGH &&
(usb_endpoint_xfer_bulk(dep->endpoint.desc) ||
- usb_endpoint_xfer_isoc(dep->endpoint.desc)) && DWC3_IP_IS(DWC31))
- num_fifos = dwc->tx_fifo_resize_max_num;
+ usb_endpoint_xfer_isoc(dep->endpoint.desc)))
+ num_fifos = min_t(unsigned int,
+ dep->endpoint.maxburst,
+ dwc->tx_fifo_resize_max_num);
/* FIFO size for a single buffer */
fifo = dwc3_gadget_calc_tx_fifo_size(dwc, 1);
--
2.17.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3] usb: dwc3: gadget: Refine the logic for resizing Tx FIFOs
2024-09-17 10:13 [PATCH v3] usb: dwc3: gadget: Refine the logic for resizing Tx FIFOs Akash Kumar
@ 2024-09-17 22:40 ` Thinh Nguyen
2024-09-18 9:43 ` AKASH KUMAR
0 siblings, 1 reply; 5+ messages in thread
From: Thinh Nguyen @ 2024-09-17 22:40 UTC (permalink / raw)
To: Akash Kumar
Cc: Thinh Nguyen, Greg Kroah-Hartman, Jing Leng, Felipe Balbi,
Jack Pham, kernel, Wesley Cheng, Laurent Pinchart, Daniel Scally,
Vijayavardhan Vennapusa, Krishna Kurapati, linux-usb,
linux-kernel
On Tue, Sep 17, 2024, Akash Kumar wrote:
> The current logic is rigid, setting num_fifos to fixed values:
>
> 3 for any maxburst greater than 1.
> tx_fifo_resize_max_num for maxburst greater than 6.
> Additionally, it did not differentiate much between bulk and
> isochronous transfers, applying similar logic to both.
>
> The new logic is more dynamic and tailored to the specific needs of
> bulk and isochronous transfers:
>
> Bulk Transfers: Ensures that num_fifos is optimized by considering
> both the maxburst value and the maximum allowed number of FIFOs
> based on the DT property tx_fifo_resize_max_num and the maximum
> packet multiplier for HS.
>
> Isochronous Transfers: Ensures that num_fifos is sufficient by
> considering the maximum packet multiplier for HS and maxburst for SS,
> along with a constraint with the DT property tx_fifo_resize_max_num.
>
> This change aims to optimize the allocation of Tx FIFOs for both bulk
> and isochronous endpoints, potentially improving data transfer
> efficiency and overall performance. It also enhances support for all
> use cases, which can be tweaked with DT parameters and the
You should clarify that this is only verified on your specific platform
and specific application. It may not be applicable to all cases.
However, we try our best to make it so. Please reword as such. The
commit message makes it seems that this will work for all cases.
> endpoint’s maxburst and maxpacket
>
> Signed-off-by: Akash Kumar <quic_akakum@quicinc.com>
> ---
> Changes for v3:
> Redefine logic for resizing tx fifos,added check based on
> operating speed and used maxp for HS and maxburst for SS
> and defined max allocation based on dt property.
>
> Changes for v2:
> Redefine logic for resizing tx fifos, handled fifo based on
> minimum of maxp and maxburts.
>
> Changes for v1:
> Added additional condition to allocate tx fifo for hs isoc
> eps, keeping the other resize logic same.
> ---
> drivers/usb/dwc3/gadget.c | 18 +++++++++++-------
> 1 file changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 89fc690fdf34..7557bd0053a7 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -778,15 +778,19 @@ static int dwc3_gadget_resize_tx_fifos(struct dwc3_ep *dep)
>
> ram1_depth = DWC3_RAM1_DEPTH(dwc->hwparams.hwparams7);
>
> - if ((dep->endpoint.maxburst > 1 &&
> - usb_endpoint_xfer_bulk(dep->endpoint.desc)) ||
> - usb_endpoint_xfer_isoc(dep->endpoint.desc))
> - num_fifos = 3;
> + if (dwc->gadget->speed <= USB_SPEED_HIGH &&
> + (usb_endpoint_xfer_bulk(dep->endpoint.desc) ||
> + usb_endpoint_xfer_isoc(dep->endpoint.desc)))
> + num_fifos = min_t(unsigned int,
> + usb_endpoint_maxp_mult(dep->endpoint.desc) + 1,
This logic looks wrong. This implies maxp_mult is applicable to bulk and
that it's also applicable to speed below highspeed, which it isn't.
> + dwc->tx_fifo_resize_max_num);
>
> - if (dep->endpoint.maxburst > 6 &&
> + if (dwc->gadget->speed > USB_SPEED_HIGH &&
> (usb_endpoint_xfer_bulk(dep->endpoint.desc) ||
> - usb_endpoint_xfer_isoc(dep->endpoint.desc)) && DWC3_IP_IS(DWC31))
> - num_fifos = dwc->tx_fifo_resize_max_num;
> + usb_endpoint_xfer_isoc(dep->endpoint.desc)))
> + num_fifos = min_t(unsigned int,
> + dep->endpoint.maxburst,
maxburst can be 0 right?
> + dwc->tx_fifo_resize_max_num);
>
> /* FIFO size for a single buffer */
> fifo = dwc3_gadget_calc_tx_fifo_size(dwc, 1);
> --
> 2.17.1
>
Thanks,
Thinh
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3] usb: dwc3: gadget: Refine the logic for resizing Tx FIFOs
2024-09-17 22:40 ` Thinh Nguyen
@ 2024-09-18 9:43 ` AKASH KUMAR
2024-09-26 22:16 ` Thinh Nguyen
0 siblings, 1 reply; 5+ messages in thread
From: AKASH KUMAR @ 2024-09-18 9:43 UTC (permalink / raw)
To: Thinh Nguyen
Cc: Greg Kroah-Hartman, Felipe Balbi, Jack Pham, kernel,
Wesley Cheng, Laurent Pinchart, Daniel Scally,
Vijayavardhan Vennapusa, Krishna Kurapati, linux-usb,
linux-kernel
Hi Thinh,
On 9/18/2024 4:10 AM, Thinh Nguyen wrote:
> On Tue, Sep 17, 2024, Akash Kumar wrote:
>> The current logic is rigid, setting num_fifos to fixed values:
>>
>> 3 for any maxburst greater than 1.
>> tx_fifo_resize_max_num for maxburst greater than 6.
>> Additionally, it did not differentiate much between bulk and
>> isochronous transfers, applying similar logic to both.
>>
>> The new logic is more dynamic and tailored to the specific needs of
>> bulk and isochronous transfers:
>>
>> Bulk Transfers: Ensures that num_fifos is optimized by considering
>> both the maxburst value and the maximum allowed number of FIFOs
>> based on the DT property tx_fifo_resize_max_num and the maximum
>> packet multiplier for HS.
>>
>> Isochronous Transfers: Ensures that num_fifos is sufficient by
>> considering the maximum packet multiplier for HS and maxburst for SS,
>> along with a constraint with the DT property tx_fifo_resize_max_num.
>>
>> This change aims to optimize the allocation of Tx FIFOs for both bulk
>> and isochronous endpoints, potentially improving data transfer
>> efficiency and overall performance. It also enhances support for all
>> use cases, which can be tweaked with DT parameters and the
> You should clarify that this is only verified on your specific platform
> and specific application. It may not be applicable to all cases.
> However, we try our best to make it so. Please reword as such. The
> commit message makes it seems that this will work for all cases.
Sure will rephrase it, but i believe since it can be customized based on
maxpacket and
maxburst , anyone can choose his desired fifo size and since we will be
get maxp as 1 (for any streaming packet < 2048) and additional +1 which
we are appending which
makes minimum 2k fifo size for HS which should be sufficient for HS
transfers and it can increased with maxpacket when increased to 2k or 3k.
Similar with maxburst for >= SS.
>
>> endpoint’s maxburst and maxpacket
>>
>> Signed-off-by: Akash Kumar <quic_akakum@quicinc.com>
>> ---
>> Changes for v3:
>> Redefine logic for resizing tx fifos,added check based on
>> operating speed and used maxp for HS and maxburst for SS
>> and defined max allocation based on dt property.
>>
>> Changes for v2:
>> Redefine logic for resizing tx fifos, handled fifo based on
>> minimum of maxp and maxburts.
>>
>> Changes for v1:
>> Added additional condition to allocate tx fifo for hs isoc
>> eps, keeping the other resize logic same.
>> ---
>> drivers/usb/dwc3/gadget.c | 18 +++++++++++-------
>> 1 file changed, 11 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>> index 89fc690fdf34..7557bd0053a7 100644
>> --- a/drivers/usb/dwc3/gadget.c
>> +++ b/drivers/usb/dwc3/gadget.c
>> @@ -778,15 +778,19 @@ static int dwc3_gadget_resize_tx_fifos(struct dwc3_ep *dep)
>>
>> ram1_depth = DWC3_RAM1_DEPTH(dwc->hwparams.hwparams7);
>>
>> - if ((dep->endpoint.maxburst > 1 &&
>> - usb_endpoint_xfer_bulk(dep->endpoint.desc)) ||
>> - usb_endpoint_xfer_isoc(dep->endpoint.desc))
>> - num_fifos = 3;
>> + if (dwc->gadget->speed <= USB_SPEED_HIGH &&
>> + (usb_endpoint_xfer_bulk(dep->endpoint.desc) ||
>> + usb_endpoint_xfer_isoc(dep->endpoint.desc)))
>> + num_fifos = min_t(unsigned int,
>> + usb_endpoint_maxp_mult(dep->endpoint.desc) + 1,
> This logic looks wrong. This implies maxp_mult is applicable to bulk and
> that it's also applicable to speed below highspeed, which it isn't.
This is to make it generic as 2k fifo should be suffiecient for bulk eps
in HS as explained above
and we can increase maxpacket size when required which will incresse
fifo size as well.
>
>> + dwc->tx_fifo_resize_max_num);
>>
>> - if (dep->endpoint.maxburst > 6 &&
>> + if (dwc->gadget->speed > USB_SPEED_HIGH &&
>> (usb_endpoint_xfer_bulk(dep->endpoint.desc) ||
>> - usb_endpoint_xfer_isoc(dep->endpoint.desc)) && DWC3_IP_IS(DWC31))
>> - num_fifos = dwc->tx_fifo_resize_max_num;
>> + usb_endpoint_xfer_isoc(dep->endpoint.desc)))
>> + num_fifos = min_t(unsigned int,
>> + dep->endpoint.maxburst,
> maxburst can be 0 right?
At composite layer we are incrementing maxburst by 1 so while allocating
fifo maxburst can be as minimum as 1,
which is required and can be changed anytime with exposed configfs
attribute.
>
>> + dwc->tx_fifo_resize_max_num);
>>
>> /* FIFO size for a single buffer */
>> fifo = dwc3_gadget_calc_tx_fifo_size(dwc, 1);
>> --
>> 2.17.1
Thanks,
Akash
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3] usb: dwc3: gadget: Refine the logic for resizing Tx FIFOs
2024-09-18 9:43 ` AKASH KUMAR
@ 2024-09-26 22:16 ` Thinh Nguyen
2024-09-27 15:25 ` AKASH KUMAR
0 siblings, 1 reply; 5+ messages in thread
From: Thinh Nguyen @ 2024-09-26 22:16 UTC (permalink / raw)
To: AKASH KUMAR
Cc: Thinh Nguyen, Greg Kroah-Hartman, Felipe Balbi, Jack Pham,
kernel, Wesley Cheng, Laurent Pinchart, Daniel Scally,
Vijayavardhan Vennapusa, Krishna Kurapati, linux-usb,
linux-kernel
Hi Akash,
Sorry for the delay response.
On Wed, Sep 18, 2024, AKASH KUMAR wrote:
> Hi Thinh,
>
> On 9/18/2024 4:10 AM, Thinh Nguyen wrote:
> > On Tue, Sep 17, 2024, Akash Kumar wrote:
> > > The current logic is rigid, setting num_fifos to fixed values:
> > >
> > > 3 for any maxburst greater than 1.
> > > tx_fifo_resize_max_num for maxburst greater than 6.
> > > Additionally, it did not differentiate much between bulk and
> > > isochronous transfers, applying similar logic to both.
> > >
> > > The new logic is more dynamic and tailored to the specific needs of
> > > bulk and isochronous transfers:
> > >
> > > Bulk Transfers: Ensures that num_fifos is optimized by considering
> > > both the maxburst value and the maximum allowed number of FIFOs
> > > based on the DT property tx_fifo_resize_max_num and the maximum
> > > packet multiplier for HS.
> > >
> > > Isochronous Transfers: Ensures that num_fifos is sufficient by
> > > considering the maximum packet multiplier for HS and maxburst for SS,
> > > along with a constraint with the DT property tx_fifo_resize_max_num.
> > >
> > > This change aims to optimize the allocation of Tx FIFOs for both bulk
> > > and isochronous endpoints, potentially improving data transfer
> > > efficiency and overall performance. It also enhances support for all
> > > use cases, which can be tweaked with DT parameters and the
> > You should clarify that this is only verified on your specific platform
> > and specific application. It may not be applicable to all cases.
> > However, we try our best to make it so. Please reword as such. The
> > commit message makes it seems that this will work for all cases.
> Sure will rephrase it, but i believe since it can be customized based on
> maxpacket and
> maxburst , anyone can choose his desired fifo size and since we will be get
The logic depends on what the endpoint's setting will be. The endpoint
setting is not some runtime field that the user can set. The point I
want to make is that this is validated on your platform, but it may not
work for all cases. (e.g. this may use up more available txfifo for
other endpoints on a more fifo constraint platform)
> maxp as 1 (for any streaming packet < 2048) and additional +1 which we are
> appending which
> makes minimum 2k fifo size for HS which should be sufficient for HS
> transfers and it can increased with maxpacket when increased to 2k or 3k.
> Similar with maxburst for >= SS.
> >
> > > endpoint’s maxburst and maxpacket
> > >
> > > Signed-off-by: Akash Kumar <quic_akakum@quicinc.com>
> > > ---
> > > Changes for v3:
> > > Redefine logic for resizing tx fifos,added check based on
> > > operating speed and used maxp for HS and maxburst for SS
> > > and defined max allocation based on dt property.
> > >
> > > Changes for v2:
> > > Redefine logic for resizing tx fifos, handled fifo based on
> > > minimum of maxp and maxburts.
> > >
> > > Changes for v1:
> > > Added additional condition to allocate tx fifo for hs isoc
> > > eps, keeping the other resize logic same.
> > > ---
> > > drivers/usb/dwc3/gadget.c | 18 +++++++++++-------
> > > 1 file changed, 11 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> > > index 89fc690fdf34..7557bd0053a7 100644
> > > --- a/drivers/usb/dwc3/gadget.c
> > > +++ b/drivers/usb/dwc3/gadget.c
> > > @@ -778,15 +778,19 @@ static int dwc3_gadget_resize_tx_fifos(struct dwc3_ep *dep)
> > > ram1_depth = DWC3_RAM1_DEPTH(dwc->hwparams.hwparams7);
> > > - if ((dep->endpoint.maxburst > 1 &&
> > > - usb_endpoint_xfer_bulk(dep->endpoint.desc)) ||
> > > - usb_endpoint_xfer_isoc(dep->endpoint.desc))
> > > - num_fifos = 3;
> > > + if (dwc->gadget->speed <= USB_SPEED_HIGH &&
> > > + (usb_endpoint_xfer_bulk(dep->endpoint.desc) ||
> > > + usb_endpoint_xfer_isoc(dep->endpoint.desc)))
> > > + num_fifos = min_t(unsigned int,
> > > + usb_endpoint_maxp_mult(dep->endpoint.desc) + 1,
> > This logic looks wrong. This implies maxp_mult is applicable to bulk and
> > that it's also applicable to speed below highspeed, which it isn't.
> This is to make it generic as 2k fifo should be suffiecient for bulk eps in
> HS as explained above
> and we can increase maxpacket size when required which will incresse fifo
> size as well.
I think you missed my point here. The maxp_mult should only be checked
against isoc and only for highspeed. Please split it out. Make it clear
that the bulk uses 2k txfifo for highspeed and below.
> >
> > > + dwc->tx_fifo_resize_max_num);
> > > - if (dep->endpoint.maxburst > 6 &&
> > > + if (dwc->gadget->speed > USB_SPEED_HIGH &&
> > > (usb_endpoint_xfer_bulk(dep->endpoint.desc) ||
> > > - usb_endpoint_xfer_isoc(dep->endpoint.desc)) && DWC3_IP_IS(DWC31))
> > > - num_fifos = dwc->tx_fifo_resize_max_num;
> > > + usb_endpoint_xfer_isoc(dep->endpoint.desc)))
> > > + num_fifos = min_t(unsigned int,
> > > + dep->endpoint.maxburst,
> > maxburst can be 0 right?
> At composite layer we are incrementing maxburst by 1 so while allocating
> fifo maxburst can be as minimum as 1,
> which is required and can be changed anytime with exposed configfs
> attribute.
Ok.
> >
> > > + dwc->tx_fifo_resize_max_num);
> > > /* FIFO size for a single buffer */
> > > fifo = dwc3_gadget_calc_tx_fifo_size(dwc, 1);
> > > --
> > > 2.17.1
Thanks,
Thinh
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3] usb: dwc3: gadget: Refine the logic for resizing Tx FIFOs
2024-09-26 22:16 ` Thinh Nguyen
@ 2024-09-27 15:25 ` AKASH KUMAR
0 siblings, 0 replies; 5+ messages in thread
From: AKASH KUMAR @ 2024-09-27 15:25 UTC (permalink / raw)
To: Thinh Nguyen
Cc: Greg Kroah-Hartman, Felipe Balbi, Jack Pham, kernel,
Wesley Cheng, Laurent Pinchart, Daniel Scally,
Vijayavardhan Vennapusa, Krishna Kurapati, linux-usb,
linux-kernel
Hi Thinh,
On 9/27/2024 3:46 AM, Thinh Nguyen wrote:
> Hi Akash,
>
> Sorry for the delay response.
>
> On Wed, Sep 18, 2024, AKASH KUMAR wrote:
>> Hi Thinh,
>>
>> On 9/18/2024 4:10 AM, Thinh Nguyen wrote:
>>> On Tue, Sep 17, 2024, Akash Kumar wrote:
>>>> The current logic is rigid, setting num_fifos to fixed values:
>>>>
>>>> 3 for any maxburst greater than 1.
>>>> tx_fifo_resize_max_num for maxburst greater than 6.
>>>> Additionally, it did not differentiate much between bulk and
>>>> isochronous transfers, applying similar logic to both.
>>>>
>>>> The new logic is more dynamic and tailored to the specific needs of
>>>> bulk and isochronous transfers:
>>>>
>>>> Bulk Transfers: Ensures that num_fifos is optimized by considering
>>>> both the maxburst value and the maximum allowed number of FIFOs
>>>> based on the DT property tx_fifo_resize_max_num and the maximum
>>>> packet multiplier for HS.
>>>>
>>>> Isochronous Transfers: Ensures that num_fifos is sufficient by
>>>> considering the maximum packet multiplier for HS and maxburst for SS,
>>>> along with a constraint with the DT property tx_fifo_resize_max_num.
>>>>
>>>> This change aims to optimize the allocation of Tx FIFOs for both bulk
>>>> and isochronous endpoints, potentially improving data transfer
>>>> efficiency and overall performance. It also enhances support for all
>>>> use cases, which can be tweaked with DT parameters and the
>>> You should clarify that this is only verified on your specific platform
>>> and specific application. It may not be applicable to all cases.
>>> However, we try our best to make it so. Please reword as such. The
>>> commit message makes it seems that this will work for all cases.
>> Sure will rephrase it, but i believe since it can be customized based on
>> maxpacket and
>> maxburst , anyone can choose his desired fifo size and since we will be get
> The logic depends on what the endpoint's setting will be. The endpoint
> setting is not some runtime field that the user can set. The point I
> want to make is that this is validated on your platform, but it may not
> work for all cases. (e.g. this may use up more available txfifo for
> other endpoints on a more fifo constraint platform)
Ack.
>
>> maxp as 1 (for any streaming packet < 2048) and additional +1 which we are
>> appending which
>> makes minimum 2k fifo size for HS which should be sufficient for HS
>> transfers and it can increased with maxpacket when increased to 2k or 3k.
>> Similar with maxburst for >= SS.
>>>> endpoint’s maxburst and maxpacket
>>>>
>>>> Signed-off-by: Akash Kumar <quic_akakum@quicinc.com>
>>>> ---
>>>> Changes for v3:
>>>> Redefine logic for resizing tx fifos,added check based on
>>>> operating speed and used maxp for HS and maxburst for SS
>>>> and defined max allocation based on dt property.
>>>>
>>>> Changes for v2:
>>>> Redefine logic for resizing tx fifos, handled fifo based on
>>>> minimum of maxp and maxburts.
>>>>
>>>> Changes for v1:
>>>> Added additional condition to allocate tx fifo for hs isoc
>>>> eps, keeping the other resize logic same.
>>>> ---
>>>> drivers/usb/dwc3/gadget.c | 18 +++++++++++-------
>>>> 1 file changed, 11 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>>>> index 89fc690fdf34..7557bd0053a7 100644
>>>> --- a/drivers/usb/dwc3/gadget.c
>>>> +++ b/drivers/usb/dwc3/gadget.c
>>>> @@ -778,15 +778,19 @@ static int dwc3_gadget_resize_tx_fifos(struct dwc3_ep *dep)
>>>> ram1_depth = DWC3_RAM1_DEPTH(dwc->hwparams.hwparams7);
>>>> - if ((dep->endpoint.maxburst > 1 &&
>>>> - usb_endpoint_xfer_bulk(dep->endpoint.desc)) ||
>>>> - usb_endpoint_xfer_isoc(dep->endpoint.desc))
>>>> - num_fifos = 3;
>>>> + if (dwc->gadget->speed <= USB_SPEED_HIGH &&
>>>> + (usb_endpoint_xfer_bulk(dep->endpoint.desc) ||
>>>> + usb_endpoint_xfer_isoc(dep->endpoint.desc)))
>>>> + num_fifos = min_t(unsigned int,
>>>> + usb_endpoint_maxp_mult(dep->endpoint.desc) + 1,
>>> This logic looks wrong. This implies maxp_mult is applicable to bulk and
>>> that it's also applicable to speed below highspeed, which it isn't.
>> This is to make it generic as 2k fifo should be suffiecient for bulk eps in
>> HS as explained above
>> and we can increase maxpacket size when required which will incresse fifo
>> size as well.
> I think you missed my point here. The maxp_mult should only be checked
> against isoc and only for highspeed. Please split it out. Make it clear
> that the bulk uses 2k txfifo for highspeed and below.
Sure let me update.
>
>>>> + dwc->tx_fifo_resize_max_num);
>>>> - if (dep->endpoint.maxburst > 6 &&
>>>> + if (dwc->gadget->speed > USB_SPEED_HIGH &&
>>>> (usb_endpoint_xfer_bulk(dep->endpoint.desc) ||
>>>> - usb_endpoint_xfer_isoc(dep->endpoint.desc)) && DWC3_IP_IS(DWC31))
>>>> - num_fifos = dwc->tx_fifo_resize_max_num;
>>>> + usb_endpoint_xfer_isoc(dep->endpoint.desc)))
>>>> + num_fifos = min_t(unsigned int,
>>>> + dep->endpoint.maxburst,
>>> maxburst can be 0 right?
>> At composite layer we are incrementing maxburst by 1 so while allocating
>> fifo maxburst can be as minimum as 1,
>> which is required and can be changed anytime with exposed configfs
>> attribute.
> Ok.
>
>>>> + dwc->tx_fifo_resize_max_num);
>>>> /* FIFO size for a single buffer */
>>>> fifo = dwc3_gadget_calc_tx_fifo_size(dwc, 1);
>>>> --
>>>> 2.17.1
Thanks,
Akash
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-09-27 15:25 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-09-17 10:13 [PATCH v3] usb: dwc3: gadget: Refine the logic for resizing Tx FIFOs Akash Kumar
2024-09-17 22:40 ` Thinh Nguyen
2024-09-18 9:43 ` AKASH KUMAR
2024-09-26 22:16 ` Thinh Nguyen
2024-09-27 15:25 ` AKASH KUMAR
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®