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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox