[U-Boot] [PATCH v6 1/2] common: usb_storage: Make common function for usb_stor_read/usb_stor_write

[Rajesh Bhagat]

> -----Original Message-----
> From: Marek Vasut [mailto:marex at denx.de]
> Sent: Thursday, June 16, 2016 3:33 AM
> To: Rajesh Bhagat <rajesh.bhagat at nxp.com>; u-boot at lists.denx.de
> Cc: sjg at chromium.org; york sun <york.sun at nxp.com>; Sriram Dash
> <sriram.dash at nxp.com>
> Subject: Re: [PATCH v6 1/2] common: usb_storage: Make common function for
> usb_stor_read/usb_stor_write
> 
> On 06/15/2016 09:00 AM, Rajesh Bhagat wrote:
> > Performs code cleanup by making common function for usb_stor_read/
> > usb_stor_write. Currently only difference in these fucntions is call
> > to usb_read_10/usb_write_10 scsi commands.
> >
> > Signed-off-by: Rajesh Bhagat <rajesh.bhagat at nxp.com>
> > ---
> > Changes in v6:
> >  - Removes USB_STOR_OP_TYPE macro and adds __func__ in debug prints
> >  - Unifies usb_read_10/usb_write_10 functions to usb_read_write_10
> >
> > Changes in v5:
> >  - Converts USB_STOR_OPERATION_FUNC macro to a function
> >  - Corrects the multi line comment accroding to coding style
> >  - Renames variable to dev to remove code duplication
> >
> > Changes in v4:
> >  - Adds code to make common function for read/write
> >
> >  common/usb_storage.c | 147
> > +++++++++++++--------------------------------------
> >  1 file changed, 36 insertions(+), 111 deletions(-)
> >
> > diff --git a/common/usb_storage.c b/common/usb_storage.c index
> > 7e6e52d..41e5aa3 100644
> > --- a/common/usb_storage.c
> > +++ b/common/usb_storage.c
> > @@ -1046,11 +1046,11 @@ static int usb_read_capacity(ccb *srb, struct us_data
> *ss)
> >  	return -1;
> >  }
> >
> > -static int usb_read_10(ccb *srb, struct us_data *ss, unsigned long start,
> > -		       unsigned short blocks)
> > +static int usb_read_write_10(ccb *srb, struct us_data *ss, unsigned long start,
> > +		       unsigned short blocks, bool is_write)
> >  {
> >  	memset(&srb->cmd[0], 0, 12);
> > -	srb->cmd[0] = SCSI_READ10;
> > +	srb->cmd[0] = is_write ? SCSI_WRITE10 : SCSI_READ10;
> >  	srb->cmd[1] = srb->lun << 5;
> >  	srb->cmd[2] = ((unsigned char) (start >> 24)) & 0xff;
> >  	srb->cmd[3] = ((unsigned char) (start >> 16)) & 0xff; @@ -1059,28
> > +1059,10 @@ static int usb_read_10(ccb *srb, struct us_data *ss, unsigned long
> start,
> >  	srb->cmd[7] = ((unsigned char) (blocks >> 8)) & 0xff;
> >  	srb->cmd[8] = (unsigned char) blocks & 0xff;
> >  	srb->cmdlen = 12;
> > -	debug("read10: start %lx blocks %x\n", start, blocks);
> > +	debug("%s: start %lx blocks %x\n", __func__, start, blocks);
> >  	return ss->transport(srb, ss);
> >  }
> >
> > -static int usb_write_10(ccb *srb, struct us_data *ss, unsigned long start,
> > -			unsigned short blocks)
> > -{
> > -	memset(&srb->cmd[0], 0, 12);
> > -	srb->cmd[0] = SCSI_WRITE10;
> > -	srb->cmd[1] = srb->lun << 5;
> > -	srb->cmd[2] = ((unsigned char) (start >> 24)) & 0xff;
> > -	srb->cmd[3] = ((unsigned char) (start >> 16)) & 0xff;
> > -	srb->cmd[4] = ((unsigned char) (start >> 8)) & 0xff;
> > -	srb->cmd[5] = ((unsigned char) (start)) & 0xff;
> > -	srb->cmd[7] = ((unsigned char) (blocks >> 8)) & 0xff;
> > -	srb->cmd[8] = (unsigned char) blocks & 0xff;
> > -	srb->cmdlen = 12;
> > -	debug("write10: start %lx blocks %x\n", start, blocks);
> > -	return ss->transport(srb, ss);
> > -}
> > -

Hello Marek, 

> 
> This stuff should be in a separate patch, otherwise it's not bisectable.
> Looks pretty OK otherwise.

Will take care in v7.

Best Regards,
Rajesh Bhagat 

> 
> [...]
> 
> --
> Best regards,
> Marek Vasut