Tweak arg checking and comments

Author: tannewt

ports/espressif/bindings/espulp/ULP.c

@@ -56,7 +56,7 @@ STATIC espulp_ulp_obj_t *get_ulp_obj(mp_obj_t self_in) {
 }
 
 //|     def deinit(self) -> None:
-//|         """Deinitialises the camera and releases all memory resources for reuse."""
+//|         """Deinitialises the ULP and releases it for another program."""
 //|         ...
 STATIC mp_obj_t espulp_ulp_deinit(mp_obj_t self_in) {
     espulp_ulp_obj_t *self = get_ulp_obj(self_in);
@@ -104,12 +104,19 @@ STATIC mp_obj_t espulp_ulp_run(size_t n_args, const mp_obj_t *pos_args, mp_map_t
 
     mp_obj_t pins_in = args[ARG_pins].u_obj;
     const size_t num_pins = (size_t)MP_OBJ_SMALL_INT_VALUE(mp_obj_len(pins_in));
+
+    // The ULP only supports 21 pins on the ESP32-S2 and S3. So we can store it
+    // as a bitmask in a 32 bit number. The common-hal code does further checks.
     uint32_t pin_mask = 0;
 
     for (mp_uint_t i = 0; i < num_pins; i++) {
         mp_obj_t pin_obj = mp_obj_subscr(pins_in, MP_OBJ_NEW_SMALL_INT(i), MP_OBJ_SENTINEL);
         validate_obj_is_free_pin(pin_obj);
-        pin_mask |= 1 << ((const mcu_pin_obj_t *)pin_obj)->number;
+        const mcu_pin_obj_t *pin = ((const mcu_pin_obj_t *)pin_obj);
+        if (pin->number >= 32) {
+            raise_ValueError_invalid_pin();
+        }
+        pin_mask |= 1 << pin->number;
     }
 
     common_hal_espulp_ulp_run(self, bufinfo.buf, bufinfo.len, pin_mask);

ports/espressif/common-hal/espulp/ULP.c

@@ -61,7 +61,7 @@ void common_hal_espulp_ulp_run(espulp_ulp_obj_t *self, uint32_t *program, size_t
     }
 
     if (pin_mask >= (1 << 22)) {
-        mp_raise_ValueError(translate("Pins 21+ not supported from ULP"));
+        raise_ValueError_invalid_pin();
     }
 
     for (uint8_t i = 0; i < 32; i++) {
@@ -89,7 +89,7 @@ void common_hal_espulp_ulp_halt(espulp_ulp_obj_t *self) {
     // ulp_riscv_timer_stop();
     // ulp_riscv_halt();
 
-    // stop the ulp timer so that is doesn't restart the cpu
+    // stop the ulp timer so that it doesn't restart the cpu
     CLEAR_PERI_REG_MASK(RTC_CNTL_ULP_CP_TIMER_REG, RTC_CNTL_ULP_CP_SLP_TIMER_EN);
 
     // suspends the ulp operation
@@ -107,6 +107,9 @@ void common_hal_espulp_ulp_halt(espulp_ulp_obj_t *self) {
 }
 
 void common_hal_espulp_ulp_construct(espulp_ulp_obj_t *self) {
+    // Use a static variable to track ULP in use so that subsequent code runs can
+    // use a running ULP. This is only to prevent multiple portions of user code
+    // from using the ULP concurrently.
     if (ulp_used) {
         mp_raise_ValueError_varg(translate("%q in use"), MP_QSTR_ULP);
     }

shared-bindings/memorymap/AddressRange.c

@@ -105,17 +105,17 @@ STATIC MP_DEFINE_CONST_DICT(memorymap_addressrange_locals_dict, memorymap_addres
 //|     def __getitem__(self, index: int) -> int:
 //|         """Returns the value(s) at the given index.
 //|
-//|         1, 2, 4 and 8 byte reads will be done in one assignment. All others
-//|         will use memcpy."""
+//|         1, 2, 4 and 8 byte aligned reads will be done in one transaction.
+//|         All others may use multiple transactions."""
 //|         ...
 //|     @overload
 //|     def __setitem__(self, index: slice, value: ReadableBuffer) -> None: ...
 //|     @overload
 //|     def __setitem__(self, index: int, value: int) -> None:
 //|         """Set the value(s) at the given index.
 //|
-//|         1, 2, 4 and 8 byte writes will be done in one assignment. All others
-//|         will use memcpy."""
+//|         1, 2, 4 and 8 byte aligned writes will be done in one transaction.
+//|         All others may use multiple transactions."""
 //|         ...
 //|
 STATIC mp_obj_t memorymap_addressrange_subscr(mp_obj_t self_in, mp_obj_t index_in, mp_obj_t value) {