Skip to content

Conversation

@benediktibk
Copy link
Contributor

This will fix #77983, but I'm still uncertain if we should actually merge it. Because at least for the hardware which I have available deinit is not yet implemented, so I wasn't even able to test it.

With this PR we will basically change the situation for some devices from the state "you can have only one ICE40" to "the ICE40 is not supported". But, at least in such cases there is now a clear path forward: Implement deinit for the SPI peripheral on this device. Considering this, as well as that this bitbang version of the driver won't be in use for most devices anyway, I think it is better to move along this path.

@benediktibk benediktibk added the DNM This PR should not be merged (Do Not Merge) label Mar 31, 2025
@benediktibk
Copy link
Contributor Author

Based upon #87740, hence the DNM.

@benediktibk benediktibk force-pushed the fix/multiple_ice40_bitbang branch 2 times, most recently from 8722dae to c7202ef Compare March 31, 2025 12:02
@benediktibk benediktibk removed the DNM This PR should not be merged (Do Not Merge) label Mar 31, 2025
@benediktibk benediktibk force-pushed the fix/multiple_ice40_bitbang branch from c7202ef to f876e7e Compare March 31, 2025 12:52
@benediktibk benediktibk marked this pull request as ready for review March 31, 2025 12:52
@github-actions github-actions bot added the area: FPGA Field-Programmable Gate Array (FPGA) label Mar 31, 2025
@cfriedt
Copy link
Member

cfriedt commented Mar 31, 2025

Looks good.

I might take a crack at implementing device deinit for esp32-c3 SPI (if it isn't already done) prior to this being merged, just because this driver was initially added for that platform and it would be good to ensure there are no regressions with it.

@benediktibk
Copy link
Contributor Author

The CI error seems to be unrelated to these changes:

sample.basic.blinky on frdm_k64f/mk64f12 error (Not in testsuite platform allow list but is one of the integration platforms)

Copy link
Member

@cfriedt cfriedt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@benediktibk - I don't want to block progress here and haven't had the time I would have hoped to keep going with #87995, so preemptive approval. If you rebase and things magically work again, I'll approve as well.

Replace reapplying the original pin configuration via pinctrl
in the ICE40 bitbang driver with a device_deinit/device_init.
Fixes zephyrproject-rtos#77983.

Signed-off-by: Benedikt Schmidt <[email protected]>
@benediktibk benediktibk force-pushed the fix/multiple_ice40_bitbang branch from f876e7e to c2da7f3 Compare April 23, 2025 05:27
@benediktibk
Copy link
Contributor Author

@benediktibk - I don't want to block progress here and haven't had the time I would have hoped to keep going with #87995, so preemptive approval. If you rebase and things magically work again, I'll approve as well.

Thanks, rebase onto main did the trick.

@kartben kartben merged commit 21c1a9e into zephyrproject-rtos:main Apr 24, 2025
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: FPGA Field-Programmable Gate Array (FPGA)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Multiple ICE40 instances on the same SPI master

4 participants