[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <1d26bcf4-ac25-4161-ab5e-2f823099cfb7@altera.com>
Date: Tue, 2 Sep 2025 15:38:58 +0530
From: Mahesh Rao <mahesh.rao@...era.com>
To: Dinh Nguyen <dinguyen@...nel.og>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
<conor+dt@...nel.org>, Dinh Nguyen <dinguyen@...nel.org>
Cc: devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
Matthew Gerlach <matthew.gerlach@...era.com>
Subject: Re: [PATCH RESEND v2 4/4] firmware: stratix10-svc: Add for SDM
mailbox doorbell interrupt
Hi Dinh,
On 01-09-2025 08:46 pm, Dinh Nguyen wrote:
>
>
> On 8/12/25 07:59, Mahesh Rao via B4 Relay wrote:
>> From: Mahesh Rao <mahesh.rao@...era.com>
>>
>> Add support for SDM (Secure Device Manager) mailbox
>> doorbell interrupt for async transactions. On interrupt,
>> a workqueue is triggered which polls the ATF for
>> pending responses and retrieves the bitmap of all
>> retrieved and unprocessed transaction ids of mailbox
>> responses from SDM. It then triggers the corresponding
>> registered callbacks.
>
> You should configure your editor to use a full 80-char width. Why stop
> at ~50? When you're unsure, look at that other commit logs from other
> developers. If yours doesn't look similar, its probably a problem. For
> example:
>
> "Add support for SDM (Secure Device Manager) mailbox doorbell interrupt
> for async transactions. On interrupt, a workqueue is triggered which
> polls the ATF for pending responses and retrieves the bitmap of all
> retrieved and unprocessed transaction ids of mailbox responses from SDM.
> It then triggers the corresponding registered callbacks."
ok, will do.
>
>>
>> Signed-off-by: Mahesh Rao <mahesh.rao@...era.com>
>> Reviewed-by: Matthew Gerlach <matthew.gerlach@...era.com>
>> ---
>> drivers/firmware/stratix10-svc.c | 117 +++++++++++++++++
>> +++++++---
>> include/linux/firmware/intel/stratix10-smc.h | 23 ++++++
>> 2 files changed, 130 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/firmware/stratix10-svc.c b/drivers/firmware/
>> stratix10-svc.c
>> index
>> + struct work_struct async_work;
>> DECLARE_HASHTABLE(trx_list, ASYNC_TRX_HASH_BITS);
>> };
>> @@ -1709,14 +1716,81 @@ static inline void stratix10_smc_1_2(struct
>> stratix10_async_ctrl *actrl,
>> arm_smccc_1_2_smc(args, res);
>> }
>> +static irqreturn_t stratix10_svc_async_irq_handler(int irq, void
>> *dev_id)
>> +{
>> + struct stratix10_svc_controller *ctrl = dev_id;
>> + struct stratix10_async_ctrl *actrl = &ctrl->actrl;
>> +
>> + queue_work(system_bh_wq, &actrl->async_work);
>> + disable_irq_nosync(actrl->irq);
>> + return IRQ_HANDLED;
>> +}
>
> Add a newline here.
ok, will make this change.
>
>> +/**
>> + * stratix10_async_workqueue_handler - Handler for the asynchronous
>> + * workqueue in Stratix10 service controller.
>> + * @work: Pointer to the work structure that contains the asynchronous
>> + * workqueue handler.
>> + * This function is the handler for the asynchronous workqueue. It
>> performs
>> + * the following tasks:
>> + * - Invokes the asynchronous polling on interrupt supervisory call.
>> + * - On success,it retrieves the bitmap of pending transactions from
>> mailbox
>> + * fifo in ATF.
>> + * - It processes each pending transaction by calling the corresponding
>> + * callback function.
>> + *
>> + * The function ensures that the IRQ is enabled after processing the
>> transactions
>> + * and logs the total time taken to handle the transactions along
>> with the number
>> + * of transactions handled and the CPU on which the handler ran.
>> - int ret;
>> + int ret, irq;
>> if (!controller)
>> return -EINVAL;
>> @@ -1775,6 +1849,22 @@ static int stratix10_svc_async_init(struct
>> stratix10_svc_controller *controller)
>> hash_init(actrl->trx_list);
>> atomic_set(&actrl->common_achan_refcount, 0);
>> + irq = of_irq_get(dev_of_node(dev), 0);
>> + if (irq < 0) {
>
> 0 is a failing value as well.
will add this.
>
>> + dev_warn(dev, "Failed to get IRQ, falling back to polling
>> mode\n");
>> + } else {
>> - * the asynchronous ID pool and invoke function pointers to NULL.
>> + * This function performs the necessary cleanup for the asynchronous
>> service
>> + * controller. It checks if the controller is valid and if it has been
>> + * initialized. Also If the controller has an IRQ assigned, it frees
>> the IRQ
>> + * and flushes any pending asynchronous work. It then locks the
>> transaction
>> + * list and safely removes and deallocates each handler in the list.
>> + * The function also removes any asynchronous clients associated with
>> the
>> + * controller's channels and destroys the asynchronous ID pool.
>> Finally, it
>> + * resets the asynchronous ID pool and invoke function pointers to NULL.
>
> Did you mean to repeat the same paragraph twice?
>
> Dinh
Powered by blists - more mailing lists