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

Why was _nodma used in the SWCAN implementation, and is it still necessary? #1101

Open
Donovan22 opened this issue Jan 20, 2025 · 2 comments

Comments

@Donovan22
Copy link

OVMS won't build when _nodma is included in components/swcan/swcan.cpp, components/swcan/swcan.h, and main/ovms_peripheral.cpp. The error says "undefined type spi_nodma_host_device_t". The only references I can find to spi having dma disabled on an esp32 is an obscure project from 2018.

To resolve this, I removed all _nodma references and switched to the standard SPI driver. This fixed the compilation issue but then there was a related runtime issue with chip select (CS) pins. (when SWCAN (can4) was enabled, the system ran out of available CS pins because can3 was also being initialized)

To address this:
I modified ovms_peripherals.cpp to conditionally initialize can3 only when SWCAN is disabled.
This freed up resources, allowing can4 to initialize correctly when SWCAN was enabled.

Questions:
What was the original purpose of _nodma in this context? Was it intended for compatibility or performance reasons?
Could removing _nodma cause any regressions or unexpected behavior, especially in high-speed or resource-intensive scenarios?
Is there a better solution to manage the CS pin limitation, or does this approach of conditionally initializing can3 seem appropriate?

@dexterbg
Copy link
Member

The "nodma" driver fork was picked for the initial implementation, AFAIK because the IDF driver was limited to only three devices, and the OVMS was meant & designed to support more. @markwj can probably tell you more about the background.

I suggest reading the comments & posts here: e53a626

That was the commit switching back to the IDF (DMA based) SPI driver, with some explanation by @markwj on the developer list. To summarize: the nodma driver did not offer the performance needed for high speed CAN operations, so we needed to go back to the DMA driver.

The SWCAN extension was added before, but no longer maintained, so support got lost in that transition. AFAIK @kssmll (https://github.com/kssmll) tried to pick up on it, but never submitted a PR for his/her changes. Maybe you can join efforts to build a new working integration.

If you can find a way to support SWCAN without disabling can3, SWCAN support could be included by default in the firmware builds. If can3 needs to be disabled, maybe you still can find a way to make this configurable or auto-detect the extension.

Regards,
Michael

@Donovan22
Copy link
Author

Thanks for the detailed explanation. It’s helpful to understand the context that led to the transition back to the IDF (DMA-based) SPI driver.

My changes are based on ksmll's work, but they co-opted can3 for swcan, which can't work for every vehicle (as you made me aware of in my last pull request).

When the board is powered on and starts setting up can3 and can4, it fails to 'assert' the CS (Chip Select) pin:
spi_master: spi_bus_add_device(442): no free cs pins for host assertion

I am currently investigating options to:
Auto-detect the SWCAN board: This would allow the system to dynamically enable or disable can3 based on the presence of the SWCAN module.
Optimize CS Pin Usage: Exploring ways to use software CS for can3/4 or other devices where feasible, freeing up hardware CS pins for SWCAN.

Next Steps
If you or anyone else has insights into previous efforts on SWCAN or ideas for managing CS pins, I’d be happy to collaborate.
Let me know if there are any other suggestions or considerations I should keep in mind while working on this.

Best regards,
Donovan

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants