From: Jonathan Cameron <jic23@kernel.org>
To: Arnav Kapoor <kapoorarnav43@gmail.com>
Cc: "David Lechner" <dlechner@baylibre.com>,
"Nuno Sá" <nuno.sa@analog.com>,
"Andy Shevchenko" <andy@kernel.org>,
"Andy Shevchenko" <andriy.shevchenko@intel.com>,
"Gwendal Grignou" <gwendal@chromium.org>,
linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] iio: proximity: sx9360: fix sampling frequency write and available range
Date: Sun, 25 Oct 2026 22:27:15 +0000 [thread overview]
Message-ID: <20261025222715.6526ae07@jic23-hlaptop> (raw)
In-Reply-To: <20261025221241.19596-1-kapoorarnav43@gmail.com>
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
prev parent reply other threads:[~2026-10-25 22:27 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-10-25 22:12 Arnav Kapoor
2026-10-25 22:27 ` Jonathan Cameron [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20261025222715.6526ae07@jic23-hlaptop \
--to=jic23@kernel.org \
--cc=andriy.shevchenko@intel.com \
--cc=andy@kernel.org \
--cc=dlechner@baylibre.com \
--cc=gwendal@chromium.org \
--cc=kapoorarnav43@gmail.com \
--cc=linux-iio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=nuno.sa@analog.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®