Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

W5500 Ethernet without interrupt (IDFGH-11561) #12682

Closed
br101 opened this issue Nov 28, 2023 · 6 comments
Closed

W5500 Ethernet without interrupt (IDFGH-11561) #12682

br101 opened this issue Nov 28, 2023 · 6 comments
Assignees
Labels
Resolution: NA Issue resolution is unavailable Status: Done Issue is done internally Type: Feature Request Feature request for IDF

Comments

@br101
Copy link

br101 commented Nov 28, 2023

Is your feature request related to a problem?

I have encountered a few different hardware designs which connect the W5500 to an ESP without the INT line connected (the Arduino Ethernet Shield 2 or M5Stack PoECAM for example) and it would be good to be able to support it with the W5500 driver in esp-idf.

Describe the solution you'd like.

When the INT line is unavailable int_gpio_num could be set to -1 and the driver should work around it by using polling for events.

Describe alternatives you've considered.

No response

Additional context.

No response

@br101 br101 added the Type: Feature Request Feature request for IDF label Nov 28, 2023
@espressif-bot espressif-bot added the Status: Opened Issue is new label Nov 28, 2023
@github-actions github-actions bot changed the title W5500 Ethernet without interrupt W5500 Ethernet without interrupt (IDFGH-11561) Nov 28, 2023
@br101
Copy link
Author

br101 commented Nov 28, 2023

Something like:

diff --git a/components/esp_eth/src/esp_eth_mac_w5500.c b/components/esp_eth/src/esp_eth_mac_w5500.c
index 7a7708d0ca..58596cff4d 100644
--- a/components/esp_eth/src/esp_eth_mac_w5500.c
+++ b/components/esp_eth/src/esp_eth_mac_w5500.c
@@ -652,10 +652,14 @@ static void emac_w5500_task(void *arg)
     uint32_t frame_len = 0;
     uint32_t buf_len = 0;
     while (1) {
-        /* check if the task receives any notification */
-        if (ulTaskNotifyTake(pdTRUE, pdMS_TO_TICKS(1000)) == 0 &&    // if no notification ...
-            gpio_get_level(emac->int_gpio_num) != 0) {               // ...and no interrupt asserted
-            continue;                                                // -> just continue to check again
+        /* if using interrupts: check if the task receives any notification */
+        if (emac->int_gpio_num >= 0) {
+            if (ulTaskNotifyTake(pdTRUE, pdMS_TO_TICKS(1000)) == 0 &&    // if no notification ...
+                gpio_get_level(emac->int_gpio_num) != 0) {               // ...and no interrupt asserted
+                continue;                                                // -> just continue to check again
+            }
+        } else {
+            vTaskDelay(1);
         }
         /* read interrupt status */
         w5500_read(emac, W5500_REG_SOCK_IR(0), &status, sizeof(status));
@@ -702,12 +706,14 @@ static esp_err_t emac_w5500_init(esp_eth_mac_t *mac)
     esp_err_t ret = ESP_OK;
     emac_w5500_t *emac = __containerof(mac, emac_w5500_t, parent);
     esp_eth_mediator_t *eth = emac->eth;
-    esp_rom_gpio_pad_select_gpio(emac->int_gpio_num);
-    gpio_set_direction(emac->int_gpio_num, GPIO_MODE_INPUT);
-    gpio_set_pull_mode(emac->int_gpio_num, GPIO_PULLUP_ONLY);
-    gpio_set_intr_type(emac->int_gpio_num, GPIO_INTR_NEGEDGE); // active low
-    gpio_intr_enable(emac->int_gpio_num);
-    gpio_isr_handler_add(emac->int_gpio_num, w5500_isr_handler, emac);
+    if (emac->int_gpio_num >= 0) {
+        esp_rom_gpio_pad_select_gpio(emac->int_gpio_num);
+        gpio_set_direction(emac->int_gpio_num, GPIO_MODE_INPUT);
+        gpio_set_pull_mode(emac->int_gpio_num, GPIO_PULLUP_ONLY);
+        gpio_set_intr_type(emac->int_gpio_num, GPIO_INTR_NEGEDGE); // active low
+        gpio_intr_enable(emac->int_gpio_num);
+        gpio_isr_handler_add(emac->int_gpio_num, w5500_isr_handler, emac);
+    }
     ESP_GOTO_ON_ERROR(eth->on_state_changed(eth, ETH_STATE_LLINIT, NULL), err, TAG, "lowlevel init failed");
     /* reset w5500 */
     ESP_GOTO_ON_ERROR(w5500_reset(emac), err, TAG, "reset w5500 failed");
@@ -717,8 +723,10 @@ static esp_err_t emac_w5500_init(esp_eth_mac_t *mac)
     ESP_GOTO_ON_ERROR(w5500_setup_default(emac), err, TAG, "w5500 default setup failed");
     return ESP_OK;
 err:
-    gpio_isr_handler_remove(emac->int_gpio_num);
-    gpio_reset_pin(emac->int_gpio_num);
+    if (emac->int_gpio_num >= 0) {
+        gpio_isr_handler_remove(emac->int_gpio_num);
+        gpio_reset_pin(emac->int_gpio_num);
+    }
     eth->on_state_changed(eth, ETH_STATE_DEINIT, NULL);
     return ret;
 }
@@ -728,8 +736,10 @@ static esp_err_t emac_w5500_deinit(esp_eth_mac_t *mac)
     emac_w5500_t *emac = __containerof(mac, emac_w5500_t, parent);
     esp_eth_mediator_t *eth = emac->eth;
     mac->stop(mac);
-    gpio_isr_handler_remove(emac->int_gpio_num);
-    gpio_reset_pin(emac->int_gpio_num);
+    if (emac->int_gpio_num >= 0) {
+        gpio_isr_handler_remove(emac->int_gpio_num);
+        gpio_reset_pin(emac->int_gpio_num);
+    }
     eth->on_state_changed(eth, ETH_STATE_DEINIT, NULL);
     return ESP_OK;
 }
