kingst-la2016: more checks on configured rate/depth/channels

Extend the list of supported samplerates, start from 10kHz (used to
start at 20kHz before). Comment on hardware constraints which limit
the available rates, for awareness during maintenance.

Move the check for enabled channels from api.c to protocol.c, it was
only used for the trigger configuration. Rename the routine since it
never configured any channels, only got the list of currently enabled
channels to pass on to the device. Extend the range checks for the
rate and depth on acquisition start, also check the lower limits.

Moving codes which references declarations required that definitions
also moved to public location of greater scope. This is acceptable since
all of them remain local to the kingst-la2016 driver. Stick with names
as they are used right now to reduce the diff and help reviewers, these
can get unified in a later commit.

diff --git a/src/hardware/kingst-la2016/api.c b/src/hardware/kingst-la2016/api.c
index 45262c1eb48bae88475e352df40499da9a7f26d6..f11c21ecd30a6d18201c0fff9c7cab0dd14f6531 100644 (file)
--- a/src/hardware/kingst-la2016/api.c
+++ b/src/hardware/kingst-la2016/api.c
@@ -65,7 +65,17 @@ static const char *channel_names[] = {
        "CH8", "CH9", "CH10", "CH11", "CH12", "CH13", "CH14", "CH15",
 };
 
+/*
+ * The hardware uses a (model dependent) 100/200/500MHz base clock and
+ * a 16bit divider (common across all models). The range from 10kHz to
+ * 100/200/500MHz should be applicable to all devices. High rates may
+ * suffer from coarse resolution (e.g. in the "500MHz div 2" case) and
+ * may not provide the desired 1/2/5 steps. This is not an issue now,
+ * the 500MHz model is not supported yet by this driver.
+ */
+
 static const uint64_t samplerates_la2016[] = {
+       SR_KHZ(10),
        SR_KHZ(20),
        SR_KHZ(50),
        SR_KHZ(100),
@@ -84,6 +94,7 @@ static const uint64_t samplerates_la2016[] = {
 };
 
 static const uint64_t samplerates_la1016[] = {
+       SR_KHZ(10),
        SR_KHZ(20),
        SR_KHZ(50),
        SR_KHZ(100),
@@ -571,25 +582,6 @@ static int config_list(uint32_t key, GVariant **data,
        return SR_OK;
 }
 
-static int configure_channels(const struct sr_dev_inst *sdi)
-{
-       struct dev_context *devc;
-       GSList *l;
-       struct sr_channel *ch;
-
-       devc = sdi->priv;
-
-       devc->cur_channels = 0;
-       for (l = sdi->channels; l; l = l->next) {
-               ch = l->data;
-               if (!ch->enabled)
-                       continue;
-               devc->cur_channels |= 1UL << ch->index;
-       }
-
-       return SR_OK;
-}
-
 static int dev_acquisition_start(const struct sr_dev_inst *sdi)
 {
        struct sr_dev_driver *di;
@@ -603,11 +595,6 @@ static int dev_acquisition_start(const struct sr_dev_inst *sdi)
        ctx = drvc->sr_ctx;;
        devc = sdi->priv;
 
-       if (configure_channels(sdi) != SR_OK) {
-               sr_err("Cannot configure channels.");
-               return SR_ERR;
-       }
-
        devc->convbuffer_size = LA2016_CONVBUFFER_SIZE;
        devc->convbuffer = g_try_malloc(devc->convbuffer_size);
        if (!devc->convbuffer) {

diff --git a/src/hardware/kingst-la2016/protocol.c b/src/hardware/kingst-la2016/protocol.c
index 3ef10f6ef77fbe60f827282fe75b75b41ace61ac..f21a2ba0613b75bd7130b7ad2c054a92bb8031b4 100644 (file)
--- a/src/hardware/kingst-la2016/protocol.c
+++ b/src/hardware/kingst-la2016/protocol.c
@@ -34,13 +34,6 @@
 #define FPGA_FW_LA1016 "kingst-la1016-fpga.bitstream"
 #define FPGA_FW_LA1016A        "kingst-la1016a1-fpga.bitstream"
 
-/* Maximum device capabilities. May differ between models. */
-#define MAX_SAMPLE_RATE_LA2016 SR_MHZ(200)
-#define MAX_SAMPLE_RATE_LA1016 SR_MHZ(100)
-#define MAX_SAMPLE_DEPTH       10e9
-#define MAX_PWM_FREQ           SR_MHZ(20)
-#define PWM_CLOCK              SR_MHZ(200)     /* 200MHz for both LA2016 and LA1016 */
-
 /*
  * Default device configuration. Must be applicable to any of the
  * supported devices (no model specific default values yet). Specific
@@ -51,14 +44,6 @@
 #define LA2016_DFLT_SAMPLEDEPTH        (5 * 1000 * 1000)
 #define LA2016_DFLT_CAPT_RATIO 5 /* Capture ratio, in percent. */
 
-/* TODO
- * What is the origin and motivation of that 128Mi literal? What is its
- * unit? How does it relate to a device's hardware capabilities? How to
- * map the 1GiB of RAM of an LA2016 (at 16 channels) to the 128Mi value?
- * It cannot be sample count. Is it memory size in bytes perhaps?
- */
-#define LA2016_PRE_MEM_LIMIT_BASE      (128 * 1024 * 1024)
-
 /* USB vendor class control requests, executed by the Cypress FX2 MCU. */
 #define CMD_FPGA_ENABLE        0x10
 #define CMD_FPGA_SPI   0x20    /* R/W access to FPGA registers via SPI. */
@@ -514,6 +499,25 @@ static int set_defaults(const struct sr_dev_inst *sdi)
        return SR_OK;
 }
 
+static uint16_t get_channels_mask(const struct sr_dev_inst *sdi)
+{
+       uint16_t channels;
+       GSList *l;
+       struct sr_channel *ch;
+
+       channels = 0;
+       for (l = sdi->channels; l; l = l->next) {
+               ch = l->data;
+               if (ch->type != SR_CHANNEL_LOGIC)
+                       continue;
+               if (!ch->enabled)
+                       continue;
+               channels |= 1UL << ch->index;
+       }
+
+       return channels;
+}
+
 static int set_trigger_config(const struct sr_dev_inst *sdi)
 {
        struct dev_context *devc;
@@ -533,7 +537,7 @@ static int set_trigger_config(const struct sr_dev_inst *sdi)
 
        memset(&cfg, 0, sizeof(cfg));
 
-       cfg.channels = devc->cur_channels;
+       cfg.channels = get_channels_mask(sdi);
 
        if (trigger && trigger->stages) {
                stages = trigger->stages;
@@ -623,7 +627,12 @@ static int set_sample_config(const struct sr_dev_inst *sdi)
        if (devc->cur_samplerate > devc->max_samplerate) {
                sr_err("Too high a sample rate: %" PRIu64 ".",
                        devc->cur_samplerate);
-               return SR_ERR;
+               return SR_ERR_ARG;
+       }
+       if (devc->cur_samplerate < MIN_SAMPLE_RATE_LA2016) {
+               sr_err("Too low a sample rate: %" PRIu64 ".",
+                       devc->cur_samplerate);
+               return SR_ERR_ARG;
        }
 
        clock_divisor = devc->max_samplerate / (double)devc->cur_samplerate;
@@ -632,11 +641,16 @@ static int set_sample_config(const struct sr_dev_inst *sdi)
        divider_u16 = (uint16_t)(clock_divisor + 0.5);
        devc->cur_samplerate = devc->max_samplerate / divider_u16;
 
-       if (devc->limit_samples > MAX_SAMPLE_DEPTH) {
+       if (devc->limit_samples > LA2016_NUM_SAMPLES_MAX) {
                sr_err("Too high a sample depth: %" PRIu64 ".",
                        devc->limit_samples);
                return SR_ERR;
        }
+       if (devc->limit_samples < LA2016_NUM_SAMPLES_MIN) {
+               sr_err("Too low a sample depth: %" PRIu64 ".",
+                       devc->limit_samples);
+               return SR_ERR;
+       }
 
        /*
         * The acquisition configuration communicates "pre-trigger"

diff --git a/src/hardware/kingst-la2016/protocol.h b/src/hardware/kingst-la2016/protocol.h
index d5c967ad053c69889601b79bf973f6d7c850746b..9eb483ce62650c84c899cc39090eb8c3e43a5ca8 100644 (file)
--- a/src/hardware/kingst-la2016/protocol.h
+++ b/src/hardware/kingst-la2016/protocol.h
@@ -69,8 +69,23 @@
 #define LA2016_THR_VOLTAGE_MIN 0.40
 #define LA2016_THR_VOLTAGE_MAX 4.00
 
-#define LA2016_NUM_SAMPLES_MIN 256
-#define LA2016_NUM_SAMPLES_MAX (10UL * 1000 * 1000 * 1000)
+#define LA2016_NUM_SAMPLES_MIN (UINT64_C(256))
+#define LA2016_NUM_SAMPLES_MAX (UINT64_C(10 * 1000 * 1000 * 1000))
+
+/* Maximum device capabilities. May differ between models. */
+#define MAX_SAMPLE_RATE_LA2016 SR_MHZ(200)
+#define MAX_SAMPLE_RATE_LA1016 SR_MHZ(100)
+#define MIN_SAMPLE_RATE_LA2016 SR_KHZ(10)
+#define MAX_PWM_FREQ           SR_MHZ(20)
+#define PWM_CLOCK              SR_MHZ(200)     /* 200MHz for both LA2016 and LA1016 */
+
+/* TODO
+ * What is the origin and motivation of that 128Mi literal? What is its
+ * unit? How does it relate to a device's hardware capabilities? How to
+ * map the 1GiB of RAM of an LA2016 (at 16 channels) to the 128Mi value?
+ * It cannot be sample count. Is it memory size in bytes perhaps?
+ */
+#define LA2016_PRE_MEM_LIMIT_BASE      (128 * 1024 * 1024)
 
 #define LA2016_NUM_PWMCH_MAX   2
 
@@ -114,7 +129,6 @@ struct dev_context {
        uint64_t cur_samplerate;
        uint64_t limit_samples;
        uint64_t capture_ratio;
-       uint16_t cur_channels;
 
        /* Internal acquisition and download state. */
        gboolean trigger_involved;