Hi Mauro, Thank you for the patch. On Thursday, 28 September 2017 00:46:47 EEST Mauro Carvalho Chehab wrote: > There are several ancillary functions that aren't documented. > > Document them. > > Signed-off-by: Mauro Carvalho Chehab <mchehab@xxxxxxxxxxxxxxxx> > --- > drivers/media/v4l2-core/v4l2-common.c | 14 ----- > include/media/v4l2-common.h | 104 +++++++++++++++++++++++++++---- > 2 files changed, 94 insertions(+), 24 deletions(-) > > diff --git a/drivers/media/v4l2-core/v4l2-common.c > b/drivers/media/v4l2-core/v4l2-common.c index fb9a2a3c1072..ac404d5083e0 > 100644 > --- a/drivers/media/v4l2-core/v4l2-common.c > +++ b/drivers/media/v4l2-core/v4l2-common.c > @@ -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. > - */ I still think documentation would be much better placed in C files than header files. Maybe we could quickly discuss this during the media summit at ELCE. > void v4l_bound_align_image(u32 *w, unsigned int wmin, unsigned int wmax, > unsigned int walign, > u32 *h, unsigned int hmin, unsigned int hmax, > diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h > index 7ae7840df068..d3f5f907d566 100644 > --- a/include/media/v4l2-common.h > +++ b/include/media/v4l2-common.h > @@ -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 s/tuner type/tuner/ > + * &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. s/a given criteria/the given constraints/ > + * > + * @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. s/Bound/Bind/ ? I think "Clip" would be better actually. > + * > + * 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. It's not the image that will be shrunk or enlarged, but the clipping rectangle. > + * #. @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, > - 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. I'd say "length" instead of "size", or "number of elements in the @sizes array". Size could be interpreted as the array size in bytes. > + * @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. This function isn't restricted to be used to handle userspace sizes, I wouldn't mention userspace here. > + */ > const struct v4l2_frmsize_discrete > *v4l2_find_nearest_format(const struct v4l2_frmsize_discrete *sizes, > 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. s/with/which/ > + * > + * @tv: pointer to &stuct timeval to be filled. > + */ > void v4l2_get_timestamp(struct timeval *tv); > > #endif /* V4L2_COMMON_H_ */ -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html