1. Patchew
  2. QEMU
  3. [PATCH v2 00/10] Cadence GEM Fixes
  4. View patch

[PATCH v2 05/10] net: cadence_gem: Set ISR according to queue in use

Sai Pavan Boddu posted 10 patches 5 years, 9 months ago
Maintainers: Alistair Francis <alistair@alistair23.me>, "Edgar E. Iglesias" <edgar.iglesias@gmail.com>, Jason Wang <jasowang@redhat.com>, Peter Maydell <peter.maydell@linaro.org>
There is a newer version of this series
[PATCH v2 05/10] net: cadence_gem: Set ISR according to queue in use
Posted by Sai Pavan Boddu 5 years, 9 months ago 
Set ISR according to queue in use, added interrupt support for
all queues.

Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@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
Re: [PATCH v2 05/10] net: cadence_gem: Set ISR according to queue in use
Posted by Edgar E. Iglesias 5 years, 9 months ago 
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...

Anyway, the logic looks good to me:
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>



> 
> Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@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
>
RE: [PATCH v2 05/10] net: cadence_gem: Set ISR according to queue in use
Posted by Sai Pavan Boddu 5 years, 9 months ago 
Hi Edgar,

> -----Original Message-----
> From: Edgar E. Iglesias <edgar.iglesias@gmail.com>
> Sent: Monday, May 4, 2020 8:32 PM
> To: Sai Pavan Boddu <saipava@xilinx.com>
> Cc: Alistair Francis <Alistair.Francis@wdc.com>; Peter Maydell
> <peter.maydell@linaro.org>; Jason Wang <jasowang@redhat.com>; Markus
> Armbruster <armbru@redhat.com>; Philippe Mathieu-Daudé
> <philmd@redhat.com>; Tong Ho <tongh@xilinx.com>; Ramon Fried
> <rfried.dev@gmail.com>; qemu-arm@nongnu.org; qemu-
> devel@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.iglesias@xilinx.com>
> 
> 
> 
> >
> > Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@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
> >

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.