* [PATCH] smiapp: re-use clamp_t instead of min(..., max(...))
@ 2013-07-24 15:21 Andy Shevchenko
2013-07-24 15:45 ` Sakari Ailus
0 siblings, 1 reply; 6+ messages in thread
From: Andy Shevchenko @ 2013-07-24 15:21 UTC (permalink / raw)
To: Sakari Ailus, Mauro Carvalho Chehab, linux-media; +Cc: Andy Shevchenko
clamp_t does the job to put a variable into the given range.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/media/i2c/smiapp/smiapp-core.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/media/i2c/smiapp/smiapp-core.c b/drivers/media/i2c/smiapp/smiapp-core.c
index 7ac7580..914e52f 100644
--- a/drivers/media/i2c/smiapp/smiapp-core.c
+++ b/drivers/media/i2c/smiapp/smiapp-core.c
@@ -1835,12 +1835,12 @@ static void smiapp_set_compose_scaler(struct v4l2_subdev *subdev,
* sensor->limits[SMIAPP_LIMIT_SCALER_N_MIN]
/ sensor->limits[SMIAPP_LIMIT_MIN_X_OUTPUT_SIZE];
- a = min(sensor->limits[SMIAPP_LIMIT_SCALER_M_MAX],
- max(a, sensor->limits[SMIAPP_LIMIT_SCALER_M_MIN]));
- b = min(sensor->limits[SMIAPP_LIMIT_SCALER_M_MAX],
- max(b, sensor->limits[SMIAPP_LIMIT_SCALER_M_MIN]));
- max_m = min(sensor->limits[SMIAPP_LIMIT_SCALER_M_MAX],
- max(max_m, sensor->limits[SMIAPP_LIMIT_SCALER_M_MIN]));
+ a = clamp_t(u32, a, sensor->limits[SMIAPP_LIMIT_SCALER_M_MIN],
+ sensor->limits[SMIAPP_LIMIT_SCALER_M_MAX]);
+ b = clamp_t(u32, b, sensor->limits[SMIAPP_LIMIT_SCALER_M_MIN],
+ sensor->limits[SMIAPP_LIMIT_SCALER_M_MAX]);
+ max_m = clamp_t(u32, max_m, sensor->limits[SMIAPP_LIMIT_SCALER_M_MIN],
+ sensor->limits[SMIAPP_LIMIT_SCALER_M_MAX]);
dev_dbg(&client->dev, "scaling: a %d b %d max_m %d\n", a, b, max_m);
--
1.8.3.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] smiapp: re-use clamp_t instead of min(..., max(...))
2013-07-24 15:21 [PATCH] smiapp: re-use clamp_t instead of min(..., max(...)) Andy Shevchenko
@ 2013-07-24 15:45 ` Sakari Ailus
2013-07-24 15:49 ` Andy Shevchenko
0 siblings, 1 reply; 6+ messages in thread
From: Sakari Ailus @ 2013-07-24 15:45 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: linux-media
Hi Andy,
Thanks for the patch.
(Drop Mauro from cc.)
On Wed, Jul 24, 2013 at 06:21:18PM +0300, Andy Shevchenko wrote:
> clamp_t does the job to put a variable into the given range.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> drivers/media/i2c/smiapp/smiapp-core.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/media/i2c/smiapp/smiapp-core.c b/drivers/media/i2c/smiapp/smiapp-core.c
> index 7ac7580..914e52f 100644
> --- a/drivers/media/i2c/smiapp/smiapp-core.c
> +++ b/drivers/media/i2c/smiapp/smiapp-core.c
> @@ -1835,12 +1835,12 @@ static void smiapp_set_compose_scaler(struct v4l2_subdev *subdev,
> * sensor->limits[SMIAPP_LIMIT_SCALER_N_MIN]
> / sensor->limits[SMIAPP_LIMIT_MIN_X_OUTPUT_SIZE];
>
> - a = min(sensor->limits[SMIAPP_LIMIT_SCALER_M_MAX],
> - max(a, sensor->limits[SMIAPP_LIMIT_SCALER_M_MIN]));
> - b = min(sensor->limits[SMIAPP_LIMIT_SCALER_M_MAX],
> - max(b, sensor->limits[SMIAPP_LIMIT_SCALER_M_MIN]));
> - max_m = min(sensor->limits[SMIAPP_LIMIT_SCALER_M_MAX],
> - max(max_m, sensor->limits[SMIAPP_LIMIT_SCALER_M_MIN]));
> + a = clamp_t(u32, a, sensor->limits[SMIAPP_LIMIT_SCALER_M_MIN],
> + sensor->limits[SMIAPP_LIMIT_SCALER_M_MAX]);
> + b = clamp_t(u32, b, sensor->limits[SMIAPP_LIMIT_SCALER_M_MIN],
> + sensor->limits[SMIAPP_LIMIT_SCALER_M_MAX]);
> + max_m = clamp_t(u32, max_m, sensor->limits[SMIAPP_LIMIT_SCALER_M_MIN],
> + sensor->limits[SMIAPP_LIMIT_SCALER_M_MAX]);
Do you need clamp_t()? Wouldn't plain clamp() do?
I can change it if you're ok with that.
--
Cheers,
Sakari Ailus
e-mail: sakari.ailus@iki.fi XMPP: sailus@retiisi.org.uk
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] smiapp: re-use clamp_t instead of min(..., max(...))
2013-07-24 15:45 ` Sakari Ailus
@ 2013-07-24 15:49 ` Andy Shevchenko
2013-07-24 15:55 ` Sakari Ailus
0 siblings, 1 reply; 6+ messages in thread
From: Andy Shevchenko @ 2013-07-24 15:49 UTC (permalink / raw)
To: Sakari Ailus; +Cc: Andy Shevchenko, linux-media
On Wed, Jul 24, 2013 at 6:45 PM, Sakari Ailus <sakari.ailus@iki.fi> wrote:
[]
>> + max_m = clamp_t(u32, max_m, sensor->limits[SMIAPP_LIMIT_SCALER_M_MIN],
>> + sensor->limits[SMIAPP_LIMIT_SCALER_M_MAX]);
>
> Do you need clamp_t()? Wouldn't plain clamp() do?
The *_t variants are preferred due to they are faster (no type checking).
> I can change it if you're ok with that.
I don't know why you may choose clamp instead of clamp_t here. Are you
going to change variable types?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] smiapp: re-use clamp_t instead of min(..., max(...))
2013-07-24 15:49 ` Andy Shevchenko
@ 2013-07-24 15:55 ` Sakari Ailus
2013-07-25 7:47 ` Andy Shevchenko
2013-08-08 22:43 ` Laurent Pinchart
0 siblings, 2 replies; 6+ messages in thread
From: Sakari Ailus @ 2013-07-24 15:55 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: Andy Shevchenko, linux-media
On Wed, Jul 24, 2013 at 06:49:24PM +0300, Andy Shevchenko wrote:
> On Wed, Jul 24, 2013 at 6:45 PM, Sakari Ailus <sakari.ailus@iki.fi> wrote:
>
> []
>
> >> + max_m = clamp_t(u32, max_m, sensor->limits[SMIAPP_LIMIT_SCALER_M_MIN],
> >> + sensor->limits[SMIAPP_LIMIT_SCALER_M_MAX]);
> >
> > Do you need clamp_t()? Wouldn't plain clamp() do?
>
> The *_t variants are preferred due to they are faster (no type checking).
>
> > I can change it if you're ok with that.
>
> I don't know why you may choose clamp instead of clamp_t here. Are you
> going to change variable types?
Probably not. But clamp() would serve as a sanity check vs. clamp_t() which
just does the thing. I'd prefer clamp() --- the compiler will not spend much
time on it anyway.
--
Cheers,
Sakari Ailus
e-mail: sakari.ailus@iki.fi XMPP: sailus@retiisi.org.uk
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] smiapp: re-use clamp_t instead of min(..., max(...))
2013-07-24 15:55 ` Sakari Ailus
@ 2013-07-25 7:47 ` Andy Shevchenko
2013-08-08 22:43 ` Laurent Pinchart
1 sibling, 0 replies; 6+ messages in thread
From: Andy Shevchenko @ 2013-07-25 7:47 UTC (permalink / raw)
To: Sakari Ailus; +Cc: Andy Shevchenko, linux-media
On Wed, Jul 24, 2013 at 6:55 PM, Sakari Ailus <sakari.ailus@iki.fi> wrote:
> On Wed, Jul 24, 2013 at 06:49:24PM +0300, Andy Shevchenko wrote:
>> On Wed, Jul 24, 2013 at 6:45 PM, Sakari Ailus <sakari.ailus@iki.fi> wrote:
>>
>> []
>>
>> >> + max_m = clamp_t(u32, max_m, sensor->limits[SMIAPP_LIMIT_SCALER_M_MIN],
>> >> + sensor->limits[SMIAPP_LIMIT_SCALER_M_MAX]);
>> >
>> > Do you need clamp_t()? Wouldn't plain clamp() do?
>>
>> The *_t variants are preferred due to they are faster (no type checking).
>>
>> > I can change it if you're ok with that.
>>
>> I don't know why you may choose clamp instead of clamp_t here. Are you
>> going to change variable types?
>
> Probably not. But clamp() would serve as a sanity check vs. clamp_t() which
> just does the thing. I'd prefer clamp()
You may adjust original patch if you want.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] smiapp: re-use clamp_t instead of min(..., max(...))
2013-07-24 15:55 ` Sakari Ailus
2013-07-25 7:47 ` Andy Shevchenko
@ 2013-08-08 22:43 ` Laurent Pinchart
1 sibling, 0 replies; 6+ messages in thread
From: Laurent Pinchart @ 2013-08-08 22:43 UTC (permalink / raw)
To: Sakari Ailus; +Cc: Andy Shevchenko, Andy Shevchenko, linux-media
Hi Sakari,
On Wednesday 24 July 2013 18:55:38 Sakari Ailus wrote:
> On Wed, Jul 24, 2013 at 06:49:24PM +0300, Andy Shevchenko wrote:
> > On Wed, Jul 24, 2013 at 6:45 PM, Sakari Ailus <sakari.ailus@iki.fi> wrote:
> >
> > []
> >
> > >> + max_m = clamp_t(u32, max_m,
> > >> sensor->limits[SMIAPP_LIMIT_SCALER_M_MIN], +
> > >> sensor->limits[SMIAPP_LIMIT_SCALER_M_MAX]);
> > >
> > > Do you need clamp_t()? Wouldn't plain clamp() do?
> >
> > The *_t variants are preferred due to they are faster (no type checking).
> >
> > > I can change it if you're ok with that.
> >
> > I don't know why you may choose clamp instead of clamp_t here. Are you
> > going to change variable types?
>
> Probably not. But clamp() would serve as a sanity check vs. clamp_t() which
> just does the thing. I'd prefer clamp() --- the compiler will not spend much
> time on it anyway.
Should I take this patch in my tree ? If so, could you please repost it with
clamp() instead of clamp_t(), and your SoB or Acked-by ?
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-08-08 22:42 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-07-24 15:21 [PATCH] smiapp: re-use clamp_t instead of min(..., max(...)) Andy Shevchenko
2013-07-24 15:45 ` Sakari Ailus
2013-07-24 15:49 ` Andy Shevchenko
2013-07-24 15:55 ` Sakari Ailus
2013-07-25 7:47 ` Andy Shevchenko
2013-08-08 22:43 ` Laurent Pinchart
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox