- incorporated PR review comments

drivers/include/w5500.h

@@ -46,13 +46,6 @@
 extern "C" {
 #endif
 
-/**
- * @brief   W5500 error codes
- */
-enum {
-    W5500_ERR_BUS = -1,
-};
-
 /**
  * @brief   W5500 device descriptor
  */
@@ -68,12 +61,12 @@ typedef struct {
  * @brief   Device descriptor for W5500 devices
  */
 typedef struct w5500 {
-    netdev_t netdev;                /**< extends the netdev structure */
-    w5500_params_t p;               /**< device configuration parameters */
-    uint16_t frame_size_to_be_send; /**< size of the frame which is currently being send */
-    uint16_t frame_size_sent;       /**< size of the frame which has been send */
-    bool link_up;                   /**< used to prevent sending the same LINK event twice */
-    ztimer_t timerInstance;         /**< stores the polling interval timer in polling mode */
+    netdev_t netdev;           /**< extends the netdev structure */
+    w5500_params_t p;          /**< device configuration parameters */
+    uint16_t frame_size;       /**< size of the frame which has been send */
+    bool link_up;              /**< used to prevent sending the same LINK event twice */
+    bool frame_sent;           /**< indicates that the frame has been transmitted */
+    ztimer_t timerInstance;    /**< stores the polling interval timer in polling mode */
 } w5500_t;
 
 /**

drivers/w5500/include/w5500_params.h

@@ -39,10 +39,10 @@ extern "C" {
 #define W5500_PARAM_CS          (GPIO_PIN(0, 27))   /**< Default SPI chip select pin */
 #endif
 #ifndef W5500_PARAM_EVT
-#define CONFIG_W5500_POLLING_INTERVAL 100u          /**< default polling interval 100 ms */
 #define W5500_PARAM_EVT         GPIO_UNDEF          /**< set to invalid */
-#else
-#define CONFIG_W5500_POLLING_INTERVAL 0u
+#endif
+#ifndef CONFIG_W5500_POLLING_INTERVAL
+#define CONFIG_W5500_POLLING_INTERVAL 100u          /**< default polling interval 100 ms */
 #endif
 
 #ifndef W5500_PARAMS

drivers/w5500/w5500.c

@@ -46,13 +46,8 @@ static const netdev_driver_t netdev_driver_w5500;
 
 static inline void send_addr(w5500_t *dev, uint16_t addr)
 {
-#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
     spi_transfer_byte(dev->p.spi, dev->p.cs, true, (addr >> 8));
     spi_transfer_byte(dev->p.spi, dev->p.cs, true, (addr & 0xff));
-#else
-    spi_transfer_byte(dev->p.spi, dev->p.cs, true, (addr & 0xff));
-    spi_transfer_byte(dev->p.spi, dev->p.cs, true, (addr >> 8));
-#endif
 }
 
 static uint8_t read_register(w5500_t *dev, uint16_t reg)
@@ -115,7 +110,7 @@ static void extint(void *arg)
     w5500_t *dev = (w5500_t *)arg;
 
     netdev_trigger_event_isr(&dev->netdev);
-    if (dev->p.polling_interval_ms > 0u) {
+    if (!gpio_is_valid(dev->p.evt)) {
         /* restart timer if we are polling */
         (void)ztimer_set(ZTIMER_MSEC, &dev->timerInstance, dev->p.polling_interval_ms);
     }
@@ -133,7 +128,9 @@ void w5500_setup(w5500_t *dev, const w5500_params_t *params, uint8_t index)
     /* Initialize the device descriptor. */
     dev->p = *params;
 
+    dev->frame_size = 0u;
     dev->link_up = false;
+    dev->frame_sent = false;
 
     /* Initialize the chip select pin. */
     spi_init_cs(dev->p.spi, dev->p.cs);
@@ -144,24 +141,11 @@ void w5500_setup(w5500_t *dev, const w5500_params_t *params, uint8_t index)
     netdev_register(&dev->netdev, NETDEV_W5500, index);
 }
 
-static bool check_configuration(w5500_t *dev)
-{
-    /* Polling mode: W5500_PARAM_EVT == GPIO_UNDEF and polling_interval_ms > 0u    */
-    /* Interrupt mode: W5500_PARAM_EVT != GPIO_UNDEF and polling_interval_ms == 0u */
-    return((dev->p.evt == GPIO_UNDEF && dev->p.polling_interval_ms > 0u) ||
-           (dev->p.evt != GPIO_UNDEF && dev->p.polling_interval_ms == 0u));
-}
-
 static int init(netdev_t *netdev)
 {
     w5500_t *dev = (w5500_t *)netdev;
     uint8_t tmp;
 
-    if (check_configuration(dev) == false) {
-        LOG_ERROR("[w5500] error: Check configuration: W5500_PARAM_EVT and polling_interval_ms\n");
-        return -EINVAL;
-    }
-
     spi_acquire(dev->p.spi, dev->p.cs, SPI_CONF, dev->p.clk);
 
     /* Reset the device. */
@@ -172,8 +156,8 @@ static int init(netdev_t *netdev)
     tmp = read_register(dev, REG_VERSIONR);
     if (tmp != CHIP_VERSION) {
         spi_release(dev->p.spi);
-        LOG_ERROR("[w5500] error: no SPI connection\n");
-        return W5500_ERR_BUS;
+        LOG_ERROR("[w5500] error: invalid chip ID %x\n", tmp);
+        return -ENODEV;
     }
 
     /* Write the MAC address. */
@@ -203,7 +187,7 @@ static int init(netdev_t *netdev)
     write_register(dev, REG_SIPR2, 0x00);
     write_register(dev, REG_SIPR3, 0x00);
 
-    if (dev->p.polling_interval_ms > 0u) {
+    if (!gpio_is_valid(dev->p.evt)) {
         dev->timerInstance.callback = extint;
         dev->timerInstance.arg = dev;
         (void)ztimer_set(ZTIMER_MSEC, &dev->timerInstance, dev->p.polling_interval_ms);
@@ -264,18 +248,17 @@ static int send(netdev_t *netdev, const iolist_t *iolist)
         /* Make sure we can write the full packet. */
         if (data_length <= tx_free) {
             /* reset the frame size information */
-            dev->frame_size_sent = 0u;
-            dev->frame_size_to_be_send = 0u;
+            dev->frame_size = 0u;
 
             for (const iolist_t *iol = iolist; iol; iol = iol->iol_next) {
                 size_t len = iol->iol_len;
-                write_tx0_buffer(dev, socket0_write_pointer + dev->frame_size_to_be_send,
+                write_tx0_buffer(dev, socket0_write_pointer + dev->frame_size,
                                  iol->iol_base, len);
-                dev->frame_size_to_be_send += len;
+                dev->frame_size += len;
             }
             /* Update the write pointer. */
             write_address(dev, REG_S0_TX_WR0, REG_S0_TX_WR1,
-                  socket0_write_pointer + dev->frame_size_to_be_send);
+                  socket0_write_pointer + dev->frame_size);
             /* Trigger the sending process. */
             write_register(dev, REG_S0_CR, CR_SEND);
 
@@ -301,11 +284,11 @@ static int confirm_send(netdev_t *netdev, void *info)
 
     (void)info;
 
-    if (dev->frame_size_sent == 0u) {
+    if (dev->frame_sent == false) {
         return -EAGAIN;
     }
     else {
-        return dev->frame_size_sent;
+        return dev->frame_size;
     }
 }
 
@@ -416,8 +399,7 @@ static void isr(netdev_t *netdev)
         }
         if (socket0_interrupt_status & IR_SEND_OK) {
             DEBUG("[w5500] netdev TX complete\n");
-            dev->frame_size_sent = dev->frame_size_to_be_send;
-            dev->frame_size_to_be_send = 0u;
+            dev->frame_sent = true;
             netdev->event_callback(netdev, NETDEV_EVENT_TX_COMPLETE);
         }
 

sys/net/gnrc/netif/init_devs/auto_init_w5500.c

@@ -22,14 +22,6 @@
 #include "net/gnrc/netif/ethernet.h"
 #include "include/init_devs.h"
 
-/**
- * @brief   Define stack parameters for the MAC layer thread
- * @{
- */
-#define MAC_STACKSIZE   (GNRC_NETIF_STACKSIZE_DEFAULT)
-#define MAC_PRIO        (GNRC_NETIF_PRIO)
-/*** @} */
-
 /**
  * @brief   Find out how many of these devices we need to care for
  */
@@ -47,7 +39,7 @@ static gnrc_netif_t _netif[W5500_NUM];
 /**
  * @brief   Stacks for the MAC layer threads
  */
-static char stack[W5500_NUM][MAC_STACKSIZE];
+static char stack[W5500_NUM][GNRC_NETIF_STACKSIZE_DEFAULT];
 
 void auto_init_w5500(void)
 {
@@ -57,8 +49,8 @@ void auto_init_w5500(void)
         /* setup netdev device */
         w5500_setup(&dev[i], &w5500_params[i], i);
         /* initialize netdev <-> gnrc adapter state */
-        gnrc_netif_ethernet_create(&_netif[i], stack[i], MAC_STACKSIZE, MAC_PRIO, "w5500",
-                                   &dev[i].netdev);
+        gnrc_netif_ethernet_create(&_netif[i], stack[i], GNRC_NETIF_STACKSIZE_DEFAULT,
+                                   GNRC_NETIF_PRIO, "w5500", &dev[i].netdev);
     }
 }
 /** @} */