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.

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

cpu/samd5x write can driver #19736

Merged
merged 10 commits into from
Mar 28, 2024
Merged

cpu/samd5x write can driver #19736

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
3eeac98
makefiles: add CAN RX mailbox feature
firas-hamdi Mar 26, 2024
e4e5558
cpu/samd5x: implement driver for samd5x CAN controller
firas-hamdi Mar 22, 2024
477f9cd
boards/same54-xpro: add CAN periph definitions
firas-hamdi Mar 22, 2024
a40735a
sys/include/can: include mailbox feature
firas-hamdi Mar 22, 2024
01782e8
dist/tools/doccheck: exclude macro def error
firas-hamdi Mar 22, 2024
74bf01b
drivers/mcp2515: load RX mailbox
firas-hamdi Mar 26, 2024
491a1cd
tests/drivers/candev: add test commands
firas-hamdi Mar 22, 2024
16cfaab
cpu/samd5x: fix C++ build
benpicco Mar 26, 2024
568d607
sys/auto_init/can: include board periph_conf.h
kfessel Mar 27, 2024
9d2dda2
Merge branch 'master' into cpu/samd5x-write-CAN-driver
kfessel Mar 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions boards/same54-xpro/Makefile.features
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ FEATURES_PROVIDED += periph_uart_hw_fc
FEATURES_PROVIDED += periph_adc
FEATURES_PROVIDED += periph_usbdev
FEATURES_PROVIDED += periph_freqm
FEATURES_PROVIDED += periph_can
kfessel marked this conversation as resolved.
Show resolved Hide resolved

