samd/machine_pwm: Add init() method to PWM and simplify the PWM code.

The PWM.init() method has been added.  Calling init() without arguments
restarts a PWM channel stopped with deinit().  Otherwise single parameters
except for "device=n" can be changed again.  The device can only be
specified once, either in the constructor or the first init() call.

Also simplify get_pwm_config() and get_adc_config(), and shrink the PWM
object.

4 files changed, 118 insertions, 136 deletions

diff --git a/docs/samd/quickref.rst b/docs/samd/quickref.rst
index cdfb0a475..7a0786d4c 100644
--- a/docs/samd/quickref.rst
+++ b/docs/samd/quickref.rst
@@ -191,7 +191,7 @@ It supports all basic methods listed for that class. ::
 PWM Constructor
 ```````````````
 
-.. class:: PWM(dest, freq, duty_u16, duty_ns, *, invert, device)
+.. class:: PWM(dest, *, freq, duty_u16, duty_ns, invert, device)
   :noindex:
 
     Construct and return a new PWM object using the following parameters:
diff --git a/ports/samd/machine_pwm.c b/ports/samd/machine_pwm.c
index f6b417631..d01ce2ff1 100644
--- a/ports/samd/machine_pwm.c
+++ b/ports/samd/machine_pwm.c
@@ -40,15 +40,14 @@
 typedef struct _machine_pwm_obj_t {
     mp_obj_base_t base;
     Tcc *instance;
+    bool defer_start;
     uint8_t pin_id;
     uint8_t alt_fct;
-    uint8_t device;
+    int8_t device;
     uint8_t channel;
     uint8_t output;
     uint16_t prescaler;
-    uint32_t period;  // full period count ticks
-    uint32_t duty_ns; // just for reporting
-    uint16_t duty_u16; // just for reporting
+    int32_t freq;    // for re-init.
 } machine_pwm_obj_t;
 
 #define PWM_NOT_INIT    (0)
@@ -58,6 +57,8 @@ typedef struct _machine_pwm_obj_t {
 #define PWM_FULL_SCALE  (65536)
 #define PWM_UPDATE_TIMEOUT (2000)
 
+#define VALUE_NOT_SET    (-1)
+
 static Tcc *tcc_instance[] = TCC_INSTS;
 
 #if defined(MCU_SAMD21)
@@ -104,10 +105,12 @@ static uint8_t device_status[TCC_INST_NUM];
 static uint8_t output_active[TCC_INST_NUM];
 const uint16_t prescaler_table[] = {1, 2, 4, 8, 16, 64, 256, 1024};
 
-STATIC void pwm_stop_device(int device);
 STATIC void mp_machine_pwm_freq_set(machine_pwm_obj_t *self, mp_int_t freq);
 STATIC void mp_machine_pwm_duty_set_u16(machine_pwm_obj_t *self, mp_int_t duty_u16);
 STATIC void mp_machine_pwm_duty_set_ns(machine_pwm_obj_t *self, mp_int_t duty_ns);
+STATIC void mp_machine_pwm_start(machine_pwm_obj_t *self);
+STATIC void mp_machine_pwm_stop(machine_pwm_obj_t *self);
+
 
 STATIC void mp_machine_pwm_print(const mp_print_t *print, mp_obj_t self_in, mp_print_kind_t kind) {
     machine_pwm_obj_t *self = MP_OBJ_TO_PTR(self_in);
@@ -115,48 +118,39 @@ STATIC void mp_machine_pwm_print(const mp_print_t *print, mp_obj_t self_in, mp_p
         pin_name(self->pin_id), self->device, self->channel, self->output);
 }
 
-// PWM(pin)
-STATIC mp_obj_t mp_machine_pwm_make_new(const mp_obj_type_t *type, size_t n_args, size_t n_kw, const mp_obj_t *all_args) {
-    enum { ARG_pin, ARG_freq, ARG_duty_u16, ARG_duty_ns, ARG_invert, ARG_device };
+// called by the constructor and init()
+STATIC void mp_machine_pwm_init_helper(machine_pwm_obj_t *self,
+    size_t n_args, const mp_obj_t *pos_args, mp_map_t *kw_args) {
+    enum { ARG_freq, ARG_duty_u16, ARG_duty_ns, ARG_invert, ARG_device };
     static const mp_arg_t allowed_args[] = {
-        { MP_QSTR_pin,      MP_ARG_REQUIRED | MP_ARG_OBJ },
         { MP_QSTR_freq,     MP_ARG_INT, {.u_int = -1} },
-        { MP_QSTR_duty_u16, MP_ARG_KW_ONLY | MP_ARG_INT, {.u_int = -1} },
-        { MP_QSTR_duty_ns,  MP_ARG_KW_ONLY | MP_ARG_INT, {.u_int = -1} },
-        { MP_QSTR_invert,   MP_ARG_KW_ONLY | MP_ARG_INT, {.u_int = -1} },
-        { MP_QSTR_device,   MP_ARG_KW_ONLY | MP_ARG_INT, {.u_int = -1} },
+        { MP_QSTR_duty_u16, MP_ARG_KW_ONLY | MP_ARG_INT, {.u_int = VALUE_NOT_SET} },
+        { MP_QSTR_duty_ns,  MP_ARG_KW_ONLY | MP_ARG_INT, {.u_int = VALUE_NOT_SET} },
+        { MP_QSTR_invert,   MP_ARG_KW_ONLY | MP_ARG_INT, {.u_int = VALUE_NOT_SET} },
+        { MP_QSTR_device,   MP_ARG_KW_ONLY | MP_ARG_INT, {.u_int = VALUE_NOT_SET} },
     };
 
     // Parse the arguments.
     mp_arg_val_t args[MP_ARRAY_SIZE(allowed_args)];
-    mp_arg_parse_all_kw_array(n_args, n_kw, all_args, MP_ARRAY_SIZE(allowed_args), allowed_args, args);
-
-    // Get GPIO and optional device to connect to PWM.
-    uint32_t pin_id = mp_hal_get_pin_obj(args[ARG_pin].u_obj);
-    int32_t wanted_dev = args[ARG_device].u_int; // -1 = any
-
-    // Get the peripheral object and populate it
-
-    pwm_config_t config = get_pwm_config(pin_id, wanted_dev, device_status);
-    uint8_t device = config.device_channel >> 4;
-    if (device >= TCC_INST_NUM) {
-        mp_raise_ValueError(MP_ERROR_TEXT("wrong device"));
+    mp_arg_parse_all(n_args, pos_args, kw_args,
+        MP_ARRAY_SIZE(allowed_args), allowed_args, args);
+
+    int8_t device = self->device;
+    if (device == VALUE_NOT_SET) { // Device not set, just get & set
+        int32_t wanted_dev = args[ARG_device].u_int; // -1 = any
+        pwm_config_t config = get_pwm_config(self->pin_id, wanted_dev, device_status);
+        device = config.device_channel >> 4;
+        self->instance = tcc_instance[device];
+        self->device = device;
+        self->alt_fct = config.alt_fct;
+        self->channel = (config.device_channel & 0x0f) % tcc_channel_count[device];
+        self->output = config.device_channel & 0x0f;
+        put_duty_value(device, self->channel, 0);
     }
 
-    machine_pwm_obj_t *self = mp_obj_malloc(machine_pwm_obj_t, &machine_pwm_type);
-    self->instance = tcc_instance[device];
-    self->device = device;
-    self->pin_id = pin_id;
-    self->alt_fct = config.alt_fct;
-    self->channel = (config.device_channel & 0x0f) % tcc_channel_count[device];
-    self->output = config.device_channel & 0x0f;
-    self->prescaler = 1;
-    self->period = 1; // Use an invalid but safe value
-    self->duty_u16 = self->duty_ns = 0;
-    put_duty_value(self->device, self->channel, 0);
-
     Tcc *tcc = self->instance;
 
+    // Initialize the hardware if needed
     if (device_status[device] == PWM_NOT_INIT) {
         // Enable the device clock at first use.
         #if defined(MCU_SAMD21)
@@ -203,10 +197,11 @@ STATIC mp_obj_t mp_machine_pwm_make_new(const mp_obj_type_t *type, size_t n_args
         device_status[device] = PWM_CLK_READY;
     }
 
-    if (args[ARG_invert].u_int != -1) {
+    self->defer_start = true;
+    if (args[ARG_invert].u_int != VALUE_NOT_SET) {
         bool invert = !!args[ARG_invert].u_int;
         if (device_status[device] != PWM_CLK_READY) {
-            pwm_stop_device(device);
+            mp_machine_pwm_stop(self);
         }
         uint32_t mask = 1 << (self->output + TCC_DRVCTRL_INVEN0_Pos);
         if (invert) {
@@ -215,24 +210,47 @@ STATIC mp_obj_t mp_machine_pwm_make_new(const mp_obj_type_t *type, size_t n_args
             tcc->DRVCTRL.reg &= ~(mask);
         }
     }
-    if (args[ARG_duty_u16].u_int != -1) {
+    if (args[ARG_freq].u_int != VALUE_NOT_SET) {
+        mp_machine_pwm_freq_set(self, args[ARG_freq].u_int);
+    }
+    if (args[ARG_duty_u16].u_int != VALUE_NOT_SET) {
         mp_machine_pwm_duty_set_u16(self, args[ARG_duty_u16].u_int);
     }
-    if (args[ARG_duty_ns].u_int != -1) {
+    if (args[ARG_duty_ns].u_int != VALUE_NOT_SET) {
         mp_machine_pwm_duty_set_ns(self, args[ARG_duty_ns].u_int);
     }
-    if (args[ARG_freq].u_int != -1) {
-        mp_machine_pwm_freq_set(self, args[ARG_freq].u_int);
-    }
+    self->defer_start = false;
+    // Start the PWM if properly set.
+    mp_machine_pwm_start(self);
+}
+
+// PWM(pin)
+STATIC mp_obj_t mp_machine_pwm_make_new(const mp_obj_type_t *type, size_t n_args, size_t n_kw, const mp_obj_t *args) {
+
+    // Check number of arguments
+    mp_arg_check_num(n_args, n_kw, 1, MP_OBJ_FUN_ARGS_MAX, true);
+
+    // Get the peripheral object and populate it
+    machine_pwm_obj_t *self = mp_obj_malloc(machine_pwm_obj_t, &machine_pwm_type);
+    self->pin_id = mp_hal_get_pin_obj(args[0]);
+    self->device = VALUE_NOT_SET;
+    self->prescaler = 1;
+    self->freq = VALUE_NOT_SET;
+
+    // Process the remaining parameters.
+    mp_map_t kw_args;
+    mp_map_init_fixed_table(&kw_args, n_kw, args + n_args);
+    mp_machine_pwm_init_helper(self, n_args - 1, args + 1, &kw_args);
+
     return MP_OBJ_FROM_PTR(self);
 }
 
-STATIC void pwm_stop_device(int device) {
-    Tcc *tcc = tcc_instance[device];
+STATIC void mp_machine_pwm_stop(machine_pwm_obj_t *self) {
+    Tcc *tcc = tcc_instance[self->device];
     tcc->CTRLA.bit.ENABLE = 0;
     while (tcc->SYNCBUSY.reg & TCC_SYNCBUSY_ENABLE) {
     }
-    device_status[device] = PWM_CLK_READY;
+    device_status[self->device] = PWM_CLK_READY;
 }
 
 // Stop all TTC devices
@@ -252,13 +270,13 @@ void pwm_deinit_all(void) {
 // Switch off an output. If all outputs of a device are off,
 // switch off that device.
 // This stops all channels, but keeps the configuration
-// Calling pwm.freq(n) will start an instance again.
+// Calling pwm.freq(n), pwm.duty_x() or pwm.init() will start it again.
 STATIC void mp_machine_pwm_deinit(machine_pwm_obj_t *self) {
     mp_hal_clr_pin_mux(self->pin_id); // Switch the output off
     output_active[self->device] &= ~(1 << self->output);  // clear output flasg
     // Stop the device, if no output is active.
     if (output_active[self->device] == 0) {
-        pwm_stop_device(self->device);
+        mp_machine_pwm_stop(self);
     }
 }
 
@@ -275,80 +293,52 @@ STATIC void wait_for_register_update(Tcc *tcc) {
     tcc->INTFLAG.reg = TCC_INTFLAG_OVF;
 }
 
-STATIC mp_obj_t mp_machine_pwm_freq_get(machine_pwm_obj_t *self) {
-    if (self->instance->CTRLA.reg & TCC_CTRLA_ENABLE) {
-        return MP_OBJ_NEW_SMALL_INT(PWM_MASTER_CLK / self->prescaler / self->period);
-    } else {
-        return MP_OBJ_NEW_SMALL_INT(0);
-    }
-}
-
-STATIC void mp_machine_pwm_freq_set(machine_pwm_obj_t *self, mp_int_t freq) {
-    // Set the frequency. The period counter is 24 bit or 16 bit with a pre-scaling
+STATIC void mp_machine_pwm_start(machine_pwm_obj_t *self) {
+    // Start the PWM. The period counter is 24 bit or 16 bit with a pre-scaling
     // of up to 1024, allowing a range from 24 MHz down to 1 Hz.
     static const uint32_t max_period[5] = {1 << 24, 1 << 24, 1 << 16, 1 << 16, 1 << 16};
 
-    Tcc *tcc = self->instance;
-    if (freq < 1) {
-        pwm_stop_device(self->device);
+    if (self->freq < 1 || self->defer_start == true) {
         return;
     }
-
-    // Get the actual settings of prescaler & period from the unit
-    // To be able for cope for changes.
-    uint32_t prev_period = tcc->PER.reg + 1;
+    Tcc *tcc = self->instance;
 
     // Check for the right prescaler
     uint8_t index;
     for (index = 0; index < 8; index++) {
-        uint32_t temp = PWM_MASTER_CLK / prescaler_table[index] / freq;
+        uint32_t temp = PWM_MASTER_CLK / prescaler_table[index] / self->freq;
         if (temp < max_period[self->device]) {
             break;
         }
     }
     self->prescaler = prescaler_table[index];
 
-    uint32_t period = PWM_MASTER_CLK / self->prescaler / freq;
+    uint32_t period = PWM_MASTER_CLK / self->prescaler / self->freq;
     if (period < 2) {
         mp_raise_ValueError(MP_ERROR_TEXT("freq too large"));
     }
     // If the PWM is running, ensure that a cycle has passed since the
-    // previous setting before setting a new frequency/duty value
+    // previous setting before setting frequency and duty.
     if (tcc->CTRLA.reg & TCC_CTRLA_ENABLE) {
         wait_for_register_update(tcc);
     }
     // Check, if the prescaler has to be changed and stop the device if so.
     if (index != tcc->CTRLA.bit.PRESCALER) {
-        // stop the device
-        pwm_stop_device(self->device);
-        // update the prescaler
+        mp_machine_pwm_stop(self);
         tcc->CTRLA.bit.PRESCALER = index;
     }
     // Lock the update to get a glitch-free change of period and duty cycle
     tcc->CTRLBSET.reg = TCC_CTRLBSET_LUPD;
     tcc->PERBUF.reg = period - 1;
-    self->period = period;
-
-    // Check if the Duty rate has to be aligned again when freq or prescaler were changed.
-    // This condition is as well true on first call after instantiation. So (re-)configure
-    // all channels with a duty_u16 setting.
-    if (period != prev_period) {
-        for (uint16_t ch = 0; ch < tcc_channel_count[self->device]; ch++) {
-            if ((duty_type_flags[self->device] & (1 << ch)) != 0) {  // duty_u16 type?
-                tcc->CCBUF[ch].reg = (uint64_t)get_duty_value(self->device, ch) * period /
-                    PWM_FULL_SCALE;
-            }
-        }
-    }
-    // If the prescaler was changed, the device is disabled. So this condition is true
-    // after the instantiation and after a prescaler change.
-    // (re-)configure all channels with a duty_ns setting.
-    if (!(tcc->CTRLA.reg & TCC_CTRLA_ENABLE)) {
-        for (uint16_t ch = 0; ch < tcc_channel_count[self->device]; ch++) {
-            if ((duty_type_flags[self->device] & (1 << ch)) == 0) {  // duty_ns type?
-                tcc->CCBUF[ch].reg = (uint64_t)get_duty_value(self->device, ch) * PWM_MASTER_CLK /
-                    self->prescaler / 1000000000ULL;
-            }
+
+    // (re-) configure the duty type settings.
+    for (uint16_t ch = 0; ch < tcc_channel_count[self->device]; ch++) {
+        if ((duty_type_flags[self->device] & (1 << ch)) != 0) {  // duty_u16 type?
+            tcc->CCBUF[ch].reg = (uint64_t)get_duty_value(self->device, ch) * period /
+                PWM_FULL_SCALE;
+        } else { // duty_ns type
+            tcc->CCBUF[ch].reg = (uint64_t)get_duty_value(self->device, ch) * PWM_MASTER_CLK /
+                self->prescaler / 1000000000ULL;
         }
     }
     // Remember the output as active.
@@ -367,38 +357,39 @@ STATIC void mp_machine_pwm_freq_set(machine_pwm_obj_t *self, mp_int_t freq) {
     tcc->CTRLBCLR.reg = TCC_CTRLBCLR_LUPD;
 }
 
+STATIC mp_obj_t mp_machine_pwm_freq_get(machine_pwm_obj_t *self) {
+    return MP_OBJ_NEW_SMALL_INT(self->freq);
+}
+
+STATIC void mp_machine_pwm_freq_set(machine_pwm_obj_t *self, mp_int_t freq) {
+    self->freq = freq;
+    mp_machine_pwm_start(self);
+}
+
 STATIC mp_obj_t mp_machine_pwm_duty_get_u16(machine_pwm_obj_t *self) {
-    return MP_OBJ_NEW_SMALL_INT(self->duty_u16);
+    if (duty_type_flags[self->device] & (1 << self->channel)) {
+        return MP_OBJ_NEW_SMALL_INT(get_duty_value(self->device, self->channel));
+    } else {
+        return MP_OBJ_NEW_SMALL_INT(-1);
+    }
 }
 
 STATIC void mp_machine_pwm_duty_set_u16(machine_pwm_obj_t *self, mp_int_t duty_u16) {
-    // Remember the values for update & reporting
     put_duty_value(self->device, self->channel, duty_u16);
-    self->duty_u16 = duty_u16;
-    self->duty_ns = 0;
-    // If the device is enabled, than the period is set and we get a reasonable value for
-    // the duty cycle, set to the CCBUF register. Otherwise, PWM does not start.
-    if (self->instance->CTRLA.reg & TCC_CTRLA_ENABLE) {
-        // Ensure that a cycle has passed updating the registers
-        // since the previous setting before setting a new duty value
-        wait_for_register_update(self->instance);
-        self->instance->CCBUF[self->channel].reg = (uint64_t)duty_u16 * (self->period) / PWM_FULL_SCALE;
-    }
     duty_type_flags[self->device] |= 1 << self->channel;
+    mp_machine_pwm_start(self);
 }
 
 STATIC mp_obj_t mp_machine_pwm_duty_get_ns(machine_pwm_obj_t *self) {
-    return MP_OBJ_NEW_SMALL_INT(self->duty_ns);
+    if (!(duty_type_flags[self->device] & (1 << self->channel))) {
+        return MP_OBJ_NEW_SMALL_INT(get_duty_value(self->device, self->channel));
+    } else {
+        return MP_OBJ_NEW_SMALL_INT(-1);
+    }
 }
 
 STATIC void mp_machine_pwm_duty_set_ns(machine_pwm_obj_t *self, mp_int_t duty_ns) {
-    // Remember the values for update & reporting
     put_duty_value(self->device, self->channel, duty_ns);
-    self->duty_ns = duty_ns;
-    self->duty_u16 = 0;
-    // Ensure that a cycle has passed updating the registers
-    // since the previous setting before setting a new duty value
-    wait_for_register_update(self->instance);
-    self->instance->CCBUF[self->channel].reg = (uint64_t)duty_ns * PWM_MASTER_CLK / self->prescaler / 1000000000ULL;
     duty_type_flags[self->device] &= ~(1 << self->channel);
+    mp_machine_pwm_start(self);
 }
diff --git a/ports/samd/mpconfigport.h b/ports/samd/mpconfigport.h
index 779bc4713..6ed8186ec 100644
--- a/ports/samd/mpconfigport.h
+++ b/ports/samd/mpconfigport.h
@@ -107,7 +107,7 @@
 #define MICROPY_PY_MACHINE_BITSTREAM        (1)
 #define MICROPY_PY_MACHINE_PULSE            (1)
 #define MICROPY_PY_MACHINE_PWM              (1)
-#define MICROPY_PY_MACHINE_PWM_INIT         (0)
+#define MICROPY_PY_MACHINE_PWM_INIT         (1)
 #define MICROPY_PY_MACHINE_PWM_DUTY_U16_NS  (1)
 #define MICROPY_PY_MACHINE_PWM_INCLUDEFILE  "ports/samd/machine_pwm.c"
 #define MICROPY_PY_MACHINE_PIN_MAKE_NEW     mp_pin_make_new
diff --git a/ports/samd/pin_af.c b/ports/samd/pin_af.c
index 2664fd410..15743d1df 100644
--- a/ports/samd/pin_af.c
+++ b/ports/samd/pin_af.c
@@ -142,11 +142,6 @@ sercom_pad_config_t get_sercom_config(int pin_id, uint8_t sercom_nr) {
 
 adc_config_t get_adc_config(int pin_id, int32_t flag) {
     const machine_pin_obj_t *pct_ptr = get_pin_obj_ptr(pin_id);
-    #if defined(MCU_SAMD51)
-    if (pct_ptr->adc1 != 0xff && (flag & (1 << (pct_ptr->adc1 + 16))) == 0) {
-        return (adc_config_t) {1, pct_ptr->adc1};
-    } else
-    #endif
     if (pct_ptr->adc0 != 0xff && (flag & (1 << pct_ptr->adc0)) == 0) {
         return (adc_config_t) {0, pct_ptr->adc0};
     #if defined(MUC_SAMD51)
@@ -174,26 +169,22 @@ pwm_config_t get_pwm_config(int pin_id, int wanted_dev, uint8_t device_status[])
             return (pwm_config_t) {ALT_FCT_TCC1, tcc1};
         } else if ((tcc2 >> 4) == wanted_dev) {
             return (pwm_config_t) {ALT_FCT_TCC2, tcc2};
-        } else {
-            mp_raise_ValueError(MP_ERROR_TEXT("wrong device or channel"));
         }
     } else {
-        pwm_config_t ret = {};
+        // Try to get a unused PWM device at the pin
+        if (((tcc1 >> 4) < TCC_INST_NUM) && (device_status[tcc1 >> 4] == 0)) {
+            return (pwm_config_t) {ALT_FCT_TCC1, tcc1};
+        }
+        if (((tcc2 >> 4) < TCC_INST_NUM) && (device_status[tcc2 >> 4] == 0)) {
+            return (pwm_config_t) {ALT_FCT_TCC2, tcc2};
+        }
+        // If all devices are used, return one from the pin if available
         if ((tcc1 >> 4) < TCC_INST_NUM) {
-            ret = (pwm_config_t) {ALT_FCT_TCC1, tcc1};
-            if (tcc2 == 0xff) {
-                return ret;
-            }
+            return (pwm_config_t) {ALT_FCT_TCC1, tcc1};
         }
         if ((tcc2 >> 4) < TCC_INST_NUM) {
-            // if a device in slot 1 is not available or already in use, use the one in slot 2
-            if (tcc1 == 0xff || device_status[(ret.device_channel >> 4)] != 0) {
-                return (pwm_config_t) {ALT_FCT_TCC2, tcc2};
-            } else {
-                return ret;
-            }
-        } else {
-            mp_raise_ValueError(MP_ERROR_TEXT("not a PWM pin"));
+            return (pwm_config_t) {ALT_FCT_TCC2, tcc2};
         }
     }
+    mp_raise_ValueError(MP_ERROR_TEXT("not a PWM Pin"));
 }