[Qemu-devel] [PATCH trivial v2 0/4] Cadence GEM patches

[Qemu-devel] [PATCH trivial v2 0/4] Cadence GEM patches

[Peter Crosthwaite]

Hi Michael,

Found a bug in the Cadence GEM. Fixed in P1. Have some follow up
trivials as well (P2-4).

Changed since v1:
Addressed mjt review
Dropped "top comment patch" (applied to trivial already).
Fixed other &foo[0] usages

Regards,
Peter


Peter Crosthwaite (4):
  net: cadence_gem: Fix Tx descriptor update
  net: cadence_gem: Add Tx descriptor fetch printf
  net: cadence_gem: Comment spelling sweep
  net: cadence_gem: Remove &desc[0] usages

 hw/net/cadence_gem.c | 31 ++++++++++++++++++-------------
 1 file changed, 18 insertions(+), 13 deletions(-)

-- 
1.9.3.1.ga73a6ad

[Qemu-devel] [PATCH trivial v2 1/4] net: cadence_gem: Fix Tx descriptor update

[Peter Crosthwaite]

The local variable "desc" was being used to read-modify-write the
first descriptor (of a multi-desc packet) upon packet completion.
desc however continues to be used by the code as the current
descriptor. Give this first desc RMW it's own local variable to
avoid trampling.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---
changed since v1 (MJT review):
Removed &foo[0] usage
Corrected sizeof(desc) to sizeof(desc_first)

 hw/net/cadence_gem.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index 47e7038..1721fb7 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -911,14 +911,16 @@ static void gem_transmit(GemState *s)
 
         /* Last descriptor for this packet; hand the whole thing off */
         if (tx_desc_get_last(desc)) {
+            unsigned    desc_first[2];
+
             /* Modify the 1st descriptor of this packet to be owned by
              * the processor.
              */
-            cpu_physical_memory_read(s->tx_desc_addr,
-                                     (uint8_t *)&desc[0], sizeof(desc));
-            tx_desc_set_used(desc);
-            cpu_physical_memory_write(s->tx_desc_addr,
-                                      (uint8_t *)&desc[0], sizeof(desc));
+            cpu_physical_memory_read(s->tx_desc_addr, (uint8_t *)desc_first,
+                                     sizeof(desc_first));
+            tx_desc_set_used(desc_first);
+            cpu_physical_memory_write(s->tx_desc_addr, (uint8_t *)desc_first,
+                                      sizeof(desc_first));
             /* Advance the hardare current descriptor past this packet */
             if (tx_desc_get_wrap(desc)) {
                 s->tx_desc_addr = s->regs[GEM_TXQBASE];
-- 
1.9.3.1.ga73a6ad

[Qemu-devel] [PATCH trivial v2 2/4] net: cadence_gem: Add Tx descriptor fetch printf

[Peter Crosthwaite]

Add a debug printf for TX descriptor fetching. This helpful to anyone
needing to debug TX ring buffer traversal. Its also now consistent with
the RX code which has a similar printf.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---
changed since v1:
don't cast to unsigned (mjt review).

 hw/net/cadence_gem.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index 1721fb7..f0e9f5b 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -880,6 +880,8 @@ static void gem_transmit(GemState *s)
 
     /* read current descriptor */
     packet_desc_addr = s->tx_desc_addr;
+
+    DB_PRINT("read descriptor 0x%" HWADDR_PRIx "\n", packet_desc_addr);
     cpu_physical_memory_read(packet_desc_addr,
                              (uint8_t *)&desc[0], sizeof(desc));
     /* Handle all descriptors owned by hardware */
@@ -962,6 +964,7 @@ static void gem_transmit(GemState *s)
         } else {
             packet_desc_addr += 8;
         }
+        DB_PRINT("read descriptor 0x%" HWADDR_PRIx "\n", packet_desc_addr);
         cpu_physical_memory_read(packet_desc_addr,
                                  (uint8_t *)&desc[0], sizeof(desc));
     }
-- 
1.9.3.1.ga73a6ad

[Qemu-devel] [PATCH trivial v2 3/4] net: cadence_gem: Comment spelling sweep

[Peter Crosthwaite]

Fix some typos in comments.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 hw/net/cadence_gem.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index f0e9f5b..7cd4b54 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -50,7 +50,7 @@
 #define GEM_IER           (0x00000028/4) /* Interrupt Enable reg */
 #define GEM_IDR           (0x0000002C/4) /* Interrupt Disable reg */
 #define GEM_IMR           (0x00000030/4) /* Interrupt Mask reg */
-#define GEM_PHYMNTNC      (0x00000034/4) /* Phy Maintaince reg */
+#define GEM_PHYMNTNC      (0x00000034/4) /* Phy Maintenance reg */
 #define GEM_RXPAUSE       (0x00000038/4) /* RX Pause Time reg */
 #define GEM_TXPAUSE       (0x0000003C/4) /* TX Pause Time reg */
 #define GEM_TXPARTIALSF   (0x00000040/4) /* TX Partial Store and Forward */
@@ -150,7 +150,7 @@
 #define GEM_NWCTRL_LOCALLOOP   0x00000002 /* Local Loopback */
 
 #define GEM_NWCFG_STRIP_FCS    0x00020000 /* Strip FCS field */
-#define GEM_NWCFG_LERR_DISC    0x00010000 /* Discard RX frames with lenth err */
+#define GEM_NWCFG_LERR_DISC    0x00010000 /* Discard RX frames with len err */
 #define GEM_NWCFG_BUFF_OFST_M  0x0000C000 /* Receive buffer offset mask */
 #define GEM_NWCFG_BUFF_OFST_S  14         /* Receive buffer offset shift */
 #define GEM_NWCFG_UCAST_HASH   0x00000080 /* accept unicast if hash match */
@@ -397,7 +397,7 @@ static const uint8_t broadcast_addr[] = { 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF };
  */
 static void gem_init_register_masks(GemState *s)
 {
-    /* Mask of register bits which are read only*/
+    /* Mask of register bits which are read only */
     memset(&s->regs_ro[0], 0, sizeof(s->regs_ro));
     s->regs_ro[GEM_NWCTRL]   = 0xFFF80000;
     s->regs_ro[GEM_NWSTATUS] = 0xFFFFFFFF;
@@ -719,7 +719,7 @@ static ssize_t gem_receive(NetClientState *nc, const uint8_t *buf, size_t size)
         unsigned crc_val;
 
         /* The application wants the FCS field, which QEMU does not provide.
-         * We must try and caclculate one.
+         * We must try and calculate one.
          */
 
         memcpy(rxbuf, buf, size);
@@ -871,7 +871,7 @@ static void gem_transmit(GemState *s)
 
     DB_PRINT("\n");
 
-    /* The packet we will hand off to qemu.
+    /* The packet we will hand off to QEMU.
      * Packets scattered across multiple descriptors are gathered to this
      * one contiguous buffer first.
      */
@@ -923,7 +923,7 @@ static void gem_transmit(GemState *s)
             tx_desc_set_used(desc_first);
             cpu_physical_memory_write(s->tx_desc_addr, (uint8_t *)desc_first,
                                       sizeof(desc_first));
-            /* Advance the hardare current descriptor past this packet */
+            /* Advance the hardware current descriptor past this packet */
             if (tx_desc_get_wrap(desc)) {
                 s->tx_desc_addr = s->regs[GEM_TXQBASE];
             } else {
-- 
1.9.3.1.ga73a6ad

[Qemu-devel] [PATCH trivial v2 4/4] net: cadence_gem: Remove &desc[0] usages

[Peter Crosthwaite]

Just use desc instead.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 hw/net/cadence_gem.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index 7cd4b54..7d3355e 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -883,7 +883,7 @@ static void gem_transmit(GemState *s)
 
     DB_PRINT("read descriptor 0x%" HWADDR_PRIx "\n", packet_desc_addr);
     cpu_physical_memory_read(packet_desc_addr,
-                             (uint8_t *)&desc[0], sizeof(desc));
+                             (uint8_t *)desc, sizeof(desc));
     /* Handle all descriptors owned by hardware */
     while (tx_desc_get_used(desc) == 0) {
 
@@ -966,7 +966,7 @@ static void gem_transmit(GemState *s)
         }
         DB_PRINT("read descriptor 0x%" HWADDR_PRIx "\n", packet_desc_addr);
         cpu_physical_memory_read(packet_desc_addr,
-                                 (uint8_t *)&desc[0], sizeof(desc));
+                                 (uint8_t *)desc, sizeof(desc));
     }
 
     if (tx_desc_get_used(desc)) {
-- 
1.9.3.1.ga73a6ad