On Mon 12 Jul 06:23 PDT 2021, Arnaud Pouliquen wrote: > Allow the user space application to create and release an rpmsg device > by adding RPMSG_CREATE_DEV_IOCTL and RPMSG_RELEASE_DEV_IOCTL ioctrls to > the /dev/rpmsg_ctrl interface > With https://lore.kernel.org/linux-remoteproc/CAHk-=wgea9bo-j4+LAvZF7OKPAXKqrGgiBAhXTJ3Jv5JAZgA+A@xxxxxxxxxxxxxx/ and https://lore.kernel.org/linux-arm-msm/1609958656-15064-1-git-send-email-hemantk@xxxxxxxxxxxxxx/ in mind, I would like some concrete examples of when and how this is going to be used. Also, as I said previously, this would have been better to put together with the split out of rpmsg_ctrl, because afaict this is the only reason for doing that. Or am I simply missing something? > The RPMSG_CREATE_DEV_IOCTL Ioctl can be used to instantiate a local rpmsg > device. > Depending on the back-end implementation, the associated rpmsg driver is > probed and a NS announcement can be sent to the remote processor. > So, does this imply that in order to spawn a new rpmsg_char from the host side, I would use RPMSG_CREATE_DEV_IOCTL and to open a channel announced by the remote I would use RPMSG_CREATE_EPT_IOCTL? > The RPMSG_RELEASE_DEV_IOCTL allows the user application to release a > rpmsg device created either by the remote processor or with the > RPMSG_CREATE_DEV_IOCTL call. Is this a side effect, bug or a feature? Regards, Bjorn > Depending on the back-end implementation, the associated rpmsg driver is > removed and a NS destroy rpmsg can be sent to the remote processor. > > Suggested-by: Mathieu Poirier <mathieu.poirier@xxxxxxxxxx> > Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@xxxxxxxxxxx> > Reviewed-by: Mathieu Poirier <mathieu.poirier@xxxxxxxxxx> > --- > update from V2 > - add Reviewed-by: Mathieu Poirier <mathieu.poirier@xxxxxxxxxx> > - rebased on kernel V.14-rc1 + > series V5 Restructure the rpmsg char to decorrelate the control part [1] > > > [1] https://patchwork.kernel.org/project/linux-remoteproc/list/?series=514017 > > --- > drivers/rpmsg/rpmsg_ctrl.c | 37 +++++++++++++++++++++++++++++++++---- > include/uapi/linux/rpmsg.h | 10 ++++++++++ > 2 files changed, 43 insertions(+), 4 deletions(-) > > diff --git a/drivers/rpmsg/rpmsg_ctrl.c b/drivers/rpmsg/rpmsg_ctrl.c > index eeb1708548c1..cb19e32d05e1 100644 > --- a/drivers/rpmsg/rpmsg_ctrl.c > +++ b/drivers/rpmsg/rpmsg_ctrl.c > @@ -23,6 +23,7 @@ > #include <uapi/linux/rpmsg.h> > > #include "rpmsg_char.h" > +#include "rpmsg_internal.h" > > static dev_t rpmsg_major; > > @@ -37,11 +38,13 @@ static DEFINE_IDA(rpmsg_minor_ida); > * @rpdev: underlaying rpmsg device > * @cdev: cdev for the ctrl device > * @dev: device for the ctrl device > + * @ctrl_lock: serialize the ioctrls. > */ > struct rpmsg_ctrldev { > struct rpmsg_device *rpdev; > struct cdev cdev; > struct device dev; > + struct mutex ctrl_lock; > }; > > static int rpmsg_ctrldev_open(struct inode *inode, struct file *filp) > @@ -70,9 +73,8 @@ static long rpmsg_ctrldev_ioctl(struct file *fp, unsigned int cmd, > void __user *argp = (void __user *)arg; > struct rpmsg_endpoint_info eptinfo; > struct rpmsg_channel_info chinfo; > - > - if (cmd != RPMSG_CREATE_EPT_IOCTL) > - return -EINVAL; > + struct rpmsg_device *rpdev; > + int ret = 0; > > if (copy_from_user(&eptinfo, argp, sizeof(eptinfo))) > return -EFAULT; > @@ -82,7 +84,33 @@ static long rpmsg_ctrldev_ioctl(struct file *fp, unsigned int cmd, > chinfo.src = eptinfo.src; > chinfo.dst = eptinfo.dst; > > - return rpmsg_chrdev_eptdev_create(ctrldev->rpdev, &ctrldev->dev, chinfo); > + mutex_lock(&ctrldev->ctrl_lock); > + switch (cmd) { > + case RPMSG_CREATE_EPT_IOCTL: > + ret = rpmsg_chrdev_eptdev_create(ctrldev->rpdev, &ctrldev->dev, chinfo); > + break; > + > + case RPMSG_CREATE_DEV_IOCTL: > + rpdev = rpmsg_create_channel(ctrldev->rpdev, &chinfo); > + if (!rpdev) { > + dev_err(&ctrldev->dev, "failed to create %s channel\n", chinfo.name); > + ret = -ENXIO; > + } > + break; > + > + case RPMSG_RELEASE_DEV_IOCTL: > + ret = rpmsg_release_channel(ctrldev->rpdev, &chinfo); > + if (ret) > + dev_err(&ctrldev->dev, "failed to release %s channel (%d)\n", > + chinfo.name, ret); > + break; > + > + default: > + ret = -EINVAL; > + } > + mutex_unlock(&ctrldev->ctrl_lock); > + > + return ret; > }; > > static const struct file_operations rpmsg_ctrldev_fops = { > @@ -119,6 +147,7 @@ static int rpmsg_ctrldev_probe(struct rpmsg_device *rpdev) > device_initialize(dev); > dev->parent = &rpdev->dev; > > + mutex_init(&ctrldev->ctrl_lock); > cdev_init(&ctrldev->cdev, &rpmsg_ctrldev_fops); > ctrldev->cdev.owner = THIS_MODULE; > > diff --git a/include/uapi/linux/rpmsg.h b/include/uapi/linux/rpmsg.h > index f5ca8740f3fb..1637e68177d9 100644 > --- a/include/uapi/linux/rpmsg.h > +++ b/include/uapi/linux/rpmsg.h > @@ -33,4 +33,14 @@ struct rpmsg_endpoint_info { > */ > #define RPMSG_DESTROY_EPT_IOCTL _IO(0xb5, 0x2) > > +/** > + * Instantiate a new local rpmsg service device. > + */ > +#define RPMSG_CREATE_DEV_IOCTL _IOW(0xb5, 0x3, struct rpmsg_endpoint_info) > + > +/** > + * Release a local rpmsg device. > + */ > +#define RPMSG_RELEASE_DEV_IOCTL _IOW(0xb5, 0x4, struct rpmsg_endpoint_info) > + > #endif > -- > 2.17.1 >
