From 3d9b81e7f1a1f26f42dce4b768b4a088f73c8bf0 Mon Sep 17 00:00:00 2001 From: Gidon Studinski Date: Thu, 7 Nov 2019 16:58:20 +0200 Subject: [PATCH 1/8] wigig_sensing: add GET_NUM_AVAIL_BURSTS ioctl WIGIG_SENSING_IOCTL_GET_NUM_AVAIL_BURSTS returns the number of available bursts in the drivers buffer. Change-Id: I474770a817393e1b64831886803d65a3f76d92e0 Signed-off-by: Gidon Studinski Signed-off-by: Alexei Avshalom Lazar --- drivers/misc/wigig_sensing.c | 14 ++++++++++++++ include/uapi/misc/wigig_sensing_uapi.h | 10 +++++++++- 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/drivers/misc/wigig_sensing.c b/drivers/misc/wigig_sensing.c index ec64b6531dc3..21446e2f45e9 100644 --- a/drivers/misc/wigig_sensing.c +++ b/drivers/misc/wigig_sensing.c @@ -629,6 +629,16 @@ static int wigig_sensing_ioc_get_num_dropped_bursts( return ctx->dropped_bursts; } +static int wigig_sensing_ioc_get_num_avail_bursts( + struct wigig_sensing_ctx *ctx) +{ + if (ctx->stm.burst_size) + return circ_cnt(&ctx->cir_data.b, ctx->cir_data.size_bytes) / + ctx->stm.burst_size; + else + return 0; +} + static int wigig_sensing_ioc_get_event(struct wigig_sensing_ctx *ctx) { return 0; @@ -814,6 +824,10 @@ static long wigig_sensing_ioctl(struct file *file, unsigned int cmd, pr_info("Received WIGIG_SENSING_IOCTL_GET_EVENT command\n"); rc = wigig_sensing_ioc_get_event(ctx); break; + case WIGIG_SENSING_IOCTL_GET_NUM_AVAIL_BURSTS: + pr_info("Received WIGIG_SENSING_IOCTL_GET_NUM_AVAIL_BURSTS command\n"); + rc = wigig_sensing_ioc_get_num_avail_bursts(ctx); + break; default: rc = -EINVAL; break; diff --git a/include/uapi/misc/wigig_sensing_uapi.h b/include/uapi/misc/wigig_sensing_uapi.h index 6ab94f3ef260..c53d6bfa6010 100644 --- a/include/uapi/misc/wigig_sensing_uapi.h +++ b/include/uapi/misc/wigig_sensing_uapi.h @@ -39,6 +39,7 @@ enum wigig_sensing_event { #define WIGIG_SENSING_IOCTL_CLEAR_DATA (3) #define WIGIG_SENSING_IOCTL_GET_NUM_DROPPED_BURSTS (4) #define WIGIG_SENSING_IOCTL_GET_EVENT (5) +#define WIGIG_SENSING_IOCTL_GET_NUM_AVAIL_BURSTS (6) /** * Set auto recovery, which means that the system will go back to search mode @@ -89,4 +90,11 @@ enum wigig_sensing_event { _IOR(WIGIG_SENSING_IOC_MAGIC, WIGIG_SENSING_IOCTL_GET_EVENT, \ sizeof(enum wigig_sensing_event)) -#endif /* ____WIGIG_SENSING_UAPI_H__ */ +/** + * Get number of available bursts in the data buffer + */ +#define WIGIG_SENSING_IOC_GET_NUM_AVAIL_BURSTS \ + _IOR(WIGIG_SENSING_IOC_MAGIC, WIGIG_SENSING_IOCTL_GET_NUM_AVAIL_BURSTS,\ + sizeof(uint32_t)) + +#endif /* __WIGIG_SENSING_UAPI_H__ */ From 03547d5eb7a2109306a6bd2b2f44e93b019dfc22 Mon Sep 17 00:00:00 2001 From: Alexei Avshalom Lazar Date: Sun, 19 Apr 2020 12:39:35 +0300 Subject: [PATCH 2/8] wigig_sensing: add support for asynchronous events Driver can now send asynchronous events to user space application. Available events are RESET and FW_READY. Change-Id: I8b3819927ac3e55b69be03d288600c81ad23e319 Signed-off-by: Gidon Studinski Signed-off-by: Alexei Avshalom Lazar --- drivers/misc/wigig_sensing.c | 44 +++++++++++++++++++++++--- drivers/misc/wigig_sensing.h | 2 ++ include/uapi/misc/wigig_sensing_uapi.h | 4 ++- 3 files changed, 44 insertions(+), 6 deletions(-) diff --git a/drivers/misc/wigig_sensing.c b/drivers/misc/wigig_sensing.c index 21446e2f45e9..e3758b116338 100644 --- a/drivers/misc/wigig_sensing.c +++ b/drivers/misc/wigig_sensing.c @@ -2,7 +2,6 @@ /* * Copyright (c) 2019, The Linux foundation. All rights reserved. */ - #include #include #include @@ -15,6 +14,7 @@ #include #include #include +#include #include #include #include @@ -639,9 +639,19 @@ static int wigig_sensing_ioc_get_num_avail_bursts( return 0; } -static int wigig_sensing_ioc_get_event(struct wigig_sensing_ctx *ctx) +static int wigig_sensing_ioc_get_event(struct wigig_sensing_ctx *ctx, + enum wigig_sensing_event *event) { - return 0; + u32 copied; + + if (!ctx->event_pending) + return -EINVAL; + + if (kfifo_len(&ctx->events_fifo) == 1) + ctx->event_pending = false; + + return kfifo_to_user(&ctx->events_fifo, event, + sizeof(enum wigig_sensing_event), &copied); } static int wigig_sensing_open(struct inode *inode, struct file *filp) @@ -769,7 +779,7 @@ static int wigig_sensing_release(struct inode *inode, struct file *filp) } static long wigig_sensing_ioctl(struct file *file, unsigned int cmd, - unsigned long arg) + __user unsigned long arg) { int rc; struct wigig_sensing_ctx *ctx = file->private_data; @@ -822,7 +832,8 @@ static long wigig_sensing_ioctl(struct file *file, unsigned int cmd, break; case WIGIG_SENSING_IOCTL_GET_EVENT: pr_info("Received WIGIG_SENSING_IOCTL_GET_EVENT command\n"); - rc = wigig_sensing_ioc_get_event(ctx); + rc = wigig_sensing_ioc_get_event(ctx, + (enum wigig_sensing_event *)arg); break; case WIGIG_SENSING_IOCTL_GET_NUM_AVAIL_BURSTS: pr_info("Received WIGIG_SENSING_IOCTL_GET_NUM_AVAIL_BURSTS command\n"); @@ -1145,6 +1156,22 @@ cmd_reply_buf_alloc_failed: return rc; } +static int wigig_sensing_send_event(struct wigig_sensing_ctx *ctx, + enum wigig_sensing_event event) +{ + if (kfifo_is_full(&ctx->events_fifo)) { + pr_err("events fifo is full, unable to send event\n"); + return -EFAULT; + } + + kfifo_in(&ctx->events_fifo, &event, 1); + ctx->event_pending = true; + + wake_up_interruptible(&ctx->cmd_wait_q); + + return 0; +} + static irqreturn_t wigig_sensing_dri_isr_thread(int irq, void *cookie) { struct wigig_sensing_ctx *ctx = cookie; @@ -1232,6 +1259,9 @@ static irqreturn_t wigig_sensing_dri_isr_thread(int irq, void *cookie) wigig_sensing_change_state(ctx, &ctx->stm, WIGIG_SENSING_STATE_READY_STOPPED); + /* Send asynchronous FW_READY event to application */ + wigig_sensing_send_event(ctx, WIGIG_SENSING_EVENT_FW_READY); + spi_status.v &= ~INT_FW_READY; } if (spi_status.b.int_data_ready) { @@ -1254,6 +1284,9 @@ static irqreturn_t wigig_sensing_dri_isr_thread(int irq, void *cookie) ctx->stm.state != WIGIG_SENSING_STATE_SYS_ASSERT) pr_err("State change to WIGIG_SENSING_SYS_ASSERT failed\n"); + /* Send asynchronous RESET event to application */ + wigig_sensing_send_event(ctx, WIGIG_SENSING_EVENT_RESET); + ctx->stm.spi_malfunction = true; spi_status.v &= ~INT_SYSASSERT; } @@ -1326,6 +1359,7 @@ static int wigig_sensing_probe(struct spi_device *spi) init_waitqueue_head(&ctx->cmd_wait_q); init_waitqueue_head(&ctx->data_wait_q); ctx->stm.state = WIGIG_SENSING_STATE_INITIALIZED; + INIT_KFIFO(ctx->events_fifo); /* Allocate memory for the CIRs */ /* Allocate a 2MB == 2^21 buffer for CIR data */ diff --git a/drivers/misc/wigig_sensing.h b/drivers/misc/wigig_sensing.h index c4b291062454..eaf2023ba25c 100644 --- a/drivers/misc/wigig_sensing.h +++ b/drivers/misc/wigig_sensing.h @@ -7,6 +7,7 @@ #define __WIGIG_SENSING_H__ #include #include +#include #include #include @@ -193,6 +194,7 @@ struct wigig_sensing_ctx { struct cir_data cir_data; u8 *temp_buffer; bool event_pending; + DECLARE_KFIFO(events_fifo, enum wigig_sensing_event, 8); u32 dropped_bursts; }; diff --git a/include/uapi/misc/wigig_sensing_uapi.h b/include/uapi/misc/wigig_sensing_uapi.h index c53d6bfa6010..d1c69645c024 100644 --- a/include/uapi/misc/wigig_sensing_uapi.h +++ b/include/uapi/misc/wigig_sensing_uapi.h @@ -27,8 +27,10 @@ struct wigig_sensing_change_mode { }; enum wigig_sensing_event { + WIGIG_SENSING_EVENT_MIN, WIGIG_SENSING_EVENT_FW_READY, WIGIG_SENSING_EVENT_RESET, + WIGIG_SENSING_EVENT_MAX, }; #define WIGIG_SENSING_IOC_MAGIC 'r' @@ -84,7 +86,7 @@ enum wigig_sensing_event { WIGIG_SENSING_IOCTL_GET_NUM_DROPPED_BURSTS, uint32_t) /** - * Get number of bursts that where dropped due to data buffer overflow + * Get asynchronous event (FW_READY, RESET) */ #define WIGIG_SENSING_IOC_GET_EVENT \ _IOR(WIGIG_SENSING_IOC_MAGIC, WIGIG_SENSING_IOCTL_GET_EVENT, \ From 813b74096e792c9265dc34fabf573102b173ae6d Mon Sep 17 00:00:00 2001 From: Gidon Studinski Date: Tue, 19 Nov 2019 12:34:14 +0200 Subject: [PATCH 3/8] wigig_sensing: return error code after change_mode failure change_mode command sometimes fail. In such a case, a proper error code should be returned to the calling user space application. Otherwise, the application tries to read data, which is not available. Change-Id: I03778cace54fd98847a099bbeaf7884cfbaac586 Signed-off-by: Gidon Studinski Signed-off-by: Alexei Avshalom Lazar --- drivers/misc/wigig_sensing.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/drivers/misc/wigig_sensing.c b/drivers/misc/wigig_sensing.c index e3758b116338..059676600ab2 100644 --- a/drivers/misc/wigig_sensing.c +++ b/drivers/misc/wigig_sensing.c @@ -546,7 +546,8 @@ static int wigig_sensing_ioc_change_mode(struct wigig_sensing_ctx *ctx, int rc; u32 ch; - pr_info("mode = %d, channel = %d\n", req.mode, req.channel); + pr_info("mode = %d, channel = %d, has_channel = %d\n", + req.mode, req.channel, req.has_channel); if (!ctx) return -EINVAL; @@ -583,12 +584,13 @@ static int wigig_sensing_ioc_change_mode(struct wigig_sensing_ctx *ctx, /* Interrupted by a signal */ pr_err("wait_event_interruptible_timeout() interrupted by a signal (%d)\n", rc); - return rc; + goto End; } if (rc == 0) { /* Timeout, FW did not respond in time */ pr_err("wait_event_interruptible_timeout() timed out\n"); - return -ETIME; + rc = -ETIME; + goto End; } /* Change internal state */ @@ -605,7 +607,8 @@ static int wigig_sensing_ioc_change_mode(struct wigig_sensing_ctx *ctx, ctx->stm.change_mode_in_progress = false; End: - return ctx->stm.burst_size; + ctx->stm.change_mode_in_progress = false; + return (rc == 0) ? ctx->stm.burst_size : rc; } static int wigig_sensing_ioc_clear_data(struct wigig_sensing_ctx *ctx) From bf6131b55c12e6caa3844de939729caac4e36a6b Mon Sep 17 00:00:00 2001 From: Gidon Studinski Date: Mon, 25 Nov 2019 14:16:29 +0200 Subject: [PATCH 4/8] wigig_sensing: make change_mode ioctl more robust Prevent state changes during DRI handling, which may cause the driver to get out of synchronization. Prevent data read from user space and from kernel space during change_mode processing. Change-Id: I2ee277c8894996119d13acbe87a5163e4064923c Signed-off-by: Gidon Studinski Signed-off-by: Alexei Avshalom Lazar --- drivers/misc/wigig_sensing.c | 57 ++++++++++++++++++++++++++---------- drivers/misc/wigig_sensing.h | 3 ++ 2 files changed, 44 insertions(+), 16 deletions(-) diff --git a/drivers/misc/wigig_sensing.c b/drivers/misc/wigig_sensing.c index 059676600ab2..42d6208ee048 100644 --- a/drivers/misc/wigig_sensing.c +++ b/drivers/misc/wigig_sensing.c @@ -542,7 +542,6 @@ static int wigig_sensing_ioc_change_mode(struct wigig_sensing_ctx *ctx, struct wigig_sensing_change_mode req) { struct wigig_sensing_stm sim_state; - enum wigig_sensing_stm_e new_state; int rc; u32 ch; @@ -551,11 +550,15 @@ static int wigig_sensing_ioc_change_mode(struct wigig_sensing_ctx *ctx, if (!ctx) return -EINVAL; + /* Save the request for later use */ + ctx->stm.mode_request = req.mode; + /* Simulate a state change */ - new_state = convert_mode_to_state(req.mode); + ctx->stm.state_request = convert_mode_to_state(req.mode); sim_state = ctx->stm; - rc = wigig_sensing_change_state(ctx, &sim_state, new_state); - if (rc || sim_state.state != new_state) { + rc = wigig_sensing_change_state(ctx, &sim_state, + ctx->stm.state_request); + if (rc || sim_state.state != ctx->stm.state_request) { pr_err("State change not allowed\n"); rc = -EFAULT; goto End; @@ -564,6 +567,7 @@ static int wigig_sensing_ioc_change_mode(struct wigig_sensing_ctx *ctx, /* Send command to FW */ ctx->stm.change_mode_in_progress = true; ch = req.has_channel ? req.channel : 0; + ctx->stm.channel_request = ch; ctx->stm.burst_size_ready = false; /* Change mode command must not be called during DRI processing */ mutex_lock(&ctx->dri_lock); @@ -593,21 +597,15 @@ static int wigig_sensing_ioc_change_mode(struct wigig_sensing_ctx *ctx, goto End; } - /* Change internal state */ - rc = wigig_sensing_change_state(ctx, &ctx->stm, new_state); - if (rc || ctx->stm.state != new_state) { + if (ctx->stm.state != ctx->stm.state_request) { pr_err("wigig_sensing_change_state() failed\n"); rc = -EFAULT; - goto End; } - ctx->dropped_bursts = 0; - ctx->stm.channel_request = ch; - ctx->stm.mode = req.mode; - ctx->stm.change_mode_in_progress = false; - End: - ctx->stm.change_mode_in_progress = false; + ctx->stm.state_request = WIGIG_SENSING_STATE_MIN; + ctx->stm.channel_request = 0; + ctx->stm.mode_request = WIGIG_SENSING_MODE_STOP; return (rc == 0) ? ctx->stm.burst_size : rc; } @@ -685,7 +683,8 @@ static unsigned int wigig_sensing_poll(struct file *filp, poll_table *wait) poll_wait(filp, &ctx->data_wait_q, wait); - if (circ_cnt(&ctx->cir_data.b, ctx->cir_data.size_bytes)) + if (!ctx->stm.change_mode_in_progress && + circ_cnt(&ctx->cir_data.b, ctx->cir_data.size_bytes)) mask |= (POLLIN | POLLRDNORM); if (ctx->event_pending) @@ -709,6 +708,9 @@ static ssize_t wigig_sensing_read(struct file *filp, char __user *buf, (!d->b.buf)) return -ENODEV; + if (ctx->stm.change_mode_in_progress) + return -EINVAL; + /* No data in the buffer */ while (circ_cnt(&d->b, d->size_bytes) == 0) { if (filp->f_flags & O_NONBLOCK) @@ -979,9 +981,25 @@ static int wigig_sensing_handle_fifo_ready_dri(struct wigig_sensing_ctx *ctx) ctx->temp_buffer = 0; } - wake_up_interruptible(&ctx->cmd_wait_q); + /* Change internal state */ + rc = wigig_sensing_change_state(ctx, &ctx->stm, ctx->stm.state_request); + if (rc || ctx->stm.state != ctx->stm.state_request) { + pr_err("wigig_sensing_change_state() failed\n"); + rc = -EFAULT; + goto End; + } + + /* Initialize head and tail pointers to 0 */ + wigig_sensing_ioc_clear_data(ctx); + + ctx->dropped_bursts = 0; + ctx->stm.channel = ctx->stm.channel_request; + ctx->stm.mode = ctx->stm.mode_request; + End: + ctx->stm.change_mode_in_progress = false; mutex_unlock(&ctx->spi_lock); + wake_up_interruptible(&ctx->cmd_wait_q); return rc; } @@ -997,6 +1015,7 @@ static int wigig_sensing_chip_data_ready(struct wigig_sensing_ctx *ctx, u32 idx = 0; u32 spi_transaction_size; u32 available_space_to_end; + u32 orig_head; if (stm_state == WIGIG_SENSING_STATE_INITIALIZED || stm_state == WIGIG_SENSING_STATE_SPI_READY || @@ -1045,8 +1064,14 @@ static int wigig_sensing_chip_data_ready(struct wigig_sensing_ctx *ctx, spi_transaction_size = calc_spi_transaction_size(fill_level, SPI_MAX_TRANSACTION_SIZE); local = d->b; + orig_head = local.head; mutex_lock(&ctx->spi_lock); while (fill_level > 0) { + if (ctx->stm.change_mode_in_progress) { + local.head = orig_head; + break; + } + bytes_to_read = (fill_level < spi_transaction_size) ? fill_level : spi_transaction_size; available_space_to_end = diff --git a/drivers/misc/wigig_sensing.h b/drivers/misc/wigig_sensing.h index eaf2023ba25c..87da91f85728 100644 --- a/drivers/misc/wigig_sensing.h +++ b/drivers/misc/wigig_sensing.h @@ -155,7 +155,10 @@ struct wigig_sensing_stm { enum wigig_sensing_stm_e state; enum wigig_sensing_mode mode; u32 burst_size; + u32 channel; u32 channel_request; + enum wigig_sensing_stm_e state_request; + enum wigig_sensing_mode mode_request; }; struct wigig_sensing_ctx { From 12db099039c1a8a1b9ddd8873677c1f5753aa0d8 Mon Sep 17 00:00:00 2001 From: Gidon Studinski Date: Mon, 25 Nov 2019 15:00:45 +0200 Subject: [PATCH 5/8] wigig_sensing: make sys-assert DRI priority higher In case sys-assert DRI is pending, there is no need to process other pending DRIs. Change-Id: I3f1422d75ae5d966c855213031b3524c46d7f5d5 Signed-off-by: Gidon Studinski Signed-off-by: Alexei Avshalom Lazar --- drivers/misc/wigig_sensing.c | 41 +++++++++++++++++++----------------- 1 file changed, 22 insertions(+), 19 deletions(-) diff --git a/drivers/misc/wigig_sensing.c b/drivers/misc/wigig_sensing.c index 42d6208ee048..a7751e0afef4 100644 --- a/drivers/misc/wigig_sensing.c +++ b/drivers/misc/wigig_sensing.c @@ -1276,8 +1276,26 @@ static irqreturn_t wigig_sensing_dri_isr_thread(int irq, void *cookie) goto bail_out; } + if (spi_status.b.int_sysassert) { + pr_info_ratelimited("SYSASSERT INTERRUPT\n"); + ctx->stm.fw_is_ready = false; + + rc = wigig_sensing_change_state(ctx, &ctx->stm, + WIGIG_SENSING_STATE_SYS_ASSERT); + if (rc != 0 || + ctx->stm.state != WIGIG_SENSING_STATE_SYS_ASSERT) + pr_err("State change to WIGIG_SENSING_SYS_ASSERT failed\n"); + + /* Send asynchronous RESET event to application */ + wigig_sensing_send_event(ctx, WIGIG_SENSING_EVENT_RESET); + + ctx->stm.spi_malfunction = true; + memset(&ctx->inb_cmd, 0, sizeof(ctx->inb_cmd)); + spi_status.v &= ~INT_SYSASSERT; + goto deassert_and_bail_out; + } if (spi_status.b.int_fw_ready) { - pr_debug("FW READY INTERRUPT\n"); + pr_info_ratelimited("FW READY INTERRUPT\n"); ctx->stm.fw_is_ready = true; ctx->stm.channel_request = 0; ctx->stm.burst_size = 0; @@ -1302,27 +1320,11 @@ static irqreturn_t wigig_sensing_dri_isr_thread(int irq, void *cookie) pr_debug("Change mode in progress, aborting data processing\n"); spi_status.v &= ~INT_DATA_READY; } - if (spi_status.b.int_sysassert) { - pr_debug("SYSASSERT INTERRUPT\n"); - ctx->stm.fw_is_ready = false; - - rc = wigig_sensing_change_state(ctx, &ctx->stm, - WIGIG_SENSING_STATE_SYS_ASSERT); - if (rc != 0 || - ctx->stm.state != WIGIG_SENSING_STATE_SYS_ASSERT) - pr_err("State change to WIGIG_SENSING_SYS_ASSERT failed\n"); - - /* Send asynchronous RESET event to application */ - wigig_sensing_send_event(ctx, WIGIG_SENSING_EVENT_RESET); - - ctx->stm.spi_malfunction = true; - spi_status.v &= ~INT_SYSASSERT; - } if (spi_status.b.int_deep_sleep_exit || (ctx->stm.waiting_for_deep_sleep_exit && ctx->stm.waiting_for_deep_sleep_exit_first_pass)) { if (spi_status.b.int_deep_sleep_exit) - pr_debug("DEEP SLEEP EXIT INTERRUPT\n"); + pr_info_ratelimited("DEEP SLEEP EXIT INTERRUPT\n"); if (ctx->stm.waiting_for_deep_sleep_exit) { additional_inb_command = ctx->inb_cmd; @@ -1335,7 +1337,7 @@ static irqreturn_t wigig_sensing_dri_isr_thread(int irq, void *cookie) spi_status.v &= ~INT_DEEP_SLEEP_EXIT; } if (spi_status.b.int_fifo_ready) { - pr_debug("FIFO READY INTERRUPT\n"); + pr_info_ratelimited("FIFO READY INTERRUPT\n"); wigig_sensing_handle_fifo_ready_dri(ctx); spi_status.v &= ~INT_FIFO_READY; @@ -1349,6 +1351,7 @@ static irqreturn_t wigig_sensing_dri_isr_thread(int irq, void *cookie) pr_err("Unexpected interrupt received, spi_status=0x%X\n", spi_status.v & CLEAR_LOW_23_BITS); +deassert_and_bail_out: /* Notify FW we are done with interrupt handling */ rc = wigig_sensing_deassert_dri(ctx, additional_inb_command); if (rc) From 4868bfcaf76bf8101e473394e5cc9adf569e8455 Mon Sep 17 00:00:00 2001 From: Gidon Studinski Date: Tue, 19 Nov 2019 18:15:35 +0200 Subject: [PATCH 6/8] wigig_sensing: enforce data read in multiple of burst size In case user space application requests for data size which is not a multiple of the burst size and there is enough data in the drivers internal buffer, a partial burst may be read by the application, which may cause loss of burst boundary. This patch fixes the above. In case the application supplies a buffer which is smaller than the burst size, an error is returned to the application. Change-Id: I5233476d99937dd1a13ceaec625588414a1dc04a Signed-off-by: Gidon Studinski Signed-off-by: Alexei Avshalom Lazar --- drivers/misc/wigig_sensing.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/misc/wigig_sensing.c b/drivers/misc/wigig_sensing.c index a7751e0afef4..739747e0ff04 100644 --- a/drivers/misc/wigig_sensing.c +++ b/drivers/misc/wigig_sensing.c @@ -711,6 +711,12 @@ static ssize_t wigig_sensing_read(struct file *filp, char __user *buf, if (ctx->stm.change_mode_in_progress) return -EINVAL; + /* Read buffer too small */ + if (count < ctx->stm.burst_size) { + pr_err("Read buffer must be larger than burst size\n"); + return -EINVAL; + } + /* No data in the buffer */ while (circ_cnt(&d->b, d->size_bytes) == 0) { if (filp->f_flags & O_NONBLOCK) @@ -720,11 +726,11 @@ static ssize_t wigig_sensing_read(struct file *filp, char __user *buf, circ_cnt(&d->b, d->size_bytes) != 0)) return -ERESTARTSYS; } - if (mutex_lock_interruptible(&d->lock)) return -ERESTARTSYS; copy_size = min_t(u32, circ_cnt(&d->b, d->size_bytes), count); + copy_size -= copy_size % ctx->stm.burst_size; size_to_end = circ_cnt_to_end(&d->b, d->size_bytes); tail = d->b.tail; pr_debug("copy_size=%u, size_to_end=%u, head=%u, tail=%u\n", From 551d76c84c66db9002619f6f12438a2ad300514b Mon Sep 17 00:00:00 2001 From: Gidon Studinski Date: Wed, 20 Nov 2019 17:41:08 +0200 Subject: [PATCH 7/8] wigig_sensing: relax state machine restrictions Impose less restrictions on state changes, as required by the new system design. Change-Id: I9ea6d2fcf2bde4dd013cf7c494dcb5c0c4948fd7 Signed-off-by: Gidon Studinski Signed-off-by: Alexei Avshalom Lazar --- drivers/misc/wigig_sensing.c | 111 +++++++++++------------------------ drivers/misc/wigig_sensing.h | 3 - 2 files changed, 33 insertions(+), 81 deletions(-) diff --git a/drivers/misc/wigig_sensing.c b/drivers/misc/wigig_sensing.c index 739747e0ff04..d9ce462f3177 100644 --- a/drivers/misc/wigig_sensing.c +++ b/drivers/misc/wigig_sensing.c @@ -433,7 +433,7 @@ static int wigig_sensing_change_state(struct wigig_sensing_ctx *ctx, enum wigig_sensing_stm_e new_state) { enum wigig_sensing_stm_e curr_state; - bool transition_allowed = false; + bool transition_allowed = true; if (!state) { pr_err("state is NULL\n"); @@ -441,75 +441,39 @@ static int wigig_sensing_change_state(struct wigig_sensing_ctx *ctx, } if (new_state <= WIGIG_SENSING_STATE_MIN || new_state >= WIGIG_SENSING_STATE_MAX) { - pr_err("new_state is invalid\n"); + pr_err("new_state (%d) is invalid\n", new_state); return -EINVAL; } curr_state = state->state; - if (new_state == curr_state) { - pr_debug("Already in the requested state, bailing out\n"); - return 0; - } - if ((new_state == WIGIG_SENSING_STATE_SYS_ASSERT && - !state->fw_is_ready) || - (new_state == WIGIG_SENSING_STATE_SPI_READY)) { + /* Moving to SYS_ASSEERT state is always allowed */ + if (new_state == WIGIG_SENSING_STATE_SYS_ASSERT) transition_allowed = true; - } else { - switch (curr_state) { - case WIGIG_SENSING_STATE_INITIALIZED: - if (new_state == WIGIG_SENSING_STATE_SPI_READY && - state->fw_is_ready) - transition_allowed = true; - break; - case WIGIG_SENSING_STATE_SPI_READY: - if (new_state == WIGIG_SENSING_STATE_READY_STOPPED && - state->enabled) - transition_allowed = true; - break; - case WIGIG_SENSING_STATE_READY_STOPPED: - if (new_state == WIGIG_SENSING_STATE_SEARCH || - new_state == WIGIG_SENSING_STATE_FACIAL || - new_state == WIGIG_SENSING_STATE_GESTURE || - new_state == WIGIG_SENSING_STATE_CUSTOM || - new_state == WIGIG_SENSING_STATE_GET_PARAMS) - transition_allowed = true; - break; - case WIGIG_SENSING_STATE_SEARCH: - if (new_state == WIGIG_SENSING_STATE_READY_STOPPED || - new_state == WIGIG_SENSING_STATE_SEARCH || - new_state == WIGIG_SENSING_STATE_FACIAL || - new_state == WIGIG_SENSING_STATE_GESTURE) - transition_allowed = true; - break; - case WIGIG_SENSING_STATE_FACIAL: - if (new_state == WIGIG_SENSING_STATE_READY_STOPPED || - new_state == WIGIG_SENSING_STATE_SEARCH) - transition_allowed = true; - break; - case WIGIG_SENSING_STATE_GESTURE: - if (new_state == WIGIG_SENSING_STATE_READY_STOPPED || - new_state == WIGIG_SENSING_STATE_SEARCH) - transition_allowed = true; - break; - case WIGIG_SENSING_STATE_CUSTOM: - if (new_state == WIGIG_SENSING_STATE_READY_STOPPED) - transition_allowed = true; - break; - case WIGIG_SENSING_STATE_GET_PARAMS: - if (new_state == WIGIG_SENSING_STATE_READY_STOPPED) - transition_allowed = true; - break; - case WIGIG_SENSING_STATE_SYS_ASSERT: - if (new_state == WIGIG_SENSING_STATE_READY_STOPPED && - state->fw_is_ready) - transition_allowed = true; - break; - default: - pr_err("new_state is invalid\n"); - return -EINVAL; - } - } + /* + * Moving from INITIALIZED state is allowed only to READY_STOPPED state + */ + else if (curr_state == WIGIG_SENSING_STATE_INITIALIZED && + new_state != WIGIG_SENSING_STATE_READY_STOPPED) + transition_allowed = false; + /* + * Moving to GET_PARAMS state is allowed only from READY_STOPPED state + */ + else if (curr_state != WIGIG_SENSING_STATE_READY_STOPPED && + new_state == WIGIG_SENSING_STATE_GET_PARAMS) + transition_allowed = false; + /* + * Moving from GET_PARAMS state is allowed only to READY_STOPPED state + */ + else if (curr_state == WIGIG_SENSING_STATE_GET_PARAMS && + new_state != WIGIG_SENSING_STATE_READY_STOPPED) + transition_allowed = false; + /* + * Moving from SYS_ASSERT state is allowed only to READY_STOPPED state + */ + else if (curr_state == WIGIG_SENSING_STATE_SYS_ASSERT && + new_state != WIGIG_SENSING_STATE_READY_STOPPED) + transition_allowed = false; if (transition_allowed) { pr_info("state transition (%d) --> (%d)\n", curr_state, @@ -944,17 +908,11 @@ static int wigig_sensing_handle_fifo_ready_dri(struct wigig_sensing_ctx *ctx) goto End; } - if (!ctx->stm.enabled && burst_size != 0) { - pr_info("Invalid burst size while disabled %d\n", burst_size); - rc = -EFAULT; - goto End; - } - ctx->stm.burst_size = burst_size; - if (!ctx->stm.enabled || - ctx->stm.state >= WIGIG_SENSING_STATE_SYS_ASSERT || - ctx->stm.state < WIGIG_SENSING_STATE_SPI_READY) { - pr_err("Received burst_size in an unexpected state\n"); + if (ctx->stm.state >= WIGIG_SENSING_STATE_SYS_ASSERT || + ctx->stm.state < WIGIG_SENSING_STATE_READY_STOPPED) { + pr_err("Received burst_size in an unexpected state (%d)\n", + ctx->stm.state); rc = -EFAULT; goto End; } @@ -1024,7 +982,6 @@ static int wigig_sensing_chip_data_ready(struct wigig_sensing_ctx *ctx, u32 orig_head; if (stm_state == WIGIG_SENSING_STATE_INITIALIZED || - stm_state == WIGIG_SENSING_STATE_SPI_READY || stm_state == WIGIG_SENSING_STATE_READY_STOPPED || stm_state == WIGIG_SENSING_STATE_SYS_ASSERT) { pr_err("Received data ready interrupt in an unexpected stm_state, disregarding\n"); @@ -1245,7 +1202,7 @@ static irqreturn_t wigig_sensing_dri_isr_thread(int irq, void *cookie) ctx->stm.spi_malfunction = false; if (ctx->stm.state == WIGIG_SENSING_STATE_INITIALIZED) wigig_sensing_change_state(ctx, &ctx->stm, - WIGIG_SENSING_STATE_SPI_READY); + WIGIG_SENSING_STATE_READY_STOPPED); } pr_debug("Reading SANITY register\n"); @@ -1306,8 +1263,6 @@ static irqreturn_t wigig_sensing_dri_isr_thread(int irq, void *cookie) ctx->stm.channel_request = 0; ctx->stm.burst_size = 0; ctx->stm.mode = WIGIG_SENSING_MODE_STOP; - ctx->stm.enabled = true; - wigig_sensing_change_state(ctx, &ctx->stm, WIGIG_SENSING_STATE_READY_STOPPED); diff --git a/drivers/misc/wigig_sensing.h b/drivers/misc/wigig_sensing.h index 87da91f85728..1389c3f77aac 100644 --- a/drivers/misc/wigig_sensing.h +++ b/drivers/misc/wigig_sensing.h @@ -131,7 +131,6 @@ struct spi_fifo { enum wigig_sensing_stm_e { WIGIG_SENSING_STATE_MIN = 0, WIGIG_SENSING_STATE_INITIALIZED, - WIGIG_SENSING_STATE_SPI_READY, WIGIG_SENSING_STATE_READY_STOPPED, WIGIG_SENSING_STATE_SEARCH, WIGIG_SENSING_STATE_FACIAL, @@ -144,10 +143,8 @@ enum wigig_sensing_stm_e { struct wigig_sensing_stm { bool auto_recovery; - bool enabled; bool fw_is_ready; bool spi_malfunction; - bool sys_assert; bool waiting_for_deep_sleep_exit; bool waiting_for_deep_sleep_exit_first_pass; bool burst_size_ready; From cec1c1c8d2c959fc3da91add825d867e7659696a Mon Sep 17 00:00:00 2001 From: Gidon Studinski Date: Wed, 27 Nov 2019 19:01:18 +0200 Subject: [PATCH 8/8] wigig_sensing: handle SYS_ASSERT corner cases Return -ENODEV in case a change_mode command is issued when the system is in SYS_ASSERT state. This guarantees that the user space application is aware of the bad state and waits for the FW_READY event. Change-Id: If26dc115078a3cd1af5d7efa93225019c1105184 Signed-off-by: Gidon Studinski Signed-off-by: Alexei Avshalom Lazar --- drivers/misc/wigig_sensing.c | 58 ++++++++++++++++++++++-------------- 1 file changed, 35 insertions(+), 23 deletions(-) diff --git a/drivers/misc/wigig_sensing.c b/drivers/misc/wigig_sensing.c index d9ce462f3177..c6f7e0cee7c2 100644 --- a/drivers/misc/wigig_sensing.c +++ b/drivers/misc/wigig_sensing.c @@ -434,6 +434,7 @@ static int wigig_sensing_change_state(struct wigig_sensing_ctx *ctx, { enum wigig_sensing_stm_e curr_state; bool transition_allowed = true; + int rc = 0; if (!state) { pr_err("state is NULL\n"); @@ -449,42 +450,52 @@ static int wigig_sensing_change_state(struct wigig_sensing_ctx *ctx, /* Moving to SYS_ASSEERT state is always allowed */ if (new_state == WIGIG_SENSING_STATE_SYS_ASSERT) - transition_allowed = true; + goto skip; + /* * Moving from INITIALIZED state is allowed only to READY_STOPPED state */ else if (curr_state == WIGIG_SENSING_STATE_INITIALIZED && - new_state != WIGIG_SENSING_STATE_READY_STOPPED) + new_state != WIGIG_SENSING_STATE_READY_STOPPED) { transition_allowed = false; + rc = -EFAULT; + } /* * Moving to GET_PARAMS state is allowed only from READY_STOPPED state */ else if (curr_state != WIGIG_SENSING_STATE_READY_STOPPED && - new_state == WIGIG_SENSING_STATE_GET_PARAMS) + new_state == WIGIG_SENSING_STATE_GET_PARAMS) { transition_allowed = false; + rc = -EFAULT; + } /* * Moving from GET_PARAMS state is allowed only to READY_STOPPED state */ else if (curr_state == WIGIG_SENSING_STATE_GET_PARAMS && - new_state != WIGIG_SENSING_STATE_READY_STOPPED) + new_state != WIGIG_SENSING_STATE_READY_STOPPED) { transition_allowed = false; + rc = -EFAULT; + } /* * Moving from SYS_ASSERT state is allowed only to READY_STOPPED state */ else if (curr_state == WIGIG_SENSING_STATE_SYS_ASSERT && - new_state != WIGIG_SENSING_STATE_READY_STOPPED) + new_state != WIGIG_SENSING_STATE_READY_STOPPED) { transition_allowed = false; + rc = -ENODEV; + } +skip: if (transition_allowed) { pr_info("state transition (%d) --> (%d)\n", curr_state, new_state); state->state = new_state; } else { - pr_info("state transition rejected (%d) xx> (%d)\n", - curr_state, new_state); + pr_err("state transition rejected (%d) xx> (%d)\n", + curr_state, new_state); } - return 0; + return rc; } static int wigig_sensing_ioc_set_auto_recovery(struct wigig_sensing_ctx *ctx) @@ -522,9 +533,8 @@ static int wigig_sensing_ioc_change_mode(struct wigig_sensing_ctx *ctx, sim_state = ctx->stm; rc = wigig_sensing_change_state(ctx, &sim_state, ctx->stm.state_request); - if (rc || sim_state.state != ctx->stm.state_request) { + if (rc) { pr_err("State change not allowed\n"); - rc = -EFAULT; goto End; } @@ -562,8 +572,11 @@ static int wigig_sensing_ioc_change_mode(struct wigig_sensing_ctx *ctx, } if (ctx->stm.state != ctx->stm.state_request) { - pr_err("wigig_sensing_change_state() failed\n"); - rc = -EFAULT; + pr_err("%s() failed\n", __func__); + if (ctx->stm.state == WIGIG_SENSING_STATE_SYS_ASSERT) + rc = -ENODEV; + else + rc = -EFAULT; } End: @@ -667,9 +680,8 @@ static ssize_t wigig_sensing_read(struct file *filp, char __user *buf, struct cir_data *d = &ctx->cir_data; /* Driver not ready to send data */ - if ((!ctx) || - (!ctx->spi_dev) || - (!d->b.buf)) + if (!ctx || !ctx->spi_dev || !d->b.buf || + ctx->stm.state == WIGIG_SENSING_STATE_SYS_ASSERT) return -ENODEV; if (ctx->stm.change_mode_in_progress) @@ -947,9 +959,8 @@ static int wigig_sensing_handle_fifo_ready_dri(struct wigig_sensing_ctx *ctx) /* Change internal state */ rc = wigig_sensing_change_state(ctx, &ctx->stm, ctx->stm.state_request); - if (rc || ctx->stm.state != ctx->stm.state_request) { + if (rc) { pr_err("wigig_sensing_change_state() failed\n"); - rc = -EFAULT; goto End; } @@ -1240,17 +1251,18 @@ static irqreturn_t wigig_sensing_dri_isr_thread(int irq, void *cookie) } if (spi_status.b.int_sysassert) { + enum wigig_sensing_stm_e old_state = ctx->stm.state; + pr_info_ratelimited("SYSASSERT INTERRUPT\n"); ctx->stm.fw_is_ready = false; - rc = wigig_sensing_change_state(ctx, &ctx->stm, - WIGIG_SENSING_STATE_SYS_ASSERT); - if (rc != 0 || - ctx->stm.state != WIGIG_SENSING_STATE_SYS_ASSERT) - pr_err("State change to WIGIG_SENSING_SYS_ASSERT failed\n"); + wigig_sensing_change_state(ctx, &ctx->stm, + WIGIG_SENSING_STATE_SYS_ASSERT); /* Send asynchronous RESET event to application */ - wigig_sensing_send_event(ctx, WIGIG_SENSING_EVENT_RESET); + if (old_state != WIGIG_SENSING_STATE_READY_STOPPED) + wigig_sensing_send_event(ctx, + WIGIG_SENSING_EVENT_RESET); ctx->stm.spi_malfunction = true; memset(&ctx->inb_cmd, 0, sizeof(ctx->inb_cmd));