Re: [PATCH v2] staging: cedrus: Fix checkpatch issues
Hi,
On Thu, 2018-09-13 at 11:53 -0300, Mauro Carvalho Chehab wrote:
> Em Thu, 13 Sep 2018 16:40:47 +0200
> Maxime Ripard escreveu:
>
>
> > --- a/drivers/staging/media/sunxi/cedrus/cedrus_video.c
> > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
> > @@ -82,10 +82,7 @@ static struct cedrus_format *cedrus_find_format(u32
> > pixelformat, u32 directions,
> > static bool cedrus_check_format(u32 pixelformat, u32 directions,
> > unsigned int capabilities)
> > {
> > - struct cedrus_format *fmt = cedrus_find_format(pixelformat, directions,
> > - capabilities);
> > -
> > - return fmt != NULL;
> > + return cedrus_find_format(pixelformat, directions, capabilities);
> > }
>
> Hmm... just occurred to me... Why do you need this? I mean, you
> could simply do something like:
>
> $ git filter-branch -f --tree-filter 'for i in $(git grep -l
> cedrus_check_format); do \
> sed -E s,\\bcedrus_check_format\\b,cedrus_find_format,g -i $i; done '
> origin/master..
>
> (or just do a sed -E s,\\bcedrus_check_format\\b,cedrus_find_format,g as
> a separate patch)
>
> and get rid of cedrus_check_format() for good.
Agreed, the name is probably explicit enough anyway. I probably should
have done that in the first place anyway.
Cheers,
Paul
--
Developer of free digital technology and hardware support.
Website: https://www.paulk.fr/
Coding blog: https://code.paulk.fr/
Git repositories: https://git.paulk.fr/ https://git.code.paulk.fr/
signature.asc
Description: This is a digitally signed message part
Re: [PATCH v2] staging: cedrus: Fix checkpatch issues
On 09/13/2018 04:53 PM, Mauro Carvalho Chehab wrote:
> Em Thu, 13 Sep 2018 16:40:47 +0200
> Maxime Ripard escreveu:
>
>
>> --- a/drivers/staging/media/sunxi/cedrus/cedrus_video.c
>> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
>> @@ -82,10 +82,7 @@ static struct cedrus_format *cedrus_find_format(u32
>> pixelformat, u32 directions,
>> static bool cedrus_check_format(u32 pixelformat, u32 directions,
>> unsigned int capabilities)
>> {
>> -struct cedrus_format *fmt = cedrus_find_format(pixelformat, directions,
>> - capabilities);
>> -
>> -return fmt != NULL;
>> +return cedrus_find_format(pixelformat, directions, capabilities);
>> }
>
> Hmm... just occurred to me... Why do you need this? I mean, you
> could simply do something like:
>
> $ git filter-branch -f --tree-filter 'for i in $(git grep -l
> cedrus_check_format); do \
> sed -E s,\\bcedrus_check_format\\b,cedrus_find_format,g -i $i; done '
> origin/master..
>
> (or just do a sed -E s,\\bcedrus_check_format\\b,cedrus_find_format,g as
> a separate patch)
>
> and get rid of cedrus_check_format() for good.
That looks like a nice follow-up patch. It's a staging driver, it doesn't
have to be perfect.
I'll prepare a pull request tomorrow.
Regards,
Hans
Re: [PATCH v2] staging: cedrus: Fix checkpatch issues
Em Thu, 13 Sep 2018 16:40:47 +0200
Maxime Ripard escreveu:
> --- a/drivers/staging/media/sunxi/cedrus/cedrus_video.c
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
> @@ -82,10 +82,7 @@ static struct cedrus_format *cedrus_find_format(u32
> pixelformat, u32 directions,
> static bool cedrus_check_format(u32 pixelformat, u32 directions,
> unsigned int capabilities)
> {
> - struct cedrus_format *fmt = cedrus_find_format(pixelformat, directions,
> -capabilities);
> -
> - return fmt != NULL;
> + return cedrus_find_format(pixelformat, directions, capabilities);
> }
Hmm... just occurred to me... Why do you need this? I mean, you
could simply do something like:
$ git filter-branch -f --tree-filter 'for i in $(git grep -l
cedrus_check_format); do \
sed -E s,\\bcedrus_check_format\\b,cedrus_find_format,g -i $i; done '
origin/master..
(or just do a sed -E s,\\bcedrus_check_format\\b,cedrus_find_format,g as
a separate patch)
and get rid of cedrus_check_format() for good.
Thanks,
Mauro
[PATCH v2] staging: cedrus: Fix checkpatch issues
Checkpatch, when used with --strict, reports a number of issues on the
cedrus driver.
Fix those warnings, except for a few, minor, lines too long warnings.
Signed-off-by: Maxime Ripard
---
Changes from v1:
- Removed the find_control wrapping
- Added the bit length to the required variable
- Added __ to the macro arguments
drivers/staging/media/sunxi/cedrus/cedrus.c | 10 +-
drivers/staging/media/sunxi/cedrus/cedrus.h | 8 +---
.../staging/media/sunxi/cedrus/cedrus_dec.c | 10 ++
.../staging/media/sunxi/cedrus/cedrus_mpeg2.c | 19 ++-
.../staging/media/sunxi/cedrus/cedrus_regs.h | 18 ++
.../staging/media/sunxi/cedrus/cedrus_video.c | 6 ++
6 files changed, 42 insertions(+), 29 deletions(-)
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.c
b/drivers/staging/media/sunxi/cedrus/cedrus.c
index 1f5f20a1f849..82558455384a 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus.c
@@ -48,7 +48,7 @@ void *cedrus_find_control_data(struct cedrus_ctx *ctx, u32 id)
{
unsigned int i;
- for (i = 0; ctx->ctrls[i] != NULL; i++)
+ for (i = 0; ctx->ctrls[i]; i++)
if (ctx->ctrls[i]->id == id)
return ctx->ctrls[i]->p_cur.p;
@@ -147,10 +147,10 @@ static int cedrus_request_validate(struct media_request
*req)
continue;
ctrl_test = v4l2_ctrl_request_hdl_ctrl_find(hdl,
- cedrus_controls[i].id);
+
cedrus_controls[i].id);
if (!ctrl_test) {
v4l2_info(&ctx->dev->v4l2_dev,
-"Missing required codec control\n");
+ "Missing required codec control\n");
return -ENOENT;
}
}
@@ -310,8 +310,8 @@ static int cedrus_probe(struct platform_device *pdev)
dev->mdev.ops = &cedrus_m2m_media_ops;
dev->v4l2_dev.mdev = &dev->mdev;
- ret = v4l2_m2m_register_media_controller(dev->m2m_dev,
- vfd, MEDIA_ENT_F_PROC_VIDEO_DECODER);
+ ret = v4l2_m2m_register_media_controller(dev->m2m_dev, vfd,
+
MEDIA_ENT_F_PROC_VIDEO_DECODER);
if (ret) {
v4l2_err(&dev->v4l2_dev,
"Failed to initialize V4L2 M2M media controller\n");
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.h
b/drivers/staging/media/sunxi/cedrus/cedrus.h
index 3262341e8c9a..3f61248c57ac 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus.h
+++ b/drivers/staging/media/sunxi/cedrus/cedrus.h
@@ -44,7 +44,7 @@ struct cedrus_control {
u32 id;
u32 elem_size;
enum cedrus_codec codec;
- boolrequired;
+ unsigned char required:1;
};
struct cedrus_mpeg2_run {
@@ -150,12 +150,14 @@ static inline dma_addr_t cedrus_dst_buf_addr(struct
cedrus_ctx *ctx,
return buf ? cedrus_buf_addr(buf, &ctx->dst_fmt, plane) : 0;
}
-static inline struct cedrus_buffer *vb2_v4l2_to_cedrus_buffer(const struct
vb2_v4l2_buffer *p)
+static inline struct cedrus_buffer *
+vb2_v4l2_to_cedrus_buffer(const struct vb2_v4l2_buffer *p)
{
return container_of(p, struct cedrus_buffer, m2m_buf.vb);
}
-static inline struct cedrus_buffer *vb2_to_cedrus_buffer(const struct
vb2_buffer *p)
+static inline struct cedrus_buffer *
+vb2_to_cedrus_buffer(const struct vb2_buffer *p)
{
return vb2_v4l2_to_cedrus_buffer(to_vb2_v4l2_buffer(p));
}
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
b/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
index e40180a33951..788811a1414e 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
@@ -43,10 +43,12 @@ void cedrus_device_run(void *priv)
switch (ctx->src_fmt.pixelformat) {
case V4L2_PIX_FMT_MPEG2_SLICE:
- run.mpeg2.slice_params = cedrus_find_control_data(ctx,
- V4L2_CID_MPEG_VIDEO_MPEG2_SLICE_PARAMS);
- run.mpeg2.quantization = cedrus_find_control_data(ctx,
- V4L2_CID_MPEG_VIDEO_MPEG2_QUANTIZATION);
+ run.mpeg2.slice_params =
+ cedrus_find_control_data(ctx,
+
V4L2_CID_MPEG_VIDEO_MPEG2_SLICE_PARAMS);
+ run.mpeg2.quantization =
+ cedrus_find_control_data(ctx,
+
V4L2_CID_MPEG_VIDEO_MPEG2_QUANTIZATION);
break;
default:
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c
b/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c
index 029eb1626bf4..9abd39cae38c 100644
--- a/drivers/staging/media/sunxi/ced
