BusPirate / Bus_Pirate

Community driven firmware and hardware for Bus Pirate version 3 and 4
619 stars 133 forks source link

bp_delay_us is not accurate #23

Open agatti opened 7 years ago

agatti commented 7 years ago

A more precise way to perform delays should be implemented as this might mess with timing-sensitive protocols such as 1-Wire. Unfortunately I bricked my v4 and I'm away for a week so I can't really work on this at the moment, but before the bricking I made 1-Wire work more reliably by changing the amount of time being waited, and my logic analyser all of a sudden would detect the protocol state changes.

Something that on paper should be more accurate would be this (assuming the number of milliseconds is in W0 and that it's running on a v4 board, which means it can execute 16 instructions each microsecond):

  sub #1, w0    /*  1 / 16 */ /* Compensate for first batch of NOPs */
  nop           /*  2 / 16 */
  nop           /*  3 / 16 */
  nop           /*  4 / 16 */
  nop           /*  5 / 16 */
  nop           /*  6 / 16 */
  nop           /*  7 / 16 */
  nop           /*  8 / 16 */
  nop           /*  9 / 16 */
  nop           /* 10 / 16 */
  nop           /* 11 / 16 */
  nop           /* 12 / 16 */
  nop           /* 13 / 16 */
  nop           /* 14 / 16 */
  nop           /* 15 / 16 */
  nop           /* 16 / 16 */

.loop: 

  nop           /*  1 / 16 */
  nop           /*  2 / 16 */
  nop           /*  3 / 16 */
  nop           /*  4 / 16 */
  nop           /*  5 / 16 */
  nop           /*  6 / 16 */
  nop           /*  7 / 16 */
  nop           /*  8 / 16 */
  nop           /*  9 / 16 */
  nop           /* 10 / 16 */
  nop           /* 11 / 16 */
  nop           /* 12 / 16 */
  nop           /* 13 / 16 */

  sub #1, w0    /* 14 / 16 */
  bra z, .end   /* 15 / 16 */
  bra .loop     /* 16 / 16 */

.end:

  nop           /* 16 / 16 */ /* Align to 16 cycles for when W0 is 0 */

However I can't really try this out at the moment, if somebody with a logic analyser or a PIC simulator or a PicKit and a spare board wants to get wild with this, that'd be awesome!

agatti commented 7 years ago

Oh, forgot to mention that for this to work interrupts need to be disabled beforehand and restored once finished (which is not much of an issue as the board is stuck in a busy loop when this code is ran).

agatti commented 7 years ago

Fixed with 4685d15bd79e20e000c10723f2f57882b17ad90e.

USBEprom commented 7 years ago

Sorry to bother. I know I am babo as hell and more serious this issue is closed, I wonder how it works though, because it seems to me that now I2C does not honor the target speed choosed into the menu. No matter the speed that is chosen (~5kHz, ~50kHz, ~100kHz, ~400kHz), then the responses that are shown in the terminal seems to be slow, much slower than setting for ~5kHz using previous versions of the firmware where the issue #23 had not been fixed yet. With the previous versions of the firmware by choosing for different speed even the refresh of the characters on the terminal are different, instead now with the #23 fixed it is not so any more. I just built a new firmware for the BP revision 3 starting from the current repository and I noticed this. Is there is a way to ensure that is the correct behavior? Thanks. I know this is already been closed, so I apologize for the hijacking, but in my opinion something is weird there. Thank you.

News December 9, 2016:

I have digged further and by applying the fix #23 then I2CEEPROMWIN (source code here: http://web.archive.org/web/20150927140926/http://the-bus-pirate.googlecode.com/svn/trunk/scripts/I2CEEPROMWIN.c) does not work any more, it read wrong (all values are 0x01 does not matter what).

agatti commented 7 years ago

The changes applied for #23 were supposed to fix 1-Wire, where the delays are grossly miscalculated and causing the protocol to simply operate out of spec. I wonder why I2C stops working since the two calls to bp_delay_us in i2c.c are commented out...

Would it be possible to get some logic analyser dumps / screenshots / etc. for before and after https://github.com/BusPirate/Bus_Pirate/commit/4685d15bd79e20e000c10723f2f57882b17ad90e got applied? The problem may be somewhere else, but I'll see if there was something obvious I missed that triggered this.

Thanks for reporting!

USBEprom commented 7 years ago

@agatti

Sorry for the late reply. Sadly I know nothing about why I2C stops working while the patch #23 is applied. However the usual friend of mine did some shots that I have not any analyzer or oscilloscope. Based on what is possible to see the new firmware built starting from the current repository where patch #23 is applied it works with a reduced speed clock. By setting I2C for ~50kHz into the menu of the Bus Pirate the actual speed used is about ~730Hz, max ~1kHz, depending it if in setup or read/write over the I2C protocol, while with the previous release which has not the patch #23 applied the clock speed for the same ~50kHz setting is actually ~31kHz, max ~35kHz, still it depending if in setup or read/write over the I2C protocol. New speeds are lower than before, this is for sure. For instance has been read the full contents of a memory IC over a I2C bus using the ~50kHz speed and the entire reading was acquired with a logic analyzer set for 4MHz sampling and 1 second lasting. The logs of the two readings are here as attachment. It is possible to see that using the new firmware incorporating the patch #23 into 1 second of acquiring only 111 data bytes are logged, while using the previous release in the same conditions all the 2048 data bytes are collected within 1 second which is largely enough to complete the job. In the end into the firmware which has the patch #23 despite the various settings for the speed, ~5kHz, ~50kHz, ~100kHz and ~400kHz, the actual speed provided by the Bus Pirate is between ~635Hz and ~1kHz does not matter what (~635Hz by setting for ~5kHz into the menu of the Bus Pirate, for the remaining values, ~50kHz, ~100kHz and ~400kHz, it is ~1kHz for them all) rather far from specifications, lower than projected.

log i2c

PATCH #23 read.txt

NO PATCH #23 read.txt

agatti commented 7 years ago

@USBEprom Can you please to try changing the value of FCY in base.h to see if things improve? I'm not sure if v3 runs at the same frequency as v4.

Is v3 supposed to run at 16MHz too?

USBEprom commented 7 years ago

@agatti

Unfortunately inside of base.h I can not find the word FCY, where it should be? I know I am pretty babo, so sorry please. However based on this:

http://dangerousprototypes.com/docs/index.php?title=BPv3_FTDI_UART_demo&oldid=16133

It is stated that "The Bus Pirate uses an 8 MHz internal clock multiplied by a 4x PLL to produce a 32 MHz CPU clock" and when I have calculated the prescalers for the new speeds into the menu of SPI I have used the 16MHz value. It is likely that FCY is 16MHz also for the Bus Pirate revision 3. Here:

http://dangerousprototypes.com/forum/viewtopic.php?f=40&t=3864&start=15#p41505

it is said "The 16MHz SPI speed mode is not valid, the max SPI peripheral speed is 8MHz (10MHz slave)", ignoring the context (SPI) seems that also for the Bus Pirate v3 FCY is really 16MHz. Here:

http://dangerousprototypes.com/forum/viewtopic.php?f=4&t=2262&p=22362#p22362

it is said "the bus pirate is running at Fosc 32MHz so Fcy becomes 16Mhz". At this point I guess that 16MHz is exact as FCY also for the Bus Pirate revision 3. Soon when I will understand where is the parameter FCY I will try to do some changes on it and then I will post the results. See you later and thanks for your reply sir!

agatti commented 7 years ago

@USBEprom FCY is here: https://github.com/BusPirate/Bus_Pirate/blob/master/Firmware/base.h#L24

Now, the only difference I can see that might impact delays would be linking libpic30 together with the project. I'll see if I can just extract the delay function alone and see if that helps.

USBEprom commented 7 years ago

@agatti

OK, still you won while I lose being a total babo. Of course you are right the parameter FCY is inside base.h as you have written, in the current repository though! Yesterday I was looking for it inside the previous release of the Bus_Pirate-master, not the last one, so it is quite normal that I did not find it! However today using the current repository I have built a test firmware for my Bus Pirate revision 3 where line 24 has been changed as "#define FCY 8000000UL". It works exactly like the same firmware made leaving unaltered the content of the line number 24 inside the base.h (#define FCY 16000000UL). The test firmware behaves just the same as the current released one, no difference. The usual friend of mine took a couple of shots, here you go:

fcy

As it is possible to see the clock speed is still low compared the expected. Is there nothing else I can do to help out? Thank you sir.

agatti commented 7 years ago

@USBEprom can you please try to build this version and see if things change? The only difference is that libpic30 is not linked in anymore, but its delay function has been merged in anyway - as it allows for 1-Wire to function reliably.

fw_no_libpic30.zip

USBEprom commented 7 years ago

@agatti

Unfortunately nothing happens. I have just built a new test firmware starting from your fw_no_libpic30.zip but the speed of the clock for the I2C protocol is still wrong, too low compared to what is expected. As your reference here you go a couple of shots:

fw_no_libpic30

Thank you very much for your support sir.

agatti commented 7 years ago

@USBEprom I think I found the problem... Can you please give this version a try? It theoretically should fix the extra delay issue on I2C, but I cannot really test things at the moment.

firmware-delay-fix.zip

USBEprom commented 7 years ago

@agatti

Before all merry Christmas to you and thanks a lot for your hard work. Unfortunately nothing happened. I have just built a new test firmware starting from your firmware-delay-fix.zip but the speed of the clock for the I2C protocol is still wrong, too low compared to what is expected. As your reference here you go a couple of shots:

firmware-delay-fix

I am really sorry that you alone have to take all this trouble also spending your holidays by working, as I understand that fixing the problem is really very, very hard. Basically you alone are doing the work of hundreds of thousands of people. Thank you sir.

agatti commented 7 years ago

@USBEprom thanks for trying that out. I'll do some more test on my end with some sample circuits (I only have SPI devices to test with at the moment) and hopefully see what can be done. However we've been delaying v7.1 for quite some time now, so there might be chances that we will release said version with that I2C issue in. Still, we'll look into this and try our best to fix this issue, hopefully we'll be able to sort it out in time.

USBEprom commented 7 years ago

@agatti

I agree with you. In the end the firmware v7.0 was conceived basically for the Bus Pirate revision 4, good for its possible compatibility with previous revision 3. I deal either with SPI and I2C so until the issue will not fixed I will still use the firmware that I built myself based on the previous repository which it has not the patch #23:

dangerousprototypes.com/forum/download/file.php?id=12196

Thank you very much sir.

agatti commented 7 years ago

@USBEprom which mode did you use for I2C? Hardware or software?

For software I2C I do get ~700Hz on SCL:

sw_i2c

For hardware I2C I get ~500kHz on SCL:

hw_i2c

Both of those using the maximum speed available.

agatti commented 7 years ago

@USBEprom I tried with the very latest version, just rebuilt and installed and I get ~100kHz in software mode:

master_sw_i2c

I have the feeling that I hit a v3-only issue I don't have a board to test with...

agatti commented 7 years ago

@USBEprom Can you please try this firmware, just to be sure? This is the last version that does not have the bug #23 fix. On a v4, in software mode I have around 111kHz on SCL, which is slower than what I get after I applied the fix.

Please try this one in software and hardware modes if possible, thank you!

fw_before_patch23.zip

agatti commented 7 years ago

I just tried the last version from Dangerous Prototypes (the one from here: https://github.com/DangerousPrototypes/Bus_Pirate) and I have the exact same bus speeds in software mode. Maybe I should rename the 400kHz entry into "Maximum speed" if software mode is chosen on a v4...

USBEprom commented 7 years ago

Sorry sir. Not without a lot of shame I have to admit I did not understand the question. Simply the usual friend of mine have read an I2C memory chip using a command like this {0xa0 0x00}{0xa1 r:256} by acquiring the data bus while the process. I know nothing if in that context I2C is hardware or firmware, a friend of mine too. I guess it is hardware, but I am not sure. In my opinion it is software I2C while sniffing or something like that. In your shots that you have posted the logic analyzer was in free run due nothing valid trigger enabled. Could you explain better so that I can test the matter in the right way? Apologize, I am really much babo. Meanwhile I make the new firmware for test starting from your fw_before_patch23.zip. Thank you very much for your support and patience sir.

agatti commented 7 years ago

Basically, when you enter I2C mode (on v4 at least) you get asked the mode and the speed:

I2C mode:
 1. Software
 2. Hardware

(1)>

and

Set speed:
 1. ~5KHz
 2. ~50KHz
 3. ~100KHz
 4. ~400KHz

(1)>

So, my test setup at the moment is fairly simple, I have the BP4 connected to an Arduino Nano that has its internal pullups enabled (I2C needs weaker resistors than the internal ones, but they are enough for the logic analyser), and the analyser connected to those pins as well (I'm not at the lab at the moment so I don't have resistors for external pullups here). Then what I do is simply fire up the BP4, enter I2C, select software or hardware mode, choose the fastest speed and then [0xFF:100] just to see how fast SCL can go.

Now, I tried your command {0xa0 0x00}{0xa1 r:256} and I still get the same SCL speeds in software mode using the Dangerous Prototypes' firmware so I am not sure at the moment on what's going on. Can you please post a schematic on how the circuit should look like so I can order parts and see what's going on here?

USBEprom commented 7 years ago

OK sir, I get it now. Before releasing the test results I have to clarify something. The part

I2C mode:

  1. Software
  2. Hardware

(1)>

it is not present with Bus Pirate revision 3. Using Bus Pirate revision 3 there is no way to choose Hardware rather than Software mode. Perhaps the fault is due this. I do not know. Do you know what are the differences between Hardware and Software mode? However I have just built a new test firmware starting from your fw_before_patch23.zip and with it the speed of the clock for the I2C protocol is correct, all it is just as expected. Exactly as supposed before the application of the patch #23 the problem does not exist. As your reference here you go a couple of shots where I have also added the schematic diagram that you have asked for and the acquiring while execution of the command [0xFF:100] that you wrote:

fw_before_patch23 diagram clk

Thank you sir.

agatti commented 7 years ago

Ok, I really need to get a BPv3 and a 24LC16 EEPROM then.

No matter how hard I try I still get ~110kHz I2C speed. My setup is as such:

screen shot 2016-12-29 at 22 30 34

using this Arduino sketch: https://gist.github.com/agatti/d8bf74440e0995b9b297e505c3583e15

and this command sequence:

HiZ>m
1. HiZ
2. 1-WIRE
3. UART
4. I2C
5. SPI
6. 2WIRE
7. 3WIRE
8. KEYB
9. LCD
10. PIC
11. DIO
x. exit(without change)

(1)>4
I2C mode:
 1. Software
 2. Hardware

(1)>1
Set speed:
 1. ~5KHz
 2. ~50KHz
 3. ~100KHz
 4. ~400KHz

(1)>4
Clutch disengaged!!!
To finish setup, start up the power supplies with command 'W'

Ready
I2C>W
POWER SUPPLIES ON
Clutch engaged!!!
I2C>e
Select Vpu (Pullup) Source:
 1) External (or None)
 2) Onboard 3.3v
 3) Onboard 5.0v

