stlink: fix execution order in stlink_config_trace()

The change [1] guarantees that the value pointed by 'prescaler'
gets always set, even when the adapter does not support the
specific mode requested (e.g. sync), or during trace disabling.
This works fine with the code in armv7m_trace_tpiu_config(), but
requires all the parameters to be valid also to disable the trace
(with 'enable==false'), otherwise returns error on incorrect
parameters or even causes segmentation fault if pointers
'trace_freq' or 'prescaler' are NULL.

Another problem in stlink_config_trace(), not linked with [1], is
caused by a tentative to change the settings on an already enabled
trace; the trace is disabled before the new parameters are fully
validated and in case of invalid parameters the trace is not
re-enabled.
It would be more logical to first check all the parameters, then
disable the trace, change the settings and re-enable the trace.

Practically revert [1] by checking 'enable==false' at function
entry, then disable trace and exit without any further check on
the other parameters.
For the case 'enable==true', validate all the function parameters
then disable the trace, update the trace settings and re-enable
the trace.
Modify the caller armv7m_trace_tpiu_config() to initialize the
variable 'prescaler' to a safe value to avoid the issue targeted
by [1].

[1] commit 38277fa752 ("jtag/drivers/stlink_usb: fix SWO prescaler")

Change-Id: Ia6530682162ca2c9f5ac64301f2456f70cc07ed2
Signed-off-by: Antonio Borneo <borneo.antonio@gmail.com>
Reviewed-on: http://openocd.zylin.com/5934
Tested-by: jenkins
Reviewed-by: Adrian M Negreanu <adrian.negreanu@nxp.com>
This commit is contained in:
Antonio Borneo 2020-11-12 15:27:01 +01:00
parent 47fa000193
commit faaa42283f
2 changed files with 17 additions and 8 deletions

View File

@ -3528,8 +3528,20 @@ static int stlink_config_trace(void *handle, bool enabled,
{
struct stlink_usb_handle_s *h = handle;
if (enabled && (!(h->version.flags & STLINK_F_HAS_TRACE) ||
pin_protocol != TPIU_PIN_PROTOCOL_ASYNC_UART)) {
if (!(h->version.flags & STLINK_F_HAS_TRACE)) {
LOG_ERROR("The attached ST-LINK version doesn't support trace");
return ERROR_FAIL;
}
if (!enabled) {
stlink_usb_trace_disable(h);
return ERROR_OK;
}
assert(trace_freq != NULL);
assert(prescaler != NULL);
if (pin_protocol != TPIU_PIN_PROTOCOL_ASYNC_UART) {
LOG_ERROR("The attached ST-LINK version doesn't support this trace mode");
return ERROR_FAIL;
}
@ -3538,14 +3550,12 @@ static int stlink_config_trace(void *handle, bool enabled,
STLINK_V3_TRACE_MAX_HZ : STLINK_TRACE_MAX_HZ;
/* Only concern ourselves with the frequency if the STlink is processing it. */
if (enabled && *trace_freq > max_trace_freq) {
if (*trace_freq > max_trace_freq) {
LOG_ERROR("ST-LINK doesn't support SWO frequency higher than %u",
max_trace_freq);
return ERROR_FAIL;
}
stlink_usb_trace_disable(h);
if (!*trace_freq)
*trace_freq = max_trace_freq;
@ -3567,8 +3577,7 @@ static int stlink_config_trace(void *handle, bool enabled,
*prescaler = presc;
if (!enabled)
return ERROR_OK;
stlink_usb_trace_disable(h);
h->trace.source_hz = *trace_freq;

View File

@ -87,7 +87,7 @@ int armv7m_trace_tpiu_config(struct target *target)
{
struct armv7m_common *armv7m = target_to_armv7m(target);
struct armv7m_trace_config *trace_config = &armv7m->trace_config;
uint16_t prescaler;
uint16_t prescaler = TPIU_ACPR_MAX_SWOSCALER + 1;
int retval;
target_unregister_timer_callback(armv7m_poll_trace, target);