Add pull up testing, proper us delay and stop supporting 45 and 46 for I2C

2 files changed, 28 insertions, 47 deletions

diff --git a/ports/esp32s2/common-hal/busio/I2C.c b/ports/esp32s2/common-hal/busio/I2C.c
index 0a696bed6..fd983619f 100644
--- a/ports/esp32s2/common-hal/busio/I2C.c
+++ b/ports/esp32s2/common-hal/busio/I2C.c
@@ -3,7 +3,7 @@
  *
  * The MIT License (MIT)
  *
- * Copyright (c) 2016 Scott Shawcroft
+ * Copyright (c) 2020 Scott Shawcroft for Adafruit Industries LLC
  *
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the "Software"), to deal
@@ -30,16 +30,9 @@
 
 #include "driver/i2c.h"
 
-#include "esp_log.h"
-
 #include "shared-bindings/microcontroller/__init__.h"
 #include "supervisor/shared/translate.h"
 
-// Number of times to try to send packet if failed.
-#define ATTEMPTS 2
-
-static const char* TAG = "CircuitPython I2C";
-
 typedef enum {
     STATUS_FREE = 0,
     STATUS_IN_USE,
@@ -63,42 +56,43 @@ void i2c_reset(void) {
 
 void common_hal_busio_i2c_construct(busio_i2c_obj_t *self,
         const mcu_pin_obj_t* scl, const mcu_pin_obj_t* sda, uint32_t frequency, uint32_t timeout) {
-
-    // Make sure scl and sda aren't input only
+    // Pins 45 and 46 are "strapping" pins that impact start up behavior. They usually need to
+    // be pulled-down so pulling them up for I2C is a bad idea. To make this hard, we don't
+    // support I2C on these pins.
+    //
+    // 46 is also input-only so it'll never work.
+    if (scl->number == 45 || scl->number == 46 || sda->number == 45 || sda->number == 46) {
+        mp_raise_ValueError(translate("Invalid pins"));
+    }
 
 #if CIRCUITPY_REQUIRE_I2C_PULLUPS
-    // // Test that the pins are in a high state. (Hopefully indicating they are pulled up.)
-    // gpio_set_pin_function(sda->number, GPIO_PIN_FUNCTION_OFF);
-    // gpio_set_pin_function(scl->number, GPIO_PIN_FUNCTION_OFF);
-    // gpio_set_pin_direction(sda->number, GPIO_DIRECTION_IN);
-    // gpio_set_pin_direction(scl->number, GPIO_DIRECTION_IN);
+    // Test that the pins are in a high state. (Hopefully indicating they are pulled up.)
+    gpio_set_direction(sda->number, GPIO_MODE_DEF_INPUT);
+    gpio_set_direction(scl->number, GPIO_MODE_DEF_INPUT);
 
-    // gpio_set_pin_pull_mode(sda->number, GPIO_PULL_DOWN);
-    // gpio_set_pin_pull_mode(scl->number, GPIO_PULL_DOWN);
+    gpio_pulldown_en(sda->number);
+    gpio_pulldown_en(scl->number);
 
-    // common_hal_mcu_delay_us(10);
+    common_hal_mcu_delay_us(10);
 
-    // gpio_set_pin_pull_mode(sda->number, GPIO_PULL_OFF);
-    // gpio_set_pin_pull_mode(scl->number, GPIO_PULL_OFF);
+    gpio_pulldown_dis(sda->number);
+    gpio_pulldown_dis(scl->number);
 
-    // // We must pull up within 3us to achieve 400khz.
-    // common_hal_mcu_delay_us(3);
+    // We must pull up within 3us to achieve 400khz.
+    common_hal_mcu_delay_us(3);
 
-    // if (!gpio_get_pin_level(sda->number) || !gpio_get_pin_level(scl->number)) {
-    //     reset_pin_number(sda->number);
-    //     reset_pin_number(scl->number);
-    //     mp_raise_RuntimeError(translate("SDA or SCL needs a pull up"));
-    // }
+    if (gpio_get_level(sda->number) == 0 || gpio_get_level(scl->number) == 0) {
+        reset_pin_number(sda->number);
+        reset_pin_number(scl->number);
+        mp_raise_RuntimeError(translate("SDA or SCL needs a pull up"));
+    }
 #endif
-    ESP_EARLY_LOGW(TAG, "create I2C");
 
 
     self->semaphore_handle = xSemaphoreCreateBinaryStatic(&self->semaphore);
     xSemaphoreGive(self->semaphore_handle);
-    ESP_EARLY_LOGW(TAG, "new I2C lock %x", self->semaphore_handle);
     self->sda_pin = sda;
     self->scl_pin = scl;
-    ESP_EARLY_LOGW(TAG, "scl %d sda %d", self->sda_pin->number, self->scl_pin->number);
     self->i2c_num = I2C_NUM_MAX;
     for (i2c_port_t num = 0; num < I2C_NUM_MAX; num++) {
         if (i2c_status[num] == STATUS_FREE) {
@@ -120,15 +114,10 @@ void common_hal_busio_i2c_construct(busio_i2c_obj_t *self,
             .clk_speed = frequency,
         }
     };
-    // ESP_EARLY_LOGW(TAG, "param config %p %p %d %d", &i2c_conf, &(i2c_conf.mode), i2c_conf.mode, I2C_MODE_MAX);
-    ESP_EARLY_LOGW(TAG, "param config %p %d %d", &i2c_conf, i2c_conf.mode, I2C_MODE_MAX);
     esp_err_t result = i2c_param_config(self->i2c_num, &i2c_conf);
     if (result != ESP_OK) {
-        ESP_EARLY_LOGW(TAG, "error %d %p %d %d", result, &i2c_conf, (&i2c_conf)->mode, I2C_MODE_MAX);
-        vTaskDelay(0);
         mp_raise_ValueError(translate("Invalid pins"));
     }
-    ESP_EARLY_LOGW(TAG, "param config I2C done");
     result = i2c_driver_install(self->i2c_num,
                                 I2C_MODE_MASTER,
                                 0,
@@ -140,7 +129,6 @@ void common_hal_busio_i2c_construct(busio_i2c_obj_t *self,
 
     claim_pin(sda);
     claim_pin(scl);
-    ESP_EARLY_LOGW(TAG, "create I2C done");
 }
 
 bool common_hal_busio_i2c_deinited(busio_i2c_obj_t *self) {
@@ -166,19 +154,13 @@ bool common_hal_busio_i2c_probe(busio_i2c_obj_t *self, uint8_t addr) {
     i2c_master_start(cmd);
     i2c_master_write_byte(cmd, addr << 1, true);
     i2c_master_stop(cmd);
-    esp_err_t result = i2c_master_cmd_begin(self->i2c_num, cmd, 100);
+    esp_err_t result = i2c_master_cmd_begin(self->i2c_num, cmd, 10);
     i2c_cmd_link_delete(cmd);
     return result == ESP_OK;
 }
 
 bool common_hal_busio_i2c_try_lock(busio_i2c_obj_t *self) {
-    ESP_EARLY_LOGW(TAG, "locking I2C %x", self->semaphore_handle);
     self->has_lock = xSemaphoreTake(self->semaphore_handle, 0) == pdTRUE;
-    if (self->has_lock) {
-        ESP_EARLY_LOGW(TAG, "lock grabbed");
-    } else {
-        ESP_EARLY_LOGW(TAG, "unable to grab lock");
-    }
     return self->has_lock;
 }
 
@@ -187,7 +169,6 @@ bool common_hal_busio_i2c_has_lock(busio_i2c_obj_t *self) {
 }
 
 void common_hal_busio_i2c_unlock(busio_i2c_obj_t *self) {
-    ESP_EARLY_LOGW(TAG, "unlocking I2C");
     xSemaphoreGive(self->semaphore_handle);
     self->has_lock = false;
 }
@@ -213,8 +194,7 @@ uint8_t common_hal_busio_i2c_write(busio_i2c_obj_t *self, uint16_t addr,
 }
 
 uint8_t common_hal_busio_i2c_read(busio_i2c_obj_t *self, uint16_t addr,
-        uint8_t *data, size_t len) {
-
+                                  uint8_t *data, size_t len) {
     i2c_cmd_handle_t cmd = i2c_cmd_link_create();
     i2c_master_start(cmd);
     i2c_master_write_byte(cmd, addr << 1 | 1, true); // | 1 to indicate read
diff --git a/ports/esp32s2/mphalport.c b/ports/esp32s2/mphalport.c
index da5258b0e..e1662a6ce 100644
--- a/ports/esp32s2/mphalport.c
+++ b/ports/esp32s2/mphalport.c
@@ -32,9 +32,10 @@
 #include "py/gc.h"
 
 #include "esp-idf/components/xtensa/include/esp_debug_helpers.h"
+#include "esp-idf/components/esp_rom/include/esp32s2/rom/ets_sys.h"
 
 void mp_hal_delay_us(mp_uint_t delay) {
-    mp_hal_delay_ms(delay / 1000);
+    ets_delay_us(delay);
 }
 
 // This is provided by the esp-idf/components/xtensa/esp32s2/libhal.a binary