RE: [PATCH v2] hw/net/imx_fec: write TGSR and TCSR3 in imx_enet_write()

[Chenqun (kuhn)]

>-----Original Message-----
>From: Peter Maydell [mailto:address@hidden]
>Sent: Friday, March 13, 2020 1:01 AM
>To: Chenqun (kuhn) <address@hidden>
>Cc: QEMU Developers <address@hidden>; QEMU Trivial <qemu-
>address@hidden>; Zhanghailiang <address@hidden>;
>Jason Wang <address@hidden>; Peter Chubb
><address@hidden>; qemu-arm <address@hidden>; Euler
>Robot <address@hidden>
>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) <address@hidden>
>wrote:
>>
>> >-----Original Message-----
>> >From: Peter Maydell [mailto:address@hidden]
>> >>
>> >> 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.