From 3358f55c6ae972e8f2e7a8336177dbed143b2fe4 Mon Sep 17 00:00:00 2001 From: Michael Hennerich Date: Mon, 29 Sep 2025 13:08:36 +0200 Subject: [PATCH 1/2] libiio: backend: Add atomic register access support for local backend This patch introduces thread-safe register read/write operations for the local backend using `direct_reg_access` via debugfs. It adds two new backend ops: - `reg_read()` - `reg_write()` These functions use file locking (`flock`) to ensure atomic access and prevent race conditions when multiple threads interact with the same register interface. The fallback logic in `iio_device_reg_read()` and `iio_device_reg_write()` is preserved for backends that do not implement these atomic operations. This improves reliability for multi-threaded applications using libiio with the local backend. Signed-off-by: Michael Hennerich --- device.c | 22 ++++++-- include/iio/iio-backend.h | 4 ++ local.c | 114 +++++++++++++++++++++++++++++++++++++- 3 files changed, 135 insertions(+), 5 deletions(-) diff --git a/device.c b/device.c index 4245f6b47..3de2f6362 100644 --- a/device.c +++ b/device.c @@ -299,6 +299,15 @@ int iio_device_reg_write(struct iio_device *dev, uint32_t address, uint32_t value) { const struct iio_attr *attr; + const struct iio_backend_ops *ops = dev->ctx->ops; + + /* Use atomic backend operation if available (thread-safe for local backend) */ + if (ops && ops->reg_write) { + return ops->reg_write(dev, address, value); + } + + /* Fallback to the original implementation for other backends. + * NOTE: This has a potential race condition with reg_read operations. */ ssize_t ret; char buf[1024]; @@ -317,11 +326,16 @@ int iio_device_reg_write(struct iio_device *dev, int iio_device_reg_read(struct iio_device *dev, uint32_t address, uint32_t *value) { - /* NOTE: There is a race condition here. But it is extremely unlikely to - * happen, and as this is a debug function, it shouldn't be used for - * something else than debug. */ - const struct iio_attr *attr; + const struct iio_backend_ops *ops = dev->ctx->ops; + + /* Use atomic backend operation if available (thread-safe for local backend) */ + if (ops && ops->reg_read) { + return ops->reg_read(dev, address, value); + } + + /* Fallback to the original implementation for other backends. + * NOTE: This has a race condition but it's unlikely to happen in practice. */ long long val; int ret; diff --git a/include/iio/iio-backend.h b/include/iio/iio-backend.h index 8efc18ace..d7db3bb0d 100644 --- a/include/iio/iio-backend.h +++ b/include/iio/iio-backend.h @@ -138,6 +138,10 @@ struct iio_backend_ops { int (*read_ev)(struct iio_event_stream_pdata *pdata, struct iio_event *out_event, bool nonblock); + + /* Atomic register operations - optional, for thread-safe local backend */ + int (*reg_read)(const struct iio_device *dev, uint32_t address, uint32_t *value); + int (*reg_write)(const struct iio_device *dev, uint32_t address, uint32_t value); }; /** diff --git a/local.c b/local.c index b0eed8fc6..982bf3359 100644 --- a/local.c +++ b/local.c @@ -34,6 +34,8 @@ #include #include #include +#include +#include #define NB_BLOCKS 4 @@ -495,7 +497,6 @@ static ssize_t local_do_read_dev_attr(const char *id, unsigned int buf_id, else dst[0] = '\0'; - fflush(f); if (ferror(f)) ret = -errno; fclose(f); @@ -639,6 +640,115 @@ static int local_set_trigger(const struct iio_device *dev, return 0; } +static int local_reg_read(const struct iio_device *dev, + uint32_t address, uint32_t *value) +{ + char buf[1024]; + char addr_str[32]; + char val_str[32]; + FILE *f; + int fd; + ssize_t ret; + + iio_snprintf(buf, sizeof(buf), "/sys/kernel/debug/iio/%s/direct_reg_access", + dev->id); + + f = fopen(buf, "r+"); + if (!f) + return -errno; + + fd = fileno(f); + + /* Get exclusive lock for atomic write-then-read */ + if (flock(fd, LOCK_EX) < 0) { + ret = -errno; + fclose(f); + return ret; + } + + iio_snprintf(addr_str, sizeof(addr_str), "0x%" PRIx32, address); + ret = fwrite(addr_str, 1, strlen(addr_str), f); + fflush(f); + if ((size_t)ret < strlen(addr_str)) { + if (ferror(f)) + ret = -errno; + else + ret = -EIO; + + goto unlock; + } + + rewind(f); + ret = fread(val_str, 1, sizeof(val_str) - 1, f); + if (!feof(f)){ + ret = -EFBIG; + goto unlock; + } + if (ferror(f)) { + ret = -errno; + goto unlock; + } + + if (ret > 0) + val_str[ret - 1] = '\0'; + else + val_str[0] = '\0'; + + if (sscanf(val_str, "0x%x", value) != 1) { + ret = -EIO; + goto unlock; + } + + ret = 0; + +unlock: + flock(fd, LOCK_UN); + fclose(f); + return ret; +} + +static int local_reg_write(const struct iio_device *dev, + uint32_t address, uint32_t value) +{ + char buf[1024]; + char write_str[64]; + FILE *f; + int fd; + ssize_t ret; + + iio_snprintf(buf, sizeof(buf), "/sys/kernel/debug/iio/%s/direct_reg_access", + dev->id); + + f = fopen(buf, "w"); + if (!f) + return -errno; + + fd = fileno(f); + + /* Get exclusive lock for atomic write operation */ + if (flock(fd, LOCK_EX) < 0) { + ret = -errno; + fclose(f); + return ret; + } + + iio_snprintf(write_str, sizeof(write_str), "0x%" PRIx32 " 0x%" PRIx32, address, value); + ret = fwrite(write_str, 1, strlen(write_str), f); + fflush(f); + if ((size_t)ret < strlen(write_str)) { + if (ferror(f)) + ret = -errno; + else + ret = -EIO; + } else { + ret = 0; + } + + flock(fd, LOCK_UN); + fclose(f); + return ret; +} + static bool is_channel(const struct iio_device *dev, const char *attr, bool strict) { char *ptr = NULL; @@ -1768,6 +1878,8 @@ static const struct iio_backend_ops local_ops = { .get_dmabuf_fd = local_get_dmabuf_fd, .disable_cpu_access = local_disable_cpu_access, + .reg_read = local_reg_read, + .reg_write = local_reg_write, }; const struct iio_backend iio_local_backend = { From 22331ae12ad178b7c347bc65d0454a5ec303b5a7 Mon Sep 17 00:00:00 2001 From: Dan Nechita Date: Thu, 20 Nov 2025 14:30:01 +0200 Subject: [PATCH 2/2] treewide: New OPCODES for register read & write Implement atomic register operations which ensures that a register read or a register write cannot be interrupted by other clients of IIOD. Advantages: - half round trips: single command vs write-address + read-value - binary addresses/values vs hex string converion On client-server compatibility: v1.0 clients can connect to v1.0+ servers (atomic ops used) v1.0 clients connecting to v0.x servers fall back to attribute method Signed-off-by: Dan Nechita --- device.c | 12 +++---- iiod-client.c | 49 ++++++++++++++++++++++++++++ iiod-responder.h | 3 ++ iiod/responder.c | 67 +++++++++++++++++++++++++++++++++++++++ include/iio/iio-backend.h | 2 +- include/iio/iiod-client.h | 7 ++++ network.c | 33 +++++++++++++++++++ serial.c | 33 +++++++++++++++++++ usb.c | 33 +++++++++++++++++++ 9 files changed, 232 insertions(+), 7 deletions(-) diff --git a/device.c b/device.c index 3de2f6362..9dbd7e744 100644 --- a/device.c +++ b/device.c @@ -301,13 +301,14 @@ int iio_device_reg_write(struct iio_device *dev, const struct iio_attr *attr; const struct iio_backend_ops *ops = dev->ctx->ops; - /* Use atomic backend operation if available (thread-safe for local backend) */ + /* Use atomic backend operation if available */ if (ops && ops->reg_write) { return ops->reg_write(dev, address, value); } - /* Fallback to the original implementation for other backends. - * NOTE: This has a potential race condition with reg_read operations. */ + /* Fallback to the original attribute-based implementation. + * NOTE: This has a potential race condition with reg_read operations + * when multiple clients access the same IIOD server. */ ssize_t ret; char buf[1024]; @@ -329,13 +330,12 @@ int iio_device_reg_read(struct iio_device *dev, const struct iio_attr *attr; const struct iio_backend_ops *ops = dev->ctx->ops; - /* Use atomic backend operation if available (thread-safe for local backend) */ + /* Use atomic backend operation if available */ if (ops && ops->reg_read) { return ops->reg_read(dev, address, value); } - /* Fallback to the original implementation for other backends. - * NOTE: This has a race condition but it's unlikely to happen in practice. */ + /* Fallback to the original attribute-based implementation. */ long long val; int ret; diff --git a/iiod-client.c b/iiod-client.c index faab598d8..397f30d60 100644 --- a/iiod-client.c +++ b/iiod-client.c @@ -1885,3 +1885,52 @@ int iiod_client_read_event(struct iio_event_stream_pdata *pdata, return ret; } + +int iiod_client_reg_read(struct iiod_client *client, + const struct iio_device *dev, + uint32_t address, uint32_t *value) +{ + struct iiod_io *io = iiod_responder_get_default_io(client->responder); + struct iiod_command cmd; + struct iiod_buf buf; + int ret; + + cmd.op = IIOD_OP_REG_READ; + cmd.dev = (uint8_t) iio_device_get_index(dev); + cmd.code = (int32_t) address; + + buf.ptr = value; + buf.size = sizeof(*value); + + ret = iiod_io_exec_command(io, &cmd, NULL, &buf); + if (ret < 0) + return ret; + + if (ret != sizeof(*value)) + return -EIO; + + return 0; +} + +int iiod_client_reg_write(struct iiod_client *client, + const struct iio_device *dev, + uint32_t address, uint32_t value) +{ + struct iiod_io *io = iiod_responder_get_default_io(client->responder); + struct iiod_command cmd; + struct iiod_buf buf; + int ret; + + cmd.op = IIOD_OP_REG_WRITE; + cmd.dev = (uint8_t) iio_device_get_index(dev); + cmd.code = (int32_t) address; + + buf.ptr = &value; + buf.size = sizeof(value); + + ret = iiod_io_send_command(io, &cmd, &buf, 1); + if (ret < 0) + return ret; + + return iiod_io_wait_for_command_done(io); +} diff --git a/iiod-responder.h b/iiod-responder.h index 24e96add0..c1f4f26d3 100644 --- a/iiod-responder.h +++ b/iiod-responder.h @@ -55,6 +55,9 @@ enum iiod_opcode { IIOD_OP_FREE_EVSTREAM, IIOD_OP_READ_EVENT, + IIOD_OP_REG_READ, + IIOD_OP_REG_WRITE, + IIOD_NB_OPCODES, }; diff --git a/iiod/responder.c b/iiod/responder.c index 7dcc7fbfa..0ce61c3a4 100644 --- a/iiod/responder.c +++ b/iiod/responder.c @@ -1170,6 +1170,70 @@ static void handle_read_event(struct parser_pdata *pdata, } } +static void handle_reg_read(struct parser_pdata *pdata, + const struct iiod_command *cmd, + struct iiod_command_data *cmd_data) +{ + struct iiod_io *io = iiod_command_get_default_io(cmd_data); + const struct iio_context *ctx = pdata->ctx; + const struct iio_device *dev; + uint32_t address, value; + struct iiod_buf buf; + int ret; + + dev = iio_context_get_device(ctx, cmd->dev); + if (!dev) { + ret = -ENODEV; + goto out_send_response; + } + + address = (uint32_t) cmd->code; + + ret = iio_device_reg_read((struct iio_device *)dev, address, &value); + if (ret < 0) { + iiod_io_send_response_code(io, ret); + } else { + buf.ptr = &value; + buf.size = sizeof(value); + iiod_io_send_response(io, sizeof(value), &buf, 1); + } + + return; + +out_send_response: + iiod_io_send_response_code(io, ret); +} + +static void handle_reg_write(struct parser_pdata *pdata, + const struct iiod_command *cmd, + struct iiod_command_data *cmd_data) +{ + struct iiod_io *io = iiod_command_get_default_io(cmd_data); + const struct iio_context *ctx = pdata->ctx; + const struct iio_device *dev; + uint32_t address, value; + struct iiod_buf buf; + int ret; + + dev = iio_context_get_device(ctx, cmd->dev); + if (!dev) { + ret = -ENODEV; + goto out_send_response; + } + + address = (uint32_t) cmd->code; + buf.ptr = &value; + buf.size = sizeof(value); + ret = iiod_command_data_read(cmd_data, &buf); + if (ret < 0) + goto out_send_response; + + ret = iio_device_reg_write((struct iio_device *)dev, address, value); + +out_send_response: + iiod_io_send_response_code(io, ret); +} + typedef void (*iiod_opcode_fn)(struct parser_pdata *, const struct iiod_command *, struct iiod_command_data *cmd_data); @@ -1202,6 +1266,9 @@ static const iiod_opcode_fn iiod_op_functions[] = { [IIOD_OP_CREATE_EVSTREAM] = handle_create_evstream, [IIOD_OP_FREE_EVSTREAM] = handle_free_evstream, [IIOD_OP_READ_EVENT] = handle_read_event, + + [IIOD_OP_REG_READ] = handle_reg_read, + [IIOD_OP_REG_WRITE] = handle_reg_write, }; static int iiod_cmd(const struct iiod_command *cmd, diff --git a/include/iio/iio-backend.h b/include/iio/iio-backend.h index d7db3bb0d..d8d88efb8 100644 --- a/include/iio/iio-backend.h +++ b/include/iio/iio-backend.h @@ -139,7 +139,7 @@ struct iio_backend_ops { struct iio_event *out_event, bool nonblock); - /* Atomic register operations - optional, for thread-safe local backend */ + /* Atomic register operations */ int (*reg_read)(const struct iio_device *dev, uint32_t address, uint32_t *value); int (*reg_write)(const struct iio_device *dev, uint32_t address, uint32_t value); }; diff --git a/include/iio/iiod-client.h b/include/iio/iiod-client.h index e66c6d0ac..01967f62a 100644 --- a/include/iio/iiod-client.h +++ b/include/iio/iiod-client.h @@ -101,6 +101,13 @@ __api int iiod_client_read_event(struct iio_event_stream_pdata *pdata, struct iio_event *out_event, bool nonblock); +__api int iiod_client_reg_read(struct iiod_client *client, + const struct iio_device *dev, + uint32_t address, uint32_t *value); +__api int iiod_client_reg_write(struct iiod_client *client, + const struct iio_device *dev, + uint32_t address, uint32_t value); + #undef __api #endif /* _IIOD_CLIENT_H */ diff --git a/network.c b/network.c index 8248bb757..5d2b76557 100644 --- a/network.c +++ b/network.c @@ -513,6 +513,36 @@ network_open_events_fd(const struct iio_device *dev) return iiod_client_open_event_stream(pdata->iiod_client, dev); } +static int network_reg_read(const struct iio_device *dev, + uint32_t address, uint32_t *value) +{ + const struct iio_context *ctx = iio_device_get_context(dev); + struct iio_context_pdata *pdata = iio_context_get_pdata(ctx); + struct iiod_client *client = pdata->iiod_client; + + if (!iiod_client_uses_binary_interface(client)) { + /* Fallback to attribute-based method for ASCII protocol */ + return -ENOSYS; + } + + return iiod_client_reg_read(client, dev, address, value); +} + +static int network_reg_write(const struct iio_device *dev, + uint32_t address, uint32_t value) +{ + const struct iio_context *ctx = iio_device_get_context(dev); + struct iio_context_pdata *pdata = iio_context_get_pdata(ctx); + struct iiod_client *client = pdata->iiod_client; + + if (!iiod_client_uses_binary_interface(client)) { + /* Fallback to attribute-based method for ASCII protocol */ + return -ENOSYS; + } + + return iiod_client_reg_write(client, dev, address, value); +} + static const struct iio_backend_ops network_ops = { .scan = IF_ENABLED(HAVE_DNS_SD, dnssd_context_scan), .create = network_create_context, @@ -539,6 +569,9 @@ static const struct iio_backend_ops network_ops = { .open_ev = network_open_events_fd, .close_ev = iiod_client_close_event_stream, .read_ev = iiod_client_read_event, + + .reg_read = network_reg_read, + .reg_write = network_reg_write, }; __api_export_if(WITH_NETWORK_BACKEND_DYNAMIC) diff --git a/serial.c b/serial.c index bda81e867..0aeeece4e 100644 --- a/serial.c +++ b/serial.c @@ -291,6 +291,36 @@ serial_open_events_fd(const struct iio_device *dev) return iiod_client_open_event_stream(pdata->iiod_client, dev); } +static int serial_reg_read(const struct iio_device *dev, + uint32_t address, uint32_t *value) +{ + const struct iio_context *ctx = iio_device_get_context(dev); + struct iio_context_pdata *pdata = iio_context_get_pdata(ctx); + struct iiod_client *client = pdata->iiod_client; + + if (!iiod_client_uses_binary_interface(client)) { + /* Fallback to attribute-based method for ASCII protocol */ + return -ENOSYS; + } + + return iiod_client_reg_read(client, dev, address, value); +} + +static int serial_reg_write(const struct iio_device *dev, + uint32_t address, uint32_t value) +{ + const struct iio_context *ctx = iio_device_get_context(dev); + struct iio_context_pdata *pdata = iio_context_get_pdata(ctx); + struct iiod_client *client = pdata->iiod_client; + + if (!iiod_client_uses_binary_interface(client)) { + /* Fallback to attribute-based method for ASCII protocol */ + return -ENOSYS; + } + + return iiod_client_reg_write(client, dev, address, value); +} + static const struct iio_backend_ops serial_ops = { .create = serial_create_context_from_args, .read_attr = serial_read_attr, @@ -314,6 +344,9 @@ static const struct iio_backend_ops serial_ops = { .open_ev = serial_open_events_fd, .close_ev = iiod_client_close_event_stream, .read_ev = iiod_client_read_event, + + .reg_read = serial_reg_read, + .reg_write = serial_reg_write, }; __api_export_if(WITH_SERIAL_BACKEND_DYNAMIC) diff --git a/usb.c b/usb.c index 506a26c17..4b480d6b9 100644 --- a/usb.c +++ b/usb.c @@ -275,6 +275,36 @@ static int usb_set_timeout(struct iio_context *ctx, unsigned int timeout) return iiod_client_set_timeout(pdata->io_ctx.iiod_client, timeout); } +static int usb_reg_read(const struct iio_device *dev, + uint32_t address, uint32_t *value) +{ + const struct iio_context *ctx = iio_device_get_context(dev); + struct iio_context_pdata *pdata = iio_context_get_pdata(ctx); + struct iiod_client *client = pdata->io_ctx.iiod_client; + + if (!iiod_client_uses_binary_interface(client)) { + /* Fallback to attribute-based method for ASCII protocol */ + return -ENOSYS; + } + + return iiod_client_reg_read(client, dev, address, value); +} + +static int usb_reg_write(const struct iio_device *dev, + uint32_t address, uint32_t value) +{ + const struct iio_context *ctx = iio_device_get_context(dev); + struct iio_context_pdata *pdata = iio_context_get_pdata(ctx); + struct iiod_client *client = pdata->io_ctx.iiod_client; + + if (!iiod_client_uses_binary_interface(client)) { + /* Fallback to attribute-based method for ASCII protocol */ + return -ENOSYS; + } + + return iiod_client_reg_write(client, dev, address, value); +} + static const struct iio_device * usb_get_trigger(const struct iio_device *dev) { const struct iio_context *ctx = iio_device_get_context(dev); @@ -538,6 +568,9 @@ static const struct iio_backend_ops usb_ops = { .open_ev = usb_open_events_fd, .close_ev = iiod_client_close_event_stream, .read_ev = iiod_client_read_event, + + .reg_read = usb_reg_read, + .reg_write = usb_reg_write, }; __api_export_if(WITH_USB_BACKEND_DYNAMIC)