Improve rockusb support in U-Boot

PATCH Superseded Alberto Panizzo 3 participants 22 messages Jul 3, 2018 – Jul 5, 2018

Subsystems (4)

Arm (32-bit) USB Arm sub-architectures Arm Rockchip

lore

Share & tools

lore

Alberto Panizzo

author

2018-07-03 19:02 UTC #0 lore raw

cover Improve rockusb support in U-Boot

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

Alberto Panizzo (7):
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

 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 ++++
 drivers/usb/gadget/f_rockusb.c                 | 225 ++++++++++++++++++++++++-
 4 files changed, 253 insertions(+), 9 deletions(-)
 create mode 100644 arch/arm/mach-rockchip/rk3288/rockusb_rk3288.c

Signature · 1 line

2.7.4

Alberto Panizzo

author

2018-07-03 19:02 UTC #1 lore raw

PATCH 1/7 usb: rockchip: fix command failed on host side due to missing data

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 une 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 request to complete.

This patch add a way to postpone sending csw after first rockusb_tx_write is completed (indeed inside rockusb_complete) fixing execution of:

$ rkdeveloptool rfi
when DEBUG is not defined.

Signed-off-by Alberto Panizzo <[email protected]>

---
 arch/arm/include/asm/arch-rockchip/f_rockusb.h |  1 +
 drivers/usb/gadget/f_rockusb.c                 | 37 ++++++++++++++++++++++----
 2 files changed, 33 insertions(+), 5 deletions(-)

arch/arm/include/asm/arch-rockchip/f_rockusb.h +1 -0

index 0b62771..f5cad8e 100644
@@ -124,6 +124,7 @@ struct f_rockusb {
 	int reboot_flag;
 	void *buf;
 	void *buf_head;
+	struct bulk_cs_wrap *next_csw;
 };
 
 /* init rockusb device, tell rockusb which device you want to read/write*/

drivers/usb/gadget/f_rockusb.c +32 -5

index b8833d0..a39ad51 100644
@@ -97,6 +97,7 @@ static struct usb_gadget_strings *rkusb_strings[] = {
 
 static struct f_rockusb *rockusb_func;
 static void rx_handler_command(struct usb_ep *ep, struct usb_request *req);
+static int rockusb_tx_write(const char *buffer, unsigned int buffer_size);
 static int rockusb_tx_write_csw(u32 tag, int residue, u8 status, int size);
 
 struct f_rockusb *get_rkusb(void)

@@ -136,11 +137,22 @@ struct usb_endpoint_descriptor *hs)
 
 static void rockusb_complete(struct usb_ep *ep, struct usb_request *req)
 {
+	struct f_rockusb *f_rkusb = get_rkusb();
 	int status = req->status;
 
-	if (!status)
-		return;
-	debug("status: %d ep '%s' trans: %d\n", status, ep->name, req->actual);
+	if (status)
+		debug("status: %d ep '%s' trans: %d\n",
+		      status, ep->name, req->actual);
+
+	/* Send Command Status on previous transfer complete */
+	if (f_rkusb->next_csw) {
+#ifdef DEBUG
+		printcsw((char *)f_rkusb->next_csw);
+#endif
+		rockusb_tx_write((char *)f_rkusb->next_csw,
+				 USB_BULK_CS_WRAP_LEN);
+	}
+	f_rkusb->next_csw = NULL;
 }
 
 /* config the rockusb device*/

@@ -388,6 +400,21 @@ static int rockusb_tx_write_csw(u32 tag, int residue, u8 status, int size)
 	return rockusb_tx_write((char *)csw, size);
 }
 
+struct bulk_cs_wrap g_next_csw;
+static void rockusb_tx_write_csw_on_complete(u32 tag, int residue, u8 status)
+{
+	struct f_rockusb *f_rkusb = get_rkusb();
+
+	g_next_csw.signature = cpu_to_le32(USB_BULK_CS_SIG);
+	g_next_csw.tag = tag;
+	g_next_csw.residue = cpu_to_be32(residue);
+	g_next_csw.status = status;
+#ifdef DEBUG
+	printcsw((char *)&g_next_csw);
+#endif
+	f_rkusb->next_csw = &g_next_csw;
+}
+
 static unsigned int rx_bytes_expected(struct usb_ep *ep)
 {
 	struct f_rockusb *f_rkusb = get_rkusb();

@@ -501,8 +528,8 @@ static void cb_read_storage_id(struct usb_ep *ep, struct usb_request *req)
 	printf("read storage id\n");
 	memcpy((char *)cbw, req->buf, USB_BULK_CB_WRAP_LEN);
 	rockusb_tx_write_str(emmc_id);
-	rockusb_tx_write_csw(cbw->tag, cbw->data_transfer_length, CSW_GOOD,
-			     USB_BULK_CS_WRAP_LEN);
+	rockusb_tx_write_csw_on_complete(cbw->tag, cbw->data_transfer_length,
+					 CSW_GOOD);
 }
 
 static void cb_write_lba(struct usb_ep *ep, struct usb_request *req)

Signature · 1 line

2.7.4

Lukasz Majewski

2018-07-03 21:24 UTC #2 lore raw

PATCH 1/7 usb: rockchip: fix command failed on host side due to missing data

Hi Alberto,

Quoted 11 lines

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 une 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 request to complete.

Serialization by printf.....

Quoted 44 lines

This patch add a way to postpone sending csw after first rockusb_tx_write is completed (indeed inside rockusb_complete) fixing execution of: $ rkdeveloptool rfi
when DEBUG is not defined.
Signed-off-by Alberto Panizzo <[email protected]>
---
 arch/arm/include/asm/arch-rockchip/f_rockusb.h |  1 +
 drivers/usb/gadget/f_rockusb.c                 | 37
++++++++++++++++++++++---- 2 files changed, 33 insertions(+), 5 deletions(-)
(quoted diff) +0 -0
b/arch/arm/include/asm/arch-rockchip/f_rockusb.h index 0b62771..f5cad8e 100644 --- a/arch/arm/include/asm/arch-rockchip/f_rockusb.h +++ b/arch/arm/include/asm/arch-rockchip/f_rockusb.h @@ -124,6 +124,7 @@ struct f_rockusb { int reboot_flag;
arch/arm/include/asm/arch-rockchip/f_rockusb.h +1 -0
 	void *buf;
 	void *buf_head;
+	struct bulk_cs_wrap *next_csw;
 };
 
 /* init rockusb device, tell rockusb which device you want to
read/write*/ diff --git a/drivers/usb/gadget/f_rockusb.c b/drivers/usb/gadget/f_rockusb.c index b8833d0..a39ad51 100644
drivers/usb/gadget/f_rockusb.c +1 -0
@@ -97,6 +97,7 @@ static struct usb_gadget_strings *rkusb_strings[] =
 { 
 static struct f_rockusb *rockusb_func;
 static void rx_handler_command(struct usb_ep *ep, struct usb_request
 *req); +static int rockusb_tx_write(const char *buffer, unsigned int
 buffer_size); static int rockusb_tx_write_csw(u32 tag, int residue,
 u8 status, int size); 
 struct f_rockusb *get_rkusb(void)
@@ -136,11 +137,22 @@ struct usb_endpoint_descriptor *hs)
 
 static void rockusb_complete(struct usb_ep *ep, struct usb_request
 *req) {
+	struct f_rockusb *f_rkusb = get_rkusb();
 	int status = req->status;
 

drivers/usb/gadget/f_rockusb.c struct usb_endpoint_descriptor *hs) · line 149

