[PATCH 1/2] usb: dwc2: add amcc,dwc-otg support

[PATCH 1/2] usb: dwc2: add amcc,dwc-otg support

[Christian Lamparter]

This patch adds support for the "amcc,usb-otg" device
which is found in the PowerPC Canyonlands' dts.

The device definition was added by:
commit c89b3458d8cc ("powerpc/44x: Add USB DWC DTS entry to Canyonlands board")'
but without any driver support as the dwc2 driver wasn't
available at that time.

Note: The system can't use the generic "snps,dwc2" compatible
because of the special ahbcfg configuration. The default
GAHBCFG_HBSTLEN_INCR4 of snps,dwc2 can cause a system hang
when the USB and SATA is used concurrently.

Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
Cc: John Youn <johnyoun@synopsys.com>
Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
---
v1->v2
	- moved definitons to params.c
	- removed dma_enable / host_dma parameter
	- added dma_desc_fs_enable parameter
---
 Documentation/devicetree/bindings/usb/dwc2.txt |  1 +
 drivers/usb/dwc2/params.c                      | 33 ++++++++++++++++++++++++++
 2 files changed, 34 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt b/Documentation/devicetree/bindings/usb/dwc2.txt
index 10a2a4b..6ccfe85 100644
--- a/Documentation/devicetree/bindings/usb/dwc2.txt
+++ b/Documentation/devicetree/bindings/usb/dwc2.txt
@@ -12,6 +12,7 @@ Required properties:
   - "lantiq,xrx200-usb": The DWC2 USB controller instance in Lantiq XRX SoCs;
   - "amlogic,meson8b-usb": The DWC2 USB controller instance in Amlogic Meson8b SoCs;
   - "amlogic,meson-gxbb-usb": The DWC2 USB controller instance in Amlogic S905 SoCs;
+  - "amcc,dwc-otg": The DWC2 USB controller instance in AMCC Canyonlands 460EX SoCs;
   - snps,dwc2: A generic DWC2 USB controller with default parameters.
 - reg : Should contain 1 register range (address and length)
 - interrupts : Should contain 1 interrupt
diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
index 64d5c66..5d822c5 100644
--- a/drivers/usb/dwc2/params.c
+++ b/drivers/usb/dwc2/params.c
@@ -192,6 +192,38 @@ static const struct dwc2_core_params params_amlogic = {
 	.hibernation			= -1,
 };
 
