stm32-rs / bxcan

bxCAN peripheral driver for STM32 chips
Apache License 2.0
32 stars 22 forks source link

Fix definition of bits in Interrupt enum #29

Closed samcrow closed 3 years ago

samcrow commented 3 years ago

The problem

When using Can::enable_interrupt or Can::disable_interruptwith the Interrupt enum (not the Interrupts flags), the incorrect bits in the interrupt enable register are modified.

Example

The code

fn read_ier() -> u32 {
    unsafe { core::ptr::read_volatile((0x4000_6400_usize + 0x14_usize) as *const u32) }
}

can.enable_interrupt(bxcan::Interrupt::Fifo0MessagePending);
rprintln!(
    "Interupt enable register set using single interrupt: {:#010x}",
    read_ier()
);
can.disable_interrupts(bxcan::Interrupts::all());
rprintln!("Interupt enable register cleared: {:#010x}", read_ier());
can.enable_interrupts(bxcan::Interrupts::FIFO0_MESSAGE_PENDING);
rprintln!(
    "Interupt enable register set using interrupt flags: {:#010x}",
    read_ier()
);

prints

Interupt enable register set using single interrupt: 0x00000001
Interupt enable register cleared: 0x00000000
Interupt enable register set using interrupt flags: 0x00000002

Why this happens

The Can::enable_interrupt and Can::disable_interruptfunctions convert the Interrupt into a bit flag. However, the Interrupt discriminant values are not flags, they are bit positions. In the above example, Interrupt::Fifo0MessagePending (defined as 1) gets converted into Interrupts::TRANSMIT_MAILBOX_EMPTY (also defined as 1), and the transmit mailbox empty bit gets set in the interrupt enable register.

The fix

This change simply changes the discriminant values of Interrupt so that they have the same values as the corresponding Interrupts flags. The modified code correctly enables interrupts in my tests so far.

jonas-schievink commented 3 years ago

Thanks!

bors r+

bors[bot] commented 3 years ago

Build succeeded: