Thanks, Avri +AD4- -----Original Message----- +AD4- From: Bart Van Assche +AD4- Sent: Wednesday, August 01, 2018 6:36 PM +AD4- To: hch+AEA-lst.de+ADs- Avri Altman +ADs- linux-scsi+AEA-vger.kernel.org+ADs- +AD4- jthumshirn+AEA-suse.de+ADs- hare+AEA-suse.com+ADs- martin.petersen+AEA-oracle.com+ADs- +AD4- jejb+AEA-linux.vnet.ibm.com +AD4- Cc: Vinayak Holikatti +ADs- Avi Shchislowski +ADs- Alex Lemberg +ADs- Stanislav Nijnikov +ADs- +AD4- subhashj+AEA-codeaurora.org +AD4- Subject: Re: +AFs-PATCH 5/6+AF0- scsi: ufs-bsg: Add support for raw upiu in +AD4- ufs+AF8-bsg+AF8-request() +AD4- +AD4- On Wed, 2018-08-01 at 11:04 +-0300, Avri Altman wrote: +AD4- +AD4- +- if (qr-+AD4-opcode +ACEAPQ- UPIU+AF8-QUERY+AF8-OPCODE+AF8-WRITE+AF8-DESC +AHwAfA- +AD4- +AD4- +- desc+AF8-size +ADwAPQ- 0) +AD4- +AD4- +- return -EINVAL+ADs- +AD4- +AD4- Please use the full line length and don't split lines if that is not necessary. Done. +AD4- +AD4- +AD4- +- ret +AD0- ufshcd+AF8-map+AF8-desc+AF8-id+AF8-to+AF8-length(bsg+AF8-host-+AD4-hba, desc+AF8-id, +AD4- buff+AF8-len)+ADs- +AD4- +AD4- +- +AD4- +AD4- +- if (ret +AHwAfA- +ACE-buff+AF8-len) +AD4- +AD4- +- return -EINVAL+ADs- +AD4- +AD4- Why is buff+AF8-len only tested after it has been passed as an argument to +AD4- ufshcd+AF8-map+AF8-desc+AF8-id+AF8-to+AF8-length()? That seems weird to me. buff+AF8-len here is an int +ACo- and I'm using it to verify that enough space was allocated in the case it is a +ACI-write descriptor+ACI- upiu. So I need to fix 2 things: 1) should be if (ret +AHwAfA- +ACE-(+ACo-buff+AF8-len)) and not if (ret +AHwAfA- +ACE-buff+AF8-len) 2) don't utilize the buff+AF8-len variable for that, which is using to obtain The reply+AF8-payload+AF8-rcv+AF8-len eventually. Use a separate desc+AF8-len variable. +AD4- +AD4- +AD4- +-static int ufs+AF8-bsg+AF8-verify+AF8-query+AF8-size(unsigned int request+AF8-len, +AD4- +AD4- +- unsigned int reply+AF8-len, +AD4- +AD4- +- int rw, int buff+AF8-len) +AD4- +AD4- +-+AHs- +AD4- +AD4- +- int min+AF8-req+AF8-len +AD0- sizeof(struct ufs+AF8-bsg+AF8-request)+ADs- +AD4- +AD4- +- int min+AF8-rsp+AF8-len +AD0- sizeof(struct ufs+AF8-bsg+AF8-reply)+ADs- +AD4- +AD4- +- +AD4- +AD4- +- if (rw +AD0APQ- UFS+AF8-BSG+AF8-NOP) +AD4- +AD4- +- goto null+AF8-buff+ADs- +AD4- +AD4- +- +AD4- +AD4- +- if (rw +AD0APQ- WRITE) +AD4- +AD4- +- min+AF8-req+AF8-len +-+AD0- buff+AF8-len+ADs- +AD4- +AD4- Can the +ACI-goto+ACI- statement be avoided by using a switch/case on 'rw'? Yes. Actually if (rw +AD0APQ- UFS+AF8-BSG+AF8-NOP) is not needed at all. +AD4- +AD4- Thanks, +AD4- +AD4- Bart.