Quoted 10 lines

drivers/usb/gadget/f_rockusb.c +6 -3

-	if (!status)
-		return;
-	debug("status: %d ep '%s' trans: %d\n", status, ep->name,
 req->actual);
+	if (status)
+		debug("status: %d ep '%s' trans: %d\n",
+		      status, ep->name, req->actual);
+
+	/* Send Command Status on previous transfer complete */
+	if (f_rkusb->next_csw) {

^^^^^^^^ - isn't this a bit misleading? We send the status for the previous transfer.

Quoted 11 lines

drivers/usb/gadget/f_rockusb.c +7 -0

+#ifdef DEBUG
+		printcsw((char *)f_rkusb->next_csw);
+#endif
+		rockusb_tx_write((char *)f_rkusb->next_csw,
+				 USB_BULK_CS_WRAP_LEN);
+	}
+	f_rkusb->next_csw = NULL;
 }

/* config the rockusb device*/

drivers/usb/gadget/f_rockusb.c static int rockusb_tx_write_csw(u32 tag, int · line 403

Quoted 4 lines

drivers/usb/gadget/f_rockusb.c +1 -0

@@ -388,6 +400,21 @@ static int rockusb_tx_write_csw(u32 tag, int
 residue, u8 status, int size) return rockusb_tx_write((char *)csw,
 size); }
 
+struct bulk_cs_wrap g_next_csw;

You have added the pointer to struct bulk_cs_wrap_g to struct f_rockusb, and here we do have global definition.

Two issues with cache; alignment and padding.

Maybe it would be better to allocate it and store pointer int struct f_rockusb ?

Quoted 18 lines

drivers/usb/gadget/f_rockusb.c +13 -0

+static void rockusb_tx_write_csw_on_complete(u32 tag, int residue,
 u8 status) +{
+	struct f_rockusb *f_rkusb = get_rkusb();
+
+	g_next_csw.signature = cpu_to_le32(USB_BULK_CS_SIG);
+	g_next_csw.tag = tag;
+	g_next_csw.residue = cpu_to_be32(residue);
+	g_next_csw.status = status;
+#ifdef DEBUG
+	printcsw((char *)&g_next_csw);
+#endif
+	f_rkusb->next_csw = &g_next_csw;
+}
+
 static unsigned int rx_bytes_expected(struct usb_ep *ep)
 {
 	struct f_rockusb *f_rkusb = get_rkusb();

drivers/usb/gadget/f_rockusb.c static void cb_read_storage_id(struct usb_ep *ep, · line 534

Quoted 9 lines

drivers/usb/gadget/f_rockusb.c +2 -2

@@ -501,8 +528,8 @@ static void cb_read_storage_id(struct usb_ep *ep,
 struct usb_request *req) printf("read storage id\n");
 	memcpy((char *)cbw, req->buf, USB_BULK_CB_WRAP_LEN);
 	rockusb_tx_write_str(emmc_id);
-	rockusb_tx_write_csw(cbw->tag, cbw->data_transfer_length,
 CSW_GOOD,
-			     USB_BULK_CS_WRAP_LEN);
+	rockusb_tx_write_csw_on_complete(cbw->tag,
 cbw->data_transfer_length,
+					 CSW_GOOD);

It seems like you are preparing the content of the csw structure to be ready when the completion is called. Am I right?

What I'm concerned about - with your patch we do have two functions with almost the same code - namely rockusb_tx_write_csw() and rockusb_tx_write_csw_on_complete().

Would it be possible to unify (reuse) the code?

One more remark - shouldn't we set content of g_next_csw in the rockusb_complete() ?

Quoted 3 lines

drivers/usb/gadget/f_rockusb.c +0 -0
 }
static void cb_write_lba(struct usb_ep *ep, struct usb_request *req)

Best regards,

Lukasz Majewski

DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de -------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180703/ad42e197/attachment.sig>

Alberto Panizzo

author

2018-07-04 10:11 UTC #3 lore raw

PATCH 1/7 usb: rockchip: fix command failed on host side due to missing data

Hi Lukasz,

Quoted 60 lines · Lukasz Majewski

Hi Alberto,
Quoted 11 lines
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 une 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 request to complete.
Serialization by printf.....
Quoted 44 lines
This patch add a way to postpone sending csw after first rockusb_tx_write is completed (indeed inside rockusb_complete) fixing execution of: $ rkdeveloptool rfi
when DEBUG is not defined.
Signed-off-by Alberto Panizzo <[email protected]>
---
 arch/arm/include/asm/arch-rockchip/f_rockusb.h |  1 +
 drivers/usb/gadget/f_rockusb.c                 | 37
++++++++++++++++++++++---- 2 files changed, 33 insertions(+), 5 deletions(-)
(quoted diff) +0 -0
b/arch/arm/include/asm/arch-rockchip/f_rockusb.h index 0b62771..f5cad8e 100644 --- a/arch/arm/include/asm/arch-rockchip/f_rockusb.h +++ b/arch/arm/include/asm/arch-rockchip/f_rockusb.h @@ -124,6 +124,7 @@ struct f_rockusb { int reboot_flag;
arch/arm/include/asm/arch-rockchip/f_rockusb.h +1 -0
 	void *buf;
 	void *buf_head;
+	struct bulk_cs_wrap *next_csw;
 };
 
 /* init rockusb device, tell rockusb which device you want to
read/write*/ diff --git a/drivers/usb/gadget/f_rockusb.c b/drivers/usb/gadget/f_rockusb.c index b8833d0..a39ad51 100644
drivers/usb/gadget/f_rockusb.c +1 -0
@@ -97,6 +97,7 @@ static struct usb_gadget_strings *rkusb_strings[] =
 { 
 static struct f_rockusb *rockusb_func;
 static void rx_handler_command(struct usb_ep *ep, struct usb_request
 *req); +static int rockusb_tx_write(const char *buffer, unsigned int
 buffer_size); static int rockusb_tx_write_csw(u32 tag, int residue,
 u8 status, int size); 
 struct f_rockusb *get_rkusb(void)
@@ -136,11 +137,22 @@ struct usb_endpoint_descriptor *hs)
 
 static void rockusb_complete(struct usb_ep *ep, struct usb_request
 *req) {
+	struct f_rockusb *f_rkusb = get_rkusb();
 	int status = req->status;
 

Quoted 13 lines

drivers/usb/gadget/f_rockusb.c struct usb_endpoint_descriptor *hs) · line 148
Quoted 10 lines
drivers/usb/gadget/f_rockusb.c +6 -3
-	if (!status)
-		return;
-	debug("status: %d ep '%s' trans: %d\n", status, ep->name,
 req->actual);
+	if (status)
+		debug("status: %d ep '%s' trans: %d\n",
+		      status, ep->name, req->actual);
+
+	/* Send Command Status on previous transfer complete */
+	if (f_rkusb->next_csw) {
^^^^^^^^ - isn't this a bit misleading? We send the status for the previous transfer.

This is part of the rockusb protocol: every commands issued by the workstation will have back an optional payload request + this CSW control block.
Commands are serialized by workstation: on rockusb you do not have a real parallel execution of multiple commands.
Thus the simple caching of next CSW block to send is enough to bring back things working.

The other possibility is to have a different _complete function to send back the CSW request as the write_lba is doing.

Quoted 19 lines

Quoted 11 lines
drivers/usb/gadget/f_rockusb.c +7 -0
+#ifdef DEBUG
+		printcsw((char *)f_rkusb->next_csw);
+#endif
+		rockusb_tx_write((char *)f_rkusb->next_csw,
+				 USB_BULK_CS_WRAP_LEN);
+	}
+	f_rkusb->next_csw = NULL;
 }
 
