[Pdbg] [PATCH v3 2/2] pdbg: Remove default_backend() selection

Alistair Popple alistair at popple.id.au
Wed Jul 3 13:20:43 AEST 2019


Now that libpdbg can automatically select the correct device-tree the
pdbg application doesn't need code to do the same thing. By default
pdbg can just rely on the device-tree selected by libpdbg.

Unfortunately to maintain backwards compatibility for users explicitly
selecting a device-tree and device-tree node we need to retain most of
the code and continue linking the device-trees into the
application. It would be good to remove these one day so print a
deprecation warning when a user does explicitly select a backend.

Signed-off-by: Alistair Popple <alistair at popple.id.au>
---
 Makefile.am       |  14 +----
 src/main.c        | 121 ++++++++------------------------------------
 src/main.h        |   2 +-
 src/options.h     |  27 ----------
 src/options_arm.c | 149 ------------------------------------------------------
 src/options_def.c |  39 --------------
 src/options_ppc.c |  72 --------------------------
 7 files changed, 23 insertions(+), 401 deletions(-)
 delete mode 100644 src/options.h
 delete mode 100644 src/options_arm.c
 delete mode 100644 src/options_def.c
 delete mode 100644 src/options_ppc.c

diff --git a/Makefile.am b/Makefile.am
index b317b96..e155583 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -70,7 +70,6 @@ endif
 
 DT = fake.dts $(DT_ARM) $(DT_PPC)
 
-DT_objects = $(DT:.dts=.dtb.lo)
 DT_sources = $(DT:.dts=.dtb.S)
 DT_headers = $(DT:.dts=.dt.h)
 
@@ -106,16 +105,6 @@ pdbg_SOURCES = \
 	src/util.c \
 	src/util.h
 
-if TARGET_ARM
-pdbg_SOURCES += src/options_arm.c
-else
-if TARGET_PPC
-pdbg_SOURCES += src/options_ppc.c
-else
-pdbg_SOURCES += src/options_def.c
-endif
-endif
-
 pdbg_CFLAGS = -I$(top_srcdir)/libpdbg -Wall -Werror -DGIT_SHA1=\"${GIT_SHA1}\" \
 	      $(ARCH_FLAGS)
 
@@ -129,12 +118,11 @@ else
 pdbg_CFLAGS += -DDISABLE_GDBSERVER
 endif
 
-src/main.c: $(DT_headers)
 libpdbg/dtb.c: $(DT_headers)
 
 src/pdbg-gdb_parser.$(OBJEXT): CFLAGS+=-Wno-unused-const-variable
 
-pdbg_LDADD = $(DT_objects) libpdbg.la libccan.a \
+pdbg_LDADD = libpdbg.la libccan.a \
 	-L.libs -lrt
 
 lib_LTLIBRARIES = libpdbg.la
diff --git a/src/main.c b/src/main.c
index ad40d19..5ce4fdb 100644
--- a/src/main.c
+++ b/src/main.c
@@ -35,7 +35,6 @@
 
 #include "main.h"
 #include "htm.h"
-#include "options.h"
 #include "optcmd.h"
 #include "progress.h"
 #include "pdbgproxy.h"
@@ -64,7 +63,7 @@
 
 #define THREADS_PER_CORE	8
 
-static enum backend backend = KERNEL;
+static enum backend backend = DEFAULT_BACKEND;
 
 static char const *device_node;
 static int i2c_addr = 0x50;
@@ -407,7 +406,6 @@ static bool parse_options(int argc, char *argv[])
 				backend = HOST;
 			} else {
 				fprintf(stderr, "Invalid backend '%s'\n", optarg);
-				print_backends(stderr);
 				opt_error = true;
 			}
 			break;
@@ -531,96 +529,6 @@ static bool parse_options(int argc, char *argv[])
 	return true;
 }
 
-static bool target_selection(void)
-{
-	switch (backend) {
-#ifdef TARGET_ARM
-	case I2C:
-		pdbg_targets_init(&_binary_p8_i2c_dtb_o_start);
-		/* Set device for I2C backend */
-		if (device_node) {
-			struct pdbg_target *pib;
-			const size_t len = strlen(device_node) + 1 /* include last null */;
-			pdbg_for_each_class_target("pib", pib) {
-				pdbg_target_set_property(pib, "bus", device_node, len);
-			}
-		}
-		break;
-
-	case FSI:
-		if (device_node == NULL) {
-			PR_ERROR("FSI backend requires a device type\n");
-			return false;
-		}
-		if (!strcmp(device_node, "p8"))
-			pdbg_targets_init(&_binary_p8_fsi_dtb_o_start);
-		else if (!strcmp(device_node, "p9w"))
-			pdbg_targets_init(&_binary_p9w_fsi_dtb_o_start);
-		else if (!strcmp(device_node, "p9r"))
-			pdbg_targets_init(&_binary_p9r_fsi_dtb_o_start);
-		else if (!strcmp(device_node, "p9z"))
-			pdbg_targets_init(&_binary_p9z_fsi_dtb_o_start);
-		else {
-			PR_ERROR("Invalid device type specified\n");
-			return false;
-		}
-		break;
-
-	case KERNEL:
-		if (device_node == NULL) {
-			PR_ERROR("kernel backend requires a device type\n");
-                        return false;
-		}
-		if (!strcmp(device_node, "p8"))
-			pdbg_targets_init(&_binary_p8_kernel_dtb_o_start);
-		else
-			pdbg_targets_init(&_binary_p9_kernel_dtb_o_start);
-		break;
-
-#endif
-
-#ifdef TARGET_PPC
-	case HOST:
-		if (device_node == NULL) {
-			PR_ERROR("Host backend requires a device type\n");
-			return false;
-		}
-		if (!strcmp(device_node, "p8"))
-			pdbg_targets_init(&_binary_p8_host_dtb_o_start);
-		else if (!strcmp(device_node, "p9"))
-			pdbg_targets_init(&_binary_p9_host_dtb_o_start);
-		else {
-			PR_ERROR("Unsupported device type for host backend\n");
-			return false;
-		}
-		break;
-#endif
-
-	case FAKE:
-		pdbg_targets_init(&_binary_fake_dtb_o_start);
-		break;
-
-	default:
-		/* parse_options deals with parsing user input, so it should be
-		 * impossible to get here */
-		assert(0);
-		return false;
-	}
-
-	if (pathsel_count) {
-		if (!path_target_parse(pathsel, pathsel_count))
-			return false;
-	}
-
-	if (!path_target_present()) {
-		printf("No valid targets found or specified. Try adding -p/-c/-t options to specify a target.\n");
-		printf("Alternatively run 'pdbg -a probe' to get a list of all valid targets\n");
-		return false;
-	}
-
-	return true;
-}
-
 static void print_target(struct pdbg_target *target, int level)
 {
 	int i;
@@ -677,11 +585,6 @@ int main(int argc, char *argv[])
 	optcmd_cmd_t *cmd;
 	struct pdbg_target *target;
 
-	backend = default_backend();
-
-	if (!device_node)
-		device_node = default_target(backend);
-
 	if (!parse_options(argc, argv))
 		return 1;
 
@@ -690,9 +593,27 @@ int main(int argc, char *argv[])
 		return 1;
 	}
 
