Cadence GEM patches

PATCH Applied Peter Crosthwaite 2 participants 6 messages May 26, 2014 – May 28, 2014

lore

Share & tools

lore

Peter Crosthwaite

author

2014-05-26 08:40 UTC #0 lore raw

v2 cover Cadence GEM patches

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(-)

Signature · 1 line

1.9.3.1.ga73a6ad

Peter Crosthwaite

author

2014-05-26 08:40 UTC #1 lore raw

PATCH v2 1/4 net: cadence_gem: Fix Tx descriptor update

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 <[email protected]>

---
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(-)

hw/net/cadence_gem.c +7 -5

index 47e7038..1721fb7 100644
@@ -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];

Signature · 1 line

1.9.3.1.ga73a6ad

Peter Crosthwaite

author

2014-05-26 08:41 UTC #2 lore raw

PATCH v2 2/4 net: cadence_gem: Add Tx descriptor fetch printf

In reply to 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 <[email protected]>

---
changed since v1:
don't cast to unsigned (mjt review).

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

hw/net/cadence_gem.c +3 -0

index 1721fb7..f0e9f5b 100644
@@ -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));
     }

Signature · 1 line

1.9.3.1.ga73a6ad

Peter Crosthwaite

author

2014-05-26 08:42 UTC #3 lore raw

PATCH v2 3/4 net: cadence_gem: Comment spelling sweep

In reply to Peter Crosthwaite

Fix some typos in comments.

Signed-off-by Peter Crosthwaite <[email protected]>

---

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

hw/net/cadence_gem.c +6 -6

index f0e9f5b..7cd4b54 100644
@@ -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 {

Signature · 1 line

1.9.3.1.ga73a6ad

Peter Crosthwaite

author

2014-05-26 08:42 UTC #4 lore raw

PATCH v2 4/4 net: cadence_gem: Remove &desc[0] usages

In reply to Peter Crosthwaite

Just use desc instead.

Signed-off-by Peter Crosthwaite <[email protected]>

---

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

hw/net/cadence_gem.c +2 -2

index 7cd4b54..7d3355e 100644
@@ -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)) {

Signature · 1 line

1.9.3.1.ga73a6ad

Michael Tokarev

Applied

2014-05-28 08:44 UTC #5 lore raw

In reply to Peter Crosthwaite

Quoted 4 lines · Peter Crosthwaite

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

Applied to -trivial, with some very minor comment grammar tweak in 2/4.

Thank you!

/mjt