/* config the rockusb device*/
drivers/usb/gadget/f_rockusb.c static int rockusb_tx_write_csw(u32 tag, int · line 403
Quoted 4 lines
drivers/usb/gadget/f_rockusb.c +1 -0
@@ -388,6 +400,21 @@ static int rockusb_tx_write_csw(u32 tag, int
 residue, u8 status, int size) return rockusb_tx_write((char *)csw,
 size); }
 
+struct bulk_cs_wrap g_next_csw;
You have added the pointer to struct bulk_cs_wrap_g to struct f_rockusb, and here we do have global definition.

Quoted 5 lines

Two issues with cache; alignment and padding.
Maybe it would be better to allocate it and store pointer int struct f_rockusb ?

Well, not really because rockusb_tx_write will not use directly our g_next_csw, but copy its data into rockusb_func->in_req->buf which is indeed allocated aligned for DMA.

Style wide I agree to replace the global pointer with alloc/free sequences

Quoted 22 lines

drivers/usb/gadget/f_rockusb.c +13 -1

+static void rockusb_tx_write_csw_on_complete(u32 tag, int residue,
 u8 status) +{
+	struct f_rockusb *f_rkusb = get_rkusb();
+
+	g_next_csw.signature = cpu_to_le32(USB_BULK_CS_SIG);
+	g_next_csw.tag = tag;
+	g_next_csw.residue = cpu_to_be32(residue);
+	g_next_csw.status = status;
+#ifdef DEBUG
+	printcsw((char *)&g_next_csw);
+#endif
+	f_rkusb->next_csw = &g_next_csw;
+}
+
 static unsigned int rx_bytes_expected(struct usb_ep *ep)
 {
 	struct f_rockusb *f_rkusb = get_rkusb();

@@ -501,8 +528,8 @@ static void cb_read_storage_id(struct usb_ep *ep,
 struct usb_request *req) printf("read storage id\n");
 	memcpy((char *)cbw, req->buf, USB_BULK_CB_WRAP_LEN);
 	rockusb_tx_write_str(emmc_id);
-	rockusb_tx_write_csw(cbw->tag, cbw->data_transfer_length,

Quoted 8 lines

drivers/usb/gadget/f_rockusb.c static void cb_read_storage_id(struct usb_ep *ep, · line 534
Quoted 5 lines
drivers/usb/gadget/f_rockusb.c +2 -1
 CSW_GOOD,
-			     USB_BULK_CS_WRAP_LEN);
+	rockusb_tx_write_csw_on_complete(cbw->tag,
 cbw->data_transfer_length,
+					 CSW_GOOD);
It seems like you are preparing the content of the csw structure to be ready when the completion is called. Am I right?

Yes

Quoted 7 lines

What I'm concerned about - with your patch we do have two functions with almost the same code - namely rockusb_tx_write_csw() and rockusb_tx_write_csw_on_complete().
Would it be possible to unify (reuse) the code?

It is possible to reuse more the code yes.

Quoted 2 lines

One more remark - shouldn't we set content of g_next_csw in the rockusb_complete() ?

This no. because CSW cotains data of the generating request (like tag) and status is not something related to USB bus (being able to complete transfer) but status is related to the ability to complete the issued command.
This way if I will construct the csw block on rockusb_complete() function I will need to store anyway somewere the same data generated on rockusb_tx_write_csw_on_complete() function call.

Quoted 6 lines

Quoted 4 lines
drivers/usb/gadget/f_rockusb.c +0 -0
 }
static void cb_write_lba(struct usb_ep *ep, struct usb_request *req)

Best Regards,

Alberto Panizzo

--
Presidente CDA
Amarula Solutions SRL Via le Canevare 30 31100 Treviso Italy
CTO - Co-Founder
Amarula Solutions BV Cruquiuskade 47 Amsterdam 1018 AM The Netherlands
Phone. +31(0)851119171 Fax. +31(0)204106211 www.amarulasolutions.com

Alberto Panizzo

author

2018-07-03 19:02 UTC #4 lore raw

PATCH 2/7 usb: rockchip: implement skeleton for K_FW_GET_CHIP_VER command

In reply to 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 will show: A02341023180002V

Signed-off-by Alberto Panizzo <[email protected]>

---
 drivers/usb/gadget/f_rockusb.c | 38 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 37 insertions(+), 1 deletion(-)

drivers/usb/gadget/f_rockusb.c +37 -1

index a39ad51..7612871 100644
@@ -532,6 +532,42 @@ static void cb_read_storage_id(struct usb_ep *ep, struct usb_request *req)
 					 CSW_GOOD);
 }
 
+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));
+	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 < 16; i++) {
+		int shift = (3 - (i % 4)) * 8;
+
+		printf("%c", (char)((chip_info[i / 4] >> shift) & 0xFF));
+	}
+	printf("\n");
+	memcpy((char *)cbw, req->buf, USB_BULK_CB_WRAP_LEN);
+	rockusb_tx_write((char *)chip_info, sizeof(chip_info));
+	rockusb_tx_write_csw_on_complete(cbw->tag, cbw->data_transfer_length,
+					 CSW_GOOD);
+}
+
 static void cb_write_lba(struct usb_ep *ep, struct usb_request *req)
 {
 	ALLOC_CACHE_ALIGN_BUFFER(struct fsg_bulk_cb_wrap, cbw,

@@ -670,7 +706,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,

Signature · 1 line

2.7.4

Lukasz Majewski

2018-07-03 21:33 UTC #5 lore raw

PATCH 2/7 usb: rockchip: implement skeleton for K_FW_GET_CHIP_VER command

Hi Alberto,

Quoted 52 lines

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

Signed-off-by Alberto Panizzo <[email protected]>

---
 drivers/usb/gadget/f_rockusb.c | 38

+++++++++++++++++++++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-)

(quoted diff) +0 -0

b/drivers/usb/gadget/f_rockusb.c index a39ad51..7612871 100644

drivers/usb/gadget/f_rockusb.c +14 -0

@@ -532,6 +532,42 @@ static void cb_read_storage_id(struct usb_ep
 *ep, struct usb_request *req) CSW_GOOD);
 }
 
+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));
+	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

drivers/usb/gadget/f_rockusb.c +5 -0

+	 *
+	 * Ex for rk3288: 0x33323041 0x32303134 0x30383133 0x56323030
+	 * which brings:  320A20140813V200
+	 *
+	 * Note that memory version do invert MSB/LSB so printing

the char

drivers/usb/gadget/f_rockusb.c +7 -0

