diff mbox

[v2,2/2,media] media: Improve documentation for link_setup/link_modify

Message ID 0a562f0766e40f47092716e0f7744368bd6d963d.1459243882.git.mchehab@osg.samsung.com (mailing list archive)
State New, archived
Headers show

Commit Message

Mauro Carvalho Chehab March 29, 2016, 9:31 a.m. UTC
Those callbacks are called with the media_device.graph_mutex hold.

Add a note about that, as the code called by those notifiers should
not be touching in the mutex.

Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
---
 include/media/media-device.h | 3 ++-
 include/media/media-entity.h | 3 +++
 2 files changed, 5 insertions(+), 1 deletion(-)

Comments

Sakari Ailus March 29, 2016, 10:23 a.m. UTC | #1
Hi Mauro,

On Tue, Mar 29, 2016 at 06:31:28AM -0300, Mauro Carvalho Chehab wrote:
> Those callbacks are called with the media_device.graph_mutex hold.
> 
> Add a note about that, as the code called by those notifiers should
> not be touching in the mutex.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> ---
>  include/media/media-device.h | 3 ++-
>  include/media/media-entity.h | 3 +++
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/include/media/media-device.h b/include/media/media-device.h
> index b04cfa907350..e6ad30c323fc 100644
> --- a/include/media/media-device.h
> +++ b/include/media/media-device.h
> @@ -312,7 +312,8 @@ struct media_entity_notify {
>   * @enable_source: Enable Source Handler function pointer
>   * @disable_source: Disable Source Handler function pointer
>   *
> - * @link_notify: Link state change notification callback
> + * @link_notify: Link state change notification callback. This callback is
> + * Called with the graph_mutex hold.

s/Called/called/
s/hold/held/

>   *
>   * This structure represents an abstract high-level media device. It allows easy
>   * access to entities and provides basic media device-level support. The
> diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> index 6dc9e4e8cbd4..0b16ebe36db7 100644
> --- a/include/media/media-entity.h
> +++ b/include/media/media-entity.h
> @@ -179,6 +179,9 @@ struct media_pad {
>   * @link_validate:	Return whether a link is valid from the entity point of
>   *			view. The media_entity_pipeline_start() function
>   *			validates all links by calling this operation. Optional.
> + *
> + * Note: Those ioctls should not touch the struct media_device.@graph_mutex
> + * field, as they're called with it already hold.

These aren't really IOCTLs. They are operation callbacks.

How about:

Note: the callbacks are called struct media_device.@graph_mutex held.

Feel free to replace "callbacks" with "ops" or "operations" or such.

With these changes,

Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>

>   */
>  struct media_entity_operations {
>  	int (*link_setup)(struct media_entity *entity,
diff mbox

Patch

diff --git a/include/media/media-device.h b/include/media/media-device.h
index b04cfa907350..e6ad30c323fc 100644
--- a/include/media/media-device.h
+++ b/include/media/media-device.h
@@ -312,7 +312,8 @@  struct media_entity_notify {
  * @enable_source: Enable Source Handler function pointer
  * @disable_source: Disable Source Handler function pointer
  *
- * @link_notify: Link state change notification callback
+ * @link_notify: Link state change notification callback. This callback is
+ * Called with the graph_mutex hold.
  *
  * This structure represents an abstract high-level media device. It allows easy
  * access to entities and provides basic media device-level support. The
diff --git a/include/media/media-entity.h b/include/media/media-entity.h
index 6dc9e4e8cbd4..0b16ebe36db7 100644
--- a/include/media/media-entity.h
+++ b/include/media/media-entity.h
@@ -179,6 +179,9 @@  struct media_pad {
  * @link_validate:	Return whether a link is valid from the entity point of
  *			view. The media_entity_pipeline_start() function
  *			validates all links by calling this operation. Optional.
+ *
+ * Note: Those ioctls should not touch the struct media_device.@graph_mutex
+ * field, as they're called with it already hold.
  */
 struct media_entity_operations {
 	int (*link_setup)(struct media_entity *entity,