-
Notifications
You must be signed in to change notification settings - Fork 131
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
avrdude support broken #122
Comments
the same for this version: S_1-05112018.hex. |
I am sorry for the issue you are dealing with. (http://dangerousprototypes.com/forum/viewtopic.php?f=4&t=9017) |
No problem, it is a debug device, sometimes it needs some debug itself ;-) I don't know the latest working version, but as soon as possible I going to try other versions.
|
Working: |
Thank you for reporting this. |
cool, very ambitious. Call me if I can support you. |
Would it be possible to try out different code versions to see exactly which code change broke the feature? The two post-DangerousPrototypes versions you mention are two years apart, and a lot changed in the meantime. If you are able to build the firmware yourself, can you try to check out older versions in the git repository to narrow down what broke the code? |
@agatti It would take some time |
So bitbang works, but not normal operation... |
Yeah, all that preprocessor stuff make it look like real spaghetti code. Trying to beautify and accommodate for all incompatible hardware versions in the same files... I can't even be bothered to look at it. Would have been better to use separate branches for the various HW revisions. We'd need a huge block diagram of the whole code to understand how stuff is done and when/where it is executed. Reverse engineering the firmware executable with IDA would probably provide better insight how the code is executed and in what order. |
I am still waiting for the ATTINY chips I ordered on January 18th. |
Finally I received the chips so I could verify the matter with the firmware U_1-05112018.hex that I released in the forum of dangerousprototypes (http://dangerousprototypes.com/forum/viewtopic.php?f=28&t=8498&start=120#p67834). @E3V3A Attempting to initiate BusPirate bitbang binary mode... BusPirate binmode version: 1 avrdude.exe: AVR device initialized and ready to accept instructions Reading | ################################################## | 100% 1.52s avrdude.exe: Device signature = 0x1e930b (probably t85) I have also tried without GUI with avrdude-6.3-mingw32 and everything works without problems: I:\Bus Pirate\avrdude-6.3-mingw32>avrdude -c buspirate_bb -p t85 -P COM3 -b 115200 -F Attempting to initiate BusPirate bitbang binary mode... Reading | ################################################## | 100% 1.54s avrdude: Device signature = 0x1e930b (probably t85) avrdude: safemode: Fuses OK (E:FF, H:DF, L:62) avrdude done. Thank you. |
It is all the bitbanging mode |
@ coelner Ok, but is it sure it should always work both ways? |
Well, if it worked with a previous BusPirate version with buspirate_bb, but not in the latest version (with AVRDUDE unchanged), there is likely a regression on the BusPirate side. |
http://dangerousprototypes.com/forum/viewtopic.php?f=28&t=3441&start=60#p63153
S_1-28102018.hex
http://dangerousprototypes.com/forum/viewtopic.php?f=28&t=8498#p65290
@USBEprom avrdudess does nothing at all, it is only a gui for avrdude. You can see the command line which is used for avrdude. I have currently not the time to test each commit and compile the firmware and test it. |
I agree. Thanks for your report. |
@USBEprom wrote:
I am not really sure what you are trying to say, but I agree with you that it would be nice to fix the underlying issue. So you are saying that some versions of avrdude do not support both modes. Not sure, how exactly they do vary, because I am not familiar with AVR flashing, but from the output below, I would guess that buspirate_bb supports TPI, while buspirate does not. (Disclaimer: I didn't know what TPI is without googling...):
@coelner @USBEprom What I can offer is to try to flash an Atmega 328 using BPv4 (I don't have a v3) with the latest version. This is a bit of gamble, because this bug might only occur with an ATTiny or only with BPv3, but if I can reproduce this, we are already one step further. @USBEprom Are you lacking an avrdude-version that supports buspirate (without the bb)? If no, can you try to reproduce with the command-line that @coelner uses? |
sorry i don't have much to add except i ran into an issue with avrdude on attiny44 tonight, using a git checkout from 2/22/19. 4ba17bf on bp v3.6 to be precise. edit: forgot to mention that the old 6.1 firmware was much faster as well. |
bpv3_fw7.0_opt0_18092016.hex works both ways with ATMega328p (arduino due board) I going to sniff the traffic, maybe someone can use it. |
i bisected and it seems it was broken a long time ago in 47f5c59. i will need to double check when i get some more time. here is the log:
|
Does changing spi_state.clock_edge =
(inByte & 0b0010) ? SPI_TRANSITION_FROM_IDLE_TO_ACTIVE :
SPI_TRANSITION_FROM_ACTIVE_TO_IDLE; to spi_state.clock_edge =
(inByte & 0b0010) ? SPI_TRANSITION_FROM_ACTIVE_TO_IDLE :
SPI_TRANSITION_FROM_IDLE_TO_ACTIVE; in fix the issue over there? |
yes, that fixed it! thanks! i will see if i can find the performance regression as well later tonight (if it still exists with this fixed). |
Thank you Sir! BP_v3_07032019_patched_OPT1.zip For what I can verify, AVRDUDE now works with both buspirate_bb and buspirate syntaxes. Thanks for having bisected the source, Sir! Very interesting the fact that with different chips, ATMega328p in your case, the result can change, thanks for sharing the thing, Sir! Thank you for your kind help, Sir. |
with regards to the performance regression, i wasn't able to bisect it exactly because the compilation failed due to after these commits, performance went from < 5s to flash my attiny44, to 10x that - a bit more than 50s. it seems only writes are affected. so, one of these - the first that compiles is cd6559d, but i have a feeling it was introduced in 8df3c33.
bisect log:
non-regressed avrdude:
regressed avrdude:
to be clear, this patch was applied while bisecting the performance regression:
|
So, I've taken a look at the offending commits. I'd be surprised if cd6559d is actually slowing things down tenfold, but uart i/o should take a non-trivial amount of time for these sort of operations. 0f2a533 contains no code changes at all and can be safely ignored. 8df3c33 should perform the same set of operations as before, but it is organised differently - the new code is not enabled by default, too. Unless I've messed up badly in 8df3c33, can you please try with 0f2a533 and 8df3c33 still in and manually revert cd6559d to see if things actually get as fast as before? |
There is a possibility that interleaving the user_serial_transmit_character() and spi_write_byte() operations leads to poorer USB utilization, ie. fewer bytes transmitted per packet. This should be visible in a packet capture. |
S_1-05112018_attiny85_sniff.zip My sniffer max rate is 24MHz, I hope it was fast enough. The mapping for the pulseview file. |
@andersm SPI streaming is disabled by default, though - if it was left alone then it's either the new uart u32 reading code or I simply messed up when rearranging code for the spi read/write operations I guess. |
i'm not quite understanding what you mean by manually revert cd6559d. i can't move back to a state where so, i interpreted your request as to test from tip of current master and reverted like so:
that didn't fix it, though. if you had something else in mind let me know, thanks. edit: fwiw i defined |
Yep, that was what I had in mind. It's weird that speed went down like that, since the changes weren't supposed to impact on speed, of all things. Streaming read/writes were supposed to allow transferring up to 64k per session (since the transfer length is 16 bits) instead of being limited by the buffer size set at compile time - which is definitely less than 64k anyway. |
For what I can measure it seems to me that the syntaxes buspirate is roughly 17 times faster than the buspirate_bb one. |
My logic analyzer manages a different format from that used by @coelner, but I can provide specific data if it is indicated to me exactly what to acquire With the buspirate syntax the duration of each single byte is about ~0.2395ms, instead with the syntax buspirate_bb every single byte takes about ~0.127s, where the ratio between the two values is about ~532. If I can help you in something else, you just have to tell me, thanks. |
yeah, i can get you a pulseview capture of the spi and a usbmon (pcap) of the usb traffic. hopefully both timestamped so you can correlate the two although i've never tried doing so. |
@agatti please see attached pulseview+usbmon pcaps. seems like lots big pauses between writes with the regressed version. i was able to use the SPI decoder to view the bytes that corresponded to the .hex i was flashing, so it seems the captures are good. thanks! |
I don't know if it's relevant, but it seems to me like 8df3c33 introduced one change in behaviour: if the host only wants to write, ie. It should be easy to test if this has any effect. Move lines 985-992 of spi.c to 1045 (below releasing CS), and the old behaviour should be restored. Obviously streaming reads must be disabled. |
adding Lines 1023 to 1025 in 8df3c33
to:
if you had something else in mind and provide a patch i'd be happy to test it. |
I meant something like this:
At a quick glance, it looked like there were places in the Bus Pirate support code of AVRDUDE where missing success responses could cause repeated host serial port read timeouts without actually failing the operation, but the default read timeout looked to be 100ms, not the 30ms delay shown in the packet traces. |
the patch didn't seem to make a difference :/ |
if someone wanted one of the attiny boards i'm experiencing the issue on to debug, i could mail one to you. they are soil moisture sensors (older version of this: https://www.tindie.com/products/miceuz/i2c-soil-moisture-sensor/). some have corroded at the bottom making them useless for soil measurement but the attiny still works OK. they have a 6 pin avr isp header. |
revisiting this after running into further issues with an atmega32u4. newer avrdude helpfully outputs this message:
looking back at #122 (comment), i noticed that the avrdude test with non-regressed firmware output the thinking it might have to do with this section of avrdude: https://github.com/manison/avrdude/blob/a87a2c4eefe77ebc522b0a7a4c5ff83c09e3ab3e/buspirate.c#L582-L607 perhaps the lack of paged write explains the performance regression? |
as mentioned: With this firmware (U_1-05112018.hex) I can't use avrdude to flash a attiny85. With a usbasp it works well, therefore the attiny seems working and the wiring should be fine.
VCC <-> 5V
GND <-> GND
PB5 <-> CS (RES)
PB2 <-> CLK
PB1 <-> MISO
PB0 <-> MOSI
I am aware of this: http://dangerousprototypes.com/docs/Bus_Pirate_AVR_Programming#initialization_failed.2C_rc.3D-2
The wires are working, I used them with the usbasp. I apply 5V from buspirate or directly via PSU. The timing of buspirate should be the lowest as possible.
Any other suggestions?
The text was updated successfully, but these errors were encountered: