espressif / esp-hosted

Hosted Solution (Linux/MCU) with ESP32 (Wi-Fi + BT + BLE)
Other
665 stars 151 forks source link

Add device tree support for ESP-hosted_ng #431

Open ForgetCSX opened 1 month ago

ForgetCSX commented 1 month ago

I see that the current IO configuration and hardware interface ID information of NG are solidified inside the driver using macro definitions, is it possible to use the device tree to obtain this information in a unified way, so that you only need to modify the interface information on the host side, and you don't need to modify the source code on the slave side, so that it will be more convenient to port.

mantriyogesh commented 1 month ago

Sure, input taken. We actually worked on ESP-Hosted-Fg, to do add these as module params.

I'm not saying it might be perfect, but there are multiple angles we tried to cover in this change:

  1. If users do not change the params, by any means, the kernel module insert should fail.
  2. This verification is done much late, when spi actually use those passed parameters. So in case user wish to initialise them, with other means, for example, KConfig, is should be really easy.
  3. Direct usage of device tree params, would be confusing to develop, as dts may have multiple instances of bus. What GPIO and what peripheral etc config is known to user.
  4. Current module params (FG recent addition)
    • resetpin : GPIO to use from Linux to reset the ESP on loading of driver (Resetpin was missing at you side, because of which when you manually reset the ESP, it provided you the init event in dmesg of host)
    • clockspeed : use custom clock freq for spi/sdio clock
    • spi_bus : spi bus instance
    • spi_cs : chipset select instance on that bus
    • spi_handshake : handshake gpio to be passed
    • spi_dataready : data ready gpio to be used
    • spi_mode: mode to be used
  5. Ways to configure:
    • module params direct
    • straight forward way, WYSIWYG
    • insmod esp32_spi.ko resetpin=<> spi_bus=<> spi_cs=<> spi_mode<> spi_handshake=<> spi_dataready=<>
    • obviously same way for modprobe
    • helper script: rpi_init.sh handles easy configuration and user control
    • hardcoding : although hardcoding is bad, might be real convenient for some users for evaluation. Earlier problem was the parameters were spread different places, hard for newbies to locate (although documentation clearly mentioned)
    • at start of main.c, now user has all config at once
  6. Other potential ways to provide config: Not all users have similar config and similar systems
    • They could create n use KConfig at kernel module level, which would be easy when module expected to be inside kernel, for example yocto users. Not sure if this method would be beneficial for buildroot environment. May be @Mr-Bossman can add some inputs here.
    • /etc/modprobe/.conf
    • echo "options esp32_spi spi_bus=0" >> /etc/modprobe.d/esp32_spi.conf
    • other params similar fashion
    • grub direct references (non usual way)
Mr-Bossman commented 1 month ago

we tried to cover in this change

Did you make a change to add this feature?

I was trying to make the pins and spi bus configurable via Devicetree but never finished.

I started rebaseing all the commits onto master here https://github.com/Mr-Bossman/esp-hosted/releases/tag/example I have not tested this btw.

Also yes they are using the same SOC. https://github.com/espressif/esp-hosted/issues/253#issuecomment-2226521260

mantriyogesh commented 1 month ago

Yes I changed it here : https://github.com/espressif/esp-hosted/commit/8824a3f3c3dfb27003811f6d2c97020d6d6ef2e3

I had seen your code, and is indeed a good change. Thanks! This indeed is a nice change.

Unfortunately, we can't test it, unless extra efforts, so would you be able to complete this change some time? For now can you use FG as base (with recent change)? We are yet to add these params in NG..

I think, we can give alternate configuration methods. For example, module params not available, fetch these params from DT.

In any way, the error checking that these params are not initialised is intentionally is kept late checked, so that by any means, the parameters are available, should use them.. In worst case the error would flag, but parameters mandatorily need to be configured by user, somewhere..

Do you agree @Mr-Bossman ?

mantriyogesh commented 1 month ago

It is on master FG, already, which I forgot to mention.

https://github.com/espressif/esp-hosted/commit/8824a3f3c3dfb27003811f6d2c97020d6d6ef2e3#diff-7183abc3a1c81c0900093cac7fde0f05e4cf3e566d19ccd574a340bc43b06bc3R820 Here I update the module params if passed. If the params are still not updated, we can retrieve from device tree. Try not fail of some param is not updated from device tree. Anyway if the patram is not updated by any means, we will fail at last stage, just before using them.

clockspeed is considered optional. If not initialised through any means, we keep it default to 10MHz. I am planning to auto change to its maximum value depending upon the bootup event from slave, where it exposes chip id of its own. But the risk here is that max clocks are 40MHz for some chipsets. But if users connections are lengthy, the 40Mhz would not be optimal. So yet thinking..

ForgetCSX commented 1 month ago

Recently saw a similar ESP wifi, in this repo, with a similar io initialization process https://github.com/Evlers/esp8089_wifi_drivers.git