Enable CAN peripheral on STM32G0 #34963

str4t0m · 2021-05-07T11:33:08Z

This PR enables CAN support for stm32g0.
The driver introduced in #31061 by @alexanderwachter does work for this series as well without issues.
All tests pass with this definitions on nucleo_g0b1re.

str4t0m · 2021-05-07T11:44:24Z

@raul-klg CANFD works on stm32g0 with this definitions.

raul-klg · 2021-05-07T12:17:30Z

Thanks, @str4t0m That's something very similar to what I had. Probably I'll review again according to your comments and news from https://github.com/alexanderwachter/zephyr/tree/can_mcan_stm
Regards,

alexanderwachter · 2021-06-05T15:16:56Z

dts/arm/st/g0/stm32g0b1.dtsi

+				#size-cells = <0>;
+				reg = <0x40006800 0x400>, <0x4000B800 0x350>;
+				reg-names = "m_can", "message_ram";
+				interrupts = <21 0>, <22 0>;


Does can1 and can2 share the IRQ line? If not, this is probably a copy/paste error

Thanks for the review.
Yes they share the interrupt:
Interrupt 21 is TIM16 and FDCAN_IT0 global interrupt and 22 is TIM17 and FDCAN_IT1 global interrupt

Oh, then you have to remove the can2 node or modify the can_stm32fd.c frontend implementation to support shared IRQ lines.

alexanderwachter · 2021-06-08T16:10:58Z

dts/arm/st/g0/stm32g0b1.dtsi

+				#size-cells = <0>;
+				reg = <0x40006800 0x400>, <0x4000B800 0x350>;
+				reg-names = "m_can", "message_ram";
+				interrupts = <21 0>, <22 0>;


Oh, then you have to remove the can2 node or modify the can_stm32fd.c frontend implementation to support shared IRQ lines.

str4t0m · 2021-06-11T09:58:52Z

I Will push code for interrupt sharing later, therefore I marked it as draft for now.

erwango · 2021-06-16T07:22:59Z

boards/arm/nucleo_g0b1re/nucleo_g0b1re.dts

+	shared-irqs = <&shared_irq21>, <&shared_irq22>;
+	shared-irq-names = "LINE_0", "LINE_1";


I wonder why these are defined at board level.
Is there some level of configuration that is left to the user?

My initial idea was that irq conflicts arise depending on which peripherals are used in the application.
In cases where only CAN1 is used there is no need to share the interrupt.
In cases where only CAN1 and TIM16 are used irq21 could be shared while irq22 is only used by CAN1 line_1.

However this has the downside that additional configuration by the "user" is needed.
Would you prefer if the driver would simply use shared-irqs all the time, or enable shared-irq support by another means?

ok, I get your point.

Actually I was not sure you considered the case of having different IRQ scheme depending on conflicts possibility (looking at the modifications done to config_can_##inst##_irq function.

I'd prefer to avoid having users configuring this. Device tree is already complex for new comers, and this would add one more significant step.

I see following options:

A] Keep shared IRQ used even if there is no conflict in user configuration
(+) Simple to handle at driver level
(+) Same code used with or w/o conflicting devices enabled. Which is better in general for analysis if real time issues
(-) Useless footprint/performance overhead
(-) Different real time behavior of potentially conflicting nodes and no conflict nodes

B] Detect conflicting configurations (using DT macros) and enable shared IRQ only when required
(+) Performance/Footprint optimization
(-) Increased complexity at driver level
(-) Different code used in both cases

C] User configurable B] (same as B], but with Kconfig option that allows to always use shared IRQ, even if no conflict is possible)
(+) User can choose between Performance/Footprint optimization and similar real time behavior
(-) Driver complexity ++

Thanks for this detailed listing of options.
I have now as a first step moved the lines highlighted above into the soc's dtsi.
There was really no good reason not to have them already defined. Now shared-irqs are used if the referenced shared-irq is enabled, meaning the user would still have to set status = "okay" for the relevant irq nodes.
Before I comment on above options, may I ask if

I'd prefer to avoid having users configuring this.

also refers to setting status = "okay" for the shared-irq nodes?

also refers to setting status = "okay" for the shared-irq nodes?

It depends on the solution chosen in the end, but if we can minimize the possibility of errors for end users, it would be great

alexanderwachter · 2021-06-16T07:52:35Z

