V4L cleanups and documentation improvements

This struct is not used anymore. Get rid of it and update the documentation about what should still be converted.

Signed-off-by Mauro Carvalho Chehab <[email protected]>

---
 include/media/v4l2-common.h | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

include/media/v4l2-common.h +0 -2

index aac8b7b6e691..7dbecbe3009c 100644
@@ -224,10 +224,11 @@ void v4l2_spi_subdev_init(struct v4l2_subdev *sd,
 struct spi_device *spi,
 
 /*
-------------------------------------------------------------------------
 */
 
-/* Note: these remaining ioctls/structs should be removed as well, but they
 are -   still used in tuner-simple.c (TUNER_SET_CONFIG), cx18/ivtv (RESET)

and - v4l2-int-device.h (v4l2_routing). To remove these ioctls some more
cleanup - is needed in those modules. */
+/*
+ * FIXME: these remaining ioctls/structs should be removed as well, but they + * are still used in tuner-simple.c (TUNER_SET_CONFIG) and cx18/ivtv (RESET). + * To remove these ioctls some more cleanup is needed in those modules. + */

/* s_config */
struct v4l2_priv_tun_config {

include/media/v4l2-common.h +0 -6

@@ -238,11 +239,6 @@ struct v4l2_priv_tun_config {
 
 #define VIDIOC_INT_RESET            	_IOW ('d', 102, u32)
 
-struct v4l2_routing {
-	u32 input;
-	u32 output;
-};
-
 /*
-------------------------------------------------------------------------
 */
 
 /* Miscellaneous helper functions */

This struct is not used anymore. Get rid of it and update the documentation about what should still be converted.

Signed-off-by Mauro Carvalho Chehab <[email protected]>

---
 include/media/v4l2-common.h | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

include/media/v4l2-common.h +5 -4

index aac8b7b6e691..7dbecbe3009c 100644
@@ -224,10 +224,11 @@ void v4l2_spi_subdev_init(struct v4l2_subdev *sd, struct spi_device *spi,
 
 /* ------------------------------------------------------------------------- */
 
-/* Note: these remaining ioctls/structs should be removed as well, but they are
-   still used in tuner-simple.c (TUNER_SET_CONFIG), cx18/ivtv (RESET) and
-   v4l2-int-device.h (v4l2_routing). To remove these ioctls some more cleanup
-   is needed in those modules. */
+/*
+ * FIXME: these remaining ioctls/structs should be removed as well, but they
+ * are still used in tuner-simple.c (TUNER_SET_CONFIG) and cx18/ivtv (RESET).
+ * To remove these ioctls some more cleanup is needed in those modules.
+ */
 
 /* s_config */
 struct v4l2_priv_tun_config {

include/media/v4l2-common.h +0 -5

-struct v4l2_routing {
-	u32 input;
-	u32 output;
-};
-
 /* ------------------------------------------------------------------------- */
 
 /* Miscellaneous helper functions */
 

Quoted 7 lines

---
 include/media/v4l2-common.h | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

include/media/v4l2-common.h +0 -0

@@ -238,11 +239,6 @@ struct v4l2_priv_tun_config {
 

include/media/v4l2-common.h struct v4l2_priv_tun_config { · line 240

Quoted 1 line

include/media/v4l2-common.h +0 -0

 #define VIDIOC_INT_RESET            	_IOW ('d', 102, u32)  

Regarding this one: I *think* (long time ago) that the main reason for this was to reset a locked up IR blaster. I wonder if this is still needed after Sean's rework of this. Once that's all done and merged this would probably merit another look to see if it can be removed.

Em Fri, 13 Oct 2017 15:24:34 +0200
Hans Verkuil <[email protected]> escreveu:

Quoted 7 lines

Quoted 7 lines

---
 include/media/v4l2-common.h | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

include/media/v4l2-common.h +0 -0

@@ -238,11 +239,6 @@ struct v4l2_priv_tun_config {
 

Quoted 7 lines

include/media/v4l2-common.h struct v4l2_priv_tun_config { · line 240

Quoted 1 line

include/media/v4l2-common.h +0 -0

 #define VIDIOC_INT_RESET            	_IOW ('d', 102, u32)  

Regarding this one: I *think* (long time ago) that the main reason for this was to reset a locked up IR blaster. I wonder if this is still needed after Sean's rework of this. Once that's all done and merged this would probably merit another look to see if it can be removed.

Sean,

Could you please double-check if this is still required on RC?

This struct is there just two store two arguments of v4l2_find_nearest_format(). The other two arguments are passed as parameter.

IMHO, there isn't much sense on doing that, and that will just add one more struct to document ;)

So, let's get rid of the struct, passing the parameters directly.

Signed-off-by Mauro Carvalho Chehab <[email protected]>

---
 drivers/media/platform/vivid/vivid-vid-cap.c |  9 +++------
 drivers/media/v4l2-core/v4l2-common.c        | 13 +++++++------
 include/media/v4l2-common.h                  | 12 ++++--------
 3 files changed, 14 insertions(+), 20 deletions(-)

(quoted diff) +0 -0

b/drivers/media/platform/vivid/vivid-vid-cap.c index 01419455e545..0fbbcde19f0d 100644

drivers/media/platform/vivid/vivid-vid-cap.c +3 -6

@@ -93,11 +93,6 @@ static const struct v4l2_fract
 webcam_intervals[VIVID_WEBCAM_IVALS] = { {  1, 60 },
 };
 
-static const struct v4l2_discrete_probe webcam_probe = {
-	webcam_sizes,
-	VIVID_WEBCAM_SIZES
-};
-
 static int vid_cap_queue_setup(struct vb2_queue *vq,
 		       unsigned *nbuffers, unsigned *nplanes,
 		       unsigned sizes[], struct device *alloc_devs[])

@@ -578,7 +573,9 @@ int vivid_try_fmt_vid_cap(struct file *file, void *priv,
 mp->field = vivid_field_cap(dev, mp->field);
 	if (vivid_is_webcam(dev)) {
 		const struct v4l2_frmsize_discrete *sz =
-			v4l2_find_nearest_format(&webcam_probe, mp->width, mp->height);
+			v4l2_find_nearest_format(webcam_sizes,
+						 VIVID_WEBCAM_SIZES,
+						 mp->width, mp->height);
 
 		w = sz->width;
 		h = sz->height;

(quoted diff) +0 -0

b/drivers/media/v4l2-core/v4l2-common.c index a5ea1f517291..fb9a2a3c1072 100644

drivers/media/v4l2-core/v4l2-common.c +7 -6

@@ -371,18 +371,19 @@ void v4l_bound_align_image(u32 *w, unsigned int wmin,
 unsigned int wmax, }
 EXPORT_SYMBOL_GPL(v4l_bound_align_image);
 
-const struct v4l2_frmsize_discrete *v4l2_find_nearest_format(
-		const struct v4l2_discrete_probe *probe,
-		s32 width, s32 height)
+const struct v4l2_frmsize_discrete
+*v4l2_find_nearest_format(const struct v4l2_frmsize_discrete *sizes,
+			  const size_t num_sizes,
+			  s32 width, s32 height)
 {
 	int i;
 	u32 error, min_error = UINT_MAX;
 	const struct v4l2_frmsize_discrete *size, *best = NULL;
 
-	if (!probe)
-		return best;
+	if (!sizes)
+		return NULL;
 
-	for (i = 0, size = probe->sizes; i < probe->num_sizes; i++, size++) {
+	for (i = 0, size = sizes; i < num_sizes; i++, size++) {
 		error = abs(size->width - width) + abs(size->height - height);
 		if (error < min_error) {
 			min_error = error;

include/media/v4l2-common.h +0 -1

index 7dbecbe3009c..7ae7840df068 100644
@@ -249,14 +249,10 @@ void v4l_bound_align_image(unsigned int *w, unsigned
 int wmin, unsigned int hmax, unsigned int halign,
 			   unsigned int salign);
 
-struct v4l2_discrete_probe {

include/media/v4l2-common.h +1 -0

+			  s32 width, s32 height);
 
 void v4l2_get_timestamp(struct timeval *tv);

This struct is there just two store two arguments of v4l2_find_nearest_format(). The other two arguments are passed as parameter.

IMHO, there isn't much sense on doing that, and that will just add one more struct to document ;)

So, let's get rid of the struct, passing the parameters directly.

Signed-off-by Mauro Carvalho Chehab <[email protected]>

---
 drivers/media/platform/vivid/vivid-vid-cap.c |  9 +++------
 drivers/media/v4l2-core/v4l2-common.c        | 13 +++++++------
 include/media/v4l2-common.h                  | 12 ++++--------
 3 files changed, 14 insertions(+), 20 deletions(-)

drivers/media/platform/vivid/vivid-vid-cap.c +3 -6

index 01419455e545..0fbbcde19f0d 100644
@@ -93,11 +93,6 @@ static const struct v4l2_fract webcam_intervals[VIVID_WEBCAM_IVALS] = {
 	{  1, 60 },
 };
 
-static const struct v4l2_discrete_probe webcam_probe = {
-	webcam_sizes,
-	VIVID_WEBCAM_SIZES
-};
-
 static int vid_cap_queue_setup(struct vb2_queue *vq,
 		       unsigned *nbuffers, unsigned *nplanes,
 		       unsigned sizes[], struct device *alloc_devs[])

@@ -578,7 +573,9 @@ int vivid_try_fmt_vid_cap(struct file *file, void *priv,
 	mp->field = vivid_field_cap(dev, mp->field);
 	if (vivid_is_webcam(dev)) {
 		const struct v4l2_frmsize_discrete *sz =
-			v4l2_find_nearest_format(&webcam_probe, mp->width, mp->height);
+			v4l2_find_nearest_format(webcam_sizes,
+						 VIVID_WEBCAM_SIZES,
+						 mp->width, mp->height);
 
 		w = sz->width;
 		h = sz->height;

drivers/media/v4l2-core/v4l2-common.c +0 -0

index a5ea1f517291..fb9a2a3c1072 100644

drivers/media/v4l2-core/v4l2-common.c +5 -3

+			  const size_t num_sizes,
+			  s32 width, s32 height)
 {
 	int i;
 	u32 error, min_error = UINT_MAX;
 	const struct v4l2_frmsize_discrete *size, *best = NULL;
 
-	if (!probe)
-		return best;
+	if (!sizes)
+		return NULL;
 
-	for (i = 0, size = probe->sizes; i < probe->num_sizes; i++, size++) {
+	for (i = 0, size = sizes; i < num_sizes; i++, size++) {
 		error = abs(size->width - width) + abs(size->height - height);
 		if (error < min_error) {
 			min_error = error;

include/media/v4l2-common.h +0 -0

index 7dbecbe3009c..7ae7840df068 100644
@@ -249,14 +249,10 @@ void v4l_bound_align_image(unsigned int *w, unsigned int wmin,
 			   unsigned int hmax, unsigned int halign,
 			   unsigned int salign);
 

There are several ancillary functions that aren't documented.

Document them.

Signed-off-by Mauro Carvalho Chehab <[email protected]>

---
 drivers/media/v4l2-core/v4l2-common.c |  14 -----
 include/media/v4l2-common.h           | 104 +++++++++++++++++++++++++++----
 2 files changed, 94 insertions(+), 24 deletions(-)

(quoted diff) +0 -0

b/drivers/media/v4l2-core/v4l2-common.c index fb9a2a3c1072..ac404d5083e0 100644

drivers/media/v4l2-core/v4l2-common.c +0 -2

@@ -320,20 +320,6 @@ static unsigned int clamp_align(unsigned int x,
 unsigned int min, return x;
 }
 
-/* Bound an image to have a width between wmin and wmax, and height between
- * hmin and hmax, inclusive.  Additionally, the width will be a multiple

of - * 2^walign, the height will be a multiple of 2^halign, and the overall size - * (width*height) will be a multiple of 2^salign. The image may be shrunk - * or enlarged to fit the alignment constraints.
- *

- * The width or height maximum must not be smaller than the corresponding
- * minimum. The alignments must not be so high there are no possible image
- * sizes within the allowed bounds. wmin and hmin must be at least 1 - * (don't use 0). If you don't care about a certain alignment, specify 0, - * as 2^0 is 1 and one byte alignment is equivalent to no alignment. If - * you only want to adjust downward, specify a maximum that's the same as - * the initial value.
- */

void v4l_bound_align_image(u32 *w, unsigned int wmin, unsigned int wmax,

drivers/media/v4l2-core/v4l2-common.c +0 -0

 			   unsigned int walign,
 			   u32 *h, unsigned int hmin, unsigned int hmax,

include/media/v4l2-common.h +0 -0

index 7ae7840df068..d3f5f907d566 100644
@@ -174,17 +174,43 @@ void v4l2_i2c_subdev_init(struct v4l2_subdev *sd,

struct i2c_client *client, */
unsigned short v4l2_i2c_subdev_addr(struct v4l2_subdev *sd);

include/media/v4l2-common.h +24 -6

+/**
+ * enum v4l2_i2c_tuner_type - specifies the range of tuner address that
+ *	should be used when seeking for I2C devices.
+ *
+ * @ADDRS_RADIO:		Radio tuner addresses.
+ * 				Represent the following I2C addresses:
+ * 				0x10 (if compiled with tea5761 support)
+ *				and 0x60.
+ * @ADDRS_DEMOD:		Demod tuner addresses.
+ * 				Represent the following I2C addresses:
+ * 				0x42, 0x43, 0x4a and 0x4b.
+ * @ADDRS_TV:			TV tuner addresses.
+ * 				Represent the following I2C addresses:
+ * 				0x42, 0x43, 0x4a, 0x4b, 0x60, 0x61, 0x62,
+ *				0x63 and 0x64.
+ * @ADDRS_TV_WITH_DEMOD:	TV tuner addresses if demod is present, this
+ *				excludes addresses used by the demodulator
+ *				from the list of candidates.
+ * 				Represent the following I2C addresses:
+ * 				0x60, 0x61, 0x62, 0x63 and 0x64.
+ *
+ * NOTE: All I2C addresses above use the 7-bit notation.
+ */
 enum v4l2_i2c_tuner_type {
-	ADDRS_RADIO,	/* Radio tuner addresses */
-	ADDRS_DEMOD,	/* Demod tuner addresses */
-	ADDRS_TV,	/* TV tuner addresses */
-	/* TV tuner addresses if demod is present, this excludes
-	   addresses used by the demodulator from the list of
-	   candidates. */
+	ADDRS_RADIO,

+ * &enum v4l2_i2c_tuner_type.
+ *
+ * NOTE: Use only if the tuner addresses are unknown.
+ */
const unsigned short *v4l2_i2c_tuner_addrs(enum v4l2_i2c_tuner_type type);

/* --------------------------------------------------------------------- */ @@ -228,6 +254,9 @@ void v4l2_spi_subdev_init(struct v4l2_subdev *sd,
struct spi_device *spi,
* FIXME: these remaining ioctls/structs should be removed as well, but they
* are still used in tuner-simple.c (TUNER_SET_CONFIG) and cx18/ivtv (RESET).
* To remove these ioctls some more cleanup is needed in those modules.
+ *
+ * It doesn't make much sense on documenting them, as what we really want is
+ * to get rid of them.
*/

/* s_config */

include/media/v4l2-common.h +13 -0

+ *
+ * @width:	pointer to width that will be adjusted if needed.
+ * @wmin:	minimum width.
+ * @wmax:	maximum width.
+ * @walign:	least significant bit on width.
+ * @height:	pointer to height that will be adjusted if needed.
+ * @hmin:	minimum height.
+ * @hmax:	maximum height.
+ * @halign:	least significant bit on width.
+ * @salign:	least significant bit for the image size (e. g.
+ *		:math:`width * height`).
+ *
+ * Bound an image to have @width between @wmin and @wmax, and @height

between
+ * @hmin and @hmax, inclusive.

+ *
+ * Additionally, the @width will be a multiple of :math:2^{walign},
+ * the @height will be a multiple of :math:2^{halign}, and the overall
+ * size :math:width * height will be a multiple of :math:2^{salign}.
+ *
+ * .. note::
+ *
+ * #. The image may be shrunk or enlarged to fit the alignment
constraints.

+ *    #. @wmax must not be smaller than @wmin.
+ *    #. @hmax must not be smaller than @hmin.
+ *    #. The alignments must not be so high there are no possible image

+ * sizes within the allowed bounds.
+ * #. @wmin and @hmin must be at least 1 (don't use 0).
+ * #. For @walign, @halign and @salign, if you don't care about a
certain
+ * alignment, specify ``0``, as :math:2^0 = 1 and one byte
alignment
+ * is equivalent to no alignment.
+ * #. If you only want to adjust downward, specify a maximum that's the
+ * same as the initial value.
+ */

include/media/v4l2-common.h +7 -1

+void v4l_bound_align_image(unsigned int *width, unsigned int wmin,
 			   unsigned int wmax, unsigned int walign,
-			   unsigned int *h, unsigned int hmin,
+			   unsigned int *height, unsigned int hmin,
 			   unsigned int hmax, unsigned int halign,
 			   unsigned int salign);
 
+/**
+ * v4l2_find_nearest_format - find the nearest format size among a discrete
+ *	set of resolutions.
+ *
+ * @sizes: array with a pointer to & struct v4l2_frmsize_discrete image

sizes.
+ * @num_sizes: size of @sizes array.

+ * @width: desired width.
+ * @height: desired heigth.
+ *
+ * Finds the closest resolution to minimize the width and height differences
+ * between what userspace requested and the supported resolutions.

+ */
const struct v4l2_frmsize_discrete *v4l2_find_nearest_format(const struct v4l2_frmsize_discrete *sizes,

include/media/v4l2-common.h +2 -0

 			  const size_t num_sizes,
 			  s32 width, s32 height);
 
+/**
+ * v4l2_get_timestamp - ancillary routine to get a timestamp to be used

when
+ * filling streaming metadata. Internally, it uses ktime_get_ts(), +
* with is the recommended way to get it.

+ *
+ * @tv: pointer to &stuct timeval to be filled.
+ */
void v4l2_get_timestamp(struct timeval *tv);

#endif /* V4L2_COMMON_H_ */

There are several ancillary functions that aren't documented.

Document them.

Signed-off-by Mauro Carvalho Chehab <[email protected]>

---
 drivers/media/v4l2-core/v4l2-common.c |  14 -----
 include/media/v4l2-common.h           | 104 ++++++++++++++++++++++++++++++----
 2 files changed, 94 insertions(+), 24 deletions(-)

drivers/media/v4l2-core/v4l2-common.c +0 -14

index fb9a2a3c1072..ac404d5083e0 100644
@@ -320,20 +320,6 @@ static unsigned int clamp_align(unsigned int x, unsigned int min,
 	return x;
 }
 
-/* Bound an image to have a width between wmin and wmax, and height between
- * hmin and hmax, inclusive.  Additionally, the width will be a multiple of
- * 2^walign, the height will be a multiple of 2^halign, and the overall size
- * (width*height) will be a multiple of 2^salign.  The image may be shrunk
- * or enlarged to fit the alignment constraints.
- *
- * The width or height maximum must not be smaller than the corresponding
- * minimum.  The alignments must not be so high there are no possible image
- * sizes within the allowed bounds.  wmin and hmin must be at least 1
- * (don't use 0).  If you don't care about a certain alignment, specify 0,
- * as 2^0 is 1 and one byte alignment is equivalent to no alignment.  If
- * you only want to adjust downward, specify a maximum that's the same as
- * the initial value.
- */
 void v4l_bound_align_image(u32 *w, unsigned int wmin, unsigned int wmax,
 			   unsigned int walign,
 			   u32 *h, unsigned int hmin, unsigned int hmax,

include/media/v4l2-common.h +74 -9

index 7ae7840df068..d3f5f907d566 100644
@@ -174,17 +174,43 @@ void v4l2_i2c_subdev_init(struct v4l2_subdev *sd, struct i2c_client *client,
  */
 unsigned short v4l2_i2c_subdev_addr(struct v4l2_subdev *sd);
 
+/**
+ * enum v4l2_i2c_tuner_type - specifies the range of tuner address that
+ *	should be used when seeking for I2C devices.
+ *
+ * @ADDRS_RADIO:		Radio tuner addresses.
+ * 				Represent the following I2C addresses:
+ * 				0x10 (if compiled with tea5761 support)
+ *				and 0x60.
+ * @ADDRS_DEMOD:		Demod tuner addresses.
+ * 				Represent the following I2C addresses:
+ * 				0x42, 0x43, 0x4a and 0x4b.
+ * @ADDRS_TV:			TV tuner addresses.
+ * 				Represent the following I2C addresses:
+ * 				0x42, 0x43, 0x4a, 0x4b, 0x60, 0x61, 0x62,
+ *				0x63 and 0x64.
+ * @ADDRS_TV_WITH_DEMOD:	TV tuner addresses if demod is present, this
+ *				excludes addresses used by the demodulator
+ *				from the list of candidates.
+ * 				Represent the following I2C addresses:
+ * 				0x60, 0x61, 0x62, 0x63 and 0x64.
+ *
+ * NOTE: All I2C addresses above use the 7-bit notation.
+ */
 enum v4l2_i2c_tuner_type {
-	ADDRS_RADIO,	/* Radio tuner addresses */
-	ADDRS_DEMOD,	/* Demod tuner addresses */
-	ADDRS_TV,	/* TV tuner addresses */
-	/* TV tuner addresses if demod is present, this excludes
-	   addresses used by the demodulator from the list of
-	   candidates. */
+	ADDRS_RADIO,
+	ADDRS_DEMOD,
+	ADDRS_TV,
 	ADDRS_TV_WITH_DEMOD,
 };
-/* Return a list of I2C tuner addresses to probe. Use only if the tuner
-   addresses are unknown. */
+/**
+ * v4l2_i2c_tuner_addrs - Return a list of I2C tuner addresses to probe.
+ *
+ * @type: type of the tuner type to seek, as defined by
+ *	  &enum v4l2_i2c_tuner_type.
+ *
+ * NOTE: Use only if the tuner addresses are unknown.
+ */
 const unsigned short *v4l2_i2c_tuner_addrs(enum v4l2_i2c_tuner_type type);
 
 /* ------------------------------------------------------------------------- */

@@ -228,6 +254,9 @@ void v4l2_spi_subdev_init(struct v4l2_subdev *sd, struct spi_device *spi,
  * FIXME: these remaining ioctls/structs should be removed as well, but they
  * are still used in tuner-simple.c (TUNER_SET_CONFIG) and cx18/ivtv (RESET).
  * To remove these ioctls some more cleanup is needed in those modules.
+ *
+ * It doesn't make much sense on documenting them, as what we really want is
+ * to get rid of them.
  */
 
 /* s_config */

@@ -243,17 +272,72 @@ struct v4l2_priv_tun_config {
 
 /* Miscellaneous helper functions */
 
-void v4l_bound_align_image(unsigned int *w, unsigned int wmin,
+/**
+ * v4l_bound_align_image - adjust video dimensions according to
+ * 	a given criteria.
+ *
+ * @width:	pointer to width that will be adjusted if needed.
+ * @wmin:	minimum width.
+ * @wmax:	maximum width.
+ * @walign:	least significant bit on width.
+ * @height:	pointer to height that will be adjusted if needed.
+ * @hmin:	minimum height.
+ * @hmax:	maximum height.
+ * @halign:	least significant bit on width.
+ * @salign:	least significant bit for the image size (e. g.
+ *		:math:`width * height`).
+ *
+ * Bound an image to have @width between @wmin and @wmax, and @height between
+ * @hmin and @hmax, inclusive.
+ *
+ * Additionally, the @width will be a multiple of :math:`2^{walign}`,
+ * the @height will be a multiple of :math:`2^{halign}`, and the overall
+ * size :math:`width * height` will be a multiple of :math:`2^{salign}`.
+ *
+ * .. note::
+ *
+ *    #. The image may be shrunk or enlarged to fit the alignment constraints.
+ *    #. @wmax must not be smaller than @wmin.
+ *    #. @hmax must not be smaller than @hmin.
+ *    #. The alignments must not be so high there are no possible image
+ *       sizes within the allowed bounds.
+ *    #. @wmin and @hmin must be at least 1 (don't use 0).
+ *    #. For @walign, @halign and @salign, if you don't care about a certain
+ *       alignment, specify ``0``, as :math:`2^0 = 1` and one byte alignment
+ *       is equivalent to no alignment.
+ *    #. If you only want to adjust downward, specify a maximum that's the
+ *       same as the initial value.
+ */
+void v4l_bound_align_image(unsigned int *width, unsigned int wmin,
 			   unsigned int wmax, unsigned int walign,

include/media/v4l2-common.h +4 -0

+ *
+ * Finds the closest resolution to minimize the width and height differences
+ * between what userspace requested and the supported resolutions.
+ */
 const struct v4l2_frmsize_discrete

There are several widely macros that aren't documented using kernel-docs

markups.

Add it.

Signed-off-by Mauro Carvalho Chehab <[email protected]>

---
 include/media/v4l2-device.h | 238 +++++++++++++++++++++++++++++++++-------
 1 file changed, 204 insertions(+), 34 deletions(-)

include/media/v4l2-device.h +0 -1

index 8ffa94009d1a..d6d1c4f7d42c 100644
@@ -56,7 +56,6 @@ struct v4l2_ctrl_handler;
  *    #) @dev->driver_data points to this struct.
  *    #) @dev might be %NULL if there is no parent device
  */
-
 struct v4l2_device {
 	struct device *dev;
 #if defined(CONFIG_MEDIA_CONTROLLER)

@@ -166,7 +165,7 @@ void v4l2_device_unregister(struct v4l2_device
 *v4l2_dev); * v4l2_device_register_subdev - Registers a subdev with a v4l2

device. *
* @v4l2_dev: pointer to struct &v4l2_device
- * @sd: pointer to struct &v4l2_subdev
+ * @sd: pointer to &struct v4l2_subdev
*
* While registered, the subdev module is marked as in-use. *

include/media/v4l2-device.h +0 -0

@@ -179,7 +178,7 @@ int __must_check v4l2_device_register_subdev(struct

v4l2_device *v4l2_dev, /**
* v4l2_device_unregister_subdev - Unregisters a subdev with a v4l2 device.
*
- * @sd: pointer to struct &v4l2_subdev
+ * @sd: pointer to &struct v4l2_subdev
*
* .. note ::
*

include/media/v4l2-device.h +38 -4

+ *	all subdevs matching the condition.
+ *
+ * @v4l2_dev: pointer to &struct v4l2_device
+ * @sd: pointer that will be filled by the macro with all
+ *	&struct v4l2_subdev sub-devices associated with @v4l2_dev.
+ * @cond: condition to be match
+ * @o: name of the element at &struct v4l2_subdev_ops that contains @f.
+ *     Each element there groups a set of callbacks functions.
+ * @f: callback function that will be called if @cond matches.
+ * 	The callback functions are defined in groups, according to
+ *	each element at &struct v4l2_subdev_ops.
+ * @args...: arguments for @f.
+ *
+ * Ignore any errors.
+ *
+ * Note: subdevs cannot be added or deleted while walking at
+ * the subdevs list.
+ */
 #define __v4l2_device_call_subdevs_p(v4l2_dev, sd, cond, o, f, args...)	\
 	do {								\
 		list_for_each_entry((sd), &(v4l2_dev)->subdevs, list)	\

@@ -228,6 +257,24 @@ static inline void v4l2_subdev_notify(struct
 v4l2_subdev *sd, (sd)->ops->o->f((sd) , ##args);		\
 	} while (0)
 
+/**
+ * __v4l2_device_call_subdevs - Calls the specified callback for
+ *	all subdevs matching the condition.
+ *
+ * @v4l2_dev: pointer to &struct v4l2_device
+ * @cond: condition to be match
+ * @o: name of the element at &struct v4l2_subdev_ops that contains @f.
+ *     Each element there groups a set of callbacks functions.
+ * @f: callback function that will be called if @cond matches.
+ * 	The callback functions are defined in groups, according to
+ *	each element at &struct v4l2_subdev_ops.
+ * @args...: arguments for @f.
+ *
+ * Ignore any errors.
+ *
+ * Note: subdevs cannot be added or deleted while walking at
+ * the subdevs list.
+ */
 #define __v4l2_device_call_subdevs(v4l2_dev, cond, o, f, args...)	\
 	do {								\
 		struct v4l2_subdev *__sd;				\

@@ -236,10 +283,29 @@ static inline void v4l2_subdev_notify(struct
 v4l2_subdev *sd, f , ##args);		\
 	} while (0)
 
-/* Call the specified callback for all subdevs matching the condition.
-   If the callback returns an error other than 0 or -ENOIOCTLCMD, then
-   return with that error code. Note that you cannot add or delete a
-   subdev while walking the subdevs list. */
+/**
+ * __v4l2_device_call_subdevs_until_err_p - Calls the specified callback

for
+ * all subdevs matching the condition.
+ *
+ * @v4l2_dev: pointer to &struct v4l2_device
+ * @sd: pointer that will be filled by the macro with all
+ * &struct v4l2_subdev sub-devices associated with @v4l2_dev.
+ * @cond: condition to be match
+ * @o: name of the element at &struct v4l2_subdev_ops that contains @f.
+ * Each element there groups a set of callbacks functions.
+ * @f: callback function that will be called if @cond matches.
+ * The callback functions are defined in groups, according to
+ * each element at &struct v4l2_subdev_ops.
+ * @args...: arguments for @f.
+ *
+ * Return:
+ *
+ * If the callback returns an error other than 0 or ``-ENOIOCTLCMD``
+ * for any subdevice, then abort and return with that error code.
+ *
+ * Note: subdevs cannot be added or deleted while walking at
+ * the subdevs list.
+ */
#define __v4l2_device_call_subdevs_until_err_p(v4l2_dev, sd, cond, o, f,
args...) \ ({ \

include/media/v4l2-device.h +21 -0

 	long __err = 0;							\

@@ -253,6 +319,27 @@ static inline void v4l2_subdev_notify(struct
 v4l2_subdev *sd, (__err == -ENOIOCTLCMD) ? 0 : __err;				\
 })
 
+/**
+ * __v4l2_device_call_subdevs_until_err - Calls the specified callback for
+ *	all subdevs matching the condition.
+ *
+ * @v4l2_dev: pointer to &struct v4l2_device
+ * @cond: condition to be match
+ * @o: name of the element at &struct v4l2_subdev_ops that contains @f.
+ *     Each element there groups a set of callbacks functions.
+ * @f: callback function that will be called if @cond matches.
+ * 	The callback functions are defined in groups, according to
+ *	each element at &struct v4l2_subdev_ops.
+ * @args...: arguments for @f.
+ *
+ * Return:
+ *
+ * If the callback returns an error other than 0 or ``-ENOIOCTLCMD``
+ * for any subdevice, then abort and return with that error code.
+ *
+ * Note: subdevs cannot be added or deleted while walking at
+ * the subdevs list.
+ */
 #define __v4l2_device_call_subdevs_until_err(v4l2_dev, cond, o, f, args...)
 \ ({									\
 	struct v4l2_subdev *__sd;					\

include/media/v4l2-device.h +10 -4

+ * the subdevs list.
+ */
 #define v4l2_device_call_all(v4l2_dev, grpid, o, f, args...)		\
 	do {								\
 		struct v4l2_subdev *__sd;				\

@@ -272,10 +375,28 @@ static inline void v4l2_subdev_notify(struct
 v4l2_subdev *sd, ##args);					\
 	} while (0)
 
-/* Call the specified callback for all subdevs matching grp_id (if 0, then
-   match them all). If the callback returns an error other than 0 or
-   -ENOIOCTLCMD, then return with that error code. Note that you cannot
-   add or delete a subdev while walking the subdevs list. */
+/**
+ * v4l2_device_call_until_err - Calls the specified callback for
+ *	all subdevs matching a device-specific group ID.
+ *
+ * @v4l2_dev: pointer to &struct v4l2_device
+ * @grpid: &struct v4l2_subdev->grp_id group ID to match.
+ * 	   Use 0 to match them all.
+ * @o: name of the element at &struct v4l2_subdev_ops that contains @f.

include/media/v4l2-device.h +21 -8

+ * Note: subdevs cannot be added or deleted while walking at
+ * the subdevs list.
+ */
 #define v4l2_device_call_until_err(v4l2_dev, grpid, o, f, args...)	\
 ({									\
 	struct v4l2_subdev *__sd;					\

@@ -284,10 +405,24 @@ static inline void v4l2_subdev_notify(struct
 v4l2_subdev *sd, ##args);					\
 })
 
-/*
- * Call the specified callback for all subdevs where grp_id & grpmsk != 0
- * (if grpmsk == `0, then match them all). Ignore any errors. Note that you
- * cannot add or delete a subdev while walking the subdevs list.
+/**
+ * v4l2_device_mask_call_all - Calls the specified callback for
+ *	all subdevices where a group ID matches a specified bitmask.
+ *
+ * @v4l2_dev: pointer to &struct v4l2_device
+ * @grpmsk: bitmask to be checked against &struct v4l2_subdev->grp_id
+ *	    group ID to be matched. Use 0 to match them all.
+ * @o: name of the element at &struct v4l2_subdev_ops that contains @f.
+ *     Each element there groups a set of callbacks functions.
+ * @f: callback function that will be called if @cond matches.
+ * 	The callback functions are defined in groups, according to
+ *	each element at &struct v4l2_subdev_ops.
+ * @args...: arguments for @f.
+ *
+ * Ignore any errors.
+ *
+ * Note: subdevs cannot be added or deleted while walking at
+ * the subdevs list.
  */
 #define v4l2_device_mask_call_all(v4l2_dev, grpmsk, o, f, args...)	\
 	do {								\

@@ -298,11 +433,27 @@ static inline void v4l2_subdev_notify(struct
 v4l2_subdev *sd, ##args);					\
 	} while (0)
 
-/*
- * Call the specified callback for all subdevs where grp_id & grpmsk != 0
- * (if grpmsk == 0, then match them all). If the callback returns an error
- * other than 0 or %-ENOIOCTLCMD, then return with that error code. Note

that
- * you cannot add or delete a subdev while walking the subdevs list.

include/media/v4l2-device.h +34 -5

+/**
+ * v4l2_device_mask_call_until_err - Calls the specified callback for
+ *	all subdevices where a group ID matches a specified bitmask.
+ *
+ * @v4l2_dev: pointer to &struct v4l2_device
+ * @grpmsk: bitmask to be checked against &struct v4l2_subdev->grp_id
+ *	    group ID to be matched. Use 0 to match them all.
+ * @o: name of the element at &struct v4l2_subdev_ops that contains @f.
+ *     Each element there groups a set of callbacks functions.
+ * @f: callback function that will be called if @cond matches.
+ * 	The callback functions are defined in groups, according to
+ *	each element at &struct v4l2_subdev_ops.
+ * @args...: arguments for @f.
+ *
+ * Return:
+ *
+ * If the callback returns an error other than 0 or ``-ENOIOCTLCMD``
+ * for any subdevice, then abort and return with that error code.
+ *
+ * Note: subdevs cannot be added or deleted while walking at
+ * the subdevs list.
  */
 #define v4l2_device_mask_call_until_err(v4l2_dev, grpmsk, o, f, args...) \
 ({									\

@@ -312,9 +463,19 @@ static inline void v4l2_subdev_notify(struct
 v4l2_subdev *sd, ##args);					\
 })
 
-/*
- * Does any subdev with matching grpid (or all if grpid == 0) has the given
- * op?
+
+/**
+ * v4l2_device_has_op - checks if any subdev with matching grpid has a
+ * 	given ops.
+ *
+ * @v4l2_dev: pointer to &struct v4l2_device
+ * @grpid: &struct v4l2_subdev->grp_id group ID to match.
+ * 	   Use 0 to match them all.
+ * @o: name of the element at &struct v4l2_subdev_ops that contains @f.
+ *     Each element there groups a set of callbacks functions.
+ * @f: callback function that will be called if @cond matches.
+ * 	The callback functions are defined in groups, according to
+ *	each element at &struct v4l2_subdev_ops.
  */
 #define v4l2_device_has_op(v4l2_dev, grpid, o, f)			\
 ({									\

@@ -331,9 +492,18 @@ static inline void v4l2_subdev_notify(struct
 v4l2_subdev *sd, __result;							\
 })
 
-/*
- * Does any subdev with matching grpmsk (or all if grpmsk == 0) has the

given
- * op?
+/**
+ * v4l2_device_mask_has_op - checks if any subdev with matching group
+ * mask has a given ops.
+ *
+ * @v4l2_dev: pointer to &struct v4l2_device
+ * @grpmsk: bitmask to be checked against &struct v4l2_subdev->grp_id
+ * group ID to be matched. Use 0 to match them all.
+ * @o: name of the element at &struct v4l2_subdev_ops that contains @f.
+ * Each element there groups a set of callbacks functions.
+ * @f: callback function that will be called if @cond matches.
+ * The callback functions are defined in groups, according to
+ * each element at &struct v4l2_subdev_ops.
*/
#define v4l2_device_mask_has_op(v4l2_dev, grpmsk, o, f) \
({ \

Hi Mauro,

Thank you for the patch.

Quoted 1 line · On Thursday, 28 September 2017 00:46:48 EEST Mauro Carvalho Chehab wrote:

There are several widely macros that aren't documented using kernel-docs

What's a widely macro ? :-)

Quoted 3 lines

markups.

Add it.

Did you mean "Add documentation." ? "Document them." is also an option.

Quoted 2 lines

Signed-off-by Mauro Carvalho Chehab <[email protected]>

Review

Quoted 5 lines

(quoted diff) +2 -3

-/* Call the specified callback for all subdevs matching the condition.
-   Ignore any errors. Note that you cannot add or delete a subdev
-   while walking the subdevs list. */
+/**
+ * __v4l2_device_call_subdevs_p - Calls the specified callback for  

All the __v4l2_device_* macros are internal, I don't think there's a need to document them just for the sake of it.

Quoted 2 lines

+ * all subdevs matching a device-specific group ID.

How exactly is the group ID device-specific ?

Quoted 8 lines

+ * @v4l2_dev: pointer to &struct v4l2_device
+ * @grpid: &struct v4l2_subdev->grp_id group ID to match.
+ * Use 0 to match them all.
+ * @o: name of the element at &struct v4l2_subdev_ops that contains @f.
+ * Each element there groups a set of callbacks functions.
+ * @f: callback function that will be called if @cond matches.
+ * The callback functions are defined in groups, according to
+ * each element at &struct v4l2_subdev_ops.

Using the word "group" here makes it very confusing. You could use "operation class" instead.

Another option would be to document @o.@f of Sphinx doesn't complain/

There are several widely macros that aren't documented using kernel-docs markups.

Add it.

Signed-off-by Mauro Carvalho Chehab <[email protected]>

---
 include/media/v4l2-device.h | 238 +++++++++++++++++++++++++++++++++++++-------
 1 file changed, 204 insertions(+), 34 deletions(-)

include/media/v4l2-device.h +204 -34

index 8ffa94009d1a..d6d1c4f7d42c 100644
@@ -56,7 +56,6 @@ struct v4l2_ctrl_handler;
  *    #) @dev->driver_data points to this struct.
  *    #) @dev might be %NULL if there is no parent device
  */
-
 struct v4l2_device {
 	struct device *dev;
 #if defined(CONFIG_MEDIA_CONTROLLER)

@@ -166,7 +165,7 @@ void v4l2_device_unregister(struct v4l2_device *v4l2_dev);
  * v4l2_device_register_subdev - Registers a subdev with a v4l2 device.
  *
  * @v4l2_dev: pointer to struct &v4l2_device
- * @sd: pointer to struct &v4l2_subdev
+ * @sd: pointer to &struct v4l2_subdev
  *
  * While registered, the subdev module is marked as in-use.
  *

@@ -179,7 +178,7 @@ int __must_check v4l2_device_register_subdev(struct v4l2_device *v4l2_dev,
 /**
  * v4l2_device_unregister_subdev - Unregisters a subdev with a v4l2 device.
  *
- * @sd: pointer to struct &v4l2_subdev
+ * @sd: pointer to &struct v4l2_subdev
  *
  * .. note ::
  *

@@ -201,7 +200,7 @@ v4l2_device_register_subdev_nodes(struct v4l2_device *v4l2_dev);
 /**
  * v4l2_subdev_notify - Sends a notification to v4l2_device.
  *
- * @sd: pointer to struct &v4l2_subdev
+ * @sd: pointer to &struct v4l2_subdev
  * @notification: type of notification. Please notice that the notification
  *	type is driver-specific.
  * @arg: arguments for the notification. Those are specific to each

@@ -214,13 +213,43 @@ static inline void v4l2_subdev_notify(struct v4l2_subdev *sd,
 		sd->v4l2_dev->notify(sd, notification, arg);
 }
 
-/* Iterate over all subdevs. */
+/* Ancillary macros to iterate over all subdevs. */
+
+/**
+ * v4l2_device_for_each_subdev - Ancillary macro that interates over all
+ *	sub-devices
+ *
+ * @sd: pointer that will be filled by the macro with all
+ *	&struct v4l2_subdev sub-devices associated with @v4l2_dev.
+ * @v4l2_dev: pointer to &struct v4l2_device
+ *
+ * Sometimes, a driver may need to broadcast a command to all subdevices.
+ * This ancillary macro allows interacting to all sub-devices associated
+ * to a device.
+ */
 #define v4l2_device_for_each_subdev(sd, v4l2_dev)			\
 	list_for_each_entry(sd, &(v4l2_dev)->subdevs, list)
 
-/* Call the specified callback for all subdevs matching the condition.
-   Ignore any errors. Note that you cannot add or delete a subdev
-   while walking the subdevs list. */
+/**
+ * __v4l2_device_call_subdevs_p - Calls the specified callback for
+ *	all subdevs matching the condition.
+ *
+ * @v4l2_dev: pointer to &struct v4l2_device
+ * @sd: pointer that will be filled by the macro with all
+ *	&struct v4l2_subdev sub-devices associated with @v4l2_dev.
+ * @cond: condition to be match
+ * @o: name of the element at &struct v4l2_subdev_ops that contains @f.
+ *     Each element there groups a set of callbacks functions.
+ * @f: callback function that will be called if @cond matches.
+ * 	The callback functions are defined in groups, according to
+ *	each element at &struct v4l2_subdev_ops.
+ * @args...: arguments for @f.
+ *
+ * Ignore any errors.
+ *
+ * Note: subdevs cannot be added or deleted while walking at
+ * the subdevs list.
+ */
 #define __v4l2_device_call_subdevs_p(v4l2_dev, sd, cond, o, f, args...)	\
 	do {								\
 		list_for_each_entry((sd), &(v4l2_dev)->subdevs, list)	\

@@ -228,6 +257,24 @@ static inline void v4l2_subdev_notify(struct v4l2_subdev *sd,
 				(sd)->ops->o->f((sd) , ##args);		\
 	} while (0)
 
+/**
+ * __v4l2_device_call_subdevs - Calls the specified callback for
+ *	all subdevs matching the condition.
+ *
+ * @v4l2_dev: pointer to &struct v4l2_device
+ * @cond: condition to be match
+ * @o: name of the element at &struct v4l2_subdev_ops that contains @f.
+ *     Each element there groups a set of callbacks functions.
+ * @f: callback function that will be called if @cond matches.
+ * 	The callback functions are defined in groups, according to
+ *	each element at &struct v4l2_subdev_ops.
+ * @args...: arguments for @f.
+ *
+ * Ignore any errors.
+ *
+ * Note: subdevs cannot be added or deleted while walking at
+ * the subdevs list.
+ */
 #define __v4l2_device_call_subdevs(v4l2_dev, cond, o, f, args...)	\
 	do {								\
 		struct v4l2_subdev *__sd;				\

@@ -236,10 +283,29 @@ static inline void v4l2_subdev_notify(struct v4l2_subdev *sd,
 						f , ##args);		\
 	} while (0)
 
-/* Call the specified callback for all subdevs matching the condition.
-   If the callback returns an error other than 0 or -ENOIOCTLCMD, then
-   return with that error code. Note that you cannot add or delete a
-   subdev while walking the subdevs list. */
+/**
+ * __v4l2_device_call_subdevs_until_err_p - Calls the specified callback for
+ *	all subdevs matching the condition.
+ *
+ * @v4l2_dev: pointer to &struct v4l2_device
+ * @sd: pointer that will be filled by the macro with all
+ *	&struct v4l2_subdev sub-devices associated with @v4l2_dev.
+ * @cond: condition to be match
+ * @o: name of the element at &struct v4l2_subdev_ops that contains @f.
+ *     Each element there groups a set of callbacks functions.
+ * @f: callback function that will be called if @cond matches.
+ * 	The callback functions are defined in groups, according to
+ *	each element at &struct v4l2_subdev_ops.
+ * @args...: arguments for @f.
+ *
+ * Return:
+ *
+ * If the callback returns an error other than 0 or ``-ENOIOCTLCMD``
+ * for any subdevice, then abort and return with that error code.
+ *
+ * Note: subdevs cannot be added or deleted while walking at
+ * the subdevs list.
+ */
 #define __v4l2_device_call_subdevs_until_err_p(v4l2_dev, sd, cond, o, f, args...) \
 ({									\
 	long __err = 0;							\

@@ -253,6 +319,27 @@ static inline void v4l2_subdev_notify(struct v4l2_subdev *sd,
 	(__err == -ENOIOCTLCMD) ? 0 : __err;				\
 })
 
+/**
+ * __v4l2_device_call_subdevs_until_err - Calls the specified callback for
+ *	all subdevs matching the condition.
+ *
+ * @v4l2_dev: pointer to &struct v4l2_device
+ * @cond: condition to be match
+ * @o: name of the element at &struct v4l2_subdev_ops that contains @f.
+ *     Each element there groups a set of callbacks functions.
+ * @f: callback function that will be called if @cond matches.
+ * 	The callback functions are defined in groups, according to
+ *	each element at &struct v4l2_subdev_ops.
+ * @args...: arguments for @f.
+ *
+ * Return:
+ *
+ * If the callback returns an error other than 0 or ``-ENOIOCTLCMD``
+ * for any subdevice, then abort and return with that error code.
+ *
+ * Note: subdevs cannot be added or deleted while walking at
+ * the subdevs list.
+ */
 #define __v4l2_device_call_subdevs_until_err(v4l2_dev, cond, o, f, args...) \
 ({									\
 	struct v4l2_subdev *__sd;					\

@@ -260,9 +347,25 @@ static inline void v4l2_subdev_notify(struct v4l2_subdev *sd,
 						f , ##args);		\
 })
 
-/* Call the specified callback for all subdevs matching grp_id (if 0, then
-   match them all). Ignore any errors. Note that you cannot add or delete
-   a subdev while walking the subdevs list. */
+/**
+ * v4l2_device_call_all - Calls the specified callback for
+ *	all subdevs matching a device-specific group ID.
+ *
+ * @v4l2_dev: pointer to &struct v4l2_device
+ * @grpid: &struct v4l2_subdev->grp_id group ID to match.
+ * 	   Use 0 to match them all.
+ * @o: name of the element at &struct v4l2_subdev_ops that contains @f.
+ *     Each element there groups a set of callbacks functions.
+ * @f: callback function that will be called if @cond matches.
+ * 	The callback functions are defined in groups, according to
+ *	each element at &struct v4l2_subdev_ops.
+ * @args...: arguments for @f.
+ *
+ * Ignore any errors.
+ *
+ * Note: subdevs cannot be added or deleted while walking at
+ * the subdevs list.
+ */
 #define v4l2_device_call_all(v4l2_dev, grpid, o, f, args...)		\
 	do {								\
 		struct v4l2_subdev *__sd;				\

@@ -272,10 +375,28 @@ static inline void v4l2_subdev_notify(struct v4l2_subdev *sd,
 			##args);					\
 	} while (0)
 
-/* Call the specified callback for all subdevs matching grp_id (if 0, then
-   match them all). If the callback returns an error other than 0 or
-   -ENOIOCTLCMD, then return with that error code. Note that you cannot
-   add or delete a subdev while walking the subdevs list. */
+/**
+ * v4l2_device_call_until_err - Calls the specified callback for
+ *	all subdevs matching a device-specific group ID.
+ *
+ * @v4l2_dev: pointer to &struct v4l2_device
+ * @grpid: &struct v4l2_subdev->grp_id group ID to match.
+ * 	   Use 0 to match them all.
+ * @o: name of the element at &struct v4l2_subdev_ops that contains @f.
+ *     Each element there groups a set of callbacks functions.
+ * @f: callback function that will be called if @cond matches.
+ * 	The callback functions are defined in groups, according to
+ *	each element at &struct v4l2_subdev_ops.
+ * @args...: arguments for @f.
+ *
+ * Return:
+ *
+ * If the callback returns an error other than 0 or ``-ENOIOCTLCMD``
+ * for any subdevice, then abort and return with that error code.
+ *
+ * Note: subdevs cannot be added or deleted while walking at
+ * the subdevs list.
+ */
 #define v4l2_device_call_until_err(v4l2_dev, grpid, o, f, args...)	\
 ({									\
 	struct v4l2_subdev *__sd;					\

@@ -284,10 +405,24 @@ static inline void v4l2_subdev_notify(struct v4l2_subdev *sd,
 			##args);					\
 })
 
-/*
- * Call the specified callback for all subdevs where grp_id & grpmsk != 0
- * (if grpmsk == `0, then match them all). Ignore any errors. Note that you
- * cannot add or delete a subdev while walking the subdevs list.
+/**
+ * v4l2_device_mask_call_all - Calls the specified callback for
+ *	all subdevices where a group ID matches a specified bitmask.
+ *
+ * @v4l2_dev: pointer to &struct v4l2_device
+ * @grpmsk: bitmask to be checked against &struct v4l2_subdev->grp_id
+ *	    group ID to be matched. Use 0 to match them all.
+ * @o: name of the element at &struct v4l2_subdev_ops that contains @f.
+ *     Each element there groups a set of callbacks functions.
+ * @f: callback function that will be called if @cond matches.
+ * 	The callback functions are defined in groups, according to
+ *	each element at &struct v4l2_subdev_ops.
+ * @args...: arguments for @f.
+ *
+ * Ignore any errors.
+ *
+ * Note: subdevs cannot be added or deleted while walking at
+ * the subdevs list.
  */
 #define v4l2_device_mask_call_all(v4l2_dev, grpmsk, o, f, args...)	\
 	do {								\

@@ -298,11 +433,27 @@ static inline void v4l2_subdev_notify(struct v4l2_subdev *sd,
 			##args);					\
 	} while (0)
 
-/*
- * Call the specified callback for all subdevs where grp_id & grpmsk != 0
- * (if grpmsk == 0, then match them all). If the callback returns an error
- * other than 0 or %-ENOIOCTLCMD, then return with that error code. Note that
- * you cannot add or delete a subdev while walking the subdevs list.
+/**
+ * v4l2_device_mask_call_until_err - Calls the specified callback for
+ *	all subdevices where a group ID matches a specified bitmask.
+ *
+ * @v4l2_dev: pointer to &struct v4l2_device
+ * @grpmsk: bitmask to be checked against &struct v4l2_subdev->grp_id
+ *	    group ID to be matched. Use 0 to match them all.
+ * @o: name of the element at &struct v4l2_subdev_ops that contains @f.
+ *     Each element there groups a set of callbacks functions.
+ * @f: callback function that will be called if @cond matches.
+ * 	The callback functions are defined in groups, according to
+ *	each element at &struct v4l2_subdev_ops.
+ * @args...: arguments for @f.
+ *
+ * Return:
+ *
+ * If the callback returns an error other than 0 or ``-ENOIOCTLCMD``
+ * for any subdevice, then abort and return with that error code.
+ *
+ * Note: subdevs cannot be added or deleted while walking at
+ * the subdevs list.
  */
 #define v4l2_device_mask_call_until_err(v4l2_dev, grpmsk, o, f, args...) \
 ({									\

@@ -312,9 +463,19 @@ static inline void v4l2_subdev_notify(struct v4l2_subdev *sd,
 			##args);					\
 })
 
-/*
- * Does any subdev with matching grpid (or all if grpid == 0) has the given
- * op?
+
+/**
+ * v4l2_device_has_op - checks if any subdev with matching grpid has a
+ * 	given ops.
+ *
+ * @v4l2_dev: pointer to &struct v4l2_device
+ * @grpid: &struct v4l2_subdev->grp_id group ID to match.
+ * 	   Use 0 to match them all.
+ * @o: name of the element at &struct v4l2_subdev_ops that contains @f.
+ *     Each element there groups a set of callbacks functions.
+ * @f: callback function that will be called if @cond matches.
+ * 	The callback functions are defined in groups, according to
+ *	each element at &struct v4l2_subdev_ops.
  */
 #define v4l2_device_has_op(v4l2_dev, grpid, o, f)			\
 ({									\

@@ -331,9 +492,18 @@ static inline void v4l2_subdev_notify(struct v4l2_subdev *sd,
 	__result;							\
 })
 
-/*
- * Does any subdev with matching grpmsk (or all if grpmsk == 0) has the given
- * op?
+/**
+ * v4l2_device_mask_has_op - checks if any subdev with matching group
+ *	mask has a given ops.
+ *
+ * @v4l2_dev: pointer to &struct v4l2_device
+ * @grpmsk: bitmask to be checked against &struct v4l2_subdev->grp_id
+ *	    group ID to be matched. Use 0 to match them all.
+ * @o: name of the element at &struct v4l2_subdev_ops that contains @f.
+ *     Each element there groups a set of callbacks functions.
+ * @f: callback function that will be called if @cond matches.
+ * 	The callback functions are defined in groups, according to
+ *	each element at &struct v4l2_subdev_ops.
  */
 #define v4l2_device_mask_has_op(v4l2_dev, grpmsk, o, f)			\
 ({									\

The can_reduce_fps() is already documented, but it is not using the kernel-doc markup. Convert it, in order to generate documentation from it.

Signed-off-by Mauro Carvalho Chehab <[email protected]>

---
 include/media/v4l2-dv-timings.h | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

(quoted diff) +0 -0

b/include/media/v4l2-dv-timings.h index 61a18893e004..c0855887ad87 100644

include/media/v4l2-dv-timings.h +0 -0

@@ -203,13 +203,15 @@ struct v4l2_fract v4l2_calc_aspect_ratio(u8

hor_landscape, u8 vert_portrait); */
struct v4l2_fract v4l2_dv_timings_aspect_ratio(const struct v4l2_dv_timings
*t);

- * are met -
- * For CVT timings: if reduced blanking v2 (vsync == 8) is true.
- * For CEA861 timings: if V4L2_DV_FL_CAN_REDUCE_FPS flag is true.
+ * are met:
+ *
+ * - For CVT timings: if reduced blanking v2 (vsync == 8) is true.
+ * - For CEA861 timings: if %V4L2_DV_FL_CAN_REDUCE_FPS flag is true.
*/
static inline bool can_reduce_fps(struct v4l2_bt_timings *bt) {

The can_reduce_fps() is already documented, but it is not using the kernel-doc markup. Convert it, in order to generate documentation from it.

Signed-off-by Mauro Carvalho Chehab <[email protected]>

---
 include/media/v4l2-dv-timings.h | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

include/media/v4l2-dv-timings.h +8 -6

index 61a18893e004..c0855887ad87 100644
@@ -203,13 +203,15 @@ struct v4l2_fract v4l2_calc_aspect_ratio(u8 hor_landscape, u8 vert_portrait);
  */
 struct v4l2_fract v4l2_dv_timings_aspect_ratio(const struct v4l2_dv_timings *t);
 
-/*
- * reduce_fps - check if conditions for reduced fps are true.
- * bt - v4l2 timing structure
+/**
+ * can_reduce_fps - check if conditions for reduced fps are true.
+ * @bt: v4l2 timing structure
+ *
  * For different timings reduced fps is allowed if following conditions
- * are met -
- * For CVT timings: if reduced blanking v2 (vsync == 8) is true.
- * For CEA861 timings: if V4L2_DV_FL_CAN_REDUCE_FPS flag is true.
+ * are met:
+ *
+ *   - For CVT timings: if reduced blanking v2 (vsync == 8) is true.
+ *   - For CEA861 timings: if %V4L2_DV_FL_CAN_REDUCE_FPS flag is true.
  */
 static inline  bool can_reduce_fps(struct v4l2_bt_timings *bt)
 {

Both v4l2-event.rst and v4l2-event.h have an overview of events, but there are some inconsistencies there:

- at v4l2-event, the event's ringbuffer is called kevent. Its

name is, instead, v4l2_kevent;

- Some things are mentioned on both places (with different words), others are either on one of the files.

In order to cleanup this mess, put everything at v4l2-event.rst and improve it to be a little more coherent and to have cross references.

Signed-off-by Mauro Carvalho Chehab <[email protected]>

---
 Documentation/media/kapi/v4l2-event.rst | 64 ++++++++++++++++++++++++++-------
 include/media/v4l2-event.h              | 34 ------------------
 2 files changed, 52 insertions(+), 46 deletions(-)

Documentation/media/kapi/v4l2-event.rst +52 -12

index 9938d21ef4d1..7831a503e2aa 100644
@@ -5,27 +5,67 @@ V4L2 events
 The V4L2 events provide a generic way to pass events to user space.
 The driver must use :c:type:`v4l2_fh` to be able to support V4L2 events.
 
-Events are defined by a type and an optional ID. The ID may refer to a V4L2
-object such as a control ID. If unused, then the ID is 0.
+Events are subscribed per-filehandle. An event specification consists of a
+``type`` and is optionally associated with an object identified through the
+``id`` field. If unused, then the ``id`` is 0. So an event is uniquely
+identified by the ``(type, id)`` tuple.
 
-When the user subscribes to an event the driver will allocate a number of
-kevent structs for that event. So every (type, ID) event tuple will have
-its own set of kevent structs. This guarantees that if a driver is generating
-lots of events of one type in a short time, then that will not overwrite
-events of another type.
+The :c:type:`v4l2_fh` struct has a list of subscribed events on its
+``subscribed`` field.
 
-But if you get more events of one type than the number of kevents that were
-reserved, then the oldest event will be dropped and the new one added.
+When the user subscribes to an event, a :c:type:`v4l2_subscribed_event`
+struct is added to :c:type:`v4l2_fh`\ ``.subscribed``, one for every
+subscribed event.
+
+Each :c:type:`v4l2_subscribed_event` struct ends with a
+:c:type:`v4l2_kevent` ringbuffer, with the size given by the caller
+of :c:func:`v4l2_event_subscribe`. Such ringbuffer is used to store any events
+raised by the driver.
+
+So every ``(type, ID)`` event tuple will have its own set of
+:c:type:`v4l2_kevent` ringbuffer. This guarantees that if a driver is
+generating lots of events of one type in a short time, then that will
+not overwrite events of another type.
+
+But if you get more events of one type than the size of the
+:c:type:`v4l2_kevent` ringbuffer, then the oldest event will be dropped
+and the new one added.
+
+The :c:type:`v4l2_kevent` struct links into the ``available``
+list of the :c:type:`v4l2_fh` struct so :ref:`VIDIOC_DQEVENT` will
+know which event to dequeue first.
+
+Finally, if the event subscription is associated with a particular object
+such as a V4L2 control, then that object needs to know about that as well
+so that an event can be raised by that object. So the ``node`` field can
+be used to link the :c:type:`v4l2_subscribed_event` struct into a list of
+such object.
+
+So to summarize:
+
+- struct :c:type:`v4l2_fh` has two lists: one of the ``subscribed`` events,
+  and one of the ``available`` events.
+
+- struct :c:type:`v4l2_subscribed_event` has a ringbuffer of raised
+  (pending) events of that particular type.
+
+- If struct :c:type:`v4l2_subscribed_event` is associated with a specific
+  object, then that object will have an internal list of
+  struct :c:type:`v4l2_subscribed_event` so it knows who subscribed an
+  event to that object.
 
 Furthermore, the internal struct :c:type:`v4l2_subscribed_event` has
 ``merge()`` and ``replace()`` callbacks which drivers can set. These
 callbacks are called when a new event is raised and there is no more room.
+
 The ``replace()`` callback allows you to replace the payload of the old event
 with that of the new event, merging any relevant data from the old payload
 into the new payload that replaces it. It is called when this event type has
-only one kevent struct allocated. The ``merge()`` callback allows you to merge
-the oldest event payload into that of the second-oldest event payload. It is
-called when there are two or more kevent structs allocated.
+only one :c:type:`v4l2_kevent` struct allocated.
+
+The ``merge()`` callback allows you to merge the oldest event payload into
+that of the second-oldest event payload. It is called when there are two
+or more :c:type:`v4l2_kevent` structs allocated.
 
 This way no status information is lost, just the intermediate steps leading
 up to that state.

include/media/v4l2-event.h +0 -34

index 6741910c3a18..4e83529117f7 100644
@@ -24,40 +24,6 @@
 #include <linux/videodev2.h>
 #include <linux/wait.h>
 
-/*
- * Overview:
- *
- * Events are subscribed per-filehandle. An event specification consists of a
- * type and is optionally associated with an object identified through the
- * 'id' field. So an event is uniquely identified by the (type, id) tuple.
- *
- * The v4l2-fh struct has a list of subscribed events. The v4l2_subscribed_event
- * struct is added to that list, one for every subscribed event.
- *
- * Each v4l2_subscribed_event struct ends with an array of v4l2_kevent structs.
- * This array (ringbuffer, really) is used to store any events raised by the
- * driver. The v4l2_kevent struct links into the 'available' list of the
- * v4l2_fh struct so VIDIOC_DQEVENT will know which event to dequeue first.
- *
- * Finally, if the event subscription is associated with a particular object
- * such as a V4L2 control, then that object needs to know about that as well
- * so that an event can be raised by that object. So the 'node' field can
- * be used to link the v4l2_subscribed_event struct into a list of that
- * object.
- *
- * So to summarize:
- *
- * struct v4l2_fh has two lists: one of the subscribed events, and one of the
- * pending events.
- *
- * struct v4l2_subscribed_event has a ringbuffer of raised (pending) events of
- * that particular type.
- *
- * If struct v4l2_subscribed_event is associated with a specific object, then
- * that object will have an internal list of struct v4l2_subscribed_event so
- * it knows who subscribed an event to that object.
- */
-
 struct v4l2_fh;
 struct v4l2_subdev;
 struct v4l2_subscribed_event;

Signature · 1 line

2.13.5

Both v4l2-event.rst and v4l2-event.h have an overview of events, but there are some inconsistencies there:

- at v4l2-event, the event's ringbuffer is called kevent. Its name is, instead, v4l2_kevent;

- Some things are mentioned on both places (with different words), others are either on one of the files.

In order to cleanup this mess, put everything at v4l2-event.rst and improve it to be a little more coherent and to have cross references.

Signed-off-by Mauro Carvalho Chehab <[email protected]>

---
 Documentation/media/kapi/v4l2-event.rst | 64 ++++++++++++++++++++++++++-------
 include/media/v4l2-event.h              | 34 ------------------
 2 files changed, 52 insertions(+), 46 deletions(-)

Documentation/media/kapi/v4l2-event.rst +1 -2

index 9938d21ef4d1..7831a503e2aa 100644
@@ -5,27 +5,67 @@ V4L2 events
 The V4L2 events provide a generic way to pass events to user space.
 The driver must use :c:type:`v4l2_fh` to be able to support V4L2 events.
 
-Events are defined by a type and an optional ID. The ID may refer to a V4L2
-object such as a control ID. If unused, then the ID is 0.
+Events are subscribed per-filehandle. An event specification consists of a

Documentation/media/kapi/v4l2-event.rst +7 -0

+:c:type:`v4l2_kevent` ringbuffer. This guarantees that if a driver is
+generating lots of events of one type in a short time, then that will
+not overwrite events of another type.
+
+But if you get more events of one type than the size of the
+:c:type:`v4l2_kevent` ringbuffer, then the oldest event will be dropped
+and the new one added.

Documentation/media/kapi/v4l2-event.rst +13 -0

+
+So to summarize:
+
+- struct :c:type:`v4l2_fh` has two lists: one of the ``subscribed`` events,
+  and one of the ``available`` events.
+
+- struct :c:type:`v4l2_subscribed_event` has a ringbuffer of raised
+  (pending) events of that particular type.
+
+- If struct :c:type:`v4l2_subscribed_event` is associated with a specific
+  object, then that object will have an internal list of
+  struct :c:type:`v4l2_subscribed_event` so it knows who subscribed an
+  event to that object.
 
 Furthermore, the internal struct :c:type:`v4l2_subscribed_event` has

Documentation/media/kapi/v4l2-event.rst +0 -0

 This way no status information is lost, just the intermediate steps leading
 up to that state.

include/media/v4l2-event.h +0 -27

index 6741910c3a18..4e83529117f7 100644
@@ -24,40 +24,6 @@
 #include <linux/videodev2.h>
 #include <linux/wait.h>
 
-/*
- * Overview:
- *
- * Events are subscribed per-filehandle. An event specification consists of a
- * type and is optionally associated with an object identified through the
- * 'id' field. So an event is uniquely identified by the (type, id) tuple.
- *
- * The v4l2-fh struct has a list of subscribed events. The v4l2_subscribed_event
- * struct is added to that list, one for every subscribed event.
- *
- * Each v4l2_subscribed_event struct ends with an array of v4l2_kevent structs.
- * This array (ringbuffer, really) is used to store any events raised by the
- * driver. The v4l2_kevent struct links into the 'available' list of the
- * v4l2_fh struct so VIDIOC_DQEVENT will know which event to dequeue first.
- *
- * Finally, if the event subscription is associated with a particular object
- * such as a V4L2 control, then that object needs to know about that as well
- * so that an event can be raised by that object. So the 'node' field can
- * be used to link the v4l2_subscribed_event struct into a list of that
- * object.
- *
- * So to summarize:
- *
- * struct v4l2_fh has two lists: one of the subscribed events, and one of the
- * pending events.
- *
- * struct v4l2_subscribed_event has a ringbuffer of raised (pending) events of

Currently, there's no way to document #define foo <value> with kernel-doc. So, convert it to an enum, and document.

Signed-off-by Mauro Carvalho Chehab <[email protected]>

---
 include/media/v4l2-ioctl.h | 33 +++++++++++++++++++--------------
 1 file changed, 19 insertions(+), 14 deletions(-)

include/media/v4l2-ioctl.h +0 -14

index bd5312118013..136e2cffcf9e 100644
@@ -588,20 +588,25 @@ struct v4l2_ioctl_ops {
 };
 
 
-/* v4l debugging and diagnostics */
-
-/* Device debug flags to be used with the video device debug attribute */
-
-/* Just log the ioctl name + error code */
-#define V4L2_DEV_DEBUG_IOCTL		0x01
-/* Log the ioctl name arguments + error code */
-#define V4L2_DEV_DEBUG_IOCTL_ARG	0x02
-/* Log the file operations open, release, mmap and get_unmapped_area */
-#define V4L2_DEV_DEBUG_FOP		0x04
-/* Log the read and write file operations and the VIDIOC_(D)QBUF ioctls */
-#define V4L2_DEV_DEBUG_STREAMING	0x08
-/* Log poll() */
-#define V4L2_DEV_DEBUG_POLL		0x10

include/media/v4l2-ioctl.h +9 -0

+ * @V4L2_DEV_DEBUG_POLL:	Log poll syscalls.
+ */
+enum v4l2_debug_flags {
+	V4L2_DEV_DEBUG_IOCTL		= 0x01,
+	V4L2_DEV_DEBUG_IOCTL_ARG	= 0x02,
+	V4L2_DEV_DEBUG_FOP		= 0x04,
+	V4L2_DEV_DEBUG_STREAMING	= 0x08,
+	V4L2_DEV_DEBUG_POLL		= 0x10,
+};
 
 /*  Video standard functions  */

Hi Mauro,

Thank you for the patch.

Quoted 2 lines · On Thursday, 28 September 2017 00:46:51 EEST Mauro Carvalho Chehab wrote:

Currently, there's no way to document #define foo <value> with kernel-doc. So, convert it to an enum, and document.

The documentation seems fine to me (except for one comment below). However, converting macros to an enum just to work around a defect of the documentation system doesn't seem like a good idea to me. I'd rather find a way to document macros.

Em Fri, 13 Oct 2017 15:38:11 +0300 Laurent Pinchart escreveu:

Quoted 8 lines · On Thursday, 28 September 2017 00:46:51 EEST Mauro Carvalho Chehab wrote:

Quoted 2 lines

Currently, there's no way to document #define foo <value> with kernel-doc. So, convert it to an enum, and document.

The documentation seems fine to me (except for one comment below). However, converting macros to an enum just to work around a defect of the documentation system doesn't seem like a good idea to me. I'd rather find a way to document macros.

I agree that this limitation should be fixed.

Yet, in this specific case where we have an "array" of defines, all associated to the same field (even being a bitmask), and assuming that we would add a way for kernel-doc to parse this kind of defines (not sure how easy/doable would be), then, in order to respect the way kernel-doc markup is, the documentation for those macros would be:

/**
* define: Just log the ioctl name + error code
*/
#define V4L2_DEV_DEBUG_IOCTL 0x01
/**
* define: Log the ioctl name arguments + error code
*/
#define V4L2_DEV_DEBUG_IOCTL_ARG 0x02
/**
* define: Log the file operations open, release, mmap and get_unmapped_area
*/
#define V4L2_DEV_DEBUG_FOP 0x04
/**
* define: Log the read and write file operations and the VIDIOC_(D)QBUF
ioctls */
#define V4L2_DEV_DEBUG_STREAMING 0x08

IMHO, this is a way easier to read/understand by humans, and a way more coincise:

/**
* enum v4l2_debug_flags - Device debug flags to be used with the video
* device debug attribute
*
* @V4L2_DEV_DEBUG_IOCTL: Just log the ioctl name + error code.
* @V4L2_DEV_DEBUG_IOCTL_ARG: Log the ioctl name arguments + error code.
* @V4L2_DEV_DEBUG_FOP: Log the file operations and open, release,
* mmap and get_unmapped_area syscalls.
* @V4L2_DEV_DEBUG_STREAMING: Log the read and write syscalls and
* :c:ref:VIDIOC_[Q|DQ]BUFF <VIDIOC_QBUF> ioctls.
*/

It also underlines the aspect that those names are grouped altogether.

So, IMHO, the main reason to place them inside an enum and document as such is that it looks a way better for humans to read.

Hi Mauro,

Quoted 51 lines · On Monday, 18 December 2017 17:13:26 EET Mauro Carvalho Chehab wrote:

Em Fri, 13 Oct 2017 15:38:11 +0300 Laurent Pinchart escreveu:

Quoted 8 lines · On Thursday, 28 September 2017 00:46:51 EEST Mauro Carvalho Chehab wrote:

Quoted 2 lines

Currently, there's no way to document #define foo <value> with kernel-doc. So, convert it to an enum, and document.

The documentation seems fine to me (except for one comment below). However, converting macros to an enum just to work around a defect of the documentation system doesn't seem like a good idea to me. I'd rather find a way to document macros.

I agree that this limitation should be fixed.

Yet, in this specific case where we have an "array" of defines, all associated to the same field (even being a bitmask), and assuming that we would add a way for kernel-doc to parse this kind of defines (not sure how easy/doable would be), then, in order to respect the way kernel-doc markup is, the documentation for those macros would be:

/**
* define: Just log the ioctl name + error code
*/
#define V4L2_DEV_DEBUG_IOCTL 0x01
/**
* define: Log the ioctl name arguments + error code
*/
#define V4L2_DEV_DEBUG_IOCTL_ARG 0x02
/**
* define: Log the file operations open, release, mmap and get_unmapped_area
*/
#define V4L2_DEV_DEBUG_FOP 0x04
/**
* define: Log the read and write file operations and the VIDIOC_(D)QBUF
ioctls */
#define V4L2_DEV_DEBUG_STREAMING 0x08

IMHO, this is a way easier to read/understand by humans, and a way more coincise:

/**
* enum v4l2_debug_flags - Device debug flags to be used with the video
* device debug attribute
*
* @V4L2_DEV_DEBUG_IOCTL: Just log the ioctl name + error code.
* @V4L2_DEV_DEBUG_IOCTL_ARG: Log the ioctl name arguments + error code.
* @V4L2_DEV_DEBUG_FOP: Log the file operations and open, release,
* mmap and get_unmapped_area syscalls.
* @V4L2_DEV_DEBUG_STREAMING: Log the read and write syscalls and
* :c:ref:VIDIOC_[Q|DQ]BUFF <VIDIOC_QBUF> ioctls.
*/

Quoted 4 lines

It also underlines the aspect that those names are grouped altogether.

So, IMHO, the main reason to place them inside an enum and document as such is that it looks a way better for humans to read.

As we're talking about extending kerneldoc to document macros, we're free to decide on a format that would make it easy and clear. Based on your above example, we could write it

/**
* define v4l2_debug_flags - Device debug flags to be used with the video
* device debug attribute
*
* @V4L2_DEV_DEBUG_IOCTL: Just log the ioctl name + error code.
* @V4L2_DEV_DEBUG_IOCTL_ARG: Log the ioctl name arguments + error code.
* @V4L2_DEV_DEBUG_FOP: Log the file operations and open, release,
* mmap and get_unmapped_area syscalls.
* @V4L2_DEV_DEBUG_STREAMING: Log the read and write syscalls and
* :c:ref:VIDIOC_[Q|DQ]BUFF <VIDIOC_QBUF> ioctls.
*/

That would be simple, clear, and wouldn't require a code change to workaround a limitation of the documentation system.

Currently, there's no way to document #define foo <value> with kernel-doc. So, convert it to an enum, and document.

Signed-off-by Mauro Carvalho Chehab <[email protected]>

---
 include/media/v4l2-ioctl.h | 33 +++++++++++++++++++--------------
 1 file changed, 19 insertions(+), 14 deletions(-)

include/media/v4l2-ioctl.h +19 -14

index bd5312118013..136e2cffcf9e 100644
@@ -588,20 +588,25 @@ struct v4l2_ioctl_ops {
 };
 
 
-/* v4l debugging and diagnostics */
-
-/* Device debug flags to be used with the video device debug attribute */
-
-/* Just log the ioctl name + error code */
-#define V4L2_DEV_DEBUG_IOCTL		0x01
-/* Log the ioctl name arguments + error code */
-#define V4L2_DEV_DEBUG_IOCTL_ARG	0x02
-/* Log the file operations open, release, mmap and get_unmapped_area */
-#define V4L2_DEV_DEBUG_FOP		0x04
-/* Log the read and write file operations and the VIDIOC_(D)QBUF ioctls */
-#define V4L2_DEV_DEBUG_STREAMING	0x08
-/* Log poll() */
-#define V4L2_DEV_DEBUG_POLL		0x10
+/**
+ * enum v4l2_debug_flags - Device debug flags to be used with the video
+ *	device debug attribute
+ *
+ * @V4L2_DEV_DEBUG_IOCTL:	Just log the ioctl name + error code.
+ * @V4L2_DEV_DEBUG_IOCTL_ARG:	Log the ioctl name arguments + error code.
+ * @V4L2_DEV_DEBUG_FOP:		Log the file operations and open, release,
+ *				mmap and get_unmapped_area syscalls.
+ * @V4L2_DEV_DEBUG_STREAMING:	Log the read and write syscalls and
+ *				:c:ref:`VIDIOC_[Q|DQ]BUFF <VIDIOC_QBUF>` ioctls.
+ * @V4L2_DEV_DEBUG_POLL:	Log poll syscalls.
+ */
+enum v4l2_debug_flags {
+	V4L2_DEV_DEBUG_IOCTL		= 0x01,
+	V4L2_DEV_DEBUG_IOCTL_ARG	= 0x02,
+	V4L2_DEV_DEBUG_FOP		= 0x04,
+	V4L2_DEV_DEBUG_STREAMING	= 0x08,
+	V4L2_DEV_DEBUG_POLL		= 0x10,
+};
 
 /*  Video standard functions  */
 

This enum is already documented, but it is not using a kernel-doc format. Convert its format, in order to produce documentation.

Signed-off-by Mauro Carvalho Chehab <[email protected]>

---
 include/media/cec-pin.h | 81 +++++++++++++++++++++++++++++++------------------
 1 file changed, 52 insertions(+), 29 deletions(-)

include/media/cec-pin.h +52 -29

index f09cc9579d53..fbe8c256820e 100644
@@ -24,65 +24,88 @@
 #include <linux/atomic.h>
 #include <media/cec.h>
 
+/**
+ * enum cec_pin_state - state of CEC pins
+ * @CEC_ST_OFF:
+ *	CEC is off
+ * @CEC_ST_IDLE:
+ *	CEC is idle, waiting for Rx or Tx
+ * @CEC_ST_TX_WAIT:
+ *	Pending Tx, waiting for Signal Free Time to expire
+ * @CEC_ST_TX_WAIT_FOR_HIGH:
+ *	Low-drive was detected, wait for bus to go high
+ * @CEC_ST_TX_START_BIT_LOW:
+ *	Drive CEC low for the start bit
+ * @CEC_ST_TX_START_BIT_HIGH:
+ *	Drive CEC high for the start bit
+ * @CEC_ST_TX_DATA_BIT_0_LOW:
+ *	Drive CEC low for the 0 bit
+ * @CEC_ST_TX_DATA_BIT_0_HIGH:
+ *	Drive CEC high for the 0 bit
+ * @CEC_ST_TX_DATA_BIT_1_LOW:
+ *	Drive CEC low for the 1 bit
+ * @CEC_ST_TX_DATA_BIT_1_HIGH:
+ *	Drive CEC high for the 1 bit
+ * @CEC_ST_TX_DATA_BIT_1_HIGH_PRE_SAMPLE:
+ *	Wait for start of sample time to check for Ack bit or first
+ *	four initiator bits to check for Arbitration Lost.
+ * @CEC_ST_TX_DATA_BIT_1_HIGH_POST_SAMPLE:
+ *	Wait for end of bit period after sampling
+ * @CEC_ST_RX_START_BIT_LOW:
+ *	Start bit low detected
+ * @CEC_ST_RX_START_BIT_HIGH:
+ *	Start bit high detected
+ * @CEC_ST_RX_DATA_SAMPLE:
+ *	Wait for bit sample time
+ * @CEC_ST_RX_DATA_POST_SAMPLE:
+ *	Wait for earliest end of bit period after sampling
+ * @CEC_ST_RX_DATA_HIGH:
+ *	Wait for CEC to go high (i.e. end of bit period
+ * @CEC_ST_RX_ACK_LOW:
+ *	Drive CEC low to send 0 Ack bit
+ * @CEC_ST_RX_ACK_LOW_POST:
+ *	End of 0 Ack time, wait for earliest end of bit period
+ * @CEC_ST_RX_ACK_HIGH_POST:
+ *	Wait for CEC to go high (i.e. end of bit period
+ * @CEC_ST_RX_ACK_FINISH:
+ *	Wait for earliest end of bit period and end of message
+ * @CEC_ST_LOW_DRIVE:
+ *	Start low drive
+ * @CEC_ST_RX_IRQ:
+ *	Monitor pin using interrupts
+ * @CEC_PIN_STATES:
+ *	Total number of pin states
+ */
 enum cec_pin_state {
-	/* CEC is off */
 	CEC_ST_OFF,
-	/* CEC is idle, waiting for Rx or Tx */
 	CEC_ST_IDLE,
 
 	/* Tx states */
-
-	/* Pending Tx, waiting for Signal Free Time to expire */
 	CEC_ST_TX_WAIT,
-	/* Low-drive was detected, wait for bus to go high */
 	CEC_ST_TX_WAIT_FOR_HIGH,
-	/* Drive CEC low for the start bit */
 	CEC_ST_TX_START_BIT_LOW,
-	/* Drive CEC high for the start bit */
 	CEC_ST_TX_START_BIT_HIGH,
-	/* Drive CEC low for the 0 bit */
 	CEC_ST_TX_DATA_BIT_0_LOW,
-	/* Drive CEC high for the 0 bit */
 	CEC_ST_TX_DATA_BIT_0_HIGH,
-	/* Drive CEC low for the 1 bit */
 	CEC_ST_TX_DATA_BIT_1_LOW,
-	/* Drive CEC high for the 1 bit */
 	CEC_ST_TX_DATA_BIT_1_HIGH,
-	/*
-	 * Wait for start of sample time to check for Ack bit or first
-	 * four initiator bits to check for Arbitration Lost.
-	 */
 	CEC_ST_TX_DATA_BIT_1_HIGH_PRE_SAMPLE,
-	/* Wait for end of bit period after sampling */
 	CEC_ST_TX_DATA_BIT_1_HIGH_POST_SAMPLE,
 
 	/* Rx states */
-
-	/* Start bit low detected */
 	CEC_ST_RX_START_BIT_LOW,
-	/* Start bit high detected */
 	CEC_ST_RX_START_BIT_HIGH,
-	/* Wait for bit sample time */
 	CEC_ST_RX_DATA_SAMPLE,
-	/* Wait for earliest end of bit period after sampling */
 	CEC_ST_RX_DATA_POST_SAMPLE,
-	/* Wait for CEC to go high (i.e. end of bit period */
 	CEC_ST_RX_DATA_HIGH,
-	/* Drive CEC low to send 0 Ack bit */
 	CEC_ST_RX_ACK_LOW,
-	/* End of 0 Ack time, wait for earliest end of bit period */
 	CEC_ST_RX_ACK_LOW_POST,
-	/* Wait for CEC to go high (i.e. end of bit period */
 	CEC_ST_RX_ACK_HIGH_POST,
-	/* Wait for earliest end of bit period and end of message */
 	CEC_ST_RX_ACK_FINISH,
 
-	/* Start low drive */
 	CEC_ST_LOW_DRIVE,
-	/* Monitor pin using interrupts */
 	CEC_ST_RX_IRQ,
 
-	/* Total number of pin states */
 	CEC_PIN_STATES
 };
 

Quoted 4 lines

This enum is already documented, but it is not using a kernel-doc format. Convert its format, in order to produce documentation.

Signed-off-by Mauro Carvalho Chehab <[email protected]>

No, this does not belong in the kernel API doc. It is really just an internal API.

The only things that belong in this header are the two function prototypes and struct cec-pin_ops. Everything else should be moved to a cec-pin-priv.h header inside drivers/media/cec.

If it is OK with you, then I'll take care of that.

Now that kernel-doc handles nested unions, better document the bus union at struct v4l2_fwnode_endpoint.

Signed-off-by Mauro Carvalho Chehab <[email protected]>

---
 include/media/v4l2-fwnode.h | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

include/media/v4l2-fwnode.h +0 -0

index 7adec9851d9e..5f4716f967d0 100644