Skip to content

Commit

Permalink
Merge #1874
Browse files Browse the repository at this point in the history
1874: nrf52: dont write to read only register r=ppannuto a=hudson-ayers

### Pull Request Overview

While trying to debug what is wrong with the nrf52 15.4 driver, I realized that the nrf52 clock code attempts to write to a register that both the nrf52840 and nrf52832 datasheets define as a read only register (page 107: https://infocenter.nordicsemi.com/pdf/nRF52832_PS_v1.4.pdf , https://infocenter.nordicsemi.com/index.jsp?topic=%2Fps_nrf52840%2Fclock.html&cp=4_0_0_4_3_2_16&anchor=register.HFCLKSTAT) .

This PR removes this incorrect code. Despite this function call not working, the code was actually functionally correct, because the subsequent line that starts the high speed clock has a side effect of setting the HighClockSource to XTAL anyway (per https://infocenter.nordicsemi.com/topic/ps_nrf52840/clock.html?cp=4_0_0_4_3_2_0#register.TASKS_HFCLKSTART).

### Testing Strategy

Flashing the 15.4 radio_tx app on an nrf52840. It works the same as before this change.


### TODO or Help Wanted

N/A.


### Documentation Updated

- [x] No updates are required.

### Formatting

- [x] Ran `make format`.
- [x] Fixed errors surfaced by `make clippy`.


Co-authored-by: Hudson Ayers <hayers@stanford.edu>
  • Loading branch information
bors[bot] and hudson-ayers committed May 22, 2020
2 parents 956c737 + 368f83d commit b9804f3
Show file tree
Hide file tree
Showing 3 changed files with 3 additions and 11 deletions.
1 change: 0 additions & 1 deletion boards/acd52832/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -446,7 +446,6 @@ pub unsafe fn reset_handler() {

nrf52832::clock::CLOCK.low_set_source(nrf52832::clock::LowClockSource::XTAL);
nrf52832::clock::CLOCK.low_start();
nrf52832::clock::CLOCK.high_set_source(nrf52832::clock::HighClockSource::XTAL);
nrf52832::clock::CLOCK.high_start();
while !nrf52832::clock::CLOCK.low_started() {}
while !nrf52832::clock::CLOCK.high_started() {}
Expand Down
1 change: 0 additions & 1 deletion boards/nordic/nrf52dk_base/src/nrf52_components/startup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,6 @@ impl Component for NrfClockComponent {

nrf52::clock::CLOCK.low_set_source(nrf52::clock::LowClockSource::XTAL);
nrf52::clock::CLOCK.low_start();
nrf52::clock::CLOCK.high_set_source(nrf52::clock::HighClockSource::XTAL);
nrf52::clock::CLOCK.high_start();
while !nrf52::clock::CLOCK.low_started() {}
while !nrf52::clock::CLOCK.high_started() {}
Expand Down
12 changes: 3 additions & 9 deletions chips/nrf52/src/clock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ register_structs! {
(0x308 => intenclr: ReadWrite<u32, Interrupt::Register>),
(0x30C => _reserved4),
(0x408 => hfclkrun: ReadOnly<u32, Status::Register>),
(0x40C => hfclkstat: ReadWrite<u32, HfClkStat::Register>),
(0x40C => hfclkstat: ReadOnly<u32, HfClkStat::Register>),
(0x410 => _reserved5),
(0x414 => lfclkrun: ReadOnly<u32, Control::Register>),
(0x418 => lfclkstat: ReadWrite<u32, LfClkStat::Register>),
Expand Down Expand Up @@ -200,7 +200,8 @@ impl Clock {
}
}

/// Start the high frequency clock
/// Start the high frequency clock - specifically HFXO, and sets the high frequency
/// clock source to HFXO
pub fn high_start(&self) {
let regs = &*self.registers;
regs.tasks_hfclkstart.write(Control::ENABLE::SET);
Expand Down Expand Up @@ -272,11 +273,4 @@ impl Clock {
let regs = &*self.registers;
regs.lfclksrc.write(LfClkSrc::SRC.val(clock_source as u32));
}

/// Set high frequency clock source
pub fn high_set_source(&self, clock_source: HighClockSource) {
let regs = &*self.registers;
regs.hfclkstat
.write(HfClkStat::SRC.val(clock_source as u32));
}
}

0 comments on commit b9804f3

Please sign in to comment.