Migrate KSZ8863 driver to new I2C driver implementation #46
Conversation
bogdankolendovskyy
force-pushed
the
ksz8863_new_i2c_dev
branch
from
7af04fb to
c0f1edf
Compare
|
Please double check if CMakelists needs to be updated too https://docs.espressif.com/projects/esp-idf/en/latest/esp32/api-reference/peripherals/i2c.html?highlight=i2c_master_transmit#api-reference |
bogdankolendovskyy
force-pushed
the
ksz8863_new_i2c_dev
branch
from
c0f1edf to
6d7e870
Compare
bogdankolendovskyy
force-pushed
the
ksz8863_new_i2c_dev
branch
from
6d7e870 to
9ec466d
Compare
bogdankolendovskyy
force-pushed
the
ksz8863_new_i2c_dev
branch
2 times, most recently
from
54d4049 to
947cfb7
Compare
ksz8863/src/ksz8863_ctrl.c
Outdated
| memcpy(reg_addr_and_data + 1, data, len); | ||
| // When performing a soft reset, the KSZ8863 doesn't produce an ACK. Print a warning that the error is expected and ignore it. | ||
| if unlikely(reg_addr == KSZ8863_RESET_ADDR) { | ||
| ESP_LOGW(TAG, "Ignore the error below. KSZ8863 does not produce ACK when performing soft reset."); |
There was a problem hiding this comment.
It needs to be specifically said it's I2C error to ignore.
There was a problem hiding this comment.
Rephrased. Also added clarification of why we won't support SMI access to the README.
|
@bogdankolendovskyy don't forget to update |
bogdankolendovskyy
force-pushed
the
ksz8863_new_i2c_dev
branch
2 times, most recently
from
7056470 to
dd0d710
Compare
| .master.clk_speed = CONFIG_EXAMPLE_I2C_CLOCK_KHZ * 1000, | ||
| .sda_io_num = CONFIG_EXAMPLE_I2C_SDA_GPIO, | ||
| .glitch_ignore_cnt = 7, | ||
| .flags.enable_internal_pullup = true |
There was a problem hiding this comment.
No need to be set to true in my opinion. It's common practice the I2C bus to be always designed with external pullups.
There was a problem hiding this comment.
Please fix in all applicable examples.
| memcpy(reg_addr_and_data + 1, data, len); | ||
| // When performing a soft reset, the KSZ8863 doesn't produce an ACK. Print a warning that the error is expected and ignore it. | ||
| if unlikely(reg_addr == KSZ8863_RESET_ADDR) { | ||
| ESP_LOGW(TAG, "Ignore the error below. It is produced by the I2C driver because KSZ8863 does not produce ACK when performing soft reset. It is epxected behaviour and requires no actions on your side."); |
There was a problem hiding this comment.
Please reformulate in something like "The following I2C error can be ignored ..." or something similar. Problematic part is "Ignore the error below" because logs and printfs in general are not deterministic. There might be context switch and different error could be printed before printing the I2C error.
| break; | ||
| case KSZ8863_SPI_MODE: | ||
| spi_bus_remove_device(s_ksz8863_ctrl_intf->spi_bus_spec.spi_handle); | ||
| vSemaphoreDelete(s_ksz8863_ctrl_intf->bus_lock); | ||
| spi_bus_remove_device(s_ksz8863_ctrl_intf->spi_handle); | ||
| case KSZ8863_SMI_MODE: |
There was a problem hiding this comment.
Please remove all the case KSZ8863_SMI_MODE: cases.
Description
Migrate KSZ8863 driver to use
i2c_master.hinstead of legacyi2c.h. Examples updated accrodingly, and ioctl errorsuppressed. It resulted from
ETHERNET_EVENT_CONNECTEDhandler trying to use KSZ8863 custom ioctl commandKSZ8863_ETH_CMD_G_PORT_NUMwhenhost_eth_handleproducedETHERNET_EVENT_CONNECTED.Related
Fixes #45
Testing
I2C bus probed to verify correct data transfer, examples are confirmed to be running without issues.
Checklist
Before submitting a Pull Request, please ensure the following: