pkg/driver_atwinc15x0: remove the compilation of the SPI Flash driver part #21494
Conversation
The SPI flash integrated in the ATWINC15x0 chip is not used in RIOT. It is therefore not necessary to use the SPI flash driver of the package.
The only function of the SPI flash driver part that is called by other parts of the package is `spi_enable_flash(0)` to disable the SPI flash during driver deinitialisation for power consumption reasons. Since we are not deinitialising the driver, the function is not called and can be commented out to compile the package without the SPI flash driver part.
c804868 to
5668a96
Compare
drivers/atwinc15x0/atwinc15x0_bus.c
Outdated
| @@ -40,8 +40,10 @@ sint8 nm_bus_init(void *arg) | |||
| assert(atwinc15x0); | |||
| assert(gpio_is_valid(atwinc15x0->params.ssn_pin)); | |||
|
|
|||
| #if !defined(CPU_ESP32) && !defined(CPU_ESP8266) | |||
There was a problem hiding this comment.
Why do these need a special case?
There was a problem hiding this comment.
🤔 For the ESP32x, the CS pin is implicitly initialised as an SPI pin in spi_acquire, so it is not necessary to initialise it explicitly as GPIO_OUT. If the CS pin also corresponds to the pin defined as CS0 pin, which is the case here, it is already initialized as CS pin during auto-initialization, i.e. it is already marked as SPI pin. If the pin is marked as an SPI pin, the GPIO peripheral driver will complain if the application tries to initialize the pin as something else to prevent attempts to use a GPIO for multiple purposes at the same time.
To avoid this special case, we would have to remove this already used as something else check from the ESP32x GPIO peripheral driver.
Lines 181 to 192 in 41c4f6f
Line 198 in 41c4f6f
There was a problem hiding this comment.
But won't this be a problem for every driver that expects a software CS pin?
On all other platforms we don't user hardware CS since that sometimes has a weird behavior (deasserts after every byte) so I'd be surprised if ESP32 is special here.
There was a problem hiding this comment.
Ok, we could just remove the check _gpio_init_mode_check from gpio_init. Then it should be possible to reinitialize a GPIO. However, the check for accidental multiple use would then no longer work.
There was a problem hiding this comment.
I'd rather remove the initialization of the CS pin from the cpu, all drivers that make use of a CS pin will initialize it on their own.
There was a problem hiding this comment.
all drivers that make use of a CS pin will initialize it on their own.
Yes, but they use spi_init_cs instead of gpio_init 🤔 So the problem is that the driver doesn't use the right function to initialize the CS pin.
There was a problem hiding this comment.
I replaced gpio_init by spi_init_cs, it seems to work.
|
@benpicco Thanks for reviewing and merging. |
Contribution description
The PR removes the compilation of the SPI Flash driver part of the package. This allows also to use the ATWINC15x0 for ESPs.
The SPI flash integrated in the ATWINC15x0 chip is not used in RIOT. It is therefore not necessary to compile the SPI flash driver part of the package. The only function of this part that is called by other parts of the driver package is
spi_enable_flash(0)to disable the SPI flash during the driver deinitialization for power consumption reasons. Since we are not deinitializing the driver, the function is not called and can be commented out to be able to compile the package without the SPI flash driver part.Testing procedure
A networking application should still work for any board with a ATWINC15x0 WiFi module that uses the ATWINC15x0 driver package, for example:
The PR has been tested with
feather-m0-wifiarduino-mkr1000esp32-wemos-d1-r32with a Adafruit WINC1500 WiFi Shield for ArduinoIssues/PRs references