[U-Boot] [PATCH v3 0/4] rockchip: rk3288: add fastboot support
This patchset add the fastboot support for rk3288, and I have tested on firefly-rk3288 board.
Fix an issue which was mentioned in V1's cover-letter: The DMA buffer was always zero after DMA transfer is complete, It takes no effect that invalidate dcache after the DMA is complete and before the CPU reads it. It's important to invalidate dcache before starting DMA, ensure coherency and prevent from any dirty lines in the cache which from the DMA buffer.
Summary of changes in this series: - Achieve UOC_CON_OFFSET physical address from DT - Make UOC_CON registers to be unfixed which should be got from DT - Add new commit dd77b11fd44a84 (usb: dwc2: Invalidate dcache before staring DMA)
Changes in v3: - Make UOC_CON registers to be unfixed which should be got from DT - Achieve UOC_CON_OFFSET physical address from DT - New commit since v3 to fix the coherence issue between memory and cache
Changes in v2: - Rename rk3288_usb_phy.c to rockchip_usb_syno_phy.c - Rework the behaviour in otg_phy_init() and otg_phy_off() - Update detailed commit message - Modify the macro's values - Achieve the regs_phy from DT - Update comments a little
Xu Ziyuan (4): usb: rockchip-phy: implement USB2.0 phy control for Synopsys usb: dwc2-otg: re-define fifo-size for Rockchip SoCs rockchip: rk3288: add fastboot support usb: dwc2: invalidate dcache before starting DMA
arch/arm/dts/rk3288.dtsi | 1 + arch/arm/mach-rockchip/board.c | 60 ++++++++++++++++++++++++++++++ drivers/usb/gadget/dwc2_udc_otg_regs.h | 6 +++ drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c | 3 ++ drivers/usb/phy/Makefile | 1 + drivers/usb/phy/rockchip_usb_syno_phy.c | 47 +++++++++++++++++++++++ include/configs/rk3288_common.h | 26 +++++++++++++ 7 files changed, 144 insertions(+) create mode 100644 drivers/usb/phy/rockchip_usb_syno_phy.c
From: Xu Ziyuan xzy.xu@rock-chips.com
So far, Rockchip SoCs have two kinds of USB2.0 phy, like Synopsys and Innosilicon. This patch applys dwc2 usb driver framework to implement phy_init and phy_off for Synopsys phy on Rockchip platform.
Signed-off-by: Ziyuan Xu xzy.xu@rock-chips.com
---
Changes in v3: - Make UOC_CON registers to be unfixed which should be got from DT
Changes in v2: - Rename rk3288_usb_phy.c to rockchip_usb_syno_phy.c - Rework the behaviour in otg_phy_init() and otg_phy_off()
drivers/usb/phy/Makefile | 1 + drivers/usb/phy/rockchip_usb_syno_phy.c | 47 +++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+) create mode 100644 drivers/usb/phy/rockchip_usb_syno_phy.c
diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile index 93d147e..8002a18 100644 --- a/drivers/usb/phy/Makefile +++ b/drivers/usb/phy/Makefile @@ -7,3 +7,4 @@
obj-$(CONFIG_TWL4030_USB) += twl4030.o obj-$(CONFIG_OMAP_USB_PHY) += omap_usb_phy.o +obj-$(CONFIG_ROCKCHIP_USB_SYNO_PHY) += rockchip_usb_syno_phy.o diff --git a/drivers/usb/phy/rockchip_usb_syno_phy.c b/drivers/usb/phy/rockchip_usb_syno_phy.c new file mode 100644 index 0000000..ab049e1 --- /dev/null +++ b/drivers/usb/phy/rockchip_usb_syno_phy.c @@ -0,0 +1,47 @@ +/* + * Copyright 2016 Rockchip Electronics Co., Ltd + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#include <common.h> +#include <asm/io.h> + +#include "../gadget/dwc2_udc_otg_priv.h" + +#define UOC_CON(x) ((x) * 0x4) + +#define RESET_WRITE_ENA BIT(28) +#define PORT_RESET BIT(12) +#define PORT_NORMAL (0 << 12) + +#define SOFT_CTRL_WRITE_ENA BIT(18) +#define SOFT_CTRL_ENABLE BIT(2) + +#define SUSPEND_SETTING 0x2A +#define SUSPEND_WRITE_ENA (0x3f << 16) + + +void otg_phy_init(struct dwc2_udc *dev) +{ + /* disable software control */ + writel(SOFT_CTRL_WRITE_ENA | (0 << 2), + dev->pdata->regs_phy + UOC_CON(2)); + /* reset otg port */ + writel(RESET_WRITE_ENA | PORT_RESET, + dev->pdata->regs_phy + UOC_CON(0)); + mdelay(1); + writel(RESET_WRITE_ENA | PORT_NORMAL, + dev->pdata->regs_phy + UOC_CON(0)); + udelay(1); +} + +void otg_phy_off(struct dwc2_udc *dev) +{ + /* enable software control */ + writel(SOFT_CTRL_WRITE_ENA | SOFT_CTRL_ENABLE, + dev->pdata->regs_phy + UOC_CON(2)); + /* enter suspend */ + writel(SUSPEND_WRITE_ENA | SUSPEND_SETTING, + dev->pdata->regs_phy + UOC_CON(3)); +}
Hi heiko,
On 2016年07月06日 17:34, Ziyuan Xu wrote:
From: Xu Ziyuan xzy.xu@rock-chips.com
So far, Rockchip SoCs have two kinds of USB2.0 phy, like Synopsys and Innosilicon. This patch applys dwc2 usb driver framework to implement phy_init and phy_off for Synopsys phy on Rockchip platform.
Signed-off-by: Ziyuan Xu xzy.xu@rock-chips.com
Changes in v3:
- Make UOC_CON registers to be unfixed which should be got from DT
Changes in v2:
Rename rk3288_usb_phy.c to rockchip_usb_syno_phy.c
Rework the behaviour in otg_phy_init() and otg_phy_off()
drivers/usb/phy/Makefile | 1 + drivers/usb/phy/rockchip_usb_syno_phy.c | 47 +++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+) create mode 100644 drivers/usb/phy/rockchip_usb_syno_phy.c
diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile index 93d147e..8002a18 100644 --- a/drivers/usb/phy/Makefile +++ b/drivers/usb/phy/Makefile @@ -7,3 +7,4 @@
obj-$(CONFIG_TWL4030_USB) += twl4030.o obj-$(CONFIG_OMAP_USB_PHY) += omap_usb_phy.o +obj-$(CONFIG_ROCKCHIP_USB_SYNO_PHY) += rockchip_usb_syno_phy.o diff --git a/drivers/usb/phy/rockchip_usb_syno_phy.c b/drivers/usb/phy/rockchip_usb_syno_phy.c new file mode 100644 index 0000000..ab049e1 --- /dev/null +++ b/drivers/usb/phy/rockchip_usb_syno_phy.c @@ -0,0 +1,47 @@ +/*
- Copyright 2016 Rockchip Electronics Co., Ltd
- SPDX-License-Identifier: GPL-2.0+
- */
+#include <common.h> +#include <asm/io.h>
+#include "../gadget/dwc2_udc_otg_priv.h"
+#define UOC_CON(x) ((x) * 0x4)
As you know, dwc2 usb driver didn't apply devicetree model. It's hard to implement a compatible driver for Rockchip SoCs which use a same udc. For example, SOFT_CTRL bit is UOC_CON2[2] on RK3288 I can't assure it's also UOC_CON[2] on other socs. Do you have any good ideas?
+#define RESET_WRITE_ENA BIT(28) +#define PORT_RESET BIT(12) +#define PORT_NORMAL (0 << 12)
+#define SOFT_CTRL_WRITE_ENA BIT(18) +#define SOFT_CTRL_ENABLE BIT(2)
+#define SUSPEND_SETTING 0x2A +#define SUSPEND_WRITE_ENA (0x3f << 16)
+void otg_phy_init(struct dwc2_udc *dev) +{
- /* disable software control */
- writel(SOFT_CTRL_WRITE_ENA | (0 << 2),
dev->pdata->regs_phy + UOC_CON(2));- /* reset otg port */
- writel(RESET_WRITE_ENA | PORT_RESET,
dev->pdata->regs_phy + UOC_CON(0));- mdelay(1);
- writel(RESET_WRITE_ENA | PORT_NORMAL,
dev->pdata->regs_phy + UOC_CON(0));- udelay(1);
+}
+void otg_phy_off(struct dwc2_udc *dev) +{
- /* enable software control */
- writel(SOFT_CTRL_WRITE_ENA | SOFT_CTRL_ENABLE,
dev->pdata->regs_phy + UOC_CON(2));- /* enter suspend */
- writel(SUSPEND_WRITE_ENA | SUSPEND_SETTING,
dev->pdata->regs_phy + UOC_CON(3));+}
Am Mittwoch, 6. Juli 2016, 18:20:04 schrieb Ziyuan Xu:
Hi heiko,
On 2016年07月06日 17:34, Ziyuan Xu wrote:
From: Xu Ziyuan xzy.xu@rock-chips.com
So far, Rockchip SoCs have two kinds of USB2.0 phy, like Synopsys and Innosilicon. This patch applys dwc2 usb driver framework to implement phy_init and phy_off for Synopsys phy on Rockchip platform.
Signed-off-by: Ziyuan Xu xzy.xu@rock-chips.com
Changes in v3:
- Make UOC_CON registers to be unfixed which should be got from DT
Changes in v2:
Rename rk3288_usb_phy.c to rockchip_usb_syno_phy.c
Rework the behaviour in otg_phy_init() and otg_phy_off()
drivers/usb/phy/Makefile | 1 + drivers/usb/phy/rockchip_usb_syno_phy.c | 47 +++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+) create mode 100644 drivers/usb/phy/rockchip_usb_syno_phy.c
diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile index 93d147e..8002a18 100644 --- a/drivers/usb/phy/Makefile +++ b/drivers/usb/phy/Makefile @@ -7,3 +7,4 @@
obj-$(CONFIG_TWL4030_USB) += twl4030.o obj-$(CONFIG_OMAP_USB_PHY) += omap_usb_phy.o
+obj-$(CONFIG_ROCKCHIP_USB_SYNO_PHY) += rockchip_usb_syno_phy.o diff --git a/drivers/usb/phy/rockchip_usb_syno_phy.c b/drivers/usb/phy/rockchip_usb_syno_phy.c new file mode 100644 index 0000000..ab049e1 --- /dev/null +++ b/drivers/usb/phy/rockchip_usb_syno_phy.c @@ -0,0 +1,47 @@ +/*
- Copyright 2016 Rockchip Electronics Co., Ltd
- SPDX-License-Identifier: GPL-2.0+
- */
+#include <common.h> +#include <asm/io.h>
+#include "../gadget/dwc2_udc_otg_priv.h"
+#define UOC_CON(x) ((x) * 0x4)
As you know, dwc2 usb driver didn't apply devicetree model. It's hard to implement a compatible driver for Rockchip SoCs which use a same udc.
Yeah, it is pretty strange that the dwc2 host part in uboot uses the devicetree while the gadget part does not - but I maybe someone else can answer this, as my contact with uboot was limited until now. The dwc2 host driver works just fine on my rk3036 kylin board.
For example, SOFT_CTRL bit is UOC_CON2[2] on RK3288 I can't assure it's also UOC_CON[2] on other socs. Do you have any good ideas?
Operating under the current constraints, I guess you could simply do something similar to what socfpga does in its board file.
Aka get the usb controller node, read the phy phandle and get the phy compatible string from the dts. The usbphys each have per-soc compatible values "rockchip,rk3288-usbphy" etc, so you can determine the needed bits over that.
Heiko
Am Mittwoch, 6. Juli 2016, 14:48:57 schrieb Heiko Stuebner:
Am Mittwoch, 6. Juli 2016, 18:20:04 schrieb Ziyuan Xu:
Hi heiko,
On 2016年07月06日 17:34, Ziyuan Xu wrote:
From: Xu Ziyuan xzy.xu@rock-chips.com
So far, Rockchip SoCs have two kinds of USB2.0 phy, like Synopsys and Innosilicon. This patch applys dwc2 usb driver framework to implement phy_init and phy_off for Synopsys phy on Rockchip platform.
Signed-off-by: Ziyuan Xu xzy.xu@rock-chips.com
Changes in v3:
- Make UOC_CON registers to be unfixed which should be got from DT
Changes in v2:
Rename rk3288_usb_phy.c to rockchip_usb_syno_phy.c
Rework the behaviour in otg_phy_init() and otg_phy_off()
drivers/usb/phy/Makefile | 1 + drivers/usb/phy/rockchip_usb_syno_phy.c | 47 +++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+) create mode 100644 drivers/usb/phy/rockchip_usb_syno_phy.c
diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile index 93d147e..8002a18 100644 --- a/drivers/usb/phy/Makefile +++ b/drivers/usb/phy/Makefile @@ -7,3 +7,4 @@
obj-$(CONFIG_TWL4030_USB) += twl4030.o obj-$(CONFIG_OMAP_USB_PHY) += omap_usb_phy.o
+obj-$(CONFIG_ROCKCHIP_USB_SYNO_PHY) += rockchip_usb_syno_phy.o diff --git a/drivers/usb/phy/rockchip_usb_syno_phy.c b/drivers/usb/phy/rockchip_usb_syno_phy.c new file mode 100644 index 0000000..ab049e1 --- /dev/null +++ b/drivers/usb/phy/rockchip_usb_syno_phy.c @@ -0,0 +1,47 @@ +/*
- Copyright 2016 Rockchip Electronics Co., Ltd
- SPDX-License-Identifier: GPL-2.0+
- */
+#include <common.h> +#include <asm/io.h>
+#include "../gadget/dwc2_udc_otg_priv.h"
+#define UOC_CON(x) ((x) * 0x4)
As you know, dwc2 usb driver didn't apply devicetree model. It's hard to implement a compatible driver for Rockchip SoCs which use a same udc.
Yeah, it is pretty strange that the dwc2 host part in uboot uses the devicetree while the gadget part does not - but I maybe someone else can answer this, as my contact with uboot was limited until now. The dwc2 host driver works just fine on my rk3036 kylin board.
For example, SOFT_CTRL bit is UOC_CON2[2] on RK3288 I can't assure it's also UOC_CON[2] on other socs. Do you have any good ideas?
Operating under the current constraints, I guess you could simply do something similar to what socfpga does in its board file.
Aka get the usb controller node, read the phy phandle and get the phy compatible string from the dts. The usbphys each have per-soc compatible values "rockchip,rk3288-usbphy" etc, so you can determine the needed bits over that.
forgot to add, with this same mechanism you could also resolve the fifo length from the devicetree into the platdata struct instead of hard-coding it with an ARCH_ROCKCHIP define.
Heiko
On 2016年07月06日 21:42, Heiko Stuebner wrote:
Am Mittwoch, 6. Juli 2016, 14:48:57 schrieb Heiko Stuebner:
Am Mittwoch, 6. Juli 2016, 18:20:04 schrieb Ziyuan Xu:
Hi heiko,
On 2016年07月06日 17:34, Ziyuan Xu wrote:
From: Xu Ziyuan xzy.xu@rock-chips.com
So far, Rockchip SoCs have two kinds of USB2.0 phy, like Synopsys and Innosilicon. This patch applys dwc2 usb driver framework to implement phy_init and phy_off for Synopsys phy on Rockchip platform.
Signed-off-by: Ziyuan Xu xzy.xu@rock-chips.com
Changes in v3:
- Make UOC_CON registers to be unfixed which should be got from DT
Changes in v2:
Rename rk3288_usb_phy.c to rockchip_usb_syno_phy.c
Rework the behaviour in otg_phy_init() and otg_phy_off()
drivers/usb/phy/Makefile | 1 + drivers/usb/phy/rockchip_usb_syno_phy.c | 47 +++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+) create mode 100644 drivers/usb/phy/rockchip_usb_syno_phy.c
diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile index 93d147e..8002a18 100644 --- a/drivers/usb/phy/Makefile +++ b/drivers/usb/phy/Makefile @@ -7,3 +7,4 @@
obj-$(CONFIG_TWL4030_USB) += twl4030.o obj-$(CONFIG_OMAP_USB_PHY) += omap_usb_phy.o
+obj-$(CONFIG_ROCKCHIP_USB_SYNO_PHY) += rockchip_usb_syno_phy.o diff --git a/drivers/usb/phy/rockchip_usb_syno_phy.c b/drivers/usb/phy/rockchip_usb_syno_phy.c new file mode 100644 index 0000000..ab049e1 --- /dev/null +++ b/drivers/usb/phy/rockchip_usb_syno_phy.c @@ -0,0 +1,47 @@ +/*
- Copyright 2016 Rockchip Electronics Co., Ltd
- SPDX-License-Identifier: GPL-2.0+
- */
+#include <common.h> +#include <asm/io.h>
+#include "../gadget/dwc2_udc_otg_priv.h"
+#define UOC_CON(x) ((x) * 0x4)
As you know, dwc2 usb driver didn't apply devicetree model. It's hard to implement a compatible driver for Rockchip SoCs which use a same udc.
Yeah, it is pretty strange that the dwc2 host part in uboot uses the devicetree while the gadget part does not - but I maybe someone else can answer this, as my contact with uboot was limited until now. The dwc2 host driver works just fine on my rk3036 kylin board.
For example, SOFT_CTRL bit is UOC_CON2[2] on RK3288 I can't assure it's also UOC_CON[2] on other socs. Do you have any good ideas?
Operating under the current constraints, I guess you could simply do something similar to what socfpga does in its board file.
socfpga? Could you please indicate which project and file? I can't find out it.
Aka get the usb controller node, read the phy phandle and get the phy compatible string from the dts. The usbphys each have per-soc compatible values "rockchip,rk3288-usbphy" etc, so you can determine the needed bits over that.
In patchset [v3.3/4](http://patchwork.ozlabs.org/patch/645188/), I had get usb-phy offset from grf. If I understand it correctly, you mean that implement a struct in driver to match compatible and platdata. Then define any properties in platdata, contains of some bits which will be used. Something similar to https://patchwork.kernel.org/patch/9190287/?
forgot to add, with this same mechanism you could also resolve the fifo length from the devicetree into the platdata struct instead of hard-coding it with an ARCH_ROCKCHIP define.
Good idea, I will do it.
Heiko
Am Donnerstag, 7. Juli 2016, 09:58:38 schrieb Ziyuan Xu:
On 2016年07月06日 21:42, Heiko Stuebner wrote:
Am Mittwoch, 6. Juli 2016, 14:48:57 schrieb Heiko Stuebner:
Am Mittwoch, 6. Juli 2016, 18:20:04 schrieb Ziyuan Xu:
Hi heiko,
On 2016年07月06日 17:34, Ziyuan Xu wrote:
From: Xu Ziyuan xzy.xu@rock-chips.com
So far, Rockchip SoCs have two kinds of USB2.0 phy, like Synopsys and Innosilicon. This patch applys dwc2 usb driver framework to implement phy_init and phy_off for Synopsys phy on Rockchip platform.
Signed-off-by: Ziyuan Xu xzy.xu@rock-chips.com
Changes in v3:
- Make UOC_CON registers to be unfixed which should be got from DT
Changes in v2:
Rename rk3288_usb_phy.c to rockchip_usb_syno_phy.c
Rework the behaviour in otg_phy_init() and otg_phy_off()
drivers/usb/phy/Makefile | 1 + drivers/usb/phy/rockchip_usb_syno_phy.c | 47 +++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+) create mode 100644 drivers/usb/phy/rockchip_usb_syno_phy.c
diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile index 93d147e..8002a18 100644 --- a/drivers/usb/phy/Makefile +++ b/drivers/usb/phy/Makefile @@ -7,3 +7,4 @@
obj-$(CONFIG_TWL4030_USB) += twl4030.o obj-$(CONFIG_OMAP_USB_PHY) += omap_usb_phy.o
+obj-$(CONFIG_ROCKCHIP_USB_SYNO_PHY) += rockchip_usb_syno_phy.o diff --git a/drivers/usb/phy/rockchip_usb_syno_phy.c b/drivers/usb/phy/rockchip_usb_syno_phy.c new file mode 100644 index 0000000..ab049e1 --- /dev/null +++ b/drivers/usb/phy/rockchip_usb_syno_phy.c @@ -0,0 +1,47 @@ +/*
- Copyright 2016 Rockchip Electronics Co., Ltd
- SPDX-License-Identifier: GPL-2.0+
- */
+#include <common.h> +#include <asm/io.h>
+#include "../gadget/dwc2_udc_otg_priv.h"
+#define UOC_CON(x) ((x) * 0x4)
As you know, dwc2 usb driver didn't apply devicetree model. It's hard to implement a compatible driver for Rockchip SoCs which use a same udc.
Yeah, it is pretty strange that the dwc2 host part in uboot uses the devicetree while the gadget part does not - but I maybe someone else can answer this, as my contact with uboot was limited until now. The dwc2 host driver works just fine on my rk3036 kylin board.
For example, SOFT_CTRL bit is UOC_CON2[2] on RK3288 I can't assure it's also UOC_CON[2] on other socs. Do you have any good ideas?
Operating under the current constraints, I guess you could simply do something similar to what socfpga does in its board file.
socfpga? Could you please indicate which project and file? I can't find out it.
Aka get the usb controller node, read the phy phandle and get the phy compatible string from the dts. The usbphys each have per-soc compatible values "rockchip,rk3288-usbphy" etc, so you can determine the needed bits over that.
In patchset [v3.3/4](http://patchwork.ozlabs.org/patch/645188/), I had get usb-phy offset from grf. If I understand it correctly, you mean that implement a struct in driver to match compatible and platdata. Then define any properties in platdata, contains of some bits which will be used. Something similar to https://patchwork.kernel.org/patch/9190287/?
yep, you just need to find some mechanism to identify the soc you're running on so that the phy part can access the right registers on each one.
Heiko
On 2016年07月08日 04:33, Heiko Stuebner wrote:
Am Donnerstag, 7. Juli 2016, 09:58:38 schrieb Ziyuan Xu:
On 2016年07月06日 21:42, Heiko Stuebner wrote:
Am Mittwoch, 6. Juli 2016, 14:48:57 schrieb Heiko Stuebner:
Am Mittwoch, 6. Juli 2016, 18:20:04 schrieb Ziyuan Xu:
Hi heiko,
On 2016年07月06日 17:34, Ziyuan Xu wrote:
From: Xu Ziyuan xzy.xu@rock-chips.com
So far, Rockchip SoCs have two kinds of USB2.0 phy, like Synopsys and Innosilicon. This patch applys dwc2 usb driver framework to implement phy_init and phy_off for Synopsys phy on Rockchip platform.
Signed-off-by: Ziyuan Xu xzy.xu@rock-chips.com
Changes in v3:
- Make UOC_CON registers to be unfixed which should be got from DT
Changes in v2:
Rename rk3288_usb_phy.c to rockchip_usb_syno_phy.c
Rework the behaviour in otg_phy_init() and otg_phy_off()
drivers/usb/phy/Makefile | 1 + drivers/usb/phy/rockchip_usb_syno_phy.c | 47 +++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+) create mode 100644 drivers/usb/phy/rockchip_usb_syno_phy.c
diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile index 93d147e..8002a18 100644 --- a/drivers/usb/phy/Makefile +++ b/drivers/usb/phy/Makefile @@ -7,3 +7,4 @@
obj-$(CONFIG_TWL4030_USB) += twl4030.o obj-$(CONFIG_OMAP_USB_PHY) += omap_usb_phy.o+obj-$(CONFIG_ROCKCHIP_USB_SYNO_PHY) += rockchip_usb_syno_phy.o diff --git a/drivers/usb/phy/rockchip_usb_syno_phy.c b/drivers/usb/phy/rockchip_usb_syno_phy.c new file mode 100644 index 0000000..ab049e1 --- /dev/null +++ b/drivers/usb/phy/rockchip_usb_syno_phy.c @@ -0,0 +1,47 @@ +/*
- Copyright 2016 Rockchip Electronics Co., Ltd
- SPDX-License-Identifier: GPL-2.0+
- */
+#include <common.h> +#include <asm/io.h>
+#include "../gadget/dwc2_udc_otg_priv.h"
+#define UOC_CON(x) ((x) * 0x4)
As you know, dwc2 usb driver didn't apply devicetree model. It's hard to implement a compatible driver for Rockchip SoCs which use a same udc.
Yeah, it is pretty strange that the dwc2 host part in uboot uses the devicetree while the gadget part does not - but I maybe someone else can answer this, as my contact with uboot was limited until now. The dwc2 host driver works just fine on my rk3036 kylin board.
For example, SOFT_CTRL bit is UOC_CON2[2] on RK3288 I can't assure it's also UOC_CON[2] on other socs. Do you have any good ideas?
Operating under the current constraints, I guess you could simply do something similar to what socfpga does in its board file.
socfpga? Could you please indicate which project and file? I can't find out it.
Aka get the usb controller node, read the phy phandle and get the phy compatible string from the dts. The usbphys each have per-soc compatible values "rockchip,rk3288-usbphy" etc, so you can determine the needed bits over that.
In patchset [v3.3/4](http://patchwork.ozlabs.org/patch/645188/), I had get usb-phy offset from grf. If I understand it correctly, you mean that implement a struct in driver to match compatible and platdata. Then define any properties in platdata, contains of some bits which will be used. Something similar to https://patchwork.kernel.org/patch/9190287/?
yep, you just need to find some mechanism to identify the soc you're running on so that the phy part can access the right registers on each one.
Thanks for your opinion, I will do it.
Heiko
From: Xu Ziyuan xzy.xu@rock-chips.com
The total FIFO size of dwc2 on Rockchip SoCs is shorter than the existen, so re-define them to fit Rockchip SoCs.
Signed-off-by: Ziyuan Xu xzy.xu@rock-chips.com
---
Changes in v3: None Changes in v2: - Update detailed commit message - Modify the macro's values
drivers/usb/gadget/dwc2_udc_otg_regs.h | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/drivers/usb/gadget/dwc2_udc_otg_regs.h b/drivers/usb/gadget/dwc2_udc_otg_regs.h index 78ec90e..6004a4b 100644 --- a/drivers/usb/gadget/dwc2_udc_otg_regs.h +++ b/drivers/usb/gadget/dwc2_udc_otg_regs.h @@ -130,9 +130,15 @@ struct dwc2_usbotg_reg { #define HIGH_SPEED_CONTROL_PKT_SIZE 64 #define HIGH_SPEED_BULK_PKT_SIZE 512
+#ifdef CONFIG_ARCH_ROCKCHIP +#define RX_FIFO_SIZE (512*4) +#define NPTX_FIFO_SIZE (16*4) +#define PTX_FIFO_SIZE (128*4) +#else #define RX_FIFO_SIZE (1024*4) #define NPTX_FIFO_SIZE (1024*4) #define PTX_FIFO_SIZE (1536*1) +#endif
#define DEPCTL_TXFNUM_0 (0x0<<22) #define DEPCTL_TXFNUM_1 (0x1<<22)
From: Xu Ziyuan xzy.xu@rock-chips.com
Enable fastboot feature on rk3288.
This path doesn't support the fastboot flash function command entirely. We will hit "cannot find partition" assertion without specified partition environment. Define gpt partition layout in specified board such as firefly-rk3288, then enjoy it!
Signed-off-by: Ziyuan Xu xzy.xu@rock-chips.com
---
Changes in v3: - Achieve UOC_CON_OFFSET physical address from DT
Changes in v2: - Achieve the regs_phy from DT - Update comments a little
arch/arm/dts/rk3288.dtsi | 1 + arch/arm/mach-rockchip/board.c | 60 +++++++++++++++++++++++++++++++++++++++++ include/configs/rk3288_common.h | 26 ++++++++++++++++++ 3 files changed, 87 insertions(+)
diff --git a/arch/arm/dts/rk3288.dtsi b/arch/arm/dts/rk3288.dtsi index 3dab0fc..bcf051a 100644 --- a/arch/arm/dts/rk3288.dtsi +++ b/arch/arm/dts/rk3288.dtsi @@ -454,6 +454,7 @@ interrupts = <GIC_SPI 23 IRQ_TYPE_LEVEL_HIGH>; clocks = <&cru HCLK_OTG0>; clock-names = "otg"; + dr_mode = "otg"; phys = <&usbphy0>; phy-names = "usb2-phy"; status = "disabled"; diff --git a/arch/arm/mach-rockchip/board.c b/arch/arm/mach-rockchip/board.c index 816540e..56e3c31 100644 --- a/arch/arm/mach-rockchip/board.c +++ b/arch/arm/mach-rockchip/board.c @@ -52,6 +52,66 @@ void lowlevel_init(void) { }
+#if defined(CONFIG_USB_GADGET) && defined(CONFIG_USB_GADGET_DWC2_OTG) +#include <usb.h> +#include <usb/dwc2_udc.h> + +static struct dwc2_plat_otg_data rk3288_otg_data; + +int board_usb_init(int index, enum usb_init_type init) +{ + int offset; + const char *mode; + bool matched = false; + const void *blob = gd->fdt_blob; + u32 grf_phy_offset; + + /* find the usb_otg node */ + offset = fdt_node_offset_by_compatible(blob, -1, + "rockchip,rk3288-usb"); + + while (offset > 0) { + mode = fdt_getprop(blob, offset, "dr_mode", NULL); + if (mode && strcmp(mode, "otg") == 0) { + matched = true; + break; + } + + offset = fdt_node_offset_by_compatible(blob, offset, + "rockchip,rk3288-usb"); + } + if (!matched) { + debug("Not found usb_otg device\n"); + return -ENODEV; + } + rk3288_otg_data.regs_otg = fdtdec_get_addr(blob, offset, "reg"); + + offset = fdtdec_lookup_phandle(blob, offset, "phys"); + if (offset <= 0) { + debug("Not found usb phy device\n"); + return -ENODEV; + } + grf_phy_offset = fdtdec_get_addr(blob, offset, "reg"); + + /* find the grf node */ + offset = fdt_node_offset_by_compatible(blob, -1, + "rockchip,rk3288-grf"); + if (offset <= 0) { + debug("Not found grf device\n"); + return -ENODEV; + } + rk3288_otg_data.regs_phy = grf_phy_offset + + fdtdec_get_addr(blob, offset, "reg"); + + return dwc2_udc_probe(&rk3288_otg_data); +} + +int board_usb_cleanup(int index, enum usb_init_type init) +{ + return 0; +} +#endif + static int do_clock(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) { diff --git a/include/configs/rk3288_common.h b/include/configs/rk3288_common.h index 9d50d83..94fd13f 100644 --- a/include/configs/rk3288_common.h +++ b/include/configs/rk3288_common.h @@ -80,6 +80,32 @@ #define CONFIG_SPI #define CONFIG_SF_DEFAULT_SPEED 20000000
+/* usb otg */ +#define CONFIG_USB_GADGET +#define CONFIG_USB_GADGET_DUALSPEED +#define CONFIG_USB_GADGET_DWC2_OTG +#define CONFIG_ROCKCHIP_USB_SYNO_PHY +#define CONFIG_USB_GADGET_VBUS_DRAW 0 + +/* fastboot */ +#define CONFIG_CMD_FASTBOOT +#define CONFIG_USB_FUNCTION_FASTBOOT +#define CONFIG_FASTBOOT_FLASH +#define CONFIG_FASTBOOT_FLASH_MMC_DEV 1 /* eMMC */ +/* stroe safely fastboot buffer data to the middle of bank */ +#define CONFIG_FASTBOOT_BUF_ADDR (CONFIG_SYS_SDRAM_BASE \ + + SDRAM_BANK_SIZE / 2) +#define CONFIG_FASTBOOT_BUF_SIZE 0x08000000 + +#define CONFIG_USB_GADGET_DOWNLOAD +#define CONFIG_G_DNL_MANUFACTURER "Rockchip" +#define CONFIG_G_DNL_VENDOR_NUM 0x2207 +#define CONFIG_G_DNL_PRODUCT_NUM 0x320a + +/* Enable gpt partition table */ +#define CONFIG_CMD_GPT +#define CONFIG_EFI_PARTITION + #ifndef CONFIG_SPL_BUILD #include <config_distro_defaults.h>
From: Xu Ziyuan xzy.xu@rock-chips.com
Invalidate dcache before starting the DMA to ensure coherency. In case there are any dirty lines from the DMA buffer in the cache, subsequent cache-line replacements may corrupt the buffer in memory while the DMA is still going on. Cache-line replacement can happen if the CPU tries to bring some other memory locations into the cache while the DMA is going on.
Signed-off-by: Ziyuan Xu xzy.xu@rock-chips.com ---
Changes in v3: - New commit since v3 to fix the coherence issue between memory and cache
Changes in v2: None
drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c b/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c index 12f5c85..0d6d2fb 100644 --- a/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c +++ b/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c @@ -110,6 +110,9 @@ static int setdma_rx(struct dwc2_ep *ep, struct dwc2_request *req)
ctrl = readl(®->out_endp[ep_num].doepctl);
+ invalidate_dcache_range((unsigned long) ep->dma_buf, + (unsigned long) ep->dma_buf + ep->len); + writel((unsigned int) ep->dma_buf, ®->out_endp[ep_num].doepdma); writel(DOEPT_SIZ_PKT_CNT(pktcnt) | DOEPT_SIZ_XFER_SIZE(length), ®->out_endp[ep_num].doeptsiz);
Hi Ziyuan,
On 6 July 2016 at 03:34, Ziyuan Xu xzy.xu@rock-chips.com wrote:
From: Xu Ziyuan xzy.xu@rock-chips.com
Invalidate dcache before starting the DMA to ensure coherency. In case there are any dirty lines from the DMA buffer in the cache, subsequent cache-line replacements may corrupt the buffer in memory while the DMA is still going on. Cache-line replacement can happen if the CPU tries to bring some other memory locations into the cache while the DMA is going on.
Signed-off-by: Ziyuan Xu xzy.xu@rock-chips.com
Changes in v3:
- New commit since v3 to fix the coherence issue between memory and
cache
Changes in v2: None
drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c b/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c index 12f5c85..0d6d2fb 100644 --- a/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c +++ b/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c @@ -110,6 +110,9 @@ static int setdma_rx(struct dwc2_ep *ep, struct dwc2_request *req)
ctrl = readl(®->out_endp[ep_num].doepctl);
invalidate_dcache_range((unsigned long) ep->dma_buf,(unsigned long) ep->dma_buf + ep->len);
There is an invalidate in complete_rx() which is one of the callers for this function. It seems to me that we should not have this in two places. Why do we have this problem? Is it because the other calls to setdma_rx() don't invalidate?
I think the invalidate should happen just before reading the data. Can you please check if the other invalidate is needed? Also see how it cache-aligns the end address.
writel((unsigned int) ep->dma_buf, ®->out_endp[ep_num].doepdma); writel(DOEPT_SIZ_PKT_CNT(pktcnt) | DOEPT_SIZ_XFER_SIZE(length), ®->out_endp[ep_num].doeptsiz);-- 1.9.1
Regards, Simon
On 2016年07月12日 20:59, Simon Glass wrote:
Hi Ziyuan,
On 6 July 2016 at 03:34, Ziyuan Xu xzy.xu@rock-chips.com wrote:
From: Xu Ziyuan xzy.xu@rock-chips.com
Invalidate dcache before starting the DMA to ensure coherency. In case there are any dirty lines from the DMA buffer in the cache, subsequent cache-line replacements may corrupt the buffer in memory while the DMA is still going on. Cache-line replacement can happen if the CPU tries to bring some other memory locations into the cache while the DMA is going on.
Signed-off-by: Ziyuan Xu xzy.xu@rock-chips.com
Changes in v3:
- New commit since v3 to fix the coherence issue between memory and
cache
Changes in v2: None
drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c b/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c index 12f5c85..0d6d2fb 100644 --- a/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c +++ b/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c @@ -110,6 +110,9 @@ static int setdma_rx(struct dwc2_ep *ep, struct dwc2_request *req)
ctrl = readl(®->out_endp[ep_num].doepctl);
invalidate_dcache_range((unsigned long) ep->dma_buf,(unsigned long) ep->dma_buf + ep->len);There is an invalidate in complete_rx() which is one of the callers for this function. It seems to me that we should not have this in two places. Why do we have this problem? Is it because the other calls to setdma_rx() don't invalidate?
Yup, invoke invalidate method twice during one complete transmission: 1- invalidate in setdma_rx() in case of the write back cache, present from cache-line replacement after DMA transmission complete. i.e. 1) dma_buf = "123456789abcd"; 2) didn't invalidate in setdma_rx() 3) DMA complete interrupt coming 4) invalidate in complete_rx() 5) read dma_buf, it's "123456789abcd"
If invalidate in step 2, we will achieve correct data. I think it's necessary to invalidate before starting DMA, and doc/README.arm-caches describe details. 2- invalidate in complete_rx(), cpu read the dma_buf from memory directly.
I think the invalidate should happen just before reading the data. Can you please check if the other invalidate is needed? Also see how it cache-aligns the end address.
memalign(CONFIG_SYS_CACHELINE_SIZE, EP_BUFFER_SIZE); cache-aligns: 64 bytes. dma_buffer size: 4096
I had check cache-aligins and end address, rightful. Furthermore, everything works fine with noncached_alloc().
writel((unsigned int) ep->dma_buf, ®->out_endp[ep_num].doepdma); writel(DOEPT_SIZ_PKT_CNT(pktcnt) | DOEPT_SIZ_XFER_SIZE(length), ®->out_endp[ep_num].doeptsiz);-- 1.9.1
Regards, Simon
On 12 July 2016 at 19:06, Ziyuan Xu xzy.xu@rock-chips.com wrote:
On 2016年07月12日 20:59, Simon Glass wrote:
Hi Ziyuan,
On 6 July 2016 at 03:34, Ziyuan Xu xzy.xu@rock-chips.com wrote:
From: Xu Ziyuan xzy.xu@rock-chips.com
Invalidate dcache before starting the DMA to ensure coherency. In case there are any dirty lines from the DMA buffer in the cache, subsequent cache-line replacements may corrupt the buffer in memory while the DMA is still going on. Cache-line replacement can happen if the CPU tries to bring some other memory locations into the cache while the DMA is going on.
Signed-off-by: Ziyuan Xu xzy.xu@rock-chips.com
Changes in v3:
- New commit since v3 to fix the coherence issue between memory and
cache
Changes in v2: None
drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c b/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c index 12f5c85..0d6d2fb 100644 --- a/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c +++ b/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c @@ -110,6 +110,9 @@ static int setdma_rx(struct dwc2_ep *ep, struct dwc2_request *req)
ctrl = readl(®->out_endp[ep_num].doepctl);
invalidate_dcache_range((unsigned long) ep->dma_buf,(unsigned long) ep->dma_buf + ep->len);There is an invalidate in complete_rx() which is one of the callers for this function. It seems to me that we should not have this in two places. Why do we have this problem? Is it because the other calls to setdma_rx() don't invalidate?
Yup, invoke invalidate method twice during one complete transmission: 1- invalidate in setdma_rx() in case of the write back cache, present from cache-line replacement after DMA transmission complete. i.e.
- dma_buf = "123456789abcd";
- didn't invalidate in setdma_rx()
- DMA complete interrupt coming
- invalidate in complete_rx()
- read dma_buf, it's "123456789abcd"
If invalidate in step 2, we will achieve correct data. I think it's necessary to invalidate before starting DMA, and doc/README.arm-caches describe details. 2- invalidate in complete_rx(), cpu read the dma_buf from memory directly.
I think the invalidate should happen just before reading the data. Can you please check if the other invalidate is needed? Also see how it cache-aligns the end address.
memalign(CONFIG_SYS_CACHELINE_SIZE, EP_BUFFER_SIZE); cache-aligns: 64 bytes. dma_buffer size: 4096
I had check cache-aligins and end address, rightful. Furthermore, everything works fine with noncached_alloc().
OK, thanks for the details. Can the invalidate in (4) be dropped? We should only need one invalidate.
Reviewed-by: Simon Glass sjg@chromium.org
writel((unsigned int) ep->dma_buf,®->out_endp[ep_num].doepdma); writel(DOEPT_SIZ_PKT_CNT(pktcnt) | DOEPT_SIZ_XFER_SIZE(length), ®->out_endp[ep_num].doeptsiz); -- 1.9.1
Regards, Simon
Hi Simon,
On 2016年07月14日 23:00, Simon Glass wrote:
On 12 July 2016 at 19:06, Ziyuan Xu xzy.xu@rock-chips.com wrote:
On 2016年07月12日 20:59, Simon Glass wrote:
Hi Ziyuan,
On 6 July 2016 at 03:34, Ziyuan Xu xzy.xu@rock-chips.com wrote:
From: Xu Ziyuan xzy.xu@rock-chips.com
Invalidate dcache before starting the DMA to ensure coherency. In case there are any dirty lines from the DMA buffer in the cache, subsequent cache-line replacements may corrupt the buffer in memory while the DMA is still going on. Cache-line replacement can happen if the CPU tries to bring some other memory locations into the cache while the DMA is going on.
Signed-off-by: Ziyuan Xu xzy.xu@rock-chips.com
Changes in v3:
- New commit since v3 to fix the coherence issue between memory and
cache
Changes in v2: None
drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c b/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c index 12f5c85..0d6d2fb 100644 --- a/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c +++ b/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c @@ -110,6 +110,9 @@ static int setdma_rx(struct dwc2_ep *ep, struct dwc2_request *req)
ctrl = readl(®->out_endp[ep_num].doepctl);
invalidate_dcache_range((unsigned long) ep->dma_buf,(unsigned long) ep->dma_buf + ep->len);There is an invalidate in complete_rx() which is one of the callers for this function. It seems to me that we should not have this in two places. Why do we have this problem? Is it because the other calls to setdma_rx() don't invalidate?
Yup, invoke invalidate method twice during one complete transmission: 1- invalidate in setdma_rx() in case of the write back cache, present from cache-line replacement after DMA transmission complete. i.e.
- dma_buf = "123456789abcd";
- didn't invalidate in setdma_rx()
- DMA complete interrupt coming
- invalidate in complete_rx()
- read dma_buf, it's "123456789abcd"
If invalidate in step 2, we will achieve correct data. I think it's necessary to invalidate before starting DMA, and doc/README.arm-caches describe details. 2- invalidate in complete_rx(), cpu read the dma_buf from memory directly.
I think the invalidate should happen just before reading the data. Can you please check if the other invalidate is needed? Also see how it cache-aligns the end address.
memalign(CONFIG_SYS_CACHELINE_SIZE, EP_BUFFER_SIZE); cache-aligns: 64 bytes. dma_buffer size: 4096
I had check cache-aligins and end address, rightful. Furthermore, everything works fine with noncached_alloc().
OK, thanks for the details. Can the invalidate in (4) be dropped? We should only need one invalidate.
We also need invalidate in after DMA transfer completed, because in some processors memory contents can spontaneouslycome to the cache due to speculative memory access by the CPU. If this happens with the DMA buffer while DMA is going on we have a coherency problem. Thanks for your review!
Reviewed-by: Simon Glass sjg@chromium.org
writel((unsigned int) ep->dma_buf,®->out_endp[ep_num].doepdma); writel(DOEPT_SIZ_PKT_CNT(pktcnt) | DOEPT_SIZ_XFER_SIZE(length), ®->out_endp[ep_num].doeptsiz); -- 1.9.1
Regards, Simon
Hi Ziyuan,
On 14 July 2016 at 09:43, Ziyuan Xu xzy.xu@rock-chips.com wrote:
Hi Simon,
On 2016年07月14日 23:00, Simon Glass wrote:
On 12 July 2016 at 19:06, Ziyuan Xu xzy.xu@rock-chips.com wrote:
On 2016年07月12日 20:59, Simon Glass wrote:
Hi Ziyuan,
On 6 July 2016 at 03:34, Ziyuan Xu xzy.xu@rock-chips.com wrote:
From: Xu Ziyuan xzy.xu@rock-chips.com
Invalidate dcache before starting the DMA to ensure coherency. In case there are any dirty lines from the DMA buffer in the cache, subsequent cache-line replacements may corrupt the buffer in memory while the DMA is still going on. Cache-line replacement can happen if the CPU tries to bring some other memory locations into the cache while the DMA is going on.
Signed-off-by: Ziyuan Xu xzy.xu@rock-chips.com
Changes in v3:
- New commit since v3 to fix the coherence issue between memory and
cache
Changes in v2: None
drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c b/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c index 12f5c85..0d6d2fb 100644 --- a/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c +++ b/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c @@ -110,6 +110,9 @@ static int setdma_rx(struct dwc2_ep *ep, struct dwc2_request *req)
ctrl = readl(®->out_endp[ep_num].doepctl);
invalidate_dcache_range((unsigned long) ep->dma_buf,(unsigned long) ep->dma_buf + ep->len);There is an invalidate in complete_rx() which is one of the callers for this function. It seems to me that we should not have this in two places. Why do we have this problem? Is it because the other calls to setdma_rx() don't invalidate?
Yup, invoke invalidate method twice during one complete transmission: 1- invalidate in setdma_rx() in case of the write back cache, present from cache-line replacement after DMA transmission complete. i.e.
- dma_buf = "123456789abcd";
- didn't invalidate in setdma_rx()
- DMA complete interrupt coming
- invalidate in complete_rx()
- read dma_buf, it's "123456789abcd"
If invalidate in step 2, we will achieve correct data. I think it's necessary to invalidate before starting DMA, and doc/README.arm-caches describe details. 2- invalidate in complete_rx(), cpu read the dma_buf from memory directly.
I think the invalidate should happen just before reading the data. Can you please check if the other invalidate is needed? Also see how it cache-aligns the end address.
memalign(CONFIG_SYS_CACHELINE_SIZE, EP_BUFFER_SIZE); cache-aligns: 64 bytes. dma_buffer size: 4096
I had check cache-aligins and end address, rightful. Furthermore, everything works fine with noncached_alloc().
OK, thanks for the details. Can the invalidate in (4) be dropped? We should only need one invalidate.
We also need invalidate in after DMA transfer completed, because in some processors memory contents can spontaneouslycome to the cache due to speculative memory access by the CPU. If this happens with the DMA buffer while DMA is going on we have a coherency problem.
Gosh that is horrible.
Thanks for your review!
Reviewed-by: Simon Glass sjg@chromium.org
writel((unsigned int) ep->dma_buf,®->out_endp[ep_num].doepdma); writel(DOEPT_SIZ_PKT_CNT(pktcnt) | DOEPT_SIZ_XFER_SIZE(length), ®->out_endp[ep_num].doeptsiz); -- 1.9.1
Regards, Simon
Regards, Simon
On 14 July 2016 at 21:20, Simon Glass sjg@chromium.org wrote:
Hi Ziyuan,
On 14 July 2016 at 09:43, Ziyuan Xu xzy.xu@rock-chips.com wrote:
Hi Simon,
On 2016年07月14日 23:00, Simon Glass wrote:
On 12 July 2016 at 19:06, Ziyuan Xu xzy.xu@rock-chips.com wrote:
On 2016年07月12日 20:59, Simon Glass wrote:
Hi Ziyuan,
On 6 July 2016 at 03:34, Ziyuan Xu xzy.xu@rock-chips.com wrote:
From: Xu Ziyuan xzy.xu@rock-chips.com
Invalidate dcache before starting the DMA to ensure coherency. In case there are any dirty lines from the DMA buffer in the cache, subsequent cache-line replacements may corrupt the buffer in memory while the DMA is still going on. Cache-line replacement can happen if the CPU tries to bring some other memory locations into the cache while the DMA is going on.
Signed-off-by: Ziyuan Xu xzy.xu@rock-chips.com
Changes in v3:
- New commit since v3 to fix the coherence issue between memory and
cache
Changes in v2: None
drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c b/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c index 12f5c85..0d6d2fb 100644 --- a/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c +++ b/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c @@ -110,6 +110,9 @@ static int setdma_rx(struct dwc2_ep *ep, struct dwc2_request *req)
ctrl = readl(®->out_endp[ep_num].doepctl);
invalidate_dcache_range((unsigned long) ep->dma_buf,(unsigned long) ep->dma_buf + ep->len);There is an invalidate in complete_rx() which is one of the callers for this function. It seems to me that we should not have this in two places. Why do we have this problem? Is it because the other calls to setdma_rx() don't invalidate?
Yup, invoke invalidate method twice during one complete transmission: 1- invalidate in setdma_rx() in case of the write back cache, present from cache-line replacement after DMA transmission complete. i.e.
- dma_buf = "123456789abcd";
- didn't invalidate in setdma_rx()
- DMA complete interrupt coming
- invalidate in complete_rx()
- read dma_buf, it's "123456789abcd"
If invalidate in step 2, we will achieve correct data. I think it's necessary to invalidate before starting DMA, and doc/README.arm-caches describe details. 2- invalidate in complete_rx(), cpu read the dma_buf from memory directly.
I think the invalidate should happen just before reading the data. Can you please check if the other invalidate is needed? Also see how it cache-aligns the end address.
memalign(CONFIG_SYS_CACHELINE_SIZE, EP_BUFFER_SIZE); cache-aligns: 64 bytes. dma_buffer size: 4096
I had check cache-aligins and end address, rightful. Furthermore, everything works fine with noncached_alloc().
OK, thanks for the details. Can the invalidate in (4) be dropped? We should only need one invalidate.
We also need invalidate in after DMA transfer completed, because in some processors memory contents can spontaneouslycome to the cache due to speculative memory access by the CPU. If this happens with the DMA buffer while DMA is going on we have a coherency problem.
Gosh that is horrible.
Thanks for your review!
Applied to u-boot-rockchip, thanks!
Hi,
On 6 July 2016 at 03:34, Ziyuan Xu xzy.xu@rock-chips.com wrote:
This patchset add the fastboot support for rk3288, and I have tested on firefly-rk3288 board.
Fix an issue which was mentioned in V1's cover-letter: The DMA buffer was always zero after DMA transfer is complete, It takes no effect that invalidate dcache after the DMA is complete and before the CPU reads it. It's important to invalidate dcache before starting DMA, ensure coherency and prevent from any dirty lines in the cache which from the DMA buffer.
Summary of changes in this series:
- Achieve UOC_CON_OFFSET physical address from DT
- Make UOC_CON registers to be unfixed which should be got from DT
- Add new commit dd77b11fd44a84 (usb: dwc2: Invalidate dcache before
staring DMA)
Changes in v3:
- Make UOC_CON registers to be unfixed which should be got from DT
- Achieve UOC_CON_OFFSET physical address from DT
- New commit since v3 to fix the coherence issue between memory and
cache
Changes in v2:
- Rename rk3288_usb_phy.c to rockchip_usb_syno_phy.c
- Rework the behaviour in otg_phy_init() and otg_phy_off()
- Update detailed commit message
- Modify the macro's values
- Achieve the regs_phy from DT
- Update comments a little
Xu Ziyuan (4): usb: rockchip-phy: implement USB2.0 phy control for Synopsys usb: dwc2-otg: re-define fifo-size for Rockchip SoCs rockchip: rk3288: add fastboot support usb: dwc2: invalidate dcache before starting DMA
arch/arm/dts/rk3288.dtsi | 1 + arch/arm/mach-rockchip/board.c | 60 ++++++++++++++++++++++++++++++ drivers/usb/gadget/dwc2_udc_otg_regs.h | 6 +++ drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c | 3 ++ drivers/usb/phy/Makefile | 1 + drivers/usb/phy/rockchip_usb_syno_phy.c | 47 +++++++++++++++++++++++ include/configs/rk3288_common.h | 26 +++++++++++++ 7 files changed, 144 insertions(+) create mode 100644 drivers/usb/phy/rockchip_usb_syno_phy.c
I'm holding off applying for a new version of this series.
Regards, Simon
hi Simon,
On 2016年07月12日 07:54, Simon Glass wrote:
Hi,
On 6 July 2016 at 03:34, Ziyuan Xu xzy.xu@rock-chips.com wrote:
This patchset add the fastboot support for rk3288, and I have tested on firefly-rk3288 board.
Fix an issue which was mentioned in V1's cover-letter: The DMA buffer was always zero after DMA transfer is complete, It takes no effect that invalidate dcache after the DMA is complete and before the CPU reads it. It's important to invalidate dcache before starting DMA, ensure coherency and prevent from any dirty lines in the cache which from the DMA buffer.
Summary of changes in this series:
- Achieve UOC_CON_OFFSET physical address from DT
- Make UOC_CON registers to be unfixed which should be got from DT
- Add new commit dd77b11fd44a84 (usb: dwc2: Invalidate dcache before
staring DMA)
Changes in v3:
- Make UOC_CON registers to be unfixed which should be got from DT
- Achieve UOC_CON_OFFSET physical address from DT
- New commit since v3 to fix the coherence issue between memory and
cache
Changes in v2:
- Rename rk3288_usb_phy.c to rockchip_usb_syno_phy.c
- Rework the behaviour in otg_phy_init() and otg_phy_off()
- Update detailed commit message
- Modify the macro's values
- Achieve the regs_phy from DT
- Update comments a little
Xu Ziyuan (4): usb: rockchip-phy: implement USB2.0 phy control for Synopsys usb: dwc2-otg: re-define fifo-size for Rockchip SoCs rockchip: rk3288: add fastboot support usb: dwc2: invalidate dcache before starting DMA
arch/arm/dts/rk3288.dtsi | 1 + arch/arm/mach-rockchip/board.c | 60 ++++++++++++++++++++++++++++++ drivers/usb/gadget/dwc2_udc_otg_regs.h | 6 +++ drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c | 3 ++ drivers/usb/phy/Makefile | 1 + drivers/usb/phy/rockchip_usb_syno_phy.c | 47 +++++++++++++++++++++++ include/configs/rk3288_common.h | 26 +++++++++++++ 7 files changed, 144 insertions(+) create mode 100644 drivers/usb/phy/rockchip_usb_syno_phy.c
I'm holding off applying for a new version of this series.
okay, I will send v4 patch later, thanks!
Regards, Simon
participants (3)
-
Heiko Stuebner -
Simon Glass -
Ziyuan Xu