@@ -752,8 +762,11 @@ esp_eth_mac_t *esp_eth_mac_new_w5500(const eth_w5500_config_t *w5500_config, con
     ESP_GOTO_ON_FALSE(w5500_config && mac_config, NULL, err, TAG, "invalid argument");
     emac = calloc(1, sizeof(emac_w5500_t));
     ESP_GOTO_ON_FALSE(emac, NULL, err, TAG, "no mem for MAC instance");
-    /* w5500 driver is interrupt driven */
-    ESP_GOTO_ON_FALSE(w5500_config->int_gpio_num >= 0, NULL, err, TAG, "invalid interrupt gpio number");
+
+    if (w5500_config->int_gpio_num >= 0) {
+        /* w5500 driver is interrupt driven */
+        ESP_GOTO_ON_FALSE(w5500_config->int_gpio_num >= 0, NULL, err, TAG, "invalid interrupt gpio number");
+    }
     /* SPI device init */
     spi_device_interface_config_t spi_devcfg;
     memcpy(&spi_devcfg, w5500_config->spi_devcfg, sizeof(spi_device_interface_config_t));

@KaeLL
Copy link
Contributor

KaeLL commented Nov 28, 2023

Conditional macros fit best rather than if statements.

@br101
Copy link
Author

br101 commented Nov 29, 2023

OK, see #12692

@kapyaar
Copy link

kapyaar commented Jan 7, 2024

@br101 Thanks, I was also looking for this :) Hope this gets merged soon, and be availble on the arduino side.

@kostaond
Copy link
Collaborator

@br101 thank you for the feature request and for suggestion how it could be implemented. The main idea is good and justification is reasonable. However, the proposed PR solution has following limitations:

  • It is compile time options => it's less usable for Arduio users. They usually use pre-compiled version of ESP-IDF.
  • Polling period is not configurable.
  • Polling period cannot be less than one tick (10 ms by default) when using vTaskDelay.
  • The solution is proposed only for W5500.

Therefore I would like to propose a little bit different approach using ESP timer to cover all above mentioned limitations. I've already prepared a demo to be discussed internally with my colleagues. I can share with you if you are interested....

@KaeLL
Copy link
Contributor

KaeLL commented Jan 29, 2024

I can share with you if you are interested

I'm interested.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Resolution: NA Issue resolution is unavailable Status: Done Issue is done internally Type: Feature Request Feature request for IDF
Projects
None yet
Development

No branches or pull requests

5 participants