Re: [RFC PATCH] iio: proximity: sx9360: fix hardware gain encoding

[Jonathan Cameron <jic23@kernel.org>]

On Sat,  7 Nov 2026 16:53:34 +0530
Arnav Kapoor <kapoorarnav43@gmail.com> wrote:

> The sx9360 PROX_CTRL0 gain handling was copied from the original sx9324
> driver and has the same bugs that commit 108e4d4de2b5
> ("iio:proximity:sx9324: Fix hardware gain read/write") fixed there:
> 
>   - The gain field holds ilog2(<gain>) + 1, with 0 reserved, so a gain
>     of 1 is 0x1 and a gain of 8 is 0x4. read_gain() and write_gain()
>     use ilog2(<gain>) instead, so every gain is off by one: writing 1
>     programs the reserved code 0 and writing 8 programs a gain of 4.
>   - SX9360_REG_PROX_CTRL0_GAIN_1 is 0x80, which sets the reserved bit 7
>     and leaves the gain field at the reserved code 0, instead of
>     0x1 << 3.
> 
> Use the same encoding as sx9324: add one when writing, subtract one when
> reading, return -EINVAL for the reserved and out-of-range codes, and
> build the default with FIELD_PREP_CONST(). Use BIT() for the read value,
> as Andy suggested for the same code in 2022.
> 
> Fixes: 1cdb4c47f7f5 ("iio:proximity:sx9360: Add sx9360 support")
> Signed-off-by: Arnav Kapoor <kapoorarnav43@gmail.com>
> ---
> RFC because I don't have the SX9360 datasheet. The only public copy I
> found (the one linked at the top of sx9360.c) is a short excerpt of the
> SX9324 datasheet without the register map, so this follows the sx9324
> fix by analogy: the sx9360 gain code is a copy of the pre-fix sx9324
> code, with the same mask, the same 0x80 default and the same missing +1.
> Gwendal, Stephen: could you confirm that SX9360 PROX_CTRL0 uses the same
> gain encoding as SX9324 (bits 5:3, ilog2(gain) + 1, 0 reserved, bit 7
> reserved)?

The default value is odd.  Do we have any idea what is in that top bit?
I'm going to guess something harmless and that perhaps the gain value
due to writing a 0 is either rejected, or actually mapped to same as 1.

You are correct that we really need someone with datasheet access to
sanity check this sone.

Thanks,

J

> 
> Xiaohui Zhang posted the same change in June 2022 ("iio:proximity:sx9360:
> Fix hardware gain read/write"); it had build errors and a dead-code
> read path and was not respun. The read path here uses the corrected
> check from the sx9324 read_gain() fix I sent on 2026-11-04.
> 
> Applies on top of my series "iio: proximity: sx9310/sx9360: reject
> unsupported hardware gain values":
> https://lore.kernel.org/all/20261107111710.442318-1-kapoorarnav43@gmail.com/
> 
> Compile-tested only (W=1); I don't have SX9360 hardware.
> 
>  drivers/iio/proximity/sx9360.c | 19 ++++++++++++++-----
>  1 file changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/iio/proximity/sx9360.c b/drivers/iio/proximity/sx9360.c
> index c6e5b3e557bb..01a4a94219ec 100644
> --- a/drivers/iio/proximity/sx9360.c
> +++ b/drivers/iio/proximity/sx9360.c
> @@ -67,7 +67,9 @@
>  #define SX9360_REG_PROX_CTRL0_PHR	0x40
>  #define SX9360_REG_PROX_CTRL0_PHM	0x41
>  #define SX9360_REG_PROX_CTRL0_GAIN_MASK	GENMASK(5, 3)
> -#define SX9360_REG_PROX_CTRL0_GAIN_1		0x80
> +#define SX9360_REG_PROX_CTRL0_GAIN_RSVD		0x0
> +#define SX9360_REG_PROX_CTRL0_GAIN_1		0x1
> +#define SX9360_REG_PROX_CTRL0_GAIN_8		0x4
>  #define SX9360_REG_PROX_CTRL0_RAWFILT_MASK	GENMASK(2, 0)
>  #define SX9360_REG_PROX_CTRL0_RAWFILT_1P50	0x01
>  #define SX9360_REG_PROX_CTRL1		0x42
> @@ -291,7 +293,12 @@ static int sx9360_read_gain(struct sx_common_data *data,
>  	if (ret)
>  		return ret;
>  
> -	*val = 1 << FIELD_GET(SX9360_REG_PROX_CTRL0_GAIN_MASK, regval);
> +	regval = FIELD_GET(SX9360_REG_PROX_CTRL0_GAIN_MASK, regval);
> +	if (regval == SX9360_REG_PROX_CTRL0_GAIN_RSVD ||
> +	    regval > SX9360_REG_PROX_CTRL0_GAIN_8)
> +		return -EINVAL;
> +
> +	*val = BIT(regval - 1);
>  
>  	return IIO_VAL_INT;
>  }
> @@ -628,7 +635,7 @@ static int sx9360_write_gain(struct sx_common_data *data,
>  	if (val <= 0 || val > 8 || !is_power_of_2(val))
>  		return -EINVAL;
>  
> -	gain = ilog2(val);
> +	gain = ilog2(val) + 1;
>  	reg = SX9360_REG_PROX_CTRL0_PHR + chan->channel;
>  	gain = FIELD_PREP(SX9360_REG_PROX_CTRL0_GAIN_MASK, gain);
>  
> @@ -676,9 +683,11 @@ static const struct sx_common_reg_default sx9360_default_regs[] = {
>  	{ SX9360_REG_AFE_PARAM1_PHM, SX9360_REG_AFE_PARAM1_AGAIN_PHM_6PF |
>  		SX9360_REG_AFE_PARAM1_FREQ_83_33HZ, "afe_param1_phm" },
>  
> -	{ SX9360_REG_PROX_CTRL0_PHR, SX9360_REG_PROX_CTRL0_GAIN_1 |
> +	{ SX9360_REG_PROX_CTRL0_PHR,
> +		FIELD_PREP_CONST(SX9360_REG_PROX_CTRL0_GAIN_MASK, SX9360_REG_PROX_CTRL0_GAIN_1) |
>  		SX9360_REG_PROX_CTRL0_RAWFILT_1P50, "prox_ctrl0_phr" },
> -	{ SX9360_REG_PROX_CTRL0_PHM, SX9360_REG_PROX_CTRL0_GAIN_1 |
> +	{ SX9360_REG_PROX_CTRL0_PHM,
> +		FIELD_PREP_CONST(SX9360_REG_PROX_CTRL0_GAIN_MASK, SX9360_REG_PROX_CTRL0_GAIN_1) |
>  		SX9360_REG_PROX_CTRL0_RAWFILT_1P50, "prox_ctrl0_phm" },
>  	{ SX9360_REG_PROX_CTRL1, SX9360_REG_PROX_CTRL1_AVGNEG_THRESH_16K, "prox_ctrl1" },
>  	{ SX9360_REG_PROX_CTRL2, SX9360_REG_PROX_CTRL2_AVGDEB_2SAMPLES |
> 
> base-commit: 509224d37ff169a02f388ab6e525d5517fb570dc