>-----Original Message----- >From: Peter Maydell [mailto:peter.mayd...@linaro.org] >Sent: Friday, March 13, 2020 1:01 AM >To: Chenqun (kuhn) <kuhn.chen...@huawei.com> >Cc: QEMU Developers <qemu-devel@nongnu.org>; QEMU Trivial <qemu- >triv...@nongnu.org>; Zhanghailiang <zhang.zhanghaili...@huawei.com>; >Jason Wang <jasow...@redhat.com>; Peter Chubb ><peter.ch...@nicta.com.au>; qemu-arm <qemu-...@nongnu.org>; Euler >Robot <euler.ro...@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.chen...@huawei.com> >wrote: >> >> >-----Original Message----- >> >From: Peter Maydell [mailto:peter.mayd...@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.
