++ Alan Stern
On Fri, Nov 08, 2024, Selvarasu Ganesan wrote:
- ram_depth = spram_type ? DWC3_RAM0_DEPTH(dwc->hwparams.hwparams6) :
DWC3_RAM1_DEPTH(dwc->hwparams.hwparams7);Don't use spram_type as a boolean. Perhaps define a macro for type value 1 and 0 (for single vs 2-port)
Are you expecting something like below?
#define DWC3_SINGLE_PORT_RAM 1 #define DWC3_TW0_PORT_RAM 0
Yes. I think it's more readable if we name the variable to "ram_type" and use the macros above as I suggested.
If you still plan to use it as boolean, please rename the variable spram_type to is_single_port_ram (no one knows what "spram_type" mean without the programming guide or some documention).
< snip >
We may need to think a little more on how to budgeting the resource properly to accomodate for different requirements. If there's no single formula to satisfy for all platform, perhaps we may need to introduce parameters that users can set base on the needs of their application.
Agree. Need to introduce some parameters to control the required fifos by user that based their usecase. Here's a rephrased version of your proposal:
To address the issue of determining the required number of FIFOs for different types of transfers, we propose introducing dynamic FIFO calculation for all type of EP transfers based on the maximum packet size, and remove hard code value for required fifos in driver,
The current fifo calculation already takes on the max packet size into account.
For SuperSpeed and above, we can guess how much fifo is needed base on the maxburst and mult settings. However, for bulk endpoint in highspeed, it needs a bit more checking.
Additionally, we suggest introducing DT properties(tx-fifo-max-num-iso, tx-fifo-max-bulk and tx-fifo-max-intr) for all types of transfers
This constraint should be decided from the function driver. We should try to keep this more generic since your gadget may be used as mass storage device instead of UVC where bulk performance is needed more.
(except control EP) to allow users to control the required FIFOs instead of relying solely on the tx-fifo-max-num. This approach will provide more flexibility and customization options for users based on their specific use cases.
Please let me know if you have any comments on the above approach.
How about this: Implement gadget->ops->match_ep() for dwc3 and update the note in usb_ep_autoconfig() API.
If the function driver looks for an endpoint by passing in the descriptor with wMaxPacketSize set to 0, mark the endpoint to used for performance. This is closely related to the usb_ep_autoconfig() behavior where it returns the endpoint's maxpacket_limit if wMaxPacketSize is not provided. We just need to expand this behavior to look for performance endpoint.
If the function driver provides the wMaxPacketSize during usb_ep_autoconfig(), then use the minimum required fifo.
What do you think? Will this work for you?
BR, Thinh