<re_irc>
<@gauteh:matrix.org> firefrommoonlight: I'm writing the driver, so I could design it differently. So I'd have to borrow its peripherals for the duration of sleep, are there other drivers that do this?
<re_irc>
<@gauteh:matrix.org> Maybe I'm overcomplicating this.. I'll try a bit more.
fabic_ has quit [Remote host closed the connection]
fabic has joined #rust-embedded
<re_irc>
<@firefrommoonlight:matrix.org> I can't tell from what you've posted so far, but it does sound like a design issue
<re_irc>
<@firefrommoonlight:matrix.org> Is it that the peripheral struct in your driver is accepting pins generically, then losing the ability to do things other than setting high/low?
fabic has quit [Ping timeout: 268 seconds]
<re_irc>
<@gauteh:matrix.org> Yes, the driver needs to own pins and the i2c interface. I need to power down and disable both during deep-sleep. I don't want to desctruct the driver every sleep since it has state, and partial data. If the driver is to stay device agnostic it needs to use embedded-hal traits, but there isn't hal traits for power down or similar. I think I need to make a borrow_peripherals() or something on the driver which...
<re_irc>
... makes it disabled until the peripherals are put back in the correct state.
<re_irc>
<@gauteh:matrix.org> There might be something in shared-bus that can help me.
unmanbearpig has quit [Ping timeout: 260 seconds]
unmanbearpig has joined #rust-embedded
Shellhound is now known as Shell
<re_irc>
<@ryan-summers:matrix.org> I would probably implement a separate trait in your device crate so that you require something like `<T: i2c::Write + crate::PowerDown>`
<re_irc>
<@ryan-summers:matrix.org> Obviously if you're remotely managing the powerstate of your interface, you cannot safely share the interface with multiple drivers unless you have some common crate that implements both i2c + power management and throws an error if you try to use i2c while powered down
<re_irc>
<@ryan-summers:matrix.org> But it honestly seems to me that deep-sleep may be better managed at a higher level interface
<re_irc>
<@ryan-summers:matrix.org> E.g. in user applications, where they have concrete knowledge of the implementation details
<re_irc>
<@henrikssn:matrix.org> I handled this with success in the idle task when using RTIC
<re_irc>
<@henrikssn:matrix.org> so idle task would power down all connected sensors before calling WFI
<re_irc>
<@henrikssn:matrix.org> Then power them back up again after the WFI call
<re_irc>
<@firefrommoonlight:matrix.org> gauteh:matrix.org: Don't use EH here
<re_irc>
<@firefrommoonlight:matrix.org> What you're trying to do is straight fwd and common
<re_irc>
<@firefrommoonlight:matrix.org> It should just be an issue of calling the apt methods on the pin structs etc
fabic has joined #rust-embedded
<re_irc>
<@gauteh:matrix.org> firefrommoonlight:matrix.org: what is EH? embedded-hal? Are you saying that the design I just described is common?
<re_irc>
<@jamwaffles:matrix.org> > what is EH? embedded-hal?
<re_irc>
<@jamwaffles:matrix.org> As an aside, yep, EH = embedded-hal. I've also often seen `e-h` as well
crabbedhaloablut has quit [Ping timeout: 276 seconds]
crabbedhaloablut has joined #rust-embedded
Darius has quit [Ping timeout: 260 seconds]
Darius has joined #rust-embedded
fabic has quit [Ping timeout: 268 seconds]
fabic has joined #rust-embedded
fabic has quit [Ping timeout: 264 seconds]
<re_irc>
<@eldruin:matrix.org> hmm, the problem with the reconfiguration of the pins does not have much to do with `e-h` really but rather with your HAL impl of an I2C struct (which then happens to implement the e-h I2C traits) and your driver.
<re_irc>
<@eldruin:matrix.org> There are always a few possibilities, depending on how much you want your driver to know about this suspension.
<re_irc>
<@eldruin:matrix.org> - Option 1 "your driver knows": store the I2C peripheral inside an `Option` and offer methods to take the struct out and back in when you are back from sleep. However, your driver now needs to unpack the `Option` when it interacts with the peripheral and handle when it is not there with a new error variant like `CannotAccessI2cNow` or so. The user of your driver needs to know about this as well.
<re_irc>
<@eldruin:matrix.org> - Option 2 "stealing": Your driver stays as-is. You write a wrapper around your HAL I2C peripheral that basically implements the whole `Option` thing and methods to take the peripheral in and out and also implements `e-h`'s I2C traits. If you call when the pins are not there, you panic. You just promise not to do this.
<re_irc>
<@eldruin:matrix.org> If you can change your HAL impl you can also do some parts of this in the HAL impl as well
emerent has quit [Ping timeout: 260 seconds]
emerent has joined #rust-embedded
<re_irc>
<@sourcebox:matrix.org> Because someone asked a few days ago: GitHub images are now on 1.56 for Linux and macOS, Windows is still 1.55.
fabic has joined #rust-embedded
fabic has quit [Ping timeout: 260 seconds]
<re_irc>
<@firefrommoonlight:matrix.org> gauteh:matrix.org: I'm saying manipulating pins (Changing mode, state etc) is common when entering and exiting low power mode
<re_irc>
<@firefrommoonlight:matrix.org> And that using embedded-hal traits on a periph struct that owns pins isn't an appropriate patterns
<re_irc>
<@firefrommoonlight:matrix.org> The op you described should be easy, but the design pattern you're using is complicating it
<re_irc>
<@firefrommoonlight:matrix.org> *I agree with eldruin that this is probably more to do with the HAL I2C owning pins. I'm unclear on if it's the I2C pins, other pins used by your driver, or both
<re_irc>
<@gauteh:matrix.org> This is I2C pins + some peripherals related to the hardware i2c on the chip
<re_irc>
<@firefrommoonlight:matrix.org> What is the problem specifically?
<re_irc>
<@firefrommoonlight:matrix.org> Not being able to set SDA/SCL pin state, or something else?
<re_irc>
<@gauteh:matrix.org> The I2C struct implemented by the hal (which I have also written), takes ownership of the pins + the IOM peripherals. This is passed on to the driver crate for another device connected by i2c. When I go to low-power mode I would need to power down the IOM + disable the pins.
<re_irc>
<@gauteh:matrix.org> But these are owned by the i2c struct so that they cannot be used multiple times. and passed on to the driver, which owns the i2c struct.
<re_irc>
<@gauteh:matrix.org> the driver only knows about embedded-hal.
<re_irc>
<@firefrommoonlight:matrix.org> So, both?
<re_irc>
<@firefrommoonlight:matrix.org> How do you power down an IOM?
<re_irc>
<@firefrommoonlight:matrix.org> I recommend not having the I2 C struct own the pins
<re_irc>
<@firefrommoonlight:matrix.org> You could work around it by using the PAC directly, but I'd modify the I2C struct to not own
<re_irc>
<@gauteh:matrix.org> Why? Then it would be possible to safely mess around with the pins while the i2c is active.
<re_irc>
<@gauteh:matrix.org> firefrommoonlight:matrix.org: The chip has an hardware i2c/iom controller which fills up buffers with sent or received bytes, including ACKing, NACKing, etc. But it only does so if it is enabled.
<re_irc>
<@firefrommoonlight:matrix.org> I don't understand what you're trying to do other than change GPIO state
<re_irc>
<@firefrommoonlight:matrix.org> The API you're using yo manage I2C pins is inadequate
<re_irc>
<@gauteh:matrix.org> Yes, I'm just trying to figure out a good way to do this. eldruin s suggestions would all fix this.
<re_irc>
<@firefrommoonlight:matrix.org> If you're more worried about stopping yourself from altering I2C pin state than having a working or simple API, I question your intent
<re_irc>
<@gauteh:matrix.org> But they wouldn't be necessary if the I2C struct didn't own its pins.
<re_irc>
<@firefrommoonlight:matrix.org> Owning pins is a bad pattern since it makes procedures like the one you're doing (Which is common) awkward
<re_irc>
<@dirbaio:matrix.org> i2c owning the pins is good, it prevents you from trying to use the same pin for 2 things at the same time
* re_irc
@eldruin:matrix.org is leaving ð
<re_irc>
<@firefrommoonlight:matrix.org> Alternatively, don't use it for 2 things at the same time
<re_irc>
<@gauteh:matrix.org> thanks for your suggestions
<re_irc>
<@dirbaio:matrix.org> alternatively, write C ðĪŠ
<re_irc>
<@dirbaio:matrix.org> safety is overrated
<re_irc>
<@dirbaio:matrix.org> the way I'd do it is to tear down i2c + the pins before sleep, reconstruct on wakeup
<re_irc>
<@dirbaio:matrix.org> the driver could be set up to borrow i2c instead of owning it
<re_irc>
<@dirbaio:matrix.org> or to take it at every method
<re_irc>
<@dirbaio:matrix.org> embedded-hal 1.0 will have blanket impls for &mut's so you'll be able to construct a driver with a &mut T instead of a T, without the driver having to explicitly opt in
<re_irc>
<@thejpster:matrix.org> I was writing C today. It was like performing surgery with a chainsaw.
<re_irc>
<@firefrommoonlight:matrix.org> That's a nice change
<re_irc>
<@dirbaio:matrix.org> gauteh:matrix.org: if the driver's struct has the &mut then maybe no. It'd only help if you're constructing/dropping the driver every time
<re_irc>
<@dirbaio:matrix.org> otherwise you could make the driver take the &mut as a param on each method instead of having it in the struct (eldruin's option 3)
<re_irc>
<@firefrommoonlight:matrix.org> Please tell me that bool line doesn't compile
<re_irc>
<@newam:matrix.org> firefrommoonlight:matrix.org: It compiles with GCC 11 on godbolt, no warnings.
<re_irc>
<@dirbaio:matrix.org> it sure compiles lmao
<re_irc>
<@firefrommoonlight:matrix.org> TWIM never owns the pins
<re_irc>
<@dirbaio:matrix.org> when you do `mem::drop(twi);` the borrows on the pins get cleared, so you could do `Output::new( &mut p.P0_03, Level::Low)` to set it low while sleeping
<re_irc>
<@firefrommoonlight:matrix.org> The 'unsafe' part is due to not having made I2C methods to connect and disconnect the pins - I should add them
<re_irc>
<@dirbaio:matrix.org> no need to do raw register writes
<re_irc>
<@dirbaio:matrix.org> adding methods to TWIM to connect/disconnect pins is still "dangerous", you can try to use the TWIM while the pins are disconnected which will fail
<re_irc>
<@firefrommoonlight:matrix.org> Then don't do that!
<re_irc>
<@firefrommoonlight:matrix.org> You could do the write on the SPI periph instead and that would also fail
<re_irc>
<@firefrommoonlight:matrix.org> Or do any of a bunch of wrong things
<re_irc>
<@firefrommoonlight:matrix.org> Guards are fine as long as they don't get in the way
<re_irc>
<@dirbaio:matrix.org> guards rarely get in the way
<re_irc>
<@dirbaio:matrix.org> even less with embassy's ability to create TWIM with &mut pins (vs traditional hals which can only own and then you have to `.free()` the TWIM)
<re_irc>
<@dirbaio:matrix.org> I'd rather have the "main" API have as few footguns as possible
<re_irc>
<@firefrommoonlight:matrix.org> I think the crux of this is that the 'safety' concerns here are set up so that only the most conservative use case is allowed
<re_irc>
<@firefrommoonlight:matrix.org> I don't think that's practical
<re_irc>
<@firefrommoonlight:matrix.org> APIs that block useful operations aren't my cup of tea
<re_irc>
<@dirbaio:matrix.org> option 1: have a main API that covers 100% of the cases, but has footguns everywhere
<re_irc>
<@dirbaio:matrix.org> option 2: have a main API that covers 99.9% of the cases with NO footguns, have `steal()` for the remanining 0.1%
<re_irc>
<@dirbaio:matrix.org> IMO option 2 is better
<re_irc>
<@firefrommoonlight:matrix.org> I don't think manipulating a pin into the wrong state, then having it not work is a foot gun
<re_irc>
<@firefrommoonlight:matrix.org> Any more than adding 1 and 1 and expecting 3 is
<re_irc>
<@firefrommoonlight:matrix.org> Changing pin state for SDA etc is valid
<re_irc>
<@firefrommoonlight:matrix.org> Unless you make the most conservative assumption about usage
<re_irc>
<@dirbaio:matrix.org> it is a footgun lol
<re_irc>
<@dirbaio:matrix.org> you yourself ran into it like last week
<re_irc>
<@firefrommoonlight:matrix.org> I prefer clear, ergonomic APIs
<re_irc>
<@firefrommoonlight:matrix.org> That was dumb
<re_irc>
<@firefrommoonlight:matrix.org> (context: I forgot to set Open Drain on I2C pins)
<re_irc>
<@gauteh:matrix.org> Maybe the quickest is to make a suspend method which in addition to the pins also returns the state of the driver (incl buffers), so that it can be reconstructed from the state. Should compile down to nothing. With &mut on i2c, the pins will be deconfigured automatically, but it's not the end of the world if I have to do that manually. Still requires me to not mess around with the device in the meantime, but...
<re_irc>
<@sourcebox:matrix.org> What I'm missing the most is that you can't just deactivate optimization for a block of code like with pragmas in gcc. Or did I miss something here? To my knowledge, the crate level is the smallest granularity for optimization settings.
<re_irc>
<@therealprof:matrix.org> Not sure. There was some discussion of adding attributes which would allow one to change it per function, but I haven't been tracking this.
<re_irc>
<@sourcebox:matrix.org> Ok. I thought it would be easier to add such a thing than implementing new optimization levels.
<re_irc>
<@therealprof:matrix.org> Apparently it's not quite that simple to feed that information back into LLVM because optimization is happening on a lot of different levels.
<re_irc>
<@sourcebox:matrix.org> Usually, when debugging, I concentrate on a certain block of code, so I need good debugging capabilities just there. The other parts could be heavily optimized.
<re_irc>
<@therealprof:matrix.org> Honestly I would have loved to just being able to properly control this per crate. But I haven't quite figured out how to do that even though that is supposedly possible.
<re_irc>
<@sourcebox:matrix.org> What also could happen is the opposite: have the code optimized for size except some DSP-heavy parts where full loop unrolling etc. would be apreciated.
<re_irc>
<@sourcebox:matrix.org> What I put into my cargo.toml is:
<re_irc>
<@sourcebox:matrix.org> opt-level = "z"
<re_irc>
<@sourcebox:matrix.org> `
<re_irc>
<@sourcebox:matrix.org> This should optimize the dependencies even when you're main code stays in debug mode.
<re_irc>
<@therealprof:matrix.org> Does that work? I've never seen it work. That would be quite useful, in fact that's what I've been requesting as the default for a long time. It's quite rare that you want to debug foreign crates, especially not libstd or libcore; those really should always be optimized unless you're running cargo check.
<re_irc>
<@sourcebox:matrix.org> Try it out. But I thing it does not work well for macros that you import from other crates.
<re_irc>
<@sourcebox:matrix.org> At least it reduces the code size in my case.
<re_irc>
<@dirbaio:matrix.org> how does inlining work when different crates have different opt-levels?
<re_irc>
<@dirbaio:matrix.org> the caller crate's optlevel matters? or the callee's?
<re_irc>
<@sourcebox:matrix.org> Good question, also when taking LTO into account.
<re_irc>
<@dirbaio:matrix.org> hmm is LTO even controllable per-crate? hehe
<re_irc>
<@sourcebox:matrix.org> No, but it does overall heavy inlining.
<re_irc>
<@sourcebox:matrix.org> LTO has become more and more agressive in the last years.
<re_irc>
<@sourcebox:matrix.org> But this overall optimization thing needs some solution in the future. Otherwise certain things become hard to deal with, e.g. writing a bootloader that's only allowed to take a few k.
<re_irc>
<@sourcebox:matrix.org> Just some quick test: 16524 vs 21596 bytes with/without `[profile.dev.package."*"] opt-level = "z"`
<re_irc>
<@sourcebox:matrix.org> So it does something at least.
<re_irc>
<@therealprof:matrix.org> I'm trying it right now with my bloat bench. I'm seeing misterious new compiler errors though.
<re_irc>
<@therealprof:matrix.org> It does make a difference, albeit not where I would expect it...
<re_irc>
<@therealprof:matrix.org> Seems it works on custom dependencies but not libcore. âđïļ
<re_irc>
<@dirbaio:matrix.org> -Zbuild-std=core?
<re_irc>
<@dirbaio:matrix.org> otherwise it'll use the prebuilt one
<re_irc>
<@therealprof:matrix.org> Right, I'm not using -Zbuild-std=core...
<re_irc>
<@sourcebox:matrix.org> Maybe I should also try that.
<re_irc>
<@therealprof:matrix.org> Only works on nightly, though.
<re_irc>
<@sourcebox:matrix.org> Then not ;-)
<re_irc>
<@sourcebox:matrix.org> I try to get away with stable. Although I found out that missing inline asm makes some stuff like critical sections expensive.
<re_irc>
<@therealprof:matrix.org> Hum?
<re_irc>
<@sourcebox:matrix.org> Because a simple disable/enable imterrupt goes through linked C code.
<re_irc>
<@sourcebox:matrix.org> Normally only 1 instruction.
<re_irc>
<@therealprof:matrix.org> I don't follow I'm afraid. Our CSes are quite fast, only very few instructions last time I checked.
<re_irc>
<@therealprof:matrix.org> 1 Instruction seems very optimistic.
<re_irc>
<@dirbaio:matrix.org> the cortex-m inline-outline-asm stuff doesn't get inlined
<re_irc>
<@sourcebox:matrix.org> Not the whole CS but the disable/enable
<re_irc>
<@dirbaio:matrix.org> you need nightly and features=inline-asm to get the cortex-m asm to inline
<re_irc>
<@adamgreig:matrix.org> or linker-plugin-lto iirc
<re_irc>
<@adamgreig:matrix.org> but yea, it's otherwise a function call so adds a jump and return
<re_irc>
<@thalesfragoso:matrix.org> Yeah, linker-plugin-lto works on stable
<re_irc>
<@dirbaio:matrix.org> the normal lto=fat doesn't inline them either
<re_irc>
<@dirbaio:matrix.org> I haven't tried linker-plugin-lto, interesting! though I use nighlty anyway so I'd rather use inline-asm heh
<re_irc>
<@sourcebox:matrix.org> This I already realised.
<re_irc>
<@adamgreig:matrix.org> if you're on nightly then inline-asm makes most sense
<re_irc>
<@thalesfragoso:matrix.org> And it will inline cortex-m asm
<re_irc>
<@adamgreig:matrix.org> there's also the, uh, more cursed option for inline asm on stable
<re_irc>
<@thalesfragoso:matrix.org> But you need to enable the cortex-m feature too
<re_irc>
<@dirbaio:matrix.org> maybe -O3 instead of -Oz?
<re_irc>
<@dirbaio:matrix.org> O3 can get reaaally bloaty
<re_irc>
<@adamgreig:matrix.org> gotta go fast
<re_irc>
<@therealprof:matrix.org> That's the claim. At least it's not inlined and it certainly gets the monomorphisation treatment...
<re_irc>
<@therealprof:matrix.org> If you're doing a ton of operations on `Option<T>` and `Result<T>` you're hit with the full hammer of a lot of copies...
<re_irc>
<@thalesfragoso:matrix.org> That is with a stable compiler :)
<re_irc>
<@therealprof:matrix.org> sourcebox:matrix.org: Weird. The `__cpsie` and `__cpsid` functions are only 2 instructions each. Not sure where the rest would come from.
<re_irc>
<@sourcebox:matrix.org> Ok, there's some more you need because you have to check if interrupts are enabled before you enter and react accordingly.
<re_irc>
<@sourcebox:matrix.org> thalesfragoso:matrix.org: This does a good job on my test code: 16524 vs 16524 bytes.
<re_irc>
<@sourcebox:matrix.org> But still around 40 cycles for a CS.
<re_irc>
<@dirbaio:matrix.org> or llvm-objdump but its output is a bit less nice
<re_irc>
<@dirbaio:matrix.org> or `rustup install llvm-tools-preview; cargo install cargo-binutils` then you can do `cargo objdump` (but this uses llvm-objdump)
<re_irc>
<@sourcebox:matrix.org> I have cargo-binutils installed. This should also do it somehow.
<re_irc>
<@dirbaio:matrix.org> arm-none-eabi-objdump puts more names in the asm output, like function names and so
<re_irc>
<@thalesfragoso:matrix.org> You might also need an `#[inline(never)]` on the `ticks` fn, I mean, you can still find it if it gets inlined, but it's a bit harder...
<re_irc>
<@sourcebox:matrix.org> The problem is, it generates a lot of output. The whole code contains more than just this test.
<re_irc>
<@dirbaio:matrix.org> put it in a fn with a recognizable name, make it inline(never), then control+f the output for that name
<re_irc>
<@thalesfragoso:matrix.org> In gdb you should be able to do something like `disasm function_name`
<re_irc>
<@sourcebox:matrix.org> It will end up with the same no of cycles because TICK_FREQ is 1000 and the division will be eliminated completely. At least this is what I think what's happening.