Skip to content

Commit

Permalink
[media] s5p-mfc: Fix sparse errors in the MFC driver
Browse files Browse the repository at this point in the history
The following error: "error: incompatible types in conditional expression
(different base types)" was reported multiple times for the s5p-mfc
driver. This error was caused by two macro definitions - s5p_mfc_hw_call
(in s5p_mfc_common.h) and WRITEL (in s5p_mfc_opr_v6.c).

In the former case the macro assumed that all ops return a value, but some
ops return void. The solution to this problem was the addition of a
s5p_mfc_hw_call_void macro.

In the latter case the macro used the ?: construction to check whether
the address is non zero. This is not necessary after the driver left the
development and debugging cycle, so the READL and WRITEL macros were
removed.

Signed-off-by: Kamil Debski <k.debski@samsung.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
  • Loading branch information
Kamil Debski authored and Mauro Carvalho Chehab committed Sep 26, 2014
1 parent 9aee8b8 commit e2c3be2
Show file tree
Hide file tree
Showing 6 changed files with 293 additions and 292 deletions.
46 changes: 23 additions & 23 deletions drivers/media/platform/s5p-mfc/s5p_mfc.c
Original file line number Diff line number Diff line change
Expand Up @@ -150,10 +150,10 @@ static void s5p_mfc_watchdog_worker(struct work_struct *work)
if (!ctx)
continue;
ctx->state = MFCINST_ERROR;
s5p_mfc_hw_call(dev->mfc_ops, cleanup_queue, &ctx->dst_queue,
&ctx->vq_dst);
s5p_mfc_hw_call(dev->mfc_ops, cleanup_queue, &ctx->src_queue,
&ctx->vq_src);
s5p_mfc_hw_call_void(dev->mfc_ops, cleanup_queue,
&ctx->dst_queue, &ctx->vq_dst);
s5p_mfc_hw_call_void(dev->mfc_ops, cleanup_queue,
&ctx->src_queue, &ctx->vq_src);
clear_work_bit(ctx);
wake_up_ctx(ctx, S5P_MFC_R2H_CMD_ERR_RET, 0);
}
Expand Down Expand Up @@ -327,12 +327,12 @@ static void s5p_mfc_handle_frame(struct s5p_mfc_ctx *ctx,
if (res_change == S5P_FIMV_RES_INCREASE ||
res_change == S5P_FIMV_RES_DECREASE) {
ctx->state = MFCINST_RES_CHANGE_INIT;
s5p_mfc_hw_call(dev->mfc_ops, clear_int_flags, dev);
s5p_mfc_hw_call_void(dev->mfc_ops, clear_int_flags, dev);
wake_up_ctx(ctx, reason, err);
if (test_and_clear_bit(0, &dev->hw_lock) == 0)
BUG();
s5p_mfc_clock_off();
s5p_mfc_hw_call(dev->mfc_ops, try_run, dev);
s5p_mfc_hw_call_void(dev->mfc_ops, try_run, dev);
return;
}
if (ctx->dpb_flush_flag)
Expand Down Expand Up @@ -400,7 +400,7 @@ static void s5p_mfc_handle_frame(struct s5p_mfc_ctx *ctx,
if ((ctx->src_queue_cnt == 0 && ctx->state != MFCINST_FINISHING)
|| ctx->dst_queue_cnt < ctx->pb_count)
clear_work_bit(ctx);
s5p_mfc_hw_call(dev->mfc_ops, clear_int_flags, dev);
s5p_mfc_hw_call_void(dev->mfc_ops, clear_int_flags, dev);
wake_up_ctx(ctx, reason, err);
if (test_and_clear_bit(0, &dev->hw_lock) == 0)
BUG();
Expand All @@ -409,7 +409,7 @@ static void s5p_mfc_handle_frame(struct s5p_mfc_ctx *ctx,
if (test_bit(0, &dev->enter_suspend))
wake_up_dev(dev, reason, err);
else
s5p_mfc_hw_call(dev->mfc_ops, try_run, dev);
s5p_mfc_hw_call_void(dev->mfc_ops, try_run, dev);
}

/* Error handling for interrupt */
Expand All @@ -435,10 +435,10 @@ static void s5p_mfc_handle_error(struct s5p_mfc_dev *dev,
ctx->state = MFCINST_ERROR;
/* Mark all dst buffers as having an error */
spin_lock_irqsave(&dev->irqlock, flags);
s5p_mfc_hw_call(dev->mfc_ops, cleanup_queue,
s5p_mfc_hw_call_void(dev->mfc_ops, cleanup_queue,
&ctx->dst_queue, &ctx->vq_dst);
/* Mark all src buffers as having an error */
s5p_mfc_hw_call(dev->mfc_ops, cleanup_queue,
s5p_mfc_hw_call_void(dev->mfc_ops, cleanup_queue,
&ctx->src_queue, &ctx->vq_src);
spin_unlock_irqrestore(&dev->irqlock, flags);
wake_up_ctx(ctx, reason, err);
Expand All @@ -452,7 +452,7 @@ static void s5p_mfc_handle_error(struct s5p_mfc_dev *dev,
}
if (test_and_clear_bit(0, &dev->hw_lock) == 0)
BUG();
s5p_mfc_hw_call(dev->mfc_ops, clear_int_flags, dev);
s5p_mfc_hw_call_void(dev->mfc_ops, clear_int_flags, dev);
s5p_mfc_clock_off();
wake_up_dev(dev, reason, err);
return;
Expand All @@ -476,7 +476,7 @@ static void s5p_mfc_handle_seq_done(struct s5p_mfc_ctx *ctx,
ctx->img_height = s5p_mfc_hw_call(dev->mfc_ops, get_img_height,
dev);

s5p_mfc_hw_call(dev->mfc_ops, dec_calc_dpb_size, ctx);
s5p_mfc_hw_call_void(dev->mfc_ops, dec_calc_dpb_size, ctx);

ctx->pb_count = s5p_mfc_hw_call(dev->mfc_ops, get_dpb_count,
dev);
Expand All @@ -503,12 +503,12 @@ static void s5p_mfc_handle_seq_done(struct s5p_mfc_ctx *ctx,
ctx->head_processed = 1;
}
}
s5p_mfc_hw_call(dev->mfc_ops, clear_int_flags, dev);
s5p_mfc_hw_call_void(dev->mfc_ops, clear_int_flags, dev);
clear_work_bit(ctx);
if (test_and_clear_bit(0, &dev->hw_lock) == 0)
BUG();
s5p_mfc_clock_off();
s5p_mfc_hw_call(dev->mfc_ops, try_run, dev);
s5p_mfc_hw_call_void(dev->mfc_ops, try_run, dev);
wake_up_ctx(ctx, reason, err);
}

Expand All @@ -523,7 +523,7 @@ static void s5p_mfc_handle_init_buffers(struct s5p_mfc_ctx *ctx,
if (ctx == NULL)
return;
dev = ctx->dev;
s5p_mfc_hw_call(dev->mfc_ops, clear_int_flags, dev);
s5p_mfc_hw_call_void(dev->mfc_ops, clear_int_flags, dev);
ctx->int_type = reason;
ctx->int_err = err;
ctx->int_cond = 1;
Expand All @@ -550,7 +550,7 @@ static void s5p_mfc_handle_init_buffers(struct s5p_mfc_ctx *ctx,
s5p_mfc_clock_off();

wake_up(&ctx->queue);
s5p_mfc_hw_call(dev->mfc_ops, try_run, dev);
s5p_mfc_hw_call_void(dev->mfc_ops, try_run, dev);
} else {
if (test_and_clear_bit(0, &dev->hw_lock) == 0)
BUG();
Expand Down Expand Up @@ -591,7 +591,7 @@ static void s5p_mfc_handle_stream_complete(struct s5p_mfc_ctx *ctx,

s5p_mfc_clock_off();
wake_up(&ctx->queue);
s5p_mfc_hw_call(dev->mfc_ops, try_run, dev);
s5p_mfc_hw_call_void(dev->mfc_ops, try_run, dev);
}

/* Interrupt processing */
Expand Down Expand Up @@ -628,12 +628,12 @@ static irqreturn_t s5p_mfc_irq(int irq, void *priv)
if (ctx->c_ops->post_frame_start) {
if (ctx->c_ops->post_frame_start(ctx))
mfc_err("post_frame_start() failed\n");
s5p_mfc_hw_call(dev->mfc_ops, clear_int_flags, dev);
s5p_mfc_hw_call_void(dev->mfc_ops, clear_int_flags, dev);
wake_up_ctx(ctx, reason, err);
if (test_and_clear_bit(0, &dev->hw_lock) == 0)
BUG();
s5p_mfc_clock_off();
s5p_mfc_hw_call(dev->mfc_ops, try_run, dev);
s5p_mfc_hw_call_void(dev->mfc_ops, try_run, dev);
} else {
s5p_mfc_handle_frame(ctx, reason, err);
}
Expand Down Expand Up @@ -663,7 +663,7 @@ static irqreturn_t s5p_mfc_irq(int irq, void *priv)
case S5P_MFC_R2H_CMD_WAKEUP_RET:
if (ctx)
clear_work_bit(ctx);
s5p_mfc_hw_call(dev->mfc_ops, clear_int_flags, dev);
s5p_mfc_hw_call_void(dev->mfc_ops, clear_int_flags, dev);
wake_up_dev(dev, reason, err);
clear_bit(0, &dev->hw_lock);
clear_bit(0, &dev->enter_suspend);
Expand All @@ -685,12 +685,12 @@ static irqreturn_t s5p_mfc_irq(int irq, void *priv)

default:
mfc_debug(2, "Unknown int reason\n");
s5p_mfc_hw_call(dev->mfc_ops, clear_int_flags, dev);
s5p_mfc_hw_call_void(dev->mfc_ops, clear_int_flags, dev);
}
mfc_debug_leave();
return IRQ_HANDLED;
irq_cleanup_hw:
s5p_mfc_hw_call(dev->mfc_ops, clear_int_flags, dev);
s5p_mfc_hw_call_void(dev->mfc_ops, clear_int_flags, dev);
ctx->int_type = reason;
ctx->int_err = err;
ctx->int_cond = 1;
Expand All @@ -699,7 +699,7 @@ static irqreturn_t s5p_mfc_irq(int irq, void *priv)

s5p_mfc_clock_off();

s5p_mfc_hw_call(dev->mfc_ops, try_run, dev);
s5p_mfc_hw_call_void(dev->mfc_ops, try_run, dev);
mfc_debug(2, "Exit via irq_cleanup_hw\n");
return IRQ_HANDLED;
}
Expand Down
6 changes: 6 additions & 0 deletions drivers/media/platform/s5p-mfc/s5p_mfc_common.h
Original file line number Diff line number Diff line change
Expand Up @@ -698,6 +698,12 @@ struct mfc_control {
#define s5p_mfc_hw_call(f, op, args...) \
((f && f->op) ? f->op(args) : -ENODEV)

#define s5p_mfc_hw_call_void(f, op, args...) \
do { \
if (f && f->op) \
f->op(args); \
} while (0)

#define fh_to_ctx(__fh) container_of(__fh, struct s5p_mfc_ctx, fh)
#define ctrl_to_ctx(__ctrl) \
container_of((__ctrl)->handler, struct s5p_mfc_ctx, ctrl_handler)
Expand Down
16 changes: 8 additions & 8 deletions drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ void s5p_mfc_deinit_hw(struct s5p_mfc_dev *dev)
s5p_mfc_clock_on();

s5p_mfc_reset(dev);
s5p_mfc_hw_call(dev->mfc_ops, release_dev_context_buffer, dev);
s5p_mfc_hw_call_void(dev->mfc_ops, release_dev_context_buffer, dev);

s5p_mfc_clock_off();
}
Expand Down Expand Up @@ -397,7 +397,7 @@ int s5p_mfc_open_mfc_inst(struct s5p_mfc_dev *dev, struct s5p_mfc_ctx *ctx)

set_work_bit_irqsave(ctx);
s5p_mfc_clean_ctx_int_flags(ctx);
s5p_mfc_hw_call(dev->mfc_ops, try_run, dev);
s5p_mfc_hw_call_void(dev->mfc_ops, try_run, dev);
if (s5p_mfc_wait_for_done_ctx(ctx,
S5P_MFC_R2H_CMD_OPEN_INSTANCE_RET, 0)) {
/* Error or timeout */
Expand All @@ -411,9 +411,9 @@ int s5p_mfc_open_mfc_inst(struct s5p_mfc_dev *dev, struct s5p_mfc_ctx *ctx)

err_free_desc_buf:
if (ctx->type == MFCINST_DECODER)
s5p_mfc_hw_call(dev->mfc_ops, release_dec_desc_buffer, ctx);
s5p_mfc_hw_call_void(dev->mfc_ops, release_dec_desc_buffer, ctx);
err_free_inst_buf:
s5p_mfc_hw_call(dev->mfc_ops, release_instance_buffer, ctx);
s5p_mfc_hw_call_void(dev->mfc_ops, release_instance_buffer, ctx);
err:
return ret;
}
Expand All @@ -423,17 +423,17 @@ void s5p_mfc_close_mfc_inst(struct s5p_mfc_dev *dev, struct s5p_mfc_ctx *ctx)
ctx->state = MFCINST_RETURN_INST;
set_work_bit_irqsave(ctx);
s5p_mfc_clean_ctx_int_flags(ctx);
s5p_mfc_hw_call(dev->mfc_ops, try_run, dev);
s5p_mfc_hw_call_void(dev->mfc_ops, try_run, dev);
/* Wait until instance is returned or timeout occurred */
if (s5p_mfc_wait_for_done_ctx(ctx,
S5P_MFC_R2H_CMD_CLOSE_INSTANCE_RET, 0))
mfc_err("Err returning instance\n");

/* Free resources */
s5p_mfc_hw_call(dev->mfc_ops, release_codec_buffers, ctx);
s5p_mfc_hw_call(dev->mfc_ops, release_instance_buffer, ctx);
s5p_mfc_hw_call_void(dev->mfc_ops, release_codec_buffers, ctx);
s5p_mfc_hw_call_void(dev->mfc_ops, release_instance_buffer, ctx);
if (ctx->type == MFCINST_DECODER)
s5p_mfc_hw_call(dev->mfc_ops, release_dec_desc_buffer, ctx);
s5p_mfc_hw_call_void(dev->mfc_ops, release_dec_desc_buffer, ctx);

ctx->inst_no = MFC_NO_INSTANCE_SET;
ctx->state = MFCINST_FREE;
Expand Down
20 changes: 10 additions & 10 deletions drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
Original file line number Diff line number Diff line change
Expand Up @@ -543,7 +543,7 @@ static int reqbufs_capture(struct s5p_mfc_dev *dev, struct s5p_mfc_ctx *ctx,
ret = vb2_reqbufs(&ctx->vq_dst, reqbufs);
if (ret)
goto out;
s5p_mfc_hw_call(dev->mfc_ops, release_codec_buffers, ctx);
s5p_mfc_hw_call_void(dev->mfc_ops, release_codec_buffers, ctx);
ctx->dst_bufs_cnt = 0;
} else if (ctx->capture_state == QUEUE_FREE) {
WARN_ON(ctx->dst_bufs_cnt != 0);
Expand Down Expand Up @@ -571,7 +571,7 @@ static int reqbufs_capture(struct s5p_mfc_dev *dev, struct s5p_mfc_ctx *ctx,

if (s5p_mfc_ctx_ready(ctx))
set_work_bit_irqsave(ctx);
s5p_mfc_hw_call(dev->mfc_ops, try_run, dev);
s5p_mfc_hw_call_void(dev->mfc_ops, try_run, dev);
s5p_mfc_wait_for_done_ctx(ctx, S5P_MFC_R2H_CMD_INIT_BUFFERS_RET,
0);
} else {
Expand Down Expand Up @@ -846,7 +846,7 @@ static int vidioc_decoder_cmd(struct file *file, void *priv,
if (s5p_mfc_ctx_ready(ctx))
set_work_bit_irqsave(ctx);
spin_unlock_irqrestore(&dev->irqlock, flags);
s5p_mfc_hw_call(dev->mfc_ops, try_run, dev);
s5p_mfc_hw_call_void(dev->mfc_ops, try_run, dev);
} else {
mfc_err("EOS: marking last buffer of stream");
buf = list_entry(ctx->src_queue.prev,
Expand Down Expand Up @@ -1044,7 +1044,7 @@ static int s5p_mfc_start_streaming(struct vb2_queue *q, unsigned int count)
/* If context is ready then dev = work->data;schedule it to run */
if (s5p_mfc_ctx_ready(ctx))
set_work_bit_irqsave(ctx);
s5p_mfc_hw_call(dev->mfc_ops, try_run, dev);
s5p_mfc_hw_call_void(dev->mfc_ops, try_run, dev);
return 0;
}

Expand All @@ -1065,8 +1065,8 @@ static void s5p_mfc_stop_streaming(struct vb2_queue *q)
}
if (q->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) {
spin_lock_irqsave(&dev->irqlock, flags);
s5p_mfc_hw_call(dev->mfc_ops, cleanup_queue, &ctx->dst_queue,
&ctx->vq_dst);
s5p_mfc_hw_call_void(dev->mfc_ops, cleanup_queue,
&ctx->dst_queue, &ctx->vq_dst);
INIT_LIST_HEAD(&ctx->dst_queue);
ctx->dst_queue_cnt = 0;
ctx->dpb_flush_flag = 1;
Expand All @@ -1076,16 +1076,16 @@ static void s5p_mfc_stop_streaming(struct vb2_queue *q)
ctx->state = MFCINST_FLUSH;
set_work_bit_irqsave(ctx);
s5p_mfc_clean_ctx_int_flags(ctx);
s5p_mfc_hw_call(dev->mfc_ops, try_run, dev);
s5p_mfc_hw_call_void(dev->mfc_ops, try_run, dev);
if (s5p_mfc_wait_for_done_ctx(ctx,
S5P_MFC_R2H_CMD_DPB_FLUSH_RET, 0))
mfc_err("Err flushing buffers\n");
}
}
if (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) {
spin_lock_irqsave(&dev->irqlock, flags);
s5p_mfc_hw_call(dev->mfc_ops, cleanup_queue, &ctx->src_queue,
&ctx->vq_src);
s5p_mfc_hw_call_void(dev->mfc_ops, cleanup_queue,
&ctx->src_queue, &ctx->vq_src);
INIT_LIST_HEAD(&ctx->src_queue);
ctx->src_queue_cnt = 0;
spin_unlock_irqrestore(&dev->irqlock, flags);
Expand Down Expand Up @@ -1124,7 +1124,7 @@ static void s5p_mfc_buf_queue(struct vb2_buffer *vb)
}
if (s5p_mfc_ctx_ready(ctx))
set_work_bit_irqsave(ctx);
s5p_mfc_hw_call(dev->mfc_ops, try_run, dev);
s5p_mfc_hw_call_void(dev->mfc_ops, try_run, dev);
}

static struct vb2_ops s5p_mfc_dec_qops = {
Expand Down
Loading

0 comments on commit e2c3be2

Please sign in to comment.