+	 * buffer will show: A02341023180002V
+	 */
+	printf("read chip version: ");
+	for (i = 0; i < 16; i++) {
+		int shift = (3 - (i % 4)) * 8;
+
+		printf("%c", (char)((chip_info[i / 4] >> shift) &

Quoted 1 line

0xFF));

A lot of magic numbers. Just to ask - isn't this the same type of conversion as we got with the network code?

Cannot we have simple macro (or static inline) with byte swap called three times?

drivers/usb/gadget/f_rockusb.c static void cb_read_storage_id(struct usb_ep *ep, struct usb_request *req) · line 566

Quoted 4 lines

drivers/usb/gadget/f_rockusb.c +4 -0

+	}
+	printf("\n");
+	memcpy((char *)cbw, req->buf, USB_BULK_CB_WRAP_LEN);
+	rockusb_tx_write((char *)chip_info, sizeof(chip_info));

        ^---- [1]

drivers/usb/gadget/f_rockusb.c

Quoted 1 line

drivers/usb/gadget/f_rockusb.c +1 -0
+	rockusb_tx_write_csw_on_complete(cbw->tag,

        ^-----[2]

drivers/usb/gadget/f_rockusb.c static void cb_read_storage_id(struct usb_ep *ep, struct usb_request *req) · line 568

Quoted 2 lines

cbw->data_transfer_length,
drivers/usb/gadget/f_rockusb.c +1 -0
+					 CSW_GOOD);

Just to be sure if I understand the protocol -> you write the data in
[1]
And then immediately you prepare next block (structure) [2] to be written back after receiving reply data from host?

Is this behaviour in sync with README in ./doc/README.rockusb ?

Quoted 6 lines

drivers/usb/gadget/f_rockusb.c +2 -0

+}
+
 static void cb_write_lba(struct usb_ep *ep, struct usb_request *req)
 {
 	ALLOC_CACHE_ALIGN_BUFFER(struct fsg_bulk_cb_wrap, cbw,

drivers/usb/gadget/f_rockusb.c static const struct cmd_dispatch_info · line 712

Quoted 8 lines

drivers/usb/gadget/f_rockusb.c +1 -1

@@ -670,7 +706,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,

Best regards,

Lukasz Majewski

Alberto Panizzo

author

2018-07-04 13:27 UTC #6 lore raw

PATCH 2/7 usb: rockchip: implement skeleton for K_FW_GET_CHIP_VER command

Quoted 59 lines · Lukasz Majewski

Hi Alberto,
Quoted 52 lines
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
Signed-off-by Alberto Panizzo <[email protected]>
---
 drivers/usb/gadget/f_rockusb.c | 38
+++++++++++++++++++++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-)
(quoted diff) +0 -0
b/drivers/usb/gadget/f_rockusb.c index a39ad51..7612871 100644
drivers/usb/gadget/f_rockusb.c +14 -0
@@ -532,6 +532,42 @@ static void cb_read_storage_id(struct usb_ep
 *ep, struct usb_request *req) CSW_GOOD);
 }
 
+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));
+	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
drivers/usb/gadget/f_rockusb.c +5 -0
+	 *
+	 * Ex for rk3288: 0x33323041 0x32303134 0x30383133 0x56323030
+	 * which brings:  320A20140813V200
+	 *
+	 * Note that memory version do invert MSB/LSB so printing
the char
drivers/usb/gadget/f_rockusb.c +7 -0
+	 * buffer will show: A02341023180002V
+	 */
+	printf("read chip version: ");
+	for (i = 0; i < 16; i++) {
+		int shift = (3 - (i % 4)) * 8;
+
+		printf("%c", (char)((chip_info[i / 4] >> shift) &
Quoted 1 line
0xFF));
A lot of magic numbers. Just to ask - isn't this the same type of conversion as we got with the network code?

Quoted 3 lines

Cannot we have simple macro (or static inline) with byte swap called three times?

Will doublecheck but looks possible.

Quoted 9 lines

drivers/usb/gadget/f_rockusb.c static void cb_read_storage_id(struct usb_ep *ep, struct usb_request *req) · line 566
Quoted 4 lines
drivers/usb/gadget/f_rockusb.c +4 -0
+	}
+	printf("\n");
+	memcpy((char *)cbw, req->buf, USB_BULK_CB_WRAP_LEN);
+	rockusb_tx_write((char *)chip_info, sizeof(chip_info));
        ^---- [1]
drivers/usb/gadget/f_rockusb.c
Quoted 1 line
drivers/usb/gadget/f_rockusb.c +1 -0
+	rockusb_tx_write_csw_on_complete(cbw->tag,
        ^-----[2]

Quoted 7 lines

drivers/usb/gadget/f_rockusb.c static void cb_read_storage_id(struct usb_ep *ep, struct usb_request *req) · line 568
Quoted 2 lines
cbw->data_transfer_length,
drivers/usb/gadget/f_rockusb.c +1 -0
+					 CSW_GOOD);
Just to be sure if I understand the protocol -> you write the data in
[1]
And then immediately you prepare next block (structure) [2] to be written back after receiving reply data from host?

No, Host just waits for a second message as CSW.
What I did with first patch was to serialize sending of both USB messages, so that sending the second would not voiding the first.

Quoted 3 lines

Is this behaviour in sync with README in ./doc/README.rockusb ?

You are right, I completely missed to update doc/README.rockusb

Thanks,

Best Regards,
Alberto Panizzo

Quoted 27 lines