(1)>3
5V on-board pullup voltage enabled
I2C>P
Pull-up resistors ON
I2C>(1)
Searching I2C address space. Found devices at:
0xA0(0x50 W) 0xA1(0x50 R) 0xA3(0x51 R) 

I2C>{0xA0 0xFF}{0xA1 r r}
I2C START BIT
WRITE: 0xA0 ACK 
WRITE: 0xFF ACK 
I2C STOP BIT
I2C START BIT
WRITE: 0xA1 ACK 
READ: 0xFF 
READ:  ACK 0xAB 
NACK
I2C STOP BIT
I2C>

I get this on the logic analyser:

screen shot 2016-12-29 at 22 36 46 screen shot 2016-12-29 at 22 37 03 screen shot 2016-12-29 at 22 37 34 screen shot 2016-12-29 at 22 38 25

Am I missing something here? I'm going to see if I can find a spare I2C EEPROM this weekend and see if there's anything new - only SPI and 1-Wire devices here at the moment unfortunately...

USBEprom commented 7 years ago

@agatti

Thank you sir. Unfortunately I know nothing. I am really sorry that you have to take all this trouble. I can not help but give the configuration of the my setup. Maybe it could be of help. In this occasion the speed for the I2C clock that I have used is ~400kHz, all the previous times it was ~50kHz. Here you go:

m

  1. HiZ
  2. 1-WIRE
  3. UART
  4. I2C
  5. SPI
  6. 2WIRE
  7. 3WIRE
  8. KEYB
  9. LCD
  10. PIC
  11. DIO x. exit(without change)

(1)>4 Set speed:

  1. ~5KHz
  2. ~50KHz
  3. ~100KHz
  4. ~400KHz

(1)>4 Clutch disengaged!!! To finish setup, start up the power supplies with command 'W'

Ready I2C>P Pull-up resistors ON Warning: no voltage on Vpullup pin I2C>W POWER SUPPLIES ON Clutch engaged!!! I2C>{0xA0 0xFF}{0xA1 r r} I2C START BIT WRITE: 0xA0 ACK WRITE: 0xFF ACK I2C STOP BIT I2C START BIT WRITE: 0xA1 ACK READ: 0xFF READ: ACK 0x00 NACK I2C STOP BIT I2C>

400khz

The Bus Pirate is revision 3, the I2C chip is a 24lc16 eeprom.

I am babo as hell, I know. However as simple curiosity, what happens by using a Bus Pirate revision 4 with firmware release that does not contain patch #23? Is the clock speed the same in both cases or there are differences which are not negligible? Thanks.

agatti commented 7 years ago

@USBEprom I haven't tried that yet as I don't have the board with me at the moment. However, I added the ability to use the hardware I2C interface on v3 boards. Keep in mind that this is experimental and that may mess up with your hardware. If you want to try that out, please download the latest source code and make sure that BP_I2C_USE_HW_BUS is defined in your configuration. When you enter I2C mode you will be asked which interface to use, just like the v4.

