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

[Lukasz Majewski <lukma@denx.de>]

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>