+static const struct dwc2_core_params params_amcc_dwc_otg = {
+	.otg_cap			= DWC2_CAP_PARAM_HNP_SRP_CAPABLE,
+	.otg_ver			= -1,
+	.dma_desc_enable		= -1,
+	.dma_desc_fs_enable		= -1,
+	.speed				= -1,
+	.enable_dynamic_fifo		= -1,
+	.en_multiple_tx_fifo		= -1,
+	.host_rx_fifo_size		= -1,
+	.host_nperio_tx_fifo_size	= -1,
+	.host_perio_tx_fifo_size	= -1,
+	.max_transfer_size		= -1,
+	.max_packet_count		= -1,
+	.host_channels			= -1,
+	.phy_type			= -1,
+	.phy_utmi_width			= -1,
+	.phy_ulpi_ddr			= -1,
+	.phy_ulpi_ext_vbus		= -1,
+	.i2c_enable			= -1,
+	.ulpi_fs_ls			= -1,
+	.host_support_fs_ls_low_power	= -1,
+	.host_ls_low_power_phy_clk	= -1,
+	.ts_dline			= -1,
+	.reload_ctl			= -1,
+	/* Avoid system hang during concurrently using USB and SATA */
+	.ahbcfg				= GAHBCFG_HBSTLEN_INCR16 <<
+					  GAHBCFG_HBSTLEN_SHIFT,
+	.uframe_sched			= -1,
+	.external_id_pin_ctl		= -1,
+	.hibernation			= -1,
+};
+
 static const struct dwc2_core_params params_default = {
 	.otg_cap			= -1,
 	.otg_ver			= -1,
@@ -239,6 +271,7 @@ const struct of_device_id dwc2_of_match_table[] = {
 	{ .compatible = "samsung,s3c6400-hsotg", .data = NULL},
 	{ .compatible = "amlogic,meson8b-usb", .data = &params_amlogic },
 	{ .compatible = "amlogic,meson-gxbb-usb", .data = &params_amlogic },
+	{ .compatible = "amcc,dwc-otg", .data = &params_amcc_dwc_otg },
 	{},
 };
 MODULE_DEVICE_TABLE(of, dwc2_of_match_table);
-- 
2.10.2

[PATCH 2/2] usb: dwc2: fixes host_dma logic

[Christian Lamparter]

This patch moves the the host_dma initialization
before dwc2_set_param_dma_desc_enable and
dwc2_set_param_dma_desc_fs_enable. The reason being
that both function need it.

Fixes: 1205489cee75bf39 ("usb: dwc2: Get host DMA device properties")

Cc: John Youn <johnyoun@synopsys.com>
Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
---
 drivers/usb/dwc2/params.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
index 5d822c5..222a83c 100644
--- a/drivers/usb/dwc2/params.c
+++ b/drivers/usb/dwc2/params.c
@@ -1157,9 +1157,6 @@ static void dwc2_set_parameters(struct dwc2_hsotg *hsotg,
 	bool dma_capable = !(hw->arch == GHWCFG2_SLAVE_ONLY_ARCH);
 
 	dwc2_set_param_otg_cap(hsotg, params->otg_cap);
-	dwc2_set_param_dma_desc_enable(hsotg, params->dma_desc_enable);
-	dwc2_set_param_dma_desc_fs_enable(hsotg, params->dma_desc_fs_enable);
-
 	if ((hsotg->dr_mode == USB_DR_MODE_HOST) ||
 	    (hsotg->dr_mode == USB_DR_MODE_OTG)) {
 		bool disable;
@@ -1174,6 +1171,8 @@ static void dwc2_set_parameters(struct dwc2_hsotg *hsotg,
 				    !disable, false,
 				    dma_capable);
 	}
+	dwc2_set_param_dma_desc_enable(hsotg, params->dma_desc_enable);
+	dwc2_set_param_dma_desc_fs_enable(hsotg, params->dma_desc_fs_enable);
 
 	dwc2_set_param_host_support_fs_ls_low_power(hsotg,
 			params->host_support_fs_ls_low_power);
-- 
2.10.2

[PATCH 2/2] usb: dwc2: fixes host_dma logic

[Christian Lamparter]

This patch moves the the host_dma initialization
before dwc2_set_param_dma_desc_enable and
dwc2_set_param_dma_desc_fs_enable. The reason being
that both function need it.

Fixes: 1205489cee75bf39 ("usb: dwc2: Get host DMA device properties")

Cc: John Youn <johnyoun-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
Cc: Felipe Balbi <felipe.balbi-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Signed-off-by: Christian Lamparter <chunkeey-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/usb/dwc2/params.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
index 5d822c5..222a83c 100644
--- a/drivers/usb/dwc2/params.c
+++ b/drivers/usb/dwc2/params.c
@@ -1157,9 +1157,6 @@ static void dwc2_set_parameters(struct dwc2_hsotg *hsotg,
 	bool dma_capable = !(hw->arch == GHWCFG2_SLAVE_ONLY_ARCH);
 
 	dwc2_set_param_otg_cap(hsotg, params->otg_cap);
-	dwc2_set_param_dma_desc_enable(hsotg, params->dma_desc_enable);
-	dwc2_set_param_dma_desc_fs_enable(hsotg, params->dma_desc_fs_enable);
-
 	if ((hsotg->dr_mode == USB_DR_MODE_HOST) ||
 	    (hsotg->dr_mode == USB_DR_MODE_OTG)) {
 		bool disable;
@@ -1174,6 +1171,8 @@ static void dwc2_set_parameters(struct dwc2_hsotg *hsotg,
 				    !disable, false,
 				    dma_capable);
 	}
+	dwc2_set_param_dma_desc_enable(hsotg, params->dma_desc_enable);
+	dwc2_set_param_dma_desc_fs_enable(hsotg, params->dma_desc_fs_enable);
 
 	dwc2_set_param_host_support_fs_ls_low_power(hsotg,
 			params->host_support_fs_ls_low_power);
-- 
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 1/2] usb: dwc2: add amcc,dwc-otg support

[John Youn]

On 11/11/2016 12:59 PM, Christian Lamparter wrote:
> This patch adds support for the "amcc,usb-otg" device
> which is found in the PowerPC Canyonlands' dts.
> 
> The device definition was added by:
> commit c89b3458d8cc ("powerpc/44x: Add USB DWC DTS entry to Canyonlands board")'
> but without any driver support as the dwc2 driver wasn't
> available at that time.
> 
> Note: The system can't use the generic "snps,dwc2" compatible
> because of the special ahbcfg configuration. The default
> GAHBCFG_HBSTLEN_INCR4 of snps,dwc2 can cause a system hang
> when the USB and SATA is used concurrently.

Hi,

I don't want to add any more of these param structures to the driver
unless really necessary. We're trying to remove usage of them in favor
of using auto-detected defaults and device properties to override
them.

The AHB Burst is actually one of the ones we were going to do next
because our platform also doesn't work well with INCR4. In fact I'm
thinking of making the default INCR.

If that's all you need then a devicetree binding should be enough
right?

Regards,
John

Re: [PATCH 1/2] usb: dwc2: add amcc,dwc-otg support

[Christian Lamparter]

Hello,

On Friday, November 11, 2016 1:22:16 PM CET John Youn wrote:
> On 11/11/2016 12:59 PM, Christian Lamparter wrote:
> > This patch adds support for the "amcc,usb-otg" device
> > which is found in the PowerPC Canyonlands' dts.
> > 
> > The device definition was added by:
> > commit c89b3458d8cc ("powerpc/44x: Add USB DWC DTS entry to Canyonlands board")'
> > but without any driver support as the dwc2 driver wasn't
> > available at that time.
> > 
> > Note: The system can't use the generic "snps,dwc2" compatible
> > because of the special ahbcfg configuration. The default
> > GAHBCFG_HBSTLEN_INCR4 of snps,dwc2 can cause a system hang
> > when the USB and SATA is used concurrently.
> 
> I don't want to add any more of these param structures to the driver
> unless really necessary. We're trying to remove usage of them in favor
> of using auto-detected defaults and device properties to override
> them.
Ok, thanks. I think that would work. I've attached an updated patch.
Can it be applied/queued now? Or do you want me to resent it later?

> The AHB Burst is actually one of the ones we were going to do next
> because our platform also doesn't work well with INCR4. In fact I'm
> thinking of making the default INCR.
Is that actually possible to change the default still? This would
require to re-evaluate all existing archs/platforms that use 
"snps,dwc2" for INCR16 compatibility. 

>From what I can tell based would be:
bcm11351, bcm21664, bcm23550, exynos3250, stm32f429, rk3xxx,
stratix10, meson-gxbb, rt3050 and some Altera FPGAs.

> If that's all you need then a devicetree binding should be enough
> right?
Yes. The device is working fine so far.

Regards,
Christian

---
>From 70dd4be016b89655a56bc8260f04683b50f07644 Mon Sep 17 00:00:00 2001
From: Christian Lamparter <chunkeey@gmail.com>
Date: Sun, 6 Nov 2016 00:39:24 +0100
Subject: [PATCH] usb: dwc2: add amcc,dwc-otg support

This patch adds support for the "amcc,usb-otg" device
which is found in the PowerPC Canyonlands' dts.

The device definition was added by:
commit c89b3458d8cc ("powerpc/44x: Add USB DWC DTS entry to Canyonlands board")'
but without any driver support as the dwc2 driver wasn't
available at that time.

Note: The system can't use the generic "snps,dwc2" compatible
because of the special ahbcfg configuration. The default
GAHBCFG_HBSTLEN_INCR4 of snps,dwc2 can cause a system hang
when the USB and SATA is used concurrently.

Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
Cc: John Youn <johnyoun@synopsys.com>
Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
---
v1->v2:
	- moved definitons to params.c
	- removed dma_enable / host_dma parameter
	- added dma_desc_fs_enable parameter
v2->v3:
	- removed parameters

Please queue this patch until GAHBCFG_HBSTLEN_INCR16 is the default
for ahbcfg.
---
 Documentation/devicetree/bindings/usb/dwc2.txt | 1 +
 drivers/usb/dwc2/params.c                      | 1 +
 2 files changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt b/Documentation/devicetree/bindings/usb/dwc2.txt
index 10a2a4b..6ccfe85 100644
--- a/Documentation/devicetree/bindings/usb/dwc2.txt
+++ b/Documentation/devicetree/bindings/usb/dwc2.txt
@@ -12,6 +12,7 @@ Required properties:
   - "lantiq,xrx200-usb": The DWC2 USB controller instance in Lantiq XRX SoCs;
   - "amlogic,meson8b-usb": The DWC2 USB controller instance in Amlogic Meson8b SoCs;
   - "amlogic,meson-gxbb-usb": The DWC2 USB controller instance in Amlogic S905 SoCs;
+  - "amcc,dwc-otg": The DWC2 USB controller instance in AMCC Canyonlands 460EX SoCs;
   - snps,dwc2: A generic DWC2 USB controller with default parameters.
 - reg : Should contain 1 register range (address and length)
 - interrupts : Should contain 1 interrupt
diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
index 64d5c66..9506ab0 100644
--- a/drivers/usb/dwc2/params.c
+++ b/drivers/usb/dwc2/params.c
@@ -239,6 +239,7 @@ const struct of_device_id dwc2_of_match_table[] = {
 	{ .compatible = "samsung,s3c6400-hsotg", .data = NULL},
 	{ .compatible = "amlogic,meson8b-usb", .data = &params_amlogic },
 	{ .compatible = "amlogic,meson-gxbb-usb", .data = &params_amlogic },
+	{ .compatible = "amcc,dwc-otg", .data = NULL },
 	{},
 };
 MODULE_DEVICE_TABLE(of, dwc2_of_match_table);
-- 
2.10.2

Re: [PATCH 1/2] usb: dwc2: add amcc,dwc-otg support

[Christian Lamparter]

Hello,

On Friday, November 11, 2016 1:22:16 PM CET John Youn wrote:
> On 11/11/2016 12:59 PM, Christian Lamparter wrote:
> > This patch adds support for the "amcc,usb-otg" device
> > which is found in the PowerPC Canyonlands' dts.
> > 
> > The device definition was added by:
> > commit c89b3458d8cc ("powerpc/44x: Add USB DWC DTS entry to Canyonlands board")'
> > but without any driver support as the dwc2 driver wasn't
> > available at that time.
> > 
> > Note: The system can't use the generic "snps,dwc2" compatible
> > because of the special ahbcfg configuration. The default
> > GAHBCFG_HBSTLEN_INCR4 of snps,dwc2 can cause a system hang
> > when the USB and SATA is used concurrently.
> 
> I don't want to add any more of these param structures to the driver
> unless really necessary. We're trying to remove usage of them in favor
> of using auto-detected defaults and device properties to override
> them.
Ok, thanks. I think that would work. I've attached an updated patch.
Can it be applied/queued now? Or do you want me to resent it later?

> The AHB Burst is actually one of the ones we were going to do next
> because our platform also doesn't work well with INCR4. In fact I'm
> thinking of making the default INCR.
Is that actually possible to change the default still? This would
require to re-evaluate all existing archs/platforms that use 
"snps,dwc2" for INCR16 compatibility. 

>From what I can tell based would be:
bcm11351, bcm21664, bcm23550, exynos3250, stm32f429, rk3xxx,
stratix10, meson-gxbb, rt3050 and some Altera FPGAs.

> If that's all you need then a devicetree binding should be enough
> right?
Yes. The device is working fine so far.

Regards,
Christian

---
>From 70dd4be016b89655a56bc8260f04683b50f07644 Mon Sep 17 00:00:00 2001
From: Christian Lamparter <chunkeey-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Date: Sun, 6 Nov 2016 00:39:24 +0100
Subject: [PATCH] usb: dwc2: add amcc,dwc-otg support

This patch adds support for the "amcc,usb-otg" device
which is found in the PowerPC Canyonlands' dts.

The device definition was added by:
commit c89b3458d8cc ("powerpc/44x: Add USB DWC DTS entry to Canyonlands board")'
but without any driver support as the dwc2 driver wasn't
available at that time.

Note: The system can't use the generic "snps,dwc2" compatible
because of the special ahbcfg configuration. The default
GAHBCFG_HBSTLEN_INCR4 of snps,dwc2 can cause a system hang
when the USB and SATA is used concurrently.

Cc: Felipe Balbi <felipe.balbi-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Cc: John Youn <johnyoun-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
Signed-off-by: Christian Lamparter <chunkeey-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
v1->v2:
	- moved definitons to params.c
	- removed dma_enable / host_dma parameter
	- added dma_desc_fs_enable parameter
v2->v3:
	- removed parameters

Please queue this patch until GAHBCFG_HBSTLEN_INCR16 is the default
for ahbcfg.
---
 Documentation/devicetree/bindings/usb/dwc2.txt | 1 +
 drivers/usb/dwc2/params.c                      | 1 +
 2 files changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt b/Documentation/devicetree/bindings/usb/dwc2.txt
index 10a2a4b..6ccfe85 100644
--- a/Documentation/devicetree/bindings/usb/dwc2.txt
+++ b/Documentation/devicetree/bindings/usb/dwc2.txt
@@ -12,6 +12,7 @@ Required properties:
   - "lantiq,xrx200-usb": The DWC2 USB controller instance in Lantiq XRX SoCs;
   - "amlogic,meson8b-usb": The DWC2 USB controller instance in Amlogic Meson8b SoCs;
   - "amlogic,meson-gxbb-usb": The DWC2 USB controller instance in Amlogic S905 SoCs;
+  - "amcc,dwc-otg": The DWC2 USB controller instance in AMCC Canyonlands 460EX SoCs;
   - snps,dwc2: A generic DWC2 USB controller with default parameters.
 - reg : Should contain 1 register range (address and length)
 - interrupts : Should contain 1 interrupt
diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
index 64d5c66..9506ab0 100644
--- a/drivers/usb/dwc2/params.c
+++ b/drivers/usb/dwc2/params.c
@@ -239,6 +239,7 @@ const struct of_device_id dwc2_of_match_table[] = {
 	{ .compatible = "samsung,s3c6400-hsotg", .data = NULL},
 	{ .compatible = "amlogic,meson8b-usb", .data = &params_amlogic },
 	{ .compatible = "amlogic,meson-gxbb-usb", .data = &params_amlogic },
+	{ .compatible = "amcc,dwc-otg", .data = NULL },
 	{},
 };
 MODULE_DEVICE_TABLE(of, dwc2_of_match_table);
-- 
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 1/2] usb: dwc2: add amcc,dwc-otg support

[John Youn]

On 11/11/2016 2:05 PM, Christian Lamparter wrote:
> Hello,
> 
> On Friday, November 11, 2016 1:22:16 PM CET John Youn wrote:
>> On 11/11/2016 12:59 PM, Christian Lamparter wrote:
>>> This patch adds support for the "amcc,usb-otg" device
>>> which is found in the PowerPC Canyonlands' dts.
>>>
>>> The device definition was added by:
>>> commit c89b3458d8cc ("powerpc/44x: Add USB DWC DTS entry to Canyonlands board")'
>>> but without any driver support as the dwc2 driver wasn't
>>> available at that time.
>>>
>>> Note: The system can't use the generic "snps,dwc2" compatible
>>> because of the special ahbcfg configuration. The default
>>> GAHBCFG_HBSTLEN_INCR4 of snps,dwc2 can cause a system hang
>>> when the USB and SATA is used concurrently.
>>
>> I don't want to add any more of these param structures to the driver
>> unless really necessary. We're trying to remove usage of them in favor
>> of using auto-detected defaults and device properties to override
>> them.
> Ok, thanks. I think that would work. I've attached an updated patch.
> Can it be applied/queued now? Or do you want me to resent it later?
> 
>> The AHB Burst is actually one of the ones we were going to do next
>> because our platform also doesn't work well with INCR4. In fact I'm
>> thinking of making the default INCR.
> Is that actually possible to change the default still? This would
> require to re-evaluate all existing archs/platforms that use 
> "snps,dwc2" for INCR16 compatibility. 

INCR, not INCR16, but you're right, so we may not change it even
though though INCR is usually the right choice over INCR4.

Anyways, with the binding, can't you just set the compatible string to
snps,dwc2?

Regards,
John


> 
> From what I can tell based would be:
> bcm11351, bcm21664, bcm23550, exynos3250, stm32f429, rk3xxx,
> stratix10, meson-gxbb, rt3050 and some Altera FPGAs.
> 
>> If that's all you need then a devicetree binding should be enough
>> right?
> Yes. The device is working fine so far.
> 
> Regards,
> Christian
> 
> ---
> From 70dd4be016b89655a56bc8260f04683b50f07644 Mon Sep 17 00:00:00 2001
> From: Christian Lamparter <chunkeey@gmail.com>
> Date: Sun, 6 Nov 2016 00:39:24 +0100
> Subject: [PATCH] usb: dwc2: add amcc,dwc-otg support
> 
> This patch adds support for the "amcc,usb-otg" device
> which is found in the PowerPC Canyonlands' dts.
> 
> The device definition was added by:
> commit c89b3458d8cc ("powerpc/44x: Add USB DWC DTS entry to Canyonlands board")'
> but without any driver support as the dwc2 driver wasn't
> available at that time.
> 
> Note: The system can't use the generic "snps,dwc2" compatible
> because of the special ahbcfg configuration. The default
> GAHBCFG_HBSTLEN_INCR4 of snps,dwc2 can cause a system hang
> when the USB and SATA is used concurrently.
> 
> Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
> Cc: John Youn <johnyoun@synopsys.com>
> Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
> ---
> v1->v2:
> 	- moved definitons to params.c
> 	- removed dma_enable / host_dma parameter
> 	- added dma_desc_fs_enable parameter
> v2->v3:
> 	- removed parameters
> 
> Please queue this patch until GAHBCFG_HBSTLEN_INCR16 is the default
> for ahbcfg.
> ---
>  Documentation/devicetree/bindings/usb/dwc2.txt | 1 +
>  drivers/usb/dwc2/params.c                      | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt b/Documentation/devicetree/bindings/usb/dwc2.txt
> index 10a2a4b..6ccfe85 100644
> --- a/Documentation/devicetree/bindings/usb/dwc2.txt
> +++ b/Documentation/devicetree/bindings/usb/dwc2.txt
> @@ -12,6 +12,7 @@ Required properties:
>    - "lantiq,xrx200-usb": The DWC2 USB controller instance in Lantiq XRX SoCs;
>    - "amlogic,meson8b-usb": The DWC2 USB controller instance in Amlogic Meson8b SoCs;
>    - "amlogic,meson-gxbb-usb": The DWC2 USB controller instance in Amlogic S905 SoCs;
> +  - "amcc,dwc-otg": The DWC2 USB controller instance in AMCC Canyonlands 460EX SoCs;
>    - snps,dwc2: A generic DWC2 USB controller with default parameters.
>  - reg : Should contain 1 register range (address and length)
>  - interrupts : Should contain 1 interrupt
> diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
> index 64d5c66..9506ab0 100644
> --- a/drivers/usb/dwc2/params.c
> +++ b/drivers/usb/dwc2/params.c
> @@ -239,6 +239,7 @@ const struct of_device_id dwc2_of_match_table[] = {
>  	{ .compatible = "samsung,s3c6400-hsotg", .data = NULL},
>  	{ .compatible = "amlogic,meson8b-usb", .data = &params_amlogic },
>  	{ .compatible = "amlogic,meson-gxbb-usb", .data = &params_amlogic },
> +	{ .compatible = "amcc,dwc-otg", .data = NULL },
>  	{},
>  };
>  MODULE_DEVICE_TABLE(of, dwc2_of_match_table);
> 

Re: [PATCH 1/2] usb: dwc2: add amcc,dwc-otg support

[Christian Lamparter]

On Friday, November 11, 2016 2:20:42 PM CET John Youn wrote:
> On 11/11/2016 2:05 PM, Christian Lamparter wrote:
> > On Friday, November 11, 2016 1:22:16 PM CET John Youn wrote:
> >> On 11/11/2016 12:59 PM, Christian Lamparter wrote:
> >>> This patch adds support for the "amcc,usb-otg" device
> >>> which is found in the PowerPC Canyonlands' dts.
> >>>
> >>> The device definition was added by:
> >>> commit c89b3458d8cc ("powerpc/44x: Add USB DWC DTS entry to Canyonlands board")'
> >>> but without any driver support as the dwc2 driver wasn't
> >>> available at that time.
> >>>
> >>> Note: The system can't use the generic "snps,dwc2" compatible
> >>> because of the special ahbcfg configuration. The default
> >>> GAHBCFG_HBSTLEN_INCR4 of snps,dwc2 can cause a system hang
> >>> when the USB and SATA is used concurrently.
> >>
> >> I don't want to add any more of these param structures to the driver
> >> unless really necessary. We're trying to remove usage of them in favor
> >> of using auto-detected defaults and device properties to override
> >> them.
> > Ok, thanks. I think that would work. I've attached an updated patch.
> > Can it be applied/queued now? Or do you want me to resent it later?
> > 
> >> The AHB Burst is actually one of the ones we were going to do next
> >> because our platform also doesn't work well with INCR4. In fact I'm
> >> thinking of making the default INCR.
> > Is that actually possible to change the default still? This would
> > require to re-evaluate all existing archs/platforms that use 
> > "snps,dwc2" for INCR16 compatibility. 
> 
> INCR, not INCR16, but you're right, so we may not change it even
> though though INCR is usually the right choice over INCR4.
What about making a device-tree property?

Recommended properties:
 - g-ahb-bursts : specifies the ahb bursts length, should be one of
   "single", "INCRx", "INCR4", "INCR8", or "INCR16". If not specified
   the safer but inefficient "INCR4" is used. The optimal setting is
   "INCRx".

Would this work? If so, I can make a patch over the weekend.
> Anyways, with the binding, can't you just set the compatible string to
> snps,dwc2?

Ah, let me explain. I had a discussion with Mark Rutland and Rob Herring
a while back about device-tree bindings.

They made it very clear to me, that they don't want any generic "catch all
compatible" strings:

"Bindings should be for hardware (either specific device models, or for
classes), and not for Linux drivers. The latter is subject to arbitrary
changes while the former is not, as old hardware continues to exist and
does not change while drivers get completely reworked." [0]

Furthermore, this is an existing binding in kernel's canyonlands.dts [1]
and this binding can't be easily changed. Rob Herring explained this in
the context of the "basic-mmio-gpio" patch [2] when I was editing the dts
to make them work with the changes I made:

"You can't remove the old drivers as they are needed to work with 
old dtbs, so there is no gain.

You would need to match on existing compatibles such as
moxa,moxart-gpio and provide a match data struct that has all the info
you are adding here (e.g. data register offset). Then additionally you
could add "basic-mmio-gpio" (I would drop "basic" part) and the
additional data associated with it. But it has to be new properties,
not changing properties. Changing the reg values doesn't work."

So, for this to work with the existing canyonlands.dts, I need to have
the "amcc,dwc-otg" compatible string.

Of course, it would be great to hear from Rob Herring and/or Mark Rutland
about this case.

Regards,
Christian

[0] <https://patchwork.kernel.org/patch/8976221/>
[1] <http://lxr.free-electrons.com/source/arch/powerpc/boot/dts/canyonlands.dts#L181>
[2] <http://www.spinics.net/lists/devicetree/msg124538.html>

 
> > 
> > From what I can tell based would be:
> > bcm11351, bcm21664, bcm23550, exynos3250, stm32f429, rk3xxx,
> > stratix10, meson-gxbb, rt3050 and some Altera FPGAs.
> > 
> >> If that's all you need then a devicetree binding should be enough
> >> right?
> > Yes. The device is working fine so far.
> > 
> > Regards,
> > Christian
> > 
> > ---
> > From 70dd4be016b89655a56bc8260f04683b50f07644 Mon Sep 17 00:00:00 2001
> > From: Christian Lamparter <chunkeey@gmail.com>
> > Date: Sun, 6 Nov 2016 00:39:24 +0100
> > Subject: [PATCH] usb: dwc2: add amcc,dwc-otg support
> > 
> > This patch adds support for the "amcc,usb-otg" device
> > which is found in the PowerPC Canyonlands' dts.
> > 
> > The device definition was added by:
> > commit c89b3458d8cc ("powerpc/44x: Add USB DWC DTS entry to Canyonlands board")'
> > but without any driver support as the dwc2 driver wasn't
> > available at that time.
> > 
> > Note: The system can't use the generic "snps,dwc2" compatible
> > because of the special ahbcfg configuration. The default
> > GAHBCFG_HBSTLEN_INCR4 of snps,dwc2 can cause a system hang
> > when the USB and SATA is used concurrently.
> > 
> > Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
> > Cc: John Youn <johnyoun@synopsys.com>
> > Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
> > ---
> > v1->v2:
> > 	- moved definitons to params.c
> > 	- removed dma_enable / host_dma parameter
> > 	- added dma_desc_fs_enable parameter
> > v2->v3:
> > 	- removed parameters
> > 
> > Please queue this patch until GAHBCFG_HBSTLEN_INCR16 is the default
> > for ahbcfg.
> > ---
> >  Documentation/devicetree/bindings/usb/dwc2.txt | 1 +
> >  drivers/usb/dwc2/params.c                      | 1 +
> >  2 files changed, 2 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt b/Documentation/devicetree/bindings/usb/dwc2.txt
> > index 10a2a4b..6ccfe85 100644
> > --- a/Documentation/devicetree/bindings/usb/dwc2.txt
> > +++ b/Documentation/devicetree/bindings/usb/dwc2.txt
> > @@ -12,6 +12,7 @@ Required properties:
> >    - "lantiq,xrx200-usb": The DWC2 USB controller instance in Lantiq XRX SoCs;
> >    - "amlogic,meson8b-usb": The DWC2 USB controller instance in Amlogic Meson8b SoCs;
> >    - "amlogic,meson-gxbb-usb": The DWC2 USB controller instance in Amlogic S905 SoCs;
> > +  - "amcc,dwc-otg": The DWC2 USB controller instance in AMCC Canyonlands 460EX SoCs;
> >    - snps,dwc2: A generic DWC2 USB controller with default parameters.
> >  - reg : Should contain 1 register range (address and length)
> >  - interrupts : Should contain 1 interrupt
> > diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
> > index 64d5c66..9506ab0 100644
> > --- a/drivers/usb/dwc2/params.c
> > +++ b/drivers/usb/dwc2/params.c
> > @@ -239,6 +239,7 @@ const struct of_device_id dwc2_of_match_table[] = {
> >  	{ .compatible = "samsung,s3c6400-hsotg", .data = NULL},
> >  	{ .compatible = "amlogic,meson8b-usb", .data = &params_amlogic },
> >  	{ .compatible = "amlogic,meson-gxbb-usb", .data = &params_amlogic },
> > +	{ .compatible = "amcc,dwc-otg", .data = NULL },
> >  	{},
> >  };
> >  MODULE_DEVICE_TABLE(of, dwc2_of_match_table);
> > 
> 
> 

Re: [PATCH 2/2] usb: dwc2: fixes host_dma logic

[John Youn]

On 11/11/2016 12:59 PM, Christian Lamparter wrote:
> This patch moves the the host_dma initialization
> before dwc2_set_param_dma_desc_enable and
> dwc2_set_param_dma_desc_fs_enable. The reason being
> that both function need it.
> 
> Fixes: 1205489cee75bf39 ("usb: dwc2: Get host DMA device properties")

This should probably be omitted since it's only in Felipe's
testing/next.

Otherwise looks good.

Acked-by: John Youn <johnyoun@synopsys.com>

Regards,
John


> 
> Cc: John Youn <johnyoun@synopsys.com>
> Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
> Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
> ---
>  drivers/usb/dwc2/params.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
> index 5d822c5..222a83c 100644
> --- a/drivers/usb/dwc2/params.c
> +++ b/drivers/usb/dwc2/params.c
> @@ -1157,9 +1157,6 @@ static void dwc2_set_parameters(struct dwc2_hsotg *hsotg,
>  	bool dma_capable = !(hw->arch == GHWCFG2_SLAVE_ONLY_ARCH);
>  
>  	dwc2_set_param_otg_cap(hsotg, params->otg_cap);
> -	dwc2_set_param_dma_desc_enable(hsotg, params->dma_desc_enable);
> -	dwc2_set_param_dma_desc_fs_enable(hsotg, params->dma_desc_fs_enable);
> -
>  	if ((hsotg->dr_mode == USB_DR_MODE_HOST) ||
>  	    (hsotg->dr_mode == USB_DR_MODE_OTG)) {
>  		bool disable;
> @@ -1174,6 +1171,8 @@ static void dwc2_set_parameters(struct dwc2_hsotg *hsotg,
>  				    !disable, false,
>  				    dma_capable);
>  	}
> +	dwc2_set_param_dma_desc_enable(hsotg, params->dma_desc_enable);
> +	dwc2_set_param_dma_desc_fs_enable(hsotg, params->dma_desc_fs_enable);
>  
>  	dwc2_set_param_host_support_fs_ls_low_power(hsotg,
>  			params->host_support_fs_ls_low_power);
> 

Re: [PATCH 2/2] usb: dwc2: fixes host_dma logic

[John Youn]

On 11/11/2016 12:59 PM, Christian Lamparter wrote:
> This patch moves the the host_dma initialization
> before dwc2_set_param_dma_desc_enable and
> dwc2_set_param_dma_desc_fs_enable. The reason being
> that both function need it.
> 
> Fixes: 1205489cee75bf39 ("usb: dwc2: Get host DMA device properties")

This should probably be omitted since it's only in Felipe's
testing/next.

Otherwise looks good.

Acked-by: John Youn <johnyoun-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>

Regards,
John


> 
> Cc: John Youn <johnyoun-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
> Cc: Felipe Balbi <felipe.balbi-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> Signed-off-by: Christian Lamparter <chunkeey-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  drivers/usb/dwc2/params.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
> index 5d822c5..222a83c 100644
> --- a/drivers/usb/dwc2/params.c
> +++ b/drivers/usb/dwc2/params.c
> @@ -1157,9 +1157,6 @@ static void dwc2_set_parameters(struct dwc2_hsotg *hsotg,
>  	bool dma_capable = !(hw->arch == GHWCFG2_SLAVE_ONLY_ARCH);
>  
>  	dwc2_set_param_otg_cap(hsotg, params->otg_cap);
> -	dwc2_set_param_dma_desc_enable(hsotg, params->dma_desc_enable);
> -	dwc2_set_param_dma_desc_fs_enable(hsotg, params->dma_desc_fs_enable);
> -
>  	if ((hsotg->dr_mode == USB_DR_MODE_HOST) ||
>  	    (hsotg->dr_mode == USB_DR_MODE_OTG)) {
>  		bool disable;
> @@ -1174,6 +1171,8 @@ static void dwc2_set_parameters(struct dwc2_hsotg *hsotg,
>  				    !disable, false,
>  				    dma_capable);
>  	}
> +	dwc2_set_param_dma_desc_enable(hsotg, params->dma_desc_enable);
> +	dwc2_set_param_dma_desc_fs_enable(hsotg, params->dma_desc_fs_enable);
>  
>  	dwc2_set_param_host_support_fs_ls_low_power(hsotg,
>  			params->host_support_fs_ls_low_power);
> 
--
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 1/2] usb: dwc2: add amcc,dwc-otg support

[John Youn]

On 11/11/2016 3:12 PM, Christian Lamparter wrote:
> On Friday, November 11, 2016 2:20:42 PM CET John Youn wrote:
>> On 11/11/2016 2:05 PM, Christian Lamparter wrote:
>>> On Friday, November 11, 2016 1:22:16 PM CET John Youn wrote:
>>>> On 11/11/2016 12:59 PM, Christian Lamparter wrote:
>>>>> This patch adds support for the "amcc,usb-otg" device
>>>>> which is found in the PowerPC Canyonlands' dts.
>>>>>
>>>>> The device definition was added by:
>>>>> commit c89b3458d8cc ("powerpc/44x: Add USB DWC DTS entry to Canyonlands board")'
>>>>> but without any driver support as the dwc2 driver wasn't
>>>>> available at that time.
>>>>>
>>>>> Note: The system can't use the generic "snps,dwc2" compatible
>>>>> because of the special ahbcfg configuration. The default
>>>>> GAHBCFG_HBSTLEN_INCR4 of snps,dwc2 can cause a system hang
>>>>> when the USB and SATA is used concurrently.
>>>>
>>>> I don't want to add any more of these param structures to the driver
>>>> unless really necessary. We're trying to remove usage of them in favor
>>>> of using auto-detected defaults and device properties to override
>>>> them.
>>> Ok, thanks. I think that would work. I've attached an updated patch.
>>> Can it be applied/queued now? Or do you want me to resent it later?
>>>
>>>> The AHB Burst is actually one of the ones we were going to do next
>>>> because our platform also doesn't work well with INCR4. In fact I'm
>>>> thinking of making the default INCR.
>>> Is that actually possible to change the default still? This would
>>> require to re-evaluate all existing archs/platforms that use 
>>> "snps,dwc2" for INCR16 compatibility. 
>>
>> INCR, not INCR16, but you're right, so we may not change it even
>> though though INCR is usually the right choice over INCR4.
> What about making a device-tree property?

Yes, that's what I meant. I'll send a change for this shortly.

> 
> Recommended properties:
>  - g-ahb-bursts : specifies the ahb bursts length, should be one of
>    "single", "INCRx", "INCR4", "INCR8", or "INCR16". If not specified
>    the safer but inefficient "INCR4" is used. The optimal setting is
>    "INCRx".
> 
> Would this work? If so, I can make a patch over the weekend.
>> Anyways, with the binding, can't you just set the compatible string to
>> snps,dwc2?
> 
> Ah, let me explain. I had a discussion with Mark Rutland and Rob Herring
> a while back about device-tree bindings.
> 
> They made it very clear to me, that they don't want any generic "catch all
> compatible" strings:
> 
> "Bindings should be for hardware (either specific device models, or for
> classes), and not for Linux drivers. The latter is subject to arbitrary
> changes while the former is not, as old hardware continues to exist and
> does not change while drivers get completely reworked." [0]
> 
> Furthermore, this is an existing binding in kernel's canyonlands.dts [1]
> and this binding can't be easily changed. Rob Herring explained this in
> the context of the "basic-mmio-gpio" patch [2] when I was editing the dts
> to make them work with the changes I made:
> 
> "You can't remove the old drivers as they are needed to work with 
> old dtbs, so there is no gain.
> 
> You would need to match on existing compatibles such as
> moxa,moxart-gpio and provide a match data struct that has all the info
> you are adding here (e.g. data register offset). Then additionally you
> could add "basic-mmio-gpio" (I would drop "basic" part) and the
> additional data associated with it. But it has to be new properties,
> not changing properties. Changing the reg values doesn't work."
> 
> So, for this to work with the existing canyonlands.dts, I need to have
> the "amcc,dwc-otg" compatible string.

Ok, if that's the case. But still a bit confused as to what driver was
working with it before since the binding was not defined for dwc2.

