[U-Boot] [PATCH v3 0/9] Improve rockusb support in U-Boot

[U-Boot] [PATCH v3 0/9] Improve rockusb support in U-Boot

[Alberto Panizzo]

rockusb protocol has been introduced by Eddie Cai in U-Boot mainline
allowing to write internal eMMC of RK3288 based boards (and potentially
all other Rockchip's CPUs).

On workstation side the open source project rkdeveloptool do implement
the rockusb protocol. You can find it on GitHub here:
https://github.com/rockchip-linux/rkdeveloptool

This patchset increase the supported functionalities on target side
allowing developers to:
- Read flash: rl command of rkdeveloptool
- Read chip version: rci command of rkdeveloptool
- Complete the write cycle implementing block erase
- Improve read/write speed

Changes in v2:
- Reworked patch 1/8 to obtain simpler and more logical code
- Rewrote some patch messages
- Updated documentation in README.rockusb  patch by patch
- Added patch 8/8 to fix debug prints of original code

Changes in v3:
- Removed magic block size of 512 bytes for all implemented commands
  original behavior of re-probing a non present block device
  (like a non plugged SD-Card) is kept

Alberto Panizzo (9):
  usb: rockchip: fix command failed on host side due to missing data
  usb: rockchip: implement skeleton for K_FW_GET_CHIP_VER command
  rockchip: rk3288: implement reading chip version from bootrom code
  usb: rockchip: implement K_FW_LBA_READ_10 command
  usb: rockchip: implement K_FW_LBA_ERASE_10 command
  usb: rockchip: be quiet on serial port while transferring data
  usb: rockchip: boost up write speed from 4MB/s to 15MB/s
  usb: rockchip: fix printing csw debug info
  usb: rockchip: on K_FW_LBA_WRITE_10 remove magic block size of 512
    bytes

 arch/arm/include/asm/arch-rockchip/f_rockusb.h |   6 +-
 arch/arm/mach-rockchip/rk3288/Makefile         |   1 +
 arch/arm/mach-rockchip/rk3288/rockusb_rk3288.c |  30 +++
 doc/README.rockusb                             |  11 +-
 drivers/usb/gadget/f_rockusb.c                 | 261 ++++++++++++++++++++++---
 5 files changed, 281 insertions(+), 28 deletions(-)
 create mode 100644 arch/arm/mach-rockchip/rk3288/rockusb_rk3288.c

-- 
2.7.4

[U-Boot] [PATCH v3 1/9] usb: rockchip: fix command failed on host side due to missing data

[Alberto Panizzo]

Two consecutive rockusb_tx_write without waiting for request complete
do results in transfer reset of first request and thus no or incomplete
data transfer. This because rockusb_tx_write do use just one USB request
to keep serialization.

So calls like:
rockusb_tx_write_str(emmc_id);
rockusb_tx_write_csw(cbw->tag, cbw->data_transfer_length, CSW_GOOD);

was succeeding only when DEBUG was defined because the time spent
printing debug info was enough for transfer to complete.

This patch fixes the issue adding a simple request complete handler
called rockusb_tx_write_csw to be set as complete handler of in_req
when sending back simple payload + CSW replies to commands.

This new handler will always send CSW_GOOD replies because in case
of error the command callback itself must send back an error CSW as
unique reply to command.

This patch fixes execution of:
$ rkdeveloptool rfi
when DEBUG is not defined.

Signed-off-by: Alberto Panizzo <alberto@amarulasolutions.com>
---
 drivers/usb/gadget/f_rockusb.c | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/f_rockusb.c b/drivers/usb/gadget/f_rockusb.c
index b8833d0..58e483c 100644
--- a/drivers/usb/gadget/f_rockusb.c
+++ b/drivers/usb/gadget/f_rockusb.c
@@ -388,6 +388,20 @@ static int rockusb_tx_write_csw(u32 tag, int residue, u8 status, int size)
 	return rockusb_tx_write((char *)csw, size);
 }
 
+static void tx_handler_send_csw(struct usb_ep *ep, struct usb_request *req)
+{
+	struct f_rockusb *f_rkusb = get_rkusb();
+	int status = req->status;
+
+	if (status)
+		debug("status: %d ep '%s' trans: %d\n",
+		      status, ep->name, req->actual);
+
+	/* Return back to default in_req complete function after sending CSW */
+	req->complete = rockusb_complete;
+	rockusb_tx_write_csw(f_rkusb->tag, 0, CSW_GOOD, USB_BULK_CS_WRAP_LEN);
+}
+
 static unsigned int rx_bytes_expected(struct usb_ep *ep)
 {
 	struct f_rockusb *f_rkusb = get_rkusb();
@@ -496,13 +510,17 @@ static void cb_read_storage_id(struct usb_ep *ep, struct usb_request *req)
 {
 	ALLOC_CACHE_ALIGN_BUFFER(struct fsg_bulk_cb_wrap, cbw,
 				 sizeof(struct fsg_bulk_cb_wrap));
+	struct f_rockusb *f_rkusb = get_rkusb();
 	char emmc_id[] = "EMMC ";
 
 	printf("read storage id\n");
 	memcpy((char *)cbw, req->buf, USB_BULK_CB_WRAP_LEN);
+
+	/* Prepare for sending subsequent CSW_GOOD */
+	f_rkusb->tag = cbw->tag;
+	f_rkusb->in_req->complete = tx_handler_send_csw;
+
 	rockusb_tx_write_str(emmc_id);
-	rockusb_tx_write_csw(cbw->tag, cbw->data_transfer_length, CSW_GOOD,
-			     USB_BULK_CS_WRAP_LEN);
 }
 
 static void cb_write_lba(struct usb_ep *ep, struct usb_request *req)
-- 
2.7.4

[U-Boot] [PATCH v3 2/9] usb: rockchip: implement skeleton for K_FW_GET_CHIP_VER command

[Alberto Panizzo]

Chip Version is a string saved in BOOTROM address space Little Endian.

Ex for rk3288: 0x33323041 0x32303134 0x30383133 0x56323030
which brings:  320A20140813V200

Note that memory version do invert MSB/LSB so printing the char
buffer would show: A02341023180002V

Signed-off-by: Alberto Panizzo <alberto@amarulasolutions.com>
---
 doc/README.rockusb             |  9 ++++++---
 drivers/usb/gadget/f_rockusb.c | 44 +++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 49 insertions(+), 4 deletions(-)

diff --git a/doc/README.rockusb b/doc/README.rockusb
index 5405dc4..3a93edc 100644
--- a/doc/README.rockusb
+++ b/doc/README.rockusb
@@ -42,9 +42,12 @@ see doc/README.rockchip for more detail about how to get U-Boot binary.
 
 sudo rkdeveloptool wl  64 <U-Boot binary>
 
-There are plenty of Rockusb command. but wl(write lba) and
-rd(reboot) command. These two command can let people flash
-image to device.
+Current set of rkdeveloptool commands supported:
+- rci: Read Chip Info
+- rfi: Read Flash Id
+- rd : Reset Device
+- td : Test Device Ready
+- wl : Write blocks using LBA
 
 To do
 -----
diff --git a/drivers/usb/gadget/f_rockusb.c b/drivers/usb/gadget/f_rockusb.c
index 58e483c..0314ff0 100644
--- a/drivers/usb/gadget/f_rockusb.c
+++ b/drivers/usb/gadget/f_rockusb.c
@@ -523,6 +523,48 @@ static void cb_read_storage_id(struct usb_ep *ep, struct usb_request *req)
 	rockusb_tx_write_str(emmc_id);
 }
 
