nrf-rs/nrf-hal

Incorrect I2S MckFreq mapping

RuneKokNielsen opened this issue · 2 comments

Observation

Calling set_mck_frequency(&self, freq: MckFreq) on I2S instance with any choice of MckFreq will cause the driver to stop functioning. Specifically, subsequent transmissions will stall.

Cause

The set_mck_frequency method uses the following From<MckFreq> implementation to retrieve 32 bit values for the underlying driver:

impl From<MckFreq> for u32 { 
      fn from(variant: MckFreq) -> Self {
          variant as _
      }
}

This assumes that each of the 12 variants should be mapped into the [0-12] range. However, the actual option values are not in this range. Thus, invalid values are put in the configuration.

From the nRF52840 docs:


A	RW	MCKFREQ	 	 	Master clock generator frequency.
 	 	 32MDIV8	0x20000000	32 MHz / 8 = 4.0 MHz
 	 	 32MDIV10	0x18000000	32 MHz / 10 = 3.2 MHz
 	 	 32MDIV11	0x16000000	32 MHz / 11 = 2.9090909 MHz
 	 	 32MDIV15	0x11000000	32 MHz / 15 = 2.1333333 MHz
 	 	 32MDIV16	0x10000000	32 MHz / 16 = 2.0 MHz
 	 	 32MDIV21	0x0C000000	32 MHz / 21 = 1.5238095
 	 	 32MDIV23	0x0B000000	32 MHz / 23 = 1.3913043 MHz
 	 	 32MDIV30	0x08800000	32 MHz / 30 = 1.0666667 MHz
 	 	 32MDIV31	0x08400000	32 MHz / 31 = 1.0322581 MHz
 	 	 32MDIV32	0x08000000	32 MHz / 32 = 1.0 MHz
 	 	 32MDIV42	0x06000000	32 MHz / 42 = 0.7619048 MHz
 	 	 32MDIV63	0x04100000	32 MHz / 63 = 0.5079365 MHz
 	 	 32MDIV125	0x020C0000	32 MHz / 125 = 0.256 MHz

Patched From implementation:

impl From<MckFreq> for u32 {
    fn from(variant: MckFreq) -> Self {
        match variant {
            _32MDiv8   => 0x20000000,
            _32MDiv10  => 0x18000000,
            _32MDiv11  => 0x16000000,
            _32MDiv15  => 0x11000000,
            _32MDiv16  => 0x10000000,
            _32MDiv21  => 0x0C000000,
            _32MDiv23  => 0x0B000000,
            _32MDiv30  => 0x08800000,
            _32MDiv31  => 0x08400000,
            _32MDiv32  => 0x08000000,
            _32MDiv42  => 0x06000000,
            _32MDiv63  => 0x04100000,
            _32MDiv125 => 0x020C0000
        }
    }
}

We could also assign explicit discriminants, that avoids conversion overhead