[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <739CB48052CD4F44BB4283AAE6D7CEF2FD223915@am04wembxb.internal.synopsys.com>
Date: Thu, 4 Feb 2016 13:07:55 +0000
From: Vahram Aharonyan <Vahram.Aharonyan@...opsys.com>
To: Robert Baldyga <r.baldyga@...sung.com>,
"balbi@...nel.org" <balbi@...nel.org>,
"John.Youn@...opsys.com" <John.Youn@...opsys.com>
CC: "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
"linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"m.szyprowski@...sung.com" <m.szyprowski@...sung.com>,
"b.zolnierkie@...sung.com" <b.zolnierkie@...sung.com>
Subject: Re: [PATCH v2 2/5] usb: dwc2: gadget: fix TX FIFO size and address
initialization
On 2/3/2016 3:38 PM, Robert Baldyga wrote:
> According to DWC2 documentation, DPTxFSize field of DPTXFSIZn register
> is read only, which means that software cannot change FIFO size.
>
> Register description says:
> "The value of this register is the Largest Device Mode Periodic Tx Data
> FIFO Depth (parameter OTG_TX_DPERIO_DFIFO_DEPTH_n), as specified during
> coreConsultant configuration."
>
> That means, that we have to setup only FIFO start addresses (DPTxFStAddr),
> taking into account reset values of DPTxFSize.
>
> Initialize FIFO start addresses properly and remove unneeded core related
> to incorrect FIFO size initialization.
>
> Signed-off-by: Robert Baldyga <r.baldyga@...sung.com>
> ---
> drivers/usb/dwc2/core.h | 7 -------
> drivers/usb/dwc2/gadget.c | 47 ++++++++---------------------------------------
> 2 files changed, 8 insertions(+), 46 deletions(-)
>
> diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
> index 7fb6434..441da5c 100644
> --- a/drivers/usb/dwc2/core.h
> +++ b/drivers/usb/dwc2/core.h
> @@ -208,13 +208,6 @@ enum dwc2_lx_state {
> DWC2_L3, /* Off state */
> };
>
> -/*
> - * Gadget periodic tx fifo sizes as used by legacy driver
> - * EP0 is not included
> - */
> -#define DWC2_G_P_LEGACY_TX_FIFO_SIZE {256, 256, 256, 256, 768, 768, 768, \
> - 768, 0, 0, 0, 0, 0, 0, 0}
> -
> /* Gadget ep0 states */
> enum dwc2_ep0_state {
> DWC2_EP0_SETUP,
> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
> index 0d0f6fe..5de9236 100644
> --- a/drivers/usb/dwc2/gadget.c
> +++ b/drivers/usb/dwc2/gadget.c
> @@ -170,6 +170,7 @@ static void dwc2_hsotg_init_fifo(struct dwc2_hsotg *hsotg)
> unsigned int ep;
> unsigned int addr;
> int timeout;
> + u32 dptxfsizn;
> u32 val;
>
> /* Reset fifo map if not correctly cleared during previous session */
> @@ -198,13 +199,13 @@ static void dwc2_hsotg_init_fifo(struct dwc2_hsotg *hsotg)
> * given endpoint.
> */
> for (ep = 1; ep < MAX_EPS_CHANNELS; ep++) {
> - if (!hsotg->g_tx_fifo_sz[ep])
> - continue;
> - val = addr;
> - val |= hsotg->g_tx_fifo_sz[ep] << FIFOSIZE_DEPTH_SHIFT;
> - WARN_ONCE(addr + hsotg->g_tx_fifo_sz[ep] > hsotg->fifo_mem,
> - "insufficient fifo memory");
> - addr += hsotg->g_tx_fifo_sz[ep];
> + dptxfsizn = dwc2_readl(hsotg->regs + DPTXFSIZN(ep));
> +
> + val = (dptxfsizn & FIFOSIZE_DEPTH_MASK) | addr;
> + addr += dptxfsizn >> FIFOSIZE_DEPTH_SHIFT;
> +
> + if (addr > hsotg->fifo_mem)
> + break;
>
> dwc2_writel(val, hsotg->regs + DPTXFSIZN(ep));
> }
> @@ -3453,36 +3454,10 @@ static void dwc2_hsotg_dump(struct dwc2_hsotg *hsotg)
> static void dwc2_hsotg_of_probe(struct dwc2_hsotg *hsotg)
> {
> struct device_node *np = hsotg->dev->of_node;
> - u32 len = 0;
> - u32 i = 0;
>
> /* Enable dma if requested in device tree */
> hsotg->g_using_dma = of_property_read_bool(np, "g-use-dma");
>
> - /*
> - * Register TX periodic fifo size per endpoint.
> - * EP0 is excluded since it has no fifo configuration.
> - */
> - if (!of_find_property(np, "g-tx-fifo-size", &len))
> - goto rx_fifo;
> -
> - len /= sizeof(u32);
> -
> - /* Read tx fifo sizes other than ep0 */
> - if (of_property_read_u32_array(np, "g-tx-fifo-size",
> - &hsotg->g_tx_fifo_sz[1], len))
> - goto rx_fifo;
> -
> - /* Add ep0 */
> - len++;
> -
> - /* Make remaining TX fifos unavailable */
> - if (len < MAX_EPS_CHANNELS) {
> - for (i = len; i < MAX_EPS_CHANNELS; i++)
> - hsotg->g_tx_fifo_sz[i] = 0;
> - }
> -
> -rx_fifo:
> /* Register RX fifo size */
> of_property_read_u32(np, "g-rx-fifo-size", &hsotg->g_rx_fifo_sz);
>
> @@ -3504,13 +3479,10 @@ int dwc2_gadget_init(struct dwc2_hsotg *hsotg, int irq)
> struct device *dev = hsotg->dev;
> int epnum;
> int ret;
> - int i;
> - u32 p_tx_fifo[] = DWC2_G_P_LEGACY_TX_FIFO_SIZE;
>
> /* Initialize to legacy fifo configuration values */
> hsotg->g_rx_fifo_sz = 2048;
> hsotg->g_np_g_tx_fifo_sz = 1024;
> - memcpy(&hsotg->g_tx_fifo_sz[1], p_tx_fifo, sizeof(p_tx_fifo));
> /* Device tree specific probe */
> dwc2_hsotg_of_probe(hsotg);
>
> @@ -3528,9 +3500,6 @@ int dwc2_gadget_init(struct dwc2_hsotg *hsotg, int irq)
> dev_dbg(dev, "NonPeriodic TXFIFO size: %d\n",
> hsotg->g_np_g_tx_fifo_sz);
> dev_dbg(dev, "RXFIFO size: %d\n", hsotg->g_rx_fifo_sz);
> - for (i = 0; i < MAX_EPS_CHANNELS; i++)
> - dev_dbg(dev, "Periodic TXFIFO%2d size: %d\n", i,
> - hsotg->g_tx_fifo_sz[i]);
>
> hsotg->gadget.max_speed = USB_SPEED_HIGH;
> hsotg->gadget.ops = &dwc2_hsotg_gadget_ops;
>
Hi Robert,
Memory access type of this HW register's field depends on the core configuration. It's read-only in case of shared FIFO mode of the controller. In dedicated FIFO mode with dynamic FIFO sizing enabled this bit-field is writable and SW is allowed to set user-defined values in the HW during core initialization.
In Shared FIFO operation HW register with offset 104h + (FIFO_num -1) * 04h is named DPTXFSIZn with read-only upper bits. For dedicated FIFO mode SW should base on DIEPTXFn register description.
Thanks,
Vahram.
Powered by blists - more mailing lists