Hi, On Sat, Sep 27, 2014 at 01:05:46AM +0000, Paul Zimmerman wrote: > > From: Felipe Balbi [mailto:[email protected]] > > 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:[email protected]] > > > > 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
signature.asc
Description: Digital signature
