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

Incorrect CS hold time in ENC28J60 example using ESP32S2 (IDFGH-6077) #7758

Closed
NorbertoNorbs opened this issue Oct 23, 2021 · 8 comments
Closed
Labels
Resolution: Done Issue is done internally Status: Done Issue is done internally

Comments

@NorbertoNorbs
Copy link

Environment

  • Development Kit: ESP32-S2-Saola-1
  • Kit version (for WroverKit/PicoKit/DevKitC): N/A
  • Module or chip used: ESP32S2-WROVER
  • IDF version : v4.4-dev-2875-g5f38b766a8
  • Build System: CMake
  • Compiler version: xtensa-esp32s2-elf-gcc.exe (crosstool-NG esp-2021r1) 8.4.0
  • Operating System: Windows
  • (Windows only) environment type: ESP Command Prompt
  • Using an IDE?: Yes Eclipse with IDF Plugin
  • Power Supply: external 5V

Problem Description

When using the ENC28J60 example the CS hold time is not enough and causes failure reading/write phy registers.
This is issue is related to #7156. The fix provided don't seems to work for ESP32S2 (I don't have a ESP32 to test).

I'm using the following config:

SPI Host: 2
SCLK GPIO: 12
MOSI GPIO: 11
MISO GPIO: 13
CS GPIO: 10
Clock: 8Mhz
Interrupt GPIO: 14

Expected Behavior

The value for cs_ena_posttrans calculated by the function enc28j60_cal_spi_cs_hold_time for 8Mhz is 2 but the hardware seems not to hold the CS for 2 cycles (I would expect 2 cycles to be 250ns) but only 1.

Actual Behavior

A value of 1 in cs_ena_posttrans generates a CS hold of ~13ns, 2 generates ~134ns and 3 ~263ns

Attached is the scope capture of the communication (showing clock and CS only).

cs_ena_posttrans = 2:
cs_ena_posttrans_with_2

cs_ena_posttrans = 3:
cs_ena_posttrans_with_3

Possible fix

Since cs_ena_posttrans seems to start with 2 I changed enc28j60_cal_spi_cs_hold_time to account for that:

static inline uint8_t enc28j60_cal_spi_cs_hold_time(int clock_speed_mhz)
{
    if (clock_speed_mhz <= 0 || clock_speed_mhz > 20) {
        return 0;
    }
    int temp = clock_speed_mhz * CS_HOLD_TIME_MIN_NS;
    uint8_t cs_posttrans = temp / 1000;
    if (temp % 1000) {
        cs_posttrans += 1;
    }

    return cs_posttrans+1;  //<< 
}

Debug Logs

I (288) gpio: GPIO[14]| InputEn: 0| OutputEn: 0| OpenDrain: 0| Pullup: 1| Pulldown: 0| Intr:0
I (298) enc28j60: revision: 6
E (308) enc28j60: enc28j60_init(307): wrong chip ID
E (308) esp_eth: esp_eth_driver_install(222): init phy failed
ESP_ERROR_CHECK failed: esp_err_t 0xffffffff (ESP_FAIL) at 0x40027644

@espressif-bot espressif-bot added the Status: Opened Issue is new label Oct 23, 2021
@github-actions github-actions bot changed the title Incorrect CS hold time in ENC28J60 example using ESP32S2 Incorrect CS hold time in ENC28J60 example using ESP32S2 (IDFGH-6077) Oct 23, 2021
@kostaond
Copy link
Collaborator

Hi @NorbertoNorbs, thank you for reporting the issue. We will need to investigate it internally with SPI team, since it seems it performs as expected with ESP-WROVER-KIT V4.1 (ESP32-WROVER-E module).

SPI Clock: 8 MHz
pic_35_2

@kostaond
Copy link
Collaborator

@NorbertoNorbs, we can confirm that we observe the same SPI behavior as you reported on EPS32-S2. Thank you for pointing this issue out and for very well written issue report!

@NorbertoNorbs
Copy link
Author

Hello @kostaond, thanks for your reply, after submitting the issue I thought it could be a diference between the chips, glad you guys figured it out.

Thanks again.

@espressif-bot espressif-bot added Status: In Progress Work is in progress and removed Status: Opened Issue is new labels Nov 15, 2021
@kostaond
Copy link
Collaborator

@NorbertoNorbs just FYI, the SPI driver fix is currently in internal review...

@NorbertoNorbs
Copy link
Author

@kostaond Thanks for the update, when ready I will test again with the same setup.

@espressif-bot espressif-bot added Resolution: NA Issue resolution is unavailable Status: Done Issue is done internally Resolution: Done Issue is done internally and removed Status: In Progress Work is in progress Resolution: NA Issue resolution is unavailable labels Jan 7, 2022
@Alvin1Zhang
Copy link
Collaborator

Thanks for reporting, the fix on master is d500c82, we are back porting the fix until release/4.2, will update further when the fixes sync to GitHub.

@AxelLin
Copy link
Contributor

AxelLin commented Feb 17, 2022

v4.4: 1140036
v4.3: b3c51e7
v4.2: ca6636c
This can be closed.

@Alvin1Zhang
Copy link
Collaborator

Thanks for reporting and sharing the updates, feel free to reopen if the issue still happens, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Resolution: Done Issue is done internally Status: Done Issue is done internally
Projects
None yet
Development

No branches or pull requests

5 participants