[EV3] Implement I2C support via PRU1 #360
Conversation
|
We should probably have a "PRU1" driver to handle shared init, and we should figure out how to coordinate the PRU->ARM interrupt between PRU0 and PRU1 |
Yes, a new
Probably |
Although there are 8 different |
|
I was referring to the setting up of the interrupt channel mapping ( https://github.com/pybricks/pybricks-micropython/blob/master/lib/pbio/drv/uart/uart_ev3_pru_lib/suart_api.c#L2324 ) |
|
Ah, I would pull out all of the code for the PRU's interrupt controller to the new PRU rproc driver as well. Since we don't need to make this general purpose, we can just do all of the setup for both PRUs in one place and hard-code it. The various drivers that depend on the PRUs can then just wait for a "kick" from the PRU when it is actually up and running to know when they can actually start using the PRU. |
|
I spent a bunch of time chasing down a strange race condition and realized something that will be very annoying: The AM1808 has "set bit" and "clear bit" registers for the GPIO output value but, critically, NOT for the GPIO direction (which is what an open-drain bus like I2C needs). Because this requires a RmW sequence, the PRU and ARM can corrupt each other's access unless we either implement locking of some form or else make the PRU completely in control of all direction changes on IO banks 0/1. The only thing this is likely to affect after initial setup is completed is peripherals of some form, but this implies that parts of the DCM logic and/or the NXT color sensor handling might end up depending on the PRU |
|
Note that I am currently testing with this patch, because I2CDevice isn't ported yet: diff --git a/lib/pbio/drv/i2c/i2c_ev3.c b/lib/pbio/drv/i2c/i2c_ev3.c
index 853994a8..5bad271f 100644
--- a/lib/pbio/drv/i2c/i2c_ev3.c
+++ b/lib/pbio/drv/i2c/i2c_ev3.c
@@ -13,6 +13,7 @@
#include <string.h>
#include <tiam1808/armv5/am1808/interrupt.h>
+#include <tiam1808/hw/hw_syscfg0_AM1808.h>
#include <tiam1808/hw/hw_types.h>
#include <tiam1808/pruss.h>
@@ -22,8 +23,10 @@
#include <pbio/util.h>
#include <pbdrv/cache.h>
+#include <pbdrv/gpio.h>
#include <pbdrv/i2c.h>
+#include "../drv/gpio/gpio_ev3.h"
#include "../drv/rproc/rproc.h"
#include "../rproc/rproc_ev3.h"
@@ -171,9 +174,115 @@ pbio_error_t pbdrv_i2c_placeholder_operation(
PBIO_OS_ASYNC_END(PBIO_SUCCESS);
}
+static pbdrv_gpio_t test_2_scl = PBDRV_GPIO_EV3_PIN(0, 15, 12, 0, 12);
+static pbdrv_gpio_t test_2_sda = PBDRV_GPIO_EV3_PIN(2, 7, 4, 1, 14);
+static pbdrv_gpio_t test_3_scl = PBDRV_GPIO_EV3_PIN(1, 27, 24, 0, 1);
+static pbdrv_gpio_t test_3_sda = PBDRV_GPIO_EV3_PIN(2, 3, 0, 1, 15);
+
+static pbio_os_process_t ev3_i2c_wip_ultrasonic_process;
+static pbio_os_process_t ev3_i2c_wip_rpi_pico_process;
+
+pbio_error_t ev3_i2c_wip_ultrasonic_process_thread(pbio_os_state_t *state, void *context) {
+ static pbio_os_timer_t timer;
+ static pbio_os_state_t sub;
+ static int i;
+ static uint8_t buf[7];
+
+ PBIO_OS_ASYNC_BEGIN(state);
+
+ while (1) {
+ memset(buf, 0, sizeof(buf));
+ buf[0] = 0x10;
+
+ pbio_error_t err;
+ PBIO_OS_AWAIT(state, &sub, err = pbdrv_i2c_placeholder_operation(
+ &sub,
+ &i2c_devs[2],
+ 0x01,
+ &buf[0],
+ 1,
+ &buf[1],
+ 6,
+ true
+ ));
+
+ if (err) {
+ debug_pr("i2c ultrasonic err %d\r\n", err);
+ } else {
+ for (i = 0; i < 6; i++) {
+ debug_pr("i2c ultrasonic get %02x\r\n", buf[1 + i]);
+ }
+ }
+
+ PBIO_OS_AWAIT_MS(state, &timer, 1000);
+ }
+
+ PBIO_OS_ASYNC_END(PBIO_SUCCESS);
+}
+
+pbio_error_t ev3_i2c_wip_rpi_pico_process_thread(pbio_os_state_t *state, void *context) {
+ static pbio_os_timer_t timer;
+ static pbio_os_state_t sub;
+ static int i;
+ static uint8_t buf[7];
+
+ PBIO_OS_ASYNC_BEGIN(state);
+
+ while (1) {
+ memset(buf, 0, sizeof(buf));
+ buf[0] = 0x10;
+ buf[1] = 0xaa;
+ buf[2] = 0x55;
+ buf[3] = 0x12;
+ buf[4] = 0x34;
+
+ pbio_error_t err;
+ PBIO_OS_AWAIT(state, &sub, err = pbdrv_i2c_placeholder_operation(
+ &sub,
+ &i2c_devs[3],
+ 0x17,
+ &buf[0],
+ 5,
+ 0,
+ 0,
+ false
+ ));
+
+ if (err) {
+ debug_pr("i2c rpi pico #1 err %d\r\n", err);
+ }
+
+ buf[0] = 0x0e;
+ PBIO_OS_AWAIT(state, &sub, err = pbdrv_i2c_placeholder_operation(
+ &sub,
+ &i2c_devs[3],
+ 0x17,
+ &buf[0],
+ 1,
+ &buf[1],
+ 6,
+ false
+ ));
+
+ if (err) {
+ debug_pr("i2c rpi pico #2 err %d\r\n", err);
+ } else {
+ for (i = 0; i < 6; i++) {
+ debug_pr("i2c rpi pico get %02x\r\n", buf[1 + i]);
+ }
+ }
+
+ PBIO_OS_AWAIT_MS(state, &timer, 1000);
+ }
+
+ PBIO_OS_ASYNC_END(PBIO_SUCCESS);
+}
+
static pbio_os_process_t ev3_i2c_init_process;
pbio_error_t ev3_i2c_init_process_thread(pbio_os_state_t *state, void *context) {
+ static pbio_os_timer_t timer;
+
PBIO_OS_ASYNC_BEGIN(state);
// Need rproc to be initialized, because it sets up the PRU INTC
@@ -209,6 +318,22 @@ pbio_error_t ev3_i2c_init_process_thread(pbio_os_state_t *state, void *context)
pbio_busy_count_down();
+
+
+ pbdrv_gpio_out_low(&test_2_scl);
+ pbdrv_gpio_input(&test_2_scl);
+ pbdrv_gpio_out_low(&test_2_sda);
+ pbdrv_gpio_input(&test_2_sda);
+ pbdrv_gpio_out_low(&test_3_scl);
+ pbdrv_gpio_input(&test_3_scl);
+ pbdrv_gpio_out_low(&test_3_sda);
+ pbdrv_gpio_input(&test_3_sda);
+
+ PBIO_OS_AWAIT_MS(state, &timer, 200);
+
+ pbio_os_process_start(&ev3_i2c_wip_ultrasonic_process, ev3_i2c_wip_ultrasonic_process_thread, NULL);
+ pbio_os_process_start(&ev3_i2c_wip_rpi_pico_process, ev3_i2c_wip_rpi_pico_process_thread, NULL);
+
PBIO_OS_ASYNC_END(PBIO_SUCCESS);
}
diff --git a/lib/pbio/platform/ev3/platform.c b/lib/pbio/platform/ev3/platform.c
index 42add184..fbbe3143 100644
--- a/lib/pbio/platform/ev3/platform.c
+++ b/lib/pbio/platform/ev3/platform.c
@@ -367,7 +367,7 @@ const pbdrv_ioport_platform_data_t pbdrv_ioport_platform_data[PBDRV_CONFIG_IOPOR
.adc_p1 = 10,
.adc_p6 = 9,
},
- .supported_modes = PBIO_PORT_MODE_UART | PBIO_PORT_MODE_LEGO_DCM,
+ .supported_modes = 0,
},
{
.port_id = PBIO_PORT_ID_4,
@@ -389,7 +389,7 @@ const pbdrv_ioport_platform_data_t pbdrv_ioport_platform_data[PBDRV_CONFIG_IOPOR
.adc_p1 = 12,
.adc_p6 = 11,
},
- .supported_modes = PBIO_PORT_MODE_UART | PBIO_PORT_MODE_LEGO_DCM,
+ .supported_modes = 0,
},
}; |
This makes it much easier to keep the projects in sync.
The firmware expects .bss variables to be zeroed by the loader, which is this code here.
ArcaneNibble
force-pushed
the
i2c
branch
2 times, most recently
from
c91d164 to
911ceb6
Compare
|
All updated, both here and in PRU firmware. |
|
I think we should move the firmware update patch before any patch that actually makes use of the new firmware so that git bisect works. Other than that, this looks good to go. |
This is the released firmware binary which implements I2C.
Give the PRU complete control over GPIO banks 0/1. The PRU needs to be able to change GPIO pin directions for I2C. However, if the ARM and the PRU both attempt read-modify-write operations on this register, they can corrupt each other's access. Solve this by giving the PRU complete control over these banks once the PRU has booted.
This defines the communications structure and sets up the resources for PRU1's software I2C controller.
This calls the I2C init function from core.c and removes the platform data which is unneeded with how we're using the PRU.
The PRU uses these events to inform the ARM of completion (both success and failure).
These buffers are sized for a maximum-length transaction and are DMA-aligned. This API currently does not / will not support zero-copy operation, and user buffers will be copied to/from these DMA buffers.
This implements performing an I2C operation asynchronously. It does not hook this up to any higher-level functionality.
The only platform we have with this SoC is the EV3, so rename this configuration parameter to match.
|
You're right, done. |
| GPIODirModeSet(SOC_GPIO_0_REGS, pin_index, GPIO_DIR_OUTPUT); | ||
| if (PBDRV_GPIO_EV3_ARM_OWNS_GPIO_BANK(pin_index)) { | ||
| GPIODirModeSet(SOC_GPIO_0_REGS, pin_index, GPIO_DIR_OUTPUT); | ||
| } else if (GPIODirModeGet(SOC_GPIO_0_REGS, pin_index) != GPIO_DIR_OUTPUT) { |
There was a problem hiding this comment.
Shouldn't this check go as the first if, so we don't re-set it for pins owned by the ARM either?
(I know it used to double-set it before as well, but now that we have the test we might as well cover both cases.)
There was a problem hiding this comment.
I figured that code path was cheap enough (a single register write) that it wasn't worth bothering
+ pbdrv_gpio_out_low(&test_2_scl);
+ pbdrv_gpio_input(&test_2_scl);
+ pbdrv_gpio_out_low(&test_2_sda);
+ pbdrv_gpio_input(&test_2_sda);
+ pbdrv_gpio_out_low(&test_3_scl);
+ pbdrv_gpio_input(&test_3_scl);
+ pbdrv_gpio_out_low(&test_3_sda);
+ pbdrv_gpio_input(&test_3_sda);
+
+ PBIO_OS_AWAIT_MS(state, &timer, 200);What is the delay for and why is it 200 ms? Some delay does seem to be needed the first time after doing this (gives ERROR_IO otherwise.) Knowing what it is for would help me find the best place for this delay. Just 10 ms seems to be OK too. I think we can make the port mode change awaitable, so we can do that here. As well as await cancellation (or completion) of ongoing UART/I2C transations. Looks like the delay is needed between two transactions too. Could we make |
|
I'll move it to a dedicated issue. |
4 participants
This implements the basic I2C driver for the EV3.
Depends on pybricks/pybricks-pru#2