Messages in this thread Patch in this message | | | Date | Wed, 15 Jan 2020 15:28:26 +0100 (CET) | From | Clément Leger <> | Subject | Re: [PATCH] remoteproc: Add support for predefined notifyids |
| |
Hi Arnaud,
----- On 15 Jan, 2020, at 15:06, Arnaud Pouliquen arnaud.pouliquen@st.com wrote:
> Hi Clément, > > On 1/15/20 11:21 AM, Clement Leger wrote: >> In order to support preallocated notify ids, if their value is >> equal to FW_RSC_NOTIFY_ID_ANY, then do no allocate a notify id >> dynamically but try to allocate the requested one. This is useful when >> using custom ids to bind them to custom vendor resources. For instance, >> it allow to assign a group of queues to a specific interrupti in order >> to dispatch notifications. >> >> Signed-off-by: Clement Leger <cleger@kalray.eu> >> --- >> drivers/remoteproc/remoteproc_core.c | 27 +++++++++++++++++++-------- >> include/linux/remoteproc.h | 1 + >> 2 files changed, 20 insertions(+), 8 deletions(-) >> >> diff --git a/drivers/remoteproc/remoteproc_core.c >> b/drivers/remoteproc/remoteproc_core.c >> index 307df98347ba..b1485fcd0f11 100644 >> --- a/drivers/remoteproc/remoteproc_core.c >> +++ b/drivers/remoteproc/remoteproc_core.c >> @@ -351,14 +351,27 @@ int rproc_alloc_vring(struct rproc_vdev *rvdev, int i) >> /* >> * Assign an rproc-wide unique index for this vring >> * TODO: assign a notifyid for rvdev updates as well >> - * TODO: support predefined notifyids (via resource table) >> */ >> - ret = idr_alloc(&rproc->notifyids, rvring, 0, 0, GFP_KERNEL); >> - if (ret < 0) { >> - dev_err(dev, "idr_alloc failed: %d\n", ret); >> - return ret; >> + if (rsc->vring[i].notifyid == FW_RSC_NOTIFY_ID_ANY) { >> + ret = idr_alloc(&rproc->notifyids, rvring, 0, 0, GFP_KERNEL); >> + if (ret < 0) { >> + dev_err(dev, "idr_alloc failed: %d\n", ret); >> + return ret; >> + } >> + notifyid = ret; >> + >> + /* Let the rproc know the notifyid of this vring.*/ >> + rsc->vring[i].notifyid = notifyid; >> + } else { >> + /* Reserve requested notify_id */ >> + notifyid = rsc->vring[i].notifyid; >> + ret = idr_alloc(&rproc->notifyids, rvring, notifyid, >> + notifyid + 1, GFP_KERNEL); >> + if (ret < 0) { >> + dev_err(dev, "idr_alloc failed: %d\n", ret); >> + return ret; >> + } >> } >> - notifyid = ret; >> >> /* Potentially bump max_notifyid */ >> if (notifyid > rproc->max_notifyid) >> @@ -366,8 +379,6 @@ int rproc_alloc_vring(struct rproc_vdev *rvdev, int i) >> >> rvring->notifyid = notifyid; >> >> - /* Let the rproc know the notifyid of this vring.*/ >> - rsc->vring[i].notifyid = notifyid; >> return 0; >> } > The rproc_free_vring function resets the notifyid to -1 on free. > This could generate a side effect if the resource table is not reloaded.
Oh indeed, I did not thought of that. What would you recommend ? If using -1 in free vring, notify ids will be reallocated at next round.
I was also worried that it would break some existing user applications which uses "0" as a notify id in vring but expect the id to be allocated dynamically. With my modification, it means it will try to use "0" as a predefined id, leading to allocation failure.
> >> >> diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h >> index 16ad66683ad0..dcae3394243e 100644 >> --- a/include/linux/remoteproc.h >> +++ b/include/linux/remoteproc.h >> @@ -123,6 +123,7 @@ enum fw_resource_type { >> }; >> >> #define FW_RSC_ADDR_ANY (-1) >> +#define FW_RSC_NOTIFY_ID_ANY (-1)This define can also be used in >> rproc_free_vring
Indeed.
Thanks for your review.
Regards,
Clément
> > Regards, > Arnaud >> >> /** >> * struct fw_rsc_carveout - physically contiguous memory request
| |