[PATCH v2] iio: proximity: sx9360: fix sampling frequency write and available range

[PATCH v2] iio: proximity: sx9360: fix sampling frequency write and available range

[Arnav Kapoor]

GNRL_CTRL1/2 hold the scan period in units of 8192 / FOSC, but
sx9360_set_samp_freq() computes freq * 8192 / FOSC and mis-scales val2,
which is in micro units. Any integer frequency then programs a period
of 0, and fractional parts above ~0.26 Hz overflow the int, writing
0x10 to GNRL_CTRL1 outside the SCANPERIOD field.

Compute FOSC / (freq * 8192), rounded to the closest value, and reject
frequencies outside the 1..2047 period range.

The available range doesn't match the limits in the comments next to it
either: 4 MHz / 8192 is 488.28125 Hz, not 448.28125 Hz,
4 MHz / (8192 * 2047) is 0.238535 Hz, and (max - min) / 2047 is
0.238419 Hz. Use those values.

Fixes: 1cdb4c47f7f5 ("iio:proximity:sx9360: Add sx9360 support")
Signed-off-by: Arnav Kapoor <kapoorarnav43@gmail.com>
---
Changes in v2:
- Rename period to period_ticks and say in the comment what a tick is (Andy)
- Rename freq_uhz to freq_uHz and use MICROHZ_PER_HZ instead of MICRO (Andy)
- Add a blank line before guard() (Andy)

Converting sx9360_samp_freq_interval[] to a plain int array is left for
a separate cleanup.

Compile-tested only (W=1); I don't have SX9360 hardware.

v1: https://lore.kernel.org/all/20261024180149.144609-1-kapoorarnav43@gmail.com/

 drivers/iio/proximity/sx9360.c | 29 +++++++++++++++++++++++------
 1 file changed, 23 insertions(+), 6 deletions(-)

diff --git a/drivers/iio/proximity/sx9360.c b/drivers/iio/proximity/sx9360.c
index ed83d809ecf5..2a9b37df8f62 100644
--- a/drivers/iio/proximity/sx9360.c
+++ b/drivers/iio/proximity/sx9360.c
@@ -14,10 +14,12 @@
 #include <linux/interrupt.h>
 #include <linux/kernel.h>
 #include <linux/log2.h>
+#include <linux/math64.h>
 #include <linux/module.h>
 #include <linux/pm.h>
 #include <linux/property.h>
 #include <linux/regmap.h>
+#include <linux/units.h>
 
 #include <linux/iio/iio.h>
 
@@ -43,6 +45,7 @@
 #define SX9360_REG_GNRL_CTRL1_SCANPERIOD_MASK GENMASK(2, 0)
 #define SX9360_REG_GNRL_CTRL2		0x12
 #define SX9360_REG_GNRL_CTRL2_PERIOD_102MS	0x32
+#define SX9360_REG_GNRL_SCANPERIOD_MAX		GENMASK(10, 0)
 #define SX9360_REG_GNRL_REG_2_PERIOD_MS(_r)	\
 	(((_r) * 8192) / (SX9360_FOSC_HZ / 1000))
 #define SX9360_REG_GNRL_FREQ_2_REG(_f)  (((_f) * 8192) / SX9360_FOSC_HZ)
