Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OLS Improvements #51

Draft
wants to merge 20 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 25 additions & 12 deletions src/hardware/openbench-logic-sniffer/protocol.c
Original file line number Diff line number Diff line change
Expand Up @@ -397,7 +397,7 @@ SR_PRIV int ols_receive_data(int fd, int revents, void *cb_data)
struct sr_datafeed_logic logic;
uint32_t sample;
int num_bytes_read;
unsigned int i, j, num_changroups;
unsigned int num_changroups;
gboolean received_a_byte;

(void)fd;
Expand All @@ -412,7 +412,7 @@ SR_PRIV int ols_receive_data(int fd, int revents, void *cb_data)
}

num_changroups = 0;
for (i = 0x20; i > 0x02; i >>= 1) {
for (uint16_t i = 0x20; i > 0x02; i >>= 1) {
if ((devc->capture_flags & i) == 0) {
num_changroups++;
}
Expand All @@ -433,7 +433,8 @@ SR_PRIV int ols_receive_data(int fd, int revents, void *cb_data)
devc->raw_sample_size);

if (devc->raw_sample_size == num_changroups) {
unsigned int samples_to_write, new_sample_buf_size;
unsigned int samples_to_write;
uint64_t new_sample_buf_size;

devc->cnt_rx_raw_samples++;
/*
Expand All @@ -458,8 +459,8 @@ SR_PRIV int ols_receive_data(int fd, int revents, void *cb_data)
sample &= ~(0x80
<< (devc->raw_sample_size -
1) * 8);
devc->rle_count = sample;
sr_dbg("RLE count: %u.",
devc->rle_count += sample;
sr_dbg("RLE count: %" PRIu64,
devc->rle_count);
devc->raw_sample_size = 0;

Expand Down Expand Up @@ -490,9 +491,9 @@ SR_PRIV int ols_receive_data(int fd, int revents, void *cb_data)
* expecting a full 32-bit sample, based on
* the number of channels.
*/
j = 0;
unsigned int j = 0;
uint8_t tmp_sample[4] = { 0, 0, 0, 0 };
for (i = 0; i < 4; i++) {
for (unsigned int i = 0; i < 4; i++) {
if (((devc->capture_flags >> 2) &
(1 << i)) == 0) {
/*
Expand All @@ -518,7 +519,7 @@ SR_PRIV int ols_receive_data(int fd, int revents, void *cb_data)
devc->cnt_samples + samples_to_write);

if (devc->sample_buf_size < new_sample_buf_size) {
unsigned int old_size = devc->sample_buf_size;
uint64_t old_size = devc->sample_buf_size;
new_sample_buf_size *= 2;
devc->sample_buf = g_try_realloc(
devc->sample_buf, new_sample_buf_size);
Expand All @@ -542,7 +543,7 @@ SR_PRIV int ols_receive_data(int fd, int revents, void *cb_data)
devc->trigger_rle_at_smpl_from_end =
devc->cnt_samples;

for (i = 0; i < samples_to_write; i++)
for (uint64_t i = 0; i < samples_to_write; i++)
memcpy(devc->sample_buf +
(devc->cnt_samples + i) * 4,
devc->raw_sample, 4);
Expand Down Expand Up @@ -570,7 +571,8 @@ SR_PRIV int ols_receive_data(int fd, int revents, void *cb_data)
* we've acquired all the samples we asked for -- we're done.
* Send the (properly-ordered) buffer to the frontend.
*/
sr_dbg("Received %d bytes, %d raw samples, %d decompressed samples.",
sr_dbg("Received %d bytes, %d raw samples, %" PRIu64
" decompressed samples.",
devc->cnt_rx_bytes, devc->cnt_rx_raw_samples,
devc->cnt_samples);

Expand All @@ -591,7 +593,7 @@ SR_PRIV int ols_receive_data(int fd, int revents, void *cb_data)
* The OLS sends its sample buffer backwards.
* Flip it back before sending it on the session bus.
*/
for (i = 0; i < devc->cnt_samples / 2; i++) {
for (uint64_t i = 0; i < devc->cnt_samples / 2; i++) {
uint8_t temp[4];
memcpy(temp, &devc->sample_buf[4 * i], 4);
memmove(&devc->sample_buf[4 * i],
Expand Down Expand Up @@ -803,7 +805,18 @@ SR_PRIV int ols_prepare_acquisition(const struct sr_dev_inst *sdi)
devc->capture_flags &= ~0x3c;
devc->capture_flags |= ~(changroup_mask << 2) & 0x3c;

/* RLE mode is always zero, for now. */
/*
* Demon Core supports RLE mode 3. In this mode, an arbitrary number of
* consecutive RLE messages can occur. The value is only sent whenever
* it changes. In contrast, mode 0 repeats the value after every RLE
* message, even if it didn't change.
*/
if (devc->device_flags & DEVICE_FLAG_IS_DEMON_CORE)
devc->capture_flags |= CAPTURE_FLAG_RLEMODE0 |
CAPTURE_FLAG_RLEMODE1;
else
devc->capture_flags &=
~(CAPTURE_FLAG_RLEMODE0 | CAPTURE_FLAG_RLEMODE1);

if (ols_send_longdata(serial, CMD_SET_FLAGS, devc->capture_flags) != SR_OK)
return SR_ERR;
Expand Down
4 changes: 2 additions & 2 deletions src/hardware/openbench-logic-sniffer/protocol.h
Original file line number Diff line number Diff line change
Expand Up @@ -121,10 +121,10 @@ struct dev_context {
raw_sample[4]; /* raw sample, assembled from received bytes */
unsigned int cnt_rx_raw_samples; /* number of raw samples received */

unsigned int rle_count;
uint64_t rle_count;
unsigned char *sample_buf;
unsigned int sample_buf_size;
unsigned int cnt_samples; /* number of final samples in sample_buf */
uint64_t cnt_samples; /* number of final samples in sample_buf */

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do all these type changes have to do with RLE mode 3? I agree that it is probably cleaner to have them, but I don't understand why this is connected to RLE mode 3.

That being said, I will stop here for this review round. The final patch left is huge and I think it makes sense to discuss my comments now up to here and see that we can get another PR in good shape. Do you agree?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, the type changes are more to simplify the pointer arithmetic by avoiding theoretical overflows (which usually pessimize the code that the compiler generates). Sorry, that was more a habit than a change motivated by the support of RLE mode 3.

};

SR_PRIV extern const char *ols_channel_names[];
Expand Down