2015-05-17 17:19:09

by Alex Dowad

Subject: [PATCH] Clarify expression which uses both multiplication and pointer dereference

This fixes a checkpatch style error in vpfe_buffer_queue_setup.

Signed-off-by: Alex Dowad <[email protected]>
---
drivers/staging/media/davinci_vpfe/vpfe_video.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/media/davinci_vpfe/vpfe_video.c b/drivers/staging/media/davinci_vpfe/vpfe_video.c
index 06d48d5..04a687c 100644
--- a/drivers/staging/media/davinci_vpfe/vpfe_video.c
+++ b/drivers/staging/media/davinci_vpfe/vpfe_video.c
@@ -1095,7 +1095,7 @@ vpfe_buffer_queue_setup(struct vb2_queue *vq, const struct v4l2_format *fmt,
size = video->fmt.fmt.pix.sizeimage;

if (vpfe_dev->video_limit) {
- while (size * *nbuffers > vpfe_dev->video_limit)
+ while (size * (*nbuffers) > vpfe_dev->video_limit)
(*nbuffers)--;
}
if (pipe->state == VPFE_PIPELINE_STREAM_CONTINUOUS) {
--
2.0.0.GIT

2015-05-17 19:43:45

by Laurent Pinchart

Subject: Re: [PATCH] Clarify expression which uses both multiplication and pointer dereference

Hi Alex,

Thank you for the patch.

On Sunday 17 May 2015 19:18:42 Alex Dowad wrote:
> This fixes a checkpatch style error in vpfe_buffer_queue_setup.
>
> Signed-off-by: Alex Dowad <[email protected]>

Acked-by: Laurent Pinchart <[email protected]>

> ---
> drivers/staging/media/davinci_vpfe/vpfe_video.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/media/davinci_vpfe/vpfe_video.c
> b/drivers/staging/media/davinci_vpfe/vpfe_video.c index 06d48d5..04a687c
> 100644
> --- a/drivers/staging/media/davinci_vpfe/vpfe_video.c
> +++ b/drivers/staging/media/davinci_vpfe/vpfe_video.c
> @@ -1095,7 +1095,7 @@ vpfe_buffer_queue_setup(struct vb2_queue *vq, const
> struct v4l2_format *fmt, size = video->fmt.fmt.pix.sizeimage;
>
> if (vpfe_dev->video_limit) {
> - while (size * *nbuffers > vpfe_dev->video_limit)
> + while (size * (*nbuffers) > vpfe_dev->video_limit)
> (*nbuffers)--;
> }
> if (pipe->state == VPFE_PIPELINE_STREAM_CONTINUOUS) {

--
Regards,

Laurent Pinchart

2015-05-17 20:17:01

by Joe Perches

Subject: Re: [PATCH] Clarify expression which uses both multiplication and pointer dereference

On Sun, 2015-05-17 at 19:18 +0200, Alex Dowad wrote:
> This fixes a checkpatch style error in vpfe_buffer_queue_setup.

There is no checkpatch message for this style.

Nor should there be.

> diff --git a/drivers/staging/media/davinci_vpfe/vpfe_video.c b/drivers/staging/media/davinci_vpfe/vpfe_video.c
[]
> @@ -1095,7 +1095,7 @@ vpfe_buffer_queue_setup(struct vb2_queue *vq, const struct v4l2_format *fmt,
> - while (size * *nbuffers > vpfe_dev->video_limit)
> + while (size * (*nbuffers) > vpfe_dev->video_limit)

2015-05-18 06:52:11

by Michal Kubecek

Subject: Re: [PATCH] Clarify expression which uses both multiplication and pointer dereference

On Sun, May 17, 2015 at 07:18:42PM +0200, Alex Dowad wrote:
> This fixes a checkpatch style error in vpfe_buffer_queue_setup.
>
> Signed-off-by: Alex Dowad <[email protected]>
> ---
> drivers/staging/media/davinci_vpfe/vpfe_video.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/media/davinci_vpfe/vpfe_video.c b/drivers/staging/media/davinci_vpfe/vpfe_video.c
> index 06d48d5..04a687c 100644
> --- a/drivers/staging/media/davinci_vpfe/vpfe_video.c
> +++ b/drivers/staging/media/davinci_vpfe/vpfe_video.c
> @@ -1095,7 +1095,7 @@ vpfe_buffer_queue_setup(struct vb2_queue *vq, const struct v4l2_format *fmt,
> size = video->fmt.fmt.pix.sizeimage;
>
> if (vpfe_dev->video_limit) {
> - while (size * *nbuffers > vpfe_dev->video_limit)
> + while (size * (*nbuffers) > vpfe_dev->video_limit)
> (*nbuffers)--;
> }
> if (pipe->state == VPFE_PIPELINE_STREAM_CONTINUOUS) {

Style issue aside, is there a reason not to use

if (size * *nbuffers > vpfe_dev->video_limit)
*nbuffers = vpfe_dev->video_limit / size;

instead?

Michal Kubecek

2015-05-18 08:06:59

by Prabhakar

Subject: Re: [PATCH] Clarify expression which uses both multiplication and pointer dereference

On Mon, May 18, 2015 at 7:51 AM, Michal Kubecek <[email protected]> wrote:
> On Sun, May 17, 2015 at 07:18:42PM +0200, Alex Dowad wrote:
>> This fixes a checkpatch style error in vpfe_buffer_queue_setup.
>>
>> Signed-off-by: Alex Dowad <[email protected]>
>> ---
>> drivers/staging/media/davinci_vpfe/vpfe_video.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/staging/media/davinci_vpfe/vpfe_video.c b/drivers/staging/media/davinci_vpfe/vpfe_video.c
>> index 06d48d5..04a687c 100644
>> --- a/drivers/staging/media/davinci_vpfe/vpfe_video.c
>> +++ b/drivers/staging/media/davinci_vpfe/vpfe_video.c
>> @@ -1095,7 +1095,7 @@ vpfe_buffer_queue_setup(struct vb2_queue *vq, const struct v4l2_format *fmt,
>> size = video->fmt.fmt.pix.sizeimage;
>>
>> if (vpfe_dev->video_limit) {
>> - while (size * *nbuffers > vpfe_dev->video_limit)
>> + while (size * (*nbuffers) > vpfe_dev->video_limit)
>> (*nbuffers)--;
>> }
>> if (pipe->state == VPFE_PIPELINE_STREAM_CONTINUOUS) {
>
> Style issue aside, is there a reason not to use
>
> if (size * *nbuffers > vpfe_dev->video_limit)
> *nbuffers = vpfe_dev->video_limit / size;
>
> instead?
>
I would prefer this.

Cheers,
--Prabhakar Lad

2015-05-18 10:07:29

by Prabhakar

Subject: Re: [PATCH] Clarify expression which uses both multiplication and pointer dereference

Hi Hans,

On Mon, May 18, 2015 at 9:15 AM, Hans Verkuil <[email protected]> wrote:
>
>
> On 05/18/2015 10:06 AM, Lad, Prabhakar wrote:
>> On Mon, May 18, 2015 at 7:51 AM, Michal Kubecek <[email protected]> wrote:
>>> On Sun, May 17, 2015 at 07:18:42PM +0200, Alex Dowad wrote:
>>>> This fixes a checkpatch style error in vpfe_buffer_queue_setup.
>>>>
>>>> Signed-off-by: Alex Dowad <[email protected]>
>>>> ---
>>>> drivers/staging/media/davinci_vpfe/vpfe_video.c | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/staging/media/davinci_vpfe/vpfe_video.c b/drivers/staging/media/davinci_vpfe/vpfe_video.c
>>>> index 06d48d5..04a687c 100644
>>>> --- a/drivers/staging/media/davinci_vpfe/vpfe_video.c
>>>> +++ b/drivers/staging/media/davinci_vpfe/vpfe_video.c
>>>> @@ -1095,7 +1095,7 @@ vpfe_buffer_queue_setup(struct vb2_queue *vq, const struct v4l2_format *fmt,
>>>> size = video->fmt.fmt.pix.sizeimage;
>>>>
>>>> if (vpfe_dev->video_limit) {
>>>> - while (size * *nbuffers > vpfe_dev->video_limit)
>>>> + while (size * (*nbuffers) > vpfe_dev->video_limit)
>>>> (*nbuffers)--;
>>>> }
>>>> if (pipe->state == VPFE_PIPELINE_STREAM_CONTINUOUS) {
>>>
>>> Style issue aside, is there a reason not to use
>>>
>>> if (size * *nbuffers > vpfe_dev->video_limit)
>>> *nbuffers = vpfe_dev->video_limit / size;
>>>
>>> instead?
>>>
>> I would prefer this.
>
> As far as I can see video_limit is never set at all, so this code (and the video_limit
> field) can just be removed.
>
> I think this is a left-over from old code, long since removed.
>
Yes makes sense, I'll fix it up and post a patch for it.

Cheers,
--Prabhakar Lad