Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752737AbdLFGJT convert rfc822-to-8bit (ORCPT ); Wed, 6 Dec 2017 01:09:19 -0500 Received: from smtprelay4.synopsys.com ([198.182.47.9]:43120 "EHLO smtprelay.synopsys.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752425AbdLFGJQ (ORCPT ); Wed, 6 Dec 2017 01:09:16 -0500 From: John Youn To: Minas Harutyunyan , Felipe Balbi , Greg Kroah-Hartman , "linux-usb@vger.kernel.org" , "linux-kernel@vger.kernel.org" CC: Gevorg Sahakyan Subject: Re: [PATCH] usb: dwc2: Fix TxFIFOn sizes and total TxFIFO size issues Thread-Topic: [PATCH] usb: dwc2: Fix TxFIFOn sizes and total TxFIFO size issues Thread-Index: AQHTabOPB9b/CvHH/k2Kg3K/HpXCmw== Date: Wed, 6 Dec 2017 06:08:33 +0000 Message-ID: <2B3535C5ECE8B5419E3ECBE300772909026A0246E2@US01WEMBX2.internal.synopsys.com> References: <3d1993d2-0465-44eb-811a-bfc7d4249f44@US01WEHTC2.internal.synopsys.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.10.186.99] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 8BIT MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 7853 Lines: 226 On 11/30/2017 12:16 AM, Minas Harutyunyan wrote: > In host mode reading from DPTXSIZn returning invalid value in > dwc2_check_param_tx_fifo_sizes function. > > In total TxFIFO size calculations unnecessarily reducing by ep_info. > hw->total_fifo_size can be fully allocated for FIFO's. > > Added num_dev_in_eps member in dwc2_hw_params structure to save number > of IN EPs. > > Added g_tx_fifo_size array in dwc2_hw_params structure to store power > on reset values of DPTXSIZn registers in forced device mode. > > Updated dwc2_hsotg_tx_fifo_count() function to get TxFIFO count from > num_dev_in_eps. > > Updated dwc2_get_dev_hwparams() function to store DPTXFSIZn in > g_tx_fifo_size array. > > dwc2_get_host/dev_hwparams() functions call moved after num_dev_in_eps > set from hwcfg4. > > Modified dwc2_check_param_tx_fifo_sizes() function to check TxFIFOn > sizes based on g_tx_fifo_size array. > > Removed ep_info subtraction during calculation of tx_addr_max in > dwc2_hsotg_tx_fifo_total_depth() function. Also removed > dwc2_hsotg_ep_info_size() function as no more need. > > Signed-off-by: Gevorg Sahakyan > Signed-off-by: Minas Harutyunyan > --- > drivers/usb/dwc2/core.h | 4 ++++ > drivers/usb/dwc2/gadget.c | 42 ++---------------------------------------- > drivers/usb/dwc2/params.c | 29 +++++++++++++++++++---------- > 3 files changed, 25 insertions(+), 50 deletions(-) > > diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h > index 8367d4f985c1..686e9b5527dd 100644 > --- a/drivers/usb/dwc2/core.h > +++ b/drivers/usb/dwc2/core.h > @@ -532,6 +532,7 @@ struct dwc2_core_params { > * 2 - Internal DMA > * @power_optimized Are power optimizations enabled? > * @num_dev_ep Number of device endpoints available > + * @num_dev_in_eps Number of device IN endpoints available > * @num_dev_perio_in_ep Number of device periodic IN endpoints > * available > * @dev_token_q_depth Device Mode IN Token Sequence Learning Queue > @@ -560,6 +561,7 @@ struct dwc2_core_params { > * 2 - 8 or 16 bits > * @snpsid: Value from SNPSID register > * @dev_ep_dirs: Direction of device endpoints (GHWCFG1) > + * @g_tx_fifo_size[] Power-on values of TxFIFO sizes > */ > struct dwc2_hw_params { > unsigned op_mode:3; > @@ -581,12 +583,14 @@ struct dwc2_hw_params { > unsigned fs_phy_type:2; > unsigned i2c_enable:1; > unsigned num_dev_ep:4; > + unsigned num_dev_in_eps : 4; > unsigned num_dev_perio_in_ep:4; > unsigned total_fifo_size:16; > unsigned power_optimized:1; > unsigned utmi_phy_data_width:2; > u32 snpsid; > u32 dev_ep_dirs; > + u32 g_tx_fifo_size[MAX_EPS_CHANNELS]; > }; > > /* Size of control and EP0 buffers */ > diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c > index 0d8e09ccb59c..55950baae437 100644 > --- a/drivers/usb/dwc2/gadget.c > +++ b/drivers/usb/dwc2/gadget.c > @@ -198,55 +198,18 @@ int dwc2_hsotg_tx_fifo_count(struct dwc2_hsotg *hsotg) > { > if (hsotg->hw_params.en_multiple_tx_fifo) > /* In dedicated FIFO mode we need count of IN EPs */ > - return (dwc2_readl(hsotg->regs + GHWCFG4) & > - GHWCFG4_NUM_IN_EPS_MASK) >> GHWCFG4_NUM_IN_EPS_SHIFT; > + return hsotg->hw_params.num_dev_in_eps; > else > /* In shared FIFO mode we need count of Periodic IN EPs */ > return hsotg->hw_params.num_dev_perio_in_ep; > } > > -/** > - * dwc2_hsotg_ep_info_size - return Endpoint Info Control block size in DWORDs > - */ > -static int dwc2_hsotg_ep_info_size(struct dwc2_hsotg *hsotg) > -{ > - int val = 0; > - int i; > - u32 ep_dirs; > - > - /* > - * Don't need additional space for ep info control registers in > - * slave mode. > - */ > - if (!using_dma(hsotg)) { > - dev_dbg(hsotg->dev, "Buffer DMA ep info size 0\n"); > - return 0; > - } > - > - /* > - * Buffer DMA mode - 1 location per endpoit > - * Descriptor DMA mode - 4 locations per endpoint > - */ > - ep_dirs = hsotg->hw_params.dev_ep_dirs; > - > - for (i = 0; i <= hsotg->hw_params.num_dev_ep; i++) { > - val += ep_dirs & 3 ? 1 : 2; > - ep_dirs >>= 2; > - } > - > - if (using_desc_dma(hsotg)) > - val = val * 4; > - > - return val; > -} > - > /** > * dwc2_hsotg_tx_fifo_total_depth - return total FIFO depth available for > * device mode TX FIFOs > */ > int dwc2_hsotg_tx_fifo_total_depth(struct dwc2_hsotg *hsotg) > { > - int ep_info_size; > int addr; > int tx_addr_max; > u32 np_tx_fifo_size; > @@ -255,8 +218,7 @@ int dwc2_hsotg_tx_fifo_total_depth(struct dwc2_hsotg *hsotg) > hsotg->params.g_np_tx_fifo_size); > > /* Get Endpoint Info Control block size in DWORDs. */ > - ep_info_size = dwc2_hsotg_ep_info_size(hsotg); > - tx_addr_max = hsotg->hw_params.total_fifo_size - ep_info_size; > + tx_addr_max = hsotg->hw_params.total_fifo_size; > > addr = hsotg->params.g_rx_fifo_size + np_tx_fifo_size; > if (tx_addr_max <= addr) > diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c > index a3ffe97170ff..ad2ebff74924 100644 > --- a/drivers/usb/dwc2/params.c > +++ b/drivers/usb/dwc2/params.c > @@ -469,8 +469,7 @@ static void dwc2_check_param_tx_fifo_sizes(struct dwc2_hsotg *hsotg) > } > > for (fifo = 1; fifo <= fifo_count; fifo++) { > - dptxfszn = (dwc2_readl(hsotg->regs + DPTXFSIZN(fifo)) & > - FIFOSIZE_DEPTH_MASK) >> FIFOSIZE_DEPTH_SHIFT; > + dptxfszn = hsotg->hw_params.g_tx_fifo_size[fifo]; > > if (hsotg->params.g_tx_fifo_size[fifo] < min || > hsotg->params.g_tx_fifo_size[fifo] > dptxfszn) { > @@ -594,6 +593,7 @@ static void dwc2_get_dev_hwparams(struct dwc2_hsotg *hsotg) > struct dwc2_hw_params *hw = &hsotg->hw_params; > bool forced; > u32 gnptxfsiz; > + int fifo, fifo_count; > > if (hsotg->dr_mode == USB_DR_MODE_HOST) > return; > @@ -602,6 +602,14 @@ static void dwc2_get_dev_hwparams(struct dwc2_hsotg *hsotg) > > gnptxfsiz = dwc2_readl(hsotg->regs + GNPTXFSIZ); > > + fifo_count = dwc2_hsotg_tx_fifo_count(hsotg); > + > + for (fifo = 1; fifo <= fifo_count; fifo++) { > + hw->g_tx_fifo_size[fifo] = > + (dwc2_readl(hsotg->regs + DPTXFSIZN(fifo)) & > + FIFOSIZE_DEPTH_MASK) >> FIFOSIZE_DEPTH_SHIFT; > + } > + > if (forced) > dwc2_clear_force_mode(hsotg); > > @@ -646,14 +654,6 @@ int dwc2_get_hwparams(struct dwc2_hsotg *hsotg) > hwcfg4 = dwc2_readl(hsotg->regs + GHWCFG4); > grxfsiz = dwc2_readl(hsotg->regs + GRXFSIZ); > > - /* > - * Host specific hardware parameters. Reading these parameters > - * requires the controller to be in host mode. The mode will > - * be forced, if necessary, to read these values. > - */ > - dwc2_get_host_hwparams(hsotg); > - dwc2_get_dev_hwparams(hsotg); > - > /* hwcfg1 */ > hw->dev_ep_dirs = hwcfg1; > > @@ -696,6 +696,8 @@ int dwc2_get_hwparams(struct dwc2_hsotg *hsotg) > hw->en_multiple_tx_fifo = !!(hwcfg4 & GHWCFG4_DED_FIFO_EN); > hw->num_dev_perio_in_ep = (hwcfg4 & GHWCFG4_NUM_DEV_PERIO_IN_EP_MASK) >> > GHWCFG4_NUM_DEV_PERIO_IN_EP_SHIFT; > + hw->num_dev_in_eps = (hwcfg4 & GHWCFG4_NUM_IN_EPS_MASK) >> > + GHWCFG4_NUM_IN_EPS_SHIFT; > hw->dma_desc_enable = !!(hwcfg4 & GHWCFG4_DESC_DMA); > hw->power_optimized = !!(hwcfg4 & GHWCFG4_POWER_OPTIMIZ); > hw->utmi_phy_data_width = (hwcfg4 & GHWCFG4_UTMI_PHY_DATA_WIDTH_MASK) >> > @@ -704,6 +706,13 @@ int dwc2_get_hwparams(struct dwc2_hsotg *hsotg) > /* fifo sizes */ > hw->rx_fifo_size = (grxfsiz & GRXFSIZ_DEPTH_MASK) >> > GRXFSIZ_DEPTH_SHIFT; > + /* > + * Host specific hardware parameters. Reading these parameters > + * requires the controller to be in host mode. The mode will > + * be forced, if necessary, to read these values. > + */ > + dwc2_get_host_hwparams(hsotg); > + dwc2_get_dev_hwparams(hsotg); > > return 0; > } > Acked-by: John Youn John