diff mbox series

[18/26] hw/intc/arm_gicv3_its: Fix handling of process_its_cmd() return value

Message ID 20211211191135.1764649-19-peter.maydell@linaro.org
State Superseded
Headers show
Series arm gicv3 ITS: Various bug fixes and refactorings | expand

Commit Message

Peter Maydell Dec. 11, 2021, 7:11 p.m. UTC 
process_its_cmd() returns a bool, like all the other process_ functions.
However we were putting its return value into 'res', not 'result',
which meant we would ignore it when deciding whether to continue
or stall the command queue. Fix the typo.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/intc/arm_gicv3_its.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Richard Henderson Dec. 12, 2021, 8:53 p.m. UTC | #1 
On 12/11/21 11:11 AM, Peter Maydell wrote:
> process_its_cmd() returns a bool, like all the other process_ functions.
> However we were putting its return value into 'res', not 'result',
> which meant we would ignore it when deciding whether to continue
> or stall the command queue. Fix the typo.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   hw/intc/arm_gicv3_its.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~
Philippe Mathieu-Daudé Dec. 13, 2021, 1:01 p.m. UTC | #2 
On 12/11/21 20:11, Peter Maydell wrote:
> process_its_cmd() returns a bool, like all the other process_ functions.
> However we were putting its return value into 'res', not 'result',
> which meant we would ignore it when deciding whether to continue
> or stall the command queue. Fix the typo.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  hw/intc/arm_gicv3_its.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Alex Bennée Dec. 13, 2021, 1:41 p.m. UTC | #3 
Peter Maydell <peter.maydell@linaro.org> writes:

> process_its_cmd() returns a bool, like all the other process_ functions.
> However we were putting its return value into 'res', not 'result',
> which meant we would ignore it when deciding whether to continue
> or stall the command queue. Fix the typo.

Arguably having to generic result types is confusing. Naming things is
hard but maybe it should be a clear:

      MemTxResult memtx_res = MEMTX_OK;
      ItsCmdResult its_res = CMD_CONTINUE;

?

Anyway:

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
diff mbox series

Patch

diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
index e6b380f663c..32cf18c10af 100644
--- a/hw/intc/arm_gicv3_its.c
+++ b/hw/intc/arm_gicv3_its.c
@@ -676,10 +676,10 @@  static void process_cmdq(GICv3ITSState *s)
 
         switch (cmd) {
         case GITS_CMD_INT:
-            res = process_its_cmd(s, data, cq_offset, INTERRUPT);
+            result = process_its_cmd(s, data, cq_offset, INTERRUPT);
             break;
         case GITS_CMD_CLEAR:
-            res = process_its_cmd(s, data, cq_offset, CLEAR);
+            result = process_its_cmd(s, data, cq_offset, CLEAR);
             break;
         case GITS_CMD_SYNC:
             /*