hw/net/imx_fec.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
The current code causes clang static code analyzer generate warning:
hw/net/imx_fec.c:858:9: warning: Value stored to 'value' is never read
value = value & 0x0000000f;
^ ~~~~~~~~~~~~~~~~~~
hw/net/imx_fec.c:864:9: warning: Value stored to 'value' is never read
value = value & 0x000000fd;
^ ~~~~~~~~~~~~~~~~~~
According to the definition of the function, the two “value” assignments
should be written to registers.
Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com>
---
I'm not sure if this modification is correct, just from the function
definition, it is correct.
---
hw/net/imx_fec.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/net/imx_fec.c b/hw/net/imx_fec.c
index 6a124a154a..92f6215712 100644
--- a/hw/net/imx_fec.c
+++ b/hw/net/imx_fec.c
@@ -855,13 +855,13 @@ static void imx_enet_write(IMXFECState *s, uint32_t index, uint32_t value)
break;
case ENET_TGSR:
/* implement clear timer flag */
- value = value & 0x0000000f;
+ s->regs[index] = value & 0x0000000f;
break;
case ENET_TCSR0:
case ENET_TCSR1:
case ENET_TCSR2:
case ENET_TCSR3:
- value = value & 0x000000fd;
+ s->regs[index] = value & 0x000000fd;
break;
case ENET_TCCR0:
case ENET_TCCR1:
--
2.23.0
On 2020/2/25 上午10:59, Chen Qun wrote: > The current code causes clang static code analyzer generate warning: > hw/net/imx_fec.c:858:9: warning: Value stored to 'value' is never read > value = value & 0x0000000f; > ^ ~~~~~~~~~~~~~~~~~~ > hw/net/imx_fec.c:864:9: warning: Value stored to 'value' is never read > value = value & 0x000000fd; > ^ ~~~~~~~~~~~~~~~~~~ > > According to the definition of the function, the two “value” assignments > should be written to registers. > > Reported-by: Euler Robot <euler.robot@huawei.com> > Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com> > --- > I'm not sure if this modification is correct, just from the function > definition, it is correct. > --- > hw/net/imx_fec.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/hw/net/imx_fec.c b/hw/net/imx_fec.c > index 6a124a154a..92f6215712 100644 > --- a/hw/net/imx_fec.c > +++ b/hw/net/imx_fec.c > @@ -855,13 +855,13 @@ static void imx_enet_write(IMXFECState *s, uint32_t index, uint32_t value) > break; > case ENET_TGSR: > /* implement clear timer flag */ > - value = value & 0x0000000f; > + s->regs[index] = value & 0x0000000f; > break; > case ENET_TCSR0: > case ENET_TCSR1: > case ENET_TCSR2: > case ENET_TCSR3: > - value = value & 0x000000fd; > + s->regs[index] = value & 0x000000fd; > break; > case ENET_TCCR0: > case ENET_TCCR1: Applied. Thanks
On Tue, 25 Feb 2020 at 05:41, Jason Wang <jasowang@redhat.com> wrote: > > > On 2020/2/25 上午10:59, Chen Qun wrote: > > The current code causes clang static code analyzer generate warning: > > hw/net/imx_fec.c:858:9: warning: Value stored to 'value' is never read > > value = value & 0x0000000f; > > ^ ~~~~~~~~~~~~~~~~~~ > > hw/net/imx_fec.c:864:9: warning: Value stored to 'value' is never read > > value = value & 0x000000fd; > > ^ ~~~~~~~~~~~~~~~~~~ > > > > According to the definition of the function, the two “value” assignments > > should be written to registers. > > > > Reported-by: Euler Robot <euler.robot@huawei.com> > > Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com> > > --- > > I'm not sure if this modification is correct, just from the function > > definition, it is correct. > > --- > > hw/net/imx_fec.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/hw/net/imx_fec.c b/hw/net/imx_fec.c > > index 6a124a154a..92f6215712 100644 > > --- a/hw/net/imx_fec.c > > +++ b/hw/net/imx_fec.c > > @@ -855,13 +855,13 @@ static void imx_enet_write(IMXFECState *s, uint32_t index, uint32_t value) > > break; > > case ENET_TGSR: > > /* implement clear timer flag */ > > - value = value & 0x0000000f; > > + s->regs[index] = value & 0x0000000f; > > break; Hi; the datasheet for this SoC says that these bits of the register are write-1-to-clear, so while this is definitely a bug I don't think this is the right fix. > > case ENET_TCSR0: > > case ENET_TCSR1: > > case ENET_TCSR2: > > case ENET_TCSR3: > > - value = value & 0x000000fd; > > + s->regs[index] = value & 0x000000fd; > > break; Here bit 7 is write-1-to-clear, though bits 0 and 2..5 are simple write-the-value. > > case ENET_TCCR0: > > case ENET_TCCR1: > > > Applied. Could you drop this from your queue, please? thanks -- PMM
On 2020/2/25 下午6:18, Peter Maydell wrote: > On Tue, 25 Feb 2020 at 05:41, Jason Wang <jasowang@redhat.com> wrote: >> >> On 2020/2/25 上午10:59, Chen Qun wrote: >>> The current code causes clang static code analyzer generate warning: >>> hw/net/imx_fec.c:858:9: warning: Value stored to 'value' is never read >>> value = value & 0x0000000f; >>> ^ ~~~~~~~~~~~~~~~~~~ >>> hw/net/imx_fec.c:864:9: warning: Value stored to 'value' is never read >>> value = value & 0x000000fd; >>> ^ ~~~~~~~~~~~~~~~~~~ >>> >>> According to the definition of the function, the two “value” assignments >>> should be written to registers. >>> >>> Reported-by: Euler Robot <euler.robot@huawei.com> >>> Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com> >>> --- >>> I'm not sure if this modification is correct, just from the function >>> definition, it is correct. >>> --- >>> hw/net/imx_fec.c | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/hw/net/imx_fec.c b/hw/net/imx_fec.c >>> index 6a124a154a..92f6215712 100644 >>> --- a/hw/net/imx_fec.c >>> +++ b/hw/net/imx_fec.c >>> @@ -855,13 +855,13 @@ static void imx_enet_write(IMXFECState *s, uint32_t index, uint32_t value) >>> break; >>> case ENET_TGSR: >>> /* implement clear timer flag */ >>> - value = value & 0x0000000f; >>> + s->regs[index] = value & 0x0000000f; >>> break; > Hi; the datasheet for this SoC says that these bits > of the register are write-1-to-clear, so while this > is definitely a bug I don't think this is the right fix. > >>> case ENET_TCSR0: >>> case ENET_TCSR1: >>> case ENET_TCSR2: >>> case ENET_TCSR3: >>> - value = value & 0x000000fd; >>> + s->regs[index] = value & 0x000000fd; >>> break; > Here bit 7 is write-1-to-clear, though bits 0 and > 2..5 are simple write-the-value. > >>> case ENET_TCCR0: >>> case ENET_TCCR1: >> >> Applied. > Could you drop this from your queue, please? > > thanks > -- PMM Sure, Chen please send V2 to address Peter's comment. Thanks
>-----Original Message----- >From: Jason Wang [mailto:jasowang@redhat.com] >Sent: Wednesday, February 26, 2020 11:03 AM >To: Peter Maydell <peter.maydell@linaro.org> >Cc: Chenqun (kuhn) <kuhn.chenqun@huawei.com>; QEMU Developers ><qemu-devel@nongnu.org>; QEMU Trivial <qemu-trivial@nongnu.org>; >Zhanghailiang <zhang.zhanghailiang@huawei.com>; Peter Chubb ><peter.chubb@nicta.com.au>; qemu-arm <qemu-arm@nongnu.org> >Subject: Re: [PATCH] hw/net/imx_fec: write TGSR and TCSR3 in >imx_enet_write() > > >On 2020/2/25 下午6:18, Peter Maydell wrote: >> On Tue, 25 Feb 2020 at 05:41, Jason Wang <jasowang@redhat.com> wrote: >>> >>> On 2020/2/25 上午10:59, Chen Qun wrote: >>>> The current code causes clang static code analyzer generate warning: >>>> hw/net/imx_fec.c:858:9: warning: Value stored to 'value' is never read >>>> value = value & 0x0000000f; >>>> ^ ~~~~~~~~~~~~~~~~~~ >>>> hw/net/imx_fec.c:864:9: warning: Value stored to 'value' is never read >>>> value = value & 0x000000fd; >>>> ^ ~~~~~~~~~~~~~~~~~~ >>>> >>>> According to the definition of the function, the two “value” assignments >>>> should be written to registers. >>>> >>>> Reported-by: Euler Robot <euler.robot@huawei.com> >>>> Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com> >>>> --- >>>> I'm not sure if this modification is correct, just from the function >>>> definition, it is correct. >>>> --- >>>> hw/net/imx_fec.c | 4 ++-- >>>> 1 file changed, 2 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/hw/net/imx_fec.c b/hw/net/imx_fec.c index >>>> 6a124a154a..92f6215712 100644 >>>> --- a/hw/net/imx_fec.c >>>> +++ b/hw/net/imx_fec.c >>>> @@ -855,13 +855,13 @@ static void imx_enet_write(IMXFECState *s, >uint32_t index, uint32_t value) >>>> break; >>>> case ENET_TGSR: >>>> /* implement clear timer flag */ >>>> - value = value & 0x0000000f; >>>> + s->regs[index] = value & 0x0000000f; >>>> break; >> Hi; the datasheet for this SoC says that these bits of the register >> are write-1-to-clear, so while this is definitely a bug I don't think >> this is the right fix. >> >>>> case ENET_TCSR0: >>>> case ENET_TCSR1: >>>> case ENET_TCSR2: >>>> case ENET_TCSR3: >>>> - value = value & 0x000000fd; >>>> + s->regs[index] = value & 0x000000fd; >>>> break; >> Here bit 7 is write-1-to-clear, though bits 0 and >> 2..5 are simple write-the-value. >> >>>> case ENET_TCCR0: >>>> case ENET_TCCR1: >>> >>> Applied. >> Could you drop this from your queue, please? >> >> thanks >> -- PMM > > >Sure, Chen please send V2 to address Peter's comment. OK, but I didn't find the datasheet that contains these two registers description. Could someone provide me with a connection for the datasheet ?
© 2016 - 2025 Red Hat, Inc.