Skip to content
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

Issue with qemu i2c bitbang #96

Closed
fariouche opened this issue Sep 12, 2023 · 10 comments
Closed

Issue with qemu i2c bitbang #96

fariouche opened this issue Sep 12, 2023 · 10 comments

Comments

@fariouche
Copy link

Hello,

So, I had some time to continue on the i2c implementation on my esp32 board.
I've found an error, were the i2c send does not work.
There is a small mistake in the code:
in picsimlab_i2c_event(), (bsim_qemu.cc), case I2C_READ, the return should be "datas" and not "datar"

With that modification, i can send and receive i2c packet just fine.

By the way, I now understand better what you meant back then by saying that because of the synchronous i2c implementation, you are blocking the simulation until the i2c is read or written.

I have good news: the version of qemu used by esp32 supports async i2c.
https://patchwork.ozlabs.org/project/qemu-devel/patch/20220606150732.2282041-18-clg@kaod.org/

I didn't check the stm32 qemu.
The calls are a little different in async mode, and i do not know yet how to modify the code to implement this.

@lcgamboa
Copy link
Owner

Hi,

the bsim_qemu use only the bitbang_i2c_ctrl (I2C Controller/Master). The "datas" (data to send) is not used in the controller implementation. The "datas" is only used by I2C peripherals (bitbang_i2c).
The "datar" means data received. With your modification, the examples and tests that use I2C don´t work anymore.

To implement your display communication you should add an I2C peripheral or check the I2C address of the picsimlab_i2c_event callback and do the communication directly without using the I2C controller (and bit-bang), implementing your own switch/case for the address of your display.

I know that Qemu now supports async I2C, but the Espressif ESP32 doesn´t yet. I believe that adding this support to Qemu/PICSimLab will not improve the simulation speed significantly.

@fariouche
Copy link
Author

fariouche commented Sep 13, 2023

oh, so I misunderstood then how to do it.
What I've done is create an i2c device, and call bitbang_i2c from it from a function named (xxx_io) (bitbang_i2c_io, bitbang_i2c_send and bitbang_i2c_get_status) (I basically copied the behavior of rtc_pfc8563)
This xxx_io function is called by my board from inside Run_CPU_ns...

What I noticed is that the bitbang_i2c_send call was never received by my firmware application. After changing the bitbang_i2c_ctrl, it started to work. (I was confused by the fact that datas is not used anywhere except as a return status in bitbang_i2c... so I don't understand how this send function can work)

here is the modification I've done. What example are you talking about? (because I checked all the qemu boards and I didn't see any breakage, but they do not use i2c)

index 941e74f7..1007fab7 100644
--- a/src/boards/bsim_qemu.cc
+++ b/src/boards/bsim_qemu.cc
@@ -160,7 +160,7 @@ static int picsimlab_i2c_event(const uint8_t id, const uint8_t addr, const uint1
             g_board->timer.last += 72000;
             g_board->Run_CPU_ns(72000);
             dprintf("<== recv addr=0x%02x value=0x%02x\n", addr, g_board->master_i2c[id].datar);
-            return g_board->master_i2c[id].datar;
+            return g_board->master_i2c[id].datas;
             break;
     }
     return 0;

@lcgamboa
Copy link
Owner

From the description you are on the correct path, but from your result you must be using just one bitbang_i2c_t object shared between the controller and peripheral (which doesn't work) or you are using two bitbang_i2c_t objects but passing one as an exchanged parameter in the I2C functions.

Are you using one structure like the board K16F uses for PCF8563? Note that pre-treatment must be done on the pins used for I2C for the bus to work. (You can use VCD dump with pulseview I2C decoder to verify)

Theoretically, placing the I2C peripheral update at the end of the if that contains the MStep method call (which updates the I2C controller) should work.

The example is for BM280. The automated tests (in folder tests) uses the BM280 for test I2C of ESP32 too. With your modification, the example and test failed to read data from BM280.

@lcgamboa
Copy link
Owner

Observs that in the code of pre-treatment when the pins are accessed directly from pins array the index is subtracted from one. For example, pin number 3 is accessed as pins[2], and the functions use the number 3 directly.

@fariouche
Copy link
Author

Yes, I've noticed the minus one when accessing the pins directly.

About my device, yes it has its own bitbang_i2c_t struct. And the bsim_qemu has an array of bitbang_i2c_t named master_i2c[]
I've used vcdump and pulseview and I did see the I2C signal.

I will check the example to understand how it's done.

Since my device is directly used inside my board, I may have overlooked something. Most likely the access to the pins (however, I know that the pin values are correctly reflecting the I2C transfert since I'm using MGetPin(scl-1) and MGetPin(sda-1) in my device Io function)
I will also push my code in my fork too very soon.

@lcgamboa
Copy link
Owner

Hi @fariouche ,

I have implemented a simple test adding a bmp280 I2C sensor to the devkit board. It works without problems.
The modified code and one workspace to test are in the annexed zip file. (It uses the same code as the original bm280 example).
You can use it as a base to discover the problem in your implementation.

builtin_bmp280.zip

@fariouche
Copy link
Author

Hi @lcgamboa ,

Thanks for the sample. I know understand why my modification was wrong and also why it worked by chance in my use case.
I've found the reason my send was not working....
I had to do something similar than SpareParts.SetPullupBus, like so:
pins[22 - 1].value &= io_axp192_io(&axp192, MGetPin(23)/scl/, MGetPin(22)/sda/);

It's a bit ugly, as I was not able to use the SpareParts function. Most likely because my device is not a spare part but directly called from inside my board?

@lcgamboa
Copy link
Owner

For historical reasons, the spare parts simulation is isolated from the board simulation. In the beginning, there was only the board simulation. Spare parts are created after. In the future, perhaps the board could become one spare part.
But now I believe you are on the right path.

@fariouche
Copy link
Author

it does not shock me to have them separate. One evolution that would be helpful is to hide this "pullup" code so that it is transparent. Maybe inside bitbang_i2c_io?

@lcgamboa
Copy link
Owner

I'll put this on the to-do list. Lately, I haven't had time to work on PICSimLab.

@lcgamboa lcgamboa closed this as completed Dec 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants