Skip to content

Commit

Permalink
hosted/*_swd: Unify all parity calculations in SWD mode adapter code
Browse files Browse the repository at this point in the history
* Always use a parity function, not `popcount & 1`
* Hide most `__builtin` usage (from entities like MSVC)
* `ftdi_swd_seq_in_parity_mpsse()`: break up the bitmask calculation to enforce uint32_t intermediate
* `jlink_swd_seq_in_parity()`: cosmetic: fix a typo in comment
  • Loading branch information
ALTracer authored and dragonmux committed Dec 31, 2023
1 parent 648365f commit d7767d4
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 12 deletions.
7 changes: 4 additions & 3 deletions src/platforms/hosted/dap_swd.c
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
#include "dap.h"
#include "dap_command.h"
#include "buffer_utils.h"
#include "maths_utils.h"

typedef enum dap_swd_turnaround_cycles {
DAP_SWD_TURNAROUND_1_CYCLE = 0U,
Expand Down Expand Up @@ -127,7 +128,7 @@ static void dap_swd_seq_out_parity(const uint32_t tms_states, const size_t clock
DAP_SWD_OUT_SEQUENCE,
};
write_le4(sequence.data, 0, tms_states);
sequence.data[4] = __builtin_parity(tms_states);
sequence.data[4] = calculate_odd_parity(tms_states);
/* And perform it */
if (!perform_dap_swd_sequences(&sequence, 1U))
DEBUG_ERROR("dap_swd_seq_out_parity failed\n");
Expand Down Expand Up @@ -169,7 +170,7 @@ static bool dap_swd_seq_in_parity(uint32_t *const result, const size_t clock_cyc
for (size_t offset = 0; offset < clock_cycles; offset += 8U)
data |= sequence.data[offset >> 3U] << offset;
*result = data;
uint8_t parity = __builtin_parity(data) & 1U;
uint8_t parity = calculate_odd_parity(data);
parity ^= sequence.data[4] & 1U;
return !parity;
}
Expand All @@ -194,7 +195,7 @@ static bool dap_write_reg_no_check(const uint16_t addr, const uint32_t data)
33U,
DAP_SWD_OUT_SEQUENCE,
/* The 4 data bytes are filled in below with write_le4() */
{0U, 0U, 0U, 0U, __builtin_parity(data)},
{0U, 0U, 0U, 0U, calculate_odd_parity(data)},
},
};
write_le4(sequences[3].data, 0, data);
Expand Down
7 changes: 5 additions & 2 deletions src/platforms/hosted/ftdi_swd.c
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
#include <ftdi.h>
#include "ftdi_bmp.h"
#include "buffer_utils.h"
#include "maths_utils.h"

