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

[Marek Vasut <marex@denx.de>]

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

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

[...]

-- 
Best regards,
Marek Vasut