Again, I cannot stress enough the fact that it is currently experimental. I checked with the PIC24FJ64GA004 datasheet and with the v3 schematic at http://dangerousprototypes.com/docs/images/4/4d/Cct-BusPirate-v3.5.png and it should theoretically work. I assume no responsibilities if that accidentally fries your board or anything...

USBEprom commented 7 years ago

Thank you sir. As reference I have borrowed an Arduino from the usual friend of mine who had loaded into it your sketch:

https://gist.github.com/agatti/d8bf74440e0995b9b297e505c3583e15

He also lent me his logic analyzer. By using that stuff I have performed some tests almost in the same way you did. I says almost because the Arduino is not the NANO like your. I think this is not a problem, at least I hope. The fact is that with this different Arduino, which to be exact it is the MEGA model, using your sketch I have a different behavior based on the speed of the clock I choose for the I2C bus. The Bus Pirate is a revision 3 on which is loaded the last firmware v7.0 which has not the patch #23 which therefore it does not have the problems that are being discussed. However here you go some shots documenting the matter.

cb036676-ce16-11e6-88d1-719459ee8eb9

This is the report performed at ~400kHz in accordance with your settings. How it is possible to see the answer of macro (1) is 0xA0(0x50 W).


~400kHz

Bus Pirate v3.5 Community Firmware v7.0 - goo.gl/gCzQnW [HiZ 1-WIRE UART I2C SPI 2WIRE 3WIRE KEYB LCD PIC DIO] Bootloader v4.4 DEVID:0x0447 REVID:0x3046 (24FJ64GA002 B8) http://dangerousprototypes.com HiZ>m

  1. HiZ
  2. 1-WIRE
  3. UART
  4. I2C
  5. SPI
  6. 2WIRE
  7. 3WIRE
  8. KEYB
  9. LCD
  10. PIC
  11. DIO x. exit(without change)

(1)>4 Set speed:

  1. ~5KHz
  2. ~50KHz
  3. ~100KHz
  4. ~400KHz

(1)>4 Clutch disengaged!!! To finish setup, start up the power supplies with command 'W'

Ready I2C>W POWER SUPPLIES ON Clutch engaged!!! I2C>P Pull-up resistors ON I2C>(1) Searching I2C address space. Found devices at: 0xA0(0x50 W)

I2C>(1) Searching I2C address space. Found devices at: 0xA0(0x50 W)

I2C>{0xA0 0xFF}{0xA1 r r} I2C START BIT WRITE: 0xA0 ACK WRITE: 0xFF ACK I2C STOP BIT I2C START BIT WRITE: 0xA1 ACK READ: 0x55 READ: ACK 0xFF NACK I2C STOP BIT I2C>

400khz_mega

Instead this is the report performed at ~50kHz. How it is possible to see in this case the answer of macro (1) is 0xA0(0x50 W) 0xA1(0x50 R).


~50kHz

Bus Pirate v3.5 Community Firmware v7.0 - goo.gl/gCzQnW [HiZ 1-WIRE UART I2C SPI 2WIRE 3WIRE KEYB LCD PIC DIO] Bootloader v4.4 DEVID:0x0447 REVID:0x3046 (24FJ64GA002 B8) http://dangerousprototypes.com HiZ>m

  1. HiZ
  2. 1-WIRE
  3. UART
  4. I2C
  5. SPI
  6. 2WIRE
  7. 3WIRE
  8. KEYB
  9. LCD
  10. PIC
  11. DIO x. exit(without change)

(1)>4 Set speed:

  1. ~5KHz
  2. ~50KHz
  3. ~100KHz
  4. ~400KHz

(1)>2 Clutch disengaged!!! To finish setup, start up the power supplies with command 'W'

Ready I2C>W POWER SUPPLIES ON Clutch engaged!!! I2C>P Pull-up resistors ON I2C>(1) Searching I2C address space. Found devices at: 0xA0(0x50 W) 0xA1(0x50 R)

I2C>{0xA0 0xFF}{0xA1 r r} I2C START BIT WRITE: 0xA0 ACK WRITE: 0xFF ACK I2C STOP BIT I2C START BIT WRITE: 0xA1 ACK READ: 0x55 READ: ACK 0xFF NACK I2C STOP BIT I2C>

50khz_mega

But talking about something else related, I have read what you wrote into new issue #39 The answer at the command "i" of my Bus Pirate is this:

HiZ>i Bus Pirate v3.5 Community Firmware v7.0 - goo.gl/gCzQnW [HiZ 1-WIRE UART I2C SPI 2WIRE 3WIRE KEYB LCD PIC DIO] Bootloader v4.4 DEVID:0x0447 REVID:0x3046 (24FJ64GA002 B8) http://dangerousprototypes.com HiZ>

Before trying what you have suggested I must read well the documentation you provided because I fear to brick my Bus Pirate. When I will have understood well all the matter, then I will. Thank you very much sir!

USBEprom commented 7 years ago

@agatti

By referring at this:

https://github.com/BusPirate/Bus_Pirate/issues/39

Here you go the results using the latest test firmware:

cb036676-ce16-11e6-88d1-719459ee8eb9

For the report I have used ~400kHz in accordance with your settings. How it is possible to see the answer of macro (1) is 0xA0(0x50 W).


~400kHz

RESET

Bus Pirate v3.5 Community Firmware v7.1 - goo.gl/gCzQnW [HiZ UART I2C SPI] Bootloader v4.4 DEVID:0x0447 REVID:0x3046 (24FJ64GA00 2 B8) http://dangerousprototypes.com HiZ>

m

  1. HiZ
  2. UART
  3. I2C
  4. SPI x. exit(without change)

