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

initial CRC support #202

Merged
merged 2 commits into from
May 15, 2020
Merged

initial CRC support #202

merged 2 commits into from
May 15, 2020

Conversation

yjh0502
Copy link
Contributor

@yjh0502 yjh0502 commented Apr 19, 2020

I'm complete newcomer on microcontroller & stm32, so please feel free to comment.

  • Tested on stm32f103c8
  • I'm not sure how it interacts with other peripherals. For example it seems that SPI protocol requires CRC calculation. Does stm32 SPI uses CRC peripherals for CRC calculation of there's another CRC hardware dedicated to SPI CRC?
  • I'm not sure if I should use split or constrain.
  • small additional diff on prelude caused by rustfmt.

Copy link
Member

@TheZoq2 TheZoq2 left a comment

Choose a reason for hiding this comment

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

Thanks for the PR and sorry for the delayed reply!

Is there a reason you split the peripheral into a DR and a CR part instead of keeping them in one struct? Doing the latter would get rid of all the pointer dereferencing the CRC struct to get access to the registers instead of doing a bunch of unsafe pointer dereferencing

src/crc.rs Outdated
_0: (),
}

#[allow(dead_code)]
Copy link
Member

Choose a reason for hiding this comment

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

This probably shouldn't be needed

Copy link
Member

Choose a reason for hiding this comment

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

Really, why not just:

    let mut crc = Crc::new(p.CRC, &mut rcc.ahb);

    crc.reset();
    crc.write(0x12345678);

    let val = crc.read();

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed, could you review it again?

@yjh0502 yjh0502 force-pushed the master branch 2 times, most recently from 37fb18c to 43ad70e Compare May 2, 2020 04:30
src/crc.rs Outdated
}

pub fn write(&mut self, val: u32) {
unsafe { self.crc.dr.write(|w| w.bits(val)) }
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed, thanks!

@TheZoq2
Copy link
Member

TheZoq2 commented May 14, 2020

Looks good to me, thanks!

@TheZoq2
Copy link
Member

TheZoq2 commented May 14, 2020

Actually, would you mind adding an entry to the Changelog.md before we merge?

@yjh0502
Copy link
Contributor Author

yjh0502 commented May 15, 2020

@TheZoq2 I updated CHANGELOG.md :)

@TheZoq2
Copy link
Member

TheZoq2 commented May 15, 2020

Lovely, thanks!

@TheZoq2 TheZoq2 merged commit df0f47f into stm32-rs:master May 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants