ChanServ changed the topic of #rust-embedded to: Welcome to the Rust Embedded IRC channel! Bridged to #rust-embedded:matrix.org and logged at https://libera.irclog.whitequark.org/rust-embedded, code of conduct at https://www.rust-lang.org/conduct.html
starblue1 has quit [Ping timeout: 256 seconds]
starblue1 has joined #rust-embedded
cr1901_ is now known as cr1901
gsalazar has quit [Ping timeout: 272 seconds]
jackneill has quit [Remote host closed the connection]
jackneill has joined #rust-embedded
<re_irc> <gauteh> I am wondering about some general read-modify-write stuff wrt to registers or memory: e.g. the cortex_m::free: it reads the primask, then disables and enables if primask is enabled. is there a possible race-condition there? where another task could disable the interrupts between the read and the disable? is it possible to read and clear the primask in one operation?
<re_irc> <gauteh> i've tried to read up on this, but i'm still not completely sure: in a pac. register.modify is also vulnerable to theses race conditions? (not UB, but race).
<re_irc> <gauteh> is there a difference to register.read + register.write, compared to register.modify?
<cr1901> write will set bits that you don't explicitly set/clear to their reset value
<cr1901> modify will leave the same bits alone
<cr1901> modify should be an atomic single instruction
<re_irc> <burrbull> write will set bits that you don't explicitly set/clear to their reset value
<cr1901> hmmm... yea I could've worded that better
<re_irc> <gauteh> Yes, I am more thinking about if some other interrupt modifies the register between read-modify-write
<re_irc> <gauteh> does both register.write and register.modify return the new value (or the old value?)
<re_irc> <gauteh> and how is cortex_m::interrupt::free primask -> read -> write not open for this RC?
<re_irc> if another task disables interrupts: by definition you aren't currently running, won't be running again until interrupts are reenabled, and by then system state should be restored.
<re_irc> <9names (@9names:matrix.org)> not sure I understand what you're asking here.
<re_irc> <9names (@9names:matrix.org)> if you succeeded in disabling interrupts: nothing else is running, and you can read/write without data races - at least if you can assume no multicore / DMA writes to the register.
<re_irc> <jannic> cr1901: Btw this is not true in general. I don't know if it's implemented as an atomic instruction on CPUs which have such atomics, but some don't, and there it's definitely implemented as read-modify-write. Without any internal locking to avoid races.
<cr1901> Hmmm, AIUI according to svd2rust that shouldn't be the case. But ahh well :(
<re_irc> resets.reset.modify(|_, w| w.uart0().set_bit());
<re_irc> <jannic> pub fn modify_example(resets: &mut pac::RESETS) {
<re_irc> }
<re_irc> gets compiled to:
<re_irc> <jannic> pub fn modify_example(resets: &mut pac::RESETS) {
<re_irc> resets.reset.modify(|_, w| w.uart0().set_bit());
<re_irc> gets compiled to:
<re_irc> }
<re_irc> <jannic> (highlighting doesn't seem to work in code blocks - the three lines with the stars are the read-modify-write instructions)
<re_irc> <adamgreig> gauteh: the svd2rust register.modify() _would_ race if you could call it from two contexts (two interrupts or an interrupt and main) at the same time
<re_irc> <adamgreig> _but_ you can only get one instance of the peripheral object, so you can't have it in both places (without using "unsafe")
<re_irc> <adamgreig> you can move it between contexts, but then it can't be in both at once, or you can use some synchronisation primitive (like cortex_m::Mutex or RTIC) to share it, but those tools enforce that the sharing is synchronised/unique so that you can't be calling it at the same time
<re_irc> <adamgreig> so in safe code, you cannot end up calling modify() in two places at the same time, which means you can't cause that race
<re_irc> <adamgreig> there's no general way to read-modify-write otherwise. on some platforms there are alternatives - armv7 (cortex-m3/4/7) supports exclusive load/store operations, though it's a bit controversial whether they work on peripheral registers or just normal memory, some microcontrollers (stm32f1/f4 for example) support "bitbanding" where every bit of a configuration register is mapped to a whole u32 elsewhere in memory so...
<re_irc> ... you can set bits individually, but that doesn't let you update multiple bits without racing, and rp2040 supports arbitrary AND/OR/XOR atomic operations on its configuration registers via another mechanism
<re_irc> <adamgreig> but none of those options are present on all the platforms svd2rust targets, and they usually have some other cost associated with them, so it just does a normal "read value from memory into register, update value in register, write register back to memory"
<re_irc> <adamgreig> with interrupt::free, as 9names said the race is harmless because if someone else does call it between you reading primask and you disabling interrupts, they must also restore primask to whatever it was by the time they return
<cr1901> "someone else does call it" <-- what is "it" in this context?
<re_irc> <adamgreig> the interrupt::free() function
<cr1901> Oh right, CortexM doesn't disable interrupts when an interrupt happens
<re_irc> <dngrs (spookyvision@github)> any serial port cracks around? I am sending some data from mcu to host as soon as "dtr() == true" and the data gets swallowed in every case I've tried ("serialport" crate, "tokio-serial" async crate, "picocom(1)", "miniterm.py") _except_ "screen(1)"
<re_irc> <dngrs (spookyvision@github)> a 20ms delay fixes the issue but this seems like duct taping over something more fundamental
<re_irc> <gauteh> Thanks a lot for the explanation! adamgreig 9names and others!
<re_irc> <jannic> dngrs (spookyvision@github): I modified https://github.com/rp-rs/rp-hal/blob/main/boards/rp-pico/examples/pico_usb_serial.rs so it does wait for dtr and then writes it's welcome message. (And to also toggle the LED so I can see when it happens)
<re_irc> loop {
<re_irc> // A welcome message at the beginning
<re_irc> if !said_hello && serial.dtr() {
<re_irc> <gauteh> adamgreig: but someone else could call just interrupt::disable() in the mean time, after free the interrupts would be enabled again
<re_irc> <gauteh> (but maybe that is documented / unsafe)
<re_irc> <adamgreig> I don't think that can lead to any unsafety though?
<re_irc> <adamgreig> The original call to free will still only enable interrupts if they were enabled when free was originally called
<re_irc> <gauteh> not unsafety, but it could cause interrupts to be re-enabled
<re_irc> <dngrs (spookyvision@github)> jannic: I am waiting for dtr and then writing my buffered data, yes. And this doesn't work in any case except "screen" being on the receiving end. If I add 20ms delay after dtr it works in all cases.
<re_irc> <adamgreig> Sure, but the only worry we have with enabling interrupts is whether it gets in the way of using critical sections for memory safety, essentially
<re_irc> <gauteh> Ok... but if you're using the cortex_m Mutex with free for instance, this could lead to rather surprising behaviour
<re_irc> <gauteh> if you're changing interrupts on main
<re_irc> <dngrs (spookyvision@github)> old code:
<re_irc> fn usb_tx(cx: usb_tx::Context) {
<re_irc> #[task(binds = OTG_FS, local = [usb_dev, serial, rx], shared = [do_debug])]
<re_irc> let rx = cx.local.rx;
<re_irc> <dngrs (spookyvision@github)> (and a serial poll below that)
<re_irc> <gauteh> If the cpu supports Read-Modify-Write it seems that this would be a less likely issue?
<re_irc> <dngrs (spookyvision@github)> new code adds another safety bool that gets set to "true" 20ms after the first time dtr becomes "true"
<re_irc> <jannic> The original code, without an additional wait, works for me. With "screen", with "picocom", with "cat /dev/ttyACM0" and with a simple rust program using "serialport". But, interestingly, the rust program stops working when I run it with "strace"!?
<re_irc> mrs r0, PRIMASK
<re_irc> <gauteh> looking at some C hal that does this, it contains:
<re_irc> cpsie i
<re_irc> so i guess that between the mrs and the cpsid/cpise the race could happen. there isn't really any way to read PRIMASK and cpisd i atomically
<re_irc> bx lr
<re_irc> <jannic> dngrs (spookyvision@github): But I think the "strace" still found the culprit:
<re_irc> openat(AT_FDCWD, "/dev/ttyACM0", O_RDWR|O_NOCTTY|O_NONBLOCK) = 3
<re_irc> ioctl(3, TCGETS, {B9600 opost isig icanon echo ...}) = 0
<re_irc> ioctl(3, TCGETS, {B9600 opost isig icanon echo ...}) = 0
<re_irc> <jannic> That explains why "strace" made a difference. Without "strace", in my case, the flush happened before the MCU was able to send it's string. But with "strace", the computer was slightly slower, the MCU sent first, and the string was thrown away by the flush command.
<re_irc> <dngrs (spookyvision@github)> jannic: eunrrrgh! FWIW, I'm on MacOS, so platform differences, too…
<re_irc> <dngrs (spookyvision@github)> I guess I'll just live with the 20ms delay for now then. Maybe make it 50 even to have a bit of safety buffer…
<re_irc> <dngrs (spookyvision@github)> oh, what I could try for shits & giggles is to turn on turbo boost on my laptop and see if it works then. The machine -is- pretty slow rn on purpose (less fan noise/heat)
<re_irc> <dngrs (spookyvision@github)> doesn't work with 1ms delay even, so "no delay" is out
<re_irc> <dngrs (spookyvision@github)> trying to find the precise threshold…
<re_irc> <dngrs (spookyvision@github)> seems to be 3ms no matter whether TB is active 🤔
<re_irc> <dngrs (spookyvision@github)> might be of interest:
<re_irc> - I also encounter the issue with "tokio-serial" which seems to use a different serial port impl under the hood. But maybe it's modeled after "serialport-rs" and makes the same mistake, haven't looked into the source yet
<re_irc> - "serialport-rs" maintainer is AWOL, but a resurrection is on the way
<re_irc> <dngrs (spookyvision@github)> (and with picocom, and miniterm.py)
<re_irc> <jannic> "tokio-serial" depends on "mio-serial", which in turn depends on "serialport". So it's quite possible that the same code is used.
<re_irc> <dngrs (spookyvision@github)> ah ok, didn't notice the last part there.
<re_irc> <dngrs (spookyvision@github)> was too tired when reading up on dependencies 😬
<re_irc> In the end, it makes sense, at least for "real" serial ports: After all, you must set the baud rate and possibly several other settings. Any data received before those settings are made is probably garbage. So flushing everything after setting those values is sensible. The real issue is that you can't open the device without asserting DTR. So it's a limitation of the underlying API and not really fixable from rust.
<re_irc> <jannic> It seems like "picocom" has the same issue, just using a different systemcall.
<re_irc> With "virtual" serial ports over USB it's a little bit different: If there is no real UART involved, the serial port settings don't matter, so you may actually start receiving valid data immediately after opening the device.
<re_irc> <dngrs (spookyvision@github)> leaky abstractions, the bane of our existence
<re_irc> <dngrs (spookyvision@github)> so I guess "screen" probably just doesn't flush then?
<re_irc> <jannic> Or it's somehow fast enough.
<re_irc> <Lachlan Sneff> Any of you used the MCP9600 or MCP9601 thermocouple ICs?
<re_irc> <Lachlan Sneff> Can't seem to get anything out of the hot-junction register (i2c), but I can definitely communicate with it
<re_irc> <firefrommoonlight> So, the other registers all work for read/write?
<re_irc> <Lachlan Sneff> Seems to be the case
<re_irc> <Lachlan Sneff> I can read the cold junction register and I get some data out of it
<re_irc> <yruama_lairba> hi, i'm asking for some opinions, is it stupid to write a pac style crate for something that is not a microcontroller ?
<re_irc> <Lachlan Sneff> The cold-junction temperature that it seems to be reading doesn't make much sense though
<re_irc> <yruama_lairba> i want to do that for a codec configuration where a minimum overhead safe HAL that fit everyone use case seems very hard to write
<re_irc> <firefrommoonlight> Can you write to a config reg and read the result back?
<re_irc> <firefrommoonlight> Do you have the pull ups set up? Open drain?
<re_irc> <Lachlan Sneff> Yeah, I can read back a register correctly
<re_irc> <yruama_lairba> so instead, i'm thinking to provide a "pac style" abstraction to help people to writing their own safe hal
<re_irc> <firefrommoonlight> yruama_lairba: Can you be more specific about what you're doing?
<re_irc> <firefrommoonlight> Ie, what is "something"
<re_irc> <firefrommoonlight> Lachlan Sneff: Sounds like either something with the thermocouple and/or how it's connected, or something not configured properly on the IC's config regs
<re_irc> <firefrommoonlight> I would try plugging the IC into a raspberry Pi etc and see if you can get it working there
<re_irc> <firefrommoonlight> To narrow down if it's the thermocouple or something else
<re_irc> <firefrommoonlight> eg using this https://docs.circuitpython.org/projects/mcp9600/en/latest/
<re_irc> <Lachlan Sneff> I can't exactly plug it into another device
<re_irc> <Lachlan Sneff> I'm able to read the temperature of the IC itself correctly now
<re_irc> <yruama_lairba> firefrommoonlight: since setup is done by writing some register, i'd like to imitate package access crate of microcontrollers, that is "periph.reg.write(|w| .);" where my "periph" is in fact an object representing configuration interface of my codec.
<re_irc> <yruama_lairba> * ...);"
<re_irc> <yruama_lairba> i even wonder if i can write a svd file for that :p
<re_irc> <yruama_lairba> *i mean use a svd file
<re_irc> <yruama_lairba> no comments ?
<re_irc> <Lachlan Sneff> Ah, it turns out that some of the resistor values are wrong and the IC thinks that no thermocouple is connected
<re_irc> <Lachlan Sneff> I may have to convert the voltage to temperature manually
<re_irc> <Lachlan Sneff> Speaking of, the MCU doesn't have an FPU, so does it makes sense to use softfloat or a fixed point library?
<re_irc> <Lachlan Sneff> Ah, it works now
<re_irc> <9names (@9names:matrix.org)> yruama_lairba: A lot of complex driver crates do end up with something like that internally anyway. I think that would be fine to expose as an API for end users.
<re_irc> <yruama_lairba> 9names: thnaks for feeback
<re_irc> <yruama_lairba> in fact, i already writted somthing very similar, but i wanted to use tystate typing to implment safety, wich would reduce usability if implement fully :p
<re_irc> <firefrommoonlight> Lachlan Sneff: It depends. If there are no performance concerns (Not maxing out capability of MCU, doesn't use battery power etc), software floats may be easier to work into your code. Otherwise, go fixed... that will be fine too, and most of these sensors return fixed point data anyway, and don't have near the precision where you'd need a float
<re_irc> <firefrommoonlight> So, doesn't really matter, but floats are easier to work with
<re_irc> <Lachlan Sneff> I ended up with fixed point. It’s too bad the fixed crate doesn’t use const generics yet
<re_irc> <Lachlan Sneff> Thank you for the advice