1. Patchew
  2. linux
  3. View series

[PATCH v2] iio: adc: ad7625: fix type mismatch in clamp() macro

Giorgi Tchankvetadze posted 1 patch 1 month, 3 weeks ago 
drivers/iio/adc/ad7625.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
[PATCH v2] iio: adc: ad7625: fix type mismatch in clamp() macro
Posted by Giorgi Tchankvetadze 1 month, 3 weeks ago 
clamp() expects compatible operand types. The period calculation uses
nanosecond constants, while the local target variable was narrower than
the upper bound expression.

Make target unsigned long and use unsigned long bounds, including
NSEC_PER_USEC for the upper limit. This keeps the operands naturally
aligned without adding casts.

Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Giorgi Tchankvetadze <giorgitchankvetadze1997@gmail.com>
---
Changes in v2:
 - Make target unsigned long and use unsigned long clamp bounds.
 - Use NSEC_PER_USEC for the upper bound.
 - Avoid casts, as suggested by Andy.

 drivers/iio/adc/ad7625.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/adc/ad7625.c b/drivers/iio/adc/ad7625.c
index 0466c0c7eae4..b5a7911c75df 100644
--- a/drivers/iio/adc/ad7625.c
+++ b/drivers/iio/adc/ad7625.c
@@ -175,12 +175,12 @@ enum ad7960_mode {
 
 static int ad7625_set_sampling_freq(struct ad7625_state *st, u32 freq)
 {
-	u32 target;
+	unsigned long target;
 	struct pwm_waveform clk_gate_wf = { }, cnv_wf = { };
 	int ret;
 
 	target = DIV_ROUND_UP(NSEC_PER_SEC, freq);
-	cnv_wf.period_length_ns = clamp(target, 100, 10 * KILO);
+	cnv_wf.period_length_ns = clamp(target, 100UL, 10UL * NSEC_PER_USEC);
 
 	/*
 	 * Use the maximum conversion time t_CNVH from the datasheet as
-- 
2.52.0
Re: [PATCH v2] iio: adc: ad7625: fix type mismatch in clamp() macro
Posted by David Laight 1 month, 2 weeks ago 
On Sat, 25 Apr 2026 11:16:16 +0400
Giorgi Tchankvetadze <giorgitchankvetadze1997@gmail.com> wrote:

> clamp() expects compatible operand types. The period calculation uses
> nanosecond constants, while the local target variable was narrower than
> the upper bound expression.

All the variables and values look to be valid for unsigned comparisons.
So what are you trying to fix?

> 
> Make target unsigned long and use unsigned long bounds, including
> NSEC_PER_USEC for the upper limit. This keeps the operands naturally
> aligned without adding casts.

Changing 'u32' to 'unsigned long' shouldn't make any difference.
The code (probably) has to run on 32bit cpu where both are 32bits.

The upper limit is on 10000 (regardless of how you define it)
so it always fits in u32 (or even u16 - but don't go there).

Indeed NSEC_PER_SEC is less than INT_MAX, so dividing it by anything
gives a value that fits in u32.

So exactly what are you trying to fix?

	David

> 
> Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Giorgi Tchankvetadze <giorgitchankvetadze1997@gmail.com>
> ---
> Changes in v2:
>  - Make target unsigned long and use unsigned long clamp bounds.
>  - Use NSEC_PER_USEC for the upper bound.
>  - Avoid casts, as suggested by Andy.
> 
>  drivers/iio/adc/ad7625.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad7625.c b/drivers/iio/adc/ad7625.c
> index 0466c0c7eae4..b5a7911c75df 100644
> --- a/drivers/iio/adc/ad7625.c
> +++ b/drivers/iio/adc/ad7625.c
> @@ -175,12 +175,12 @@ enum ad7960_mode {
>  
>  static int ad7625_set_sampling_freq(struct ad7625_state *st, u32 freq)
>  {
> -	u32 target;
> +	unsigned long target;
>  	struct pwm_waveform clk_gate_wf = { }, cnv_wf = { };
>  	int ret;
>  
>  	target = DIV_ROUND_UP(NSEC_PER_SEC, freq);
> -	cnv_wf.period_length_ns = clamp(target, 100, 10 * KILO);
> +	cnv_wf.period_length_ns = clamp(target, 100UL, 10UL * NSEC_PER_USEC);
>  
>  	/*
>  	 * Use the maximum conversion time t_CNVH from the datasheet as
Re: [PATCH v2] iio: adc: ad7625: fix type mismatch in clamp() macro
Posted by Andy Shevchenko 1 month, 2 weeks ago 
On Sat, Apr 25, 2026 at 11:16:16AM +0400, Giorgi Tchankvetadze wrote:
> clamp() expects compatible operand types. The period calculation uses
> nanosecond constants, while the local target variable was narrower than
> the upper bound expression.
> 
> Make target unsigned long and use unsigned long bounds, including
> NSEC_PER_USEC for the upper limit. This keeps the operands naturally
> aligned without adding casts.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

...

> -	u32 target;
> +	unsigned long target;
>  	struct pwm_waveform clk_gate_wf = { }, cnv_wf = { };
>  	int ret;

Preferred to keep the reversed xmas tree order (no need to resend, hopefully
Jonathan tweaks this whilst applying).

	struct pwm_waveform clk_gate_wf = { }, cnv_wf = { };
	unsigned long target;
	int ret;

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH v2] iio: adc: ad7625: fix type mismatch in clamp() macro
Posted by Jonathan Cameron 1 month, 2 weeks ago 
On Mon, 27 Apr 2026 10:31:28 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> On Sat, Apr 25, 2026 at 11:16:16AM +0400, Giorgi Tchankvetadze wrote:
> > clamp() expects compatible operand types. The period calculation uses
> > nanosecond constants, while the local target variable was narrower than
> > the upper bound expression.
> > 
> > Make target unsigned long and use unsigned long bounds, including
> > NSEC_PER_USEC for the upper limit. This keeps the operands naturally
> > aligned without adding casts.  
> 
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> ...
> 
> > -	u32 target;
> > +	unsigned long target;
> >  	struct pwm_waveform clk_gate_wf = { }, cnv_wf = { };
> >  	int ret;  
> 
> Preferred to keep the reversed xmas tree order (no need to resend, hopefully
> Jonathan tweaks this whilst applying).
> 
> 	struct pwm_waveform clk_gate_wf = { }, cnv_wf = { };
> 	unsigned long target;
> 	int ret;
> 

Done and applied. Thanks.

J

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.