On Wed, Jun 18, 2025 at 01:15:16PM +0800, Ling Xu wrote: > 在 6/16/2025 7:28 PM, Ling Xu 写道: > > 在 4/8/2025 4:14 PM, Srinivas Kandagatla 写道: > >> > >> > >> On 07/04/2025 10:13, Ling Xu wrote: > >>> 在 3/21/2025 1:11 AM, Srinivas Kandagatla 写道: > >>>> > >>>> > >>>> On 20/03/2025 09:14, Ling Xu wrote: > >>>>> The fastrpc driver has support for 5 types of remoteprocs. There are > >>>>> some products which support GPDSP remoteprocs. Add changes to support > >>>>> GPDSP remoteprocs. > >>>>> > >>>>> Reviewed-by: Dmitry Baryshkov <dmitry.barysh...@oss.qualcomm.com> > >>>>> Signed-off-by: Ling Xu <quic_l...@quicinc.com> > >>>>> --- > >>>>> drivers/misc/fastrpc.c | 10 ++++++++-- > >>>>> 1 file changed, 8 insertions(+), 2 deletions(-) > >>>>> > >>>>> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c > >>>>> index 7b7a22c91fe4..80aa554b3042 100644 > >>>>> --- a/drivers/misc/fastrpc.c > >>>>> +++ b/drivers/misc/fastrpc.c > >>>>> @@ -28,7 +28,9 @@ > >>>>> #define SDSP_DOMAIN_ID (2) > >>>>> #define CDSP_DOMAIN_ID (3) > >>>>> #define CDSP1_DOMAIN_ID (4) > >>>>> -#define FASTRPC_DEV_MAX 5 /* adsp, mdsp, slpi, cdsp, cdsp1 */ > >>>>> +#define GDSP0_DOMAIN_ID (5) > >>>>> +#define GDSP1_DOMAIN_ID (6) > >>>> > >>>> We have already made the driver look silly here, Lets not add domain ids > >>>> for each instance, which is not a scalable. > >>>> > >>>> Domain ids are strictly for a domain not each instance. > >>>> > >>>> > >>>>> +#define FASTRPC_DEV_MAX 7 /* adsp, mdsp, slpi, cdsp, cdsp1, > >>>>> gdsp0, gdsp1 */ > >>>>> #define FASTRPC_MAX_SESSIONS 14 > >>>>> #define FASTRPC_MAX_VMIDS 16 > >>>>> #define FASTRPC_ALIGN 128 > >>>>> @@ -107,7 +109,9 @@ > >>>>> #define miscdev_to_fdevice(d) container_of(d, struct fastrpc_device, > >>>>> miscdev) > >>>>> static const char *domains[FASTRPC_DEV_MAX] = { "adsp", "mdsp", > >>>>> - "sdsp", "cdsp", "cdsp1" }; > >>>>> + "sdsp", "cdsp", > >>>>> + "cdsp1", "gdsp0", > >>>>> + "gdsp1" }; > >>>>> struct fastrpc_phy_page { > >>>>> u64 addr; /* physical address */ > >>>>> u64 size; /* size of contiguous region */ > >>>>> @@ -2338,6 +2342,8 @@ static int fastrpc_rpmsg_probe(struct > >>>>> rpmsg_device *rpdev) > >>>>> break; > >>>>> case CDSP_DOMAIN_ID: > >>>>> case CDSP1_DOMAIN_ID: > >>>>> + case GDSP0_DOMAIN_ID: > >>>>> + case GDSP1_DOMAIN_ID: > >>>>> data->unsigned_support = true; > >>>>> /* Create both device nodes so that we can allow both Signed > >>>>> and Unsigned PD */ > >>>>> err = fastrpc_device_register(rdev, data, true, > >>>>> domains[domain_id]); > >>>> > >>>> > >>>> Can you try this patch: only compile tested. > >>>> > >>>> ---------------------------------->cut<--------------------------------------- > >>>> From 3f8607557162e16673b26fa253d11cafdc4444cf Mon Sep 17 00:00:00 2001 > >>>> From: Srinivas Kandagatla <srinivas.kandaga...@linaro.org> > >>>> Date: Thu, 20 Mar 2025 17:07:05 +0000 > >>>> Subject: [PATCH] misc: fastrpc: cleanup the domain names > >>>> > >>>> Currently the domain ids are added for each instance of domain, this is > >>>> totally not scalable approch. > >>>> > >>>> Clean this mess and create domain ids for only domains not its > >>>> instances. > >>>> This patch also moves the domain ids to uapi header as this is required > >>>> for FASTRPC_IOCTL_GET_DSP_INFO ioctl. > >>>> > >>>> Signed-off-by: Srinivas Kandagatla <srinivas.kandaga...@linaro.org> > >>>> --- > >>>> drivers/misc/fastrpc.c | 45 ++++++++++++++++++++----------------- > >>>> include/uapi/misc/fastrpc.h | 7 ++++++ > >>>> 2 files changed, 32 insertions(+), 20 deletions(-) > >>>> > >>>> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c > >>>> index 7b7a22c91fe4..b3932897a437 100644 > >>>> --- a/drivers/misc/fastrpc.c > >>>> +++ b/drivers/misc/fastrpc.c > >>>> @@ -23,12 +23,6 @@ > >>>> #include <uapi/misc/fastrpc.h> > >>>> #include <linux/of_reserved_mem.h> > >>>> > >>>> -#define ADSP_DOMAIN_ID (0) > >>>> -#define MDSP_DOMAIN_ID (1) > >>>> -#define SDSP_DOMAIN_ID (2) > >>>> -#define CDSP_DOMAIN_ID (3) > >>>> -#define CDSP1_DOMAIN_ID (4) > >>>> -#define FASTRPC_DEV_MAX 5 /* adsp, mdsp, slpi, cdsp, cdsp1 */ > >>>> #define FASTRPC_MAX_SESSIONS 14 > >>>> #define FASTRPC_MAX_VMIDS 16 > >>>> #define FASTRPC_ALIGN 128 > >>>> @@ -106,8 +100,6 @@ > >>>> > >>>> #define miscdev_to_fdevice(d) container_of(d, struct fastrpc_device, > >>>> miscdev) > >>>> > >>>> -static const char *domains[FASTRPC_DEV_MAX] = { "adsp", "mdsp", > >>>> - "sdsp", "cdsp", "cdsp1" }; > >>>> struct fastrpc_phy_page { > >>>> u64 addr; /* physical address */ > >>>> u64 size; /* size of contiguous region */ > >>>> @@ -1769,7 +1761,7 @@ static int fastrpc_get_dsp_info(struct > >>>> fastrpc_user *fl, char __user *argp) > >>>> return -EFAULT; > >>>> > >>>> cap.capability = 0; > >>>> - if (cap.domain >= FASTRPC_DEV_MAX) { > >>>> + if (cap.domain >= FASTRPC_DOMAIN_MAX) { > >>>> dev_err(&fl->cctx->rpdev->dev, "Error: Invalid domain id:%d, > >>>> err:%d\n", > >>>> cap.domain, err); > >>>> return -ECHRNG; > >>> > >>> I tested this patch and saw one issue. > >>> Here FASTRPC_DOMAIN_MAX is set to 4, but in userspace, cdsp1 is 4, gdsp0 > >>> is 5 and gdsp1 is 6. > >> > >> > >> Why is the userspace using something that is not uAPI? > >> > >> Why does it matter if its gdsp0 or gdsp1 for the userspace? > >> It should only matter if its gdsp domain or not. > >> > > > > Give an example here: > > In test example, user can use below API to query the notification > > capability of the specific domain_id, > > (actually this will not have any functional issue, but just return an error > > and lead wrong message): > > request_status_notifications_enable(domain_id, (void*)STATUS_CONTEXT, > > pd_status_notifier_callback) > > > > this will call ioctl_getdspinfo in fastrpc_ioctl.c: > > https://github.com/quic-lxu5/fastrpc/blob/8feccfd2eb46272ad1fabed195bfddb7fd680cbd/src/fastrpc_ioctl.c#L201 > > > > code snip: > > FARF(ALWAYS, "ioctl_getdspinfo in ioctl.c domain:%d", domain); > > ioErr = ioctl(dev, FASTRPC_IOCTL_GET_DSP_INFO, &cap); > > FARF(ALWAYS, "done ioctl_getdspinfo in ioctl.c ioErr:%x", ioErr); > > > > and finally call fastrpc_get_dsp_info in fastrpc.c. > > > > if I use the patch you shared, it will report below error: > > > > UMD log: > > 2025-01-08T18:45:03.168718+00:00 qcs9100-ride-sx calculator: > > fastrpc_ioctl.c:201: ioctl_getdspinfo in ioctl.c domain:5 > > 2025-01-08T18:45:03.169307+00:00 qcs9100-ride-sx calculator: > > log_config.c:396: file_watcher_thread starting for domain 5 > > 2025-01-08T18:45:03.180355+00:00 qcs9100-ride-sx calculator: > > fastrpc_ioctl.c:203: done ioctl_getdspinfo in ioctl.c ioErr:ffffffff > > > > putty log: > > [ 1332.308444] qcom,fastrpc > > 20c00000.remoteproc:glink-edge.fastrpcglink-apps-dsp.-1.-1: Error: Invalid > > domain id:5, err:0 > > > > Because on the user side, gdsp0 and gdsp1 will be distinguished to 5 and 6. > > so do you mean you want me to modify UMD code to transfer both gdsp0 and > > gdsp1 to gdsp just in ioctl_getdspinfo? > >> > > There is no error message after removing these lines based on srini's patch, > is this modification appropriate? If so, I will submit a new patch. > @@ -1771,17 +1763,6 @@ static int fastrpc_get_dsp_info(struct fastrpc_user > *fl, char __user *argp) > return -EFAULT; > > cap.capability = 0; > - if (cap.domain >= FASTRPC_DEV_MAX) { > - dev_err(&fl->cctx->rpdev->dev, "Error: Invalid domain id:%d, > err:%d\n", > - cap.domain, err); > - return -ECHRNG; > - } > - > - /* Fastrpc Capablities does not support modem domain */ > - if (cap.domain == MDSP_DOMAIN_ID) { > - dev_err(&fl->cctx->rpdev->dev, "Error: modem not supported > %d\n", err); > - return -ECHRNG; > - } > > if (cap.attribute_id >= FASTRPC_MAX_DSP_ATTRIBUTES) { > dev_err(&fl->cctx->rpdev->dev, "Error: invalid attribute: %d, > err: %d\n",
You also need to modify fastrpc_ioctl_capability definition to drop the domain field and also modify fastrpc_get_info_from_kernel(). -- With best wishes Dmitry