Hi, On Sat, Sep 27, 2014 at 01:05:46AM +0000, Paul Zimmerman wrote: > > From: Felipe Balbi [mailto:balbi@xxxxxx] > > Sent: Friday, September 26, 2014 5:54 PM > > > > On Fri, Sep 26, 2014 at 11:18:48PM +0000, Paul Zimmerman wrote: > > > > From: Felipe Balbi [mailto:balbi@xxxxxx] > > > > Sent: Friday, September 26, 2014 2:40 PM > > > > > > > > On Fri, Sep 26, 2014 at 08:57:19PM +0000, Paul Zimmerman wrote: > > > > > > > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c > > > > > > > index 0fcc0a3..8277065 100644 > > > > > > > --- a/drivers/usb/dwc3/gadget.c > > > > > > > +++ b/drivers/usb/dwc3/gadget.c > > > > > > > @@ -2635,6 +2635,7 @@ static irqreturn_t dwc3_interrupt(int irq, void *_dwc) > > > > > > > */ > > > > > > > int dwc3_gadget_init(struct dwc3 *dwc) > > > > > > > { > > > > > > > + u32 reg; > > > > > > > int ret; > > > > > > > > > > > > > > dwc->ctrl_req = dma_alloc_coherent(dwc->dev, sizeof(*dwc->ctrl_req), > > > > > > > @@ -2689,6 +2690,13 @@ int dwc3_gadget_init(struct dwc3 *dwc) > > > > > > > if (ret) > > > > > > > goto err4; > > > > > > > > > > > > > > + if (dwc->quirks & DWC3_AMD_NL_PLAT) { > > > > > > > + reg = dwc3_readl(dwc->regs, DWC3_DCTL); > > > > > > > + reg |= DWC3_DCTL_LPM_ERRATA(0xf); > > > > > > > > > > > > weird, why would Synopsys put this here ? It seems like this is only > > > > > > useful when LPM Errata is enabled and that's, apparently, a host-side > > > > > > thing. > > > > > > > > > > > > Paul, can you comment ? > > > > > > > > > > These bits contribute to how the device responds to an LPM transaction > > > > > from the host. If DCFG.LPMCap=1, they set the BESL value above which > > > > > the core will send a NYET. If DCFG.LPMCap=0, they have no effect. So > > > > > it's definitely a device-side thing, but only if the core is configured > > > > > with LPM Errata support enabled. > > > > > > > > right, and how can SW detect if LPM Errata was enabled ? From the host > > > > point of view, we can check bit 20 of xHCI capability register. What > > > > about device ? I can't seem to find anything :-s > > > > > > I just talked to one of our RTL designers. You're right, there is no > > > way to tell from the Device registers alone whether the controller is > > > configured with LPM Errata support or not. > > > > > > You can tell for sure if it is *not* enabled, by checking GSNPSID, and > > > if the version is earlier than 2.40a, the feature wasn't available. > > > > alright, we can use this for something like: > > > > WARN(rev < 2.40a && (flags & LPM_ERRATA || of_property_read_bool("lpm-errata")), > > "LPM Errata not available on dwc3 revisions <= 2.40a\n"); > > > > > So for Device-mode only controllers, I guess you will need a DT property > > > for this. > > > > right, DT property and platform_data feature flag, or something. I don't > > wanna call it a quirk though, although it _is_ an erratum WRT LPM > > support. Dunno. Any strong feelings ? > > 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); + } + dwc->start_config_issued = false; /* Start with SuperSpeed Default */ diff --git a/drivers/usb/dwc3/platform_data.h b/drivers/usb/dwc3/platform_data.h index 7db34f0..4bcec7c 100644 --- a/drivers/usb/dwc3/platform_data.h +++ b/drivers/usb/dwc3/platform_data.h @@ -24,4 +24,6 @@ struct dwc3_platform_data { enum usb_device_speed maximum_speed; enum usb_dr_mode dr_mode; bool tx_fifo_resize; + + unsigned has_lpm_erratum:1; }; note that I have also moved DCTL access from gadget_init to gadget_start. cheers -- balbi
Attachment:
signature.asc
Description: Digital signature
