Whoops, resending as text instead of html. > From: Matthijs Kooijman [mailto:matthijs@xxxxxxxx] > Sent: Thursday, March 28, 2013 9:32 AM > > Hi Paul, > > while continuing this patch, I stumbled upon a bit of code which doesn't > make sense to me. In dwc2_dump_global_registers is the following bit: > > 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; > txfsiz = "DPTXFSIZ"; > } else { > ep_num = hsotg->hwcfg4 >> GHWCFG4_NUM_IN_EPS_SHIFT & > GHWCFG4_NUM_IN_EPS_MASK >> GHWCFG4_NUM_IN_EPS_SHIFT; > txfsiz = "DIENPTXF"; > } > > for (i = 0; i < ep_num; i++) { > addr = hsotg->regs + DPTXFSIZN(i + 1); > dev_dbg(hsotg->dev, "%s[%d] @0x%08lX : 0x%08X\n", txfsiz, i + 1, > (unsigned long)addr, readl(addr)); > } > > There's a number of things: > - In the else above, the register name is set to DIENPTXF, but the values are > always read from DPTXFSIZN? If you look closely at the databook, you will see that DPTXFSIZn and DIEPTXFn are both aliases for the same register address. > - According to my docs for GHWCFG4_NUM_IN_EPS, the actual number of endpoints > is the value read + 1. > - This would result in 1-16 eps, but FIFO numbers are from 1-15 for DPTXFSIZ. That's a bug. There are a max of 15 registers, covering EPs 1-15. EP 0 is handled by GNPTXFSIZ. So I think the loop should be: for (i = 0; i < ep_num + 1; i++) { addr = hsotg->regs + DPTXFSIZN(i); dev_dbg(hsotg->dev, "%s[%d] @0x%08lX : 0x%08X\n", txfsiz, i, (unsigned long)addr, readl(addr)); } > Do you know what the deal is here? Or, given that this is only debug output for > device mode only, shall we just remove this code for now? Sure, that's fine with me too. Will you submit a patch for that? -- Paul -- 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