@@ -182,9 +185,9 @@ static const struct {
 	int val;
 	int val2;
 } sx9360_samp_freq_interval[] = {
-	{ 0, 281250 },  /* 4MHz / (8192 * 2047) */
-	{ 0, 281250 },
-	{ 448, 281250 },  /* 4MHz / 8192 */
+	{ 0, 238535 },  /* 4MHz / (8192 * 2047) */
+	{ 0, 238419 },
+	{ 488, 281250 },  /* 4MHz / 8192 */
 };
 
 static const struct regmap_range sx9360_writable_reg_ranges[] = {
@@ -383,11 +386,25 @@ static int sx9360_read_avail(struct iio_dev *indio_dev,
 static int sx9360_set_samp_freq(struct sx_common_data *data,
 				int val, int val2)
 {
-	int reg;
+	unsigned int period_ticks;
+	u64 freq_uHz;
 	__be16 buf;
 
-	reg = val * 8192 / SX9360_FOSC_HZ + val2 * 8192 / (SX9360_FOSC_MHZ);
-	buf = cpu_to_be16(reg);
+	if (val < 0 || val2 < 0)
+		return -EINVAL;
+
+	freq_uHz = (u64)val * MICROHZ_PER_HZ + val2;
+	if (!freq_uHz)
+		return -EINVAL;
+
+	/* The register holds the scan period in ticks of 8192 FOSC cycles. */
+	period_ticks = DIV64_U64_ROUND_CLOSEST((u64)SX9360_FOSC_HZ * MICROHZ_PER_HZ,
+					       freq_uHz * 8192);
+	if (!period_ticks || period_ticks > SX9360_REG_GNRL_SCANPERIOD_MAX)
+		return -EINVAL;
+
+	buf = cpu_to_be16(period_ticks);
+
 	guard(mutex)(&data->mutex);
 
 	return regmap_bulk_write(data->regmap, SX9360_REG_GNRL_CTRL1, &buf,

base-commit: 2ef6f5c4ee56e90c237c5f7687f09e635f7d30a9
-- 
2.53.0

Re: [PATCH v2] iio: proximity: sx9360: fix sampling frequency write and available range

[Jonathan Cameron]

On Mon, 26 Oct 2026 03:42:38 +0530
Arnav Kapoor <kapoorarnav43@gmail.com> wrote:

> GNRL_CTRL1/2 hold the scan period in units of 8192 / FOSC, but
> sx9360_set_samp_freq() computes freq * 8192 / FOSC and mis-scales val2,
> which is in micro units. Any integer frequency then programs a period
> of 0, and fractional parts above ~0.26 Hz overflow the int, writing
> 0x10 to GNRL_CTRL1 outside the SCANPERIOD field.
> 
> Compute FOSC / (freq * 8192), rounded to the closest value, and reject
> frequencies outside the 1..2047 period range.
> 
> The available range doesn't match the limits in the comments next to it
> either: 4 MHz / 8192 is 488.28125 Hz, not 448.28125 Hz,
> 4 MHz / (8192 * 2047) is 0.238535 Hz, and (max - min) / 2047 is
> 0.238419 Hz. Use those values.
> 
> Fixes: 1cdb4c47f7f5 ("iio:proximity:sx9360: Add sx9360 support")
> Signed-off-by: Arnav Kapoor <kapoorarnav43@gmail.com>
Seems like you've addressed Andy's comments (except for the 'at some
point' one which is fine to leave for now).
Looks good to me, so rather than keep track of this one I'm
going to speculatively apply it to the fixes-togreg branch of iio.git
and mark it for stable.

Note that I'll be rebasing that tree anyway on rc1 when it is available
so there is still time for others to take a look

Thanks

Jonathan

> ---
> Changes in v2:
> - Rename period to period_ticks and say in the comment what a tick is (Andy)
> - Rename freq_uhz to freq_uHz and use MICROHZ_PER_HZ instead of MICRO (Andy)
> - Add a blank line before guard() (Andy)
> 
> Converting sx9360_samp_freq_interval[] to a plain int array is left for
> a separate cleanup.
> 
> Compile-tested only (W=1); I don't have SX9360 hardware.
> 
> v1: https://lore.kernel.org/all/20261024180149.144609-1-kapoorarnav43@gmail.com/
> 
>  drivers/iio/proximity/sx9360.c | 29 +++++++++++++++++++++++------
>  1 file changed, 23 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/iio/proximity/sx9360.c b/drivers/iio/proximity/sx9360.c
> index ed83d809ecf5..2a9b37df8f62 100644
> --- a/drivers/iio/proximity/sx9360.c
> +++ b/drivers/iio/proximity/sx9360.c
> @@ -14,10 +14,12 @@
>  #include <linux/interrupt.h>
>  #include <linux/kernel.h>
>  #include <linux/log2.h>
> +#include <linux/math64.h>
>  #include <linux/module.h>
>  #include <linux/pm.h>
>  #include <linux/property.h>
>  #include <linux/regmap.h>
> +#include <linux/units.h>
>  
>  #include <linux/iio/iio.h>
>  
> @@ -43,6 +45,7 @@
>  #define SX9360_REG_GNRL_CTRL1_SCANPERIOD_MASK GENMASK(2, 0)
>  #define SX9360_REG_GNRL_CTRL2		0x12
>  #define SX9360_REG_GNRL_CTRL2_PERIOD_102MS	0x32
> +#define SX9360_REG_GNRL_SCANPERIOD_MAX		GENMASK(10, 0)
>  #define SX9360_REG_GNRL_REG_2_PERIOD_MS(_r)	\
>  	(((_r) * 8192) / (SX9360_FOSC_HZ / 1000))
>  #define SX9360_REG_GNRL_FREQ_2_REG(_f)  (((_f) * 8192) / SX9360_FOSC_HZ)
> @@ -182,9 +185,9 @@ static const struct {
>  	int val;
>  	int val2;
>  } sx9360_samp_freq_interval[] = {
> -	{ 0, 281250 },  /* 4MHz / (8192 * 2047) */
> -	{ 0, 281250 },
> -	{ 448, 281250 },  /* 4MHz / 8192 */
> +	{ 0, 238535 },  /* 4MHz / (8192 * 2047) */
> +	{ 0, 238419 },
> +	{ 488, 281250 },  /* 4MHz / 8192 */
>  };
>  
>  static const struct regmap_range sx9360_writable_reg_ranges[] = {
> @@ -383,11 +386,25 @@ static int sx9360_read_avail(struct iio_dev *indio_dev,
>  static int sx9360_set_samp_freq(struct sx_common_data *data,
>  				int val, int val2)
>  {
> -	int reg;
> +	unsigned int period_ticks;
> +	u64 freq_uHz;
>  	__be16 buf;
>  
> -	reg = val * 8192 / SX9360_FOSC_HZ + val2 * 8192 / (SX9360_FOSC_MHZ);
> -	buf = cpu_to_be16(reg);
> +	if (val < 0 || val2 < 0)
> +		return -EINVAL;
> +
> +	freq_uHz = (u64)val * MICROHZ_PER_HZ + val2;
> +	if (!freq_uHz)
> +		return -EINVAL;
> +
> +	/* The register holds the scan period in ticks of 8192 FOSC cycles. */
> +	period_ticks = DIV64_U64_ROUND_CLOSEST((u64)SX9360_FOSC_HZ * MICROHZ_PER_HZ,
> +					       freq_uHz * 8192);
> +	if (!period_ticks || period_ticks > SX9360_REG_GNRL_SCANPERIOD_MAX)
> +		return -EINVAL;
> +
> +	buf = cpu_to_be16(period_ticks);
> +
>  	guard(mutex)(&data->mutex);
>  
>  	return regmap_bulk_write(data->regmap, SX9360_REG_GNRL_CTRL1, &buf,
> 
> base-commit: 2ef6f5c4ee56e90c237c5f7687f09e635f7d30a9