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

Adding support for the STM32G0 #123

Open
ryedwards opened this issue Oct 29, 2022 · 17 comments
Open

Adding support for the STM32G0 #123

ryedwards opened this issue Oct 29, 2022 · 17 comments

Comments

@ryedwards
Copy link
Contributor

Hey everyone. I already have written a codebase for the STM32G0 chip that supports gs_usb and shares most of the USB code with candleLight. I've used additional features in my code such as FreeRTOS and other HAL items that make it impossible to fully merge back into candleLight.

I've reviewed what would need to be done and found that the only major issue is the can handler. Since the FDCAN HAL is significantly different than the bxCAN on the F0 and F4 it doesn't make much sense to basically have an entire can.c/can.h that is split into two with a #define.

What are the thoughts of creating a canfd.c and canfd.h? Since most of the chips that support FDCAN (G0, G4, H7, etc) all use the same HAL it will be portable to those. I'm wondering what the code paradigm is with having split functionality in a single code base.

I am currently working on getting the code working in a fork with the canfd.x design.

@fenugrec
Copy link
Collaborator

it doesn't make much sense to basically have an entire can.c/can.h that is split into two with a #define.

Agreed

What are the thoughts of creating a canfd.c and canfd.h?

Should be OK - we're already doing some amount of per-device-target config in CMakeLists. But why do you need a separate canfd.h , is it entirely different from the "API" in can.h ? I haven't looked at your code, just wanting to keep things as simple as possible.

Of course our codebase will need to be reworked and reorganized as we go forward - it has grown from supporting just one type of F0 board, to F042+F072, multiple boards, then F407, now this G0 port, and soon possibly CAN-FD (pending linklayer/gs_usb_fd#2 and https://lore.kernel.org/all/[email protected]/ )

(btw, did you implement CANFD or just basic CAN ?)

@normaldotcom
Copy link

If you're interested, I have a (fairly out of date) branch that implements multitarget support for cmake, pulling in the correct cmsis, HAL, linker file, hal config, startup file, etc and passes correct compiler options for each core type. It ended up getting a bit messy but it does work! I have implemented FDCAN peripheral support on the G4 processor (for CANable 2.0), but it just does standard CAN TX/RX right now until kernel stuff gets pushed along a bit further.

https://github.com/normaldotcom/candleLight_fw/tree/multitarget

@ryedwards
Copy link
Contributor Author

ryedwards commented Oct 29, 2022

So the one I wrote has full implementation of CANFD per the latest gs_usb module.

I have the candleLight code compiling with the candfd.h structure but do see a way to get it working in just can.h/.c (the file will grow considerably but it may end up being a cleaner implementation).

Here is a link to my current G0 code:
https://github.com/ryedwards/BUDGETCANFD_G0

My code sticks with the CubeMX codebase tightly (all custom code is in the USER blocks) so it can be regenerated in CubeMX. I'll need to check but it may be easier to revert my code to support the F0/F4 vs. forward porting the candleLight codebase. You would be able to use the F0 CAN HAL going this route vs. manual register manipulation. It is also coded to support the STM32 IDE right out of the box. The missing part is the CMake building utilities.

@fenugrec
Copy link
Collaborator

fenugrec commented Oct 29, 2022

I have the candleLight code compiling with the candfd.h structure but do see a way to get it working in just can.h/.c (the file will grow considerably but it may end up being a cleaner implementation).

I'm perfectly fine with compile-time selecting the proper .c file depending on target (in fact some F4 code is almost annoying enough to justify that already), I was more thinking of having (if possible) a common .h file, if it's reasonable as an abstraction layer. I'm not sure if it's still realistic as we add FD support, I haven't looked closely.

My code sticks with the CubeMX codebase tightly

Understandable, but I don't think it's a direction we want to take for this project. I've had issues collaborating on cube projects where stuff starts breaking when you have the slightest version mismatch of cubemx or libraries, etc.

You would be able to use the F0 CAN HAL going this route vs. manual register manipulation.

In my experience HAL is fairly ROM-hungry. We already use it in other parts (GPIO, RCC), that's perfectly fine, but I would need compelling arguments to switch existing parts to HAL. Some targets with 16k flash are nearly full already.

@ryedwards
Copy link
Contributor Author

Perfect. I'm about complete with the porting using the candleLight codebase. I should be done this weekend and will upload to my fork in git.

@ryedwards
Copy link
Contributor Author

So I put it all together and gave it some light testing. There is FDCAN support is baked in (using the new structs within the latest gs_usb module) but I still need to update the bit timing routines. I did end up making all of the changes within the can.c file. The error parsing function still needs work. I was using the error IRQ callback in my code so I need to think of a better way.

https://github.com/ryedwards/candleLight_fw

@fenugrec
Copy link
Collaborator

Ok. Let me know when it's "ready enough" for a code review / PR

Oh hey @normaldotcom , I completely missed your reply up there. Seems like that branch diverged quite a bit, but I might steal some cmake bits eventually.

@ryedwards
Copy link
Contributor Author

I've pushed my latest changes and I feel it's ready for code review. I made a few minor but fundamental changes that I think made sense as I was adding functionality. The main one was to move the CAN_INTERFACE and CAN_CLOCK_SPEED into the config.h as the CAN clock is more HW design dependent than it is chipset based. Also, INTERFACE varies based on how many CHANNELS you decide to implement.

Also, per your above, (@marckleinebudde correct me if I'm wrong) but fdcan was first available in the 5.19 mainline kernel. This is now available on the latest Ubuntu release (22.10). I've been testing my source against Marc's code changes for a few months and it is working. I've also integrated the latest gs_usb.h. This code fully supports CANFD. I've done a little bit of stress testing but nothing formal or with any real vehicle data. If anyone wants to play around with the G0 based HW I have a few extra I could ship out for testing.

I also reviewed @normaldotcom cmake goodies. I think many of those would make sense given the number of flavors of STM32s folks are using since we design around what we can find in the marketplace at the moment.

@marckleinebudde
Copy link
Contributor

marckleinebudde commented Oct 31, 2022

@ryedwards, please separate your changes by topic:

  • update .gitignote
  • move stuff to config.h
  • update struct gs_host_frame
  • add CAN-FD
  • add stm32g0 hal
  • add infrastructure for new µC to Cmake
  • add new board

otherwise review is a PITA. 😸

Don't remove the USER_ID flash support, there's a dedicated PR #102 for that.

STM32G4 support would be a new PR, after we've added STM32G0 support.

@fenugrec
Copy link
Collaborator

and at some point, run uncrustify with our .cfg P )

@ryedwards
Copy link
Contributor Author

@ryedwards, please separate your changes by topic:

  • update .gitignote
  • move stuff to config.h
  • update struct gs_host_frame
  • add CAN-FD
  • add stm32g0 hal
  • add infrastructure for new µC to Cmake
  • add new board

otherwise review is a PITA. 😸

Don't remove the USER_ID flash support, there's a dedicated PR #102 for that.

STM32G4 support would be a new PR, after we've added STM32G0 support.

Creating so much extra work for me! :)
I'll have to rethink verifying drivers for you in the future.

