| Message ID | 20220130235821.48076-6-djrscally@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Introduce ancillary links | expand |
Hi Daniel, I love your patch! Perhaps something to improve: [auto build test WARNING on media-tree/master] [also build test WARNING on v5.17-rc2 next-20220131] [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/20220131-080041 base: git://linuxtv.org/media_tree.git master reproduce: make htmldocs 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 >>): >> include/media/media-entity.h:1126: warning: expecting prototype for media_entity_call(). Prototype was for media_create_ancillary_link() instead vim +1126 include/media/media-entity.h 1094 1095 /** 1096 * media_entity_call - Calls a struct media_entity_operations operation on 1097 * an entity 1098 * 1099 * @entity: entity where the @operation will be called 1100 * @operation: type of the operation. Should be the name of a member of 1101 * struct &media_entity_operations. 1102 * 1103 * This helper function will check if @operation is not %NULL. On such case, 1104 * it will issue a call to @operation\(@entity, @args\). 1105 */ 1106 1107 /** 1108 * media_create_ancillary_link() - create an ancillary link between two 1109 * instances of &media_entity 1110 * 1111 * @primary: pointer to the primary &media_entity 1112 * @ancillary: pointer to the ancillary &media_entity 1113 * 1114 * Create an ancillary link between two entities, indicating that they 1115 * represent two connected pieces of hardware that form a single logical unit. 1116 * A typical example is a camera lens being linked to the sensor that it is 1117 * supporting. 1118 * 1119 * The function sets both MEDIA_LNK_FL_ENABLED and MEDIA_LNK_FL_IMMUTABLE for 1120 * the new link. This behaviour may be subject to change in the future, so 1121 * userspace applications using ancillary links should ensure that ancillary 1122 * links are enabled when in use. 1123 */ 1124 struct media_link * 1125 media_create_ancillary_link(struct media_entity *primary, > 1126 struct media_entity *ancillary); 1127 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Hi Dan, Thank you for the patch. On Sun, Jan 30, 2022 at 11:58:20PM +0000, Daniel Scally wrote: > Add functions to create ancillary links, so that they don't need to > be manually created by users. > > Signed-off-by: Daniel Scally <djrscally@gmail.com> > --- > > Changes since v1: > > - Hardcoded MEDIA_LINK_FL_IMMUTABLE and MEDIA_LINK_FL_ENABLED (Laurent) > > 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 | 22 ++++++++++++++++++++++ > include/media/media-entity.h | 21 +++++++++++++++++++++ > 2 files changed, 43 insertions(+) > > diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c > index 29d1285c805a..7bf2c73a3886 100644 > --- a/drivers/media/mc/mc-entity.c > +++ b/drivers/media/mc/mc-entity.c > @@ -1050,3 +1050,25 @@ 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) > +{ > + 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 = MEDIA_LNK_FL_IMMUTABLE | MEDIA_LNK_FL_ENABLED | > + MEDIA_LNK_FL_ANCILLARY_LINK; > + > + /* Initialize graph object embedded in 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); > diff --git a/include/media/media-entity.h b/include/media/media-entity.h > index fea489f03d57..afeda41ece4c 100644 > --- a/include/media/media-entity.h > +++ b/include/media/media-entity.h > @@ -1104,6 +1104,27 @@ void media_remove_intf_links(struct media_interface *intf); > * it will issue a call to @operation\(@entity, @args\). > */ > > +/** > + * media_create_ancillary_link() - create an ancillary link between two > + * instances of &media_entity > + * > + * @primary: pointer to the primary &media_entity > + * @ancillary: pointer to the ancillary &media_entity > + * > + * Create an ancillary link between two entities, indicating that they > + * represent two connected pieces of hardware that form a single logical unit. Here you say logical unit, while in patch 3/6 you use the term "physical relationship". I think I'd go for "logical" there too. > + * A typical example is a camera lens being linked to the sensor that it is s/lens/lens controller/ > + * supporting. > + * > + * The function sets both MEDIA_LNK_FL_ENABLED and MEDIA_LNK_FL_IMMUTABLE for > + * the new link. This behaviour may be subject to change in the future, so > + * userspace applications using ancillary links should ensure that ancillary > + * links are enabled when in use. I'd drop the last two lines as this is kernel documentation, not userspace documentation. > + */ > +struct media_link * > +media_create_ancillary_link(struct media_entity *primary, > + struct media_entity *ancillary); As reported by the kernel buildbot, this should go after media_entity_call(). > + > #define media_entity_call(entity, operation, args...) \ > (((entity)->ops && (entity)->ops->operation) ? \ > (entity)->ops->operation((entity) , ##args) : -ENOIOCTLCMD)
On Thu, Feb 03, 2022 at 01:32:28AM +0200, Laurent Pinchart wrote: > Hi Dan, > > Thank you for the patch. > > On Sun, Jan 30, 2022 at 11:58:20PM +0000, Daniel Scally wrote: > > Add functions to create ancillary links, so that they don't need to > > be manually created by users. > > > > Signed-off-by: Daniel Scally <djrscally@gmail.com> > > --- > > > > Changes since v1: > > > > - Hardcoded MEDIA_LINK_FL_IMMUTABLE and MEDIA_LINK_FL_ENABLED (Laurent) > > > > 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 | 22 ++++++++++++++++++++++ > > include/media/media-entity.h | 21 +++++++++++++++++++++ > > 2 files changed, 43 insertions(+) > > > > diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c > > index 29d1285c805a..7bf2c73a3886 100644 > > --- a/drivers/media/mc/mc-entity.c > > +++ b/drivers/media/mc/mc-entity.c > > @@ -1050,3 +1050,25 @@ 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) > > +{ > > + 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 = MEDIA_LNK_FL_IMMUTABLE | MEDIA_LNK_FL_ENABLED | > > + MEDIA_LNK_FL_ANCILLARY_LINK; > > + > > + /* Initialize graph object embedded in 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); > > diff --git a/include/media/media-entity.h b/include/media/media-entity.h > > index fea489f03d57..afeda41ece4c 100644 > > --- a/include/media/media-entity.h > > +++ b/include/media/media-entity.h > > @@ -1104,6 +1104,27 @@ void media_remove_intf_links(struct media_interface *intf); > > * it will issue a call to @operation\(@entity, @args\). > > */ > > > > +/** > > + * media_create_ancillary_link() - create an ancillary link between two > > + * instances of &media_entity > > + * > > + * @primary: pointer to the primary &media_entity > > + * @ancillary: pointer to the ancillary &media_entity > > + * > > + * Create an ancillary link between two entities, indicating that they > > + * represent two connected pieces of hardware that form a single logical unit. > > Here you say logical unit, while in patch 3/6 you use the term "physical > relationship". I think I'd go for "logical" there too. > > > + * A typical example is a camera lens being linked to the sensor that it is > > s/lens/lens controller/ > > > + * supporting. > > + * > > + * The function sets both MEDIA_LNK_FL_ENABLED and MEDIA_LNK_FL_IMMUTABLE for > > + * the new link. This behaviour may be subject to change in the future, so > > + * userspace applications using ancillary links should ensure that ancillary > > + * links are enabled when in use. > > I'd drop the last two lines as this is kernel documentation, not > userspace documentation. > > > + */ > > +struct media_link * > > +media_create_ancillary_link(struct media_entity *primary, > > + struct media_entity *ancillary); > > As reported by the kernel buildbot, this should go after > media_entity_call(). And I forgot to add Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > + > > #define media_entity_call(entity, operation, args...) \ > > (((entity)->ops && (entity)->ops->operation) ? \ > > (entity)->ops->operation((entity) , ##args) : -ENOIOCTLCMD)
diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c index 29d1285c805a..7bf2c73a3886 100644 --- a/drivers/media/mc/mc-entity.c +++ b/drivers/media/mc/mc-entity.c @@ -1050,3 +1050,25 @@ 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) +{ + 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 = MEDIA_LNK_FL_IMMUTABLE | MEDIA_LNK_FL_ENABLED | + MEDIA_LNK_FL_ANCILLARY_LINK; + + /* Initialize graph object embedded in 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); diff --git a/include/media/media-entity.h b/include/media/media-entity.h index fea489f03d57..afeda41ece4c 100644 --- a/include/media/media-entity.h +++ b/include/media/media-entity.h @@ -1104,6 +1104,27 @@ void media_remove_intf_links(struct media_interface *intf); * it will issue a call to @operation\(@entity, @args\). */ +/** + * media_create_ancillary_link() - create an ancillary link between two + * instances of &media_entity + * + * @primary: pointer to the primary &media_entity + * @ancillary: pointer to the ancillary &media_entity + * + * Create an ancillary link between two entities, indicating that they + * represent two 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. + * + * The function sets both MEDIA_LNK_FL_ENABLED and MEDIA_LNK_FL_IMMUTABLE for + * the new link. This behaviour may be subject to change in the future, so + * userspace applications using ancillary links should ensure that ancillary + * links are enabled when in use. + */ +struct media_link * +media_create_ancillary_link(struct media_entity *primary, + struct media_entity *ancillary); + #define media_entity_call(entity, operation, args...) \ (((entity)->ops && (entity)->ops->operation) ? \ (entity)->ops->operation((entity) , ##args) : -ENOIOCTLCMD)
Add functions to create ancillary links, so that they don't need to be manually created by users. Signed-off-by: Daniel Scally <djrscally@gmail.com> --- Changes since v1: - Hardcoded MEDIA_LINK_FL_IMMUTABLE and MEDIA_LINK_FL_ENABLED (Laurent) 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 | 22 ++++++++++++++++++++++ include/media/media-entity.h | 21 +++++++++++++++++++++ 2 files changed, 43 insertions(+)