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.

Sign up for GitHub

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

Jump to bottom

Add support for CAN to SiLabs targets #10468

Merged

adbridge merged 3 commits into ARMmbed:master from petroborys:master

May 7, 2019

Contributor

petroborys commented Apr 24, 2019

Description

Added support for CAN to SiLabs targets

Pull request type

[ ] Fix
[ ] Refactor
[x] Target update
[ ] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change

Release Notes

Tested on the EFM32GG11_STK3701

ciarmcom requested a review from a team

April 25, 2019 11:07

Member

ciarmcom commented Apr 25, 2019

@petroborys, thank you for your changes.
@ARMmbed/mbed-os-maintainers please review.

ciarmcom added the needs: review label

stevew817 suggested changes

View reviewed changes

Contributor

stevew817 left a comment

A minor change request for getting peripheral usage in line & forwards compatible with products that potentially only have one CAN interface. Otherwise, looking good!

I'll open a PR for updating emlib to latest, since the rest of emlib now looks pretty out-of-date.

targets/TARGET_Silicon_Labs/TARGET_EFM32/PeripheralNames.h

@@ @@ -136,6 +136,13 @@ typedef enum { @@
               } UARTName;
               #endif
+              #if DEVICE_CAN
+              typedef enum {
+                  CAN_0 = (int)CAN0_BASE,

Contributor

stevew817 Apr 25, 2019

Please guard each enum entry with the respective base address define, in line with the other definitions in this file

targets/TARGET_Silicon_Labs/TARGET_EFM32/can_api.c

+              	}
+              }
+              void CAN0_IRQHandler(void)

Contributor

stevew817 Apr 25, 2019

Please wrap in respective peripheral base definition, as this shouldn't be there if there is no (for example) CAN1.

targets/TARGET_Silicon_Labs/TARGET_EFM32/can_api.c

+                  }
+              	switch ((CANName)obj->instance) {
+                      case CAN_0:

Contributor

stevew817 Apr 25, 2019

Usage of enum values should be wrapped and compiled in/out based on whether the base peripheral is present or not.

targets/TARGET_Silicon_Labs/TARGET_EFM32/can_api.c

+              	CAN_MessageIntClear(obj->instance, 0xFFFFFFFF);
+                  switch ((CANName)obj->instance) {
+                      case CAN_0:

Contributor

stevew817 Apr 25, 2019

Same here, please wrap in presence of the base peripheral

targets/TARGET_Silicon_Labs/TARGET_EFM32/can_api.c

+              	int index = 0;
+                  switch ((CANName)obj->instance) {
+                      case CAN_0:

Contributor

stevew817 Apr 25, 2019

Same here, please wrap in presence of base peripheral

targets/TARGET_Silicon_Labs/TARGET_EFM32/can_api.c

+                  CMU_Clock_TypeDef cmuClock_number;
+                  switch ((CANName)obj->instance) {
+                      case CAN_0:

Contributor

stevew817 Apr 25, 2019

Should be wrapped based on presence of base peripheral

Contributor Author

petroborys commented Apr 26, 2019

Thank for your review. I have made all the changes.
In EmLib 5.3.3. do not read message ID (in CAN_ReadMessage).

stevew817 approved these changes

View reviewed changes

0xc0170 approved these changes

View reviewed changes

0xc0170 added needs: CI and removed needs: review labels

Contributor

adbridge commented Apr 29, 2019

CI started

mbed-ci commented Apr 29, 2019

Test run: FAILED

Summary: 2 of 7 test jobs failed
Build number : 1
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_build-IAR
jenkins-ci/mbed-os-ci_mbed2-build-IAR

0xc0170 added needs: work and removed needs: CI labels

Contributor

0xc0170 commented Apr 30, 2019

Please review build failures (build artifacts above)

Contributor Author

petroborys commented Apr 30, 2019

Please review build failures (build artifacts above)

Thank. Done.

0xc0170 added needs: CI and removed needs: work labels

Contributor

0xc0170 commented Apr 30, 2019

CI started

Contributor

0xc0170 commented Apr 30, 2019 •

edited

Loading

I've noticed the commit messages (1-10) - what do the numbers mean? It would make sense to provide more meaningful commit messages instead.

Either to squash them or rewrite.

Contributor Author

petroborys commented Apr 30, 2019 •

edited

Loading

I've noticed the commit messages (1-10) - what do the numbers mean?

Just numeration.

It would make sense to provide more meaningful commit messages instead.

Ok, sure

mbed-ci commented Apr 30, 2019

Test run: FAILED

Summary: 1 of 11 test jobs failed
Build number : 2
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_exporter

petroborys and others added 3 commits

May 1, 2019 00:12


          Add can_api.c for efm32

0a82591


          Add can_api.c for efm32: wrap in presence of the base peripheral

f29dffa


          Add can_api.c for efm32:review build-IAR

b635c02

petroborys force-pushed the master branch from bc133a9 to b635c02 Compare

May 1, 2019 00:13

Contributor Author

petroborys commented May 1, 2019

Either to squash them or rewrite.

Done.

Contributor Author

petroborys commented May 1, 2019

I didn't see any error in the logs.

Contributor

0xc0170 commented May 2, 2019

Will restart tests later today

0xc0170 changed the title ~~Added support for CAN to SiLabs targets~~ Add support for CAN to SiLabs targets

Contributor

0xc0170 commented May 3, 2019

CI started

mbed-ci commented May 3, 2019

Test run: SUCCESS

Summary: 11 of 11 test jobs passed
Build number : 3
Build artifacts

0xc0170 added ready for merge release-version: 5.12.4 and removed needs: CI labels

adbridge merged commit bab78c6 into ARMmbed:master

adbridge removed the ready for merge label

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release-version: 5.12.4