stm32-rs / stm32f7xx-hal

A Rust embedded-hal HAL for all MCUs in the STM32 F7 family
Apache License 2.0
117 stars 68 forks source link

Add new implementation for RCC #74

Closed no111u3 closed 4 years ago

no111u3 commented 4 years ago

Create new version of RCC setup, with direct setup clocks. Also update clock calculation and add usb pll configuration.

mvertescher commented 4 years ago

We should do some testing on this PR before it lands just in case. If anyone has a chance to run through some sanity configurations that would be helpful. I may have some time later this week to test, unsure though.

Disasm commented 4 years ago

Result of calculations depends on the position of .set_sysclk() call in a builder chain, this is not good.

Disasm commented 4 years ago

Also this test fails:

    #[test]
    fn test_rcc_calc1() {
        use super::{PLLP, HSEClock, HSEClockMode};
        use crate::time::U32Ext;

        let cfgr = CFGR {
            hse: None,
            hclk: None,
            pclk1: None,
            pclk2: None,
            use_pll: false,
            use_pll48clk: false,
            pllm: 2,
            plln: 50,
            pllp: PLLP::Div2,
            pllq: 2,
        };

        let cfgr = cfgr
            .hse(HSEClock::new(25.mhz(), HSEClockMode::Bypass))
            .use_pll()
            .use_pll48clk()
            .sysclk(216.mhz());

        let (clocks, config) = cfgr.calculate_clocks();
        assert_eq!(clocks.sysclk().0, 216_000_000);
        assert!(clocks.is_pll48clk_valid());
    }
no111u3 commented 4 years ago

Result of calculations depends on the position of .set_sysclk() call in a builder chain, this is not good.

Fixed.

no111u3 commented 4 years ago

Also this test fails:

    #[test]
    fn test_rcc_calc1() {
        use super::{PLLP, HSEClock, HSEClockMode};
        use crate::time::U32Ext;

        let cfgr = CFGR {
            hse: None,
            hclk: None,
            pclk1: None,
            pclk2: None,
            use_pll: false,
            use_pll48clk: false,
            pllm: 2,
            plln: 50,
            pllp: PLLP::Div2,
            pllq: 2,
        };

        let cfgr = cfgr
            .hse(HSEClock::new(25.mhz(), HSEClockMode::Bypass))
            .use_pll()
            .use_pll48clk()
            .sysclk(216.mhz());

        let (clocks, config) = cfgr.calculate_clocks();
        assert_eq!(clocks.sysclk().0, 216_000_000);
        assert!(clocks.is_pll48clk_valid());
    }

Fixed, test added to test group in rcc.

mvertescher commented 4 years ago

@Disasm any other thoughts on this PR? If not, I'll merge this and we can follow up with any issues that arise.

Disasm commented 4 years ago

Apparently, tests should be updated to reflect pll_configure() signature change.

no111u3 commented 4 years ago

Apparently, tests should be updated to reflect pll_configure() signature change.

Fixed.

Disasm commented 4 years ago

@mvertescher I think this PR is ready for merge.