From: Hans Verkuil <hverkuil@xs4all.nl>
To: Pawel Osciak <pawel@osciak.com>
Cc: LMML <linux-media@vger.kernel.org>,
Sylwester Nawrocki <s.nawrocki@samsung.com>,
Sakari Ailus <sakari.ailus@iki.fi>,
Marek Szyprowski <m.szyprowski@samsung.com>,
Hans Verkuil <hans.verkuil@cisco.com>
Subject: Re: [REVIEW PATCH 04/11] vb2: use correct prefix
Date: Mon, 07 Apr 2014 09:43:30 +0200 [thread overview]
Message-ID: <53425722.8090902@xs4all.nl> (raw)
In-Reply-To: <CAMm-=zANa3wF5kd2dBZMWtdr4rMGRtEizwJsY9nUrQZPkuHQrg@mail.gmail.com>
On 04/07/2014 09:30 AM, Pawel Osciak wrote:
> The idea behind not using __func__ was that it was much more
> informative when debugging to see a "reqbufs" prefix instead of, for
> example "__verify_memory_type". But since some of the functions are
> shared across multiple ioctl impls now (e.g. __verify_memory_type is
> used by both reqbufs and createbufs), as much as I would prefer to
> keep this convention, it'd probably be too much to maintain.
>
> But if we want to do this, we should move "__func__" to dprintk()
> definition, instead of adding it to the call sites.
Good idea. I'll do that.
Regards,
Hans
>
> On Tue, Mar 11, 2014 at 6:20 AM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
>> From: Hans Verkuil <hans.verkuil@cisco.com>
>>
>> Many dprintk's in vb2 use a hardcoded prefix with the function name. In
>> many cases that is now outdated. Replace prefixes by the function name using
>> __func__. At least now I know if I see a 'qbuf:' prefix whether that refers
>> to the mmap, userptr or dmabuf variant.
>>
>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>> ---
>> drivers/media/v4l2-core/videobuf2-core.c | 102 ++++++++++++++++---------------
>> 1 file changed, 54 insertions(+), 48 deletions(-)
>>
>> diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
>> index 83e78e9..71be247 100644
>> --- a/drivers/media/v4l2-core/videobuf2-core.c
>> +++ b/drivers/media/v4l2-core/videobuf2-core.c
>> @@ -371,7 +371,8 @@ static int __vb2_queue_free(struct vb2_queue *q, unsigned int buffers)
>> if (q->bufs[buffer] == NULL)
>> continue;
>> if (q->bufs[buffer]->state == VB2_BUF_STATE_PREPARING) {
>> - dprintk(1, "reqbufs: preparing buffers, cannot free\n");
>> + dprintk(1, "%s: preparing buffers, cannot free\n",
>> + __func__);
>> return -EAGAIN;
>> }
>> }
>> @@ -656,12 +657,12 @@ int vb2_querybuf(struct vb2_queue *q, struct v4l2_buffer *b)
>> int ret;
>>
>> if (b->type != q->type) {
>> - dprintk(1, "querybuf: wrong buffer type\n");
>> + dprintk(1, "%s: wrong buffer type\n", __func__);
>> return -EINVAL;
>> }
>>
>> if (b->index >= q->num_buffers) {
>> - dprintk(1, "querybuf: buffer index out of range\n");
>> + dprintk(1, "%s: buffer index out of range\n", __func__);
>> return -EINVAL;
>> }
>> vb = q->bufs[b->index];
>> @@ -721,12 +722,12 @@ static int __verify_memory_type(struct vb2_queue *q,
>> {
>> if (memory != V4L2_MEMORY_MMAP && memory != V4L2_MEMORY_USERPTR &&
>> memory != V4L2_MEMORY_DMABUF) {
>> - dprintk(1, "reqbufs: unsupported memory type\n");
>> + dprintk(1, "%s: unsupported memory type\n", __func__);
>> return -EINVAL;
>> }
>>
>> if (type != q->type) {
>> - dprintk(1, "reqbufs: requested type is incorrect\n");
>> + dprintk(1, "%s: requested type is incorrect\n", __func__);
>> return -EINVAL;
>> }
>>
>> @@ -735,17 +736,20 @@ static int __verify_memory_type(struct vb2_queue *q,
>> * are available.
>> */
>> if (memory == V4L2_MEMORY_MMAP && __verify_mmap_ops(q)) {
>> - dprintk(1, "reqbufs: MMAP for current setup unsupported\n");
>> + dprintk(1, "%s: MMAP for current setup unsupported\n",
>> + __func__);
>> return -EINVAL;
>> }
>>
>> if (memory == V4L2_MEMORY_USERPTR && __verify_userptr_ops(q)) {
>> - dprintk(1, "reqbufs: USERPTR for current setup unsupported\n");
>> + dprintk(1, "%s: USERPTR for current setup unsupported\n",
>> + __func__);
>> return -EINVAL;
>> }
>>
>> if (memory == V4L2_MEMORY_DMABUF && __verify_dmabuf_ops(q)) {
>> - dprintk(1, "reqbufs: DMABUF for current setup unsupported\n");
>> + dprintk(1, "%s: DMABUF for current setup unsupported\n",
>> + __func__);
>> return -EINVAL;
>> }
>>
>> @@ -755,7 +759,7 @@ static int __verify_memory_type(struct vb2_queue *q,
>> * do the memory and type validation.
>> */
>> if (q->fileio) {
>> - dprintk(1, "reqbufs: file io in progress\n");
>> + dprintk(1, "%s: file io in progress\n", __func__);
>> return -EBUSY;
>> }
>> return 0;
>> @@ -790,7 +794,7 @@ static int __reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req)
>> int ret;
>>
>> if (q->streaming) {
>> - dprintk(1, "reqbufs: streaming active\n");
>> + dprintk(1, "%s: streaming active\n", __func__);
>> return -EBUSY;
>> }
>>
>> @@ -800,7 +804,7 @@ static int __reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req)
>> * are not in use and can be freed.
>> */
>> if (q->memory == V4L2_MEMORY_MMAP && __buffers_in_use(q)) {
>> - dprintk(1, "reqbufs: memory in use, cannot free\n");
>> + dprintk(1, "%s: memory in use, cannot free\n", __func__);
>> return -EBUSY;
>> }
>>
>> @@ -1272,15 +1276,14 @@ static int __qbuf_userptr(struct vb2_buffer *vb, const struct v4l2_buffer *b)
>> && vb->v4l2_planes[plane].length == planes[plane].length)
>> continue;
>>
>> - dprintk(3, "qbuf: userspace address for plane %d changed, "
>> - "reacquiring memory\n", plane);
>> + dprintk(3, "%s: userspace address for plane %d changed, reacquiring memory\n",
>> + __func__, plane);
>>
>> /* Check if the provided plane buffer is large enough */
>> if (planes[plane].length < q->plane_sizes[plane]) {
>> - dprintk(1, "qbuf: provided buffer size %u is less than "
>> - "setup size %u for plane %d\n",
>> - planes[plane].length,
>> - q->plane_sizes[plane], plane);
>> + dprintk(1, "%s: provided buffer size %u is less than setup size %u for plane %d\n",
>> + __func__, planes[plane].length,
>> + q->plane_sizes[plane], plane);
>> ret = -EINVAL;
>> goto err;
>> }
>> @@ -1302,8 +1305,8 @@ static int __qbuf_userptr(struct vb2_buffer *vb, const struct v4l2_buffer *b)
>> planes[plane].m.userptr,
>> planes[plane].length, write);
>> if (IS_ERR_OR_NULL(mem_priv)) {
>> - dprintk(1, "qbuf: failed acquiring userspace "
>> - "memory for plane %d\n", plane);
>> + dprintk(1, "%s: failed acquiring userspace memory for plane %d\n",
>> + __func__, plane);
>> fail_memop(vb, get_userptr);
>> ret = mem_priv ? PTR_ERR(mem_priv) : -EINVAL;
>> goto err;
>> @@ -1326,7 +1329,8 @@ static int __qbuf_userptr(struct vb2_buffer *vb, const struct v4l2_buffer *b)
>> */
>> ret = call_vb_qop(vb, buf_init, vb);
>> if (ret) {
>> - dprintk(1, "qbuf: buffer initialization failed\n");
>> + dprintk(1, "%s: buffer initialization failed\n",
>> + __func__);
>> fail_vb_qop(vb, buf_init);
>> goto err;
>> }
>> @@ -1334,7 +1338,7 @@ static int __qbuf_userptr(struct vb2_buffer *vb, const struct v4l2_buffer *b)
>>
>> ret = call_vb_qop(vb, buf_prepare, vb);
>> if (ret) {
>> - dprintk(1, "qbuf: buffer preparation failed\n");
>> + dprintk(1, "%s: buffer preparation failed\n", __func__);
>> fail_vb_qop(vb, buf_prepare);
>> call_vb_qop(vb, buf_cleanup, vb);
>> goto err;
>> @@ -1388,8 +1392,8 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const struct v4l2_buffer *b)
>> struct dma_buf *dbuf = dma_buf_get(planes[plane].m.fd);
>>
>> if (IS_ERR_OR_NULL(dbuf)) {
>> - dprintk(1, "qbuf: invalid dmabuf fd for plane %d\n",
>> - plane);
>> + dprintk(1, "%s: invalid dmabuf fd for plane %d\n",
>> + __func__, plane);
>> ret = -EINVAL;
>> goto err;
>> }
>> @@ -1399,8 +1403,8 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const struct v4l2_buffer *b)
>> planes[plane].length = dbuf->size;
>>
>> if (planes[plane].length < q->plane_sizes[plane]) {
>> - dprintk(1, "qbuf: invalid dmabuf length for plane %d\n",
>> - plane);
>> + dprintk(1, "%s: invalid dmabuf length for plane %d\n",
>> + __func__, plane);
>> ret = -EINVAL;
>> goto err;
>> }
>> @@ -1412,7 +1416,8 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const struct v4l2_buffer *b)
>> continue;
>> }
>>
>> - dprintk(1, "qbuf: buffer for plane %d changed\n", plane);
>> + dprintk(1, "%s: buffer for plane %d changed\n",
>> + __func__, plane);
>>
>> if (!reacquired) {
>> reacquired = true;
>> @@ -1427,7 +1432,7 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const struct v4l2_buffer *b)
>> mem_priv = call_memop(vb, attach_dmabuf, q->alloc_ctx[plane],
>> dbuf, planes[plane].length, write);
>> if (IS_ERR(mem_priv)) {
>> - dprintk(1, "qbuf: failed to attach dmabuf\n");
>> + dprintk(1, "%s: failed to attach dmabuf\n", __func__);
>> fail_memop(vb, attach_dmabuf);
>> ret = PTR_ERR(mem_priv);
>> dma_buf_put(dbuf);
>> @@ -1445,8 +1450,8 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const struct v4l2_buffer *b)
>> for (plane = 0; plane < vb->num_planes; ++plane) {
>> ret = call_memop(vb, map_dmabuf, vb->planes[plane].mem_priv);
>> if (ret) {
>> - dprintk(1, "qbuf: failed to map dmabuf for plane %d\n",
>> - plane);
>> + dprintk(1, "%s: failed to map dmabuf for plane %d\n",
>> + __func__, plane);
>> fail_memop(vb, map_dmabuf);
>> goto err;
>> }
>> @@ -1467,7 +1472,8 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const struct v4l2_buffer *b)
>> */
>> ret = call_vb_qop(vb, buf_init, vb);
>> if (ret) {
>> - dprintk(1, "qbuf: buffer initialization failed\n");
>> + dprintk(1, "%s: buffer initialization failed\n",
>> + __func__);
>> fail_vb_qop(vb, buf_init);
>> goto err;
>> }
>> @@ -1475,7 +1481,7 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const struct v4l2_buffer *b)
>>
>> ret = call_vb_qop(vb, buf_prepare, vb);
>> if (ret) {
>> - dprintk(1, "qbuf: buffer preparation failed\n");
>> + dprintk(1, "%s: buffer preparation failed\n", __func__);
>> fail_vb_qop(vb, buf_prepare);
>> call_vb_qop(vb, buf_cleanup, vb);
>> goto err;
>> @@ -1671,7 +1677,7 @@ static int vb2_start_streaming(struct vb2_queue *q)
>> return 0;
>>
>> fail_qop(q, start_streaming);
>> - dprintk(1, "qbuf: driver refused to start streaming\n");
>> + dprintk(1, "%s: driver refused to start streaming\n", __func__);
>> if (WARN_ON(atomic_read(&q->owned_by_drv_count))) {
>> unsigned i;
>>
>> @@ -1709,7 +1715,7 @@ static int vb2_internal_qbuf(struct vb2_queue *q, struct v4l2_buffer *b)
>> case VB2_BUF_STATE_PREPARED:
>> break;
>> case VB2_BUF_STATE_PREPARING:
>> - dprintk(1, "qbuf: buffer still being prepared\n");
>> + dprintk(1, "%s: buffer still being prepared\n", __func__);
>> return -EINVAL;
>> default:
>> dprintk(1, "%s(): invalid buffer state %d\n", __func__,
>> @@ -1945,7 +1951,7 @@ static int vb2_internal_dqbuf(struct vb2_queue *q, struct v4l2_buffer *b, bool n
>> int ret;
>>
>> if (b->type != q->type) {
>> - dprintk(1, "dqbuf: invalid buffer type\n");
>> + dprintk(1, "%s: invalid buffer type\n", __func__);
>> return -EINVAL;
>> }
>> ret = __vb2_get_done_vb(q, &vb, b, nonblocking);
>> @@ -1954,13 +1960,13 @@ static int vb2_internal_dqbuf(struct vb2_queue *q, struct v4l2_buffer *b, bool n
>>
>> switch (vb->state) {
>> case VB2_BUF_STATE_DONE:
>> - dprintk(3, "dqbuf: Returning done buffer\n");
>> + dprintk(3, "%s: Returning done buffer\n", __func__);
>> break;
>> case VB2_BUF_STATE_ERROR:
>> - dprintk(3, "dqbuf: Returning done buffer with errors\n");
>> + dprintk(3, "%s: Returning done buffer with errors\n", __func__);
>> break;
>> default:
>> - dprintk(1, "dqbuf: Invalid buffer state\n");
>> + dprintk(1, "%s: Invalid buffer state\n", __func__);
>> return -EINVAL;
>> }
>>
>> @@ -2004,7 +2010,7 @@ static int vb2_internal_dqbuf(struct vb2_queue *q, struct v4l2_buffer *b, bool n
>> int vb2_dqbuf(struct vb2_queue *q, struct v4l2_buffer *b, bool nonblocking)
>> {
>> if (q->fileio) {
>> - dprintk(1, "dqbuf: file io in progress\n");
>> + dprintk(1, "%s: file io in progress\n", __func__);
>> return -EBUSY;
>> }
>> return vb2_internal_dqbuf(q, b, nonblocking);
>> @@ -2076,7 +2082,7 @@ static int vb2_internal_streamon(struct vb2_queue *q, enum v4l2_buf_type type)
>> int ret;
>>
>> if (type != q->type) {
>> - dprintk(1, "streamon: invalid stream type\n");
>> + dprintk(1, "%s: invalid stream type\n", __func__);
>> return -EINVAL;
>> }
>>
>> @@ -2086,17 +2092,17 @@ static int vb2_internal_streamon(struct vb2_queue *q, enum v4l2_buf_type type)
>> }
>>
>> if (!q->num_buffers) {
>> - dprintk(1, "streamon: no buffers have been allocated\n");
>> + dprintk(1, "%s: no buffers have been allocated\n", __func__);
>> return -EINVAL;
>> }
>>
>> if (!q->num_buffers) {
>> - dprintk(1, "streamon: no buffers have been allocated\n");
>> + dprintk(1, "%s: no buffers have been allocated\n", __func__);
>> return -EINVAL;
>> }
>> if (q->num_buffers < q->min_buffers_needed) {
>> - dprintk(1, "streamon: need at least %u allocated buffers\n",
>> - q->min_buffers_needed);
>> + dprintk(1, "%s: need at least %u allocated buffers\n",
>> + __func__, q->min_buffers_needed);
>> return -EINVAL;
>> }
>>
>> @@ -2134,7 +2140,7 @@ static int vb2_internal_streamon(struct vb2_queue *q, enum v4l2_buf_type type)
>> int vb2_streamon(struct vb2_queue *q, enum v4l2_buf_type type)
>> {
>> if (q->fileio) {
>> - dprintk(1, "streamon: file io in progress\n");
>> + dprintk(1, "%s: file io in progress\n", __func__);
>> return -EBUSY;
>> }
>> return vb2_internal_streamon(q, type);
>> @@ -2144,7 +2150,7 @@ EXPORT_SYMBOL_GPL(vb2_streamon);
>> static int vb2_internal_streamoff(struct vb2_queue *q, enum v4l2_buf_type type)
>> {
>> if (type != q->type) {
>> - dprintk(1, "streamoff: invalid stream type\n");
>> + dprintk(1, "%s: invalid stream type\n", __func__);
>> return -EINVAL;
>> }
>>
>> @@ -2181,7 +2187,7 @@ static int vb2_internal_streamoff(struct vb2_queue *q, enum v4l2_buf_type type)
>> int vb2_streamoff(struct vb2_queue *q, enum v4l2_buf_type type)
>> {
>> if (q->fileio) {
>> - dprintk(1, "streamoff: file io in progress\n");
>> + dprintk(1, "%s: file io in progress\n", __func__);
>> return -EBUSY;
>> }
>> return vb2_internal_streamoff(q, type);
>> @@ -2249,7 +2255,7 @@ int vb2_expbuf(struct vb2_queue *q, struct v4l2_exportbuffer *eb)
>> }
>>
>> if (eb->type != q->type) {
>> - dprintk(1, "qbuf: invalid buffer type\n");
>> + dprintk(1, "%s: invalid buffer type\n", __func__);
>> return -EINVAL;
>> }
>>
>> @@ -2863,7 +2869,7 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q, char __user *data, size_
>> fileio->b.index = index;
>> fileio->b.bytesused = buf->pos;
>> ret = vb2_internal_qbuf(q, &fileio->b);
>> - dprintk(5, "file io: vb2_dbuf result: %d\n", ret);
>> + dprintk(5, "file io: vb2_internal_qbuf result: %d\n", ret);
>> if (ret)
>> return ret;
>>
>> --
>> 1.9.0
>>
>
>
>
next prev parent reply other threads:[~2014-04-07 7:44 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-03-10 21:20 vb2: various small fixes/improvements Hans Verkuil
2014-03-10 21:20 ` [REVIEW PATCH 01/11] vb2: stop_streaming should return void Hans Verkuil
2014-04-07 4:58 ` Pawel Osciak
2014-03-10 21:20 ` [REVIEW PATCH 02/11] vb2: fix handling of data_offset and v4l2_plane.reserved[] Hans Verkuil
2014-04-07 5:11 ` Pawel Osciak
2014-04-07 11:47 ` Hans Verkuil
2014-03-10 21:20 ` [REVIEW PATCH 03/11] vb2: if bytesused is 0, then fill with output buffer length Hans Verkuil
2014-04-07 7:20 ` Pawel Osciak
2014-04-07 7:39 ` Hans Verkuil
2014-04-07 7:46 ` Pawel Osciak
2014-03-10 21:20 ` [REVIEW PATCH 04/11] vb2: use correct prefix Hans Verkuil
2014-04-07 7:30 ` Pawel Osciak
2014-04-07 7:43 ` Hans Verkuil [this message]
2014-03-10 21:20 ` [REVIEW PATCH 05/11] vb2: move __qbuf_mmap before __qbuf_userptr Hans Verkuil
2014-04-07 8:09 ` Pawel Osciak
2014-03-10 21:20 ` [REVIEW PATCH 06/11] vb2: set timestamp when using write() Hans Verkuil
2014-04-07 8:32 ` Pawel Osciak
2014-04-07 9:02 ` Hans Verkuil
2014-03-10 21:20 ` [REVIEW PATCH 07/11] vb2: reject output buffers with V4L2_FIELD_ALTERNATE Hans Verkuil
2014-04-07 8:38 ` Pawel Osciak
2014-03-10 21:20 ` [REVIEW PATCH 08/11] vb2: simplify a confusing condition Hans Verkuil
2014-04-07 8:42 ` Pawel Osciak
2014-03-10 21:20 ` [REVIEW PATCH 09/11] vb2: add vb2_fileio_is_active and check it more often Hans Verkuil
2014-03-10 21:20 ` [REVIEW PATCH 10/11] vb2: set v4l2_buffer.bytesused to 0 for mp buffers Hans Verkuil
2014-04-09 17:21 ` Sakari Ailus
2014-04-11 7:42 ` Hans Verkuil
2014-03-10 21:20 ` [REVIEW PATCH 11/11] vb2: allow read/write as long as the format is single planar Hans Verkuil
2014-04-04 10:01 ` vb2: various small fixes/improvements Hans Verkuil
2014-04-09 17:27 ` Sakari Ailus
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=53425722.8090902@xs4all.nl \
--to=hverkuil@xs4all.nl \
--cc=hans.verkuil@cisco.com \
--cc=linux-media@vger.kernel.org \
--cc=m.szyprowski@samsung.com \
--cc=pawel@osciak.com \
--cc=s.nawrocki@samsung.com \
--cc=sakari.ailus@iki.fi \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.