> 
> Of course, it would be great to hear from Rob Herring and/or Mark Rutland
> about this case.
> 
> Regards,
> Christian
> 
> [0] <https://patchwork.kernel.org/patch/8976221/>
> [1] <http://lxr.free-electrons.com/source/arch/powerpc/boot/dts/canyonlands.dts#L181>
> [2] <http://www.spinics.net/lists/devicetree/msg124538.html>
> 
>  
>>>
>>> From what I can tell based would be:
>>> bcm11351, bcm21664, bcm23550, exynos3250, stm32f429, rk3xxx,
>>> stratix10, meson-gxbb, rt3050 and some Altera FPGAs.
>>>
>>>> If that's all you need then a devicetree binding should be enough
>>>> right?
>>> Yes. The device is working fine so far.
>>>
>>> Regards,
>>> Christian
>>>
>>> ---
>>> From 70dd4be016b89655a56bc8260f04683b50f07644 Mon Sep 17 00:00:00 2001
>>> From: Christian Lamparter <chunkeey@gmail.com>
>>> Date: Sun, 6 Nov 2016 00:39:24 +0100
>>> Subject: [PATCH] usb: dwc2: add amcc,dwc-otg support
>>>
>>> This patch adds support for the "amcc,usb-otg" device
>>> which is found in the PowerPC Canyonlands' dts.
>>>
>>> The device definition was added by:
>>> commit c89b3458d8cc ("powerpc/44x: Add USB DWC DTS entry to Canyonlands board")'
>>> but without any driver support as the dwc2 driver wasn't
>>> available at that time.
>>>
>>> Note: The system can't use the generic "snps,dwc2" compatible
>>> because of the special ahbcfg configuration. The default
>>> GAHBCFG_HBSTLEN_INCR4 of snps,dwc2 can cause a system hang
>>> when the USB and SATA is used concurrently.
>>>
>>> Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
>>> Cc: John Youn <johnyoun@synopsys.com>
>>> Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
>>> ---
>>> v1->v2:
>>> 	- moved definitons to params.c
>>> 	- removed dma_enable / host_dma parameter
>>> 	- added dma_desc_fs_enable parameter
>>> v2->v3:
>>> 	- removed parameters
>>>
>>> Please queue this patch until GAHBCFG_HBSTLEN_INCR16 is the default
>>> for ahbcfg.
>>> ---
>>>  Documentation/devicetree/bindings/usb/dwc2.txt | 1 +
>>>  drivers/usb/dwc2/params.c                      | 1 +
>>>  2 files changed, 2 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt b/Documentation/devicetree/bindings/usb/dwc2.txt
>>> index 10a2a4b..6ccfe85 100644
>>> --- a/Documentation/devicetree/bindings/usb/dwc2.txt
>>> +++ b/Documentation/devicetree/bindings/usb/dwc2.txt
>>> @@ -12,6 +12,7 @@ Required properties:
>>>    - "lantiq,xrx200-usb": The DWC2 USB controller instance in Lantiq XRX SoCs;
>>>    - "amlogic,meson8b-usb": The DWC2 USB controller instance in Amlogic Meson8b SoCs;
>>>    - "amlogic,meson-gxbb-usb": The DWC2 USB controller instance in Amlogic S905 SoCs;
>>> +  - "amcc,dwc-otg": The DWC2 USB controller instance in AMCC Canyonlands 460EX SoCs;
>>>    - snps,dwc2: A generic DWC2 USB controller with default parameters.
>>>  - reg : Should contain 1 register range (address and length)
>>>  - interrupts : Should contain 1 interrupt
>>> diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
>>> index 64d5c66..9506ab0 100644
>>> --- a/drivers/usb/dwc2/params.c
>>> +++ b/drivers/usb/dwc2/params.c
>>> @@ -239,6 +239,7 @@ const struct of_device_id dwc2_of_match_table[] = {
>>>  	{ .compatible = "samsung,s3c6400-hsotg", .data = NULL},
>>>  	{ .compatible = "amlogic,meson8b-usb", .data = &params_amlogic },
>>>  	{ .compatible = "amlogic,meson-gxbb-usb", .data = &params_amlogic },
>>> +	{ .compatible = "amcc,dwc-otg", .data = NULL },
>>>  	{},
>>>  };
>>>  MODULE_DEVICE_TABLE(of, dwc2_of_match_table);
>>>

For dwc2 part:

Acked-by: John Youn <johnyoun@synopsys.com>

Regards,
John

Re: [PATCH 1/2] usb: dwc2: add amcc,dwc-otg support

[John Youn]

On 11/14/2016 3:00 PM, John Youn wrote:
> On 11/11/2016 3:12 PM, Christian Lamparter wrote:
>> On Friday, November 11, 2016 2:20:42 PM CET John Youn wrote:
>>> On 11/11/2016 2:05 PM, Christian Lamparter wrote:
>>>> On Friday, November 11, 2016 1:22:16 PM CET John Youn wrote:
>>>>> On 11/11/2016 12:59 PM, Christian Lamparter wrote:
>>>>>> This patch adds support for the "amcc,usb-otg" device
>>>>>> which is found in the PowerPC Canyonlands' dts.
>>>>>>
>>>>>> The device definition was added by:
>>>>>> commit c89b3458d8cc ("powerpc/44x: Add USB DWC DTS entry to Canyonlands board")'
>>>>>> but without any driver support as the dwc2 driver wasn't
>>>>>> available at that time.
>>>>>>
>>>>>> Note: The system can't use the generic "snps,dwc2" compatible
>>>>>> because of the special ahbcfg configuration. The default
>>>>>> GAHBCFG_HBSTLEN_INCR4 of snps,dwc2 can cause a system hang
>>>>>> when the USB and SATA is used concurrently.
>>>>>
>>>>> I don't want to add any more of these param structures to the driver
>>>>> unless really necessary. We're trying to remove usage of them in favor
>>>>> of using auto-detected defaults and device properties to override
>>>>> them.
>>>> Ok, thanks. I think that would work. I've attached an updated patch.
>>>> Can it be applied/queued now? Or do you want me to resent it later?
>>>>
>>>>> The AHB Burst is actually one of the ones we were going to do next
>>>>> because our platform also doesn't work well with INCR4. In fact I'm
>>>>> thinking of making the default INCR.
>>>> Is that actually possible to change the default still? This would
>>>> require to re-evaluate all existing archs/platforms that use 
>>>> "snps,dwc2" for INCR16 compatibility. 
>>>
>>> INCR, not INCR16, but you're right, so we may not change it even
>>> though though INCR is usually the right choice over INCR4.
>> What about making a device-tree property?
> 
> Yes, that's what I meant. I'll send a change for this shortly.
> 
>>
>> Recommended properties:
>>  - g-ahb-bursts : specifies the ahb bursts length, should be one of
>>    "single", "INCRx", "INCR4", "INCR8", or "INCR16". If not specified
>>    the safer but inefficient "INCR4" is used. The optimal setting is
>>    "INCRx".
>>
>> Would this work? If so, I can make a patch over the weekend.
>>> Anyways, with the binding, can't you just set the compatible string to
>>> snps,dwc2?
>>
>> Ah, let me explain. I had a discussion with Mark Rutland and Rob Herring
>> a while back about device-tree bindings.
>>
>> They made it very clear to me, that they don't want any generic "catch all
>> compatible" strings:
>>
>> "Bindings should be for hardware (either specific device models, or for
>> classes), and not for Linux drivers. The latter is subject to arbitrary
>> changes while the former is not, as old hardware continues to exist and
>> does not change while drivers get completely reworked." [0]
>>
>> Furthermore, this is an existing binding in kernel's canyonlands.dts [1]
>> and this binding can't be easily changed. Rob Herring explained this in
>> the context of the "basic-mmio-gpio" patch [2] when I was editing the dts
>> to make them work with the changes I made:
>>
>> "You can't remove the old drivers as they are needed to work with 
>> old dtbs, so there is no gain.
>>
>> You would need to match on existing compatibles such as
>> moxa,moxart-gpio and provide a match data struct that has all the info
>> you are adding here (e.g. data register offset). Then additionally you
>> could add "basic-mmio-gpio" (I would drop "basic" part) and the
>> additional data associated with it. But it has to be new properties,
>> not changing properties. Changing the reg values doesn't work."
>>
>> So, for this to work with the existing canyonlands.dts, I need to have
>> the "amcc,dwc-otg" compatible string.
> 
> Ok, if that's the case. But still a bit confused as to what driver was
> working with it before since the binding was not defined for dwc2.
> 
>>
>> Of course, it would be great to hear from Rob Herring and/or Mark Rutland
>> about this case.
>>
>> Regards,
>> Christian
>>
>> [0] <https://patchwork.kernel.org/patch/8976221/>
>> [1] <http://lxr.free-electrons.com/source/arch/powerpc/boot/dts/canyonlands.dts#L181>
>> [2] <http://www.spinics.net/lists/devicetree/msg124538.html>
>>
>>  
>>>>
>>>> From what I can tell based would be:
>>>> bcm11351, bcm21664, bcm23550, exynos3250, stm32f429, rk3xxx,
>>>> stratix10, meson-gxbb, rt3050 and some Altera FPGAs.
>>>>
>>>>> If that's all you need then a devicetree binding should be enough
>>>>> right?
>>>> Yes. The device is working fine so far.
>>>>
>>>> Regards,
>>>> Christian
>>>>
>>>> ---
>>>> From 70dd4be016b89655a56bc8260f04683b50f07644 Mon Sep 17 00:00:00 2001
>>>> From: Christian Lamparter <chunkeey@gmail.com>
>>>> Date: Sun, 6 Nov 2016 00:39:24 +0100
>>>> Subject: [PATCH] usb: dwc2: add amcc,dwc-otg support
>>>>
>>>> This patch adds support for the "amcc,usb-otg" device
>>>> which is found in the PowerPC Canyonlands' dts.
>>>>
>>>> The device definition was added by:
>>>> commit c89b3458d8cc ("powerpc/44x: Add USB DWC DTS entry to Canyonlands board")'
>>>> but without any driver support as the dwc2 driver wasn't
>>>> available at that time.
>>>>
>>>> Note: The system can't use the generic "snps,dwc2" compatible
>>>> because of the special ahbcfg configuration. The default
>>>> GAHBCFG_HBSTLEN_INCR4 of snps,dwc2 can cause a system hang
>>>> when the USB and SATA is used concurrently.
>>>>
>>>> Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
>>>> Cc: John Youn <johnyoun@synopsys.com>
>>>> Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
>>>> ---
>>>> v1->v2:
>>>> 	- moved definitons to params.c
>>>> 	- removed dma_enable / host_dma parameter
>>>> 	- added dma_desc_fs_enable parameter
>>>> v2->v3:
>>>> 	- removed parameters
>>>>
>>>> Please queue this patch until GAHBCFG_HBSTLEN_INCR16 is the default
>>>> for ahbcfg.
>>>> ---
>>>>  Documentation/devicetree/bindings/usb/dwc2.txt | 1 +
>>>>  drivers/usb/dwc2/params.c                      | 1 +
>>>>  2 files changed, 2 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt b/Documentation/devicetree/bindings/usb/dwc2.txt
>>>> index 10a2a4b..6ccfe85 100644
>>>> --- a/Documentation/devicetree/bindings/usb/dwc2.txt
>>>> +++ b/Documentation/devicetree/bindings/usb/dwc2.txt
>>>> @@ -12,6 +12,7 @@ Required properties:
>>>>    - "lantiq,xrx200-usb": The DWC2 USB controller instance in Lantiq XRX SoCs;
>>>>    - "amlogic,meson8b-usb": The DWC2 USB controller instance in Amlogic Meson8b SoCs;
>>>>    - "amlogic,meson-gxbb-usb": The DWC2 USB controller instance in Amlogic S905 SoCs;
>>>> +  - "amcc,dwc-otg": The DWC2 USB controller instance in AMCC Canyonlands 460EX SoCs;
>>>>    - snps,dwc2: A generic DWC2 USB controller with default parameters.
>>>>  - reg : Should contain 1 register range (address and length)
>>>>  - interrupts : Should contain 1 interrupt
>>>> diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
>>>> index 64d5c66..9506ab0 100644
>>>> --- a/drivers/usb/dwc2/params.c
>>>> +++ b/drivers/usb/dwc2/params.c
>>>> @@ -239,6 +239,7 @@ const struct of_device_id dwc2_of_match_table[] = {
>>>>  	{ .compatible = "samsung,s3c6400-hsotg", .data = NULL},
>>>>  	{ .compatible = "amlogic,meson8b-usb", .data = &params_amlogic },
>>>>  	{ .compatible = "amlogic,meson-gxbb-usb", .data = &params_amlogic },
>>>> +	{ .compatible = "amcc,dwc-otg", .data = NULL },
>>>>  	{},
>>>>  };
>>>>  MODULE_DEVICE_TABLE(of, dwc2_of_match_table);
>>>>
> 
> For dwc2 part:
> 
> Acked-by: John Youn <johnyoun@synopsys.com>
> 