@mbolivar can you check the IRQ device-tree API extension?

str4t0m · 2021-06-17T09:59:30Z

I am wondering if the Macros with the _COND suffix are worth it or this logic should be implemented by each driver using shared-irqs itself? Mostly because zephyr also has ZLI and #32575 for example provides the possibility to choose between "normal" and direct IRQs .

Would you prefer to open a separate PR for the commit "drivers: shared_irq: add devicetree helper macros", or should I rename this PR accordingly.

This commit adds helper macros to reduce boilerplate code in drivers that (conditionally) use the shared irq driver. Signed-off-by: Thomas Stranger <thomas.stranger@outlook.com>

This commit enables SHARED_IRQ config option per default if any shared_irq node is enabled. Signed-off-by: Thomas Stranger <thomas.stranger@outlook.com>

This commit adds shared_irq support to can_stm32fd driver, such that two can instances can be used on stm32g0. Additinally to the necessary code changes, bindings are added to st,stm32-fdacn.yaml, and the SHARED_IRQ is automatically enabled in KConfig if the can devices define a shared-irqs phandle-array. Signed-off-by: Thomas Stranger <thomas.stranger@outlook.com>

The shared-irq nodes are necessary to use the shared irq-driver This commit adds the necessary nodes for can1, can2, tim16, and tim17. Signed-off-by: Thomas Stranger <thomas.stranger@outlook.com>

In stm32g0 series only stm32g0b1 and stm32g0c1 socs support can. This commit adds the CAN nodes to the stm32g0b1 soc. Signed-off-by: Thomas Stranger <thomas.stranger@outlook.com>

This commit adds CAN support to the nucleo_g0b1re board. Signed-off-by: Thomas Stranger <thomas.stranger@outlook.com>

This commit should be removed once the pr is not in draft status anymore. It adds some additionl definitions which are not intended to be merged. Those definitions enable 2 can instances and the shared irq driver on the nucleo_g0b1re board. Signed-off-by: Thomas Stranger <thomas.stranger@outlook.com>

erwango · 2021-06-21T07:42:22Z

drivers/interrupt_controller/Kconfig.shared_irq

@@ -5,6 +5,7 @@

 menuconfig SHARED_IRQ
 	bool "Shared interrupt driver"
+	default $(dt_compat_enabled,shared-irq)


Since shared-irq can't be used alone, what about having this symbol selected by clients, when needed.

github-actions · 2021-08-21T00:28:36Z

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

dresco · 2022-11-21T17:15:15Z

Hi, just wondering if there are still plans for moving this forwards? I have an app running on F4 that I'd like to port to G0, and found this issue while searching for info on CAN support.

I also found the shared interrupts issue, so perhaps is waiting for that to be added first?

Thanks in advance for any feedback..

github-actions bot added area: Boards area: Devicetree area: Documentation platform: STM32 ST Micro STM32 labels May 7, 2021

str4t0m force-pushed the stm32g0_can branch from 52be83c to bc8d3ae Compare May 7, 2021 11:48

str4t0m marked this pull request as ready for review May 7, 2021 15:30

str4t0m requested review from ABOSTM, erwango, FRASTM and nashif as code owners May 7, 2021 15:30

str4t0m force-pushed the stm32g0_can branch from bc8d3ae to f43502d Compare May 8, 2021 07:50

str4t0m requested a review from alexanderwachter as a code owner May 8, 2021 07:50

github-actions bot added the area: CAN label May 8, 2021

str4t0m force-pushed the stm32g0_can branch from f43502d to e104f9e Compare May 8, 2021 13:35

erwango added this to the v2.7.0 milestone May 10, 2021

zephyrbot requested review from henrikbrixandersen, karstenkoenig, legoabram, martinjaeger and nixward May 11, 2021 13:18

zephyrbot assigned erwango May 11, 2021

str4t0m mentioned this pull request May 20, 2021

doc: release-notes-2.6: CAN bus and Task Watchdog #35487

Merged

str4t0m force-pushed the stm32g0_can branch from e104f9e to b65c20c Compare May 28, 2021 10:03

str4t0m force-pushed the stm32g0_can branch 3 times, most recently from baa9693 to 8afb5f7 Compare June 5, 2021 14:47

alexanderwachter requested changes Jun 8, 2021

View reviewed changes

