On 2018.11.26 13:32:20 +0100, Cornelia Huck wrote:
> > @@ -104,12 +104,17 @@ static inline void mdev_put_parent(struct mdev_parent *parent)
> > }
> >
> > static int mdev_device_create_ops(struct kobject *kobj,
> > - struct mdev_device *mdev)
> > + struct mdev_device *mdev,
> > + unsigned int instances)
> > {
> > struct mdev_parent *parent = mdev->parent;
> > int ret;
> >
> > - ret = parent->ops->create(kobj, mdev);
> > + if (instances > 1) {
> > + ret = parent->ops->create_with_instances(kobj, mdev,
> > + instances);
> > + } else
> > + ret = parent->ops->create(kobj, mdev);
>
> So, that implies that a driver that supports aggregation needs to
> provide both ->create_with_instances and ->create? Won't that lead
> to code duplication in those drivers?
>
> (I'd probably just call ->create_with_instances if it exists and else
> use ->create, as you already do sanity checks in mdev_device_create().)
>
yeah, agreed, I think that may also simplify for future new mdev driver.
> > @@ -276,7 +281,8 @@ static void mdev_device_release(struct device *dev)
> > kfree(mdev);
> > }
> >
> > -int mdev_device_create(struct kobject *kobj, struct device *dev, uuid_le uuid)
> > +int mdev_device_create(struct kobject *kobj, struct device *dev, uuid_le uuid,
> > + unsigned int instances)
> > {
> > int ret;
> > struct mdev_device *mdev, *tmp;
> > @@ -287,6 +293,12 @@ int mdev_device_create(struct kobject *kobj, struct device *dev, uuid_le uuid)
> > if (!parent)
> > return -EINVAL;
> >
> > + if (instances > 1 &&
> > + !parent->ops->create_with_instances) {
>
> Maybe log a message here, so that the user can get an idea what went
> wrong?
>
yep, will add.
> > diff --git a/include/linux/mdev.h b/include/linux/mdev.h
> > index b6e048e1045f..c12c0bfc5598 100644
> > --- a/include/linux/mdev.h
> > +++ b/include/linux/mdev.h
> > @@ -31,6 +31,14 @@ struct mdev_device;
> > * @mdev: mdev_device structure on of mediated device
> > * that is being created
> > * Returns integer: success (0) or error (< 0)
> > + * @create_with_instances: Allocate aggregated instances' resources in parent device's
> > + * driver for a particular mediated device. Optional if aggregated
> > + * resources are not supported.
> > + * @kobj: kobject of type for which 'create' is called.
> > + * @mdev: mdev_device structure on of mediated device
>
> s/on of/of/
>
> (Yes, I know that @create has the same typo :)
>
blindly copied that one ;)
Thanks for the review!
--
Open Source Technology Center, Intel ltd.
$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827
Attachment:
signature.asc
Description: PGP signature
