<re_irc>
<adamgreig> cr1901: hm, I haven't had time yet to look in much detail, but enabling the inline-asm feature on cortex-m crate in your critical crate reduces the diff to 4 bytes; it's enabled by default in upcoming cortex-m and now works on stable rustc in current cortex-m, stops the cpsid/cpsie functions being outlined
<re_irc>
<adamgreig> and then the "critical/inline" feature makes no difference either
<cr1901>
Confirmed on my end. Interesting...
<re_irc>
<adamgreig> the 4 byte diff doesn't make a ton of sense to me, the generated assembly is quite different (but i've not read the actual source yet)
<cr1901>
critical/inline does make a difference for me _if use-extern-cs feature isn't used_
<cr1901>
380 bytes with "critical/cortex-m,critical/inline,cortex-m/inline-asm"
<cr1901>
384 bytes with "critical/cortex-m,cortex-m/inline-asm"
<re_irc>
<adamgreig> hmmm i get 380 with critical/cortex-m,cortex-m/inline-asm :/
<re_irc>
<adamgreig> slightly old nightly, i'll try updating it
<cr1901>
adamgreig: Paste exact command line please?
<re_irc>
<adamgreig> git s
<cr1901>
ghost in the shell?
<re_irc>
<adamgreig> so rustc nightly 2022-08-28, unmodified clone of your repo, cargo +nightly build --manifest-path=./take-api/Cargo
<cr1901>
adamgreig: I suppose "that's what I get" for simplifying the actual commands I've been running to avoid sending you down the path of my madness
<re_irc>
<adamgreig> :D
<cr1901>
I do everything via the Justfile provided w/ my repo
<cr1901>
I don't actually type that shit out
<re_irc>
<adamgreig> phew lol, I was trying to use cargo-size but it doesn't even know about --manifest-path 🙃
<cr1901>
I probably shouldn't be using a workspace, but... this repo is a few years old now
<cr1901>
So inline-asm makes the problem go away. Hmmm...
<cr1901>
adamgreig: If you have just installed: "just build-example critical $FEATURES thumbv6m-none-eabi"
<cr1901>
where $FEATURES is e.g. "critical/cortex-m,critical/inline,cortex-m/inline-asm"
<re_irc>
<adamgreig> I don't have it... yet. bedtime here now though, sorry, i'll try and get just installed and keep looking tomorrow
<re_irc>
<adamgreig> though if it's down to a 4 byte difference i suspect it's "compilers being weird" and not so clear if it will cause a meaningful size different for applications? might need to try a bigger example to compare
<re_irc>
<adamgreig> might be worth seeing what opt-level=3 does differently too
<cr1901>
This is a minimized example. In real code, every critical section has the size penalty. Trying to figure out if it's inherent to rustc or a "bad LLVM being emitted" problem
<cr1901>
I'll keep poking around
limpkin has quit [Ping timeout: 268 seconds]
limpkin has joined #rust-embedded
starblue has quit [Ping timeout: 252 seconds]
starblue has joined #rust-embedded
<re_irc>
<762spr> I asked this over in the probe-rs group but I'm not sure if that is the right place to ask? If here isn't good either, let me know where would be best... I am having a problem with my defmt formatting. It is printing as two lines and makes diagnosing these sensor readings very difficult as I need to print them real time (I probably could have had the whole driver written in the time I have been trying to sort out this...
<re_irc>
... defmt printing...) I went blank slate and started with the knurling-rs app template. after everything is set up for my STM32 L4, I compile, flash and get: Hello, world! └─ hello::__cortex_m_rt_main @ src\bin\hello.rs:8 instead of: 0.000000 INFO Hello, world! I also can't get any of the other defmt prints to work like info! I have tried in VSCodium and cmd but both yield the same results. What have I done wrong? Thanks!
thomas25 has quit [Killed (NickServ (GHOST command used by Guest21!~Guest21@31.223.63.23))]
thomas25- has joined #rust-embedded
<re_irc>
<762spr> ok I think I finally got it. in cargo.toml [profile.dev] needs debug set to less than 2 to remove the second line. I have no idea what these values equate to and will have to investigate further... for the template app, the debug was set to 2 automatically, so I'm a little salty the default configuration didn't yield results the same as the expected output on the website. Also, it doesn't print the leading number. I don't...
<re_irc>
... want that, but it is a discrepancy as to what the website suggests should be printed. I think that's the timestamp?
<re_irc>
<762spr> secondly in config.toml I needed to add the [env] field FEFMT_LOG = "trace", and then do a clean build for the change to take effect. This again is completely different from how the ferrus systems documentation suggests changing the log level by placing DEFMT_LOG=level before cargo run (which will error). Is this because I am running on windows or because things have changes since the docs were written? This kind of stuff...
<re_irc>
... makes it extremely hard for a beginner to get going because it is kind of misleading when I am trying the exact stuff that the docs say and it flat out errors. Sorry for polluting the channel but I'm trying my best to learn and not sure where else to get help...
<re_irc>
<762spr> **DEFMT_LOG, not FEFMT_LOG 😅
<re_irc>
<jannic> Yes, that's just a Unix/Windows difference. Not much the docs could do about it, short of providing the examples twice, for both operating systems.
<re_irc>
<jannic> Perhaps add a footnote to explain the difference without bloating the main contents?
crabbedhaloablut has quit [Ping timeout: 258 seconds]
crabbedhaloablut has joined #rust-embedded
ni has quit [Quit: WeeChat 3.0]
ni has joined #rust-embedded
<re_irc>
<762spr> good to know. yeah at least something indicating that it should be different for those of us stuck working on the cursed operating system would be nice. At least so dummys/noobs like myself can figure it out a little easier!
cr1901_ has joined #rust-embedded
cr1901 has quit [Ping timeout: 244 seconds]
edm_ is now known as edm
starblue has quit [Ping timeout: 268 seconds]
starblue has joined #rust-embedded
<re_irc>
<chemicstry> 762spr: If you have installed git for windows, you can use git bash terminal instead, much better. It also comes with a lot of unix command line tools
causal has quit [Quit: WeeChat 3.6]
dc740 has joined #rust-embedded
crabbedhaloablut has quit [Remote host closed the connection]
crabbedhaloablut has joined #rust-embedded
dc740 has quit [Ping timeout: 248 seconds]
dc740 has joined #rust-embedded
dc740 has quit [Read error: Connection reset by peer]
dc740 has joined #rust-embedded
<re_irc>
<maix522> How do I set the asm mode with the "asm!()" macro ?
<re_irc>
I am on a thumbv4t cpu (ARM7TDMI) and I want to use the "MRS" instruction but it errors out saying that I could fix this error by being in "thumb2" mode or "arm-mode".
<re_irc>
Obviously I want to be in arm-mode but I don't know how to set this
dc740 has quit [Ping timeout: 255 seconds]
cr1901_ is now known as cr1901
starblue has quit [Ping timeout: 268 seconds]
starblue has joined #rust-embedded
GenTooMan has quit [Ping timeout: 244 seconds]
GenTooMan has joined #rust-embedded
<re_irc>
<adamgreig> hi room, meeting time again! agenda is https://hackmd.io/xzKkNLnaRY6h3EdR4dewbw, please add anything you'd like to announce/discuss and we'll kick off in about 5min
<re_irc>
<thejpster> I’ll pass today. I’m still waiting to be deplaned in Norway.
<re_irc>
<thejpster> (Say hi if you’re at NDC TechTown this week and come see me y’all about Neotron and old computers again)
<cr1901>
I won't be long re: the msp430/cortex-m bloat, but I have "more news" :)
<re_irc>
<adamgreig> cool, well let's start, only quick announcement from me is that the friday showcase thing happened on twitter, thanks James Munns and everyone who submitted entries! looks like it got a good reception
<re_irc>
<James Munns> Thanks all for submitting!
<re_irc>
<James Munns> I set a calendar event for next month :)
<re_irc>
<adamgreig> thanks for the announcements :)
<re_irc>
<adamgreig> ok, first up, cr1901 do you wanna update?
<cr1901>
So, I've wrote an example crate where both msp430 and cortex-m has the same codegen for critical sections where interrupt enable code is unnecessarily duplicated across both sides of a branch. So cortex-m has the problem too. 1/2
<cr1901>
However, for cortex-m _when the inline-asm feature is enabled (default for 0.8)_, the codegen size difference is minimal, even when the interrupt enable code is duplicated across branches
<cr1901>
So, I guess next step is to figure out why msp430's codegen sucks in the presence of duplicated interrupt enable code
<re_irc>
<dirbaio> is this with LTO=fat?
<cr1901>
yes
<cr1901>
it has to do with "extern Rust release()"... when the extern qualifier is there, even w/ LTO
<re_irc>
<adamgreig> so the problem is that for every CS, the "release()" code ends up called/inlined after the end of each branch?
<re_irc>
<dirbaio> i see... I have a vague feeling something's wrong with the inlining heursitics with "extern fn"s
<cr1901>
Rust decides to create two copies of release() if the critical section has a branch
<re_irc>
<dirbaio> I saw a similar thing while trying to reduce defmt's code size (defmt uses "extern fn"s too)
<cr1901>
(one for each side of the of the branch)
<re_irc>
<adamgreig> if it's not allowed to inline release, does it insert two calls, or does it combine the control flow instead?
<re_irc>
<dirbaio> it's like it assumes the extern fn's "cost" is zero or something, so it LOVES to inline them EVERYWHERE
<cr1901>
#[inline(never)] creates two calls
<cr1901>
#[inline] or #[inline(always)] has no effect on "extern Rust"
<re_irc>
<dirbaio> would be interesting to try to repro it without the "extern fn" (having everything in the same crate)
<cr1901>
dirbaio: Without extern fn, _on msp430 only_ (I did not check ARM yet) 1/2
<cr1901>
If I don't include #[inline] or #[inline(always)], ironically enough, I can duplicate the release code being copied twice for the branch
<re_irc>
<dirbaio> ah huh, how odd
<cr1901>
if I include #[inline] or #[inline(always)], the release code is only generated once (after both sides of the branch have converged)
<re_irc>
<dirbaio> so perhaps it's not (just) the inliner then
<re_irc>
<adamgreig> wonder why it appears to be worse for msp430 than cortex-m, too
<cr1901>
That's what I need to find out next
<re_irc>
<dirbaio> maybe the inline asm throws it off?
<cr1901>
It might not be a big deal for many applications, but the overhead of calling a critical section adds up
<cr1901>
The inline-asm feature makes the code diff between "extern Rust" and "not extern Rust" minimal for cortex-m
<cr1901>
_however_, the duplicated release code is still there
<re_irc>
<adamgreig> hmm, is it?
<cr1901>
I have some snippets :)
<re_irc>
<adamgreig> I didn't see duplicated release on cortex-m, just slightly different codegen
<re_irc>
<adamgreig> on your sandbox from last night
<cr1901>
I saw cspie twice in the objdump
<cr1901>
Well, perhaps you can redup after the meeting?
<cr1901>
I swear I saw it :)
<re_irc>
<adamgreig> sure
<cr1901>
it's like "the optimizer thinks duplicating the code is a size win, when it's neutral for cortex-m, and bad for msp430"
<re_irc>
<adamgreig> ok, well keep plugging away on that and see where you get to I guess
<re_irc>
<adamgreig> hopefully we can find a way to make it not terrible for msp430
<cr1901>
Can't go back to the old way. But I hope maybe this can culminate in an actionable bug report
<cr1901>
I don't like the idea that every critical section call has more overhead now
<re_irc>
<adamgreig> yea, ideally it _shouldn't_ have any more overhead
<re_irc>
<adamgreig> and being able to plug them rather than using each arch's hardcoded "no interrupts" is the only way to make things sound in general, so just need to work out how to get rid of the overhead
<re_irc>
<adamgreig> linux spi: basically spidev already owns a particular spi device and toggles its CS on every read()/write() call, there's no way to manually control the CS pin, but you can have spidev stop controlling CS, and you can batch up some read/writes into a single CS transaction
<re_irc>
<adamgreig> and linux doesn't guarantee no one else will use the bus in between your calls either
<re_irc>
<dirbaio> for "write, write" it could buffer everything then do one big write..
<re_irc>
<adamgreig> not if the closure does other stuff between writes like wait for an io
<re_irc>
<dirbaio> unclear how to impl flush though
<re_irc>
<adamgreig> you can change the device tree to use some random unconnected io as CS, tell spidev to not drive cs, then use normal gpio for all the real cs and implement spidevice on top of that
<re_irc>
<dirbaio> and for other combinations ("write, read" or "read, read" etc) you need cooperation from the kernel for sure 😓
<re_irc>
<dirbaio> how is linux spi api so broken? 🤣
<re_irc>
<adamgreig> but often people would want to use the existing spidev devices i assume, and you can't at runtime take gpio control of a pin that's already assigned as a spidev cs afaict
<re_irc>
<adamgreig> I think it works OK in isolation, it's just very hard to make it work with e-h 1.0 traits
<re_irc>
<adamgreig> like, you can queue up multiple reads/writes into one kernel call and it will set cs, run them all, then release cs
<re_irc>
<adamgreig> but it's sort of a pain because it sure would be nice for linux-embedded-hal to have a compliant SpiBus and SpiDevice impl that works with normal linux spi devices
<re_irc>
<adamgreig> i'm hoping someone will know a clever trick :P
<re_irc>
<adamgreig> (I was really hoping it would be OK to open the CS pin as a normal GPIO and then tell spidev to not change CS, but seemingly that's not allowed)
<re_irc>
<adamgreig> anyway, if anyone's got some experience with linux spi/spidev and has any ideas, please put them in the issue thread!
<re_irc>
<dirbaio> perhaps closest is to buffer all writes, then when a read comes do a "write all, read" transaction
<re_irc>
<dirbaio> so it'd only break if you have multiple reads, or a write after a read
<re_irc>
<dirbaio> which would be rarer
<re_irc>
<adamgreig> and you can also end the transaction on flush I guess
<re_irc>
<adamgreig> it would be better than currently at least
<re_irc>
<adamgreig> I wonder if it should have a public SpiBus type at all
<re_irc>
<adamgreig> it's a lie, if you use any SpiBus methods it'l drive the CS pin for the particular spidev device you opened, and another process or thread might have opened another spidev device on the same bus and use it at the same time as you, too
<re_irc>
<dirbaio> yeah :(
<re_irc>
<adamgreig> if you just want SpiBus to do like ws2812 or something then it would still "work"
<re_irc>
<adamgreig> so it might just need a very big disclaimer
<re_irc>
<adamgreig> or write a new linux kernel driver instead of spidev :P or I think in theory you could also write a usermode driver for the rpi chip's spi peripheral or something cursed like that, but, ugh
<re_irc>
<dirbaio> there's some interesting points in the PR discussion though
<re_irc>
<eldruin> hey
<re_irc>
<eldruin> not from me either
<re_irc>
<eldruin> there was a proposal for software_reset that I was wondering how it would fit
<re_irc>
<adamgreig> that's probably worth discussing too
<re_irc>
<adamgreig> it seems quite different to all the other trait methods, in that it just calls the other methods, and the default impl seems like all anyone will ever need
<re_irc>
<dirbaio> about software_reset: I think that's starting to get into "driver territory", not just "hal territory"
<re_irc>
<adamgreig> it's also far from universally supported among i2c devices, right?
<re_irc>
<eldruin> indeed
<re_irc>
<eldruin> I see the potential for resets of a random bunch of devices instead of just one because somebody in a driver understood it wrong
<re_irc>
<Catherine> you really have to fish rarely sold devices from catalogs if you do want a reset
<re_irc>
<Catherine> sometimes those only come on order even
<re_irc>
<Catherine> regretful, considering the propensity for i2c to lock up
<re_irc>
<adamgreig> yea, i don't think i've ever issued a 'general reset' command...
<re_irc>
<Catherine> oh, General Call? that's almost never supported outside of subsets
<re_irc>
<Catherine> SMBus/PMBus i think has it
<re_irc>
<adamgreig> general call address, data 0x06 for 'please reset'
<re_irc>
<adamgreig> yea
<re_irc>
<Catherine> but i've looked at a fairly wide variety of general purpose I2C devices and none of them supports those commands
<re_irc>
<Catherine> * supported
<re_irc>
<adamgreig> given as even when someone does want it, it's always just a one-liner "bus.write(addr, &[0x06])", it seems maybe not worth having in the standard trait
<re_irc>
<Catherine> yes
<re_irc>
<therealprof> SMBus is very special.
<re_irc>
<adamgreig> I love that pmbus/smbus did actually standardise a set of units to report things in...
<re_irc>
<adamgreig> ...and that they're completely cursed, obviously
<re_irc>
<therealprof> Maybe one day i3c will take over.
<re_irc>
<adamgreig> dirbaio, I think the conclusion is to keep embedded-can in the same repo as a new directory called "embedded-can", but I guess there were some other todos for the pr
<re_irc>
<dirbaio> naming it "embedded-can" instead of "embedded-hal-can"?
<re_irc>
<adamgreig> anything you want to discuss there?
<re_irc>
<adamgreig> yea
<re_irc>
<adamgreig> since the crate is embedded-can
<re_irc>
<dirbaio> okay
<re_irc>
<dirbaio> thoughts on this?
<re_irc>
> What should the nb, can crate versions be? 1.0? (no opinion on this from me).
<re_irc>
<dirbaio> okay, added to TODO list
<re_irc>
<adamgreig> for can i'd go with 0.4 next, since it has breaking changes from the existing embedded-can 0.3, but if it was ready for 1.0 we'd be including it in embedded-hal lol
<re_irc>
<dirbaio> 1.0 probably makes sense?
<re_irc>
<adamgreig> not sure about nb
<re_irc>
<dirbaio> +for nb
<re_irc>
<dirbaio> also we don't have to release it at the same time as embedded-hal 1.0
<re_irc>
<adamgreig> yea
<re_irc>
<eldruin> yeah last time we had some discussion about can and so on and it seemed not ready for 1.0
<re_irc>
<adamgreig> i think we want to release embedded-can 0.4 asap as a backport of the traits from embedded-hal 0.2, which are a little different to e-c 0.3
<re_irc>
<eldruin> dirbaio: exactly
<re_irc>
<adamgreig> but there are enough outstanding concerns that we probably don't want it to be 1.0 yet
<re_irc>
<dirbaio> for nb?
<re_irc>
<adamgreig> no, sorry, all e-c
<re_irc>
<eldruin> I have not looked what it was in nb. better prepare a lower number and then we can see
<re_irc>
<dirbaio> okay, so: e-h-nb 1.0, e-can 0.4?
<re_irc>
<adamgreig> I think that sounds fine but don't release e-h-nb yet, whereas do release e-can 0.4
<re_irc>
<dirbaio> nb has serial, spi
<re_irc>
<dirbaio> the only "concern" I can think of is "nb is not great", but that's not a solvable concern :D
<re_irc>
<eldruin> what about spi bus/device split for nb?
<re_irc>
<dirbaio> so "1.0" as in "it's done, we won't break it anymore", not as in "it'e great"
<re_irc>
<dirbaio> eldruin: you can kinda use it today by doing "where SPI: embedded_hal::spi::SpiDevice, SPI::Bus: embedded_hal_nb::spi::FullDuplex"
<re_irc>
<dirbaio> but the ".transaction()" itself is not non-blocking
<re_irc>
<dirbaio> and making a really "nb" version of it would suck, you can't use closures
<re_irc>
<eldruin> dirbaio: that does not work, does it? we would need to implement SpiBusWrite/Read for the fullduplex bus, which would not work because the methods are blocking
<re_irc>
<dirbaio> nb FullDuplex doesn't require SpiBusWrite/SpiBusRead