Re: [PATCH] soc_camera: match signedness of soc_camera_limit_side()
Hi Guennadi,
Guennadi Liakhovetski wrote:
> Hi Németh
>
> On Tue, 9 Feb 2010, Guennadi Liakhovetski wrote:
>
>> Ok, I modified your patch a bit, how about the below version? If you
>> agree, I'll commit it in that form (after converting to utf-8):
>>
>> From: Márton Németh
>>
>> The first two parameters of soc_camera_limit_side() are usually pointers
>> to struct v4l2_rect elements. They are signed, so adjust the prototype
>> accordingly.
>>
>> This will remove the following sparse warning (see "make C=1"):
>>
>> * incorrect type in argument 1 (different signedness)
>>expected unsigned int *start
>>got signed int *
>>
>> as well as a couple more signedness mismatches.
>>
>> Signed-off-by: Márton Németh
>> Signed-off-by: Guennadi Liakhovetski
>
> please confirm, if you agree with my version of your patch, or I'll have
> to leave it out from my pull request.
I confirm.
Sorry about the late response, I was not able to access my emails for a while.
Regards,
Márton Németh
>> ---
>> diff --git a/drivers/media/video/mt9v022.c b/drivers/media/video/mt9v022.c
>> index 1a34d29..e5bae4c 100644
>> --- a/drivers/media/video/mt9v022.c
>> +++ b/drivers/media/video/mt9v022.c
>> @@ -325,7 +325,7 @@ static int mt9v022_s_crop(struct v4l2_subdev *sd, struct
>> v4l2_crop *a)
>> if (ret < 0)
>> return ret;
>>
>> -dev_dbg(&client->dev, "Frame %ux%u pixel\n", rect.width, rect.height);
>> +dev_dbg(&client->dev, "Frame %dx%d pixel\n", rect.width, rect.height);
>>
>> mt9v022->rect = rect;
>>
>> diff --git a/drivers/media/video/mx3_camera.c
>> b/drivers/media/video/mx3_camera.c
>> index bd297f5..d477e30 100644
>> --- a/drivers/media/video/mx3_camera.c
>> +++ b/drivers/media/video/mx3_camera.c
>> @@ -796,7 +796,7 @@ static int acquire_dma_channel(struct mx3_camera_dev
>> *mx3_cam)
>> * FIXME: learn to use stride != width, then we can keep stride properly
>> aligned
>> * and support arbitrary (even) widths.
>> */
>> -static inline void stride_align(__s32 *width)
>> +static inline void stride_align(__u32 *width)
>> {
>> if (((*width + 7) & ~7) < 4096)
>> *width = (*width + 7) & ~7;
>> @@ -844,7 +844,7 @@ static int mx3_camera_set_crop(struct soc_camera_device
>> *icd,
>> * So far only direct camera-to-memory is supported
>> */
>> if (channel_change_requested(icd, rect)) {
>> -int ret = acquire_dma_channel(mx3_cam);
>> +ret = acquire_dma_channel(mx3_cam);
>> if (ret < 0)
>> return ret;
>> }
>> diff --git a/drivers/media/video/rj54n1cb0c.c
>> b/drivers/media/video/rj54n1cb0c.c
>> index 9277194..bbd9c11 100644
>> --- a/drivers/media/video/rj54n1cb0c.c
>> +++ b/drivers/media/video/rj54n1cb0c.c
>> @@ -555,15 +555,15 @@ static int rj54n1_commit(struct i2c_client *client)
>> return ret;
>> }
>>
>> -static int rj54n1_sensor_scale(struct v4l2_subdev *sd, u32 *in_w, u32 *in_h,
>> - u32 *out_w, u32 *out_h);
>> +static int rj54n1_sensor_scale(struct v4l2_subdev *sd, s32 *in_w, s32 *in_h,
>> + s32 *out_w, s32 *out_h);
>>
>> static int rj54n1_s_crop(struct v4l2_subdev *sd, struct v4l2_crop *a)
>> {
>> struct i2c_client *client = sd->priv;
>> struct rj54n1 *rj54n1 = to_rj54n1(client);
>> struct v4l2_rect *rect = &a->c;
>> -unsigned int dummy = 0, output_w, output_h,
>> +int dummy = 0, output_w, output_h,
>> input_w = rect->width, input_h = rect->height;
>> int ret;
>>
>> @@ -577,7 +577,7 @@ static int rj54n1_s_crop(struct v4l2_subdev *sd, struct
>> v4l2_crop *a)
>> output_w = (input_w * 1024 + rj54n1->resize / 2) / rj54n1->resize;
>> output_h = (input_h * 1024 + rj54n1->resize / 2) / rj54n1->resize;
>>
>> -dev_dbg(&client->dev, "Scaling for %ux%u : %u = %ux%u\n",
>> +dev_dbg(&client->dev, "Scaling for %dx%d : %u = %dx%d\n",
>> input_w, input_h, rj54n1->resize, output_w, output_h);
>>
>> ret = rj54n1_sensor_scale(sd, &input_w, &input_h, &output_w, &output_h);
>> @@ -638,8 +638,8 @@ static int rj54n1_g_fmt(struct v4l2_subdev *sd,
>> * the output one, updates the window sizes and returns an error or the
>> resize
>> * coefficient on success. Note: we only use the "Fixed Scaling" on this
>> camera.
>> */
>> -static int rj54n1_sensor_scale(struct v4l2_subdev *sd, u32 *in_w, u32 *in_h,
>> - u32 *out_w, u32 *out_h)
>> +static int rj54n1_sensor_scale(struct v4l2_subdev *sd, s32 *in_w, s32 *in_h,
>> + s32 *out_w, s32 *out_h)
>> {
>> struct i2c_client *client = sd->priv;
>> struct rj54n1 *rj54n1 = to_rj54n1(client);
>> @@ -749,7 +749,7 @@ static int rj54n1_sensor_scale(struct v4l2_subdev *sd,
>> u32 *in_w, u32 *in_h,
>> * improve the image quality or stability for larger frames (see commen
Re: [PATCH] soc_camera: match signedness of soc_camera_limit_side()
Hi Németh
On Tue, 9 Feb 2010, Guennadi Liakhovetski wrote:
> Ok, I modified your patch a bit, how about the below version? If you
> agree, I'll commit it in that form (after converting to utf-8):
>
> From: Márton Németh
>
> The first two parameters of soc_camera_limit_side() are usually pointers
> to struct v4l2_rect elements. They are signed, so adjust the prototype
> accordingly.
>
> This will remove the following sparse warning (see "make C=1"):
>
> * incorrect type in argument 1 (different signedness)
>expected unsigned int *start
>got signed int *
>
> as well as a couple more signedness mismatches.
>
> Signed-off-by: Márton Németh
> Signed-off-by: Guennadi Liakhovetski
please confirm, if you agree with my version of your patch, or I'll have
to leave it out from my pull request.
Thanks
Guennadi
> ---
> diff --git a/drivers/media/video/mt9v022.c b/drivers/media/video/mt9v022.c
> index 1a34d29..e5bae4c 100644
> --- a/drivers/media/video/mt9v022.c
> +++ b/drivers/media/video/mt9v022.c
> @@ -325,7 +325,7 @@ static int mt9v022_s_crop(struct v4l2_subdev *sd, struct
> v4l2_crop *a)
> if (ret < 0)
> return ret;
>
> - dev_dbg(&client->dev, "Frame %ux%u pixel\n", rect.width, rect.height);
> + dev_dbg(&client->dev, "Frame %dx%d pixel\n", rect.width, rect.height);
>
> mt9v022->rect = rect;
>
> diff --git a/drivers/media/video/mx3_camera.c
> b/drivers/media/video/mx3_camera.c
> index bd297f5..d477e30 100644
> --- a/drivers/media/video/mx3_camera.c
> +++ b/drivers/media/video/mx3_camera.c
> @@ -796,7 +796,7 @@ static int acquire_dma_channel(struct mx3_camera_dev
> *mx3_cam)
> * FIXME: learn to use stride != width, then we can keep stride properly
> aligned
> * and support arbitrary (even) widths.
> */
> -static inline void stride_align(__s32 *width)
> +static inline void stride_align(__u32 *width)
> {
> if (((*width + 7) & ~7) < 4096)
> *width = (*width + 7) & ~7;
> @@ -844,7 +844,7 @@ static int mx3_camera_set_crop(struct soc_camera_device
> *icd,
>* So far only direct camera-to-memory is supported
>*/
> if (channel_change_requested(icd, rect)) {
> - int ret = acquire_dma_channel(mx3_cam);
> + ret = acquire_dma_channel(mx3_cam);
> if (ret < 0)
> return ret;
> }
> diff --git a/drivers/media/video/rj54n1cb0c.c
> b/drivers/media/video/rj54n1cb0c.c
> index 9277194..bbd9c11 100644
> --- a/drivers/media/video/rj54n1cb0c.c
> +++ b/drivers/media/video/rj54n1cb0c.c
> @@ -555,15 +555,15 @@ static int rj54n1_commit(struct i2c_client *client)
> return ret;
> }
>
> -static int rj54n1_sensor_scale(struct v4l2_subdev *sd, u32 *in_w, u32 *in_h,
> -u32 *out_w, u32 *out_h);
> +static int rj54n1_sensor_scale(struct v4l2_subdev *sd, s32 *in_w, s32 *in_h,
> +s32 *out_w, s32 *out_h);
>
> static int rj54n1_s_crop(struct v4l2_subdev *sd, struct v4l2_crop *a)
> {
> struct i2c_client *client = sd->priv;
> struct rj54n1 *rj54n1 = to_rj54n1(client);
> struct v4l2_rect *rect = &a->c;
> - unsigned int dummy = 0, output_w, output_h,
> + int dummy = 0, output_w, output_h,
> input_w = rect->width, input_h = rect->height;
> int ret;
>
> @@ -577,7 +577,7 @@ static int rj54n1_s_crop(struct v4l2_subdev *sd, struct
> v4l2_crop *a)
> output_w = (input_w * 1024 + rj54n1->resize / 2) / rj54n1->resize;
> output_h = (input_h * 1024 + rj54n1->resize / 2) / rj54n1->resize;
>
> - dev_dbg(&client->dev, "Scaling for %ux%u : %u = %ux%u\n",
> + dev_dbg(&client->dev, "Scaling for %dx%d : %u = %dx%d\n",
> input_w, input_h, rj54n1->resize, output_w, output_h);
>
> ret = rj54n1_sensor_scale(sd, &input_w, &input_h, &output_w, &output_h);
> @@ -638,8 +638,8 @@ static int rj54n1_g_fmt(struct v4l2_subdev *sd,
> * the output one, updates the window sizes and returns an error or the
> resize
> * coefficient on success. Note: we only use the "Fixed Scaling" on this
> camera.
> */
> -static int rj54n1_sensor_scale(struct v4l2_subdev *sd, u32 *in_w, u32 *in_h,
> -u32 *out_w, u32 *out_h)
> +static int rj54n1_sensor_scale(struct v4l2_subdev *sd, s32 *in_w, s32 *in_h,
> +s32 *out_w, s32 *out_h)
> {
> struct i2c_client *client = sd->priv;
> struct rj54n1 *rj54n1 = to_rj54n1(client);
> @@ -749,7 +749,7 @@ static int rj54n1_sensor_scale(struct v4l2_subdev *sd,
> u32 *in_w, u32 *in_h,
>* improve the image quality or stability for larger frames (see comment
>* above), but I didn't check the framerate.
>*/
> - skip = min(resize / 1024, (unsigned)15);
> + skip = min(resize / 1024, 15U);
>
> inc_sel = 1 << skip;
>
> @@ -819,7 +819,7 @@ static int rj54n1_
Re: [PATCH] soc_camera: match signedness of soc_camera_limit_side()
On Thu, 28 Jan 2010, Németh Márton wrote:
> Guennadi Liakhovetski wrote:
> > On Wed, 27 Jan 2010, Németh Márton wrote:
> >
> >> Guennadi Liakhovetski wrote:
> >>> You didn't reply to my most important objection:
> >>>
> >>> On Wed, 27 Jan 2010, Németh Márton wrote:
> >>>
> diff -r 31eaa9423f98 linux/include/media/soc_camera.h
> --- a/linux/include/media/soc_camera.h Mon Jan 25 15:04:15 2010 -0200
> +++ b/linux/include/media/soc_camera.h Wed Jan 27 20:49:57 2010 +0100
> @@ -264,9 +264,8 @@
> common_flags;
> }
>
> -static inline void soc_camera_limit_side(unsigned int *start,
> -unsigned int *length, unsigned int start_min,
> -unsigned int length_min, unsigned int length_max)
> +static inline void soc_camera_limit_side(int *start, int *length,
> +int start_min, int length_min, int length_max)
> {
> if (*length < length_min)
> *length = length_min;
> >>> I still do not believe this function will work equally well with signed
> >>> parameters, as it works with unsigned ones.
> >> I implemented some test cases to find out whether the
> >> soc_camera_limit_side() works correctly or not. My biggest problem is that
> >> I'm
> >> not sure what is the expected working of the soc_camera_limit_side()
> >> function.
> >
> > Well, the expected behaviour is simple: the function treats all its
> > parameters as unsigned, and puts the former two input/output parameters
> > within the limits, provided by the latter three parameters. Well, taking
>
> For the length parameter it is clear to put them between length_min and
> length_max.
> But for start there is only one limit given: start_min. Does this mean that
> any *start value bigger than start_min is acceptable?
Isn't this not clear enough?
if (*start < start_min)
*start = start_min;
else if (*start > start_min + length_max - *length)
*start = start_min + length_max - *length;
> (I would like to find out the meaning, not to read back what is written in
> the source code because it is no use to define test cases out of the source
> code.)
>
> > into account, that when comparing a signed and an unsigned integers, the
> > comparison is performed unsigned, I think, it should be ok to do what I
> > suggested in the last email: change prototype to
> >
> > +static inline void soc_camera_limit_side(int *start, int *length,
> > + unsigned int start_min, unsigned int length_min,
> > + unsigned int length_max)
> >
> > Maybe also provide a comment above the function explaining, why the first
> > two parameters are signed. And cast explicitly in sh_mobile_ceu_camera.c:
> >
> > soc_camera_limit_side(&rect->left, &rect->width,
> > (unsigned int)cap.bounds.left, 2,
> > (unsigned int)cap.bounds.width);
> > soc_camera_limit_side(&rect->top, &rect->height,
> > (unsigned int)cap.bounds.top, 4,
> > (unsigned int)cap.bounds.height);
>
> I'm afraid that casting __s32 to unsigned int just cannot work. Let's take an
> example on a 32bit machine:
>
>-1 = 0x
> (unsigned int)-1 = 0x = 4294967295
>
> and
>
>-2147483648 = 0x8000
> (unsigned int)-2147483648 = 0x8000 = 2147483648
>
> This means that any negative number will be mapped to a large positive number
> when casting to (unsigned int) and I think this is not the wanted behaviour.
That's exactly the expected behaviour.
> > Could you check if this would make both sparse and the compiler happy?
>
> There is no compiler warning nor sparse warning when applying the following
> version of the patch. I'm not sure, however, that the simple cast will do
> the right thing here.
Ok, I modified your patch a bit, how about the below version? If you
agree, I'll commit it in that form (after converting to utf-8):
From: Márton Németh
The first two parameters of soc_camera_limit_side() are usually pointers
to struct v4l2_rect elements. They are signed, so adjust the prototype
accordingly.
This will remove the following sparse warning (see "make C=1"):
* incorrect type in argument 1 (different signedness)
expected unsigned int *start
got signed int *
as well as a couple more signedness mismatches.
Signed-off-by: Márton Németh
Signed-off-by: Guennadi Liakhovetski
---
diff --git a/drivers/media/video/mt9v022.c b/drivers/media/video/mt9v022.c
index 1a34d29..e5bae4c 100644
--- a/drivers/media/video/mt9v022.c
+++ b/drivers/media/video/mt9v022.c
@@ -325,7 +325,7 @@ static int mt9v022_s_crop(struct v4l2_subdev *sd, struct
v4l2_crop *a)
if (ret < 0)
return ret;
- dev_dbg(&client->dev, "Frame %ux%u pixel\n", rect.width, rect.height);
+ dev_dbg(&client->dev, "Frame %dx%d pi
Re: [PATCH] soc_camera: match signedness of soc_camera_limit_side()
Guennadi Liakhovetski wrote:
> On Wed, 27 Jan 2010, Németh Márton wrote:
>
>> Guennadi Liakhovetski wrote:
>>> You didn't reply to my most important objection:
>>>
>>> On Wed, 27 Jan 2010, Németh Márton wrote:
>>>
diff -r 31eaa9423f98 linux/include/media/soc_camera.h
--- a/linux/include/media/soc_camera.h Mon Jan 25 15:04:15 2010 -0200
+++ b/linux/include/media/soc_camera.h Wed Jan 27 20:49:57 2010 +0100
@@ -264,9 +264,8 @@
common_flags;
}
-static inline void soc_camera_limit_side(unsigned int *start,
- unsigned int *length, unsigned int start_min,
- unsigned int length_min, unsigned int length_max)
+static inline void soc_camera_limit_side(int *start, int *length,
+ int start_min, int length_min, int length_max)
{
if (*length < length_min)
*length = length_min;
>>> I still do not believe this function will work equally well with signed
>>> parameters, as it works with unsigned ones.
>> I implemented some test cases to find out whether the
>> soc_camera_limit_side() works correctly or not. My biggest problem is that
>> I'm
>> not sure what is the expected working of the soc_camera_limit_side()
>> function.
>
> Well, the expected behaviour is simple: the function treats all its
> parameters as unsigned, and puts the former two input/output parameters
> within the limits, provided by the latter three parameters. Well, taking
For the length parameter it is clear to put them between length_min and
length_max.
But for start there is only one limit given: start_min. Does this mean that
any *start value bigger than start_min is acceptable?
(I would like to find out the meaning, not to read back what is written in
the source code because it is no use to define test cases out of the source
code.)
> into account, that when comparing a signed and an unsigned integers, the
> comparison is performed unsigned, I think, it should be ok to do what I
> suggested in the last email: change prototype to
>
> +static inline void soc_camera_limit_side(int *start, int *length,
> + unsigned int start_min, unsigned int length_min,
> + unsigned int length_max)
>
> Maybe also provide a comment above the function explaining, why the first
> two parameters are signed. And cast explicitly in sh_mobile_ceu_camera.c:
>
> soc_camera_limit_side(&rect->left, &rect->width,
> (unsigned int)cap.bounds.left, 2,
> (unsigned int)cap.bounds.width);
> soc_camera_limit_side(&rect->top, &rect->height,
> (unsigned int)cap.bounds.top, 4,
> (unsigned int)cap.bounds.height);
I'm afraid that casting __s32 to unsigned int just cannot work. Let's take an
example on a 32bit machine:
-1 = 0x
(unsigned int)-1 = 0x = 4294967295
and
-2147483648 = 0x8000
(unsigned int)-2147483648 = 0x8000 = 2147483648
This means that any negative number will be mapped to a large positive number
when casting to (unsigned int) and I think this is not the wanted behaviour.
> Could you check if this would make both sparse and the compiler happy?
There is no compiler warning nor sparse warning when applying the following
version of the patch. I'm not sure, however, that the simple cast will do
the right thing here.
Regards,
Márton Németh
---
From: Márton Németh
The parameters of soc_camera_limit_side() are either a pointer to
a structure element from v4l2_rect, or constants. The structure elements
of the v4l2_rect are signed (see ) so do the computations
also with signed values.
The *s_crop() functions may receive negative numbers through the c field of
struct v4l2_crop. These negative values then limited by the start_min and
length_min parameters of soc_camera_limit_side().
This will remove the following sparse warning (see "make C=1"):
* incorrect type in argument 1 (different signedness)
expected unsigned int *start
got signed int *
Signed-off-by: Márton Németh
---
diff -r 31eaa9423f98 linux/drivers/media/video/mt9v022.c
--- a/linux/drivers/media/video/mt9v022.c Mon Jan 25 15:04:15 2010 -0200
+++ b/linux/drivers/media/video/mt9v022.c Thu Jan 28 22:24:35 2010 +0100
@@ -326,7 +326,7 @@
if (ret < 0)
return ret;
- dev_dbg(&client->dev, "Frame %ux%u pixel\n", rect.width, rect.height);
+ dev_dbg(&client->dev, "Frame %dx%d pixel\n", rect.width, rect.height);
mt9v022->rect = rect;
diff -r 31eaa9423f98 linux/drivers/media/video/rj54n1cb0c.c
--- a/linux/drivers/media/video/rj54n1cb0c.cMon Jan 25 15:04:15 2010 -0200
+++ b/linux/drivers/media/video/rj54n1cb0c.cThu Jan 28 22:24:35 2010 +0100
@@ -555,15 +555,15 @@
return ret;
}
-static int rj54n1_sensor_scale(struct v4l2_subdev *sd, u32 *in_w, u32 *in_h,
-
Re: [PATCH] soc_camera: match signedness of soc_camera_limit_side()
On Wed, 27 Jan 2010, Németh Márton wrote:
> Guennadi Liakhovetski wrote:
> > You didn't reply to my most important objection:
> >
> > On Wed, 27 Jan 2010, Németh Márton wrote:
> >
> >> diff -r 31eaa9423f98 linux/include/media/soc_camera.h
> >> --- a/linux/include/media/soc_camera.h Mon Jan 25 15:04:15 2010 -0200
> >> +++ b/linux/include/media/soc_camera.h Wed Jan 27 20:49:57 2010 +0100
> >> @@ -264,9 +264,8 @@
> >>common_flags;
> >> }
> >>
> >> -static inline void soc_camera_limit_side(unsigned int *start,
> >> - unsigned int *length, unsigned int start_min,
> >> - unsigned int length_min, unsigned int length_max)
> >> +static inline void soc_camera_limit_side(int *start, int *length,
> >> + int start_min, int length_min, int length_max)
> >> {
> >>if (*length < length_min)
> >>*length = length_min;
> >
> > I still do not believe this function will work equally well with signed
> > parameters, as it works with unsigned ones.
>
> I implemented some test cases to find out whether the
> soc_camera_limit_side() works correctly or not. My biggest problem is that I'm
> not sure what is the expected working of the soc_camera_limit_side() function.
Well, the expected behaviour is simple: the function treats all its
parameters as unsigned, and puts the former two input/output parameters
within the limits, provided by the latter three parameters. Well, taking
into account, that when comparing a signed and an unsigned integers, the
comparison is performed unsigned, I think, it should be ok to do what I
suggested in the last email: change prototype to
+static inline void soc_camera_limit_side(int *start, int *length,
+ unsigned int start_min, unsigned int length_min,
+ unsigned int length_max)
Maybe also provide a comment above the function explaining, why the first
two parameters are signed. And cast explicitly in sh_mobile_ceu_camera.c:
soc_camera_limit_side(&rect->left, &rect->width,
(unsigned int)cap.bounds.left, 2,
(unsigned int)cap.bounds.width);
soc_camera_limit_side(&rect->top, &rect->height,
(unsigned int)cap.bounds.top, 4,
(unsigned int)cap.bounds.height);
Could you check if this would make both sparse and the compiler happy?
Thanks
Guennadi
>
> Nevertheless I tried expect some values, you can probably verify whether my
> expectations are correct or not (see the test attached).
>
> The signed and unsigned version of the function works differently because
> the unsigned version cannot accept negative values. These values will be
> implicitly casted to an unsigned value which means that they will be
> interpreted
> as a big positive value.
>
> Here are the test results:
>
> Test Case 1: PASSED
> Test Case 2: PASSED
> Test Case 3: FAILED: start=50, length=8, start_unsigned=0,
> length_unsigned=1600
> Test Case 4: PASSED
> Test Case 5: PASSED
> Test Case 6: PASSED
> Test Case 7: PASSED
> Test Case 8: PASSED
>
> There is a difference in case 3, but which is the correct one?
>
> Regard,
>
> Márton Németh
>
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] soc_camera: match signedness of soc_camera_limit_side()
Guennadi Liakhovetski wrote:
> You didn't reply to my most important objection:
>
> On Wed, 27 Jan 2010, Németh Márton wrote:
>
>> diff -r 31eaa9423f98 linux/include/media/soc_camera.h
>> --- a/linux/include/media/soc_camera.h Mon Jan 25 15:04:15 2010 -0200
>> +++ b/linux/include/media/soc_camera.h Wed Jan 27 20:49:57 2010 +0100
>> @@ -264,9 +264,8 @@
>> common_flags;
>> }
>>
>> -static inline void soc_camera_limit_side(unsigned int *start,
>> -unsigned int *length, unsigned int start_min,
>> -unsigned int length_min, unsigned int length_max)
>> +static inline void soc_camera_limit_side(int *start, int *length,
>> +int start_min, int length_min, int length_max)
>> {
>> if (*length < length_min)
>> *length = length_min;
>
> I still do not believe this function will work equally well with signed
> parameters, as it works with unsigned ones.
I implemented some test cases to find out whether the
soc_camera_limit_side() works correctly or not. My biggest problem is that I'm
not sure what is the expected working of the soc_camera_limit_side() function.
Nevertheless I tried expect some values, you can probably verify whether my
expectations are correct or not (see the test attached).
The signed and unsigned version of the function works differently because
the unsigned version cannot accept negative values. These values will be
implicitly casted to an unsigned value which means that they will be interpreted
as a big positive value.
Here are the test results:
Test Case 1: PASSED
Test Case 2: PASSED
Test Case 3: FAILED: start=50, length=8, start_unsigned=0, length_unsigned=1600
Test Case 4: PASSED
Test Case 5: PASSED
Test Case 6: PASSED
Test Case 7: PASSED
Test Case 8: PASSED
There is a difference in case 3, but which is the correct one?
Regard,
Márton Németh
#include
static inline void soc_camera_limit_side_UNSIGNED(unsigned int *start, unsigned int *length,
unsigned int start_min, unsigned int length_min, unsigned int length_max)
{
if (*length < length_min)
*length = length_min;
else if (*length > length_max)
*length = length_max;
if (*start < start_min)
*start = start_min;
else if (*start > start_min + length_max - *length)
*start = start_min + length_max - *length;
}
static inline void soc_camera_limit_side(int *start, int *length,
int start_min, int length_min, int length_max)
{
if (*length < length_min)
*length = length_min;
else if (*length > length_max)
*length = length_max;
if (*start < start_min)
*start = start_min;
else if (*start > start_min + length_max - *length)
*start = start_min + length_max - *length;
}
int main() {
int start, length;
unsigned int start_unsigned, length_unsigned;
printf("Test Case 1: ");
start = 0;
length = 8;
start_unsigned = start;
length_unsigned = length;
soc_camera_limit_side(&start, &length, 0, 8, 1600);
soc_camera_limit_side_UNSIGNED(&start_unsigned, &length_unsigned, 0, 8, 1600);
if (start == 0 && length == 8 && start_unsigned == start && length_unsigned == length) {
printf("PASSED\n");
} else {
printf("FAILED: start=%i, length=%i, start_unsigned=%i, length_unsigned=%i\n", start, length, start_unsigned, length_unsigned);
}
printf("Test Case 2: ");
start = -5;
length = 1600;
start_unsigned = start;
length_unsigned = length;
soc_camera_limit_side(&start, &length, 0, 8, 1600);
soc_camera_limit_side_UNSIGNED(&start_unsigned, &length_unsigned, 0, 8, 1600);
if (start == 0 && length == 1600 && start_unsigned == start && length_unsigned == length) {
printf("PASSED\n");
} else {
printf("FAILED: start=%i, length=%i, start_unsigned=%i, length_unsigned=%i\n", start, length, start_unsigned, length_unsigned);
}
printf("Test Case 3: ");
start = 50;
length = -15;
start_unsigned = start;
length_unsigned = length;
soc_camera_limit_side(&start, &length, 0, 8, 1600);
soc_camera_limit_side_UNSIGNED(&start_unsigned, &length_unsigned, 0, 8, 1600);
if (start == 50 && length == 8 && start_unsigned == start && length_unsigned == length) {
printf("PASSED\n");
} else {
printf("FAILED: start=%i, length=%i, start_unsigned=%i, length_unsigned=%i\n", start, length, start_unsigned, length_unsigned);
}
printf("Test Case 4: ");
start = 500;
length = 2000;
start_unsigned = start;
length_unsigned = length;
soc_camera_limit_side(&start, &length, 0, 8, 1600);
soc_camera_limit_side_UNSIGNED(&start_unsigned, &length_unsigned, 0, 8, 1600);
if (start == 0 && length == 1600 && start_unsigned == start && length_unsigned == length) {
printf("PASSED\n");
} else {
printf("FAILED: start=%i, length=%i, start_unsigned=%i, length_unsigned=%i\n", start, length, start_unsigned, length_unsigned);
}
printf("Test Case 5: ");
start = -20;
length = 1600;
start_unsigned = start;
length_unsigned = length;
soc_camera_limit_side(&start, &length, 100, 8, 1600);
soc_camera_limit_side_UNSIGNED(&start_unsigned, &length_unsigned, 100,
Re: [PATCH] soc_camera: match signedness of soc_camera_limit_side()
You didn't reply to my most important objection:
On Wed, 27 Jan 2010, Németh Márton wrote:
> diff -r 31eaa9423f98 linux/include/media/soc_camera.h
> --- a/linux/include/media/soc_camera.hMon Jan 25 15:04:15 2010 -0200
> +++ b/linux/include/media/soc_camera.hWed Jan 27 20:49:57 2010 +0100
> @@ -264,9 +264,8 @@
> common_flags;
> }
>
> -static inline void soc_camera_limit_side(unsigned int *start,
> - unsigned int *length, unsigned int start_min,
> - unsigned int length_min, unsigned int length_max)
> +static inline void soc_camera_limit_side(int *start, int *length,
> + int start_min, int length_min, int length_max)
> {
> if (*length < length_min)
> *length = length_min;
I still do not believe this function will work equally well with signed
parameters, as it works with unsigned ones.
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] soc_camera: match signedness of soc_camera_limit_side()
From: Márton Németh
The parameters of soc_camera_limit_side() are either a pointer to
a structure element from v4l2_rect, or constants. The structure elements
of the v4l2_rect are signed (see ) so do the computations
also with signed values.
The *s_crop() functions may receive negative numbers through the c field of
struct v4l2_crop. These negative values then limited by the start_min and
length_min parameters of soc_camera_limit_side().
This will remove the following sparse warning (see "make C=1"):
* incorrect type in argument 1 (different signedness)
expected unsigned int *start
got signed int *
Signed-off-by: Márton Németh
---
diff -r 31eaa9423f98 linux/drivers/media/video/mt9v022.c
--- a/linux/drivers/media/video/mt9v022.c Mon Jan 25 15:04:15 2010 -0200
+++ b/linux/drivers/media/video/mt9v022.c Wed Jan 27 20:49:57 2010 +0100
@@ -326,7 +326,7 @@
if (ret < 0)
return ret;
- dev_dbg(&client->dev, "Frame %ux%u pixel\n", rect.width, rect.height);
+ dev_dbg(&client->dev, "Frame %dx%d pixel\n", rect.width, rect.height);
mt9v022->rect = rect;
diff -r 31eaa9423f98 linux/drivers/media/video/rj54n1cb0c.c
--- a/linux/drivers/media/video/rj54n1cb0c.cMon Jan 25 15:04:15 2010 -0200
+++ b/linux/drivers/media/video/rj54n1cb0c.cWed Jan 27 20:49:57 2010 +0100
@@ -555,15 +555,15 @@
return ret;
}
-static int rj54n1_sensor_scale(struct v4l2_subdev *sd, u32 *in_w, u32 *in_h,
- u32 *out_w, u32 *out_h);
+static int rj54n1_sensor_scale(struct v4l2_subdev *sd, s32 *in_w, s32 *in_h,
+ s32 *out_w, s32 *out_h);
static int rj54n1_s_crop(struct v4l2_subdev *sd, struct v4l2_crop *a)
{
struct i2c_client *client = sd->priv;
struct rj54n1 *rj54n1 = to_rj54n1(client);
struct v4l2_rect *rect = &a->c;
- unsigned int dummy = 0, output_w, output_h,
+ int dummy = 0, output_w, output_h,
input_w = rect->width, input_h = rect->height;
int ret;
@@ -577,7 +577,7 @@
output_w = (input_w * 1024 + rj54n1->resize / 2) / rj54n1->resize;
output_h = (input_h * 1024 + rj54n1->resize / 2) / rj54n1->resize;
- dev_dbg(&client->dev, "Scaling for %ux%u : %u = %ux%u\n",
+ dev_dbg(&client->dev, "Scaling for %dx%d : %d = %dx%d\n",
input_w, input_h, rj54n1->resize, output_w, output_h);
ret = rj54n1_sensor_scale(sd, &input_w, &input_h, &output_w, &output_h);
@@ -638,12 +638,12 @@
* the output one, updates the window sizes and returns an error or the resize
* coefficient on success. Note: we only use the "Fixed Scaling" on this
camera.
*/
-static int rj54n1_sensor_scale(struct v4l2_subdev *sd, u32 *in_w, u32 *in_h,
- u32 *out_w, u32 *out_h)
+static int rj54n1_sensor_scale(struct v4l2_subdev *sd, s32 *in_w, s32 *in_h,
+ s32 *out_w, s32 *out_h)
{
struct i2c_client *client = sd->priv;
struct rj54n1 *rj54n1 = to_rj54n1(client);
- unsigned int skip, resize, input_w = *in_w, input_h = *in_h,
+ int skip, resize, input_w = *in_w, input_h = *in_h,
output_w = *out_w, output_h = *out_h;
u16 inc_sel, wb_bit8, wb_left, wb_right, wb_top, wb_bottom;
unsigned int peak, peak_50, peak_60;
@@ -655,7 +655,7 @@
* case we have to either reduce the input window to equal or below
* 512x384 or the output window to equal or below 1/2 of the input.
*/
- if (output_w > max(512U, input_w / 2)) {
+ if (output_w > max(512, input_w / 2)) {
if (2 * output_w > RJ54N1_MAX_WIDTH) {
input_w = RJ54N1_MAX_WIDTH;
output_w = RJ54N1_MAX_WIDTH / 2;
@@ -663,11 +663,11 @@
input_w = output_w * 2;
}
- dev_dbg(&client->dev, "Adjusted output width: in %u, out %u\n",
+ dev_dbg(&client->dev, "Adjusted output width: in %d, out %d\n",
input_w, output_w);
}
- if (output_h > max(384U, input_h / 2)) {
+ if (output_h > max(384, input_h / 2)) {
if (2 * output_h > RJ54N1_MAX_HEIGHT) {
input_h = RJ54N1_MAX_HEIGHT;
output_h = RJ54N1_MAX_HEIGHT / 2;
@@ -675,7 +675,7 @@
input_h = output_h * 2;
}
- dev_dbg(&client->dev, "Adjusted output height: in %u, out %u\n",
+ dev_dbg(&client->dev, "Adjusted output height: in %d, out %d\n",
input_h, output_h);
}
@@ -749,7 +749,7 @@
* improve the image quality or stability for larger frames (see comment
* above), but I didn't check the framerate.
*/
- skip = min(resize / 1024, (unsigned)15);
+ skip = min(resize / 1024, 15);
inc_sel = 1 << skip;
@@ -819,7 +819,7 @@
*out_w
Re: [PATCH] soc_camera: match signedness of soc_camera_limit_side()
On Wed, 27 Jan 2010, Németh Márton wrote:
> Guennadi Liakhovetski wrote:
[snip]
> > And these:
> >
> > if (output_w > max(512U, input_w / 2)) {
> > if (output_h > max(384U, input_h / 2)) {
> >
> > would now produce compiler warnings... Have you actually tried to compile
> > your patch? You'll also have to change formats in dev_dbg() calls here...
>
> Interesting to hear about compiler warnings. I don't get them if I apply the
> patch
> on top of version 14064:31eaa9423f98 of http://linuxtv.org/hg/v4l-dvb/ . What
> is your compiler version?
Well, it's my built-in mental compiler, I haven't started versioning it
yet;) Strange, that (your) compiler doesn't complain - max() does
type-checking and now it's signed against unsigned, hm... In any case,
that one is easy to fix - just remove the "U"s, but I'm wondering why the
compiler didn't shout and whether there can be other similar mismatches...
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] soc_camera: match signedness of soc_camera_limit_side()
Guennadi Liakhovetski wrote:
> On Sat, 23 Jan 2010, Németh Márton wrote:
>
>> From: Márton Németh
>>
>> The parameters of soc_camera_limit_side() are either a pointer to
>> a structure element from v4l2_rect, or constants. The structure elements
>> of the v4l2_rect are signed (see ) so do the computations
>> also with signed values.
>>
>> This will remove the following sparse warning (see "make C=1"):
>> * incorrect type in argument 1 (different signedness)
>>expected unsigned int *start
>>got signed int *
>
> Well, it is interesting, but insufficient. And, unfortunately, I don't
> have a good (and easy) recipe for how to fix this properly.
>
> The problem is, that in soc_camera_limit_side all tests and arithmetics
> are performed with unsigned in mind, now, if you change them to signed,
> think what happens, if some of them are negative. No, I don't know when
> negative members of struct v4l2_rect make sense, having them signed
> doesn't seem a very good idea to me. But they cannot be changed - that's a
> part of the user-space API...
>
> Casting all parameters inside that inline to unsigned would be way too
> ugly. Maybe we could at least keep start_min, length_min, and length_max
> unsigned, and only change start and length to signed, and only cast those
> two inside the function. Then, if you grep through all the drivers,
> there's only one location, where soc_camera_limit_side() is called with
> the latter 3 parameters not constant - two calls in
> sh_mobile_ceu_camera.c. So, to keep sparse happy, you'd have to cast
> there. Ideally, you would also add checks there for negative values...
I'll have a look to see what can be done to handle the negative values properly.
>> Signed-off-by: Márton Németh
>> ---
>> diff -r 2a50a0a1c951 linux/include/media/soc_camera.h
>> --- a/linux/include/media/soc_camera.h Sat Jan 23 00:14:32 2010 -0200
>> +++ b/linux/include/media/soc_camera.h Sat Jan 23 10:09:41 2010 +0100
>> @@ -264,9 +264,8 @@
>> common_flags;
>> }
>>
>> -static inline void soc_camera_limit_side(unsigned int *start,
>> -unsigned int *length, unsigned int start_min,
>> -unsigned int length_min, unsigned int length_max)
>> +static inline void soc_camera_limit_side(int *start, int *length,
>> +int start_min, int length_min, int length_max)
>> {
>> if (*length < length_min)
>> *length = length_min;
>> diff -r 2a50a0a1c951 linux/drivers/media/video/rj54n1cb0c.c
>> --- a/linux/drivers/media/video/rj54n1cb0c.c Sat Jan 23 00:14:32 2010 -0200
>> +++ b/linux/drivers/media/video/rj54n1cb0c.c Sat Jan 23 10:09:41 2010 +0100
>> @@ -555,15 +555,15 @@
>> return ret;
>> }
>>
>> -static int rj54n1_sensor_scale(struct v4l2_subdev *sd, u32 *in_w, u32 *in_h,
>> - u32 *out_w, u32 *out_h);
>> +static int rj54n1_sensor_scale(struct v4l2_subdev *sd, s32 *in_w, s32 *in_h,
>> + s32 *out_w, s32 *out_h);
>>
>> static int rj54n1_s_crop(struct v4l2_subdev *sd, struct v4l2_crop *a)
>> {
>> struct i2c_client *client = sd->priv;
>> struct rj54n1 *rj54n1 = to_rj54n1(client);
>> struct v4l2_rect *rect = &a->c;
>> -unsigned int dummy = 0, output_w, output_h,
>> +int dummy = 0, output_w, output_h,
>> input_w = rect->width, input_h = rect->height;
>> int ret;
>
> And these:
>
> if (output_w > max(512U, input_w / 2)) {
> if (output_h > max(384U, input_h / 2)) {
>
> would now produce compiler warnings... Have you actually tried to compile
> your patch? You'll also have to change formats in dev_dbg() calls here...
Interesting to hear about compiler warnings. I don't get them if I apply the
patch
on top of version 14064:31eaa9423f98 of http://linuxtv.org/hg/v4l-dvb/ . What
is your compiler version?
I used the attached configuration. Maybe some other options has to be
turned on?
localhost:/usr/src/linuxtv.org/v4l-dvb$ patch -p1
<../soc_camera_signedness.patch
patching file linux/include/media/soc_camera.h
patching file linux/drivers/media/video/rj54n1cb0c.c
localhost:/usr/src/linuxtv.org/v4l-dvb$ make C=1 2>&1 |tee result1.txt
make -C /usr/src/linuxtv.org/v4l-dvb/v4l
make[1]: Entering directory `/usr/src/linuxtv.org/v4l-dvb/v4l'
creating symbolic links...
make -C firmware prep
make[2]: Entering directory `/usr/src/linuxtv.org/v4l-dvb/v4l/firmware'
make[2]: Leaving directory `/usr/src/linuxtv.org/v4l-dvb/v4l/firmware'
make -C firmware
make[2]: Entering directory `/usr/src/linuxtv.org/v4l-dvb/v4l/firmware'
make[2]: Nothing to be done for `default'.
make[2]: Leaving directory `/usr/src/linuxtv.org/v4l-dvb/v4l/firmware'
Kernel build directory is /lib/modules/2.6.33-rc4/build
make -C /lib/modules/2.6.33-rc4/build SUBDIRS=/usr/src/linuxtv.org/v4l-dvb/v4l
modules
make[2]: Entering directory `/usr/src/linuxtv.org/pinchartl/uvcvideo/v4l-dvb'
CHECK /usr/src/linuxtv.org/v4l-dvb/v4l/mt9m001.c
CC [M] /usr/src/l
Re: [PATCH] soc_camera: match signedness of soc_camera_limit_side()
On Sat, 23 Jan 2010, Németh Márton wrote:
> From: Márton Németh
>
> The parameters of soc_camera_limit_side() are either a pointer to
> a structure element from v4l2_rect, or constants. The structure elements
> of the v4l2_rect are signed (see ) so do the computations
> also with signed values.
>
> This will remove the following sparse warning (see "make C=1"):
> * incorrect type in argument 1 (different signedness)
>expected unsigned int *start
>got signed int *
Well, it is interesting, but insufficient. And, unfortunately, I don't
have a good (and easy) recipe for how to fix this properly.
The problem is, that in soc_camera_limit_side all tests and arithmetics
are performed with unsigned in mind, now, if you change them to signed,
think what happens, if some of them are negative. No, I don't know when
negative members of struct v4l2_rect make sense, having them signed
doesn't seem a very good idea to me. But they cannot be changed - that's a
part of the user-space API...
Casting all parameters inside that inline to unsigned would be way too
ugly. Maybe we could at least keep start_min, length_min, and length_max
unsigned, and only change start and length to signed, and only cast those
two inside the function. Then, if you grep through all the drivers,
there's only one location, where soc_camera_limit_side() is called with
the latter 3 parameters not constant - two calls in
sh_mobile_ceu_camera.c. So, to keep sparse happy, you'd have to cast
there. Ideally, you would also add checks there for negative values...
>
> Signed-off-by: Márton Németh
> ---
> diff -r 2a50a0a1c951 linux/include/media/soc_camera.h
> --- a/linux/include/media/soc_camera.hSat Jan 23 00:14:32 2010 -0200
> +++ b/linux/include/media/soc_camera.hSat Jan 23 10:09:41 2010 +0100
> @@ -264,9 +264,8 @@
> common_flags;
> }
>
> -static inline void soc_camera_limit_side(unsigned int *start,
> - unsigned int *length, unsigned int start_min,
> - unsigned int length_min, unsigned int length_max)
> +static inline void soc_camera_limit_side(int *start, int *length,
> + int start_min, int length_min, int length_max)
> {
> if (*length < length_min)
> *length = length_min;
> diff -r 2a50a0a1c951 linux/drivers/media/video/rj54n1cb0c.c
> --- a/linux/drivers/media/video/rj54n1cb0c.c Sat Jan 23 00:14:32 2010 -0200
> +++ b/linux/drivers/media/video/rj54n1cb0c.c Sat Jan 23 10:09:41 2010 +0100
> @@ -555,15 +555,15 @@
> return ret;
> }
>
> -static int rj54n1_sensor_scale(struct v4l2_subdev *sd, u32 *in_w, u32 *in_h,
> -u32 *out_w, u32 *out_h);
> +static int rj54n1_sensor_scale(struct v4l2_subdev *sd, s32 *in_w, s32 *in_h,
> +s32 *out_w, s32 *out_h);
>
> static int rj54n1_s_crop(struct v4l2_subdev *sd, struct v4l2_crop *a)
> {
> struct i2c_client *client = sd->priv;
> struct rj54n1 *rj54n1 = to_rj54n1(client);
> struct v4l2_rect *rect = &a->c;
> - unsigned int dummy = 0, output_w, output_h,
> + int dummy = 0, output_w, output_h,
> input_w = rect->width, input_h = rect->height;
> int ret;
And these:
if (output_w > max(512U, input_w / 2)) {
if (output_h > max(384U, input_h / 2)) {
would now produce compiler warnings... Have you actually tried to compile
your patch? You'll also have to change formats in dev_dbg() calls here...
>
> @@ -638,8 +638,8 @@
> * the output one, updates the window sizes and returns an error or the
> resize
> * coefficient on success. Note: we only use the "Fixed Scaling" on this
> camera.
> */
> -static int rj54n1_sensor_scale(struct v4l2_subdev *sd, u32 *in_w, u32 *in_h,
> -u32 *out_w, u32 *out_h)
> +static int rj54n1_sensor_scale(struct v4l2_subdev *sd, s32 *in_w, s32 *in_h,
> +s32 *out_w, s32 *out_h)
> {
> struct i2c_client *client = sd->priv;
> struct rj54n1 *rj54n1 = to_rj54n1(client);
> @@ -1017,7 +1017,7 @@
> struct i2c_client *client = sd->priv;
> struct rj54n1 *rj54n1 = to_rj54n1(client);
> const struct rj54n1_datafmt *fmt;
> - unsigned int output_w, output_h, max_w, max_h,
> + int output_w, output_h, max_w, max_h,
> input_w = rj54n1->rect.width, input_h = rj54n1->rect.height;
> int ret;
and here.
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] soc_camera: match signedness of soc_camera_limit_side()
From: Márton Németh
The parameters of soc_camera_limit_side() are either a pointer to
a structure element from v4l2_rect, or constants. The structure elements
of the v4l2_rect are signed (see ) so do the computations
also with signed values.
This will remove the following sparse warning (see "make C=1"):
* incorrect type in argument 1 (different signedness)
expected unsigned int *start
got signed int *
Signed-off-by: Márton Németh
---
diff -r 2a50a0a1c951 linux/include/media/soc_camera.h
--- a/linux/include/media/soc_camera.h Sat Jan 23 00:14:32 2010 -0200
+++ b/linux/include/media/soc_camera.h Sat Jan 23 10:09:41 2010 +0100
@@ -264,9 +264,8 @@
common_flags;
}
-static inline void soc_camera_limit_side(unsigned int *start,
- unsigned int *length, unsigned int start_min,
- unsigned int length_min, unsigned int length_max)
+static inline void soc_camera_limit_side(int *start, int *length,
+ int start_min, int length_min, int length_max)
{
if (*length < length_min)
*length = length_min;
diff -r 2a50a0a1c951 linux/drivers/media/video/rj54n1cb0c.c
--- a/linux/drivers/media/video/rj54n1cb0c.cSat Jan 23 00:14:32 2010 -0200
+++ b/linux/drivers/media/video/rj54n1cb0c.cSat Jan 23 10:09:41 2010 +0100
@@ -555,15 +555,15 @@
return ret;
}
-static int rj54n1_sensor_scale(struct v4l2_subdev *sd, u32 *in_w, u32 *in_h,
- u32 *out_w, u32 *out_h);
+static int rj54n1_sensor_scale(struct v4l2_subdev *sd, s32 *in_w, s32 *in_h,
+ s32 *out_w, s32 *out_h);
static int rj54n1_s_crop(struct v4l2_subdev *sd, struct v4l2_crop *a)
{
struct i2c_client *client = sd->priv;
struct rj54n1 *rj54n1 = to_rj54n1(client);
struct v4l2_rect *rect = &a->c;
- unsigned int dummy = 0, output_w, output_h,
+ int dummy = 0, output_w, output_h,
input_w = rect->width, input_h = rect->height;
int ret;
@@ -638,8 +638,8 @@
* the output one, updates the window sizes and returns an error or the resize
* coefficient on success. Note: we only use the "Fixed Scaling" on this
camera.
*/
-static int rj54n1_sensor_scale(struct v4l2_subdev *sd, u32 *in_w, u32 *in_h,
- u32 *out_w, u32 *out_h)
+static int rj54n1_sensor_scale(struct v4l2_subdev *sd, s32 *in_w, s32 *in_h,
+ s32 *out_w, s32 *out_h)
{
struct i2c_client *client = sd->priv;
struct rj54n1 *rj54n1 = to_rj54n1(client);
@@ -1017,7 +1017,7 @@
struct i2c_client *client = sd->priv;
struct rj54n1 *rj54n1 = to_rj54n1(client);
const struct rj54n1_datafmt *fmt;
- unsigned int output_w, output_h, max_w, max_h,
+ int output_w, output_h, max_w, max_h,
input_w = rj54n1->rect.width, input_h = rj54n1->rect.height;
int ret;
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
