On 2024/6/18 03:22, Jonathan Cameron wrote:
On Mon, 17 Jun 2024 19:34:30 +0800
kernel test robot <lkp@xxxxxxxxx> wrote:
Hi Yasin,
kernel test robot noticed the following build warnings:
[auto build test WARNING on jic23-iio/togreg]
[also build test WARNING on robh/for-next linus/master v6.10-rc4 next-20240613]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Yasin-Lee/dt-bindings-iio-proximity-Add-hx9023s-binding/20240616-154122
base: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
patch link: https://lore.kernel.org/r/SN7PR12MB8101D4BC788B5954608D677DA4CC2%40SN7PR12MB8101.namprd12.prod.outlook.com
patch subject: [PATCH v5 3/3] iio:proximity:hx9023s: Add TYHX HX9023S sensor driver
config: arm64-randconfig-r132-20240617 (https://download.01.org/0day-ci/archive/20240617/202406171946.qe83Tde0-lkp@xxxxxxxxx/config)
compiler: aarch64-linux-gcc (GCC) 13.2.0
reproduce: (https://download.01.org/0day-ci/archive/20240617/202406171946.qe83Tde0-lkp@xxxxxxxxx/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@xxxxxxxxx>
| Closes: https://lore.kernel.org/oe-kbuild-all/202406171946.qe83Tde0-lkp@xxxxxxxxx/
sparse warnings: (new ones prefixed by >>)
drivers/iio/proximity/hx9023s.c:955:44: sparse: sparse: incorrect type in assignment (different base types) @@ expected restricted __be16 @@ got int diff @@
drivers/iio/proximity/hx9023s.c:955:44: sparse: expected restricted __be16
drivers/iio/proximity/hx9023s.c:955:44: sparse: got int diff
vim +955 drivers/iio/proximity/hx9023s.c
931
932 static irqreturn_t hx9023s_trigger_handler(int irq, void *private)
933 {
934 struct iio_poll_func *pf = private;
935 struct iio_dev *indio_dev = pf->indio_dev;
936 struct hx9023s_data *data = iio_priv(indio_dev);
937 struct device *dev = regmap_get_device(data->regmap);
938 int ret;
939 unsigned int bit, i = 0;
940
941 guard(mutex)(&data->mutex);
942 ret = hx9023s_sample(data);
943 if (ret) {
944 dev_warn(dev, "sampling failed\n");
945 goto out;
946 }
947
948 ret = hx9023s_get_prox_state(data);
949 if (ret) {
950 dev_warn(dev, "get prox failed\n");
951 goto out;
952 }
953
954 for_each_set_bit(bit, indio_dev->active_scan_mask, indio_dev->masklength)
> 955 data->buffer.channels[i++] = data->ch_data[indio_dev->channels[bit].channel].diff;
956
This looks very odd. Diff is an int filled with get_unaligned_le16()
which you then write to a __be16 here.
It should remain little endian, if that is appropriate, throughout.
Also, very long line. Use a local variable for
indio_dev->channels[bit].channel.
Hi Jonathan,
I reviewed the code and saw that data->buffer.channels[i] needs to be
filled with the MSB and LSB of the diff data register. I can read the
two bytes of diff data using regmap_bulk_read and fill
data->buffer.channels[i]. However, the diff data register in this chip
is multiplexed with the low pass data register. Thus, in some cases,
diff data can't be directly read and must be calculated as the
difference between low pass data and baseline data. Therefore, I can't
directly store the register value in data->buffer.channels[i]. I plan to
make the following changes to the code. Do you think this is feasible?
@@ -141,7 +141,7 @@ struct hx9023s_data {
bool trigger_enabled;
struct {
- __be16 channels[HX9023S_CH_NUM];
+ __le16 channels[HX9023S_CH_NUM];
s64 ts __aligned(8);
} buffer;
@@ -936,7 +936,7 @@ static irqreturn_t hx9023s_trigger_handler(int irq,
void *private)
struct hx9023s_data *data = iio_priv(indio_dev);
struct device *dev = regmap_get_device(data->regmap);
int ret;
- unsigned int bit, i = 0;
+ unsigned int bit, index, i = 0;
guard(mutex)(&data->mutex);
ret = hx9023s_sample(data);
@@ -951,8 +951,10 @@ static irqreturn_t hx9023s_trigger_handler(int irq,
void *private)
goto out;
}
- for_each_set_bit(bit, indio_dev->active_scan_mask,
indio_dev->masklength)
- data->buffer.channels[i++] =
data->ch_data[indio_dev->channels[bit].channel].diff;
+ for_each_set_bit(bit, indio_dev->active_scan_mask,
indio_dev->masklength) {
+ index = indio_dev->channels[bit].channel;
+ data->buffer.channels[i++] =
cpu_to_le16(data->ch_data[index].diff);
+ }
iio_push_to_buffers_with_timestamp(indio_dev, &data->buffer,
pf->timestamp);
Best regards,
Yasin Lee
957 iio_push_to_buffers_with_timestamp(indio_dev, &data->buffer, pf->timestamp);
958
959 out:
960 iio_trigger_notify_done(indio_dev->trig);
961
962 return IRQ_HANDLED;
963 }
964
