RE: [PATCH 09/13] dma/xlnx-zdma: Remove redundant statement in zdma_write_dst()

2020-02-25 Thread Chenqun (kuhn)


>-Original Message-
>From: Philippe Mathieu-Daudé [mailto:phi...@redhat.com]
>Sent: Tuesday, February 25, 2020 6:12 PM
>To: Chenqun (kuhn) ; qemu-
>de...@nongnu.org; qemu-triv...@nongnu.org
>Cc: peter.mayd...@linaro.org; Zhanghailiang
>; Alistair Francis ;
>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) ; qemu-
>de...@nongnu.org;
>>> qemu-triv...@nongnu.org
>>> Cc: peter.mayd...@linaro.org; Zhanghailiang
>>> ; Alistair Francis
>>> ; 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 
>>>>
>>>> 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 
>>>> Signed-off-by: Chen Qun 
>>>> ---
>>>> Cc: Alistair Francis 
>>>> Cc: "Edgar E. Iglesias" 
>>>> Cc: Peter Maydell 
>>>> 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, >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, >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, >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.
>---



Re: [PATCH 09/13] dma/xlnx-zdma: Remove redundant statement in zdma_write_dst()

2020-02-25 Thread Philippe Mathieu-Daudé

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) ; qemu-
de...@nongnu.org; qemu-triv...@nongnu.org
Cc: peter.mayd...@linaro.org; Zhanghailiang
; Alistair Francis ;
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 

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 
Signed-off-by: Chen Qun 
---
Cc: Alistair Francis 
Cc: "Edgar E. Iglesias" 
Cc: Peter Maydell 
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, >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, >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, >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);
 }
---




RE: [PATCH 09/13] dma/xlnx-zdma: Remove redundant statement in zdma_write_dst()

2020-02-25 Thread Chenqun (kuhn)


>-Original Message-
>From: Philippe Mathieu-Daudé [mailto:phi...@redhat.com]
>Sent: Tuesday, February 25, 2020 5:36 PM
>To: Chenqun (kuhn) ; qemu-
>de...@nongnu.org; qemu-triv...@nongnu.org
>Cc: peter.mayd...@linaro.org; Zhanghailiang
>; Alistair Francis ;
>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 
>>
>> 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 
>> Signed-off-by: Chen Qun 
>> ---
>> Cc: Alistair Francis 
>> Cc: "Edgar E. Iglesias" 
>> Cc: Peter Maydell 
>> 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, >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, >dsc_dst);
dst_size = FIELD_EX32(s->dsc_dst.words[2], ZDMA_CH_DST_DSCR_WORD2,
  SIZE);
}
   ...
   }

Thanks.
>>   }
>>
>>   /* Match what hardware does by ignoring the dst_size and
>> only using
>>



Re: [PATCH 09/13] dma/xlnx-zdma: Remove redundant statement in zdma_write_dst()

2020-02-25 Thread Philippe Mathieu-Daudé

On 2/25/20 3:09 AM, kuhn.chen...@huawei.com wrote:

From: Chen Qun 

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 
Signed-off-by: Chen Qun 
---
Cc: Alistair Francis 
Cc: "Edgar E. Iglesias" 
Cc: Peter Maydell 
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, >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?


  }
  
  /* Match what hardware does by ignoring the dst_size and only using







[PATCH 09/13] dma/xlnx-zdma: Remove redundant statement in zdma_write_dst()

2020-02-24 Thread kuhn.chenqun
From: Chen Qun 

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 
Signed-off-by: Chen Qun 
---
Cc: Alistair Francis 
Cc: "Edgar E. Iglesias" 
Cc: Peter Maydell 
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, >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);
 }
 
 /* Match what hardware does by ignoring the dst_size and only using
-- 
2.23.0