[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
RE: [PATCH v4 3/3] timer/exynos4210_mct: Remove redundant statement in e
From: |
Chenqun (kuhn) |
Subject: |
RE: [PATCH v4 3/3] timer/exynos4210_mct: Remove redundant statement in exynos4210_mct_write() |
Date: |
Tue, 24 Mar 2020 12:22:37 +0000 |
>-----Original Message-----
>From: Laurent Vivier [mailto:address@hidden]
>Sent: Tuesday, March 24, 2020 6:59 PM
>To: Chenqun (kuhn) <address@hidden>; address@hidden;
>address@hidden
>Cc: Zhanghailiang <address@hidden>; address@hidden;
>Euler Robot <address@hidden>; Igor Mitsyanko
><address@hidden>; Peter Maydell <address@hidden>
>Subject: Re: [PATCH v4 3/3] timer/exynos4210_mct: Remove redundant
>statement in exynos4210_mct_write()
>
>Le 24/03/2020 à 09:22, Chen Qun a écrit :
>> Clang static code analyzer show warning:
>> hw/timer/exynos4210_mct.c:1370:9: warning: Value stored to 'index' is never
>read
>> index = GET_L_TIMER_CNT_REG_IDX(offset, lt_i);
>> ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> hw/timer/exynos4210_mct.c:1399:9: warning: Value stored to 'index' is never
>read
>> index = GET_L_TIMER_CNT_REG_IDX(offset, lt_i);
>> ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> hw/timer/exynos4210_mct.c:1441:9: warning: Value stored to 'index' is never
>read
>> index = GET_L_TIMER_CNT_REG_IDX(offset, lt_i);
>> ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
>It would be interesting to understand why we need the index for these registers
>in exynos4210_mct_read() and not in exynos4210_mct_write().
>
I think the index can also be used in exynos4210_mct_write(), but the original
author used a more obvious reg name instead of it.
The obvious reg name:
case L0_TCNTB: case L1_TCNTB:
reg name is : L_REG_CNT_TCNTB
case L0_ICNTB: case L1_ICNTB:
reg name is : L_REG_CNT_ICNTB
case L0_FRCNTB: case L1_FRCNTB:
reg name is : L_REG_CNT_FRCCNTB
Thanks.
>
>> Reported-by: Euler Robot <address@hidden>
>> Signed-off-by: Chen Qun <address@hidden>
>> ---
>> Cc: Igor Mitsyanko <address@hidden>
>> Cc: Peter Maydell <address@hidden>
>> ---
>> hw/timer/exynos4210_mct.c | 4 ----
>> 1 file changed, 4 deletions(-)
>>
>> diff --git a/hw/timer/exynos4210_mct.c b/hw/timer/exynos4210_mct.c
>> index 944120aea5..570cf7075b 100644
>> --- a/hw/timer/exynos4210_mct.c
>> +++ b/hw/timer/exynos4210_mct.c
>> @@ -1367,7 +1367,6 @@ static void exynos4210_mct_write(void *opaque,
>> hwaddr offset,
>>
>> case L0_TCNTB: case L1_TCNTB:
>> lt_i = GET_L_TIMER_IDX(offset);
>> - index = GET_L_TIMER_CNT_REG_IDX(offset, lt_i);
>>
>> /*
>> * TCNTB is updated to internal register only after CNT expired.
>> @@ -1396,7 +1395,6 @@ static void exynos4210_mct_write(void *opaque,
>> hwaddr offset,
>>
>> case L0_ICNTB: case L1_ICNTB:
>> lt_i = GET_L_TIMER_IDX(offset);
>> - index = GET_L_TIMER_CNT_REG_IDX(offset, lt_i);
>>
>> s->l_timer[lt_i].reg.wstat |= L_WSTAT_ICNTB_WRITE;
>> s->l_timer[lt_i].reg.cnt[L_REG_CNT_ICNTB] = value & @@
>> -1438,8 +1436,6 @@ static void exynos4210_mct_write(void *opaque,
>> hwaddr offset,
>>
>> case L0_FRCNTB: case L1_FRCNTB:
>> lt_i = GET_L_TIMER_IDX(offset);
>> - index = GET_L_TIMER_CNT_REG_IDX(offset, lt_i);
>> -
>> DPRINTF("local timer[%d] FRCNTB write %llx\n", lt_i, value);
>>
>> s->l_timer[lt_i].reg.wstat |= L_WSTAT_FRCCNTB_WRITE;
>>
[PATCH v4 1/3] scsi/esp-pci: Remove redundant statement in esp_pci_io_write(), Chen Qun, 2020/03/24