Modify Stm32Can to support multiple CAN interfaces on ST Micro MCUs that have multiple CAN IFs #816

RobertPHeller · 2025-01-31T21:12:12Z

This is a modification to the Stm32Can that allows for multiple CAN IFs running on those ST Micro MCUs that have multiple CAN IFs. When the class is instanciated, the trailing digit in the path name determines which CAN IF this instance is for. If the path ends in '0', CAN1 (or the only) interface is used, if it ends in '1', CAN2 is used (if it exists), and if the path ends in '2', CAN3 is used.

Typical usage:

In HwInit.cxx:

/** CAN 0 CAN driver instance */
static Stm32Can can0("/dev/can0");
/** CAN 1 CAN driver instance */
static Stm32Can can1("/dev/can1");

Later:

   __HAL_RCC_CAN1_CLK_ENABLE();
   __HAL_RCC_CAN2_CLK_ENABLE();

and:

    /* CAN1 pinmux on PB8 and PB9 */
    gpio_init.Mode = GPIO_MODE_AF_PP;
    gpio_init.Pull = GPIO_PULLUP;
    gpio_init.Speed = GPIO_SPEED_FREQ_HIGH;
    gpio_init.Alternate = GPIO_AF9_CAN1;
    gpio_init.Pin = GPIO_PIN_8;
    HAL_GPIO_Init(GPIOB, &gpio_init);
    gpio_init.Pin = GPIO_PIN_9;
    HAL_GPIO_Init(GPIOB, &gpio_init);

    /* CAN2 pinmux on PB12 and PB13 */
    gpio_init.Mode = GPIO_MODE_AF_PP;
    gpio_init.Pull = GPIO_PULLUP;
    gpio_init.Speed = GPIO_SPEED_FREQ_HIGH;
    gpio_init.Alternate = GPIO_AF9_CAN2;
    gpio_init.Pin = GPIO_PIN_12;
    HAL_GPIO_Init(GPIOB, &gpio_init);
    gpio_init.Pin = GPIO_PIN_13;
    HAL_GPIO_Init(GPIOB, &gpio_init);

Then in main.cxx:

    HubDeviceSelect<CanHubFlow> seg1(&can_hub0,"/dev/can0");
    HubDeviceSelect<CanHubFlow> seg2(&can_hub0,"/dev/can1");

I have tested this:

https://github.com/RobertPHeller/RPi-RRCircuits/tree/master/STM32F767ZI_LCC_PNET_Router

src/freertos_drivers/st/Stm32Can.cxx

RobertPHeller · 2025-01-31T22:32:25Z

Opps! Fixed. At Fri, 31 Jan 2025 13:56:38 -0800 bakerstu/openmrn ***@***.***> wrote:

…

@atanisoft commented on this pull request. > @@ -542,6 +634,41 @@ void can1_sce_interrupt_handler(void) Stm32Can::instances[0]->sce_interrupt_handler(); } +<<<<<<< HEAD looks like a merge artifact snuck in.

-- Robert Heller -- Cell: 413-658-7953 GV: 978-633-5364 Deepwoods Software -- Custom Software Services http://www.deepsoft.com/ -- Linux Administration Services ***@***.*** -- Webhosting Services

bakerstu · 2025-02-03T00:04:13Z

src/freertos_drivers/st/Stm32Can.cxx

