lib: Use VIRTIO_{DRIVER,DEVICE}_SUPPORT to improve readability

Currently compiler defines are defined when support for driver or device is the only support being built. This is a negative define, it surrounds the code to not be built and we use ifndef. This is confusing. It also leaves ifndefs all throughout the code-base. Instead, define a macro that is set to 1 if support is enabled. Use this inline in if statements where possible. Any sane compiler will optimize away the code in the branch when support is not enabled just the same as when using the preprocessor so we keep the same binary size. Signed-off-by: Andrew Davis <[email protected]>
OpenAMP · Mar 19, 2024 · 1a71c61 · 1a71c61
1 parent 15b4a8b
commit 1a71c61
Show file tree

Hide file tree

Showing 7 changed files with 70 additions and 160 deletions.
diff --git a/cmake/options.cmake b/cmake/options.cmake
@@ -71,11 +71,15 @@ if (WITH_VIRTIO_SLAVE)
 endif (WITH_VIRTIO_SLAVE)
 
 if (NOT WITH_VIRTIO_DRIVER AND NOT WITH_VIRTIO_MASTER)
-	add_definitions(-DVIRTIO_DEVICE_ONLY)
+	add_definitions(-DVIRTIO_DRIVER_SUPPORT=0)
+else (NOT WITH_VIRTIO_DRIVER AND NOT WITH_VIRTIO_MASTER)
+	add_definitions(-DVIRTIO_DRIVER_SUPPORT=1)
 endif (NOT WITH_VIRTIO_DRIVER AND NOT WITH_VIRTIO_MASTER)
 
 if (NOT WITH_VIRTIO_DEVICE AND NOT WITH_VIRTIO_SLAVE)
-	add_definitions(-DVIRTIO_DRIVER_ONLY)
+	add_definitions(-DVIRTIO_DEVICE_SUPPORT=0)
+else (NOT WITH_VIRTIO_DEVICE AND NOT WITH_VIRTIO_SLAVE)
+	add_definitions(-DVIRTIO_DEVICE_SUPPORT=1)
 endif (NOT WITH_VIRTIO_DEVICE AND NOT WITH_VIRTIO_SLAVE)
 
 option (WITH_VIRTIO_MMIO_DRV "Build with virtio mmio driver support enabled" OFF)

diff --git a/lib/include/openamp/virtio.h b/lib/include/openamp/virtio.h
@@ -85,14 +85,12 @@ __deprecated static inline int deprecated_virtio_dev_slave(void)
 	return VIRTIO_DEV_DEVICE;
 }
 
-#ifdef VIRTIO_MASTER_ONLY
-#define VIRTIO_DRIVER_ONLY
-#warning "VIRTIO_MASTER_ONLY is deprecated, please use VIRTIO_DRIVER_ONLY"
+#if defined(VIRTIO_MASTER_ONLY) || defined(VIRTIO_DRIVER_ONLY)
+#warning "VIRTIO_MASTER_ONLY and VIRTIO_DRIVER_ONLY are deprecated, please use VIRTIO_DEVICE_SUPPORT=0"
 #endif
 
-#ifdef VIRTIO_SLAVE_ONLY
-#define VIRTIO_DEVICE_ONLY
-#warning "VIRTIO_SLAVE_ONLY is deprecated, please use VIRTIO_DEVICE_ONLY"
+#if defined(VIRTIO_SLAVE_ONLY) || defined(VIRTIO_DEVICE_ONLY)
+#warning "VIRTIO_SLAVE_ONLY and VIRTIO_DEVICE_ONLY are deprecated, please use VIRTIO_DRIVER_SUPPORT=0"
 #endif
 
 /** @brief Virtio device identifier. */

