Invalid drop of ClkCfg reference

[mkroman]

Clippy has found a bug in clock::Strict::freeze:

error: calls to `std::mem::drop` with a reference instead of an owned value. Dropping a reference does nothing.
   --> src/clock.rs:149:9
    |
149 |         drop(clk_cfg); // logically use its ownership
    |         ^^^^^^^^^^^^^
    |
    = note: `#[deny(clippy::drop_ref)]` on by default
note: argument has type `&mut gpio::ClkCfg`
   --> src/clock.rs:149:14
    |
149 |         drop(clk_cfg); // logically use its ownership
    |              ^^^^^^^
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#drop_ref

I don't quite understand the point of ClkCfg at all - why does it just have an inner unit type, and why is it immediately being dropped after being passed as an argument?

[luojia65]

My original design of this configurator is that it make uses of Rust's ownership system. The ClkCfg struct represents all the registers we should use in freeze procedure. In other HAL crates, HAL library uses this ownership representation to modify the registers logically belong to it. This strucutre shouldn't be dropped in further designs, it should have functions which returns PAC register structures to modify clock values. If there's no such field, all the contexts can freeze clock configurators which may result in data races (if I am correct). Should have something like:

impl ClkCfg fn clk_cfg2() -> pac::(path)::ClkCfg2 return unsafe { &*pac::GLB::ptr() }.clk_cfg2

(sorry for mobile phone texting, will reformat after i got a computer)

[mkroman]

I see. So the inner unit type in ClkCfg (()) is just a placeholder for now?

With regards to the drop call - is there any reason for this to be there? We don't take ownership of the type, and we don't borrow anything from it, so the mutable borrow only lives for as long as the call to freeze anyway.

[luojia65]

At first I thought the clock configuration is done by modifying one set of register like stm32, but this chip requires multiple registers from different peripherals. Maybe we can split out more register ownership from each of peripherals, or if possible just remove this ClkCfg struct totally.

[mkroman]

I'm going to remove the manual drop from clock::Strict::freeze in #11 since it's literally a no-op, but I'll of course keep the mutable borrow of the struct for forward compatibility.

We can always split the registers and use them in the future.

[mkroman]

Fixed in #11