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

cpu/samd5x: improve can-initialization #21173

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
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
46 changes: 37 additions & 9 deletions cpu/samd5x/periph/can.c
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
*/

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

#include "periph/can.h"
Expand Down Expand Up @@ -187,15 +188,39 @@

static void _setup_clock(can_t *dev)
{
if (dev->conf->can == CAN0) {
GCLK->PCHCTRL[CAN0_GCLK_ID].reg = GCLK_PCHCTRL_CHEN | GCLK_PCHCTRL_GEN(dev->conf->gclk_src);
int pchid = 0;
if (dev->conf->can == CAN0){
pchid = CAN0_GCLK_ID;
}
else if (dev->conf->can == CAN1) {
GCLK->PCHCTRL[CAN1_GCLK_ID].reg = GCLK_PCHCTRL_CHEN | GCLK_PCHCTRL_GEN(dev->conf->gclk_src);
else if (dev->conf->can == CAN1){
pchid = CAN1_GCLK_ID;
}
else {
/* only CAN0 and CAN1 supported. When ported to MCUs with more
* CAN controllers, this code needs to be adapted */
DEBUG_PUTS("CAN channel not supported");
assert(0);
return;
}

uint32_t pchctrl = GCLK->PCHCTRL[pchid].reg;

/* disable */
GCLK->PCHCTRL[pchid].reg = pchctrl & (~GCLK_PCHCTRL_CHEN);
do {
pchctrl = GCLK->PCHCTRL[pchid].reg;
} while (pchctrl & GCLK_PCHCTRL_CHEN);

/* setup */
pchctrl = GCLK_PCHCTRL_GEN(dev->conf->gclk_src);
GCLK->PCHCTRL[pchid].reg = pchctrl;
Copy link
Contributor

Choose a reason for hiding this comment

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

why no busy wait loop here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

at this point the register is changeable (since the channel is deactivated)

Copy link
Member

Choose a reason for hiding this comment

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

I think @mguetschow suggested to add a comment here to explain why not waiting here, in contrast to the other writes.


/* enable */
pchctrl |= GCLK_PCHCTRL_CHEN;
GCLK->PCHCTRL[pchid].reg = pchctrl;
do {
pchctrl = GCLK->PCHCTRL[pchid].reg;
} while (!(pchctrl & GCLK_PCHCTRL_CHEN));
}

static void _set_bit_timing(can_t *dev)
Expand All @@ -214,7 +239,7 @@

/* Set bit timing */
dev->conf->can->NBTP.reg = (uint32_t)((CAN_NBTP_NTSEG2(dev->candev.bittiming.phase_seg2 - 1))
| (CAN_NBTP_NTSEG1(dev->candev.bittiming.phase_seg1 + dev->candev.bittiming.prop_seg - 1))

Check warning on line 242 in cpu/samd5x/periph/can.c

View workflow job for this annotation

GitHub Actions / static-tests

line is longer than 100 characters
| (CAN_NBTP_NBRP(dev->candev.bittiming.brp - 1))
| (CAN_NBTP_NSJW(dev->candev.bittiming.sjw - 1)));
}
Expand Down Expand Up @@ -355,9 +380,9 @@
| CAN_RXF1C_F1S((uint32_t)(ARRAY_SIZE(dev->msg_ram_conf.rx_fifo_1)));
dev->conf->can->RXBC.reg = CAN_RXBC_RBSA((uint32_t)(dev->msg_ram_conf.rx_buffer));
dev->conf->can->TXEFC.reg = CAN_TXEFC_EFSA((uint32_t)(dev->msg_ram_conf.tx_event_fifo))
| CAN_TXEFC_EFS((uint32_t)(ARRAY_SIZE(dev->msg_ram_conf.tx_event_fifo)));

Check warning on line 383 in cpu/samd5x/periph/can.c

View workflow job for this annotation

GitHub Actions / static-tests

line is longer than 100 characters
dev->conf->can->TXBC.reg = CAN_TXBC_TBSA((uint32_t)(dev->msg_ram_conf.tx_fifo_queue))
| CAN_TXBC_TFQS((uint32_t)(ARRAY_SIZE(dev->msg_ram_conf.tx_fifo_queue)));

Check warning on line 385 in cpu/samd5x/periph/can.c

View workflow job for this annotation

GitHub Actions / static-tests

line is longer than 100 characters

/* In the vendor file, the data field size in CanMramTxbe is set to 64 bytes
although it can be configurable. That's why 64 bytes is used here by default */
Expand All @@ -377,17 +402,20 @@
}
/* Disable automatic retransmission by default */
/* This can be added as a configuration parameter for the CAN controller */
dev->conf->can->CCCR.reg |= CAN_CCCR_DAR;
dev->conf->can->CCCR.reg = CAN_CCCR_DAR;

/* Reject all remote frames */
dev->conf->can->GFC.reg |= CAN_GFC_RRFE | CAN_GFC_RRFS;
dev->conf->can->GFC.reg = CAN_GFC_RRFE | CAN_GFC_RRFS;

