Hi, On Wed, Mar 05, 2014 at 12:08:08AM -0600, [email protected] wrote: > From: Dinh Nguyen <[email protected]> > > The dwc2 IP on the SOCFPGA cannot use the default HW configured > FIFO sizes. The total FIFO depth as read from GHWCFG3 reports 0x1f80 or 8064 > 32-bit words. But the GRXFSIZ, GNPTXFSIZ, and HPTXFSIZ register defaults > to 0x2000 or 8192 32-bit words. So the driver cannot just use the fifo sizes > as read from those registers. > > For platforms that face the same issue, this commits sets the RX, periodic TX, > and non-periodic TX fifo size to those that are recommended v2.93a spec for > the DWC2 IP. Implements Method #2 from the Synopsys v2.93a spec for the DWC2. > > Signed-off-by: Dinh Nguyen <[email protected]> > Acked-by: Paul Zimmerman <[email protected]> > --- > v3: > - Address comments from Felipe > - Used Method 2 as recommended by Synopsys to high-bandwidth endpoints. > > v2: > - Fix coding style with braces around both if() branches > --- > drivers/usb/dwc2/core.c | 53 > +++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 53 insertions(+) > > diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c > index 1d12988..8d8c100 100644 > --- a/drivers/usb/dwc2/core.c > +++ b/drivers/usb/dwc2/core.c > @@ -507,6 +507,56 @@ void dwc2_disable_host_interrupts(struct dwc2_hsotg > *hsotg) > writel(intmsk, hsotg->regs + GINTMSK); > } > > +/* > + * dwc2_calculate_dynamic_fifo() - Calculates the default fifo size > + * For system that have a total fifo depth that is smaller than the default > + * RX + TX fifo size. > + * > + * @hsotg: Programming view of DWC_otg controller > + */ > +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; > + > + /* Will use Method 2 defined in the DWC2 spec: minimum FIFO depth
according to CodingStyle, this should be:
/*
* Will use Method 2 defined in the DWC2 spec: minimum FIFO depth
> + * allocation with support for high bandwidth endpoints. Synopsys
> + * defines MPS(Max Packet size) for a periodic EP=1024, and for
> + * non-periodic as 512.
> + */
> + if (total_fifo_size < (rxfsiz + nptxfsiz + ptxfsiz)) {
> + /* For Buffer DMA mode/Scatter Gather DMA mode
> + * 2 * ((Largest Packet size / 4) + 1 + 1) + n
> + * with n = number of host channel.
> + * 2 * ((1024/4) + 2) = 516
> + */
comment style it wrong.
> + rxfsiz = 516 + hw->host_channels;
add a blank line here
> + /* min non-periodic tx fifo depth
> + * 2 * (largest non-periodic USB packet used / 4)
> + * 2 * (512/4) = 256
> + */
comment style
> + nptxfsiz = 256;
blank line.
> + /* min periodic tx fifo depth
> + * (largest packet size*MC)/4
> + * (1024 * 3)/4 = 768
> + */
> + ptxfsiz = 768;
> +
> + params->host_rx_fifo_size = rxfsiz;
> + params->host_nperio_tx_fifo_size = nptxfsiz;
> + params->host_perio_tx_fifo_size = ptxfsiz;
> + }
> +
> + if (total_fifo_size < (rxfsiz + nptxfsiz + ptxfsiz))
this is what I don't get, why do you have this branch again ? Is it just
to see if you *still* have fifo issues ? It looks like you could add a
comment here and make this unlikely:
/*
* If the summation of RX, NPTX and PTX fifo sizes is still
* bigger than the total_fifo_size, then we have a problem.
*
* We won't be able to allocate as many endpoints. Right now,
* we're just printing an error message, but ideally this FIFO
* allocation algorithm would be improved in the future.
*
* FIXME improve this FIFO allocation algorithm.
*/
if (unlikely(total_fifo_size < (rxfsiz + nptxfsiz + ptxfsiz))
that way, it's clearly documented what you're trying to achieve and
someone looking at the code years from now, won't get confused.
> + dev_err(hsotg->dev, "invalid fifo sizes\n");
> +}
> +
> static void dwc2_config_fifos(struct dwc2_hsotg *hsotg)
> {
> struct dwc2_core_params *params = hsotg->core_params;
> @@ -515,6 +565,9 @@ static void dwc2_config_fifos(struct dwc2_hsotg *hsotg)
> if (!params->enable_dynamic_fifo)
> return;
>
> + /* Calculate the correct FIFO sizes */
this comment is a little bit pointless as it says the same thing as the
function name itself ;-)
after fixing those:
Reviewed-by: Felipe Balbi <[email protected]>
cheers
--
balbi
signature.asc
Description: Digital signature
