transport: allow transport name jtag/swd for hla and dapdirect

The transport used on adapter's wires is either 'jtag' or 'swd'
but, depending on the adapter, in the command 'transport select'
we have to use either 'jtag' or 'swd' or the similar 'hla_jtag',
'hla_swd', 'dapdirect_jtag' or 'dapdirect_swd'.
This becomes cumbersome when we just want to change adapter and we
get forced to modify the 'transport select' command too.

There is no reason for an adapter driver to support two of the
similar transports. In fact 'dapdirect' one is a superset of the
'hla', and the native 'jtag' or 'swd' is a superset of the
'dapdirect' one.
While the adapter could support more than one similar transports,
its adapter driver should only support the most complete of these
similar transports.

Modify the 'transport select' code to accept 'jtag' or 'swd' for
the 'dapdirect' and the 'hla' adapters too.
Issue a deprecated message for the old 'dapdirect' and 'hla'
transport names.
In command 'transport list', print only the transport names that
can be selected through 'transport select' skipping information
about 'dapdirect' and 'hla' versions and avoid duplicated entries.

This improvement was listed in the TODO file. Update it!

Change-Id: I626b50e7a94c141c042eab388cd1ffe77eb864c2
Signed-off-by: Antonio Borneo <[email protected]>
Reviewed-on: https://review.openocd.org/c/openocd/+/8677
Tested-by: jenkins

Author: borneoa

README

@@ -42,7 +42,7 @@ openocd -f interface/ftdi/jtagkey2.cfg -c "transport select jtag" \
 ```
 
 ```
