Hi Edgar,
> -----Original Message-----
> From: Edgar E. Iglesias <edgar.igles...@gmail.com>
> Sent: Monday, May 4, 2020 8:27 PM
> To: Sai Pavan Boddu <saip...@xilinx.com>
> Cc: Alistair Francis <alistair.fran...@wdc.com>; Peter Maydell
> <peter.mayd...@linaro.org>; Jason Wang <jasow...@redhat.com>; Markus
> Armbruster <arm...@redhat.com>; Philippe Mathieu-Daudé
> <phi...@redhat.com>; Tong Ho <to...@xilinx.com>; Ramon Fried
> <rfried....@gmail.com>; qemu-...@nongnu.org; qemu-
> de...@nongnu.org
> Subject: Re: [PATCH v2 04/10] net: cadence_gem: Define access permission
> for interrupt registers
>
> On Mon, May 04, 2020 at 07:36:02PM +0530, Sai Pavan Boddu wrote:
> > Q1 to Q7 ISR's are clear-on-read, IER/IDR registers are write-only,
> > mask reg are read-only.
> >
> > Signed-off-by: Sai Pavan Boddu <sai.pavan.bo...@xilinx.com>
> > ---
> > hw/net/cadence_gem.c | 14 ++++++++++++++
> > 1 file changed, 14 insertions(+)
> >
> > diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c index
> > a930bf1..c532a14 100644
> > --- a/hw/net/cadence_gem.c
> > +++ b/hw/net/cadence_gem.c
> > @@ -458,6 +458,7 @@ static const uint8_t broadcast_addr[] = { 0xFF, 0xFF,
> 0xFF, 0xFF, 0xFF, 0xFF };
> > */
> > static void gem_init_register_masks(CadenceGEMState *s) {
> > + unsigned int i;
> > /* Mask of register bits which are read only */
> > memset(&s->regs_ro[0], 0, sizeof(s->regs_ro));
> > s->regs_ro[GEM_NWCTRL] = 0xFFF80000;
> > @@ -470,10 +471,19 @@ static void
> gem_init_register_masks(CadenceGEMState *s)
> > s->regs_ro[GEM_ISR] = 0xFFFFFFFF;
> > s->regs_ro[GEM_IMR] = 0xFFFFFFFF;
> > s->regs_ro[GEM_MODID] = 0xFFFFFFFF;
> > + for (i = 0; i < s->num_priority_queues; i++) {
> > + s->regs_ro[GEM_INT_Q1_STATUS + i] = 0xFFFFFFFF;
> > + s->regs_ro[GEM_INT_Q1_ENABLE + i] = 0xFFFFE319;
> > + s->regs_ro[GEM_INT_Q1_DISABLE + i] = 0xFFFFE319;
>
> Shouldn't these be 0xfffff319?
[Sai Pavan Boddu] This one is right. I would fix it thanks.
Regards,
Sai Pavan
> Perhaps I'm looking at old specs but mine says bits upper bits [31:12] are
> reserved and read-only.
>
>
> With that fixed:
> Reviewed-by: Edgar E. Iglesias <edgar.igles...@xilinx.com>
>
>
>
>
>
> > + s->regs_ro[GEM_INT_Q1_MASK + i] = 0xFFFFFFFF;
>
> > + }
> >
> > /* Mask of register bits which are clear on read */
> > memset(&s->regs_rtc[0], 0, sizeof(s->regs_rtc));
> > s->regs_rtc[GEM_ISR] = 0xFFFFFFFF;
> > + for (i = 0; i < s->num_priority_queues; i++) {
> > + s->regs_rtc[GEM_INT_Q1_STATUS + i] = 0x00000CE6;
> > + }
> >
> > /* Mask of register bits which are write 1 to clear */
> > memset(&s->regs_w1c[0], 0, sizeof(s->regs_w1c)); @@ -485,6
> > +495,10 @@ static void gem_init_register_masks(CadenceGEMState *s)
> > s->regs_wo[GEM_NWCTRL] = 0x00073E60;
> > s->regs_wo[GEM_IER] = 0x07FFFFFF;
> > s->regs_wo[GEM_IDR] = 0x07FFFFFF;
> > + for (i = 0; i < s->num_priority_queues; i++) {
> > + s->regs_wo[GEM_INT_Q1_ENABLE + i] = 0x00000CE6;
> > + s->regs_wo[GEM_INT_Q1_DISABLE + i] = 0x00000CE6;
> > + }
> > }
> >
> > /*
> > --
> > 2.7.4
> >
