From ca5ccfb0d3ea5f3f878e5e2e9502ccdcd63ef04e Mon Sep 17 00:00:00 2001 From: Gwenhael Goavec-Merou Date: Sat, 9 Nov 2024 09:08:02 +0100 Subject: [PATCH] libgpiodJtagBitbang: simplify code, improved perf --- src/libgpiodJtagBitbang.cpp | 104 ++++++++++++++++-------------------- src/libgpiodJtagBitbang.hpp | 42 +++++++-------- 2 files changed, 66 insertions(+), 80 deletions(-) diff --git a/src/libgpiodJtagBitbang.cpp b/src/libgpiodJtagBitbang.cpp index 1b184d9e37..87a38a2bb5 100644 --- a/src/libgpiodJtagBitbang.cpp +++ b/src/libgpiodJtagBitbang.cpp @@ -35,7 +35,8 @@ LibgpiodJtagBitbang::LibgpiodJtagBitbang( const jtag_pins_conf_t *pin_conf, const std::string &dev, __attribute__((unused)) uint32_t clkHZ, - int8_t verbose):_verbose(verbose>1) + int8_t verbose):_verbose(verbose>1), _curr_tms(GPIOD_LINE_VALUE_ACTIVE), + _curr_tdi(GPIOD_LINE_VALUE_INACTIVE), _curr_tck(0) { _tck_pin = pin_conf->tck_pin; _tms_pin = pin_conf->tms_pin; @@ -57,10 +58,10 @@ LibgpiodJtagBitbang::LibgpiodJtagBitbang( /* Validate pins */ #ifdef GPIOD_APIV2 const unsigned int pins[] = {_tck_pin, _tms_pin, _tdi_pin, _tdo_pin}; - in_pins[0] = _tdo_pin; - out_pins[0] = _tdi_group[1] = _tms_group[1] = _tck_pin; - out_pins[1] = _tms_group[0] = _tms_pin; - out_pins[2] = _tdi_group[0] = _tdi_pin; + _out_pins[0] = _tdi_group[0] = _tms_group[0] = _tck_pin; + _out_pins[1] = _tdi_group[1] = _tdi_pin; + _out_pins[2] = _tms_group[1] = _tms_pin; + _out_pins[3] = _tdo_pin; #else const int pins[] = {_tck_pin, _tms_pin, _tdi_pin, _tdo_pin}; #endif @@ -85,11 +86,9 @@ LibgpiodJtagBitbang::LibgpiodJtagBitbang( } #ifdef GPIOD_APIV2 - _in_req_cfg = gpiod_request_config_new(); _out_req_cfg = gpiod_request_config_new(); - gpiod_request_config_set_consumer(_in_req_cfg, "ofl_in"); - gpiod_request_config_set_consumer(_out_req_cfg, "ofl_out"); + gpiod_request_config_set_consumer(_out_req_cfg, "openFPGALoader_out"); _out_settings = gpiod_line_settings_new(); _in_settings = gpiod_line_settings_new(); @@ -105,17 +104,18 @@ LibgpiodJtagBitbang::LibgpiodJtagBitbang( _out_settings, GPIOD_LINE_BIAS_DISABLED); _out_line_cfg = gpiod_line_config_new(); - _in_line_cfg = gpiod_line_config_new(); gpiod_line_config_add_line_settings( _out_line_cfg, _out_pins, 3, _out_settings); gpiod_line_config_add_line_settings( - _in_line_cfg, _in_pins, 1, _in_settings); + _out_line_cfg, &_out_pins[3], 1, _in_settings); _out_request = gpiod_chip_request_lines( _chip, _out_req_cfg, _out_line_cfg); - _in_request = gpiod_chip_request_lines( - _chip, _in_req_cfg, _in_line_cfg); + + /* set default values for all GPIOs */ + enum gpiod_line_value val[3] = {GPIOD_LINE_VALUE_INACTIVE, _curr_tdi, _curr_tms}; + gpiod_line_request_set_values_subset(_out_request, 3, _out_pins, val); #else _tdo_line = get_line(_tdo_pin, 0, GPIOD_LINE_REQUEST_DIRECTION_INPUT); _tdi_line = get_line(_tdi_pin, 0, GPIOD_LINE_REQUEST_DIRECTION_OUTPUT); @@ -123,10 +123,6 @@ LibgpiodJtagBitbang::LibgpiodJtagBitbang( _tms_line = get_line(_tms_pin, 1, GPIOD_LINE_REQUEST_DIRECTION_OUTPUT); #endif - _curr_tdi = 0; - _curr_tck = 0; - _curr_tms = 1; - // FIXME: I'm unsure how this value should be set. // Maybe experiment, or think through what it should be. _clkHZ = 5000000; @@ -137,13 +133,9 @@ LibgpiodJtagBitbang::~LibgpiodJtagBitbang() #ifdef GPIOD_APIV2 if (_out_request) gpiod_line_request_release(_out_request); - if (_in_request) - gpiod_line_request_release(_in_request); if (_out_line_cfg) gpiod_line_config_free(_out_line_cfg); - if (_in_line_cfg) - gpiod_line_config_free(_in_line_cfg); if (_out_settings) gpiod_line_settings_free(_out_settings); @@ -192,8 +184,27 @@ gpiod_line *LibgpiodJtagBitbang::get_line(unsigned int offset, int val, int dir) } #endif -int LibgpiodJtagBitbang::update_pins(int tck, int tms, int tdi) +int LibgpiodJtagBitbang::update_pins(gpiod_line_value tms, gpiod_line_value tdi) { +#if 1 + enum gpiod_line_value val[3] = {GPIOD_LINE_VALUE_INACTIVE, tdi, tms}; + bool tdi_wr = tdi != _curr_tdi; + bool tms_wr = tms != _curr_tms; + + if (tdi_wr & tms_wr) { + gpiod_line_request_set_values_subset(_out_request, 3, _out_pins, val); + } else if (tdi_wr) { + gpiod_line_request_set_values_subset(_out_request, 2, _tdi_group, val); + } else if (tms_wr) { + val[1] = tms; + gpiod_line_request_set_values_subset(_out_request, 2, _tms_group, val); + } else { + gpiod_line_request_set_value(_out_request, _tck_pin, + GPIOD_LINE_VALUE_INACTIVE); + } + gpiod_line_request_set_value(_out_request, _tck_pin, + GPIOD_LINE_VALUE_ACTIVE); +#else if (tdi != _curr_tdi) { #ifdef GPIOD_APIV2 if (gpiod_line_request_set_value(_tdi_request, _tdi_pin, @@ -227,9 +238,9 @@ int LibgpiodJtagBitbang::update_pins(int tck, int tms, int tdi) display("Unable to set gpio pin tck\n"); } +#endif _curr_tdi = tdi; _curr_tms = tms; - _curr_tck = tck; return 0; } @@ -238,7 +249,7 @@ int LibgpiodJtagBitbang::read_tdo() { #ifdef GPIOD_APIV2 gpiod_line_value req = gpiod_line_request_get_value( - _tdo_request, _tdo_pin); + _out_request, _tdo_pin); if (req == GPIOD_LINE_VALUE_ERROR) { display("Error reading TDO line\n"); @@ -262,76 +273,53 @@ int LibgpiodJtagBitbang::writeTMS(const uint8_t *tms_buf, uint32_t len, __attribute__((unused)) bool flush_buffer, __attribute__((unused)) uint8_t tdi) { - int tms; - enum gpiod_line_value val[2] = {GPIOD_LINE_VALUE_INACTIVE, GPIOD_LINE_VALUE_INACTIVE}; - if (len == 0) // nothing -> stop return len; for (uint32_t i = 0; i < len; i++) { - _curr_tms = ((tms_buf[i >> 3] & (1 << (i & 7))) ? 1 : 0); - val[0] = (_curr_tms == 1) ? GPIOD_LINE_VALUE_ACTIVE : GPIOD_LINE_VALUE_INACTIVE; - gpiod_line_set_values_subset(out_req, 2, _tms_group, val); - gpiod_line_request_set_value(out_req, _tck_pin, GPIOD_LINE_VALUE_ACTIVE); + gpiod_line_value tms = ((tms_buf[i >> 3] & (1 << (i & 7))) ? + GPIOD_LINE_VALUE_ACTIVE : GPIOD_LINE_VALUE_INACTIVE); + update_pins(tms, _curr_tdi); } - /* force TCK low */ - gpiod_line_set_value(out_req, _tck_pin, GPIOD_LINE_VALUE_INACTIVE); - return len; } int LibgpiodJtagBitbang::writeTDI(const uint8_t *tx, uint8_t *rx, uint32_t len, bool end) { - enum gpiod_line_value val[4] = {0, 0, 0, 1}; + gpiod_line_value tdi = GPIOD_LINE_VALUE_INACTIVE; if (rx) memset(rx, 0, len / 8); for (uint32_t i = 0; i < len; i++) { - if (tx) - _curr_tdi = (tx[i >> 3] & (1 << (i & 7))) ? 1 : 0; + if (tx) { + tdi = (tx[i >> 3] & (1 << (i & 7))) ? + GPIOD_LINE_VALUE_ACTIVE : GPIOD_LINE_VALUE_INACTIVE; + } /* When TMS needs to be also updated (because of end) */ if (end && (i == len - 1)) { - _curr_tms = 1; - val[0] = _curr_tms; - val[1] = _curr_tdi; - val[2] = 0; - /* Write all output GPIOs in one command */ - gpiod_line_set_values(out_req, val); - /* Only update TCK (Rising edge) */ - gpiod_line_request_set_value(out_req, _tck_pin, GPIOD_LINE_VALUE_ACTIVE); + update_pins(GPIOD_LINE_VALUE_ACTIVE, tdi); } else { - /* otherwise only update TDI + TCK */ - val[0] = val[2] = _curr_tdi; - gpiod_line_request_set_values_subset(out_req, 2, tdi_group, val); - gpiod_line_request_set_value(out_req, _tck_pin, GPIOD_LINE_VALUE_ACTIVE); + update_pins(GPIOD_LINE_VALUE_INACTIVE, tdi); } - if (rx) { if (read_tdo() > 0) rx[i >> 3] |= 1 << (i & 7); } } - /* Is it really required? First step is always to set TCK low - * maybe only a simple update at destructor to have coherent state - */ - gpiod_line_request_set_value(out_req, _tck_pin, GPIOD_LINE_VALUE_INACTIVE); - return len; } int LibgpiodJtagBitbang::toggleClk(uint8_t tms, uint8_t tdi, uint32_t clk_len) { for (uint32_t i = 0; i < clk_len; i++) { - gpiod_line_request_set_value(out_req, _tck_pin, GPIOD_LINE_VALUE_INACTIVE); - gpiod_line_request_set_value(out_req, _tck_pin, GPIOD_LINE_VALUE_ACTIVE); + gpiod_line_request_set_value(_out_request, _tck_pin, GPIOD_LINE_VALUE_INACTIVE); + gpiod_line_request_set_value(_out_request, _tck_pin, GPIOD_LINE_VALUE_ACTIVE); } - gpiod_line_request_set_value(out_req, _tck_pin, GPIOD_LINE_VALUE_INACTIVE); - return clk_len; } diff --git a/src/libgpiodJtagBitbang.hpp b/src/libgpiodJtagBitbang.hpp index de64ad804f..1111916d9d 100644 --- a/src/libgpiodJtagBitbang.hpp +++ b/src/libgpiodJtagBitbang.hpp @@ -44,8 +44,11 @@ class LibgpiodJtagBitbang : public JtagInterface { private: #ifndef GPIOD_APIV2 gpiod_line *get_line(unsigned int offset, int val, int dir); -#endif int update_pins(int tck, int tms, int tdi); +#else + int update_pins(gpiod_line_value tms, gpiod_line_value tdi); +#endif + int read_tdo(); bool _verbose; @@ -55,6 +58,9 @@ class LibgpiodJtagBitbang : public JtagInterface { unsigned int _tms_pin; unsigned int _tdo_pin; unsigned int _tdi_pin; + unsigned int _out_pins[4]; + unsigned int _tms_group[2]; + unsigned int _tdi_group[2]; #else int _tck_pin; int _tms_pin; @@ -65,34 +71,26 @@ class LibgpiodJtagBitbang : public JtagInterface { gpiod_chip *_chip; #ifdef GPIOD_APIV2 - gpiod_request_config *_tck_req_cfg; - gpiod_request_config *_tms_req_cfg; - gpiod_request_config *_tdo_req_cfg; - gpiod_request_config *_tdi_req_cfg; - - gpiod_line_config *_tck_line_cfg; - gpiod_line_config *_tms_line_cfg; - gpiod_line_config *_tdo_line_cfg; - gpiod_line_config *_tdi_line_cfg; - - gpiod_line_settings *_tck_settings; - gpiod_line_settings *_tms_settings; - gpiod_line_settings *_tdo_settings; - gpiod_line_settings *_tdi_settings; - - gpiod_line_request *_tdo_request; - gpiod_line_request *_tdi_request; - gpiod_line_request *_tck_request; - gpiod_line_request *_tms_request; + gpiod_request_config *_out_req_cfg; + + gpiod_line_config *_out_line_cfg; + + gpiod_line_settings *_in_settings; + gpiod_line_settings *_out_settings; + + gpiod_line_request *_out_request; + + gpiod_line_value _curr_tms; + gpiod_line_value _curr_tdi; #else gpiod_line *_tck_line; gpiod_line *_tms_line; gpiod_line *_tdo_line; gpiod_line *_tdi_line; -#endif - int _curr_tms; int _curr_tdi; +#endif + int _curr_tck; }; #endif