msp432-rust / msp432p401r-hal

Hardware Abstraction Layer for MSP432P401R microcontroller
MIT License
4 stars 2 forks source link

Clock SMCLK Prescaler is incorrect #24

Closed prabhpreet closed 2 years ago

prabhpreet commented 2 years ago

Hello,

Thanks for this crate. I am using a MSP432P401R and I encountered an issue with this crate.

The SMCLK prescalar for SMCLK is converted to u32, which is incorrect. For example, SMPrescaler::DIVS_0 as u32 is evaluated as 0, which actually indicates no scaling, i.e. a division of 1.

I'd be happy to contribute a PR. Can you please merge the hotfix PR #22 so I can fix this? My changes are based on the hotfix.

Thanks

JoseClaudioSJr commented 2 years ago

Hi @prabhpreet,

I assume you refer to this line: https://github.com/msp432-rust/msp432p401r-hal/blob/003307d0b6b0186e8ed595f3e3b6e27b2e56ba97/src/clock.rs#L520

In fact, the right shift operator (>>) must be adopted, as in the line below: https://github.com/msp432-rust/msp432p401r-hal/blob/003307d0b6b0186e8ed595f3e3b6e27b2e56ba97/src/clock.rs#L518

But the prescaler configuration seems correct to me:
https://github.com/msp432-rust/msp432p401r-hal/blob/003307d0b6b0186e8ed595f3e3b6e27b2e56ba97/src/clock.rs#L469-L477

Thanks

prabhpreet commented 2 years ago

I assume you refer to this line:

https://github.com/msp432-rust/msp432p401r-hal/blob/003307d0b6b0186e8ed595f3e3b6e27b2e56ba97/src/clock.rs#L520

In fact, the right shift operator (>>) must be adopted, as in the line below:

https://github.com/msp432-rust/msp432p401r-hal/blob/003307d0b6b0186e8ed595f3e3b6e27b2e56ba97/src/clock.rs#L518 @JoseClaudioSJr

Yes, this is exactly the error, and causes a divide by zero error on SMPrescaler::DIVS_0 due to the division. Also, the built Clocks struct is used by modules such as Timer will result in incorrect timing if using SMCLK.

The bit shift is a nice trick. I've committed this as PR #25 for your convenience.

You're right, the prescaler configuration for CSCTL1 register configuration is correct and doesn't need any changes.

Thanks