Em Tue, 10 Apr 2018 05:35:37 +0000
Tomasz Figa <tfiga@xxxxxxxxxx> escreveu:
> Hi Hans,
>
> On Mon, Apr 9, 2018 at 11:20 PM Hans Verkuil <hverkuil@xxxxxxxxx> wrote:
> [snip]
> > diff --git a/drivers/media/media-request.c b/drivers/media/media-request.c
> > new file mode 100644
> > index 000000000000..ead78613fdbe
> > --- /dev/null
> > +++ b/drivers/media/media-request.c
> > @@ -0,0 +1,23 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Media device request objects
> > + *
> > + * Copyright (C) 2018 Intel Corporation
> > + * Copyright (C) 2018, The Chromium OS Authors. All rights reserved.
>
> I'm not sure about the origin of this line, but it's not a correct
> copyright for kernel code produced as a part of Chrome OS project. It would
> normally be something like
>
> Copyright (C) 2018 Google, Inc.
Also, it sounds a lot of copyright for a file with just stub :-)
>
> > + *
> > + * Author: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
> > + */
> > +
> > +#include <linux/anon_inodes.h>
> > +#include <linux/file.h>
> > +#include <linux/mm.h>
> > +#include <linux/string.h>
> > +
> > +#include <media/media-device.h>
> > +#include <media/media-request.h>
> > +
> > +int media_request_alloc(struct media_device *mdev,
> > + struct media_request_alloc *alloc)
> > +{
> > + return -ENOMEM;
> > +}
> > diff --git a/include/media/media-device.h b/include/media/media-device.h
> > index bcc6ec434f1f..07e323c57202 100644
> > --- a/include/media/media-device.h
> > +++ b/include/media/media-device.h
> > @@ -19,6 +19,7 @@
> > #ifndef _MEDIA_DEVICE_H
> > #define _MEDIA_DEVICE_H
>
> > +#include <linux/anon_inodes.h>
>
> What is the need for anon_inodes in this header?
>
> > #include <linux/list.h>
> > #include <linux/mutex.h>
>
> > @@ -27,6 +28,7 @@
>
> > struct ida;
> > struct device;
> > +struct media_device;
>
> > /**
> > * struct media_entity_notify - Media Entity Notify
> > @@ -50,10 +52,16 @@ struct media_entity_notify {
> > * struct media_device_ops - Media device operations
> > * @link_notify: Link state change notification callback. This callback
> is
> > * called with the graph_mutex held.
> > + * @req_alloc: Allocate a request
> > + * @req_free: Free a request
> > + * @req_queue: Queue a request
> > */
> > struct media_device_ops {
> > int (*link_notify)(struct media_link *link, u32 flags,
> > unsigned int notification);
> > + struct media_request *(*req_alloc)(struct media_device *mdev);
> > + void (*req_free)(struct media_request *req);
> > + int (*req_queue)(struct media_request *req);
> > };
>
> > /**
> > @@ -88,6 +96,8 @@ struct media_device_ops {
> > * @disable_source: Disable Source Handler function pointer
> > *
> > * @ops: Operation handler callbacks
> > + * @req_lock: Serialise access to requests
> > + * @req_queue_mutex: Serialise validating and queueing requests
>
> Let's bikeshed a bit! "access" sounds like a superset of "validating and
> queuing" to me. Perhaps it could make sense to be a bit more specific on
> what type of access the spinlock is used for?
>
> Best regards,
> Tomasz
Thanks,
Mauro