Hi Felipe,

Can you drop this from your testing/next?

I meant for the 2nd version to be applied, without the params
structure.

I can send you a clean version to apply later today.

Regards,
John

Re: [PATCH 1/2] usb: dwc2: add amcc,dwc-otg support

[John Youn]

On 11/14/2016 3:00 PM, John Youn wrote:
> On 11/11/2016 3:12 PM, Christian Lamparter wrote:
>> On Friday, November 11, 2016 2:20:42 PM CET John Youn wrote:
>>> On 11/11/2016 2:05 PM, Christian Lamparter wrote:
>>>> On Friday, November 11, 2016 1:22:16 PM CET John Youn wrote:
>>>>> On 11/11/2016 12:59 PM, Christian Lamparter wrote:
>>>>>> This patch adds support for the "amcc,usb-otg" device
>>>>>> which is found in the PowerPC Canyonlands' dts.
>>>>>>
>>>>>> The device definition was added by:
>>>>>> commit c89b3458d8cc ("powerpc/44x: Add USB DWC DTS entry to Canyonlands board")'
>>>>>> but without any driver support as the dwc2 driver wasn't
>>>>>> available at that time.
>>>>>>
>>>>>> Note: The system can't use the generic "snps,dwc2" compatible
>>>>>> because of the special ahbcfg configuration. The default
>>>>>> GAHBCFG_HBSTLEN_INCR4 of snps,dwc2 can cause a system hang
>>>>>> when the USB and SATA is used concurrently.
>>>>>
>>>>> I don't want to add any more of these param structures to the driver
>>>>> unless really necessary. We're trying to remove usage of them in favor
>>>>> of using auto-detected defaults and device properties to override
>>>>> them.
>>>> Ok, thanks. I think that would work. I've attached an updated patch.
>>>> Can it be applied/queued now? Or do you want me to resent it later?
>>>>
>>>>> The AHB Burst is actually one of the ones we were going to do next
>>>>> because our platform also doesn't work well with INCR4. In fact I'm
>>>>> thinking of making the default INCR.
>>>> Is that actually possible to change the default still? This would
>>>> require to re-evaluate all existing archs/platforms that use 
>>>> "snps,dwc2" for INCR16 compatibility. 
>>>
>>> INCR, not INCR16, but you're right, so we may not change it even
>>> though though INCR is usually the right choice over INCR4.
>> What about making a device-tree property?
> 
> Yes, that's what I meant. I'll send a change for this shortly.
> 
>>
>> Recommended properties:
>>  - g-ahb-bursts : specifies the ahb bursts length, should be one of
>>    "single", "INCRx", "INCR4", "INCR8", or "INCR16". If not specified
>>    the safer but inefficient "INCR4" is used. The optimal setting is
>>    "INCRx".
>>
>> Would this work? If so, I can make a patch over the weekend.
>>> Anyways, with the binding, can't you just set the compatible string to
>>> snps,dwc2?
>>
>> Ah, let me explain. I had a discussion with Mark Rutland and Rob Herring
>> a while back about device-tree bindings.
>>
>> They made it very clear to me, that they don't want any generic "catch all
>> compatible" strings:
>>
>> "Bindings should be for hardware (either specific device models, or for
>> classes), and not for Linux drivers. The latter is subject to arbitrary
>> changes while the former is not, as old hardware continues to exist and
>> does not change while drivers get completely reworked." [0]
>>
>> Furthermore, this is an existing binding in kernel's canyonlands.dts [1]
>> and this binding can't be easily changed. Rob Herring explained this in
>> the context of the "basic-mmio-gpio" patch [2] when I was editing the dts
>> to make them work with the changes I made:
>>
>> "You can't remove the old drivers as they are needed to work with 
>> old dtbs, so there is no gain.
>>
>> You would need to match on existing compatibles such as
>> moxa,moxart-gpio and provide a match data struct that has all the info
>> you are adding here (e.g. data register offset). Then additionally you
>> could add "basic-mmio-gpio" (I would drop "basic" part) and the
>> additional data associated with it. But it has to be new properties,
>> not changing properties. Changing the reg values doesn't work."
>>
>> So, for this to work with the existing canyonlands.dts, I need to have
>> the "amcc,dwc-otg" compatible string.
> 
> Ok, if that's the case. But still a bit confused as to what driver was
> working with it before since the binding was not defined for dwc2.
> 
>>
>> Of course, it would be great to hear from Rob Herring and/or Mark Rutland
>> about this case.
>>
>> Regards,
>> Christian
>>
>> [0] <https://patchwork.kernel.org/patch/8976221/>
>> [1] <http://lxr.free-electrons.com/source/arch/powerpc/boot/dts/canyonlands.dts#L181>
>> [2] <http://www.spinics.net/lists/devicetree/msg124538.html>
>>
>>  
>>>>
>>>> From what I can tell based would be:
>>>> bcm11351, bcm21664, bcm23550, exynos3250, stm32f429, rk3xxx,
>>>> stratix10, meson-gxbb, rt3050 and some Altera FPGAs.
>>>>
>>>>> If that's all you need then a devicetree binding should be enough
>>>>> right?
>>>> Yes. The device is working fine so far.
>>>>
>>>> Regards,
>>>> Christian
>>>>
>>>> ---
>>>> From 70dd4be016b89655a56bc8260f04683b50f07644 Mon Sep 17 00:00:00 2001
>>>> From: Christian Lamparter <chunkeey-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>> Date: Sun, 6 Nov 2016 00:39:24 +0100
>>>> Subject: [PATCH] usb: dwc2: add amcc,dwc-otg support
>>>>
>>>> This patch adds support for the "amcc,usb-otg" device
>>>> which is found in the PowerPC Canyonlands' dts.
>>>>
>>>> The device definition was added by:
>>>> commit c89b3458d8cc ("powerpc/44x: Add USB DWC DTS entry to Canyonlands board")'
>>>> but without any driver support as the dwc2 driver wasn't
>>>> available at that time.
>>>>
>>>> Note: The system can't use the generic "snps,dwc2" compatible
>>>> because of the special ahbcfg configuration. The default
>>>> GAHBCFG_HBSTLEN_INCR4 of snps,dwc2 can cause a system hang
>>>> when the USB and SATA is used concurrently.
>>>>
>>>> Cc: Felipe Balbi <felipe.balbi-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
>>>> Cc: John Youn <johnyoun-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
>>>> Signed-off-by: Christian Lamparter <chunkeey-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>> ---
>>>> v1->v2:
>>>> 	- moved definitons to params.c
>>>> 	- removed dma_enable / host_dma parameter
>>>> 	- added dma_desc_fs_enable parameter
>>>> v2->v3:
>>>> 	- removed parameters
>>>>
>>>> Please queue this patch until GAHBCFG_HBSTLEN_INCR16 is the default
>>>> for ahbcfg.
>>>> ---
>>>>  Documentation/devicetree/bindings/usb/dwc2.txt | 1 +
>>>>  drivers/usb/dwc2/params.c                      | 1 +
>>>>  2 files changed, 2 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt b/Documentation/devicetree/bindings/usb/dwc2.txt
>>>> index 10a2a4b..6ccfe85 100644
>>>> --- a/Documentation/devicetree/bindings/usb/dwc2.txt
>>>> +++ b/Documentation/devicetree/bindings/usb/dwc2.txt
>>>> @@ -12,6 +12,7 @@ Required properties:
>>>>    - "lantiq,xrx200-usb": The DWC2 USB controller instance in Lantiq XRX SoCs;
>>>>    - "amlogic,meson8b-usb": The DWC2 USB controller instance in Amlogic Meson8b SoCs;
>>>>    - "amlogic,meson-gxbb-usb": The DWC2 USB controller instance in Amlogic S905 SoCs;
>>>> +  - "amcc,dwc-otg": The DWC2 USB controller instance in AMCC Canyonlands 460EX SoCs;
>>>>    - snps,dwc2: A generic DWC2 USB controller with default parameters.
>>>>  - reg : Should contain 1 register range (address and length)
>>>>  - interrupts : Should contain 1 interrupt
>>>> diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
>>>> index 64d5c66..9506ab0 100644
>>>> --- a/drivers/usb/dwc2/params.c
>>>> +++ b/drivers/usb/dwc2/params.c
>>>> @@ -239,6 +239,7 @@ const struct of_device_id dwc2_of_match_table[] = {
>>>>  	{ .compatible = "samsung,s3c6400-hsotg", .data = NULL},
>>>>  	{ .compatible = "amlogic,meson8b-usb", .data = &params_amlogic },
>>>>  	{ .compatible = "amlogic,meson-gxbb-usb", .data = &params_amlogic },
>>>> +	{ .compatible = "amcc,dwc-otg", .data = NULL },
>>>>  	{},
>>>>  };
>>>>  MODULE_DEVICE_TABLE(of, dwc2_of_match_table);
>>>>
> 
> For dwc2 part:
> 
> Acked-by: John Youn <johnyoun-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
> 