/* Enable reception interrupts: reception on FIFO0 and FIFO1 */
dev->conf->can->IE.reg |= CAN_IE_RF0NE | CAN_IE_RF1NE;
uint32_t ie_reg = CAN_IE_RF0NE | CAN_IE_RF1NE;
/* Enable transmission events interrupts */
dev->conf->can->IE.reg |= CAN_IE_TEFNE;
ie_reg |= CAN_IE_TEFNE;
/* Enable errors interrupts */
dev->conf->can->IE.reg |= CAN_IE_PEDE | CAN_IE_PEAE | CAN_IE_BOE | CAN_IE_EWE | CAN_IE_EPE;
ie_reg |= CAN_IE_PEDE | CAN_IE_PEAE | CAN_IE_BOE | CAN_IE_EWE | CAN_IE_EPE;
/* write Interrupt enable register */
dev->conf->can->IE.reg = ie_reg;

/* Enable the interrupt lines */
dev->conf->can->ILE.reg = CAN_ILE_EINT0 | CAN_ILE_EINT1;

Expand Down Expand Up @@ -458,7 +486,7 @@
/* Write the second word */
dev->msg_ram_conf.tx_fifo_queue[can_mm.put].TXBE_1.reg = txbe1;

memcpy((void *)dev->msg_ram_conf.tx_fifo_queue[can_mm.put].TXBE_DATA, frame->data, frame->can_dlc);

Check warning on line 489 in cpu/samd5x/periph/can.c

View workflow job for this annotation

GitHub Actions / static-tests

line is longer than 100 characters

/* Request transmission */
dev->conf->can->TXBAR.reg |= (1 << can_mm.put);
Expand Down Expand Up @@ -544,9 +572,9 @@
DEBUG("Extended Filter to add at idx = %d\n", idx);
dev->msg_ram_conf.ext_filter[idx].XIDFE_0.reg |= CAN_XIDFE_0_EFEC(filter_conf);
/* For now, only CLASSIC filters are supported */
dev->msg_ram_conf.ext_filter[idx].XIDFE_1.reg |= CAN_XIDFE_1_EFT(CANDEV_SAMD5X_CLASSIC_FILTER);

Check warning on line 575 in cpu/samd5x/periph/can.c

View workflow job for this annotation

GitHub Actions / static-tests

line is longer than 100 characters
dev->msg_ram_conf.ext_filter[idx].XIDFE_0.reg |= CAN_XIDFE_0_EFID1(filter->can_id);
dev->msg_ram_conf.ext_filter[idx].XIDFE_1.reg |= CAN_XIDFE_1_EFID2(filter->can_mask & CAN_EFF_MASK);

Check warning on line 577 in cpu/samd5x/periph/can.c

View workflow job for this annotation

GitHub Actions / static-tests

line is longer than 100 characters
DEBUG("Extended filter element N°%d: F0 = 0x%08lx, F1 = 0x%08lx\n", idx,
(uint32_t)(dev->msg_ram_conf.ext_filter[idx].XIDFE_0.reg),
(uint32_t)(dev->msg_ram_conf.ext_filter[idx].XIDFE_1.reg));
Expand Down Expand Up @@ -583,9 +611,9 @@
DEBUG("Standard Filter to add at idx = %d\n", idx);
/* For now, only CLASSIC filters are supported */
dev->msg_ram_conf.std_filter[idx].SIDFE_0.reg = CAN_SIDFE_0_SFEC(filter_conf)
| CAN_SIDFE_0_SFT(CANDEV_SAMD5X_CLASSIC_FILTER)

Check warning on line 614 in cpu/samd5x/periph/can.c

View workflow job for this annotation

GitHub Actions / static-tests

line is longer than 100 characters
| CAN_SIDFE_0_SFID1(filter->can_id & CAN_SFF_MASK)

Check warning on line 615 in cpu/samd5x/periph/can.c

View workflow job for this annotation

GitHub Actions / static-tests

line is longer than 100 characters
| CAN_SIDFE_0_SFID2(filter->can_mask & CAN_SFF_MASK);

Check warning on line 616 in cpu/samd5x/periph/can.c

View workflow job for this annotation

GitHub Actions / static-tests

line is longer than 100 characters

DEBUG("Standard filter element N°%d: S0 = 0x%08lx\n", idx,
(uint32_t)(dev->msg_ram_conf.std_filter[idx].SIDFE_0.reg));
Expand Down Expand Up @@ -749,7 +777,7 @@
frame_received.can_dlc =
(dev->msg_ram_conf.rx_fifo_0[rx_get_idx].RXF0E_1.reg & CAN_RXF0E_1_DLC_Msk) >>
CAN_RXF0E_1_DLC_Pos;
memcpy(frame_received.data, (uint32_t *)dev->msg_ram_conf.rx_fifo_0[rx_get_idx].RXF0E_DATA, frame_received.can_dlc);

Check warning on line 780 in cpu/samd5x/periph/can.c

View workflow job for this annotation

GitHub Actions / static-tests

line is longer than 100 characters

dev->conf->can->RXF0A.reg = CAN_RXF0A_F0AI(rx_get_idx);
if (dev->candev.event_callback) {
Expand Down