[v1] iio: adc: ti-ads1018: fix type overflow for data rate

[PATCH] iio: adc: ti-ads1018: fix type overflow for data rate

Posted by Chunyang Chen 1 month ago

The variable 'drate' is currently defined as u8. However, the data rate
values in ads1018 can reach up to 3300 Hz (e.g., in ads1018_data_rate[]),
which exceeds the maximum value of 255 that a u8 can hold.

Change the type of 'drate' to u16 to prevent integer overflow and
ensure the data rate is handled correctly.

Signed-off-by: Chunyang Chen <chenchunyang0908@gmail.com>
---
 drivers/iio/adc/ti-ads1018.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/adc/ti-ads1018.c b/drivers/iio/adc/ti-ads1018.c
index 6246b3cab..049b8c00f 100644
--- a/drivers/iio/adc/ti-ads1018.c
+++ b/drivers/iio/adc/ti-ads1018.c
@@ -249,7 +249,7 @@ static int ads1018_single_shot(struct ads1018 *ads1018,
 			       struct iio_chan_spec const *chan, u16 *cnv)
 {
 	u8 max_drate_mode = ads1018->chip_info->num_data_rate_mode_to_hz - 1;
-	u8 drate = ads1018->chip_info->data_rate_mode_to_hz[max_drate_mode];
+	u16 drate = ads1018->chip_info->data_rate_mode_to_hz[max_drate_mode];
 	u8 pga_mode = ads1018->chan_data[chan->scan_index].pga_mode;
 	struct spi_transfer xfer[2] = {
 		{
-- 
2.34.1

Re: [PATCH] iio: adc: ti-ads1018: fix type overflow for data rate

Posted by Andy Shevchenko 1 month ago

On Thu, Mar 05, 2026 at 10:15:50AM +0800, Chunyang Chen wrote:
> The variable 'drate' is currently defined as u8. However, the data rate
> values in ads1018 can reach up to 3300 Hz (e.g., in ads1018_data_rate[]),
> which exceeds the maximum value of 255 that a u8 can hold.
> 
> Change the type of 'drate' to u16 to prevent integer overflow and
> ensure the data rate is handled correctly.

Needs Fixes tag.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] iio: adc: ti-ads1018: fix type overflow for data rate

Posted by Chunyang Chen 1 month ago

Hi Kurt, Andy,

Thanks for your review and suggestions!
I have sent out the v2 patch, which addresses the following:

On Thu, Mar 5, 2026, Kurt Borja wrote:
> data_rate_mode_to_hz[] is u32 so I prefer for this to be u32. This also
> kinda matches ads1018_calc_delay() which takes unsigned int.

Changed 'drate' type to u32 to match the array definition and maintain
consistency, as suggested by Kurt.

On Thu, Mar 5, 2026, Andy Shevchenko wrote:
> Needs Fixes tag.

Added the Fixes tag as requested by Andy.

Best regards,
Chunyang Chen

Re: [PATCH] iio: adc: ti-ads1018: fix type overflow for data rate

Posted by Kurt Borja 1 month ago

Hi Chunyang,

On Wed Mar 4, 2026 at 11:15 PM -03, Chunyang Chen wrote:
> The variable 'drate' is currently defined as u8. However, the data rate
> values in ads1018 can reach up to 3300 Hz (e.g., in ads1018_data_rate[]),
> which exceeds the maximum value of 255 that a u8 can hold.
>
> Change the type of 'drate' to u16 to prevent integer overflow and
> ensure the data rate is handled correctly.

Good catch -- thanks!

Interestingly, my SPI controller has so much latency that I didn't catch
this while developing :/

>
> Signed-off-by: Chunyang Chen <chenchunyang0908@gmail.com>
> ---
>  drivers/iio/adc/ti-ads1018.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/iio/adc/ti-ads1018.c b/drivers/iio/adc/ti-ads1018.c
> index 6246b3cab..049b8c00f 100644
> --- a/drivers/iio/adc/ti-ads1018.c
> +++ b/drivers/iio/adc/ti-ads1018.c
> @@ -249,7 +249,7 @@ static int ads1018_single_shot(struct ads1018 *ads1018,
>  			       struct iio_chan_spec const *chan, u16 *cnv)
>  {
>  	u8 max_drate_mode = ads1018->chip_info->num_data_rate_mode_to_hz - 1;
> -	u8 drate = ads1018->chip_info->data_rate_mode_to_hz[max_drate_mode];
> +	u16 drate = ads1018->chip_info->data_rate_mode_to_hz[max_drate_mode];

data_rate_mode_to_hz[] is u32 so I prefer for this to be u32. This also
kinda matches ads1018_calc_delay() which takes unsigned int.

I'm a bit disturbed by the type inconsistency here...

>  	u8 pga_mode = ads1018->chan_data[chan->scan_index].pga_mode;
>  	struct spi_transfer xfer[2] = {
>  		{

-- 
Thanks,
 ~ Kurt

[PATCH v2] iio: adc: ti-ads1018: fix type overflow for data rate

Posted by Chunyang Chen 1 month ago

The variable 'drate' is currently defined as u8. However, the data rate
values in ads1018 can reach up to 3300 Hz, which exceeds the maximum
value of 255 that a u8 can hold.

Change the type of 'drate' to u32 to match the data_rate_mode_to_hz
array definition and ensure the data rate is handled correctly.

Fixes: bf0bba486b5b ("iio: adc: ti-ads1018: add ti-ads1018 driver")
Signed-off-by: Chunyang Chen <chenchunyang0908@gmail.com>

---
v2:
- Change drate type to u32 as suggested by Kurt.
- Add Fixes tag as requested by Andy.
---
 drivers/iio/adc/ti-ads1018.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/adc/ti-ads1018.c b/drivers/iio/adc/ti-ads1018.c
index 6246b3cab..0780abd0d 100644
--- a/drivers/iio/adc/ti-ads1018.c
+++ b/drivers/iio/adc/ti-ads1018.c
@@ -249,7 +249,7 @@ static int ads1018_single_shot(struct ads1018 *ads1018,
 			       struct iio_chan_spec const *chan, u16 *cnv)
 {
 	u8 max_drate_mode = ads1018->chip_info->num_data_rate_mode_to_hz - 1;
-	u8 drate = ads1018->chip_info->data_rate_mode_to_hz[max_drate_mode];
+	u32 drate = ads1018->chip_info->data_rate_mode_to_hz[max_drate_mode];
 	u8 pga_mode = ads1018->chan_data[chan->scan_index].pga_mode;
 	struct spi_transfer xfer[2] = {
 		{
-- 
2.34.1