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