(1)>3 Set speed:

  1. ~5KHz
  2. ~50KHz
  3. ~100KHz
  4. ~400KHz

(1)>4 Clutch disengaged!!! To finish setup, start up the power supplies with command 'W' Ready I2C>W POWER SUPPLIES ON Clutch engaged!!! I2C>P Pull-up resistors ON I2C>(1) Searching I2C address space. Found devices at: 0xA0(0x50 W)

I2C>{0xA0 0xFF}{0xA1 r r} I2C START BIT WRITE: 0xA0 ACK WRITE: 0xFF ACK I2C STOP BIT I2C START BIT WRITE: 0xA1 ACK READ: 0x55 READ: ACK 0xFF NACK I2C STOP BIT I2C>

01012017

Many thanks and happy new year!

USBEprom commented 7 years ago

Unfortunately actually it does not work (see #39). In order to further dig the matter I have tried to compile some previous repository that have not the patch #23 but I have failed. An alternative would be to use the current repository and compile the firmware by disabling the patch #23. Is it possible? How? That could be a way to identify the culprit by analyzing the behavior of a known working firmware when the I2C protocol is enabled both with the Software and Hardware mode while the patch #23 is missing. I guess that for the Software side, even due the sign "~", run clock at roughly half speed is correct. However I can not verify what happens when both the Software mode and the Hardware one are enabled simultaneously. Thanks.

zzmaro commented 7 years ago

Hello Gentlemen,

I have BP 3.6v which I wanted to use as I2C master device. However, in my design it totally didn't work, probably due to I2C clock stretching. Moreover, due to timing problems many of I2C devices didn't behave as they should. Then I started looking for some information and found multiple threads (also yours) regarding problems in BP FW, etc...

What I would like to know is:

  1. What is the currently developed FW version with latest and greatest I2C? On github I can see Agatti version Bus_Pirate/Bus_Pirate and different one in mikebdp2/Bus_Pirate.
  2. Is clock stretching planned to be included anytime soon?

I'm not a programmer, so I won't rather help with code, but I have access to BP v3.6, some logic analyzer, some prototype boards with many different I2C chips, so maybe I could help with debugging. I also have access to Aardvark SPI/I2C tool, so I can compare BP with proper timings. I don't have any Arduino boards.

Currently, I'm using BP v3.6 SW v6.1 r1676, HW rev B8. At the moment I don't have Microchip IDE, so I can't compile FW on my own.

@USBEprom, @agatti, If you have any new version compiled, please send it to me, I would like to test it. If you have any particular case you would like to test, let me know, maybe I could help.

USBEprom commented 7 years ago

@zzmaro

For what I know the issue with the protocol I2C did started from the application of the patch #23. I know nothing about the firmwares built by agatti and mikebdp2, but I am pretty sure they do not have the patch #23. Perhaps those firmwares have a limited unlocked features. If you need something with all the available features unlocked try this:

dangerousprototypes.com/forum/download/file.php?id=12196 http://dangerousprototypes.com/forum/viewtopic.php?f=28&t=8498&start=45#p65526

I made it myself follow the teachings of agatti, mikebdp2 and others. All the features are unlocked and I think it is the newest firmware v7.0 available for the Bus Pirate revision 3. Of course as all the firmwares v7.0 built before the applying of the patch #23 also my version allows only software mode for the protocol I2C.

zzmaro commented 7 years ago

@USBEprom Exactly. I read these threads and I would really like to test HW I2C. I have B8 silicon revision, so I should be safe. Do you have compiled version you could share with me? I will try to compile few versions, but I need some spare time to do it.

Regarding issue #23 - what exactly do you think has changed after fix? I'm not sure what software you are reffering to, but I have v6.1 (really old) and I also don't get desired speed. If I set 400kHz in SW mode, I get roughly 115kHz. Also, there are problems with delays e.g. START bit - SDA low to first clock takes ~2ms... Similar problem exist with STOP bit. image

While with Aardvark interface: image

I can see SDA during START conditon unnecessarily goes HIGH for 2ms. Without this SDA low time is ~8us, which is similar to Aardvark. I didn't find possible cause in source yet.

USBEprom commented 7 years ago

@zzmaro

zzmaro wrote:

Exactly. I read these threads and I would really like to test HW I2C. I have B8 silicon revision, so I should be safe. Do you have compiled version you could share with me? I will try to compile few versions, but I need some spare time to do it.

The firmware I have is in the archive 12112016.zip that I already wrote. The link to it is that I wrote above. My firmware 12112016 is for the Bus Pirate revision 3 and it has not the patch #23 applied, nor the workarounds that agatti wrote in order to unlock I2C hardware. Delays apart, it does work but the clock for the bus is roughly the half of what is shown into the menu. I know nothing if I2C software was purposely thought in that way, but the sign ~ before of the speed values (~5kHz, ~30kHz, 100kHz and ~400kHz) maybe it was put there for that reason. I own also some later revision of the firmware which was compiled with the patch #23 applied together with the workarounds that agatti wrote, so that it allows for I2C hardware. Unfortunately those versions do not work correctly. By the software side only the 400kHz setting does work fine, all the other settings give wrong clock for the bus I2C. While by the hardwae side seems it does work fine for all the speeds. Look at these:

https://github.com/BusPirate/Bus_Pirate/issues/39#issuecomment-271161988

zzmaro wrote:

Regarding issue #23 - what exactly do you think has changed after fix?

For what I saw the speed of the clock for the bus has become very slow, roughly 700Hz indiscriminately for all the speeds available in the menu. Take a look:

https://github.com/BusPirate/Bus_Pirate/issues/23#issuecomment-265763591 https://github.com/BusPirate/Bus_Pirate/issues/23#issuecomment-266292852

zzmaro wrote:

I'm not sure what software you are reffering to, but I have v6.1 (really old) and I also don't get desired speed. If I set 400kHz in SW mode, I get roughly 115kHz. Also, there are problems with delays e.g. START bit - SDA low to first clock takes ~2ms... Similar problem exist with STOP bit.

The firmware of which I am talking about is 12112016 that I built by myself. By using it for what I was able to see the protocol I2C does work fine although the clock for the bus is only roughly the half of the value of the speed shown into the menu of the Bus Pirate. I know that also agatti for his Bus Pirate revision 4 has described the same behaviour you wrote. My Bus Pirate is a revision 3 and I have not experienced that kind of behaviour. In my experience all the firmwares for the Bus Pirate revision 3 without the patch #23 applied that I have tested, the speed for the protocol I2C have always been consistent with the rule of the half of the value shown into the menu. I know that there are delays just as you have written, I also measured the same values like you (roughly 2ms), but I know nothing if it is wrong or right. The whole thing might be related to #18

I know nothing about Aardvark. However since you have it you could pursue the matter doing some measurement.

agatti commented 7 years ago

Moving to 7.2.

agatti commented 6 years ago

@USBEprom is this problem still happening on your board?

USBEprom commented 6 years ago

Sorry for the late reply. I have not any analyzer or oscilloscope and the usual friend of mine did some shots for me. Based on what is possible to see even the new firmware built starting from the current repository dated January 7, 2018 works with a reduced speed clock. Sadly the thing is working bad as before. Using I2C HARDWARE mode the clock timing are the correct ones, 100kHz, 400kHz and 1MHz I already wrote above. Instead by using I2C SOFTWARE mode only ~400KHz seems match in some way it being ~200KHz which is the half of the value shown on the menu, all other values are about 700Hz just as I already described here too. Is still the same even on Bus Pirate v4?

USBEprom commented 5 years ago

I just tested this with the new firmware U_1-28102018 I built yesterday (http://dangerousprototypes.com/forum/viewtopic.php?f=28&t=8498&start=105#p67796) by using the current repository dated october 28, 2018 which should have the fixes for the I2C protocol provided by Christopher Sam Soon (https://github.com/ChristopherSamSoon/Bus_Pirate). Sadly still there are problems as before while using I2C software. By choising ~5KHz, ~50KHz or ~100KHz, does not matter what of them, the I2C software clock is alway about ~700Hz, while by choosing ~400KHz it is about ~200kHz that is the half of the value set. Instead I2C hardware seems to be good being I2C hardware clock is about ~100kHz, about ~400kHz and about ~1MHz depending on whether it is choose 100KHz, 400KHz or 1MHz, so quite good. i2c

ChristopherSamSoon commented 5 years ago

I just tested this with the new firmware U_1-28102018 I built yesterday (http://dangerousprototypes.com/forum/viewtopic.php?f=28&t=8498&start=105#p67796) by using the current repository dated october 28, 2018 which should have the fixes for the I2C protocol provided by Christopher Sam Soon (https://github.com/ChristopherSamSoon/Bus_Pirate). Sadly still there are problems as before while using I2C software. By choising ~5KHz, ~50KHz or ~100KHz, does not matter what of them, the I2C software clock is alway about ~700Hz, while by choosing ~400KHz it is about ~200kHz that is the half of the value set. Instead I2C hardware seems to be good being I2C hardware clock is about ~100kHz, about ~400kHz and about ~1MHz depending on whether it is choose 100KHz, 400KHz or 1MHz, so quite good. i2c

Hi USBEprom, the I2C software clock is a known issue and the fix that I uploaded only fixed the I2C-restart problem (write then read) during read after write operations.

I have also noticed that the software I2C clock is not accurate (grossly inaccurate). However, to fix it requires a major overhaul of the software design, as the delay function is used thorough the various communication modes.

Another major problem with I2C software mode is that clock stretching is not supported. [ Big problem when talking to MCUs that are slow to respond to I2C commands from buspirate ]

I will see what I can do to fix the clock. My main gear is bus Pirate v4, therefore, I may need your help to test on v3 gear

Thank you for helping the bus-pirate community and provide test results and screenshots.

ChristopherSamSoon commented 5 years ago

I just tested this with the new firmware U_1-28102018 I built yesterday (http://dangerousprototypes.com/forum/viewtopic.php?f=28&t=8498&start=105#p67796) by using the current repository dated october 28, 2018 which should have the fixes for the I2C protocol provided by Christopher Sam Soon (https://github.com/ChristopherSamSoon/Bus_Pirate). Sadly still there are problems as before while using I2C software. By choising ~5KHz, ~50KHz or ~100KHz, does not matter what of them, the I2C software clock is alway about ~700Hz, while by choosing ~400KHz it is about ~200kHz that is the half of the value set. Instead I2C hardware seems to be good being I2C hardware clock is about ~100kHz, about ~400kHz and about ~1MHz depending on whether it is choose 100KHz, 400KHz or 1MHz, so quite good. i2c

Hi USBEprom, the I2C software clock is a known issue and the fix that I uploaded only fixed the I2C-restart problem (write then read) during read after write operations.

I have also noticed that the software I2C clock is not accurate (grossly inaccurate). However, to fix it requires a major overhaul of the software design, as the delay function is used thorough the various communication modes.

Another major problem with I2C software mode is that clock stretching is not supported. [ Big problem when talking to MCUs that are slow to respond to I2C commands from buspirate ]

I will see what I can do to fix the clock. My main gear is bus Pirate v4, therefore, I may need your help to test on v3 gear

Thank you for helping the bus-pirate community and provide test results and screenshots.

So I took a look at the delay function. There is no simple fix for it unless a hardware based timer is used with interrupt.

The issue arises because 100kHz for example is very short in comparison to the clock frequency. The Buspirate v4 runs at 16MIPS, or 16 mega instructions per second.

Now, 160 instructions is not a lot, given the fact that the CPU need to process while loops, arithmetics, stack push / pop for function call to the delay function, setting / reading IO port registers. This may probably amount to 60 instruction cycles depending on the instruction set used. At I2C setting of 400kHz, the software basically does not introduce any delay at all, and everything is limited to the CPU processing speed. Even with no delay, we are only getting 200kHz. Therefore, we can estimate that the CPU processing time for each I2C bit takes 80 instruction cycles (without any artificially introduced delay !)

Therefore: 1) 400kHz is not achievable 2) Accurate 100, 50, 5kHz may be possible if we tweak the delay values to compensate for the instruction processing time of the CPU, but this is not elegant and is considered to be a poor hack 3) Alternatively, use a hardware based timer approach running on interrupt level, but the changes to the software are very significant ( I may not be able to take on this task )

Workaround: Use hardware based I2C that supports clock stretching as well (software I2C does not currently support clock stretching)

USBEprom commented 5 years ago

@ChristopherSamSoon

Thank you very much sir for clearing things up! Even if your bug fixes are marginal in this context and they do not cover the problem under consideration here, they represent precious improvements as any possible improvement is welcome for everyone, so I want to thank you very much for your hard work and for the help that you are providing with your replies, thanks! Ok, I understand it is hard if not impossible to achieve but perhaps it is possible make the things better. It is to be considered that using old firmwares, I tested v5.10 for instance, although the things are anyway wrong they are slight better because we have I2C software clock which works half speed on regards of its value shown into the menu, so do not always ~700Hz does not matter what. The main problem there is that by setting for ~400kHz then actual I2C software clock is about ~100kHz, a quarter of the value into the menu instead of half as it is for speeds below the value of ~400kHz. This thing is in agreement with your explanation about the fact that it is impossible to reach 400kHz speed as I2C software clock. However with the current firmware v7.x for the I2C software clock we have ~700Hz for ~5kHz, ~50kHz and ~100kHz which is fully wrong and 200kHz for ~400kHz which is half of the value shown on menu. Sadly I am not a software guy but I think it may be possible to extrapolate the code responsible for the I2C software clock from the old firmwares (v5.10 for example) and add it to the current firmware v7.x. Probably I am wrong, but I would try in that direction, also because all the old firmwares supplied for the Bus Pirate have always worked with I2C clock halved compared to what is shown on the menu. I understand that it is not easy also because everything is also related with the silicon revision of the PIC welded on the Bus Pirate (#39 and http://ww1.microchip.com/downloads/en/DeviceDoc/80000470j.pdf), but maybe it is possible to manage something, of course beyond the matter of the delays and how they can possibly be arranged or bypassed. We proceed by degrees improving the possible, where there is no hope we will live with that. The last thing I can add is that anyway, for those who have a Bus Pirate that mounts the PIC with the correct silicon revision, the hardware I2C clock for it itself works according to the specifications, though then of course there is the problem of delays. Anyway, it's better than nothing. A thought of mine. Maybe it is possible to add 5kHz, 50kHz or other values as further items in the I2C hardware menu where currently it is possible choose between 100kHz, 400kHz and 1MHz.

ChristopherSamSoon commented 5 years ago

@USBEprom

So I spent my afternoon to fix this problem. Major overhaul of the bp_delay_us was done. The code was completely re-written to make use of the assistance of a hardware based timer. Actual delay is computed by reading the counter value of the free running TIMER1. This also means that TIMER1 is no longer available for use for any other purposes. (Previously TIMER1 was available for use)

Screenshots: 5kHz i2c_5khz

50kHz i2c_50khz

100kHz i2c_100khz

400kHz i2c_400khz

Unfortunately 400kHz is not achievable and the max clock rate is ~240kHz.

Note: The above was tested only on hardware v4. If you can help to test on v3 hardware, it would be great

The other advantage of using a hardware based timer is that the delay is now more predictable and jitter is minimized. If you notice, the pulses duration and duty cycle are now very consistent unlike previously

Binary: https://github.com/ChristopherSamSoon/Bus_Pirate/blob/master/package/BPv4-firmware/BPv4-firmware-v7.1-1.hex

Note: Clock stretching is also implemented in the above binary.

Let me know of your results for v3 hardware!

Best

Christopher Sam Soon

ChristopherSamSoon commented 5 years ago

@USBEprom

So I spent my afternoon to fix this problem. Major overhaul of the bp_delay_us was done. The code was completely re-written to make use of the assistance of a hardware based timer. Actual delay is computed by reading the counter value of the free running TIMER1. This also means that TIMER1 is no longer available for use for any other purposes. (Previously TIMER1 was available for use)

Screenshots: 5kHz i2c_5khz

50kHz i2c_50khz

100kHz i2c_100khz

400kHz i2c_400khz

Unfortunately 400kHz is not achievable and the max clock rate is ~240kHz.

Note: The above was tested only on hardware v4. If you can help to test on v3 hardware, it would be great

Binary: https://github.com/ChristopherSamSoon/Bus_Pirate/blob/master/package/BPv4-firmware/BPv4-firmware-v7.1-1.hex

Note: Clock stretching is also implemented in the above binary.

Let me know of your results for v3 hardware!

Best

Christopher Sam Soon

Note: since bp_delay_us is now fixed, issue (#111) and (#112) should theoretically be fixed as well

agatti commented 5 years ago

Apologies if I just chime in just now, between personal issues and work commitments my available spare time is extremely limited these days.

@ChristopherSamSoon thank you for spending time on this issue, can you please make sure that interrupts are not going to mess with other things? The main reason for switching from a busy loop based delay to a repeat NOP instruction pair provided by Microchip was to make 1-Wire work once more. Apparently that protocol is more timing-sensitive than I2C, SPI, or the like. If that does not impact on 1-Wire, then I would appreciate a pull request for me to fit in the code and (finally?) cut a v7.1 release now that OpenOCD for v4 has been added.

ChristopherSamSoon commented 5 years ago

Apologies if I just chime in just now, between personal issues and work commitments my available spare time is extremely limited these days.

@ChristopherSamSoon thank you for spending time on this issue, can you please make sure that interrupts are not going to mess with other things? The main reason for switching from a busy loop based delay to a repeat NOP instruction pair provided by Microchip was to make 1-Wire work once more. Apparently that protocol is more timing-sensitive than I2C, SPI, or the like. If that does not impact on 1-Wire, then I would appreciate a pull request for me to fit in the code and (finally?) cut a v7.1 release now that OpenOCD for v4 has been added.

@agatti Solution does not use interrupts at all. I have no means to test 1-wire protocol as I do not have 1-wire hardware. I will need help to test on 1-wire protocol here

agatti commented 5 years ago

Oh ok, I thought you were relying on TIMER1 asynchronously, my bad.

For 1-Wire, a quick and dirty way to test things up is to hook up a logic analyser with the software of your choice - as long as it can decode 1-Wire - and use https://gitea.youb.fr/youen/OneWireArduinoSlave on any Arduino board you have laying around. That's how I debugged things back then before I stumbled upon a proper 1-Wire device.

ChristopherSamSoon commented 5 years ago

@agatti Thank you for the proposal of using Arduino, I will see what I can do to test 1-wire

Best

Christopher

USBEprom commented 5 years ago

@ChristopherSamSoon

Thank you very much sir for having spent time on fixing this! I will built a test firmware for the Bus Pirate v3 starting from your repository https://github.com/ChristopherSamSoon/Bus_Pirate and I let you know the result as soon as possible. About #111 and #112 me too I hope are gone together this #23, however I will test them all and I let you know as soon as possible.

@agatti

As for you suggestion I will surely test 1-WIRE protocol too. About that kind of protocol I have doubt it is working though, because I have a DS1990A iButtons and I did not able to use it with firmware v7.x. Maybe it is me, maybe it is something wrong I did, though I have not tried yet using old firmware as comparison. However thank you a lot sir for having provided a new different way to test it if functioning or not, that is great useful. I will test the whole thing and I let you know as soon as possible.

USBEprom commented 5 years ago

@ChristopherSamSoon

thank you very much sir for your invaluable code!

@all Really sorry for the delay. Finally I was able to build a test firmware starting from the latest repository dated 04 November, 2018 released by Christopher Sam Soon (https://github.com/ChristopherSamSoon/Bus_Pirate). For testing I built both SAFE (only I2C software) that UNSAFE (freed I2C hardware) firmwares and as promised here in attachment are the results. For me all it works fine, however I will continue the tests in order to assure there are not any sort of problem somewhere and then I will let you know.

About #111 and #112 I must write that I am not 100% sure it totally works now because I tested them in an unorthodox manner. Surely the things ar better with the new code and for 2-WIRE ~5kHz, ~50kHz, ~100kHz and ~400kHz are the same as for the I2C software protocol, though for 3-WIRE too it is for ~5kHz and ~50kHz as for I2C, but ~100kHz and ~400kHz are respectively ~80kHz and ~170kHz. I tested the 2-WIRE and 3-WIRE behaviour simply entering in the protocol and sending 1hex while connected to the same Arduino on which was running the I2C sketch provided by agatti and then acquiring the traffic with the logic analyzer setted for I2C decoding. Maybe that did the result, maybe did not, but I do not know other way to test the matter. However I would dare to say that even #111 and #112 have now gone.

test

Please pay attention to the fact that orange color for the caption about ~400kHz SOFTWARE mode does not mean it is working bad, simply that does not match the value into the menu. Christopher Sam Soon has already explained there is a speed limit there so it may even be useful to change the value in the menu from ~400kHz to ~220kHz hence it would be full green as color.

USBEprom commented 5 years ago

For 1-Wire, a quick and dirty way to test things up is to hook up a logic analyser with the software of your choice - as long as it can decode 1-Wire - and use https://gitea.youb.fr/youen/OneWireArduinoSlave on any Arduino board you have laying around. That's how I debugged things back then before I stumbled upon a proper 1-Wire device.

@agatti Sorry to keep bothering you. Seems to me that library does not work with MEGA 2560 because some incompatibility with the utility LowLevel.h I guess due of these:

\libraries\OneWireSlave\src/utility/LowLevel.h: In member function 'void Pin::attachInterrupt(void (*)(), int)':

\libraries\OneWireSlave\src/utility/LowLevel.h:157:3: error: 'CLEARINTERRUPT' was not declared in this scope

CLEARINTERRUPT; // clear any pending interrupt (we want to call the handler only for interrupts happening after it is attached)

^

I wonder if it possible to fix the issue or otherwise use something else to check 1-WIRE protocol. Library and sketch are compiled without problems for Arduino/Genuino UNO but by changing target as for Arduino MEGA 2560 them fail with the erros I wrote and sadly I own only the latter. The only change I made to the original sketch was to change the interrupt pin from 2 to 20 due to the fact that the pin numbering on the MEGA 2560 is different from that of the UNO.

And another thing if possible. What on how connect together Bus Pirate and Arduino? I am scared to damage Arduino and/or Bus Pirate due wrong connections because each of them provides its own +5V. I guess it is need to connect the ground on the Bus Pirate to the one on the Arduino, then MOSI on the Bus Pirate which is the pins responsible for 1-WIRE protocol on it to an interrupt pin on the Arduino (I chose pin 20 of the Arduino MEGA 2560 which is labeled as interrupt 1 [INT1] so that I have really used it on the sketch). I do not sure for the VPU and pull-up resistor, though. Should I jumpering + 5V with VPU and activate power supply and pull-up resistors (command W and P on the Bus Pirate side) or is it enough to activate the pull-up resistors by issuing the P command without turning on the power supply with the command W due the fact the +5V is already provided by the Arduino for the simulation? Thanks.

agatti commented 5 years ago

@USBEprom I used an Arduino Nano for this, even if I had the time to look into this I do not have neither a Uno nor a Mega 2560 I'm afraid. For connections, if you connect the two GND pins together and MOSI and 1-Wire together as well, that should be it - if both devices are using 5v logic levels (that's important!) for data transmission it shouldn't damage anything. At most it won't detect any data coming in or out - it's the same principle that applies for UART to USB adapters, for example...

USBEprom commented 5 years ago

@agatti

Please do not worry sir. So for now I will try with iButton I own, although I already did without result. First I will test the thing with old firmwares in order to try to understand where is the problem. Thanks a lot sir!