Hi folks, while going through the dwc2 driver, I've found that there is a lot of places that need info from the hwcfg registers. Currently, each of these places do their own bitshifting and masking, but it occurs to me that the code would become cleaner if all this happened in single place, so all the other spots can just refer to a proper variable instead. I've made a start at implementing this, but before I finish up this patch, I'd like to know what other people think. Is this a good idea, or would you prefer to keep it like this? The advantage of this patch would be that the code is more clear. The disadvantage is a couple of dozen bytes of extra memory used (though this could be reduced to just a few to no bytes by using bitfields to store the values instead, if preferred). Another disadvantage could be that it would be a pretty big, but simple, patch to review. I've put my WIP patch below, let me know what you think. Gr. Matthijs commit 5f67447a06581bf9d9d5f154e635a45a1406994c Author: Matthijs Kooijman <matthijs@xxxxxxxx> Date: Tue Mar 26 17:35:34 2013 +0100 staging: dwc2: Interpret all hwcfg and related register at init time Before, the hwcfg4 registers were read at device init time, but interpreted at various parts in the code. This commit unpacks the hwcfg register values into a struct with properly labeled variables, which makes all the other code using these values more consise and easier to read. This commit mostly moves code, but also attempts to simplify some expressions, from (val >> shift) & (mask >> shift) to (val & mask) >> shift. Note: This patch is unfinished, it does not convert all hwcfg accesses yet. diff --git a/drivers/staging/dwc2/core.c b/drivers/staging/dwc2/core.c index 71a6547..3a9a9ee 100644 --- a/drivers/staging/dwc2/core.c +++ b/drivers/staging/dwc2/core.c @@ -90,12 +90,10 @@ static void dwc2_enable_common_interrupts(struct dwc2_hsotg *hsotg) */ static void dwc2_init_fs_ls_pclk_sel(struct dwc2_hsotg *hsotg) { - u32 hs_phy_type = hsotg->hwcfg2 & GHWCFG2_HS_PHY_TYPE_MASK; - u32 fs_phy_type = hsotg->hwcfg2 & GHWCFG2_FS_PHY_TYPE_MASK; u32 hcfg, val; - if ((hs_phy_type == GHWCFG2_HS_PHY_TYPE_ULPI && - fs_phy_type == GHWCFG2_FS_PHY_TYPE_DEDICATED && + if ((hsotg->hw_params.hs_phy_type == GHWCFG2_HS_PHY_TYPE_ULPI && + hsotg->hw_params.fs_phy_type == GHWCFG2_FS_PHY_TYPE_DEDICATED && hsotg->core_params->ulpi_fs_ls > 0) || hsotg->core_params->phy_type == DWC2_PHY_TYPE_PARAM_FS) { /* Full speed PHY */ @@ -245,7 +243,7 @@ static void dwc2_hs_phy_init(struct dwc2_hsotg *hsotg, bool select_phy) static void dwc2_phy_init(struct dwc2_hsotg *hsotg, bool select_phy) { - u32 usbcfg, hs_phy_type, fs_phy_type; + u32 usbcfg; if (hsotg->core_params->speed == DWC2_SPEED_PARAM_FULL && hsotg->core_params->phy_type == DWC2_PHY_TYPE_PARAM_FS) { @@ -256,11 +254,8 @@ static void dwc2_phy_init(struct dwc2_hsotg *hsotg, bool select_phy) dwc2_hs_phy_init(hsotg, select_phy); } - hs_phy_type = hsotg->hwcfg2 & GHWCFG2_HS_PHY_TYPE_MASK; - fs_phy_type = hsotg->hwcfg2 & GHWCFG2_FS_PHY_TYPE_MASK; - - if (hs_phy_type == GHWCFG2_HS_PHY_TYPE_ULPI && - fs_phy_type == GHWCFG2_FS_PHY_TYPE_DEDICATED && + if (hsotg->hw_params.hs_phy_type == GHWCFG2_HS_PHY_TYPE_ULPI && + hsotg->hw_params.fs_phy_type == GHWCFG2_FS_PHY_TYPE_DEDICATED && hsotg->core_params->ulpi_fs_ls > 0) { dev_dbg(hsotg->dev, "Setting ULPI FSLS\n"); usbcfg = readl(hsotg->regs + GUSBCFG); @@ -279,7 +274,7 @@ static int dwc2_gahbcfg_init(struct dwc2_hsotg *hsotg) { u32 ahbcfg = 0; - switch (hsotg->hwcfg2 & GHWCFG2_ARCHITECTURE_MASK) { + switch (hsotg->hw_params.arch) { case GHWCFG2_EXT_DMA_ARCH: dev_err(hsotg->dev, "External DMA Mode not supported\n"); return -EINVAL; @@ -331,7 +326,7 @@ static void dwc2_gusbcfg_init(struct dwc2_hsotg *hsotg) usbcfg = readl(hsotg->regs + GUSBCFG); usbcfg &= ~(GUSBCFG_HNPCAP | GUSBCFG_SRPCAP); - switch (hsotg->hwcfg2 & GHWCFG2_OP_MODE_MASK) { + switch (hsotg->hw_params.op_mode) { case GHWCFG2_OP_MODE_HNP_SRP_CAPABLE: if (hsotg->core_params->otg_cap == DWC2_CAP_PARAM_HNP_SRP_CAPABLE) @@ -392,12 +387,9 @@ int dwc2_core_init(struct dwc2_hsotg *hsotg, bool select_phy) dwc2_core_reset(hsotg); dev_dbg(hsotg->dev, "num_dev_perio_in_ep=%d\n", - hsotg->hwcfg4 >> GHWCFG4_NUM_DEV_PERIO_IN_EP_SHIFT & - GHWCFG4_NUM_DEV_PERIO_IN_EP_MASK >> - GHWCFG4_NUM_DEV_PERIO_IN_EP_SHIFT); + hsotg->hw_params.num_dev_perio_in_ep); - hsotg->total_fifo_size = hsotg->hwcfg3 >> GHWCFG3_DFIFO_DEPTH_SHIFT & - GHWCFG3_DFIFO_DEPTH_MASK >> GHWCFG3_DFIFO_DEPTH_SHIFT; + hsotg->total_fifo_size = hsotg->hw_params.dfifo_depth; hsotg->rx_fifo_size = readl(hsotg->regs + GRXFSIZ); hsotg->nperio_tx_fifo_size = readl(hsotg->regs + GNPTXFSIZ) >> 16 & 0xffff; @@ -591,13 +583,12 @@ void dwc2_core_host_init(struct dwc2_hsotg *hsotg) } if (hsotg->core_params->dma_desc_enable > 0) { - u32 op_mode = hsotg->hwcfg2 & GHWCFG2_OP_MODE_MASK; - + /*TODO: Check this in dwc2_set_param_dma_desc_enable ? */ if (hsotg->snpsid < DWC2_CORE_REV_2_90a || !(hsotg->hwcfg4 & GHWCFG4_DESC_DMA) || - op_mode == GHWCFG2_OP_MODE_SRP_CAPABLE_DEVICE || - op_mode == GHWCFG2_OP_MODE_NO_SRP_CAPABLE_DEVICE || - op_mode == GHWCFG2_OP_MODE_UNDEFINED) { + hsotg->hw_params.op_mode == GHWCFG2_OP_MODE_SRP_CAPABLE_DEVICE || + hsotg->hw_params.op_mode == GHWCFG2_OP_MODE_NO_SRP_CAPABLE_DEVICE || + hsotg->hw_params.op_mode == GHWCFG2_OP_MODE_UNDEFINED) { dev_err(hsotg->dev, "Hardware does not support descriptor DMA mode -\n"); dev_err(hsotg->dev, @@ -1635,18 +1626,16 @@ void dwc2_hc_do_ping(struct dwc2_hsotg *hsotg, struct dwc2_host_chan *chan) u32 dwc2_calc_frame_interval(struct dwc2_hsotg *hsotg) { u32 usbcfg; - u32 hwcfg2; u32 hprt0; int clock = 60; /* default value */ usbcfg = readl(hsotg->regs + GUSBCFG); - hwcfg2 = readl(hsotg->regs + GHWCFG2); hprt0 = readl(hsotg->regs + HPRT0); if (!(usbcfg & GUSBCFG_PHYSEL) && (usbcfg & GUSBCFG_ULPI_UTMI_SEL) && !(usbcfg & GUSBCFG_PHYIF16)) clock = 60; - if ((usbcfg & GUSBCFG_PHYSEL) && (hwcfg2 & GHWCFG2_FS_PHY_TYPE_MASK) == + if ((usbcfg & GUSBCFG_PHYSEL) && hsotg->hw_params.fs_phy_type == GHWCFG2_FS_PHY_TYPE_SHARED_ULPI) clock = 48; if (!(usbcfg & GUSBCFG_PHY_LP_CLK_SEL) && !(usbcfg & GUSBCFG_PHYSEL) && @@ -1659,10 +1648,10 @@ u32 dwc2_calc_frame_interval(struct dwc2_hsotg *hsotg) !(usbcfg & GUSBCFG_ULPI_UTMI_SEL) && (usbcfg & GUSBCFG_PHYIF16)) clock = 48; if ((usbcfg & GUSBCFG_PHYSEL) && !(usbcfg & GUSBCFG_PHYIF16) && - (hwcfg2 & GHWCFG2_FS_PHY_TYPE_MASK) == + hsotg->hw_params.fs_phy_type == GHWCFG2_FS_PHY_TYPE_SHARED_UTMI) clock = 48; - if ((usbcfg & GUSBCFG_PHYSEL) && (hwcfg2 & GHWCFG2_FS_PHY_TYPE_MASK) == + if ((usbcfg & GUSBCFG_PHYSEL) && hsotg->hw_params.fs_phy_type == GHWCFG2_FS_PHY_TYPE_DEDICATED) clock = 48; @@ -1863,13 +1852,13 @@ void dwc2_dump_global_registers(struct dwc2_hsotg *hsotg) (unsigned long)addr, readl(addr)); if (hsotg->core_params->en_multiple_tx_fifo <= 0) { - ep_num = hsotg->hwcfg4 >> GHWCFG4_NUM_DEV_PERIO_IN_EP_SHIFT & - GHWCFG4_NUM_DEV_PERIO_IN_EP_MASK >> - GHWCFG4_NUM_DEV_PERIO_IN_EP_SHIFT; + ep_num = hsotg->hw_params.num_dev_perio_in_ep; txfsiz = "DPTXFSIZ"; } else { - ep_num = hsotg->hwcfg4 >> GHWCFG4_NUM_IN_EPS_SHIFT & - GHWCFG4_NUM_IN_EPS_MASK >> GHWCFG4_NUM_IN_EPS_SHIFT; + // SHould this be 1 +? Why isn't the DIENPTXF register + // dumped below but always DPTXFSIZ? + ep_num = 1 + (hsotg->hwcfg4 >> GHWCFG4_NUM_IN_EPS_SHIFT & + GHWCFG4_NUM_IN_EPS_MASK >> GHWCFG4_NUM_IN_EPS_SHIFT); txfsiz = "DIENPTXF"; } @@ -1954,17 +1943,14 @@ int dwc2_set_param_otg_cap(struct dwc2_hsotg *hsotg, int val) { int valid = 1; int retval = 0; - u32 op_mode; - - op_mode = hsotg->hwcfg2 & GHWCFG2_OP_MODE_MASK; switch (val) { case DWC2_CAP_PARAM_HNP_SRP_CAPABLE: - if (op_mode != GHWCFG2_OP_MODE_HNP_SRP_CAPABLE) + if (hsotg->hw_params.op_mode != GHWCFG2_OP_MODE_HNP_SRP_CAPABLE) valid = 0; break; case DWC2_CAP_PARAM_SRP_ONLY_CAPABLE: - switch (op_mode) { + switch (hsotg->hw_params.op_mode) { case GHWCFG2_OP_MODE_HNP_SRP_CAPABLE: case GHWCFG2_OP_MODE_SRP_ONLY_CAPABLE: case GHWCFG2_OP_MODE_SRP_CAPABLE_DEVICE: @@ -1988,7 +1974,7 @@ int dwc2_set_param_otg_cap(struct dwc2_hsotg *hsotg, int val) dev_err(hsotg->dev, "%d invalid for otg_cap parameter. Check HW configuration.\n", val); - switch (op_mode) { + switch (hsotg->hw_params.op_mode) { case GHWCFG2_OP_MODE_HNP_SRP_CAPABLE: val = DWC2_CAP_PARAM_HNP_SRP_CAPABLE; break; @@ -2014,8 +2000,7 @@ int dwc2_set_param_dma_enable(struct dwc2_hsotg *hsotg, int val) int valid = 1; int retval = 0; - if (val > 0 && (hsotg->hwcfg2 & GHWCFG2_ARCHITECTURE_MASK) == - GHWCFG2_SLAVE_ONLY_ARCH) + if (val > 0 && hsotg->hw_params.arch == GHWCFG2_SLAVE_ONLY_ARCH) valid = 0; if (val < 0) valid = 0; @@ -2025,8 +2010,7 @@ int dwc2_set_param_dma_enable(struct dwc2_hsotg *hsotg, int val) dev_err(hsotg->dev, "%d invalid for dma_enable parameter. Check HW configuration.\n", val); - val = (hsotg->hwcfg2 & GHWCFG2_ARCHITECTURE_MASK) != - GHWCFG2_SLAVE_ONLY_ARCH; + val = hsotg->hw_params.arch != GHWCFG2_SLAVE_ONLY_ARCH; dev_dbg(hsotg->dev, "Setting dma_enable to %d\n", val); retval = -EINVAL; } @@ -2041,7 +2025,7 @@ int dwc2_set_param_dma_desc_enable(struct dwc2_hsotg *hsotg, int val) int retval = 0; if (val > 0 && (hsotg->core_params->dma_enable <= 0 || - !(hsotg->hwcfg4 & GHWCFG4_DESC_DMA))) + !(hsotg->hw_params.dma_desc_enable))) valid = 0; if (val < 0) valid = 0; @@ -2052,7 +2036,7 @@ int dwc2_set_param_dma_desc_enable(struct dwc2_hsotg *hsotg, int val) "%d invalid for dma_desc_enable parameter. Check HW configuration.\n", val); val = (hsotg->core_params->dma_enable > 0 && - (hsotg->hwcfg4 & GHWCFG4_DESC_DMA)); + hsotg->hw_params.dma_desc_enable); dev_dbg(hsotg->dev, "Setting dma_desc_enable to %d\n", val); retval = -EINVAL; } @@ -2088,7 +2072,7 @@ int dwc2_set_param_enable_dynamic_fifo(struct dwc2_hsotg *hsotg, int val) int valid = 1; int retval = 0; - if (val > 0 && !(hsotg->hwcfg2 & GHWCFG2_DYNAMIC_FIFO)) + if (val > 0 && !hsotg->hw_params.enable_dynamic_fifo) valid = 0; if (val < 0) valid = 0; @@ -2098,7 +2082,7 @@ int dwc2_set_param_enable_dynamic_fifo(struct dwc2_hsotg *hsotg, int val) dev_err(hsotg->dev, "%d invalid for enable_dynamic_fifo parameter. Check HW configuration.\n", val); - val = !!(hsotg->hwcfg2 & GHWCFG2_DYNAMIC_FIFO); + val = hsotg->hw_params.enable_dynamic_fifo; dev_dbg(hsotg->dev, "Setting enable_dynamic_fifo to %d\n", val); retval = -EINVAL; } @@ -2112,7 +2096,7 @@ int dwc2_set_param_host_rx_fifo_size(struct dwc2_hsotg *hsotg, int val) int valid = 1; int retval = 0; - if (val < 16 || val > readl(hsotg->regs + GRXFSIZ)) + if (val < 16 || val > hsotg->hw_params.host_rx_fifo_size) valid = 0; if (!valid) { @@ -2120,7 +2104,7 @@ int dwc2_set_param_host_rx_fifo_size(struct dwc2_hsotg *hsotg, int val) dev_err(hsotg->dev, "%d invalid for host_rx_fifo_size. Check HW configuration.\n", val); - val = readl(hsotg->regs + GRXFSIZ); + val = hsotg->hw_params.host_rx_fifo_size; dev_dbg(hsotg->dev, "Setting host_rx_fifo_size to %d\n", val); retval = -EINVAL; } @@ -2134,7 +2118,7 @@ int dwc2_set_param_host_nperio_tx_fifo_size(struct dwc2_hsotg *hsotg, int val) int valid = 1; int retval = 0; - if (val < 16 || val > (readl(hsotg->regs + GNPTXFSIZ) >> 16 & 0xffff)) + if (val < 16 || val > hsotg->hw_params.host_nperio_tx_fifo_size) valid = 0; if (!valid) { @@ -2142,7 +2126,7 @@ int dwc2_set_param_host_nperio_tx_fifo_size(struct dwc2_hsotg *hsotg, int val) dev_err(hsotg->dev, "%d invalid for host_nperio_tx_fifo_size. Check HW configuration.\n", val); - val = readl(hsotg->regs + GNPTXFSIZ) >> 16 & 0xffff; + val = hsotg->hw_params.host_nperio_tx_fifo_size; dev_dbg(hsotg->dev, "Setting host_nperio_tx_fifo_size to %d\n", val); retval = -EINVAL; @@ -2157,7 +2141,7 @@ int dwc2_set_param_host_perio_tx_fifo_size(struct dwc2_hsotg *hsotg, int val) int valid = 1; int retval = 0; - if (val < 16 || val > (hsotg->hptxfsiz >> 16)) + if (val < 16 || val > hsotg->hw_params.host_perio_tx_fifo_size) valid = 0; if (!valid) { @@ -2165,7 +2149,7 @@ int dwc2_set_param_host_perio_tx_fifo_size(struct dwc2_hsotg *hsotg, int val) dev_err(hsotg->dev, "%d invalid for host_perio_tx_fifo_size. Check HW configuration.\n", val); - val = hsotg->hptxfsiz >> 16; + val = hsotg->hw_params.host_perio_tx_fifo_size; dev_dbg(hsotg->dev, "Setting host_perio_tx_fifo_size to %d\n", val); retval = -EINVAL; @@ -2179,11 +2163,8 @@ int dwc2_set_param_max_transfer_size(struct dwc2_hsotg *hsotg, int val) { int valid = 1; int retval = 0; - int width = hsotg->hwcfg3 >> GHWCFG3_XFER_SIZE_CNTR_WIDTH_SHIFT & - GHWCFG3_XFER_SIZE_CNTR_WIDTH_MASK >> - GHWCFG3_XFER_SIZE_CNTR_WIDTH_SHIFT; - if (val < 2047 || val >= (1 << (width + 11))) + if (val < 2047 || val > hsotg->hw_params.max_transfer_size) valid = 0; if (!valid) { @@ -2191,7 +2172,7 @@ int dwc2_set_param_max_transfer_size(struct dwc2_hsotg *hsotg, int val) dev_err(hsotg->dev, "%d invalid for max_transfer_size. Check HW configuration.\n", val); - val = (1 << (width + 11)) - 1; + val = hsotg->hw_params.max_transfer_size; dev_dbg(hsotg->dev, "Setting max_transfer_size to %d\n", val); retval = -EINVAL; } @@ -2204,11 +2185,8 @@ int dwc2_set_param_max_packet_count(struct dwc2_hsotg *hsotg, int val) { int valid = 1; int retval = 0; - int width = hsotg->hwcfg3 >> GHWCFG3_PACKET_SIZE_CNTR_WIDTH_SHIFT & - GHWCFG3_PACKET_SIZE_CNTR_WIDTH_MASK >> - GHWCFG3_PACKET_SIZE_CNTR_WIDTH_SHIFT; - if (val < 15 || val >= (1 << (width + 4))) + if (val < 15 || val > hsotg->hw_params.max_packet_count) valid = 0; if (!valid) { @@ -2216,7 +2194,7 @@ int dwc2_set_param_max_packet_count(struct dwc2_hsotg *hsotg, int val) dev_err(hsotg->dev, "%d invalid for max_packet_count. Check HW configuration.\n", val); - val = (1 << (width + 4)) - 1; + val = hsotg->hw_params.max_packet_count; dev_dbg(hsotg->dev, "Setting max_packet_count to %d\n", val); retval = -EINVAL; } @@ -2229,10 +2207,8 @@ int dwc2_set_param_host_channels(struct dwc2_hsotg *hsotg, int val) { int valid = 1; int retval = 0; - int num_chan = hsotg->hwcfg2 >> GHWCFG2_NUM_HOST_CHAN_SHIFT & - GHWCFG2_NUM_HOST_CHAN_MASK >> GHWCFG2_NUM_HOST_CHAN_SHIFT; - if (val < 1 || val > num_chan + 1) + if (val < 1 || val > hsotg->hw_params.host_channels) valid = 0; if (!valid) { @@ -2240,7 +2216,7 @@ int dwc2_set_param_host_channels(struct dwc2_hsotg *hsotg, int val) dev_err(hsotg->dev, "%d invalid for host_channels. Check HW configuration.\n", val); - val = num_chan + 1; + val = hsotg->hw_params.host_channels; dev_dbg(hsotg->dev, "Setting host_channels to %d\n", val); retval = -EINVAL; } @@ -2253,8 +2229,6 @@ int dwc2_set_param_phy_type(struct dwc2_hsotg *hsotg, int val) { #ifndef NO_FS_PHY_HW_CHECKS int valid = 0; - u32 hs_phy_type; - u32 fs_phy_type; #endif int retval = 0; @@ -2275,19 +2249,16 @@ int dwc2_set_param_phy_type(struct dwc2_hsotg *hsotg, int val) } #ifndef NO_FS_PHY_HW_CHECKS - hs_phy_type = hsotg->hwcfg2 & GHWCFG2_HS_PHY_TYPE_MASK; - fs_phy_type = hsotg->hwcfg2 & GHWCFG2_FS_PHY_TYPE_MASK; - if (val == DWC2_PHY_TYPE_PARAM_UTMI && - (hs_phy_type == GHWCFG2_HS_PHY_TYPE_UTMI || - hs_phy_type == GHWCFG2_HS_PHY_TYPE_UTMI_ULPI)) + (hsotg->hw_params.hs_phy_type == GHWCFG2_HS_PHY_TYPE_UTMI || + hsotg->hw_params.hs_phy_type == GHWCFG2_HS_PHY_TYPE_UTMI_ULPI)) valid = 1; else if (val == DWC2_PHY_TYPE_PARAM_ULPI && - (hs_phy_type == GHWCFG2_HS_PHY_TYPE_ULPI || - hs_phy_type == GHWCFG2_HS_PHY_TYPE_UTMI_ULPI)) + (hsotg->hw_params.hs_phy_type == GHWCFG2_HS_PHY_TYPE_ULPI || + hsotg->hw_params.hs_phy_type == GHWCFG2_HS_PHY_TYPE_UTMI_ULPI)) valid = 1; else if (val == DWC2_PHY_TYPE_PARAM_FS && - fs_phy_type == GHWCFG2_FS_PHY_TYPE_DEDICATED) + hsotg->hw_params.fs_phy_type == GHWCFG2_FS_PHY_TYPE_DEDICATED) valid = 1; if (!valid) { @@ -2296,9 +2267,9 @@ int dwc2_set_param_phy_type(struct dwc2_hsotg *hsotg, int val) "%d invalid for phy_type. Check HW configuration.\n", val); val = DWC2_PHY_TYPE_PARAM_FS; - if (hs_phy_type != GHWCFG2_HS_PHY_TYPE_NOT_SUPPORTED) { - if (hs_phy_type == GHWCFG2_HS_PHY_TYPE_UTMI || - hs_phy_type == GHWCFG2_HS_PHY_TYPE_UTMI_ULPI) + if (hsotg->hw_params.hs_phy_type != GHWCFG2_HS_PHY_TYPE_NOT_SUPPORTED) { + if (hsotg->hw_params.hs_phy_type == GHWCFG2_HS_PHY_TYPE_UTMI || + hsotg->hw_params.hs_phy_type == GHWCFG2_HS_PHY_TYPE_UTMI_ULPI) val = DWC2_PHY_TYPE_PARAM_UTMI; else val = DWC2_PHY_TYPE_PARAM_ULPI; @@ -2428,6 +2399,8 @@ int dwc2_set_param_phy_utmi_width(struct dwc2_hsotg *hsotg, int val) { int retval = 0; + /* TODO: Check against GHWCFG4_UMTI_PHY_DATA_WIDTH? */ + if (DWC2_PARAM_TEST(val, 8, 8) && DWC2_PARAM_TEST(val, 16, 16)) { if (val >= 0) { dev_err(hsotg->dev, "Wrong value for phy_utmi_width\n"); @@ -2501,7 +2474,7 @@ int dwc2_set_param_i2c_enable(struct dwc2_hsotg *hsotg, int val) } #ifndef NO_FS_PHY_HW_CHECKS - if (val == 1 && !(hsotg->hwcfg3 & GHWCFG3_I2C)) + if (val == 1 && !(hsotg->hw_params.i2c_enable)) valid = 0; if (!valid) { @@ -2509,7 +2482,7 @@ int dwc2_set_param_i2c_enable(struct dwc2_hsotg *hsotg, int val) dev_err(hsotg->dev, "%d invalid for i2c_enable. Check HW configuration.\n", val); - val = !!(hsotg->hwcfg3 & GHWCFG3_I2C); + val = hsotg->hw_params.i2c_enable; dev_dbg(hsotg->dev, "Setting i2c_enable to %d\n", val); retval = -EINVAL; } @@ -2534,7 +2507,7 @@ int dwc2_set_param_en_multiple_tx_fifo(struct dwc2_hsotg *hsotg, int val) valid = 0; } - if (val == 1 && !(hsotg->hwcfg4 & GHWCFG4_DED_FIFO_EN)) + if (val == 1 && !(hsotg->hw_params.en_multiple_tx_fifo)) valid = 0; if (!valid) { @@ -2542,7 +2515,7 @@ int dwc2_set_param_en_multiple_tx_fifo(struct dwc2_hsotg *hsotg, int val) dev_err(hsotg->dev, "%d invalid for parameter en_multiple_tx_fifo. Check HW configuration.\n", val); - val = !!(hsotg->hwcfg4 & GHWCFG4_DED_FIFO_EN); + val = hsotg->hw_params.en_multiple_tx_fifo; dev_dbg(hsotg->dev, "Setting en_multiple_tx_fifo to %d\n", val); retval = -EINVAL; } @@ -2633,6 +2606,39 @@ int dwc2_set_param_otg_ver(struct dwc2_hsotg *hsotg, int val) return retval; } +/** + * During device initialization, read various hardware configuration + * registers and interpret the contents. + */ +int dwc2_get_hwparams(struct dwc2_hsotg *hsotg) +{ + struct dwc2_hw_params *hw = &hsotg->hw_params; + unsigned width; + + hw->op_mode = hsotg->hwcfg2 & GHWCFG2_OP_MODE_MASK; //TODO: shift + hw->arch = hsotg->hwcfg2 & GHWCFG2_ARCHITECTURE_MASK; //TODO: shift + hw->dma_desc_enable = !!(hsotg->hwcfg4 & GHWCFG4_DESC_DMA); + hw->enable_dynamic_fifo = !!(hsotg->hwcfg2 & GHWCFG2_DYNAMIC_FIFO); + hw->host_rx_fifo_size = readl(hsotg->regs + GRXFSIZ); + hw->host_nperio_tx_fifo_size = (readl(hsotg->regs + GNPTXFSIZ) & GNPTXFSIZ_NP_TXF_DEP_MASK) << GNPTXFSIZ_NP_TXF_DEP_SHIFT; + hw->host_perio_tx_fifo_size = (hsotg->hptxfsiz & HPTXFSIZ_P_TXF_DEP_MASK) >> HPTXFSIZ_P_TXF_DEP_SHIFT; + width = (hsotg->hwcfg3 & GHWCFG3_XFER_SIZE_CNTR_WIDTH_MASK) >> GHWCFG3_XFER_SIZE_CNTR_WIDTH_SHIFT; + hw->max_transfer_size = (1 << (width + 11)) - 1; + width = (hsotg->hwcfg3 & GHWCFG3_PACKET_SIZE_CNTR_WIDTH_MASK) >> GHWCFG3_PACKET_SIZE_CNTR_WIDTH_SHIFT; + hw->max_packet_count = (1 << (width + 4)) - 1; + hw->host_channels = 1 + ((hsotg->hwcfg2 & GHWCFG2_NUM_HOST_CHAN_MASK) >> GHWCFG2_NUM_HOST_CHAN_SHIFT); + hw->hs_phy_type = hsotg->hwcfg2 & GHWCFG2_HS_PHY_TYPE_MASK; //TODO shift + hw->fs_phy_type = hsotg->hwcfg2 & GHWCFG2_FS_PHY_TYPE_MASK; //TODO shift + hw->i2c_enable = !!(hsotg->hwcfg3 & GHWCFG3_I2C); + hw->en_multiple_tx_fifo = !!(hsotg->hwcfg4 & GHWCFG4_DED_FIFO_EN); + hw->num_dev_perio_in_ep = (hsotg->hwcfg4 & GHWCFG4_NUM_DEV_PERIO_IN_EP_MASK) >> GHWCFG4_NUM_DEV_PERIO_IN_EP_SHIFT; + hw->dfifo_depth = (hsotg->hwcfg3 & GHWCFG3_DFIFO_DEPTH_MASK) >> GHWCFG3_DFIFO_DEPTH_SHIFT; + + // TODO Dump all values using dev_dbg + + return 0; +} + /* * This function is called during module intialization to pass module parameters * for the DWC_otg core. It returns non-0 if any parameters are invalid. diff --git a/drivers/staging/dwc2/core.h b/drivers/staging/dwc2/core.h index 934971b..18d1d61 100644 --- a/drivers/staging/dwc2/core.h +++ b/drivers/staging/dwc2/core.h @@ -220,6 +220,36 @@ struct dwc2_core_params { }; /** + * struct dwc2_hw_params - Autodetected parameters. + * + * These parameters are the various parameters read from hardware + * registers during initialization. They typically contain the best + * supported or maximum value that can be configured in the + * corresponding dwc_core_params value. + * + * See struct dwc_core_params for a description of the meaning of each + * of these values. + */ +struct dwc2_hw_params { + int op_mode; + int otg_cap; + int arch; + int dma_enable; + int dma_desc_enable; + int enable_dynamic_fifo; + int en_multiple_tx_fifo; + int host_rx_fifo_size; + int host_nperio_tx_fifo_size; + int host_perio_tx_fifo_size; + int max_transfer_size; + int max_packet_count; + int host_channels; + int hs_phy_type; + int fs_phy_type; + int i2c_enable; +}; + +/** * struct dwc2_hsotg - Holds the state of the driver, including the non-periodic * and periodic schedules * @@ -322,6 +352,9 @@ struct dwc2_core_params { struct dwc2_hsotg { struct device *dev; void __iomem *regs; + /** Params detected from hardware */ + struct dwc2_hw_params hw_params; + /** Params to actually use */ struct dwc2_core_params *core_params; u32 hwcfg1; u32 hwcfg2; diff --git a/drivers/staging/dwc2/hcd.c b/drivers/staging/dwc2/hcd.c index c0ae02e..a878a6b 100644 --- a/drivers/staging/dwc2/hcd.c +++ b/drivers/staging/dwc2/hcd.c @@ -2794,6 +2794,9 @@ int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int irq, dwc2_set_uninitialized((int *)hsotg->core_params, sizeof(*hsotg->core_params) / sizeof(int)); + /* Detect config values from hardware */ + dwc2_get_hwparams(hsotg); + /* Validate parameter values */ dwc2_set_parameters(hsotg, params); diff --git a/drivers/staging/dwc2/hcd.h b/drivers/staging/dwc2/hcd.h index d7f8c2f..3d8ab9b 100644 --- a/drivers/staging/dwc2/hcd.h +++ b/drivers/staging/dwc2/hcd.h @@ -452,6 +452,7 @@ extern int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int irq, extern void dwc2_hcd_remove(struct dwc2_hsotg *hsotg); extern int dwc2_set_parameters(struct dwc2_hsotg *hsotg, struct dwc2_core_params *params); +extern int dwc2_get_hwparams(struct dwc2_hsotg *hsotg); /* Transaction Execution Functions */ extern enum dwc2_transaction_type dwc2_hcd_select_transactions( -- 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