-openocd -f interface/stlink.cfg -c "transport select hla_swd" \
+openocd -f interface/stlink.cfg -c "transport select swd" \
         -f target/stm32l0.cfg
 ```
 

TODO

@@ -60,8 +60,6 @@ changes pending in gerrit.
   to replicate it in the drivers, apart in case the driver sets TRST
   independently
 - add .hla_ops to "adapter"
-- HLA is a API level (.hla_ops). Transport should simply be {jtag,swd},
-  not {hla_jtag,hla_swd}.
 
 @subsection thelistadapterjtagcore JTAG Core
 

doc/openocd.texi

@@ -3783,10 +3783,8 @@ JTAG supports both debugging and boundary scan testing.
 Flash programming support is built on top of debug support.
 
 JTAG transport is selected with the command @command{transport select
-jtag}. Unless your adapter uses either @ref{hla_interface,the hla interface
-driver} (in which case the command is @command{transport select hla_jtag})
-or @ref{st_link_dap_interface,the st-link interface driver} (in which case
-the command is @command{transport select dapdirect_jtag}).
+jtag}. This command has to be used also for @ref{hla_interface,the hla interface
+driver} and @ref{st_link_dap_interface,the st-link interface driver}.
 
 @subsection SWD Transport
 @cindex SWD
@@ -3799,10 +3797,8 @@ Flash programming support is built on top of debug support.
 (Some processors support both JTAG and SWD.)
 
 SWD transport is selected with the command @command{transport select
-swd}. Unless your adapter uses either @ref{hla_interface,the hla interface
-driver} (in which case the command is @command{transport select hla_swd})
-or @ref{st_link_dap_interface,the st-link interface driver} (in which case
-the command is @command{transport select dapdirect_swd}).
+swd}. This command has to be used also for @ref{hla_interface,the hla interface
+driver} and @ref{st_link_dap_interface,the st-link interface driver}.
 
 @deffn {Config Command} {swd newdap} ...
 Declares a single DAP which uses SWD transport.
@@ -10889,7 +10885,7 @@ baud with our custom divisor to get 12MHz)
 @item OpenOCD invocation line:
 @example
 openocd -f interface/stlink.cfg \
--c "transport select dapdirect_swd" \
+-c "transport select swd" \
 -f target/stm32l1.cfg \
 -c "stm32l1.tpiu configure -protocol uart" \
 -c "stm32l1.tpiu configure -traceclk 24000000 -pin-freq 12000000" \

src/transport/transport.c

@@ -49,14 +49,16 @@ extern struct command_context *global_cmd_ctx;
 static const struct {
 	unsigned int id;
 	const char *name;
+	const char *full_name;
+	const char *deprecated_name;
 } transport_names[] = {
-	{ TRANSPORT_JTAG,           "jtag",           },
-	{ TRANSPORT_SWD,            "swd",            },
-	{ TRANSPORT_HLA_JTAG,       "hla_jtag",       },
-	{ TRANSPORT_HLA_SWD,        "hla_swd",        },
-	{ TRANSPORT_DAPDIRECT_JTAG, "dapdirect_jtag", },
-	{ TRANSPORT_DAPDIRECT_SWD,  "dapdirect_swd",  },
-	{ TRANSPORT_SWIM,           "swim",           },
+	{ TRANSPORT_JTAG,           "jtag", "jtag",             NULL,             },
+	{ TRANSPORT_SWD,            "swd",  "swd",              NULL,             },
+	{ TRANSPORT_HLA_JTAG,       "jtag", "jtag (hla)",       "hla_jtag",       },
+	{ TRANSPORT_HLA_SWD,        "swd",  "swd (hla)",        "hla_swd",        },
+	{ TRANSPORT_DAPDIRECT_JTAG, "jtag", "jtag (dapdirect)", "dapdirect_jtag", },
+	{ TRANSPORT_DAPDIRECT_SWD,  "swd",  "swd (dapdirect)",  "dapdirect_swd",  },
+	{ TRANSPORT_SWIM,           "swim", "swim",             NULL,             },
 };
 
 /** List of transports registered in OpenOCD, alphabetically sorted per name. */
@@ -90,18 +92,36 @@ const char *transport_name(unsigned int id)
 	return NULL;
 }
 
+static const char *transport_full_name(unsigned int id)
+{
+	for (unsigned int i = 0; i < ARRAY_SIZE(transport_names); i++)
+		if (id == transport_names[i].id)
+			return transport_names[i].full_name;
+
+	return NULL;
+}
+
+static const char *transport_deprecated_name(unsigned int id)
+{
+	for (unsigned int i = 0; i < ARRAY_SIZE(transport_names); i++)
+		if (id == transport_names[i].id)
+			return transport_names[i].deprecated_name;
+
+	return NULL;
+}
+
 static bool is_transport_id_valid(unsigned int id)
 {
 	return (id != 0) && ((id & ~TRANSPORT_VALID_MASK) == 0) && IS_PWR_OF_2(id);
 }
 
-static int transport_select(struct command_context *ctx, const char *name)
+static int transport_select(struct command_context *ctx, unsigned int transport_id)
 {
 	/* name may only identify a known transport;
 	 * caller guarantees session's transport isn't yet set.*/
 	struct transport *t;
 	list_for_each_entry(t, &transport_list, lh) {
-		if (!strcmp(transport_name(t->id), name)) {
+		if (t->id == transport_id) {
 			int retval = t->select(ctx);
 			/* select() registers commands specific to this
 			 * transport, and may also reset the link, e.g.
@@ -110,12 +130,14 @@ static int transport_select(struct command_context *ctx, const char *name)
 			if (retval == ERROR_OK)
 				session = t;
 			else
-				LOG_ERROR("Error selecting '%s' as transport", name);
+				LOG_ERROR("Error selecting '%s' as transport",
+					transport_full_name(transport_id));
 			return retval;
 		}
 	}
 
-	LOG_ERROR("No transport named '%s' is available.", name);
+	LOG_ERROR("No transport named '%s' is available.",
+		transport_full_name(transport_id));
 	return ERROR_FAIL;
 }
 
@@ -165,7 +187,7 @@ int allow_transports(struct command_context *ctx, unsigned int transport_ids,
 	if (IS_PWR_OF_2(transport_ids)) {
 		LOG_DEBUG("only one transport option; autoselecting '%s'", transport_name(transport_ids));
 		transport_single_is_autoselected = true;
-		return transport_select(ctx, transport_name(transport_ids));
+		return transport_select(ctx, transport_ids);
 	}
 
 	return ERROR_OK;
@@ -205,7 +227,7 @@ int transport_register(struct transport *new_transport)
 
 	if (!new_transport->select || !new_transport->init)
 		LOG_ERROR("invalid transport %s",
-				  transport_name(new_transport->id));
+				  transport_full_name(new_transport->id));
 
 	/* splice this into the list, sorted in alphabetic order */
 	list_for_each_entry(t, &transport_list, lh) {
@@ -216,7 +238,7 @@ int transport_register(struct transport *new_transport)
 	list_add_tail(&new_transport->lh, &t->lh);
 
 	LOG_DEBUG("register '%s' (ID %d)",
-			  transport_name(new_transport->id), new_transport->id);
+			  transport_full_name(new_transport->id), new_transport->id);
 
 	return ERROR_OK;
 }
@@ -238,7 +260,7 @@ const char *get_current_transport_name(void)
 	if (!session || !is_transport_id_valid(session->id))
 		NULL;
 
-	return transport_name(session->id);
+	return transport_full_name(session->id);
 }
 
 /*-----------------------------------------------------------------------*/
@@ -257,15 +279,15 @@ COMMAND_HANDLER(handle_transport_init)
 		LOG_ERROR("Transports available:");
 		for (unsigned int i = BIT(0); i & TRANSPORT_VALID_MASK; i <<= 1) {
 			if (i & allowed_transports)
-				LOG_ERROR("%s", transport_name(i));
+				LOG_ERROR("%s", transport_full_name(i));
 		}
 		return ERROR_FAIL;
 	}
 
 	if (transport_single_is_autoselected)
 		LOG_WARNING("DEPRECATED: auto-selecting transport \"%s\". "
 			"Use 'transport select %s' to suppress this message.",
-			transport_name(session->id), transport_name(session->id));
+			transport_full_name(session->id), transport_name(session->id));
 
 	return session->init(CMD_CTX);
 }
@@ -278,8 +300,14 @@ COMMAND_HANDLER(handle_transport_list)
 	command_print(CMD, "The following transports are available:");
 
 	struct transport *t;
-	list_for_each_entry(t, &transport_list, lh)
-		command_print(CMD, "\t%s", transport_name(t->id));
+	const char *prev_name = NULL;
+	/* list is sorted, don't print duplicated transport names */
+	list_for_each_entry(t, &transport_list, lh) {
+		const char *name = transport_name(t->id);
+		if (!prev_name || strcmp(prev_name, name))
+			command_print(CMD, "\t%s", name);
+		prev_name = name;
+	}
 
 	return ERROR_OK;
 }
@@ -304,19 +332,21 @@ COMMAND_HANDLER(handle_transport_select)
 			}
 			LOG_WARNING("DEPRECATED: auto-selecting transport \"%s\". "
 				"Use 'transport select %s' to suppress this message.",
-				transport_name(preferred_transport),
+				transport_full_name(preferred_transport),
 				transport_name(preferred_transport));
-			int retval = transport_select(CMD_CTX, transport_name(preferred_transport));
+			int retval = transport_select(CMD_CTX, preferred_transport);
 			if (retval != ERROR_OK)
 				return retval;
 		}
-		command_print(CMD, "%s", transport_name(session->id));
+		command_print(CMD, "%s", transport_full_name(session->id));
 		return ERROR_OK;
 	}
 
 	/* assign transport */
 	if (session) {
-		if (!strcmp(transport_name(session->id), CMD_ARGV[0])) {
+		if (!strcmp(transport_name(session->id), CMD_ARGV[0])
+			|| (transport_deprecated_name(session->id)
+				&& !strcmp(transport_deprecated_name(session->id), CMD_ARGV[0]))) {
 			if (transport_single_is_autoselected) {
 				/* Nothing to do, but also nothing to complain */
 				transport_single_is_autoselected = false;
@@ -341,12 +371,17 @@ COMMAND_HANDLER(handle_transport_select)
 	}
 
 	for (unsigned int i = BIT(0); i & TRANSPORT_VALID_MASK; i <<= 1) {
-		if ((i & allowed_transports)
-			&& !strcmp(transport_name(i), CMD_ARGV[0])) {
-			int retval = transport_select(CMD_CTX, CMD_ARGV[0]);
-			if (retval != ERROR_OK)
-				return retval;
-			return ERROR_OK;
+		if (!(i & allowed_transports))
+			continue;
+
+		if (!strcmp(transport_name(i), CMD_ARGV[0]))
+			return transport_select(CMD_CTX, i);
+
+		if (transport_deprecated_name(i)
+			&& !strcmp(transport_deprecated_name(i), CMD_ARGV[0])) {
+			LOG_WARNING("DEPRECATED! use 'transport select %s', not 'transport select %s'",
+				transport_name(i), transport_deprecated_name(i));
+			return transport_select(CMD_CTX, i);
 		}
 	}