Quoted 6 lines
drivers/usb/gadget/f_rockusb.c +2 -0
+}
+
 static void cb_write_lba(struct usb_ep *ep, struct usb_request *req)
 {
 	ALLOC_CACHE_ALIGN_BUFFER(struct fsg_bulk_cb_wrap, cbw,
drivers/usb/gadget/f_rockusb.c static const struct cmd_dispatch_info · line 712
Quoted 8 lines
drivers/usb/gadget/f_rockusb.c +1 -1
@@ -670,7 +706,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,
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de

Alberto Panizzo

author

2018-07-03 19:02 UTC #7 lore raw

PATCH 3/7 rockchip: rk3288: implement reading chip version from bootrom code

In reply to 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 <[email protected]>

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

arch/arm/mach-rockchip/rk3288/Makefile +1 -0

index a0033a0..da0eb4a 100644
@@ -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

arch/arm/mach-rockchip/rk3288/rockusb_rk3288.c new file +30 -0

new file mode 100644
index 0000000..62057c1
@@ -0,0 +1,30 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2018 Amarula Solutions
+ * Written by Alberto Panizzo <[email protected]>
+ */
+
+#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;
+}

Signature · 1 line

2.7.4

Alberto Panizzo

author

2018-07-03 19:02 UTC #8 lore raw

PATCH 4/7 usb: rockchip: implement K_FW_LBA_READ_10 command

In reply to Alberto Panizzo

It is now possible to read from block device al logic layer. Corresponding command on workstation is:
rkdeveloptool rl <start_blk> <blk_cnt> <file>

Signed-off-by Alberto Panizzo <[email protected]>

---
 arch/arm/include/asm/arch-rockchip/f_rockusb.h |   2 +
 drivers/usb/gadget/f_rockusb.c                 | 102 ++++++++++++++++++++++++-
 2 files changed, 103 insertions(+), 1 deletion(-)

arch/arm/include/asm/arch-rockchip/f_rockusb.h +2 -0

index f5cad8e..f04d401 100644
@@ -120,6 +120,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;

drivers/usb/gadget/f_rockusb.c +101 -1

126 lines not on this page · raw message

Signature · 1 line

2.7.4

Lukasz Majewski

2018-07-03 21:42 UTC #9 lore raw

PATCH 4/7 usb: rockchip: implement K_FW_LBA_READ_10 command

Hi Alberto,

Quoted 1 line

It is now possible to read from block device al logic layer.

						  ^^^^^^^ - what do you
						  mean by logic layer?

I suppose this code is to read N blocks (512B) from eMMC device?

NOTE:

Please consider adding tests into test/py/tests/ as we already have such tests for test_ums.py and test_dfu.py

Quoted 16 lines

Corresponding command on workstation is:
rkdeveloptool rl <start_blk> <blk_cnt> <file>
Signed-off-by Alberto Panizzo <[email protected]>
---
 arch/arm/include/asm/arch-rockchip/f_rockusb.h |   2 +
 drivers/usb/gadget/f_rockusb.c                 | 102
++++++++++++++++++++++++- 2 files changed, 103 insertions(+), 1 deletion(-)
(quoted diff) +0 -0
b/arch/arm/include/asm/arch-rockchip/f_rockusb.h index f5cad8e..f04d401 100644 --- a/arch/arm/include/asm/arch-rockchip/f_rockusb.h +++ b/arch/arm/include/asm/arch-rockchip/f_rockusb.h @@ -120,6 +120,8 @@ struct f_rockusb { unsigned int lba;

arch/arm/include/asm/arch-rockchip/f_rockusb.h struct f_rockusb { · line 124

Quoted 4 lines

arch/arm/include/asm/arch-rockchip/f_rockusb.h +2 -0

 	unsigned int dl_size;
 	unsigned int dl_bytes;
+	unsigned int ul_size;
+	unsigned int ul_bytes;

We had similar problem with Samsung's THOR. unsigned int may be too little in a while (we got int) ...

Quoted 16 lines

arch/arm/include/asm/arch-rockchip/f_rockusb.h +0 -0

 	struct blk_desc *desc;
 	int reboot_flag;
 	void *buf;

(quoted diff) +0 -0

b/drivers/usb/gadget/f_rockusb.c index 7612871..dbf31cb 100644

drivers/usb/gadget/f_rockusb.c +1 -0

@@ -340,6 +340,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)

drivers/usb/gadget/f_rockusb.c static unsigned int rx_bytes_expected(struct · line 441

Quoted 7 lines

drivers/usb/gadget/f_rockusb.c +3 -0

@@ -434,6 +435,79 @@ 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) +{
+#define RBUFFER_SIZE	4096

This can be moved to header file.

Quoted 111 lines

drivers/usb/gadget/f_rockusb.c +25 -0

+	ALLOC_CACHE_ALIGN_BUFFER(char, rbuffer, RBUFFER_SIZE);
+	struct f_rockusb *f_rkusb = get_rkusb();
+	struct usb_request *in_req = rockusb_func->in_req;
+	int ret;
+
+	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;
+		}
+	}
+
+	/* 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 */

drivers/usb/gadget/f_rockusb.c +11 -0

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

drivers/usb/gadget/f_rockusb.c +6 -0

+	if (transfer_size > RBUFFER_SIZE)
+		transfer_size = RBUFFER_SIZE;
+	/* Read at least one block */
+	unsigned int blkcount = (transfer_size + 511) / 512;
+
+	debug("ul %x bytes, %x blks, read lba %x, ul_size:%x,

ul_bytes:%x, ",

drivers/usb/gadget/f_rockusb.c +4 -0

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

drivers/usb/gadget/f_rockusb.c +21 -0

+	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) {

drivers/usb/gadget/f_rockusb.c +26 -0

@@ -568,6 +642,32 @@ static void cb_get_chip_version(struct usb_ep
 *ep, struct usb_request *req) CSW_GOOD);
 }
 
