[PATCH v2 0/4] usb: dwc2: Add AHB burst configuration

[PATCH v2 0/4] usb: dwc2: Add AHB burst configuration

[John Youn]

This series adds a binding for AHB burst, reads it in, and configures
the controller for the specified burst type.

Tested on HAPS platform with DWC_hsotg IP version 3.30a.

v2:
* Don't remove the bcm2835 ahbcfg param and document why.


John Youn (4):
  usb: dwc2: Document the AHB burst value for bcm2835
  usb: dwc2: Add binding for AHB burst
  usb: dwc2: Use the ahb_burst param
  usb: dwc2: pci: Add AHB burst property for HAPS

 Documentation/devicetree/bindings/usb/dwc2.txt |  2 +
 drivers/usb/dwc2/core.h                        |  9 ++++
 drivers/usb/dwc2/gadget.c                      |  2 +-
 drivers/usb/dwc2/hcd.c                         |  8 ++-
 drivers/usb/dwc2/params.c                      | 73 ++++++++++++++++++++++----
 drivers/usb/dwc2/pci.c                         |  1 +
 6 files changed, 79 insertions(+), 16 deletions(-)

-- 
2.10.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 2/4] usb: dwc2: Add binding for AHB burst

[John Youn]

Add the "snps,ahb-burst" binding and read it in.

This property controls which burst type to perform on the AHB bus as a
master in internal DMA mode. This overrides the legacy param value,
which we need to keep around for now since several platforms use it.

Some platforms may see better or worse performance based on this
value. The HAPS platform is one example where all INCRx have worse
performance than INCR.

Other platforms (such as the Canyonlands board) report that the default
value causes system hangs.

Signed-off-by: John Youn <johnyoun-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
Cc: Christian Lamparter <chunkeey-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
---
 Documentation/devicetree/bindings/usb/dwc2.txt |  2 +
 drivers/usb/dwc2/core.h                        |  9 +++++
 drivers/usb/dwc2/params.c                      | 56 ++++++++++++++++++++++++++
 3 files changed, 67 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt b/Documentation/devicetree/bindings/usb/dwc2.txt
index 6c7c2bce..9e7b4b4 100644
--- a/Documentation/devicetree/bindings/usb/dwc2.txt
+++ b/Documentation/devicetree/bindings/usb/dwc2.txt
@@ -26,6 +26,8 @@ Optional properties:
 Refer to phy/phy-bindings.txt for generic phy consumer properties
 - dr_mode: shall be one of "host", "peripheral" and "otg"
   Refer to usb/generic.txt
+- snps,ahb-burst: specifies the ahb burst length. Valid arguments are:
+  "SINGLE", "INCR", "INCR4", "INCR8", "INCR16". Defaults to "INCR4".
 - g-rx-fifo-size: size of rx fifo size in gadget mode.
 - g-np-tx-fifo-size: size of non-periodic tx fifo size in gadget mode.
 - g-tx-fifo-size: size of periodic tx fifo per endpoint (except ep0) in gadget mode.
diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 9548d3e..75c238c 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -430,6 +430,12 @@ enum dwc2_ep0_state {
  *			needed.
  *			0 - No (default)
  *			1 - Yes
+ * @ahb_burst:          Specifies the AHB burst.
+ *                       0 - Single
+ *                       1 - INCR
+ *                       3 - INCR4 (default)
+ *                       5 - INCR8
+ *                       7 - INCR16
  * @g_dma:              Enables gadget dma usage (default: autodetect).
  * @g_dma_desc:         Enables gadget descriptor DMA (default: autodetect).
  * @g_rx_fifo_size:	The periodic rx fifo size for the device, in
@@ -507,6 +513,9 @@ struct dwc2_core_params {
 	 * properties and cannot be set directly in this structure.
 	 */
 
+	/* Global parameters */
+	u8 ahb_burst;
+
 	/* Host parameters */
 	bool host_dma;
 
diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
index d44b31c..8bc2745 100644
--- a/drivers/usb/dwc2/params.c
+++ b/drivers/usb/dwc2/params.c
@@ -1098,6 +1098,60 @@ static void dwc2_set_param_tx_fifo_sizes(struct dwc2_hsotg *hsotg)
 	}
 }
 
