>-----Original Message----- >From: Philippe Mathieu-Daudé [mailto:phi...@redhat.com] >Sent: Tuesday, February 25, 2020 6:12 PM >To: Chenqun (kuhn) <kuhn.chen...@huawei.com>; qemu- >de...@nongnu.org; qemu-triv...@nongnu.org >Cc: peter.mayd...@linaro.org; Zhanghailiang ><zhang.zhanghaili...@huawei.com>; Alistair Francis <alist...@alistair23.me>; >qemu-...@nongnu.org >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:phi...@redhat.com] >>> Sent: Tuesday, February 25, 2020 5:36 PM >>> To: Chenqun (kuhn) <kuhn.chen...@huawei.com>; qemu- >de...@nongnu.org; >>> qemu-triv...@nongnu.org >>> Cc: peter.mayd...@linaro.org; Zhanghailiang >>> <zhang.zhanghaili...@huawei.com>; Alistair Francis >>> <alist...@alistair23.me>; qemu-...@nongnu.org >>> Subject: Re: [PATCH 09/13] dma/xlnx-zdma: Remove redundant statement >>> in >>> zdma_write_dst() >>> >>> On 2/25/20 3:09 AM, kuhn.chen...@huawei.com wrote: >>>> From: Chen Qun <kuhn.chen...@huawei.com> >>>> >>>> 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 <euler.ro...@huawei.com> >>>> Signed-off-by: Chen Qun <kuhn.chen...@huawei.com> >>>> --- >>>> Cc: Alistair Francis <alist...@alistair23.me> >>>> Cc: "Edgar E. Iglesias" <edgar.igles...@gmail.com> >>>> Cc: Peter Maydell <peter.mayd...@linaro.org> >>>> Cc: qemu-...@nongnu.org >>>> --- >>>> 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. >---