+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->lba = get_unaligned_be32(&cbw->CDB[2]);
+	f_rkusb->ul_size = sector_count * 512;
+	f_rkusb->ul_bytes = 0;
+	f_rkusb->tag = cbw->tag;
+	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)
 {

drivers/usb/gadget/f_rockusb.c +0 -0

 	ALLOC_CACHE_ALIGN_BUFFER(struct fsg_bulk_cb_wrap, cbw,

drivers/usb/gadget/f_rockusb.c static const struct cmd_dispatch_info · line 781

Quoted 5 lines

drivers/usb/gadget/f_rockusb.c +1 -1

@@ -678,7 +778,7 @@ static const struct cmd_dispatch_info
 cmd_dispatch_info[] = { },
 	{
 		.cmd = K_FW_LBA_READ_10,
-		.cb = cb_not_support,
+		.cb = cb_read_lba,

Please add entry explaining this part of the protocol into README.rockusb

drivers/usb/gadget/f_rockusb.c static const struct cmd_dispatch_info cmd_dispatch_info[] = { · line 784

Quoted 3 lines

drivers/usb/gadget/f_rockusb.c +0 -0
 	},
 	{
 		.cmd = K_FW_LBA_WRITE_10,

Best regards,

Lukasz Majewski

Alberto Panizzo

author

2018-07-04 13:36 UTC #10 lore raw

PATCH 4/7 usb: rockchip: implement K_FW_LBA_READ_10 command

Hi Lukasz,

Quoted 6 lines · Lukasz Majewski

Hi Alberto,
Quoted 1 line
It is now possible to read from block device al logic layer.
						  ^^^^^^^ - what do you
						  mean by logic layer?

This does refer to rockusb nomenclature: where with rkdeveloptool from workstation we use the "rl" command using LBA addresses.

I'll rephrase.

Quoted 6 lines

I suppose this code is to read N blocks (512B) from eMMC device?
NOTE:
Please consider adding tests into test/py/tests/ as we already have such tests for test_ums.py and test_dfu.py

I'll try, it's on my TODO

Quoted 162 lines

Quoted 17 lines

Corresponding command on workstation is:
rkdeveloptool rl <start_blk> <blk_cnt> <file>
Signed-off-by Alberto Panizzo <[email protected]>
---
 arch/arm/include/asm/arch-rockchip/f_rockusb.h |   2 +
 drivers/usb/gadget/f_rockusb.c                 | 102
++++++++++++++++++++++++- 2 files changed, 103 insertions(+), 1 deletion(-)
(quoted diff) +0 -0
b/arch/arm/include/asm/arch-rockchip/f_rockusb.h index f5cad8e..f04d401 100644 --- a/arch/arm/include/asm/arch-rockchip/f_rockusb.h +++ b/arch/arm/include/asm/arch-rockchip/f_rockusb.h @@ -120,6 +120,8 @@ struct f_rockusb { unsigned int lba;

arch/arm/include/asm/arch-rockchip/f_rockusb.h struct f_rockusb { · line 124

Quoted 4 lines

arch/arm/include/asm/arch-rockchip/f_rockusb.h +2 -0

 	unsigned int dl_size;
 	unsigned int dl_bytes;
+	unsigned int ul_size;
+	unsigned int ul_bytes;

We had similar problem with Samsung's THOR. unsigned int may be too little in a while (we got int) ...

Quoted 16 lines

arch/arm/include/asm/arch-rockchip/f_rockusb.h +0 -0

 	struct blk_desc *desc;
 	int reboot_flag;
 	void *buf;

(quoted diff) +0 -0

b/drivers/usb/gadget/f_rockusb.c index 7612871..dbf31cb 100644

drivers/usb/gadget/f_rockusb.c +1 -0

@@ -340,6 +340,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)

drivers/usb/gadget/f_rockusb.c static unsigned int rx_bytes_expected(struct · line 441

Quoted 7 lines

drivers/usb/gadget/f_rockusb.c +3 -0

@@ -434,6 +435,79 @@ 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) +{
+#define RBUFFER_SIZE	4096

This can be moved to header file.

Quoted 111 lines

drivers/usb/gadget/f_rockusb.c +25 -0

+	ALLOC_CACHE_ALIGN_BUFFER(char, rbuffer, RBUFFER_SIZE);
+	struct f_rockusb *f_rkusb = get_rkusb();
+	struct usb_request *in_req = rockusb_func->in_req;
+	int ret;
+
+	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;
+		}
+	}
+
+	/* 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 */

drivers/usb/gadget/f_rockusb.c +11 -0

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

drivers/usb/gadget/f_rockusb.c +6 -0

+	if (transfer_size > RBUFFER_SIZE)
+		transfer_size = RBUFFER_SIZE;
+	/* Read at least one block */
+	unsigned int blkcount = (transfer_size + 511) / 512;
+
+	debug("ul %x bytes, %x blks, read lba %x, ul_size:%x,

ul_bytes:%x, ",

drivers/usb/gadget/f_rockusb.c +4 -0

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

drivers/usb/gadget/f_rockusb.c +21 -0

+	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) {

drivers/usb/gadget/f_rockusb.c +26 -0

@@ -568,6 +642,32 @@ static void cb_get_chip_version(struct usb_ep
 *ep, struct usb_request *req) CSW_GOOD);
 }
 
+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->lba = get_unaligned_be32(&cbw->CDB[2]);
+	f_rkusb->ul_size = sector_count * 512;
+	f_rkusb->ul_bytes = 0;
+	f_rkusb->tag = cbw->tag;
+	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)
 {

drivers/usb/gadget/f_rockusb.c +0 -0

 	ALLOC_CACHE_ALIGN_BUFFER(struct fsg_bulk_cb_wrap, cbw,

@@ -678,7 +778,7 @@ static const struct cmd_dispatch_info

Quoted 9 lines

drivers/usb/gadget/f_rockusb.c static const struct cmd_dispatch_info cmd_dispatch_info[] = { · line 781
Quoted 5 lines
drivers/usb/gadget/f_rockusb.c +1 -1
 cmd_dispatch_info[] = { },
 	{
 		.cmd = K_FW_LBA_READ_10,
-		.cb = cb_not_support,
+		.cb = cb_read_lba,
Please add entry explaining this part of the protocol into README.rockusb

Yes.

Best Regards,

Alberto Panizzo

Quoted 14 lines

drivers/usb/gadget/f_rockusb.c static const struct cmd_dispatch_info cmd_dispatch_info[] = { · line 784
Quoted 3 lines
drivers/usb/gadget/f_rockusb.c +0 -0
 	},
 	{
 		.cmd = K_FW_LBA_WRITE_10,
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de

Kever Yang

2018-07-05 01:19 UTC #11 lore raw

PATCH 4/7 usb: rockchip: implement K_FW_LBA_READ_10 command

In reply to Alberto Panizzo

Hi Alberto,

Quoted 36 lines · Alberto Panizzo

It is now possible to read from block device al logic layer. Corresponding command on workstation is:
rkdeveloptool rl <start_blk> <blk_cnt> <file>

Signed-off-by Alberto Panizzo <[email protected]>

---
 arch/arm/include/asm/arch-rockchip/f_rockusb.h |   2 +
 drivers/usb/gadget/f_rockusb.c                 | 102 ++++++++++++++++++++++++-
 2 files changed, 103 insertions(+), 1 deletion(-)

arch/arm/include/asm/arch-rockchip/f_rockusb.h +2 -0

index f5cad8e..f04d401 100644
@@ -120,6 +120,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;

drivers/usb/gadget/f_rockusb.c +1 -0

index 7612871..dbf31cb 100644
@@ -340,6 +340,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)

drivers/usb/gadget/f_rockusb.c static unsigned int rx_bytes_expected(struct usb_ep *ep) · line 439

Quoted 5 lines

drivers/usb/gadget/f_rockusb.c +2 -0

@@ -434,6 +435,79 @@ 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)

Could you use another name like '_rl_' instead of '_ul_', and since there is a prefix 'tx',
maybe we don't need this '_ul_'? Because there is a rockusb cmd 'UL' or 'ul' means
'upgrade loader'.

drivers/usb/gadget/f_rockusb.c static unsigned int rx_bytes_expected(struct usb_ep *ep) · line 441

Quoted 2 lines

drivers/usb/gadget/f_rockusb.c +2 -0
+{
+#define RBUFFER_SIZE	4096

You can use the same size for TX and RX buffer.

BTW: I don't like the design of the return value here, there should always have return
value instead of void, many error cat may happen during the TX.

Thanks,
- Kever

Quoted 114 lines

drivers/usb/gadget/f_rockusb.c +96 -1

+	ALLOC_CACHE_ALIGN_BUFFER(char, rbuffer, RBUFFER_SIZE);
+	struct f_rockusb *f_rkusb = get_rkusb();
+	struct usb_request *in_req = rockusb_func->in_req;
+	int ret;
+
+	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;
+		}
+	}
+
+	/* 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 > RBUFFER_SIZE)
+		transfer_size = RBUFFER_SIZE;
+	/* Read at least one block */
+	unsigned int blkcount = (transfer_size + 511) / 512;
+
+	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)
 {

@@ -568,6 +642,32 @@ static void cb_get_chip_version(struct usb_ep *ep, struct usb_request *req)
 					 CSW_GOOD);
 }
 
+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->lba = get_unaligned_be32(&cbw->CDB[2]);
+	f_rkusb->ul_size = sector_count * 512;
+	f_rkusb->ul_bytes = 0;
+	f_rkusb->tag = cbw->tag;
+	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,

@@ -678,7 +778,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,

Alberto Panizzo

author

2018-07-05 08:52 UTC #12 lore raw

PATCH 4/7 usb: rockchip: implement K_FW_LBA_READ_10 command

Hi Kever,

Quoted 44 lines · Kever Yang

Hi Alberto,

Quoted 39 lines · Alberto Panizzo

It is now possible to read from block device al logic layer. Corresponding command on workstation is:
rkdeveloptool rl <start_blk> <blk_cnt> <file>

Signed-off-by Alberto Panizzo <[email protected]>

---
 arch/arm/include/asm/arch-rockchip/f_rockusb.h |   2 +
 drivers/usb/gadget/f_rockusb.c                 | 102 ++++++++++++++++++++++++-
 2 files changed, 103 insertions(+), 1 deletion(-)

arch/arm/include/asm/arch-rockchip/f_rockusb.h +2 -0

index f5cad8e..f04d401 100644
@@ -120,6 +120,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;

drivers/usb/gadget/f_rockusb.c +1 -0

index 7612871..dbf31cb 100644
@@ -340,6 +340,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)

@@ -434,6 +435,79 @@ static unsigned int rx_bytes_expected(struct usb_ep *ep)
      return rx_remain;
 }

