On Wed, 2014-09-24 at 17:48 +0200, Tomas Henzl wrote: > 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. You are right. ACB_F_MESSAGE_WQBUFFER_CLEARED, ACB_F_MESSAGE_RQBUFFER_CLEARED, ACB_F_MESSAGE_WQBUFFER_READED are seem useless. > ... > > @@ -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? There is not special reason have to int32, int is OK. > > /* 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