No problem., though That does make sense.

@ryedwards
Copy link
Contributor Author

@fenugrec

In my experience HAL is fairly ROM-hungry. We already use it in other parts (GPIO, RCC), that's perfectly fine, but I would need compelling arguments to switch existing parts to HAL. Some targets with 16k flash are nearly full already.

I was having issues with the code playing nice with the non-HAL F0/F4 code and the G0 CAN code (The can handle pointer was making for some messy code). I ported over the F0/F4 code to the HAL and checked the ROM usage. It went from 16020 to 16312. So an additional ~300 bytes. Not trivial but an improvement in cleanliness and clarity. RAM usage is unchanged.

@ryedwards
Copy link
Contributor Author

So PR #126 is all ready for review. There's a bunch of comments in the PR and on the individual commits.

@akohlsmith
Copy link

@ryedwards I checked out your repo, switched to the implement-CANFD branch and can build, but it doesn't look like any of the targets are using the G0 HAL, and none of the target .elf files even mention FDCAN in a basic strings search. I don't see any (obvious) changes in the 14 commits in that branch which create a G0 target...

How do I build for canable 2.0 to play around with what you've done?

@marckleinebudde
Copy link
Contributor

Hey @akohlsmith,

please try the multichannel branch in my repo: https://github.com/marckleinebudde/candleLight_fw/tree/multichannel. There's wokring support for STM32G0B1.

@akohlsmith
Copy link

Thanks @marckleinebudde ; is it the budgetcan_fw target I want to build for canable2.0?

semi-but-not-releated: is there a fork of slcand which supports FD? The official stance is that slcand will not support FD since gs_can can do it all by itself.

@marckleinebudde
Copy link
Contributor

In this image I see a STM32G431 not a STM32G0B1. I think you need to add the HAL, USB drivers, liker file, etc... for STM32G4 first. Then you can add your board.

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

5 participants