>-----Original Message----- >From: Philippe Mathieu-Daudé [mailto:[email protected]] >Sent: Tuesday, February 25, 2020 6:12 PM >To: Chenqun (kuhn) <[email protected]>; qemu- >[email protected]; [email protected] >Cc: [email protected]; Zhanghailiang ><[email protected]>; Alistair Francis <[email protected]>; >[email protected] >Subject: Re: [PATCH 09/13] dma/xlnx-zdma: Remove redundant statement in >zdma_write_dst() > >On 2/25/20 11:01 AM, Chenqun (kuhn) wrote: >>> -----Original Message----- >>> From: Philippe Mathieu-Daudé [mailto:[email protected]] >>> Sent: Tuesday, February 25, 2020 5:36 PM >>> To: Chenqun (kuhn) <[email protected]>; qemu- >[email protected]; >>> [email protected] >>> Cc: [email protected]; Zhanghailiang >>> <[email protected]>; Alistair Francis >>> <[email protected]>; [email protected] >>> Subject: Re: [PATCH 09/13] dma/xlnx-zdma: Remove redundant statement >>> in >>> zdma_write_dst() >>> >>> On 2/25/20 3:09 AM, [email protected] wrote: >>>> From: Chen Qun <[email protected]> >>>> >>>> Clang static code analyzer show warning: >>>> hw/dma/xlnx-zdma.c:399:13: warning: Value stored to 'dst_type' is >>>> never >>> read >>>> dst_type = FIELD_EX32(s->dsc_dst.words[3], >>> ZDMA_CH_DST_DSCR_WORD3, >>>> ^ >>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >>>> >>>> Reported-by: Euler Robot <[email protected]> >>>> Signed-off-by: Chen Qun <[email protected]> >>>> --- >>>> Cc: Alistair Francis <[email protected]> >>>> Cc: "Edgar E. Iglesias" <[email protected]> >>>> Cc: Peter Maydell <[email protected]> >>>> Cc: [email protected] >>>> --- >>>> hw/dma/xlnx-zdma.c | 2 -- >>>> 1 file changed, 2 deletions(-) >>>> >>>> diff --git a/hw/dma/xlnx-zdma.c b/hw/dma/xlnx-zdma.c index >>>> 8fb83f5b07..45355c5d59 100644 >>>> --- a/hw/dma/xlnx-zdma.c >>>> +++ b/hw/dma/xlnx-zdma.c >>>> @@ -396,8 +396,6 @@ static void zdma_write_dst(XlnxZDMA *s, uint8_t >>> *buf, uint32_t len) >>>> zdma_load_descriptor(s, next, &s->dsc_dst); >>>> dst_size = FIELD_EX32(s->dsc_dst.words[2], >>> ZDMA_CH_DST_DSCR_WORD2, >>>> SIZE); >>>> - dst_type = FIELD_EX32(s->dsc_dst.words[3], >>> ZDMA_CH_DST_DSCR_WORD3, >>>> - TYPE); >>> >>> Maybe move dst_type to this if() statement now? >>> >> Sorry, I don't follow you. I didn't find where I could move dst_type. >> Do you mean to move the first dst_type to the if(). >> Modify it like this: >> while (len) { >> dst_size = FIELD_EX32(s->dsc_dst.words[2], >ZDMA_CH_DST_DSCR_WORD2, >> SIZE); >> if (dst_size == 0 && ptype == PT_MEM) { >> uint64_t next; >> dst_type = FIELD_EX32(s->dsc_dst.words[3], >ZDMA_CH_DST_DSCR_WORD3, >> TYPE); >> next = zdma_update_descr_addr(s, dst_type, >> R_ZDMA_CH_DST_CUR_DSCR_LSB); >> zdma_load_descriptor(s, next, &s->dsc_dst); >> dst_size = FIELD_EX32(s->dsc_dst.words[2], >ZDMA_CH_DST_DSCR_WORD2, >> SIZE); >> } >> ... >> } > >No, like this: > >-- >8 -- >@@ -373,7 +373,7 @@ static uint64_t zdma_update_descr_addr(XlnxZDMA >*s, bool type, > static void zdma_write_dst(XlnxZDMA *s, uint8_t *buf, uint32_t len) > { > uint32_t dst_size, dlen; >- bool dst_intr, dst_type; >+ bool dst_intr; > unsigned int ptype = ARRAY_FIELD_EX32(s->regs, ZDMA_CH_CTRL0, >POINT_TYPE); > unsigned int rw_mode = ARRAY_FIELD_EX32(s->regs, ZDMA_CH_CTRL0, >MODE); > unsigned int burst_type = ARRAY_FIELD_EX32(s->regs, >ZDMA_CH_DATA_ATTR, @@ -387,17 +387,17 @@ static void >zdma_write_dst(XlnxZDMA *s, uint8_t *buf, uint32_t len) > while (len) { > dst_size = FIELD_EX32(s->dsc_dst.words[2], >ZDMA_CH_DST_DSCR_WORD2, > SIZE); >- dst_type = FIELD_EX32(s->dsc_dst.words[3], >ZDMA_CH_DST_DSCR_WORD3, >- TYPE); > if (dst_size == 0 && ptype == PT_MEM) { > uint64_t next; >+ bool dst_type; >+ >+ dst_type = FIELD_EX32(s->dsc_dst.words[3], >ZDMA_CH_DST_DSCR_WORD3, >+ TYPE); > next = zdma_update_descr_addr(s, dst_type, > R_ZDMA_CH_DST_CUR_DSCR_LSB); > zdma_load_descriptor(s, next, &s->dsc_dst); > dst_size = FIELD_EX32(s->dsc_dst.words[2], >ZDMA_CH_DST_DSCR_WORD2, > SIZE); >- dst_type = FIELD_EX32(s->dsc_dst.words[3], >ZDMA_CH_DST_DSCR_WORD3, >- TYPE); > } Hmm, this is better. I will modify it later in V2.
Thanks. >---
