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

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

[Alberto Panizzo]

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

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

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

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

-- 
2.7.4

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

[Alberto Panizzo]

Two consecutive rockusb_tx_write without waiting for request complete
do results in transfer reset of first request and thus no or incomplete
data transfer. This because rockusb_tx_write do use just 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)
-- 
2.7.4

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

[Lukasz Majewski]

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

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

[Alberto Panizzo]

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

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

[Alberto Panizzo]

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

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

Note that memory version do invert MSB/LSB so printing the char
buffer 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,
-- 
2.7.4

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

[Lukasz Majewski]

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

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,
>  	},
>  	{
>  		.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
-------------- 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/f2cb77ce/attachment.sig>

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

[Alberto Panizzo]

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 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));
> 
> 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,
> >  	},
> >  	{
> >  		.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

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

[Alberto Panizzo]

This allows rockusb code to reply correctly to K_FW_GET_CHIP_VER
command.

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

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

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

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

[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 <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,
-- 
2.7.4

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

[Lukasz Majewski]

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 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/02824332/attachment.sig>

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

[Alberto Panizzo]

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 at denx.de

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

[Kever Yang]

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,
>  	},
>  	{
>  		.cmd = K_FW_LBA_WRITE_10,

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

[Alberto Panizzo]

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

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

[Alberto Panizzo]

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,
 	},
-- 
2.7.4

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

[Lukasz Majewski]

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 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/c9f4dc86/attachment.sig>

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

[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 <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);
-- 
2.7.4

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

[Lukasz Majewski]

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 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/b905df20/attachment.sig>

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

[Alberto Panizzo]

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 at denx.de

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

[Alberto Panizzo]

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

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

diff --git a/arch/arm/include/asm/arch-rockchip/f_rockusb.h b/arch/arm/include/asm/arch-rockchip/f_rockusb.h
index 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
-- 
2.7.4

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

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

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
>

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

[Alberto Panizzo]

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

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

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

[Lukasz Majewski]

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

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 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/20180705/76d69d0c/attachment.sig>