Quoted 8 lines

drivers/usb/gadget/f_rockusb.c static unsigned int rx_bytes_expected(struct usb_ep *ep) · line 439
Quoted 2 lines
drivers/usb/gadget/f_rockusb.c +2 -0
+/* usb_request complete call back to handle upload image */
+static void tx_handler_ul_image(struct usb_ep *ep, struct usb_request *req)
Could you use another name like '_rl_' instead of '_ul_', and since there is a prefix 'tx',
maybe we don't need this '_ul_'? Because there is a rockusb cmd 'UL' or 'ul' means
'upgrade loader'.

Name has been chosen following the sibling rx_handler_dl_image function name:
tx_handler_ul_image is
- a transfer -> tx_
- a complete handler -> handler
- it does upload data to the usb host -> ul_image

I know upgrade_tool/rkdeveloptool do have ul command but this is a totally different context

Quoted 9 lines

drivers/usb/gadget/f_rockusb.c static unsigned int rx_bytes_expected(struct usb_ep *ep) · line 441
Quoted 4 lines
drivers/usb/gadget/f_rockusb.c +2 -0
+{
+#define RBUFFER_SIZE 4096
You can use the same size for TX and RX buffer.
BTW: I don't like the design of the return value here, there should always have return
value instead of void, many error cat may happen during the TX.

This is a request complete handler, so function arguments and return value cannot be
different than the proposed.

Errors have to be reported to the host with a CSW_FAIL message and managed here on target switching back to the default complete handler.

Best Regards,
Alberto Panizzo

--
Amarula Solutions SRL Via le Canevare 30 31100 Treviso Italy
Amarula Solutions BV Cruquiuskade 47 Amsterdam 1018 AM The Netherlands
Phone. +31(0)851119171 Fax. +31(0)204106211 www.amarulasolutions.com

Quoted 119 lines

Thanks,
- Kever

Quoted 114 lines

drivers/usb/gadget/f_rockusb.c +96 -1

+     ALLOC_CACHE_ALIGN_BUFFER(char, rbuffer, RBUFFER_SIZE);
+     struct f_rockusb *f_rkusb = get_rkusb();
+     struct usb_request *in_req = rockusb_func->in_req;
+     int ret;
+
+     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;
+             }
+     }
+
+     /* 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 > RBUFFER_SIZE)
+             transfer_size = RBUFFER_SIZE;
+     /* Read at least one block */
+     unsigned int blkcount = (transfer_size + 511) / 512;
+
+     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)
 {

@@ -568,6 +642,32 @@ static void cb_get_chip_version(struct usb_ep *ep, struct usb_request *req)
                                       CSW_GOOD);
 }
 
+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->lba = get_unaligned_be32(&cbw->CDB[2]);
+     f_rkusb->ul_size = sector_count * 512;
+     f_rkusb->ul_bytes = 0;
+     f_rkusb->tag = cbw->tag;
+     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,

@@ -678,7 +778,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,

Alberto Panizzo

author

2018-07-03 19:02 UTC #13 lore raw

PATCH 5/7 usb: rockchip: implement K_FW_LBA_ERASE_10 command

In reply to Alberto Panizzo

This command is part of the write LBA sector sequence performed by rkdeveloptool

Signed-off-by Alberto Panizzo <[email protected]>

---
 arch/arm/include/asm/arch-rockchip/f_rockusb.h |  1 +
 drivers/usb/gadget/f_rockusb.c                 | 46 ++++++++++++++++++++++++++
 2 files changed, 47 insertions(+)

arch/arm/include/asm/arch-rockchip/f_rockusb.h +1 -0

index f04d401..77c3a87 100644
@@ -62,6 +62,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,

drivers/usb/gadget/f_rockusb.c +46 -0

58 lines not on this page · raw message

Signature · 1 line

2.7.4

Lukasz Majewski

2018-07-03 21:47 UTC #14 lore raw

PATCH 5/7 usb: rockchip: implement K_FW_LBA_ERASE_10 command

Hi Alberto,

Quoted 2 lines

This command is part of the write LBA sector sequence performed by rkdeveloptool

You mentioned the LBA sector and the need of erasing it.

AFAIK, block devices (eMMC) perform internally erase on write.

Is this protocol also supposed to work with NAND flash? If yes, please adjust the patch description.

Also, please extend the README.rockusb file.

Quoted 75 lines

Signed-off-by Alberto Panizzo <[email protected]>

---
 arch/arm/include/asm/arch-rockchip/f_rockusb.h |  1 +
 drivers/usb/gadget/f_rockusb.c                 | 46

++++++++++++++++++++++++++ 2 files changed, 47 insertions(+)

(quoted diff) +0 -0

b/arch/arm/include/asm/arch-rockchip/f_rockusb.h index f04d401..77c3a87 100644 --- a/arch/arm/include/asm/arch-rockchip/f_rockusb.h +++ b/arch/arm/include/asm/arch-rockchip/f_rockusb.h @@ -62,6 +62,7 @@ K_FW_LOW_FORMAT = 0x1C, K_FW_SET_RESET_FLAG = 0x1E,
K_FW_SPI_READ_10 = 0x21,
K_FW_SPI_WRITE_10 = 0x22,

arch/arm/include/asm/arch-rockchip/f_rockusb.h +1 -0

+K_FW_LBA_ERASE_10 = 0x25,
 
 K_FW_SESSION = 0X30,
 K_FW_RESET = 0xff,

(quoted diff) +0 -0

b/drivers/usb/gadget/f_rockusb.c index dbf31cb..230fbec 100644

drivers/usb/gadget/f_rockusb.c +42 -0

@@ -693,6 +693,48 @@ 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]);
+	lba = get_unaligned_be32(&cbw->CDB[2]);
+	f_rkusb->tag = cbw->tag;
+	debug("require erase %x sectors from lba %x\n",
+	      sector_count, lba);
+
+	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,
+				cbw->data_transfer_length, CSW_FAIL,
+				USB_BULK_CS_WRAP_LEN);
+			return;
+		}
+	}
+	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)

{

drivers/usb/gadget/f_rockusb.c +0 -0

 	struct f_rockusb *f_rkusb = get_rkusb();

drivers/usb/gadget/f_rockusb.c static const struct cmd_dispatch_info · line 876

Quoted 10 lines

drivers/usb/gadget/f_rockusb.c +4 -0

@@ -825,6 +867,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,
 	},

Best regards,

Lukasz Majewski

Alberto Panizzo

author

2018-07-03 19:02 UTC #15 lore raw

PATCH 6/7 usb: rockchip: be quiet on serial port while transferring data

In reply to Alberto Panizzo

While downloading or uploading megabytes of data we had thousands of dummy lines like:

transfer 0x10000 bytes done
OR
Uploading 0x1000 bytes

even on non-debug builds. This because transfers are chunked and code running on target does not have any clue about when the current chunk is the last one.

Signed-off-by Alberto Panizzo <[email protected]>

---
 drivers/usb/gadget/f_rockusb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

drivers/usb/gadget/f_rockusb.c +2 -2

index 230fbec..4fc8484 100644
@@ -501,7 +501,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)

@@ -563,7 +563,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);

Signature · 1 line