# Put other features for this board (in alphabetical order)
FEATURES_PROVIDED += riotboot
Expand Down
7 changes: 7 additions & 0 deletions boards/same54-xpro/include/board.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,13 @@ extern "C" {
#define BTN0_MODE GPIO_IN_PU
/** @} */

/**
* @name ATA6561 STANDBY pin definition
* @{
*/
#define AT6561_STBY_PIN GPIO_PIN(PC, 13)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this could have a name like CAN_TRANSCEIVER_STBY..

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AT6561 is the CAN transceiver name. I followed the naming of the other components defined in the board

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure but the name is not relevant - you want to use a generic name so the test app where you are using it can be generic too.

On a second note, shouldn't that pin be handled by the driver in the first place?
Only the CAN driver would know when to put the transceiver into standby mode.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It probably is the application that knows best how to handle the standby pin if it exists (or at least it is up to the board) the standby pin is often usable to stay silent on the bus but still receive

Copy link
Contributor

@benpicco benpicco Feb 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But we have CANOPT_STATE_SLEEP/CANOPT_STATE_LISTEN_ONLY for that. When set, the driver should put the transceiver into standby mode - not the application.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

someone might use two can controllers that the samd5 has, each of them has a transceiver -> this would add another element to can_conf_t , but such an addition would not cover the varianz of can transceivers not all boards have a standby, some transceivers have extra configuration like slop rate (spi or i2c), i don't think we should add a traciever driver into the can driver and adding even a simple one might lead there

also not all transcievers have this monitor in standby feature ( might have another name in that case like sleep)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://e2e.ti.com/support/interface-group/interface/f/interface-forum/755985/tcan1042-q1-difference-between-silent-mode-and-standby-mode

seems like for that transceiver standby isn't as transparent (listen only would not work with that, for other transceivers it might, more like silent by TI definition)

maybe the can_trx driver interface should be used for that (driver/tja1042 seems to have a standby pin)

kfessel marked this conversation as resolved.
Show resolved Hide resolved
/** @} */

/**
* @name MTD configuration
* @{
Expand Down
21 changes: 21 additions & 0 deletions boards/same54-xpro/include/periph_conf.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,27 @@ static const tc32_conf_t timer_config[] = {
#define TIMER_NUMOF ARRAY_SIZE(timer_config)
/** @} */

/**
* @name CAN configuration
* @{
*/
/** Available CAN interfaces */
static const can_conf_t candev_conf[] = {
{
.can = CAN1,
.rx_pin = GPIO_PIN(PB, 13),
.tx_pin = GPIO_PIN(PB, 12),
.gclk_src = SAM0_GCLK_PERIPH,
}
};

/** CAN 1 configuration */
#define ISR_CAN1 isr_can1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you might want to align this line with the other defines in the file


/** Number of CAN interfaces */
#define CAN_NUMOF ARRAY_SIZE(candev_conf)
/** @} */

/**
* @name UART configuration
* @{
Expand Down
4 changes: 4 additions & 0 deletions cpu/samd5x/Makefile.dep
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,8 @@ ifneq (,$(filter periph_gpio_tamper_wake,$(USEMODULE)))
USEMODULE += periph_rtc_rtt
endif

ifneq (,$(filter periph_can,$(USEMODULE)))
FEATURES_REQUIRED += can_rx_mailbox
endif

include $(RIOTCPU)/sam0_common/Makefile.dep
1 change: 1 addition & 0 deletions cpu/samd5x/Makefile.features
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,6 @@ FEATURES_PROVIDED += periph_gpio_tamper_wake
FEATURES_PROVIDED += periph_rtc_mem
FEATURES_PROVIDED += periph_spi_on_qspi
FEATURES_PROVIDED += periph_uart_collision
FEATURES_PROVIDED += can_rx_mailbox

include $(RIOTCPU)/sam0_common/Makefile.features
44 changes: 44 additions & 0 deletions cpu/samd5x/include/can_params.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/*
* Copyright (C) 2024 ML!PA Consulting GmbH
*
* This file is subject to the terms and conditions of the GNU Lesser
* General Public License v2.1. See the file LICENSE in the top level
* directory for more details.
*/

/**
* @ingroup cpu_samd5x
* @brief CPU specific definitions for CAN controllers
* @{
*
* @file
* @brief SAMD5x CAN controller (M_CAN Bosch) default device parameters
*
* @author Firas Hamdi <firas.hamdi@ml-pa.com>
*/

#ifndef CAN_PARAMS_H
benpicco marked this conversation as resolved.
Show resolved Hide resolved
#define CAN_PARAMS_H

#ifdef __cplusplus
extern "C" {
#endif

#include "can/device.h"

/** Default SAMD5x CAN devices parameters */
static const candev_params_t candev_params[] = {
{
.name = "can_samd5x_0",
},
{
.name = "can_samd5x_1",
}
};

#ifdef __cplusplus
}
#endif

#endif /* CAN_PARAMS_H */
firas-hamdi marked this conversation as resolved.
Show resolved Hide resolved
/** @} */
161 changes: 161 additions & 0 deletions cpu/samd5x/include/candev_samd5x.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,161 @@
/*
* Copyright (C) 2024 ML!PA Consulting GmbH
*
* This file is subject to the terms and conditions of the GNU Lesser
* General Public License v2.1. See the file LICENSE in the top level
* directory for more details.
*/

/**
* @ingroup cpu_samd5x
* @brief CPU specific definitions for CAN controllers
* @{
*
* @file
* @brief SAMD5x CAN controller (M_CAN Bosch) driver
*
* @author Firas Hamdi <firas.hamdi@ml-pa.com>
*/

#ifndef CANDEV_SAMD5X_H
benpicco marked this conversation as resolved.
Show resolved Hide resolved
benpicco marked this conversation as resolved.
Show resolved Hide resolved
#define CANDEV_SAMD5X_H

#ifdef __cplusplus
extern "C" {
#endif

#if defined(CAN_INST_NUM)
#include "can/candev.h"

#ifndef CANDEV_SAMD5X_DEFAULT_BITRATE
/** Default bitrate */
#define CANDEV_SAMD5X_DEFAULT_BITRATE 500000U
#endif

#ifndef CANDEV_SAMD5X_DEFAULT_SPT
/** Default sampling-point */
#define CANDEV_SAMD5X_DEFAULT_SPT 875
#endif

#ifndef CANDEV_SAMD5X_DEFAULT_STD_FILTER_NUM
#define CANDEV_SAMD5X_DEFAULT_STD_FILTER_NUM 3
#endif

#ifndef CANDEV_SAMD5X_DEFAULT_EXT_FILTER_NUM
#define CANDEV_SAMD5X_DEFAULT_EXT_FILTER_NUM 3
#endif

#ifndef CANDEV_SAMD5X_DEFAULT_RX_FIFO_0_ELTS_NUM
#define CANDEV_SAMD5X_DEFAULT_RX_FIFO_0_ELTS_NUM 32
#endif

#ifndef CANDEV_SAMD5X_DEFAULT_RX_FIFO_1_ELTS_NUM
#define CANDEV_SAMD5X_DEFAULT_RX_FIFO_1_ELTS_NUM 32
#endif

#ifndef CANDEV_SAMD5X_DEFAULT_TX_EVT_FIFO_ELTS_NUM
#define CANDEV_SAMD5X_DEFAULT_TX_EVT_FIFO_ELTS_NUM 16
#endif

#ifndef CANDEV_SAMD5X_DEFAULT_TX_BUFFER_NUM
#define CANDEV_SAMD5X_DEFAULT_TX_BUFFER_NUM 16
#endif

#ifndef CANDEV_SAMD5X_DEFAULT_TX_BUFFER_FIFO_QUEUE_NUM
#define CANDEV_SAMD5X_DEFAULT_TX_BUFFER_FIFO_QUEUE_NUM 16
#endif

/* unit: elements */
#define CANDEV_SAMD5X_MAX_STD_FILTER 128
#define CANDEV_SAMD5X_MAX_EXT_FILTER 64
#define CANDEV_SAMD5X_MAX_RX_FIFO_0_ELTS 64
#define CANDEV_SAMD5X_MAX_RX_FIFO_1_ELTS 64
#define CANDEV_SAMD5X_MAX_RX_BUFFER 64
#define CANDEV_SAMD5X_MAX_TX_EVT_FIFO_ELTS 32
#define CANDEV_SAMD5X_MAX_TX_BUFFER 32
#define CANDEV_SAMD5X_MSG_RAM_MAX_SIZE 448

/**
* @brief CAN device configuration descriptor
*/
typedef struct {
/** CAN device handler */
Can *can;
/** CAN Rx pin */
gpio_t rx_pin;
/** CAN Tx pin */
gpio_t tx_pin;
/** GCLK source supplying the CAN controller */
uint8_t gclk_src;
} can_conf_t;
#define HAVE_CAN_CONF_T

/**
* @brief CAN message RAM accessible to the CAN controller
*/
typedef struct {
/** Standard filters space in the CAN message RAM */
CanMramSidfe std_filter[CANDEV_SAMD5X_DEFAULT_STD_FILTER_NUM];
/** Extended filters space in the CAN message RAM */
CanMramXifde ext_filter[CANDEV_SAMD5X_DEFAULT_EXT_FILTER_NUM];
/** Reception FIFO_0 space in the CAN message RAM */
CanMramRxf0e rx_fifo_0[CANDEV_SAMD5X_DEFAULT_RX_FIFO_0_ELTS_NUM];
/** Reception FIFO_1 space in the CAN message RAM */
CanMramRxf1e rx_fifo_1[CANDEV_SAMD5X_DEFAULT_RX_FIFO_1_ELTS_NUM];
/** Reception buffers space in the CAN message RAM */
CanMramRxbe rx_buffer[CANDEV_SAMD5X_MAX_RX_BUFFER];
/** Transmission events FIFO space in the CAN message RAM */
CanMramTxefe tx_event_fifo[CANDEV_SAMD5X_DEFAULT_TX_EVT_FIFO_ELTS_NUM];
/** Transmission FIFO space in the CAN message RAM */
CanMramTxbe tx_fifo_queue[CANDEV_SAMD5X_DEFAULT_TX_BUFFER_FIFO_QUEUE_NUM];
} msg_ram_conf_t;

/**
* @brief CAN device descriptor
*/
typedef struct {
/** Structure to hold driver state */
candev_t candev;
/** CAN device configuration descriptor */
const can_conf_t *conf;
/** CAN message RAM accessible to the CAN controller */
msg_ram_conf_t msg_ram_conf;
/** Enable/Disable Transceiver Delay Compensation */
bool tdc_ctrl;
/** False to use Tx FIFO operation, True to use Tx Queue operation */
bool tx_fifo_queue_ctrl;
} can_t;
#define HAVE_CAN_T

/**
* @brief Enable/Disable the transmitter delay compensation
*
* @param[in] dev device descriptor
*
*/
void candev_samd5x_tdc_control(can_t *dev);

/**
* @brief Enter the device into sleep mode
*
* @param[in] candev candev driver
*
*/
void candev_samd5x_enter_sleep_mode(candev_t *candev);

/**
* @brief Wake up the device from sleep mode
*
* @param[in] candev candev driver
*
*/
void candev_samd5x_exit_sleep_mode(candev_t *candev);

#endif /* CAN_INST_NUM */

#ifdef __cplusplus
}
#endif

#endif /* CANDEV_SAMD5X_H */
firas-hamdi marked this conversation as resolved.
Show resolved Hide resolved
/** @} */
1 change: 1 addition & 0 deletions cpu/samd5x/include/periph_cpu.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include "macros/units.h"
#include "periph_cpu_common.h"

#include "candev_samd5x.h"
firas-hamdi marked this conversation as resolved.
Show resolved Hide resolved
#ifdef __cplusplus
extern "C" {
#endif
Expand Down
Loading
Loading