diff for duplicates of <5004544.CpPfGJfHMn@avalon> diff --git a/a/1.txt b/N1/1.txt index 0e49953..3ed72b6 100644 --- a/a/1.txt +++ b/N1/1.txt @@ -408,9 +408,9 @@ What's wrong with a void * for a data pointer ? [snip] > +/* -> + * ==================================== +> + * ======================================================================== > + * video ioctl operations -> + * ==================================== +> + * ======================================================================== > + */ > +static void put_qtbl(u8 *p, const unsigned int *qtbl) > +{ @@ -463,11 +463,11 @@ generating them in big endian directly and then using a simple memcpy() ? > + int byte; > + > + byte = get_byte(buf); -> + if (byte = -1) +> + if (byte == -1) > + return -1; > + temp = byte << 8; > + byte = get_byte(buf); -> + if (byte = -1) +> + if (byte == -1) > + return -1; > + *word = (unsigned int)byte | temp; > + return 0; @@ -519,16 +519,16 @@ You can easily optimize that: > + while (!found) { > + int c = get_byte(&jpeg_buffer); > + -> + if (c = -1) +> + if (c == -1) > + return false; > + if (c != 0xff) > + continue; > + do > + c = get_byte(&jpeg_buffer); -> + while (c = 0xff); -> + if (c = -1) +> + while (c == 0xff); +> + if (c == -1) > + return false; -> + if (c = 0) +> + if (c == 0) > + continue; > + length = 0; > + switch (c) { @@ -536,18 +536,18 @@ You can easily optimize that: > + case SOF0: > + if (get_word_be(&jpeg_buffer, &word)) > + break; -> + if (get_byte(&jpeg_buffer) = -1) +> + if (get_byte(&jpeg_buffer) == -1) > + break; > + if (get_word_be(&jpeg_buffer, height)) > + break; > + if (get_word_be(&jpeg_buffer, width)) > + break; > + components = get_byte(&jpeg_buffer); -> + if (components = -1) +> + if (components == -1) > + break; > + found = true; > + -> + if (components = 1) { +> + if (components == 1) { > + subsampling = 0x33; > + } else { > + skip(&jpeg_buffer, 1); @@ -614,10 +614,10 @@ dev_dbg() would be more appropriate. Same comment for all the pr_* calls below. > + if (ctx->encoder) -> + pixelformat = (f_type = JPU_FMT_TYPE_OUTPUT) ? +> + pixelformat = (f_type == JPU_FMT_TYPE_OUTPUT) ? > + V4L2_PIX_FMT_NV16M : V4L2_PIX_FMT_JPEG; > + else -> + pixelformat = (f_type = JPU_FMT_TYPE_CAPTURE) ? +> + pixelformat = (f_type == JPU_FMT_TYPE_CAPTURE) ? > + V4L2_PIX_FMT_NV16M : V4L2_PIX_FMT_JPEG; > + fmt = jpu_find_format(ctx->encoder, pixelformat, f_type); > + } @@ -635,7 +635,7 @@ below. > + w = pix->width; > + h = pix->height; > + -> + if (fmt->fourcc = V4L2_PIX_FMT_JPEG) { +> + if (fmt->fourcc == V4L2_PIX_FMT_JPEG) { > + if (pix->plane_fmt[0].sizeimage <= 0) > + pix->plane_fmt[0].sizeimage = JPU_JPEG_HDR_SIZE + > + (JPU_JPEG_MAX_BITS_PER_PIXEL * w * h); @@ -655,8 +655,10 @@ below. > + w / hsub * fmt->bpp[i] / 8, > + JPU_WIDTH_MAX); > + -> + pix->plane_fmt[i].bytesperline > + round_up(bpl, JPU_MEMALIGN); -> + pix->plane_fmt[i].sizeimage > + pix->plane_fmt[i].bytesperline * h / vsub; +> + pix->plane_fmt[i].bytesperline = +> + round_up(bpl, JPU_MEMALIGN); +> + pix->plane_fmt[i].sizeimage = +> + pix->plane_fmt[i].bytesperline * h / vsub; > + memset(pix->plane_fmt[i].reserved, 0, > + sizeof(pix->plane_fmt[i].reserved)); > + } @@ -671,9 +673,9 @@ below. [snip] > +/* -> + * ==================================== +> + * ======================================================================== > + * Queue operations -> + * --=================================== +> + * --====================================================================== > + */ > +static int jpu_queue_setup(struct vb2_queue *vq, > + const struct v4l2_format *fmt, @@ -780,9 +782,9 @@ though, as the payload value will be overwritten later. [snip] > +/* -> + * ==================================== +> + * ======================================================================== > + * Device file operations -> + * ==================================== +> + * ======================================================================== > + */ > +static int jpu_open(struct file *file) > +{ @@ -808,7 +810,7 @@ Does all the code below reallly need to be protected by the mutex ? > + v4l2_fh_add(&ctx->fh); > + > + ctx->jpu = jpu; -> + ctx->encoder = (vfd = jpu->vfd_encoder); +> + ctx->encoder = (vfd == jpu->vfd_encoder); > + > + __jpu_try_fmt(ctx, &ctx->out_q.format, > + V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE, &ctx->out_q.fmtinfo); @@ -825,7 +827,7 @@ Does all the code below reallly need to be protected by the mutex ? > + if (ret < 0) > + goto error; > + -> + if (jpu->ref_count = 0) { +> + if (jpu->ref_count == 0) { > + ret = clk_prepare_enable(jpu->clk); > + if (ret < 0) > + goto error; @@ -855,7 +857,7 @@ Does all the code below reallly need to be protected by the mutex ? > + struct jpu_ctx *ctx = fh_to_ctx(file->private_data); > + > + mutex_lock(&jpu->mutex); -> + if (--jpu->ref_count = 0) +> + if (--jpu->ref_count == 0) > + clk_disable_unprepare(jpu->clk); > + v4l2_m2m_ctx_release(ctx->fh.m2m_ctx); > + v4l2_ctrl_handler_free(&ctx->ctrl_handler); @@ -881,9 +883,9 @@ protect the clock disabling code ? > +}; > + > +/* -> + * ==================================== +> + * ======================================================================== > + * mem2mem callbacks -> + * ==================================== +> + * ======================================================================== > + */ > +static void jpu_cleanup(struct jpu_ctx *ctx) > +{ @@ -962,7 +964,7 @@ At this point I think the buffer belongs to the device. Have you considered possible caching issues ? Would it make sense to write the header when the buffer is prepared ? -> + if (subsampling = JPU_JPEG_420) { +> + if (subsampling == JPU_JPEG_420) { > + redu = JCMOD_REDU_420; > + inft = JIFECNT_INFT_420; > + } else { @@ -1072,9 +1074,9 @@ buffer is prepared ? > +}; > + > +/* -> + * ==================================== +> + * ======================================================================== > + * IRQ handler -> + * ==================================== +> + * ======================================================================== > + */ > +static irqreturn_t jpu_irq_handler(int irq, void *dev_id) > +{ @@ -1165,7 +1167,8 @@ That's a strange hardware design :-) > + dst_buf->v4l2_buf.timecode = src_buf->v4l2_buf.timecode; > + dst_buf->v4l2_buf.timestamp = src_buf->v4l2_buf.timestamp; > + dst_buf->v4l2_buf.flags &= ~V4L2_BUF_FLAG_TSTAMP_SRC_MASK; -> + dst_buf->v4l2_buf.flags |> + src_buf->v4l2_buf.flags & V4L2_BUF_FLAG_TSTAMP_SRC_MASK; +> + dst_buf->v4l2_buf.flags |= +> + src_buf->v4l2_buf.flags & V4L2_BUF_FLAG_TSTAMP_SRC_MASK; > + > + v4l2_m2m_buf_done(src_buf, VB2_BUF_STATE_DONE); > + v4l2_m2m_buf_done(dst_buf, VB2_BUF_STATE_DONE); @@ -1189,9 +1192,9 @@ That's a strange hardware design :-) > +} > + > +/* -> + * ==================================== +> + * ======================================================================== > + * Driver basic infrastructure -> + * ==================================== +> + * ======================================================================== > + */ > +static const struct of_device_id jpu_dt_ids[] = { > + { .compatible = "renesas,jpu-r8a7790" }, /* H2 */ @@ -1376,7 +1379,7 @@ would be a bit confusing if the probe function then fails. > +{ > + struct jpu *jpu = dev_get_drvdata(dev); > + -> + if (jpu->ref_count = 0) +> + if (jpu->ref_count == 0) > + return 0; > + > + clk_disable_unprepare(jpu->clk); @@ -1394,7 +1397,7 @@ wondering whether the same would apply to the JPU. > +{ > + struct jpu *jpu = dev_get_drvdata(dev); > + -> + if (jpu->ref_count = 0) +> + if (jpu->ref_count == 0) > + return 0; > + > + clk_prepare_enable(jpu->clk); diff --git a/a/content_digest b/N1/content_digest index 1f889dc..0a83655 100644 --- a/a/content_digest +++ b/N1/content_digest @@ -1,7 +1,7 @@ "ref\01430344409-11928-1-git-send-email-mikhail.ulyanov@cogentembedded.com\0" "From\0Laurent Pinchart <laurent.pinchart@ideasonboard.com>\0" "Subject\0Re: [PATCH v3 1/1] V4L2: platform: Renesas R-Car JPEG codec driver\0" - "Date\0Sun, 03 May 2015 23:32:05 +0000\0" + "Date\0Mon, 04 May 2015 02:32:05 +0300\0" "To\0Mikhail Ulyanov <mikhail.ulyanov@cogentembedded.com>\0" "Cc\0hverkuil@xs4all.nl" horms@verge.net.au @@ -421,9 +421,9 @@ "[snip]\n" "\n" "> +/*\n" - "> + * ====================================\n" + "> + * ========================================================================\n" "> + * video ioctl operations\n" - "> + * ====================================\n" + "> + * ========================================================================\n" "> + */\n" "> +static void put_qtbl(u8 *p, const unsigned int *qtbl)\n" "> +{\n" @@ -476,11 +476,11 @@ "> +\tint byte;\n" "> +\n" "> +\tbyte = get_byte(buf);\n" - "> +\tif (byte = -1)\n" + "> +\tif (byte == -1)\n" "> +\t\treturn -1;\n" "> +\ttemp = byte << 8;\n" "> +\tbyte = get_byte(buf);\n" - "> +\tif (byte = -1)\n" + "> +\tif (byte == -1)\n" "> +\t\treturn -1;\n" "> +\t*word = (unsigned int)byte | temp;\n" "> +\treturn 0;\n" @@ -532,16 +532,16 @@ "> +\twhile (!found) {\n" "> +\t\tint c = get_byte(&jpeg_buffer);\n" "> +\n" - "> +\t\tif (c = -1)\n" + "> +\t\tif (c == -1)\n" "> +\t\t\treturn false;\n" "> +\t\tif (c != 0xff)\n" "> +\t\t\tcontinue;\n" "> +\t\tdo\n" "> +\t\t\tc = get_byte(&jpeg_buffer);\n" - "> +\t\twhile (c = 0xff);\n" - "> +\t\tif (c = -1)\n" + "> +\t\twhile (c == 0xff);\n" + "> +\t\tif (c == -1)\n" "> +\t\t\treturn false;\n" - "> +\t\tif (c = 0)\n" + "> +\t\tif (c == 0)\n" "> +\t\t\tcontinue;\n" "> +\t\tlength = 0;\n" "> +\t\tswitch (c) {\n" @@ -549,18 +549,18 @@ "> +\t\tcase SOF0:\n" "> +\t\t\tif (get_word_be(&jpeg_buffer, &word))\n" "> +\t\t\t\tbreak;\n" - "> +\t\t\tif (get_byte(&jpeg_buffer) = -1)\n" + "> +\t\t\tif (get_byte(&jpeg_buffer) == -1)\n" "> +\t\t\t\tbreak;\n" "> +\t\t\tif (get_word_be(&jpeg_buffer, height))\n" "> +\t\t\t\tbreak;\n" "> +\t\t\tif (get_word_be(&jpeg_buffer, width))\n" "> +\t\t\t\tbreak;\n" "> +\t\t\tcomponents = get_byte(&jpeg_buffer);\n" - "> +\t\t\tif (components = -1)\n" + "> +\t\t\tif (components == -1)\n" "> +\t\t\t\tbreak;\n" "> +\t\t\tfound = true;\n" "> +\n" - "> +\t\t\tif (components = 1) {\n" + "> +\t\t\tif (components == 1) {\n" "> +\t\t\t\tsubsampling = 0x33;\n" "> +\t\t\t} else {\n" "> +\t\t\t\tskip(&jpeg_buffer, 1);\n" @@ -627,10 +627,10 @@ "below.\n" "\n" "> +\t\tif (ctx->encoder)\n" - "> +\t\t\tpixelformat = (f_type = JPU_FMT_TYPE_OUTPUT) ?\n" + "> +\t\t\tpixelformat = (f_type == JPU_FMT_TYPE_OUTPUT) ?\n" "> +\t\t\t\tV4L2_PIX_FMT_NV16M : V4L2_PIX_FMT_JPEG;\n" "> +\t\telse\n" - "> +\t\t\tpixelformat = (f_type = JPU_FMT_TYPE_CAPTURE) ?\n" + "> +\t\t\tpixelformat = (f_type == JPU_FMT_TYPE_CAPTURE) ?\n" "> +\t\t\t\tV4L2_PIX_FMT_NV16M : V4L2_PIX_FMT_JPEG;\n" "> +\t\tfmt = jpu_find_format(ctx->encoder, pixelformat, f_type);\n" "> +\t}\n" @@ -648,7 +648,7 @@ "> +\tw = pix->width;\n" "> +\th = pix->height;\n" "> +\n" - "> +\tif (fmt->fourcc = V4L2_PIX_FMT_JPEG) {\n" + "> +\tif (fmt->fourcc == V4L2_PIX_FMT_JPEG) {\n" "> +\t\tif (pix->plane_fmt[0].sizeimage <= 0)\n" "> +\t\t\tpix->plane_fmt[0].sizeimage = JPU_JPEG_HDR_SIZE +\n" "> +\t\t\t\t\t(JPU_JPEG_MAX_BITS_PER_PIXEL * w * h);\n" @@ -668,8 +668,10 @@ "> +\t\t\t\t w / hsub * fmt->bpp[i] / 8,\n" "> +\t\t\t\t JPU_WIDTH_MAX);\n" "> +\n" - "> +\t\t\tpix->plane_fmt[i].bytesperline > +\t\t\t\t\tround_up(bpl, JPU_MEMALIGN);\n" - "> +\t\t\tpix->plane_fmt[i].sizeimage > +\t\t\t\tpix->plane_fmt[i].bytesperline * h / vsub;\n" + "> +\t\t\tpix->plane_fmt[i].bytesperline =\n" + "> +\t\t\t\t\tround_up(bpl, JPU_MEMALIGN);\n" + "> +\t\t\tpix->plane_fmt[i].sizeimage =\n" + "> +\t\t\t\tpix->plane_fmt[i].bytesperline * h / vsub;\n" "> +\t\t\tmemset(pix->plane_fmt[i].reserved, 0,\n" "> +\t\t\t sizeof(pix->plane_fmt[i].reserved));\n" "> +\t\t}\n" @@ -684,9 +686,9 @@ "[snip]\n" "\n" "> +/*\n" - "> + * ====================================\n" + "> + * ========================================================================\n" "> + * Queue operations\n" - "> + * --===================================\n" + "> + * --======================================================================\n" "> + */\n" "> +static int jpu_queue_setup(struct vb2_queue *vq,\n" "> +\t\t\t const struct v4l2_format *fmt,\n" @@ -793,9 +795,9 @@ "[snip]\n" "\n" "> +/*\n" - "> + * ====================================\n" + "> + * ========================================================================\n" "> + * Device file operations\n" - "> + * ====================================\n" + "> + * ========================================================================\n" "> + */\n" "> +static int jpu_open(struct file *file)\n" "> +{\n" @@ -821,7 +823,7 @@ "> +\tv4l2_fh_add(&ctx->fh);\n" "> +\n" "> +\tctx->jpu = jpu;\n" - "> +\tctx->encoder = (vfd = jpu->vfd_encoder);\n" + "> +\tctx->encoder = (vfd == jpu->vfd_encoder);\n" "> +\n" "> +\t__jpu_try_fmt(ctx, &ctx->out_q.format,\n" "> +\t\t V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE, &ctx->out_q.fmtinfo);\n" @@ -838,7 +840,7 @@ "> +\tif (ret < 0)\n" "> +\t\tgoto error;\n" "> +\n" - "> +\tif (jpu->ref_count = 0) {\n" + "> +\tif (jpu->ref_count == 0) {\n" "> +\t\tret = clk_prepare_enable(jpu->clk);\n" "> +\t\tif (ret < 0)\n" "> +\t\t\tgoto error;\n" @@ -868,7 +870,7 @@ "> +\tstruct jpu_ctx *ctx = fh_to_ctx(file->private_data);\n" "> +\n" "> +\tmutex_lock(&jpu->mutex);\n" - "> +\tif (--jpu->ref_count = 0)\n" + "> +\tif (--jpu->ref_count == 0)\n" "> +\t\tclk_disable_unprepare(jpu->clk);\n" "> +\tv4l2_m2m_ctx_release(ctx->fh.m2m_ctx);\n" "> +\tv4l2_ctrl_handler_free(&ctx->ctrl_handler);\n" @@ -894,9 +896,9 @@ "> +};\n" "> +\n" "> +/*\n" - "> + * ====================================\n" + "> + * ========================================================================\n" "> + * mem2mem callbacks\n" - "> + * ====================================\n" + "> + * ========================================================================\n" "> + */\n" "> +static void jpu_cleanup(struct jpu_ctx *ctx)\n" "> +{\n" @@ -975,7 +977,7 @@ "possible caching issues ? Would it make sense to write the header when the \n" "buffer is prepared ?\n" "\n" - "> +\t\tif (subsampling = JPU_JPEG_420) {\n" + "> +\t\tif (subsampling == JPU_JPEG_420) {\n" "> +\t\t\tredu = JCMOD_REDU_420;\n" "> +\t\t\tinft = JIFECNT_INFT_420;\n" "> +\t\t} else {\n" @@ -1085,9 +1087,9 @@ "> +};\n" "> +\n" "> +/*\n" - "> + * ====================================\n" + "> + * ========================================================================\n" "> + * IRQ handler\n" - "> + * ====================================\n" + "> + * ========================================================================\n" "> + */\n" "> +static irqreturn_t jpu_irq_handler(int irq, void *dev_id)\n" "> +{\n" @@ -1178,7 +1180,8 @@ "> +\t\tdst_buf->v4l2_buf.timecode = src_buf->v4l2_buf.timecode;\n" "> +\t\tdst_buf->v4l2_buf.timestamp = src_buf->v4l2_buf.timestamp;\n" "> +\t\tdst_buf->v4l2_buf.flags &= ~V4L2_BUF_FLAG_TSTAMP_SRC_MASK;\n" - "> +\t\tdst_buf->v4l2_buf.flags |> +\t\t src_buf->v4l2_buf.flags & V4L2_BUF_FLAG_TSTAMP_SRC_MASK;\n" + "> +\t\tdst_buf->v4l2_buf.flags |=\n" + "> +\t\t src_buf->v4l2_buf.flags & V4L2_BUF_FLAG_TSTAMP_SRC_MASK;\n" "> +\n" "> +\t\tv4l2_m2m_buf_done(src_buf, VB2_BUF_STATE_DONE);\n" "> +\t\tv4l2_m2m_buf_done(dst_buf, VB2_BUF_STATE_DONE);\n" @@ -1202,9 +1205,9 @@ "> +}\n" "> +\n" "> +/*\n" - "> + * ====================================\n" + "> + * ========================================================================\n" "> + * Driver basic infrastructure\n" - "> + * ====================================\n" + "> + * ========================================================================\n" "> + */\n" "> +static const struct of_device_id jpu_dt_ids[] = {\n" "> +\t{ .compatible = \"renesas,jpu-r8a7790\" }, /* H2 */\n" @@ -1389,7 +1392,7 @@ "> +{\n" "> +\tstruct jpu *jpu = dev_get_drvdata(dev);\n" "> +\n" - "> +\tif (jpu->ref_count = 0)\n" + "> +\tif (jpu->ref_count == 0)\n" "> +\t\treturn 0;\n" "> +\n" "> +\tclk_disable_unprepare(jpu->clk);\n" @@ -1407,7 +1410,7 @@ "> +{\n" "> +\tstruct jpu *jpu = dev_get_drvdata(dev);\n" "> +\n" - "> +\tif (jpu->ref_count = 0)\n" + "> +\tif (jpu->ref_count == 0)\n" "> +\t\treturn 0;\n" "> +\n" "> +\tclk_prepare_enable(jpu->clk);\n" @@ -1423,4 +1426,4 @@ "\n" Laurent Pinchart -185352b6b96c167ab8b7cf06be9863ea65e73a27e128dd625daa34cd3413204a +f71d9b35d8ebcddb1394da1788e2d6744da0788da2884a43d710d1b78fcbf7ef
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.