From: Christian Lamparter <chunkeey@googlemail.com> Date: 2016-11-11 20:59:22
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 <redacted>
Cc: John Youn <redacted>
Signed-off-by: Christian Lamparter <redacted>
---
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(+)
@@ -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
@@ -192,6 +192,38 @@ static const struct dwc2_core_params params_amlogic = {.hibernation=-1,};+staticconststructdwc2_core_paramsparams_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,+};+staticconststructdwc2_core_paramsparams_default={.otg_cap=-1,.otg_ver=-1,
From: Christian Lamparter <chunkeey@googlemail.com> Date: 2016-11-11 20:59:20
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 <redacted>
Cc: Felipe Balbi <redacted>
Signed-off-by: Christian Lamparter <redacted>
---
drivers/usb/dwc2/params.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
From: John Youn <hidden> Date: 2016-11-11 21:22:44
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
From: Christian Lamparter <chunkeey@googlemail.com> Date: 2016-11-11 22:05:42
Hello,
On Friday, November 11, 2016 1:22:16 PM CET John Youn wrote:
On 11/11/2016 12:59 PM, Christian Lamparter wrote:
quoted
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 <redacted>
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 <redacted>
Cc: John Youn <redacted>
Signed-off-by: Christian Lamparter <redacted>
---
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(+)
@@ -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
From: John Youn <hidden> Date: 2016-11-11 22:20:48
On 11/11/2016 2:05 PM, Christian Lamparter wrote:
Hello,
On Friday, November 11, 2016 1:22:16 PM CET John Youn wrote:
quoted
On 11/11/2016 12:59 PM, Christian Lamparter wrote:
quoted
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?
quoted
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
quoted hunk
From what I can tell based would be:
bcm11351, bcm21664, bcm23550, exynos3250, stm32f429, rk3xxx,
stratix10, meson-gxbb, rt3050 and some Altera FPGAs.
quoted
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 <redacted>
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 <redacted>
Cc: John Youn <redacted>
Signed-off-by: Christian Lamparter <redacted>
---
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(+)
@@ -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
From: Christian Lamparter <chunkeey@googlemail.com> Date: 2016-11-11 23:12:37
On Friday, November 11, 2016 2:20:42 PM CET John Youn wrote:
On 11/11/2016 2:05 PM, Christian Lamparter wrote:
quoted
On Friday, November 11, 2016 1:22:16 PM CET John Youn wrote:
quoted
On 11/11/2016 12:59 PM, Christian Lamparter wrote:
quoted
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?
quoted
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>
quoted
From what I can tell based would be:
bcm11351, bcm21664, bcm23550, exynos3250, stm32f429, rk3xxx,
stratix10, meson-gxbb, rt3050 and some Altera FPGAs.
quoted
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 <redacted>
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 <redacted>
Cc: John Youn <redacted>
Signed-off-by: Christian Lamparter <redacted>
---
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(+)
@@ -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
From: John Youn <hidden> Date: 2016-11-14 22:53:22
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 <redacted>
Regards,
John
quoted hunk
Cc: John Youn <redacted>
Cc: Felipe Balbi <redacted>
Signed-off-by: Christian Lamparter <redacted>
---
drivers/usb/dwc2/params.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
From: John Youn <hidden> Date: 2016-11-14 23:00:56
On 11/11/2016 3:12 PM, Christian Lamparter wrote:
On Friday, November 11, 2016 2:20:42 PM CET John Youn wrote:
quoted
On 11/11/2016 2:05 PM, Christian Lamparter wrote:
quoted
On Friday, November 11, 2016 1:22:16 PM CET John Youn wrote:
quoted
On 11/11/2016 12:59 PM, Christian Lamparter wrote:
quoted
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?
quoted
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.
quoted
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.
From what I can tell based would be:
bcm11351, bcm21664, bcm23550, exynos3250, stm32f429, rk3xxx,
stratix10, meson-gxbb, rt3050 and some Altera FPGAs.
quoted
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 <redacted>
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 <redacted>
Cc: John Youn <redacted>
Signed-off-by: Christian Lamparter <redacted>
---
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(+)
@@ -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
From: John Youn <hidden> Date: 2016-11-15 19:08:28
On 11/14/2016 3:00 PM, John Youn wrote:
On 11/11/2016 3:12 PM, Christian Lamparter wrote:
quoted
On Friday, November 11, 2016 2:20:42 PM CET John Youn wrote:
quoted
On 11/11/2016 2:05 PM, Christian Lamparter wrote:
quoted
On Friday, November 11, 2016 1:22:16 PM CET John Youn wrote:
quoted
On 11/11/2016 12:59 PM, Christian Lamparter wrote:
quoted
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?
quoted
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.
quoted
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.
quoted
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.
From what I can tell based would be:
bcm11351, bcm21664, bcm23550, exynos3250, stm32f429, rk3xxx,
stratix10, meson-gxbb, rt3050 and some Altera FPGAs.
quoted
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 <redacted>
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 <redacted>
Cc: John Youn <redacted>
Signed-off-by: Christian Lamparter <redacted>
---
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(+)
@@ -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
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
From: Felipe Balbi <hidden> Date: 2016-11-16 09:25:45
Hi,
John Youn [off-list ref] writes:
On 11/14/2016 3:00 PM, John Youn wrote:
quoted
On 11/11/2016 3:12 PM, Christian Lamparter wrote:
quoted
On Friday, November 11, 2016 2:20:42 PM CET John Youn wrote:
quoted
On 11/11/2016 2:05 PM, Christian Lamparter wrote:
quoted
On Friday, November 11, 2016 1:22:16 PM CET John Youn wrote:
quoted
On 11/11/2016 12:59 PM, Christian Lamparter wrote:
quoted
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?
quoted
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.
quoted
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.
quoted
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.
From what I can tell based would be:
bcm11351, bcm21664, bcm23550, exynos3250, stm32f429, rk3xxx,
stratix10, meson-gxbb, rt3050 and some Altera FPGAs.
quoted
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 <redacted>
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 <redacted>
Cc: John Youn <redacted>
Signed-off-by: Christian Lamparter <redacted>
---
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(+)
@@ -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
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.