Thread (9 messages) 9 messages, 4 authors, 2021-02-15

Re: linux-next: build warning after merge of the v4l-dvb tree

From: Mauro Carvalho Chehab <mchehab@kernel.org>
Date: 2021-02-15 10:50:42
Also in: lkml
Subsystem: media input infrastructure (v4l/dvb), the rest, v4l2 async and fwnode frameworks · Maintainers: Mauro Carvalho Chehab, Linus Torvalds, Sakari Ailus

Em Mon, 15 Feb 2021 11:20:24 +0100
Mauro Carvalho Chehab [off-list ref] escreveu:
Em Mon, 08 Feb 2021 15:53:00 -0300
Ezequiel Garcia [off-list ref] escreveu:
quoted
On Mon, 2021-02-08 at 18:40 +0100, Mauro Carvalho Chehab wrote:
quoted
Em Mon, 08 Feb 2021 13:57:56 -0300
Ezequiel Garcia [off-list ref] escreveu:
  
quoted
On Mon, 2021-02-08 at 18:46 +0200, Sakari Ailus wrote:  
quoted
Hi Ezequiel,

Thanks for addressing this.

On Mon, Feb 08, 2021 at 01:42:21PM -0300, Ezequiel Garcia wrote:    
quoted
Hi Stephen,

On Mon, 2021-02-08 at 23:37 +1100, Stephen Rothwell wrote:    
quoted
Hi all,

After merging the v4l-dvb tree, today's linux-next build (htmldocs)
produced this warning:

include/media/v4l2-async.h:178: warning: expecting prototype for v4l2_async_notifier_add_fwnode_subdev(). Prototype was for
__v4l2_async_notifier_add_fwnode_subdev() instead
include/media/v4l2-async.h:207: warning: expecting prototype for v4l2_async_notifier_add_fwnode_remote_subdev(). Prototype was for
__v4l2_async_notifier_add_fwnode_remote_subdev() instead
include/media/v4l2-async.h:230: warning: expecting prototype for v4l2_async_notifier_add_i2c_subdev(). Prototype was for
__v4l2_async_notifier_add_i2c_subdev() instead

Maybe introduced by commit

  c1cc23625062 ("media: v4l2-async: Discourage use of v4l2_async_notifier_add_subdev")
    
Thanks for spotting this. Should be fixed by:
diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h
index 6f22daa6f067..3785445282fc 100644
--- a/include/media/v4l2-async.h
+++ b/include/media/v4l2-async.h
@@ -157,7 +157,7 @@ int __v4l2_async_notifier_add_subdev(struct v4l2_async_notifier *notifier,
                                   struct v4l2_async_subdev *asd);
 
 /**
- * v4l2_async_notifier_add_fwnode_subdev - Allocate and add a fwnode async
+ * __v4l2_async_notifier_add_fwnode_subdev - Allocate and add a fwnode async    
The problem with the approach is that this no longer documents the API that
drivers are intended to use, but the intermediate one.  
Yep. the better would be to keep documenting what will be used.
  
Is there a way to silence/ignore the warning for a specific function(s)?
No. The warning is there for a very good reason: if you do something like:


	/**
	 * delete - delete some file
	 *
	 * This function deletes something.
	 */
	void insert() {}
	/**
	 * insert - creates a new file
	 *
	 * This function creates a file and inserts something.
	 */
	void delete() {}

the output of it will be:

	$ ./scripts/kernel-doc -sphinx-version 3.0.0 silly.c
	.. c:function:: void insert ()
	
	   delete some file
	
	**Parameters**
	
	**Description**
	
	
	This function deletes something.
	
	
	.. c:function:: void delete ()
	
	   creates a new file
	
	**Parameters**
	
	**Description**
	
	
	This function creates a file and inserts something.

Which is completely wrong. If someone tries to write a code using the
above, the result will be just the opposite than what it was documented.

Btw, I noticed several places where things like that happened, because
some code were added between a Kernel-doc markup and some function.
Btw, in the specific case of this change, something like the above
quick hack would fix it.

PS:  I didn't think much when I wrote the __type description.

Also, keeping the symbols that should be documented as __foo
doesn't seem the right thing to me :-)


---

[PATCH] v4l2-async.h: Fix kerneldoc markups

Document the functions that should be used by the kAPI clients,
instead of the internal functions.

Reported-by: Stephen Rothwell <redacted>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h
index 6f22daa6f067..91dbeaa4e6ee 100644
--- a/include/media/v4l2-async.h
+++ b/include/media/v4l2-async.h
@@ -156,42 +156,38 @@ void v4l2_async_notifier_init(struct v4l2_async_notifier *notifier);
 int __v4l2_async_notifier_add_subdev(struct v4l2_async_notifier *notifier,
 				   struct v4l2_async_subdev *asd);
 
