]> sigrok.org Git - libsigrok.git/commitdiff
kingst-la2016: rework the device side of PWM configuration
authorGerhard Sittig <redacted>
Sun, 30 Jan 2022 09:25:41 +0000 (10:25 +0100)
committerGerhard Sittig <redacted>
Sun, 6 Feb 2022 17:53:53 +0000 (18:53 +0100)
Assign an initial PWM configuration during scan (arbitrary frequencies
at 50% duty cycle as before, but off by default). Ideally the initial
assignment gets updated by user specs at runtime, and hardware gets
configured as needed from internal data which was kept at defaults or
updated from user specs.

Combine the previous enable_pwm() and configure_pwm() routines into
set_pwm_config() which configures one channel in the device's hardware
(and transparently controls the enable flag). This eliminates the
unfortunate "channel unrolling" in the helper's API. Which eliminates
la2016_init_params() which redundantly configured the logic threshold
(done upon acquisition start again) and kept reassigning PWM parameters
across multiple open calls.

Providing the config API to adjust PWM settings remains to be done.

src/hardware/kingst-la2016/api.c
src/hardware/kingst-la2016/protocol.c
src/hardware/kingst-la2016/protocol.h

index 8f8afef8728f4c1468d6ee798018ccad4994f4cb..7a963a2b43ab4c10f6d7f73f29c04518e54b8142 100644 (file)
@@ -567,12 +567,27 @@ static GSList *scan(struct sr_dev_driver *di, GSList *options)
                        ch_off++;
                }
 
+               /*
+                * Ideally we'd get the previous configuration from the
+                * hardware, but this device is write-only. So we have
+                * to assign a fixed set of initial configuration values.
+                */
                sr_sw_limits_init(&devc->sw_limits);
                devc->sw_limits.limit_samples = 0;
                devc->capture_ratio = 50;
                devc->cur_samplerate = devc->model->samplerate;
                devc->threshold_voltage_idx = 0;
                devc->threshold_voltage = logic_threshold_value[devc->threshold_voltage_idx];
+               if  (ARRAY_SIZE(devc->pwm_setting) >= 1) {
+                       devc->pwm_setting[0].enabled = FALSE;
+                       devc->pwm_setting[0].freq = SR_KHZ(1);
+                       devc->pwm_setting[0].duty = 50;
+               }
+               if  (ARRAY_SIZE(devc->pwm_setting) >= 2) {
+                       devc->pwm_setting[1].enabled = FALSE;
+                       devc->pwm_setting[1].freq = SR_KHZ(100);
+                       devc->pwm_setting[1].duty = 50;
+               }
 
                sdi->status = SR_ST_INACTIVE;
                devices = g_slist_append(devices, sdi);
