On 09/24/2014 11:33 AM, Ching Huang wrote: > From: Ching Huang <ching2048@xxxxxxxxxxxx> > > This patch is relative to: > http://git.infradead.org/users/hch/scsi-queue.git/tree/drivers-for-3.18:/drivers/scsi/arcmsr > > change in v5: > 1. rename firstindex to getIndex, lastindex to putIndex for readability > 2. define ARCMSR_API_DATA_BUFLEN as 1032 > 3. simplify ioctl data read by marcro CIRC_CNT_TO_END and CIRC_CNT > > Signed-off-by: Ching Huang <ching 2048@xxxxxxxxxxxx> > --- > ... > + pQbuffer = &acb->wqbuffer[acb->wqbuf_putIndex]; > + cnt2end = ARCMSR_MAX_QBUFFER - acb->wqbuf_putIndex; > + if (user_len > cnt2end) { > + memcpy(pQbuffer, ptmpuserbuffer, cnt2end); > + ptmpuserbuffer += cnt2end; > + user_len -= cnt2end; > + acb->wqbuf_putIndex = 0; > + pQbuffer = acb->wqbuffer; > } > + memcpy(pQbuffer, ptmpuserbuffer, user_len); > + acb->wqbuf_putIndex += user_len; > + acb->wqbuf_putIndex %= ARCMSR_MAX_QBUFFER; > + if (acb->acb_flags & ACB_F_MESSAGE_WQBUFFER_CLEARED) { This test^ is most likely useless, it looks like you set the ACB_F_MESSAGE_WQBUFFER_CLEARED every time you have added some data to the buffer and clear it when the buffer gets empty. I think you could get rid of the ACB_F_MESSAGE_WQBUFFER_CLEARED completely. Also the ACB_F_MESSAGE_RQBUFFER_CLEARED doesn't seems to be ever evaluated. I'm not sure with the ACB_F_MESSAGE_WQBUFFER_READED, but that one probably is also a candidate for removal. ... > @@ -678,15 +679,15 @@ struct AdapterControlBlock > unsigned int uncache_size; > uint8_t rqbuffer[ARCMSR_MAX_QBUFFER]; > /* data collection buffer for read from 80331 */ > - int32_t rqbuf_firstindex; > + int32_t rqbuf_getIndex; What is the reason for using an exact size int32 (instead of a plain int) here? > /* first of read buffer */ > - int32_t rqbuf_lastindex; > + int32_t rqbuf_putIndex; > /* last of read buffer */ > uint8_t wqbuffer[ARCMSR_MAX_QBUFFER]; > /* data collection buffer for write to 80331 */ > - int32_t wqbuf_firstindex; > + int32_t wqbuf_getIndex; > /* first of write buffer */ > - int32_t wqbuf_lastindex; > + int32_t wqbuf_putIndex; > /* last of write buffer */ > uint8_t devstate[ARCMSR_MAX_TARGETID][ARCMSR_MAX_TARGETLUN]; > /* id0 ..... id15, lun0...lun7 */ The comments I've added are not directly related to this patch, but you may still address them in a new patch so - Reviewed-by: Tomas Henzl <thenzl@xxxxxxxxxx> -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html