[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <c27930bd-b2b8-4131-95f7-ff8435080c02@altera.com>
Date: Fri, 22 Aug 2025 15:18:06 +0530
From: Mahesh Rao <mahesh.rao@...era.com>
To: Greg KH <gregkh@...uxfoundation.org>, Dinh Nguyen <dinguyen@...nel.org>
Cc: linux-kernel@...r.kernel.org, Matthew Gerlach <matthew.gerlach@...era.com>
Subject: Re: [PATCH 3/5] firmware: stratix10-svc: Add initial support for
asynchronous communication with Stratix10 service channel
Hi Greg,
On 19-08-2025 04:38 pm, Greg KH wrote:
> On Tue, Jul 22, 2025 at 11:30:43AM -0500, Dinh Nguyen wrote:
>> From: Mahesh Rao <mahesh.rao@...era.com>
>>
>> Introduce support for asynchronous communication
>> with the Stratix10 service channel. Define new
>> structures to enable asynchronous messaging with
>> the Secure Device Manager (SDM). Add and remove
>> asynchronous support for existing channels.
>> Implement initialization and cleanup routines for
>> the asynchronous framework. Enable sending and
>> polling of messages to the SDM asynchronously.
>>
>> The new public functions added are:
>> - stratix10_svc_add_async_client: Adds an client
>> to the service channel.
>> - stratix10_svc_remove_async_client: Removes an
>> asynchronous client from the service
>> channel.
>> - stratix10_svc_async_send: Sends an asynchronous
>> message to the SDM mailbox in EL3 secure
>> firmware.
>> - stratix10_svc_async_poll: Polls the status of an
>> asynchronous service request in EL3 secure
>> firmware.
>> - stratix10_svc_async_done: Marks an asynchronous
>> transaction as complete and free's up the
>> resources.
>>
>> These changes enhance the functionality of the
>> Stratix10 service channel by allowing for more
>> efficient and flexible communication with the
>> firmware.
>>
>> Signed-off-by: Mahesh Rao <mahesh.rao@...era.com>
>> Reviewed-by: Matthew Gerlach <matthew.gerlach@...era.com>
>> Signed-off-by: Dinh Nguyen <dinguyen@...nel.org>
>> ---
>> drivers/firmware/stratix10-svc.c | 657 +++++++++++++++++-
>> include/linux/firmware/intel/stratix10-smc.h | 24 +
>> .../firmware/intel/stratix10-svc-client.h | 88 +++
>> 3 files changed, 765 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/firmware/stratix10-svc.c b/drivers/firmware/stratix10-svc.c
>> index a8c57963da99..4480ba8b2e11 100644
>> --- a/drivers/firmware/stratix10-svc.c
>> +++ b/drivers/firmware/stratix10-svc.c
>> @@ -4,9 +4,11 @@
>> * Copyright (C) 2025, Altera Corporation
>> */
>>
>> +#include <linux/atomic.h>
>> #include <linux/completion.h>
>> #include <linux/delay.h>
>> #include <linux/genalloc.h>
>> +#include <linux/hashtable.h>
>> #include <linux/io.h>
>> #include <linux/kfifo.h>
>> #include <linux/kthread.h>
>> @@ -44,6 +46,47 @@
>> #define STRATIX10_RSU "stratix10-rsu"
>> #define INTEL_FCS "intel-fcs"
>>
>> +/*Maximum number of SDM client IDs.*/
>> +#define MAX_SDM_CLIENT_IDS 16
>> +/*Client ID for SIP Service Version 1.*/
>> +#define SIP_SVC_V1_CLIENT_ID 0x1
>> +/*Maximum number of SDM job IDs.*/
>> +#define MAX_SDM_JOB_IDS 16
>> +/*Number of bits used for asynchronous transaction hashing.*/
>> +#define ASYNC_TRX_HASH_BITS 3
>
> Tabs for defines to make them more readable?
>
> And use spaces in your comments please.
>
>
>> +/* Total number of transaction IDs, which is a combination of
>> + * client ID and job ID.
>> + */
>
> Wrong comment style.
>
> I stopped reading here :(
I will fix this in the next revision.
>
> thanks,
>
> greg k-h
Powered by blists - more mailing lists