+static const char *const ahb_bursts[] = {
+	[GAHBCFG_HBSTLEN_SINGLE]	= "SINGLE",
+	[GAHBCFG_HBSTLEN_INCR]		= "INCR",
+	[GAHBCFG_HBSTLEN_INCR4]		= "INCR4",
+	[GAHBCFG_HBSTLEN_INCR8]		= "INCR8",
+	[GAHBCFG_HBSTLEN_INCR16]	= "INCR16",
+};
+
+static int dwc2_get_property_ahb_burst(struct dwc2_hsotg *hsotg)
+{
+	const char *str = NULL;
+	int burst;
+	int ret;
+
+	ret = device_property_read_string(hsotg->dev,
+					  "snps,ahb-burst", &str);
+	if (ret < 0)
+		return ret;
+
+	burst = match_string(ahb_bursts,
+			     ARRAY_SIZE(ahb_bursts), str);
+	if (burst < 0) {
+		dev_err(hsotg->dev,
+			"Invalid parameter '%s' for ahb-burst\n", str);
+	}
+
+	return burst;
+}
+
+static void dwc2_set_ahb_burst(struct dwc2_hsotg *hsotg)
+{
+	struct dwc2_core_params *p = &hsotg->params;
+	int burst;
+	int ret;
+
+	/* Default burst value */
+	burst = GAHBCFG_HBSTLEN_INCR4;
+
+	/* Get the legacy param value, if set. */
+	if (p->ahbcfg != -1) {
+		burst = (p->ahbcfg & GAHBCFG_HBSTLEN_MASK) >>
+			GAHBCFG_HBSTLEN_SHIFT;
+	}
+
+	/* Override it from devicetree, if set. */
+	ret = dwc2_get_property_ahb_burst(hsotg);
+	if (ret >= 0)
+		burst = ret;
+
+	/* Set the parameter */
+	p->ahb_burst = (u8)burst;
+	dev_dbg(hsotg->dev, "Setting ahb-burst to %d\n", burst);
+}
+
 static void dwc2_set_gadget_dma(struct dwc2_hsotg *hsotg)
 {
 	struct dwc2_hw_params *hw = &hsotg->hw_params;
@@ -1178,6 +1232,8 @@ static void dwc2_set_parameters(struct dwc2_hsotg *hsotg,
 	dwc2_set_param_external_id_pin_ctl(hsotg, params->external_id_pin_ctl);
 	dwc2_set_param_hibernation(hsotg, params->hibernation);
 
+	dwc2_set_ahb_burst(hsotg);
+
 	/*
 	 * Set devicetree-only parameters. These parameters do not
 	 * take any values from @params.
-- 
2.10.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/4] usb: dwc2: Add binding for AHB burst

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 911 bytes --]


Hi,

John Youn <johnyoun-HKixBCOQz3hWk0Htik3J/w@public.gmane.org> writes:
> Add the "snps,ahb-burst" binding and read it in.
>
> This property controls which burst type to perform on the AHB bus as a
> master in internal DMA mode. This overrides the legacy param value,
> which we need to keep around for now since several platforms use it.
>
> Some platforms may see better or worse performance based on this
> value. The HAPS platform is one example where all INCRx have worse
> performance than INCR.
>
> Other platforms (such as the Canyonlands board) report that the default
> value causes system hangs.
>
> Signed-off-by: John Youn <johnyoun-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
> Cc: Christian Lamparter <chunkeey-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>

it's getting rather late for this merge window. I still need an ack by
Rob or any of the devicetree folks.

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [PATCH v2 2/4] usb: dwc2: Add binding for AHB burst

[John Youn]

On 11/17/2016 3:28 AM, Felipe Balbi wrote:
> 
> Hi,
> 
> John Youn <johnyoun-HKixBCOQz3hWk0Htik3J/w@public.gmane.org> writes:
>> Add the "snps,ahb-burst" binding and read it in.
>>
>> This property controls which burst type to perform on the AHB bus as a
>> master in internal DMA mode. This overrides the legacy param value,
>> which we need to keep around for now since several platforms use it.
>>
>> Some platforms may see better or worse performance based on this
>> value. The HAPS platform is one example where all INCRx have worse
>> performance than INCR.
>>
>> Other platforms (such as the Canyonlands board) report that the default
>> value causes system hangs.
>>
>> Signed-off-by: John Youn <johnyoun-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
>> Cc: Christian Lamparter <chunkeey-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> 
> it's getting rather late for this merge window. I still need an ack by
> Rob or any of the devicetree folks.
> 

Sure, no problem if it doesn't make it.

Regards,
John

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/4] usb: dwc2: Add binding for AHB burst

[Stefan Wahren]

Hi John,

Am 17.11.2016 um 00:47 schrieb John Youn:
> Add the "snps,ahb-burst" binding and read it in.
>
> This property controls which burst type to perform on the AHB bus as a
> master in internal DMA mode. This overrides the legacy param value,
> which we need to keep around for now since several platforms use it.
>
> Some platforms may see better or worse performance based on this
> value. The HAPS platform is one example where all INCRx have worse
> performance than INCR.
>
> Other platforms (such as the Canyonlands board) report that the default
> value causes system hangs.
>
> Signed-off-by: John Youn <johnyoun-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
> Cc: Christian Lamparter <chunkeey-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/usb/dwc2.txt |  2 +
>  drivers/usb/dwc2/core.h                        |  9 +++++
>  drivers/usb/dwc2/params.c                      | 56 ++++++++++++++++++++++++++
>  3 files changed, 67 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt b/Documentation/devicetree/bindings/usb/dwc2.txt
> index 6c7c2bce..9e7b4b4 100644
> --- a/Documentation/devicetree/bindings/usb/dwc2.txt
> +++ b/Documentation/devicetree/bindings/usb/dwc2.txt

according to Documentation/devicetree/bindings/submitting-patches.txt
this change should be a separate patch.

> @@ -26,6 +26,8 @@ Optional properties:
>  Refer to phy/phy-bindings.txt for generic phy consumer properties
>  - dr_mode: shall be one of "host", "peripheral" and "otg"
>    Refer to usb/generic.txt
> +- snps,ahb-burst: specifies the ahb burst length. Valid arguments are:
> +  "SINGLE", "INCR", "INCR4", "INCR8", "INCR16". Defaults to "INCR4".

This doesn't apply in case of the bcm2835. I would prefer this option is
ignored in that case with a dev_warn("snps,ahb-burst is not supported on
this platform").

Stefan
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/4] usb: dwc2: Add binding for AHB burst

[John Youn]

On 11/17/2016 7:35 AM, Stefan Wahren wrote:
> Hi John,
> 
> Am 17.11.2016 um 00:47 schrieb John Youn:
>> Add the "snps,ahb-burst" binding and read it in.
>>
>> This property controls which burst type to perform on the AHB bus as a
>> master in internal DMA mode. This overrides the legacy param value,
>> which we need to keep around for now since several platforms use it.
>>
>> Some platforms may see better or worse performance based on this
>> value. The HAPS platform is one example where all INCRx have worse
>> performance than INCR.
>>
>> Other platforms (such as the Canyonlands board) report that the default
>> value causes system hangs.
>>
>> Signed-off-by: John Youn <johnyoun-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
>> Cc: Christian Lamparter <chunkeey-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
>> ---
>>  Documentation/devicetree/bindings/usb/dwc2.txt |  2 +
>>  drivers/usb/dwc2/core.h                        |  9 +++++
>>  drivers/usb/dwc2/params.c                      | 56 ++++++++++++++++++++++++++
>>  3 files changed, 67 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt b/Documentation/devicetree/bindings/usb/dwc2.txt
>> index 6c7c2bce..9e7b4b4 100644
>> --- a/Documentation/devicetree/bindings/usb/dwc2.txt
>> +++ b/Documentation/devicetree/bindings/usb/dwc2.txt
> 
> according to Documentation/devicetree/bindings/submitting-patches.txt
> this change should be a separate patch.

Ok

> 
>> @@ -26,6 +26,8 @@ Optional properties:
>>  Refer to phy/phy-bindings.txt for generic phy consumer properties
>>  - dr_mode: shall be one of "host", "peripheral" and "otg"
>>    Refer to usb/generic.txt
>> +- snps,ahb-burst: specifies the ahb burst length. Valid arguments are:
>> +  "SINGLE", "INCR", "INCR4", "INCR8", "INCR16". Defaults to "INCR4".
> 
> This doesn't apply in case of the bcm2835. I would prefer this option is
> ignored in that case with a dev_warn("snps,ahb-burst is not supported on
> this platform").

Sure I'll add this.

Regards,
John


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/4] usb: dwc2: Add binding for AHB burst

[Rob Herring]

On Thu, Nov 17, 2016 at 04:35:10PM +0100, Stefan Wahren wrote:
> Hi John,
> 
> Am 17.11.2016 um 00:47 schrieb John Youn:
> > Add the "snps,ahb-burst" binding and read it in.
> >
> > This property controls which burst type to perform on the AHB bus as a
> > master in internal DMA mode. This overrides the legacy param value,
> > which we need to keep around for now since several platforms use it.
> >
> > Some platforms may see better or worse performance based on this
> > value. The HAPS platform is one example where all INCRx have worse
> > performance than INCR.
> >
> > Other platforms (such as the Canyonlands board) report that the default
> > value causes system hangs.
> >
> > Signed-off-by: John Youn <johnyoun-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
> > Cc: Christian Lamparter <chunkeey-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> > ---
> >  Documentation/devicetree/bindings/usb/dwc2.txt |  2 +
> >  drivers/usb/dwc2/core.h                        |  9 +++++
> >  drivers/usb/dwc2/params.c                      | 56 ++++++++++++++++++++++++++
> >  3 files changed, 67 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt b/Documentation/devicetree/bindings/usb/dwc2.txt
> > index 6c7c2bce..9e7b4b4 100644
> > --- a/Documentation/devicetree/bindings/usb/dwc2.txt
> > +++ b/Documentation/devicetree/bindings/usb/dwc2.txt
> 
> according to Documentation/devicetree/bindings/submitting-patches.txt
> this change should be a separate patch.
> 
> > @@ -26,6 +26,8 @@ Optional properties:
> >  Refer to phy/phy-bindings.txt for generic phy consumer properties
> >  - dr_mode: shall be one of "host", "peripheral" and "otg"
> >    Refer to usb/generic.txt
> > +- snps,ahb-burst: specifies the ahb burst length. Valid arguments are:
> > +  "SINGLE", "INCR", "INCR4", "INCR8", "INCR16". Defaults to "INCR4".
> 
> This doesn't apply in case of the bcm2835. I would prefer this option is
> ignored in that case with a dev_warn("snps,ahb-burst is not supported on
> this platform").

Also, perhaps you should allow that the compatible string can define the 
default.

Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/4] usb: dwc2: Add binding for AHB burst

[Christian Lamparter]

On Friday, November 18, 2016 8:16:08 AM CET Rob Herring wrote:
> On Thu, Nov 17, 2016 at 04:35:10PM +0100, Stefan Wahren wrote:
> > Hi John,
> > 
> > Am 17.11.2016 um 00:47 schrieb John Youn:
> > > Add the "snps,ahb-burst" binding and read it in.
> > >
> > > This property controls which burst type to perform on the AHB bus as a
> > > master in internal DMA mode. This overrides the legacy param value,
> > > which we need to keep around for now since several platforms use it.
> > >
> > > Some platforms may see better or worse performance based on this
> > > value. The HAPS platform is one example where all INCRx have worse
> > > performance than INCR.
> > >
> > > Other platforms (such as the Canyonlands board) report that the default
> > > value causes system hangs.
> > >
> > > Signed-off-by: John Youn <johnyoun-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
> > > Cc: Christian Lamparter <chunkeey-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> > > ---
> > >  Documentation/devicetree/bindings/usb/dwc2.txt |  2 +
> > >  drivers/usb/dwc2/core.h                        |  9 +++++
> > >  drivers/usb/dwc2/params.c                      | 56 ++++++++++++++++++++++++++
> > >  3 files changed, 67 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt b/Documentation/devicetree/bindings/usb/dwc2.txt
> > > index 6c7c2bce..9e7b4b4 100644
> > > --- a/Documentation/devicetree/bindings/usb/dwc2.txt
> > > +++ b/Documentation/devicetree/bindings/usb/dwc2.txt
> > 
> > according to Documentation/devicetree/bindings/submitting-patches.txt
> > this change should be a separate patch.
> > 
> > > @@ -26,6 +26,8 @@ Optional properties:
> > >  Refer to phy/phy-bindings.txt for generic phy consumer properties
> > >  - dr_mode: shall be one of "host", "peripheral" and "otg"
> > >    Refer to usb/generic.txt
> > > +- snps,ahb-burst: specifies the ahb burst length. Valid arguments are:
> > > +  "SINGLE", "INCR", "INCR4", "INCR8", "INCR16". Defaults to "INCR4".
> > 
> > This doesn't apply in case of the bcm2835. I would prefer this option is
> > ignored in that case with a dev_warn("snps,ahb-burst is not supported on
> > this platform").
> 
> Also, perhaps you should allow that the compatible string can define the 
> default.
> 
I hoped you would say that :).

I've attached a patch (on top of John Youn changes) that does
just that for the amcc,dwc-otg. I put the GAHBCFG_HBSTLEN_INCR
value into the .data, if that's a problem, I can certainly 
respin the patch and put it in a dedicated struct.

Regards

Christian
---
>From 4c31a029dde714828810b1c3e61a5b1412ac939a Mon Sep 17 00:00:00 2001
From: Christian Lamparter <chunkeey-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Date: Fri, 18 Nov 2016 21:03:19 +0100
Subject: [PATCH] usb: dwc2: add a default ahb-burst setting for amcc,dwc-otg

This patch adds a of_device_id table which can be used by
existing devices to supply a ahb-burst value for the platform
without having to add a "snps,ahb-burst" entry to the dts.

Note: Adding new devices to this table is discouraged.
      please consider adding the "snps,ahb-burst" property
      with the correct configuration to your device tree
      file instead.

Signed-off-by: Christian Lamparter <chunkeey-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/usb/dwc2/params.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
index e0fc9aa..51be266 100644
--- a/drivers/usb/dwc2/params.c
+++ b/drivers/usb/dwc2/params.c
@@ -1097,6 +1097,22 @@ static const char *const ahb_bursts[] = {
 	[GAHBCFG_HBSTLEN_INCR16]	= "INCR16",
 };
 
+/*
+ * This table provides AHB burst configuration for existing
+ * device tree bindings that work poorly with the default setting.
+ *
+ * Note: Adding new devices to this table is discouraged.
+ *	 please consider adding the "snps,ahb-burst" property
+ *	 with the correct configuration to your device tree
+ *	 file instead.
+ */
+static const struct of_device_id dwc2_compat_ahb_bursts[] = {
+	{
+		.compatible = "amcc,dwc-otg",
+		.data = (void *) GAHBCFG_HBSTLEN_INCR16,
+	},
+};
+
 static int dwc2_get_property_ahb_burst(struct dwc2_hsotg *hsotg)
 {
 	struct device_node *node = hsotg->dev->of_node;
@@ -1107,6 +1123,12 @@ static int dwc2_get_property_ahb_burst(struct dwc2_hsotg *hsotg)
 	ret = device_property_read_string(hsotg->dev,
 					  "snps,ahb-burst", &str);
 	if (ret < 0) {
+		const struct of_device_id *match;
+
+		match = of_match_node(dwc2_compat_ahb_bursts, node);
+		if (match)
+			ret = (int)match->data;
+
 		return ret;
 	} else if (of_device_is_compatible(node, "brcm,bcm2835-usb")) {
 		dev_warn(hsotg->dev,
-- 
2.10.2





--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/4] usb: dwc2: Add binding for AHB burst

[John Youn]

On 11/18/2016 12:18 PM, Christian Lamparter wrote:
> On Friday, November 18, 2016 8:16:08 AM CET Rob Herring wrote:
>> On Thu, Nov 17, 2016 at 04:35:10PM +0100, Stefan Wahren wrote:
>>> Hi John,
>>>
>>> Am 17.11.2016 um 00:47 schrieb John Youn:
>>>> Add the "snps,ahb-burst" binding and read it in.
>>>>
>>>> This property controls which burst type to perform on the AHB bus as a
>>>> master in internal DMA mode. This overrides the legacy param value,
>>>> which we need to keep around for now since several platforms use it.
>>>>
>>>> Some platforms may see better or worse performance based on this
>>>> value. The HAPS platform is one example where all INCRx have worse
>>>> performance than INCR.
>>>>
>>>> Other platforms (such as the Canyonlands board) report that the default
>>>> value causes system hangs.
>>>>
>>>> Signed-off-by: John Youn <johnyoun-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
>>>> Cc: Christian Lamparter <chunkeey-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
>>>> ---
>>>>  Documentation/devicetree/bindings/usb/dwc2.txt |  2 +
>>>>  drivers/usb/dwc2/core.h                        |  9 +++++
>>>>  drivers/usb/dwc2/params.c                      | 56 ++++++++++++++++++++++++++
>>>>  3 files changed, 67 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt b/Documentation/devicetree/bindings/usb/dwc2.txt
>>>> index 6c7c2bce..9e7b4b4 100644
>>>> --- a/Documentation/devicetree/bindings/usb/dwc2.txt
>>>> +++ b/Documentation/devicetree/bindings/usb/dwc2.txt
>>>
>>> according to Documentation/devicetree/bindings/submitting-patches.txt
>>> this change should be a separate patch.
>>>
>>>> @@ -26,6 +26,8 @@ Optional properties:
>>>>  Refer to phy/phy-bindings.txt for generic phy consumer properties
>>>>  - dr_mode: shall be one of "host", "peripheral" and "otg"
>>>>    Refer to usb/generic.txt
>>>> +- snps,ahb-burst: specifies the ahb burst length. Valid arguments are:
>>>> +  "SINGLE", "INCR", "INCR4", "INCR8", "INCR16". Defaults to "INCR4".
>>>
>>> This doesn't apply in case of the bcm2835. I would prefer this option is
>>> ignored in that case with a dev_warn("snps,ahb-burst is not supported on
>>> this platform").
>>
>> Also, perhaps you should allow that the compatible string can define the 
>> default.
>>
> I hoped you would say that :).
> 
> I've attached a patch (on top of John Youn changes) that does
> just that for the amcc,dwc-otg. I put the GAHBCFG_HBSTLEN_INCR
> value into the .data, if that's a problem, I can certainly 
> respin the patch and put it in a dedicated struct.
> 
> Regards
> 
> Christian
> ---
> From 4c31a029dde714828810b1c3e61a5b1412ac939a Mon Sep 17 00:00:00 2001
> From: Christian Lamparter <chunkeey-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Date: Fri, 18 Nov 2016 21:03:19 +0100
> Subject: [PATCH] usb: dwc2: add a default ahb-burst setting for amcc,dwc-otg
> 
> This patch adds a of_device_id table which can be used by
> existing devices to supply a ahb-burst value for the platform
> without having to add a "snps,ahb-burst" entry to the dts.
> 
> Note: Adding new devices to this table is discouraged.
>       please consider adding the "snps,ahb-burst" property
>       with the correct configuration to your device tree
>       file instead.
> 
> Signed-off-by: Christian Lamparter <chunkeey-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  drivers/usb/dwc2/params.c | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
> index e0fc9aa..51be266 100644
> --- a/drivers/usb/dwc2/params.c
> +++ b/drivers/usb/dwc2/params.c
> @@ -1097,6 +1097,22 @@ static const char *const ahb_bursts[] = {
>  	[GAHBCFG_HBSTLEN_INCR16]	= "INCR16",
>  };
>  
> +/*
> + * This table provides AHB burst configuration for existing
> + * device tree bindings that work poorly with the default setting.
> + *
> + * Note: Adding new devices to this table is discouraged.
> + *	 please consider adding the "snps,ahb-burst" property
> + *	 with the correct configuration to your device tree
> + *	 file instead.
> + */
> +static const struct of_device_id dwc2_compat_ahb_bursts[] = {
> +	{
> +		.compatible = "amcc,dwc-otg",
> +		.data = (void *) GAHBCFG_HBSTLEN_INCR16,
> +	},
> +};
> +
>  static int dwc2_get_property_ahb_burst(struct dwc2_hsotg *hsotg)
>  {
>  	struct device_node *node = hsotg->dev->of_node;
> @@ -1107,6 +1123,12 @@ static int dwc2_get_property_ahb_burst(struct dwc2_hsotg *hsotg)
>  	ret = device_property_read_string(hsotg->dev,
>  					  "snps,ahb-burst", &str);
>  	if (ret < 0) {
> +		const struct of_device_id *match;
> +
> +		match = of_match_node(dwc2_compat_ahb_bursts, node);
> +		if (match)
> +			ret = (int)match->data;
> +
>  		return ret;
>  	} else if (of_device_is_compatible(node, "brcm,bcm2835-usb")) {
>  		dev_warn(hsotg->dev,
> 

Hi Christian,

I'd prefer if you use the binding which requires no extra code in
dwc2.

Regards,
John
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/4] usb: dwc2: Add binding for AHB burst

[Christian Lamparter]

Hello John,

On Monday, November 21, 2016 12:16:31 PM CET John Youn wrote:
> On 11/18/2016 12:18 PM, Christian Lamparter wrote:
> > On Friday, November 18, 2016 8:16:08 AM CET Rob Herring wrote:
> >> On Thu, Nov 17, 2016 at 04:35:10PM +0100, Stefan Wahren wrote:
> >>> Hi John,
> >>>
> >>> Am 17.11.2016 um 00:47 schrieb John Youn:
> >>>> Add the "snps,ahb-burst" binding and read it in.
> >>>>
> >>>> This property controls which burst type to perform on the AHB bus as a
> >>>> master in internal DMA mode. This overrides the legacy param value,
> >>>> which we need to keep around for now since several platforms use it.
> >>>>
> >>>> Some platforms may see better or worse performance based on this
> >>>> value. The HAPS platform is one example where all INCRx have worse
> >>>> performance than INCR.
> >>>>
> >>>> Other platforms (such as the Canyonlands board) report that the default
> >>>> value causes system hangs.
> >>>>
> >>>> Signed-off-by: John Youn <johnyoun-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
> >>>> Cc: Christian Lamparter <chunkeey-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> >>>> ---
> >>>>  Documentation/devicetree/bindings/usb/dwc2.txt |  2 +
> >>>>  drivers/usb/dwc2/core.h                        |  9 +++++
> >>>>  drivers/usb/dwc2/params.c                      | 56 ++++++++++++++++++++++++++
> >>>>  3 files changed, 67 insertions(+)
> >>>>
> >>>> diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt b/Documentation/devicetree/bindings/usb/dwc2.txt
> >>>> index 6c7c2bce..9e7b4b4 100644
> >>>> --- a/Documentation/devicetree/bindings/usb/dwc2.txt
> >>>> +++ b/Documentation/devicetree/bindings/usb/dwc2.txt
> >>>
> >>> according to Documentation/devicetree/bindings/submitting-patches.txt
> >>> this change should be a separate patch.
> >>>
> >>>> @@ -26,6 +26,8 @@ Optional properties:
> >>>>  Refer to phy/phy-bindings.txt for generic phy consumer properties
> >>>>  - dr_mode: shall be one of "host", "peripheral" and "otg"
> >>>>    Refer to usb/generic.txt
> >>>> +- snps,ahb-burst: specifies the ahb burst length. Valid arguments are:
> >>>> +  "SINGLE", "INCR", "INCR4", "INCR8", "INCR16". Defaults to "INCR4".
> >>>
> >>> This doesn't apply in case of the bcm2835. I would prefer this option is
> >>> ignored in that case with a dev_warn("snps,ahb-burst is not supported on
> >>> this platform").
> >>
> >> Also, perhaps you should allow that the compatible string can define the 
> >> default.
> >>
> > I hoped you would say that :).
> > 
> > I've attached a patch (on top of John Youn changes) that does
> > just that for the amcc,dwc-otg. I put the GAHBCFG_HBSTLEN_INCR
> > value into the .data, if that's a problem, I can certainly 
> > respin the patch and put it in a dedicated struct.
> > 
> > Regards
> > 
> > Christian
> > ---
> > From 4c31a029dde714828810b1c3e61a5b1412ac939a Mon Sep 17 00:00:00 2001
> > From: Christian Lamparter <chunkeey-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > Date: Fri, 18 Nov 2016 21:03:19 +0100
> > Subject: [PATCH] usb: dwc2: add a default ahb-burst setting for amcc,dwc-otg
> > 
> > This patch adds a of_device_id table which can be used by
> > existing devices to supply a ahb-burst value for the platform
> > without having to add a "snps,ahb-burst" entry to the dts.
> > 
> > Note: Adding new devices to this table is discouraged.
> >       please consider adding the "snps,ahb-burst" property
> >       with the correct configuration to your device tree
> >       file instead.
> > 
> > Signed-off-by: Christian Lamparter <chunkeey-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > ---
> >  drivers/usb/dwc2/params.c | 22 ++++++++++++++++++++++
> >  1 file changed, 22 insertions(+)
> > 
> > diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
> > index e0fc9aa..51be266 100644
> > --- a/drivers/usb/dwc2/params.c
> > +++ b/drivers/usb/dwc2/params.c
> > @@ -1097,6 +1097,22 @@ static const char *const ahb_bursts[] = {
> >  	[GAHBCFG_HBSTLEN_INCR16]	= "INCR16",
> >  };
> >  
> > +/*
> > + * This table provides AHB burst configuration for existing
> > + * device tree bindings that work poorly with the default setting.
> > + *
> > + * Note: Adding new devices to this table is discouraged.
> > + *	 please consider adding the "snps,ahb-burst" property
> > + *	 with the correct configuration to your device tree
> > + *	 file instead.
> > + */
> > +static const struct of_device_id dwc2_compat_ahb_bursts[] = {
> > +	{
> > +		.compatible = "amcc,dwc-otg",
> > +		.data = (void *) GAHBCFG_HBSTLEN_INCR16,
> > +	},
> > +};
> > +
> >  static int dwc2_get_property_ahb_burst(struct dwc2_hsotg *hsotg)
> >  {
> >  	struct device_node *node = hsotg->dev->of_node;
> > @@ -1107,6 +1123,12 @@ static int dwc2_get_property_ahb_burst(struct dwc2_hsotg *hsotg)
> >  	ret = device_property_read_string(hsotg->dev,
> >  					  "snps,ahb-burst", &str);
> >  	if (ret < 0) {
> > +		const struct of_device_id *match;
> > +
> > +		match = of_match_node(dwc2_compat_ahb_bursts, node);
> > +		if (match)
> > +			ret = (int)match->data;
> > +
> >  		return ret;
> >  	} else if (of_device_is_compatible(node, "brcm,bcm2835-usb")) {
> >  		dev_warn(hsotg->dev,
> > 
> I'd prefer if you use the binding which requires no extra code in
> dwc2.
I'm fine with either option. However it think that this would require
that either Mark or Rob would allow an exception to the "keep existing
dts the way they are) and ack the following change to the canyonlands.dts. 

In that case I wouldn't need the overwrite in dwc2_get_property_ahb_burst.

Regards,
Christian
---
>From e78604cb0b8ea8db277ef9bf321a613f8e0c7129 Mon Sep 17 00:00:00 2001
From: Christian Lamparter <chunkeey-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Date: Mon, 21 Nov 2016 21:46:19 +0100
Subject: [PATCH] powerpc/dts: set snps,ahb-burst to INCR16

The dwc2 driver defaults to INCR4 which can cause a
system hang when the USB and SATA is used concurrently.

Note: This patch requires:
	"usb: dwc2: add amcc,dwc-otg support"
	(which already landed in the usb subsystem queue)
	and "usb: dwc2: Add AHB burst configuration"

Signed-off-by: Christian Lamparter <chunkeey-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 arch/powerpc/boot/dts/canyonlands.dts | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/boot/dts/canyonlands.dts b/arch/powerpc/boot/dts/canyonlands.dts
index 0d6ac92..90db712 100644
--- a/arch/powerpc/boot/dts/canyonlands.dts
+++ b/arch/powerpc/boot/dts/canyonlands.dts
@@ -179,6 +179,7 @@
 
 		USBOTG0: usbotg@bff80000 {
 			compatible = "amcc,dwc-otg";
+			snps,ahb-burst = "INCR16";
 			reg = <0x4 0xbff80000 0x10000>;
 			interrupt-parent = <&USBOTG0>;
 			#interrupt-cells = <1>;
-- 
2.10.2


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/4] usb: dwc2: Add binding for AHB burst

[Christian Lamparter]

Hello John,

On Monday, November 21, 2016 12:16:31 PM CET John Youn wrote:
> On 11/18/2016 12:18 PM, Christian Lamparter wrote:
> > On Friday, November 18, 2016 8:16:08 AM CET Rob Herring wrote:
> >> On Thu, Nov 17, 2016 at 04:35:10PM +0100, Stefan Wahren wrote:
> >>> Hi John,
> >>>
> >>> Am 17.11.2016 um 00:47 schrieb John Youn:
> >>>> Add the "snps,ahb-burst" binding and read it in.
> >>>>
> >>>> This property controls which burst type to perform on the AHB bus as a
> >>>> master in internal DMA mode. This overrides the legacy param value,
> >>>> which we need to keep around for now since several platforms use it.
> >>>>
> >>>> Some platforms may see better or worse performance based on this
> >>>> value. The HAPS platform is one example where all INCRx have worse
> >>>> performance than INCR.
> >>>>
> >>>> Other platforms (such as the Canyonlands board) report that the default
> >>>> value causes system hangs.
> >>>>
> >>>> Signed-off-by: John Youn <johnyoun@synopsys.com>
> >>>> Cc: Christian Lamparter <chunkeey@googlemail.com>
> >>>> ---
> >>>>  Documentation/devicetree/bindings/usb/dwc2.txt |  2 +
> >>>>  drivers/usb/dwc2/core.h                        |  9 +++++
> >>>>  drivers/usb/dwc2/params.c                      | 56 ++++++++++++++++++++++++++
> >>>>  3 files changed, 67 insertions(+)
> >>>>
> >>>> diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt b/Documentation/devicetree/bindings/usb/dwc2.txt
> >>>> index 6c7c2bce..9e7b4b4 100644
> >>>> --- a/Documentation/devicetree/bindings/usb/dwc2.txt
> >>>> +++ b/Documentation/devicetree/bindings/usb/dwc2.txt
> >>>
> >>> according to Documentation/devicetree/bindings/submitting-patches.txt
> >>> this change should be a separate patch.
> >>>
> >>>> @@ -26,6 +26,8 @@ Optional properties:
> >>>>  Refer to phy/phy-bindings.txt for generic phy consumer properties
> >>>>  - dr_mode: shall be one of "host", "peripheral" and "otg"
> >>>>    Refer to usb/generic.txt
> >>>> +- snps,ahb-burst: specifies the ahb burst length. Valid arguments are:
> >>>> +  "SINGLE", "INCR", "INCR4", "INCR8", "INCR16". Defaults to "INCR4".
> >>>
> >>> This doesn't apply in case of the bcm2835. I would prefer this option is
> >>> ignored in that case with a dev_warn("snps,ahb-burst is not supported on
> >>> this platform").
> >>
> >> Also, perhaps you should allow that the compatible string can define the 
> >> default.
> >>
> > I hoped you would say that :).
> > 
> > I've attached a patch (on top of John Youn changes) that does
> > just that for the amcc,dwc-otg. I put the GAHBCFG_HBSTLEN_INCR
> > value into the .data, if that's a problem, I can certainly 
> > respin the patch and put it in a dedicated struct.
> > 
> > Regards
> > 
> > Christian
> > ---
> > From 4c31a029dde714828810b1c3e61a5b1412ac939a Mon Sep 17 00:00:00 2001
> > From: Christian Lamparter <chunkeey@gmail.com>
> > Date: Fri, 18 Nov 2016 21:03:19 +0100
> > Subject: [PATCH] usb: dwc2: add a default ahb-burst setting for amcc,dwc-otg
> > 
> > This patch adds a of_device_id table which can be used by
> > existing devices to supply a ahb-burst value for the platform
> > without having to add a "snps,ahb-burst" entry to the dts.
> > 
> > Note: Adding new devices to this table is discouraged.
> >       please consider adding the "snps,ahb-burst" property
> >       with the correct configuration to your device tree
> >       file instead.
> > 
> > Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
> > ---
> >  drivers/usb/dwc2/params.c | 22 ++++++++++++++++++++++
> >  1 file changed, 22 insertions(+)
> > 
> > diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
> > index e0fc9aa..51be266 100644
> > --- a/drivers/usb/dwc2/params.c
> > +++ b/drivers/usb/dwc2/params.c
> > @@ -1097,6 +1097,22 @@ static const char *const ahb_bursts[] = {
> >  	[GAHBCFG_HBSTLEN_INCR16]	= "INCR16",
> >  };
> >  
> > +/*
> > + * This table provides AHB burst configuration for existing
> > + * device tree bindings that work poorly with the default setting.
> > + *
> > + * Note: Adding new devices to this table is discouraged.
> > + *	 please consider adding the "snps,ahb-burst" property
> > + *	 with the correct configuration to your device tree
> > + *	 file instead.
> > + */
> > +static const struct of_device_id dwc2_compat_ahb_bursts[] = {
> > +	{
> > +		.compatible = "amcc,dwc-otg",
> > +		.data = (void *) GAHBCFG_HBSTLEN_INCR16,
> > +	},
> > +};
> > +
> >  static int dwc2_get_property_ahb_burst(struct dwc2_hsotg *hsotg)
> >  {
> >  	struct device_node *node = hsotg->dev->of_node;
> > @@ -1107,6 +1123,12 @@ static int dwc2_get_property_ahb_burst(struct dwc2_hsotg *hsotg)
> >  	ret = device_property_read_string(hsotg->dev,
> >  					  "snps,ahb-burst", &str);
> >  	if (ret < 0) {
> > +		const struct of_device_id *match;
> > +
> > +		match = of_match_node(dwc2_compat_ahb_bursts, node);
> > +		if (match)
> > +			ret = (int)match->data;
> > +
> >  		return ret;
> >  	} else if (of_device_is_compatible(node, "brcm,bcm2835-usb")) {
> >  		dev_warn(hsotg->dev,
> > 
> I'd prefer if you use the binding which requires no extra code in
> dwc2.
I'm fine with either option. However it think that this would require
that either Mark or Rob would allow an exception to the "keep existing
dts the way they are) and ack the following change to the canyonlands.dts. 

In that case I wouldn't need the overwrite in dwc2_get_property_ahb_burst.

Regards,
Christian
---
>From e78604cb0b8ea8db277ef9bf321a613f8e0c7129 Mon Sep 17 00:00:00 2001
From: Christian Lamparter <chunkeey@gmail.com>
Date: Mon, 21 Nov 2016 21:46:19 +0100
Subject: [PATCH] powerpc/dts: set snps,ahb-burst to INCR16

The dwc2 driver defaults to INCR4 which can cause a
system hang when the USB and SATA is used concurrently.

Note: This patch requires:
	"usb: dwc2: add amcc,dwc-otg support"
	(which already landed in the usb subsystem queue)
	and "usb: dwc2: Add AHB burst configuration"

Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
---
 arch/powerpc/boot/dts/canyonlands.dts | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/boot/dts/canyonlands.dts b/arch/powerpc/boot/dts/canyonlands.dts
index 0d6ac92..90db712 100644
--- a/arch/powerpc/boot/dts/canyonlands.dts
+++ b/arch/powerpc/boot/dts/canyonlands.dts
@@ -179,6 +179,7 @@
 
 		USBOTG0: usbotg@bff80000 {
 			compatible = "amcc,dwc-otg";
+			snps,ahb-burst = "INCR16";
 			reg = <0x4 0xbff80000 0x10000>;
 			interrupt-parent = <&USBOTG0>;
 			#interrupt-cells = <1>;
-- 
2.10.2

Re: [PATCH v2 2/4] usb: dwc2: Add binding for AHB burst

[John Youn]

On 11/21/2016 1:10 PM, Christian Lamparter wrote:
> Hello John,
> 
> On Monday, November 21, 2016 12:16:31 PM CET John Youn wrote:
>> On 11/18/2016 12:18 PM, Christian Lamparter wrote:
>>> On Friday, November 18, 2016 8:16:08 AM CET Rob Herring wrote:
>>>> On Thu, Nov 17, 2016 at 04:35:10PM +0100, Stefan Wahren wrote:
>>>>> Hi John,
>>>>>
>>>>> Am 17.11.2016 um 00:47 schrieb John Youn:
>>>>>> Add the "snps,ahb-burst" binding and read it in.
>>>>>>
>>>>>> This property controls which burst type to perform on the AHB bus as a
>>>>>> master in internal DMA mode. This overrides the legacy param value,
>>>>>> which we need to keep around for now since several platforms use it.
>>>>>>
>>>>>> Some platforms may see better or worse performance based on this
>>>>>> value. The HAPS platform is one example where all INCRx have worse
>>>>>> performance than INCR.
>>>>>>
>>>>>> Other platforms (such as the Canyonlands board) report that the default
>>>>>> value causes system hangs.
>>>>>>
>>>>>> Signed-off-by: John Youn <johnyoun-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
>>>>>> Cc: Christian Lamparter <chunkeey-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
>>>>>> ---
>>>>>>  Documentation/devicetree/bindings/usb/dwc2.txt |  2 +
>>>>>>  drivers/usb/dwc2/core.h                        |  9 +++++
>>>>>>  drivers/usb/dwc2/params.c                      | 56 ++++++++++++++++++++++++++
>>>>>>  3 files changed, 67 insertions(+)
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt b/Documentation/devicetree/bindings/usb/dwc2.txt
>>>>>> index 6c7c2bce..9e7b4b4 100644
>>>>>> --- a/Documentation/devicetree/bindings/usb/dwc2.txt
>>>>>> +++ b/Documentation/devicetree/bindings/usb/dwc2.txt
>>>>>
>>>>> according to Documentation/devicetree/bindings/submitting-patches.txt
>>>>> this change should be a separate patch.
>>>>>
>>>>>> @@ -26,6 +26,8 @@ Optional properties:
>>>>>>  Refer to phy/phy-bindings.txt for generic phy consumer properties
>>>>>>  - dr_mode: shall be one of "host", "peripheral" and "otg"
>>>>>>    Refer to usb/generic.txt
>>>>>> +- snps,ahb-burst: specifies the ahb burst length. Valid arguments are:
>>>>>> +  "SINGLE", "INCR", "INCR4", "INCR8", "INCR16". Defaults to "INCR4".
>>>>>
>>>>> This doesn't apply in case of the bcm2835. I would prefer this option is
>>>>> ignored in that case with a dev_warn("snps,ahb-burst is not supported on
>>>>> this platform").
>>>>
>>>> Also, perhaps you should allow that the compatible string can define the 
>>>> default.
>>>>
>>> I hoped you would say that :).
>>>
>>> I've attached a patch (on top of John Youn changes) that does
>>> just that for the amcc,dwc-otg. I put the GAHBCFG_HBSTLEN_INCR
>>> value into the .data, if that's a problem, I can certainly 
>>> respin the patch and put it in a dedicated struct.
>>>
>>> Regards
>>>
>>> Christian
>>> ---
>>> From 4c31a029dde714828810b1c3e61a5b1412ac939a Mon Sep 17 00:00:00 2001
>>> From: Christian Lamparter <chunkeey-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>> Date: Fri, 18 Nov 2016 21:03:19 +0100
>>> Subject: [PATCH] usb: dwc2: add a default ahb-burst setting for amcc,dwc-otg
>>>
>>> This patch adds a of_device_id table which can be used by
>>> existing devices to supply a ahb-burst value for the platform
>>> without having to add a "snps,ahb-burst" entry to the dts.
>>>
>>> Note: Adding new devices to this table is discouraged.
>>>       please consider adding the "snps,ahb-burst" property
>>>       with the correct configuration to your device tree
>>>       file instead.
>>>
>>> Signed-off-by: Christian Lamparter <chunkeey-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>> ---
>>>  drivers/usb/dwc2/params.c | 22 ++++++++++++++++++++++
>>>  1 file changed, 22 insertions(+)
>>>
>>> diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
>>> index e0fc9aa..51be266 100644
>>> --- a/drivers/usb/dwc2/params.c
>>> +++ b/drivers/usb/dwc2/params.c
>>> @@ -1097,6 +1097,22 @@ static const char *const ahb_bursts[] = {
>>>  	[GAHBCFG_HBSTLEN_INCR16]	= "INCR16",
>>>  };
>>>  
>>> +/*
>>> + * This table provides AHB burst configuration for existing
>>> + * device tree bindings that work poorly with the default setting.
>>> + *
>>> + * Note: Adding new devices to this table is discouraged.
>>> + *	 please consider adding the "snps,ahb-burst" property
>>> + *	 with the correct configuration to your device tree
>>> + *	 file instead.
>>> + */
>>> +static const struct of_device_id dwc2_compat_ahb_bursts[] = {
>>> +	{
>>> +		.compatible = "amcc,dwc-otg",
>>> +		.data = (void *) GAHBCFG_HBSTLEN_INCR16,
>>> +	},
>>> +};
>>> +
>>>  static int dwc2_get_property_ahb_burst(struct dwc2_hsotg *hsotg)
>>>  {
>>>  	struct device_node *node = hsotg->dev->of_node;
>>> @@ -1107,6 +1123,12 @@ static int dwc2_get_property_ahb_burst(struct dwc2_hsotg *hsotg)
>>>  	ret = device_property_read_string(hsotg->dev,
>>>  					  "snps,ahb-burst", &str);
>>>  	if (ret < 0) {
>>> +		const struct of_device_id *match;
>>> +
>>> +		match = of_match_node(dwc2_compat_ahb_bursts, node);
>>> +		if (match)
>>> +			ret = (int)match->data;
>>> +
>>>  		return ret;
>>>  	} else if (of_device_is_compatible(node, "brcm,bcm2835-usb")) {
>>>  		dev_warn(hsotg->dev,
>>>
>> I'd prefer if you use the binding which requires no extra code in
>> dwc2.
> I'm fine with either option. However it think that this would require
> that either Mark or Rob would allow an exception to the "keep existing
> dts the way they are) and ack the following change to the canyonlands.dts. 

I don't know about that. Under what circumstance can the dts change?

The canyonlands dts was binding to an external vendor driver. So it
wasn't documented nor expected to work with dwc2 until your recent
patch adding the compatible string.

Systems that use the vendor driver will still work with the dts. If
you remove the vendor driver and configure it to use dwc2, it won't
work due to a quirk of the canyonlands hardware, for which you need to
add a dts property.

I think this is reasonable. Rob or Mark, any feedback?

One of the reasons I don't want to add the code in dwc2 is because I'm
trying to make dwc2 a generic IP driver like dwc3.

Regards,
John

> 
> In that case I wouldn't need the overwrite in dwc2_get_property_ahb_burst.
> 
> Regards,
> Christian
> ---
> From e78604cb0b8ea8db277ef9bf321a613f8e0c7129 Mon Sep 17 00:00:00 2001
> From: Christian Lamparter <chunkeey-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Date: Mon, 21 Nov 2016 21:46:19 +0100
> Subject: [PATCH] powerpc/dts: set snps,ahb-burst to INCR16
> 
> The dwc2 driver defaults to INCR4 which can cause a
> system hang when the USB and SATA is used concurrently.
> 
> Note: This patch requires:
> 	"usb: dwc2: add amcc,dwc-otg support"
> 	(which already landed in the usb subsystem queue)
> 	and "usb: dwc2: Add AHB burst configuration"
> 
> Signed-off-by: Christian Lamparter <chunkeey-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  arch/powerpc/boot/dts/canyonlands.dts | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/powerpc/boot/dts/canyonlands.dts b/arch/powerpc/boot/dts/canyonlands.dts
> index 0d6ac92..90db712 100644
> --- a/arch/powerpc/boot/dts/canyonlands.dts
> +++ b/arch/powerpc/boot/dts/canyonlands.dts
> @@ -179,6 +179,7 @@
>  
>  		USBOTG0: usbotg@bff80000 {
>  			compatible = "amcc,dwc-otg";
> +			snps,ahb-burst = "INCR16";
>  			reg = <0x4 0xbff80000 0x10000>;
>  			interrupt-parent = <&USBOTG0>;
>  			#interrupt-cells = <1>;
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/4] usb: dwc2: Add binding for AHB burst

[John Youn]

On 11/21/2016 1:10 PM, Christian Lamparter wrote:
> Hello John,
> 
> On Monday, November 21, 2016 12:16:31 PM CET John Youn wrote:
>> On 11/18/2016 12:18 PM, Christian Lamparter wrote:
>>> On Friday, November 18, 2016 8:16:08 AM CET Rob Herring wrote:
>>>> On Thu, Nov 17, 2016 at 04:35:10PM +0100, Stefan Wahren wrote:
>>>>> Hi John,
>>>>>
>>>>> Am 17.11.2016 um 00:47 schrieb John Youn:
>>>>>> Add the "snps,ahb-burst" binding and read it in.
>>>>>>
>>>>>> This property controls which burst type to perform on the AHB bus as a
>>>>>> master in internal DMA mode. This overrides the legacy param value,
>>>>>> which we need to keep around for now since several platforms use it.
>>>>>>
>>>>>> Some platforms may see better or worse performance based on this
>>>>>> value. The HAPS platform is one example where all INCRx have worse
>>>>>> performance than INCR.
>>>>>>
>>>>>> Other platforms (such as the Canyonlands board) report that the default
>>>>>> value causes system hangs.
>>>>>>
>>>>>> Signed-off-by: John Youn <johnyoun@synopsys.com>
>>>>>> Cc: Christian Lamparter <chunkeey@googlemail.com>
>>>>>> ---
>>>>>>  Documentation/devicetree/bindings/usb/dwc2.txt |  2 +
>>>>>>  drivers/usb/dwc2/core.h                        |  9 +++++
>>>>>>  drivers/usb/dwc2/params.c                      | 56 ++++++++++++++++++++++++++
>>>>>>  3 files changed, 67 insertions(+)
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt b/Documentation/devicetree/bindings/usb/dwc2.txt
>>>>>> index 6c7c2bce..9e7b4b4 100644
>>>>>> --- a/Documentation/devicetree/bindings/usb/dwc2.txt
>>>>>> +++ b/Documentation/devicetree/bindings/usb/dwc2.txt
>>>>>
>>>>> according to Documentation/devicetree/bindings/submitting-patches.txt
>>>>> this change should be a separate patch.
>>>>>
>>>>>> @@ -26,6 +26,8 @@ Optional properties:
>>>>>>  Refer to phy/phy-bindings.txt for generic phy consumer properties
>>>>>>  - dr_mode: shall be one of "host", "peripheral" and "otg"
>>>>>>    Refer to usb/generic.txt
>>>>>> +- snps,ahb-burst: specifies the ahb burst length. Valid arguments are:
>>>>>> +  "SINGLE", "INCR", "INCR4", "INCR8", "INCR16". Defaults to "INCR4".
>>>>>
>>>>> This doesn't apply in case of the bcm2835. I would prefer this option is
>>>>> ignored in that case with a dev_warn("snps,ahb-burst is not supported on
>>>>> this platform").
>>>>
>>>> Also, perhaps you should allow that the compatible string can define the 
>>>> default.
>>>>
>>> I hoped you would say that :).
>>>
>>> I've attached a patch (on top of John Youn changes) that does
>>> just that for the amcc,dwc-otg. I put the GAHBCFG_HBSTLEN_INCR
>>> value into the .data, if that's a problem, I can certainly 
>>> respin the patch and put it in a dedicated struct.
>>>
>>> Regards
>>>
>>> Christian
>>> ---
>>> From 4c31a029dde714828810b1c3e61a5b1412ac939a Mon Sep 17 00:00:00 2001
>>> From: Christian Lamparter <chunkeey@gmail.com>
>>> Date: Fri, 18 Nov 2016 21:03:19 +0100
>>> Subject: [PATCH] usb: dwc2: add a default ahb-burst setting for amcc,dwc-otg
>>>
>>> This patch adds a of_device_id table which can be used by
>>> existing devices to supply a ahb-burst value for the platform
>>> without having to add a "snps,ahb-burst" entry to the dts.
>>>
>>> Note: Adding new devices to this table is discouraged.
>>>       please consider adding the "snps,ahb-burst" property
>>>       with the correct configuration to your device tree
>>>       file instead.
>>>
>>> Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
>>> ---
>>>  drivers/usb/dwc2/params.c | 22 ++++++++++++++++++++++
>>>  1 file changed, 22 insertions(+)
>>>
>>> diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
>>> index e0fc9aa..51be266 100644
>>> --- a/drivers/usb/dwc2/params.c
>>> +++ b/drivers/usb/dwc2/params.c
>>> @@ -1097,6 +1097,22 @@ static const char *const ahb_bursts[] = {
>>>  	[GAHBCFG_HBSTLEN_INCR16]	= "INCR16",
>>>  };
>>>  
>>> +/*
>>> + * This table provides AHB burst configuration for existing
>>> + * device tree bindings that work poorly with the default setting.
>>> + *
>>> + * Note: Adding new devices to this table is discouraged.
>>> + *	 please consider adding the "snps,ahb-burst" property
>>> + *	 with the correct configuration to your device tree
>>> + *	 file instead.
>>> + */
>>> +static const struct of_device_id dwc2_compat_ahb_bursts[] = {
>>> +	{
>>> +		.compatible = "amcc,dwc-otg",
>>> +		.data = (void *) GAHBCFG_HBSTLEN_INCR16,
>>> +	},
>>> +};
>>> +
>>>  static int dwc2_get_property_ahb_burst(struct dwc2_hsotg *hsotg)
>>>  {
>>>  	struct device_node *node = hsotg->dev->of_node;
>>> @@ -1107,6 +1123,12 @@ static int dwc2_get_property_ahb_burst(struct dwc2_hsotg *hsotg)
>>>  	ret = device_property_read_string(hsotg->dev,
>>>  					  "snps,ahb-burst", &str);
>>>  	if (ret < 0) {
>>> +		const struct of_device_id *match;
>>> +
>>> +		match = of_match_node(dwc2_compat_ahb_bursts, node);
>>> +		if (match)
>>> +			ret = (int)match->data;
>>> +
>>>  		return ret;
>>>  	} else if (of_device_is_compatible(node, "brcm,bcm2835-usb")) {
>>>  		dev_warn(hsotg->dev,
>>>
>> I'd prefer if you use the binding which requires no extra code in
>> dwc2.
> I'm fine with either option. However it think that this would require
> that either Mark or Rob would allow an exception to the "keep existing
> dts the way they are) and ack the following change to the canyonlands.dts. 

I don't know about that. Under what circumstance can the dts change?

The canyonlands dts was binding to an external vendor driver. So it
wasn't documented nor expected to work with dwc2 until your recent
patch adding the compatible string.

Systems that use the vendor driver will still work with the dts. If
you remove the vendor driver and configure it to use dwc2, it won't
work due to a quirk of the canyonlands hardware, for which you need to
add a dts property.

I think this is reasonable. Rob or Mark, any feedback?

One of the reasons I don't want to add the code in dwc2 is because I'm
trying to make dwc2 a generic IP driver like dwc3.

Regards,
John

> 
> In that case I wouldn't need the overwrite in dwc2_get_property_ahb_burst.
> 
> Regards,
> Christian
> ---
> From e78604cb0b8ea8db277ef9bf321a613f8e0c7129 Mon Sep 17 00:00:00 2001
> From: Christian Lamparter <chunkeey@gmail.com>
> Date: Mon, 21 Nov 2016 21:46:19 +0100
> Subject: [PATCH] powerpc/dts: set snps,ahb-burst to INCR16
> 
> The dwc2 driver defaults to INCR4 which can cause a
> system hang when the USB and SATA is used concurrently.
> 
> Note: This patch requires:
> 	"usb: dwc2: add amcc,dwc-otg support"
> 	(which already landed in the usb subsystem queue)
> 	and "usb: dwc2: Add AHB burst configuration"
> 
> Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
> ---
>  arch/powerpc/boot/dts/canyonlands.dts | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/powerpc/boot/dts/canyonlands.dts b/arch/powerpc/boot/dts/canyonlands.dts
> index 0d6ac92..90db712 100644
> --- a/arch/powerpc/boot/dts/canyonlands.dts
> +++ b/arch/powerpc/boot/dts/canyonlands.dts
> @@ -179,6 +179,7 @@
>  
>  		USBOTG0: usbotg@bff80000 {
>  			compatible = "amcc,dwc-otg";
> +			snps,ahb-burst = "INCR16";
>  			reg = <0x4 0xbff80000 0x10000>;
>  			interrupt-parent = <&USBOTG0>;
>  			#interrupt-cells = <1>;
> 

Re: [PATCH v2 2/4] usb: dwc2: Add binding for AHB burst

[Christian Lamparter]

On Monday, November 21, 2016 7:32:30 PM CET John Youn wrote:
> On 11/21/2016 1:10 PM, Christian Lamparter wrote:
> > On Monday, November 21, 2016 12:16:31 PM CET John Youn wrote:
> >> On 11/18/2016 12:18 PM, Christian Lamparter wrote:
> >>> On Friday, November 18, 2016 8:16:08 AM CET Rob Herring wrote:
> >>>> Also, perhaps you should allow that the compatible string can define the 
> >>>> default.
> >>>>
> >>> I hoped you would say that :).
> >>>
> >>> I've attached a patch (on top of John Youn changes) [...]
> >>> ---
> >>> Subject: [PATCH] usb: dwc2: add a default ahb-burst setting for amcc,dwc-otg
> >>> [...]
> >>> @@ -1097,6 +1097,22 @@ static const char *const ahb_bursts[] = {
> >>> +/* [...] */
> >>> +static const struct of_device_id dwc2_compat_ahb_bursts[] = {
> >>> +	{
> >>> +		.compatible = "amcc,dwc-otg",
> >>> +		.data = (void *) GAHBCFG_HBSTLEN_INCR16,
> >>> +	},
> >>> +};
> [...]
> > >>> @@ -1107,6 +1123,12 @@ static int dwc2_get_property_ahb_burst(struct dwc2_hsotg *hsotg)
> >>>  	ret = device_property_read_string(hsotg->dev, "snps,ahb-burst", &str);
> >>>  	if (ret < 0) {
> >>> +		const struct of_device_id *match;
> >>> +
> >>> +		match = of_match_node(dwc2_compat_ahb_bursts, node);
> >>> +		if (match)
> >>> +			ret = (int)match->data;
> >>> +
> [...]
> >> I'd prefer if you use the binding which requires no extra code in
> >> dwc2.
> > I'm fine with either option. However it think that this would require
> > that either Mark or Rob would allow an exception to the "keep existing
> > dts the way they are) and ack the following change to the canyonlands.dts. 
> 
> I don't know about that. Under what circumstance can the dts change?
As far as I know, the justification for not changing the DTS is that a
compiled DTB might be stored in an read-only ROM on a board. So it would
be impossible to update it. Hence, the driver have work with the existing
(and sometimes buggy or incomplete) information to stay compatible.

(Note: Thankfully, the canyonlands dtb is stored in flash, it's possible
to update it. But it is an extra step that's not done automatically
with make install). 

> The canyonlands dts was binding to an external vendor driver. So it
> wasn't documented nor expected to work with dwc2 until your recent
> patch adding the compatible string.

Oh, no that's not what happend. Let me explain why there was no "external 
vendor driver": AMCC/APM were planing to upstream their hole platform. And
in fact, the devs tried very hard to include their driver back in 2011 [0].
But this driver was denied inclusion back then due to:

"[...]
I would also like to point out that the same Synopsys USB controller
is used in a number of other SoCs (especially ARM chips), and
supported by other drivers, some of these even in mainline.

See http://thread.gmane.org/gmane.linux.usb.general/61714/focus=62139
for a related thread.

Instead of trying to add a completely new driver to mainline (and one
which has been repeatedly been rejected), I vote for focusing on the
existing driver code that is already in mainline, and testing and
improving this so we can use a single implementation of this driver
code for all SoCs that use the same IP block." [1]

Of course: The listed link goes the "USB Host driver for i.MX28" driver.
And this is an ehci-hcd like driver... Which is as you are well aware not
that similar to the dwc2 OTG. And as far as I can tell: AMCC abandoned
the patch series right there. 

Note: AMCC did however succeed in pushing your employer's Synopsys
DesignWare SATA and DMA drivers to the kernel back then. And I'm happy
to report that both drivers are still around and working fine for the 460EX 
(sata_dwc_460ex.c[2] and the DW AHB DMA [3]). (The drivers also work for
different platforms than the original PPC. I know that because I helped
Andy Shevchenko with testing and pushing some fixes to it when he was
adding support for the Intel Quark SoC, which uses the DWC SATA and DMA).

So Please?
> Systems that use the vendor driver will still work with the dts. If
> you remove the vendor driver and configure it to use dwc2, it won't
> work due to a quirk of the canyonlands hardware, for which you need to
> add a dts property.
Sadly, there is no up to date vendor driver. The canyonlands.dts binding
is still in place and the hardware works fine. I'm interested in this
platform since it is a cheap BigEndian system which is useful for usb
driver development (carl9170 and rtl8192su)... and I would like to
have out-of-the-box support.

> I think this is reasonable. Rob or Mark, any feedback?
I recall that Rob has already voiced his opinion about the ahb-burst setting: 
"Also, perhaps you should allow that the compatible string can define the default."

And based on that, I made the "add a default ahb-burst setting for amcc,dwc-otg"
patch above. Of course, it would be nice to have any feedback too. But unless I
hear otherwise, I'll continue with posting patches to the dwc2 driver :). 

> One of the reasons I don't want to add the code in dwc2 is because I'm
> trying to make dwc2 a generic IP driver like dwc3.
I understand that. And let me say, that I also have a dwc3 in my IPQ4019. 
And adding support for it was as simple as adding just one compatible
binding in dwc-of-simple [4].

Regards,
Christian

[0] <http://thread.gmane.org/gmane.linux.usb.general/53348/focus=53913>
[1] <https://lists.ozlabs.org/pipermail/linuxppc-dev/2012-May/097850.html>
[2] <http://lxr.free-electrons.com/source/drivers/ata/sata_dwc_460ex.c>
[3] <http://lxr.free-electrons.com/source/drivers/dma/dw/core.c>
[4] <https://github.com/chunkeey/LEDE-IPQ40XX/blob/staging/target/linux/ipq40xx/patches-4.8/830-usb-dwc3-register-qca-ipq4019-dwc3-in-dwc3-of-simple.patch>

Re: [PATCH v2 2/4] usb: dwc2: Add binding for AHB burst

[Christian Lamparter]

On Monday, November 21, 2016 7:32:30 PM CET John Youn wrote:
> On 11/21/2016 1:10 PM, Christian Lamparter wrote:
> > On Monday, November 21, 2016 12:16:31 PM CET John Youn wrote:
> >> On 11/18/2016 12:18 PM, Christian Lamparter wrote:
> >>> On Friday, November 18, 2016 8:16:08 AM CET Rob Herring wrote:
> >>>> Also, perhaps you should allow that the compatible string can define the 
> >>>> default.
> >>>>
> >>> I hoped you would say that :).
> >>>
> >>> I've attached a patch (on top of John Youn changes) [...]
> >>> ---
> >>> Subject: [PATCH] usb: dwc2: add a default ahb-burst setting for amcc,dwc-otg
> >>> [...]
> >>> @@ -1097,6 +1097,22 @@ static const char *const ahb_bursts[] = {
> >>> +/* [...] */
> >>> +static const struct of_device_id dwc2_compat_ahb_bursts[] = {
> >>> +	{
> >>> +		.compatible = "amcc,dwc-otg",
> >>> +		.data = (void *) GAHBCFG_HBSTLEN_INCR16,
> >>> +	},
> >>> +};
> [...]
> > >>> @@ -1107,6 +1123,12 @@ static int dwc2_get_property_ahb_burst(struct dwc2_hsotg *hsotg)
> >>>  	ret = device_property_read_string(hsotg->dev, "snps,ahb-burst", &str);
> >>>  	if (ret < 0) {
> >>> +		const struct of_device_id *match;
> >>> +
> >>> +		match = of_match_node(dwc2_compat_ahb_bursts, node);
> >>> +		if (match)
> >>> +			ret = (int)match->data;
> >>> +
> [...]
> >> I'd prefer if you use the binding which requires no extra code in
> >> dwc2.
> > I'm fine with either option. However it think that this would require
> > that either Mark or Rob would allow an exception to the "keep existing
> > dts the way they are) and ack the following change to the canyonlands.dts. 
> 
> I don't know about that. Under what circumstance can the dts change?
As far as I know, the justification for not changing the DTS is that a
compiled DTB might be stored in an read-only ROM on a board. So it would
be impossible to update it. Hence, the driver have work with the existing
(and sometimes buggy or incomplete) information to stay compatible.

(Note: Thankfully, the canyonlands dtb is stored in flash, it's possible
to update it. But it is an extra step that's not done automatically
with make install). 

> The canyonlands dts was binding to an external vendor driver. So it
> wasn't documented nor expected to work with dwc2 until your recent
> patch adding the compatible string.

Oh, no that's not what happend. Let me explain why there was no "external 
vendor driver": AMCC/APM were planing to upstream their hole platform. And
in fact, the devs tried very hard to include their driver back in 2011 [0].
But this driver was denied inclusion back then due to:

"[...]
I would also like to point out that the same Synopsys USB controller
is used in a number of other SoCs (especially ARM chips), and
supported by other drivers, some of these even in mainline.

See http://thread.gmane.org/gmane.linux.usb.general/61714/focus=62139
for a related thread.

Instead of trying to add a completely new driver to mainline (and one
which has been repeatedly been rejected), I vote for focusing on the
existing driver code that is already in mainline, and testing and
improving this so we can use a single implementation of this driver
code for all SoCs that use the same IP block." [1]

Of course: The listed link goes the "USB Host driver for i.MX28" driver.
And this is an ehci-hcd like driver... Which is as you are well aware not
that similar to the dwc2 OTG. And as far as I can tell: AMCC abandoned
the patch series right there. 

Note: AMCC did however succeed in pushing your employer's Synopsys
DesignWare SATA and DMA drivers to the kernel back then. And I'm happy
to report that both drivers are still around and working fine for the 460EX 
(sata_dwc_460ex.c[2] and the DW AHB DMA [3]). (The drivers also work for
different platforms than the original PPC. I know that because I helped
Andy Shevchenko with testing and pushing some fixes to it when he was
adding support for the Intel Quark SoC, which uses the DWC SATA and DMA).

So Please?
> Systems that use the vendor driver will still work with the dts. If
> you remove the vendor driver and configure it to use dwc2, it won't
> work due to a quirk of the canyonlands hardware, for which you need to
> add a dts property.
Sadly, there is no up to date vendor driver. The canyonlands.dts binding
is still in place and the hardware works fine. I'm interested in this
platform since it is a cheap BigEndian system which is useful for usb
driver development (carl9170 and rtl8192su)... and I would like to
have out-of-the-box support.

> I think this is reasonable. Rob or Mark, any feedback?
I recall that Rob has already voiced his opinion about the ahb-burst setting: 
"Also, perhaps you should allow that the compatible string can define the default."

And based on that, I made the "add a default ahb-burst setting for amcc,dwc-otg"
patch above. Of course, it would be nice to have any feedback too. But unless I
hear otherwise, I'll continue with posting patches to the dwc2 driver :). 

> One of the reasons I don't want to add the code in dwc2 is because I'm
> trying to make dwc2 a generic IP driver like dwc3.
I understand that. And let me say, that I also have a dwc3 in my IPQ4019. 
And adding support for it was as simple as adding just one compatible
binding in dwc-of-simple [4].

Regards,
Christian

[0] <http://thread.gmane.org/gmane.linux.usb.general/53348/focus=53913>
[1] <https://lists.ozlabs.org/pipermail/linuxppc-dev/2012-May/097850.html>
[2] <http://lxr.free-electrons.com/source/drivers/ata/sata_dwc_460ex.c>
[3] <http://lxr.free-electrons.com/source/drivers/dma/dw/core.c>
[4] <https://github.com/chunkeey/LEDE-IPQ40XX/blob/staging/target/linux/ipq40xx/patches-4.8/830-usb-dwc3-register-qca-ipq4019-dwc3-in-dwc3-of-simple.patch>

Re: [PATCH v2 2/4] usb: dwc2: Add binding for AHB burst

[Rob Herring]

On Tue, Nov 22, 2016 at 2:51 PM, Christian Lamparter
<chunkeey@googlemail.com> wrote:
> On Monday, November 21, 2016 7:32:30 PM CET John Youn wrote:
>> On 11/21/2016 1:10 PM, Christian Lamparter wrote:
>> > On Monday, November 21, 2016 12:16:31 PM CET John Youn wrote:
>> >> On 11/18/2016 12:18 PM, Christian Lamparter wrote:
>> >>> On Friday, November 18, 2016 8:16:08 AM CET Rob Herring wrote:
>> >>>> Also, perhaps you should allow that the compatible string can define the
>> >>>> default.
>> >>>>
>> >>> I hoped you would say that :).
>> >>>
>> >>> I've attached a patch (on top of John Youn changes) [...]
>> >>> ---
>> >>> Subject: [PATCH] usb: dwc2: add a default ahb-burst setting for amcc,dwc-otg
>> >>> [...]
>> >>> @@ -1097,6 +1097,22 @@ static const char *const ahb_bursts[] = {
>> >>> +/* [...] */
>> >>> +static const struct of_device_id dwc2_compat_ahb_bursts[] = {
>> >>> + {
>> >>> +         .compatible = "amcc,dwc-otg",
>> >>> +         .data = (void *) GAHBCFG_HBSTLEN_INCR16,
>> >>> + },
>> >>> +};
>> [...]
>> > >>> @@ -1107,6 +1123,12 @@ static int dwc2_get_property_ahb_burst(struct dwc2_hsotg *hsotg)
>> >>>   ret = device_property_read_string(hsotg->dev, "snps,ahb-burst", &str);
>> >>>   if (ret < 0) {
>> >>> +         const struct of_device_id *match;
>> >>> +
>> >>> +         match = of_match_node(dwc2_compat_ahb_bursts, node);
>> >>> +         if (match)
>> >>> +                 ret = (int)match->data;
>> >>> +
>> [...]
>> >> I'd prefer if you use the binding which requires no extra code in
>> >> dwc2.
>> > I'm fine with either option. However it think that this would require
>> > that either Mark or Rob would allow an exception to the "keep existing
>> > dts the way they are) and ack the following change to the canyonlands.dts.
>>
>> I don't know about that. Under what circumstance can the dts change?
> As far as I know, the justification for not changing the DTS is that a
> compiled DTB might be stored in an read-only ROM on a board. So it would
> be impossible to update it. Hence, the driver have work with the existing
> (and sometimes buggy or incomplete) information to stay compatible.
>
> (Note: Thankfully, the canyonlands dtb is stored in flash, it's possible
> to update it. But it is an extra step that's not done automatically
> with make install).
>
>> The canyonlands dts was binding to an external vendor driver. So it
>> wasn't documented nor expected to work with dwc2 until your recent
>> patch adding the compatible string.
>
> Oh, no that's not what happend. Let me explain why there was no "external
> vendor driver": AMCC/APM were planing to upstream their hole platform. And
> in fact, the devs tried very hard to include their driver back in 2011 [0].
> But this driver was denied inclusion back then due to:
>
> "[...]
> I would also like to point out that the same Synopsys USB controller
> is used in a number of other SoCs (especially ARM chips), and
> supported by other drivers, some of these even in mainline.
>
> See http://thread.gmane.org/gmane.linux.usb.general/61714/focus=62139
> for a related thread.
>
> Instead of trying to add a completely new driver to mainline (and one
> which has been repeatedly been rejected), I vote for focusing on the
> existing driver code that is already in mainline, and testing and
> improving this so we can use a single implementation of this driver
> code for all SoCs that use the same IP block." [1]
>
> Of course: The listed link goes the "USB Host driver for i.MX28" driver.
> And this is an ehci-hcd like driver... Which is as you are well aware not
> that similar to the dwc2 OTG. And as far as I can tell: AMCC abandoned
> the patch series right there.
>
> Note: AMCC did however succeed in pushing your employer's Synopsys
> DesignWare SATA and DMA drivers to the kernel back then. And I'm happy
> to report that both drivers are still around and working fine for the 460EX
> (sata_dwc_460ex.c[2] and the DW AHB DMA [3]). (The drivers also work for
> different platforms than the original PPC. I know that because I helped
> Andy Shevchenko with testing and pushing some fixes to it when he was
> adding support for the Intel Quark SoC, which uses the DWC SATA and DMA).
>
> So Please?
>> Systems that use the vendor driver will still work with the dts. If
>> you remove the vendor driver and configure it to use dwc2, it won't
>> work due to a quirk of the canyonlands hardware, for which you need to
>> add a dts property.
> Sadly, there is no up to date vendor driver. The canyonlands.dts binding
> is still in place and the hardware works fine. I'm interested in this
> platform since it is a cheap BigEndian system which is useful for usb
> driver development (carl9170 and rtl8192su)... and I would like to
> have out-of-the-box support.
>
>> I think this is reasonable. Rob or Mark, any feedback?
> I recall that Rob has already voiced his opinion about the ahb-burst setting:
> "Also, perhaps you should allow that the compatible string can define the default."
>
> And based on that, I made the "add a default ahb-burst setting for amcc,dwc-otg"
> patch above. Of course, it would be nice to have any feedback too. But unless I
> hear otherwise, I'll continue with posting patches to the dwc2 driver :).

And this is the correct thing to do. Requiring a dtb update is not.

Rob

Re: [PATCH v2 2/4] usb: dwc2: Add binding for AHB burst

[Rob Herring]

On Tue, Nov 22, 2016 at 2:51 PM, Christian Lamparter
<chunkeey@googlemail.com> wrote:
> On Monday, November 21, 2016 7:32:30 PM CET John Youn wrote:
>> On 11/21/2016 1:10 PM, Christian Lamparter wrote:
>> > On Monday, November 21, 2016 12:16:31 PM CET John Youn wrote:
>> >> On 11/18/2016 12:18 PM, Christian Lamparter wrote:
>> >>> On Friday, November 18, 2016 8:16:08 AM CET Rob Herring wrote:
>> >>>> Also, perhaps you should allow that the compatible string can define the
>> >>>> default.
>> >>>>
>> >>> I hoped you would say that :).
>> >>>
>> >>> I've attached a patch (on top of John Youn changes) [...]
>> >>> ---
>> >>> Subject: [PATCH] usb: dwc2: add a default ahb-burst setting for amcc,dwc-otg
>> >>> [...]
>> >>> @@ -1097,6 +1097,22 @@ static const char *const ahb_bursts[] = {
>> >>> +/* [...] */
>> >>> +static const struct of_device_id dwc2_compat_ahb_bursts[] = {
>> >>> + {
>> >>> +         .compatible = "amcc,dwc-otg",
>> >>> +         .data = (void *) GAHBCFG_HBSTLEN_INCR16,
>> >>> + },
>> >>> +};
>> [...]
>> > >>> @@ -1107,6 +1123,12 @@ static int dwc2_get_property_ahb_burst(struct dwc2_hsotg *hsotg)
>> >>>   ret = device_property_read_string(hsotg->dev, "snps,ahb-burst", &str);
>> >>>   if (ret < 0) {
>> >>> +         const struct of_device_id *match;
>> >>> +
>> >>> +         match = of_match_node(dwc2_compat_ahb_bursts, node);
>> >>> +         if (match)
>> >>> +                 ret = (int)match->data;
>> >>> +
>> [...]
>> >> I'd prefer if you use the binding which requires no extra code in
>> >> dwc2.
>> > I'm fine with either option. However it think that this would require
>> > that either Mark or Rob would allow an exception to the "keep existing
>> > dts the way they are) and ack the following change to the canyonlands.dts.
>>
>> I don't know about that. Under what circumstance can the dts change?
> As far as I know, the justification for not changing the DTS is that a
> compiled DTB might be stored in an read-only ROM on a board. So it would
> be impossible to update it. Hence, the driver have work with the existing
> (and sometimes buggy or incomplete) information to stay compatible.
>
> (Note: Thankfully, the canyonlands dtb is stored in flash, it's possible
> to update it. But it is an extra step that's not done automatically
> with make install).
>
>> The canyonlands dts was binding to an external vendor driver. So it
>> wasn't documented nor expected to work with dwc2 until your recent
>> patch adding the compatible string.
>
> Oh, no that's not what happend. Let me explain why there was no "external
> vendor driver": AMCC/APM were planing to upstream their hole platform. And
> in fact, the devs tried very hard to include their driver back in 2011 [0].
> But this driver was denied inclusion back then due to:
>
> "[...]
> I would also like to point out that the same Synopsys USB controller
> is used in a number of other SoCs (especially ARM chips), and
> supported by other drivers, some of these even in mainline.
>
> See http://thread.gmane.org/gmane.linux.usb.general/61714/focus=62139
> for a related thread.
>
> Instead of trying to add a completely new driver to mainline (and one
> which has been repeatedly been rejected), I vote for focusing on the
> existing driver code that is already in mainline, and testing and
> improving this so we can use a single implementation of this driver
> code for all SoCs that use the same IP block." [1]
>
> Of course: The listed link goes the "USB Host driver for i.MX28" driver.
> And this is an ehci-hcd like driver... Which is as you are well aware not
> that similar to the dwc2 OTG. And as far as I can tell: AMCC abandoned
> the patch series right there.
>
> Note: AMCC did however succeed in pushing your employer's Synopsys
> DesignWare SATA and DMA drivers to the kernel back then. And I'm happy
> to report that both drivers are still around and working fine for the 460EX
> (sata_dwc_460ex.c[2] and the DW AHB DMA [3]). (The drivers also work for
> different platforms than the original PPC. I know that because I helped
> Andy Shevchenko with testing and pushing some fixes to it when he was
> adding support for the Intel Quark SoC, which uses the DWC SATA and DMA).
>
> So Please?
>> Systems that use the vendor driver will still work with the dts. If
>> you remove the vendor driver and configure it to use dwc2, it won't
>> work due to a quirk of the canyonlands hardware, for which you need to
>> add a dts property.
> Sadly, there is no up to date vendor driver. The canyonlands.dts binding
> is still in place and the hardware works fine. I'm interested in this
> platform since it is a cheap BigEndian system which is useful for usb
> driver development (carl9170 and rtl8192su)... and I would like to
> have out-of-the-box support.
>
>> I think this is reasonable. Rob or Mark, any feedback?
> I recall that Rob has already voiced his opinion about the ahb-burst setting:
> "Also, perhaps you should allow that the compatible string can define the default."
>
> And based on that, I made the "add a default ahb-burst setting for amcc,dwc-otg"
> patch above. Of course, it would be nice to have any feedback too. But unless I
> hear otherwise, I'll continue with posting patches to the dwc2 driver :).

And this is the correct thing to do. Requiring a dtb update is not.

Rob

Re: [PATCH v2 2/4] usb: dwc2: Add binding for AHB burst

[John Youn]

On 11/22/2016 12:51 PM, Christian Lamparter wrote:
> On Monday, November 21, 2016 7:32:30 PM CET John Youn wrote:
>> On 11/21/2016 1:10 PM, Christian Lamparter wrote:
>>> On Monday, November 21, 2016 12:16:31 PM CET John Youn wrote:
>>>> On 11/18/2016 12:18 PM, Christian Lamparter wrote:
>>>>> On Friday, November 18, 2016 8:16:08 AM CET Rob Herring wrote:
>>>>>> Also, perhaps you should allow that the compatible string can define the 
>>>>>> default.
>>>>>>
>>>>> I hoped you would say that :).
>>>>>
>>>>> I've attached a patch (on top of John Youn changes) [...]
>>>>> ---
>>>>> Subject: [PATCH] usb: dwc2: add a default ahb-burst setting for amcc,dwc-otg
>>>>> [...]
>>>>> @@ -1097,6 +1097,22 @@ static const char *const ahb_bursts[] = {
>>>>> +/* [...] */
>>>>> +static const struct of_device_id dwc2_compat_ahb_bursts[] = {
>>>>> +	{
>>>>> +		.compatible = "amcc,dwc-otg",
>>>>> +		.data = (void *) GAHBCFG_HBSTLEN_INCR16,
>>>>> +	},
>>>>> +};
>> [...]
>>>>>> @@ -1107,6 +1123,12 @@ static int dwc2_get_property_ahb_burst(struct dwc2_hsotg *hsotg)
>>>>>  	ret = device_property_read_string(hsotg->dev, "snps,ahb-burst", &str);
>>>>>  	if (ret < 0) {
>>>>> +		const struct of_device_id *match;
>>>>> +
>>>>> +		match = of_match_node(dwc2_compat_ahb_bursts, node);
>>>>> +		if (match)
>>>>> +			ret = (int)match->data;
>>>>> +
>> [...]
>>>> I'd prefer if you use the binding which requires no extra code in
>>>> dwc2.
>>> I'm fine with either option. However it think that this would require
>>> that either Mark or Rob would allow an exception to the "keep existing
>>> dts the way they are) and ack the following change to the canyonlands.dts. 
>>
>> I don't know about that. Under what circumstance can the dts change?
> As far as I know, the justification for not changing the DTS is that a
> compiled DTB might be stored in an read-only ROM on a board. So it would
> be impossible to update it. Hence, the driver have work with the existing
> (and sometimes buggy or incomplete) information to stay compatible.
> 
> (Note: Thankfully, the canyonlands dtb is stored in flash, it's possible
> to update it. But it is an extra step that's not done automatically
> with make install). 
> 
>> The canyonlands dts was binding to an external vendor driver. So it
>> wasn't documented nor expected to work with dwc2 until your recent
>> patch adding the compatible string.
> 
> Oh, no that's not what happend. Let me explain why there was no "external 
> vendor driver": AMCC/APM were planing to upstream their hole platform. And
> in fact, the devs tried very hard to include their driver back in 2011 [0].
> But this driver was denied inclusion back then due to:
> 
> "[...]
> I would also like to point out that the same Synopsys USB controller
> is used in a number of other SoCs (especially ARM chips), and
> supported by other drivers, some of these even in mainline.
> 
> See http://thread.gmane.org/gmane.linux.usb.general/61714/focus=62139
> for a related thread.
> 
> Instead of trying to add a completely new driver to mainline (and one
> which has been repeatedly been rejected), I vote for focusing on the
> existing driver code that is already in mainline, and testing and
> improving this so we can use a single implementation of this driver
> code for all SoCs that use the same IP block." [1]
> 
> Of course: The listed link goes the "USB Host driver for i.MX28" driver.
> And this is an ehci-hcd like driver... Which is as you are well aware not
> that similar to the dwc2 OTG. And as far as I can tell: AMCC abandoned
> the patch series right there. 
> 
> Note: AMCC did however succeed in pushing your employer's Synopsys
> DesignWare SATA and DMA drivers to the kernel back then. And I'm happy
> to report that both drivers are still around and working fine for the 460EX 
> (sata_dwc_460ex.c[2] and the DW AHB DMA [3]). (The drivers also work for
> different platforms than the original PPC. I know that because I helped
> Andy Shevchenko with testing and pushing some fixes to it when he was
> adding support for the Intel Quark SoC, which uses the DWC SATA and DMA).

Ok thanks for clearing that up. I understand.

For now we can just set the property to "INCR16" based on the
compatible string. Perhaps in the future do this from a glue-layer
driver which binds to all compatible strings other than "snps,dwc2".

I won't be able to do anything with this until next week though.

Regards,
John
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/4] usb: dwc2: Add binding for AHB burst

[John Youn]

On 11/22/2016 12:51 PM, Christian Lamparter wrote:
> On Monday, November 21, 2016 7:32:30 PM CET John Youn wrote:
>> On 11/21/2016 1:10 PM, Christian Lamparter wrote:
>>> On Monday, November 21, 2016 12:16:31 PM CET John Youn wrote:
>>>> On 11/18/2016 12:18 PM, Christian Lamparter wrote:
>>>>> On Friday, November 18, 2016 8:16:08 AM CET Rob Herring wrote:
>>>>>> Also, perhaps you should allow that the compatible string can define the 
>>>>>> default.
>>>>>>
>>>>> I hoped you would say that :).
>>>>>
>>>>> I've attached a patch (on top of John Youn changes) [...]
>>>>> ---
>>>>> Subject: [PATCH] usb: dwc2: add a default ahb-burst setting for amcc,dwc-otg
>>>>> [...]
>>>>> @@ -1097,6 +1097,22 @@ static const char *const ahb_bursts[] = {
>>>>> +/* [...] */
>>>>> +static const struct of_device_id dwc2_compat_ahb_bursts[] = {
>>>>> +	{
>>>>> +		.compatible = "amcc,dwc-otg",
>>>>> +		.data = (void *) GAHBCFG_HBSTLEN_INCR16,
>>>>> +	},
>>>>> +};
>> [...]
>>>>>> @@ -1107,6 +1123,12 @@ static int dwc2_get_property_ahb_burst(struct dwc2_hsotg *hsotg)
>>>>>  	ret = device_property_read_string(hsotg->dev, "snps,ahb-burst", &str);
>>>>>  	if (ret < 0) {
>>>>> +		const struct of_device_id *match;
>>>>> +
>>>>> +		match = of_match_node(dwc2_compat_ahb_bursts, node);
>>>>> +		if (match)
>>>>> +			ret = (int)match->data;
>>>>> +
>> [...]
>>>> I'd prefer if you use the binding which requires no extra code in
>>>> dwc2.
>>> I'm fine with either option. However it think that this would require
>>> that either Mark or Rob would allow an exception to the "keep existing
>>> dts the way they are) and ack the following change to the canyonlands.dts. 
>>
>> I don't know about that. Under what circumstance can the dts change?
> As far as I know, the justification for not changing the DTS is that a
> compiled DTB might be stored in an read-only ROM on a board. So it would
> be impossible to update it. Hence, the driver have work with the existing
> (and sometimes buggy or incomplete) information to stay compatible.
> 
> (Note: Thankfully, the canyonlands dtb is stored in flash, it's possible
> to update it. But it is an extra step that's not done automatically
> with make install). 
> 
>> The canyonlands dts was binding to an external vendor driver. So it
>> wasn't documented nor expected to work with dwc2 until your recent
>> patch adding the compatible string.
> 
> Oh, no that's not what happend. Let me explain why there was no "external 
> vendor driver": AMCC/APM were planing to upstream their hole platform. And
> in fact, the devs tried very hard to include their driver back in 2011 [0].
> But this driver was denied inclusion back then due to:
> 
> "[...]
> I would also like to point out that the same Synopsys USB controller
> is used in a number of other SoCs (especially ARM chips), and
> supported by other drivers, some of these even in mainline.
> 
> See http://thread.gmane.org/gmane.linux.usb.general/61714/focus=62139
> for a related thread.
> 
> Instead of trying to add a completely new driver to mainline (and one
> which has been repeatedly been rejected), I vote for focusing on the
> existing driver code that is already in mainline, and testing and
> improving this so we can use a single implementation of this driver
> code for all SoCs that use the same IP block." [1]
> 
> Of course: The listed link goes the "USB Host driver for i.MX28" driver.
> And this is an ehci-hcd like driver... Which is as you are well aware not
> that similar to the dwc2 OTG. And as far as I can tell: AMCC abandoned
> the patch series right there. 
> 
> Note: AMCC did however succeed in pushing your employer's Synopsys
> DesignWare SATA and DMA drivers to the kernel back then. And I'm happy
> to report that both drivers are still around and working fine for the 460EX 
> (sata_dwc_460ex.c[2] and the DW AHB DMA [3]). (The drivers also work for
> different platforms than the original PPC. I know that because I helped
> Andy Shevchenko with testing and pushing some fixes to it when he was
> adding support for the Intel Quark SoC, which uses the DWC SATA and DMA).

Ok thanks for clearing that up. I understand.

For now we can just set the property to "INCR16" based on the
compatible string. Perhaps in the future do this from a glue-layer
driver which binds to all compatible strings other than "snps,dwc2".

I won't be able to do anything with this until next week though.

Regards,
John

Re: [PATCH v2 2/4] usb: dwc2: Add binding for AHB burst

[Christian Lamparter]

Hello John, hello Felipe

On Monday, November 28, 2016 7:32:20 PM CET John Youn wrote:
> On 11/22/2016 12:51 PM, Christian Lamparter wrote:
> > On Monday, November 21, 2016 7:32:30 PM CET John Youn wrote:
> >> On 11/21/2016 1:10 PM, Christian Lamparter wrote:
> >>> On Monday, November 21, 2016 12:16:31 PM CET John Youn wrote:
> >>>> On 11/18/2016 12:18 PM, Christian Lamparter wrote:
> >>>>> On Friday, November 18, 2016 8:16:08 AM CET Rob Herring wrote:
> >>>>>> Also, perhaps you should allow that the compatible string can define the 
> >>>>>> default.
> >>>>>>
> >>>>> I hoped you would say that :).
> >>>>>
> >>>>> I've attached a patch (on top of John Youn changes) [...]
> >>>>> ---
> >>>>> Subject: [PATCH] usb: dwc2: add a default ahb-burst setting for amcc,dwc-otg
> >>>>> [...]
> >>>>> @@ -1097,6 +1097,22 @@ static const char *const ahb_bursts[] = {
> >>>>> +/* [...] */
> >>>>> +static const struct of_device_id dwc2_compat_ahb_bursts[] = {
> >>>>> +	{
> >>>>> +		.compatible = "amcc,dwc-otg",
> >>>>> +		.data = (void *) GAHBCFG_HBSTLEN_INCR16,
> >>>>> +	},
> >>>>> +};
> >> [...]
> >>>>>> @@ -1107,6 +1123,12 @@ static int dwc2_get_property_ahb_burst(struct dwc2_hsotg *hsotg)
> >>>>>  	ret = device_property_read_string(hsotg->dev, "snps,ahb-burst", &str);
> >>>>>  	if (ret < 0) {
> >>>>> +		const struct of_device_id *match;
> >>>>> +
> >>>>> +		match = of_match_node(dwc2_compat_ahb_bursts, node);
> >>>>> +		if (match)
> >>>>> +			ret = (int)match->data;
> >>>>> +
> >> [...]
> >>>> I'd prefer if you use the binding which requires no extra code in
> >>>> dwc2.
> >>> I'm fine with either option. However it think that this would require
> >>> that either Mark or Rob would allow an exception to the "keep existing
> >>> dts the way they are) and ack the following change to the canyonlands.dts. 
> >> [...]
>
> Ok thanks for clearing that up. I understand.
> 
> For now we can just set the property to "INCR16" based on the
> compatible string. Perhaps in the future do this from a glue-layer
> driver which binds to all compatible strings other than "snps,dwc2".
> 
> I won't be able to do anything with this until next week though.
Ok, I think enough time has passed. I would like to see this
patch series (v3 [0]) being queued for 4.11+ together with
"usb: dwc2: add a default ahb-burst setting for amcc,dwc-otg" [1].

Felipe, if you want I can resend the series and add the
"amcc,dwc-otg" patch to it as well. Just let me know what you
prefer here.

Regards,
Christian

[0] <https://www.mail-archive.com/linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org/msg83401.html>
[1] <https://www.spinics.net/lists/linux-usb/msg149663.html>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/4] usb: dwc2: Add binding for AHB burst

[John Youn]

On 12/19/2016 6:49 AM, Christian Lamparter wrote:
> Hello John, hello Felipe
> 
> On Monday, November 28, 2016 7:32:20 PM CET John Youn wrote:
>> On 11/22/2016 12:51 PM, Christian Lamparter wrote:
>>> On Monday, November 21, 2016 7:32:30 PM CET John Youn wrote:
>>>> On 11/21/2016 1:10 PM, Christian Lamparter wrote:
>>>>> On Monday, November 21, 2016 12:16:31 PM CET John Youn wrote:
>>>>>> On 11/18/2016 12:18 PM, Christian Lamparter wrote:
>>>>>>> On Friday, November 18, 2016 8:16:08 AM CET Rob Herring wrote:
>>>>>>>> Also, perhaps you should allow that the compatible string can define the 
>>>>>>>> default.
>>>>>>>>
>>>>>>> I hoped you would say that :).
>>>>>>>
>>>>>>> I've attached a patch (on top of John Youn changes) [...]
>>>>>>> ---
>>>>>>> Subject: [PATCH] usb: dwc2: add a default ahb-burst setting for amcc,dwc-otg
>>>>>>> [...]
>>>>>>> @@ -1097,6 +1097,22 @@ static const char *const ahb_bursts[] = {
>>>>>>> +/* [...] */
>>>>>>> +static const struct of_device_id dwc2_compat_ahb_bursts[] = {
>>>>>>> +	{
>>>>>>> +		.compatible = "amcc,dwc-otg",
>>>>>>> +		.data = (void *) GAHBCFG_HBSTLEN_INCR16,
>>>>>>> +	},
>>>>>>> +};
>>>> [...]
>>>>>>>> @@ -1107,6 +1123,12 @@ static int dwc2_get_property_ahb_burst(struct dwc2_hsotg *hsotg)
>>>>>>>  	ret = device_property_read_string(hsotg->dev, "snps,ahb-burst", &str);
>>>>>>>  	if (ret < 0) {
>>>>>>> +		const struct of_device_id *match;
>>>>>>> +
>>>>>>> +		match = of_match_node(dwc2_compat_ahb_bursts, node);
>>>>>>> +		if (match)
>>>>>>> +			ret = (int)match->data;
>>>>>>> +
>>>> [...]
>>>>>> I'd prefer if you use the binding which requires no extra code in
>>>>>> dwc2.
>>>>> I'm fine with either option. However it think that this would require
>>>>> that either Mark or Rob would allow an exception to the "keep existing
>>>>> dts the way they are) and ack the following change to the canyonlands.dts. 
>>>> [...]
>>
>> Ok thanks for clearing that up. I understand.
>>
>> For now we can just set the property to "INCR16" based on the
>> compatible string. Perhaps in the future do this from a glue-layer
>> driver which binds to all compatible strings other than "snps,dwc2".
>>
>> I won't be able to do anything with this until next week though.
> Ok, I think enough time has passed. I would like to see this
> patch series (v3 [0]) being queued for 4.11+ together with
> "usb: dwc2: add a default ahb-burst setting for amcc,dwc-otg" [1].
> 
> Felipe, if you want I can resend the series and add the
> "amcc,dwc-otg" patch to it as well. Just let me know what you
> prefer here.
> 
> Regards,
> Christian
> 
> [0] <https://urldefense.proofpoint.com/v2/url?u=https-3A__www.mail-2Darchive.com_linux-2Dusb-40vger.kernel.org_msg83401.html&d=DgICAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=0VsY7UdB1d6feXmjAhNrI_qghu91wtaLLnO3I3Uw2os&m=9a_ZbkfhbOdNjPstQXQq2hm157AzXQX03qKZIddHarU&s=zb0Zx7ZwvnKxcbpCJ_RlLrt1JtzhwUm9DffIAn9oR3k&e= >
> [1] <https://urldefense.proofpoint.com/v2/url?u=https-3A__www.spinics.net_lists_linux-2Dusb_msg149663.html&d=DgICAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=0VsY7UdB1d6feXmjAhNrI_qghu91wtaLLnO3I3Uw2os&m=9a_ZbkfhbOdNjPstQXQq2hm157AzXQX03qKZIddHarU&s=23LsbEs2DjUVLrj4ifAU2LCzEi-U3wn1G1Nx9FBIxvw&e= >
> 

Hi Christian,

This should be fixed against the latest dwc2 param rework series [1]
which i hope to get queued for 4.11. If you can give it a test, that
would be great.

Regards,
John

[1] https://www.spinics.net/lists/linux-usb/msg151693.html
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/4] usb: dwc2: Add binding for AHB burst

[Christian Lamparter]

On Tuesday, January 10, 2017 1:46:56 PM CET John Youn wrote:
> On 12/19/2016 6:49 AM, Christian Lamparter wrote:
> > (Lot's of old stuff, that doesn't matter anymore)

Hello John,
 
> This should be fixed against the latest dwc2 param rework series [1]
> which i hope to get queued for 4.11. If you can give it a test, that
> would be great.

oh Ok. I see you added it to
"[PATCH 16/21] usb: dwc2: Remove platform static params" [0].
Yes, I think this should work nicely. Thank you very much for
your time, even though it's just for like "one old board" :-).

Do you have a public git tree with your patches that I can
clone/checkout?  If not, I'll take some time on the weekend
for this and write back on monday. But yeah, this should 
work.

Regards,
Christian

[0] <https://www.spinics.net/lists/linux-usb/msg151709.html>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/4] usb: dwc2: Add binding for AHB burst

[John Youn]

On 1/10/2017 3:03 PM, Christian Lamparter wrote:
> On Tuesday, January 10, 2017 1:46:56 PM CET John Youn wrote:
>> On 12/19/2016 6:49 AM, Christian Lamparter wrote:
>>> (Lot's of old stuff, that doesn't matter anymore)
> 
> Hello John,
>  
>> This should be fixed against the latest dwc2 param rework series [1]
>> which i hope to get queued for 4.11. If you can give it a test, that
>> would be great.
> 
> oh Ok. I see you added it to
> "[PATCH 16/21] usb: dwc2: Remove platform static params" [0].
> Yes, I think this should work nicely. Thank you very much for
> your time, even though it's just for like "one old board" :-).

No problem. Sorry for the delay getting the param stuff sorted.

> 
> Do you have a public git tree with your patches that I can
> clone/checkout?  If not, I'll take some time on the weekend
> for this and write back on monday. But yeah, this should 
> work.

Yes check here on branch 'next'

https://github.com/synopsys-usb/linux.git

Regards,
John
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/4] usb: dwc2: Add binding for AHB burst

[Christian Lamparter]

On Tuesday, January 10, 2017 3:23:24 PM CET John Youn wrote:
> On 1/10/2017 3:03 PM, Christian Lamparter wrote:
> > On Tuesday, January 10, 2017 1:46:56 PM CET John Youn wrote:
> >> On 12/19/2016 6:49 AM, Christian Lamparter wrote:
> >>> (Lot's of old stuff, that doesn't matter anymore)
> > 
> > Hello John,
> >  
> >> This should be fixed against the latest dwc2 param rework series [1]
> >> which i hope to get queued for 4.11. If you can give it a test, that
> >> would be great.
> > 
> > oh Ok. I see you added it to
> > "[PATCH 16/21] usb: dwc2: Remove platform static params" [0].
> > Yes, I think this should work nicely. Thank you very much for
> > your time, even though it's just for like "one old board" :-).
> 
> No problem. Sorry for the delay getting the param stuff sorted.
> 
> > 
> > Do you have a public git tree with your patches that I can
> > clone/checkout?  If not, I'll take some time on the weekend
> > for this and write back on monday. But yeah, this should 
> > work.
> Yes check here on branch 'next'
> 
> https://github.com/synopsys-usb/linux.git
Ok thanks. I cloned it and built a new kernel for the thing. 

>From the (attached) bootlog:
GAHBCFG   @0xD1210008 : 0x0000002E
0x2E = Bit 5 | (Bit 3 | Bit 2 | Bit 1)
     = GAHBCFG_DMA_EN |
       (GAHBCFG_HBSTLEN_INCR16 << GAHBCFG_HBSTLEN_SHIFT)

I've attached an old 1GiB USB-Stick to the dwc2 managed port
and it does work as expected. The same goes for a usb-3.0 HDD
and a USB 2.0 11n WLAN stick. (All while the DWC SATA is
copying data).

Tested-by: Christian Lamparter <chunkeey-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>

Again, thank you for your work!

Regards,
Christian
---
These are the kernel messages.

dwc2 4bff80000.usbotg: mapped PA bff80000 to VA d1210000
dwc2 4bff80000.usbotg: registering common handler for irq35
dwc2 4bff80000.usbotg: Forcing mode to host
dwc2 4bff80000.usbotg: Core Release: 2.90a (snpsid=4f54290a)
dwc2 4bff80000.usbotg: Forcing mode to host
dwc2 4bff80000.usbotg: DWC OTG HCD INIT
dwc2 4bff80000.usbotg: hcfg=00000200
dwc2 4bff80000.usbotg: dwc2_core_init(ca890810)
dwc2 4bff80000.usbotg: HS UTMI+ PHY selected
dwc2 4bff80000.usbotg: Internal DMA Mode
dwc2 4bff80000.usbotg: host_dma:1 dma_desc_enable:0
dwc2 4bff80000.usbotg: Using Buffer DMA mode
dwc2 4bff80000.usbotg: Host Mode
dwc2 4bff80000.usbotg: DWC OTG Controller
dwc2 4bff80000.usbotg: new USB bus registered, assigned bus number 1
dwc2 4bff80000.usbotg: irq 35, io mem 0x00000000
dwc2 4bff80000.usbotg: DWC OTG HCD START
dwc2 4bff80000.usbotg: dwc2_core_host_init(ca890810)
dwc2 4bff80000.usbotg: Initializing HCFG.FSLSPClkSel to 00000000
dwc2 4bff80000.usbotg: initial grxfsiz=00000213
dwc2 4bff80000.usbotg: new grxfsiz=00000213
dwc2 4bff80000.usbotg: initial gnptxfsiz=01000213
dwc2 4bff80000.usbotg: new gnptxfsiz=01000213
dwc2 4bff80000.usbotg: initial hptxfsiz=01000313
dwc2 4bff80000.usbotg: new hptxfsiz=01000313
dwc2 4bff80000.usbotg: dwc2_core_host_init: Halt channel 0
dwc2 4bff80000.usbotg: dwc2_core_host_init: Halt channel 1
dwc2 4bff80000.usbotg: dwc2_core_host_init: Halt channel 2
dwc2 4bff80000.usbotg: dwc2_core_host_init: Halt channel 3
dwc2 4bff80000.usbotg: Init: Port Power? op_state=9
dwc2 4bff80000.usbotg: Init: Power Port (0)
dwc2 4bff80000.usbotg: dwc2_enable_host_interrupts()
dwc2 4bff80000.usbotg: DWC OTG HCD Has Root Hub
dwc2 4bff80000.usbotg: DWC OTG HCD EP RESET: bEndpointAddress=0x81
hub 1-0:1.0: USB hub found
dwc2 4bff80000.usbotg: GetHubDescriptor
hub 1-0:1.0: 1 port detected
dwc2 4bff80000.usbotg: GetHubStatus
dwc2 4bff80000.usbotg: SetPortFeature
dwc2 4bff80000.usbotg: 
dwc2 4bff80000.usbotg: ************************************************************
dwc2 4bff80000.usbotg: HCD State:
dwc2 4bff80000.usbotg:   Num channels: 4
dwc2 4bff80000.usbotg:   Channel 0:
dwc2 4bff80000.usbotg:     dev_addr: 0, ep_num: 0, ep_is_in: 0
dwc2 4bff80000.usbotg:     speed: 0
dwc2 4bff80000.usbotg:     ep_type: 0
dwc2 4bff80000.usbotg:     max_packet: 0
dwc2 4bff80000.usbotg:     data_pid_start: 0
dwc2 4bff80000.usbotg:     multi_count: 0
dwc2 4bff80000.usbotg:     xfer_started: 0
dwc2 4bff80000.usbotg:     xfer_buf:   (null)
dwc2 4bff80000.usbotg:     xfer_dma: 00000000
dwc2 4bff80000.usbotg:     xfer_len: 0
dwc2 4bff80000.usbotg:     xfer_count: 0
dwc2 4bff80000.usbotg:     halt_on_queue: 0
dwc2 4bff80000.usbotg:     halt_pending: 0
dwc2 4bff80000.usbotg:     halt_status: 0
dwc2 4bff80000.usbotg:     do_split: 0
dwc2 4bff80000.usbotg:     complete_split: 0
dwc2 4bff80000.usbotg:     hub_addr: 0
dwc2 4bff80000.usbotg:     hub_port: 0
dwc2 4bff80000.usbotg:     xact_pos: 0
dwc2 4bff80000.usbotg:     requests: 0
dwc2 4bff80000.usbotg:     qh:   (null)
dwc2 4bff80000.usbotg:   Channel 1:
dwc2 4bff80000.usbotg:     dev_addr: 0, ep_num: 0, ep_is_in: 0
dwc2 4bff80000.usbotg:     speed: 0
dwc2 4bff80000.usbotg:     ep_type: 0
dwc2 4bff80000.usbotg:     max_packet: 0
dwc2 4bff80000.usbotg:     data_pid_start: 0
dwc2 4bff80000.usbotg:     multi_count: 0
dwc2 4bff80000.usbotg:     xfer_started: 0
dwc2 4bff80000.usbotg:     xfer_buf:   (null)
dwc2 4bff80000.usbotg:     xfer_dma: 00000000
dwc2 4bff80000.usbotg:     xfer_len: 0
dwc2 4bff80000.usbotg:     xfer_count: 0
dwc2 4bff80000.usbotg:     halt_on_queue: 0
dwc2 4bff80000.usbotg:     halt_pending: 0
dwc2 4bff80000.usbotg:     halt_status: 0
dwc2 4bff80000.usbotg:     do_split: 0
dwc2 4bff80000.usbotg:     complete_split: 0
dwc2 4bff80000.usbotg:     hub_addr: 0
dwc2 4bff80000.usbotg:     hub_port: 0
dwc2 4bff80000.usbotg:     xact_pos: 0
dwc2 4bff80000.usbotg:     requests: 0
dwc2 4bff80000.usbotg:     qh:   (null)
dwc2 4bff80000.usbotg:   Channel 2:
dwc2 4bff80000.usbotg:     dev_addr: 0, ep_num: 0, ep_is_in: 0
dwc2 4bff80000.usbotg:     speed: 0
dwc2 4bff80000.usbotg:     ep_type: 0
dwc2 4bff80000.usbotg:     max_packet: 0
dwc2 4bff80000.usbotg:     data_pid_start: 0
dwc2 4bff80000.usbotg:     multi_count: 0
dwc2 4bff80000.usbotg:     xfer_started: 0
dwc2 4bff80000.usbotg:     xfer_buf:   (null)
dwc2 4bff80000.usbotg:     xfer_dma: 00000000
dwc2 4bff80000.usbotg:     xfer_len: 0
dwc2 4bff80000.usbotg:     xfer_count: 0
dwc2 4bff80000.usbotg:     halt_on_queue: 0
dwc2 4bff80000.usbotg:     halt_pending: 0
dwc2 4bff80000.usbotg:     halt_status: 0
dwc2 4bff80000.usbotg:     do_split: 0
dwc2 4bff80000.usbotg:     complete_split: 0
dwc2 4bff80000.usbotg:     hub_addr: 0
dwc2 4bff80000.usbotg:     hub_port: 0
dwc2 4bff80000.usbotg:     xact_pos: 0
dwc2 4bff80000.usbotg:     requests: 0
dwc2 4bff80000.usbotg:     qh:   (null)
dwc2 4bff80000.usbotg:   Channel 3:
dwc2 4bff80000.usbotg:     dev_addr: 0, ep_num: 0, ep_is_in: 0
dwc2 4bff80000.usbotg:     speed: 0
dwc2 4bff80000.usbotg:     ep_type: 0
dwc2 4bff80000.usbotg:     max_packet: 0
dwc2 4bff80000.usbotg:     data_pid_start: 0
dwc2 4bff80000.usbotg:     multi_count: 0
dwc2 4bff80000.usbotg:     xfer_started: 0
dwc2 4bff80000.usbotg:     xfer_buf:   (null)
dwc2 4bff80000.usbotg:     xfer_dma: 00000000
dwc2 4bff80000.usbotg:     xfer_len: 0
dwc2 4bff80000.usbotg:     xfer_count: 0
dwc2 4bff80000.usbotg:     halt_on_queue: 0
dwc2 4bff80000.usbotg:     halt_pending: 0
dwc2 4bff80000.usbotg:     halt_status: 0
dwc2 4bff80000.usbotg:     do_split: 0
dwc2 4bff80000.usbotg:     complete_split: 0
dwc2 4bff80000.usbotg:     hub_addr: 0
dwc2 4bff80000.usbotg:     hub_port: 0
dwc2 4bff80000.usbotg:     xact_pos: 0
dwc2 4bff80000.usbotg:     requests: 0
dwc2 4bff80000.usbotg:     qh:   (null)
dwc2 4bff80000.usbotg:   non_periodic_channels: 0
dwc2 4bff80000.usbotg:   periodic_channels: 0
dwc2 4bff80000.usbotg:   periodic_usecs: 0
dwc2 4bff80000.usbotg:   NP Tx Req Queue Space Avail: 8
dwc2 4bff80000.usbotg:   NP Tx FIFO Space Avail: 256
dwc2 4bff80000.usbotg:   P Tx Req Queue Space Avail: 8
dwc2 4bff80000.usbotg:   P Tx FIFO Space Avail: 256
dwc2 4bff80000.usbotg: Core Global Registers
dwc2 4bff80000.usbotg: GOTGCTL   @0xD1210000 : 0x001E0001
dwc2 4bff80000.usbotg: GOTGINT   @0xD1210004 : 0x00080000
dwc2 4bff80000.usbotg: GAHBCFG   @0xD1210008 : 0x0000002E
dwc2 4bff80000.usbotg: GUSBCFG   @0xD121000C : 0x20001708
dwc2 4bff80000.usbotg: GRSTCTL   @0xD1210010 : 0x80000000
dwc2 4bff80000.usbotg: GINTSTS   @0xD1210014 : 0x05000025
dwc2 4bff80000.usbotg: GINTMSK   @0xD1210018 : 0xF3000806
dwc2 4bff80000.usbotg: GRXSTSR   @0xD121001C : 0x025B34D8
dwc2 4bff80000.usbotg: GRXFSIZ   @0xD1210024 : 0x00000213
dwc2 4bff80000.usbotg: GNPTXFSIZ         @0xD1210028 : 0x01000213
dwc2 4bff80000.usbotg: GNPTXSTS  @0xD121002C : 0x00080100
dwc2 4bff80000.usbotg: GI2CCTL   @0xD1210030 : 0x00000000
dwc2 4bff80000.usbotg: GPVNDCTL  @0xD1210034 : 0x00000000
dwc2 4bff80000.usbotg: GGPIO     @0xD1210038 : 0x00000000
dwc2 4bff80000.usbotg: GUID      @0xD121003C : 0x00000000
dwc2 4bff80000.usbotg: GSNPSID   @0xD1210040 : 0x4F54290A
dwc2 4bff80000.usbotg: GHWCFG1   @0xD1210044 : 0x00000000
dwc2 4bff80000.usbotg: GHWCFG2   @0xD1210048 : 0x228CC850
dwc2 4bff80000.usbotg: GHWCFG3   @0xD121004C : 0x07FA0CE8
dwc2 4bff80000.usbotg: GHWCFG4   @0xD1210050 : 0x09F04011
dwc2 4bff80000.usbotg: GLPMCFG   @0xD1210054 : 0x00000000
dwc2 4bff80000.usbotg: GPWRDN    @0xD1210058 : 0x00000000
dwc2 4bff80000.usbotg: GDFIFOCFG         @0xD121005C : 0x00000000
dwc2 4bff80000.usbotg: HPTXFSIZ  @0xD1210100 : 0x01000313
dwc2 4bff80000.usbotg: PCGCTL    @0xD1210E00 : 0x00000000
dwc2 4bff80000.usbotg: Host Global Registers
dwc2 4bff80000.usbotg: HCFG      @0xD1210400 : 0x00000200
dwc2 4bff80000.usbotg: HFIR      @0xD1210404 : 0x0000EA60
dwc2 4bff80000.usbotg: HFNUM     @0xD1210408 : 0xEA603FFF
dwc2 4bff80000.usbotg: HPTXSTS   @0xD1210410 : 0x00080100
dwc2 4bff80000.usbotg: HAINT     @0xD1210414 : 0x00000000
dwc2 4bff80000.usbotg: HAINTMSK  @0xD1210418 : 0x00000000
dwc2 4bff80000.usbotg: HPRT0     @0xD1210440 : 0x00021403
dwc2 4bff80000.usbotg: Host Channel 0 Specific Registers
dwc2 4bff80000.usbotg: HCCHAR    @0xD1210500 : 0x00000000
dwc2 4bff80000.usbotg: HCSPLT    @0xD1210504 : 0x00000000
dwc2 4bff80000.usbotg: HCINT     @0xD1210508 : 0x00000002
dwc2 4bff80000.usbotg: HCINTMSK  @0xD121050C : 0x00000000
dwc2 4bff80000.usbotg: HCTSIZ    @0xD1210510 : 0x00000000
dwc2 4bff80000.usbotg: HCDMA     @0xD1210514 : 0x8D289912
dwc2 4bff80000.usbotg: Host Channel 1 Specific Registers
dwc2 4bff80000.usbotg: HCCHAR    @0xD1210520 : 0x00000000
dwc2 4bff80000.usbotg: HCSPLT    @0xD1210524 : 0x00000000
dwc2 4bff80000.usbotg: HCINT     @0xD1210528 : 0x00000002
dwc2 4bff80000.usbotg: HCINTMSK  @0xD121052C : 0x00000000
dwc2 4bff80000.usbotg: HCTSIZ    @0xD1210530 : 0x00000000
dwc2 4bff80000.usbotg: HCDMA     @0xD1210534 : 0xBBC5AD7A
dwc2 4bff80000.usbotg: Host Channel 2 Specific Registers
dwc2 4bff80000.usbotg: HCCHAR    @0xD1210540 : 0x00000000
dwc2 4bff80000.usbotg: HCSPLT    @0xD1210544 : 0x00000000
dwc2 4bff80000.usbotg: HCINT     @0xD1210548 : 0x00000002
dwc2 4bff80000.usbotg: HCINTMSK  @0xD121054C : 0x00000000
dwc2 4bff80000.usbotg: HCTSIZ    @0xD1210550 : 0x00000000
dwc2 4bff80000.usbotg: HCDMA     @0xD1210554 : 0xC7D821B3
dwc2 4bff80000.usbotg: Host Channel 3 Specific Registers
dwc2 4bff80000.usbotg: HCCHAR    @0xD1210560 : 0x00000000
dwc2 4bff80000.usbotg: HCSPLT    @0xD1210564 : 0x00000000
dwc2 4bff80000.usbotg: HCINT     @0xD1210568 : 0x00000002
dwc2 4bff80000.usbotg: HCINTMSK  @0xD121056C : 0x00000000
dwc2 4bff80000.usbotg: HCTSIZ    @0xD1210570 : 0x00000000
dwc2 4bff80000.usbotg: HCDMA     @0xD1210574 : 0x7DEDDB55
dwc2 4bff80000.usbotg: ************************************************************
dwc2 4bff80000.usbotg: 
dwc2 4bff80000.usbotg: gintsts=05000025  gintmsk=f3000806
dwc2 4bff80000.usbotg: ++OTG Interrupt gotgint=80000 [a_host]
dwc2 4bff80000.usbotg:  ++OTG Interrupt: Debounce Done++
dwc2 4bff80000.usbotg: ClearPortFeature USB_PORT_FEAT_C_CONNECTION
root@lede:/tmp# dwc2 4bff80000.usbotg: SetPortFeature
dwc2 4bff80000.usbotg: SetPortFeature - USB_PORT_FEAT_RESET
dwc2 4bff80000.usbotg: In host mode, hprt0=00021501
dwc2 4bff80000.usbotg: gintsts=05000021  gintmsk=f3000806
dwc2 4bff80000.usbotg: ClearPortFeature USB_PORT_FEAT_C_RESET
usb 1-1: new high-speed USB device number 2 using dwc2
dwc2 4bff80000.usbotg: SetPortFeature
dwc2 4bff80000.usbotg: SetPortFeature - USB_PORT_FEAT_RESET
dwc2 4bff80000.usbotg: In host mode, hprt0=00001101
dwc2 4bff80000.usbotg: gintsts=05000029  gintmsk=f3000806
dwc2 4bff80000.usbotg: gintsts=05000029  gintmsk=f3000806
dwc2 4bff80000.usbotg: ClearPortFeature USB_PORT_FEAT_C_RESET
dwc2 4bff80000.usbotg: DWC OTG HCD EP DISABLE: bEndpointAddress=0x00, ep->hcpriv=ca80e6c0
dwc2 4bff80000.usbotg: DWC OTG HCD EP DISABLE: bEndpointAddress=0x00, ep->hcpriv=  (null)
dwc2 4bff80000.usbotg: DWC OTG HCD EP RESET: bEndpointAddress=0x00
dwc2 4bff80000.usbotg: DWC OTG HCD HUB STATUS DATA: Root port status changed
dwc2 4bff80000.usbotg:   port_connect_status_change: 0
dwc2 4bff80000.usbotg:   port_reset_change: 0
dwc2 4bff80000.usbotg:   port_enable_change: 1
dwc2 4bff80000.usbotg:   port_suspend_change: 0
dwc2 4bff80000.usbotg:   port_over_current_change: 0
dwc2 4bff80000.usbotg: DWC OTG HCD EP RESET: bEndpointAddress=0x81
dwc2 4bff80000.usbotg: DWC OTG HCD EP RESET: bEndpointAddress=0x02
usb-storage 1-1:1.0: USB Mass Storage device detected
scsi host2: usb-storage 1-1:1.0
dwc2 4bff80000.usbotg: ClearPortFeature USB_PORT_FEAT_C_ENABLE
scsi 2:0:0:0: Direct-Access              FlashPen Fancy   1100 PQ: 0 ANSI: 0 CCS
sd 2:0:0:0: [sda] 1957888 512-byte logical blocks: (1.00 GB/956 MiB)
sd 2:0:0:0: [sda] Write Protect is off
sd 2:0:0:0: [sda] Mode Sense: 43 00 00 00
sd 2:0:0:0: [sda] No Caching mode page found
sd 2:0:0:0: [sda] Assuming drive cache: write through
 sda: sda1
sd 2:0:0:0: [sda] Attached SCSI removable disk
random: crng init done


--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/4] usb: dwc2: Add binding for AHB burst

[Rob Herring]

On Wed, Nov 16, 2016 at 03:47:18PM -0800, John Youn wrote:
> Add the "snps,ahb-burst" binding and read it in.
> 
> This property controls which burst type to perform on the AHB bus as a
> master in internal DMA mode. This overrides the legacy param value,
> which we need to keep around for now since several platforms use it.
> 
> Some platforms may see better or worse performance based on this
> value. The HAPS platform is one example where all INCRx have worse
> performance than INCR.
> 
> Other platforms (such as the Canyonlands board) report that the default
> value causes system hangs.
> 
> Signed-off-by: John Youn <johnyoun-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
> Cc: Christian Lamparter <chunkeey-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/usb/dwc2.txt |  2 +

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

>  drivers/usb/dwc2/core.h                        |  9 +++++
>  drivers/usb/dwc2/params.c                      | 56 ++++++++++++++++++++++++++
>  3 files changed, 67 insertions(+)
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html