Skip to content

Commit

Permalink
Deprecate split transactions status field
Browse files Browse the repository at this point in the history
It is a left over from previous serial implementations like the helix
keyboard. The field is read and written to in split communications, but
never evaluated as the current core implementation relies on the return
value of soft_serial_transaction which is treated as a bool. Therefore
status is removed and soft_serial_transaction returns a bool now.

soft_serial_get_and_clean_status is removed as it is dead code.
  • Loading branch information
KarlK90 committed Jan 24, 2022
1 parent 489c5ff commit 5929e5d
Show file tree
Hide file tree
Showing 7 changed files with 28 additions and 91 deletions.
19 changes: 1 addition & 18 deletions drivers/serial.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,21 +26,4 @@ void soft_serial_initiator_init(void);
// target is interrupt accept side
void soft_serial_target_init(void);

// initiator result
#define TRANSACTION_END 0
#define TRANSACTION_NO_RESPONSE 0x1
#define TRANSACTION_DATA_ERROR 0x2
#define TRANSACTION_TYPE_ERROR 0x4
int soft_serial_transaction(int sstd_index);

// target status
// *SSTD_t.status has
// initiator:
// TRANSACTION_END
// or TRANSACTION_NO_RESPONSE
// or TRANSACTION_DATA_ERROR
// target:
// TRANSACTION_DATA_ERROR
// or TRANSACTION_ACCEPTED
#define TRANSACTION_ACCEPTED 0x8
int soft_serial_get_and_clean_status(int sstd_index);
bool soft_serial_transaction(int sstd_index);
39 changes: 7 additions & 32 deletions platforms/avr/drivers/serial.c
Original file line number Diff line number Diff line change
Expand Up @@ -409,13 +409,7 @@ ISR(SERIAL_PIN_INTERRUPT) {

// target recive phase
if (trans->initiator2target_buffer_size > 0) {
if (serial_recive_packet((uint8_t *)split_trans_initiator2target_buffer(trans), trans->initiator2target_buffer_size)) {
*trans->status = TRANSACTION_ACCEPTED;
} else {
*trans->status = TRANSACTION_DATA_ERROR;
}
} else {
*trans->status = TRANSACTION_ACCEPTED;
serial_recive_packet((uint8_t *)split_trans_initiator2target_buffer(trans), trans->initiator2target_buffer_size);
}

sync_recv(); // weit initiator output to high
Expand All @@ -424,19 +418,13 @@ ISR(SERIAL_PIN_INTERRUPT) {
/////////
// start transaction by initiator
//
// int soft_serial_transaction(int sstd_index)
// bool soft_serial_transaction(int sstd_index)
//
// Returns:
// TRANSACTION_END
// TRANSACTION_NO_RESPONSE
// TRANSACTION_DATA_ERROR
// this code is very time dependent, so we need to disable interrupts
int soft_serial_transaction(int sstd_index) {
if (sstd_index > NUM_TOTAL_TRANSACTIONS) return TRANSACTION_TYPE_ERROR;
bool soft_serial_transaction(int sstd_index) {
if (sstd_index > NUM_TOTAL_TRANSACTIONS) return false;
split_transaction_desc_t *trans = &split_transaction_table[sstd_index];

if (!trans->status) return TRANSACTION_TYPE_ERROR; // not registered

cli();

// signal to the target that we want to start a transaction
Expand All @@ -463,9 +451,8 @@ int soft_serial_transaction(int sstd_index) {
// slave failed to pull the line low, assume not present
serial_output();
serial_high();
*trans->status = TRANSACTION_NO_RESPONSE;
sei();
return TRANSACTION_NO_RESPONSE;
return false;
}
_delay_sub_us(SLAVE_INT_ACK_WIDTH_UNIT);
}
Expand All @@ -476,9 +463,8 @@ int soft_serial_transaction(int sstd_index) {
if (!serial_recive_packet((uint8_t *)split_trans_target2initiator_buffer(trans), trans->target2initiator_buffer_size)) {
serial_output();
serial_high();
*trans->status = TRANSACTION_DATA_ERROR;
sei();
return TRANSACTION_DATA_ERROR;
return false;
}
}

Expand All @@ -493,19 +479,8 @@ int soft_serial_transaction(int sstd_index) {
// always, release the line when not in use
sync_send();

*trans->status = TRANSACTION_END;
sei();
return TRANSACTION_END;
}

int soft_serial_get_and_clean_status(int sstd_index) {
split_transaction_desc_t *trans = &split_transaction_table[sstd_index];
cli();
int retval = *trans->status;
*trans->status = 0;
;
sei();
return retval;
return true;
}
#endif

Expand Down
19 changes: 6 additions & 13 deletions platforms/chibios/drivers/serial.c
Original file line number Diff line number Diff line change
Expand Up @@ -179,8 +179,6 @@ void interrupt_handler(void *arg) {
// wait for the sync to finish sending
serial_delay();

*trans->status = (checksum_computed == checksum_received) ? TRANSACTION_ACCEPTED : TRANSACTION_DATA_ERROR;

// end transaction
serial_input();

Expand All @@ -193,17 +191,12 @@ void interrupt_handler(void *arg) {
/////////
// start transaction by initiator
//
// int soft_serial_transaction(int sstd_index)
// bool soft_serial_transaction(int sstd_index)
//
// Returns:
// TRANSACTION_END
// TRANSACTION_NO_RESPONSE
// TRANSACTION_DATA_ERROR
// this code is very time dependent, so we need to disable interrupts
int soft_serial_transaction(int sstd_index) {
if (sstd_index > NUM_TOTAL_TRANSACTIONS) return TRANSACTION_TYPE_ERROR;
bool soft_serial_transaction(int sstd_index) {
if (sstd_index > NUM_TOTAL_TRANSACTIONS) return false;
split_transaction_desc_t *trans = &split_transaction_table[sstd_index];
if (!trans->status) return TRANSACTION_TYPE_ERROR; // not registered

// TODO: remove extra delay between transactions
serial_delay();
Expand All @@ -226,7 +219,7 @@ int soft_serial_transaction(int sstd_index) {
// slave failed to pull the line low, assume not present
dprintf("serial::NO_RESPONSE\n");
chSysUnlock();
return TRANSACTION_NO_RESPONSE;
return false;
}

// if the slave is present syncronize with it
Expand Down Expand Up @@ -266,13 +259,13 @@ int soft_serial_transaction(int sstd_index) {
serial_high();

chSysUnlock();
return TRANSACTION_DATA_ERROR;
return false;
}

// always, release the line when not in use
serial_high();
serial_output();

chSysUnlock();
return TRANSACTION_END;
return true;
}
32 changes: 10 additions & 22 deletions platforms/chibios/drivers/serial_usart.c
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ static SerialDriver* serial_driver = &SERIAL_USART_DRIVER;
static inline bool react_to_transactions(void);
static inline bool __attribute__((nonnull)) receive(uint8_t* destination, const size_t size);
static inline bool __attribute__((nonnull)) send(const uint8_t* source, const size_t size);
static inline int initiate_transaction(uint8_t sstd_index);
static inline bool initiate_transaction(uint8_t sstd_index);
static inline void usart_clear(void);

/**
Expand Down Expand Up @@ -206,14 +206,12 @@ static inline bool react_to_transactions(void) {
to signal that the slave is ready to receive possible transaction buffers */
sstd_index ^= HANDSHAKE_MAGIC;
if (!send(&sstd_index, sizeof(sstd_index))) {
*trans->status = TRANSACTION_DATA_ERROR;
return false;
}

/* Receive transaction buffer from the master. If this transaction requires it.*/
if (trans->initiator2target_buffer_size) {
if (!receive(split_trans_initiator2target_buffer(trans), trans->initiator2target_buffer_size)) {
*trans->status = TRANSACTION_DATA_ERROR;
return false;
}
}
Expand All @@ -226,12 +224,10 @@ static inline bool react_to_transactions(void) {
/* Send transaction buffer to the master. If this transaction requires it. */
if (trans->target2initiator_buffer_size) {
if (!send(split_trans_target2initiator_buffer(trans), trans->target2initiator_buffer_size)) {
*trans->status = TRANSACTION_DATA_ERROR;
return false;
}
}

*trans->status = TRANSACTION_ACCEPTED;
return true;
}

Expand All @@ -252,11 +248,9 @@ void soft_serial_initiator_init(void) {
* @brief Start transaction from the master half to the slave half.
*
* @param index Transaction Table index of the transaction to start.
* @return int TRANSACTION_NO_RESPONSE in case of Timeout.
* TRANSACTION_TYPE_ERROR in case of invalid transaction index.
* TRANSACTION_END in case of success.
* @return bool Indicates success of transaction.
*/
int soft_serial_transaction(int index) {
bool soft_serial_transaction(int index) {
/* Clear the receive queue, to start with a clean slate.
* Parts of failed transactions or spurious bytes could still be in it. */
usart_clear();
Expand All @@ -266,25 +260,19 @@ int soft_serial_transaction(int index) {
/**
* @brief Initiate transaction to slave half.
*/
static inline int initiate_transaction(uint8_t sstd_index) {
static inline bool initiate_transaction(uint8_t sstd_index) {
/* Sanity check that we are actually starting a valid transaction. */
if (sstd_index >= NUM_TOTAL_TRANSACTIONS) {
dprintln("USART: Illegal transaction Id.");
return TRANSACTION_TYPE_ERROR;
return false;
}

split_transaction_desc_t* trans = &split_transaction_table[sstd_index];

/* Transaction is not registered. Abort. */
if (!trans->status) {
dprintln("USART: Transaction not registered.");
return TRANSACTION_TYPE_ERROR;
}

/* Send transaction table index to the slave, which doubles as basic handshake token. */
if (!send(&sstd_index, sizeof(sstd_index))) {
dprintln("USART: Send Handshake failed.");
return TRANSACTION_TYPE_ERROR;
return false;
}

uint8_t sstd_index_shake = 0xFF;
Expand All @@ -295,24 +283,24 @@ static inline int initiate_transaction(uint8_t sstd_index) {
*/
if (!receive(&sstd_index_shake, sizeof(sstd_index_shake)) || (sstd_index_shake != (sstd_index ^ HANDSHAKE_MAGIC))) {
dprintln("USART: Handshake failed.");
return TRANSACTION_NO_RESPONSE;
return false;
}

/* Send transaction buffer to the slave. If this transaction requires it. */
if (trans->initiator2target_buffer_size) {
if (!send(split_trans_initiator2target_buffer(trans), trans->initiator2target_buffer_size)) {
dprintln("USART: Send failed.");
return TRANSACTION_NO_RESPONSE;
return false;
}
}

/* Receive transaction buffer from the slave. If this transaction requires it. */
if (trans->target2initiator_buffer_size) {
if (!receive(split_trans_target2initiator_buffer(trans), trans->target2initiator_buffer_size)) {
dprintln("USART: Receive failed.");
return TRANSACTION_NO_RESPONSE;
return false;
}
}

return TRANSACTION_END;
return true;
}
7 changes: 3 additions & 4 deletions quantum/split_common/transactions.c
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,11 @@
#define sizeof_member(type, member) sizeof(((type *)NULL)->member)

#define trans_initiator2target_initializer_cb(member, cb) \
{ &dummy, sizeof_member(split_shared_memory_t, member), offsetof(split_shared_memory_t, member), 0, 0, cb }
{ sizeof_member(split_shared_memory_t, member), offsetof(split_shared_memory_t, member), 0, 0, cb }
#define trans_initiator2target_initializer(member) trans_initiator2target_initializer_cb(member, NULL)

#define trans_target2initiator_initializer_cb(member, cb) \
{ &dummy, 0, 0, sizeof_member(split_shared_memory_t, member), offsetof(split_shared_memory_t, member), cb }
{ 0, 0, sizeof_member(split_shared_memory_t, member), offsetof(split_shared_memory_t, member), cb }
#define trans_target2initiator_initializer(member) trans_target2initiator_initializer_cb(member, NULL)

#define transport_write(id, data, length) transport_execute_transaction(id, data, length, NULL, 0)
Expand Down Expand Up @@ -658,10 +658,9 @@ static void pointing_handlers_slave(matrix_row_t master_matrix[], matrix_row_t s

////////////////////////////////////////////////////

uint8_t dummy;
split_transaction_desc_t split_transaction_table[NUM_TOTAL_TRANSACTIONS] = {
// Set defaults
[0 ...(NUM_TOTAL_TRANSACTIONS - 1)] = {NULL, 0, 0, 0, 0, 0},
[0 ...(NUM_TOTAL_TRANSACTIONS - 1)] = {0, 0, 0, 0, 0},

#ifdef USE_I2C
[I2C_EXECUTE_CALLBACK] = trans_initiator2target_initializer(transaction_id),
Expand Down
1 change: 0 additions & 1 deletion quantum/split_common/transactions.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ typedef void (*slave_callback_t)(uint8_t initiator2target_buffer_size, const voi

// Split transaction Descriptor
typedef struct _split_transaction_desc_t {
uint8_t * status;
uint8_t initiator2target_buffer_size;
uint16_t initiator2target_offset;
uint8_t target2initiator_buffer_size;
Expand Down
2 changes: 1 addition & 1 deletion quantum/split_common/transport.c
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ bool transport_execute_transaction(int8_t id, const void *initiator2target_buf,
memcpy(split_trans_initiator2target_buffer(trans), initiator2target_buf, len);
}

if (soft_serial_transaction(id) != TRANSACTION_END) {
if (!soft_serial_transaction(id)) {
return false;
}

Expand Down

0 comments on commit 5929e5d

Please sign in to comment.