+    instances[index] = this;
+    switch (index)
+    {
+    case 0: /* CAN1... */


The style guidelines require case labels to be indented four spaces.

bakerstu · 2025-02-03T00:05:59Z

src/freertos_drivers/st/Stm32Can.hxx

    /** Default constructor.
     */
    Stm32Can();
-
+


Remove the extra empty line. Only one empty line required for spacing.

bakerstu · 2025-02-03T00:11:46Z

src/freertos_drivers/st/Stm32Can.cxx

@@ -103,13 +132,58 @@ Stm32Can::Stm32Can(const char *name)
    : Can(name)
    , state_(CAN_STATE_STOPPED)
 {
-    /* only one instance allowed */
-    HASSERT(instances[0] == NULL);
+    /* Get dev num (digit at the end of the devname). */


I don't like that we have programmatically tied the device "name" to the hardware instance. As far as I recall, we don't have any such tie anywhere else in the device file system. I think it makes since to instead pass the hardware references in (base address and interrupt number), similar to what we do in the Tiva driver:

openmrn/src/freertos_drivers/ti/TivaCan.cxx

Line 58 in e35c58d

TivaCan::TivaCan(const char *name, unsigned long base, uint32_t interrupt)

I strongly encourage that with the addition of these parameters, either we have an overloaded constructor that is backwards compatible fixed to the first CAN instance, or we use "default" values for the parameters to point to the first CAN instance. That way we don't break other existing uses.

bakerstu · 2025-02-03T00:14:09Z

src/freertos_drivers/st/Stm32Can.hxx

@@ -45,6 +45,12 @@

 #include "stm32f_hal_conf.hxx"

+// Max possible number of CAN ifs across the whole STM32 -- the constructor


Please turn this into a Doxygen style comment with three slashes.

bakerstu · 2025-02-03T00:14:31Z

src/freertos_drivers/st/Stm32Can.hxx

@@ -91,11 +97,17 @@ private:
    static unsigned int intCount;

    uint8_t state_; ///< present bus state
-
+
+    CAN_TypeDef *can_;


Please add Doxygen comments for these members.

bakerstu · 2025-02-03T00:14:40Z

src/freertos_drivers/st/Stm32Can.hxx

+// will calculate the actual max for the specific chip compiling for
+#define MAXCANIFS 3 
+
+//struct CAN_TypeDef;


Is this dead code?

bakerstu · 2025-02-03T00:15:52Z

src/freertos_drivers/st/Stm32Can.cxx

@@ -36,6 +36,7 @@
 #include "Stm32Can.hxx"

 #include <stdint.h>
+#include <string.h>


This include might be able to be removed if the programatic select of device instance by name is also removed.

RobertPHeller · 2025-02-03T01:35:47Z

At Sun, 02 Feb 2025 16:16:26 -0800 bakerstu/openmrn ***@***.***> wrote:

@bakerstu commented on this pull request. > - instances[0] = this; + instances[index] = this; + switch (index) + { + case 0: /* CAN1... */ The style guidelines require case labels to be indented four spaces.

OK, easy fix. (My editor has a different auto-indent process for switch labels...)

> /** Default constructor. */ Stm32Can(); - + Remove the extra empty line. Only one empty line required for spacing.

OK.

> @@ -103,13 +132,58 @@ Stm32Can::Stm32Can(const char *name) : Can(name) , state_(CAN_STATE_STOPPED) { - /* only one instance allowed */ - HASSERT(instances[0] == NULL); + /* Get dev num (digit at the end of the devname). */ I don't like that we have programmatically tied the device "name" to the hardware instance. As far as I recall, we don't have any such tie anywhere else in the device file system. I think it makes since to instead pass the hardware references in (base address and interrupt number), similar to what we do in the Tiva driver: https://github.com/bakerstu/openmrn/blob/e35c58d4aba4b9d5a4dc355684a2eb1c87008f01/src/freertos_drivers/ti/TivaCan.cxx#L58

I just picked the "Linux" way of naming devices -- on the Linux SBCs that I have with multiple CAN IFs (Beagle boards), the devices are named can0, can1, etc. I can fix this in the consstructor, but I'll keep the path names the same, since they are used elsewhere.

I strongly encourage that with the addition of these parameters, either we have an overloaded constructor that is backwards compatible fixed to the first CAN instance, or we use "default" values for the parameters to point to the first CAN instance. That way we don't break other existing uses.

OK, I can do this.

> @@ -45,6 +45,12 @@ #include "stm32f_hal_conf.hxx" +// Max possible number of CAN ifs across the whole STM32 -- the constructor

Easy fix/

Please turn this into a Doxygen style comment with three slashes. > @@ -91,11 +97,17 @@ private: static unsigned int intCount; uint8_t state_; ///< present bus state - + + CAN_TypeDef *can_; Please add Doxygen comments for these members.

OK.

> @@ -45,6 +45,12 @@ #include "stm32f_hal_conf.hxx" +// Max possible number of CAN ifs across the whole STM32 -- the constructor +// will calculate the actual max for the specific chip compiling for +#define MAXCANIFS 3 + +//struct CAN_TypeDef; Is this dead code?

I think so, The original had a single instance of the CAN_TypeDef struct outside of the class, but the new version has it as a instance variable.

> @@ -36,6 +36,7 @@ #include "Stm32Can.hxx" #include <stdint.h> +#include <string.h> This include might be able to be removed if the programatic select of device instance by name is also removed.

OK, I'll check that.

…

-- Robert Heller -- Cell: 413-658-7953 GV: 978-633-5364 Deepwoods Software -- Custom Software Services http://www.deepsoft.com/ -- Linux Administration Services ***@***.*** -- Webhosting Services

RobertPHeller added 8 commits April 29, 2024 15:24

Modify Stm32Can to support multiple CAN interfaces.

1ed2b90

Minor fixes (CAN_TypeDef decl).

38941d2

Minor fixes (typo).

97b0c9e

Minor fixes (IRQn_Type).

2944bd6

Minor fixes (L4 CAN1 def).

85328ce

Minor fixes (L4 CAN2 def).

f9a71b9

Remove Conflicts.

60b000a

Multiple CAN IFs on ST Micro MCUs.

0072cc8

atanisoft reviewed Jan 31, 2025

View reviewed changes

src/freertos_drivers/st/Stm32Can.cxx Outdated Show resolved Hide resolved

Fix git merge artifacts.

d2c0a1f

bakerstu reviewed Feb 3, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Modify Stm32Can to support multiple CAN interfaces on ST Micro MCUs that have multiple CAN IFs #816

Modify Stm32Can to support multiple CAN interfaces on ST Micro MCUs that have multiple CAN IFs #816

RobertPHeller commented Jan 31, 2025

RobertPHeller commented Jan 31, 2025 via email

bakerstu Feb 3, 2025

bakerstu Feb 3, 2025

bakerstu Feb 3, 2025

bakerstu Feb 3, 2025

bakerstu Feb 3, 2025

bakerstu Feb 3, 2025

bakerstu Feb 3, 2025

RobertPHeller commented Feb 3, 2025 via email

		@@ -45,6 +45,12 @@

		#include "stm32f_hal_conf.hxx"

		// Max possible number of CAN ifs across the whole STM32 -- the constructor

Modify Stm32Can to support multiple CAN interfaces on ST Micro MCUs that have multiple CAN IFs #816

Are you sure you want to change the base?

Modify Stm32Can to support multiple CAN interfaces on ST Micro MCUs that have multiple CAN IFs #816

Conversation

RobertPHeller commented Jan 31, 2025

RobertPHeller commented Jan 31, 2025 via email

bakerstu Feb 3, 2025

Choose a reason for hiding this comment

bakerstu Feb 3, 2025

Choose a reason for hiding this comment

bakerstu Feb 3, 2025

Choose a reason for hiding this comment

bakerstu Feb 3, 2025

Choose a reason for hiding this comment

bakerstu Feb 3, 2025

Choose a reason for hiding this comment

bakerstu Feb 3, 2025

Choose a reason for hiding this comment

bakerstu Feb 3, 2025

Choose a reason for hiding this comment

RobertPHeller commented Feb 3, 2025 via email