Hi Felipe,

Can you drop this from your testing/next?

I meant for the 2nd version to be applied, without the params
structure.

I can send you a clean version to apply later today.

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 1/2] usb: dwc2: add amcc,dwc-otg support

[Felipe Balbi]

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


Hi, 

John Youn <John.Youn@synopsys.com> writes:
> On 11/14/2016 3:00 PM, John Youn wrote:
>> On 11/11/2016 3:12 PM, Christian Lamparter wrote:
>>> On Friday, November 11, 2016 2:20:42 PM CET John Youn wrote:
>>>> On 11/11/2016 2:05 PM, Christian Lamparter wrote:
>>>>> On Friday, November 11, 2016 1:22:16 PM CET John Youn wrote:
>>>>>> On 11/11/2016 12:59 PM, Christian Lamparter wrote:
>>>>>>> This patch adds support for the "amcc,usb-otg" device
>>>>>>> which is found in the PowerPC Canyonlands' dts.
>>>>>>>
>>>>>>> The device definition was added by:
>>>>>>> commit c89b3458d8cc ("powerpc/44x: Add USB DWC DTS entry to Canyonlands board")'
>>>>>>> but without any driver support as the dwc2 driver wasn't
>>>>>>> available at that time.
>>>>>>>
>>>>>>> Note: The system can't use the generic "snps,dwc2" compatible
>>>>>>> because of the special ahbcfg configuration. The default
>>>>>>> GAHBCFG_HBSTLEN_INCR4 of snps,dwc2 can cause a system hang
>>>>>>> when the USB and SATA is used concurrently.
>>>>>>
>>>>>> I don't want to add any more of these param structures to the driver
>>>>>> unless really necessary. We're trying to remove usage of them in favor
>>>>>> of using auto-detected defaults and device properties to override
>>>>>> them.
>>>>> Ok, thanks. I think that would work. I've attached an updated patch.
>>>>> Can it be applied/queued now? Or do you want me to resent it later?
>>>>>
>>>>>> The AHB Burst is actually one of the ones we were going to do next
>>>>>> because our platform also doesn't work well with INCR4. In fact I'm
>>>>>> thinking of making the default INCR.
>>>>> Is that actually possible to change the default still? This would
>>>>> require to re-evaluate all existing archs/platforms that use 
>>>>> "snps,dwc2" for INCR16 compatibility. 
>>>>
>>>> INCR, not INCR16, but you're right, so we may not change it even
>>>> though though INCR is usually the right choice over INCR4.
>>> What about making a device-tree property?
>> 
>> Yes, that's what I meant. I'll send a change for this shortly.
>> 
>>>
>>> Recommended properties:
>>>  - g-ahb-bursts : specifies the ahb bursts length, should be one of
>>>    "single", "INCRx", "INCR4", "INCR8", or "INCR16". If not specified
>>>    the safer but inefficient "INCR4" is used. The optimal setting is
>>>    "INCRx".
>>>
>>> Would this work? If so, I can make a patch over the weekend.
>>>> Anyways, with the binding, can't you just set the compatible string to
>>>> snps,dwc2?
>>>
>>> Ah, let me explain. I had a discussion with Mark Rutland and Rob Herring
>>> a while back about device-tree bindings.
>>>
>>> They made it very clear to me, that they don't want any generic "catch all
>>> compatible" strings:
>>>
>>> "Bindings should be for hardware (either specific device models, or for
>>> classes), and not for Linux drivers. The latter is subject to arbitrary
>>> changes while the former is not, as old hardware continues to exist and
>>> does not change while drivers get completely reworked." [0]
>>>
>>> Furthermore, this is an existing binding in kernel's canyonlands.dts [1]
>>> and this binding can't be easily changed. Rob Herring explained this in
>>> the context of the "basic-mmio-gpio" patch [2] when I was editing the dts
>>> to make them work with the changes I made:
>>>
>>> "You can't remove the old drivers as they are needed to work with 
>>> old dtbs, so there is no gain.
>>>
>>> You would need to match on existing compatibles such as
>>> moxa,moxart-gpio and provide a match data struct that has all the info
>>> you are adding here (e.g. data register offset). Then additionally you
>>> could add "basic-mmio-gpio" (I would drop "basic" part) and the
>>> additional data associated with it. But it has to be new properties,
>>> not changing properties. Changing the reg values doesn't work."
>>>
>>> So, for this to work with the existing canyonlands.dts, I need to have
>>> the "amcc,dwc-otg" compatible string.
>> 
>> Ok, if that's the case. But still a bit confused as to what driver was
>> working with it before since the binding was not defined for dwc2.
>> 
>>>
>>> Of course, it would be great to hear from Rob Herring and/or Mark Rutland
>>> about this case.
>>>
>>> Regards,
>>> Christian
>>>
>>> [0] <https://patchwork.kernel.org/patch/8976221/>
>>> [1] <http://lxr.free-electrons.com/source/arch/powerpc/boot/dts/canyonlands.dts#L181>
>>> [2] <http://www.spinics.net/lists/devicetree/msg124538.html>
>>>
>>>  
>>>>>
>>>>> From what I can tell based would be:
>>>>> bcm11351, bcm21664, bcm23550, exynos3250, stm32f429, rk3xxx,
>>>>> stratix10, meson-gxbb, rt3050 and some Altera FPGAs.
>>>>>
>>>>>> If that's all you need then a devicetree binding should be enough
>>>>>> right?
>>>>> Yes. The device is working fine so far.
>>>>>
>>>>> Regards,
>>>>> Christian
>>>>>
>>>>> ---
>>>>> From 70dd4be016b89655a56bc8260f04683b50f07644 Mon Sep 17 00:00:00 2001
>>>>> From: Christian Lamparter <chunkeey@gmail.com>
>>>>> Date: Sun, 6 Nov 2016 00:39:24 +0100
>>>>> Subject: [PATCH] usb: dwc2: add amcc,dwc-otg support
>>>>>
>>>>> This patch adds support for the "amcc,usb-otg" device
>>>>> which is found in the PowerPC Canyonlands' dts.
>>>>>
>>>>> The device definition was added by:
>>>>> commit c89b3458d8cc ("powerpc/44x: Add USB DWC DTS entry to Canyonlands board")'
>>>>> but without any driver support as the dwc2 driver wasn't
>>>>> available at that time.
>>>>>
>>>>> Note: The system can't use the generic "snps,dwc2" compatible
>>>>> because of the special ahbcfg configuration. The default
>>>>> GAHBCFG_HBSTLEN_INCR4 of snps,dwc2 can cause a system hang
>>>>> when the USB and SATA is used concurrently.
>>>>>
>>>>> Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
>>>>> Cc: John Youn <johnyoun@synopsys.com>
>>>>> Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
>>>>> ---
>>>>> v1->v2:
>>>>> 	- moved definitons to params.c
>>>>> 	- removed dma_enable / host_dma parameter
>>>>> 	- added dma_desc_fs_enable parameter
>>>>> v2->v3:
>>>>> 	- removed parameters
>>>>>
>>>>> Please queue this patch until GAHBCFG_HBSTLEN_INCR16 is the default
>>>>> for ahbcfg.
>>>>> ---
>>>>>  Documentation/devicetree/bindings/usb/dwc2.txt | 1 +
>>>>>  drivers/usb/dwc2/params.c                      | 1 +
>>>>>  2 files changed, 2 insertions(+)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt b/Documentation/devicetree/bindings/usb/dwc2.txt
>>>>> index 10a2a4b..6ccfe85 100644
>>>>> --- a/Documentation/devicetree/bindings/usb/dwc2.txt
>>>>> +++ b/Documentation/devicetree/bindings/usb/dwc2.txt
>>>>> @@ -12,6 +12,7 @@ Required properties:
>>>>>    - "lantiq,xrx200-usb": The DWC2 USB controller instance in Lantiq XRX SoCs;
>>>>>    - "amlogic,meson8b-usb": The DWC2 USB controller instance in Amlogic Meson8b SoCs;
>>>>>    - "amlogic,meson-gxbb-usb": The DWC2 USB controller instance in Amlogic S905 SoCs;
>>>>> +  - "amcc,dwc-otg": The DWC2 USB controller instance in AMCC Canyonlands 460EX SoCs;
>>>>>    - snps,dwc2: A generic DWC2 USB controller with default parameters.
>>>>>  - reg : Should contain 1 register range (address and length)
>>>>>  - interrupts : Should contain 1 interrupt
>>>>> diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
>>>>> index 64d5c66..9506ab0 100644
>>>>> --- a/drivers/usb/dwc2/params.c
>>>>> +++ b/drivers/usb/dwc2/params.c
>>>>> @@ -239,6 +239,7 @@ const struct of_device_id dwc2_of_match_table[] = {
>>>>>  	{ .compatible = "samsung,s3c6400-hsotg", .data = NULL},
>>>>>  	{ .compatible = "amlogic,meson8b-usb", .data = &params_amlogic },
>>>>>  	{ .compatible = "amlogic,meson-gxbb-usb", .data = &params_amlogic },
>>>>> +	{ .compatible = "amcc,dwc-otg", .data = NULL },
>>>>>  	{},
>>>>>  };
>>>>>  MODULE_DEVICE_TABLE(of, dwc2_of_match_table);
>>>>>
>> 
>> For dwc2 part:
>> 
>> Acked-by: John Youn <johnyoun@synopsys.com>
>> 
>
> Hi Felipe,
>
> Can you drop this from your testing/next?
>
> I meant for the 2nd version to be applied, without the params
> structure.
>
> I can send you a clean version to apply later today.

