Re: [PATCH 3/5] firmware: stratix10-svc: Add initial support for asynchronous communication with Stratix10 service channel

[Greg KH <gregkh@linuxfoundation.org>]

On Tue, Jul 22, 2025 at 11:30:43AM -0500, Dinh Nguyen wrote:
> From: Mahesh Rao <mahesh.rao@altera.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@altera.com>
> Reviewed-by: Matthew Gerlach <matthew.gerlach@altera.com>
> Signed-off-by: Dinh Nguyen <dinguyen@kernel.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 :(

thanks,

greg k-h