Hi,
On Wed, Mar 05, 2014 at 12:08:08AM -0600, dinguyen@xxxxxxxxxx wrote:
> From: Dinh Nguyen <dinguyen@xxxxxxxxxx>
>
> 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 <dinguyen@xxxxxxxxxx>
> Acked-by: Paul Zimmerman <paulz@xxxxxxxxxxxx>
> ---
> 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 <balbi@xxxxxx>
cheers
--
balbi
Attachment:
signature.asc
Description: Digital signature
