> From: Felipe Balbi [mailto:balbi@xxxxxx] > Sent: Friday, September 26, 2014 9:31 PM > > On Sat, Sep 27, 2014 at 01:05:46AM +0000, Paul Zimmerman wrote: > > > > Well, it's called LPM Errata because the feature was added to the USB > > spec as an erratum. It's not an erratum to our controller. But I don't > > have any strong feelings about how the driver support is implemented. > > how about adding a "has_lpm_erratum" to struct dwc3 which gets > initialized either through pdata or DT ? Then above WARN() would become: > > WARN(dwc->revision < DWC3_REVISION_240A && dwc->has_lpm_erratum", > "LPM Erratum not available on dwc3 revisisions < 2.40a\n"); > > Then we're not really calling it a quirk. In fact Huang, when respining > your series, let's convert your quirk bits into single-bit fields inside > struct dwc3 and struct platform_data. Like so: > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c > index 4d4e854..e233ce1 100644 > --- a/drivers/usb/dwc3/core.c > +++ b/drivers/usb/dwc3/core.c > @@ -695,11 +695,13 @@ static int dwc3_probe(struct platform_device *pdev) > > if (node) { > dwc->maximum_speed = of_usb_get_maximum_speed(node); > + dwc->has_lpm_erratum = of_property_read_bool(node, "snps,has-lpm-erratum"); > > dwc->needs_fifo_resize = of_property_read_bool(node, "tx-fifo-resize"); > dwc->dr_mode = of_usb_get_dr_mode(node); > } else if (pdata) { > dwc->maximum_speed = pdata->maximum_speed; > + dwc->has_lpm_erratum = pdata->has_lpm_erratum; > > dwc->needs_fifo_resize = pdata->tx_fifo_resize; > dwc->dr_mode = pdata->dr_mode; > diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h > index 66f6256..23e1504 100644 > --- a/drivers/usb/dwc3/core.h > +++ b/drivers/usb/dwc3/core.h > @@ -661,6 +661,8 @@ struct dwc3_scratchpad_array { > * @ep0_bounced: true when we used bounce buffer > * @ep0_expect_in: true when we expect a DATA IN transfer > * @has_hibernation: true when dwc3 was configured with Hibernation > + * @has_lpm_erratum: true when core was configured with LPM Erratum. Note that > + * there's now way for software to detect this in runtime. > * @is_selfpowered: true when we are selfpowered > * @needs_fifo_resize: not all users might want fifo resizing, flag it > * @pullups_connected: true when Run/Stop bit is set > @@ -764,6 +766,7 @@ struct dwc3 { > unsigned ep0_bounced:1; > unsigned ep0_expect_in:1; > unsigned has_hibernation:1; > + unsigned has_lpm_erratum:1; > unsigned is_selfpowered:1; > unsigned needs_fifo_resize:1; > unsigned pullups_connected:1; > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c > index 68497b3..112352e 100644 > --- a/drivers/usb/dwc3/gadget.c > +++ b/drivers/usb/dwc3/gadget.c > @@ -1577,6 +1577,13 @@ static int dwc3_gadget_start(struct usb_gadget *g, > } > dwc3_writel(dwc->regs, DWC3_DCFG, reg); > > + if (dwc->has_lpm_erratum) { > + reg = dwc3_readl(dwc->regs, DWC3_DCTL); > + /* REVISIT should this be configurable ? */ > + reg |= DWC3_DCTL_LPM_ERRATA(0xf); > + dwc3_writel(dwc->regs, DWC3_DCTL, reg); > + } Yes, I think this really wants to be configurable. The value used is supposed to depend on the latencies in the system etc. -- 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
