From be59e8af2b8efcee2933f3b9e40129bc1d8f9d1c Mon Sep 17 00:00:00 2001 From: Stefan Kerkmann Date: Thu, 27 Jan 2022 06:13:27 +0100 Subject: [PATCH] Deprecate split transactions status field (#16023) --- drivers/serial.h | 19 +----------- platforms/avr/drivers/serial.c | 39 +++++------------------- platforms/chibios/drivers/serial.c | 19 ++++-------- platforms/chibios/drivers/serial_usart.c | 32 ++++++------------- quantum/split_common/transactions.c | 7 ++--- quantum/split_common/transactions.h | 1 - quantum/split_common/transport.c | 2 +- 7 files changed, 28 insertions(+), 91 deletions(-) diff --git a/drivers/serial.h b/drivers/serial.h index d9c2a69e969..0cfdbd99594 100644 --- a/drivers/serial.h +++ b/drivers/serial.h @@ -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); diff --git a/platforms/avr/drivers/serial.c b/platforms/avr/drivers/serial.c index ab0b52afd19..62908e58757 100644 --- a/platforms/avr/drivers/serial.c +++ b/platforms/avr/drivers/serial.c @@ -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 @@ -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 @@ -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); } @@ -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; } } @@ -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 diff --git a/platforms/chibios/drivers/serial.c b/platforms/chibios/drivers/serial.c index ef6f0aa8d5f..17d0b32be8a 100644 --- a/platforms/chibios/drivers/serial.c +++ b/platforms/chibios/drivers/serial.c @@ -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(); @@ -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(); @@ -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 @@ -266,7 +259,7 @@ int soft_serial_transaction(int sstd_index) { serial_high(); chSysUnlock(); - return TRANSACTION_DATA_ERROR; + return false; } // always, release the line when not in use @@ -274,5 +267,5 @@ int soft_serial_transaction(int sstd_index) { serial_output(); chSysUnlock(); - return TRANSACTION_END; + return true; } diff --git a/platforms/chibios/drivers/serial_usart.c b/platforms/chibios/drivers/serial_usart.c index 124e4be685e..42c476a374a 100644 --- a/platforms/chibios/drivers/serial_usart.c +++ b/platforms/chibios/drivers/serial_usart.c @@ -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); /** @@ -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; } } @@ -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; } @@ -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(); @@ -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; @@ -295,14 +283,14 @@ 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; } } @@ -310,9 +298,9 @@ static inline int initiate_transaction(uint8_t sstd_index) { 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; } diff --git a/quantum/split_common/transactions.c b/quantum/split_common/transactions.c index d9e8fd1f6b9..4b059a2b8a8 100644 --- a/quantum/split_common/transactions.c +++ b/quantum/split_common/transactions.c @@ -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) @@ -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), diff --git a/quantum/split_common/transactions.h b/quantum/split_common/transactions.h index 53610d6f8e8..e38ec79ce91 100644 --- a/quantum/split_common/transactions.h +++ b/quantum/split_common/transactions.h @@ -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; diff --git a/quantum/split_common/transport.c b/quantum/split_common/transport.c index bcc0261417d..060ba8a9274 100644 --- a/quantum/split_common/transport.c +++ b/quantum/split_common/transport.c @@ -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; }