2.7.4

Lukasz Majewski

2018-07-03 21:49 UTC #16 lore raw

PATCH 6/7 usb: rockchip: be quiet on serial port while transferring data

Hi Alberto,

Quoted 10 lines

While downloading or uploading megabytes of data we had thousands of dummy lines like:
transfer 0x10000 bytes done
OR
Uploading 0x1000 bytes
even on non-debug builds. This because transfers are chunked and code running on target does not have any clue about when the current chunk is the last one.

Is there any other indication to show if the board is still working?

Something like '........................' present in fastboot?

Quoted 28 lines

Signed-off-by Alberto Panizzo <[email protected]>

---
 drivers/usb/gadget/f_rockusb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

(quoted diff) +0 -0

b/drivers/usb/gadget/f_rockusb.c index 230fbec..4fc8484 100644

drivers/usb/gadget/f_rockusb.c +2 -2

@@ -501,7 +501,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)

@@ -563,7 +563,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,

Quoted 1 line

USB_BULK_CS_WRAP_LEN);

Best regards,

Lukasz Majewski

Alberto Panizzo

author

2018-07-04 13:44 UTC #17 lore raw

PATCH 6/7 usb: rockchip: be quiet on serial port while transferring data

Hi Lukasz

Quoted 9 lines · Lukasz Majewski

Hi Alberto,
Quoted 7 lines
While downloading or uploading megabytes of data we had thousands of dummy lines like:
transfer 0x10000 bytes done
OR
Uploading 0x1000 bytes

Quoted 8 lines

Quoted 3 lines
even on non-debug builds. This because transfers are chunked and code running on target does not have any clue about when the current chunk is the last one.
Is there any other indication to show if the board is still working?
Something like '........................' present in fastboot?

No there is no other indication on board side, but workstation side will print percentage of complete.
Since chunks are small and all chunks will feedback success or error, on workstation you'll have clear view about board frozen or transfer failed. On board side errors will be prompted.

Fastboot is different: workstation tool do just download data which is buffered in memory and than flashed. Workstation than obtain a result at the end of the process, so an indication about things are proceeding is than really useful.

Best Regards,

Alberto Panizzo

Quoted 42 lines

Quoted 28 lines
Signed-off-by Alberto Panizzo <[email protected]>
---
 drivers/usb/gadget/f_rockusb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
(quoted diff) +0 -0
b/drivers/usb/gadget/f_rockusb.c index 230fbec..4fc8484 100644
drivers/usb/gadget/f_rockusb.c +2 -2
@@ -501,7 +501,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)
@@ -563,7 +563,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,
Quoted 1 line
USB_BULK_CS_WRAP_LEN);
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de

Alberto Panizzo

author

2018-07-03 19:02 UTC #18 lore raw

PATCH 7/7 usb: rockchip: boost up write speed from 4MB/s to 15MB/s

In reply to 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 <[email protected]>

---
 arch/arm/include/asm/arch-rockchip/f_rockusb.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

arch/arm/include/asm/arch-rockchip/f_rockusb.h +1 -1

index 77c3a87..41c6188 100644
@@ -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

Signature · 1 line

2.7.4

Kever Yang

2018-07-05 01:15 UTC #19 lore raw

cover Improve rockusb support in U-Boot

In reply to Alberto Panizzo

Hi Alberto,

Thanks for your patches, and I'm so glad for people using rockusb and try to improve it.

You can reference to rockchip source code here:
https://github.com/rockchip-linux/u-boot/blob/release/drivers/usb/gadget/f_rockusb.c

We use msc as base framework instead of dfu because of the big performance improvement, and the cmd handling part will be the same.

PS: Yes, rockusb is available for all Rockchip's SoCs.

Thanks,
- Kever

Quoted 31 lines · 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
Alberto Panizzo (7):
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
 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 ++++
 drivers/usb/gadget/f_rockusb.c                 | 225 ++++++++++++++++++++++++-
 4 files changed, 253 insertions(+), 9 deletions(-)
 create mode 100644 arch/arm/mach-rockchip/rk3288/rockusb_rk3288.c

Alberto Panizzo

author

2018-07-05 08:39 UTC #20 lore raw

cover Improve rockusb support in U-Boot

Dear Kever,

Quoted 11 lines · Kever Yang

Hi Alberto,
Thanks for your patches, and I'm so glad for people using rockusb
and try to improve it.
You can reference to rockchip source code here:
https://github.com/rockchip-linux/u-boot/blob/release/drivers/usb/gadget/f_rockusb.c
We use msc as base framework instead of dfu because of the big
performance improvement, and the cmd handling part will be the same.

Don't know if injecting rockusb protocol in f_mass_storage.c will be acceptable in terms of coexistence and maintainability of resulting f_mass_storage.c Do you plan to upstream your tree?

I saw patches in your tree and resulting f_rockusb.c is more clear yes. But we pay the price in f_mass_storage.c.

Best Regards,
Alberto Panizzo

Quoted 40 lines

PS: Yes, rockusb is available for all Rockchip's SoCs.
Thanks,
- Kever
Quoted 31 lines · 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
Alberto Panizzo (7):
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
 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 ++++
 drivers/usb/gadget/f_rockusb.c                 | 225 ++++++++++++++++++++++++-
 4 files changed, 253 insertions(+), 9 deletions(-)
 create mode 100644 arch/arm/mach-rockchip/rk3288/rockusb_rk3288.c

Lukasz Majewski

2018-07-05 09:07 UTC #21 lore raw

cover Improve rockusb support in U-Boot

Hi Alberto,

Quoted 13 lines

Dear Kever,
Quoted 9 lines · Kever Yang
Hi Alberto,
Thanks for your patches, and I'm so glad for people using
rockusb and try to improve it.
You can reference to rockchip source code here:
https://github.com/rockchip-linux/u-boot/blob/release/drivers/usb/gadget/f_rockusb.c

Quoted 8 lines

Quoted 3 lines
We use msc as base framework instead of dfu because of the big
performance improvement, and the cmd handling part will be the same.
Don't know if injecting rockusb protocol in f_mass_storage.c will be acceptable in terms of coexistence and maintainability of resulting f_mass_storage.c

I would like to have the rockchip part separate to be included by enabling Kconfig option in u-boot.

Quoted 4 lines

Do you plan to upstream your tree?
I saw patches in your tree and resulting f_rockusb.c is more clear yes. But we pay the price in f_mass_storage.c.

It would be nice to see the code posted on u-boot ML.

Quoted 50 lines

Best Regards,
Alberto Panizzo
--
Amarula Solutions SRL Via le Canevare 30 31100
Treviso Italy Amarula Solutions BV Cruquiuskade 47
Amsterdam 1018 AM The Netherlands Phone. +31(0)851119171 Fax.
+31(0)204106211 www.amarulasolutions.com
Quoted 40 lines
PS: Yes, rockusb is available for all Rockchip's SoCs.
Thanks,
- Kever
Quoted 31 lines · 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
Alberto Panizzo (7):
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
 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 ++++
 drivers/usb/gadget/f_rockusb.c                 | 225
++++++++++++++++++++++++- 4 files changed, 253 insertions(+), 9 deletions(-) create mode 100644 arch/arm/mach-rockchip/rk3288/rockusb_rk3288.c

Best regards,

Lukasz Majewski