@@ -584,7 +599,11 @@ static GSList *scan(struct sr_dev_driver *di, GSList *options)
 
 static int dev_open(struct sr_dev_inst *sdi)
 {
+       struct dev_context *devc;
        int ret;
+       size_t ch;
+
+       devc = sdi->priv;
 
        ret = la2016_open_enum(sdi);
        if (ret != SR_OK) {
@@ -592,14 +611,11 @@ static int dev_open(struct sr_dev_inst *sdi)
                return ret;
        }
 
-       /*
-        * Setup a default configuration of device features. This
-        * affects the logic threshold, PWM channels, and similar.
-        */
-       ret = la2016_init_params(sdi);
-       if (ret != SR_OK) {
-               sr_err("Cannot initialize device's hardware.");
-               return ret;
+       /* Send most recent PWM configuration to the device. */
+       for (ch = 0; ch < ARRAY_SIZE(devc->pwm_setting); ch++) {
+               ret = la2016_write_pwm_config(sdi, ch);
+               if (ret != SR_OK)
+                       return ret;
        }
 
        return SR_OK;
index 9ecaa7180961d4c18299705a4916c7a8b23ba24a..f7e2f208d18d4ee2f5faba7bb95545b13b5a5397 100644 (file)
@@ -392,110 +392,101 @@ static int set_threshold_voltage(const struct sr_dev_inst *sdi, float voltage)
        return SR_OK;
 }
 
-static int enable_pwm(const struct sr_dev_inst *sdi, gboolean p1, gboolean p2)
-{
-       struct dev_context *devc;
-       uint8_t cfg;
-       int ret;
-
-       devc = sdi->priv;
-
-       cfg = 0;
-       if (p1)
-               cfg |= 1U << 0;
-       if (p2)
-               cfg |= 1U << 1;
-       sr_dbg("Set PWM enable %d %d. Config 0x%02x.", p1, p2, cfg);
-
-       ret = ctrl_out(sdi, CMD_FPGA_SPI, REG_PWM_EN, 0, &cfg, sizeof(cfg));
-       if (ret != SR_OK) {
-               sr_err("Cannot setup PWM enabled state.");
-               return ret;
-       }
-
-       devc->pwm_setting[0].enabled = (p1) ? 1 : 0;
-       devc->pwm_setting[1].enabled = (p2) ? 1 : 0;
-
-       return SR_OK;
-}
-
-static int configure_pwm(const struct sr_dev_inst *sdi, uint8_t which,
-       float freq, float duty)
+/*
+ * Communicates a channel's configuration to the device after the
+ * parameters may have changed. Configuration of one channel may
+ * interfere with other channels since they share FPGA registers.
+ */
+static int set_pwm_config(const struct sr_dev_inst *sdi, size_t idx)
 {
-       static uint8_t ctrl_reg_tab[] = { REG_PWM1, REG_PWM2, };
+       static uint8_t reg_bases[] = { REG_PWM1, REG_PWM2, };
 
        struct dev_context *devc;
-       uint8_t ctrl_reg;
-       struct pwm_setting_dev cfg;
-       struct pwm_setting *setting;
+       struct pwm_setting *params;
+       uint8_t reg_base;
+       double val_f;
+       uint32_t val_u;
+       uint32_t period, duty;
+       size_t ch;
        int ret;
-       uint8_t buf[2 * sizeof(uint32_t)];
+       uint8_t enable_all, enable_cfg, reg_val;
+       uint8_t buf[REG_PWM2 - REG_PWM1]; /* Width of one REG_PWMx. */
        uint8_t *wrptr;
 
        devc = sdi->priv;
+       if (idx >= ARRAY_SIZE(devc->pwm_setting))
+               return SR_ERR_ARG;
+       params = &devc->pwm_setting[idx];
+       if (idx >= ARRAY_SIZE(reg_bases))
+               return SR_ERR_ARG;
+       reg_base = reg_bases[idx];
 
-       if (which < 1 || which > ARRAY_SIZE(ctrl_reg_tab)) {
-               sr_err("Invalid PWM channel: %d.", which);
-               return SR_ERR;
-       }
-       if (freq < 0 || freq > MAX_PWM_FREQ) {
-               sr_err("Too high a PWM frequency: %.1f.", freq);
-               return SR_ERR;
-       }
-       if (duty < 0 || duty > 100) {
-               sr_err("Invalid PWM duty cycle: %f.", duty);
-               return SR_ERR;
+       /*
+        * Map application's specs to hardware register values. Do math
+        * in floating point initially, but convert to u32 eventually.
+        */
+       sr_dbg("PWM config, app spec, ch %zu, en %d, freq %.1f, duty %.1f.",
+               idx, params->enabled ? 1 : 0, params->freq, params->duty);
+       val_f = PWM_CLOCK;
+       val_f /= params->freq;
+       val_u = val_f;
+       period = val_u;
+       val_f = period;
+       val_f *= params->duty;
+       val_f /= 100.0;
+       val_f += 0.5;
+       val_u = val_f;
+       duty = val_u;
+       sr_dbg("PWM config, reg 0x%04x, freq %u, duty %u.",
+               (unsigned)reg_base, (unsigned)period, (unsigned)duty);
+
+       /* Get the "enabled" state of all supported PWM channels. */
+       enable_all = 0;
+       for (ch = 0; ch < ARRAY_SIZE(devc->pwm_setting); ch++) {
+               if (!devc->pwm_setting[ch].enabled)
+                       continue;
+               enable_all |= 1U << ch;
        }
+       enable_cfg = 1U << idx;
+       sr_spew("PWM config, enable all 0x%02hhx, cfg 0x%02hhx.",
+               enable_all, enable_cfg);
 
-       memset(&cfg, 0, sizeof(cfg));
-       cfg.period = (uint32_t)(PWM_CLOCK / freq);
-       cfg.duty = (uint32_t)(0.5f + (cfg.period * duty / 100.));
-       sr_dbg("Set PWM%d period %d, duty %d.", which, cfg.period, cfg.duty);
-
-       ctrl_reg = ctrl_reg_tab[which - 1];
-       wrptr = buf;
-       write_u32le_inc(&wrptr, cfg.period);
-       write_u32le_inc(&wrptr, cfg.duty);
-       ret = ctrl_out(sdi, CMD_FPGA_SPI, ctrl_reg, 0, buf, wrptr - buf);
+       /*
+        * Disable the to-get-configured channel before its parameters
+        * will change. Or disable and exit when the channel is supposed
+        * to get turned off.
+        */
+       sr_spew("PWM config, disabling before param change.");
+       reg_val = enable_all & ~enable_cfg;
+       ret = ctrl_out(sdi, CMD_FPGA_SPI, REG_PWM_EN, 0,
+               &reg_val, sizeof(reg_val));
        if (ret != SR_OK) {
-               sr_err("Cannot setup PWM%d configuration %d %d.",
-                       which, cfg.period, cfg.duty);
+               sr_err("Cannot adjust PWM enabled state.");
                return ret;
        }
+       if (!params->enabled)
+               return SR_OK;
 
-       setting = &devc->pwm_setting[which - 1];
-       setting->freq = freq;
-       setting->duty = duty;
-
-       return SR_OK;
-}
-
-static int set_defaults(const struct sr_dev_inst *sdi)
-{
-       struct dev_context *devc;
-       int ret;
-
-       devc = sdi->priv;
-
-       ret = set_threshold_voltage(sdi, devc->threshold_voltage);
-       if (ret)
-               return ret;
-
-       ret = enable_pwm(sdi, FALSE, FALSE);
-       if (ret)
-               return ret;
-
-       ret = configure_pwm(sdi, 1, SR_KHZ(1), 50);
-       if (ret)
-               return ret;
-
-       ret = configure_pwm(sdi, 2, SR_KHZ(100), 50);
-       if (ret)
+       /* Write register values to device. */
+       sr_spew("PWM config, sending new parameters.");
+       wrptr = buf;
+       write_u32le_inc(&wrptr, period);
+       write_u32le_inc(&wrptr, duty);
+       ret = ctrl_out(sdi, CMD_FPGA_SPI, reg_base, 0, buf, wrptr - buf);
+       if (ret != SR_OK) {
+               sr_err("Cannot change PWM parameters.");
                return ret;
+       }
 
-       ret = enable_pwm(sdi, TRUE, TRUE);
-       if (ret)
+       /* Enable configured channel after write completion. */
+       sr_spew("PWM config, enabling after param change.");
+       reg_val = enable_all | enable_cfg;
+       ret = ctrl_out(sdi, CMD_FPGA_SPI, REG_PWM_EN, 0,
+               &reg_val, sizeof(reg_val));
+       if (ret != SR_OK) {
+               sr_err("Cannot adjust PWM enabled state.");
                return ret;
+       }
 
        return SR_OK;
 }
@@ -1411,17 +1402,6 @@ SR_PRIV int la2016_init_hardware(const struct sr_dev_inst *sdi)
        return SR_OK;
 }
 
-SR_PRIV int la2016_init_params(const struct sr_dev_inst *sdi)
-{
-       int ret;
-
-       ret = set_defaults(sdi);
-       if (ret != SR_OK)
-               return ret;
-
-       return SR_OK;
-}
-
 SR_PRIV int la2016_deinit_hardware(const struct sr_dev_inst *sdi)
 {
        int ret;
@@ -1434,3 +1414,8 @@ SR_PRIV int la2016_deinit_hardware(const struct sr_dev_inst *sdi)
 
        return SR_OK;
 }
+
+SR_PRIV int la2016_write_pwm_config(const struct sr_dev_inst *sdi, size_t idx)
+{
+       return set_pwm_config(sdi, idx);
+}
index c308005e3535cb02581da8bea5ffe3440ede2e32..2569da50d5e72b9b6a026cf019b73c5d074305cc 100644 (file)
@@ -88,11 +88,6 @@ struct kingst_model {
        uint64_t memory_bits;   /* RAM capacity in Gbit (1, 2, 4). */
 };
 
-struct pwm_setting_dev {
-       uint32_t period;
-       uint32_t duty;
-};
-
 struct trigger_cfg {
        uint32_t channels;
        uint32_t enabled;
@@ -109,12 +104,6 @@ struct capture_info {
 #define NUM_PACKETS_IN_CHUNK   5
 #define TRANSFER_PACKET_LENGTH 16
 
-struct pwm_setting {
-       gboolean enabled;
-       float freq;
-       float duty;
-};
-
 struct dev_context {
        uint16_t usb_pid;
        char *mcu_firmware;
@@ -124,7 +113,11 @@ struct dev_context {
        const struct kingst_model *model;
 
        /* User specified parameters. */
-       struct pwm_setting pwm_setting[LA2016_NUM_PWMCH_MAX];
+       struct pwm_setting {
+               gboolean enabled;
+               float freq;
+               float duty;
+       } pwm_setting[LA2016_NUM_PWMCH_MAX];
        size_t threshold_voltage_idx;
        float threshold_voltage;
        uint64_t cur_samplerate;
@@ -156,7 +149,7 @@ SR_PRIV int la2016_receive_data(int fd, int revents, void *cb_data);
 SR_PRIV int la2016_identify_device(const struct sr_dev_inst *sdi,
        gboolean show_message);
 SR_PRIV int la2016_init_hardware(const struct sr_dev_inst *sdi);
-SR_PRIV int la2016_init_params(const struct sr_dev_inst *sdi);
 SR_PRIV int la2016_deinit_hardware(const struct sr_dev_inst *sdi);
+SR_PRIV int la2016_write_pwm_config(const struct sr_dev_inst *sdi, size_t idx);
 
 #endif