[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
RE: [PATCH v2] hw/net/imx_fec: write TGSR and TCSR3 in imx_enet_write()
From: |
Chenqun (kuhn) |
Subject: |
RE: [PATCH v2] hw/net/imx_fec: write TGSR and TCSR3 in imx_enet_write() |
Date: |
Fri, 13 Mar 2020 02:08:37 +0000 |
>-----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.