[libcamera-devel,4/5] media: entity: Add support for ancillary links
diff mbox series

Message ID 20211213232849.40071-5-djrscally@gmail.com
State Not Applicable
Headers show
Series
  • Introduce ancillary links
Related show

Commit Message

Daniel Scally Dec. 13, 2021, 11:28 p.m. UTC
Add functions to create and destroy ancillary links, so that they
don't need to be manually created by users.

Signed-off-by: Daniel Scally <djrscally@gmail.com>
---
Changes since the rfc:

	- (Laurent) Set gobj0 and gobj1 directly instead of the other union
	members
	- (Laurent) Added MEDIA_LNK_FL_IMMUTABLE to the kerneldoc for the new
	create function

 drivers/media/mc/mc-entity.c | 30 ++++++++++++++++++++++++++++++
 include/media/media-entity.h | 29 +++++++++++++++++++++++++++++
 2 files changed, 59 insertions(+)

Comments

kernel test robot Dec. 14, 2021, 4:06 a.m. UTC | #1
Hi Daniel,

I love your patch! Perhaps something to improve:

[auto build test WARNING on media-tree/master]
[also build test WARNING on v5.16-rc5]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Daniel-Scally/Introduce-ancillary-links/20211214-073020
base:   git://linuxtv.org/media_tree.git master
config: i386-buildonly-randconfig-r003-20211213 (https://download.01.org/0day-ci/archive/20211214/202112141239.xdqTNOOD-lkp@intel.com/config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project b6a2ddb6c8ac29412b1361810972e15221fa021c)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/627c8446267d301ed36953f7e4fa616ab6cb771a
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Daniel-Scally/Introduce-ancillary-links/20211214-073020
        git checkout 627c8446267d301ed36953f7e4fa616ab6cb771a
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/media/mc/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/media/mc/mc-entity.c:1062:6: warning: no previous prototype for function 'media_remove_ancillary_link' [-Wmissing-prototypes]
   void media_remove_ancillary_link(struct media_link *link)
        ^
   drivers/media/mc/mc-entity.c:1062:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   void media_remove_ancillary_link(struct media_link *link)
   ^
   static 
   drivers/media/mc/mc-entity.c:17:27: warning: unused function 'intf_type' [-Wunused-function]
   static inline const char *intf_type(struct media_interface *intf)
                             ^
   2 warnings generated.


vim +/media_remove_ancillary_link +1062 drivers/media/mc/mc-entity.c

  1061	
> 1062	void media_remove_ancillary_link(struct media_link *link)

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Sakari Ailus Dec. 14, 2021, 9:25 p.m. UTC | #2
Hi Daniel,

On Mon, Dec 13, 2021 at 11:28:48PM +0000, Daniel Scally wrote:
> Add functions to create and destroy ancillary links, so that they
> don't need to be manually created by users.
> 
> Signed-off-by: Daniel Scally <djrscally@gmail.com>
> ---
> Changes since the rfc:
> 
> 	- (Laurent) Set gobj0 and gobj1 directly instead of the other union
> 	members
> 	- (Laurent) Added MEDIA_LNK_FL_IMMUTABLE to the kerneldoc for the new
> 	create function
> 
>  drivers/media/mc/mc-entity.c | 30 ++++++++++++++++++++++++++++++
>  include/media/media-entity.h | 29 +++++++++++++++++++++++++++++
>  2 files changed, 59 insertions(+)
> 
> diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c
> index aeddc3f6310e..4e39e100ea03 100644
> --- a/drivers/media/mc/mc-entity.c
> +++ b/drivers/media/mc/mc-entity.c
> @@ -1052,3 +1052,33 @@ void media_remove_intf_links(struct media_interface *intf)
>  	mutex_unlock(&mdev->graph_mutex);
>  }
>  EXPORT_SYMBOL_GPL(media_remove_intf_links);
> +
> +struct media_link *media_create_ancillary_link(struct media_entity *primary,
> +					       struct media_entity *ancillary,
> +					       u32 flags)
> +{
> +	struct media_link *link;
> +
> +	link = media_add_link(&primary->links);
> +	if (!link)
> +		return ERR_PTR(-ENOMEM);
> +
> +	link->gobj0 = &primary->graph_obj;
> +	link->gobj1 = &ancillary->graph_obj;
> +	link->flags = flags | MEDIA_LNK_FL_ANCILLARY_LINK;
> +
> +	/* Initialize graph object embedded at the new link */
> +	media_gobj_create(primary->graph_obj.mdev, MEDIA_GRAPH_LINK,
> +			  &link->graph_obj);
> +
> +	return link;
> +}
> +EXPORT_SYMBOL_GPL(media_create_ancillary_link);
> +
> +void media_remove_ancillary_link(struct media_link *link)
> +{
> +	list_del(&link->list);
> +	media_gobj_destroy(&link->graph_obj);
> +	kfree(link);
> +}
> +EXPORT_SYMBOL_GPL(media_remove_ancillary_link);
> diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> index fea489f03d57..f7b1738cef88 100644
> --- a/include/media/media-entity.h
> +++ b/include/media/media-entity.h
> @@ -1104,6 +1104,35 @@ void media_remove_intf_links(struct media_interface *intf);
>   * it will issue a call to @operation\(@entity, @args\).
>   */
>  
> +/**
> + * media_create_ancillary_link() - creates a link between two entities
> + *
> + * @primary:	pointer to the primary &media_entity
> + * @ancillary:	pointer to the ancillary &media_entity
> + * @flags:	Link flags, as defined in
> + *		:ref:`include/uapi/linux/media.h <media_header>`
> + *		( seek for ``MEDIA_LNK_FL_*``)
> + *
> + *
> + * Valid values for flags:
> + *
> + * %MEDIA_LNK_FL_ENABLED
> + *   Indicates that the two entities are connected pieces of hardware that form
> + *   a single logical unit.
> + *
> + *   A typical example is a camera lens being linked to the sensor that it is
> + *   supporting.
> + *
> + * %MEDIA_LNK_FL_IMMUTABLE
> + *   Indicates that the link enabled state can't be modified at runtime. If
> + *   %MEDIA_LNK_FL_IMMUTABLE is set, then %MEDIA_LNK_FL_ENABLED must also be
> + *   set, since an immutable link is always enabled.

What's the use case for both of the flags?

I know the flags are there but what will they mean in practice for
ancillary links?

> + */
> +struct media_link *
> +media_create_ancillary_link(struct media_entity *primary,
> +			    struct media_entity *ancillary,
> +			    u32 flags);
> +
>  #define media_entity_call(entity, operation, args...)			\
>  	(((entity)->ops && (entity)->ops->operation) ?			\
>  	 (entity)->ops->operation((entity) , ##args) : -ENOIOCTLCMD)
Daniel Scally Dec. 14, 2021, 9:54 p.m. UTC | #3
Hi Sakari

On 14/12/2021 21:25, Sakari Ailus wrote:
> Hi Daniel,
>
> On Mon, Dec 13, 2021 at 11:28:48PM +0000, Daniel Scally wrote:
>> Add functions to create and destroy ancillary links, so that they
>> don't need to be manually created by users.
>>
>> Signed-off-by: Daniel Scally <djrscally@gmail.com>
>> ---
>> Changes since the rfc:
>>
>> 	- (Laurent) Set gobj0 and gobj1 directly instead of the other union
>> 	members
>> 	- (Laurent) Added MEDIA_LNK_FL_IMMUTABLE to the kerneldoc for the new
>> 	create function
>>
>>  drivers/media/mc/mc-entity.c | 30 ++++++++++++++++++++++++++++++
>>  include/media/media-entity.h | 29 +++++++++++++++++++++++++++++
>>  2 files changed, 59 insertions(+)
>>
>> diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c
>> index aeddc3f6310e..4e39e100ea03 100644
>> --- a/drivers/media/mc/mc-entity.c
>> +++ b/drivers/media/mc/mc-entity.c
>> @@ -1052,3 +1052,33 @@ void media_remove_intf_links(struct media_interface *intf)
>>  	mutex_unlock(&mdev->graph_mutex);
>>  }
>>  EXPORT_SYMBOL_GPL(media_remove_intf_links);
>> +
>> +struct media_link *media_create_ancillary_link(struct media_entity *primary,
>> +					       struct media_entity *ancillary,
>> +					       u32 flags)
>> +{
>> +	struct media_link *link;
>> +
>> +	link = media_add_link(&primary->links);
>> +	if (!link)
>> +		return ERR_PTR(-ENOMEM);
>> +
>> +	link->gobj0 = &primary->graph_obj;
>> +	link->gobj1 = &ancillary->graph_obj;
>> +	link->flags = flags | MEDIA_LNK_FL_ANCILLARY_LINK;
>> +
>> +	/* Initialize graph object embedded at the new link */
>> +	media_gobj_create(primary->graph_obj.mdev, MEDIA_GRAPH_LINK,
>> +			  &link->graph_obj);
>> +
>> +	return link;
>> +}
>> +EXPORT_SYMBOL_GPL(media_create_ancillary_link);
>> +
>> +void media_remove_ancillary_link(struct media_link *link)
>> +{
>> +	list_del(&link->list);
>> +	media_gobj_destroy(&link->graph_obj);
>> +	kfree(link);
>> +}
>> +EXPORT_SYMBOL_GPL(media_remove_ancillary_link);
>> diff --git a/include/media/media-entity.h b/include/media/media-entity.h
>> index fea489f03d57..f7b1738cef88 100644
>> --- a/include/media/media-entity.h
>> +++ b/include/media/media-entity.h
>> @@ -1104,6 +1104,35 @@ void media_remove_intf_links(struct media_interface *intf);
>>   * it will issue a call to @operation\(@entity, @args\).
>>   */
>>  
>> +/**
>> + * media_create_ancillary_link() - creates a link between two entities
>> + *
>> + * @primary:	pointer to the primary &media_entity
>> + * @ancillary:	pointer to the ancillary &media_entity
>> + * @flags:	Link flags, as defined in
>> + *		:ref:`include/uapi/linux/media.h <media_header>`
>> + *		( seek for ``MEDIA_LNK_FL_*``)
>> + *
>> + *
>> + * Valid values for flags:
>> + *
>> + * %MEDIA_LNK_FL_ENABLED
>> + *   Indicates that the two entities are connected pieces of hardware that form
>> + *   a single logical unit.
>> + *
>> + *   A typical example is a camera lens being linked to the sensor that it is
>> + *   supporting.
>> + *
>> + * %MEDIA_LNK_FL_IMMUTABLE
>> + *   Indicates that the link enabled state can't be modified at runtime. If
>> + *   %MEDIA_LNK_FL_IMMUTABLE is set, then %MEDIA_LNK_FL_ENABLED must also be
>> + *   set, since an immutable link is always enabled.
> What's the use case for both of the flags?
>
> I know the flags are there but what will they mean in practice for
> ancillary links?


Within the kernel, I don't think they have any effect now (without patch
#3 of this series the graph iteration would have tried to walk them). I
mostly intended that they would be set so that future userspace users
wouldn't be able to flag them as disabled.

>
>> + */
>> +struct media_link *
>> +media_create_ancillary_link(struct media_entity *primary,
>> +			    struct media_entity *ancillary,
>> +			    u32 flags);
>> +
>>  #define media_entity_call(entity, operation, args...)			\
>>  	(((entity)->ops && (entity)->ops->operation) ?			\
>>  	 (entity)->ops->operation((entity) , ##args) : -ENOIOCTLCMD)
Laurent Pinchart Dec. 14, 2021, 10:14 p.m. UTC | #4
Hi Daniel,

Thank you for the patch.

On Tue, Dec 14, 2021 at 09:54:32PM +0000, Daniel Scally wrote:
> On 14/12/2021 21:25, Sakari Ailus wrote:
> > On Mon, Dec 13, 2021 at 11:28:48PM +0000, Daniel Scally wrote:
> >> Add functions to create and destroy ancillary links, so that they
> >> don't need to be manually created by users.
> >>
> >> Signed-off-by: Daniel Scally <djrscally@gmail.com>
> >> ---
> >> Changes since the rfc:
> >>
> >> 	- (Laurent) Set gobj0 and gobj1 directly instead of the other union
> >> 	members
> >> 	- (Laurent) Added MEDIA_LNK_FL_IMMUTABLE to the kerneldoc for the new
> >> 	create function
> >>
> >>  drivers/media/mc/mc-entity.c | 30 ++++++++++++++++++++++++++++++
> >>  include/media/media-entity.h | 29 +++++++++++++++++++++++++++++
> >>  2 files changed, 59 insertions(+)
> >>
> >> diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c
> >> index aeddc3f6310e..4e39e100ea03 100644
> >> --- a/drivers/media/mc/mc-entity.c
> >> +++ b/drivers/media/mc/mc-entity.c
> >> @@ -1052,3 +1052,33 @@ void media_remove_intf_links(struct media_interface *intf)
> >>  	mutex_unlock(&mdev->graph_mutex);
> >>  }
> >>  EXPORT_SYMBOL_GPL(media_remove_intf_links);
> >> +
> >> +struct media_link *media_create_ancillary_link(struct media_entity *primary,
> >> +					       struct media_entity *ancillary,
> >> +					       u32 flags)
> >> +{
> >> +	struct media_link *link;
> >> +
> >> +	link = media_add_link(&primary->links);

Not a candidate for this series, but returning an error pointer from
media_add_link() could be nice.

> >> +	if (!link)
> >> +		return ERR_PTR(-ENOMEM);
> >> +
> >> +	link->gobj0 = &primary->graph_obj;
> >> +	link->gobj1 = &ancillary->graph_obj;
> >> +	link->flags = flags | MEDIA_LNK_FL_ANCILLARY_LINK;
> >> +
> >> +	/* Initialize graph object embedded at the new link */

s/embedded at/embedded in/ ?

> >> +	media_gobj_create(primary->graph_obj.mdev, MEDIA_GRAPH_LINK,
> >> +			  &link->graph_obj);
> >> +
> >> +	return link;
> >> +}
> >> +EXPORT_SYMBOL_GPL(media_create_ancillary_link);
> >> +
> >> +void media_remove_ancillary_link(struct media_link *link)
> >> +{
> >> +	list_del(&link->list);
> >> +	media_gobj_destroy(&link->graph_obj);
> >> +	kfree(link);
> >> +}
> >> +EXPORT_SYMBOL_GPL(media_remove_ancillary_link);

Non-static (and especially exported) functions must be declared in a
header file. You don't seem to use this function anywhere in this series
though, is it a leftover ?

> >> diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> >> index fea489f03d57..f7b1738cef88 100644
> >> --- a/include/media/media-entity.h
> >> +++ b/include/media/media-entity.h
> >> @@ -1104,6 +1104,35 @@ void media_remove_intf_links(struct media_interface *intf);
> >>   * it will issue a call to @operation\(@entity, @args\).
> >>   */
> >>  
> >> +/**
> >> + * media_create_ancillary_link() - creates a link between two entities

s/link/ancillary link/

> >> + *
> >> + * @primary:	pointer to the primary &media_entity
> >> + * @ancillary:	pointer to the ancillary &media_entity
> >> + * @flags:	Link flags, as defined in
> >> + *		:ref:`include/uapi/linux/media.h <media_header>`
> >> + *		( seek for ``MEDIA_LNK_FL_*``)
> >> + *
> >> + *
> >> + * Valid values for flags:
> >> + *
> >> + * %MEDIA_LNK_FL_ENABLED
> >> + *   Indicates that the two entities are connected pieces of hardware that form
> >> + *   a single logical unit.
> >> + *
> >> + *   A typical example is a camera lens being linked to the sensor that it is
> >> + *   supporting.
> >> + *
> >> + * %MEDIA_LNK_FL_IMMUTABLE
> >> + *   Indicates that the link enabled state can't be modified at runtime. If
> >> + *   %MEDIA_LNK_FL_IMMUTABLE is set, then %MEDIA_LNK_FL_ENABLED must also be
> >> + *   set, since an immutable link is always enabled.
> >
> > What's the use case for both of the flags?
> >
> > I know the flags are there but what will they mean in practice for
> > ancillary links?
> 
> Within the kernel, I don't think they have any effect now (without patch
> #3 of this series the graph iteration would have tried to walk them). I
> mostly intended that they would be set so that future userspace users
> wouldn't be able to flag them as disabled.

How about removing the flags parameter, hardcoding both
MEDIA_LNK_FL_ENABLED and MEDIA_LNK_FL_IMMUTABLE inside the function, and
specifying in the userspace API documentation that both flags are always
set of ancillary links ?

Thinking a bit further, what would be the implications of changing this
rule in the future ? I don't know what use cases may require that, but
let's assume we start exposing mutable ancillary links, or mutable and
disabled ancillary links. How will existing userspace react to that, do
we need to specify rules in the uAPI documentation to tell userspace how
to prepare for the future ?

> >> + */
> >> +struct media_link *
> >> +media_create_ancillary_link(struct media_entity *primary,
> >> +			    struct media_entity *ancillary,
> >> +			    u32 flags);
> >> +
> >>  #define media_entity_call(entity, operation, args...)			\
> >>  	(((entity)->ops && (entity)->ops->operation) ?			\
> >>  	 (entity)->ops->operation((entity) , ##args) : -ENOIOCTLCMD)
Daniel Scally Jan. 16, 2022, 11:52 p.m. UTC | #5
Hi Laurent, sorry - realised I never replied to this one

On 14/12/2021 22:14, Laurent Pinchart wrote:
> Hi Daniel,
>
> Thank you for the patch.
>
> On Tue, Dec 14, 2021 at 09:54:32PM +0000, Daniel Scally wrote:
>> On 14/12/2021 21:25, Sakari Ailus wrote:
>>> On Mon, Dec 13, 2021 at 11:28:48PM +0000, Daniel Scally wrote:
>>>> Add functions to create and destroy ancillary links, so that they
>>>> don't need to be manually created by users.
>>>>
>>>> Signed-off-by: Daniel Scally <djrscally@gmail.com>
>>>> ---
>>>> Changes since the rfc:
>>>>
>>>> 	- (Laurent) Set gobj0 and gobj1 directly instead of the other union
>>>> 	members
>>>> 	- (Laurent) Added MEDIA_LNK_FL_IMMUTABLE to the kerneldoc for the new
>>>> 	create function
>>>>
>>>>  drivers/media/mc/mc-entity.c | 30 ++++++++++++++++++++++++++++++
>>>>  include/media/media-entity.h | 29 +++++++++++++++++++++++++++++
>>>>  2 files changed, 59 insertions(+)
>>>>
>>>> diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c
>>>> index aeddc3f6310e..4e39e100ea03 100644
>>>> --- a/drivers/media/mc/mc-entity.c
>>>> +++ b/drivers/media/mc/mc-entity.c
>>>> @@ -1052,3 +1052,33 @@ void media_remove_intf_links(struct media_interface *intf)
>>>>  	mutex_unlock(&mdev->graph_mutex);
>>>>  }
>>>>  EXPORT_SYMBOL_GPL(media_remove_intf_links);
>>>> +
>>>> +struct media_link *media_create_ancillary_link(struct media_entity *primary,
>>>> +					       struct media_entity *ancillary,
>>>> +					       u32 flags)
>>>> +{
>>>> +	struct media_link *link;
>>>> +
>>>> +	link = media_add_link(&primary->links);
> Not a candidate for this series, but returning an error pointer from
> media_add_link() could be nice.
>
>>>> +	if (!link)
>>>> +		return ERR_PTR(-ENOMEM);
>>>> +
>>>> +	link->gobj0 = &primary->graph_obj;
>>>> +	link->gobj1 = &ancillary->graph_obj;
>>>> +	link->flags = flags | MEDIA_LNK_FL_ANCILLARY_LINK;
>>>> +
>>>> +	/* Initialize graph object embedded at the new link */
> s/embedded at/embedded in/ ?
>
>>>> +	media_gobj_create(primary->graph_obj.mdev, MEDIA_GRAPH_LINK,
>>>> +			  &link->graph_obj);
>>>> +
>>>> +	return link;
>>>> +}
>>>> +EXPORT_SYMBOL_GPL(media_create_ancillary_link);
>>>> +
>>>> +void media_remove_ancillary_link(struct media_link *link)
>>>> +{
>>>> +	list_del(&link->list);
>>>> +	media_gobj_destroy(&link->graph_obj);
>>>> +	kfree(link);
>>>> +}
>>>> +EXPORT_SYMBOL_GPL(media_remove_ancillary_link);
> Non-static (and especially exported) functions must be declared in a
> header file. You don't seem to use this function anywhere in this series
> though, is it a leftover ?
>
>>>> diff --git a/include/media/media-entity.h b/include/media/media-entity.h
>>>> index fea489f03d57..f7b1738cef88 100644
>>>> --- a/include/media/media-entity.h
>>>> +++ b/include/media/media-entity.h
>>>> @@ -1104,6 +1104,35 @@ void media_remove_intf_links(struct media_interface *intf);
>>>>   * it will issue a call to @operation\(@entity, @args\).
>>>>   */
>>>>  
>>>> +/**
>>>> + * media_create_ancillary_link() - creates a link between two entities
> s/link/ancillary link/
>
>>>> + *
>>>> + * @primary:	pointer to the primary &media_entity
>>>> + * @ancillary:	pointer to the ancillary &media_entity
>>>> + * @flags:	Link flags, as defined in
>>>> + *		:ref:`include/uapi/linux/media.h <media_header>`
>>>> + *		( seek for ``MEDIA_LNK_FL_*``)
>>>> + *
>>>> + *
>>>> + * Valid values for flags:
>>>> + *
>>>> + * %MEDIA_LNK_FL_ENABLED
>>>> + *   Indicates that the two entities are connected pieces of hardware that form
>>>> + *   a single logical unit.
>>>> + *
>>>> + *   A typical example is a camera lens being linked to the sensor that it is
>>>> + *   supporting.
>>>> + *
>>>> + * %MEDIA_LNK_FL_IMMUTABLE
>>>> + *   Indicates that the link enabled state can't be modified at runtime. If
>>>> + *   %MEDIA_LNK_FL_IMMUTABLE is set, then %MEDIA_LNK_FL_ENABLED must also be
>>>> + *   set, since an immutable link is always enabled.
>>> What's the use case for both of the flags?
>>>
>>> I know the flags are there but what will they mean in practice for
>>> ancillary links?
>> Within the kernel, I don't think they have any effect now (without patch
>> #3 of this series the graph iteration would have tried to walk them). I
>> mostly intended that they would be set so that future userspace users
>> wouldn't be able to flag them as disabled.
> How about removing the flags parameter, hardcoding both
> MEDIA_LNK_FL_ENABLED and MEDIA_LNK_FL_IMMUTABLE inside the function, and
> specifying in the userspace API documentation that both flags are always
> set of ancillary links ?


This is ok by me

>
> Thinking a bit further, what would be the implications of changing this
> rule in the future ? I don't know what use cases may require that, but
> let's assume we start exposing mutable ancillary links, or mutable and
> disabled ancillary links. How will existing userspace react to that, do
> we need to specify rules in the uAPI documentation to tell userspace how
> to prepare for the future ?


Yeah maybe; at the moment my libcamera implementation wouldn't treat a
disabled link any differently, and for a lens at least I can't really
see any situation where a disabled link would be valid. Perhaps multiple
LED drivers supporting the same sensor for different flash modes might
be a thing though, in which case it might be helpful to disable one of
the ancillary links so that you can flag a particular driver as the
active one, or something long those lines.


So perhaps the uAPI docs should explain that both ENABLED and IMMUTABLE
are currently hardcoded, but that userspace should still check for an
enabled link before following it.

>
>>>> + */
>>>> +struct media_link *
>>>> +media_create_ancillary_link(struct media_entity *primary,
>>>> +			    struct media_entity *ancillary,
>>>> +			    u32 flags);
>>>> +
>>>>  #define media_entity_call(entity, operation, args...)			\
>>>>  	(((entity)->ops && (entity)->ops->operation) ?			\
>>>>  	 (entity)->ops->operation((entity) , ##args) : -ENOIOCTLCMD)

Patch
diff mbox series

diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c
index aeddc3f6310e..4e39e100ea03 100644
--- a/drivers/media/mc/mc-entity.c
+++ b/drivers/media/mc/mc-entity.c
@@ -1052,3 +1052,33 @@  void media_remove_intf_links(struct media_interface *intf)
 	mutex_unlock(&mdev->graph_mutex);
 }
 EXPORT_SYMBOL_GPL(media_remove_intf_links);
+
+struct media_link *media_create_ancillary_link(struct media_entity *primary,
+					       struct media_entity *ancillary,
+					       u32 flags)
+{
+	struct media_link *link;
+
+	link = media_add_link(&primary->links);
+	if (!link)
+		return ERR_PTR(-ENOMEM);
+
+	link->gobj0 = &primary->graph_obj;
+	link->gobj1 = &ancillary->graph_obj;
+	link->flags = flags | MEDIA_LNK_FL_ANCILLARY_LINK;
+
+	/* Initialize graph object embedded at the new link */
+	media_gobj_create(primary->graph_obj.mdev, MEDIA_GRAPH_LINK,
+			  &link->graph_obj);
+
+	return link;
+}
+EXPORT_SYMBOL_GPL(media_create_ancillary_link);
+
+void media_remove_ancillary_link(struct media_link *link)
+{
+	list_del(&link->list);
+	media_gobj_destroy(&link->graph_obj);
+	kfree(link);
+}
+EXPORT_SYMBOL_GPL(media_remove_ancillary_link);
diff --git a/include/media/media-entity.h b/include/media/media-entity.h
index fea489f03d57..f7b1738cef88 100644
--- a/include/media/media-entity.h
+++ b/include/media/media-entity.h
@@ -1104,6 +1104,35 @@  void media_remove_intf_links(struct media_interface *intf);
  * it will issue a call to @operation\(@entity, @args\).
  */
 
+/**
+ * media_create_ancillary_link() - creates a link between two entities
+ *
+ * @primary:	pointer to the primary &media_entity
+ * @ancillary:	pointer to the ancillary &media_entity
+ * @flags:	Link flags, as defined in
+ *		:ref:`include/uapi/linux/media.h <media_header>`
+ *		( seek for ``MEDIA_LNK_FL_*``)
+ *
+ *
+ * Valid values for flags:
+ *
+ * %MEDIA_LNK_FL_ENABLED
+ *   Indicates that the two entities are connected pieces of hardware that form
+ *   a single logical unit.
+ *
+ *   A typical example is a camera lens being linked to the sensor that it is
+ *   supporting.
+ *
+ * %MEDIA_LNK_FL_IMMUTABLE
+ *   Indicates that the link enabled state can't be modified at runtime. If
+ *   %MEDIA_LNK_FL_IMMUTABLE is set, then %MEDIA_LNK_FL_ENABLED must also be
+ *   set, since an immutable link is always enabled.
+ */
+struct media_link *
+media_create_ancillary_link(struct media_entity *primary,
+			    struct media_entity *ancillary,
+			    u32 flags);
+
 #define media_entity_call(entity, operation, args...)			\
 	(((entity)->ops && (entity)->ops->operation) ?			\
 	 (entity)->ops->operation((entity) , ##args) : -ENOIOCTLCMD)