[U-Boot] [PATCH v3 0/4] rockchip: rk3288: add fastboot support

[U-Boot] [PATCH v3 0/4] rockchip: rk3288: add fastboot support

[Ziyuan Xu]

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

-- 
1.9.1

[U-Boot] [PATCH v3 1/4] usb: rockchip-phy: implement USB2.0 phy control for Synopsys

[Ziyuan Xu]

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));
+}
-- 
1.9.1

[U-Boot] [PATCH v3 2/4] usb: dwc2-otg: re-define fifo-size for Rockchip SoCs

[Ziyuan Xu]

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)
-- 
1.9.1

[U-Boot] [PATCH v3 3/4] rockchip: rk3288: add fastboot support

[Ziyuan Xu]

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>
 
-- 
1.9.1

[U-Boot] [PATCH v3 4/4] usb: dwc2: invalidate dcache before starting DMA

[Ziyuan Xu]

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(&reg->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, &reg->out_endp[ep_num].doepdma);
 	writel(DOEPT_SIZ_PKT_CNT(pktcnt) | DOEPT_SIZ_XFER_SIZE(length),
 	       &reg->out_endp[ep_num].doeptsiz);
-- 
1.9.1

[U-Boot] [PATCH v3 1/4] usb: rockchip-phy: implement USB2.0 phy control for Synopsys

[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.
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));
> +}

[U-Boot] [PATCH v3 1/4] usb: rockchip-phy: implement USB2.0 phy control for Synopsys

[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.


Heiko

[U-Boot] [PATCH v3 1/4] usb: rockchip-phy: implement USB2.0 phy control for Synopsys

[Heiko Stuebner]

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

[U-Boot] [PATCH v3 1/4] usb: rockchip-phy: implement USB2.0 phy control for Synopsys

[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/?
> 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
>
>
>

[U-Boot] [PATCH v3 1/4] usb: rockchip-phy: implement USB2.0 phy control for Synopsys

[Heiko Stuebner]

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

[U-Boot] [PATCH v3 1/4] usb: rockchip-phy: implement USB2.0 phy control for Synopsys

[Ziyuan Xu]

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
>
>
>

[U-Boot] [PATCH v3 0/4] rockchip: rk3288: add fastboot support

[Simon Glass]

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

[U-Boot] [PATCH v3 0/4] rockchip: rk3288: add fastboot support

[Ziyuan Xu]

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
>
>
>

[U-Boot] [PATCH v3 4/4] usb: dwc2: invalidate dcache before starting DMA

[Simon Glass]

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(&reg->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, &reg->out_endp[ep_num].doepdma);
>         writel(DOEPT_SIZ_PKT_CNT(pktcnt) | DOEPT_SIZ_XFER_SIZE(length),
>                &reg->out_endp[ep_num].doeptsiz);
> --
> 1.9.1
>
>

Regards,
Simon

[U-Boot] [PATCH v3 4/4] usb: dwc2: invalidate dcache before starting DMA

[Ziyuan Xu]

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(&reg->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, &reg->out_endp[ep_num].doepdma);
>>          writel(DOEPT_SIZ_PKT_CNT(pktcnt) | DOEPT_SIZ_XFER_SIZE(length),
>>                 &reg->out_endp[ep_num].doeptsiz);
>> --
>> 1.9.1
>>
>>
> Regards,
> Simon
>
>
>

[U-Boot] [PATCH v3 4/4] usb: dwc2: invalidate dcache before starting DMA

[Simon Glass]

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(&reg->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().
>

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,
>>> &reg->out_endp[ep_num].doepdma);
>>>          writel(DOEPT_SIZ_PKT_CNT(pktcnt) | DOEPT_SIZ_XFER_SIZE(length),
>>>                 &reg->out_endp[ep_num].doeptsiz);
>>> --
>>> 1.9.1
>>>
>>>
>> Regards,
>> Simon
>>
>>
>>
>
>

[U-Boot] [PATCH v3 4/4] usb: dwc2: invalidate dcache before starting DMA

[Ziyuan Xu]

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(&reg->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().
>>
> 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,
>>>> &reg->out_endp[ep_num].doepdma);
>>>>           writel(DOEPT_SIZ_PKT_CNT(pktcnt) | DOEPT_SIZ_XFER_SIZE(length),
>>>>                  &reg->out_endp[ep_num].doeptsiz);
>>>> --
>>>> 1.9.1
>>>>
>>>>
>>> Regards,
>>> Simon
>>>
>>>
>>>
>>
>
>

[U-Boot] [PATCH v3 4/4] usb: dwc2: invalidate dcache before starting DMA

[Simon Glass]

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(&reg->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().
>>>
>> 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,
>>>>> &reg->out_endp[ep_num].doepdma);
>>>>>           writel(DOEPT_SIZ_PKT_CNT(pktcnt) |
>>>>> DOEPT_SIZ_XFER_SIZE(length),
>>>>>                  &reg->out_endp[ep_num].doeptsiz);
>>>>> --
>>>>> 1.9.1
>>>>>
>>>>>
>>>> Regards,
>>>> Simon
>>>>
>>>>
>>>>
>>>
>>
>>
>
>

Regards,
Simon

[U-Boot] [PATCH v3 4/4] usb: dwc2: invalidate dcache before starting DMA

[Simon Glass]

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(&reg->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().
>>>>
>>> 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!