[riot-notifications] [RIOT-OS/RIOT] cpu/esp32: fix of periph_* submodule compilation (#11337)

Leandro Lanzieri notifications at github.com
Mon Apr 15 10:22:09 CEST 2019


leandrolanzieri requested changes on this pull request.

Just a few comments, it looks good. I will be testing this shortly.

> @@ -0,0 +1,100 @@
+/*
+ * Copyright (C) 2018 Gunar Schorcht

```suggestion
 * Copyright (C) 2019 Gunar Schorcht
```
I guess? Here and in all new files.

>  else
     # PLEASE NOTE: because of the very poor and faulty hardware implementation
     # we use software implementation by default for the moment (if module
     # esp_i2c_hw is not explicitly used)
     USEMODULE += esp_i2c_sw
+    USEMODULE += periph_i2c_sw

Would this include the I2C module always?

> @@ -0,0 +1,152 @@
+/*
+ * Copyright (C) 2018 Gunar Schorcht
+ *
+ * This file is subject to the terms and conditions of the GNU Lesser General
+ * Public License v2.1. See the file LICENSE in the top level directory for more
+ * details.
+ */
+
+/**
+ * @ingroup     cpu_esp32
+ * @ingroup     drivers_periph_adc
+ * @{
+ *
+ * @file
+ * @brief       Low-level ADC driver implementation

```suggestion
 * @brief       Low-level DAC driver implementation
```

> +            return -1;
+        }
+        _dac_module_initialized = true;
+    }
+
+    _adc2_ctrl_init();
+
+    uint8_t rtcio = _gpio_rtcio_map[dac_pins[line]];
+    uint8_t idx;
+
+    /* try to initialize the pin as DAC ouput */
+    if (gpio_get_pin_usage(_adc_hw[rtcio].gpio) != _GPIO) {
+        LOG_TAG_ERROR("dac", "GPIO%d is used for %s and cannot be used as "
+                      "DAC output\n", _adc_hw[rtcio].gpio,
+                      gpio_get_pin_usage_str(_adc_hw[rtcio].gpio));
+        return DAC_NOLINE;

Is it ok that `_dac_module_initialized` is not set to false in this case and bellow?

> @@ -38,6 +38,7 @@
 
 #include "esp_common.h"
 #include "adc_arch.h"
+#include "adc_ctrl.h"

Is this being used here?

> +
+    /* SAR ADC2 controller configuration */
+    SENS.sar_read_ctrl2.sar2_dig_force = 0;     /* SAR ADC2 controlled by RTC not DIG*/
+    SENS.sar_meas_start2.meas2_start_force = 1; /* SAR ADC2 started by SW */
+    SENS.sar_meas_start2.sar2_en_pad_force = 1; /* pad enable bitmap controlled by SW */
+    SENS.sar_read_ctrl2.sar2_data_inv = 1;      /* invert data */
+    SENS.sar_atten2 = 0xffffffff;               /* set attenuation to 11 dB for all pads
+                                                   (input range 0 ... 3,3 V) */
+    /* set default resolution */
+    SENS.sar_start_force.sar2_bit_width = ADC_RES_12BIT;
+    SENS.sar_read_ctrl2.sar2_sample_bit = ADC_RES_12BIT;
+
+    _adc2_ctrl_initialized = true;
+}
+
+extern const gpio_t _gpio_rtcio_map[];

This is not needed right? It's defined before.

> +
+/* forward declarations of internal functions */
+static bool _dac_conf_check(void);
+static bool _dac_module_initialized  = false;
+
+/* external variable declarations */
+extern const gpio_t _gpio_rtcio_map[];
+
+int8_t dac_init (dac_t line)
+{
+    CHECK_PARAM_RET (line < dac_chn_num, DAC_NOLINE)
+
+    if (!_dac_module_initialized) {
+        /* do some configuration checks */
+        if (!_dac_conf_check()) {
+            return -1;

Is it possible to use the error code `DAC_NOLINE` here?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/RIOT-OS/RIOT/pull/11337#pullrequestreview-225927977
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20190415/51adfaad/attachment-0001.html>


More information about the notifications mailing list