done

-- 
balbi

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

Re: [PATCH 1/2] usb: dwc2: add amcc,dwc-otg support

[Felipe Balbi]

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


Hi, 

John Youn <John.Youn-HKixBCOQz3hWk0Htik3J/w@public.gmane.org> writes:
> On 11/14/2016 3:00 PM, John Youn wrote:
>> On 11/11/2016 3:12 PM, Christian Lamparter wrote:
>>> On Friday, November 11, 2016 2:20:42 PM CET John Youn wrote:
>>>> On 11/11/2016 2:05 PM, Christian Lamparter wrote:
>>>>> On Friday, November 11, 2016 1:22:16 PM CET John Youn wrote:
>>>>>> On 11/11/2016 12:59 PM, Christian Lamparter wrote:
>>>>>>> This patch adds support for the "amcc,usb-otg" device
>>>>>>> which is found in the PowerPC Canyonlands' dts.
>>>>>>>
>>>>>>> The device definition was added by:
>>>>>>> commit c89b3458d8cc ("powerpc/44x: Add USB DWC DTS entry to Canyonlands board")'
>>>>>>> but without any driver support as the dwc2 driver wasn't
>>>>>>> available at that time.
>>>>>>>
>>>>>>> Note: The system can't use the generic "snps,dwc2" compatible
>>>>>>> because of the special ahbcfg configuration. The default
>>>>>>> GAHBCFG_HBSTLEN_INCR4 of snps,dwc2 can cause a system hang
>>>>>>> when the USB and SATA is used concurrently.
>>>>>>
>>>>>> I don't want to add any more of these param structures to the driver
>>>>>> unless really necessary. We're trying to remove usage of them in favor
>>>>>> of using auto-detected defaults and device properties to override
>>>>>> them.
>>>>> Ok, thanks. I think that would work. I've attached an updated patch.
>>>>> Can it be applied/queued now? Or do you want me to resent it later?
>>>>>
>>>>>> The AHB Burst is actually one of the ones we were going to do next
>>>>>> because our platform also doesn't work well with INCR4. In fact I'm
>>>>>> thinking of making the default INCR.
>>>>> Is that actually possible to change the default still? This would
>>>>> require to re-evaluate all existing archs/platforms that use 
>>>>> "snps,dwc2" for INCR16 compatibility. 
>>>>
>>>> INCR, not INCR16, but you're right, so we may not change it even
>>>> though though INCR is usually the right choice over INCR4.
>>> What about making a device-tree property?
>> 
>> Yes, that's what I meant. I'll send a change for this shortly.
>> 
>>>
>>> Recommended properties:
>>>  - g-ahb-bursts : specifies the ahb bursts length, should be one of
>>>    "single", "INCRx", "INCR4", "INCR8", or "INCR16". If not specified
>>>    the safer but inefficient "INCR4" is used. The optimal setting is
>>>    "INCRx".
>>>
>>> Would this work? If so, I can make a patch over the weekend.
>>>> Anyways, with the binding, can't you just set the compatible string to
>>>> snps,dwc2?
>>>
>>> Ah, let me explain. I had a discussion with Mark Rutland and Rob Herring
>>> a while back about device-tree bindings.
>>>
>>> They made it very clear to me, that they don't want any generic "catch all
>>> compatible" strings:
>>>
>>> "Bindings should be for hardware (either specific device models, or for
>>> classes), and not for Linux drivers. The latter is subject to arbitrary
>>> changes while the former is not, as old hardware continues to exist and
>>> does not change while drivers get completely reworked." [0]
>>>
>>> Furthermore, this is an existing binding in kernel's canyonlands.dts [1]
>>> and this binding can't be easily changed. Rob Herring explained this in
>>> the context of the "basic-mmio-gpio" patch [2] when I was editing the dts
>>> to make them work with the changes I made:
>>>
>>> "You can't remove the old drivers as they are needed to work with 
>>> old dtbs, so there is no gain.
>>>
>>> You would need to match on existing compatibles such as
>>> moxa,moxart-gpio and provide a match data struct that has all the info
>>> you are adding here (e.g. data register offset). Then additionally you
>>> could add "basic-mmio-gpio" (I would drop "basic" part) and the
>>> additional data associated with it. But it has to be new properties,
>>> not changing properties. Changing the reg values doesn't work."
>>>
>>> So, for this to work with the existing canyonlands.dts, I need to have
>>> the "amcc,dwc-otg" compatible string.
>> 
>> Ok, if that's the case. But still a bit confused as to what driver was
>> working with it before since the binding was not defined for dwc2.
>> 
>>>
>>> Of course, it would be great to hear from Rob Herring and/or Mark Rutland
>>> about this case.
>>>
>>> Regards,
>>> Christian
>>>
>>> [0] <https://patchwork.kernel.org/patch/8976221/>
>>> [1] <http://lxr.free-electrons.com/source/arch/powerpc/boot/dts/canyonlands.dts#L181>
>>> [2] <http://www.spinics.net/lists/devicetree/msg124538.html>
>>>
>>>  
>>>>>
>>>>> From what I can tell based would be:
>>>>> bcm11351, bcm21664, bcm23550, exynos3250, stm32f429, rk3xxx,
>>>>> stratix10, meson-gxbb, rt3050 and some Altera FPGAs.
>>>>>
>>>>>> If that's all you need then a devicetree binding should be enough
>>>>>> right?
>>>>> Yes. The device is working fine so far.
>>>>>
>>>>> Regards,
>>>>> Christian
>>>>>
>>>>> ---
>>>>> From 70dd4be016b89655a56bc8260f04683b50f07644 Mon Sep 17 00:00:00 2001
>>>>> From: Christian Lamparter <chunkeey-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>>> Date: Sun, 6 Nov 2016 00:39:24 +0100
>>>>> Subject: [PATCH] usb: dwc2: add amcc,dwc-otg support
>>>>>
>>>>> This patch adds support for the "amcc,usb-otg" device
>>>>> which is found in the PowerPC Canyonlands' dts.
>>>>>
>>>>> The device definition was added by:
>>>>> commit c89b3458d8cc ("powerpc/44x: Add USB DWC DTS entry to Canyonlands board")'
>>>>> but without any driver support as the dwc2 driver wasn't
>>>>> available at that time.
>>>>>
>>>>> Note: The system can't use the generic "snps,dwc2" compatible
>>>>> because of the special ahbcfg configuration. The default
>>>>> GAHBCFG_HBSTLEN_INCR4 of snps,dwc2 can cause a system hang
>>>>> when the USB and SATA is used concurrently.
>>>>>
>>>>> Cc: Felipe Balbi <felipe.balbi-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
>>>>> Cc: John Youn <johnyoun-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
>>>>> Signed-off-by: Christian Lamparter <chunkeey-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>>> ---
>>>>> v1->v2:
>>>>> 	- moved definitons to params.c
>>>>> 	- removed dma_enable / host_dma parameter
>>>>> 	- added dma_desc_fs_enable parameter
>>>>> v2->v3:
>>>>> 	- removed parameters
>>>>>
>>>>> Please queue this patch until GAHBCFG_HBSTLEN_INCR16 is the default
>>>>> for ahbcfg.
>>>>> ---
>>>>>  Documentation/devicetree/bindings/usb/dwc2.txt | 1 +
>>>>>  drivers/usb/dwc2/params.c                      | 1 +
>>>>>  2 files changed, 2 insertions(+)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt b/Documentation/devicetree/bindings/usb/dwc2.txt
>>>>> index 10a2a4b..6ccfe85 100644
>>>>> --- a/Documentation/devicetree/bindings/usb/dwc2.txt
>>>>> +++ b/Documentation/devicetree/bindings/usb/dwc2.txt
>>>>> @@ -12,6 +12,7 @@ Required properties:
>>>>>    - "lantiq,xrx200-usb": The DWC2 USB controller instance in Lantiq XRX SoCs;
>>>>>    - "amlogic,meson8b-usb": The DWC2 USB controller instance in Amlogic Meson8b SoCs;
>>>>>    - "amlogic,meson-gxbb-usb": The DWC2 USB controller instance in Amlogic S905 SoCs;
>>>>> +  - "amcc,dwc-otg": The DWC2 USB controller instance in AMCC Canyonlands 460EX SoCs;
>>>>>    - snps,dwc2: A generic DWC2 USB controller with default parameters.
>>>>>  - reg : Should contain 1 register range (address and length)
>>>>>  - interrupts : Should contain 1 interrupt
>>>>> diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
>>>>> index 64d5c66..9506ab0 100644
>>>>> --- a/drivers/usb/dwc2/params.c
>>>>> +++ b/drivers/usb/dwc2/params.c
>>>>> @@ -239,6 +239,7 @@ const struct of_device_id dwc2_of_match_table[] = {
>>>>>  	{ .compatible = "samsung,s3c6400-hsotg", .data = NULL},
>>>>>  	{ .compatible = "amlogic,meson8b-usb", .data = &params_amlogic },
>>>>>  	{ .compatible = "amlogic,meson-gxbb-usb", .data = &params_amlogic },
>>>>> +	{ .compatible = "amcc,dwc-otg", .data = NULL },
>>>>>  	{},
>>>>>  };
>>>>>  MODULE_DEVICE_TABLE(of, dwc2_of_match_table);
>>>>>
>> 
>> For dwc2 part:
>> 
>> Acked-by: John Youn <johnyoun-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
>> 
>
> Hi Felipe,
>
> Can you drop this from your testing/next?
>
> I meant for the 2nd version to be applied, without the params
> structure.
>
> I can send you a clean version to apply later today.

done

-- 
balbi

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