[U-Boot] [PATCH 0/7] 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
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 alberto@amarulasolutions.com --- arch/arm/include/asm/arch-rockchip/f_rockusb.h | 1 + drivers/usb/gadget/f_rockusb.c | 37 ++++++++++++++++++++++---- 2 files changed, 33 insertions(+), 5 deletions(-)
diff --git a/arch/arm/include/asm/arch-rockchip/f_rockusb.h 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; 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 --- a/drivers/usb/gadget/f_rockusb.c +++ b/drivers/usb/gadget/f_rockusb.c @@ -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)
Hi Alberto,
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.....
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 alberto@amarulasolutions.com
arch/arm/include/asm/arch-rockchip/f_rockusb.h | 1 + drivers/usb/gadget/f_rockusb.c | 37 ++++++++++++++++++++++---- 2 files changed, 33 insertions(+), 5 deletions(-)
diff --git a/arch/arm/include/asm/arch-rockchip/f_rockusb.h 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; 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 --- a/drivers/usb/gadget/f_rockusb.c +++ b/drivers/usb/gadget/f_rockusb.c @@ -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) {
^^^^^^^^ - isn't this a bit misleading? We send the status for the previous transfer.
+#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;
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 ?
+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);
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() ?
}
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@denx.de
Hi Lukasz,
On Tue, Jul 03, 2018 at 11:24:34PM +0200, Lukasz Majewski wrote:
Hi Alberto,
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.....
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 alberto@amarulasolutions.com
arch/arm/include/asm/arch-rockchip/f_rockusb.h | 1 + drivers/usb/gadget/f_rockusb.c | 37 ++++++++++++++++++++++---- 2 files changed, 33 insertions(+), 5 deletions(-)
diff --git a/arch/arm/include/asm/arch-rockchip/f_rockusb.h 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; 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 --- a/drivers/usb/gadget/f_rockusb.c +++ b/drivers/usb/gadget/f_rockusb.c @@ -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) {
^^^^^^^^ - 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.
+#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;
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 ?
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
+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);It seems like you are preparing the content of the csw structure to be ready when the completion is called. Am I right?
Yes
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.
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.
}
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
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 alberto@amarulasolutions.com --- drivers/usb/gadget/f_rockusb.c | 38 +++++++++++++++++++++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/gadget/f_rockusb.c b/drivers/usb/gadget/f_rockusb.c index a39ad51..7612871 100644 --- a/drivers/usb/gadget/f_rockusb.c +++ b/drivers/usb/gadget/f_rockusb.c @@ -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,
Hi Alberto,
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 alberto@amarulasolutions.com
drivers/usb/gadget/f_rockusb.c | 38 +++++++++++++++++++++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/gadget/f_rockusb.c b/drivers/usb/gadget/f_rockusb.c index a39ad51..7612871 100644 --- a/drivers/usb/gadget/f_rockusb.c +++ b/drivers/usb/gadget/f_rockusb.c @@ -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 spaceLittle Endian
** Ex for rk3288: 0x33323041 0x32303134 0x30383133 0x56323030* which brings: 320A20140813V200** Note that memory version do invert MSB/LSB so printingthe 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));
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?
- }
- printf("\n");
- memcpy((char *)cbw, req->buf, USB_BULK_CB_WRAP_LEN);
- rockusb_tx_write((char *)chip_info, sizeof(chip_info));
^---- [1]
- rockusb_tx_write_csw_on_complete(cbw->tag,
^-----[2]
cbw->data_transfer_length,
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 ?
+}
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,
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@denx.de
On Tue, Jul 03, 2018 at 11:33:45PM +0200, Lukasz Majewski wrote:
Hi Alberto,
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 alberto@amarulasolutions.com
drivers/usb/gadget/f_rockusb.c | 38 +++++++++++++++++++++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/gadget/f_rockusb.c b/drivers/usb/gadget/f_rockusb.c index a39ad51..7612871 100644 --- a/drivers/usb/gadget/f_rockusb.c +++ b/drivers/usb/gadget/f_rockusb.c @@ -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 spaceLittle Endian
** Ex for rk3288: 0x33323041 0x32303134 0x30383133 0x56323030* which brings: 320A20140813V200** Note that memory version do invert MSB/LSB so printingthe 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));
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?
Will doublecheck but looks possible.
- }
- printf("\n");
- memcpy((char *)cbw, req->buf, USB_BULK_CB_WRAP_LEN);
- rockusb_tx_write((char *)chip_info, sizeof(chip_info));
^---- [1]
- rockusb_tx_write_csw_on_complete(cbw->tag,
^-----[2]cbw->data_transfer_length,
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.
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
-- 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
+}
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,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@denx.de
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; +}
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 alberto@amarulasolutions.com --- arch/arm/include/asm/arch-rockchip/f_rockusb.h | 2 + drivers/usb/gadget/f_rockusb.c | 102 ++++++++++++++++++++++++- 2 files changed, 103 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 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; 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/drivers/usb/gadget/f_rockusb.c b/drivers/usb/gadget/f_rockusb.c index 7612871..dbf31cb 100644 --- a/drivers/usb/gadget/f_rockusb.c +++ b/drivers/usb/gadget/f_rockusb.c @@ -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; }
+/* 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 + 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,
Hi Alberto,
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
Corresponding command on workstation is: rkdeveloptool rl <start_blk> <blk_cnt> <file>
Signed-off-by: Alberto Panizzo alberto@amarulasolutions.com
arch/arm/include/asm/arch-rockchip/f_rockusb.h | 2 + drivers/usb/gadget/f_rockusb.c | 102 ++++++++++++++++++++++++- 2 files changed, 103 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 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; 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) ...
struct blk_desc *desc; int reboot_flag; void *buf; diff --git a/drivers/usb/gadget/f_rockusb.c b/drivers/usb/gadget/f_rockusb.c index 7612871..dbf31cb 100644 --- a/drivers/usb/gadget/f_rockusb.c +++ b/drivers/usb/gadget/f_rockusb.c @@ -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; }
+/* 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.
- 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,
Please add entry explaining this part of the protocol into README.rockusb
}, { .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@denx.de
Hi Lukasz,
On Tue, Jul 03, 2018 at 11:42:11PM +0200, Lukasz Majewski wrote:
Hi Alberto,
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.
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
Corresponding command on workstation is: rkdeveloptool rl <start_blk> <blk_cnt> <file>
Signed-off-by: Alberto Panizzo alberto@amarulasolutions.com
arch/arm/include/asm/arch-rockchip/f_rockusb.h | 2 + drivers/usb/gadget/f_rockusb.c | 102 ++++++++++++++++++++++++- 2 files changed, 103 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 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; 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) ...
struct blk_desc *desc; int reboot_flag; void *buf; diff --git a/drivers/usb/gadget/f_rockusb.c b/drivers/usb/gadget/f_rockusb.c index 7612871..dbf31cb 100644 --- a/drivers/usb/gadget/f_rockusb.c +++ b/drivers/usb/gadget/f_rockusb.c @@ -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; }
+/* 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.
- 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,Please add entry explaining this part of the protocol into README.rockusb
Yes.
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
}, { .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@denx.de
Hi Alberto,
On 07/04/2018 03:02 AM, Alberto Panizzo wrote:
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 alberto@amarulasolutions.com
arch/arm/include/asm/arch-rockchip/f_rockusb.h | 2 + drivers/usb/gadget/f_rockusb.c | 102 ++++++++++++++++++++++++- 2 files changed, 103 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 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; 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/drivers/usb/gadget/f_rockusb.c b/drivers/usb/gadget/f_rockusb.c index 7612871..dbf31cb 100644 --- a/drivers/usb/gadget/f_rockusb.c +++ b/drivers/usb/gadget/f_rockusb.c @@ -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; }
+/* 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'.
+{ +#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
- 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,
Hi Kever,
On Thu, Jul 5, 2018 at 3:19 AM, Kever Yang kever.yang@rock-chips.com wrote:
Hi Alberto,
On 07/04/2018 03:02 AM, Alberto Panizzo wrote:
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 alberto@amarulasolutions.com
arch/arm/include/asm/arch-rockchip/f_rockusb.h | 2 + drivers/usb/gadget/f_rockusb.c | 102 ++++++++++++++++++++++++- 2 files changed, 103 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 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; 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/drivers/usb/gadget/f_rockusb.c b/drivers/usb/gadget/f_rockusb.c index 7612871..dbf31cb 100644 --- a/drivers/usb/gadget/f_rockusb.c +++ b/drivers/usb/gadget/f_rockusb.c @@ -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; }
+/* 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
+{ +#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
Thanks,
- Kever
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,
This command is part of the write LBA sector sequence performed by rkdeveloptool
Signed-off-by: Alberto Panizzo alberto@amarulasolutions.com --- arch/arm/include/asm/arch-rockchip/f_rockusb.h | 1 + drivers/usb/gadget/f_rockusb.c | 46 ++++++++++++++++++++++++++ 2 files changed, 47 insertions(+)
diff --git a/arch/arm/include/asm/arch-rockchip/f_rockusb.h 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, +K_FW_LBA_ERASE_10 = 0x25,
K_FW_SESSION = 0X30, K_FW_RESET = 0xff, diff --git a/drivers/usb/gadget/f_rockusb.c b/drivers/usb/gadget/f_rockusb.c index dbf31cb..230fbec 100644 --- a/drivers/usb/gadget/f_rockusb.c +++ b/drivers/usb/gadget/f_rockusb.c @@ -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) { struct f_rockusb *f_rkusb = get_rkusb(); @@ -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, },
Hi Alberto,
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.
Signed-off-by: Alberto Panizzo alberto@amarulasolutions.com
arch/arm/include/asm/arch-rockchip/f_rockusb.h | 1 + drivers/usb/gadget/f_rockusb.c | 46 ++++++++++++++++++++++++++ 2 files changed, 47 insertions(+)
diff --git a/arch/arm/include/asm/arch-rockchip/f_rockusb.h 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, +K_FW_LBA_ERASE_10 = 0x25,
K_FW_SESSION = 0X30, K_FW_RESET = 0xff, diff --git a/drivers/usb/gadget/f_rockusb.c b/drivers/usb/gadget/f_rockusb.c index dbf31cb..230fbec 100644 --- a/drivers/usb/gadget/f_rockusb.c +++ b/drivers/usb/gadget/f_rockusb.c @@ -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) { struct f_rockusb *f_rkusb = get_rkusb(); @@ -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
--
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@denx.de
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 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 230fbec..4fc8484 100644 --- a/drivers/usb/gadget/f_rockusb.c +++ b/drivers/usb/gadget/f_rockusb.c @@ -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);
Hi Alberto,
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?
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 230fbec..4fc8484 100644 --- a/drivers/usb/gadget/f_rockusb.c +++ b/drivers/usb/gadget/f_rockusb.c @@ -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);
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@denx.de
Hi Lukasz On Tue, Jul 03, 2018 at 11:49:16PM +0200, Lukasz Majewski wrote:
Hi Alberto,
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?
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
-- 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
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 230fbec..4fc8484 100644 --- a/drivers/usb/gadget/f_rockusb.c +++ b/drivers/usb/gadget/f_rockusb.c @@ -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);
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@denx.de
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 77c3a87..41c6188 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
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_r...
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 On 07/04/2018 03:02 AM, Alberto Panizzo wrote:
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
Dear Kever,
On Thu, Jul 5, 2018 at 3:15 AM, Kever Yang kever.yang@rock-chips.com wrote:
Hi Alberto,
Thanks for your patches, and I'm so glad for people using rockusband 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_r...
We use msc as base framework instead of dfu because of the bigperformance 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
-- 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
PS: Yes, rockusb is available for all Rockchip's SoCs.
Thanks,
- Kever
On 07/04/2018 03:02 AM, Alberto Panizzo wrote:
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
Hi Alberto,
Dear Kever,
On Thu, Jul 5, 2018 at 3:15 AM, Kever Yang kever.yang@rock-chips.com wrote:
Hi Alberto,
Thanks for your patches, and I'm so glad for people usingrockusb 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_r...
We use msc as base framework instead of dfu because of the bigperformance 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.
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.
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
PS: Yes, rockusb is available for all Rockchip's SoCs.
Thanks,
- Kever
On 07/04/2018 03:02 AM, Alberto Panizzo wrote:
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
--
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@denx.de
participants (3)
-
Alberto Panizzo -
Kever Yang -
Lukasz Majewski