typedef enum swdio_status {
SWDIO_STATUS_DRIVE,
Expand Down Expand Up @@ -237,7 +238,9 @@ static bool ftdi_swd_seq_in_parity_mpsse(uint32_t *const result, const size_t cl
uint8_t data_out[5U] = {0};
ftdi_jtag_tdi_tdo_seq(data_out, false, NULL, clock_cycles + 1U);
const uint32_t data = read_le4(data_out, 0);
uint8_t parity = __builtin_parity(data & ((UINT64_C(1) << clock_cycles) - 1U));
/* NB: This calculation must be done in 64-bit space due to `1U << 32U` value of 0x00000001'00000000ULL */
const uint32_t bitmask = (UINT64_C(1) << clock_cycles) - 1U;
uint8_t parity = calculate_odd_parity(data & bitmask);
parity ^= data_out[4] & 1U;
DEBUG_PROBE("%s %zu clock_cycles: %08" PRIx32 " %s\n", __func__, clock_cycles, data, parity ? "ERR" : "OK");
*result = data;
Expand Down Expand Up @@ -428,7 +431,7 @@ static void ftdi_swd_seq_out_parity(uint32_t tms_states, size_t clock_cycles)
{
if (clock_cycles > 32U)
return;
const uint8_t parity = __builtin_parity(tms_states) & 1U;
const uint8_t parity = calculate_odd_parity(tms_states);
ftdi_swd_turnaround(SWDIO_STATUS_DRIVE);
if (do_mpsse)
ftdi_swd_seq_out_parity_mpsse(tms_states, parity, clock_cycles);
Expand Down
15 changes: 8 additions & 7 deletions src/platforms/hosted/jlink_swd.c
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
#include "jlink.h"
#include "jlink_protocol.h"
#include "buffer_utils.h"
#include "maths_utils.h"
#include "cli.h"

/*
Expand Down Expand Up @@ -121,7 +122,7 @@ static void jlink_swd_seq_out_parity(const uint32_t tms_states, const size_t clo
/* Construct the parity bit */
const size_t byte = clock_cycles >> 3U;
const uint8_t bit = clock_cycles & 7U;
data[byte] |= (__builtin_parity(tms_states) & 1U) << bit;
data[byte] |= calculate_odd_parity(tms_states) << bit;
/* Attempt the transfer */
if (!jlink_transfer_swd(clock_cycles + 1U, JLINK_SWD_OUT, data, NULL)) {
/* If things go wrong, report it */
Expand Down Expand Up @@ -159,9 +160,9 @@ static bool jlink_swd_seq_in_parity(uint32_t *const result, const size_t clock_c
/* Compute the parity and validate it */
const size_t byte = clock_cycles >> 3U;
const uint8_t bit = clock_cycles & 7U;
uint8_t parity = __builtin_parity(data) & 1U;
uint8_t parity = calculate_odd_parity(data);
parity ^= (data_out[byte] >> bit) & 1U;
/* Retrn the result of the calculation */
/* Return the result of the calculation */
DEBUG_PROBE("%s %zu clock_cycles: %08" PRIx32 " %s\n", __func__, clock_cycles, data, parity ? "ERR" : "OK");
*result = data;
return !parity;
Expand All @@ -183,7 +184,7 @@ static bool jlink_adiv5_raw_write_no_check(const uint16_t addr, const uint32_t d
/* Build the response payload buffer */
uint8_t response[6] = {0};
write_le4(response, 0, data);
response[4] = __builtin_popcount(data) & 1U;
response[4] = calculate_odd_parity(data);
/* Try sending the data to the device */
if (!jlink_transfer(33U + 8U, jlink_adiv5_write_request, response, NULL)) {
DEBUG_ERROR("jlink_adiv5_raw_write_no_check failed\n");
Expand Down Expand Up @@ -213,7 +214,7 @@ static uint32_t jlink_adiv5_raw_read_no_check(const uint16_t addr)
/* Extract the data phase and return it if the transaction succeeded */
const uint32_t data = read_le4(response, 0);
DEBUG_PROBE("jlink_adiv5_raw_read_no_check %04x -> %08" PRIx32 " %s\n", addr, data,
__builtin_parity(data) ^ response[4] ? "ERR" : "OK");
calculate_odd_parity(data) != response[4] ? "ERR" : "OK");
return ack == SWDP_ACK_OK ? data : 0U;
}

Expand All @@ -228,7 +229,7 @@ static uint32_t jlink_adiv5_raw_read(adiv5_debug_port_s *const dp)
/* Extract the data phase */
const uint32_t response = read_le4(result, 0);
/* Calculate and do a parity check */
uint8_t parity = __builtin_parity(response) & 1U;
uint8_t parity = calculate_odd_parity(response);
parity ^= result[4] & 1U;
/* If that fails, turn it into an error */
if (parity) {
Expand All @@ -244,7 +245,7 @@ static uint32_t jlink_adiv5_raw_write(const uint32_t request_value)
/* Build the response payload buffer */
uint8_t request[6] = {0};
write_le4(request, 0, request_value);
request[4] = __builtin_popcount(request_value) & 1U;
request[4] = calculate_odd_parity(request_value);
/* Allocate storage for the result */
uint8_t result[6] = {0};
/* Try sending the data to the device */
Expand Down

0 comments on commit d7767d4

Please sign in to comment.