[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <0e03d788-a4d1-494e-a103-9f57ad81b016@linaro.org>
Date: Wed, 14 Feb 2024 07:46:46 +0000
From: Srinivas Kandagatla <srinivas.kandagatla@...aro.org>
To: Ekansh Gupta <quic_ekangupt@...cinc.com>, linux-arm-msm@...r.kernel.org
Cc: gregkh@...uxfoundation.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 04/16] misc: fastrpc: Add fastrpc multimode invoke
request support
On 02/02/2024 06:40, Ekansh Gupta wrote:
> Multimode invocation request is intended to support multiple
> different type of requests. This will include enhanced invoke
> request to support CRC check and performance counter enablement.
> This will also support few driver level user controllable
> mechanisms like usage of shared context banks, wakelock support,
> etc. This IOCTL is also added with the aim to support few
> new fastrpc features like DSP PD notification framework,
> DSP Signalling mechanism etc.
>
> Signed-off-by: Ekansh Gupta <quic_ekangupt@...cinc.com>
> ---
> drivers/misc/fastrpc.c | 176 ++++++++++++++++++++++++++----------
> include/uapi/misc/fastrpc.h | 26 ++++++
> 2 files changed, 154 insertions(+), 48 deletions(-)
>
..
> diff --git a/include/uapi/misc/fastrpc.h b/include/uapi/misc/fastrpc.h
> index f33d914d8f46..45c15be1de58 100644
> --- a/include/uapi/misc/fastrpc.h
> +++ b/include/uapi/misc/fastrpc.h
> @@ -16,6 +16,7 @@
> #define FASTRPC_IOCTL_INIT_CREATE_STATIC _IOWR('R', 9, struct fastrpc_init_create_static)
> #define FASTRPC_IOCTL_MEM_MAP _IOWR('R', 10, struct fastrpc_mem_map)
> #define FASTRPC_IOCTL_MEM_UNMAP _IOWR('R', 11, struct fastrpc_mem_unmap)
> +#define FASTRPC_IOCTL_MULTIMODE_INVOKE _IOWR('R', 12, struct fastrpc_ioctl_multimode_invoke)
> #define FASTRPC_IOCTL_GET_DSP_INFO _IOWR('R', 13, struct fastrpc_ioctl_capability)
>
> /**
> @@ -80,6 +81,31 @@ struct fastrpc_invoke {
> __u64 args;
> };
>
> +struct fastrpc_enhanced_invoke {
> + struct fastrpc_invoke inv;
> + __u64 crc;
> + __u64 perf_kernel;
> + __u64 perf_dsp;
> + __u32 reserved[8]; /* keeping reserved bits for new requirements */
> +};
> +
> +struct fastrpc_ioctl_multimode_invoke {
This struct needs some documentation.
> + __u32 req;
we use req here and then in next few lines we define the same as
fastrpc_multimode_invoke_type. I would recommend to make this type
instead of req.
> + __u32 rsvd; /* padding field */
reserved?
<---
> + __u64 invparam;
> + __u64 size;
-->
Isn't size obvious when we know request type?
This is also opening up a path for userspace to pass some random structures.
It makes more sense to have a union of all the request structures.
Why not add all the enhanced invoke uapi structures as part of this patch?
> + __u32 reserved[8]; /* keeping reserved bits for new requirements */
> +};
> +
> +enum fastrpc_multimode_invoke_type {
> + FASTRPC_INVOKE = 1,
> + FASTRPC_INVOKE_ENHANCED = 2,
> + FASTRPC_INVOKE_CONTROL = 3,
> + FASTRPC_INVOKE_DSPSIGNAL = 4,
> + FASTRPC_INVOKE_NOTIF = 5,
> + FASTRPC_INVOKE_MULTISESSION = 6,
All of these needs a proper documentation. Its impossible to understand
what they actually mean.
This applies to all the enums that are added as part of other patches to
the uapi headers.
thanks,
Srini
> +};
> +
> struct fastrpc_init_create {
> __u32 filelen; /* elf file length */
> __s32 filefd; /* fd for the file */
Powered by blists - more mailing lists