str4t0m requested a review from alexanderwachter June 8, 2021 14:19

alexanderwachter requested changes Jun 8, 2021

View reviewed changes

str4t0m marked this pull request as draft June 11, 2021 09:57

str4t0m force-pushed the stm32g0_can branch from 8afb5f7 to 65ccb7d Compare June 15, 2021 16:41

github-actions bot added area: API Changes to public APIs area: Devicetree Binding PR modifies or adds a Device Tree binding labels Jun 15, 2021

str4t0m force-pushed the stm32g0_can branch from 65ccb7d to c393ff6 Compare June 16, 2021 01:03

erwango reviewed Jun 16, 2021

View reviewed changes

str4t0m force-pushed the stm32g0_can branch 2 times, most recently from 419f91c to 338127a Compare June 17, 2021 09:49

str4t0m added 7 commits June 19, 2021 21:25

drivers: shared_irq: add devicetree helper macros

2b67405

This commit adds helper macros to reduce boilerplate code in drivers that (conditionally) use the shared irq driver. Signed-off-by: Thomas Stranger <thomas.stranger@outlook.com>

drivers: intc: shared_irq: CONFIG_SHARED_IRQ for any enabled compatible

566df20

This commit enables SHARED_IRQ config option per default if any shared_irq node is enabled. Signed-off-by: Thomas Stranger <thomas.stranger@outlook.com>

dts: arm: stm32g0: add shared_irq nodes for irq 21 and 22

583cbd1

The shared-irq nodes are necessary to use the shared irq-driver This commit adds the necessary nodes for can1, can2, tim16, and tim17. Signed-off-by: Thomas Stranger <thomas.stranger@outlook.com>

dts: arm: stm32g0: add can bindings for stm32g0b1 soc

0841be5

In stm32g0 series only stm32g0b1 and stm32g0c1 socs support can. This commit adds the CAN nodes to the stm32g0b1 soc. Signed-off-by: Thomas Stranger <thomas.stranger@outlook.com>

board: nucleo_g0b1re: add CAN support

94b215e

This commit adds CAN support to the nucleo_g0b1re board. Signed-off-by: Thomas Stranger <thomas.stranger@outlook.com>

str4t0m force-pushed the stm32g0_can branch from 338127a to ececbf8 Compare June 19, 2021 19:40

erwango reviewed Jun 21, 2021

View reviewed changes

github-actions bot added the Stale label Aug 21, 2021

henrikbrixandersen removed their request for review August 25, 2021 09:20

github-actions bot closed this Sep 9, 2021

WardBenjamin mentioned this pull request Jan 1, 2024

doc: CAN-FD support not in support matrix for STM32 Nucleo G0B1 or G474 #67087

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable CAN peripheral on STM32G0 #34963

Enable CAN peripheral on STM32G0 #34963

str4t0m commented May 7, 2021 •

edited

Loading

str4t0m commented May 7, 2021

raul-klg commented May 7, 2021

alexanderwachter Jun 5, 2021

str4t0m Jun 8, 2021

alexanderwachter Jun 8, 2021

alexanderwachter Jun 8, 2021

str4t0m commented Jun 11, 2021

erwango Jun 16, 2021

str4t0m Jun 17, 2021

erwango Jun 17, 2021

str4t0m Jun 19, 2021

erwango Jun 21, 2021

alexanderwachter commented Jun 16, 2021 •

edited

Loading

str4t0m commented Jun 17, 2021

erwango Jun 21, 2021

github-actions bot commented Aug 21, 2021

dresco commented Nov 21, 2022

		shared-irqs = <&shared_irq21>, <&shared_irq22>;
		shared-irq-names = "LINE_0", "LINE_1";

Enable CAN peripheral on STM32G0 #34963

Enable CAN peripheral on STM32G0 #34963

Conversation

str4t0m commented May 7, 2021 • edited Loading

str4t0m commented May 7, 2021

raul-klg commented May 7, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

str4t0m commented Jun 11, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexanderwachter commented Jun 16, 2021 • edited Loading

str4t0m commented Jun 17, 2021

Choose a reason for hiding this comment

github-actions bot commented Aug 21, 2021

dresco commented Nov 21, 2022

str4t0m commented May 7, 2021 •

edited

Loading

alexanderwachter commented Jun 16, 2021 •

edited

Loading