-	/* Disable unselected targets */
-	if (!target_selection())
+	if (backend) {
+		fprintf(stderr, "WARNING: Explicit backend selection should no longer be required\n");
+		fprintf(stderr, "         and will be deprecated in a future release.\n");
+		fprintf(stderr, "         Removing -b/-d command line options will remove this warning.\n");
+		fprintf(stderr, "         An explicit device-tree can still be selected using the\n");
+		fprintf(stderr, "         LPDBG_DTB environment variable instead.\n");
+		pdbg_set_backend(backend, device_node);
+	}
+
+	pdbg_targets_init(NULL);
+
+	if (pathsel_count) {
+		if (!path_target_parse(pathsel, pathsel_count))
+			return 1;
+	}
+
+	if (!path_target_present()) {
+		printf("No valid targets found or specified. Try adding -p/-c/-t options to specify a target.\n");
+		printf("Alternatively run 'pdbg -a probe' to get a list of all valid targets\n");
 		return 1;
+	}
 
 	/* Probe all selected targets */
 	for_each_path_target(target) {
diff --git a/src/main.h b/src/main.h
index 78b4d92..1d5913e 100644
--- a/src/main.h
+++ b/src/main.h
@@ -19,7 +19,7 @@
 
 #include <libpdbg.h>
 
-enum backend { FSI, I2C, KERNEL, FAKE, HOST };
+enum backend { DEFAULT_BACKEND = 0, FSI, I2C, KERNEL, FAKE, HOST };
 
 static inline bool target_is_disabled(struct pdbg_target *target)
 {
diff --git a/src/options.h b/src/options.h
deleted file mode 100644
index 67f15a8..0000000
--- a/src/options.h
+++ /dev/null
@@ -1,27 +0,0 @@
-/* Copyright 2017 IBM Corp.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * 	http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
- * implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-/* Default backend on this platform */
-enum backend default_backend(void);
-
-/* Print all possible backends on this platform */
-void print_backends(FILE *stream);
-
-/* The default (perhaps only) target for this backend */
-const char *default_target(enum backend backend);
-
-/* Print all possible targets on this platform */
-void print_targets(FILE *stream);
diff --git a/src/options_arm.c b/src/options_arm.c
deleted file mode 100644
index 0dbc731..0000000
--- a/src/options_arm.c
+++ /dev/null
@@ -1,149 +0,0 @@
-/* Copyright 2017 IBM Corp.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * 	http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
- * implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-#include <stdio.h>
-#include <string.h>
-#include <unistd.h>
-#include <errno.h>
-
-#include "main.h"
-
-#define AMI_BMC "/proc/ractrends/Helper/FwInfo"
-#define OPENFSI_BMC "/sys/bus/platform/devices/gpio-fsi/fsi0/"
-#define FSI_CFAM_ID "/sys/devices/platform/gpio-fsi/fsi0/slave at 00:00/cfam_id"
-
-#define CHIP_ID_P8  0xea
-#define CHIP_ID_P9  0xd1
-#define CHIP_ID_P8P 0xd3
-
-enum backend default_backend(void)
-{
-	int rc;
-	rc = access(AMI_BMC, F_OK);
-	if (rc == 0) /* AMI BMC */
-		return I2C;
-
-	rc = access(OPENFSI_BMC, F_OK);
-	if (rc == 0) /* Kernel interface. OpenBMC */
-		return KERNEL;
-
-	return FAKE;
-}
-
-void print_backends(FILE *stream)
-{
-	fprintf(stream, "Valid backends: i2c kernel fsi fake\n");
-}
-
-void print_targets(FILE *stream)
-{
-	fprintf(stream, "kernel: No target is necessary\n");
-	fprintf(stream, "i2c: No target is necessary\n");
-	fprintf(stream, "fsi: p8 p9w p9r p9z\n");
-}
-
-static const char *default_kernel_target(void)
-{
-	FILE *cfam_id_file;
-
-	/* Try and determine the correct device type */
-	cfam_id_file = fopen(FSI_CFAM_ID, "r");
-	if (cfam_id_file) {
-		uint32_t cfam_id = 0;
-		int rc;
-
-		rc = fscanf(cfam_id_file, "0x%" PRIx32, &cfam_id);
-		if (rc != 1)
-			pdbg_log(PDBG_ERROR, "%s", strerror(errno));
-		fclose(cfam_id_file);
-
-		switch((cfam_id >> 4) & 0xff) {
-		case CHIP_ID_P9:
-			return "p9";
-			break;
-
-		case CHIP_ID_P8:
-		case CHIP_ID_P8P:
-			return "p8";
-			break;
-
-		default:
-			pdbg_log(PDBG_ERROR, "Unknown chip-id detected\n");
-			pdbg_log(PDBG_ERROR, "You will need to specify a host type with -d <host type>\n");
-			return NULL;
-		}
-	} else {
-		/* The support for POWER8 included the cfam_id
-		 * so if it doesn't exist assume we must be on
-		 * P9 */
-		pdbg_log(PDBG_WARNING, "Unable to determine host type, defaulting to p9\n");
-		return "p9";
-	}
-
-	return NULL;
-}
-
-static const char *default_fsi_target(void)
-{
-	FILE *dt_compatible;
-	char line[256];
-	char *p;
-
-	dt_compatible = fopen("/proc/device-tree/compatible", "r");
-	if (!dt_compatible)
-		return NULL;
-
-	p = fgets(line, sizeof(line), dt_compatible);
-	fclose(dt_compatible);
-	if (!p) /* Uh oh*/
-		return NULL;
-
-	if (strstr(line, "witherspoon"))
-		return "p9w";
-
-	if (strstr(line, "romulus"))
-		return "p9r";
-
-	if (strstr(line, "zaius"))
-		return "p9z";
-
-	if (strstr(line, "palmetto"))
-		return "p8";
-
-	return NULL;
-}
-
-const char *default_target(enum backend backend)
-{
-	switch(backend) {
-	case I2C:
-		return NULL;
-		break;
-
-	case KERNEL:
-		return default_kernel_target();
-		break;
-
-	case FSI:
-		return default_fsi_target();
-		break;
-
-	default:
-		return NULL;
-		break;
-	}
-
-	return NULL;
-}
diff --git a/src/options_def.c b/src/options_def.c
deleted file mode 100644
index 4092cea..0000000
--- a/src/options_def.c
+++ /dev/null
@@ -1,39 +0,0 @@
-/* Copyright 2017 IBM Corp.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * 	http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
- * implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-#include <stdio.h>
-#include "main.h"
-
-enum backend default_backend(void)
-{
-	return FAKE;
-}
-
-void print_backends(FILE *stream)
-{
-	fprintf(stream, "Valid backends: fake\n");
-}
-
-/* Theres no target for FAKE backend */
-const char *default_target(enum backend backend)
-{
-	return NULL;
-}
-
-void print_targets(FILE *stream)
-{
-	fprintf(stream, "fake: No target is necessary\n");
-}
diff --git a/src/options_ppc.c b/src/options_ppc.c
deleted file mode 100644
index 62eb7b0..0000000
--- a/src/options_ppc.c
+++ /dev/null
@@ -1,72 +0,0 @@
-/* Copyright 2017 IBM Corp.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * 	http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
- * implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-#include <stdio.h>
-#include <string.h>
-
-#include "main.h"
-
-static const char p8[] = "p8";
-static const char p9[] = "p9";
-
-enum backend default_backend(void)
-{
-	return HOST;
-}
-
-void print_backends(FILE *stream)
-{
-	fprintf(stream, "Valid backends: host fake\n");
-}
-
-const char *default_target(enum backend backend)
-{
-	const char *pos = NULL;
-	char line[256];
-	FILE *cpuinfo;
-
-	cpuinfo = fopen("/proc/cpuinfo", "r");
-	if (!cpuinfo)
-		return NULL;
-
-	while ((pos = fgets(line, sizeof(line), cpuinfo)))
-		if (strncmp(line, "cpu", 3) == 0)
-			break;
-	fclose(cpuinfo);
-
-	if (!pos) /* Got to EOF without a break */
-		return NULL;
-
-	pos = strchr(line, ':');
-	if (!pos)
-		return NULL;
-
-	if (*(pos + 1) == '\0')
-		return NULL;
-	pos += 2;
-
-	if (strncmp(pos, "POWER8", 6) == 0)
-		return p8;
-
-	if (strncmp(pos, "POWER9", 6) == 0)
-		return p9;
-
-	return NULL;
-}
-
-void print_targets(FILE *stream)
-{
-	fprintf(stream, "host: p8 p9\n");
-}
-- 
2.11.0



More information about the Pdbg mailing list