> From: Matthijs Kooijman [mailto:matthijs@xxxxxxxx] > Sent: Wednesday, March 27, 2013 8:42 AM > To: Greg KH > > Hi Greg, > > > But yes, it does seem to be a good idea, > > Ok, thanks. > > > as long as these options can't change over time. > It's only read-only registers, or registers whose power-on values > determine their maximum possible value, so they indeed won't change over > time. Overall this seems to be an improvement, so I have no objections. There is a pretty high likelihood of something getting broken in the translation, however. For example, this: + hw->host_nperio_tx_fifo_size = (readl(hsotg->regs + GNPTXFSIZ) & GNPTXFSIZ_NP_TXF_DEP_MASK) << GNPTXFSIZ_NP_TXF_DEP_SHIFT; looks like a bug (shifted the wrong direction). But since the driver is still in staging and has no real users yet, that's not a big deal I guess. You probably should use u32 instead of int for the members of struct dwc2_hw_params, otherwise you might introduce a sign bug if any of the values get shifted into the high bit and become unexpectedly negative. And I would use either u8, bool, or bitfields for the true/false values, to save a little space. -- 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
