On 03/03/2014 08:42 PM, Felipe Balbi wrote:
Hi,
On Mon, Mar 03, 2014 at 05:09:27PM -0600, dinguyen@xxxxxxxxxx wrote:
From: Dinh Nguyen <dinguyen@xxxxxxxxxx>
Remove reading the fifo sizes from dts in platform.c
Add dwc2_calculate_dynamic_fifo
Conflicts:
arch/arm/boot/dts/socfpga.dtsi
drivers/staging/dwc2/core.c
<snip>
diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c
index 6d001b5..3acb7ee 100644
--- a/drivers/usb/dwc2/core.c
+++ b/drivers/usb/dwc2/core.c
@@ -478,6 +478,38 @@ void dwc2_disable_host_interrupts(struct dwc2_hsotg *hsotg)
writel(intmsk, hsotg->regs + GINTMSK);
}
+static void dwc2_calculate_dynamic_fifo(struct dwc2_hsotg *hsotg)
+{
+ struct dwc2_core_params *params = hsotg->core_params;
+ struct dwc2_hw_params *hw = &hsotg->hw_params;
+ u32 rxfsiz, nptxfsiz, ptxfsiz, total_fifo_size;
+
+ total_fifo_size = hw->total_fifo_size;
+ rxfsiz = params->host_rx_fifo_size;
+ nptxfsiz = params->host_nperio_tx_fifo_size;
+ ptxfsiz = params->host_perio_tx_fifo_size;
+
+ if (total_fifo_size >= (rxfsiz + nptxfsiz + ptxfsiz))
+ /* Params are valid, nothing to do */
+ return;
You really don't need this branch but in any case, if one branch has
curly braces, both branches should have it too.
Will fix...
+ else {
+ /* min rx fifo size = ((largest packet/4)*2)+2 */
+ rxfsiz = (((1024/4) + 1 + 1) * 3) + 1;
+ /* min non-periodic tx fifo depth */
+ nptxfsiz = 3 * (1024/4);
+ /* min periodic tx fifo depth */
+ ptxfsiz = ((1024*3)/4) * 3;
+ }
+
+ if (total_fifo_size < (rxfsiz + nptxfsiz + ptxfsiz))
hmm, so in the if condition above you do:
if (total_fifo_size >= (rxfsiz + nptxfsiz + ptxfsiz))
then you have an else branch following it and the only way for that else
branch to be reached is if this other condition is true. I think this
code could be rewritten as:
if (total_fifo_size < (rxfsiz + nptxfsiz + ptxfsiz)) {
/* min rx fifo size = ((largest packet / 4) * 2) + 2 */
rxfsiz = (((1024 / 4) + 1 + 1) * 3) + 1;
/* min non-periodic tx fifo depth */
nptxfsiz = 3 * (1024 / 4);
/* min periodic tx fifo depth */
ptxfsiz = ((1024 * 3) / 4) * 3;
}
Ok.. will re-write...
Also, you could be a bit more verbose on your comments above. One thing
I see is that your fifo allocation, apparently, won't work for
high-bandwidth ISOC endpoints. Is that correct ? Another thing is that
pre-allocating FIFO sizes might not be the best choice. I mean, if you
only have two bulk endpoints enabled, you would be better off allocating
bigger FIFOs for those endpoints in order to allow for double- or
triple-buffering as that would give SW more time to program DMA to read
out the data.
yes, I am just coding this to the minimum fifo settings. Let me see if
the function can
be smarter about the fifo allocation.
Thanks,
Dinh
--
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
