From: Jonathan Cameron <jic23@kernel.org>
To: Aldo Conte <aldocontelk@gmail.com>
Cc: ktsai@capellamicro.com, dlechner@baylibre.com,
nuno.sa@analog.com, andy@kernel.org, daniel.baluta@intel.com,
linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org,
skhan@linuxfoundation.org, me@brighamcampbell.com,
linux-kernel-mentees@lists.linux.dev
Subject: Re: [PATCH v4 1/2] iio: light: cm3323: fix reg_conf not being initialized correctly
Date: Mon, 20 Apr 2026 18:53:33 +0100 [thread overview]
Message-ID: <20260420185333.3bab224d@jic23-huawei> (raw)
In-Reply-To: <20260407151702.15208-2-aldocontelk@gmail.com>
On Tue, 7 Apr 2026 17:17:01 +0200
Aldo Conte <aldocontelk@gmail.com> wrote:
> The code stores the return value of i2c_smbus_write_word_data()
> in data->reg_conf; however, this value represents the result
> of the write operation and not the value actually written to
> the configuration register. This meant that the contents of
> data->reg_conf did not truly reflect the contents
> of the hardware register.
>
> Instead, save the value of the register before the write
> and use this value in the I2C write.
>
> The bug was found by code inspection: i2c_smbus_write_word_data()
> returns 0 on success, not the value written to the register.
>
> Tested using i2c-stub on a Raspberry Pi 3B running a custom 6.19.10
> kernel. Before loading the driver, the configuration register 0x00
> CM3323_CMD_CONF was populated with 0x0030 using
> `i2cset -y 11 0x10 0x00 0x0030 w`, encoding an integration time of 320ms
> in bits[6:4].
>
> Due to incorrect initialization of data->reg_conf in
> cm3323_init(), the print of integration_time returns 0.040000
> instead of the expected 0.320000. This happens because the read of the
> integration_time depends on cm3323_get_it_bits() that is based on the
> value of data->reg_conf, which is erroneously set to 0.
>
> With this fix applied, data->reg_conf correctly saves 0x0030 after init
> and the successive integration_time reports 0.320000 as expected.
>
> Fixes: 8b0544263761 ("iio: light: Add support for Capella CM3323 color sensor")
> Cc: stable@vger.kernel.org
> Signed-off-by: Aldo Conte <aldocontelk@gmail.com>
Applied this patch, but patch 2 will need to wait for this to make its way
into upstream - probably middle of the next cycle before that is in place.
thanks,
Jonathan
> ---
> drivers/iio/light/cm3323.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iio/light/cm3323.c b/drivers/iio/light/cm3323.c
> index 79ad6e2209ca..0fe61b8a7029 100644
> --- a/drivers/iio/light/cm3323.c
> +++ b/drivers/iio/light/cm3323.c
> @@ -89,15 +89,14 @@ static int cm3323_init(struct iio_dev *indio_dev)
>
> /* enable sensor and set auto force mode */
> ret &= ~(CM3323_CONF_SD_BIT | CM3323_CONF_AF_BIT);
> + data->reg_conf = ret;
>
> - ret = i2c_smbus_write_word_data(data->client, CM3323_CMD_CONF, ret);
> + ret = i2c_smbus_write_word_data(data->client, CM3323_CMD_CONF, data->reg_conf);
> if (ret < 0) {
> dev_err(&data->client->dev, "Error writing reg_conf\n");
> return ret;
> }
>
> - data->reg_conf = ret;
> -
> return 0;
> }
>
next prev parent reply other threads:[~2026-04-20 17:53 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-04-07 15:17 [PATCH v4 0/2] iio: light: cm3323: fix and cleanup Aldo Conte
2026-04-07 15:17 ` [PATCH v4 1/2] iio: light: cm3323: fix reg_conf not being initialized correctly Aldo Conte
2026-04-20 17:53 ` Jonathan Cameron [this message]
2026-07-26 11:44 ` Aldo Conte
2026-04-07 15:17 ` [PATCH v4 2/2] iio: light: cm3323: use dev_err_probe() in probe path Aldo Conte
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=20260420185333.3bab224d@jic23-huawei \
--to=jic23@kernel.org \
--cc=aldocontelk@gmail.com \
--cc=andy@kernel.org \
--cc=daniel.baluta@intel.com \
--cc=dlechner@baylibre.com \
--cc=ktsai@capellamicro.com \
--cc=linux-iio@vger.kernel.org \
--cc=linux-kernel-mentees@lists.linux.dev \
--cc=linux-kernel@vger.kernel.org \
--cc=me@brighamcampbell.com \
--cc=nuno.sa@analog.com \
--cc=skhan@linuxfoundation.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.