On Fri, Mar 05, 2021 at 11:46:47AM +0100, Arnaud POULIQUEN wrote:
>
>
> On 3/4/21 7:55 PM, Mathieu Poirier wrote:
> > On Fri, Feb 19, 2021 at 12:14:58PM +0100, Arnaud Pouliquen wrote:
> >> Introduce the __rpmsg_chrdev_create_eptdev internal function that returns
> >> the rpmsg_eptdev context structure.
> >> This patch prepares the introduction of a RPMsg device for the
> >> char device. the RPMsg device will need a reference to the context.
> >>
> >> Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@xxxxxxxxxxx>
> >> ---
> >> drivers/rpmsg/rpmsg_char.c | 23 ++++++++++++++++++-----
> >> 1 file changed, 18 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/drivers/rpmsg/rpmsg_char.c b/drivers/rpmsg/rpmsg_char.c
> >> index 09ae1304837c..66dcb8845d6c 100644
> >> --- a/drivers/rpmsg/rpmsg_char.c
> >> +++ b/drivers/rpmsg/rpmsg_char.c
> >> @@ -328,8 +328,9 @@ int rpmsg_chrdev_eptdev_destroy(struct device *dev, void *data)
> >> }
> >> EXPORT_SYMBOL(rpmsg_chrdev_eptdev_destroy);
> >>
> >> -int rpmsg_chrdev_create_eptdev(struct rpmsg_device *rpdev, struct device *parent,
> >> - struct rpmsg_channel_info chinfo)
> >> +static struct rpmsg_eptdev *__rpmsg_chrdev_create_eptdev(struct rpmsg_device *rpdev,
> >> + struct device *parent,
> >> + struct rpmsg_channel_info chinfo)
> >> {
> >> struct rpmsg_eptdev *eptdev;
> >> struct device *dev;
> >> @@ -337,7 +338,7 @@ int rpmsg_chrdev_create_eptdev(struct rpmsg_device *rpdev, struct device *parent
> >>
> >> eptdev = kzalloc(sizeof(*eptdev), GFP_KERNEL);
> >> if (!eptdev)
> >> - return -ENOMEM;
> >> + return ERR_PTR(-ENOMEM);
> >>
> >> dev = &eptdev->dev;
> >> eptdev->rpdev = rpdev;
> >> @@ -381,7 +382,7 @@ int rpmsg_chrdev_create_eptdev(struct rpmsg_device *rpdev, struct device *parent
> >> put_device(dev);
> >> }
> >>
> >> - return ret;
> >> + return eptdev;
> >>
> >> free_ept_ida:
> >> ida_simple_remove(&rpmsg_ept_ida, dev->id);
> >> @@ -391,7 +392,19 @@ int rpmsg_chrdev_create_eptdev(struct rpmsg_device *rpdev, struct device *parent
> >> put_device(dev);
> >> kfree(eptdev);
> >>
> >> - return ret;
> >> + return ERR_PTR(ret);
> >> +}
> >> +
> >> +int rpmsg_chrdev_create_eptdev(struct rpmsg_device *rpdev, struct device *parent,
> >> + struct rpmsg_channel_info chinfo)
> >> +{
> >> + struct rpmsg_eptdev *eptdev;
> >> +
> >> + eptdev = __rpmsg_chrdev_create_eptdev(rpdev, &rpdev->dev, chinfo);
> >
> > Shouldn't the second argument to __rpmsg_chrdev_create_eptdev() be @parent?
>
> This keep the legacy hierarchy:
> https://elixir.bootlin.com/linux/latest/source/drivers/rpmsg/rpmsg_char.c#L362
In patch 12 it is clear the first and second arguments are ctrldev->rpdev and
ctrldev->dev. In this set the second arguments becomes rpdev->dev, which is
different than ctrldev->dev. Goind back to rpmsg_ctrl_probe() we have:
dev = &ctrldev->dev;
device_initialize(dev);
dev->parent = &rpdev->dev;
As such in __rpmsg_chrdev_create_eptdev(), eptdev->dev->parent becomes
ctrldev->dev->parent rather than ctrldev->dev.
>
> Thanks,
> Arnaud
>
>
> >
> >> + if (IS_ERR(eptdev))
> >> + return PTR_ERR(eptdev);
> >> +
> >> + return 0;
> >> }
> >> EXPORT_SYMBOL(rpmsg_chrdev_create_eptdev);
> >>
> >> --
> >> 2.17.1
> >>