+struct v4l2_async_subdev *
+__v4l2_async_notifier_add_fwnode_subdev(struct v4l2_async_notifier *notifier,
+					struct fwnode_handle *fwnode,
+					unsigned int asd_struct_size);
 /**
  * v4l2_async_notifier_add_fwnode_subdev - Allocate and add a fwnode async
  *				subdev to the notifier's master asd_list.
  *
- * @notifier: pointer to &struct v4l2_async_notifier
- * @fwnode: fwnode handle of the sub-device to be matched
- * @asd_struct_size: size of the driver's async sub-device struct, including
- *		     sizeof(struct v4l2_async_subdev). The &struct
- *		     v4l2_async_subdev shall be the first member of
- *		     the driver's async sub-device struct, i.e. both
- *		     begin at the same memory address.
+ * @__notifier: pointer to &struct v4l2_async_notifier
+ * @__fwnode: fwnode handle of the sub-device to be matched
+ * @__type: type of the struct that contains a struct v4l2_async_subdev.
  *
  * Allocate a fwnode-matched asd of size asd_struct_size, and add it to the
  * notifiers @asd_list. The function also gets a reference of the fwnode which
  * is released later at notifier cleanup time.
  */
-struct v4l2_async_subdev *
-__v4l2_async_notifier_add_fwnode_subdev(struct v4l2_async_notifier *notifier,
-					struct fwnode_handle *fwnode,
-					unsigned int asd_struct_size);
 #define v4l2_async_notifier_add_fwnode_subdev(__notifier, __fwnode, __type)	\
 ((__type *)__v4l2_async_notifier_add_fwnode_subdev(__notifier, __fwnode,	\
 						   sizeof(__type)))
 
+struct v4l2_async_subdev *
+__v4l2_async_notifier_add_fwnode_remote_subdev(struct v4l2_async_notifier *notif,
+					       struct fwnode_handle *endpoint,
+					       unsigned int asd_struct_size);
 /**
  * v4l2_async_notifier_add_fwnode_remote_subdev - Allocate and add a fwnode
  *						  remote async subdev to the
  *						  notifier's master asd_list.
  *
- * @notif: pointer to &struct v4l2_async_notifier
- * @endpoint: local endpoint pointing to the remote sub-device to be matched
- * @asd_struct_size: size of the driver's async sub-device struct, including
- *		     sizeof(struct v4l2_async_subdev). The &struct
- *		     v4l2_async_subdev shall be the first member of
- *		     the driver's async sub-device struct, i.e. both
- *		     begin at the same memory address.
+ * @__notifier: pointer to &struct v4l2_async_notifier
+ * @__ep: local endpoint pointing to the remote sub-device to be matched
+ * @__type: type of the struct that contains a struct v4l2_async_subdev.
  *
  * Gets the remote endpoint of a given local endpoint, set it up for fwnode
  * matching and adds the async sub-device to the notifier's @asd_list. The
@@ -201,33 +197,25 @@ __v4l2_async_notifier_add_fwnode_subdev(struct v4l2_async_notifier *notifier,
  * This is just like @v4l2_async_notifier_add_fwnode_subdev, but with the
  * exception that the fwnode refers to a local endpoint, not the remote one.
  */
-struct v4l2_async_subdev *
-__v4l2_async_notifier_add_fwnode_remote_subdev(struct v4l2_async_notifier *notif,
-					       struct fwnode_handle *endpoint,
-					       unsigned int asd_struct_size);
 #define v4l2_async_notifier_add_fwnode_remote_subdev(__notifier, __ep, __type)	\
 ((__type *)__v4l2_async_notifier_add_fwnode_remote_subdev(__notifier, __ep,	\
 							  sizeof(__type)))
 
+struct v4l2_async_subdev *
+__v4l2_async_notifier_add_i2c_subdev(struct v4l2_async_notifier *notifier,
+				     int adapter_id, unsigned short address,
+				     unsigned int asd_struct_size);
 /**
  * v4l2_async_notifier_add_i2c_subdev - Allocate and add an i2c async
  *				subdev to the notifier's master asd_list.
  *
- * @notifier: pointer to &struct v4l2_async_notifier
- * @adapter_id: I2C adapter ID to be matched
- * @address: I2C address of sub-device to be matched
- * @asd_struct_size: size of the driver's async sub-device struct, including
- *		     sizeof(struct v4l2_async_subdev). The &struct
- *		     v4l2_async_subdev shall be the first member of
- *		     the driver's async sub-device struct, i.e. both
- *		     begin at the same memory address.
+ * @__notifier: pointer to &struct v4l2_async_notifier
+ * @__adap: I2C adapter ID to be matched
+ * @__addr: I2C address of sub-device to be matched
+ * @__type: type of the struct that contains a struct v4l2_async_subdev.
  *
  * Same as above but for I2C matched sub-devices.
  */
-struct v4l2_async_subdev *
-__v4l2_async_notifier_add_i2c_subdev(struct v4l2_async_notifier *notifier,
-				     int adapter_id, unsigned short address,
-				     unsigned int asd_struct_size);
 #define v4l2_async_notifier_add_i2c_subdev(__notifier, __adap, __addr, __type)	\
 ((__type *)__v4l2_async_notifier_add_i2c_subdev(__notifier, __adap, __addr,	\
 						sizeof(__type)))

Thanks,
Mauro
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help