Skip to content

Commit

Permalink
lib: Use VIRTIO_{DRIVER,DEVICE}_SUPPORT to improve readability
Browse files Browse the repository at this point in the history
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 <afd@ti.com>
  • Loading branch information
glneo committed Mar 13, 2024
1 parent 79b795e commit c17bba3
Show file tree
Hide file tree
Showing 7 changed files with 68 additions and 168 deletions.
8 changes: 6 additions & 2 deletions cmake/options.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
10 changes: 0 additions & 10 deletions lib/include/openamp/virtio.h
Original file line number Diff line number Diff line change
Expand Up @@ -85,16 +85,6 @@ __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"
#endif

#ifdef VIRTIO_SLAVE_ONLY
#define VIRTIO_DEVICE_ONLY
#warning "VIRTIO_SLAVE_ONLY is deprecated, please use VIRTIO_DEVICE_ONLY"
#endif

/** @brief Virtio device identifier. */
struct virtio_device_id {
/** Virtio subsystem device ID. */
Expand Down
14 changes: 8 additions & 6 deletions lib/remoteproc/remoteproc.c
Original file line number Diff line number Diff line change
Expand Up @@ -926,13 +926,15 @@ 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;
#endif
if (role == VIRTIO_DEV_DRIVER && !VIRTIO_DRIVER_SUPPORT) {
metal_log(METAL_LOG_ERROR, "VirtIO role is set to Driver, but Driver support is not enabled\n");

Check warning on line 930 in lib/remoteproc/remoteproc.c

View workflow job for this annotation

GitHub Actions / checkpatch review

LONG_LINE_STRING

lib/remoteproc/remoteproc.c:930 line over 100 characters
return NULL;
}

#ifdef VIRTIO_DEVICE_ONLY
role = (role != VIRTIO_DEV_DEVICE) ? 0xFFFFFFFFUL : role;
#endif
if (role == VIRTIO_DEV_DEVICE && !VIRTIO_DEVICE_SUPPORT) {
metal_log(METAL_LOG_ERROR, "VirtIO role is set to Device, but Device support is not enabled\n");

Check warning on line 935 in lib/remoteproc/remoteproc.c

View workflow job for this annotation

GitHub Actions / checkpatch review

LONG_LINE_STRING

lib/remoteproc/remoteproc.c:935 line over 100 characters
return NULL;
}

if (!rproc || (role != VIRTIO_DEV_DEVICE && role != VIRTIO_DEV_DRIVER))
return NULL;
Expand Down
22 changes: 9 additions & 13 deletions lib/remoteproc/remoteproc_virtio.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -144,7 +143,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)
{
Expand Down Expand Up @@ -199,7 +198,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)
{
Expand Down Expand Up @@ -249,7 +248,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)
{
Expand Down Expand Up @@ -288,7 +287,7 @@ static const struct virtio_dispatch remoteproc_virtio_dispatch_funcs = {
.read_config = rproc_virtio_read_config,
.notify = rproc_virtio_virtqueue_notify,
.wait_notified = rproc_virtio_wait_notified,
#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
Expand Down Expand Up @@ -343,13 +342,11 @@ 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 (role == VIRTIO_DEV_DRIVER) {
if (VIRTIO_DRIVER_SUPPORT && role == VIRTIO_DEV_DRIVER) {
uint32_t dfeatures = rproc_virtio_get_dfeatures(vdev);
/* Assume the virtio driver support all remote features */
rproc_virtio_negotiate_features(vdev, dfeatures);
}
#endif

return &rpvdev->vdev;
err:
Expand Down Expand Up @@ -417,15 +414,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)
Expand Down
Loading

0 comments on commit c17bba3

Please sign in to comment.