From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 6915254652 for ; Tue, 3 Mar 2026 04:21:27 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1772511687; cv=none; b=MO41DN5OY81jRk2nIRMhGvbm8/O5pRbj8SkCZAdvJv6ZxVVIr5lJQYfbIkgDS013ahmenDWpTvL62Z/wHXyftatwn4ua079pYuYEcslUoqFSqxIZNAIhI5yMfmMWGLap78483NDPdae3iFxETGuvltEnXhlOJIPR7rfOPCAeOSo= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1772511687; c=relaxed/simple; bh=k2kyhmOSZmYFxaLtCDm1h58U0OJK2hGAmS15jMlPVlM=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=cAC75fb0aj+YuGLEiM8rBvdwxmFj1gxmYvksB1viVzbTkDaI+dDsUzQ+VmoeQ6RySy+xW3zbqVTayZPozH6keO6FPaadvkPRuujdl3Ay4mqZPOIkRXNWIEM46ZWm4Yd97m7pJnsWj0AJyF8w5sF3gxJ227VWRxn6bib/q8Zslt0= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=KF9/yKXn; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="KF9/yKXn" Received: by smtp.kernel.org (Postfix) with ESMTPSA id A4410C116C6; Tue, 3 Mar 2026 04:21:26 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1772511687; bh=k2kyhmOSZmYFxaLtCDm1h58U0OJK2hGAmS15jMlPVlM=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=KF9/yKXn+fUOXv7vgeXKiNAJzmuGW4fRjii4z8heyeBEYxwkXclyzj7fDNbS4bbd0 qpinDQnSGvFqtn4WdHJ7Nf4Guif+UUbggy0bb6YzCK1d2j5BLZSxj3tSYIBPOiAs9R dC5qkryruPXCdLMg2d/KgCGfZnFqlh1ZWI30Ro8rJMxsW/2gnkglP22RMGdFlT0Etw nYkTQ+j0vaeMFHuSHPsjK+y6mrUN+MCqJyjI4PWZ1CBzlEpbRZB2H+nK2RLSg7PImb hzURFy07WYo9aVGgzCtVKNr6lwrCEACLxYrRNL61aIOYPdaW5M3glaWh3CZNtEv4UT ldsI20PpvSHGg== Message-ID: <6f4588b7-22fe-4f92-9449-aae82767d111@kernel.org> Date: Mon, 2 Mar 2026 22:21:25 -0600 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v5] firmware: stratix10-svc: Add Multi SVC clients support Content-Language: en-US To: Muhammad Amirul Asyraf Mohamad Jamian , linux-kernel@vger.kernel.org, Ang Tien Sung Cc: Markus Elfring References: <20260302053944.14300-1-muhammad.amirul.asyraf.mohamad.jamian@altera.com> From: Dinh Nguyen In-Reply-To: <20260302053944.14300-1-muhammad.amirul.asyraf.mohamad.jamian@altera.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 3/1/26 23:39, Muhammad Amirul Asyraf Mohamad Jamian wrote: > In the current implementation, SVC client drivers such as socfpga-hwmon, > intel_fcs, stratix10-soc, stratix10-rsu each send an SMC command that > triggers a single thread in the stratix10-svc driver. Upon receiving a > callback, the initiating client driver sends a stratix10-svc-done signal, > terminating the thread without waiting for other pending SMC commands to > complete. This leads to a timeout issue in the firmware SVC mailbox service > when multiple client drivers send SMC commands concurrently. > > To resolve this issue, a dedicated thread is now created per channel. The > stratix10-svc driver will support up to the number of channels defined by > SVC_NUM_CHANNEL. Thread synchronization is handled using a mutex to prevent > simultaneous issuance of SMC commands by multiple threads. > > Additionally, a thread task is now validated before invoking kthread_stop > when the user aborts, ensuring safe termination. > > Timeout values have also been adjusted to accommodate the increased load > from concurrent client driver activity. > > Fixes: 7ca5ce896524 ("firmware: add Intel Stratix10 service layer driver") > Cc: stable@vger.kernel.org > Signed-off-by: Ang Tien Sung > Signed-off-by: Fong, Yan Kei > Signed-off-by: Muhammad Amirul Asyraf Mohamad Jamian > Reported-by: kernel test robot > Closes: https://lore.kernel.org/oe-kbuild-all/202602032325.MuglZ156-lkp@intel.com/ > --- > Changes in v5: > - Fix mutex locking: replace guard(mutex) with explicit lock/unlock, > use mutex_lock_interruptible() so kthread_stop() can unblock the > thread when the SDM lock is contended, fix lock/unlock imbalances, > and on interrupted lock notify client with SVC_STATUS_ERROR so it > does not block on a response that never arrives > - Fix TOCTOU race in stratix10_svc_send(): protect chan->task creation > with chan->lock spinlock to prevent concurrent callers on the same > channel from spawning duplicate threads > - Fix probe error path: initialize controller->node with INIT_LIST_HEAD > and guard list_del() so it is only called when list_add_tail() was > reached, preventing NULL deref on early FIFO allocation failure > - Remove stray platform_device_unregister(svc->intel_svc_fcs) call in > remove function, as FCS device is not manually registered in probe; > also remove the now-unused INTEL_FCS macro, stale intel_svc_fcs field > from struct stratix10_svc, and stale @task kdoc from > struct stratix10_svc_controller > - Remove unnecessary NULL check on svc pointer in err_put_device > - Clean up: convert FIFO allocations to for loop, simplify > stratix10_svc_done() to a single debug message, fix whitespace, > typos and log messages > > Changes in v4: > - Signed-off-by chain clean up > - Remove duplicate inline comment on sdm_lock field > - Move structure field comments to structure header > - Remove unnecessary checks in stratix10_svc_done() > - Fix error handling to properly free FIFOs on probe failure > - Reorder error labels for proper cleanup cascade > - Remove redundant fifo_size initialization > > Changes in v3: > - Fix grammar: "will supports" -> "will support" > - Add Fixes tag and Cc stable as suggested by reviewer > > Changes in v2: > - Initialize 'svc' to NULL and add NULL check in error path to fix uninitialized variable warning > --- > --- > drivers/firmware/stratix10-svc.c | 225 ++++++++++-------- > .../firmware/intel/stratix10-svc-client.h | 8 +- > 2 files changed, 128 insertions(+), 105 deletions(-) > > diff --git a/drivers/firmware/stratix10-svc.c b/drivers/firmware/stratix10-svc.c > index 6f5c298582ab..2af95018f06d 100644 > --- a/drivers/firmware/stratix10-svc.c > +++ b/drivers/firmware/stratix10-svc.c > @@ -37,15 +37,14 @@ > * service layer will return error to FPGA manager when timeout occurs, > * timeout is set to 30 seconds (30 * 1000) at Intel Stratix10 SoC. > */ > -#define SVC_NUM_DATA_IN_FIFO 32 > +#define SVC_NUM_DATA_IN_FIFO 8 Your commit message does not explain this change. I don't see how this change is related to this patch. > #define SVC_NUM_CHANNEL 4 > -#define FPGA_CONFIG_DATA_CLAIM_TIMEOUT_MS 200 > +#define FPGA_CONFIG_DATA_CLAIM_TIMEOUT_MS 2000 > #define FPGA_CONFIG_STATUS_TIMEOUT_SEC 30 > #define BYTE_TO_WORD_SIZE 4 > > /* stratix10 service layer clients */ > #define STRATIX10_RSU "stratix10-rsu" > -#define INTEL_FCS "intel-fcs" > > /* Maximum number of SDM client IDs. */ > #define MAX_SDM_CLIENT_IDS 16 > @@ -105,11 +104,9 @@ struct stratix10_svc_chan; > /** > * struct stratix10_svc - svc private data > * @stratix10_svc_rsu: pointer to stratix10 RSU device > - * @intel_svc_fcs: pointer to the FCS device > */ > struct stratix10_svc { > struct platform_device *stratix10_svc_rsu; > - struct platform_device *intel_svc_fcs; > }; > > /** > @@ -251,12 +248,10 @@ struct stratix10_async_ctrl { > * @num_active_client: number of active service client > * @node: list management > * @genpool: memory pool pointing to the memory region > - * @task: pointer to the thread task which handles SMC or HVC call > - * @svc_fifo: a queue for storing service message data > * @complete_status: state for completion > - * @svc_fifo_lock: protect access to service message data queue > * @invoke_fn: function to issue secure monitor call or hypervisor call > * @svc: manages the list of client svc drivers > + * @sdm_lock: only allows a single command single response to SDM > * @actrl: async control structure > * > * This struct is used to create communication channels for service clients, to > @@ -269,12 +264,10 @@ struct stratix10_svc_controller { > int num_active_client; > struct list_head node; > struct gen_pool *genpool; > - struct task_struct *task; > - struct kfifo svc_fifo; > struct completion complete_status; > - spinlock_t svc_fifo_lock; > svc_invoke_fn *invoke_fn; > struct stratix10_svc *svc; > + struct mutex *sdm_lock; > struct stratix10_async_ctrl actrl; > }; > > @@ -283,6 +276,9 @@ struct stratix10_svc_controller { > * @ctrl: pointer to service controller which is the provider of this channel > * @scl: pointer to service client which owns the channel > * @name: service client name associated with the channel > + * @task: pointer to the thread task which handles SMC or HVC call > + * @svc_fifo: a queue for storing service message data (separate fifo for every channel) > + * @svc_fifo_lock: protect access to service message data queue (locking pending fifo) > * @lock: protect access to the channel > * @async_chan: reference to asynchronous channel object for this channel > * > @@ -293,6 +289,9 @@ struct stratix10_svc_chan { > struct stratix10_svc_controller *ctrl; > struct stratix10_svc_client *scl; > char *name; > + struct task_struct *task; > + struct kfifo svc_fifo; > + spinlock_t svc_fifo_lock; > spinlock_t lock; > struct stratix10_async_chan *async_chan; > }; > @@ -527,10 +526,10 @@ static void svc_thread_recv_status_ok(struct stratix10_svc_data *p_data, > */ > static int svc_normal_to_secure_thread(void *data) > { > - struct stratix10_svc_controller > - *ctrl = (struct stratix10_svc_controller *)data; > - struct stratix10_svc_data *pdata; > - struct stratix10_svc_cb_data *cbdata; > + struct stratix10_svc_chan *chan = (struct stratix10_svc_chan *)data; > + struct stratix10_svc_controller *ctrl = chan->ctrl; > + struct stratix10_svc_data *pdata = NULL; > + struct stratix10_svc_cb_data *cbdata = NULL; > struct arm_smccc_res res; > unsigned long a0, a1, a2, a3, a4, a5, a6, a7; > int ret_fifo = 0; > @@ -555,12 +554,12 @@ static int svc_normal_to_secure_thread(void *data) > a6 = 0; > a7 = 0; > > - pr_debug("smc_hvc_shm_thread is running\n"); > + pr_debug("%s: %s: Thread is running!\n", __func__, chan->name); > > while (!kthread_should_stop()) { > - ret_fifo = kfifo_out_spinlocked(&ctrl->svc_fifo, > + ret_fifo = kfifo_out_spinlocked(&chan->svc_fifo, > pdata, sizeof(*pdata), > - &ctrl->svc_fifo_lock); > + &chan->svc_fifo_lock); > > if (!ret_fifo) > continue; > @@ -569,9 +568,25 @@ static int svc_normal_to_secure_thread(void *data) > (unsigned int)pdata->paddr, pdata->command, > (unsigned int)pdata->size); > > + /* SDM can only process one command at a time */ > + pr_debug("%s: %s: Thread is waiting for mutex!\n", > + __func__, chan->name); > + if (mutex_lock_interruptible(ctrl->sdm_lock)) { > + /* item already dequeued; notify client to unblock it */ > + cbdata->status = BIT(SVC_STATUS_ERROR); > + cbdata->kaddr1 = NULL; > + cbdata->kaddr2 = NULL; > + cbdata->kaddr3 = NULL; > + if (pdata->chan->scl) > + pdata->chan->scl->receive_cb(pdata->chan->scl, > + cbdata); > + break; > + } > + > switch (pdata->command) { > case COMMAND_RECONFIG_DATA_CLAIM: > svc_thread_cmd_data_claim(ctrl, pdata, cbdata); > + mutex_unlock(ctrl->sdm_lock); > continue; > case COMMAND_RECONFIG: > a0 = INTEL_SIP_SMC_FPGA_CONFIG_START; > @@ -700,10 +715,11 @@ static int svc_normal_to_secure_thread(void *data) > break; > default: > pr_warn("it shouldn't happen\n"); > - break; > + mutex_unlock(ctrl->sdm_lock); > + continue; > } > - pr_debug("%s: before SMC call -- a0=0x%016x a1=0x%016x", > - __func__, > + pr_debug("%s: %s: before SMC call -- a0=0x%016x a1=0x%016x", > + __func__, chan->name, > (unsigned int)a0, > (unsigned int)a1); > pr_debug(" a2=0x%016x\n", (unsigned int)a2); > @@ -712,8 +728,8 @@ static int svc_normal_to_secure_thread(void *data) > pr_debug(" a5=0x%016x\n", (unsigned int)a5); > ctrl->invoke_fn(a0, a1, a2, a3, a4, a5, a6, a7, &res); > > - pr_debug("%s: after SMC call -- res.a0=0x%016x", > - __func__, (unsigned int)res.a0); > + pr_debug("%s: %s: after SMC call -- res.a0=0x%016x", > + __func__, chan->name, (unsigned int)res.a0); > pr_debug(" res.a1=0x%016x, res.a2=0x%016x", > (unsigned int)res.a1, (unsigned int)res.a2); > pr_debug(" res.a3=0x%016x\n", (unsigned int)res.a3); > @@ -728,6 +744,7 @@ static int svc_normal_to_secure_thread(void *data) > cbdata->kaddr2 = NULL; > cbdata->kaddr3 = NULL; > pdata->chan->scl->receive_cb(pdata->chan->scl, cbdata); > + mutex_unlock(ctrl->sdm_lock); > continue; > } > > @@ -801,6 +818,8 @@ static int svc_normal_to_secure_thread(void *data) > break; > > } > + > + mutex_unlock(ctrl->sdm_lock); > } > > kfree(cbdata); > @@ -1697,21 +1716,25 @@ int stratix10_svc_send(struct stratix10_svc_chan *chan, void *msg) > return -ENOMEM; > > /* first client will create kernel thread */ > - if (!chan->ctrl->task) { > - chan->ctrl->task = > + spin_lock(&chan->lock); > + if (!chan->task) { > + chan->task = > kthread_run_on_cpu(svc_normal_to_secure_thread, > - (void *)chan->ctrl, > + (void *)chan, > cpu, "svc_smc_hvc_thread"); > - if (IS_ERR(chan->ctrl->task)) { > + if (IS_ERR(chan->task)) { > + chan->task = NULL; > + spin_unlock(&chan->lock); > dev_err(chan->ctrl->dev, > "failed to create svc_smc_hvc_thread\n"); > kfree(p_data); > return -EINVAL; > } > } > + spin_unlock(&chan->lock); > > - pr_debug("%s: sent P-va=%p, P-com=%x, P-size=%u\n", __func__, > - p_msg->payload, p_msg->command, > + pr_debug("%s: %s: sent P-va=%p, P-com=%x, P-size=%u\n", __func__, > + chan->name, p_msg->payload, p_msg->command, > (unsigned int)p_msg->payload_length); > > if (list_empty(&svc_data_mem)) { > @@ -1747,12 +1770,16 @@ int stratix10_svc_send(struct stratix10_svc_chan *chan, void *msg) > p_data->arg[2] = p_msg->arg[2]; > p_data->size = p_msg->payload_length; > p_data->chan = chan; > - pr_debug("%s: put to FIFO pa=0x%016x, cmd=%x, size=%u\n", __func__, > - (unsigned int)p_data->paddr, p_data->command, > - (unsigned int)p_data->size); > - ret = kfifo_in_spinlocked(&chan->ctrl->svc_fifo, p_data, > + pr_debug("%s: %s: put to FIFO pa=0x%016x, cmd=%x, size=%u\n", > + __func__, > + chan->name, > + (unsigned int)p_data->paddr, > + p_data->command, > + (unsigned int)p_data->size); > + > + ret = kfifo_in_spinlocked(&chan->svc_fifo, p_data, > sizeof(*p_data), > - &chan->ctrl->svc_fifo_lock); > + &chan->svc_fifo_lock); > > kfree(p_data); > > @@ -1773,11 +1800,12 @@ EXPORT_SYMBOL_GPL(stratix10_svc_send); > */ > void stratix10_svc_done(struct stratix10_svc_chan *chan) > { > - /* stop thread when thread is running AND only one active client */ > - if (chan->ctrl->task && chan->ctrl->num_active_client <= 1) { > - pr_debug("svc_smc_hvc_shm_thread is stopped\n"); > - kthread_stop(chan->ctrl->task); > - chan->ctrl->task = NULL; > + /* stop thread when thread is running */ > + if (chan->task) { > + pr_debug("%s: %s: svc_smc_hvc_shm_thread is stopping\n", > + __func__, chan->name); > + kthread_stop(chan->task); > + chan->task = NULL; > } > } > EXPORT_SYMBOL_GPL(stratix10_svc_done); > @@ -1817,8 +1845,8 @@ void *stratix10_svc_allocate_memory(struct stratix10_svc_chan *chan, > pmem->paddr = pa; > pmem->size = s; > list_add_tail(&pmem->node, &svc_data_mem); > - pr_debug("%s: va=%p, pa=0x%016x\n", __func__, > - pmem->vaddr, (unsigned int)pmem->paddr); > + pr_debug("%s: %s: va=%p, pa=0x%016x\n", __func__, > + chan->name, pmem->vaddr, (unsigned int)pmem->paddr); > > return (void *)va; > } > @@ -1855,6 +1883,18 @@ static const struct of_device_id stratix10_svc_drv_match[] = { > {}, > }; > > +/** > + * mailbox_lock protects access to the SDM if thread is busy > + */ > +static DEFINE_MUTEX(mailbox_lock); Should you use a global mutex? I'd consider using a local mutex that you already have defined in the stratix10_svc_controller->sdm_lock. > + > +static const char * const chan_names[SVC_NUM_CHANNEL] = { > + SVC_CLIENT_FPGA, > + SVC_CLIENT_RSU, > + SVC_CLIENT_FCS, > + SVC_CLIENT_HWMON > +}; > + > static int stratix10_svc_drv_probe(struct platform_device *pdev) > { > struct device *dev = &pdev->dev; > @@ -1862,11 +1902,11 @@ static int stratix10_svc_drv_probe(struct platform_device *pdev) > struct stratix10_svc_chan *chans; > struct gen_pool *genpool; > struct stratix10_svc_sh_memory *sh_memory; > - struct stratix10_svc *svc; > + struct stratix10_svc *svc = NULL; > > svc_invoke_fn *invoke_fn; > size_t fifo_size; > - int ret; > + int ret, i = 0; > > /* get SMC or HVC function */ > invoke_fn = get_invoke_func(dev); > @@ -1905,8 +1945,8 @@ static int stratix10_svc_drv_probe(struct platform_device *pdev) > controller->num_active_client = 0; > controller->chans = chans; > controller->genpool = genpool; > - controller->task = NULL; > controller->invoke_fn = invoke_fn; > + INIT_LIST_HEAD(&controller->node); > init_completion(&controller->complete_status); > > ret = stratix10_svc_async_init(controller); > @@ -1917,32 +1957,24 @@ static int stratix10_svc_drv_probe(struct platform_device *pdev) > } > > fifo_size = sizeof(struct stratix10_svc_data) * SVC_NUM_DATA_IN_FIFO; > - ret = kfifo_alloc(&controller->svc_fifo, fifo_size, GFP_KERNEL); > - if (ret) { > - dev_err(dev, "failed to allocate FIFO\n"); > - goto err_async_exit; > - } > - spin_lock_init(&controller->svc_fifo_lock); > - > - chans[0].scl = NULL; > - chans[0].ctrl = controller; > - chans[0].name = SVC_CLIENT_FPGA; > - spin_lock_init(&chans[0].lock); > - > - chans[1].scl = NULL; > - chans[1].ctrl = controller; > - chans[1].name = SVC_CLIENT_RSU; > - spin_lock_init(&chans[1].lock); > - > - chans[2].scl = NULL; > - chans[2].ctrl = controller; > - chans[2].name = SVC_CLIENT_FCS; > - spin_lock_init(&chans[2].lock); > + /* > + * This mutex is used to block threads from utilizing > + * SDM to prevent out of order command tx > + */ > + controller->sdm_lock = &mailbox_lock; > > - chans[3].scl = NULL; > - chans[3].ctrl = controller; > - chans[3].name = SVC_CLIENT_HWMON; > - spin_lock_init(&chans[3].lock); > + for (i = 0; i < SVC_NUM_CHANNEL; i++) { > + chans[i].scl = NULL; > + chans[i].ctrl = controller; > + chans[i].name = (char *)chan_names[i]; > + spin_lock_init(&chans[i].lock); > + ret = kfifo_alloc(&chans[i].svc_fifo, fifo_size, GFP_KERNEL); > + if (ret) { > + dev_err(dev, "failed to allocate FIFO %d\n", i); > + goto err_free_fifos; > + } > + spin_lock_init(&chans[i].svc_fifo_lock); > + } > > list_add_tail(&controller->node, &svc_ctrl); > platform_set_drvdata(pdev, controller); > @@ -1951,7 +1983,7 @@ static int stratix10_svc_drv_probe(struct platform_device *pdev) > svc = devm_kzalloc(dev, sizeof(*svc), GFP_KERNEL); > if (!svc) { > ret = -ENOMEM; > - goto err_free_kfifo; > + goto err_free_fifos; > } > controller->svc = svc; > > @@ -1959,51 +1991,40 @@ static int stratix10_svc_drv_probe(struct platform_device *pdev) > if (!svc->stratix10_svc_rsu) { > dev_err(dev, "failed to allocate %s device\n", STRATIX10_RSU); > ret = -ENOMEM; > - goto err_free_kfifo; > + goto err_free_fifos; > } > > ret = platform_device_add(svc->stratix10_svc_rsu); > - if (ret) { > - platform_device_put(svc->stratix10_svc_rsu); > - goto err_free_kfifo; > - } > - > - svc->intel_svc_fcs = platform_device_alloc(INTEL_FCS, 1); > - if (!svc->intel_svc_fcs) { > - dev_err(dev, "failed to allocate %s device\n", INTEL_FCS); > - ret = -ENOMEM; > - goto err_unregister_rsu_dev; > - } > - > - ret = platform_device_add(svc->intel_svc_fcs); > - if (ret) { > - platform_device_put(svc->intel_svc_fcs); > - goto err_unregister_rsu_dev; > - } > + if (ret) > + goto err_put_device; > > ret = of_platform_default_populate(dev_of_node(dev), NULL, dev); > if (ret) > - goto err_unregister_fcs_dev; > + goto err_put_device; You need a new error path here because platform_device_add() has succeeded, but your error path is only calling platform_device_put() which only decrements the refcount, but does not unregister. You need an error path here that will call platform_device_unregister(). > > pr_info("Intel Service Layer Driver Initialized\n"); > > return 0; > > -err_unregister_fcs_dev: > - platform_device_unregister(svc->intel_svc_fcs); > -err_unregister_rsu_dev: > - platform_device_unregister(svc->stratix10_svc_rsu); > -err_free_kfifo: > - kfifo_free(&controller->svc_fifo); > -err_async_exit: > +err_put_device: > + platform_device_put(svc->stratix10_svc_rsu); > +err_free_fifos: > + /* only remove from list if list_add_tail() was reached */ > + if (!list_empty(&controller->node)) > + list_del(&controller->node); > + /* free only the FIFOs that were successfully allocated */ > + while (i--) > + kfifo_free(&chans[i].svc_fifo); > stratix10_svc_async_exit(controller); > err_destroy_pool: > gen_pool_destroy(genpool); > + > return ret; > } > > static void stratix10_svc_drv_remove(struct platform_device *pdev) > { > + int i; > struct stratix10_svc_controller *ctrl = platform_get_drvdata(pdev); > struct stratix10_svc *svc = ctrl->svc; > > @@ -2011,14 +2032,16 @@ static void stratix10_svc_drv_remove(struct platform_device *pdev) > > of_platform_depopulate(ctrl->dev); > > - platform_device_unregister(svc->intel_svc_fcs); > platform_device_unregister(svc->stratix10_svc_rsu); > > - kfifo_free(&ctrl->svc_fifo); > - if (ctrl->task) { > - kthread_stop(ctrl->task); > - ctrl->task = NULL; > + for (i = 0; i < SVC_NUM_CHANNEL; i++) { > + if (ctrl->chans[i].task) { > + kthread_stop(ctrl->chans[i].task); > + ctrl->chans[i].task = NULL; > + } > + kfifo_free(&ctrl->chans[i].svc_fifo); > } > + > if (ctrl->genpool) > gen_pool_destroy(ctrl->genpool); > list_del(&ctrl->node); > diff --git a/include/linux/firmware/intel/stratix10-svc-client.h b/include/linux/firmware/intel/stratix10-svc-client.h > index d290060f4c73..91013161e9db 100644 > --- a/include/linux/firmware/intel/stratix10-svc-client.h > +++ b/include/linux/firmware/intel/stratix10-svc-client.h > @@ -68,12 +68,12 @@ > * timeout value used in Stratix10 FPGA manager driver. > * timeout value used in RSU driver > */ > -#define SVC_RECONFIG_REQUEST_TIMEOUT_MS 300 > -#define SVC_RECONFIG_BUFFER_TIMEOUT_MS 720 > -#define SVC_RSU_REQUEST_TIMEOUT_MS 300 > +#define SVC_RECONFIG_REQUEST_TIMEOUT_MS 5000 > +#define SVC_RECONFIG_BUFFER_TIMEOUT_MS 5000 > +#define SVC_RSU_REQUEST_TIMEOUT_MS 2000 > #define SVC_FCS_REQUEST_TIMEOUT_MS 2000 > #define SVC_COMPLETED_TIMEOUT_MS 30000 > -#define SVC_HWMON_REQUEST_TIMEOUT_MS 300 > +#define SVC_HWMON_REQUEST_TIMEOUT_MS 2000 > > struct stratix10_svc_chan; > Dinh