Hi Edgar,
> -----Original Message-----
> From: Edgar E. Iglesias <edgar.igles...@gmail.com>
> Sent: Monday, May 4, 2020 8:32 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 05/10] net: cadence_gem: Set ISR according to queue
> in use
>
> On Mon, May 04, 2020 at 07:36:03PM +0530, Sai Pavan Boddu wrote:
> > Set ISR according to queue in use, added interrupt support for all
> > queues.
>
> Would it help to add a gem_set_isr(CadenceGEMState *s, int q, uint32_t
> flag) ?
> Instead of open coding these if (q == 0) else... all over the place...
[Sai Pavan Boddu] Yeah, it would be nice. Will try to include this in v3
Thanks,
Sai Pavan
>
> Anyway, the logic looks good to me:
> Reviewed-by: Edgar E. Iglesias <edgar.igles...@xilinx.com>
>
>
>
> >
> > Signed-off-by: Sai Pavan Boddu <sai.pavan.bo...@xilinx.com>
> > ---
> > hw/net/cadence_gem.c | 31 ++++++++++++++++++++++---------
> > 1 file changed, 22 insertions(+), 9 deletions(-)
> >
> > diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c index
> > c532a14..beb38ec 100644
> > --- a/hw/net/cadence_gem.c
> > +++ b/hw/net/cadence_gem.c
> > @@ -896,7 +896,13 @@ static void gem_get_rx_desc(CadenceGEMState
> *s, int q)
> > if (rx_desc_get_ownership(s->rx_desc[q]) == 1) {
> > DB_PRINT("descriptor 0x%" HWADDR_PRIx " owned by sw.\n",
> desc_addr);
> > s->regs[GEM_RXSTATUS] |= GEM_RXSTATUS_NOBUF;
> > - s->regs[GEM_ISR] |= GEM_INT_RXUSED & ~(s->regs[GEM_IMR]);
> > + if (q == 0) {
> > + s->regs[GEM_ISR] |= GEM_INT_RXUSED & ~(s->regs[GEM_IMR]);
> > + } else {
> > + s->regs[GEM_INT_Q1_STATUS + q - 1] |= GEM_INT_RXUSED &
> > + ~(s->regs[GEM_INT_Q1_MASK + q -
> > 1]);
> > + }
> > +
> > /* Handle interrupt consequences */
> > gem_update_int_status(s);
> > }
> > @@ -1071,8 +1077,12 @@ static ssize_t gem_receive(NetClientState *nc,
> const uint8_t *buf, size_t size)
> > gem_receive_updatestats(s, buf, size);
> >
> > s->regs[GEM_RXSTATUS] |= GEM_RXSTATUS_FRMRCVD;
> > - s->regs[GEM_ISR] |= GEM_INT_RXCMPL & ~(s->regs[GEM_IMR]);
> > -
> > + if (q == 0) {
> > + s->regs[GEM_ISR] |= GEM_INT_RXCMPL & ~(s->regs[GEM_IMR]);
> > + } else {
> > + s->regs[GEM_INT_Q1_STATUS + q - 1] |= GEM_INT_RXCMPL &
> > + ~(s->regs[GEM_INT_Q1_MASK + q - 1]);
> > + }
> > /* Handle interrupt consequences */
> > gem_update_int_status(s);
> >
> > @@ -1223,12 +1233,12 @@ static void gem_transmit(CadenceGEMState
> *s)
> > DB_PRINT("TX descriptor next: 0x%08x\n",
> > s->tx_desc_addr[q]);
> >
> > s->regs[GEM_TXSTATUS] |= GEM_TXSTATUS_TXCMPL;
> > - s->regs[GEM_ISR] |= GEM_INT_TXCMPL & ~(s->regs[GEM_IMR]);
> > -
> > + if (q == 0) {
> > + s->regs[GEM_ISR] |= GEM_INT_TXCMPL & ~(s-
> >regs[GEM_IMR]);
> > + } else {
> > /* Update queue interrupt status */
> > - if (s->num_priority_queues > 1) {
> > - s->regs[GEM_INT_Q1_STATUS + q] |=
> > - GEM_INT_TXCMPL & ~(s->regs[GEM_INT_Q1_MASK +
> > q]);
> > + s->regs[GEM_INT_Q1_STATUS + q - 1] |=
> > + GEM_INT_TXCMPL & ~s->regs[GEM_INT_Q1_MASK
> > + + q - 1];
> > }
> >
> > /* Handle interrupt consequences */ @@ -1280,7
> > +1290,10 @@ static void gem_transmit(CadenceGEMState *s)
> >
> > if (tx_desc_get_used(desc)) {
> > s->regs[GEM_TXSTATUS] |= GEM_TXSTATUS_USED;
> > - s->regs[GEM_ISR] |= GEM_INT_TXUSED & ~(s->regs[GEM_IMR]);
> > + /* IRQ TXUSED is defined only for queue 0 */
> > + if (q == 0) {
> > + s->regs[GEM_ISR] |= GEM_INT_TXUSED & ~(s-
> >regs[GEM_IMR]);
> > + }
> > gem_update_int_status(s);
> > }
> > }
> > --
> > 2.7.4
> >
