* [PATCH v2] hw/net/imx_fec: write TGSR and TCSR3 in imx_enet_write()
@ 2020-03-05 10:53 Chen Qun
2020-03-05 11:04 ` Chenqun (kuhn)
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: Chen Qun @ 2020-03-05 10:53 UTC (permalink / raw)
To: qemu-devel, qemu-trivial
Cc: peter.maydell, zhang.zhanghailiang, jasowang, qemu-arm,
peter.chubb, Euler Robot, Chen Qun
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>
---
v1->v2:
The register 'ENET_TGSR' write-1-to-clear timer flag.
The register 'ENET_TCSRn' 7bit(TF) write-1-to-clear timer flag.
---
hw/net/imx_fec.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/hw/net/imx_fec.c b/hw/net/imx_fec.c
index 6a124a154a..322cbdcc17 100644
--- a/hw/net/imx_fec.c
+++ b/hw/net/imx_fec.c
@@ -855,13 +855,15 @@ 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] ^= s->regs[index] & value;
+ s->regs[index] &= 0x0000000f;
break;
case ENET_TCSR0:
case ENET_TCSR1:
case ENET_TCSR2:
case ENET_TCSR3:
- value = value & 0x000000fd;
+ s->regs[index] = (value & 0x00000080) ? (0x0000007d & value) :
+ (value & 0x000000fd);
break;
case ENET_TCCR0:
case ENET_TCCR1:
--
2.23.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* RE: [PATCH v2] hw/net/imx_fec: write TGSR and TCSR3 in imx_enet_write()
2020-03-05 10:53 [PATCH v2] hw/net/imx_fec: write TGSR and TCSR3 in imx_enet_write() Chen Qun
@ 2020-03-05 11:04 ` Chenqun (kuhn)
2020-03-05 11:50 ` no-reply
` (2 subsequent siblings)
3 siblings, 0 replies; 8+ messages in thread
From: Chenqun (kuhn) @ 2020-03-05 11:04 UTC (permalink / raw)
To: qemu-devel, qemu-trivial
Cc: peter.maydell, Zhanghailiang, jasowang, qemu-arm, peter.chubb,
Euler Robot
>-----Original Message-----
>From: Chenqun (kuhn)
>Sent: Thursday, March 5, 2020 6:53 PM
>To: qemu-devel@nongnu.org; qemu-trivial@nongnu.org
>Cc: peter.maydell@linaro.org; Zhanghailiang
><zhang.zhanghailiang@huawei.com>; jasowang@redhat.com;
>peter.chubb@nicta.com.au; qemu-arm@nongnu.org; Chenqun (kuhn)
><kuhn.chenqun@huawei.com>; Euler Robot <euler.robot@huawei.com>
>Subject: [PATCH v2] hw/net/imx_fec: write TGSR and TCSR3 in
>imx_enet_write()
>
>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>
>---
>v1->v2:
> The register 'ENET_TGSR' write-1-to-clear timer flag.
> The register 'ENET_TCSRn' 7bit(TF) write-1-to-clear timer flag.
>---
> hw/net/imx_fec.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
>diff --git a/hw/net/imx_fec.c b/hw/net/imx_fec.c index
>6a124a154a..322cbdcc17 100644
>--- a/hw/net/imx_fec.c
>+++ b/hw/net/imx_fec.c
>@@ -855,13 +855,15 @@ 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] ^= s->regs[index] & value;
>+ s->regs[index] &= 0x0000000f;
>
In "i.MX 6Dual/6Quad Applications Processor Reference Manual" documentation,
the register 'ENET_TGSR' all fields TFn is write-1-to-clear.
It is described in detail as follows:
-------------------------------------------------------
Field Description
-------------------------------------------------------
31–4 This field is reserved.
-------------------------------------------------------
3 Copy Of Timer Flag For Channel 3
TF3 0 Timer Flag for Channel 3 is clear
1 Timer Flag for Channel 3 is set
-------------------------------------------------------
2 Copy Of Timer Flag For Channel 2
TF2 0 Timer Flag for Channel 2 is clear
1 Timer Flag for Channel 2 is set
-------------------------------------------------------
1 Copy Of Timer Flag For Channel 1
TF1 0 Timer Flag for Channel 1 is clear
1 Timer Flag for Channel 1 is set
-------------------------------------------------------
0 Copy Of Timer Flag For Channel 0
TF0 0 Timer Flag for Channel 0 is clear
1 Timer Flag for Channel 0 is set
------------------------------------------------------
> break;
> case ENET_TCSR0:
> case ENET_TCSR1:
> case ENET_TCSR2:
> case ENET_TCSR3:
>- value = value & 0x000000fd;
>+ s->regs[index] = (value & 0x00000080) ? (0x0000007d & value) :
>+ (value & 0x000000fd);
>
The ENET_TCSRn field descriptions:
-------------------------------------------------------
Field Description
-------------------------------------------------------
31–8 This field is reserved.
------------------------------------------------------
7 Timer Flag
TF Sets when input capture or output compare occurs.
This flag is double buffered between the module clock and 1588 clock domains.
When this field is 1, it can be cleared to 0 by writing 1to it.
0 Input Capture or Output Compare has not occurred
1 Input Capture or Output Compare has occurred
--------------------------------------------------------
6 Timer Interrupt Enable
TIE 0 Interrupt is disabled
1 Interrupt is enabled
--------------------------------------------------------
2-5 Timer Mode
..................
--------------------------------------------------------
1 This field is reserved.
--------------------------------------------------------
0 Timer DMA Request Enable
TDRE 0 DMA request is disabled
1 DMA request is enabled
--------------------------------------------------------
> break;
> case ENET_TCCR0:
> case ENET_TCCR1:
>--
>2.23.0
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] hw/net/imx_fec: write TGSR and TCSR3 in imx_enet_write()
2020-03-05 10:53 [PATCH v2] hw/net/imx_fec: write TGSR and TCSR3 in imx_enet_write() Chen Qun
2020-03-05 11:04 ` Chenqun (kuhn)
@ 2020-03-05 11:50 ` no-reply
2020-03-05 11:55 ` no-reply
2020-03-09 11:35 ` Peter Maydell
3 siblings, 0 replies; 8+ messages in thread
From: no-reply @ 2020-03-05 11:50 UTC (permalink / raw)
To: kuhn.chenqun
Cc: peter.maydell, zhang.zhanghailiang, qemu-trivial, jasowang,
qemu-devel, qemu-arm, peter.chubb, euler.robot, kuhn.chenqun
Patchew URL: https://patchew.org/QEMU/20200305105325.31264-1-kuhn.chenqun@huawei.com/
Hi,
This series failed the docker-quick@centos7 build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.
=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-centos7 V=1 NETWORK=1
time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===
Submodule 'slirp' (https://git.qemu.org/git/libslirp.git) registered for path 'slirp'
Cloning into 'slirp'...
remote: Counting objects: 3095, done.
error: RPC failed; result=18, HTTP code = 200
fatal: The remote end hung up unexpectedly
fatal: protocol error: bad pack header
Clone of 'https://git.qemu.org/git/libslirp.git' into submodule path 'slirp' failed
failed to update submodule slirp
Cleared directory 'dtc'
Submodule 'dtc' (https://git.qemu.org/git/dtc.git) unregistered for path 'dtc'
Submodule 'slirp' (https://git.qemu.org/git/libslirp.git) unregistered for path 'slirp'
make[1]: *** [/var/tmp/patchew-tester-tmp-vmdpiqef/src/docker-src.2020-03-05-06.41.02.14569] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-vmdpiqef/src'
make: *** [docker-run-test-quick@centos7] Error 2
real 9m36.630s
user 0m4.117s
The full log is available at
http://patchew.org/logs/20200305105325.31264-1-kuhn.chenqun@huawei.com/testing.docker-quick@centos7/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] hw/net/imx_fec: write TGSR and TCSR3 in imx_enet_write()
2020-03-05 10:53 [PATCH v2] hw/net/imx_fec: write TGSR and TCSR3 in imx_enet_write() Chen Qun
2020-03-05 11:04 ` Chenqun (kuhn)
2020-03-05 11:50 ` no-reply
@ 2020-03-05 11:55 ` no-reply
2020-03-09 11:35 ` Peter Maydell
3 siblings, 0 replies; 8+ messages in thread
From: no-reply @ 2020-03-05 11:55 UTC (permalink / raw)
To: kuhn.chenqun
Cc: peter.maydell, zhang.zhanghailiang, qemu-trivial, jasowang,
qemu-devel, qemu-arm, peter.chubb, euler.robot, kuhn.chenqun
Patchew URL: https://patchew.org/QEMU/20200305105325.31264-1-kuhn.chenqun@huawei.com/
Hi,
This series failed the docker-mingw@fedora build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.
=== TEST SCRIPT BEGIN ===
#! /bin/bash
export ARCH=x86_64
make docker-image-fedora V=1 NETWORK=1
time make docker-test-mingw@fedora J=14 NETWORK=1
=== TEST SCRIPT END ===
Submodule 'dtc' (https://git.qemu.org/git/dtc.git) registered for path 'dtc'
Cloning into 'dtc'...
remote: Counting objects: 5394, done.
error: RPC failed; result=18, HTTP code = 200
fatal: The remote end hung up unexpectedly
fatal: protocol error: bad pack header
Clone of 'https://git.qemu.org/git/dtc.git' into submodule path 'dtc' failed
failed to update submodule dtc
Submodule 'dtc' (https://git.qemu.org/git/dtc.git) unregistered for path 'dtc'
make[1]: *** [/var/tmp/patchew-tester-tmp-u0wjl6xm/src/docker-src.2020-03-05-06.51.13.15558] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-u0wjl6xm/src'
make: *** [docker-run-test-mingw@fedora] Error 2
real 4m2.749s
user 0m2.356s
The full log is available at
http://patchew.org/logs/20200305105325.31264-1-kuhn.chenqun@huawei.com/testing.docker-mingw@fedora/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] hw/net/imx_fec: write TGSR and TCSR3 in imx_enet_write()
2020-03-05 10:53 [PATCH v2] hw/net/imx_fec: write TGSR and TCSR3 in imx_enet_write() Chen Qun
` (2 preceding siblings ...)
2020-03-05 11:55 ` no-reply
@ 2020-03-09 11:35 ` Peter Maydell
2020-03-10 8:08 ` Chenqun (kuhn)
3 siblings, 1 reply; 8+ messages in thread
From: Peter Maydell @ 2020-03-09 11:35 UTC (permalink / raw)
To: Chen Qun
Cc: zhanghailiang, QEMU Trivial, Jason Wang, QEMU Developers,
qemu-arm, Peter Chubb, Euler Robot
On Thu, 5 Mar 2020 at 10:53, Chen Qun <kuhn.chenqun@huawei.com> 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>
> ---
> v1->v2:
> The register 'ENET_TGSR' write-1-to-clear timer flag.
> The register 'ENET_TCSRn' 7bit(TF) write-1-to-clear timer flag.
> ---
> hw/net/imx_fec.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/hw/net/imx_fec.c b/hw/net/imx_fec.c
> index 6a124a154a..322cbdcc17 100644
> --- a/hw/net/imx_fec.c
> +++ b/hw/net/imx_fec.c
> @@ -855,13 +855,15 @@ 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] ^= s->regs[index] & value;
> + s->regs[index] &= 0x0000000f;
> break;
> case ENET_TCSR0:
> case ENET_TCSR1:
> case ENET_TCSR2:
> case ENET_TCSR3:
> - value = value & 0x000000fd;
> + s->regs[index] = (value & 0x00000080) ? (0x0000007d & value) :
> + (value & 0x000000fd);
> break;
> case ENET_TCCR0:
> case ENET_TCCR1:
This isn't the usual way to write W1C behaviour.
If all the relevant bits are W1C, as for TGSR:
s->regs[index] &= ~(value & 0xf); /* all bits W1C */
If some but not all bits are W1C, as for TCSR*:
s->regs[index] &= ~(value & 0x80); /* W1C bits */
s->regs[index] &= ~0x7d; /* writable fields */
s->regs[index] |= (value & 0x7d);
thanks
-- PMM
^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH v2] hw/net/imx_fec: write TGSR and TCSR3 in imx_enet_write()
2020-03-09 11:35 ` Peter Maydell
@ 2020-03-10 8:08 ` Chenqun (kuhn)
2020-03-12 17:00 ` Peter Maydell
0 siblings, 1 reply; 8+ messages in thread
From: Chenqun (kuhn) @ 2020-03-10 8:08 UTC (permalink / raw)
To: Peter Maydell
Cc: Zhanghailiang, QEMU Trivial, Jason Wang, QEMU Developers,
qemu-arm, Peter Chubb, Euler Robot
>-----Original Message-----
>From: Peter Maydell [mailto:peter.maydell@linaro.org]
>Sent: Monday, March 9, 2020 7:36 PM
>To: Chenqun (kuhn) <kuhn.chenqun@huawei.com>
>Cc: QEMU Developers <qemu-devel@nongnu.org>; QEMU Trivial <qemu-
>trivial@nongnu.org>; Zhanghailiang <zhang.zhanghailiang@huawei.com>;
>Jason Wang <jasowang@redhat.com>; Peter Chubb
><peter.chubb@nicta.com.au>; qemu-arm <qemu-arm@nongnu.org>; Euler
>Robot <euler.robot@huawei.com>
>Subject: Re: [PATCH v2] hw/net/imx_fec: write TGSR and TCSR3 in
>imx_enet_write()
>
>On Thu, 5 Mar 2020 at 10:53, Chen Qun <kuhn.chenqun@huawei.com> 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>
>> ---
>> v1->v2:
>> The register 'ENET_TGSR' write-1-to-clear timer flag.
>> The register 'ENET_TCSRn' 7bit(TF) write-1-to-clear timer flag.
>> ---
>> hw/net/imx_fec.c | 6 ++++--
>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/net/imx_fec.c b/hw/net/imx_fec.c index
>> 6a124a154a..322cbdcc17 100644
>> --- a/hw/net/imx_fec.c
>> +++ b/hw/net/imx_fec.c
>> @@ -855,13 +855,15 @@ 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] ^= s->regs[index] & value;
>> + s->regs[index] &= 0x0000000f;
>> break;
>> case ENET_TCSR0:
>> case ENET_TCSR1:
>> case ENET_TCSR2:
>> case ENET_TCSR3:
>> - value = value & 0x000000fd;
>> + s->regs[index] = (value & 0x00000080) ? (0x0000007d & value) :
>> + (value & 0x000000fd);
>> break;
>> case ENET_TCCR0:
>> case ENET_TCCR1:
>
>This isn't the usual way to write W1C behaviour.
>If all the relevant bits are W1C, as for TGSR:
>
> s->regs[index] &= ~(value & 0xf); /* all bits W1C */
>
Yes, it looks better.
But do we need clear the reserved bit (31 - 4 bits) explicitly ?
We see that other registers have explicitly cleared the reserved bit, which also meets the I.MX datasheet requirements.
s->regs[index] &= ~(value & 0xf) & 0xf; /* 0-3 bits W1C, 4-31 reserved bits write zero */
>If some but not all bits are W1C, as for TCSR*:
>
Yes, this patch is just only W1C for 7bit, not all bits for TCSRn.
But do we need clear the reserved bit (31 - 8 bits) explicitly ?
> s->regs[index] &= ~(value & 0x80); /* W1C bits */
s->regs[index] &= ~(value & 0x80) & 0xff ; /* 7 bits W1C, 8-31 reserved bits write zero */
> s->regs[index] &= ~0x7d; /* writable fields */
> s->regs[index] |= (value & 0x7d);
>
>thanks
>-- PMM
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] hw/net/imx_fec: write TGSR and TCSR3 in imx_enet_write()
2020-03-10 8:08 ` Chenqun (kuhn)
@ 2020-03-12 17:00 ` Peter Maydell
2020-03-13 2:08 ` Chenqun (kuhn)
0 siblings, 1 reply; 8+ messages in thread
From: Peter Maydell @ 2020-03-12 17:00 UTC (permalink / raw)
To: Chenqun (kuhn)
Cc: Zhanghailiang, QEMU Trivial, Jason Wang, QEMU Developers,
qemu-arm, Peter Chubb, Euler Robot
On Tue, 10 Mar 2020 at 08:08, Chenqun (kuhn) <kuhn.chenqun@huawei.com> wrote:
>
> >-----Original Message-----
> >From: Peter Maydell [mailto:peter.maydell@linaro.org]
> >>
> >> diff --git a/hw/net/imx_fec.c b/hw/net/imx_fec.c index
> >> 6a124a154a..322cbdcc17 100644
> >> --- a/hw/net/imx_fec.c
> >> +++ b/hw/net/imx_fec.c
> >> @@ -855,13 +855,15 @@ 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] ^= s->regs[index] & value;
> >> + s->regs[index] &= 0x0000000f;
> >> break;
> >> case ENET_TCSR0:
> >> case ENET_TCSR1:
> >> case ENET_TCSR2:
> >> case ENET_TCSR3:
> >> - value = value & 0x000000fd;
> >> + s->regs[index] = (value & 0x00000080) ? (0x0000007d & value) :
> >> + (value & 0x000000fd);
> >> break;
> >> case ENET_TCCR0:
> >> case ENET_TCCR1:
> >
> >This isn't the usual way to write W1C behaviour.
> >If all the relevant bits are W1C, as for TGSR:
> >
> > s->regs[index] &= ~(value & 0xf); /* all bits W1C */
> >
> Yes, it looks better.
> But do we need clear the reserved bit (31 - 4 bits) explicitly ?
Not necessarily, but it seems to be how the other registers
in the device have generally been coded, and it's clearly
what the intent was here given that the original (buggy)
code was masking out reserved bits. So I think it makes
sense to continue in that style.
thanks
-- PMM
^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH v2] hw/net/imx_fec: write TGSR and TCSR3 in imx_enet_write()
2020-03-12 17:00 ` Peter Maydell
@ 2020-03-13 2:08 ` Chenqun (kuhn)
0 siblings, 0 replies; 8+ messages in thread
From: Chenqun (kuhn) @ 2020-03-13 2:08 UTC (permalink / raw)
To: Peter Maydell
Cc: Zhanghailiang, QEMU Trivial, Jason Wang, QEMU Developers,
qemu-arm, Peter Chubb, Euler Robot
>-----Original Message-----
>From: Peter Maydell [mailto:peter.maydell@linaro.org]
>Sent: Friday, March 13, 2020 1:01 AM
>To: Chenqun (kuhn) <kuhn.chenqun@huawei.com>
>Cc: QEMU Developers <qemu-devel@nongnu.org>; QEMU Trivial <qemu-
>trivial@nongnu.org>; Zhanghailiang <zhang.zhanghailiang@huawei.com>;
>Jason Wang <jasowang@redhat.com>; Peter Chubb
><peter.chubb@nicta.com.au>; qemu-arm <qemu-arm@nongnu.org>; Euler
>Robot <euler.robot@huawei.com>
>Subject: Re: [PATCH v2] hw/net/imx_fec: write TGSR and TCSR3 in
>imx_enet_write()
>
>On Tue, 10 Mar 2020 at 08:08, Chenqun (kuhn) <kuhn.chenqun@huawei.com>
>wrote:
>>
>> >-----Original Message-----
>> >From: Peter Maydell [mailto:peter.maydell@linaro.org]
>> >>
>> >> diff --git a/hw/net/imx_fec.c b/hw/net/imx_fec.c index
>> >> 6a124a154a..322cbdcc17 100644
>> >> --- a/hw/net/imx_fec.c
>> >> +++ b/hw/net/imx_fec.c
>> >> @@ -855,13 +855,15 @@ 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] ^= s->regs[index] & value;
>> >> + s->regs[index] &= 0x0000000f;
>> >> break;
>> >> case ENET_TCSR0:
>> >> case ENET_TCSR1:
>> >> case ENET_TCSR2:
>> >> case ENET_TCSR3:
>> >> - value = value & 0x000000fd;
>> >> + s->regs[index] = (value & 0x00000080) ? (0x0000007d & value) :
>> >> + (value & 0x000000fd);
>> >> break;
>> >> case ENET_TCCR0:
>> >> case ENET_TCCR1:
>> >
>> >This isn't the usual way to write W1C behaviour.
>> >If all the relevant bits are W1C, as for TGSR:
>> >
>> > s->regs[index] &= ~(value & 0xf); /* all bits W1C */
>> >
>> Yes, it looks better.
>> But do we need clear the reserved bit (31 - 4 bits) explicitly ?
>
>Not necessarily, but it seems to be how the other registers in the device have
>generally been coded, and it's clearly what the intent was here given that the
>original (buggy) code was masking out reserved bits. So I think it makes sense
>to continue in that style.
>
OK, let's keep original code style, and clear reserved bit. I will provide v3 version for it.
Thanks.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-03-13 2:09 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-05 10:53 [PATCH v2] hw/net/imx_fec: write TGSR and TCSR3 in imx_enet_write() Chen Qun
2020-03-05 11:04 ` Chenqun (kuhn)
2020-03-05 11:50 ` no-reply
2020-03-05 11:55 ` no-reply
2020-03-09 11:35 ` Peter Maydell
2020-03-10 8:08 ` Chenqun (kuhn)
2020-03-12 17:00 ` Peter Maydell
2020-03-13 2:08 ` Chenqun (kuhn)
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.