diff --git a/lib/remoteproc/remoteproc.c b/lib/remoteproc/remoteproc.c
@@ -916,12 +916,14 @@ remoteproc_create_virtio(struct remoteproc *rproc,
 	unsigned int num_vrings, i;
 	struct metal_list *node;
 
-#ifdef VIRTIO_DRIVER_ONLY
-	role = (role != VIRTIO_DEV_DRIVER) ? 0xFFFFFFFFUL : role;
+#if !VIRTIO_DRIVER_SUPPORT
+	if (role == VIRTIO_DEV_DRIVER)
+		return NULL;
 #endif
 
-#ifdef VIRTIO_DEVICE_ONLY
-	role = (role != VIRTIO_DEV_DEVICE) ? 0xFFFFFFFFUL : role;
+#if !VIRTIO_DEVICE_SUPPORT
+	if (role == VIRTIO_DEV_DEVICE)
+		return NULL;
 #endif
 
 	if (!rproc || (role != VIRTIO_DEV_DEVICE && role != VIRTIO_DEV_DRIVER))

diff --git a/lib/remoteproc/remoteproc_virtio.c b/lib/remoteproc/remoteproc_virtio.c
@@ -55,14 +55,13 @@ static int rproc_virtio_create_virtqueue(struct virtio_device *vdev,
 	if (!vring_info->vq)
 		return ERROR_NO_MEM;
 
-#ifndef VIRTIO_DEVICE_ONLY
-	if (vdev->role == VIRTIO_DEV_DRIVER) {
+	if (VIRTIO_DRIVER_SUPPORT && vdev->role == VIRTIO_DEV_DRIVER) {
 		size_t offset = metal_io_virt_to_offset(vring_info->io, vring_alloc->vaddr);
 		size_t size = vring_size(vring_alloc->num_descs, vring_alloc->align);
 
 		metal_io_block_set(vring_info->io, offset, 0, size);
 	}
-#endif
+
 	ret = virtqueue_create(vdev, idx, name, vring_alloc, callback,
 			       vdev->func->notify, vring_info->vq);
 	if (ret)
@@ -129,7 +128,7 @@ static unsigned char rproc_virtio_get_status(struct virtio_device *vdev)
 	return status;
 }
 
-#ifndef VIRTIO_DEVICE_ONLY
+#if VIRTIO_DRIVER_SUPPORT
 static void rproc_virtio_set_status(struct virtio_device *vdev,
 				    unsigned char status)
 {
@@ -184,7 +183,7 @@ static uint32_t rproc_virtio_get_features(struct virtio_device *vdev)
 	return dfeatures & gfeatures;
 }
 
-#ifndef VIRTIO_DEVICE_ONLY
+#if VIRTIO_DRIVER_SUPPORT
 static void rproc_virtio_set_features(struct virtio_device *vdev,
 				      uint32_t features)
 {
@@ -234,7 +233,7 @@ static void rproc_virtio_read_config(struct virtio_device *vdev,
 	}
 }
 
-#ifndef VIRTIO_DEVICE_ONLY
+#if VIRTIO_DRIVER_SUPPORT
 static void rproc_virtio_write_config(struct virtio_device *vdev,
 				      uint32_t offset, void *src, int length)
 {
@@ -272,7 +271,7 @@ static const struct virtio_dispatch remoteproc_virtio_dispatch_funcs = {
 	.get_features = rproc_virtio_get_features,
 	.read_config = rproc_virtio_read_config,
 	.notify = rproc_virtio_virtqueue_notify,
-#ifndef VIRTIO_DEVICE_ONLY
+#if VIRTIO_DRIVER_SUPPORT
 	/*
 	 * We suppose here that the vdev is in a shared memory so that can
 	 * be access only by one core: the host. In this case salve core has
@@ -327,7 +326,7 @@ rproc_virtio_create_vdev(unsigned int role, unsigned int notifyid,
 	vdev->vrings_num = num_vrings;
 	vdev->func = &remoteproc_virtio_dispatch_funcs;
 
-#ifndef VIRTIO_DEVICE_ONLY
+#if VIRTIO_DRIVER_SUPPORT
 	if (role == VIRTIO_DEV_DRIVER) {
 		uint32_t dfeatures = rproc_virtio_get_dfeatures(vdev);
 		/* Assume the virtio driver support all remote features */
@@ -401,15 +400,14 @@ void rproc_virtio_wait_remote_ready(struct virtio_device *vdev)
 {
 	uint8_t status;
 
-#ifndef VIRTIO_DEVICE_ONLY
 	/*
 	 * No status available for remote. As virtio driver has not to wait
 	 * remote action, we can return. Behavior should be updated
 	 * in future if a remote status is added.
 	 */
-	if (vdev->role == VIRTIO_DEV_DRIVER)
+	if (VIRTIO_DRIVER_SUPPORT && vdev->role == VIRTIO_DEV_DRIVER)
 		return;
-#endif
+
 	while (1) {
 		status = rproc_virtio_get_status(vdev);
 		if (status & VIRTIO_CONFIG_STATUS_DRIVER_OK)