+int __weak rk_get_bootrom_chip_version(unsigned int *chip_info, int size)
+{
+	return 0;
+}
+
+static void cb_get_chip_version(struct usb_ep *ep, struct usb_request *req)
+{
+	ALLOC_CACHE_ALIGN_BUFFER(struct fsg_bulk_cb_wrap, cbw,
+				 sizeof(struct fsg_bulk_cb_wrap));
+	struct f_rockusb *f_rkusb = get_rkusb();
+	unsigned int chip_info[4], i;
+
+	memset(chip_info, 0, sizeof(chip_info));
+	rk_get_bootrom_chip_version(chip_info, 4);
+
+	/*
+	 * Chip Version is a string saved in BOOTROM address space Little Endian
+	 *
+	 * Ex for rk3288: 0x33323041 0x32303134 0x30383133 0x56323030
+	 * which brings:  320A20140813V200
+	 *
+	 * Note that memory version do invert MSB/LSB so printing the char
+	 * buffer will show: A02341023180002V
+	 */
+	printf("read chip version: ");
+	for (i = 0; i < 4; i++) {
+		printf("%c%c%c%c",
+		       (chip_info[i] >> 24) & 0xFF,
+		       (chip_info[i] >> 16) & 0xFF,
+		       (chip_info[i] >>  8) & 0xFF,
+		       (chip_info[i] >>  0) & 0xFF);
+	}
+	printf("\n");
+	memcpy((char *)cbw, req->buf, USB_BULK_CB_WRAP_LEN);
+
+	/* Prepare for sending subsequent CSW_GOOD */
+	f_rkusb->tag = cbw->tag;
+	f_rkusb->in_req->complete = tx_handler_send_csw;
+
+	rockusb_tx_write((char *)chip_info, sizeof(chip_info));
+}
+
 static void cb_write_lba(struct usb_ep *ep, struct usb_request *req)
 {
 	ALLOC_CACHE_ALIGN_BUFFER(struct fsg_bulk_cb_wrap, cbw,
@@ -661,7 +703,7 @@ static const struct cmd_dispatch_info cmd_dispatch_info[] = {
 	},
 	{
 		.cmd = K_FW_GET_CHIP_VER,
-		.cb = cb_not_support,
+		.cb = cb_get_chip_version,
 	},
 	{
 		.cmd = K_FW_LOW_FORMAT,
-- 
2.7.4

[U-Boot] [PATCH v3 3/9] rockchip: rk3288: implement reading chip version from bootrom code

[Alberto Panizzo]

This allows rockusb code to reply correctly to K_FW_GET_CHIP_VER
command.

On RK3288 chip version is at 0xffff4ff0 and on tested hardware it
corresponds at the string "320A20140813V200"

Signed-off-by: Alberto Panizzo <alberto@amarulasolutions.com>
---
 arch/arm/mach-rockchip/rk3288/Makefile         |  1 +
 arch/arm/mach-rockchip/rk3288/rockusb_rk3288.c | 30 ++++++++++++++++++++++++++
 2 files changed, 31 insertions(+)
 create mode 100644 arch/arm/mach-rockchip/rk3288/rockusb_rk3288.c

diff --git a/arch/arm/mach-rockchip/rk3288/Makefile b/arch/arm/mach-rockchip/rk3288/Makefile
index a0033a0..da0eb4a 100644
--- a/arch/arm/mach-rockchip/rk3288/Makefile
+++ b/arch/arm/mach-rockchip/rk3288/Makefile
@@ -7,3 +7,4 @@
 obj-y += clk_rk3288.o
 obj-y += rk3288.o
 obj-y += syscon_rk3288.o
+obj-$(CONFIG_USB_FUNCTION_ROCKUSB) += rockusb_rk3288.o
diff --git a/arch/arm/mach-rockchip/rk3288/rockusb_rk3288.c b/arch/arm/mach-rockchip/rk3288/rockusb_rk3288.c
new file mode 100644
index 0000000..62057c1
--- /dev/null
+++ b/arch/arm/mach-rockchip/rk3288/rockusb_rk3288.c
@@ -0,0 +1,30 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2018 Amarula Solutions
+ * Written by Alberto Panizzo <alberto@amarulasolutions.com>
+ */
+
+#include <config.h>
+#include <common.h>
+#include <linux/delay.h>
+#include <asm/io.h>
+#include <asm/arch/boot_mode.h>
+#include <asm/arch/pmu_rk3288.h>
+
+#define ROM_PHYS	0xffff0000
+
+#define ROM_CHIP_VER_ADDR_ADDR		(ROM_PHYS + 0x4FF0)
+#define ROM_CHIP_VER_ADDR_SIZE		16
+
+int rk_get_bootrom_chip_version(unsigned int *chip_info, int size)
+{
+	if (!chip_info)
+		return -1;
+	if (size < ROM_CHIP_VER_ADDR_SIZE / sizeof(int))
+		return -1;
+
+	memcpy((char *)chip_info, (char *)ROM_CHIP_VER_ADDR_ADDR,
+	       ROM_CHIP_VER_ADDR_SIZE);
+
+	return 0;
+}
-- 
2.7.4

[U-Boot] [PATCH v3 4/9] usb: rockchip: implement K_FW_LBA_READ_10 command

[Alberto Panizzo]

This patch implement reading blocks form selected device with
LBA addressing.

Corresponding command on workstation is:
rkdeveloptool rl <start_blk> <blk_cnt> <file>

While we support reading more than one blocks per K_FW_LBA_READ_10
request, rkdeveloptool and original rockchip tool do perform
chunk reads limiting the maximum size per chunk far lower
than max int values.

Signed-off-by: Alberto Panizzo <alberto@amarulasolutions.com>
---
 arch/arm/include/asm/arch-rockchip/f_rockusb.h |   3 +
 doc/README.rockusb                             |   1 +
 drivers/usb/gadget/f_rockusb.c                 | 104 ++++++++++++++++++++++++-
 3 files changed, 107 insertions(+), 1 deletion(-)

diff --git a/arch/arm/include/asm/arch-rockchip/f_rockusb.h b/arch/arm/include/asm/arch-rockchip/f_rockusb.h
index 0b62771..3f2e763 100644
--- a/arch/arm/include/asm/arch-rockchip/f_rockusb.h
+++ b/arch/arm/include/asm/arch-rockchip/f_rockusb.h
@@ -27,6 +27,7 @@
  */
 
 #define RKUSB_BUF_SIZE		EP_BUFFER_SIZE * 2
+#define RKBLOCK_BUF_SIZE		4096
 
 #define RKUSB_STATUS_IDLE			0
 #define RKUSB_STATUS_CMD			1
@@ -120,6 +121,8 @@ struct f_rockusb {
 	unsigned int lba;
 	unsigned int dl_size;
 	unsigned int dl_bytes;
+	unsigned int ul_size;
+	unsigned int ul_bytes;
 	struct blk_desc *desc;
 	int reboot_flag;
 	void *buf;
diff --git a/doc/README.rockusb b/doc/README.rockusb
index 3a93edc..7f58296 100644
--- a/doc/README.rockusb
+++ b/doc/README.rockusb
@@ -47,6 +47,7 @@ Current set of rkdeveloptool commands supported:
 - rfi: Read Flash Id
 - rd : Reset Device
 - td : Test Device Ready
+- rl : Read blocks using LBA
 - wl : Write blocks using LBA
 
 To do
diff --git a/drivers/usb/gadget/f_rockusb.c b/drivers/usb/gadget/f_rockusb.c
index 0314ff0..82f421c 100644
--- a/drivers/usb/gadget/f_rockusb.c
+++ b/drivers/usb/gadget/f_rockusb.c
@@ -328,6 +328,7 @@ static int rockusb_tx_write(const char *buffer, unsigned int buffer_size)
 
 	memcpy(in_req->buf, buffer, buffer_size);
 	in_req->length = buffer_size;
+	debug("Transferring 0x%x bytes\n", buffer_size);
 	usb_ep_dequeue(rockusb_func->in_ep, in_req);
 	ret = usb_ep_queue(rockusb_func->in_ep, in_req, 0);
 	if (ret)
@@ -421,6 +422,65 @@ static unsigned int rx_bytes_expected(struct usb_ep *ep)
 	return rx_remain;
 }
 
+/* usb_request complete call back to handle upload image */
+static void tx_handler_ul_image(struct usb_ep *ep, struct usb_request *req)
+{
+	ALLOC_CACHE_ALIGN_BUFFER(char, rbuffer, RKBLOCK_BUF_SIZE);
+	struct f_rockusb *f_rkusb = get_rkusb();
+	struct usb_request *in_req = rockusb_func->in_req;
+	int ret;
+
+	/* Print error status of previous transfer */
+	if (req->status)
+		debug("status: %d ep '%s' trans: %d len %d\n", req->status,
+		      ep->name, req->actual, req->length);
+
+	/* On transfer complete reset in_req and feedback host with CSW_GOOD */
+	if (f_rkusb->ul_bytes >= f_rkusb->ul_size) {
+		in_req->length = 0;
+		in_req->complete = rockusb_complete;
+
+		rockusb_tx_write_csw(f_rkusb->tag, 0, CSW_GOOD,
+				     USB_BULK_CS_WRAP_LEN);
+		return;
+	}
+
+	/* Proceed with current chunk */
+	unsigned int transfer_size = f_rkusb->ul_size - f_rkusb->ul_bytes;
+
+	if (transfer_size > RKBLOCK_BUF_SIZE)
+		transfer_size = RKBLOCK_BUF_SIZE;
+	/* Read at least one block */
+	unsigned int blkcount = (transfer_size + f_rkusb->desc->blksz - 1) /
+				f_rkusb->desc->blksz;
+
+	debug("ul %x bytes, %x blks, read lba %x, ul_size:%x, ul_bytes:%x, ",
+	      transfer_size, blkcount, f_rkusb->lba,
+	      f_rkusb->ul_size, f_rkusb->ul_bytes);
+
+	int blks = blk_dread(f_rkusb->desc, f_rkusb->lba, blkcount, rbuffer);
+
+	if (blks != blkcount) {
+		printf("failed reading from device %s: %d\n",
+		       f_rkusb->dev_type, f_rkusb->dev_index);
+		rockusb_tx_write_csw(f_rkusb->tag, 0, CSW_FAIL,
+				     USB_BULK_CS_WRAP_LEN);
+		return;
+	}
+	f_rkusb->lba += blkcount;
+	f_rkusb->ul_bytes += transfer_size;
+
+	/* Proceed with USB request */
+	memcpy(in_req->buf, rbuffer, transfer_size);
+	in_req->length = transfer_size;
+	in_req->complete = tx_handler_ul_image;
+	printf("Uploading 0x%x bytes\n", transfer_size);
+	usb_ep_dequeue(rockusb_func->in_ep, in_req);
+	ret = usb_ep_queue(rockusb_func->in_ep, in_req, 0);
+	if (ret)
+		printf("Error %d on queue\n", ret);
+}
+
 /* usb_request complete call back to handle down load image */
 static void rx_handler_dl_image(struct usb_ep *ep, struct usb_request *req)
 {
@@ -565,6 +625,48 @@ static void cb_get_chip_version(struct usb_ep *ep, struct usb_request *req)
 	rockusb_tx_write((char *)chip_info, sizeof(chip_info));
 }
 
+static void cb_read_lba(struct usb_ep *ep, struct usb_request *req)
+{
+	ALLOC_CACHE_ALIGN_BUFFER(struct fsg_bulk_cb_wrap, cbw,
+				 sizeof(struct fsg_bulk_cb_wrap));
+	struct f_rockusb *f_rkusb = get_rkusb();
+	int sector_count;
+
+	memcpy((char *)cbw, req->buf, USB_BULK_CB_WRAP_LEN);
+	sector_count = (int)get_unaligned_be16(&cbw->CDB[7]);
+	f_rkusb->tag = cbw->tag;
+
+	if (!f_rkusb->desc) {
+		char *type = f_rkusb->dev_type;
+		int index = f_rkusb->dev_index;
+
+		f_rkusb->desc = blk_get_dev(type, index);
+		if (!f_rkusb->desc ||
+		    f_rkusb->desc->type == DEV_TYPE_UNKNOWN) {
+			printf("invalid device \"%s\", %d\n", type, index);
+			rockusb_tx_write_csw(f_rkusb->tag, 0, CSW_FAIL,
+					     USB_BULK_CS_WRAP_LEN);
+			return;
+		}
+	}
+
+	f_rkusb->lba = get_unaligned_be32(&cbw->CDB[2]);
+	f_rkusb->ul_size = sector_count * f_rkusb->desc->blksz;
+	f_rkusb->ul_bytes = 0;
+
+	debug("require read %x bytes, %x sectors from lba %x\n",
+	      f_rkusb->ul_size, sector_count, f_rkusb->lba);
+
+	if (f_rkusb->ul_size == 0)  {
+		rockusb_tx_write_csw(cbw->tag, cbw->data_transfer_length,
+				     CSW_FAIL, USB_BULK_CS_WRAP_LEN);
+		return;
+	}
+
+	/* Start right now sending first chunk */
+	tx_handler_ul_image(ep, req);
+}
+
 static void cb_write_lba(struct usb_ep *ep, struct usb_request *req)
 {
 	ALLOC_CACHE_ALIGN_BUFFER(struct fsg_bulk_cb_wrap, cbw,
@@ -675,7 +777,7 @@ static const struct cmd_dispatch_info cmd_dispatch_info[] = {
 	},
 	{
 		.cmd = K_FW_LBA_READ_10,
-		.cb = cb_not_support,
+		.cb = cb_read_lba,
 	},
 	{
 		.cmd = K_FW_LBA_WRITE_10,
-- 
2.7.4

[U-Boot] [PATCH v3 5/9] usb: rockchip: implement K_FW_LBA_ERASE_10 command

[Alberto Panizzo]

This command is part of the write partition sequence performed by
rkdeveloptool: one partition is first completely erased and
than wrote.

Signed-off-by: Alberto Panizzo <alberto@amarulasolutions.com>
---
 arch/arm/include/asm/arch-rockchip/f_rockusb.h |  1 +
 doc/README.rockusb                             |  1 +
 drivers/usb/gadget/f_rockusb.c                 | 48 ++++++++++++++++++++++++++
 3 files changed, 50 insertions(+)

diff --git a/arch/arm/include/asm/arch-rockchip/f_rockusb.h b/arch/arm/include/asm/arch-rockchip/f_rockusb.h
index 3f2e763..9772321 100644
--- a/arch/arm/include/asm/arch-rockchip/f_rockusb.h
+++ b/arch/arm/include/asm/arch-rockchip/f_rockusb.h
@@ -63,6 +63,7 @@ K_FW_LOW_FORMAT = 0x1C,
 K_FW_SET_RESET_FLAG = 0x1E,
 K_FW_SPI_READ_10 = 0x21,
 K_FW_SPI_WRITE_10 = 0x22,
+K_FW_LBA_ERASE_10 = 0x25,
 
 K_FW_SESSION = 0X30,
 K_FW_RESET = 0xff,
diff --git a/doc/README.rockusb b/doc/README.rockusb
index 7f58296..66437e1 100644
--- a/doc/README.rockusb
+++ b/doc/README.rockusb
@@ -49,6 +49,7 @@ Current set of rkdeveloptool commands supported:
 - td : Test Device Ready
 - rl : Read blocks using LBA
 - wl : Write blocks using LBA
+- wlx: Write partition
 
 To do
 -----
diff --git a/drivers/usb/gadget/f_rockusb.c b/drivers/usb/gadget/f_rockusb.c
index 82f421c..bcdc2ba 100644
--- a/drivers/usb/gadget/f_rockusb.c
+++ b/drivers/usb/gadget/f_rockusb.c
@@ -692,6 +692,50 @@ static void cb_write_lba(struct usb_ep *ep, struct usb_request *req)
 	}
 }
 
+static void cb_erase_lba(struct usb_ep *ep, struct usb_request *req)
+{
+	ALLOC_CACHE_ALIGN_BUFFER(struct fsg_bulk_cb_wrap, cbw,
+				 sizeof(struct fsg_bulk_cb_wrap));
+	struct f_rockusb *f_rkusb = get_rkusb();
+	int sector_count, lba, blks;
+
+	memcpy((char *)cbw, req->buf, USB_BULK_CB_WRAP_LEN);
+	sector_count = (int)get_unaligned_be16(&cbw->CDB[7]);
+	f_rkusb->tag = cbw->tag;
+
+	if (!f_rkusb->desc) {
+		char *type = f_rkusb->dev_type;
+		int index = f_rkusb->dev_index;
+
+		f_rkusb->desc = blk_get_dev(type, index);
+		if (!f_rkusb->desc ||
+		    f_rkusb->desc->type == DEV_TYPE_UNKNOWN) {
+			printf("invalid device \"%s\", %d\n", type, index);
+			rockusb_tx_write_csw(f_rkusb->tag, 0, CSW_FAIL,
+					     USB_BULK_CS_WRAP_LEN);
+			return;
+		}
+	}
+
+	lba = get_unaligned_be32(&cbw->CDB[2]);
+
+	debug("require erase %x sectors from lba %x\n",
+	      sector_count, lba);
+
+	blks = blk_derase(f_rkusb->desc, lba, sector_count);
+	if (blks != sector_count) {
+		printf("failed erasing device %s: %d\n", f_rkusb->dev_type,
+		       f_rkusb->dev_index);
+		rockusb_tx_write_csw(f_rkusb->tag,
+				     cbw->data_transfer_length, CSW_FAIL,
+				     USB_BULK_CS_WRAP_LEN);
+		return;
+	}
+
+	rockusb_tx_write_csw(cbw->tag, cbw->data_transfer_length, CSW_GOOD,
+			     USB_BULK_CS_WRAP_LEN);
+}
+
 void __weak rkusb_set_reboot_flag(int flag)
 {
 	struct f_rockusb *f_rkusb = get_rkusb();
@@ -824,6 +868,10 @@ static const struct cmd_dispatch_info cmd_dispatch_info[] = {
 		.cb = cb_not_support,
 	},
 	{
+		.cmd = K_FW_LBA_ERASE_10,
+		.cb = cb_erase_lba,
+	},
+	{
 		.cmd = K_FW_SESSION,
 		.cb = cb_not_support,
 	},
-- 
2.7.4

[U-Boot] [PATCH v3 6/9] usb: rockchip: be quiet on serial port while transferring data

[Alberto Panizzo]

While downloading or uploading megabytes of data we had thousands of
printf in console like:

transfer 0x10000 bytes done
OR
Uploading 0x1000 bytes

This because transfers are chunked and there is no way on target
side to know the overall transfer size (to print one string per
overall transfer).

All these prints on serial console do slow down significantly the
transfer and does not offer a significant information to the
developer: rkdeveloptool and Rockchip original tool do use small
chunks read/writes on big transfers. This allows on workstation
to print percentage of transfer complete and as well offers to
developer the information about: transfer is running OK.
On error, either the percentage will stop or an error will be shown
on workstation console.

Signed-off-by: Alberto Panizzo <alberto@amarulasolutions.com>
---
 drivers/usb/gadget/f_rockusb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/f_rockusb.c b/drivers/usb/gadget/f_rockusb.c
index bcdc2ba..0959b8c 100644
--- a/drivers/usb/gadget/f_rockusb.c
+++ b/drivers/usb/gadget/f_rockusb.c
@@ -474,7 +474,7 @@ static void tx_handler_ul_image(struct usb_ep *ep, struct usb_request *req)
 	memcpy(in_req->buf, rbuffer, transfer_size);
 	in_req->length = transfer_size;
 	in_req->complete = tx_handler_ul_image;
-	printf("Uploading 0x%x bytes\n", transfer_size);
+	debug("Uploading 0x%x bytes\n", transfer_size);
 	usb_ep_dequeue(rockusb_func->in_ep, in_req);
 	ret = usb_ep_queue(rockusb_func->in_ep, in_req, 0);
 	if (ret)
@@ -536,7 +536,7 @@ static void rx_handler_dl_image(struct usb_ep *ep, struct usb_request *req)
 		req->complete = rx_handler_command;
 		req->length = EP_BUFFER_SIZE;
 		f_rkusb->buf = f_rkusb->buf_head;
-		printf("transfer 0x%x bytes done\n", f_rkusb->dl_size);
+		debug("transfer 0x%x bytes done\n", f_rkusb->dl_size);
 		f_rkusb->dl_size = 0;
 		rockusb_tx_write_csw(f_rkusb->tag, 0, CSW_GOOD,
 				     USB_BULK_CS_WRAP_LEN);
-- 
2.7.4

[U-Boot] [PATCH v3 7/9] usb: rockchip: boost up write speed from 4MB/s to 15MB/s

[Alberto Panizzo]

Speedup transfers increasing the max chunk size.
Buffers are allocated with memalign thus developer is noticed when heap is
full and in current configuration a buffer allocation of 64K till now
is safe.

Signed-off-by: Alberto Panizzo <alberto@amarulasolutions.com>
---
 arch/arm/include/asm/arch-rockchip/f_rockusb.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/include/asm/arch-rockchip/f_rockusb.h b/arch/arm/include/asm/arch-rockchip/f_rockusb.h
index 9772321..141aae6 100644
--- a/arch/arm/include/asm/arch-rockchip/f_rockusb.h
+++ b/arch/arm/include/asm/arch-rockchip/f_rockusb.h
@@ -19,7 +19,7 @@
 #define RX_ENDPOINT_MAXIMUM_PACKET_SIZE_1_1  0x0040
 #define TX_ENDPOINT_MAXIMUM_PACKET_SIZE      0x0040
 
-#define EP_BUFFER_SIZE			4096
+#define EP_BUFFER_SIZE			65536
 /*
  * EP_BUFFER_SIZE must always be an integral multiple of maxpacket size
  * (64 or 512 or 1024), else we break on certain controllers like DWC3
-- 
2.7.4

[U-Boot] [PATCH v3 8/9] usb: rockchip: fix printing csw debug info

[Alberto Panizzo]

Workstation tool was happy while console on device were printing
random numbers..

Signed-off-by: Alberto Panizzo <alberto@amarulasolutions.com>
---
 drivers/usb/gadget/f_rockusb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/f_rockusb.c b/drivers/usb/gadget/f_rockusb.c
index 0959b8c..574d610 100644
--- a/drivers/usb/gadget/f_rockusb.c
+++ b/drivers/usb/gadget/f_rockusb.c
@@ -384,7 +384,7 @@ static int rockusb_tx_write_csw(u32 tag, int residue, u8 status, int size)
 	csw->residue = cpu_to_be32(residue);
 	csw->status = status;
 #ifdef DEBUG
-	printcsw((char *)&csw);
+	printcsw((char *)csw);
 #endif
 	return rockusb_tx_write((char *)csw, size);
 }
-- 
2.7.4

[U-Boot] [PATCH v3 9/9] usb: rockchip: on K_FW_LBA_WRITE_10 remove magic block size of 512 bytes

[Alberto Panizzo]

As well as in K_FW_LBA_READ_10 and K_FW_LBA_ERASE_10 take device's
block size from f_rkusb->desc->blksz instead of the fixed 512 bytes.

Keep original behaviour of retry probing assigned block device on
every host request to manage late SDCard plugs.

Signed-off-by: Alberto Panizzo <alberto@amarulasolutions.com>
---
 drivers/usb/gadget/f_rockusb.c | 39 +++++++++++++++++++++------------------
 1 file changed, 21 insertions(+), 18 deletions(-)

diff --git a/drivers/usb/gadget/f_rockusb.c b/drivers/usb/gadget/f_rockusb.c
index 574d610..e81eb16 100644
--- a/drivers/usb/gadget/f_rockusb.c
+++ b/drivers/usb/gadget/f_rockusb.c
@@ -490,19 +490,6 @@ static void rx_handler_dl_image(struct usb_ep *ep, struct usb_request *req)
 	unsigned int buffer_size = req->actual;
 
 	transfer_size = f_rkusb->dl_size - f_rkusb->dl_bytes;
-	if (!f_rkusb->desc) {
-		char *type = f_rkusb->dev_type;
-		int index = f_rkusb->dev_index;
-
-		f_rkusb->desc = blk_get_dev(type, index);
-		if (!f_rkusb->desc ||
-		    f_rkusb->desc->type == DEV_TYPE_UNKNOWN) {
-			puts("invalid mmc device\n");
-			rockusb_tx_write_csw(f_rkusb->tag, 0, CSW_FAIL,
-					     USB_BULK_CS_WRAP_LEN);
-			return;
-		}
-	}
 
 	if (req->status != 0) {
 		printf("Bad status: %d\n", req->status);
@@ -516,7 +503,7 @@ static void rx_handler_dl_image(struct usb_ep *ep, struct usb_request *req)
 
 	memcpy((void *)f_rkusb->buf, buffer, transfer_size);
 	f_rkusb->dl_bytes += transfer_size;
-	int blks = 0, blkcnt = transfer_size  / 512;
+	int blks = 0, blkcnt = transfer_size  / f_rkusb->desc->blksz;
 
 	debug("dl %x bytes, %x blks, write lba %x, dl_size:%x, dl_bytes:%x, ",
 	      transfer_size, blkcnt, f_rkusb->lba, f_rkusb->dl_size,
@@ -547,8 +534,8 @@ static void rx_handler_dl_image(struct usb_ep *ep, struct usb_request *req)
 		else
 			f_rkusb->buf = f_rkusb->buf_head;
 
-		debug("remain %x bytes, %x sectors\n", req->length,
-		      req->length / 512);
+		debug("remain %x bytes, %lx sectors\n", req->length,
+		      req->length / f_rkusb->desc->blksz);
 	}
 
 	req->actual = 0;
@@ -676,10 +663,26 @@ static void cb_write_lba(struct usb_ep *ep, struct usb_request *req)
 
 	memcpy((char *)cbw, req->buf, USB_BULK_CB_WRAP_LEN);
 	sector_count = (int)get_unaligned_be16(&cbw->CDB[7]);
+	f_rkusb->tag = cbw->tag;
+
+	if (!f_rkusb->desc) {
+		char *type = f_rkusb->dev_type;
+		int index = f_rkusb->dev_index;
+
+		f_rkusb->desc = blk_get_dev(type, index);
+		if (!f_rkusb->desc ||
+		    f_rkusb->desc->type == DEV_TYPE_UNKNOWN) {
+			printf("invalid device \"%s\", %d\n", type, index);
+			rockusb_tx_write_csw(f_rkusb->tag, 0, CSW_FAIL,
+					     USB_BULK_CS_WRAP_LEN);
+			return;
+		}
+	}
+
 	f_rkusb->lba = get_unaligned_be32(&cbw->CDB[2]);
-	f_rkusb->dl_size = sector_count * 512;
+	f_rkusb->dl_size = sector_count * f_rkusb->desc->blksz;
 	f_rkusb->dl_bytes = 0;
-	f_rkusb->tag = cbw->tag;
+
 	debug("require write %x bytes, %x sectors to lba %x\n",
 	      f_rkusb->dl_size, sector_count, f_rkusb->lba);
 
-- 
2.7.4

[U-Boot] [PATCH v3 3/9] rockchip: rk3288: implement reading chip version from bootrom code

[Simon Glass]

On 12 July 2018 at 05:05, Alberto Panizzo <alberto@amarulasolutions.com> wrote:
> This allows rockusb code to reply correctly to K_FW_GET_CHIP_VER
> command.
>
> On RK3288 chip version is at 0xffff4ff0 and on tested hardware it
> corresponds at the string "320A20140813V200"
>
> Signed-off-by: Alberto Panizzo <alberto@amarulasolutions.com>
> ---
>  arch/arm/mach-rockchip/rk3288/Makefile         |  1 +
>  arch/arm/mach-rockchip/rk3288/rockusb_rk3288.c | 30 ++++++++++++++++++++++++++
>  2 files changed, 31 insertions(+)
>  create mode 100644 arch/arm/mach-rockchip/rk3288/rockusb_rk3288.c

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH v3 4/9] usb: rockchip: implement K_FW_LBA_READ_10 command

[Simon Glass]

On 12 July 2018 at 05:05, Alberto Panizzo <alberto@amarulasolutions.com> wrote:
> This patch implement reading blocks form selected device with
> LBA addressing.
>
> Corresponding command on workstation is:
> rkdeveloptool rl <start_blk> <blk_cnt> <file>
>
> While we support reading more than one blocks per K_FW_LBA_READ_10
> request, rkdeveloptool and original rockchip tool do perform
> chunk reads limiting the maximum size per chunk far lower
> than max int values.
>
> Signed-off-by: Alberto Panizzo <alberto@amarulasolutions.com>
> ---
>  arch/arm/include/asm/arch-rockchip/f_rockusb.h |   3 +
>  doc/README.rockusb                             |   1 +
>  drivers/usb/gadget/f_rockusb.c                 | 104 ++++++++++++++++++++++++-
>  3 files changed, 107 insertions(+), 1 deletion(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH v3 5/9] usb: rockchip: implement K_FW_LBA_ERASE_10 command

[Simon Glass]

On 12 July 2018 at 05:05, Alberto Panizzo <alberto@amarulasolutions.com> wrote:
> This command is part of the write partition sequence performed by
> rkdeveloptool: one partition is first completely erased and
> than wrote.
>
> Signed-off-by: Alberto Panizzo <alberto@amarulasolutions.com>
> ---
>  arch/arm/include/asm/arch-rockchip/f_rockusb.h |  1 +
>  doc/README.rockusb                             |  1 +
>  drivers/usb/gadget/f_rockusb.c                 | 48 ++++++++++++++++++++++++++
>  3 files changed, 50 insertions(+)
>

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH v3 7/9] usb: rockchip: boost up write speed from 4MB/s to 15MB/s

[Simon Glass]

On 12 July 2018 at 05:05, Alberto Panizzo <alberto@amarulasolutions.com> wrote:
> Speedup transfers increasing the max chunk size.
> Buffers are allocated with memalign thus developer is noticed when heap is
> full and in current configuration a buffer allocation of 64K till now
> is safe.
>
> Signed-off-by: Alberto Panizzo <alberto@amarulasolutions.com>
> ---
>  arch/arm/include/asm/arch-rockchip/f_rockusb.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Really your commit subject should say that you are increasing the
buffer size IMO, since that is the change. Then your commit message
can discuss motivation and impact.

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [U-Boot, v3, 4/9] usb: rockchip: implement K_FW_LBA_READ_10 command

[Philipp Tomsich]

> This patch implement reading blocks form selected device with
> LBA addressing.
> 
> Corresponding command on workstation is:
> rkdeveloptool rl <start_blk> <blk_cnt> <file>
> 
> While we support reading more than one blocks per K_FW_LBA_READ_10
> request, rkdeveloptool and original rockchip tool do perform
> chunk reads limiting the maximum size per chunk far lower
> than max int values.
> 
> Signed-off-by: Alberto Panizzo <alberto@amarulasolutions.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> ---
>  arch/arm/include/asm/arch-rockchip/f_rockusb.h |   3 +
>  doc/README.rockusb                             |   1 +
>  drivers/usb/gadget/f_rockusb.c                 | 104 ++++++++++++++++++++++++-
>  3 files changed, 107 insertions(+), 1 deletion(-)
> 

Reviewed-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>

[U-Boot] [U-Boot, v3, 5/9] usb: rockchip: implement K_FW_LBA_ERASE_10 command

[Philipp Tomsich]

> This command is part of the write partition sequence performed by
> rkdeveloptool: one partition is first completely erased and
> than wrote.
> 
> Signed-off-by: Alberto Panizzo <alberto@amarulasolutions.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> ---
>  arch/arm/include/asm/arch-rockchip/f_rockusb.h |  1 +
>  doc/README.rockusb                             |  1 +
>  drivers/usb/gadget/f_rockusb.c                 | 48 ++++++++++++++++++++++++++
>  3 files changed, 50 insertions(+)
> 

Reviewed-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>

[U-Boot] [U-Boot, v3, 6/9] usb: rockchip: be quiet on serial port while transferring data

[Philipp Tomsich]

> While downloading or uploading megabytes of data we had thousands of
> printf in console like:
> 
> transfer 0x10000 bytes done
> OR
> Uploading 0x1000 bytes
> 
> This because transfers are chunked and there is no way on target
> side to know the overall transfer size (to print one string per
> overall transfer).
> 
> All these prints on serial console do slow down significantly the
> transfer and does not offer a significant information to the
> developer: rkdeveloptool and Rockchip original tool do use small
> chunks read/writes on big transfers. This allows on workstation
> to print percentage of transfer complete and as well offers to
> developer the information about: transfer is running OK.
> On error, either the percentage will stop or an error will be shown
> on workstation console.
> 
> Signed-off-by: Alberto Panizzo <alberto@amarulasolutions.com>
> ---
>  drivers/usb/gadget/f_rockusb.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 

Reviewed-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>

[U-Boot] [U-Boot, v3, 1/9] usb: rockchip: fix command failed on host side due to missing data

[Philipp Tomsich]

On Thu, 12 Jul 2018, Alberto Panizzo wrote:

> Two consecutive rockusb_tx_write without waiting for request complete
> do results in transfer reset of first request and thus no or incomplete
> data transfer. This because rockusb_tx_write do use just one USB request
> to keep serialization.
>
> So calls like:
> rockusb_tx_write_str(emmc_id);
> rockusb_tx_write_csw(cbw->tag, cbw->data_transfer_length, CSW_GOOD);
>
> was succeeding only when DEBUG was defined because the time spent
> printing debug info was enough for transfer to complete.
>
> This patch fixes the issue adding a simple request complete handler
> called rockusb_tx_write_csw to be set as complete handler of in_req
> when sending back simple payload + CSW replies to commands.
>
> This new handler will always send CSW_GOOD replies because in case
> of error the command callback itself must send back an error CSW as
> unique reply to command.
>
> This patch fixes execution of:
> $ rkdeveloptool rfi
> when DEBUG is not defined.
>
> Signed-off-by: Alberto Panizzo <alberto@amarulasolutions.com>
> ---
> drivers/usb/gadget/f_rockusb.c | 22 ++++++++++++++++++++--
> 1 file changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/gadget/f_rockusb.c b/drivers/usb/gadget/f_rockusb.c
> index b8833d0..58e483c 100644
> --- a/drivers/usb/gadget/f_rockusb.c
> +++ b/drivers/usb/gadget/f_rockusb.c
> @@ -388,6 +388,20 @@ static int rockusb_tx_write_csw(u32 tag, int residue, u8 status, int size)
> 	return rockusb_tx_write((char *)csw, size);
> }
>
> +static void tx_handler_send_csw(struct usb_ep *ep, struct usb_request *req)
> +{
> +	struct f_rockusb *f_rkusb = get_rkusb();
> +	int status = req->status;
> +
> +	if (status)
> +		debug("status: %d ep '%s' trans: %d\n",
> +		      status, ep->name, req->actual);

This looks like an error-message.  So shouldn't you always signal the 
error to the use, even if DEBUG is not defined?

> +
> +	/* Return back to default in_req complete function after sending CSW */
> +	req->complete = rockusb_complete;
> +	rockusb_tx_write_csw(f_rkusb->tag, 0, CSW_GOOD, USB_BULK_CS_WRAP_LEN);
> +}
> +
> static unsigned int rx_bytes_expected(struct usb_ep *ep)
> {
> 	struct f_rockusb *f_rkusb = get_rkusb();
> @@ -496,13 +510,17 @@ static void cb_read_storage_id(struct usb_ep *ep, struct usb_request *req)
> {
> 	ALLOC_CACHE_ALIGN_BUFFER(struct fsg_bulk_cb_wrap, cbw,
> 				 sizeof(struct fsg_bulk_cb_wrap));
> +	struct f_rockusb *f_rkusb = get_rkusb();
> 	char emmc_id[] = "EMMC ";
>
> 	printf("read storage id\n");

Should this really be a 'printf'?

> 	memcpy((char *)cbw, req->buf, USB_BULK_CB_WRAP_LEN);
> +
> +	/* Prepare for sending subsequent CSW_GOOD */
> +	f_rkusb->tag = cbw->tag;
> +	f_rkusb->in_req->complete = tx_handler_send_csw;
> +
> 	rockusb_tx_write_str(emmc_id);
> -	rockusb_tx_write_csw(cbw->tag, cbw->data_transfer_length, CSW_GOOD,
> -			     USB_BULK_CS_WRAP_LEN);
> }
>
> static void cb_write_lba(struct usb_ep *ep, struct usb_request *req)
>

[U-Boot] [U-Boot, v3, 2/9] usb: rockchip: implement skeleton for K_FW_GET_CHIP_VER command

[Philipp Tomsich]

On Thu, 12 Jul 2018, Alberto Panizzo wrote:

> Chip Version is a string saved in BOOTROM address space Little Endian.

There's just one tiny problem: the BOOTROM will not be accessible on ARMv8 
targets after ATF has been loaded.

We should define a common mechanism how to deal with this (e.g. by 
injecting it into the DTS of later stages during SPL) and then provide a 
common abstraction over the platforms that always allow accessing the 
BootROM and those that require additional magic.

>
> Ex for rk3288: 0x33323041 0x32303134 0x30383133 0x56323030
> which brings:  320A20140813V200
>
> Note that memory version do invert MSB/LSB so printing the char
> buffer would show: A02341023180002V
>
> Signed-off-by: Alberto Panizzo <alberto@amarulasolutions.com>

Reviewed-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>

Other requested changes (on top of the architectural one from above) 
below.

> ---
> doc/README.rockusb             |  9 ++++++---
> drivers/usb/gadget/f_rockusb.c | 44 +++++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 49 insertions(+), 4 deletions(-)
>
> diff --git a/doc/README.rockusb b/doc/README.rockusb
> index 5405dc4..3a93edc 100644
> --- a/doc/README.rockusb
> +++ b/doc/README.rockusb
> @@ -42,9 +42,12 @@ see doc/README.rockchip for more detail about how to get U-Boot binary.
>
> sudo rkdeveloptool wl  64 <U-Boot binary>
>
> -There are plenty of Rockusb command. but wl(write lba) and
> -rd(reboot) command. These two command can let people flash
> -image to device.
> +Current set of rkdeveloptool commands supported:
> +- rci: Read Chip Info
> +- rfi: Read Flash Id
> +- rd : Reset Device
> +- td : Test Device Ready
> +- wl : Write blocks using LBA
>
> To do
> -----
> diff --git a/drivers/usb/gadget/f_rockusb.c b/drivers/usb/gadget/f_rockusb.c
> index 58e483c..0314ff0 100644
> --- a/drivers/usb/gadget/f_rockusb.c
> +++ b/drivers/usb/gadget/f_rockusb.c
> @@ -523,6 +523,48 @@ static void cb_read_storage_id(struct usb_ep *ep, struct usb_request *req)
> 	rockusb_tx_write_str(emmc_id);
> }
>
> +int __weak rk_get_bootrom_chip_version(unsigned int *chip_info, int size)
> +{
> +	return 0

The default implementation should return an appropriate error code.
Also, I would recommend to return the number of bytes used in the bootrom 
chip-version on the chip this is implemented for.

Documentation for the rk_get_bootrom_chip_version might be useful too 
(e.g. is 'size' a maximum buffer size that needs to be checked against)?

> +}
> +
> +static void cb_get_chip_version(struct usb_ep *ep, struct usb_request *req)
> +{
> +	ALLOC_CACHE_ALIGN_BUFFER(struct fsg_bulk_cb_wrap, cbw,
> +				 sizeof(struct fsg_bulk_cb_wrap));
> +	struct f_rockusb *f_rkusb = get_rkusb();
> +	unsigned int chip_info[4], i;
> +
> +	memset(chip_info, 0, sizeof(chip_info));

While it doesn't hurt, there's no need to perform a memset... you will 
call rk_get_bootrom_chip_version() next anyway.

> +	rk_get_bootrom_chip_version(chip_info, 4);

You should check the error code returned (e.g. in case that a CPU does not 
implement this).  Also: why is the 4 open-coded here and sizeof(chip_info) 
above?

> +
> +	/*
> +	 * Chip Version is a string saved in BOOTROM address space Little Endian
> +	 *
> +	 * Ex for rk3288: 0x33323041 0x32303134 0x30383133 0x56323030
> +	 * which brings:  320A20140813V200
> +	 *
> +	 * Note that memory version do invert MSB/LSB so printing the char
> +	 * buffer will show: A02341023180002V
> +	 */
> +	printf("read chip version: ");
> +	for (i = 0; i < 4; i++) {
> +		printf("%c%c%c%c",
> +		       (chip_info[i] >> 24) & 0xFF,
> +		       (chip_info[i] >> 16) & 0xFF,
> +		       (chip_info[i] >>  8) & 0xFF,
> +		       (chip_info[i] >>  0) & 0xFF);

I'd recommend to do first swap using the functions from 
include/linux/byteorder/generic.h and then just print as a string...

Why the printf? I think you want a debug() ...

> +	}
> +	printf("\n");

Same.

> +	memcpy((char *)cbw, req->buf, USB_BULK_CB_WRAP_LEN);
> +
> +	/* Prepare for sending subsequent CSW_GOOD */
> +	f_rkusb->tag = cbw->tag;
> +	f_rkusb->in_req->complete = tx_handler_send_csw;
> +
> +	rockusb_tx_write((char *)chip_info, sizeof(chip_info));
> +}
> +
> static void cb_write_lba(struct usb_ep *ep, struct usb_request *req)
> {
> 	ALLOC_CACHE_ALIGN_BUFFER(struct fsg_bulk_cb_wrap, cbw,
> @@ -661,7 +703,7 @@ static const struct cmd_dispatch_info cmd_dispatch_info[] = {
> 	},
> 	{
> 		.cmd = K_FW_GET_CHIP_VER,
> -		.cb = cb_not_support,
> +		.cb = cb_get_chip_version,
> 	},
> 	{
> 		.cmd = K_FW_LOW_FORMAT,
>

[U-Boot] [U-Boot, v3, 3/9] rockchip: rk3288: implement reading chip version from bootrom code

[Philipp Tomsich]

On Thu, 12 Jul 2018, Alberto Panizzo wrote:

> This allows rockusb code to reply correctly to K_FW_GET_CHIP_VER
> command.
>
> On RK3288 chip version is at 0xffff4ff0 and on tested hardware it
> corresponds at the string "320A20140813V200"
>
> Signed-off-by: Alberto Panizzo <alberto@amarulasolutions.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Reviewed-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>

See below for requested changes.

> ---
> arch/arm/mach-rockchip/rk3288/Makefile         |  1 +
> arch/arm/mach-rockchip/rk3288/rockusb_rk3288.c | 30 ++++++++++++++++++++++++++
> 2 files changed, 31 insertions(+)
> create mode 100644 arch/arm/mach-rockchip/rk3288/rockusb_rk3288.c
>
> diff --git a/arch/arm/mach-rockchip/rk3288/Makefile b/arch/arm/mach-rockchip/rk3288/Makefile
> index a0033a0..da0eb4a 100644
> --- a/arch/arm/mach-rockchip/rk3288/Makefile
> +++ b/arch/arm/mach-rockchip/rk3288/Makefile
> @@ -7,3 +7,4 @@
> obj-y += clk_rk3288.o
> obj-y += rk3288.o
> obj-y += syscon_rk3288.o
> +obj-$(CONFIG_USB_FUNCTION_ROCKUSB) += rockusb_rk3288.o

This should be conditional on the RK3288.
After all, the below code implements a weak function specifically for the 
RK3288.

We might want to decide on a better architecture (a misc device maybe?) 
that allows us to abstract the various chips' methods to access the device 
id in a more DM-aware way.

> diff --git a/arch/arm/mach-rockchip/rk3288/rockusb_rk3288.c b/arch/arm/mach-rockchip/rk3288/rockusb_rk3288.c
> new file mode 100644
> index 0000000..62057c1
> --- /dev/null
> +++ b/arch/arm/mach-rockchip/rk3288/rockusb_rk3288.c
> @@ -0,0 +1,30 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2018 Amarula Solutions
> + * Written by Alberto Panizzo <alberto@amarulasolutions.com>
> + */
> +
> +#include <config.h>
> +#include <common.h>
> +#include <linux/delay.h>
> +#include <asm/io.h>
> +#include <asm/arch/boot_mode.h>
> +#include <asm/arch/pmu_rk3288.h>
> +
> +#define ROM_PHYS	0xffff0000
> +
> +#define ROM_CHIP_VER_ADDR_ADDR		(ROM_PHYS + 0x4FF0)
> +#define ROM_CHIP_VER_ADDR_SIZE		16
> +
> +int rk_get_bootrom_chip_version(unsigned int *chip_info, int size)
> +{
> +	if (!chip_info)
> +		return -1;
> +	if (size < ROM_CHIP_VER_ADDR_SIZE / sizeof(int))
> +		return -1;

This should use appropriate error-codes.

> +
> +	memcpy((char *)chip_info, (char *)ROM_CHIP_VER_ADDR_ADDR,
> +	       ROM_CHIP_VER_ADDR_SIZE);
> +
> +	return 0;
> +}
>

[U-Boot] [U-Boot, v3, 8/9] usb: rockchip: fix printing csw debug info

[Philipp Tomsich]

On Thu, 12 Jul 2018, Alberto Panizzo wrote:

> Workstation tool was happy while console on device were printing
> random numbers..

Please provide a more meaningful commit message.

>
> Signed-off-by: Alberto Panizzo <alberto@amarulasolutions.com>
> ---
> drivers/usb/gadget/f_rockusb.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/gadget/f_rockusb.c b/drivers/usb/gadget/f_rockusb.c
> index 0959b8c..574d610 100644
> --- a/drivers/usb/gadget/f_rockusb.c
> +++ b/drivers/usb/gadget/f_rockusb.c
> @@ -384,7 +384,7 @@ static int rockusb_tx_write_csw(u32 tag, int residue, u8 status, int size)
> 	csw->residue = cpu_to_be32(residue);
> 	csw->status = status;
> #ifdef DEBUG
> -	printcsw((char *)&csw);
> +	printcsw((char *)csw);
> #endif
> 	return rockusb_tx_write((char *)csw, size);
> }
>